Review copy. On the personal fork for review before it goes to basicmachines-co/basic-memory. Merging it here is not the goal.

Proposed upstream title: fix(core): a search refresh that loses a race never fails the save. The body below is the draft upstream description. Upstream, it opens with the line Fixes #1681; that line is left out here so it closes nothing on this fork.

Rebased on upstream main 023eb1cc, past the revert of the edit_note change whose integration test timed out on Postgres. This fork's main is behind upstream, so the Files tab here also shows upstream commits. The change under review is the two commits on top: 4b6135f9 (the fix) and 773d4c7d (one integration test that relied on the old transaction shape)

Why

On Postgres, saving a long note twice in quick succession could answer 500. Both saves were accepted or about to be; what failed was a refresh of the note's search rows. Every refresh of an entity's search rows is delete-then-insert in one transaction (since basicmachines-co#1623), and two of them for the same entity at once either deadlock on the DELETE or both reach their INSERTs together and hit search_index_pkey, which the upsert's permalink arbiter cannot absorb. basicmachines-co#1665 has the full diagnosis.

One save runs several refreshes of the same entity, and two of them sat in the save's failure path:

1. The freshen reindex before the accept. freshen_existing_note_content indexes the file inline, and its refresh can overlap the previous save's materialization index. A lost race propagated, and the save answered 500 before anything was accepted.
2. The hot search row inside the accept transaction. persist_accepted_note_snapshot refreshed the entity's search row in the same transaction as note_content. A lost race there rolled back the canonical write with it. This is also the cycle noted on fix(core): serialize search-index rewrites of one entity on Postgres basicmachines-co/basic-memory#1665: the accept transaction holds its note-content and entity locks while it waits on search_index.

Search rows are derived state (Consistency Model in CLAUDE.md, basicmachines-co#1213, basicmachines-co#1224, basicmachines-co#1214). The refresh that loses should be logged and left for the next refresh, reindex or sweeper, and it must never fail the canonical write.

What changed

No locks, no retries, no wider transactions.

• repository/search_refresh_race.py (new): lost_search_refresh_race(error) names the two ways a refresh loses to a concurrent refresh of the same entity: deadlock (asyncpg DeadlockDetectedError) and search_index_pkey (UniqueViolationError on that constraint only). It returns None for anything else, including unique violations on other constraints, so real integrity bugs still raise.
• SearchService.index_entity_data: when the delete-then-insert transaction fails with a lost race, the rollback has already left the winner's whole projection in place; the refresh logs a warning and returns. Any other error raises as before. This covers every caller of the indexer's refresh: the freshen reindex, materialization, the watcher, relation resolution and reindex.
• The hot search row leaves the accept transaction. persist_accepted_note_snapshot and persist_accepted_note_move now accept note_content only and return the row to write (AcceptedPersistedNoteWrite.search_row, carried on AcceptedNoteMutationResult the same way relation_publication already is). NoteContentMutationService._finish_mutation writes it after commit through refresh_accepted_note_search_index, which opens its own transaction and treats a lost race the same way. Any other failure there is logged rather than raised, for the reason graph publication already gives in the same method: the canonical write has committed, and failing the response would report it as failed and strand its materialization. The accept transaction no longer touches search_index at all, so the cycle above cannot form.
• docs/ARCHITECTURE.md: the paragraph on persist_accepted_note_snapshot said graph rows and the search row are written inside the caller's transaction. Graph rows already were not; now neither is.

The happy path is unchanged: _finish_mutation still runs before the route returns, so the hot row is in place by the time the save answers.

Convergence

A refresh that loses leaves the rows of the one that won. That projection is whole (one row per entity, observation and relation, never a mix), but it may hold an older version of the text than the loser had. The next refresh converges it. On a save there always is one: the hot row is followed by the note's materialization index, which reads the accepted file. The regression tests below check both the whole projection after the overlap and convergence on the next refresh.

On structured concurrency

The issue asks for post-accept refresh work to run as owned structured concurrency (a TaskGroup) rather than inline in the request's failure path. What this PR does instead: the post-commit steps in _finish_mutation (hot search row, then graph publication) run one after the other, each owning its own failure, and neither can turn the response into a 500. I did not put them in a TaskGroup, for two reasons:

• They would then be concurrent write transactions on SQLite. In WAL mode, a transaction that has read and then tries to write after another connection committed gets SQLITE_BUSY at once, without the busy timeout, and graph publication is a series of short generation-checked transactions. index_entities already keeps index writes sequential because SQLite connections cannot safely run these mutations concurrently. I did not want to trade a Postgres failure for a possible SQLite one.
• A TaskGroup re-raises a child's failure into the awaiting request, so it isolates nothing by itself; each child would still need the handler it has now.

The deferred work that is already off the request (the materialization worker pool, and _background_tasks for vector sync and relation resolution) could move under one app-lifetime TaskGroup. That is a wider change to the runtime's lifecycle and to the cloud/local scheduler parity contract, so I left it out. Happy to take it on as a follow-up if you want it.

Testing

The reproduction tests from basicmachines-co#1665 are kept as the regression check in tests/services/test_search_refresh_concurrency.py (Postgres; skipped on SQLite). Every refresh in them pauses 50 ms between its delete and its insert, and the collision rounds start from an entity whose rows were just deleted, so the race is certain rather than likely. Changes from basicmachines-co#1665:

• The negative controls now remove the race tolerance (instead of the lock) and assert that the refreshes still collide with search_index_pkey or deadlock detected. They show the race is still there underneath, so the positive tests are not passing by luck.
• The first test asserted "the last save wins". Without serialization that is not guaranteed, by design. It now asserts that every overlapping save succeeds, that the projection is whole and is one of the saved versions, and that the next refresh converges it to the file on disk.
• The accepted-write test goes through refresh_accepted_note_search_index, the post-commit path a save now uses.

New:

• tests/api/v2/test_overlapping_saves.py::test_overlapping_saves_of_a_long_note_all_answer_202 (Postgres): three rounds of six concurrent PUT /v2/projects/{project}/knowledge/entities/{id} on a long note with relations and observations, with the same 50 ms pause. Every save must answer 202.
• test_a_failed_post_commit_search_refresh_does_not_fail_the_save (both backends): the hot search refresh raising after commit leaves the save at 202, accepted and materialized.
• tests/repository/test_search_refresh_race.py, and race / non-race branch tests in tests/services/test_search_refresh_atomicity.py and tests/indexing/test_accepted_note_write_runner.py (both backends).

Changed: test-int/test_accepted_note_mutation_runner_db.py::test_delete_removes_the_note_and_returns_guarded_cleanup seeded its search rows through the accept transaction. It now writes result.search_row after commit, as the service does, before deleting.

Results, Postgres 17 via testcontainers:

Postgres, tests/services tests/repository tests/indexing tests/index tests/api/v2: 2841 passed, 71 skipped, 0 failed. 23 tests errored in setup with DiskFullError from the local Docker disk (index, rerank-pagination and vector-filter fixtures), not from this change.

SQLite: tests/services tests/repository tests/indexing tests/index tests/api tests/mcp: 4270 passed, 60 skipped. The 42 failures and 50 errors are the same set as on main in this environment (sqlite-vec cannot load in this Python build, so the semantic tests fail on both).

ruff check, ruff format --check, ty check src tests test-int: clean apart from the unresolved pymilvus imports that main shows in an environment without the Milvus extra.

Risks / follow-ups

• Hosts that run the mutation runners without NoteContentMutationService._finish_mutation must now write result.search_row after commit themselves, or the hot row waits for the materialization index. In this repo every caller goes through _finish_mutation.
• A save's search row now commits after the accept rather than with it. Between the two commits, search shows the previous version. If the post-commit refresh fails or loses, search shows the previous version until the materialization index lands.
• The accepted-note delete still deletes the entity's search rows inside the delete transaction. It did not show up in the failures, and the entity delete removes them anyway; it can move post-commit the same way if you want it to.
• Only search_index_pkey counts as a lost race. A concurrent refresh could in principle also conflict on the Postgres FTS chunk table's key; I have not seen it, so it still raises.