Align tool cards with Codex UI - #73
Conversation
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThis change centralizes tool-card icons and display metadata, updates patch and workspace summaries, refactors review-card rendering, adds related tests, and revises tool-card styling with responsive adjustments. ChangesTool-card display refresh
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Server
participant WorkspaceApp
participant ToolDisplay
participant IconRenderer
Server->>WorkspaceApp: tool result card and metadata summary
WorkspaceApp->>ToolDisplay: getToolDisplay(card)
ToolDisplay->>IconRenderer: renderIcon(display.icon)
IconRenderer-->>WorkspaceApp: SVG icon element
WorkspaceApp->>WorkspaceApp: renderHeaderSummary(card) and review payload
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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: 3
🧹 Nitpick comments (1)
src/ui/tool-display.test.ts (1)
1-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd test coverage for remaining tool types and edge cases.
The current tests cover
show_changes,exec_command,write_stdin, andopen_workspaceheader summaries, but several tool display paths are untested:read,write,edit,apply_patch,grep,glob,ls,bash, andopen_workspacedisplay. Header summary branches for shell, search, read/ls, and the empty fallback are also uncovered. Adding a few more assertions following the existing pattern would prevent regressions in title/tone/label formatting.🤖 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 `@src/ui/tool-display.test.ts` around lines 1 - 57, Extend the tests in the existing tool-display assertions to cover display results for read, write, edit, apply_patch, grep, glob, ls, bash, and open_workspace, including their expected titles and tones. Add header-summary assertions for shell, search, read/ls, and the empty fallback branches of getToolHeaderSummary. Follow the existing assert.deepEqual/assert.equal pattern and use pickDisplay where appropriate.
🤖 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 `@src/ui/tool-display.ts`:
- Around line 150-155: Update patchIcon so the "delete" operation uses a
delete-specific icon instead of toolIcons.readFile. Add the chosen delete/trash
icon to toolIcons and reference it from patchIcon, while preserving the existing
icons for add, move, and other operations.
- Around line 72-80: Update the “glob” case in the tool display summary logic to
format summary.lines as a file-count label, matching the existing “Found N
files” title and avoiding the generic line-count formatting used by other search
tools. Preserve the current fallback behavior when the count is undefined.
In `@src/ui/workspace-app.css`:
- Around line 351-368: Update the mobile breakpoint rules for `.tool-header` and
`.review-header` so their `.chevron` element fits the 18px final grid track at
widths up to 520px. Add a mobile-specific size override matching that track
while preserving the existing `.tool-icon` behavior and header layout.
---
Nitpick comments:
In `@src/ui/tool-display.test.ts`:
- Around line 1-57: Extend the tests in the existing tool-display assertions to
cover display results for read, write, edit, apply_patch, grep, glob, ls, bash,
and open_workspace, including their expected titles and tones. Add
header-summary assertions for shell, search, read/ls, and the empty fallback
branches of getToolHeaderSummary. Follow the existing
assert.deepEqual/assert.equal pattern and use pickDisplay where appropriate.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a74f1576-e8f6-42c9-810e-cd6b0f6d2bc0
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
package.jsonsrc/server.tssrc/ui/icons.tssrc/ui/patch-display.test.tssrc/ui/patch-display.tssrc/ui/tool-display.test.tssrc/ui/tool-display.tssrc/ui/workspace-app.csssrc/ui/workspace-app.tsx
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Waishnav
commented
Jul 13, 2026
Also addressed the test-coverage nitpick in |
Uh oh!
There was an error while loading. Please reload this page.
Summary
Validation
npm testnpm run typechecknpm run buildSummary by CodeRabbit