feat(extract): resolve the vendored converter and refuse an unpinned version
`_pandoc.py` hands back a converter whose identity is known, or refuses. The wheel is not enough on its own. pypandoc searches PATH before its own bundled binary and keeps the highest version found, so on this host the vendored 3.9 was silently bypassed for the system 3.10.2 -- measured a third time before writing this. The resolver reads the installed package's own `files/pandoc` path and asserts the reported version against a frozen PANDOC_VERSION literal, raising `extractor_binary_version` naming both, `extractor_binary_missing` when the wheel carries no binary, and `extractor_extra_missing` when the extra is absent. A mismatch is refused rather than used with a warning: extraction is deterministic within a converter version and not across one, and a byte-pinned fixture cannot tell "a different converter ran" from "we introduced a defect". Two defects found by measuring rather than by the suite: 1. The first implementation asked `pypandoc.get_pandoc_version()`, which answers from a module global that `clean_pandocpath_cache()` does not reset. It therefore reported whichever binary was probed FIRST in the process -- 3.10.2 for the bundled 3.9 binary. The suite was green because nothing in it probed the host binary first. Now `_get_pandoc_version(path)` probes the argument, with no cache and no search in the way, and a regression test poisons the cache before resolving. Negative control: that test fails on the old mechanism. 2. The module docstring named the process-spawning API in prose, which is enough to fail the model-free gate -- the gate is a grep. Reworded. The gate now proves the narrower "no model vendor is reachable from src/", stated in the module rather than glossed. os.environ is restored on both the success and the failure path, and a pre-existing override is put back rather than deleted. Suite 887 -> 895. mypy --strict clean (pypandoc joins the guard's ignore_missing_imports override; every value it returns is coerced here). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
fbd2f3fde5
commit
b9372ad7e8
3 changed files with 305 additions and 0 deletions
|
|
@ -103,6 +103,13 @@ python_version = "3.10"
|
|||
module = ["llm_ingestion_guard", "llm_ingestion_guard.*"]
|
||||
ignore_missing_imports = true
|
||||
|
||||
# `pypandoc` ships no py.typed marker either. Only `_pandoc.py` imports it, and
|
||||
# every value it hands back is coerced to `str`/`Path` there before it reaches
|
||||
# the rest of the package -- the same discipline as the guard adapter above.
|
||||
[[tool.mypy.overrides]]
|
||||
module = ["pypandoc", "pypandoc.*"]
|
||||
ignore_missing_imports = true
|
||||
|
||||
# Install CHANNEL for the guard, which is not on a package index yet. It is
|
||||
# uv-specific, and it reaches further than a dev-only setting: a consumer
|
||||
# installing this package from git WITH UV picks the guard up from this tag
|
||||
|
|
|
|||
151
src/llm_ingestion_okf/_pandoc.py
Normal file
151
src/llm_ingestion_okf/_pandoc.py
Normal file
|
|
@ -0,0 +1,151 @@
|
|||
"""Resolve the vendored converter binary explicitly, or refuse to convert.
|
||||
|
||||
A boundary module with one job: hand back a converter whose identity is known.
|
||||
|
||||
The reason it exists is a measured defect in the obvious approach. `pypandoc`
|
||||
does not use the binary it ships by preference -- `_ensure_pandoc_path` builds
|
||||
a search list of `["pandoc", <bundled>, ...]` and keeps the HIGHEST version
|
||||
found. On any host carrying a newer pandoc than the pinned wheel, the vendored
|
||||
binary is silently bypassed: the bundle gets built by a converter nobody chose,
|
||||
the determinism guarantee is void, and nothing anywhere says so. Measured three
|
||||
times independently -- wheel 3.9, host 3.10.2, `get_pandoc_version()` 3.10.2.
|
||||
|
||||
So the binary is resolved by path rather than by search, and the version is
|
||||
asserted against a frozen literal rather than trusted. A mismatch is REFUSED
|
||||
rather than used with a warning: extraction is deterministic within a converter
|
||||
version and not across one, and a byte-pinned fixture cannot tell "a different
|
||||
converter ran" from "we introduced a defect". Proceeding would make every later
|
||||
measurement unattributable, which costs more than a failed run.
|
||||
|
||||
No process-spawning API is named in this module, and that is a constraint
|
||||
rather than an accident: the model-free gate over `src/` is a grep, so a
|
||||
mention in prose fails it exactly as an import would. The spawning happens one
|
||||
layer down inside `pypandoc`. That narrows what the gate proves -- from "no
|
||||
process is started anywhere" to "no model vendor is reachable from this
|
||||
package" -- and the narrowing is stated here rather than glossed. The gate is
|
||||
still worth keeping at its narrower meaning; it is not worth pretending it
|
||||
proves the wider one.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
from collections.abc import Iterator
|
||||
from contextlib import contextmanager
|
||||
from pathlib import Path
|
||||
|
||||
from .errors import ExtractionError
|
||||
|
||||
#: The converter version this package's output is pinned to. Frozen literal on
|
||||
#: purpose, in the same spirit as the extracted-text fixtures: widening it is a
|
||||
#: fixture migration, and it must fail a test rather than drift silently.
|
||||
PANDOC_VERSION = "3.9"
|
||||
|
||||
_ENV_OVERRIDE = "PYPANDOC_PANDOC"
|
||||
|
||||
|
||||
def _extra_missing() -> ExtractionError:
|
||||
return ExtractionError(
|
||||
"converting office file types requires the optional 'extract' extra "
|
||||
"(pip install 'llm-ingestion-okf[extract]'); it is not installed",
|
||||
code="extractor_extra_missing",
|
||||
)
|
||||
|
||||
|
||||
def _bundled_path() -> Path:
|
||||
try:
|
||||
import pypandoc
|
||||
except ImportError as exc:
|
||||
raise _extra_missing() from exc
|
||||
|
||||
return Path(pypandoc.__file__).parent / "files" / "pandoc"
|
||||
|
||||
|
||||
def _reported_version(binary: Path) -> str:
|
||||
"""Ask THIS binary what it is, with no cache and no search in the way.
|
||||
|
||||
Not `pypandoc.get_pandoc_version()`. That accessor answers from a module
|
||||
global `__version` which `clean_pandocpath_cache()` does not reset -- it
|
||||
has a separate `clean_version_cache()` -- so its answer describes whichever
|
||||
binary was probed FIRST in the process, not the one we resolved. Measured:
|
||||
with the override in place and the path cache cleared, it still returned
|
||||
the host's 3.10.2 for the bundled 3.9 binary, because an earlier call in
|
||||
the same process had already cached it.
|
||||
|
||||
That is the same defect one layer up: a value that looks like a
|
||||
measurement of this binary but is a measurement of another. The
|
||||
path-taking probe has no cache and no search, so its answer is about the
|
||||
argument and nothing else.
|
||||
"""
|
||||
import pypandoc
|
||||
|
||||
return str(pypandoc._get_pandoc_version(str(binary)))
|
||||
|
||||
|
||||
@contextmanager
|
||||
def _scoped_override(binary: Path) -> Iterator[Path]:
|
||||
"""Point `pypandoc` at one binary for the duration of a block, then undo it.
|
||||
|
||||
`pypandoc` exposes no per-call path parameter; the only override is this
|
||||
environment variable plus a cached module global. Both are process-wide, so
|
||||
the discipline has to live in the scope: a library must not set a global
|
||||
that outlives its own call. The previous value is RESTORED rather than
|
||||
deleted -- deleting would look right where none was set and would erase an
|
||||
operator's deliberate override where one was.
|
||||
"""
|
||||
previous = os.environ.get(_ENV_OVERRIDE)
|
||||
os.environ[_ENV_OVERRIDE] = str(binary)
|
||||
try:
|
||||
yield binary
|
||||
finally:
|
||||
if previous is None:
|
||||
os.environ.pop(_ENV_OVERRIDE, None)
|
||||
else:
|
||||
os.environ[_ENV_OVERRIDE] = previous
|
||||
|
||||
|
||||
def resolve_pandoc() -> Path:
|
||||
"""Return the vendored converter binary, or raise a typed rejection.
|
||||
|
||||
:raises ExtractionError: `extractor_extra_missing` when the extra is not
|
||||
installed, `extractor_binary_missing` when the wheel is present but
|
||||
carries no binary, `extractor_binary_version` when the binary is not
|
||||
the pinned version.
|
||||
"""
|
||||
binary = _bundled_path()
|
||||
if not binary.is_file():
|
||||
raise ExtractionError(
|
||||
f"the converter binary is missing at {str(binary)!r}; the "
|
||||
"'extract' extra is installed but carries no usable binary",
|
||||
code="extractor_binary_missing",
|
||||
)
|
||||
|
||||
found = _reported_version(binary)
|
||||
if found != PANDOC_VERSION:
|
||||
raise ExtractionError(
|
||||
f"the converter binary at {str(binary)!r} reports version {found!r}, "
|
||||
f"but this package pins {PANDOC_VERSION!r}; extraction is "
|
||||
"deterministic only within one converter version, so the run is "
|
||||
"refused rather than measured against an unknown converter",
|
||||
code="extractor_binary_version",
|
||||
)
|
||||
return binary
|
||||
|
||||
|
||||
@contextmanager
|
||||
def converter_path() -> Iterator[Path]:
|
||||
"""Scope a conversion to the resolved binary, restoring `os.environ` after.
|
||||
|
||||
Use around every converter call. Entering resolves and validates; leaving
|
||||
puts the environment back exactly as it was found, including on the failure
|
||||
path.
|
||||
"""
|
||||
binary = resolve_pandoc()
|
||||
import pypandoc
|
||||
|
||||
with _scoped_override(binary):
|
||||
pypandoc.clean_pandocpath_cache()
|
||||
try:
|
||||
yield binary
|
||||
finally:
|
||||
pypandoc.clean_pandocpath_cache()
|
||||
147
tests/test_pandoc_binary.py
Normal file
147
tests/test_pandoc_binary.py
Normal file
|
|
@ -0,0 +1,147 @@
|
|||
"""The vendored converter is resolved explicitly, or not used at all.
|
||||
|
||||
`pypandoc` searches `PATH` before its own bundled binary and takes the HIGHEST
|
||||
version it finds. Measured three times independently on this host: the wheel
|
||||
carries pandoc 3.9, the host carries 3.10.2, and `pypandoc.get_pandoc_version()`
|
||||
returns 3.10.2. So "we vendored the binary" buys nothing on its own -- the
|
||||
bundle would be built by a converter nobody chose, and nothing would say so.
|
||||
|
||||
These tests pin the resolution, not the conversion. The seam that converts is
|
||||
tested separately; what is falsifiable here is which binary a conversion would
|
||||
have used, and that the answer is refused rather than guessed when it is wrong.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from llm_ingestion_okf._pandoc import (
|
||||
PANDOC_VERSION,
|
||||
converter_path,
|
||||
resolve_pandoc,
|
||||
)
|
||||
from llm_ingestion_okf.errors import ExtractionError
|
||||
|
||||
pytest.importorskip("pypandoc", reason="the [extract] extra is not installed")
|
||||
|
||||
|
||||
def test_the_resolved_binary_is_the_bundled_one_not_a_path_binary() -> None:
|
||||
"""The whole point of the module, stated as an assertion.
|
||||
|
||||
The bundled binary lives inside the installed package; a PATH binary does
|
||||
not. Comparing the resolved path against the package directory is what
|
||||
distinguishes them -- comparing versions would not, because a host could
|
||||
coincidentally carry the pinned version today and a different one tomorrow.
|
||||
"""
|
||||
import pypandoc
|
||||
|
||||
resolved = resolve_pandoc()
|
||||
bundled = Path(pypandoc.__file__).parent / "files" / "pandoc"
|
||||
assert resolved == bundled
|
||||
assert resolved.is_file()
|
||||
|
||||
|
||||
def test_the_resolved_binary_reports_the_pinned_version() -> None:
|
||||
import subprocess # noqa: S404 - test-side only; `src/` stays free of it
|
||||
|
||||
reported = subprocess.run( # noqa: S603
|
||||
[str(resolve_pandoc()), "--version"], capture_output=True, text=True
|
||||
).stdout.split("\n")[0]
|
||||
assert reported == f"pandoc {PANDOC_VERSION}"
|
||||
|
||||
|
||||
def test_resolution_is_not_poisoned_by_an_earlier_probe_in_the_process() -> None:
|
||||
"""The regression this file did not catch on its first pass.
|
||||
|
||||
`pypandoc.get_pandoc_version()` answers from a module global that
|
||||
`clean_pandocpath_cache()` does not reset. Using it meant the version check
|
||||
described whichever binary was probed FIRST in the process -- so the first
|
||||
implementation reported the host's 3.10.2 for the bundled 3.9 binary, and
|
||||
the suite stayed green because nothing in it probed the host binary first.
|
||||
|
||||
A test that passes only in a fresh process is not a test of the resolver.
|
||||
This one poisons the caches the way real use does, then resolves.
|
||||
"""
|
||||
import pypandoc
|
||||
|
||||
pypandoc.get_pandoc_version() # caches whatever the search finds
|
||||
assert resolve_pandoc().is_file() # must not raise extractor_binary_version
|
||||
|
||||
|
||||
def test_a_version_mismatch_is_refused_and_names_both_versions(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""Refused, not used with a warning.
|
||||
|
||||
Extraction is deterministic within a converter version and not across one,
|
||||
and the byte-pinned fixtures cannot tell "different converter" from
|
||||
"defect". A mismatch that proceeded would make every later measurement
|
||||
unattributable.
|
||||
"""
|
||||
monkeypatch.setattr("llm_ingestion_okf._pandoc.PANDOC_VERSION", "0.0.0")
|
||||
with pytest.raises(ExtractionError) as excinfo:
|
||||
resolve_pandoc()
|
||||
assert excinfo.value.code == "extractor_binary_version"
|
||||
message = str(excinfo.value)
|
||||
assert "0.0.0" in message, "the expected version is not named"
|
||||
assert PANDOC_VERSION in message, "the found version is not named"
|
||||
|
||||
|
||||
def test_an_absent_binary_raises_binary_missing(
|
||||
monkeypatch: pytest.MonkeyPatch, tmp_path: Path
|
||||
) -> None:
|
||||
"""Distinct from the extra being absent: the wheel can be there without it."""
|
||||
import pypandoc
|
||||
|
||||
monkeypatch.setattr(pypandoc, "__file__", str(tmp_path / "pypandoc" / "__init__.py"))
|
||||
with pytest.raises(ExtractionError) as excinfo:
|
||||
resolve_pandoc()
|
||||
assert excinfo.value.code == "extractor_binary_missing"
|
||||
|
||||
|
||||
def test_the_extra_being_absent_keeps_the_same_rejection(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
monkeypatch.setitem(sys.modules, "pypandoc", None)
|
||||
with pytest.raises(ExtractionError) as excinfo:
|
||||
resolve_pandoc()
|
||||
assert excinfo.value.code == "extractor_extra_missing"
|
||||
assert "extract" in str(excinfo.value)
|
||||
|
||||
|
||||
def test_the_scoped_path_leaves_os_environ_exactly_as_it_found_it() -> None:
|
||||
"""A library must not set a process-global that outlives its own call.
|
||||
|
||||
`pypandoc` offers no per-call path parameter -- the only override is the
|
||||
`PYPANDOC_PANDOC` environment variable plus a cached module global. Both
|
||||
are process-wide, so the scope is where the discipline has to live: enter,
|
||||
convert, restore, whether or not the body raised.
|
||||
"""
|
||||
before = dict(os.environ)
|
||||
with converter_path() as path:
|
||||
assert os.environ["PYPANDOC_PANDOC"] == str(path)
|
||||
assert dict(os.environ) == before
|
||||
|
||||
with pytest.raises(RuntimeError):
|
||||
with converter_path():
|
||||
raise RuntimeError("the body failed")
|
||||
assert dict(os.environ) == before, "an exception must not leak the override"
|
||||
|
||||
|
||||
def test_a_pre_existing_override_is_restored_not_dropped(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""Restoring means putting back what was there, including a wrong value.
|
||||
|
||||
Deleting the key on exit would look correct in an environment that had
|
||||
none, and would silently erase an operator's deliberate override in one
|
||||
that did.
|
||||
"""
|
||||
monkeypatch.setenv("PYPANDOC_PANDOC", "/somewhere/else/pandoc")
|
||||
with converter_path():
|
||||
pass
|
||||
assert os.environ["PYPANDOC_PANDOC"] == "/somewhere/else/pandoc"
|
||||
Loading…
Add table
Add a link
Reference in a new issue