perf: batch message inserts in a per-session transaction during sync - #63
Merged
Conversation
Message inserts during sync currently autocommit one row at a time. Wrap each session's insert loop, cursor advance, and message count in a single transaction so a session's messages commit as one unit. The cursor update is inside the same transaction on purpose: if a session fails partway, SQLite rolls rows and the cursor back together, so a resync reprocesses the file from the old cursor instead of skipping rows that were never committed. Co-authored-by: crystal-marlin-69 <crystal-marlin-69@pi-agent.local>
Two component tests prove the change's contract under partial failure, using a gated RAISE(ABORT) trigger as a deterministic write failure (the sync path's parse + INSERT OR IGNORE are otherwise fully defensive. 1. A session that fails mid-sync leaves zero rows — its session row, messages, and cursor all roll back together. 2. Appending a failing row after a successful commit does not advance the resume cursor past the rolled-back rows; removing the failure and re-syncing imports them. Both were falsified: reverting to per-row autocommit, or letting the cursor commit despite a rollback, makes each test fail. Co-authored-by: crystal-marlin-69 <crystal-marlin-69@pi-agent.local> EOF )
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
When the syncer imports a session's chat history into the database, it used to write each message as its own little "commit". Now it writes all of one session's messages as a single unit — one commit per session instead of one per message. Same data on disk, just written much more cheaply.
Why it's safe (the important part)
The one thing that can silently go wrong with batching is the resume cursor (the marker that says "this session was imported up to line N"). If a session is interrupted halfway, the old code left the partial messages and left the cursor far ahead — so the next run would skip past messages that were never written.
This change puts the cursor advance inside the same transaction as the messages. If a session fails partway, the database rolls back both the messages and the cursor together — nothing partial is left, and the next run re-imports cleanly from where it left off. Committed rows are never skipped.
Plain-English acceptance check
Tests (new — this is behaviour under partial failure)
Two component tests in
tests/component/sync-transaction.test.tsexercise the rollback path directly, using a gatedRAISE(ABORT)trigger as a deterministic write failure (the sync path's parsing andINSERT OR IGNOREare otherwise fully defensive, so a real session file can't naturally throw):Both were falsified: reverting to per-row autocommit, or letting the cursor update commit despite a rollback, makes each test fail — so they are not vacuous and genuinely protect the invariant.
Technical detail
Both sync paths (
syncPiSession,syncClaudeSession) now wrap the per-session work — session upsert, the message-insert loop,updateCursor, and the message count — in adb.transaction(...). In WAL mode each autocommit is a commit cycle, so this replaces one commit per row with one commit per session. The FTS trigger (messages_ai) still fires per row, so its cost is unchanged, which is exactly why this win is smaller than the ~5.7× seen batching analysis-node writes (no trigger attached).Measurement
Synthetic corpus matching the issue (60 sessions, 2,160 messages) on a fresh scratch SQLite DB under
/tmp, no LLM calls,node --import tsx, wall time of the fullrunSync, 3 runs each:main, per-row autocommit)~2.5× end-to-end. The issue's micro-benchmark baseline (115 ms) matches mine (116.5 ms), confirming the sync insert path had not already been batched by #55 (which was the analysis-node path), so this is not a double-count.
Out of scope, found while testing
While writing the resume-cursor test I confirmed a pre-existing cursor miscount on incremental re-sync, present in unchanged
main:last_lineis set tolines.length(the split count), which includes the trailing empty element from a final newline. Appending new lines to an already-synced session therefore skips the first appended line. This affects the untouched autocommit code identically and is independent of this change, so it is not fixed here — flagged for a follow-up.— crystal-marlin-69