mirror of
https://github.com/Crosstalk-Solutions/project-nomad.git
synced 2026-10-02 03:24:51 +08:00
fix(kb): stop orphan sweep from purging an empty but present scan root
Boot runs ensureDirectoryExists() on the zim root, so a separate volume
that failed to mount leaves an empty mountpoint behind. The scan walks it
without error and reports it as scanned. One upload in kb_uploads keeps
the overall scan non-empty, so the next manual sync purged the vectors
for every ZIM under that root.
decideOrphans() now decides per scanned root and returns
{ orphans, withheld }. A root keeps its indexed sources when:
- it holds no embeddable files (kiwix-library.xml does not count)
- the purge would remove more than 50% of its indexed sources and at
least 5 of them, which points to a wrong or stale mount rather than
hand deletions
scanAndSyncStorage() logs each withheld root and appends a note to the
sync result message, so the user sees a likely unmounted drive instead
of losing hours of embeddings.
Replaces the unit test that asserted the old empty-root purge with
regression coverage for #1378 and both guards.
Closes #1378
This commit is contained in:
@@ -22,11 +22,11 @@ import { OllamaService } from './ollama_service.js'
|
||||
import { SERVICE_NAMES } from '../../constants/service_names.js'
|
||||
import { removeStopwords } from 'stopword'
|
||||
import { randomUUID } from 'node:crypto'
|
||||
import { join, resolve, sep } from 'node:path'
|
||||
import { join, relative, 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, filterOrphanCandidates } from '../utils/kb_orphan_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'
|
||||
@@ -2097,9 +2097,11 @@ export class RagService {
|
||||
* install legitimately has no kb_uploads until the first upload. That makes
|
||||
* an absent root indistinguishable from a present-but-empty one by looking
|
||||
* at the file list alone — and the orphan sweep in scanAndSyncStorage()
|
||||
* cannot afford to confuse the two. "This root scanned clean and had no
|
||||
* files" means everything indexed under it is an orphan; "this root wasn't
|
||||
* there" means we know nothing about it and must not touch it.
|
||||
* cannot afford to confuse the two. "This root wasn't there" means we know
|
||||
* nothing about it and must not touch it. A root that was there but empty
|
||||
* is not much better evidence: boot creates the zim directory, so an
|
||||
* unmounted volume shows up as an empty one (#1378). decideOrphans()
|
||||
* therefore also refuses to purge under a walked root with no files.
|
||||
*
|
||||
* That distinction is the whole reason this variant exists. If the zim root
|
||||
* is missing, renamed, or not yet mounted (see #1050 — relocating the data
|
||||
@@ -2388,24 +2390,29 @@ export class RagService {
|
||||
// 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."
|
||||
// installs already in this state.
|
||||
//
|
||||
// Allowlisted (via filterOrphanCandidates) to `scannedRoots` — the roots
|
||||
// the scan above actually walked, not the roots it meant to walk. Two
|
||||
// separate things are excluded by that one rule. Nomad's own bundled
|
||||
// docs (README.md + docs/) are embedded by discoverNomadDocs() from
|
||||
// outside these roots, so they're left alone without being named here.
|
||||
// And a root that wasn't present at scan time contributes no candidates
|
||||
// at all, because a missing root is skipped rather than fatal: without
|
||||
// this, a relocated or unmounted zim directory (#1050) would leave the
|
||||
// scan non-empty via kb_uploads, sail past decideOrphans' empty-scan
|
||||
// guard, and purge every ZIM in the index in a single batch.
|
||||
const orphanCandidates = filterOrphanCandidates([...sourcesInQdrant], scannedRoots)
|
||||
const orphans = decideOrphans(orphanCandidates, embeddableFiles)
|
||||
// Confined to `scannedRoots`, the roots the scan above actually walked,
|
||||
// not the roots it meant to walk. Nomad's own bundled docs (README.md +
|
||||
// docs/) live outside these roots, so they're left alone without being
|
||||
// named here, and a root missing at scan time contributes nothing
|
||||
// (#1050). Within each walked root, decideOrphans also withholds the
|
||||
// purge when the root holds no embeddable files (an unmounted volume
|
||||
// leaves an empty mountpoint behind, #1378) or when it would remove most
|
||||
// of the root at once. Withheld roots are reported back to the operator
|
||||
// rather than silently skipped.
|
||||
const { orphans, withheld } = decideOrphans(
|
||||
[...sourcesInQdrant],
|
||||
embeddableFiles,
|
||||
scannedRoots
|
||||
)
|
||||
for (const w of withheld) {
|
||||
logger.warn(
|
||||
`[RAG] Withheld purge of ${w.count} indexed source(s) under ${w.root} (${w.reason}); the directory may be unmounted or pointing at the wrong location`
|
||||
)
|
||||
}
|
||||
let orphansPurged = 0
|
||||
if (orphans && orphans.length > 0) {
|
||||
if (orphans.length > 0) {
|
||||
logger.info(
|
||||
`[RAG] Found ${orphans.length} orphaned source(s) with no corresponding file on disk`
|
||||
)
|
||||
@@ -2478,10 +2485,20 @@ export class RagService {
|
||||
`[RAG] Scan results (policy=${policy}): ${filesToEmbed.length} to embed, ${backfilled} backfilled, ${createdRows} new pending, ${createdPending} waiting on user, ${skipped} skipped`
|
||||
)
|
||||
|
||||
const withheldNote = withheld
|
||||
.map(
|
||||
(w) =>
|
||||
`; left ${w.count} indexed source${w.count !== 1 ? 's' : ''} under ${relative(process.cwd(), w.root)} untouched because ${
|
||||
w.reason === 'empty_root'
|
||||
? 'that folder has no files (is the drive mounted?)'
|
||||
: 'removing them would clear most of that folder (is it pointing at the right drive?)'
|
||||
}`
|
||||
)
|
||||
.join('')
|
||||
const orphanNote =
|
||||
orphansPurged > 0
|
||||
(orphansPurged > 0
|
||||
? `; purged ${orphansPurged} orphaned source${orphansPurged !== 1 ? 's' : ''}`
|
||||
: ''
|
||||
: '') + withheldNote
|
||||
|
||||
if (filesToEmbed.length === 0) {
|
||||
return {
|
||||
|
||||
@@ -1,5 +1,43 @@
|
||||
import { sep } from 'node:path'
|
||||
|
||||
/**
|
||||
* Past this share of a root's indexed sources, a single sync refuses to purge
|
||||
* that root at all. Losing most of a root at once is far likelier to mean the
|
||||
* root points somewhere wrong (a different disk, a stale copy, a half-finished
|
||||
* data-path move per #1050) than that the user deleted most of their library
|
||||
* by hand. Real deletions through the UI purge their own vectors immediately
|
||||
* (ZimService.delete, deleteFileBySource), so the sweep only ever mops up
|
||||
* stragglers and has no business removing the bulk of a root.
|
||||
*/
|
||||
export const ORPHAN_PURGE_MAX_FRACTION = 0.5
|
||||
|
||||
/**
|
||||
* The mass-removal guard only engages once this many sources would go. Below
|
||||
* it, the fraction is too noisy to mean anything (1 of 2 is 50%) and the cost
|
||||
* of a wrong purge is a few minutes of re-embedding, not hours.
|
||||
*/
|
||||
export const ORPHAN_PURGE_MIN_GUARDED = 5
|
||||
|
||||
export type WithheldOrphans = {
|
||||
/** Scan root whose orphans were left in place. */
|
||||
root: string
|
||||
/** How many indexed sources under it would have been purged. */
|
||||
count: number
|
||||
/**
|
||||
* `empty_root`: the root was walked but held no embeddable files.
|
||||
* `mass_removal`: the purge would remove more than ORPHAN_PURGE_MAX_FRACTION
|
||||
* of the root's indexed sources.
|
||||
*/
|
||||
reason: 'empty_root' | 'mass_removal'
|
||||
}
|
||||
|
||||
export type OrphanDecision = {
|
||||
/** Sources safe to purge. */
|
||||
orphans: string[]
|
||||
/** Roots whose missing sources were held back, for the operator to see. */
|
||||
withheld: WithheldOrphans[]
|
||||
}
|
||||
|
||||
/**
|
||||
* Decision for the reverse sweep in `RagService.scanAndSyncStorage`.
|
||||
*
|
||||
@@ -12,26 +50,64 @@ import { sep } from 'node:path'
|
||||
* `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.
|
||||
* Decided per scanned root, because the roots fail independently. Two guards
|
||||
* apply to each, and either one withholds the whole root for this cycle:
|
||||
*
|
||||
* 1. A root with zero embeddable files keeps its sources (#1378). Walking a
|
||||
* directory successfully does not prove it holds the content: boot runs
|
||||
* ensureDirectoryExists() on the zim root, so a separate volume that
|
||||
* failed to mount leaves a real, empty mountpoint behind (often holding a
|
||||
* freshly regenerated kiwix-library.xml, which is why this counts
|
||||
* embeddable files rather than directory entries). An empty root is
|
||||
* indistinguishable from an unmounted one, and a single upload in
|
||||
* kb_uploads is enough to make the overall scan non-empty.
|
||||
*
|
||||
* 2. A purge that would remove more than ORPHAN_PURGE_MAX_FRACTION of a
|
||||
* root's indexed sources (and at least ORPHAN_PURGE_MIN_GUARDED of them) is
|
||||
* withheld. This catches the root pointing at the wrong place while still
|
||||
* holding a few files, which guard 1 can't see.
|
||||
*
|
||||
* Both guards trade a missed cleanup for safety: re-embedding a wrongly purged
|
||||
* library takes hours to days, while a stale source costs nothing until the
|
||||
* next sync or an explicit reset.
|
||||
*/
|
||||
export function decideOrphans(
|
||||
sourcesInQdrant: string[],
|
||||
embeddableFiles: string[]
|
||||
): string[] | null {
|
||||
if (embeddableFiles.length === 0) return null
|
||||
|
||||
embeddableFiles: string[],
|
||||
scannedRoots: string[]
|
||||
): OrphanDecision {
|
||||
const onDisk = new Set(embeddableFiles)
|
||||
return sourcesInQdrant.filter((source) => !onDisk.has(source))
|
||||
const orphans: string[] = []
|
||||
const withheld: WithheldOrphans[] = []
|
||||
|
||||
for (const root of scannedRoots) {
|
||||
const indexed = filterOrphanCandidates(sourcesInQdrant, [root])
|
||||
const missing = indexed.filter((source) => !onDisk.has(source))
|
||||
if (missing.length === 0) continue
|
||||
|
||||
if (filterOrphanCandidates(embeddableFiles, [root]).length === 0) {
|
||||
withheld.push({ root, count: missing.length, reason: 'empty_root' })
|
||||
continue
|
||||
}
|
||||
|
||||
if (
|
||||
missing.length >= ORPHAN_PURGE_MIN_GUARDED &&
|
||||
missing.length / indexed.length > ORPHAN_PURGE_MAX_FRACTION
|
||||
) {
|
||||
withheld.push({ root, count: missing.length, reason: 'mass_removal' })
|
||||
continue
|
||||
}
|
||||
|
||||
orphans.push(...missing)
|
||||
}
|
||||
|
||||
return { orphans, withheld }
|
||||
}
|
||||
|
||||
/**
|
||||
* Narrows Qdrant sources down to the ones decideOrphans() can actually make an
|
||||
* informed call about: sources under a root the disk scan genuinely walked.
|
||||
* Narrows paths down to the ones under a root the disk scan genuinely walked.
|
||||
* decideOrphans() applies it per root, both to Qdrant sources and to the files
|
||||
* found on disk.
|
||||
*
|
||||
* `scannedRoots` is deliberately the roots that were *walked*, not the roots
|
||||
* that were *configured*. Two different failure modes collapse into that one
|
||||
@@ -45,9 +121,9 @@ export function decideOrphans(
|
||||
* 2. A configured root that wasn't there at scan time. `_discoverKbFiles()`
|
||||
* skips a missing root rather than failing, so a relocated or not-yet-
|
||||
* mounted zim directory (#1050) still leaves the scan non-empty via
|
||||
* kb_uploads. decideOrphans' empty-scan guard doesn't fire, and without
|
||||
* this filter every ZIM in the index would be purged in one batch. A root
|
||||
* we couldn't read tells us nothing about what belongs under it.
|
||||
* kb_uploads. A root we couldn't read tells us nothing about what belongs
|
||||
* under it. (A root that exists but is empty is the same problem one step
|
||||
* removed; decideOrphans' per-root guard handles that, see #1378.)
|
||||
*
|
||||
* Passing an empty `scannedRoots` therefore yields no candidates, which is the
|
||||
* correct reading of "we couldn't see any of the storage."
|
||||
|
||||
@@ -20,28 +20,39 @@ const upload = (name: string) => join(KB_UPLOADS_ROOT, name)
|
||||
const zim = (name: string) => join(ZIM_ROOT, name)
|
||||
|
||||
test('no sources in Qdrant → no orphans', () => {
|
||||
assert.deepEqual(decideOrphans([], [zim('a.zim')]), [])
|
||||
assert.deepEqual(decideOrphans([], [zim('a.zim')], SCAN_ROOTS), { orphans: [], withheld: [] })
|
||||
})
|
||||
|
||||
test('every Qdrant source still has a file on disk → no orphans', () => {
|
||||
assert.deepEqual(decideOrphans([zim('a.zim'), zim('b.zim')], [zim('a.zim'), zim('b.zim')]), [])
|
||||
assert.deepEqual(
|
||||
decideOrphans([zim('a.zim'), zim('b.zim')], [zim('a.zim'), zim('b.zim')], SCAN_ROOTS),
|
||||
{ orphans: [], withheld: [] }
|
||||
)
|
||||
})
|
||||
|
||||
test('a source with no matching file on disk is an orphan', () => {
|
||||
assert.deepEqual(decideOrphans([zim('a.zim'), zim('gone.zim')], [zim('a.zim')]), [
|
||||
zim('gone.zim'),
|
||||
])
|
||||
assert.deepEqual(decideOrphans([zim('a.zim'), zim('gone.zim')], [zim('a.zim')], SCAN_ROOTS), {
|
||||
orphans: [zim('gone.zim')],
|
||||
withheld: [],
|
||||
})
|
||||
})
|
||||
|
||||
test('every Qdrant source is orphaned when none remain on disk (but disk scan was non-empty)', () => {
|
||||
assert.deepEqual(decideOrphans([zim('gone1.zim'), zim('gone2.zim')], [zim('unrelated.zim')]), [
|
||||
zim('gone1.zim'),
|
||||
zim('gone2.zim'),
|
||||
])
|
||||
test('sources outside every scanned root are never orphans (e.g. bundled docs)', () => {
|
||||
const readme = join('/data', 'README.md')
|
||||
assert.deepEqual(decideOrphans([readme, zim('a.zim')], [zim('a.zim')], SCAN_ROOTS), {
|
||||
orphans: [],
|
||||
withheld: [],
|
||||
})
|
||||
})
|
||||
|
||||
test('empty disk scan is treated as a transient failure, not "everything was deleted"', () => {
|
||||
assert.equal(decideOrphans([zim('a.zim'), zim('b.zim')], []), null)
|
||||
test('empty disk scan withholds every root instead of purging everything', () => {
|
||||
assert.deepEqual(decideOrphans([upload('a.pdf'), zim('a.zim')], [], SCAN_ROOTS), {
|
||||
orphans: [],
|
||||
withheld: [
|
||||
{ root: KB_UPLOADS_ROOT, count: 1, reason: 'empty_root' },
|
||||
{ root: ZIM_ROOT, count: 1, reason: 'empty_root' },
|
||||
],
|
||||
})
|
||||
})
|
||||
|
||||
test('filterOrphanCandidates keeps sources under the kb_uploads or zim scan roots', () => {
|
||||
@@ -80,7 +91,10 @@ test('a root that was not scanned contributes no candidates, even though the sca
|
||||
assert.deepEqual(candidates, [upload('a.pdf')])
|
||||
|
||||
// End to end: the ZIMs survive despite having no backing file in the scan.
|
||||
assert.deepEqual(decideOrphans(candidates, [upload('a.pdf')]), [])
|
||||
assert.deepEqual(decideOrphans(sourcesInQdrant, [upload('a.pdf')], [KB_UPLOADS_ROOT]), {
|
||||
orphans: [],
|
||||
withheld: [],
|
||||
})
|
||||
})
|
||||
|
||||
test('no roots scanned at all yields no candidates', () => {
|
||||
@@ -89,12 +103,67 @@ test('no roots scanned at all yields no candidates', () => {
|
||||
assert.deepEqual(filterOrphanCandidates([zim('a.zim')], []), [])
|
||||
})
|
||||
|
||||
test('a scanned-but-empty root still yields orphans for what it contains', () => {
|
||||
// The counterpart to the case above, and the reason the distinction matters:
|
||||
// zim scanned clean and legitimately holds no files, so its indexed sources
|
||||
// really are orphaned and should be purged. A missing root and an empty one
|
||||
// must not behave the same way.
|
||||
const candidates = filterOrphanCandidates([zim('gone.zim')], SCAN_ROOTS)
|
||||
assert.deepEqual(candidates, [zim('gone.zim')])
|
||||
assert.deepEqual(decideOrphans(candidates, [upload('a.pdf')]), [zim('gone.zim')])
|
||||
test('a walked root with no embeddable files keeps its sources (#1378)', () => {
|
||||
// The unmounted-volume shape: boot's ensureDirectoryExists() recreates the
|
||||
// zim mountpoint as an empty directory, so the scan walks it without error
|
||||
// and reports it as scanned. kb_uploads keeps the overall scan non-empty.
|
||||
// Treating "walked and empty" as "everything under it was deleted" would
|
||||
// purge every ZIM in the index.
|
||||
const sourcesInQdrant = [upload('a.pdf'), zim('wikipedia_en_all_maxi.zim'), zim('gutenberg.zim')]
|
||||
assert.deepEqual(decideOrphans(sourcesInQdrant, [upload('a.pdf')], SCAN_ROOTS), {
|
||||
orphans: [],
|
||||
withheld: [{ root: ZIM_ROOT, count: 2, reason: 'empty_root' }],
|
||||
})
|
||||
})
|
||||
|
||||
test('a root holding only non-embeddable files still counts as empty', () => {
|
||||
// Kiwix regenerates kiwix-library.xml in the empty mountpoint, so the
|
||||
// directory has entries. Only embeddable files are evidence the content is
|
||||
// there, and _discoverKbFilesWithRoots() drops the XML before this point, so
|
||||
// the zim root arrives with nothing under it.
|
||||
const embeddable = [upload('a.pdf')]
|
||||
assert.deepEqual(decideOrphans([zim('a.zim')], embeddable, SCAN_ROOTS).withheld, [
|
||||
{ root: ZIM_ROOT, count: 1, reason: 'empty_root' },
|
||||
])
|
||||
})
|
||||
|
||||
test('an empty kb_uploads root keeps its sources while zim is swept normally', () => {
|
||||
const zims = ['a', 'b', 'c'].map((n) => zim(`${n}.zim`))
|
||||
assert.deepEqual(
|
||||
decideOrphans([upload('a.pdf'), ...zims, zim('gone.zim')], zims, SCAN_ROOTS),
|
||||
{
|
||||
orphans: [zim('gone.zim')],
|
||||
withheld: [{ root: KB_UPLOADS_ROOT, count: 1, reason: 'empty_root' }],
|
||||
}
|
||||
)
|
||||
})
|
||||
|
||||
test('removing most of a root in one sync is withheld as a likely wrong mount', () => {
|
||||
// The root is present and non-empty, but it holds 2 of 10 indexed ZIMs:
|
||||
// far likelier a different disk or stale copy than 8 hand-deletions.
|
||||
const indexed = Array.from({ length: 10 }, (_, i) => zim(`z${i}.zim`))
|
||||
const onDisk = indexed.slice(0, 2)
|
||||
assert.deepEqual(decideOrphans(indexed, onDisk, SCAN_ROOTS), {
|
||||
orphans: [],
|
||||
withheld: [{ root: ZIM_ROOT, count: 8, reason: 'mass_removal' }],
|
||||
})
|
||||
})
|
||||
|
||||
test('removing up to half a root is still purged', () => {
|
||||
const indexed = Array.from({ length: 10 }, (_, i) => zim(`z${i}.zim`))
|
||||
const onDisk = indexed.slice(0, 5)
|
||||
assert.deepEqual(decideOrphans(indexed, onDisk, SCAN_ROOTS), {
|
||||
orphans: indexed.slice(5),
|
||||
withheld: [],
|
||||
})
|
||||
})
|
||||
|
||||
test('the mass-removal guard does not engage below the minimum count', () => {
|
||||
// 4 of 5 missing is 80%, but only 4 sources: too few for the ratio to mean
|
||||
// anything, and cheap to re-embed if wrong.
|
||||
const indexed = Array.from({ length: 5 }, (_, i) => zim(`z${i}.zim`))
|
||||
assert.deepEqual(decideOrphans(indexed, indexed.slice(0, 1), SCAN_ROOTS), {
|
||||
orphans: indexed.slice(1),
|
||||
withheld: [],
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user