Repository navigation
fix(codex): preserve GPT-5.6 reasoning contract - #7012
diegosouzapw merged 6 commits into
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Thanks for merge request |
|
sad to see this still not merged |
|
Really thorough validation write-up. This is orthogonal to the other Codex-reasoning PRs in this batch (#7176/#7095 touch response-side reasoning-summary synthesis; this is request-side effort mapping + catalog contract), so no reconciliation needed there — it merges independently. Confirmed locally: the executor/catalog regression tests reproduce the pre-fix |
Splitting the supported_endpoints/output_modalities guard clauses into excludesChatAndResponsesEndpoints() / excludesTextOutputModality() drops isUsableChatModel's cyclomatic complexity from 16 to under the ratchet's max of 15 (complexity-ratchets gate: 2057 -> 2056, back at baseline). Behavior is unchanged; existing vscode/codex route tests cover it. Co-authored-by: diegosouzapw <diegosouza.pw@gmail.com>
Babysit summaryDrove the CI reds down from 4 to a single root cause. 10/12 checks now green; the 2 remaining reds are the same assertion and need a maintainer decision (see below). Fixed
No test was weakened, no CI/baseline edited. All 76 codex/vscode tests pass locally. 🛑 Remaining red — needs a maintainer decision (not auto-fixable)Both This is not a broken test — it is this PR's intent colliding with an existing contract.
Verified this is caused by the PR, not a base-red: 6/6 pass on the Updating that assertion means ratifying the policy change ("live is source of truth" → "pinned local contract wins for GPT-5.6"), so I've deliberately left it to a maintainer rather than editing the guard myself.
|
…pinned wins) Resolve the cheaper-inference#7012 catalog-merge policy collision: instead of the pinned GPT-5.6 contract always winning for a fixed set of model ids, capacity limits (inputTokenLimit/outputTokenLimit) now merge via mergeCapacityLimitConservatively — Math.min(pinned, live) when both are present, so OmniRoute never promises more context than the account can actually serve. All other overlapping fields still take the live value unconditionally. Guard tests cover both directions (pinned smaller wins / pinned larger loses) at the route level and via an isolated helper-level unit test. Co-authored-by: diegosouzapw <diegosouza.pw@gmail.com>
e5b2404
into
cheaper-inference:release/v3.8.49
|
Merged into |
|
Please merge this as soon as possible. Thanks |
…g routes (#7587) (#8564) isUsableChatModel() was copy-pasted into 5 vscode listing routes. PR #7012 widened only models/route.ts to accept api_format "responses"/"openai-responses" alongside "chat-completions"; the other 4 copies (token+raw api/tags and api/show) still rejected anything that wasn't literally "chat-completions", silently dropping Codex-discovery-synced GPT models (apiFormat "responses") from the Ollama-compatible /api/tags endpoint VS Code's "Ollama" provider import flow actually calls. Extracted the predicate into a single shared module (vscode/[token]/usableChatModel.ts) imported by all 5 routes so this one-fixed-four-left-behind drift cannot recur. Co-authored-by: ikelvingo <im.kelvinwong@gmail.com>
* fix(codex): preserve GPT-5.6 reasoning contract * fix(vscode): expose Responses text models * fix(codex): keep GPT-5.6 limits through discovery * fix(ci): extract isUsableChatModel helpers to satisfy complexity ratchet Splitting the supported_endpoints/output_modalities guard clauses into excludesChatAndResponsesEndpoints() / excludesTextOutputModality() drops isUsableChatModel's cyclomatic complexity from 16 to under the ratchet's max of 15 (complexity-ratchets gate: 2057 -> 2056, back at baseline). Behavior is unchanged; existing vscode/codex route tests cover it. Co-authored-by: diegosouzapw <diegosouza.pw@gmail.com> * fix(codex): merge capacity limits conservatively (smaller of live vs pinned wins) Resolve the cheaper-inference#7012 catalog-merge policy collision: instead of the pinned GPT-5.6 contract always winning for a fixed set of model ids, capacity limits (inputTokenLimit/outputTokenLimit) now merge via mergeCapacityLimitConservatively — Math.min(pinned, live) when both are present, so OmniRoute never promises more context than the account can actually serve. All other overlapping fields still take the live value unconditionally. Guard tests cover both directions (pinned smaller wins / pinned larger loses) at the route level and via an isolated helper-level unit test. Co-authored-by: diegosouzapw <diegosouza.pw@gmail.com> --------- Co-authored-by: Xiangzhe <xz-dev@users.noreply.github.com> Co-authored-by: diegosouzapw <diegosouza.pw@gmail.com>
…g routes (cheaper-inference#7587) (cheaper-inference#8564) isUsableChatModel() was copy-pasted into 5 vscode listing routes. PR cheaper-inference#7012 widened only models/route.ts to accept api_format "responses"/"openai-responses" alongside "chat-completions"; the other 4 copies (token+raw api/tags and api/show) still rejected anything that wasn't literally "chat-completions", silently dropping Codex-discovery-synced GPT models (apiFormat "responses") from the Ollama-compatible /api/tags endpoint VS Code's "Ollama" provider import flow actually calls. Extracted the predicate into a single shared module (vscode/[token]/usableChatModel.ts) imported by all 5 routes so this one-fixed-four-left-behind drift cannot recur. Co-authored-by: ikelvingo <im.kelvinwong@gmail.com>
* fix(codex): preserve GPT-5.6 reasoning contract * fix(vscode): expose Responses text models * fix(codex): keep GPT-5.6 limits through discovery * fix(ci): extract isUsableChatModel helpers to satisfy complexity ratchet Splitting the supported_endpoints/output_modalities guard clauses into excludesChatAndResponsesEndpoints() / excludesTextOutputModality() drops isUsableChatModel's cyclomatic complexity from 16 to under the ratchet's max of 15 (complexity-ratchets gate: 2057 -> 2056, back at baseline). Behavior is unchanged; existing vscode/codex route tests cover it. Co-authored-by: diegosouzapw <diegosouza.pw@gmail.com> * fix(codex): merge capacity limits conservatively (smaller of live vs pinned wins) Resolve the cheaper-inference#7012 catalog-merge policy collision: instead of the pinned GPT-5.6 contract always winning for a fixed set of model ids, capacity limits (inputTokenLimit/outputTokenLimit) now merge via mergeCapacityLimitConservatively — Math.min(pinned, live) when both are present, so OmniRoute never promises more context than the account can actually serve. All other overlapping fields still take the live value unconditionally. Guard tests cover both directions (pinned smaller wins / pinned larger loses) at the route level and via an isolated helper-level unit test. Co-authored-by: diegosouzapw <diegosouza.pw@gmail.com> --------- Co-authored-by: Xiangzhe <xz-dev@users.noreply.github.com> Co-authored-by: diegosouzapw <diegosouza.pw@gmail.com>
…g routes (cheaper-inference#7587) (cheaper-inference#8564) isUsableChatModel() was copy-pasted into 5 vscode listing routes. PR cheaper-inference#7012 widened only models/route.ts to accept api_format "responses"/"openai-responses" alongside "chat-completions"; the other 4 copies (token+raw api/tags and api/show) still rejected anything that wasn't literally "chat-completions", silently dropping Codex-discovery-synced GPT models (apiFormat "responses") from the Ollama-compatible /api/tags endpoint VS Code's "Ollama" provider import flow actually calls. Extracted the predicate into a single shared module (vscode/[token]/usableChatModel.ts) imported by all 5 routes so this one-fixed-four-left-behind drift cannot recur. Co-authored-by: ikelvingo <im.kelvinwong@gmail.com>
Summary
maxreasoning effort for GPT-5.6 Sol, Terra, and Luna through the Chat Completions → Responses → Codex request path while retaining the legacymax → xhighcompatibility mapping for older models.lowand supports throughultra; Terra defaults tomediumand supports throughultra; Luna defaults tomediumand supports throughmax./responses, while retaining the existing non-text and specialty-model exclusions.First-party reference: OpenAI Codex
rust-v0.144.1model catalog.Related Issues
Validation
npm run lintnpm run test:unitnpm run test:coverage>= 60%for statements, lines, functions, and branchesCompleted locally:
272000versus pinned372000; after the fix its focused suite passed 28/28.npm run typecheck:core: passed after the Responses metadata and discovery follow-ups.npm run lint:json -- --max-warnings 0: passed locally with 0 errors and 0 warnings. The corresponding CI job exited 1 silently after its full-tree run with no reported diagnostic; a failed-job rerun was unavailable because the fork author lacks upstream repository admin permission.npm run check:file-size: passed.npm run check:dead-code: passed (225 vs baseline 227).npm run check:cognitive-complexity: passed (890 vs baseline 890).git diff --check: passed.npm run test:unit: 22,799 passed, 6 failed, 14 skipped. The same six failures reproduce on the unmodifiedorigin/release/v3.8.47tree: three platform-specificdoas -SDNS tests, two existing i18n integrity tests, and the existing Cloud Code Gemini request-default assertion.tests/unit/vscode-token-routes.test.tsreports its existing 65no-explicit-anyerrors on both this branch and the same unmodified release tree; changed source and other focused tests are clean.Tests Added Or Updated
tests/unit/executor-codex-gpt56.test.tstests/unit/codex-gpt56-catalog.test.tstests/unit/vscode-shared-metadata.test.tstests/unit/vscode-token-routes-gpt56.test.tstests/unit/vscode-token-routes.test.tstests/unit/vscode-responses-models.test.tstests/unit/provider-models-discovery-split.test.tsCoverage Notes
The cross-layer executor test starts with a Chat Completions request, passes it through
openaiToOpenAIResponsesRequest, then throughCodexExecutor.transformRequest. It catches the originalmax → xhighloss while retaining a GPT-5.5 compatibility assertion. Existing catalog and VS Code route tests cover the context, supported-effort, and default-effort metadata exposed to clients.The Responses metadata regression seeds a future Responses-only text model, asserts it survives both raw and grouped supplemental endpoints, and verifies the grouped import URL targets
/responses.Coverage was not run locally; CI should provide the authoritative coverage result.
Reviewer Notes
max → xhighbehavior.ultraremains a client/catalog preset for Sol and Terra;CodexExecutorintentionally sends upstream wire effortmax. Luna advertises noultrasupport./v1/models.