Memory backend: store-owned delta retain, bank-list times, and generic comments (#3660)

* comments: keep the store-extension comments backend-generic

These four comments named a specific downstream store. The seam they describe is the generic
store-owned one (store_owned_retain_for / writes_memory_rows_in_sql_for), and the reasoning holds
for any store that keeps memories outside SQL, so the comments now say that instead. Comment-only;
no behaviour change.

* bank list: take last_write_at and last_document_at from the store

A store-owned bank's row had neither: both come from MAX over SQL tables such a bank leaves empty,
and the response model drops null keys, so the fields vanished from the JSON rather than reading
null. The list is ordered by last write, so that ordering silently degenerated to created-at.

The two times are asked separately because ingestion time must not move when a document is
rewritten, and applied before the sort over all banks because the counters call is batched.

* retain: an oversized replacement diffs the whole document, and every slice may delta

Re-ingesting an oversized document re-extracted the parts that had not changed and tombstoned their
memories on the way. The retain is split into slices, and each slice diffed its own text against the
whole stored document, so every chunk merely absent from that slice classified as REMOVED.

Each slice now diffs the complete body, so the first does the real work and the rest find nothing
left to change. An APPEND is excluded: there the override holds only the new tail (the stored body
is prepended onto slice 1), so it keeps diffing the already-prepended contents, slice 1 only.

* maintenance: the best-effort txn reap survives a loop with no engine

The reap declares itself best-effort, but read `self._engine._backend` outside the try — so a
MaintenanceLoop constructed without an engine lost the whole tick to an AttributeError. Only a
store-owned deployment could reach it: the SQL stores return one line above.

* tests: mark the conn-seeded consolidation tests memory_backend_incompatible

These seed source liveness through a mocked `conn`. A store-owned bank never reads it: the
preflight asks the store, finds nothing, and short-circuits — so the test never reaches the
behaviour it is asserting (the fold, or the zero-length-embedding rejection), and reports a failure
about storage layout rather than behaviour. That is exactly what the marker is for; they still run,
and must, on Postgres.

* reflect: expand reads the chunk and the document from whichever store holds them

tool_expand routed its memories read to the memories store but left the chunk and document reads as
plain SQL against the chunks / documents tables. A store that owns the document store leaves those
empty by construction, so expand answered with the memory and silently without the chunk or the
document it was asked for — on real data, with no error to notice. Measured: 'expand returned no
document for <id>' with the memory itself present.

Chunks come back via get_chunk_texts, addressed by (document_id, index): chunk_id is
{bank_id}_{document_id}_{index} by construction, so the index is what remains once that known
prefix is removed — built from the ids in hand rather than by splitting on '_', which a bank or
document id containing one would break. Documents come back via get_document_record, taking
retain_params out of the record's metadata bag, which is where the store keeps them (serialised as
JSON, the same form _document_metadata_from_retain_params already parses from JSONB).

tests/test_reflect_expand_store_reads.py covers it through a real retain and a real connection: the
existing coverage seeds memories through a mocked connection a store-owned bank never reads, which
is why this went unnoticed. Those three are marked memory_backend_incompatible, pointing at the new
test.

* tests: mark the raw-SQL-seeded history and retraction tests memory_backend_incompatible

Each establishes its precondition by writing Postgres-internal state directly — INSERT INTO
memory_units for the observation, DELETE FROM memory_units to retract a grounding, UPDATE
memory_units.consolidated_at to mark facts pending. A store-owned bank keeps its memories
elsewhere, so those statements touch nothing, the precondition is never set up, and the test
reports a failure about storage layout rather than behaviour. They still run, and must, on
Postgres.

* transfer: export and import actually move a store-owned bank's documents

Three defects on the same seam, each silent.

export_bank/export_documents took memories=None and treated "no store supplied" as "SQL-backed".
The loaders read documents/memory_units directly, which an external document store leaves empty, so
the archive came back valid and EMPTY — document_count=0, fact_count=0 — with a success status.
This had already happened once and was fixed at the two call sites while the default that causes it
stayed; the default now asks for the store rather than assuming.

_resolve_target_id decided skip/replace/new-id from a SQL existence check alone, so for such a bank
every conflict mode went inert: skip re-imported the document it was told to leave alone, new-id
kept the original id, replace degenerated to a plain insert. It now asks whichever store holds the
document.

And the importer never wrote the document to the store at all — _store_document_bodies is the retain
path's document write and the importer called nothing equivalent, so a restore produced a bank
listing no documents. That write now happens before the connection is taken, as in retain: it is the
slow object-store write and does not belong in the write transaction.

