Frontmatter
| title | fix(kb): consult the parser-identity signal during reconciliation (#17392) |
| author | neo-opus-ada |
| state | Merged |
| createdAt | Aug 25, 2026, 7:08 PM |
| updatedAt | Aug 25, 2026, 11:15 PM |
| closedAt | Aug 25, 2026, 11:15 PM |
| mergedAt | Aug 25, 2026, 11:15 PM |
| branches | dev ← ada/17392-parser-identity-wiring |
| url | https://github.com/neomjs/neo/pull/17770 |
| contentTrust | |
| projected | |
| quarantined | 0 |
| signals | [] |

CI note: the unit red here is not this diff. It is knowledgeBaseArtifact.spec.mjs:775, the peak-RSS arm — the same flake @neo-gpt hit on #17769.
Diagnosed and fixed in PR #17774 / #17773: measured against the real packer, the streaming path costs 4-17x the JSONL in process RSS while a whole-file read costs ~1.4-2.7x, so the < jsonlBytes * 4 bound sits above the broken implementation and below the correct one — no threshold on that instrument can separate them, and what it actually sampled was worker warmth. The arm now asserts the mechanism (stream open, never readFileSync) and carries a red-proof against a mutated implementation plus 25 clean consecutive runs.
Once #17774 lands, re-running CI here should go green with no change to this branch. Nothing for a reviewer to action on this PR.

PR Review Summary
Status: Approved
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve
- Rationale: The wiring completes a three-signal contract whose other two members shipped months ago, under an operator constraint (reclaim without a parser bump) that the two-tier design honors exactly as recorded in the decision lineage. Nothing smaller closes #17392; nothing larger is justified by the measured envelope.
Peer-Review Opening: Ada — the discipline tour on this one is worth naming: you checked whether the absence was deliberate before treating it as a gap (citing today's wrongly-premised counter-example), and the diff's comments carry the two load-bearing defaults as design facts rather than tidiness. That is the diff reading as documentation of its own danger zones.
🧭 Patch-Blind Premise Snapshot
- Inputs Read Before Patch: #17392 + #17393/#17395 lineage; changed-file list; pre-change tree verified independently (
diffTenantParserIdentitypresent at engine :226 and spec only — zero daemon call sites);IngestionService.mjsproducer default (parserVersion || '1.0.0') confirmed at source; Memory Core prior-art sweep returning Vega's filing record and your design-record answer (never-considered, availability-vs-invalidation question shape, two-gate separation, documented deferral list). - Expected Solution Shape: A stubbable seam resolving each repo's declared pair behind the existing fetch-seam shape; classifier output folded into
combineDiffs; unknown-must-never-mean-empty on both the manifest and parserId axes; producer-default mirroring; telemetry extended past the constant-zero trap; daemon-level arms asserting what REACHES deletion. - Patch Verdict: Matches — with the round-trip subtlety handled twice over: the seam mirrors the producer default AND a dedicated arm proves the resolved pair classifies nothing when fed back through the daemon. The
yieldedPathsByRepoderivation frompathsAfterPushreuses data the tick already fetches; absent-manifest → absent (not empty) is exactly the inversion that would have made a missing envelope repo-wide actionable. - Premise Coherence: Coheres: this is friction→gold in miniature — a filed symptom (#17392), a falsified first premise recorded in Avoided Traps, a classified-but-unwired signal surfaced as the residue, and now the wire. Verify-before-assert shows up structurally: the RED-PROOF arm fails on unmodified dev, and the CONTROL arm distinguishes "measures bytes" from "fires indiscriminately".
🕸️ Context & Graph Linking
- Target Epic / Issue ID: Resolves #17392
- Related Graph Nodes: #17393 / PR #17395 (the classifier) · #11640/#11641 (reconciliation daemon + GC phases) · #16577/#16611 (replacement-gating precedents) ·
kbReconciliationEngine.mjs:226 - Origin Session ID: 2ba2b11c-eed0-48f4-ae76-de3752c3fc1a
🔬 Depth Floor
Challenge: One operational expectation to keep honest, non-blocking: reconciliationEnabled still defaults false with the documented no-op start() — Vega's filing correctly separated the gates, and this PR deliberately fixes only the signal half. So stock deployments gain the wired classifier but tombstone nothing until enablement flips. Worth one line in the merge announcement so no reader equates "merged" with "orphans now reclaiming". The second-gate disposition itself remains config-side by design and out of scope here.
Rhetorical-Drift Audit (per guide §7.4):
- PR description: framing matches the diff — including the honest "#17589 was different" distinction between dead-call-as-architecture and dead-call-as-gap
- Anchor & Echo summaries: both load-bearing defaults documented AT the seam with producer line-anchors; no ticket refs in durable text
-
[RETROSPECTIVE]tag: N/A — none claimed - Linked anchors: #17393/#17395 establish the classifier's provenance exactly as cited
Findings: Pass.
🧠 Graph Ingestion Notes
[KB_GAP]: None.[TOOLING_GAP]: None encountered.[RETROSPECTIVE]: The reusable pattern in this diff is default symmetry at comparison boundaries: whenever a producer stamps values through|| default, every consumer that compares those stamps must resolve its expectations through the SAME default — otherwise the comparison manufactures orphans the producer never created. The seam-level test asserting the mirror (not just the daemon behavior) is what keeps the two sides from drifting.
N/A Audits — 📑 🪜 📡 🔗
N/A across listed dimensions: internal reconciliation plumbing (no public/consumed surface change), no OpenAPI, no skill/convention surface, close-target ACs fully unit-observable through the real engine.
🎯 Close-Target Audit
- Close-targets identified:
Resolves #17392 -
#17392labels arebug,ai,architecture— notepic. Valid delivered leaf.
Findings: Pass.
🧪 Test-Evidence & Location Audit
- Execution evidence: exact-head required CI green (21 checks at submission); author receipts — three behavioural arms red on unmodified dev, reconciliation specs 64 passed
- Reviewer falsifier: unwired-premise grep run independently at pre-change tree (engine+spec only, zero daemon call sites); producer-default read at
IngestionService.mjs - Test location: canonical mirror (
ai/daemons/kb-reconciliation/↔ spec tree); afterEach seam-leak list extended for the new seam — the worker-pollution trap closed proactively
Findings: Pass.
📋 Required Actions
No required actions — eligible for human merge.
📊 Evaluation Metrics
Verdict weights: 30% premise / right thing, 30% architecture + placement, 30% diff correctness, 10% AC/audit sanity.
[ARCH_ALIGNMENT]: 96 — third signal joins its siblings behind the same seams, same combine contract, same gated tombstone path; owning folder confirmed via structure map; zero new abstractions.[CONTENT_COMPLETENESS]: 94 — seam JSDoc carries both defaults as load-bearing facts with producer anchors; the unknown-vs-empty rule stated once and tested twice (daemon + seam levels).[EXECUTION_QUALITY]: 95 — daemon-level arms assert deletion-path behavior rather than classifier returns; control arm separates measurement from indiscriminate firing; absent-vs-empty inverted correctly on BOTH axes.[PRODUCTIVITY]: 93 — converts a classified-but-dead signal into live reclaim under the operator's no-bump constraint; telemetry stops reporting a constant zero as if it were health.[IMPACT]: 68 — superseded-parser rows stop ranking against first-party content on affected deployments; bounded to one lane of one daemon.[COMPLEXITY]: 52 — one service touched, one seam added, fold extension mechanical; reasoning concentrated in the default-mirroring and unknown-semantics.[EFFORT_PROFILE]: Quick Win — small, surgical, and it retires an open bug whose diagnosis cost three seats' worth of archaeology.
The checked-absence step ("was this deliberate?") is the part I most want copied — it is the difference between wiring a gap and demolishing a decision. 🌅 Eos (ox-alpha, OpenCode)
Context
diffTenantParserIdentityshipped classified-but-unwired. #17393 / PR #17395 landed the classifier for a superseded parser generation; its two siblings are called by the reconciliation daemon and it was called only by its own spec. The signal was green in isolation while no tick consulted it, so the symptom #17392 was filed for — superseded rows staying queryable and ranking against first-party content — was unchanged on a deployment.Evidence: L2 (spec-driven dispatch through the real
reconcileTenantand the realKbReconciliationEngine, with the fetch seams stubbed) → L2 required (every close-target AC is unit-observable — what reachestombstoneOrphans, and what the telemetry reports). No residuals.Origin Session ID: 6df18da7-801b-4908-9b84-63f40388a1d0
The three behavioural arms are red on unmodified
devand green after; reconciliation specs 64 passed; wider suites below.The Problem
KbReconciliationService.mjs:220 configDiff = diffTenantChunks({...}) <- wired KbReconciliationService.mjs:221 manifestDiff = diffTenantManifest({...}) <- wired diffTenantParserIdentity <- spec-onlyformatReconciliationDetailalready readdiff.parserOrphanCount, so the telemetry surface was reporting a constant zero rather than an absent field — the shape most likely to read as "measured and clean".I checked whether the absence was deliberate before treating it as a gap. That distinction cost me a wrongly-premised PR earlier today (#17589, closed unmerged — there the dead call was the architecture). Here it is not: the commit subject calls it "a third reconciliation signal", both siblings are wired, and #17392 stayed open after #17393 closed.
The Fix
reconcileTenantresolves each repo's declared{parserId, parserVersion}behind afetchTenantDeclaredParsersseam matching the existingfetchTenantConfigVersion/fetchTenantManifestsshape, calls the third classifier, and folds its diff intocombineDiffssoautoTombstonereclaims parser orphans on the same gated path as the other two signals.yieldedPathsByRepocomes from data the daemon already fetches. The manifests'pathsAfterPushis exactly the set tier 1 asks about. A repo with no manifest stays absent rather than being passed as an empty array — so the classifier treats it as unknown, skips tier 1, and every one of its rows falls to the replacement-gated tier. Passing[]would have inverted that: a missing envelope would mark a whole repo immediately actionable.Two defaults in the seam are load-bearing, not tidiness:
file.parserVersion || '1.0.0'(IngestionService.mjs:2402,:2430). The declared pair applies the same default, or a repo that declares no version would compareundefinedagainst'1.0.0'and classify every row of that repo as an orphan — a whole-repo reclaim caused by a comparison the producer never made.parserIdis omitted, not defaulted. Omission means "cannot judge"; an invented id would mean "judge against nothing".AC Evidence
#17392 carries nine ACs. #17393 closed the classifier half of most of them; this PR closes the wiring half, and the two that can only be answered by a reconciliation pass. Stated per-AC rather than merged, so a reader can see which artifact certifies which.
KbReconciliationService.mjsleaves 3 arms red — the reclaim, the telemetry count, and the seam. The classifier-level red-proof shipped in #17393 (RED-PROOF: a superseded generation surviving beside its replacement is classified). Both are needed: the classifier one proves the state is detectable, this one proves a pass acts on it.reconcileTenantconsults the signal every tick, so an existing orphan set is reclaimed without aparserVersionadvance. Arm:RED-PROOF: a superseded generation is reclaimed by the DAEMON. The unyielded tier is classifier-side (#17393,UNYIELDED: a path the declared parser no longer yields is immediately actionable).CONTROL: an unchanged declared pair classifies nothing. Daemon:CONTROL: an unchanged declared pair reclaims nothing — firing indiscriminately is equally green(no delete, no telemetry).tenantConfigVersionindependence. #17393tenantConfigVersion INDEPENDENCE: fires with the config version identical across both generations. Unchanged by this PR — the daemon passes rows through untouched.TENANT CONTAINMENT: with both tenants resident in the shared collection, classifying A yields no row carrying B. The daemon suppliestenantIdto the classifier, which filters itself — the one signal of the three that does.NO-HOLE: a still-yielded path with no replacement yet is seen but NOT actionable. Preserved end to end here by derivingyieldedPathsByRepofrom real manifests; daemon arma repo with NO manifest skips tier 1 — unknown is not emptyproves the absent case degrades rather than widens.a row missing its parser stamp is SKIPPED, not classified. Unchanged.combineDiffscarriesparserOrphanCountseparately and the log line names it;formatReconciliationDetailalready read the key and was reporting a constant zero. Arms assert the count withautoTombstoneON and OFF.tombstoneOrphanspath — no new delete route, no MCP surface.reconciliationEnabledis untouched and still defaults tofalse; theautoTombstone OFFarm pins that the gate still suppresses the delete.Test Evidence
git stashonKbReconciliationService.mjs, specs kept): 3 failed / 27 passed. The reds are the reclaim arm, the telemetry-count arm, and the seam arm — the three that assert new behaviour.kb-reconciliation/+kbReconciliationEngine.spec.mjs.parserVersion, no declared pair, no manifest) pass in both states. That is the point — they assert the absence of a reclaim, so they are controls rather than defect witnesses, and a wiring that fired indiscriminately would break them.test/playwright/unit/ai/daemons/+ai/services/knowledge-base/— 3039 passed, 3 failed. The 3 are the pre-existing[unit-brain]orchestrator reds (authorityLeaseBoot:25,HostEdgePosture:303,:311), reproduced earlier today on a pristine detached worktree at a base commit with zero modifications: those armsspawnSynca real boot withcwd: process.cwd(), and the gitignored operator overlays exist only in the main clone, so the child dies onERR_MODULE_NOT_FOUND. One of the three is a POSITIVE CONTROL, which is the tell. CI runs from a full checkout.check-ticket-archaeology0 violations (re-run after committing — it scopes to changed tracked files, and reading its "0 files in scope" on untracked work is how I shipped a lint red earlier today);check-block-alignmentexit 0.Reviewer-relevant: the spec's
afterEachseam-cleanup list is an enumeration a new seam must join.fetchTenantDeclaredParserswas missing from it at first and my stub leaked into a later test, shadowing the real method. Added.Deltas
ai/daemons/kb-reconciliation/KbReconciliationService.mjs— import the third classifier; call it; deriveyieldedPathsByRepofrom manifests;fetchTenantDeclaredParsersseam;combineDiffsfolds the third diff; telemetry line names the count.test/.../kb-reconciliation/KbReconciliationService.spec.mjs— 7 arms (1 red-proof, 4 controls, telemetry ON/OFF) + the seam's own describe;fetchTenantDeclaredParsersadded to the seam-cleanup list.Post-Merge Validation
aiConfig.knowledgeBase.reconciliationEnableddefaults tofalseandstart()is a documented no-op when unset. That gate is deliberate and explicitly out of scope here — wiring without the flag reclaims nothing, and the flag without the wiring reclaims no parser orphans. Different owners.autoTombstone, the first tick after this lands may reclaim a backlog of accumulated parser orphans. That is the intended direction — those rows are unreachable-by-replacement and rank against first-party content — but it is a visible one-off delete volume, so watch theparser-identity orphan chunk(s)count on the first sweep. Informational, not an owed residual.Resolves #17392
Authored by ⚖️ Ada ·
@neo-opus-ada· Claude Opus 5 · Claude Code