From e5aa548d67f0c64f3c8c456470ff19a5be420dbc Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Tue, 23 Jun 2026 12:18:44 -0400 Subject: [PATCH] fix(#4756): refresh stale model catalogs on session visit --- api/config.py | 58 +++- api/routes.py | 6 + static/sessions.js | 8 + static/ui.js | 19 +- ...t_issue4756_session_visit_model_refresh.py | 281 ++++++++++++++++++ 5 files changed, 362 insertions(+), 10 deletions(-) create mode 100644 tests/test_issue4756_session_visit_model_refresh.py diff --git a/api/config.py b/api/config.py index fc42f035..fec6959b 100644 --- a/api/config.py +++ b/api/config.py @@ -3732,6 +3732,7 @@ _available_models_cache: dict | None = None _available_models_cache_ts: float = 0.0 _available_models_cache_source_fingerprint: dict | None = None _AVAILABLE_MODELS_CACHE_TTL: float = 86400.0 # 24 hours +_SESSION_VISIT_MODELS_FRESHNESS_SECONDS: float = 300.0 _available_models_cache_lock = threading.RLock() # must be RLock: cold path refactoring moved slow work inside this lock, requiring re-entry _cache_build_cv = threading.Condition(_available_models_cache_lock) # shares underlying RLock so notify_all() is safe inside with _available_models_cache_lock _cache_build_in_progress = False # True while a cold path is actively building @@ -5228,7 +5229,7 @@ def _read_visible_codex_cache_model_ids() -> list[str]: return ordered -def get_available_models(*, prefer_cache: bool = False) -> dict: +def get_available_models(*, prefer_cache: bool = False, force_refresh: bool = False) -> dict: """ Return available models grouped by provider. @@ -5252,6 +5253,10 @@ def get_available_models(*, prefer_cache: bool = False) -> dict: server-initiated wakeup turn (Option Z) takes so a cold catalog can never block the wakeup chat/start on a flaky network. A normal human request leaves this False and keeps the full live-discovery behaviour. + + ``force_refresh=True`` is an internal escape hatch for bounded freshness + checks that need a real live rebuild while preserving the default cache + contract for every existing caller. """ global _cache_build_in_progress, _available_models_cache, _available_models_cache_ts, _available_models_cache_source_fingerprint, _cache_build_cv # Config mtime check — must come before any config reads. @@ -6678,10 +6683,12 @@ def get_available_models(*, prefer_cache: bool = False) -> dict: # so only one thread rebuilds while others wait. disk_groups = None stale_disk_groups = None - if _available_models_cache is None: + if _available_models_cache is None and not force_refresh: disk_groups = _load_models_cache_from_disk() if disk_groups is None: stale_disk_groups = _load_stale_models_cache_from_disk() + elif force_refresh: + stale_disk_groups = _load_stale_models_cache_from_disk() with _available_models_cache_lock: # If another thread is already building, wait for its result instead @@ -6706,16 +6713,16 @@ def get_available_models(*, prefer_cache: bool = False) -> dict: # Serve from memory cache if fresh now = time.monotonic() - cached = _get_fresh_memory_models_cache(now) - if cached is not None: - return cached + if not force_refresh: + cached = _get_fresh_memory_models_cache(now) + if cached is not None: + return cached # Cold path: disk cache hit — use it (fast, no lock contention) - if disk_groups is not None: + if disk_groups is not None and not force_refresh: _available_models_cache = disk_groups _available_models_cache_ts = now _available_models_cache_source_fingerprint = _models_cache_source_fingerprint() - _save_models_cache_to_disk(disk_groups) return copy.deepcopy(disk_groups) # ── prefer_cache: NEVER run the live provider rebuild ──────────────── @@ -6933,6 +6940,43 @@ def get_available_models(*, prefer_cache: bool = False) -> dict: return copy.deepcopy(_static_models_catalog_without_live_probes()) +def _models_cache_file_age_seconds(cache_path: Path, now: float) -> float | None: + try: + return max(0.0, now - cache_path.stat().st_mtime) + except OSError: + return None + + +def get_available_models_for_session_visit() -> dict: + """Return /api/models with a short session-visit freshness horizon.""" + global _available_models_cache, _available_models_cache_ts, _available_models_cache_source_fingerprint + cache_path = _get_models_cache_path() + cache_age = _models_cache_file_age_seconds(cache_path, time.time()) + disk_cached = None + if cache_age is not None and cache_age < _SESSION_VISIT_MODELS_FRESHNESS_SECONDS: + now_mono = time.monotonic() + with _available_models_cache_lock: + cached = _get_fresh_memory_models_cache(now_mono) + if cached is not None: + return cached + disk_cached = _load_models_cache_from_disk() + if disk_cached is not None: + with _available_models_cache_lock: + _available_models_cache = copy.deepcopy(disk_cached) + _available_models_cache_ts = time.monotonic() + _available_models_cache_source_fingerprint = _models_cache_source_fingerprint() + return copy.deepcopy(disk_cached) + + stale_cached = disk_cached or _load_stale_models_cache_from_disk() + try: + return get_available_models(force_refresh=True) + except Exception: + logger.debug("session-visit models refresh failed", exc_info=True) + if stale_cached is not None: + return copy.deepcopy(stale_cached) + return get_available_models(prefer_cache=True) + + # ── Static file path ───────────────────────────────────────────────────────── _INDEX_HTML_PATH = REPO_ROOT / "static" / "index.html" diff --git a/api/routes.py b/api/routes.py index 31c6ac34..bec2ed92 100644 --- a/api/routes.py +++ b/api/routes.py @@ -2278,6 +2278,7 @@ from api.config import ( _resolve_cli_toolsets, _INDEX_HTML_PATH, get_available_models, + get_available_models_for_session_visit, _provider_is_known_or_configured, IMAGE_EXTS, MD_EXTS, @@ -9158,6 +9159,11 @@ def handle_get(handler, parsed) -> bool: # the detached rebuild worker (and the legacy synchronous rebuild), # which the request-thread wrapper could not reach. See # api.config.get_available_models cold path + profile_scope_for_detached_worker. + freshness = parse_qs(parsed.query or "").get("freshness", [""])[0].strip().lower() + if freshness == "session_visit": + return j(handler, get_available_models_for_session_visit()) + if freshness: + return bad(handler, f"unknown models freshness: {freshness}", status=400) return j(handler, get_available_models()) if parsed.path == "/api/models/live": diff --git a/static/sessions.js b/static/sessions.js index f01741bc..6ec9870d 100644 --- a/static/sessions.js +++ b/static/sessions.js @@ -1192,6 +1192,14 @@ async function loadSession(sid){ return loadSession(continuationSid,{...opts,skipLineageResolve:true,skipContinuationResolve:true,force:true}); } S.session=data.session; + if(typeof populateModelDropdown==='function'){ + const modelRefreshSid=sid; + const modelRefreshPromise=Promise.resolve().then(()=>{ + if(_loadingSessionId!==modelRefreshSid) return; + return populateModelDropdown({freshness:'session_visit'}); + }).catch(()=>{}); + if(typeof window!=='undefined') window._modelDropdownReady=modelRefreshPromise; + } // Loading a real existing session abandons any pre-session toolset override // staged on the empty composer — clear it so it can't leak into a later New // Chat started from this session (#4490 follow-up). diff --git a/static/ui.js b/static/ui.js index 1c9f759c..e7ddc376 100644 --- a/static/ui.js +++ b/static/ui.js @@ -1992,6 +1992,8 @@ function _persistSessionModelCorrection(model, provider, opts){ }); return opts&&opts.propagateErrors ? request : request.catch(()=>{}); } +let _modelDropdownRequestSeq=0; + function _applySessionModelFallback(sel){ if(!sel) return null; const configuredDefault=String(window._defaultModel||'').trim(); @@ -2014,10 +2016,15 @@ function _applySessionModelFallback(sel){ async function populateModelDropdown(opts={}){ const sel=$('modelSelect'); if(!sel) return; + const requestSeq=++_modelDropdownRequestSeq; try{ - const _modelsRes=await fetch(new URL('api/models',document.baseURI||location.href).href,{credentials:'include'}); + const modelsUrl=new URL('api/models',document.baseURI||location.href); + if(opts&&opts.freshness) modelsUrl.searchParams.set('freshness',opts.freshness); + const _modelsRes=await fetch(modelsUrl.href,{credentials:'include'}); + if(requestSeq!==_modelDropdownRequestSeq) return; if(_redirectIfUnauth(_modelsRes)) return; const data=await _modelsRes.json(); + if(requestSeq!==_modelDropdownRequestSeq) return; // Store active provider globally so the send path can warn on mismatch window._activeProvider=data.active_provider||null; // Store default model so newSession() can apply it (#872). @@ -2112,8 +2119,9 @@ async function populateModelDropdown(opts={}){ } // Kick off a background live-model fetch for the active provider. // This runs after the static list is already shown (no blocking flicker). - if(data.active_provider) _fetchLiveModels(data.active_provider, sel); + if(data.active_provider) _fetchLiveModels(data.active_provider, sel, requestSeq); }catch(e){ + if(requestSeq!==_modelDropdownRequestSeq) return; // API unavailable -- keep the hardcoded HTML options as fallback console.warn('Failed to load models from server:',e.message); if(typeof syncModelChip==='function') syncModelChip(); @@ -2203,10 +2211,12 @@ function _addLiveModelsToSelect(provider, models, sel){ return added; } -async function _fetchLiveModels(provider, sel){ +async function _fetchLiveModels(provider, sel, requestSeq=null){ if(!provider||!sel) return; + if(requestSeq!==null&&requestSeq!==_modelDropdownRequestSeq) return; // Already fetched — apply cached models to this select element (#872) if(_liveModelCache[provider]){ + if(requestSeq!==null&&requestSeq!==_modelDropdownRequestSeq) return; const added=_addLiveModelsToSelect(provider,_liveModelCache[provider],sel); if(added>0 && typeof syncModelChip==='function') syncModelChip(); return; @@ -2216,10 +2226,13 @@ async function _fetchLiveModels(provider, sel){ const url=new URL('api/models/live',document.baseURI||location.href); url.searchParams.set('provider',provider); const _liveRes=await fetch(url.href,{credentials:'include'}); + if(requestSeq!==null&&requestSeq!==_modelDropdownRequestSeq) return; if(_redirectIfUnauth(_liveRes)) return; const data=await _liveRes.json(); + if(requestSeq!==null&&requestSeq!==_modelDropdownRequestSeq) return; if(!data.models||!data.models.length) return; _liveModelCache[provider]=data.models; + if(requestSeq!==null&&requestSeq!==_modelDropdownRequestSeq) return; const added=_addLiveModelsToSelect(provider,data.models,sel); if(added>0){ if(typeof syncModelChip==='function') syncModelChip(); diff --git a/tests/test_issue4756_session_visit_model_refresh.py b/tests/test_issue4756_session_visit_model_refresh.py new file mode 100644 index 00000000..43208aeb --- /dev/null +++ b/tests/test_issue4756_session_visit_model_refresh.py @@ -0,0 +1,281 @@ +"""Regression coverage for #4756 session-visit model catalog freshness.""" + +from __future__ import annotations + +import io +import json +import os +import time +from pathlib import Path +from urllib.parse import urlparse + +REPO = Path(__file__).resolve().parent.parent + + +def _catalog(label: str) -> dict: + return { + "active_provider": "openai", + "default_model": label, + "configured_model_badges": {}, + "groups": [ + { + "provider": "OpenAI", + "provider_id": "openai", + "models": [{"id": label, "label": label}], + } + ], + "aliases": {}, + } + + +def _reset_models_memory_cache(monkeypatch): + import api.config as cfg + + monkeypatch.setattr(cfg, "_available_models_cache", None, raising=False) + monkeypatch.setattr(cfg, "_available_models_cache_ts", 0.0, raising=False) + monkeypatch.setattr(cfg, "_available_models_cache_source_fingerprint", None, raising=False) + monkeypatch.setattr(cfg, "_cache_build_in_progress", False, raising=False) + + +def test_session_visit_fresh_profile_cache_returns_without_live_rebuild(tmp_path, monkeypatch): + import api.config as cfg + + _reset_models_memory_cache(monkeypatch) + disk_catalog = _catalog("cached-model") + cache_path = tmp_path / "models_cache.profile.json" + cache_path.write_text("{}", encoding="utf-8") + now = time.time() + os.utime(cache_path, (now, now)) + + monkeypatch.setattr(cfg, "_SESSION_VISIT_MODELS_FRESHNESS_SECONDS", 300.0, raising=False) + monkeypatch.setattr(cfg, "_get_models_cache_path", lambda: cache_path) + monkeypatch.setattr(cfg, "_load_models_cache_from_disk", lambda: disk_catalog) + monkeypatch.setattr(cfg, "_load_stale_models_cache_from_disk", lambda: None) + monkeypatch.setattr(cfg, "_models_cache_source_fingerprint", lambda: {"profile": "demo"}) + + def _unexpected_live_rebuild(**_kwargs): + raise AssertionError("fresh session-visit cache must not run a live rebuild") + + monkeypatch.setattr(cfg, "get_available_models", _unexpected_live_rebuild) + + assert cfg.get_available_models_for_session_visit() == disk_catalog + + +def test_session_visit_ignores_recently_warmed_memory_when_disk_cache_is_stale(tmp_path, monkeypatch): + import api.config as cfg + + _reset_models_memory_cache(monkeypatch) + stale_catalog = _catalog("stale-model") + rebuilt_catalog = _catalog("rebuilt-model") + cache_path = tmp_path / "models_cache.profile.json" + cache_path.write_text("{}", encoding="utf-8") + old = time.time() - 600.0 + os.utime(cache_path, (old, old)) + calls = [] + + monkeypatch.setattr(cfg, "_SESSION_VISIT_MODELS_FRESHNESS_SECONDS", 300.0, raising=False) + monkeypatch.setattr(cfg, "_get_models_cache_path", lambda: cache_path) + monkeypatch.setattr(cfg, "_load_models_cache_from_disk", lambda: stale_catalog) + monkeypatch.setattr(cfg, "_load_stale_models_cache_from_disk", lambda: stale_catalog) + monkeypatch.setattr(cfg, "_models_cache_source_fingerprint", lambda: {"profile": "demo"}) + monkeypatch.setattr(cfg, "_available_models_cache", stale_catalog, raising=False) + monkeypatch.setattr(cfg, "_available_models_cache_ts", time.monotonic(), raising=False) + monkeypatch.setattr(cfg, "_available_models_cache_source_fingerprint", {"profile": "demo"}, raising=False) + + def _live_rebuild(**kwargs): + calls.append(kwargs) + return rebuilt_catalog + + monkeypatch.setattr(cfg, "get_available_models", _live_rebuild) + + assert cfg.get_available_models_for_session_visit() == rebuilt_catalog + assert calls == [{"force_refresh": True}] + + +def test_session_visit_stale_profile_cache_revalidates_with_live_rebuild(tmp_path, monkeypatch): + import api.config as cfg + + _reset_models_memory_cache(monkeypatch) + stale_catalog = _catalog("stale-model") + rebuilt_catalog = _catalog("rebuilt-model") + cache_path = tmp_path / "models_cache.profile.json" + cache_path.write_text("{}", encoding="utf-8") + old = time.time() - 600.0 + os.utime(cache_path, (old, old)) + calls = [] + + monkeypatch.setattr(cfg, "_SESSION_VISIT_MODELS_FRESHNESS_SECONDS", 300.0, raising=False) + monkeypatch.setattr(cfg, "_get_models_cache_path", lambda: cache_path) + monkeypatch.setattr(cfg, "_load_models_cache_from_disk", lambda: stale_catalog) + monkeypatch.setattr(cfg, "_load_stale_models_cache_from_disk", lambda: stale_catalog) + + def _live_rebuild(**kwargs): + calls.append(kwargs) + return rebuilt_catalog + + monkeypatch.setattr(cfg, "get_available_models", _live_rebuild) + + assert cfg.get_available_models_for_session_visit() == rebuilt_catalog + assert calls == [{"force_refresh": True}] + + +def test_default_disk_hit_does_not_restamp_stale_cache_for_session_visit(tmp_path, monkeypatch): + import api.config as cfg + + _reset_models_memory_cache(monkeypatch) + stale_catalog = _catalog("stale-model") + rebuilt_catalog = _catalog("rebuilt-model") + cache_path = tmp_path / "models_cache.profile.json" + cache_path.write_text("{}", encoding="utf-8") + old = time.time() - 600.0 + os.utime(cache_path, (old, old)) + refresh_calls = [] + + monkeypatch.setattr(cfg, "_SESSION_VISIT_MODELS_FRESHNESS_SECONDS", 300.0, raising=False) + monkeypatch.setattr(cfg, "_get_models_cache_path", lambda: cache_path) + monkeypatch.setattr(cfg, "_load_models_cache_from_disk", lambda: stale_catalog) + monkeypatch.setattr(cfg, "_load_stale_models_cache_from_disk", lambda: stale_catalog) + monkeypatch.setattr(cfg, "_models_cache_source_fingerprint", lambda: {"profile": "demo"}) + monkeypatch.setattr(cfg, "_cfg_mtime", 0.0, raising=False) + monkeypatch.setattr(cfg, "_save_models_cache_to_disk", lambda _cache: (_ for _ in ()).throw( + AssertionError("plain disk hits must not rewrite the models cache file") + )) + + assert cfg.get_available_models() == stale_catalog + + def _live_rebuild(**kwargs): + refresh_calls.append(kwargs) + return rebuilt_catalog + + monkeypatch.setattr(cfg, "get_available_models", _live_rebuild) + + assert cfg.get_available_models_for_session_visit() == rebuilt_catalog + assert refresh_calls == [{"force_refresh": True}] + + +def test_session_visit_live_rebuild_failure_falls_back_to_cached_catalog(tmp_path, monkeypatch): + import api.config as cfg + + _reset_models_memory_cache(monkeypatch) + stale_catalog = _catalog("fallback-model") + cache_path = tmp_path / "models_cache.profile.json" + cache_path.write_text("{}", encoding="utf-8") + old = time.time() - 600.0 + os.utime(cache_path, (old, old)) + + monkeypatch.setattr(cfg, "_SESSION_VISIT_MODELS_FRESHNESS_SECONDS", 300.0, raising=False) + monkeypatch.setattr(cfg, "_get_models_cache_path", lambda: cache_path) + monkeypatch.setattr(cfg, "_load_models_cache_from_disk", lambda: None) + monkeypatch.setattr(cfg, "_load_stale_models_cache_from_disk", lambda: stale_catalog) + + def _failing_live_rebuild(**kwargs): + assert kwargs == {"force_refresh": True} + raise RuntimeError("provider unavailable") + + monkeypatch.setattr(cfg, "get_available_models", _failing_live_rebuild) + + assert cfg.get_available_models_for_session_visit() == stale_catalog + + +class _FakeHandler: + def __init__(self): + self.status = None + self.sent_headers = [] + self.body = bytearray() + self.wfile = self + self.rfile = io.BytesIO(b"") + self.headers = {"Content-Length": "0"} + self.request = None + + def send_response(self, status): + self.status = status + + def send_header(self, name, value): + self.sent_headers.append((name, value)) + + def end_headers(self): + pass + + def write(self, data): + self.body.extend(data) + + def json_body(self): + return json.loads(bytes(self.body).decode("utf-8")) + + +def test_models_route_session_visit_freshness_uses_bounded_helper(monkeypatch): + import api.routes as routes + + expected = _catalog("route-model") + calls = [] + + def _session_visit_catalog(): + calls.append("session_visit") + return expected + + monkeypatch.setattr(routes, "get_available_models_for_session_visit", _session_visit_catalog) + + handler = _FakeHandler() + parsed = urlparse("http://example.com/api/models?freshness=session_visit") + routes.handle_get(handler, parsed) + + assert handler.status == 200 + assert handler.json_body()["default_model"] == "route-model" + assert calls == ["session_visit"] + + +def _read_static(name: str) -> str: + return (REPO / "static" / name).read_text(encoding="utf-8") + + +def _extract_function_body(src: str, signature: str) -> str: + idx = src.find(signature) + if idx == -1: + raise AssertionError(f"signature {signature!r} not found") + header_end = src.find("){", idx) + if header_end == -1: + raise AssertionError(f"function body start for {signature!r} not found") + open_idx = header_end + 1 + depth = 0 + for i in range(open_idx, len(src)): + if src[i] == "{": + depth += 1 + elif src[i] == "}": + depth -= 1 + if depth == 0: + return src[idx : i + 1] + raise AssertionError(f"unbalanced braces in {signature!r}") + + +def test_populate_model_dropdown_accepts_session_visit_freshness_and_guards_stale_responses(): + body = _extract_function_body(_read_static("ui.js"), "async function populateModelDropdown(") + live_tail = _extract_function_body(_read_static("ui.js"), "async function _fetchLiveModels(") + + assert "modelsUrl.searchParams.set('freshness',opts.freshness)" in body + assert "const requestSeq=++_modelDropdownRequestSeq" in body + assert body.count("requestSeq!==_modelDropdownRequestSeq") >= 3 + assert "_fetchLiveModels(data.active_provider, sel, requestSeq)" in body + assert live_tail.count("requestSeq!==null&&requestSeq!==_modelDropdownRequestSeq") >= 4 + + +def test_load_session_schedules_async_session_visit_model_refresh_after_metadata_load(): + body = _extract_function_body(_read_static("sessions.js"), "async function loadSession(") + + assign_idx = body.index("S.session=data.session") + promise_idx = body.index("const modelRefreshPromise=Promise.resolve().then(") + ready_idx = body.index("window._modelDropdownReady=modelRefreshPromise") + refresh_idx = body.index("populateModelDropdown({freshness:'session_visit'})") + stale_guard_idx = body.index("_loadingSessionId!==modelRefreshSid") + + assert assign_idx < promise_idx < refresh_idx < ready_idx + assert promise_idx < stale_guard_idx < refresh_idx + + +def test_boot_model_dropdown_reuses_inflight_session_visit_refresh(): + body = _extract_function_body(_read_static("boot.js"), "const _startBootModelDropdown=()=>") + + ready_idx = body.index("const ready=window._modelDropdownReady;") + reuse_idx = body.index("if(ready&&typeof ready.then==='function') return ready;") + hydrate_idx = body.index("const next=_hydrateBootModelDropdown();") + + assert ready_idx < reuse_idx < hydrate_idx