fix(chat): include summary on replayed reasoning items

[Ingwannu]

Fixes #4628.

Summary

• emit summary: [] on Chat-to-Responses assistant reasoning replay items
• preserve raw reasoning_content only in content instead of exposing it as a visible summary
• document the wire/privacy boundary and pin it with regression assertions

Why

OpenAI Responses requires the summary member on replayed reasoning items. The local schema allowed it to be absent, so Astra tool continuations passed local validation and failed upstream with missing_required_parameter.

Using an empty summary satisfies the upstream shape without reclassifying raw provider reasoning as a user-visible summary.

Verification

• bun test tests/responses/chat-inbound-reasoning-replay.test.ts — 11 pass
• bun run typecheck — pass
• bun run structure:check — pass
• bun run privacy:scan — pass
• bun test tests/responses/chat-completions-endpoint.test.ts — 121 pass, 1 existing unrelated failure
• the same surrogate byte-accounting failure reproduces on unmodified origin/dev at 4f788f916ed08f629303f5e5608b275f56e59f77

All tests used isolated temporary HOME, OPENCODEX_HOME, and CODEX_HOME; protected local runtime settings were unchanged.

@lidge-jun This is a narrow current-release regression fix. Please review the exact head before merge; I will not self-approve.

Summary by CodeRabbit

• Bug Fixes
  • Improved compatibility when replaying assistant reasoning in Chat-to-Responses conversions.
  • Reasoning items now follow the expected response structure while remaining adjacent to the associated assistant message.
  • Provider-specific reasoning details remain protected and are not exposed as visible summary content.
• Tests
  • Added coverage to verify reasoning replay structure and prevent unintended disclosure of raw reasoning data.

[github-actions]

✅ Deterministic PR hygiene checks passed.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 07fd1029-3f7d-4870-8747-153d23d15e6a

📥 Commits

Reviewing files that changed from the base of the PR and between 4f788f9 and f48e582.

📒 Files selected for processing (3)

• src/chat/inbound.ts
• structure/data-planes/inbound-compat.md
• tests/responses/chat-inbound-reasoning-replay.test.ts

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

---

📝 Walkthrough

Walkthrough

The Chat-to-Responses translator now adds summary: [] to reconstructed reasoning items. Documentation records the shape, and tests verify both required-field presence and provider-reasoning isolation.

Changes

Reasoning Summary Compatibility

Layer / File(s)	Summary
Reasoning item shape src/chat/inbound.ts, structure/data-planes/inbound-compat.md	At src/chat/inbound.ts:313-322, reconstructed reasoning items now include summary: []. The compatibility documentation at structure/data-planes/inbound-compat.md:306-308 records the same shape and states that provider-specific attestation data is not reconstructed.
Reasoning replay validation tests/responses/chat-inbound-reasoning-replay.test.ts	The test at line 38 verifies the empty summary array. The safety assertions at lines 61-63 verify that provider reasoning is not placed in summary.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to f48e5

The translator now emits the required empty reasoning summary while keeping provider reasoning out of visible summary content; no merge-blocking risk is supported.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the primary change: adding summary data to replayed reasoning items. This matches the implementation in src/chat/inbound.ts and the related documentation and …
Linked Issues check	✅ Passed	Issue #4628 requires chatCompletionsToResponsesBody to emit a summary array on replayed Responses type: "reasoning" items. In src/chat/inbound.ts, the assistant branch creates the reasoning it…
Out of Scope Changes check	✅ Passed	The reviewed changes are limited to the Chat-to-Responses reasoning projection, its regression assertions, and documentation of the related compatibility and privacy boundary. The reasoning preservati…
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. 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 …

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/chat-reasoning-summary-4628

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[lidge-jun]

리뷰 · 우선순위 74 / 80

설명

