feat: UI updates to ChatInterface component and global styles. Changi…#218
feat: UI updates to ChatInterface component and global styles. Changi…#218
Conversation
…ng how tools look like
WalkthroughThe PR enhances the chat interface's tool message rendering by introducing specialized UI for search tools (Grep/Glob), propagating structured tool results via a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)✅ Unit Test PR creation complete.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/ChatInterface.jsx (1)
511-556: Edit diff reconstruction only reverts first occurrencecurrentContent.replace(...) undoes only the first match of new_string. If the snippet appears multiple times, the “before” view is wrong. Use replaceAll (with fallback) to revert all occurrences, or fetch the exact pre/post from backend when available.
- const oldContent = currentContent.replace(input.new_string, input.old_string); + const oldContent = + (typeof currentContent.replaceAll === 'function' + ? currentContent.replaceAll(input.new_string, input.old_string) + : currentContent.split(input.new_string).join(input.old_string));Optional: prefer the existing authenticatedFetch('/api/git/file-with-diff') path (used later) to obtain accurate old/new content.
Also applies to: 569-590
🧹 Nitpick comments (1)
src/components/ChatInterface.jsx (1)
4151-4158: Remove inline <style> chevron rule; rely on global CSSInline details[open] .details-chevron duplicates index.css and risks drift. Prefer a single source in CSS.
- <style> - {` - details[open] .details-chevron { - transform: rotate(180deg); - } - `} - </style> + {/* Chevron rotation handled in src/index.css */}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
src/components/ChatInterface.jsx(20 hunks)src/index.css(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/ChatInterface.jsx (2)
src/App.jsx (4)
autoExpandTools(58-58)showRawParameters(59-59)sendByCtrlEnter(62-62)isInputFocused(55-55)src/components/TodoList.jsx (1)
TodoList(5-89)
🔇 Additional comments (4)
src/index.css (1)
829-839: Narrow the selectors to avoid unintended matches and duration conflictsUsing summary svg[class*="group-open"] and summary svg[class*="transition-transform"] is overly broad and can override Tailwind utilities unexpectedly. You already have .details-chevron—target that instead and drop the substring selectors.
[Suggest_recommended_refactor]Apply:
- details[open] summary svg[class*="group-open"] { - transform: rotate(180deg); - } + /* Let .details-chevron be the sole rotation target */ + /* Rotation for open state is already handled by details[open] .details-chevron */ @@ - .details-chevron, - summary svg[class*="transition-transform"] { - transition: transform 200ms cubic-bezier(0.4, 0, 0.2, 1); - } + /* Smooth chevron transition */ + .details-chevron { + transition: transform 200ms cubic-bezier(0.4, 0, 0.2, 1); + }src/components/ChatInterface.jsx (3)
419-463: Minimized Grep/Glob header: concise and readable — LGTMGood UX to collapse frequent search tools and show key params inline; parsing is guarded. Nice touch linking to results.
1034-1081: Structured Grep/Glob results rendering — LGTMSolid use of structured toolUseResult with file open callbacks and scrollable list. Good typography and affordances.
If toolData.numFiles can disagree with filenames.length, consider showing both or trusting one source consistently.
2481-2484: toolUseResult propagation attached to tool results — LGTMThe shape extension is backward‑compatible (null-safe) and enables richer rendering for search tools.
Also applies to: 2567-2571
| <a | ||
| href={`#tool-result-${message.toolId}`} | ||
| className="flex-shrink-0 text-xs text-blue-600 dark:text-blue-400 hover:text-blue-700 dark:hover:text-blue-300 font-medium transition-colors flex items-center gap-1" | ||
| > | ||
| <span>results</span> | ||
| <svg className="w-3 h-3" fill="none" stroke="currentColor" viewBox="0 0 24 24"> | ||
| <path strokeLinecap="round" strokeLinejoin="round" strokeWidth={2} d="M19 9l-7 7-7-7" /> | ||
| </svg> | ||
| </a> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Sanitize anchor IDs to avoid invalid fragments and collisions
message.toolId may contain spaces/special chars. Use a sanitized ID for both href and id to ensure valid, unique anchors.
- href={`#tool-result-${message.toolId}`}
+ href={`#tool-result-${String(message.toolId ?? '').replace(/[^a-zA-Z0-9_-]/g, '-')}`}
@@
- id={`tool-result-${message.toolId}`}
+ id={`tool-result-${String(message.toolId ?? '').replace(/[^a-zA-Z0-9_-]/g, '-')}`}Please verify whether toolId is guaranteed unique per message; if not, consider appending the array index or timestamp to the slug.
Also applies to: 929-930
🏁 Script executed:
#!/bin/bash
# Inspect target lines in ChatInterface.jsx
sed -n '440,465p' src/components/ChatInterface.jsx
echo "---"
sed -n '920,935p' src/components/ChatInterface.jsxLength of output: 2265
🏁 Script executed:
#!/bin/bash
# Search for toolId usage and its source
rg -nP -C5 'message\.toolId' src/components/ChatInterface.jsx | head -80Length of output: 2039
🏁 Script executed:
#!/bin/bash
# Search for where toolId is assigned or originates
rg -nP 'toolId\s*[:=]' src/components/ChatInterface.jsx | head -40Length of output: 401
🏁 Script executed:
#!/bin/bash
# Look for unique/identifier generation patterns in the component
rg -nP '(uuid|crypto|timestamp|index|Math\.random)' src/components/ChatInterface.jsxLength of output: 4393
🏁 Script executed:
#!/bin/bash
# Search for anchor id patterns elsewhere in the codebase
rg -nP 'id=\{' src/components/ChatInterface.jsx | head -20Length of output: 60
🏁 Script executed:
#!/bin/bash
# Find where blobIdx is defined and its scope
rg -nP -B10 'blobIdx\s*=|for\s*\(\s*.*\s+blobIdx' src/components/ChatInterface.jsx | head -60Length of output: 594
🏁 Script executed:
#!/bin/bash
# Check what toolCallId values look like (from API responses or comments)
rg -nP -C3 'toolCallId' src/components/ChatInterface.jsx | head -40Length of output: 1320
🏁 Script executed:
#!/bin/bash
# Verify message index is passed to MessageComponent for rendering
rg -nP 'MessageComponent|message.*index' src/components/ChatInterface.jsx | grep -A2 -B2 'index='Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search for where toolId is used as a key or identifier elsewhere
rg -nP '\btoolId\b' src/components/ChatInterface.jsx | wc -lLength of output: 2
🏁 Script executed:
#!/bin/bash
# Find where visibleMessages is defined and how messages are collected
rg -nP 'visibleMessages\s*=' src/components/ChatInterface.jsx | head -20Length of output: 46
🏁 Script executed:
#!/bin/bash
# Check if messages from different sessions can appear together
rg -nP 'visibleMessages|chatMessages' src/components/ChatInterface.jsx | grep -E '(const|let|filter|concat|\.map)' | head -30Length of output: 256
🏁 Script executed:
#!/bin/bash
# Look at the message rendering context to understand data flow
sed -n '4340,4360p' src/components/ChatInterface.jsxLength of output: 783
🏁 Script executed:
#!/bin/bash
# Check if there's any deduplication or ID collision handling
rg -nP '(duplicate|collision|unique|Set\(|toolId)' src/components/ChatInterface.jsx | grep -v 'toolId.*=\|toolId.*:' | head -20Length of output: 291
🏁 Script executed:
#!/bin/bash
# Check the visibleMessages computation to understand data sources
sed -n '3493,3550p' src/components/ChatInterface.jsxLength of output: 2419
🏁 Script executed:
#!/bin/bash
# Search for whether multiple sessions/datasets can coexist in visibleMessages
rg -nP 'chatMessages|visibleMessages' src/components/ChatInterface.jsx | grep -E 'filter|slice|concat|\.\.\.' | head -20Length of output: 57
🏁 Script executed:
#!/bin/bash
# Check if toolCallId format from API is documented or validated anywhere
rg -nP 'toolCallId|call_' src/components/ChatInterface.jsx | grep -v '\.replace\|toolId\|:' | head -20Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search for API documentation or type hints about toolCallId format
rg -nP 'toolCallId|tool.*Id.*:.*string' src/ --type=ts --type=js | head -20Length of output: 486
🏁 Script executed:
#!/bin/bash
# Check if there are existing sanitization utilities or patterns in codebase
rg -nP 'sanitize|escape|replace.*special' src/ --type=js --type=ts | head -20Length of output: 874
Sanitize anchor IDs and use message index to guarantee uniqueness
External API sources (toolCallId, part.id) may contain special characters; HTML id attributes require [a-zA-Z0-9_-] subset. Additionally, use message index to ensure anchors remain unique even if toolIds collide.
Apply at lines 450 (href) and 929 (id):
- href={`#tool-result-${message.toolId}`}
+ href={`#tool-result-${index}-${String(message.toolId ?? '').replace(/[^a-zA-Z0-9_-]/g, '-')}`}- id={`tool-result-${message.toolId}`}
+ id={`tool-result-${index}-${String(message.toolId ?? '').replace(/[^a-zA-Z0-9_-]/g, '-')}`}This pattern aligns with existing sanitization practices in the codebase (e.g., PRDEditor.jsx line 547).
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/components/ChatInterface.jsx around lines 449 to 457 (href) and line 929
(id), the anchor href/id use message.toolId (and other external IDs) directly
which may include disallowed characters and can collide; sanitize the ID to only
[a-zA-Z0-9_-] (e.g. strip/replace invalid chars) and append the message index
(or a per-message unique index) to guarantee uniqueness; update the href to
`#tool-result-{sanitizedId}-{index}` and the corresponding element id at line
929 to match the same sanitizedId-index pattern so anchors are safe and unique
across messages.
|
Note Unit test generation is an Early Access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
✅ UTG Post-Process Complete No new issues were detected in the generated code and all check runs have completed. The unit test generation process has completed successfully. |
|
Creating a PR to put the unit tests in... The changes have been created in this pull request: View PR |
1 participant
…ng how tools look like
Summary by CodeRabbit
Release Notes