Preserve agentId anchors in parallel-Task stitch + tool-param UI fix#115
Preserve agentId anchors in parallel-Task stitch + tool-param UI fix#115
Conversation
Parallel `Task` tool_uses emit a chain of assistant tool_use entries whose sibling tool_result anchors each reference a different subagent via agentId. Variant 2 of `_stitch_tool_results` classified the intermediate assistant subtrees as "dead-end" and let the walker mark their descendants — including agentId-bearing tool_results — as skipped, so only the outermost subagent session had an attachment point the traversal could find. Fix: add `_collect_agent_anchors` to walk an assistant subtree for UserTranscriptEntry descendants with agentId, and surface those anchors into the stitch result alongside the assistant dead-ends. The anchors still end up in `skipped` via `_collect_descendants` (since they're descendants of the assistant), but now they also live in the DAG-line's chain — which is what `build_session_tree` reads when choosing attachment points for subagent sessions. Regression tests cover the 2-teammate case, 3-teammate case (the experiments/worktrees shape), and an end-to-end splice verifying a subagent entry is actually reached through the anchor. Fixes issue #91 symptom: only 2 of 6 subagents rendering in a teammates session. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The generic tool renderer (render_params_table) wraps long parameter values in <details class='tool-param-collapsible'>. The CSS hid the entire <summary> on open, which also removed the browser's default disclosure arrow — leaving no visible way to collapse the expanded value. Wrap the preview text in <span class='tool-param-preview'> so the [open] rule can hide just the preview, not the whole summary. The summary (with its list-item marker / arrow) stays visible, and a "collapse" hint via ::after makes the click target obvious. Covers both call sites in tool_formatters.py: structured (JSON) and simple (string) long values. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThis change preserves user entries that carry Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
claude_code_log/html/templates/components/message_styles.css (1)
734-745: Minor: "collapse" hint is hardcoded in CSScontentand not localizable.The
summary::after { content: "collapse" }literal bypasses any i18n pipeline — unlike text emitted via the Python templates, CSScontentstrings can't be translated at render time. If localization is on the roadmap, consider sourcing this label from a CSS custom property set per-locale (e.g.,content: var(--label-collapse, "collapse")) or emitting the hint as a real element in the template instead. For a single-language dev tool this is fine as-is.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@claude_code_log/html/templates/components/message_styles.css` around lines 734 - 745, The CSS uses a hardcoded string in .tool-param-collapsible[open] > summary::after (content: "collapse") which prevents localization; change it to use a CSS custom property (e.g., content: var(--label-collapse, "collapse")) or move the hint into the template as a real element so the text can be produced by the i18n pipeline; update the stylesheet rule for summary::after and ensure templates set --label-collapse per locale (or render an element inside summary that gets translated) so the hint becomes localizable.claude_code_log/dag.py (1)
389-400: Variant 2 anchor preservation looks correct.
extracted_anchorsare appended todead_endsand timestamp-sorted alongsideuser_dead + assistant_children, so the relative emission order is deterministic and the continuation (user_with_cont) stays last. Because the downstream_walk_session_with_forksloop (Lines 491-495) runs_collect_descendantson everystitched[:-1]entry, anchors also get recorded inskipped, which keeps the coverage check happy — no risk of the fallback timestamp-sort kicking in for these sessions.One small, non-blocking note:
_collect_agent_anchorsis invoked once peracinassistant_children. If two assistant siblings ever shared a descendant (unlikely in a proper tree, but possible after orphan promotion), the same anchor UUID could be appended twice. If you want to be defensive, aset()dedupe before sort would be cheap:Optional dedupe
extracted_anchors: list[str] = [] for ac in assistant_children: extracted_anchors.extend(_collect_agent_anchors(ac, session_uuids, nodes)) # Stitch: dead-end children first, then the continuing user child - dead_ends = user_dead + assistant_children + extracted_anchors + dead_ends = list(dict.fromkeys(user_dead + assistant_children + extracted_anchors)) dead_ends.sort(key=lambda c: nodes[c].timestamp)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@claude_code_log/dag.py` around lines 389 - 400, The code collects anchors into extracted_anchors by calling _collect_agent_anchors for each assistant child, which can produce duplicate UUIDs if two assistant siblings share a descendant; to fix, dedupe extracted_anchors (e.g., convert to a set and back to a list) before appending to dead_ends so each anchor UUID appears only once, then sort dead_ends by nodes[c].timestamp as before to preserve deterministic emission and the final user_with_cont ordering.test/test_dag.py (1)
1166-1301: Regression coverage for the anchor-preservation fix is well targeted.The three scenarios (2-teammate, 3-teammate, end-to-end splice) directly exercise the regression path in
_stitch_tool_resultsVariant 2 and would have caught the original bug. A couple of small, optional tightenings you may want to consider:
test_inner_anchor_preserved_in_dead_end_subtreeandtest_three_parallel_agents_all_anchors_preservedonly assert membership. Pinning the expected order (e.g.["a", "tool1", "tool2", "res2", "res1", "att1", "cont"]) would lock in the timestamp-sort behaviour of the newdead_ends + extracted_anchorspath and catch future reorderings.test_anchor_attachment_routes_subagent_sessioncould additionally assert thatsub1is emitted immediately afterres2in the traversal, which is the user-visible property the UI depends on:Optional stronger assertion
- traversed_uuids = [getattr(e, "uuid", "") for e in traverse_session_tree(tree)] - assert "sub1" in traversed_uuids, ( - "subagent entry not reached via anchor — agent session not spliced" - ) + traversed_uuids = [getattr(e, "uuid", "") for e in traverse_session_tree(tree)] + assert "sub1" in traversed_uuids, ( + "subagent entry not reached via anchor — agent session not spliced" + ) + # Subagent must be spliced right after its anchor. + assert traversed_uuids.index("sub1") == traversed_uuids.index("res2") + 1
- Minor: the local
from claude_code_log.converter import _integrate_agent_entriescould move to module-level imports for consistency with the other tests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/test_dag.py` around lines 1166 - 1301, The tests currently only assert membership; tighten them by (1) replacing the broad membership checks in test_inner_anchor_preserved_in_dead_end_subtree and test_three_parallel_agents_all_anchors_preserved with exact expected-order assertions (e.g. assert the main traversal/UUID list equals the timestamp-sorted sequence like ["a","tool1","tool2","res2","res1","att1","cont"] or contains those in that order) to lock in the dead_ends+extracted_anchors ordering, (2) in test_anchor_attachment_routes_subagent_session add a stronger assertion that "sub1" is immediately after "res2" in the traversal result (check the index of "res2" and assert traversal[index+1] == "sub1"), and (3) move the local import of _integrate_agent_entries to the module-level imports for consistency with other tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@claude_code_log/dag.py`:
- Around line 389-400: The code collects anchors into extracted_anchors by
calling _collect_agent_anchors for each assistant child, which can produce
duplicate UUIDs if two assistant siblings share a descendant; to fix, dedupe
extracted_anchors (e.g., convert to a set and back to a list) before appending
to dead_ends so each anchor UUID appears only once, then sort dead_ends by
nodes[c].timestamp as before to preserve deterministic emission and the final
user_with_cont ordering.
In `@claude_code_log/html/templates/components/message_styles.css`:
- Around line 734-745: The CSS uses a hardcoded string in
.tool-param-collapsible[open] > summary::after (content: "collapse") which
prevents localization; change it to use a CSS custom property (e.g., content:
var(--label-collapse, "collapse")) or move the hint into the template as a real
element so the text can be produced by the i18n pipeline; update the stylesheet
rule for summary::after and ensure templates set --label-collapse per locale (or
render an element inside summary that gets translated) so the hint becomes
localizable.
In `@test/test_dag.py`:
- Around line 1166-1301: The tests currently only assert membership; tighten
them by (1) replacing the broad membership checks in
test_inner_anchor_preserved_in_dead_end_subtree and
test_three_parallel_agents_all_anchors_preserved with exact expected-order
assertions (e.g. assert the main traversal/UUID list equals the timestamp-sorted
sequence like ["a","tool1","tool2","res2","res1","att1","cont"] or contains
those in that order) to lock in the dead_ends+extracted_anchors ordering, (2) in
test_anchor_attachment_routes_subagent_session add a stronger assertion that
"sub1" is immediately after "res2" in the traversal result (check the index of
"res2" and assert traversal[index+1] == "sub1"), and (3) move the local import
of _integrate_agent_entries to the module-level imports for consistency with
other tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 334a1eb4-1635-4784-a10d-ea78b5e7acb0
📒 Files selected for processing (5)
claude_code_log/dag.pyclaude_code_log/html/templates/components/message_styles.cssclaude_code_log/html/tool_formatters.pytest/__snapshots__/test_snapshot_html.ambrtest/test_dag.py
The detail-level filter and compact-Markdown mode landed in #96 and #114 but weren't surfaced in the README. Add: - Key Features bullet calling out the pairing with --format md for feeding past conversations to an LLM - New Usage subsection "Feeding Past Conversations to an LLM" with the --detail low --format md --compact combo and a short level reference - Problem-statement entry for LLM-based analysis/experience building - Markdown Output Features bullet for --compact Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Tasktool_uses (e.g. teammates orchestrating multiple subagents) produced tool_result anchors nested inside sibling assistant subtrees. The DAG walker's variant 2 stitch classified those subtrees as dead-end and dropped the agentId-bearing anchors, leaving subagent sessions with no attachment point. Only the outermost subagent rendered. Fixes the issue Support "teammates" #91 symptom where 4 of 6 subagents were missing from a teammates session.<details class='tool-param-collapsible'>had[open] > summary { display: none }, hiding the browser's disclosure marker along with the preview — expanded values had no visible collapse affordance.Changes
claude_code_log/dag.py+test/test_dag.py(commit 6b37e8d)_collect_agent_anchors(uuid, session_uuids, nodes)walks an assistant subtree to findUserTranscriptEntrydescendants withagentIdset._stitch_tool_resultsvariant 2, after dead-end checks pass, extract agent anchors fromassistant_childrensubtrees and include them indead_ends(timestamp-sorted). Anchors end up in both the chain (preserving attachment points) andskipped(via_collect_descendants(assistant, ...)) — that's fine,walked ∩ skippedis allowed.experiments/worktreesshape), end-to-end splice verifying a subagent entry is reached through the anchor.claude_code_log/html/tool_formatters.py+ CSS (commit a0ed7d7)<span class='tool-param-preview'>at bothrender_params_tablecall sites (structured JSON and simple string).display: noneon the whole<summary>withdisplay: noneon.tool-param-previewonly, plus::after { content: "collapse"; }for an explicit hint. Summary keepslist-itemdisplay so the browser's disclosure arrow stays visible.Test plan
uv run pytest -n auto -m "not (tui or browser)"— 864 pass, 7 skip (19 pre-existing pagination failures on main, unrelated)uv run pytest -n auto -m browser— 29 pass, 1 skipuv run pyright— 0 errorsuv run ty check— all checks passeduv run ruff check/ruff format --check— cleanexperiments/worktreesregenerates 488K (2 subagents) → 768K (all 6 subagents)Relates to #91.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation