diff --git a/CHANGELOG.md b/CHANGELOG.md index 887c3e99..5d655e35 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,11 @@ ## [Unreleased] +## [v0.51.192] — 2026-05-31 — Release FL (stage-batch4 — per-model context_length default-only guard) + +### Fixed +- A global `model.context_length` cap (set in config for the default model, e.g. 232000) no longer silently shrinks **non-default** models' real context windows. The cap is now applied only when the session model equals `model.default`; other models (e.g. a 1M-context variant) keep their real metadata window. The guard is applied consistently across the session context-length resolver (`api/routes.py`), the per-turn persistence path, and the live SSE usage payload, and the auto-compress `threshold_tokens` is rescaled to the real cap so the context-window indicator and compression trigger reflect the actual window. The live-usage perf path caches the resolved per-model window once per stream (it runs ~10×/sec during streaming) so non-default-model streams don't take a config/metadata lookup on every metering tick. Backend-only; default-model sessions are unaffected. Closes #3256 (#3263, @allenliang2022). + ## [v0.51.191] — 2026-05-31 — Release FK (stage-batch3 — skills-detail markdown styling + launchd duplicate-start guard) ### Fixed diff --git a/api/routes.py b/api/routes.py index 69982cd6..23994f04 100644 --- a/api/routes.py +++ b/api/routes.py @@ -1750,6 +1750,70 @@ def _split_provider_qualified_model(model: str) -> tuple[str, str | None]: return model, None +def _model_matches_configured_default( + session_model: str | None, + cfg_default: str | None, + provider: str | None = None, +) -> bool: + """Return True when ``session_model`` refers to the configured ``model.default``. + + The global ``model.context_length`` cap applies ONLY to the default model + (#3256/#3263). An exact string compare is not enough because ``model.default`` + and the session model can be stored in different but equivalent shapes: + - bare: ``claude-opus-4.8`` + - slash-prefixed: ``anthropic/claude-opus-4.8`` (OpenRouter-style) + - @provider:model: ``@anthropic:claude-opus-4.8`` + + Matching rule (correct in both directions): + 1. Identical strings → match. + 2. Otherwise compare BARE model ids — BUT only after a provider-compatibility + check: if BOTH sides carry an identifiable provider (from a ``provider/`` + prefix, an ``@provider:`` qualifier, or the explicit ``provider`` arg for + the session side) and those providers DIFFER, it is NOT a match. This + stops a non-default model on a different provider that happens to share a + bare name (``openai/gpt-4o`` vs default ``openrouter/gpt-4o``) from being + treated as the default and wrongly receiving its cap. + 3. When a provider can't be identified on one side, fall through to the bare + comparison (lenient-when-unknown — a bare default config still matches a + bare/prefixed session model). + Empty default → no match. + """ + sess = str(session_model or "").strip() + default = str(cfg_default or "").strip() + if not sess or not default: + return False + if sess == default: + return True + + def _split(value: str) -> tuple[str, str | None]: + """Return (bare_model, provider_or_None) for any of the 3 shapes.""" + value = str(value or "").strip() + # @provider:model + unq, q_prov = _split_provider_qualified_model(value) + if q_prov: + return unq.strip(), str(q_prov).strip().lower() + # provider/model (single leading slash segment) + if "/" in value: + prefix, rest = value.split("/", 1) + return rest.strip(), prefix.strip().lower() + return value, None + + sess_bare, sess_prov = _split(sess) + default_bare, default_prov = _split(default) + # The explicit provider arg is the session side's provider when the model + # string itself didn't carry one. + if not sess_prov and provider: + sess_prov = str(provider).strip().lower() or None + + if not sess_bare or not default_bare or sess_bare != default_bare: + return False + # Bare ids match. Reject only when both sides name DIFFERENT providers. + if sess_prov and default_prov and sess_prov != default_prov: + return False + return True + + + def _should_attach_codex_provider_context(model: str, raw_active_provider: str, catalog: dict) -> bool: """Return True when a bare Codex model needs separate provider context. @@ -2045,8 +2109,16 @@ def _resolve_context_length_for_session_model( try: _model_cfg_load = _cfg_for_cl.get('model', {}) if isinstance(_cfg_for_cl, dict) else {} if isinstance(_model_cfg_load, dict): + # Only apply the global model.context_length override when the + # session model matches model.default. Otherwise a global cap + # set for the default model (e.g. 232000) silently clobbers + # other models' real metadata (e.g. a 1M-context variant). + _cfg_default_model = str(_model_cfg_load.get('default') or '').strip() _raw_cfg_ctx_load = _model_cfg_load.get('context_length') - if _raw_cfg_ctx_load is not None: + if _raw_cfg_ctx_load is not None and ( + not _cfg_default_model + or _model_matches_configured_default(model_for_lookup, _cfg_default_model, provider) + ): try: _parsed_load = int(_raw_cfg_ctx_load) if _parsed_load > 0: diff --git a/api/streaming.py b/api/streaming.py index a83d1e06..11d8c9e9 100644 --- a/api/streaming.py +++ b/api/streaming.py @@ -3912,6 +3912,15 @@ def _run_agent_streaming( _live_prompt_exact_tokens = [0] _live_prompt_estimate_tool_delta_tokens = [0] _live_prompt_estimate_seen_ids = set() + # Per-stream cache for the real per-model context_length (#3256 perf). + # _live_usage_snapshot() runs on every metering tick (~10x/sec during + # streaming); recomputing get_model_context_length() there triggered a + # config read + potential metadata/network probe on every token for + # non-default models (e.g. claude-opus-4.7-1m), freezing the stream while + # the default model was unaffected. The value is constant for a given + # (model, base_url, provider) within one stream, so resolve it at most + # once. Sentinel: None=not computed, 0=not applicable/failed, >0=real cap. + _real_ctx_cache = [None] def _seed_live_prompt_estimate() -> int: """Capture the latest exact prompt size before adding live tool deltas.""" @@ -3988,9 +3997,76 @@ def _run_agent_streaming( try: _cc = getattr(_agent, 'context_compressor', None) if _cc: - _usage['context_length'] = getattr(_cc, 'context_length', 0) or 0 - _usage['threshold_tokens'] = getattr(_cc, 'threshold_tokens', 0) or 0 - _usage['last_prompt_tokens'] = getattr(_cc, 'last_prompt_tokens', 0) or 0 + _cc_cl_u = getattr(_cc, 'context_length', 0) or 0 + # Default-only guard (#3256): the agent-side compressor is + # built in agent_init with the global model.context_length + # applied unconditionally — for non-default models that + # value is the stale global cap (e.g. 232K). Drop it here + # so the live usage payload doesn't surface the wrong cap. + # PERF: resolve the real per-model cap at most once per + # stream (cached in _real_ctx_cache). This snapshot runs on + # every metering tick; doing the config read + metadata + # lookup per tick froze non-default-model streams. + if _real_ctx_cache[0] is None: + _resolved_real = 0 # 0 = guard not applicable / failed + try: + from api.config import get_config as _gc_u + _cfg_u = _gc_u() + _mcfg_u = _cfg_u.get('model', {}) if isinstance(_cfg_u, dict) else {} + if isinstance(_mcfg_u, dict): + _def_u = str(_mcfg_u.get('default') or '').strip() + _raw_u = _mcfg_u.get('context_length') + try: + _cl_u = int(_raw_u) if _raw_u is not None else 0 + except (TypeError, ValueError): + _cl_u = 0 + _sm_u = str(getattr(_agent, 'model', '') or '').strip() + from api.routes import _model_matches_configured_default as _mmcd_u + if ( + _cl_u > 0 + and _cc_cl_u == _cl_u + and _def_u + and _sm_u + and not _mmcd_u(_sm_u, _def_u, getattr(_agent, 'provider', '') or '') + ): + # Recompute from real per-model metadata. + try: + from agent.model_metadata import get_model_context_length as _g_u + _real_u = _g_u( + _sm_u, + getattr(_agent, 'base_url', '') or '', + config_context_length=None, + provider=getattr(_agent, 'provider', '') or '', + ) or 0 + if _real_u: + _resolved_real = _real_u + except Exception: + pass + except Exception: + _resolved_real = 0 + _real_ctx_cache[0] = _resolved_real + # Apply the cached real cap when the guard determined one. + if _real_ctx_cache[0]: + # Also rescale threshold_tokens by the same ratio so the + # auto-compress trigger reflects the real window, not + # the stale global cap (e.g. 197.2k @ 232K cap → ~850k + # @ 1M real cap). + _orig_cc_cl = getattr(_cc, 'context_length', 0) or 0 + _orig_thresh = getattr(_cc, 'threshold_tokens', 0) or 0 + _cc_cl_u = _real_ctx_cache[0] + if _orig_cc_cl > 0 and _orig_thresh > 0: + _scaled_thresh = int(_orig_thresh * _real_ctx_cache[0] / _orig_cc_cl) + _usage['context_length'] = _cc_cl_u + _usage['threshold_tokens'] = _scaled_thresh + _usage['last_prompt_tokens'] = getattr(_cc, 'last_prompt_tokens', 0) or 0 + else: + _usage['context_length'] = _cc_cl_u + _usage['threshold_tokens'] = _orig_thresh + _usage['last_prompt_tokens'] = getattr(_cc, 'last_prompt_tokens', 0) or 0 + else: + _usage['context_length'] = _cc_cl_u + _usage['threshold_tokens'] = getattr(_cc, 'threshold_tokens', 0) or 0 + _usage['last_prompt_tokens'] = getattr(_cc, 'last_prompt_tokens', 0) or 0 except Exception: pass @@ -5793,8 +5869,45 @@ def _run_agent_streaming( # block writes them to the session itself so GET /api/session # returns them on reload instead of falling back to 0. _cc_for_save = getattr(agent, 'context_compressor', None) + # Initialized before the compressor block so the #3256/#3263 + # threshold-rescale below is safe even when there is no + # compressor (fresh agent / interrupted stream): _skip_cc_cl + # stays False and _cc_cl stays 0, so the rescale is a no-op. + _skip_cc_cl = False + _cc_cl = 0 if _cc_for_save: - s.context_length = getattr(_cc_for_save, 'context_length', 0) or 0 + _cc_cl = getattr(_cc_for_save, 'context_length', 0) or 0 + # Same guard as routes._resolve_context_length_for_session_model: + # the agent-side context_compressor was constructed with the + # global model.context_length applied to EVERY model. If the + # session's model isn't model.default, that value is a stale + # cap (e.g. 232K) that would clobber the real 1M metadata + # on every stream end. In that case skip the compressor + # value and let the fallback resolver below recompute. + _skip_cc_cl = False + try: + _model_cfg_cc = _cfg.get('model', {}) if isinstance(_cfg, dict) else {} + if isinstance(_model_cfg_cc, dict): + _cfg_default_cc = str(_model_cfg_cc.get('default') or '').strip() + _raw_cfg_cl_cc = _model_cfg_cc.get('context_length') + try: + _cfg_cl_cc = int(_raw_cfg_cl_cc) if _raw_cfg_cl_cc is not None else 0 + except (TypeError, ValueError): + _cfg_cl_cc = 0 + _sess_model_cc = str(getattr(agent, 'model', resolved_model or '') or '').strip() + from api.routes import _model_matches_configured_default as _mmcd_cc + if ( + _cfg_cl_cc > 0 + and _cc_cl == _cfg_cl_cc + and _cfg_default_cc + and _sess_model_cc + and not _mmcd_cc(_sess_model_cc, _cfg_default_cc, resolved_provider or '') + ): + _skip_cc_cl = True + except Exception: + pass + if not _skip_cc_cl: + s.context_length = _cc_cl s.threshold_tokens = getattr(_cc_for_save, 'threshold_tokens', 0) or 0 s.last_prompt_tokens = getattr(_cc_for_save, 'last_prompt_tokens', 0) or 0 # Fallback: if the compressor didn't report a context_length @@ -5812,7 +5925,14 @@ def _run_agent_streaming( # window in the persisted session and the SSE payload — # which then trips LCM auto-compress at ~25% of the wrong # value, cascading into 429 floods. - if not getattr(s, 'context_length', 0): + # + # #3256/#3263: ALSO run this fallback when _skip_cc_cl is true + # (non-default model whose compressor carried the stale global + # cap). Without this, a session that already had a stale 232K + # context_length persisted keeps it forever — skipping the + # compressor write removes the re-clobber but never recomputes + # the real per-model window. Recompute and overwrite in that case. + if (not getattr(s, 'context_length', 0)) or _skip_cc_cl: try: from agent.model_metadata import get_model_context_length _cfg_ctx_len = None @@ -5821,7 +5941,20 @@ def _run_agent_streaming( _model_cfg_for_ctx = _cfg.get('model', {}) if isinstance(_cfg, dict) else {} if isinstance(_model_cfg_for_ctx, dict): _raw_cfg_ctx = _model_cfg_for_ctx.get('context_length') - if _raw_cfg_ctx is not None: + # Default-only guard: only apply the global + # model.context_length cap when the session + # model equals model.default. Otherwise the + # cap (e.g. 232K set for the default model) + # silently shrinks other models' real metadata. + _cfg_default_ctx = str(_model_cfg_for_ctx.get('default') or '').strip() + _sess_model_ctx = str(getattr(agent, 'model', resolved_model or '') or '').strip() + from api.routes import _model_matches_configured_default as _mmcd_ctx + _apply_cfg_ctx = ( + not _cfg_default_ctx + or not _sess_model_ctx + or _mmcd_ctx(_sess_model_ctx, _cfg_default_ctx, resolved_provider or '') + ) + if _raw_cfg_ctx is not None and _apply_cfg_ctx: try: _parsed_cfg_ctx = int(_raw_cfg_ctx) if _parsed_cfg_ctx > 0: @@ -5863,6 +5996,23 @@ def _run_agent_streaming( # Older hermes-agent builds may not expose this helper. # Better to leave context_length=0 than crash the save. pass + # #3256/#3263: when we skipped the stale compressor cap for a + # non-default model and recomputed the real per-model window + # above, rescale the persisted threshold_tokens to that real cap + # so the auto-compress trigger and the reloaded context-ring + # match the live snapshot (which already rescales). Without this, + # a reload shows a smaller compression trigger than streaming did. + # Only rescale when both the original cap and threshold are + # positive; otherwise clear the threshold to 0 (consistent with + # the live-snapshot path) rather than leave a stale value. + if _skip_cc_cl: + _orig_cap = _cc_cl # the stale global cap the compressor reported + _orig_thresh = getattr(s, 'threshold_tokens', 0) or 0 + _real_cap = getattr(s, 'context_length', 0) or 0 + if _real_cap > 0 and _orig_cap > 0 and _orig_thresh > 0: + s.threshold_tokens = int(_orig_thresh * _real_cap / _orig_cap) + else: + s.threshold_tokens = 0 if not ephemeral and s.messages: _latest_assistant_idx = next( (idx for idx in range(len(s.messages) - 1, -1, -1) @@ -5991,7 +6141,45 @@ def _run_agent_streaming( # survive a page reload; this block only populates the live SSE usage payload. _cc = getattr(agent, 'context_compressor', None) if _cc: - usage['context_length'] = getattr(_cc, 'context_length', 0) or 0 + _cc_cl_sse = getattr(_cc, 'context_length', 0) or 0 + # #3256/#3263: remember the original compressor cap + threshold + # so that if we drop the stale cap below and the fallback + # resolves the real per-model window, we can rescale the + # threshold consistently (the live snapshot already does this). + _orig_cc_cl_sse = _cc_cl_sse + _orig_cc_thresh_sse = getattr(_cc, 'threshold_tokens', 0) or 0 + _dropped_stale_cap_sse = False + # Default-only guard (#3256): the agent-side context_compressor + # is constructed in agent_init with the global model.context_length + # applied unconditionally, so for non-default models its + # context_length is the stale global cap (e.g. 232K) — surfacing + # it via SSE makes the indicator show the wrong window even + # after the session was correctly resized. Drop the compressor + # value in that case and let the fallback resolver below recompute. + try: + _model_cfg_sse = _cfg.get('model', {}) if isinstance(_cfg, dict) else {} + if isinstance(_model_cfg_sse, dict): + _cfg_default_sse = str(_model_cfg_sse.get('default') or '').strip() + _raw_cfg_cl_sse = _model_cfg_sse.get('context_length') + try: + _cfg_cl_sse = int(_raw_cfg_cl_sse) if _raw_cfg_cl_sse is not None else 0 + except (TypeError, ValueError): + _cfg_cl_sse = 0 + _sess_model_sse = str(getattr(agent, 'model', resolved_model or '') or '').strip() + from api.routes import _model_matches_configured_default as _mmcd_sse + if ( + _cfg_cl_sse > 0 + and _cc_cl_sse == _cfg_cl_sse + and _cfg_default_sse + and _sess_model_sse + and not _mmcd_sse(_sess_model_sse, _cfg_default_sse, resolved_provider or '') + ): + _cc_cl_sse = 0 + _dropped_stale_cap_sse = True + except Exception: + pass + if _cc_cl_sse: + usage['context_length'] = _cc_cl_sse usage['threshold_tokens'] = getattr(_cc, 'threshold_tokens', 0) or 0 usage['last_prompt_tokens'] = getattr(_cc, 'last_prompt_tokens', 0) or 0 # Fallback: when the compressor is absent or reports context_length=0, @@ -6013,7 +6201,19 @@ def _run_agent_streaming( _model_cfg_for_ctx = _cfg.get('model', {}) if isinstance(_cfg, dict) else {} if isinstance(_model_cfg_for_ctx, dict): _raw_cfg_ctx = _model_cfg_for_ctx.get('context_length') - if _raw_cfg_ctx is not None: + # Default-only guard (see #3256): the global + # model.context_length cap only applies to + # model.default; other models keep their real + # metadata. + _cfg_default_ctx = str(_model_cfg_for_ctx.get('default') or '').strip() + _sess_model_ctx = str(getattr(agent, 'model', resolved_model or '') or '').strip() + from api.routes import _model_matches_configured_default as _mmcd_sfb + _apply_cfg_ctx = ( + not _cfg_default_ctx + or not _sess_model_ctx + or _mmcd_sfb(_sess_model_ctx, _cfg_default_ctx, resolved_provider or '') + ) + if _raw_cfg_ctx is not None and _apply_cfg_ctx: try: _parsed_cfg_ctx = int(_raw_cfg_ctx) if _parsed_cfg_ctx > 0: @@ -6041,6 +6241,15 @@ def _run_agent_streaming( ) if _fb_cl: usage['context_length'] = _fb_cl + # #3256/#3263: if we dropped the stale compressor cap + # for a non-default model, the threshold_tokens written + # above is still the stale compressor value. Rescale it + # to the real resolved window so the terminal `done` + # payload matches the live snapshot (which rescales) — + # otherwise messages.js overwrites S.lastUsage with the + # stale threshold and the indicator reverts on stream end. + if _dropped_stale_cap_sse and _orig_cc_cl_sse > 0 and _orig_cc_thresh_sse > 0: + usage['threshold_tokens'] = int(_orig_cc_thresh_sse * _fb_cl / _orig_cc_cl_sse) except Exception: pass # Fallback: when last_prompt_tokens is missing (no compressor), use the diff --git a/tests/test_issue3256_context_length_default_only_guard.py b/tests/test_issue3256_context_length_default_only_guard.py new file mode 100644 index 00000000..fab27aba --- /dev/null +++ b/tests/test_issue3256_context_length_default_only_guard.py @@ -0,0 +1,203 @@ +"""Regression coverage for #3256 / #3263: the global model.context_length cap +must apply ONLY when the session model equals model.default. + +Bug: a global `model.context_length` (set for the default model, e.g. 232000) +was applied to EVERY model, silently shrinking a non-default model's real +context window (e.g. a 1M-context variant). The fix scopes the override to +`model.default` only, in `_resolve_context_length_for_session_model`. + +This test drives the real resolver with a monkeypatched +`agent.model_metadata.get_model_context_length` that records the +`config_context_length` it receives, so we assert the gating without needing +the real agent metadata catalog. +""" +import sys +import types +from pathlib import Path as _Path + + +def _install_fake_get_model_context_length(monkeypatch, recorder): + """Install a fake get_model_context_length into a stand-in agent.model_metadata.""" + mod = types.ModuleType("agent.model_metadata") + + def _fake(model, base_url="", config_context_length=None, provider="", custom_providers=None): + recorder["model"] = model + recorder["config_context_length"] = config_context_length + # Pretend the real per-model metadata window is 1,000,000 unless the + # caller forced a config cap, in which case honor the cap (mirrors the + # real helper's contract). + if config_context_length: + return int(config_context_length) + return 1_000_000 + + mod.get_model_context_length = _fake + # Ensure a parent `agent` package exists so `from agent.model_metadata import ...` resolves. + if "agent" not in sys.modules: + agent_pkg = types.ModuleType("agent") + agent_pkg.__path__ = [] + monkeypatch.setitem(sys.modules, "agent", agent_pkg) + monkeypatch.setitem(sys.modules, "agent.model_metadata", mod) + + +def _resolver(): + import api.routes as routes + return routes._resolve_context_length_for_session_model + + +def test_global_cap_applies_to_default_model(monkeypatch): + """When the session model IS model.default, the global cap is passed through.""" + import api.config as config + rec = {} + _install_fake_get_model_context_length(monkeypatch, rec) + monkeypatch.setattr( + config, "get_config", + lambda *a, **k: {"model": {"default": "claude-opus-4.8", "context_length": 232000}}, + ) + result = _resolver()("claude-opus-4.8") + assert rec["config_context_length"] == 232000, "default model must receive the global cap" + assert result == 232000 + + +def test_global_cap_NOT_applied_to_non_default_model(monkeypatch): + """When the session model is NOT model.default, the global cap is dropped so + the model's real (larger) window is used — the core #3256/#3263 fix.""" + import api.config as config + rec = {} + _install_fake_get_model_context_length(monkeypatch, rec) + monkeypatch.setattr( + config, "get_config", + lambda *a, **k: {"model": {"default": "claude-opus-4.8", "context_length": 232000}}, + ) + result = _resolver()("claude-opus-4.7-1m-internal") + assert rec["config_context_length"] is None, ( + "non-default model must NOT receive the global cap (it would clobber real metadata)" + ) + assert result == 1_000_000, "non-default model should resolve to its real 1M window, not the 232K cap" + + +def test_no_default_configured_still_applies_cap(monkeypatch): + """If model.default is unset, the cap applies (backward-compatible).""" + import api.config as config + rec = {} + _install_fake_get_model_context_length(monkeypatch, rec) + monkeypatch.setattr( + config, "get_config", + lambda *a, **k: {"model": {"context_length": 200000}}, + ) + result = _resolver()("some-model") + assert rec["config_context_length"] == 200000 + assert result == 200000 + + +def test_empty_model_returns_zero(monkeypatch): + rec = {} + _install_fake_get_model_context_length(monkeypatch, rec) + assert _resolver()("") == 0 + assert _resolver()(None) == 0 + + +# --- #3263 provider-aware default match (Codex final-gate finding, v0.51.192) --- +# model.default and the session model can be stored in equivalent-but-different +# shapes (bare / provider-prefixed / @provider:model). An exact string compare +# wrongly treats the actual default model as non-default and drops its configured +# context_length cap. _model_matches_configured_default() normalizes the shapes. + +def _matcher(): + import api.routes as routes + return routes._model_matches_configured_default + + +def test_default_match_exact(): + assert _matcher()("claude-opus-4.8", "claude-opus-4.8") is True + + +def test_default_match_bare_session_vs_prefixed_default(): + # Config default is provider-prefixed; session model is bare — still the default. + assert _matcher()("claude-opus-4.8", "anthropic/claude-opus-4.8", "anthropic") is True + + +def test_default_match_prefixed_session_vs_bare_default(): + assert _matcher()("anthropic/claude-opus-4.8", "claude-opus-4.8", "anthropic") is True + + +def test_default_match_at_qualified_session(): + assert _matcher()("@anthropic:claude-opus-4.8", "claude-opus-4.8") is True + + +def test_default_match_distinct_models_do_not_match(): + assert _matcher()("claude-opus-4.7-1m", "claude-opus-4.8", "anthropic") is False + assert _matcher()("claude-opus-4.7-1m", "anthropic/claude-opus-4.8", "anthropic") is False + assert _matcher()("gpt-5.5", "claude-opus-4.8", "openai") is False + + +def test_default_match_same_bare_different_provider_does_NOT_match(): + """Same bare model id on a DIFFERENT provider is NOT the configured default + and must not receive its cap (Codex final-gate over-match finding).""" + assert _matcher()("openai/gpt-4o", "openrouter/gpt-4o", "openai") is False + assert _matcher()("@openai:gpt-4o", "@openrouter:gpt-4o") is False + assert _matcher()("gpt-4o", "openrouter/gpt-4o", "openai") is False + # but the SAME provider (or unknown session provider) still matches: + assert _matcher()("openrouter/gpt-4o", "openrouter/gpt-4o") is True + assert _matcher()("gpt-4o", "openrouter/gpt-4o", "openrouter") is True + + +def test_default_match_empty_inputs_are_false(): + assert _matcher()("claude-opus-4.8", "") is False + assert _matcher()("", "claude-opus-4.8") is False + + +def test_prefixed_default_still_receives_cap(monkeypatch): + """The actual default model, configured provider-prefixed, must still get + the global cap when the session stores it bare (the Codex final-gate bug).""" + import api.config as config + rec = {} + _install_fake_get_model_context_length(monkeypatch, rec) + monkeypatch.setattr( + config, "get_config", + lambda *a, **k: {"model": {"default": "anthropic/claude-opus-4.8", "context_length": 232000}}, + ) + # session model is the bare form of the prefixed default + result = _resolver()("claude-opus-4.8", "anthropic") + assert rec["config_context_length"] == 232000, ( + "provider-prefixed default model must still receive its configured cap" + ) + assert result == 232000 + + +# --- #3263 dual-gate MUST-FIX invariants (Codex regression gate, v0.51.192) --- +# These pin the two consistency fixes applied after the gate found that the +# default-only guard dropped the stale cap but didn't (a) recompute a persisted +# stale context_length, or (b) rescale the terminal SSE threshold. Both live +# deep inside _run_agent_streaming, so we pin them at the source-structure level +# (the live-snapshot path already had behavioral coverage; these guard the two +# sibling paths from silently regressing back to the stale value). +_STREAMING_SRC = (_Path(__file__).resolve().parent.parent / "api" / "streaming.py").read_text(encoding="utf-8") + + +def test_persistence_fallback_also_runs_when_skip_cc_cl(): + """The per-turn persistence fallback must recompute the real cap when the + stale compressor cap was skipped — not only when context_length is falsy. + Otherwise a previously-persisted stale 232K survives forever.""" + assert "(not getattr(s, 'context_length', 0)) or _skip_cc_cl:" in _STREAMING_SRC, ( + "persistence fallback gate must also fire on _skip_cc_cl (#3263 MUST-FIX 1)" + ) + + +def test_persistence_rescales_threshold_when_cap_skipped(): + """When the stale cap is skipped and the real cap recomputed, the persisted + threshold_tokens must be rescaled to the real cap (or cleared), so a reload + matches the live snapshot.""" + assert "if _skip_cc_cl:" in _STREAMING_SRC + assert "s.threshold_tokens = int(_orig_thresh * _real_cap / _orig_cap)" in _STREAMING_SRC, ( + "persistence path must rescale threshold_tokens to the real cap (#3263 MUST-FIX 2)" + ) + + +def test_sse_done_payload_rescales_threshold_when_cap_dropped(): + """The terminal SSE usage payload must rescale threshold_tokens when it + dropped the stale compressor cap, so the indicator doesn't revert on stream + end (messages.js overwrites S.lastUsage with this payload).""" + assert "_dropped_stale_cap_sse" in _STREAMING_SRC + assert "usage['threshold_tokens'] = int(_orig_cc_thresh_sse * _fb_cl / _orig_cc_cl_sse)" in _STREAMING_SRC, ( + "SSE done payload must rescale threshold_tokens to the resolved window (#3263 MUST-FIX 3)" + ) diff --git a/tests/test_pr1318_context_length_fallback.py b/tests/test_pr1318_context_length_fallback.py index ec4e3add..e564a19b 100644 --- a/tests/test_pr1318_context_length_fallback.py +++ b/tests/test_pr1318_context_length_fallback.py @@ -47,17 +47,21 @@ def test_fallback_uses_model_metadata(): def test_fallback_gates_on_falsy_context_length(): - """Fallback runs only when the compressor didn't populate s.context_length. + """Fallback runs when the compressor didn't populate s.context_length. - The gate must check s.context_length (not _cc_for_save) — if the compressor - set context_length but it was 0, we still want the fallback to fire. + The gate must still check s.context_length being falsy — if the compressor + set context_length but it was 0, we want the fallback to fire. (#3256/#3263 + widened the gate to ALSO fire on `_skip_cc_cl` — a non-default model whose + compressor carried the stale global cap — so the gate is now + `(not getattr(s, 'context_length', 0)) or _skip_cc_cl`. The falsy-check + remains; this test asserts that invariant is intact, not the exact spelling.) """ block = _persistence_block() - # The conditional must reference s.context_length (or getattr(s, 'context_length', 0)) + # The conditional must still reference s.context_length being falsy. assert ( - "if not getattr(s, 'context_length'" in block - or "if not s.context_length" in block - ), "Fallback must gate on s.context_length being falsy" + "not getattr(s, 'context_length'" in block + or "not s.context_length" in block + ), "Fallback must still gate on s.context_length being falsy" def test_fallback_passes_model_and_base_url(): diff --git a/tests/test_pr1341_context_window_persistence.py b/tests/test_pr1341_context_window_persistence.py index bcbf99c3..97009278 100644 --- a/tests/test_pr1341_context_window_persistence.py +++ b/tests/test_pr1341_context_window_persistence.py @@ -38,12 +38,14 @@ def test_streaming_persists_context_fields_on_session_before_save(): # Save call follows shortly after save_call = src.find("\n s.save()", block_start) assert save_call != -1, "s.save() not found after the post-merge marker" - # Limit bumped to 9000 by cancellation finalization guards: the block now also - # checks for a late user cancel immediately before the durable final save, - # preventing a race that would otherwise save/emit a completed turn after Stop. - # The context_length fallback is still a single focused resolver call with - # arg-prep scaffold and commentary explaining the failure mode it prevents. - assert save_call - block_start < 9000, ( + # Limit bumped to 15000 by the #3256/#3263 default-only context_length guard + # plus its dual-gate consistency fixes (recompute persisted stale cap + + # rescale threshold_tokens). The pre-save block legitimately grew here. NOTE: + # this byte-distance assertion is itself brittle (it must be bumped whenever a + # legitimate pre-save mutation block is added) — a structural check (presence + # of s.save() shortly after the post-merge marker) would be more durable; left + # as a follow-up. Earlier limits: 9000 (cancellation guards) → 13000 (#3263 v1). + assert save_call - block_start < 15000, ( "s.save() should be close to the post-merge marker — block expanded unexpectedly. " "If you've added a new pre-save mutation block here, bump this limit." )