Skip to content

Commit aa9cb7c

Browse files
committed
Fix: reviewer's findings
1 parent f6d3ff9 commit aa9cb7c

3 files changed

Lines changed: 90 additions & 20 deletions

File tree

services/workspace_tabs.py

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -689,26 +689,27 @@ def _safe_fetchall(query: str, params: tuple = ()) -> list:
689689
cd = composer.raw
690690

691691
# Verify the conversation belongs to the requested workspace.
692-
# Use MRC-based project layouts (no bubble scan) for the resolver.
693-
project_layouts_map: dict[str, list] = load_project_layouts_map(global_db)
694-
695-
# Build aliases for workspace resolution (empty bubble_map — summary path)
696-
composer_rows_for_aliases = _safe_fetchall(
697-
"SELECT key, value FROM cursorDiskKV WHERE key LIKE 'composerData:%'"
698-
" AND value IS NOT NULL"
699-
" AND value LIKE '%fullConversationHeadersOnly%'"
700-
" AND value NOT LIKE '%fullConversationHeadersOnly\":[]%'"
701-
)
702-
invalid_workspace_aliases = infer_invalid_workspace_aliases(
703-
composer_rows=composer_rows_for_aliases,
704-
project_layouts_map=project_layouts_map,
705-
project_name_map=project_name_map,
706-
workspace_path_map=workspace_path_map,
707-
workspace_entries=workspace_entries,
708-
bubble_map={},
709-
composer_id_to_ws=composer_id_to_ws,
710-
invalid_workspace_ids=invalid_workspace_ids,
711-
)
692+
# Scoped MRC load for this composer only; full map + alias scan only
693+
# when invalid workspace folders need majority-vote reassignment.
694+
project_layouts_map: dict[str, list] = {}
695+
invalid_workspace_aliases: dict[str, str] = {}
696+
if invalid_workspace_ids:
697+
project_layouts_map = load_project_layouts_map(global_db)
698+
composer_rows_for_aliases = _safe_fetchall(COMPOSER_ROWS_WITH_HEADERS_SQL)
699+
invalid_workspace_aliases = infer_invalid_workspace_aliases(
700+
composer_rows=composer_rows_for_aliases,
701+
project_layouts_map=project_layouts_map,
702+
project_name_map=project_name_map,
703+
workspace_path_map=workspace_path_map,
704+
workspace_entries=workspace_entries,
705+
bubble_map={},
706+
composer_id_to_ws=composer_id_to_ws,
707+
invalid_workspace_ids=invalid_workspace_ids,
708+
)
709+
else:
710+
project_layouts_map[composer_id] = load_project_layouts_for_composer(
711+
global_db, composer_id,
712+
)
712713

713714
pid = determine_project_for_conversation(
714715
cd, composer_id, project_layouts_map,

tests/test_api_endpoints.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,52 @@ def test_missing_global_storage_returns_404(self, empty_workspace_client):
102102
body = response.get_json()
103103
assert "error" in body
104104

105+
def test_summary_query_returns_tab_list_without_bubbles(self, client):
106+
response = client.get(f"/api/workspaces/{HAPPY_WORKSPACE_ID}/tabs?summary=1")
107+
assert response.status_code == 200
108+
body = response.get_json()
109+
assert "tabs" in body and isinstance(body["tabs"], list)
110+
assert body["tabs"], "expected at least one summary tab"
111+
tab = next(t for t in body["tabs"] if t["id"] == HAPPY_COMPOSER_ID)
112+
assert "title" in tab
113+
assert "timestamp" in tab and isinstance(tab["timestamp"], int)
114+
assert "messageCount" in tab and isinstance(tab["messageCount"], int)
115+
assert "bubbles" not in tab
116+
117+
def test_summary_query_global_workspace(self, client):
118+
response = client.get("/api/workspaces/global/tabs?summary=1")
119+
assert response.status_code == 200
120+
body = response.get_json()
121+
assert "tabs" in body and isinstance(body["tabs"], list)
122+
123+
124+
class TestGetWorkspaceTab:
125+
def test_happy_path_returns_single_tab(self, client):
126+
response = client.get(
127+
f"/api/workspaces/{HAPPY_WORKSPACE_ID}/tabs/{HAPPY_COMPOSER_ID}"
128+
)
129+
assert response.status_code == 200
130+
body = response.get_json()
131+
assert "tab" in body
132+
tab = body["tab"]
133+
assert tab["id"] == HAPPY_COMPOSER_ID
134+
assert "title" in tab
135+
assert "timestamp" in tab and isinstance(tab["timestamp"], int)
136+
assert "bubbles" in tab and isinstance(tab["bubbles"], list)
137+
assert "codeBlockDiffs" in tab
138+
139+
def test_unknown_composer_returns_404(self, client):
140+
response = client.get(
141+
f"/api/workspaces/{HAPPY_WORKSPACE_ID}/tabs/no-such-composer"
142+
)
143+
assert response.status_code == 404
144+
assert "error" in response.get_json()
145+
146+
def test_cli_workspace_returns_400(self, client):
147+
response = client.get("/api/workspaces/cli:proj-1/tabs/cmp-happy")
148+
assert response.status_code == 400
149+
assert "error" in response.get_json()
150+
105151

106152
# ---------------------------------------------------------------------------
107153
# GET /api/search?q=...

tests/test_workspace_tabs_summary.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,29 @@ def test_scoped_bubble_query_only(self):
203203
msg=f"assemble_single_tab ran a non-scoped bubble scan:\n{global_bubble_scans}",
204204
)
205205

206+
def test_scoped_mrc_load_no_invalid_workspaces(self):
207+
"""Without invalid workspace folders, per-tab load must not full-scan MRC or composers."""
208+
(_, _), queries = _collect_queries(
209+
self.ws_path,
210+
lambda p: assemble_single_tab("global", COMPOSER_ID, p, rules=[]),
211+
)
212+
mrc_scans = [
213+
q for q in queries
214+
if "messageRequestContext:%" in q
215+
and f"messageRequestContext:{COMPOSER_ID}:%" not in q
216+
]
217+
self.assertEqual(
218+
mrc_scans,
219+
[],
220+
msg=f"assemble_single_tab ran a global MRC scan:\n{mrc_scans}",
221+
)
222+
composer_scans = [q for q in queries if "composerData:%" in q]
223+
self.assertEqual(
224+
composer_scans,
225+
[],
226+
msg=f"assemble_single_tab scanned all composers for aliases:\n{composer_scans}",
227+
)
228+
206229
def test_single_tab_structure(self):
207230
payload, status = assemble_single_tab("global", COMPOSER_ID, self.ws_path, rules=[])
208231
self.assertEqual(status, 200)

0 commit comments

Comments
 (0)