Surface the model each agent ran on, once per agent context (#246)#252
Open
cboos wants to merge 5 commits into
Open
Surface the model each agent ran on, once per agent context (#246)#252cboos wants to merge 5 commits into
cboos wants to merge 5 commits into
Conversation
Sub-agent transcripts can run on a different model than the trunk (e.g. a workflow fans out haiku agents under an opus main). Surface the model id so a reader can see it at a glance, without repeating it on every message: - the session header carries the trunk/main model (first non-sidechain assistant entry's model); - the first message of each sub-agent carries that sub-agent's model. A mid-course /model switch shows as its own command message, so a single first-seen value per context suffices. Plumbing: AssistantMessageModel.model -> MessageMeta.model (set in the meta factory) -> TemplateMessage.display_model, marked once per agent context by _surface_agent_models. HTML renders a muted .message-model pill in the header / on the session line; Markdown emits a `Model:` line per sub-agent and an inline `— Model:` suffix on the session heading. dev-docs/agents.md §5.4 documents the new annotation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The nested_agents fixture runs the trunk and every sub-agent on the same haiku id, so the once-per-context tests assert the same string in two places and can't distinguish correct attribution from cross- contamination — which is exactly #246's headline. Retarget only the trunk's assistant entries to a distinct model (opus) in-test and pin that the session header shows IT while each sub-agent keeps haiku, with no leakage either way, across both renderers. (monk review #3959.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR adds assistant model metadata, surfaces it once per agent context, and renders it in HTML and Markdown session and task headers. Documentation, tests, and snapshots were updated to reflect the new model labels. ChangesSub-agent and trunk model surfacing
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
claude_code_log/markdown/renderer.py (1)
708-744: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocstring example doesn't reflect the new Model suffix.
The docstring still documents the title shape as
'📋 Session \abc12345`: summary — Team: `t`'with no mention of the new "— Model: \...`" segment that's now inserted before Team for non-branch sessions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@claude_code_log/markdown/renderer.py` around lines 708 - 744, Update the docstring for title_SessionHeaderMessage so its example matches the current SessionHeaderMessage title format, including the new “— Model: `...`” segment for non-branch sessions before the Team suffix. Keep the branch example accurate too, and make sure the documented shapes reflect the logic in title_SessionHeaderMessage and the message.display_model branch.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@claude_code_log/html/templates/transcript.html`:
- Line 132: The session header rendering is inconsistent between HTML and
Markdown because the shared `div.header` in `transcript.html` always shows
`message.display_model`, while `markdown/renderer.py`’s
`title_SessionHeaderMessage` only adds the model suffix on the trunk path.
Update the branch-header logic so `message.is_branch_header` and
`message.display_model` are handled consistently in both renderers, using the
same rule for whether `.message-model` or the “— Model:” suffix should appear.
Check the `header` block in `transcript.html` and `title_SessionHeaderMessage`
in `markdown/renderer.py` together so branch session headers and trunk headers
stay in parity.
In `@claude_code_log/renderer.py`:
- Around line 4856-4892: The _surface_agent_models helper is letting sidechain
messages without msg.meta.agent_id fall through into the trunk/session header
path, which can overwrite the main header’s display_model with a sub-agent
model. Update _surface_agent_models so any msg.meta.is_sidechain entry is
handled only in the sidechain branch, and skip the header-update logic when
agent_id is missing. Use msg.meta.agent_id, msg.meta.is_sidechain,
msg.meta.session_id, and ctx.session_first_message to keep trunk/main models on
the session header and sub-agent models only on the first sidechain message.
---
Nitpick comments:
In `@claude_code_log/markdown/renderer.py`:
- Around line 708-744: Update the docstring for title_SessionHeaderMessage so
its example matches the current SessionHeaderMessage title format, including the
new “— Model: `...`” segment for non-branch sessions before the Team suffix.
Keep the branch example accurate too, and make sure the documented shapes
reflect the logic in title_SessionHeaderMessage and the message.display_model
branch.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6454b951-4a48-44af-bfa1-97b91f415d25
📒 Files selected for processing (10)
claude_code_log/factories/meta_factory.pyclaude_code_log/html/templates/components/message_styles.cssclaude_code_log/html/templates/transcript.htmlclaude_code_log/markdown/renderer.pyclaude_code_log/models.pyclaude_code_log/renderer.pydev-docs/agents.mdtest/__snapshots__/test_snapshot_html.ambrtest/__snapshots__/test_snapshot_markdown.ambrtest/test_nested_agents.py
- _surface_agent_models: a sidechain message without an agent_id no longer falls through to the trunk-header branch (it could otherwise overwrite the session header's main model with a sub-agent's). Now an explicit is_sidechain branch leaves such an entry unattributed. - transcript.html: gate the session-header model pill on `not is_branch_header`, matching the Markdown renderer's structural guard so branch and trunk headers stay in parity across formats. - title_SessionHeaderMessage docstring: document the new `— Model: \`…\`` segment in the title example. Pinned by test_sidechain_without_agent_id_never_leaks_into_header (rogue sidechain entry registered before the trunk model — the old fall-through would have set the header to the rogue model). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The per-message pill sat on the sub-agent's first message, which is
hidden whenever the sub-agent transcript is folded — i.e. exactly where
a reader scanning a spawn ("🔧 Agent … (Explore)") looks for it. Move it
to the spawn card itself: the Task/Agent tool_use that opens the
sub-agent, which stays on screen when the transcript collapses and
already carries the depth badge, so the two annotations sit together.
- `_surface_agent_models` now runs late in `generate_template_messages`
(after pairing + async linking, so a spawn card can reach its paired
tool_result and hoisted `minted_agent_id`). Pass 1 maps agent→model
and stamps the session header; pass 2 stamps each `TaskInput` spawn
card, resolving the spawned agent via `minted_agent_id` (async #90),
else the paired result's `spawned_agent_id` (#213) or `agent_id`.
Gated to `TaskInput` so a regular tool inside a sub-agent never picks
up the model.
- HTML: `_agent_model_badge` appends the pill to the spawn-card title
beside the depth badge (not suppressed at depth 1). The generic
header-info pill is removed.
- Markdown: the model is appended as ` · `model`` to the Task title;
the per-message `Model:` line is removed. Session-header trunk model
unchanged in both renderers.
Verified on the nested (sync), async (#90), and a real multi-agent
transcript: model on every spawn card, none on sub-agent body messages.
dev-docs/agents.md §5.4 updated.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
) The all-haiku fixture can't catch a spawn card resolving to the wrong sub-agent (parent/child collapse in the nschain1→2→3 chain, or an async-fallback mixup). Give every sub-agent a distinct model and assert the bijection: each spawn card carries exactly its own spawned child's model — no duplicates, no leakage, chain each distinct. (monk #3990, who proved this out-of-band; committing it keeps it in CI.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/test_nested_agents.py`:
- Around line 370-390: The expected spawn-model assertion in
test_nested_agents.py should not include INTR because the fixture never creates
an AssistantTranscriptEntry for nsintr01 and _surface_agent_models only assigns
display_model for spawned agents that actually exist. Update the spawn-model
expectation in generate_template_messages-related assertions so it derives from
the agents present in entries (or explicitly excludes nsintr01) while keeping
the bijection check on spawn_models intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 38ffa4eb-a5ae-4375-a8c9-788244259059
📒 Files selected for processing (1)
test/test_nested_agents.py
Comment on lines
+370
to
+390
| entries = _load_integrated() | ||
| for entry in entries: | ||
| if ( | ||
| isinstance(entry, AssistantTranscriptEntry) | ||
| and entry.isSidechain | ||
| and entry.agentId | ||
| ): | ||
| entry.message.model = f"model-{entry.agentId}" | ||
|
|
||
| _roots, _nav, ctx = generate_template_messages(entries) | ||
| msgs = [m for m in ctx.messages if m is not None] | ||
| spawn_models = [ | ||
| m.display_model | ||
| for m in msgs | ||
| if m.display_model and isinstance(m.content, ToolUseMessage) | ||
| ] | ||
| # Bijection: one distinct model per spawn card, each its own sub-agent's. | ||
| assert len(spawn_models) == len(set(spawn_models)), ( | ||
| "a spawn card reused another agent's model (parent/child collapse)" | ||
| ) | ||
| assert set(spawn_models) == {f"model-{agent}" for agent in ALL_AGENTS} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether the interrupted-spawn fixture agent has any assistant entries with an agentId,
# which would determine whether it ever contributes a model.
fd -e jsonl -e json . test | xargs rg -l 'isSidechain' 2>/dev/null | head -20Repository: daaain/claude-code-log
Length of output: 1499
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the test file and inspect the relevant section.
ast-grep outline test/test_nested_agents.py --view expanded >/tmp/nested_outline.txt
sed -n '1,120p' /tmp/nested_outline.txt
echo
echo "=== Relevant test section ==="
sed -n '340,410p' test/test_nested_agents.py
echo
echo "=== Find ALL_AGENTS and INTR definitions ==="
rg -n 'ALL_AGENTS|INTR|test_each_spawn_card_shows_its_own_subagents_model' test/test_nested_agents.py
echo
echo "=== Search fixture/data references for INTR and assistant entries ==="
rg -n '"agentId"|isSidechain|INTR|interrupted spawn|AssistantTranscriptEntry' test/test_data test/test_nested_agents.pyRepository: daaain/claude-code-log
Length of output: 50381
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== test/test_nested_agents.py around the test ==="
sed -n '1,120p' test/test_nested_agents.py
echo
sed -n '230,405p' test/test_nested_agents.py
echo
echo "=== Locate _load_integrated and its fixture path ==="
rg -n 'def _load_integrated|_load_integrated\(|integrated' test/test_nested_agents.py test -g '!test/test_data/**' -g '!**/*.pyc'
echo
echo "=== Search candidate fixture files for INTR and sidechain assistant entries ==="
rg -n '"agentId":"INTR"|\"agentId\": \"INTR\"|INTR|isSidechain|AssistantTranscriptEntry' test/test_data -g '*.jsonl' -g '*.json'
echo
echo "=== If there is a specific integrated fixture path, print the lines around INTR entries ==="Repository: daaain/claude-code-log
Length of output: 18089
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== nested_agents fixture files ==="
fd -t f '33330000-0000-4000-8000-000000000001*.jsonl' test/test_data/nested_agents
echo
echo "=== lines mentioning nsintr01 in the trunk fixture ==="
rg -n 'nsintr01|INTR|assistant|tool_use|tool_result|isSidechain' test/test_data/nested_agents/33330000-0000-4000-8000-000000000001.jsonl
echo
echo "=== count entry types for nsintr01 sidechain records ==="
python3 - <<'PY'
import json
from collections import Counter
from pathlib import Path
path = Path("test/test_data/nested_agents/33330000-0000-4000-8000-000000000001.jsonl")
counts = Counter()
rows = []
with path.open() as f:
for i, line in enumerate(f, 1):
obj = json.loads(line)
msg = obj.get("message") or {}
entry_type = obj.get("type")
agent_id = obj.get("agentId") or msg.get("agentId")
is_sidechain = obj.get("isSidechain")
if agent_id == "nsintr01" or obj.get("spawnedAgentId") == "nsintr01":
rows.append((i, entry_type, is_sidechain, agent_id, obj.get("spawnedAgentId"), msg.get("role"), msg.get("type")))
counts[(entry_type, is_sidechain, msg.get("role"), msg.get("type"))] += 1
print("matching rows:", len(rows))
for row in rows[:50]:
print(row)
print("counts:")
for k, v in counts.items():
print(v, k)
PYRepository: daaain/claude-code-log
Length of output: 6170
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== renderer model-surfacing logic ==="
rg -n '_surface_agent_models|display_model|agent_model|spawnedAgentId|isSidechain' claude_code_log test -g '*.py'
echo
echo "=== relevant renderer slice ==="
sed -n '1,220p' claude_code_log/renderer.pyRepository: daaain/claude-code-log
Length of output: 50378
Exclude INTR from the expected spawn-model set. The fixture has no AssistantTranscriptEntry for nsintr01, and _surface_agent_models only stamps display_model when a spawned agent actually has one, so set(spawn_models) can never include model-nsintr01.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/test_nested_agents.py` around lines 370 - 390, The expected spawn-model
assertion in test_nested_agents.py should not include INTR because the fixture
never creates an AssistantTranscriptEntry for nsintr01 and _surface_agent_models
only assigns display_model for spawned agents that actually exist. Update the
spawn-model expectation in generate_template_messages-related assertions so it
derives from the agents present in entries (or explicitly excludes nsintr01)
while keeping the bijection check on spawn_models intact.
Collaborator
Author
|
(Claude) Re CodeRabbit's note on |
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.
Closes #246.
Sub-agent transcripts can run on a different model than the trunk — e.g. a workflow fans out
haikuagents under anopusmain, or aTask(model="sonnet")spawn. This surfaces the model id each agent ran on so a reader can see it at a glance, once per agent context rather than repeated on every message:A mid-course
/modelswitch already shows as its own command message, so a single first-seen value per context is enough.How it looks
.message-modelpill in the message header (beside the timestamp) and inline on the session-header line, mirroring the existingworkflow-agent-modelstyle.Model:`…`line per sub-agent and an inline— Model:suffix on the session heading.Plumbing
AssistantMessageModel.model→MessageMeta.model(set in the meta factory, guarded to assistant entries) →TemplateMessage.display_model, the render-once decision made by the new_surface_agent_modelspass at the end of_render_messages(keys sub-agents byagent_id, the trunk bysession_id→ its session header; guards ghostedNoneslots).The model id is transcript-derived (attacker-controllable via a crafted transcript, post-#245), so both sinks neutralise it: HTML autoescapes via Jinja (no
|safe), Markdown fences it via_inline_code.Tests
test/test_nested_agents.py(TestNestedVisualLayer): once-per-sub-agent marking, trunk-model-on-session-header-only, HTML+Markdown rendering, and — pinning #246's core promise — a distinct-model case (retarget the trunk toopus, sub-agents stayhaiku) asserting correct per-context attribution with no cross-contamination.dev-docs/agents.md§5.4 documents the new annotation.just cigreen (incl. tui+browser); snapshots updated.Reviewed by
monk(approved; 3 watch-points + XSS verified). Design driven in-session by @daaain.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests