fix: maxTokens not used #3013
fix: maxTokens not used #3013
Conversation
|
@dingdinglz is attempting to deploy a commit to the Onlook Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughAdds a maxOutputTokens argument to the root stream call and changes a chat tool-call handler from fire-and-forget to awaiting the tool call completion, altering execution ordering and error propagation. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller
participant RootAgent
participant TextStreamer as streamText
Caller->>RootAgent: createRootAgentStream(modelConfig)
Note right of RootAgent: reads modelConfig.maxOutputTokens
RootAgent->>TextStreamer: streamText({... , maxOutputTokens})
TextStreamer-->>RootAgent: Streamed text chunks
RootAgent-->>Caller: Forward streamed output
sequenceDiagram
autonumber
participant UI
participant useChatHook as Hook
participant Tool
UI->>Hook: onToolCall(params)
Note right of Hook #DDEBF7: previously used `void Tool.call()` (fire-and-forget)
Hook->>Tool: await Tool.call(params)
Tool-->>Hook: result / error
Hook-->>UI: update isExecutingToolCall = false (after completion)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsx (1)
57-62: Simplify redundantawait+.then()pattern and add error handling for cleanup.The pattern
await promise.then(callback)is unconventional and less readable than standard approaches. Additionally, ifhandleToolCallthrows an error (despite its internal try-catch),setIsExecutingToolCall(false)may not execute, potentially leaving the UI in a stuck state.Apply this diff to simplify the pattern and ensure cleanup always runs:
onToolCall: async (toolCall) => { setIsExecutingToolCall(true); - await handleToolCall(toolCall.toolCall, editorEngine, addToolResult).then(() => { + try { + await handleToolCall(toolCall.toolCall, editorEngine, addToolResult); + } finally { setIsExecutingToolCall(false); - }); + } },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/app/**/*.tsx: Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Do not use process.env in client code; import env from @/env insteadAvoid hardcoded user-facing text; use next-intl messages/hooks
Files:
apps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsx
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsx
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libs
Files:
apps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
apps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsx
🧬 Code graph analysis (1)
apps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsx (1)
apps/web/client/src/components/tools/tools.ts (1)
handleToolCall(6-37)
1 participant
Description
maxOutputTokens set but never used
Type of Change
Important
Fix missing
maxOutputTokensparameter instreamTextcall increateRootAgentStreaminroot.ts.maxOutputTokens: modelConfig.maxOutputTokenstostreamTextcall increateRootAgentStreaminroot.tsto ensure token limit is applied.This description was created by
for 7e42a07. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Bug Fixes / UX