feat(extract): five office formats through a table-driven converter seam
`_PANDOC_FORMATS` names the rows and no others: docx, xlsx, pptx, odt, rtf. `.html` stays on its stdlib extractor -- routing it through the converter would buy nothing and would add CVE-2025-51591 (SSRF via an iframe in HTML input), unpatched in every converter version. `.epub` is out on the "no gain" half of that. `_EVIDENCE` records what each row rests on, asserted in the suite rather than written in a comment: docx and xlsx are `measured`, and pptx, odt and rtf are `unmeasured` because the corpus contains ZERO files of those types. Three of five rows therefore leave this step working by construction and never checked against a document anyone wrote, and the assertion is what keeps that visible. Three converter arguments, all measured and none of them hygiene: `--eol=lf --wrap=none` because the defaults produce different bytes (max line length 75 against 447), and `-t markdown` never `-t plain` because plain destroys the headings the segment proposer reads -- 15 entries with two real headings become 13 with none. `_UNPARSED_OPTIONAL_EXTENSIONS` is now empty and kept rather than deleted: the branch still raises, and a future type arriving before its reader belongs there rather than in a new mechanism. This is what the first step was for -- both tests for `extractor_extra_missing` were repointed at the import probe before the set emptied under them. The converter call is isolated behind `_convert_bytes` so the seam's own logic is testable without the binary; the conversion itself is pinned by frozen-text fixtures in the next step. Checked live against a hand-laid docx through the real vendored binary: heading and body both survive. Suite 895 -> 908. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
b9372ad7e8
commit
cd7b792aaf
2 changed files with 293 additions and 10 deletions
|
|
@ -3,11 +3,18 @@
|
|||
All file-type -> text extraction lives here (the guard is text-only). The core
|
||||
registry is stdlib-only and deterministic: `md`/`txt` pass through, `csv` renders
|
||||
the Phase 1 markdown table, `json` is fenced verbatim, and `html`/`htm` are
|
||||
reduced to text with `html.parser`. Binary types (`pdf`/`docx`/`xlsx`) are
|
||||
`[extract]`-gated: `pdf` is extracted with `pdfplumber` when the extra is
|
||||
installed and rejected with the same typed error when it is not (the gate is an
|
||||
import probe, not a membership test), while `docx`/`xlsx` ship no parser yet and
|
||||
always fail fast. Never a silent skip and never a bundled parser in core.
|
||||
reduced to text with `html.parser`. Binary types are `[extract]`-gated and each
|
||||
now has a reader: `pdf` through `pdfplumber`, and the five office rows
|
||||
(`docx`/`xlsx`/`pptx`/`odt`/`rtf`) through a table-driven converter seam over
|
||||
the vendored binary. Every one of those gates is an IMPORT PROBE rather than a
|
||||
membership test, so an absent extra is rejected with the same typed error
|
||||
whatever the type. Never a silent skip and never a bundled parser in core.
|
||||
|
||||
Two of the five office rows are `measured` and three are `unmeasured` -- the
|
||||
corpus this arm was built on contains zero `pptx`, `odt` or `rtf` files, so
|
||||
those rows work by construction and have never met a document anyone wrote.
|
||||
`_EVIDENCE` carries that per row and the suite asserts it, because an
|
||||
unmeasured row must not read as a supported one.
|
||||
|
||||
`extract_text` returns the extracted text *content*; final LF framing and the
|
||||
concept frontmatter are the materializer's concern (Phase 2 step 2), not this
|
||||
|
|
@ -17,9 +24,10 @@ registry's. No guard call and no model call anywhere in this module.
|
|||
from __future__ import annotations
|
||||
|
||||
import csv
|
||||
import functools
|
||||
import io
|
||||
import warnings
|
||||
from collections.abc import Callable
|
||||
from collections.abc import Callable, Sequence
|
||||
from html.parser import HTMLParser
|
||||
from pathlib import Path
|
||||
|
||||
|
|
@ -27,10 +35,63 @@ from .errors import ExtractionError, ExtractionWarning
|
|||
from .render import render_fenced_block, render_table
|
||||
|
||||
# Binary types gated behind the optional `[extract]` extra that it ships no
|
||||
# parser for. `.pdf` is no longer here: its gate is the import probe inside
|
||||
# `_extract_pdf`, which raises the same code and the same message when the
|
||||
# extra is absent. These two still fail fast unconditionally.
|
||||
_UNPARSED_OPTIONAL_EXTENSIONS = frozenset({".docx", ".xlsx"})
|
||||
# parser for. EMPTY, and kept rather than deleted: the dispatch branch it feeds
|
||||
# still raises `extractor_extra_missing`, and a later type that arrives before
|
||||
# its reader belongs here rather than in a new mechanism. Every type the extra
|
||||
# names now has a reader -- `.pdf` through the import probe in `_extract_pdf`,
|
||||
# the five office rows through the converter seam below -- so the gate for all
|
||||
# of them is an import probe, which is why the two tests for that code reach it
|
||||
# that way.
|
||||
_UNPARSED_OPTIONAL_EXTENSIONS: frozenset[str] = frozenset()
|
||||
|
||||
# The office rows: suffix -> the converter's reader name. THESE ROWS AND NO
|
||||
# OTHERS. `.html` is excluded although the converter can read it: it already
|
||||
# has a stdlib extractor here, so routing it through the converter would buy
|
||||
# nothing and would add CVE-2025-51591 (SSRF via an iframe in HTML input),
|
||||
# unpatched in every converter version. `.epub` is excluded on the "no gain"
|
||||
# half of the same reason.
|
||||
_PANDOC_FORMATS: dict[str, str] = {
|
||||
".docx": "docx",
|
||||
".xlsx": "xlsx",
|
||||
".pptx": "pptx",
|
||||
".odt": "odt",
|
||||
".rtf": "rtf",
|
||||
}
|
||||
|
||||
# What each row's behaviour actually rests on, asserted in the suite rather
|
||||
# than written in a comment that rots. `measured` means real corpus files and a
|
||||
# hand-counted fasit; `unmeasured` means the corpus contains ZERO files of that
|
||||
# type, so the row works by construction and has never been checked against a
|
||||
# document anyone wrote. An unmeasured row must not read as a supported one.
|
||||
_EVIDENCE: dict[str, str] = {
|
||||
".docx": "measured",
|
||||
".xlsx": "measured",
|
||||
".pptx": "unmeasured",
|
||||
".odt": "unmeasured",
|
||||
".rtf": "unmeasured",
|
||||
}
|
||||
|
||||
# Load-bearing, all three, and none of them hygiene:
|
||||
#
|
||||
# --eol=lf the defaults produce DIFFERENT BYTES (maximum line length 75
|
||||
# --wrap=none against 447), which a byte-pinned golden registers as a change
|
||||
# nobody made.
|
||||
# -t markdown never `-t plain`: plain destroys the headings the segment
|
||||
# proposer reads. Measured -- a document yielding 15 entries
|
||||
# including two real headings yields 13 with none under `plain`,
|
||||
# so the writer choice silently sets the ceiling for the arm
|
||||
# downstream of it.
|
||||
_PANDOC_WRITER = "markdown"
|
||||
_PANDOC_ARGS = ("--eol=lf", "--wrap=none")
|
||||
|
||||
# Conversion recovers text, on the same terms as PDF extraction: a drawing has
|
||||
# no text to recover. Said out loud on every conversion rather than detected
|
||||
# per document, for the same reason.
|
||||
_OFFICE_LOSSY_WARNING = (
|
||||
"office-file conversion recovers text only: figures, diagrams, images and "
|
||||
"drawn shapes are not represented in the output (their captions are). A "
|
||||
"bundle built from drawn documents is incomplete by construction."
|
||||
)
|
||||
|
||||
# Text extraction recovers text. A figure is a vector drawing with no text to
|
||||
# recover — only its caption survives — so any bundle built from drawn
|
||||
|
|
@ -182,6 +243,59 @@ def _extract_pdf(data: bytes) -> str:
|
|||
return text
|
||||
|
||||
|
||||
def _convert_bytes(source: bytes, to: str, format: str, extra_args: Sequence[str]) -> str:
|
||||
"""The one converter call, isolated so the seam above it is testable.
|
||||
|
||||
Separated for a reason beyond tidiness: every test of the seam's behaviour
|
||||
would otherwise need the binary present and a real office document, which
|
||||
would make the seam's own logic untestable on a machine without the extra.
|
||||
The conversion itself is covered by the frozen-text fixtures instead.
|
||||
"""
|
||||
import pypandoc
|
||||
|
||||
from ._pandoc import converter_path
|
||||
|
||||
with converter_path():
|
||||
return str(pypandoc.convert_text(source, to, format=format, extra_args=list(extra_args)))
|
||||
|
||||
|
||||
def _extract_office(suffix: str, data: bytes) -> str:
|
||||
"""The five office rows, converted through the vendored binary.
|
||||
|
||||
Shaped after `_extract_pdf`: the gate is an import probe rather than a
|
||||
membership test, third-party failures are wrapped rather than leaked, empty
|
||||
output is refused rather than persisted, and the lossiness is stated after
|
||||
the parse rather than before it.
|
||||
"""
|
||||
try:
|
||||
import pypandoc # noqa: F401
|
||||
except ImportError as exc:
|
||||
raise _extra_missing(suffix) from exc
|
||||
|
||||
try:
|
||||
text = _convert_bytes(data, _PANDOC_WRITER, _PANDOC_FORMATS[suffix], _PANDOC_ARGS)
|
||||
except ExtractionError:
|
||||
raise
|
||||
except Exception as exc: # noqa: BLE001 - third-party converter, wrapped never leaked
|
||||
raise ExtractionError(
|
||||
f"the converter failed on this {suffix} file: {exc}",
|
||||
code="extractor_convert_error",
|
||||
) from exc
|
||||
|
||||
text = text.strip()
|
||||
if not text:
|
||||
raise ExtractionError(
|
||||
f"the converter returned no text for this {suffix} file; refused "
|
||||
"rather than persisted as an empty concept",
|
||||
code="extractor_empty_conversion",
|
||||
)
|
||||
|
||||
# After the parse, not before: a run that produced no text has nothing to
|
||||
# be lossy about, and warning there would just add noise to a failure.
|
||||
warnings.warn(_OFFICE_LOSSY_WARNING, ExtractionWarning, stacklevel=3)
|
||||
return text
|
||||
|
||||
|
||||
_CORE_EXTRACTORS: dict[str, Callable[[bytes], str]] = {
|
||||
".md": _extract_passthrough,
|
||||
".txt": _extract_passthrough,
|
||||
|
|
@ -195,6 +309,7 @@ _CORE_EXTRACTORS: dict[str, Callable[[bytes], str]] = {
|
|||
# registry so "adds no runtime dependency" stays readable at a glance.
|
||||
_OPTIONAL_EXTRACTORS: dict[str, Callable[[bytes], str]] = {
|
||||
".pdf": _extract_pdf,
|
||||
**{suffix: functools.partial(_extract_office, suffix) for suffix in _PANDOC_FORMATS},
|
||||
}
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -133,6 +133,174 @@ def test_pdf_without_the_extra_installed_keeps_the_same_rejection(
|
|||
assert "extract" in str(excinfo.value)
|
||||
|
||||
|
||||
# --- office types: the table-driven converter seam ---
|
||||
|
||||
|
||||
def test_every_office_row_names_its_reader() -> None:
|
||||
"""The table is the contract; these are all the rows there are.
|
||||
|
||||
Written as an exact equality rather than a series of `in` checks: a row
|
||||
added without a fixture and without an evidence class is the failure mode
|
||||
this pins, and only equality catches an ADDITION.
|
||||
"""
|
||||
from llm_ingestion_okf.extract import _PANDOC_FORMATS
|
||||
|
||||
assert _PANDOC_FORMATS == {
|
||||
".docx": "docx",
|
||||
".xlsx": "xlsx",
|
||||
".pptx": "pptx",
|
||||
".odt": "odt",
|
||||
".rtf": "rtf",
|
||||
}
|
||||
|
||||
|
||||
def test_html_and_epub_are_excluded_from_the_converter_on_purpose() -> None:
|
||||
"""`.html` has a stdlib extractor; routing it through the converter would
|
||||
buy nothing and would add CVE-2025-51591 (SSRF via an iframe in HTML
|
||||
input), which is unpatched in every converter version. `.epub` is out for
|
||||
the same "no gain" half of that reason.
|
||||
"""
|
||||
from llm_ingestion_okf.extract import _CORE_EXTRACTORS, _PANDOC_FORMATS
|
||||
|
||||
assert ".html" not in _PANDOC_FORMATS
|
||||
assert ".epub" not in _PANDOC_FORMATS
|
||||
assert ".html" in _CORE_EXTRACTORS
|
||||
|
||||
|
||||
def test_evidence_class_is_asserted_not_commented() -> None:
|
||||
"""Which rows were measured is a fact about this work, not a footnote.
|
||||
|
||||
`.pptx`, `.odt` and `.rtf` have denominator ZERO in the corpus this arm was
|
||||
measured on. A comment saying so rots; an assertion that names them keeps
|
||||
an unmeasured row from quietly presenting as a supported one.
|
||||
"""
|
||||
from llm_ingestion_okf.extract import _EVIDENCE, _PANDOC_FORMATS
|
||||
|
||||
assert set(_EVIDENCE) == set(_PANDOC_FORMATS), "every row needs an evidence class"
|
||||
assert {s for s, e in _EVIDENCE.items() if e == "measured"} == {".docx", ".xlsx"}
|
||||
assert {s for s, e in _EVIDENCE.items() if e == "unmeasured"} == {
|
||||
".pptx",
|
||||
".odt",
|
||||
".rtf",
|
||||
}
|
||||
|
||||
|
||||
def test_the_unparsed_set_is_empty_now_that_every_row_has_a_reader() -> None:
|
||||
"""The membership branch that used to raise `extractor_extra_missing` has
|
||||
no members left. This is why both tests for that code were repointed at the
|
||||
import probe before this step landed.
|
||||
"""
|
||||
from llm_ingestion_okf.extract import _UNPARSED_OPTIONAL_EXTENSIONS
|
||||
|
||||
assert _UNPARSED_OPTIONAL_EXTENSIONS == frozenset()
|
||||
|
||||
|
||||
def test_the_converter_is_called_with_the_load_bearing_arguments(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""`--eol=lf --wrap=none` and the markdown writer, all three measured.
|
||||
|
||||
Not hygiene, and not style. The defaults produce DIFFERENT BYTES (maximum
|
||||
line length 75 against 447), which a byte-pinned golden would register as a
|
||||
change nobody made. And `-t plain` destroys the headings the segment
|
||||
proposer reads: measured, a document yielding 15 entries including two real
|
||||
headings yields 13 with none once the writer is `plain`.
|
||||
"""
|
||||
import llm_ingestion_okf.extract as extract_module
|
||||
|
||||
seen: dict[str, object] = {}
|
||||
|
||||
def fake_convert(source: object, to: str, format: str, extra_args: object) -> str:
|
||||
seen.update(to=to, format=format, extra_args=list(extra_args)) # type: ignore[call-overload]
|
||||
return "# Heading\n\nBody."
|
||||
|
||||
monkeypatch.setattr(extract_module, "_convert_bytes", fake_convert)
|
||||
with pytest.warns(ExtractionWarning):
|
||||
text = extract_text("note.docx", b"PK\x03\x04")
|
||||
|
||||
assert text == "# Heading\n\nBody."
|
||||
assert seen["to"] == "markdown"
|
||||
assert seen["format"] == "docx"
|
||||
assert "--eol=lf" in seen["extra_args"] # type: ignore[operator]
|
||||
assert "--wrap=none" in seen["extra_args"] # type: ignore[operator]
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("name", "reader"),
|
||||
[
|
||||
("a.docx", "docx"),
|
||||
("b.xlsx", "xlsx"),
|
||||
("c.pptx", "pptx"),
|
||||
("d.odt", "odt"),
|
||||
("e.rtf", "rtf"),
|
||||
],
|
||||
)
|
||||
def test_each_row_dispatches_to_its_reader(
|
||||
name: str, reader: str, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
import llm_ingestion_okf.extract as extract_module
|
||||
|
||||
seen: dict[str, object] = {}
|
||||
|
||||
def fake_convert(source: object, to: str, format: str, extra_args: object) -> str:
|
||||
seen["format"] = format
|
||||
return "text"
|
||||
|
||||
monkeypatch.setattr(extract_module, "_convert_bytes", fake_convert)
|
||||
with pytest.warns(ExtractionWarning):
|
||||
extract_text(name, b"bytes")
|
||||
assert seen["format"] == reader
|
||||
|
||||
|
||||
def test_an_empty_conversion_is_refused_not_persisted(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""Same reason as `extractor_empty_pdf`: an empty concept is the silent
|
||||
skip this registry exists to prevent."""
|
||||
import llm_ingestion_okf.extract as extract_module
|
||||
|
||||
monkeypatch.setattr(
|
||||
extract_module,
|
||||
"_convert_bytes",
|
||||
lambda source, to, format, extra_args: " \n\t ",
|
||||
)
|
||||
with pytest.raises(ExtractionError) as excinfo:
|
||||
extract_text("empty.docx", b"bytes")
|
||||
assert excinfo.value.code == "extractor_empty_conversion"
|
||||
|
||||
|
||||
def test_a_converter_failure_is_wrapped_never_leaked(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
import llm_ingestion_okf.extract as extract_module
|
||||
|
||||
def boom(source: object, to: str, format: str, extra_args: object) -> str:
|
||||
raise RuntimeError("pandoc said no")
|
||||
|
||||
monkeypatch.setattr(extract_module, "_convert_bytes", boom)
|
||||
with pytest.raises(ExtractionError) as excinfo:
|
||||
extract_text("bad.docx", b"bytes")
|
||||
assert excinfo.value.code == "extractor_convert_error"
|
||||
assert "pandoc said no" in str(excinfo.value)
|
||||
|
||||
|
||||
def test_office_conversion_warns_that_it_is_lossy(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""Drawn content does not survive extraction here either, and the warning
|
||||
is emitted after the parse -- a run that produced no text has nothing to be
|
||||
lossy about."""
|
||||
import llm_ingestion_okf.extract as extract_module
|
||||
|
||||
monkeypatch.setattr(
|
||||
extract_module,
|
||||
"_convert_bytes",
|
||||
lambda source, to, format, extra_args: "body",
|
||||
)
|
||||
with pytest.warns(ExtractionWarning):
|
||||
extract_text("note.docx", b"bytes")
|
||||
|
||||
|
||||
# --- pdf: the [extract] parser (pdfplumber) ---
|
||||
|
||||
# The expected text is frozen against a committed fixture ON PURPOSE. Extraction
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue