feat(ui): refine tool cards and change review - #131
Conversation
Greptile SummaryThe PR streamlines tool-card styling and workspace metadata presentation while improving patch/review operation classification, rename display, scrolling, and initial expansion behavior.
Confidence Score: 4/5The missing workspace skill-diagnostics rendering should be fixed before merging because affected users lose the only visible explanation for skill-loading problems. open_workspace continues to emit skill diagnostics and treats them as expandable content, but the new structured payload renderer omits them and can display an empty-details message instead. Files Needing Attention: src/ui/workspace-app.tsx
|
| Filename | Overview |
|---|---|
| src/ui/workspace-app.tsx | Reworks card expansion and workspace payload rendering, but drops the still-produced skill diagnostics from the expanded UI. |
| src/ui/patch-display.ts | Centralizes file operation classification, rename-path formatting, and rendered diff/card correlation. |
| src/ui/review-payload.tsx | Adds immediate single-file rendering and richer multi-file operation and rename headers. |
| src/apply-patch.ts | Correctly reports an Add File directive that overwrites an existing file as an update. |
| src/ui/card-types.ts | Adds typed review operations and initial-expansion policy for review and single-file patch cards. |
| src/ui/workspace-app.css | Refines card sizing, semantic accents, workspace disclosures, review rows, and responsive styling. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Tool result card] --> B{Tool type}
B -->|apply_patch / show_changes| C[Parse diff files]
C --> D[Classify operation and paths]
D --> E[Render single diff or expandable file rows]
B -->|open_workspace| F[Render structured metadata rows]
F --> G[Instructions and previews]
F --> H[Skills, providers, and agents]
Reviews (1): Last reviewed commit: "feat(ui): open single-file patch diffs" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const agentNames = (card.agents ?? []).map((agent) => { | ||
| const name = agent.name ?? "Unnamed agent"; | ||
| return agent.provider ? `${name} · ${agent.provider}` : name; | ||
| }); | ||
| if (agentNames.length > 0) { | ||
| appendWorkspaceTextListRow( | ||
| rows, | ||
| "Agents", | ||
| agentNames, | ||
| toolIcons.agents, | ||
| "agents", | ||
| ); | ||
| } |
There was a problem hiding this comment.
When open_workspace returns skill diagnostics, this renderer ignores them even though they make the card expandable, causing skill-loading errors to disappear and diagnostics-only cards to report “No workspace details available.”
There was a problem hiding this comment.
no need to show it to user, it's pretty much internal details
There was a problem hiding this comment.
Got it — keeping diagnostics as internal-only makes sense. I'll drop that concern.
Shall I remember this as a rule: "skill diagnostics from open_workspace are internal details and should not be surfaced in the UI"?
Tip: You can customize Greptile's behavior for this repo with .greptile/rules.md and .greptile/config.json.
|
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:
📝 WalkthroughWalkthroughThe change aligns patch result classification with file-change display, adds initial card expansion rules, replaces plain-text workspace payloads with structured rendering, and updates icons, scrollbars, accessibility, responsive layouts, and card styling. ChangesPatch and workspace UI
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ToolDisplay
participant WorkspaceApp
participant DOM
ToolDisplay->>WorkspaceApp: identify open_workspace payload
WorkspaceApp->>DOM: render structured workspace details
WorkspaceApp->>DOM: preserve or reset disclosure and preview state
Possibly related PRs
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: 6
🧹 Nitpick comments (5)
src/ui/tool-display.ts (1)
112-116: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCollapse the duplicated ternary branch.
Both the
fileCount > 0branch and thecard.payload?.patchbranch returndisplay.title. The nested conditional therefore reduces to a single boolean test.♻️ Proposed simplification
- title: fileCount > 0 - ? display.title - : card.payload?.patch - ? display.title - : "No changes", + title: fileCount > 0 || card.payload?.patch + ? display.title + : "No changes",🤖 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.ts` around lines 112 - 116, In the title expression within the tool display construction, collapse the nested ternary so display.title is selected when either fileCount > 0 or card.payload?.patch is truthy; otherwise return "No changes".src/ui/workspace-app.tsx (2)
532-544: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSurface agent availability and model data instead of dropping it.
agentNameskeeps onlynameandprovider. The card also carriesmodel,thinking,providerAvailable, andproviderUnavailableReason(seesrc/ui/card-types.tsLines 70-79). The rendered workspace details discard all four.This is inconsistent inside the same renderer. For providers at Lines 518-530 you preserve
reasonas the chip title and mark unavailable entries with themutedtone. An agent whose provider is unavailable renders identically to a usable agent, so the user cannot tell which agents they can actually run.Consider rendering agents as chips with the same tone and title treatment used for providers.
♻️ Proposed change to preserve agent availability
- const agentNames = (card.agents ?? []).map((agent) => { - const name = agent.name ?? "Unnamed agent"; - return agent.provider ? `${name} · ${agent.provider}` : name; - }); - if (agentNames.length > 0) { - appendWorkspaceTextListRow( - rows, - "Agents", - agentNames, - toolIcons.agents, - "agents", - ); - } + const agents = card.agents ?? []; + if (agents.length > 0) { + const agentChips: WorkspaceChip[] = agents.map((agent) => { + const name = agent.name ?? "Unnamed agent"; + const unavailable = agent.providerAvailable === false; + const details = [agent.provider, agent.model].filter(Boolean).join(" · "); + return { + label: details ? `${name} · ${details}` : name, + tone: unavailable ? "muted" : undefined, + title: unavailable + ? agent.providerUnavailableReason ?? "Provider unavailable" + : undefined, + }; + }); + appendWorkspaceChipRow(rows, "Agents", agentChips, toolIcons.agents); + }Based on learnings from the coding guidelines: "Preserve host and provider data unless DevSpace has a concrete reason to normalize it, and add compatibility behavior only for an identified consumer with a real upgrade path."
🤖 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 532 - 544, Update the agent rendering in the workspace details renderer to preserve each agent’s model, thinking, providerAvailable, and providerUnavailableReason data instead of reducing agents to plain name strings. Render agents as chips using the same availability tone and reason title behavior as the provider rendering near the existing provider logic, while retaining the current name/provider display and Agents row behavior.Source: Coding guidelines
862-902: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared disclosure row builder.
appendWorkspaceSkills(Lines 875-901),appendWorkspaceTextListRow(Lines 802-835), andappendWorkspaceInstructions(Lines 595-632) each build the same disclosure scaffolding: theworkspace-row workspace-row-disclosureclass with a conditionalexpandedsuffix, the matchingworkspace-disclosurespan, a toggle that flips both classes, and the same add/delete calls againstexpandedWorkspaceDisclosures.Three copies of this logic will drift. Extract one helper that accepts the label, the icon, the disclosure key, the content element, the item total, and an optional extra class name.
🤖 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 862 - 902, Extract the duplicated disclosure-row construction from appendWorkspaceSkills, appendWorkspaceTextListRow, and appendWorkspaceInstructions into one shared helper. Have it accept the label, icon, disclosure key, content element, item total, and optional extra class name, while preserving the existing expanded-state class toggling and expandedWorkspaceDisclosures add/delete behavior. Replace each local scaffold with calls to the helper, retaining each row’s existing content and styling.src/ui/heavy-payload.tsx (1)
152-169: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShare one diff options builder with
review-payload.tsx.This options object now matches
diffOptionsinsrc/ui/review-payload.tsxLines 196-212 on eleven fields, including the four this PR added here:unsafeCSS,collapsedContextThreshold,expansionLineCount, anddisableFileHeader. The only difference isstickyHeader, which istruehere andfalsethere while both setdisableFileHeader: true.Extract a shared builder that takes
themeTypeand thestickyHeadervalue. The two diff surfaces then cannot drift apart.🤖 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/heavy-payload.tsx` around lines 152 - 169, Extract the duplicated diff options object into a shared builder accepting themeType and stickyHeader, preserving all existing option values. Update the diff configuration in heavy-payload.tsx and review-payload.tsx to use this builder with their respective stickyHeader values, so both surfaces share one source of truth.src/ui/scrollbar.ts (1)
10-41: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd the standard scrollbar properties for non-WebKit browsers.
The whole block sits inside
@supports selector(::-webkit-scrollbar). Firefox does not support that pseudo-element, so it evaluates to false and Firefox users get the default scrollbar. The standardscrollbar-widthandscrollbar-colorproperties cover Firefox and can sit outside the feature query.🎨 Proposed addition
[data-code] { scrollbar-gutter: auto; + scrollbar-width: thin; + scrollbar-color: var(--scrollbar-thumb, rgb(128 128 128 / 55%)) transparent; }🤖 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/scrollbar.ts` around lines 10 - 41, Update the scrollbar styling around the [data-code] rules to add standard scrollbar-width and scrollbar-color declarations outside the WebKit feature query, using the existing thumb and transparent track colors. Keep the existing `@supports` selector(::-webkit-scrollbar) block unchanged for WebKit-specific styling.
🤖 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/patch-display.ts`:
- Around line 145-149: Update the matched-file selection near matchedFile and
cardFile to prefer indexedFile when its path matches parsedFile.path, using the
existing files.find path match only as a fallback. Preserve the current
previousPath matching behavior for fallback lookup, and add a test covering two
moves with the same destination path to verify the indexed row is displayed.
In `@src/ui/review-payload.tsx`:
- Around line 118-143: Update the renamed-file header in the review payload
rendering to expose its path text: add an appropriate naming role to the
file-kind badge span, and remove the aria-hidden attributes from the previous,
arrow, and current path spans so the renamed path remains readable to assistive
technology. Use the existing role="img" pattern from
renderWorkspaceInstructionStatus, while preserving the visual structure and
labels.
In `@src/ui/workspace-app.css`:
- Around line 178-207: Replace the shared --font-text-sm-size usage across the
affected typography roles with distinct sizing tokens or fixed values: preserve
the 14px tool-title size, use the appropriate smaller token for .tool-label and
.stats, and separately maintain the 13px, 11px, and 10px sizes used by
.review-diff-file-name, .workspace-chip, .workspace-instruction-preview, and
.workspace-instruction-path. Ensure host overrides cannot collapse these roles
into one size.
- Around line 615-616: Replace the deprecated word-break: break-word declaration
in the affected style block with overflow-wrap: break-word while preserving
white-space: pre-wrap. Also update the matching deprecated declaration in
.text-payload for consistency.
- Around line 11-12: Update the --tool-accent-soft definition in the tone
override rules so it is recomputed from each tone’s --tool-accent value, rather
than remaining fixed from :root. Preserve the existing color-mix behavior and
ensure .tool-icon.color receives the corresponding semantic tint for every tone.
In `@src/ui/workspace-app.tsx`:
- Around line 574-588: Update the loaded-file handling in the workspace
instruction rendering flow to preserve an undefined path instead of defaulting
to “AGENTS.md”; use a neutral display label for pathless files without
presenting it as a host-reported path. Change preview identity and
synchronization in the relevant instruction rendering and
syncWorkspaceInstructionPreviews logic to use each file’s index, including
dataset.instructionPath and matching, so multiple pathless files remain
distinct.
---
Nitpick comments:
In `@src/ui/heavy-payload.tsx`:
- Around line 152-169: Extract the duplicated diff options object into a shared
builder accepting themeType and stickyHeader, preserving all existing option
values. Update the diff configuration in heavy-payload.tsx and
review-payload.tsx to use this builder with their respective stickyHeader
values, so both surfaces share one source of truth.
In `@src/ui/scrollbar.ts`:
- Around line 10-41: Update the scrollbar styling around the [data-code] rules
to add standard scrollbar-width and scrollbar-color declarations outside the
WebKit feature query, using the existing thumb and transparent track colors.
Keep the existing `@supports` selector(::-webkit-scrollbar) block unchanged for
WebKit-specific styling.
In `@src/ui/tool-display.ts`:
- Around line 112-116: In the title expression within the tool display
construction, collapse the nested ternary so display.title is selected when
either fileCount > 0 or card.payload?.patch is truthy; otherwise return "No
changes".
In `@src/ui/workspace-app.tsx`:
- Around line 532-544: Update the agent rendering in the workspace details
renderer to preserve each agent’s model, thinking, providerAvailable, and
providerUnavailableReason data instead of reducing agents to plain name strings.
Render agents as chips using the same availability tone and reason title
behavior as the provider rendering near the existing provider logic, while
retaining the current name/provider display and Agents row behavior.
- Around line 862-902: Extract the duplicated disclosure-row construction from
appendWorkspaceSkills, appendWorkspaceTextListRow, and
appendWorkspaceInstructions into one shared helper. Have it accept the label,
icon, disclosure key, content element, item total, and optional extra class
name, while preserving the existing expanded-state class toggling and
expandedWorkspaceDisclosures add/delete behavior. Replace each local scaffold
with calls to the helper, retaining each row’s existing content and styling.
🪄 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: 8cdbd34e-eaaa-4f58-a252-86754724cb48
📒 Files selected for processing (14)
src/apply-patch.test.tssrc/apply-patch.tssrc/ui/card-types.test.tssrc/ui/card-types.tssrc/ui/heavy-payload.tsxsrc/ui/icons.tssrc/ui/patch-display.test.tssrc/ui/patch-display.tssrc/ui/review-payload.tsxsrc/ui/scrollbar.tssrc/ui/tool-display.test.tssrc/ui/tool-display.tssrc/ui/workspace-app.csssrc/ui/workspace-app.tsx
[GPT-5.6-THINKING] RESPONDING ON BEHALF OF WAISHNAVFollow-up on the review items that were grouped in the review summary rather than attached to inline threads:
Verification on |
[GPT-5.6-THINKING] RESPONDING ON BEHALF OF WAISHNAVAddressed both follow-up cleanups in separate commits.
Verified with the full test suite, TypeScript typecheck, production build, and |
[GPT-5.6-THINKING] RESPONDING ON BEHALF OF WAISHNAVFinal cleanup pushed in Verified with the full test suite, production build, and |
1 participant
Tool cards had become visually heavy and inconsistent, while workspace metadata and file-change details were harder to scan than they needed to be. This refines the card shell, spacing, semantic accents, payload scrolling, and workspace detail layout, including expandable skills and instruction files with inline previews.
Patch and review cards now classify file operations from the actual diff, keep repeated-path operations distinct, show rename source and destination paths, align multi-file rows, and open a single-file apply_patch diff immediately without expanding large multi-file patches by default.
Screenshots
Summary by CodeRabbit
New Features
Bug Fixes
Style