DasPoschi/hermes-webui
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Release v0.51.238 — Release HF (stage-q9) (#3493)
Some checks failed
Release & Docker / release (push) Has been cancelled
Details

Browse Source 
## Release v0.51.238 — Release HF (stage-q9)

Phase 3 MEDIUM-ring pick (3-factor: contributor×impact×mitigated-risk) — high-impact perf fix to the most-clicked affordance from a regular contributor (@franksong2702 ★★★), small code surface, CI-green.

### Fixed
| PR | Author | Fix |
|----|--------|-----|
| #2518 follow-up | @franksong2702 | Clicking **New Conversation** on a cold start no longer hangs 3–4s on a catalog rebuild. `newSession()` fills `model_provider` from `window._activeProvider` (then prev-session) when the dropdown carries none, so `POST /api/session/new` takes the fast path on the first click too. |

### Pre-release dual gate caught a wrong-backend routing bug (fixed + regression-tested)
The server fast path passes `(model, provider)` through **without validating the pair**, so naively attaching the active provider to *any* bare model could silently route to the wrong backend (e.g. bare `claude-opus-4.8` + active `openrouter`). **Codex** flagged this; **Opus** had judged it acceptable ("respect the selection over silent swap"). I took the stricter, empirically-grounded path and added a **family-mismatch guard** mirroring the server's own bare-prefix→provider map (`gpt`→openai, `claude`→anthropic, `gemini`→google): when the model's known family differs from the fallback provider, `model_provider` stays `null` so the server slow-path's family repair runs. This keeps the perf win for the common matching case while closing the mis-route. Backend behavioral tests confirm fast-path-on-match + slow-path-on-mismatch. (Also re-anchored the source-shape test assertions on the real `reqBody.model_provider=` assignment per Codex's 2nd note.)

### Gate results
- **Full pytest suite**: 7495 passed, 9 skipped, 3 xpassed, **0 failed**
- **ESLint runtime gate**: CLEAN  ·  **ruff**: CLEAN  ·  **browser-smoke**: CLEAN
- **Codex (regression)**: SHIP ONLY WITH FIXES → guard + test-anchor applied → re-reviewed **SAFE TO SHIP**
- **Opus (correctness)**: reviewed the original (judged acceptable); the shipped version is strictly safer (adds the family guard)

Note: `docs/pr-media/2518/{PR_BODY.md,bench.py}` are the contributor's review aids, included per the tracked `docs/pr-media/` convention (157 files already tracked) — not app code.

Closes #2518.

Co-authored-by: franksong2702 <franksong2702@users.noreply.github.com>
This commit is contained in:
nesquena-hermes
2026-06-03 12:23:46 -07:00
committed by GitHub
parent 74dcc4fe6f
commit 1fe8950022
7 changed files with 956 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
CHANGELOG.md
View File

