Repository navigation
Let the agent transcript view show thinking bodies - #272
apoapostolov wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
There are no active review issues for the change that makes child-agent reasoning visible in transcripts.
No open issues 🎉
What changed in this PR
This PR addresses #271 by showing child-agent reasoning in the transcript instead of a bare Thinking... placeholder, matching the main chat’s default behavior.
Changes:
- Enables thinking bodies for both live and stored assistant messages.
- Updates the transcript test to require reasoning text.
- Leaves click-to-collapse support outside this change’s scope.
| File | Description |
|---|---|
test/agent-view.test.ts |
Changes the assertion to require visible reasoning. |
src/agent-view.ts |
Disables thinking-body hiding at both component construction sites. |
Review effort: Balanced
|
Thanks! This is a real bug: the subagent transcript view always hides thinking ( One gap: it should follow Pi's own hide-thinking setting (Ctrl+T) instead of always showing thinking. Reading (The pre-existing failing test you mentioned passes for us on a clean |
c827c94 to
0fb667a
Compare
|
Added, thanks — good catch.
I also gave it a test rather than only the existing "reasoning is visible" assertion. It renders a saved child session containing a thinking block, checks the reasoning text appears, then writes
One limit worth naming: the setting is read when the view opens, so toggling Ctrl+T while you are inside a child transcript takes effect the next time that transcript is opened, not live. Updating it live would mean rebuilding the message components on each toggle — happy to add that here if you want it. |
There was a problem hiding this comment.
🟡 Changes recommended
Reasoning visibility fails to honor hideThinkingBlock consistently because connected windows inherit the visible default and agent views use child-project settings instead of the host window’s preference.
2 open issues
Review effort: Balanced
| private expanded = false; | ||
| constructor(private readonly tui: TUI, private readonly cwd: string, private readonly definition: (name: string) => Renderers = () => undefined) {} | ||
| constructor(private readonly tui: TUI, private readonly cwd: string, private readonly definition: (name: string) => Renderers = () => undefined, | ||
| private readonly hideThinking = false) {} |
| const run = children.history.records.get(id); | ||
| this.transcript = new TranscriptView(tui, run?.cwd ?? process.cwd(), name => children.live(id)?.session.getToolDefinition(name)); | ||
| // Follow Pi's hide-thinking setting (Ctrl+T), like the main chat's transcript does. | ||
| const hideThinking = SettingsManager.create(run?.cwd ?? process.cwd(), getAgentDir()).getHideThinkingBlock(); |
The view built its own AssistantMessageComponent with hideThinkingBlock set, so a child agent's reasoning never reached the screen: the block rendered as a bare "Thinking..." line with nothing behind it and no key to open it. Pass false, as the main chat does, so the reasoning shows with the rest of the answer. The transcript flattens its parts into lines for rendering, so a click still cannot reach the block's own collapse toggle. That is a separate job: this only stops the label from being a dead end.
The maintainer asked for one addition: the transcript view should hide thinking bodies when the user hides them in the main chat (Ctrl+T) instead of always showing them. Read SettingsManager.getHideThinkingBlock() once in the constructor and hand it to the message components; nothing else changes.
0fb667a to
27eb785
Compare
Closes #271.
What changed
TranscriptViewbuilt its ownAssistantMessageComponentwithhideThinkingBlockset, so a child agent's reasoning never reached the screen: the block rendered as a bareThinking...line with nothing behind it. Both construction sites now passfalse, which is what Pi's main chat does by default.Two lines of source and one assertion.
Evidence
test/agent-view.test.tsalready builds a child conversation carrying{ type: 'thinking', thinking: 'hidden reasoning' }, and used to assert that the text stays out of the transcript. Onf724d31that view renders:With the change, the reasoning renders in place of the bare label and the assertion flips to require it. Nothing else in the transcript moved:
npm testgives 366 tests, 364 pass, 1 fail, 1 skipped, and the failure (agents.test.ts, "a task may name its own model and thinking level…") also fails onf724d31without this branch.npm run checkis clean.What this does not fix
The transcript flattens its parts into rendered lines (
TranscriptView.render,parts.flatMap(part => part.render(width))) andhandleMouseonly forwards wheel events, so a click cannot reach the block's own collapse toggle. That was invisible while the body never showed. Making it work means keeping each part's row range to hit-test against, which is a different change from this one, so I left it out rather than grow the diff.If the maintainer's answer to #271 is option 2 instead (keep a child's reasoning out of its transcript), this PR is the wrong one and the honest fix is to stop drawing the placeholder, which is a smaller patch than it sounds.