Move sandbox run details to compact end-of-message card#1610
Move sandbox run details to compact end-of-message card#1610ivaavimusic wants to merge 3 commits intorecoupable:mainfrom
Conversation
|
@ivaavimusic is attempting to deploy a commit to the Recoupable Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis PR introduces compact UI components and utilities for displaying task run details in a chat context. It adds a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bad20d018e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| isStreaming={ | ||
| status === "streaming" && | ||
| partIndex === message.parts.length - 1 | ||
| status === "streaming" && partIndex === orderedParts.length - 1 | ||
| } |
There was a problem hiding this comment.
When deferred sandbox tool results are moved to the end, partIndex === orderedParts.length - 1 no longer identifies the actively streaming reasoning chunk in messages that continue with reasoning/text after a sandbox result. In that case EnhancedReasoning receives isStreaming=false while the assistant is still streaming, so shimmer/auto-collapse behavior can stop early and show reasoning as finalized before generation actually completes.
Useful? React with 👍 / 👎.
| const TERMINAL_STATUSES = new Set([ | ||
| "COMPLETED", | ||
| "FAILED", | ||
| "CRASHED", | ||
| "CANCELED", | ||
| "SYSTEM_FAILURE", | ||
| "INTERRUPTED", | ||
| ]); | ||
|
|
||
| const STATUS_BADGE_CLASSES: Record<string, string> = { | ||
| COMPLETED: | ||
| "border-green-200 bg-green-50 text-green-700 dark:border-green-900/60 dark:bg-green-950/50 dark:text-green-300", | ||
| FAILED: | ||
| "border-red-200 bg-red-50 text-red-700 dark:border-red-900/60 dark:bg-red-950/50 dark:text-red-300", | ||
| CRASHED: | ||
| "border-red-200 bg-red-50 text-red-700 dark:border-red-900/60 dark:bg-red-950/50 dark:text-red-300", | ||
| SYSTEM_FAILURE: | ||
| "border-red-200 bg-red-50 text-red-700 dark:border-red-900/60 dark:bg-red-950/50 dark:text-red-300", | ||
| INTERRUPTED: | ||
| "border-red-200 bg-red-50 text-red-700 dark:border-red-900/60 dark:bg-red-950/50 dark:text-red-300", | ||
| CANCELED: | ||
| "border-gray-200 bg-gray-50 text-gray-700 dark:border-gray-800 dark:bg-gray-950/50 dark:text-gray-300", | ||
| EXECUTING: | ||
| "border-yellow-200 bg-yellow-50 text-yellow-700 dark:border-yellow-900/60 dark:bg-yellow-950/50 dark:text-yellow-300", | ||
| REATTEMPTING: | ||
| "border-yellow-200 bg-yellow-50 text-yellow-700 dark:border-yellow-900/60 dark:bg-yellow-950/50 dark:text-yellow-300", | ||
| QUEUED: | ||
| "border-gray-200 bg-gray-50 text-gray-700 dark:border-gray-800 dark:bg-gray-950/50 dark:text-gray-300", | ||
| DELAYED: | ||
| "border-gray-200 bg-gray-50 text-gray-700 dark:border-gray-800 dark:bg-gray-950/50 dark:text-gray-300", | ||
| FROZEN: | ||
| "border-gray-200 bg-gray-50 text-gray-700 dark:border-gray-800 dark:bg-gray-950/50 dark:text-gray-300", | ||
| PENDING_VERSION: | ||
| "border-gray-200 bg-gray-50 text-gray-700 dark:border-gray-800 dark:bg-gray-950/50 dark:text-gray-300", | ||
| }; |
There was a problem hiding this comment.
Open Closed Principle
- actual: adding new const definitions to an existing RunDetails component.
- required: move new const definitions to new files.
There was a problem hiding this comment.
I added those constants directly while building the compact run view, but I agree they should be extracted instead of expanding RunDetails further. I’ll move them into dedicated files.
| ); | ||
|
|
||
| if (variant === "chat-compact") { | ||
| return ( |
There was a problem hiding this comment.
Open Closed Principle
- actual: new components defined inline of an existing component file.
- required: new component file for this new code.
There was a problem hiding this comment.
I agree those new pieces should be extracted into separate component files. I’ll split them out.
| import RunDetails from "@/components/TasksPage/Run/RunDetails"; | ||
| import RunPageSkeleton from "@/components/TasksPage/Run/RunPageSkeleton"; | ||
|
|
||
| function CompactRunSkeleton() { |
There was a problem hiding this comment.
Open Closed Principle
- actual: CompactRunSkeleton defined within RunSandboxCommandResultWithPolling
- required: new component file for CompactRunSkeleton
There was a problem hiding this comment.
Agreed. I’ll extract it into its own file.
| "prompt_sandbox", | ||
| ]); | ||
|
|
||
| function isDeferredSandboxResultPart( |
There was a problem hiding this comment.
Single Responsibility Principle
- actual: function isDeferredSandboxResultPart is defined within a component file.
- required: new standalone function file for isDeferredSandboxResultPart.
There was a problem hiding this comment.
Understood. That helper was added inline for the sandbox-result reordering, but I agree it should be extracted so MessageParts stays focused on rendering. I’ll move it into a standalone function file.
![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
🧹 Nitpick comments (5)
components/TasksPage/Run/CompactRunDetails.tsx (2)
5-5: Verify icon-library consistency for this component tier.If this component is considered part of the UI-component layer, replace
lucide-reacthere with@radix-ui/react-iconsto match the repo convention.As per coding guidelines, "Use
@radix-ui/react-iconsfor UI component icons (not Lucide React)".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/TasksPage/Run/CompactRunDetails.tsx` at line 5, This file imports ChevronDown from lucide-react which violates the repo guideline to use `@radix-ui/react-icons` for UI-component layer icons; replace the import of ChevronDown with the equivalent icon from `@radix-ui/react-icons` (or a suitable Radix icon name) in CompactRunDetails (and update any JSX references to that symbol if renamed), run a quick search in the component for ChevronDown usage to ensure the symbol name matches the new import, and update imports-only (no other logic changes) so the component consumes the Radix icon library consistently.
12-24: ExportCompactRunDetailsPropsfor consistency.The type name is good; exporting it would align with the project’s TS component API convention.
As per coding guidelines, "Export component prop types with explicit
ComponentNamePropsnaming convention".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/TasksPage/Run/CompactRunDetails.tsx` around lines 12 - 24, The CompactRunDetailsProps interface is currently unexported; update it to a named export so it follows the project's ComponentNameProps convention and can be reused externally—change the declaration of CompactRunDetailsProps to an exported interface (export interface CompactRunDetailsProps { ... }) in the CompactRunDetails.tsx file and keep the existing shape and name exactly the same.components/TasksPage/Run/RunDetails.tsx (2)
18-22: Export the props type for API clarity.
RunDetailsPropsalready uses the right naming; exporting it would align with the shared TS component contract pattern.As per coding guidelines, "Export component prop types with explicit
ComponentNamePropsnaming convention".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/TasksPage/Run/RunDetails.tsx` around lines 18 - 22, Export the RunDetailsProps interface so the prop type is available to other modules and matches the ComponentNameProps convention; update the declaration for RunDetailsProps to be exported (export interface RunDetailsProps { ... }) and ensure any usages/imports of the RunDetails component or its props (e.g., RunDetails) reference the exported RunDetailsProps type where needed.
36-50: Scope disclosure state to the compact variant path.
isOpenand theuseEffectrun even forvariant="full"where they are unused. Consider moving this state intoCompactRunDetails(or initializing only whenvariant === "chat-compact") to keep Line 36–50 variant-specific and reduce unnecessary state work.As per coding guidelines, "Create single-responsibility components with obvious data flow" and "Write minimal code - use only the absolute minimum code needed".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/TasksPage/Run/RunDetails.tsx` around lines 36 - 50, The open/close state and effect are being created in RunDetails even when variant !== "chat-compact"; move the isOpen useState and its useEffect into the compact-specific component (e.g., CompactRunDetails) or conditionally initialize them only when variant === "chat-compact" to avoid unnecessary state; specifically relocate/remove isOpen, setIsOpen, and the useEffect from RunDetails and implement the same logic inside CompactRunDetails (or wrap useState/useEffect in a guard checking variant) so the terminal-checking logic (isTerminal = TERMINAL_STATUSES.has(data.status)) and summaryText remain in RunDetails while compact-only UI state lives in the compact variant.components/VercelChat/tools/sandbox/CompactRunSkeleton.tsx (1)
3-12: Hide decorative skeleton blocks from screen readers.This skeleton is visual-only; consider marking the wrapper
aria-hidden="true"so assistive tech doesn’t read placeholder structure (Line 3).♻️ Suggested tweak
- <div className="w-full rounded-2xl border bg-background/80 px-4 py-3 shadow-sm"> + <div + aria-hidden="true" + className="w-full rounded-2xl border bg-background/80 px-4 py-3 shadow-sm" + >As per coding guidelines, "Provide proper ARIA roles/states and test with screen readers" and "Start with semantic HTML first, then augment with ARIA if needed".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/VercelChat/tools/sandbox/CompactRunSkeleton.tsx` around lines 3 - 12, The outer wrapper div in CompactRunSkeleton.tsx is purely decorative and should be hidden from assistive tech; update the top-level div (the one with className "w-full rounded-2xl border bg-background/80 px-4 py-3 shadow-sm") to include aria-hidden="true" so screen readers ignore the placeholder skeleton, and ensure none of the inner elements are focusable or contain interactive attributes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/TasksPage/Run/CompactRunDetails.tsx`:
- Around line 91-95: The external link rendering in the Link component (the JSX
element with href={`/tasks/${runId}`} and target="_blank`} in CompactRunDetails)
is missing the rel attribute; update that Link to include rel="noopener
noreferrer" to prevent tabnabbing and ensure safe external/new-tab behavior.
In `@components/VercelChat/MessageParts.tsx`:
- Around line 30-37: The render keys are unstable because getOrderedMessageParts
reorders originalParts but keys are derived from the rendered partIndex; change
getOrderedMessageParts to attach the original index (e.g., add originalIndex or
sourceIndex property to each returned part) and update all consumers (the render
loop and lastTextPartIndex computation) to use that preserved originalIndex as
the React key and for identity checks instead of the current partIndex; this
ensures components (e.g., expandable run card) keep stable identity when parts
are moved.
---
Nitpick comments:
In `@components/TasksPage/Run/CompactRunDetails.tsx`:
- Line 5: This file imports ChevronDown from lucide-react which violates the
repo guideline to use `@radix-ui/react-icons` for UI-component layer icons;
replace the import of ChevronDown with the equivalent icon from
`@radix-ui/react-icons` (or a suitable Radix icon name) in CompactRunDetails (and
update any JSX references to that symbol if renamed), run a quick search in the
component for ChevronDown usage to ensure the symbol name matches the new
import, and update imports-only (no other logic changes) so the component
consumes the Radix icon library consistently.
- Around line 12-24: The CompactRunDetailsProps interface is currently
unexported; update it to a named export so it follows the project's
ComponentNameProps convention and can be reused externally—change the
declaration of CompactRunDetailsProps to an exported interface (export interface
CompactRunDetailsProps { ... }) in the CompactRunDetails.tsx file and keep the
existing shape and name exactly the same.
In `@components/TasksPage/Run/RunDetails.tsx`:
- Around line 18-22: Export the RunDetailsProps interface so the prop type is
available to other modules and matches the ComponentNameProps convention; update
the declaration for RunDetailsProps to be exported (export interface
RunDetailsProps { ... }) and ensure any usages/imports of the RunDetails
component or its props (e.g., RunDetails) reference the exported RunDetailsProps
type where needed.
- Around line 36-50: The open/close state and effect are being created in
RunDetails even when variant !== "chat-compact"; move the isOpen useState and
its useEffect into the compact-specific component (e.g., CompactRunDetails) or
conditionally initialize them only when variant === "chat-compact" to avoid
unnecessary state; specifically relocate/remove isOpen, setIsOpen, and the
useEffect from RunDetails and implement the same logic inside CompactRunDetails
(or wrap useState/useEffect in a guard checking variant) so the
terminal-checking logic (isTerminal = TERMINAL_STATUSES.has(data.status)) and
summaryText remain in RunDetails while compact-only UI state lives in the
compact variant.
In `@components/VercelChat/tools/sandbox/CompactRunSkeleton.tsx`:
- Around line 3-12: The outer wrapper div in CompactRunSkeleton.tsx is purely
decorative and should be hidden from assistive tech; update the top-level div
(the one with className "w-full rounded-2xl border bg-background/80 px-4 py-3
shadow-sm") to include aria-hidden="true" so screen readers ignore the
placeholder skeleton, and ensure none of the inner elements are focusable or
contain interactive attributes.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: de087a20-f73a-4ddf-b35a-a0fda1c01d6a
📒 Files selected for processing (8)
components/TasksPage/Run/CompactRunDetails.tsxcomponents/TasksPage/Run/RunDetails.tsxcomponents/TasksPage/Run/runDetailsConstants.tscomponents/VercelChat/MessageParts.tsxcomponents/VercelChat/getOrderedMessageParts.tscomponents/VercelChat/isDeferredSandboxResultPart.tscomponents/VercelChat/tools/sandbox/CompactRunSkeleton.tsxcomponents/VercelChat/tools/sandbox/RunSandboxCommandResultWithPolling.tsx
| <Link | ||
| href={`/tasks/${runId}`} | ||
| target="_blank" | ||
| className="text-xs font-medium text-muted-foreground transition-colors hover:text-foreground" | ||
| > |
There was a problem hiding this comment.
Add rel for the new-tab link.
Line 93 opens a new tab but omits rel. Please add rel="noopener noreferrer" to harden against tabnabbing.
🔐 Suggested fix
<Link
href={`/tasks/${runId}`}
target="_blank"
+ rel="noopener noreferrer"
className="text-xs font-medium text-muted-foreground transition-colors hover:text-foreground"
>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/TasksPage/Run/CompactRunDetails.tsx` around lines 91 - 95, The
external link rendering in the Link component (the JSX element with
href={`/tasks/${runId}`} and target="_blank`} in CompactRunDetails) is missing
the rel attribute; update that Link to include rel="noopener noreferrer" to
prevent tabnabbing and ensure safe external/new-tab behavior.
| const originalParts = message.parts ?? []; | ||
| const orderedParts = getOrderedMessageParts(originalParts); | ||
| const lastOriginalPart = originalParts[originalParts.length - 1]; | ||
| const lastTextPartIndex = orderedParts.reduce( | ||
| (lastIndex, part, partIndex) => | ||
| part.type === "text" ? partIndex : lastIndex, | ||
| -1, | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that reordered parts are rendered from `orderedParts`
# while the row key still derives from the rendered index.
# Expected: one match for `getOrderedMessageParts(...)` and one match
# showing `message-${message.id}-part-${partIndex}`.
rg -n -C2 'getOrderedMessageParts|message-\$\{message\.id\}-part-\$\{partIndex\}' components/VercelChat/MessageParts.tsxRepository: recoupable/chat
Length of output: 799
🏁 Script executed:
# Check the implementation of getOrderedMessageParts
find . -name "*getOrderedMessageParts*" -type fRepository: recoupable/chat
Length of output: 108
🏁 Script executed:
# Also check the complete key derivation logic and map structure
rg -n "orderedParts\.map|getOrderedMessagePart" components/VercelChat/MessageParts.tsx -A 5Repository: recoupable/chat
Length of output: 822
🏁 Script executed:
cat components/VercelChat/getOrderedMessageParts.tsRepository: recoupable/chat
Length of output: 653
Use stable keys based on part identity, not rendered position.
The getOrderedMessageParts function reorders parts by moving deferred sandbox results to the end of the array. Since the key is derived from partIndex (the current rendered index), React will remount components when parts shift—resetting local UI state in stateful children like the expandable run card.
Preserve the original index through reordering and use it as the key source instead:
♻️ Suggested approach
+export interface OrderedMessagePart {
+ part: UIMessagePart<UIDataTypes, UITools>;
+ originalIndex: number;
+}
+
export function getOrderedMessageParts(
parts: UIMessagePart<UIDataTypes, UITools>[],
-) {
- const regularParts: UIMessagePart<UIDataTypes, UITools>[] = [];
- const deferredSandboxParts: UIMessagePart<UIDataTypes, UITools>[] = [];
+): OrderedMessagePart[] {
+ const regularParts: OrderedMessagePart[] = [];
+ const deferredSandboxParts: OrderedMessagePart[] = [];
- for (const part of parts) {
+ for (const [originalIndex, part] of parts.entries()) {
+ const orderedPart = { part, originalIndex };
if (isDeferredSandboxResultPart(part)) {
- deferredSandboxParts.push(part);
+ deferredSandboxParts.push(orderedPart);
continue;
}
- regularParts.push(part);
+ regularParts.push(orderedPart);
}
return [...regularParts, ...deferredSandboxParts];
}Then update the render loop:
- {orderedParts.map(
- (part: UIMessagePart<UIDataTypes, UITools>, partIndex) => {
+ {orderedParts.map(({ part, originalIndex }, partIndex) => {
const { type } = part;
- const key = `message-${message.id}-part-${partIndex}`;
+ const key = `message-${message.id}-part-${originalIndex}`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/VercelChat/MessageParts.tsx` around lines 30 - 37, The render keys
are unstable because getOrderedMessageParts reorders originalParts but keys are
derived from the rendered partIndex; change getOrderedMessageParts to attach the
original index (e.g., add originalIndex or sourceIndex property to each returned
part) and update all consumers (the render loop and lastTextPartIndex
computation) to use that preserved originalIndex as the React key and for
identity checks instead of the current partIndex; this ensures components (e.g.,
expandable run card) keep stable identity when parts are moved.
2 participants
Summary
This PR improves how sandbox task results appear inside chat responses.
Changes
Why
Inline sandbox details were easy to miss and visually heavy. This makes the run status easier to scan while still keeping full logs and error details available on demand.
Risk
Low to medium. The change is scoped to chat rendering for sandbox task results and does not alter backend task execution.
Summary by CodeRabbit
Release Notes
New Features
Refactor