Skip to content

Commit 59ae103

Browse files
committed
review: tighten ID + createdAt type gates across the model layer (#30)
Three related CodeRabbit findings, addressed together: - Composer.composer_id, Bubble.bubble_id, Workspace.workspace_id now require ``isinstance(..., str) and value != ""``. Truthiness alone let non-string IDs (e.g. ``123``) slip past the boundary; aligned with the pattern already in WorkspaceLocalComposer. - Composer.createdAt is now type-gated. Presence was already enforced but any value type (e.g. ``"2026-05-12"``) silently became the timestamp. Required values must be ``int`` or ``float`` and explicitly not ``bool`` (since ``bool`` is a subclass of ``int`` in Python and a boolean is never a real timestamp). Regression tests pin each new gate: - test_non_string_composer_id_raises - test_non_string_bubble_id_raises - test_non_string_workspace_id_raises - test_non_numeric_created_at_raises - test_numeric_created_at_passes 209 tests pass (was 204; +5 new).
1 parent 26b565e commit 59ae103

3 files changed

Lines changed: 58 additions & 7 deletions

File tree

models/conversation.py

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,25 @@ def from_dict(cls, raw: dict[str, Any], *, composer_id: str) -> "Composer":
2626
"composerData",
2727
hint=f"expected object, got {type(raw).__name__}",
2828
)
29-
if not composer_id:
30-
raise SchemaError("Composer", "composerId", hint="empty composer ID")
29+
if not isinstance(composer_id, str) or not composer_id:
30+
raise SchemaError(
31+
"Composer",
32+
"composerId",
33+
hint=f"expected non-empty str, got {type(composer_id).__name__}",
34+
)
3135
if "fullConversationHeadersOnly" not in raw:
3236
raise SchemaError("Composer", "fullConversationHeadersOnly")
3337
if "createdAt" not in raw:
3438
raise SchemaError("Composer", "createdAt")
3539

40+
created_at = raw.get("createdAt")
41+
if not isinstance(created_at, (int, float)) or isinstance(created_at, bool):
42+
raise SchemaError(
43+
"Composer",
44+
"createdAt",
45+
hint=f"expected timestamp number, got {type(created_at).__name__}",
46+
)
47+
3648
headers = raw.get("fullConversationHeadersOnly")
3749
if not isinstance(headers, list):
3850
raise SchemaError(
@@ -48,7 +60,7 @@ def from_dict(cls, raw: dict[str, Any], *, composer_id: str) -> "Composer":
4860
return cls(
4961
composer_id=composer_id,
5062
full_conversation_headers_only=headers,
51-
created_at=raw.get("createdAt"),
63+
created_at=created_at,
5264
name=raw.get("name"),
5365
last_updated_at=raw.get("lastUpdatedAt"),
5466
model_config=model_config,
@@ -101,6 +113,10 @@ def from_dict(cls, raw: dict[str, Any], *, bubble_id: str) -> "Bubble":
101113
"bubble",
102114
hint=f"expected object, got {type(raw).__name__}",
103115
)
104-
if not bubble_id:
105-
raise SchemaError("Bubble", "bubbleId", hint="empty bubble ID")
116+
if not isinstance(bubble_id, str) or not bubble_id:
117+
raise SchemaError(
118+
"Bubble",
119+
"bubbleId",
120+
hint=f"expected non-empty str, got {type(bubble_id).__name__}",
121+
)
106122
return cls(bubble_id=bubble_id, raw=raw)

models/workspace.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,12 @@ def from_dict(cls, raw: dict[str, Any], *, workspace_id: str) -> "Workspace":
2222
"workspace.json",
2323
hint=f"expected object, got {type(raw).__name__}",
2424
)
25-
if not workspace_id:
26-
raise SchemaError("Workspace", "workspaceId", hint="empty workspace ID")
25+
if not isinstance(workspace_id, str) or not workspace_id:
26+
raise SchemaError(
27+
"Workspace",
28+
"workspaceId",
29+
hint=f"expected non-empty str, got {type(workspace_id).__name__}",
30+
)
2731
folder = raw.get("folder")
2832
if folder is not None and not isinstance(folder, str):
2933
raise SchemaError(

tests/test_models.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,12 @@ def test_workspace_parses_with_optional_folder(self) -> None:
6464
ws_no_folder = Workspace.from_dict({}, workspace_id="cli-only")
6565
self.assertEqual(ws_no_folder.folder, None)
6666

67+
def test_non_string_workspace_id_raises(self) -> None:
68+
for bad_id in (None, "", 123, [], {}, True, b"bytes"):
69+
with self.assertRaises(SchemaError, msg=f"failed for {bad_id!r}") as cm:
70+
Workspace.from_dict({}, workspace_id=bad_id) # type: ignore[arg-type]
71+
self.assertEqual(cm.exception.field, "workspaceId")
72+
6773
def test_workspace_local_composer_parses(self) -> None:
6874
c = WorkspaceLocalComposer.from_dict({
6975
"composerId": "cid-local-1",
@@ -99,11 +105,30 @@ def test_missing_created_at_raises(self) -> None:
99105
Composer.from_dict(bad, composer_id="cid-001")
100106
self.assertEqual(cm.exception.field, "createdAt")
101107

108+
def test_non_numeric_created_at_raises(self) -> None:
109+
for bad_value in ("2026-05-12", None, [], {}, True, False, b"bytes"):
110+
bad = dict(GOOD_COMPOSER_RAW, createdAt=bad_value)
111+
with self.assertRaises(SchemaError, msg=f"failed for {bad_value!r}") as cm:
112+
Composer.from_dict(bad, composer_id="cid-001")
113+
self.assertEqual(cm.exception.field, "createdAt")
114+
115+
def test_numeric_created_at_passes(self) -> None:
116+
for ok_value in (1_715_000_000_000, 1_715_000_000_000.5, 0, -1):
117+
ok = dict(GOOD_COMPOSER_RAW, createdAt=ok_value)
118+
composer = Composer.from_dict(ok, composer_id="cid-001")
119+
self.assertEqual(composer.created_at, ok_value)
120+
102121
def test_empty_composer_id_raises(self) -> None:
103122
with self.assertRaises(SchemaError) as cm:
104123
Composer.from_dict(GOOD_COMPOSER_RAW, composer_id="")
105124
self.assertEqual(cm.exception.field, "composerId")
106125

126+
def test_non_string_composer_id_raises(self) -> None:
127+
for bad_id in (None, 123, [], {}, True, b"bytes"):
128+
with self.assertRaises(SchemaError, msg=f"failed for {bad_id!r}") as cm:
129+
Composer.from_dict(GOOD_COMPOSER_RAW, composer_id=bad_id) # type: ignore[arg-type]
130+
self.assertEqual(cm.exception.field, "composerId")
131+
107132
def test_headers_wrong_type_raises(self) -> None:
108133
bad = dict(GOOD_COMPOSER_RAW, fullConversationHeadersOnly={"not": "a list"})
109134
with self.assertRaises(SchemaError) as cm:
@@ -125,6 +150,12 @@ def test_bubble_empty_id_raises(self) -> None:
125150
with self.assertRaises(SchemaError):
126151
Bubble.from_dict({"text": "hi"}, bubble_id="")
127152

153+
def test_non_string_bubble_id_raises(self) -> None:
154+
for bad_id in (None, 123, [], {}, True, b"bytes"):
155+
with self.assertRaises(SchemaError, msg=f"failed for {bad_id!r}") as cm:
156+
Bubble.from_dict({"text": "hi"}, bubble_id=bad_id) # type: ignore[arg-type]
157+
self.assertEqual(cm.exception.field, "bubbleId")
158+
128159
def test_workspace_local_composer_missing_id_raises(self) -> None:
129160
for bad in (
130161
{"lastUpdatedAt": 0}, # composerId absent

0 commit comments

Comments
 (0)