fix(#4650): stop /api/reasoning request storm + memoize config YAML parse

Two-layer fix for the performance regression where streaming output dropped to
~2 tok/s because the UI issued a GET /api/reasoning per token.

Frontend (primary — restores pre-a9ce2889 behavior):
syncTopbar() calls syncReasoningChip() on every routine UI refresh, and during
streaming those fire at high frequency. Commit a9ce2889 ("hide reasoning chip
when model lacks effort levels", v0.51.141) changed syncReasoningChip() from
serving the cached _currentReasoningEffort to refetching unconditionally, so
ordinary syncs became a network storm. Restore the cache short-circuit while
keeping a9ce2889's intent: only fetch when nothing is cached yet OR the
model/provider identity changed since the last fetch (the only inputs that
change /api/reasoning's answer). The user-pick (POST) and model-switch paths
still update the cache directly, so a model switch refreshes supported-efforts
exactly once.

Backend (defense in depth):
_load_yaml_config_file() reparsed config.yaml on every call; yaml.safe_load on
an ~800-line / 24KB config costs ~125ms, so each storm request also paid a full
YAML parse. Memoize the raw parsed dict keyed on (path, st_mtime_ns, st_size)
and re-run _expand_env_vars() per call — env expansion is cheap, always returns
a fresh structure (so the ~7 read-modify-save callers never corrupt the cache),
and keeps ${VAR} live. Any on-disk edit (including by _save_yaml_config_file)
invalidates via the mtime/size key. Helps all callers of _load_yaml_config_file.

Tests:
- tests/test_issue4650_reasoning_chip_no_storm.py — node-driven behavioural test
  counting network calls: 1 fetch cold, 0 on 10 same-model syncs (the storm),
  1 on model switch, 0 after. Pins the storm cannot return.
- tests/test_issue4650_yaml_config_cache.py — cache hit skips reparse, mtime
  change invalidates, env expansion runs per call, caller mutation can't corrupt
  the cache, missing file returns {}.

api/config.py

@@ -467,21 +467,56 @@ def reload_config() -> None:
             _delete_models_cache_on_disk()
 
 
+# Memoized parse cache for _load_yaml_config_file, keyed on (resolved path,
+# st_mtime_ns, st_size). yaml.safe_load on an ~800-line / 24KB config.yaml costs
+# ~125ms of pure-Python parsing, and hot read paths (e.g. GET /api/reasoning ->
+# get_reasoning_status) call this on every request. Without a cache, a UI sync
+# storm turns into a YAML-reparse storm (#4650). We cache the RAW parsed dict and
+# re-run _expand_env_vars() on every call: env expansion is cheap, always returns
+# a fresh structure (so callers that read-modify-save the result never corrupt the
+# cache), and keeps ${VAR} references live against the current os.environ. The
+# (mtime_ns, size) key means any on-disk edit (including by _save_yaml_config_file)
+# is picked up on the next read.
+_yaml_file_cache: dict[str, tuple] = {}
+_yaml_file_cache_lock = threading.Lock()
+
+
 def _load_yaml_config_file(config_path: Path) -> dict:
     try:
         import yaml as _yaml
     except ImportError:
         return {}
 
-    if not config_path.exists():
+    try:
+        st = config_path.stat()
+    except OSError:
+        # Missing or unstattable file — preserve the original "no config" contract.
         return {}
+
+    cache_key = str(config_path)
+    stat_key = (st.st_mtime_ns, st.st_size)
+    with _yaml_file_cache_lock:
+        cached = _yaml_file_cache.get(cache_key)
+        if cached is not None and cached[0] == stat_key:
+            expanded = _expand_env_vars(cached[1])
+            return expanded if isinstance(expanded, dict) else {}
+
+    # Cache miss / stale: parse off disk. Done outside the lock so a slow parse
+    # doesn't serialize unrelated paths; a concurrent duplicate parse is harmless.
     try:
         loaded = _yaml.safe_load(config_path.read_text(encoding="utf-8"))
-        return _expand_env_vars(loaded) if isinstance(loaded, dict) else {}
     except Exception:
         logger.debug("Failed to parse yaml config from %s", config_path)
         return {}
 
+    raw = loaded if isinstance(loaded, dict) else {}
+    with _yaml_file_cache_lock:
+        _yaml_file_cache[cache_key] = (stat_key, raw)
+    if not raw:
+        return {}
+    expanded = _expand_env_vars(raw)
+    return expanded if isinstance(expanded, dict) else {}
+
 
 def get_config_for_profile_home(profile_home: "Path | str | None") -> dict:
     """Return the config dict for an explicit profile home directory.

static/ui.js

@@ -3069,13 +3069,33 @@ function _applyReasoningChip(eff){
   _highlightReasoningOption(effort);
 }
 
+// Tracks the model/provider identity of the last reasoning fetch so routine
+// topbar syncs can serve the cached chip state instead of re-hitting the
+// network. null = never fetched.
+let _lastReasoningFetchKey=null;
+
 function fetchReasoningChip(){
+  _lastReasoningFetchKey=_reasoningEffortQuery();
   api('/api/reasoning'+_reasoningEffortQuery()).then(function(st){
     _applyReasoningChip((st&&st.reasoning_effort)||'', st||{});
   }).catch(function(){_applyReasoningChip('', {supported_efforts:[]});});
 }
 
 function syncReasoningChip(){
+  // #4650: syncTopbar() calls this on every routine UI refresh, and during
+  // streaming those fire at high frequency. Before a9ce2889 this served the
+  // cached _currentReasoningEffort after the first load; that commit made it
+  // refetch unconditionally to refresh supported-efforts after a model switch,
+  // which turned ordinary syncs into a GET /api/reasoning storm (one per token).
+  // Restore the cache short-circuit but keep a9ce2889's intent: only hit the
+  // network when nothing is cached yet OR the model/provider identity changed
+  // since the last fetch (the only inputs that change /api/reasoning's answer).
+  // The user-pick and model-switch paths still update the cache directly.
+  const key=_reasoningEffortQuery();
+  if(_currentReasoningEffort!==null && _lastReasoningFetchKey===key){
+    _applyReasoningChip(_currentReasoningEffort);
+    return;
+  }
   fetchReasoningChip();
 }
 

tests/test_issue4650_reasoning_chip_no_storm.py

@@ -0,0 +1,165 @@
+"""Behavioural test for the #4650 reasoning-chip request-storm fix.
+
+`syncTopbar()` calls `syncReasoningChip()` on every routine UI refresh, and
+during streaming those fire at high frequency. Commit a9ce2889 made
+`syncReasoningChip()` refetch `GET /api/reasoning` unconditionally, so ordinary
+syncs became a network storm (one request per token -> ~2 tok/s).
+
+The fix restores the pre-a9ce2889 cache short-circuit while keeping that
+commit's intent (refresh supported-efforts after a model switch): fetch only
+when nothing is cached yet OR the model/provider identity changed since the
+last fetch. This test drives the ACTUAL functions from static/ui.js via node
+and counts network calls, so the storm cannot silently come back.
+"""
+import json
+import shutil
+import subprocess
+from pathlib import Path
+
+import pytest
+
+REPO_ROOT = Path(__file__).parent.parent.resolve()
+UI_JS_PATH = REPO_ROOT / "static" / "ui.js"
+
+NODE = shutil.which("node")
+pytestmark = pytest.mark.skipif(NODE is None, reason="node not on PATH")
+
+
+_DRIVER_SRC = r"""
+const fs = require('fs');
+const src = fs.readFileSync(process.argv[2], 'utf8');
+
+function makeEl() {
+  return {
+    style: {}, dataset: {}, title: '', textContent: '', value: '',
+    classList: { add(){}, remove(){}, toggle(){}, contains(){return false} },
+    querySelectorAll(){return []}, querySelector(){return null},
+    getBoundingClientRect(){return {left:0,top:0,width:0,height:0}},
+  };
+}
+
+const els = {
+  composerReasoningWrap: makeEl(),
+  composerReasoningLabel: makeEl(),
+  composerReasoningChip: makeEl(),
+  composerReasoningDropdown: makeEl(),
+  modelSelect: makeEl(),
+};
+
+// Mutable app state the reasoning helpers read from.
+global.S = { session: { model: 'gpt-5', model_provider: 'openai' } };
+global.window = {};
+global.document = { createElement: makeEl, addEventListener(){}, querySelectorAll(){return []}, querySelector(){return null} };
+global.$ = id => els[id] || null;
+
+// Count every network call and remember the URL it hit.
+let CALLS = [];
+global.api = (url) => { CALLS.push(url); return { then: () => ({ catch: () => {} }), catch: () => {} }; };
+
+// Helpers the reasoning code calls.
+global._modelStateForSelect = () => ({ model: '', model_provider: null });
+global._highlightReasoningOption = () => {};
+global._applyReasoningOptions = () => {};
+
+function extractFunc(name) {
+  const re = new RegExp('function\\s+' + name + '\\s*\\(');
+  const start = src.search(re);
+  if (start < 0) throw new Error(name + ' not found');
+  let i = src.indexOf('{', start); let depth = 1; i++;
+  while (depth > 0 && i < src.length) {
+    if (src[i] === '{') depth++; else if (src[i] === '}') depth--; i++;
+  }
+  return src.slice(start, i);
+}
+
+// `let _currentReasoningEffort` / `_currentReasoningEffortsSupported` /
+// `_lastReasoningFetchKey` are module-scope state the functions close over;
+// declare them in this eval scope so the extracted functions can see them.
+var _currentReasoningEffort = null;
+var _currentReasoningEffortsSupported = null;
+var _lastReasoningFetchKey = null;
+
+eval(extractFunc('_normalizeReasoningEffort'));
+eval(extractFunc('_formatReasoningEffortLabel'));
+eval(extractFunc('_reasoningEffortContext'));
+eval(extractFunc('_reasoningEffortQuery'));
+eval(extractFunc('_applyReasoningChip'));
+eval(extractFunc('fetchReasoningChip'));
+eval(extractFunc('syncReasoningChip'));
+
+// ── Scenario ───────────────────────────────────────────────────────────────
+const result = {};
+
+// 1. First sync with nothing cached -> must fetch.
+syncReasoningChip();
+result.after_first_sync = CALLS.length;
+
+// Simulate the fetch resolving (what _applyReasoningChip does on response).
+_applyReasoningChip('high', { supported_efforts: ['low','high'] });
+
+// 2. Ten routine syncs with the SAME model (the streaming storm) -> no new fetch.
+for (let i = 0; i < 10; i++) syncReasoningChip();
+result.after_ten_same_model_syncs = CALLS.length;
+
+// 3. Model switch -> exactly one more fetch.
+global.S.session.model = 'claude-opus-4';
+global.S.session.model_provider = 'anthropic';
+syncReasoningChip();
+result.after_model_switch = CALLS.length;
+
+// 4. More routine syncs on the new model -> still no new fetch.
+_applyReasoningChip('low', { supported_efforts: ['low','high'] });
+for (let i = 0; i < 5; i++) syncReasoningChip();
+result.after_new_model_syncs = CALLS.length;
+
+result.calls = CALLS;
+process.stdout.write(JSON.stringify(result));
+"""
+
+
+@pytest.fixture(scope="module")
+def driver_path(tmp_path_factory):
+    p = tmp_path_factory.mktemp("reasoning_storm_driver") / "driver.js"
+    p.write_text(_DRIVER_SRC, encoding="utf-8")
+    return str(p)
+
+
+@pytest.fixture(scope="module")
+def outcome(driver_path):
+    result = subprocess.run(
+        [NODE, driver_path, str(UI_JS_PATH)],
+        capture_output=True, text=True, timeout=30,
+    )
+    if result.returncode != 0:
+        raise RuntimeError(f"node driver failed: {result.stderr}")
+    return json.loads(result.stdout)
+
+
+def test_first_sync_fetches_once(outcome):
+    assert outcome["after_first_sync"] == 1, (
+        f"first sync (cold cache) must fetch exactly once: {outcome['calls']}"
+    )
+
+
+def test_repeated_syncs_same_model_do_not_refetch(outcome):
+    """The core #4650 regression: 10 routine syncs on the same model must NOT
+    issue 10 network calls — that was the per-token request storm."""
+    assert outcome["after_ten_same_model_syncs"] == 1, (
+        "routine topbar syncs on an unchanged model must serve the cache, not "
+        f"refetch — request storm regression: {outcome['calls']}"
+    )
+
+
+def test_model_switch_triggers_one_refetch(outcome):
+    """a9ce2889's intent must survive: switching models refreshes the chip's
+    supported-efforts, so exactly one fetch fires on the switch."""
+    assert outcome["after_model_switch"] == 2, (
+        f"a model switch must trigger exactly one refetch: {outcome['calls']}"
+    )
+
+
+def test_syncs_after_switch_do_not_refetch(outcome):
+    assert outcome["after_new_model_syncs"] == 2, (
+        "routine syncs after a model switch must serve the cache again: "
+        f"{outcome['calls']}"
+    )

tests/test_issue4650_yaml_config_cache.py

@@ -0,0 +1,111 @@
+"""Backend half of the #4650 fix: _load_yaml_config_file memoizes the parsed
+config keyed on (path, mtime_ns, size).
+
+GET /api/reasoning -> get_reasoning_status() calls _load_yaml_config_file on
+every request, and yaml.safe_load on an ~800-line config costs ~125ms. Under
+the (pre-fix) request storm that became a YAML-reparse storm. This test pins
+the cache contract:
+
+  * a second read of an unchanged file does NOT re-run yaml.safe_load,
+  * an on-disk edit (new mtime/size) DOES trigger a fresh parse,
+  * env-var expansion still runs per call (so ${VAR} stays live and callers
+    that mutate the returned dict never corrupt the cached copy).
+"""
+import importlib
+import time
+
+import pytest
+
+config = importlib.import_module("api.config")
+
+
+@pytest.fixture
+def clean_cache():
+    config._yaml_file_cache.clear()
+    yield
+    config._yaml_file_cache.clear()
+
+
+def _write(p, text):
+    p.write_text(text, encoding="utf-8")
+
+
+def test_second_read_uses_cache_no_reparse(tmp_path, monkeypatch, clean_cache):
+    cfg = tmp_path / "config.yaml"
+    _write(cfg, "agent:\n  reasoning_effort: high\n")
+
+    calls = {"n": 0}
+    import yaml as _yaml
+    real_load = _yaml.safe_load
+
+    def counting_load(text):
+        calls["n"] += 1
+        return real_load(text)
+
+    monkeypatch.setattr(_yaml, "safe_load", counting_load)
+
+    first = config._load_yaml_config_file(cfg)
+    second = config._load_yaml_config_file(cfg)
+
+    assert first == {"agent": {"reasoning_effort": "high"}}
+    assert second == first
+    assert calls["n"] == 1, "second read of an unchanged file must hit the cache, not reparse"
+
+
+def test_mtime_change_invalidates_cache(tmp_path, monkeypatch, clean_cache):
+    cfg = tmp_path / "config.yaml"
+    _write(cfg, "agent:\n  reasoning_effort: high\n")
+
+    calls = {"n": 0}
+    import yaml as _yaml
+    real_load = _yaml.safe_load
+
+    def counting_load(text):
+        calls["n"] += 1
+        return real_load(text)
+
+    monkeypatch.setattr(_yaml, "safe_load", counting_load)
+
+    assert config._load_yaml_config_file(cfg)["agent"]["reasoning_effort"] == "high"
+    # Rewrite with different content + bump mtime so (mtime_ns, size) changes.
+    time.sleep(0.01)
+    _write(cfg, "agent:\n  reasoning_effort: low\n")
+    import os
+    st = cfg.stat()
+    os.utime(cfg, ns=(st.st_atime_ns, st.st_mtime_ns + 1_000_000))
+
+    reloaded = config._load_yaml_config_file(cfg)
+    assert reloaded["agent"]["reasoning_effort"] == "low", "an on-disk edit must be picked up"
+    assert calls["n"] == 2, "changed file must trigger exactly one fresh parse"
+
+
+def test_env_expansion_runs_per_call_even_from_cache(tmp_path, monkeypatch, clean_cache):
+    cfg = tmp_path / "config.yaml"
+    _write(cfg, "agent:\n  api_key: ${MY_TEST_KEY}\n")
+
+    monkeypatch.setenv("MY_TEST_KEY", "first")
+    first = config._load_yaml_config_file(cfg)
+    assert first["agent"]["api_key"] == "first"
+
+    # Cache hit (file unchanged) but env changed -> expansion must reflect it.
+    monkeypatch.setenv("MY_TEST_KEY", "second")
+    second = config._load_yaml_config_file(cfg)
+    assert second["agent"]["api_key"] == "second", "env expansion must run per call, not be frozen in the cache"
+
+
+def test_caller_mutation_does_not_corrupt_cache(tmp_path, clean_cache):
+    cfg = tmp_path / "config.yaml"
+    _write(cfg, "display:\n  show_reasoning: true\n")
+
+    first = config._load_yaml_config_file(cfg)
+    # Mirror the read-modify-save callers (set_reasoning_display etc.).
+    first["display"]["show_reasoning"] = False
+    first["injected"] = "mutation"
+
+    second = config._load_yaml_config_file(cfg)
+    assert second["display"]["show_reasoning"] is True, "caller mutation must not leak into the cache"
+    assert "injected" not in second
+
+
+def test_missing_file_returns_empty(tmp_path, clean_cache):
+    assert config._load_yaml_config_file(tmp_path / "nope.yaml") == {}