Implement AI "stop" -- in the client, open ai responses/chat, and gemini backends #2704

sawka · 2025-12-19T23:42:05Z

No description provided.

coderabbitai · 2025-12-19T23:42:17Z

Warning

Rate limit exceeded

@sawka has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 8 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between f51674b and 6db7c33.

📒 Files selected for processing (1)

pkg/aiusechat/usechat.go (7 hunks)

Walkthrough

This pull request adds comprehensive connection architecture docs, front-end connection docs, and multiple code changes: refactors tab-model exports and introduces maybeUseTabModel; conditions AIPanel rendering on active tab; updates AIPanel input to show a Stop Response during streaming; exposes new public fields and adds reloadChatFromBackend and stopResponse on WaveAIModel; adds ChatStore.RemoveMessage; adds client-disconnect partial-message handling across Gemini/OpenAI/OpenAI Chat streaming paths; introduces tool-use removal/update functions and refactors tool-call processing; and exposes SSEHandlerCh.Context().

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Streaming client-disconnect and partial-message logic across gemini-backend.go, openai-backend.go, and openaichat-backend.go — verify consistency and correctness of accumulated-text capture and returned partial messages.
Tool-use lifecycle changes: UpdateToolUseData, RemoveToolUseCall implementations, usechat backend interface change, and processAllToolCalls refactor — ensure idempotence, cleanup, and correct posting behavior.
WaveAIModel API expansion and AIPanel UI changes — review state management, concurrency, and UI callbacks (stopResponse, reloadChatFromBackend).
Tab-model context and maybeUseTabModel adoption plus workspace/app provider move — ensure no places assume non-null context and lifecycle implications.
ChatStore.RemoveMessage and slices usage — confirm locking and slice mutations are safe.
SSEHandlerCh.Context() exposure — confirm callers use the context correctly and cancellation semantics align with SSE handling.

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	No pull request description was provided by the author, making it impossible to verify if any description relates to the changeset.	Add a pull request description explaining the purpose, scope, and impact of the AI 'stop' implementation changes across frontends and backends.
Docstring Coverage	⚠️ Warning	Docstring coverage is 42.31% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title directly and clearly describes the main change: implementing AI 'stop' functionality across the client, OpenAI, and Gemini backends.

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 3

🧹 Nitpick comments (4)

pkg/aiusechat/gemini/gemini-convertmessage.go (1)
461-506: Consistent implementation with OpenAI backend.

The RemoveToolUseCall function correctly:

Uses slices.Clone before slices.Delete to avoid mutating the original slice

Removes the entire message if no parts remain

Returns nil (not an error) when tool call is not found, matching the OpenAI backend behavior

One minor observation: line 500 ignores the error from PostMessage. Consider logging or returning the error for debugging purposes.
🔎 Optional: Handle PostMessage error
 		if len(updatedMsg.Parts) == 0 {
 			chatstore.DefaultChatStore.RemoveMessage(chatId, chatMsg.MessageId)
 		} else {
 			aiOpts := &uctypes.AIOptsType{
 				APIType:    chat.APIType,
 				Model:      chat.Model,
 				APIVersion: chat.APIVersion,
 			}
-			chatstore.DefaultChatStore.PostMessage(chatId, aiOpts, updatedMsg)
+			if err := chatstore.DefaultChatStore.PostMessage(chatId, aiOpts, updatedMsg); err != nil {
+				return err
+			}
 		}
aiprompts/conn-arch.md (1)
9-48: Add language identifier to fenced code block.

Per static analysis (markdownlint MD040), fenced code blocks should have a language specified. This ASCII diagram block and similar blocks throughout the file (lines 90, 137, 199, 326, 426) could use a language identifier like text or plaintext for consistency.
🔎 Example fix
-```
+```text
 ┌─────────────────────────────────────────────────────────────────┐
 │                    Block Controllers                             │
aiprompts/fe-conn-arch.md (1)

1-1002: Excellent comprehensive architecture documentation.