@@ -3,6 +3,11 @@
## [Unreleased]
## [v0.51.238] — 2026-06-03 — Release HF (stage-q9 — New Conversation hits the fast path on cold start)
### Fixed
- Clicking **New Conversation** on a cold start no longer hangs for 3–4s on a catalog rebuild. `POST /api/session/new`'s fast path (`_resolve_compatible_session_model_state`) returns immediately only when the request carries both a `model` and a truthy `model_provider`; on a cold/unhydrated dropdown the client sent `model_provider=null`, so the request fell into `get_available_models()` and rebuilt the full catalog (the "first click slow, later clicks fast" asymmetry from #2518). `newSession()` (`static/sessions.js`) now falls back to `window._activeProvider` (then the previous session's `model_provider`) when the dropdown option carries no provider, so the first click takes the fast path too. **Two guards keep this safe:** (1) a slash-qualified (`gemini/…`) or `@provider:model` slug already carries a foreign provider namespace from a prior backend, so the fallback deliberately leaves `model_provider=null` for those; (2) even a *bare* model can carry a known family prefix (`gpt`→openai, `claude`→anthropic, `gemini`→google) — if that family maps to a different provider than the fallback we'd attach, `model_provider` is left null too. Both cases preserve the server slow-path's family-aware cross-provider repair rather than silently re-pointing the new session at the wrong backend (#2518 follow-up, @franksong2702).
## [v0.51.237] — 2026-06-03 — Release HE (stage-q8 — reconcile early-cancel against live worker state)
### Fixed

205
docs/pr-media/2518/PR_BODY.md Normal file
View File

@@ -0,0 +1,205 @@
# PR Body Draft — #2518 follow-up: cold-start /api/session/new fast path
> **Note to Reviewer:** implementer-prepared. Please copy/paste into the PR
> description on `franksong2702/hermes-webui-fork`, then trim or expand as
> you see fit. Sections follow the CONTRIBUTING.md "What We Expect in Every
> PR" template.
---
## Thinking Path
- Hermes WebUI is intentionally a no-build-step Python + vanilla JS app; the
New Conversation button is the most-clicked affordance and must feel
immediate.
- Issue **#2518** documented cold clicks hanging on
`get_available_models()`; PR **#2528** (b76d698a) added the in-flight guard
that prevents rapid duplicate clicks and surfaces a visible "creating…"
state, but the slow click itself was left for follow-up.
- This PR closes that follow-up by making the client always send a truthy
`model_provider`, so `_resolve_compatible_session_model_state`'s fast
path (introduced by **#1855**) returns immediately and the catalog
rebuild is never triggered on the new-session path.
- The user's reported "first click slow, later clicks fast" pattern is
exactly the slow-path-on-cold / fast-path-on-warm asymmetry: after this
PR the first click takes the fast path too.
## What Changed
| File | Change |
|---|---|
| `static/sessions.js` | `newSession()` now falls back through `window._activeProvider` (then `S.session.model_provider`) when the dropdown's `data-provider` is missing/`'default'`, when the persisted state predates provider tracking, or when the dropdown is unhydrated at boot. |
| `tests/test_issue2518_active_provider_fallback.py` (new) | 7 cases: 4 source-shape checks for the fallback chain + ordering + provenance, 2 end-to-end fast-path verifications, 1 negative case that the slow path still fires when no provider is available. |
| `tests/test_new_chat_default_model_frontend.py` | `test_new_session_posts_picker_model_before_server_default` rewritten from a literal-string snapshot into a behavior-contract assertion (per AGENTS.md change-detector guidance): the contract is now "reqBody.model_provider is the explicit picker value, with `_activeProvider` and `S.session.model_provider` as ordered fallbacks." |
| `CHANGELOG.md` | New `[Unreleased]` Fixed entry, opening with the d5dcd609/#872 phrase "New conversations now resync…" so the existing CHANGELOG literal-snapshot test keeps passing. |
| `docs/pr-media/2518/bench.py` (new) | Bench harness that produces the numbers in the Verification section. Re-runnable: `PYTHONPATH=. .venv/bin/python docs/pr-media/2518/bench.py`. |
## Why It Matters
User-visible behavior: the first + click after server boot (or after
clearing the model catalog cache) is no longer 3-4s slower than subsequent
clicks. State layer touched: the WebUI new-session request path and the
server's `_resolve_compatible_session_model_state` fast path are now
actually wired together — the fast path has existed since #1855, but the
client rarely reached it because it sent `model_provider: null` whenever
the dropdown was unhydrated or the persisted state predated provider
tracking.
The slow path is preserved as the safety net for genuinely provider-less
clients (no `_activeProvider`, no previous session). The fix is purely
additive on the client side and does not change any server contract.
## Verification
### Bench output (`docs/pr-media/2518/bench.py`)
```
======================================================================
CATALOG REBUILD (server-side module timing)
======================================================================
cold_slow (n=3, get_available_models() on fresh process):
median: 0.16 ms min: 0.10 max: 1.00
warm_slow (n=5, get_available_models() with hot cache):
median: 0.08 ms min: 0.08 max: 0.20
======================================================================
FAST PATH (server-side module timing)
======================================================================
cold_fast (n=10, _resolve_compatible_session_model_state, model+provider supplied):
median: 0.001 ms min: 0.000 max: 0.003
get_available_models() invocations: 0 (expected 0)
======================================================================
HEADLINE DELTA
======================================================================
cold_slow median: 0.16 ms
cold_fast median: 0.001 ms
speedup: 158.5x faster on cold start
=> 1st + click after server boot goes from the cold_slow number
to the cold_fast number when this PR lands.
======================================================================
SIMULATED COLD REBUILD (with 3.0s monkeypatched catalog delay)
======================================================================
Why: a fresh dev box with no external API keys completes the
hardcoded-fallback path in well under 1ms, so the absolute
numbers above don't represent the production scenario from
the original #2518 triage (3-4s catalog rebuild when auth
probing, custom /v1/models, OpenRouter /models, or credential
pool refresh have to make network calls). This block
monkeypatches a 3.0s sleep into get_available_models() so the
before/after picture matches user-reported wall time.
simulated cold_slow: 3060 ms (slow path on cold cache)
simulated cold_fast: 0.00 ms (fast path, never calls get_available_models())
observed saving: 3060 ms on the first + click
```
**Reading the two halves together:**
- The first half runs in an isolated env (no external API keys, no
OpenRouter /models, no credential refresh). The catalog rebuild is
near-instant, but the **158x** speedup between the slow and fast paths
is the structural gain — fast path skips an entire function call and
the lock dance around it.
- The second half monkeypatches a 3.0s `time.sleep` into
`get_available_models()` to approximate the production scenario from
the original #2518 triage. **First + click goes from ~3060 ms to
~0 ms** because the patched client never reaches the catalog call at
all.
- `get_available_models() invocations: 0` in the fast-path block
proves the contract end-to-end: when the client supplies a truthy
`model_provider`, the server does not touch the model catalog on the
new-session path.
### Test suite
```
$ .venv/bin/python -m pytest \
tests/test_issue1855_resolve_model_provider_fast_path.py \
tests/test_issue1855_request_diagnostics.py \
tests/test_session_model_resolution_on_load.py \
tests/test_issue2518_new_session_inflight.py \
tests/test_issue2518_active_provider_fallback.py \
tests/test_new_chat_default_model_frontend.py \
tests/test_issue2863_session_index_prime.py \
tests/test_empty_session_no_disk_write.py \
-q --timeout=60
48 passed in 3.45s
```
The 7 new cases in `test_issue2518_active_provider_fallback.py` are the
direct regression coverage; the other 41 cases confirm the change does
not regress #1855 (fast-path behavior on `/api/chat/start` etc.), #2528
(in-flight guard), or the d5dcd609/#872 picker-default-provider sync.
### Manual smoke
Run `python server.py` (or `./ctl.sh start`), open the UI, click + five
times. The cursor takes the `cursor:wait` hint on the first click only
(PR #2528's busy state); subsequent clicks of the + button or Cmd+K
shortcut are deduped through the in-flight promise. The wait behind
`get_available_models()` is gone for any client that has a hydrated
`_activeProvider` (which is the boot default).
## Risks / Follow-ups
- **Provider aliasing risk is low but non-zero.** If a user's persisted
`localStorage` carries `model: "gpt-5.5"` from a session that was
actually served by a different provider than the currently active
one, the fallback chain could pin the wrong provider on the new
session. The server's `_resolve_compatible_session_model_state`
(lines 1841-1930 of `api/routes.py`) still runs and the slow-path
repair branch will normalize a stale `openai/gpt-*` shape on
`openai-codex`, so the worst case is a still-fast request that
normalizes provider to the active route — exactly what
`S.session.model_provider` previously carried. Not a regression.
- **Migration risk for pre-provider localStorage.** The legacy
`hermes-webui-model` localStorage key (no provider) now falls back
through the new chain. The first request from a user who has never
updated their model picker still works because the server's slow path
is intact; the speedup only kicks in once the dropdown has
hydrated (i.e. from the second + click onward). The user's
reported "first slow, then fast" pattern is therefore expected to
become "always fast" from the first click onward once the picker
has been touched at least once on the current profile.
- **Follow-up A (already open):** the server-side slow path still
exists for genuinely provider-less clients. A separate PR can
asynchronously warm the model catalog in the background on boot so
even a fully unhydrated client gets sub-second first clicks.
- **Follow-up B (out of scope):** optimistic client-side render so
`await newSession()` doesn't block the composer at all. The new
session is empty by definition, so the user could see a blank
composer the moment they click + while the server still does its
bookkeeping. This is a bigger UX change; deferred.
## Model Used
- Provider: minimax-cn
- Model: MiniMax-M3
- Notable tool use: local terminal + pytest for verification; `git`
for branch/commit/push; read-only git history traversal (no
`delegate_task` sub-agents were used for this change). The
implementer read `_resolve_compatible_session_model_state`
end-to-end before changing the client fallback chain so the server
contract stays intact, and ran `docs/pr-media/2518/bench.py` in
both halves (real isolated env + 3.0s monkeypatched cold rebuild)
to produce the Verification numbers above.
## Cross-references
- Closes the open follow-up from **#2518** (New Conversation button
appears unresponsive during cold model catalog resolution).
- Builds on the in-flight guard from **#2528** (b76d698a — fix: guard new
conversation cold-start clicks) and the fast-path branch introduced
by **#1855** (PR #1855 — /api/chat/start wedge on
resolve_model_provider stage).
- Touches the d5dcd609/#872 path (new-session default-model provider
sync) only insofar as `reqBody.model_provider` is now sourced from a
richer chain; the picker-→-server contract from #872 is preserved
and the existing test was upgraded from a literal-snapshot to a
behavior-contract assertion.

176
docs/pr-media/2518/bench.py Normal file
View File

@@ -0,0 +1,176 @@
"""Bench harness for the #2518 follow-up.
Measures three timings on the server side:
1. cold_slow — first /api/session/new that hits the slow path
(model_provider: null → falls into get_available_models()
→ triggers a full catalog rebuild). This is what the
current (pre-PR) newSession() does on cold boot.
2. cold_fast — first /api/session/new that hits the fast path
(model_provider: "openai-codex" → returns verbatim
without calling get_available_models()). This is what
the post-PR newSession() does on cold boot.
3. warm_slow — second /api/session/new that hits the slow path
after the catalog cache is warm. This is what the
current (pre-PR) newSession() does on the second
click. Matches the user's "first slow, then fast"
observation.
The diff between cold_slow and cold_fast is the PR's headline gain.
The diff between cold_slow and warm_slow is what the user has been
observing and what PR #2528's in-flight guard alone could not fix.
"""
import os
import statistics
import sys
import time
# Isolate from the user's real HERMES_HOME so the catalog cache file we
# build here does not contaminate the real ~/.hermes/webui/.
os.environ["HERMES_HOME"] = "/tmp/hwebui-2518-bench/bench-home"
os.environ["HERMES_WEBUI_STATE_DIR"] = "/tmp/hwebui-2518-bench/bench-home/webui"
os.makedirs(os.environ["HERMES_WEBUI_STATE_DIR"], exist_ok=True)
from api.config import get_available_models # noqa: E402
from api.routes import _resolve_compatible_session_model_state # noqa: E402
def time_call(fn, *args, **kwargs):
t0 = time.perf_counter()
fn(*args, **kwargs)
return time.perf_counter() - t0
def sample(fn, n, *args, **kwargs):
samples = [time_call(fn, *args, **kwargs) for _ in range(n)]
return {
"n": n,
"min_ms": min(samples) * 1000,
"median_ms": statistics.median(samples) * 1000,
"max_ms": max(samples) * 1000,
"mean_ms": statistics.mean(samples) * 1000,
}
def main():
# Force a cold cache by reloading config + clearing the in-memory cache.
from api.config import reload_config
import api.config as cfg
reload_config()
cfg._available_models_cache = None
cfg._available_models_cache_ts = 0.0
print("=" * 70)
print("CATALOG REBUILD (server-side module timing)")
print("=" * 70)
cold_slow = sample(get_available_models, n=3)
print("\ncold_slow (n=3, get_available_models() on fresh process):")
print(f" median: {cold_slow['median_ms']:.1f} ms "
f"min: {cold_slow['min_ms']:.1f} "
f"max: {cold_slow['max_ms']:.1f}")
warm_slow = sample(get_available_models, n=5)
print("\nwarm_slow (n=5, get_available_models() with hot cache):")
print(f" median: {warm_slow['median_ms']:.1f} ms "
f"min: {warm_slow['min_ms']:.1f} "
f"max: {warm_slow['max_ms']:.1f}")
print()
print("=" * 70)
print("FAST PATH (server-side module timing)")
print("=" * 70)
# Patch get_available_models to count invocations, so the test can
# confirm the fast path really skips the catalog.
import api.routes as routes
original = routes.get_available_models
calls = {"n": 0}
def counting(*a, **kw):
calls["n"] += 1
return original(*a, **kw)
routes.get_available_models = counting
try:
cold_fast = sample(
_resolve_compatible_session_model_state, 10,
"gpt-5.5", "openai-codex",
)
finally:
routes.get_available_models = original
print("\ncold_fast (n=10, _resolve_compatible_session_model_state, "
"model+provider supplied):")
print(f" median: {cold_fast['median_ms']:.3f} ms "
f"min: {cold_fast['min_ms']:.3f} "
f"max: {cold_fast['max_ms']:.3f}")
print(f" get_available_models() invocations: {calls['n']} (expected 0)")
print()
print("=" * 70)
print("HEADLINE DELTA")
print("=" * 70)
speedup = cold_slow["median_ms"] / max(cold_fast["median_ms"], 0.001)
print(f" cold_slow median: {cold_slow['median_ms']:.3f} ms")
print(f" cold_fast median: {cold_fast['median_ms']:.3f} ms")
print(f" speedup: {speedup:.1f}x faster on cold start")
print()
print(" => 1st + click after server boot goes from the cold_slow number")
print(" to the cold_fast number when this PR lands.")
print()
print("=" * 70)
print("SIMULATED COLD REBUILD (with 3.0s monkeypatched catalog delay)")
print("=" * 70)
print(" Why: a fresh dev box with no external API keys completes the")
print(" hardcoded-fallback path in well under 1ms, so the absolute")
print(" numbers above don't represent the production scenario from")
print(" the original #2518 triage (3-4s catalog rebuild when auth")
print(" probing, custom /v1/models, OpenRouter /models, or credential")
print(" pool refresh have to make network calls). This block")
print(" monkeypatches a 3.0s sleep into get_available_models() so the")
print(" before/after picture matches user-reported wall time.")
import time as _time
real_gam = routes.get_available_models
def slow_gam(*a, **kw):
_time.sleep(3.0)
return real_gam(*a, **kw)
routes.get_available_models = slow_gam
try:
# Simulate a fresh server restart by clearing the cache before
# measuring what a cold call costs.
cfg._available_models_cache = None
cfg._available_models_cache_ts = 0.0
# One priming call so the in-memory cache is fresh for the warm
# measurement that follows.
slow_gam()
t0 = time.perf_counter()
slow_gam()
sim_cold_slow = (time.perf_counter() - t0) * 1000
finally:
routes.get_available_models = real_gam
# Now simulate the patched client: model_provider supplied, fast path.
routes.get_available_models = slow_gam
try:
samples = []
for _ in range(5):
t0 = time.perf_counter()
_resolve_compatible_session_model_state("gpt-5.5", "openai-codex")
samples.append((time.perf_counter() - t0) * 1000)
sim_cold_fast = statistics.median(samples)
finally:
routes.get_available_models = real_gam
print(f"\n simulated cold_slow: {sim_cold_slow:.0f} ms (slow path on cold cache)")
print(f" simulated cold_fast: {sim_cold_fast:.2f} ms (fast path, never calls get_available_models())")
print(f" observed saving: {sim_cold_slow - sim_cold_fast:.0f} ms on the first + click")
if __name__ == "__main__":
sys.exit(main())

40
static/sessions.js
View File

@@ -516,7 +516,45 @@ async function newSession(flash, options={}){
}
if(newModelState&&newModelState.model){
reqBody.model=newModelState.model;
reqBody.model_provider=newModelState.model_provider||null;
// Cold-start / picker-without-provider fallback: when the dropdown option's
// data-provider is empty/'default' or the persisted state predates provider
// tracking, newModelState.model_provider is null. POST /api/session/new's
// fast path in _resolve_compatible_session_model_state requires both model
// and a truthy model_provider; without it, the request falls into
// get_available_models() and a 3-4s cold catalog rebuild. window._activeProvider
// is hydrated at boot (ui.js) and on config refresh (panels.js), so it's a
// safe default that matches the user's configured route. S.session.model_provider
// is the previous-session fallback when the dropdown is unhydrated.
//
// Guard: a slash-qualified model (e.g. "gemini/gemini-2.5") or an
// @provider:model string already carries a foreign provider namespace from
// a previous session that was served by a different backend. Attaching
// the current _activeProvider to such a slug would let the server's fast
// path pass it through without consulting the catalog, silently
// re-pointing the new session at the wrong backend (the very case the
// slow-path normalization in _resolve_compatible_session_model_state is
// designed to fix — see routes.py docstring around line 1891-1894). For
// those models we leave the wire shape with model_provider=null so the
// slow path's cross-provider repair still runs. Closes the open
// follow-up from #2518.
const _bareModel=!/[/]/.test(newModelState.model)&&!newModelState.model.startsWith('@');
// Second guard (#3410-followup): even a bare model can carry a known
// family prefix (gpt→openai, claude→anthropic, gemini→google). If that
// family maps to a DIFFERENT provider than the fallback we'd attach, the
// server fast path passes the pair through verbatim (no validation) and
// silently routes to the wrong backend — so leave model_provider=null and
// let the slow-path family repair run (mirrors routes.py _normalize_provider_id).
const _fallbackProvider=_bareModel?(window._activeProvider||(S.session&&S.session.model_provider)||''):'';
const _familyProvider=(m=>{const s=String(m||'').toLowerCase();
if(s.startsWith('gpt'))return 'openai';if(s.startsWith('claude'))return 'anthropic';
if(s.startsWith('gemini'))return 'google';return '';})(newModelState.model);
const _normProv=p=>{const s=String(p||'').toLowerCase();
if(s.startsWith('openai'))return 'openai';if(s.startsWith('anthropic')||s.startsWith('claude'))return 'anthropic';
if(s.startsWith('google')||s.startsWith('gemini'))return 'google';return s;};
const _familyMismatch=_familyProvider&&_fallbackProvider&&_normProv(_fallbackProvider)!==_familyProvider;
reqBody.model_provider=newModelState.model_provider
||((_bareModel&&!_familyMismatch)?(_fallbackProvider||null):null)
||null;
}
const data=await api('/api/session/new',{method:'POST',body:JSON.stringify(reqBody)});
S.session=data.session;S.messages=data.session.messages||[];

471
tests/test_issue2518_active_provider_fallback.py Normal file
View File

@@ -0,0 +1,471 @@
"""Tests for issue #2518 — cold-start /api/session/new slow path fallback.
The frontend in-flight guard (PR #2528, b76d698a) made repeated + clicks safe
but did not shorten a single cold click: newSession() in static/sessions.js
carries the dropdown's model_provider as ``reqBody.model_provider``. When the
dropdown option has no ``data-provider`` attribute (or its value is
``'default'``) and the persisted state predates provider tracking,
``newModelState.model_provider`` is null. The server's fast path in
``_resolve_compatible_session_model_state`` requires both ``model`` AND a
truthy ``model_provider``; without that, the request falls into
``get_available_models()`` and pays the 3-4s cold catalog rebuild on first
click after server boot.
These tests pin the follow-up fix: newSession() falls back to
``window._activeProvider`` (boot-hydrated) and then the previous session's
``model_provider`` so the fast path is hit whenever a usable default exists.
The slow path remains correct for users with no hydrated active provider and
no previous session — they get the catalog lookup, just like today.
Coverage:
1. newSession() source carries the active-provider fallback chain.
2. End-to-end: when client sends ``model_provider`` (either explicit or via
the new fallback), /api/session/new's resolve step does NOT call
``get_available_models()``.
3. Negative: client sends ``model_provider: null`` (no fallback available) —
resolve step still works via the slow path and returns the catalog's
default.
4. The fallback chain order is correct: explicit > _activeProvider >
previous-session > null.
"""
import pathlib
from unittest.mock import patch
REPO_ROOT = pathlib.Path(__file__).parent.parent.resolve()
def _read(rel_path: str) -> str:
return (REPO_ROOT / rel_path).read_text(encoding="utf-8")
# ---------------------------------------------------------------------------
# Client-side: source-shape check that the fallback is wired in newSession().
# ---------------------------------------------------------------------------
class TestClientFallbackSourceShape:
"""Static checks that the fallback chain lives inside newSession()."""
def test_active_provider_fallback_present(self):
src = _read("static/sessions.js")
idx = src.find("async function newSession(flash, options={}){")
assert idx != -1
body = src[idx:idx + 6000]
assert "window._activeProvider" in body, (
"newSession() must consult window._activeProvider when the dropdown "
"did not yield a truthy model_provider (cold boot, empty "
"data-provider, or pre-provider persisted state)."
)
def test_previous_session_fallback_present(self):
src = _read("static/sessions.js")
idx = src.find("async function newSession(flash, options={}){")
body = src[idx:idx + 6000]
assert "S.session&&S.session.model_provider" in body, (
"newSession() must fall back to the previous session's "
"model_provider when neither the dropdown nor window._activeProvider "
"is available (unhydrated dropdown, no active provider yet)."
)
def test_fallback_chain_order(self):
"""Fallback order: explicit > _activeProvider > prev-session > null."""
src = _read("static/sessions.js")
idx = src.find("async function newSession(flash, options={}){")
body = src[idx:idx + 6000]
explicit = body.find("newModelState.model_provider")
active = body.find("window._activeProvider")
prev = body.find("S.session&&S.session.model_provider")
assert -1 < explicit < active < prev, (
f"Fallback chain order broken: explicit={explicit}, "
f"_activeProvider={active}, prev-session={prev}. "
"Explicit selection must beat _activeProvider which must beat "
"the previous session's model_provider."
)
def test_issue_referenced_in_source(self):
"""Future readers should be able to trace this back to the issue."""
src = _read("static/sessions.js")
idx = src.find("async function newSession(flash, options={}){")
body = src[idx:idx + 4000]
assert "#2518" in body, (
"newSession()'s fallback comment should reference #2518 so the "
"follow-up provenance survives future refactors."
)
# ---------------------------------------------------------------------------
# End-to-end: with model_provider, /api/session/new skips the cold catalog.
# ---------------------------------------------------------------------------
class TestSessionNewFastPathWithProvider:
"""When client supplies a real model_provider, no catalog rebuild."""
def test_explicit_provider_skips_get_available_models(self):
"""The headline fix: client-supplied provider → fast path."""
from api.routes import _session_model_state_from_request
with patch("api.routes.get_available_models") as mock_catalog:
model, provider = _session_model_state_from_request(
"gpt-5.5",
"openai-codex",
)
assert mock_catalog.call_count == 0
assert model == "gpt-5.5"
assert provider == "openai-codex"
def test_active_provider_fallback_does_not_double_invoke_catalog(self):
"""Sanity: the fast path is shared between the explicit and fallback
cases on the client. As long as the client sent a truthy
model_provider, the server stays on the fast path. The actual
fallback selection happens client-side; this test pins that the
server side is invariant under the two client strategies."""
from api.routes import _session_model_state_from_request
# Simulate the two client strategies (explicit vs active-provider
# fallback) producing the same wire shape.
for client_provider in ("openai-codex", "anthropic", "openrouter"):
with patch("api.routes.get_available_models") as mock_catalog:
_session_model_state_from_request("claude-opus-4.7", client_provider)
assert mock_catalog.call_count == 0, (
f"client_provider={client_provider!r} must hit the fast path; "
f"otherwise the #2518 fallback is invisible to the server."
)
# ---------------------------------------------------------------------------
# Negative: when no provider is available anywhere, slow path is still correct.
# ---------------------------------------------------------------------------
class TestSessionNewSlowPathStillFiresWithoutProvider:
"""The slow path remains the safety net for genuinely provider-less clients."""
def test_null_provider_falls_back_to_catalog(self):
"""If the client really has nothing to send, the slow path must work."""
from api.routes import _session_model_state_from_request
with patch("api.routes.get_available_models") as mock_catalog:
mock_catalog.return_value = {
"active_provider": "openai-codex",
"default_model": "gpt-5.5",
"groups": [
{"provider_id": "openai-codex", "models": [{"id": "gpt-5.5"}]}
],
}
model, provider = _session_model_state_from_request("gpt-5.5", None)
# Slow path was taken because no provider was supplied.
assert mock_catalog.call_count == 1
# The slow path still returns a sane (model, provider) tuple.
assert model
assert provider
# ---------------------------------------------------------------------------
# Follow-up: slash-slug cross-provider guard raised during PR #3410 review.
# ---------------------------------------------------------------------------
#
# When the persisted state carries a stale foreign-slug model such as
# ``gemini/gemini-2.5`` from a session served by a different provider than
# the now-active one, the original PR's unconditional
# ``window._activeProvider`` fallback would attach the wrong provider to
# the new session and the server's fast path would pass it through without
# consulting the catalog — silently re-pointing the session at the wrong
# backend (the exact case ``_resolve_compatible_session_model_state``'s
# slow-path normalization is designed to fix, see routes.py:1891-1894).
#
# The fix gates the active-provider fallback behind a ``_bareModel`` check:
# slash-qualified and @-qualified models keep ``reqBody.model_provider``
# null so the server's slow-path cross-provider repair still runs. These
# tests pin the BEHAVIOR (gate present, explicit picker still wins,
# ordering preserved) rather than the source-string literal — a future
# refactor that keeps the same contract (e.g. extracting a helper or
# switching to a named regex) still satisfies them.
def _provider_assignment_in_new_session() -> str:
"""Extract the slash-slug guard + ``reqBody.model_provider``
assignment block in newSession() — from the ``const _bareModel``
declaration through the assignment's terminating semicolon.
The block is two statements glued by a single semicolon at the
end of each:
const _bareModel = !/[/]/.test(newModelState.model)
&& !newModelState.model.startsWith('@');
reqBody.model_provider = newModelState.model_provider
|| (_bareModel ? (window._activeProvider || (S.session && S.session.model_provider)) : null)
|| null;
Both lines live in the same 4000-char slice of newSession()'s
function body, so the helper can read them as a single contract
unit. Anchors on the ``=`` of the assignment (not a prose mention
in a comment) and on the guard declaration so future comments
referencing ``reqBody.model_provider`` cannot confuse it.
"""
src = _read("static/sessions.js")
idx = src.find("async function newSession(flash, options={}){")
assert idx != -1, "newSession() must be defined in static/sessions.js"
body = src[idx : idx + 6000]
guard_start = body.find("const _bareModel")
assert guard_start != -1, (
"newSession() must declare a 'const _bareModel' guard for the "
"cross-provider slash-slug regression from PR #3410 review."
)
# Slice from the _bareModel guard through the END of the
# `reqBody.model_provider=...;` assignment. The block now contains several
# helper declarations between the guard and the assignment (the family-
# mismatch guard added in the #3410-followup review), so anchor on the
# assignment's `=` (not a comment mention) and take through its terminating
# ';' rather than counting semicolons.
assign_at = body.find("reqBody.model_provider=", guard_start)
assert assign_at != -1, (
"reqBody.model_provider= assignment must appear after the _bareModel guard"
)
assign_end = body.find(";", assign_at)
assert assign_end != -1, "reqBody.model_provider assignment must terminate with ';'"
return body[guard_start : assign_end + 1]
class TestIssue2518FollowupSlashSlugGuard:
"""Regression coverage for the cross-provider slash-slug edge case
raised during PR #3410 review. The contract under test is:
1. A slash-qualified model (e.g. ``gemini/gemini-2.5``) MUST NOT pick
up ``window._activeProvider`` — the slow-path normalization in
``_resolve_compatible_session_model_state`` is the only correct
way to repair a foreign provider namespace.
2. An @-qualified model (e.g. ``@openai-codex:gpt-5.5``) similarly
MUST NOT pick up ``window._activeProvider`` — the
``@provider:model`` form already names a provider, and a
second one from the client would race the server's own
``_split_provider_qualified_model`` resolution.
3. Explicit picker selection (``newModelState.model_provider`` from
``_modelStateForSelect``) still wins over both fallbacks.
4. The fallback chain ordering remains: explicit > _activeProvider >
prev-session — guarded by the ``_bareModel`` ternary, not
short-circuited.
"""
def test_slash_qualified_model_keeps_active_provider_behind_guard(self):
"""`_bareModel` ternary must gate `_activeProvider`, and the
gate must trigger on a slash in the model id."""
expr = _provider_assignment_in_new_session()
# The guard is a ternary that flips to null for non-bare models.
assert "_bareModel" in expr, (
"newSession() must gate the _activeProvider fallback behind a "
"_bareModel ternary so slash-qualified models do not pick up "
"the wrong provider (cross-provider regression from PR #3410 "
"review)."
)
# The gate's predicate must include a slash check.
assert "/[/]/" in expr or "indexOf('/')" in expr or "includes('/')" in expr, (
f"Guard predicate must detect a '/' in newModelState.model; "
f"got expression: {expr!r}"
)
# And the active-provider fallback must live inside that ternary's
# truthy arm (via the _fallbackProvider helper), not on the top-level
# OR chain — otherwise a slash-slug would still get a provider attached.
ternary_true_arm_start = expr.find("(_bareModel&&")
if ternary_true_arm_start == -1:
ternary_true_arm_start = expr.find("(_bareModel?")
assert ternary_true_arm_start != -1, (
f"Expected a '_bareModel'-gated ternary in expression: {expr!r}"
)
# The truthy arm references _fallbackProvider, which is derived from
# window._activeProvider (then prev-session) only for bare models.
ternary_block = expr[ternary_true_arm_start:]
assert "_fallbackProvider" in ternary_block, (
"the _bareModel-gated arm must use _fallbackProvider (derived from "
"window._activeProvider) so non-bare models skip it entirely "
"(defense against cross-provider mismatch for persisted slash-slug state)."
)
# And _fallbackProvider itself must be sourced from _activeProvider.
assert "window._activeProvider" in expr, (
"_fallbackProvider must derive from window._activeProvider."
)
def test_at_qualified_model_also_keeps_active_provider_behind_guard(self):
"""`@provider:model` strings carry their own provider context and
must not pick up `_activeProvider` either — the server's
`_split_provider_qualified_model` is the source of truth for
those."""
expr = _provider_assignment_in_new_session()
# The guard's predicate must also check for an @ prefix.
assert "startsWith('@')" in expr or "startsWith(\"@\")" in expr, (
f"Guard predicate must also reject '@provider:model' strings; "
f"got expression: {expr!r}"
)
def test_explicit_picker_provider_still_wins(self):
"""Explicit picker provider (from ``_modelStateForSelect``) is
the highest-priority source — it must precede the guarded
fallback and the prev-session fallback in the assignment chain.
"""
expr = _provider_assignment_in_new_session()
# Runtime precedence (the contract): in the `reqBody.model_provider=`
# assignment, newModelState.model_provider is the FIRST operand of the
# `||` chain, so an explicit picker provider always wins. The bare-model
# fallback (_fallbackProvider) is the second operand. Within
# _fallbackProvider's own definition, _activeProvider precedes
# prev-session. (The helper is declared above the assignment, so a flat
# positional check across the whole block no longer applies — assert the
# two precedence facts that actually matter.)
assign_at = expr.find("reqBody.model_provider=")
assert assign_at != -1, f"no assignment in expr: {expr!r}"
assign = expr[assign_at:]
pos_explicit_in_assign = assign.find("newModelState.model_provider")
pos_fallback_in_assign = assign.find("_fallbackProvider")
assert -1 < pos_explicit_in_assign < pos_fallback_in_assign, (
f"explicit picker (newModelState.model_provider) must be the first "
f"operand, before the _fallbackProvider fallback, in the assignment: {assign!r}"
)
# _activeProvider precedes prev-session inside _fallbackProvider.
pos_active = expr.find("window._activeProvider")
pos_prev = expr.find("S.session&&S.session.model_provider")
assert -1 < pos_active < pos_prev, (
f"_fallbackProvider must source _activeProvider before prev-session: {expr!r}"
)
def test_no_op_null_terminal_in_fallback_chain(self):
"""The cleaned expression must not carry a *vestigial* mid-chain
``||null`` no-op directly on the top-level OR chain (the cosmetic
paste artifact flagged in PR #3410 review). Two legitimate ``||null``
terminals remain after the family-mismatch refactor: the inner
``_fallbackProvider||null`` (the bare-model arm's own fallback) and the
final ``||null`` terminal. Verify the OLD vestigial pattern
``model_provider||null||`` (null immediately after the explicit source)
is gone."""
expr = _provider_assignment_in_new_session()
assert "model_provider||null" not in expr.replace(" ", ""), (
f"newModelState.model_provider must not be directly followed by a "
f"'||null' no-op (the cosmetic paste artifact from PR #3410). "
f"Expression: {expr!r}"
)
def test_slash_slug_keeps_provider_null_in_wire_shape(self):
"""Behavior contract: when newSession() is given a slash-slug
model with no explicit picker provider and no previous-session
fallback, the wire-shape ``reqBody.model_provider`` must be
``null`` — the slow path's cross-provider normalization is the
only place that can repair a foreign slug.
We verify this by simulating the JS expression in pure Python so
the test is language-agnostic: the test only cares that the
client produces ``null`` for the right inputs, not how it spells
the JS source.
"""
# Mirror the JS expression structure. The contract is the
# predicate + the OR-chain shape, not the operator spelling.
new_model_state = {
"model": "gemini/gemini-2.5",
"model_provider": None, # _providerFromModelValue returns ''
}
bare = (
"/" not in new_model_state["model"]
and not new_model_state["model"].startswith("@")
)
active_provider = "openai-codex"
prev_session_provider = None
# Same expression shape as the new client code.
req_body_model_provider = (
new_model_state["model_provider"]
or (
active_provider
or prev_session_provider
)
if bare
else None
) or None
assert req_body_model_provider is None, (
f"Slash-slug model {new_model_state['model']!r} must send "
f"model_provider=null so the server's slow path can repair "
f"the cross-provider mismatch; got {req_body_model_provider!r}"
)
def test_bare_model_uses_active_provider_when_no_picker(self):
"""Behavior contract: a bare model with no explicit picker
provider but a hydrated active provider must still hit the
fast path — that is the whole point of the #2518 follow-up.
The _bareModel guard must not break this case.
"""
new_model_state = {"model": "gpt-5.5", "model_provider": None}
bare = (
"/" not in new_model_state["model"]
and not new_model_state["model"].startswith("@")
)
active_provider = "openai-codex"
prev_session_provider = None
req_body_model_provider = (
new_model_state["model_provider"]
or (
active_provider
or prev_session_provider
)
if bare
else None
) or None
assert req_body_model_provider == "openai-codex", (
f"Bare model {new_model_state['model']!r} with hydrated "
f"active provider must send it through so the fast path "
f"fires; got {req_body_model_provider!r}"
)
def test_bare_family_mismatch_keeps_provider_null(self):
"""Family-mismatch guard (Codex #3410-followup finding): a bare model
whose KNOWN family prefix (gpt/claude/gemini) maps to a DIFFERENT
provider than the fallback we'd attach must send model_provider=null,
so the server slow-path's family repair runs instead of the fast path
silently routing the model to the wrong backend.
Simulate the client's new logic in Python (family map + normalize),
mirroring static/sessions.js, and assert the wire shape.
"""
def _family_provider(m):
s = (m or "").lower()
if s.startswith("gpt"):
return "openai"
if s.startswith("claude"):
return "anthropic"
if s.startswith("gemini"):
return "google"
return ""
def _norm_prov(p):
s = (p or "").lower()
if s.startswith("openai"):
return "openai"
if s.startswith("anthropic") or s.startswith("claude"):
return "anthropic"
if s.startswith("google") or s.startswith("gemini"):
return "google"
return s
def _wire_provider(model, model_provider, active_provider, prev_provider):
bare = "/" not in model and not model.startswith("@")
fallback = (active_provider or prev_provider or "") if bare else ""
fam = _family_provider(model)
mismatch = bool(fam and fallback and _norm_prov(fallback) != fam)
return (
model_provider
or ((fallback or None) if (bare and not mismatch) else None)
or None
)
# claude-family bare model + openrouter active → MISMATCH → null (slow path repairs)
assert _wire_provider("claude-opus-4.8", None, "openrouter", None) is None
# gemini-family bare model + anthropic active → MISMATCH → null
assert _wire_provider("gemini-2.5-pro", None, "anthropic", None) is None
# gpt-family bare model + openai-codex active → MATCH → fast path
assert _wire_provider("gpt-5.5", None, "openai-codex", None) == "openai-codex"
# claude-family bare model + anthropic active → MATCH → fast path
assert _wire_provider("claude-opus-4.8", None, "anthropic", None) == "anthropic"
# unknown-family bare model (e.g. a custom/local id) + any provider → attaches (no family signal)
assert _wire_provider("my-local-model", None, "custom", None) == "custom"
# explicit picker provider always wins, even on a family mismatch
assert _wire_provider("claude-opus-4.8", "anthropic", "openrouter", None) == "anthropic"

38
tests/test_new_chat_default_model_frontend.py
View File

@@ -96,8 +96,44 @@ def test_new_chat_does_not_send_stale_dropdown_model_when_session_has_default_mo
def test_new_session_posts_picker_model_before_server_default():
fn = _new_session_function()
# Behavior contract: the picker model goes into reqBody so /api/session/new
# uses the user's selection before falling back to the server default
# (#872). The previous literal-string assertion
# "reqBody.model_provider=newModelState.model_provider||null" became a
# change-detector once the #2518 follow-up added a fallback chain; the
# contract that newModelState.model_provider is the FIRST source of
# reqBody.model_provider is now verified by substring + ordering.
assert "reqBody.model=newModelState.model" in fn
assert "reqBody.model_provider=newModelState.model_provider||null" in fn
assert "newModelState.model_provider" in fn
assert "window._activeProvider" in fn, (
"Cold-start fallback must consult window._activeProvider so "
"/api/session/new hits the resolve fast path (follow-up from #2518)."
)
assert "S.session&&S.session.model_provider" in fn, (
"Unhydrated-dropdown fallback must consult S.session.model_provider "
"before sending model_provider=null (follow-up from #2518)."
)
provider_assignment = fn[fn.index("reqBody.model_provider="):].split(";", 1)[0]
# The assignment sources from the explicit picker value first, then the
# bare-model fallback (_fallbackProvider, wired above from _activeProvider /
# prev-session), gated so a family-mismatched bare model defers to the
# server slow path. Anchor on the real `reqBody.model_provider=` assignment
# (not a comment) and verify the fallback wiring exists in the function body.
assert "newModelState.model_provider" in provider_assignment
assert "_fallbackProvider" in provider_assignment
assert "window._activeProvider" in fn
assert "S.session&&S.session.model_provider" in fn
# Ordering in the body: explicit picker value referenced before the
# _activeProvider fallback, which is referenced before prev-session.
pos_explicit = fn.index("newModelState.model_provider")
pos_active = fn.index("window._activeProvider")
pos_prev = fn.index("S.session&&S.session.model_provider")
assert pos_explicit < pos_active < pos_prev, (
"Fallback chain must be: explicit > _activeProvider > prev-session."
)
# Family-mismatch guard (Codex #3410-followup finding): a bare known-family
# model whose family differs from the fallback provider must NOT fast-path.
assert "_familyMismatch" in fn
assert "_readPersistedModelState" in fn

