fix(proxy): recognize terminal Responses compaction streams (#1410) - #1411
Conversation
Some Responses upstreams return compaction output only in response.completed without first emitting response.output_item.done, so the content gate treated the stream as empty and triggered false failover with circuit-breaking. classifyParsedFrame now accepts the protocol family and, for openai-responses, detects a compaction output item with non-empty encrypted_content inside response.completed, classifying the frame as content so the gate commits the stream. Regression tests cover compaction-only terminal frames and custom tool-call input deltas across the classifier, content gate, and forwarder integration paths. Fixes #1410
📝 WalkthroughWalkthrough本次修改扩展 OpenAI Responses 流内容分类,识别 compaction 完成帧和 custom tool-call 输入增量,并补充内容门、转发及 Replay owner 模式测试。 ChangesOpenAI Responses 流网关
Estimated code review effort: 3 (Moderate) | ~20 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@src/app/v1/_lib/proxy/stream-gate/frame-classifier.ts`:
- Around line 418-422: 将 frame-classifier.ts 中 response.completed 的 compaction
encrypted_content 校验改为仅接受非空字符串,并在 response.output_item.done 使用相同约束,替换
isNonEmptyValue 的宽泛判断;在 tests/unit/proxy/stream-gate-frame-classifier.test.ts
的相关测试中补充 true、数字、对象等非字符串值,断言其分类为 terminal 或 neutral。
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ec92618c-a1f6-4346-a891-1ad2bc0cdc45
📒 Files selected for processing (4)
src/app/v1/_lib/proxy/stream-gate/frame-classifier.tstests/unit/proxy/stream-gate-content-gate.test.tstests/unit/proxy/stream-gate-forwarder-integration.test.tstests/unit/proxy/stream-gate-frame-classifier.test.ts
🧪 测试结果
总体结果: ✅ 所有测试通过 |
There was a problem hiding this comment.
Code Review Summary
No significant issues identified in this PR.
PR Size: M
- Lines changed: 209
- Files changed: 4
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - Adequate
- Code clarity - Good
Automated review by Codex AI
There was a problem hiding this comment.
- Reviewed PR
#1411across the modified lines insrc/app/v1/_lib/proxy/stream-gate/frame-classifier.ts:359,tests/unit/proxy/stream-gate-content-gate.test.ts:257,tests/unit/proxy/stream-gate-forwarder-integration.test.ts:227, andtests/unit/proxy/stream-gate-frame-classifier.test.ts:273; I did not find any issues that cleared the reporting threshold. - Applied the
size/Mlabel and posted the required PR review summary with no inline defect comments. - I also attempted focused Vitest validation, but this runner does not have
bun/bunxornode_modulesavailable, so the review outcome is based on full diff/context analysis rather than local execution.
…ontent The compaction signal rule on response.output_item.done accepted any truthy encrypted_content value, allowing non-string types (booleans, numbers, objects) to be misclassified as content. Consolidate the per-item type guard into isNonEmptyCompactionItem and apply it to both response.output_item.done and response.completed paths so the opaque state must be a non-empty string before a frame is committed as content. Add regression tests covering malformed encrypted_content types across event variants.
🧪 测试结果
总体结果: ✅ 所有测试通过 |
问题
OpenAI Responses API 的合法流不一定包含
response.output_text.delta。当上游只在 terminalresponse.completed的response.output[]中返回type: "compaction"和非空encrypted_content时, stream gate 会先看到 terminal 而没有识别到有效内容, 最终以empty_stream返回 502, 并错误触发 provider failover 与 circuit breaker failure recording.response.custom_tool_call_input.delta/.done在当前dev已有非空内容识别规则, 但此前缺少覆盖 #1410 实际 Replay owner 与 forwarder 路径的直接回归测试.Fixes #1410
修改
openai-responses启用 terminal compaction 识别.response.completed的response.output[]逐项检查, 只在同一个 item 同时满足type === "compaction"且encrypted_content非空时判为content.response.output_item.done与response.completed共享同一个 exact predicate,encrypted_content只接受官方 schema 定义的非空 string; boolean、number、array/object 不再误提交.Replay owner 语义
本 PR 不移除 Replay owner 的 precommit gate。
stream_gate_mode=off仍只关闭普通请求 gate; Replay owner 继续执行 pre-content safety check, 防止错误或畸形前缀进入共享 Replay spool 并污染 attach follower.修复后的行为是: 即使
stream_gate_mode=off且请求为 Replay owner, 合法 terminal compaction 与 custom tool-call input stream 也会被识别为有效内容, 原样返回 HTTP 200, 不发生 502、failover 或 circuit breaker failure recording.验收
bun run lint:fix: passed.bun run lint: passed.bun run typecheck: passed.bun run build: passed.git diff --check: passed.keys-edit-key-expires-at-clear.test.ts出现 2 个 flaky failures; 该文件单独重跑 6/6 passed; 修复前完整复验为 860 files / 8453 tests passed; review 修复后完整复验为 860 files / 8454 tests passed, 0 failures, exit 0.encrypted_content类型契约 finding 已按官方 OpenAI API schema 修复并补 TDD 负例.结构化验收报告:
风险边界
本次没有单独新增 hedge-path 的 compaction fixture、同帧
response.error+ compaction 组合或无 SSEevent:行组合。实现仍通过共享 content gate、embeddeddata.type推导和 error-first 顺序覆盖这些路径; 独立审查未发现缺陷.Greptile Summary
The PR updates the OpenAI Responses stream classifier to recognize non-empty compaction payloads in both output-item and terminal completion frames.
Confidence Score: 5/5
The PR appears safe to merge.
No blocking failure remains.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[OpenAI Responses SSE frame] --> B[Parse event name and data] B --> C{Error rule matches?} C -->|Yes| D[Classify as error] C -->|No| E{Compaction item with non-empty encrypted content?} E -->|Yes| F[Classify as content and commit stream] E -->|No| G{Existing content rule matches?} G -->|Yes| F G -->|No| H{Terminal event?} H -->|Yes| I[Classify as terminal] H -->|No| J[Classify as neutral]Reviews (2): Last reviewed commit: "fix(stream-gate): require non-empty stri..." | Re-trigger Greptile
Context used: