Keep v2 reads off whole-table scans found by a SQL plan audit

[MaxGhenis]

Summary

#210 found two inbox reads doing SCAN inbox. This audits every SQL statement in internal/storage/sqlite, plus the v2 read paths that drive them (internal/v2read, /api/status, the MCP tools), for plans that read a whole table that grows with history, and fixes the ones on hot or growing paths.

Where something was found:

• Partial indexes. Every one was checked against the queries that should use it; table below.
• The biggest cost was not a single statement. /api/status freshness walked every message every 30 s: 1,679 statements, 0.8 s on today's store, 19 s at 30×. Its keyset cursor also made the walk quadratic within a thread.
• The rest: an unindexed time listing behind MCP get_messages, a sender filter that walked a whole index, a conversation list loaded entirely into Go, and three rebind lookups whose default-statistics join order started from the account side.

Method

• Inventory. 209 runtime statements, from per-file agents plus cross-checks for hidden FK child scans, partial-index use, caller hotness, and v2-store SQL outside the package. The only out-of-package SQL is internal/migration/validate.go, which runs once per openmessage migrate.
• Measurement. A throwaway in-package harness ran EXPLAIN QUERY PLAN and timed each statement through the repo's modernc driver (SQLite 3.51.2) on copies of the live v2 store. The live store was never opened.
  • 1×: 72k messages.
  • 30×: 30× the conversations; whole-table reads grow 30-fold.
  • deep 10×: the same conversations with 10× the history; per-thread work grows 10-fold.
  • Each store was also measured after ANALYZE. The app never runs ANALYZE, so default-statistics plans are the ones that matter, but the fixes hold either way.
• Full rerun after the fix. The migration was applied through the real Open(), then all 214 statements were rerun on six stores. That catches a new index changing some other statement's plan.

Timings were taken on a heavily loaded machine (load average 40–170). Read the ratios, not the absolute values.

Partial indexes

index	verdict
inbox_dedupe_uq	AppendInbox's lookup can't use it (dedupe_key = ? does not imply <> ''): SCAN inbox per duplicate frame. Fixed by #210.
messages_sender_time_idx	Implied, but the sender search joined identities and filtered on i.canonical_value. With no index leading on that column, the planner walked the whole index. Fixed here.
inbox_unprocessed_idx, message_attachments_blob_hash_idx	Implied; used.
outbox_expired_lease_idx	Implied (literal state). The planner picks outbox_due_idx (state=?), which reads the same in-flight rows; with ANALYZE it picks the partial index. Fine either way.
devices_current_local_uq	GetLocalInstallationDevice deliberately falls back to a non-current device. devices has 3 rows.
accounts_remote_uq, devices_remote_uq, person_primary_identity_uq, outbox_send_again_of_idx	Uniqueness or FK action only; no reader.

Fixed

Median ms at 1× / 30× / deep 10×:

