Conversation
📝 WalkthroughWalkthroughAdds a Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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
🤖 Fix all issues with AI agents
In `@src/components/shared/ToolCalls.tsx`:
- Line 146: Wrap the JSON.stringify calls that render inputs and return_value in
a try/catch to avoid runtime crashes on circular structures: in the JSX where
{JSON.stringify(inputs, null, 2)} is used (and likewise where return_value is
stringified around lines referencing return_value), attempt JSON.stringify and
on error render a safe fallback such as a descriptive string ("[Circular]" or
util.inspect/another safe serializer) or the object's toString(); update the
rendering logic in the ToolCalls component to catch stringify errors and display
the fallback instead of letting the exception propagate.
🧹 Nitpick comments (3)
src/contexts/MessagesContext.tsx (1)
66-74: Consider makingurloptional.The
urlfield is currently required, but not all tool calls may have an associated URL. If the backend can send tool calls without URLs, this could cause type mismatches.💡 Suggested change
export interface ToolCall { id: string | number; title: string; - url: string; + url?: string; inputs: Record<string, any>; return_value: any; is_running: boolean; icon?: string; }src/components/shared/ToolCalls.tsx (2)
52-56: Inline<style>tag renders per card instance.This
<style>block will be duplicated for eachToolCallCardrendered, which is inefficient. Consider using theaddInlineStylepattern used elsewhere in the codebase (seeIncomingMsg.tsxline 19), or move this to a shared CSS file.💡 Suggested approach
Create a separate style file or add to an existing one:
/* toolcalls.scss */ .tool-call-summary::after { display: none !important; }Then import it once at component level using
addInlineStyle:import React, { useState } from "react"; import clsx from "clsx"; import { ToolCall } from "src/contexts/MessagesContext"; +import { addInlineStyle } from "src/addStyles"; +import style from "./toolcalls.scss?inline"; +addInlineStyle(style);And remove the inline
<style>tag from the component.
67-82: Icon URL check may missdata:URLs.The check
icon.startsWith('http') || icon.startsWith('/')handles absolute and relative URLs but would treatdata:URLs (base64 images) as text/emoji icons.💡 Suggested fix
-{icon && ( - icon.startsWith('http') || icon.startsWith('/') ? ( +{icon && ( + icon.startsWith('http') || icon.startsWith('/') || icon.startsWith('data:') ? ( <img
| lineHeight: "1.4", | ||
| }} | ||
| > | ||
| {JSON.stringify(inputs, null, 2)} |
There was a problem hiding this comment.
JSON.stringify can throw on circular references.
If inputs contains circular references, JSON.stringify will throw an error. Consider wrapping in a try/catch with a fallback.
🛡️ Proposed defensive fix
+const safeStringify = (obj: any): string => {
+ try {
+ return JSON.stringify(obj, null, 2);
+ } catch {
+ return "[Unable to display]";
+ }
+};
+
// Then use in the component:
-{JSON.stringify(inputs, null, 2)}
+{safeStringify(inputs)}Apply the same pattern for return_value on lines 168-170.
🤖 Prompt for AI Agents
In `@src/components/shared/ToolCalls.tsx` at line 146, Wrap the JSON.stringify
calls that render inputs and return_value in a try/catch to avoid runtime
crashes on circular structures: in the JSX where {JSON.stringify(inputs, null,
2)} is used (and likewise where return_value is stringified around lines
referencing return_value), attempt JSON.stringify and on error render a safe
fallback such as a descriptive string ("[Circular]" or util.inspect/another safe
serializer) or the object's toString(); update the rendering logic in the
ToolCalls component to catch stringify errors and display the fallback instead
of letting the exception propagate.
![entelligence-ai-pr-reviews[bot]](https://avatars.githubusercontent.com/in/932812?s=60&v=4){kind=small}
| </div> | ||
| )} | ||
|
|
||
| {!is_running && return_value && ( |
There was a problem hiding this comment.
Correctness: The conditional !is_running && return_value on line 156 incorrectly hides valid falsy return values such as 0, false, or an empty string. This prevents legitimate tool outputs from being displayed in the UI. Use an explicit check (e.g., return_value !== undefined) to ensure all completed results are rendered.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
File: src/components/shared/ToolCalls.tsx. Update the conditional rendering of the return value so it doesn't filter out valid falsy values. Replace `!is_running && return_value` with a null/undefined check (e.g., `return_value != null`) while keeping the rest of the JSX intact.
✨ Committable Code Suggestion
💡 This is a one-click fix! Click "Commit suggestion" to apply this change directly to your branch.
| {!is_running && return_value && ( | |
| import React, { useState } from "react"; | |
| import clsx from "clsx"; | |
| import { ToolCall } from "src/contexts/MessagesContext"; | |
| interface ToolCallsProps { | |
| toolCalls: ToolCall[]; | |
| className?: string; | |
| } | |
| const extractArgumentSummary = (inputs: Record<string, any>): string | null => { | |
| const findFirstString = (obj: any, maxLength = 50): string | null => { | |
| if (typeof obj === "string") { | |
| return obj.length > maxLength ? obj.substring(0, maxLength) + "..." : obj; | |
| } | |
| if (Array.isArray(obj)) { | |
| for (const item of obj) { | |
| const result = findFirstString(item, maxLength); | |
| if (result) return result; | |
| } | |
| } | |
| if (obj && typeof obj === "object") { | |
| for (const value of Object.values(obj)) { | |
| const result = findFirstString(value, maxLength); | |
| if (result) return result; | |
| } | |
| } | |
| return null; | |
| }; | |
| if (!inputs || Object.keys(inputs).length === 0) { | |
| return null; | |
| } | |
| return findFirstString(inputs); | |
| }; | |
| const ToolCallCard: React.FC<{ toolCall: ToolCall }> = ({ toolCall }) => { | |
| const { title, inputs, return_value, is_running, icon } = toolCall; | |
| const [isExpanded, setIsExpanded] = useState(false); | |
| const argumentSummary = extractArgumentSummary(inputs || {}); | |
| return ( | |
| <details | |
| open={isExpanded} | |
| onToggle={(e) => setIsExpanded((e.target as HTMLDetailsElement).open)} | |
| style={{ | |
| backgroundColor: "transparent", | |
| border: "1px solid #e9ecef", | |
| }} | |
| > | |
| <style>{` | |
| .tool-call-summary::after { | |
| display: none !important; | |
| } | |
| `}</style> | |
| <summary | |
| className="tool-call-summary" | |
| style={{ | |
| backgroundColor: "transparent", | |
| display: "flex", | |
| alignItems: "center", | |
| gap: "6px", | |
| listStyle: "none", | |
| }} | |
| > | |
| {icon && ( | |
| icon.startsWith('http') || icon.startsWith('/') ? ( | |
| <img | |
| src={icon} | |
| alt="" | |
| style={{ | |
| width: "16px", | |
| height: "16px", | |
| objectFit: "contain", | |
| flexShrink: 0, | |
| }} | |
| /> | |
| ) : ( | |
| <span style={{ fontSize: "16px", flexShrink: 0 }}>{icon}</span> | |
| ) | |
| )} | |
| <div | |
| style={{ | |
| overflow: "hidden", | |
| textOverflow: "ellipsis", | |
| whiteSpace: "nowrap", | |
| minWidth: 0, | |
| flex: 1, | |
| }} | |
| > | |
| <span className="font_14_600">{title}</span> | |
| {argumentSummary && ( | |
| <> | |
| <span className="font_14_600"> - </span> | |
| <span className="font_14_400 text-muted">{argumentSummary}</span> | |
| </> | |
| )} | |
| {is_running && ( | |
| <span className="font_12_400 text-muted"> Running...</span> | |
| )} | |
| </div> | |
| <div | |
| className={clsx( | |
| "tool-status-indicator rounded-circle", | |
| is_running ? "bg-primary" : "bg-success" | |
| )} | |
| style={{ | |
| width: "8px", | |
| height: "8px", | |
| animation: is_running ? "pulse 2s infinite" : "none", | |
| flexShrink: 0, | |
| }} | |
| /> | |
| <div | |
| style={{ | |
| fontSize: "10px", | |
| color: "#6c757d", | |
| flexShrink: 0, | |
| transform: isExpanded ? "rotate(180deg)" : "rotate(0deg)", | |
| transition: "transform 0.2s ease", | |
| }} | |
| > | |
| ▼ | |
| </div> | |
| </summary> | |
| <div style={{ padding: "8px 0" }}> | |
| {inputs && Object.keys(inputs).length > 0 && ( | |
| <div className="mb-3"> | |
| <div className="font_14_600 text-dark mb-2">Inputs</div> | |
| <div | |
| className="font_12_400" | |
| style={{ | |
| fontFamily: "ui-monospace, SFMono-Regular, 'SF Mono', Monaco, 'Cascadia Code', 'Roboto Mono', Consolas, 'Courier New', monospace", | |
| backgroundColor: "#f8f9fa", | |
| border: "1px solid #e9ecef", | |
| padding: "12px", | |
| borderRadius: "6px", | |
| maxHeight: "200px", | |
| overflow: "auto", | |
| whiteSpace: "pre-wrap", | |
| lineHeight: "1.4", | |
| }} | |
| > | |
| {JSON.stringify(inputs, null, 2)} | |
| </div> | |
| </div> | |
| )} | |
| {!is_running && return_value && ( | |
| <div> | |
| <div className="font_14_600 text-dark mb-2">Return value</div> | |
| <div | |
| className="font_12_400" | |
| style={{ | |
| fontFamily: "ui-monospace, SFMono-Regular, 'SF Mono', Monaco, 'Cascadia Code', 'Roboto Mono', Consolas, 'Courier New', monospace", | |
| backgroundColor: "#f8f9fa", | |
| border: "1px solid #e9ecef", | |
| padding: "12px", | |
| borderRadius: "6px", | |
| maxHeight: "200px", | |
| overflow: "auto", | |
| whiteSpace: "pre-wrap", | |
| lineHeight: "1.4", | |
| }} | |
| > | |
| {typeof return_value === "string" | |
| ? return_value | |
| : JSON.stringify(return_value, null, 2)} | |
| </div> | |
| </div> | |
| )} | |
| </div> | |
| </details> | |
| ); | |
| }; | |
| const ToolCalls: React.FC<ToolCallsProps> = ({ toolCalls, className }) => { | |
| if (!toolCalls || toolCalls.length === 0) { | |
| return null; | |
| } | |
| return ( | |
| <div className={clsx("tool-calls-container", className)}> | |
| {toolCalls.map((toolCall) => ( | |
| <ToolCallCard key={String(toolCall.id)} toolCall={toolCall} /> | |
| ))} | |
| </div> | |
| ); | |
| }; | |
| export default ToolCalls; | |
| buttons, | ||
| tool_calls: payload.tool_calls || prevMessage?.tool_calls || [], | ||
| }); | ||
| return newConversations; |
There was a problem hiding this comment.
Correctness: The assignment to tool_calls in the MESSAGE_PART block overwrites the existing state whenever payload.tool_calls is present. This causes data loss when tool calls are delivered incrementally across multiple chunks, inconsistent with the accumulation logic used for text and buttons in the same block.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In `src/contexts/messages/useStreamingHandler.ts` at line 141, the new `tool_calls` assignment overwrites previous streamed tool calls. Update it to merge `prevMessage.tool_calls` with `payload.tool_calls` (similar to `buttons`) so partial chunks are preserved. Replace the line with a concatenated array of both.
✨ Committable Code Suggestion
💡 This is a one-click fix! Click "Commit suggestion" to apply this change directly to your branch.
| return newConversations; | |
| ...payload, | |
| id: currentStreamRef.current, | |
| text, | |
| buttons, | |
| tool_calls: [ | |
| ...(prevMessage?.tool_calls || []), | |
| ...(payload.tool_calls || []), | |
| ], | |
| }); | |
| return newConversations; | |
| } |
|
✅ Review completed for commit e9149f0. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/components/shared/ToolCalls.tsx`:
- Line 194: The render condition currently checks {!is_running && return_value}
which incorrectly hides valid falsy outputs (0, "", false); update the condition
in the ToolCalls component to explicitly check for null/undefined instead (e.g.,
{!is_running && return_value !== null && return_value !== undefined}) so valid
falsy return_value values still render, while still suppressing truly absent
values.
🧹 Nitpick comments (1)
src/components/shared/ToolCalls.tsx (1)
55-59: Move<style>tag outside the component to avoid DOM duplication.This
<style>tag is rendered inside eachToolCallCardinstance. If multiple tool calls are displayed, identical style blocks will be duplicated in the DOM.Consider moving this to a shared CSS file, or lifting it to the parent
ToolCallscomponent so it renders only once.♻️ Suggested refactor: lift style to parent component
const ToolCalls: React.FC<ToolCallsProps> = ({ toolCalls, className }) => { if (!toolCalls || toolCalls.length === 0) { return null; } return ( <div className={clsx("tool-calls-container", className)}> + <style>{` + .tool-call-summary::after { + display: none !important; + } + `}</style> {toolCalls.map((toolCall) => ( <ToolCallCard key={String(toolCall.id)} toolCall={toolCall} /> ))} </div> ); };Then remove the
<style>block fromToolCallCard(lines 55-59).
| </div> | ||
| )} | ||
|
|
||
| {!is_running && return_value && ( |
There was a problem hiding this comment.
Falsy return_value values (0, "", false) will not render.
The condition return_value will evaluate to false for valid return values like 0, "", or false. If these are valid tool outputs, consider using an explicit check:
-{!is_running && return_value && (
+{!is_running && return_value !== undefined && return_value !== null && (Alternatively, if the intent is to hide empty outputs, the current behavior may be acceptable.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {!is_running && return_value && ( | |
| {!is_running && return_value !== undefined && return_value !== null && ( |
🤖 Prompt for AI Agents
In `@src/components/shared/ToolCalls.tsx` at line 194, The render condition
currently checks {!is_running && return_value} which incorrectly hides valid
falsy outputs (0, "", false); update the condition in the ToolCalls component to
explicitly check for null/undefined instead (e.g., {!is_running && return_value
!== null && return_value !== undefined}) so valid falsy return_value values
still render, while still suppressing truly absent values.
| display: "flex", | ||
| alignItems: "center", | ||
| fontSize: "11px", | ||
| textDecoration: "none", |
There was a problem hiding this comment.
Correctness: The href={url} attribute is vulnerable to Cross-Site Scripting (XSS). It allows execution of arbitrary code via javascript: or data: URIs. Sanitize url to permit only http: and https: protocols.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
File: src/components/shared/ToolCalls.tsx. Lines: 155-158. The new external link uses toolCall.url directly, which can allow javascript: or data: URLs. Guard rendering with a protocol allowlist (http/https) or validate via new URL() before rendering. Apply the diff: wrap the link with a protocol check so only http/https URLs render.
1 participant
Legal Boilerplate
Look, I get it. The entity doing business as “Gooey.AI” and/or “Dara.network” was incorporated in the State of Delaware in 2020 as Dara Network Inc. and is gonna need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Dara Network Inc can use, modify, copy, and redistribute my contributions, under its choice of terms.