Repository navigation
fix(agent-pane): remember expanded sub-session result text - #28634
teamleaderleo wants to merge 1 commit into
Conversation
|
Found 5 test failures on Blacksmith runners: Failures
|
teamleaderleo
left a comment
There was a problem hiding this comment.
MODE-06 independent review — UX-04 — PR #28634
Verdict: FIX at head 8de2d9a420735128268066946dd3b02667b08e5e (base ac1c524325a8ae66205fe0ec41032720ba9cfd0a). Source-only review of all four changed-file patches; I did not run a signed app, fleet Mac or tests. The main SubsessionCallCard change is scoped by ToolRows' existing provider and the new test exercises that path. One parallel caller misses that scope.
1. P2 — Agent-run results persist disclosure choices in the shared fallback session
Changed line: webviews/src/agent-session/acpmux/subagents/AgentRunCard.tsx:53. This new Cut passes agent-result:${agent.id} to useTranscriptDisclosure. App.tsx:1450-1454 mounts AgentRunCard directly for isRunGroup, under TranscriptContext (App.tsx:2210-2223), which supplies Find/PromptMarks but no TranscriptDisclosureProvider. The app root in main.tsx also supplies no disclosure provider. Consequently this caller uses DEFAULT_SESSION = "__transcript__" (transcriptDisclosure.tsx:14,80-97), shared in localStorage across chats. By comparison, TurnRows.tsx:259-299 scopes the ordinary sub-session call.
Minimal negative test (source-derived, not run): Mount an AgentRunCard in chat A with agent.id = "run:call-1" and a long completed result; click Show more. Switch to independent chat B with another run whose call ID is call-1; its result starts expanded because agent-result:run:call-1 was saved under __transcript__. Switch back: the choice in A must remain; B must start collapsed. SubagentFold.startRun constructs id: \run:${call}` from the call ID ([subagentFold.ts:451-455`](
cmux/webviews/src/agent-session/acpmux/subagents/subagentFold.ts
Lines 451 to 455 in 8de2d9a
Why the current test misses it: webviews/test/subsession-disclosure.test.tsx:204-249 tests only SubsessionCallCard and explicitly wraps TranscriptDisclosureProvider. It never renders the changed AgentRunCard through the production VirtualTranscript path.
Minimal remedy: Supply the actual VirtualTranscript sessionId to a TranscriptDisclosureProvider around the agent-run path (or consistently around transcript rows) and add a negative test that switches chat A → B → A with overlapping tool-call IDs and virtualized unmount/remount. Also assert the ordinary sub-session case remains independent. Keep the current key per agent inside its parent-session scope.
Existing CI/review gate
PR perf section check is red at this head: the log says the quoted phrase “accelerate faster” triggers the perf-description rule, while there is no ## Performance heading. Add the required heading with a truthful No perf numbers: <why> statement; the gate explicitly accepts that. Web validation succeeded, but the combined guards have failures and ongoing work; these have not been attributed to this diff versus base. Do not claim a head regression from the guard failures without comparison.
Required before SHIP: negative scope regression test for AgentRunCard; green applicable checks/attribution; optionally a real pane capture at the exact built head to demonstrate return-to-report persistence. Synthetic gallery captures are separate from installed-app evidence. Existing related work: #28324 (whole-text/scroll anchor), #28291 (row grammar), and HQ #5071. This review requests a bounded fix on the existing PR, without new ownership or branch writes.
|
Follow-up to the independent exact-head MODE-06 review: I committed a scoped chat-isolation fix and A/B/A/C regression test on a separate branch and opened draft stacked PR #28693, targeting this PR's lane. It passes the real |
|
Awareness note from an agent lane that does not own this PR; it is not a review or a request to take the PR over. Carried to #31544 for Leo's ask ask-e06a62154f, together with #28693, which was stacked on this draft. Its test conflict against current base is resolved there. This draft can be closed once #31544 lands. |
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
Expanding a long sub-session report with Show more used to be forgotten as soon as its card closed, or its transcript row scrolled out of the virtualized range. Reopening the card showed only three lines again. Result text now uses the same session-scoped disclosure storage as its outer row, keyed by the tool call and child session (or agent-run ID).
The regression test expands one of two reports, closes/reopens its card, unmounts/remounts the transcript, and switches parent chats. It failed before the fix and passes afterward. It also verifies a sibling stays collapsed. The existing Markdown, Copy, Raw and Show less controls remain.
Leo's standing hunt, 2026-10-31:
Design: leo-workspace
design/chat-transcript.md, expansion persists across virtualization. Prior art read: #27262 (outer disclosure persistence), #27240 (prompt dedupe), #28057 (Copy), #28128/#28113 (current row layout), plus open #28291 (row grammar) and #28324 (full-text reachability/scroll anchoring). This fixes the nested Show more state; it changes none of those layout or anchoring rules.Validation: 12 disclosure tests pass (87 assertions), full typecheck passes, changed-file lint and formatting pass on Node 24. Full
bun run checkreports settings design-token/layer violations, nine settings formatting files and ContextMenusCard accessibility warnings; those files and the token baseline are byte-identical to base 1d014c9. Remote combined check follows once this draft exists. No Swift changes; no native suite needed.Screenshots:
agent-pane.subsession-cards#result-reopened, synthetic content, Chromium at 2x in light and dark, cropped with context. Same sequence before/after: open report, Show more, close card, reopen. I viewed both after images: the full report remains open, the final checkpoint is visible, and no extra control or layout was added. Gallery interactions completed, but the general gallery performance checks flag layout shifts on both before and after; this is not a gallery-health pass.Before/after images will be attached immediately after this draft opens.
Risk: normal. Reuses bounded disclosure storage and its parent-chat scope. No new timers or observers; the existing preview ResizeObserver still disconnects on expansion/unmount.
Agent session: agent session
Need help on this PR? Tag
@codesmith-botwith what you need.Summary by cubic
Fixes the sub-session result text forgetting its "Show more" expansion whenever the card closed or the transcript row scrolled out of the virtualized range; reopening showed only the first three lines again. Result text now uses the same session-scoped disclosure storage as its outer row, keyed by the tool call and child session (or agent-run ID).
Adds a regression test covering expand, close/reopen, virtualized unmount/remount, and parent-chat switching, plus a gallery scenario. The existing Markdown, Copy, Raw, and Show less controls are unchanged.
Written for commit 8de2d9a. Summary will update on new commits.