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
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 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 |
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
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/ui/workspace-disclosures.test.ts (2)
9-16: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a positive case with different
rootandpathvalues.The current positive assertion uses the same
rootandpathon both cards. A regression that compares either field would still pass. Add an assertion with the sameworkspaceIdand differentrootandpathvalues, and expecttrue.Suggested regression assertion
+ assert.equal( + isSameWorkspaceCard( + { tool: "open_workspace", workspaceId: "ws_same", root: "/tmp/root-a", path: "/tmp/path-a" }, + { tool: "open_workspace", workspaceId: "ws_same", root: "/tmp/root-b", path: "/tmp/path-b" }, + ), + true, + );As per coding guidelines for
src/**/*.ts: “useworkspaceIdas the opaque handle returned byopen_workspace. Do not conflate workspaces, allowed roots, checkouts, or worktrees.”🤖 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-disclosures.test.ts` around lines 9 - 16, Add a positive assertion in the workspace card state test for isSameWorkspaceCard using identical workspaceId values but different root and path values, and expect true. Keep the test focused on workspaceId as the sole identity key without changing the existing assertion.Source: Coding guidelines
33-43: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert that both toggle helpers preserve the input
Set.The tests overwrite
openimmediately. A mutating implementation would pass. Keep the original set, assert that the returned set is a different object, and verify that the original contents remain unchanged.Suggested test change
let open = new Set<"instructions" | "skills" | "agents">(["skills"]); + const original = open; - open = toggleWorkspaceDisclosure(open, "instructions"); + const next = toggleWorkspaceDisclosure(open, "instructions"); + assert.notStrictEqual(next, original); + assert.deepEqual([...original], ["skills"]); + open = next;Apply the same pattern to
toggleWorkspaceDocument.The line-range change details describe both helpers as immutable toggles; these tests should enforce that contract.
Also applies to: 45-53
🤖 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-disclosures.test.ts` around lines 33 - 43, Update the tests for both toggleWorkspaceDisclosure and toggleWorkspaceDocument to retain each original Set before toggling, assert the returned Set is a different object, and verify the original Set contents remain unchanged. Preserve the existing assertions for the returned toggled contents while applying the same immutability checks to both helpers.
🤖 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 `@src/ui/workspace-disclosures.test.ts`:
- Around line 9-16: Add a positive assertion in the workspace card state test
for isSameWorkspaceCard using identical workspaceId values but different root
and path values, and expect true. Keep the test focused on workspaceId as the
sole identity key without changing the existing assertion.
- Around line 33-43: Update the tests for both toggleWorkspaceDisclosure and
toggleWorkspaceDocument to retain each original Set before toggling, assert the
returned Set is a different object, and verify the original Set contents remain
unchanged. Preserve the existing assertions for the returned toggled contents
while applying the same immutability checks to both helpers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: df795041-714f-4def-94ca-dc8867fcb411
📒 Files selected for processing (4)
src/ui/workspace-app.csssrc/ui/workspace-app.tsxsrc/ui/workspace-disclosures.test.tssrc/ui/workspace-disclosures.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/ui/workspace-app.css
- src/ui/workspace-app.tsx
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