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.
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 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 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.
Extract model-command, questions, and resume sub-machines from the
~1600-line createTuiChat closure into src/chat/ factories that take
explicit dependency bundles (shared ChatChannelDeps/ChannelNotice).
Reorganize src/ so chat/ holds all chat-channel concerns (former input
and session/ files move under it); xml-tool-output moves to components/;
TuiRuntime/TuiResumeHost move to runtime.ts. index.ts drops 2067->~1530
lines. Behavior identical: 167 tests and all TUI snapshots pass unchanged.
The running status glyph clipped its cosine throb to a blank column below
STATUS_FADE_MIN_OPACITY, so each breath read as bold->dim->disappear rather
than bold->dim->bold. Remove the cutoff and the constant; the trough now
renders as the dimmest gray, giving a symmetric breathe. Turn-boundary
appear/disappear is unchanged (fade envelope and non-truecolor visible
threshold).
Two CI-only test failures on the personal TUI/skills stack:
- packages/ui/tui/tests/tui.spec.ts anchored the dark-palette test at
process.cwd(), which is not guaranteed under $HOME; on CI runners the
prompt rendered an absolute path instead of the `~/` abbreviation. Anchor
cwd under homedir() so the assertion is deterministic.
- examples/tui-agent/tests/tui-keyless-smoke.e2e.ts asserted stale
dsh-customize / dsh-upgrade skill descriptions. Sync the expectations to
the bundled SKILL.md frontmatter.
Consolidates the personal dsh-tui customizations (module split into
components/session/extension, prompt template + running-glyph indicator,
copyable transcript, tool-card headers, timing placement, XML tool output,
status/footer rework) and ports upstream's model reasoning-effort selector
(Shift+Tab effort cycling, effort-aware /model, footer, and /status) onto
the personal module layout.
agent/disposed only cleared the status line, so an agent-loop-only reload that
disposed the agent while the TUI stayed mounted left the local disposed flag
false. Since retained agents accept deliveries after detachment, later input
drove a zombie agent/session. Set disposed on agent/disposed so dispatchMessage
reports it. Adds a regression that sends after disposal and asserts no delivery.
The reference-admission discard listener matched on followup()'s returned id,
but an agent/inbox/enqueue listener that synchronously cancels emits
agent/inbox/discard before followup() returns to assign that id. The match
then missed, leaking both the submit and discard listeners plus the attached
context per referenced prompt. Match on the content reference instead — the
same value send() carries onto the message, known before followup() runs, and
symmetric with the submit wrapper's content check.
The prior "ordinary allowed path" test passed only because the fake agent
returned a fixed 'stub' id that collided with the id the test constructed;
it now releases each wrapper through its own allowed admission, and a new
regression drives the synchronous-discard timing directly.
