fix: use stream for codex auto test - #4325
Conversation
WalkthroughThe changes enhance channel test response validation by distinguishing between stream and non-stream responses. Stream responses now require valid SSE event payloads beyond just error detection, and automatic channel testing now enables streaming mode exclusively for Codex channels rather than unconditionally disabling it. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
controller/channel-test.go (1)
573-603: Optional: deduplicate SSE line scanning withdetectErrorFromTestResponseBody.
validateStreamTestResponseBodyanddetectErrorFromTestResponseBodyshare nearly identical line-splitting /data:/[DONE]logic.validateTestResponseBodycurrently walks the body twice for stream responses. A small helper that yields decoded SSE payloads (or a single pass that both checks for upstream errors and tracks whether a valid event was seen) would DRY this up and halve the work on large bodies. Not blocking.♻️ Sketch
// iterateSSEPayloads invokes fn for each non-empty, non-[DONE] data: payload. // fn returns true to stop iteration. func iterateSSEPayloads(b []byte, fn func(payload []byte) bool) { for _, line := range bytes.Split(b, []byte{'\n'}) { line = bytes.TrimSpace(line) if len(line) == 0 || !bytes.HasPrefix(line, []byte("data:")) { continue } payload := bytes.TrimSpace(bytes.TrimPrefix(line, []byte("data:"))) if len(payload) == 0 || bytes.Equal(payload, []byte("[DONE]")) { continue } if fn(payload) { return } } }Then both validators can share it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/channel-test.go` around lines 573 - 603, The SSE parsing in validateStreamTestResponseBody is duplicated with detectErrorFromTestResponseBody and causes double work for stream responses; refactor by extracting a shared iterator (e.g., iterateSSEPayloads or similar) that scans bytes.Split lines, trims, filters non-"data:" lines and "[DONE]" payloads, and invokes a callback for each decoded payload; update validateStreamTestResponseBody to use the iterator to detect any valid event and update detectErrorFromTestResponseBody to use the same iterator to detect upstream error payloads, then have validateTestResponseBody call detectErrorFromTestResponseBody and, if streaming, rely on validateStreamTestResponseBody which now uses the shared iterator so the body is only scanned once per flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@controller/channel-test.go`:
- Around line 573-603: The SSE parsing in validateStreamTestResponseBody is
duplicated with detectErrorFromTestResponseBody and causes double work for
stream responses; refactor by extracting a shared iterator (e.g.,
iterateSSEPayloads or similar) that scans bytes.Split lines, trims, filters
non-"data:" lines and "[DONE]" payloads, and invokes a callback for each decoded
payload; update validateStreamTestResponseBody to use the iterator to detect any
valid event and update detectErrorFromTestResponseBody to use the same iterator
to detect upstream error payloads, then have validateTestResponseBody call
detectErrorFromTestResponseBody and, if streaming, rely on
validateStreamTestResponseBody which now uses the shared iterator so the body is
only scanned once per flow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 52527016-0799-4889-bd01-372dbf6e19ed
📒 Files selected for processing (1)
controller/channel-test.go
Resolve conflict in controller/channel-test.go: - Drop the legacy single-model testChannel call from origin/main (replaced by the per-model loop introduced in this PR). - Adopt the new shouldUseStreamForAutomaticChannelTest(channel) helper (added by upstream PR QuantumNous#4325) inside the per-model goroutine, so codex channels are still tested with stream mode under the per-model regime.
Important
📝 变更描述 / Description
codex渠道自动测试没有使用stream导致无法自动启用
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix,我已提交或关联对应 Issue,且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
(请在此粘贴截图、关键日志或测试报告,以证明变更生效)
Summary by CodeRabbit