fix(chat): give each same-tick tool row its own disclosure identity

[NicholasRBowers]

Problem / Motivation

Two or more tool rows appended in the same server tick share a single disclosure entry: expanding one tool pill expands every same-tick sibling, and collapsing one collapses them all.

Root cause (verified on main): messageRowKey (website/src/pages/ChatPage.tsx) returns ${role}-${clientTs ?? ts}, and tool rows are never clientTs-stamped — so a coarse OS clock maps a burst of tool rows onto one key. The render site passed that same string as BOTH the React key and the toolDisclosure map key, and the disclosure-map role is the load-bearing one (each renderMessage element is the sole child of a separately keyed wrapper, so sibling uniqueness was never at stake).

Split out of #4394 as the measured residue after #7207 closed the primary site.

Why it matters

Any turn where the agent fires several fast tool calls (the common case: parallel reads, batched checks) lands multiple tool rows on one tick. A user trying to inspect ONE tool's output gets every sibling's panel flung open at once, and cannot collapse one without collapsing all — the disclosure control is effectively broken for exactly the busy turns where it is most needed.

What changed (motivation → approach → change)

Symptom → root cause: same-tick tool rows collide on messageRowKey, and the disclosure map was keyed by it.

Approach: disambiguate the disclosure identity only, leaving messageRowKey and the React key untouched. Two constraints picked this seam:

• The key-stability contract (chatSlice.streamingKeyStability.test.ts) pins messageRowKey(tool) to exactly tool-<ts> — folding the id into messageRowKey itself would break that adjudicated contract.
• The React-key role is not at stake, and re-keying it would remount rows (dropping measured heights) for no benefit.

Change: a new exported toolDisclosureKey(m, key) appends meta.tool_call_id (ACP-issued, globally unique) to the row key for role === 'tool' rows that carry one, and returns the key unchanged for every other role and for id-less tool rows. The ToolCallLine render site now reads and writes the toolDisclosure map under that key. The sibling SDK render path (app-sdk/ChatMessageList.tsx) needs no change — its msgKey is index-bearing and never collides.

