fix(rag): don't abandon a ZIM after one sparse batch

The batch continuation gated on articles that produced chunks rather than
articles the extractor consumed. Articles whose text is empty after HTML
cleaning — redirect stubs, category listings, media and PDF wrappers —
return no chunks and so no documentIds, which made a normal sparse batch
look like the end of the archive. Ingestion stopped after the first batch
and the rest of the file was silently skipped.

Medicine LibreTexts embedded 16 chunks out of 23,171 articles: its first
50 entries are navigation pages, only 10 of which had text, so 10 >= 50
was false and no continuation was ever dispatched. Measured against the
same install, WikiMed reached 28% of its articles, Libre Pathology 31%,
MedlinePlus 84%, and CDC Travelers' Health 2.7%.

The same value was also returned as `articlesProcessed`, which
EmbedFileJob uses to advance the next batch offset. Advancing by the
content-bearing count re-read the overlap on every sparse batch, and a
batch yielding no text at all would have advanced by zero and re-run the
same window indefinitely.

Both now use the extractor's consumed-article count, which it already
computed and logged but discarded. The end-of-archive signal is the
iterator running dry (articlesProcessed < batchSize); archive.articleCount
cannot serve as a bound because iterByPath() legitimately yields more
article entries than that figure for some archives.

The log line now reports both counts (content-bearing/consumed) so a
sparse batch is visible without re-deriving it.
This commit is contained in:
rcorvus 2026-08-13 17:56:14 -07:00
parent aff56ad4a6
commit ab31e208d1
4 changed files with 123 additions and 14 deletions

View File

@ -37,6 +37,7 @@ const EXCLUDE_EVAL_FILTER = {
import type { KbIngestStateValue } from '../../types/kb_ingest_state.js'
import { ZIMExtractionService } from './zim_extraction_service.js'
import { ZIM_BATCH_SIZE } from '../../constants/zim_extraction.js'
import { hasMoreArticleBatches } from '../utils/zim_batch_decision.js'
import { EMBEDDING_MODEL_NAME } from '../../constants/ollama.js'
import { ProcessAndEmbedFileResponse, ProcessZIMFileResponse, RAGResult, RerankedRAGResult } from '../../types/rag.js'
@ -556,10 +557,14 @@ export class RagService {
`[RAG] Extracting ZIM content (batch: offset=${startOffset}, size=${ZIM_BATCH_SIZE})`
)
const { chunks: zimChunks, totalArticles } = await zimExtractionService.extractZIMContent(
filepath,
{ startOffset, batchSize: ZIM_BATCH_SIZE }
)
const {
chunks: zimChunks,
totalArticles,
articlesProcessed,
} = await zimExtractionService.extractZIMContent(filepath, {
startOffset,
batchSize: ZIM_BATCH_SIZE,
})
logger.info(
`[RAG] Extracted ${zimChunks.length} chunks from ZIM file with enhanced metadata (file totalArticles=${totalArticles})`
@ -610,15 +615,19 @@ export class RagService {
}
}
// Count unique articles processed in this batch. hasMoreBatches gates on the article
// count — zimChunks.length counts section-level chunks (multiple per article under the
// 'structured' strategy), so comparing it to ZIM_BATCH_SIZE (an article limit) caps
// processing at the first batch for any real archive.
const articlesInBatch = new Set(zimChunks.map((c) => c.documentId)).size
const hasMoreBatches = articlesInBatch >= ZIM_BATCH_SIZE
// Gate the continuation on articles the extractor CONSUMED, not on articles that
// produced chunks. Articles whose text is empty after cleaning (redirect stubs,
// category pages, media/PDF wrappers) contribute no documentIds, so a chunk-derived
// count reads a normal sparse batch as end-of-archive and silently abandons the rest
// of the file. See `zim_batch_decision.ts` for the full rationale.
const articlesWithContent = new Set(zimChunks.map((c) => c.documentId)).size
const hasMoreBatches = hasMoreArticleBatches({
articlesProcessed,
batchSize: ZIM_BATCH_SIZE,
})
logger.info(
`[RAG] Successfully embedded ${totalChunks} total chunks from ${articlesInBatch} articles (hasMore: ${hasMoreBatches})`
`[RAG] Successfully embedded ${totalChunks} total chunks from ${articlesWithContent}/${articlesProcessed} articles (hasMore: ${hasMoreBatches})`
)
// Only delete the file when:
@ -640,7 +649,11 @@ export class RagService {
: 'ZIM file processed and embedded successfully with enhanced metadata.',
chunks: totalChunks,
hasMoreBatches,
articlesProcessed: articlesInBatch,
// MUST be the consumed count: EmbedFileJob advances the next batch offset by this
// value. Reporting the content-bearing count instead re-reads the overlap on every
// sparse batch, and a batch that yields no text at all would advance the offset by
// zero and re-run the same window forever.
articlesProcessed,
totalArticles,
}
}

View File

@ -44,7 +44,7 @@ export class ZIMExtractionService {
async extractZIMContent(
filePath: string,
opts: ExtractZIMContentOptions = {}
): Promise<{ chunks: ZIMContentChunk[]; totalArticles: number }> {
): Promise<{ chunks: ZIMContentChunk[]; totalArticles: number; articlesProcessed: number }> {
try {
logger.info(`[ZIMExtractionService]: Processing ZIM file at path: ${filePath}`)
@ -165,7 +165,7 @@ export class ZIMExtractionService {
textPreview: c.text.substring(0, 100)
})))
logger.debug("Total structured sections extracted:", toReturn.length)
return { chunks: toReturn, totalArticles: archive.articleCount }
return { chunks: toReturn, totalArticles: archive.articleCount, articlesProcessed }
} catch (error) {
logger.error('Error processing ZIM file:', error)
throw error

View File

@ -0,0 +1,41 @@
/**
* Decision for whether a batched ZIM ingestion should dispatch a continuation.
*
* This is the pure, I/O-free core of the batch loop in
* `RagService.processZIMFile` (mirrors `decideScanAction` in
* `kb_ingest_decision.ts`).
*
* The signal MUST be the number of articles the extractor *consumed*, never the
* number that produced chunks. An article yields zero chunks whenever its text
* is empty after HTML cleaning redirect stubs, category listings, image/video
* wrappers, PDF containers. Those are perfectly normal ZIM entries, but they are
* invisible to any count derived from the returned chunks.
*
* Gating on chunk-derived counts silently truncates ingestion the first time a
* window of `batchSize` articles happens to contain mostly empty ones: the
* continuation is never dispatched and every remaining article in the archive is
* skipped. That is not a rare edge case it is the norm for scraped-site and
* media-heavy archives, where the opening entries are usually navigation pages.
*
* `articlesProcessed < batchSize` means the extractor's iterator ran dry, which
* is the only reliable end-of-archive signal available: `archive.articleCount`
* cannot serve as an upper bound, because `iterByPath()` legitimately yields
* more article entries than that figure for some archives.
*
* A full final batch costs one extra dispatch that extracts nothing and stops.
* That is deliberate: over-running by a single empty batch is cheap, while
* stopping one batch early loses the remainder of the archive.
*/
export interface ZimBatchProgress {
/** Articles the extractor consumed in this batch (not articles that produced chunks). */
articlesProcessed: number
/** The article ceiling requested for this batch. */
batchSize: number
}
export function hasMoreArticleBatches({
articlesProcessed,
batchSize,
}: ZimBatchProgress): boolean {
return articlesProcessed >= batchSize
}

View File

@ -0,0 +1,55 @@
import * as assert from 'node:assert/strict'
import { test } from 'node:test'
import { hasMoreArticleBatches } from '../../app/utils/zim_batch_decision.js'
const BATCH = 50
test('continues while the extractor keeps filling the batch', () => {
assert.equal(hasMoreArticleBatches({ articlesProcessed: BATCH, batchSize: BATCH }), true)
})
test('stops once the extractor runs out of article entries', () => {
assert.equal(hasMoreArticleBatches({ articlesProcessed: 12, batchSize: BATCH }), false)
})
test('stops on an archive smaller than one batch', () => {
assert.equal(hasMoreArticleBatches({ articlesProcessed: 1, batchSize: BATCH }), false)
})
test('stops on an exhausted iterator that yielded nothing', () => {
assert.equal(hasMoreArticleBatches({ articlesProcessed: 0, batchSize: BATCH }), false)
})
/**
* Regression: Medicine LibreTexts (23,171 articles) embedded only 16 chunks.
*
* Its first 50 article entries are navigation and category pages; only 10 of
* them produced any text. The previous gate counted articles that produced
* chunks, so it evaluated `10 >= 50` -> false, never dispatched a continuation,
* and abandoned the remaining 23,121 articles. A full batch was consumed, so the
* ingestion must continue regardless of how little text came out of it.
*/
test('continues through a full batch that produced almost no text', () => {
const articlesWithContent = 10
assert.equal(
hasMoreArticleBatches({ articlesProcessed: BATCH, batchSize: BATCH }),
true,
'a full batch must continue even when most of its articles are empty'
)
assert.equal(
articlesWithContent >= BATCH,
false,
'the old chunk-derived gate stopped here — this is the bug being fixed'
)
})
/**
* Regression: a media archive (Canadian Prepper, 92 articles / 187 media files)
* yields zero text chunks, but its articles still have to be walked to the end
* rather than abandoned after the first batch.
*/
test('walks a zero-text archive to the end instead of stopping at batch one', () => {
assert.equal(hasMoreArticleBatches({ articlesProcessed: BATCH, batchSize: BATCH }), true)
assert.equal(hasMoreArticleBatches({ articlesProcessed: 42, batchSize: BATCH }), false)
})