feat(kernel): SQLite concurrency posture — WAL, busy_timeout, OperationalError → WorkspaceBusy (#80)

docs/ingest.md

@@ -150,7 +150,8 @@ emitter in this kernel follows.
 
 Step 2 is outside a transaction because decoding is a Pillow pass over thousands of files or an
 out-of-process ffmpeg, and holding a write transaction open across either is how a single-writer
-SQLite store starts reporting "database is locked". The blob writes are out there too, before any
+SQLite store starts making every other writer wait out its `busy_timeout` and fail with
+`WorkspaceBusy`. The blob writes are out there too, before any
 row exists: `BlobStore.put` is not transactional and a rollback cannot unwrite it — but a blob
 nothing points at is harmless (content-addressed, shared, never deleted), while a row naming bytes
 that were never stored is not.

docs/persistence.md

@@ -89,6 +89,48 @@ Foreign keys are declared `ON DELETE CASCADE` — and the store issues
 `PRAGMA foreign_keys = ON` for every connection, because SQLite ships with foreign keys
 **off**. Without that pragma every constraint here would be decorative.
 
+## Connection posture
+
+Three settings, and they are applied in two different places for one reason:
+
+| Setting | Where | Why there |
+| --- | --- | --- |
+| `PRAGMA foreign_keys = ON` | every connection | Per connection, and SQLite forgets it on close. Reads nothing, writes nothing. |
+| `PRAGMA busy_timeout = 5000` | every connection | Likewise per connection. Configurable: `SqliteMetadataStore(path, busy_timeout_ms=…)`. |
+| `PRAGMA journal_mode = WAL` | `initialize()` only | **Switching to WAL writes the file header.** |
+
+That last row is the interesting one. WAL is recorded in the database header and persists, so
+it only ever needs setting once — but turning it on *grows an empty file to a full page*, and
+`WorkspaceService.open` reads `format_version` before it has decided the file is a workspace
+at all. A connect-time pragma would therefore leave a 4 KB mark on any stranger's file merely
+inspected, breaking the invariant that **`open` creates nothing when it refuses**. Setting it
+in `initialize()` puts it exactly where the caller has already established the file is ours to
+write to, and re-running it there is how a workspace written before WAL converts on its next
+open. It also has to sit outside the migration transaction: SQLite refuses to change journal
+mode inside one.
+
+The consequences for callers are in
+[workspaces.md § Concurrency, plainly](workspaces.md#concurrency-plainly): readers never
+block, writers are serialized and wait up to the timeout, and a wait that runs out is
+`WorkspaceBusy`.
+
+### No SQLAlchemy exception escapes
+
+Every `DatabaseError` the engine raises goes through one function in the adapter, and the
+order of its tests is the dispatch:
+
+| SQLAlchemy raises | Becomes | Because |
+| --- | --- | --- |
+| `IntegrityError` | `ConstraintViolated` | A constraint refused the write. Ends the transaction. |
+| `OperationalError` with a `SQLITE_BUSY` / `SQLITE_LOCKED` result code | `WorkspaceBusy` | Contention. Transient — retry. |
+| any other `DatabaseError`, including the rest of `OperationalError` | `WorkspaceCorrupt` | Cannot open the file, disk I/O error, disk full, not a database. Waiting will not fix it. |
+
+Contention is told apart from damage by SQLite's **result code** (`sqlite_errorname`), not by
+the wording of the message, so a reworded SQLite release cannot silently reroute a lock into
+`WorkspaceCorrupt`. `WorkspaceCorrupt` is the widest of the three deliberately: it is where
+everything unusable-for-a-reason-you-cannot-wait-out lands, and splitting it further would
+invent errors nobody catches.
+
 `CASCADE` is the rule because the child is normally *part of* the parent. `ingest_job.batch_id`
 is the exception and states the other case: a run is a record of work done, not a child of the
 batch it filled, so deleting the batch nulls the link rather than erasing the run. The same
@@ -131,10 +173,10 @@ missing:
 | lower | the pending migrations run; the file is restamped |
 | higher | `WorkspaceFormatTooNew` — migrations only run forward |
 | not a readable database | `WorkspaceCorrupt` |
+| held by another writer past the timeout | `WorkspaceBusy` |
 
-`OperationalError` — "database is locked", "unable to open database file" — is deliberately
-*not* translated. Those are environmental, not structural, and calling them corruption would
-be a lie. It is a known gap: a SQLAlchemy exception can still escape on a locked file.
+`initialize()` also switches the file to WAL, for the reason given under
+[Connection posture](#connection-posture).
 
 **Adding a migration:** append a `Migration` with the next version and an `upgrade` taking a
 live `Connection`. Never edit an existing migration — a workspace already stamped at that

docs/sources.md

@@ -27,8 +27,8 @@ anything used it.
 
 `register_video` probes the file through the workspace's `VideoProcessor` and stores the answer.
 The probe runs **before** the transaction opens: it is an out-of-process decoder, and holding a
-write transaction open across a subprocess is how a single-writer SQLite store ends up reporting
-"database is locked".
+write transaction open across a subprocess is how a single-writer SQLite store ends up making
+every other writer wait out its `busy_timeout` and fail with `WorkspaceBusy`.
 
 ## What a source records
 
@@ -98,8 +98,8 @@ mistaken for a real rate: `extraction_fps` is `gt=0`.
 The two layers do what they do everywhere else in this store. The pre-check is what produces a
 friendly answer; the index is the guarantee. A caller that loses the race sees a raw
 `ConstraintViolated`, and the remedy is to call the same method again, which finds the winner's
-row and returns it. The store's other known concurrency gap — the untranslated `OperationalError`
-— is still open.
+row and returns it. A caller that instead waits out the store's `busy_timeout` sees
+`WorkspaceBusy`, and the remedy is the same.
 
 ## Paths are canonicalized once
 

docs/workspaces.md

@@ -7,13 +7,19 @@ happens in the context of exactly one, so `WorkspaceService` is both the way in
 ```
 <root>/
   visionset.db      the metadata store — its presence is what makes a directory a workspace
+  visionset.db-wal  SQLite's write-ahead log — present only while the workspace is open
+  visionset.db-shm  its shared-memory index — likewise
   blobs/            FilesystemBlobStore root, sharded <hh>/<hh>/<hash>
 ```
 
-Nothing else is written. WAL is deliberately **not** enabled in M1, so those two entries
-are the whole format — if WAL is ever adopted, `visionset.db-wal` and `-shm` become part
-of it and this page has to say so, because a user who copies only `visionset.db` under WAL
-loses committed data.
+Nothing else is written. The store runs in **WAL mode**, which is why the two sidecars are
+part of the format: `close()` checkpoints them into `visionset.db` and removes them, so a
+workspace at rest is still just the database and the blobs — but a workspace that is *open*,
+or one whose process was killed, is all four entries.
+
+**Copying a workspace that is open loses committed data** if you take only `visionset.db`.
+Everything written since the last checkpoint lives in `visionset.db-wal` until then. Close the
+workspace first, or copy all three files together.
 
 Three of the five ports have no line in that layout, and that is the point: the [event
 bus](events.md) is in-process and the two [media processors](media.md) are decoders, so none of
@@ -177,27 +183,46 @@ than the index, so nothing slips through.
 
 ## Concurrency, plainly
 
-One SQLite file, rollback-journal mode, no cross-process lock, one engine per open
-`WorkspaceService`.
+One SQLite file in **WAL mode**, a **5 s `busy_timeout`** on every connection, no cross-process
+lock, one engine per open `WorkspaceService`.
 
 **Guaranteed, under any number of processes:** a workspace can never contain two `project`
 rows whose names are equal under ASCII case folding. That holds because the guarantee is an
 index evaluated inside SQLite's write transaction, not the service's `SELECT`. Every write
 is a transaction, so no half-finished operation can be observed.
 
-**Not guaranteed:** which error the loser of a race sees. Two processes can both pass the
-pre-check; then either the second insert hits the index and raises `ConstraintViolated` — a
-caller re-raises that as `ProjectNameTaken`, so user-visible behavior stays correct — or the
-writes interleave and the loser eventually gets `database is locked` as an untranslated
-SQLAlchemy `OperationalError`. That leak is a known M1 gap: mapping it to `WorkspaceCorrupt`
-would be a lie, and inventing an error for transient failure with no caller would be
-speculative.
+**Readers never block, and are never blocked.** That is what WAL buys, and it is the reason
+this milestone adopted it: a background ingest holding a write transaction does not stall the
+request handlers reading beside it. A reader sees the last committed state, not the writer's
+work in progress.
+
+**Writers are serialized, and wait up to `busy_timeout` for their turn.** SQLite permits one
+writer at a time regardless of journal mode. A write that waits the timeout out raises
+`WorkspaceBusy` — a domain error, translated in the adapter like every other (see
+[persistence.md](persistence.md#connection-posture)), and transient: the remedy is to retry.
+It is deliberately *not* `WorkspaceCorrupt`, which is
+where the adapter files the failures retrying cannot fix.
+
+**Still not guaranteed:** which error the loser of a name race sees. Two processes can both
+pass the pre-check; then either the second insert hits the index and raises
+`ConstraintViolated` — a caller re-raises that as `ProjectNameTaken`, so user-visible
+behavior stays correct — or one of them waits out the timeout and gets `WorkspaceBusy`. Both
+are honest answers, and both are now domain errors.
 
 Opening the same path twice yields two independent engines with no shared cache and no
-in-process lock: **VisionSet is single-writer by convention, not by enforcement.** Hardenings
-in the order they would be taken: `PRAGMA busy_timeout`, `BEGIN IMMEDIATE` for write
-transactions (which removes the race entirely), WAL (changes the on-disk format), an advisory
-lock file.
+in-process lock: **VisionSet is single-writer by convention, not by enforcement.** A long
+write transaction is therefore still a thing to avoid rather than a thing the store defends
+against — which is why, for example, `SourceService` probes a clip *outside* its write
+transaction. `busy_timeout` shortens the window; it does not make holding a transaction
+across a subprocess acceptable.
+
+Two hardenings remain untaken, and one of them is declined rather than merely pending.
+`BEGIN IMMEDIATE` for write transactions would make every contended write wait instead of
+ever failing fast — but `unit_of_work()` serves reads and writes alike, with no read-only
+variant, so an immediate transaction would take the write lock for *every* read and serialize
+exactly the concurrency WAL was adopted for. It stays off unless the unit of work grows a
+read-only form. An advisory lock file, which would turn "single-writer by convention" into
+enforcement, is still open.
 
 ## How later services are composed
 

examples/ingest_end_to_end.py

@@ -364,7 +364,10 @@ def _clear_previous_run(dest: Path) -> None:
         return
     if not dest.is_dir():
         raise SystemExit(f"refusing to run: {dest} exists and is not a directory")
-    ours = {"visionset.db", "blobs", "clips", "incoming"}
+    # The two ``-wal``/``-shm`` entries are SQLite's WAL sidecars. A clean close
+    # removes them, so they are only here if a previous run was killed — which
+    # is exactly when this function has to be able to clean up.
+    ours = {"visionset.db", "visionset.db-wal", "visionset.db-shm", "blobs", "clips", "incoming"}
     stray = {entry.name for entry in dest.iterdir()} - ours
     if stray:
         raise SystemExit(

examples/sdk_end_to_end.py

@@ -438,7 +438,11 @@ def _clear_previous_run(dest: Path) -> None:
         return
     if not dest.is_dir():
         raise SystemExit(f"refusing to run: {dest} exists and is not a directory")
-    stray = {entry.name for entry in dest.iterdir()} - {"visionset.db", "blobs", "incoming"}
+    # The two ``-wal``/``-shm`` entries are SQLite's WAL sidecars. A clean close
+    # removes them, so they are only here if a previous run was killed — which
+    # is exactly when this function has to be able to clean up.
+    ours = {"visionset.db", "visionset.db-wal", "visionset.db-shm", "blobs", "incoming"}
+    stray = {entry.name for entry in dest.iterdir()} - ours
     if stray:
         raise SystemExit(
             f"refusing to remove {dest}: it holds {', '.join(sorted(stray))}, "

src/visionset/kernel/__init__.py

@@ -53,6 +53,7 @@
     UnsupportedMedia,
     VisionSetError,
     WorkspaceAlreadyExists,
+    WorkspaceBusy,
     WorkspaceCorrupt,
     WorkspaceFormatTooNew,
     WorkspaceNotEmpty,
@@ -105,6 +106,7 @@
     "UnsupportedMedia",
     "VisionSetError",
     "WorkspaceAlreadyExists",
+    "WorkspaceBusy",
     "WorkspaceCorrupt",
     "WorkspaceFormatTooNew",
     "WorkspaceNotEmpty",