jerry/buzz

mirror of https://github.com/block/buzz.git synced 2026-10-29 21:41:33 +00:00

Files

ss-dev-02 ac794843a6 feat(desktop): fold focus-mode agent work into one transcript block

Focus mode gave every thought and tool step its own row and its own
disclosure, so a turn that did real work scrolled its answer off screen
behind a stack of chrome. This adopts berd's shipping transcript model:
within a turn, everything between the prompt and the answer that is
thinking, a tool step, or an interim agent note becomes one "work block"
— a thin rail of small glyph bullets that folds to a single "N steps"
line when the work is done.

Grouping is variant-aware AT THE LIST BOUNDARY rather than inside the
grouping module. `TranscriptDisplayBlockView` runs the additive
work-block transform only for `conversation`, so `default` and
`compactPreview` walk main's segments on the identical code path they
always did. That makes Slice B's byte-for-byte baseline fixture hold BY
CONSTRUCTION rather than by assertion, which is the property worth having
here: no future edit to the block can regress the other two variants
without first moving this branch.

Behaviour, mirroring berd:

- Live: the block is open and the rail IS the status, so there is no
  header line to restate it. The last three steps show in true arrival
  order; older ones go behind an "N previous steps" disclosure at the top
  of the rail, so a long run cannot push the answer out of view.
- Finished: folds to "N steps" with a chevron that rotates on open.
- The fold ANIMATES. `<details>` cannot do this — its content is either
  laid out or not, with no intermediate height — so the collapse would
  snap. A block that was live when it mounted renders open for a paint
  and then settles closed, giving the height tween a start state that was
  actually painted; a block already finished on mount (scrollback) never
  had a rail on screen and closes immediately, so the animation stays
  meaningful instead of firing on every mount.
- Reader choice wins: once the reader toggles a block, policy stops
  opening and closing it.

One deliberate departure from berd, per core-02: a finished block holding
a failure folds to "N steps · 1 failed". A bare count is the one thing a
reader cannot distinguish a clean run from a broken one by, and a fold
that hides a failure behind a neutral number invites them not to open it.
The rail bullet itself stays muted, per berd — failure is a glyph shape,
not a colour, so one bad step does not read as an alarm across the run.

Items are projected ONCE into rail entries, and the glyph, the body and
the folded line's counts all read that projection. Two independent
classifications of the same item is precisely how the headline and the
chain eligibility drifted apart on the abandoned tool-chain card, so both
render sites are exhaustive switches over the entry and neither asks
`item.type` again.

The projection is closed in the TYPE SYSTEM, not only by convention. An
earlier revision made the entry a product of independent fields
(`{ item, kind, state }`) with a catch-all `return "tool"`, which left two
things wrong that no test could see: a future `TranscriptItem` variant
would silently wear a wrench, and impossible pairs like
`{ kind: "note", item: <thought> }` stayed representable — so the body
switch still had to re-check `item.type` and render `""` on a mismatch it
could not otherwise handle. Meaning was therefore still derived in two
places.

Now:

- `WorkBlockItem` is the closed union of items a block admits, and
  `isWorkItem` is a type guard, so the membership decision is made once
  and every later stage receives the narrowed type. `admittedWorkItems`
  returns the narrowed array rather than a boolean because an `.every()`
  guard cannot narrow the array it tested.
- `WorkBlockEntry` is a discriminated union pairing each kind with its own
  item type, and fixing `state: "settled"` on the prose kinds. Both classes
  of impossible entry are now unrepresentable rather than defended against.
- `projectWorkBlockEntry` switches exhaustively over `WorkBlockItem` with
  no default, so admitting a new item type without deciding how it renders
  is a compile error (`TS2366: Function lacks ending return statement`),
  not a wrench.
- The body switch takes the whole entry, so narrowing on `kind` narrows
  `item` too. The `item.type === "thought" ? item.text : ""` fallbacks are
  gone because there is no longer a mismatch to fall back from.

Verified by compiling three mutants, each of which now fails `tsc` where
before it type-checked: admitting `plan` to `WorkBlockItem` without a
projection case (TS2366), projecting a thought as a note (TS2322 on
`item`), and giving a thought `state: "failed"` (TS2322 on `state`). The
runtime kind/item pairing is also asserted in
`agentSessionWorkBlockGrouping.test.mjs`, because types are stripped at
runtime and swapping the two prose branches by hand is the easy mistake —
that mutant fails the test.

The projection also fixes an ordering bug the old split invited: a tool
carrying a stale `isError` from a retry while the new attempt executes
reads as `running`, not `failed`, so a live block cannot fold its own
count to "N steps · 1 failed" while the work is still in flight.

**Interim notes suppress the identity row.** #6720 gives every
conversation-variant assistant message a 20px avatar + name row, which is
right for the turn's answer. A rail note is the same item type, so
routing it through that presenter would render a fully attributed agent
turn nested inside a muted step row — the agent apparently replying twice,
once inside the work it was doing. Notes render through a dedicated rail
prose body instead, keeping markdown and the focus code-block recipe by
providing the same `CodeBlockVariantContext` value the presenter would.
Done on this side rather than by reaching into #6720, so that PR keeps one
rule for what a message looks like. Notes share the thought's speech
bubble, matching berd's `progress` entry: both are the agent talking.

- `useControlledDisclosure` is deleted rather than reused. Its entire
  reason to exist was the `<details>` echo trap — `<details>` fires
  `toggle` for programmatic `open` changes indistinguishably from clicks,
  so a policy-driven open echoes back looking like reader intent. This
  block's trigger is a `<button>`, where the only thing that can call the
  handler is a real click. Keeping the guard would have been dead code
  masquerading as load-bearing. Nothing else in the tree imported it.
