Repository navigation
[3318 by Claude Opus 5.5] Separate font readiness from creation-scroll geometry settling - #3319
Conversation
… 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.
|
/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. |
|
✅ AI Code Review Complete requested by @integry Posted 1 review: |
🔍 AI Code Review — codex-astraOverall EvaluationThe 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 — ✅ 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 blockersNo merge blockers. SuggestionsThese are optional follow-ups and are not sent to S1: 🟢 Exercise cumulative loading transitionsIn S2: 🟢 Release held fonts on failureConsider releasing the probe’s font gate in a S3: 🟢 Record cross-browser control resultsThe 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. ScoreThe 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
Copy and edit to choose records from this review: 🤖 Review by ProPR |
AI Implementation Summary
Closes #3318
Branch:
3318/claude-opus-5-5-separate-font-readiness-f-20261021-1710-eleCommits: ✅ 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:document.fonts.readyno longer counts against the 5-second limit, which is unchanged and now exported asSETTLE_BUDGET_MS. The wait for a font is bounded only by the test's own timeout.No dependence on Google Fonts:
e2e/creation-short-window.pw.tsnow serves the repo's local copy of Inter (the existingserveInterhelper) 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:Test results:
creation-short-window.pw.ts(28 cases)creation-scroll-settled.pw.ts(3 cases)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 -> loadedsample. Test 2 passes because the original helper already failed on real movement. ESLint andtsc --noEmitare 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:
f7038ea0-37ff-47d1-9ee6-5e5558c8ec9bLog files stored at:
/usr/src/app/logs/executions/issue-3318-2026-10-21T17-34-57-245Z-conversation.json/usr/src/app/logs/executions/issue-3318-2026-10-21T17-34-57-245Z-output.txtLatest Conversation Messages
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!