Together the first and third meant backup and restore were both broken for a store-owned bank:
export produced an empty archive, import put the documents nowhere. test_document_transfer: 29/29.

* retain: sync document metadata onto surviving units for a store-owned bank

update_memory_units_metadata_and_tags exists so delta retain's optimised result matches a full
replace. Its SQL branch sets tags AND metadata; the store branch set tags only, so a surviving unit
kept the previous retain's metadata — measured on an append, older units still read
{"source": "email"} after a retain carrying {"source": "crm"}.

The metadata goes under META_METADATA_JSON as one JSON value, which is where the bag contract puts a
memory's user metadata and what every read reconstructs it from. Passing the dict flat instead
merges stray top-level keys into the record's own bag: applied, reported as applied, and invisible
to every reader. It also could not be told apart from a patch setting an internal bag field
(context, chunk_id, consolidation_failed_at) — an earlier attempt that mapped all patch metadata
into metadata_json inside the provider clobbered user metadata with consolidation_failed_at, which
is why the encoding belongs at the caller that knows which of the two it means.

Set unconditionally, mirroring SET metadata = $4: a document whose metadata was cleared must clear
on its survivors too, which an absent key would not do.

test_append_mode_metadata_consistent_for_unchanged_and_new_units passes.

* retain: a metadata-only delta relabels the memories too, not just the document

When a re-retain changes only tags or metadata, every chunk is unchanged and _delta_metadata_only is
the whole operation. Its store-owned branch re-put the document record with the new labels and
stopped; the SQL branch propagates them onto the units with
update_memory_units_metadata_and_tags. So a tags-only re-retain relabelled the document and left
every unit carrying the OLD tags and metadata — measured, v2 units still read ['team-a'] after a
retain carrying ['team-b', 'important'], with the retain reporting success.

test_delta_retain_tags_propagated_to_existing_units passes.

* tests: mark the raw-chunks-table delta assertion memory_backend_incompatible

It asserts through a raw SELECT on `chunks`, which a bank whose document store is external leaves
empty by construction — the chunks and their content hashes live in the store. The behaviour it
guards (chunks carry a content hash, so the delta diff can compare them) is exercised by the rest of
the module, which passes against both stores.

* tests: format the two new store-backed tests with the project's ruff

verify-generated-files runs scripts/hooks/lint.sh, which formats with `uv run --frozen ruff` — the
version the project pins, not whatever an unpinned uvx fetches.

* tests: mark the two documents-table retain assertions memory_backend_incompatible

Both assert by selecting from the Postgres `documents` table, which the fully store-owned retain
path does not write — the document lives in the store. The properties they guard hold there and are
covered: created_at preservation is enforced in the provider's put_document (it reuses the existing
record's created_at on re-ingest), and the document's existence is what every store-backed read in
this suite depends on.

test_streaming_chunk_batching_produces_same_facts is deliberately NOT marked: its failing assertion
goes through the public read API (list_memory_units total), not raw SQL, and reports 10 facts where
streaming returned 100 unit_ids. That is a real discrepancy and marking the file would bury it.

* retain: a streaming batch must not replace what the previous batch wrote

Silent data loss on any document large enough to split into consumer batches. The store-owned
streaming path decides per batch whether to replace the document's prior version, gated on
`is_first_batch` — which is a parameter of the whole retain_batch call and stays True for every
batch it produces. So batch 2 replaced batch 1, batch 3 replaced batch 2, and only the last batch's
facts survived. Measured on a ten-batch document: the retain returned 100 unit ids and the bank held
10.

Nothing surfaced: the retain reports every id it created, so the caller sees success.

The rule was already written down one line above ("replacing again would tombstone those siblings"),
and `doc_tracking_done` — the latch that can express "the first batch of THIS document" — was set
after the first batch and never read. The gate now consults it.

test_every_streaming_batch_survives_the_next_one guards it, asserting through the public read API so
it also runs against a bank whose document store is external, which is where the bug lived. It
checks the surviving ids and not merely the count: a replace leaving an equal number of DIFFERENT
memories would satisfy a count check. Verified load-bearing — reverting the gate reproduces
100-returned/10-held.

* tests: classify the last storage-layout failures against a store-owned backend

Seven tests that cannot run against a bank whose memories live outside SQL, each marked with the
specific statement that cannot apply: raw INSERTs into memory_units / documents (directly or via the
_insert_* helpers), assertions selecting from chunks / entities / unit_entities, a race forced by
writing documents.updated_at, and the in-transaction FOR SHARE liveness skip.

