From ca753b96cbc7d808280e2e6644e42e73ea067800 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 26 Aug 2026 22:14:26 -0700 Subject: [PATCH] fix(tui-gateway): unset semantics for every live-adopted compression/model key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Independent review finding on the merged #95980: _apply_live_compression_config only acted on PRESENT keys, so removing tail_mode / model.context_length / target_ratio / model_thresholds / proactive_prune_* / protect_last_n / min_tail_user_messages / threshold / idle_compact_after_seconds from config.yaml left stale values active in live sessions forever (probe-verified: all six stale after applying empty mappings). Absence now restores the normalized default — or the model-derived value — through the SAME derivation the construction path uses: - ContextCompressor ctor defaults read off its real __init__ signature (no hardcoded copies to drift) - compression.threshold removal re-derives via agent_init's _resolve_compression_threshold (Codex gpt-5.4/5.5 + spark autoraise included) - model.context_length removal drops the config override and forces re-inference through the deferred get_model_context_length resolution, which also re-applies the small-context threshold floor - model_thresholds removal clears stale per-model overrides from the live threshold; tail_mode falls back to the ctor's 'lean' (the old present-key path normalized invalid values to 'legacy', diverging from the compressor's own fallback) Also fixes proactive_prune_min_reclaim_tokens's present-but-null default (was 0; the real default is 4096). Refs #94724 --- .../test_compression_config_hot_reload.py | 126 +++++++++++ tui_gateway/server.py | 208 ++++++++++++++---- 2 files changed, 290 insertions(+), 44 deletions(-) diff --git a/tests/tui_gateway/test_compression_config_hot_reload.py b/tests/tui_gateway/test_compression_config_hot_reload.py index 36e8ec941f..90caaa43dd 100644 --- a/tests/tui_gateway/test_compression_config_hot_reload.py +++ b/tests/tui_gateway/test_compression_config_hot_reload.py @@ -107,3 +107,129 @@ def test_prompt_submit_calls_compression_sync_after_model_sync(): assert model_idx != -1 assert compression_idx != -1 assert model_idx < compression_idx + + +# ── Unset semantics (#94724 review finding on #95980) ──────────────────── +# ``_apply_live_compression_config`` used to act only on PRESENT keys, so +# removing tail_mode / context_length / target_ratio / model_thresholds / +# proactive_prune_* / protect_last_n / min_tail_user_messages / threshold / +# idle_compact_after_seconds from config.yaml left stale values active in +# live sessions forever. Absence must restore the normalized default (or the +# model-derived value) through the same derivation the agent-construction +# path uses. + + +def _neutral_session(**compression_ctor): + """Session on a model with no per-model threshold override in play.""" + compressor = ContextCompressor( + model="unset-test-model", + config_context_length=600_000, # >=512K: no small-context floor + quiet_mode=True, + **compression_ctor, + ) + agent = SimpleNamespace( + model="unset-test-model", + provider="", + context_compressor=compressor, + compression_enabled=True, + compression_idle_compact_after_seconds=0, + ) + return {"agent": agent, "session_key": "session-unset"}, compressor + + +def _sync_with_cfg(monkeypatch, session, cfg): + monkeypatch.setattr(server, "_load_cfg", lambda: cfg) + server._sync_agent_compression_with_config("sid-unset", session) + + +def test_removing_tail_mode_restores_lean_default(monkeypatch): + session, compressor = _neutral_session(tail_mode="legacy") + assert compressor.tail_mode == "legacy" + _sync_with_cfg(monkeypatch, session, {"compression": {}}) + assert compressor.tail_mode == "lean" + + +def test_removing_target_ratio_restores_default(monkeypatch): + session, compressor = _neutral_session(summary_target_ratio=0.60) + assert compressor.summary_target_ratio == 0.60 + _sync_with_cfg(monkeypatch, session, {"compression": {}}) + assert compressor.summary_target_ratio == 0.20 + + +def test_removing_protect_last_n_restores_default(monkeypatch): + session, compressor = _neutral_session(protect_last_n=5) + _sync_with_cfg(monkeypatch, session, {"compression": {}}) + assert compressor.protect_last_n == 20 + + +def test_removing_proactive_prune_keys_restores_defaults(monkeypatch): + session, compressor = _neutral_session( + proactive_prune_tokens=48_000, + proactive_prune_min_result_chars=30_000, + proactive_prune_min_reclaim_tokens=1, + ) + _sync_with_cfg(monkeypatch, session, {"compression": {}}) + assert compressor.proactive_prune_tokens == 0 + assert compressor.proactive_prune_min_result_chars == 8000 + assert compressor.proactive_prune_min_reclaim_tokens == 4096 + + +def test_removing_min_tail_user_messages_restores_default(monkeypatch): + session, compressor = _neutral_session(min_tail_user_messages=4) + _sync_with_cfg(monkeypatch, session, {"compression": {}}) + assert compressor.min_tail_user_messages == 1 + + +def test_removing_model_thresholds_restores_empty_map(monkeypatch): + session, compressor = _neutral_session( + model_thresholds={"unset-test-model": 0.95} + ) + assert compressor.threshold_percent == 0.95 + _sync_with_cfg(monkeypatch, session, {"compression": {}}) + assert compressor.model_thresholds == {} + # The stale per-model override must stop steering the live threshold too. + assert compressor.threshold_percent == 0.50 + + +def test_removing_threshold_restores_derived_default(monkeypatch): + session, compressor = _neutral_session(threshold_percent=0.85) + assert compressor.threshold_percent == 0.85 + _sync_with_cfg( + monkeypatch, + session, + {"model": {"context_length": 600_000}, "compression": {}}, + ) + assert compressor._config_threshold_percent == 0.50 + assert compressor.threshold_percent == 0.50 + assert compressor.threshold_tokens == int(600_000 * 0.50) + + +def test_removing_context_length_reinfers_from_model_metadata(monkeypatch): + import agent.context_compressor as cc_mod + + session, compressor = _neutral_session() + assert compressor.context_length == 600_000 + + monkeypatch.setattr( + cc_mod, + "get_model_context_length", + lambda *a, **k: 1_000_000, + ) + _sync_with_cfg(monkeypatch, session, {"model": {}, "compression": {}}) + assert compressor._config_context_length is None + assert compressor.context_length == 1_000_000 + assert compressor.threshold_tokens == int(1_000_000 * 0.50) + + +def test_removing_idle_compact_after_seconds_restores_zero(monkeypatch): + session, _ = _neutral_session() + session["agent"].compression_idle_compact_after_seconds = 1800 + _sync_with_cfg(monkeypatch, session, {"compression": {}}) + assert session["agent"].compression_idle_compact_after_seconds == 0 + + +def test_removing_enabled_restores_true(monkeypatch): + session, _ = _neutral_session() + session["agent"].compression_enabled = False + _sync_with_cfg(monkeypatch, session, {"compression": {}}) + assert session["agent"].compression_enabled is True diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 1c78363b13..f110cc0d8f 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -6093,6 +6093,73 @@ def _tui_compression_config_signature(cfg: dict | None) -> tuple: return tuple(sorted(picked.items())) +def _compressor_ctor_default(name: str, fallback: Any) -> Any: + """Read a normalized default from ContextCompressor's REAL signature. + + Unset restoration must go through the same derivation the construction + path uses (#94724 review finding on #95980) — pulling the default off + ``ContextCompressor.__init__`` itself instead of hardcoding copies keeps + the two from drifting. + """ + try: + import inspect + + from agent.context_compressor import ContextCompressor + + default = inspect.signature(ContextCompressor.__init__).parameters[ + name + ].default + if default is inspect.Parameter.empty: + return fallback + return default + except Exception: + return fallback + + +def _derived_default_threshold_percent(agent: Any, compression: dict) -> float: + """Default compaction threshold when ``compression.threshold`` is unset. + + Mirrors agent_init exactly: the ctor's global default, then the per-model + resolution (Codex gpt-5.4/5.5 + spark autoraise, Arcee Trinity, etc.) + via the SAME ``_resolve_compression_threshold`` helper — so removing the + key restores the model-derived value, not a bare constant. + """ + try: + pct = float(_compressor_ctor_default("threshold_percent", 0.50)) + except (TypeError, ValueError): + pct = 0.50 + try: + from agent.agent_init import _resolve_compression_threshold + from agent.auxiliary_client import ( + _compression_threshold_for_model, + _is_codex_gpt54_or_gpt55, + _is_codex_spark, + ) + + model = getattr(agent, "model", "") or "" + provider = getattr(agent, "provider", "") or "" + autoraise_enabled = str( + compression.get("codex_gpt55_autoraise", True) + ).lower() in {"true", "1", "yes"} + model_cthresh = _compression_threshold_for_model( + model, + provider, + allow_codex_gpt55_autoraise=autoraise_enabled, + ) + pct, _notice = _resolve_compression_threshold( + pct, + model_cthresh, + model=model, + is_codex_autoraise=( + _is_codex_gpt54_or_gpt55(model, provider) + or _is_codex_spark(model, provider) + ), + ) + except Exception: + pass + return pct + + def _apply_live_compression_config(agent: Any, cfg: dict | None) -> None: """Update a live session's compressor from current config.yaml. @@ -6100,6 +6167,15 @@ def _apply_live_compression_config(agent: Any, cfg: dict | None) -> None: Recomputes the trigger from the ratio-based threshold and then applies ``compression.threshold_tokens`` so raising, lowering, or clearing the cap all take effect on the next preflight. + + Every adopted key has UNSET semantics (#94724 review finding on the + merged #95980): removing a key from config.yaml restores the normalized + default — or the model-derived value — on the next turn, through the + same derivation the construction path uses (ContextCompressor ctor + defaults read off its real signature, the Codex threshold autoraise via + ``_resolve_compression_threshold``, context-length re-inference via the + deferred ``get_model_context_length`` resolution). The old behavior + acted only on PRESENT keys, leaving stale values active forever. """ cfg = cfg if isinstance(cfg, dict) else {} compression = cfg.get("compression") if isinstance(cfg.get("compression"), dict) else {} @@ -6111,10 +6187,8 @@ def _apply_live_compression_config(agent: Any, cfg: dict | None) -> None: else: agent.compression_enabled = str(enabled_raw).lower() in {"true", "1", "yes"} - idle_raw = compression.get( - "idle_compact_after_seconds", - getattr(agent, "compression_idle_compact_after_seconds", 0), - ) + # Absence restores the agent_init/config default (0 = disabled). + idle_raw = compression.get("idle_compact_after_seconds", 0) try: agent.compression_idle_compact_after_seconds = max(0, int(idle_raw or 0)) except (TypeError, ValueError): @@ -6124,31 +6198,55 @@ def _apply_live_compression_config(agent: Any, cfg: dict | None) -> None: if cc is None: return - if "tail_mode" in compression: - mode = str(compression.get("tail_mode") or "legacy").strip().lower() - cc.tail_mode = mode if mode in ("legacy", "lean") else "legacy" + # tail_mode: ctor normalization — unknown/absent values land on "lean", + # matching agent_init's default and the compressor's own fallback. + default_tail = str(_compressor_ctor_default("tail_mode", "lean")) + mode = str(compression.get("tail_mode", default_tail) or default_tail) + mode = mode.strip().lower() + cc.tail_mode = mode if mode in ("legacy", "lean") else default_tail def _assign_int(key: str, attr: str, default: int, min_value: int = 0) -> None: - if key not in compression: - return + raw = compression.get(key, default) try: - raw = compression.get(key) value = default if raw is None else int(raw) - setattr(cc, attr, max(min_value, value)) except (TypeError, ValueError): return + setattr(cc, attr, max(min_value, value)) - _assign_int("proactive_prune_tokens", "proactive_prune_tokens", 0) - _assign_int("proactive_prune_min_result_chars", "proactive_prune_min_result_chars", 8000) - _assign_int("proactive_prune_min_reclaim_tokens", "proactive_prune_min_reclaim_tokens", 0) - _assign_int("protect_last_n", "protect_last_n", 20) - _assign_int("min_tail_user_messages", "min_tail_user_messages", 1, min_value=1) + _assign_int( + "proactive_prune_tokens", + "proactive_prune_tokens", + int(_compressor_ctor_default("proactive_prune_tokens", 0)), + ) + _assign_int( + "proactive_prune_min_result_chars", + "proactive_prune_min_result_chars", + int(_compressor_ctor_default("proactive_prune_min_result_chars", 8000)), + ) + _assign_int( + "proactive_prune_min_reclaim_tokens", + "proactive_prune_min_reclaim_tokens", + int(_compressor_ctor_default("proactive_prune_min_reclaim_tokens", 4096)), + ) + _assign_int( + "protect_last_n", + "protect_last_n", + int(_compressor_ctor_default("protect_last_n", 20)), + ) + _assign_int( + "min_tail_user_messages", + "min_tail_user_messages", + int(_compressor_ctor_default("min_tail_user_messages", 1)), + min_value=1, + ) - if "target_ratio" in compression: - try: - cc.summary_target_ratio = max(0.10, min(float(compression["target_ratio"]), 0.80)) - except (TypeError, ValueError): - pass + try: + ratio_raw = compression.get( + "target_ratio", _compressor_ctor_default("summary_target_ratio", 0.20) + ) + cc.summary_target_ratio = max(0.10, min(float(ratio_raw), 0.80)) + except (TypeError, ValueError): + pass raw_thresholds = compression.get("model_thresholds") if isinstance(raw_thresholds, dict): @@ -6157,34 +6255,46 @@ def _apply_live_compression_config(agent: Any, cfg: dict | None) -> None: for k, v in raw_thresholds.items() if isinstance(v, (int, float)) and not isinstance(v, bool) } + else: + # Absent (or invalid shape — agent_init treats both as empty): + # stale per-model overrides must stop steering the live threshold. + cc.model_thresholds = {} + # threshold: present value wins; absence derives the default through + # the same agent_init resolution (global default + per-model autoraise). + pct: float | None = None if "threshold" in compression: try: pct = float(compression["threshold"]) - cc._config_threshold_percent = pct - cc._configured_threshold_percent = pct - base = pct - model_thresholds = getattr(cc, "model_thresholds", None) or {} - if model_thresholds: - from agent.context_compressor import resolve_model_threshold - - base = resolve_model_threshold( - getattr(agent, "model", "") or "", - model_thresholds, - pct, - ) - cc._base_threshold_percent = base - if hasattr(cc, "_effective_threshold_percent"): - try: - cc.threshold_percent = cc._effective_threshold_percent( - cc.context_length, base - ) - except Exception: - cc.threshold_percent = pct - else: - cc.threshold_percent = pct except (TypeError, ValueError): - pass + pct = None + if pct is None: + pct = _derived_default_threshold_percent(agent, compression) + try: + cc._config_threshold_percent = pct + cc._configured_threshold_percent = pct + base = pct + model_thresholds = getattr(cc, "model_thresholds", None) or {} + if model_thresholds: + from agent.context_compressor import resolve_model_threshold + + base = resolve_model_threshold( + getattr(agent, "model", "") or "", + model_thresholds, + pct, + ) + cc._base_threshold_percent = base + if hasattr(cc, "_effective_threshold_percent"): + try: + cc.threshold_percent = cc._effective_threshold_percent( + cc.context_length, base + ) + except Exception: + cc.threshold_percent = pct + else: + cc.threshold_percent = pct + except (TypeError, ValueError): + pass raw_ctx = model_cfg.get("context_length") if raw_ctx is not None: @@ -6198,6 +6308,14 @@ def _apply_live_compression_config(agent: Any, cfg: dict | None) -> None: cc.context_length = new_ctx except Exception: pass + elif getattr(cc, "_config_context_length", None) is not None: + # model.context_length removed: drop the config override and force + # re-inference from model metadata on next access — the same + # deferred get_model_context_length resolution agent construction + # uses (#32221). The re-resolve also re-applies the small-context + # threshold floor for the genuinely re-inferred window. + cc._config_context_length = None + cc._resolved_context_length = None coerce_cap = getattr(cc, "_coerce_threshold_tokens_cap", None) if callable(coerce_cap): @@ -6208,6 +6326,8 @@ def _apply_live_compression_config(agent: Any, cfg: dict | None) -> None: cc.threshold_tokens_cap = cap if cap > 0 else None except (TypeError, ValueError): cc.threshold_tokens_cap = None + else: + cc.threshold_tokens_cap = None # Invalidate cached trigger so the next preflight re-derives from the # current percent/window and then applies the (possibly new) cap.