fix(bridge): stop raw CoT from scrolling through the desktop thinking band on routed models - #4301
Conversation
Routed open-source models streamed their raw chain of thought through the summary channel (3183154, issue lidge-jun#45), so the Codex desktop thinking band scrolled unsummarized CoT with its flicker animation — a presentation that only fits native OpenAI providers that author real summaries. Stream raw reasoning as response.reasoning_text.delta (content_index 0) and close the item with content: [{type: "reasoning_text"}] plus an empty summary — the native gpt-oss shape, so Codex applies its own display policy: the desktop band shows the "Thinking..." placeholder and the CLI keeps raw display behind show_raw_agent_reasoning. Delete the content-to-summary payload rewrite for native Responses passthrough: its only purpose was that display, and DeepSeek's native content-channel round-trip works unchanged upstream. Hidden mode (summary absent/"none") is unchanged: envelope-only items with the txt-only ocxr1 round-trip for preserveReasoningContentModels replay. Signed thinking_delta (Claude/kiro visible mode) intentionally stays on the summary channel. Video evidence of the band behavior: .github/pr-assets/raw-reasoning-band.gif (reviewer's own desktop recording, published in this PR with consent).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (13)
💤 Files with no reviewable changes (4)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThis change routes visible raw reasoning through the Responses content channel, removes the server-side summary-channel rewrite, and updates tests, docs, and test-layout metadata to match the native ChangesReasoning content-channel routing
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix · Severity of issue fixed: Low Sequence Diagram(s)sequenceDiagram
participant Provider
participant Bridge
participant ResponsesCore
participant Client
Provider->>Bridge: raw reasoning delta
Bridge->>Client: response.reasoning_text.delta
Provider->>Bridge: raw reasoning complete
Bridge->>Client: output item content[{type:"reasoning_text", text}]
Provider->>ResponsesCore: native /responses reasoning_text events
ResponsesCore->>Client: passthrough without summary rewrite
Merge Risk: ⚪ Minimal · up to The reasoning channel migration is covered by updated streaming, buffered, passthrough, replay, and hidden-mode tests; no current merge-blocking risk was identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 7 files. (2 skipped: 2 unsupported.)
✨ 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 |
|
✅ Deterministic PR hygiene checks passed. |
✅ READY
Review readiness checklist
✅ 4/4 boxes ticked. This pull request is already Ready for Review. Hygiene✅ Deterministic PR hygiene checks passed. |
리뷰 · 우선순위 70 / 80설명 이 PR은 라우팅된 오픈소스 모델(GLM / DeepSeek / Grok chat, Kiro raw 태그)의 날것 사고 과정(CoT)이 Codex 데스크톱 thinking 밴드에 그대로 스크롤되던 증상을 고칩니다. 지금 고치는 방향은 네이티브 gpt-oss와 같은 content 채널입니다. 라이브는 이 변경이 중요한 이유는 제품 선택이기 때문입니다. #45가 주려던 “데스크톱에서 펼쳐 읽는 CoT”를 되돌리고, 대신 밴드를 안정시키는 쪽입니다. structure 문서
메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
|
Thanks for the review. Working through the three points:
Signed SIGSEGV flake — separated from real regressions: the four named files pass in isolation (27 pass / 0 fail), the crash reproduces on the pre-change tree, and a clean rerun of the full local suite finished with the parallel suite at exit 0 (200.9 s), every serial file exit 0, zero fail lines and zero worker crashes. The two Readiness — the checklist is complete and the gate marked this PR ready (4/4, head On #45 — the product call you flagged is the maintainer decision, and the PR body states the tradeoff explicitly: desktop keeps the "Thinking…" placeholder instead of an expandable raw CoT, and the raw trace stays available in the CLI behind |
RobinBially
changed the title
showThinkingSummary's job is to take a provider's genuine reasoning out of the hidden replay envelope. Which channel carries the visible text is the bridge's decision, not this flag's: lidge-jun#4301 moves raw reasoning from the summary channel to the content channel (the native gpt-oss shape), so asserting the summary channel here would pin the opposite of whichever behaviour is current. Rewritten around the Cloud Code Assist path the flag exists for. That also lets the request-side half be asserted in the same file: includeThoughts reaching the wire when a provider opts in, and not being bought at all for a turn the client asked to hide. The passthrough-based cases are dropped -- lidge-jun#4301 deletes the content-to-summary rewrite they exercised, and with it their subject. Comment-only edits keep provider.ts, registry.ts and core.ts from claiming the summary channel as the contract; the docs row says the same and documents the explicit false.
|
Landed via #4351 at #4345가 raw CoT vs provider summary 분리를 |
lidge-jun
added
the
landed-via-maintainer
2 participants
Summary
response.reasoning_text.deltalive plus a finalreasoningitem withcontent: [{type: "reasoning_text", text}]and an emptysummary— the exact nativegpt-ossshape from the100_codex-native-parity/51_raw-reasoning-bridgeresearch. Codex applies its own display policy: the desktop band shows the generic "Thinking…" placeholder, and the CLI keeps raw display behindshow_raw_agent_reasoning(opt-in).reasoning.summaryabsent/"none") is unchanged — envelope-only items with the txt-onlyocxr1round-trip keeppreserveReasoningContentModelsreplay working for GLM interleaved thinking.Video evidence
raw-reasoning-band.mp4
Verification
bun run typecheck,bun run privacy:scan,bun run structure:check— all green.bun run test:changedon this change set: 8891 pass / 0 fail / 2 skip across 388 files (91 s), covering the bridge, passthrough relay, reasoning replay, xAI streaming, web-search, and layout gates.bun run test(local): clean rerun — parallel suite exit 0 (200.9 s), every serial test file exit 0, zero fail lines, zero worker crashes. The first two runs exited 1 through a Bun worker SIGSEGV (tests/routing/routing-policy-surface-parity.test.ts) with cascading worker aborts; all four named files pass in isolation (27 pass / 0 fail) and the same crash appears on the pre-change tree, so it is infrastructure flake, not a regression. The twoexit 1fixture invocations inside the runner log are expected negative-fixture cases, not failures.bun test tests/adapters/bridge.test.ts tests/adapters/bridge-raw-reasoning-hidden.test.ts tests/adapters/bridge-reasoning-replay-batch.test.ts tests/adapters/reasoning-replay-robustness.test.ts tests/images/loop-reasoning-replay.test.ts tests/web-search/web-search.test.ts tests/server/server-xai-chat-reasoning-streaming.test.ts tests/responses/responses-reasoning-summary-passthrough.test.ts tests/providers/opencode-go-luna-wire.test.ts— green.Checklist
structure/providers/chat-compat.md, devlog record).devcommit.Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
All CI tests are green on my local testing.
I pushed my PR to the latest dev commit.
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.
Summary by CodeRabbit
Behavior Changes
reasoning_textevents and content items instead of summary-channel reasoning events.Documentation
Tests