이 PR(#4629, fix/chat-reasoning-summary-4628)은 #4628을 고친다. 현재 dev tip 4f788f916의 src/chat/inbound.ts는 Chat Completions 히스토리를 Responses input으로 옮길 때, assistant의 reasoning_content/reasoning_details를 { type: "reasoning", content: [{ type: "reasoning_text", text }] } 형태로만 넣고 summary 키를 아예 안 붙인다. 로컬 Zod(src/responses/schema.ts의 reasoningItemSchema.summary)는 optional이라 통과하지만, OpenAI Responses 쪽은 replay된 reasoning 항목에 summary를 필수로 본다. 그래서 gpt-6-astra처럼 reasoning+tools를 쓰는 Chat 클라이언트가 첫 라운드 뒤 이력을 다시 보내면 upstream이 HTTP 400 missing_required_parameter (input[N].summary)로 거절한다. 이슈 본문의 재현과 일치한다.

고치는 방법은 한 줄에 가깝다. reasoning 아이템에 summary: []를 명시한다. 이슈 제안처럼 raw reasoningText를 summary_text로 복사하지 않는다. 그 선택은 맞다. 저장소 불변식(Raw reasoning is never relabeled as a summary, src/types/provider.ts)과 Claude inbound·bridge가 raw/encrypted 경로에서 빈 summary: []를 쓰는 패턴과 같다. wire 모양만 맞추고, 제공자 raw thinking을 사용자에게 보이는 summary로 재분류하지 않는다. structure/data-planes/inbound-compat.md에도 그 경계를 한 문단 보강했다.

테스트는 tests/responses/chat-inbound-reasoning-replay.test.ts 두 곳에 expect(...summary).toEqual([])를 추가했다. (1) reasoning_content → reasoning 아이템 변환 직후, (2) signature/encrypted/id를 위조하지 않는다는 기존 privacy 테스트에 summary 비어 있음을 같이 핀다. PR 본문 기준 전용 스위트 11 pass, typecheck·structure·privacy 통과. chat-completions-endpoint의 기존 무관 실패 1건은 tip 4f788f916에서도 재현된다고 적어 두었다. 변경량은 +15/−3, base dev, 라벨 bug. types.ts/config.ts 분할과 무관하고 중복 PR도 없다.

74점 이유: 현재 릴리스(2.55.0 계열)에서 Chat→Responses 도구 연속 턴이 깨지는 명확한 회귀를, privacy 경계를 지키며 최소 diff로 막는다. Claude 쪽은 이미 summary를 내고 있어서 Chat 경로만의 구멍이다. 점수를 더 올리지 않은 이유는 로컬 스키마는 여전히 summary optional이라 ‘다른 생성 경로가 또 omit하면 같은 400이 난다’는 계약을 스키마로까지는 조이지 않았고, 라이브 Astra 재현 로그는 PR에 없고 단위 핀만 있기 때문이다. 그래도 머지 후보는 분명하다.

라인 ~316 src/chat/inbound.ts - 기존 push는 content만 있었다. 이번엔 summary: [] + content. adjacency(reasoning 직후 assistant message)는 그대로다. 주석이 ‘OpenAI requires summary / keep empty = raw not user-visible’를 설명해 이슈 제안(summary에 text 복사)과 의도적으로 갈라선 이유를 남긴다.
경로 src/responses/schema.ts reasoningItemSchema - 이 PR은 건드리지 않는다. summary는 계속 optional. 로컬 검증만으로는 이번 버그를 다시 못 잡는다. upstream 필수와 로컬 optional 사이의 간극이 남는다.
경로 structure/data-planes/inbound-compat.md - ‘always carries summary: []’ 한 문장 추가. Chat 경계에서 opaque reasoning replay는 여전히 unimplemented by design — 그 문장은 유지된다.
경로 tests/.../chat-inbound-reasoning-replay.test.ts - summary=[] 두 assert. tool-calling assistant 분기에도 reasoning이 생기므로, 그 테스트에 summary 핀을 하나 더 넣으면 도구 연속 턴 회귀(#4628 재현 축)가 더 직접적으로 잠긴다. 필수는 아니다.
심볼 assistantReasoningText / chatCompletionsToResponsesBody - reasoning 텍스트는 content에만 남고 summary로는 안 올라간다. Claude inbound가 thinking 길이에 따라 summary_text를 채우는 경우와 역할이 다르다(그쪽은 Messages thinking → summary 매핑이 계약). Chat 쪽 빈 배열 선택이 일관된다.
경로 #4628 이슈 제안 vs 이 PR - 이슈는 summary에 reasoningText를 넣자고 했다. PR은 []. privacy·showThinkingSummary 불변식상 PR이 맞다. 머지 코멘트에 ‘이슈 제안은 wire-only empty summary로 채택’이라고 한 줄 남기면 후속자가 다시 text를 넣지 않는다.

메인테이너의 판단이 필요한 지점

• 로컬 reasoningItemSchema.summary를 required(빈 배열 허용)로 올려 같은 omit을 로컬에서도 막을지, 아니면 이번처럼 emit 경로만 고칠지.
• [Bug]: Chat Completions translator omits reasoning.summary; gpt-6-astra 400s after tool turns #4628 제안의 summary_text 복사를 명시적으로 거절한 결정을 이슈 close 코멘트에 고정할지.
• 2.55.x 핫픽스/체리픽 대상인지, dev→다음 마이너(2.56.0 열린 상태)로만 태울지.
• tool-calling 분기 테스트에 summary 핀을 같은 PR에 더 넣을지.

너의 추천
CI green 확인 후 dev에 바로 착륙 후보. 좁은 현재 릴리스 회귀이고 privacy 선택이 저장소 불변식과 같다. 머지 시 #4628을 닫고, close 코멘트에 ‘summary는 빈 배열(raw를 summary_text로 올리지 않음)’을 한 줄 적는다. 스키마 required 강화는 별도 후속(다른 emit 경로 전수 조사 후)으로 남겨도 된다. types/config 분할·중복 close 대상 아님. 셀프 어프루브 금지 요청은 그대로 존중 — 메인테이너 리뷰 후 머지.

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

[abhisheksharma2411]

Read this against f48e5829. The fix looks right to me, and I think the reason it's right is a bit stronger than the PR body claims — worth writing down, plus one thing I'd want your read on.

The shape isn't new, it's the one the bridge already uses. I went looking for whether summary: [] or summary: [{summary_text}] was the correct call here, and the codebase already answers it consistently:

site	summary	content
src/bridge.ts:702 (raw reasoning)	[]	[{reasoning_text}]
src/bridge.ts:1831 (raw reasoning)	[]	[{reasoning_text}]
src/bridge.ts:684, :1810 (summary reasoning)	[{summary_text}]	—

So the rule in force is summary holds summary text, content holds raw text, and a raw item carries an empty summary. chat/inbound.ts was the one raw-reasoning producer not following it. That's a cleaner justification than "empty satisfies the wire shape", because it means you're not making a privacy judgement call here at all — you're matching the two sites that already made it.

I also checked src/claude/inbound.ts:259, which does put text into summary, in case this introduced an inconsistency. I don't think it does: Anthropic thinking blocks are already the user-visible summarized form, not raw provider reasoning, so summary is the right slot there. Different input, same rule.

The one thing I'd push on: the schema still admits the bug.

src/responses/schema.ts:69 is still summary: z.array(summaryTextSchema).optional() — which is exactly what #4628 names as the reason the broken payload passed local validation and only failed upstream. This PR fixes the producer and leaves the validator permissive, so a fifth reasoning-item producer added next month regresses identically and still passes bun test + typecheck locally, with the 400 showing up at Astra again.

I'm not saying make it required, because I don't think you can: reasoningItemSchema is a member of inputItemSchema (schema.ts:111), which validates input on the inbound request (schema.ts:155). Tightening it would start rejecting client payloads that omit summary, and a client is entitled to omit it on a request — the requirement is on replay to OpenAI, not on what we accept. So the obvious fix is the wrong one.

Which leaves the guard somewhere else, and I'd rather ask than prescribe. Something like a test that sweeps the Chat→Responses projection and asserts every emitted type: "reasoning" item has summary defined, rather than asserting it at out[idx] for the one item the two current tests construct — that catches a new producer on this path without touching what the proxy accepts. Does that match how you'd want it guarded, or is there a producer-side invariant helper already that I've missed?

Two smaller notes, neither blocking:

• The structure/data-planes/inbound-compat.md wording ("The item always carries summary: []") is now a documented invariant with nothing enforcing it, which is the same gap from the doc side. If the sweep test above happens, that sentence becomes true-by-test rather than true-by-convention.
• Your verification note that the surrogate byte-accounting failure also reproduces on unmodified origin/dev at 4f788f91 is the right way to report a pre-existing red — thanks for pinning the base commit rather than just calling it unrelated.

[Ingwannu]

Superseded by the broader merged fix in #4673 (369be813c4c8bcfd9d99d000085f7f6baf97e4c5). That change fixes the Chat ingress omission and also repairs summary-less reasoning items at the final Responses adapter boundary, so keeping this narrower alternative open would duplicate the same defect with a different continuity policy. Closing without merge.