From 68bb5103518a6aa78cc765397a2b7dc44406b155 Mon Sep 17 00:00:00 2001 From: John Cortright Date: Mon, 10 Aug 2026 23:04:49 -0700 Subject: [PATCH] fix(rag): reap orphaned qdrant sources during sync (#1170) scanAndSyncStorage builds sourcesInQdrant and embeddableFiles in the same pass but only ever checks one direction - is this on-disk file already embedded? It never checks the reverse, so points left behind by a deleted or replaced ZIM just sit in Qdrant forever and keep showing up as search hits for content that doesn't exist anymore. Added a reverse sweep that diffs the two sets and purges anything left in Qdrant with no file backing it. Guarded so it's a no-op when the disk scan comes back empty, so a filesystem hiccup can't get misread as "delete everything." Also has to exclude README.md/docs, since those get embedded by discoverNomadDocs and live outside the kb_uploads/zim paths the scanner actually walks. First pass without that exclusion would have wiped the docs KB on every single sync - caught it by testing against a live install where those sources were already indexed. Verified live: found exactly one real orphan out of 29 indexed sources (an old devdocs_en_bash zim superseded by a newer download), purged it, and left the other 28 - including all the docs - alone. --- admin/app/services/rag_service.ts | 72 +++++++++++++++++++-- admin/app/utils/kb_orphan_decision.ts | 25 +++++++ admin/tests/unit/kb_orphan_decision.spec.ts | 33 ++++++++++ 3 files changed, 126 insertions(+), 4 deletions(-) create mode 100644 admin/app/utils/kb_orphan_decision.ts create mode 100644 admin/tests/unit/kb_orphan_decision.spec.ts diff --git a/admin/app/services/rag_service.ts b/admin/app/services/rag_service.ts index 858b58a..ba66ad9 100644 --- a/admin/app/services/rag_service.ts +++ b/admin/app/services/rag_service.ts @@ -19,6 +19,7 @@ import { join, resolve, sep } from 'node:path' import KVStore from '#models/kv_store' import KbIngestState from '#models/kb_ingest_state' import { decideScanAction, type IngestPolicy } from '../utils/kb_ingest_decision.js' +import { decideOrphans } from '../utils/kb_orphan_decision.js' import { decideContentReindex, type ReindexOutcome } from '../utils/content_reindex_decision.js' import KbRatioRegistry from '#models/kb_ratio_registry' import { decideWarnings } from '../utils/kb_warning_decision.js' @@ -1714,10 +1715,23 @@ export class RagService { return outcome } + /** + * Root paths for Nomad's own bundled docs (README.md + docs/), embedded by + * discoverNomadDocs(). They live outside kb_uploads/zim storage, so + * _discoverKbFiles()'s scan never sees them — the orphan sweep in + * scanAndSyncStorage() uses this to exclude them rather than hardcoding + * the same two paths a second time. + */ + private _nomadDocsRoots(): { readmePath: string; docsDir: string } { + return { + readmePath: join(process.cwd(), 'README.md'), + docsDir: join(process.cwd(), 'docs'), + } + } + public async discoverNomadDocs(force?: boolean): Promise<{ success: boolean; message: string }> { try { - const README_PATH = join(process.cwd(), 'README.md') - const DOCS_DIR = join(process.cwd(), 'docs') + const { readmePath: README_PATH, docsDir: DOCS_DIR } = this._nomadDocsRoots() const alreadyEmbeddedRaw = await KVStore.getValue('rag.docsEmbedded') if (alreadyEmbeddedRaw && !force) { @@ -1929,6 +1943,19 @@ export class RagService { }) } + /** + * Purge a source's Qdrant points and its `KbIngestState` row without + * touching the file on disk. For callers where the file is already gone + * (or never existed as a knowledge-base upload) — `ZimService.delete()` + * (#1170) and the orphan sweep in `scanAndSyncStorage()` below. Does NOT + * attempt to delete a physical file; use `deleteFileBySource()` for the + * user-triggered "remove this file" action instead. + */ + public async purgeIndexedSource(source: string): Promise { + await this._deletePointsBySource(source) + await KbIngestState.remove(source) + } + /** * Returns true if the file-embeddings queue has any in-flight work * (waiting, active, delayed, or paused). Bulk re-embed actions use this @@ -1998,6 +2025,41 @@ export class RagService { (filePath) => determineFileType(filePath) !== 'unknown' ) + // Reverse sweep (#1170): sourcesInQdrant and embeddableFiles are both + // already known at this point — the forward loop below only ever asks + // "is this on-disk file already embedded?" This closes the other + // direction: a source in Qdrant with no corresponding file on disk is a + // leftover from ZimService.delete() (which never touched Qdrant) or + // from reconcileReplacedContentFile's qdrant_not_running no-op. Running + // this in sync (rather than only in the delete path) also self-heals + // installs already in this state. decideOrphans no-ops when + // embeddableFiles came back empty, so a filesystem hiccup can't be + // misread as "every file was deleted." + // + // Nomad's own bundled docs (README.md + docs/) are embedded by + // discoverNomadDocs() above, not by the kb_uploads/zim scan that built + // embeddableFiles — excluded here so they aren't misclassified as + // orphans and purged on every sync. + const { readmePath, docsDir } = this._nomadDocsRoots() + const docsDirPrefix = docsDir + sep + const orphanCandidates = [...sourcesInQdrant].filter( + (source) => source !== readmePath && !source.startsWith(docsDirPrefix) + ) + const orphans = decideOrphans(orphanCandidates, embeddableFiles) + let orphansPurged = 0 + if (orphans && orphans.length > 0) { + logger.info(`[RAG] Found ${orphans.length} orphaned source(s) with no corresponding file on disk`) + for (const orphan of orphans) { + try { + await this.purgeIndexedSource(orphan) + orphansPurged++ + } catch (error) { + logger.error(`[RAG] Failed to purge orphaned source ${orphan}:`, error) + } + } + logger.info(`[RAG] Purged ${orphansPurged}/${orphans.length} orphaned source(s)`) + } + // Read the global ingest policy. Unset is treated as 'Always' so legacy // installs keep their current behavior until the user explicitly opts // into Manual mode from the KB panel. @@ -2058,10 +2120,12 @@ export class RagService { `[RAG] Scan results (policy=${policy}): ${filesToEmbed.length} to embed, ${backfilled} backfilled, ${createdRows} new pending, ${createdPending} waiting on user, ${skipped} skipped` ) + const orphanNote = orphansPurged > 0 ? `; purged ${orphansPurged} orphaned source${orphansPurged !== 1 ? 's' : ''}` : '' + if (filesToEmbed.length === 0) { return { success: true, - message: 'Knowledge base is already in sync', + message: `Knowledge base is already in sync${orphanNote}`, filesScanned: filesInStorage.length, filesQueued: 0, } @@ -2071,7 +2135,7 @@ export class RagService { const dedupeNote = dedupedCount > 0 ? ` (${dedupedCount} already queued)` : '' return { success: true, - message: `Scanned ${filesInStorage.length} files, queued ${queuedCount} for embedding${dedupeNote}`, + message: `Scanned ${filesInStorage.length} files, queued ${queuedCount} for embedding${dedupeNote}${orphanNote}`, filesScanned: filesInStorage.length, filesQueued: queuedCount, } diff --git a/admin/app/utils/kb_orphan_decision.ts b/admin/app/utils/kb_orphan_decision.ts new file mode 100644 index 0000000..660a4a7 --- /dev/null +++ b/admin/app/utils/kb_orphan_decision.ts @@ -0,0 +1,25 @@ +/** + * Decision for the reverse sweep in `RagService.scanAndSyncStorage`. + * + * This is the pure, I/O-free core of the orphan check described in issue + * #1170: `scanAndSyncStorage` already builds `sourcesInQdrant` (from a facet + * query) and `embeddableFiles` (from a disk scan) in the same pass, but only + * ever asked "is this on-disk file already embedded?" — never the reverse + * "does this Qdrant source still have a file on disk?" Sources left behind by + * `ZimService.delete()` (which never touched Qdrant) or by + * `reconcileReplacedContentFile`'s `qdrant_not_running` no-op therefore never + * got reaped. + * + * Guarded so a transient failure can't be misread as "every file was + * deleted": if the disk scan came back empty, we return `null` (do nothing) + * rather than treating every indexed source as an orphan. An empty + * `embeddableFiles` list is indistinguishable from a filesystem hiccup, and + * the blast radius of wrongly deleting a healthy knowledge base outweighs the + * cost of skipping a sweep for one cycle. + */ +export function decideOrphans(sourcesInQdrant: string[], embeddableFiles: string[]): string[] | null { + if (embeddableFiles.length === 0) return null + + const onDisk = new Set(embeddableFiles) + return sourcesInQdrant.filter((source) => !onDisk.has(source)) +} diff --git a/admin/tests/unit/kb_orphan_decision.spec.ts b/admin/tests/unit/kb_orphan_decision.spec.ts new file mode 100644 index 0000000..352684a --- /dev/null +++ b/admin/tests/unit/kb_orphan_decision.spec.ts @@ -0,0 +1,33 @@ +import * as assert from 'node:assert/strict' +import { test } from 'node:test' + +import { decideOrphans } from '../../app/utils/kb_orphan_decision.js' + +test('no sources in Qdrant → no orphans', () => { + assert.deepEqual(decideOrphans([], ['/storage/zim/a.zim']), []) +}) + +test('every Qdrant source still has a file on disk → no orphans', () => { + assert.deepEqual( + decideOrphans(['/storage/zim/a.zim', '/storage/zim/b.zim'], ['/storage/zim/a.zim', '/storage/zim/b.zim']), + [] + ) +}) + +test('a source with no matching file on disk is an orphan', () => { + assert.deepEqual( + decideOrphans(['/storage/zim/a.zim', '/storage/zim/gone.zim'], ['/storage/zim/a.zim']), + ['/storage/zim/gone.zim'] + ) +}) + +test('every Qdrant source is orphaned when none remain on disk (but disk scan was non-empty)', () => { + assert.deepEqual( + decideOrphans(['/storage/zim/gone1.zim', '/storage/zim/gone2.zim'], ['/storage/zim/unrelated.zim']), + ['/storage/zim/gone1.zim', '/storage/zim/gone2.zim'] + ) +}) + +test('empty disk scan is treated as a transient failure, not "everything was deleted"', () => { + assert.equal(decideOrphans(['/storage/zim/a.zim', '/storage/zim/b.zim'], []), null) +})