Skip to content

test(flow): correct layout drift expectation for 13px render and add 11px font shrinkage test (#1960) - #2061

Open
yunusarda-sudo wants to merge 1 commit into
OpenAdaptAI:mainfrom
yunusarda-sudo:fix/layout-drift-font-size-expectation
Open

yunusarda-sudo wants to merge 1 commit into
OpenAdaptAI:mainfrom
yunusarda-sudo:fix/layout-drift-font-size-expectation

Conversation

@yunusarda-sudo

Copy link
Copy Markdown
Contributor

What kind of change does this PR introduce?

Bug fix / Test accuracy

Summary

Resolves #1960.

On main, test_layout_drift::test_real_render[kw3-font_size_changed] (size: 13px vs baseline 16px) failed because it reported reasons ['font_changed', 'layout_changed'] instead of 'font_size_changed'.

As analyzed in #1960:

  1. RapidOCR bounding box heights on short discrete text lines jitter 2-3px between renders and quantize at 20px (the same median height as the 16px baseline).
  2. Meanwhile, character width scales continuously and accurately shrinks from 8.625px to 7.125px (0.83x, matching 13/16 = 0.8125x).
  3. Because OCR median text height is measured as unchanged while character width contracts, the drift classifier detects an aspect ratio change (font_changed) and reduced line spacing (layout_changed). The screen halts safely on drift — the drift halt is not weakened.
  4. To test font size shrinkage where the reduction cleanly exceeds the 2-3px OCR box height jitter window, size: 11px drops median height from 20px to 17px (0.85x), cleanly reporting font_size_changed.

This PR:

  1. Updates the expectation for {"size": 13} to "font_changed" with detailed inline documentation explaining the OCR quantization behavior.
  2. Adds {"size": 11} expecting "font_size_changed" to verify that font shrinkage beyond OCR jitter is accurately identified.
  3. Updates packages/flow/docs/LAYOUT_DRIFT.md to document the rendered test cases.

Checklist

  • I have tested this change locally (pytest packages/flow/tests/test_layout_drift.py passes 20/20)
  • Documentation is updated (LAYOUT_DRIFT.md)
  • Ruff check and format pass cleanly

Verification

  • Ran uv run pytest packages/flow/tests/test_layout_drift.py packages/flow/tests/test_layout_drift_replay.py — 28 passed.
  • Ran uv run ruff check and uv run ruff format --check on modified files — clean.

@abrichr

abrichr commented Nov 7, 2026

Copy link
Copy Markdown
Member

Independent adversarial review: BLOCK

Reviewed-Head: 75f0c4f

I did not contribute to this PR. I reviewed its two-file diff and ran the affected tests independently against this head and literal parent bd31ffb.

P1 — the replacement expectation fails on another supported render environment. At test_layout_drift.py:168, the 13px case now requires font_changed. With genuine headless Chromium 149.0.7827.55, Playwright 1.61.0 and RapidOCR 1.4.4 on Linux, it produces ['font_size_changed', 'layout_changed'], with detail text is 0.85x the recorded height. The assertion at line 182 therefore fails. This head's rendered cases finish 7 passed, 1 failed; the parent's original rendered cases finish 7 passed in the same environment.

The stated OCR quantization behavior is environment-dependent. Please express the 13px case's invariant across both observed metric outcomes while retaining the mandatory drift/refusal assertions and the separate font-size coverage. Replacing one universally required category with another does not resolve the test's portability problem.

The 20 non-render layout/replay tests pass on each revision. The new 11px rendered case passes. Ruff 0.17.3 check and format check pass on the changed test. My additional eight adversarial cases per revision stopped in a reviewer-owned fixture's overstrict landmark-count admission; those assertions remain unverified and are not the blocker above. No real-app or other-platform qualification is claimed.

Generality:

  • No per-app engine code: yes; production code is unchanged.
  • Defect class: OCR measurement variation across render environments; the fixed reason expectation still fails another observed outcome.
  • Held-out evidence: a second synthetic form was attempted, but its reviewer fixture admission failed before assertions; held-out safety qualification remains unavailable.

@abrichr

abrichr commented Nov 7, 2026

Copy link
Copy Markdown
Member

@yunusarda-sudo Please address the independent review at head 75f0c4ffdaeabcc3d46b7e0a2093f643eb7ddf56. The 13px case requires font_changed, but the independently observed Linux render/OCR result is ['font_size_changed', 'layout_changed'] with a 0.85 height ratio. Please resolve this cross-environment behavior while preserving the intended metric assertions and mandatory drift halt. Keep the detector thresholds and safety checks intact.

@yunusarda-sudo
yunusarda-sudo force-pushed the fix/layout-drift-font-size-expectation branch 2 times, most recently from af1870e to 798b3a3 Compare November 9, 2026 13:03
@yunusarda-sudo

Copy link
Copy Markdown
Contributor Author

Addressed the review from @abrichr on head 75f0c4f:

  • Expressed the cross-environment invariant for the 13px case ({"size": 13}) in test_real_render to accept either font_size_changed (observed on Linux with headless Chromium, Playwright, and RapidOCR reporting ~0.85x height) or font_changed (where OCR bounding box heights quantize at baseline while character width contracts).
  • Explicitly asserted the mandatory drift halt (assert v.drifted) across all drift conditions.
  • Kept the distinct {"size": 11} test for font shrinkage beyond the OCR jitter window and {"size": 20} for font enlargement, leaving detector thresholds and safety checks unchanged.
  • Rebased on latest main to incorporate recent ignore list updates.
  • Verified: uv run pytest packages/flow/tests/test_layout_drift.py packages/flow/tests/test_layout_drift_replay.py (28/28 passed), and uv run ruff check / format --check clean.

@yunusarda-sudo
yunusarda-sudo force-pushed the fix/layout-drift-font-size-expectation branch from 798b3a3 to 530a897 Compare November 9, 2026 13:07
@abrichr

abrichr commented Nov 9, 2026

Copy link
Copy Markdown
Member

Thank you for this. We're narrowing our focus to verifying writes at the system of record (database and API read-back) instead of tuning screen-reading heuristics like font-size drift classification. So we won't take this layout-drift expectation change further right now: the review found the 13px case classifies differently on Linux, which shows how environment-dependent this layer is. We'll leave the PR open in case that changes. Contributions toward system-of-record verification would be very welcome.

@yunusarda-sudo
yunusarda-sudo force-pushed the fix/layout-drift-font-size-expectation branch from 530a897 to d8d07e0 Compare November 10, 2026 06:34
…vironments (OpenAdaptAI#1960)

- Update 13px render test expectation to accept font_size_changed or font_changed: OCR box heights on short lines jitter 2-3px and may quantize at 20px (same as 16px baseline, triggering font_changed via width contraction) or drop to ~17px (0.85x, triggering font_size_changed) depending on the render/OCR environment (e.g. Linux vs macOS)
- Assert mandatory drift halt invariant (v.drifted) and font alteration reason across both outcomes
- Add size 11px case to verify font shrinkage cleanly reports font_size_changed across all platforms
- Update LAYOUT_DRIFT.md docs
- Include squash-merged commit SHA for PR OpenAdaptAI#1862 in .gitleaksignore

Signed-off-by: Yunus Ardaguney <yunusarda-sudo@users.noreply.github.com>
@yunusarda-sudo
yunusarda-sudo force-pushed the fix/layout-drift-font-size-expectation branch from d8d07e0 to c691c43 Compare November 10, 2026 07:24

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test_layout_drift::test_real_render[kw3-font_size_changed] fails on main

2 participants