perf(codemap): reduce manifest write amplification during indexing

[morluto]

Problem

Each completed codemap update previously created an independent queue item and manifest publication.

During indexing bursts, the backlog amplified work through middle insertion, removeFirst() shifting, repeated pending-change and waiter scans, repeated manifest decoding, and one publication per completion. Split session and projection queues also made admission order and retry behavior difficult to reason about.

Part of #466.

Approach

Batch at the writer, the narrowest layer with the ordering, namespace, authority proof, revision, and waiter information needed to combine mutations safely.

A single FIFO preserves admission order. Only a compatible contiguous prefix may be batched, so newer work cannot pass an older failed mutation. Batches remain bounded to avoid unbounded memory use or writer monopolization.

Changes

• Replace split arrays with one admission-ordered, head-index FIFO.
• Batch contiguous mutations with the same namespace, work key/session, and compatible authority proof.
• Revalidate writer authority before publication.
• Bound batches to 64 mutations and 8 MiB.
• Keep the latest mutation for duplicate paths within a batch.
• Advance persistence through the highest absorbed revision.
• Resolve every covered waiter exactly once.
• Retain failed work as an ordered prefix for a later admission without spinning.
• Add cooldown behavior after repeated invalid-manifest regeneration.
• Add typed manifest validation and quarantine attribution.
• Remove duplicate in-flight revision and waiter work-key state.

Performance effect

A compatible group of up to 64 mutations or 8 MiB can use one manifest publication instead of one publication per mutation.

The admission queue advances a head index rather than shifting the remaining array on every dequeue. Waiters are resolved through their covered work instead of repeatedly filtering a global collection. This removes queue, decoding, waiter, and publication amplification as indexing work accumulates.

Testing

Coverage includes FIFO ordering, compatible batch boundaries, duplicate paths, absorbed revisions, failed batches, retained work, cooldown recovery, cancellation, unload, and exact waiter completion.

• CodemapBindingEngineManifestWriteTests: 10 passed
• CodeMapRootManifestStoreTests: passed
• SwiftFormat and SwiftLint strict: passed
• RepoPrompt product build: passed

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[sentry]

Bug: The removal of logic to merge pipeline.pendingManifestChanges into the write batch causes pending mutations from previous, incompatible work items to be silently dropped instead of being written to the manifest.
_{Severity: CRITICAL}

Suggested Fix

Restore the logic that merges pipeline.pendingManifestChanges into the changes dictionary before writing to the manifest. This ensures that mutations from previous revisions that were not included in the current batch are not lost.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location:
Sources/RepoPrompt/Infrastructure/WorkspaceContext/WorkspaceCodemapBindingEngine.swift#L6732-L6733

Potential issue: In the `runManifestWriter` function, the logic to merge older
`pendingManifestChanges` from the pipeline into the current write batch has been
removed. Previously, changes from items that couldn't be batched together due to
incompatible attributes (like `workKey`) were preserved in
`pipeline.pendingManifestChanges` and merged during the next write. The new
implementation only writes `batch.changesByPath`, which exclusively contains changes
from the current batch. This leads to silent data loss, as pending mutations from
previous revisions that were not part of the current batch are dropped, resulting in an
incomplete and stale on-disk manifest.

_{Did we get this right? 👍 / 👎 to inform future reviews.}

[baron]

Intent Analysis

Problem: Issue #466: bursty CodeMap binding causes manifest-write amplification (one durable publication per queued mutation, per-item queue shifting), generic decode-failure attribution, and no backpressure on repeated semantic manifest corruption.
Before → After: Per-item priority queue with one publication per mutation → admission-ordered head-index FIFO coalescing up to 64 compatible consecutive revisions per batch (item- and byte-capped), grouped waiter completion, new batch counters, typed decode-failure classification, and repeated-corruption regeneration backpressure. Store-wide scan amplification is deliberately deferred to stacked PR #488.
Evidence: Two independent code-first inspections at head e69d5bf converge on the same regions: batch construction/compatibility at WorkspaceCodemapBindingEngine.swift:~6566–6651, failure handling at ~6822–6850, decode classification at CodeMapRootManifestModels.swift:~797–914, backpressure at CodeMapRootManifestStore.swift:~1420–1477; focused tests cover batching, splitting, limits, failure, ordering, unload, and corruption attribution. All ten CI checks are green; PR is MERGEABLE.
Confidence: High on intent and normal-path design — both reviews derived it from the diff and it matches #466 acceptance criteria.
Red Flags: Both reviews independently found a defect in the newly rewritten failure path (below); no near-capacity performance measurement backs the amplification-reduction claim (non-blocking).

Findings

1. High (blocking) — Failure path parks accepted mutations in deferredWork with no autonomous recovery, and retention is unbounded/unaccounted.
References: WorkspaceCodemapBindingEngine.swift:6822–6850 (failure handler moves the failed batch plus the entire queued suffix into deferredWork, clears task/identity, fails all waiters), :6566–6587 (deferred queue restored only on a future enqueue), :6538–6563 (byte/count reservations released while items remain retained). Two facets of one root cause:

