diff --git a/CHANGELOG.md b/CHANGELOG.md index 6aed2bcb..9bfe9e8c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,8 @@ - **The reasoning-effort chip is correct right after an empty startup.** On a normal boot with no active session restored, the top-bar reasoning chip wasn't refreshed for the hydrated default model, so it could show a stale value until you changed the model. It now refreshes on empty boot too. Thanks @rodboev. (#5967, #5954) +- **Custom/proxy providers keep the full vendor namespace on the model id (P0 routing fix).** When you run multiple upstream vendors behind one OpenAI-compatible proxy (`provider: custom` / `custom:`) and pick a vendor-namespaced model such as `x-ai/grok-4.5`, the WebUI was stripping the `x-ai/` namespace before sending, so the proxy received a bare `grok-4.5` it couldn't route and rejected the turn with `HTTP 400: Invalid model format`. The strip was a catalog-*family* guess that couldn't tell a namespace the proxy actually routes on (`x-ai/grok-4.5`, keep) apart from a stale cross-provider leftover on a relay that only serves bare ids (`openai/gpt-5.4` → `gpt-5.4`, strip). Resolution is now based on **what the endpoint actually advertised**: if the proxy's own catalog lists the full `vendor/model`, it is sent whole; if it lists only the bare id, the redundant prefix is stripped; and on a cold/unbuilt catalog the id is preserved verbatim (fail-safe toward the id you selected). First-party providers pointed at an OpenAI-compatible proxy still send the bare id as before. Regression was latent — it surfaced only once a model graduated into the first-party catalog. Thanks @b3nw for the report and root-cause analysis. (#5979) + - **Transparent Stream no longer bloats memory (or freezes the tab) on long tool-heavy chats.** With Transparent Stream enabled, opening a conversation with many prompts built a DOM node for every activity event of every past turn at load and eagerly rendered each tool row's full detail body — a long reasoning-heavy history could balloon to multiple GB and lock up the tab. Settled tool rows now render header-only and build their detail on first expand (identical to the old view once opened), and a turn with a very large number of steps shows its most recent 30 behind a "Show N earlier steps" control. Live turns are unchanged. Thanks @curtisszmania-cytocync for the report. (#5974, #5966) - **Dictation no longer wipes text you already typed.** When you typed part of a prompt and then used the microphone (server-side transcription / MediaRecorder path), the recognized text replaced your typed prefix instead of being appended to it. Dictation now appends to whatever is already in the composer, so you can type a partial sentence and dictate the rest. A Settings toggle lets you opt back into the old replace behavior. Thanks @Kopamed. (#5895) diff --git a/api/config.py b/api/config.py index 44e44463..7fdb0ed4 100644 --- a/api/config.py +++ b/api/config.py @@ -2808,9 +2808,46 @@ def resolve_model_provider(model_id: str) -> tuple: # earlier by the ``prefix == config_provider`` branch. _cp_lower = (config_provider or "").strip().lower() _is_custom = _cp_lower == "custom" or _cp_lower.startswith("custom:") - if prefix in _PROVIDER_MODELS and ( - not _is_custom or _is_first_party_model(prefix, bare) - ): + if _is_custom: + # Vendor-routing proxy: the ONLY reliable signal for whether the + # endpoint wants the full ``vendor/model`` id or the bare id is + # what its own catalog actually advertised (the ids the user + # picked from the dropdown, populated by the endpoint's live + # ``/v1/models`` probe or a ``custom_providers[].models`` + # allowlist). The catalog-FAMILY heuristic (_is_first_party_model) + # is the wrong question: it answered "is this bare id a first- + # party model of the prefix's home vendor?" which is True for BOTH + # ``x-ai/grok-4.5`` (proxy advertised it whole — must preserve, + # #5979) and ``openai/gpt-5.4`` (a stale leftover on a relay that + # only serves bare ``gpt-5.4`` — must strip, #433). Those two are + # structurally identical to the family heuristic, so a model + # graduating into a first-party catalog (agent commit 62ada5175 + # adding grok-4.5) silently flipped a working custom-proxy id from + # preserved to stripped. Provenance tells them apart: + _advertised = _endpoint_advertised_model_ids(config_provider) + if _advertised: + # Full id advertised → the endpoint routes on it verbatim (#5979/#3872/#548). + if model_id in _advertised: + return model_id, config_provider, config_base_url + # ONLY the bare id advertised → the prefix is a redundant + # leftover the relay rejects; strip it (#433). Keep the + # ``prefix in _PROVIDER_MODELS`` belt so an adversarial catalog + # advertising a bare id can't strip an unknown-vendor prefix. + if bare in _advertised and prefix in _PROVIDER_MODELS: + return bare, config_provider, config_base_url + # Cold/unbuilt catalog OR neither shape advertised → preserve the + # id verbatim. This is the fail-SAFE default: a wrong strip would + # destroy a namespace the user actively selected this session + # (recurs every turn, unrepairable — the info is gone), while a + # wrong preserve only mis-sends a stale leftover that fails loudly + # and self-heals the moment the catalog builds. Matches the + # agent's own normalize_model_for_provider (custom = pass-through) + # and the #4210 no-base_url custom behaviour. + return model_id, config_provider, config_base_url + # Non-custom first-party provider pointed at an OpenAI-compatible + # proxy (e.g. provider=openai + base_url=litellm): the bare id is + # what it expects — "openai/gpt-5.4" → "gpt-5.4" (#433). + if prefix in _PROVIDER_MODELS: return bare, config_provider, config_base_url # Intrinsic / unknown prefix — pass the full model_id through unchanged. return model_id, config_provider, config_base_url @@ -4511,6 +4548,64 @@ _available_models_cache_lock = threading.RLock() # must be RLock: cold path ref _cache_build_cv = threading.Condition(_available_models_cache_lock) # shares underlying RLock so notify_all() is safe inside with _available_models_cache_lock _cache_build_in_progress = False # True while a cold path is actively building +# Memoized (snapshot_ref, {provider_slug: frozenset(model_ids)}) derived from +# the published models-catalog snapshot. Used by _endpoint_advertised_model_ids +# to answer "did this endpoint actually advertise this exact id?" in O(1) per +# send without rebuilding. Keyed on the snapshot object identity so it is +# recomputed exactly once per catalog publish (the cache is replaced wholesale, +# never mutated) and can never serve stale ids from a superseded catalog. +_advertised_model_ids_memo: tuple | None = None + + +def _endpoint_advertised_model_ids(provider_id: str | None) -> frozenset | None: + """Model ids the given provider's group advertised in the current catalog. + + Reads ONLY the already-published in-memory catalog snapshot + (``_available_models_cache``) — it never builds, live-probes, or touches + disk, so it is safe to call on the per-turn send hot path. Returns: + + * a ``frozenset`` of the ids advertised by ``provider_id``'s own group + (bare ids for the active provider, e.g. ``x-ai/grok-4.5``), or + * ``None`` when the catalog is cold/unbuilt OR the provider has no group. + + ``None`` means "no provenance signal available" — callers MUST treat that as + "preserve the model id verbatim" so a cache miss never silently strips a + vendor namespace off an id the user actively selected (#5979). Scoping to + the provider's OWN group prevents a same-named id in a sibling group (e.g. + an ``openai/gpt-5.4`` sitting in the OpenRouter group) from masquerading as + something this custom endpoint advertised. + """ + global _advertised_model_ids_memo + snapshot = _available_models_cache # atomic reference read; publishes replace wholesale + if snapshot is None: + return None + memo = _advertised_model_ids_memo + # Identity check (``is``), not id(): holding the snapshot reference in the + # memo keeps it alive, so a freed-then-reused id() can't cause a false hit. + if memo is None or memo[0] is not snapshot: + by_slug: dict[str, frozenset] = {} + try: + groups = snapshot.get("groups", []) or [] + except AttributeError: + return None + for group in groups: + if not isinstance(group, dict): + continue + slug = str(group.get("provider_id") or "").strip().lower() + if not slug: + continue + ids = frozenset( + str(m.get("id")) + for m in (group.get("models") or []) + if isinstance(m, dict) and m.get("id") + ) + by_slug[slug] = by_slug.get(slug, frozenset()) | ids + memo = (snapshot, by_slug) + _advertised_model_ids_memo = memo + slug = str(provider_id or "").strip().lower() + return memo[1].get(slug) + + # Hard wall-clock budget for a COLD live provider-catalog rebuild when it is # run from a foreground request path. The live rebuild does one network probe # per detected provider (Copilot token-exchange HTTPS, OpenRouter /v1/models, diff --git a/tests/test_model_resolver.py b/tests/test_model_resolver.py index 0f6f5ee5..3e0826be 100644 --- a/tests/test_model_resolver.py +++ b/tests/test_model_resolver.py @@ -27,6 +27,36 @@ def _resolve_with_config(model_id, provider=None, base_url=None, default=None, c config.cfg.update(old_cfg) +def _resolve_with_catalog(model_id, advertised_ids, *, provider=None, base_url=None, + provider_id='custom', default=None): + """Resolve with a seeded models-catalog snapshot (#5979 provenance). + + ``advertised_ids`` is the list of model ids the endpoint's own group + advertised (what the user could have picked from the dropdown). Pass ``None`` + to simulate a COLD/unbuilt catalog. Seeds ``config._available_models_cache`` + (the snapshot ``_endpoint_advertised_model_ids`` reads) for the duration of + the call, resets the derivation memo, and restores both afterwards so tests + don't leak catalog state into each other. + """ + old_cache = config._available_models_cache + old_memo = config._advertised_model_ids_memo + if advertised_ids is None: + config._available_models_cache = None + else: + config._available_models_cache = { + 'groups': [{ + 'provider_id': provider_id, + 'models': [{'id': mid, 'label': mid} for mid in advertised_ids], + }] + } + config._advertised_model_ids_memo = None # force recompute against the seeded snapshot + try: + return _resolve_with_config(model_id, provider=provider, base_url=base_url, default=default) + finally: + config._available_models_cache = old_cache + config._advertised_model_ids_memo = old_memo + + # ── OpenRouter prefix handling ──────────────────────────────────────────── def test_openrouter_free_keeps_full_path(): @@ -170,10 +200,12 @@ def test_custom_remote_preserves_intrinsic_vendor_prefix_3872(): A bare ``custom`` provider with a remote base_url is a vendor-routing proxy (LiteLLM, Bedrock gateway). ``bedrock/`` is an intrinsic routing segment the proxy needs whole; stripping it to ``opus-4-6`` makes the proxy return 403 - "model not allowed for your group". + "model not allowed for your group". The proxy advertised the full id (the + user picked it from the dropdown), so provenance preserves it. """ - model, provider, base_url = _resolve_with_config( + model, provider, base_url = _resolve_with_catalog( 'bedrock/opus-4-6', + advertised_ids=['bedrock/opus-4-6'], provider='custom', base_url='https://router.example.com/v1', ) @@ -183,15 +215,20 @@ def test_custom_remote_preserves_intrinsic_vendor_prefix_3872(): def test_custom_remote_strips_redundant_first_party_prefix_433(): - """#433: bare-custom remote proxy still strips a REDUNDANT first-party prefix. + """#433: bare-custom remote proxy strips a prefix ONLY when the endpoint + advertised just the BARE id (not the full ``vendor/model``). - ``gpt-5.4`` IS a first-party OpenAI model, so ``openai/`` is a redundant - leftover and the proxy expects the bare id. This is the behaviour pinned by - test_sprint40_ui_polish.py::test_prefixed_model_stripped_for_custom_endpoint; - the #3872 fix must keep it while preserving intrinsic vendor prefixes. + #433 is a verified real relay whose ``/v1/models`` returned bare ``gpt-5.4`` + and rejected ``openai/gpt-5.4`` (a stale cross-provider leftover). The strip + is now justified by PROVENANCE — the catalog advertises exactly ``gpt-5.4`` + and NOT ``openai/gpt-5.4`` — instead of the old catalog-family guess that + couldn't tell this stale-leftover case apart from #5979's advertised-full-id + case. Behaviour is also pinned by + test_sprint40_ui_polish.py::test_prefixed_model_stripped_for_custom_endpoint. """ - model, provider, base_url = _resolve_with_config( + model, provider, base_url = _resolve_with_catalog( 'openai/gpt-5.4', + advertised_ids=['gpt-5.4'], # relay advertises ONLY the bare id provider='custom', base_url='https://router.example.com/v1', ) @@ -199,10 +236,93 @@ def test_custom_remote_strips_redundant_first_party_prefix_433(): assert provider == 'custom' +def test_custom_remote_preserves_advertised_full_id_5979(): + """#5979 (the P0 regression): a custom proxy that advertises the FULL + ``x-ai/grok-4.5`` must receive it whole — never the bare ``grok-4.5``. + + The old code stripped it because ``grok-4.5`` had graduated into the x-ai + first-party catalog (agent commit 62ada5175), flipping ``_is_first_party_model`` + to True for a model the proxy routes on by its full ``x-ai/`` namespace. This + is the exact HTTP 400 b3nw hit ("Invalid model format ... grok-4.5"). The + provenance rule preserves it because the endpoint advertised the full id. + """ + # Reproduce the data-driven trigger: grok-4.5 present in the x-ai catalog. + xai_catalog = config._PROVIDER_MODELS.get('x-ai') + assert isinstance(xai_catalog, list) + assert any(isinstance(m, dict) and m.get('id') == 'grok-4.5' for m in xai_catalog), ( + "guard: grok-4.5 must be first-party for this regression test to be meaningful" + ) + model, provider, base_url = _resolve_with_catalog( + 'x-ai/grok-4.5', + advertised_ids=['x-ai/grok-4.5'], # proxy advertises the FULL namespaced id + provider='custom', + base_url='https://proxy.example.com/v1', + ) + assert model == 'x-ai/grok-4.5', ( + f"advertised full id must be preserved for routing, got {model!r}" + ) + assert provider == 'custom' + assert base_url == 'https://proxy.example.com/v1' + + +def test_named_custom_slug_preserves_advertised_full_id_5979(): + """#5979 (named-custom variant): provider=custom: proxy advertising the + full ``x-ai/grok-4.5`` also preserves it.""" + model, provider, base_url = _resolve_with_catalog( + 'x-ai/grok-4.5', + advertised_ids=['x-ai/grok-4.5'], + provider='custom:my-gateway', + provider_id='custom:my-gateway', + base_url='https://proxy.example.com/v1', + ) + assert model == 'x-ai/grok-4.5', f"full id must be preserved for custom:slug, got {model!r}" + + +def test_custom_remote_cold_catalog_preserves_verbatim_5979(): + """#5979 fail-safe: with a COLD/unbuilt catalog (no provenance signal), a + custom proxy preserves the model id verbatim — for BOTH the advertised-full + case and the stale-leftover case. + + Preserve is the correct cold default: a wrong strip destroys a namespace the + user actively selected this session (recurs every turn, unrepairable), while + a wrong preserve only mis-sends a stale leftover that fails loudly and heals + the instant the catalog builds. The active-user-selection case (#5979) is + cold-safe by construction — you can't select from a dropdown that was never + built. + """ + model_a, _, _ = _resolve_with_catalog( + 'x-ai/grok-4.5', advertised_ids=None, + provider='custom', base_url='https://proxy.example.com/v1', + ) + assert model_a == 'x-ai/grok-4.5', f"cold catalog must preserve verbatim, got {model_a!r}" + model_b, _, _ = _resolve_with_catalog( + 'openai/gpt-5.4', advertised_ids=None, + provider='custom', base_url='https://router.example.com/v1', + ) + assert model_b == 'openai/gpt-5.4', f"cold catalog must preserve verbatim, got {model_b!r}" + + +def test_custom_remote_prefers_full_id_when_both_advertised_5979(): + """When a proxy advertises BOTH the full ``x-ai/grok-4.5`` and a bare + ``grok-4.5``, the exact full selection wins (preserve).""" + model, _, _ = _resolve_with_catalog( + 'x-ai/grok-4.5', + advertised_ids=['x-ai/grok-4.5', 'grok-4.5'], + provider='custom', + base_url='https://proxy.example.com/v1', + ) + assert model == 'x-ai/grok-4.5', f"exact full selection must win, got {model!r}" + + def test_custom_remote_preserves_unknown_prefix_548(): - """#548: an unknown vendor prefix (zai-org/GLM-5.1) is always preserved.""" - model, provider, base_url = _resolve_with_config( + """#548: an unknown vendor prefix (zai-org/GLM-5.1) is always preserved. + + The proxy advertised the full id; ``zai-org`` isn't in _PROVIDER_MODELS so + even the bare-advertised belt could never strip it. + """ + model, provider, base_url = _resolve_with_catalog( 'zai-org/GLM-5.1', + advertised_ids=['zai-org/GLM-5.1'], provider='custom', base_url='https://api.deepinfra.com/v1/openai', ) @@ -212,15 +332,13 @@ def test_custom_remote_preserves_unknown_prefix_548(): def test_named_custom_slug_preserves_intrinsic_vendor_prefix_3872(): """#3872 (named-custom variant): provider=custom: + remote base_url also - preserves an intrinsic vendor prefix when the typed model isn't in the entry. - - A model typed/selected that is not listed in the custom_providers[] entry - falls through to the base_url branch; a bare id NOT first-party of the prefix - (bedrock/opus-4-6) must still be kept whole, same as bare ``custom``. + preserves an intrinsic vendor prefix the endpoint advertised whole. """ - model, provider, base_url = _resolve_with_config( + model, provider, base_url = _resolve_with_catalog( 'bedrock/opus-4-6', + advertised_ids=['bedrock/opus-4-6'], provider='custom:my-gateway', + provider_id='custom:my-gateway', base_url='https://router.example.com/v1', ) assert model == 'bedrock/opus-4-6', f"intrinsic prefix must be preserved for custom:slug, got {model!r}" diff --git a/tests/test_sprint40_ui_polish.py b/tests/test_sprint40_ui_polish.py index 5743ceba..c802af95 100644 --- a/tests/test_sprint40_ui_polish.py +++ b/tests/test_sprint40_ui_polish.py @@ -146,27 +146,49 @@ if __name__ == "__main__": class TestCustomEndpointModelStripping: """Tests for fix #433: strip provider prefix when custom base_url is set.""" - def _resolve(self, model_id, provider=None, base_url=None): - """Helper: set cfg directly (same pattern as test_model_resolver.py).""" + def _resolve(self, model_id, provider=None, base_url=None, advertised_ids=None): + """Helper: set cfg directly (same pattern as test_model_resolver.py). + + ``advertised_ids`` seeds the models-catalog snapshot that provenance + resolution reads (#5979). Pass the ids the endpoint's own group + advertised; None leaves the catalog cold (preserve-verbatim default). + """ old_cfg = dict(_api_config.cfg) + old_cache = _api_config._available_models_cache + old_memo = _api_config._advertised_model_ids_memo model_cfg = {} if provider: model_cfg['provider'] = provider if base_url: model_cfg['base_url'] = base_url _api_config.cfg['model'] = model_cfg + if advertised_ids is None: + _api_config._available_models_cache = None + else: + _api_config._available_models_cache = { + 'groups': [{ + 'provider_id': provider or 'custom', + 'models': [{'id': mid, 'label': mid} for mid in advertised_ids], + }] + } + _api_config._advertised_model_ids_memo = None try: return _api_config.resolve_model_provider(model_id) finally: _api_config.cfg.clear() _api_config.cfg.update(old_cfg) + _api_config._available_models_cache = old_cache + _api_config._advertised_model_ids_memo = old_memo def test_prefixed_model_stripped_for_custom_endpoint(self): - """Issue #433: 'openai/gpt-5.4' with custom base_url returns bare 'gpt-5.4'.""" + """Issue #433: 'openai/gpt-5.4' with custom base_url returns bare 'gpt-5.4' + when the endpoint advertised ONLY the bare id (provenance strip). + """ model, provider, base_url = self._resolve( 'openai/gpt-5.4', provider='custom', base_url='http://my-proxy.local:8080/v1', + advertised_ids=['gpt-5.4'], # relay serves the bare id only ) assert model == 'gpt-5.4', ( "Expected bare 'gpt-5.4' for custom endpoint, got '{}'."