read	path	before	after
/api/status freshness (v2 PlatformStats)	every 30 s while the app is open	1,679 statements: 800 / 19,263 / 15,072	4 statements: 1.8 / 2.4 / 5.9
keyset page before/after a cursor	UI "load older", messages-around, MCP person tools	2.4 / 1.8 / 15.6	0.41 / 0.39 / 0.42
empty-query search (MCP get_messages)	per agent call	95 / 1,758 / 397	0.06 / 0.11 / 0.17
search by sender phone	MCP / UI	65 / 2,251 / 582	2.9 / 11.7 / 51.6 (that sender's history, plus one scan of identities for the canonical value: 3.2k rows today)
cross-account conversation list	/api/conversations, every ~5 s per open UI	every conversation into Go: 1.8 / 101 / 2.0 per account	0.60 / 0.73 / 0.48 (limit plus the archived count; see below)
conversation roster	per Google message/conversation frame	0.61 / 24.8 / 0.71	0.03 / 0.06 / 0.03
direct thread by sole peer	per message on an unbound thread	2.5 / 70.8 / 1.7	0.17 / 0.18 / 0.28
group thread by peer set	per unbound group snapshot	0.56 / 22.0 / 0.85, plus a roster query per group	0.10 / 0.31 / 0.11, plus rosters only for groups holding the first member
outbox state per rendered outgoing message	per UI request	whole account's outbox, then sort	index seek
MessageHasReadCursor	openmessage repair, per planned row	SCAN read_cursors	seek on the message's conversation
readCursorDeviceOutside (added by #218)	merges across conversations: repair moves, and echo merges whose duplicate sits in another conversation	SCAN read_cursors + sort	seek on the message's conversation
MessageHasOpenSend and the open-send-elsewhere check (added by #229)	openmessage repair, per planned row	SCAN outbox	seek on outbox_local_message_idx (local_message_id=?)

How:

• Freshness. It reads each account's newest message and newest incoming message with MAX seeks on a new messages_account_direction_time_idx, through a count-free PlatformLatest that the v2 read source implements. /api/status uses it when available. PlatformStats, MessageCount, ConversationCount and LatestTimestamp become per-account aggregates. The counts are one covering COUNT(*) each (5.6 / 125 / 47 ms) and run for the CLI and MCP status, /api/diagnostics and the Google pull-health counter (ConversationCount("sms")). Before, all four loaded each account's whole conversation list; PlatformStats and MessageCount then paged through every message, and LatestTimestamp read each conversation's newest message.
• Keyset cursors. These compare (occurred_at_ms, message_id) as a row value, which SQLite turns into an index range. The old OR form bounded only conversation_id. Both columns are NOT NULL (STRICT primary keys reject NULL; checked), so the two forms select the same rows.
• Search.
  • A listing with no substring reads a new messages_time_idx from its newest end. Filtered by account (no current caller does), direction IN ('incoming', 'outgoing') restates the column's CHECK so messages_account_direction_time_idx bounds a date window per direction.
  • The sender filter selects identity IDs in a subquery. The composite FK messages(account_id, sender_identity_id) → identities(account_id, identity_id) makes the dropped account match redundant.
  • A substring search over all conversations keeps its sequential scan, pinned with NOT INDEXED. With the time index present, the planner would otherwise walk it and fetch rows out of order: 3.4 s versus 0.45 s for a rare term at 30×.
• Conversation list. One statement over both arms of conversations_recency_idx. The unarchived IS NULL prefix is already in recency order, so that arm stops at the limit. The archived >= 0 range (the column's CHECK makes that every non-NULL value) is ordered by archive time first, so that arm reads and sorts every archived conversation. The cost is the limit plus the archived count (zero archived conversations on the live store), never the whole table unless every conversation is archived.
• Rebind lookups. These start from conversation_participants. A CROSS JOIN pins the roster order; the peer and group lookups drive from the identity's participant rows. The group roster comparison is unchanged: a match must contain every wanted identity, so only groups holding the first one can match.
• Outbox lookups. A new partial outbox_local_message_idx (local_message_id, account_id, created_at_ms, outbox_id) serves the per-row state lookup (both bound, already in the newest-first order) and repair's open-send checks (local_message_id alone).
• Cursor lookups by message. read_cursors has no index on last_read_message_id, but the composite FK (conversation_id, last_read_message_id) → messages(conversation_id, message_id) puts any cursor naming a message in that message's own conversation. MessageHasReadCursor and readCursorDeviceOutside therefore seek read_cursors_conversation_idx there.

Migration 0011 read_path_indexes

Three indexes, no row changes. On copies, the migration itself (ledger execution_ms) took 1.5 s on the live store, 4.7 s at deep 10× and 20 s at 30×; the whole Open() took 2.1 / 6.1 / 26.5 s. Integrity was ok each time.

Read clients never migrate (from review round 1)

The reviewer showed that a newer openmessage read/status or MCP client process would apply pending migrations underneath the running daemon. A migration holds SQLite's write lock for its whole run, and the daemon's writers give up after the 5 s busy timeout, so inbox appends could be dropped (reproduced with a held migration transaction). This was true for any migration on main, but 0011's index builds made it concrete. Read clients now open the store with sqlite.OpenWithoutMigrating:

• it never creates or migrates the store;
• it checks the ledger in a read-only (deferred) transaction, which takes no write reservation;
• it refuses a store with pending migrations (ErrMigrationPending, "quit and reopen the OpenMessage app").

Round 2 found one more non-owner path. openmessage repair google-idspace without --apply skips the daemon probe and instance lock, yet opened the store with the migrating Open. Now:

• a dry run uses OpenWithoutMigrating, and only --apply, which has checked the daemon is down and holds the instance lock, migrates;
• --reference reads a migrated temporary copy, so a user's older backup is never rewritten;
• OpenWithoutMigrating connects with mode=rw, so a store removed after the existence check is not recreated.

It is otherwise an ordinary read-write connection: like any SQLite connection that closes last, it may checkpoint a WAL nobody else has open, which changes no row. #166's OpenReadOnly is the fuller version of this (a mode=ro connection, schema-range reads) and can replace it at the same call sites when #166 lands.

The migrating opens left are the daemon (the first step of its v2 stack, before v2 ingest exists), repair --apply and cutover. The daemon's ownership is assumed, not locked: a second daemon started by hand (serve --web beside the app) would still migrate before failing on the busy port, as on main today. The runbook forbids that shape, and locking daemon startup before it opens stores is #242.

A full rerun of every statement found one other plan change. FindMessageContentDuplicate (per Google message) now seeks the new account/direction/time index on three equalities instead of the conversation index on two. Both read only the rows at that exact millisecond, at the same speed. It's pinned as "never scans".

Not fixed, and why

read	why
AppendInbox dedupe lookup; InboxReceiptsBetween; InboxReceiptsAfterRow full rescan	fixed by #210
substring search over all messages; SearchConversationsByName	inherent to a leading-wildcard LIKE (0.45–1 s at 30×); needs FTS5 (follow-up)
outbox listings (ListPending, ListConfirmedSince)	bounded by the outbox (51 rows)
UpsertDevice	legacy-mirror path only; follow-up (below)
startup sweeps; openmessage repair statements; openmessage migrate validation	once per start, manual CLI, or one-off validation

Invariants (stated, and tested for all inputs the generators reach)

• Keyset. For every cursor and limit, the row-value cursor returns exactly what the OR-form cursor returned, in the same order. Paging a thread visits every message once, newest first. Differential property against the old SQL, kept verbatim in the test.

• Search. For every filter combination (term, account, conversation, sender phone, since, until), the new statement returns the same messages in the same order as the old one. Same differential style, over stores where phone numbers repeat across accounts and kinds and some senders are NULL.

• Rebind. The roster, sole-peer and peer-set lookups equal the replaced queries and the replaced Go group scan for every account, identity and roster in random stores.

• Conversation list. For every limit, it equals a full sort of all conversations (archived included, ties broken by ID), truncated.

• Aggregates:

  • latest = max occurred_at_ms (0 when none);
  • latest incoming = max over incoming;
  • counts are row counts;
  • a platform appears in PlatformStats iff it has a message;
  • PlatformLatest = PlatformStats minus Count.

  The read-cursor check equals counting cursors; the outbox state equals the newest row by (created_at_ms, outbox_id).

• Migration. It applies to blank and to pre-0011 stores with rows. Earlier ledger rows and all rows are unchanged, the checksum is pinned, and a reopen applies nothing.

• Plans. Every fixed statement's EXPLAIN QUERY PLAN is pinned (read_plans_test.go), including the one deliberate scan, and both direction seeks of the newest-times read.

• Read clients. A store with pending migrations is refused and left byte-identical; a current store opens; a newer one is refused; a missing file is not created. The open never waits for, or blocks, the owner's write lock. read and the MCP client refuse a store one migration behind and apply nothing.

Mutation check: 33 planted bugs, all caught by these tests (mutants.txt records which test failed for each; a mutant that doesn't build counts as not caught). They include the review's surviving candidates: an outgoing seek without its direction, and a search projection that blanks bodies (now caught because the differentials compare whole rows). They also cover a client ledger check that takes the write lock, and both call sites reverting to the migrating Open.

Coordination

• Migration number. Prune old v2 inbox payloads but keep every row and dedupe key #196, Keep same-roster Google threads distinct; re-key only threads whose id is dead #201, Truthful send states, hard channel contract, and outbox safeguards #166 and Index inbox receipts by codec, and keep inbox reads off full scans #210 each also use 0011. Whichever merges first keeps it; the rest renumber. This PR's tests find its migration by name, so renumbering it touches migrations.go, the file name, and the version-count lines every migration PR edits (*_test.go, internal/migration/validate.go and transform_test.go).
• Keep same-roster Google threads distinct; re-key only threads whose id is dead #201 overlap. Keep same-roster Google threads distinct; re-key only threads whose id is dead #201 splits FindDirectConversationBySolePeer and FindGroupConversationByPeerSet into List* variants with the same SQL. Whichever lands second carries the other's change over: here that's the two query constants, which stay exact when all matches are returned.
• Deploy order. Deploy the app first: it applies 0011 when it starts (about 2 s on the live store). Then update /usr/local/bin/openmessage, which the MCP server runs (it is currently a July 21 build). Until the two match, CLI and MCP reads, and repair dry runs, refuse the store with a clear error; nothing migrates under the running daemon.

Test plan

• go test ./internal/storage/sqlite ./internal/v2read ./internal/web ./internal/db ./internal/tools ./internal/ingest ./internal/v2wire ./internal/migration
• go test ./cmd -run 'Status|Read|Migrate'
• New tests under -race (sqlite package: 150 s locally on a loaded machine at 60 cases; now 30)
• go test ./cmd -run 'TestOpenCommandReadSource|TestRunServeMCP|Client|Status'
• go test ./cmd -run 'TestRepair'
• Rebased onto Merge echo duplicates into the local row, and run migrations with foreign keys off #218 and Merge id-space repair duplicates into the copy that stays #229, and their new statements audited (above)
• Mutation check: 33/33 caught
• Independent review round 1 (GPT-6.1 Sol): REQUEST_CHANGES, one blocking finding (client migration under a live daemon) and five non-blocking; all addressed
• Round 2: REQUEST_CHANGES, one blocking finding (repair dry run migrates), one non-blocking and one nit; all addressed in bb35860
• Round 3: APPROVE at bb35860 (one non-blocking follow-up, Lock daemon startup before it opens (and migrates) the v2 store #242: lock daemon startup before opening stores; one wording nit, fixed here)
• CI green at bb35860

Evidence (local): ~/reviews/openmessage-sqlite-plan-audit-2026-10-09/. It holds AUDIT.md, the per-statement appendix, the raw results for all twelve store/label runs, the harness, the scale scripts and the mutants.

🤖 Generated with Claude Code

[MaxGhenis]

Stacked on this PR: #223 (FTS5 trigram substring search) adds migration 0012 substring_search after this PR's 0011 and replaces searchMessagesStatement's unscoped LIKE path (it keeps this PR's account/direction restatement and NOT INDEXED pin for queries the trigram index can't narrow, and rewrites TestUnscopedSubstringSearchKeepsSequentialScan as TestShortSubstringSearchKeepsSequentialScan). #223 will be rebased onto main and retargeted once this merges.

[MaxGhenis]

Merged at reviewed head bb35860 (squash 29a9a8e). Gates: gh pr checks exit 0 (Go Test, Go Race, Web E2E, Site Build, macOS App Build), mergeable CLEAN, independent review by GPT-6.1 Sol over three rounds (r1 and r2 REQUEST_CHANGES, both addressed; r3 APPROVE at this head). Reports are kept at ~/reviews/openmessage-sqlite-plan-audit-2026-10-09/review-r{1,2,3}-full.md. Follow-up: #242. Deploy the app before /usr/local/bin/openmessage: the app applies 0011 at startup, and until the two match, CLI and MCP reads refuse the store with a clear error.

🤖 Generated with Claude Code