Merge d365ed6160 into e6acf9938a

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<void> {
+    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,
       }

admin/app/services/zim_service.ts

@@ -33,6 +33,8 @@ import { SERVICE_NAMES } from '../../constants/service_names.js'
 import { CollectionManifestService } from './collection_manifest_service.js'
 import { KiwixCatalogService } from './kiwix_catalog_service.js'
 import { KiwixLibraryService } from './kiwix_library_service.js'
+import { RagService } from './rag_service.js'
+import { OllamaService } from './ollama_service.js'
 import type { CategoryWithStatus } from '../../types/collections.js'
 import CustomLibrarySource from '#models/custom_library_source'
 import { assertNotPrivateUrl } from '#validators/common'
@@ -659,6 +661,18 @@ export class ZimService {
 
     await deleteFileIfExists(fullPath)
 
+    // Purge this file's Qdrant points and KbIngestState row directly so a
+    // deleted ZIM stops surfacing as a stale search citation immediately,
+    // rather than waiting for the next scanAndSyncStorage reverse sweep to
+    // catch it (#1170). Never touched Qdrant here before — the file's points
+    // and state row would otherwise linger indefinitely.
+    try {
+      const ragService = new RagService(this.dockerService, new OllamaService())
+      await ragService.purgeIndexedSource(fullPath)
+    } catch (err) {
+      logger.error(`[ZimService] Failed to purge knowledge-base entries for ${fullPath}:`, err)
+    }
+
     // Remove from kiwix library XML so --monitorLibrary stops serving the deleted file
     const kiwixLibraryService = new KiwixLibraryService()
     await kiwixLibraryService.removeBook(fileName).catch((err) => {

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))
+}

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)
+})