fix(dashboard): make transcript disclosure state survive row recycling

[hoang-phan98]

Problem

Expanding anything collapsible in the chat transcript while an agent is working
does not stick. A tool call, a thinking block, a workflow card: you open it, and
a moment later it closes on its own. Re-opening does not help, and it keeps
closing for as long as the turn runs. It looks intermittent, because whether it
happens depends on scroll position and streaming timing rather than on anything
the user did.

Why it matters

Reading tool calls and reasoning as they stream is the main way a user follows
what an agent is doing during a long turn, and it is how they catch a wrong
command early. A disclosure control that discards an explicit click is worse
than one that never opened: the user keeps re-clicking, loses their place in the
transcript each time, and reasonably concludes the control is broken. It lands
at the moment attention matters most, mid-turn while work is in flight.

Fix (symptoms → root cause → change)

This is a bug class, not one bug, and that is the important part of the
diagnosis.

The transcript is virtualised. useVirtualChat renders a row only while
item.mounted holds, and unmounts it once the row leaves the window plus
overscan band, which streaming does routinely as it scrolls content past. Every
collapsible control in the transcript kept its open/closed state in row-local
useState, so each recycle destroyed it. Nine controls had the defect:

ThinkingBlock, ToolCallLine, TurnBlock, CollapsibleToolGroup,
NudgeCard, RecoveryCard, WorkflowCompletionCard, FileChangeChips,
TaskProgressBar.

ToolCallLine already carried a comment naming "virtualizer recycling" as a
known hazard for a neighbouring concern, so the mechanism was known, just not
followed through to disclosure state.

A guard cannot fix this. A remount discards the guard along with the state it
protects, so the state has to outlive the row.

