Repository navigation
fix(web): stop reading anchor from undoing a pending scroll; drop duplicated settings spec - #919
Merged
Merged
Conversation
…oll event The reading anchor restored a stale position whenever a commit ran between a scroll and its scroll event, snapping the reader back (flaky session-history e2e). Restore now only corrects layout shifts at the captured scroll position. Delete the jsdom 'saves every result display and footer control' spec: it duplicates settings-parity.e2e.ts:414 step for step, was the slowest spec in the file (5s timeouts on Windows), and its leaked continuation after a timeout broke the next test. Keep the unique 'configure not called' check in the routing spec.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
README PR #918 went red on two flaky checks (run 42630501387). The same code was green on
main(6b18c71). This PR handles both at the root. It does not add retries, widen timeouts or rerun CI.Verdicts
session-history.e2e.ts:340"prefetched native history loads on upward reading…" (toBeInViewport, ratio 0)scrollToandscrollTopon.conversationcaptured the bad run: the test'sscrollTo(0), then 88 ms lateruseReadingAnchor.restore()(in a layout effect) calledscrollTo(645)before any scroll event fired. The anchor was captured at the old position, and a render landed between the scroll and its event, so it pulled the reader back. Real wheel scrolls go through the same path.provider-settings-page.spec.ts:1564"saves every result display and footer control…" (5000 ms timeout on Windows; 5040 ms before)settings-parity.e2e.ts:414step for step: the same three display selects → Full, footer style → Powerline Mono, footer switch off, then a rejected save that keeps the value. The e2e also asserts the exact patch payloads and a reload round-trip against a real browser. It was the slowest jsdom test in the file (2.2 s on Linux vs ~0.7 s for its neighbours). Its only unique assertion, that a display change never calls setup/configure, now lives in the spec below.provider-settings-page.spec.ts:1636"routes agent controls through canonical setup…" (expected 1 preferences call, got 2)fetchstub. Reproduced on Linux with--testTimeout=1000: the previous test timed out and this one got 3 calls. It is the only test that protects an unsaved concurrency draft across a preference save and the configure prompt text. AddedtoHaveBeenCalledTimes(1).Changes
use-reading-anchor.ts: the anchor recordsscrollTopat capture time.restore()only corrects layout shifts at that same scroll position. If the scrollport has moved since capture (scroll event still pending), it leaves the position alone, and the layout effect recaptures.reading-anchor.spec.ts: new test. A layout shift at the same position is still corrected, and a scroll that lands before its event survives a re-render. Mutation check: without the fix it fails withexpected 370 to be 0.provider-settings-page.spec.ts: deleted the duplicate and strengthened the routing test.web/dist: rebuilt.Verification
bun run check && bun run test: 109 files, 1647 tests passing.toBeInViewportfailures. One run hit the 30 s overall test budget under that overload; its trace shows the viewport assertion passed and the run was still progressing.