fix(retain): queue retain.completed webhook on boundary and zero-fact batches

[nicoloboschi]

Summary

The transactional-outbox callback that queues the retain.completed webhook delivery only fires inside the final facts-bearing batch's write transaction (is_last=True). Two successful retain paths never reach that fire, so the webhook delivery is silently dropped — no error, no log, no retry — even though the memory data is stored correctly.

This is a lost notification, not lost data: facts, chunks, entities and the document are all written. But a caller relying on retain.completed to learn "my ingest finished" (advance a pipeline, mark a job done, trigger downstream indexing) never hears back.

The two drop paths (in _streaming_retain_batch)

The streaming consumer drains extracted chunks in batches of retain_chunk_batch_size (default 100). Full batches flush with is_last=False; only the leftover partial batch drained by the queue sentinel is marked is_last=True, and the callback is threaded into the write TXN at exactly one place (outbox_callback=outbox_callback if is_last else None).

1. Exact chunk-batch boundary. When the committed-chunk count is an exact multiple of retain_chunk_batch_size, there is no leftover — the last full batch already flushed as is_last=False, and the sentinel drains an empty batch (guarded by if batch and not pipeline_aborted[0]), so is_last=True is never passed. A document that extracts into exactly 100/200/300… chunks loses its webhook; 199 or 201 fire fine.

2. Zero-fact final batch. _process_db_batch returns early when a batch extracts no facts, before reaching the fact-insert call site that carries the callback. Any retain whose final (or only) batch yields no facts — common for boilerplate content — never queues its delivery, regardless of is_last.

There is no backstop: the delivery row is only ever inserted by this callback, and the worker poller re-delivers rows that exist — a never-inserted row is gone.

Fix

Track whether the callback fired in-TXN (outbox_fired). On any successful, non-aborted retain that didn't fire it, queue the delivery exactly once in a dedicated transaction after the consumer loop — the same place the existing no-batch document-tracking fallback already runs. This covers both drop paths (and the all-chunks-skipped case) without having to reason about why is_last didn't fire, which is what made a chunk-counting approach fragile. Concurrent-takeover (pipeline_aborted) retains are skipped so an aborted request never emits a completion event.

Verification

Two regression tests, both fail with 0 deliveries on main and pass with the fix, asserting exactly one retain.completed delivery through the real retain pipeline + a real WebhookManager:

• test_retain_fires_outbox_on_chunk_batch_boundary — retain_chunk_batch_size=1 makes every retain land on a boundary, reproducing the drop deterministically.
• test_retain_fires_outbox_when_final_batch_extracts_zero_facts — extraction stubbed to return no facts.

Full tests/test_webhooks.py, test_delta_retain.py, test_large_document_replacement.py, test_async_batch_retain.py green (92 passed). ruff + ty check clean.

Scope

Deliberately minimal — this fixes only the confirmed outbox drop. A separate, lower-priority cost issue (an oversized replace whose slice-1 prefix changes re-extracts trailing history one time via the removed_indices deletion path) is not in scope here; it produces correct state and is a cost optimization, tracked separately.

Supersedes #2728, which diagnosed a different mechanism and whose is_last fix counted against total_chunks rather than queued chunks (so it didn't fire on the recovery/skip path).

[koriyoshi2041]

The two regressions are real, but the fallback appears to reintroduce the same silent-loss class across a smaller crash window. On the boundary path, the final facts batch commits first; the delivery row is inserted only afterward in a separate transaction. A process exit between those commits leaves the retain stored with no retain.completed row to retry, which is exactly what the transactional-outbox coupling was meant to prevent.

Could the boundary case instead identify the final queued batch before its write transaction commits (for example with sentinel/lookahead semantics), and could the zero-fact early-return path invoke the callback inside the transaction that records the successful document/no-fact outcome? A crash-point regression or fault-injection test between the data commit and fallback would make the atomicity requirement explicit. The current tests prove eventual insertion on an uninterrupted run, but not the outbox invariant under interruption.