fix: review findings

Author: bradjin8

services/summary_cache.py

@@ -61,7 +61,7 @@ def fingerprint_workspace_storage(
     cli_chats_path: str | None = None,
 ) -> dict[str, Any]:
     """Build a fingerprint dict for cache invalidation."""
-    ws_mt: list[tuple[str, int]] = []
+    ws_mt: list[list[str | int]] = []
     for entry in workspace_entries:
         name = entry.get("name")
         if not isinstance(name, str):
@@ -71,8 +71,8 @@ def fingerprint_workspace_storage(
             p = os.path.join(base, rel)
             mtime = _file_mtime_ns(p)
             if mtime is not None:
-                ws_mt.append((f"{name}/{rel}", mtime))
-    ws_mt.sort(key=lambda x: x[0])
+                ws_mt.append([f"{name}/{rel}", mtime])
+    ws_mt.sort(key=lambda row: row[0])
 
     return {
         "version": CACHE_VERSION,
@@ -84,10 +84,22 @@ def fingerprint_workspace_storage(
     }
 
 
+def _normalize_fingerprint(fp: dict[str, Any]) -> dict[str, Any]:
+    """Normalize fingerprint for comparison (JSON round-trip uses lists, not tuples)."""
+    normalized = dict(fp)
+    wf = fp.get("workspace_files")
+    if isinstance(wf, list):
+        normalized["workspace_files"] = [
+            [row[0], row[1]] if isinstance(row, (list, tuple)) and len(row) == 2 else row
+            for row in wf
+        ]
+    return normalized
+
+
 def _fingerprint_equal(a: object, b: dict[str, Any]) -> bool:
     if not isinstance(a, dict):
         return False
-    return a == b
+    return _normalize_fingerprint(a) == _normalize_fingerprint(b)
 
 
 def _read_cache_file(path: Path | str) -> dict[str, Any] | None:

services/workspace_db.py

@@ -296,6 +296,7 @@ def collect_invalid_workspace_ids(workspace_entries: list[dict]) -> set[str]:
     " AND LENGTH(value) > 10"
     " AND value LIKE '%fullConversationHeadersOnly%'"
     " AND value NOT LIKE '%fullConversationHeadersOnly\":[]%'"
+    " AND value NOT LIKE '%fullConversationHeadersOnly\": []%'"
 )
 
 

templates/workspace.html

@@ -122,6 +122,7 @@ <h2 id="project-name">Loading...</h2>
 }
 
 async function selectTab(id) {
+  selectTab._latest = id;
   const summary = allTabs.find(t => t.id === id);
   if (!summary) return;
 
@@ -139,9 +140,11 @@ <h2 id="project-name">Loading...</h2>
 
   // Resolve full tab: use cache if available, lazy-fetch if not
   if (tabCache[id]) {
+    if (selectTab._latest !== id) return;
     selectedTab = tabCache[id];
   } else if (summary.bubbles) {
     // Fallback: summary endpoint returned a full tab (e.g. old server version)
+    if (selectTab._latest !== id) return;
     selectedTab = summary;
     tabCache[id] = summary;
   } else {
@@ -151,13 +154,16 @@ <h2 id="project-name">Loading...</h2>
       '<div class="loading-center" style="min-height:60vh"><div class="spinner"></div><p>Loading conversation…</p></div>';
     try {
       const res = await fetch(`/api/workspaces/${WORKSPACE_ID}/tabs/${id}`);
+      if (selectTab._latest !== id) return;
       if (!res.ok) throw new Error(`HTTP ${res.status}`);
       const data = await res.json();
+      if (selectTab._latest !== id) return;
       if (data.error) throw new Error(data.error);
       showIncompleteResultsBanner('parse-warnings-host', data.warnings);
       tabCache[id] = data.tab;
       selectedTab = data.tab;
     } catch (e) {
+      if (selectTab._latest !== id) return;
       const main = document.getElementById('main-content');
       main.classList.remove('is-loading');
       main.innerHTML =
@@ -166,6 +172,8 @@ <h2 id="project-name">Loading...</h2>
     }
   }
 
+  if (selectTab._latest !== id) return;
+
   document.getElementById('action-buttons').style.display = 'flex';
 
   const codeEditsBtn = document.getElementById('dl-code-edits');

tests/test_summary_cache.py

@@ -65,6 +65,25 @@ def test_fingerprint_includes_workspace_files(self):
             fp = fingerprint_workspace_storage(ws, entries, global_db_path=None, rules=[])
             self.assertTrue(fp["workspace_files"])
 
+    def test_workspace_files_fingerprint_round_trip(self):
+        """JSON cache round-trip must match freshly computed workspace_files fingerprints."""
+        with tempfile.TemporaryDirectory() as ws:
+            entry_dir = os.path.join(ws, "entry1")
+            os.makedirs(entry_dir)
+            db = os.path.join(entry_dir, "state.vscdb")
+            with open(db, "wb") as f:
+                f.write(b"x")
+            entries = [{"name": "entry1", "workspaceJsonPath": os.path.join(entry_dir, "workspace.json")}]
+            fp = fingerprint_workspace_storage(ws, entries, global_db_path=None, rules=[])
+            projects = [{"id": "a", "name": "A", "conversationCount": 1, "lastModified": "x"}]
+            warnings: list = []
+            set_cached_projects(fp, projects, warnings)
+            fp2 = fingerprint_workspace_storage(ws, entries, global_db_path=None, rules=[])
+            hit = get_cached_projects(fp2)
+            self.assertIsNotNone(hit, msg="cache miss after JSON round-trip of workspace_files")
+            assert hit is not None
+            self.assertEqual(hit[0], projects)
+
 
 if __name__ == "__main__":
     unittest.main()

tests/test_workspace_listing_performance.py

@@ -13,12 +13,13 @@
 import sys
 import tempfile
 import unittest
-from unittest.mock import call, patch
+from unittest.mock import patch
 
 REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
 if REPO_ROOT not in sys.path:
     sys.path.insert(0, REPO_ROOT)
 
+import services.workspace_listing as _ws_listing_mod
 from services.workspace_listing import list_workspace_projects
 
 COMPOSER_ID = "composer-list-perf"
@@ -78,29 +79,21 @@ def test_no_global_bubble_id_query(self):
 
         executed_queries: list[str] = []
 
-        original_open_global_db = None
-        import services.workspace_db as _ws_db_mod
-
-        original_open_global_db = _ws_db_mod.open_global_db
+        original_open_global_db = _ws_listing_mod.open_global_db
 
         from contextlib import contextmanager
 
         @contextmanager
         def _spying_open_global_db(workspace_path):
             with original_open_global_db(workspace_path) as (conn, path):
                 if conn is not None:
-                    original_execute = conn.execute
-
-                    def _spying_execute(query, params=()):
-                        executed_queries.append(query)
-                        return original_execute(query, params)
-
-                    conn.execute = _spying_execute  # type: ignore[method-assign]
+                    conn.set_trace_callback(executed_queries.append)
                 yield conn, path
 
-        with patch.object(_ws_db_mod, "open_global_db", _spying_open_global_db):
-            projects, warnings = list_workspace_projects(ws_path, rules=[])
+        with patch.object(_ws_listing_mod, "open_global_db", _spying_open_global_db):
+            list_workspace_projects(ws_path, rules=[], nocache=True)
 
+        self.assertTrue(executed_queries, msg="expected SQL queries to be recorded")
         bubble_scans = [q for q in executed_queries if "bubbleId:%" in q]
         self.assertEqual(
             bubble_scans,

tests/test_workspace_tabs_summary.py

@@ -110,25 +110,19 @@ def _make_fixture(base: str) -> str:
 
 def _collect_queries(ws_path, fn):
     """Call fn(ws_path) while recording every SQL query; return (result, queries)."""
-    import services.workspace_db as _ws_db_mod
+    import services.workspace_tabs as _ws_tabs_mod
 
-    orig = _ws_db_mod.open_global_db
+    orig = _ws_tabs_mod.open_global_db
     executed: list[str] = []
 
     @contextmanager
     def _spy(workspace_path):
         with orig(workspace_path) as (conn, path):
             if conn is not None:
-                orig_exec = conn.execute
-
-                def _tracking_execute(query, params=()):
-                    executed.append(query)
-                    return orig_exec(query, params)
-
-                conn.execute = _tracking_execute  # type: ignore[method-assign]
+                conn.set_trace_callback(executed.append)
             yield conn, path
 
-    with patch.object(_ws_db_mod, "open_global_db", _spy):
+    with patch.object(_ws_tabs_mod, "open_global_db", _spy):
         result = fn(ws_path)
     return result, executed
 
@@ -145,8 +139,9 @@ def test_no_global_bubble_scan(self):
         """list_workspace_tab_summaries must not issue a bubbleId:% LIKE query."""
         (payload, status), queries = _collect_queries(
             self.ws_path,
-            lambda p: list_workspace_tab_summaries("global", p, rules=[]),
+            lambda p: list_workspace_tab_summaries("global", p, rules=[], nocache=True),
         )
+        self.assertTrue(queries, msg="expected SQL queries to be recorded")
         bubble_scans = [q for q in queries if "bubbleId:%" in q]
         self.assertEqual(
             bubble_scans,
@@ -198,6 +193,7 @@ def test_scoped_bubble_query_only(self):
             self.ws_path,
             lambda p: assemble_single_tab("global", COMPOSER_ID, p, rules=[]),
         )
+        self.assertTrue(queries, msg="expected SQL queries to be recorded")
         global_bubble_scans = [
             q for q in queries if "bubbleId:%" in q and f"bubbleId:{COMPOSER_ID}:%" not in q
         ]