Stop a turn locking out its own memory bank, and let the long run notice

The first M01 trial with the memory bank on was 26 turns on a GPU host. It
accepted every turn and reported "complete". It also wrote two memories and
no summary, and logged 180 `database is locked` errors, while derived status
still read `idle`.

The cause was a single uncommitted UPDATE. Retrieval bumped each used
memory's counter before the model call, and the turn commits only after the
reply has streamed. SQLite has one writer, so the turn held the write lock for
the whole reply. Every post-turn memory, summary and status write in that
window waited out the five-second timeout and failed. Recording the failure
needed a write as well, and without a rollback first it raised
PendingRollbackError. The loss therefore reached the log and never reached
the status the Insights panel reads, which F08 forbids. The draco run never
hit this because the bank was off there.

- `retrieve_memories` now only reads. `record_use` writes the counters in the
  turn's single commit, so a turn that never lands counts nothing.
- The post-turn task's outer handler rolls back before it records a failure.

The harness could not have caught any of this. It read three prompt sections
under names the builder does not use: `memories` (really `used_memories`),
`story_history` (really `history`/`recent_history`), and a `knowledge` prefix
that matched the fixed instruction section instead of the imported passages.
Memory tokens read 0 whatever the prompt held, and the in-history and
in-memories recall checks could never come out true. The labels are now
constants, pinned by a test against a prompt the real builder assembled.

The harness also stops at the first sign of failed post-turn work. It checks
/derived and new server.log lines after every turn, keeps its log position
across --resume, and waits for background work to settle before its final
checks. A run with no memories or no summaries now ends "failed", not
"complete".

Both new application tests fail on fec46f6: the lock probe sees
`database is locked`, and memory status stays `idle`. The full backend suite
passes (1392 passed, 17 skipped). A 26-turn re-run against the same host had
0 lock errors, wrote 7 memories and 2 summaries, and used them in the prompt
from turn 8.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0136VBTMUKWYeU6G9HgbDbND

backend/app/memorybank.py

@@ -539,7 +539,6 @@ async def retrieve_memories(
     adventure: models.Adventure,
     settings: models.Settings,
     *,
-    update_stats: bool,
     exclude_action_id: int | None = None,
 ) -> dict | None:
     """Returns the memories to inject, or None when the bank is off.
@@ -548,8 +547,9 @@ async def retrieve_memories(
     `{"used": [{id, text, similarity, pinned}], "error": str | None}`. It is
     None when the memory bank is disabled for this adventure.
 
-    Set `update_stats` to True to increment the use counters. Only real turns
-    should do this, not the dry runs that Insights performs.
+    This only reads. A turn counts the memories it used with `record_use`, just
+    before the commit that saves the turn; see that function for why the count
+    cannot be written here.
 
     `exclude_action_id` removes the action being retried from the similarity
     query, so that a discarded attempt cannot influence which memories are
@@ -640,18 +640,6 @@ async def retrieve_memories(
     }
     texts = {memory_id: row.text for memory_id, row in detail.items()}
 
-    if update_stats:
-        # Pass `synchronize_session=False` because nothing in this request
-        # reads the counters back. Matching the UPDATE against loaded objects
-        # would require loading those objects, which is the cost this code path
-        # exists to avoid.
-        db.execute(
-            update(models.Memory)
-            .where(models.Memory.id.in_(used_ids))
-            .values(use_count=models.Memory.use_count + 1, last_used_at=models.utcnow())
-            .execution_options(synchronize_session=False)
-        )
-
     return {
         "used": [
             {
@@ -681,6 +669,38 @@ async def retrieve_memories(
     }
 
 
+def record_use(db: Session, memory_bank: dict | None) -> None:
+    """Counts the memories a turn was given, as part of that turn's commit.
+
+    Call this immediately before the commit that saves the turn, and never
+    before the model call. This counter used to be written during retrieval, and
+    the UPDATE opened a write transaction that stayed open for the whole reply,
+    because the turn commits only once the narration has streamed. SQLite has
+    one writer. Every post-turn memory, summary and status write that arrived
+    during the reply waited out the driver's five-second timeout and failed with
+    `database is locked`. Recording those failures also needs a write, so it
+    failed the same way, and derived status kept reporting `idle`. A 26-turn
+    run on a GPU host wrote two memories and no summary while every turn was
+    accepted.
+
+    Only real turns count, never Insights' dry runs. A turn that fails before
+    its commit counts nothing, because nothing was used.
+
+    Pass `synchronize_session=False` because nothing in this request reads the
+    counters back. Matching the UPDATE against loaded objects would require
+    loading those objects, which is the cost this code path exists to avoid.
+    """
+    used_ids = [m["id"] for m in (memory_bank or {}).get("used") or []]
+    if not used_ids:
+        return
+    db.execute(
+        update(models.Memory)
+        .where(models.Memory.id.in_(used_ids))
+        .values(use_count=models.Memory.use_count + 1, last_used_at=models.utcnow())
+        .execution_options(synchronize_session=False)
+    )
+
+
 # ---------- Post-turn background work ----------
 
 def schedule_post_turn(adventure: models.Adventure) -> None:
@@ -768,6 +788,11 @@ async def run_post_turn(adventure_id: int) -> None:
         # setup above, or in eviction. M2's lesson is that the one thing this
         # may not do is vanish. Re-raising would only feed an unobserved task.
         try:
+            # Roll back first. The failure is often a flush or commit that
+            # failed, which leaves the session unusable until it is rolled
+            # back, and the record then fails with `PendingRollbackError`
+            # instead of being written. `_guarded` already does this.
+            db.rollback()
             derived.failed(db, adventure_id, derived.MEMORY, exc)
             db.commit()
         except BaseException:  # noqa: BLE001 - the recorder must not mask it

backend/app/routers/adventures/insights.py

@@ -25,7 +25,7 @@ async def dry_run_context(
 ):
     """Returns what the app would send to the AI if the player continued now."""
     settings = get_settings(db, user)
-    memories = await memorybank.retrieve_memories(adventure, settings, update_stats=False)
+    memories = await memorybank.retrieve_memories(adventure, settings)
     # M7: retrieved here too, and by the same call the turn makes. A dry run
     # that skipped the library would show a prompt the next turn will not send,
     # which is the one thing this panel must never do.

backend/app/routers/adventures/turns.py

@@ -193,8 +193,12 @@ async def _generate_turn(
     # context. Otherwise the model reads the attempt it is replacing as
     # established story and writes a sequel to it.
     replacing_id = retry_of.id if retry_of is not None else None
+    # Retrieval only reads. The use counters are written by `record_use` in
+    # the turn's single commit below. Writing them here would hold SQLite's
+    # write lock for the whole model call, and would lock out every post-turn
+    # write that ran during the reply.
     memories = await memorybank.retrieve_memories(
-        adventure, settings, update_stats=True, exclude_action_id=replacing_id
+        adventure, settings, exclude_action_id=replacing_id
     )
     # M7: the imported library, retrieved for the position being read. Excluding
     # the attempt being replaced matters here for the same reason it does for
@@ -372,6 +376,7 @@ async def _generate_turn(
         "summary": narrative.apply.diff(before_state, new_state),
     }
     attempts.snapshot_outcome(adventure, ai_action)
+    memorybank.record_use(db, memories)
     adventure.updated_at = models.utcnow()
     db.commit()
     db.refresh(ai_action)