Skip to content

fix(sse): restores #5887 openai precedence for bare gpt-5.5 routing - #9440

Closed
wgordon17 wants to merge 4 commits into
diegosouzapw:release/v3.8.50from
wgordon17:fix/codex-gpt55-openai-precedence-5887
Closed

wgordon17 wants to merge 4 commits into
diegosouzapw:release/v3.8.50from
wgordon17:fix/codex-gpt55-openai-precedence-5887

Conversation

@wgordon17

Copy link
Copy Markdown
Contributor

Summary

wgordon17 and others added 3 commits August 4, 2026 11:03
….5 routing

diegosouzapw#9275 added gpt-5.5 (+ effort variants) to CODEX_NATIVE_UNPREFIXED_MODELS,
an unconditional early-return check at the top of
resolveModelByProviderInference(). This silently made unreachable the
codex-vs-openai precedence logic a few lines below it, still present and
still correctly commented, just dead code for gpt-5.5, that issue diegosouzapw#5887
built specifically for this model: codex-only installs route to codex,
but when openai is also active, the historical openai default wins
(tests/unit/codex-gpt55-routing-5887.test.ts).

Verified this was a genuine regression, not a deliberate override:
- Ran the diegosouzapw#5887(b) test against the commit immediately before diegosouzapw#9275
  merged, and it passed. diegosouzapw#9275's own CI run shows it failing, and diegosouzapw#9275
  merged anyway.
- agentrouter's static registry does not catalog gpt-5.5 at all (only
  gpt-5.6-sol), so the inference-race bug diegosouzapw#9275 exists to prevent
  cannot occur for gpt-5.5. There was no technical reason to add it to
  the same unconditional set as the gpt-5.6-sol tier.
- tests/unit/codex-synced-bare-model-routing.test.ts independently
  encodes the same openai-wins-when-both-active contract for gpt-5.5
  in two more tests, both broken by the same commit.

Fix: remove only gpt-5.5 (+ variants) from CODEX_NATIVE_UNPREFIXED_MODELS,
restoring its resolution through the existing, unmodified, already-tested
precedence logic. gpt-5.6-sol and the other models diegosouzapw#9275 actually needed
to fix are untouched.

Also fixes two pre-existing, unrelated vscode-token-routes.test.ts
failures (a diegosouzapw#9275 error-message format change, trailing period, that
predates this branch and would fail regardless) and rebaselines two
inherited, pre-existing file-size drifts (base.ts, chat.ts) from an
unrelated already-merged commit (7163081), so check:file-size passes
cleanly on this branch.
Previous commit fixed the regression by simply omitting gpt-5.5 from
CODEX_NATIVE_UNPREFIXED_MODELS, relying on fallthrough to the
pre-existing precedence logic below. That's structurally the same
failure mode that caused the original bug: a check "just happens" not
to apply to a given model id, with correctness depending on nobody
re-adding it later without reading a comment 20 lines away from the
call site (exactly what happened once already, in diegosouzapw#9275).

Adds a second, explicitly-named set,
CODEX_NATIVE_MODELS_WITH_OPENAI_PRECEDENCE, and checks it directly at
the call site: `CODEX_NATIVE_UNPREFIXED_MODELS.has(modelId) &&
!CODEX_NATIVE_MODELS_WITH_OPENAI_PRECEDENCE.has(modelId)`. A future
engineer reading this exact line sees the carve-out and why it exists,
instead of needing to notice gpt-5.5's absence from a list defined
well above it. Test updated to assert membership in the new set
directly, rather than only asserting absence from the old one.
…gpt-5.5 contract (owner decision)

Keeps this PR's surviving value (the diegosouzapw#9275 comment fixes + the documented
diegosouzapw#5887×diegosouzapw#9447 interplay at the preemption site + own-growth baseline entry) and
drops the behavioral carve-out: the base's diegosouzapw#9447 active-connection bound already
delivers the agreed contract (OpenAI while codex is inactive, active codex
preempts, openai/ prefix overrides), encoded in codex-gpt55-routing-5887.test.ts.
Inherited file-size bumps reverted (release captain domain; diegosouzapw#9355 base-relative
mode protects innocent PRs).
@diegosouzapw

Copy link
Copy Markdown
Owner

Maintainer update: merged release/v3.8.50 into this branch and realigned it to the routing contract that shipped in the meantime via #9447 (owner-confirmed today): for bare gpt-5.5, OpenAI serves while no codex connection is active, an active codex connection preempts (the ChatGPT subscription is the quota source of truth), and an explicit openai/… prefix stays the per-request override — see tests/unit/codex-gpt55-routing-5887.test.ts.

What survives from this PR (thanks for these!): the #9275 trailing-period comment fixes in vscode-token-routes.test.ts, the documented #5887×#9447 interplay at the preemption site in model.ts, and your own-growth file-size baseline entry. The behavioral carve-out (removing gpt-5.5 from CODEX_NATIVE_UNPREFIXED_MODELS) was dropped since the shipped bound already covers the scenario it targeted. All 54 tests across the touched files pass. CI should go green on the next run.

@wgordon17

Copy link
Copy Markdown
Contributor Author

Superseded by #9447, which already shipped the maintainers' own resolution to the #5887/#9275 conflict: keep codex winning when both providers are active (matching #9275's intent), gated on codex actually being configured, with an explicit openai/ prefix as the documented escape hatch (see tests/unit/codex-gpt55-routing-5887.test.ts). This PR's remaining diff (the file-size-baseline rebaseline and the vscode-token-routes trailing-period fix) is also redundant now — the file-size fix is superseded by quality.yml's check:file-size -- --base-ref PR-mode change, and the test-string fix already landed upstream independently. Closing with nothing left to merge.

@wgordon17 wgordon17 closed this Aug 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants