Skip to content

fix(providers): omit model field for local OpenAI-compatible servers with no configured model - #2808

Merged
esokullu merged 3 commits into
webbrain-one:mainfrom
alectimison-maker:fix/local-provider-empty-model
Aug 17, 2026
Merged

fix(providers): omit model field for local OpenAI-compatible servers with no configured model#2808
esokullu merged 3 commits into
webbrain-one:mainfrom
alectimison-maker:fix/local-provider-empty-model

Conversation

@alectimison-maker

Copy link
Copy Markdown
Contributor

Summary

  • Local OpenAI-compatible servers (Ollama, LM Studio, Jan, vLLM, SGLang, LocalAI, GPT4All, local proxy) no longer receive a fabricated model: "gpt-4o" when the Model field is empty.
  • The model getter returns null for category === "local" with no configured model, and Chat Completions / Responses request bodies omit the field entirely (mirroring LlamaCppProvider), letting the server apply its own default.
  • Non-local providers keep the existing fallback behavior (gpt-4o, or gpt-5.6-terra on the official OpenAI base URL).

Motivation

Every local provider defaults to an empty Model field (manager.js _defaultConfigs). With the field empty, requests sent model: "gpt-4o" to the local server, which 404s with "model 'gpt-4o' not found, try pulling it first" (Ollama) or rejects the unknown id (LM Studio / vLLM / SGLang). Onboarding connection tests failed the same way. There was no UI or agent guard preventing it, and no way to tell the server "use your default".

Design

Followed the existing LlamaCppProvider precedent (get model() returns config.model || null; body sets model only when truthy). ProviderManager._createProvider always normalizes config.category via categoryFor, so this.config.category === "local" is reliable for all local ids. Non-local behavior is untouched and covered by tests.

Testing

  • node test/run.js — 1767 passed, 0 failed (2 new tests, both Chrome and Firefox providers)
  • npm run test:security — 60/60 passed
  • npm run test:toolbar-guard — 33 passed

New tests assert: local providers with an empty model get model === null and no model key in _buildChatCompletionsBody; a configured local model is still sent; non-local fallbacks (openroutergpt-4o, official OpenAI → gpt-5.6-terra) stay on the wire.

Compatibility and risks

  • Custom OpenAI-compatible providers classified cloud keep the legacy fallback (existing behavior, unchanged).
  • model consumers in the agent (traces, cost estimation) already use config.model fallbacks or config-driven pricing, so a null provider model is safe; shouldUseOpenAIResponsesApi short-circuits for non-official configs.

Scope

  • Deferred: a clearer "select a model" error for servers that require a model (e.g. Ollama still errors server-side if no model is available) — the fix here is to stop sending a bogus id, not to pick a model for the user.

@vercel

vercel Bot commented Aug 16, 2026

Copy link
Copy Markdown

@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.

@esokullu

Copy link
Copy Markdown
Collaborator

Reviewed this together with #2809, #2813, #2814 and #2815. Five things, roughly in order of impact.

1. Local providers now send a body with no model key

get model() returns null for category: 'local' (src/chrome/src/providers/openai.js:86), and the new guards at :387 and :521 drop the field entirely. Every built-in local entry in the catalog ships model: '' (manager.js:262-383), so this is the state a user is in right after enabling one.

Ollama's OpenAI-compatible route answers that with 400 {"error":{"message":"model is required"}}. SGLang, LocalAI, Jan and GPT4All want the field too. The new comment says the server "applies its own default", which holds for llama.cpp and LM Studio but not for the rest.

There's also no config path back: model is in RESERVED_EXTRA_BODY_KEYS (provider-compatibility.js:25), so extraBody: {"model": "..."} gets stripped before the request goes out.

Knock-on effect: provider.model is now null in testVisionConnection's result (manager.js:1592) and in every trace.recordLLMRequest payload (agent.js:10299, :10729, :10801, :11712, :25502, :25540), so traces record null where they used to record an id.

2. The Azure contract check reads a deployment name, not a model id

azure-openai.js:73 tests /^(gpt-5|gpt-4\.1|o1|o3|o4)/ against this.config.model. For Azure that field holds the deployment name the user typed — get deployment() at :21 documents exactly that — and deployment names don't have to resemble the model behind them.

