perf(read-path): O(1) stats, SQLite retrieval adapter, bounded source-file fetch (#1657)

[trek-e]

Closes #1657.

Read-path operations were O(N) in palace size and slow on large palaces (mempalace status took minutes on ~400K drawers). Three cohesive read-path deepenings, plus the two fixes from an adversarial review.

1. Metadata aggregation seam

• New primitives on BaseCollection: count_by, crosstab, count_matching (full-scan defaults so every backend works), with SQL GROUP BY / COUNT(*) overrides in ChromaCollection.
• The SQL runs over the same embeddings ⋈ segments ⋈ collections join that repair.sqlite_drawer_count already trusts as the ground-truth drawer count, so grouped counts sum back to collection.count(). LEFT JOIN on embedding_metadata plus a typed-column COALESCE keeps byte-for-byte parity with the prior Python scan (including missing keys and int/float/bool-valued keys).
• New palace_stats module turns those primitives into the wing/room/taxonomy shapes. tool_status / tool_list_wings / tool_list_rooms / tool_get_taxonomy and miner.status now aggregate in one query.
• Deletes the dead full-scan path (_fetch_all_metadata, _get_cached_metadata, the _metadata_cache globals and their write-tool invalidations). Minutes → ms.

2. Verbatim-over-SQLite retrieval adapter

• Extracted the 237-line BM25/SQLite fallback into verbatim_sqlite.SqliteExactRetriever, which owns all chroma.sqlite3 schema knowledge behind a small search(...) interface and is testable in isolation. searcher keeps a thin delegating shim, so existing call sites/tests are unchanged.

3. Bounded source-file access

• New source_file_access module: count_drawers via COUNT(*) (no documents materialized) and a capped fetch_drawers, replacing two unbounded get(where=source_file) calls on the per-query path.

Adversarial-review fixes

• [high] Closet-boost hydration (now _enrich_closet_hits) counts first and skips the grep-overwrite for files larger than the fetch cap — a capped, unordered subset could otherwise omit the matched chunk and overwrite the hit with an unrelated early one. Oversized hits keep their own correct text and report the accurate COUNT(*) total.
• [medium] SQL aggregates COALESCE the typed metadata columns so int/float/bool keys keep parity with the Python scan instead of collapsing into the missing-key bucket.

Testing

• New tests/test_palace_stats.py: parity oracle (SQL == Python scan) for string, int, crosstab, and missing-key metadata; bounded source-file access; palace_path=None fallback.
• New oversized-source integration test in tests/test_closets.py; direct SqliteExactRetriever interface test in tests/test_hnsw_capacity.py.
• Full suite: 2289 passed, 3 skipped; ruff check + ruff format --check clean.

No new dependencies. Default behavior unchanged; only performance and internal structure improve.

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request optimizes the read-path aggregation by shifting metadata tallying and taxonomy counting from slow Python-side pagination to efficient SQL GROUP BY queries directly on the SQLite database. It introduces dedicated modules for statistics generation (palace_stats), bounded source-file access (source_file_access), and verbatim BM25 retrieval (verbatim_sqlite), which allows removing the old in-memory metadata cache. The review feedback identifies a potential cross-platform issue where constructing SQLite file URIs using raw f-strings can fail on Windows or with paths containing spaces, and suggests using pathlib.Path.as_uri() for robust URI formatting.

[trek-e]

Revived this PR — rebased, conflict-free, and green. Ready for another look. 🙏

This had gone stale (113 commits behind develop, CONFLICTING). I've merged develop in and resolved the conflicts so both sides survive:

• Kept this PR's deepenings: the SQL-aggregation seam (count_by/crosstab/count_matching), palace_stats, source_file_access, and verbatim_sqlite.SqliteExactRetriever.
• Kept develop's hardening: the corruption-resilience work and the new non-chroma backend paths (sqlite_exact / qdrant / pgvector).
• Reconciled the two independent status rewrites. develop and this PR each added a SQL-aggregate status path; they turned out to be complementary. _sqlite_wing_room_counts() reads chroma.sqlite3 by path and never opens the collection → status avoids cold-loading the HNSW index (~60s on large palaces, status cold-loads the HNSW vector index just to count drawers (~60s/call on large palaces) #1681); palace_stats.taxonomy(col) is the general seam for callers already holding a collection. miner.status now uses the former as the primary fast path and the latter as the fallback. (An earlier resolution had deleted _sqlite_wing_room_counts as "superseded" — that broke miner.py and 11 status tests; it's restored.)

✅ All 8 CI checks green (build, build-gpu, test-linux 3.9/3.11/3.13, test-macos, test-windows, lint). Full local suite: 2816 passed, 5 skipped; ruff clean. The earlier as_uri() review comments were already addressed in bcd745d.

A follow-up is planned as a separate PR (deliberately not added here, to keep this one focused and landable): an O(N) streaming scan() primitive for the full-corpus iteration paths (rebuild / dedup / migrate / wake-up) that still use offset pagination — building on the SqliteExactRetriever pattern this PR introduces.

[messelink]

@trek-e — heads-up: we've filed #2038 for pgvector's facet_counts implementation (taking over from #1824 which stalled after a CHANGES_REQUESTED review 30 days ago).

Different reason for filing separately here vs. that one: our PR is scope-disjoint from yours rather than a take-over. You're adding the SQLite/chroma fast path plus the count_by / crosstab / count_matching primitives; ours adds the pgvector fast path aligned to the merged #1868 facet_counts(field, where, limit) contract. The two shouldn't collide file-wise (different backends).

There is an underlying naming question that will presumably want to settle at some point: count_by(field) from this PR overlaps functionally with the merged facet_counts(field, ...) from #1868. Not for us to decide — flagging it so it's visible on both threads. Our PR just implements the pgvector side of facet_counts because that's the merged contract today. If count_by ultimately wins, mapping our implementation over shouldn't be much work.

[igorls]

Thanks for this contribution, and apologies for the slow turnaround.

develop has moved a fair way since this was opened and the branch no longer merges cleanly. If you're still interested in landing it, could you rebase onto current develop? Once it merges cleanly and CI is green I'll get it reviewed for the 3.8.0 cycle.

If you'd rather not pick it back up, no problem at all — just say so and I'll close it out, and thanks either way for taking the time to send it.

[igorls]

Thank you @trek-e — both for this PR and for #1657. The diagnosis (overview reads were O(N) because SQL aggregation only ran when the index was broken) was right, and it is part of why the later read-path work happened at all.

We are going to close this as superseded, not rejected. The stats path landed on develop by another route:

• sqlite GROUP BY for status / list_wings / list_rooms / taxonomy (_sqlite_wing_room_counts, facet_counts)
• sqlite_exact locus columns for wing/room/hall
• chroma sqlite listing so list_drawers / find_tunnels / traverse do not open HNSW

palace_stats.py, verbatim_sqlite.py, and source_file_access.py never made it onto develop. count_by / crosstab would also name-clash with facet_counts (as messelink noted here). We are not asking you to rebase this 1.1k-line branch — it would fight what is already on develop.

What is still left from #1657 (item 1 only):

The per-query total_drawers count in searcher.py still does an unbounded get(where=...) of metadatas per hit. A follow-up must preserve parent_drawer_id scoping (_scoped_source_filter, from the #1580 fix). A naive file-global COUNT(*) on source_file would silently reopen #1580. That needs a count_matching-style backend method that accepts the $and where-shape, on every backend that implements lexical_search — not a small scrap.

If you want that follow-up, please open a new PR against current develop (not this branch). No deadline and no expectation that you take it. If you do not, we will open a dedicated issue under #1657 and do it here.

#2296 (count_by/crosstab O(N) on Qdrant/pgvector) is a follow-up to this PR's palace_stats API. Current develop already has facet_counts on those backends and has no count_by, so #2296 is not applicable unless palace_stats is reintroduced. We will close it alongside this PR.

Thank you again.