Skip to content

test(desktop): settle the prompt rail across two consecutive reads #4685

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Adarsh-Me wants to merge 3 commits into from
Closed

test(desktop): settle the prompt rail across two consecutive reads #4685

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
009fd2c
test(desktop): settle the prompt rail across painted frames (#4675)
Adarsh-Me Sep 3, 2026
d4c6677
test(desktop): gate the prompt rail on painted-frame quiescence (#4675)
Adarsh-Me Sep 3, 2026
ab2484d
test(desktop): pass the rail settle loop as a function, not source (#…
Adarsh-Me Sep 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 25 additions & 8 deletions apps/desktop/e2e/prompt-rail.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,21 +189,38 @@ async function activePromptRailSnapshot(page: Page): Promise<ActivePromptRailSna
}, { promptCount: PROMPT_RAIL_PROMPT_COUNT });
}

// The transcript can still be settling right after a scroll, so one agreeing
// snapshot can pass mid-settle (#4675). Two protocol reads alone are not
// enough either: both can execute inside one rendered frame, so an update the
// rail queued on requestAnimationFrame lands only after they agreed. The rail
// resolves on the frame after a scroll and keeps re-resolving for six frames
// after a mutation (packages/ui/src/prompt-anchor-rail.tsx), so the two reads
// are separated by that whole window: agreement across six painted frames is
// a settled rail, and nothing is read after the poll that the poll did not
// see.
const RAIL_SETTLE_FRAMES = 6;

async function expectPromptRailMatchesReadingPosition(page: Page): Promise<void> {
let lastSnapshot: ActivePromptRailSnapshot | null = null;
try {
await expect.poll(async () => {
lastSnapshot = await activePromptRailSnapshot(page);
return lastSnapshot.expectedId !== null
&& lastSnapshot.currentIds.length === 1
&& lastSnapshot.currentIds[0] === lastSnapshot.expectedId;
}, { message: 'the one current tick maps from the Turn being read' }).toBe(true);
const first = await activePromptRailSnapshot(page);
await waitForPaintedFrames(page, RAIL_SETTLE_FRAMES);
const second = await activePromptRailSnapshot(page);

@hqhq1025 hqhq1025 Sep 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Require stability across a rendered frame, not two immediate protocol reads

These two page.evaluate snapshots do not establish a settle boundary. On this exact head, a frame counter in the real Electron fixture showed that 91 of 100 snapshot pairs ran in the same animation frame, and a queued requestAnimationFrame mutation could run only after both snapshots had returned the matching turn-prompt-rail-120 state. That is the same scheduling mechanism used by the production rail resolver in packages/ui/src/prompt-anchor-rail.tsx:580-614.

The practical failure remains reproducible: --workers=4 --repeat-each=40 failed this test 2/40 times at the subsequent last-tick assertion because the final tick no longer had aria-current. The helper can therefore accept a mid-settle state, and this PR has not stabilized the target test. Please require agreement across an actual painted-frame/quiescence window, then rerun the concurrent stress case.

@Adarsh-Me Adarsh-Me Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed on add2b7c. The two reads are now separated by waitForPaintedFrames(page, 6) — the same re-resolve window the production rail uses after a mutation (membershipFramesLeft = 6 in packages/ui/src/prompt-anchor-rail.tsx), so agreement now spans a real painted-frame boundary instead of two same-frame protocol reads. The poll timeout is raised to 15s to leave room for the six-frame windows across retries. The post-poll re-read stays deleted, so nothing is read after the poll that the poll did not see.

One honest limitation: I could not run the --workers=4 --repeat-each stress case locally — the Desktop E2E suite needs Linux/Xvfb Electron and this dev machine is Windows. The concurrent stress run on this PR's CI job is the confirmation; if it still trips, the next suspect is the poll interval vs. frame cadence interaction.

@Adarsh-Me Adarsh-Me Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed on d4c6677, and the diagnosis was the key input: in every stress failure the helper had already returned, which means the state that flipped was stable — no fixed frame count between two reads can certify it. The mechanism behind the flip: the transcript keeps remeasuring turn content after scrollTranscriptTo('bottom'), scrollHeight grows while scrollTop stays put, the snapshot's atEnd branch (scrollHeight - scrollTop - clientHeight <= 2) turns false, sourceTurn falls back to the reading-band turn, and the rail moves current off the last prompt for good — exactly the aria-current-never-returns failure you saw at line 394.

The helper is now a single in-page settle loop instead of protocol-side reads: every painted frame it re-reads the tick mapping together with scrollTop/scrollHeight/clientHeight (the inputs whose drift drives the resolver), and it resolves only when that full state is byte-identical for six consecutive frames while one current tick maps from the Turn being read. That is the observable quiet state you asked for: every input the rail's resolver reacts to — scroll, mutation, geometry — is exactly what produced the asserted state, so reads after the helper see the same rail. The loop hands the verified state back and the helper asserts it; nothing is read after settle. In-page timeout is 10s, inside the test's 60s budget.

Same limitation as before: no local Linux/Xvfb here, so please rerun the --workers=4 --repeat-each=40 case against d4c6677 — if the flip still happens, the next place to look would be a resolver input the state key does not cover.

lastSnapshot = second;
return second.expectedId !== null
&& second.currentIds.length === 1
&& second.currentIds[0] === second.expectedId
&& first.expectedId === second.expectedId
&& first.currentIds.length === 1
&& first.currentIds[0] === second.expectedId;
}, {
message: 'the one current tick maps from the Turn being read, across six painted frames',
timeout: 15_000,
}).toBe(true);
} catch {
throw new Error(`the prompt rail did not settle on the reading position: ${JSON.stringify(lastSnapshot)}`);
}
const snapshot = await activePromptRailSnapshot(page);
expect(snapshot.expectedId, `no visible Turn in ${JSON.stringify(snapshot)}`).not.toBeNull();
expect(snapshot.currentIds).toEqual([snapshot.expectedId]);
}

async function scrollTranscriptThroughHistory(page: Page): Promise<void> {
Expand Down