Skip to content

[3318 by Claude Opus 5.5] Separate font readiness from creation-scroll geometry settling - #3319

Merged
integry merged 1 commit into
mainfrom
3318/claude-opus-5-5-separate-font-readiness-f-20261021-1710-ele
Oct 21, 2026
Merged

integry merged 1 commit into
mainfrom
3318/claude-opus-5-5-separate-font-readiness-f-20261021-1710-ele

Conversation

@propr-dev

@propr-dev propr-dev Bot commented Oct 21, 2026

Copy link
Copy Markdown
Contributor

AI Implementation Summary

Closes #3318

Branch: 3318/claude-opus-5-5-separate-font-readiness-f-20261021-1710-ele
Commits: ✅ Changes committed (6785873)


AI Processing Completed

Execution Details:

Summary:
I've fixed the creation-scroll helper so that a slow font download no longer counts as a moving layout. Real layout movement still fails it within 5 seconds. Only test files changed; no product UI, dependency, permission or credential changes.

The fix, in e2e/creation-scroll.fixture.ts:

  • Waiting for a web font is now a separate step from timing the layout. Time spent waiting for document.fonts.ready no longer counts against the 5-second limit, which is unchanged and now exported as SETTLE_BUDGET_MS. The wait for a font is bounded only by the test's own timeout.
  • When a font arrives, the helper still takes fresh measurements: the font status is part of each sample, so the layout after arrival must again hold still for two animation frames.
  • Layout that keeps moving still fails, both before and after a late font. There are no retry loops, no longer deadlines, and no failures swallowed.

No dependence on Google Fonts: e2e/creation-short-window.pw.ts now serves the repo's local copy of Inter (the existing serveInter helper) for every case, so how fast the external font arrives no longer affects any test. The assertions, timeouts, trusted input and the controlled late-font cases, including their stale-pointer check, are unchanged.

New regression tests, in e2e/creation-scroll-settled.pw.ts. Each one controls when the font file is delivered:

  1. Font held 6.5 seconds, past the 5-second limit: the helper keeps waiting without failing. After the font is released, it waits out 12 frames of movement triggered by the arrival before reporting stable. The font re-wraps the text, so the layout really does change.
  2. Layout that moves every frame: the helper fails with "the page kept moving" after 5 to 8 seconds.
  3. Movement after a late font: the helper fails, and the 5-second count starts when the font arrives.

Test results:

Run Chromium WebKit
creation-short-window.pw.ts (28 cases) 28 passed 28 passed
creation-scroll-settled.pw.ts (3 cases) 3 passed 3 passed
New tests against the original helper 1 and 3 fail, 2 passes 1 and 3 fail, 2 passes

All runs had no retries, skips or flaky results. Against the original helper, tests 1 and 3 fail exactly as in the issue, on the first loading -> loaded sample. Test 2 passes because the original helper already failed on real movement. ESLint and tsc --noEmit are clean.

The WebKit runs used a WebKit build with system libraries unpacked by hand in this container, and a temporary config that I've since deleted. I didn't touch the clock reliability files from #3316. There are no visual previews because nothing a user sees has changed.

Detailed Logs:

  • Conversation: 85 messages
  • Session: f7038ea0-37ff-47d1-9ee6-5e5558c8ec9b

Log files stored at:

  • conversation: /usr/src/app/logs/executions/issue-3318-2026-10-21T17-34-57-245Z-conversation.json
  • output: /usr/src/app/logs/executions/issue-3318-2026-10-21T17-34-57-245Z-output.txt
Latest Conversation Messages
ASSISTANT: [content unavailable]

ASSISTANT: I've fixed the creation-scroll helper so that a slow font download no longer counts as a moving layout. Real layout movement still fails it within 5 seconds. Only test files changed; no product UI, de...


This PR was created automatically by ProPR after processing issue #3318.


💡 Need changes?

Comment on this PR to request refinements — the AI agent monitors comments and will update the implementation based on your feedback. Keep iterating until you're satisfied!

… longer counts as a moving layout. Real layout movement still fails it within 5 seconds. Only test files changed; no product UI, dependency, permission or credential changes.

The fix, in `e2e/creation-scroll.fixture.ts`:
- Waiting for a web font is now a separate step from timing the layout. Time spent waiting for `document.fonts.ready` no longer counts against the 5-second limit, which is unchanged and now exported as `SETTLE_BUDGET_MS`. The wait for a font is bounded only by the test's own timeout.
- When a font arrives, the helper still takes fresh measurements: the font status is part of each sample, so the layout after arrival must again hold still for two animation frames.
- Layout that keeps moving still fails, both before and after a late font. There are no retry loops, no longer deadlines, and no failures swallowed.

No dependence on Google Fonts: `e2e/creation-short-window.pw.ts` now serves the repo's local copy of Inter (the existing `serveInter` helper) for every case, so how fast the external font arrives no longer affects any test. The assertions, timeouts, trusted input and the controlled late-font cases, including their stale-pointer check, are unchanged.

New regression tests, in `e2e/creation-scroll-settled.pw.ts`. Each one controls when the font file is delivered:
1. Font held 6.5 seconds, past the 5-second limit: the helper keeps waiting without failing. After the font is released, it waits out 12 frames of movement triggered by the arrival before reporting stable. The font re-wraps the text, so the layout really does change.
2. Layout that moves every frame: the helper fails with "the page kept moving" after 5 to 8 seconds.
3. Movement after a late font: the helper fails, and the 5-second count starts when the font arrives.

Test results:

| Run | Chromium | WebKit |
|---|---|---|
| `creation-short-window.pw.ts` (28 cases) | 28 passed | 28 passed |
| `creation-scroll-settled.pw.ts` (3 cases) | 3 passed | 3 passed |
| New tests against the original helper | 1 and 3 fail, 2 passes | 1 and 3 fail, 2 passes |

All runs had no retries, skips or flaky results. Against the original helper, tests 1 and 3 fail exactly as in the issue, on the first `loading -> loaded` sample. Test 2 passes because the original helper already failed on real movement. ESLint and `tsc --noEmit` are clean.

The WebKit runs used a WebKit build with system libraries unpacked by hand in this container, and a temporary config that I've since deleted. I didn't touch the clock reliability files from #3316. There are no visual previews because nothing a user sees has changed.
@integry

integry commented Oct 21, 2026

Copy link
Copy Markdown
Owner

/review codex-astra

!gitfix Independent Astra review of exact head 6785873 for #3318. Test-only repair following a preserved 6.5-second external-font delay in frozen Linux/mobile acceptance. Verify font readiness is bounded by the test timeout, geometry still has a cumulative five-second budget across loading transitions, and persistent or post-font movement cannot be hidden. Assess the three new controls, local Inter routing and preservation of all 28 short-window journeys, trusted gestures, action visibility, caret and draft assertions, controlled late-font reflow and existing timeouts. Check failure-path cleanup and cross-browser behavior. No production code changes, retries, skipped cases or increased timeouts. Review only; root runtime validation and frozen-preview merge timing are separate. Do not merge, publish or deploy.

@propr-dev

propr-dev Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor Author

✅ AI Code Review Complete requested by @integry

Posted 1 review:

View Task Details

@propr-dev

propr-dev Bot commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

🔍 AI Code Review — codex-astra

Overall Evaluation

The repair is focused and appears merge-ready within the reviewed scope, conditional on the pending Validate Changes check. No verified merge blocker was found through static review. No commands were run; runtime and cross-browser behavior were not independently validated. The issue excerpt is truncated, so requirement assessment uses its visible text and the explicit review request.

✅ Separate readiness budget — settled excludes time awaiting document.fonts.ready while retaining accumulated geometry time in spent across loading transitions. Font readiness remains bounded by the enclosing test timeout.

✅ Reachability coverage preserved — All 28 short-window journeys remain, including trusted gestures, whole-action visibility, caret and draft assertions, and controlled reflow. The diff changes no production code, retries, skips, or existing timeouts.

✅ Targeted regression controls — The three new tests exercise a font held beyond five seconds, persistent movement without a pending font, and persistent movement after delayed font delivery. Local Inter routing matches the application’s stylesheet and font origins.

Merge blockers

No merge blockers.

Suggestions

These are optional follow-ups and are not sent to /fix.

S1: 🟢 Exercise cumulative loading transitions

In propr-ui/e2e/creation-scroll-settled.pw.ts, add an optional control that spends part of the geometry budget moving, starts another font-loading interval, then resumes movement. The implementation preserves spent correctly by inspection, but the current controls would not distinguish cumulative accounting from resetting the entire budget after each font wait.

S2: 🟢 Release held fonts on failure

Consider releasing the probe’s font gate in a finally block, including when probe initialization fails. An assertion before release() currently leaves the route callback awaiting an unresolved promise until teardown. Playwright’s isolated page/context lifecycle limits the consequence; explicit release would make failure-path cleanup clearer. The existing .catch() handles fulfillment against a closed page, but does not release the gate itself.

S3: 🟢 Record cross-browser control results

The supplied configuration runs Chromium. Running the new controls explicitly under Firefox and WebKit would strengthen confidence in their delayed-font status assertions and post-font animation-frame ordering, particularly given the documented WebKit font behavior in the surrounding context. This is additional validation, not a demonstrated defect in the changed implementation.

Score

The implementation satisfies the visible objective and preserves acceptance coverage, with no verified code blocker or reported current-head check failure. Static review supports approval within scope; runtime validation remains separate and one check is pending.

Score: 9/10


🤖 Review Details

  • Model: GPT-6 Astra
  • Time: 31s
  • Tokens: 36,564 (35,856 in / 708 out)
  • Cost: $0.28

View Task


💡 Next step: Comment /fix to address every F# merge blocker, /fix all for every pending blocker and suggestion, or name records explicitly, as in /fix F3 S5.
F# and S# IDs increment across review comments and remain permanent, so selectors such as /fix F3 F5 stay unambiguous across cycles. S# suggestions stay optional: they are implemented only when you name them or request /fix all, and they never relax a merge blocker.

Copy and edit to choose records from this review:

/fix S1 S2 S3

🤖 Review by ProPR

@integry
integry merged commit 42931b1 into main Oct 21, 2026
52 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Separate font readiness from creation-scroll geometry settling

1 participant