fix(stream): preserve usage chunk before non-standard upstream frames - #6070
fix(stream): preserve usage chunk before non-standard upstream frames#6070Ankairis wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
Walkthrough
ChangesStream usage handling
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
relay/channel/openai/relay-openai.go (1)
140-149: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAdd a
strings.Containspre-filter before the per-chunk unmarshal.Every incoming chunk is unmarshaled into a full
dto.ChatCompletionsStreamResponsejust to check for ausagefield. Most chunks are content-only and won't contain usage, so this adds unnecessary overhead on the streaming hot path. A lightweight substring check before the unmarshal skips the majority of parses.♻️ Proposed optimization
// 检测当前 chunk 是否含有 usage,保存备用 // (OpenCode.ai 等上游在 usage chunk 后还会发非标准块,会覆盖 lastStreamData) - var chunkWithUsage dto.ChatCompletionsStreamResponse - if err := common.UnmarshalJsonStr(data, &chunkWithUsage); err == nil && chunkWithUsage.Usage != nil { - if service.ValidUsage(chunkWithUsage.Usage) { - usageStreamData = data - } + if strings.Contains(data, `"usage"`) { + var chunkWithUsage dto.ChatCompletionsStreamResponse + if err := common.UnmarshalJsonStr(data, &chunkWithUsage); err == nil && chunkWithUsage.Usage != nil { + if service.ValidUsage(chunkWithUsage.Usage) { + usageStreamData = data + } + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@relay/channel/openai/relay-openai.go` around lines 140 - 149, In the usage-detection block near the per-chunk unmarshal, add a strings.Contains pre-filter for the usage field before calling common.UnmarshalJsonStr. Only unmarshal into dto.ChatCompletionsStreamResponse and run service.ValidUsage when the chunk contains that substring, preserving the existing assignment to usageStreamData.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@relay/channel/openai/relay-openai.go`:
- Around line 140-149: In the usage-detection block near the per-chunk
unmarshal, add a strings.Contains pre-filter for the usage field before calling
common.UnmarshalJsonStr. Only unmarshal into dto.ChatCompletionsStreamResponse
and run service.ValidUsage when the chunk contains that substring, preserving
the existing assignment to usageStreamData.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f6d38d5f-2263-4512-a5ea-62b9f3522e67
📒 Files selected for processing (1)
relay/channel/openai/relay-openai.go
|
This issue exists in the opencode interface. This results in the cache hits recorded by new-api always being 0 |
|
是的,根因就是 OpenCode.ai 在 这个 PR 的改法是在每个 chunk 处理时独立保存最后一个含有效 目前已在我这边生产环境验证通过, |
|
Rebased onto the latest I also added a regression test ( This has been running in production on my side since the original submission — OpenCode streams now record real |
OaiStreamHandler holds every frame in lastStreamData and writes the previous one out only when the next arrives, so a client gets frame 1 at frame 2's arrival time. Against upstreams that pause after a short opening frame — a role-only delta, a block-buffered tool-call parser — that adds a whole upstream frame interval to first-token latency. HandleStreamFormat dispatches on RelayFormat, so the delay hits Claude, Gemini and OpenAI clients alike; QuantumNous#7033 lifts it only for RelayFormatOpenAI, because the Claude and Gemini conversions need the terminal frame in HandleFinalResponse to emit their closing events. That covers the OpenCode and Grok traffic on /v1/chat/completions; Claude Code on /v1/messages keeps the old path. QuantumNous#7033 and QuantumNous#6070 both rewrite the StreamScannerHandler callback. QuantumNous#6070 stashes the usage-bearing chunk in usageStreamData because upstreams like OpenCode.ai emit non-standard frames after it and overwrite lastStreamData; QuantumNous#7033 turns the callback's "if len(data) > 0" body into an early return. Downloading QuantumNous#7033 plain gives one reject there. The two are orthogonal — one is about billing reading real usage, the other about not delaying a frame — so the merge is mechanical: QuantumNous#7033's restructured callback with QuantumNous#6070's usageStreamData capture put back immediately after "lastStreamData = data". Everything else in the PR is untouched. The patch header records how to redo it. Verified: all seventeen patches apply in sequence over rc.26, gofmt clean, both modules build (relaykit with GOWORK=off), and go test ./... is green in both — including QuantumNous#7033's eight new direct-forward subcases running alongside QuantumNous#6070. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JLS2huh1TDmNyhGgKq5NDU
.dockerignore excludes *.md from the build context, so scripts/apply-patches.sh runs inside the image against a tree with no README.md and PR QuantumNous#6949's hunk for it failed the build. A local dry-run cannot catch this — the file is there — so the image build was the first place it showed up. Dropped that hunk; it only documented the new RELAY_RESPONSE_HEADER_TIMEOUT variable. .env.example is not excluded and is kept, so the variable is still documented where it matters for a deploy. The patch header records the drop and how to get it back. Added the rule to patches/README.md with the grep to run before adding any patch, and documented the QuantumNous#7033-onto-QuantumNous#6070 reconciliation in the same file. Verified: docker compose build new-api is green, all seventeen patches apply under Alpine's GNU patch with *.md absent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JLS2huh1TDmNyhGgKq5NDU
Problem
When streaming with upstreams that send non-standard chunks after the usage-bearing
finish_reasonchunk (e.g. OpenCode.ai sendsx-opencode-typeand extracostframes),OaiStreamHandleroverwriteslastStreamDataon every frame.handleLastResponsethen sees a chunk without usage,containStreamUsagestaysfalse, and the code falls back toResponseText2Usage(local token counting). This causescache_tokensto always be 0 regardless of actual upstream cache hits.Root Cause
relay/channel/openai/relay-openai.go—OaiStreamHandler:Upstream stream order:
Fix
usageStreamData— stores the last chunk that contained validUsageusageStreamDataif it has valid usageResponseText2Usage, tryusageStreamDatafirstusageStreamDatatoapplyUsagePostProcessingso the original upstream body is still used for cache-token extractionTesting
Verified with upstream curl test that OpenCode.ai returns
prompt_cache_hit_tokensandprompt_tokens_details.cached_tokenscorrectly in thefinish_reasonchunk. After this fix, those values are preserved instead of being lost to the subsequent non-standard frame.Closes #(issue if applicable)
Summary by CodeRabbit