Change: one shared mechanism. useRowDisclosure(key, fallback) in
website/src/pages/chat/rowDisclosure.tsx, backed by an external store with
per-key subscriptions via useSyncExternalStore. Per-key subscription is a
deliberate choice over holding the map in provider state: the latter re-renders
every consumer on every toggle, and these are the rows whose render cost is the
most sensitive in the app. The hook degrades to plain local state when no
provider or key is present, so hosts that render transcript components outside
ChatPage (split-view ChatPane, app-sdk's ChatMessageList) keep working
untouched. An entry exists only once the user has made an explicit choice, so a
control that was never touched keeps its previous default behaviour. ChatPage
supplies the store and resets it on slot switch, since row keys are unique only
within a slot.

A second, independent defect in TurnBlock is also fixed. It collapsed on
any turn.complete false→true transition, and complete is derived from the
slot's running flag, which ChatPage re-reconciles from every slots
broadcast, accepting any running: false not shadowed by a locally pending
turn. A broadcast catching the slot momentarily idle between tool calls flipped
it true mid-turn and fired the collapse. An explicit click now pins that state.

Tests

21 cases across four files. The mechanism is proven once rather than
re-proven per component: durability across unmount/remount, per-key isolation,
reset on slot switch, and the no-provider fallback. Component-level cases then
cover the tool pill and the turn-level toggle, including running-flag churn.

Writing the mechanism tests caught a real bug in the hook itself: expanded = stored ?? local let a stale local true survive reset(), leaking a choice
across a slot switch. The store has to be the sole source of truth when
present. That case now guards it.

Six cases fail without the fix:

Case	Locks in
hook: choice across unmount	the store outlives the row
tool pill recycled	the pill's panel survives a recycle
turn row scrolls away and back	turn-level disclosure survives a recycle
recycle combined with flag churn	both mechanisms together
stale running:false frame	one spurious frame does not discard an expand
repeated flag oscillation	five true/false cycles do not either

Five more guard the opposite direction and pass in both states, covering
explicit collapse, an untouched control still auto-collapsing, per-key
isolation, and a new turn not disturbing an earlier one. Mutating either pin to
be too broad fails one of them, so the fix cannot be widened into disabling
intended behaviour unnoticed.

One it.fails case pins a separate pre-existing defect found while
diagnosing this: flushTurn only emits a turn object once the trailing group
exceeds items.length > 2, so crossing that threshold wraps the items in
TurnBlock and remounts every row beneath it. it.fails asserts the defect is
still present, so fixing the promotion turns the test red and forces the flip to
it rather than rotting silently. Follows existing in-repo precedent
(website/src/i18n/memoBailout.test.tsx).

Manual verification

Reported from real use, and reproduced by hand across three separate controls
(the turn-level toggle, the tool pill, the thinking block). Each iteration was
built and synced to a live gateway for hands-on confirmation.

Diagnosis was instrumented rather than assumed. Mount counters separated a
re-render (state preserved) from a remount (state destroyed), which is what
identified the virtualizer recycle as the dominant path and ruled out an earlier
suspect: a 60 Hz style-recalc loop from a never-resolving spinner, measured and
excluded because it produces zero DOM mutations and so never re-renders
React. That is a real but separate CPU-waste issue, not folded in here.

Worth recording, because it explains the shape of this change. The first pass
fixed only the flag-churn mechanism and asserted the remount was "no longer
urgent once disclosure is pinned". That was wrong: a remount takes the guard
with it. The second pass fixed the remount for the wrong control, the
turn-level summary rather than the tool pill the report was about. Only after a
third report did enumerating every expandable control in the transcript
reveal this as a class, at which point a shared mechanism replaced the two
bespoke patches. Fixing controls one at a time was the error; the class fix is
the correction.

Two controls are migrated to the hook but intentionally not keyed, and fall
back to local state exactly as before: TaskProgressBar renders outside the
virtualised list so it was never exposed, and KnowledgeBubbleChip has no
stable identity at its call site.

Also worth flagging for reviewers: the migration initially introduced three new
react-hooks/exhaustive-deps warnings, because setExpanded is now a memoized
callback rather than a stable useState setter. tsc was clean throughout and
would not have caught it, and the warning ratchet would have failed the build.
Fixed by adding it to the three dependency arrays.

Screenshots

Not applicable, and worth being explicit rather than silently dropping the
section. This changes temporal behaviour, not appearance: no pixel differs
in any single frame, because the bug is that a correctly-rendered open panel
closes a moment later. A still of either state looks identical before and after,
so it would be evidence of nothing. The behaviour over time is what the six
revert-verified cases capture, reproducing the exact frame sequences that
trigger it.

Follow-ups (not in this PR)

1. Converge the last two controls onto the hook. ToolCallLine and
   TurnBlock still use ChatPage-state props from the earlier bespoke passes.
   Behaviourally equivalent to the hook, but two mechanisms for one job is
   worth collapsing.
2. The running-flag oscillation itself. syncSlotRunningFromServer accepts
   a running: false that contradicts an in-flight turn. Whether it should
   reject that is a separate question with its own blast radius, and it is a
   plausible cause of other transient UI churn. This PR makes the disclosure
   controls robust either way.
3. The loose→turn promotion remount, pinned by the it.fails case. Row
   identity across that promotion is structural. Disclosure no longer pays for
   it, but other row-local state remains exposed.
4. The credits-pill spinner, which burns 60 Hz of style recalc whenever
   /api/sessions/usage returns null because both null and undefined fall
   into the same falsy branch. Measured, unrelated to this bug, unfixed.

[github-actions]

UX Review (Fable 5) — ✅ PASS

Advisory UX-level review of 1d07da1a86fe51f1214f73e85a7c17b3ad6a4d08 — updated in place on each push; does not block merge.

UX-Verdict: PASS

Disclosure now honors user intent through scrolling and mid-turn churn — the core frustration (a click the UI silently discards) is genuinely fixed.

Suggestions

• ToolCallLine's applyExpanded writes system-initiated opens (permission auto-expand, focusToolCallId reveal) into the durable map; if the approval resolves while the row is recycled, the remount never runs the auto-collapse effect, so pills the user never opened stay expanded for the rest of the slot — persist only explicit toggles, or clear the entry when the approval resolves.
• useEffect(() => { setTurnDisclosure({}); setToolDisclosure({}) }, [activeSlot]) wipes choices on every session switch; namespacing the maps by slot (${activeSlot}:${key}) would let a user flip between sessions without losing their expands, at no cost to the key-collision concern the comment cites.

[UX-REVIEWED] 1d07da1

[github-actions]

Opus 5 Review — ✅ no blocking findings

Reviewed 1d07da1a86fe51f1214f73e85a7c17b3ad6a4d08 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 1d07da1

Verdict parsed from the review's SHA-scoped output markers for commit 1d07da1a86fe51f1214f73e85a7c17b3ad6a4d08.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 1d07da1a86fe51f1214f73e85a7c17b3ad6a4d08: <one-sentence reason>

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 1d07da1a86fe51f1214f73e85a7c17b3ad6a4d08 and found no blocking issues.

This comment is updated in place on each push.

Review details

FINDING -- website/src/pages/chat/CollapsibleToolGroup.tsx:52 -- Expanding a tool group, scrolling it out of the virtualized window, then returning remounts it and "setExpanded(!!autoExpand)" overwrites the stored choice with false -> Fix: run this synchronization only when autoExpand !== undefined.
[GPT-REVIEWED] 1d07da1

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 1d07da1a86fe51f1214f73e85a7c17b3ad6a4d08: <one-sentence reason>

[github-actions]

Design Review (Fable 5) — 🟡 CONCERNS

Advisory design-level review of 1d07da1a86fe51f1214f73e85a7c17b3ad6a4d08 — updated in place on each push; does not block merge.

Design-Verdict: CONCERNS

Two parallel mechanisms now solve one problem — the new keyed store and ChatPage-owned maps — doubling the surface future disclosure work must navigate.

Watch

• The PR ships RowDisclosureStore/useRowDisclosure (126-line external store, adopted by seven components) and prop-drilled turnDisclosure/toolDisclosure useState maps in ChatPage for TurnBlock/ToolCallLine. The store's own docstring justifies it as avoiding host re-renders ("a toggle notifies only the ONE consumer"), yet the hottest controls — the tool pill and turn summary this PR is about — use the map path, so every pill toggle re-renders ChatPage and rebuilds renderMessage (it's in the dep array). The mechanism invented for render-sensitive rows was applied everywhere except the most render-sensitive rows. Consolidating onto the store (both already share the same slot-reset and unprovided-host fallback semantics) would delete the maps, the two reset effects, and the ref plumbing in ToolCallLine.
• The store migration of seven additional components (ThinkingBlock, NudgeCard, RecoveryCard, WorkflowCompletionCard, TaskProgressBar, FileChangeChips, CollapsibleToolGroup) is absent from the description, which only claims "ChatPage owns both disclosure maps". Same bug class, so defensible — but it's roughly half the diff and undescribed.
• CollapsibleToolGroup gets positional keys (ctg-g-${it.startIdx}, ctg-${vi.key}), contradicting the design's own premise of stable identity: loading older history shifts indices, so an expanded group's state can land on a different group. The message-keyed controls don't have this problem.

