test(desktop): reduce Electron coverage from 41 to 34 tests

[Astro-Han]

Summary

Desktop's Electron tier still started separate apps for fixture CSS and renderer-only settings/workbar behavior. This reduces the live-main budget from 41 tests in 20 files to 34 in 20 files, removing seven fixture launches and one full Host history traversal.

• Move fixture motion and Chromium Turn render-skipping checks into TranscriptRenderCost, reusing the existing shell and a 32-Turn slice of the ManyTurns data. Keep the original 120-Turn catalog scene. Restrict visibility events to the Turn itself: bubbled events from nested blocks must not satisfy the containment assertion.
• Move loading-surface Escape into a small SettingsOverlay composition boundary used by production, and remove the settings.chunk preload latch. Move settings reopen continuity into the existing SettingsSurface story harness, retaining intermediate missing-control/loading-alert detection and the same renderer snapshot cache across unmounts.
• Exercise the composer usage entry in the existing per-Session workbar/reload test. Exercise settings workbar hide/restore in the existing pending-rename test, which now also reads the renamed record back from the Host. These retain real AppShell wiring without adding a second shell host.
• Combine mounted-range bounds and quiet-frame reader displacement in one real Host paging traversal; retain progress to the oldest Turn, carried-boundary evidence, return to latest, and the tail range bound. Preserve the original post-page sampling and existing thresholds/timeouts.

Refs #4761. Merged main f4148595d; preserve its new sessions.observe latch while removing only the migrated settings.chunk latch. No changes are copied from #5192 (b55755651), and its worktree, branch, and PR are untouched. Both PRs touch transcript-scroll-cost.spec.ts and the budget; resolution must preserve #5192's new geometry protections. This PR retains main's quiet-frame displacement scope and does not claim #5192's input-period geometry coverage.

Add directory-scoped Electron admission instructions in apps/desktop/e2e/AGENTS.md: justify each boundary, prefer existing Node/component/browser coverage, and do not treat Host setup, reload, CDP, or an already-open window as an exemption. Correct the Storybook fidelity guide's obsolete instruction to send viewport/theme geometry to Electron, and describe the actual smoke runner's selected variants. The project-root Agent instructions remain untouched. Both Storybook directories now have short AGENTS.md entry points linking to the same fidelity and Electron admission rules, with the existing unit-test locations and shared Desktop build/smoke commands.

Verification

• Affected Electron suite: 8 tests passed in 1.1 minutes, zero retries.
• Built Storybook; targeted play smoke for ManyTurns, TranscriptRenderCost, SettingsLoadingEscape, GeneralReopenKeepsReadyControls, and GeneralCachedRevalidation. The three added behavioral stories also pass at 4x CPU throttle with the existing 15-second smoke timeout.
• Mutation checks reject removal of the Escape listener, loss of the settings snapshot cache, removal of Turn containment, and restoration of fixture transition: all 0.01ms. All mutations reverted.
• Desktop typecheck, renderer architecture check (103 checker tests), E2E budget check (5 checker tests), settings resource/cache/authority unit tests (20), changed-file lint, and git diff --check.
• Reproduced the CI Knip failure from two unused SettingsModal re-exports, removed them and their architecture-ledger dependency edge, and reran Desktop Knip, all four Desktop typechecks, architecture validation, and changed-file lint successfully.
• The unmodified main paging tests passed in 33.6 seconds combined; the merged traversal passed in 21.4 seconds in a local comparison. This is one local observation, not a benchmark or CI estimate.

Full Storybook smoke now passes: 337 stories / 364 theme renders using the CI runner's four concurrent workers. Full Electron catalog was not run. No retries or timeouts were increased. No product scroll implementation or CSS changed.

• Follow-up CI timeout: replaced 40 incremental paints in TranscriptRenderCost with two distant scroll positions, retaining all motion/render-skipping assertions. The old story timed out at 12x CPU throttle; the updated story passes at 8x and isolated 12x (12x alongside the full catalog still timed out). Removing Turn skipping and restoring transitions each fail the corresponding assertion. After merging main, Desktop typecheck, Knip, architecture/budget checks, lint, and 6 preload observation regression tests pass. New remote CI is not yet verified.

AI use

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope: OpenAI Codex implemented the test consolidation, renderer overlay extraction, and local validation.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No

[zhiiw]

Scoped review (second seat) at exact head 93094cac (verified unchanged at review time). Scope: protection equivalence of the Electron→lower-tier migration, merged/paginated test retention, SettingsOverlay Escape/Suspense lifecycle, Storybook fixture fidelity, the e2e-budget/architecture ratchets, and the AGENTS.md/FIDELITY layering.

The gate is red on this head: the hosted test job fails in Storybook smoke on the new TranscriptRenderCost story — see the inline [P1]. The migration's core evidence vehicle does not finish there, so the equivalence argument is unproven at the merge gate until that is resolved.

What I verified before the gate:

• The merged Electron traversal keeps every dropped assertion: mounted-range bound (mountedMax sampled per page, MOUNTED_TURNS_MAX), reaching turn-prompt-rail-1, per-boundary reader displacement (≤40px, with the probe-must-have-seen-something guard), return-to-latest, and the tail bound all survive inside the one Host paging journey — the two deleted standalone tests' assertions are present, not just nearby.
• The Storybook moves use the production subjects, not copies: SettingsLoadingEscape drives the real new SettingsOverlay boundary (which is also the production wrapper), including the modifier-Escape matrix, unmodified-Escape dismissal while suspended, and Escape no longer being prevented after dismissal (the last check is slightly stronger than the deleted E2E). GeneralReopenKeepsReadyControls reproduces the intermediate-state probe (MutationObserver watching for the loading alert and missing controls) across a real unmount/reopen with the retained snapshot cache.
• The Escape/Suspense lifecycle move is behavior-preserving: the listener now lives in SettingsOverlay, mounted exactly when Settings is open — installed before paint for both the fallback and the resolved surface, same modifier/defaultPrevented rules, latest callback read through a ref; the preload settings.chunk latch and its lazy-chunk consult are gone with zero residual references.
• The transcript-cost story's assertion is the right shape: static transition-property: none on the Turn and its pseudo-elements (stronger than counting), zero transitions/animations/unfinished after a real scrollport sweep, and content-visibility skipping restricted to the Turn itself (event.target !== event.currentTarget filters bubbled nested-block events). One honest difference from the E2E original: the story dispatches synthetic wheel events and drives scrollTop directly instead of CDP input — adequate here because the subjects are browser rendering behaviors, not the input pipeline.
• Ratchets hold: check:e2e-budget passes locally (34 tests / 20 files, matching the title), the architecture ledger's token/edge adjustments match the deletion (the settings-modal re-export edge is gone; remaining SETTINGS_NAV consumers import the owning module directly), and the AGENTS.md/FIDELITY layering is consistent across the three directories with the same escalation rules.

Local boundary, stated honestly: I ran the budget checker, the Storybook typecheck, and the affected settings unit suites on my machine; I did not run the Electron or full Storybook suites. One unrelated local observation, not this PR's account: app-shell-session-settings-actions fails 13/13 on my machine against current main content (a locale-catalog fallback gap, getShellCopy(undefined) returning undefined) — flagged separately to the maintainers, outside this review's scope.

---

Automated review notice: This comment was posted by an automated review agent operated by zhiiw. It is not an independent human review and does not replace one.

简体中文

