fix(responses): preserve visible reasoning when summary mode is omitted

[Yum-wu]

Summary

Fixes an issue where requests requesting active reasoning effort (e.g. reasoning.effort: "high", including combo virtual models with injected default effort) unintentionally had visible reasoning suppressed because parser.ts defaulted hideThinkingSummary = true when reasoning.summary was omitted.

Root Cause

1. In src/responses/parser.ts:544-545, options.hideThinkingSummary was set to true whenever !summaryMode || summaryMode === "none".
2. However, standard clients (such as OpenAI Chat Completions clients or generic Responses callers) specifying reasoning.effort commonly omit the Responses-exclusive reasoning.summary parameter. Defaulting to hideThinkingSummary = true caused bridge/sse.ts to hide raw reasoning deltas into hiddenRawReasoning instead of streaming them visibly via response.reasoning_text.delta.
3. Furthermore, when combos/request.ts injected a default effort into child requests, it set reasoning = { effort: resolvedEffort } without specifying summary: "auto".

Fix

1. In src/responses/parser.ts, only default to hideThinkingSummary = true when summaryMode === "none" or when no active reasoning effort was requested (!summaryMode && !reasoningActive). If an active effort (e.g. high, max) is requested, visible reasoning is preserved.
2. In src/combos/request.ts, default summary: "auto" when injecting default reasoning effort into combo child requests.
3. Keep src/bridge/sse.ts untouched, fully preserving the existing raw-reasoning contract tested in bridge-raw-reasoning-hidden.test.ts.

Rebase and a registration gap this PR had introduced

Rebased onto the current dev head (a4bdc03054d4a449c7762eec9853f04557a06fb4), previously 30 commits behind. No conflicts; the source diff is unchanged.

The rebase surfaced a real defect in the earlier head: the new test file was never listed in the test-layout tables, so tests/test-layout.test.ts and tests/test-layout-tooling.test.ts both failed with unresolved: ["responses/reasoning-effort-summary-default.test.ts"]. The regex seed for the responses domain does not cover a reasoning- prefix, and a file only the seeds know is exactly the state AGENTS.md says must not persist for a merged regression test. scripts/test-layout/layout.json and tests/fixtures/test-layout-expected.json now both classify it as responses, so the two tables stay identical.

Verification

Focused suites on the rebased head — all pass:

bun test tests/responses/reasoning-effort-summary-default.test.ts \
         tests/responses/responses-parser.test.ts \
         tests/adapters/bridge-raw-reasoning-hidden.test.ts \
         tests/responses/chat-inbound-reasoning-none.test.ts \
         tests/responses/responses-show-thinking-summary.test.ts \
         tests/responses/responses-reasoning-summary-passthrough.test.ts \
         tests/responses/responses-reasoning-effort-downgrade.test.ts \
         tests/test-layout.test.ts \
         tests/test-layout-tooling.test.ts
# 153 pass / 0 fail across 9 files

The two layout guards were also run on the unmodified dev head first (18 pass / 0 fail), which is how the missing registration above was attributed to this branch rather than to dev.

Coverage added by tests/responses/reasoning-effort-summary-default.test.ts (5/5):

• Active reasoning effort preserves visible thinking when summary is omitted.
• Explicit summary: "none" still hides thinking summary.
• Omitted effort and effort: "none" still default to hideThinkingSummary = true.
• Combo child request injection defaults summary to "auto".

Two suites were not run to a green result locally, and neither is a regression from this branch:

• bun run typecheck fails on src/usage/log.ts, src/usage/user-cost-overlay-reconciler.ts and src/vision/index.ts (TS2591 / TS2693 / TS7006). This environment is missing @types/node; the unmodified dev head fails identically at the same errors.
• bun run test hit the runner's 900s ceiling (exit 124) on this Windows box, with unrelated 5s timeouts in responses-self-named-namespace-scrub.test.ts and plaintext-v2-agent-messages-server.test.ts. Both files were then run against the unmodified dev head and fail the same way (4 pass / 33 fail), so the timeouts are environmental rather than caused by this change.

Typecheck and the full suite are therefore left to CI, which runs on a provisioned runner. Scope exception per AGENTS.md: focused regression tests for the changed behaviour are green, the full run was impractical on the available machine, and the commands and results are recorded above.

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.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their 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: 8e509c8c-389b-4763-a0c9-f471eec47d3f

📥 Commits

Reviewing files that changed from the base of the PR and between 011f98c and f4d3f95.

📒 Files selected for processing (2)

• scripts/test-layout/layout.json
• tests/fixtures/test-layout-expected.json

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

---

📝 Walkthrough

Walkthrough

The change updates thinking-summary visibility in parseRequest and adds automatic reasoning summaries in concreteComboRequestBody. Tests cover active reasoning, disabled reasoning, explicit suppression, and injected default effort.

Changes

Reasoning Summary Defaults

Layer / File(s)	Summary
Parser summary visibility src/responses/parser.ts, tests/responses/reasoning-effort-summary-default.test.ts, scripts/test-layout/layout.json, tests/fixtures/test-layout-expected.json	At lines 545-546, parseRequest hides summaries for summaryMode === "none" or inactive reasoning when no summary mode is set. Active reasoning without a summary mode no longer hides the thinking summary. Tests and test-layout mappings cover these cases.
Combo reasoning defaults src/combos/request.ts	At lines 113-119, injected reasoning uses summary: "auto" when no summary exists and preserves an existing summary.

Priority: ⬇️ Low

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

Change: Bug fix

Merge Risk: 🔵 Low · up to f4d3f

Unsupported effort values can unexpectedly expose reasoning summaries, but the established impact is limited to this narrow malformed-input path.

🚥 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 2 functions across 3 files. (2 skipped: 2 …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: preserving visible reasoning when the summary mode is omitted. It is concise, specific, and consistent with the changes in src/responses/parser.ts and the …

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 2 functions across 3 files. (2 skipped: 2 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[github-actions]

✅ Deterministic PR hygiene checks passed.

[lidge-jun]

리뷰 · 우선순위 61 / 80

이 PR은 Responses 요청에서 reasoning.effort만 넣고 reasoning.summary를 빼먹었을 때, 생각 과정(reasoning)이 화면에서 사라져 버리던 버그를 고칩니다. 예전에는 summary가 없거나 "none"이면 무조건 hideThinkingSummary = true로 숨겼습니다. 그래서 Chat Completions 같은 클라이언트가 effort: "high"만 보내도 살아 있는 추론 텍스트가 hiddenRawReasoning으로 들어가 보이지 않았습니다. 지금은 summary가 명시적으로 "none"이거나, 활성 effort가 없을 때만 숨깁니다. combo 자식 요청에 기본 effort를 넣을 때도 summary: "auto"를 같이 넣어, 같은 함정을 한 번 더 막습니다. sse.ts는 손대지 않았고, 회귀 테스트 5개가 핵심 경우를 덮습니다. base는 dev라 맞습니다.

라인 - src/responses/parser.ts의 reasoningActive: REASONING_EFFORTS 소속 여부를 보지 않습니다. "banana"처럼 모르는 effort 문자열이 오면 options.reasoning은 안 잡히는데 hideThinkingSummary도 안 켜져, “reasoning 없음 + 생각 노출” 조합이 될 수 있습니다. "off"도 집합에 없는데 따로 걸러 두어, 허용 목록과 활성 판정이 어긋납니다.
라인 - src/combos/request.ts: 호출자가 이미 effort만 넣은 경우 early return이라 summary: "auto"는 주입되지 않습니다. 숨김 해제는 parser 쪽에만 의존합니다. 의도된 이중 안전장치라면 괜찮지만, 나중에 한쪽만 바꾸면 다시 어긋날 수 있습니다.
라인 - tests/responses/reasoning-effort-summary-default.test.ts: effort: "off", 알 수 없는 effort, “이미 effort만 있는 combo 본문” 경로는 없습니다. happy path와 summary: "none"은 잘 잡혀 있습니다.

메인테이너의 판단이 필요한 지점
summary를 생략한 채 활성 effort만 보내는 클라이언트를 “보여 주기”로 바꿀지, 예전처럼 “숨기기”로 둘지입니다. OpenAI Responses 스펙에 가깝게 가려면 이번 방향이 자연스럽지만, 숨김에 기대던 Chat 호환 클라이언트에는 보이는 추론이 늘어납니다. 또한 parser 수정만으로도 증상이 풀리므로, combo의 summary: "auto" 주입을 유지할지(업스트림/프로바이더 계약용) 단순화할지도 정하면 좋습니다.

너의 추천
머지해도 됩니다. 다만 reasoningActive를 REASONING_EFFORTS.has(requestedEffort) && requestedEffort !== "none"처럼 허용 목록과 맞춰 두면 더 안전합니다. combo summary: "auto"는 유지해도 해는 없고, 유지 이유를 한 줄 주석으로 남기면 이후 중복 이슈를 줄일 수 있습니다.

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

[github-actions]

✅ READY

• all PR quality gates passed; the review readiness checklist is complete.

Review readiness checklist

• ✅ Required local validation passed; commands, results, and any full-suite exception are documented.
• ✅ I pushed my PR to the latest dev commit.
• ✅ I resolved all correct Codex and CodeRabbit findings.
• ✅ My PR is ready for review.

✅ 4/4 boxes ticked.

This pull request has been marked Ready for Review.
The review-ready label marks this PR as ready; review automation runs independently.
Maintainers notified: @lidge-jun @Ingwannu

[lidge-jun]

Closing as superseded. The changes from this PR (head f4d3f955959f, by @Yum-wu) 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 omitted-summary default now applies only to a validated active reasoning effort, via hasValidatedActiveReasoningEffort. An explicit summary: "none" still hides the summary.

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.