db: reuse prepared statements via a per-connection cache - #61
Merged
Conversation
Nearly every SQL access called db.prepare() per invocation, so SQLite re-parsed and re-planned each statement every time it ran. Route the hot helpers in queries.ts and analysis-queries.ts through a new prep() helper that returns a cached, connection-bound Statement. The cache is a WeakMap keyed by the Database instance, giving each connection its own Map<sql, Statement>: better-sqlite3 statements are bound to the connection they were prepared on, so a module-global map would hand a statement from a closed database to the next (test) case. Population is lazy and every caller runs after migrate(), so a cached statement can never capture a pre-migration query plan. No behaviour change — a hygiene change, not a speedup. Tests unchanged and passing; a component test documents the reuse + per-connection guarantees. Co-authored-by: frost-falcon-24 <frost-falcon-24@pi-agent.local>
db_loadMessages builds a static SELECT over two adjacent string literals (stable SQL text), yet called db.prepare() per session — the analyzer's hot read path. Route it through prep(db, ...) like queries.ts and analysis-queries.ts. Confirmed the SQL is not dynamic, so caching on the SQL text is sound. Co-authored-by: frost-falcon-24 <frost-falcon-24@pi-agent.local>
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.
What this does (plain English)
pi-prospector talks to a small local database that stores your analysis. Every time it wanted to ask the database a question, it would re-teach the database the full wording of that question from scratch — thousands of times in a single run, even for the same question repeated over and over. Repeating that teaching step (SQLite calls it "preparing" a statement) is busier than it looks.
This change lets the program remember each question it has already taught, so repeated questions reuse the remembered version instead of re-teaching it every time. It is pure housekeeping: the answers it gets are exactly the same — it just stops doing the same setup work repeatedly.
Technical detail
Nearly every db access went through
db.prepare(), so SQLite re-parsed and re-planned each statement on every call (db.cacheis undefined in better-sqlite3). This adds a per-connection prepared-statement cache and routes the hot helpers through it:src/db/statement-cache.ts— aWeakMap<Database, Map<sql, Statement>>. The cache is keyed by theDatabaseinstance because a better-sqlite3Statementis bound to the connection it was prepared on; a module-global SQL→Statement map would hand a statement from a closed temp database to the next test.prepare(db, sql)returns the cached statement for that connection, preparing once and reusing afterwards. Only connections that have runmigrate()(and therefore carry the final schema) ever store; a raw connection that skippedmigrate()still executes but caches nothing — so it is purely an optimisation and never changes behaviour.initializeStatementCache(db)is called as the final step ofmigrate(), tying cache lifetime to the connection after schema migration. A statement is never compiled against pre-migration schema.queries.ts,analysis-queries.ts, and the framework's hotdb_loadMessages(the sanity check the issue asks for).Correctness / no-behaviour-change verification
npm test: 441 passed, 0 failed (unit + component, mock LLM).test/integration/test-commands.ts): 21 passed, 0 failed — full sync→analyze→proposal flow, idempotent re-runs, revise-mode lineage, proposal lifecycle,prospect verify.tsc --noEmitclean.tests/component/statement-cache.test.tscovers: same statement reused for identical SQL on a connection; statements not shared across connections (a closed connection's cached statement is never handed to another); uninitialized connections still execute but don't cache;migrate()initializes the cache.Measurement (exactly what and against what)
Measured on this branch's code vs raw per-call
db.prepareon the same connection, fresh scratch SQLite DB under/tmp, 200,000 bound lookups of a simple indexed query:db.prepare: ~1910 ms → ~1.2×.This compares the statement-reuse effect directly against
main's per-call behaviour on the same connection, so it does not double-count #55 (that PR batched node+edge writes into a singledb.transaction(), which is orthogonal). It is not a headline speedup: the whole write path is a fraction of a percent of wall time on a real corpus, and this change is framed as hygiene, not as a wall-time target.— quiet-sloth-61