Merge ab31e208d1 into b6f45d4b24

admin/app/services/rag_service.ts

@@ -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,
     }
   }

admin/app/services/zim_extraction_service.ts

@@ -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

admin/app/utils/zim_batch_decision.ts

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

admin/tests/unit/zim_batch_decision.spec.ts

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