test(desktop): move the composer slash, paste and Skill-draft journeys off Electron - #4786
Conversation
The render smoke waits on Storybook's `storyFinished` channel event, so every `play` function executes and a failed assertion inside one fails the lane. FIDELITY said the opposite — that the smoke mounts with autoplay disabled — which is how apache#4766's assertion-bearing stories came to be read as review-only scaffolding. Say what the lane actually does, and keep the two limits that are real: geometry and theme matrices still belong elsewhere because CI mounts once at 1280 in light, and a probe that must be installed before mount belongs in a test that owns the global, not in a story that can only observe after it renders. Generated-by: Claude Code
The transcript-wide IntersectionObserver must be built once, not once per delta. Two mechanisms hold that in series — ChatView memoizes the rail's turn list on persisted text, and PromptAnchorRail stabilizes `orderedTurnIdsRef` on the turn ids — and neither is observable from a story: the probe has to be installed before the rail's own observer, and a story can only observe after it has mounted. So this is a component test that owns the global, counting constructions across ten re-renders that grow only the streaming tail. Collapse the two spellings of the reading band onto `READING_BAND_TOP_PERCENT`, so asserting the `rootMargin` string is a contract about the band rather than a copy of the literal next to it. Generated-by: Claude Code
apache#2948 and apache#3289 were the only two flake reports before apache#4523, and both were this file. Nothing it asserts needs a layout engine. Each of the six cases lands at the tier that owns what it guards: - Which commands a state offers is pure, and `desktop-slash-command.test.ts` already covers it; only the rendering — two groups, 命令 then Skills, each row carrying its `/id` token — needs a browser. - Whether a `/` addresses a command is `slashCommandQuery`, pure, so it becomes a unit test alongside `skillMentionQuery`. - Path separators (apache#3849) and block-break line boundaries live in the patched Astryx `useTriggerMenu` and need a live Selection across text nodes; the same-content projection refresh (apache#2667) needs a portal whose identity survives a re-render. Those become stories, mounting the real Composer inside the real ComposerMentionsProvider over a scoped bridge so `invocableSkillListsEqual` is genuinely exercised. `PathSeparatorsAreNotTriggers` opens the menu first, from the same editor with the same synthetic input, so a menu that stopped opening at all cannot pass it. The command list is assembled in the story from the same three authorities app-shell reads, rather than extracted into `desktop-slash-command.ts`: the architecture ratchet counts dependencies per file, so moving the catalog import into the smaller module books new debt there instead of retiring any — the reason apache#4762 left the catalog query where it was. Generated-by: Claude Code
Six more cases whose subject is the browser, not the app: an undo
transaction, a paste that must not open a trigger menu, and a Skill token
redrawn after a draft scope switch. All of them run in the smoke's real
Chromium.
Undo is driven with `document.execCommand('insertText' | 'undo')` rather
than synthetic keys, because `userEvent` events are not trusted input and
the browser's undo stack ignores them — the same reason apache#3787 made the
paste path use `insertHTML` in the first place.
The deleted spec counted `sessionStorage` writes to prove a paste
synchronizes the draft once. The story spies on `draftPersistence.write`
instead: the seam `chat-composer-region.tsx` actually calls, one level
above the storage the counter was reading downstream of.
Generated-by: Claude Code
It was the last CI test sharing the warm prompt-rail window, and sharing bought it nothing: search reads the Host through the bridge and renders nothing, so it needs neither that window's compositor nor its between-test reset. Moving it onto an ordinary per-test window retires the cross-test readiness bleed behind apache#4707. The warm fixture itself stays — `native-transcript-perf` still needs a window whose compositor has settled. Generated-by: Claude Code
It failed on CI with `element(s) not found` after Enter, and the failure snapshot carries the whole signature apache#4573 described: the draft still sitting in the composer, no Turn, and 发送 already enabled by the time the assertion gave up. A successful send clears the draft, so the keypress was dropped by `sendCurrent` while admission was still unresolved — not rendered slowly. apache#4577 fenced the three specs that had failed by then and left the rest to adopt `awaitSendReady` when they failed. This one just did. Generated-by: Claude Code
The scroll authority decided "this was content, not the reader" from the geometry having changed since the last event. During growth it always has, so a reader's own scroll arrived carrying a changed `scrollHeight` and was discarded along with it: the pin stayed set, and the next ResizeObserver delivery wrote the view straight back to the tail. Scroll up while an answer streams and the transcript pulls you back — the one moment a reader most needs to be believed. Geometry cannot decide it, because during growth both readings are true. What content can do to the offset is bounded: anchoring pushes it down by at most what was inserted above the reader, and a transcript that shrank clamps it up to the new end. A move outside that band has no explanation but the reader, so that is what decides now. The case the old rule existed for is unchanged and still covered: growth that outran this authority's own write moves the offset by nothing while the tail runs away, which is inside the band and so is not a reader. This is what made `ReaderScrolledUpIsNotPulledBack` and `DockAffordanceReturnsToTail` fail on CI and never locally — CI is slow enough for a resolution to land between the write and its event. Both are about the reader being left alone, so both were telling the truth. Generated-by: Claude Code
Review of the previous commit found the band right and the reading of it wrong, in two ways that cancelled part of the fix and cost some of what already worked. Content can pull the offset up as well as push it down: anchoring answers a removal above the reader — a tool block folding, a status line going — by moving the offset up by exactly what went. The lower bound covered only end-clamping, so a fold above a pinned reader read as a reader and quietly stopped the transcript following its own answer. And the slack was `PIN_THRESHOLD_PX`, which is a distance from the tail and says nothing about how far an offset may miss its explanation. Ten pixels of it, spent afresh on every event, swallowed every move smaller than that: a reader crossing the transcript on a trackpad was never heard at all, growth or no growth. What is left over is now kept between events and measured against the sub-pixel rounding it actually is, so a run of small moves adds up to the reader it plainly is. Three cases cover the corrections: a slow reader, a fold above the reader, and a scrollport that grew. Re-baselining every event turns the first red; dropping the removal term turns the second red. Generated-by: Claude Code
jackwener
left a comment
jackwener
left a comment
There was a problem hiding this comment.
Approving at exact head 23b6b1a5779d4e2e81851ba6317a83e8a3789268. No P0 or P1. Three P2s and one P3, none blocking. Four reviews ran on this, each sealing before reading the others; a fifth is still replaying the named regressions against their historical implementations and I will post that separately.
The mutation table is real, and it is second-tier evidence
Every moved assertion having been shown to redden against an injected defect is more than most moves of this kind arrive with, and five independent seam mutations reproduce it — getCharacterBeforeNode → '', createsLineBoundary → false, invocableSkillListsEqual → false, a failing paste execCommand, and redrawSkillTokens → false each turn their story red.
That proves the assertions run. It does not prove they inherit what the deleted tests intercepted, and two cases below show the difference concretely.
[P2] Command availability never crosses the production AppShell seam
composer-slash-menu.stories.tsx:64-68 assembles the options itself and passes hasSession directly, so the real activeId → hasSession binding at app-shell.tsx:1326-1361 is never exercised.
Changing production hasSession: Boolean(activeId) to false leaves BeforeAnySessionExists, InAnActiveSession and PickingACommandWritesItsInvocation all green, along with 13/13 focused tests. The deleted E2E fails immediately on that mutation — four rows expected, two present. The ordinary consequence is an active Session silently losing /compact and /side, with no test able to see it.
Keep the pure predicate test, and add one story or test that reads the menu rows through a real AppShell active-Session state.
[P2] /compact coverage stops before execution
The deleted slash-command-menu.spec.ts:52-110 covered the click, the Host's sessions.compact(), the completion event, the composer clearing, the absence of a failure, a normal send afterwards, and /compact never reaching the model. PickingACommandWritesItsInvocation asserts that selecting it writes /compact and closes the menu.
Making the compact branch at app-shell.tsx:1958 unreachable leaves that story green — and the entire Desktop test:dist suite green at 2157/2157. With that mutation live, /compact falls through to an ordinary model send. The deleted E2E failed on it.
An AppShell/Host integration test that submits the invocation and asserts the compaction happened, the command did not reach the model, and the next send still works would close it.
[P2] The #4707 attribution
Moving fixture-thread-search onto an ordinary window does not retire #4707. That issue is following the tail does not ask for the history above it (transcript-scroll.spec.ts:712) — a pinned tail admitting an earlier-range load, in a spec already deleted by #4766, and the issue text separates it from the prompt-rail snapshot bleed.
What is true and worth claiming instead: promptRailWindow / promptRailWorker now have one consumer, native-transcript-perf.spec.ts, whose tests skip unless MAKA_TRANSCRIPT_PERF=1 — so after this PR CI no longer exercises the reused worker window at all.
[P3] The summary's production-change claim is now out of date
It says prompt-anchor-rail.tsx is the one production change. On this head there is a second, added after that sentence was written: transcript-scroll-authority.tsx, and it is a behaviour fix rather than a test move.
It is a good fix, and it deserves more visibility than a stale summary gives it. The old rule treated any geometry change as content moving rather than the reader — and during streaming the geometry always changes, so a reader who scrolled while an answer grew was discarded along with it, the pin held, and the next growth wrote the view back to the tail. The comment puts it exactly right: scrolling away from a streaming answer is the one moment a reader most needs to be believed.
The replacement bounds what content can account for — anchoring pushes the offset down by at most the growth in maxScroll, a shrunk transcript clamps it up by at most the loss — and treats anything outside that band as the reader's. The edges hold: tail growth gives topDelta = 0 inside the band; a reader scrolling up 500px against 200px of insertion gives −300, outside it. a reader who scrolls up while the answer grows is still the reader covers it.
Worth updating the summary, and worth considering whether a user-visible scroll fix belongs in a test-move PR at all.
Verified
The other ten mappings hold, including the pure slash and skill query rules, path separators, DOM block breaks, same-content projection identity, paste undo granularity, menu suppression, single draft write, and Skill draft scope restoration. 12 of 13 is accurate — six from slash-command-menu.spec.ts, five from composer-undo.spec.ts, one from composer-skill-invocation.spec.ts, with composer-directory-reference.spec.ts staying; the Playwright list moving 89 → 77 matches.
The reason given for keeping that spec also holds: add, duplicate, remove and foreign-host are all in composer-directories.test.ts as hook state, chip DOM stays covered by the E2E that remains, and reload, cwd and the transcript's must-not-contain assertions have nowhere else to go.
The one production change I reviewed in full — READING_BAND_TOP_PERCENT — is behaviour-preserving to the bit. 34/100 === 0.34 exactly, and 100 - 34 is exactly 66, so both the geometry seed and the rootMargin string are unchanged. The comment's reason for whole percent is also correct rather than stylistic: storing 0.34 would render the margin as -65.99999999999999%.
test is green on this head and the merge tree against current main is clean. This is a test move, so the merge decision is a human's.
简体中文
在 23b6b1a5779d4e2e81851ba6317a83e8a3789268 上批准。没有 P0/P1。 三条 P2、一条 P3,均不阻塞。四次评审在这个 head 上进行,各自封存后才互看;第五席仍在把那些具名回归回放到它们的历史实现上,结果我会另行发布。
变异表格是真的,但它是第二层证据
「每一条被搬移的断言都被证明会对着注入的缺陷变红」——这比同类搬移通常带来的东西多得多,而且有五处独立的接缝变异复现了它:getCharacterBeforeNode → ''、createsLineBoundary → false、invocableSkillListsEqual → false、失败的 paste execCommand、redrawSkillTokens → false,各自让对应 story 变红。
这证明了断言在跑。它不证明它们继承了被删测试所拦截的东西,下面两条把这个区别摆成了具体的。
[P2] 命令可用性从未穿过生产 AppShell 接缝
composer-slash-menu.stories.tsx:64-68 自己组装选项并直接传入 hasSession,所以 app-shell.tsx:1326-1361 那个真实的 activeId → hasSession 绑定从未被演练。
把生产的 hasSession: Boolean(activeId) 改成 false,BeforeAnySessionExists、InAnActiveSession、PickingACommandWritesItsInvocation 全部保持绿,13/13 focused 测试也全绿。而被删的 E2E 会立刻在这个变异上失败——应有四行,实际只有两行。普通后果是:一个 active Session 静默失去 /compact 与 /side,而没有任何测试看得见。
保留纯 predicate 单测,再加一条通过真实 AppShell active-Session 状态读取菜单行的 story 或测试。
[P2] /compact 的覆盖停在执行之前
被删的 slash-command-menu.spec.ts:52-110 覆盖了点击、Host 的 sessions.compact()、完成事件、composer 清空、没有失败、之后的一次正常发送,以及 /compact 从不抵达模型。而 PickingACommandWritesItsInvocation 断言的是「选中它会写入 /compact 并关闭菜单」。
让 app-shell.tsx:1958 的 compact 分支变为不可达,这个 story 仍然绿——而且整个 Desktop test:dist 套件也绿,2157/2157。 在那个变异下,/compact 会落入一次普通的模型发送。被删的 E2E 会失败。
补一条 AppShell/Host 集成测试:提交该 invocation 后断言 compaction 真的发生、命令没有抵达模型、下一次正常发送仍然可用。
[P2] 关于 #4707 的归因
把 fixture-thread-search 挪到普通窗口并没有让 #4707 退休。那个 issue 是 following the tail does not ask for the history above it(transcript-scroll.spec.ts:712)——钉住尾部却接纳了一次更早范围的加载,而那个 spec 已经在 #4766 里被删掉了,并且 issue 正文把它与 prompt-rail 快照串扰分开了。
真正为真、也更值得写的是:promptRailWindow / promptRailWorker 现在只剩一个消费者 native-transcript-perf.spec.ts,而它的测试除非 MAKA_TRANSCRIPT_PERF=1 否则跳过——所以这个 PR 之后,CI 根本不再演练那个被复用的 worker 窗口。
[P3] 摘要里关于生产改动的那句话已经过期
它说 prompt-anchor-rail.tsx 是唯一的生产改动。在这个 head 上还有第二处,是在那句话写下之后加进来的:transcript-scroll-authority.tsx,而且它是一次行为修复,不是测试搬移。
这是个好修复,而且它值得比一句过期摘要更多的能见度。 旧规则把任何几何变化都当作「内容在动」而非「读者在动」——而在流式输出期间几何始终在变,于是一个在回答生长时滚动了的读者,就连同那次变化一起被丢弃了:pin 保持不变,下一次生长把视图写回尾部。注释说得非常准确:从一个正在流式输出的回答旁滚开,恰恰是读者最需要被相信的那一刻。
新的写法给「内容能解释的位移」划了一条带——anchoring 最多把偏移下推 maxScroll 的增量,收缩的 transcript 最多把它上钳所减少的量——带外的位移就是读者的。边界是成立的:尾部生长给出 topDelta = 0,落在带内;读者在 200px 插入之下向上滚 500px 给出 −300,落在带外。新测试 a reader who scrolls up while the answer grows is still the reader 覆盖了它。
建议更新摘要,也值得想一想:一个用户可见的滚动修复,是否该出现在一个测试搬移 PR 里。
已验证
其余十条映射成立,包括纯 slash/skill 查询规则、路径分隔符、DOM 块边界、同内容 projection identity、paste 撤销粒度、菜单抑制、单次 draft 写入,以及 Skill draft 作用域恢复。12 of 13 是准确的——slash-command-menu.spec.ts 六条、composer-undo.spec.ts 五条、composer-skill-invocation.spec.ts 一条,composer-directory-reference.spec.ts 留下;Playwright 列表 89 → 77 与之吻合。
保留那个 spec 所给的理由也成立:add、duplicate、remove 与 foreign-host 都以 hook 状态存在于 composer-directories.test.ts,chip DOM 仍由留下的 E2E 覆盖,而 reload、cwd 以及 transcript 的「必须不包含」断言无处可去。
我完整评审的那一处生产改动——READING_BAND_TOP_PERCENT——逐位保持行为不变:34/100 精确等于 0.34,100 - 34 精确等于 66,所以几何种子与 rootMargin 字符串都没有变化。注释给出的「用整数百分比」的理由也是实证而非风格:存成 0.34 会让 margin 渲染成 -65.99999999999999%。
这个 head 上 test 为绿,与当前 main 的合并树干净。这是一次测试搬移,合并与否由人决定。
Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.
|
Scope note on the approval above: it landed on The increment touches only The band itself is unchanged. What changed is how the band's edge is measured, in two ways: A dedicated rounding tolerance. The unexplained residue now accumulates across events, rather than each event being judged alone: unexplained += topDelta - Math.min(explainedHigh, Math.max(explainedLow, topDelta));
const readerMoved = Math.abs(unexplained) > GEOMETRY_ROUNDING_PX;This closes a hole I did not find. When I checked the previous version I reasoned through tail growth and through a reader scrolling 500px against 200px of insertion, and both classified correctly — but I never considered a reader moving in steps each smaller than the tolerance. Under per-event judgement every one of those steps is discardable as content, and the reader is never believed no matter how far they travel. So the fix is better than the version I reviewed, and the reason I can say that is that the author found the case my edge-checking missed. 简体中文关于上面那次批准的范围说明:它落在了 增量只触及 带本身没有变。 变的是如何度量带的边缘,有两处: 一个专用的舍入容差。 未被解释的残差现在跨事件累积,而不再是每个事件单独判断: unexplained += topDelta - Math.min(explainedHigh, Math.max(explainedLow, topDelta));
const readerMoved = Math.abs(unexplained) > GEOMETRY_ROUNDING_PX;这补上了一个我没有找到的洞。 我检查上一个版本时,推了尾部生长、也推了「读者在 200px 插入之下向上滚 500px」,两者分类都正确——但我从未考虑过一个每一步都小于容差的读者。 在逐事件判断下,那样的每一步都可以被当作内容而丢弃,于是无论他滚了多远,都不会被相信。 所以这个修复比我评审过的那一版更好;而我之所以能这么说,是因为作者找到了我边界检查漏掉的那个用例。
|
Ablation: removing it fails nothing, and it cannot — the offset is never past the end it is read from, so `maxScroll - lastScrollTop` is always the looser of the two and the minimum was never taking it. What content did to the offset is one number, and the band is between nothing and that. Generated-by: Claude Code
Review of the move found the leaf assertions carried over and the wiring under them left behind, in two places, and it was right on both. The stories are handed `hasSession`, so `Boolean(activeId)` in app-shell is theirs to assume. Setting it to `false` in production leaves every slash story green while an active Session silently loses `/compact` and `/side`. And selecting `/compact` only writes an invocation into the draft. What that draft does when submitted — `sessions.compact()`, a status change, a composer that clears, and above all a command that never reaches the model — was covered by the deleted spec and by nothing after it. Making the branch unreachable leaves the story green and the whole 2157-test Desktop suite green; the deleted E2E failed on it. Neither has a home outside Electron today: the routing is inline in app-shell, `src/renderer` has no test directory, and opening a seam there is what the architecture ratchet exists to refuse. So they stay, in one test named for the two claims it holds. 11 of 13 moved, not 12. Both mutations confirmed red against it. Generated-by: Claude Code
|
Three P2s all held up, and two of them were real coverage losses rather than description problems. Fixed at [P2] The AppShell seam and [P2] Neither has a home outside Electron today. The Verified: passes in 9.0s, and red under both of your mutations — [P2] The #4707 attribution. Wrong, as you say — #4707 is pinned tail can load earlier history after its settle check, already deleted by #4766. I have replaced the claim with the one you offered instead, which is both true and stronger: [P3] The stale summary. Already corrected before your scope note landed; the description now names both production changes and gives the scroll fix its own section. On whether a user-visible scroll fix belongs in a test-move PR: a fair question, and I do not think the answer is obvious. It is here because the two stories it fixes are the ones this series depends on — a tier migration whose replacement stories are red is not a migration — but it is a core-authority behaviour change with its own blast radius, and splitting it out so it can be reviewed, bisected and reverted on its own is a defensible call. Happy to split if a maintainer prefers that. One correction to your increment note, since it credits me with something that was not mine: |
The rule that tells a reader's scroll from content moving under them carried a 2px tolerance and a running total of what the content could not explain. Neither was measured. `packages/ui` runs on a fake DOM of integers, so no test there could reach either one, and an ablation passed whatever the numbers were. Measured in a real layout engine, on fractional heights, through both paths that move the offset without the reader: native anchoring misses its own band by exactly one pixel on about a quarter of the frames it compensates, and clamping never misses. So the tolerance is one pixel rather than two, and the running total has to go — the error is bounded per event but one-directional across a stream, so accumulating it turns a pixel of arithmetic into a drift that crosses any threshold and hands a streaming reader back to the tail. The story holds the measurement rather than the conclusion: shrink the constant and the misses are reported as gestures, widen it and a two pixel gesture goes unheard. Generated-by: Claude Code
|
On splitting the The two stories this series moves work onto, The split I would accept is a merge constraint — some part having to land on its own schedule or through a different path. There isn't one here: both halves go to the same branch at the same time, and neither is independently mergeable in a state anyone would want. What did change since the approval is the shape of the fix itself, and it is worth a second look because it is smaller now, not larger. The band comparison mixes CSSOM's two number systems, and the first version guessed at that with a 2px tolerance plus a running total of unexplained movement. Neither was falsifiable in The constant is now pinned from both sides: at |
CI caught the tolerance being cut to one pixel: a real transcript's content steps miss their own band by more than the probe scroller's do, so growth was read as the reader and the view stopped following the tail mid-answer. Two pixels is what the band actually needs, and the reason it looked too wide is that it was being spent everywhere. An event that finds the content unchanged has no rounded reading in it — the band is a point, and the offset either moved or it did not. So the slack belongs to events the content moved, and a reader inching down a settled transcript is heard exactly rather than swallowed by arithmetic that did not happen to them. Generated-by: Claude Code
|
Follow-up to the approval above: the historical replay came back clean. I said I would post it either way. The stories were applied unchanged on top of the commits that produced the behaviours they name, and both selected regressions turn them red deterministically:
No P0 or P1 from that pass. The three P2s and one P3 in the approval stand as written, and the approval does not need revisiting. Worth stating why this pass was run separately at all. The PR's own mutation table — a defect injected into production, the replacement confirmed red — establishes that the assertions execute and are wired to the code they name. It cannot establish that they catch what the deleted tests caught, because the injected defect is chosen by the same person who wrote the replacement. Replaying against the implementation that actually shipped the regression removes that degree of freedom, and here it agrees with the table rather than contradicting it. That agreement is the useful outcome: for these two, the move preserved interception rather than merely preserving green. 简体中文对上面那次批准的跟进:历史回放的结果是干净的。 我说过无论结果如何都会发。 这些 story 被原样叠加到了产生它们所命名行为的那些提交之上,两个被选中的回归都确定性地让它们变红:
这一轮没有 P0/P1。 批准中的三条 P2 与一条 P3 按原文成立,批准本身无需重新考虑。 值得说明为什么要单独跑这一轮。PR 自带的变异表格——在生产代码里注入缺陷、确认替代测试变红——只能确立「这些断言会执行,并且确实连着它们所命名的代码」。 它无法确立「它们能抓住被删测试所抓住的东西」,因为那个被注入的缺陷,是由写这些替代测试的同一个人挑选的。 回放到真正发布过该回归的那个实现上,消除了这一个自由度——而在这里,它与表格是一致的,不是相反。 这种一致本身就是有用的结果:对这两条而言,这次搬移保住的是拦截力,而不只是绿色。
|
2 participants
Summary
PR 2 of #4761: move the composer's slash-command, paste and Skill-draft journeys out of Electron. Eleven E2E tests are deleted and replaced in this same commit range — nine as Storybook
playstories in real Chromium, three as pure unit assertions — plus one supplementary component test the plan calls for, and one new Electron spec holding the two claims that turned out to have no home outside it.The slash menu goes first because #2948 and #3289 were the only two flake reports before #4523 and both were
slash-command-menu.spec.ts. Nothing it asserted needed a layout engine; it was a state machine on a deterministic fixture backend, paying Electron's price for it.Each assertion landed at the tier that owns what it guards, not at the tier that is cheapest:
desktop-slash-command.test.ts; only the rendering moved/idtoken are not/addresses a commandchat-input-behavior.test.ts(slashCommandQuery,skillMentionQuery)composer-slash-menu.stories.tsxuseTriggerMenu; needs a liveSelectionacross text nodesComposerMentionsProvidersoinvocableSkillListsEqualactually runscomposer-paste.stories.tsxcomposer-skill-draft.stories.tsxredrawSkillTokensagainst a real contentEditableThree things worth a reviewer's attention:
composer-directory-reference.spec.tsstays in Electron. Its claims are reload persistence, the real session store'scwd, and a persisted transcript that must not containREADME.md/deep.txt/"status":"listed"— all squarely in tracking(desktop): move every test out of Electron E2E that does not need it #4761's "What stays in Electron". Its movable half (chip add/remove/duplicate/foreign-host) is already covered bycomposer-directories.test.ts. Moving it would delete coverage and add none.app-shell.tsxintodesktop-slash-command.ts. It shrank app-shell (nonTriviaTokens15601→15402) but added two new dependencies to the smaller module, which the per-file monotonic ratchet rejects — exactly what test(desktop): sink the /compact menu rule to a tested predicate #4762 documented when it left the catalog query in app-shell. Reverted; the stories assemble the list from the same three authorities, with a comment saying why.packages/ui.prompt-anchor-rail.tsxgetsREADING_BAND_TOP_PERCENT, collapsing the two spellings of the reading band (arootMarginstring and a geometry fraction) onto one constant, so the new test asserts a contract about the band rather than copying the literal beside it. Andtranscript-scroll-authority.tsxstops pulling a reader back to the tail when they scroll during growth — a behaviour change, described under its own heading below.Review found two claims the move had dropped, and both held up: the stories are handed
hasSession, soBoolean(activeId)in app-shell is theirs to assume rather than check; and selecting/compactonly writes an invocation, while what that draft does on submit —sessions.compact(), a status change, a cleared composer, and a command that must never reach the model — was covered by the deleted spec and by nothing after it. SettinghasSession: false, or making the compact branch unreachable, each leaves every slash story green and the whole 2157-test Desktop suite green.Neither has a home outside Electron today. The routing is inline in
app-shell.tsx,src/rendererhas no test directory at all, and opening a seam there is what the architecture ratchet exists to refuse. Soslash-command-compact.spec.tskeeps both, in one test named for them, red under both mutations. That is why this is eleven moved rather than twelve.The supplementary component test is
StreamingDeltasKeepThePromptRailObserver, written as a story in #4766 and removed. It countsIntersectionObserverconstructions, so its probe must be installed before the rail's observer — a story can only observe after it has mounted, which would leave therootMarginassertion permanently green. It is anode:testcomponent test that owns the global instead.fixture-thread-searchmoves off the warm fixture onto an ordinarywithE2eWindow. Search reads the Host through the bridge and renders nothing, so it needed neither the warm window's compositor nor its between-test reset.An earlier revision of this description credited that with retiring #4707. That was wrong, and review caught it: #4707 is pinned tail can load earlier history after its settle check, a different spec, already deleted by #4766. What is true is stronger —
promptRailWorkernow has exactly one consumer,native-transcript-perf.spec.ts, whose tests skip unlessMAKA_TRANSCRIPT_PERF=1, so after this PR CI does not exercise the reused worker window at all. The mechanism still stays, for PR 6.FIDELITY.mdsaid the smoke mounts stories with autoplay disabled, so aplayfunction was "a local review driver, not a CI test". That is false: the smoke waits on Storybook'sstoryFinishedevent, nothing passesembed, and #4766 landed 18 stories whose assertions are the coverage. Corrected, keeping the two limits that are real.Refs #4761
Verification
Every moved assertion was mutation-tested — the defect it claims to guard was created in the production source and the replacement confirmed red, then reverted:
desktopSlashCommandAvailabilitybefore-any-session-exists/{command.id}token spanin-an-active-session, +2onSelectwrites/idwithout the trailing spacepicking-a-command-writes-its-invocationgetCharacterBeforeNodeprefix →''(pre-#3849 behaviour)path-separators-are-not-triggerscreatesLineBoundary→ always falsea-block-break-is-a-line-boundaryinvocableSkillListsEqual→falsesurvives-a-same-content-projection-refreshexecCommandpaste-is-its-own-undo-stepplainTextPasteInputActiveRefstopPropagationpaste-synchronizes-the-draft-once, +2menuWasOpenEscape dispatchpaste-closes-an-open-trigger-menuredrawSkillTokens→return falsestaged-skills-survive-a-draft-scope-switchnextOrderedTurnIdsinstead of the stable refprompt-rail-observer-identityslashCommandQueryguards, removed one at a timechat-input-behaviorTwo mutations passed at first and the tests were fixed, not the mutations. Breaking ChatView's turn-list memo left the observer test green, because
PromptAnchorRailstabilizesorderedTurnIdsRefindependently — two mechanisms in series, and the test now says so and asserts the composed outcome. And removing thetriggerStart < nodeStartcomparison did not break the path-separator story, because the prefix fromgetCharacterBeforeNodeis what actually suppresses the trigger; the mutation was retargeted there.Checks run locally from the worktree:
The Electron E2E suite itself was not run locally. CI ran it: the first run failed on two specs this PR does not move, diagnosed below; the rerun was green.
The reader pulled back to the tail
Two Storybook stories from #4766 —
ReaderScrolledUpIsNotPulledBackandDockAffordanceReturnsToTail— failed on CI here and once on this branch's own base commit onmain, and pass locally every time. They were right, and what they caught is a product defect, so it is fixed here rather than recorded.TranscriptScrollAuthoritydecided that a scroll event was content rather than the reader by asking whether the geometry had changed since the last event. During growth it always has. So a reader's own scroll arrived carrying a changedscrollHeightand was discarded along with it: the pin stayed set, and the nextResizeObserverdelivery wrote the view straight back to the tail. In the product that is scroll up while an answer is streaming and get pulled back to the bottom — reachable on the ordinary path, needing nothing but a slow enough frame.Geometry cannot decide it, because during growth both readings are true at once. What content can do to the offset is bounded, though: anchoring pushes it down by at most what was inserted above the reader, and a transcript that shrank clamps it up to the new end. A move outside that band has no explanation but the reader, and that is the rule now.
The case the old rule was written for is unchanged and still covered by its own test: growth that outran this authority's write moves the offset by nothing while the tail runs away, which is inside the band and so is not a reader.
Reproduced deterministically before the fix, by driving the story's own sequence in a real Chromium and changing the content's height in the same task as the reader's scroll write — the interleaving CI's slowness produces by itself:
distance 537distance 0← pulled backdistance 537distance 537CPU throttling alone (8× and 20×, five runs each) never reproduced it, and neither did injecting continuous growth: the window is an interleaving, not a duration.
a reader who scrolls up while the answer grows is still the readerintranscript-scroll-authority.test.tspins it deterministically, and reverting the rule turns it red.What the band tolerates, and where it is spent
The band comparison mixes number systems: CSSOM gives
scrollTopas a double andscrollHeight/clientHeightas rounded integers, and native anchoring rounds the anchor's own positions separately again. So a step that is entirely the content still lands a pixel or two outside its own band. The first version of this rule guessed at that with a 2px tolerance plus a running total of the unexplained movement, and neither was falsifiable where it lived —packages/uiruns on a fake DOM of integers, so an ablation there passes whatever the numbers are.Product/Transcript Scroll Roundingmeasures it in a real layout engine on fractional heights, through both paths that move the offset without the reader:2pxThe running total is retired: the error is bounded per event but one-directional across a stream, so accumulating it drifts across any threshold and hands a streaming reader back to the tail — the same defect arriving the other way round.
What replaces it is not a different number but a smaller scope. The slack is the rounding of the content reading, so it is spent only on events where the content moved. An event that finds the content unchanged has no rounded reading in it — the band is a point, the offset either moved or it did not — and a reader inching down a settled transcript is heard exactly. That is what a slack applied unconditionally would swallow, and it is why no accumulator is needed to buy it back.
CI wrote the last part of this. An intermediate revision cut the tolerance to 1px on the strength of the probe scroller alone;
TailFollowsGrowthOutsideTurnsfailed, because a real transcript's steps miss by more than a synthetic one's. The story now holds both halves: shrink the constant and content is reported as gestures, spend it unconditionally and the settled-content reader goes unheard. Both mutations turn it red.It ships in this PR rather than its own because it is what the stories this series depends on were failing on: a tier migration whose replacement stories are red is not a migration.
Two E2E flakes this run surfaced
Both are pre-existing and neither is reachable from this diff. The other production change here is
READING_BAND_TOP_PERCENT, and it is bit-identical:34 / 100 === 0.34is true (both are0.34000000000000002442), and the template string equals'0px 0px -66% 0px'exactly. Neither failing spec uses a fixture this PR touches.new-task-reload.spec.ts:28— fixed here.expect(getByText(/Fake backend received: create history/))timed out at 10s. The failure snapshot carries the full #4573 signature: the draftcreate historystill sitting in the composer, no Turn, and 发送 already enabled by the time the assertion gave up. A send that lands clears the draft, so the Enter was dropped bysendCurrentwhile admission was unresolved — it was not rendered slowly, and a longer timeout would only have made the red slower.#4577 fenced the three specs that had failed by then and said the rest could adopt
awaitSendReadywhen they failed. This one did, so it gets the fence and a 20s assertion. The roughly 40 other bare-Enter sends are deliberately left alone: a fence with no failure behind it is the same guess as a timeout with no failure behind it.partial-history-notice.spec.ts:136— recorded, not fixed. After clicking 返回最新消息,expect(notice).toHaveCount(0)passed and then[data-turn-id="turn-partial-history-8"]never appeared. The failure snapshot shows why, and it is not slowness: the notice is back on screen ("正在查看较早的消息" with its button), the transcript holds onlyturn-partial-history-1, and the rail still shows all eight ticks because it reads the prompt index rather than the resident range.So the range did return to the tail — that is what unmounted the notice — and was then pushed back to the head, evicting turn 8. Widening the timeout cannot fix a settled wrong state.
The likely mechanism is the rail claim #4577 introduced: the test clicked turn 1's tick earlier in the run, and that hold re-aims at turn 1, which the shell answers by loading around turn 1 again. A viewport change from 1400×800 to 520×720 sits between the two. That is a hypothesis, not a diagnosis — separating it from the load-around path needs instrumentation this PR has no business adding, and the artifact from run 33881601168 has the trace for whoever picks it up.
AI use
Select exactly one:
Tool(s) and scope: Claude Code (Opus). Wrote the replacement stories and tests, ran the mutation matrix and the verification above, and drafted these commit messages. Tier placement per assertion, the decision to keep
composer-directory-reference.spec.tsin Electron, and the revert of the production extraction were reviewed against #4761 before landing. Affected commits carryGenerated-by: Claude Code; please retain the trailer on the squash commit.Checklist
Does this PR entail a change in behavior?