kbSync checkpoints up to 33× later than the fairness bound it must respect

Context

#16817 proved the heavy-maintenance fairness bound is cooperative: shouldYieldHeavyMaintenanceLease is a pure predicate the holder must choose to call, and withHeavyMaintenanceLease has no timer, no abort signal and no watchdog. A holder that never checkpoints is never preempted.

The obvious next question is whether kbSync — the holder observed on #16566 keeping the slot 13 hours — is such a holder. It is not. VectorService.embedChunks consults shouldYield() between outer batches (ai/services/knowledge-base/VectorService.mjs:644), the predicate is threaded live from ai/scripts/maintenance/syncKnowledgeBase.mjs:121, and completed batches are durably upserted into the shadow so a yield resumes rather than restarts.

So the checkpoint exists, fires, and preserves progress. What no one has multiplied is how far apart two consecutive checkpoints can be.

The Problem

The interval between two shouldYield() consultations is a product of four independently-declared leaves that no single site reads together:

Worst case between two consultations:

(1 + unloadRetryCount) x batchEmbeddingTimeoutMs      =  4 x 300s   =    20 min   per provider chunk
ceil(batchSize / batchEmbeddingChunkSize)             =  ceil(50/5) =    10       chunks per embedTexts call
maxRetries                                            =              x   5        attempts per outer batch
                                                                      -----------
                                                                        1000 min  =  16 h 40 min

Against a fairness bound of 30 minutes. A ratio of 33×. The observed 13-hour hold on #16566 sits inside that analytic bound.

Three mechanisms compose, none of them individually wrong:

• ai/services/memory-core/TextEmbeddingService.mjs:1109 — #embedOpenAiCompatibleBatch loops over ceil(N / batchEmbeddingChunkSize) provider chunks. It has a natural checkpoint at operation.phase = 'batch-yield' (:1129) and consults nothing there.
• ai/services/memory-core/TextEmbeddingService.mjs:888 — each chunk re-posts up to unloadRetryCount times on a model-load failure, each attempt carrying the full requestTimeoutMs. The leaf's own comment says the timeout exists so a request "must not hold the provider queue forever" — that is a per-request contract, and it is honoured. Nothing states an operation-level one.
• ai/services/knowledge-base/VectorService.mjs:686,712 — the outer retry loop is a bare catch (err), so a timeout is retried like any other error, multiplying the whole inner cost by maxRetries.

A cooperative bound whose checkpoint interval exceeds the bound is not a bound. maxActiveHoldMs can be tuned to any value below 16 h 40 min and change nothing observable, because the first opportunity to honour it may not arrive until after it.

The Fix

The checkpoint that has to fire already exists one call-frame too high. Move the consultation to the boundary that is already there, and make the resulting inequality executable.

1. Consult the yield predicate at the inner chunk boundary. embedChunks already owns shouldYield as an injected predicate; thread it through embedTexts into #embedOpenAiCompatibleBatch and consult it at the existing batch-yield point. Sharing a predicate is ordinary reuse, not ADR-0019 B5 — no config value is threaded.
2. Abandon with a typed error, not a partial array. A partial embedding array would silently misalign with collection.upsert's ids. The inner loop throws a distinguishable yield error carrying how many chunks completed.
3. embedChunks must treat that error as a yield, never as a retry. Today's bare catch would swallow it and burn all maxRetries attempts — turning the fix into a 5× amplifier. This is the one place the repair can go wrong, and it needs its own fixture.
4. Make the inequality executable. A spec that computes (1 + unloadRetryCount) x batchEmbeddingTimeoutMs from the live leaves and asserts it is strictly less than maxActiveHoldMs. A future leaf change that reopens the gap fails CI instead of being discovered in a 13-hour hold.

After the repair the worst-case interval is one provider chunk — 20 min against a 30 min bound.

Acceptance Criteria

• A fixture proves the yield predicate is consulted at the provider-chunk boundary, not only between outer batches: with a predicate that turns true after chunk 1, a multi-chunk embedTexts stops without issuing the remaining chunk requests.
• A fixture proves the yield error is not counted as a retry by embedChunks — the outer loop must not re-attempt, and yielded must be reported to the caller so the lease is released.
• A fixture proves a yield preserves durably-upserted progress: chunks embedded in prior outer batches remain in the shadow and are skipped on the next sweep by selectResumableChunks.
• An executable invariant asserts (1 + unloadRetryCount) * batchEmbeddingTimeoutMs < maxActiveHoldMs against the resolved leaves, and it fails when any of the three is moved to reopen the gap.
• Negative control: a sweep whose predicate never returns true completes every chunk and every outer batch unchanged. A yield that fires on legitimate work is worse than none.
• Negative control: the yield does not fire before the first provider chunk of an acquisition, preserving embedChunks's existing forward-progress guarantee (at least one unit lands per lease).
• Coverage fails against today's code and passes against the repair.

Out of Scope

• The re-embed ratio, the declared in-flight concurrency, and the public-surface disproportion (#16780 AC-3/AC-5/AC-7) — the reporting half. This ticket is the bound. #16780 stays open for them.
• Why tenant ingestion fails at the embed stage, and the starvation it causes — #16566.
• Preemption at the lease layer — rejected on #16817/PR #16818: it abandons a holder mid-batch, and the whole reason a checkpoint is the right instrument is that the resume store already exists.
• Retuning maxActiveHoldMs, batchSize or maxRetries. The defect is that the interval is unbounded relative to the bound, not that any single leaf holds a wrong number.

Avoided Traps

• Reading "the holder never checkpoints" from #16817. It does checkpoint. The measurement had to be the interval, not the presence — and the presence is what a grep for shouldYield shows you.
• Putting the deadline on the lease. Already falsified on #16818: a mutex-level timeout abandons a holder with no resumable checkpoint. The checkpoint is the instrument precisely because kbEmbeddingResumeStore exists.
• Putting a wall-clock deadline on the whole batch. A full 65k-document corpus is legitimately hours of work at a healthy 0.86 s/chunk. A total-duration deadline converts a slow-but-progressing sync into one that never finishes — the alarm-on-the-absolute trap #16780 already names, one layer down.
• Treating the bare catch as incidental. It is the mechanism that turns a correct yield into a 5× retry storm, so the fix is incomplete without it.

Related

#16817 / PR #16818 (the fairness bound is cooperative — the direct predecessor) · #16780 (the missing bound and undeclared concurrency — parent; the reporting half stays there) · #16566 (the 13-hour kbSync hold, and the ingestion failure itself) · #16561 (the lease had no fairness — where maxActiveHoldMs came from) · #16690 (KB ingest awaits embedding inline where Memory Core defers) · #16706 (deployment-readiness tracker)

Origin Session ID: 87f453f9-aa80-4487-9ed1-b5d91e052c43

Retrieval Hint: kbSync checkpoint interval fairness bound shouldYield provider chunk boundary unloadRetryCount batchEmbeddingTimeoutMs maxActiveHoldMs 33x