[loveRhythm1990]

A new single-chunk Add currently performs five durable SQLite commits. All journal operations share the writer gate, so repeated commits add storage latency and queueing across concurrent requests.

This change reduces that path to three commits:

1. Register/validate the request and persist atomic send intent together.
2. Retain the backend acknowledgment checkpoint before visibility verification.
3. Persist the last verified chunk and request completion together.

Existing atomic inflight intents are reused without an identical rewrite. New multi-chunk requests use 3N commits instead of 3N+2. Completed replays remain read-only. The existing schema, journal mode, synchronous=FULL, per-request locks, and cross-process writer gate remain compatible. No SQLite transaction or global journal gate spans a backend call. Unknown legacy writes retain query-only recovery, and older stored-but-incomplete requests still finish on retry.

Validation:

• uv run --extra evaluation pytest -q: 410 passed.
• Ruff checks and git diff --check passed.
• Fault injection covers all three commit failures, rollback between the final stored marker and request completion, interruption after completion, restart/replay, legacy recovery, and release of the writer gate before network calls. The existing abrupt-process-exit test now interrupts after the combined intent commit.
• Added a reproducible, isolated synthetic benchmark: uv run python scripts/benchmark_journal.py --commit-delay-ms 10. Use --source-root /path/to/baseline/agent_memory_leaderboard to run the same harness against the baseline.

Local single-run results, 64 one-chunk requests per case, synthetic in-memory backend:

Injected delay per commit	Concurrency	Main RPS	New RPS	Main P95 ms	New P95 ms
0 ms	16	387.16	626.90	45.56	25.98
10 ms	16	14.61	24.45	1170.27	664.09
10 ms	32	14.89	25.50	2212.55	1282.79
10 ms	64	14.50	24.35	4363.42	2574.21

All 1/16/32/64-concurrency cases succeeded with exactly 64 synthetic backend records, both with and without injected latency. Commits fell from 320 to 192 per 64 requests (40% fewer). These are isolated local measurements, not shared-environment capacity results; deployed-storage validation remains necessary. SQLite still has one writer, and this PR does not change probe policy or deploy an image.

[loveRhythm1990]

Code Review (automated)

Is the change reasonable?

Yes. Every journal operation goes through a single writer gate, so each durable commit (synchronous=FULL) adds latency to every queued request. Cutting a single-chunk Add from 5 commits to 3 is a sound target. The three remaining commit points are the right ones: (1) register the request and record the send intent, (2) record the backend acknowledgment before verifying visibility, (3) mark the last chunk stored and the request complete together. Each one is still durable before the next backend call, and no transaction or gate is held across a network call.

Is the implementation correct?

On the production path, yes. I found no hard correctness bug. I checked these specifically:

• Fingerprint conflicts: reusing a request id with a different payload still gets a 400, including after the request has completed.
• Rollback: if marking the request complete fails, the final "stored" marker is rolled back with it, so they still commit together.
• Read-only replays: replaying already-stored chunks opens no write transaction, because Python's sqlite3 only starts one implicitly before an INSERT or UPDATE.
• Empty requests: pairwise() would skip an empty chunk list, but validation guarantees at least one non-blank message, so this can't happen.

The main concern is that correctness now depends on callers using write() the right way, and nothing enforces that:

1. src/service.py:76: write() no longer checks the row's state or protocol, and no longer writes the inflight intent itself. It assumes the caller already ran prepare_write(). test_write_visibility.py now calls write() with the result of journal.state(...) (('pending', None, ...)), which skips the intent write. Without it:

• A crash between the POST and the ack record leaves no evidence that a send happened.
• A legacy-protocol pending row records its memory_id under legacy instead of being upgraded to idempotent_v1.
• An unknown state such as 'garbage' is treated as a fresh send instead of being rejected with a 503.

Production code always calls prepare_write() first, so this is not a live bug. But the safety now rests on convention. Consider asserting the expected state/protocol inside write(), or having write() take only the token that prepare_write() returns.

2. src/journal.py:275: record(complete_request=True) runs UPDATE requests SET complete = 1 without checking how many rows changed. If the request was never registered (e.g. the direct write() call in the test above), the chunk is marked stored and the request silently never completes. Checking rowcount == 1 would turn this into an error instead of a silent gap.

3. src/journal.py:213: begin() and state() are no longer used by production code, only by tests. That leaves a second, unenforced way to reach the same state, and it's what hides finding #1. Make them private, or move the tests onto prepare_write().

4. src/service.py:88: Completing a stored-but-incomplete request from an older journal now happens in write() as a special case for the last chunk, with its own complete() call. Request completion is now spread across prepare_write, record(complete_request) and that separate complete(). Having prepare_write take a last flag and handle this case itself would keep journal-format details inside the journal.

Efficiency (minor)

5. src/service.py:42: pairwise() renders the first two chunk bodies before prepare_write() runs. A completed replay, or a conflicting reused id, now does that rendering work before getting its early return or 400; previously begin() returned before any rendering happened. If rendering throws, the caller gets a 500 instead of the 400.

6. src/journal.py:253: prepare_write() re-runs the requests-table lookup and fingerprint check for every chunk, even though the per-request lock is held for the whole Add and the answer can't change. An N-chunk Add runs N identical queries while holding the shared writer gate, which works against what this PR is trying to achieve. Checking once on the first chunk is enough.

Benchmark script

7. scripts/benchmark_journal.py:98: The script monkeypatches sqlite3.connect process-wide and only counts commits made through an explicit db.commit(). Commits that happen implicitly through with sqlite3.connect(...) as db: go straight through the C implementation, so they are neither counted nor delayed. If the baseline checkout passed via --source-root commits that way, it will under-report commits and injected delay. That would make the main-vs-new comparison in the table unreliable. Worth confirming that the baseline only uses explicit commit() calls.

8. scripts/benchmark_journal.py:109: The status-count dict comprehension is O(n²). collections.Counter(str(code) for code, _ in results) does the same thing in one pass.

---

Verdict: the change is sound and the production path is correct. Before merging I'd address #1–#2, so the commit-ordering guarantees don't depend on every caller doing the right thing, and confirm #7, so the benchmark comparison holds. #3–#6 and #8 are cleanup.

🤖 Generated with Claude Code