This documentation provides thorough coverage of Wave Terminal's frontend connection architecture, including:

Clear architecture diagrams and data flow patterns

Detailed component responsibilities and interactions

Code examples that align with the actual codebase

Connection lifecycle and error handling patterns

Best practices for view model and RPC usage

The documentation will be valuable for developers working with the connection system.

Optional: Address markdown linting suggestions

The static analysis tools suggest minor markdown style improvements:

Add language identifiers to plain code blocks (lines 9, 627, 655, 689) - helps with syntax highlighting

Consider using proper headings instead of bold text for section titles (multiple locations) - improves document structure and navigation

These are purely stylistic and don't affect the technical content.

frontend/app/aipanel/waveai-model.tsx (1)

45-50: Field visibility changes enable new public API.

Making these fields public allows the new stopResponse() method to access the underlying useChat functions. While this exposes internal state, it's a pragmatic approach for implementing the stop functionality.

Consideration: If more external access patterns emerge in the future, consider adding getter methods or creating a dedicated interface for external interactions to maintain better encapsulation.

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3084e9d and 91cc8f0.

📒 Files selected for processing (17)

aiprompts/conn-arch.md (1 hunks)
aiprompts/fe-conn-arch.md (1 hunks)
frontend/app/aipanel/aipanel.tsx (2 hunks)
frontend/app/aipanel/aipanelinput.tsx (1 hunks)
frontend/app/aipanel/waveai-model.tsx (3 hunks)
frontend/app/app.tsx (2 hunks)
frontend/app/store/tab-model.ts (3 hunks)
frontend/app/workspace/workspace.tsx (2 hunks)
pkg/aiusechat/chatstore/chatstore.go (2 hunks)
pkg/aiusechat/gemini/gemini-backend.go (2 hunks)
pkg/aiusechat/gemini/gemini-convertmessage.go (2 hunks)
pkg/aiusechat/openai/openai-backend.go (7 hunks)
pkg/aiusechat/openaichat/openaichat-backend.go (2 hunks)
pkg/aiusechat/openaichat/openaichat-convertmessage.go (2 hunks)
pkg/aiusechat/usechat-backend.go (5 hunks)
pkg/aiusechat/usechat.go (5 hunks)
pkg/web/sse/ssehandler.go (1 hunks)

🧰 Additional context used

🧠 Learnings (4)

📓 Common learnings

Learnt from: sawka
Repo: wavetermdev/waveterm PR: 2433
File: pkg/aiusechat/tools_readfile.go:197-197
Timestamp: 2025-10-15T03:21:02.229Z
Learning: In Wave Terminal's AI tool definitions (pkg/aiusechat/tools_*.go), the Description field should not mention approval requirements even when ToolApproval returns ApprovalNeedsApproval. This prevents the LLM from asking users for approval before calling the tool, avoiding redundant double-approval prompts since the runtime will enforce approval anyway.

📚 Learning: 2025-10-15T03:21:02.229Z

Learnt from: sawka
Repo: wavetermdev/waveterm PR: 2433
File: pkg/aiusechat/tools_readfile.go:197-197
Timestamp: 2025-10-15T03:21:02.229Z
Learning: In Wave Terminal's AI tool definitions (pkg/aiusechat/tools_*.go), the Description field should not mention approval requirements even when ToolApproval returns ApprovalNeedsApproval. This prevents the LLM from asking users for approval before calling the tool, avoiding redundant double-approval prompts since the runtime will enforce approval anyway.

Applied to files:

pkg/aiusechat/gemini/gemini-backend.go
pkg/aiusechat/usechat-backend.go
pkg/aiusechat/openaichat/openaichat-convertmessage.go
pkg/aiusechat/usechat.go

📚 Learning: 2025-10-21T05:09:26.916Z

Learnt from: sawka
Repo: wavetermdev/waveterm PR: 2465
File: frontend/app/onboarding/onboarding-upgrade.tsx:13-21
Timestamp: 2025-10-21T05:09:26.916Z
Learning: In the waveterm codebase, clientData is loaded and awaited in wave.ts before React runs, ensuring it is always available when components mount. This means atoms.client will have data on first render.