A deployment called prod-chat pointing at o3-mini therefore still gets max_tokens and temperature: 0.7, and still returns the 400 this PR is meant to fix. The comment offers omitTemperature plus a maxTokensField override as the way out, but those are two separate settings and the user has to discover both.

It misfires the other way too. o365-assistant starts with the characters o3, so a gpt-35-turbo deployment under that name now sends max_completion_tokens — unrecognized under the default apiVersion: '2024-10-21' at :30 — and loses its temperature. Same for anything named o1-… or o4-….

An explicit per-deployment setting would be more reliable here than guessing from the name.

3. The contract probe reads the field untrimmed

azure-openai.js:72 does (this.config.model || '').toLowerCase() while get deployment() at :22 trims. Paste " o3-mini" with a leading space and _chatUrl() builds the correct URL while the contract check returns false: 400 from Azure, with a settings page that looks right.

4. The Compatibility panel doesn't know about the new Azure branch

automaticTokenField() (src/chrome/src/ui/settings.js:2311, firefox :1938) gates on config.type === 'openai', but supportsProviderCompatibilitySettings at :2293 renders that panel for azure_openai as well. After this change an Azure o3-mini deployment sends max_completion_tokens while the summary still displays max_tokens. Anyone debugging the 400 who trusts the panel and pins the displayed value walks straight back into the original bug.

5. Duplication, and an overlap with #2809

This adds a fourth copy of the heuristic per tree — openai.js:179, azure-openai.js:73, settings.js:2311, and the stricter variant at provider-compatibility.js:164 — which is eight across both browsers. provider-compatibility.js already exists as the shared home for this kind of thing. #2815 edits only the openai.js copy, and that's how items 2 and 4 turn into live inconsistencies rather than theoretical ones.

Separately: #2809 carries a byte-identical azure-openai.js (blob 959400a2 in both trees) and the same Azure tests. The two branches were cut from different bases, so whichever lands second will either conflict in test/run.js or duplicate the test block.

… for LM Studio

Local OpenAI-compatible servers reject an empty model field, so sending a
fabricated fallback id (previously gpt-4o) or omitting the field both
fail. Mark ollama, vLLM, SGLang, LocalAI, Jan, and GPT4All as
requiresModel so an empty Model field fails with a clear error instead of
an opaque server 400. LM Studio (like llama.cpp) applies its own default,
so it omits the field when unset. Non-local providers keep their fallback.
@alectimison-maker
alectimison-maker force-pushed the fix/local-provider-empty-model branch from 08732e3 to 8cc6726 Compare August 16, 2026 12:48
@alectimison-maker

Copy link
Copy Markdown
Contributor Author

Reworked per review. The fabricated-fallback was the real bug, but omitting the field broke Ollama/SGLang/LocalAI/Jan/GPT4All (they require it) and left model: null in traces.

New approach: mark the local servers that require a model (ollama, vLLM, SGLang, LocalAI, Jan, GPT4All) with requiresModel: true in the catalog, so an empty Model field fails with a clear "model is required" error instead of a fabricated id. LM Studio (like llama.cpp, your call-out) omits the field when unset and applies its own default. Non-local providers keep their existing fallback unchanged. Traces no longer record null (the throw happens before a request is built).

The branch was also cut from a different base that bundled the Azure change — I've rebased it onto main so it now contains only the local-model change. Same for #2809.

@webbrain-one webbrain-one left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking local-provider coverage gap remains. Every unset local provider must avoid the cloud-model fallback, including persisted configurations that do not inherit new catalog flags.

Comment thread src/chrome/src/providers/openai.js Outdated
// Local servers that reject an empty model instead carry
// `requiresModel: true` in the catalog and throw above, so no local
// server ever receives a fabricated model id.
if (this.config.category === 'local'

@webbrain-one webbrain-one Aug 16, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Handle every unset local provider before cloud fallback

This only special-cases LM Studio. An unset privatemode-ai configuration is also local but lacks requiresModel, and older persisted local entries can remain empty because ProviderManager.load() carries duplicates through without merging new defaults. Both paths fall through to gpt-4o, recreating the fabricated-model failure. Handle the entire local category explicitly (omitting the field or throwing when the provider requires a model), and ensure migrated/persisted entries inherit that contract. The Firefox mirror needs the same treatment.

@esokullu
esokullu merged commit ce5db07 into webbrain-one:main Aug 17, 2026
1 of 2 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.

3 participants