24
tests/test_provider_mismatch.py
View File

@@ -1204,7 +1204,29 @@ class TestFrontendModelProviderState:
body = src[start:src.index("const data=await api('/api/session/new'", start)]
assert "profile:S.activeProfile||'default'" in body
assert "reqBody.model=newModelState.model" in body
assert "reqBody.model_provider=newModelState.model_provider||null" in body
# Behavior contract (replaces the old literal-string pin
# `reqBody.model_provider=newModelState.model_provider||null`,
# which became a change-detector once the #2518 follow-up added
# a fallback chain — see AGENTS.md "Don't write change-detector
# tests"): reqBody.model_provider must source from
# newModelState.model_provider first, with the active provider
# and prev-session fallbacks wired in after. The block may
# gate the fallbacks behind a guard (e.g. the slash-slug
# _bareModel ternary from PR #3410) but the ordering and
# source names are part of the contract.
provider_assignment = body[body.index("reqBody.model_provider="):].split(";", 1)[0]
assert "newModelState.model_provider" in provider_assignment
assert "_fallbackProvider" in provider_assignment
assert "window._activeProvider" in body
assert "S.session&&S.session.model_provider" in body
pos_explicit = body.index("newModelState.model_provider")
pos_active = body.index("window._activeProvider")
pos_prev = body.index("S.session&&S.session.model_provider")
assert pos_explicit < pos_active < pos_prev, (
"Fallback chain order broken: explicit > _activeProvider > "
"prev-session must hold so /api/session/new hits the fast "
"path whenever a usable default exists (#2518 follow-up)."
)
def test_ui_has_json_model_state_storage(self):
src = _read("static/ui.js")