LearnNewsExamplesServices
Frontmatter
id16843
titleOne failing embed batch permanently strands every chunk after it
stateClosed
labels
bugai
assigneesneo-opus-grace
createdAtAug 10, 2026, 1:22 AM
updatedAtAug 10, 2026, 4:22 AM
githubUrlhttps://github.com/neomjs/neo/issues/16843
authorneo-opus-grace
commentsCount0
parentIssuenull
subIssues[]
subIssuesCompleted0
subIssuesTotal0
contentTrust
projected
quarantined0
signals[]
blockedBy[]
blocking[]
closedAtAug 10, 2026, 4:22 AM

One failing embed batch permanently strands every chunk after it

neo-opus-grace
neo-opus-grace commented on Aug 10, 2026, 1:22 AM

Context

Found while tracing why an external plane's Knowledge Base holds count: 0 (#16706). That plane's cause is upstream of this — its provider has never returned a usable embedding — but the trace surfaced a separate, independent defect in the live tenant-ingest path that bites any plane whose provider works most of the time.

Two competing explanations for an empty corpus were tested against source and both eliminated, which is what left this one standing:

  • "a first sync is one all-or-nothing embed call"false: VectorService.embedChunks sub-batches on batchSize and upserts each completed batch;
  • "finished batches are staged and discarded on failure"false: tenant ingest runs deleteStale: falseSTALE_STRATEGY_SKIPshouldShadowSwap is false, so it writes to the canonical collection directly.

Partial progress therefore is durable. The defect is not that progress is lost — it is that progress cannot resume past a batch that fails.

The Problem

embedChunks retries a failing batch maxRetries times and then throws, aborting the sweep:

} else {
    throw new Error(`Failed to process batch ${i / batchSize + 1} after ${maxRetries} retries. Aborting.`);
}

That alone would be survivable if the next sweep could step over the bad batch. It cannot, because the work set is order-preserving and success-derived: chunksToProcess is built by walking expandedKnowledgeBase in corpus order and keeping every chunk whose id is not already in the collection. Batches that succeeded are excluded next time; the batch that failed is still first in line.

So for a batch that fails deterministically — one chunk the provider rejects, a single oversized payload that slips the guardrail, one malformed record — the steady state is:

sweep behaviour
1 batches 1..N-1 upserted · batch N fails maxRetries · throw
2 1..N-1 excluded (already present) · starts at N · fails · throw
identical, forever

Chunks after batch N are never attempted — not "delayed", never. The corpus silently freezes at a partial state, and every subsequent sync re-pays the full retry cost of the poison batch before aborting at exactly the same place. No status field distinguishes this from "up to date".

The forward-progress guarantee exists and does not cover this. The cooperative-yield path was explicitly hardened for exactly this hazard — its own comment reads "a holder that yields at the same chunk every time never advances", and it persists the completed prefix plus a resume marker so the next sweep continues. The failure path got no equivalent. One arm of the same loop guarantees advancement; the other guarantees repetition.

The Architectural Reality

  • ai/services/knowledge-base/VectorService.mjsembedChunks({collection, chunksToProcess, shouldYield}): the batch loop, its retry arm, and the terminal throw.
  • Same file, embed(): derives existingIds (paginated, scoped to the corpus the call owns) and builds chunksToProcess in corpus order — the mechanism that makes the failing batch permanently first.
  • ai/services/knowledge-base/IngestionService.mjsembedChunkGroups() catches the throw per repoSlug group and continues, so one repo's poison batch does not stop other repos. The blast radius is one tenant repo's corpus, which is why this can sit undetected.
  • selectResumableChunks / decideResume (helpers/resumableEmbedding.mjs) serve the shadow-swap path only (embedViaShadowSwap). Tenant ingest does not take that path, so no resume-marker logic applies here.

The Fix

Distinguish a poisoned batch from a dead provider, using state the loop already has — no new configuration.

