diff --git a/leanbench/config.py b/leanbench/config.py index dc9f99b..c0201f6 100644 --- a/leanbench/config.py +++ b/leanbench/config.py @@ -17,7 +17,15 @@ DEFAULT_API_KEY_ENV = "DASHSCOPE_API_KEY" def load_dotenv(path: Path | str = ".env") -> None: - """Load ``KEY=VALUE`` lines into os.environ without overwriting what is set.""" + """Load ``KEY=VALUE`` lines into os.environ without overwriting a real value. + + A variable that is set but *empty* counts as unset. This is not pedantry: the + Docker image declares ``ENV DASHSCOPE_API_KEY=`` so the variable is documented + and overridable at ``docker run``, which left it present-and-empty in every + container. With plain ``setdefault`` that empty string shadowed the .env file, + and the only symptom was every OCR page failing with "No API key" -- a whole + 25-page run spent on text the pipeline had already judged unreadable. + """ path = Path(path) if not path.is_file(): return @@ -26,7 +34,10 @@ def load_dotenv(path: Path | str = ".env") -> None: if not line or line.startswith("#") or "=" not in line: continue key, _, value = line.partition("=") - os.environ.setdefault(key.strip(), value.strip().strip('"').strip("'")) + key = key.strip() + if os.environ.get(key): # a non-empty value wins; empty or absent does not + continue + os.environ[key] = value.strip().strip('"').strip("'") def _env(name: str, default: str) -> str: diff --git a/leanbench/document.py b/leanbench/document.py index fa388e2..965a180 100644 --- a/leanbench/document.py +++ b/leanbench/document.py @@ -46,6 +46,10 @@ def normalise_page_text(text: str) -> str: return text +class OcrUnavailable(RuntimeError): + """Raised when OCR was asked for and could not read a single page.""" + + @dataclass(frozen=True) class Page: number: int # 1-based, physical page order in the file @@ -178,6 +182,19 @@ class Document: replaced.append(page.number) say(f" page {page.number}: OCR (embedded text scored {quality})") + # One page the model chokes on is a bad page; every page failing is a broken + # configuration -- a missing key, an unavailable model, no network. Falling + # back page by page is right in the first case and disastrous in the second, + # because the fallback text is exactly the mangled text OCR exists to replace: + # the run completes, costs a full detection pass, and yields nothing. It is + # cheaper to stop here than to explain the empty database afterwards. + if skipped and not replaced: + reason = next(iter(ocr.failures.values()), "no reason recorded") + raise OcrUnavailable( + f"OCR produced nothing for all {len(skipped)} page(s) via " + f"{ocr.backend.name}; first failure: {reason}" + ) + if replaced: self.extractor = f"{self.extractor}+{ocr.backend.name}@{ocr.dpi}dpi" self.meta["ocr"] = { diff --git a/tests/test_config.py b/tests/test_config.py new file mode 100644 index 0000000..25476e7 --- /dev/null +++ b/tests/test_config.py @@ -0,0 +1,69 @@ +"""The .env loader, which is only interesting where it disagrees with itself. + +These tests exist because of one real incident: the Docker image declares +``ENV DASHSCOPE_API_KEY=`` so the variable shows up in ``docker inspect`` and can +be overridden at ``docker run``. That left it set-but-empty inside every container, +where it shadowed the mounted .env file. Every OCR page failed with "No API key", +the pipeline fell back page by page as designed, and a 25-page run finished +successfully having stored nothing. +""" +from __future__ import annotations + +from pathlib import Path + +from leanbench.config import load_dotenv + + +def write_env(directory: Path, body: str) -> Path: + path = directory / ".env" + path.write_text(body, encoding="utf-8") + return path + + +def test_an_empty_variable_does_not_shadow_the_file(tmp_path: Path, monkeypatch) -> None: + """The incident, in one assertion.""" + monkeypatch.setenv("LEANBENCH_TEST_KEY", "") + load_dotenv(write_env(tmp_path, "LEANBENCH_TEST_KEY=from-file\n")) + import os + + assert os.environ["LEANBENCH_TEST_KEY"] == "from-file" + + +def test_a_real_value_still_wins(tmp_path: Path, monkeypatch) -> None: + """Overriding at the command line must keep working -- that is the whole point.""" + monkeypatch.setenv("LEANBENCH_TEST_KEY", "from-environment") + load_dotenv(write_env(tmp_path, "LEANBENCH_TEST_KEY=from-file\n")) + import os + + assert os.environ["LEANBENCH_TEST_KEY"] == "from-environment" + + +def test_an_unset_variable_is_taken_from_the_file(tmp_path: Path, monkeypatch) -> None: + monkeypatch.delenv("LEANBENCH_TEST_KEY", raising=False) + load_dotenv(write_env(tmp_path, "LEANBENCH_TEST_KEY=from-file\n")) + import os + + assert os.environ["LEANBENCH_TEST_KEY"] == "from-file" + + +def test_comments_blanks_and_quotes(tmp_path: Path, monkeypatch) -> None: + monkeypatch.delenv("LEANBENCH_A", raising=False) + monkeypatch.delenv("LEANBENCH_B", raising=False) + monkeypatch.delenv("LEANBENCH_C", raising=False) + load_dotenv(write_env(tmp_path, """ +# a comment +LEANBENCH_A = "quoted value" + +LEANBENCH_B='single' +not a pair +LEANBENCH_C=has=equals=inside +""")) + import os + + assert os.environ["LEANBENCH_A"] == "quoted value" + assert os.environ["LEANBENCH_B"] == "single" + assert os.environ["LEANBENCH_C"] == "has=equals=inside" + + +def test_a_missing_file_is_not_an_error(tmp_path: Path) -> None: + load_dotenv(tmp_path / "nope.env") diff --git a/tests/test_ocr.py b/tests/test_ocr.py index 1a0d580..df52ea3 100644 --- a/tests/test_ocr.py +++ b/tests/test_ocr.py @@ -4,7 +4,7 @@ from pathlib import Path import pytest -from leanbench.document import Document +from leanbench.document import Document, OcrUnavailable from leanbench.ocr import ( OcrCache, OcrReader, @@ -64,6 +64,22 @@ class ExplodingBackend: raise RuntimeError("model refused") +class FlakyBackend: + """Fails on chosen pages only -- the ordinary case of one unreadable page.""" + + name = "flaky:v1" + + def __init__(self, fail_on: set[int]) -> None: + self.fail_on = fail_on + self.seen = 0 + + def transcribe(self, image: bytes) -> str: + self.seen += 1 + if self.seen in self.fail_on: + raise RuntimeError(f"page {self.seen} refused") + return CLEAN + + # -- the quality heuristic ------------------------------------------------- @@ -225,11 +241,37 @@ def test_text_mode_never_touches_the_model(mixed_book: Path) -> None: def test_a_failed_page_keeps_its_embedded_text(mixed_book: Path, monkeypatch) -> None: + """One bad page falls back; the rest of the book is unaffected.""" + monkeypatch.setattr("leanbench.ocr.render_page", lambda *a, **k: b"png") + reader = OcrReader(backend=FlakyBackend(fail_on={2}), cache=None) + document = Document.load(mixed_book, mode="ocr", ocr=reader) + assert document.meta["ocr"]["pages_failed"] == [2] + assert document.meta["ocr"]["pages_replaced"] == [1] + assert "Š" in document.page(2).text, "we fall back, we do not lose the page" + + +def test_total_ocr_failure_is_loud(mixed_book: Path, monkeypatch) -> None: + """Every page failing is a broken configuration, not a run to complete. + + Silently falling back here is the expensive failure: the fallback text is the + mangled text OCR was called in to replace, so the run costs a full detection + pass and stores nothing. + """ monkeypatch.setattr("leanbench.ocr.render_page", lambda *a, **k: b"png") reader = OcrReader(backend=ExplodingBackend(), cache=None) - document = Document.load(mixed_book, mode="ocr", ocr=reader) - assert document.meta["ocr"]["pages_failed"] == [1, 2] - assert "Š" in document.page(2).text, "we fall back, we do not lose the page" + with pytest.raises(OcrUnavailable) as caught: + Document.load(mixed_book, mode="ocr", ocr=reader) + assert "model refused" in str(caught.value), "the real cause must reach the operator" + assert "boom:v1" in str(caught.value) + + +def test_auto_mode_with_nothing_to_do_does_not_raise(tmp_path: Path) -> None: + """No page needed OCR, so nothing failed -- that is success, not an outage.""" + path = tmp_path / "clean.txt" + path.write_text(PROSE, encoding="utf-8") + document = Document.load(path, mode="auto", ocr=OcrReader(backend=ExplodingBackend())) + assert document.meta["ocr"]["pages_replaced"] == [] + assert document.meta["ocr"]["pages_failed"] == [] def test_offsets_stay_usable_after_ocr(mixed_book: Path, monkeypatch) -> None: