stage #4652 (nesquena-hermes): stop /api/reasoning storm + memoize YAML config parse (fixes #4650) (#4659)

Two-layer perf fix for the /api/reasoning storm dropping streaming to ~2 tok/s:
- ui.js: syncReasoningChip() serves the cached chip on routine topbar syncs, only
  re-fetching when the model/provider key (_lastReasoningFetchKey) changes — restores
  pre-a9ce2889 behavior while keeping its model-switch-refresh intent.
- config.py: memoize _load_yaml_config_file keyed on (path, st_mtime_ns, st_size) under
  a lock; cache the RAW parsed dict and re-run cheap _expand_env_vars per call (cache stays
  correct vs os.environ and read-modify-save callers get a fresh structure; on-disk edits
  picked up via mtime/size). Code byte-faithful from PR head f868f10a1a. 9 own tests pass.

Co-authored-by: nesquena-hermes <agent@nesquena-hermes>

CHANGELOG.md

@@ -3,6 +3,12 @@
 
 ## [Unreleased]
 
+## [v0.51.573] — 2026-06-22 — Release UF (reasoning request-storm fix)
+
+### Fixed
+
+- **Fixed a streaming slowdown caused by a `/api/reasoning` request storm.** A recent change made the reasoning-effort chip re-fetch `/api/reasoning` on every routine topbar sync; during streaming those fire at high frequency, so output could drop to ~2 tokens/sec under the request flood. The chip now serves its cached state on routine syncs and only re-fetches when the model/provider identity actually changes (the only input that changes the endpoint's answer), restoring fast streaming. As defense-in-depth, the server also memoizes the `config.yaml` parse (keyed on file mtime+size; ~125 ms per parse on a large config) so a read storm can't turn into a YAML-reparse storm. Fixes #4650.
+
 ## [v0.51.572] — 2026-06-22 — Release UE (in-chat cronjob profile pinning)
 
 ### Fixed

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.
@@ -564,6 +599,12 @@ def _save_yaml_config_file(config_path: Path, config_data: dict) -> None:
         _yaml.safe_dump(_config_for_yaml_save(config_data), sort_keys=False, allow_unicode=True),
         encoding="utf-8",
     )
+    # Invalidate the memoized parse for this path so the next read re-parses the
+    # bytes we just wrote. mtime_ns+size keying normally catches edits, but a
+    # WebUI save that preserves size with a coarse/unchanged mtime could otherwise
+    # serve a stale dict (#4650 review) — evicting on our own write closes that gap.
+    with _yaml_file_cache_lock:
+        _yaml_file_cache.pop(str(config_path), None)
 
 
 # Initial load

static/messages.js

@@ -4300,6 +4300,13 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){
           if(typeof _messageRenderableMessageCount==='function'&&typeof _messageRenderWindowSize!=='undefined'){
             _messageRenderWindowSize=Math.max(typeof _currentMessageRenderWindowSize==='function'?_currentMessageRenderWindowSize():50, _messageRenderableMessageCount());
           }
+          // #4650 review: the agent turn that just completed may have changed
+          // server-side reasoning config (e.g. a `/reasoning <level>` slash
+          // command writes agent.reasoning_effort) WITHOUT changing the model/
+          // provider cache key. Invalidate the reasoning-chip cache once at the
+          // turn boundary so the following syncTopbar() refetches the authoritative
+          // effort exactly once (not per-token — the storm short-circuit is intact).
+          if(typeof _lastReasoningFetchKey!=='undefined') _lastReasoningFetchKey=null;
           syncTopbar();renderMessages({preserveScroll:true});
           if(shouldFollowOnDone&&typeof scrollToBottom==='function') scrollToBottom();
           if(typeof noteWorkspaceMutationsFromToolCalls==='function') noteWorkspaceMutationsFromToolCalls(S.toolCalls);

static/panels.js

@@ -5899,6 +5899,12 @@ async function switchToProfile(name) {
     if (_switchGen !== _profileSwitchGeneration) return;
     S.activeProfile = data.active || name;
     S.activeProfileIsDefault = !!data.is_default;
+    // #4650 review: a profile switch can change agent.reasoning_effort (and other
+    // reasoning inputs like base_url) WITHOUT changing the default model/provider,
+    // which is all the reasoning-chip cache key tracks. Force exactly one reasoning
+    // refetch for the new profile so the chip reflects the new profile's effort
+    // (the syncTopbar() calls below route through syncReasoningChip()).
+    if (typeof _lastReasoningFetchKey !== 'undefined') _lastReasoningFetchKey = null;
 
     // Reconnect the gateway SSE to the NEW profile's watcher. The backend watcher
     // registry is now profile-keyed (#3629), but this tab's existing EventSource is
@@ -9226,6 +9232,14 @@ function _openAuxAdvancedOptions(taskKey,cfg){
     if(typeof showToast==='function') showToast(isMain?(t('settings_main_advanced_saved')||'Main model options saved'):(t('settings_aux_advanced_saved')||'Auxiliary options saved'));
     overlay.style.display='none';
     _loadAuxiliaryModels();
+    // #4650 review: a main-model advanced save can change base_url, which
+    // /api/reasoning's answer depends on for some providers (e.g. LM Studio),
+    // WITHOUT changing the model/provider cache key. Invalidate the reasoning
+    // cache and refresh so the chip reflects the new config (one refetch).
+    if(isMain){
+      if(typeof _lastReasoningFetchKey!=='undefined') _lastReasoningFetchKey=null;
+      if(typeof fetchReasoningChip==='function') fetchReasoningChip();
+    }
    }catch(e){
     if(typeof showToast==='function') showToast(isMain?(t('settings_main_advanced_save_failed')||'Failed to save main model options'):(t('settings_aux_advanced_save_failed')||'Failed to save auxiliary options'));
    }

static/ui.js

@@ -3069,13 +3069,61 @@ 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;
+// Monotonic dispatch counter. Each fetchReasoningChip() increments it and the
+// async handlers capture their own value; a response (success OR failure) only
+// applies if it is still the most recent dispatch. This defeats out-of-order
+// resolution even when two fetches share the same model/provider key (e.g. a
+// profile switch that resets the cache and refetches the same default model but
+// a different agent.reasoning_effort) — #4650 review.
+let _reasoningFetchSeq=0;
+
 function fetchReasoningChip(){
-  api('/api/reasoning'+_reasoningEffortQuery()).then(function(st){
+  // Set the cache key OPTIMISTICALLY before the request so rapid routine syncs
+  // while this GET is in flight short-circuit instead of re-dispatching (that
+  // in-flight window is exactly where the #4650 storm lived).
+  const key=_reasoningEffortQuery();
+  const seq=++_reasoningFetchSeq;
+  _lastReasoningFetchKey=key;
+  api('/api/reasoning'+key).then(function(st){
+    // Ignore a stale/superseded response: only the most recent dispatch may
+    // apply, so an older in-flight GET can't poison the current chip (#4650).
+    if(seq!==_reasoningFetchSeq) return;
     _applyReasoningChip((st&&st.reasoning_effort)||'', st||{});
-  }).catch(function(){_applyReasoningChip('', {supported_efforts:[]});});
+  }).catch(function(){
+    // Same staleness guard on failure: a stale error must neither hide the chip
+    // nor clear a newer fetch's key. Only the latest dispatch clears the key so
+    // routine syncs retry after a genuine transient failure.
+    if(seq!==_reasoningFetchSeq) return;
+    _lastReasoningFetchKey=null;
+    _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();
+  // Short-circuit on the KEY alone: if a fetch for this exact model/provider has
+  // already been dispatched (in-flight) or completed, do not dispatch another —
+  // this is what stops the #4650 storm, including the COLD-cache window where
+  // _currentReasoningEffort is still null between the first dispatch and its
+  // response (10 syncs before the first GET resolves must produce ONE request,
+  // not ten). Apply the cached chip only once we actually have an effort value.
+  if(_lastReasoningFetchKey===key){
+    if(_currentReasoningEffort!==null) _applyReasoningChip(_currentReasoningEffort);
+    return;
+  }
   fetchReasoningChip();
 }
 

tests/test_issue1103_reasoning_chip_visibility.py

@@ -56,7 +56,22 @@ def test_ui_js_passes_model_context_to_reasoning_api():
     assert "_reasoningEffortQuery" in src, (
         "ui.js must pass the active session model/provider to /api/reasoning"
     )
-    assert "api('/api/reasoning'+_reasoningEffortQuery())" in src
+    # The /api/reasoning GET must carry the model/provider query. #4650 captures
+    # the query into a local `key` first (for the in-flight storm + stale-success
+    # guards), so accept either the inlined form or the captured-key form — both
+    # pass _reasoningEffortQuery()'s output to the endpoint.
+    fetch_match = re.search(r"function fetchReasoningChip\(\)\{(.+?)\n\}", src, re.DOTALL)
+    assert fetch_match, "fetchReasoningChip function must exist"
+    fetch_body = fetch_match.group(1)
+    inlined = "api('/api/reasoning'+_reasoningEffortQuery())" in src
+    captured = (
+        re.search(r"const\s+key\s*=\s*_reasoningEffortQuery\(\)", fetch_body)
+        and "api('/api/reasoning'+key)" in fetch_body
+    )
+    assert inlined or captured, (
+        "fetchReasoningChip must pass _reasoningEffortQuery() (model/provider context) "
+        "to GET /api/reasoning, either inlined or via a captured key"
+    )
 
 
 def test_fetchReasoningChip_calls_apply():

tests/test_issue4650_reasoning_chip_no_storm.py

@@ -0,0 +1,227 @@
+"""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;
+var _reasoningFetchSeq = 0;
+
+eval(extractFunc('_normalizeReasoningEffort'));
+eval(extractFunc('_formatReasoningEffortLabel'));
+eval(extractFunc('_reasoningEffortContext'));
+eval(extractFunc('_reasoningEffortQuery'));
+eval(extractFunc('_applyReasoningChip'));
+eval(extractFunc('fetchReasoningChip'));
+eval(extractFunc('syncReasoningChip'));
+
+// ── Scenario ───────────────────────────────────────────────────────────────
+const result = {};
+
+// 0. COLD-CACHE in-flight burst: 10 syncs BEFORE the first GET resolves (no
+//    _applyReasoningChip yet, so _currentReasoningEffort is still null). This is
+//    the streaming storm's worst case — it must produce exactly ONE request,
+//    relying on the optimistic key short-circuit, not on a resolved chip.
+for (let i = 0; i < 10; i++) syncReasoningChip();
+result.after_cold_inflight_burst = CALLS.length;
+
+// 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;
+
+// 5. OUT-OF-ORDER staleness guard: capture each fetch's success callback and
+//    fire an OLDER fetch's response AFTER a newer dispatch. The stale response
+//    must be ignored (generation guard), even though both share the same key
+//    after a profile-switch-style cache reset. Swap in a capturing api() mock.
+const HANDLERS = [];
+global.api = (url) => ({
+  then: (onOk) => { HANDLERS.push({ url, onOk }); return { catch: (onErr) => { HANDLERS[HANDLERS.length-1].onErr = onErr; } }; },
+  catch: () => {},
+});
+_currentReasoningEffort = null; _lastReasoningFetchKey = null;
+fetchReasoningChip();                 // dispatch #A (seq N)
+fetchReasoningChip();                 // dispatch #B (seq N+1) — supersedes A
+// Resolve the OLDER one (#A) last with one effort; must be ignored.
+HANDLERS[0].onOk({ reasoning_effort: 'low', supported_efforts: ['low','high'] });
+result.effort_after_stale_A = _currentReasoningEffort;   // expect null (ignored)
+// Now resolve the NEWER one (#B) with a different effort; must apply.
+HANDLERS[1].onOk({ reasoning_effort: 'high', supported_efforts: ['low','high'] });
+result.effort_after_fresh_B = _currentReasoningEffort;   // expect 'high'
+// A late FAILURE from the older dispatch must NOT hide the fresh chip.
+if (HANDLERS[0].onErr) HANDLERS[0].onErr();
+result.effort_after_stale_A_fail = _currentReasoningEffort; // expect still 'high'
+
+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_cold_inflight_burst_fetches_once(outcome):
+    """The worst-case streaming storm: 10 syncs BEFORE the first GET resolves
+    (cold cache, _currentReasoningEffort still null) must produce exactly ONE
+    request. The optimistic key short-circuit — not a resolved chip — closes
+    this in-flight window (#4650 review finding 2)."""
+    assert outcome["after_cold_inflight_burst"] == 1, (
+        "a cold-cache burst of syncs before the first response settles must fire "
+        f"exactly one GET, not one-per-sync: {outcome['calls']}"
+    )
+
+
+def test_first_sync_fetches_once(outcome):
+    assert outcome["after_first_sync"] == 1, (
+        f"a further sync on the same in-flight key must not add a request: {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']}"
+    )
+
+
+def test_stale_out_of_order_success_is_ignored(outcome):
+    """#4650 review: an older in-flight /api/reasoning success that resolves AFTER
+    a newer dispatch (same key, e.g. profile switch) must not poison the chip."""
+    assert outcome["effort_after_stale_A"] is None, (
+        "a superseded older fetch's success must be ignored (generation guard), "
+        f"got {outcome['effort_after_stale_A']!r}"
+    )
+    assert outcome["effort_after_fresh_B"] == "high", (
+        "the newest fetch's success must apply"
+    )
+
+
+def test_stale_out_of_order_failure_does_not_hide_fresh_chip(outcome):
+    """A late failure from a superseded dispatch must not clear/hide the chip
+    that a newer successful fetch already applied."""
+    assert outcome["effort_after_stale_A_fail"] == "high", (
+        "a stale failure must not overwrite the fresh chip state: "
+        f"got {outcome['effort_after_stale_A_fail']!r}"
+    )

tests/test_issue4650_yaml_config_cache.py

@@ -0,0 +1,135 @@
+"""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") == {}
+
+
+def test_save_evicts_cache_so_next_read_is_fresh(tmp_path, clean_cache):
+    """#4650 review: a WebUI save that preserves (mtime_ns, size) could otherwise
+    serve a stale cached parse. _save_yaml_config_file must evict the cache entry
+    for the path it writes, so the next read reflects the saved content."""
+    cfg = tmp_path / "config.yaml"
+    _write(cfg, "agent:\n  reasoning_effort: high\n")
+
+    first = config._load_yaml_config_file(cfg)
+    assert first["agent"]["reasoning_effort"] == "high"
+    assert str(cfg) in config._yaml_file_cache, "first read should populate the cache"
+
+    # Save new content through the WebUI writer.
+    config._save_yaml_config_file(cfg, {"agent": {"reasoning_effort": "low"}})
+    assert str(cfg) not in config._yaml_file_cache, (
+        "_save_yaml_config_file must evict the memoized parse for the written path"
+    )
+
+    # Next read must reflect the saved value even if mtime/size happened to collide.
+    fresh = config._load_yaml_config_file(cfg)
+    assert fresh["agent"]["reasoning_effort"] == "low", (
+        "read after save must return the freshly-written config, not a stale cache hit"
+    )