test_consumer_failure_cancels_in_flight_extractions is the odd one: it injects the consumer failure
through an exploding Postgres pool, and the store-owned write path is PG-free and never acquires
from it — so the failure never fires, the deliberately-hanging extraction is never cancelled, and
the test burned 600s of every run before timing out.

Where the property still matters for a store-owned bank it is noted as covered: the stale-retain
race is fenced by the store's compare-and-set on the document watermark, which
test_concurrent_appends_keep_every_turn exercises.

* tests: classify the streaming-batch SQL assertions, keeping the public-API guard separate

test_streaming_chunk_batching_produces_same_facts also reads documents/chunks directly. The property
worth guarding — a multi-batch streaming retain keeps every batch's facts — now lives in
test_every_streaming_batch_survives_the_next_one, which asserts through the public read API and
therefore runs against a store-owned bank. That is the assertion that caught the data loss.

* retain: keep delta on the first sub-batch, and latch the replace where it happens

Two fixes from review of the delta change.

1. The gate is narrowed back to `is_first_batch`. Letting every slice of an oversized item run delta
changed the Postgres path, not just a store-owned one, and three things downstream assume the narrow
gate: the caller keeps one result list per sub-batch item (`sub_origins` is length 1 for an oversized
slice, so a multi-chunk delta's extra unit ids are dropped), `chunk_index_offset` advances by the
splitter's per-slice count rather than by what a delta wrote, and a brand-new oversized document
would extract its whole tail in one step — the bound the sub-batch splitting exists to keep. The
full-body DIFF stays: slice 1 must compare the whole body against the stored chunks, or every chunk
merely absent from that slice classifies as REMOVED.

2. `doc_tracking_done` does not mean "the replace was issued". It is set even when a batch wrote zero
units, and in that case `provider.retain` was never called — so batch 1 could replace nothing, latch,
and leave the document's prior version standing beside the new memories for every batch after it.
That state is reachable: `had_extracted_facts` is true when a chunk reports facts even though the
degenerate-text guard rejected all of them. A dedicated `doc_replace_done`, set where the replace is
actually issued, is what the gate now reads.

Known consequence, no regression: with the gate narrowed,
test_oversized_replacement_still_skips_unchanged_chunks fails again on a store-owned bank, as it does
on main. Routing recovery detection to the store does NOT fix it — measured: for such a bank
`_store_document_bodies` writes the document record and the full chunk-text list up front, so the
check matches its own write on the first slice (stored=492a32c9 new=492a32c9). The store's chunk
texts are a body upload, not a record of committed facts; the equivalent signal is which chunk ids
already have memories.

* transfer/bank-list: cover the store-routing fixes, and stop the loop lying about its engine

Review items 4 and 5.

Tests, on the existing store-owned harness so they run on Postgres CI rather than only against a
real store — which is where a regression would otherwise surface as a bank that restores with no
documents:

* export_bank called WITHOUT `memories=` must still route to the store. Asserted on archive
  contents, because a valid EMPTY archive with a success status is exactly what the broken version
  produced. Patches `get_memories` rather than `_resolve_memories`, so the resolution under test
  actually runs.
* _resolve_target_id must ask whichever store holds the document, for all three on_conflict modes
  plus the absent case. SQL-only, every mode went inert: skip re-imported, new-id kept the id,
  replace became an insert.

Both verified load-bearing: reverting the fixes reproduces `document_count: 0, fact_count: 0` and
the SQL path respectively.

MaintenanceLoop declared `engine: "MemoryEngine"` while the cadence tests construct it with None.
The annotation is widened and says why. The comment there also overstated the damage: `_run` catches
per tick and this reap is the tick's last job, so the cost was a logged traceback, not lost work.

* review nits: chunk ordering, duplicate chunk reads, and two stale claims

* test_append_coalesces ordered chunk ids lexicographically to find 'the last chunk', which picks
  the wrong one from ten chunks on (_10 sorts before _2). Ordered by parsed index.
* tool_expand asked the store for the same chunk once per memory sitting in it; the SQL branch
  collapses those through = ANY($1). Deduped.
* The _try_delta_retain comment still opened 'Delta is DISABLED for a store-owned bank', which
  stopped being true when the store-side CAS replaced the row lock. Rewritten to describe what
  replaced what, and why leaving delta off was not free.
* Dropped sort_keys so the metadata_json write matches base.py's.

* tests: pass the replace latch to the ext writer

_streaming_batch_write_ext gained a required `doc_replace_done` and these tests call it directly.
Threaded through the shared kwargs helper rather than given a default: a default would reset the
latch per batch, so a caller that forgot it would silently replace on every batch — the data loss
the latch exists to prevent.