Skip to content

Commit 4e7a23b

Browse files
committed
fix: code rabbitai comments
1 parent d55881f commit 4e7a23b

4 files changed

Lines changed: 118 additions & 4 deletions

File tree

services/workspace_resolver.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,13 @@ def _infer_invalid_workspace_aliases(
342342
e,
343343
)
344344
continue
345+
if not isinstance(cd, dict):
346+
_logger.warning(
347+
"Failed to parse Composer from composerData:%s: expected object, got %s",
348+
cid,
349+
type(cd).__name__,
350+
)
351+
continue
345352
inferred = _determine_project_for_conversation(
346353
cd,
347354
cid,

services/workspace_tabs.py

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,15 @@ def _safe_fetchall(query: str, params: tuple = ()) -> list:
114114
parts = row["key"].split(":")
115115
if len(parts) >= 3:
116116
bid = parts[2]
117-
parsed = _try_loads_kv_value(row["value"])
118-
if parsed is None:
117+
try:
118+
parsed = json.loads(row["value"])
119+
except json.JSONDecodeError as e:
120+
_logger.warning(
121+
"Failed to decode Bubble from %s: %s (value: %r)",
122+
row["key"],
123+
e,
124+
row["value"],
125+
)
119126
continue
120127
try:
121128
bubble_obj = Bubble.from_dict(parsed, bubble_id=bid)
@@ -185,8 +192,15 @@ def _safe_fetchall(query: str, params: tuple = ()) -> list:
185192

186193
for row in composer_rows:
187194
composer_id = row["key"].split(":")[1]
188-
parsed = _try_loads_kv_value(row["value"])
189-
if parsed is None:
195+
try:
196+
parsed = json.loads(row["value"])
197+
except json.JSONDecodeError as e:
198+
_logger.warning(
199+
"Failed to decode Composer from composerData:%s: %s (value: %r)",
200+
composer_id,
201+
e,
202+
row["value"],
203+
)
190204
continue
191205
try:
192206
composer = Composer.from_dict(parsed, composer_id=composer_id)

tests/test_invalid_workspace_aliases.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,36 @@ def test_drifted_composer_does_not_skew_vote(self):
8787
# cid-3 is dropped (drift), so boost-ws wins 2-0 (not 2-1)
8888
self.assertEqual(aliases.get("invalid-ws"), "boost-ws")
8989

90+
def test_non_dict_composer_json_skipped_without_crash(self) -> None:
91+
composer_rows = [
92+
{"key": "composerData:cid-1", "value": json.dumps({"createdAt": 1_715_000_000_000, "fullConversationHeadersOnly": []})},
93+
{"key": "composerData:cid-2", "value": json.dumps({"createdAt": 1_715_000_000_000, "fullConversationHeadersOnly": []})},
94+
{"key": "composerData:cid-bad", "value": json.dumps("not-a-dict")},
95+
]
96+
composer_id_to_ws = {"cid-1": "invalid-ws", "cid-2": "invalid-ws", "cid-bad": "invalid-ws"}
97+
project_layouts_map = {
98+
"cid-1": [normalize_file_path(r"d:\_Cpp_Digest\boostbacklog")],
99+
"cid-2": [normalize_file_path(r"d:\_Cpp_Digest\boostbacklog")],
100+
"cid-bad": [normalize_file_path(r"d:\_Cpp_Digest\team-brain")],
101+
}
102+
workspace_path_map = {
103+
normalize_file_path(r"d:\_cpp_digest\boostbacklog"): "boost-ws",
104+
normalize_file_path(r"d:\_cpp_digest\team-brain"): "team-ws",
105+
}
106+
107+
aliases = _infer_invalid_workspace_aliases(
108+
composer_rows=composer_rows,
109+
project_layouts_map=project_layouts_map,
110+
project_name_map={},
111+
workspace_path_map=workspace_path_map,
112+
workspace_entries=[],
113+
bubble_map={},
114+
composer_id_to_ws=composer_id_to_ws,
115+
invalid_workspace_ids={"invalid-ws"},
116+
)
117+
118+
self.assertEqual(aliases.get("invalid-ws"), "boost-ws")
119+
90120

91121
if __name__ == "__main__":
92122
unittest.main()

tests/test_parse_failure_logging.py

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import sqlite3
99
import sys
1010
import tempfile
11+
from contextlib import closing
1112

1213
import pytest
1314

@@ -116,6 +117,68 @@ def test_listing_logs_composer_schema_drift(caplog_at_warning: pytest.LogCapture
116117
)
117118

118119

120+
def test_workspace_tabs_logs_bubble_json_decode_failure(
121+
caplog_at_warning: pytest.LogCaptureFixture,
122+
) -> None:
123+
from flask import Flask
124+
125+
app = Flask(__name__)
126+
app.config["TESTING"] = True
127+
app.config["EXCLUSION_RULES"] = []
128+
129+
with tempfile.TemporaryDirectory() as tmp:
130+
ws_root = _seed_tabs_with_drifted_bubble(tmp)
131+
global_db = os.path.join(tmp, "globalStorage", "state.vscdb")
132+
with closing(sqlite3.connect(global_db)) as conn:
133+
conn.execute(
134+
"INSERT INTO cursorDiskKV ([key], value) VALUES (?, ?)",
135+
("bubbleId:cmp-ok:b-json", "{not valid json"),
136+
)
137+
conn.commit()
138+
with caplog_at_warning.at_level(logging.WARNING, logger="services.workspace_tabs"):
139+
with app.test_request_context("/api/workspaces/global/tabs"):
140+
payload, status = assemble_workspace_tabs("global", ws_root, rules=[])
141+
142+
assert status == 200
143+
messages = [r.getMessage() for r in caplog_at_warning.records]
144+
assert any("decode Bubble" in m and "b-json" in m for m in messages), (
145+
f"expected JSON decode warning for b-json, got: {messages}"
146+
)
147+
148+
149+
def test_workspace_tabs_logs_composer_json_decode_failure(
150+
caplog_at_warning: pytest.LogCaptureFixture,
151+
) -> None:
152+
from flask import Flask
153+
154+
app = Flask(__name__)
155+
app.config["TESTING"] = True
156+
app.config["EXCLUSION_RULES"] = []
157+
158+
with tempfile.TemporaryDirectory() as tmp:
159+
ws_root = _seed_tabs_with_drifted_bubble(tmp)
160+
global_db = os.path.join(tmp, "globalStorage", "state.vscdb")
161+
# Value must match composer_rows LIKE '%fullConversationHeadersOnly%' to reach parse.
162+
bad_composer_value = (
163+
'{"fullConversationHeadersOnly": [{"bubbleId": "b1"}], "createdAt":'
164+
)
165+
with closing(sqlite3.connect(global_db)) as conn:
166+
conn.execute(
167+
"INSERT INTO cursorDiskKV ([key], value) VALUES (?, ?)",
168+
("composerData:cmp-json", bad_composer_value),
169+
)
170+
conn.commit()
171+
with caplog_at_warning.at_level(logging.WARNING, logger="services.workspace_tabs"):
172+
with app.test_request_context("/api/workspaces/global/tabs"):
173+
payload, status = assemble_workspace_tabs("global", ws_root, rules=[])
174+
175+
assert status == 200
176+
messages = [r.getMessage() for r in caplog_at_warning.records]
177+
assert any("decode Composer" in m and "cmp-json" in m for m in messages), (
178+
f"expected JSON decode warning for cmp-json, got: {messages}"
179+
)
180+
181+
119182
def test_workspace_tabs_logs_bubble_schema_drift(caplog_at_warning: pytest.LogCaptureFixture) -> None:
120183
from flask import Flask
121184

0 commit comments

Comments
 (0)