refactor(providers): use extractReasoningFromDelta helper for reasoni…#588
Conversation
…ng streams Replace duplicated reasoning_content extraction logic across 7 providers (deepseek, mimo, openai, opencode-go, qwen-code, requesty, unbound) with the shared extractReasoningFromDelta helper. This also adds the OpenRouter- style 'reasoning' field fallback to each provider for free.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughProvider streaming handlers (DeepSeek, MiMo, OpenAI, Opencode-Go, Qwen-Code, Requesty, Unbound) now import and use ChangesConsolidate Reasoning Extraction Helper Adoption
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~28 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Add streaming tests for openai, requesty, and unbound covering: - delta.reasoning_content yields a reasoning chunk - delta.reasoning fallback yields a reasoning chunk when reasoning_content is absent (OpenRouter-style) This raises line coverage on the changed reasoning paths from 50%.
edelauna
left a comment
There was a problem hiding this comment.
Thanks for adding this, had one question related to test coverage
| it("falls back to delta.reasoning when reasoning_content is absent", async () => { | ||
| mockCreate.mockImplementationOnce(async () => ({ | ||
| [Symbol.asyncIterator]: async function* () { | ||
| yield { choices: [{ delta: { reasoning: "router-style thought" }, index: 0 }] } | ||
| yield { | ||
| choices: [{ delta: {}, index: 0 }], | ||
| usage: { prompt_tokens: 1, completion_tokens: 1, total_tokens: 2 }, | ||
| } | ||
| }, | ||
| })) | ||
|
|
||
| const chunks: any[] = [] | ||
| for await (const chunk of handler.createMessage(systemPrompt, messages)) { | ||
| chunks.push(chunk) | ||
| } | ||
|
|
||
| expect(chunks).toContainEqual({ type: "reasoning", text: "router-style thought" }) | ||
| }) |
There was a problem hiding this comment.
Does the helper priority rule get exercised here? If a delta has both reasoning_content and reasoning populated, the expected winner is reasoning_content — but that case isn't covered at the provider level. The unit tests for extractReasoningFromDelta cover it, but a provider-level check would catch a future regression where the helper call gets bypassed or re-inlined.
There was a problem hiding this comment.
Good catch. I added a provider-level priority test to cover the case where both delta.reasoning_content and delta.reasoning are present. The test asserts that only reasoning_content is emitted as the reasoning chunk, so it should catch regressions where the helper is bypassed or the logic is re-inlined incorrectly.
…imo, opencode-go, qwen-code
edelauna
left a comment
There was a problem hiding this comment.
Thank you for taking on this cleanup.
#588) * refactor(providers): use extractReasoningFromDelta helper for reasoning streams Replace duplicated reasoning_content extraction logic across 7 providers (deepseek, mimo, openai, opencode-go, qwen-code, requesty, unbound) with the shared extractReasoningFromDelta helper. This also adds the OpenRouter- style 'reasoning' field fallback to each provider for free. * test(providers): cover reasoning_content + reasoning fallback streaming Add streaming tests for openai, requesty, and unbound covering: - delta.reasoning_content yields a reasoning chunk - delta.reasoning fallback yields a reasoning chunk when reasoning_content is absent (OpenRouter-style) This raises line coverage on the changed reasoning paths from 50%. * test(providers): add reasoning_content streaming tests to deepseek, mimo, opencode-go, qwen-code --------- Co-authored-by: Oh Daewoong <dw.oh@samsung.com>
Zoo-Code-Org#588) * refactor(providers): use extractReasoningFromDelta helper for reasoning streams Replace duplicated reasoning_content extraction logic across 7 providers (deepseek, mimo, openai, opencode-go, qwen-code, requesty, unbound) with the shared extractReasoningFromDelta helper. This also adds the OpenRouter- style 'reasoning' field fallback to each provider for free. * test(providers): cover reasoning_content + reasoning fallback streaming Add streaming tests for openai, requesty, and unbound covering: - delta.reasoning_content yields a reasoning chunk - delta.reasoning fallback yields a reasoning chunk when reasoning_content is absent (OpenRouter-style) This raises line coverage on the changed reasoning paths from 50%. * test(providers): add reasoning_content streaming tests to deepseek, mimo, opencode-go, qwen-code --------- Co-authored-by: Oh Daewoong <dw.oh@samsung.com>
Related GitHub Issue
#570
Description
This PR replaces duplicated inline reasoning-delta extraction logic across seven providers with the shared
extractReasoningFromDeltahelper.Affected providers:
deepseekmimoopenaiopencode-goqwen-coderequestyunboundPreviously, each provider had a near-identical streaming check for
delta.reasoning_contentand emitted reasoning chunks inline. Sincesrc/api/providers/utils/extract-reasoning.tsalready provides the canonical extraction behavior and is already used by other OpenAI-compatible providers, this PR brings the remaining duplicated call sites in line with the shared implementation.Test Procedure
Pre-Submission Checklist
Existing tests cover the changed provider behavior, and the shared helper is already test-covered.
Screenshots / Videos
N/A
Documentation Updates
Additional Notes
This is a focused provider refactor. It removes duplicated reasoning extraction logic and routes all seven affected providers through the shared
extractReasoningFromDeltahelper.The intended behavior for existing
reasoning_contentchunks is unchanged for normal string payloads. The practical improvement is that these providers now also handle thereasoningfallback supported by the helper, so future or model-specific deltas using that field will stream reasoning text instead of being ignored.Get in Touch
hehegwk_23849
Summary by CodeRabbit
reasoningchunks using a shared approach (including correct field precedence and fallback).