fix(tui): store raw assistant text for Ctrl+Y copy#45
Conversation
Subagent output is typically markdown text, but was previously displayed as raw plain text with a truncated 8-line preview. This change renders subagent output through glamour for proper markdown formatting (headings, lists, code blocks, etc.) and adds interactive expand/collapse support. Changes: - formatSubagentOutput: render markdown via glamour before display - Collapsed state: show first 12 rendered lines with hint to expand - Expanded state: show full rendered output with hint to collapse - Use purple left border (subagentBodyStyle) to distinguish from normal tool output - Add ctrl+e keybinding to toggle expand/collapse of nearest subagent result in viewport - Simplify SubagentDoneMsg to show concise summary without duplicating truncated content (full content shown via ToolResultMsg) - Session replay also stores subagent results as toolResultContentLine for consistent markdown rendering - Add help panel entry for ctrl+e
…g lines Previously Ctrl+Y copied the last assistant response by reverse-scanning m.lines and stripping ANSI codes. This included structural divider lines (── ◇ model via provider ──) in the clipboard output. Now the raw markdown text is stored in lastAssistantRawText during flushText() and session resume, so Ctrl+Y always copies clean content without dividers or other rendered decorations.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 51 minutes and 20 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThe PR enhances subagent output rendering in the TUI by adding expandable/collapsible display with optional Markdown formatting. Ctrl+E toggles expansion state of subagent tool results, Ctrl+Y now copies the stored assistant raw response, and a purple-accented style differentiates subagent output from other tool outputs. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TUI as TUI Controller
participant Formatter
participant Renderer as Markdown Renderer
User->>TUI: Press Ctrl+E on subagent output
activate TUI
TUI->>TUI: Toggle expanded state
TUI->>Formatter: formatToolResultBody(expanded=true, mdRenderer)
deactivate TUI
activate Formatter
alt Markdown renderer available
Formatter->>Renderer: Render markdown content
Renderer-->>Formatter: Rendered output
Formatter->>Formatter: Display full rendered content
else Renderer nil or render fails
Formatter->>Formatter: Display raw content truncated/full
end
deactivate Formatter
Formatter-->>User: Display with control hint (▲ ctrl+e 收起)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/tui/tui.go (1)
2111-2126:⚠️ Potential issue | 🔴 CriticalMissing result output in live subagent completion.
The success branch only appends a
✓ Subagent Doneheader — there is notoolResultContentLine("subagent", ...)emission formsg.Result(the subagent's output). Compare with the session-resume path at line 1818 which emits the result viatoolResultContentLine("subagent", sanitize(e.Output), nil).Since
SubagentDoneMsg.Resultis populated with output data but never appended tom.lineson completion, users would only see output flicker in the in-progress box (renderSubagentBox) during the run. The final output would be missing from the conversation history, defeating the purpose of thectrl+eexpand toggle and makingCtrl+Ycopy ineffective on recent live subagent messages.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/tui/tui.go` around lines 2111 - 2126, The SubagentDoneMsg success branch currently only appends the "✓ Subagent Done" header and omits the subagent output; update the success branch in the SubagentDoneMsg case to append the subagent result into m.lines by calling toolResultContentLine("subagent", sanitize(msg.Result), nil) (or use truncate(sanitize(msg.Result), maxToolOutputLen) if consistent with other uses), mirroring the session-resume path’s behavior; ensure you only emit the result when msg.Result is non-empty and keep the existing refreshViewport() and spinner.Tick append.
🧹 Nitpick comments (2)
internal/tui/tui.go (2)
2682-2697: Minor: backward scan finds the earliest, not nearest, subagent block.The function comment says it finds the nearest subagent block, and the forward scan starting at
startIdxhonors that. The fallback loop, however, iteratesi := 0; i < startIdx, which returns the first subagent block from the top of history rather than the one closest to the viewport top. For a session with many subagent results, pressing Ctrl+E with no subagent below the fold will jump all the way back to the very first one.♻️ Suggested fix to scan backward from `startIdx-1`
// If not found forward, search from beginning to viewport top. - for i := 0; i < startIdx; i++ { + for i := startIdx - 1; i >= 0; i-- { if m.lines[i].tool != nil && m.lines[i].tool.name == "subagent" { m.lines[i].tool.expanded = !m.lines[i].tool.expanded m.refreshViewport() return } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/tui/tui.go` around lines 2682 - 2697, The fallback loop scans from the top and returns the earliest subagent instead of the nearest before startIdx; change the backward search to iterate from startIdx-1 down to 0 so it finds the closest prior m.lines entry whose tool.name == "subagent", toggle its tool.expanded and call m.refreshViewport() just like the forward branch; update the second loop logic that currently uses for i := 0; i < startIdx to a reverse loop (starting at startIdx-1, decrementing to 0) to locate the nearest previous subagent block.
2620-2646:maxRenderedLinesignores markdown rendering and expanded/collapsed state.For
subagenttool results, the actual rendered height depends on:
- Whether
expandedis true (full glamour output) vs false (capped atcollapsedLines = 12+ hint).- Glamour markdown rendering, which can substantially expand line count (headings, code-block framing, padding).
Estimating from the raw output (with simple wrap math) can mis-locate the viewport top by many lines when a long expanded subagent block is on screen, causing
toggleSubagentExpandto pick the wrong block in some cases. The current heuristic is good enough for common cases, but consider either:
- Caching the last rendered line count per
contentLine, or- Accounting for
tool.expandedand thecollapsedLinescap whentool.name == "subagent".Not a blocker — the dual forward/backward scan masks small inaccuracies — but worth a follow-up if users report ctrl+e toggling unexpected blocks.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/tui/tui.go` around lines 2620 - 2646, maxRenderedLines currently estimates height from raw text and ignores subagent expanded/collapsed state and markdown glamour rendering; update maxRenderedLines to special-case when tool.name == "subagent": if tool.expanded is false return min(estimatedLines, collapsedLines)+1 (include the hint line), and if tool.expanded is true compute the rendered line count by either (a) rendering the tool.output through the same glamour/markdown renderer used elsewhere and counting the resulting lines, or (b) using a cached rendered line value on toolResultData (e.g., tool.lastRenderedLines) that the renderer updates when drawing; ensure you reference tool.expanded, tool.name == "subagent", collapsedLines, and optionally tool.lastRenderedLines so toggleSubagentExpand will get accurate heights.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/tui/format.go`:
- Around line 222-260: The function formatSubagentOutput currently shows the "▲
ctrl+e 收起" hint only when expanded AND len(rawLines) > collapsedLines, but it
doesn't indicate when expanded==true and len(rawLines) <= collapsedLines; update
formatSubagentOutput to either (A) suppress the expand/collapse hint entirely
for short content by ensuring the hint is only added when len(rawLines) >
collapsedLines, or (B) explicitly render a no-op/disabled hint when expanded is
true but content is short so the UI reflects the state; alternatively, also
update toggleSubagentExpand to be a no-op when len(rawLines) <= collapsedLines
to prevent state flips—make the change around the expanded check and the place
that appends lipgloss.NewStyle().Render("▲ ctrl+e 收起") so the behavior is
consistent.
- Around line 253-275: The UI hints in the subagent output box are in Chinese;
update the two rendered hint strings so they’re in English and match the rest of
the TUI (replace "▲ ctrl+e 收起" with something like "▲ ctrl+e Collapse" and the
trailing hint "… %d more lines (ctrl+e 展开)" with "… %d more lines (ctrl+e
Expand)") inside the code that builds boxContent (references: rawLines,
collapsedLines, colorMuted, lipgloss.NewStyle(), boxContent.WriteString()) so
both expand/collapse prompts are consistent with other labels (e.g., "Expand
subagent output").
---
Outside diff comments:
In `@internal/tui/tui.go`:
- Around line 2111-2126: The SubagentDoneMsg success branch currently only
appends the "✓ Subagent Done" header and omits the subagent output; update the
success branch in the SubagentDoneMsg case to append the subagent result into
m.lines by calling toolResultContentLine("subagent", sanitize(msg.Result), nil)
(or use truncate(sanitize(msg.Result), maxToolOutputLen) if consistent with
other uses), mirroring the session-resume path’s behavior; ensure you only emit
the result when msg.Result is non-empty and keep the existing refreshViewport()
and spinner.Tick append.
---
Nitpick comments:
In `@internal/tui/tui.go`:
- Around line 2682-2697: The fallback loop scans from the top and returns the
earliest subagent instead of the nearest before startIdx; change the backward
search to iterate from startIdx-1 down to 0 so it finds the closest prior
m.lines entry whose tool.name == "subagent", toggle its tool.expanded and call
m.refreshViewport() just like the forward branch; update the second loop logic
that currently uses for i := 0; i < startIdx to a reverse loop (starting at
startIdx-1, decrementing to 0) to locate the nearest previous subagent block.
- Around line 2620-2646: maxRenderedLines currently estimates height from raw
text and ignores subagent expanded/collapsed state and markdown glamour
rendering; update maxRenderedLines to special-case when tool.name == "subagent":
if tool.expanded is false return min(estimatedLines, collapsedLines)+1 (include
the hint line), and if tool.expanded is true compute the rendered line count by
either (a) rendering the tool.output through the same glamour/markdown renderer
used elsewhere and counting the resulting lines, or (b) using a cached rendered
line value on toolResultData (e.g., tool.lastRenderedLines) that the renderer
updates when drawing; ensure you reference tool.expanded, tool.name ==
"subagent", collapsedLines, and optionally tool.lastRenderedLines so
toggleSubagentExpand will get accurate heights.
🪄 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: 8097cb5d-b688-42c4-a08d-bbc59f709012
📒 Files selected for processing (4)
internal/tui/format.gointernal/tui/pickers.gointernal/tui/styles.gointernal/tui/tui.go
| func formatSubagentOutput(output string, termWidth int, expanded bool, mdRenderer *glamour.TermRenderer) []string { | ||
| output = strings.TrimRight(output, "\n") | ||
| if output == "" { | ||
| return nil | ||
| } | ||
|
|
||
| shown := rawLines | ||
| hidden := 0 | ||
| if len(rawLines) > tailLines { | ||
| shown = rawLines[:tailLines] | ||
| hidden = len(rawLines) - tailLines | ||
| // Render markdown via glamour if available. | ||
| rendered := output | ||
| if mdRenderer != nil { | ||
| if md, err := mdRenderer.Render(output); err == nil { | ||
| rendered = strings.TrimRight(md, "\n") | ||
| } | ||
| } | ||
|
|
||
| const collapsedLines = 12 | ||
| rawLines := strings.Split(rendered, "\n") | ||
|
|
||
| boxWidth := termWidth - 8 | ||
| if boxWidth < 30 { | ||
| boxWidth = 30 | ||
| } | ||
|
|
||
| if expanded || len(rawLines) <= collapsedLines { | ||
| // Show all content. | ||
| var boxContent strings.Builder | ||
| for i, line := range rawLines { | ||
| boxContent.WriteString(line) | ||
| if i < len(rawLines)-1 { | ||
| boxContent.WriteString("\n") | ||
| } | ||
| } | ||
| if expanded && len(rawLines) > collapsedLines { | ||
| boxContent.WriteString("\n") | ||
| boxContent.WriteString(lipgloss.NewStyle().Foreground(colorMuted).Italic(true). | ||
| Render("▲ ctrl+e 收起")) | ||
| } | ||
| box := subagentBodyStyle.Width(boxWidth).Render(boxContent.String()) | ||
| return []string{box} | ||
| } |
There was a problem hiding this comment.
Edge case: short rendered content + expanded=true shows no collapse hint.
When expanded is true but len(rawLines) <= collapsedLines, the user sees the same output as the collapsed/short state with no indication that they're already in "expanded" mode or that Ctrl+E will toggle. This is a minor UX wart since pressing Ctrl+E will silently flip a hidden state, then a second press still does nothing visible. Not a blocker — the content is already fully visible — but worth confirming intent. If desired, you could also skip the toggle entirely for short content in toggleSubagentExpand.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/tui/format.go` around lines 222 - 260, The function
formatSubagentOutput currently shows the "▲ ctrl+e 收起" hint only when expanded
AND len(rawLines) > collapsedLines, but it doesn't indicate when expanded==true
and len(rawLines) <= collapsedLines; update formatSubagentOutput to either (A)
suppress the expand/collapse hint entirely for short content by ensuring the
hint is only added when len(rawLines) > collapsedLines, or (B) explicitly render
a no-op/disabled hint when expanded is true but content is short so the UI
reflects the state; alternatively, also update toggleSubagentExpand to be a
no-op when len(rawLines) <= collapsedLines to prevent state flips—make the
change around the expanded check and the place that appends
lipgloss.NewStyle().Render("▲ ctrl+e 收起") so the behavior is consistent.
Summary
Previously
Ctrl+Ycopied the last assistant response by reverse-scanningm.linesand stripping ANSI codes. This included structural divider lines (── ◇ model via provider ──) in the clipboard output.Changes
lastAssistantRawTextfield to the TUI Model structflushText()before renderingSessionResumedMsg/EntryAssistant)Ctrl+Yhandler to read directly from the variablegetLastAssistantText()andisDividerLine()— no more reverse line scanningTest Plan
Ctrl+Y— clipboard should contain only the assistant markdown, no dividerCtrl+Y— clipboard should contain the last assistant response from the resumed sessionCtrl+Yduring streaming — should copy the current streaming textSummary by CodeRabbit
Release Notes
New Features
Improvements
Style