From 009fd2c13f9c42fb25c9b5902a7ecd2edfdfc716 Mon Sep 17 00:00:00 2001 From: Adarsh Date: Fri, 4 Sep 2026 00:29:52 +0530 Subject: [PATCH 1/3] test(desktop): settle the prompt rail across painted frames (#4675) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `expectPromptRailMatchesReadingPosition` polled until one snapshot agreed with the reading position, then took a fresh snapshot a round trip later and asserted it again. After `scrollTranscriptTo(page, 'bottom')` the transcript can still be settling, so the poll could pass on a frame where rail and reading position agreed and the second read could then see the rail one tick further — the exact failure CI reported at prompt-rail.spec.ts:206 (`turn-prompt-rail-119` vs `turn-prompt-rail-120`). The second read cannot fail on a settled page, so it only added a way to fail on an unsettled one. Requiring two agreeing snapshots is not enough on its own: both protocol reads 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 poll now requires two snapshots agreeing across that whole painted-frame window, and the post-poll re-read is gone: nothing is read after the poll that the poll did not see. Closes #4675 --- apps/desktop/e2e/prompt-rail.spec.ts | 33 +++++++++++++++++++++------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/apps/desktop/e2e/prompt-rail.spec.ts b/apps/desktop/e2e/prompt-rail.spec.ts index 7af51a35c2..2b1679acea 100644 --- a/apps/desktop/e2e/prompt-rail.spec.ts +++ b/apps/desktop/e2e/prompt-rail.spec.ts @@ -189,21 +189,38 @@ async function activePromptRailSnapshot(page: Page): Promise { 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); + 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 { From d4c66770150c6e7658a61e615c0cd8c84e888468 Mon Sep 17 00:00:00 2001 From: Adarsh Date: Fri, 4 Sep 2026 01:32:42 +0530 Subject: [PATCH 2/3] test(desktop): gate the prompt rail on painted-frame quiescence (#4675) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two-consecutive-reads poll still flaked 4/40 under 4-worker stress: it proved agreement at a moment, not quiescence, and the rail state changed after the helper returned — the test then failed at the last-tick aria-current assertion because the transcript kept remeasuring after the scroll, flipped the snapshot's atEnd branch, and moved the current tick off the last prompt for good. No fixed frame count can rule that out. The settle loop now runs in the page: each painted frame re-reads the tick mapping together with the scroll metrics that feed it, and the helper returns only once that full state is unchanged for six consecutive frames while one current tick maps from the Turn being read. Every input the rail's resolver reacts to (scroll, mutation, geometry) is then exactly what produced the asserted state, so later reads see the same rail. The helper asserts the state the loop verified and reads nothing after it. Closes #4675 --- apps/desktop/e2e/prompt-rail.spec.ts | 190 ++++++++++++++++----------- 1 file changed, 111 insertions(+), 79 deletions(-) diff --git a/apps/desktop/e2e/prompt-rail.spec.ts b/apps/desktop/e2e/prompt-rail.spec.ts index 2b1679acea..e47f8becaf 100644 --- a/apps/desktop/e2e/prompt-rail.spec.ts +++ b/apps/desktop/e2e/prompt-rail.spec.ts @@ -137,90 +137,122 @@ interface ActivePromptRailSnapshot { sourceTurnId: string | null; } -async function activePromptRailSnapshot(page: Page): Promise { - return page.evaluate(async ({ promptCount }) => { - const root = document.querySelector('[data-chat-scroll-container="true"]'); - if (!root) throw new Error('the chat scroll container is missing'); - const ticks = [...document.querySelectorAll('.maka-prompt-rail-tick')]; - const currentIds = ticks - .filter((tick) => tick.getAttribute('aria-current') === 'true') - .map((tick) => tick.dataset.promptTurnId ?? ''); - const rootBounds = root.getBoundingClientRect(); - const atEnd = root.scrollHeight - root.scrollTop - root.clientHeight <= 2; - const turns = [...root.querySelectorAll('[data-transcript-turn-id]')] - .map((turn) => ({ - element: turn, - id: turn.dataset.transcriptTurnId ?? '', - index: Number(turn.dataset.transcriptTurnId?.split('-').at(-1)) - 1, - })) - .filter((turn) => turn.id.length > 0 && Number.isFinite(turn.index)); - const readingBandTurns = turns - .filter(({ element }) => { - const bounds = element.getBoundingClientRect(); - return bounds.bottom > rootBounds.top - && bounds.top < rootBounds.top + rootBounds.height * 0.34; - }) - .sort((left, right) => left.index - right.index); - const scrollportTurns = turns - .filter(({ element }) => { - const bounds = element.getBoundingClientRect(); - return bounds.bottom > rootBounds.top && bounds.top < rootBounds.bottom; - }) - .sort((left, right) => left.index - right.index); - const sourceTurn = atEnd - ? turns.reduce( - (latest, turn) => latest === null || turn.index > latest.index ? turn : latest, - null, - ) - : readingBandTurns[0] ?? scrollportTurns[0] ?? null; - const expectedRailIndex = sourceTurn === null || ticks.length === 0 - ? null - : Math.round( - sourceTurn.index * (ticks.length - 1) / (promptCount - 1), - ); - const expectedId = expectedRailIndex === null - ? null - : ticks[expectedRailIndex]?.dataset.promptTurnId ?? null; - return { - currentIds, - expectedId, - sourceTurnId: sourceTurn?.id ?? null, - }; - }, { promptCount: PROMPT_RAIL_PROMPT_COUNT }); +interface RailStateSnapshot extends ActivePromptRailSnapshot { + scrollTop: number; + scrollHeight: number; + clientHeight: number; +} + +interface RailSettleOutcome { + settled: boolean; + state: RailStateSnapshot | null; } -// 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; +// One full rail read, in-page: the tick mapping plus the scroll metrics that +// feed it. A source string so the settle loop below and any diagnostic read +// run the exact same logic. +const READ_PROMPT_RAIL_STATE = `() => { + const promptCount = ${PROMPT_RAIL_PROMPT_COUNT}; + const root = document.querySelector('[data-chat-scroll-container="true"]'); + if (!root) throw new Error('the chat scroll container is missing'); + const ticks = [...document.querySelectorAll('.maka-prompt-rail-tick')]; + const currentIds = ticks + .filter((tick) => tick.getAttribute('aria-current') === 'true') + .map((tick) => tick.dataset.promptTurnId ?? ''); + const rootBounds = root.getBoundingClientRect(); + const atEnd = root.scrollHeight - root.scrollTop - root.clientHeight <= 2; + const turns = [...root.querySelectorAll('[data-transcript-turn-id]')] + .map((turn) => ({ + element: turn, + id: turn.dataset.transcriptTurnId ?? '', + index: Number(turn.dataset.transcriptTurnId?.split('-').at(-1)) - 1, + })) + .filter((turn) => turn.id.length > 0 && Number.isFinite(turn.index)); + const readingBandTurns = turns + .filter(({ element }) => { + const bounds = element.getBoundingClientRect(); + return bounds.bottom > rootBounds.top + && bounds.top < rootBounds.top + rootBounds.height * 0.34; + }) + .sort((left, right) => left.index - right.index); + const scrollportTurns = turns + .filter(({ element }) => { + const bounds = element.getBoundingClientRect(); + return bounds.bottom > rootBounds.top && bounds.top < rootBounds.bottom; + }) + .sort((left, right) => left.index - right.index); + const sourceTurn = atEnd + ? turns.reduce((latest, turn) => latest === null || turn.index > latest.index ? turn : latest, null) + : readingBandTurns[0] ?? scrollportTurns[0] ?? null; + const expectedRailIndex = sourceTurn === null || ticks.length === 0 + ? null + : Math.round(sourceTurn.index * (ticks.length - 1) / (promptCount - 1)); + const expectedId = expectedRailIndex === null + ? null + : ticks[expectedRailIndex]?.dataset.promptTurnId ?? null; + return { + currentIds, + expectedId, + sourceTurnId: sourceTurn?.id ?? null, + scrollTop: root.scrollTop, + scrollHeight: root.scrollHeight, + clientHeight: root.clientHeight, + }; +}`; + +// The rail resolves on the frame after a scroll and keeps re-resolving for +// six frames after each transcript mutation, and the transcript keeps +// remeasuring under them — so a snapshot can agree mid-settle and differ one +// mutation later. No fixed delay or pair of protocol reads proves the rail +// settled; that is how #4675 flaked and how frame-count fixes kept flaking +// under 4-worker stress. The observable quiet state is instead one read — +// mapping plus the scroll metrics that feed it — unchanged across six +// consecutive painted frames while mapping to the Turn being read. Every +// input the rail's resolver reacts to (scroll, mutation, geometry) is then +// exactly what produced the asserted state, so the reads a test makes after +// this helper see the same rail. The loop resolves with the state it +// verified; the passing path reads nothing after it. +const RAIL_QUIET_FRAMES = 6; +const RAIL_SETTLE_TIMEOUT_MS = 10_000; + +const SETTLE_PROMPT_RAIL_SOURCE = `({ quietFrames, timeoutMs }) => new Promise((resolve) => { + const read = ${READ_PROMPT_RAIL_STATE}; + let previousKey = null; + let quietFramesSeen = 0; + let lastState = null; + const timer = setTimeout( + () => resolve({ settled: false, state: lastState }), + timeoutMs, + ); + const frame = () => { + const state = read(); + const key = JSON.stringify(state); + quietFramesSeen = key === previousKey ? quietFramesSeen + 1 : 0; + previousKey = key; + lastState = state; + const agreed = state.expectedId !== null + && state.currentIds.length === 1 + && state.currentIds[0] === state.expectedId; + if (agreed && quietFramesSeen >= quietFrames) { + clearTimeout(timer); + resolve({ settled: true, state }); + return; + } + requestAnimationFrame(frame); + }; + frame(); +})`; async function expectPromptRailMatchesReadingPosition(page: Page): Promise { - let lastSnapshot: ActivePromptRailSnapshot | null = null; - try { - await expect.poll(async () => { - const first = await activePromptRailSnapshot(page); - await waitForPaintedFrames(page, RAIL_SETTLE_FRAMES); - const second = await activePromptRailSnapshot(page); - 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 outcome = await page.evaluate(SETTLE_PROMPT_RAIL_SOURCE, { + quietFrames: RAIL_QUIET_FRAMES, + timeoutMs: RAIL_SETTLE_TIMEOUT_MS, + }) as RailSettleOutcome; + if (!outcome.settled) { + throw new Error(`the prompt rail did not settle on the reading position: ${JSON.stringify(outcome.state)}`); } + expect(outcome.state?.expectedId, `no visible Turn in ${JSON.stringify(outcome.state)}`).not.toBeNull(); + expect(outcome.state?.currentIds).toEqual([outcome.state?.expectedId]); } async function scrollTranscriptThroughHistory(page: Page): Promise { From ab2484d37a3db5779d2f9b500dbf0756f6db3196 Mon Sep 17 00:00:00 2001 From: Adarsh Date: Fri, 4 Sep 2026 10:50:34 +0530 Subject: [PATCH 3/3] test(desktop): pass the rail settle loop as a function, not source (#4675) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `page.evaluate(SETTLE_PROMPT_RAIL_SOURCE, arg)` evaluated the generated source as an expression: Playwright sends string page functions with `isFunction: false`, so the arrow function came back uncalled and serialization reduced it to `undefined` — the helper threw on `outcome.settled` before the first settle frame ran, failing the stress run 40/40 and the spec at this path (review on #4685). The settle loop is now a real function passed to `page.evaluate`, with `promptCount`, `quietFrames`, and `timeoutMs` as the argument — the pattern this file already used for its snapshot helper and that `waitForStableTurnAtScrollerStart` uses in transcript-scroll.spec.ts. The quiescence semantics are unchanged: full rail state (tick mapping plus scroll metrics) identical across six consecutive painted frames while one current tick maps from the Turn being read, with the helper asserting the state the loop verified. --- apps/desktop/e2e/prompt-rail.spec.ts | 191 +++++++++++++++------------ 1 file changed, 103 insertions(+), 88 deletions(-) diff --git a/apps/desktop/e2e/prompt-rail.spec.ts b/apps/desktop/e2e/prompt-rail.spec.ts index e47f8becaf..87cf9551e1 100644 --- a/apps/desktop/e2e/prompt-rail.spec.ts +++ b/apps/desktop/e2e/prompt-rail.spec.ts @@ -143,116 +143,131 @@ interface RailStateSnapshot extends ActivePromptRailSnapshot { clientHeight: number; } -interface RailSettleOutcome { - settled: boolean; - state: RailStateSnapshot | null; +interface RailSettleArgs { + promptCount: number; + quietFrames: number; + timeoutMs: number; } -// One full rail read, in-page: the tick mapping plus the scroll metrics that -// feed it. A source string so the settle loop below and any diagnostic read -// run the exact same logic. -const READ_PROMPT_RAIL_STATE = `() => { - const promptCount = ${PROMPT_RAIL_PROMPT_COUNT}; - const root = document.querySelector('[data-chat-scroll-container="true"]'); - if (!root) throw new Error('the chat scroll container is missing'); - const ticks = [...document.querySelectorAll('.maka-prompt-rail-tick')]; - const currentIds = ticks - .filter((tick) => tick.getAttribute('aria-current') === 'true') - .map((tick) => tick.dataset.promptTurnId ?? ''); - const rootBounds = root.getBoundingClientRect(); - const atEnd = root.scrollHeight - root.scrollTop - root.clientHeight <= 2; - const turns = [...root.querySelectorAll('[data-transcript-turn-id]')] - .map((turn) => ({ - element: turn, - id: turn.dataset.transcriptTurnId ?? '', - index: Number(turn.dataset.transcriptTurnId?.split('-').at(-1)) - 1, - })) - .filter((turn) => turn.id.length > 0 && Number.isFinite(turn.index)); - const readingBandTurns = turns - .filter(({ element }) => { - const bounds = element.getBoundingClientRect(); - return bounds.bottom > rootBounds.top - && bounds.top < rootBounds.top + rootBounds.height * 0.34; - }) - .sort((left, right) => left.index - right.index); - const scrollportTurns = turns - .filter(({ element }) => { - const bounds = element.getBoundingClientRect(); - return bounds.bottom > rootBounds.top && bounds.top < rootBounds.bottom; - }) - .sort((left, right) => left.index - right.index); - const sourceTurn = atEnd - ? turns.reduce((latest, turn) => latest === null || turn.index > latest.index ? turn : latest, null) - : readingBandTurns[0] ?? scrollportTurns[0] ?? null; - const expectedRailIndex = sourceTurn === null || ticks.length === 0 - ? null - : Math.round(sourceTurn.index * (ticks.length - 1) / (promptCount - 1)); - const expectedId = expectedRailIndex === null - ? null - : ticks[expectedRailIndex]?.dataset.promptTurnId ?? null; - return { - currentIds, - expectedId, - sourceTurnId: sourceTurn?.id ?? null, - scrollTop: root.scrollTop, - scrollHeight: root.scrollHeight, - clientHeight: root.clientHeight, - }; -}`; +type RailSettleOutcome = + | { settled: true; state: RailStateSnapshot } + | { settled: false; state: RailStateSnapshot | null }; // The rail resolves on the frame after a scroll and keeps re-resolving for // six frames after each transcript mutation, and the transcript keeps // remeasuring under them — so a snapshot can agree mid-settle and differ one -// mutation later. No fixed delay or pair of protocol reads proves the rail -// settled; that is how #4675 flaked and how frame-count fixes kept flaking -// under 4-worker stress. The observable quiet state is instead one read — -// mapping plus the scroll metrics that feed it — unchanged across six +// mutation later. After a scroll to the bottom that remeasure grows +// `scrollHeight` while `scrollTop` stays put, the atEnd branch flips, and the +// current tick moves off the last prompt for good: a stable flip that no +// fixed delay between two reads can rule out (#4675 and the 4-worker stress +// on its review thread). The observable quiet state is instead one full read +// — tick mapping plus the scroll metrics that feed it — unchanged across six // consecutive painted frames while mapping to the Turn being read. Every // input the rail's resolver reacts to (scroll, mutation, geometry) is then // exactly what produced the asserted state, so the reads a test makes after // this helper see the same rail. The loop resolves with the state it // verified; the passing path reads nothing after it. +// +// The loop runs in one `page.evaluate` so the per-frame reads cannot straddle +// a protocol round trip, and it is passed as a real function: a string +// pageFunction is evaluated as an expression and never invoked. const RAIL_QUIET_FRAMES = 6; const RAIL_SETTLE_TIMEOUT_MS = 10_000; -const SETTLE_PROMPT_RAIL_SOURCE = `({ quietFrames, timeoutMs }) => new Promise((resolve) => { - const read = ${READ_PROMPT_RAIL_STATE}; - let previousKey = null; - let quietFramesSeen = 0; - let lastState = null; - const timer = setTimeout( - () => resolve({ settled: false, state: lastState }), - timeoutMs, - ); - const frame = () => { - const state = read(); - const key = JSON.stringify(state); - quietFramesSeen = key === previousKey ? quietFramesSeen + 1 : 0; - previousKey = key; - lastState = state; - const agreed = state.expectedId !== null - && state.currentIds.length === 1 - && state.currentIds[0] === state.expectedId; - if (agreed && quietFramesSeen >= quietFrames) { - clearTimeout(timer); - resolve({ settled: true, state }); - return; - } - requestAnimationFrame(frame); +async function settlePromptRailInPage({ + promptCount, + quietFrames, + timeoutMs, +}: RailSettleArgs): Promise { + const read = (): RailStateSnapshot => { + const root = document.querySelector('[data-chat-scroll-container="true"]'); + if (!root) throw new Error('the chat scroll container is missing'); + const ticks = [...document.querySelectorAll('.maka-prompt-rail-tick')]; + const currentIds = ticks + .filter((tick) => tick.getAttribute('aria-current') === 'true') + .map((tick) => tick.dataset.promptTurnId ?? ''); + const rootBounds = root.getBoundingClientRect(); + const atEnd = root.scrollHeight - root.scrollTop - root.clientHeight <= 2; + const turns = [...root.querySelectorAll('[data-transcript-turn-id]')] + .map((turn) => ({ + element: turn, + id: turn.dataset.transcriptTurnId ?? '', + index: Number(turn.dataset.transcriptTurnId?.split('-').at(-1)) - 1, + })) + .filter((turn) => turn.id.length > 0 && Number.isFinite(turn.index)); + const readingBandTurns = turns + .filter(({ element }) => { + const bounds = element.getBoundingClientRect(); + return bounds.bottom > rootBounds.top + && bounds.top < rootBounds.top + rootBounds.height * 0.34; + }) + .sort((left, right) => left.index - right.index); + const scrollportTurns = turns + .filter(({ element }) => { + const bounds = element.getBoundingClientRect(); + return bounds.bottom > rootBounds.top && bounds.top < rootBounds.bottom; + }) + .sort((left, right) => left.index - right.index); + const sourceTurn = atEnd + ? turns.reduce( + (latest, turn) => latest === null || turn.index > latest.index ? turn : latest, + null, + ) + : readingBandTurns[0] ?? scrollportTurns[0] ?? null; + const expectedRailIndex = sourceTurn === null || ticks.length === 0 + ? null + : Math.round(sourceTurn.index * (ticks.length - 1) / (promptCount - 1)); + const expectedId = expectedRailIndex === null + ? null + : ticks[expectedRailIndex]?.dataset.promptTurnId ?? null; + return { + currentIds, + expectedId, + sourceTurnId: sourceTurn?.id ?? null, + scrollTop: root.scrollTop, + scrollHeight: root.scrollHeight, + clientHeight: root.clientHeight, + }; }; - frame(); -})`; + return new Promise((resolve) => { + let previousKey: string | null = null; + let quietFramesSeen = 0; + let lastState: RailStateSnapshot | null = null; + const timer = setTimeout( + () => resolve({ settled: false, state: lastState }), + timeoutMs, + ); + const frame = (): void => { + const state = read(); + const key = JSON.stringify(state); + quietFramesSeen = key === previousKey ? quietFramesSeen + 1 : 0; + previousKey = key; + lastState = state; + const agreed = state.expectedId !== null + && state.currentIds.length === 1 + && state.currentIds[0] === state.expectedId; + if (agreed && quietFramesSeen >= quietFrames) { + clearTimeout(timer); + resolve({ settled: true, state }); + return; + } + requestAnimationFrame(frame); + }; + frame(); + }); +} async function expectPromptRailMatchesReadingPosition(page: Page): Promise { - const outcome = await page.evaluate(SETTLE_PROMPT_RAIL_SOURCE, { + const outcome = await page.evaluate(settlePromptRailInPage, { + promptCount: PROMPT_RAIL_PROMPT_COUNT, quietFrames: RAIL_QUIET_FRAMES, timeoutMs: RAIL_SETTLE_TIMEOUT_MS, - }) as RailSettleOutcome; + }); if (!outcome.settled) { throw new Error(`the prompt rail did not settle on the reading position: ${JSON.stringify(outcome.state)}`); } - expect(outcome.state?.expectedId, `no visible Turn in ${JSON.stringify(outcome.state)}`).not.toBeNull(); - expect(outcome.state?.currentIds).toEqual([outcome.state?.expectedId]); + expect(outcome.state.expectedId, `no visible Turn in ${JSON.stringify(outcome.state)}`).not.toBeNull(); + expect(outcome.state.currentIds).toEqual([outcome.state.expectedId]); } async function scrollTranscriptThroughHistory(page: Page): Promise {