Skip to content

fix(routing): connection defaultModel only replaces a placeholder bare model - #16550

Merged
diegosouzapw merged 2 commits into
cheaper-inference:release/v3.8.52from
shipsfromrio:fix/bare-model-default-placeholder-v3852
Oct 16, 2026
Merged

diegosouzapw merged 2 commits into
cheaper-inference:release/v3.8.52from
shipsfromrio:fix/bare-model-default-placeholder-v3852

Conversation

@shipsfromrio

Copy link
Copy Markdown
Contributor

⚠️ base-red inherited: #15306

Follow-up to #4825, the PR that added resolveBareModelToConnectionDefault. No open issue or PR
covers this; see Notes.

Problem

resolveBareModelToConnectionDefault (open-sse/services/model.ts) exists so that a bare request
(no /, such as an alias that resolved to auto) reaches the upstream with the connection's
defaultModel instead of the placeholder. On e2d5747872 it replaces any bare name: as soon as
the selected connection has a defaultModel, the model the client asked for is dropped and the body
is rewritten to ${provider}/${defaultModel} (src/sse/handlers/chat.ts, the effectiveModel /
requestBody pair right after credential selection). Nothing fails: the request is answered with
200 by another model.

The base's own tests pin that behaviour:

So a concrete bare catalog name (claude-haiku-4-5), or a bare alias that already resolved to a
concrete id, is served by whatever the connection default is, across model families included
(claude-haiku-4-5 with a gemini-2.5-flash default, in the helper test). The embeddings path
(src/lib/embeddings/service.ts) calls the same helper.

Fix

  • The connection defaultModel now replaces the resolved model only when that model is a
    placeholder: null, empty, or auto. That is the case fix(sse): resolve bare model names to connection defaultModel before upstream calls #4825 was written for, and it is
    unchanged.
  • A concrete bare name, and a bare alias resolved to a concrete id, are kept. A resolved model
    equal to the default is returned as-is.
  • /-qualified names were already left untouched and still are. A connection without a usable
    defaultModel still returns the resolved model.
  • BARE_MODEL_DEFAULT_OVERRIDE_LEGACY=true restores the previous replacement. It is read on every
    call (1, true, yes, on, case-insensitive, the same reader as
    OMNIROUTE_PREFER_CLAUDE_CODE_FOR_UNPREFIXED_CLAUDE_MODELS in the same file) and logs one
    warning per (model, default) pair through the MODEL logger. Documented in .env.example and
    docs/reference/ENVIRONMENT.md, with a fragment in changelog.d/fixes/.

open-sse/services/model.ts goes 852 → 884 lines by the check:file-size count
(split("\n").length), and its merge with the release tip 9ecddd15a1 measures 893 (#16493 adds
9 lines elsewhere in the file). The cap for files outside file-size-baseline.json is 1200, and
none of the four touched .ts files is in the baseline. The three test files measure 92, 295 and
591 against a testCap of 1200. The merge with 9ecddd15a1 is clean (git merge-tree): #16493
(model.ts) and #16510 (.env.example, ENVIRONMENT.md) touch other hunks of the same files.

Tests

Run on GitHub Actions (ubuntu-latest, Node 24 from .nvmrc), the three files below together,
twice: once with the branch (57 tests, 57 pass), and once with .env.example and
open-sse/services/model.ts restored to e2d5747872 while the test files stay (57 tests, 53 pass,
4 fail).

File (tests/unit/) Case Base e2d5747872 This PR
resolve-bare-model-to-connection-default-474 concrete bare model name is NOT replaced by the connection defaultModel (claude-haiku-4-5 with a gemini-2.5-flash default; gpt-4o with gpt-4o-mini) ✖ ✔
resolve-bare-model-to-connection-default-474 bare alias that resolved to a concrete model keeps the resolved model (haiku → claude-haiku-4-5-20251001 over gemini-2.5-flash) ✖ ✔
hidden-model-dispatch a connection default does not replace a visible concrete bare model (gpt-4o dispatched, 200; still 200 and gpt-4o after gpt-4o-mini is hidden) ✖ ✔
suffix-effort-propagation a bare alias keeps its concrete target and suffix effort over a connection default (agnes-3.0-flash sent, reasoning_effort: "max") ✖ ✔
resolve-bare-model-to-connection-default-474 bare model name resolves to the connection defaultModel (auto → gpt-4o-mini) ✔ ✔
resolve-bare-model-to-connection-default-474 empty or null resolved model still falls back to the connection defaultModel ✔ ✔
resolve-bare-model-to-connection-default-474 defaultModel equal to the resolved model is returned as-is ✔ ✔
resolve-bare-model-to-connection-default-474 BARE_MODEL_DEFAULT_OVERRIDE_LEGACY=true restores the old replacement ✔ ✔
hidden-model-dispatch a connection default cannot replace a visible request with a hidden model (the #16110 case, now under the override: gpt-4o-mini dispatched, then 404 once hidden) ✔ ✔
suffix-effort-propagation connection default replacement does not inherit a bare alias suffix (the #14260 case, now under the override: agnes-default-replacement sent, no reasoning_effort) ✔ ✔

The last six pass on both sides on purpose. The first three of them pin the placeholder behaviour
#4825 added; the three override cases pass on the base because the base replaces whatever the
variable says. The remaining 47 cases in the three files pass on both sides.

The three tests that set the override save the previous value, set true, and restore (or delete)
it in a finally, with the exercised call inside the try.

Notes

  • fix(routing): enforce individual hidden-model policy before dispatch #16110 (merged 2026-10-11, in the base) added the pre-dispatch hidden-model gate and
    tests/unit/hidden-model-dispatch.test.ts. Its case "a connection default cannot replace a
    visible request with a hidden model" used the replacement of a concrete gpt-4o as its scenario,
    so with this fix the gate on that swap would no longer be exercised. The second commit keeps the
    original assertions under BARE_MODEL_DEFAULT_OVERRIDE_LEGACY=true and adds the default-path
    case. tests/unit/suffix-effort-propagation.test.ts (fix(providers): stop treating Anthropic max_tokens as the context window #14260) gets the same treatment: the
    original "replacement drops the suffix" assertions run under the override, and the default path
    asserts the alias target and its effort.
  • Kill switch. The override is environment-only; there is no dashboard toggle, unlike
    OMNIROUTE_PREFER_CLAUDE_CODE_FOR_UNPREFIXED_CLAUDE_MODELS. The per-pair warning set lives for
    the process.
  • Not in this PR:
    • The embeddings path changes with the helper; no embeddings case is added here.
    • The call-site comments in src/sse/handlers/chat.ts and src/lib/embeddings/service.ts keep
      their original wording; the helper's own docblock carries the new rule.
    • The placeholder match is exact and case-sensitive (auto), as the rest of the helper is.
    • The dashboard hints for the connection's default model field do not describe the replacement,
      so no UI copy changes.
    • Searched upstream for open PRs and issues on resolveBareModelToConnectionDefault,
      defaultModel, bare model and connection default: none covers this. fix(sse): resolve bare model names to connection defaultModel before upstream calls #4825 is the merged
      PR that added the helper. Of the 100 most recently updated open PRs, one touches
      open-sse/services/model.ts, perf(sse): bound parseModel and canonical-provider caches #16341; its hunks there are the imports, ResolvedModelTarget,
      shouldTreatAsExactModelId and parseModel, none of them the helper.
  • Changelog fragment: changelog.d/fixes/bare-model-default-only-placeholder.md.

…e model

A concrete bare model name such as claude-haiku-4-5 was swapped for the
selected connection's defaultModel by resolveBareModelToConnectionDefault,
so a Claude request could be served by a Gemini default with no error. The
defaultModel now only replaces auto or an empty model.
BARE_MODEL_DEFAULT_OVERRIDE_LEGACY=true restores the old behaviour and logs
one warning per pair through the level-gated MODEL logger.
With the connection defaultModel limited to placeholders, a concrete bare
model such as gpt-4o is no longer replaced by the default. Two route tests
used that replacement only as their scenario:

- hidden-model-dispatch: the default path now keeps gpt-4o (200, and still
  200 after gpt-4o-mini is hidden); the original swap and 404 assertions run
  with BARE_MODEL_DEFAULT_OVERRIDE_LEGACY=true, so the hidden-model gate on
  that swap stays covered.
- suffix-effort-propagation: the default path keeps the bare alias target
  and its max effort; the replacement without the suffix runs with the
  legacy override on.

The override is read on every call and restored in a finally block.
@shipsfromrio

Copy link
Copy Markdown
Contributor Author

CI on 384e7b78c6, tested as the merge with 9ecddd15a1 (CI 38463185630, Quality Gates 38463185637):

  • The three test files this PR changes pass in full, each in one CI unit shard and one fast-path shard: resolve-bare-model-to-connection-default-474 11/11 (Unit Tests (5/8), fast-path (1/4)), hidden-model-dispatch 18/18 ((2/8), fast-path (2/4)) and suffix-effort-propagation 28/28 ((3/8), fast-path (3/4)). That includes the 7 new cases and the 2 rewritten to run under BARE_MODEL_DEFAULT_OVERRIDE_LEGACY=true.
  • Unit Tests (2/8) to (8/8) fail on the same 13 tests, in the same ten files and with the same assertion messages, as on 9ecddd15a1's push run (CI 38444167795) (below). (5/8) and (6/8) run out of heap after the test summary on both sides, and (6/8) has no failing test on either side. (1/8) passes on both.
  • Fast-path (1/4) to (3/4) fail on the same 8 tests as on the base push run (Quality Gates 38444167890). Fast-path (4/4) was cancelled without a summary. Its output stops at 00:17:27 UTC, and the job hit its 30-minute limit at 00:39:40. On the base, that shard finishes in 9 minutes and fails codex-provider-overlay-12782, combo-dispatch-prelude and messages-count-tokens-route. Here those three fail in Unit Tests (4/8) and (8/8), as they do in the base's unit shards, and the tests that follow the stall point on the base pass in those two shards.
  • Integration Tests (1/2): video-raw-transcript-retention-12430 fails the same 3 tests as on the base. resilience-http-e2e fails only here. Its before hook warm-up (POST with p2/test-model) got a 502 fetch failed (cause: UND_ERR_INVALID_ARG: invalid onRequestStart method), so 5 tests fail and 3 are skipped. That model string is /-qualified, and resolveBareModelToConnectionDefault returns it unchanged before reaching any line this PR changes. On the base push run, the same undici error fails fingerprint-expansion in Integration Tests (2/2). That shard passes here: 414 pass, 0 fail, 48 skipped.
  • Vitest (MCP / autoCombo / UI components): the 3 failures are all in tests/unit/ui/LocaleAutoDetect-refresh.test.tsx. The run also logs 4 unhandled ReferenceError: window is not defined: two from a FlowCanvas.tsx timer during tests/unit/ui/flowCanvas.test.tsx, and two from the React scheduler during providerCardAudioBadge.test.tsx. The base run fails the same 3 tests plus one in tierResolver.test.ts, and logs no unhandled errors. This PR touches no UI file. Vitest (fast-path) passes on both sides.
  • Lint fails on both sides at check:file-size (absolute mode): open-sse/services/combo/executeTargetAttempt.ts is at 1288 lines against a frozen 1284. This PR does not touch that file.
  • Docs Sync (Strict): check-translation-drift lists the same 5 drifted sources on both sides. One is docs/reference/ENVIRONMENT.md, which this PR edits, and it is already flagged on the base.
  • Fast Quality Gates fails mutation-test-coverage --strict and check:dashboard-typecheck:
    • The mutation report is identical to the base push run's (12 unit tests across 5 modules). None of the 12 is a file this PR touches, and open-sse/services/model.ts is not one of the 5 modules.
    • Dashboard typecheck prints the same report on both sides: 196 errors, 86 baselined ones gone, and one new TS2339 in src/app/(dashboard)/dashboard/providers/[id]/components/ConnectionRow.tsx.
    • file-size and complexity-ratchets pass here. In new-code mode, complexity has 3 violations against 3 on the base and cognitive complexity 2 against 2. The base push run also fails file-size in absolute mode, on executeTargetAttempt.ts.
  • Merge integrity: changelog integrity passes, and check:agent-skills-sync fails on both sides with the same 20 generated skills out of date.
  • Build lost its runner 5.5 minutes into next build ("The runner has received a shutdown signal", exit 143). On the base push run, Build sat in the queue for about 11 hours (21:36 UTC on 13 Oct to 08:33 UTC on 14 Oct), then started on a self-hosted runner and was in progress when checked.
  • Gate / CI and Gate / Quality only aggregate the jobs above.

No failing job traces to this change.

The 13 failing unit tests (ten files), the same on both sides
  • codex-provider-overlay-12782 (2)
  • combo-dispatch-prelude (2)
  • combo-scoring-inspector (2)
  • messages-count-tokens-route (1)
  • model-hidden-dashboard-modality-read (1)
  • model-test-route (1)
  • oauth-providers-config (1)
  • quality-rail-gate-membership (1)
  • route-explainability (1)
  • server-owned-tool-loop-flag (1)

All are under tests/unit/.

@diegosouzapw
diegosouzapw merged commit 54afdc3 into cheaper-inference:release/v3.8.52 Oct 16, 2026
31 of 51 checks passed
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