Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
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 (4)
📝 WalkthroughWalkthrough本次变更将非空 Changesreasoning_content 流处理
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
github-project-automation
Bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
🧪 测试结果
总体结果: ✅ 所有测试通过 |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| // chunk 无事件名；delta 携带 content/reasoning/tool_calls/refusal/audio 即内容 | ||
| anyPaths: [ | ||
| "choices.#.delta.content", | ||
| "choices.#.delta.reasoning_content", |
There was a problem hiding this comment.
[HIGH] [LOGIC-BUG] reasoning_content is only supported on the live-stream gate, so fake-streaming requests still reject or drop DeepSeek reasoning-only output.
Why this is a problem: This line makes the normal SSE gate treat choices[].delta.reasoning_content as deliverable content, but the OpenAI-chat fake-streaming path still ignores the same field in both validateOpenAIChatCompletion() and emitOpenAIChatStream(). For a fake-streaming-eligible OpenAI request, tryFakeStreamingPath() rewrites the upstream call to non-stream, so a DeepSeek response whose payload is only message.reasoning_content will still fail validation with no_deliverable_content, and the stream emitter would omit the reasoning text even if validation were relaxed. That means the PR fixes the direct streaming path but leaves the fake-streaming path broken for the same provider behavior.
Suggested fix:
const typed = message as {
content?: unknown;
reasoning_content?: unknown;
tool_calls?: unknown;
function_call?: unknown;
};
if (isNonEmptyString(typed.reasoning_content)) return true;
if (typeof message.reasoning_content === "string") {
delta.reasoning_content = message.reasoning_content;
}Also add a fake-streaming regression test that covers a reasoning-only OpenAI-chat completion.
There was a problem hiding this comment.
Code Review Summary
The stream-gate change is small, but the new reasoning_content support is only wired into the live SSE gate. The fake-streaming OpenAI-chat path still treats reasoning-only DeepSeek completions as empty and can drop that output, so the fix is incomplete for fake-streaming-eligible requests.
PR Size: S
- Lines changed: 60
- Files changed: 4
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 1 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Critical Issues (Must Fix)
None.
High Priority Issues (Should Fix)
src/app/v1/_lib/proxy/stream-gate/frame-classifier.ts:103addsreasoning_contentto the live stream classifier, but fake-streaming still ignoresmessage.reasoning_content, so reasoning-only OpenAI-chat completions can still be rejected or emitted without their reasoning payload.
Review Coverage
- Logic and correctness
- Security (OWASP Top 10)
- Error handling
- Type safety
- Documentation accuracy
- Test coverage
- Code clarity
Automated review by Codex AI
There was a problem hiding this comment.
Review Complete
- Applied the
size/Slabel to PR#1396and submitted the review summary on GitHub. - Posted 1 high-priority inline comment on
src/app/v1/_lib/proxy/stream-gate/frame-classifier.ts:103. - Main finding: the new
reasoning_contentsupport is wired into the live SSE gate, but the fake-streaming OpenAI-chat path still ignoresmessage.reasoning_content, so fake-streaming-eligible DeepSeek requests can still reject or drop reasoning-only output. - I did not find additional high-confidence issues in the modified lines.
If you want, I can also prepare the concrete code patch for the fake-streaming validator/emitter gap.
1 participant
Release v0.9.2
Verification
dev.Review
Greptile Summary
Adds DeepSeek reasoning-stream support to the OpenAI-compatible stream gate.
choices[].delta.reasoning_contentvalues as usable stream content.Confidence Score: 5/5
The PR appears safe to merge with focused coverage of the new DeepSeek reasoning-stream behavior.
Non-empty reasoning deltas now follow the established OpenAI-chat content path, while empty deltas remain neutral and integration tests confirm that valid reasoning output avoids erroneous failover.
Important Files Changed
reasoning_contentdelta path to the existing OpenAI-chat content classification rules while preserving empty-value handling.reasoning_contentclassification behavior.Reviews (1): Last reviewed commit: "fix(proxy): recognize DeepSeek reasoning..." | Re-trigger Greptile
Context used: