Conversation
WalkthroughAdds SSE-based streaming infrastructure, provider adapters, CLI test tooling, schemas, docs, and web routing changes to support useChat-compatible AI streaming. New channel-driven SSE handler (pkg/waveai/ssehandlerch.go) and AI message helpers are introduced. Provider integrations for Anthropic and OpenAI streaming (multiple files under pkg/waveai) map provider events to the SSE protocol. A top-level useChat handler and routing (pkg/waveai/usechat.go, pkg/web/web.go) resolve AI config and route requests. A CLI test harness and JSON test schema are added (cmd/testai/). Documentation files describing streaming protocols and backend design are added (aiprompts/.md). go.mod updated for OpenAI client and indirect deps. Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
pkg/waveai/googlebackend.go (3)
48-51: Never return a nil channel on errors; emit a single error on a real channel to avoid deadlocks.Current early-return paths return nil, which can block forever or panic at call sites expecting a readable channel.
Apply this diff to return a non-nil channel carrying the error (and consider removing the unreachable nil-model check in this SDK):
func (GoogleBackend) StreamCompletion(ctx context.Context, request wshrpc.WaveAIStreamRequest) chan wshrpc.RespOrErrorUnion[wshrpc.WaveAIPacketType] { @@ - client, err := genai.NewClient(ctx, clientOptions...) - if err != nil { - log.Printf("failed to create client: %v", err) - return nil - } + client, err := genai.NewClient(ctx, clientOptions...) + if err != nil { + rtn := make(chan wshrpc.RespOrErrorUnion[wshrpc.WaveAIPacketType]) + go func() { + defer close(rtn) + rtn <- makeAIError(fmt.Errorf("failed to create Google client: %v", err)) + }() + return rtn + } @@ - model := client.GenerativeModel(request.Opts.Model) - if model == nil { - log.Println("model not found") - client.Close() - return nil - } + model := client.GenerativeModel(request.Opts.Model) + // genai.GenerativeModel typically does not return nil; treat unknown model as runtime error later.Also applies to: 54-58
60-66: Validate that prompt is non-empty before accessing the last element.Accessing the last message without checking length can panic when
request.Promptis empty.Apply this diff to fail fast with a clear error:
- cs := model.StartChat() + if len(request.Prompt) == 0 { + client.Close() + rtn := make(chan wshrpc.RespOrErrorUnion[wshrpc.WaveAIPacketType]) + go func() { + defer close(rtn) + rtn <- makeAIError(fmt.Errorf("prompt must contain at least one message")) + }() + return rtn + } + cs := model.StartChat()
91-107: Guard helper functions against empty slices to prevent panics.Both helpers assume at least one element; add length checks to be defensive.
Apply this diff:
func extractHistory(history []wshrpc.WaveAIPromptMessageType) []*genai.Content { var rtn []*genai.Content - for _, h := range history[:len(history)-1] { + if len(history) <= 1 { + return nil + } + for _, h := range history[:len(history)-1] { if h.Role == "user" || h.Role == "model" { rtn = append(rtn, &genai.Content{ Role: h.Role, Parts: []genai.Part{genai.Text(h.Content)}, }) } } return rtn } func extractPrompt(prompt []wshrpc.WaveAIPromptMessageType) genai.Part { - p := prompt[len(prompt)-1] - return genai.Text(p.Content) + if len(prompt) == 0 { + return genai.Text("") + } + p := prompt[len(prompt)-1] + return genai.Text(p.Content) }
🧹 Nitpick comments (24)
pkg/waveai/googlebackend.go (2)
38-45: Add sane HTTP transport timeouts when configuring a custom proxy client.The custom client currently lacks dial/TLS timeouts, which can hang connections on network issues. Keep stream-friendly semantics by avoiding a client-wide Timeout, but set transport-level timeouts.
Apply this diff and add imports for
netandtime:+ // Optional: tighten transport timeouts while preserving streaming behavior. + // Requires: import "net" and "time". transport := &http.Transport{ - Proxy: http.ProxyURL(proxyURL), + Proxy: http.ProxyURL(proxyURL), + TLSHandshakeTimeout: 10 * time.Second, + ExpectContinueTimeout: 1 * time.Second, + IdleConnTimeout: 90 * time.Second, + DialContext: (&net.Dialer{ + Timeout: 10 * time.Second, + KeepAlive: 30 * time.Second, + }).DialContext, } httpClient := &http.Client{ Transport: transport, }
109-117: Defensive nil checks when flattening candidates.Some SDK responses can include nil candidates or nil Content; skip safely.
Apply this diff:
func convertCandidatesToText(candidates []*genai.Candidate) string { var rtn string for _, c := range candidates { - for _, p := range c.Content.Parts { + if c == nil || c.Content == nil { + continue + } + for _, p := range c.Content.Parts { rtn += fmt.Sprintf("%v", p) } } return rtn }go.mod (1)
26-26: Consider using the sashabaranov/go-openai library consistentlyThe module adds both
github.com/openai/openai-go/v2(line 26) and already hasgithub.com/sashabaranov/go-openai(line 27). Having two different OpenAI client libraries in the same project could lead to confusion and maintenance overhead.Consider standardizing on one OpenAI client library throughout the codebase. The sashabaranov library appears to already be in use and supports streaming, so you might not need the additional openai-go/v2 dependency unless it provides specific features you require.
aiprompts/aisdk-streaming.md (1)
82-82: Format URLs properly in examplesThe bare URLs in the examples should be wrapped in backticks or formatted as proper Markdown links for better readability.
-data: {"type":"source-url","sourceId":"https://example.com","url":"https://example.com"} +data: {"type":"source-url","sourceId":"https://example.com","url":"https://example.com"}The same applies to lines 90 and 98.
Also applies to: 90-90, 98-98
aiprompts/usechat-backend-design.md (1)
10-16: Add language specifiers to fenced code blocksSeveral code blocks are missing language specifiers, which affects syntax highlighting and readability.
For example, line 10 should be:
-``` +```textOr use appropriate language identifiers like
httpfor endpoint definitions andtextfor plain text blocks.Also applies to: 19-25, 32-34, 37-39, 145-150, 153-163, 173-176, 439-442, 445-449
pkg/waveai/ssehandlerch.go (2)
129-130: Consider making keepalive interval configurableThe keepalive ticker is hardcoded to 1 second. Some proxies or clients might need different intervals.
Consider adding a configurable keepalive interval:
+type SSEHandlerOptions struct { + KeepaliveInterval time.Duration +} + -func MakeSSEHandlerCh(w http.ResponseWriter, ctx context.Context) *SSEHandlerCh { +func MakeSSEHandlerCh(w http.ResponseWriter, ctx context.Context, opts *SSEHandlerOptions) *SSEHandlerCh { + if opts == nil { + opts = &SSEHandlerOptions{ + KeepaliveInterval: SSEKeepaliveInterval, + } + }
235-242: Consider non-blocking writes to prevent deadlockThe WriteData method could block if the write channel is full and the context isn't done. This uses a default case that returns an error, but in high-throughput scenarios, you might want to consider a more graceful degradation.
Consider implementing a timeout or exponential backoff for writes:
select { case h.writeCh <- SSEMessage{Type: SSEMsgData, Data: data}: return nil case <-h.ctx.Done(): return h.ctx.Err() -default: - return fmt.Errorf("write channel is full") +case <-time.After(100 * time.Millisecond): + return fmt.Errorf("write timeout: channel is full") }pkg/waveai/usechat-openai-responses.go (2)
19-80: Consider adding validation for empty input items list.The function correctly handles message conversion and empty content filtering, but it doesn't validate whether the resulting
inputItemslist is empty after filtering. This could lead to an API error if all messages are filtered out.Add validation after filtering:
// Convert messages to input items, filtering out empty content var inputItems []responses.ResponseInputItemUnionParam for _, msg := range messages { content := msg.GetContent() // Skip messages with empty content as OpenAI requires non-empty content if strings.TrimSpace(content) == "" { continue } // Convert role to EasyInputMessageRole var role responses.EasyInputMessageRole switch msg.Role { case "user": role = responses.EasyInputMessageRoleUser case "assistant": role = responses.EasyInputMessageRoleAssistant case "system": role = responses.EasyInputMessageRoleSystem default: role = responses.EasyInputMessageRoleUser } inputItems = append(inputItems, responses.ResponseInputItemParamOfMessage(content, role)) } + + // Ensure we have at least one input item + if len(inputItems) == 0 { + inputItems = append(inputItems, responses.ResponseInputItemParamOfMessage("Hello", responses.EasyInputMessageRoleUser)) + }
176-194: Redundant cleanup code in response.completed handler.The cleanup code for ending reasoning and text streams is duplicated in the
response.completedhandler (lines 176-185) when it's already handled by theresponse.output_item.doneevent. This could lead to duplicate end events being sent.Consider removing the redundant cleanup since
response.output_item.doneshould have already handled it:case "response.completed": responseDone := event.AsResponseCompleted() if !finished { usage := &OpenAIUsageResponse{} responseUsage := responseDone.Response.Usage usage.PromptTokens = int(responseUsage.InputTokens) usage.CompletionTokens = int(responseUsage.OutputTokens) usage.TotalTokens = usage.PromptTokens + usage.CompletionTokens - // End reasoning if it was started but not ended - if reasoningStarted && !reasoningEnded { - sseHandler.AiMsgReasoningEnd(reasoningId) - reasoningEnded = true - } - // End text if it was started but not ended - if textStarted && !textEnded { - sseHandler.AiMsgTextEnd(textId) - textEnded = true - } - finishReason := "stop" if responseDone.Response.Status == "completed" { finishReason = "stop" } sseHandler.AiMsgFinish(finishReason, usage) finished = true } returnaiprompts/anthropic-streaming.md (1)
373-476: Fix inconsistent budget_tokens value in thinking example.The shell example uses
"budget_tokens": 16000(line 390) while the Python example uses"budget_tokens": 16000(line 411). These are consistent, but the response shows a much smaller thinking output than would be expected with a 16000 token budget.Consider updating the example to show a more realistic budget_tokens value or expanding the response to better demonstrate the thinking process with the allocated budget.
cmd/testai/main-testai.go (1)
60-89: Add error handling for missing schema properties.The
getToolDefinitionsfunction assumes the schema structure exists but doesn't validate if the "config" key is present in the unmarshaled schema.Add validation for the config schema:
configSchema := schemas["config"] + if configSchema == nil { + fmt.Printf("Warning: 'config' schema not found in testschema.json\n") + configSchema = map[string]interface{}{ + "type": "object", + } + }pkg/web/web.go (1)
476-494: Consider extracting CORS configuration to constants.The CORS configuration in dev mode is hardcoded inline. Consider extracting these values to constants for better maintainability.
Extract CORS configuration to constants:
+const ( + DevCORSMethods = "GET, POST, PUT, DELETE, OPTIONS" + DevCORSHeaders = "Content-Type, X-Session-Id, X-AuthKey, Authorization, X-Requested-With, Accept, x-vercel-ai-ui-message-stream" + DevCORSExposeHeaders = "X-ZoneFileInfo, Content-Length, Content-Type, x-vercel-ai-ui-message-stream" +) + if wavebase.IsDevMode() { originalHandler := handler handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { origin := r.Header.Get("Origin") if origin != "" { w.Header().Set("Access-Control-Allow-Origin", origin) } - w.Header().Set("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS") - w.Header().Set("Access-Control-Allow-Headers", "Content-Type, X-Session-Id, X-AuthKey, Authorization, X-Requested-With, Accept, x-vercel-ai-ui-message-stream") - w.Header().Set("Access-Control-Expose-Headers", "X-ZoneFileInfo, Content-Length, Content-Type, x-vercel-ai-ui-message-stream") + w.Header().Set("Access-Control-Allow-Methods", DevCORSMethods) + w.Header().Set("Access-Control-Allow-Headers", DevCORSHeaders) + w.Header().Set("Access-Control-Expose-Headers", DevCORSExposeHeaders) w.Header().Set("Access-Control-Allow-Credentials", "true")pkg/waveai/usechat-openai-completions.go (1)
41-90: Consider adding validation for empty chatMessages list.Similar to the Responses API implementation, this function should validate that chatMessages is not empty after filtering.
Add validation after filtering:
} } + + // Ensure we have at least one message + if len(chatMessages) == 0 { + chatMessages = append(chatMessages, openai.UserMessage("Hello")) + } // Create request using Chat Completions APIpkg/waveai/usechat.go (6)
91-95: Simplify error handling for JSON unmarshaling.The current error handling pattern silently ignores unmarshaling errors. Consider logging or handling the error more explicitly rather than setting to nil.
- presetAiSettings = &wconfig.AiSettingsType{} - if err := json.Unmarshal(mustMarshal(preset), presetAiSettings); err == nil { - // Successfully unmarshaled preset - } else { - presetAiSettings = nil - } + presetAiSettings = &wconfig.AiSettingsType{} + if err := json.Unmarshal(mustMarshal(preset), presetAiSettings); err != nil { + // Log the error for debugging purposes + log.Printf("Failed to unmarshal preset %s: %v", presetKey, err) + presetAiSettings = nil + }
102-106: Apply consistent error handling pattern for block metadata unmarshaling.Similar to the preset unmarshaling above, this should follow the same error handling pattern.
- blockAiSettings = &wconfig.AiSettingsType{} - if err := json.Unmarshal(mustMarshal(block.Meta), blockAiSettings); err != nil { - blockAiSettings = nil - } + blockAiSettings = &wconfig.AiSettingsType{} + if err := json.Unmarshal(mustMarshal(block.Meta), blockAiSettings); err != nil { + // Log the error for debugging purposes + log.Printf("Failed to unmarshal block metadata for block %s: %v", blockId, err) + blockAiSettings = nil + }
125-127: Consider making the default model configurable or using a constant.The hardcoded model "gpt-4.1" might not be available or appropriate for all users. Consider making this configurable or at least defining it as a constant.
Define a constant at the package level:
const DefaultAIModel = "gpt-4o-mini"Then use it here:
- if aiOpts.Model == "" { - aiOpts.Model = "gpt-4.1" - } + if aiOpts.Model == "" { + aiOpts.Model = DefaultAIModel + }
167-169: Potential null pointer dereference in generateID.The function doesn't check the error from
rand.Read. While extremely unlikely to fail, it's good practice to handle it.func generateID() string { bytes := make([]byte, 16) - rand.Read(bytes) + if _, err := rand.Read(bytes); err != nil { + // Fallback to a timestamp-based ID if random generation fails + return fmt.Sprintf("%d", time.Now().UnixNano()) + } return hex.EncodeToString(bytes) }Add the import at the top:
import ( ... "time" ... )
210-213: Improve API type validation logic.The current condition is confusing with the double negative. Consider restructuring for clarity and maintainability.
- // Support OpenAI and Anthropic - if aiOpts.APIType != APIType_OpenAI && aiOpts.APIType != APIType_Anthropic && aiOpts.APIType != "" { - http.Error(w, fmt.Sprintf("Unsupported API type: %s (only OpenAI and Anthropic supported)", aiOpts.APIType), http.StatusBadRequest) - return - } + // Validate API type - support OpenAI and Anthropic + switch aiOpts.APIType { + case APIType_OpenAI, APIType_Anthropic, "": + // Valid API types (empty defaults to OpenAI) + default: + http.Error(w, fmt.Sprintf("Unsupported API type: %s (only OpenAI and Anthropic supported)", aiOpts.APIType), http.StatusBadRequest) + return + }
231-234: Unify streaming error handling across providers.StreamAnthropicResponses returns (*StopReason, error) (pkg/waveai/usechat-anthropic.go) and its error is checked/logged at pkg/waveai/usechat.go:229–236, while OpenAI streamers (pkg/waveai/usechat-openai-completions.go / pkg/waveai/usechat-openai-responses.go) return no error. Choose one consistent approach: either make the OpenAI stream functions return (*StopReason, error) and propagate/handle errors in pkg/waveai/usechat.go, or have pkg/waveai/usechat.go treat Anthropic the same as OpenAI (ignore/handle the returned error consistently); ensure SSE error events are emitted uniformly.
pkg/waveai/usechat-anthropic.go (5)
1-10: Clean up documentation comments.The comments contain reference markers (
:contentReference[oaicite:X]{index=X}) that appear to be from an internal documentation system. These should be removed or replaced with actual references for clarity.-// Package anthropicadapter streams Anthropic Messages API events and adapts them -// to our AI‑SDK style SSE parts. Mapping is based on the AI‑SDK data stream -// protocol (start/text-start/text-delta/text-end, reasoning-*, tool-input-*, finish, finish-step) :contentReference[oaicite:0]{index=0} -// and Anthropic's Messages + Streaming event schemas (message_start, -// content_block_start/delta/stop, message_delta, message_stop, error). :contentReference[oaicite:1]{index=1} :contentReference[oaicite:2]{index=2} -// -// NOTE: The public signature in api.txt references wshrpc.WaveAIOptsType; -// for this self-contained package we define WaveAIOptsType locally with the -// same shape. Adapt the import/alias as needed in your codebase. :contentReference[oaicite:3]{index=3} +// Package waveai contains the Anthropic streaming adapter that converts Anthropic Messages API events +// to AI-SDK style SSE parts. Mapping is based on the AI-SDK data stream +// protocol (start/text-start/text-delta/text-end, reasoning-*, tool-input-*, finish, finish-step) +// and Anthropic's Messages + Streaming event schemas (message_start, +// content_block_start/delta/stop, message_delta, message_stop, error).
308-309: Consider making buffer sizes configurable.The hardcoded buffer sizes (64KB initial, 8MB max) might not be suitable for all use cases. Consider making these configurable or at least defining them as constants.
+const ( + // SSE scanner buffer sizes + sseInitialBufferSize = 64 * 1024 // 64KB + sseMaxBufferSize = 8 * 1024 * 1024 // 8MB +) // Stream decoding state scanner := bufio.NewScanner(resp.Body) - scanner.Buffer(make([]byte, 64*1024), 8*1024*1024) // allow large lines + scanner.Buffer(make([]byte, sseInitialBufferSize), sseMaxBufferSize)
342-355: Simplify nested condition logic.The nested if-else structure for handling event results is complex. Consider simplifying for better readability.
- if curEvent != "" { - if stop, ret, rerr := handleAnthropicEvent(curEvent, dataBuf.String(), sse, blockMap, &toolCalls, &msgID, &model, finalStop); rerr != nil { - // Anthropic sent error event or malformed JSON. - return stop, rerr - } else if ret != nil { - // message_stop triggered return - return ret, nil - } else { - // maybe updated final stop reason (from message_delta) - if stop != nil && stop.RawReason != "" { - finalStop = stop.RawReason - } - } - } + if curEvent != "" { + stop, ret, rerr := handleAnthropicEvent(curEvent, dataBuf.String(), sse, blockMap, &toolCalls, &msgID, &model, finalStop) + + if rerr != nil { + // Anthropic sent error event or malformed JSON. + return stop, rerr + } + + if ret != nil { + // message_stop triggered return + return ret, nil + } + + // maybe updated final stop reason (from message_delta) + if stop != nil && stop.RawReason != "" { + finalStop = stop.RawReason + } + }
641-645: Simplify empty content handling.The condition checking and reassigning empty content is redundant.
} else { // Shorthand: string becomes a single text block. :contentReference[oaicite:25]{index=25} - if m.Content == "" { - m.Content = "" - } aim.Content = json.RawMessage(fmt.Sprintf("%q", m.Content)) }
652-656: Handle potential error from rand.Read.Similar to the generateID function in usechat.go, this should handle the potential error from rand.Read.
func genID(prefix string) string { var b [8]byte - _, _ = rand.Read(b[:]) + if _, err := rand.Read(b[:]); err != nil { + // Fallback to timestamp-based ID if random generation fails + return fmt.Sprintf("%s_%d", prefix, time.Now().UnixNano()) + } return fmt.Sprintf("%s_%s", prefix, hex.EncodeToString(b[:])) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (14)
aiprompts/aisdk-streaming.md(1 hunks)aiprompts/anthropic-streaming.md(1 hunks)aiprompts/usechat-backend-design.md(1 hunks)cmd/testai/main-testai.go(1 hunks)cmd/testai/testschema.json(1 hunks)go.mod(2 hunks)pkg/waveai/googlebackend.go(1 hunks)pkg/waveai/ssehandlerch.go(1 hunks)pkg/waveai/usechat-anthropic.go(1 hunks)pkg/waveai/usechat-openai-completions.go(1 hunks)pkg/waveai/usechat-openai-responses.go(1 hunks)pkg/waveai/usechat.go(1 hunks)pkg/waveai/waveai.go(2 hunks)pkg/web/web.go(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
pkg/waveai/usechat-openai-responses.go (5)
pkg/wshrpc/wshrpctypes.go (1)
WaveAIOptsType(492-503)pkg/waveai/usechat.go (1)
UseChatMessage(27-31)pkg/waveai/ssehandlerch.go (9)
SSEHandlerCh(63-75)AiMsgStart(38-38)AiMsgReasoningStart(42-42)AiMsgReasoningDelta(43-43)AiMsgTextStart(39-39)AiMsgTextDelta(40-40)AiMsgTextEnd(41-41)AiMsgReasoningEnd(44-44)AiMsgFinish(51-51)pkg/waveai/usechat-openai-completions.go (1)
OpenAIUsageResponse(35-39)pkg/wavebase/wavebase.go (1)
IsDevMode(102-104)
pkg/waveai/usechat-openai-completions.go (3)
pkg/waveai/ssehandlerch.go (6)
SSEHandlerCh(63-75)AiMsgStart(38-38)AiMsgTextStart(39-39)AiMsgTextDelta(40-40)AiMsgTextEnd(41-41)AiMsgFinish(51-51)pkg/wshrpc/wshrpctypes.go (1)
WaveAIOptsType(492-503)pkg/waveai/usechat.go (1)
UseChatMessage(27-31)
pkg/waveai/waveai.go (2)
pkg/waveai/anthropicbackend.go (2)
AnthropicBackend(21-21)AnthropicBackend(113-316)pkg/waveai/perplexitybackend.go (2)
PerplexityBackend(21-21)PerplexityBackend(53-193)
pkg/waveai/usechat-anthropic.go (3)
pkg/waveai/ssehandlerch.go (14)
SSEHandlerCh(63-75)AiMsgError(52-52)AiMsgFinish(51-51)AiMsgStart(38-38)AiMsgTextStart(39-39)AiMsgReasoningStart(42-42)AiMsgToolInputStart(45-45)AiMsgTextDelta(40-40)AiMsgReasoningDelta(43-43)AiMsgToolInputDelta(46-46)AiMsgTextEnd(41-41)AiMsgReasoningEnd(44-44)AiMsgToolInputAvailable(47-47)AiMsgFinishStep(50-50)pkg/wshrpc/wshrpctypes.go (1)
WaveAIOptsType(492-503)pkg/waveai/usechat.go (1)
UseChatMessage(27-31)
cmd/testai/main-testai.go (4)
pkg/waveai/usechat-anthropic.go (2)
ToolDefinition(62-66)StreamAnthropicResponses(197-382)pkg/wshrpc/wshrpctypes.go (1)
WaveAIOptsType(492-503)pkg/waveai/usechat.go (2)
UseChatMessage(27-31)StreamOpenAIToUseChat(155-164)pkg/waveai/ssehandlerch.go (1)
MakeSSEHandlerCh(78-86)
pkg/waveai/usechat.go (9)
pkg/wconfig/settingsconfig.go (2)
AiSettingsType(35-52)MergeAiSettings(165-229)pkg/wshrpc/wshrpctypes.go (1)
WaveAIOptsType(492-503)pkg/wstore/wstore_dbops.go (1)
DBMustGet(138-149)pkg/wconfig/filewatcher.go (1)
GetWatcher(33-57)pkg/waveai/waveai.go (2)
APIType_OpenAI(21-21)APIType_Anthropic(18-18)pkg/waveai/ssehandlerch.go (2)
SSEHandlerCh(63-75)MakeSSEHandlerCh(78-86)pkg/waveai/usechat-openai-completions.go (1)
StreamOpenAIChatCompletions(41-162)pkg/waveai/usechat-openai-responses.go (1)
StreamOpenAIResponsesAPI(82-225)pkg/waveai/usechat-anthropic.go (1)
StreamAnthropicResponses(197-382)
pkg/web/web.go (4)
pkg/waveai/usechat.go (1)
HandleAIChat(172-239)pkg/docsite/docsite.go (1)
GetDocsiteHandler(17-31)pkg/schema/schema.go (1)
GetSchemaHandler(17-31)pkg/wavebase/wavebase.go (1)
IsDevMode(102-104)
🪛 markdownlint-cli2 (0.17.2)
aiprompts/anthropic-streaming.md
23-23: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
aiprompts/aisdk-streaming.md
82-82: Bare URL used
(MD034, no-bare-urls)
90-90: Bare URL used
(MD034, no-bare-urls)
98-98: Bare URL used
(MD034, no-bare-urls)
aiprompts/usechat-backend-design.md
10-10: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
19-19: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
32-32: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
37-37: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
145-145: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
153-153: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
173-173: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
439-439: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
445-445: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (19)
pkg/waveai/googlebackend.go (1)
1-3: License header addition looks good.SPDX and copyright are correctly placed above the package declaration.
go.mod (1)
85-88: tidwall deps are transitive — keep as indirectNo direct imports found in the repo; go mod graph / go list show github.com/openai/openai-go/v2@v2.1.1 pulls in github.com/tidwall/{gjson,match,pretty,sjson}, so leaving them as indirect in go.mod (lines 85–88) is correct.
pkg/waveai/waveai.go (2)
18-19: LGTM! Good API naming standardizationThe renaming from
ApiType_toAPIType_follows Go naming conventions where acronyms should be consistently capitalized.
84-89: LGTM! Consistent application of naming conventionThe constant references have been properly updated to use the new naming convention throughout the function.
cmd/testai/testschema.json (1)
1-104: Well-structured test schemaThe JSON schema file is well-organized with proper validation constraints, including:
- Appropriate min/max values for numeric fields
- Pattern validation for repository and workflow file names
- Clear separation between configuration and data schemas
- Proper use of JSON Schema features ($ref, definitions)
aiprompts/aisdk-streaming.md (1)
1-186: Comprehensive SSE protocol documentationThe documentation effectively describes the Data Stream Protocol with:
- Clear explanation of each message type and its purpose
- Proper SSE format examples
- Support for advanced features (reasoning, tool use, multi-modal)
- Compatibility notes for frontend hooks
This aligns well with the implementation in the SSE handler code.
aiprompts/usechat-backend-design.md (1)
1-463: Excellent architectural design documentThis is a well-thought-out design that:
- Clearly explains the migration from RPC to HTTP/SSE
- Provides concrete implementation examples
- Addresses configuration resolution comprehensively
- Includes a sensible phased migration strategy
- Demonstrates significant complexity reduction (90% frontend code reduction)
The approach of proxying provider SSE streams directly rather than converting back and forth is particularly elegant.
pkg/waveai/ssehandlerch.go (2)
314-329: LGTM! Well-implemented graceful shutdownThe Close method properly handles concurrent access with appropriate locking, closes the write channel to trigger the [DONE] message, and waits for the writer goroutine to complete. This ensures clean shutdown without data loss.
1-440: Well-structured SSE handler implementationOverall, this is a solid implementation of a channel-based SSE handler with:
- Good separation of concerns between writing and message handling
- Proper concurrency control with mutexes
- Automatic keepalive handling
- Clean shutdown mechanism
- Comprehensive AI message helper methods
The implementation aligns well with the documented protocol and provides a robust foundation for the AI streaming functionality.
pkg/waveai/usechat-openai-responses.go (2)
1-17: LGTM! License header and imports are properly structured.The copyright notice and imports are appropriate for the functionality being implemented.
205-209: LGTM! Proper error handling for stream errors.The error handling correctly captures stream errors and sends them through the SSE handler before returning.
aiprompts/anthropic-streaming.md (1)
1-40: LGTM! Comprehensive documentation for Anthropic streaming.The documentation clearly explains the streaming setup with good SDK examples for both Python and TypeScript.
cmd/testai/main-testai.go (2)
20-22: LGTM! Proper use of go:embed for test schema.The embedded test schema is a clean way to include test data without external file dependencies.
178-209: LGTM! Well-structured command-line interface.The main function provides clear usage instructions and appropriate error handling for missing arguments and environment variables.
pkg/web/web.go (2)
406-412: LGTM! Proper CORS preflight handling.The addition of OPTIONS request handling without auth validation is correct for CORS preflight requests.
452-474: Good separation of concerns with router architecture.The separation of routes into
waveRouterandvdomRouterwith appropriate timeout handling is well-structured. The/api/aichatendpoint correctly bypasses timeout handling for streaming support.pkg/waveai/usechat-openai-completions.go (3)
1-15: LGTM! License header and imports are properly structured.The copyright notice and imports are appropriate for the Chat Completions API implementation.
108-144: LGTM! Clean streaming implementation.The streaming logic properly handles text deltas and finish reasons with appropriate state tracking.
153-162: LGTM! Proper cleanup handling.The cleanup logic correctly ensures that any started text stream is properly ended before sending the finish message.
| event: content_block_stop | ||
| data: {"type":"content_block_stop","index":1 } |
There was a problem hiding this comment.
Fix formatting issue in JSON response.
There's a missing space before the closing brace on line 583.
-data: {"type":"content_block_stop","index":1 }
+data: {"type":"content_block_stop","index":1}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| event: content_block_stop | |
| data: {"type":"content_block_stop","index":1 } | |
| event: content_block_stop | |
| data: {"type":"content_block_stop","index":1} |
🤖 Prompt for AI Agents
In aiprompts/anthropic-streaming.md around lines 582 to 583, the JSON line
`data: {"type":"content_block_stop","index":1 }` has a missing space before the
closing brace; update the JSON formatting to include a space before the `}` (or
otherwise make spacing consistent) so the line reads with proper spacing, e.g.,
ensure there's a space before the closing brace after the index value.
| // Example: Direct OpenAI streaming (minimal conversion) | ||
| func streamOpenAIToUseChat(w http.ResponseWriter, ctx context.Context, opts *WaveAIOptsType, messages []Message) { | ||
| client := openai.NewClient(opts.APIToken) | ||
|
|
||
| stream, err := client.CreateChatCompletionStream(ctx, openai.ChatCompletionRequest{ | ||
| Model: opts.Model, | ||
| Messages: convertToOpenAIMessages(messages), | ||
| Stream: true, | ||
| }) | ||
| if err != nil { | ||
| fmt.Fprintf(w, "data: {\"type\":\"error\",\"error\":%q}\n\n", err.Error()) | ||
| fmt.Fprintf(w, "data: [DONE]\n\n") | ||
| return | ||
| } | ||
| defer stream.Close() | ||
|
|
||
| for { | ||
| response, err := stream.Recv() | ||
| if errors.Is(err, io.EOF) { | ||
| fmt.Fprintf(w, "data: [DONE]\n\n") | ||
| return | ||
| } | ||
| if err != nil { | ||
| fmt.Fprintf(w, "data: {\"type\":\"error\",\"error\":%q}\n\n", err.Error()) | ||
| fmt.Fprintf(w, "data: [DONE]\n\n") | ||
| return | ||
| } | ||
|
|
||
| // Direct transformation: OpenAI format → useChat format | ||
| for _, choice := range response.Choices { | ||
| if choice.Delta.Content != "" { | ||
| fmt.Fprintf(w, "data: {\"type\":\"text\",\"text\":%q}\n\n", choice.Delta.Content) | ||
| } | ||
| if choice.FinishReason != "" { | ||
| fmt.Fprintf(w, "data: {\"type\":\"finish\",\"finish_reason\":%q}\n\n", choice.FinishReason) | ||
| } | ||
| } | ||
|
|
||
| w.(http.Flusher).Flush() | ||
| } | ||
| } |
There was a problem hiding this comment.
Potential resource leak in streamOpenAIToUseChat
The function creates a stream but only defers stream.Close() after checking for an error. If CreateChatCompletionStream succeeds but an error occurs before the defer statement, the stream won't be closed.
func streamOpenAIToUseChat(w http.ResponseWriter, ctx context.Context, opts *WaveAIOptsType, messages []Message) {
client := openai.NewClient(opts.APIToken)
stream, err := client.CreateChatCompletionStream(ctx, openai.ChatCompletionRequest{
Model: opts.Model,
Messages: convertToOpenAIMessages(messages),
Stream: true,
})
+ if stream != nil {
+ defer stream.Close()
+ }
if err != nil {
fmt.Fprintf(w, "data: {\"type\":\"error\",\"error\":%q}\n\n", err.Error())
fmt.Fprintf(w, "data: [DONE]\n\n")
return
}
- defer stream.Close()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Example: Direct OpenAI streaming (minimal conversion) | |
| func streamOpenAIToUseChat(w http.ResponseWriter, ctx context.Context, opts *WaveAIOptsType, messages []Message) { | |
| client := openai.NewClient(opts.APIToken) | |
| stream, err := client.CreateChatCompletionStream(ctx, openai.ChatCompletionRequest{ | |
| Model: opts.Model, | |
| Messages: convertToOpenAIMessages(messages), | |
| Stream: true, | |
| }) | |
| if err != nil { | |
| fmt.Fprintf(w, "data: {\"type\":\"error\",\"error\":%q}\n\n", err.Error()) | |
| fmt.Fprintf(w, "data: [DONE]\n\n") | |
| return | |
| } | |
| defer stream.Close() | |
| for { | |
| response, err := stream.Recv() | |
| if errors.Is(err, io.EOF) { | |
| fmt.Fprintf(w, "data: [DONE]\n\n") | |
| return | |
| } | |
| if err != nil { | |
| fmt.Fprintf(w, "data: {\"type\":\"error\",\"error\":%q}\n\n", err.Error()) | |
| fmt.Fprintf(w, "data: [DONE]\n\n") | |
| return | |
| } | |
| // Direct transformation: OpenAI format → useChat format | |
| for _, choice := range response.Choices { | |
| if choice.Delta.Content != "" { | |
| fmt.Fprintf(w, "data: {\"type\":\"text\",\"text\":%q}\n\n", choice.Delta.Content) | |
| } | |
| if choice.FinishReason != "" { | |
| fmt.Fprintf(w, "data: {\"type\":\"finish\",\"finish_reason\":%q}\n\n", choice.FinishReason) | |
| } | |
| } | |
| w.(http.Flusher).Flush() | |
| } | |
| } | |
| // Example: Direct OpenAI streaming (minimal conversion) | |
| func streamOpenAIToUseChat(w http.ResponseWriter, ctx context.Context, opts *WaveAIOptsType, messages []Message) { | |
| client := openai.NewClient(opts.APIToken) | |
| stream, err := client.CreateChatCompletionStream(ctx, openai.ChatCompletionRequest{ | |
| Model: opts.Model, | |
| Messages: convertToOpenAIMessages(messages), | |
| Stream: true, | |
| }) | |
| if stream != nil { | |
| defer stream.Close() | |
| } | |
| if err != nil { | |
| fmt.Fprintf(w, "data: {\"type\":\"error\",\"error\":%q}\n\n", err.Error()) | |
| fmt.Fprintf(w, "data: [DONE]\n\n") | |
| return | |
| } | |
| for { | |
| response, err := stream.Recv() | |
| if errors.Is(err, io.EOF) { | |
| fmt.Fprintf(w, "data: [DONE]\n\n") | |
| return | |
| } | |
| if err != nil { | |
| fmt.Fprintf(w, "data: {\"type\":\"error\",\"error\":%q}\n\n", err.Error()) | |
| fmt.Fprintf(w, "data: [DONE]\n\n") | |
| return | |
| } | |
| // Direct transformation: OpenAI format → useChat format | |
| for _, choice := range response.Choices { | |
| if choice.Delta.Content != "" { | |
| fmt.Fprintf(w, "data: {\"type\":\"text\",\"text\":%q}\n\n", choice.Delta.Content) | |
| } | |
| if choice.FinishReason != "" { | |
| fmt.Fprintf(w, "data: {\"type\":\"finish\",\"finish_reason\":%q}\n\n", choice.FinishReason) | |
| } | |
| } | |
| w.(http.Flusher).Flush() | |
| } | |
| } |
🤖 Prompt for AI Agents
aiprompts/usechat-backend-design.md around lines 233 to 273: the stream returned
by CreateChatCompletionStream must be closed on every exit path; ensure you call
defer stream.Close() immediately after confirming err==nil (no statements
between creation and defer) so the stream is always closed, and for any other
early-return error paths that may occur before the defer place an explicit
stream.Close() (or restructure into a small helper/closure that defers close) so
no code path leaks the stream.
| func (h *SSEHandlerCh) AiMsgFinish(finishReason string, usage interface{}) error { | ||
| resp := map[string]interface{}{ | ||
| "type": AiMsgFinish, | ||
| } | ||
| return h.WriteJsonData(resp) | ||
| } |
There was a problem hiding this comment.
Unused parameters in AiMsgFinish
The finishReason and usage parameters are accepted but not included in the response map.
func (h *SSEHandlerCh) AiMsgFinish(finishReason string, usage interface{}) error {
resp := map[string]interface{}{
"type": AiMsgFinish,
}
+ if finishReason != "" {
+ resp["finishReason"] = finishReason
+ }
+ if usage != nil {
+ resp["usage"] = usage
+ }
return h.WriteJsonData(resp)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func (h *SSEHandlerCh) AiMsgFinish(finishReason string, usage interface{}) error { | |
| resp := map[string]interface{}{ | |
| "type": AiMsgFinish, | |
| } | |
| return h.WriteJsonData(resp) | |
| } | |
| func (h *SSEHandlerCh) AiMsgFinish(finishReason string, usage interface{}) error { | |
| resp := map[string]interface{}{ | |
| "type": AiMsgFinish, | |
| } | |
| if finishReason != "" { | |
| resp["finishReason"] = finishReason | |
| } | |
| if usage != nil { | |
| resp["usage"] = usage | |
| } | |
| return h.WriteJsonData(resp) | |
| } |
🤖 Prompt for AI Agents
In pkg/waveai/ssehandlerch.go around lines 366 to 371, AiMsgFinish currently
ignores its finishReason and usage parameters; update the response map to
include them (for example add "finish_reason": finishReason and "usage": usage),
ensure usage is JSON-serializable (handle nil by omitting or setting to nil
explicitly), then call h.WriteJsonData(resp) as before so the finish reason and
usage are sent in the SSE payload.
pkg/waveai/usechat-anthropic.go
Outdated
| endpoint := strings.TrimRight(baseURL, "/") + "/v1/messages" // :contentReference[oaicite:11]{index=11} | ||
| apiVersion := opts.APIVersion | ||
| if apiVersion == "" { | ||
| apiVersion = "2023-06-01" // default from examples :contentReference[oaicite:12]{index=12} | ||
| } | ||
| maxTokens := opts.MaxTokens | ||
| if maxTokens <= 0 { | ||
| maxTokens = 1024 // safe default per docs/examples :contentReference[oaicite:13]{index=13} | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for base URL format.
The endpoint construction should validate the base URL format to prevent potential issues with malformed URLs.
baseURL := opts.BaseURL
if baseURL == "" {
baseURL = "https://api.anthropic.com"
}
- endpoint := strings.TrimRight(baseURL, "/") + "/v1/messages" // :contentReference[oaicite:11]{index=11}
+ // Validate and normalize base URL
+ parsedURL, err := url.Parse(baseURL)
+ if err != nil {
+ return nil, fmt.Errorf("invalid base URL: %w", err)
+ }
+ endpoint := strings.TrimRight(parsedURL.String(), "/") + "/v1/messages"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| endpoint := strings.TrimRight(baseURL, "/") + "/v1/messages" // :contentReference[oaicite:11]{index=11} | |
| apiVersion := opts.APIVersion | |
| if apiVersion == "" { | |
| apiVersion = "2023-06-01" // default from examples :contentReference[oaicite:12]{index=12} | |
| } | |
| maxTokens := opts.MaxTokens | |
| if maxTokens <= 0 { | |
| maxTokens = 1024 // safe default per docs/examples :contentReference[oaicite:13]{index=13} | |
| } | |
| baseURL := opts.BaseURL | |
| if baseURL == "" { | |
| baseURL = "https://api.anthropic.com" | |
| } | |
| // Validate and normalize base URL | |
| parsedURL, err := url.Parse(baseURL) | |
| if err != nil { | |
| return nil, fmt.Errorf("invalid base URL: %w", err) | |
| } | |
| endpoint := strings.TrimRight(parsedURL.String(), "/") + "/v1/messages" // :contentReference[oaicite:11]{index=11} | |
| apiVersion := opts.APIVersion | |
| if apiVersion == "" { | |
| apiVersion = "2023-06-01" // default from examples :contentReference[oaicite:12]{index=12} | |
| } | |
| maxTokens := opts.MaxTokens | |
| if maxTokens <= 0 { | |
| maxTokens = 1024 // safe default per docs/examples :contentReference[oaicite:13]{index=13} | |
| } |
🤖 Prompt for AI Agents
In pkg/waveai/usechat-anthropic.go around lines 217 to 225, the code currently
concatenates baseURL to build endpoint without validating baseURL; add
validation by parsing baseURL with net/url (url.Parse), ensure it has a valid
scheme (http or https) and host, normalize by trimming any trailing slash or
using url.ResolveReference to join the path, and if parsing/validation fails
return or propagate an error instead of constructing the endpoint; update the
surrounding function to handle and return this error if not already handled.
| func StreamOpenAIResponsesAPI(sseHandler *SSEHandlerCh, ctx context.Context, opts *wshrpc.WaveAIOptsType, messages []UseChatMessage) { | ||
| client, req := createOpenAIRequest(opts, messages) | ||
|
|
||
| // Create stream using Responses API | ||
| stream := client.Responses.NewStreaming(ctx, req) | ||
| defer stream.Close() | ||
|
|
||
| // Generate IDs for the streaming protocol | ||
| messageId := generateID() | ||
| textId := generateID() | ||
| reasoningId := generateID() | ||
|
|
||
| // Send message start | ||
| sseHandler.AiMsgStart(messageId) | ||
|
|
||
| // Track whether we've started text/reasoning streaming and finished | ||
| textStarted := false | ||
| textEnded := false | ||
| reasoningStarted := false | ||
| reasoningEnded := false | ||
| finished := false |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Clean up debug print statements before production.
There are multiple debug print statements that should be removed or gated behind a debug flag before production deployment.
Remove or properly gate the debug statements:
- fmt.Printf("DEBUG: Received event type: %s\n", event.Type)
+ if wavebase.IsDevMode() {
+ fmt.Printf("DEBUG: Received event type: %s\n", event.Type)
+ }Apply similar changes to lines 109, 124, and 130.
Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 30
🧹 Nitpick comments (9)
aiprompts/usechat-backend-design.md (2)
10-16: Specify language for fenced code blocks.The architecture comparison diagrams should specify a language for better rendering.
### Current Architecture -``` +```text Frontend (React) → Custom RPC → Go Backend → AI Providers - 10+ Jotai atoms for state management - Custom WaveAIStreamRequest/WaveAIPacketType - Complex configuration merging in frontend - Custom streaming protocol over WebSocketTarget Architecture
-
+text
Frontend (useChat) → HTTP/SSE → Go Backend → AI Providers
- Single useChat() hook manages all state
- Standard HTTP POST + SSE streaming
- Backend-driven configuration resolution
- Standard AI SDK streaming format
Also applies to: 19-25
32-40: Add language specifiers to remaining code blocks.Several code blocks are missing language specifiers which helps with syntax highlighting and readability.
**Chat Streaming Endpoint:** -``` +```text POST /api/ai/chat/{blockId}?preset={presetKey}Conversation Persistence Endpoints:
-+text
POST /api/ai/conversations/{blockId} # Save conversation
GET /api/ai/conversations/{blockId} # Load conversation**Headers:** -``` +```http Content-Type: text/event-stream Cache-Control: no-cache Connection: keep-alive Access-Control-Allow-Origin: *useChat Expected Format:
-+text
data: {"type":"text","text":"Hello"}data: {"type":"text","text":" world"}
data: {"type":"text","text":"!"}
data: {"type":"finish","finish_reason":"stop","usage":{"prompt_tokens":10,"completion_tokens":3,"total_tokens":13}}
data: [DONE]
**Error Format:** -``` +```text data: {"type":"error","error":"API key invalid"} data: [DONE]URL-based Configuration
-
+text
POST /api/ai/chat/block-123?preset=claude-coding
POST /api/ai/chat/block-456?preset=gpt4-creative### Header-based Overrides -``` +```http POST /api/ai/chat/block-123 X-AI-Model: gpt-4-turbo X-AI-Temperature: 0.8Also applies to: 145-150, 153-177, 439-461 </blockquote></details> <details> <summary>pkg/waveai/usechat-openai-responses.go (1)</summary><blockquote> `187-191`: **Simplify redundant finish reason logic** The finish reason assignment is redundant - it always sets "stop" regardless of the condition. ```diff - finishReason := "stop" - if responseDone.Response.Status == "completed" { - finishReason = "stop" - } + finishReason := "stop" + // TODO: Map different response statuses to appropriate finish reasonscmd/testai/testschema.json (1)
17-17: Non-standard JSON Schema propertyThe
unitsproperty is not part of the JSON Schema specification. Consider using thedescriptionfield to document units or use a custom annotation with anx-prefix.- "units": "s" + "x-units": "s"Or include the unit in the description:
- "description": "Polling interval for GitHub API requests", + "description": "Polling interval for GitHub API requests (in seconds)",cmd/testai/main-testai.go (1)
167-168: Remove trailing whitespaceThere's trailing whitespace on line 168.
tools = getToolDefinitions() } - + stopReason, err := waveai.StreamAnthropicResponses(ctx, sseHandler, opts, messages, tools)pkg/waveai/usechat.go (1)
91-95: Improve error handling for preset unmarshalingThe error from JSON unmarshaling is silently ignored. Consider logging the error for debugging purposes.
presetAiSettings = &wconfig.AiSettingsType{} if err := json.Unmarshal(mustMarshal(preset), presetAiSettings); err == nil { // Successfully unmarshaled preset } else { + log.Printf("Warning: Failed to unmarshal preset %s: %v", presetKey, err) presetAiSettings = nil }pkg/waveai/usechat-anthropic.go (1)
308-309: Consider potential scanner buffer overflow for very large SSE events.Setting a large maximum buffer size (8MB) is good, but the code doesn't handle the case where a line exceeds even this limit.
Consider logging a warning or handling this edge case explicitly if scanner.Err() returns bufio.ErrTooLong.
pkg/waveai/ssehandlerch.go (2)
217-222: Potential race condition with error channel.The error channel has a buffer of 1, but the comment says "Send error to error channel if there's space" with a non-blocking select. If multiple errors occur concurrently, only the first will be captured in errCh. Consider if this is the intended behavior or if you need a larger buffer or different error handling strategy.
241-242: Inconsistent error messages for full channel conditions.The error message "write channel is full" could be more helpful by indicating that this is likely a backpressure issue.
default: - return fmt.Errorf("write channel is full") + return fmt.Errorf("SSE write channel is full (backpressure)")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (14)
aiprompts/aisdk-streaming.md(1 hunks)aiprompts/anthropic-streaming.md(1 hunks)aiprompts/usechat-backend-design.md(1 hunks)cmd/testai/main-testai.go(1 hunks)cmd/testai/testschema.json(1 hunks)go.mod(2 hunks)pkg/waveai/googlebackend.go(1 hunks)pkg/waveai/ssehandlerch.go(1 hunks)pkg/waveai/usechat-anthropic.go(1 hunks)pkg/waveai/usechat-openai-completions.go(1 hunks)pkg/waveai/usechat-openai-responses.go(1 hunks)pkg/waveai/usechat.go(1 hunks)pkg/waveai/waveai.go(2 hunks)pkg/web/web.go(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
pkg/waveai/usechat-openai-responses.go (5)
pkg/wshrpc/wshrpctypes.go (1)
WaveAIOptsType(492-503)pkg/waveai/usechat.go (1)
UseChatMessage(27-31)pkg/waveai/ssehandlerch.go (9)
SSEHandlerCh(63-75)AiMsgStart(38-38)AiMsgReasoningStart(42-42)AiMsgReasoningDelta(43-43)AiMsgTextStart(39-39)AiMsgTextDelta(40-40)AiMsgTextEnd(41-41)AiMsgReasoningEnd(44-44)AiMsgFinish(51-51)pkg/waveai/usechat-openai-completions.go (1)
OpenAIUsageResponse(35-39)pkg/wavebase/wavebase.go (1)
IsDevMode(102-104)
pkg/waveai/usechat-openai-completions.go (3)
pkg/waveai/ssehandlerch.go (6)
SSEHandlerCh(63-75)AiMsgStart(38-38)AiMsgTextStart(39-39)AiMsgTextDelta(40-40)AiMsgTextEnd(41-41)AiMsgFinish(51-51)pkg/wshrpc/wshrpctypes.go (1)
WaveAIOptsType(492-503)pkg/waveai/usechat.go (1)
UseChatMessage(27-31)
pkg/waveai/waveai.go (2)
pkg/waveai/anthropicbackend.go (2)
AnthropicBackend(21-21)AnthropicBackend(113-316)pkg/waveai/perplexitybackend.go (2)
PerplexityBackend(21-21)PerplexityBackend(53-193)
pkg/waveai/usechat-anthropic.go (3)
pkg/waveai/ssehandlerch.go (14)
SSEHandlerCh(63-75)AiMsgError(52-52)AiMsgFinish(51-51)AiMsgStart(38-38)AiMsgTextStart(39-39)AiMsgReasoningStart(42-42)AiMsgToolInputStart(45-45)AiMsgTextDelta(40-40)AiMsgReasoningDelta(43-43)AiMsgToolInputDelta(46-46)AiMsgTextEnd(41-41)AiMsgReasoningEnd(44-44)AiMsgToolInputAvailable(47-47)AiMsgFinishStep(50-50)pkg/wshrpc/wshrpctypes.go (1)
WaveAIOptsType(492-503)pkg/waveai/usechat.go (1)
UseChatMessage(27-31)
cmd/testai/main-testai.go (4)
pkg/waveai/usechat-anthropic.go (2)
ToolDefinition(62-66)StreamAnthropicResponses(197-382)pkg/wshrpc/wshrpctypes.go (1)
WaveAIOptsType(492-503)pkg/waveai/usechat.go (2)
UseChatMessage(27-31)StreamOpenAIToUseChat(155-164)pkg/waveai/ssehandlerch.go (1)
MakeSSEHandlerCh(78-86)
pkg/waveai/usechat.go (9)
pkg/wconfig/settingsconfig.go (2)
AiSettingsType(35-52)MergeAiSettings(165-229)pkg/wshrpc/wshrpctypes.go (1)
WaveAIOptsType(492-503)pkg/wstore/wstore_dbops.go (1)
DBMustGet(138-149)pkg/wconfig/filewatcher.go (1)
GetWatcher(33-57)pkg/waveai/waveai.go (2)
APIType_OpenAI(21-21)APIType_Anthropic(18-18)pkg/waveai/ssehandlerch.go (2)
SSEHandlerCh(63-75)MakeSSEHandlerCh(78-86)pkg/waveai/usechat-openai-completions.go (1)
StreamOpenAIChatCompletions(41-162)pkg/waveai/usechat-openai-responses.go (1)
StreamOpenAIResponsesAPI(82-225)pkg/waveai/usechat-anthropic.go (1)
StreamAnthropicResponses(197-382)
pkg/web/web.go (2)
pkg/waveai/usechat.go (1)
HandleAIChat(172-239)pkg/wavebase/wavebase.go (1)
IsDevMode(102-104)
🪛 markdownlint-cli2 (0.17.2)
aiprompts/usechat-backend-design.md
10-10: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
19-19: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
32-32: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
37-37: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
145-145: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
153-153: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
173-173: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
439-439: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
445-445: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (15)
pkg/waveai/googlebackend.go (1)
1-3: LGTM! Copyright header added correctly.The addition of the copyright and SPDX license header is appropriate and follows standard practices.
go.mod (2)
85-88: Indirect tidwall dependencies look appropriate for JSON handling.The tidwall libraries (gjson, sjson, match, pretty) are commonly used for JSON manipulation and are reasonable indirect dependencies for the streaming infrastructure.
26-26: Action: Confirm and consolidate OpenAI SDK usageBoth SDKs are actually imported:
- pkg/waveai/openaibackend.go — imports github.com/sashabaranov/go-openai (alias openaiapi)
- pkg/waveai/usechat-openai-completions.go — imports github.com/openai/openai-go/v2 (and option)
- pkg/waveai/usechat-openai-responses.go — imports github.com/openai/openai-go/v2 (and responses/shared)
Actions:
- Confirm both SDKs are intentional; if not, remove the unused SDK from go.mod and update imports.
- Prefer consolidating to a single SDK for consistency; if both must remain, add comments documenting the reason and verify no conflicting configs (API keys, base URLs, duplicated behavior).
pkg/waveai/waveai.go (2)
18-19: Good standardization of API constant naming.The renaming from
ApiType_toAPIType_follows Go naming conventions where acronyms should be consistently capitalized.
84-89: Backend type assignment is consistent with the renamed constants.The updates to use
APIType_AnthropicandAPIType_Perplexityare correctly applied, maintaining the same functional behavior while improving naming consistency.aiprompts/aisdk-streaming.md (1)
1-186: Comprehensive SSE streaming protocol documentation.This documentation provides excellent coverage of the SSE-based streaming protocol with clear examples for each message type. The protocol design aligns well with the implementation in the codebase and supports multiple AI provider integrations.
aiprompts/anthropic-streaming.md (1)
1-632: Excellent Anthropic streaming documentation with error recovery.The documentation thoroughly covers Anthropic's streaming API including tool use, extended thinking, and importantly, error recovery strategies. The examples are comprehensive and will be valuable for implementation and debugging.
aiprompts/usechat-backend-design.md (1)
183-310: Well-structured implementation examples.The Go implementation examples provide excellent guidance for the HTTP handler and streaming logic. The direct proxy approach for OpenAI/Anthropic and the conversion only for Wave Cloud is an efficient design choice.
pkg/web/web.go (5)
31-31: Good integration of the AI chat functionality.The addition of the
waveaipackage import is appropriate for supporting the new AI chat endpoint.
408-412: CORS preflight handling is correctly implemented.The addition of CORS preflight handling in
WebFnWrapproperly responds to OPTIONS requests without auth validation, which is essential for browser-based clients.
453-466: Good separation of routes with different timeout requirements.Creating separate routers for
waveandvdomendpoints with appropriate timeout handlers is a clean architectural improvement. This prevents timeout issues for long-running operations while maintaining protection for standard endpoints.
469-469: AI chat endpoint correctly configured without timeout.The
/api/aichatendpoint is appropriately configured without a timeout handler, which is necessary for SSE streaming that can run for extended periods.
477-494: Approve dev-only CORS behavior — IsDevMode checks WAVETERM_DEV env varIsDevMode returns true when Dev_VarCache (populated from os.Getenv(WaveDevVarName) and then unset) is non-empty — see pkg/wavebase/wavebase.go:52, 96–104; dynamic Access-Control-Allow-Origin is therefore gated to explicit dev mode.
pkg/waveai/usechat.go (1)
146-153: Define isReasoningModel functionThis file references
isReasoningModelfunction inshouldUseChatCompletionsAPIbut doesn't define it. This is the missing function that's causing compilation issues in other files.The
isReasoningModelfunction needs to be defined. Add it to this file or create a shared utility file:+func isReasoningModel(model string) bool { + m := strings.ToLower(model) + // Models with reasoning capabilities (o3, o4, etc.) + return strings.HasPrefix(m, "o3") || + strings.HasPrefix(m, "o4") || + strings.Contains(m, "reasoning") +} + func shouldUseChatCompletionsAPI(model string) bool {Likely an incorrect or invalid review comment.
pkg/waveai/usechat-anthropic.go (1)
234-238: Add proper cancellation of timeout context.The timeout context should be cancelled on all return paths to ensure resources are properly cleaned up, not just deferred.
// Context with timeout if provided. if opts.TimeoutMs > 0 { var cancel context.CancelFunc ctx, cancel = context.WithTimeout(ctx, time.Duration(opts.TimeoutMs)*time.Millisecond) defer cancel() }Likely an incorrect or invalid review comment.
| func (h *SSEHandlerCh) AiMsgFinish(finishReason string, usage interface{}) error { | ||
| resp := map[string]interface{}{ | ||
| "type": AiMsgFinish, | ||
| } | ||
| return h.WriteJsonData(resp) | ||
| } |
There was a problem hiding this comment.
Missing finishReason and usage parameters in AiMsgFinish.
The function accepts finishReason and usage parameters but doesn't include them in the response map. This could lead to missing information in the SSE stream.
func (h *SSEHandlerCh) AiMsgFinish(finishReason string, usage interface{}) error {
resp := map[string]interface{}{
"type": AiMsgFinish,
}
+ if finishReason != "" {
+ resp["finishReason"] = finishReason
+ }
+ if usage != nil {
+ resp["usage"] = usage
+ }
return h.WriteJsonData(resp)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func (h *SSEHandlerCh) AiMsgFinish(finishReason string, usage interface{}) error { | |
| resp := map[string]interface{}{ | |
| "type": AiMsgFinish, | |
| } | |
| return h.WriteJsonData(resp) | |
| } | |
| func (h *SSEHandlerCh) AiMsgFinish(finishReason string, usage interface{}) error { | |
| resp := map[string]interface{}{ | |
| "type": AiMsgFinish, | |
| } | |
| if finishReason != "" { | |
| resp["finishReason"] = finishReason | |
| } | |
| if usage != nil { | |
| resp["usage"] = usage | |
| } | |
| return h.WriteJsonData(resp) | |
| } |
🤖 Prompt for AI Agents
In pkg/waveai/ssehandlerch.go around lines 366 to 371, the AiMsgFinish function
currently ignores its finishReason and usage parameters; update the response map
to include these values (e.g., add keys "finish_reason": finishReason and
"usage": usage or use the existing key naming convention in the file), so the
SSE payload contains both fields before calling WriteJsonData; ensure nil usage
is handled (can be included as-is) and no other call sites need changing.
| fmt.Printf("DEBUG: Received event type: %s\n", event.Type) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove or conditionally compile debug output
Debug print statements should be removed or wrapped in a debug mode check to avoid cluttering production logs.
- fmt.Printf("DEBUG: Received event type: %s\n", event.Type)
+ if wavebase.IsDevMode() {
+ fmt.Printf("DEBUG: Received event type: %s\n", event.Type)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fmt.Printf("DEBUG: Received event type: %s\n", event.Type) | |
| if wavebase.IsDevMode() { | |
| fmt.Printf("DEBUG: Received event type: %s\n", event.Type) | |
| } |
🤖 Prompt for AI Agents
In pkg/waveai/usechat-openai-responses.go around lines 108-109, the
unconditional fmt.Printf debug statement should be removed or guarded; either
delete the print or replace it with a proper logger debug call (e.g.,
logger.Debugf("Received event type: %s", event.Type)) or wrap it in a debug-mode
check/read-from-ENV flag (if os.Getenv("DEBUG") == "1") so it only executes in
debug builds/environments; ensure no stray fmt.Printf remains in production
code.
| fmt.Printf("DEBUG: reasoning delta - reasoningEnded=%t, delta='%s'\n", reasoningEnded, reasoningDelta.Delta) | ||
| if reasoningDelta.Delta != "" && !reasoningEnded { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove or conditionally compile debug output
Debug print statements should be removed or controlled by debug mode flag.
- fmt.Printf("DEBUG: reasoning delta - reasoningEnded=%t, delta='%s'\n", reasoningEnded, reasoningDelta.Delta)
+ if wavebase.IsDevMode() {
+ fmt.Printf("DEBUG: reasoning delta - reasoningEnded=%t, delta='%s'\n", reasoningEnded, reasoningDelta.Delta)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fmt.Printf("DEBUG: reasoning delta - reasoningEnded=%t, delta='%s'\n", reasoningEnded, reasoningDelta.Delta) | |
| if reasoningDelta.Delta != "" && !reasoningEnded { | |
| if wavebase.IsDevMode() { | |
| fmt.Printf("DEBUG: reasoning delta - reasoningEnded=%t, delta='%s'\n", reasoningEnded, reasoningDelta.Delta) | |
| } | |
| if reasoningDelta.Delta != "" && !reasoningEnded { |
🤖 Prompt for AI Agents
In pkg/waveai/usechat-openai-responses.go around lines 124 to 125, the
fmt.Printf debug output should not be printed unconditionally; either remove it
or guard it behind a debug flag or logger. Replace the direct fmt.Printf call
with a conditional check against an existing debug/logging flag (or add one if
missing) and use the package logger or a debug-level log method so the message
is emitted only when debug mode is enabled, or delete the statement entirely if
no debug telemetry is required.
| } | ||
|
|
||
| case "response.reasoning_summary_text.done": | ||
| fmt.Printf("DEBUG: reasoning summary text done - reasoningStarted=%t, reasoningEnded=%t (not ending here, waiting for output_item.done)\n", reasoningStarted, reasoningEnded) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove or conditionally compile debug output
Debug print statements should be removed or controlled by debug mode flag.
- fmt.Printf("DEBUG: reasoning summary text done - reasoningStarted=%t, reasoningEnded=%t (not ending here, waiting for output_item.done)\n", reasoningStarted, reasoningEnded)
+ if wavebase.IsDevMode() {
+ fmt.Printf("DEBUG: reasoning summary text done - reasoningStarted=%t, reasoningEnded=%t (not ending here, waiting for output_item.done)\n", reasoningStarted, reasoningEnded)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fmt.Printf("DEBUG: reasoning summary text done - reasoningStarted=%t, reasoningEnded=%t (not ending here, waiting for output_item.done)\n", reasoningStarted, reasoningEnded) | |
| if wavebase.IsDevMode() { | |
| fmt.Printf("DEBUG: reasoning summary text done - reasoningStarted=%t, reasoningEnded=%t (not ending here, waiting for output_item.done)\n", reasoningStarted, reasoningEnded) | |
| } |
🤖 Prompt for AI Agents
In pkg/waveai/usechat-openai-responses.go around line 130, there is a raw
fmt.Printf used for debugging; remove this unconditional stdout print or guard
it behind a debug mode check (use an existing debug/config flag or add a
package-level debug boolean) and replace with a proper debug-level logger call
(e.g., logger.Debugf) or only execute the fmt.Printf when the debug flag is
true; ensure imports and callers reflect the change so no stray debug output
remains in production builds.
| bytes := make([]byte, 16) | ||
| rand.Read(bytes) | ||
| return hex.EncodeToString(bytes) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Potential panic from uninitialized random source
The rand.Read function may not be properly initialized. Use crypto/rand which is already imported but ensure error handling.
func generateID() string {
bytes := make([]byte, 16)
- rand.Read(bytes)
+ if _, err := rand.Read(bytes); err != nil {
+ // Fallback to timestamp-based ID or handle error
+ return fmt.Sprintf("%d", time.Now().UnixNano())
+ }
return hex.EncodeToString(bytes)
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In pkg/waveai/usechat.go around lines 167–169, replace the call to
math/rand.Read with crypto/rand.Read and handle its returned error;
specifically, call crypto/rand.Read(bytes), check err, and propagate the error
to the caller (preferably change the function signature to return (string,
error)) or, if changing the signature is impractical, log the error and return a
safe zero value instead of ignoring it.
| if aiOpts.APIType != APIType_OpenAI && aiOpts.APIType != APIType_Anthropic && aiOpts.APIType != "" { | ||
| http.Error(w, fmt.Sprintf("Unsupported API type: %s (only OpenAI and Anthropic supported)", aiOpts.APIType), http.StatusBadRequest) | ||
| return | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Inconsistent API type validation
The validation allows empty APIType but then checks for specific values. This could lead to unexpected behavior.
- // Support OpenAI and Anthropic
- if aiOpts.APIType != APIType_OpenAI && aiOpts.APIType != APIType_Anthropic && aiOpts.APIType != "" {
- http.Error(w, fmt.Sprintf("Unsupported API type: %s (only OpenAI and Anthropic supported)", aiOpts.APIType), http.StatusBadRequest)
+ // Validate API type - default was already set to OpenAI if empty
+ if aiOpts.APIType != APIType_OpenAI && aiOpts.APIType != APIType_Anthropic {
+ http.Error(w, fmt.Sprintf("Unsupported API type: %s (only 'openai' and 'anthropic' supported)", aiOpts.APIType), http.StatusBadRequest)
return
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if aiOpts.APIType != APIType_OpenAI && aiOpts.APIType != APIType_Anthropic && aiOpts.APIType != "" { | |
| http.Error(w, fmt.Sprintf("Unsupported API type: %s (only OpenAI and Anthropic supported)", aiOpts.APIType), http.StatusBadRequest) | |
| return | |
| } | |
| // Validate API type - default was already set to OpenAI if empty | |
| if aiOpts.APIType != APIType_OpenAI && aiOpts.APIType != APIType_Anthropic { | |
| http.Error(w, fmt.Sprintf("Unsupported API type: %s (only 'openai' and 'anthropic' supported)", aiOpts.APIType), http.StatusBadRequest) | |
| return | |
| } |
🤖 Prompt for AI Agents
In pkg/waveai/usechat.go around lines 210 to 213, the APIType check currently
allows an empty string which is inconsistent; update validation to explicitly
reject empty APIType (or alternatively set a clear default) and return a Bad
Request when aiOpts.APIType == ""; then simplify the conditional to only allow
APIType == APIType_OpenAI or APIType_Anthropic and return an error listing the
allowed values when validation fails.
…easons, and streaming errors
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (7)
pkg/waveai/usechat.go (3)
166-170: Handle crypto/rand.Read error in generateID.Avoid ignoring entropy errors; add a safe fallback.
func generateID() string { bytes := make([]byte, 16) - rand.Read(bytes) + if _, err := rand.Read(bytes); err != nil { + // fallback: time-based id (low collision risk for our usage) + return fmt.Sprintf("%x", time.Now().UnixNano()) + } return hex.EncodeToString(bytes) }Also add:
-import ( +import ( + "time"
209-213: Tighten API type validation (empty should not pass).-// Support OpenAI and Anthropic -if aiOpts.APIType != APIType_OpenAI && aiOpts.APIType != APIType_Anthropic && aiOpts.APIType != "" { - http.Error(w, fmt.Sprintf("Unsupported API type: %s (only OpenAI and Anthropic supported)", aiOpts.APIType), http.StatusBadRequest) +// Support OpenAI and Anthropic only +if aiOpts.APIType != APIType_OpenAI && aiOpts.APIType != APIType_Anthropic { + http.Error(w, fmt.Sprintf("Unsupported API type: %q (only 'openai' and 'anthropic' supported)", aiOpts.APIType), http.StatusBadRequest) return }
55-61: Preserve ErrNotFound end-to-end; return 404 (not 500).You now wrap with %w (good), but HandleAIChat treats all resolveAIConfig errors as 500. Map wstore.ErrNotFound to 404.
@@ -import ( +import ( "context" "crypto/rand" "encoding/hex" "encoding/json" "fmt" + "errors" "log" "net/http" "strings" @@ - aiOpts, err := resolveAIConfig(r.Context(), blockId, presetKey, req.Options) - if err != nil { - http.Error(w, fmt.Sprintf("Configuration error: %v", err), http.StatusInternalServerError) - return - } + aiOpts, err := resolveAIConfig(r.Context(), blockId, presetKey, req.Options) + if err != nil { + if errors.Is(err, wstore.ErrNotFound) { + http.Error(w, "block not found", http.StatusNotFound) + return + } + http.Error(w, fmt.Sprintf("Configuration error: %v", err), http.StatusInternalServerError) + return + }Also applies to: 195-200
pkg/waveai/usechat-anthropic.go (3)
1-10: Clean up documentation artifacts and incorrect package preamble.Remove :contentReference[...] tokens and fix package comment to match package waveai.
-// Package anthropicadapter streams Anthropic Messages API events and adapts them +// Package waveai streams Anthropic Messages API events and adapts them @@ -// protocol (start/text-start/text-delta/text-end, reasoning-*, tool-input-*, finish, finish-step) :contentReference[oaicite:0]{index=0} +// protocol (start/text-start/text-delta/text-end, reasoning-*, tool-input-*, finish, finish-step) @@ -// content_block_start/delta/stop, message_delta, message_stop, error). :contentReference[oaicite:1]{index=1} :contentReference[oaicite:2]{index=2} +// content_block_start/delta/stop, message_delta, message_stop, error). @@ -// same shape. Adapt the import/alias as needed in your codebase. :contentReference[oaicite:3]{index=3} +// same shape. Adapt the import/alias as needed in your codebase. @@ -// Derived from anthropic-messages-api.md and anthropic-streaming.md. :contentReference[oaicite:6]{index=6} :contentReference[oaicite:7]{index=7} +// Derived from anthropic-messages-api.md and anthropic-streaming.md. @@ -// input_json_delta (concat, then parse once on content_block_stop). :contentReference[oaicite:8]{index=8} +// input_json_delta (concat, then parse once on content_block_stop). @@ -// - On Anthropic error event: AiMsgError and return StopKindError. :contentReference[oaicite:9]{index=9} :contentReference[oaicite:10]{index=10} +// - On Anthropic error event: AiMsgError and return StopKindError. @@ -endpoint := strings.TrimRight(baseURL, "/") + "/v1/messages" // :contentReference[oaicite:11]{index=11} +endpoint := strings.TrimRight(baseURL, "/") + "/v1/messages" @@ -// Request streaming SSE. :contentReference[oaicite:14]{index=14} +// Request streaming SSE. @@ -// Try to decode Anthropic error JSON schema then surface it. :contentReference[oaicite:15]{index=15} +// Try to decode Anthropic error JSON schema then surface it. @@ -// Event model: anthropic-streaming.md. :contentReference[oaicite:16]{index=16} +// Event model: anthropic-streaming.md. @@ -// Example: data: {"type":"error","error":{"type":"overloaded_error","message":"Overloaded"}} :contentReference[oaicite:17]{index=17} +// Example: data: {"type":"error","error":{"type":"overloaded_error","message":"Overloaded"}} @@ -// ignore other block types gracefully per Anthropic guidance :contentReference[oaicite:18]{index=18} +// ignore other block types gracefully per Anthropic guidance @@ -// ignore; integrity metadata for thinking blocks. :contentReference[oaicite:19]{index=19} +// ignore; integrity metadata for thinking blocks. @@ -// ignore unknown deltas gracefully. :contentReference[oaicite:20]{index=20} +// ignore unknown deltas gracefully. @@ -// Unknown event names may appear over time; ignore. :contentReference[oaicite:22]{index=22} +// Unknown event names may appear over time; ignore. @@ -// body with stream=true and configured model/max_tokens. :contentReference[oaicite:23]{index=23} +// body with stream=true and configured model/max_tokens. @@ -// Content may be a string or array of blocks; support text only. :contentReference[oaicite:24]{index=24} +// Content may be a string or array of blocks; support text only. @@ -// Shorthand: string becomes a single text block. :contentReference[oaicite:25]{index=25} +// Shorthand: string becomes a single text block.Also applies to: 69-70, 151-153, 189-196, 217-225, 260-261, 284-285, 388-389, 404-405, 468-469, 500-503, 596-599, 602-604, 618-619, 632-633
213-225: Validate and normalize BaseURL before building endpoint.- baseURL := opts.BaseURL + baseURL := opts.BaseURL if baseURL == "" { baseURL = "https://api.anthropic.com" } - endpoint := strings.TrimRight(baseURL, "/") + "/v1/messages" + u, uerr := url.Parse(baseURL) + if uerr != nil || u.Scheme == "" || u.Host == "" { + return nil, fmt.Errorf("invalid base URL: %q", baseURL) + } + endpoint := strings.TrimRight(u.String(), "/") + "/v1/messages"
550-556: Bug: message_stop reads transient stopFromDelta instead of persisted finalStop.This can drop the stop_reason; use the function param finalStop.
- reason := "end_turn" - if stopFromDelta != nil && *stopFromDelta != "" { - reason = *stopFromDelta - } + reason := "end_turn" + if finalStop != "" { + reason = finalStop + }pkg/waveai/ssehandlerch.go (1)
365-370: Include finishReason and usage in AiMsgFinish payload.func (h *SSEHandlerCh) AiMsgFinish(finishReason string, usage interface{}) error { resp := map[string]interface{}{ "type": AiMsgFinish, } + if finishReason != "" { + resp["finishReason"] = finishReason + } + if usage != nil { + resp["usage"] = usage + } return h.WriteJsonData(resp) }
🧹 Nitpick comments (5)
pkg/waveai/usechat.go (3)
172-177: CORS preflight likely incomplete; set allow headers/methods.if r.Method == http.MethodOptions { - w.WriteHeader(http.StatusOK) + w.Header().Set("Access-Control-Allow-Origin", "*") + w.Header().Set("Access-Control-Allow-Methods", "POST, OPTIONS") + w.Header().Set("Access-Control-Allow-Headers", "Content-Type, Authorization") + w.WriteHeader(http.StatusNoContent) return }If upstream middleware already sets CORS, ignore this; otherwise this fixes OPTIONS failing in browsers.
188-194: Limit request body size and close it.Defend against large bodies and ensure r.Body is closed.
-// Parse request body completely before sending any response -var req UseChatRequest -if err := json.NewDecoder(r.Body).Decode(&req); err != nil { +// Parse request body completely before sending any response +r.Body = http.MaxBytesReader(w, r.Body, 1<<20) // 1 MiB cap +defer r.Body.Close() +var req UseChatRequest +if err := json.NewDecoder(r.Body).Decode(&req); err != nil { http.Error(w, fmt.Sprintf("Invalid request body: %v", err), http.StatusBadRequest) return }
189-193: Validate non-empty messages early.if err := json.NewDecoder(r.Body).Decode(&req); err != nil { http.Error(w, fmt.Sprintf("Invalid request body: %v", err), http.StatusBadRequest) return } +if len(req.Messages) == 0 { + http.Error(w, "At least one message is required", http.StatusBadRequest) + return +}pkg/waveai/usechat-anthropic.go (2)
371-378: Emit finish with reason captured (not empty).Sends finish with empty reason; prefer finalStop.
- _ = sse.AiMsgFinish("", nil) + _ = sse.AiMsgFinish(finalStop, nil)
643-647: Handle crypto/rand errors in genID.func genID(prefix string) string { var b [8]byte - _, _ = rand.Read(b[:]) + if _, err := rand.Read(b[:]); err != nil { + return fmt.Sprintf("%s_%x", prefix, time.Now().UnixNano()) + } return fmt.Sprintf("%s_%s", prefix, hex.EncodeToString(b[:])) }And ensure time is imported if not already.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
pkg/waveai/ssehandlerch.go(1 hunks)pkg/waveai/usechat-anthropic.go(1 hunks)pkg/waveai/usechat.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
pkg/waveai/usechat.go (7)
pkg/wconfig/settingsconfig.go (2)
AiSettingsType(35-52)MergeAiSettings(165-229)pkg/wshrpc/wshrpctypes.go (1)
WaveAIOptsType(492-503)pkg/wstore/wstore_dbops.go (1)
DBMustGet(138-149)pkg/waveai/waveai.go (2)
APIType_OpenAI(21-21)APIType_Anthropic(18-18)pkg/waveai/ssehandlerch.go (2)
SSEHandlerCh(63-75)MakeSSEHandlerCh(78-86)pkg/waveai/usechat-openai-completions.go (1)
StreamOpenAIChatCompletions(41-162)pkg/waveai/usechat-openai-responses.go (1)
StreamOpenAIResponsesAPI(82-225)
pkg/waveai/usechat-anthropic.go (3)
pkg/waveai/ssehandlerch.go (14)
SSEHandlerCh(63-75)AiMsgError(52-52)AiMsgFinish(51-51)AiMsgStart(38-38)AiMsgTextStart(39-39)AiMsgReasoningStart(42-42)AiMsgToolInputStart(45-45)AiMsgTextDelta(40-40)AiMsgReasoningDelta(43-43)AiMsgToolInputDelta(46-46)AiMsgTextEnd(41-41)AiMsgReasoningEnd(44-44)AiMsgToolInputAvailable(47-47)AiMsgFinishStep(50-50)pkg/wshrpc/wshrpctypes.go (1)
WaveAIOptsType(492-503)pkg/waveai/usechat.go (1)
UseChatMessage(27-31)
🔇 Additional comments (2)
pkg/waveai/usechat.go (1)
146-153: Do not remove the o1- branch — o1 models use Chat Completions (not Responses API)*OpenAI's model-availability docs list the o1 family (o1, o1-preview, o1-pro, o1-mini) on the Chat Completions / Assistants / Batch endpoints, so keep the strings.HasPrefix(m, "o1-") branch in shouldUseChatCompletionsAPI.
Likely an incorrect or invalid review comment.
pkg/waveai/ssehandlerch.go (1)
102-108: LGTM: Cache-Control consolidated.The duplicate Cache-Control header issue is resolved by a single combined directive.
…to match anthropic, work on cleaning up anthropic code...
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
cmd/testai/main-testai.go (1)
100-139: Use the --tools flag in OpenAI path and pass tools to StreamOpenAIToUseChatThe
withToolsflag is ignored for OpenAI; tools are never constructed or passed, so tool flows aren’t exercised. Mirror the Anthropic path by creating tool definitions whenwithToolsis true and pass them into the call.func testOpenAI(model, message string, withTools bool) { @@ messages := []waveai.UseChatMessage{ { Role: "user", Content: message, }, } + var tools []waveai.ToolDefinition + if withTools { + tools = getToolDefinitions() + fmt.Printf("Tools enabled: %d tools configured\n", len(tools)) + } + fmt.Printf("Testing OpenAI streaming with model: %s\n", model) fmt.Printf("Message: %s\n", message) fmt.Println("---") @@ - _, err = waveai.StreamOpenAIToUseChat(ctx, sseHandler, opts, messages, nil) + stopReason, err := waveai.StreamOpenAIToUseChat(ctx, sseHandler, opts, messages, tools) if err != nil { fmt.Printf("Error streaming OpenAI: %v\n", err) } + if stopReason != nil { + fmt.Printf("Stop reason: %+v\n", stopReason) + }pkg/waveai/usechat.go (2)
240-244: Tighten API type validation (don’t special-case empty string here)You already default APIType to OpenAI in resolveAIConfig. Simplify validation to only accept known values.
- // Support OpenAI and Anthropic - if aiOpts.APIType != APIType_OpenAI && aiOpts.APIType != APIType_Anthropic && aiOpts.APIType != "" { - http.Error(w, fmt.Sprintf("Unsupported API type: %s (only OpenAI and Anthropic supported)", aiOpts.APIType), http.StatusBadRequest) + // Support OpenAI and Anthropic + if aiOpts.APIType != APIType_OpenAI && aiOpts.APIType != APIType_Anthropic { + http.Error(w, fmt.Sprintf("Unsupported API type: %s (only 'openai' and 'anthropic' supported)", aiOpts.APIType), http.StatusBadRequest) return }
197-201: Handle crypto/rand.Read error in generateID (fallback or propagate)
rand.Readcan fail; ignoring the error risks returning an all-zero ID. Add error handling and a safe fallback.func generateID() string { bytes := make([]byte, 16) - rand.Read(bytes) + if _, err := rand.Read(bytes); err != nil { + // Fallback to time-based ID if CSPRNG fails + return fmt.Sprintf("%x", time.Now().UnixNano()) + } return hex.EncodeToString(bytes) }Also add missing imports:
import ( "context" "crypto/rand" "encoding/hex" "encoding/json" "fmt" + "errors" "log" "net/http" "strings" + "time"
🧹 Nitpick comments (2)
pkg/waveai/usechat.go (2)
107-124: Avoid shadowing presetKey and use a clearer name for the preset map keyThe inner
var presetKey stringshadows the function parameterpresetKey, which is confusing and error-prone. Rename the inner variable to something distinct.// Load preset configuration var presetAiSettings *wconfig.AiSettingsType if finalPreset != "default" { - var presetKey string + var presetMapKey string if strings.HasPrefix(finalPreset, "ai@") { - presetKey = finalPreset + presetMapKey = finalPreset } else { - presetKey = fmt.Sprintf("ai@%s", finalPreset) + presetMapKey = fmt.Sprintf("ai@%s", finalPreset) } - if preset, ok := fullConfig.Presets[presetKey]; ok { + if preset, ok := fullConfig.Presets[presetMapKey]; ok { presetAiSettings = &wconfig.AiSettingsType{} if err := json.Unmarshal(mustMarshal(preset), presetAiSettings); err == nil { // Successfully unmarshaled preset } else { presetAiSettings = nil } } }
203-209: Preflight response lacks CORS headersIf CORS isn’t injected by an upstream middleware, OPTIONS should include Access-Control-Allow-* headers. Verify upstream handling; if absent, add them here.
Would you like me to add a minimal header set here guarded by a dev flag?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (5)
cmd/testai/main-testai.go(1 hunks)go.mod(2 hunks)pkg/waveai/usechat-anthropic.go(1 hunks)pkg/waveai/usechat-openai-responses.go(1 hunks)pkg/waveai/usechat.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- pkg/waveai/usechat-openai-responses.go
- pkg/waveai/usechat-anthropic.go
- go.mod
🧰 Additional context used
🧬 Code graph analysis (2)
cmd/testai/main-testai.go (4)
pkg/waveai/usechat.go (3)
ToolDefinition(54-58)UseChatMessage(27-31)StreamOpenAIToUseChat(182-195)pkg/wshrpc/wshrpctypes.go (1)
WaveAIOptsType(492-503)pkg/waveai/ssehandlerch.go (1)
MakeSSEHandlerCh(78-86)pkg/waveai/usechat-anthropic.go (1)
StreamAnthropicResponses(207-309)
pkg/waveai/usechat.go (8)
pkg/waveai/usechat-anthropic.go (3)
StopReasonKind(30-30)StopKindDone(33-33)StreamAnthropicResponses(207-309)pkg/wconfig/settingsconfig.go (2)
AiSettingsType(35-52)MergeAiSettings(165-229)pkg/wshrpc/wshrpctypes.go (1)
WaveAIOptsType(492-503)pkg/wstore/wstore_dbops.go (1)
DBMustGet(138-149)pkg/waveai/waveai.go (2)
APIType_OpenAI(21-21)APIType_Anthropic(18-18)pkg/waveai/ssehandlerch.go (2)
SSEHandlerCh(63-75)MakeSSEHandlerCh(78-86)pkg/waveai/usechat-openai-completions.go (1)
StreamOpenAIChatCompletions(41-162)pkg/waveai/usechat-openai-responses.go (1)
StreamOpenAIResponsesAPI(92-235)
| // Resolve AI configuration | ||
| aiOpts, err := resolveAIConfig(r.Context(), blockId, presetKey, req.Options) | ||
| if err != nil { | ||
| http.Error(w, fmt.Sprintf("Configuration error: %v", err), http.StatusInternalServerError) | ||
| return | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Return 404 when block is not found (preserve sentinel with errors.Is)
All configuration errors are mapped to 500. Distinguish wstore.ErrNotFound and return a proper 404.
- aiOpts, err := resolveAIConfig(r.Context(), blockId, presetKey, req.Options)
- if err != nil {
- http.Error(w, fmt.Sprintf("Configuration error: %v", err), http.StatusInternalServerError)
- return
- }
+ aiOpts, err := resolveAIConfig(r.Context(), blockId, presetKey, req.Options)
+ if err != nil {
+ if errors.Is(err, wstore.ErrNotFound) {
+ http.Error(w, "Block not found", http.StatusNotFound)
+ } else {
+ http.Error(w, fmt.Sprintf("Configuration error: %v", err), http.StatusInternalServerError)
+ }
+ return
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Resolve AI configuration | |
| aiOpts, err := resolveAIConfig(r.Context(), blockId, presetKey, req.Options) | |
| if err != nil { | |
| http.Error(w, fmt.Sprintf("Configuration error: %v", err), http.StatusInternalServerError) | |
| return | |
| } | |
| // Resolve AI configuration | |
| aiOpts, err := resolveAIConfig(r.Context(), blockId, presetKey, req.Options) | |
| if err != nil { | |
| if errors.Is(err, wstore.ErrNotFound) { | |
| http.Error(w, "Block not found", http.StatusNotFound) | |
| } else { | |
| http.Error(w, fmt.Sprintf("Configuration error: %v", err), http.StatusInternalServerError) | |
| } | |
| return | |
| } |
🤖 Prompt for AI Agents
In pkg/waveai/usechat.go around lines 226 to 231, the handler currently maps all
resolveAIConfig errors to 500; change it to detect a not-found sentinel by using
errors.Is(err, wstore.ErrNotFound) and return http.StatusNotFound in that case,
otherwise keep returning http.StatusInternalServerError; ensure you preserve
sentinel semantics by using errors.Is (and add an import for the standard
"errors" package if missing).
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (6)
cmd/testai/main-testai.go (1)
100-139: Hook up --tools for OpenAI path (parity with Anthropic).
withToolsis ignored here, so OpenAI streaming never exercises tool support. Pass tools when requested and surface the stop reason (matches Anthropic test).Apply:
func testOpenAI(model, message string, withTools bool) { @@ messages := []waveai.UseChatMessage{ { Role: "user", Content: message, }, } + var tools []waveai.ToolDefinition + if withTools { + tools = getToolDefinitions() + fmt.Printf("Tools enabled: %d tools configured\n", len(tools)) + } + fmt.Printf("Testing OpenAI streaming with model: %s\n", model) fmt.Printf("Message: %s\n", message) fmt.Println("---") @@ defer sseHandler.Close() - _, err = waveai.StreamOpenAIToUseChat(ctx, sseHandler, opts, messages, nil) + stop, err := waveai.StreamOpenAIToUseChat(ctx, sseHandler, opts, messages, tools) if err != nil { fmt.Printf("Error streaming OpenAI: %v\n", err) } + if stop != nil { + fmt.Printf("Stop reason: %+v\n", stop) + } }pkg/waveai/usechat.go (2)
197-201: Handle crypto/rand.Read errors in ID generation.Avoid ignoring failures; provide a safe fallback.
Apply:
func generateID() string { bytes := make([]byte, 16) - rand.Read(bytes) - return hex.EncodeToString(bytes) + if _, err := rand.Read(bytes); err != nil { + // extremely unlikely; fall back to timestamp-based ID + return fmt.Sprintf("id_%x", time.Now().UnixNano()) + } + return hex.EncodeToString(bytes) }And add import:
- "log" + "log" + "time"
240-244: Tighten API type validation (empty string shouldn’t slip through).Defaulting to OpenAI earlier means this check can be simplified and made stricter.
Apply:
- // Support OpenAI and Anthropic - if aiOpts.APIType != APIType_OpenAI && aiOpts.APIType != APIType_Anthropic && aiOpts.APIType != "" { - http.Error(w, fmt.Sprintf("Unsupported API type: %s (only OpenAI and Anthropic supported)", aiOpts.APIType), http.StatusBadRequest) + // Support exactly 'openai' or 'anthropic' + if aiOpts.APIType != APIType_OpenAI && aiOpts.APIType != APIType_Anthropic { + http.Error(w, fmt.Sprintf("Unsupported API type: %s (only 'openai' and 'anthropic' supported)", aiOpts.APIType), http.StatusBadRequest) return }pkg/waveai/usechat-anthropic.go (2)
1-10: Clean up package comment and doc artifacts.Top comment names wrong package and several lines contain generated :contentReference markers. Remove artifacts and fix the package comment.
Apply:
-// Package anthropicadapter streams Anthropic Messages API events and adapts them +// Package waveai streams Anthropic Messages API events and adapts them // to our AI‑SDK style SSE parts. Mapping is based on the AI‑SDK data stream -// protocol (start/text-start/text-delta/text-end, reasoning-*, tool-input-*, finish, finish-step) :contentReference[oaicite:0]{index=0} +// protocol (start/text-start/text-delta/text-end, reasoning-*, tool-input-*, finish, finish-step) // and Anthropic's Messages + Streaming event schemas (message_start, -// content_block_start/delta/stop, message_delta, message_stop, error). :contentReference[oaicite:1]{index=1} :contentReference[oaicite:2]{index=2} +// content_block_start/delta/stop, message_delta, message_stop, error). @@ -// Derived from anthropic-messages-api.md and anthropic-streaming.md. :contentReference[oaicite:6]{index=6} :contentReference[oaicite:7]{index=7} +// Derived from anthropic-messages-api.md and anthropic-streaming.md. @@ -// input_json_delta (concat, then parse once on content_block_stop). :contentReference[oaicite:8]{index=8} +// input_json_delta (concat, then parse once on content_block_stop). @@ -// - On Anthropic error event: AiMsgError and return StopKindError. :contentReference[oaicite:9]{index=10} +// - On Anthropic error event: AiMsgError and return StopKindError. @@ - // Example: data: {"type":"error","error":{"type":"overloaded_error","message":"Overloaded"}} :contentReference[oaicite:17]{index=17} + // Example: data: {"type":"error","error":{"type":"overloaded_error","message":"Overloaded"}} @@ - // ignore other block types gracefully per Anthropic guidance :contentReference[oaicite:18]{index=18} + // ignore other block types gracefully per Anthropic guidance @@ - // ignore; integrity metadata for thinking blocks. :contentReference[oaicite:19]{index=19} + // ignore; integrity metadata for thinking blocks. @@ - // Unknown event names may appear over time; ignore. :contentReference[oaicite:22]{index=22} + // Unknown event names may appear over time; ignore. @@ - // Content may be a string or array of blocks; support text only. :contentReference[oaicite:24]{index=24} + // Content may be a string or array of blocks; support text only. @@ - // Shorthand: string becomes a single text block. :contentReference[oaicite:25]{index=25} + // Shorthand: string becomes a single text block.Also applies to: 47-49, 130-131, 166-176, 334-336, 398-400, 430-434, 528-529, 572-575, 588-593
545-551: Validate and normalize base URL.Avoid concatenating unvalidated base URLs; parse and ensure scheme/host.
Apply:
- baseURL := opts.BaseURL - if baseURL == "" { - baseURL = AnthropicDefaultBaseURL - } - endpoint := strings.TrimRight(baseURL, "/") + "/v1/messages" + baseURL := opts.BaseURL + if baseURL == "" { + baseURL = AnthropicDefaultBaseURL + } + u, err := url.Parse(baseURL) + if err != nil || u.Scheme == "" || u.Host == "" { + return nil, fmt.Errorf("invalid base URL: %q", baseURL) + } + endpoint := strings.TrimRight(u.String(), "/") + "/v1/messages"pkg/waveai/usechat-openai-responses.go (1)
118-118: Gate debug prints behind dev mode.Avoid unconditional stdout logging in production.
Apply:
- fmt.Printf("DEBUG: Received event type: %s\n", event.Type) + if wavebase.IsDevMode() { + fmt.Printf("DEBUG: Received event type: %s\n", event.Type) + } @@ - fmt.Printf("DEBUG: reasoning delta - reasoningEnded=%t, delta='%s'\n", reasoningEnded, reasoningDelta.Delta) + if wavebase.IsDevMode() { + fmt.Printf("DEBUG: reasoning delta - reasoningEnded=%t, delta='%s'\n", reasoningEnded, reasoningDelta.Delta) + } @@ - fmt.Printf("DEBUG: reasoning summary text done - reasoningStarted=%t, reasoningEnded=%t (not ending here, waiting for output_item.done)\n", reasoningStarted, reasoningEnded) + if wavebase.IsDevMode() { + fmt.Printf("DEBUG: reasoning summary text done - reasoningStarted=%t, reasoningEnded=%t (not ending here, waiting for output_item.done)\n", reasoningStarted, reasoningEnded) + } @@ - // Log unhandled event types in dev mode - if wavebase.IsDevMode() { + // Log unhandled event types in dev mode + if wavebase.IsDevMode() { fmt.Printf("DEBUG: Unhandled event type: %s\n", event.Type) }Also applies to: 134-134, 140-140, 207-211
🧹 Nitpick comments (6)
pkg/waveai/usechat.go (2)
77-81: Allow tool definitions in the HTTP request (optional).If you intend to support ToolUse via HTTP, add
Toolsto the request DTO and pass through to backends.Apply:
type UseChatRequest struct { Messages []UseChatMessage `json:"messages"` Options *wconfig.AiSettingsType `json:"options,omitempty"` + Tools []ToolDefinition `json:"tools,omitempty"` }And:
- if aiOpts.APIType == APIType_Anthropic { - _, err := StreamAnthropicResponses(r.Context(), sseHandler, aiOpts, req.Messages, nil) + if aiOpts.APIType == APIType_Anthropic { + _, err := StreamAnthropicResponses(r.Context(), sseHandler, aiOpts, req.Messages, req.Tools) if err != nil { log.Printf("Anthropic streaming error: %v", err) } } else { // Default to OpenAI - _, err := StreamOpenAIToUseChat(r.Context(), sseHandler, aiOpts, req.Messages, nil) + _, err := StreamOpenAIToUseChat(r.Context(), sseHandler, aiOpts, req.Messages, req.Tools) if err != nil { log.Printf("OpenAI streaming error: %v", err) } }Also applies to: 260-272
182-195: Consider returning the real StopReason from OpenAI paths.Wrapper currently always returns Kind=done, which hides tool_use/max_tokens endings on Responses API. Unify return semantics with Anthropic for observability.
pkg/waveai/usechat-anthropic.go (1)
615-619: Handle crypto/rand failure when generating IDs.Add a minimal fallback; time is already imported.
Apply:
func genID(prefix string) string { var b [8]byte - _, _ = rand.Read(b[:]) + if _, err := rand.Read(b[:]); err != nil { + return fmt.Sprintf("%s_%d", prefix, time.Now().UnixNano()) + } return fmt.Sprintf("%s_%s", prefix, hex.EncodeToString(b[:])) }pkg/waveai/usechat-openai-responses.go (3)
92-98: Honor TimeoutMs like the Anthropic path.Wrap ctx with a timeout when provided for parity and to prevent hanging streams.
Apply:
func StreamOpenAIResponsesAPI(sseHandler *SSEHandlerCh, ctx context.Context, opts *wshrpc.WaveAIOptsType, messages []UseChatMessage, tools []ToolDefinition) { + if opts.TimeoutMs > 0 { + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, time.Duration(opts.TimeoutMs)*time.Millisecond) + defer cancel() + } client, req := createOpenAIRequest(opts, messages, tools)And add import:
import ( "context" "fmt" + "time" "strings"
19-33: Validate non-empty message input or document behavior.If all messages are filtered out as empty, the request has an empty Input list. Consider validating earlier and returning an explicit error to callers, or inject a minimal system/user placeholder.
Also applies to: 59-90
19-33: Proxy support parity with Anthropic (optional).If
opts.ProxyURLis set, configure the OpenAI client with a custom HTTP client using that proxy.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (5)
cmd/testai/main-testai.go(1 hunks)go.mod(2 hunks)pkg/waveai/usechat-anthropic.go(1 hunks)pkg/waveai/usechat-openai-responses.go(1 hunks)pkg/waveai/usechat.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- go.mod
🧰 Additional context used
🧬 Code graph analysis (4)
cmd/testai/main-testai.go (4)
pkg/waveai/usechat.go (3)
ToolDefinition(54-58)UseChatMessage(27-31)StreamOpenAIToUseChat(182-195)pkg/wshrpc/wshrpctypes.go (1)
WaveAIOptsType(492-503)pkg/waveai/ssehandlerch.go (1)
MakeSSEHandlerCh(78-86)pkg/waveai/usechat-anthropic.go (1)
StreamAnthropicResponses(207-309)
pkg/waveai/usechat.go (8)
pkg/waveai/usechat-anthropic.go (3)
StopReasonKind(30-30)StopKindDone(33-33)StreamAnthropicResponses(207-309)pkg/wconfig/settingsconfig.go (2)
AiSettingsType(35-52)MergeAiSettings(165-229)pkg/wshrpc/wshrpctypes.go (1)
WaveAIOptsType(492-503)pkg/wstore/wstore_dbops.go (1)
DBMustGet(138-149)pkg/waveai/waveai.go (2)
APIType_OpenAI(21-21)APIType_Anthropic(18-18)pkg/waveai/ssehandlerch.go (2)
SSEHandlerCh(63-75)MakeSSEHandlerCh(78-86)pkg/waveai/usechat-openai-completions.go (1)
StreamOpenAIChatCompletions(41-162)pkg/waveai/usechat-openai-responses.go (1)
StreamOpenAIResponsesAPI(92-235)
pkg/waveai/usechat-anthropic.go (3)
pkg/waveai/usechat.go (4)
ToolDefinition(54-58)StopReason(39-51)UseChatMessage(27-31)ToolCall(33-37)pkg/waveai/ssehandlerch.go (14)
SSEHandlerCh(63-75)AiMsgError(52-52)AiMsgFinish(51-51)AiMsgStart(38-38)AiMsgTextStart(39-39)AiMsgReasoningStart(42-42)AiMsgToolInputStart(45-45)AiMsgTextDelta(40-40)AiMsgReasoningDelta(43-43)AiMsgToolInputDelta(46-46)AiMsgTextEnd(41-41)AiMsgReasoningEnd(44-44)AiMsgToolInputAvailable(47-47)AiMsgFinishStep(50-50)pkg/wshrpc/wshrpctypes.go (1)
WaveAIOptsType(492-503)
pkg/waveai/usechat-openai-responses.go (5)
pkg/wshrpc/wshrpctypes.go (1)
WaveAIOptsType(492-503)pkg/waveai/usechat.go (2)
UseChatMessage(27-31)ToolDefinition(54-58)pkg/waveai/ssehandlerch.go (9)
SSEHandlerCh(63-75)AiMsgStart(38-38)AiMsgReasoningStart(42-42)AiMsgReasoningDelta(43-43)AiMsgTextStart(39-39)AiMsgTextDelta(40-40)AiMsgTextEnd(41-41)AiMsgReasoningEnd(44-44)AiMsgFinish(51-51)pkg/waveai/usechat-openai-completions.go (1)
OpenAIUsageResponse(35-39)pkg/wavebase/wavebase.go (1)
IsDevMode(102-104)
🔇 Additional comments (1)
pkg/waveai/usechat-openai-responses.go (1)
77-83: isReasoningModel present — no changes needed.
Defined at pkg/waveai/openaibackend.go:32; referenced from usechat-openai-responses.go and usechat-openai-completions.go.
| // Resolve AI configuration | ||
| aiOpts, err := resolveAIConfig(r.Context(), blockId, presetKey, req.Options) | ||
| if err != nil { | ||
| http.Error(w, fmt.Sprintf("Configuration error: %v", err), http.StatusInternalServerError) | ||
| return | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Return 404 when block is not found (preserve sentinel).
resolveAIConfig wraps with %w, so wstore.ErrNotFound survives. Map it to 404 instead of 500.
Apply:
- aiOpts, err := resolveAIConfig(r.Context(), blockId, presetKey, req.Options)
+ aiOpts, err := resolveAIConfig(r.Context(), blockId, presetKey, req.Options)
if err != nil {
- http.Error(w, fmt.Sprintf("Configuration error: %v", err), http.StatusInternalServerError)
+ if errors.Is(err, wstore.ErrNotFound) {
+ http.Error(w, "block not found", http.StatusNotFound)
+ } else {
+ http.Error(w, fmt.Sprintf("Configuration error: %v", err), http.StatusInternalServerError)
+ }
return
}And add the import:
import (
"context"
"crypto/rand"
"encoding/hex"
"encoding/json"
+ "errors"
"fmt"
"log"
"net/http"
"strings"🤖 Prompt for AI Agents
In pkg/waveai/usechat.go around lines 226-231, the code currently always returns
HTTP 500 on resolveAIConfig errors; since resolveAIConfig wraps errors with %w
and preserves the sentinel wstore.ErrNotFound, change the error handling to
check errors.Is(err, wstore.ErrNotFound) and return http.StatusNotFound in that
case, otherwise return http.StatusInternalServerError as before; add the
"errors" import (and ensure wstore is imported/available) so you can use
errors.Is to preserve the sentinel comparison.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
cmd/testai/main-testai.go (1)
135-139: Resolved: OpenAI test now forwards tools to the streamer.This addresses the earlier note about unused tool definitions in the OpenAI path.
🧹 Nitpick comments (4)
cmd/testai/main-testai.go (4)
120-123: Surface tool count and StopReason for OpenAI to match Anthropic logs.Small UX boost when debugging streaming and tool usage.
Apply this diff:
@@ fmt.Printf("Testing OpenAI streaming with model: %s\n", model) fmt.Printf("Message: %s\n", message) fmt.Println("---") + if len(tools) > 0 { + fmt.Printf("Tools enabled: %d tools configured\n", len(tools)) + } @@ - _, err = waveai.StreamOpenAIToUseChat(ctx, sseHandler, opts, messages, tools) - if err != nil { - fmt.Printf("Error streaming OpenAI: %v\n", err) - } + stopReason, err := waveai.StreamOpenAIToUseChat(ctx, sseHandler, opts, messages, tools) + if err != nil { + fmt.Printf("Error streaming OpenAI: %v\n", err) + } else if stopReason != nil { + fmt.Printf("Stop reason: %+v\n", stopReason) + }Also applies to: 135-139
124-127: Avoid hanging runs: add a timeout to the OpenAI test context.Prevents indefinite waits if a stream never finishes.
Apply this diff:
- ctx := context.Background() - testWriter := &TestResponseWriter{} - sseHandler := waveai.MakeSSEHandlerCh(testWriter, ctx) + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) + defer cancel() + testWriter := &TestResponseWriter{} + sseHandler := waveai.MakeSSEHandlerCh(testWriter, ctx)
161-164: Also log tool count for Anthropic for parity.Keeps both paths consistent when troubleshooting tools.
Apply this diff:
fmt.Printf("Testing Anthropic streaming with model: %s\n", model) fmt.Printf("Message: %s\n", message) fmt.Println("---") + if len(tools) > 0 { + fmt.Printf("Tools enabled: %d tools configured\n", len(tools)) + }
165-168: Add a timeout to the Anthropic test context.Same rationale as OpenAI path.
Apply this diff:
- ctx := context.Background() - testWriter := &TestResponseWriter{} - sseHandler := waveai.MakeSSEHandlerCh(testWriter, ctx) + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) + defer cancel() + testWriter := &TestResponseWriter{} + sseHandler := waveai.MakeSSEHandlerCh(testWriter, ctx)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cmd/testai/main-testai.go(1 hunks)pkg/waveai/usechat-anthropic.go(1 hunks)pkg/waveai/usechat.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- pkg/waveai/usechat-anthropic.go
- pkg/waveai/usechat.go
🧰 Additional context used
🧬 Code graph analysis (1)
cmd/testai/main-testai.go (4)
pkg/waveai/usechat.go (3)
ToolDefinition(54-58)UseChatMessage(27-31)StreamOpenAIToUseChat(182-195)pkg/wshrpc/wshrpctypes.go (1)
WaveAIOptsType(492-503)pkg/waveai/ssehandlerch.go (1)
MakeSSEHandlerCh(78-86)pkg/waveai/usechat-anthropic.go (1)
StreamAnthropicResponses(207-309)
🔇 Additional comments (2)
cmd/testai/main-testai.go (2)
49-58: Good: implements ResponseController-required methods on the test writer.Providing SetWriteDeadline/SetReadDeadline and Flusher avoids ErrNotSupported from http.NewResponseController during SSE. Solid for a CLI harness.
60-99: Schema-driven tools look sane.Graceful fallback to a minimal object schema when "config" is absent keeps tests resilient. No issues spotted.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
cmd/testai/main-testai.go (5)
60-66: Log schema parse errors to stderr.Keeps SSE output on stdout clean and separates diagnostics.
Apply:
- fmt.Printf("Error parsing schema: %v\n", err) + fmt.Fprintf(os.Stderr, "Error parsing schema: %v\n", err)
100-112: Propagate ctx deadline into opts.TimeoutMs (optional).Some backends respect
opts.TimeoutMs. Deriving it from the passed ctx keeps behavior consistent across providers.Apply in both test functions after constructing
opts:opts := &wshrpc.WaveAIOptsType{ APIToken: apiKey, Model: model, MaxTokens: 1000, } +if dl, ok := ctx.Deadline(); ok { + opts.TimeoutMs = int(time.Until(dl).Milliseconds()) +}
120-123: Optional: warn when tools may be ignored by older OpenAI models.The OpenAI adapter routes some models via Chat Completions, which don’t use tools; a small notice helps avoid confusion.
Apply:
fmt.Printf("Testing OpenAI streaming with model: %s\n", model) fmt.Printf("Message: %s\n", message) fmt.Println("---") +if len(tools) > 0 { + fmt.Println("Note: Tools are only used with OpenAI Responses API models; Chat Completions models may ignore them.") +}
186-205: Add a configurable timeout flag (optional).Hard-coded 60s can be restrictive. Let users override and plumb it through ctx.
Apply:
func main() { - var anthropic, tools bool + var anthropic, tools bool + var timeout time.Duration flag.BoolVar(&anthropic, "anthropic", false, "Use Anthropic API instead of OpenAI") flag.BoolVar(&tools, "tools", false, "Enable GitHub Actions Monitor tools for testing") + flag.DurationVar(&timeout, "timeout", 60*time.Second, "Overall request timeout (e.g., 30s, 2m)") flag.Parse() @@ - ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), timeout) defer cancel()Consider also reflecting the chosen timeout into
opts.TimeoutMsas suggested earlier.
212-216: Optional: confirm tool count when enabled.Useful for quick sanity checks in CLI output.
Apply:
if tools { toolDefs = getToolDefinitions() + fmt.Printf("Tools enabled: %d\n", len(toolDefs)) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cmd/testai/main-testai.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
cmd/testai/main-testai.go (4)
pkg/waveai/usechat.go (3)
ToolDefinition(54-58)UseChatMessage(27-31)StreamOpenAIToUseChat(182-195)pkg/wshrpc/wshrpctypes.go (1)
WaveAIOptsType(492-503)pkg/waveai/ssehandlerch.go (1)
MakeSSEHandlerCh(78-86)pkg/waveai/usechat-anthropic.go (1)
StreamAnthropicResponses(207-309)
🔇 Additional comments (6)
cmd/testai/main-testai.go (6)
67-76: Sane fallback when config schema is absent.Good defensive default to an empty object.
77-98: Tool definitions align with ToolDefinition schema.Names/descriptions are clear;
update_configcorrectly reuses the embedded config schema.
134-141: OpenAI path now forwards tools — resolves prior gap.Passing
toolsintoStreamOpenAIToUseChataddresses the previously noted omission. Nicely done.
143-184: Anthropic streaming path looks solid.Consistent setup with OpenAI path, prints stop reason, and leverages the same SSE writer.
20-22: Embedded schema path verified — no change requiredtestschema.json is present at cmd/testai/testschema.json, so //go:embed testschema.json is correct.
23-58: TestResponseWriter is compatible with http.ResponseController — Go version confirmed (go 1.24.6).No-op Flush and SetRead/SetWriteDeadline are acceptable for a CLI test harness; approve the changes.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
cmd/testai/main-testai.go (1)
101-142: Past concern resolved: OpenAI path now wires tools through.You now pass tools into StreamOpenAIToUseChat; this addresses the previously raised gap.
🧹 Nitpick comments (6)
cmd/testai/main-testai.go (6)
36-39: Avoid allocation and ensure readable output in Write.Use %s with []byte directly and add a trailing newline so events don’t run together.
-func (w *TestResponseWriter) Write(data []byte) (int, error) { - fmt.Printf("SSE: %s", string(data)) +func (w *TestResponseWriter) Write(data []byte) (int, error) { + // %s accepts []byte; adding newline keeps output legible. + fmt.Printf("SSE: %s\n", data) return len(data), nil }
108-113: Propagate CLI timeout to provider opts for consistent cancellation.Set TimeoutMs from ctx.Deadline so provider HTTP clients can honor it (Anthropic already wraps ctx; this keeps parity and future-proofs OpenAI paths).
opts := &wshrpc.WaveAIOptsType{ APIToken: apiKey, Model: model, MaxTokens: 1000, } +if deadline, ok := ctx.Deadline(); ok { + opts.TimeoutMs = int(time.Until(deadline).Milliseconds()) +}opts := &wshrpc.WaveAIOptsType{ APIToken: apiKey, Model: model, MaxTokens: 1000, } +if deadline, ok := ctx.Deadline(); ok { + opts.TimeoutMs = int(time.Until(deadline).Milliseconds()) +}Also applies to: 151-156
121-124: Surface tool usage and warn when models may ignore tools.Given pkg/waveai/usechat.go only passes tools on the Responses API path, older Chat Completions models will ignore them; make this explicit to users.
fmt.Printf("Testing OpenAI streaming with model: %s\n", model) fmt.Printf("Message: %s\n", message) fmt.Println("---") +if len(tools) > 0 { + fmt.Printf("Tools enabled: %d definitions\n", len(tools)) + // Tools are used on the Responses API path; Chat Completions models may ignore them. + fmt.Println("Note: some OpenAI Chat Completions models may ignore tools; prefer Responses API models (gpt-4.1, gpt-4o, o4).") +}Also applies to: 135-141
164-167: Parity: log tool count for Anthropic too.Tiny UX improvement for consistency.
fmt.Printf("Testing Anthropic streaming with model: %s\n", model) fmt.Printf("Message: %s\n", message) fmt.Println("---") +if len(tools) > 0 { + fmt.Printf("Tools enabled: %d definitions\n", len(tools)) +}
187-192: Make timeout configurable via flag; default to 60s.Some streams exceed 60s. Add --timeout to control both ctx and provider opts (with the earlier change).
- var anthropic, tools bool + var anthropic, tools bool + var timeoutSec int flag.BoolVar(&anthropic, "anthropic", false, "Use Anthropic API instead of OpenAI") flag.BoolVar(&tools, "tools", false, "Enable GitHub Actions Monitor tools for testing") + flag.IntVar(&timeoutSec, "timeout", 60, "Request timeout in seconds (0 = no timeout)") flag.Parse()- ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) + var ctx context.Context + var cancel context.CancelFunc + if timeoutSec > 0 { + ctx, cancel = context.WithTimeout(context.Background(), time.Duration(timeoutSec)*time.Second) + } else { + ctx, cancel = context.WithCancel(context.Background()) + } defer cancel()Also applies to: 218-220
101-142: Reduce duplication between provider test functions.Shared setup/teardown and logging can be factored into a small helper to keep drift down.
Example shape:
- helper setupSSE(ctx) -> (*waveai.SSEHandlerCh, func())
- helper printPreamble(provider, model, message, toolsCount)
Also applies to: 144-185
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cmd/testai/main-testai.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
cmd/testai/main-testai.go (4)
pkg/waveai/usechat.go (3)
ToolDefinition(54-58)UseChatMessage(27-31)StreamOpenAIToUseChat(182-195)pkg/wshrpc/wshrpctypes.go (1)
WaveAIOptsType(492-503)pkg/waveai/ssehandlerch.go (1)
MakeSSEHandlerCh(78-86)pkg/waveai/usechat-anthropic.go (1)
StreamAnthropicResponses(207-309)
🔇 Additional comments (1)
cmd/testai/main-testai.go (1)
61-99: Tool schema parsing and fallbacks look solid.Graceful fallback when config schema is missing and clear descriptions. No changes needed.
1 participant
Working on AI SDK compatible backends for OpenAI and Anthropic. Thinking + ToolUse etc.