feat(ui): overhaul tool result cards - #130
Conversation
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (16)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Greptile SummaryThe PR overhauls tool-result cards to provide operation-specific presentation, immediate single-file mutation and review diffs, structured workspace disclosures, and less intrusive scrollbar styling.
Confidence Score: 5/5The PR appears safe to merge, with no concrete changed-code failure identified. Current result producers satisfy the revised card projection and identity contracts, while the new expansion, disclosure, review, and scrollbar behavior remains consistent with the supplied card data.
|
| Filename | Overview |
|---|---|
| src/ui/workspace-app.tsx | Introduces automatic card expansion, structured workspace disclosures, card-state styling, and explicit UI-card projection without an established defect. |
| src/ui/review-payload.tsx | Auto-opens single-file reviews and adds operation-aware multi-file headers while retaining expandable file diffs. |
| src/ui/card-types.ts | Refines workspace expandability and centralizes automatic expansion behavior for reviews and single-file mutations. |
| src/server.ts | Removes model-only workspace diagnostics and instructions from the user-facing card projection. |
| src/ui/workspace-app.css | Implements the new card hierarchy, operation tones, disclosure layout, focus states, and neutral WebKit scrollbars. |
| src/ui/tool-display.ts | Updates workspace identity cues, review file-count titles, and workspace metadata summaries. |
| src/ui/scrollbar-styles.ts | Adds shadow-root-compatible scrollbar styling for Pierre payload components. |
Reviews (1): Last reviewed commit: "feat(ui): overhaul tool result cards" | Re-trigger Greptile
📝 WalkthroughWalkthroughThe PR updates workspace card metadata and expansion rules, adds structured workspace disclosures, improves review file rendering, introduces shared scrollbar styles, and updates icons, styling, and test coverage. ChangesWorkspace and review UI
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant Server as server.ts
participant WorkspaceApp as workspace-app.tsx
participant DisclosureState as workspace-disclosures.ts
participant Browser as Workspace card DOM
Server->>WorkspaceApp: provide workspace metadata and structured content
WorkspaceApp->>WorkspaceApp: sanitize card data and build workspace sections
WorkspaceApp->>DisclosureState: toggle section or document
DisclosureState-->>WorkspaceApp: return updated open-key set
WorkspaceApp->>Browser: render workspace disclosures and card state
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/workspace-app.css`:
- Around line 514-517: Update the .workspace-disclosures CSS declaration block
and the additionally affected block around the corresponding later rules: insert
a blank line after --workspace-inline-padding, and replace every deprecated
word-break: break-word declaration with overflow-wrap: anywhere.
In `@src/ui/workspace-app.tsx`:
- Around line 676-689: Update workspaceCardIdentity and isSameWorkspaceCard to
use only workspaceId for open_workspace cards; remove root and path fallbacks.
Treat cards without a workspaceId as having no identity so disclosure and
document state reset rather than being reused across workspace instances.
🪄 Autofix
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: 2ab6b2f2-8c29-4316-8666-988932a72b5f
📒 Files selected for processing (16)
package.jsonsrc/server.test.tssrc/server.tssrc/ui/card-types.test.tssrc/ui/card-types.tssrc/ui/heavy-payload.tsxsrc/ui/icons.tssrc/ui/review-payload.tsxsrc/ui/scrollbar-styles.test.tssrc/ui/scrollbar-styles.tssrc/ui/tool-display.test.tssrc/ui/tool-display.tssrc/ui/workspace-app.csssrc/ui/workspace-app.tsxsrc/ui/workspace-disclosures.test.tssrc/ui/workspace-disclosures.ts
💤 Files with no reviewable changes (1)
- src/server.ts
| .workspace-disclosures { | ||
| --workspace-inline-padding: 14px; | ||
| display: grid; | ||
| padding: 0 0 6px; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Fix the reported Stylelint errors.
Add an empty line between --workspace-inline-padding and display. Replace deprecated word-break: break-word with overflow-wrap: anywhere.
Proposed fix
.workspace-disclosures {
--workspace-inline-padding: 14px;
+
display: grid;
padding: 0 0 6px;
}
.workspace-document {
@@
- word-break: break-word;
+ overflow-wrap: anywhere;
}Also applies to: 678-689
🧰 Tools
🪛 Stylelint (17.14.0)
[error] 516-516: Expected empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
🤖 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/workspace-app.css` around lines 514 - 517, Update the
.workspace-disclosures CSS declaration block and the additionally affected block
around the corresponding later rules: insert a blank line after
--workspace-inline-padding, and replace every deprecated word-break: break-word
declaration with overflow-wrap: anywhere.
Source: Linters/SAST tools
| function workspaceCardIdentity( | ||
| value: { tool?: ToolName; workspaceId?: string; root?: string; path?: string } | null, | ||
| ): string | undefined { | ||
| if (value?.tool !== "open_workspace") return undefined; | ||
| return value.workspaceId ?? value.root ?? value.path; | ||
| } | ||
|
|
||
| function formatAgentsFilesForPayload( | ||
| agentsFiles: NonNullable<ToolResultCard["agentsFiles"]>, | ||
| ): string { | ||
| return agentsFiles | ||
| .map((file) => { | ||
| const path = file.path ?? "AGENTS.md"; | ||
| const content = file.content?.trim(); | ||
| return content ? `${path}\n\n${content}` : `${path}\n\nNo content loaded.`; | ||
| }) | ||
| .join("\n\n"); | ||
| function isSameWorkspaceCard( | ||
| previous: { tool?: ToolName; workspaceId?: string; root?: string; path?: string } | null, | ||
| next: { tool?: ToolName; workspaceId?: string; root?: string; path?: string }, | ||
| ): boolean { | ||
| const previousIdentity = workspaceCardIdentity(previous); | ||
| const nextIdentity = workspaceCardIdentity(next); | ||
| return previousIdentity !== undefined && previousIdentity === nextIdentity; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Use workspaceId as the only workspace identity.
Line 680 falls back to root and path. These values identify filesystem locations, not workspace instances. Two workspace instances at the same location can retain each other’s disclosure and document state.
Require open_workspace cards to carry workspaceId. Compare only that opaque value. Reset the disclosure state when it is absent.
As per coding guidelines, use workspaceId as the opaque handle returned by open_workspace and do not conflate workspaces, allowed roots, checkouts, or worktrees.
Proposed local guard
function workspaceCardIdentity(
value: { tool?: ToolName; workspaceId?: string; root?: string; path?: string } | null,
): string | undefined {
if (value?.tool !== "open_workspace") return undefined;
- return value.workspaceId ?? value.root ?? value.path;
+ return value.workspaceId;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function workspaceCardIdentity( | |
| value: { tool?: ToolName; workspaceId?: string; root?: string; path?: string } | null, | |
| ): string | undefined { | |
| if (value?.tool !== "open_workspace") return undefined; | |
| return value.workspaceId ?? value.root ?? value.path; | |
| } | |
| function formatAgentsFilesForPayload( | |
| agentsFiles: NonNullable<ToolResultCard["agentsFiles"]>, | |
| ): string { | |
| return agentsFiles | |
| .map((file) => { | |
| const path = file.path ?? "AGENTS.md"; | |
| const content = file.content?.trim(); | |
| return content ? `${path}\n\n${content}` : `${path}\n\nNo content loaded.`; | |
| }) | |
| .join("\n\n"); | |
| function isSameWorkspaceCard( | |
| previous: { tool?: ToolName; workspaceId?: string; root?: string; path?: string } | null, | |
| next: { tool?: ToolName; workspaceId?: string; root?: string; path?: string }, | |
| ): boolean { | |
| const previousIdentity = workspaceCardIdentity(previous); | |
| const nextIdentity = workspaceCardIdentity(next); | |
| return previousIdentity !== undefined && previousIdentity === nextIdentity; | |
| function workspaceCardIdentity( | |
| value: { tool?: ToolName; workspaceId?: string; root?: string; path?: string } | null, | |
| ): string | undefined { | |
| if (value?.tool !== "open_workspace") return undefined; | |
| return value.workspaceId; | |
| } | |
| function isSameWorkspaceCard( | |
| previous: { tool?: ToolName; workspaceId?: string; root?: string; path?: string } | null, | |
| next: { tool?: ToolName; workspaceId?: string; root?: string; path?: string }, | |
| ): boolean { | |
| const previousIdentity = workspaceCardIdentity(previous); | |
| const nextIdentity = workspaceCardIdentity(next); | |
| return previousIdentity !== undefined && previousIdentity === nextIdentity; |
🤖 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/workspace-app.tsx` around lines 676 - 689, Update
workspaceCardIdentity and isSameWorkspaceCard to use only workspaceId for
open_workspace cards; remove root and path fallbacks. Treat cards without a
workspaceId as having no identity so disclosure and document state reset rather
than being reused across workspace instances.
Source: Coding guidelines
1 participant
DevSpace tool cards were showing repeated metadata, inconsistent spacing, and intrusive scrollbars. Workspace cards also surfaced model-only diagnostics, while single-file mutations required extra clicks before users could see their diff.\n\nThis refresh gives cards an operation-specific visual hierarchy, controlled workspace disclosures, neutral WebKit scrollbar styling, and immediate single-file mutation diffs. Multi-file reviews retain per-file expansion, while the single-file review path drops the redundant file header so the diff begins directly under the card summary.
Screenshots
Summary by CodeRabbit
New Features
Improvements