feat(chat): add compact tool UI mode for executed tool blocks (#322)#327
feat(chat): add compact tool UI mode for executed tool blocks (#322)#327proyectoauraorg wants to merge 3 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds an optional ChangesCompact Tool UI Feature
Sequence DiagramsequenceDiagram
participant SettingsView
participant ClineProvider
participant ExtensionStateContext
participant ChatRow
participant User
SettingsView->>ClineProvider: set cached compactToolUI (toggle)
ClineProvider->>ExtensionStateContext: include compactToolUI in posted state
ExtensionStateContext->>ChatRow: provide compactToolUI via context
ChatRow->>User: render compact single-line summary when enabled
User->>ChatRow: click summary to expand tool details
ChatRow->>User: render full tool UI
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/core/webview/ClineProvider.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
webview-ui/src/components/settings/SettingsView.tsx (1)
366-426:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
compactToolUIis not persisted on Save.Line 366 builds
updatedSettings, butcompactToolUIis missing, so the new toggle won’t survive save/reload.💡 Suggested fix
updatedSettings: { language, @@ showRooIgnoredFiles: showRooIgnoredFiles ?? true, + compactToolUI: compactToolUI ?? false, enableSubfolderRules: enableSubfolderRules ?? false,As per coding guidelines, “For
SettingsView, preserve the cached-state pattern: inputs should operate on localcachedStateuntil the user saves…”.🤖 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 `@webview-ui/src/components/settings/SettingsView.tsx` around lines 366 - 426, The updatedSettings object built in SettingsView omits compactToolUI so the toggle in cachedState never gets persisted; add compactToolUI: compactToolUI ?? /* default value or cachedState.compactToolUI */ to the updatedSettings payload (keeping the same null/undefined handling pattern as other fields) so the local cachedState value for compactToolUI is saved on Save and survives reload.
🧹 Nitpick comments (1)
webview-ui/src/components/settings/__tests__/ContextManagementSettings.spec.tsx (1)
154-169: ⚡ Quick winAdd one SettingsView save-path regression test for
compactToolUI.These tests validate checkbox wiring, but not persistence through
updateSettings. A SettingsView test assertingvscode.postMessage({ type: "updateSettings", updatedSettings: { compactToolUI: ... } })would catch the current integration bug.As per coding guidelines, “Prefer local
webview-uitests for React/webview behavior such as component rendering, local state, hooks, form dirty-state, validation, or prop wiring.”🤖 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 `@webview-ui/src/components/settings/__tests__/ContextManagementSettings.spec.tsx` around lines 154 - 169, Tests for ContextManagementSettings only check checkbox wiring but do not assert that toggling persists via SettingsView's updateSettings message; add a new test in the SettingsView test suite that mounts the SettingsView (or renders the component that wires ContextManagementSettings into the webview messaging layer), simulates toggling the compactToolUI checkbox, and asserts that vscode.postMessage was called with { type: "updateSettings", updatedSettings: { compactToolUI: true/false } }; locate the integration wiring around SettingsView (where ContextManagementSettings is used and updateSettings is invoked) and mock/spy on vscode.postMessage to validate the exact payload for compactToolUI.
🤖 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 `@webview-ui/src/components/chat/ChatRow.tsx`:
- Around line 1423-1434: Replace the clickable div used for compact-row
expansion with a semantic <button type="button"> so keyboard users can activate
it; move the onClick={handleToggleExpand} to the button, keep the className and
children (ChevronRight, PocketKnife, span), add an aria-expanded attribute bound
to the component's expanded state (e.g., aria-expanded={isExpanded} or the
appropriate state variable) and preserve data-testid="compact-tool-row"; ensure
the button does not submit forms (type="button") and that any focus/hover styles
remain intact so keyboard users can tab and press Enter/Space to toggle the
compact tool row.
---
Outside diff comments:
In `@webview-ui/src/components/settings/SettingsView.tsx`:
- Around line 366-426: The updatedSettings object built in SettingsView omits
compactToolUI so the toggle in cachedState never gets persisted; add
compactToolUI: compactToolUI ?? /* default value or cachedState.compactToolUI */
to the updatedSettings payload (keeping the same null/undefined handling pattern
as other fields) so the local cachedState value for compactToolUI is saved on
Save and survives reload.
---
Nitpick comments:
In
`@webview-ui/src/components/settings/__tests__/ContextManagementSettings.spec.tsx`:
- Around line 154-169: Tests for ContextManagementSettings only check checkbox
wiring but do not assert that toggling persists via SettingsView's
updateSettings message; add a new test in the SettingsView test suite that
mounts the SettingsView (or renders the component that wires
ContextManagementSettings into the webview messaging layer), simulates toggling
the compactToolUI checkbox, and asserts that vscode.postMessage was called with
{ type: "updateSettings", updatedSettings: { compactToolUI: true/false } };
locate the integration wiring around SettingsView (where
ContextManagementSettings is used and updateSettings is invoked) and mock/spy on
vscode.postMessage to validate the exact payload for compactToolUI.
🪄 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 Plus
Run ID: 24ee0f03-114c-47e8-8ef6-5328ecd340a5
📒 Files selected for processing (43)
packages/types/src/global-settings.tspackages/types/src/vscode-extension-host.tswebview-ui/src/components/chat/ChatRow.tsxwebview-ui/src/components/settings/ContextManagementSettings.tsxwebview-ui/src/components/settings/SettingsView.tsxwebview-ui/src/components/settings/__tests__/ContextManagementSettings.spec.tsxwebview-ui/src/context/ExtensionStateContext.tsxwebview-ui/src/i18n/locales/ca/chat.jsonwebview-ui/src/i18n/locales/ca/settings.jsonwebview-ui/src/i18n/locales/de/chat.jsonwebview-ui/src/i18n/locales/de/settings.jsonwebview-ui/src/i18n/locales/en/chat.jsonwebview-ui/src/i18n/locales/en/settings.jsonwebview-ui/src/i18n/locales/es/chat.jsonwebview-ui/src/i18n/locales/es/settings.jsonwebview-ui/src/i18n/locales/fr/chat.jsonwebview-ui/src/i18n/locales/fr/settings.jsonwebview-ui/src/i18n/locales/hi/chat.jsonwebview-ui/src/i18n/locales/hi/settings.jsonwebview-ui/src/i18n/locales/id/chat.jsonwebview-ui/src/i18n/locales/id/settings.jsonwebview-ui/src/i18n/locales/it/chat.jsonwebview-ui/src/i18n/locales/it/settings.jsonwebview-ui/src/i18n/locales/ja/chat.jsonwebview-ui/src/i18n/locales/ja/settings.jsonwebview-ui/src/i18n/locales/ko/chat.jsonwebview-ui/src/i18n/locales/ko/settings.jsonwebview-ui/src/i18n/locales/nl/chat.jsonwebview-ui/src/i18n/locales/nl/settings.jsonwebview-ui/src/i18n/locales/pl/chat.jsonwebview-ui/src/i18n/locales/pl/settings.jsonwebview-ui/src/i18n/locales/pt-BR/chat.jsonwebview-ui/src/i18n/locales/pt-BR/settings.jsonwebview-ui/src/i18n/locales/ru/chat.jsonwebview-ui/src/i18n/locales/ru/settings.jsonwebview-ui/src/i18n/locales/tr/chat.jsonwebview-ui/src/i18n/locales/tr/settings.jsonwebview-ui/src/i18n/locales/vi/chat.jsonwebview-ui/src/i18n/locales/vi/settings.jsonwebview-ui/src/i18n/locales/zh-CN/chat.jsonwebview-ui/src/i18n/locales/zh-CN/settings.jsonwebview-ui/src/i18n/locales/zh-TW/chat.jsonwebview-ui/src/i18n/locales/zh-TW/settings.json
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…de-Org#322) Adds an opt-in global setting (compactToolUI, off by default) that collapses executed (say) tool blocks in the chat history to a single clickable line, hiding verbose descriptions and JSON payloads until the user expands them. Approval prompts (ask) are never compacted, so users always see the parameters they are approving. Toggle lives in Context Management settings; new i18n keys added across all 18 locales. Closes Zoo-Code-Org#322.
d5ae590 to
fd5ecdd
Compare
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 `@webview-ui/src/components/settings/SettingsView.tsx`:
- Around line 838-839: The save payload omits the compactToolUI field so
toggling it is not persisted; update the handleSubmit function to include
compactToolUI (the same state/prop used when rendering) in the updatedSettings
object before sending/saving, e.g. add compactToolUI: compactToolUI (or
compactToolUI ?? false) to updatedSettings so the toggle is persisted.
🪄 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 Plus
Run ID: 932d5575-a57a-49f6-b31c-0dc5f5e739cc
📒 Files selected for processing (43)
packages/types/src/global-settings.tspackages/types/src/vscode-extension-host.tswebview-ui/src/components/chat/ChatRow.tsxwebview-ui/src/components/settings/ContextManagementSettings.tsxwebview-ui/src/components/settings/SettingsView.tsxwebview-ui/src/components/settings/__tests__/ContextManagementSettings.spec.tsxwebview-ui/src/context/ExtensionStateContext.tsxwebview-ui/src/i18n/locales/ca/chat.jsonwebview-ui/src/i18n/locales/ca/settings.jsonwebview-ui/src/i18n/locales/de/chat.jsonwebview-ui/src/i18n/locales/de/settings.jsonwebview-ui/src/i18n/locales/en/chat.jsonwebview-ui/src/i18n/locales/en/settings.jsonwebview-ui/src/i18n/locales/es/chat.jsonwebview-ui/src/i18n/locales/es/settings.jsonwebview-ui/src/i18n/locales/fr/chat.jsonwebview-ui/src/i18n/locales/fr/settings.jsonwebview-ui/src/i18n/locales/hi/chat.jsonwebview-ui/src/i18n/locales/hi/settings.jsonwebview-ui/src/i18n/locales/id/chat.jsonwebview-ui/src/i18n/locales/id/settings.jsonwebview-ui/src/i18n/locales/it/chat.jsonwebview-ui/src/i18n/locales/it/settings.jsonwebview-ui/src/i18n/locales/ja/chat.jsonwebview-ui/src/i18n/locales/ja/settings.jsonwebview-ui/src/i18n/locales/ko/chat.jsonwebview-ui/src/i18n/locales/ko/settings.jsonwebview-ui/src/i18n/locales/nl/chat.jsonwebview-ui/src/i18n/locales/nl/settings.jsonwebview-ui/src/i18n/locales/pl/chat.jsonwebview-ui/src/i18n/locales/pl/settings.jsonwebview-ui/src/i18n/locales/pt-BR/chat.jsonwebview-ui/src/i18n/locales/pt-BR/settings.jsonwebview-ui/src/i18n/locales/ru/chat.jsonwebview-ui/src/i18n/locales/ru/settings.jsonwebview-ui/src/i18n/locales/tr/chat.jsonwebview-ui/src/i18n/locales/tr/settings.jsonwebview-ui/src/i18n/locales/vi/chat.jsonwebview-ui/src/i18n/locales/vi/settings.jsonwebview-ui/src/i18n/locales/zh-CN/chat.jsonwebview-ui/src/i18n/locales/zh-CN/settings.jsonwebview-ui/src/i18n/locales/zh-TW/chat.jsonwebview-ui/src/i18n/locales/zh-TW/settings.json
✅ Files skipped from review due to trivial changes (21)
- webview-ui/src/i18n/locales/hi/chat.json
- webview-ui/src/i18n/locales/de/settings.json
- webview-ui/src/i18n/locales/es/chat.json
- webview-ui/src/i18n/locales/nl/settings.json
- webview-ui/src/i18n/locales/en/chat.json
- webview-ui/src/i18n/locales/hi/settings.json
- webview-ui/src/i18n/locales/vi/settings.json
- webview-ui/src/i18n/locales/ja/settings.json
- webview-ui/src/i18n/locales/fr/settings.json
- webview-ui/src/i18n/locales/tr/settings.json
- webview-ui/src/i18n/locales/zh-TW/chat.json
- webview-ui/src/i18n/locales/en/settings.json
- webview-ui/src/i18n/locales/ru/settings.json
- webview-ui/src/i18n/locales/ru/chat.json
- webview-ui/src/i18n/locales/ko/chat.json
- webview-ui/src/i18n/locales/ko/settings.json
- webview-ui/src/i18n/locales/ca/settings.json
- webview-ui/src/i18n/locales/fr/chat.json
- webview-ui/src/i18n/locales/zh-CN/settings.json
- webview-ui/src/i18n/locales/ja/chat.json
- webview-ui/src/i18n/locales/pt-BR/settings.json
- Add compactToolUI to updatedSettings in handleSubmit so the preference persists when saving - Change <div onClick> to <button> for keyboard accessibility and proper semantics
There was a problem hiding this comment.
🧹 Nitpick comments (1)
webview-ui/src/components/chat/ChatRow.tsx (1)
1423-1434: ⚡ Quick winAdd
aria-expandedattribute for screen reader support.The button correctly uses semantic HTML and supports keyboard interaction, but adding
aria-expanded={isExpanded}would help screen readers announce whether the compact tool row is currently expanded or collapsed.♿ Proposed accessibility enhancement
<button type="button" onClick={handleToggleExpand} + aria-expanded={isExpanded} className="flex items-center gap-2 py-0.5 cursor-pointer text-vscode-descriptionForeground hover:text-vscode-foreground bg-transparent border-none text-inherit w-full text-left" data-testid="compact-tool-row" title={t("chat:compactTool.expandHint")}>🤖 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 `@webview-ui/src/components/chat/ChatRow.tsx` around lines 1423 - 1434, The compact tool toggle button in ChatRow.tsx lacks an aria-expanded attribute; update the button rendered alongside <ChevronRight /> and <PocketKnife /> (the element that calls handleToggleExpand and uses compactLabel) to include aria-expanded={isExpanded} so screen readers can detect expanded/collapsed state, keeping the existing props like data-testid and title unchanged.
🤖 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.
Nitpick comments:
In `@webview-ui/src/components/chat/ChatRow.tsx`:
- Around line 1423-1434: The compact tool toggle button in ChatRow.tsx lacks an
aria-expanded attribute; update the button rendered alongside <ChevronRight />
and <PocketKnife /> (the element that calls handleToggleExpand and uses
compactLabel) to include aria-expanded={isExpanded} so screen readers can detect
expanded/collapsed state, keeping the existing props like data-testid and title
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0afb8605-c6b1-4de5-8573-86a528b7da68
📒 Files selected for processing (2)
webview-ui/src/components/chat/ChatRow.tsxwebview-ui/src/components/settings/SettingsView.tsx
getState() read compactToolUI from storage but never returned it, so getStateToPostToWebview() never sent the persisted value to the webview and the `false` default always won on reload. Wire it through both getState() returns following the showRooIgnoredFiles pattern. Addresses PR Zoo-Code-Org#327 review (edelauna).
|
Thanks for the review @edelauna!
|
edelauna
left a comment
There was a problem hiding this comment.
Ok - lets just update the PR description, we can handle MCP history compaction in another issue/pr.
Had some minor questions.
| onClick={handleToggleExpand} | ||
| className="flex items-center gap-2 py-0.5 cursor-pointer text-vscode-descriptionForeground hover:text-vscode-foreground bg-transparent border-none text-inherit w-full text-left" | ||
| data-testid="compact-tool-row" | ||
| title={t("chat:compactTool.expandHint")}> |
There was a problem hiding this comment.
Would adding aria-expanded={false} here round out the a11y fix? Since this button only renders in the collapsed state, the value is always false, but it signals to screen readers that this is an expandable control.
| title={t("chat:compactTool.expandHint")}> | |
| title={t("chat:compactTool.expandHint")} | |
| aria-expanded={false}> |
| await waitFor(() => { | ||
| expect(setCachedStateField).toHaveBeenCalledWith("compactToolUI", true) | ||
| }) | ||
| }) |
There was a problem hiding this comment.
These cover the settings toggle nicely. Would it be worth also adding a test that mounts ChatRowContent with compactToolUI=true and a say tool message, to verify the compact row renders (and that clicking it calls onToggleExpand)? That would give a regression guard on the actual chat-side behaviour.
Related GitHub Issue
Closes #322
Description
Adds an opt-in Compact tool display setting (
compactToolUI, off by default) that collapses executed (say) tool blocks in the chat history to a single, clickable line — hiding long descriptions and JSON payloads until the user clicks to expand.This directly addresses the issue's complaint: verbose tool blocks (especially MCP tools like firecrawl with long descriptions) push the actual conversation out of view and make history tedious to scroll.
Design decision: approval prompts (
asktool messages) are never compacted — only completed/history (say) tool rows are — so users always see the parameters they are approving before acting.How it works
compactToolUIinglobalSettingsSchema+ExtensionState.compact-tool-ui-checkbox).ChatRow, thesaytool branch renders a single line (chevron + 🔧 + Tool: <name/path>) whencompactToolUI && !isExpanded; clicking toggles expansion and falls through to the normal full render.settings:contextManagement.compactToolUI.*,chat:compactTool.*) added across all 18 locales.Testing
ContextManagementSettings.spec.tsx(toggle renders + firessetCachedStateField("compactToolUI", …)). 28 passing.tsc -b(webview) andtsc --noEmit(src) clean; eslint clean;find-missing-translationsreports complete parity.Summary by CodeRabbit
New Features
Settings
Localization
Tests