Daily branch 2025 08 21 #12
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. WalkthroughAdds Todo support end-to-end (API, tools, UI, global state); renames tool identifiers to snake_case; changes createTools and systemPrompt signatures; introduces token-aware truncation and terminal streaming handlers; seeds TodoManager and consolidates todo tooling to a single todo_write surface. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as Client UI (Page)
participant Chat as /api/chat
participant Tools as createTools
participant Model as AI Stream
participant GS as GlobalState
User->>UI: Send message
UI->>Chat: POST { messages, mode, todos? }
Chat->>Tools: createTools(userID, writer, mode, execMode, location, todos?)
Tools-->>Chat: tools { ..., todo_write, ... }
Chat->>Model: systemPrompt(modelId, mode, executionMode) & messages
Chat-->>UI: Stream tokens/events
rect rgba(200,230,255,0.25)
Model-->>UI: toolCall { toolName: "todo_write", input: { merge?, todos } }
UI->>GS: mergeTodos(input.todos)
GS-->>UI: todos updated
UI-->>User: Todo panel/statistics update
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (6)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/components/tools/TodoToolHandler.tsx (1)
50-67: Defensive guards for possibly missing output/currentTodos.If the tool returns no
currentTodos, the component will passundefinedtoTodoBlock, risking runtime issues. Default to empty arrays.Apply this diff:
case "output-available": { - const todoOutput = output as { - result: string; - counts: { - completed: number; - total: number; - }; - currentTodos: Todo[]; - }; - - return ( - <TodoBlock - todos={todoOutput.currentTodos} - inputTodos={todoInput?.todos} - blockId={toolCallId} - messageId={message.id} - /> - ); + const todoOutput = (output || {}) as { + result?: string; + counts?: { completed: number; total: number }; + currentTodos?: Todo[]; + }; + const currentTodos = todoOutput?.currentTodos ?? []; + const inputTodos = todoInput?.todos ?? []; + + return ( + <TodoBlock + todos={currentTodos} + inputTodos={inputTodos} + blockId={toolCallId} + messageId={message.id} + /> + ); }lib/ai/tools/todo-manager.ts (1)
1-216: Remove the unusedcreateTodoManagertool and consolidate withtodo_writeThe
createTodoManagerexport inlib/ai/tools/todo-manager.tsis not referenced or registered anywhere in the codebase; all task-management calls use thetodo_writetool. Leaving this dead code risks confusion and diverging tool behavior.• Delete
lib/ai/tools/todo-manager.ts(or stop exporting it).
• If a distinct “manager” interface is intended, alias it tocreateTodoWriteand register it inlib/ai/tools/index.tsunder its desired key.
• Ensure the system-prompt, message-processor, and UI handlers remain aligned to the singletodo_writetool.This refactor will remove dead code and prevent API drift between tool definitions and tool listeners.
🧹 Nitpick comments (24)
app/layout.tsx (2)
45-51: Use Next.js Metadata/Viewport APIs instead of a manual to avoid duplication and hydration issues.In the App Router, manually rendering a block is discouraged. Prefer:
- export const viewport for the viewport meta
- metadata.icons for the apple-touch-icon
This removes the risk of duplicate tags and keeps SSR/CSR consistent.
Apply this diff to remove the manual head block:
return ( <html lang="en" className="dark" suppressHydrationWarning> - <head> - <meta - name="viewport" - content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=no" - /> - <link rel="apple-touch-icon" href="/apple-touch-icon.png" /> - </head> <body className={`${geistSans.variable} ${geistMono.variable} antialiased`} >And add the following outside this range:
- import type { Metadata } from "next"; + import type { Metadata, Viewport } from "next"; export const metadata: Metadata = { title: "Create Next App", description: "Generated by create next app", + icons: { + apple: "/apple-touch-icon.png", + }, }; +export const viewport: Viewport = { + width: "device-width", + initialScale: 1, + // Consider leaving user-scalable enabled for accessibility (see next comment). +};
45-51: Don’t disable pinch-zoom; it harms accessibility.The current viewport string sets maximum-scale=1.0 and user-scalable=no, which blocks zooming. This is an a11y anti-pattern. Omit those settings so users can zoom.
If you still need to limit zoom for a specific embedded view, gate it behind a feature flag or device breakpoint rather than globally.
app/hooks/useSidebarAutoOpen.ts (1)
152-160: Guard against missing parts to avoid runtime errors during streaming.lastAssistantMessage.parts may be undefined depending on upstream message construction. A small defensive change prevents .find/.filter on undefined.
Apply this diff:
- const terminalToolPart = lastAssistantMessage.parts.find( + const parts = lastAssistantMessage.parts ?? []; + const terminalToolPart = parts.find( (part: any) => part.type === "tool-run_terminal_cmd" && part.toolCallId === sidebarContent.toolCallId, ); - const terminalDataParts = lastAssistantMessage.parts.filter( + const terminalDataParts = parts.filter( (part: any) => part.type === "data-terminal" && part.data?.toolCallId === sidebarContent.toolCallId, );Also applies to: 165-173
components/ui/todo-block.tsx (1)
36-36: Minor: consider reusing getTodoStats across backend too for single source of truth.TodoManager.getStats (lib/ai/tools/utils/todo-manager.ts) duplicates stats logic. Importing getTodoStats there would ensure parity between UI and tool backends.
If you want, I can draft the change in TodoManager to delegate to getTodoStats.
lib/ai/tools/utils/todo-manager.ts (1)
77-91: Avoid duplicated stats logic by delegating to the shared utility.getStats reimplements counts also present in getTodoStats (lib/utils/todo-utils.ts). Using the shared util prevents drift.
Here’s a minimal change:
+import { getTodoStats } from "@/lib/utils/todo-utils"; ... - getStats() { - const todos = this.getAllTodos(); - const completed = todos.filter((t) => t.status === "completed").length; - const cancelled = todos.filter((t) => t.status === "cancelled").length; - - return { - total: todos.length, - pending: todos.filter((t) => t.status === "pending").length, - inProgress: todos.filter((t) => t.status === "in_progress").length, - completed: completed, - cancelled: cancelled, - // Count both completed and cancelled as "done" for progress tracking - done: completed + cancelled, - }; - } + getStats() { + return getTodoStats(this.getAllTodos()); + }app/components/MessagePartHandler.tsx (1)
42-67: Consider centralizing tool type strings to avoid typos and ease future additions.Right now, tool identifiers are repeated as string literals. A small abstraction reduces error-prone duplication and makes it obvious where to add new tools.
You could define a typed constant for file tools and narrow
part.type:// near the top of the file const FILE_TOOL_TYPES = [ "tool-read_file", "tool-write_file", "tool-delete_file", "tool-search_replace", "tool-multi_edit", ] as const; type FileToolType = (typeof FILE_TOOL_TYPES)[number]; type KnownPartType = | FileToolType | "tool-web_search" | "tool-run_terminal_cmd" | "tool-todo_write" | "data-terminal" | "text";Then:
switch (part.type as KnownPartType) { // ...existing cases }app/components/tools/TodoToolHandler.tsx (2)
20-24: Update comment to match new token name.The comment still mentions
tool-todoWrite; the actual token istool-todo_write. Keep comments in sync to avoid confusion during future maintenance.- // Handle tool-todoWrite type + // Handle tool-todo_write type
8-12: Type thepartprop to prevent runtime shape errors.Casting
anyinvites silent breakage if the tool payload shape changes. Narrowpartto a discriminated union fortool-todo_write.Here’s a minimal local type you can use:
type TodoToolPart = { type: "tool-todo_write"; toolCallId: string; state: "input-streaming" | "input-available" | "output-available"; input?: { merge: boolean; todos: Todo[] }; output?: { result?: string; counts?: { completed: number; total: number }; currentTodos?: Todo[]; }; }; interface TodoToolHandlerProps { message: UIMessage; part: TodoToolPart; status: ChatStatus; }app/contexts/GlobalState.tsx (1)
105-128: Memoize context value to reduce gratuitous re-renders in all consumers.The provider
valueobject is recreated on every render, forcing all context consumers to re-render even when irrelevant state hasn’t changed.A lightweight step is to wrap
valueinuseMemoand wrap the inline functions (clearInput,resetChat,openSidebar,updateSidebarContent,closeSidebar) inuseCallbackso their identities are stable.-import React, { +import React, { createContext, useContext, useState, useCallback, + useMemo, ReactNode, } from "react"; @@ - const clearInput = () => { + const clearInput = useCallback(() => { setInput(""); - }; + }, []); @@ - const resetChat = () => { + const resetChat = useCallback(() => { setInput(""); setChatTitle(null); setTodos([]); setIsTodoPanelExpanded(false); - }; + }, []); @@ - const openSidebar = (content: SidebarContent) => { + const openSidebar = useCallback((content: SidebarContent) => { setSidebarContent(content); setSidebarOpen(true); - }; + }, []); @@ - const updateSidebarContent = (updates: Partial<SidebarContent>) => { - setSidebarContent((current) => { - if (current) { - return { ...current, ...updates }; - } - return current; - }); - }; + const updateSidebarContent = useCallback((updates: Partial<SidebarContent>) => { + setSidebarContent((current) => (current ? { ...current, ...updates } : current)); + }, []); @@ - const closeSidebar = () => { + const closeSidebar = useCallback(() => { setSidebarOpen(false); setSidebarContent(null); - }; + }, []); @@ - const value: GlobalStateType = { + const value = useMemo<GlobalStateType>(() => ({ input, setInput, mode, setMode, chatTitle, setChatTitle, sidebarOpen, setSidebarOpen, sidebarContent, setSidebarContent, todos, setTodos, mergeTodos, isTodoPanelExpanded, setIsTodoPanelExpanded, clearInput, resetChat, openSidebar, updateSidebarContent, closeSidebar, - }; + }), [ + input, mode, chatTitle, sidebarOpen, sidebarContent, todos, isTodoPanelExpanded, + setInput, setMode, setChatTitle, setSidebarOpen, setSidebarContent, setTodos, setIsTodoPanelExpanded, + mergeTodos, clearInput, resetChat, openSidebar, updateSidebarContent, closeSidebar + ]); @@ - <GlobalStateContext.Provider value={value}> + <GlobalStateContext.Provider value={value}>Note: This is optional; adopt only if you observe unnecessary re-renders in consumers.
Also applies to: 130-134
lib/utils/todo-utils.ts (4)
11-37: Optional: reduce merge complexity from O(n·m) to O(n + m).Using a map of current IDs avoids repeated
findIndexscans if todo lists grow.export const mergeTodos = (currentTodos: Todo[], newTodos: Todo[]): Todo[] => { - let hasChanges = false; - const updatedTodos = [...currentTodos]; - - for (const newTodo of newTodos) { - const existingIndex = updatedTodos.findIndex((t) => t.id === newTodo.id); - - if (existingIndex >= 0) { - // Check if the todo actually changed - const existing = updatedTodos[existingIndex]; - if ( - existing.content !== newTodo.content || - existing.status !== newTodo.status - ) { - updatedTodos[existingIndex] = newTodo; - hasChanges = true; - } - } else { - // Add new todo - updatedTodos.push(newTodo); - hasChanges = true; - } - } + let hasChanges = false; + const updatedTodos = [...currentTodos]; + const indexById = new Map<string, number>( + currentTodos.map((t, i) => [t.id, i]), + ); + + for (const newTodo of newTodos) { + const existingIndex = indexById.get(newTodo.id); + if (existingIndex !== undefined) { + const existing = updatedTodos[existingIndex]; + if ( + existing.content !== newTodo.content || + existing.status !== newTodo.status + ) { + updatedTodos[existingIndex] = newTodo; + hasChanges = true; + } + } else { + updatedTodos.push(newTodo); + indexById.set(newTodo.id, updatedTodos.length - 1); + hasChanges = true; + } + }
42-44: Clarify equality semantics or rename for precision.
areTodosEqualignoresidand only checks content/status. Either document that intent explicitly or rename to something likehaveSameContentAndStatusto avoid misinterpretation.-/** - * Checks if two todos are equal (same content and status) - */ -export const areTodosEqual = (todo1: Todo, todo2: Todo): boolean => { +/** + * Checks if two todos have the same content and status (ID is ignored intentionally) + */ +export const areTodosEqual = (todo1: Todo, todo2: Todo): boolean => { return todo1.content === todo2.content && todo1.status === todo2.status; };
49-65: Export a typed shape for stats and annotate the return type.This improves editor hints and reduces downstream mistakes.
+export type TodoStats = { + completed: number; + inProgress: number; + pending: number; + cancelled: number; + total: number; + done: number; +}; + -export const getTodoStats = (todos: Todo[]) => { +export const getTodoStats = (todos: Todo[]): TodoStats => { const completed = todos.filter((t) => t.status === "completed").length; const inProgress = todos.filter((t) => t.status === "in_progress").length; const pending = todos.filter((t) => t.status === "pending").length; const cancelled = todos.filter((t) => t.status === "cancelled").length; const total = todos.length; const done = completed + cancelled;
11-37: Add unit tests for merge semantics and stats.Recommend tests that validate:
- No-change merge preserves array reference.
- Existing items update only when content/status differ.
- New items append.
- Stats totals and done counts are correct.
I can draft a small Jest test suite for
mergeTodosandgetTodoStatsif helpful.Also applies to: 49-65
app/components/TodoPanel.tsx (3)
65-76: Improve accessibility with ARIA attributes and association.Add
aria-expandedandaria-controlsto the toggle, and anidon the collapsible section.<button onClick={handleToggleExpand} className="flex items-center gap-2 hover:opacity-80 transition-opacity cursor-pointer focus:outline-none rounded-md p-1 -m-1 flex-1" aria-label={isExpanded ? "Collapse todos" : "Expand todos"} + aria-expanded={isExpanded} + aria-controls="todo-panel-list" tabIndex={0} onKeyDown={(e) => { @@ - {isExpanded && ( - <div className="border-t border-border px-4 py-3 space-y-2"> + {isExpanded && ( + <div + id="todo-panel-list" + className="border-t border-border px-4 py-3 space-y-2" + > {todos.map((todo) => ( <SharedTodoItem key={todo.id} todo={todo} /> ))} </div> )}Also applies to: 91-97
33-39: Optional: also collapse local panel state when it becomes hidden.You reset the global flag; consider syncing local
isExpandedto false to avoid stale UI when the panel reappears.useEffect(() => { if (!hasTodos || !hasActiveTodos) { - setIsTodoPanelExpanded(false); + setIsTodoPanelExpanded(false); + setIsExpanded(false); } }, [hasTodos, hasActiveTodos, setIsTodoPanelExpanded]);
10-15: Remove unusedstatusprop from TodoPanelThe
statusprop is declared on TodoPanelProps and passed in from ChatInput, but it’s never read insideTodoPanel. You can simplify the component API by dropping it entirely:• In app/components/TodoPanel.tsx
– Remove thestatus: ChatStatus;line from theTodoPanelPropsinterface (currently line 11).
– Dropstatusfrom the function signature (line 14) and, since there are no remaining props, eliminate the interface and change the signature to:- interface TodoPanelProps { - status: ChatStatus; - } - - export const TodoPanel = ({ status }: TodoPanelProps) => { + export const TodoPanel = () => { const [isExpanded, setIsExpanded] = useState(false); …• In app/components/ChatInput.tsx
– Update the call site at line 67 to no longer passstatus:- <TodoPanel status={status} /> + <TodoPanel />After applying these edits, rebuild and verify that no type errors or test failures remain.
lib/utils/message-processor.ts (2)
141-150: Add backward-compat for pre-rename terminal tool types (optional)If any older conversations or interim clients still emit "tool-terminal" (or similar), consider tolerating both to avoid dropping terminal output during rollouts.
Apply this minimal guard:
- if (toolPart.type === "tool-run_terminal_cmd") { + if (toolPart.type === "tool-run_terminal_cmd" || toolPart.type === "tool-terminal") { return transformTerminalToolPart( toolPart as TerminalToolPart, terminalDataMap, ); }
183-204: Interrupted todo_write default output is fine; consider richer stats (optional)Counts.completed currently implies “done = completed+cancelled” elsewhere. If UI consumes these counts, consider returning { done, cancelled, inProgress, total } to avoid ambiguity and downstream re-derivation.
lib/ai/tools/todo-write.ts (1)
144-147: Tighten semantics around counts and input hygiene (optional)
- counts.completed maps to stats.done (completed+cancelled). Either rename to done or include cancelled/inProgress explicitly to avoid misreads by clients.
- Trim/validate todo.id and content to prevent accidental whitespace-only values; zod preprocess can help.
Example tweaks:
- todos: z + todos: z .array( z.object({ - id: z.string().describe("Unique identifier for the todo item"), - content: z - .string() + id: z.preprocess(v => typeof v === 'string' ? v.trim() : v, z.string().min(1)) + .describe("Unique identifier for the todo item"), + content: z.preprocess(v => typeof v === 'string' ? v.trim() : v, z.string()) .optional() .describe("The description/content of the todo item"),And consider returning richer stats:
- const counts = { - completed: stats.done, - total: stats.total, - }; + const counts = { + done: stats.done, + cancelled: stats.cancelled, + inProgress: stats.inProgress, + total: stats.total, + };Also applies to: 153-153, 165-165
app/api/chat/route.ts (1)
13-14: Remove unused importcreateTodoWrite is imported but not used in this file.
-import { createTodoWrite } from "@/lib/ai/tools/todo-write";app/page.tsx (1)
241-258: Tailwind class likely invalid: use an arbitrary value or scale tokenbottom-42 is not a default Tailwind spacing token. Use an arbitrary value to ensure it compiles.
- <div - className={`fixed bottom-42 z-40 transition-all duration-300 ${ + <div + className={`fixed bottom-[42px] z-40 transition-all duration-300 ${ sidebarOpen ? "left-1/2 desktop:left-1/4 -translate-x-1/2" : "left-1/2 -translate-x-1/2" }`} >lib/ai/tools/index.ts (3)
63-66: Avoid double-instantiating todo_write in ask modeYou already create todo_write in allTools; re-creating it in the ask branch is unnecessary and can split internal metrics/state if the tool ever becomes stateful. Reuse allTools.todo_write for consistency.
? { - read_file: allTools.read_file, - todo_write: createTodoWrite(context), - ...(process.env.EXA_API_KEY && { web_search: allTools.web_search }), + read_file: allTools.read_file, + todo_write: allTools.todo_write, + ...(process.env.EXA_API_KEY && { web_search: allTools.web_search }), }
47-56: All tool IDs are snake_case—no runtime mismatches detectedI’ve swept the codebase for camelCase tool identifiers and confirmed that all runtime references (tool-dispatch,
toolPart.type, etc.) use snake_case. The only remaining camelCase occurrences are in documentation strings and comments, which don’t impact execution but could be updated for consistency:• lib/ai/tools/web-search.ts (lines ~14–29): description text refers to the “webSearch” tool several times.
– Suggest replacing backtick-quoted “webSearch” with “web_search” in examples and prose.• app/components/tools/TodoToolHandler.tsx (around line 20): comment reads “// Handle tool-todoWrite type”.
– Update to “// Handle tool-todo_write type” to match the actual identifier.Optional: As previously noted, you could centralize all tool IDs in a
type ToolId = 'run_terminal_cmd' | …union exported from a single module to catch future mismatches at compile time. I’m happy to help generate that if useful.
23-24: Signature change verified: allcreateToolscall sites updatedI’ve confirmed that the
createToolssignature inlib/ai/tools/index.tsnow has six parameters, withinitialTodos?: Todo[]at the end, and that there is only one call site in the repo:
- In
app/api/chat/route.ts,createTools(userID, writer, mode, executionMode, userLocation, todos)now correctly provides the sixthtodosargument forinitialTodos. No othercreateToolsinvocations exist.Everything lines up, so there’s no missing or out-of-order call.
• Optional refactor: to avoid future argument-order drift, you may want to switch to a single “options” object after the first two required parameters (e.g.
{ mode, executionMode, userLocation, initialTodos }).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (16)
app/api/chat/route.ts(3 hunks)app/components/MessagePartHandler.tsx(1 hunks)app/components/TodoPanel.tsx(1 hunks)app/components/tools/FileToolsHandler.tsx(1 hunks)app/components/tools/TodoToolHandler.tsx(1 hunks)app/contexts/GlobalState.tsx(5 hunks)app/hooks/useSidebarAutoOpen.ts(4 hunks)app/layout.tsx(1 hunks)app/page.tsx(5 hunks)components/ui/todo-block.tsx(2 hunks)lib/ai/tools/index.ts(4 hunks)lib/ai/tools/todo-manager.ts(1 hunks)lib/ai/tools/todo-write.ts(3 hunks)lib/ai/tools/utils/todo-manager.ts(1 hunks)lib/utils/message-processor.ts(4 hunks)lib/utils/todo-utils.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (11)
lib/ai/tools/utils/todo-manager.ts (1)
types/chat.ts (1)
Todo(36-40)
components/ui/todo-block.tsx (1)
lib/utils/todo-utils.ts (1)
getTodoStats(49-65)
app/contexts/GlobalState.tsx (2)
types/chat.ts (1)
Todo(36-40)lib/utils/todo-utils.ts (1)
mergeTodos(11-37)
app/components/MessagePartHandler.tsx (3)
app/components/tools/FileToolsHandler.tsx (1)
FileToolsHandler(11-321)app/components/tools/WebSearchToolHandler.tsx (1)
WebSearchToolHandler(10-52)app/components/tools/TerminalToolHandler.tsx (1)
TerminalToolHandler(15-111)
app/components/TodoPanel.tsx (2)
app/contexts/GlobalState.tsx (1)
useGlobalState(137-143)lib/utils/todo-utils.ts (1)
getTodoStats(49-65)
lib/ai/tools/todo-manager.ts (2)
types/agent.ts (1)
ToolContext(16-22)types/chat.ts (1)
Todo(36-40)
lib/utils/todo-utils.ts (1)
types/chat.ts (1)
Todo(36-40)
app/page.tsx (4)
app/contexts/GlobalState.tsx (1)
useGlobalState(137-143)types/chat.ts (1)
Todo(36-40)lib/utils/todo-utils.ts (1)
mergeTodos(11-37)app/components/ScrollToBottomButton.tsx (1)
ScrollToBottomButton(8-24)
lib/ai/tools/todo-write.ts (2)
types/agent.ts (1)
ToolContext(16-22)types/chat.ts (1)
Todo(36-40)
app/api/chat/route.ts (1)
types/chat.ts (2)
ChatMode(1-1)Todo(36-40)
lib/ai/tools/index.ts (3)
types/chat.ts (1)
Todo(36-40)lib/ai/tools/utils/todo-manager.ts (1)
TodoManager(13-92)lib/ai/tools/todo-write.ts (1)
createTodoWrite(5-215)
🔇 Additional comments (17)
app/layout.tsx (1)
45-51: apple-touch-icon asset verifiedThe file
public/apple-touch-icon.pngexists (size 9 449 bytes, last modified Aug 17 17:35), so there will be no 404 error in the<head>. No further action required.app/hooks/useSidebarAutoOpen.ts (2)
37-44: Terminal tool rename to tool-run_terminal_cmd is correct in input-available branch.Logic remains unchanged and aligns with other files.
59-86: File tool renames to tool-read_file/tool-write_file in output-available branch are correct.Behavior remains consistent, including range derivation and content cleaning.
components/ui/todo-block.tsx (1)
7-7: Good move: centralize todo stats via getTodoStats.Reduces duplication and keeps stats consistent with other views.
lib/ai/tools/utils/todo-manager.ts (1)
16-20: Constructor behavior is sound; shallow copy prevents external mutation of internal state.No issues; keeping state encapsulated is correct.
app/components/MessagePartHandler.tsx (1)
46-50: Snake_case tool routing looks correct and consistent with handlers.The updated cases for file tools, web search, terminal, and todos map cleanly to the existing handlers. This should align with the new tool registry without changing behavior.
Also applies to: 53-53, 57-58, 62-62
app/components/tools/TodoToolHandler.tsx (1)
1-7: Good move: use shared Todo type from public types.Switching to the shared
Todotype reduces duplication and keeps UI aligned with domain contracts.app/contexts/GlobalState.tsx (2)
33-41: Global todos merging and panel UI state are well-integrated.
mergeTodosuses a functional state update and a pure merge utility—good for referential stability.- Adding
isTodoPanelExpandedwith reset inresetChatis consistent with other UI state.Also applies to: 70-74, 118-122
79-84: Consider closing the sidebar in resetChatIt looks like resetChat clears the input, title, todos, and collapses the todo panel, but doesn’t close the sidebar. If your “new chat” UX should reset the entire workspace (including hiding the sidebar), you can include closeSidebar() here.
• File: app/contexts/GlobalState.tsx (Line 79)
• Update:const resetChat = () => { setInput(""); setChatTitle(null); setTodos([]); setIsTodoPanelExpanded(false); + closeSidebar(); // ← add this if starting a new chat should also hide the sidebar };• Next steps:
- Verify where resetChat is invoked (I didn’t find any direct calls in the repo) to ensure this change takes effect.
- Confirm with the UX/design team whether a “new chat” should close the sidebar or leave it open.
lib/utils/todo-utils.ts (1)
11-37: Merge logic is pure, referentially stable, and readable.Returns the original array when no changes—great for React rendering performance.
app/components/TodoPanel.tsx (1)
20-29: Gating and global sync are solid.
- Panel visibility tied to presence of active todos is clear.
- Global
isTodoPanelExpandedreflects local expand/collapse and resets on unmount/when hidden.Also applies to: 33-39, 40-46
lib/utils/message-processor.ts (2)
14-28: Terminal tool rename + input shape look correctThe rename to "tool-run_terminal_cmd" and the addition of input.is_background are consistent and non-breaking for downstream usages that read input. No issues spotted.
162-174: No separate stderr field—current implementation already aggregates both streamsI confirmed that in both
local-terminal.tsandrun-terminal-cmd.ts, theonStdoutandonStderrhandlers each write to the samedata.terminalfield. There is no distinctdata.stderrpayload anywhere in the codebase, so all stderr output is already captured and aggregated alongside stdout. No changes are needed to the message processor.Likely an incorrect or invalid review comment.
lib/ai/tools/todo-write.ts (1)
5-5: Signature simplification is goodDropping the mode flag and standardizing on a single createTodoWrite(context) reduces surface area and call-site branching. Runtime validation keeps non-merge safety intact.
app/api/chat/route.ts (1)
30-35: Route now accepts todos and forwards to createTools — goodPassing optional todos through to tool initialization is a clean integration point and keeps the route thin. No issues here.
Also applies to: 64-71
lib/ai/tools/index.ts (2)
35-36: TodoManager seeding looks goodPassing initialTodos into new TodoManager(initialTodos) aligns with the new utilities and ensures state continuity. No concerns.
13-13: Todo and ToolContext shape verified– The
types/index.tsbarrel re-exports everything fromchat.tsandagent.ts, including theTodointerface defined intypes/chat.ts.
– TheToolContextinterface intypes/agent.tsincludes bothwriter: UIMessageStreamWriterandtodoManager: TodoManager, matching the imports inlib/ai/tools/index.ts.Optional: To reduce coupling on the concrete
TodoManagerclass, you may introduce a minimal interface (e.g.ITodoManager) and updateToolContext.todoManagerto use that instead of the class directly.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
types/chat.ts (1)
49-52: Make TodoWriteInput resilient to creation + merge flows (allow optional id, readonly array, doc defaults).Right now
todos?: Todo[]forces callers to supplyidfor every item, which is awkward for "create" flows where ids are generated server-side. It also leaves the default semantics ofmergeunspecified. Consider this small, backward-compatible refinement.Apply this diff:
-export interface TodoWriteInput { - merge?: boolean; - todos?: Todo[]; -} +export type UpsertTodo = Omit<Todo, "id"> & { id?: string }; + +/** + * If `merge` is true (default), input todos are merged into the global state. + * If `merge` is false, the tool may replace the global list (implementation-defined). + */ +export interface TodoWriteInput { + merge?: boolean; + readonly todos?: ReadonlyArray<UpsertTodo>; +}I can follow up with adjustments in
lib/ai/tools/todo-write.tsto treat missingidas "create" and present in-PR guardrails to prevent accidental full replacement withoutmerge: false. Want me to push that patch?app/api/chat/route.ts (2)
29-34: Add minimal runtime validation/sanitization for client-supplied todos.We currently trust
req.json()to match{ todos?: Todo[] }. A malformed body (e.g.,todos: {}) would flow into tools. At minimum, guard the array shape here.Apply this focused change:
- const { - messages, - mode, - todos, - }: { messages: UIMessage[]; mode: ChatMode; todos?: Todo[] } = - await req.json(); + const body = (await req.json()) as Partial<{ + messages: UIMessage[]; + mode: ChatMode; + todos: unknown; + }>; + const messages = body.messages as UIMessage[]; + const mode = body.mode as ChatMode; + const todos: Todo[] | undefined = Array.isArray(body.todos) + ? (body.todos as Todo[]) + : undefined;Optionally, use a schema for strict validation and clearer 4xx errors (requires a tiny import):
// add near the top of the file: import { z } from "zod"; const TodoSchema = z.object({ id: z.string().min(1), content: z.string().min(1), status: z.enum(["pending", "in_progress", "completed", "cancelled"]), }); const RequestSchema = z.object({ messages: z.array(z.any()), // or a stricter UIMessage schema if available mode: z.enum(["agent", "ask"]), todos: z.array(TodoSchema).optional(), });Then parse with:
const { messages, mode, todos } = RequestSchema.parse(await req.json());I can wire this in and convert schema violations into
ChatSDKErrorresponses if you’d like.
63-70: Pass sanitized/immutable todos into createTools to avoid accidental mutation.Even with light validation, tools shouldn’t mutate the caller’s object graph. Pass a defensive copy (or
undefined).Apply this diff:
- const { tools, getSandbox } = createTools( + const { tools, getSandbox } = createTools( userID, writer, mode, executionMode, userLocation, - todos, + todos ? structuredClone(todos) : undefined, );If
structuredClonesupport is a concern in your runtime, I can switch this to a minimalmapcopy or a JSON round-trip.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
app/api/chat/route.ts(3 hunks)app/components/tools/TodoToolHandler.tsx(1 hunks)types/chat.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/components/tools/TodoToolHandler.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
app/api/chat/route.ts (1)
types/chat.ts (2)
ChatMode(1-1)Todo(36-40)
🔇 Additional comments (3)
app/api/chat/route.ts (3)
29-34: Overall: Nice incremental, low-risk API surface change.The optional
todos?: Todo[]plumbs cleanly through the route without perturbing unrelated flow. With a tiny guard as suggested above, this will be robust in production.
18-18: No action required:Todois correctly re-exported from@/types.The barrel file at
types/index.tsusesexport * from "./chat", andtypes/chat.tsdefines and exports theTodointerface. Therefore, importingTodofrom@/typeswill not break.• types/index.ts:
export * from "./chat"; export * from "./agent";• types/chat.ts:
export interface Todo { … }
29-34: createTools signature and call sites are now consistent
- The factory in
lib/ai/tools/index.tsdeclares six parameters (includinginitialTodos?: Todo[]).- The only usage in
app/api/chat/route.tspasses all six arguments (userID, writer, mode, executionMode, userLocation, todos).No mismatches or missing arguments found—no further action required.
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 (1)
app/api/chat/route.ts (1)
25-25: Execution time mismatch with terminal tool can cause cancellationsRoute maxDuration is 300s while run-terminal-cmd uses a 360s timeout. Align both to avoid abrupt termination.
See the paired suggestion in lib/ai/tools/run-terminal-cmd.ts to reduce tool timeout, or raise this route duration if your platform permits.
🧹 Nitpick comments (5)
lib/token-utils.ts (3)
71-95: Docstring mentions a maxTokens parameter that doesn’t existThe JSDoc says “@param maxTokens …” but the function signature only has (output, tool). Either add the parameter or update the doc. Allowing an override would be useful per-tool.
Apply one of the following:
Option A — add an optional maxTokens parameter:
-export const truncateContentByTokens = ( - output: string, - tool: string, -): string => { +export const truncateContentByTokens = ( + output: string, + tool: string, + maxTokens: number = TOOL_DEFAULT_MAX_TOKENS, +): string => { const truncationMessage = tool === "read-file" ? FILE_READ_TRUNCATION_MESSAGE : TRUNCATION_MESSAGE; - const tokens = encode(output); + const tokens = encode(output); - if (tokens.length <= TOOL_DEFAULT_MAX_TOKENS) { + if (tokens.length <= maxTokens) { return output; } // Reserve tokens for truncation message const truncationTokens = countTokens(truncationMessage); - const availableTokens = TOOL_DEFAULT_MAX_TOKENS - truncationTokens; + const availableTokens = Math.max(0, maxTokens - truncationTokens);Option B — fix the docstring:
- * @param maxTokens - Maximum number of tokens allowed (defaults to TOOL_DEFAULT_MAX_TOKENS) + * Truncates to TOOL_DEFAULT_MAX_TOKENS. For custom limits, prefer an overload or a wrapper.
97-129: Streaming wrapper may exceed the budget by the size of the truncation messageWhen the next chunk would exceed maxTokens, you emit TRUNCATION_MESSAGE without accounting for its tokens, which can push the total over budget.
Clamp the message if needed and record the final token count:
const handler = (output: string) => { if (truncated) return; const outputTokens = countTokens(output); if (totalTokens + outputTokens > maxTokens) { truncated = true; - originalHandler(TRUNCATION_MESSAGE); + // Only emit the truncation notice if we have room + if (totalTokens + countTokens(TRUNCATION_MESSAGE) <= maxTokens) { + totalTokens += countTokens(TRUNCATION_MESSAGE); + originalHandler(TRUNCATION_MESSAGE); + } return; }
138-168: Shared stdout/stderr streaming wrapper has the same “truncation message budget” concernSame budget overshoot as above; please apply the same guard here as well.
if (totalTokens + outputTokens > maxTokens) { truncated = true; - originalHandler(TRUNCATION_MESSAGE); + if (totalTokens + countTokens(TRUNCATION_MESSAGE) <= maxTokens) { + totalTokens += countTokens(TRUNCATION_MESSAGE); + originalHandler(TRUNCATION_MESSAGE); + } return; }lib/ai/tools/run-terminal-cmd.ts (1)
87-100: Preserve stdout/stderr channel info in streamed terminal events (if UI supports it)You currently multiplex both into the same writer and schema, making it impossible for the UI to distinguish stderr from stdout.
If the UI schema allows, extend the payload to include the channel:
-const createTerminalWriter = (output: string) => { +const createTerminalWriter = (output: string, channel: "stdout" | "stderr") => { writer.write({ type: "data-terminal", id: `${terminalSessionId}-${++outputCounter}`, - data: { terminal: output, toolCallId }, + data: { terminal: output, channel, toolCallId }, }); }; -const { stdoutHandler, stderrHandler } = - createSharedTokenAwareHandlers(createTerminalWriter, createTerminalWriter); +const { stdoutHandler, stderrHandler } = createSharedTokenAwareHandlers( + (s) => createTerminalWriter(s, "stdout"), + (s) => createTerminalWriter(s, "stderr"), +);If the schema can’t change, consider prefixing the streamed chunks with “[STDERR] ” in the stderr handler.
lib/ai/tools/read-file.ts (1)
20-37: Strengthen schema validation for offset/limit to prevent negative or zero valuesAlthough not introduced in this diff, offset <= 0 or negative limits will produce surprising slices (e.g., offset -1 reads from the end). Tighten the zod schema.
- offset: z - .number() + offset: z + .number() + .int() + .min(1) .optional() @@ - limit: z - .number() + limit: z + .number() + .int() + .min(1) .optional()Also consider validating offset <= total lines and returning a clear error when out of range.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
app/api/chat/route.ts(4 hunks)lib/ai/tools/read-file.ts(3 hunks)lib/ai/tools/run-terminal-cmd.ts(4 hunks)lib/system-prompt.ts(4 hunks)lib/token-utils.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
lib/ai/tools/run-terminal-cmd.ts (2)
lib/token-utils.ts (2)
createSharedTokenAwareHandlers(138-168)truncateCombinedOutput(177-239)lib/ai/tools/utils/local-terminal.ts (1)
executeLocalCommand(23-61)
lib/ai/tools/read-file.ts (1)
lib/token-utils.ts (1)
truncateContentByTokens(71-95)
lib/system-prompt.ts (2)
types/chat.ts (1)
ChatMode(1-1)types/agent.ts (1)
ExecutionMode(24-24)
app/api/chat/route.ts (2)
types/chat.ts (2)
ChatMode(1-1)Todo(36-40)lib/system-prompt.ts (1)
systemPrompt(11-114)
🔇 Additional comments (17)
lib/token-utils.ts (2)
6-14: Token budgets and user-facing truncation messages are a good baselineSTREAM_MAX_TOKENS and TOOL_DEFAULT_MAX_TOKENS look reasonable, and the distinct truncation messages for file reads vs generic output improve UX.
18-28: Validate UIMessage.parts before tokenizationIt looks like
getMessageTokenCount(and a few other helpers) assumemessage.partsis always a defined array containing{ type: "text" | "reasoning"; text?: string }. If anyUIMessagecomes in with a different shape (e.g. plaincontent: string, or parts of other types), this will throw or miscount tokens.• lib/token-utils.ts (getMessageTokenCount)
• lib/actions/index.ts (generateTitleFromUserMessage –firstMessage.parts)
• Any other codepaths that domessage.parts.filter(…)Consider guarding against non-array or missing parts, for example:
- const textContent = message.parts - .filter((part: { type: string; text?: string }) => - part.type === "text" || part.type === "reasoning", - ) - .map((part: { type: string; text?: string }) => part.text || "") - .join(" "); + const textContent = Array.isArray(message.parts) + ? message.parts + .filter((p: any) => p?.type === "text" || p?.type === "reasoning") + .map((p: any) => p.text ?? "") + .join(" ") + : String((message as any).content ?? "");– Please verify the actual
UIMessagedefinition from the"ai"package to confirm whetherpartsis always present and structured as expected. If it isn’t, this fallback will ensure accurate token counts across all message shapes.lib/ai/tools/run-terminal-cmd.ts (2)
63-70: Good use of shared token-aware handlers for streamingWrapping onStdout/onStderr with a shared token budget prevents runaway terminal streams from monopolizing tokens.
73-79: Final result truncation is consistent with streaming budget enforcementPost-exec proportional truncation avoids huge payloads while preserving both ends of the output, matching the behavior of other tools.
Also applies to: 117-123
lib/ai/tools/read-file.ts (3)
5-5: Token-aware truncation import is appropriateImporting truncateContentByTokens here keeps file tool outputs aligned with the global token budget strategy.
51-53: Apply truncation to local file reads — goodReturning an annotated truncated payload improves UX and keeps the model within limits.
81-84: Apply truncation to sandbox reads — goodConsistent with local path and with the specialized file truncation message.
lib/system-prompt.ts (7)
21-25: Agent-mode gating is clear and scopedConditionally adding the “keep going” directive only for agent mode avoids over-eager behavior in ask mode.
40-49: Tool-calling guidance expanded appropriately for agent modeGood balance: general rules apply to all, extra resilience rules apply when autonomous.
65-75: Editing guidance properly gated; avoids code-dump responses in ask modeThis reduces noise when users are only asking questions.
78-82: “Do what has been asked…” constraints tighten scope creepThese constraints help keep autonomous behaviors in check.
91-112: Sandbox environment block inclusion is correct and helpfulClear env disclosure improves reproducibility and tool selection.
114-114: Final directive aligns behavior with tool availability and parameter inferenceThis sets the right defaults for tool usage and parameter handling.
11-15: AllsystemPromptcall sites include the newmodeparameter
- Verified via
rg -n -C1 '\bsystemPrompt\s*\('that the only occurrence is in
• app/api/chat/route.ts:99 —systemPrompt(model.modelId, mode, executionMode)
No other call sites were found; there are no missing-argument risks.app/api/chat/route.ts (3)
18-18: Type surface update is correctImporting Todo and adding ChatMode/ExecutionMode types matches the broader changes.
29-34: Parsing todos from the request body — LGTMOptional todos input is a safe extension of the API.
97-103: systemPrompt invocation updated with mode — goodThis matches the new signature and ensures mode-specific instruction gating.
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lib/ai/tools/read-file.ts (2)
25-36: Validate offset/limit to be positive integersPrevent negative/zero values reaching slice(offset - 1) and slice(0, limit).
- offset: z - .number() + offset: z + .number() + .int() + .min(1) .optional() @@ - limit: z - .number() + limit: z + .number() + .int() + .min(1) .optional()
16-18: Ensure consistent line numbering in local file readsThe current
readLocalFileimplementation inlib/ai/tools/utils/local-file-operations.tsreturns raw file content (a plain string) without any line‐number prefixes, whereas the documentation inlib/ai/tools/read-file.ts(lines 16–18) promises output formatted asLINE_NUMBER|LINE_CONTENT. This discrepancy will cause the local execution mode to diverge from the sandbox behavior.Please address this by:
- Adding a line‐numbering step for local reads. For example, after reading the content in
readLocalFile, split on\nand prefix each line with its 1-based index and a pipe (|) before returning or passing totruncateOutput.- Or, centralize numbering inside
truncateOutput({ content, mode: "read-file" })so that both sandbox and local modes share the exact same formatting logic.- Confirm that once implemented, both execution paths produce identical numbered output and that empty files still yield the
"File is empty."message as documented.Files to update:
lib/ai/tools/utils/local-file-operations.ts(update or wrapreadLocalFile)- Optionally,
lib/ai/tools/utils/truncate-output.ts(or wherevertruncateOutputlives) to consolidate numbering logic
♻️ Duplicate comments (2)
app/page.tsx (1)
72-80: Bug: UI merges todos even when the tool requests a full replaceSame issue as previously flagged: when toolCall.input.merge is false, the UI still calls mergeTodos(), desyncing client and server.
Apply this diff to honor merge=false and add minimal guards:
- onToolCall: ({ toolCall }) => { - // Handle todo-write tool calls to update global state - if (toolCall.toolName === "todo_write" && toolCall.input) { - const todoInput = toolCall.input as { merge: boolean; todos: Todo[] }; - if (todoInput.todos) { - mergeTodos(todoInput.todos); - } - } - }, + onToolCall: ({ toolCall }) => { + // Handle todo-write tool calls to update global state + if (toolCall.toolName === "todo_write" && toolCall.input) { + const { merge, todos: incoming } = toolCall.input as Partial<{ + merge: boolean; + todos: Todo[]; + }>; + if (Array.isArray(incoming)) { + // Default to merge when merge is undefined; replace when explicitly false + if (merge === false) { + replaceTodos(incoming); + } else { + mergeTodos(incoming); + } + } + } + },Run to verify replaceTodos is exported and wired in GlobalState:
#!/bin/bash # Verify replaceTodos exists and is exposed by GlobalState rg -n -C2 'replaceTodos' app/contexts/GlobalState.tsx # Ensure the context value includes replaceTodos rg -nP -C2 'GlobalStateContext|createContext|value\s*=\s*{[^}]*replaceTodos' app/contexts/GlobalState.tsxlib/ai/tools/run-terminal-cmd.ts (1)
12-14: Time budget mismatch vs. route maxDuration (duplicate of prior feedback)MAX_COMMAND_EXECUTION_TIME is 6 minutes while app/api/chat/route.ts sets maxDuration = 300s; the route may terminate the request while the tool still runs. Align the tool timeout below the route ceiling.
-const MAX_COMMAND_EXECUTION_TIME = 6 * 60 * 1000; // 6 minutes +// Stay safely below route maxDuration (300s) with buffer for cleanup/streaming +const MAX_COMMAND_EXECUTION_TIME = 4.5 * 60 * 1000; // 270_000 ms#!/bin/bash # Confirm current route maxDuration and occurrences of MAX_COMMAND_EXECUTION_TIME rg -nP 'export\s+const\s+maxDuration\s*=\s*\d+' app | sed -n 'p' rg -n 'MAX_COMMAND_EXECUTION_TIME' -g 'lib/**/run-terminal-cmd.ts'
🧹 Nitpick comments (4)
lib/ai/tools/run-terminal-cmd.ts (2)
30-33: Tiny grammar fix in tool guidelines"Don't" vs "Dont".
-7. Dont include any newlines in the command. +7. Don't include any newlines in the command.
115-121: Preserve stdout/stderr stream identity in sandbox eventsSandbox streaming ignores isStderr, unlike local streaming. Emit the stream flag so the UI can render correctly. Also cleanup on timeout.
- const createTerminalWriter = (output: string) => { + const createTerminalWriter = (output: string, isStderr = false) => { writer.write({ type: "data-terminal", id: `${terminalSessionId}-${++outputCounter}`, - data: { terminal: output, toolCallId }, + data: { terminal: output, isStderr, toolCallId }, }); }; @@ - const handler = createTerminalHandler( - (output) => createTerminalWriter(output), + const handler = createTerminalHandler( + (output, isStderr) => createTerminalWriter(output, isStderr), { timeoutSeconds: STREAM_TIMEOUT_SECONDS, onTimeout: () => { if (!resolved) { resolved = true; const result = handler.getResult(); - resolve({ result: { ...result, exitCode: null } }); + handler.cleanup(); + resolve({ result: { ...result, exitCode: null } }); } }, }, );Note: if consumers of data-terminal events don’t expect isStderr yet, update them accordingly.
Also applies to: 126-139
lib/token-utils.ts (1)
64-83: Expose an optional maxTokens param for truncateContenttruncateContent always uses TOOL_DEFAULT_MAX_TOKENS. Allowing an optional maxTokens unlocks per-consumer budgets without re-implementing token slicing elsewhere. Backward-compatible via default.
-export const truncateContent = ( - content: string, - suffix: string = TRUNCATION_MESSAGE, -): string => { - const tokens = encode(content); - if (tokens.length <= TOOL_DEFAULT_MAX_TOKENS) return content; +export const truncateContent = ( + content: string, + suffix: string = TRUNCATION_MESSAGE, + maxTokens: number = TOOL_DEFAULT_MAX_TOKENS, +): string => { + const tokens = encode(content); + if (tokens.length <= maxTokens) return content; - const suffixTokens = countTokens(suffix); - if (TOOL_DEFAULT_MAX_TOKENS <= suffixTokens) { - return TOOL_DEFAULT_MAX_TOKENS <= 0 + const suffixTokens = countTokens(suffix); + if (maxTokens <= suffixTokens) { + return maxTokens <= 0 ? "" - : decode(encode(suffix).slice(-TOOL_DEFAULT_MAX_TOKENS)); + : decode(encode(suffix).slice(-maxTokens)); } - const budgetForContent = TOOL_DEFAULT_MAX_TOKENS - suffixTokens; + const budgetForContent = maxTokens - suffixTokens; return decode(tokens.slice(0, budgetForContent)) + suffix; };lib/utils/terminal-executor.ts (1)
109-112: Micro-opt: avoid re-encoding combined stdout+stderrCompute token counts separately and add them rather than re-encoding the concatenation. Low impact, but cheaper on large outputs.
- const combined = stdout + stderr; - if (countTokens(combined) <= TOOL_DEFAULT_MAX_TOKENS) { + const stdoutTokens = countTokens(stdout); + const stderrTokens = countTokens(stderr); + if (stdoutTokens + stderrTokens <= TOOL_DEFAULT_MAX_TOKENS) { return { stdout, stderr }; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
app/components/tools/FileToolsHandler.tsx(2 hunks)app/components/tools/TodoToolHandler.tsx(1 hunks)app/page.tsx(6 hunks)lib/ai/tools/read-file.ts(3 hunks)lib/ai/tools/run-terminal-cmd.ts(4 hunks)lib/token-utils.ts(2 hunks)lib/utils/terminal-executor.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- app/components/tools/FileToolsHandler.tsx
- app/components/tools/TodoToolHandler.tsx
🧰 Additional context used
🧬 Code graph analysis (4)
lib/utils/terminal-executor.ts (1)
lib/token-utils.ts (5)
STREAM_MAX_TOKENS(7-7)TRUNCATION_MESSAGE(11-11)truncateContent(67-83)TIMEOUT_MESSAGE(14-15)TOOL_DEFAULT_MAX_TOKENS(8-8)
lib/ai/tools/read-file.ts (1)
lib/token-utils.ts (1)
truncateOutput(88-96)
lib/ai/tools/run-terminal-cmd.ts (2)
lib/utils/terminal-executor.ts (1)
createTerminalHandler(19-100)lib/ai/tools/utils/local-terminal.ts (1)
executeLocalCommand(23-61)
app/page.tsx (4)
app/contexts/GlobalState.tsx (1)
useGlobalState(137-143)types/chat.ts (1)
Todo(36-40)lib/utils/todo-utils.ts (1)
mergeTodos(11-37)app/components/ScrollToBottomButton.tsx (1)
ScrollToBottomButton(8-24)
🪛 Biome (2.1.2)
lib/ai/tools/run-terminal-cmd.ts
[error] 62-106: Promise executor functions should not be async.
(lint/suspicious/noAsyncPromiseExecutor)
[error] 123-174: Promise executor functions should not be async.
(lint/suspicious/noAsyncPromiseExecutor)
🔇 Additional comments (9)
app/page.tsx (4)
20-20: LGTM: Type-only import for TodoImporting Todo as a type keeps it erased at runtime and is correct here.
121-122: Todos included in send payload — goodPassing the current todos with mode keeps the backend in sync during message sends.
134-139: Todos included in regenerate payload — goodRegeneration now uses the same state snapshot, avoiding divergence between runs.
174-174: User-friendly title fallbackDisplaying "New Chat" when no title exists is a sensible UI default.
lib/ai/tools/read-file.ts (2)
5-5: Good addition: token-aware truncation wired inImporting truncateOutput keeps read-file responses within budget and aligns with the new truncation utilities. No issues.
84-90: LGTM: consistent truncation for sandbox outputJoining with numbering, then applying truncateOutput with read-file mode is correct and consistent with the local path after the above fix.
lib/token-utils.ts (2)
6-16: LGTM: clear, reusable truncation constantsSTREAM_MAX_TOKENS, TOOL_DEFAULT_MAX_TOKENS, and message constants provide a consistent surface for tool consumers.
20-27: Message token counting scope looks goodCounting text + reasoning parts and ignoring others is sane for budgeting.
lib/utils/terminal-executor.ts (1)
84-99: Timeout handling looks correctAppending TIMEOUT_MESSAGE in getResult and clearing the timer in cleanup is sound.
Summary by CodeRabbit