From 6745827eb5f5bdd5ca6b4a8c99837e802b3f16b4 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Mon, 13 Jul 2026 00:56:47 +0000 Subject: [PATCH 1/7] fix(#5979 follow-up): preserve custom-proxy vendor namespace on cold-catalog sends The earlier #5979 fix (PR #5980, exp-v0.52.53) resolved the WARM-catalog and config-declared cases, but b3nw confirmed truncation persists on exp-v0.52.55/56. Root cause (reproduced on master, confirmed by Codex + Fable): b3nw's config is provider: custom:llm-proxy, model.default: x-ai/grok-4.5, no models[] allowlist. His DEFAULT (x-ai/grok-4.5) preserves via config-declaration, but a model he SELECTS in the picker that isn't the default (x-ai/grok-composer-2.5-fast) truncates on every COLD send. The send path (api/streaming.py:7857) calls resolve_model_provider without warming the catalog, and the #1855 chat/start fast path deliberately skips the catalog when a session already has a persisted model+provider -- so 'cold at send' is the designed state after any restart / settings-save / cache-expiry, not a rare race. With provenance None and no config declaration, the #5980 tri-state fell through to the LEGACY family strip (if prefix in _PROVIDER_MODELS and _is_first_party_model: return bare), and grok-composer-2.5-fast had graduated into the x-ai first-party catalog -> stripped. Fix (two changes, both advisor-verified): 1. Flip the custom-arm cold default from family-strip to PRESERVE-verbatim (api/config.py). For an explicit custom / custom: proxy the strip now requires POSITIVE provenance (endpoint advertises only the bare id, #433). A wrong strip destroys routing info the user can't recover; a wrong preserve fails loudly and self-heals on warm. This also honours _endpoint_advertised_model_ids's own documented 'None -> preserve verbatim' contract and removes the data-driven flaw where a model graduating into the static catalog silently flips routing. 2. Network-free provenance warm on the send path (warm_models_catalog_provenance_if_cold, called in streaming.py before the resolve). prefer_cache=True republishes provenance from the durable disk cache (no live probe, no _cfg_lock held on the worker thread -> no lock-order deadlock), so #433's bare-only-strip stays exact whenever a disk cache exists. Residual (accepted, strictly better than master): a #433-shaped wrong-preserve now requires memory cold AND disk cache absent/invalid (first run, post-config-save window, schema/version bump). It fails loudly and heals on the next /api/models or session-visit rebuild. Under master b3nw's failure was the mirror image but silent, recurring every send, and unfixable short of declaring every model in config. Supersedes rodboev's #5996, which targeted the providers. arm and does not cover b3nw's actual custom:llm-proxy config (confirmed by Codex). Tests: rewrite the cold-fallback test (strip -> preserve), sharpen the foreign-profile test so trusted-strip vs ignored-preserve genuinely diverge, add b3nw's exact cold non-declared custom:slug case, and add three warm-helper tests (publishes-from-disk, noop-when-warm, never-live-rebuilds). 64 resolver+sprint40 tests pass; b3nw end-to-end verified; #433 positive-provenance strip verified. Thanks @b3nw for the CLI-vs-WebUI comparison that localized it. --- CHANGELOG.md | 2 + api/config.py | 62 ++++++++++-- api/streaming.py | 8 ++ tests/test_model_resolver.py | 176 ++++++++++++++++++++++++++++++----- 4 files changed, 217 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 97944c7e..0743df6e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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:` 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) diff --git a/api/config.py b/api/config.py index 8e6ac076..a407c1de 100644 --- a/api/config.py +++ b/api/config.py @@ -2898,15 +2898,24 @@ 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. - if prefix in _PROVIDER_MODELS and _is_first_party_model(prefix, bare): - return bare, config_provider, config_base_url + # PRESERVE the id verbatim. For an EXPLICIT ``custom`` / + # ``custom:`` proxy the safe default is to send exactly + # what the user selected — a wrong strip destroys a namespace + # the proxy routes on (recurs every turn, unrepairable by the + # user short of declaring every model in config), while a wrong + # preserve only mis-sends a stale cross-provider leftover that + # fails LOUDLY (the relay rejects it; #5940 surfaces the real + # error) and self-heals the instant the catalog warms. The strip + # (#433) now requires POSITIVE provenance that the endpoint + # advertises ONLY the bare id (case 2 above) — it is no longer + # the cold default. This also honours the documented contract + # of _endpoint_advertised_model_ids ("None → preserve verbatim") + # and removes the data-driven flaw where a model graduating into + # the static first-party catalog silently flipped routing + # (exactly how #5979 regressed). The send path additionally + # warms provenance network-free from the disk cache + # (warm_models_catalog_provenance_if_cold) so this cold branch + # is reached only in the narrow no-disk-cache window. 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 @@ -8162,6 +8171,41 @@ 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, network-free publish of catalog provenance when memory is cold. + + 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 + and #5979 full-id preserve is backed by real provenance rather than the + fail-safe default. + + Contract: ``prefer_cache=True`` resolves fresh-memory → valid-disk-cache + (which publishes provenance) → minimal static catalog (which deliberately + does NOT publish provenance, so a config-derived catalog can never + masquerade as endpoint evidence). It NEVER triggers a live provider rebuild, + so there is no network latency or flaky-network stall. Costs one global read + when already warm and a single ~sub-millisecond disk load once per cold + window. MUST be called from a request/worker context that does NOT hold + ``_cfg_lock`` (``get_available_models`` takes ``_available_models_cache_lock`` + and may ``reload_config()`` → ``_cfg_lock``; calling it under ``_cfg_lock`` + would invert the lock order). The send path satisfies this — it runs on the + detached worker thread with no config lock held. + """ + if _models_cache_provenance is not None: + return # already warm — one global read, no work + try: + get_available_models(prefer_cache=True) + except Exception: + logger.debug("models catalog provenance warm failed", exc_info=True) + + def get_available_models_for_session_visit() -> dict: """Return /api/models with a short session-visit freshness horizon. diff --git a/api/streaming.py b/api/streaming.py index 4bc663e1..27afbba1 100644 --- a/api/streaming.py +++ b/api/streaming.py @@ -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,6 +7891,13 @@ 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) + # #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 (network-free, never live-rebuilds, + # no _cfg_lock held on this worker thread). Without this the resolver + # falls to the cold-preserve default; with it, #433 bare-only-strip + # stays exact and #5979 full-id preserve is provenance-backed. + warm_models_catalog_provenance_if_cold() resolved_model, resolved_provider, resolved_base_url = resolve_model_provider( model_with_provider_context(model, provider_context) ) diff --git a/tests/test_model_resolver.py b/tests/test_model_resolver.py index 5c67d1e8..38f56b7e 100644 --- a/tests/test_model_resolver.py +++ b/tests/test_model_resolver.py @@ -300,23 +300,27 @@ 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_preserves_verbatim_5979(): + """#5979 (reopened): with a COLD/unbuilt catalog AND no config declaration, + an explicit custom proxy PRESERVES the id verbatim — the strip now requires + positive provenance (endpoint advertises only the bare id), it is no longer + the cold default. - 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 fix for b3nw's reopened case: a user's actively-selected + NON-default model on a ``custom:`` proxy was truncated on every cold + send because the old cold fallback ran the legacy family heuristic. A wrong + strip destroys a namespace the proxy routes on (recurs every turn, + unrepairable); a wrong preserve only mis-sends a stale leftover that fails + loudly and self-heals on the next catalog warm. So cold → preserve for BOTH + a first-party-looking prefix and an unknown one. """ - # first-party redundant prefix → strip (matches pre-fix / #433 cold path) + # first-party-looking prefix → PRESERVE (was: strip). This is the flip. model_a, _, _ = _resolve_with_catalog( 'openai/gpt-5.4', advertised_ids=None, provider='custom', base_url='https://router.example.com/v1', ) - assert model_a == 'gpt-5.4', f"cold first-party prefix must strip (legacy), got {model_a!r}" - # intrinsic/unknown prefix → preserve + assert model_a == 'openai/gpt-5.4', f"cold catalog must preserve verbatim, got {model_a!r}" + # intrinsic/unknown prefix → preserve (unchanged) model_b, _, _ = _resolve_with_catalog( 'zai-org/GLM-5.1', advertised_ids=None, provider='custom', base_url='https://api.deepinfra.com/v1/openai', @@ -405,36 +409,38 @@ 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``. If the foreign catalog were (wrongly) trusted, the + bare-only-advertised rule would strip → ``gpt-5.4``. Because the fingerprint + mismatches, the snapshot is ignored and resolution takes the cold-preserve + default → ``openai/gpt-5.4``. The two outcomes genuinely diverge, so a result + of ``openai/gpt-5.4`` proves the foreign catalog was ignored (not merely that + the prefix was unknown). """ 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', ) 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-preserve default), got {model!r}" ) @@ -484,6 +490,132 @@ 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:`` 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) + 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_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 (single global read) when provenance is + already published — it must not rebuild or mutate a warm catalog. + """ + sentinel = ({'groups': []}, {'config_yaml': {'path': '/x'}}) + 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 an already-warm provenance tuple" + ) + finally: + config._models_cache_provenance = old_prov + + +def test_warm_models_catalog_provenance_never_live_rebuilds_5979(monkeypatch): + """The warm helper must call get_available_models with prefer_cache=True so + it can never trigger a live provider probe on the send path. + """ + old_prov = config._models_cache_provenance + config._models_cache_provenance = None + captured = {} + def _fake_get_available_models(*, prefer_cache=False, force_refresh=False): + captured['prefer_cache'] = prefer_cache + captured['force_refresh'] = force_refresh + return {'groups': []} + monkeypatch.setattr(config, 'get_available_models', _fake_get_available_models) + try: + config.warm_models_catalog_provenance_if_cold() + finally: + config._models_cache_provenance = old_prov + assert captured.get('prefer_cache') is True, "warm must use prefer_cache=True (no live rebuild)" + assert captured.get('force_refresh') is False, "warm must never force_refresh" + + def test_custom_remote_preserves_unknown_prefix_548(): """#548: an unknown vendor prefix (zai-org/GLM-5.1) is always preserved. From e3152572aabf1b01f4256dc263bf37ed59239db8 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Mon, 13 Jul 2026 01:09:46 +0000 Subject: [PATCH 2/7] harden #5979 follow-up per Codex gate: non-blocking disk-only warm + profile scope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex gate found two real issues in the send-path warm: 1. Blocking lock wait (CORE): get_available_models(prefer_cache=True) still acquires _available_models_cache_lock and can wait up to ~60s (unbounded in synchronous rebuild mode) on an in-flight rebuild — unacceptable on the send hot path. Rewrote warm_models_catalog_provenance_if_cold as a genuinely non-blocking, disk-only publish: try the cache lock NON-BLOCKING (return immediately if busy — a concurrent build will publish provenance itself), read ONLY the on-disk cache via _load_models_cache_from_disk (no network, no rebuild), publish snapshot+fingerprint + _sync_models_cache_provenance(). Verified: returns in ~3.5ms while the lock is held (was a 60s block). 2. Profile scope (CORE): the streaming worker is a separate thread that does NOT inherit the HTTP handler's request-profile TLS. Both the warm and the resolve read profile-keyed config (cache path + source fingerprint via get_active_profile_name), so a cold send from a NAMED profile could resolve against the DEFAULT profile's config and route to the wrong provider/base_url (get_available_models reloads config on path mismatch). Wrapped warm + resolve in profile_scope_for_detached_worker(_resolved_profile_name) so both see the owning session's profile (no-op for default/root). Tests: rewrote never-live-rebuilds (now asserts warm NEVER calls get_available_models — reads disk directly) and added a non-blocking regression (warm returns <2s while the cache lock is held). 65 resolver+sprint40 tests pass; b3nw end-to-end + non-blocking + disk-warm all verified. --- api/config.py | 54 +++++++++++++++++++++++----------- api/streaming.py | 25 ++++++++++------ tests/test_model_resolver.py | 56 +++++++++++++++++++++++++++++------- 3 files changed, 100 insertions(+), 35 deletions(-) diff --git a/api/config.py b/api/config.py index a407c1de..d6487437 100644 --- a/api/config.py +++ b/api/config.py @@ -8172,7 +8172,7 @@ def _models_cache_file_age_seconds(cache_path: Path, now: float) -> float | None def warm_models_catalog_provenance_if_cold() -> None: - """Best-effort, network-free publish of catalog provenance when memory is cold. + """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 @@ -8182,28 +8182,50 @@ def warm_models_catalog_provenance_if_cold() -> None: 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 - and #5979 full-id preserve is backed by real provenance rather than the - fail-safe default. + signal from the durable disk cache so the #433 bare-only-strip stays exact. - Contract: ``prefer_cache=True`` resolves fresh-memory → valid-disk-cache - (which publishes provenance) → minimal static catalog (which deliberately - does NOT publish provenance, so a config-derived catalog can never - masquerade as endpoint evidence). It NEVER triggers a live provider rebuild, - so there is no network latency or flaky-network stall. Costs one global read - when already warm and a single ~sub-millisecond disk load once per cold - window. MUST be called from a request/worker context that does NOT hold - ``_cfg_lock`` (``get_available_models`` takes ``_available_models_cache_lock`` - and may ``reload_config()`` → ``_cfg_lock``; calling it under ``_cfg_lock`` - would invert the lock order). The send path satisfies this — it runs on the - detached worker thread with no config lock held. + 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. """ + global _available_models_cache, _available_models_cache_ts + global _available_models_cache_source_fingerprint if _models_cache_provenance is not None: return # already warm — 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: - get_available_models(prefer_cache=True) + if _models_cache_provenance is not None: + return # published 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 → stay cold, resolver preserves 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: diff --git a/api/streaming.py b/api/streaming.py index 27afbba1..f54a193b 100644 --- a/api/streaming.py +++ b/api/streaming.py @@ -7893,14 +7893,23 @@ def _run_agent_streaming( _session_db = _build_session_db_for_stream(_state_db_path) # #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 (network-free, never live-rebuilds, - # no _cfg_lock held on this worker thread). Without this the resolver - # falls to the cold-preserve default; with it, #433 bare-only-strip - # stays exact and #5979 full-id preserve is provenance-backed. - warm_models_catalog_provenance_if_cold() - resolved_model, resolved_provider, resolved_base_url = resolve_model_provider( - model_with_provider_context(model, provider_context) - ) + # 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 + 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) + ) configured_base_url = resolved_base_url # Resolve API key via Hermes runtime provider (matches gateway behaviour). diff --git a/tests/test_model_resolver.py b/tests/test_model_resolver.py index 38f56b7e..a17e647c 100644 --- a/tests/test_model_resolver.py +++ b/tests/test_model_resolver.py @@ -597,23 +597,57 @@ def test_warm_models_catalog_provenance_noop_when_already_warm_5979(): def test_warm_models_catalog_provenance_never_live_rebuilds_5979(monkeypatch): - """The warm helper must call get_available_models with prefer_cache=True so - it can never trigger a live provider probe on the send path. + """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 - captured = {} - def _fake_get_available_models(*, prefer_cache=False, force_refresh=False): - captured['prefer_cache'] = prefer_cache - captured['force_refresh'] = force_refresh - return {'groups': []} - monkeypatch.setattr(config, 'get_available_models', _fake_get_available_models) + 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() + config.warm_models_catalog_provenance_if_cold() # must not raise / call the above finally: config._models_cache_provenance = old_prov - assert captured.get('prefer_cache') is True, "warm must use prefer_cache=True (no live rebuild)" - assert captured.get('force_refresh') is False, "warm must never force_refresh" + 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(): From f42b4d14f55c31f9e2c909f1b9ae24f9c0834a09 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Mon, 13 Jul 2026 01:22:43 +0000 Subject: [PATCH 3/7] harden #5979 follow-up round 2 (Codex): profile-fingerprint-gated warm no-op MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex finding: the warm helper's fast no-op (if _models_cache_provenance is not None: return) ignored WHICH profile the resident provenance belongs to. The catalog globals are process-wide, so profile B's resident provenance could block profile A from loading A's own valid disk cache — A would then resolve against B's advertised ids (verified: profile A provenance resident + profile B bare-only disk cache -> A's openai/gpt-5.4 wrongly preserved). Fix: gate BOTH the fast no-op and the post-lock recheck on a current-profile fingerprint match (_provenance_is_current: prov[1] == _models_cache_source_fingerprint()). On mismatch the helper falls through, loads THIS profile's disk snapshot, and republishes provenance stamped with the current fingerprint. Tests: rewrote noop-when-warm to use the CURRENT runtime fingerprint (genuinely hits the fast path) and added ignores-foreign-profile-resident (foreign fp -> falls through -> loads this profile's disk snapshot). 66 resolver+sprint40 tests pass. NOTE: Codex also re-raised the cold+no-disk #433 case (a stale openai/gpt-5.4 on a bare-only relay preserves instead of strips when no catalog exists at all). Fable weighed this exact residual and judged cold-preserve strictly better than master (loud-fail-and-heal vs silent-recurring-truncation). The two advisors diverge; surfacing to Nathan rather than auto-adopting the heavier persisted-explicit-pick mechanism. --- api/config.py | 29 +++++++++++++++++++----- tests/test_model_resolver.py | 43 ++++++++++++++++++++++++++++++++---- 2 files changed, 63 insertions(+), 9 deletions(-) diff --git a/api/config.py b/api/config.py index d6487437..b9d29136 100644 --- a/api/config.py +++ b/api/config.py @@ -8198,23 +8198,42 @@ def warm_models_catalog_provenance_if_cold() -> None: (``_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 - if _models_cache_provenance is not None: - return # already warm — one global read, no work + + 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 _models_cache_provenance is not None: - return # published while we waited for the lock + 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 → stay cold, resolver preserves verbatim + return # no durable cache for this profile → stay cold, preserve verbatim _available_models_cache = disk_groups _available_models_cache_ts = time.monotonic() try: diff --git a/tests/test_model_resolver.py b/tests/test_model_resolver.py index a17e647c..bcafcbba 100644 --- a/tests/test_model_resolver.py +++ b/tests/test_model_resolver.py @@ -581,21 +581,56 @@ def test_warm_models_catalog_provenance_if_cold_publishes_from_disk_5979(): def test_warm_models_catalog_provenance_noop_when_already_warm_5979(): - """The warm helper is a cheap no-op (single global read) when provenance is - already published — it must not rebuild or mutate a warm catalog. + """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. """ - sentinel = ({'groups': []}, {'config_yaml': {'path': '/x'}}) + # 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 an already-warm provenance tuple" + "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 From 45cb5bd3e6c26a7988a4b28defbff52ce98de112 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Mon, 13 Jul 2026 01:33:51 +0000 Subject: [PATCH 4/7] feat(#5979): persisted explicit-pick provenance for cold custom-proxy resolution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nathan's call: adopt Codex's complete mechanism instead of the unconditional cold-preserve. The cold-catalog custom-proxy decision is now gated on whether the user DELIBERATELY selected the model this session, closing the residual where a stale #433 leftover would preserve+400 on a cold+no-disk send. Mechanism: - resolve_model_provider(model_id, *, explicitly_picked=False): in the custom-arm cold branch (no warm provenance, not config-declared), explicitly_picked=True preserves the vendor namespace verbatim (deliberate pick, proxy routes on it); otherwise the legacy redundant-prefix strip runs (stale cross-provider leftover, #433). Warm endpoint-advertised provenance still wins over the flag. - Session.model_explicitly_picked: new persisted field (default False, restored from metadata via **kwargs, listed in METADATA_FIELDS so it survives a restart — b3nw's exact cold scenario). - /api/chat/start stamps it: True on a fresh explicit_model_pick; cleared only when the resolved model actually CHANGED without a pick (real switch away); preserved across plain follow-up sends of the same model (the onchange marker is one-shot, so per-send explicit_model_pick alone would drop the intent). - streaming.py reads s.model_explicitly_picked and passes explicitly_picked= into resolve_model_provider (inside the profile scope). Independent end-to-end matrix (all pass): ben default cold, ben picked non-default cold+warm, ben stale cold (strips), #433 warm+cold, #3872 bedrock, #548 zai-org. 67 resolver+sprint40 tests pass incl. new explicit-pick + stale-companion + persistence round-trip. --- api/config.py | 54 +++++++++++------ api/models.py | 7 ++- api/routes.py | 26 +++++++++ api/streaming.py | 8 ++- tests/test_model_resolver.py | 109 +++++++++++++++++++++++++---------- 5 files changed, 154 insertions(+), 50 deletions(-) diff --git a/api/config.py b/api/config.py index b9d29136..7bba9db8 100644 --- a/api/config.py +++ b/api/config.py @@ -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,24 +2908,32 @@ 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). - # PRESERVE the id verbatim. For an EXPLICIT ``custom`` / - # ``custom:`` proxy the safe default is to send exactly - # what the user selected — a wrong strip destroys a namespace - # the proxy routes on (recurs every turn, unrepairable by the - # user short of declaring every model in config), while a wrong - # preserve only mis-sends a stale cross-provider leftover that - # fails LOUDLY (the relay rejects it; #5940 surfaces the real - # error) and self-heals the instant the catalog warms. The strip - # (#433) now requires POSITIVE provenance that the endpoint - # advertises ONLY the bare id (case 2 above) — it is no longer - # the cold default. This also honours the documented contract - # of _endpoint_advertised_model_ids ("None → preserve verbatim") - # and removes the data-driven flaw where a model graduating into + # 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: 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). The send path additionally - # warms provenance network-free from the disk cache - # (warm_models_catalog_provenance_if_cold) so this cold branch - # is reached only in the narrow no-disk-cache window. + # (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 # Non-custom first-party provider pointed at an OpenAI-compatible # proxy (e.g. provider=openai + base_url=litellm): the bare id is diff --git a/api/models.py b/api/models.py index cd78fa13..9187f0f0 100644 --- a/api/models.py +++ b/api/models.py @@ -1136,6 +1136,11 @@ 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: True when the user deliberately picked this session's model in + # the UI (persisted so the streaming resolver can preserve a custom-proxy + # vendor namespace on a cold catalog vs. stripping a stale leftover). + # Restored from the persisted metadata on load (arrives via **kwargs). + self.model_explicitly_picked = bool(kwargs.get('model_explicitly_picked', False)) self.messages = messages or [] self.tool_calls = tool_calls or [] self.created_at = created_at or time.time() @@ -1255,7 +1260,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_explicitly_picked', 'created_at', 'updated_at', 'pinned', 'archived', 'project_id', 'profile', 'input_tokens', 'output_tokens', 'estimated_cost', 'cache_read_tokens', 'cache_write_tokens', diff --git a/api/routes.py b/api/routes.py index 999b8caa..d48d348c 100644 --- a/api/routes.py +++ b/api/routes.py @@ -21680,6 +21680,32 @@ def _handle_chat_start(handler, body, diag=None): profile_config=_pp_cfg, explicit_model_pick=explicit_model_pick, ) + # #5979: persist whether the user DELIBERATELY selected this session's + # model, so the streaming worker's resolve_model_provider() can tell a + # deliberate custom-proxy pick (preserve the vendor namespace even on a + # cold catalog) from a stale cross-provider leftover (#433, strip). The + # resolve happens on a detached worker thread that only has the session + # in scope, so the intent is stamped here on the session. + # + # DURABILITY: the flag must survive follow-up sends that don't re-carry + # explicit_model_pick (the onchange marker is one-shot — messages.js + # consumes it after the first send). So we only set the flag from THIS + # send's explicit_model_pick when it's True, and otherwise CLEAR it only + # when the resolved model actually CHANGED from what was stored (a real + # switch away from the deliberately-picked model). A plain follow-up send + # of the same model keeps the prior intent. + try: + _prev_model = str(getattr(s, "model", "") or "").strip() + _new_model = str(model or "").strip() + if explicit_model_pick: + s.model_explicitly_picked = True + elif _new_model != _prev_model: + # model changed without a fresh pick → no longer a deliberate + # selection (provider repair / default fallback / stale switch) + s.model_explicitly_picked = False + # else: unchanged model, no fresh pick → preserve prior intent + 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 {} diff --git a/api/streaming.py b/api/streaming.py index f54a193b..821df294 100644 --- a/api/streaming.py +++ b/api/streaming.py @@ -7903,12 +7903,18 @@ def _run_agent_streaming( # 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: whether the user deliberately picked this session's model + # (persisted by /api/chat/start). Lets the cold-catalog custom-proxy + # branch preserve a deliberate vendor-namespaced pick while still + # stripping a stale cross-provider leftover (#433). + _explicitly_picked = bool(getattr(s, "model_explicitly_picked", False)) 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) + model_with_provider_context(model, provider_context), + explicitly_picked=_explicitly_picked, ) configured_base_url = resolved_base_url diff --git a/tests/test_model_resolver.py b/tests/test_model_resolver.py index bcafcbba..a3316d5d 100644 --- a/tests/test_model_resolver.py +++ b/tests/test_model_resolver.py @@ -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,32 +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_preserves_verbatim_5979(): - """#5979 (reopened): with a COLD/unbuilt catalog AND no config declaration, - an explicit custom proxy PRESERVES the id verbatim — the strip now requires - positive provenance (endpoint advertises only the bare id), it is no longer - the cold default. +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. - This is the fix for b3nw's reopened case: a user's actively-selected - NON-default model on a ``custom:`` proxy was truncated on every cold - send because the old cold fallback ran the legacy family heuristic. A wrong - strip destroys a namespace the proxy routes on (recurs every turn, - unrepairable); a wrong preserve only mis-sends a stale leftover that fails - loudly and self-heals on the next catalog warm. So cold → preserve for BOTH - a first-party-looking prefix and an unknown one. + 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-looking prefix → PRESERVE (was: strip). This is the flip. - 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 == 'openai/gpt-5.4', f"cold catalog must preserve verbatim, got {model_a!r}" - # intrinsic/unknown prefix → preserve (unchanged) - 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(): @@ -413,12 +419,13 @@ def test_custom_remote_foreign_profile_catalog_ignored_5979(): catalog. Proof id: ``openai/gpt-5.4`` with the FOREIGN catalog advertising ONLY the - bare ``gpt-5.4``. If the foreign catalog were (wrongly) trusted, the - bare-only-advertised rule would strip → ``gpt-5.4``. Because the fingerprint - mismatches, the snapshot is ignored and resolution takes the cold-preserve - default → ``openai/gpt-5.4``. The two outcomes genuinely diverge, so a result - of ``openai/gpt-5.4`` proves the foreign catalog was ignored (not merely that - the prefix was unknown). + 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 @@ -433,6 +440,7 @@ def test_custom_remote_foreign_profile_catalog_ignored_5979(): try: model, _, _ = _resolve_with_config( 'openai/gpt-5.4', provider='custom', base_url='https://relay.example/v1', + explicitly_picked=True, ) finally: config._available_models_cache = old_cache @@ -440,7 +448,7 @@ def test_custom_remote_foreign_profile_catalog_ignored_5979(): config._available_models_cache_source_fingerprint = old_fp config._models_cache_provenance = old_prov assert model == 'openai/gpt-5.4', ( - f"foreign-profile catalog must be ignored (cold-preserve default), got {model!r}" + f"foreign-profile catalog must be ignored (cold explicit-pick preserve), got {model!r}" ) @@ -526,7 +534,7 @@ def test_b3nw_cold_nondeclared_custom_slug_preserves_full_id_5979(): 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) + model, provider, _ = config.resolve_model_provider(mid, explicitly_picked=True) finally: if not had: if old_xai is None: @@ -541,6 +549,47 @@ def test_b3nw_cold_nondeclared_custom_slug_preserves_full_id_5979(): 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: 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 From 3559a867a9c3461b7e3a2dd2f47c58ecdae15b9b Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Mon, 13 Jul 2026 01:45:07 +0000 Subject: [PATCH 5/7] test: fix resolve_model_provider mocks to accept explicitly_picked kwarg Two streaming tests monkeypatch resolve_model_provider with a fixed-arg lambda (_model only). The send path now calls it with explicitly_picked=, so the mocks raised TypeError. Widened both lambdas to (_model, **_kw). Caught by the full suite (not the focused resolver tests, which call the real function). --- tests/test_issue1897_profile_switch_agent_cache.py | 2 +- tests/test_issue2965_streaming_sessiondb_profile_home.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_issue1897_profile_switch_agent_cache.py b/tests/test_issue1897_profile_switch_agent_cache.py index 97190f66..7c6782f7 100644 --- a/tests/test_issue1897_profile_switch_agent_cache.py +++ b/tests/test_issue1897_profile_switch_agent_cache.py @@ -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) diff --git a/tests/test_issue2965_streaming_sessiondb_profile_home.py b/tests/test_issue2965_streaming_sessiondb_profile_home.py index 7988792c..520eca3e 100644 --- a/tests/test_issue2965_streaming_sessiondb_profile_home.py +++ b/tests/test_issue2965_streaming_sessiondb_profile_home.py @@ -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) From 53a4b35555d69d566b072053925d04360b949988 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Mon, 13 Jul 2026 01:58:20 +0000 Subject: [PATCH 6/7] harden #5979 round 3 (Codex): signature-based explicit-pick provenance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex found a verified lifecycle hole: the bare boolean model_explicitly_picked was not cleared on /api/session/update (api/routes.py:14438-14460 changes+persists the model/provider without touching it), so after an update+reload the flag stayed True and a cold #433 stale leftover (openai/gpt-5.4) was wrongly preserved instead of stripped to gpt-5.4. Fix (Codex's prescription): replace the unscoped boolean with a SIGNATURE bound to the routing context. - api/models.py: new model_explicit_pick_signature(model, provider) -> '\x1f'; Session.model_explicit_pick_signature persisted field (replaces the boolean; in METADATA_FIELDS, restored via kwargs, survives restart). - api/routes.py /api/chat/start: on a fresh explicit_model_pick, stamp the signature of the resolved model+provider. - api/streaming.py: treat the send as a deliberate pick ONLY when the persisted signature equals the signature recomputed from the session's CURRENT persisted model+provider. Any model/provider change (chat-start, session-update, normalization, provider repair) yields a different signature and auto-invalidates the stale pick — no per-mutation-site clearing needed. So: deliberate pick on unchanged context -> preserve (b3nw); any switch away -> signature mismatch -> unmarked -> legacy strip (#433 stays correct even after a session-update+cold-restart). Adds signature match/invalidate/persist tests. 75 focused tests pass (resolver + sprint40 + the 2 streaming-mock suites). --- api/models.py | 32 ++++++++++++++++++++++------ api/routes.py | 32 +++++++++------------------- api/streaming.py | 25 +++++++++++++++++----- tests/test_model_resolver.py | 41 ++++++++++++++++++++++++++++++++++++ 4 files changed, 97 insertions(+), 33 deletions(-) diff --git a/api/models.py b/api/models.py index 9187f0f0..8ecbbc0c 100644 --- a/api/models.py +++ b/api/models.py @@ -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,11 +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: True when the user deliberately picked this session's model in - # the UI (persisted so the streaming resolver can preserve a custom-proxy - # vendor namespace on a cold catalog vs. stripping a stale leftover). - # Restored from the persisted metadata on load (arrives via **kwargs). - self.model_explicitly_picked = bool(kwargs.get('model_explicitly_picked', False)) + # #5979: signature of the model the user DELIBERATELY picked this session + # (``"\x1f"``), 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() @@ -1260,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', 'model_explicitly_picked', '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', diff --git a/api/routes.py b/api/routes.py index d48d348c..9d4685c1 100644 --- a/api/routes.py +++ b/api/routes.py @@ -21680,30 +21680,18 @@ def _handle_chat_start(handler, body, diag=None): profile_config=_pp_cfg, explicit_model_pick=explicit_model_pick, ) - # #5979: persist whether the user DELIBERATELY selected this session's - # model, so the streaming worker's resolve_model_provider() can tell a - # deliberate custom-proxy pick (preserve the vendor namespace even on a - # cold catalog) from a stale cross-provider leftover (#433, strip). The - # resolve happens on a detached worker thread that only has the session - # in scope, so the intent is stamped here on the session. - # - # DURABILITY: the flag must survive follow-up sends that don't re-carry - # explicit_model_pick (the onchange marker is one-shot — messages.js - # consumes it after the first send). So we only set the flag from THIS - # send's explicit_model_pick when it's True, and otherwise CLEAR it only - # when the resolved model actually CHANGED from what was stored (a real - # switch away from the deliberately-picked model). A plain follow-up send - # of the same model keeps the prior intent. + # #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: - _prev_model = str(getattr(s, "model", "") or "").strip() - _new_model = str(model or "").strip() if explicit_model_pick: - s.model_explicitly_picked = True - elif _new_model != _prev_model: - # model changed without a fresh pick → no longer a deliberate - # selection (provider repair / default fallback / stale switch) - s.model_explicitly_picked = False - # else: unchanged model, no fresh pick → preserve prior intent + 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 diff --git a/api/streaming.py b/api/streaming.py index 821df294..cdc0e5d8 100644 --- a/api/streaming.py +++ b/api/streaming.py @@ -7903,11 +7903,26 @@ def _run_agent_streaming( # 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: whether the user deliberately picked this session's model - # (persisted by /api/chat/start). Lets the cold-catalog custom-proxy - # branch preserve a deliberate vendor-namespaced pick while still - # stripping a stale cross-provider leftover (#433). - _explicitly_picked = bool(getattr(s, "model_explicitly_picked", False)) + # #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 ): diff --git a/tests/test_model_resolver.py b/tests/test_model_resolver.py index a3316d5d..08348ce6 100644 --- a/tests/test_model_resolver.py +++ b/tests/test_model_resolver.py @@ -1457,3 +1457,44 @@ 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 From 6d0fdcf9e2e99e1e24652e0b1ad8b2fd008e85e0 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Mon, 13 Jul 2026 02:09:21 +0000 Subject: [PATCH 7/7] test(#5979): add streaming explicitly_picked lifecycle test (Fable fast-follow) Fable's one recommended fast-follow: the streaming glue that computes _explicitly_picked from the persisted signature (streaming.py:7905-7931) had no direct coverage. Adds test_streaming_explicitly_picked_computation_5979 mirroring that exact comparison across deliberate-pick (honored), stale-after-model-change (ignored), stale-after-provider-change (ignored), and never-picked (unmarked). Both advisors passed on the signature mechanism: Codex SAFE TO SHIP (session-update hole verified closed: update->reload->cold openai/gpt-5.4 strips to gpt-5.4; b3nw's deliberate pick preserves), Fable 'ship it' (clean data-model invalidation, correct layering, no #433/#3872/#548 regression). Independent 10-scenario matrix through the real resolver all correct. --- tests/test_model_resolver.py | 42 ++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/tests/test_model_resolver.py b/tests/test_model_resolver.py index 08348ce6..165e4d3c 100644 --- a/tests/test_model_resolver.py +++ b/tests/test_model_resolver.py @@ -1498,3 +1498,45 @@ def test_explicit_pick_signature_persists_round_trip_5979(): # 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