fix(inspector): close the follow-ups left on the trace panel

[ARE404]

Summary

The three substantive suggestions from #2018's approving review, plus its nits. Refs #1625.

One unreadable run no longer fails the whole trace.readRunEvents had no per-run catch, so a corrupt row — or a run whose header has gone missing — turned every inspector:trace into INSPECTOR_TRACE_FAILED, and retry hit the same row forever. You put it exactly right: a read failure is just another way a record can be unreadable, which is the rule this projection is built on. A failed run now folds into unreadableRecords as one counted gap rather than a guess at its size.

The hook has the regression test the original defect deserved. The P2 last round was a doc comment describing a subscription that did not exist, and the fix landed without a test that could catch that class. Four now render the hook for real — subscribe only while active, unsubscribe on hide, one read per closing burst, no read for streaming deltas, no read after the panel hides — and I confirmed they fail with the subscription removed rather than trusting that they would.

Two things made that possible, and one of them was my mistake to begin with: the fake DOM moved from ui-render-memo-boundary-contract.test.ts into fake-dom.ts so a second suite can render without duplicating it. I had concluded this repo had no hook-test idiom, having searched only for renderHook and testing-library. Thanks for the correction. useSessionTrace also takes its copy as an argument now instead of reading the locale context, so the hook is renderable without the UI package behind it — which matters here, since @maka/ui does not build in my worktree and a test I cannot run is how the Result typo reached CI.

A reported gap is never an empty session. With every record undecodable the panel showed "Nothing to trace in this session yet" beside "N unreadable records" — opposite claims, and the second is precisely what this surface exists to show.

Nits, all taken

• The IPC join test asserts something only the runtime-event ledger can satisfy; you were right that it passed with that ledger empty.
• The projection suite pins unreadableRecords, which only the IPC test referenced.
• Re-activation preserves the timeline instead of blanking it — not intentional, and it contradicted the file's own rationale.
• The coverage notice renders the turn counts the model computes.
• The recovery label is localized rather than leaking English into the zh UI; the model now carries the disposition structurally and the panel owns the wording.

Still deferred

Unbounded per-activation reads and timeline virtualization. Still a real limit; the fix is a paging contract over the trace rather than a patch here, and I would rather do it deliberately.

Verification

25 desktop inspector tests, @maka/core 755/755, projection 18/18. check-a11y, check-dead-css, check-copy, check-console and format:check clean. Renderer and main typechecks clean for every touched file.

🤖 Generated with Claude Code

[Astro-Han]

Nice to see this round land. The three items from the approving review are handled: the per-run catch covers the corrupt-event-row class, the hook now renders in a real suite, and the empty change stops the panel from claiming "nothing to trace" next to a gap report. Two things I found on top:

P2: corrupt run headers still fail the whole trace, one layer above the new catch

readSessionTrace wraps only readRunEvents (inspector-ipc-main.ts:46-63). The Promise.all over readSessionRuntimeEvents and listSessionRuns above it (:38-40) still rejects as a whole. In the real wiring (boot.ts:1210), listSessionRuns runs listSessionRunsForRecovery (agent-run-store.ts:333-348), which JSON.parses and normalizes every header row with no per-row catch: one corrupt header rejects the list, and retry hits the same row forever. That is the same failure class this PR removes for event rows, at a reachable layer. The missing-header ENOENT the new catch covers only occurs in the narrow race where a header disappears between list and read; the comment at inspector-ipc-main.ts:45-46 names that narrower case.

