LearnNewsExamplesServices
Frontmatter
id16599
titleMerge keyed on a content digest silently duplicates the corpus
stateClosed
labels
bugaiarchitecture
assigneesneo-opus-vega
createdAtAug 6, 2026, 9:07 PM
updatedAtAug 7, 2026, 10:35 AM
githubUrlhttps://github.com/neomjs/neo/issues/16599
authorneo-opus-vega
commentsCount0
parentIssuenull
subIssues[]
subIssuesCompleted0
subIssuesTotal0
contentTrust
projected
quarantined0
signals[]
blockedBy[]
blocking[]
closedAtAug 7, 2026, 10:35 AM

Merge keyed on a content digest silently duplicates the corpus

Closed Backlog/active-chunk-13 bugaiarchitecture
neo-opus-vega
neo-opus-vega commented on Aug 6, 2026, 9:07 PM

Context

ai:restore --mode merge was run twice on 2026-08-06 during the corpus-loss incident (#16549): once into a disposable collection (completed, 59,754 rows), and a second run into the canonical collection was prepared but not executed. Measuring what that second run would have produced is what surfaced this.

Observed, by fetching every id and every metadata row from both collections (17,002 + 59,754 — no sampling):

natural keys present in BOTH collections   15,520
  ...same id (id already present)           7,900
  ...DIFFERENT id                           7,620
rows a --merge would have added as duplicates  7,655
post-merge total   68,856 rows for 61,103 distinct chunks  = 1.13 rows per chunk

A "natural key" here is {tenantId, repoSlug, source, name, type} — chunk identity independent of content.

Inference, stated separately: those 7,620 are one logical chunk appearing twice. Verified directly on one file rather than assumed — src/component/Base.mjs holds 50 chunks in each collection, 47 names in both, ids differing, so a merge yields 97 rows for a 50-chunk file.

The divergence itself has a known cause under investigation on #16549 (an extends value absent from one side, and extends is a hash input). That cause is not this ticket. This ticket is that --mode merge cannot see the condition at all, and reports success while creating duplicates.

The Problem

The KB's chunk id is a content digest, and --mode merge treats it as a stable identity.

createContentHash (ai/services/knowledge-base/DatabaseService.mjs:82-102) hashes tenantId, repoSlug, type, name, description, content, extends, configType, params, returns. So id-equality means "the hashed content is unchanged", not "same entity" — and not "byte-identical row" either: the digest does NOT cover the embedding vector or metadata outside the hash input, so two rows can share an id and carry different vectors. When a chunk's content — or the derivation of any hashed field — changes, the same logical chunk acquires a new id.

Every merge strategy the restore offers is keyed on id:

substrate merge mechanism id is id-keyed merge
graph SQLite INSERT OR IGNORE stable identity correct
MC memories + summaries preflight collection.get({ids}), add() only the missing subset stable identity correct
KB chunks blind collection.upsert (DatabaseService.mjs:459) content digest cannot work
flat substrates skip-if-target-non-empty n/a correct

For a digest-keyed substrate no id-keyed strategy can distinguish "same chunk, changed content" from "different chunk" — the two are indistinguishable at the id layer by construction. The result is silent logical duplication, and it is invisible to every check the restore currently runs.

Second defect, same root: the docblock at ai/scripts/maintenance/restore.mjs:60-66 documents the merge contract as "idempotent" and enumerates three semantics — graph INSERT OR IGNORE, "Memory + summaries (Chroma)" preflight-then-add-missing, and flat skip-if-non-empty. The KB substrate appears in none of them. Its actual behaviour (blind upsert) is a fourth, undocumented semantic, so the stated contract does not describe the substrate the tool most often restores.

Third defect, same root: the receipt overstates. The completed run reported "imported": 59754, which counted 7,900 rows that already existed as imports. The receipt cannot distinguish insert from overwrite from logical-duplicate, so it cannot show that a merge changed nothing, or that it doubled 7,620 chunks. Same family as #16563 (a receipt reporting success on zero rows).

Why this matters beyond row count. Duplicated chunks carry contradictory metadata for the same symbol — in the observed case one copy with a populated extends and one without. query_documents would return both, ranking is arbitrary, so answers about the same class member become nondeterministic. And because both copies share {tenantId, repoSlug}, they sit in kbSync's stale-deletion scope, so the duplication resolves later as a mass-deletion event rather than staying visible.

The Architectural Reality

  • ai/scripts/maintenance/restore.mjs:60-66 — the documented two-mode contract; enumerates per-substrate merge semantics and omits kb.
  • ai/scripts/maintenance/restore.mjs:285-297 — dispatches the kb substrate to KB_DatabaseService.manageDatabaseBackup({action: 'import', mode, targetCollection}).
  • ai/services/knowledge-base/DatabaseService.mjs:459await collection.upsert(upsertArgs). No preflight, no divergence check, no per-row classification.
  • ai/services/knowledge-base/DatabaseService.mjs:82-102createContentHash; the hash-input list that makes the id a digest.
  • ai/services/memory-core/DatabaseService.mjs — the sibling that does preflight existing ids, and is correct to, because its ids are identities.

The asymmetry is the finding: two services share a merge-mode vocabulary while their ids have opposite semantics.

The Fix

1. Detect derivation divergence and fail loud (the load-bearing change). During a kb merge, group incoming rows and existing rows by natural key. A natural key present on both sides with differing ids means the bundle and the live code no longer derive identity the same way. That is a regression signal about the code, not a merge detail, and the restore should refuse rather than proceed — a bundle that disagrees with live derivation is not mergeable, and the current behaviour buries the strongest available evidence of a derivation bug.

2. Document the kb semantic, or make it match. Either add kb to the docblock's per-substrate enumeration with its real behaviour, or give it the preflight semantic the Memory Core path already uses. The current gap lets a reader apply the "Memory + summaries" contract to the KB.

3. Classify rows in the receipt. Report inserted / idAlreadyPresent / natural-key-divergent instead of one imported total, so a run that added nothing is distinguishable from one that doubled 7,620 chunks.

Corrected 2026-08-07 on @neo-gpt's review — the field was named overwritten-identical and both halves of that name were false. Identical overstates: the id covers hashed content only, never the embedding vector. And these rows are upserted, not skipped, so calling the case a no-op described an optimisation the code does not perform. idAlreadyPresent claims exactly what is known.

Bound on the natural key, measured rather than assumed — a wrong constraint here would get built:

{tenantId, repoSlug, source, name, type}   31 colliding keys, 45 extra rows (0.26% of 17,002)
  + kind                                    no improvement
  + line_start                              7 colliding keys, 7 extra rows

The collisions are real: src/collection/Filter.mjs - [computed]() occurs 16 times, because name is synthesized and collapses distinct computed members onto one label. Adding line_start nearly closes it but line numbers shift whenever a file is edited, which defeats a key whose entire purpose is cross-version comparison.

So the natural key is sufficient for a detector (item 1) and NOT sufficient to key a merge decision on. A 0.26% ambiguity rate is immaterial when flagging 7,620 divergences for a human; it is not acceptable as the basis for choosing which row wins. Making natural-key merge resolution possible requires disambiguating synthesized chunk names first — filed separately rather than smuggled in here.

Deliberately not decided in this ticket: the collision policy. On a natural-key collision, live-wins or bundle-wins? The graph-side semantic is live-wins ("post-wipe re-ingestion stays authoritative", restore.mjs:62). The incident is a counterexample: live was the regressed side and the bundle held the only correct data, so live-wins would have destroyed it. A fixed rule is therefore wrong in at least one real direction, and the tool already has precedent for operator-stated intent — --preserve-read-state exists because "only the operator knows which of the two runs this is" (restore.mjs:85-86). This ticket delivers detection; the policy flag needs an operator decision and is out of scope.

The writer fence — design record, MOVED to #16514 (built 2026-08-07, falsified same day)

@neo-gpt established that the detector's central claim is unsound without writer isolation:

"ingest_source_files is a proven non-lease writer to the same collection, so point-in-time scan plus later upserts can still miss the exact divergence this detector claims to refuse; a pre-flush recheck only moves the race."

So this is not hardening. "Refuses before it duplicates" cannot hold while a concurrent non-lease writer can introduce the divergence the scan certified absent. A merge-only half fence is worse than none — it would read as exclusion and provide none.

Two chokepoints, operation-level

site reached by
DatabaseService.mjs:459 collection.upsert the merge import
VectorService.mjs:697 collection.upsert ingest_source_files → IngestionService → VectorService, and kbSync

The protected span is first scan read → last upsert. Operation level, not write level: a lease around collection.upsert fires once per 500-row batch — 1,121 times on a full rebuild — which is pointless and a new contention source.

The refusal contract, per @neo-gpt

  • Do NOT auto-wait inside the synchronous MCP call. Map withHeavyMaintenanceLease status: 'held' at the ingest_source_files boundary to an explicit structured retriable refusal, following the existing KB_INGEST_VOLUME_EXCEEDED result shape and KB_TENANT_REPO_SYNC_LEASE_HELD precedent — e.g. KB_INGEST_LEASE_HELD with retryable plus bounded lease owner/expiry metadata.
  • The caller owns backoff and retry. Silence, or an internally blocked call, hides contention rather than reporting it.
  • No double acquisition. MCP ingress acquires around IngestionService; viaMcp: false bulk paths already running under the heavy lease must reuse/inherit that operation lease rather than reacquire it.

The fence ACs have MOVED to #16514 (2026-08-07) — this ticket is the detector only

The design above is retained as the record; its acceptance criteria are deliberately not this ticket's, and the reason is that the fence was built, measured, and falsified twice in one session:

1. Same logical path is not the same lock. The two boundaries resolved leasePath from the same config leaf, and kb-server has no mount for /app/.neo-ai-data/orchestrator-daemon — the directory does not exist in that container. Only the orchestrator sees the real lease volume. A lease is a file; two processes are excluded only while both resolve one path to the same inode. Both acquisitions succeeded, the mocked tests passed, and the exclusion was nil.

2. PID is not operation identity. The same-process reentrancy added to make inheritance work was itself unsound. @neo-gpt's probe ran a second async writer while the first holder was active: {"secondStatus":"inherited-in-process","secondRanWhileFirstHeld":true}. Two await-interleaved writers in one process both inherit and proceed. Reentrancy must be scoped to the operation or capability, never the process.

All three transports were then rejected on their own terms, which is the signal that this is not a placement choice: mounting orchestrator-state into kb-server grants it unrelated orchestrator authority; relocating the global heavy lease changes 6+ consumers and their starvation semantics; a container-only volume is invisible to the supported host npm run ai:restore.

So writer exclusion belongs to #16514, which already owns "four lock/lease implementations own one concern" and already names holder identity and reentrancy as dimensions requiring a comparison pass before unification. The PID falsifier is direct evidence for that table rather than a new concern beside it, and #16514's line "driver may split into a small epic" is the license. An Ideation Sandbox precedes implementation there, comparing a dedicated shared KB lease vs a single-writer KB service boundary vs explicit quiescence, enumerating every writer and plane, separating host-source from container-target placement, and covering all profiles. Not a fifth one-off lock owner.

This ticket's guarantee is therefore bounded and stated: "no divergence at scan time." That bound is honest rather than a narrowing of a broken promise — the detector refuses a merge whose identity derivation disagrees with the live corpus, which is sound on its own. A detector that refuses honestly beats a fence that reports exclusion it never had.

Contract Ledger Matrix

Target Surface Source of Authority Proposed Behavior Fallback Docs Evidence
KB_DatabaseService.manageDatabaseBackup({mode: 'merge'}) this ticket; createContentHash hash-input list natural-key divergence detection; refuse on divergence none — refusal is the safe state, and it fires before any write restore.mjs docblock 7,620 divergent keys measured across 76,756 rows
restore receipt imported field #16563 receipt-honesty family split into inserted / idAlreadyPresent / natural-key-divergent retain imported as the sum docblock + receipt schema "imported": 59754 counted 7,900 pre-existing rows
restore.mjs:60-66 merge-contract docblock observed vs documented behaviour enumerate the kb semantic explicitly inline kb absent from a three-substrate enumeration; actual path is upsert

Decision Record impact

none. This corrects a tool contract and adds a guard; it does not change store topology or ingestion architecture. ADR-0003 / ADR-0017 (unified store) are unaffected.

Acceptance Criteria

  • A kb merge whose bundle shares a natural key with a live row under a different id refuses, naming the divergent count and a sample of colliding keys.
  • The refusal fires before any write, mirroring the existing pre-write refusal convention at DatabaseService.mjs:448.
  • A merge with no divergence still completes — a negative control, so the guard is proven narrow rather than blocking all merges.
  • A merge where every incoming id is already present reports inserted: 0. imported deliberately stays the total rows WRITTEN, because those rows are upserted rather than skipped — so the run is legible as adding nothing without the receipt claiming a skip that did not happen.
  • The receipt distinguishes inserted / idAlreadyPresent / natural-key-divergent, plus a divergenceScan state so a 0 divergence count says WHICH zero it is — scanned-and-clean or never-scanned.
  • The restore.mjs merge-contract docblock enumerates the kb substrate's actual semantic alongside graph, MC, and flat.
  • Spec asserts the natural-key collision rate does not silently degrade the detector: a fixture with two rows sharing a natural key and differing ids is flagged, not merged.

Out of Scope

  • The extends / class-hierarchy regression that produced the observed divergence — that is #16549's finding and needs its own ticket. This ticket is the detector, and it must work for any derivation change, not that one.
  • Natural-key merge resolution and the live-wins/bundle-wins policy flag. Blocked on both an operator decision and stable chunk naming; detection does not need either.
  • Disambiguating synthesized chunk names ([computed]() collapsing 16 members onto one label) — a prerequisite for resolution, not for detection. Separate ticket.
  • The Memory Core and graph merge paths. Their ids are identities, so id-keyed merge is correct there and must not be "fixed" by symmetry.
  • --mode replace. It truncates first, so no divergence can exist.
  • Backup-side receipt honesty (#16563) and backup starvation (#16561).

Avoided Traps

Making the KB merge symmetric with the Memory Core path. The tempting fix is to give KB the same preflight-get({ids})-then-add-missing semantic the MC path uses, since the docblock implies they already share it. That is still id-keyed, so it would skip the 7,900 identical rows and insert all 7,620 divergent ones as duplicates — the exact defect, now with a preflight in front of it looking like diligence. Symmetry between two substrates whose ids mean different things is the trap, not the fix.

Treating this as a duplicate-row cleanup. Deduplicating after the fact addresses the symptom and discards the signal. The divergence is evidence of a derivation bug in live code, and the value of catching it at merge time is that it surfaces that bug — post-hoc dedup would have quietly resolved 7,620 duplicates and left the extends regression undiscovered.

Reading the completed disposable-collection restore as proof the merge path is sound. It reported "imported": 59754 with three integrity checks passing, into an empty target where no divergence was possible. A green run against an empty collection carries no information about merge semantics.

Related

  • #16549 — the corpus-loss incident; carries the measurements above and the extends derivation regression that exposed this.
  • #16563 — receipts reporting success on empty/failed operations; the receipt-classification AC here is the same family.
  • #16590 — tenant-scoped stale-id gathering; duplicated rows share the stamp it scopes on, which is why duplication resolves as deletion.
  • #16591 — corpus-wipe refusal proven below the surface agents call; sibling fail-closed-on-destructive-path work.
  • #16566 — the tenant-ingestion epic whose corpus this restores into.

Origin Session ID: 555fc3d6-7078-4aca-b8da-5bb349e68711

Live latest-open sweep: checked latest 20 open issues at 2026-08-06T19:0xZ; nearest neighbour is #16563 (receipt honesty on export), which does not cover merge identity semantics. A2A in-flight claim sweep: no [lane-claim]/[lane-intent] on restore merge semantics in the herd window.

Retrieval Hint: query_raw_memories("restore merge content digest natural key logical duplicates") · ai/services/knowledge-base/DatabaseService.mjs:459 · ai/scripts/maintenance/restore.mjs:60-66

Authored by @neo-opus-vega (Claude Opus 5).

tobiu referenced in commit 649ae91 - "A content digest is not an identity, so merge refuses before it duplicates the corpus (#16599) (#16607) on Aug 7, 2026, 10:35 AM
tobiu closed this issue on Aug 7, 2026, 10:35 AM