feat: todo-write tool #11
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds an end-to-end to-do feature: in-memory TodoManager, todoWrite and todoManager tools, message normalization and runtime activation, multiple UI components/contexts (TodoToolHandler, TodoBlock, TodoPanel, TodoBlockProvider, todo block manager), type updates, and wiring across client and server layers. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant Agent as Model/Agent
participant Tool as todoWrite
participant TM as TodoManager
participant API as chat/route
participant MP as MessagePartHandler
participant TH as TodoToolHandler
participant TB as TodoBlock UI
U->>Agent: Ask to create/update todos
Agent->>Tool: invoke todoWrite(merge, todos)
Tool->>TM: setTodos(todos, merge)
TM-->>Tool: updatedTodos + stats
Tool->>Agent: return result/counts/currentTodos
Agent->>API: stream message parts (tool/data)
API->>API: mark usedTodoWriteTool when todoWrite part observed
API->>MP: emit message with tool parts
MP->>TH: delegate rendering for tool-todoWrite / tool-todoManager
alt input-streaming / input-available
TH-->>MP: render shimmering ToolBlock (creating/updating)
else output-available
TH->>TB: render TodoBlock(currentTodos, inputTodos)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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. ✨ 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: 4
🧹 Nitpick comments (12)
app/hooks/useSidebarAutoOpen.ts (1)
177-179: LGTM on execution-state check; consider typing to avoidany.No functional change here; the boolean is correct for indicating terminal execution during streaming. Optional: define a lightweight type for
terminalToolPartto remove(as any).types/chat.ts (1)
36-40: Deduplicate status union with a reusable alias.You repeat the literal union here and in the manager module. Introduce a
TodoStatusalias to reduce drift and improve readability.Apply:
-export interface Todo { - id: string; - content: string; - status: "pending" | "in_progress" | "completed" | "cancelled"; -} +export type TodoStatus = + | "pending" + | "in_progress" + | "completed" + | "cancelled"; + +export interface Todo { + id: string; + content: string; + status: TodoStatus; +}lib/ai/tools/utils/todo-manager.ts (4)
1-5: Single source of truth for Todo type.This interface duplicates the
Todotype now exported fromtypes/chat.ts. Prefer importing the canonical type and, if you adopt aTodoStatusalias, reuse that as well to avoid divergence.-export interface Todo { - id: string; - content: string; - status: "pending" | "in_progress" | "completed" | "cancelled"; -} +import type { Todo, TodoStatus } from "@/types/chat";
7-11: Use a status alias instead of repeating the union.If you adopt
TodoStatus, reference it here to keep the status domain consistent.export interface TodoUpdate { id: string; content?: string; - status?: "pending" | "in_progress" | "completed" | "cancelled"; + status?: TodoStatus; }
28-31: Consider returning immutable snapshots to avoid accidental external mutation.
getAllTodosreturns a new array but the objects are still mutable. If external code mutates items, it bypasses manager invariants. Optionally deep-clone or freeze items before returning.I can send a small patch using
structuredClone(where available) or a manual deep copy if you want stricter immutability.
110-121: Micro: prefer for-of to index loop for readability.The intent is clearer and you avoid manual index checks. Low priority.
- if (exclusiveInProgress) { - for (let i = 0; i < this.todos.length; i++) { - if (i !== todoIndex && this.todos[i].status === "in_progress") { - this.todos[i] = { - ...this.todos[i], - status: "pending", - }; - } - } - } + if (exclusiveInProgress) { + this.todos = this.todos.map((t, i) => + i !== todoIndex && t.status === "in_progress" ? { ...t, status: "pending" } : t, + ); + }lib/ai/tools/index.ts (2)
17-23: Scope TodoManager by session (avoid cross-chat bleed).Using userID as the TodoManager session key can leak to-dos across multiple chats/tabs for the same user. Prefer a session- or conversation-scoped identifier, with userID as a fallback.
Apply this diff to accept an optional sessionId and use it when available:
export const createTools = ( userID: string, writer: UIMessageStreamWriter, mode: ChatMode = "agent", executionMode: ExecutionMode = "local", - userLocation: Geo, + userLocation: Geo, + sessionId?: string, ) => { @@ - // Create TodoManager with session ID based on userID - const todoManager = new TodoManager(userID); + // Create TodoManager with session-scoped ID (fallback to userID) + const todoManager = new TodoManager(sessionId ?? userID);Also applies to: 34-36
37-43: ToolContext extension looks consistent; ensure type-only imports to avoid runtime cycles.This addition is good. In types/agent.ts, import TodoManager as a type-only import to prevent runtime circular deps:
import type { TodoManager } from "@/lib/ai/tools/utils/todo-manager";lib/ai/tools/todo-write.ts (1)
210-215: Surface errors to the UI for better UX.Currently, failures only return an error payload; the UI handler may not render any message. Consider also emitting a data event so the UI can show an error block.
Apply this diff to broadcast the error:
} catch (error) { - return { - error: `Failed to manage todos: ${error instanceof Error ? error.message : String(error)}`, - }; + const message = `Failed to manage todos: ${ + error instanceof Error ? error.message : String(error) + }`; + try { + writer.write({ + type: "data-todo", + id: `todo-${toolCallId}`, + data: { error: true, message, toolCallId }, + }); + } catch {} + return { error: message }; }app/components/tools/TodoToolHandler.tsx (2)
2-6: De-duplicate Todo type; import the shared one.Avoid drifting local definitions. Use the central Todo type from "@/types" for consistency.
Apply this diff:
import React from "react"; import { UIMessage } from "@ai-sdk/react"; import ToolBlock from "@/components/ui/tool-block"; import { TodoBlock } from "@/components/ui/todo-block"; import { ListTodo } from "lucide-react"; +import type { Todo } from "@/types"; @@ -interface Todo { - id: string; - content: string; - status: "pending" | "in_progress" | "completed" | "cancelled"; -}Also applies to: 13-17
84-89: Use a non-empty action label for fallback block.An empty action string can look odd. Reuse the created/updated phrasing here.
Apply this diff:
- <ToolBlock + <ToolBlock key={toolCallId} icon={<ListTodo />} - action="" + action={todoInput?.merge ? "Updated to-do list" : "Created to-do list"} target={outputSummary} />components/ui/todo-block.tsx (1)
123-131: Type visibleTodos to avoid implicit any[].Explicit typing improves safety under stricter TS configs.
Apply this diff:
- const visibleTodos = []; + const visibleTodos: Todo[] = [];
📜 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 (9)
app/components/MessagePartHandler.tsx(2 hunks)app/components/tools/TodoToolHandler.tsx(1 hunks)app/hooks/useSidebarAutoOpen.ts(1 hunks)components/ui/todo-block.tsx(1 hunks)lib/ai/tools/index.ts(4 hunks)lib/ai/tools/todo-write.ts(1 hunks)lib/ai/tools/utils/todo-manager.ts(1 hunks)types/agent.ts(2 hunks)types/chat.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (8)
types/chat.ts (1)
lib/ai/tools/utils/todo-manager.ts (1)
Todo(1-5)
lib/ai/tools/todo-write.ts (1)
types/agent.ts (1)
ToolContext(16-22)
types/agent.ts (1)
lib/ai/tools/utils/todo-manager.ts (1)
TodoManager(17-169)
app/components/MessagePartHandler.tsx (1)
app/components/tools/TodoToolHandler.tsx (1)
TodoToolHandler(19-95)
components/ui/todo-block.tsx (1)
types/chat.ts (2)
Todo(36-40)TodoBlockProps(42-45)
app/components/tools/TodoToolHandler.tsx (3)
lib/ai/tools/utils/todo-manager.ts (1)
Todo(1-5)types/chat.ts (1)
Todo(36-40)components/ui/todo-block.tsx (1)
TodoBlock(48-242)
lib/ai/tools/index.ts (3)
lib/ai/tools/utils/todo-manager.ts (1)
TodoManager(17-169)types/agent.ts (1)
ToolContext(16-22)lib/ai/tools/todo-write.ts (1)
createTodoWrite(5-217)
lib/ai/tools/utils/todo-manager.ts (1)
types/chat.ts (1)
Todo(36-40)
🪛 Biome (2.1.2)
app/components/tools/TodoToolHandler.tsx
[error] 57-64: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 77-81: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🔇 Additional comments (9)
lib/ai/tools/utils/todo-manager.ts (1)
140-154: LGTM: straightforward stats computation.The stats aggregation is clear and matches the documented “done = completed + cancelled” semantics.
types/agent.ts (2)
4-4: LGTM: type-only import for TodoManager keeps emit clean.Importing as type avoids bundling the class at runtime in type-only contexts.
21-21: Verify Breaking Change: RequiredtodoManagerField in ToolContextAdding a required
todoManager: TodoManagerproperty toToolContextis a breaking change. Please confirm every creator of aToolContextnow suppliestodoManager.Suggested checks:
- Search all references to
ToolContext(usages, annotations, imports).- Find object literals typed as
ToolContextand ensure they includetodoManager.- Locate any existing
todoManager:occurrences to verify correct assignment.Run these updated commands from the repo root:
# 1. References to ToolContext rg -nP '\bToolContext\b' -C3 -g '*.ts' -g '*.tsx' # 2. Object literals initializing ToolContext rg -nP '(?:const|let)\s+\w+\s*:\s*ToolContext\s*=\s*{[^}]*}' -n -U -C2 -g '*.ts' -g '*.tsx' # 3. Occurrences of todoManager: rg -nP '\btodoManager\s*:' -n -C2 -g '*.ts' -g '*.tsx'Also manually inspect:
- Tool registration modules
- Server handler factories
- Any helper functions constructing
ToolContextapp/components/MessagePartHandler.tsx (2)
6-6: LGTM: import TodoToolHandler.Import is correctly scoped and used only for the new part types.
61-64: Good: unified handler for both data and tool parts of to-do flow.Routing both
data-todoandtool-todoWriteto the same handler simplifies UI state management and keeps rendering consistent.lib/ai/tools/index.ts (1)
53-55: Nice: todoWrite is correctly wired into both full and "ask" tool sets.Good integration and sensible exposure in ask mode.
Also applies to: 62-63
lib/ai/tools/todo-write.ts (1)
175-183: Good: UI sync pushes current todo state back to the client.Emitting a deterministic id per toolCall and bundling merge + todos + toolCallId makes it straightforward for the UI to correlate state. Looks solid.
app/components/tools/TodoToolHandler.tsx (1)
24-31: Ensure part shape consistency for “data-todo” and “tool-todoWrite”I confirmed that MessagePartHandler simply does:
case "data-todo": case "tool-todoWrite": return <TodoToolHandler message={message} part={part} status={status} />;without normalizing or adding missing fields. Meanwhile, TodoToolHandler immediately does:
const { toolCallId, state, input, output } = part; const todoInput = input as { merge: boolean; todos: Todo[] };For
data-todo(writer events), those properties may be undefined by default. Please verify that the upstream part-creation logic always populatestoolCallId,state,input, andoutputfor both event types—or add a normalization step before destructuring to guard against missing fields.• File: app/components/tools/TodoToolHandler.tsx (lines 24–31)
components/ui/todo-block.tsx (1)
183-241: Overall: thoughtful UX and accessible interactions.Nice collapsible block, status-driven styling, and pragmatic “collapsed vs. view all” logic. The header summary and keyboard interactions are well done.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
app/api/chat/route.ts (1)
59-61: Gated activation of todoManager after first todoWrite is solid; a couple of edge cases to consider.
- The prepareStep-based promotion of activeTools works and is type-safe. Good use of keyof typeof tools to retain types.
- Minor caveat: if the model calls todoWrite at the very last step and finishes before another step executes, prepareStep won’t run again to activate todoManager in that same request. If that matters UX-wise, consider also allowing promotion triggered by the tool-call event itself (only if the SDK supports dynamically updating activeTools mid-step).
Optional improvements:
- Persist manager availability across requests in the same conversation (e.g., via a conversation-scoped flag in storage). Right now, availability resets per request, which may be intentional given the in-memory manager.
- If you want to keep the model’s tool selection hints minimal before activation, you could also restrict the tools object itself (not just activeTools) to omit todoManager until activated. Current approach is fine and simpler; this is purely a UX nudge.
Overall, the flow is correct and low-risk.
Also applies to: 71-76, 107-107, 111-120, 124-128
lib/ai/tools/index.ts (2)
34-35: TodoManager lifecycle note: per-request instance.Instantiating TodoManager inside createTools makes it per-request/stream and ephemeral. If that’s the intended scope (session-local and reset each turn), great. If you want todo state to persist across a chat, consider caching per conversation/user.
I can sketch a simple in-memory cache keyed by userID/conversationID if you want to persist todos for the session.
61-65: Ask mode exposure is fine; note interplay with route gating.Having both tools present in the ask-mode map is okay since the route-level activeTools gating prevents premature invocation. If you prefer stricter hiding prior to activation, you can omit todoManager here and only append it when the route promotes activeTools.
lib/ai/tools/utils/todo-manager.ts (2)
30-70: Handle duplicate IDs within a single setTodos call to avoid accidental duplicate entries.If newTodos contains the same id multiple times, you’ll push multiple entries (for merge=false after reset) or apply last write wins in an unpredictable order. Dedup input first to make behavior deterministic.
Apply this diff:
setTodos( newTodos: (Partial<Todo> & { id: string })[], merge: boolean = false, ): Todo[] { if (!merge) { // Replace all todos this.todos = []; } - for (const todo of newTodos) { + // Deduplicate by id; last occurrence wins + const deduped = new Map<string, Partial<Todo> & { id: string }>(); + for (const t of newTodos) { + if (!t.id) { + throw new Error("Todo must have an id"); + } + deduped.set(t.id, t); + } + + for (const todo of deduped.values()) { // Defensive check - should never happen with proper typing, but provides clear error if (!todo.id) { throw new Error("Todo must have an id"); } const existingIndex = this.todos.findIndex((t) => t.id === todo.id); if (existingIndex >= 0) { // Update existing todo, preserve existing content if not provided this.todos[existingIndex] = { id: todo.id, content: todo.content ?? this.todos[existingIndex].content, status: todo.status ?? this.todos[existingIndex].status, }; } else { // Add new todo // If it's the first time (not merge) and content is missing, throw error if (!merge && !todo.content) { throw new Error(`Content is required for new todos.`); } this.todos.push({ id: todo.id, content: todo.content ?? "", status: todo.status ?? "pending", }); } } return this.getAllTodos(); }
75-89: LGTM: Stats and “done = completed + cancelled” are sensible for progress.No issues. Consider exposing a reset() helper if tests or tooling need to clear state between runs, but not required.
📜 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 (6)
app/api/chat/route.ts(3 hunks)app/components/MessagePartHandler.tsx(2 hunks)app/components/tools/TodoToolHandler.tsx(1 hunks)lib/ai/tools/index.ts(4 hunks)lib/ai/tools/todo-write.ts(1 hunks)lib/ai/tools/utils/todo-manager.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- app/components/tools/TodoToolHandler.tsx
- lib/ai/tools/todo-write.ts
🧰 Additional context used
🧬 Code Graph Analysis (4)
app/api/chat/route.ts (1)
lib/actions/index.ts (1)
getAIHeaders(5-12)
lib/ai/tools/utils/todo-manager.ts (1)
types/chat.ts (1)
Todo(36-40)
app/components/MessagePartHandler.tsx (1)
app/components/tools/TodoToolHandler.tsx (1)
TodoToolHandler(19-96)
lib/ai/tools/index.ts (3)
lib/ai/tools/utils/todo-manager.ts (1)
TodoManager(17-90)types/agent.ts (1)
ToolContext(16-22)lib/ai/tools/todo-write.ts (1)
createTodoWrite(5-222)
🔇 Additional comments (6)
app/components/MessagePartHandler.tsx (1)
6-6: LGTM: correctly wires in TodoToolHandler.Import looks correct and consistent with other tool handlers.
lib/ai/tools/index.ts (3)
3-3: LGTM: new TodoManager import.
41-41: LGTM: ToolContext extended with todoManager.Type alignment with types/agent.ts looks correct.
52-54: Exposing todoWrite and todoManager via the same factory is pragmatic.Reusing createTodoWrite with isManagerMode flag keeps the surface area small and input schemas distinct. No issues.
lib/ai/tools/utils/todo-manager.ts (2)
1-5: Types are clear and match public chat types.Shapes align with types/chat.ts Todo. Good.
23-25: Good defensive copy.Returning a shallow copy prevents external mutation of internal state.
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)
types/chat.ts (1)
1-1: Add missing re-exports in thetypesbarrelThe barrel file at
types/index.tsdoes not currently re-export theTodoandTodoBlockPropsinterfaces defined intypes/chat.ts, which will break any import from@/types. Please update it to include:// types/index.ts + export { Todo, TodoBlockProps } from './chat';
🧹 Nitpick comments (7)
app/contexts/GlobalState.tsx (1)
100-104: Consider hoisting TodoBlockProvider to reduce re-rendersBecause TodoBlockProvider is nested inside GlobalStateContext, every GlobalState change re-renders the todo provider and its consumers. If feasible, place TodoBlockProvider alongside/above GlobalStateProvider in your top-level providers so the todo context is not recreated on every global state update.
types/chat.ts (1)
36-41: Deduplicate Todo type definition to a single source of truthThere is another Todo interface declared in lib/ai/tools/utils/todo-manager.ts. Duplicating domain types invites drift over time. Prefer one shared export and consume it everywhere.
Options:
- Define Todo here (types/chat.ts) and import it in lib/ai/tools/utils/todo-manager.ts.
- Or, re-export the Todo from its canonical location via a barrel (e.g., types/index.ts) and import from "@/types".
I recommend centralizing in types/chat.ts and updating the other file to import from here to keep layers clean.
As a follow-up outside this file, update lib/ai/tools/utils/todo-manager.ts:
// lib/ai/tools/utils/todo-manager.ts -import type { Todo } from "./todo-manager"; // or local declaration +import type { Todo } from "@/types/chat"; // or "@/types" if barrel re-exportslib/utils/todo-block-manager.ts (1)
17-21: Add cleanup APIs to prevent unbounded growthmessageTodoOpen can grow indefinitely across long sessions, keyed by messageId. Provide a way to clear per-message state (e.g., when a message unmounts) and optionally reset all.
You can extend the manager like this (illustrative changes outside the selected ranges):
// Add to interface interface TodoBlockManager { // ... clearMessage: (messageId: string) => void; resetAll: () => void; } // Implement const clearMessage = useCallback((messageId: string) => { setMessageTodoOpen((prev) => { if (!(messageId in prev)) return prev; const next = { ...prev }; delete next[messageId]; return next; }); }, []); const resetAll = useCallback(() => { setMessageTodoOpen({}); }, []); // Return in object return { messageTodoOpen, autoOpenTodoBlock, toggleTodoBlock, isBlockExpanded, clearMessage, resetAll, };Also applies to: 83-89
app/contexts/TodoBlockContext.tsx (2)
3-5: Import useMemo to stabilize the provider valueTo prevent unnecessary re-renders of all consumers on every parent render, memoize the value object.
Apply:
-import React, { createContext, useContext, ReactNode } from "react"; +import React, { createContext, useContext, ReactNode, useMemo } from "react";
26-31: Memoize the context value to avoid spurious consumer updatesWithout memoization, the provider’s value object changes identity each render, causing consumers to re-render even when functions are stable.
Apply:
- const value: TodoBlockContextType = { - autoOpenTodoBlock, - toggleTodoBlock, - isBlockExpanded, - }; + const value: TodoBlockContextType = useMemo( + () => ({ + autoOpenTodoBlock, + toggleTodoBlock, + isBlockExpanded, + }), + [autoOpenTodoBlock, toggleTodoBlock, isBlockExpanded], + );components/ui/todo-block.tsx (2)
223-239: Make “View All” available when collapsed (aligns with handler logic)handleToggleViewAll already promotes the block to a manual open when invoked from a collapsed state, but the control is only rendered when isExpanded. Expose it when collapsed so the UX can directly expand to the full list.
Apply:
- {headerContent.showViewAll && isExpanded && ( + {headerContent.showViewAll && ( <span onClick={handleToggleViewAll} className="text-[12px] text-muted-foreground/70 hover:text-muted-foreground transition-colors cursor-pointer p-1 ml-2" role="button" tabIndex={0} onKeyDown={(e) => { if (e.key === "Enter" || e.key === " ") { e.preventDefault(); handleToggleViewAll(e); } }} > {showAllTodos ? "Hide" : "View All"} </span> )}
140-202: Extract getVisibleTodos logic for unit testingThe visibility rules (inputTodos precedence, most-recent action, in-progress inclusion, fallback to next pending) are non-trivial. Consider extracting into a pure helper (e.g., lib/ui/todo-visibility.ts) and unit testing the key scenarios to prevent regressions.
I can extract this into a pure function and scaffold a minimal test suite covering:
- No done tasks => return all.
- With inputTodos => include those + in-progress.
- Most-recent completed vs cancelled selected correctly.
- Always include current in-progress if present.
- Fallback to next pending when nothing else visible.
Do you want me to push that refactor and tests?
📜 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 (8)
app/components/MessagePartHandler.tsx(2 hunks)app/components/tools/TodoToolHandler.tsx(1 hunks)app/contexts/GlobalState.tsx(3 hunks)app/contexts/TodoBlockContext.tsx(1 hunks)components/ui/todo-block.tsx(1 hunks)lib/ai/tools/todo-write.ts(1 hunks)lib/utils/todo-block-manager.ts(1 hunks)types/chat.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- lib/ai/tools/todo-write.ts
- app/components/tools/TodoToolHandler.tsx
- app/components/MessagePartHandler.tsx
🧰 Additional context used
🧬 Code Graph Analysis (3)
types/chat.ts (1)
lib/ai/tools/utils/todo-manager.ts (1)
Todo(1-5)
app/contexts/TodoBlockContext.tsx (1)
lib/utils/todo-block-manager.ts (1)
useTodoBlockManager(17-89)
components/ui/todo-block.tsx (2)
types/chat.ts (2)
Todo(36-40)TodoBlockProps(42-47)app/contexts/TodoBlockContext.tsx (1)
useTodoBlockContext(39-47)
🔇 Additional comments (3)
app/contexts/GlobalState.tsx (1)
100-103: TodoBlockProvider composition looks goodWrapping the app subtree with TodoBlockProvider here cleanly exposes the todo expansion state to all descendants. No functional/typing issues spotted.
lib/utils/todo-block-manager.ts (1)
22-37: State transitions and toggling behavior are sound
- autoOpenTodoBlock keeps a single autoId per message and preserves manual opens. Good.
- toggleTodoBlock correctly handles manual toggle removal, auto toggle closure, and add-to-manual semantics.
- isBlockExpanded reads from either autoId or manualIds.
No correctness issues spotted.
Also applies to: 39-68, 69-81
app/contexts/TodoBlockContext.tsx (1)
20-37: Provider wiring is correctThe context shape and provider value align with the hook’s API, and the consumer hook enforces usage inside the provider. Looks good.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
lib/ai/tools/todo-write.ts (1)
137-160: Input schema blocks legitimate updates; make fields optional and allow single-item writes.The description promises “You can leave unchanged properties undefined” during merges, but the schema requires
statusand enforces.min(2)(unless manager mode). This prevents single-item updates and content-only edits.Apply:
- todos: z - .array( - z.object({ - id: z.string().describe("Unique identifier for the todo item"), - content: isManagerMode - ? z - .string() - .optional() - .describe("The description/content of the todo item") - : z.string().describe("The description/content of the todo item"), - status: z - .enum(["pending", "in_progress", "completed", "cancelled"]) - .describe("The current status of the todo item"), - }), - ) - .min(isManagerMode ? 1 : 2) - .describe("Array of todo items to write to the workspace"), + todos: z + .array( + z.object({ + id: z.string().describe("Unique identifier for the todo item"), + content: z + .string() + .optional() + .describe("The description/content of the todo item"), + status: z + .enum(["pending", "in_progress", "completed", "cancelled"]) + .optional() + .describe("The current status of the todo item"), + }), + ) + .min(1) + .describe("Array of todo items to write to the workspace"),
🧹 Nitpick comments (3)
lib/ai/providers.ts (1)
10-11: Docs: add NEXT_PUBLIC_TITLE_MODEL default to .env and READMEQuick verification summary: lib/ai/providers.ts falls back to "qwen/qwen3-coder"; .env.local.example contains NEXT_PUBLIC_TITLE_MODEL (commented) but README/config files do not reference it.
Files to update:
- lib/ai/providers.ts — default: process.env.NEXT_PUBLIC_TITLE_MODEL || "qwen/qwen3-coder". (lib/ai/providers.ts:9-10)
- lib/actions/index.ts — uses the "title-generator-model" key. (lib/actions/index.ts:50)
- .env.local.example — currently "# NEXT_PUBLIC_TITLE_MODEL=" (line ~32); add/document the default value here.
- README / config docs — no occurrences found; add a short note documenting NEXT_PUBLIC_TITLE_MODEL and its default.
Suggested change (one-line example to add to .env.local.example):
NEXT_PUBLIC_TITLE_MODEL=qwen/qwen3-coder # default used when NEXT_PUBLIC_TITLE_MODEL is unsetlib/ai/tools/todo-write.ts (1)
9-9: Remove unused writer from context destructure.
writeris never used and may trigger lint failures.Apply:
- const { writer, todoManager } = context; + const { todoManager } = context;lib/utils/message-processor.ts (1)
92-112: Interrupted tool transformation is pragmatic; consider stronger typing to catch schema drift.The
anycasts and stringly-typedtypechecks are fine for now. As the tool surface grows, promoteBaseToolPart/unions to shared types to prevent regressions (e.g., mismatchedoutputshapes for specialized tools).I can add a minimal
ToolParttype module and plug it here plus in tool handlers/tests; want a follow-up PR?
📜 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 ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
.env.local.example(1 hunks)app/page.tsx(1 hunks)lib/ai/providers.ts(1 hunks)lib/ai/tools/todo-write.ts(1 hunks)lib/utils/message-processor.ts(2 hunks)package.json(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- .env.local.example
🔇 Additional comments (5)
package.json (1)
14-46: All clear: dependency versions aligned and useChat API usage verified
- Confirmed package.json majors:
• ai ^5.0.19 (major 5)
• @ai-sdk/react ^2.0.19 & @ai-sdk/openai ^2.0.17 (major 2)
• next 15.5.0 (major 15)
These line up with each other and your eslint-config-next peer requirement.- Verified in app/page.tsx:
• useChat(…) at line 35
• setMessages(…) at lines 30 and 46
No missing adoption of the new setMessages API.No further action needed—dependency bumps look safe and chat integration is up to date.
lib/ai/tools/todo-write.ts (1)
182-199: Renamecounts.completedtocounts.donefor clarityBased on the code in
lib/ai/tools/todo-write.ts, thecounts.completedfield is sourced fromstats.done(which includes both completed and cancelled items). We located one call site inlib/utils/message-processor.tsthat initializes a literal object withcompleted(line 190):
- lib/utils/message-processor.ts:
counts: { completed: 0, total: 0 },To avoid any downstream confusion, I recommend:
- In
lib/ai/tools/todo-write.ts(around lines 182–199), change:to:const counts = { completed: stats.done, total: stats.total, };const counts = { done: stats.done, total: stats.total, };- In
lib/utils/message-processor.ts(around line 190), update the initialization and any references:→counts: { completed: 0, total: 0 },counts: { done: 0, total: 0 },- After renaming, run a global search for
counts.completedto ensure no usages remain.If renaming the API surface proves too disruptive, another option is to leave the key as
completedbut update the user-facing message to read:“...completed or cancelled”
so that the mixed meaning is explicit.
lib/utils/message-processor.ts (2)
50-53: Return shape change is clean and localized.Switching to
{ messages, hasChanges }is a sensible improvement and the Page integration reflects it. No issues spotted.
63-79: Data-terminal payload shape verified — no fallback requiredAll
data-terminalparts in the codebase are emitted with adataobject containing onlyterminalandtoolCallId. No other keys (such aschunkoroutput) are ever set by the producers:
- In
lib/ai/tools/run-terminal-cmd.tsandlib/ai/tools/utils/local-terminal.ts,writer.writealways uses{ data: { terminal: output, toolCallId } }.- No occurrences of
dataPart.data.chunkordataPart.data.outputwere found.You can safely continue to use
dataPart.data.terminalwithout adding the suggested fallback logic.app/page.tsx (1)
41-48: Verify invocation frequency of prepareSendMessagesRequestStatic search shows a single definition of
prepareSendMessagesRequestin app/page.tsx (line 39), but that alone doesn’t guarantee it’ll only run once per user send—UI re-renders or hook re-installs can re-invoke it unexpectedly. To be safe:
- Add a lightweight trace or breakpoint inside the hook to confirm it fires exactly once per send, for example:
prepareSendMessagesRequest: ({ messages, body }) => { console.trace('prepareSendMessagesRequest called'); const { messages: normalizedMessages, hasChanges } = normalizeMessages(messages); if (hasChanges) setMessages(normalizedMessages); return { messages: normalizedMessages, body }; }- If you observe multiple calls (double-renders or batch splits), either:
- Move normalization out of the hook into the caller right before
sendMessage(...), or- Gate normalization with a
useRefflag so each batch is only processed once.File & line to review:
- app/page.tsx – line 39 (
prepareSendMessagesRequest)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
lib/ai/tools/todo-write.ts (3)
146-159: Schema blocks legitimate writes; makestatusoptional and allow single-item arrays.The manager accepts partial todos and defaults
statusto "pending". Requiringstatushere breaks merge updates and first-time creates that intend to rely on defaults. Also, requiring at least 2 items in non-manager mode prevents common single-item writes.Apply this diff:
todos: z .array( z.object({ id: z.string().describe("Unique identifier for the todo item"), content: isManagerMode ? z .string() .optional() .describe("The description/content of the todo item") : z.string().describe("The description/content of the todo item"), - status: z - .enum(["pending", "in_progress", "completed", "cancelled"]) - .describe("The current status of the todo item"), + status: z + .enum(["pending", "in_progress", "completed", "cancelled"]) + .optional() + .describe("The current status of the todo item"), }), ) - .min(isManagerMode ? 1 : 2) + .min(1) .describe("Array of todo items to write to the workspace"),
166-172: Align execute signature with the (proposed) optionalstatusfield.Once
statusis optional in the schema, the execute signature must match to avoid TS/runtime inconsistencies.}: { merge: boolean; todos: Array<{ id: string; content?: string; - status: "pending" | "in_progress" | "completed" | "cancelled"; + status?: "pending" | "in_progress" | "completed" | "cancelled"; }>; },
175-183: Creation-time validation should require bothcontentandstatuswhenmerge=false.You validate
contentonly. Enforcestatustoo so first-time creates are fully specified while merges remain flexible. This keeps behavior consistent with the tool’s guidance and avoids partially-created todos.// Runtime validation for non-merge operations if (!merge) { for (let i = 0; i < todos.length; i++) { const todo = todos[i]; - if (!todo.content || todo.content.trim() === '') { + if (!todo.content || todo.content.trim() === '') { throw new Error(`Todo at index ${i} is missing required content field`); } + if (todo.status == null) { + throw new Error(`Todo at index ${i} is missing required status field`); + } } }
🧹 Nitpick comments (7)
app/layout.tsx (1)
34-39: Optional: extract provider stack into a dedicated Providers component; consider scoping to routes that actually need itThis trims layout noise and can help you later relocate the stack to a route-segment layout (e.g., chat) to avoid shipping the Todo tooling to pages that don’t use it.
Apply within these lines:
- <TodoBlockProvider> - <TooltipProvider> - {children} - <Toaster /> - </TooltipProvider> - </TodoBlockProvider> + <Providers> + {children} + <Toaster /> + </Providers>New file suggestion (app/providers.tsx):
"use client"; import { TooltipProvider } from "@/components/ui/tooltip"; import { TodoBlockProvider } from "./contexts/TodoBlockContext"; export function Providers({ children }: { children: React.ReactNode }) { return ( <TodoBlockProvider> <TooltipProvider>{children}</TooltipProvider> </TodoBlockProvider> ); }Then import Providers at the top of layout.tsx (keep GlobalStateProvider as-is). If Todo is only used in specific areas, move Providers into that route’s layout to reduce global client bundle impact.
lib/ai/tools/todo-write.ts (1)
205-209: Nit: tighten the result message.- ? " No to-dos are marked in-progress, make sure to mark them before starting the next." + ? " No to-dos are marked in-progress; mark one before starting a new task." : "" }`,lib/ai/tools/utils/todo-manager.ts (5)
19-21: Return immutable copies to prevent external mutation.
getAllTodosreturns a new array but the inner objects are still mutable. Return cloned items (and consider a readonly return type) to protect internal state.- getAllTodos(): Todo[] { - return [...this.todos]; - } + getAllTodos(): ReadonlyArray<Todo> { + return this.todos.map((t) => ({ ...t })); + }
35-61: Normalizecontent(trim) and validate whitespace-only on replace.Tool-level code trims content when
merge=false, but other callers of the manager could skip that. Normalize here to keep invariants at the source.for (const todo of newTodos) { + const content = todo.content?.trim(); // Defensive check - should never happen with proper typing, but provides clear error if (!todo.id) { throw new Error("Todo must have an id"); } const existingIndex = this.todos.findIndex((t) => t.id === todo.id); if (existingIndex >= 0) { // Update existing todo, preserve existing content if not provided this.todos[existingIndex] = { id: todo.id, - content: todo.content ?? this.todos[existingIndex].content, + content: content ?? this.todos[existingIndex].content, status: todo.status ?? this.todos[existingIndex].status, }; } else { // Add new todo // If it's the first time (not merge) and content is missing, throw error - if (!merge && !todo.content) { + if (!merge && (!content || content.length === 0)) { throw new Error(`Content is required for new todos.`); } this.todos.push({ id: todo.id, - content: todo.content ?? "", + content: content ?? "", status: todo.status ?? "pending", }); } }
71-85: Minor: avoid an extra copy when calculating stats.
getStatscallsgetAllTodos()which now clones items (if you adopt the refactor). Iteratethis.todosdirectly to cut allocations; semantics remain the same.- getStats() { - const todos = this.getAllTodos(); + getStats() { + const todos = this.todos;
26-66: Optional: enforce “only one in_progress” invariant behind a flag.Docs say one task should be in progress at a time. Consider enforcing (or warning) post-update when enabled (e.g., constructor option), so callers can opt in to stricter discipline.
Sketch:
const inProgress = this.todos.filter(t => t.status === "in_progress"); if (inProgress.length > 1 && this.strictSingleInProgress) { throw new Error("Only one todo may be in_progress at a time."); }
3-7: Remove unusedTodoUpdateinterfaceI searched the entire codebase for references to
TodoUpdate(including: TodoUpdateand JSX generics) and only found its own declaration. Since it isn’t used anywhere, please either:
- Remove the
TodoUpdateinterface from
lib/ai/tools/utils/todo-manager.ts(lines 3–7), or- Implement the intended
updateTodosfunctionality that consumes this type.
📜 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 (6)
app/contexts/GlobalState.tsx(1 hunks)app/contexts/TodoBlockContext.tsx(1 hunks)app/layout.tsx(2 hunks)components/ui/todo-block.tsx(1 hunks)lib/ai/tools/todo-write.ts(1 hunks)lib/ai/tools/utils/todo-manager.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- app/contexts/TodoBlockContext.tsx
- components/ui/todo-block.tsx
- app/contexts/GlobalState.tsx
🧰 Additional context used
🧬 Code Graph Analysis (3)
app/layout.tsx (1)
app/contexts/TodoBlockContext.tsx (1)
TodoBlockProvider(20-40)
lib/ai/tools/todo-write.ts (1)
types/agent.ts (1)
ToolContext(16-22)
lib/ai/tools/utils/todo-manager.ts (1)
types/chat.ts (1)
Todo(36-40)
🔇 Additional comments (5)
app/layout.tsx (2)
34-39: Provider order looks goodWrapping Tooltip/Toaster under TodoBlockProvider inside GlobalStateProvider maintains stable context hierarchy. The memoized value in TodoBlockProvider (per its implementation) helps keep renders tight. LGTM.
10-10: The script will locate and print the top of your root layout so we can confirm the import alias style.lib/ai/tools/todo-write.ts (1)
192-195: Verifycounts.completedSemantics Before MergingI didn’t find any references to
counts.completed,counts.done, orcounts.cancelledin the repo, so it’s unclear how downstream consumers or the UI expect this field to behave. Please confirm whether “completed” should include cancelled items or strictly count only completed tasks.If the UI expects:
- “done” (completed + cancelled), you can rename the key:
- const counts = { - completed: stats.done, // Use 'done' which includes both completed and cancelled + const counts = { + done: stats.done, total: stats.total, };- Only completed tasks, you can expose both and/or rename:
const counts = { - completed: stats.done, + completed: stats.completed, + cancelled: stats.cancelled, + done: stats.done, total: stats.total, };Without evidence of existing references, please manually verify the intended semantics in any UI or consumer code that renders
counts.completed.lib/ai/tools/utils/todo-manager.ts (2)
26-29: Good: safer input type and removal of non-null assertions.Requiring
idin the input and removingtodo.id!eliminates a class of runtime errors. Update logic also clearly preserves existing fields.Also applies to: 35-41, 45-49, 57-61
45-49: Good: prevents overwriting withundefined.Using nullish coalescing keeps existing values intact when updates omit fields.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
app/components/MessagePartHandler.tsx (1)
62-65: Optional: Add support for data-todo parts to future-proof rendering.At runtime, some writers may emit
data-todoevents (not only tool-based). If that happens, they’ll currently render nothing. Either route"data-todo"here or ensureTodoToolHandlerunderstandspart.data.Option A (route here):
case "tool-todoWrite": case "tool-todoManager": + case "data-todo": return <TodoToolHandler message={message} part={part} status={status} />;Option B (teach TodoToolHandler to read
part.data) — see suggested handler sketch from previous review.Search whether any producer emits
data-todo:#!/bin/bash rg -nP --type ts --type tsx -C2 'data-todo'
🧹 Nitpick comments (22)
app/components/tools/WebSearchToolHandler.tsx (1)
21-39: Show shimmer during both submitted and streaming phasesCurrently the shimmer appears only when status === "streaming". During "input-streaming", UI may remain blank while the request is merely "submitted". Consider treating both "submitted" and "streaming" as loading to align with MessageActions and ChatInput.
Apply:
- return status === "streaming" ? ( + return (status === "submitted" || status === "streaming") ? ( <ToolBlock key={toolCallId} icon={<Search />} action="Searching web" isShimmer={true} /> ) : null;And likewise for the "input-available" branch:
- return status === "streaming" ? ( + return (status === "submitted" || status === "streaming") ? ( <ToolBlock key={toolCallId} icon={<Search />} action="Searching web" target={webInput.query} isShimmer={true} /> ) : null;Also applies to: 31-39
app/components/tools/TerminalToolHandler.tsx (4)
49-55: Mark terminal as executing during both submitted and streamingWhen state === "input-available", the tool may already be "in flight" while overall status is still "submitted". Reflect that in isExecuting for better UX.
- isExecuting: state === "input-available" && status === "streaming", + isExecuting: + state === "input-available" && + (status === "submitted" || status === "streaming"),
68-76: Render loading states during submitted phase tooMirror the app-wide pattern used in MessageActions/ChatInput by showing shimmer when status is either "submitted" or "streaming".
- return status === "streaming" ? ( + return (status === "submitted" || status === "streaming") ? ( <ToolBlock key={toolCallId} icon={<Terminal />} action="Generating command" isShimmer={true} /> ) : null;- return status === "streaming" ? ( + return (status === "submitted" || status === "streaming") ? ( <ToolBlock key={toolCallId} icon={<Terminal />} action="Executing" target={terminalInput?.command || ""} isShimmer={true} isClickable={true} onClick={handleOpenInSidebar} onKeyDown={handleKeyDown} /> ) : null;Also applies to: 78-89
90-96: Remove unused variablehasOutput is computed but never used.
- const hasOutput = combinedOutput || terminalOutput.result?.error;
7-7: Standardize import path for ChatStatus and SidebarTerminalTo maintain consistency and leverage the top-level re-exports in
types/index.ts, bothChatStatusandSidebarTerminalshould be imported from"@/types"instead of"@/types/chat". For example:- import type { ChatStatus, SidebarTerminal } from "@/types/chat"; + import type { ChatStatus, SidebarTerminal } from "@/types";This uses the existing wildcard exports in
types/index.ts(which re-exports everything from./chatand./agent) and keeps all type imports uniform across the codebase.app/components/tools/FileToolsHandler.tsx (2)
23-29: Robust offset/limit checks (avoid falsy traps)offset=0 (or limit=0) would be misinterpreted by truthy checks. Prefer explicit undefined/null checks.
- const getFileRange = () => { - if (readInput.offset && readInput.limit) { - return ` L${readInput.offset}-${readInput.offset + readInput.limit - 1}`; - } - if (!readInput.offset && readInput.limit) { - return ` L1-${readInput.limit}`; - } - return ""; - }; + const getFileRange = () => { + const hasOffset = readInput.offset !== undefined && readInput.offset !== null; + const hasLimit = readInput.limit !== undefined && readInput.limit !== null; + if (hasOffset && hasLimit) { + return ` L${readInput.offset}-${readInput.offset + readInput.limit - 1}`; + } + if (!hasOffset && hasLimit) { + return ` L1-${readInput.limit}`; + } + return ""; + };
34-41: Show shimmer for submitted and streaming phases across file toolsFor visual consistency with MessageActions/ChatInput, treat both "submitted" and "streaming" as loading. Repeat this change in each tool’s "input-streaming" and "input-available" branches.
- return status === "streaming" ? ( + return (status === "submitted" || status === "streaming") ? ( <ToolBlock key={toolCallId} icon={<FileText />} action="Reading file" isShimmer={true} /> ) : null;- return status === "streaming" ? ( + return (status === "submitted" || status === "streaming") ? ( <ToolBlock key={toolCallId} icon={<FileText />} action="Reading" target={`${readInput.target_file}${getFileRange()}`} isShimmer={true} /> ) : null;Apply the same replacement in the write/delete/edit/multi-edit branches shown in the selected ranges.
Also applies to: 43-51, 106-113, 115-123, 165-172, 174-182, 212-219, 221-231, 264-271, 273-281
app/components/ChatInput.tsx (3)
47-61: Add Escape as an additional stop shortcutCtrl+C is great; adding "esc" (and optionally "meta+." on macOS) improves UX without conflicts.
- useHotkeys( - "ctrl+c", + useHotkeys( + "ctrl+c, esc, meta+.", (e) => { e.preventDefault(); onStop(); },Also applies to: 145-161
20-21: Remove unusedstatusprop onTodoPanel
Thestatusprop is never referenced insideTodoPanel, so it can be dropped to keep the API surface minimal.• In app/components/ChatInput.tsx (lines 20–21 and 26–27):
- <TodoPanel status={status} /> + <TodoPanel />• In app/components/TodoPanel.tsx:
- Remove
status: ChatStatusfrom theTodoPanelPropsinterface (lines 9–11).- Delete
statusin the function signature at line 31 (export const TodoPanel = ({ status }: TodoPanelProps) => { … }).- Remove the
ChatStatusimport if it’s no longer used elsewhere.This cleanup ensures we’re not passing unused data and keeps the component interface lean.
66-68: Adjust input container border and rounding to match TodoPanelThe
<TodoPanel>wrapper usesborder-b-0and only rounds its top corners, but the ChatInput container still has a full border and all corners rounded—this can create a double-border or gap seam. We should derive ashowTodosflag from global state and then:
- In app/components/ChatInput.tsx:
- Destructure
todosfromuseGlobalState()and compute:const showTodos = todos.length > 0;- Render the panel only when
showTodosis true.- Update the input wrapper’s classes to drop its top border and top-corner rounding when the panel is visible.
Example diff:
--- a/app/components/ChatInput.tsx +++ b/app/components/ChatInput.tsx @@ 35,7 +35,9 @@ export const ChatInput = ({ - const { input, setInput, mode, setMode } = useGlobalState(); + const { input, setInput, mode, setMode, todos } = useGlobalState(); + // Only show TodoPanel if there are any todos + const showTodos = todos.length > 0; return ( <div className={`relative px-4 ${isCentered ? "" : "pb-3 mb-4"}`}> @@ 66,7 +68,7 @@ export const ChatInput = ({ - <TodoPanel status={status} /> + {showTodos && <TodoPanel status={status} />} - <div className="flex flex-col gap-3 rounded-[22px] transition-all relative bg-input-chat py-3 max-h-[300px] shadow-[0px_12px_32px_0px_rgba(0,0,0,0.02)] border border-black/8 dark:border-border"> + <div + className={`flex flex-col gap-3 transition-all relative bg-input-chat py-3 max-h-[300px] shadow-[0px_12px_32px_0px_rgba(0,0,0,0.02)] border border-black/8 dark:border-border ${ + showTodos ? "border-t-0 rounded-[0px_0px_22px_22px]" : "rounded-[22px]" + }`} + > {/* …rest of input area… */} </div>This change ensures that when the panel is visible you won’t see a seam between its bottom edge and the input container’s top edge.
types/chat.ts (2)
42-47: Clarify TodoBlockProps semantics (inputTodos vs todos) and enforce via naming.Downstream, TodoBlock seems to treat
inputTodosas the just-generated/updated set, whiletodosare the full current list. Consider renaming to reduce ambiguity (e.g.,proposedTodosvstodos) or at least add JSDoc here so consumers don’t invert them.Example JSDoc to add above the interface:
/** * todos: the current list rendered in the block. * inputTodos: optional proposed items (e.g., model output) to highlight/merge against current todos. * blockId: stable per-tool-call id for reconciliation. * messageId: parent message id for anchoring. */
36-40: Optional Refactor: Extract a sharedTodoStatustype and apply it consistently across the codebaseAll four status literals (
"pending" | "in_progress" | "completed" | "cancelled") are only ever used with those exact spellings, so it’s safe to centralize them. To reduce duplication and guard against future drift, defineTodoStatusonce and replace every inline union with the new type.Affected locations (update each to import/use
TodoStatusinstead of repeating the string literals):
- types/chat.ts (define and export the type alongside the interface)
- lib/ai/tools/todo-write.ts
- Zod schema (
.enum([...])and output type union)- Anywhere
status: "pending" | …appears- lib/ai/tools/utils/todo-manager.ts (interface and comparisons)
- components/ui/shared-todo-item.tsx (ICON map keys and status checks)
- components/ui/todo-block.tsx (filter clauses and object shapes)
- app/components/TodoPanel.tsx (filter calls and stats object)
- app/components/tools/TodoToolHandler.tsx (type definition for props)
Example diff for types/chat.ts:
export interface Todo { id: string; content: string; - status: "pending" | "in_progress" | "completed" | "cancelled"; + status: TodoStatus; } -export type TodoStatus = - | "pending" - | "in_progress" - | "completed" - | "cancelled"; + +/** A reusable union of all valid to-do statuses */ +export type TodoStatus = + | "pending" + | "in_progress" + | "completed" + | "cancelled";Then in each of the other files, import
TodoStatus:import { TodoStatus } from "types/chat"; interface SomeOtherTodoType { status: TodoStatus; }And remove any inline
"pending" | …unions.This refactor is purely optional but will streamline future changes if you ever add, remove, or rename statuses.
app/components/Messages.tsx (3)
10-16: Align ChatStatus import path across codebase.Same note as in MessagePartHandler: confirm
@/typesre-exportsChatStatus. If not, switch to@/types/chat.-import type { ChatStatus } from "@/types"; +import type { ChatStatus } from "@/types/chat";See the repo-wide import check in the other comment.
19-20: Naming: make it explicit that this prop is a ref.
resetSidebarAutoOpenis a RefObject, but the name reads like a function. ConsiderresetSidebarAutoOpenRefto avoid call-site confusion.- resetSidebarAutoOpen?: RefObject<(() => void) | null>; + resetSidebarAutoOpenRef?: RefObject<(() => void) | null>;(Propagate the rename where used.)
47-53: Expose reset function via ref is fine; guard against stale closures.Assigning
resetSidebarFlagto a ref works. IfresetSidebarFlagis recreated often, you might wrap it inuseCallbackat its origin to stabilize identity. Not blocking.components/ui/shared-todo-item.tsx (3)
3-3: Import Todo from a stable path.If there’s no barrel at
@/types, prefer@/types/chat.-import type { Todo } from "@/types"; +import type { Todo } from "@/types/chat";
5-13: Add aria-hidden to decorative icons.These icons are purely visual; mark them
aria-hiddento reduce noise for screen readers.- completed: <CircleCheck className="w-4 h-4 text-foreground" />, - in_progress: <CircleArrowRight className="w-4 h-4 text-foreground" />, - cancelled: <X className="w-4 h-4 text-muted-foreground" />, - pending: <Circle className="w-4 h-4 text-muted-foreground" />, + completed: <CircleCheck aria-hidden className="w-4 h-4 text-foreground" />, + in_progress: <CircleArrowRight aria-hidden className="w-4 h-4 text-foreground" />, + cancelled: <X aria-hidden className="w-4 h-4 text-muted-foreground" />, + pending: <Circle aria-hidden className="w-4 h-4 text-muted-foreground" />,
15-23: Text styles are sensible; consider distinct styling for cancelled vs pending.Currently both render as muted text. If you want canceled items to read as “done,” consider applying a strikethrough or different hue.
Example:
- return "text-muted-foreground"; + return status === "cancelled" + ? "line-through opacity-60 text-muted-foreground" + : "text-muted-foreground";app/components/TodoPanel.tsx (4)
13-29: Reduce passes over todos; compute stats in one scan.Current code filters four times. For small lists it’s fine; a single pass is simpler and faster.
-const getTodoStats = (todos: Todo[]) => { - 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; - - return { - completed, - inProgress, - pending, - cancelled, - total, - done, - }; -}; +const getTodoStats = (todos: Todo[]) => { + let completed = 0, + inProgress = 0, + pending = 0, + cancelled = 0; + for (const t of todos) { + if (t.status === "completed") completed++; + else if (t.status === "in_progress") inProgress++; + else if (t.status === "pending") pending++; + else if (t.status === "cancelled") cancelled++; + } + const total = todos.length; + const done = completed + cancelled; + return { completed, inProgress, pending, cancelled, total, done }; +};
31-35: Parameterstatusis unused; consider removing or prefixing with underscore to satisfy linters.Avoids no-unused-vars warnings in stricter ESLint configs.
-export const TodoPanel = ({ status }: TodoPanelProps) => { +export const TodoPanel = (_props: TodoPanelProps) => {Or:
-export const TodoPanel = ({ status }: TodoPanelProps) => { +export const TodoPanel = ({ status: _status }: TodoPanelProps) => {
41-46: Business rule: panel hides when all todos are done.This matches the intent to keep UI clean. If you want a recap after completion, consider showing a compact “All done (N)” chip instead of hiding.
65-76: Button already handles Enter/Space; add aria-expanded and drop custom keydown.The native button provides keyboard activation by default. Keep it simple and convey state to AT.
- <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"} - tabIndex={0} - onKeyDown={(e) => { - if (e.key === "Enter" || e.key === " ") { - e.preventDefault(); - handleToggleExpand(); - } - }} - > + <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} + >
📜 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 (14)
app/components/ChatInput.tsx(2 hunks)app/components/MessageActions.tsx(2 hunks)app/components/MessagePartHandler.tsx(2 hunks)app/components/Messages.tsx(1 hunks)app/components/TodoPanel.tsx(1 hunks)app/components/tools/FileToolsHandler.tsx(1 hunks)app/components/tools/TerminalToolHandler.tsx(1 hunks)app/components/tools/TodoToolHandler.tsx(1 hunks)app/components/tools/WebSearchToolHandler.tsx(1 hunks)app/contexts/GlobalState.tsx(5 hunks)components/ui/shared-todo-item.tsx(1 hunks)components/ui/todo-block.tsx(1 hunks)lib/ai/tools/todo-write.ts(1 hunks)types/chat.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- app/components/tools/TodoToolHandler.tsx
- lib/ai/tools/todo-write.ts
- app/contexts/GlobalState.tsx
- components/ui/todo-block.tsx
🧰 Additional context used
🧬 Code graph analysis (9)
app/components/ChatInput.tsx (2)
types/chat.ts (1)
ChatStatus(49-49)app/components/TodoPanel.tsx (1)
TodoPanel(31-100)
app/components/TodoPanel.tsx (3)
types/chat.ts (2)
ChatStatus(49-49)Todo(36-40)app/contexts/GlobalState.tsx (1)
useGlobalState(115-121)components/ui/shared-todo-item.tsx (1)
SharedTodoItem(25-34)
app/components/tools/FileToolsHandler.tsx (1)
types/chat.ts (1)
ChatStatus(49-49)
app/components/tools/WebSearchToolHandler.tsx (1)
types/chat.ts (1)
ChatStatus(49-49)
app/components/MessageActions.tsx (1)
types/chat.ts (1)
ChatStatus(49-49)
app/components/tools/TerminalToolHandler.tsx (1)
types/chat.ts (1)
ChatStatus(49-49)
components/ui/shared-todo-item.tsx (1)
types/chat.ts (1)
Todo(36-40)
app/components/Messages.tsx (1)
types/chat.ts (1)
ChatStatus(49-49)
app/components/MessagePartHandler.tsx (2)
types/chat.ts (1)
ChatStatus(49-49)app/components/tools/TodoToolHandler.tsx (1)
TodoToolHandler(21-89)
🔇 Additional comments (10)
app/components/tools/WebSearchToolHandler.tsx (1)
3-3: No action needed: ChatStatus imports are already unifiedAll existing references to
ChatStatusimport from the barrel module ("@/types"), and the barrel’sindex.tsre-exports everything from"./chat"(whereChatStatusis defined). There are no direct imports from"@/types/chat"remaining in the codebase, so imports are already consistent.Likely an incorrect or invalid review comment.
app/components/MessageActions.tsx (2)
49-55: Correct gating on loading stateUsing both "submitted" and "streaming" to hide actions on the last assistant message matches the intended lifecycle.
8-8: Consistent ChatStatus imports confirmedVerified that every instance of
ChatStatusin the codebase imports from the same barrel module ("@/types"), ensuring a single canonical source for the type. No discrepancies were found, so the current import paths are correct and consistent across all affected files.All set—approving these changes.
app/components/tools/FileToolsHandler.tsx (1)
4-4: No changes needed for ChatStatus importsAll instances of
ChatStatusin the codebase already use the barrel import from"@/types", so the imports are consistent across the app. Closing this suggestion.types/chat.ts (1)
49-49: AllChatStatusimports now go through@/typesbarrel
I’ve verified that every usage ofChatStatusimports from"@/types"(thetypes/index.tsbarrel which re-exports./chat) and there are no lingering imports from"@/types/chat". The centralization is complete.• app/components/MessageActions.tsx (line 8)
• app/components/MessagePartHandler.tsx (line 7)
• app/components/Messages.tsx (line 10)
• app/components/ChatInput.tsx (line 21)
• app/components/tools/WebSearchToolHandler.tsx (line 3)
• app/components/tools/TodoToolHandler.tsx (line 7)
• app/components/tools/FileToolsHandler.tsx (line 4)app/components/MessagePartHandler.tsx (2)
41-61: Message text rendering path looks solid.Plain text for user; markdown for assistant is consistent with prior behavior and prevents accidental MD interpretation of user inputs.
6-8: Ignore barrel-export concern –ChatStatusis already exported via@/typesThe root barrel file
types/index.tscontainsexport * from "./chat";and
types/chat.tsdefines and exportsexport type ChatStatus = "submitted" | "streaming" | "ready" | "error";Therefore, importing
import type { ChatStatus } from "@/types";is safe and no change is needed.
Likely an incorrect or invalid review comment.
app/components/Messages.tsx (1)
71-86: Loader gating logic is correct.The check avoids flicker by showing the loader only for the last assistant message when streaming and no visible content yet.
components/ui/shared-todo-item.tsx (1)
25-34: LGTM: memoization and displayName set correctly.Component is small, pure, and memoized; using
todo.idas key upstream is correct.app/components/TodoPanel.tsx (1)
59-61: UI polish looks good; container and collapse behaviors are consistent.Layout/styling choices are coherent with the rest of the app and reuse SharedTodoItem correctly.
Also applies to: 90-99
Summary by CodeRabbit
New Features
Bug Fixes / UX
Style