Suggestions

• Pick one mechanism (the store wins on the PR's own stated criteria) and route TurnBlock's pin/search-reveal through it, rather than leaving both patterns as precedent.

[DESIGN-REVIEWED] 1d07da1

[github-actions]

Arbiter — ✅ no blocking findings

Arbiter found no unresolved long-term items that require action before merging 1d07da1a86fe51f1214f73e85a7c17b3ad6a4d08.

Second-order review for 1d07da1a86fe51f1214f73e85a7c17b3ad6a4d08; this comment is updated in place on each push.

Review details

Both files read. All listed findings are frontend disclosure-state behavior in the virtualized chat transcript — no persisted data, no wire format, no API contract, no security surface. Judging each against the narrow bar:

• GPT 5.6 — CollapsibleToolGroup remount overwrites the stored choice (setExpanded(!!autoExpand) sync effect runs on remount with a fresh userToggled ref, writing false into the store). This is a real incompleteness in the fix — one component still loses the user's expand across recycling — but it is not a regression (pre-PR that state died on unmount anyway), and it is pure ephemeral UI state, fixable in a one-line follow-up (autoExpand !== undefined guard). No one-way door, no concrete harm.
• Design — two parallel mechanisms (store vs. ChatPage-owned maps): textbook architectural-duplication/maintainability concern, explicitly excluded from blocking. Both mechanisms work; consolidation is fully reversible later.
• Design — positional keys for CollapsibleToolGroup (ctg-g-${startIdx} shifts when older history loads): a wrong-group expand is a cosmetic misattribution of ephemeral UI state, self-correcting on next click, reversible with a key-scheme change. Not harm, not a door.
• Design — undescribed scope in PR description: process nit.
• UX — system-initiated opens persisted into the durable map: pills the user never opened can stay expanded for the slot's lifetime. Annoyance, ephemeral, reversible.
• UX — disclosure wiped on session switch: deliberate trade-off documented in the diff; namespacing is an enhancement.

Nothing here locks in a contract, schema, or persisted format (the store and maps are in-memory only, reset per slot), and nothing can crash, leak unboundedly (entries exist only for explicit toggles and are cleared on slot switch), or corrupt data.

Arbiter-Verdict: PASS

No sub-threshold finding meets the long-term-impact bar.

Suggested follow-ups (open as issues — non-blocking)

• CollapsibleToolGroup loses stored disclosure on remount (GPT 5.6, website/src/pages/chat/CollapsibleToolGroup.tsx:52): the autoExpand sync effect re-runs on every remount with a fresh userToggled ref and overwrites the store entry with false — the one migrated component where the PR's own fix doesn't hold. Safe to wait because it merely leaves this component at pre-PR behavior; fix by running the sync only when autoExpand !== undefined (or marking the store write as non-user-initiated).
• Positional disclosure keys for CollapsibleToolGroup (design reviewer, website/src/pages/ChatPage.tsx ctg-g-${it.startIdx} / ctg-${vi.key}): loading older history shifts indices so an expanded group's state can land on a different group. Ephemeral and self-correcting; fix by deriving the key from message identity (first message's key/ts) like the other controls.
• Consolidate the two disclosure mechanisms (design reviewer): the keyed RowDisclosureStore and the ChatPage-owned turnDisclosure/toolDisclosure maps solve the same problem; the hottest rows (TurnBlock, ToolCallLine) use the map path that re-renders ChatPage and rebuilds renderMessage on every toggle. Reversible refactor — route TurnBlock/ToolCallLine through the store and delete the maps, reset effects, and ref plumbing.
• System-initiated expands persisted as user choices (UX reviewer, website/src/pages/chat/ToolCallLine.tsx applyExpanded): permission auto-expand and focusToolCallId reveals write into the durable map, so a pill can stay open for the slot's lifetime if the approval resolves while the row is recycled. Fix by persisting only explicit toggles or clearing the entry on approval resolution.
• Disclosure wiped on session switch (UX reviewer, ChatPage.tsx reset effect): namespacing map keys by slot (${activeSlot}:${key}) would preserve expands across session flips with no key-collision risk. Pure enhancement.

[ARBITER-REVIEWED] 1d07da1

False positive or not applicable? A repository writer can comment:
/ai-review override arbiter 1d07da1a86fe51f1214f73e85a7c17b3ad6a4d08: <one-sentence reason>

For a broader accepted-risk deferral, apply defer-longterm and explain why.

[bolichen97]

Approved: independently re-reviewed the full diff against the GPT + Opus/AUTOSDE contracts (reachable correctness/security, fail-closed/permission boundaries, cross-OS, and deleted-in-use i18n keys per the #976 class). No reachable Critical/High found; all CI checks green.

[bolichen97]

Open PR relationship audit

This is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion.

Relationship findings

• PR #8289 is OVERLAPPING relative to this PR. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #8289: KEEP. Origin of the coupling, not a fix for it; the primary PR narrows the key without disturbing the durability property PR #1067 shipped. Files: website/src/pages/ChatPage.tsx.

No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit.