review: narrow excepts + drop duplicate queries / synthetic bubbles (#31, round 5)

Eight findings from Brad's review, addressed together. Recurring shape:
broad ``except Exception`` blocks were swallowing not just the realistic
failure (KeyError on missing dict field, sqlite3.Error on corrupt table,
OSError on missing file) but also bugs we'd want to know about. Each
catch is now narrowed to the actual failure mode the surrounding code
intends to handle, with regression coverage to pin the contract.

  1. services/workspace_db.py:_build_composer_id_to_workspace_id —
     narrow sqlite3.Error catch on the per-workspace state.vscdb open,
     json.JSONDecodeError / ValueError catch on the row parse. Round-3
     work; one site Brad flagged in the same pass.
     Test: tests/test_workspace_db_special_paths.py::
     TestBuildComposerMappingCorruptDb.

  2. services/workspace_resolver.py:_infer_workspace_name_from_context —
     same narrowing on the local-DB query (lconn.execute fetchone).
     Test: tests/test_workspace_name_db_errors.py::
     TestLocalQueryErrorSwallowed.

  3. services/workspace_listing.py:list_workspace_projects — local
     _safe_fetchall(query, params=()) helper routes the three
     cursorDiskKV LIKE queries (composerData, messageRequestContext,
     bubbleId) through sqlite3.Error → []. Same pattern as
     workspace_tabs.py round 4.
     Test: tests/test_workspace_listing_sql_errors.py.

  4. api/workspaces.py:get_workspace CLI branch — replaced
     cp["workspace_name"] / cp["last_updated_ms"] / cp["workspace_path"]
     with .get(); added isinstance(cp, dict) skip to the lookup
     generator. Brad's "unsafe dict access in CLI branch" finding.
     Test: tests/test_get_workspace_cli_malformed.py (two cases).

  5. services/workspace_listing.py CLI section — the projects loop now
     guards cp non-dict, missing project_id, non-list sessions, and
     non-dict session entries. Brad's "unsafe dict access in CLI
     project loop" finding.
     Test: tests/test_workspace_listing_cli.py::
     TestMalformedCliProjectRecordSkipped.

  6. services/cli_tabs.py:_get_cli_workspace_tabs — lookup generator
     filters isinstance(cp, dict); project field access switched to
     .get(); session iteration uses isinstance(session, dict). Brad's
     "unsafe dict access in project lookup" finding on lines 17/22/25.
     Test: tests/test_cli_tabs.py::TestMalformedCliProjectsListLookup.

  7. services/workspace_tabs.py — diffs no longer double-represented.
     Previously each diff was emitted both as ``tab.codeBlockDiffs``
     (which the frontend at dashboard/static/js/download.js:128,274
     and templates/workspace.html:135 actually reads) AND as a
     synthetic ``**Tool Action:**`` AI bubble in ``tab.bubbles`` (no
     consumer). Dropped the synthesis loop + the now-unused
     ``synthetic`` flag and its filter in the response-time pass.
     Brad's "diffs double-represented" finding.
     Test: tests/test_workspace_tabs_malformed_nested.py::
     TestDiffsEmittedOnlyAsCodeBlockDiffs replaces the prior
     synthetic-timestamp tests.

  8. services/workspace_tabs.py — merged the two
     ``messageRequestContext:%`` LIKE queries (built message_request_
     context_map and project_layouts_map respectively) into a single
     pass that fills both maps. Brad's "messageRequestContext queried
     twice" finding.

  9. services/workspace_db.py — narrowed the two remaining broad
     excepts: _collect_workspace_entries → OSError;
     _collect_invalid_workspace_ids → (OSError, ValueError, KeyError,
     TypeError). Brad's "broad silent except Exception: pass" finding.

207 tests pass (was 196; +11 across this round). The recurring
heuristic across all four rounds on this PR: every nested iteration
site needs ``isinstance(..., dict)``, every DB read needs
``sqlite3.Error`` catch, and broad excepts swallow the bugs we want
to know about.

Author: timon0305

api/workspaces.py

@@ -68,20 +68,22 @@ def get_workspace(workspace_id):
             project_id = workspace_id[4:]
             cli_projects = list_cli_projects(get_cli_chats_path())
             for cp in cli_projects:
-                if cp["project_id"] == project_id:
-                    last_ms = cp["last_updated_ms"]
-                    return jsonify({
-                        "id": workspace_id,
-                        "name": cp["workspace_name"] or project_id[:12],
-                        "path": cp["workspace_path"],
-                        "folder": cp["workspace_path"],
-                        "lastModified": (
-                            datetime.fromtimestamp(last_ms / 1000, tz=timezone.utc).isoformat()
-                            if last_ms
-                            else datetime.now(tz=timezone.utc).isoformat()
-                        ),
-                        "source": "cli",
-                    })
+                if not isinstance(cp, dict) or cp.get("project_id") != project_id:
+                    continue
+                last_ms = cp.get("last_updated_ms")
+                workspace_path_field = cp.get("workspace_path")
+                return jsonify({
+                    "id": workspace_id,
+                    "name": cp.get("workspace_name") or project_id[:12],
+                    "path": workspace_path_field,
+                    "folder": workspace_path_field,
+                    "lastModified": (
+                        datetime.fromtimestamp(last_ms / 1000, tz=timezone.utc).isoformat()
+                        if last_ms
+                        else datetime.now(tz=timezone.utc).isoformat()
+                    ),
+                    "source": "cli",
+                })
             return jsonify({"error": "CLI project not found"}), 404
 
         workspace_path = resolve_workspace_path()

services/cli_tabs.py

@@ -14,15 +14,26 @@ def _get_cli_workspace_tabs(workspace_id: str):
     try:
         project_id = workspace_id[4:]
         cli_projects = list_cli_projects(get_cli_chats_path())
-        project = next((cp for cp in cli_projects if cp["project_id"] == project_id), None)
+        project = next(
+            (
+                cp for cp in cli_projects
+                if isinstance(cp, dict) and cp.get("project_id") == project_id
+            ),
+            None,
+        )
         if project is None:
             return jsonify({"error": "CLI project not found"}), 404
 
         rules = current_app.config.get("EXCLUSION_RULES") or []
-        ws_name = project["workspace_name"] or project_id[:12]
+        ws_name = project.get("workspace_name") or project_id[:12]
+        sessions = project.get("sessions") or []
+        if not isinstance(sessions, list):
+            sessions = []
         tabs = []
 
-        for session in project["sessions"]:
+        for session in sessions:
+            if not isinstance(session, dict):
+                continue
             session_id = session.get("session_id")
             if not session_id:
                 continue

services/workspace_db.py

@@ -20,7 +20,10 @@ def _collect_workspace_entries(workspace_path: str) -> list[dict]:
                 wj = os.path.join(full, "workspace.json")
                 if os.path.isfile(wj):
                     entries.append({"name": name, "workspaceJsonPath": wj})
-    except Exception:
+    except OSError:
+        # workspace_path missing / not readable / not a directory — return what
+        # we have so far. OSError covers FileNotFoundError, PermissionError,
+        # and NotADirectoryError.
         pass
     return entries
 
@@ -34,7 +37,11 @@ def _collect_invalid_workspace_ids(workspace_entries: list[dict]) -> set[str]:
             folders = get_workspace_folder_paths(wd)
             if not folders:
                 invalid.add(entry["name"])
-        except Exception:
+        except (OSError, ValueError, KeyError, TypeError):
+            # OSError: workspace.json unreadable. ValueError covers
+            # json.JSONDecodeError. KeyError / TypeError: malformed entry
+            # dict. Any of these mean we can't resolve folders → mark invalid,
+            # matching the pre-narrowing behaviour.
             invalid.add(entry["name"])
     return invalid
 
@@ -46,27 +53,33 @@ def _build_composer_id_to_workspace_id(workspace_path: str, workspace_entries: l
         db_path = os.path.join(workspace_path, entry["name"], "state.vscdb")
         if not os.path.isfile(db_path):
             continue
+        # closing() guarantees .close() on scope exit (issue #17).
+        # Path.as_uri() percent-encodes reserved chars; ``f"file:{path}"``
+        # breaks sqlite URI parsing on paths with spaces, ``#``, etc.
+        db_uri = Path(db_path).resolve().as_uri() + "?mode=ro"
+        row: tuple | None = None
         try:
-            # closing() guarantees .close() on scope exit (issue #17).
-            # Path.as_uri() percent-encodes reserved chars; ``f"file:{path}"``
-            # breaks sqlite URI parsing on paths with spaces, ``#``, etc.
-            db_uri = Path(db_path).resolve().as_uri() + "?mode=ro"
             with closing(sqlite3.connect(db_uri, uri=True)) as conn:
                 row = conn.execute(
                     "SELECT value FROM ItemTable WHERE [key] = 'composer.composerData'"
                 ).fetchone()
-            if row and row[0]:
-                data = json.loads(row[0])
-                all_composers = data.get("allComposers")
-                if isinstance(all_composers, list):
-                    for c in all_composers:
-                        if not isinstance(c, dict):
-                            continue
-                        cid = c.get("composerId")
-                        if cid:
-                            mapping[cid] = entry["name"]
-        except Exception:
-            pass
+        except sqlite3.Error:
+            continue
+        if not (row and row[0]):
+            continue
+        try:
+            data = json.loads(row[0])
+        except (json.JSONDecodeError, ValueError):
+            continue
+        all_composers = data.get("allComposers") if isinstance(data, dict) else None
+        if not isinstance(all_composers, list):
+            continue
+        for c in all_composers:
+            if not isinstance(c, dict):
+                continue
+            cid = c.get("composerId")
+            if cid:
+                mapping[cid] = entry["name"]
     return mapping
 
 

services/workspace_listing.py

@@ -2,6 +2,7 @@
 
 import json
 import os
+import sqlite3
 from datetime import datetime, timezone
 
 from utils.cli_chat_reader import list_cli_projects
@@ -43,14 +44,19 @@ def list_workspace_projects(workspace_path: str, rules: list) -> list[dict]:
     # closing semantics now baked into the context manager (issue #17).
     with _open_global_db(workspace_path) as (global_db, _):
         if global_db:
+            def _safe_fetchall(query: str, params: tuple = ()) -> list:
+                try:
+                    return global_db.execute(query, params).fetchall()
+                except sqlite3.Error:
+                    return []
             try:
-                composer_rows = global_db.execute(
+                composer_rows = _safe_fetchall(
                     "SELECT key, value FROM cursorDiskKV WHERE key LIKE 'composerData:%' AND LENGTH(value) > 10"
-                ).fetchall()
+                )
 
-                ctx_rows = global_db.execute(
+                ctx_rows = _safe_fetchall(
                     "SELECT key, value FROM cursorDiskKV WHERE key LIKE 'messageRequestContext:%'"
-                ).fetchall()
+                )
                 project_layouts_map: dict[str, list] = {}
                 for row in ctx_rows:
                     parts = row["key"].split(":")
@@ -74,9 +80,9 @@ def list_workspace_projects(workspace_path: str, rules: list) -> list[dict]:
                     except Exception:
                         pass
 
-                bubble_rows = global_db.execute(
+                bubble_rows = _safe_fetchall(
                     "SELECT key, value FROM cursorDiskKV WHERE key LIKE 'bubbleId:%'"
-                ).fetchall()
+                )
                 bubble_map: dict[str, dict] = {}
                 for row in bubble_rows:
                     parts = row["key"].split(":")
@@ -227,11 +233,21 @@ def list_workspace_projects(workspace_path: str, rules: list) -> list[dict]:
     try:
         cli_projects = list_cli_projects(get_cli_chats_path())
         for cp in cli_projects:
-            ws_name = cp["workspace_name"] or cp["project_id"][:12]
+            if not isinstance(cp, dict):
+                continue
+            project_id = cp.get("project_id")
+            if not isinstance(project_id, str) or not project_id:
+                continue
+            ws_name = cp.get("workspace_name") or project_id[:12]
             if is_excluded_by_rules(rules, ws_name):
                 continue
+            sessions = cp.get("sessions") or []
+            if not isinstance(sessions, list):
+                continue
             cli_convos = []
-            for s in cp["sessions"]:
+            for s in sessions:
+                if not isinstance(s, dict):
+                    continue
                 session_id = s.get("session_id")
                 if not session_id:
                     continue
@@ -245,9 +261,9 @@ def list_workspace_projects(workspace_path: str, rules: list) -> list[dict]:
                     cli_convos.append(session_name)
             if not cli_convos:
                 continue
-            last_ms = cp["last_updated_ms"]
+            last_ms = cp.get("last_updated_ms")
             projects.append({
-                "id": f"cli:{cp['project_id']}",
+                "id": f"cli:{project_id}",
                 "name": ws_name,
                 "conversationCount": len(cli_convos),
                 "lastModified": (

services/workspace_resolver.py

@@ -42,23 +42,27 @@ def _infer_workspace_name_from_context(workspace_path: str, workspace_id: str) -
     if not os.path.isfile(local_db_path):
         return None
     composer_ids: list[str] = []
+    # closing() guarantees .close() on scope exit (issue #17).
+    # Path.as_uri() percent-encodes reserved chars (#, ?, spaces, etc.);
+    # naive f"file:{path}" breaks sqlite URI parsing.
+    _db_uri = Path(local_db_path).resolve().as_uri() + "?mode=ro"
+    row: tuple | None = None
     try:
-        # closing() guarantees .close() on scope exit (issue #17).
-        # Path.as_uri() percent-encodes reserved chars (#, ?, spaces, etc.);
-        # naive f"file:{path}" breaks sqlite URI parsing.
-        _db_uri = Path(local_db_path).resolve().as_uri() + "?mode=ro"
         with closing(sqlite3.connect(_db_uri, uri=True)) as lconn:
             row = lconn.execute(
                 "SELECT value FROM ItemTable WHERE [key] = 'composer.composerData'"
             ).fetchone()
-        if row and row[0]:
-            data = json.loads(row[0])
-            for c in (data.get("allComposers") or []):
-                cid = c.get("composerId") if isinstance(c, dict) else None
-                if cid:
-                    composer_ids.append(cid)
-    except Exception:
+    except sqlite3.Error:
         return None
+    if row and row[0]:
+        try:
+            data = json.loads(row[0])
+        except (json.JSONDecodeError, ValueError):
+            return None
+        for c in (data.get("allComposers") or []):
+            cid = c.get("composerId") if isinstance(c, dict) else None
+            if cid:
+                composer_ids.append(cid)
     if not composer_ids:
         return None
 

services/workspace_tabs.py

@@ -12,7 +12,7 @@
     to_epoch_ms,
 )
 from utils.exclusion_rules import build_searchable_text, is_excluded_by_rules
-from utils.text_extract import extract_text_from_bubble, format_tool_action
+from utils.text_extract import extract_text_from_bubble
 from utils.tool_parser import parse_tool_call as _parse_tool_call
 from utils.workspace_descriptor import _read_json_file
 from services.workspace_db import (
@@ -116,42 +116,41 @@ def _safe_fetchall(query: str, params: tuple = ()) -> list:
             except Exception:
                 pass
 
-        # Load messageRequestContext
+        # Load messageRequestContext rows once; build both
+        # message_request_context_map and project_layouts_map from the same pass.
+        project_layouts_map: dict[str, list] = {}
         for row in _safe_fetchall("SELECT key, value FROM cursorDiskKV WHERE key LIKE 'messageRequestContext:%'"):
             parts = row["key"].split(":")
+            if len(parts) < 2:
+                continue
+            chat_id = parts[1]
+            try:
+                ctx = json.loads(row["value"])
+            except Exception:
+                continue
+            if not isinstance(ctx, dict):
+                continue
+
+            # Per-bubble context map (needs the contextId at parts[2])
             if len(parts) >= 3:
-                chat_id = parts[1]
                 context_id = parts[2]
-                try:
-                    ctx = json.loads(row["value"])
-                    message_request_context_map.setdefault(chat_id, []).append({
-                        **ctx,
-                        "contextId": context_id,
-                    })
-                except Exception:
-                    pass
+                message_request_context_map.setdefault(chat_id, []).append({
+                    **ctx,
+                    "contextId": context_id,
+                })
 
-        # Build projectLayoutsMap
-        project_layouts_map: dict[str, list] = {}
-        for row in _safe_fetchall("SELECT key, value FROM cursorDiskKV WHERE key LIKE 'messageRequestContext:%'"):
-            parts = row["key"].split(":")
-            if len(parts) >= 2:
-                cid = parts[1]
-                try:
-                    ctx = json.loads(row["value"])
-                    layouts = ctx.get("projectLayouts")
-                    if isinstance(layouts, list):
-                        project_layouts_map.setdefault(cid, [])
-                        for layout in layouts:
-                            if isinstance(layout, str):
-                                try:
-                                    layout = json.loads(layout)
-                                except Exception:
-                                    continue
-                            if isinstance(layout, dict) and layout.get("rootPath"):
-                                project_layouts_map[cid].append(layout["rootPath"])
-                except Exception:
-                    pass
+            # Project-layout map (root paths used by the resolver)
+            layouts = ctx.get("projectLayouts")
+            if isinstance(layouts, list):
+                project_layouts_map.setdefault(chat_id, [])
+                for layout in layouts:
+                    if isinstance(layout, str):
+                        try:
+                            layout = json.loads(layout)
+                        except Exception:
+                            continue
+                    if isinstance(layout, dict) and layout.get("rootPath"):
+                        project_layouts_map[chat_id].append(layout["rootPath"])
 
         # Get composer data entries with conversations
         composer_rows = _safe_fetchall(
@@ -363,22 +362,13 @@ def _safe_fetchall(query: str, params: tuple = ()) -> list:
                 )):
                     continue
 
-                # Code block diffs as extra bubbles
+                # codeBlockDiffs are emitted as a structured ``tab.codeBlockDiffs``
+                # field below; the dashboard reads them from there (download.js,
+                # workspace.html). Previously this loop also pushed a synthetic
+                # ``Tool Action`` AI bubble into ``tab.bubbles``, double-representing
+                # every diff on the wire and forcing a ``synthetic`` filter in the
+                # response-time pass. Dropping the synthesis — frontend never read it.
                 diffs = code_block_diff_map.get(composer_id, [])
-                for diff in diffs:
-                    diff_text = format_tool_action(diff)
-                    if diff_text.strip():
-                        synthetic_ts = (
-                            to_epoch_ms(diff.get("timestamp"))
-                            or to_epoch_ms(diff.get("createdAt"))
-                            or max((b.get("timestamp") or 0) for b in bubbles)
-                        )
-                        bubbles.append({
-                            "type": "ai",
-                            "text": f"**Tool Action:**{diff_text}",
-                            "timestamp": synthetic_ts,
-                            "synthetic": True,
-                        })
 
                 bubbles.sort(key=lambda b: b.get("timestamp") or 0)
 
@@ -387,7 +377,7 @@ def _safe_fetchall(query: str, params: tuple = ()) -> list:
                 for b in bubbles:
                     if b["type"] == "user":
                         last_user_ts = b.get("timestamp")
-                    elif b["type"] == "ai" and last_user_ts is not None and not b.get("synthetic"):
+                    elif b["type"] == "ai" and last_user_ts is not None:
                         ts = b.get("timestamp")
                         if ts and ts > last_user_ts:
                             meta = b.setdefault("metadata", {})