Repository navigation
[3316 by Claude Opus 5.5] Make lazy-route browser clock pausing deterministic - #3317
Conversation
![propr-dev[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
…thing else changed. The full matrix passes in Chromium and WebKit: 24/24 in each (the 21 original cases plus 3 new ones), with zero retries. Cause. `pauseAt` moves the clock forward to an absolute time and rejects any target in the past. The fake clock follows real time until the pause actually runs in the page, so "sampled time + 50 ms" is already in the past whenever the command takes longer than 50 ms. In the failing run it took about 106 ms. Fix. The pause helper now uses three supported clock calls instead of a margin: 1. Read the page's current time `t`. 2. `page.clock.setFixedTime(t)` freezes the page's Date at `t`. Timers keep running on real time, which is the same as before the pause lands. 3. `page.clock.pauseAt(t)` now has to move the clock forward by exactly 0 ms, however late it arrives. It can't land in the past, and unlike the old +50 ms it can't fire a pending timer early. 4. `page.clock.setSystemTime(t)` releases Date while the clock is paused, so it stays stopped at `t`. Everything else in the file is unchanged: the 400/300 ms production debounces, the 1,000 ms advance, the held real Settings chunk, the input, URL and focus checks, the Settings and Back assertions, and the timeouts. I made no production, backend or dependency changes. Nothing needed a browser-specific workaround: Chromium and WebKit run the same code. Latency proof (criterion 4). The helper takes an optional `stall` hook that runs between each step. One new control case per viewport passes a fixed 250 ms wait at every step, which is more than the old 50 ms margin and the observed 106 ms. That case then: - checks the page clock reads exactly `t` after the pause. - types a Tasks search and checks the clock is still at `t` and the URL has no `search=`. - runs the full Settings navigation and Back checks. This is a fixed delay, not a timing benchmark, and nothing is retried. I also ran two throwaway checks that I didn't keep in the repo: - Old helper under the same 250 ms stall: it fails every time with `Cannot fast-forward to the past`. - Debounce still queued: for Tasks, Plans and Goals, after the new pause the typed search didn't reach the URL after 1.5 s of real time. Advancing the clock 1,000 ms while staying on the list then did write it, so the debounce was waiting, not dropped. How it was checked: - Chromium: the default config. - WebKit: a temporary config, now deleted, using browser libraries I installed locally in this container. - Lint and types: eslint with `--max-warnings 0` is clean, and the file has no type errors. These runs used this container, not your validation image or the 2-CPU/2-GiB setup from the issue. There are no previews because the change isn't visible to users.
|
/review codex-astra !gitfix Independent Astra review of exact head 478f46b. Test-only clock reliability change for #3316. Please verify supported Playwright clock semantics, the setFixedTime/pauseAt/setSystemTime sequence under command latency, and whether any pending timer can be fired or suppressed in a way that weakens the original 21 lazy-route navigation cases. Assess the 3 new latency controls and preservation of trusted input, held Settings chunk, 1000-ms advance, URL/focus/Back assertions, viewports and deadlines. Look for false confidence from pinning Date while timers still tick before pause. No production code changes, retries, skipped cases or inflated timeouts should be introduced. Review only; operator validation and frozen-preview merge timing remain separate. Do not merge, publish or deploy. |
|
✅ AI Code Review Complete requested by @integry Posted 1 review: |
🔍 AI Code Review — codex-astraOverall EvaluationMerge-ready within the supplied scope, conditional on the five pending checks completing successfully. No PR-introduced blocker was identified. This is a static review of the supplied diff and context; no commands, runtime tests, or independent inspection of Playwright 1.62.1 were performed. The linked issue excerpt is truncated, so this assessment covers its visible requirements. ✅ Correct pause sequencing — Pinning Date before targeting that same time with ✅ Navigation coverage preserved — All 21 original cases retain their trusted input, held Settings chunk, 1,000-ms advancement, URL and Back assertions, applicable focus assertions, and three viewports. The diff changes no production code, retries, skips, or timeouts. ✅ Relevant latency controls — The three added cases insert 250-ms delays between helper steps, then type while paused and exercise the existing delayed-navigation assertions. Timers can still fire before Merge blockersNo merge blockers. SuggestionsThese are optional follow-ups and are not sent to S1: 🟢 Directly verify timer preservationOptionally add a small clock-specific control that schedules a sentinel timer while paused, confirms it remains pending through real elapsed time, and verifies it fires after the intended clock advancement. The new Date equality assertions alone cannot establish timer behavior, and a missing search callback could leave the navigation assertions passing. This would strengthen positive evidence for timer preservation; the supplied sequence does not demonstrate timer suppression, so this is not a merge requirement. S2: 🟢 Clarify the helper’s timing guaranteeIn ScoreThe localized change preserves the original assertions and adds useful latency coverage, with no demonstrated regression. Twelve checks passed, five remain pending, and this review did not independently execute the browser matrix or inspect the pinned clock implementation. Score: 8/10 🤖 Review Details
Copy and edit to choose records from this review: 🤖 Review by ProPR |
AI Implementation Summary
Closes #3316
Branch:
3316/claude-opus-5-5-make-lazy-route-browser-c-20261021-1655-zfuCommits: ✅ Changes committed (478f46b)
AI Processing Completed
Execution Details:
Summary:
The fix is in
propr-ui/e2e/list-search-lazy-navigation.pw.tsand nothing else changed. The full matrix passes in Chromium and WebKit: 24/24 in each (the 21 original cases plus 3 new ones), with zero retries.Cause.
pauseAtmoves the clock forward to an absolute time and rejects any target in the past. The fake clock follows real time until the pause actually runs in the page, so "sampled time + 50 ms" is already in the past whenever the command takes longer than 50 ms. In the failing run it took about 106 ms.Fix. The pause helper now uses three supported clock calls instead of a margin:
t.page.clock.setFixedTime(t)freezes the page's Date att. Timers keep running on real time, which is the same as before the pause lands.page.clock.pauseAt(t)now has to move the clock forward by exactly 0 ms, however late it arrives. It can't land in the past, and unlike the old +50 ms it can't fire a pending timer early.page.clock.setSystemTime(t)releases Date while the clock is paused, so it stays stopped att.Everything else in the file is unchanged: the 400/300 ms production debounces, the 1,000 ms advance, the held real Settings chunk, the input, URL and focus checks, the Settings and Back assertions, and the timeouts. I made no production, backend or dependency changes. Nothing needed a browser-specific workaround: Chromium and WebKit run the same code.
Latency proof (criterion 4). The helper takes an optional
stallhook that runs between each step. One new control case per viewport passes a fixed 250 ms wait at every step, which is more than the old 50 ms margin and the observed 106 ms. That case then:tafter the pause.tand the URL has nosearch=.This is a fixed delay, not a timing benchmark, and nothing is retried.
I also ran two throwaway checks that I didn't keep in the repo:
Cannot fast-forward to the past.How it was checked:
--max-warnings 0is clean, and the file has no type errors.These runs used this container, not your validation image or the 2-CPU/2-GiB setup from the issue. There are no previews because the change isn't visible to users.
Detailed Logs:
b6783c8e-6a58-4b5a-a6ab-f43413dd495dLog files stored at:
/usr/src/app/logs/executions/issue-3316-2026-10-21T17-07-25-255Z-conversation.json/usr/src/app/logs/executions/issue-3316-2026-10-21T17-07-25-255Z-output.txtLatest Conversation Messages
24 passed (2.6m)
This PR was created automatically by ProPR after processing issue #3316.
💡 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!