Two comment-only corrections from the final review round.
The `isCompactCheckpoint` redundancy note appealed to the mandatory-marker
invariant to explain something the enclosing branch now states outright:
after e029ffb88 both call sites are literally inside an
`isReplacementSurfaceEvent` branch, so the appeal became retained
reasoning. Say the call sites test it directly.
`appendPreCompactionLog` implied its tool-result content is what survives
compaction, but `bash`'s presenter is static, so the string never reaches
a fixture — the fixtures pin that the shadowed step's card survives.
Neutral text plus a note on where the card body comes from, so a later
reader does not "fix" a fixture to make the sentence true. Verified by the
absence of fixture drift from changing the string.
The replay loop spelled the classification as
`isSurfaceEligibleType(event.type) && !isAppendSurfaceEvent(event)` while
the live listener used `isReplacementSurfaceEvent(event)`. Under the
mandatory-marker invariant these select the same set: the only divergence
is a surface-eligible event carrying no marker, which no active Session
can hold — `Session.append` and the seed construction loop both reject it
through the same `planSurfaceEvent`.
Using the same predicate on both paths makes "both paths classify a
surface event by the same marker" structural rather than argued, and
retires the TUI's last `isSurfaceEligibleType` use.
No behavior change: the TUI suite and all 113 snapshot fixtures pass
unchanged.
Review follow-ups on b9b2e593f, all documentation precision.
The redundancy note on `isCompactCheckpoint` leaned on a reading its call
site does not state: `index.ts` reaches it for surface-eligible non-append
events, which is the same set as replacements only because the marker is
mandatory. Say that instead.
The Agent Note now owns three facts it was leaving to a future reader.
`rebuildTranscript` materializes a component per append-origin event and
runs on mount, color-scheme change, and every reasoning toggle — work
compaction used to bound for exactly the long sessions it serves, so the
cost now tracks session length rather than the surface. `Consequences`
names `surface-replayed-compaction` as the durable evidence for the
live/replay equivalence claim, so the two fixtures that must move together
are findable from the Note rather than the PR thread. `Deferred` records
that a page can now carry a checkpoint whose `surfaceOp.start` fell out of
the window: pagination no longer cuts on the checkpoint's provenance
group, `FoldAdapter` pads with a non-surface sentinel, and `nodes()`
degrades to `degradedSeqs()` — which is already close to the transcript
projection A2 should build deliberately.
Also corrects the definite-assignment comment in the snapshot scenario:
the assertion rests on the awaited setup invoking `beforeMount`, not on
that call being synchronous.
Review follow-ups on the append-origin transcript projection.
The live/replay equivalence claim was stated unconditionally but does not
cover `tool/call`: only replay re-derives call pairing, because a call
event carries no `surfaceOp` of its own and inherits transcript
membership from the `assistant/message` that advertised it — which the
live listener has necessarily just rendered. Narrow the claim in the TUI
README and Agent Note, and record at `rebuildTranscript` why the filter
is replay-only rather than a missing live branch.
Add `surface-replayed-compaction`: the three existing fixtures all come
from the live path, leaving the resume case the bug report leads with
pinned only by a unit test. The new checkpoint mounts with the
replacement already stored and records byte-identical to
`surface-after-compaction-wide`, so the two fixtures now pin the
equivalence they assert. The shared fixture appends move into
`appendPreCompactionLog` / `appendCompactionCheckpoint`.
`MESSAGE_TYPES` is not "human message event types" — it includes
`assistant/message`. Say what the code distinguishes (append-origin
conversation messages vs. model-only replacement copies) at the const,
the `paginate` and `session.history` JSDoc, the apiproxy README, and the
Agent Note.
Also: spell the replace shape as `Extract<SurfaceOp, { op: 'replace' }>`
for symmetry with the module's two other uses; document why
`isCompactCheckpoint` keeps a replacement check that is redundant at both
call sites; say that Ctrl+R toggles reasoning, which rebuilds the
transcript; and qualify "the sole source of derived history" as derived
*model* history now that the transcript is the other projection.
The native adapter's route was named deepseek, colliding with pi-ai's
catalog provider of the same name, so the two DeepSeek paths could never
be mounted side by side. The web settings page needs both configurable at
once. Compositions, fixtures, goldens, scaffolding defaults, and docs all
move together (pre-release, no shim); TUI/session-query-spill/
missing-credential goldens re-recorded through their keyless refresh
modes because provider-name length shifts box padding and spill
truncation points.
The terminal and history pagination both treated the model-visible surface as
the human transcript. A landed compaction replacement therefore erased the
conversation it summarized — messages the reader had already seen — and a
model-only replacement copy consumed a page's `maxMessages` quota, which could
also split a compaction's provenance from the replacement citing it.
`dsh-session` now exports the marker split `isAppendSurfaceEvent` /
`isReplacementSurfaceEvent`. The terminal replays append-origin surface events,
keeps a shadowed step's tool cards paired through its append-origin assistant
message, and renders one dim marker where a compaction landed; the checkpoint is
recognized through the compaction seam's `isCompactCheckpointSource` contract,
not the shape of the replacement. `session.history` counts only append-origin
human messages. Everything model-facing keeps reading `session.surface`.
Lint: eslint --fix over the merge-crossed projection/command files (arrow
parens, trailing commas, unnecessary assertions), Extract<> replaces the
keyof-map & string intersections no-redundant-type-constituents rejects,
the fold-adapter's merge loop drops its non-null assertions for a
bounds-carrying cursor, one JSDoc line wrapped under max-len (api-catalog
regenerated). Snapshots: the four TUI goldens re-recorded for the merged
event-count shift (the durable command lifecycle adds one event to the
seeded diagnostics log). The headless advanced-toolchain snapshot passes
on CI and fails locally in this sandbox both with and without these
changes (30s child timeout — environment-bound, tracked in the ledger).
CommandService.execute now returns a CommandExecution — the normalized
result plus the commandId minted for its command/run/command/done records —
and the wire admission value carries commandId exactly when matched, so the
issuing client can correlate its RPC acknowledgment with the flow node the
lifecycle events produce. apiproxy api/schema/handler, the connection
fixture, and the TUI/plan/goal consumers follow the new shape.
CommandService.execute appends the log-only pair around every resolved
handler — run before invocation, done at settlement, including thrown and
aborted handlers (kind:'error'); admission misses log nothing. commandId is
minted monotonically per instance; per-session appends serialize through a
tail queue over SessionStore.appendOutOfBand (zero-step wrap on an idle log,
direct join inside an open turn). The invariant companion now asserts the
pairing relation (unique run ids; a done requires a prior in-log run).
CommandSource is a minimal merge-extensible map (user variant only).
Dependent benches mount SessionStore; TUI/e2e snapshots re-recorded for the
executor's durable-append timing and the /status event counts.
Hypatia May