-
Notifications
You must be signed in to change notification settings - Fork 1.6k
response haulting #2765
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
response haulting #2765
Conversation
@Rish-it is attempting to deploy a commit to the Onlook Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughAdds an isStopped flag and custom stop function to the chat hook/provider, updates context to expose stop and isStopped, resets isStopped across finish/error/new-message paths, and updates waiting-state computation to respect isStopped so UI/streaming halts when stopped. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as Chat UI
participant Hook as use-chat (Provider)
participant Chat as Underlying chat API
User->>UI: Click "Send"
UI->>Hook: sendMessage()
Hook->>Hook: set isStopped = false
Hook->>Chat: start stream
Chat-->>Hook: status = streaming
Hook-->>UI: context { status: streaming, isStopped: false }
UI->>UI: isWaiting = true (streaming && !isStopped)
User->>UI: Click "Stop"
UI->>Hook: stop()
Hook->>Hook: set isStopped = true
Hook->>Chat: stop()
Hook-->>UI: context { isStopped: true }
UI->>UI: isWaiting = false (not waiting when stopped)
Note over Hook,UI: onFinish/onError reset isStopped = false for next run
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/web/client/src/app/project/[id]/_hooks/use-chat.tsx (3)
37-37
: Consider stop-state reset timing to avoid UI flicker/regressions.Resetting
isStopped
inonFinish
can race with status updates. Prefer keeping it true on user-initiated stops and only clearing on the next send, or at least avoid clearing onfinishReason === 'stop'
.Apply:
- setIsStopped(false); + if (finishReason !== 'stop') { + setIsStopped(false); + }(Optional) Keeping the
onError
reset is fine; you already clear it at the start ofsendMessage()
.Also applies to: 58-58, 73-73
91-95
: Guard stop to only toggle when actively sending/streaming.Prevents setting a “stopped” flag when nothing is in-flight, which can confuse waiting-state UIs.
Apply:
- const customStop = () => { - setIsStopped(true); - chat.stop(); - }; + const customStop = () => { + if (chat.status === 'streaming' || chat.status === 'submitted') { + setIsStopped(true); + chat.stop(); + } + };
13-16
: AlignsendMessage
signature with implementation defaultThe
sendMessage
helper provides a default for itstype
parameter, but its current type definition requires callers to always pass an argument. While a search across the codebase did not surface any zero-arg invocations, updating the signature to maketype
optional will:
- Accurately reflect the implementation’s behavior
- Prevent future TypeScript errors if callers omit the argument
- Maintain backward compatibility (no existing call sites rely on a required arg)
Please update in
apps/web/client/src/app/project/[id]/_hooks/use-chat.tsx
(around lines 13–16):-type ExtendedUseChatHelpers = UseChatHelpers & { - sendMessage: (type: ChatType) => Promise<string | null | undefined>; - isStopped: boolean; -}; +type ExtendedUseChatHelpers = UseChatHelpers & { + sendMessage: (type?: ChatType) => Promise<string | null | undefined>; + isStopped: boolean; +};
📜 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 (1)
apps/web/client/src/app/project/[id]/_hooks/use-chat.tsx
(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/client/src/app/project/[id]/_hooks/use-chat.tsx (2)
apps/web/client/src/components/store/editor/chat/context.ts (1)
ChatContext
(15-229)apps/web/client/src/components/store/editor/index.tsx (1)
useEditorEngine
(9-13)
🔇 Additional comments (4)
apps/web/client/src/app/project/[id]/_hooks/use-chat.tsx (4)
11-11
: LGTM: React imports expanded appropriately.
useState
addition is correct for the new stop state.
23-23
: LGTM: Introduces explicit stop state.State is well-scoped to the provider.
96-96
: LGTM: Provider overrides stop and exposes isStopped.Shape aligns with
UseChatHelpers
(stop) plus your extension.
102-102
: Confirm all possibleChatStatus
values in @ai-sdk/react and include any tool-call states inisWaiting
.Please double-check the list of statuses returned by
useChat
(e.g. by inspecting theChatStatus
type in your@ai-sdk/react
install, such asnode_modules/@ai-sdk/react/dist/index.d.ts
) to see if there’s an intermediate “tool-call” or “awaiting-tool-calls” state. If so, update the guard inapps/web/client/src/app/project/[id]/_hooks/use-chat.tsx
(around line 102) accordingly:- const isWaiting = (context.status === 'streaming' || context.status === 'submitted') && !context.isStopped; + const isWaiting = + (['streaming', 'submitted', /* add any tool-call statuses here, e.g. 'awaiting-tool-calls' */] as const) + .includes(context.status) + && !context.isStopped;You can locate the full set of statuses with a quick grep, for example:
grep -R "export type ChatStatus" -n node_modules/@ai-sdk/react
Description
Related Issues
Fixes #2761
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Important
Fixes response halting in chat by managing
isStopped
state and refines code style by removing unnecessary parentheses.isStopped
state inuse-chat.tsx
to manage chat stopping.sendMessage
andstop
functions to handleisStopped
state.isStopped
is reset on chat finish and error.onlook-preload-script.js
and other files for cleaner code.This description was created by
for 3e274fc. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Bug Fixes