Merge #5412: speed up profile dropdown opening [ruizanthony]

static/panels.js

@@ -5921,8 +5921,97 @@ async function switchToWorkspace(path,name){
 
 // ── Profile panel + dropdown ──
 let _profilesCache = null;
+let _profileDropdownFetchPromise = null;
+let _profileDropdownCacheLoadedFromStorage = false;
+const PROFILE_DROPDOWN_CACHE_KEY = 'hermes-webui-profile-dropdown-cache-v1';
+const PROFILE_DROPDOWN_CACHE_TTL_MS = 5 * 60 * 1000;
 let _profileSwitchGeneration = 0;
 let _profileDropdownTrigger = null;  // tracks which element triggered the dropdown
+let _profileDropdownOpenGeneration = 0;
+
+function _profileDropdownClearStoredCache(){
+  try{localStorage.removeItem(PROFILE_DROPDOWN_CACHE_KEY);}catch(_){}
+}
+
+function _profileDropdownDataCacheUsable(data){
+  return !!(
+    data &&
+    Array.isArray(data.profiles) &&
+    data.profiles.length &&
+    data.profiles.every(p=>p&&typeof p.name==='string')
+  );
+}
+
+function _profileDropdownCacheUsable(data){
+  return !!(_profileDropdownDataCacheUsable(data) && data.single_profile_mode !== true);
+}
+
+function _profileDropdownReadStoredCache(){
+  if(_profileDropdownCacheLoadedFromStorage) return _profileDropdownCacheUsable(_profilesCache) ? _profilesCache : null;
+  _profileDropdownCacheLoadedFromStorage = true;
+  try{
+    const raw=localStorage.getItem(PROFILE_DROPDOWN_CACHE_KEY);
+    if(!raw) return null;
+    const parsed=JSON.parse(raw);
+    if(!parsed || typeof parsed.ts!=='number' || !parsed.data) { _profileDropdownClearStoredCache(); return null; }
+    if(Date.now()-parsed.ts>PROFILE_DROPDOWN_CACHE_TTL_MS) { _profileDropdownClearStoredCache(); return null; }
+    if(!_profileDropdownCacheUsable(parsed.data)) { _profileDropdownClearStoredCache(); return null; }
+    _profilesCache = parsed.data;
+    return _profilesCache;
+  }catch(_){_profileDropdownClearStoredCache();return null;}
+}
+
+function _profileDropdownWriteStoredCache(data){
+  if(!_profileDropdownCacheUsable(data)) { _profileDropdownClearStoredCache(); return; }
+  try{localStorage.setItem(PROFILE_DROPDOWN_CACHE_KEY, JSON.stringify({ts:Date.now(), data}));}catch(_){}
+}
+
+function _profileDropdownBestCachedData(){
+  if(_profileDropdownCacheUsable(_profilesCache)) return _profilesCache;
+  if(_profileDropdownDataCacheUsable(_profilesCache)) return null;
+  _profilesCache = null;
+  return _profileDropdownReadStoredCache();
+}
+
+function _profileDropdownFetchFresh(){
+  if(_profileDropdownFetchPromise) return _profileDropdownFetchPromise;
+  _profileDropdownFetchPromise = api('/api/profiles', {timeoutToast:false}).then(data=>{
+    if(_profileDropdownDataCacheUsable(data)) _profilesCache = data;
+    _profileDropdownWriteStoredCache(data);
+    return data;
+  }).finally(()=>{ _profileDropdownFetchPromise = null; });
+  return _profileDropdownFetchPromise;
+}
+
+function _warmProfileDropdownCache(){
+  _profileDropdownBestCachedData();
+  _profileDropdownFetchFresh().catch(()=>{});
+}
+
+if(typeof window!=='undefined'){
+  window.addEventListener('load',()=>{
+    setTimeout(()=>{
+      if(typeof document==='undefined'||!document.hidden) _warmProfileDropdownCache();
+    },1200);
+  },{once:true});
+}
+
+function _renderProfileDropdownLoading(){
+  const dd=$('profileDropdown');
+  if(!dd)return;
+  dd.innerHTML=`<div class="profile-opt profile-opt-loading"><div class="profile-opt-name">${esc(t('loading')||'Loading...')}</div></div>`;
+}
+
+function _openProfileDropdownShell(){
+  const dd=$('profileDropdown');
+  if(!dd)return;
+  dd.classList.add('open');
+  _positionProfileDropdown();
+  const chip=$('profileChip');
+  if(chip && _profileDropdownTrigger===chip) chip.classList.add('active');
+  const tbtn=$('titlebarProfileBtn');
+  if(tbtn && _profileDropdownTrigger===tbtn) tbtn.classList.add('active');
+}
 
 async function _profileSwitchPanelLoad(){
   // Cross-profile cron visibility is an active-profile opt-in; never carry it
@@ -5983,6 +6072,7 @@ async function loadProfilesPanel() {
   try {
     const data = await api('/api/profiles');
     _profilesCache = data;
+    _profileDropdownWriteStoredCache(data);
     panel.innerHTML = '';
 
     // Hide "New profile" button in single profile mode
@@ -6188,10 +6278,11 @@ async function deleteCurrentProfile(){
 }
 
 function renderProfileDropdown(data) {
+  data = data || {};
   const dd = $('profileDropdown');
   if (!dd) return;
   dd.innerHTML = '';
-  const allProfiles = data.profiles || [];
+  const allProfiles = (Array.isArray(data.profiles) ? data.profiles : []).filter(p => p && typeof p.name === 'string');
   const active = (S.activeProfile && allProfiles.some(p => p.name === S.activeProfile))
     ? S.activeProfile
     : (data.active || 'default');
@@ -6235,24 +6326,39 @@ function toggleProfileDropdown(e) {
   if(typeof closeModelDropdown==='function') closeModelDropdown();
   // Track which element triggered the dropdown for positioning
   _profileDropdownTrigger = (e && e.currentTarget) || $('profileChip');
-  api('/api/profiles').then(data => {
+  const openGen = ++_profileDropdownOpenGeneration;
+  const cached = _profileDropdownBestCachedData();
+
+  if(cached && !cached.single_profile_mode){
+    renderProfileDropdown(cached);
+    _openProfileDropdownShell();
+  }else{
+    _renderProfileDropdownLoading();
+    _openProfileDropdownShell();
+  }
+
+  _profileDropdownFetchFresh().then(data => {
+    if(openGen !== _profileDropdownOpenGeneration) return;
     // In single profile mode, don't show profile dropdown at all
     if (data.single_profile_mode) {
       closeProfileDropdown();
       return;
     }
     renderProfileDropdown(data);
-    dd.classList.add('open');
-    _positionProfileDropdown();
-    // Mark the triggering button as active
-    const chip=$('profileChip');
-    if(chip && _profileDropdownTrigger===chip) chip.classList.add('active');
-    const tbtn=$('titlebarProfileBtn');
-    if(tbtn && _profileDropdownTrigger===tbtn) tbtn.classList.add('active');
-  }).catch(e => { showToast(t('profiles_load_failed')); });
+    _openProfileDropdownShell();
+  }).catch(e => {
+    if(openGen !== _profileDropdownOpenGeneration) return;
+    if(cached && !cached.single_profile_mode){
+      // Keep the cached menu open; the next click/background refresh will retry.
+      return;
+    }
+    closeProfileDropdown();
+    showToast(t('profiles_load_failed'));
+  });
 }
 
 function closeProfileDropdown() {
+  _profileDropdownOpenGeneration++;
   const dd = $('profileDropdown');
   if (dd) dd.classList.remove('open');
   const chip=$('profileChip');

tests/test_issue3623_profile_visibility.py

@@ -107,7 +107,8 @@ def _function_body(src: str, signature: str) -> str:
 def test_profile_dropdown_filters_hidden_profiles_but_preserves_active():
     body = _function_body(_panels_js(), "function renderProfileDropdown(data)")
 
-    assert "const allProfiles = data.profiles || [];" in body
+    assert "data = data || {};" in body
+    assert "const allProfiles = (Array.isArray(data.profiles) ? data.profiles : []).filter(p => p && typeof p.name === 'string');" in body
     assert "allProfiles.some(p => p.name === S.activeProfile)" in body
     assert "const profiles = allProfiles.filter(p => p && (p.visible !== false || p.name === active));" in body
 

tests/test_profile_dropdown_fast_open.py

@@ -0,0 +1,267 @@
+"""Regression guards for fast profile dropdown opening.
+
+The user-visible failure was: clicking the composer/titlebar profile chip waited
+for a cold /api/profiles request before showing the menu. On machines where the
+profile metadata scan is slow, that made the click feel frozen for seconds.
+"""
+import json
+import subprocess
+import textwrap
+from pathlib import Path
+
+REPO_ROOT = Path(__file__).parent.parent.resolve()
+PANELS_JS = (REPO_ROOT / "static" / "panels.js").read_text(encoding="utf-8")
+
+
+def _function_body(src: str, marker: str, next_marker: str | None = None) -> str:
+    start = src.index(marker)
+    if next_marker is not None:
+        end = src.index(next_marker, start)
+        return src[start:end]
+    depth = 0
+    opened = False
+    for idx, ch in enumerate(src[start:], start):
+        if ch == "{":
+            depth += 1
+            opened = True
+        elif ch == "}":
+            depth -= 1
+            if opened and depth == 0:
+                return src[start : idx + 1]
+    raise AssertionError(f"Could not extract function body for {marker}")
+
+
+def test_profile_dropdown_opens_shell_before_network_fetch():
+    body = _function_body(PANELS_JS, "function toggleProfileDropdown(e) {")
+    cache_idx = body.index("const cached = _profileDropdownBestCachedData();")
+    open_idx = body.index("_openProfileDropdownShell();")
+    fetch_idx = body.index("_profileDropdownFetchFresh().then")
+    assert cache_idx < open_idx < fetch_idx, (
+        "toggleProfileDropdown must render/open from cache or loading shell before "
+        "awaiting the slow /api/profiles refresh"
+    )
+
+
+def test_profile_dropdown_uses_shared_fetch_promise_and_validated_local_storage_cache():
+    data_cache_body = _function_body(PANELS_JS, "function _profileDropdownDataCacheUsable(data){")
+    assert "Array.isArray(data.profiles)" in data_cache_body
+    assert "data.profiles.every(p=>p&&typeof p.name==='string')" in data_cache_body
+
+    cache_body = _function_body(PANELS_JS, "function _profileDropdownCacheUsable(data){")
+    assert "_profileDropdownDataCacheUsable(data)" in cache_body
+    assert "data.single_profile_mode !== true" in cache_body
+
+    fetch_body = _function_body(PANELS_JS, "function _profileDropdownFetchFresh(){")
+    assert "if(_profileDropdownFetchPromise) return _profileDropdownFetchPromise;" in fetch_body
+    assert "api('/api/profiles', {timeoutToast:false})" in fetch_body
+    assert "if(_profileDropdownDataCacheUsable(data)) _profilesCache = data;" in fetch_body
+    assert "_profileDropdownWriteStoredCache(data);" in fetch_body
+    assert "\n    _profilesCache = data;\n" not in fetch_body
+
+    best_body = _function_body(PANELS_JS, "function _profileDropdownBestCachedData(){")
+    assert "if(_profileDropdownDataCacheUsable(_profilesCache)) return null;" in best_body
+    assert best_body.index("if(_profileDropdownDataCacheUsable(_profilesCache)) return null;") < best_body.index("_profilesCache = null;")
+
+    read_body = _function_body(PANELS_JS, "function _profileDropdownReadStoredCache(){")
+    assert "localStorage.getItem(PROFILE_DROPDOWN_CACHE_KEY)" in read_body
+    assert "PROFILE_DROPDOWN_CACHE_TTL_MS" in read_body
+    assert "_profileDropdownClearStoredCache();" in read_body
+
+    write_body = _function_body(PANELS_JS, "function _profileDropdownWriteStoredCache(data){")
+    assert "if(!_profileDropdownCacheUsable(data)) { _profileDropdownClearStoredCache(); return; }" in write_body
+
+
+def test_profile_dropdown_closing_invalidates_inflight_refresh():
+    close_body = _function_body(PANELS_JS, "function closeProfileDropdown() {")
+    assert "_profileDropdownOpenGeneration++;" in close_body
+
+    toggle_body = _function_body(PANELS_JS, "function toggleProfileDropdown(e) {")
+    assert "const openGen = ++_profileDropdownOpenGeneration;" in toggle_body
+    assert "if(openGen !== _profileDropdownOpenGeneration) return;" in toggle_body
+
+
+def test_profiles_panel_refresh_updates_dropdown_cache():
+    body = _function_body(PANELS_JS, "async function loadProfilesPanel() {")
+    api_idx = body.index("const data = await api('/api/profiles');")
+    cache_idx = body.index("_profileDropdownWriteStoredCache(data);")
+    render_idx = body.index("panel.innerHTML = '';")
+    assert api_idx < cache_idx < render_idx
+
+
+def test_profile_dropdown_prefetches_after_page_load():
+    assert "function _warmProfileDropdownCache(){" in PANELS_JS
+    assert "window.addEventListener('load'" in PANELS_JS
+    assert "_warmProfileDropdownCache();" in PANELS_JS
+
+
+def test_poisoned_profile_cache_opens_then_switches_after_fresh_refresh():
+    snippets = [
+        PANELS_JS[
+            PANELS_JS.index("let _profilesCache = null;") : PANELS_JS.index("async function _profileSwitchPanelLoad(){")
+        ],
+        _function_body(PANELS_JS, "function renderProfileDropdown(data) {"),
+        _function_body(PANELS_JS, "function toggleProfileDropdown(e) {"),
+        _function_body(PANELS_JS, "function closeProfileDropdown() {"),
+    ]
+    script = textwrap.dedent(
+        f"""
+        const assert = require('assert');
+        const snippets = {json.dumps(snippets)};
+
+        class ClassList {{
+          constructor() {{ this.values = new Set(); }}
+          add(name) {{ this.values.add(name); }}
+          remove(name) {{ this.values.delete(name); }}
+          contains(name) {{ return this.values.has(name); }}
+        }}
+        class Element {{
+          constructor(tag, id) {{
+            this.tagName = tag;
+            this.id = id || '';
+            this.children = [];
+            this.className = '';
+            this.classList = new ClassList();
+            this.dataset = {{}};
+            this.style = {{}};
+            this.onclick = null;
+            this.textContent = '';
+            this._innerHTML = '';
+          }}
+          set innerHTML(value) {{
+            this._innerHTML = String(value || '');
+            if (this._innerHTML === '') this.children = [];
+          }}
+          get innerHTML() {{ return this._innerHTML; }}
+          appendChild(child) {{ this.children.push(child); return child; }}
+        }}
+        const elements = new Map();
+        for (const id of ['profileDropdown', 'profileChip', 'titlebarProfileBtn', 'titlebarProfileLabel']) {{
+          elements.set(id, new Element('div', id));
+        }}
+        globalThis.document = {{
+          createElement: (tag) => new Element(tag),
+          getElementById: (id) => elements.get(id) || null,
+          addEventListener: () => {{}},
+        }};
+        globalThis.window = {{ addEventListener: () => {{}} }};
+        const store = new Map();
+        globalThis.localStorage = {{
+          getItem: (key) => store.has(key) ? store.get(key) : null,
+          setItem: (key, value) => store.set(key, String(value)),
+          removeItem: (key) => store.delete(key),
+        }};
+        globalThis.$ = (id) => elements.get(id) || null;
+        globalThis.S = {{ activeProfile: 'default' }};
+        globalThis.t = (key, n) => key === 'profile_skill_count' ? `${{n}} skills` : key;
+        globalThis.esc = (value) => String(value == null ? '' : value)
+          .replace(/&/g, '&amp;').replace(/</g, '&lt;').replace(/>/g, '&gt;')
+          .replace(/"/g, '&quot;').replace(/'/g, '&#39;');
+        globalThis.li = () => '';
+        globalThis.closeWsDropdown = () => {{}};
+        globalThis.closeModelDropdown = () => {{}};
+        globalThis._positionProfileDropdown = () => {{}};
+        globalThis.showToast = () => {{}};
+        let switchedTo = null;
+        globalThis.switchToProfile = async (name) => {{ switchedTo = name; S.activeProfile = name; }};
+        const multiProfileResponse = {{
+          active: 'default',
+          single_profile_mode: false,
+          profiles: [
+            {{ name: 'default', visible: true, is_default: true }},
+            {{ name: 'other', visible: true }},
+          ],
+        }};
+        let apiResponse = multiProfileResponse;
+        globalThis.api = () => Promise.resolve(apiResponse);
+
+        eval(snippets.join(String.fromCharCode(10)) + String.fromCharCode(10) + `;globalThis.__profileTest={{
+          key: PROFILE_DROPDOWN_CACHE_KEY,
+          reset() {{
+            _profilesCache = null;
+            _profileDropdownFetchPromise = null;
+            _profileDropdownCacheLoadedFromStorage = false;
+            _profileDropdownOpenGeneration = 0;
+            switchedTo = null;
+            S.activeProfile = 'default';
+            const dd = document.getElementById('profileDropdown');
+            dd.children = [];
+            dd.innerHTML = '';
+            dd.classList.remove('open');
+          }},
+          usable: _profileDropdownCacheUsable,
+          dataUsable: _profileDropdownDataCacheUsable,
+          setApiResponse(data) {{ apiResponse = data; }},
+          setCache(data) {{ _profilesCache = data; }},
+          cache: () => _profilesCache,
+          profileDetailBacked(name) {{
+            return !!(_profilesCache && _profilesCache.profiles && _profilesCache.profiles.find(x => x.name === name));
+          }},
+          toggle: toggleProfileDropdown,
+          close: closeProfileDropdown,
+          switchedTo: () => switchedTo,
+        }};`);
+
+        async function runPoisonedCase(profiles) {{
+          __profileTest.reset();
+          __profileTest.setApiResponse(multiProfileResponse);
+          localStorage.setItem(__profileTest.key, JSON.stringify({{ ts: Date.now(), data: {{ active: 'default', profiles }} }}));
+          assert.strictEqual(__profileTest.usable({{ active: 'default', profiles }}), false);
+          assert.doesNotThrow(() => __profileTest.toggle({{ currentTarget: document.getElementById('profileChip') }}));
+          const dd = document.getElementById('profileDropdown');
+          assert.strictEqual(dd.classList.contains('open'), true, 'dropdown shell should open from loading state');
+          assert.strictEqual(localStorage.getItem(__profileTest.key), null, 'invalid stored cache should be removed before refresh');
+          await new Promise((resolve) => setImmediate(resolve));
+          const profileOptions = dd.children.filter((child) => String(child.className).includes('profile-opt'));
+          const other = profileOptions.find((child) => child.innerHTML.includes('other'));
+          assert(other, 'fresh refresh should render the valid profile option');
+          await other.onclick();
+          assert.strictEqual(__profileTest.switchedTo(), 'other');
+        }}
+
+        async function runSingleProfilePreservesSharedCache() {{
+          __profileTest.reset();
+          const singleProfileResponse = {{
+            active: 'default',
+            single_profile_mode: true,
+            profiles: [{{ name: 'default', visible: true, is_default: true }}],
+          }};
+          __profileTest.setApiResponse(singleProfileResponse);
+          __profileTest.setCache(singleProfileResponse);
+          assert.strictEqual(__profileTest.dataUsable(singleProfileResponse), true);
+          assert.strictEqual(__profileTest.usable(singleProfileResponse), false);
+          assert.strictEqual(__profileTest.profileDetailBacked('default'), true, 'single-profile cache should back Profiles panel before click');
+          assert.doesNotThrow(() => __profileTest.toggle({{ currentTarget: document.getElementById('profileChip') }}));
+          assert.strictEqual(__profileTest.profileDetailBacked('default'), true, 'dropdown open must not null the Profiles panel cache synchronously');
+          assert.strictEqual(__profileTest.cache(), singleProfileResponse);
+          await new Promise((resolve) => setImmediate(resolve));
+          assert.strictEqual(__profileTest.profileDetailBacked('default'), true, 'fresh single-profile response must keep Profiles panel data');
+          assert.strictEqual(__profileTest.cache().single_profile_mode, true);
+        }}
+
+        async function runFreshSingleProfileReplacesStaleMultiCache() {{
+          __profileTest.reset();
+          const staleMultiProfileResponse = multiProfileResponse;
+          const singleProfileResponse = {{
+            active: 'default',
+            single_profile_mode: true,
+            profiles: [{{ name: 'default', visible: true, is_default: true }}],
+          }};
+          __profileTest.setCache(staleMultiProfileResponse);
+          __profileTest.setApiResponse(singleProfileResponse);
+          assert.strictEqual(__profileTest.usable(staleMultiProfileResponse), true);
+          assert.doesNotThrow(() => __profileTest.toggle({{ currentTarget: document.getElementById('profileChip') }}));
+          await new Promise((resolve) => setImmediate(resolve));
+          assert.strictEqual(__profileTest.cache(), singleProfileResponse, 'fresh single-profile payload should replace stale multi-profile memory cache');
+          assert.strictEqual(__profileTest.profileDetailBacked('default'), true);
+          assert.strictEqual(localStorage.getItem(__profileTest.key), null, 'single-profile payload should not be persisted as dropdown cache');
+        }}
+
+        (async () => {{
+          await runPoisonedCase([null]);
+          await runPoisonedCase([{{}}]);
+          await runSingleProfilePreservesSharedCache();
+          await runFreshSingleProfileReplacesStaleMultiCache();
+        }})().catch((err) => {{ console.error(err && err.stack || err); process.exit(1); }});
+        """
+    )
+    subprocess.run(["node", "-e", script], cwd=REPO_ROOT, check=True, text=True, capture_output=True)