Skip to content

fix(api): stop reading max_tokens as a discovered context window - #14320

Closed
sxh313 wants to merge 3 commits into
diegosouzapw:release/v3.8.51from
sxh313:fix/discovery-max-tokens-not-context-window
Closed

sxh313 wants to merge 3 commits into
diegosouzapw:release/v3.8.51from
sxh313:fix/discovery-max-tokens-not-context-window

Conversation

@sxh313

@sxh313 sxh313 commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Summary

normalizeDiscoveredModels() no longer reads max_tokens as a context window
(src/lib/providerModels/modelDiscovery.ts), and a regression test pins both
directions of the candidate list. Closes #14318.

Motivation

max_tokens on an OpenAI-compatible /v1/models record is the model's maximum
output length - the same meaning as the max_tokens request parameter - not
the window. It was added to the contextWindow candidate list by #14159 (the
re-land of #12630) without a test pinning it, so a record that carries only
max_tokens now syncs as:

{ "id": "local-llm", "max_tokens": 4096 }  ->  inputTokenLimit: 4096

Consequences for a local/LM-Studio-style endpoint that reports it that way:

  • the catalog advertises a 4K window for what may be a 128K model;
  • the combo combo-context-window-filter excludes that model for any prompt
    above the output cap, so it silently stops being routable;
  • the compression / context-relay heuristics that read inputTokenLimit size
    the prompt against an output limit.

The real window fields already in the list (context_length, contextWindow,
max_model_len per #12858, max_context_window, top_provider.context_length)
cover every provider that legitimately reports a window; removing max_tokens
does not leave any of them without one.

What changed

  • src/lib/providerModels/modelDiscovery.ts: dropped record.max_tokens from
    the contextWindow candidates and left a comment saying why it must not be
    re-added, because the adjacent max_context_window IS a window field and the
    pair is easy to confuse. If a provider turns out to report its window as
    max_tokens, it belongs in per-provider with a test naming that provider.
  • tests/unit/discovery-max-tokens-context-window-14318.test.ts (new): the
    regression guard the issue asked for.

What is broken without it, and how it works after

Before: inputTokenLimit = 4096 for a max_tokens-only record (assertion output
below). After: no inputTokenLimit is invented from an output cap, and a record
that reports both context_length: 131072 and max_tokens: 4096 still keeps
131072 - the ordering already handled that case, and the test locks it so a
future candidate reshuffle cannot break it either way.

Commands run

# focused regression test - red before the fix
node --import tsx/esm --test tests/unit/discovery-max-tokens-context-window-14318.test.ts
#   AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
#   + actual   4096
#   - expected undefined
#   at tests/unit/discovery-max-tokens-context-window-14318.test.ts:16:10

# same file after the fix
node --import tsx/esm --test tests/unit/discovery-max-tokens-context-window-14318.test.ts
#   tests 2  pass 2  fail 0

# every test file that exercises model discovery (17 files, found with
# `grep -rl "normalizeDiscoveredModels|persistDiscoveredModels" tests/unit`)
node --import tsx/esm --test <those 17 files>
#   tests 150  pass 148  fail 0  skipped 2
# the 2 skips are the pre-existing "live Lemonade" tests in
# tests/unit/model-embedding-discovery-and-cache.test.ts that self-skip when the
# operator's LAN embedding server is unreachable; they were skipped the same way
# before this change.

# the affected catalog is only touched by my new test among those 17:
grep -l "max_tokens" <those 17 files>
#   tests/unit/discovery-max-tokens-context-window-14318.test.ts

npx --no-install eslint src/lib/providerModels/modelDiscovery.ts \
    tests/unit/discovery-max-tokens-context-window-14318.test.ts   # clean, exit 0
npx --no-install prettier --check src/lib/providerModels/modelDiscovery.ts \
    tests/unit/discovery-max-tokens-context-window-14318.test.ts
#   All matched files use Prettier code style!
git commit   # pre-commit gates ran and passed: lint-staged, check-docs-sync,
             # [t11:any-budget] PASS, [tracked-artifacts] OK

Every figure above was re-verified on the branch as it now stands: the discovery family is still 150 tests / 148 pass / 2 skipped at the merged head 5a19c3b0a, so the base refresh changed no outcome. Base branch: release/v3.8.51 (the highest active release/v*, and the repo default), per Golden Path step 1. Cut at 7a921299c; refreshed onto 34113170f by merging the base in (merge commit 5a19c3b0a) after the branch fell 2 commits behind, with the two SSE commits that arrived touching nothing on this path. Re-verified on the merged tree: discovery-max-tokens-context-window-14318 2/2, and provider-models-route + model-token-limit-catalog + openrouter-context-length-3202 69/69.

Migrations, feature flags, generated artifacts

A changelog fragment was added to this branch after I re-read CONTRIBUTING.md line 409: user-facing changes ship a file under the changelog directory rather than editing CHANGELOG.md, so this PR now carries one. It is the only addition beyond the fix and its test.

None. No schema, catalog file, generated reference, or feature flag is touched;
docs/reference/PROVIDER_REFERENCE.md is generated from the provider registry,
not from discovery output, so no regenerated diff is expected.

CI-only validation still pending

  • npm run typecheck:core, npm run lint (whole repo), npm run test:unit,
    npm run test:vitest and the coverage ratchet: run by CI on this PR. Locally I
    ran the focused equivalents listed above; the change only removes one argument
    from a variadic firstPositiveNumber() call and adds a test file, so no
    type surface changes.
  • Full npm run test:unit was not run locally (the suite is ~10k tests); the
    150-test discovery subset above is the focused gate for this module.

base-red note

⚠️ base-red inherited: #13866 ("Release branch not green: release/v3.8.51") is
open against this base. Any failure in CI that also reproduces on the base tip
7a921299c is inherited, not introduced here - my focused suite is green on top
of that tip.

`max_tokens` on an OpenAI-compatible /v1/models record is the model's maximum
output length, the same meaning as the request parameter -- not the context
window. Since diegosouzapw#14159 it sat in the contextWindow candidate list, so a record
carrying only `max_tokens` synced with `inputTokenLimit` set to that output cap:
the catalog advertised a 4K window for a 128K model, the combo context-window
filter excluded it for any prompt above the cap, and the compression /
context-relay heuristics sized the prompt against an output limit.

Drop it from the candidates and leave a comment saying why, since the sibling
`max_context_window` is a real window field. A provider that genuinely reports
its window that way needs a per-provider entry with a test naming it.

Regression guard: a record with only `max_tokens` yields no `inputTokenLimit`;
a record with `context_length` + `max_tokens` keeps `context_length`.

Closes diegosouzapw#14318
@sxh313

sxh313 commented Sep 22, 2026

Copy link
Copy Markdown
Contributor Author

Closing this as superseded — the base already contains the fix, and I would rather you not spend a review on two guards for one hole.

I merged origin/release/v3.8.51 (373c31f3d) into this branch to see how bad the conflict was. It is exactly one file, src/lib/providerModels/modelDiscovery.ts, two hunks, both in the same place, and resolving them means deleting my change:

max_tokens is no longer in the window candidate list on the base at all:

const contextWindow = firstPositiveNumber(
  record.context_length,
  record.contextLength,
  record.contextWindow,
  record.max_model_len,
  record.maxModelLen,
  record.max_context_window,
  record.max_input_tokens,
  record.maxInputTokens,
  topProvider.context_length
);

and the base comment where mine was now reads "Anthropic Models API reports the window as max_input_tokens and the output cap as max_tokens. #14159 briefly treated max_tokens as a window candidate; that mapped Claude Opus 5 to 128K instead of 1M. Do not put max_tokens here." — the same rule #14318 asked for.

tests/unit/anthropic-discovery-window-max-tokens.test.ts on the base already pins the behavior my test asserted, including the identical case:

test("a record with only max_tokens does not treat the output cap as the window", () => {
  const [model] = normalizeDiscoveredModels(
    [{ id: "claude-opus-5", max_tokens: 128000 }],
    "claude"
  );

  assert.equal(model.inputTokenLimit, undefined);

The one shape only this PR covered is { context_length: 131072, max_tokens: 4096 } → inputTokenLimit === 131072, i.e. context_length winning over max_tokens; with max_tokens removed from the list entirely that is the first-candidate case, and it did not seem worth a second test file next to the one above.

Say the word if you disagree and I will reopen it as a test-only PR.

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.

bug(discovery): max_tokens is read as the context window and becomes inputTokenLimit (#14159)

1 participant