Paginate chunk search with keyset cursors instead of OFFSET - #373
Merged
Merged
Conversation
The chunk scan `sec ask` runs paged the table with `LIMIT`/`OFFSET`. That bounds memory, which is what it was for, but `OFFSET n` makes SQLite walk and discard the first n rows of every page, so reading the table a page at a time costs O(rows²) — at 400k chunks, measurably slower than the unbounded read it replaced, and every question pays it. The scan is already ordered by the primary key, so the next page is a seek into the index SQLite keeps for it rather than a count of rows to throw away. `getPage` expresses exactly that and pushes the predicate down, so the page is `WHERE chunk_id > ? ORDER BY chunk_id LIMIT ?` and the plan goes from `SCAN kb_chunk USING INDEX` to `SEARCH kb_chunk USING INDEX (chunk_id>?)`. The memory bound is unchanged: still one page of rows and a list held at topK. Measured over a real SQLite kb_chunk, 64-dimension vectors, 512-row pages: | rows | OFFSET | keyset | unbounded read | |---|---|---|---| | 50k | 358 ms | 307 ms | 277 ms | | 100k | 841 ms | 515 ms | 546 ms | | 200k | 2,209 ms | 1,024 ms | 1,359 ms | | 400k | 7,042 ms | 2,062 ms | 2,133 ms | The loop ends on an empty page as well as on an absent cursor: a table whose size is an exact multiple of the page hands back a cursor for its last full page, and following it alone never terminates. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UP2rTRUU3HjFz5yvyT3dXp
… count nobody reads
Two full scans of `filing_document` on every `sec ask`, which pre-indexes
before it answers.
The selection reads the table newest first and takes a page of it, and
there was no index in that order — so SQLite read every row and sorted the
lot into a temp B-tree to hand back the first 26. Declaring
`(filing_date, accession_number)` alongside the two indexes already there
matches the ORDER BY exactly, so the index is walked backwards instead. On
300k rows:
SCAN d 139.7 ms
USE TEMP B-TREE FOR ORDER BY
SCAN d USING INDEX
filing_document_filing_date_accession_number 0.2 ms
`countAlreadyIndexed` is the other scan: a COUNT over the same table joined
to the knowledge base, ~139 ms on the same corpus and unaffected by any
index, since an unbounded aggregate has to visit every matching row. `sec
index` prints the number; `sec ask` never does, and paid for it anyway. It
is now asked for rather than assumed, and `skipped` is left absent when a
run did not count — absent rather than zero, because zero says the index
holds nothing in scope.
An existing database picks the index up from `sec db setup`, which emits
every declared index as `CREATE INDEX IF NOT EXISTS` on every run.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UP2rTRUU3HjFz5yvyT3dXp
…nows `toVector`'s `Float32Array`, and the `vector` / `metadata` property names the scan reads, are all resolved from the schema by the base class — from private fields with no accessor, which is the only reason they are restated here. They are right for the one store this class is constructed for and silently wrong for any other, so a reader deciding whether to reuse it needs that said out loud rather than inferred from the fact that it works. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UP2rTRUU3HjFz5yvyT3dXp
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
2 participants
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.
Summary
Replaces
OFFSET-based pagination in the knowledge base's chunk vector search with keyset (seek) pagination. This eliminates O(rows²) complexity when scanning large indexes, making similarity search linear again while keeping the working set bounded.Key Changes
PagedChunkVectorStorage: Switched from
OFFSET npagination to cursor-based pagination usinggetPage()withPageCursor. The cursor resumes from the lastchunk_idseen rather than counting rows, so each row is read exactly once during a full scan.Test coverage: Added three new test cases to verify pagination behavior:
OFFSETIndex optimization: Added
["filing_date", "accession_number"]index tofiling_documenttable instorageRegistry.ts. This matches theORDER BYin the index selection query exactly, allowing SQLite to walk the index backwards instead of sorting the entire table into a temp B-tree.Task output refinement: Made
skippedcount optional inIndexFilingSectionsTaskOutput. The count requires a join over every converted filing, which is expensive when the caller won't display it. AddedcountSkippedinput flag (defaults to true for backward compatibility) sosec askcan skip this count on pre-indexing runs.CLI adjustment: Updated
ask.tsto handle optionalskippedvalue and passcountSkipped: falsewhen pre-indexing before answering questions.Implementation Details
The pagination change addresses a critical performance issue:
OFFSET nforces SQLite to walk and discard the first n rows on every page, making a large corpus slower to read a page at a time than to read whole. Keyset pagination resumes from the last primary key value, which SQLite already indexes, so the scan becomes linear O(rows) instead of quadratic.The termination logic handles the edge case where a table's size is an exact multiple of the page size: the last full page still returns a cursor, but the next page is empty. The loop checks both
nextCursor === undefinedanditems.length === 0to avoid infinite loops.The
filing_documentindex addition is a complementary optimization: the index selection query reads newest filings first, and the new index's column order matches thatORDER BYexactly, eliminating the need for a temporary sort.https://claude.ai/code/session_01UP2rTRUU3HjFz5yvyT3dXp