Skip to content

fix(responses): normalize explicit incomplete reasons - #7122

Closed
Bortlesboat wants to merge 4 commits into
lidge-jun:devfrom
Bortlesboat:fix/incomplete-reason-normalization
Closed

Bortlesboat wants to merge 4 commits into
lidge-jun:devfrom
Bortlesboat:fix/incomplete-reason-normalization

Conversation

@Bortlesboat

@Bortlesboat Bortlesboat commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Normalize known incomplete_details.reason aliases across every Responses delivery path. Output-limit aliases such as length become max_output_tokens; refusal/filter aliases become content_filter. Unknown reasons, canonical reasons, partial output, usage, messages and retryability retain their existing behavior.

The contributor's bridge SSE/JSON and direct Chat/Messages fix is retained. A follow-up covers native Responses SSE/JSON delivery, WebSocket response.incomplete and response.done frames (including native control chains), and the passthrough adapter's compaction parsers. Only response metadata is rewritten; nested user output stays opaque. No dependencies or provider configuration changed.

Closes #7082.

Verification

Bun 1.4.3, macOS; validation used the repository's isolated test runner. OCX_TEST_NO_QUEUE=1 intentionally permits this lane to coexist with other triage lanes, with one test worker.

  • bun run typecheck: passed.
  • OCX_TEST_NO_QUEUE=1 bun scripts/test.ts tests/responses/incomplete-reason-normalization.test.ts tests/adapters/bridge-nonstreaming-terminal.test.ts tests/responses/protocol-direct-encoders-chat.test.ts tests/responses/protocol-direct-encoders-messages.test.ts --parallel=1: 262 passed, 0 failed, 736 assertions. The new file adds 81 cases. Before the follow-up runtime fix, the initial 72-case matrix had 36 pass / 36 fail on the contributor head; all 36 missing-boundary regressions now pass.
  • OCX_TEST_NO_QUEUE=1 bun scripts/test.ts tests/responses/ws-upstream.test.ts tests/responses/sse-failed-tail.test.ts --parallel=1: 217 passed, 1 skipped, 0 failed, 1,463 assertions. The existing skip is a version-gated older-runtime case.
  • OCX_TEST_NO_QUEUE=1 bun scripts/test.ts tests/test-layout.test.ts tests/test-layout-tooling.test.ts tests/ci-workflows/file-size-ratchet.test.ts tests/ci-workflows/structure-ssot.test.ts tests/lab/core-lab-boundary.test.ts --parallel=1: 112 passed, 0 failed, 847 assertions.
  • bun run structure:check, bun run privacy:scan, git diff --check: passed.
  • In docs-site/, bun install --frozen-lockfile and bun run build: passed; 577 pages, 81,200 internal links checked.
  • OCX_TEST_NO_QUEUE=1 OCX_TEST_MAIN_TIMEOUT_MS=60000 bun run test:changed --parallel=1: intentionally bounded for the shared 16-core machine; exit 124, with 1,107 passing records across 61 started files and no failure records before termination. This is incomplete validation, not a passing changed suite.

Resource exception: several implementation lanes share this machine, so the full bun run test was not run. Focused behavior, transport and structural checks above provide local coverage; the remaining broad Linux coverage belongs to the exact-head Ubuntu PR CI. macOS/Windows workflows were not manually dispatched.

Historical Linux failures: the contributor reported 30,981 pass / 44 skip / 14 fail / 4 errors in a broad Linux run, with a bounded base comparison. The complete original failing-file logs were not attached, so this lane does not assert attribution for every historical failure. It independently ran OCX_TEST_NO_QUEUE=1 bun scripts/test.ts tests/cli/cli-access-audio-live.test.ts --parallel=1 in fresh isolated runs at this head and the exact unchanged base 7640a942b35044c974e534b84d5a8049f4a8ae9a: both had 33 pass / 3 fail / 3 errors, in the same parameterized native open and non-readiness events cannot claim readiness cases (5-second test timeouts). The test file is byte-identical between revisions. This confirms that failure family is inherited on this host; it does not claim the original Linux run was green.

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. No security-boundary code changed.

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

  • Required local validation passed; commands, results, and any full-suite exception are documented.

  • I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

Summary by CodeRabbit

  • Behavior Updates

    • Incomplete-response reasons are consistent across HTTP, streaming, and WebSocket responses. Known output-limit and refusal/filter aliases use standardized Responses labels; unknown reasons and other response details remain unchanged.
    • Direct Chat and Messages responses apply corresponding finish-reason mappings to both streamed and non-streamed responses.
  • Documentation

    • Updated reference and protocol documentation to describe incomplete-reason normalization.

@coderabbitai

coderabbitai Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

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: a1360e6a-58f5-4521-a4ca-684ffc39b632
📥 Commits

Reviewing files that changed from the base of the PR and between 4059cb9 and e94dec4.

📒 Files selected for processing: 3
  • docs-site/src/content/docs/reference/proxy-formats.md
  • scripts/test-layout/layout.json
  • tests/fixtures/test-layout-expected.json

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.

📝 Walkthrough

The change normalizes recognized incomplete-response reason aliases to canonical Responses values across adapter-backed and native HTTP, SSE, WebSocket, Chat, and Messages paths. Unknown reasons and other response data remain unchanged. Tests cover alias mapping and streaming and non-streaming outputs.

Changes: Incomplete Reason Normalization

Layer / File(s) Summary
Normalize adapter and direct encoder reasons
src/adapters/openai-responses/passthrough.ts, src/bridge/response-json.ts, src/bridge/sse.ts, src/protocols/encoders/adapter-events.ts, tests/adapters/bridge-nonstreaming-terminal.test.ts, tests/responses/protocol-direct-encoders-chat.test.ts, tests/responses/protocol-direct-encoders-messages.test.ts, docs-site/src/content/docs/reference/proxy-formats.md, structure/data-planes/protocol-paths.md, structure/providers-and-adapters.md
Adapter parsing and event encoders map recognized reasons through truncationReasonFor, preserving the original reason when no mapping exists. Tests check streamed and buffered projections, including Direct Chat and Messages outputs. Documentation describes the mappings and preservation of other response data.
Normalize native Responses payloads
src/responses/incomplete-reason.ts, src/server/responses/codex-ws-wire.ts, src/server/responses/passthrough-delivery.ts, tests/responses/incomplete-reason-normalization.test.ts, scripts/test-layout/layout.json, tests/fixtures/test-layout-expected.json, structure/transports/responses-wire-shapes.md, structure/transports/responses.md, structure/transports/streaming-health.md
Payload and JSON helpers normalize recognized reasons at the payload root and under response. Native JSON, SSE, and WebSocket delivery use the normalization. Tests cover aliases, preservation of other data, and unchanged or malformed JSON.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant Upstream
  participant ResponsesDelivery
  participant IncompleteReasonNormalizer
  participant Client
  Upstream->>ResponsesDelivery: JSON, SSE, or WebSocket payload
  ResponsesDelivery->>IncompleteReasonNormalizer: Payload with incomplete_details.reason
  IncompleteReasonNormalizer-->>ResponsesDelivery: Payload with recognized reason normalized
  ResponsesDelivery->>Client: Rewritten payload
Loading
Merge Risk: ⚪ Minimal · up to e94de

Recognized incomplete reasons are documented as canonicalized, and the test-layout changes agree. No concrete merge-blocking behavior is identified in the reviewed changes.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 63.64% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 11 files. (3 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 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 main change: normalizing explicit incomplete reasons in Responses delivery paths.
Linked Issues check Passed PR #7122 meets the coding requirements in directly linked issue [#7082]. src/responses/incomplete-reason.ts normalizes only incomplete_details.reason at the response root and under response, and…
Out of Scope Changes check Passed The changes stay within [#7082]. Production changes centralize incomplete-reason normalization and apply it at the Responses adapter, native JSON/SSE, WebSocket, bridge, and direct encoder boundaries.…
Full details: Docstring Coverage

Explanation

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Oct 15, 2026
@github-actions

github-actions Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

⏳ 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

  • ⬜ Required local validation passed; commands, results, and any full-suite exception are documented.
  • ⬜ I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
  • ⬜ 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
lidge-jun marked this pull request as ready for review October 15, 2026 07:25
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-15T07:35:27.508621Z 4059cb9 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

…normalization

# Conflicts:
#	scripts/test-layout/layout.json
#	tests/fixtures/test-layout-expected.json
@github-actions
github-actions Bot marked this pull request as draft October 15, 2026 14:47
…normalization

# Conflicts:
#	scripts/test-layout/layout.json
@lidge-jun

Copy link
Copy Markdown
Owner

Being carried into #7218 with your authorship preserved; this PR will be closed when the bundle lands.

lidge-jun added a commit that referenced this pull request Oct 16, 2026
…continuation tests and MiMo envelope repair (#7122, #7152, #7068, #7214) (#7218)

Carried from #7122, #7152, #7068, #7214 (one commit per source PR, original authors preserved). #7095 held out.

Remote exact-head validation (hosted GitHub Actions stalled; local tests not run by maintainer rule): union candidate codex/prr-integ-6 (368006f, tree 27fdc37) on mini (Windows) and lidge (Linux), typecheck + full suite, compared with base-dev 4ae4936 on each host.

Co-authored-by: Bortlesboat <169967362+Bortlesboat@users.noreply.github.com>
Co-authored-by: AK <info@atelier-sai.jp>
Co-authored-by: Aditya Rawat <adirwt852@gmail.com>
Co-authored-by: shirruy <shirruy@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
@lidge-jun

Copy link
Copy Markdown
Owner

Landed on dev in 3785915 via #7218 (your change is its own commit in that bundle, and the squash keeps your authorship as Co-authored-by). Thank you!

@lidge-jun lidge-jun closed this Oct 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants