sequenceDiagram
  participant Client
  participant ApiSearch as api.search
  participant ParseWarnings as ParseWarningCollector
  participant SearchSvc as services.search
  participant GlobalDB as globalStorage/state.vscdb
  participant WorkspaceDB as workspaceStorage/state.vscdb
  participant CliStore as cli_chats/store.db
  Client->>ApiSearch: HTTP GET /api/search?q=...
  ApiSearch->>ParseWarnings: create collector
  ApiSearch->>SearchSvc: search_global_storage(...) when type != "chat"
  SearchSvc->>GlobalDB: query composerData and bubbleId rows
  ApiSearch->>SearchSvc: search_legacy_workspaces(...)
  SearchSvc->>WorkspaceDB: query ItemTable chat payloads
  ApiSearch->>SearchSvc: search_cli_sessions(...) when type == "all"
  SearchSvc->>CliStore: traverse session blobs
  SearchSvc-->>ApiSearch: results lists
  ApiSearch->>SearchSvc: rank_results(merged)
  ApiSearch->>ParseWarnings: attach_to(payload)
  ApiSearch-->>Client: jsonify(results + warnings)

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Must fix

CI typecheck will fail — mypy . reports 20 errors (api/search.py:54, api/workspaces.py:69,83,99,140, api/composers.py:116,186,225, api/config_api.py:42,55,64,71,81,125,147,156, api/logs.py:152, api/export_api.py:75, services/cli_tabs.py:141, api/pdf.py:55/161) (Issue #100 requires zero mypy errors on annotated modules; jsonify() is typed as Any under strict mode, and FPDF lacks proper stubs — installing types-Flask alone does not clear the no-any-return errors.)

api/pdf.py:161 — pdf.output(buf) fails strict mypy with call-overload once fpdf stubs are present (Will surface as soon as the FPDF misc subclass error is addressed; needs dest="S" / bytes return or an explicit cast.)

Should fix

Issue #100 AC gap — no Conversation model (models/conversation.py defines Composer, not Conversation) (Acceptance criteria explicitly list Bubble, Workspace, Conversation, SearchResult; Composer covers the domain but the named type is missing — add a Conversation alias/TypedDict or document the mapping in the issue close comment.)

services/workspace_db.py:load_bubble_map vs services/search.py:_build_search_bubble_map — duplicate bubble KV loaders with different validation (G1/G3) (Search path validates via Bubble.from_dict and records ParseWarningCollector skips; load_bubble_map still stores raw dicts without schema checks — a drifted bubble can pass tabs/export but fail search, or vice versa.)

services/search.py:552-553 — search_cli_sessions logs top-level failures but does not call parse_warnings.record_source_failure (search_global_storage records source_failure for operator-visible incomplete results; CLI search failures remain invisible in the API warnings payload — amplifies the silent-failure compound.)

Three bubble shapes still not converged (models/conversation.py:Bubble raw-dict accessor vs utils/cli_chat_reader.py:236 messages_to_bubbles dict shape vs export flat keys) (Issue #100 implementation notes call for a single TypedDict or discriminated union; deferred work leaves shape drift uncaught at compile time for CLI/export paths.)

Nice to have

pyproject.toml dev extras — add types-Flask (and fpdf stubs if kept) alongside mypy (Improves local/CI parity and may reduce some stub friction once return casts are in place.)

models/conversation.py:43-48 — Composer.createdAt rejects ISO strings (Intentional and documented; if Cursor migrates timestamp format, composers vanish from list/search with only drift warnings — consider relaxing when real data appears.)

@CodeRabbit. Except my findings, Can you also review in more depth and find more additional bugs or issues?

#!/bin/bash
cat services/search.py

#!/bin/bash
cat models/search.py
cat models/parse_warnings.py

#!/bin/bash
cat utils/path_helpers.py
cat utils/exclusion_rules.py

#!/bin/bash
cat api/search.py

#!/bin/bash
cat services/workspace_db.py

#!/bin/bash
cat tests/test_search_helpers.py

#!/bin/bash
cat models/conversation.py

#!/bin/bash
cat utils/cli_chat_reader.py

@clean6378-max-it: Here are additional bugs and issues found beyond your list, ordered by severity.

🔴 Must Fix

1. workspaceFolder field returns inconsistent formats across the three search sources

services/search.py, three different call sites:

The field is named workspaceFolder throughout the codebase and SearchResult declares it str | None, so callers expect a consistent path. A front-end consuming merged rank_results output would get three different shapes for the same semantic field. The test in test_search_helpers.py line ~354 even asserts the display name "myrepo" for the global-storage case, which would pass, but hides the inconsistency at the integration level.

Root cause: search_global_storage (line ~297 in services/search.py) sets:

"workspaceFolder": ws_name,   # ← display name ("myrepo"), not path

whereas search_legacy_workspaces sets:

"workspaceFolder": workspace_folder,  # ← raw "file:///..." or "/home/..."

Either normalise all three to a resolved filesystem path (same as get_workspace_folder_paths produces) or to a bare display name—but pick one contract and document it in SearchResult.

2. search_global_storage re-scans all per-workspace DBs on every request (no cache)

services/search.py line ~234:

composer_id_to_ws = build_composer_id_to_workspace_id(
    workspace_path, workspace_entries
)

build_composer_id_to_workspace_id_cached exists in services/workspace_db.py for exactly this purpose (fingerprint + disk cache). The uncached variant opens and reads every {workspace_id}/state.vscdb on each GET /api/search, which is O(number-of-workspaces) extra DB opens for every keystroke in typeahead search scenarios. Replace with:

composer_id_to_ws = build_composer_id_to_workspace_id_cached(
    workspace_path, workspace_entries, rules
)

3. traverse_blobs and iter_sessions disagree on CLI session meta validity

utils/cli_chat_reader.py:

• iter_sessions → _read_meta uses a plain json.loads + cast, returning {} on any error.
• traverse_blobs → internally calls CliSessionMeta.from_dict, raising SchemaError on missing/wrong-typed fields, and returns [] on failure.

In search_cli_sessions:

meta = session.get("meta", {})             # from _read_meta — may succeed
...
messages = traverse_blobs(session["db_path"])  # may return [] for same session
if not bubbles:
    continue                                # session silently dropped

If CliSessionMeta.from_dict is stricter than _read_meta, a session can appear valid in iter_sessions (has name, createdAt, etc.) but be silently discarded by traverse_blobs. No warning is emitted. This should at minimum log a warning when traverse_blobs returns empty for a session whose meta decoded successfully—or _read_meta should use the same schema validation path.

🟡 Should Fix

4. search_global_storage serialises the entire raw composer dict into exclusion text

services/search.py line ~286:

metadata_parts=[
    ...
    _json_dump_safe(cd),           # ← entire raw Composer.raw dict
    "\n".join(bubble_meta),        # ← raw JSON of every bubble
],

cd is composer.raw which already contains conversationSummary, usage, requestMetadata, etc. Those are also serialised individually just above it. For a 1000-bubble conversation, this double-serialises every bubble's raw JSON into a single exclusion-rule string that can easily exceed several megabytes. This runs on every composer row on every search request. The _json_dump_safe(cd) entry should be removed since the individual parts already cover all the relevant metadata fields.

5. search_legacy_workspaces per-workspace exceptions lose the stack trace

services/search.py line ~357:

except Exception as exc:
    _logger.warning("Failed to search legacy workspace %s: %s", name, exc)

Missing exc_info=True (or _logger.exception(...)). Contrast with search_global_storage which correctly uses _logger.exception("Error searching global storage"). Without the stack trace, diagnosing workspace-specific parse failures is very difficult in production.

6. _extract_snippet produces a phantom snippet when query is the empty string

services/search.py:

idx = text.lower().find(query_lower)   # find("") always returns 0

str.find("") returns 0 for any non-empty string. With query = "", _extract_snippet would return the first ~200 chars of every bubble text as a "match". The /api/search handler already rejects blank queries with a 400, so the current blast radius is zero—but _extract_snippet is a public-ish helper (imported by tests) and this edge case is silent. Add a guard at the top:

if not query_lower:
    return ""

7. rank_results _ts silently accepts bool as a valid timestamp

services/search.py:

if isinstance(t, bool) or not isinstance(t, (int, float)):
    return 0.0

This correctly rejects booleans. But the upstream to_epoch_ms in utils/path_helpers.py does not guard booleans before the isinstance(value, (int, float)) check:

if isinstance(value, (int, float)):   # bool is a subclass of int
    if value > 1e12:
        return int(value)
    if value > 0:
        return int(value * 1000)   # True → 1000 ms

to_epoch_ms(True) silently returns 1000 (1 second since epoch). The guard that exists in Composer._optional_counter and Bubble.thinking_duration_ms (check isinstance(value, bool) first) should be replicated in to_epoch_ms.

8. search_legacy_workspaces timestamp falsy-coalescing drops 0 values correctly but the type contract is not enforced

"timestamp": tab.get("lastSendTime") or _UNKNOWN_SEARCH_TIMESTAMP,

If lastSendTime is a non-zero float (e.g. 1715000000.5), rank_results._ts receives a float but SearchResult.timestamp is typed int | str. Mypy won't catch this at the dict-literal level because the value comes from tab.get(...) which returns Any. The timestamp field in SearchResult should be widened to int | float | str, or to_epoch_ms should be applied here just as in the other two search functions.

🔵 Nice to Have

9. test_search_helpers.py imports private helpers (_extract_snippet, _find_match)

These are single-underscore-prefixed functions. Testing them via direct import creates tight coupling to implementation details. They should either be made public (renamed without leading underscore) if they are part of the module's tested surface, or the tests should be updated to verify observable behaviour through the public functions (search_global_storage, etc.).

10. ParseWarningCollector has no coverage path for legacy or CLI per-item parse failures

search_global_storage records record_bubble_skipped / record_composer_skipped / record_composer_processing_failure. search_legacy_workspaces and search_cli_sessions do not accept a ParseWarningCollector parameter at all. Malformed legacy chat JSON or CLI blob parse errors are logged server-side but are never surfaced to the API caller via the warnings field. This is a smaller version of the "CLI source failure" gap already in your list.

(ᐧωᐧ)🐇

Verified each CodeRabbit finding against current code. Summary:

🔴 Must fix

🟡 Should fix

🔵 Nice to have — skipped

Validation

mypy .     → Success (86 source files)
pytest     → 450 passed, 4 skipped

services/workspace_db.py:load_bubble_map vs services/search.py:_build_search_bubble_map — duplicate bubble KV loaders with different validation (G1/G3) (Search path validates via Bubble.from_dict and records ParseWarningCollector skips; load_bubble_map still stores raw dicts without schema checks — a drifted bubble can pass tabs/export but fail search, or vice versa.)

Three bubble shapes still not converged (models/conversation.py:Bubble raw-dict accessor vs utils/cli_chat_reader.py:236 messages_to_bubbles dict shape vs export flat keys) (Issue #100 implementation notes call for a single TypedDict or discriminated union; deferred work leaves shape drift uncaught at compile time for CLI/export paths.)

not addressed yet

@coderabbitai check @clean6378-max-it 's findings are fully addressed now.

#!/bin/bash
echo "=== services/workspace_db.py: load_bubble_map ==="
rg -n "load_bubble_map" services/workspace_db.py -A 20

echo ""
echo "=== services/search.py: _build_search_bubble_map or bubble loading ==="
rg -n "_build_search_bubble_map\|bubble_map\|load_bubble_map" services/search.py -B2 -A5

echo ""
echo "=== Bubble shapes: models/conversation.py Bubble class ==="
rg -n "class Bubble" models/conversation.py -A 30

echo ""
echo "=== utils/cli_chat_reader.py messages_to_bubbles ==="
rg -n "def messages_to_bubbles" utils/cli_chat_reader.py -A 30

echo ""
echo "=== Conversation model/alias in models/ ==="
rg -rn "Conversation" models/ -B1 -A2

echo ""
echo "=== search_cli_sessions: record_source_failure ==="
rg -n "record_source_failure\|parse_warnings" services/search.py -B2 -A3

echo ""
echo "=== mypy strict errors: check api/search.py, api/workspaces.py, etc. ==="
rg -n "def search\|def list_workspaces\|def get_workspace\|def list_composers\|def detect_environment\|def get_logs\|def export_chats\|def generate_pdf" api/search.py api/workspaces.py api/composers.py api/config_api.py api/logs.py api/export_api.py api/pdf.py 2>/dev/null

echo ""
echo "=== api/pdf.py: output call and return type ==="
rg -n "output\|return\|cast" api/pdf.py -B1 -A2

echo ""
echo "=== ParseWarningCollector: record_source_failure usage in search.py ==="
grep -n "record_source_failure" services/search.py

#!/bin/bash
echo "=== api/pdf.py full content ==="
cat api/pdf.py

echo ""
echo "=== models/__init__.py full content ==="
cat models/__init__.py

echo ""
echo "=== models/search.py full content ==="
cat models/search.py

echo ""
echo "=== models/conversation.py: Conversation alias lines ==="
grep -n "^n\b\|^Conversation\b\|# is the\|^n =\|ConversationSummary\|ConvSummary" models/conversation.py

echo ""
echo "=== services/search.py: bubble loading section (lines 200-250) ==="
sed -n '200,260p' services/search.py

echo ""
echo "=== services/search.py: CLI section around line 537 ==="
sed -n '520,560p' services/search.py

@bradjin8: Confirmed — all of @clean6378-max-it's findings are fully addressed. Here's the breakdown:

✅ Must Fix

✅ Should Fix

Additionally, the workspaceFolder inconsistency (CodeRabbit finding #1) was resolved: SearchResult.workspaceFolder is documented as # display-name leaf (e.g. "myrepo"), not a file path, and all three search sources are normalised to match.