Residual scope, deliberately not covered: tool rows persisted with no tool_call_id (the backend's _tool_meta() returns None when the ACP event carries no id) keep the pre-existing colliding behavior. Both alternatives fail against evidence: meta.mid never appears on tool rows (measured in #8204 with a control mutation), and an index-based fallback would break disclosure identity across a history prepend — a worse defect than the one it patches. This PR closes the defect for all id-carrying rows, which is every live-path tool row.

Tests

• website/src/test/toolDisclosureKey.collision.test.ts — adapts Same-tick tool rows share one disclosure key (messageRowKey) #8204's reducer-driven probe (credit @chenmingwei23): 3 same-tick tool rows with distinct tool_call_ids through the real refreshSlot reducer yield 3 distinct disclosure keys, with per-item pairing assertions (not set-size). Plus: id-less tool rows and non-tool roles keep their exact existing key shape, and messageRowKey's own contract is untouched.
• website/src/test/toolDisclosureKey.renderSite.test.tsx — pins the render-site wiring through the real ChatPage: two same-tick tool pills, click one, only that one expands (and the mirror: collapsing one leaves the other open). Verified red against a revert of the wiring (dKey → key): the test fails with both pills expanded, so the exact line that regressed is covered.
• Full frontend suite green: 1805 files / 28463 tests. npx tsc -b clean. Backend isort/flake8/mypy clean (no backend files changed).

Manual verification

N/A — the render-site test drives the real ChatPage render path end to end (real reducer, real ToolCallLine, real disclosure state), which is the exact user interaction.

Screenshots / video

Why no screenshot: no at-rest pixel change — the delta is interaction-state only (which pill's panel opens on click), pinned by the committed render-site test; same evidence basis as the merged sibling fix #7207.

Related Issues

Closes #8204

Pattern harvest

Rule candidate: review-prompt
Pattern: a string passed as both a React key and a state-map key couples two identities with different uniqueness requirements — check that a colliding render key cannot alias user-visible state.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable)
• No secrets, credentials, or internal references in the diff

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of f05f0c234d7d8830ea518c8d32cb939fc4392316 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Right seam: disambiguates the load-bearing disclosure identity without disturbing the pinned row-key contract or remounting rows; residue and alternatives are evidenced, not asserted.

Verified against the code: the wrapper keys at ChatPage.tsx:7069/8308 go through msgIdentityKey, so the React-key role is genuinely not at stake; the SDK path's msgKey (ChatMessageList.tsx:70) is index-bearing as claimed; the id-less fallback preserves the exact prior key shape, so nothing regresses. The render-site test pinning the dKey wiring (verified red against the revert) covers the one real coupling hazard — a future edit passing key to one of the two disclosure props and dKey to the other.

[DESIGN-REVIEWED] f05f0c2

[github-actions]

UX Review (Fable 5) — ✅ PASS

UX-level review of f05f0c234d7d8830ea518c8d32cb939fc4392316 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

The diff is small and self-contained — one function plus a call-site change in ChatPage.tsx and two test files. No user-facing strings, no screenshots, no layout or flow changes. The change fixes a real UX defect: tool rows appended in the same server tick shared one disclosure-map key, so expanding one tool call expanded all of them; each row now gets its own identity via tool_call_id, with a graceful fallback to the old shared key when the id is absent. The React key is deliberately unchanged, so no remount/scroll-height side effects are introduced. Nothing here trips any UX lens.

UX-Verdict: PASS

Pure interaction bug fix — expanding one tool row no longer expands its same-tick siblings; no strings, layout, or flows touched.

[UX-REVIEWED] f05f0c2

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

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

Review details

No findings.

[OPUS-REVIEWED] f05f0c2

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

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

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of f05f0c234d7d8830ea518c8d32cb939fc4392316 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All claims verified. I have what I need for the verdict: the fix is cause-level for its scope, the pinned messageRowKey contract genuinely bars the deeper fix, the SDK sibling path is confirmed non-colliding, and the one real depth finding is that four other card sites in ChatPage pass the same ts-keyed row key as their disclosure identity.

First-Principles-Verdict: CONCERNS

The tool-pill fix is cause-level and complete for its scope, but the key-doubles-as-disclosure-identity cause has four unfixed sibling sites in the same file.

What this change ships

Intent: make expanding one tool pill stop expanding every tool pill appended in the same server tick — a FIX.

1. Clicking one same-tick tool pill toggles only that pill — justified (reported defect Same-tick tool rows share one disclosure key (messageRowKey) #8204)
2. New exported toolDisclosureKey helper — declared; one live consumer (ChatPage.tsx:6868), test-motivated export
3. Id-less tool rows keep the old colliding behavior — declared residual, alternatives measured and rejected
4. Two test files pinning the helper and the render-site wiring — declared

Watch

• Point patch on a counted pattern: grepping disclosureKey={key} in ChatPage.tsx finds 5 sites passing the bare ts-keyed row key as disclosure identity; 4 remain unfixed after this change — NudgeCard (6885), RecoveryCard (6901), WorkflowCompletionCard (6919), SubagentCompletionCard (6923). (ThinkingBlock at 6846 is exempt: thinking rows are clientTs-stamped at birth.) The realistic collider is SubagentCompletionCard — parallel subagents are first-class, and two completions stamped in one coarse tick would alias each other's expanded state exactly as the tool pills did. Those rows carry no tool_call_id, so this PR's fix does not transplant; the general fix (a stable per-row identity) is genuinely larger — accepted-and-deferred, but the description's residual-scope note names only id-less tool rows, not these siblings.

The deeper alternative — stamping tool rows so messageRowKey never collides — is barred by the adjudicated contract at chatSlice.streamingKeyStability.test.ts:311, so decoupling disclosure identity from the row key is the correct level. uniqueRowKeys' ~# tie-break is not a duplicate mechanism: it is positional among colliders, which is the prepend-unstable identity this PR correctly rejects.

[FIRST-PRINCIPLES-REVIEWED] f05f0c2

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

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

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] f05f0c2

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

[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

• This PR is OVERLAPPING with PR #1067. 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.
• This PR is OVERLAPPING with PR #7207. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #8289: KEEP. The merged sibling fixed a different failure mode of the same root cause and left this one open; the primary PR is the measured residue, exactly as its description states. Files: website/src/pages/ChatPage.tsx.
• This PR is OVERLAPPING with PR #7255. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #8289: KEEP. Complementary, not competing: PR #7255 changes no behavior and does not fix Issue #8204. Keep both; sequence them and rebase the loser of the race. Files: website/src/pages/ChatPage.tsx.

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

[chenmingwei23]

Tier 1 auto-approve: fix (3 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: fix -- gives each same-tick tool row its own disclosure identity so expanding one no longer expands all (#8204).

[dwu96]

Tier 1 auto-approve: fix (3 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: clear root cause (#8204) — tool rows are never clientTs-stamped, so a burst appended in one server tick all shared the messageRowKey 'tool-', and ChatPage reused that string as the toolDisclosure map key, cross-attributing one row's expand/collapse to every same-tick sibling; the new toolDisclosureKey folds meta.tool_call_id into the disclosure identity only, leaving the React key and every other role's key shape untouched. Two frontend regression specs added.