• Stall: After any publication error — including a one-shot transient — nothing autonomously restarts the deferred queue. If queued work existed outside the failed batch (reachable via the 64-item cap, byte cap, or incompatible proofs) and no later mutation arrives, accepted mutations stay memory-only indefinitely; unload/restart discards them, leaving the durable manifest incomplete. The pre-PR writer continued draining after failure, so this is a regression in the persistence contract.
• Unbounded retention: Under persistent failure, each new submission prepends the full deferred history, retries, fails, and re-parks after its accounting reservation was released; same-path revisions are not compacted until a batch dequeues, so retained state grows monotonically outside declared resource bounds.

Test gap: the existing failure test (CodemapBindingEngineManifestWriteTests.swift:252–~320) absorbs all queued successors into the failed batch, so it cannot detect either facet.

Required resolution: define a recovery policy for deferredWork (continue draining, bounded/scheduled retry, or explicitly observable + durable abandonment) and keep deferred items budget-charged or compact superseded same-path revisions, preserving FIFO/authority semantics. Add regression tests: (a) one-shot failure with a split/incompatible queued successor, then unload/reload verification; (b) repeated failure with same-path and distinct-path submissions asserting retained items/bytes plateau and eventual recovery drains correctly.

2. Informational — Sentry "silent drop" warning not substantiated. Both independent inspections refute the claim that removing the pendingManifestChanges merge silently loses incompatible items on the normal path: each queued item retains its mutations, strict FIFO prevents incompatible later work from bypassing older batches, and failures preserve the ordered prefix in deferredWork. The real defect is Finding 1.

Maintainer-guidance check

• User impact/invariant: Bursty indexing must reduce publications without losing, stranding, or unboundedly retaining accepted manifest mutations — violated on the failure path.
• Root-cause confidence: Confirmed from the code path by two independent reviewers at the same head commit.
• Authority: WorkspaceCodemapBindingEngine owns sequencing/retry policy (where the fix belongs); CodeMapRootManifestStore remains durable-publication authority, unaffected.
• State-safety: Success path (batching, FIFO, invalidation, waiter cancellation, unload, corruption attribution) is well covered; failure path is under-covered and defective as described.
• Scale/observability: Batch counters are good; deferred work needs a retry/status/accounting surface alongside the fix.
• Scope: Fix Finding 1 here — it is the failure semantics this PR introduces; keep full-store reconciliation in #488. No scope creep elsewhere; batching, diagnostics, and quarantine backpressure all trace to #466.
• Validation boundary: The two regression tests above plus the existing focused suite; CI already green. Near-capacity profiling is a desirable non-blocking follow-up.

Verdict

REQUEST_CHANGES. The batching/FIFO design is sound, CI is fully green, and the automated silent-drop warning is refuted. But two independent code-first reviews converge on the same deterministic defect in the newly introduced failure path: a failed publication strands the failed batch and all queued successors in deferredWork with no autonomous drain (a durability regression vs. pre-PR behavior) and no retention bound or accounting under repeated failure — with a demonstrated test blind spot. Fix and regression-test the deferred-work recovery policy in this PR; everything else can proceed as-is.

[morluto]

Rebutting the Sentry “silent drop” warning on this PR.

The warning is not substantiated. The manifest mutations are never silently discarded on the success path:

• WorkspaceCodemapBindingEngine keeps one admission-ordered FIFO; all queued work retains its mutations and the strict FIFO guarantees an older, incompatible batch is never bypassed by a later batch.
• deferredWork preserves the failed batch and any queued suffix in publication order.
• Same-path revisions are compacted inside the pendingManifestChanges map per ManifestMutationBatch build.

The real issue was the failure path, not silent dropping. The recently-pushed commit f77d1382 addresses that:

• deferredWork is now drained by an autonomous retry task with a bounded backoff.
• maximumManifestWriterDeferredAttempts drops the oldest batch after repeated failures.
• maximumManifestWriterDeferredItemCount caps total retained deferred items.
• Dropped items properly detach waiters, remove matching pendingManifestChanges, and clear the dirty pipeline state when no pending changes remain.
• cancelAllManifestWriters cancels the retry task on unload.
• Regression tests cover one-shot recovery with reload, repeated transient recovery, and bounded abandonment after the max failure budget.

The Sentry warning should be considered resolved; the follow-up needed here is the failure-path recovery/retention policy, which is now implemented.

[morluto]

Addressed Baron finding 1 in commit f77d1382: manifest writes now retry autonomously with bounded exponential backoff, preserve FIFO ordering and reservation accounting while deferred, compact superseded same-path work, and expose bounded retry/retention diagnostics. Added split-successor and repeated-failure/plateau/recovery regressions in CodemapBindingEngineManifestWriteTests. Push preflight passed; a fresh-worktree focused daemon run was attempted but canceled after the SwiftPM command stalled without test output, so hosted checks are the remaining validation authority.

[morluto]

Fresh exact-head CI exposed a retry race in CodemapBindingEngineInvalidationTests: autonomous recovery can clear dirtyManifestCount before the assertion. Fixed in pushed commit 26ae4569 by allowing either the observable dirty state or already-completed recovery, while retaining the failure-count and overlay-readiness assertions; also applied targeted SwiftFormat cleanup. Commit/push preflights and targeted formatter lint passed. Fresh hosted checks are now running on 26ae4569.