第二席范围审查，锚 93094ca（复核未漂）。门禁红：hosted test 在 Storybook smoke 挂在新 TranscriptRenderCost story（行内 P1）——迁移的核心证据车在门禁上没跑完，修好前等价性论证在合并闸上未成立。闸前已核实：合并后的 Electron 遍历保留了被删两个独立测试的全部断言；Storybook 迁移全部用生产主体而非副本（SettingsOverlay 即生产包装、reopen 探针跨真实卸载/重开）；Escape/Suspense 生命周期搬迁行为保持（监听挂点时机/修饰键/defaultPrevented/最新回调全等价，preload 锁零残留）；渲染成本 story 的断言形状正确（静态 none 比计数更强、嵌套块冒泡不计入）；棘轮成立（预算检查本机过、架构账本与实际删除一致、三目录规范分层一致）。本机边界如实：跑了预算检查/story 类型检查/受触设置单测；没跑 Electron/全量 Storybook。一条与本 PR 无关的本机观察（main 上 locale 目录回退缺口导致一个设置测试全挂）另报维护者。

[jackwener]

Re-review at 2befb710c0602195f65ab6bf5157868ba21ab9e4. I reviewed the previous revision of this pull request, so this is not a first reading. No blocking issue; two non-blocking notes below.

Continuity with the previous revision. Rather than assume the merge left this change intact, I compared the pull request's own contribution against each base: it is +381/−421 across fourteen files here versus +380/−421 across the same fourteen before, and the single differing line is in the render-cost story. Everything else I checked previously therefore still applies without re-reading.

The removed Electron coverage is accounted for. I traced each removed test to where its protection now lives, rather than relying on the budget file's description:

• The composer usage entry's assertions were folded into the surviving per-Session workbar test, which now opens the panel through that entry and asserts the inspector is visible in the right panel.
• The workbar hide-and-restore protection was folded into the surviving titlebar rename test; the assertions are present there, both that the workbar is hidden while Settings is open and that its toolbar returns on close.
• The Escape-during-loading test became a story that is faithful and slightly stronger: it keeps the distinction between a modified Escape, which must remain unprevented, and a plain Escape, which must be prevented, and it adds an assertion the Electron test never made — that the listener stops preventing Escape once the surface unmounts.
• The Settings reopen test became a story that inspects every DOM commit through a mutation observer rather than only the settled screen.
• The three paging tests became one traversal that keeps both the bounded mounted range and the reader-displacement assertion, and it guards against a vacuous pass by first requiring that boundaries were observed and that at least one carried a Turn across.

The budget and architecture ratchets agree with the change. The per-file counts fall from six to five, five to two and four to one, which matches the stated reduction, and the architecture entries track the code: a removed window capability, the lower token count for the overlays module and the dropped navigation dependency.

The fidelity document corrects a false premise, and that is the most valuable part of this change. The previous text stated that the smoke run mounts every story once at a single width in light and maintains no viewport or theme matrix, and concluded that responsive and theme behaviour therefore belongs in Electron. I checked the smoke runner: it selects a narrower viewport for story identifiers containing narrow, runs additional palettes for an explicit sentinel list, enables forced colors for another list, and applies these through media emulation. The old text was wrong, and its wrongness is a plausible reason Electron accumulated work that never needed it.

Two non-blocking notes.

1. The overlay's wiring is no longer covered anywhere. The new story renders the extracted overlay directly with a synthetic suspending child, and its comment describes this as the real path where the shell owns dismissal while the Settings chunk suspends. The shell component is not in that render. Restoring the shell to a plain Suspense boundary, which still type-checks, would break Escape during a real Settings load while this story stays green — and both the Electron test and the preload latch that could have caught it are removed here. This change's own admission guide asks that a replacement be shown to detect the original defect, preferably by reverting the behaviour; reverting this wiring does not turn anything red. Rendering the real shell with Settings open and its chunk suspended would close this, as would a lower-tier assertion on the wiring.

2. The render-cost story now samples two positions instead of forty. Every assertion is unchanged and no timeout or retry was added, which is the right way to respond to a slow test. The traversal is nonetheless coarser: a transition that only fires at an intermediate scroll offset has fewer opportunities to be observed than before. This is a deliberate, disclosed trade for stability under contention and it does not block, but it is a reduction in observation surface in a change whose thesis is that protection is preserved.

Coverage and limits. I read the full diff and traced each removed test to its replacement, checked the budget and architecture ratchets against the code, verified the fidelity claim against the smoke runner source, and read the three admission guides for consistency; they agree with each other and with the tier model. I ran no tests, build, Storybook smoke or Electron session, so I did not observe the replacements executing, and my continuity argument above rests on comparing patches rather than on running them. At the time of this review the hosted test check on this commit was still running; its previous failure on the earlier commit is not carried forward here.

---

Automated review notice. This review was produced by an automated reviewer (agent seat kabi-opus, model Claude Opus) acting on its own review identity. It is posted from a shared machine account, so the seat is named here because the account alone does not identify the reviewer. It is not an independent human review and does not substitute for one.

[jackwener]

Approve at 2befb710c0602195f65ab6bf5157868ba21ab9e4, completing the review I posted earlier against this same commit. Nothing in my coverage blocks this change; the two notes I raised remain open and remain non-blocking.

What changed since my earlier note on this commit. The hosted check is now terminal and successful. I read the run's own log rather than relying on the summary: the render-cost story that previously timed out now reports a pass, and the smoke completed its full catalogue. The earlier failure was on a superseded commit and is not carried forward.

Why I am comfortable approving. I traced every removed Electron test to where its protection now lives and confirmed the assertions exist at the destination rather than accepting the budget file's description; the paging merge keeps both its bound and its reader-displacement check and guards against a vacuous pass; the Escape migration preserves the prevention semantics and adds a teardown assertion the deleted test never had; the budget and architecture ratchets agree with the code; and the fidelity document's corrected claim about viewport and theme coverage matches the smoke runner's source. The three admission guides are consistent with each other and with the tier model they describe.

The two notes stay open, and I am not treating them as resolved by this approval.

1. The extracted overlay's wiring is still not covered anywhere: the story renders the overlay directly, so restoring the shell to a plain Suspense boundary would break Escape during a real Settings load while the story stays green. By this change's own admission guide, reverting that wiring should turn something red, and it does not.
2. The render-cost story now samples two positions rather than forty. Every assertion is unchanged and no timeout or retry was added, which is the right response to a slow test, but a transition that only fires at an intermediate offset has fewer chances to be observed than before.

Neither is a correctness defect in shipped behaviour, which is why they do not block; both are coverage observations in a change whose thesis is that protection is preserved, which is why I am restating rather than dropping them.

Limits of this approval. I read code and hosted results only. I ran no tests, build, Storybook smoke or Electron session, so I have not observed any replacement executing; my confidence that the earlier revision's conclusions still apply rests on comparing the change's own patch against each base, not on running it. This approval covers the tiering and coverage-equivalence question it was asked about; it is not a statement about the wider desktop surfaces these tests touch.

---

Automated review notice. This review was produced by an automated reviewer (agent seat kabi-opus, model Claude Opus) acting on its own review identity. It is posted from a shared machine account, so the seat is named here because the account alone does not identify the reviewer. It is not an independent human review and does not substitute for one. No merge was performed.

[zhiiw]

Follow-up at exact head 2befb710 (verified unchanged; test completed/success on this head).

The [P1] above is closed: TranscriptRenderCost now exercises two distant scroll positions instead of forty incremental paints (same motion/skipping contracts, bounded work), and the hosted Storybook smoke passes on this head. The merge-conflict resolution in preload.ts keeps #5217's new sessions.observe latch while keeping the migrated settings.chunk latch removed — verified in the merged file.

My earlier content findings stand as written. Remaining boundary unchanged: I did not run the Electron or full Storybook suites locally.

---

Automated review notice: This comment was posted by an automated review agent operated by zhiiw. It is not an independent human review and does not replace one.