fix: review comments

### Must fix #1 — **Fixed**
`GET /api/workspaces` always returns `{"projects": [...], "warnings"?: [...]}` so the shape no longer flips at runtime.

### Should fix #2 — **Fixed**
Removed dead `attach_warnings` from `models/__init__.py` and deleted the unused module-level helper.

### Should fix #3 — **Fixed**
Added `AND value IS NOT NULL` to bubble and composer queries in `workspace_tabs.py`; removed NULL branches that called `record_*_skipped()` (tombstones are not `parse_error`).

### Nice to have #4 — **Fixed**
Renamed/updated `test_workspaces_api_object_when_clean` and adjusted `test_api_endpoints.py` `TestListWorkspaces` + exclusion tests for the object shape.

### Nice to have #5 — **Skipped**
`if count > 0` guards — style-only; no behavior change for normal callers (`count` defaults to 1).

### Extra (follow-on from #1)
- `tests/web-ui-smoke.sh` — parse `body["projects"]` when the response is an object (smoke would otherwise fail).
- `static/js/app.js` — doc comment only; `normalizeWorkspacesResponse()` still accepts a legacy array.

### Validation
- `pytest tests/test_parse_warnings.py tests/test_api_endpoints.py` — **25 passed**
- `tests/test_workspace_tabs_null_bubble.py` — included; NULL row still must not crash (filtered in SQL, no false `parse_error` banner)

Author: bradjin8

api/workspaces.py

@@ -56,9 +56,10 @@ def list_workspaces():
         workspace_path = resolve_workspace_path()
         rules = current_app.config.get("EXCLUSION_RULES") or []
         projects, warnings = list_workspace_projects(workspace_path, rules)
+        payload: dict = {"projects": projects}
         if warnings:
-            return jsonify({"projects": projects, "warnings": warnings})
-        return jsonify(projects)
+            payload["warnings"] = warnings
+        return jsonify(payload)
     except Exception:
         _logger.exception("Failed to get workspaces")
         return jsonify({"error": "Failed to get workspaces"}), 500

models/__init__.py

@@ -1,7 +1,7 @@
 from models.cli_session import CliSessionMeta
 from models.conversation import Bubble, Composer, WorkspaceLocalComposer
 from models.errors import SchemaError
-from models.parse_warnings import ParseWarningCollector, attach_warnings
+from models.parse_warnings import ParseWarningCollector
 from models.export import ExportEntry
 from models.workspace import Workspace
 
@@ -14,5 +14,4 @@
     "SchemaError",
     "Workspace",
     "WorkspaceLocalComposer",
-    "attach_warnings",
 ]

models/parse_warnings.py

@@ -72,10 +72,3 @@ def attach_to(self, payload: dict) -> dict:
         if self.has_warnings:
             payload = {**payload, "warnings": self.to_api_list()}
         return payload
-
-
-def attach_warnings(payload: dict, warnings: list[dict]) -> dict:
-    """Merge pre-built warnings into a response dict."""
-    if warnings:
-        return {**payload, "warnings": warnings}
-    return payload

services/workspace_tabs.py

@@ -134,17 +134,13 @@ def _safe_fetchall(query: str, params: tuple = ()) -> list:
                 return []
 
         # Load bubbles
-        for row in _safe_fetchall("SELECT key, value FROM cursorDiskKV WHERE key LIKE 'bubbleId:%'"):
+        for row in _safe_fetchall(
+            "SELECT key, value FROM cursorDiskKV WHERE key LIKE 'bubbleId:%'"
+            " AND value IS NOT NULL"
+        ):
             parts = row["key"].split(":")
             if len(parts) >= 3:
                 bid = parts[2]
-                if row["value"] is None:
-                    _logger.warning(
-                        "Skipping Bubble cursorDiskKV row with NULL value: key=%r",
-                        row["key"],
-                    )
-                    parse_warnings.record_bubble_skipped()
-                    continue
                 try:
                     parsed = json.loads(row["value"])
 
@@ -214,6 +210,7 @@ def _safe_fetchall(query: str, params: tuple = ()) -> list:
         # Get composer data entries with conversations
         composer_rows = _safe_fetchall(
             "SELECT key, value FROM cursorDiskKV WHERE key LIKE 'composerData:%'"
+            " AND value IS NOT NULL"
             " AND value LIKE '%fullConversationHeadersOnly%'"
             " AND value NOT LIKE '%fullConversationHeadersOnly\":[]%'"
         )
@@ -231,13 +228,6 @@ def _safe_fetchall(query: str, params: tuple = ()) -> list:
 
         for row in composer_rows:
             composer_id = row["key"].split(":")[1]
-            if row["value"] is None:
-                _logger.warning(
-                    "Skipping Composer cursorDiskKV row with NULL value: key=%r",
-                    row["key"],
-                )
-                parse_warnings.record_composer_skipped()
-                continue
             try:
                 parsed = json.loads(row["value"])
             except (json.JSONDecodeError, TypeError, ValueError) as e:

static/js/app.js

@@ -88,8 +88,8 @@ function sanitizeFilename(name) {
 }
 
 /**
- * Normalize GET /api/workspaces body: plain array (no warnings) or
- * { projects, warnings } when parse failures occurred (issue #67).
+ * Normalize GET /api/workspaces body: { projects, warnings? } (issue #67).
+ * Accepts a legacy plain array for backward compatibility.
  */
 function normalizeWorkspacesResponse(body) {
   if (Array.isArray(body)) {

tests/test_api_endpoints.py

@@ -14,20 +14,22 @@ def test_happy_path_returns_workspace_list(self, client):
         response = client.get("/api/workspaces")
         assert response.status_code == 200
         body = response.get_json()
-        assert isinstance(body, list)
+        assert isinstance(body, dict)
+        projects = body["projects"]
+        assert isinstance(projects, list)
 
-        ids = [p["id"] for p in body]
+        ids = [p["id"] for p in projects]
         assert HAPPY_WORKSPACE_ID in ids, f"expected {HAPPY_WORKSPACE_ID} in {ids}"
 
-        ws = next(p for p in body if p["id"] == HAPPY_WORKSPACE_ID)
+        ws = next(p for p in projects if p["id"] == HAPPY_WORKSPACE_ID)
         assert "name" in ws
         assert "conversationCount" in ws and isinstance(ws["conversationCount"], int)
         assert "lastModified" in ws and "T" in ws["lastModified"]
 
     def test_empty_storage_returns_empty_list(self, empty_workspace_client):
         response = empty_workspace_client.get("/api/workspaces")
         assert response.status_code == 200
-        assert response.get_json() == []
+        assert response.get_json() == {"projects": []}
 
 
 # ---------------------------------------------------------------------------
@@ -169,7 +171,7 @@ def test_workspace_matching_rule_is_filtered_out_of_list(self, workspace_storage
         response = excluded_client.get("/api/workspaces")
         assert response.status_code == 200
         body = response.get_json()
-        ids = [w["id"] for w in body]
+        ids = [w["id"] for w in body["projects"]]
         assert HAPPY_WORKSPACE_ID not in ids, (
             f"exclusion rule did not filter {HAPPY_WORKSPACE_ID}; got {ids}"
         )
@@ -182,7 +184,7 @@ def test_workspace_not_matching_rule_still_listed(self, workspace_storage):
         response = kept_client.get("/api/workspaces")
         assert response.status_code == 200
         body = response.get_json()
-        ids = [w["id"] for w in body]
+        ids = [w["id"] for w in body["projects"]]
         assert HAPPY_WORKSPACE_ID in ids, (
             f"non-matching rule filtered the workspace; got {ids}"
         )

tests/test_parse_warnings.py

@@ -246,7 +246,7 @@ def setUp(self) -> None:
         self.app.config["TESTING"] = True
         self.client = self.app.test_client()
 
-    def test_workspaces_api_array_when_clean(self) -> None:
+    def test_workspaces_api_object_when_clean(self) -> None:
         tmp = tempfile.mkdtemp()
         try:
             ws_root = _seed_clean_workspace(tmp)
@@ -258,7 +258,9 @@ def test_workspaces_api_array_when_clean(self) -> None:
 
         self.assertEqual(res.status_code, 200)
         data = res.get_json()
-        self.assertIsInstance(data, list)
+        self.assertIsInstance(data, dict)
+        self.assertIn("projects", data)
+        self.assertNotIn("warnings", data)
 
     def test_workspaces_api_object_when_warnings(self) -> None:
         tmp = tempfile.mkdtemp()

tests/test_workspace_tabs_null_bubble.py

@@ -70,23 +70,17 @@ def tearDown(self):
         self.tmp.cleanup()
 
     def test_null_bubble_row_is_skipped_without_exception(self):
-        """assemble_workspace_tabs must not raise when a bubble row has NULL value."""
+        """assemble_workspace_tabs must not raise when a NULL bubble row exists in KV."""
         try:
-            with self.assertLogs("services.workspace_tabs", level="WARNING") as cm:
-                _payload, status = assemble_workspace_tabs(
-                    workspace_id="global",
-                    workspace_path=self.workspace_path,
-                    rules=[],
-                )
+            payload, status = assemble_workspace_tabs(
+                workspace_id="global",
+                workspace_path=self.workspace_path,
+                rules=[],
+            )
         except TypeError as exc:
             self.fail(f"NULL bubble row raised TypeError: {exc}")
 
         self.assertEqual(status, 200, "NULL bubble row must not turn tabs load into an error response")
-        messages = [r.getMessage() for r in cm.records]
-        self.assertTrue(
-            any("NULL value" in m and "bubble-null" in m for m in messages),
-            f"expected NULL-value warning for bubble-null row, got: {messages}",
-        )
 
     def test_healthy_bubbles_still_load_when_null_row_present(self):
         """The healthy bubble surfaces in a tab even when a NULL row is present."""

tests/web-ui-smoke.sh

@@ -109,7 +109,8 @@ probe "/api/search (no q -> 400)" "/api/search"             400
 if WS_ID=$(curl "${CURL_FLAGS[@]}" "$BASE/api/workspaces" | python3 -c "
 import sys, json
 data = json.load(sys.stdin)
-for w in data:
+projects = data if isinstance(data, list) else data.get("projects", [])
+for w in projects:
     if w.get('id') and w['id'] != 'global':
         print(w['id'])
         break