On maxRetries exhaustion:

  • if embeddedCount > 0 — the provider demonstrably works this sweep, so this batch is the problem. Record a structured failure entry and continue to the next batch.
  • if embeddedCount === 0 — nothing has succeeded at all; the provider is down, not poisoned. Stop the sweep (preserving today's behaviour exactly, which is the right behaviour for that case: continuing would spend batches × maxRetries × timeout against a dead provider).

Return the failures rather than swallowing them: embedChunks gains failedBatches on its result, embed() surfaces the count and reasons in its return payload, and IngestionService.embedChunkGroups folds them into summary.errors so they reach the durable ingest receipt.

Why the zero-success signal rather than a threshold leaf: it is derived from what the sweep has already proven, needs no new config surface, and encodes the actual discrimination — has this provider embedded anything at all? A count-based threshold would need a value nobody can justify and would re-introduce a hidden default.

Contract Ledger Matrix

Target Surface Source of Authority Proposed Behavior Fallback Docs Evidence
embedChunks() return VectorService adds failedBatches: Array<{batchIndex, chunkIds, reason}> existing {embedded, skipped, yielded} consumers unaffected — additive method JSDoc spec: poison batch mid-corpus yields a populated failedBatches and a completed remainder
embedChunks() control flow VectorService continue past a failed batch iff embeddedCount > 0; otherwise stop zero-success case is byte-identical to today inline rationale at the branch spec: zero-success sweep stops at the first failing batch, does not walk the corpus
embed() return VectorService surfaces failed count + reasons additive alongside embedded / deleted method JSDoc spec: reasons survive to the caller
ingest summary IngestionService.embedChunkGroups folds failedBatches into summary.errors unchanged when empty method JSDoc spec: a poisoned batch appears in the durable receipt

Decision Record impact: none — no config leaves added; the discrimination is derived from loop state, deliberately avoiding a new AiConfig surface.

Acceptance Criteria

  • A batch that fails maxRetries after at least one successful batch no longer aborts the sweep; every subsequent batch is still attempted and upserted. Receipt: VectorService.batchFailureIsolation.spec.mjsa poisoned batch is skipped and every LATER batch still embeds (embedded is 100 of 150; chunk-149 present, chunk-50 absent).
  • A sweep where the first batch fails maxRetries stops immediately, exactly as today — the dead-provider case must not become a full-corpus retry walk. Spec asserts the provider call ceiling. Receipt: same spec — CEILING: a sweep where NOTHING embeds still throws, and stops at the first batch asserts providerCalls === 1 against a 3-batch corpus, and that it rejects rather than returning.
  • Failed batches are reported, not swallowed: embedChunks returns them, and they reach summary.errors through embedChunkGroups. Receipt: failedBatches[0] carries batchIndex, chunkIds and reason in the poisoned-batch control; IngestionService.embedChunkGroups folds each into summary.errors as KB_EMBED_BATCH_SKIPPED.
  • Resumption is proven across sweeps, not within one: a second sweep over a corpus containing a deterministically-poisoned batch embeds the chunks that follow it. Receipt: CROSS-SWEEP control — after sweep 1 the collection holds chunk-100 and chunk-149, which the pre-fix tree never writes on any sweep. Correction found while writing it: sweep 2's work set is the poisoned batch alone, so nothing embeds in it and it correctly throws — indistinguishable from an outage from inside that sweep, and loud rather than silent. The spec now asserts that steady state explicitly instead of assuming a clean second sweep.
  • Mutation control, both directions: reverting the continue to throw reddens the cross-sweep resumption spec; removing the zero-success guard reddens the dead-provider ceiling spec. Neither may pass against the current tree. Receipt: both run. embeddedCount === 0true (restore the unconditional abort) reddens the poisoned-batch/cross-sweep controls; → false (delete the guard) reddens the CEILING control. Each mutation reddens its own expected control and no other.
  • Cooperative-yield behaviour is untouched — the existing yield specs stay green, and a yield is still not treated as a failure. Receipt: a cooperative yield is NOT recorded as a batch failure (yielded: true, failedBatches empty), plus the full test/playwright/unit/ai/services/knowledge-base/ directory at 572 passed, which includes the pre-existing VectorService.leaseYield.spec.mjs driving this same loop.

Out of Scope

  • The reason count: 0 happens on a starved provider — that is #16830 / #14154, and this ticket cannot fill an empty corpus.
  • The shadow-swap resume path (selectResumableChunks / decideResume) — a different path with its own working resume logic.
  • Classifying why a batch failed into terminal-vs-retryable fate classes. #16227 owns that contract for the rebuild runner; this ticket only stops one failure from stranding the remainder.
  • Any change to batchSize, maxRetries, or timeout leaves.

Avoided Traps

  • "Just don't throw" — removing the throw without the zero-success guard turns a dead provider into a full-corpus walk at maxRetries × timeout per batch, which is far worse than today's fast abort. The guard is the load-bearing half.
  • Adding a consecutive-failure threshold leaf — needs a value nobody can defend, and a hidden default is precisely the ADR-0019 shape to avoid. The sweep already knows whether the provider has ever answered.
  • Testing within a single sweep — the defect is that the next sweep re-selects the same failing prefix. A one-sweep spec passes against the broken tree.
  • Treating a yield as a failure — the yield arm is a decision, not an error, and already carries its own persistence and resume semantics. Conflating them would re-break the fairness fix.

Related

  • #16706 — the epic whose empty-corpus investigation surfaced this; the ingest-path elimination is recorded in its operator runbook.
  • #16227 — bounded-retry / failure-receipt contract for the one-off rebuild runner; adjacent concern, different surface, and its spec work is a useful reference for the failure-entry shape.
  • #16822 / #16823 — the cooperative-yield checkpoint work on this same loop; this ticket closes the sibling gap on the failure arm.
  • #16830, #14154 — why a provider returns nothing in the first place.

Live latest-open sweep: checked the latest 20 open issues at 2026-08-09T23:05Z plus a 30-message all-read-state A2A claim sweep; no equivalent filed or in-flight.

Structure map: N/A — modifies ai/services/knowledge-base/VectorService.mjs in place; no file created or relocated.

Origin Session ID: d8332b13-5d97-4839-ac11-d2de4602a989

Retrieval Hint: query_raw_memories("embed batch failure strands remainder corpus order existingIds forward progress")

tobiu referenced in commit a07f6c0 - "fix(ai): one failing embed batch no longer strands the remainder (#16843) (#16844) on Aug 10, 2026, 4:22 AM
tobiu closed this issue on Aug 10, 2026, 4:22 AM