Improve display of agent/tool cards in chat session#1360
Improve display of agent/tool cards in chat session#1360dobesv wants to merge 1 commit intokagent-dev:mainfrom
Conversation
dobesv
commented
Feb 23, 2026
- Parse JSON payloads and render basic tree view
- Render strings as markdown
- Copy to clipboard button copies original text
- Add view source button in case of render/parse issues
- Prevent ScrollArea from overflow horizontally
- Parse JSON payloads and render basic tree view - Render strings as markdown - Copy to clipboard button copies original text - Add view source button in case of render/parse issues - Prevent ScrollArea from overflow horizontally
There was a problem hiding this comment.
Pull request overview
This PR improves the display of agent and tool call cards in chat sessions by introducing smart content rendering with JSON parsing, markdown support, and better UI controls.
Changes:
- Added SmartContent component that intelligently renders content as JSON tree view, markdown, or plain text
- Added CollapsibleSection component to provide consistent expand/collapse behavior with preview
- Modified ScrollArea to prevent horizontal overflow with CSS override
- Added copy-to-clipboard functionality to chat messages and improved display of tool/agent call arguments and results
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/src/components/chat/SmartContent.tsx | New component for intelligent content rendering with JSON parsing, markdown support, type icons, and copy/view source controls |
| ui/src/components/chat/CollapsibleSection.tsx | New reusable component for collapsible content sections with preview and scroll support |
| ui/src/components/ui/scroll-area.tsx | Added CSS override to force block display on direct child divs to prevent horizontal overflow |
| ui/src/components/chat/ChatMessage.tsx | Added copy-to-clipboard button for chat messages and refactored feedback button visibility |
| ui/src/components/chat/AgentCallDisplay.tsx | Integrated SmartContent for rendering args/results, added clickable link for function calls, replaced custom collapse logic with CollapsibleSection |
| ui/src/components/ToolDisplay.tsx | Integrated SmartContent for rendering args/results, removed custom copy button, replaced custom collapse logic with CollapsibleSection |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <button | ||
| type="button" | ||
| onClick={onToggle} | ||
| className="flex justify-center w-full pt-0.5 text-muted-foreground cursor-pointer hover:text-foreground transition-colors" | ||
| > | ||
| <ChevronUp className="w-3.5 h-3.5" /> | ||
| </button> |
There was a problem hiding this comment.
The chevron buttons for expanding/collapsing sections lack aria-label attributes. While the visual chevron icons indicate their purpose, screen reader users would benefit from descriptive labels like "Expand section" or "Collapse section". This would improve accessibility for users relying on assistive technologies.
| @@ -0,0 +1,65 @@ | |||
| import React from "react"; | |||
| import { ChevronUp, ChevronDown } from "lucide-react"; | |||
| import { ScrollArea } from "@radix-ui/react-scroll-area"; | |||
There was a problem hiding this comment.
The ScrollArea import is inconsistent with the rest of the codebase. All other components import ScrollArea from "@/components/ui/scroll-area" (the local wrapper component), not from "@radix-ui/react-scroll-area" directly. This should be updated to match the pattern used in ChatInterface.tsx, LLMCallModal.tsx, SelectToolsDialog.tsx, and other components.
| import { ScrollArea } from "@radix-ui/react-scroll-area"; | |
| import { ScrollArea } from "@/components/ui/scroll-area"; |
| <Button variant="ghost" size="icon" className="h-5 w-5" onClick={handleToggleSource} title={viewSource ? "Formatted view" : "View source"}> | ||
| {viewSource ? <Eye className="w-3 h-3" /> : <Code className="w-3 h-3" />} | ||
| </Button> | ||
| <Button variant="ghost" size="icon" className="h-5 w-5" onClick={handleCopy} title={copied ? "Copied!" : "Copy to clipboard"}> |
There was a problem hiding this comment.
The icon-only buttons (Copy, View Source/Eye) in SmartContent have title attributes for hover tooltips but lack aria-label attributes. While the title provides some accessibility, aria-label is more appropriate for screen readers. Consider adding aria-label to complement or replace title for better accessibility.
| <Button variant="ghost" size="icon" className="h-5 w-5" onClick={handleToggleSource} title={viewSource ? "Formatted view" : "View source"}> | |
| {viewSource ? <Eye className="w-3 h-3" /> : <Code className="w-3 h-3" />} | |
| </Button> | |
| <Button variant="ghost" size="icon" className="h-5 w-5" onClick={handleCopy} title={copied ? "Copied!" : "Copy to clipboard"}> | |
| <Button | |
| variant="ghost" | |
| size="icon" | |
| className="h-5 w-5" | |
| onClick={handleToggleSource} | |
| title={viewSource ? "Formatted view" : "View source"} | |
| aria-label={viewSource ? "Formatted view" : "View source"} | |
| > | |
| {viewSource ? <Eye className="w-3 h-3" /> : <Code className="w-3 h-3" />} | |
| </Button> | |
| <Button | |
| variant="ghost" | |
| size="icon" | |
| className="h-5 w-5" | |
| onClick={handleCopy} | |
| title={copied ? "Copied!" : "Copy to clipboard"} | |
| aria-label={copied ? "Copied!" : "Copy to clipboard"} | |
| > |
| const handleCopy = () => { | ||
| navigator.clipboard.writeText(String(content)).then(() => { | ||
| setCopied(true); | ||
| setTimeout(() => setCopied(false), 2000); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
The handleCopy function doesn't handle promise rejection. While there's a .then() call, there's no .catch() to handle clipboard write failures. This could lead to unhandled promise rejections in browsers where clipboard access is denied or unavailable. Other clipboard implementations in the codebase (e.g., SmartContent.tsx line 189-193 and CodeBlock.tsx line 31-37) use try-catch with async/await for proper error handling.
| } | ||
| } | ||
|
|
||
| const MARKDOWN_RE = /^#{1,6}\s|^\s*[-*+]\s|\*\*|__|\[.*\]\(.*\)|```|^\s*\d+\.\s|^\s*>/m; |
There was a problem hiding this comment.
The markdown link detection pattern \[.*\]\(.*\) uses greedy matching which could cause issues. For example, the string "link1 some text link2" would match the entire string instead of individual links. Consider using non-greedy matching: \[.*?\]\(.*?\) to match individual markdown links correctly.
| const MARKDOWN_RE = /^#{1,6}\s|^\s*[-*+]\s|\*\*|__|\[.*\]\(.*\)|```|^\s*\d+\.\s|^\s*>/m; | |
| const MARKDOWN_RE = /^#{1,6}\s|^\s*[-*+]\s|\*\*|__|\[[^\]]+\]\([^)]+\)|```|^\s*\d+\.\s|^\s*>/m; |
| {value.map((item, i) => ( | ||
| <div key={i} className="ml-1 pl-2 border-l border-border"> | ||
| <ValueRenderer value={item} /> | ||
| </div> | ||
| ))} |
There was a problem hiding this comment.
Using array index as a React key in array rendering can cause issues with React's reconciliation if the array order changes or items are added/removed. While this may work for static displays, consider using a more stable key if the data could be dynamic. If the array items are guaranteed to be static (which seems likely for tool/agent call arguments), this is acceptable, but it's worth documenting this assumption.
| {value.map((item, i) => ( | |
| <div key={i} className="ml-1 pl-2 border-l border-border"> | |
| <ValueRenderer value={item} /> | |
| </div> | |
| ))} | |
| {value.map((item, i) => { | |
| const isPrimitive = | |
| typeof item === "string" || typeof item === "number" || typeof item === "boolean"; | |
| const itemKey = isPrimitive ? `${typeof item}:${String(item)}` : i; | |
| return ( | |
| <div key={itemKey} className="ml-1 pl-2 border-l border-border"> | |
| <ValueRenderer value={item} /> | |
| </div> | |
| ); | |
| })} |
| function ValueRenderer({ value, className }: { value: unknown; className?: string }) { | ||
| if (value === null || value === undefined) { | ||
| return <span className="text-xs text-muted-foreground italic">null</span>; | ||
| } | ||
|
|
||
| if (typeof value === "boolean") { | ||
| return <span className={`text-sm ${className ?? ""}`}>{value ? "true" : "false"}</span>; | ||
| } | ||
|
|
||
| if (typeof value === "number") { | ||
| return <span className={`text-sm ${className ?? ""}`}>{String(value)}</span>; | ||
| } | ||
|
|
||
| if (typeof value === "string") { | ||
| return <StringRenderer content={value} className={className} />; | ||
| } | ||
|
|
||
| if (Array.isArray(value)) { | ||
| if (value.length === 0) return <span className="text-xs text-muted-foreground italic">{"[]"}</span>; | ||
| return ( | ||
| <div className={`space-y-1 ${className ?? ""}`}> | ||
| {value.map((item, i) => ( | ||
| <div key={i} className="ml-1 pl-2 border-l border-border"> | ||
| <ValueRenderer value={item} /> | ||
| </div> | ||
| ))} | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| if (typeof value === "object") { | ||
| return <ObjectRenderer obj={value as Record<string, unknown>} className={className} />; | ||
| } | ||
|
|
||
| return <span className="text-sm">{String(value)}</span>; | ||
| } | ||
|
|
||
| function StringRenderer({ content, className }: { content: string; className?: string }) { | ||
| const parsed = tryParseJson(content); | ||
| if (parsed !== null && typeof parsed === "object") { | ||
| return <ValueRenderer value={parsed} className={className} />; | ||
| } | ||
|
|
||
| if (content.includes("\n") || looksLikeMarkdown(content)) { | ||
| return <MarkdownBlock content={content} className={className} />; | ||
| } | ||
|
|
||
| return <span className={`text-sm break-words ${className ?? ""}`}>{content}</span>; | ||
| } |
There was a problem hiding this comment.
The recursive rendering in ValueRenderer, StringRenderer, and ObjectRenderer has no depth limit. While JSON.parse will prevent circular references, extremely deeply nested structures could cause stack overflow errors or performance issues. Consider adding a maximum depth parameter (e.g., maxDepth=10) to prevent rendering issues with pathological data structures.
| {...props} | ||
| > | ||
| <ScrollAreaPrimitive.Viewport className="h-full w-full rounded-[inherit]"> | ||
| <ScrollAreaPrimitive.Viewport className="h-full w-full rounded-[inherit] [&>div]:!block"> |
There was a problem hiding this comment.
The CSS selector [&>div]:!block applies display: block !important to all direct child divs of ScrollArea.Viewport across the entire application. This is a global change that affects all usages of ScrollArea component. The !important flag makes this difficult to override and could break existing layouts that rely on flex, grid, or inline-block displays. Consider if this change should be scoped to specific use cases (e.g., via a prop like forceBlockChildren) rather than applied globally to all ScrollArea instances.
| export function parseContentString(content: string): unknown { | ||
| const trimmed = content.trim(); | ||
| if (trimmed.startsWith("{") || trimmed.startsWith("[")) { | ||
| try { return JSON.parse(trimmed); } catch { /* fall through */ } | ||
| } | ||
| return trimmed; | ||
| } |
There was a problem hiding this comment.
The parseContentString function only attempts to parse strings that start with '{' or '[', but valid JSON can also be primitives like numbers, booleans, or strings (e.g., "true", "123", or ""hello""). While this may be intentional for this use case (only parsing objects and arrays), the function name suggests it should handle all valid JSON. Consider renaming to parseJsonObjectOrArray or documenting this limitation.