fix(chroma): read list_drawers and tunnels from sqlite, skip HNSW - #2314
Merged
Conversation
On the i7-8750H chroma palace, list_drawers(limit=20) took 36s and find_tunnels died at 30s because both paged col.get() and cold-loaded the vector index. chroma.sqlite3 already has embedding_metadata. - sqlite_list_id_metadata: ids + metadata + chroma:document without HNSW - sqlite_room_wing_hall_counts shared with graph_stats - build_graph uses that grouped read when no collection is injected - find_tunnels/traverse no longer open the collection first MCP tests assert the client paging path is not used.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…o sql Follow-up on the same palace. The metadata scan joined every embedding_metadata row, and chroma:document lives in that table — so list_drawers pulled the palace's entire verbatim text into memory to render 20 previews, then filtered wing/room in Python after scanning the whole collection. Measured on the 1.7 GB / 165k-drawer chroma palace, list_drawers(wing=..., limit=20): before 2.31 s, 1148 MB peak RSS after 0.01 s, 86 MB peak RSS Unfiltered goes 1.61 s / 1162 MB -> 1.57 s / 894 MB. - exclude chroma:document from the scan; hydrate the displayed page via sqlite_documents_for_ids (two indexed steps — embedding_id is only indexed under UNIQUE(segment_id, embedding_id), so a single join on it scans embedding_metadata: 5.7 s for one page) - push the wing/room equality into SQL as a join per key - decode cells through _metadata_cell_value with column probing, so bool_value is not silently dropped on newer chroma schemas - grouped counts carry MAX(date), restoring find_tunnels' "recent", which the sqlite path had blanked (both backends) - resolve the graph's sqlite reader from the configured backend instead of sniffing the palace dir, so a two-backend directory still raises BackendMismatchError instead of being silently picked - find_tunnels/traverse fall back to the collection when sqlite cannot serve, so a missing palace reports "Chroma database missing" again rather than [] and "Room not found" Tests: sql tripwires for the scan (no documents, filter pushed down, id-scoped preview read), recent-from-sqlite, missing-palace diagnostics, and backend-gating for the graph reader.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Yes — chroma can take the same shortcut sqlite_exact did: read loci from
chroma.sqlite3, do not pagecol.get()/ HNSW.Measured on
igorls-blade-15(i7-8750H, chroma 1.7 GB, 165k drawers, develop 3.7.1):list_drawersno filter, limit=20find_tunnelsgraph_statsChanges
chroma.sqlite_list_id_metadata— ids + metadata +chroma:documentfromembedding_metadata, no vector indexchroma.sqlite_room_wing_hall_counts— shared grouped read for graph_stats andbuild_graphbuild_graph()uses that path when MCP does not inject a collection (tests that passcol=keep paging)find_tunnels/traverseno longer open the collection firstlist_drawersand tunnels become sqlite scans; search still uses HNSW (that's the right tool).Tests:
TestReadToolsgraph_stats / find_tunnels / list_drawers sqlite tripwires +test_palace_graph.py.