fix(segmentation): hash the extracted text and let the plan key fire

This commit is contained in:
Kjell Tore Guttormsen 2026-09-02 14:38:20 +02:00
commit 9e9bb8645d
13 changed files with 272 additions and 40 deletions

View file

@ -41,6 +41,7 @@ from .segmentation import (
SegmentationPlan,
SegmentEntry,
assert_plan_applies,
observed_extractor_version,
slice_segments,
)
from .structure import (
@ -411,17 +412,22 @@ def _render_segments(
A refusal is therefore reported once, for the document, rather than once
per segment: the operator's unit of review is the document they dropped.
"""
extractor_id = Path(path.name).suffix.lower().lstrip(".") or "none"
assert_plan_applies(
plan,
source_sha256=hashlib.sha256(source_bytes).hexdigest(),
extractor_id=Path(path.name).suffix.lower().lstrip(".") or "none",
# The plan's own value, passed through. Door B can observe WHICH
# extractor ran (the suffix is what dispatches it at `extract.py`) but
# not the version of a third-party parser -- `pdfplumber`'s transitive
# `pdfminer.six` pin is the measured example. Naming the key and
# leaving its value to whoever knows it is the same division D5 makes
# for `bundle_id`; a fabricated value here would make S5b decorative.
extractor_version=plan.extractor_version,
# `text` is the canonical extracted text this run produced, AFTER the
# profile's renderer -- exactly the string the plan's offsets index.
text_sha256=hashlib.sha256(text.encode("utf-8")).hexdigest(),
extractor_id=extractor_id,
# OBSERVED, never the plan's own value passed back in. That is what
# this line used to do, and comparing a value with itself made the
# version half of S5b unable to fail: a plan adjudicated under one
# converter replayed silently under another. The version of a
# third-party parser is knowable here after all -- `pdfplumber`'s
# transitive `pdfminer.six` pin is the measured example, and
# `observed_extractor_version` is where each row names its source.
extractor_version=observed_extractor_version(extractor_id),
)
sliced = slice_segments(text, plan)

View file

@ -38,6 +38,7 @@ from __future__ import annotations
from collections.abc import Mapping, Sequence
from dataclasses import dataclass, field
from importlib import metadata
from pathlib import PurePosixPath
from typing import Any
@ -50,6 +51,7 @@ from .materialize import reduce_to_id_grammar
PLAN_FIELDS = (
"version",
"source_sha256",
"text_sha256",
"extractor_id",
"extractor_version",
"adjudicated_at",
@ -71,7 +73,29 @@ FORBIDDEN_COMPONENTS = ("", ".", "..")
#: :func:`plan_cache_key` returns them. Named so a mismatch message can say
#: WHICH one moved -- that is what tells an operator whether to re-run the
#: proposer or re-adjudicate by hand.
CACHE_KEY_COMPONENTS = ("source_sha256", "extractor_id", "extractor_version")
CACHE_KEY_COMPONENTS = ("source_sha256", "text_sha256", "extractor_id", "extractor_version")
#: The version reported for the stdlib extractors. They have no third-party
#: parser to name, so the value is this package's own contract for them: a
#: frozen literal, bumped by hand when a core extractor changes the text it
#: returns. Frozen rather than derived from the package version, which moves on
#: every release and would expire every stored adjudication for no reason.
STDLIB_EXTRACTOR_VERSION = "stdlib-1"
#: Extractor ids answered by the stdlib registry. `none` is a dropped file with
#: no suffix, which the proposer and the run path both reduce to that literal.
_STDLIB_EXTRACTOR_IDS = frozenset({"md", "txt", "csv", "json", "html", "htm", "none"})
#: Extractor ids answered by the vendored converter. Held here rather than
#: imported from the extraction registry, which must not be made to depend on
#: the contract layer; a row added there and not here fails loudly on the first
#: proposal for that type rather than silently naming the wrong version.
_CONVERTED_EXTRACTOR_IDS = frozenset({"docx", "xlsx", "pptx", "odt", "rtf"})
#: The distribution whose version fixes a PDF's extracted text. `pdfplumber`
#: pins it exactly and the frozen-text fixtures are pinned against that pin,
#: so it -- not `pdfplumber` -- is what a stored adjudication is keyed to.
_PDF_DISTRIBUTION = "pdfminer.six"
@dataclass(frozen=True)
@ -98,14 +122,19 @@ class SegmentEntry:
class SegmentationPlan:
"""An adjudicated split, keyed to the extraction it was adjudicated against.
The three extractor fields are not decoration. Source bytes cannot see an
extractor swap or a version bump, so `source_sha256` alone would still
match while every offset in `entries` had silently moved -- see
:func:`assert_plan_applies`.
The four keyed fields are not decoration. Source bytes cannot see an
extractor swap, a version bump or a profile's renderer, so `source_sha256`
alone would still match while every offset in `entries` had silently moved
-- see :func:`assert_plan_applies`. `text_sha256` is the one that closes
it: it hashes the canonical extracted text, which is the string the offsets
actually index, so it moves whenever anything upstream of the offsets
moves. The other three stay because they name WHICH thing moved, and that
is what tells an operator whether to re-run the proposer or re-adjudicate.
"""
version: str
source_sha256: str
text_sha256: str
extractor_id: str
extractor_version: str
adjudicated_at: str
@ -316,6 +345,7 @@ def parse_segmentation_plan(payload: Mapping[str, Any]) -> SegmentationPlan:
return SegmentationPlan(
version=_require_str(payload, "version", where="the segmentation plan"),
source_sha256=_require_str(payload, "source_sha256", where="the segmentation plan"),
text_sha256=_require_str(payload, "text_sha256", where="the segmentation plan"),
extractor_id=_require_str(payload, "extractor_id", where="the segmentation plan"),
extractor_version=_require_str(payload, "extractor_version", where="the segmentation plan"),
adjudicated_at=_require_str(payload, "adjudicated_at", where="the segmentation plan"),
@ -323,24 +353,76 @@ def parse_segmentation_plan(payload: Mapping[str, Any]) -> SegmentationPlan:
)
def plan_cache_key(plan: SegmentationPlan) -> tuple[str, str, str]:
"""The triple an adjudication is cached under: source AND extractor identity.
def plan_cache_key(plan: SegmentationPlan) -> tuple[str, str, str, str]:
"""The quadruple an adjudication is cached under: source, text, extractor.
Not the hash alone. `source_sha256` answers "are these the same bytes?",
which is necessary and not sufficient: the offsets in a plan index the
canonical EXTRACTED text, and swapping the extractor or bumping its version
can re-shape that text while the source bytes are untouched. Keyed on the
hash alone, a stored adjudication would be replayed against text the
adjudicator never saw, and every span would land somewhere plausible and
wrong. This is design requirement S5b.
Not the source hash alone. `source_sha256` answers "are these the same
bytes?", which is necessary and not sufficient: the offsets in a plan index
the canonical EXTRACTED text, and swapping the extractor, bumping its
version or applying a profile's renderer can re-shape that text while the
source bytes are untouched. Keyed on the source hash alone, a stored
adjudication would be replayed against text the adjudicator never saw, and
every span would land somewhere plausible and wrong. This is design
requirement S5b.
`text_sha256` is the component that makes the claim true rather than
intended. The other three are each a NAME for a mechanism that can change
the text; the text hash is the text. A converter that reshapes its output
without changing its reported version moves the text hash and nothing else,
which is the measured case the first three miss.
"""
return (plan.source_sha256, plan.extractor_id, plan.extractor_version)
return (plan.source_sha256, plan.text_sha256, plan.extractor_id, plan.extractor_version)
def observed_extractor_version(extractor_id: str) -> str:
"""The version of the extractor that produces this type's canonical text.
The VALUE half of the cache key's fourth component. It exists because the
proposer used to write its OWN version there and the run path used to pass
the plan's value straight back into the check, so the component was
compared with itself and could never differ. Half of S5b was decorative,
and decorative in the direction that persists a bundle nobody adjudicated.
Three cases. A converter row is pinned to the vendored binary this package
refuses to run without. A `pdf` is pinned to whichever `pdfminer.six` the
`[extract]` extra resolved -- the frozen-text fixtures are pinned against
that same version, so an environment that resolved a different one must not
replay an adjudication made in this one. A stdlib row names this package's
own literal, because there is no third party to name.
An id no row answers is REFUSED rather than defaulted. A default would name
a version for an extractor nobody can identify, which is the failure this
whole function exists to remove.
"""
if extractor_id in _STDLIB_EXTRACTOR_IDS:
return STDLIB_EXTRACTOR_VERSION
if extractor_id in _CONVERTED_EXTRACTOR_IDS:
from ._pandoc import PANDOC_VERSION
return PANDOC_VERSION
if extractor_id == "pdf":
try:
return metadata.version(_PDF_DISTRIBUTION)
except metadata.PackageNotFoundError as exc:
raise SegmentationError(
f"cannot name the extractor version for {extractor_id!r}: the "
f"{_PDF_DISTRIBUTION!r} distribution is not installed, so there is "
"nothing to key a stored adjudication to; install the 'extract' extra",
code="segmentation_extractor_mismatch",
) from exc
raise SegmentationError(
f"no extractor version is known for extractor_id {extractor_id!r} — refusing "
"to name a version for an extractor this package cannot identify, which "
"would key an adjudication to a mechanism nobody chose",
code="segmentation_extractor_mismatch",
)
def assert_plan_applies(
plan: SegmentationPlan,
*,
source_sha256: str,
text_sha256: str,
extractor_id: str,
extractor_version: str,
) -> None:
@ -352,7 +434,7 @@ def assert_plan_applies(
which of the three components moved, because that is what tells the
operator whether to re-run the proposer or re-adjudicate by hand.
"""
observed = (source_sha256, extractor_id, extractor_version)
observed = (source_sha256, text_sha256, extractor_id, extractor_version)
differing = [
f"{name}: plan {expected!r} != run {actual!r}"
for name, expected, actual in zip(CACHE_KEY_COMPONENTS, plan_cache_key(plan), observed)