Skip to content

Commit 7774f41

Browse files
committed
review: route per-row workspace-local composer drift through SchemaError (#30)
CodeRabbit flagged that the inline ``isinstance(c, dict) or not c.get(...)`` skip in api/composers.py masked row-level drift instead of surfacing it the way the rest of the boundary does. The exact suggested fix (Composer.from_dict(c)) would have rejected every workspace-local record: those carry composerId + lastUpdatedAt but not fullConversationHeadersOnly or createdAt, which only exist on the global cursorDiskKV rows. Add WorkspaceLocalComposer to models/conversation.py — a slim sibling model that validates the workspace-local shape (composerId required as a non-empty string; non-dict raw raises). api/composers.py:list_composers now wraps each entry in WorkspaceLocalComposer.from_dict, so per-row drift logs a Schema drift warning and skips, matching every other read-site in this PR. Regression coverage in tests/test_models.py: parses good shape; raises SchemaError for missing/empty/non-string composerId and for non-dict payloads. 204 tests pass (was 201; +3); mypy clean on models/; real Cursor smoke test shows zero drift warnings.
1 parent e853dfd commit 7774f41

4 files changed

Lines changed: 78 additions & 3 deletions

File tree

api/composers.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313

1414
from utils.workspace_path import resolve_workspace_path
1515
from utils.path_helpers import to_epoch_ms
16-
from models import SchemaError
16+
from models import SchemaError, WorkspaceLocalComposer
1717

1818
bp = Blueprint("composers", __name__)
1919

@@ -71,7 +71,10 @@ def list_composers():
7171
hint=f"expected list, got {type(all_composers).__name__}",
7272
)
7373
for c in all_composers:
74-
if not isinstance(c, dict) or not c.get("composerId"):
74+
try:
75+
WorkspaceLocalComposer.from_dict(c)
76+
except SchemaError as e:
77+
print(f"Schema drift in {db_path}: {e}")
7578
continue
7679
c["conversation"] = c.get("conversation") or []
7780
c["workspaceId"] = name

models/__init__.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
"""
1010

1111
from models.cli_session import CliSessionMeta
12-
from models.conversation import Bubble, Composer
12+
from models.conversation import Bubble, Composer, WorkspaceLocalComposer
1313
from models.errors import SchemaError
1414
from models.export import ExportEntry
1515
from models.workspace import Workspace
@@ -21,4 +21,5 @@
2121
"ExportEntry",
2222
"SchemaError",
2323
"Workspace",
24+
"WorkspaceLocalComposer",
2425
]

models/conversation.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,46 @@ def from_dict(cls, raw: dict[str, Any], *, composer_id: str) -> "Composer":
7070
)
7171

7272

73+
@dataclass(frozen=True)
74+
class WorkspaceLocalComposer:
75+
"""A composer entry from ``composer.composerData`` ItemTable rows.
76+
77+
These are summary records that live in each per-workspace ``state.vscdb``.
78+
They share ``composerId`` and ``lastUpdatedAt`` with the global composer
79+
schema, but they do **not** carry ``fullConversationHeadersOnly`` or
80+
``createdAt`` — those only exist on the global ``cursorDiskKV`` rows that
81+
``Composer.from_dict`` validates. Treating both shapes through the same
82+
model would reject every workspace-local entry, so this slim model
83+
exists to keep schema-drift detection at the boundary without conflating
84+
the two storage paths.
85+
"""
86+
87+
composer_id: str
88+
last_updated_at: Any = None
89+
raw: dict[str, Any] = field(default_factory=dict)
90+
91+
@classmethod
92+
def from_dict(cls, raw: dict[str, Any]) -> "WorkspaceLocalComposer":
93+
if not isinstance(raw, dict):
94+
raise SchemaError(
95+
"WorkspaceLocalComposer",
96+
"composer",
97+
hint=f"expected object, got {type(raw).__name__}",
98+
)
99+
composer_id = raw.get("composerId")
100+
if not isinstance(composer_id, str) or not composer_id:
101+
raise SchemaError(
102+
"WorkspaceLocalComposer",
103+
"composerId",
104+
hint=f"expected non-empty str, got {type(composer_id).__name__}",
105+
)
106+
return cls(
107+
composer_id=composer_id,
108+
last_updated_at=raw.get("lastUpdatedAt"),
109+
raw=raw,
110+
)
111+
112+
73113
@dataclass(frozen=True)
74114
class Bubble:
75115
"""A single message bubble within a composer.

tests/test_models.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
ExportEntry,
2929
SchemaError,
3030
Workspace,
31+
WorkspaceLocalComposer,
3132
)
3233
from utils.cli_chat_reader import _extract_blob_refs
3334

@@ -85,6 +86,17 @@ def test_workspace_parses_with_optional_folder(self) -> None:
8586
ws_no_folder = Workspace.from_dict({}, workspace_id="cli-only")
8687
self.assertEqual(ws_no_folder.folder, None)
8788

89+
def test_workspace_local_composer_parses(self) -> None:
90+
# Workspace-local composers (composer.composerData ItemTable rows)
91+
# carry composerId + lastUpdatedAt but not the global-storage fields.
92+
c = WorkspaceLocalComposer.from_dict({
93+
"composerId": "cid-local-1",
94+
"lastUpdatedAt": 1_715_000_500_000,
95+
"conversation": [],
96+
})
97+
self.assertEqual(c.composer_id, "cid-local-1")
98+
self.assertEqual(c.last_updated_at, 1_715_000_500_000)
99+
88100
def test_export_entry_parses(self) -> None:
89101
entry = ExportEntry.from_dict({
90102
"log_id": "L1",
@@ -149,6 +161,25 @@ def test_bubble_empty_id_raises(self) -> None:
149161
with self.assertRaises(SchemaError):
150162
Bubble.from_dict({"text": "hi"}, bubble_id="")
151163

164+
def test_workspace_local_composer_missing_id_raises(self) -> None:
165+
# Regression for CodeRabbit's per-row drift comment on api/composers.py:
166+
# previously a row missing composerId was silently skipped. Now drift
167+
# raises SchemaError so api/composers.py can log + skip explicitly.
168+
for bad in (
169+
{"lastUpdatedAt": 0}, # composerId absent
170+
{"composerId": ""}, # empty string
171+
{"composerId": None}, # None
172+
{"composerId": 123}, # wrong type
173+
):
174+
with self.assertRaises(SchemaError, msg=f"failed for {bad!r}") as cm:
175+
WorkspaceLocalComposer.from_dict(bad)
176+
self.assertEqual(cm.exception.field, "composerId")
177+
178+
def test_workspace_local_composer_non_dict_raises(self) -> None:
179+
for bad in (None, [], "str", 42):
180+
with self.assertRaises(SchemaError):
181+
WorkspaceLocalComposer.from_dict(bad) # type: ignore[arg-type]
182+
152183
def test_export_entry_missing_required_raises(self) -> None:
153184
with self.assertRaises(SchemaError) as cm:
154185
ExportEntry.from_dict({"title": "x", "workspace": "w"})

0 commit comments

Comments
 (0)