fix(web-search): always stream upstream requests while buffering loop iterations - #130
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #130 +/- ##
==========================================
+ Coverage 95.01% 95.09% +0.08%
==========================================
Files 26 26
Lines 4950 4950
Branches 1432 1432
==========================================
+ Hits 4703 4707 +4
+ Misses 247 243 -4
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b3cdabbc17
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // buildUpstreamBody each iteration would re-apply prompt cache | ||
| // markers to the appended tool results. | ||
| upstreamBody: loopBody, | ||
| upstreamBody: { ...loopBody, stream: true }, |
There was a problem hiding this comment.
Preserve in-band SSE errors while buffering
When CodeBuddy reports a streaming failure as an HTTP 200 SSE frame such as data: {"error": ...}, this newly forced streaming path passes it to aggregateUpstreamStream, which ignores the error field and manufactures a successful 200 completion with empty content. The web-search loop therefore returns an apparent empty answer instead of propagating the upstream failure; detect and preserve SSE error payloads during aggregation.
Useful? React with 👍 / 👎.
Problem
With local web search enabled, requests fail immediately:
{ "msg": "Non-stream chat request is currently not supported", "code": 11101 }Cause
The upstream
/v2/chat/completionsendpoint only acceptsstream: true, andbuildUpstreamBodysets exactly that. The web search loop, however, needs a complete tool call before it can run a query, so it overrode the request withstream: false— which upstream rejects with code 11101.Fix
Follow the upstream contract instead: the request always uses
stream: true, and the loop buffers the resulting SSE into a single complete message (reusing the existingaggregateUpstreamStream()) before inspecting it for tool calls. Non-streaming callers still receive one JSON payload, so behaviour is unchanged.The
streamparameter onfetchChatCompletionis now documented as controlling only the shape handed back to the caller (pass through SSE vs. buffer into JSON) — it no longer affects what is sent upstream.Verification
stream: true. Confirmed it fails without the fix.test:cicoverage: 94.31% statements, 87.93% branches, 94.88% linesNote
This is a follow-up fix to #127, which has already been squash-merged into main, so it needs its own PR.