You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
test(scripts): settle four flaky browser checks on what they test - #2632
Four browser scripts failed now and then in "Browser regression (other scripts)" on main (run 41558343287 on c0cdc8e, and later runs). The take-over failure was a product race, fixed in #2630. This PR fixes the other three, plus the relay-media failure seen on bef2be1. Test-only: no product code changes.
chat-local-send-follow-demo-regression.ts:95: "the reader moves away"
wheelUp waited for each wheel notch to lower scrollTop by more than 20 px. A write trace on the page showed every notch moving the view 120 px. Then, as a row above the view was measured, LegendList's ScrollAdjust called scrollBy(+103) to keep the reader's rows in place, with scrollHeight growing by the same 103 px. That undid the notch in scrollTop, but the distance from the end stayed exactly 600 px before and after. The reader had moved away; the wait could not see it. The wait now measures the distance from the end, which that adjustment leaves alone.
Not a #2624 regression. An A/B of the unmodified script, alternating commits under dori heavy, gave passes/runs of: 05c9133 (before #2624) 5/7, 9ef34f8 (#2624) 5/6, main 2/6. The step fails before #2624 too; #2624 changes when rows get measured, so it lands more often. No scrollToEnd or follow write appears in the trace.
file-viewer-regression.ts: "video not visible"
The CI failure came right after setFileOpen("panel") reloads the page: videoLink.click() was a pointer click on a transcript link while the reloaded chat followed to its end, so the click landed on nothing and no popup opened (0 "locator resolved" lines on CI and locally). The script's own header comment describes this trap, and its other steps already use activateInTranscript. Under load, three more steps hit the same trap: after a goto (big code), after the panel closes (the extensionless and unknown-text loop, which keeps the link focused so the focus-return assertion still holds), and after a reload and Back at 390 px. The click on the framed page's table-of-contents link is dispatched to the link: the frame's own navigation is under test, not where the panel lays the frame out.
files-prompt-regression.ts:71: Escape left Files open after a pane switch
Not reproduced in about 435 local attempts (15 per run, fresh pages, 6x CPU throttle, load up to 92), and nothing found by reading the code. The Escape check stays: it is the only place a script checks Escape closing Files. When it fails now, the script writes what decided it to the log and to files-<page>-escape-diagnostics.json in the browser-evidence artifact: whether a window keydown probe saw the key, the focused element, the modal and dialog stack, whether FilesDialog stood down (file or diff over it, native modal), and any open picker. Tracked as #2631.
Seen inside relay-e2e-regression.ts on bef2be1 (run 41568570526). The stubbed writeText always resolves, so the click never reached Copy link. copy.click({ force: true }) is aimed at where the button was, under a frozen browser clock, while the viewer may still move it. The click is now dispatched to the button itself; the clipboard listener is still subscribed before it.
Validation
All runs went through dori heavy; host load ranged from about 4 to 92 (70 CPU hogs for part of it).
chat-local-send-follow: 22/22 consecutive.
file-viewer: 20 consecutive and counting on the final script (bun run check run, isolated herdr session). Earlier rounds under load caught the other lost-click steps fixed here.
files-prompt: 22/22 consecutive. A forced failure (key withheld) wrote the diagnostics JSON and screenshot. The review's try/catch, which keeps the original timeout as the reported error, came after those runs; it runs only when the check fails.
relay-media (fixed): 22/22 consecutive; the unmodified script passed 10/10 locally, so CI is the only evidence for that one.
omo code reviewer: APPROVE, LOW notes only. Applied: the diagnostics never replace the original error. Not changed: browser-other uploads PNGs only on failure; the 390 px reopen dispatches directly because activateInTranscript waits for a frame that section lacks; the wheelUp callers assert the distance afterwards.
- chat-local-send-follow: a wheel notch away from the end is measured as the distance from the end.
LegendList keeps the reader's rows in place as rows above the view are measured, which can undo
a notch's move in scrollTop (traced: scrollBy(+103) with scrollHeight +103, the distance from the
end unchanged).
- file-viewer: transcript links right after a reload, a goto or a panel closing are activated where
they are, as the script's own header asks; the framed page's table-of-contents click is
dispatched to its link.
- files-prompt: the Escape after a pane switch stays, and its failure now writes what decided it
(keydown probe, focus, modal stack, FilesDialog's stand-down state) to the log and evidence (#2631).
- relay-media: Copy link is clicked on the button, not at where a forced click aims.
No actionable comments were generated in the recent review.
Summary
Summary by CodeRabbit
Tests
Updated automated checks for chat scrolling, file links, and media link copying to use more consistent interaction steps.
Added diagnostic capture when a Files dialog check times out, including a screenshot and relevant interface state, to make failures easier to investigate.
The regression scripts update wheel-scroll checks and transcript-link activation. The Files dialog test adds timeout diagnostics and a screenshot-directory fallback. The media-browser QA test dispatches the Copy link click and continues to verify the copied URL.
Changes: Chat scroll regression
Layer / File(s)
Summary
Measure wheel-up movement scripts/chat-local-send-follow-demo-regression.ts
The wheel-up check now waits for the distance from the chat’s end to grow by more than 20 pixels. It no longer compares scrollTop.
The screenshot directory falls back to UI_EVIDENCE_DIR when FILES_PROMPT_SHOTS is unset or empty. On timeout, the test logs UI and keyboard diagnostics, saves JSON and a screenshot, and rethrows the original timeout.
Media browser QA
Layer / File(s)
Summary
Dispatch Copy link click scripts/relay-media-browser-qa.ts
The test dispatches a click event instead of using a forced click. It still verifies the copied URL.
The changes stabilize browser-test activation and improve failure diagnostics without changing product behavior. No actionable issue remains that should block merging after normal checks.
Pre-merge checks: 4 · 1
Failed checks: 1 warning
Check name
Status
Explanation
Resolution
Docstring Coverage
Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…
Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed checks: 4 passed
Check name
Status
Explanation
Title check
The title names the test-only change and the four flaky browser checks it settles. It matches the scope in the raw summary and objectives. It is concise and specific, and it does not list files, so it…
Description check
The description covers the problem, the fix for each of the four scripts, and the validation runs with results. It follows the template's Change and Validation sections. It names the skipped checks (f…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Title check
Explanation
The title names the test-only change and the four flaky browser checks it settles. It matches the scope in the raw summary and objectives. It is concise and specific, and it does not list files, so it passes.
Full details: Description check
Explanation
The description covers the problem, the fix for each of the four scripts, and the validation runs with results. It follows the template's Change and Validation sections. It names the skipped checks (full check, Windows VM). The changelog note is stated as not needed. The template's CONTRIBUTING.md reference is not repeated, but the required content is present, so the check passes.
Full details: Docstring Coverage
Explanation
Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 2 functions across 4 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Finishing Touches
Generate docstrings
Commit to this branch
Create a new PR
Generate unit tests (beta)
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Reviewing files that changed from the base of the PR and between bef2be1 and 6069b0a.
Files selected for processing: 4
scripts/chat-local-send-follow-demo-regression.ts
scripts/file-viewer-regression.ts
scripts/files-prompt-regression.ts
scripts/relay-media-browser-qa.ts
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.
Test-only fix for four browser scripts that rotated failures across main's CI runs (lane browser-flakes; no product code). Merge-pass read of the whole diff:
chat-local-send-follow-demo-regression.ts: each wheel notch is measured as the distance from the end rather than scrollTop, because LegendList raises scrollTop by the growth of rows measured above the view to keep the reader's rows in place; the assertion (the reader moved up by more than 20 px per notch) is unchanged. The lane's A/B in the PR shows it isn't a fix(chat): follow measured rows without a competing glide #2624 regression.
file-viewer-regression.ts: links in a chat that is still following its end after a reload, go-back or a panel closing are activated where they are (activateInTranscript / dispatchEvent), since the pointer could miss a link that moved; what each step asserts afterwards (popup vs panel, history entries, focus return) is unchanged.
relay-media-browser-qa.ts: the Copy link click is dispatched to the button instead of a forced click at its old position.
Each script ran 20+ times in a row under load on the lane's side; omo approved (LOW only). A test-only change gets no CodeRabbit round; it merges on its checks.
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
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.
Change
Four browser scripts failed now and then in "Browser regression (other scripts)" on main (run 41558343287 on c0cdc8e, and later runs). The take-over failure was a product race, fixed in #2630. This PR fixes the other three, plus the relay-media failure seen on bef2be1. Test-only: no product code changes.
chat-local-send-follow-demo-regression.ts:95: "the reader moves away"wheelUpwaited for each wheel notch to lowerscrollTopby more than 20 px. A write trace on the page showed every notch moving the view 120 px. Then, as a row above the view was measured, LegendList'sScrollAdjustcalledscrollBy(+103)to keep the reader's rows in place, withscrollHeightgrowing by the same 103 px. That undid the notch inscrollTop, but the distance from the end stayed exactly 600 px before and after. The reader had moved away; the wait could not see it. The wait now measures the distance from the end, which that adjustment leaves alone.Not a #2624 regression. An A/B of the unmodified script, alternating commits under
dori heavy, gave passes/runs of: 05c9133 (before #2624) 5/7, 9ef34f8 (#2624) 5/6, main 2/6. The step fails before #2624 too; #2624 changes when rows get measured, so it lands more often. NoscrollToEndor follow write appears in the trace.file-viewer-regression.ts: "video not visible"The CI failure came right after
setFileOpen("panel")reloads the page:videoLink.click()was a pointer click on a transcript link while the reloaded chat followed to its end, so the click landed on nothing and no popup opened (0 "locator resolved" lines on CI and locally). The script's own header comment describes this trap, and its other steps already useactivateInTranscript. Under load, three more steps hit the same trap: after agoto(big code), after the panel closes (the extensionless and unknown-text loop, which keeps the link focused so the focus-return assertion still holds), and after a reload and Back at 390 px. The click on the framed page's table-of-contents link is dispatched to the link: the frame's own navigation is under test, not where the panel lays the frame out.files-prompt-regression.ts:71: Escape left Files open after a pane switchNot reproduced in about 435 local attempts (15 per run, fresh pages, 6x CPU throttle, load up to 92), and nothing found by reading the code. The Escape check stays: it is the only place a script checks Escape closing Files. When it fails now, the script writes what decided it to the log and to
files-<page>-escape-diagnostics.jsonin the browser-evidence artifact: whether a window keydown probe saw the key, the focused element, the modal and dialog stack, whether FilesDialog stood down (file or diff over it, native modal), and any open picker. Tracked as #2631.relay-media-browser-qa.ts:44: "Timed out: copied-link clipboard event"Seen inside
relay-e2e-regression.tson bef2be1 (run 41568570526). The stubbedwriteTextalways resolves, so the click never reached Copy link.copy.click({ force: true })is aimed at where the button was, under a frozen browser clock, while the viewer may still move it. The click is now dispatched to the button itself; the clipboard listener is still subscribed before it.Validation
All runs went through
dori heavy; host load ranged from about 4 to 92 (70 CPU hogs for part of it).bun run check run, isolated herdr session). Earlier rounds under load caught the other lost-click steps fixed here.bun run check fast: success on 6069b0a. An earlier run caught a regression in this branch:ci-browser.test.tsforbids??on evidence dirs (The browser lane loses its evidence folder before omo-plan-browser-regression in bun run check full #1554). Fixed with||.activateInTranscriptwaits for a frame that section lacks; thewheelUpcallers assert the distance afterwards.No changelog fragment: test-only.