From ce69011ab6b36cf1112ac0d02a55a5cce64a93c3 Mon Sep 17 00:00:00 2001 From: Chaim Date: Tue, 28 Apr 2026 17:20:42 +0000 Subject: [PATCH] fix(kb-reingest): two regressions found by 2026-04-28 incident on source 141 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit (1) discard_job was deleting the S3 file even on re-ingest jobs. For a fresh upload the s3_key points at inbox///- which the user just dropped — deletable. For a re-ingest, s3_key points at the EXISTING source's file (kb_source.original_path); deleting it orphans the source and bricks every future re-process attempt with "original file unavailable". User hit this on source 141: started a re-process, hit discard, then the next click on re-process returned HTTP 400 because the underlying PDF was gone. Now skip the S3 delete when source_id is set. (2) start_reingest was setting original_filename = src.title or filename_from_key(key). When the source's title had no extension (literally "החלטה" in this case), parse_first_pages couldn't dispatch by suffix and returned empty text — the classifier then logged "empty text — skipping" and returned {} for ai_suggestions. Now use _filename_from_key(key) directly so the parser always sees a real .pdf/.docx/.txt extension. Source 141's PDF is permanently lost (MinIO bucket is unversioned). Recovery options for the user: (a) upload the same PDF to MinIO at the original key path manually, (b) delete source 141 and re-upload as a fresh source. --- api/services/kb/admin_sources.py | 8 +++++++- api/services/kb/ingest_jobs.py | 21 ++++++++++++++++----- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/api/services/kb/admin_sources.py b/api/services/kb/admin_sources.py index d088318..a2132ab 100644 --- a/api/services/kb/admin_sources.py +++ b/api/services/kb/admin_sources.py @@ -307,6 +307,12 @@ async def start_reingest( "reingest": True, } batch_id = _uuid.uuid4().hex + # Use the actual filename (with extension) — NOT the source title. + # parse_first_pages dispatches by extension (.pdf/.docx/.txt); a + # title like "החלטה" without an extension makes the parser bail + # and return empty text, which then makes the classifier return {}. + # See incident 2026-04-28: source 141 (title="החלטה") had every + # re-ingest classify as empty until this was fixed. pool = await get_pool() async with pool.acquire() as conn: row = await conn.fetchrow( @@ -318,7 +324,7 @@ async def start_reingest( RETURNING id """, source_id, - src.get("title") or _filename_from_key(key), + _filename_from_key(key), key, src["kind"], src["topic_id"], diff --git a/api/services/kb/ingest_jobs.py b/api/services/kb/ingest_jobs.py index 1688981..42d1b50 100644 --- a/api/services/kb/ingest_jobs.py +++ b/api/services/kb/ingest_jobs.py @@ -537,11 +537,22 @@ async def discard_job(job_id: int, requested_by: str | None) -> dict: if job["status"] in ("done",): raise ValueError(f"job {job_id} already done — cannot discard") - # Best-effort S3 cleanup; the row stays as audit trail. - try: - kb_s3._client().delete_object(Bucket=kb_s3._bucket(), Key=job["s3_key"]) - except Exception as e: - logger.warning("[discard] S3 delete failed for %s: %s", job["s3_key"], e) + # Best-effort S3 cleanup — but ONLY for fresh uploads (source_id IS + # NULL). For re-ingest jobs (source_id set), the s3_key points to + # the existing source's file in kb_source.original_path; deleting + # it would orphan the source and break any future re-process. + # See incident 2026-04-28: source 141 lost its PDF when a discarded + # re-ingest deleted the underlying file. + if job.get("source_id") is None: + try: + kb_s3._client().delete_object(Bucket=kb_s3._bucket(), Key=job["s3_key"]) + except Exception as e: + logger.warning("[discard] S3 delete failed for %s: %s", job["s3_key"], e) + else: + logger.info( + "[discard] keeping S3 file for re-ingest job %s (source_id=%s)", + job_id, job["source_id"], + ) reason = f"discarded by {requested_by or 'user'}" async with pool.acquire() as conn: