fix(chat): keep a subagent's generated files visible when its card collapses - #356
Merged
Conversation
The subagent branch of the dispatcher ended in a bare return before reaching ToolCallAttachments, so the files a `subagent` call itself produced rendered nowhere. #348 half-covered it by accident: once a group hoists its members' attachments, a subagent that happened to be grouped got its files back, while an ungrouped one still dropped them. Behaviour depended on whether grouping kicked in, which is not something the user can see or predict. Render them below the card under the same `hideAttachments` guard every other branch uses, so an enclosing activity group still hoists them instead. This is parity, not a new idea: upstream gates its own AttachmentGroup identically at `SubagentCall.tsx:577`, outside the dialog. Nested parts are untouched here — the following commit widens the hoist to cover them. No test: the change is composable declaration order, which this module has no Compose harness to assert.
An image generated inside a subagent sat behind TWO folds and vanished on reopen. The subagent trace card renders its nested parts inside its own AnimatedVisibility, and the card is itself groupable, so #348's hoist could not reach the images: a group collects the ids of its OWN parts, and a nested call's id is not among them. Widen the hoist set rather than narrow the suppression. outputToolCallIds now descends through `subagent_content`, so every id whose inline render is suppressed is already claimed by a hoist — the group's when the call is grouped, the dispatcher's own sibling slot when it is not. Only then is it safe to draw the nested parts with hideAttachments, which is why both halves land in one commit: the forward without the deeper walk is exactly the bug #348 fixed, one level down, and the walk without the forward renders the same file twice. The walk descends without a depth cap, unlike the render, which stops at 1. A depth-2 subagent falls back to the generic card and draws no nested parts at all, but its output still has to reach the surface. DELIBERATE DIVERGENCE from the web client. Upstream builds a group's attachment set from `group.parts` alone (ContentParts.tsx:360-367) and hands its subagent dialog no attachments, so a file generated inside a subagent is surfaced nowhere on web. Burying output the user cannot reach is the same class of defect as capping an image-gen render at the first attachment. The KDoc on outputToolCallIds says so, because a sync pass would otherwise read this as accidental drift and "correct" it back. Known limitation, widened but not introduced: provider tool-call ids like `call_0` repeat across agents in a handoff run, and Attachment carries no agentId to scope on (web guards with filterAttachmentsForPart). Nested ids now join that set, so one attachment can hoist under two groups in a handoff run. Fixing it needs the DTO field. Declaration order stays untestable — no Compose harness — so the sibling-after-the-collapsible contract remains KDoc and the id walk carries the tests. Reverting this commit returns the tree to upstream parity, and parity stays covered there by two tests that predate it and are deliberately left untouched: groupedToolCallIdsReturnsToolCallsInOrderAndSkipsReasoning and groupedToolCallIdsSkipsBlankIds, both asserting the top-level-only id set.
Contributor
Android debug APKArtifact:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #348. That fix hoisted a tool group's attachments out of the collapsible so generated images stopped disappearing, but it could not reach files produced inside a subagent: a group collects only the ids of its own parts, and a nested call's id is not among them. A subagent's trace card renders its nested parts in its own collapsible and is itself groupable, so those images sat behind two folds and vanished on reopen.
Changes
Two commits, deliberately separable:
parity — The subagent branch of
ToolCallDispatcherreturned before reaching the attachment render, so files thesubagentcall itself produced rendered nowhere when the call wasn't grouped, and were hoisted when it was. Behaviour depended on whether grouping happened to kick in, which the user can neither see nor predict. They now render below the card under the samehideAttachmentsguard every other branch uses, so an enclosing activity group still hoists them instead. Mirrors upstreamSubagentCall.tsx:577.the nested case —
outputToolCallIdswalks each call's own id and then the ids of the calls it ran nested inside it, so every id whose inline render is suppressed is already claimed by a hoist. Only then is it safe to draw the nested parts withhideAttachments. Both halves are one commit on purpose: the forward without the deeper walk reintroduces fix(chat): keep generated images visible when their tool block collapses #348's bug one level down, and the walk without the forward renders each file twice. The dispatcher's hoist and the card's render now resolve the run through one sharedsubagentTraceParts()helper — resolving it twice by hand is how the two would drift into hoisting one run's ids while rendering another's.Depth asymmetry, deliberate: the id walk is uncapped while the render stops at depth 1. A depth-2 subagent falls back to the generic card and draws no nested parts at all, but its output still has to reach the surface.
Testing
./gradlew test detektMetadataCommonMain detekt :app:lint :app:assembleDebug— green.ContentSegmentsTestcovers the id walk, including a depth-2 nesting case (4 new tests).groupedToolCallIdsReturnsToolCallsInOrderAndSkipsReasoningandgroupedToolCallIdsSkipsBlankIds.Notes
group.partsalone (ContentParts.tsx:365-369) and passes its subagent dialog no attachments, so a file generated inside a subagent is surfaced nowhere on web. We hoist it instead, on the same reasoning as fix(chat): keep generated images visible when their tool block collapses #348's choice to render every image rather than only the first: output the user cannot reach is the same class of defect as capping an image-gen render at the first attachment. The KDoc onoutputToolCallIdsrecords the prohibition and the upstream anchor, so a future/sync-upstreampass does not read this as accidental drift and "correct" it back.call_0repeat across agents in a handoff run, andAttachmentcarries noagentIdto scope on (web guards withfilterAttachmentsForPart). Nested ids now join that set, so one attachment can hoist under two groups in a handoff run. Fixing it needs the DTO field; worth its own issue.