Applied to files:

frontend/app/app.tsx

📚 Learning: 2025-10-14T06:30:54.763Z

Learnt from: sawka
Repo: wavetermdev/waveterm PR: 2430
File: frontend/app/aipanel/aimessage.tsx:137-144
Timestamp: 2025-10-14T06:30:54.763Z
Learning: In `frontend/app/aipanel/aimessage.tsx`, the `AIToolUseGroup` component splits file operation tool calls into separate batches (`fileOpsNeedApproval` and `fileOpsNoApproval`) based on their approval state before passing them to `AIToolUseBatch`. This ensures each batch has homogeneous approval states, making group-level approval handling valid.

Applied to files:

frontend/app/aipanel/aipanelinput.tsx

🧬 Code graph analysis (7)

frontend/app/aipanel/aipanel.tsx (1)

frontend/app/store/tab-model.ts (1)

maybeUseTabModel (70-72)

frontend/app/workspace/workspace.tsx (1)

frontend/app/tab/tabcontent.tsx (1)

TabContent (76-76)

pkg/aiusechat/gemini/gemini-backend.go (2)

pkg/aiusechat/uctypes/uctypes.go (2)

WaveStopReason (255-261)

StopKindCanceled (241-241)

pkg/aiusechat/gemini/gemini-types.go (2)

GeminiChatMessage (15-20)

GeminiMessagePart (43-66)

pkg/aiusechat/usechat-backend.go (3)

pkg/aiusechat/openai/openai-backend.go (1)

RemoveToolUseCall (442-461)

pkg/aiusechat/gemini/gemini-convertmessage.go (1)

RemoveToolUseCall (461-506)

pkg/aiusechat/openaichat/openaichat-convertmessage.go (1)

RemoveToolUseCall (363-394)

pkg/aiusechat/openaichat/openaichat-backend.go (2)

pkg/aiusechat/uctypes/uctypes.go (2)

WaveStopReason (255-261)

StopKindCanceled (241-241)

pkg/aiusechat/openaichat/openaichat-types.go (2)

StoredChatMessage (118-122)

ChatRequestMessage (23-29)

pkg/aiusechat/gemini/gemini-convertmessage.go (5)

pkg/aiusechat/openai/openai-backend.go (2)

UpdateToolUseData (411-440)

RemoveToolUseCall (442-461)

pkg/aiusechat/uctypes/uctypes.go (2)

UIMessageDataToolUse (210-221)

AIOptsType (270-284)

pkg/aiusechat/chatstore/chatstore.go (1)

DefaultChatStore (19-21)

pkg/aiusechat/gemini/gemini-types.go (2)

GeminiChatMessage (15-20)

GeminiMessagePart (43-66)

pkg/wcloud/wcloud.go (1)

APIVersion (37-37)

frontend/app/aipanel/aipanelinput.tsx (2)

frontend/app/element/tooltip.tsx (1)

Tooltip (143-172)

frontend/util/util.ts (1)

cn (509-509)

🪛 Biome (2.1.2)

frontend/app/aipanel/waveai-model.tsx

[error] 344-344: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.

For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level

(lint/correctness/useHookAtTopLevel)

🪛 LanguageTool

aiprompts/fe-conn-arch.md

[style] ~63-~63: The double modal “needed Used” is nonstandard (only accepted in certain dialects). Consider “to be Used”.
Context: ...atom initialized to default if needed - Used by view models to track their connectio...

(NEEDS_FIXED)

[grammar] ~976-~976: Use a hyphen to join words.
Context: ...2000 } // 2 seconds 3. **Batch Related Operations:** typescript ...

(QB_NEW_EN_HYPHEN)

🪛 markdownlint-cli2 (0.18.1)

aiprompts/fe-conn-arch.md

9-9: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

45-45: Emphasis used instead of a heading