Fix silent vector loss from concurrent writes in SQLiteVectorStore - #1469

Purpose of the change

Fix the concurrent-write races described in #1468. In SQLiteVectorStore every write commits its SQL transaction and only then awaits the search engine apply, so two writers on one collection can overlap. Two independent defects live in that window, and both end with SQLite and the index durably disagreeing while every surface a caller can inspect looks intact.

Description

1. row_id reuse. The per-collection records table declared Column("row_id", Integer, primary_key=True, autoincrement=True) without sqlite_autoincrement=True on the Table, so SQLAlchemy emitted a plain row_id INTEGER PRIMARY KEY (a rowid alias). SQLite assigns max(rowid) + 1, so deleting the row holding the maximum row_id frees that id for the very next insert, and a new record inherits the id an old one was being served under.

Fixed with sqlite_autoincrement=True on the records table. Ids are never reused, so a suspended delete can only ever target the dead id, and a stale engine key maps to no row instead of to whichever record inherited it.

2. Ordering. Serialization was fix option 3 in the issue, and it turns out to be load-bearing rather than optional, because AUTOINCREMENT cannot reach the case where two writes address one uuid -- an upsert of an existing uuid keeps its row_id by design:

• An upsert that overtakes a delete of the same uuid re-adds its vector after the record is gone. The vector resolves to no row, so it wins result slots and is then dropped from them (a query returns fewer matches than it found), and the next save publishes it into the index for good.
• Two upserts of one uuid inverting leaves the index serving the older vector while SQLite records the newer.
• A save falling in the window costs a write outright. A save publishes the index and then trims every operation the index now holds, and the pending log is the only other copy of those vectors. A write that applies to the engine after the file is written but before the trim is in neither: live in memory, absent from disk. That is a committed write lost to a process crash, which is the failure this store otherwise rules out.

Fixed with a per-collection write lock held across the whole sequence -- SQL commit, engine apply, mark applied, and any save it triggers. The lock lives on the store, not on a collection handle, because a handle is constructed per open_collection call and only a shared lock serializes them; shutdown's save takes it for the same reason a write's own save is inside it. Readers are untouched and still run freely.

Writers already serialized on SQLite's single-writer lock for the transaction and on the engine's own lock for the apply, so what this gives up is the overlap between one writer's transaction and another's engine work. The case worth checking is the save, which now runs inside the lock: every engine holds its own write lock for the whole of save, so a concurrent writer already blocked there for that duration, and what it waits for on top of that is one SQL transaction.

Regression tests (test_sqlite_vector_store.py), each failing on the code it fixes:

• test_row_ids_are_never_reused -- a record inserted after a delete must not receive the deleted record's row_id.
• test_a_query_cannot_return_a_record_it_never_scored -- query() scores keys and resolves them to rows in a second step, holding nothing in between. A gated engine parks in that gap while the scored record is deleted and another is inserted; the query must not return a record that was never scored, wearing the score of the one that was. This is the read-path exposure from the issue, and the reason AUTOINCREMENT is still load-bearing after writes are serialized.
• test_an_upsert_survives_a_delete_of_another_record -- the issue's original interleaving, end to end.
• test_an_upsert_cannot_overtake_a_delete_of_the_same_uuid -- the inversion above; without the lock the query returns nothing, its only slot taken by a vector that resolves to no record.
• test_a_save_cannot_trim_a_write_it_did_not_publish -- a write applied behind a parked save, then a simulated crash; without the lock the record comes back with its vector gone from the index and its log row trimmed.

No new dependencies.

Fixes/Closes

Fixes #1468

Type of change

• Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

• Unit Test

Each test was run against the code without the corresponding fix and fails there: removing sqlite_autoincrement fails the two TestRowIdReuse tests, and removing the write lock fails test_an_upsert_cannot_overtake_a_delete_of_the_same_uuid and test_a_save_cannot_trim_a_write_it_did_not_publish.

Test Results:

uv run pytest packages/server/server_tests/memmachine_server/common/vector_store -q
276 passed, 140 deselected

ruff check, ruff format --check, and ty check packages are clean.

Checklist

• I have performed a self-review of my own code
• I have commented my code
• My changes generate no new warnings
• I have added unit tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

Further comments

sqlite_autoincrement only affects CREATE TABLE DDL, so records tables of collections created before this change keep plain-rowid behavior until recreated; new collections get AUTOINCREMENT. The write lock has no such gap -- it closes the write-path half for pre-existing tables too, since serialized writers cannot be handed a freed id inside another write's window. What those tables still lack is the read-path guarantee, which only recreating them restores.

Per-insert cost of AUTOINCREMENT is SQLite's sqlite_sequence bookkeeping write, which is negligible here.