- berd brightens rail prose with `usePrimaryText={open}`; here the
  brightening is unconditional. A closed block unmounts its rows rather
  than dimming them, so there is no state in which rail prose is on
  screen and not in an open block — the flag's false branch would be
  unreachable. A test records that reasoning so the divergence is not
  mistaken for an oversight.

dev-01 flagged that `turnSegmentItems` does not know `work-block`, so a
thought followed only by block content would never settle its duration.
Tracing it: the meta is built from pre-transform display blocks, so no
`work-block` segment can reach that function — the reported bug cannot
fire. But the trace turned up something worse. `ConversationThought` was
the only reader of `thoughtDurationSecondsById` and
`formatThoughtDisclosureLabel`, and this commit deletes it, leaving ~100
lines of production code and ~200 lines of tests that only tested each
other. Fixing a bug in code with no reader would have preserved the
illusion that focus mode still shows "Thought for Ns" somewhere.

So the duration map, its label formatter and `elapsedSeconds` are gone.
`AgentSessionTranscriptTurnMeta` narrows to the one field that still has
a consumer: `streamingItemId`, which the work block needs because a
thought or note streaming in carries no status of its own. Its tests are
rewritten around what that hint must actually get right — the tail of a
live turn, skipping setup, expanding a summary segment to its last leaf,
and reporting nothing at all when the turn is idle.

Three further notes on translation rather than transcription:

- berd's bullet masks the spine with `bg-card` and its BOT-1599 note
  warns off `bg-background`. The rule is "mask with the surface the
  transcript is drawn on"; in Buzz that surface is the cover drawer,
  which is literally `bg-background`. The two tokens are NOT
  interchangeable here: `[data-buzz-content-surface]` locally overrides
  `--background` to `--buzz-content-dark` while `--card` keeps the theme
  value, so in Buzz Dark `bg-card` paints the bullet rgb(36,41,46) over an
  rgb(26,26,26) drawer — the exact BOT-1599 failure. Light mode matches
  under either class, so light-mode evidence alone would not have caught
  it. Copying the class would have followed the letter of berd's note
  against its point.
- Reduced motion is read via `matchMedia` — the way `TerminalSubstrate`
  reads it — not motion's `useReducedMotion`, which resolves the query
  once per process and caches it. That cache made the preference
  untestable (the assertion turned on module load order, not on the
  setting) and ignored a mid-session change.
- The memo takes the entry SPREAD into props, not the entry object, and
  its boundary is the step BODY rather than the whole row. The projection
  is rebuilt whenever the item array changes, so entry objects are fresh
  on every append and a memo keyed on one would never hit; spread, the
  compared props are `item` (reference-stable) plus two strings. Spreading
  also keeps the union intact, so the body switch still narrows `item`
  from `kind`. The row stays outside because the glyph depends on
  `isLast`, which changes for the previous last row on every append.

`ConversationThought` is deleted here rather than in dev-01's restyle,
per core-02's sequencing ruling: this is the commit that replaces it, so
reasoning stays visible in focus mode at every commit. Its four
disclosure-keyed tests are deleted rather than adapted — they assert a
`<details>` that no longer exists on that path. `default`/
`compactPreview` thought rendering is untouched.

Non-vacuous by mutation testing, each mutant run in isolation:

- note falls through to the tool kind: 4 failures.
- note routed through the message presenter: 2. Note given the wrench: 1.
- `entryState` checking failure before running: 1.
- rail bullet tinted on failure: 1. Prose muted instead of primary: 1.
- memo keyed on the projected entry object: 2 — this is the bug the
  projection actually introduced, caught by the pre-existing streaming
  cost tests before it shipped.
- streaming hint forced to null: 4. Summary segments not expanded when
  finding the tail: 1.
- windowing keyed off `open` instead of the reader's choice: 3. Policy
  already holds live blocks open, so this switches windowing off in
  exactly the case it exists for.
- reduced-motion preference forced false: 1.
- rail bubble suppression removed (the bug above): the relay-step test
  stops passing.

An earlier mutant also caught a test lying: the block-level "echo" test
passed with the guard removed, because the trigger is a `<button>` and
nothing was listening for `toggle` at all. It now asserts the structural
reason the trap cannot apply plus a repeated fold→reopen→fold cycle,
which is what a recorded echo would actually have disabled.

Relay posts are plain rail steps. A `buzz messages send` step classifies
as `renderClass: "message"`, so it renders through `CompactMessageSummary`
— 28px avatar, bordered speech bubble, timestamp, delivery-receipt
button. Correct in the activity feed, where a posted message is a
destination to open; on a muted rail it makes the agent appear to reply
in the middle of its own work. This is the same failure the interim-note
case avoids, reached by a different route: there the item IS an assistant
message, here it is a tool call that merely classifies as one. Caught by
looking at the seeded browser preview, not by a unit test — the JSDOM
suite was green while the rendered rail was wrong.

Suppressed with an explicit presentation signal
(`useIsInsideWorkBlockRail`, default false) rather than by reading the
transcript variant, because `conversation` alone is not the condition:
the same relay step rendered OUTSIDE a block in that variant should keep
its bubble. Both halves of that branch are now pinned by tests, so
suppressing the bubble everywhere cannot pass. Defaulting to false keeps
`default`/`compactPreview` markup byte-identical.

Rebased onto #6720 at `e8709554a` per core-02's B → #6720 → C ordering.

Verified at this tree: full desktop suite 5472 passing / 0 failing,
`tsc --noEmit` clean, `pnpm check` findings identical to main's baseline
(2 warnings + 2 infos, all pre-existing), px-text/pubkey-truncation/
file-size gates clean.

Co-authored-by: Bradley Axen <baxen@squareup.com>
Signed-off-by: Bradley Axen <baxen@squareup.com>

2026-08-24 18:12:37 -07:00