Merge pull request #5999 from nesquena/fix/5979-cold-catalog-preserve
fix(#5979 follow-up): preserve custom-proxy vendor namespace on cold-catalog sends
This commit is contained in:
8 files changed
+558
-43
No files matched your search
@@ -15,6 +15,8 @@
|
||||
|
||||
- **No more false "No response from provider" after a completed answer that used tools.** When an assistant turn ended with a real final answer delivered as structured content following tool calls (common with reasoning models and multi-tool turns), the completion detector could miss the final text and render a spurious "No response from provider" card under the finished reply. Final-answer detection now understands structured content parts (text after tool-use boundaries), while genuine empty completions still correctly show the no-response notice, and a real terminal provider error still surfaces as an error (never masked as a final answer). Thanks @jtstothard. (#5990, #5686)
|
||||
|
||||
- **Custom/proxy providers keep the full vendor namespace when you deliberately pick a non-default model (follow-up to the earlier #5979 fix).** The initial fix preserved `x-ai/grok-4.5` when it was your configured default or already in a warm model catalog, but a model you *selected in the picker* that wasn't your default (e.g. `x-ai/grok-composer-2.5-fast`) was still truncated to the bare `grok-composer-2.5-fast` when the models catalog was cold at send time — the normal state right after a restart, a settings save, or a cache expiry. The proxy then rejected the bare id with `HTTP 400`. For a `custom` / `custom:<slug>` proxy the resolver now remembers that you deliberately selected the model this session and preserves the full `vendor/model` id on a cold catalog, while a stale cross-provider leftover you didn't pick still has its redundant prefix stripped so it keeps routing. The send path also warms model-catalog provenance from the durable on-disk cache first (network-free, non-blocking, never triggers a live probe). Thanks @b3nw for the detailed follow-up and the CLI-vs-WebUI comparison that localized it. (#5979)
|
||||
|
||||
- **Cron unread ("Tasks") dots are now scoped to the active profile — no more ghost dots after switching.** In multi-profile setups, the session-sidebar unread dot and the Tasks badge could keep showing a cron completion from a *different* profile after you switched, and the count could resurrect from a stale poll. Cron completion markers now carry their source + owning profile, are cleared for the profile you switch away from (including legacy/untagged markers), and a previous-profile poll response can no longer restore a stale unread. Non-cron completion unread and the current profile's own cron unread are preserved; default ↔ renamed-root aliases are treated as the same profile so a root cron marker is neither wrongly cleared nor wrongly kept. Thanks @jbbottoms. (#5975, #5960)
|
||||
|
||||
- **The Docker "manual update" banner instruction is now localized.** On Docker/no-git deployments the update banner shows the manual update command; that guidance string was hardcoded in English. It now goes through the i18n system, so it renders in the active language (English fallback where a translation isn't present yet). Thanks @jbbottoms. (#5973, #5959)
|
||||
|
||||
+111
-8
@@ -2560,7 +2560,7 @@ def _get_provider_cfg(provider_id) -> dict:
|
||||
return provider_cfg if isinstance(provider_cfg, dict) else {}
|
||||
|
||||
|
||||
def resolve_model_provider(model_id: str) -> tuple:
|
||||
def resolve_model_provider(model_id: str, *, explicitly_picked: bool = False) -> tuple:
|
||||
"""Resolve model name, provider, and base_url for AIAgent.
|
||||
|
||||
Model IDs from the dropdown can be in several formats:
|
||||
@@ -2580,6 +2580,16 @@ def resolve_model_provider(model_id: str) -> tuple:
|
||||
provider.
|
||||
|
||||
Returns (model, provider, base_url) where provider and base_url may be None.
|
||||
|
||||
``explicitly_picked``: True when the caller knows the user DELIBERATELY
|
||||
selected ``model_id`` this session (persisted from an ``explicit_model_pick``
|
||||
UI action), as opposed to it being a stale session leftover. Used ONLY for
|
||||
the custom-proxy COLD-catalog decision (#5979): with no provenance available,
|
||||
a deliberately-picked ``vendor/model`` is preserved verbatim (the user chose
|
||||
it, the proxy routes on it), while an UNMARKED id (a stale cross-provider
|
||||
leftover, e.g. #433's ``openai/gpt-5.4`` on a bare-only relay) still gets the
|
||||
legacy redundant-prefix strip so it keeps routing when cold. Warm provenance
|
||||
(endpoint-advertised ids) always takes precedence over this flag.
|
||||
"""
|
||||
config_provider = None
|
||||
config_base_url = None
|
||||
@@ -2898,13 +2908,30 @@ def resolve_model_provider(model_id: str) -> tuple:
|
||||
return model_id, config_provider, config_base_url
|
||||
# (3) Provenance genuinely unavailable (cold/unbuilt or
|
||||
# fingerprint-mismatched catalog AND not config-declared).
|
||||
# Fall back to the LEGACY family heuristic so this narrow edge
|
||||
# is never WORSE than the pre-fix behaviour: a first-party
|
||||
# redundant prefix still strips (#433 relay works cold), while
|
||||
# an intrinsic/unknown prefix (bedrock/opus-4-6 #3872,
|
||||
# zai-org/GLM-5.1 #548) is preserved. The #5979 active-user
|
||||
# path can't reach here — a selected id is either config-
|
||||
# declared or in the catalog the dropdown was built from.
|
||||
# Distinguish a DELIBERATE selection from a stale leftover:
|
||||
#
|
||||
# * explicitly_picked → PRESERVE verbatim. The user chose this
|
||||
# exact ``vendor/model`` in the UI this session; the proxy
|
||||
# routes on it. A wrong strip destroys a namespace the proxy
|
||||
# needs (recurs every turn, unrepairable short of declaring
|
||||
# every model in config) — this is b3nw's #5979 case: a
|
||||
# non-default pick on a custom:<slug> proxy, cold catalog.
|
||||
# * NOT explicitly_picked → legacy redundant-prefix strip. An
|
||||
# unmarked id here is a stale cross-provider leftover (the
|
||||
# user switched providers and the old session model lingers,
|
||||
# e.g. #433's ``openai/gpt-5.4`` on a relay that only serves
|
||||
# bare ``gpt-5.4``); stripping keeps it routing while cold.
|
||||
#
|
||||
# Warm provenance (case 2, endpoint-advertised ids) always
|
||||
# wins over this flag; the send path also warms provenance
|
||||
# network-free from the disk cache first
|
||||
# (warm_models_catalog_provenance_if_cold), so this branch is
|
||||
# reached only in the narrow no-disk-cache window. The flag
|
||||
# removes the data-driven flaw where a model graduating into
|
||||
# the static first-party catalog silently flipped routing
|
||||
# (exactly how #5979 regressed).
|
||||
if explicitly_picked:
|
||||
return model_id, config_provider, config_base_url
|
||||
if prefix in _PROVIDER_MODELS and _is_first_party_model(prefix, bare):
|
||||
return bare, config_provider, config_base_url
|
||||
return model_id, config_provider, config_base_url
|
||||
@@ -8162,6 +8189,82 @@ def _models_cache_file_age_seconds(cache_path: Path, now: float) -> float | None
|
||||
return None
|
||||
|
||||
|
||||
def warm_models_catalog_provenance_if_cold() -> None:
|
||||
"""Best-effort, NON-BLOCKING, disk-only publish of catalog provenance.
|
||||
|
||||
The send path (``api/streaming.py``) resolves the wire model via
|
||||
``resolve_model_provider`` without ever building the models catalog, and the
|
||||
#1855 chat/start fast path deliberately skips the catalog when a session
|
||||
already carries a persisted model+provider — so "cold at send" is the
|
||||
designed behaviour after any process restart, memory-TTL expiry, or cache
|
||||
invalidation, not a rare race. In that state the custom-proxy provenance
|
||||
signal (``_endpoint_advertised_model_ids``) is ``None`` and resolution falls
|
||||
to the cold-preserve default; this helper restores the endpoint-advertised
|
||||
signal from the durable disk cache so the #433 bare-only-strip stays exact.
|
||||
|
||||
Deliberately does NOT call ``get_available_models(prefer_cache=True)``: even
|
||||
in prefer-cache mode that acquires ``_available_models_cache_lock`` and can
|
||||
block up to ~60s waiting on an in-flight rebuild (unbounded in synchronous
|
||||
rebuild mode) — unacceptable on the send hot path. Instead this:
|
||||
* tries the cache lock NON-BLOCKING and returns immediately if it's busy
|
||||
(a concurrent rebuild will publish provenance itself);
|
||||
* reads ONLY the on-disk cache (no network, no live probe, no rebuild);
|
||||
* publishes the snapshot + source fingerprint via the same globals the
|
||||
real publish sites use, then ``_sync_models_cache_provenance()``.
|
||||
Publishing the fingerprint from the CURRENT runtime is correct: the disk
|
||||
cache is validated by schema/version/source-fingerprint on load
|
||||
(``_is_loadable_disk_cache``), so a load success means it belongs to this
|
||||
profile. Callers must not hold ``_cfg_lock`` (this reads config for the
|
||||
fingerprint); the send worker satisfies that.
|
||||
|
||||
Profile isolation: the fast no-op is taken ONLY when the resident provenance
|
||||
fingerprint matches the CURRENT profile's runtime fingerprint. The catalog
|
||||
globals are process-wide, so a concurrently-active profile B could have left
|
||||
its own (or a stale) provenance resident; an unconditional non-``None``
|
||||
early return would let B's catalog block profile A from loading A's own valid
|
||||
disk cache (A would then resolve against B's advertised ids). Comparing the
|
||||
published fingerprint to the current one before short-circuiting closes that
|
||||
hole — a mismatch falls through to load THIS profile's disk snapshot.
|
||||
"""
|
||||
global _available_models_cache, _available_models_cache_ts
|
||||
global _available_models_cache_source_fingerprint
|
||||
|
||||
def _provenance_is_current() -> bool:
|
||||
prov = _models_cache_provenance
|
||||
if prov is None:
|
||||
return False
|
||||
try:
|
||||
return prov[1] == _models_cache_source_fingerprint()
|
||||
except Exception:
|
||||
return False
|
||||
|
||||
if _provenance_is_current():
|
||||
return # already warm for THIS profile — one global read, no work
|
||||
got = _available_models_cache_lock.acquire(blocking=False)
|
||||
if not got:
|
||||
return # a concurrent build/publish holds the lock; it will publish
|
||||
try:
|
||||
if _provenance_is_current():
|
||||
return # published for this profile while we waited for the lock
|
||||
try:
|
||||
disk_groups = _load_models_cache_from_disk()
|
||||
except Exception:
|
||||
disk_groups = None
|
||||
if disk_groups is None:
|
||||
return # no durable cache for this profile → stay cold, preserve verbatim
|
||||
_available_models_cache = disk_groups
|
||||
_available_models_cache_ts = time.monotonic()
|
||||
try:
|
||||
_available_models_cache_source_fingerprint = _models_cache_source_fingerprint()
|
||||
except Exception:
|
||||
_available_models_cache_source_fingerprint = None
|
||||
_sync_models_cache_provenance()
|
||||
except Exception:
|
||||
logger.debug("models catalog provenance warm failed", exc_info=True)
|
||||
finally:
|
||||
_available_models_cache_lock.release()
|
||||
|
||||
|
||||
def get_available_models_for_session_visit() -> dict:
|
||||
"""Return /api/models with a short session-visit freshness horizon.
|
||||
|
||||
|
||||
+26
-1
@@ -1081,6 +1081,21 @@ def _parse_nonnegative_int(value):
|
||||
return parsed if parsed >= 0 else None
|
||||
|
||||
|
||||
def model_explicit_pick_signature(model, model_provider) -> str:
|
||||
"""Stable signature of a (model, provider) selection for #5979 explicit-pick
|
||||
provenance. The persisted ``Session.model_explicit_pick_signature`` is set to
|
||||
this when the user deliberately picks a model; the streaming resolver only
|
||||
treats a selection as deliberate when the CURRENT routing context produces
|
||||
the same signature. Any model/provider change (chat-start, session-update,
|
||||
normalization, provider repair) yields a different signature and thus
|
||||
invalidates the stale pick — so a #433 first-party leftover is never wrongly
|
||||
preserved. Uses \\x1f (unit separator) so it can't collide with model ids.
|
||||
"""
|
||||
_m = str(model or "").strip()
|
||||
_p = str(model_provider or "").strip().lower()
|
||||
return f"{_m}\x1f{_p}"
|
||||
|
||||
|
||||
class Session:
|
||||
def __init__(self, session_id: str=None, title: str='Untitled',
|
||||
workspace=str(DEFAULT_WORKSPACE), model=DEFAULT_MODEL,
|
||||
@@ -1136,6 +1151,16 @@ class Session:
|
||||
self.workspace = str(Path(workspace).expanduser().resolve())
|
||||
self.model = model
|
||||
self.model_provider = str(model_provider).strip().lower() if model_provider else None
|
||||
# #5979: signature of the model the user DELIBERATELY picked this session
|
||||
# (``"<model>\x1f<provider>"``), or None. Used by the streaming resolver
|
||||
# to preserve a custom-proxy vendor namespace on a COLD catalog ONLY when
|
||||
# the current routing context still matches what was picked. Storing a
|
||||
# SIGNATURE (not a bare bool) means any later model/provider change — via
|
||||
# /api/chat/start, /api/session/update, normalization, or provider repair
|
||||
# — automatically invalidates the pick (the signatures no longer match),
|
||||
# so a stale first-party leftover (#433) is never wrongly preserved.
|
||||
# Restored from persisted metadata on load (arrives via **kwargs).
|
||||
self.model_explicit_pick_signature = kwargs.get('model_explicit_pick_signature') or None
|
||||
self.messages = messages or []
|
||||
self.tool_calls = tool_calls or []
|
||||
self.created_at = created_at or time.time()
|
||||
@@ -1255,7 +1280,7 @@ class Session:
|
||||
# without parsing the full messages array (which may be 400KB+).
|
||||
# Fields are listed in the order they should appear in the JSON file.
|
||||
METADATA_FIELDS = [
|
||||
'session_id', 'title', 'workspace', 'model', 'model_provider', 'created_at', 'updated_at',
|
||||
'session_id', 'title', 'workspace', 'model', 'model_provider', 'model_explicit_pick_signature', 'created_at', 'updated_at',
|
||||
'pinned', 'archived', 'project_id', 'profile',
|
||||
'input_tokens', 'output_tokens', 'estimated_cost',
|
||||
'cache_read_tokens', 'cache_write_tokens',
|
||||
|
||||
@@ -21680,6 +21680,20 @@ def _handle_chat_start(handler, body, diag=None):
|
||||
profile_config=_pp_cfg,
|
||||
explicit_model_pick=explicit_model_pick,
|
||||
)
|
||||
# #5979: record a SIGNATURE of the deliberately-picked model+provider so
|
||||
# the streaming resolver can preserve a custom-proxy vendor namespace on a
|
||||
# cold catalog — but ONLY while the routing context still matches. On a
|
||||
# fresh explicit pick, stamp the signature of the resolved model+provider;
|
||||
# otherwise leave any prior signature in place (it self-invalidates when
|
||||
# the model/provider changes, since the streaming side recomputes and
|
||||
# compares). This survives same-model follow-up sends (the onchange marker
|
||||
# is one-shot) yet can't outlive a real switch.
|
||||
try:
|
||||
if explicit_model_pick:
|
||||
from api.models import model_explicit_pick_signature as _mk_sig
|
||||
s.model_explicit_pick_signature = _mk_sig(model, model_provider)
|
||||
except Exception:
|
||||
pass
|
||||
catalog_profile_provider = _pp_provider
|
||||
if catalog_profile_provider is None and isinstance(_pp_cfg, dict):
|
||||
profile_model_config = _pp_cfg.get("model") or {}
|
||||
|
||||
+41
-3
@@ -37,6 +37,7 @@ from api.config import (
|
||||
resolve_model_provider,
|
||||
resolve_custom_provider_connection,
|
||||
model_with_provider_context,
|
||||
warm_models_catalog_provenance_if_cold,
|
||||
load_settings,
|
||||
parse_reasoning_effort,
|
||||
coerce_reasoning_effort_for_model,
|
||||
@@ -7890,9 +7891,46 @@ def _run_agent_streaming(
|
||||
# Initialize SessionDB so session_search works in WebUI sessions
|
||||
_state_db_path = (Path(_profile_home) / "state.db") if _profile_home else None
|
||||
_session_db = _build_session_db_for_stream(_state_db_path)
|
||||
resolved_model, resolved_provider, resolved_base_url = resolve_model_provider(
|
||||
model_with_provider_context(model, provider_context)
|
||||
)
|
||||
# #5979: publish catalog provenance from the durable disk cache when
|
||||
# memory is cold, so the custom-proxy resolver below sees the
|
||||
# endpoint-advertised model ids (non-blocking, disk-only, never
|
||||
# live-rebuilds). Both the warm and the resolve read profile-keyed
|
||||
# config (cache path + source fingerprint via get_active_profile_name),
|
||||
# but this streaming worker is a separate thread that does NOT inherit
|
||||
# the HTTP handler's request-profile TLS — without binding it, a cold
|
||||
# send from a NAMED profile would resolve against the DEFAULT profile's
|
||||
# config and route to the wrong provider/base_url. Bind the captured
|
||||
# owning-session profile across warm + resolve so both see the right
|
||||
# profile (no-op for the default/root profile).
|
||||
from api import profiles as profiles_api
|
||||
# #5979: treat this send as a deliberate pick ONLY when the persisted
|
||||
# explicit-pick signature matches the CURRENT model+provider routing
|
||||
# context. Storing/comparing a signature (not a bare bool) means a
|
||||
# later model/provider change via /api/chat/start, /api/session/update,
|
||||
# normalization, or provider repair automatically invalidates a stale
|
||||
# pick — so a #433 first-party leftover is never wrongly preserved on
|
||||
# a cold catalog. Only affects the cold custom-proxy branch; warm
|
||||
# endpoint-advertised provenance always wins over this flag.
|
||||
from api.models import model_explicit_pick_signature as _mk_sig
|
||||
_picked_sig = getattr(s, "model_explicit_pick_signature", None)
|
||||
# Compare against the session's persisted model+provider — the exact
|
||||
# fields /api/chat/start stamped the signature from (it persists the
|
||||
# resolved model+provider onto the session before dispatch). Falls
|
||||
# back to the worker's model/provider_context if the session fields
|
||||
# are unset. A mismatch (any later model/provider change) yields a
|
||||
# different signature → treated as NOT a deliberate pick.
|
||||
_sig_model = getattr(s, "model", None) or model
|
||||
_sig_provider = getattr(s, "model_provider", None) or provider_context
|
||||
_current_sig = _mk_sig(_sig_model, _sig_provider)
|
||||
_explicitly_picked = bool(_picked_sig) and _picked_sig == _current_sig
|
||||
with profiles_api.profile_scope_for_detached_worker(
|
||||
_resolved_profile_name, "model resolution", logger_override=logger
|
||||
):
|
||||
warm_models_catalog_provenance_if_cold()
|
||||
resolved_model, resolved_provider, resolved_base_url = resolve_model_provider(
|
||||
model_with_provider_context(model, provider_context),
|
||||
explicitly_picked=_explicitly_picked,
|
||||
)
|
||||
configured_base_url = resolved_base_url
|
||||
|
||||
# Resolve API key via Hermes runtime provider (matches gateway behaviour).
|
||||
|
||||
@@ -156,7 +156,7 @@ def test_same_session_profile_switch_rebuilds_agent_under_new_soul_home(tmp_path
|
||||
monkeypatch.setattr(
|
||||
streaming,
|
||||
"resolve_model_provider",
|
||||
lambda _model: ("test-model", "test-provider", None),
|
||||
lambda _model, **_kw: ("test-model", "test-provider", None),
|
||||
)
|
||||
monkeypatch.setattr(streaming, "_maybe_schedule_title_refresh", lambda *args, **kwargs: None)
|
||||
monkeypatch.setattr(profiles, "get_hermes_home_for_profile", home_for_profile)
|
||||
|
||||
@@ -129,7 +129,7 @@ def test_streaming_sessiondb_uses_session_profile_state_db(tmp_path, monkeypatch
|
||||
monkeypatch.setattr(
|
||||
streaming,
|
||||
"resolve_model_provider",
|
||||
lambda _model: ("test-model", "test-provider", None),
|
||||
lambda _model, **_kw: ("test-model", "test-provider", None),
|
||||
)
|
||||
monkeypatch.setattr(streaming, "_maybe_schedule_title_refresh", lambda *args, **kwargs: None)
|
||||
monkeypatch.setattr(profiles, "get_hermes_home_for_profile", lambda _profile: profile_home)
|
||||
|
||||
+362
-29
@@ -7,7 +7,8 @@ import pytest
|
||||
import api.config as config
|
||||
|
||||
|
||||
def _resolve_with_config(model_id, provider=None, base_url=None, default=None, custom_providers=None):
|
||||
def _resolve_with_config(model_id, provider=None, base_url=None, default=None, custom_providers=None,
|
||||
explicitly_picked=False):
|
||||
"""Helper: temporarily set config.cfg model/custom provider sections, call resolve, restore."""
|
||||
old_cfg = dict(config.cfg)
|
||||
model_cfg = {}
|
||||
@@ -21,14 +22,14 @@ def _resolve_with_config(model_id, provider=None, base_url=None, default=None, c
|
||||
if custom_providers is not None:
|
||||
config.cfg['custom_providers'] = custom_providers
|
||||
try:
|
||||
return config.resolve_model_provider(model_id)
|
||||
return config.resolve_model_provider(model_id, explicitly_picked=explicitly_picked)
|
||||
finally:
|
||||
config.cfg.clear()
|
||||
config.cfg.update(old_cfg)
|
||||
|
||||
|
||||
def _resolve_with_catalog(model_id, advertised_ids, *, provider=None, base_url=None,
|
||||
provider_id='custom', default=None):
|
||||
provider_id='custom', default=None, explicitly_picked=False):
|
||||
"""Resolve with a seeded models-catalog snapshot (#5979 provenance).
|
||||
|
||||
``advertised_ids`` is the list of model ids the endpoint's own group
|
||||
@@ -57,7 +58,8 @@ def _resolve_with_catalog(model_id, advertised_ids, *, provider=None, base_url=N
|
||||
config._advertised_model_ids_memo = None # force recompute against the seeded snapshot
|
||||
config._sync_models_cache_provenance() # publish the atomic (snapshot, fingerprint) pair
|
||||
try:
|
||||
return _resolve_with_config(model_id, provider=provider, base_url=base_url, default=default)
|
||||
return _resolve_with_config(model_id, provider=provider, base_url=base_url, default=default,
|
||||
explicitly_picked=explicitly_picked)
|
||||
finally:
|
||||
config._available_models_cache = old_cache
|
||||
config._advertised_model_ids_memo = old_memo
|
||||
@@ -300,28 +302,36 @@ def test_named_custom_slug_preserves_advertised_full_id_5979():
|
||||
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_falls_back_to_legacy_heuristic_5979():
|
||||
"""#5979 tri-state: with a COLD/unbuilt catalog AND no config declaration
|
||||
(no provenance at all), resolution falls back to the LEGACY family heuristic
|
||||
so this narrow edge is never worse than the pre-fix behaviour.
|
||||
def test_custom_remote_cold_catalog_explicit_pick_preserves_5979():
|
||||
"""#5979 (reopened): with a COLD catalog and no config declaration, an
|
||||
EXPLICITLY-PICKED custom-proxy id is preserved verbatim (the user chose it;
|
||||
the proxy routes on it), while an UNMARKED id gets the legacy strip.
|
||||
|
||||
A first-party redundant prefix still strips (the #433 relay keeps working
|
||||
cold), while an intrinsic/unknown prefix is preserved. The #5979 active-user
|
||||
path never reaches this branch — a selected id is either config-declared
|
||||
(see the config-declared test) or in the catalog the dropdown was built from.
|
||||
This is the persisted-explicit-pick resolution (Codex's mechanism, Nathan's
|
||||
call): the cold decision is no longer an unconditional preserve — it is
|
||||
gated on whether the user deliberately selected the model this session.
|
||||
"""
|
||||
# first-party redundant prefix → strip (matches pre-fix / #433 cold path)
|
||||
model_a, _, _ = _resolve_with_catalog(
|
||||
# explicitly picked → PRESERVE even though grok graduated into first-party x-ai
|
||||
picked, _, _ = _resolve_with_catalog(
|
||||
'openai/gpt-5.4', advertised_ids=None,
|
||||
provider='custom', base_url='https://router.example.com/v1',
|
||||
explicitly_picked=True,
|
||||
)
|
||||
assert model_a == 'gpt-5.4', f"cold first-party prefix must strip (legacy), got {model_a!r}"
|
||||
# intrinsic/unknown prefix → preserve
|
||||
model_b, _, _ = _resolve_with_catalog(
|
||||
assert picked == 'openai/gpt-5.4', f"explicit pick must preserve cold, got {picked!r}"
|
||||
# NOT picked (stale leftover) → legacy strip keeps #433 relay routing cold
|
||||
stale, _, _ = _resolve_with_catalog(
|
||||
'openai/gpt-5.4', advertised_ids=None,
|
||||
provider='custom', base_url='https://router.example.com/v1',
|
||||
explicitly_picked=False,
|
||||
)
|
||||
assert stale == 'gpt-5.4', f"unmarked stale id must strip cold (legacy #433), got {stale!r}"
|
||||
# intrinsic/unknown prefix → preserve regardless (no first-party family match)
|
||||
unknown, _, _ = _resolve_with_catalog(
|
||||
'zai-org/GLM-5.1', advertised_ids=None,
|
||||
provider='custom', base_url='https://api.deepinfra.com/v1/openai',
|
||||
explicitly_picked=False,
|
||||
)
|
||||
assert model_b == 'zai-org/GLM-5.1', f"cold unknown prefix must preserve, got {model_b!r}"
|
||||
assert unknown == 'zai-org/GLM-5.1', f"cold unknown prefix must preserve, got {unknown!r}"
|
||||
|
||||
|
||||
def test_custom_remote_config_declared_full_id_preserved_cold_5979():
|
||||
@@ -405,36 +415,40 @@ def test_custom_remote_foreign_profile_catalog_ignored_5979():
|
||||
"""Profile-isolation fail-safe: when the catalog snapshot's source
|
||||
fingerprint does NOT match the current runtime (a concurrently-active
|
||||
foreign profile published it), that snapshot is NOT trusted for provenance —
|
||||
resolution falls back to the legacy family heuristic instead of stripping
|
||||
against another profile's catalog.
|
||||
the id is preserved verbatim instead of stripped against another profile's
|
||||
catalog.
|
||||
|
||||
Proof id: ``zai-org/GLM-5.1``. The foreign snapshot advertises a bare
|
||||
``GLM-5.1`` (which, if trusted, would strip the prefix), but ``zai-org`` is
|
||||
NOT a first-party provider, so the legacy fallback preserves the full id.
|
||||
A result of ``zai-org/GLM-5.1`` therefore proves the foreign catalog was
|
||||
ignored (a trusted-catalog strip would have returned ``GLM-5.1``).
|
||||
Proof id: ``openai/gpt-5.4`` with the FOREIGN catalog advertising ONLY the
|
||||
bare ``gpt-5.4``, resolved as an EXPLICIT pick. If the foreign catalog were
|
||||
(wrongly) trusted, the bare-only-advertised rule (warm provenance, which
|
||||
wins over the pick flag) would strip → ``gpt-5.4``. Because the fingerprint
|
||||
mismatches, the snapshot is ignored and resolution takes the cold branch;
|
||||
with the explicit-pick flag set that preserves → ``openai/gpt-5.4``. The two
|
||||
outcomes genuinely diverge, so ``openai/gpt-5.4`` proves the foreign catalog
|
||||
was ignored.
|
||||
"""
|
||||
old_cache = config._available_models_cache
|
||||
old_memo = config._advertised_model_ids_memo
|
||||
old_fp = config._available_models_cache_source_fingerprint
|
||||
old_prov = config._models_cache_provenance
|
||||
config._available_models_cache = {
|
||||
'groups': [{'provider_id': 'custom', 'models': [{'id': 'GLM-5.1', 'label': 'GLM-5.1'}]}]
|
||||
'groups': [{'provider_id': 'custom', 'models': [{'id': 'gpt-5.4', 'label': 'gpt-5.4'}]}]
|
||||
}
|
||||
config._available_models_cache_source_fingerprint = {'config_yaml': {'path': '/some/other/profile'}}
|
||||
config._advertised_model_ids_memo = None
|
||||
config._sync_models_cache_provenance()
|
||||
try:
|
||||
model, _, _ = _resolve_with_config(
|
||||
'zai-org/GLM-5.1', provider='custom', base_url='https://relay.example/v1',
|
||||
'openai/gpt-5.4', provider='custom', base_url='https://relay.example/v1',
|
||||
explicitly_picked=True,
|
||||
)
|
||||
finally:
|
||||
config._available_models_cache = old_cache
|
||||
config._advertised_model_ids_memo = old_memo
|
||||
config._available_models_cache_source_fingerprint = old_fp
|
||||
config._models_cache_provenance = old_prov
|
||||
assert model == 'zai-org/GLM-5.1', (
|
||||
f"foreign-profile catalog must be ignored (legacy fallback preserves), got {model!r}"
|
||||
assert model == 'openai/gpt-5.4', (
|
||||
f"foreign-profile catalog must be ignored (cold explicit-pick preserve), got {model!r}"
|
||||
)
|
||||
|
||||
|
||||
@@ -484,6 +498,242 @@ def test_resolver_provenance_read_does_not_block_on_cache_lock_5979():
|
||||
config.invalidate_models_cache()
|
||||
|
||||
|
||||
def test_b3nw_cold_nondeclared_custom_slug_preserves_full_id_5979():
|
||||
"""#5979 (reopened, b3nw's exact case): a NON-declared model actively
|
||||
selected on a ``custom:<slug>`` proxy is preserved on a COLD catalog.
|
||||
|
||||
b3nw's config: ``provider: custom:llm-proxy``, ``model.default: x-ai/grok-4.5``,
|
||||
NO ``models[]`` allowlist. He picks ``x-ai/grok-composer-2.5-fast`` in the UI
|
||||
(not his default). On a cold send the old code stripped it to
|
||||
``grok-composer-2.5-fast`` via the legacy family heuristic (the model had
|
||||
graduated into the x-ai first-party catalog), and his proxy 400'd. The flip
|
||||
to cold-preserve fixes it. grok-composer-2.5-fast is stubbed into
|
||||
``_PROVIDER_MODELS['x-ai']`` (as test_custom_remote_preserves_advertised_full_id_5979
|
||||
does) so the first-party trigger is reproduced deterministically regardless
|
||||
of the agent catalog version.
|
||||
"""
|
||||
mid = 'x-ai/grok-composer-2.5-fast'
|
||||
bare = 'grok-composer-2.5-fast'
|
||||
xai = list(config._PROVIDER_MODELS.get('x-ai') or [])
|
||||
had = any(isinstance(m, dict) and m.get('id') == bare for m in xai)
|
||||
old_xai = config._PROVIDER_MODELS.get('x-ai')
|
||||
old_cfg = dict(config.cfg)
|
||||
old_prov = config._models_cache_provenance
|
||||
old_cache = config._available_models_cache
|
||||
if not had:
|
||||
config._PROVIDER_MODELS['x-ai'] = xai + [{'id': bare, 'label': 'Grok Composer 2.5 Fast'}]
|
||||
config.cfg.clear()
|
||||
config.cfg.update({
|
||||
'model': {'default': 'x-ai/grok-4.5', 'provider': 'custom:llm-proxy',
|
||||
'base_url': 'https://proxy.example/v1'},
|
||||
'custom_providers': [{'name': 'llm-proxy', 'base_url': 'https://proxy.example/v1',
|
||||
'key_env': 'LLM_PROXY_API_KEY'}],
|
||||
})
|
||||
config._available_models_cache = None # COLD
|
||||
config._advertised_model_ids_memo = None
|
||||
config._sync_models_cache_provenance()
|
||||
try:
|
||||
assert config._is_first_party_model('x-ai', bare), "precondition: stub failed"
|
||||
model, provider, _ = config.resolve_model_provider(mid, explicitly_picked=True)
|
||||
finally:
|
||||
if not had:
|
||||
if old_xai is None:
|
||||
config._PROVIDER_MODELS.pop('x-ai', None)
|
||||
else:
|
||||
config._PROVIDER_MODELS['x-ai'] = old_xai
|
||||
config.cfg.clear()
|
||||
config.cfg.update(old_cfg)
|
||||
config._models_cache_provenance = old_prov
|
||||
config._available_models_cache = old_cache
|
||||
assert model == mid, f"b3nw's non-declared cold custom-proxy pick must preserve, got {model!r}"
|
||||
assert provider == 'custom:llm-proxy'
|
||||
|
||||
|
||||
def test_custom_slug_cold_stale_not_picked_still_strips_5979():
|
||||
"""Companion to b3nw's case: the SAME cold custom:<slug> shape, but the id is
|
||||
NOT explicitly picked (a stale first-party leftover), still gets the legacy
|
||||
strip — so #433-style stale sessions keep routing when cold and only
|
||||
deliberate picks are preserved verbatim.
|
||||
"""
|
||||
mid = 'x-ai/grok-composer-2.5-fast'
|
||||
bare = 'grok-composer-2.5-fast'
|
||||
xai = list(config._PROVIDER_MODELS.get('x-ai') or [])
|
||||
had = any(isinstance(m, dict) and m.get('id') == bare for m in xai)
|
||||
old_xai = config._PROVIDER_MODELS.get('x-ai')
|
||||
old_cfg = dict(config.cfg)
|
||||
old_prov = config._models_cache_provenance
|
||||
old_cache = config._available_models_cache
|
||||
if not had:
|
||||
config._PROVIDER_MODELS['x-ai'] = xai + [{'id': bare, 'label': 'Grok Composer 2.5 Fast'}]
|
||||
config.cfg.clear()
|
||||
config.cfg.update({
|
||||
'model': {'default': 'x-ai/grok-4.5', 'provider': 'custom:llm-proxy',
|
||||
'base_url': 'https://proxy.example/v1'},
|
||||
'custom_providers': [{'name': 'llm-proxy', 'base_url': 'https://proxy.example/v1',
|
||||
'key_env': 'LLM_PROXY_API_KEY'}],
|
||||
})
|
||||
config._available_models_cache = None # COLD
|
||||
config._advertised_model_ids_memo = None
|
||||
config._sync_models_cache_provenance()
|
||||
try:
|
||||
model, _, _ = config.resolve_model_provider(mid, explicitly_picked=False)
|
||||
finally:
|
||||
if not had:
|
||||
if old_xai is None:
|
||||
config._PROVIDER_MODELS.pop('x-ai', None)
|
||||
else:
|
||||
config._PROVIDER_MODELS['x-ai'] = old_xai
|
||||
config.cfg.clear()
|
||||
config.cfg.update(old_cfg)
|
||||
config._models_cache_provenance = old_prov
|
||||
config._available_models_cache = old_cache
|
||||
assert model == bare, f"unmarked stale cold custom:slug id must strip (legacy), got {model!r}"
|
||||
|
||||
|
||||
def test_warm_models_catalog_provenance_if_cold_publishes_from_disk_5979():
|
||||
"""The send-path warm helper publishes provenance from a valid disk cache
|
||||
when memory is cold — restoring the endpoint-advertised signal so #433
|
||||
strips and #5979 preserves — WITHOUT a live rebuild.
|
||||
"""
|
||||
old_cfg = dict(config.cfg)
|
||||
config.cfg.clear()
|
||||
config.cfg.update({
|
||||
'model': {'default': 'x-ai/grok-4.5', 'provider': 'custom:llm-proxy',
|
||||
'base_url': 'https://proxy.example/v1'},
|
||||
'custom_providers': [{'name': 'llm-proxy', 'base_url': 'https://proxy.example/v1',
|
||||
'key_env': 'LLM_PROXY_API_KEY',
|
||||
'models': ['x-ai/grok-4.5', 'x-ai/grok-composer-2.5-fast']}],
|
||||
})
|
||||
try:
|
||||
# Build + persist to disk, then simulate a cold memory cache (restart).
|
||||
config.invalidate_models_cache()
|
||||
config.get_available_models() # publishes to memory + disk
|
||||
assert config._models_cache_provenance is not None
|
||||
# Simulate cold memory but valid disk cache (do NOT delete disk).
|
||||
config._available_models_cache = None
|
||||
config._advertised_model_ids_memo = None
|
||||
config._sync_models_cache_provenance()
|
||||
assert config._models_cache_provenance is None, "precondition: memory cold"
|
||||
# The warm helper should republish provenance from disk (network-free).
|
||||
config.warm_models_catalog_provenance_if_cold()
|
||||
assert config._models_cache_provenance is not None, (
|
||||
"warm helper must publish provenance from the disk cache"
|
||||
)
|
||||
adv = config._endpoint_advertised_model_ids('custom:llm-proxy')
|
||||
assert adv and 'x-ai/grok-composer-2.5-fast' in adv, (
|
||||
f"warmed provenance must carry the endpoint-advertised ids, got {adv!r}"
|
||||
)
|
||||
finally:
|
||||
config.cfg.clear()
|
||||
config.cfg.update(old_cfg)
|
||||
config.invalidate_models_cache()
|
||||
|
||||
|
||||
def test_warm_models_catalog_provenance_noop_when_already_warm_5979():
|
||||
"""The warm helper is a cheap no-op when provenance for the CURRENT profile
|
||||
is already published — it must not reload or mutate a warm catalog.
|
||||
"""
|
||||
# Use the CURRENT runtime fingerprint so the profile-match fast path is hit.
|
||||
current_fp = config._models_cache_source_fingerprint()
|
||||
sentinel = ({'groups': []}, current_fp)
|
||||
old_prov = config._models_cache_provenance
|
||||
config._models_cache_provenance = sentinel
|
||||
try:
|
||||
config.warm_models_catalog_provenance_if_cold()
|
||||
assert config._models_cache_provenance is sentinel, (
|
||||
"warm helper must not touch already-warm current-profile provenance"
|
||||
)
|
||||
finally:
|
||||
config._models_cache_provenance = old_prov
|
||||
|
||||
|
||||
def test_warm_models_catalog_provenance_ignores_foreign_profile_resident_5979(monkeypatch):
|
||||
"""Profile isolation (Codex finding): a FOREIGN profile's resident provenance
|
||||
must NOT satisfy the fast no-op. The helper falls through, loads THIS
|
||||
profile's own disk snapshot, and republishes provenance stamped with the
|
||||
current runtime fingerprint — so a concurrently-active profile's catalog can
|
||||
never block this profile from warming its own.
|
||||
"""
|
||||
foreign = ({'groups': [{'provider_id': 'custom', 'models': [{'id': 'gpt-5.4'}]}]},
|
||||
{'config_yaml': {'path': '/some/other/profile'}})
|
||||
my_disk = {'groups': [{'provider_id': 'custom:llm-proxy',
|
||||
'models': [{'id': 'x-ai/grok-composer-2.5-fast'}]}]}
|
||||
old_prov = config._models_cache_provenance
|
||||
old_cache = config._available_models_cache
|
||||
old_fp = config._available_models_cache_source_fingerprint
|
||||
old_memo = config._advertised_model_ids_memo
|
||||
config._models_cache_provenance = foreign
|
||||
monkeypatch.setattr(config, '_load_models_cache_from_disk', lambda: my_disk)
|
||||
try:
|
||||
config.warm_models_catalog_provenance_if_cold()
|
||||
prov = config._models_cache_provenance
|
||||
assert prov is not None and prov[0] is my_disk, (
|
||||
"warm must replace foreign provenance with THIS profile's disk snapshot"
|
||||
)
|
||||
assert prov[1] == config._models_cache_source_fingerprint(), (
|
||||
"republished provenance must carry the current profile's fingerprint"
|
||||
)
|
||||
finally:
|
||||
config._models_cache_provenance = old_prov
|
||||
config._available_models_cache = old_cache
|
||||
config._available_models_cache_source_fingerprint = old_fp
|
||||
config._advertised_model_ids_memo = old_memo
|
||||
|
||||
|
||||
def test_warm_models_catalog_provenance_never_live_rebuilds_5979(monkeypatch):
|
||||
"""The warm helper must NEVER call get_available_models (which can acquire
|
||||
the cache lock and block on / trigger a rebuild). It reads the disk cache
|
||||
directly, so a live provider probe is structurally impossible on the send
|
||||
path.
|
||||
"""
|
||||
old_prov = config._models_cache_provenance
|
||||
config._models_cache_provenance = None
|
||||
called = {'get_available_models': False}
|
||||
def _boom(*a, **k):
|
||||
called['get_available_models'] = True
|
||||
raise AssertionError("warm must not call get_available_models")
|
||||
monkeypatch.setattr(config, 'get_available_models', _boom)
|
||||
try:
|
||||
config.warm_models_catalog_provenance_if_cold() # must not raise / call the above
|
||||
finally:
|
||||
config._models_cache_provenance = old_prov
|
||||
assert called['get_available_models'] is False, (
|
||||
"warm helper must read disk directly, never via get_available_models"
|
||||
)
|
||||
|
||||
|
||||
def test_warm_models_catalog_provenance_nonblocking_when_lock_held_5979():
|
||||
"""The warm helper must NOT block on the models-cache lock. It tries the
|
||||
lock non-blocking and returns immediately when a concurrent build/publish
|
||||
holds it, so a send can never hang up to 60s behind a rebuild.
|
||||
"""
|
||||
import threading
|
||||
import time as _time
|
||||
old_prov = config._models_cache_provenance
|
||||
config._models_cache_provenance = None
|
||||
got = config._available_models_cache_lock.acquire(blocking=False)
|
||||
assert got, "precondition: could not take cache lock"
|
||||
try:
|
||||
result = {}
|
||||
def _worker():
|
||||
t0 = _time.time()
|
||||
config.warm_models_catalog_provenance_if_cold()
|
||||
result['elapsed'] = _time.time() - t0
|
||||
th = threading.Thread(target=_worker)
|
||||
th.start()
|
||||
th.join(timeout=5)
|
||||
blocked = th.is_alive()
|
||||
finally:
|
||||
config._available_models_cache_lock.release()
|
||||
config._models_cache_provenance = old_prov
|
||||
if blocked:
|
||||
th.join(timeout=5)
|
||||
assert not blocked, "warm helper blocked on a held cache lock (must be non-blocking)"
|
||||
assert result.get('elapsed', 99) < 2.0, (
|
||||
f"warm helper should return promptly when lock held, took {result.get('elapsed')!r}s"
|
||||
)
|
||||
|
||||
|
||||
def test_custom_remote_preserves_unknown_prefix_548():
|
||||
"""#548: an unknown vendor prefix (zai-org/GLM-5.1) is always preserved.
|
||||
|
||||
@@ -1207,3 +1457,86 @@ def test_providers_scan_active_own_providers_entry_owns_over_other_slug_5511():
|
||||
f"another slug's overlapping entry; got {provider!r}"
|
||||
)
|
||||
assert base_url == 'https://my.example/v1'
|
||||
|
||||
|
||||
# ── #5979: explicit-pick signature lifecycle (Codex-flagged session-update hole) ──
|
||||
|
||||
def test_explicit_pick_signature_matches_same_context_5979():
|
||||
"""The signature matches when model+provider are unchanged (deliberate pick
|
||||
is honored across same-model follow-up sends)."""
|
||||
import api.models as models
|
||||
sig = models.model_explicit_pick_signature
|
||||
picked = sig('x-ai/grok-composer-2.5-fast', 'custom:llm-proxy')
|
||||
assert picked == sig('x-ai/grok-composer-2.5-fast', 'custom:llm-proxy')
|
||||
|
||||
|
||||
def test_explicit_pick_signature_invalidated_on_model_change_5979():
|
||||
"""A model change (e.g. via /api/session/update) yields a DIFFERENT signature,
|
||||
so a stale explicit-pick can't wrongly preserve a #433 leftover on cold."""
|
||||
import api.models as models
|
||||
sig = models.model_explicit_pick_signature
|
||||
picked = sig('x-ai/grok-composer-2.5-fast', 'custom:llm-proxy')
|
||||
# switched to a stale first-party id on the same proxy → signature differs
|
||||
assert picked != sig('openai/gpt-5.4', 'custom:llm-proxy')
|
||||
# provider switch also differs
|
||||
assert picked != sig('x-ai/grok-composer-2.5-fast', 'openai')
|
||||
|
||||
|
||||
def test_explicit_pick_signature_persists_round_trip_5979():
|
||||
"""Session.model_explicit_pick_signature survives save/reload (b3nw's cold
|
||||
restart scenario), and defaults to None when absent."""
|
||||
import api.models as models
|
||||
s = models.Session(session_id='sig5979', model='x-ai/grok-composer-2.5-fast',
|
||||
model_provider='custom:llm-proxy')
|
||||
s.model_explicit_pick_signature = models.model_explicit_pick_signature(
|
||||
'x-ai/grok-composer-2.5-fast', 'custom:llm-proxy')
|
||||
data = {k: getattr(s, k, None) for k in
|
||||
['session_id', 'title', 'workspace', 'model', 'model_provider',
|
||||
'model_explicit_pick_signature']}
|
||||
s2 = models.Session(**data)
|
||||
assert s2.model_explicit_pick_signature == s.model_explicit_pick_signature
|
||||
# absent → None (unmarked)
|
||||
s3 = models.Session(session_id='nopick', model='y')
|
||||
assert s3.model_explicit_pick_signature is None
|
||||
|
||||
|
||||
def test_streaming_explicitly_picked_computation_5979():
|
||||
"""End-to-end lifecycle (Fable fast-follow): reproduce the exact
|
||||
signature-comparison the streaming worker does at api/streaming.py, proving
|
||||
the persisted signature drives explicitly_picked correctly across the
|
||||
deliberate-pick, stale-after-update, and never-picked cases.
|
||||
|
||||
Mirrors streaming.py:
|
||||
_picked_sig = s.model_explicit_pick_signature
|
||||
_current_sig = mk_sig(s.model or model, s.model_provider or provider_context)
|
||||
_explicitly_picked = bool(_picked_sig) and _picked_sig == _current_sig
|
||||
"""
|
||||
import api.models as models
|
||||
mk = models.model_explicit_pick_signature
|
||||
|
||||
def _explicitly_picked(session):
|
||||
picked = getattr(session, 'model_explicit_pick_signature', None)
|
||||
cur = mk(getattr(session, 'model', None), getattr(session, 'model_provider', None))
|
||||
return bool(picked) and picked == cur
|
||||
|
||||
# 1) deliberate pick, unchanged context → honored
|
||||
s = models.Session(session_id='e2e1', model='x-ai/grok-composer-2.5-fast',
|
||||
model_provider='custom:llm-proxy')
|
||||
s.model_explicit_pick_signature = mk('x-ai/grok-composer-2.5-fast', 'custom:llm-proxy')
|
||||
assert _explicitly_picked(s) is True
|
||||
|
||||
# 2) session/update switches the model WITHOUT restamping → stale pick ignored
|
||||
s.model = 'openai/gpt-5.4' # e.g. user switched; signature now stale
|
||||
assert _explicitly_picked(s) is False
|
||||
|
||||
# 3) provider switch without restamp → stale pick ignored
|
||||
s2 = models.Session(session_id='e2e2', model='x-ai/grok-composer-2.5-fast',
|
||||
model_provider='custom:llm-proxy')
|
||||
s2.model_explicit_pick_signature = mk('x-ai/grok-composer-2.5-fast', 'custom:llm-proxy')
|
||||
s2.model_provider = 'openai'
|
||||
assert _explicitly_picked(s2) is False
|
||||
|
||||
# 4) never picked → unmarked
|
||||
s3 = models.Session(session_id='e2e3', model='x-ai/grok-composer-2.5-fast',
|
||||
model_provider='custom:llm-proxy')
|
||||
assert _explicitly_picked(s3) is False
|
||||
Reference in new issue
Block a user