feat: Implement core chat refactor and linking#10
Conversation
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Lite 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Comment |
|
Greptile encountered an error while reviewing this PR. Please reach out to support@greptile.com for assistance. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/hooks/useMessages.ts (1)
136-139: Ensure loading resets on failureIf fetch fails, loading stays true indefinitely. Set it to false in catch.
} catch (error) { console.error("Failed to load messages:", error); + setLoading(false); }src/features/workspace/components/chat/blocks/ToolResultBlock.tsx (1)
101-105: Remove hardcoded colors and duplicate maxHeight style.Lines 101-102 hardcode terminal colors (
bg-black/90 text-green-400) instead of using Tailwind color tokens, which violates the coding guideline for this file. Additionally, line 105 applies an inlinemaxHeight: '400px'that duplicates themax-h-96utility on line 103.As per coding guidelines
Apply this diff:
<div className={cn( 'font-mono text-xs p-3 rounded overflow-x-auto', - 'bg-black/90 text-green-400', + 'bg-sidebar-accent/90 text-success', 'border border-cyan-500/20 shadow-sm', 'max-h-96 overflow-y-auto' )} - style={{ maxHeight: '400px' }} >
🧹 Nitpick comments (38)
src/features/workspace/components/chat/blocks/ToolUseBlock.tsx (1)
37-41: Consider spacing between tool blocks.The
my-1(4px vertical margin) creates tight spacing between tool blocks. While this may be intentional for compact layouts, verify it aligns with the design system's spacing tokens. As per coding guidelines, 16px is the default padding baseline.src/features/workspace/components/chat/blocks/BlockRenderer.tsx (2)
34-43: Remove internal keys; rely on the parent list’s key.Keys in the returned elements are ignored since the list key is on at the call site. Drop them to avoid confusion.
Apply:
- return <TextBlock key={`text-${index}`} block={block} role={role} />; + return <TextBlock block={block} role={role} />; @@ - return <ToolUseBlock key={`tool-use-${block.id}`} block={block} toolResult={toolResult} />; + return <ToolUseBlock block={block} toolResult={toolResult} />; @@ - return <ThinkingBlock key={`thinking-${index}`} block={block} />; + return <ThinkingBlock block={block} />; @@ - return <TextBlock key={`text-${index}`} block={{ type: 'text', text: block }} role={role} />; + return <TextBlock block={{ type: 'text', text: block }} role={role} />;Also applies to: 51-53, 60-66
18-26: Make string fallback type-safe (or remove it).You render string blocks in the default branch, but
blockis typedContentBlock, so this is unreachable to TS. Either handle strings up front and widen the prop type, or drop the fallback.Option A (support strings):
interface BlockRendererProps { - block: ContentBlock; + block: ContentBlock | string; index: number; toolResultMap: ToolResultMap; role?: MessageRole; } export function BlockRenderer({ block, index, toolResultMap, role }: BlockRendererProps) { // Handle null/undefined blocks gracefully if (!block) { @@ - // Dispatch based on block type + // Accept raw string content as text + if (typeof block === 'string') { + return <TextBlock block={{ type: 'text', text: block }} role={role} />; + } + + // Dispatch based on block type switch (block.type) {Option B (don’t support strings): remove lines 60–64.
Also applies to: 34-36, 60-66
REFACTOR_SUMMARY.md (1)
11-36: Doc nits: align examples with current behavior.
- ToolResultBlock is listed but tool_result isn’t rendered standalone. Add a note in the tree or remove that entry to avoid confusion.
- “After (Extensible)” sample should pass
toolResultMapintoBlockRenderer, since linking is required:
(<BlockRenderer key={i} block={b} index={i} toolResultMap={toolResultMap} />)- Mention that
registerToolsmust be idempotent to handle HMR/re-mounts without duplicate registrations.Also applies to: 55-67, 210-218
src/features/workspace/components/chat/tools/components/CodeBlock.tsx (1)
27-33: Add minimal a11y/semantics for code blocks.Expose language and group semantics to assistive tech.
Apply:
- <div className={cn(chatTheme.blocks.code.container, className)}> + <div + className={cn(chatTheme.blocks.code.container, className)} + role="group" + aria-label={`Code snippet${language ? ` (${language})` : ''}`} + data-language={language} + >Also applies to: 34-49
src/features/workspace/components/chat/tools/components/CopyButton.tsx (1)
48-75: Add type and cleanup timeout for safetyPrevent accidental form submits and avoid setState after unmount.
-import { useState } from 'react'; +import { useEffect, useRef, useState } from 'react'; @@ export function CopyButton({ @@ }: CopyButtonProps) { const [copied, setCopied] = useState(false); + const timerRef = useRef<number | null>(null); @@ - setCopied(true); - setTimeout(() => setCopied(false), 2000); + setCopied(true); + if (timerRef.current) clearTimeout(timerRef.current); + timerRef.current = window.setTimeout(() => setCopied(false), 2000); @@ return ( <button + type="button" onClick={handleCopy} @@ </button> ); } + +// Cleanup timer on unmount +useEffect(() => { + return () => { + if (timerRef.current) clearTimeout(timerRef.current); + }; +}, []);src/features/workspace/components/chat/theme/chatTheme.ts (2)
94-112: Prefer spring animations; consider transform-based expandGuidelines: default to spring; prefer transform over layout (height) where possible. Switch to spring now; consider scaleY in a follow-up.
messageEnter: { initial: { opacity: 0, y: 10 }, animate: { opacity: 1, y: 0 }, - transition: { duration: 0.2, ease: [0.215, 0.61, 0.355, 1] }, // ease-out-cubic + transition: { type: 'spring', stiffness: 300, damping: 24 }, }, toolExpand: { - initial: { height: 0, opacity: 0 }, - animate: { height: 'auto', opacity: 1 }, - exit: { height: 0, opacity: 0 }, - transition: { duration: 0.3, ease: [0.215, 0.61, 0.355, 1] }, + initial: { opacity: 0, scaleY: 0.95, transformOrigin: 'top' }, + animate: { opacity: 1, scaleY: 1 }, + exit: { opacity: 0, scaleY: 0.95 }, + transition: { type: 'spring', stiffness: 260, damping: 22 }, }, fadeIn: { initial: { opacity: 0 }, animate: { opacity: 1 }, - transition: { duration: 0.2 }, + transition: { type: 'spring', stiffness: 260, damping: 22 }, },Also applies to: 101-106
78-92: Normalize spacing to 16px base where feasibleMultiple gaps/paddings use 8/12/20px equivalents. For consistency, prefer 16px defaults unless a strong reason exists.
Also applies to: 114-129
src/hooks/useMessages.ts (1)
205-213: Avoid overlapping polls under slow networksPolling every 1s can overlap requests. Use AbortController or an “inFlight” guard to skip if a previous call hasn’t finished.
src/features/workspace/components/chat/tools/renderers/BashToolRenderer.tsx (1)
69-77: Reuse theme animation variants for consistencyUse chatTheme.animations.toolExpand to keep motion consistent across tools.
src/features/workspace/components/chat/tools/renderers/EditToolRenderer.tsx (1)
20-27: Deduplicate clipboard UI by using CopyButtonReplace bespoke copy logic with the shared component; adds error handling and consistent UX.
-import { ChevronDown, ChevronRight, FileEdit, Copy, Check } from 'lucide-react'; +import { ChevronDown, ChevronRight, FileEdit } from 'lucide-react'; +import { CopyButton } from '../components/CopyButton'; @@ - const [copiedOld, setCopiedOld] = useState(false); - const [copiedNew, setCopiedNew] = useState(false); @@ - const handleCopy = async (text: string, setter: (v: boolean) => void) => { - await navigator.clipboard.writeText(text); - setter(true); - setTimeout(() => setter(false), 2000); - }; + // Copy handled by CopyButton @@ - <button - onClick={(e) => { - e.stopPropagation(); - handleCopy(old_string, setCopiedOld); - }} - className="text-xs hover:bg-destructive/20 px-2 py-0.5 rounded transition-colors flex items-center gap-1" - title="Copy before" - > - {copiedOld ? ( - <> - <Check className="w-3 h-3" /> - Copied - </> - ) : ( - <> - <Copy className="w-3 h-3" /> - Copy - </> - )} - </button> + <CopyButton text={old_string} label="Copy" size="sm" className="text-xs" /> @@ - <button - onClick={(e) => { - e.stopPropagation(); - handleCopy(new_string, setCopiedNew); - }} - className="text-xs hover:bg-success/20 px-2 py-0.5 rounded transition-colors flex items-center gap-1" - title="Copy after" - > - {copiedNew ? ( - <> - <Check className="w-3 h-3" /> - Copied - </> - ) : ( - <> - <Copy className="w-3 h-3" /> - Copy - </> - )} - </button> + <CopyButton text={new_string} label="Copy" size="sm" className="text-xs" />Also applies to: 84-113, 120-146
src/features/workspace/components/chat/tools/renderers/MultiEditToolRenderer.tsx (1)
95-129: Unify copy UX with CopyButton and add error handlingUse the shared CopyButton to avoid duplicating clipboard logic and to get built‑in feedback.
Also applies to: 131-155, 29-33
src/features/workspace/components/chat/tools/renderers/LSToolRenderer.tsx (1)
105-127: Consider more stable keys for dynamic lists.Using array indices as keys (line 111:
key={index}) can cause React reconciliation issues if the list order changes or items are added/removed dynamically. Since directory listings are relatively static, this is acceptable, but consider using a more stable identifier if available (e.g., combining index with item name).Example improvement:
- key={index} + key={`${index}-${cleanItem}`}src/features/workspace/components/chat/tools/renderers/TaskToolRenderer.tsx (1)
28-36: Consider using theme tokens for consistency.Line 34 uses a hardcoded violet border color (
border-l-violet-500/50) while other renderers use theme tokens (e.g.,chatTheme.blocks.tool.borderLeft.*). For consistency and maintainability, consider defining a theme token for agent-related styling.As per coding guidelines (avoid hardcoding colors; use Tailwind color tokens consistently).
src/features/workspace/components/chat/blocks/TextBlock.tsx (1)
25-32: Improve type safety in PreWithCopy component.The
PreWithCopyfunction usesanytypes for both the props parameter and thenodeparameter ingetCodeText. This reduces type safety and could hide potential runtime errors.Consider defining proper types:
-function PreWithCopy({ children, ...props }: any) { +interface PreWithCopyProps extends React.HTMLAttributes<HTMLPreElement> { + children?: React.ReactNode; +} + +function PreWithCopy({ children, ...props }: PreWithCopyProps) { // Extract code text from children - const getCodeText = (node: any): string => { + const getCodeText = (node: unknown): string => { if (typeof node === 'string') return node; if (Array.isArray(node)) return node.map(getCodeText).join(''); - if (node?.props?.children) return getCodeText(node.props.children); + if (node && typeof node === 'object' && 'props' in node && node.props?.children) { + return getCodeText(node.props.children); + } return ''; };src/features/workspace/components/chat/tools/renderers/TodoWriteToolRenderer.tsx (2)
109-151: Consider more stable keys for todo items.Similar to other renderers, using array indices as keys (line 109) can cause React reconciliation issues. Since todos have content that could serve as part of a stable identifier, consider using a composite key.
- key={index} + key={`${index}-${todo.content.slice(0, 20)}`}
56-64: Consider using theme tokens for purple styling.Line 62 uses a hardcoded purple border color (
border-l-purple-500/50). For consistency with the theme system and maintainability, consider adding a theme token for todo-related styling.As per coding guidelines.
src/features/workspace/components/chat/tools/components/SyntaxHighlighter.tsx (1)
26-30: Type mismatch and stub implementation.Line 26 declares
highlightLineas returningReact.ReactNode, but line 29 returns a plain string. While this works (strings are valid React nodes), it's more accurate to type it asstringsince that's what the current stub returns.Additionally, the function is a complete stub with no highlighting logic. Consider either:
- Renaming the component to
CodeDisplayuntil highlighting is implemented, or- Adding a TODO comment in the component's JSDoc header to clarify this is a placeholder
- const highlightLine = (line: string, lang: string): React.ReactNode => { + const highlightLine = (line: string, lang: string): string => { // For now, return plain text. Easy to enhance later. // This keeps the component ready for Prism.js integration. return line; };src/features/workspace/components/chat/blocks/ThinkingBlock.tsx (1)
76-98: Consider adding expand/collapse animation for consistency.Unlike other renderers in this PR (LSToolRenderer, TaskToolRenderer, TodoWriteToolRenderer, GrepToolRenderer), ThinkingBlock doesn't animate the expand/collapse transition. For a consistent user experience and to follow the pattern established by other components, consider wrapping the content in
AnimatePresenceandmotion.div.As per coding guidelines for animations and based on patterns in related files.
Example:
<AnimatePresence initial={false}> {isExpanded && ( <motion.div initial={{ height: 0, opacity: 0 }} animate={{ height: 'auto', opacity: 1 }} exit={{ height: 0, opacity: 0 }} transition={{ duration: 0.2, ease: [0.23, 1, 0.32, 1] }} className="overflow-hidden" > <div className="px-3 pb-3 pt-1"> {/* existing content */} </div> </motion.div> )} </AnimatePresence>src/features/workspace/components/chat/tools/renderers/ReadToolRenderer.tsx (2)
23-38: Deduplicate language detection.getLanguage is duplicated across renderers. Extract to a shared util (e.g., detectLanguageFromPath in tools/components/lang.ts) and import here to keep mappings consistent.
As per coding guidelines.
- // Detect language from file extension - const getLanguage = (path: string): string => { - const ext = path.split('.').pop()?.toLowerCase(); - const languageMap: Record<string, string> = { - ts: 'typescript', - tsx: 'typescript', - js: 'javascript', - jsx: 'javascript', - py: 'python', - rs: 'rust', - go: 'go', - java: 'java', - json: 'json', - md: 'markdown', - }; - return languageMap[ext || ''] || 'text'; - }; + // TODO: replace with shared util to avoid drift across renderers + // const getLanguage = detectLanguageFromPath;
91-95: Animation config: prefer spring and transform-based where possible.Guidelines favor spring transitions and transform-based properties. Consider scaleY/opacity with transformOrigin:'top' and a gentle spring, or keep height but spring opacity and gate via MotionConfig reducedMotion.
As per coding guidelines.
src/features/workspace/components/chat/tools/renderers/WriteToolRenderer.tsx (3)
16-21: Auto‑expand to reveal errors.If the write failed, expand automatically so users see the error without extra clicks.
export function WriteToolRenderer({ toolUse, toolResult }: ToolRendererProps) { - const [isExpanded, setIsExpanded] = useState(true); + const [isExpanded, setIsExpanded] = useState(true); const { file_path, content } = toolUse.input; const isError = toolResult?.is_error; + useEffect(() => { + if (isError) setIsExpanded(true); + }, [isError]);Also applies to: 100-109
23-38: Deduplicate language detection.Same note as ReadToolRenderer: move getLanguage to a shared util to keep mappings in sync.
As per coding guidelines.
82-86: Animation config: consider spring and transform.Adopt a gentle spring and transform-based expand (scaleY with transformOrigin:'top') where acceptable, and ensure reduced-motion is respected globally.
As per coding guidelines.
src/features/workspace/components/chat/tools/renderers/KillShellToolRenderer.tsx (1)
76-83: Announce result to screen readers.Add aria-live to the success/error container so status changes are announced.
- {!isError ? ( - <div className="text-xs text-success-foreground bg-success/10 border border-success/20 rounded p-2"> + {!isError ? ( + <div aria-live="polite" className="text-xs text-success-foreground bg-success/10 border border-success/20 rounded p-2"> Background process terminated successfully </div> ) : ( - <div className="text-xs text-destructive-foreground bg-destructive/10 border border-destructive/30 rounded p-2 font-mono"> + <div aria-live="assertive" className="text-xs text-destructive-foreground bg-destructive/10 border border-destructive/30 rounded p-2 font-mono"> {typeof toolResult.content === 'object' ? JSON.stringify(toolResult.content, null, 2) : toolResult.content} </div> )}PHASE3_COMPLETE.md (1)
47-52: Align docs with animation guidance and implementation.Docs state transform/opacity and reduced‑motion compliance, while components animate height/opacity and don’t show explicit reduced‑motion wiring. Either update the guidance or switch components to spring + transform and configure reduced motion globally (MotionConfig/useReducedMotion).
As per coding guidelines.
Also applies to: 108-113
src/features/workspace/components/chat/tools/renderers/GlobToolRenderer.tsx (3)
110-119: Avoid index as key; use the file path.Stable keys prevent reconciliation glitches when the list changes.
- {files.map((file, index) => ( + {files.map((file, index) => ( <div - key={index} + key={file} className="text-xs font-mono bg-muted/50 hover:bg-muted transition-colors px-2 py-1 rounded group flex items-center gap-2" >
21-31: More robust result parsing.Support array payloads and trim consistently; avoid relying on “No files found” string.
- // Parse results - Glob returns newline-separated file paths or "No files found" - const parseResults = (content: string): string[] => { - if (!content || content.trim() === '' || content.includes('No files found')) { + // Parse results - accept string (newline-delimited) or string[] + const parseResults = (content: unknown): string[] => { + if (Array.isArray(content)) { + return content.map(String).filter((s) => s.trim().length > 0); + } + const text = String(content ?? '').trim(); + if (!text || /no files found/i.test(text)) { return []; } - return content.split('\n').filter(line => line.trim().length > 0); + return text.split('\n').map((l) => l.trim()).filter((l) => l.length > 0); }; @@ - const files = toolResult && !isError ? parseResults( - typeof toolResult.content === 'string' ? toolResult.content : JSON.stringify(toolResult.content) - ) : []; + const files = toolResult && !isError ? parseResults(toolResult.content) : [];
134-143: Extract shared ErrorBlock to remove duplication.The styled error container repeats across renderers; factor into a small component (tools/components/ErrorBlock.tsx).
As per coding guidelines.
src/features/workspace/components/chat/blocks/ToolResultBlock.tsx (2)
47-47: Consider using Lucide icons instead of emoji for consistency.The component uses hardcoded emoji (❌, ✅) for status indicators, while other tool renderers in this PR use Lucide React icons. For visual consistency and better accessibility, consider replacing these with Lucide icons like
XCircleandCheckCircle.Apply this diff to use Lucide icons:
+import { XCircle, CheckCircle } from 'lucide-react'; + export function ToolResultBlock({ block }: ToolResultBlockProps) { // ... existing code ... return ( <div className={cn(/* ... */)}> <div className={chatTheme.blocks.tool.header}> <span className={chatTheme.blocks.tool.icon}> - {isError ? '❌' : '✅'} + {isError ? <XCircle className="w-3 h-3" /> : <CheckCircle className="w-3 h-3" />} </span>Also applies to: 65-65
89-93: Consider using spring animation for expand/collapse.The animation uses an ease curve, but the coding guidelines recommend defaulting to spring animations when using Framer Motion for a more natural feel.
As per coding guidelines
Apply this diff:
<motion.div initial={{ height: 0, opacity: 0 }} animate={{ height: 'auto', opacity: 1 }} exit={{ height: 0, opacity: 0 }} - transition={{ duration: 0.2, ease: [0.23, 1, 0.32, 1] }} + transition={{ type: 'spring', stiffness: 300, damping: 30 }} className="overflow-hidden" >src/features/workspace/components/chat/tools/renderers/WebSearchToolRenderer.tsx (1)
112-116: Consider using spring animation for expand/collapse.The coding guidelines recommend defaulting to spring animations when using Framer Motion for a more natural feel.
As per coding guidelines
Apply this diff:
<motion.div initial={{ height: 0, opacity: 0 }} animate={{ height: 'auto', opacity: 1 }} exit={{ height: 0, opacity: 0 }} - transition={{ duration: 0.2, ease: [0.23, 1, 0.32, 1] }} + transition={{ type: 'spring', stiffness: 300, damping: 30 }} className="overflow-hidden" >src/features/workspace/components/chat/tools/renderers/DefaultToolRenderer.tsx (2)
98-129: Add expand/collapse animation for consistency.Unlike other tool renderers (BashOutputToolRenderer, WebSearchToolRenderer, WebFetchToolRenderer), this default renderer doesn't animate the expand/collapse transition. This creates an inconsistent user experience across tool types.
Apply this diff to add animation:
+import { motion, AnimatePresence } from 'framer-motion'; + export function DefaultToolRenderer({ toolUse, toolResult }: ToolRendererProps) { // ... existing code ... return ( <div className={/* ... */}> {/* Header */} {/* ... */} {/* Expandable content */} - {isExpanded && ( - <div className="space-y-2"> + <AnimatePresence initial={false}> + {isExpanded && ( + <motion.div + initial={{ height: 0, opacity: 0 }} + animate={{ height: 'auto', opacity: 1 }} + exit={{ height: 0, opacity: 0 }} + transition={{ type: 'spring', stiffness: 300, damping: 30 }} + className="overflow-hidden" + > + <div className="space-y-2"> {/* Tool input */} {/* ... existing content ... */} + </div> + </motion.div> + )} + </AnimatePresence> - </div> - )} </div> ); }
44-48: Consider memoizing parseMcpToolName result.The
parseMcpToolNamefunction is called on every render (line 48), though its result only depends ontoolUse.name. While the function is inexpensive, memoizing withuseMemowould be a minor performance optimization and best practice.Apply this diff:
+import { useState, useMemo } from 'react'; -import { useState } from 'react'; export function DefaultToolRenderer({ toolUse, toolResult }: ToolRendererProps) { const [isExpanded, setIsExpanded] = useState(false); const isError = toolResult?.is_error; - const toolInfo = parseMcpToolName(toolUse.name); + const toolInfo = useMemo(() => parseMcpToolName(toolUse.name), [toolUse.name]);src/features/workspace/components/chat/tools/renderers/WebFetchToolRenderer.tsx (1)
98-102: Consider using spring animation for expand/collapse.The coding guidelines recommend defaulting to spring animations when using Framer Motion for a more natural feel.
As per coding guidelines
Apply this diff:
<motion.div initial={{ height: 0, opacity: 0 }} animate={{ height: 'auto', opacity: 1 }} exit={{ height: 0, opacity: 0 }} - transition={{ duration: 0.2, ease: [0.23, 1, 0.32, 1] }} + transition={{ type: 'spring', stiffness: 300, damping: 30 }} className="overflow-hidden" >src/features/workspace/components/chat/tools/renderers/index.ts (1)
1-15: Verify tool renderer count consistency.This index exports 15 tool renderers, but REFACTOR_PROGRESS.md (line 163) claims "17 specialized renderers." The discrepancy appears to be:
- ThinkingBlock is a content block renderer, not a tool renderer
- DefaultToolRenderer is a fallback, not a specialized renderer
Consider updating the progress documentation to clarify that there are 15 specialized tool renderers plus the default fallback.
src/features/workspace/components/chat/tools/renderers/BashOutputToolRenderer.tsx (2)
35-35: Replace hardcoded colors with theme tokens.Multiple instances of hardcoded cyan, black, and green colors violate the coding guideline to use Tailwind color tokens. Consider using theme tokens from
chatThemeor Tailwind semantic colors (e.g.,primary,accent,muted).Examples:
- Line 35:
border-l-cyan-500/50 bg-cyan-50/20 dark:bg-cyan-950/10- Lines 52, 74:
text-cyan-600 dark:text-cyan-400- Line 101:
bg-black/90 text-green-400- Line 102:
border-cyan-500/20As per coding guidelines.
Also applies to: 52-52, 74-74, 99-102
90-93: Consider transform-based animation and spring transition.Current implementation animates
heightandopacity, which can be less performant. Per coding guidelines:
- Prefer transform-based animations (e.g.,
scaleY,translateY) to leverage hardware acceleration- Default to spring animations when using Framer Motion
Good: Duration of 200ms is within the recommended 200ms–300ms range.
Example with transform-based spring animation:
-initial={{ height: 0, opacity: 0 }} -animate={{ height: 'auto', opacity: 1 }} -exit={{ height: 0, opacity: 0 }} -transition={{ duration: 0.2, ease: [0.23, 1, 0.32, 1] }} +initial={{ scaleY: 0, opacity: 0 }} +animate={{ scaleY: 1, opacity: 1 }} +exit={{ scaleY: 0, opacity: 0 }} +transition={{ type: 'spring', stiffness: 500, damping: 30 }} +style={{ transformOrigin: 'top' }}As per coding guidelines.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (55)
CHAT_REFACTOR_PROPOSAL.md(1 hunks)CONDUCTOR_ARCHITECTURE.md(1 hunks)PHASE2_COMPLETE.md(1 hunks)PHASE3_COMPLETE.md(1 hunks)REFACTOR_COMPLETE.md(1 hunks)REFACTOR_PROGRESS.md(1 hunks)REFACTOR_SUMMARY.md(1 hunks)TESTING_INSTRUCTIONS.md(1 hunks)TOOL_LINKING_IMPLEMENTATION_COMPLETE.md(1 hunks)TOOL_USE_RESULT_LINKING_ANALYSIS.md(1 hunks)backend/lib/claude-session.cjs(1 hunks)package.json(1 hunks)src/WorkspaceChatPanel.tsx(3 hunks)src/features/workspace/components/Chat.tsx(4 hunks)src/features/workspace/components/MessageItem.tsx(2 hunks)src/features/workspace/components/chat/blocks/BlockRenderer.tsx(1 hunks)src/features/workspace/components/chat/blocks/TextBlock.tsx(1 hunks)src/features/workspace/components/chat/blocks/ThinkingBlock.tsx(1 hunks)src/features/workspace/components/chat/blocks/ToolResultBlock.tsx(1 hunks)src/features/workspace/components/chat/blocks/ToolUseBlock.tsx(1 hunks)src/features/workspace/components/chat/blocks/index.ts(1 hunks)src/features/workspace/components/chat/index.ts(1 hunks)src/features/workspace/components/chat/message/MessageItem.tsx(1 hunks)src/features/workspace/components/chat/message/index.ts(1 hunks)src/features/workspace/components/chat/theme/chatTheme.ts(1 hunks)src/features/workspace/components/chat/theme/index.ts(1 hunks)src/features/workspace/components/chat/tools/ToolRegistry.tsx(1 hunks)src/features/workspace/components/chat/tools/components/CodeBlock.tsx(1 hunks)src/features/workspace/components/chat/tools/components/CopyButton.tsx(1 hunks)src/features/workspace/components/chat/tools/components/FilePathDisplay.tsx(1 hunks)src/features/workspace/components/chat/tools/components/SyntaxHighlighter.tsx(1 hunks)src/features/workspace/components/chat/tools/components/index.ts(1 hunks)src/features/workspace/components/chat/tools/index.ts(1 hunks)src/features/workspace/components/chat/tools/registerTools.ts(1 hunks)src/features/workspace/components/chat/tools/renderers/BashOutputToolRenderer.tsx(1 hunks)src/features/workspace/components/chat/tools/renderers/BashToolRenderer.tsx(1 hunks)src/features/workspace/components/chat/tools/renderers/DefaultToolRenderer.tsx(1 hunks)src/features/workspace/components/chat/tools/renderers/EditToolRenderer.tsx(1 hunks)src/features/workspace/components/chat/tools/renderers/GlobToolRenderer.tsx(1 hunks)src/features/workspace/components/chat/tools/renderers/GrepToolRenderer.tsx(1 hunks)src/features/workspace/components/chat/tools/renderers/KillShellToolRenderer.tsx(1 hunks)src/features/workspace/components/chat/tools/renderers/LSToolRenderer.tsx(1 hunks)src/features/workspace/components/chat/tools/renderers/MultiEditToolRenderer.tsx(1 hunks)src/features/workspace/components/chat/tools/renderers/ReadToolRenderer.tsx(1 hunks)src/features/workspace/components/chat/tools/renderers/TaskToolRenderer.tsx(1 hunks)src/features/workspace/components/chat/tools/renderers/TodoWriteToolRenderer.tsx(1 hunks)src/features/workspace/components/chat/tools/renderers/WebFetchToolRenderer.tsx(1 hunks)src/features/workspace/components/chat/tools/renderers/WebSearchToolRenderer.tsx(1 hunks)src/features/workspace/components/chat/tools/renderers/WriteToolRenderer.tsx(1 hunks)src/features/workspace/components/chat/tools/renderers/index.ts(1 hunks)src/features/workspace/components/chat/types.ts(1 hunks)src/hooks/useMessages.ts(3 hunks)src/types/index.ts(1 hunks)src/types/session.types.ts(2 hunks)tailwind.config.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{tsx,jsx}: Use Tailwind for styling in UI components (avoid custom CSS when a Tailwind utility or token exists)
Base UI on shadcn/ui components
Split UI into reusable, focused components
When using Framer Motion, prefer transform-based animations over x/y to leverage hardware acceleration
Default to spring animations when using Framer Motion
Avoid bouncy spring configurations in Framer Motion unless for drag gestures
Files:
src/features/workspace/components/chat/tools/components/FilePathDisplay.tsxsrc/features/workspace/components/chat/blocks/ToolUseBlock.tsxsrc/features/workspace/components/chat/tools/components/CodeBlock.tsxsrc/features/workspace/components/chat/tools/renderers/LSToolRenderer.tsxsrc/features/workspace/components/chat/blocks/TextBlock.tsxsrc/features/workspace/components/chat/message/MessageItem.tsxsrc/features/workspace/components/chat/tools/renderers/TaskToolRenderer.tsxsrc/features/workspace/components/chat/blocks/ToolResultBlock.tsxsrc/features/workspace/components/chat/tools/renderers/WriteToolRenderer.tsxsrc/features/workspace/components/chat/tools/renderers/MultiEditToolRenderer.tsxsrc/features/workspace/components/chat/tools/renderers/BashToolRenderer.tsxsrc/features/workspace/components/chat/tools/renderers/KillShellToolRenderer.tsxsrc/features/workspace/components/chat/tools/renderers/BashOutputToolRenderer.tsxsrc/features/workspace/components/chat/tools/renderers/EditToolRenderer.tsxsrc/features/workspace/components/chat/tools/renderers/GrepToolRenderer.tsxsrc/features/workspace/components/chat/tools/renderers/DefaultToolRenderer.tsxsrc/features/workspace/components/chat/tools/renderers/WebSearchToolRenderer.tsxsrc/features/workspace/components/chat/blocks/BlockRenderer.tsxsrc/features/workspace/components/chat/tools/renderers/GlobToolRenderer.tsxsrc/WorkspaceChatPanel.tsxsrc/features/workspace/components/chat/tools/renderers/TodoWriteToolRenderer.tsxsrc/features/workspace/components/MessageItem.tsxsrc/features/workspace/components/chat/blocks/ThinkingBlock.tsxsrc/features/workspace/components/chat/tools/components/SyntaxHighlighter.tsxsrc/features/workspace/components/chat/tools/renderers/ReadToolRenderer.tsxsrc/features/workspace/components/chat/tools/ToolRegistry.tsxsrc/features/workspace/components/chat/tools/renderers/WebFetchToolRenderer.tsxsrc/features/workspace/components/Chat.tsxsrc/features/workspace/components/chat/tools/components/CopyButton.tsx
**/*.{css,scss,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{css,scss,tsx,jsx}: Use consistent paddings with a 16px default
Use consistent font sizes across the app
Avoid hardcoding colors; always use Tailwind color tokens
Default to ease-out for most animations
Animations should be ≤1s; most between 200ms–300ms
Animate elements from their trigger; set transform-origin based on trigger position
Files:
src/features/workspace/components/chat/tools/components/FilePathDisplay.tsxsrc/features/workspace/components/chat/blocks/ToolUseBlock.tsxsrc/features/workspace/components/chat/tools/components/CodeBlock.tsxsrc/features/workspace/components/chat/tools/renderers/LSToolRenderer.tsxsrc/features/workspace/components/chat/blocks/TextBlock.tsxsrc/features/workspace/components/chat/message/MessageItem.tsxsrc/features/workspace/components/chat/tools/renderers/TaskToolRenderer.tsxsrc/features/workspace/components/chat/blocks/ToolResultBlock.tsxsrc/features/workspace/components/chat/tools/renderers/WriteToolRenderer.tsxsrc/features/workspace/components/chat/tools/renderers/MultiEditToolRenderer.tsxsrc/features/workspace/components/chat/tools/renderers/BashToolRenderer.tsxsrc/features/workspace/components/chat/tools/renderers/KillShellToolRenderer.tsxsrc/features/workspace/components/chat/tools/renderers/BashOutputToolRenderer.tsxsrc/features/workspace/components/chat/tools/renderers/EditToolRenderer.tsxsrc/features/workspace/components/chat/tools/renderers/GrepToolRenderer.tsxsrc/features/workspace/components/chat/tools/renderers/DefaultToolRenderer.tsxsrc/features/workspace/components/chat/tools/renderers/WebSearchToolRenderer.tsxsrc/features/workspace/components/chat/blocks/BlockRenderer.tsxsrc/features/workspace/components/chat/tools/renderers/GlobToolRenderer.tsxsrc/WorkspaceChatPanel.tsxsrc/features/workspace/components/chat/tools/renderers/TodoWriteToolRenderer.tsxsrc/features/workspace/components/MessageItem.tsxsrc/features/workspace/components/chat/blocks/ThinkingBlock.tsxsrc/features/workspace/components/chat/tools/components/SyntaxHighlighter.tsxsrc/features/workspace/components/chat/tools/renderers/ReadToolRenderer.tsxsrc/features/workspace/components/chat/tools/ToolRegistry.tsxsrc/features/workspace/components/chat/tools/renderers/WebFetchToolRenderer.tsxsrc/features/workspace/components/Chat.tsxsrc/features/workspace/components/chat/tools/components/CopyButton.tsx
**/tailwind.config.{js,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Define and maintain font and color tokens in Tailwind config and reference them in the UI
Files:
tailwind.config.js
🧬 Code graph analysis (24)
src/features/workspace/components/chat/tools/components/FilePathDisplay.tsx (2)
src/features/workspace/components/chat/tools/components/index.ts (1)
FilePathDisplay(3-3)src/lib/utils.ts (1)
cn(4-6)
src/features/workspace/components/chat/tools/components/CodeBlock.tsx (3)
src/lib/utils.ts (1)
cn(4-6)src/features/workspace/components/chat/tools/components/CopyButton.tsx (1)
CopyButton(18-76)src/features/workspace/components/chat/tools/components/SyntaxHighlighter.tsx (1)
SyntaxHighlighter(17-58)
src/features/workspace/components/chat/tools/renderers/LSToolRenderer.tsx (4)
src/features/workspace/components/chat/tools/renderers/index.ts (1)
LSToolRenderer(15-15)src/features/workspace/components/chat/types.ts (1)
ToolRendererProps(18-21)src/lib/utils.ts (1)
cn(4-6)src-tauri/sidecar/index.bundled.cjs (1)
index(819-819)
src/features/workspace/components/chat/blocks/TextBlock.tsx (2)
src/features/workspace/components/chat/theme/chatTheme.ts (1)
chatTheme(8-129)src/lib/utils.ts (1)
cn(4-6)
src/features/workspace/components/chat/tools/renderers/TaskToolRenderer.tsx (4)
src/features/workspace/components/chat/tools/renderers/index.ts (1)
TaskToolRenderer(14-14)src/features/workspace/components/chat/types.ts (1)
ToolRendererProps(18-21)src/lib/utils.ts (1)
cn(4-6)backend/lib/config.cjs (1)
description(170-170)
src/features/workspace/components/chat/blocks/ToolResultBlock.tsx (2)
src/lib/utils.ts (1)
cn(4-6)src/features/workspace/components/chat/theme/chatTheme.ts (1)
chatTheme(8-129)
src/features/workspace/components/chat/tools/renderers/MultiEditToolRenderer.tsx (3)
src/features/workspace/components/chat/tools/renderers/index.ts (1)
MultiEditToolRenderer(10-10)src/features/workspace/components/chat/types.ts (1)
ToolRendererProps(18-21)src/lib/utils.ts (1)
cn(4-6)
src/features/workspace/components/chat/tools/renderers/BashToolRenderer.tsx (3)
src/features/workspace/components/chat/types.ts (1)
ToolRendererProps(18-21)src/lib/utils.ts (1)
cn(4-6)src/features/workspace/components/chat/tools/components/CopyButton.tsx (1)
CopyButton(18-76)
src/features/workspace/components/chat/tools/renderers/KillShellToolRenderer.tsx (4)
src/features/workspace/components/chat/tools/renderers/index.ts (1)
KillShellToolRenderer(13-13)src/features/workspace/components/chat/types.ts (1)
ToolRendererProps(18-21)src/lib/utils.ts (1)
cn(4-6)src/Terminal.tsx (1)
Terminal(16-175)
src/features/workspace/components/chat/tools/renderers/EditToolRenderer.tsx (4)
src/features/workspace/components/chat/tools/renderers/index.ts (1)
EditToolRenderer(2-2)src/features/workspace/components/chat/types.ts (1)
ToolRendererProps(18-21)src/lib/utils.ts (1)
cn(4-6)src/features/workspace/components/chat/theme/chatTheme.ts (1)
chatTheme(8-129)
src/features/workspace/components/chat/tools/renderers/GrepToolRenderer.tsx (5)
src/features/workspace/components/chat/tools/renderers/index.ts (1)
GrepToolRenderer(6-6)src/features/workspace/components/chat/types.ts (1)
ToolRendererProps(18-21)src/lib/utils.ts (1)
cn(4-6)src/features/workspace/components/chat/tools/components/index.ts (1)
CopyButton(1-1)src/features/workspace/components/chat/tools/components/CopyButton.tsx (1)
CopyButton(18-76)
src/features/workspace/components/chat/tools/renderers/DefaultToolRenderer.tsx (3)
src/features/workspace/components/chat/types.ts (1)
ToolRendererProps(18-21)src/lib/utils.ts (1)
cn(4-6)src/features/workspace/components/chat/theme/chatTheme.ts (1)
chatTheme(8-129)
src/features/workspace/components/chat/tools/renderers/GlobToolRenderer.tsx (3)
src/features/workspace/components/chat/tools/renderers/index.ts (1)
GlobToolRenderer(8-8)src/features/workspace/components/chat/types.ts (1)
ToolRendererProps(18-21)src/lib/utils.ts (1)
cn(4-6)
src/types/session.types.ts (1)
src/features/workspace/components/chat/blocks/ThinkingBlock.tsx (1)
ThinkingBlock(19-101)
src/features/workspace/components/chat/tools/renderers/TodoWriteToolRenderer.tsx (3)
src/features/workspace/components/chat/tools/renderers/index.ts (1)
TodoWriteToolRenderer(7-7)src/features/workspace/components/chat/types.ts (1)
ToolRendererProps(18-21)src/lib/utils.ts (1)
cn(4-6)
src/features/workspace/components/chat/types.ts (3)
src/features/workspace/components/chat/blocks/index.ts (2)
ToolResultBlock(4-4)ToolUseBlock(3-3)src/features/workspace/components/chat/blocks/ToolResultBlock.tsx (1)
ToolResultBlock(16-70)src/features/workspace/components/chat/blocks/ToolUseBlock.tsx (1)
ToolUseBlock(19-42)
src/features/workspace/components/chat/theme/chatTheme.ts (2)
src/features/workspace/components/chat/index.ts (1)
chatTheme(18-18)src/features/workspace/components/chat/theme/index.ts (2)
chatTheme(1-1)ChatTheme(2-2)
src/features/workspace/components/chat/blocks/ThinkingBlock.tsx (4)
src/features/workspace/components/chat/blocks/index.ts (1)
ThinkingBlock(5-5)src/types/index.ts (1)
ThinkingBlock(30-30)src/types/session.types.ts (1)
ThinkingBlock(62-66)src/lib/utils.ts (1)
cn(4-6)
src/features/workspace/components/chat/tools/components/SyntaxHighlighter.tsx (2)
src/features/workspace/components/chat/tools/components/index.ts (1)
SyntaxHighlighter(4-4)src/lib/utils.ts (1)
cn(4-6)
src/features/workspace/components/chat/tools/renderers/ReadToolRenderer.tsx (6)
src/features/workspace/components/chat/tools/renderers/index.ts (1)
ReadToolRenderer(5-5)src/features/workspace/components/chat/types.ts (1)
ToolRendererProps(18-21)src/lib/utils.ts (1)
cn(4-6)src/features/workspace/components/chat/tools/components/index.ts (2)
FilePathDisplay(3-3)CodeBlock(2-2)src/features/workspace/components/chat/tools/components/FilePathDisplay.tsx (1)
FilePathDisplay(15-57)src/features/workspace/components/chat/tools/components/CodeBlock.tsx (1)
CodeBlock(20-50)
src/features/workspace/components/chat/tools/renderers/WebFetchToolRenderer.tsx (3)
src/features/workspace/components/chat/tools/renderers/index.ts (1)
WebFetchToolRenderer(11-11)src/features/workspace/components/chat/types.ts (1)
ToolRendererProps(18-21)src/lib/utils.ts (1)
cn(4-6)
src/features/workspace/components/Chat.tsx (1)
src/features/workspace/components/chat/types.ts (1)
ToolResultMap(13-13)
src/features/workspace/components/chat/tools/registerTools.ts (2)
src/features/workspace/components/chat/tools/ToolRegistry.tsx (1)
toolRegistry(132-132)src/features/workspace/components/chat/tools/renderers/index.ts (15)
DefaultToolRenderer(1-1)EditToolRenderer(2-2)WriteToolRenderer(3-3)BashToolRenderer(4-4)ReadToolRenderer(5-5)GrepToolRenderer(6-6)TodoWriteToolRenderer(7-7)GlobToolRenderer(8-8)BashOutputToolRenderer(9-9)MultiEditToolRenderer(10-10)WebFetchToolRenderer(11-11)WebSearchToolRenderer(12-12)KillShellToolRenderer(13-13)TaskToolRenderer(14-14)LSToolRenderer(15-15)
src/features/workspace/components/chat/tools/components/CopyButton.tsx (2)
src/features/workspace/components/chat/tools/components/index.ts (1)
CopyButton(1-1)src/lib/utils.ts (1)
cn(4-6)
| ## 📦 Dependencies to Add | ||
|
|
||
| ```json | ||
| { | ||
| "dependencies": { | ||
| "react-syntax-highlighter": "^15.5.0", | ||
| "diff": "^5.1.0", | ||
| "framer-motion": "^10.16.4" | ||
| }, | ||
| "devDependencies": { | ||
| "@types/react-syntax-highlighter": "^15.5.11", | ||
| "@types/diff": "^5.0.8" | ||
| } | ||
| } | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
Update dependency versions to match actual implementation.
The proposed framer-motion version (^10.16.4) appears outdated. Based on the PR timeline (October 2025) and available package information, the actual implementation likely uses a more recent version (12.x). Verify and update the dependency versions in this proposal to match what was actually added to ensure the documentation remains accurate.
🏁 Script executed:
#!/bin/bash
# Check the actual framer-motion version used in the project
cat package.json | jq '.dependencies["framer-motion"]'Length of output: 11
🏁 Script executed:
#!/bin/bash
# Check all dependencies listed in the proposal against actual package.json
cat package.json | jq '.dependencies, .devDependencies' | grep -E '(react-syntax-highlighter|diff|framer-motion|@types/react-syntax-highlighter|@types/diff)'Length of output: 31
🏁 Script executed:
#!/bin/bash
# Get complete dependencies and devDependencies objects
echo "=== DEPENDENCIES ===" && cat package.json | jq '.dependencies' && echo "=== DEV DEPENDENCIES ===" && cat package.json | jq '.devDependencies'Length of output: 1867
Update the "Dependencies to Add" section to reflect actual implementation.
The proposal lists five dependencies, but only framer-motion is in the actual package.json. The other four packages—react-syntax-highlighter, diff, @types/react-syntax-highlighter, and @types/diff—were never added to the project. Additionally, the framer-motion version in the proposal (^10.16.4) is outdated; the actual version is ^12.23.24.
Either remove the unimplemented dependencies from this section or update it to accurately reflect which dependencies were actually integrated.
🤖 Prompt for AI Agents
In CHAT_REFACTOR_PROPOSAL.md around lines 660 to 674, the "Dependencies to Add"
list does not match the repo: four packages (react-syntax-highlighter, diff,
@types/react-syntax-highlighter, @types/diff) were never added and
framer-motion's version is outdated; update this section to accurately reflect
the implemented dependency list by either removing the unimplemented packages or
replacing them with the actual dependencies present in package.json, and correct
the framer-motion version to ^12.23.24 so the proposal matches the codebase.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/hooks/useMessages.ts (2)
31-47: Gate verbose error logs behind DEV and redact content.Current logs may leak sensitive data in production. Wrap in a DEV check and avoid printing full previews.
- console.error('[useMessages] ❌ JSON parse error:', { - error: error instanceof Error ? error.message : String(error), - contentLength: content.length, - contentPreview: content.substring(0, 200), - }); + if (import.meta.env.DEV) { + console.error('[useMessages] ❌ JSON parse error:', { + error: error instanceof Error ? error.message : String(error), + contentLength: content.length, + contentPreview: content.substring(0, 200), + }); + } else { + console.error('[useMessages] ❌ JSON parse error (redacted). length=', content.length); + } @@ - console.error('[useMessages] Context around error position:', { - position, - before: content.substring(Math.max(0, position - 50), position), - after: content.substring(position, Math.min(content.length, position + 50)) - }); + if (import.meta.env.DEV) { + console.error('[useMessages] Context around error position:', { + position, + before: content.substring(Math.max(0, position - 50), position), + after: content.substring(position, Math.min(content.length, position + 50)) + }); + }
83-91: Handle non-2xx responses from fetch.Check response.ok before .json() to avoid runtime errors and surface HTTP failures cleanly.
- const sessionRes = await fetch(`${await getBaseURL()}/sessions/${sessionId}`); - const sessionData = await sessionRes.json(); + const sessionRes = await fetch(`${await getBaseURL()}/sessions/${sessionId}`); + if (!sessionRes.ok) throw new Error(`Status ${sessionRes.status} loading session`); + const sessionData = await sessionRes.json(); @@ - const res = await fetch(`${await getBaseURL()}/sessions/${sessionId}/messages`); - const data = await res.json(); + const res = await fetch(`${await getBaseURL()}/sessions/${sessionId}/messages`); + if (!res.ok) throw new Error(`Status ${res.status} loading messages`); + const data = await res.json();Also applies to: 89-91
♻️ Duplicate comments (2)
src/features/workspace/components/chat/tools/renderers/WriteToolRenderer.tsx (1)
7-9: Use a semantic button for the toggle and wire ARIA.Clickable div blocks keyboard users. Switch to a native button with aria-expanded/aria-controls; add an id to the panel.
As per coding guidelines.
-import { useState } from 'react'; +import { useState, useId } from 'react'; @@ export function WriteToolRenderer({ toolUse, toolResult }: ToolRendererProps) { const [isExpanded, setIsExpanded] = useState(true); + const contentId = useId(); @@ - <div - className={cn( - chatTheme.blocks.tool.header, - 'cursor-pointer hover:bg-muted/50 p-2 rounded transition-colors justify-between' - )} - onClick={() => setIsExpanded(!isExpanded)} - > + <button + type="button" + className={cn( + chatTheme.blocks.tool.header, + 'w-full text-left hover:bg-muted/50 p-2 rounded transition-colors justify-between', + 'focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring' + )} + aria-expanded={isExpanded} + aria-controls={contentId} + onClick={() => setIsExpanded((v) => !v)} + > @@ - </div> + </button> @@ - {isExpanded && ( + {isExpanded && ( <motion.div + id={contentId} initial={{ height: 0, opacity: 0 }} animate={{ height: 'auto', opacity: 1 }} exit={{ height: 0, opacity: 0 }} transition={{ duration: 0.2, ease: [0.23, 1, 0.32, 1] }} className="overflow-hidden" >Also applies to: 33-39, 62-70
src/features/workspace/components/chat/tools/renderers/ReadToolRenderer.tsx (1)
7-9: Use a semantic button for the toggle and wire ARIA.Mirror the accessibility fix: button + aria-expanded/controls, and id on the collapsible panel.
As per coding guidelines.
-import { useState } from 'react'; +import { useState, useId } from 'react'; @@ export function ReadToolRenderer({ toolUse, toolResult }: ToolRendererProps) { const [isExpanded, setIsExpanded] = useState(false); // Collapsed by default + const contentId = useId(); @@ - <div - className={cn( - chatTheme.blocks.tool.header, - 'cursor-pointer hover:bg-muted/50 p-2 rounded transition-colors justify-between' - )} - onClick={() => setIsExpanded(!isExpanded)} - > + <button + type="button" + className={cn( + chatTheme.blocks.tool.header, + 'w-full text-left hover:bg-muted/50 p-2 rounded transition-colors justify-between', + 'focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring' + )} + aria-expanded={isExpanded} + aria-controls={contentId} + onClick={() => setIsExpanded((v) => !v)} + > @@ - </div> + </button> @@ - <motion.div + <motion.div + id={contentId} initial={{ height: 0, opacity: 0 }} animate={{ height: 'auto', opacity: 1 }} exit={{ height: 0, opacity: 0 }}Also applies to: 33-39, 72-79
🧹 Nitpick comments (7)
src/features/workspace/components/chat/tools/renderers/BashOutputToolRenderer.tsx (1)
93-93: Consider using spring animation with ease-out.The animation uses a custom easing curve and duration-based transition, while guidelines recommend defaulting to spring animations and ease-out easing for most cases.
As per coding guidelines
Apply this diff to align with animation guidelines:
- transition={{ duration: 0.2, ease: [0.23, 1, 0.32, 1] }} + transition={{ type: 'spring', stiffness: 300, damping: 30 }}Or if you prefer duration-based for predictable expand/collapse timing, at least use the standard ease-out:
- transition={{ duration: 0.2, ease: [0.23, 1, 0.32, 1] }} + transition={{ duration: 0.2, ease: 'easeOut' }}src/features/workspace/components/chat/tools/utils/detectLanguage.ts (1)
11-15: Broaden detection: add well-known basenames and more extensions.Extend mapping and handle files without an extension (Dockerfile, Makefile, .env, etc.) so syntax highlighting is more accurate.
Apply diff:
export function detectLanguageFromPath(path: string): string { - const ext = path.split('.').pop()?.toLowerCase(); + const basename = path.split(/[\\/]/).pop()?.toLowerCase() || ''; + const ext = basename.includes('.') ? basename.split('.').pop()!.toLowerCase() : undefined; const languageMap: Record<string, string> = { @@ - // Markdown + // Markdown md: 'markdown', mdx: 'markdown', - // Other + // Front-end frameworks + vue: 'vue', + svelte: 'svelte', + astro: 'astro', + + // Other languages sql: 'sql', + cs: 'csharp', + dart: 'dart', + scala: 'scala', + r: 'r', + jl: 'julia', + zig: 'zig', rb: 'ruby', php: 'php', swift: 'swift', + + // Data / schemas + proto: 'protobuf', + graphql: 'graphql', + gql: 'graphql', + + // Shell & scripts + bat: 'bat', + ps1: 'powershell', + ksh: 'bash', + fish: 'fish', + + // Config + ini: 'ini', + conf: 'ini', + properties: 'properties', + gradle: 'groovy', }; - return languageMap[ext || ''] || 'text'; + // Special basenames with no extension + const basenameMap: Record<string, string> = { + dockerfile: 'docker', + makefile: 'makefile', + 'cmakelists.txt': 'cmake', + procfile: 'text', + justfile: 'makefile', + 'readme': 'markdown', + 'license': 'text', + '.env': 'ini', + }; + + if (basename in basenameMap) return basenameMap[basename]; + return (ext && languageMap[ext]) || 'text'; }Also applies to: 70-78
src/features/workspace/components/chat/blocks/BlockRenderer.tsx (1)
34-67: Make the switch exhaustive to catch new block types at build time.Add a compile-time exhaustiveness check; keep the runtime fallback if you want extra resilience in prod.
switch (block.type) { @@ case 'thinking': return <ThinkingBlock block={block} />; default: + // TS exhaustive check (will error if a new type is added to ContentBlock) + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const _exhaustiveCheck: never = block as never; // Graceful fallback for unknown block types if (import.meta.env.DEV) { console.warn('[BlockRenderer] Unknown block type:', (block as any).type, block); }src/features/workspace/components/chat/tools/renderers/WriteToolRenderer.tsx (1)
61-70: Optional: align motion config with defaults.Consider a spring for expand/collapse (or unify easing via theme token) to match animation guidelines.
src/features/workspace/components/chat/tools/renderers/WebSearchToolRenderer.tsx (2)
8-10: Use a semantic button for the toggle and wire ARIA.Convert the header to a button and bind aria-expanded/controls; add an id to the panel.
-import { useState } from 'react'; +import { useState, useId } from 'react'; @@ export function WebSearchToolRenderer({ toolUse, toolResult }: ToolRendererProps) { const [isExpanded, setIsExpanded] = useState(false); // Collapsed by default + const contentId = useId(); @@ - <div - className={cn( - chatTheme.blocks.tool.header, - 'cursor-pointer hover:bg-muted/50 p-2 rounded transition-colors justify-between' - )} - onClick={() => setIsExpanded(!isExpanded)} - > + <button + type="button" + className={cn( + chatTheme.blocks.tool.header, + 'w-full text-left hover:bg-muted/50 p-2 rounded transition-colors justify-between', + 'focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring' + )} + aria-expanded={isExpanded} + aria-controls={contentId} + onClick={() => setIsExpanded((v) => !v)} + > @@ - </div> + </button> @@ - <motion.div + <motion.div + id={contentId} initial={{ height: 0, opacity: 0 }} animate={{ height: 'auto', opacity: 1 }} exit={{ height: 0, opacity: 0 }}Also applies to: 38-44, 111-118
121-130: Optional: render structured results as links if available.If toolResult.content is an array of { title, url } objects, show a list with anchors and Copy buttons. Falls back to plain preformatted text otherwise.
src/hooks/useMessages.ts (1)
59-78: Optional: avoid rebuilding the map on every poll.For large histories, consider caching parsed content per message.id or incrementally updating the map as new messages arrive.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (10)
src/features/workspace/components/chat/blocks/BlockRenderer.tsx(1 hunks)src/features/workspace/components/chat/tools/components/CopyButton.tsx(1 hunks)src/features/workspace/components/chat/tools/registerTools.ts(1 hunks)src/features/workspace/components/chat/tools/renderers/BashOutputToolRenderer.tsx(1 hunks)src/features/workspace/components/chat/tools/renderers/ReadToolRenderer.tsx(1 hunks)src/features/workspace/components/chat/tools/renderers/WebFetchToolRenderer.tsx(1 hunks)src/features/workspace/components/chat/tools/renderers/WebSearchToolRenderer.tsx(1 hunks)src/features/workspace/components/chat/tools/renderers/WriteToolRenderer.tsx(1 hunks)src/features/workspace/components/chat/tools/utils/detectLanguage.ts(1 hunks)src/hooks/useMessages.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/features/workspace/components/chat/tools/registerTools.ts
- src/features/workspace/components/chat/tools/components/CopyButton.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{tsx,jsx}: Use Tailwind for styling in UI components (avoid custom CSS when a Tailwind utility or token exists)
Base UI on shadcn/ui components
Split UI into reusable, focused components
When using Framer Motion, prefer transform-based animations over x/y to leverage hardware acceleration
Default to spring animations when using Framer Motion
Avoid bouncy spring configurations in Framer Motion unless for drag gestures
Files:
src/features/workspace/components/chat/tools/renderers/WebSearchToolRenderer.tsxsrc/features/workspace/components/chat/tools/renderers/WriteToolRenderer.tsxsrc/features/workspace/components/chat/tools/renderers/BashOutputToolRenderer.tsxsrc/features/workspace/components/chat/tools/renderers/ReadToolRenderer.tsxsrc/features/workspace/components/chat/tools/renderers/WebFetchToolRenderer.tsxsrc/features/workspace/components/chat/blocks/BlockRenderer.tsx
**/*.{css,scss,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{css,scss,tsx,jsx}: Use consistent paddings with a 16px default
Use consistent font sizes across the app
Avoid hardcoding colors; always use Tailwind color tokens
Default to ease-out for most animations
Animations should be ≤1s; most between 200ms–300ms
Animate elements from their trigger; set transform-origin based on trigger position
Files:
src/features/workspace/components/chat/tools/renderers/WebSearchToolRenderer.tsxsrc/features/workspace/components/chat/tools/renderers/WriteToolRenderer.tsxsrc/features/workspace/components/chat/tools/renderers/BashOutputToolRenderer.tsxsrc/features/workspace/components/chat/tools/renderers/ReadToolRenderer.tsxsrc/features/workspace/components/chat/tools/renderers/WebFetchToolRenderer.tsxsrc/features/workspace/components/chat/blocks/BlockRenderer.tsx
🧬 Code graph analysis (5)
src/features/workspace/components/chat/tools/renderers/WebSearchToolRenderer.tsx (3)
src/features/workspace/components/chat/tools/renderers/index.ts (1)
WebSearchToolRenderer(12-12)src/features/workspace/components/chat/types.ts (1)
ToolRendererProps(18-21)src/lib/utils.ts (1)
cn(4-6)
src/features/workspace/components/chat/tools/renderers/WriteToolRenderer.tsx (5)
src/features/workspace/components/chat/types.ts (1)
ToolRendererProps(18-21)src/lib/utils.ts (1)
cn(4-6)src/features/workspace/components/chat/tools/components/FilePathDisplay.tsx (1)
FilePathDisplay(15-57)src/features/workspace/components/chat/tools/components/CodeBlock.tsx (1)
CodeBlock(20-50)src/features/workspace/components/chat/tools/utils/detectLanguage.ts (1)
detectLanguageFromPath(11-78)
src/features/workspace/components/chat/tools/renderers/ReadToolRenderer.tsx (5)
src/features/workspace/components/chat/types.ts (1)
ToolRendererProps(18-21)src/lib/utils.ts (1)
cn(4-6)src/features/workspace/components/chat/tools/components/FilePathDisplay.tsx (1)
FilePathDisplay(15-57)src/features/workspace/components/chat/tools/components/CodeBlock.tsx (1)
CodeBlock(20-50)src/features/workspace/components/chat/tools/utils/detectLanguage.ts (1)
detectLanguageFromPath(11-78)
src/features/workspace/components/chat/tools/renderers/WebFetchToolRenderer.tsx (3)
src/features/workspace/components/chat/tools/renderers/index.ts (1)
WebFetchToolRenderer(11-11)src/features/workspace/components/chat/types.ts (1)
ToolRendererProps(18-21)src/lib/utils.ts (1)
cn(4-6)
src/hooks/useMessages.ts (2)
backend/server.cjs (1)
messages(653-657)src-tauri/sidecar/index.cjs (1)
message(104-104)
🔇 Additional comments (4)
src/features/workspace/components/chat/tools/renderers/BashOutputToolRenderer.tsx (2)
98-107: Good fixes from previous review!The previous max-height conflict (Tailwind class vs. inline style) and hardcoded colors (
bg-black/90,text-green-400) have been properly resolved. The code now uses only the Tailwindmax-h-96class and semantic color tokens (bg-sidebar-accent/90,text-success).
18-18: ****The original concern about missing input validation is unfounded. The
ToolRendererPropsinterface requirestoolUse: ToolUseBlock, andToolUseBlock.inputis a required field typed asRecord<string, any>. TypeScript's type system guarantees thattoolUse.inputis always defined when the component receives props, making the destructuring on line 18 type-safe with no risk of runtime error.Likely an incorrect or invalid review comment.
src/features/workspace/components/chat/tools/renderers/ReadToolRenderer.tsx (1)
61-68: LGTM on offset/limit display.Explicit undefined checks and correct pluralization fix the 0-value issue.
src/features/workspace/components/chat/tools/utils/detectLanguage.ts (1)
15-21: The review comment is based on incorrect assumptions about the codebase architecture.The review assumes
react-syntax-highlighteris integrated with either Prism or Highlight.js engine, but the actual implementation uses a customSyntaxHighlightercomponent without any external syntax highlighting library. The custom component doesn't validate language identifiers against external libraries—it currently returns plain text for all code. ThedetectLanguageFromPathfunction outputs (e.g.,'typescript','javascript') are placeholder values for future integration with Prism or Shiki, not current requirements tied to an active highlighter configuration.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/features/workspace/components/chat/tools/renderers/WriteToolRenderer.tsx (2)
34-61: Excellent accessibility improvements! One minor setState refinement remains.The button implementation successfully addresses nearly all accessibility concerns from the previous review:
- ✓ Semantic
<button>element- ✓ ARIA expanded/controls attributes
- ✓ Focus-visible styles
- ✓ Decorative icons marked
aria-hiddenLine 43 could use the functional setState form for better practice:
- onClick={() => setIsExpanded(!isExpanded)} + onClick={() => setIsExpanded((v) => !v)}
67-102: Expandable content is well-implemented with proper ARIA wiring.The animated content section correctly uses
id={contentId}to complete the ARIA relationship with the header button'saria-controlsattribute. The CodeBlock integration with language detection and the error display handling both work well.Note: The animation uses
heighttransitions rather than transform-based animations as suggested in the coding guidelines. While transforms generally perform better, animating height for collapse/expand patterns is a standard approach and difficult to replace without complex workarounds. Given the brief 200ms duration, performance impact is negligible.As per coding guidelines.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (6)
src/features/workspace/components/chat/tools/renderers/BashOutputToolRenderer.tsx(1 hunks)src/features/workspace/components/chat/tools/renderers/ReadToolRenderer.tsx(1 hunks)src/features/workspace/components/chat/tools/renderers/WebFetchToolRenderer.tsx(1 hunks)src/features/workspace/components/chat/tools/renderers/WebSearchToolRenderer.tsx(1 hunks)src/features/workspace/components/chat/tools/renderers/WriteToolRenderer.tsx(1 hunks)src/hooks/useMessages.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/features/workspace/components/chat/tools/renderers/BashOutputToolRenderer.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{tsx,jsx}: Use Tailwind for styling in UI components (avoid custom CSS when a Tailwind utility or token exists)
Base UI on shadcn/ui components
Split UI into reusable, focused components
When using Framer Motion, prefer transform-based animations over x/y to leverage hardware acceleration
Default to spring animations when using Framer Motion
Avoid bouncy spring configurations in Framer Motion unless for drag gestures
Files:
src/features/workspace/components/chat/tools/renderers/WriteToolRenderer.tsxsrc/features/workspace/components/chat/tools/renderers/WebSearchToolRenderer.tsxsrc/features/workspace/components/chat/tools/renderers/ReadToolRenderer.tsxsrc/features/workspace/components/chat/tools/renderers/WebFetchToolRenderer.tsx
**/*.{css,scss,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{css,scss,tsx,jsx}: Use consistent paddings with a 16px default
Use consistent font sizes across the app
Avoid hardcoding colors; always use Tailwind color tokens
Default to ease-out for most animations
Animations should be ≤1s; most between 200ms–300ms
Animate elements from their trigger; set transform-origin based on trigger position
Files:
src/features/workspace/components/chat/tools/renderers/WriteToolRenderer.tsxsrc/features/workspace/components/chat/tools/renderers/WebSearchToolRenderer.tsxsrc/features/workspace/components/chat/tools/renderers/ReadToolRenderer.tsxsrc/features/workspace/components/chat/tools/renderers/WebFetchToolRenderer.tsx
🧬 Code graph analysis (5)
src/features/workspace/components/chat/tools/renderers/WriteToolRenderer.tsx (4)
src/lib/utils.ts (1)
cn(4-6)src/features/workspace/components/chat/tools/components/FilePathDisplay.tsx (1)
FilePathDisplay(15-57)src/features/workspace/components/chat/tools/components/CodeBlock.tsx (1)
CodeBlock(20-50)src/features/workspace/components/chat/tools/utils/detectLanguage.ts (1)
detectLanguageFromPath(11-78)
src/features/workspace/components/chat/tools/renderers/WebSearchToolRenderer.tsx (3)
src/features/workspace/components/chat/tools/renderers/index.ts (1)
WebSearchToolRenderer(12-12)src/features/workspace/components/chat/types.ts (1)
ToolRendererProps(18-21)src/lib/utils.ts (1)
cn(4-6)
src/hooks/useMessages.ts (3)
backend/lib/message-sanitizer.cjs (1)
position(119-119)src-tauri/sidecar/index.cjs (1)
message(104-104)src/config/api.config.ts (1)
getBaseURL(147-150)
src/features/workspace/components/chat/tools/renderers/ReadToolRenderer.tsx (6)
src/features/workspace/components/chat/tools/renderers/index.ts (1)
ReadToolRenderer(5-5)src/lib/utils.ts (1)
cn(4-6)src/features/workspace/components/chat/tools/components/index.ts (2)
FilePathDisplay(3-3)CodeBlock(2-2)src/features/workspace/components/chat/tools/components/FilePathDisplay.tsx (1)
FilePathDisplay(15-57)src/features/workspace/components/chat/tools/components/CodeBlock.tsx (1)
CodeBlock(20-50)src/features/workspace/components/chat/tools/utils/detectLanguage.ts (1)
detectLanguageFromPath(11-78)
src/features/workspace/components/chat/tools/renderers/WebFetchToolRenderer.tsx (2)
src/features/workspace/components/chat/types.ts (1)
ToolRendererProps(18-21)src/lib/utils.ts (1)
cn(4-6)
🔇 Additional comments (16)
src/features/workspace/components/chat/tools/renderers/WriteToolRenderer.tsx (2)
1-15: LGTM! Clean imports and good accessibility setup.The component imports are well-organized. Notably,
useIdis properly imported and used for generating stable IDs for ARIA attributes, which addresses the accessibility improvements mentioned in the previous review.
17-23: LGTM! Clean component setup with proper accessibility hooks.The component setup is well-structured. The use of
useId()ensures stable IDs for ARIA relationships, and defaulting to expanded state provides good initial UX.src/hooks/useMessages.ts (4)
25-58: Enhanced error handling looks solid.The DEV/PROD separation properly balances debugging needs with security. The control character position extraction and context logging will be helpful for debugging malformed JSON.
60-83: LGTM! Tool result linking implemented correctly.The useMemo dependencies are appropriate since
parseContentis stable (useCallback with no dependencies). This effectively enables the ToolUseBlock → ToolResultBlock linking described in the PR objectives.
85-151: Improved error handling and loading state management.The stricter HTTP status checks prevent silent failures, and the added
setLoading(false)at line 149 ensures the loading state always resolves, even on error.
226-241: Public API correctly expanded.The addition of
toolResultMapto the hook's return value provides the necessary data for downstream tool rendering components.src/features/workspace/components/chat/tools/renderers/ReadToolRenderer.tsx (3)
34-61: Excellent accessibility implementation.The semantic button with proper ARIA attributes (
aria-expanded,aria-controls) and focus-visible styles ensures keyboard users can interact with the collapsible content. ThecontentIdat line 79 correctly matches thearia-controlsattribute.
67-73: Correct handling of 0 values for offset/limit.The explicit
!== undefinedchecks ensure that valid0values are displayed correctly, and the pluralization logic is accurate.
76-114: Well-structured expandable content with proper animations.The animation configuration (0.2s duration with ease-out curve) follows the coding guidelines, and the CodeBlock integration with language detection enhances readability. Error handling appropriately stringifies object content for display.
As per coding guidelines.
src/features/workspace/components/chat/tools/renderers/WebSearchToolRenderer.tsx (3)
39-76: Proper semantic button with accessibility support.The header button includes appropriate ARIA attributes and focus-visible styles, maintaining consistency with other tool renderers in the PR.
115-139: Clean styling without redundancy.The duplicate
maxHeightinline style has been properly removed, leaving only the Tailwindmax-h-96utility class. The animation configuration aligns with the guidelines.As per coding guidelines.
78-158: Well-organized display of search parameters and results.The domain filter rendering and error handling follow consistent patterns. The semantic color coding (success for allowed, destructive for blocked) enhances clarity.
src/features/workspace/components/chat/tools/renderers/WebFetchToolRenderer.tsx (4)
39-73: Semantic button with proper ARIA implementation.The collapsible header uses a semantic button element with appropriate ARIA attributes and focus-visible styles for keyboard accessibility.
77-102: Excellent URL sanitization preventing XSS.The URL validation correctly restricts rendering to http/https protocols only, blocking javascript: and other potentially malicious schemes. The try-catch with plain text fallback handles invalid URLs gracefully.
112-136: Clean animation with no style duplication.The duplicate
maxHeightinline style has been removed, leaving only the Tailwind utility class. The animation configuration (0.2s with ease curve) follows the guidelines.As per coding guidelines.
103-157: Comprehensive display of fetch parameters and results.The task display, no-result message, and error handling follow consistent patterns across the tool renderers. The implementation properly handles both string and object content types.
…mentation Fixes 9 out of 10 code review issues (#1-5, #7, #9-10): - Remove dead Claude transformer accumulation (Issue #1) - Add graceful fallback for full_message column detection (Issue #2) - Use process.argv[1] for bundle-safe vendor path resolution (Issue #3) - Static import of execSync instead of dynamic require (Issue #4) - Add platform support guard instead of silently mapping to Linux (Issue #5) - Add null check on child.stdout before readline (Issue #7) - Add identity check in finally block to prevent race condition (Issue #9) - Capture thread_id for Codex message correlation (Issue #10) The in-place mutation in codex-adapter.ts (Issue #6) is verified safe because blocks are JSON-serialized before reaching the frontend, making the mutation internal to adapter lifecycle and invisible to consumers. All 193 sidecar tests pass. Zero type errors. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
…mentation Fixes 9 out of 10 code review issues (#1-5, #7, #9-10): - Remove dead Claude transformer accumulation (Issue #1) - Add graceful fallback for full_message column detection (Issue #2) - Use process.argv[1] for bundle-safe vendor path resolution (Issue #3) - Static import of execSync instead of dynamic require (Issue #4) - Add platform support guard instead of silently mapping to Linux (Issue #5) - Add null check on child.stdout before readline (Issue #7) - Add identity check in finally block to prevent race condition (Issue #9) - Capture thread_id for Codex message correlation (Issue #10) The in-place mutation in codex-adapter.ts (Issue #6) is verified safe because blocks are JSON-serialized before reaching the frontend, making the mutation internal to adapter lifecycle and invisible to consumers. All 193 sidecar tests pass. Zero type errors. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
…mentation Fixes 9 out of 10 code review issues (#1-5, #7, #9-10): - Remove dead Claude transformer accumulation (Issue #1) - Add graceful fallback for full_message column detection (Issue #2) - Use process.argv[1] for bundle-safe vendor path resolution (Issue #3) - Static import of execSync instead of dynamic require (Issue #4) - Add platform support guard instead of silently mapping to Linux (Issue #5) - Add null check on child.stdout before readline (Issue #7) - Add identity check in finally block to prevent race condition (Issue #9) - Capture thread_id for Codex message correlation (Issue #10) The in-place mutation in codex-adapter.ts (Issue #6) is verified safe because blocks are JSON-serialized before reaching the frontend, making the mutation internal to adapter lifecycle and invisible to consumers. All 193 sidecar tests pass. Zero type errors. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
1. **inject-mode.ts: var() scan now covers padding/gap (#3)** Renamed colorProps → varScanProps, added padding/gap so getMatchedVarDeclarations captures var() tokens for spacing props. 2. **inject-mode.ts: font-weight fallback added (#10)** Added font-weight to the fallback conditional alongside font-size and border-radius. Filters out 'normal' (CSS default) as noise. 3. **BrowserPanel.tsx: DevTools state races with async invoke (#4)** Moved handleUpdateTab into .then() so devtoolsOpen only updates after the Rust command succeeds. Prevents UI desync on failure. 4. **BrowserTab.tsx: safeListen missing .catch() (#5)** Added .catch() to the listen() promise chain to prevent unhandled rejections if Tauri listener registration fails. 5. **BrowserTab.tsx: injectionFailed not cleared on success (#7)** Changed onUpdateTab to also set injectionFailed: false after successful injection, clearing stale error indicators. 6. **webview.rs: eprintln! gated for release builds (#8)** All 4 eprintln! calls in eval_browser_webview_with_result wrapped with cfg!(debug_assertions). Prevents leaking JS content, URLs, tokens, or page console data in production. 7. **webview.rs: DevTools error propagation (#9)** open/close_browser_devtools now use Arc<Mutex<Option<String>>> to propagate null-pointer errors out of with_webview closures. Frontend receives Err() instead of silent Ok(()) when DevTools unavailable. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* feat: Fix browser inspect element pills with code quality improvements ## Changes ### 1. Fix DevTools Expansion Bug (Tauri Native Layer) When clicking the DevTools button, the browser view would unexpectedly expand. Root cause: `_inspector.show()` docks by splitting WKWebView's superview, resizing the webview. After `detach()` moves inspector to floating window, frame isn't restored. **Fix:** Save WKWebView frame before show(), restore after detach(). - Modified: src-tauri/src/commands/webview.rs (lines 570-582) ### 2. Fix Cursor Animation Attribute Mismatch (Issue #1 - HIGH) Cursor visual effects (move, ripple, pin) were silently failing because they searched for `data-cursor-ref` but elements were marked with `data-hive-ref`. **Fix:** Replace all `data-cursor-ref` references with `data-hive-ref` to match the actual attribute used in inject-mode.ts. - Modified: src/features/browser/automation/visual-effects.ts - Line 18: JSDoc comment - Line 25: buildMoveCursorAndRippleJs() - Line 45: buildPinCursorJs() - Line 96: buildHighlightElementJs() ### 3. Add IME Composition Guard to MessageInput (Issue #2 - MEDIUM) CJK (Chinese, Japanese, Korean) users using Input Method Editors (IME) would lose their input when pressing Enter during composition, since the handler didn't check if composition was in progress. **Fix:** Add `!e.nativeEvent.isComposing` guard to Enter key handler to prevent message send during active IME composition. - Modified: src/features/session/ui/MessageInput.tsx (line 286) ### 4. Fix Element Selector Injection Exception Handling (Issue #3 - MEDIUM) When `verify_injection` script threw an exception, the catch block didn't return, causing exception object to fall through to `eval_browser_webview_with_result`, which would parse it as a result string instead of error. **Fix:** Add explicit `return;` in catch block to prevent fall-through. - Modified: src/features/browser/ui/BrowserTab.tsx (line 521) ### 5. Fix Selector Toggle State on Eval Failure (Issue #4 - MEDIUM) When element selector injection failed, `selectorActive` was set in the UI store but outside the try block, so errors during injection were silently swallowed. **Fix:** Move state update inside try block to fail-fast on injection errors. - Modified: src/features/browser/ui/BrowserTab.tsx (line 563) ### 6. Add Polling Guard to Inspect Event Drain (Issue #5 - MEDIUM) Inspect event drain was polling unconditionally every 200ms, consuming ~5 IPC calls/second unnecessarily. With multiple tabs this compounds quickly. **Fix:** Only poll when selector is active. Add `!tab.selectorActive` guard and add to dependency array. - Modified: src/features/browser/ui/BrowserTab.tsx (lines 396-458) ### 7. Replace if/else with ts-pattern (Issue #6 - LOW) Long if/else chain for event type dispatch violates CLAUDE.md requirement to use ts-pattern for discriminated union dispatch. **Fix:** Import ts-pattern and use `.with()` / `.otherwise()` for event type matching. - Modified: src/features/browser/ui/BrowserTab.tsx (lines 26, 427-444) ### 8. Fix Hover Animation Timing (Issue #7 - LOW) InspectedElementCard hover transition was `duration-150` instead of `duration-200`, violating CLAUDE.md animation standard of 200-300ms default duration. **Fix:** Update to `duration-200 ease` to match project animation guidelines. - Modified: src/features/session/ui/InspectedElementCard.tsx (line 48) ## Verification - TypeScript compilation: ✓ (pre-existing Lucide icon issue unrelated to changes) - All fixes verified and applied by parallel code-reviewer agents - Changes preserve existing functionality while fixing correctness issues - Performance improvement: Eliminates ~5 IPC calls/second from unconditional polling ## Impact - **Bug fixes:** 3 (DevTools expansion, cursor animations, IME composition) - **Correctness improvements:** 2 (exception handling, state sync) - **Performance:** Reduced IPC quota consumption from inspect event drain - **Code quality:** Full CLAUDE.md compliance (ts-pattern, animation timing) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: Address PR review — ts-pattern console dispatch + comment consistency 1. Replace nested ternary on log.l with ts-pattern match() in console drain loop (BrowserTab.tsx:365). Consistent with inspect event drain at line 428 which already uses match(). CLAUDE.md requires ts-pattern for all discriminator field dispatch. 2. Fix contradictory architecture comments across 3 files: - inject/inspect-mode.ts: Remove "DUAL mechanism: PRIMARY title-channel" language and phantom serialization queue details. Buffer+drain via eval_browser_webview_with_result is the sole path. - BrowserTab.tsx: Change "FALLBACK path" to "sole delivery path". - webview.rs: Remove reference to non-existent sendViaTitle(), mark title-channel handler as backward-compat only. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: Address PR review — 7 issues across 4 files 1. **inject-mode.ts: var() scan now covers padding/gap (#3)** Renamed colorProps → varScanProps, added padding/gap so getMatchedVarDeclarations captures var() tokens for spacing props. 2. **inject-mode.ts: font-weight fallback added (#10)** Added font-weight to the fallback conditional alongside font-size and border-radius. Filters out 'normal' (CSS default) as noise. 3. **BrowserPanel.tsx: DevTools state races with async invoke (#4)** Moved handleUpdateTab into .then() so devtoolsOpen only updates after the Rust command succeeds. Prevents UI desync on failure. 4. **BrowserTab.tsx: safeListen missing .catch() (#5)** Added .catch() to the listen() promise chain to prevent unhandled rejections if Tauri listener registration fails. 5. **BrowserTab.tsx: injectionFailed not cleared on success (#7)** Changed onUpdateTab to also set injectionFailed: false after successful injection, clearing stale error indicators. 6. **webview.rs: eprintln! gated for release builds (#8)** All 4 eprintln! calls in eval_browser_webview_with_result wrapped with cfg!(debug_assertions). Prevents leaking JS content, URLs, tokens, or page console data in production. 7. **webview.rs: DevTools error propagation (#9)** open/close_browser_devtools now use Arc<Mutex<Option<String>>> to propagate null-pointer errors out of with_webview closures. Frontend receives Err() instead of silent Ok(()) when DevTools unavailable. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(ci): build inject scripts before sidecar unit tests browser-templates.test.ts imports browser-utils.ts which reads dist-inject/browser-utils.js via ?raw import. The dist-inject/ directory is gitignored and must be built first. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(test): update browser-templates tests for inject refactor - Remove dead BROWSER_UTILS import (renamed to BROWSER_UTILS_SETUP) - Remove incorrect buildPressKeyJs BROWSER_UTILS test (it intentionally doesn't use browser utils — operates on document.activeElement directly) - Update assertIsIIFE to accept both classic IIFEs (function(){...})() and arrow IIFEs (() => {...})() — esbuild format: "iife" emits the latter for compiled inject scripts like VISUAL_EFFECTS_SETUP Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address code review round 3 — security, a11y, concurrency - Gate getReactProps() and getShallowInnerHTML() to local context only; external sites can leak PII/tokens via React fiber props and innerHTML - Remove 'value' from ATTR_WHITELIST (can contain passwords) - Add in-flight guards to console and inspect drain intervals to prevent overlapping async invokes when eval_browser_webview is slow - Add aria-hidden + sr-only text for injection failure indicator - Add aria-label to screenshot button - Remove duplicate comment blocks in inject-mode.ts - Fix stale "title-channel" comment in BrowserTab.tsx Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat(browser): add screenshot button + mobile viewport toggle Add two new browser panel toolbar features: 1. Screenshot button (Camera icon): captures WKWebView as JPEG via Rust IPC and attaches to chat input via CustomEvent bridge. 2. Mobile/Desktop viewport toggle (Smartphone/Monitor icon): constrains browser area to 390px (iPhone 14 logical width) centered with mx-auto. The mobile view required replacing the tab stacking strategy — absolute positioning (inset-0) didn't reliably inherit width constraints during parent restructuring, causing getBoundingClientRect() to return stale full-width values. Tabs now stack via CSS Grid ([grid-area:1/1]) which keeps them in normal document flow. A useLayoutEffect with hide → setBounds → show cycle ensures the native WKWebView repositions synchronously on toggle. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…mentation Fixes 9 out of 10 code review issues (#1-5, #7, #9-10): - Remove dead Claude transformer accumulation (Issue #1) - Add graceful fallback for full_message column detection (Issue #2) - Use process.argv[1] for bundle-safe vendor path resolution (Issue #3) - Static import of execSync instead of dynamic require (Issue #4) - Add platform support guard instead of silently mapping to Linux (Issue #5) - Add null check on child.stdout before readline (Issue #7) - Add identity check in finally block to prevent race condition (Issue #9) - Capture thread_id for Codex message correlation (Issue #10) The in-place mutation in codex-adapter.ts (Issue #6) is verified safe because blocks are JSON-serialized before reaching the frontend, making the mutation internal to adapter lifecycle and invisible to consumers. All 193 sidecar tests pass. Zero type errors. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
1. **inject-mode.ts: var() scan now covers padding/gap (#3)** Renamed colorProps → varScanProps, added padding/gap so getMatchedVarDeclarations captures var() tokens for spacing props. 2. **inject-mode.ts: font-weight fallback added (#10)** Added font-weight to the fallback conditional alongside font-size and border-radius. Filters out 'normal' (CSS default) as noise. 3. **BrowserPanel.tsx: DevTools state races with async invoke (#4)** Moved handleUpdateTab into .then() so devtoolsOpen only updates after the Rust command succeeds. Prevents UI desync on failure. 4. **BrowserTab.tsx: safeListen missing .catch() (#5)** Added .catch() to the listen() promise chain to prevent unhandled rejections if Tauri listener registration fails. 5. **BrowserTab.tsx: injectionFailed not cleared on success (#7)** Changed onUpdateTab to also set injectionFailed: false after successful injection, clearing stale error indicators. 6. **webview.rs: eprintln! gated for release builds (#8)** All 4 eprintln! calls in eval_browser_webview_with_result wrapped with cfg!(debug_assertions). Prevents leaking JS content, URLs, tokens, or page console data in production. 7. **webview.rs: DevTools error propagation (#9)** open/close_browser_devtools now use Arc<Mutex<Option<String>>> to propagate null-pointer errors out of with_webview closures. Frontend receives Err() instead of silent Ok(()) when DevTools unavailable. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…rom visual-effects.ts, un-exported 5 internal-only symbols (resolveClaudeDir, getAgentConfig, StatusPriority, StatusConfig, WorkflowStatusConfig), and removed dead barrel re-exports (createAgentEventHandler, AgentEventHandler) from agent/index.ts — net reduction of 98 lines across 5 files with all 825 tests passing and clean tsc.
* gnhf #1: Extracted two helper methods in claude-adapter.ts (closeActiveParts, accumulateStreamDelta) to eliminate 5 repeated close-text/close-thinking call sites and 2 duplicate 25-line streaming delta handlers; also replaced a duplicate 15-line formatTime function in WorkspaceItem.tsx with the existing shared formatTimeAgo utility — net reduction of 49 lines with all 375 agent-server tests passing. * gnhf #2: Extracted duplicate workspace-grouping logic (85 lines across query-engine.ts and workspaces.ts route) into shared lib/workspace-grouping.ts, and consolidated duplicate parameter-reading helpers (readString/readNumber across query-engine.ts and commands.ts) into shared lib/query-params.ts — net reduction of 41 lines with all 825 tests passing. * gnhf #3: Removed unused parameter from cancellation functions across both agent handlers, extracted helper replacing 5 identical repo-lookup patterns in repos.ts, unified duplicate git progress push functions, and removed dead export — net reduction of 23 lines across 7 files with all 825 tests passing. * gnhf #4: Removed 5 dead exports/functions and un-exported 2 internal-only functions across 6 files — net reduction of 112 lines with all 825 tests passing. * gnhf #5: Removed 2 dead exported functions (getRepoInitials, getRepoColor), un-exported 3 internal-only symbols (RECENT_PROJECT_LIMIT, resolveGitProjectRoot, setLastOpenInAppId), and replaced duplicate timeAgo in AccessSection.tsx with shared formatTimeAgo — net reduction of 41 lines across 5 files with all 825 tests passing. * gnhf #6: Un-exported 14 dead Zod schema validators (6 from shared/events.ts, 8 from shared/agent-events.ts), un-exported 6 dead type aliases from shared/events.ts, and consolidated duplicate parseGitHubRepo function from gh.service.ts and deus-import.ts into shared/lib/github.ts — reducing public API surface by 20 exports and eliminating 1 duplicate function across 5 files with all 825 tests passing. * gnhf #7: Deleted 2 dead component files (OpenInDropdown 214 lines, EmptyState 39 lines), removed 3 dead API type definitions (ApiResponse, PaginatedResponse, WorkspaceQueryParams) from shared/types/api.ts, removed 4 dead session type aliases (SessionMessageEvent, SessionErrorEvent, SessionEnterPlanModeEvent, SessionStatusEvent) from shared/types/session.ts, and cleaned up 3 barrel re-export files — net reduction of 330 lines across 7 files with all 825 tests passing. * gnhf #8: Deleted 2 dead platform files (updates.ts 31 lines, listenerGroup.ts 38 lines), removed dead function from dialog.ts, removed the entire dead StatusChanged notification pipeline across 5 files (method, schema, type, constant, test builder, union), un-exported from electron barrel, and cleaned up 3 barrel re-export files — net reduction of 121 lines across 11 files with all 825 tests passing. * gnhf #9: Removed 3 dead query hooks (useStats, useUncommittedFiles, useLastTurnFiles) with their stub service methods and query keys, removed 4 dead type definitions (ChangedFilesResult, BranchInfo, PaginationParams, DevServer) and their barrel re-exports, un-exported internal-only connectToRelay function, and cleaned up 2 dead barrel re-exports (clearToken, ConnectionIllustration) — net reduction of 121 lines across 12 files with all 825 tests passing and clean tsc. * gnhf #10: Removed 6 dead visual effect builder functions (98 lines) from visual-effects.ts, un-exported 5 internal-only symbols (resolveClaudeDir, getAgentConfig, StatusPriority, StatusConfig, WorkflowStatusConfig), and removed dead barrel re-exports (createAgentEventHandler, AgentEventHandler) from agent/index.ts — net reduction of 98 lines across 5 files with all 825 tests passing and clean tsc. * gnhf #11: Consolidated inline path validation in files.ts to use shared resolveWorkspaceRelativePath from git.service.ts, and replaced 10 hand-rolled readString+throw param validation patterns in commands.ts with the existing requireParam utility — net reduction of 17 lines across 2 files with all 825 tests passing and clean tsc. * gnhf #12: Removed 211 lines of dead CSS from global.css: the entire glitch-swap effect system (6 classes, 6 @Keyframes, 2 media queries) and the empty tool-use-enter class — none referenced by any component, with all 825 tests passing. * gnhf #13: Deleted 2 dead component files (RepoGroup.tsx 102 lines, WorkspaceItem.tsx 123 lines) from repository/ui that were never rendered anywhere, removed 14 dead barrel re-exports across 4 barrel files (repository/ui, sidebar/ui, sidebar feature, session/ui/blocks), and cleaned up knip.json — net reduction of 242 lines across 7 files with all 825 tests passing and clean tsc. * gnhf #14: Deleted the dead agent-server/messages/index.ts barrel (30 lines), removed 4 dead type exports from shared/messages/types.ts (ToolLocation, ToolOutputContent, PartType, PartTypeSchema), removed dead ToolResultMap type from chat-types.ts, and cleaned up 15 dead barrel re-exports across 4 barrel files (session/hooks, session/ui, workspace/ui, shared/hooks) — net reduction of 51 lines across 8 files with all 825 tests passing and clean tsc. * gnhf #15: Removed 10 dead type aliases from shared/agent-events.ts, un-exported 2 dead Zod schemas from shared/enums.ts, removed the dead useWindowFocus hook and its useSyncExternalStore infrastructure (50 lines) from useWindowFocus.ts, and removed dead BaseToolRendererProps barrel re-export — net reduction of 62 lines across 4 files with all 825 tests passing and clean tsc. * refactor: extract backend helpers and fix typecheck * Address CodeRabbit review feedback * Fix Claude adapter message handoff
…rom visual-effects.ts, un-exported 5 internal-only symbols (resolveClaudeDir, getAgentConfig, StatusPriority, StatusConfig, WorkflowStatusConfig), and removed dead barrel re-exports (createAgentEventHandler, AgentEventHandler) from agent/index.ts — net reduction of 98 lines across 5 files with all 825 tests passing and clean tsc.
1 participant
Background
The current chat implementation is monolithic and hard to extend. Tool interactions lack proper visualization and status feedback. This refactor aims to address these issues by introducing a modular, extensible architecture inspired by the OpenDevs project.
Changes
Refactored Chat Architecture:
src/features/workspace/components/chat/tools/ToolRegistry.tsx).theme/chatTheme.ts) for consistent styling and easy customization.blocks/TextBlock.tsxfor plain text and markdown (assistant).blocks/ToolUseBlock.tsxto wrap tool invocations.blocks/ToolResultBlock.tsxto display tool execution results.blocks/ThinkingBlock.tsxfor Claude's internal reasoning.tool_useandtool_resultblocks usinguseMemoinhooks/useMessages.tsand passingtoolResultMapdown the component tree.tool_resultblocks are no longer rendered standalone.MessageItem.tsxrefactored to use these new components, reducing its size and improving separation of concerns.MessageItem.tsxto skip messages with no renderable content, fixing empty message boxes.backend/lib/claude-session.cjs:224) where user messages with tool results were incorrectly saved as 'assistant'.EditToolRenderer.tsx(with diff view)WriteToolRenderer.tsxBashToolRenderer.tsxReadToolRenderer.tsxGrepToolRenderer.tsxTodoWriteToolRenderer.tsxGlobToolRenderer.tsxBashOutputToolRenderer.tsxMultiEditToolRenderer.tsxWebFetchToolRenderer.tsxWebSearchToolRenderer.tsxKillShellToolRenderer.tsxTaskToolRenderer.tsxLSToolRenderer.tsxDefaultToolRenderer.tsxto handle unknown tools and parse MCP tool names.SyntaxHighlighter.tsxfor basic code display and integrated it intoCodeBlock.tsx.CopyButton.tsx,FilePathDisplay.tsx.@tailwindcss/typographyfor improved markdown rendering.Dependencies Added:
react-markdown,remark-gfm,rehype-rawfor markdown rendering.framer-motionfor animations.react-syntax-highlighter(will be used for advanced syntax highlighting later).@tailwindcss/typographyfor markdown styling.lucide-reactfor icons.Testing
http://localhost:50228)[ToolRegistry]logs and zero errorsSummary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores