fix(openai-chat): preserve empty thinking replay by parkjs101 · Pull Request #5449 · lidge-jun/opencodex

parkjs101 · 2026-09-21T10:16:36Z

Summary

Preserve signed or redacted thinking-only assistant turns when translating Responses history to OpenAI Chat.
Apply the existing configured reasoning_content placeholder to those turns even when they contain no tool call, while retaining explicit provider opt-outs such as MiniMax.
Add parser-to-adapter regression coverage for the native-parent agent_message handoff and document the replay contract.

Closes #5421

Co-authored-by: Daniel Sjöstrand 16033062+Danielsjostrand1979@users.noreply.github.com

Verification

bun test tests/providers/deepseek-reasoning-replay-gaps.test.ts -t 'issue #5421' — red before the production change, green afterward.
bun test tests/providers/deepseek-reasoning-replay-gaps.test.ts — 20 passed, 0 failed.
bun run structure:check — passed.
bun run privacy:scan — passed.
git diff --check — passed.
bun run lint:gui:if-changed and bun run doctor:gui:if-changed — passed by correctly skipping because this PR has no GUI changes.
bun run typecheck — blocked by two existing RequestInit.timeout errors in src/server/claude-messages.ts and src/server/responses/fetch-helpers.ts; the same errors reproduce on a detached clean origin/dev worktree.
bun run test — exceeded the repository's 900-second parallel-suite limit after unrelated timing/platform failures; the changed DeepSeek regression file passed in that run.
bun run test:changed — selected 516 of 1501 files for this central adapter change and was stopped after roughly seven minutes; no changed-file failure was observed before stopping.

Checklist

Scope stays focused and avoids unrelated cleanup.
Docs or release notes were updated when needed.
Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

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

Bug Fixes
- Improved compatibility with reasoning-focused models when assistant responses contain thinking content without visible text.
- Preserved required assistant placeholders to prevent request rejection during subsequent conversations.
- Maintained the expected behavior for models that explicitly opt out of these placeholders.
Documentation
- Clarified reasoning placeholder behavior for thinking-only assistant responses.

Co-authored-by: Daniel Sjöstrand <16033062+Danielsjostrand1979@users.noreply.github.com>

coderabbitai · 2026-09-21T10:16:42Z

Understand this PR’s impact

Explore downstream dependencies and potential security impact with Blast Radius.

View blast radius →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: d07340c5-164d-4644-9add-aad846c858ff

📥 Commits

Reviewing files that changed from the base of the PR and between 52acf81 and 65ed520.

📒 Files selected for processing (3)

src/adapters/openai-chat/messages.ts
structure/providers/chat-compat.md
tests/providers/deepseek-reasoning-replay-gaps.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

Walkthrough

The serializer now injects a reasoning placeholder for preserved thinking-only assistant turns without plaintext or tool calls. Documentation and tests cover DeepSeek preservation and MiniMax omission behavior.

Changes

Reasoning replay handling

Layer / File(s)	Summary
Thinking-only placeholder injection `src/adapters/openai-chat/messages.ts`, `structure/providers/chat-compat.md`	The fallback now runs when an assistant turn contains thinking parts or tool calls. The documentation and comment describe the missing-reasoning case.
Provider-specific regression coverage `tests/providers/deepseek-reasoning-replay-gaps.test.ts`	Tests verify that DeepSeek receives a single-space reasoning placeholder and that MiniMax does not synthesize an assistant message when placeholders are disabled.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: lidge-jun

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (1 skipped: 1 …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Issue `#5421` requires the OpenAI Chat adapter to replay or synthesize `reasoning_content` when a preserved thinking turn has empty plaintext, including a thinking-only native-parent-to-routed-child han…
Out of Scope Changes check	✅ Passed	The reviewed changes are limited to the assistant reasoning fallback in `src/adapters/openai-chat/messages.ts`, regression coverage in `tests/providers/deepseek-reasoning-replay-gaps.test.ts`, and the…
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: preserving empty thinking replay in the OpenAI Chat adapter.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (1 skipped: 1 unsupported.)

Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create a new PR

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

github-actions · 2026-09-21T10:16:48Z

✅ Deterministic PR hygiene checks passed.

github-actions · 2026-09-21T10:17:03Z

⏳ DRAFT

review readiness checklist open (0/4 boxes ticked).

What to do

Tick all four boxes in the PR description once you're done (currently 0/4).

Review readiness checklist

⬜ 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.

0/4 boxes ticked.

This PR stays in draft until every box above is ticked.

lidge-jun · 2026-09-21T11:00:00Z

리뷰 · 우선순위 63 / 80

이 PR은 DeepSeek 생각 모드에서, 글자 없는 생각 턴이 빠지면서 다음 요청이 HTTP 400이 나는 문제를 고칩니다.

부모 에이전트가 자식에게 일을 넘기면, 생각이라고 표시된 조각은 남는데 그 안 글자는 비어 있을 수 있습니다. 예전 코드는 도구 호출이 있을 때만 빈 자리에 공백 한 칸을 넣었습니다. 도구 호출이 없으면 그 턴을 통째로 빼 버렸고, DeepSeek은 생각을 다시 넘기라며 400을 냈습니다. 이슈 #5421이 말한 막힘입니다.

이제는 생각 조각만 있어도 같은 공백(reasoning_content에 " ")을 넣어서 턴을 남깁니다. MiniMax처럼 이 공백을 쓰지 말라고 꺼 둔 모델은 예전처럼 그 턴을 뺍니다. 테스트 하나와 설명 문서 한 줄을 더했습니다. 베이스는 dev입니다. 아직 초안이고 본문 준비 체크는 0/4입니다.

src/adapters/openai-chat/messages.ts - 빈 생각 조각이 있으면 도구 호출이 없어도 공백을 넣습니다. 고친 조건은 이슈가 짚은 원인과 같습니다. 넣는 값은 원래 생각이 아니라 공백 한 칸입니다. DeepSeek이 공백을 지우고 비었다고 보면 400은 그대로입니다. 화면에 보일 글자가 있는 답변도, 빈 생각 조각이 붙어 있으면 같은 공백이 붙습니다.

tests/providers/deepseek-reasoning-replay-gaps.test.ts - 새 테스트는 서명만 있는 reasoning과 agent_message를 파서에 넣습니다. 공백이 생기고, 그 턴이 다음 사용자 글보다 앞에 있는지만 봅니다. 이슈 로그의 "암호문을 생략 표시로 바꿨다"는 경로는 재현하지 않았고, 실제 DeepSeek이 이 본문을 200으로 받는지도 확인하지 않았습니다. 본문은 이 파일 테스트는 통과했고, 저장소 전체 테스트는 시간 초과로 끝까지 돌지 않았다고 적혀 있습니다. CI는 위생 검사만 초록입니다.

메인테이너의 판단이 필요한 지점
공백 한 칸으로 이 400을 막을지, 빈 생각 턴을 다른 모양으로 남길지 정해야 합니다. 보일 글자가 있는 답변까지 공백을 붙이는 범위도 같은 판단입니다. 초안인 채로 둘지, 체크리스트를 채운 뒤에 리뷰를 열지도 정해야 합니다.

너의 추천
방향은 맞으니 닫지 말고 살리는 쪽이 좋습니다. 같은 이슈를 고치는 다른 PR은 보이지 않습니다. 머지 전에는 생각만 있는 턴의 content가 빈 문자열이고 reasoning_content가 공백일 때 DeepSeek이 200을 주는지 한 번 확인하세요. 테스트가 그 두 필드를 같이 보면, 나중에 턴이 다시 빠져도 바로 들킵니다.

이 댓글은 grok-bot이 작성했습니다

… fixes (#5619) * fix(cursor): bound capability reads and buffered tool budgets (#5533) Carries #5533 (and the closed #5233 it consolidates) onto current dev. Co-authored-by: Epinephrine <luvs01@hanmail.net> * fix(moonshot): bound normalized tool-schema expansion (#5547) Carries #5547, which consolidates #5464 and the request-wide inline budget, onto current dev. Co-authored-by: yeongjunyoo <47925973+yeongjunyoo@users.noreply.github.com> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: Epinephrine <luvs01@hanmail.net> * fix(moonshot): restore rejected inline budgets and charge nested growth once A rejected sibling-reference expansion now restores the byte, node and expansion allowances it consumed, and outer growth no longer re-charges nested copies, so later independent expansions in the same request keep their allowance. Documents the provider-driven object type inference as a deliberate tradeoff and rewrites ADR-0355 in English. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> * feat(reasoning): consolidate replay, opt-in tag parsing, and summary policy (#5566) Carries #5566, which consolidates #5449, #5205 and #5491, onto current dev. The provider guide keeps the current bridge replay paragraph and adds the inline-tag and summary paragraphs. Co-authored-by: Joonsuh Park <trckstr4422@gmail.com> Co-authored-by: Daniel Sjöstrand <16033062+Danielsjostrand1979@users.noreply.github.com> Co-authored-by: alexph-dev <alexph-dev@users.noreply.github.com> Co-authored-by: Yum-wu <1172989563@qq.com> * fix: bound Fernet slot runs, Kiro error-body read, and skill-path line slice (#5310) Carries #5310 onto current dev. The follow-up commit makes the Fernet run cap fail closed and moves the Kiro regression out of the capped stream suite. * docs(reasoning): reconcile inline-tag whitespace contract Interleaved inline-tag parsing preserves answer whitespace; only Kiro single-block mode drops the whitespace after its leading block. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> * fix(responses): fail closed on Fernet run overflow and keep the Kiro suite under its cap A slot with more than 64 structurally valid Fernet runs is now treated as unreadable or omitted as a whole, so no unexamined tail reaches the provider as text. The bounded Kiro fallback error-body regression moves byte for byte into a registered sibling file, and the Kiro, Responses and inbound contracts document the new bounds. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> * fix(reasoning): scan inline think tags with a moving cursor The parser copied, rescanned and reserved the whole remaining response after every block, so one upstream chunk carrying many short blocks cost quadratic work. It now scans each chunk from an offset and charges the translator budget only for retained carry: undecided leading input or a trailing tag fragment. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> * fix(reasoning): keep undecided leading whitespace incremental Before the format was decided, every content delta rebuilt, trimmed and re-reserved the whole leading prefix, so a stream of one-character whitespace deltas cost quadratic work. Leading whitespace is now kept in segments whose bytes are reserved once and joined only when the format is decided or the stream flushes. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> * fix(meta-muse): consolidate login admission and bounded response handling (#5591) Carries #5591, which consolidates the closed #5234 and #5432, onto current dev. The provider contract keeps the inline-tag paragraph and adds the Meta Muse admission paragraph. Co-authored-by: Epinephrine <luvs01@hanmail.net> * fix(claude-desktop): keep applied state consistent across profile edits (#5590) Carries #5590, which consolidates the closed #5337, onto current dev. Co-authored-by: Epinephrine <luvs01@hanmail.net> Co-authored-by: luvs01 <luvs01@users.noreply.github.com> * fix(claude-desktop): commit applied markers only over the observed baseline Both Desktop writers, provider-change auto-apply and client sync, now capture the desired profile and its applied marker before the Desktop write and commit the new marker only if profile presence, content, fingerprint and timestamp are unchanged. A concurrent edit, deletion or newer marker keeps its state and the write reports a skipped marker. The provider-change path no longer saves a whole stale config snapshot. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> * fix(claude-desktop): commit profile edits against the persisted marker The Desktop profile PUT built its response from an earlier snapshot and saved that whole snapshot, so a marker committed by another writer during the awaited state build could be replaced by an older one. The edit now commits in one persisted-config mutation that keeps the latest marker for unchanged content and answers 409 when the profile itself changed meanwhile. The Meta Muse overflow test now asserts that the bounded-body limit, not a generic failure, produced the error. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> * fix(claude-desktop): report an unreadable config separately from an edit conflict A missing or invalid config now answers 500 with its reason; only a concurrent profile change or exhausted rebase answers 409. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> * feat(desktop): consolidate consent-based runtime takeover and ownership contracts (#5564) Carries #5564, which consolidates #5459 and #5457, onto current dev. The review screenshot stays in the pull request description rather than the tree. Co-authored-by: jun <bitkyc08@gmail.com> Co-authored-by: sanggyulee <andy53295774@gmail.com> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(desktop): bind takeover stop to the approved runtime and fail closed Desktop takeover re-resolves ownership immediately before stopping and passes the approved PID, endpoint, config home, CLI version and compatibility token to an opt-in guarded stop. The guard is checked under the ownership mutation lease before any manager or signal stop; the approved PID and endpoint must settle and the service manager must then be proven inactive, otherwise the stop answers approval-changed or manager-still-active and the desktop neither waits for silence nor claims. Unreadable or unparseable stop output is terminal as well. A second unreadable service-state read now blocks takeover, Windows managing-CLI discovery follows PATHEXT with file-only candidates and refuses command-interpreter metacharacters, the claim refusal test uses real sandbox state, and the runtime and desktop contracts record that the claim token is a consistency check rather than consent proof. Plain ocx stop is unchanged. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> * fix(desktop): keep plain stop entry points and format the takeover changes Desktop exit keeps its plain runtime_stop::run entry while takeover uses run_approved, AttachPlan::Ask no longer carries an unread field, the Rust changes follow rustfmt, the plain CLI stop path keeps its literal outcome return, the stop source oracles follow the reader and outcome union that now include the two guarded refusals, and the runtime contract fits its 600-line budget. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> * fix(desktop): run takeover seam tests without tokio macros and harden manager and shim checks The two async takeover seam tests now run on the shell runtime already used by the crate instead of tokio test macros, which this crate does not enable. Windows command-shim probes refuse command-interpreter metacharacters in every recorded argument as well as the executable, and the guarded stop re-inspects the service manager identity immediately before the manager command, answering approval-changed without stopping if it moved. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> * fix(responses): keep effort-based reasoning visible after routing Final-route normalization recomputed hideThinkingSummary without the validated active-effort condition, so routed Chat and Kiro requests with an active effort and an omitted summary still hid raw reasoning. It now uses the same predicate as the parser; explicit "none" and requests without an active effort stay hidden. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> * fix(service): match the running CLI case-insensitively only on Windows On case-sensitive filesystems a PATH executable that differs only in case is a different file, so it must get its own version probe instead of reporting the running CLI version. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> * fix(meta-muse): require the dashboard session for manual login codes The manual-code continuation now applies the same dashboard-session admission as the login start, so a management token cannot advance a pending Meta Muse login. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> * fix(reasoning): reserve the joined leading-whitespace copy Joining retained leading whitespace allocated a second copy outside the translator budget; the join is now reserved first and released once the segments are cleared. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> * fix(service): skip CLI probes for an absent runtime and treat failed systemd units as stopped Resolve no longer spawns managing-CLI version probes when no runtime is live, since takeover is only offered for a live runtime. A systemd unit reported failed with no main PID is stopped, so a guarded stop that leaves it failed succeeds and a leftover failed unit does not block takeover. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> * fix(service): keep failed systemd units fail-closed and assess takeover only for a live runtime in tests systemd can report failed before an automatic restart, so failed with no main PID is again treated as unknown rather than stopped. The resolve contract tests that assert ownership and takeover fields now use a live runtime, matching the skip of managing-CLI probes when no runtime is live. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> --------- Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> Co-authored-by: Epinephrine <luvs01@hanmail.net> Co-authored-by: yeongjunyoo <47925973+yeongjunyoo@users.noreply.github.com> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: Joonsuh Park <trckstr4422@gmail.com> Co-authored-by: Daniel Sjöstrand <16033062+Danielsjostrand1979@users.noreply.github.com> Co-authored-by: alexph-dev <alexph-dev@users.noreply.github.com> Co-authored-by: Yum-wu <1172989563@qq.com> Co-authored-by: luvs01 <luvs01@users.noreply.github.com> Co-authored-by: sanggyulee <andy53295774@gmail.com>

lidge-jun · 2026-09-23T04:12:58Z

Closing as superseded. The changes from this PR (head 65ed520482aa, by @parkjs101) were carried with credit into #5566, which was consolidated into #5619. #5619 merged to dev as e964387. The carry was reimplemented as a squash with review repairs, not merged, so this branch's own commit history is not part of dev. I compared this head against current dev and found its behavior present, in some cases in revised form. The preservation of empty-thinking replay is present.

This is on dev only. It is not in the stable v2.63.0 release and will ship in a later release. Thank you for the contribution.

fix(openai-chat): preserve empty thinking replay

65ed520

Co-authored-by: Daniel Sjöstrand <16033062+Danielsjostrand1979@users.noreply.github.com>

github-actions Bot added the bug Something isn't working label Sep 21, 2026

parkjs101 marked this pull request as ready for review September 21, 2026 10:18

parkjs101 requested review from Ingwannu and lidge-jun as code owners September 21, 2026 10:18

github-actions Bot marked this pull request as draft September 21, 2026 10:18

parkjs101 marked this pull request as ready for review September 22, 2026 01:03

github-actions Bot marked this pull request as draft September 22, 2026 01:03

luvs01 mentioned this pull request Sep 22, 2026

feat(reasoning): consolidate replay, opt-in tag parsing, and summary policy #5566

Closed

3 tasks

lidge-jun mentioned this pull request Sep 22, 2026

L7: carry luvs01 desktop takeover, reasoning, provider and management fixes #5619

Merged

3 tasks

lidge-jun closed this Sep 23, 2026

Navigation Menu

fix(openai-chat): preserve empty thinking replay - #5449

fix(openai-chat): preserve empty thinking replay#5449
parkjs101 wants to merge 1 commit into
lidge-jun:devfrom
parkjs101:codex/fix-deepseek-empty-thinking

parkjs101 commented Sep 21, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Sep 21, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

github-actions Bot commented Sep 21, 2026

Uh oh!

github-actions Bot commented Sep 21, 2026 •

edited

Loading

Uh oh!

lidge-jun commented Sep 21, 2026

Uh oh!

lidge-jun commented Sep 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

parkjs101 commented Sep 21, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Verification

Checklist

Review readiness checklist

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Sep 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

github-actions Bot commented Sep 21, 2026

Uh oh!

github-actions Bot commented Sep 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⏳ DRAFT

What to do

Review readiness checklist

Uh oh!

lidge-jun commented Sep 21, 2026

리뷰 · 우선순위 63 / 80

Uh oh!

lidge-jun commented Sep 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

parkjs101 commented Sep 21, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Sep 21, 2026 •

edited

Loading

github-actions Bot commented Sep 21, 2026 •

edited

Loading