Options: per-row tolerance in the storage layer (it changes listSessionRunsForRecovery's contract for the recovery callers at boot.ts:557,877,947), or an explicit deferral in the PR body. Right now "one unreadable run no longer fails the whole trace" is true for only one of the two read legs. Not a merge blocker, but the claim is wider than the implementation.

P3: re-activation behavior has no test pin

The four hook tests assert subscription and read counts only. None renders a snapshot, and none goes active→hide→active. Reverting load(sessionId, true) to false passes the suite unchanged, so the one behavioral change this PR makes in the hook is unpinned. An active→hide→active sequence asserting the previous trace survives would cover it.

P3: unreadableRecords doc says "could not decode", the counter now also counts unread runs

core/session-trace.ts and the projection input doc describe records that fail to decode, but the run-level catch feeds the same counter. Update both comments to "read or decode". The unit mixing (one run counts as one gap) reads fine as a floor, but the doc should not promise decode-only.

P3: preserveTrace is a dead parameter

All three call sites pass true; the false branch is unreachable. Drop it or say why it stays.

P3: 'zh' | 'en' restates UiLocale

The injected copy types locale as a literal that duplicates UiLocale from @maka/core. A third locale will drift here; use UiLocale.

P3: housekeeping

• PR body says 25 desktop inspector tests; the four suites count 24 (ipc 5, panel model 6, hook 4, refresh 4, workbar layout 5).
• as never remains at inspector-ipc-main.test.ts:135,145.
• FakeElement is imported but unused in ui-render-memo-boundary-contract.test.ts after the extraction.
• flushRefresh hardcodes 450ms against TRACE_REFRESH_DEBOUNCE_MS = 400; export the constant so the two cannot drift.

[ARE404]

All correct. 41c4bb5d.

P2 — you're right, and the claim was mine to get wrong. I verified listSessionRunsForRecovery (agent-run-store.ts:333): rows.map with an uncaught JSON.parse, so one corrupt header rejects the list and retry hits it forever. My per-run catch covers only the event leg, and the PR body said "one unreadable run no longer fails the whole trace" without that qualifier.

Taking the deferral, not the storage change: per-row tolerance there alters what listSessionRunsForRecovery promises the recovery callers at boot.ts:557,877,947, and that is a decision about recovery semantics rather than about an observability read model. It's now stated in the code at the exact spot the reader would otherwise assume coverage, and the PR body no longer overclaims. If you'd rather it land as its own storage change, I'm happy to take it — it would want its own thinking about what a recovery caller should do with a partial run list.

P3 re-activation had no pin — confirmed by reverting it. The preserve behaviour passed the old suite unchanged, which is exactly the "unpinned behavioural change" you describe. The new test asserts that once a trace has arrived no later render goes back to nothing, and I checked it fails when the preservation is removed. Writing it also caught that my first assertion was wrong for a different reason (the mount and its loading frame legitimately have no trace yet), so it now anchors from the first frame that has content.

P3 docs — unreadableRecords now says "read or decode" in both the contract and the projection input, and documents that a whole unreadable run counting as one is a floor, since nothing is known about how many records it held.

Housekeeping, all taken:preserveTrace was dead at all three call sites and is gone; the injected copy takes UiLocale; TRACE_REFRESH_DEBOUNCE_MS is exported so the test derives its delay instead of duplicating 400 as 450; the IPC stub is typed against IpcMain['handle'] rather than cast past it; the unused FakeElement import is dropped. On the count — 25 is right after this round (ipc 5, panel model 6, hook 5, refresh 4, workbar 5); the body said 25 while the suites held 24, and the new re-activation test is the one that closes the gap rather than my arithmetic being retroactively correct.

[ARE404]

Rebased onto 1f690533; conflict resolved, and it changed one decision.

The conflict was in ui-render-memo-boundary-contract.test.ts: #2034 grew that file's fake DOM into counting ResizeObserver / IntersectionObserver stand-ins whose tallies its own assertions read. So it is no longer a general harness, and extracting it would couple every future renderer test to those counters.

I've reverted the extraction. That file is now byte-identical to main — it is not in this PR's diff at all — and fake-dom.ts stands on its own as a minimal harness for the hook test, with a comment saying why it is modelled on that one rather than shared with it. Less clever than what I claimed last round, but it stops this PR from touching a file someone else is actively developing, and the duplication is now between two things that genuinely differ.

Re-verified after the rebase: 25 desktop inspector tests, projection 18/18, format:check and check-dead-css clean. (ui-render-memo-boundary-contract itself can't run in my worktree — missing @stylexjs/stylex — but it is unmodified here, so CI covers it.)

[Astro-Han]

An independent look at the current head matches the review thread, with one boundary the deferral comment leaves half-explained.

readSessionTrace reads three legs. The per-run catch covers one; the comment states the other two are not covered, but only explains the header leg. The runtime-events leg fails the same way: readSessionRuntimeEvents (sqlite-runtime-store.ts:846) loops runs through readImmutableRuntimeEventsSync (:514), whose rows.map(decodeRuntimeEventStorageRow) has no per-row tolerance, and decodeRuntimeEventStorageRow (:3155) throws on payload decode or row/payload identity mismatch. One corrupt runtime_events row still rejects the whole trace on every retry, exactly the class this PR removed for event rows. The storage-change argument that justifies deferring the header leg applies here too, since that path is shared with recovery evidence reads, so a one-line addition naming the leg would close the comment, not the code. Not a blocker.

Two optional notes:

• recovered on InspectorStepRow and its rendering have no test pin. Same class of unpinned behavioral change this PR fixed for re-activation; one assertion in session-inspector-panel-model.test.ts covers it.
• fake-dom.ts copies roughly 140 lines verbatim from ui-render-memo-boundary-contract.test.ts. The comment says extracting would couple every future renderer test to the observer counters, but the overlap is the primitives (FakeElement, FakeText, createFakeDocument), not the counters. The revert decision itself still looks right while refactor(ui): stream the transcript incrementally instead of re-deriving it per token #2034 is in flight; the rationale just overstates the coupling. Unused members (matchMedia, setTimeout/clearTimeout, HTMLIFrameElement) and the useDelayedFlag note also don't match the "minimal on purpose" claim.

None of these block merge.