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 5 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 Walkthrough(Note: the hidden artifact above is machine-parsed; do not edit.)
|
| Layer / File(s) | Summary |
|---|---|
Schema, types, provider and context state packages/types/src/global-settings.ts, packages/types/src/vscode-extension-host.ts, webview-ui/src/context/ExtensionStateContext.tsx, src/core/webview/ClineProvider.ts |
Adds compactToolUI to globalSettingsSchema and ExtensionState, initializes compactToolUI: false in ExtensionStateContext provider, and wires compactToolUI into ClineProvider state posted to the webview. |
Settings UI and state wiring webview-ui/src/components/settings/ContextManagementSettings.tsx, webview-ui/src/components/settings/SettingsView.tsx |
ContextManagementSettings accepts a compactToolUI prop, renders a checkbox that calls setCachedStateField("compactToolUI", ...); SettingsView reads compactToolUI from cachedState, includes it in update payloads, and forwards it to ContextManagementSettings. |
Chat row compact rendering webview-ui/src/components/chat/ChatRow.tsx |
ChatRow imports ChevronRight, reads compactToolUI from extension state, and returns an early single-line clickable summary (chevron + tool label) for executed say tool rows when compact mode is enabled and the row is collapsed. |
Tests and internationalization webview-ui/src/components/settings/__tests__/ContextManagementSettings.spec.tsx, webview-ui/src/i18n/locales/*/chat.json, webview-ui/src/i18n/locales/*/settings.json |
Adds unit tests verifying the compact-tool checkbox renders and toggles cached state; adds compactTool chat strings and contextManagement.compactToolUI settings translations across many locale files. |
Sequence Diagram
sequenceDiagram
participant User
participant SettingsView
participant ClineProvider
participant ExtensionStateContext
participant ChatRow
User->>SettingsView: toggle compactToolUI
SettingsView->>ClineProvider: set cached compactToolUI
ClineProvider->>ExtensionStateContext: post state including compactToolUI
ExtensionStateContext->>ChatRow: provide compactToolUI via context
ChatRow->>User: render compact single-line summary when enabled
User->>ChatRow: click to expand
ChatRow->>User: render full tool UI
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested reviewers
- taltas
- navedmerchant
- hannesrudolph
- JamesRobert20
Poem
🐰 I nibble code and tidy threads,
A chevron tucks away long spreads.
One slim line, a click to show—
Quiet chat, then details grow.
Hop, reveal, compact and neat.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title accurately summarizes the main change: adding a compact tool UI mode for executed tool blocks in chat. |
| Description check | ✅ Passed | The PR description covers the core implementation details, design decisions (not compacting approval prompts), testing approach, and validation results with good clarity. |
| Linked Issues check | ✅ Passed | The implementation fully addresses issue #322: adds opt-in global setting, collapses executed (say) tool blocks to single line, keeps approval (ask) prompts expanded, and provides expansion interaction. |
| Out of Scope Changes check | ✅ Passed | All changes are directly scoped to the compact tool UI feature: schema/state additions, settings UI toggle, chat row rendering logic, internationalization keys, and related tests. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
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 help to get the list of available commands and usage tips.
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.
|
Conflicts resolved and branch rebased on latest main. CI is passing. Ready for re-review. 🙏 |
Addresses CodeRabbit review nitpick: add aria-expanded={isExpanded}
to the compact tool row button for screen reader support.
Refs: PR Zoo-Code-Org#327 review feedback
|
Thanks for the review @edelauna! I've addressed the feedback: Changes made:
Regarding your comments:
The CI checks should pick up the new commit shortly. Let me know if there are any other questions or changes needed! |
…dd ChatRow compact tool tests
- Change aria-expanded={isExpanded} to aria-expanded={false} per edelauna review
(button only renders when !isExpanded, so value is always false)
- Add ChatRow.compact-tool.spec.tsx with 3 tests:
1. Renders compact row when compactToolUI=true and not expanded
2. Calls onToggleExpand when compact row is clicked
3. Has aria-expanded=false on the compact button
Addresses review feedback on PR Zoo-Code-Org#327
|
Updated per your review @edelauna:
All tests pass. Let me know if you'd like any further changes! |
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