From 92e5a89ff994c39ff6c6d30ee40d062064270e47 Mon Sep 17 00:00:00 2001 From: nammovuivui Date: Fri, 23 Oct 2026 17:47:58 +0700 Subject: [PATCH] fix(translate): keep pages with no text instead of failing the run BabelDOC raises ExtractTextError ("The document contains no paragraphs.") on a chunk with no text, and a chunk is one page, so one blank, scanned or image-only page ended the whole translation as "BabelDOC chunk N failed". A 346-page book has 37 empty pages, the first of them page 2, so translating it whole failed four seconds in, every time. This fix was written on 2026-10-22 (14092cf) but only ever lived on the unmerged fix/skip-textless-pages and fix/release-freed-memory branches; no PR was opened, so main never had it. Ported here onto current main. pdf_pages.pages_without_text screens the selected pages before BabelDOC runs, sparing them a whole pipeline run; catching ExtractTextError in run_one_chunk is the backstop for pages with text BabelDOC still can't use, such as its "too many CID paragraphs" refusal. A skipped page still emits chunk_ready, re-sending the latest rolling path rather than rebuilding it, and a run in which every selected page is textless is an error, not a copy of the source labelled translated. Re-verified on this branch (PyMuPDF 1.25.5, Argos, the real pipeline): pages 1-5 of the book failed on main with 0 pages done and now finish 5/5 with pages 2 and 4 unchanged; pages 27-31 finish with the blank 28 and 30 kept and the "Part I" title page translated; page 2 alone is the "None of the selected pages has text" error; and an ExtractTextError reported for one of three pages keeps that page and finishes the run. The screen's 7 tests were seen red against two stubs (one finding no page, one finding every page) before the real helper. (cherry picked from commit 14092cfe2663f70b6d2799a72a62dd79e8003600) Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 7 + docs/architecture/translation-pipeline.md | 42 ++++++ .../processors/pdf_pages.py | 17 +++ .../processors/processor.py | 121 +++++++++++++++--- tests/test_pdf_pages.py | 80 ++++++++++++ 5 files changed, 246 insertions(+), 21 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 46934a8..e4e2abc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -255,6 +255,13 @@ rule has, unless noted, a test. They are the things most easily undone by are gone; a `config.toml` that still holds them is ignored and loses them on the next save). The Pages box only chooses *which* pages; the output is always the whole document, with unselected pages copied from the original. +- **A page with no text is kept as the original, never fails the run**, with + every provider (BabelDOC only reads the text layer). `pdf_pages.pages_without_text` + screens pages before BabelDOC runs; catching `ExtractTextError` in + `run_one_chunk` is the backstop. Keep both. A skipped page still emits + `chunk_ready` (re-sending the latest rolling path, not rebuilding it), and a + run with every page skipped is an error. Only the screen has a test; the + processor half is checked by hand. - Two caches (whole-PDF, paragraph), both SQLite, content-addressed, versioned via `storage/migrations.py`. `is_cacheable_artifact` decides what may be written. diff --git a/docs/architecture/translation-pipeline.md b/docs/architecture/translation-pipeline.md index a447fe5..375a78f 100644 --- a/docs/architecture/translation-pipeline.md +++ b/docs/architecture/translation-pipeline.md @@ -275,6 +275,48 @@ The Pages box resets when a different document opens (`setOriginalPdfPath`). Accepting the offer writes the chosen pages back into it, so Re-translate and Retry ask for the same pages. +## A page with no text is kept, not failed + +BabelDOC raises `ExtractTextError` ("The document contains no paragraphs.") on +a chunk it finds no text in, and a chunk is one page, so a single blank, +scanned or image-only page used to end the whole run as `BabelDOC chunk N +failed`. BabelDOC only translates the text layer, with every provider, so +such a page is now kept as the original, for Argos and the LLMs alike. Two +layers, and neither replaces the other: + +- **The pre-screen**, `pdf_pages.pages_without_text`, runs once before the + workers start and marks a page skipped when PyMuPDF extracts nothing from it + but whitespace. It spares that page a whole BabelDOC run (layout model, + render, save). The bar is "no visible character", not `min_text_length`, + because BabelDOC finds paragraphs its own way and must get every page that + might have one. An OCR'd scan's invisible text layer counts as text, and a + test holds that, since a "visible text only" screen would skip every such + page. What it does skip is text nobody sees either: an optional-content + layer that is switched off, or text outside the CropBox. +- **The backstop**: `run_one_chunk` matches an error event whose `error` is an + `ExtractTextError` instance (BabelDOC passes the exception, not only its + message) and marks the chunk skipped. That covers what the screen can't + see: text BabelDOC still finds no paragraph in, and its "too many CID" + refusals for garbled fonts. The loop runs on to the generator's end rather + than returning, so BabelDOC's thread finishes its cleanup first. BabelDOC + never collects that thread's future, so asyncio logs "Future exception was + never retrieved" for each page skipped here. That is noise, and the + pre-screen keeps it out of the common case. + +A skipped chunk still emits `chunk_ready`, so "N of M pages" reaches M and the +viewer's mask lifts for that page. It **re-sends the latest rolling path rather +than rebuilding**: the file already has the original there, and the store +widens the mask for an unchanged path without reloading it. Rebuilding per +skipped page put hundreds of full copies of a mostly scanned document ahead of +the pages being translated. It is left out of the ETA's rate, since it +finishes at once (counted as the loop consumes it, not when flagged). A run +with *every* page skipped is an error, since it would otherwise return, and +cache, a copy of the source labelled as a translation: refused before any +worker starts when the screen covers the selection, and after the loop when +the backstop does. Only the helper has a unit test. The processor half (`processor.py` +has no harness) was exercised by hand on Argos and Gemini, with the screen on +and off. + ## Each chunk's fonts are subset before it is merged BabelDOC subsets fonts only in its clean step, and PDFusion passes diff --git a/src/desktop_pdf_translator/processors/pdf_pages.py b/src/desktop_pdf_translator/processors/pdf_pages.py index 3cb84d6..3336823 100644 --- a/src/desktop_pdf_translator/processors/pdf_pages.py +++ b/src/desktop_pdf_translator/processors/pdf_pages.py @@ -106,6 +106,23 @@ def text_stats( return TextStats(paragraphs=paragraphs, cjk_chars=cjk, other_chars=other) +def pages_without_text(path: Path, pages: Sequence[int]) -> frozenset[int]: + """The 1-indexed pages among `pages` that carry no text at all — blank, + scanned or image-only pages, which the translator has nothing to do with + and keeps as the original. + + The bar is "nothing but whitespace", not `min_text_length`: BabelDOC finds + paragraphs its own way, so a page with any visible character goes to it, + and its "no paragraphs" error on that page is caught instead. + """ + import fitz # PyMuPDF + + with fitz.open(path) as doc: + return frozenset( + number for number in pages if not doc[number - 1].get_text().strip() + ) + + def split_into_chunks( input_path: Path, chunks_dir: Path, diff --git a/src/desktop_pdf_translator/processors/processor.py b/src/desktop_pdf_translator/processors/processor.py index e2d9a89..bdf412d 100644 --- a/src/desktop_pdf_translator/processors/processor.py +++ b/src/desktop_pdf_translator/processors/processor.py @@ -13,6 +13,7 @@ from pathlib import Path from typing import AsyncGenerator, Optional, Sequence +from babeldoc.babeldoc_exception.BabelDOCException import ExtractTextError from babeldoc.format.pdf.high_level import async_translate as babeldoc_translate from babeldoc.format.pdf.translation_config import TranslationConfig as BabelDOCConfig from babeldoc.format.pdf.translation_config import WatermarkOutputMode as BabelDOCWatermarkMode @@ -50,6 +51,7 @@ ) from .pdf_pages import ( inspect_pdf, + pages_without_text, rebuild_rolling_pdf, split_into_chunks, subset_embedded_fonts, @@ -149,6 +151,11 @@ def _schedule_temp_cleanup(path: Path) -> None: # `_handle_translation_failure` pushes onto the multiplexer queue. _FATAL_KIND = "fatal" +_NOTHING_TO_TRANSLATE = ( + "None of the selected pages has text to translate. They look scanned or " + "image-only, and only a PDF's text layer can be translated." +) + def _effective_parallel_chunks(settings, translator=None) -> int: """Resolve the chunk-concurrency knob. @@ -789,6 +796,17 @@ async def _process_with_babeldoc( ), ) + # BabelDOC fails a chunk it finds no paragraphs in, so a blank, + # scanned or image-only page used to end the whole run. Such a page + # is kept as the original instead — with every provider, since + # BabelDOC only ever translates the text layer. Screening here + # spares those pages a full BabelDOC run (layout model and all); + # the `ExtractTextError` catch in `run_one_chunk` is the backstop + # for pages whose text BabelDOC still can't use. + textless = await asyncio.to_thread(pages_without_text, file_path, pages) + if len(textless) == pages_to_translate: + raise BabelDOCError(_NOTHING_TO_TRANSLATE) + # Priority-driven scheduling. Up to `max_parallel` workers run # concurrently, each picking the **pending** chunk whose first # page is closest to the current `_priority_anchor` (the page the @@ -800,6 +818,9 @@ async def _process_with_babeldoc( chunk_first_page = [page_range[0] - 1 for _, page_range in chunks] chunk_results: list[Optional[Path]] = [None] * total_chunks chunk_errors: list[Optional[BaseException]] = [None] * total_chunks + chunk_skipped: list[bool] = [False] * total_chunks + skipped_seen = 0 + latest_rolling: Optional[Path] = None pending: set[int] = set(range(total_chunks)) completion_queue: asyncio.Queue[int] = asyncio.Queue() assert self._priority_lock is not None # set in process_pdf() @@ -824,6 +845,12 @@ async def pick_next() -> Optional[int]: async def run_one_chunk(idx: int) -> None: chunk_path, page_range = chunks[idx] try: + if all( + page in textless + for page in range(page_range[0], page_range[1] + 1) + ): + chunk_skipped[idx] = True + return chunk_out_dir = chunks_out_root / f"chunk_{idx:03d}" chunk_out_dir.mkdir(parents=True, exist_ok=True) doc_layout_model = await asyncio.to_thread(get_shared_doc_layout_model) @@ -836,6 +863,21 @@ async def run_one_chunk(idx: int) -> None: ) async for event in babeldoc_translate(config): etype = event["type"] + # BabelDOC hands over the exception itself, not just + # its message, so this needs no string matching. + if etype == "error" and isinstance( + event.get("error"), ExtractTextError + ): + logger.warning( + "Chunk %d/%d (page %d): no text BabelDOC can " + "translate (%s); keeping the original", + idx + 1, total_chunks, page_range[0], + event["error"], + ) + # Not `return`: `async_translate` ends by itself + # after an error, once BabelDOC's thread is done. + chunk_skipped[idx] = True + continue if etype == "error": raise BabelDOCError( message=( @@ -850,6 +892,8 @@ async def run_one_chunk(idx: int) -> None: idx + 1, total_chunks, page_range[0], ) break + if chunk_skipped[idx]: + return translated = self._find_translated_file( chunk_out_dir, chunk_path.stem ) @@ -998,6 +1042,8 @@ async def paragraph_ticker() -> None: raise babeldoc_chunk_error(idx + 1, err) _, page_range = chunks[idx] + if chunk_skipped[idx]: + skipped_seen += 1 rolling_path = ( output_dir / f"{file_path.stem}_translated_v{completed_count:03d}.pdf" @@ -1007,33 +1053,46 @@ async def paragraph_ticker() -> None: # other requests aren't stalled per chunk. The plan and a # copy of the results are taken here, on the loop, where # no other chunk can land in between. - segments = rolling_segments( - file_metadata.page_count, - [page_range for _, page_range in chunks], - [result is not None for result in chunk_results], - ) - await asyncio.to_thread( - rebuild_rolling_pdf, - rolling_path, - file_path, - segments, - list(chunk_results), - ) - # Bound disk usage during the run. keep=2, not 1: the - # viewer is still on v{N-1} until it processes the - # chunk_ready below — see cleanup_partial_artifacts. - await asyncio.to_thread(self.cleanup_partial_artifacts, 2) + # + # A skipped chunk changes no page: the latest file already + # has the original there. Sending its path again lifts the + # viewer's mask for those pages without a reload — on a + # mostly scanned document, rebuilding per skipped page was + # hundreds of full copies queued ahead of the real pages. + if chunk_skipped[idx] and latest_rolling is not None: + rolling_path = latest_rolling + else: + segments = rolling_segments( + file_metadata.page_count, + [page_range for _, page_range in chunks], + [result is not None for result in chunk_results], + ) + await asyncio.to_thread( + rebuild_rolling_pdf, + rolling_path, + file_path, + segments, + list(chunk_results), + ) + latest_rolling = rolling_path + # Bound disk usage during the run. keep=2, not 1: the + # viewer is still on v{N-1} until it processes the + # chunk_ready below — see cleanup_partial_artifacts. + await asyncio.to_thread(self.cleanup_partial_artifacts, 2) global_progress = 50.0 + chunk_weight * completed_count - # ETA: need ≥2 completed chunks for a stable rate. + # ETA: need ≥2 completed chunks for a stable rate. A + # skipped chunk finishes at once, so counting it would + # make a run that starts on blank pages look fast. elapsed: Optional[float] = None pages_per_second: Optional[float] = None eta_seconds: Optional[float] = None + translated_count = completed_count - skipped_seen if job_start_time is not None: elapsed = time.time() - job_start_time - if completed_count >= 2 and elapsed and elapsed > 0: - pages_per_second = completed_count / elapsed + if translated_count >= 2 and elapsed and elapsed > 0: + pages_per_second = translated_count / elapsed remaining_chunks = total_chunks - completed_count if pages_per_second > 0 and remaining_chunks > 0: eta_seconds = remaining_chunks / pages_per_second @@ -1078,12 +1137,28 @@ async def paragraph_ticker() -> None: current_step=3, total_steps=4, progress_percent=global_progress, - message=f"{pages_label} translated", + message=( + f"{pages_label}: no text to translate, kept the original" + if chunk_skipped[idx] + else f"{pages_label} translated" + ), ) finally: # Stop the ticker; completion_task is already done by sentinel. ticker_task.cancel() + # Otherwise the run would hand back — and cache — a "translation" + # identical to the source. + if all(chunk_skipped): + raise BabelDOCError(_NOTHING_TO_TRANSLATE) + skipped_pages = sum( + last - first + 1 + for (_, (first, last)), skipped in zip(chunks, chunk_skipped) + if skipped + ) + if skipped_pages: + logger.info("Kept %d page(s) with no text unchanged", skipped_pages) + yield ProgressEvent( type=EventType.PROGRESS_UPDATE, timestamp=time.time(), @@ -1093,7 +1168,11 @@ async def paragraph_ticker() -> None: current_step=3, total_steps=4, progress_percent=90.0, - message="All pages translated", + message=( + f"Done; kept {skipped_pages} page(s) with no text to translate" + if skipped_pages + else "All pages translated" + ), ) except (BabelDOCError, TranslationProcessError): diff --git a/tests/test_pdf_pages.py b/tests/test_pdf_pages.py index 220ab6f..5c4ab0d 100644 --- a/tests/test_pdf_pages.py +++ b/tests/test_pdf_pages.py @@ -22,6 +22,7 @@ ) from desktop_pdf_translator.processors.pdf_pages import ( inspect_pdf, + pages_without_text, rebuild_rolling_pdf, split_into_chunks, subset_embedded_fonts, @@ -149,6 +150,85 @@ def test_cjk_text_is_counted_as_such(tmp_path: Path): assert (stats.paragraphs, stats.cjk_chars, stats.other_chars) == (1, 4, 0) +# --------------------------------------------------------------------------- +# pages_without_text +# --------------------------------------------------------------------------- + + +def _write_mixed_pdf(path: Path, builders: list) -> Path: + """One page per builder, each given its page to draw on (or leave alone).""" + with fitz.open() as doc: + for build in builders: + build(doc.new_page()) + doc.save(path) + return path + + +def _with_text(text: str): + return lambda page: page.insert_text((72, 72), text) + + +def _blank(page) -> None: + pass + + +def _image_only(page) -> None: + pix = fitz.Pixmap(fitz.csRGB, fitz.IRect(0, 0, 20, 20), False) + page.insert_image(fitz.Rect(72, 72, 172, 172), pixmap=pix) + + +def test_a_page_with_text_is_not_returned(tmp_path: Path): + pdf = _write_mixed_pdf(tmp_path / "text.pdf", [_with_text("a whole sentence")]) + assert pages_without_text(pdf, [1]) == frozenset() + + +def test_a_blank_page_is_returned(tmp_path: Path): + pdf = _write_mixed_pdf(tmp_path / "blank.pdf", [_blank]) + assert pages_without_text(pdf, [1]) == frozenset({1}) + + +def test_an_image_only_page_is_returned(tmp_path: Path): + """A scan: ink on the page, but nothing for the translator to read.""" + pdf = _write_mixed_pdf(tmp_path / "scan.pdf", [_image_only]) + assert pages_without_text(pdf, [1]) == frozenset({1}) + + +def test_a_page_whose_only_text_is_whitespace_is_returned(tmp_path: Path): + pdf = _write_mixed_pdf(tmp_path / "spaces.pdf", [_with_text(" \t ")]) + # Guard the fixture: the page must carry a text object, or this is just + # the blank-page test again. + with fitz.open(pdf) as doc: + assert doc[0].get_text() != "" + assert doc[0].get_text().strip() == "" + assert pages_without_text(pdf, [1]) == frozenset({1}) + + +def test_only_the_requested_pages_are_inspected(tmp_path: Path): + pdf = _write_mixed_pdf( + tmp_path / "three.pdf", [_with_text("first"), _blank, _with_text("third")] + ) + assert pages_without_text(pdf, [1, 2, 3]) == frozenset({2}) + assert pages_without_text(pdf, [1, 3]) == frozenset() + + +def test_an_ocred_scan_is_not_returned(tmp_path: Path): + """A scan with an invisible OCR layer has text BabelDOC translates; a + screen that looked only at visible text would skip every such page.""" + + def ocred(page) -> None: + _image_only(page) + page.insert_text((72, 72), "recognised words", render_mode=3) + + pdf = _write_mixed_pdf(tmp_path / "ocred.pdf", [ocred]) + assert pages_without_text(pdf, [1]) == frozenset() + + +def test_a_single_visible_character_is_enough_to_keep_a_page(tmp_path: Path): + """A page number alone is still text BabelDOC gets to try.""" + pdf = _write_mixed_pdf(tmp_path / "numbered.pdf", [_with_text("7")]) + assert pages_without_text(pdf, [1]) == frozenset() + + # --------------------------------------------------------------------------- # split_into_chunks # ---------------------------------------------------------------------------