fix(providers): detect router-prefixed reasoning model ids for the new OpenAI contract - #2815
Conversation
|
@alectimison-maker is attempting to deploy a commit to the esokullu's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Reviewed this alongside #2808, #2809, #2813 and #2814. Catching router-prefixed ids is the right idea, but the relaxed pattern has no trailing boundary, so it now also matches models that do accept the legacy parameters.
Two consequences for that config. That last part is what the edited assertion at A trailing boundary would fix most of it — something along the lines of Two smaller things: The LM Studio guard is dead for the catalog config and case-sensitive for every other one. The settings panel keeps the old pattern. |
…settings The router-prefix regex had no trailing boundary, so it matched openai/gpt-4.1 (a chat model that accepts temperature and max_tokens) and look-alikes like o365-assistant, dropping explicit temperatures and sending max_completion_tokens that OpenRouter silently ignores. Bound the pattern to (?:^|\/)(?:gpt-5|o1|o3|o4)(?:$|[-_.\/]) and exclude gpt-4.1, which stays on the legacy contract. Move the predicate to a shared provider-compatibility helper used by both the provider and the settings Compatibility panel, and make the LM Studio guard case-insensitive.
d57d4ad to
a5e459f
Compare
|
Reworked per review. The trailing boundary was the key miss — fixed and expanded:
The previously-edited |
webbrain-one
left a comment
There was a problem hiding this comment.
One routed-model contract gap remains after bounding the regex: active GPT-5 Pro slugs on OpenRouter do not all advertise max_completion_tokens.
| */ | ||
| export function isNewOpenAIContractModel(model) { | ||
| const m = String(model || '').toLowerCase(); | ||
| return /(?:^|\/)(?:gpt-5|o1|o3|o4)(?:$|[-_.\/])/.test(m); |
There was a problem hiding this comment.
[P1] Do not classify the entire routed GPT-5 namespace as max_completion_tokens
The trailing delimiter prevents look-alike matches, but it still treats every GPT-5 suffix as the same wire contract. For example, this returns true for openai/gpt-5.5-pro (and openai/gpt-5.2-pro). OpenRouter's current model metadata and model page advertise max_tokens, but not max_completion_tokens, for GPT-5.5 Pro: https://openrouter.ai/openai/gpt-5.5-pro/api
Because OpenRouter stays on Chat Completions here, this branch sends max_completion_tokens and drops max_tokens; the configured output cap can therefore be ignored or rejected. The shared settings helper also reports the same incorrect automatic field.
Please classify the actual routed model families instead of the whole gpt-5 prefix (or rely on an explicit provider compatibility choice), and add regression coverage for at least openai/gpt-5.5-pro and openai/gpt-5.2-pro alongside the positive openai/gpt-5.6-terra case.
There was a problem hiding this comment.
The branch now includes the maintainer-directed fix 3292cd7f (preserved through the non-rewriting merge f1df7bdf). Routed GPT-5 Pro families (openai/gpt-5.5-pro, openai/gpt-5.2-pro, including dated/batch suffixes) remain on max_tokens; the positive openai/gpt-5.6-terra case remains on max_completion_tokens. The Chrome/Firefox shared helper and regression table cover both cases, while direct OpenAI Responses routing remains unchanged. Verified after the merge: node test/run.js 1772 passed.
There was a problem hiding this comment.
Follow-up d9000664 also closes the final review-pass finding: provider compatibility is now config-aware. The shared isNewOpenAIContractConfig keeps local/LM Studio and non-OpenRouter slash-prefixed model ids on legacy fields, while OpenRouter retains the maintainer-approved Pro exceptions and Terra/o-series behavior. Both provider request construction and Settings call this shared predicate, with Chrome/Firefox regression coverage. node test/run.js: 1773 passed.
|
Blocking compatibility finding from a second pass against OpenRouter's current model metadata:
Sources:
This is a regression for existing OpenRouter configurations: before this PR, those router-prefixed IDs stayed on Please do not merge as-is. Classify actual provider/model capabilities (or require an explicit compatibility selection), and add negative regression cases for at least |
…ract' into review-2815 # Conflicts: # src/chrome/src/providers/provider-compatibility.js # src/firefox/src/providers/provider-compatibility.js # test/run.js
|
Addressed the latest OpenRouter compatibility blocker in
Verification: |
Summary
OpenAICompatibleProvider._isNewOpenAIContract()now matches reasoning model ids at the start or after a/, so router-prefixed ids likeopenai/o1,openai/o3-mini, andopenai/gpt-5.6-terraget the new wire contract (max_completion_tokens, notemperature) instead of the legacy one.Motivation
Follow-up to #2807 (Azure reasoning deployments). The anchored regex
/^(gpt-5|gpt-4\.1|o1|o3|o4)/never matched prefixed router ids (OpenRouter-class routers), so a user routingopenai/o1receivedmax_tokens+temperature: 0.7— both rejected by OpenAI reasoning models — producing the same 400 loop the Azure fix addressed. The comment claiming "OpenRouter still uses the legacy contract" was wrong for reasoning models routed to OpenAI.Design
One regex change in both trees:
(?:^|\/)(?:gpt-5|gpt-4\.1|o1|o3|o4). Unprefixed behavior is preserved exactly (gpt-5.6-terra,o1-mini,gpt-4oclassification unchanged); only ids with a prefixing segment newly classify as the new contract.Testing
node test/run.js— 1765 passed, 0 failed (1 new test, both Chrome and Firefox providers)npm run test:security— 60/60 passednpm run test:toolbar-guard— 33 passedNew test asserts:
openai/o1,openai/o3-mini,openai/gpt-5.6-terra→ new contract (max_completion_tokens, no temperature);openai/gpt-4o,openrouter/deepseek-v3,openrouter/mistral-large→ legacy; lmstudio +openai/o1→ legacy (local guard). One pre-existing GPT-5.6 test that encoded the old behavior foropenai/gpt-5.6-terrawas updated to the corrected contract.Compatibility and risks