Skip to content

fix(codex): clamp native efforts to selected model ladder - #7731

Merged
lidge-jun merged 2 commits into
lidge-jun:devfrom
Plamyth:codex/runtime-compat-20261026
Oct 26, 2026
Merged

lidge-jun merged 2 commits into
lidge-jun:devfrom
Plamyth:codex/runtime-compat-20261026

Conversation

@Plamyth

@Plamyth Plamyth commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Normalize a requested native OpenAI reasoning effort against the selected model's published ladder before sending a Responses request. For gpt-6.1-sol, unsupported none and minimal become low.
  • Apply the same final-route check to direct requests, compaction model switches, and combo failover. Keep routed provider behavior and the existing ultra wire conversion.
  • Add a failing-before regression commit and outbound-body checks; update the configuration and transport contracts.

Verification

  • Before the fix, bun test tests/codex-integration/codex-catalog-ladders.test.ts --test-name-pattern 'native model rejects none' failed: expected low, received null.
  • After the fix, bun test tests/codex-integration/codex-catalog-ladders.test.ts tests/responses/responses-compaction-override.test.ts: 88 passed, 0 failed. The outbound-body checks cover direct, compaction-switch, and combo-failover sends.
  • bunx tsc --noEmit --pretty false, bun scripts/privacy-scan.ts, and bun scripts/structure-ssot.ts passed. An independent read-only review found no concrete defect.
  • bun scripts/test.ts --changed=dev selected 1,253 of 2,615 test files and reached its built-in 900-second limit (exit 124). The full selected suite is not claimed as passing; CI must complete the remaining coverage.
  • A standalone candidate built from this head and reported opencodex 2.93.0. The shared proxy was not replaced while other requests were active.

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:

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

@coderabbitai

coderabbitai Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 8ebe46b and 283f2d0.

Files selected for processing: 6
  • docs-site/src/content/docs/reference/configuration/agents.md
  • src/codex/catalog/effort.ts
  • structure/catalog.md
  • structure/transports/responses-failover.md
  • tests/codex-integration/codex-catalog-ladders.test.ts
  • tests/responses/responses-compaction-override.test.ts

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

Walkthrough

The native effort clamp now maps unsupported reasoning effort values to supported levels on the selected model’s ladder. Tests and documentation cover direct requests, compaction routing, and combo failover.

Changes: Native effort normalization

Layer / File(s) Summary
Clamp unsupported effort values
src/codex/catalog/effort.ts:56-69, 80-84, tests/codex-integration/codex-catalog-ladders.test.ts:218-221
The clamp recognizes none and minimal, converts ultra to max, and selects the highest supported effort at or below the request. If no supported effort is at or below the request, it selects the lowest supported effort. The test checks that none maps to low for gpt-6.1-sol.
Cover final-route normalization
tests/responses/responses-compaction-override.test.ts:296-347, docs-site/src/content/docs/reference/configuration/agents.md:387-389, structure/catalog.md:484, structure/transports/responses-failover.md:287-290
Tests cover direct and compaction-routed requests, plus combo failover to gpt-6.1-sol. The documentation describes applying the selected native model’s effort ladder before sending the request.

Priority: Normal

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

Change: Bug fix

Possibly related issues

  • Issue 7590 reports invalid native reasoning effort values for gpt-6.1-sol, including ultra. This PR changes the same native effort normalization path, while preserving the existing ultra conversion.

Possibly related PRs

  • lidge-jun/opencodex#7600: Both changes modify native reasoning-effort normalization in src/codex/catalog/effort.ts. This PR extends the clamp behavior while preserving the ultra to max conversion.
  • lidge-jun/opencodex#7605: Both describe the native-versus-routed wire contract. This PR adds effort clamping against the selected native model’s ladder.
Merge Risk: Minimal · up to 283f2

No actionable issue is established for the effort-normalization change. Complete the remaining CI checks before merging.

Pre-merge checks: Passed 4 · Failed 1

Failed 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. (3 skipped: 3 … 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 primary change: clamping native reasoning efforts against the selected model's ladder.
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.
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. (3 skipped: 3 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

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 25, 2026
@github-actions

github-actions Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

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

✅ 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

@Plamyth
Plamyth marked this pull request as ready for review October 25, 2026 20:48
@github-actions
github-actions Bot marked this pull request as draft October 25, 2026 20:48
@github-actions
github-actions Bot marked this pull request as ready for review October 25, 2026 21:08
@lidge-jun

Copy link
Copy Markdown
Owner

Maintainer dev integration record (release 2.93 dev-merge loop), per the MAINTAINERS.md maintainer-integration exception for dev. This is maintainer integration, not an approving review.

@lidge-jun
lidge-jun merged commit c2f3d8b into lidge-jun:dev Oct 26, 2026
47 checks passed
Transient-Onlooker added a commit to Transient-Onlooker/opencodex that referenced this pull request Oct 26, 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 review-ready

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants