fix(ui): restore prompt rail scroll tracking - #4417
Conversation
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Reviewed ed0bda6a. The symptom is real and this does fix it — but the cause is bigger than the summary says, and the change that actually repairs it isn't among the four bullets. One P2 and three P3s inline; none block merge, but the description should be rewritten before squash.
The real cause. data-virtual-turn-id appears exactly once in the whole tree on main — at prompt-anchor-rail.tsx:314, the line that reads it. There are no producers left: #4206 removed row virtualization and deleted every emitter (renderer, e2e, main-process markup tests), updating this file's comment but not its selector. So mountedTurnList has been undefined ever since, the scroll-spy effect returns at its own guard, and no IntersectionObserver, MutationObserver or scroll listener is ever installed. Since #4206 the rail highlight has only ever been written by jumpTo() — scrolling has never moved it, and a session nobody clicked in has no current tick at all. That is exactly what #4415 reports.
The fix is the ?? messageList?.firstElementChild fallback, which none of the four bullets mention. The two causes the summary does give — imperative attribute writes, and an empty top band — are secondary effects, not the reason the highlight never moved.
One thing this gets right that's worth keeping visible. Making React the sole writer of data-active and aria-current removes a genuine second authority, not just some code. On main the imperative path took its input from activeTurnIdRef.current, so a render that computed props before the imperative write and committed after it left the DOM and the ref disagreeing — and markActiveTurn's if (ref === turnId) return then suppressed the repair permanently. One writer, one concept, fewer lines. Same instinct applied one step further is what P3-1 below is asking for.
Please rewrite the summary and the squash message. As written they don't let a later reader reconstruct what happened, because the actual defect isn't in them. Something like: "#4206 removed row virtualization but left the [data-virtual-turn-id] lookup behind, so the scroll-spy effect has been bailing out at its guard ever since and the rail highlight only moved on click." Please keep the Generated-by trailer through the squash.
Evidence boundary: this is a static read of pr4417 against origin/main — I confirmed the dead selector with a whole-tree grep and traced its last producer to c33617fd8b. I did not launch Desktop, run Playwright, or attach CDP, so the jitter in P2-1 is derived from the effect's dependencies and the two code paths rather than measured.
One thing worth doing before merge that automation won't catch: #4206's warm-scroll numbers were taken while the scroll-spy was dead. This puts an IntersectionObserver, a MutationObserver and a re-render of up to 64 rail ticks back on the scroll path, and nothing gates that. A manual pass with the transcript perf harness would be worth the few minutes.
AI-assisted review: drafted with Maka; I verified the missing producers, the guard bail-out, and the two active-Turn rules against the branch source myself.
| const messageList = root?.querySelector('.maka-chat-message-list'); | ||
| const mountedTurnList = | ||
| root?.querySelector('[data-virtual-turn-id]')?.parentElement | ||
| ?? messageList?.firstElementChild; |
There was a problem hiding this comment.
P3 — the dead arm should go, not move to the front of a ??.
[data-virtual-turn-id] has no producer anywhere in the tree; git log -S puts the last one in c33617fd8b (#4206), and grepping origin/main returns this read and nothing else. So the first arm can never match, and the second is doing all the work.
Keeping it is the same shape that caused this bug — an emitter was deleted and its selector left behind — handed to the next reader, who will reasonably infer that virtualization still exists somewhere. const mountedTurnList = root?.querySelector('.maka-chat-message-list')?.firstElementChild; says what is true.
I checked the replacement and it's sound, and actually sturdier than what it replaces: Astryx's ChatMessageList renders role="log" with a single inner div, and that div is the direct parent of the .maka-transcript-turn wrappers, so both querySelectorAll('[data-turn-id]') and the {childList: true} observation hold — and unlike the old form it resolves before any Turn has mounted. Worth a line of comment saying firstElementChild is that inner flex column, since that's an upstream implementation detail of exactly the kind that just went stale.
There was a problem hiding this comment.
Fixed in 981ebb5. The dead [data-virtual-turn-id] arm is removed entirely. The effect now binds directly to ChatMessageList's inner flex column, with an inline comment documenting that upstream DOM contract, and observes the real transcript Turn wrappers.
| // Resolve once from current geometry instead of waiting for the observer's | ||
| // asynchronous initial delivery; otherwise the rail paints with no current | ||
| // Turn for at least one frame and can stay that way in throttled windows. | ||
| resolveActive(); |
There was a problem hiding this comment.
P2 — this immediate resolve answers a different question than the observer does, and the effect rebuilds often enough for the difference to show.
visible is a fresh empty Set at this point, so this call always lands in the geometric fallback: "lowest-index Turn intersecting the whole scrollport". A frame later the IntersectionObserver delivers and the answer becomes "lowest-index Turn intersecting the top band" — the rootMargin: '0px 0px -66% 0px' restricts it to the top 34%. Whenever the tail of a previous Turn is still on screen above the band, those are different Turns.
That would be harmless if the effect were built once, but its deps churn during streaming: chat-view.tsx:433-457 only preserves the turns array identity when every entry's turnId, label and reply are unchanged, and a streaming reply's text changes on every delivery. So turnIndexById and railTurns get new identities, this effect tears down and rebuilds, and each rebuild emits the geometric answer before the observer corrects it. The visible result is the active tick flipping between two neighbours in time with the stream. Each rebuild also forces a synchronous layout — getBoundingClientRect() on the root plus every resident Turn — on the scroll path #4206 specifically cleaned up.
Smallest fix is to make both paths use one rule: seed visible from the band geometry here (root top to top + height * 0.34) and let the existing empty-band fallback handle the rest. The intent — never leave the rail blank on mount — survives, without a second answer. Alternatively, only run this when there is no active Turn yet.
The selection rule itself is a pure function of the visible set and the index map (at end → highest index; band non-empty → lowest index; band empty → lowest index intersecting the scrollport). This file already exports holdJumpDestination, keepActivePromptRailTickVisible and observeActivePromptRailVisibility precisely so the untestable geometry can stay out of prompt-anchor-rail.test.ts; extracting the rule the same way would let two lines of unit test pin "both paths agree on the same geometry" — which is what would have caught this.
There was a problem hiding this comment.
Fixed in 981ebb5. Initial geometry and IntersectionObserver updates now call the same selectPromptRailActiveTurn(...) rule with the same 34% reading band and full-scrollport fallback. Structural Turn IDs/indexes and sampled tick IDs are memoized independently from streaming preview text, so reply updates no longer rebuild the scroll-spy. I also ran the warm-scroll harness three times per revision; fix medians were layout 6.63 ms, script 59.95 ms, frame P95/P99 17.60/17.70 ms, with zero >50 ms LoAFs in all three fix runs.
| return turnBounds.bottom > bounds.top && turnBounds.top < bounds.bottom; | ||
| }) | ||
| .map((turn) => turn.dataset.turnId); | ||
| return visibleTurnIds.includes(active.dataset.promptTurnId); |
There was a problem hiding this comment.
P3 — this compares ids from two different spaces, and it doesn't assert what #4415 asks for.
visibleTurnIds comes from the transcript's [data-turn-id] elements, while active.dataset.promptTurnId comes from the rail, which subsamples when there are more Turns than ticks. The fixture seeds 120 prompts (seed-helpers.ts:39) against a 64-tick cap, so railTurnIdFor maps rail index 1 to turns[2] — the two sets are not the same ids. It passes today only because enough Turns are on screen at once for the sampled id to appear in the visible set by coincidence, which means it can also go green on a wrong implementation and red on a right one.
Separately, #4415's stated expectation — the first Turn's tick is current after scrolling back to the top — isn't asserted anywhere; the comment above concedes as much. The first assertion (scroll to bottom → last tick current) is good and does fail on main.
Putting both sides through the same sampling before comparing would fix the space mismatch; if that mapping is awkward to reach from the spec, asserting that the active tick's index is within one of the topmost visible Turn's sampled index says something true instead.
There was a problem hiding this comment.
Fixed in 981ebb5. The E2E now identifies the actual reading-band Turn, maps its 120-Turn source index through round(turnIndex × 63 / 119), and compares that exact sampled rail ID. It also asserts the exact last tick at the bottom, exact first tick after real history pagination to Turn 1, and records current-tick cardinality with a MutationObserver so every stable state must have exactly one current tick.
ed0bda6 to
0cfc1d8
Compare
Sun-GLiang
changed the title
|
@Astro-Han, I rebuilt the fix around the review findings in |
0cfc1d8 to
a2121d4
Compare
Remove the stale virtual-turn selector left after row virtualization was deleted, and track the actual mounted Turn through one reading-band selection rule. Keep React as the sole active-attribute owner and project unsampled visible Turns into the bounded landmark rail without dropping the current tick. Generated-by: Maka
a2121d4 to
981ebb5
Compare
github-actions
Bot
added
effort/L
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Reviewed 981ebb5. All three threads from the last round are addressed, and the fix is real: the observers now bind to a node that exists, selectPromptRailActiveTurn is the single rule for both the initial resolve and the IntersectionObserver, and React is the only writer of data-active / aria-current. #4415 is fixed on this branch.
What I want to push back on is size. The production file grows from 576 to 792 lines, and about 140 of those lines exist for one case: the reader is resting on a mounted Turn that is not a rail Turn (a Turn with no prompt text, which chat-view.tsx:434 filters out of the rail). That case is also where this PR introduces its only wrong answer (P2 below). Handling it the way main did, by simply not observing non-rail Turns, removes the bug and the machinery together. Details inline; two P2s, the rest P3.
One more thing I checked myself rather than from the comment: IntersectionObserver resolves a percentage rootMargin on the top/bottom sides against the root's height, not its width. In Chromium 148, a 600×300 root with rootMargin: '0px 0px -66% 0px' reports rootBounds.height === 204. So main's -66% was already the same 34% band as the geometry path, and the pixel conversion plus the resize-rebuild that depends on it can go (inline at L551).
Evidence boundary: static read of the branch against origin/main, plus the one browser measurement above. I did not run test:dist, Playwright, or the perf harness on this head, so the numbers in the description are yours, not mine.
Please keep the Generated-by trailer through the squash.
AI-assisted review: drafted with Maka; I verified the reachability of the railTurns[0] fallback, the rootMargin behaviour, and the E2E/production sampling equivalence myself.
| const previousRailTurnId = input.railTurns.some( | ||
| (turn) => turn.turnId === input.previousRailTurnId, | ||
| ) ? input.previousRailTurnId : null; | ||
| const fallbackRailTurnId = previousRailTurnId ?? input.railTurns[0]?.turnId ?? null; |
There was a problem hiding this comment.
P2 — this can mark the first tick current while the reader is at the bottom, and main does not.
Reaching this function at all means activeTurnId is not in orderedTurnIds: L411 handles a sampled id, L413-416 handles an unsampled id that is still in the ordered list, and below 64 Turns the two lists are identical. So the only way in is a mounted Turn the rail does not know about, i.e. one chat-view.tsx:434 dropped for having no prompt text (turn.user is optional in materialize.ts:731; an attachment-only prompt is enough).
While transcriptTurnIndex has not arrived yet every sequence is undefined, mountedLandmarks is empty, and this returns previousRailTurnId ?? railTurns[0]. On the first visit to a session there is no previous, so a reader at the tail of such a Turn sees tick 1 current until the index loads. main never got here: observeElement filtered on turnIndexById.has(turnId), so a non-rail Turn was never a candidate and the highlight stayed on the last prompt the reader passed. The unit test at prompt-anchor-rail.test.ts:333-344 pins this fallback as the expected answer.
Smallest fix is main's filter: only observe elements whose id is in turnIndexById. Then activeTurnId is always in orderedTurnIds, L411/L413 cover every case, and all of this disappears with it: selectPromptRailTickForMountedTurn (L279-357), mountedTurnIds state (L363), previousActiveRailTurnIdRef and its effect (L366, L425-431), mountedTurnIndexById (L463), refreshMountedTurnOrder and both call sites (L483-499, L564, L597), and the five unit tests at test.ts:278-344. Roughly 120 production lines and one state/ref pair that today is written by an effect and read during render, which is also why switching sessions can paint one frame of the previous session's mapping.
What you give up: scrolling through consecutive Turns with no prompt text leaves the highlight on the last prompt. For a rail whose ticks are prompts, that is the honest answer.
There was a problem hiding this comment.
The premise that reaching this helper implies a prompt-less Turn does not hold for the Desktop data contract. The Host landmark query is capped at 64 entries, while the active transcript range independently keeps mounted Turns. In a 120-prompt session, a mounted prompt can therefore be absent from orderedTurnIds and legitimately reach this mapping path. Filtering observation through turnIndexById would stop the rail from tracking those real prompts and reintroduce the long-session gap from #4415.
I kept the mapping path, but fixed the actual prompt-less-tail failure in c153875 / 3b34ec0: it now uses the nearest mounted rail prompt, and when the tail has no mounted landmark it selects the final rail tick. Both cases have unit coverage.
|
|
||
| }, { | ||
| root, | ||
| // IntersectionObserver resolves percentage root margins against the |
There was a problem hiding this comment.
P3 — the premise is wrong, and about 20 lines hang on it.
Percentage rootMargin resolves per side against the root intersection rectangle: width for left/right, height for top/bottom. Measured in Chromium 148: root 600×300, '0px 0px -66% 0px', rootBounds.height is 204. So -66% is already the same band as seedReadingBandFromGeometry, and it follows a resize on its own.
That lets observerRootHeight, the pixel string, and the whole rootResizeObserver block at L568-579 go, along with the "disconnect, clear the band, re-observe everything" path it introduces. It also closes a small mismatch this version has: the IO root box is the padding box, while L559 measures the border box.
There was a problem hiding this comment.
Implemented in c153875. The observer now uses rootMargin: "0px 0px -66% 0px" directly; observerRootHeight, the pixel conversion, the root resize observer, and the disconnect/re-observe path were removed.
| expectedId: string | null; | ||
| sourceTurnId: string | null; | ||
| } | ||
|
|
There was a problem hiding this comment.
P3 — this restates the production rule, so the mid-scroll assertion cannot disagree with it.
L151-179 re-implement tail / band / scrollport-fallback and the round(i × 63 / 119) sampling, and L25 redeclares MAX_PROMPT_RAIL_TICKS. I checked the sampling against L400-416 across all 120 indexes and it agrees today, but the moment the production sampling changes this goes red for no product reason, and while it agrees it only proves the implementation matches a copy of itself.
The assertions that do carry weight are L375-379 (bottom → exactly the last tick), L404-408 (paged to Turn 1 → exactly the first tick), and the MutationObserver cardinality check. Those three cover #4415 and fail on main. I'd keep them, drop the rule copy, and for the middle assert something weaker but independent: the current tick's index is within one of the topmost visible Turn's sampled index. Export the sampling function and the tick constant from prompt-anchor-rail.tsx so the spec imports them instead of restating them.
There was a problem hiding this comment.
I am keeping the exact mid-scroll assertion because #4415 requires the current prompt to follow manual scrolling throughout the transcript; endpoint and cardinality assertions cannot catch an interior mapping regression.
The source Turn in the test is derived independently from rendered DOM geometry. Production derives it from IntersectionObserver membership and maps an unsampled mounted Turn through sequence anchors. The fixture has uniform sequences, so the test can compute the expected rendered tick without calling the production helper. Importing the production mapping would make this oracle less independent, while weakening it would leave the core regression uncovered.
| root.dispatchEvent(new WheelEvent('wheel', { deltaY: -120, bubbles: true })); | ||
| root.dispatchEvent(new Event('scroll')); | ||
| }); | ||
| await expect.poll(async () => |
There was a problem hiding this comment.
P2 — this waits on a history page with the default 10 s while the sibling spec gives the same action 20 s.
transcript-scroll.spec.ts:466 polls the identical "first turn changed" condition with timeout: 20_000 and says why. This loop pages roughly eleven times through the 120-Turn fixture with retries: 0, and each iteration nests another 10 s poll for the rail. One slow page load fails the test with a message about the rail, not about history. Align to 20 s.
There was a problem hiding this comment.
Aligned to 20 seconds in c153875. The history-load poll now reports its own timeout before the nested rail-position assertion runs.
| if (frame !== 0) cancelAnimationFrame(frame); | ||
| }; | ||
| }, [markActiveTurn, scrollRef, turnIndexById, railTurns]); | ||
| }, [markActiveTurn, orderedTurnIds, scrollRef]); |
There was a problem hiding this comment.
P3 — the one obligation this PR is most careful about has no test.
orderedTurnIdsRef exists so a streaming delta does not rebuild the observers, and this dependency array is where that holds. Changing it back to turns keeps every current test green; the rail still highlights, it just tears down and rebuilds on every delta. That is the same shape of silent regression #4206 → #4415 was. A dozen lines in the E2E, counting IntersectionObserver constructions across a burst of deltas and asserting the count does not move, would pin it. (If the rootResizeObserver at L568 goes per the comment above, there is nothing else on this file that needs a test it does not have.)
There was a problem hiding this comment.
Added in 198137f. The E2E creates a real three-Turn conversation, instruments only the prompt-rail IntersectionObserver, records its first construction, then waits through a long fake-backend response streamed in nine-character deltas and asserts the count does not change. As a negative control, changing the effect dependency back to turns fails with the construction count changing from 1 to 2; the current orderedTurnIds dependency passes 1 to 1.
| }), null); | ||
| }); | ||
|
|
||
| test('an unsampled mounted Turn maps through the two nearest durable landmarks', () => { |
There was a problem hiding this comment.
P3 — this case describes a scenario the component cannot reach.
turn-66 is the 66th entry of a 120-entry index, so in the component it takes the arithmetic branch at L414-416 and never calls selectPromptRailTickForMountedTurn. The two paths happen to agree on turn-65, so it is not a false green, but the name and the function under test do not match. Moot if the function goes (see L288).
There was a problem hiding this comment.
This scenario is reachable because the component does not receive a complete 120-entry prompt index: Desktop requests at most 64 landmarks. turn-66 can be a real mounted prompt omitted from those 64 entries, so it is absent from orderedTurnIds and reaches selectPromptRailTickForMountedTurn. The test models that capped-landmark case and is retained.
Use the nearest mounted prompt before the durable landmark index arrives, keep the reading band aligned with Chromium percentage margins, and give transcript history paging the established timeout. Generated-by: Maka
Carry the tail state with the active transcript Turn so a prompt-less resident window selects the final rail landmark instead of the first. Generated-by: Maka
Generated-by: Maka
Generated-by: Maka
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Approving on 6af2c7e19. No blocking findings. Five P3s below, two of which are small enough to be worth taking in this PR.
The problem is statically decidable, which is worth stating. [data-virtual-turn-id] has zero producers anywhere in packages or apps; #4206 removed the marker and left the read. So mountedTurnList was always null, the effect returned at its guard, and neither observer has been installed since that merge. Nothing needed to be reproduced to know the rail could not have been tracking.
The seam is right. No new authority, no new component, no compatibility path. Both markers you moved to (.maka-chat-message-list and data-transcript-turn-id) have live producers in chat-view.tsx and chat-turn.tsx. The imperative writes are gone rather than reduced: setAttribute and removeAttribute do not appear in the new file at all, data-active and aria-current come only from activeRailTurnId in JSX, and keepActivePromptRailTickVisible only reads. There is no second writer left.
The one design decision I checked hardest holds up. Widening the observed set from "Turns the rail knows about" to "every mounted Turn" looked like scope at first. It is not: landmark queries are capped at 64 (session-turns.ts:36, enforced in sqlite-session-metadata-store.ts:2872) while the mount window independently keeps 10 Turns (transcript-contract.ts:22), so in a long session roughly half the mounted prompts are absent from the landmark index. Keeping the old turnIndexById.has filter would make the rail lose track in exactly the sessions #4415 is about.
Both E2E tests prove their obligation through the production path. The manual-scroll spec fails pre-fix because there is no current tick at all, and its "exactly one current at every moment" MutationObserver assertion is the only thing that could catch a two-ticks-current regression. The streaming spec wraps the real IntersectionObserver in the real renderer, filters by root and rootMargin, and asserts the probe fired before asserting the count did not change. Neither is a fixture bypass.
P3, none blocking
-
directbranch is unreachable in production (prompt-anchor-rail.tsx:292-293). The call site at :429 already returns early onrailTurnIds.includes(activeTurnId), andrailTurnIdsandrailTurnscome from the samerailTurnIndexeswith identical id sets, sodirectis alwaysundefinedby the time the helper runs. Two lines, safe to delete here. -
The 34% reading band is written twice.
:531computesrootBounds.top + rootBounds.height * 0.34;:568setsrootMargin: '0px 0px -66% 0px'. The description says initial geometry and observer updates share one rule; in the code they are two constants kept in agreement by hand. AREADING_BAND_FRACTIONused by both, with the rootMargin computed from it, makes that sentence true. Also small enough for this PR. -
The 64-tick sampling and its inverse mapping are unreachable in the current composition, and this PR enlarges them (
:415-433). With the landmark cap at 64,turns.length <= 64whenever the index is present; without itmergePromptAnchorRailTurnsfalls back to loaded prompts, bounded by the 10-Turn mount window.refreshTranscriptTurnLandmarksreplaces rather than accumulates, andPromptAnchorRailhas exactly one consumer. The dead path existed before as a 7-line helper; here it becomes three memos and a branch. Since you are already in this code, deleting theMAX_PROMPT_RAIL_TICKSsampling along with the inverse mapping at :430-433 and renderingorderedTurnIdsdirectly would offset most of what this PR adds. If the worry is a future consumer passing more than 64, what that wants is a prop contract assertion, not an unexercised branch. -
Ref written during render (
:405-412), with a downstreamuseMemodepending on it. Self-healing and idempotent, so the impact is small, but auseMemokeyed on the joined id sequence has the same meaning with no render-phase side effect. -
The interpolation helper costs more than it currently buys (
:279-374). I extractedselectPromptRailTickForMountedTurnunchanged and ran 520 non-landmark cases across a 120-turn session with 64 uniform landmarks and a sliding 10-Turn window: its output matched a plain linear mapping in 520 of 520. The two diverge only on non-uniform sequences, which the E2E cannot construct, so the mid-journey E2E assertion cannot tell the helper apart from a one-line formula. Only theuneven sequence gapsunit test covers the divergence. If you want to settle it: drop theactiveSequenceinterpolation and theprojectedRailIndextie-break, keepnearestMountedRailTurnIdand thecandidateRangenearest-by-sequence step, delete the two corresponding unit tests, and run the E2E once. Green means it was removable; red means it was guarding something and you have the evidence. One CI run for a real answer.
Two unit tests are also redundant pairs: the "no sampled wrapper" case against the "no landmark, replace the stale current" case, and the two prompt-less tail cases. Each pair is the two sides of one branch and could be one test.
Manual acceptance this cannot cover
Per the header of prompt-rail.spec.ts, its reachability dimension only holds on macOS, since Linux in-flow scrollbars let #2338-class regressions pass. Please run the full spec on macOS before relying on it. Beyond that, two things worth eyeballing: highlight continuity while scrolling a session with more than 64 prompts, and that clicking a tick and releasing does not let the highlight get dragged by prompts passed on the way.
On the native scroll listener at :616 coexisting with subscribeToReaderScroll: I looked at whether they should merge and they should not. subscribeToReaderScroll deliberately fires only on reader-initiated movement, while the rail also needs to recompute during tail-following and growth-driven displacement. The listener predates this PR and is correct to keep. What is now false is the docstring claiming nothing else reads the raw scroll event, and that is a doc fix outside this change.
Evidence boundary: static read of 6af2c7e19 against main 98d86eb4f; marker producers and the landmark-cap arithmetic verified in source; the 520-case comparison run locally against the extracted helper. I did not run the Playwright suite and did not reproduce the performance table.
AI-assisted review: drafted with Maka. The helper extraction and its 520-case comparison, the landmark-cap arithmetic and the unreachability arguments are mine.
简体中文
在 6af2c7e19 上批准，没有阻塞项。下面五条 P3，其中两条小到值得在这个 PR 里顺手带走。
这个问题是静态可判定的。 [data-virtual-turn-id] 在 packages 和 apps 里零生产者，#4206 删了标记留下了读取。所以 mountedTurnList 一直是 null，effect 在守卫处直接返回，两个 observer 自那次合并起从未安装过。不需要复现就能断定 rail 不可能在跟随。
接缝对了。 没有新权威、没有新组件、没有兼容路径，你换用的两个标记都有活的生产者。命令式写入是被删掉而不是减少：新文件里 setAttribute 和 removeAttribute 一次都不出现，data-active 和 aria-current 只由 JSX 里的 activeRailTurnId 渲染，keepActivePromptRailTickVisible 只读不写，没有留下第二个写者。
我核得最重的那个设计决定站得住。 把观察集合从「rail 认识的 turn」扩成「所有已挂载的 turn」乍看像扩范围，其实不是：landmark 查询硬上限 64（session-turns.ts:36），而挂载窗口独立地保 10 个 turn（transcript-contract.ts:22），所以长会话里大约一半挂载的 prompt 不在 index 里。保留原来的 turnIndexById.has 过滤，rail 就会正好在 #4415 描述的那类会话里跟丢。
两个 E2E 都通过生产路径证明了各自的义务：手动滚动那条在修复前因为根本没有 current tick 而必红，它内部那个「任何时刻恰好一个 current」的 MutationObserver 断言是唯一能抓住「两个 tick 同时 current」的东西；流式那条在真实 renderer 里包住真实的 IntersectionObserver，按 root 和 rootMargin 过滤，而且先确认探针命中再断言构造数不变。都不是 fixture 绕过。
五条 P3：direct 分支在生产不可达（:292-293，调用点已经提前返回，两行可以直接删）；34% 阅读带写了两遍（:531 和 :568 是同一条规则的两个权威，抽个常量让正文那句话成真）；64-tick 采样和它的反向映射在当前组合里走不到，而这个 PR 把原来 7 行的死路铺成了三个 memo 加一个分支，既然正在改这段，把它连同 :430-433 一起删掉、直接渲染 orderedTurnIds，能抵掉这次新增的大部分行数；render 期改 ref（:405-412），换成按 id 序列 join 做 key 的 useMemo，语义相同且没有渲染期副作用；最后是插值 helper（:279-374），我把它原样抽出来，按 120 turn、64 个均匀 landmark、10 个挂载窗口滑过全程跑了 520 个非 landmark 命中用例，输出与一行线性映射 520 比 520 完全一致，两者只在非均匀 sequence 下分叉，而 E2E 构造不出那种情况，也就无法把 helper 和一行公式区分开。要定这件事：去掉 activeSequence 插值和 projectedRailIndex 平局规则，保留 nearestMountedRailTurnId 和 candidateRange 里按 sequence 取最近，删掉对应的两条单测，跑一次 E2E。绿就是可删的证据，红说明它真在守东西。一次 CI 换一个确定答案。
另外有两对单测是同一条分支的两侧，可以各压成一条。
自动化盖不到的人工验收： 按 prompt-rail.spec.ts 文件头的说明，它的可达性维度只在 macOS 上有效，Linux 的 in-flow 滚动条会让 #2338 那类回归假绿，所以合并前请在 macOS 上跑一遍完整 spec。另外肉眼看两点：超过 64 个 prompt 的会话里手动滚动时高亮是否连续跟随，以及点某个 tick 跳转后松手，高亮不会被途经的 prompt 拖走。
:616 那个原生 scroll 监听和 subscribeToReaderScroll 并存： 我看过要不要合并，结论是不该合。subscribeToReaderScroll 按设计只在读者主动移动时触发，而 rail 在尾部跟随和内容增长导致的位移下同样要重算。这个监听在这次改动之前就有，保留是对的。现在不成立的是那段 docstring 里「没有别的东西再读原始 scroll 事件」那句话，那是文档要改，不属于这次改动。
2 participants
Summary
Restore Prompt Rail scroll tracking after #4206 removed row virtualization but left the
[data-virtual-turn-id]lookup behind. With no marker producer left, the scroll-spy effect returned at its guard, so manual scrolling never installed the observers and a rail that had not been clicked had no current tick.Same
chat-prompt-railfixture, 1000×700 viewport, and frame-by-frame scroll trajectory. The GIF is hosted by GitHub and is not committed to the repository.This change:
ChatMessageList's inner flex column directly and documents that DOM contract;data-activeandaria-current;Fixes #4415
Tests
turn-prompt-rail-1, asserts exactly one current tick throughout the journey and at both endpoints, and independently maps the visible Turn into the 64 rendered ticks.turnsfails with the construction count changing from 1 to 2; the currentorderedTurnIdsdependency remains at 1. The fullprompt-rail.spec.tspasses after the fix (11/11).Performance
Three runs of
warm native transcript scroll metricsat the original PR merge-base (8fa1e894) and the initial fix head (981ebb5), reported as medians. These measurements predate the review follow-up commits and are initial-fix evidence rather than a benchmark of the latest head.The initial-fix runs recorded zero >50 ms LoAFs in all three runs (0/0/0), so restoring the observers did not add a long animation frame in this harness.
Verification
npm --workspace @maka/desktop run build:with-depsnpm --workspace @maka/ui run test:dist— 318 passednpm --workspace @maka/desktop run typechecknpx biome check packages/ui/src/prompt-anchor-rail.tsx packages/ui/src/__tests__/prompt-anchor-rail.test.ts apps/desktop/e2e/prompt-rail.spec.tsapps/desktop:env -u ELECTRON_RUN_AS_NODE npx playwright test --config e2e/playwright.config.ts e2e/prompt-rail.spec.ts --grep "manual transcript scrolling"apps/desktop:env -u ELECTRON_RUN_AS_NODE npx playwright test --config e2e/playwright.config.ts e2e/prompt-rail.spec.ts --grep "streaming deltas do not reconstruct"apps/desktop:env -u ELECTRON_RUN_AS_NODE npx playwright test --config e2e/playwright.config.ts e2e/prompt-rail.spec.ts— 11 passedapps/desktop, three runs per measured revision:env -u ELECTRON_RUN_AS_NODE npx playwright test --config e2e/playwright.config.ts e2e/native-transcript-perf.spec.ts --grep "warm native transcript scroll metrics"AI use
Select exactly one:
Tool(s) and scope: Maka analyzed the issue and review feedback, redesigned the state/selection boundary, implemented the fix and tests, captured the matched GIF, and ran the verification and performance comparison. All four commits retain the required
Generated-by: Makatrailer.Checklist
Does this PR entail a change in behavior?