From 5ecd7947df0b96d02d89f066e2b48b1c86bc27b8 Mon Sep 17 00:00:00 2001 From: defraction <796174+defraction0@users.noreply.github.com> Date: Fri, 4 Sep 2026 09:50:44 +0200 Subject: [PATCH 01/54] check: a verdict needs a block that exists, and a picture to show for it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every supported, partial and contradicted judgement is now validated against the cited source's own source_map.json: the page must exist, source_block is required rather than optional, and it must sit on the page the verdict names. Anything else is unchecked with a note. The block is what makes the evidence image unconditional — crop_for_anchor takes its crop region from the block's bbox, so a valid block always produces a picture and the anchor phrases only decide whether a red box is drawn on it. highlight enforces the same rule against reality, since a source PDF can be missing from sources_resolved/ and a source map can disagree with the PDF it was built from. A substantive judgement that produced no image is downgraded there too, so the rule holds by construction rather than by inference. The console stops calling an unsearched anchor "not found on the page". CHECK_PROMPT already asked for source_block, and already told the model to omit it only for not_addressed, so no prompt text changed and eval runs stay comparable across this release. Counts do change: a verdict that rested on page-only provenance now reports a gap. The coverage and multisource fixtures grow a source map because a source without one can no longer be judged at all. --- src/papertrace/check.py | 95 ++++++++++-- src/papertrace/cli.py | 44 +++++- tests/test_check_provenance.py | 268 +++++++++++++++++++++++++++++++++ tests/test_coverage.py | 51 ++++++- tests/test_multisource.py | 29 +++- 5 files changed, 465 insertions(+), 22 deletions(-) create mode 100644 tests/test_check_provenance.py diff --git a/src/papertrace/check.py b/src/papertrace/check.py index a1df054..78c0101 100644 --- a/src/papertrace/check.py +++ b/src/papertrace/check.py @@ -532,15 +532,54 @@ def _slug_for_ref(manifest: RefManifest, label: str): _MAX_PAGE_DIGITS = 5 # a page number, not an integer literal -def _judgement_from(entry) -> tuple[dict | None, str]: +@dataclass(frozen=True) +class SourceProvenance: + """What one source actually contains, read from its own source map. + + The yardstick a judgement is held to. Without it a verdict's page and block + are the model's unchecked word for it, which is how `page 99999` and + `block_nope` survived into a report as provenance. + """ + + pages: int + block_pages: dict[str, int] # block id -> the page it is on + + @classmethod + def from_map(cls, smap) -> SourceProvenance: + return cls(pages=smap.pages, block_pages={b.id: b.page for b in smap.blocks}) + + @classmethod + def read(cls, source_map: Path) -> SourceProvenance | None: + """None when the map is missing or unreadable — never a permissive default. + + A guessed yardstick measures nothing. The caller turns None into + `unchecked`, so an unverifiable location is refused rather than trusted. + """ + if not source_map.exists(): + return None + try: + from .models import SourceMap + + return cls.from_map(SourceMap.from_json(source_map)) + except (OSError, ValueError, KeyError, TypeError): + return None + + +def _judgement_from(entry, provenance: SourceProvenance | None) -> tuple[dict | None, str]: """Validate one model response object into claim fields, or say why not. - Total by construction: every branch is an isinstance test, so this cannot - raise. A validator that throws would turn a bug in OUR code into a note - blaming the model — the same laundering `unchecked` exists to prevent. + Total by construction: every branch is an isinstance test or a lookup, so + this cannot raise. A validator that throws would turn a bug in OUR code into + a note blaming the model — the same laundering `unchecked` exists to prevent. Rejection is all-or-nothing. The caller writes no field unless every field validated, so a bad response never leaves half-applied provenance behind. + + `provenance` is the source's own source map. A substantive verdict must + name a page that exists and a block that exists **on that page**, because + the block is what guarantees the reader an evidence image: `crop_for_anchor` + takes its region from the block's bbox, so a valid block always produces a + crop and the anchor phrases only decide whether a red box is drawn on it. """ if not isinstance(entry, dict): return None, f"model returned an unusable verdict (not an object: {type(entry).__name__})" @@ -598,12 +637,44 @@ def _judgement_from(entry) -> tuple[dict | None, str]: if page < 1: return None, unusable_page # highlight does doc[page - 1] - # optional — but a non-string block id is never coerced into one + # the source map is the only thing that can contradict the model here. With + # no map nothing can, so nothing does — and a location nobody can check is + # refused rather than trusted. + if provenance is None: + return None, ( + f"model returned {verdict!r} but the source map could not be read, so " + f"the page and block it names cannot be checked against the source — " + f"re-run `papertrace ingest` for this source, then `papertrace check`" + ) + if page > provenance.pages: + return None, ( + f"model returned {verdict!r} for a passage on page {page}, but the " + f"source has {provenance.pages} page{'s' if provenance.pages != 1 else ''} " + f"— there is no such page to show" + ) + + # REQUIRED, not optional: the block's bbox is what `crop_for_anchor` uses as + # the crop region, so a judgement without one can leave the reader with no + # evidence image at all — a verdict nobody can look at. block = entry.get("source_block") - if block is not None and not isinstance(block, str): + if not isinstance(block, str) or not block.strip(): + return None, ( + f"model returned {verdict!r} with no source_block ({block!r}) — without " + f"one there is no region to crop, so the verdict would carry no evidence " + f"image a reader could check" + ) + block = block.strip() + block_page = provenance.block_pages.get(block) + if block_page is None: + return None, ( + f"model returned {verdict!r} citing {block}, which is not a block of " + f"this source — nothing to crop, nothing to check" + ) + if block_page != page: return None, ( - f"model returned an unusable verdict for this claim: {verdict!r} with a " - f"source_block that is not a block id ({block!r})" + f"model returned {verdict!r} citing {block}, which is on page " + f"{block_page}, not the page {page} it named — a crop of page {page} " + f"would show the reader a different passage" ) # absent means "none offered" and is allowed, as is an empty list. null, a @@ -676,6 +747,11 @@ def check_claims( try: ingest_dir = case_dir / "ingest" / slug annotated = ingest_dir / "annotated.md" + # a missing source_map.json is NOT re-ingested here: `entry.pdf_path` + # may be gone, and turning one absent artifact into a group-wide + # FileNotFoundError buries the real problem. It degrades per + # judgement instead, with a note naming the fix — see + # SourceProvenance.read. if not annotated.exists(): entry = next(e for e in manifest.entries if e.slug == slug) from .ingest import ingest_pdf @@ -716,6 +792,7 @@ def check_claims( # deliberately NO per-claim `except Exception`: a blanket catch would # relabel our own bugs as the model's fault. The per-group except above # stays as scoped — ingest/prompt/_ask failures really are group-wide. + provenance = SourceProvenance.read(case_dir / "ingest" / slug / "source_map.json") for c in group: j = next((x for x in c.judgements if x.source_slug == slug), None) if j is None: # pragma: no cover - group membership implies one @@ -724,7 +801,7 @@ def check_claims( if v is None: j.verdict, j.note = "unchecked", "model returned no verdict for this claim" continue - fields, why = _judgement_from(v) + fields, why = _judgement_from(v, provenance) if fields is None: j.verdict, j.note = "unchecked", why continue diff --git a/src/papertrace/cli.py b/src/papertrace/cli.py index f77e26e..54c14b4 100644 --- a/src/papertrace/cli.py +++ b/src/papertrace/cli.py @@ -622,6 +622,32 @@ def fail(slug, msg): console.print(f"[cyan]{len(uncited)} uncited assertions[/cyan] — see report section") +def _downgrade_unshowable(anchor) -> bool: + """A substantive verdict with no evidence image stops being a verdict. + + `check` validates page and block against the source map, which is what + normally guarantees a crop. This is the same rule enforced against reality: + the PDF can be absent from `sources_resolved/`, and a source map can + disagree with the PDF it was built from. `not_addressed` is exempt — it + never claimed a passage, so it owes no picture. + + Returns True when it downgraded, so the caller can say so on the console. + """ + substantive = ("supported", "partial", "contradicted") + if anchor.verdict not in substantive or anchor.evidence_image: + return False + anchor.verdict = "unchecked" + anchor.note = ( + "no evidence image could be produced for the passage this verdict rests on " + f"(page {anchor.source_page}" + + (f", {anchor.source_block}" if anchor.source_block else "") + + ") — the source PDF is missing from sources_resolved/, or its pages no " + "longer match the source map it was ingested from. Re-run " + "`papertrace refs` and `papertrace check` for this source." + ) + return True + + @app.command(rich_help_panel="Pipeline stages — `run` calls these in order") def highlight( case: Path = typer.Option( @@ -662,12 +688,22 @@ def highlight( if img: a.evidence_image = str(Path(img).relative_to(case / "out")) done += 1 - if a.anchor_located: + # `is True` / `is False` / `is None` — never truthiness. None + # means nothing was ever searched for, and calling that "not + # found on the page" asserts a search that did not happen. + if a.anchor_located is True: console.print(f" [green]✓[/green] {tag}: {a.evidence_image}") + elif a.anchor_located is False: + console.print( + f" [yellow]○ {tag}: {a.evidence_image} — the anchor phrase " + f"was searched for and not found on the page; crop written " + f"unboxed[/yellow]" + ) else: console.print( f" [yellow]○ {tag}: {a.evidence_image} — no anchor phrase " - f"found on the page; crop written unboxed[/yellow]" + f"was offered, so none was searched for; crop written " + f"unboxed[/yellow]" ) elif a.source_slug and a.source_page: # a page the source does not have is not the same as a page that @@ -679,6 +715,10 @@ def highlight( f"but {a.source_slug} has {n} — no page to read, so no crop " f"and no anchor claim[/yellow]" ) + if _downgrade_unshowable(a): + console.print( + f" [yellow]⚠ {tag}: {a.note}[/yellow]" + ) # the claim-level evidence_image must follow the deciding judgement, or # the crop shown beside the headline belongs to a different source c.apply_headline() diff --git a/tests/test_check_provenance.py b/tests/test_check_provenance.py new file mode 100644 index 0000000..e16405c --- /dev/null +++ b/tests/test_check_provenance.py @@ -0,0 +1,268 @@ +"""A substantive verdict must point at a passage the reader can be shown. + +`supported`, `partial` and `contradicted` all assert that a specific piece of +the source settles the claim. Before this, the assertion was unchecked: the +model could name page 99999 and `block_nope`, and the verdict stood. The +report then printed "Page 99999" as provenance and `crop_for_anchor` quietly +produced nothing, so the one claim the reader most wanted to verify was the +one with no picture. + +The bar is the picture. `crop_for_anchor` writes a crop when the region comes +from a valid block, or from an anchor phrase that actually matched the page — +so requiring a **valid block** is what makes the image unconditional. With one, +`crop_evidence` always writes the block; the red box is drawn on top if a +phrase matches inside it. Anchor phrases stay optional because they decide +whether there is a box, not whether there is a picture. + +`not_addressed` is exempt and must stay exempt: the source was read and says +nothing, so there is no passage to point at, and demanding one would force the +model to invent a citation for an absence. +""" + +import json +import sys +from pathlib import Path + +import pytest + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent / "src")) + +from papertrace import check as check_mod # noqa: E402 +from papertrace.check import SourceProvenance, _judgement_from, check_claims # noqa: E402 +from papertrace.models import ClaimResult, RefEntry, RefManifest, SourceMap # noqa: E402 + +# one page, one block on it — the smallest source a verdict can point into +ONE_PAGE = SourceProvenance(pages=1, block_pages={"block_0001": 1}) +TWO_PAGE = SourceProvenance(pages=2, block_pages={"block_0001": 1, "block_0007": 2}) + + +def _entry(**kw) -> dict: + base = { + "id": 1, + "verdict": "supported", + "note": "the source states it", + "source_page": 1, + "source_block": "block_0001", + "anchor_phrases": ["84.3%"], + } + base.update(kw) + return base + + +# --- what a valid judgement still looks like ------------------------------- + + +def test_a_valid_block_on_the_named_page_is_accepted(): + fields, why = _judgement_from(_entry(), ONE_PAGE) + assert why == "" + assert fields["verdict"] == "supported" + assert fields["source_block"] == "block_0001" + + +def test_a_valid_block_with_no_anchor_phrases_is_still_accepted(): + """The block is what guarantees the crop; the phrases only add the red box. + + Rejecting this would discard a reading the model got right and merely + under-quoted — and the reader still gets an image of the exact block, shown + unboxed and captioned as such. + """ + fields, why = _judgement_from(_entry(anchor_phrases=[]), ONE_PAGE) + assert why == "" + assert fields["anchor_phrases"] == [] + + +def test_not_addressed_still_needs_no_page_or_block(): + """Absence of relevant content has no decisive passage by construction.""" + fields, why = _judgement_from( + {"id": 1, "verdict": "not_addressed", "note": "silent on mortality"}, ONE_PAGE + ) + assert why == "" + assert fields["source_page"] is None and fields["source_block"] is None + + +def test_not_addressed_survives_an_unreadable_source_map(): + """It asserts nothing about a location, so there is nothing to validate.""" + fields, why = _judgement_from({"id": 1, "verdict": "not_addressed", "note": "n"}, None) + assert why == "" and fields["verdict"] == "not_addressed" + + +# --- provenance that cannot be true ---------------------------------------- + + +@pytest.mark.parametrize( + ("entry", "expect_in_note"), + [ + (_entry(source_page=99999, source_block=None), "1 page"), + (_entry(source_page=2, source_block=None), "1 page"), + (_entry(source_block="block_9999"), "block_9999"), + (_entry(source_block=None), "no source_block"), + ], +) +def test_impossible_provenance_is_unchecked_not_a_verdict(entry, expect_in_note): + fields, why = _judgement_from(entry, ONE_PAGE) + assert fields is None + assert expect_in_note in why + + +def test_a_block_on_another_page_is_refused(): + """`block_0007` is real, but it is on page 2 — so page 1 is not where it is, + and a crop of page 1 would show the reader something else entirely.""" + fields, why = _judgement_from( + _entry(source_page=1, source_block="block_0007"), TWO_PAGE + ) + assert fields is None + assert "block_0007" in why and "page 2" in why + + +def test_an_unreadable_source_map_refuses_substantive_verdicts(): + """Unverifiable provenance is not verified provenance. The note names the + fix, because the cause is our own artifact, not the model.""" + fields, why = _judgement_from(_entry(), None) + assert fields is None + assert "source map" in why and "papertrace ingest" in why + + +# --- through check_claims, where it actually matters ------------------------ + + +def _write_source(case: Path, slug: str, pages: int = 1, + bbox: tuple = (0.0, 0.0, 10.0, 10.0)) -> None: + """An ingested source: the text and the map that says where its blocks are. + + `bbox` matters only where a crop is actually drawn — the highlight tests + need a region that covers the text on the generated page. + """ + from papertrace.models import Block + + d = case / "ingest" / slug + d.mkdir(parents=True, exist_ok=True) + (d / "annotated.md").write_text("\nThe rate was 84.3%.\n") + SourceMap( + doc=f"{slug}.pdf", + pages=pages, + blocks=[Block("block_0001", "text", 1, bbox, [], "The rate was 84.3%.")], + ).to_json(d / "source_map.json") + + +def _manifest(slug: str) -> RefManifest: + return RefManifest( + manuscript="paper.pdf", + entries=[RefEntry(num="1", raw="ref", status="retrieved", slug=slug, + pdf_path=f"/nonexistent/{slug}.pdf")], + ) + + +def test_check_claims_downgrades_an_impossible_page_to_unchecked(tmp_path, monkeypatch): + _write_source(tmp_path, "smith-2020") + monkeypatch.setattr( + check_mod, "_ask", + lambda prompt, model=None: json.dumps( + [{"id": 1, "verdict": "supported", "note": "yes", + "source_page": 99999, "source_block": "nope", "anchor_phrases": []}] + ), + ) + claims = [ClaimResult(id=1, claim="the rate was 84.3%", location="Results", refs=["1"])] + check_claims(claims, _manifest("smith-2020"), tmp_path) + + j = claims[0].judgements[0] + assert j.verdict == "unchecked" + assert claims[0].verdict == "unchecked" + # the source WAS retrieved — this must never be laundered into a gap + assert claims[0].verdict != "not_retrieved" + + +def test_check_claims_keeps_a_verdict_whose_block_is_real(tmp_path, monkeypatch): + _write_source(tmp_path, "smith-2020") + monkeypatch.setattr( + check_mod, "_ask", + lambda prompt, model=None: json.dumps( + [{"id": 1, "verdict": "contradicted", "note": "says 48%", + "source_page": 1, "source_block": "block_0001", + "anchor_phrases": ["84.3%"]}] + ), + ) + claims = [ClaimResult(id=1, claim="the rate was 84.3%", location="Results", refs=["1"])] + check_claims(claims, _manifest("smith-2020"), tmp_path) + assert claims[0].judgements[0].verdict == "contradicted" + assert claims[0].verdict == "contradicted" + + +# --- the backstop: a verdict with no picture is not a verdict --------------- +# +# Check-time validation makes this nearly unreachable, and "nearly" is not the +# standard. The PDF can be missing from sources_resolved/, and a source map can +# disagree with the PDF it was built from. Without a backstop at the point the +# image is actually produced, the rule holds by inference rather than by +# construction — so `highlight` enforces it again, against reality this time. + + +def _case_with_judgement(tmp_path: Path, *, verdict: str = "supported") -> Path: + from papertrace.models import RunResults, SourceJudgement + + case = tmp_path / "case" + (case / "out").mkdir(parents=True) + _write_source(case, "a-2020", bbox=(72.0, 90.0, 300.0, 110.0)) + + claim = ClaimResult( + id=1, claim="the rate was 84.3%", location="Results", refs=["1"], + judgements=[SourceJudgement( + source_slug="a-2020", ref="1", verdict=verdict, note="the source states it", + source_page=1, source_block="block_0001", anchor_phrases=["84.3%"], + )], + ) + claim.apply_headline() + RunResults(manuscript="m.pdf", claims=[claim]).to_json(case / "out" / "results.json") + _manifest("a-2020").to_json(case / "refs_manifest.json") + return case + + +def test_a_verdict_that_produced_no_evidence_image_is_downgraded(tmp_path): + """The block validated at check time, but the PDF is not in the case folder, + so no crop exists. A `supported` a reader cannot look at is not `supported`.""" + from papertrace import cli + from papertrace.models import RunResults + + case = _case_with_judgement(tmp_path) + cli.highlight(case=case, claim=None) + + after = RunResults.from_json(case / "out" / "results.json") + j = after.claims[0].judgements[0] + assert j.verdict == "unchecked" + assert "evidence image" in j.note + assert after.claims[0].verdict == "unchecked" + + +def test_not_addressed_is_not_downgraded_for_having_no_image(tmp_path): + """It never claimed a passage, so there is no picture it owes anyone.""" + from papertrace import cli + from papertrace.models import RunResults + + case = _case_with_judgement(tmp_path, verdict="not_addressed") + cli.highlight(case=case, claim=None) + + after = RunResults.from_json(case / "out" / "results.json") + assert after.claims[0].judgements[0].verdict == "not_addressed" + + +def test_a_verdict_whose_crop_was_written_survives(tmp_path): + """The positive control: a real PDF, a real block, a crop on disk.""" + import pymupdf + + from papertrace import cli + from papertrace.models import RunResults + + case = _case_with_judgement(tmp_path) + pdf = case / "sources_resolved" / "a-2020.pdf" + pdf.parent.mkdir(parents=True, exist_ok=True) + doc = pymupdf.open() + doc.new_page().insert_text((72, 100), "The rate was 84.3% overall.", fontsize=11) + doc.save(pdf) + doc.close() + + cli.highlight(case=case, claim=None) + + after = RunResults.from_json(case / "out" / "results.json") + j = after.claims[0].judgements[0] + assert j.verdict == "supported" + assert j.evidence_image and (case / "out" / j.evidence_image).exists() + assert j.anchor_located is True diff --git a/tests/test_coverage.py b/tests/test_coverage.py index 2b11d0a..bdf968d 100644 --- a/tests/test_coverage.py +++ b/tests/test_coverage.py @@ -153,15 +153,36 @@ def boom(prompt, model=None): assert claims2[0].verdict == "not_retrieved" +def _ingested(dirpath, slug: str, pages: int = 2) -> None: + """An ingested source: the text AND the map that proves where its blocks are. + + Both, always — a source map is no longer optional. `check` validates every + substantive verdict's page and block against it, so a source without one + can produce no verdict at all. + """ + from papertrace.models import Block, SourceMap + + dirpath.mkdir(parents=True, exist_ok=True) + (dirpath / "annotated.md").write_text( + f"\nText of {slug}.\n" + f"\nMore of {slug}.\n" + ) + SourceMap( + doc=f"{slug}.pdf", pages=pages, + blocks=[ + Block("block_0001", "text", 1, (0.0, 0.0, 100.0, 20.0), [], f"Text of {slug}."), + Block("block_0002", "text", 2, (0.0, 0.0, 100.0, 20.0), [], f"More of {slug}."), + ], + ).to_json(dirpath / "source_map.json") + + def _one_source_manifest(tmp_path, *entries): - """Manifest + an ingested annotated.md for every retrieved entry.""" + """Manifest + a fully ingested source for every retrieved entry.""" from papertrace.models import RefManifest for e in entries: if e.status in ("retrieved", "provided"): - d = tmp_path / "ingest" / e.slug - d.mkdir(parents=True, exist_ok=True) - (d / "annotated.md").write_text(f"\nText of {e.slug}.") + _ingested(tmp_path / "ingest" / e.slug, e.slug) return RefManifest(manuscript="m.pdf", entries=list(entries)) @@ -483,7 +504,7 @@ def test_a_bug_in_our_validator_is_not_relabelled_as_the_models_fault(tmp_path, ' "source_page":1,"anchor_phrases":[]}]', ) - def our_bug(entry): + def our_bug(entry, provenance): raise AttributeError("a bug in PaperTrace, not in the model's answer") monkeypatch.setattr(check_mod, "_judgement_from", our_bug) @@ -868,6 +889,16 @@ def test_the_occurrence_list_is_capped_with_a_pointer_to_results_json(tmp_path): # --- _judgement_from is total by construction, and stays that way ----------- +def _prov(): + """A three-page source with one block per page — enough that the page-shape + guards below fail on the shape, not on a location that doesn't exist.""" + from papertrace.check import SourceProvenance + + return SourceProvenance( + pages=3, block_pages={"block_0001": 1, "block_0002": 2, "block_0003": 3} + ) + + @pytest.mark.parametrize("page", [ "9" * 5000, # passes isascii() and isdigit(), then int() raises "9" * 4301, # one past CPython's default limit @@ -882,7 +913,10 @@ def test_an_absurdly_long_page_number_degrades_instead_of_raising(page): """ import papertrace.check as check_mod - j, note = check_mod._judgement_from({"id": 1, "verdict": "supported", "source_page": page}) + j, note = check_mod._judgement_from( + {"id": 1, "verdict": "supported", "source_page": page, + "source_block": "block_0003"}, _prov() + ) assert j is None assert "unusable" in note @@ -896,7 +930,10 @@ def test_other_page_shapes_still_degrade_rather_than_raise(page): by breaking another. `" 3 "` is deliberately accepted after stripping.""" import papertrace.check as check_mod - j, note = check_mod._judgement_from({"id": 1, "verdict": "supported", "source_page": page}) + j, note = check_mod._judgement_from( + {"id": 1, "verdict": "supported", "source_page": page, + "source_block": "block_0003"}, _prov() + ) if isinstance(page, str) and page.strip() == "3": assert j is not None and j["source_page"] == 3 # a dict, not a dataclass else: diff --git a/tests/test_multisource.py b/tests/test_multisource.py index 78a719d..792814a 100644 --- a/tests/test_multisource.py +++ b/tests/test_multisource.py @@ -304,14 +304,35 @@ def test_a_hand_built_legacy_results_json_loads_and_still_renders(tmp_path): # --- the fan-out ---------------------------------------------------------- +def _ingested(dirpath, slug: str, pages: int = 2) -> None: + """An ingested source: the text AND the map that proves where its blocks are. + + Both, always — a source map is no longer optional. `check` validates every + substantive verdict's page and block against it, so a source without one + can produce no verdict at all. + """ + from papertrace.models import Block, SourceMap + + dirpath.mkdir(parents=True, exist_ok=True) + (dirpath / "annotated.md").write_text( + f"\nText of {slug}.\n" + f"\nMore of {slug}.\n" + ) + SourceMap( + doc=f"{slug}.pdf", pages=pages, + blocks=[ + Block("block_0001", "text", 1, (0.0, 0.0, 100.0, 20.0), [], f"Text of {slug}."), + Block("block_0002", "text", 2, (0.0, 0.0, 100.0, 20.0), [], f"More of {slug}."), + ], + ).to_json(dirpath / "source_map.json") + + def _case(tmp_path: Path, slugs: list[str]): - """A case folder with an ingested annotated.md per source.""" + """A case folder with a fully ingested source per slug.""" from papertrace.models import RefEntry, RefManifest for slug in slugs: - d = tmp_path / "ingest" / slug - d.mkdir(parents=True) - (d / "annotated.md").write_text(f"\nText of {slug}.") + _ingested(tmp_path / "ingest" / slug, slug) manifest = RefManifest( manuscript="m.pdf", entries=[ From 51d4618b4eae84015f81f57f544f53fa7d1eb1cd Mon Sep 17 00:00:00 2001 From: defraction <796174+defraction0@users.noreply.github.com> Date: Fri, 4 Sep 2026 09:51:18 +0200 Subject: [PATCH 02/54] cli: guard the manuscript slot, and stop --parse-only rewriting it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _guard_case only ever ran inside refs, so two routes reached a case's manuscript ingest without it. papertrace ingest never consulted the guard at all: a paper whose stem is literally "manuscript", or any --out naming that path, overwrote /ingest/manuscript — the slot refs fills and coverage_audit reads — while refs_manifest.json still described the first paper. The guard now runs whenever the output is that slot, recognised by shape so --out cannot walk in behind -c. A cited source ingested into /ingest/ is untouched, because check does exactly that. refs --parse-only on a pre-hash case re-ingested the manuscript slot and then returned before writing the manifest, leaving the source map describing one paper and the manifest another. "List references, no network" is an inspection, so it reads the paper into a temporary directory and mutates nothing. The legacy-case warning promised a manifest rewrite that ingest does not do; it now says only what both callers actually do. --- src/papertrace/cli.py | 34 +++++++++++++- tests/test_case_identity.py | 92 +++++++++++++++++++++++++++++++++++++ 2 files changed, 124 insertions(+), 2 deletions(-) diff --git a/src/papertrace/cli.py b/src/papertrace/cli.py index 54c14b4..846996e 100644 --- a/src/papertrace/cli.py +++ b/src/papertrace/cli.py @@ -12,6 +12,7 @@ import os import re import sys +import tempfile from pathlib import Path import typer @@ -136,6 +137,19 @@ def _case_conflict(case: Path, manuscript: Path) -> tuple[str | None, str]: return (None if same else previous.manuscript), "name" +def _manuscript_slot_owner(out: Path) -> Path | None: + """The case folder whose manuscript slot `out` is, or None. + + `/ingest/manuscript` is the one output path that stands for the + audited paper itself. Recognised by shape rather than by flag, so `--out` + cannot walk in behind `-c`'s back. + """ + out = Path(out) + if out.name != "manuscript" or out.parent.name != "ingest": + return None + return out.parent.parent + + def _guard_case(case: Path, manuscript: Path) -> str: """Refuse a case that holds another paper; return what that rested on. @@ -161,8 +175,9 @@ def _guard_case(case: Path, manuscript: Path) -> str: # uninformed and less usable, and `refs` re-ingests to make it true console.print( "[yellow]⚠ this case folder predates content hashing, so its identity is " - "unverified — only the file name was compared. Re-reading the paper from " - "scratch so the manifest and its hash describe the same file.[/yellow]" + "unverified — only the file name was compared, and two different papers " + "are routinely both called the same thing. The paper is re-read from " + "scratch rather than trusted from cache.[/yellow]" ) return basis @@ -361,6 +376,14 @@ def ingest( # ingest -c foo` used to fail with "No such option: -c" while its # neighbours all took it. --out stays authoritative and unchanged. out = out or (case or default_case(pdf)) / "ingest" / pdf.stem + # the guard is about the manuscript SLOT, not the folder. A cited source + # ingested into /ingest/ is not the audited paper and must stay + # ingestable — `check` does exactly that. But /ingest/manuscript is + # what `refs` filled and `coverage_audit` reads, so a different paper + # landing there is the mixing `_guard_case` exists to prevent, reached by a + # command that never asked it. + if (owner := _manuscript_slot_owner(out)) is not None: + _guard_case(owner, pdf) smap = ingest_pdf(pdf, out, backend=backend) by_type = {t: sum(1 for b in smap.blocks if b.type == t) for t in ("sectionheader", "text", "table", "picture", "list")} @@ -422,6 +445,13 @@ def refs( # to share this one's name, so re-read the paper we were actually given if cached.exists() and basis != "name": smap = SourceMap.from_json(cached) + elif parse_only: + # --parse-only is an inspection: "List references, no network". It must + # not rewrite the case's manuscript slot and then return before the + # manifest catches up, which left the source map describing one paper + # and the manifest another. Read the paper somewhere disposable instead. + with tempfile.TemporaryDirectory() as scratch: + smap = ingest_pdf(manuscript, Path(scratch), backend=backend) else: smap = ingest_pdf(manuscript, ingest_dir, backend=backend) diff --git a/tests/test_case_identity.py b/tests/test_case_identity.py index 70c43cd..e1104f7 100644 --- a/tests/test_case_identity.py +++ b/tests/test_case_identity.py @@ -281,3 +281,95 @@ def test_the_wizard_suggests_the_folder_batch_mode_would_use(tmp_path): pdf.parent.mkdir(parents=True) pdf.write_bytes(b"%PDF-1.4\n") assert wizard._suggest_case(pdf) == str(cli.default_case(pdf)) + + +# --- the two ways around the guard ----------------------------------------- +# +# `_guard_case` only ever ran inside `refs`. Two other paths could write into a +# case's manuscript slot: `ingest` never consulted the guard at all, and +# `refs --parse-only` re-ingested a legacy case and then returned before the +# manifest caught up. Both leave one case folder describing two papers, which +# is precisely the state the guard exists to make impossible. + + +def test_ingest_refuses_to_overwrite_another_papers_manuscript_slot(tmp_path, offline): + """`papertrace ingest manuscript.pdf -c CASE` writes /ingest/manuscript + — the same slot `refs` filled and `coverage_audit` reads. A different paper + landing there leaves the source map describing NEW and the manifest OLD. + """ + case = tmp_path / "case" + old_pdf = _paper(tmp_path / "old" / "manuscript.pdf", "OLD", "10.1000/old") + new_pdf = _paper(tmp_path / "new" / "manuscript.pdf", "NEW", "10.1000/new") + + cli.refs(manuscript=old_pdf, case=case, provided=None, email="test@example.org", + parse_only=False, backend="pymupdf") + + with pytest.raises(typer.Exit): + cli.ingest(pdf=new_pdf, out=None, case=case, backend="pymupdf") + + smap = json.loads((case / "ingest" / "manuscript" / "source_map.json").read_text()) + body = " ".join(b.get("text", "") for b in smap["blocks"]) + assert "NEW" not in body, "a different paper overwrote the case's manuscript" + assert "OLD" in body + + +def test_ingest_of_a_cited_source_into_the_same_case_is_untouched(tmp_path, offline): + """The guard is about the manuscript slot, not the folder. A cited source + ingested into `/ingest/` is not the audited paper and must stay + ingestable — guarding it would break `check`'s own source ingest.""" + case = tmp_path / "case" + paper = _paper(tmp_path / "a" / "paper.pdf", "PAPER", "10.1000/paper") + source = _paper(tmp_path / "b" / "smith-2020.pdf", "SOURCE", "10.1000/src") + + cli.refs(manuscript=paper, case=case, provided=None, email="test@example.org", + parse_only=False, backend="pymupdf") + cli.ingest(pdf=source, out=None, case=case, backend="pymupdf") + + assert (case / "ingest" / "smith-2020" / "source_map.json").exists() + + +def test_parse_only_on_a_legacy_case_leaves_the_case_coherent(tmp_path, offline): + """`--parse-only` says "List references, no network" — an inspection. On a + legacy case it re-ingested into the manuscript slot and then returned before + writing the manifest, so the source map described NEW while the manifest and + its (absent) hash still described OLD. + """ + case = tmp_path / "case" + old_pdf = _paper(tmp_path / "old" / "paper.pdf", "OLD", "10.1000/old") + new_pdf = _paper(tmp_path / "new" / "paper.pdf", "NEW", "10.1000/new") + + cli.refs(manuscript=old_pdf, case=case, provided=None, email="test@example.org", + parse_only=False, backend="pymupdf") + payload = json.loads((case / "refs_manifest.json").read_text()) + del payload["manuscript_sha256"] + (case / "refs_manifest.json").write_text(json.dumps(payload)) + + cli.refs(manuscript=new_pdf, case=case, provided=None, email="test@example.org", + parse_only=True, backend="pymupdf") + + smap = json.loads((case / "ingest" / "manuscript" / "source_map.json").read_text()) + body = " ".join(b.get("text", "") for b in smap["blocks"]) + manifest = RefManifest.from_json(case / "refs_manifest.json") + raws = " ".join(e.raw for e in manifest.entries) + assert ("NEW" in body) == ("NEW PAPER" in raws), ( + "the source map and the manifest describe different papers" + ) + + +def test_parse_only_still_lists_the_new_papers_references(tmp_path, offline, capsys): + """Not mutating the case must not mean reading the wrong paper: the listing + is of the file that was passed, whatever the case folder holds.""" + case = tmp_path / "case" + old_pdf = _paper(tmp_path / "old" / "paper.pdf", "OLD", "10.1000/old") + new_pdf = _paper(tmp_path / "new" / "paper.pdf", "NEW", "10.1000/new") + + cli.refs(manuscript=old_pdf, case=case, provided=None, email="test@example.org", + parse_only=False, backend="pymupdf") + payload = json.loads((case / "refs_manifest.json").read_text()) + del payload["manuscript_sha256"] + (case / "refs_manifest.json").write_text(json.dumps(payload)) + capsys.readouterr() + + cli.refs(manuscript=new_pdf, case=case, provided=None, email="test@example.org", + parse_only=True, backend="pymupdf") + assert "NEW PAPER" in capsys.readouterr().out From 21ec296bf23b4ac8a7e0a89fcb208145d19a5df3 Mon Sep 17 00:00:00 2001 From: defraction <796174+defraction0@users.noreply.github.com> Date: Fri, 4 Sep 2026 09:51:40 +0200 Subject: [PATCH 03/54] report: a gap claim owes its per-source state, an anchor its tri-state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claims whose headline is not_retrieved or unchecked are routed out of the main loop into the gap sections, which printed the claim text and nothing else. So a claim citing [1,2] where source 1's check failed and source 2 was never obtainable said neither thing, and a not_addressed from a source that WAS read disappeared behind the unchecked headline that outranks it. All three formats now render the co-citation breakdown, the unretrieved co-citations and one row per judgement with its note. The editor look also labelled a whole section row with items[0].verdict, calling a mixed section whichever verdict came first; it is one row per claim now. anchor_located is True, False or None — searched and located, searched and not located, never searched. Both disclosure helpers gated on evidence_image, so a verdict carrying a page but no crop disclosed nothing at all while the report still printed the page as provenance. They gate on provenance now, and ANCHOR_NO_IMAGE carries the same three tokens with wording that does not describe a picture nobody wrote. A claim with no page still says nothing: silence about nothing is not a dropped disclosure. The parity test gains a gap claim and an image-less anchor, so the token loop covers both branches rather than only the one a crop reaches. --- src/papertrace/disclosures.py | 63 ++++++++++- src/papertrace/templates/report.md.j2 | 25 ++++- .../templates/report_editor.html.j2 | 29 ++++- .../templates/report_terminal.html.j2 | 17 ++- tests/test_anchor_state.py | 91 ++++++++++++++++ tests/test_disclosure_parity.py | 100 ++++++++++++++++++ 6 files changed, 314 insertions(+), 11 deletions(-) create mode 100644 tests/test_anchor_state.py diff --git a/src/papertrace/disclosures.py b/src/papertrace/disclosures.py index 04cb2b8..b65085c 100644 --- a/src/papertrace/disclosures.py +++ b/src/papertrace/disclosures.py @@ -98,6 +98,62 @@ def anchor_state(claim) -> str: ), } +# The same three facts when no crop was written. Same tokens on purpose — the +# parity contract is the token, so a format cannot drop one by taking this +# branch — but the sentence must not describe a picture that does not exist. +ANCHOR_NO_IMAGE: dict[str, Disclosure] = { + "located": Disclosure( + key="anchor", + level="info", + token=ANCHOR_LOCATED_TOKEN, + text=( + f"{ANCHOR_LOCATED_TOKEN} — the anchor phrase was located on this page " + "by text search, though no evidence image was written for it." + ), + short=f"{ANCHOR_LOCATED_TOKEN} — no evidence image", + ), + "not_located": Disclosure( + key="anchor", + level="warn", + token=ANCHOR_NOT_LOCATED_TOKEN, + text=( + f"{ANCHOR_NOT_LOCATED_TOKEN}, and no evidence image was produced — so " + "the page named above is the only provenance this verdict carries." + ), + short=f"{ANCHOR_NOT_LOCATED_TOKEN} — and no evidence image", + ), + "unknown": Disclosure( + key="anchor", + level="warn", + token=ANCHOR_UNKNOWN_TOKEN, + text=( + f"{ANCHOR_UNKNOWN_TOKEN} — no anchor phrase was searched for, or the " + "highlight step did not run, and no evidence image was produced. " + "Nothing here claims a match." + ), + short=f"{ANCHOR_UNKNOWN_TOKEN} — and no evidence image", + ), +} + + +def anchor_disclosure(anchor) -> Disclosure | None: + """The anchor caption for one claim or judgement, or None if it owes none. + + Gated on *provenance*, not on the picture. A judgement that names a page has + made a claim about where the evidence is, and owes the reader a statement + about whether anything was found there — whether or not a crop was written. + Gating on `evidence_image` was how a verdict with a page number and no crop + came to disclose nothing at all. + + A claim with no page (`not_retrieved`, or a check that failed before any + location was offered) gets None: silence about nothing is not a dropped + disclosure. + """ + if getattr(anchor, "source_page", None) is None: + return None + table = ANCHOR if getattr(anchor, "evidence_image", None) else ANCHOR_NO_IMAGE + return table[anchor_state(anchor)] + # -------------------------------------------------------------------------- # run-level and claim-level rules @@ -428,7 +484,8 @@ def judgement_disclosures(j) -> list[Disclosure]: has no co-citations and no breakdown of its own, only the anchor state of the single page it points at. """ - return [ANCHOR[anchor_state(j)]] if j.evidence_image else [] + d = anchor_disclosure(j) + return [d] if d else [] def claim_disclosures(claim) -> list[Disclosure]: @@ -438,6 +495,6 @@ def claim_disclosures(claim) -> list[Disclosure]: out.append(_sources(claim)) if claim.unjudged_refs: out.append(_unjudged(claim)) - if claim.evidence_image: - out.append(ANCHOR[anchor_state(claim)]) + if (d := anchor_disclosure(claim)) is not None: + out.append(d) return out diff --git a/src/papertrace/templates/report.md.j2 b/src/papertrace/templates/report.md.j2 index c296400..d41a5bf 100644 --- a/src/papertrace/templates/report.md.j2 +++ b/src/papertrace/templates/report.md.j2 @@ -53,8 +53,13 @@ Manuscript: `{{ r.manuscript }}` · Sources: `{{ r.refs_available }} / {{ r.refs {% if j.evidence_image %} ![evidence]({{ j.evidence_image }}) -{% for d in judgement_disclosures(j) %}{% if d.level == "warn" %}*⚠️ {{ d.text }}*{% else %}*{{ d.token }}*{% endif %}{% endfor %} {% endif %} +{% for d in judgement_disclosures(j) %} +{% if not j.evidence_image %} + +{% endif %} +{% if d.level == "warn" %}*⚠️ {{ d.text }}*{% else %}*{{ d.token }}*{% endif %} +{% endfor %} {% endfor %} {% else %} {% if c.source_slug %} @@ -65,8 +70,13 @@ Manuscript: `{{ r.manuscript }}` · Sources: `{{ r.refs_available }} / {{ r.refs {% if c.evidence_image %} ![evidence]({{ c.evidence_image }}) -{% for d in claim_disclosures(c) if d.key == "anchor" %}{% if d.level == "warn" %}*⚠️ {{ d.text }}*{% else %}*{{ d.token }}*{% endif %}{% endfor %} {% endif %} +{% for d in claim_disclosures(c) if d.key == "anchor" %} +{% if not c.evidence_image %} + +{% endif %} +{% if d.level == "warn" %}*⚠️ {{ d.text }}*{% else %}*{{ d.token }}*{% endif %} +{% endfor %} {% endif %} {% if c.note %} @@ -86,7 +96,16 @@ Reported as such — never filled in from memory. {% for section, items in gaps.items() %} - **{{ section }}** ({{ items|length }}): {% for c in items %} - - [{{ c.refs|join(', ') }}] {{ c.claim }}{% if c.note %} — *{{ c.note }}*{% endif %} + - {{ c.label }} · [{{ c.refs|join(', ') }}] {{ c.claim }}{% if c.note %} — *{{ c.note }}*{% endif +%} +{% for d in claim_disclosures(c) if d.key in ("sources", "unjudged_refs") %} + - {{ '⚠️ ' if d.level == 'warn' else '' }}{{ d.text }} +{% endfor %} +{% for j in c.judgements %} + - `{{ j.source_slug }}` cited as [{{ j.ref }}] — **{{ j.verdict }}**{% if j.source_page %} · p.{{ j.source_page }}{% endif %}{% if j.note %} — *{{ j.note }}*{% endif +%} +{% for d in judgement_disclosures(j) %} + - {{ '⚠️ ' if d.level == 'warn' else '' }}{{ d.text }} +{% endfor %} +{% endfor %} {% endfor %} {% endfor %} {% if r.uncited %} diff --git a/src/papertrace/templates/report_editor.html.j2 b/src/papertrace/templates/report_editor.html.j2 index 5e42f7c..ab80c4e 100644 --- a/src/papertrace/templates/report_editor.html.j2 +++ b/src/papertrace/templates/report_editor.html.j2 @@ -145,6 +145,10 @@
![evidence] {{ j.source_slug }} · page {{ j.source_page }} · {% for d in judgement_disclosures(j) %}{% if d.level == "warn" %}⚠ {{ d.text }}{% else %}{{ d.token }}{% endif %}{% endfor %}
+{% else %} +{% for d in judgement_disclosures(j) %} +

{{ '⚠ ' if d.level == 'warn' else '' }}{{ d.text }}

+{% endfor %} {% endif %} {% endfor %} {% else %} @@ -157,6 +161,10 @@
![evidence] {{ c.source_slug }} · page {{ c.source_page }} · {% for d in claim_disclosures(c) if d.key == "anchor" %}{% if d.level == "warn" %}⚠ {{ d.text }}{% else %}{{ d.token }}{% endif %}{% endfor %}
+{% else %} +{% for d in claim_disclosures(c) if d.key == "anchor" %} +

{{ '⚠ ' if d.level == 'warn' else '' }}{{ d.text }}

+{% endfor %} {% endif %} {% if c.note %}

{{ c.note }}

@@ -178,9 +186,26 @@

Either the cited PDF could not be obtained, or the check step failed on an available source (see notes). Reported as such — never filled in from memory.

+{# one row per CLAIM, not per section: `items[0].verdict` labelled a section + holding one not_retrieved and one unchecked as whichever came first #} {% for section, items in gaps.items() %} - - +{% for c in items %} + + +{% endfor %} {% endfor %}
{{ section }}{{ items[0].verdict|replace("_", " ") }} · {{ items|length }}{% for c in items %}{{ c.claim }}{% if not loop.last %}·{% endif %}{% endfor %}
{{ section if loop.first else "" }}{{ c.verdict|replace("_", " ") }}{{ c.claim }} +{% if c.note %} +
{{ c.note }}
+{% endif %} +{% for d in claim_disclosures(c) if d.key in ("sources", "unjudged_refs") %} +
{{ '⚠ ' if d.level == 'warn' else '' }}{{ d.text }}
+{% endfor %} +{% for j in c.judgements %} +
{{ j.source_slug }} cited as [{{ j.ref }}] — {{ j.verdict }}{% if j.source_page %} · p.{{ j.source_page }}{% endif %}{% if j.note %} — {{ j.note }}{% endif %}
+{% for d in judgement_disclosures(j) %} +
{{ '⚠ ' if d.level == 'warn' else '' }}{{ d.text }}
+{% endfor %} +{% endfor %} +
{% if scout and (scout.newer or scout.overlooked or scout.error) %} diff --git a/src/papertrace/templates/report_terminal.html.j2 b/src/papertrace/templates/report_terminal.html.j2 index 62d8c85..4c2070b 100644 --- a/src/papertrace/templates/report_terminal.html.j2 +++ b/src/papertrace/templates/report_terminal.html.j2 @@ -114,18 +114,18 @@ {% endif %} {% if j.evidence_image %}
+{% endif %} {% for d in judgement_disclosures(j) %}
{{ '⚠ ' if d.level == 'warn' else '' }}{{ d.short }}
{% endfor %} -{% endif %} {% endfor %} {% else %} {% if c.evidence_image %}
+{% endif %} {% for d in claim_disclosures(c) if d.key == "anchor" %}
{{ '⚠ ' if d.level == 'warn' else '' }}{{ d.short }}
{% endfor %} -{% endif %} {% if c.note %}
└─ {{ c.note }}
{% endif %} @@ -168,7 +168,18 @@
not verified — source not retrieved or check failed ({{ gap_total }} / {{ r.claims|length }})
the cited PDF couldn’t be fetched — or the check step failed on an available source — so the claim is reported unverified, never guessed.
{% for section, items in gaps.items() %} -
{{ section|lower }}[{{ items|length }}]{% for c in items %}{{ c.claim }}{% if not loop.last %}·{% endif %}{% endfor %}
+{% for c in items %} +
{{ section|lower if loop.first else "" }}{{ c.verdict|replace("_", " ") }}{{ c.claim }}{% if c.note %} · {{ c.note }}{% endif %}
+{% for d in claim_disclosures(c) if d.key in ("sources", "unjudged_refs") %} +
{{ '⚠ ' if d.level == 'warn' else '' }}{{ d.short }}
+{% endfor %} +{% for j in c.judgements %} +
[{{ j.ref }}]{{ j.source_slug }} · {{ j.verdict }}{% if j.source_page %} · p.{{ j.source_page }}{% endif %}{% if j.note %} · {{ j.note }}{% endif %}
+{% for d in judgement_disclosures(j) %} +
{{ '⚠ ' if d.level == 'warn' else '' }}{{ d.short }}
+{% endfor %} +{% endfor %} +{% endfor %} {% endfor %} {% if scout and (scout.newer or scout.overlooked or scout.error) %} diff --git a/tests/test_anchor_state.py b/tests/test_anchor_state.py new file mode 100644 index 0000000..df27d7d --- /dev/null +++ b/tests/test_anchor_state.py @@ -0,0 +1,91 @@ +"""True, False and None are three different facts, and stay three. + +- `True` — an anchor phrase was searched for and located. +- `False` — searched for and NOT located. +- `None` — no search was possible or attempted (no phrases offered, or the + highlight step never ran). + +Two ways the tri-state was being flattened. The CLI branched on truthiness, so +`None` printed "no anchor phrase found on the page" — asserting a search that +never happened. And both disclosure helpers gated on `evidence_image`, so a +judgement with a page but no crop disclosed nothing at all: the reader saw a +verdict with a page number and no statement about whether anything backed it. + +The gate is *provenance*, not the picture. A claim with no page says nothing, +because there is nothing to say. +""" + +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent / "src")) + +from papertrace.disclosures import ( # noqa: E402 + ANCHOR_LOCATED_TOKEN, + ANCHOR_NOT_LOCATED_TOKEN, + ANCHOR_UNKNOWN_TOKEN, + anchor_state, + claim_disclosures, + judgement_disclosures, +) +from papertrace.models import ClaimResult, SourceJudgement # noqa: E402 + + +def _j(**kw) -> SourceJudgement: + base = dict(source_slug="a-2020", ref="1", verdict="supported", source_page=3, + source_block="block_0007", anchor_phrases=["84.3%"]) + base.update(kw) + return SourceJudgement(**base) + + +def test_anchor_state_keeps_three_names(): + assert anchor_state(_j(anchor_located=True)) == "located" + assert anchor_state(_j(anchor_located=False)) == "not_located" + assert anchor_state(_j(anchor_located=None)) == "unknown" + + +def test_a_judgement_with_no_crop_still_discloses_its_anchor_state(): + """The old gate. No `evidence_image`, so the reader was told nothing — + while the report still printed "Page 3" as if it were provenance.""" + ds = judgement_disclosures(_j(anchor_located=False, evidence_image=None)) + assert [d.token for d in ds] == [ANCHOR_NOT_LOCATED_TOKEN] + + +def test_an_unknown_anchor_with_no_crop_is_disclosed_too(): + ds = judgement_disclosures(_j(anchor_phrases=[], anchor_located=None, evidence_image=None)) + assert [d.token for d in ds] == [ANCHOR_UNKNOWN_TOKEN] + + +def test_a_judgement_with_no_page_discloses_nothing(): + """Silence about nothing is not a dropped disclosure. An unretrieved source + has no page, so there is no anchor claim to qualify.""" + assert judgement_disclosures(_j(source_page=None, verdict="unchecked")) == [] + + +def test_the_no_crop_wording_never_mentions_a_crop(): + """Same token — the parity contract holds — but the sentence must not + describe a picture that was not written.""" + with_crop = judgement_disclosures(_j(anchor_located=False, evidence_image="e/x.png"))[0] + without = judgement_disclosures(_j(anchor_located=False, evidence_image=None))[0] + + assert with_crop.token == without.token == ANCHOR_NOT_LOCATED_TOKEN + assert "crop" in with_crop.text + assert "crop" not in without.text + assert "no evidence image" in without.text + + +def test_located_needs_no_no_crop_variant_but_still_only_fires_with_provenance(): + d = judgement_disclosures(_j(anchor_located=True, evidence_image="e/x.png"))[0] + assert d.token == ANCHOR_LOCATED_TOKEN + + +def test_claim_level_disclosure_follows_the_same_rule(): + claim = ClaimResult(id=1, claim="c", location="Results", refs=["1"], + verdict="supported", source_slug="a-2020", source_page=3, + anchor_phrases=["84.3%"], anchor_located=False) + keys = [d.key for d in claim_disclosures(claim)] + assert "anchor" in keys + + ungrounded = ClaimResult(id=2, claim="c", location="Results", refs=["1"], + verdict="not_retrieved") + assert "anchor" not in [d.key for d in claim_disclosures(ungrounded)] diff --git a/tests/test_disclosure_parity.py b/tests/test_disclosure_parity.py index 31af3df..aeceecc 100644 --- a/tests/test_disclosure_parity.py +++ b/tests/test_disclosure_parity.py @@ -292,3 +292,103 @@ def test_references_resumed_round_trips_and_older_manifests_still_load(tmp_path) path.write_text(_json.dumps(payload)) jsonschema.validate(_json.loads(path.read_text()), _json.loads(schema_path.read_text())) assert RefManifest.from_json(path).references_resumed is False + + +# --- claims whose HEADLINE is a pipeline state ------------------------------ +# +# `gaps_by_location()` routes any claim whose headline is `not_retrieved` or +# `unchecked` out of the main loop and into the gap section — which printed the +# claim text and nothing else. So a claim citing [1,2] where source 1's check +# failed and source 2 was never obtainable said neither thing, and a +# `not_addressed` from a source that WAS successfully read vanished behind the +# `unchecked` headline that outranks it. + + +def _gap_claim() -> ClaimResult: + """One source read and silent, one source's check failed, one never obtained. + + The headline is `unchecked`: a failed check makes "every available source + was read and none addressed it" an assertion the run cannot make. + """ + from papertrace.models import SourceJudgement + + claim = ClaimResult( + id=4, + claim="the intervention halved readmissions", + location="Discussion", + refs=["1", "2", "3"], + judgements=[ + SourceJudgement(source_slug="read-2019", ref="1", verdict="not_addressed", + note="reports incidence only; silent on readmission"), + SourceJudgement(source_slug="failed-2021", ref="2", verdict="unchecked", + note="check failed (TimeoutError) — the source WAS retrieved"), + ], + unjudged_refs=["3"], + ) + claim.apply_headline() + assert claim.verdict == "unchecked" + return claim + + +def test_a_gap_claims_disclosures_reach_all_three_formats(tmp_path): + claim = _gap_claim() + results = RunResults(manuscript="m.pdf", claims=[claim]) + rendered = _render(results, tmp_path) + + fired = claim_disclosures(claim) + assert {d.key for d in fired} == {"sources", "unjudged_refs"} + for d in fired: + for name, body in rendered.items(): + assert d.token in body, f"{d.key}: token {d.token!r} missing from {name}" + + +def test_a_gap_claim_names_each_source_and_its_verdict(tmp_path): + """The per-source rows themselves, not just the summary. A reader has to be + able to see that [1] was read and said nothing while [2] was never read.""" + rendered = _render(RunResults(manuscript="m.pdf", claims=[_gap_claim()]), tmp_path) + + for name, body in rendered.items(): + assert "read-2019" in body, f"the source that WAS read is missing from {name}" + assert "failed-2021" in body, f"the source whose check failed is missing from {name}" + assert "not_addressed" in body or "DOES NOT ADDRESS" in body, ( + f"a successfully-checked not_addressed verdict is invisible in {name}" + ) + + +def test_a_gap_claim_keeps_its_note_in_every_format(tmp_path): + """The two HTML looks printed no note at all for gap claims.""" + rendered = _render(RunResults(manuscript="m.pdf", claims=[_gap_claim()]), tmp_path) + for name, body in rendered.items(): + assert "silent on readmission" in body, f"per-source note missing from {name}" + + +def test_a_gap_section_row_does_not_label_a_mixed_section_with_one_verdict(tmp_path): + """The editor look printed `items[0].verdict` for the whole row, so a + section holding one `not_retrieved` and one `unchecked` claimed both were + whichever came first.""" + gap = _gap_claim() + other = ClaimResult(id=5, claim="a second claim", location="Discussion", refs=["9"], + verdict="not_retrieved", note="cited source not available (paywalled)") + rendered = _render(RunResults(manuscript="m.pdf", claims=[gap, other]), tmp_path) + + # scoped to the gap table: both verdicts appear elsewhere on the page (the + # summary counts them), so an unscoped assertion passes even unfixed + editor = rendered["report_editor.html"] + table = editor.split('')[1].split("
")[0] + assert "not retrieved" in table, table + assert "unchecked" in table, table + + +# --- provenance without a picture ------------------------------------------- + + +@pytest.mark.parametrize("anchor_located", [False, None]) +def test_an_anchor_state_without_a_crop_still_reaches_every_format(tmp_path, anchor_located): + """The disclosure used to be gated on `evidence_image`, so a verdict with a + page and no crop told the reader nothing about what backed it.""" + claim = _claim(evidence_image=None, anchor_located=anchor_located, source_page=3) + rendered = _render(RunResults(manuscript="m.pdf", claims=[claim]), tmp_path) + + anchor = next(d for d in claim_disclosures(claim) if d.key == "anchor") + for name, body in rendered.items(): + assert anchor.token in body, f"anchor token missing from {name}" From 47a88558ea6e20f034ae5ad7eefd8e2533da521c Mon Sep 17 00:00:00 2001 From: defraction <796174+defraction0@users.noreply.github.com> Date: Fri, 4 Sep 2026 09:52:03 +0200 Subject: [PATCH 04/54] evals: eligibility before alignment, and two figures named honestly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Eligibility was decided after alignment, so a case that could never be scored still competed for predictions — and a prediction is consumed once. An unresolved gold case sitting on the same citation label as an eligible one took its match, and the eligible case was then reported as the tool's extraction gap. Blame moved off the tool and onto the harness grading it, silently. score() now aligns against the eligible cases alone, and excluded cases keep their row and their reason. They also stop voting in repeated-run agreement: a case nobody could score is not evidence of the model disagreeing with itself. Duplicate prediction ids were collapsed by a dict comprehension that kept whichever came last, so input order decided which of two same-id claims was graded — the one place this module's documented order-independence did not hold. Refused with an error naming them, because the harness cannot know which was meant. require_one_set_id checked a third of what its own error message claimed. Agreement is defined within one (set_id, prompt fingerprint, ingest converter) triple, and all three are checked now: two runs that read different text, or answered different prompts, are two systems, and their difference is not instability. The intersection figure was labelled the upper bound. It is not one — dropping a case whose true agreement is high pulls the mean down, so with three or more runs it can sit either side of the truth. The two figures are complete-case (a population) and penalized (a genuine lower bound). not_addressed also gains its confusion-matrix column; the arithmetic always had four classes and the table printed three, so a mistake was counted and then hidden. --- evals/DESIGN.md | 53 ++++-- evals/README.md | 11 +- evals/agreement.py | 105 ++++++++--- evals/align.py | 13 ++ evals/runners/score_only.py | 36 ++-- evals/scoring.py | 16 +- evals/templates/eval.md.j2 | 9 +- evals/tests/test_agreement.py | 30 +-- evals/tests/test_eval_report.py | 14 +- evals/tests/test_v041_eval_corrections.py | 220 ++++++++++++++++++++++ 10 files changed, 421 insertions(+), 86 deletions(-) create mode 100644 evals/tests/test_v041_eval_corrections.py diff --git a/evals/DESIGN.md b/evals/DESIGN.md index 48c35fb..5a2a832 100644 --- a/evals/DESIGN.md +++ b/evals/DESIGN.md @@ -129,11 +129,17 @@ record and checks each `n` against its declared population, so the *next* denominator that drifts away from what it claims fails a test rather than printing a plausible number. -Judgement classes are `supported`, `partial`, `contradicted`. `not_retrieved` -is a retrieval fact; `unchecked` is a harness error. **Neither is scored as a -wrong verdict** — both get their own rates. This is the review requirement to -keep retrieval failures separate from model-judgement failures, and the -existing verdict enum already encodes the distinction. +Judgement classes are `supported`, `partial`, `contradicted` and +`not_addressed` — **four**, since `not_addressed` was added to the verdict +vocabulary. It is a judgement like the others: the model read the source and +found it silent on the claim, which is a real finding about the citation, not a +failure. It therefore takes a row *and a column* in the confusion matrix, and +counts in `macro_f1` on the same terms as the rest. + +`not_retrieved` is a retrieval fact; `unchecked` is a harness error. **Neither +is scored as a wrong verdict** — both get their own rates. This is the review +requirement to keep retrieval failures separate from model-judgement failures, +and the existing verdict enum already encodes the distinction. | Metric | Numerator / denominator | Population | |---|---|---| @@ -209,20 +215,33 @@ reason whenever anything is `__absent__`, because κ assumes every item is rated by every rater. **The run count is passed explicitly, never inferred from the first vector.** -Inference was safe only while the caller filtered to the intersection first — -the very filter that introduced the bias above. Removing the filter without -passing the count would have swapped a disclosed upward bias for an +Inference was safe only while the caller filtered to the complete-case set +first — the very filter that introduced the bias above. Removing the filter +without passing the count would have swapped a disclosed upward bias for an undisclosed arithmetic error. Ragged input raises. -**Both bounds are printed, side by side.** The *intersection* (only cases -present in every run) is the **upper** bound: it excludes the harness's own -gaps. The *union* (`__absent__`-padded) is the **lower** bound: it charges -those gaps to the model. Neither is the answer alone, so neither is printed -alone, and the omitted cases are named per run. - -**Two different `set_id`s are refused outright.** Averaging agreement across -gold sets produces a number describing no set, and no caveat repairs it. That -is a category error, not a partial comparison. +**Two populations are printed side by side, and only one of them is a bound.** +The *penalized* figure (`__absent__`-padded, every case seen in any run) is a +genuine **lower** bound: filling in any real vote where the harness never asked +can only raise the modal count. The *complete-case* figure (only cases present +in every run) was previously labelled the **upper** bound, and that was wrong — +it drops cases rather than penalising them, and a dropped case whose true +agreement is high pulls the reported mean *down*. With three or more runs the +omitted set can sit either side of the kept set, so complete-case is reported +as a different population ("how stable was the model where we actually asked +it") and explicitly not as a ceiling. The omitted cases are named per run. + +**Only eligible cases vote.** `per_case` keeps excluded rows so they can be +rendered in their own section; they are filtered out before the agreement +vectors are built. A case that was never scoreable cannot be evidence of the +model disagreeing with itself. + +**Runs that are not comparable are refused outright.** Agreement is defined +within one **(`set_id`, prompt fingerprint, ingest converter)** triple, and all +three are checked. Averaging across gold sets produces a number describing no +set; averaging across prompts or across ingest backends compares two different +systems and calls the difference instability. No caveat repairs either — a +category error, not a partial comparison. - **`modal_agreement`** (headline) — mean over cases of (modal verdict count) / k. - `unanimous_rate` — cases where all runs agree. diff --git a/evals/README.md b/evals/README.md index 7d67f1b..5ef9148 100644 --- a/evals/README.md +++ b/evals/README.md @@ -42,10 +42,13 @@ Compare repeated runs: python evals/runners/score_only.py --agreement evals/runs/ evals/runs/ evals/runs/ ``` -Reports the **intersection** (upper bound — only cases every run produced) and -the **union** (lower bound — every case seen in any run, with the gaps charged -to the model) side by side, and names which cases each run omitted. Runs from -two different gold sets are refused outright rather than averaged. +Reports the **complete-case** figure (only cases every run produced — a +different population, not a bound in either direction) and the **penalized** +figure (every case seen in any run, with the gaps charged to the model, which +is a genuine lower bound) side by side, and names which cases each run omitted. +Cases that were never eligible for scoring do not vote. Runs that differ in +gold set, prompt fingerprint or ingest converter are refused outright rather +than averaged. ## Run a live evaluation — costs money diff --git a/evals/agreement.py b/evals/agreement.py index fc1137c..5fdc991 100644 --- a/evals/agreement.py +++ b/evals/agreement.py @@ -18,6 +18,12 @@ not describe. `fleiss_kappa` itself is correct and is not touched; the guard lives in `agreement()`. +**Two figures, and only one of them is a bound.** `agreement_report` reports +a *penalized* figure over every case seen in any run and a *complete-case* +figure over the cases every run answered. The penalized one is a true lower +bound; the complete-case one is a different population and is labelled as such, +because a dropped case whose true agreement is high pulls the mean down. + **The run count is passed, never inferred.** Deriving `k` from the first vector is only safe when something upstream has already guaranteed equal lengths — which used to be the caller's intersection filter, the very thing that @@ -85,60 +91,99 @@ def agreement(vectors: dict[str, list[str]], runs: int) -> dict: } -def require_one_set_id(set_ids: list[str | None]) -> str | None: - """Refuse to aggregate two gold sets. A category error, not a partial view. +def _distinct(values: list) -> list: + """Stable, sortable distinct — values may be dicts, which are unhashable.""" + out: list = [] + for v in values: + if v not in out: + out.append(v) + return out + + +def require_one_provenance(set_ids: list[str | None], + provenances: list[dict] | None = None) -> str | None: + """Refuse to aggregate runs that are not comparable. Returns the set id. - Averaging agreement across different sets produces a number that describes - no set, and there is no caveat that repairs it — so it is refused outright - rather than reported with a warning. + Agreement is only defined within one **(set_id, prompt fingerprint, + converter)** triple. That sentence was already in the error message while + only the first third was checked: two runs of different prompts, or of + different ingest backends, were averaged into a single stability figure + that describes neither. A disagreement between them is not the model being + unstable — it is two different systems being compared. + + A category error, not a partial view, so it is refused outright rather than + reported with a caveat. """ - distinct = sorted({s for s in set_ids}, key=lambda x: (x is None, x)) - if len(distinct) > 1: + sets = _distinct(sorted(set_ids, key=lambda x: (x is None, x))) + if len(sets) > 1: raise ValueError( "refusing to aggregate runs from different gold sets: " - f"{', '.join(repr(d) for d in distinct)}. Agreement is only " + f"{', '.join(repr(d) for d in sets)}. Agreement is only " "defined within one (set_id, prompt fingerprint, converter) triple." ) - return distinct[0] if distinct else None + for field, label in (("prompt_fingerprint", "prompt fingerprint"), + ("converter", "ingest converter")): + values = [(p or {}).get(field) for p in (provenances or [])] + if len(_distinct(values)) > 1: + raise ValueError( + f"refusing to aggregate runs with a different {label}: " + f"{'; '.join(repr(v) for v in _distinct(values))}. A " + f"disagreement between two runs that read different text, or " + f"answered different prompts, is not the model being unstable." + ) -def agreement_report(vectors: dict[str, list[str]], runs: int, - run_labels: list[str], set_ids: list[str | None]) -> dict: - """Both bounds, side by side, with the omissions named. + return sets[0] if sets else None - Reporting only the intersection silently drops the cases one run never - produced; reporting only the union charges the harness's own gaps to the - model. Neither number is the answer on its own, so both are printed and - labelled as what they are. + +def agreement_report(vectors: dict[str, list[str]], runs: int, + run_labels: list[str], set_ids: list[str | None], + provenances: list[dict] | None = None) -> dict: + """Two populations, side by side, with the omissions named. + + **Neither is called a bound except the one that is.** The penalized figure + counts every case seen in any run and scores an ABSENT vote as + disagreement; replacing an ABSENT with any real vote can only raise the + modal count, so it genuinely understates stability and is a lower bound. + + The complete-case figure is *not* an upper bound, and calling it one was + wrong. It drops cases rather than penalising them, and a dropped case whose + true agreement is high pulls the reported mean **down**, not up. With three + or more runs the dropped set can sit anywhere relative to the kept set, so + the complete-case figure is simply a different population — reported + because it answers "how stable was the model where we actually asked it", + and labelled as that rather than as a bound in either direction. """ - set_id = require_one_set_id(set_ids) + set_id = require_one_provenance(set_ids, provenances) if len(run_labels) != runs: raise ValueError( f"{len(run_labels)} run label(s) for {runs} run(s)") - intersection_vectors = {c: v for c, v in vectors.items() if ABSENT not in v} + complete_vectors = {c: v for c, v in vectors.items() if ABSENT not in v} omissions = { label: sorted(c for c, v in vectors.items() if v[i] == ABSENT) for i, label in enumerate(run_labels) } - union = agreement(vectors, runs) - union["bound"] = "lower" - union["bound_note"] = ( - "includes every case seen in any run; a case the harness never asked a " - "run about counts as disagreement, so this understates stability") - inter = agreement(intersection_vectors, runs) - inter["bound"] = "upper" - inter["bound_note"] = ( - "only cases present in every run; excludes the harness's own gaps, so " - "this overstates stability") + penalized = agreement(vectors, runs) + penalized["bound"] = "lower" + penalized["bound_note"] = ( + "every case seen in any run; a case the harness never asked a run about " + "counts as disagreement. Filling in any real vote could only raise this, " + "so it is a genuine lower bound on stability") + complete = agreement(complete_vectors, runs) + complete["bound"] = None + complete["bound_note"] = ( + "only cases present in every run — a different population, not a bound. " + "The omitted cases could have agreed more or less than the kept ones, so " + "this can sit either side of the true figure") return { "set_id": set_id, "runs": runs, "run_labels": list(run_labels), - "union": union, - "intersection": inter, + "complete_case": complete, + "penalized": penalized, "omissions": omissions, "n_omitted": sum(len(v) for v in omissions.values()), } diff --git a/evals/align.py b/evals/align.py index ca0d375..9429dc2 100644 --- a/evals/align.py +++ b/evals/align.py @@ -41,6 +41,7 @@ import re import unicodedata +from collections import Counter from dataclasses import dataclass, field from difflib import SequenceMatcher @@ -124,6 +125,18 @@ def align(gold: dict, results, min_ratio: float = MATCH_MIN_RATIO, min_margin: float = MATCH_MIN_MARGIN) -> Alignment: cases = gold["cases"] by_case = {c["case_id"]: c for c in cases} + # a dict comprehension over claim ids silently keeps the LAST duplicate, so + # permuting the prediction list changed which one was graded — the one place + # this module's order-independence contract did not hold. Refused, not + # repaired: the harness cannot know which of two same-id claims was meant. + counts = Counter(c.id for c in results.claims) + if clashes := sorted(i for i, n in counts.items() if n > 1): + raise ValueError( + f"duplicate prediction id(s) in results.json: " + f"{', '.join(str(i) for i in clashes)}. Claim ids must be unique — " + f"alignment consumes each prediction once, and with a duplicate the " + f"input order would decide which one is graded." + ) pool = {c.id: c for c in results.claims} a = Alignment() diff --git a/evals/runners/score_only.py b/evals/runners/score_only.py index 916cde3..03683f6 100755 --- a/evals/runners/score_only.py +++ b/evals/runners/score_only.py @@ -73,25 +73,34 @@ def score_one(gold_path: Path, results_path: Path, out_root: Path, def score_agreement(run_dirs: list[Path], out_root: Path) -> Path: """Compare repeated runs of the SAME gold set. - The version this replaces filtered to the intersection with a bare + The version this replaces filtered to the complete-case set with a bare `if len(v) == n`, silently dropping every case one run never produced — defeating `agreement.py`'s own documented contract, in the direction that - flatters the model. Both bounds are now reported and the omissions named. + flatters the model. Both populations are now reported and the omissions + named, with only the penalized one called a bound. """ - labels, set_ids, per_run = [], [], [] + labels, set_ids, provenances, per_run = [], [], [], [] for d in run_dirs: record = json.loads((Path(d) / "eval.json").read_text()) labels.append(Path(d).name) set_ids.append((record.get("gold") or {}).get("set_id")) + provenances.append(record.get("provenance") or {}) + # `per_case` carries excluded rows on purpose — they are rendered in + # their own section. They must not therefore vote here: a case that was + # never scoreable cannot be evidence of the model disagreeing with + # itself, and an unresolved gold label is the harness's gap, not the + # model's instability. `.get("eligible", True)` so a record written + # before the flag existed still counts every row, as it used to. per_run.append({row["case_id"]: row.get("predicted") or UNMATCHED - for row in record["per_case"]}) + for row in record["per_case"] + if row.get("eligible", True)}) n = len(run_dirs) all_cases = sorted({c for run in per_run for c in run}) # a case missing from a run is ABSENT — the harness never asked — which is # a different fact from UNMATCHED, where it asked and the aligner failed vectors = {c: [run.get(c, ABSENT) for run in per_run] for c in all_cases} - result = agreement_report(vectors, n, labels, set_ids) + result = agreement_report(vectors, n, labels, set_ids, provenances) out = out_root / f"agg__{_stamp()}" out.mkdir(parents=True, exist_ok=True) @@ -105,26 +114,29 @@ def _fmt(value: float | None, spec: str) -> str: def _agreement_md(r: dict) -> str: + names = {"complete_case": "complete-case", "penalized": "penalized"} lines = [ f"# Repeated-run agreement — {r['set_id'] or 'unknown set'}", "", f"{r['runs']} runs: {', '.join(f'`{x}`' for x in r['run_labels'])}.", "", - "Two bounds, because neither is the answer alone. The intersection", - "drops cases the harness never asked some run about; the union charges", - "those gaps to the model.", "", + "Two populations, because neither answers the question alone. The", + "complete-case figure covers only the cases every run answered; the", + "penalized figure covers every case seen in any run and scores the gaps", + "as disagreement. Only the penalized figure is a bound.", "", "| | Cases | Modal agreement | Unanimous | Fleiss' kappa |", "|---|---|---|---|---|", ] - for key in ("intersection", "union"): + for key, label in names.items(): b = r[key] + qualifier = f" ({b['bound']} bound)" if b["bound"] else " (not a bound)" lines.append( - f"| **{key} ({b['bound']} bound)** | {b['cases']} | " + f"| **{label}{qualifier}** | {b['cases']} | " f"{_fmt(b['modal_agreement'], '.2f')} | " f"{_fmt(b['unanimous_rate'], '.0%')} | " f"{_fmt(b['fleiss'].get('value'), '.2f')}" f" ({b['fleiss'].get('reason', b['fleiss'].get('note', ''))}) |" ) - lines += ["", f"- *intersection* — {r['intersection']['bound_note']}", - f"- *union* — {r['union']['bound_note']}", ""] + lines += ["", f"- *complete-case* — {r['complete_case']['bound_note']}", + f"- *penalized* — {r['penalized']['bound_note']}", ""] if r["n_omitted"]: lines += ["## Cases the harness never asked about", "", "Not model disagreement — an operator gap, named so it is not", diff --git a/evals/scoring.py b/evals/scoring.py index 2c36895..0107f22 100644 --- a/evals/scoring.py +++ b/evals/scoring.py @@ -23,13 +23,19 @@ def score(gold: dict, results, gold_path: Path | None = None, if refs_drift is None: refs_drift = (provenance or {}).get("refs_status_drift") - alignment = align_mod.align(gold, results) - pairs = align_mod.matched_pairs(gold, results, alignment) - matched = {g["case_id"]: p for g, p in pairs} - - # eligibility is decided once, for both reasons, before any metric runs + # eligibility is decided once, for both reasons, BEFORE alignment — not + # merely before the metrics. A case that can never be scored used to + # compete for predictions anyway, and a prediction is consumed once: an + # unresolved gold case sitting on the same citation label as an eligible + # one took its match, and the eligible case was then reported as the tool's + # extraction gap. Blame moved off the harness and onto the tool, silently. scoreable, excluded = elig_mod.eligibility(gold, refs_drift) eligible_ids = {c["case_id"] for c in scoreable} + + eligible_gold = {**gold, "cases": scoreable} + alignment = align_mod.align(eligible_gold, results) + pairs = align_mod.matched_pairs(eligible_gold, results, alignment) + matched = {g["case_id"]: p for g, p in pairs} unresolved = [e for e in excluded if e.reason == elig_mod.GOLD_VERDICT_UNRESOLVED] drifted = [e for e in excluded if e.reason == elig_mod.REFS_STATUS_DRIFT] unverified = elig_mod.not_verified(refs_drift) diff --git a/evals/templates/eval.md.j2 b/evals/templates/eval.md.j2 index c403a98..cada7ad 100644 --- a/evals/templates/eval.md.j2 +++ b/evals/templates/eval.md.j2 @@ -102,10 +102,13 @@ asserted: ### Confusion (gold ↓ / predicted →) -| | supported | partial | contradicted | -|---|---|---|---| +{# every judgement class gets a column. `not_addressed` had a row and no + column, so a run that answered it against a `supported` gold had the + mistake counted in the arithmetic and then hidden in the picture. #} +| gold \ predicted | supported | partial | contradicted | not_addressed | +|---|---|---|---|---| {% for g, row in r.confusion.items() %} -| **{{ g }}** | {{ row.supported }} | {{ row.partial }} | {{ row.contradicted }} | +| **{{ g }}** | {{ row.supported }} | {{ row.partial }} | {{ row.contradicted }} | {{ row.not_addressed }} | {% endfor %} ## Retrieval, evidence and coverage diff --git a/evals/tests/test_agreement.py b/evals/tests/test_agreement.py index 75c42a1..604f017 100644 --- a/evals/tests/test_agreement.py +++ b/evals/tests/test_agreement.py @@ -10,7 +10,7 @@ agreement, agreement_report, fleiss_kappa, - require_one_set_id, + require_one_provenance, ) @@ -102,18 +102,22 @@ def test_fleiss_still_runs_when_nothing_is_absent(): assert "reason" in a["fleiss"] or a["fleiss"]["value"] is not None -# --- both bounds, and the omissions by name --------------------------------- +# --- two populations, and the omissions by name ----------------------------- -def test_intersection_is_the_upper_bound_and_union_the_lower(): +def test_the_penalized_figure_is_a_lower_bound_and_complete_case_is_not_a_bound(): + """Renamed from intersection/union. Only the penalized figure is a bound: + filling in an ABSENT vote can only raise the modal count. Dropping a case + can move the mean either way, so complete-case is a population, not a + ceiling — see `agreement_report`.""" vectors = {"c1": ["supported", "supported"], "c2": ["partial", ABSENT]} r = agreement_report(vectors, runs=2, run_labels=["runA", "runB"], set_ids=["demo-v1", "demo-v1"]) - assert r["intersection"]["bound"] == "upper" - assert r["union"]["bound"] == "lower" - assert r["intersection"]["cases"] == 1 - assert r["union"]["cases"] == 2 - assert r["union"]["modal_agreement"] <= r["intersection"]["modal_agreement"] + assert r["penalized"]["bound"] == "lower" + assert r["complete_case"]["bound"] is None + assert r["complete_case"]["cases"] == 1 + assert r["penalized"]["cases"] == 2 + assert r["penalized"]["modal_agreement"] <= r["complete_case"]["modal_agreement"] def test_per_run_omissions_are_named(): @@ -128,9 +132,9 @@ def test_per_run_omissions_are_named(): def test_two_different_set_ids_are_refused(): """A category error, not a partial comparison.""" with pytest.raises(ValueError) as e: - require_one_set_id(["demo-v1", "other-v2"]) + require_one_provenance(["demo-v1", "other-v2"]) assert "demo-v1" in str(e.value) and "other-v2" in str(e.value) - assert require_one_set_id(["demo-v1", "demo-v1"]) == "demo-v1" + assert require_one_provenance(["demo-v1", "demo-v1"]) == "demo-v1" with pytest.raises(ValueError): agreement_report({"c1": ["supported", "supported"]}, runs=2, @@ -154,8 +158,8 @@ def _run(name, rows): out = score_only.score_agreement([a, b], tmp_path / "out") result = json.loads((out / "agreement.json").read_text()) - assert result["union"]["cases"] == 2 - assert result["intersection"]["cases"] == 1 + assert result["penalized"]["cases"] == 2 + assert result["complete_case"]["cases"] == 1 assert result["omissions"]["b"] == ["c2"] md = (out / "AGREEMENT.md").read_text() - assert "c2" in md and "upper" in md and "lower" in md + assert "c2" in md and "not a bound" in md and "lower bound" in md diff --git a/evals/tests/test_eval_report.py b/evals/tests/test_eval_report.py index 4f464ea..7ad9859 100644 --- a/evals/tests/test_eval_report.py +++ b/evals/tests/test_eval_report.py @@ -64,10 +64,20 @@ def test_rendered_report_carries_the_denial(record): def test_every_percentage_carries_its_denominator(record): md = render(record) + # the fuzzy-alignment caveats are the one exemption: they are a property of + # the matcher, not a rate over a population. The window looks BOTH ways — + # "33% of matches were fuzzy" puts the word after the number, and a + # lookbehind-only guard missed it the moment the fraction crossed its + # threshold. + # the fuzzy-alignment caveats are the one exemption: they describe the + # matcher, not a rate over a population. The window looks BOTH ways — + # "33% of matches were fuzzy" puts the word after the number, and the + # lookbehind-only guard missed it as soon as the fraction crossed its + # reporting threshold. bare = [ m.group(0) for m in re.finditer(r"\d+%(?! \(\d+/\d+\))", md) - if "fuzzy" not in md[max(0, m.start() - 120):m.start()] + if "fuzzy" not in md[max(0, m.start() - 120):m.end() + 120] ] assert not bare, f"percentages without (k/n): {bare}" @@ -238,6 +248,6 @@ def test_no_percentage_from_the_new_sections_is_bare(gold_mini, results_mini): bare = [ m.group(0) for m in re.finditer(r"\d+%(?! \(\d+/\d+\))", md) - if "fuzzy" not in md[max(0, m.start() - 120):m.start()] + if "fuzzy" not in md[max(0, m.start() - 120):m.end() + 120] ] assert not bare, f"percentages without (k/n): {bare}" diff --git a/evals/tests/test_v041_eval_corrections.py b/evals/tests/test_v041_eval_corrections.py new file mode 100644 index 0000000..85432cc --- /dev/null +++ b/evals/tests/test_v041_eval_corrections.py @@ -0,0 +1,220 @@ +"""Six evaluation defects that let an ineligible or incomparable case count. + +Grouped in one file because they share a theme: the harness was measuring a +population it had not established. A case excluded from scoring still competed +for predictions and still voted in agreement; two runs of different prompts +were averaged together; a duplicate prediction id let input order pick a +winner; and the two agreement figures were labelled as bounds when only one of +them is one. +""" + +import copy +import json + +import pytest + +from evals import scoring +from evals.agreement import ABSENT, agreement_report, require_one_provenance +from evals.align import align + +# --- 1. eligibility is decided before alignment, not after ------------------ + + +def _gold_with_an_ineligible_rival(gold_mini: dict) -> dict: + """An unresolved case that shadows an eligible one on the same label. + + Both cases sit on label [1] and read almost alike, so they compete for the + same prediction. The unresolved one carries no gold verdict, so it can + never be scored — but it used to consume the prediction anyway, and the + eligible case was then reported as the tool's extraction failure. + """ + gold = copy.deepcopy(gold_mini) + eligible = next(c for c in gold["cases"] if c["case_id"] == "m-c01") + rival = copy.deepcopy(eligible) + rival.update({ + "case_id": "m-c00-unresolved", + "claim_text": eligible["claim_text"] + " overall", + "gold_verdict": None, + "ambiguity": "labellers split on whether this is one claim or two", + }) + rival.pop("pair", None) + gold["cases"].insert(0, rival) + return gold + + +def test_an_ineligible_case_cannot_consume_an_eligible_cases_prediction( + gold_mini, results_mini +): + gold = _gold_with_an_ineligible_rival(gold_mini) + rec = scoring.score(gold, results_mini) + + rows = {r["case_id"]: r for r in rec["per_case"]} + assert rows["m-c00-unresolved"]["eligible"] is False + assert rows["m-c01"]["predicted"] is not None, ( + "an unscoreable case took the prediction the eligible case needed" + ) + + +def test_the_ineligible_case_still_gets_a_row_and_a_reason(gold_mini, results_mini): + """Excluding it from alignment must not delete it from the record.""" + rec = scoring.score(_gold_with_an_ineligible_rival(gold_mini), results_mini) + row = next(r for r in rec["per_case"] if r["case_id"] == "m-c00-unresolved") + assert row["excluded_reason"] == "gold_verdict_unresolved" + assert "labellers" in row["excluded_detail"] + + +# --- 2. duplicate prediction ids --------------------------------------------- + + +def test_duplicate_prediction_ids_are_refused_not_silently_collapsed( + gold_mini, results_mini +): + """`{c.id: c for c in results.claims}` kept whichever came last, so input + order decided which of two same-id claims was graded. Alignment is + documented as order-independent; this was the one place it was not.""" + doubled = copy.deepcopy(results_mini) + clash = copy.deepcopy(doubled.claims[1]) + clash.id = doubled.claims[0].id + doubled.claims.append(clash) + + with pytest.raises(ValueError, match="duplicate prediction id"): + align(gold_mini, doubled) + + +def test_the_duplicate_error_names_the_offending_ids(gold_mini, results_mini): + doubled = copy.deepcopy(results_mini) + clash = copy.deepcopy(doubled.claims[1]) + clash.id = doubled.claims[0].id + doubled.claims.append(clash) + + with pytest.raises(ValueError) as exc: + align(gold_mini, doubled) + assert str(doubled.claims[0].id) in str(exc.value) + + +# --- 3. agreement compares like with like ------------------------------------- + + +def _prov(prompt="sha256:aaaa", converter="pymupdf"): + return {"prompt_fingerprint": {"scheme": "sha256-content", + "EXTRACT_PROMPT": prompt, "CHECK_PROMPT": prompt}, + "converter": converter} + + +def test_one_provenance_triple_accepts_matching_runs(): + assert require_one_provenance(["demo-v1", "demo-v1"], [_prov(), _prov()]) == "demo-v1" + + +def test_a_different_prompt_fingerprint_is_refused(): + """Two runs of different prompts describe different systems. Averaging + their agreement produces a number about neither.""" + with pytest.raises(ValueError, match="prompt"): + require_one_provenance(["demo-v1", "demo-v1"], + [_prov(), _prov(prompt="sha256:bbbb")]) + + +def test_a_different_converter_is_refused(): + """The judge read different text, so a disagreement is not the model's.""" + with pytest.raises(ValueError, match="converter"): + require_one_provenance(["demo-v1", "demo-v1"], + [_prov(), _prov(converter="docling 2.118.1")]) + + +def test_a_different_set_id_is_still_refused(): + with pytest.raises(ValueError, match="gold sets"): + require_one_provenance(["demo-v1", "other-v2"], [_prov(), _prov()]) + + +# --- 4. the two agreement figures are named for what they are ----------------- + + +def test_the_two_figures_are_complete_case_and_penalized_not_bounds(): + """`intersection` was labelled the upper bound. It is not one: dropping a + case whose true agreement is high pulls the mean DOWN, so the complete-case + figure can sit below the true value as easily as above it. Only the + penalized figure is a genuine bound, and only downward.""" + vectors = {"c1": ["supported", "supported", "supported"], + "c2": ["partial", "partial", ABSENT]} + r = agreement_report(vectors, runs=3, run_labels=["a", "b", "c"], + set_ids=["s", "s", "s"], provenances=[_prov()] * 3) + + assert set(r) >= {"complete_case", "penalized"} + assert "intersection" not in r and "union" not in r + assert r["penalized"]["bound"] == "lower" + assert r["complete_case"]["bound"] is None, ( + "the complete-case figure is not a bound in either direction" + ) + assert "not a bound" in r["complete_case"]["bound_note"] + + +def test_three_runs_with_a_missing_case_keep_both_populations_visible(): + vectors = {"c1": ["supported", "supported", "supported"], + "c2": ["partial", "partial", ABSENT]} + r = agreement_report(vectors, runs=3, run_labels=["a", "b", "c"], + set_ids=["s", "s", "s"], provenances=[_prov()] * 3) + + assert r["complete_case"]["cases"] == 1 + assert r["penalized"]["cases"] == 2 + assert r["omissions"]["c"] == ["c2"] + assert r["n_omitted"] == 1 + + +# --- 5. not_addressed is a rendered column, not just a row -------------------- + + +def test_the_confusion_matrix_renders_the_not_addressed_column(gold_mini, results_mini): + """The matrix has always computed four classes. The template printed three, + so a run that answered `not_addressed` when the gold said `supported` had + the mistake counted and then hidden.""" + from evals.eval_report import render + + rec = scoring.score(gold_mini, results_mini) + md = render(rec) + header = next(line for line in md.splitlines() if line.startswith("| gold \\")) + assert "not_addressed" in header, header + + +def test_a_not_addressed_prediction_against_not_addressed_gold_is_a_hit( + gold_mini, results_mini +): + """The diagonal cell for the fourth class must be reachable at all.""" + from evals import metrics + + gold = copy.deepcopy(gold_mini) + case = next(c for c in gold["cases"] if c["case_id"] == "m-c01") + case["gold_verdict"] = "not_addressed" + preds = copy.deepcopy(results_mini) + pred = preds.claims[0] + pred.verdict = "not_addressed" + + m = metrics.confusion([(case, pred)]) + assert m["not_addressed"]["not_addressed"] == 1 + + +# --- 6. an ineligible case does not vote in agreement ------------------------- + + +def test_score_agreement_ignores_cases_that_were_never_scoreable(tmp_path): + """An excluded case is in `per_case` by design — it must not therefore be + counted as the model disagreeing with itself.""" + from evals.runners.score_only import score_agreement + + def _run(name: str, predicted: str) -> None: + d = tmp_path / name + d.mkdir() + (d / "eval.json").write_text(json.dumps({ + "gold": {"set_id": "s"}, + "provenance": _prov(), + "per_case": [ + {"case_id": "ok", "eligible": True, "predicted": "supported"}, + {"case_id": "dropped", "eligible": False, "predicted": predicted}, + ], + })) + + _run("runA", "supported") + _run("runB", "contradicted") + + out = score_agreement([tmp_path / "runA", tmp_path / "runB"], tmp_path / "out") + result = json.loads((out / "agreement.json").read_text()) + assert result["penalized"]["cases"] == 1 + assert "dropped" not in result["penalized"]["per_case"] From 26d6bd4718b64425bfb337ea8a1601e689f32229 Mon Sep 17 00:00:00 2001 From: defraction <796174+defraction0@users.noreply.github.com> Date: Fri, 4 Sep 2026 09:52:26 +0200 Subject: [PATCH 05/54] docs: 0.4.1, and six README claims the code does not make MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The README is a correctness surface, so these are defects like any other. Page-level provenance was claimed "for every verdict" — false for not_addressed, which has no decisive passage by design, and understated for the rest, which now carry a block as well as a page. An unmatched anchor was said to be "shown unboxed", which only holds when a valid block supplied the crop region. not_addressed was folded into "the most adverse verdict any of them gave"; it is deliberately unranked and becomes the headline only when no source addressed the claim at all. Output was still documented under case/, which stopped being the default when audits moved beside the paper. Text drawn inside a figure was said to be in the PDF's text layer, which a raster figure is not — contradicted four paragraphs later by this section's own measurement. And both Quick Starts ran pip install -e . with no clone, which cannot work while PaperTrace is not on PyPI. CLAUDE.md repeats the headline rule, so it gains the same correction plus the new provenance requirement. The changelog opens a [0.4.1] section rather than folding into [0.4.0]: that one is released history now. It leads with the behaviour change, because runs that previously reported a verdict on page-only provenance will report a gap. --- CHANGELOG.md | 95 ++++++++++++++++++++++++++++++++++++++ CLAUDE.md | 5 +- README.md | 40 ++++++++++++---- src/papertrace/__init__.py | 2 +- 4 files changed, 131 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3155842..2da6d3e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,101 @@ All notable changes to PaperTrace are documented here. The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); versions follow [SemVer](https://semver.org/). +## [0.4.1] — unreleased + +### Changed + +- **A substantive verdict must now name a page and a block the source actually + has, and must be showable.** `check` validates every `supported`, `partial` + and `contradicted` judgement against the cited source's own + `source_map.json`: the page must exist, `source_block` is now **required**, + and it must sit on the page the verdict names. Anything else is + `unchecked` with a note, never a verdict. `highlight` enforces the same rule + against reality — a substantive judgement that produced no evidence image is + downgraded there too, because the PDF can be missing from + `sources_resolved/` and a source map can disagree with the PDF it came from. + + The block requirement is what makes the picture unconditional: the crop + region is the block's bbox, so a valid block always yields an image and the + anchor phrases only decide whether a red box is drawn on it. `CHECK_PROMPT` + already asked for `source_block` and already told the model to omit it only + for `not_addressed`, so no prompt text changed and eval runs stay comparable + across this release. + + **This changes counts.** A run that previously reported a verdict resting on + page-only provenance, an impossible page or a nonexistent block now reports a + gap. `not_addressed` is unaffected — it never claimed a passage. + +- **A source with no `source_map.json` can no longer produce a verdict.** Its + judgements are `unchecked`, with a note naming the re-ingest that fixes it. + Previously the location it named could not be checked against anything. + +### Fixed + +- **Two ways around the one-case-one-paper guard.** `papertrace ingest` never + consulted `_guard_case`, so a different paper could overwrite + `/ingest/manuscript` — the slot `refs` fills and the coverage audit + reads — while the manifest still described the first paper. The guard now + runs whenever the output *is* that slot, recognised by shape so `--out` + cannot walk in behind `-c`'s back; a cited source ingested into + `/ingest/` is untouched. And `refs --parse-only` on a pre-hash + case re-ingested the manuscript slot and then returned before writing the + manifest; an inspection command now reads the paper into a temporary + directory and mutates nothing. + +- **Claims whose headline is `not_retrieved` or `unchecked` now show their full + per-source state.** The gap sections printed the claim text alone, so a claim + citing [1,2] where source 1's check failed and source 2 was never obtainable + said neither thing, and a `not_addressed` from a source that *was* read + vanished behind the `unchecked` headline that outranks it. All three formats + now render the co-citation breakdown, the unretrieved co-citations and one + row per judgement with its note. The editor look also labelled a whole + section row with `items[0].verdict`, calling a mixed section whichever + verdict came first; it is one row per claim now. + +- **The anchor tri-state is no longer flattened.** `anchor_located` is `True` + (searched and located), `False` (searched, not located) or `None` (never + searched) — three facts. The disclosure was gated on `evidence_image`, so a + verdict with a page and no crop disclosed nothing; it is gated on provenance + now, with wording that does not describe a picture that was not written. The + `highlight` console branched on truthiness and described `None` as "no anchor + phrase found on the page", asserting a search that never happened. + +### Evaluation harness + +Developer tooling; none of this affects an ordinary audit. + +- Gold-case eligibility is decided **before** alignment, not after. An + unresolved or drift-invalidated case used to compete for predictions and + consume the one an eligible case needed — which then reported as the tool's + extraction gap, moving blame off the tool silently. +- Cases that were never eligible no longer vote in repeated-run agreement. +- Duplicate prediction ids are refused with an error naming them, instead of a + dict comprehension keeping whichever came last — the one place alignment's + documented order-independence did not hold. +- Repeated-run agreement enforces the whole **(`set_id`, prompt fingerprint, + ingest converter)** triple. The error message already claimed the triple + while only `set_id` was checked. +- The two agreement figures are renamed for what they are: **penalized** + (a genuine lower bound) and **complete-case** (a different population, not a + bound in either direction). `intersection` was labelled the upper bound, + which is false — dropping a case whose true agreement is high pulls the mean + down. +- `not_addressed` is a rendered confusion-matrix **column**, not only a row. + The arithmetic always had four classes; the table printed three, so a + mistake was counted and then hidden. +- `evals/DESIGN.md` describes all four judgement classes. + +### Documentation + +`README.md` corrections, each a statement that did not match the code: page +provenance is not universal (`not_addressed` has none by design) and is now +page *and* block; an unboxed crop needs a valid block to exist at all; +`not_addressed` is deliberately unranked in the headline rule; the default case +folder is the paper's stem, not `case/`; text drawn inside a raster figure has +no text layer to box; and both Quick Starts need `git clone` because PaperTrace +is not on PyPI. + ## [0.4.0] — 2026-08-30 (beta) ### Added diff --git a/CLAUDE.md b/CLAUDE.md index fb43ec5..92b2043 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -208,7 +208,10 @@ honest scope — when behaviour changes, that list changes with it. Specific current constraints documented there, worth not re-breaking: the coverage audit reads bracketed numeric labels only; batch mode judges a co-cited claim against every retrievable source and reports the most adverse verdict as the claim's -headline; the model reads extracted text with page +headline — where `not_addressed` is deliberately unranked and becomes the +headline only when no source addressed the claim at all; a substantive verdict +must name a page and a block that exist in the source's own map, so a verdict +nobody can be shown is `unchecked`; the model reads extracted text with page markers, not page images. Update `CHANGELOG.md` for any user-visible change, and `README.md` when flags, diff --git a/README.md b/README.md index bec6b79..c35cc5f 100644 --- a/README.md +++ b/README.md @@ -59,14 +59,22 @@ went uncited?** or unreadable** (scanned, image-only) **passes** — unverifiable is not the same as wrong, so a scanned source is checked rather than silently discarded. - Attempt to extract **every** citation-backed claim, then judge each against - the text of its cited source, with page-level provenance for every verdict. + the text of its cited source. Every `supported`, `partial` or `contradicted` + verdict carries a page **and** the source block it rests on, both checked + against that source's own ingest — a verdict naming a page or block the + source does not have is reported `⚠ not checked`, not published. `◌ does not + address the claim` carries no page by design: the source was read and says + nothing, so there is no passage to point at. Extraction is a model step, so it is an attempt, not a guarantee — which is why the coverage audit below exists. - Show the evidence: real page crops with the matched text boxed in red. Claude proposes the page, the block and verbatim anchor phrases; Python then finds those phrases in the PDF and draws the boxes — placed by text search, - never by hand, and never by the model. A crop whose anchor matched nothing - is shown unboxed and labelled as such. + never by hand, and never by the model. The crop region comes from the source + block the verdict names, so a crop whose anchor phrase matched nothing is + still shown — unboxed, and captioned as unboxed. Where no anchor phrase was + offered at all, the caption says that instead: "searched and not found" and + "never searched for" are different facts and are never merged. - Preserve unavailable sources as explicit gaps: a claim whose source couldn't be retrieved is `⊘ not retrieved` — recorded, never guessed. - Report every citation **occurrence** — each bracketed marker at its own place @@ -87,7 +95,11 @@ went uncited?** verdict any of them gave, so one dissenting source is never averaged away. A source that turns out to say nothing about the claim is `◌ does not address the claim` — an inapt citation, distinct from a contradiction and from a - retrieval gap. + retrieval gap. It is deliberately **not ranked** among the three: while any + source actually spoke to the claim, that source decides the headline, and + `◌` becomes the headline only when no available source addressed the claim + at all. The per-source breakdown beside the headline is where an inapt + citation stays visible. - Disclose its ingest fidelity: every report — markdown, editor and terminal — names the converter that read the **audited paper**, and a flat-text fallback says so loudly. Cited sources are ingested separately (see *Tables and @@ -138,10 +150,14 @@ went uncited?** ### Guided — `papertrace`, and answer the questions ```bash +git clone https://github.com/defraction0/PaperTrace && cd PaperTrace pip install -e ".[full]" # standard install — layout-aware ingest papertrace # asks for the paper, the DOI and your email ``` +*(PaperTrace is not on PyPI yet, so the clone is not optional — `pip install -e .` +installs the checkout you are standing in.)* + Nothing to memorise. It checks your setup first — so a missing `claude` CLI is a sentence before you type anything, not a traceback twenty minutes in — then asks one question at a time: the paper (drag the file in; quotes and escaped @@ -180,6 +196,7 @@ and batching its questions. ### Batch — one command, scriptable ```bash +git clone https://github.com/defraction0/PaperTrace && cd PaperTrace pip install -e ".[full]" # standard install (see matrix below) export PAPERTRACE_EMAIL="you@example.org" # Unpaywall asks for a contact papertrace run paper.pdf --provided ./my_pdfs # case folder: ./paper/ beside the PDF @@ -221,11 +238,13 @@ title-checked like downloaded ones, but a mismatch is recorded in the manifest rather than refused — you named the file, so it is used and the doubt is disclosed. -Output in `case/out/`: `report.md` with inline evidence images, the same +Output in `/out/` — where `` defaults to a folder named after the +paper, beside the paper (`paper.pdf` → `./paper/`), and `-c` chooses another. +It holds `report.md` with inline evidence images, the same report as a dark **editor-window** page and as a **terminal-run** page (`report_editor.html`, `report_terminal.html`), plus machine-readable `results.json` and `scout.json`. The retrieval manifest is written one level -up, at `case/refs_manifest.json`. Want shareable PNG images of the report +up, at `/refs_manifest.json`. Want shareable PNG images of the report looks? Add `--png` (one-time setup: `playwright install chromium`). **`--doi` is the DOI of the paper you are auditing** — not of anything it @@ -284,9 +303,12 @@ accusations. ## Tables and figures are evidence too -A number in a table cell, or drawn inside a figure, is still in the PDF's text -layer — so the red box lands on it whichever backend read the document. -`highlight` searches the real page, never the extracted text: +A number in a table cell is in the PDF's text layer, and so is text drawn +inside a figure **when the figure carries a text layer at all** — a vector +chart usually does, a scanned or raster-exported one does not, and nothing can +box text that is only pixels. Where the text is there, the red box lands on it +whichever backend read the document, because `highlight` searches the real +page, never the extracted text:

Two evidence crops: a table cell (N = 8382, 84.3%) and a number inside a flow-chart figure (97%), each boxed in red diff --git a/src/papertrace/__init__.py b/src/papertrace/__init__.py index e4cdfd1..3c5559f 100644 --- a/src/papertrace/__init__.py +++ b/src/papertrace/__init__.py @@ -1,3 +1,3 @@ """PaperTrace — trace a paper’s claims to their sources, then scout what came after.""" -__version__ = "0.4.0" +__version__ = "0.4.1" From 13b683f0f2a9958fa18b5269e56f762fc5278628 Mon Sep 17 00:00:00 2001 From: defraction <796174+defraction0@users.noreply.github.com> Date: Fri, 4 Sep 2026 15:14:53 +0200 Subject: [PATCH 06/54] refs: stop the tool inventing references MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first real audit reported 46 references on a paper citing 43. The three extras were the paper's own table captions, and by the time they reached the report they had real DOIs attached and were labelled paywalled — the manifest asserted three works that do not exist. references_span scanned to the end of the document for any run of blocks sharing the bibliography's block type, testing nothing about the text, so three list blocks under a TABLE TITLES heading joined the list. A candidate run must now be at least half reference-shaped. Half rather than all, because a genuine continuation can carry a bare-URL entry with no year — the case the resume feature exists for. The resolver is the second line, because the parser will be wrong again. Crossref answered a title search for "Table 1. Dataset characteristics" with 10.7717/peerj.7892/table-1, a table belonging to an unrelated paper, and nothing caught it: the title sanity check only runs on the download path and no copy was ever downloaded. Entries that read as nothing citable are refused before the search, mirroring the web-page gate directly above, and a DOI naming a table, figure or supplement is rejected wherever it came from. looks_like_reference accepts an author list as well as a year, DOI or arXiv id. That clause is load-bearing: re-running the failing paper showed two real references arriving truncated mid-title with no year, whose correct DOIs Crossref had been finding from the author string alone. A year-only test made them gaps — the silent failure, and the worse one. Accepted downloads also keep the evidence for their title check, which until now was recorded only when the check failed. --- src/papertrace/ingest/pymupdf_.py | 32 +++++-- src/papertrace/models.py | 44 +++++++++ src/papertrace/refs.py | 71 ++++++++++++++- tests/test_reference_list.py | 54 +++++++++++ tests/test_refs.py | 144 ++++++++++++++++++++++++++++++ 5 files changed, 336 insertions(+), 9 deletions(-) diff --git a/src/papertrace/ingest/pymupdf_.py b/src/papertrace/ingest/pymupdf_.py index 1c475ef..28412a7 100644 --- a/src/papertrace/ingest/pymupdf_.py +++ b/src/papertrace/ingest/pymupdf_.py @@ -16,7 +16,7 @@ except ImportError: # pragma: no cover - older PyMuPDF exposes only `fitz` import fitz -from ..models import Block, SourceMap, is_references_heading +from ..models import Block, SourceMap, is_references_heading, looks_like_reference _HEADING_MAX_LEN = 120 @@ -88,6 +88,17 @@ def ingest_blocks_pymupdf(pdf_path: Path) -> tuple[int, list[Block]]: _MIN_RESUME_RUN = 2 +def _mostly_references(run) -> bool: + """Does this run of same-typed blocks read as a bibliography? + + Half, not all. Requiring every entry would drop a real continuation over one + bare-URL entry; requiring one would let a single dated line drag a whole + section of back matter in behind it. + """ + hits = sum(1 for b in run if looks_like_reference(b.text)) + return hits * 2 >= len(run) + + def references_span(smap: SourceMap) -> tuple[str, bool]: """Reference-list text, and whether it was resumed across a section break. @@ -104,10 +115,19 @@ def references_span(smap: SourceMap) -> tuple[str, bool]: intervening section. That is worth surfacing rather than hiding: a real pre-proof put refs 1-9 on page 7, `Declaration of interests` next, then refs 10-15 on page 8, and stopping at the first header lost six sources without - saying so. Only reference-shaped runs are collected, so the prose of the - intervening section never enters the list — which matters because - `_parse_bulleted` appends a non-bullet line to the *previous* entry, so a - stray paragraph corrupts a reference rather than merely adding noise. + saying so. The prose of the intervening section never enters the list, which + matters because `_parse_bulleted` appends a non-bullet line to the *previous* + entry, so a stray paragraph corrupts a reference rather than merely adding + noise. + + A run has to look like references, not merely share their block type. This + docstring used to claim that and it was false — the only test was the type, + so three `list` blocks under a `TABLE TITLES` heading became references + 44-46 of a 43-reference paper, and the resolver title-searched the paper's + own table captions into table-component DOIs belonging to other papers. + The test is applied to the run rather than to each entry: a genuine + continuation can hold a bare URL entry with no year, and rejecting the whole + run over it would undo the fix above. """ blocks = smap.blocks start = next( @@ -143,7 +163,7 @@ def references_span(smap: SourceMap) -> tuple[str, bool]: j = k while j < len(rest) and rest[j].type == entry_type: j += 1 - if j - k >= _MIN_RESUME_RUN: + if j - k >= _MIN_RESUME_RUN and _mostly_references(rest[k:j]): out.extend(b.text for b in rest[k:j]) resumed = True k = j diff --git a/src/papertrace/models.py b/src/papertrace/models.py index 6ca7d52..a17394a 100644 --- a/src/papertrace/models.py +++ b/src/papertrace/models.py @@ -46,6 +46,50 @@ def is_references_heading(block_type: str, text: str) -> bool: return bool(_REFS_HEADING_EXACT.match(text)) +# What marks a line as a bibliographic reference rather than back matter. +# Deliberately three cheap structural marks and nothing else — the question is +# only "is this a citable work at all", not "is this a good reference". +_REF_YEAR = re.compile(r"\b(?:19|20)\d{2}\b") +_REF_DOI = re.compile(r"10\.\d{4,9}/\S", re.I) +_REF_ARXIV = re.compile(r"arxiv[:\s]*\d{4}\.\d{4,5}", re.I) +# An author list, in the two styles that actually turn up: `M.A. Slabaugh` and +# `Slabaugh MA`. Two names, not one — a single match is easy to hit by accident. +_REF_AUTHORS = re.compile( + r"\b[A-Z]\.(?:\s*[A-Z]\.)*\s*[A-Z][a-z]+" # M.A. Slabaugh + r"|\b[A-Z][a-z]+\s+[A-Z]{1,3}\b" # Slabaugh MA +) + + +def looks_like_reference(text: str) -> bool: + """Could this line be a cited work? A year, a DOI or an arXiv id. + + Lives here rather than in `refs.py` or `ingest/` for the same reason + `is_references_heading` does: two readers need the rule, they cannot import + each other, and each keeping its own copy is a defect this codebase has + already shipped once. + + The bar is deliberately low. This is not a quality test on a reference — it + is the difference between a cited work and the paper's own back matter. + `Table 1. Dataset characteristics` carries none of the three, and three of + those became references 44-46 of a 43-reference paper, were title-searched + against Crossref, and came back as table-component DOIs belonging to other + papers. + + Being wrong in the permissive direction is the cheap error: a stray line + that sneaks through is one bad entry in a manifest. Being wrong in the + strict direction drops a real reference from the audit entirely, and that + failure is silent. + """ + text = text or "" + if _REF_YEAR.search(text) or _REF_DOI.search(text) or _REF_ARXIV.search(text): + return True + # An author list, for the references that arrive truncated. Two real + # references in one audit reached the resolver as authors plus half a title + # and nothing else — no journal, no year — and Crossref found both correct + # DOIs from exactly that. A year-only test threw them away. + return len(_REF_AUTHORS.findall(text)) >= 2 + + @dataclass class Block: """One layout block of a source document, with page-level provenance. diff --git a/src/papertrace/refs.py b/src/papertrace/refs.py index fb02177..af179e9 100644 --- a/src/papertrace/refs.py +++ b/src/papertrace/refs.py @@ -15,7 +15,7 @@ import httpx from . import __version__ -from .models import RefEntry +from .models import RefEntry, looks_like_reference # Two user agents on purpose. The contact address is sent ONLY to the services # that ask for one — Unpaywall requires it, Crossref's polite pool uses it. One @@ -315,7 +315,11 @@ def _accept( ) return False entry.status, entry.resolver, entry.pdf_path = "retrieved", resolver, str(dest) - entry.reason = why + # Carry the check's own evidence. The mismatch branch above already states + # its detail; the accepting branch discarded it, so `title_check: verified` + # and `title_check: unverifiable` reached the manifest as bare assurances + # with nothing behind them — and those two mean very different things. + entry.reason = f"{why} · title check: {detail}" if detail else why return True @@ -387,6 +391,38 @@ def _match_provided(entry: RefEntry, provided_dir: Path | None) -> Path | None: ) +# A DOI naming a PART of a work: Crossref mints these for tables, figures and +# supplements, and a title search will happily return one. `/table-1` came back +# for the caption "Table 1. Dataset characteristics" and was reported as a +# paywalled cited work. +_COMPONENT_DOI_RE = re.compile( + r"/(?:table|figure|fig|scheme|supp(?:l|lement(?:al|ary)?)?)[-_.]?\d+/?$" + r"|\.s\d{3,}$", + re.I, +) + + +def _component_doi_reason(doi: str | None) -> str: + return ( + f"the only DOI available ({doi}) names a table, figure or supplement, not a " + "paper — a part of a work is never the work a reference cites. Recorded as " + "no DOI rather than resolved, because fetching it would judge claims against " + "someone else's table" + ) + + +def _is_component_doi(doi: str | None) -> bool: + """Does this DOI name a table, figure or supplement rather than a work? + + A part of a paper is never the thing a reference cites, so accepting one is + always wrong — whether it arrived from a Crossref title search or was + printed in the reference itself. Anchored at the end of the DOI so an + ordinary suffix that merely contains the word (`.../figures-in-radiology`) + is untouched. + """ + return bool(doi and _COMPONENT_DOI_RE.search(doi)) + + def _is_webpage_reference(raw: str) -> bool: """Is this reference a web page rather than an article? @@ -474,9 +510,38 @@ def resolve_entry( ) return entry + # The parser is fallible, so this is the second line of defence. An + # entry with no year, no DOI and no arXiv id is not a citable work, and + # a bibliographic search always answers with *something*: three of one + # paper's own table captions were searched by title and came back as + # table-component DOIs belonging to unrelated papers, then published as + # paywalled references. + if not entry.doi and not looks_like_reference(entry.raw): + entry.status = "no_doi" + entry.reason = ( + "this entry carries no year, DOI or arXiv id, so nothing here reads as " + "a cited work — it is more likely a caption or a heading the reference " + "parser swept in. Not searched by title: Crossref would answer with the " + "closest-looking record, and inventing a reference is worse than " + "reporting one the parser got wrong" + ) + return entry + + # a DOI printed in the reference can name a part of a paper too + if _is_component_doi(entry.doi): + entry.status, entry.reason = "no_doi", _component_doi_reason(entry.doi) + entry.doi = None + return entry + if not entry.doi: try: - entry.doi = _crossref_doi(client, entry.raw, email) + found = _crossref_doi(client, entry.raw, email) + if _is_component_doi(found): + # a title match is not a work match — a table's title is the + # table's, and this one belonged to a different paper + entry.status, entry.reason = "no_doi", _component_doi_reason(found) + return entry + entry.doi = found if entry.doi: entry.resolver = "crossref" except httpx.HTTPError: diff --git a/tests/test_reference_list.py b/tests/test_reference_list.py index d2ae453..78acbd7 100644 --- a/tests/test_reference_list.py +++ b/tests/test_reference_list.py @@ -129,3 +129,57 @@ def test_a_differently_typed_run_does_not_resume_the_list(): text, resumed = references_span(m) assert "supplementary consideration" not in text, text assert resumed is False + + +# --- back matter is not a continued bibliography ---------------------------- +# +# The first real audit of an Elsevier paper reported 46 references on a paper +# citing 43. Blocks 120-162 were the references; block 163 was a `TABLE TITLES` +# heading; blocks 164-166 were three `list` blocks holding the paper's own table +# captions. The resume scan runs to the END of the document and accepted them, +# because the only test it applied was the block *type*. Refs 44-46 were then +# title-searched against Crossref, which answered with table-component DOIs from +# unrelated papers, and the report published three works that do not exist. + +_TABLE_TITLES = _map( + ("sectionheader", 22, "REFERENCES"), + ("list", 22, "- G.C. Feuerriegel, R.P. Marcus, S. Sommer, Rotator cuff. Eur Radiol 2023."), + ("list", 22, "- D.A. Lansdown, S. Lee, C. Sam, A prospective quantitative study. 2017."), + ("list", 22, "- W.T. Dixon, Simple proton spectroscopic imaging, Radiology 153 (1984) 189-194."), + ("sectionheader", 26, "TABLE TITLES"), + ("list", 26, "- Table 1. Dataset characteristics"), + ("list", 26, "- Table 2. Accuracy and reliability of automated thresholding models"), + ("list", 26, "- Table 3. Diagnostic accuracy for clinical cutoffs of Goutallier"), +) + + +def test_table_captions_after_the_references_are_not_references(): + """Three list blocks under `TABLE TITLES` share the reference list's block + type and nothing else. None carries a year, a DOI or an arXiv id.""" + text, resumed = references_span(_TABLE_TITLES) + + assert "Table 1." not in text, "the paper's own table captions became references" + assert "Table 2." not in text + assert "Table 3." not in text + assert resumed is False, "nothing was resumed, so nothing should be reported as resumed" + assert text.count("- ") == 3, text + + +def test_the_real_references_survive_the_shape_test(): + """The other half of the same assertion: rejecting back matter must not + reject the bibliography it follows.""" + text, _ = references_span(_TABLE_TITLES) + for surname in ("Feuerriegel", "Lansdown", "Dixon"): + assert surname in text, f"{surname} was lost to the shape test" + + +def test_a_genuine_continuation_still_resumes_when_one_entry_lacks_a_year(): + """The shape test is applied to the RUN, not to each entry. `_SPLIT`'s + resumed run holds two dated references and one URL-only entry; requiring + every entry to be reference-shaped would undo the 0.4.0 fix over the one + entry that is a bare link.""" + text, resumed = references_span(_SPLIT) + + assert resumed is True + assert "Dean" in text and "Kitamura" in text + assert "Assess-AI" in text, "the year-less entry in a real run was dropped" diff --git a/tests/test_refs.py b/tests/test_refs.py index 8c1a60b..5a58e1d 100644 --- a/tests/test_refs.py +++ b/tests/test_refs.py @@ -790,3 +790,147 @@ def test_three_generic_domain_words_are_not_an_identity_check(): assert state == "unverifiable", detail # a page with none of the reference's words is still called wrong outright assert _title_check_text(ACR_WEBPAGE_REF, RIGHT_PAGE)[0] == "mismatch" + + +# --- the resolver must not mint bibliographic facts ------------------------- +# +# A first real audit reported 46 references on a paper citing 43. Three of the +# paper's own table captions reached the resolver, and Crossref answered a title +# search for "Table 1. Dataset characteristics" with 10.7717/peerj.7892/table-1 +# — a table-component DOI belonging to an unrelated paper. The report published +# all three as `paywalled`, i.e. as real works held behind a paywall. The title +# sanity check never fired, because it only runs on the download path and +# nothing was ever downloaded. + + +def test_a_non_reference_is_never_title_searched(tmp_path): + """The parser is fallible, so the resolver is the second line. An entry with + no year, no DOI and no arXiv id is not a citable work, and Crossref always + answers a title search with *something*.""" + asked: list[str] = [] + + def handler(request: httpx.Request) -> httpx.Response: + asked.append(str(request.url)) + return httpx.Response( + 200, json={"message": {"items": [{"DOI": "10.7717/peerj.7892/table-1"}]}} + ) + + e = RefEntry(num="44", raw="Table 1. Dataset characteristics") + resolve_entry(e, tmp_path, "t@example.org", httpx.Client(transport=httpx.MockTransport(handler))) + + assert asked == [], f"a table caption was sent to a bibliographic search: {asked}" + assert e.status == "no_doi" + assert e.doi is None + + +def test_the_refusal_says_why_rather_than_reading_as_a_lookup_failure(tmp_path): + """`no_doi` alone would read as "we looked and found nothing".""" + e = RefEntry(num="44", raw="Table 2. Accuracy and reliability of thresholding models") + resolve_entry(e, tmp_path, "t@example.org", _client({})) + assert "not searched by title" in e.reason.lower() + + +def test_a_component_doi_from_a_title_search_is_refused(tmp_path): + """Belt and braces: even a reference-shaped entry must not accept a DOI that + names a table, a figure or a supplement. Those are parts of a work, never a + work, whatever the title matched.""" + e = RefEntry(num="7", raw="Someone S. A real-looking reference. J Imaging 2020;5:1-9.") + client = _client({ + "api.crossref.org": httpx.Response( + 200, json={"message": {"items": [{"DOI": "10.7717/peerj-cs.847/table-10"}]}} + ), + }) + resolve_entry(e, tmp_path, "t@example.org", client) + + assert e.doi is None, f"accepted a component DOI: {e.doi}" + assert e.status == "no_doi" + assert "table" in e.reason.lower() or "component" in e.reason.lower() + + +def test_a_component_doi_printed_in_the_reference_is_also_refused(tmp_path): + """The same rule wherever the DOI came from — a supplement DOI printed in + the reference itself is still not the paper. + + Parsed through `parse_references` on purpose: building a RefEntry by hand + leaves `doi` unset, so the test would pass without exercising the guard. + """ + (e,) = parse_references( + "References\n1. Someone S. A paper. J Imaging 2020. doi:10.1234/abcd.2020.s001\n" + ) + assert e.doi == "10.1234/abcd.2020.s001", "fixture did not parse the DOI it is testing" + + resolve_entry(e, tmp_path, "t@example.org", _client({})) + assert e.status == "no_doi" + assert e.doi is None + + +def test_an_ordinary_reference_still_reaches_crossref(tmp_path): + """The guard must not gate real references — the failure that matters most + here is the strict one, because a dropped reference is silent.""" + e = RefEntry(num="1", raw="Fixture F, Example E (2023) A method. J Synth Methods 5:e230024") + client = _client({ + "api.crossref.org": httpx.Response( + 200, json={"message": {"items": [{"DOI": "10.1148/ryai.230024"}]}} + ), + "api.unpaywall.org": httpx.Response(200, json={}), + }) + resolve_entry(e, tmp_path, "t@example.org", client) + assert e.doi == "10.1148/ryai.230024" + assert e.status == "paywalled" + + +def test_an_accepted_download_records_what_its_title_check_rested_on(tmp_path): + """`title_check: verified` with no evidence beside it is a bare assurance. + + The mismatch branch always stated its detail; the accepting branch threw it + away, so a real audit's manifest showed six sources marked verified with + nothing a reader could weigh — and `unverifiable` looked the same. + """ + pdf = _real_pdf_bytes("Preoperative deltoid size and fatty infiltration of the deltoid") + (e,) = parse_references( + "References\n1. B.P. Wiater et al. Preoperative deltoid size and fatty " + "infiltration of the deltoid. Clin Orthop 2015. doi:10.1007/s11999-014-4047-2\n" + ) + client = _client({ + "api.unpaywall.org": httpx.Response( + 200, json={"best_oa_location": {"url_for_pdf": "https://x/oa.pdf"}} + ), + "https://x/oa.pdf": httpx.Response(200, content=pdf), + }) + resolve_entry(e, tmp_path, "t@example.org", client) + + assert e.status == "retrieved" and e.title_check == "verified" + assert "title check:" in e.reason, e.reason + assert "tokens on its first page" in e.reason, e.reason + + +def test_a_truncated_but_real_reference_is_still_searched(tmp_path): + """The strict-direction error, caught on real data before it shipped. + + Two references in one audit reached the resolver truncated mid-title — + authors plus half a title, no journal, no volume, no year — because the + converter cut them short. Crossref found both correct DOIs from exactly + that string. A shape test keyed on the year alone refused them, turning two + resolvable references into recorded gaps: the silent failure, and the one + that matters more than letting a stray caption through. + """ + asked: list[str] = [] + + def handler(request: httpx.Request) -> httpx.Response: + asked.append(str(request.url)) + if "api.crossref.org" in str(request.url): + return httpx.Response( + 200, json={"message": {"items": [{"DOI": "10.1177/0363546512452714"}]}} + ) + return httpx.Response(404) + + e = RefEntry( + num="14", + raw="M.A. Slabaugh, N.A. Friel, V. Karas, A.A. Romeo, N.N. Verma, B.J. Cole, " + "Interobserver and intraobserver reliability of the Goutallier Classification using", + ) + resolve_entry(e, tmp_path, "t@example.org", + httpx.Client(transport=httpx.MockTransport(handler))) + + assert any("crossref" in u for u in asked), "a real reference was never looked up" + assert e.doi == "10.1177/0363546512452714" From f0b840d71270d2eb3572c54529c1d29cdb0187c4 Mon Sep 17 00:00:00 2001 From: defraction <796174+defraction0@users.noreply.github.com> Date: Fri, 4 Sep 2026 15:15:04 +0200 Subject: [PATCH 07/54] scout: say which failure it was MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With --doi supplied and no Europe PMC record found, the scout answered "paper not identified in Europe PMC — pass --doi to pin it". The operator had just passed --doi, so they went and curled Europe PMC by hand to establish what the tool already knew: the paper is a Journal Pre-proof and is not indexed. A DOI lookup that returns nothing is a different and stronger fact than a title heuristic that missed, and it changes what the reader should conclude — both registers are empty for want of a starting point, which is absence of data rather than a clean literature search. The message now branches, and the DOI that was tried is recorded in scout.json, which had been writing an empty string and leaving the null uninterpretable from the artifact alone. --- src/papertrace/scout.py | 21 +++++++++++++---- tests/test_scout.py | 51 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 4 deletions(-) diff --git a/src/papertrace/scout.py b/src/papertrace/scout.py index 6276a5b..54e642c 100644 --- a/src/papertrace/scout.py +++ b/src/papertrace/scout.py @@ -189,10 +189,23 @@ def scout_case( with _client(email, transport) as client: paper = _resolve_paper(client, doi, _title_from_case(case)) if paper is None: - res.error = ( - "paper not identified in Europe PMC — pass --doi to pin it " - "(title heuristics can miss)" - ) + # Which failure this was decides what the reader should do, and + # the two are not the same fact. Telling an operator who just + # passed --doi to pass --doi sent them to verify by hand what + # the tool already knew. + if doi: + res.paper_doi = doi # so the artifact shows what was tried + res.error = ( + f"Europe PMC returned no record for DOI {doi}, so this paper is " + "not indexed there — usual for an in-press or pre-proof article. " + "Both registers below are empty for want of a starting point, " + "which is absence of data, not a clean literature search" + ) + else: + res.error = ( + "paper not identified in Europe PMC — pass --doi to pin it " + "(title heuristics can miss)" + ) return res res.paper_title = paper["title"] res.paper_doi = paper["doi"] diff --git a/tests/test_scout.py b/tests/test_scout.py index 135ed21..a6c9253 100644 --- a/tests/test_scout.py +++ b/tests/test_scout.py @@ -175,3 +175,54 @@ def test_email_fallback_old_env_var(monkeypatch): assert _email(None) == "old@example.org" monkeypatch.setenv("PAPERTRACE_EMAIL", "new@example.org") assert _email(None) == "new@example.org" # new name wins + + +# --- a failure has to say which failure it was ------------------------------ +# +# On a real audit the operator was told "paper not identified in Europe PMC — +# pass --doi to pin it", so they found the DOI and re-ran with it. Same message. +# They then curled Europe PMC by hand to establish what the tool already knew: +# the paper is a Journal Pre-proof and simply is not indexed. `scout.json` also +# recorded `"doi": ""`, so the artifact could not show what had been tried. + + +def _no_hits(request): + return httpx.Response(200, json={"resultList": {"result": []}}) + + +def test_a_pinned_doi_that_finds_nothing_does_not_ask_for_a_doi(tmp_path): + case = _case(tmp_path) + res = scout_case(case, doi="10.1016/j.ejrad.2026.113206", + transport=httpx.MockTransport(_no_hits)) + + assert "--doi" not in res.error, res.error + assert "10.1016/j.ejrad.2026.113206" in res.error, res.error + + +def test_a_pinned_doi_that_finds_nothing_says_the_paper_is_not_indexed(tmp_path): + """A DOI lookup returning nothing is a stronger fact than a failed title + heuristic, and a different one: absence of indexing, not absence of skill. + Zero candidates must not read as a clean literature search.""" + case = _case(tmp_path) + res = scout_case(case, doi="10.1016/j.ejrad.2026.113206", + transport=httpx.MockTransport(_no_hits)) + + assert "not indexed" in res.error.lower(), res.error + assert res.newer == [] and res.overlooked == [] + + +def test_the_doi_that_was_tried_survives_into_the_artifact(tmp_path): + """`scout.json` carried an empty doi, so the null was uninterpretable from + the file alone.""" + case = _case(tmp_path) + res = scout_case(case, doi="10.1016/j.ejrad.2026.113206", + transport=httpx.MockTransport(_no_hits)) + + assert res.paper_doi == "10.1016/j.ejrad.2026.113206" + + +def test_without_a_doi_the_advice_to_pin_one_still_stands(tmp_path): + """The original message is right when no DOI was given — keep it.""" + case = _case(tmp_path) + res = scout_case(case, transport=httpx.MockTransport(_no_hits)) + assert "--doi" in res.error From 55e49a34b239b8f461a6bcb0b2b46c562c1f253d Mon Sep 17 00:00:00 2001 From: defraction <796174+defraction0@users.noreply.github.com> Date: Fri, 4 Sep 2026 15:15:04 +0200 Subject: [PATCH 08/54] docs: changelog for the fabricated-reference fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Folded into the existing [0.4.1] section — that version is still unreleased, so these belong with it rather than opening another heading. --- CHANGELOG.md | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2da6d3e..7a2b118 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,49 @@ All notable changes to PaperTrace are documented here. The format follows ## [0.4.1] — unreleased +### Fixed — the tool could invent a reference + +Found by the first real audit: a 43-reference Elsevier paper was reported as +having 46, and the three extra "references" were the paper's own table +captions, published in the retrieval manifest as `paywalled` works with real +DOIs attached. + +- **A resumed reference list must look like references.** `references_span` + scanned to the end of the document for any run of blocks sharing the + bibliography's block *type*, with no test on the text — so three `list` + blocks under a `TABLE TITLES` heading became references 44–46. A candidate + run now has to be at least half reference-shaped. Half rather than all, + because a genuine continuation can carry a bare-URL entry with no year. The + docstring claimed this was already the case; it was not. + +- **A non-reference is never title-searched, and a component DOI is never + accepted.** Crossref answered a title search for "Table 1. Dataset + characteristics" with `10.7717/peerj.7892/table-1` — a *table* belonging to + an unrelated paper — and nothing caught it, because the title sanity check + only runs on the download path and no copy was ever downloaded. Entries that + read as nothing citable are refused before the search, mirroring the existing + web-page gate, and any DOI naming a table, figure or supplement is rejected + wherever it came from. + + `looks_like_reference` accepts a year, a DOI, an arXiv id **or an author + list**. The author clause is not decoration: two real references in the same + paper reached the resolver truncated mid-title with no year at all, and + Crossref found both correct DOIs from the author string. A year-only test + turned them into gaps. + +- **The retrieval manifest keeps the evidence for a title check that passed.** + `title_check: verified` and `title_check: unverifiable` both arrived as bare + assurances; the detail was recorded only on mismatch. Accepted downloads now + carry it too — `title check: 18/19 reference tokens on its first page`. + +- **The scout says which failure it was.** With `--doi` supplied and no record + found, it reported "paper not identified in Europe PMC — pass `--doi` to pin + it", advising the operator to do what they had just done, and wrote + `"doi": ""` into `scout.json` so the artifact could not show what was tried. + A DOI that returns nothing means the paper is not indexed — usual for an + in-press pre-proof, and a stronger fact than a failed title heuristic. Both + registers being empty is absence of data, not a clean literature search. + ### Changed - **A substantive verdict must now name a page and a block the source actually From d876382a81b37687926652712f37bbfd972a217d Mon Sep 17 00:00:00 2001 From: defraction <796174+defraction0@users.noreply.github.com> Date: Fri, 4 Sep 2026 15:45:57 +0200 Subject: [PATCH 09/54] scout: search for the subject, and stop blaming a paper for its own year MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A live audit of "Image registration improves inter-reader agreement of objective response in CT assessment of pancreas adenocarcinoma" searched Europe PMC for `image AND registration AND improves AND inter-reader`. Two faults in one line: `improves` is a verb carrying no topic, and taking the first four content words never reaches a subject that sits at the end of the title. The register came back with a stroke conference abstract matching on IMPROVES. Words are ranked by length now — one rule, a proxy for specificity — rather than by position, and words that state what a paper claims rather than what it is about join the stop list. The same title yields `adenocarcinoma AND registration AND inter-reader AND agreement`. All fifteen "existed but uncited" candidates were from the paper's own year. That register asks the reader what the authors missed, and a same-year paper may have appeared after submission, so it cannot answer that. It is not newer literature either. same_year is a third register, following the rule coverage already uses for `uncertain`: a third status is never folded into either neighbour. Dropping them instead would lose a real finding, since a paper published early in the same year is exactly what a reviewer might raise. Additive in the schema; an older scout.json still loads. Europe PMC escapes the markup in its titles, so `CTVboost` arrived as `CTV<sub>boost</sub>` and was rendered verbatim. Decoded where every hit is built. --- schemas/scout.schema.json | 99 +++++++++--- src/papertrace/cli.py | 9 ++ src/papertrace/models.py | 25 ++- src/papertrace/scout.py | 55 ++++++- src/papertrace/templates/report.md.j2 | 22 ++- .../templates/report_editor.html.j2 | 14 +- .../templates/report_terminal.html.j2 | 8 +- tests/test_scout.py | 142 +++++++++++++++++- 8 files changed, 332 insertions(+), 42 deletions(-) diff --git a/schemas/scout.schema.json b/schemas/scout.schema.json index cbea0db..2357005 100644 --- a/schemas/scout.schema.json +++ b/schemas/scout.schema.json @@ -3,43 +3,104 @@ "title": "scout", "description": "Post-publication literature scan around one paper. Search-based and incomplete by construction: absence from the registers proves nothing; a non-empty error means the scan soft-failed.", "type": "object", - "required": ["paper", "newer", "overlooked"], + "required": [ + "paper", + "newer", + "overlooked" + ], "properties": { "paper": { "type": "object", "properties": { - "title": { "type": "string" }, - "doi": { "type": "string" }, - "year": { "type": ["integer", "null"] }, - "resolved_via": { "type": "string", "enum": ["doi", "title", ""] } + "title": { + "type": "string" + }, + "doi": { + "type": "string" + }, + "year": { + "type": [ + "integer", + "null" + ] + }, + "resolved_via": { + "type": "string", + "enum": [ + "doi", + "title", + "" + ] + } } }, - "query": { "type": "string" }, - "date": { "type": "string" }, + "query": { + "type": "string" + }, + "date": { + "type": "string" + }, "counts": { "type": "object", "properties": { - "newer": { "type": "integer" }, - "overlooked": { "type": "integer" } + "newer": { + "type": "integer" + }, + "overlooked": { + "type": "integer" + }, + "same_year": { + "type": "integer" + } } }, - "newer": { "$ref": "#/$defs/hits" }, - "overlooked": { "$ref": "#/$defs/hits" }, - "error": { "type": "string" } + "newer": { + "$ref": "#/$defs/hits" + }, + "overlooked": { + "$ref": "#/$defs/hits" + }, + "error": { + "type": "string" + }, + "same_year": { + "$ref": "#/$defs/hits" + } }, "$defs": { "hits": { "type": "array", "items": { "type": "object", - "required": ["title"], + "required": [ + "title" + ], "properties": { - "title": { "type": "string" }, - "year": { "type": ["integer", "null"] }, - "doi": { "type": "string" }, - "via": { "type": "string", "enum": ["citing", "search"] }, - "journal": { "type": "string" }, - "authors": { "type": "string" } + "title": { + "type": "string" + }, + "year": { + "type": [ + "integer", + "null" + ] + }, + "doi": { + "type": "string" + }, + "via": { + "type": "string", + "enum": [ + "citing", + "search" + ] + }, + "journal": { + "type": "string" + }, + "authors": { + "type": "string" + } } } } diff --git a/src/papertrace/cli.py b/src/papertrace/cli.py index 846996e..95d625c 100644 --- a/src/papertrace/cli.py +++ b/src/papertrace/cli.py @@ -554,6 +554,15 @@ def scout( console.print(f" [cyan]{h.year or '?'}[/cyan] {h.title[:76]}") if len(res.overlooked) > 5: console.print(f" [dim]… {len(res.overlooked) - 5} more in scout.json[/dim]") + if res.same_year: + console.print( + f"[yellow]▸[/yellow] same year as the paper: [bold]{len(res.same_year)}[/bold]" + " candidates [dim]— may postdate submission, so neither newer nor owed[/dim]" + ) + for h in res.same_year[:5]: + console.print(f" [cyan]{h.year or '?'}[/cyan] {h.title[:76]}") + if len(res.same_year) > 5: + console.print(f" [dim]… {len(res.same_year) - 5} more in scout.json[/dim]") console.print( "[dim]search-based — absence from these lists proves nothing; presence is a" " candidate for your judgement, not an accusation.[/dim]" diff --git a/src/papertrace/models.py b/src/papertrace/models.py index a17394a..a1a74b7 100644 --- a/src/papertrace/models.py +++ b/src/papertrace/models.py @@ -506,10 +506,18 @@ class ScoutResults: """Post-publication scan around one paper. `newer` holds what appeared after the paper (citing articles + later - keyword hits); `overlooked` holds what existed by the paper's year but is - absent from its reference list. Both are candidates for the user's - judgement — search-based, so absence from these lists proves nothing. + keyword hits); `overlooked` holds what was in print *before* the paper's + year and is absent from its reference list. Both are candidates for the + user's judgement — search-based, so absence from these lists proves nothing. A non-empty `error` means the scan soft-failed and may be incomplete. + + `same_year` is the third register, and it is deliberately not folded into + either neighbour. A paper from the manuscript's own year may have appeared + after submission, so "existed but uncited" holds it to a standard no author + can meet — on one real 2026 manuscript all fifteen overlooked candidates + were from 2026. It is not `newer` either, since it did not appear after. + Dropping it would lose a real finding: a paper published early in the same + year is exactly what a reviewer might legitimately raise. """ paper_title: str = "" @@ -520,6 +528,8 @@ class ScoutResults: date: str = "" newer: list[ScoutHit] = field(default_factory=list) overlooked: list[ScoutHit] = field(default_factory=list) + # the paper's own year — neither "since" nor "should have known" + same_year: list[ScoutHit] = field(default_factory=list) error: str = "" def to_json(self, path: Path) -> None: @@ -532,9 +542,14 @@ def to_json(self, path: Path) -> None: }, "query": self.query, "date": self.date, - "counts": {"newer": len(self.newer), "overlooked": len(self.overlooked)}, + "counts": { + "newer": len(self.newer), + "overlooked": len(self.overlooked), + "same_year": len(self.same_year), + }, "newer": [asdict(h) for h in self.newer], "overlooked": [asdict(h) for h in self.overlooked], + "same_year": [asdict(h) for h in self.same_year], "error": self.error, } path.write_text(json.dumps(payload, indent=2, ensure_ascii=False)) @@ -552,5 +567,7 @@ def from_json(cls, path: Path) -> ScoutResults: date=data.get("date", ""), newer=[ScoutHit(**h) for h in data.get("newer", [])], overlooked=[ScoutHit(**h) for h in data.get("overlooked", [])], + # .get: a scout.json written before the third register still loads + same_year=[ScoutHit(**h) for h in data.get("same_year", [])], error=data.get("error", ""), ) diff --git a/src/papertrace/scout.py b/src/papertrace/scout.py index 54e642c..e19e7bc 100644 --- a/src/papertrace/scout.py +++ b/src/papertrace/scout.py @@ -1,11 +1,15 @@ """Scout the literature around a paper — what its reference list doesn't know. -Two registers, both candidates for the user's judgement, never accusations: +Three registers, all candidates for the user's judgement, never accusations: - ``newer`` — appeared after the paper: articles that cite it, plus later keyword hits. What the paper could not have known. -- ``overlooked`` — existed by the paper's year but is absent from its +- ``overlooked`` — in print *before* the paper's year and absent from its reference list. What it could have cited. +- ``same_year`` — the paper's own year. Split out because it answers neither + question: it may have appeared after submission, so it is + not a citation the authors owed, and it did not come after, + so it is not literature published since. Search-based (Europe PMC) and therefore incomplete by construction — absence from these lists proves nothing. Network failures soft-fail: the error is @@ -15,6 +19,7 @@ from __future__ import annotations import datetime +import html import re from pathlib import Path @@ -31,6 +36,13 @@ "the", "and", "for", "with", "from", "into", "using", "based", "toward", "towards", "study", "analysis", "review", "novel", "between", "among", "their", "this", "that", "after", "before", "during", "versus", + # verbs and framing nouns that state what a paper CLAIMS, not what it is + # about. `improves` matched a stroke abstract shouting "IMPROVES" at a + # pancreatic-cancer paper, which is how this list grew. + "improve", "improves", "improved", "improving", "improvement", + "increase", "increases", "increased", "reduce", "reduces", "reduced", + "enhance", "enhances", "enhanced", "enables", "enabling", + "assessment", "evaluation", "comparison", "investigation", } @@ -64,13 +76,35 @@ def _norm_title(title: str) -> str: def _keywords(title: str, n: int = 4) -> list[str]: + """The n most specific-looking words of a title, for the neighbour search. + + Ranked by length, not by position. Taking the first n searched the opening + of the title and never reached its subject: "Image registration improves + inter-reader agreement ... in CT assessment of pancreas adenocarcinoma" + produced `image AND registration AND improves AND inter-reader`, so the + query described a method and omitted the disease entirely. + + Length is a proxy for topical specificity and nothing more — `adenocarcinoma` + over `image`. It is a heuristic, but it is one rule rather than a word list + that has to grow with every title style. The stop list only holds words that + carry no topic in any paper; guessing at more is how a filter starts + dropping real subject terms. + """ words = re.findall(r"[A-Za-z][A-Za-z\-]{3,}", title.lower()) - return [w for w in words if w not in _STOPWORDS][:n] + seen: dict[str, int] = {} + for i, w in enumerate(words): + if w not in _STOPWORDS and w not in seen: + seen[w] = i + ranked = sorted(seen, key=lambda w: (-len(w), seen[w])) + return ranked[:n] def _hit(d: dict, via: str) -> ScoutHit: + # Europe PMC escapes the markup its titles carry, so `CTVboost` + # arrives as `CTV<sub>boost</sub>` and was rendered verbatim + # into the report. Decoded once, here, where every hit is built. return ScoutHit( - title=" ".join((d.get("title") or "").split()).rstrip("."), + title=" ".join(html.unescape(d.get("title") or "").split()).rstrip("."), year=_year(d.get("pubYear")), doi=(d.get("doi") or "").lower(), via=via, @@ -238,13 +272,20 @@ def scout_case( continue # undatable → can't be placed honestly if res.paper_year and h.year > res.paper_year: res.newer.append(h) - elif not _probably_cited(h, cited_dois, cited_slugs): + elif _probably_cited(h, cited_dois, cited_slugs): + continue + elif res.paper_year and h.year == res.paper_year: + # its own year is neither "since" nor "should have + # known" — see ScoutResults for why it gets a register + res.same_year.append(h) + else: res.overlooked.append(h) - res.newer.sort(key=lambda h: (-(h.year or 0), h.title)) - res.overlooked.sort(key=lambda h: (-(h.year or 0), h.title)) + for reg in (res.newer, res.overlooked, res.same_year): + reg.sort(key=lambda h: (-(h.year or 0), h.title)) res.newer = res.newer[:NEWER_CAP] res.overlooked = res.overlooked[:OVERLOOKED_CAP] + res.same_year = res.same_year[:OVERLOOKED_CAP] except httpx.HTTPError as e: res.error = f"network: {type(e).__name__} — scan incomplete" return res diff --git a/src/papertrace/templates/report.md.j2 b/src/papertrace/templates/report.md.j2 index d41a5bf..1ff186f 100644 --- a/src/papertrace/templates/report.md.j2 +++ b/src/papertrace/templates/report.md.j2 @@ -119,7 +119,7 @@ flagged for you to weigh. - **[U{{ u.id }}]** {{ u.claim }}{% if u.location %} *({{ u.location }})*{% endif %} {% endfor %} {% endif %} -{% if scout and (scout.newer or scout.overlooked or scout.error) %} +{% if scout and (scout.newer or scout.overlooked or scout.same_year or scout.error) %} ## Literature scout — what the reference list doesn't know @@ -146,8 +146,8 @@ What the paper could not have known — articles citing it, plus later keyword h ### Existed but uncited ({{ scout.overlooked|length }} candidates) -In print by the paper's year and absent from its reference list (matched by -DOI and first-author heuristics — verify against the reference list yourself). +In print **before** the paper's year and absent from its reference list (matched +by DOI and first-author heuristics — verify against the reference list yourself). | Year | Title | Journal | DOI | |------|-------|---------|-----| @@ -155,6 +155,22 @@ DOI and first-author heuristics — verify against the reference list yourself). | {{ h.year or "?" }} | {{ h.title[:90] }} | {{ h.journal[:30] }} | {{ h.doi or "—" }} | {% endfor %} {% endif %} +{% if scout.same_year %} + +### Same year as the paper ({{ scout.same_year|length }} candidates) + +Published in {{ scout.paper_year or "the paper's own year" }} and not cited. +Held apart from the list above on purpose: a paper from the manuscript's own +year may have appeared after it was submitted, so it is **not** evidence of +something the authors should have known. It is also not literature published +since. Judge each on its date. + +| Year | Title | Journal | DOI | +|------|-------|---------|-----| +{% for h in scout.same_year %} +| {{ h.year or "?" }} | {{ h.title[:90] }} | {{ h.journal[:30] }} | {{ h.doi or "—" }} | +{% endfor %} +{% endif %} *Search-based (Europe PMC{% if scout.query %}, query `{{ scout.query }}`{% endif %}) — absence from these lists proves nothing, and presence is a candidate for your judgement, not an accusation.* diff --git a/src/papertrace/templates/report_editor.html.j2 b/src/papertrace/templates/report_editor.html.j2 index ab80c4e..2adaa1f 100644 --- a/src/papertrace/templates/report_editor.html.j2 +++ b/src/papertrace/templates/report_editor.html.j2 @@ -208,7 +208,7 @@ {% endfor %} {% endfor %} -{% if scout and (scout.newer or scout.overlooked or scout.error) %} +{% if scout and (scout.newer or scout.overlooked or scout.same_year or scout.error) %}

## Literature scout — what the reference list doesn't know

Candidates for your judgement, not accusations — search-based (Europe PMC{% if scout.paper_year %}, paper year {{ scout.paper_year }}{% endif %}), @@ -219,11 +219,19 @@ {{ h.title }}{% if h.journal %} · {{ h.journal }}{% endif %}{% if h.doi %} · {{ h.doi }}{% endif %} {% endfor %} {% for h in scout.overlooked[:8] %} - uncited · {{ h.year or "?" }}existed at pub. time + uncited · {{ h.year or "?" }}in print before pub. + {{ h.title }}{% if h.journal %} · {{ h.journal }}{% endif %}{% if h.doi %} · {{ h.doi }}{% endif %} +{% endfor %} +{% for h in scout.same_year[:8] %} + same year · {{ h.year or "?" }}may postdate submission {{ h.title }}{% if h.journal %} · {{ h.journal }}{% endif %}{% if h.doi %} · {{ h.doi }}{% endif %} {% endfor %} -{% if scout.newer|length > 10 or scout.overlooked|length > 8 %} +{% if scout.same_year %} +

Same-year candidates are listed apart on purpose: one from the paper's own year may + have appeared after submission, so it is neither literature published since nor something the authors should have known.

+{% endif %} +{% if scout.newer|length > 10 or scout.overlooked|length > 8 or scout.same_year|length > 8 %}

Full lists in scout.json.

{% endif %} {% endif %} diff --git a/src/papertrace/templates/report_terminal.html.j2 b/src/papertrace/templates/report_terminal.html.j2 index 4c2070b..a182f6e 100644 --- a/src/papertrace/templates/report_terminal.html.j2 +++ b/src/papertrace/templates/report_terminal.html.j2 @@ -181,7 +181,7 @@ {% endfor %} {% endfor %} {% endfor %} -{% if scout and (scout.newer or scout.overlooked or scout.error) %} +{% if scout and (scout.newer or scout.overlooked or scout.same_year or scout.error) %}
literature the reference list doesn't know (scout · europe pmc{% if scout.paper_year %} · paper year {{ scout.paper_year }}{% endif %})
@@ -198,6 +198,12 @@ {% if scout.overlooked|length > 6 %}
… {{ scout.overlooked|length - 6 }} more uncited candidates in scout.json
{% endif %} +{% for h in scout.same_year[:6] %} +
same year · {{ h.year or "?" }}[{{ h.via[:6] }}]{{ h.title }}{% if h.journal %} · {{ h.journal }}{% endif %}{% if h.doi %} · {{ h.doi }}{% endif %}
+{% endfor %} +{% if scout.same_year %} +
same-year candidates listed apart — one from the paper's own year may postdate submission, so it is neither newer literature nor a citation the authors owed{% if scout.same_year|length > 6 %} · … {{ scout.same_year|length - 6 }} more in scout.json{% endif %}
+{% endif %} {% endif %}
# evidence crops are real pages of the cited sources · each crop states whether its anchor phrase was located
diff --git a/tests/test_scout.py b/tests/test_scout.py index a6c9253..a8b22df 100644 --- a/tests/test_scout.py +++ b/tests/test_scout.py @@ -109,10 +109,11 @@ def test_scout_registers_and_dedup(tmp_path): assert newer == {"A citing follow-up", "Newer keyword hit"} assert {h.via for h in res.newer} == {"citing", "search"} - # cited-by-DOI, cited-by-slug and the paper itself never reach overlooked; - # a same-year hit does (year ≤ paper year, plausibly knowable) + # cited-by-DOI, cited-by-slug and the paper itself never reach overlooked — + # and neither does a same-year hit, which has its own register overlooked = {h.title for h in res.overlooked} - assert overlooked == {"Old uncited candidate", "Same-year neighbour"} + assert overlooked == {"Old uncited candidate"} + assert {h.title for h in res.same_year} == {"Same-year neighbour"} # newest first assert [h.year for h in res.newer] == [2023, 2022] @@ -154,7 +155,7 @@ def test_scout_json_roundtrip(tmp_path): res.to_json(out) data = json.loads(out.read_text()) - assert data["counts"] == {"newer": 2, "overlooked": 2} + assert data["counts"] == {"newer": 2, "overlooked": 1, "same_year": 1} again = ScoutResults.from_json(out) assert again.paper_year == 2020 assert {h.title for h in again.newer} == {h.title for h in res.newer} @@ -164,7 +165,9 @@ def test_scout_json_roundtrip(tmp_path): def test_keywords_drop_stopwords(): kws = _keywords("Towards a novel deep learning analysis of chest radiographs") assert "towards" not in kws and "novel" not in kws and "analysis" not in kws - assert kws[:3] == ["deep", "learning", "chest"] + # ranked by length rather than by position, so the specific words win + # wherever they sit in the title — `radiographs` over `deep` + assert kws[:2] == ["radiographs", "learning"], kws def test_email_fallback_old_env_var(monkeypatch): @@ -226,3 +229,132 @@ def test_without_a_doi_the_advice_to_pin_one_still_stands(tmp_path): case = _case(tmp_path) res = scout_case(case, transport=httpx.MockTransport(_no_hits)) assert "--doi" in res.error + + +# --- the keyword query has to be about the subject --------------------------- +# +# A live audit of "Image registration improves inter-reader agreement of +# objective response in CT assessment of pancreas adenocarcinoma" searched for +# `image AND registration AND improves AND inter-reader`. Two faults in one +# line: `improves` is a verb carrying no topic, and taking the FIRST four +# content words never reaches the subject, which in this title sits at the end. +# The register came back with a stroke conference abstract whose shouted title +# contained "IMPROVES". + +_REAL_TITLE = ( + "Image registration improves inter-reader agreement of objective response " + "in CT assessment of pancreas adenocarcinoma" +) + + +def test_the_keyword_query_reaches_the_subject_of_the_paper(): + kws = _keywords(_REAL_TITLE) + assert "adenocarcinoma" in kws, kws + assert "registration" in kws, kws + + +def test_a_title_verb_is_not_a_keyword(): + """`improves` matched an unrelated abstract on the same verb.""" + assert "improves" not in _keywords(_REAL_TITLE) + + +def test_keyword_selection_does_not_depend_on_position_in_the_title(): + """The subject is as often at the end of a title as the start.""" + front = _keywords("Pancreas adenocarcinoma assessed by registration of CT") + back = _keywords("Registration of CT for assessment of pancreas adenocarcinoma") + assert "adenocarcinoma" in front and "adenocarcinoma" in back + + +# --- a same-year paper is not an overlooked one ------------------------------ + + +def test_a_same_year_hit_is_not_filed_as_overlooked(tmp_path): + """"Existed but uncited" invites the reader to ask what the authors missed. + A paper from the manuscript's own year may have appeared after submission, + so holding it to that standard is unfair — and on a real 2026 paper every + one of the fifteen candidates was from 2026.""" + case = _case(tmp_path) + res = scout_case(case, transport=_mock_transport()) + + assert "Same-year neighbour" not in {h.title for h in res.overlooked} + + +def test_a_same_year_hit_is_kept_in_its_own_register(tmp_path): + """Not dropped either: a paper published early in the same year is exactly + the kind of thing a reviewer might legitimately raise. It is a third + status, and folding it into either neighbour states something false.""" + case = _case(tmp_path) + res = scout_case(case, transport=_mock_transport()) + + assert "Same-year neighbour" in {h.title for h in res.same_year} + assert "Same-year neighbour" not in {h.title for h in res.newer} + + +def test_the_same_year_register_round_trips(tmp_path): + """Gate 2 — a new field is a schema change and a round-trip test.""" + from papertrace.models import ScoutResults + + case = _case(tmp_path) + res = scout_case(case, transport=_mock_transport()) + path = tmp_path / "scout.json" + res.to_json(path) + + back = ScoutResults.from_json(path) + assert [h.title for h in back.same_year] == [h.title for h in res.same_year] + assert json.loads(path.read_text())["counts"]["same_year"] == len(res.same_year) + + +def test_an_older_uncited_hit_is_still_overlooked(tmp_path): + """The register keeps its job — this is a narrowing, not a removal.""" + case = _case(tmp_path) + res = scout_case(case, transport=_mock_transport()) + assert "Old uncited candidate" in {h.title for h in res.overlooked} + + +# --- Europe PMC returns escaped markup --------------------------------------- + + +def test_markup_entities_in_a_title_are_decoded(tmp_path): + """Real hits arrived as `CTV<sub>boost</sub>` and were rendered + verbatim into the report.""" + def handler(request: httpx.Request) -> httpx.Response: + url = str(request.url) + if "/citations" in url: + return httpx.Response(200, json={"citationList": {"citation": []}}) + q = request.url.params.get("query", "") + if q.startswith('DOI:"') or q.startswith('TITLE:"'): + return httpx.Response(200, json={"resultList": {"result": [{ + "id": "33333333", "source": "MED", "doi": "10.1000/PAPER", + "title": PAPER_TITLE, "pubYear": "2020", + }]}}) + return httpx.Response(200, json={"resultList": {"result": [ + _epmc_result("Improving CTV<sub>boost</sub> delineation", + 2018, doi="10.1000/esc"), + ]}}) + + res = scout_case(_case(tmp_path), transport=httpx.MockTransport(handler)) + titles = " ".join(h.title for h in res.overlooked) + assert "<" not in titles, titles + assert "CTVboost" in titles + + +def test_the_same_year_register_reaches_all_three_report_formats(tmp_path): + """A register the reader of one format cannot see is a register that does + not exist for them — the same rule the disclosure parity test enforces.""" + from papertrace.models import RunResults, ScoutHit, ScoutResults + from papertrace.report import write_reports + + scout = ScoutResults( + paper_title="A paper", paper_year=2026, date="2026-09-04", + # no apostrophe: the HTML looks autoescape their interpolations, so a + # literal assertion on the rendered page must not straddle an escape + same_year=[ScoutHit(title="A neighbour from the same publication year", year=2026, + doi="10.1000/sy", via="search", journal="Eur J Radiol")], + ) + out = tmp_path / "out" + write_reports(RunResults(manuscript="m.pdf"), None, out, png=False, scout=scout) + + for name in ("report.md", "report_editor.html", "report_terminal.html"): + body = (out / name).read_text() + assert "A neighbour from the same publication year" in body, f"missing from {name}" + assert "same year" in body.lower(), f"unlabelled in {name}" From 265f3c34672cb139b39b21cf69096fa1a3c17ab0 Mon Sep 17 00:00:00 2001 From: defraction <796174+defraction0@users.noreply.github.com> Date: Fri, 4 Sep 2026 15:46:09 +0200 Subject: [PATCH 10/54] report: the HTML looks never actually escaped anything MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit select_autoescape(["html"]) matches a template name ending in `.html`. These are `report_editor.html.j2` and `report_terminal.html.j2`, so nothing matched and autoescape was off for all three formats — including the two that emit HTML. disclosures.py reasons about the setting in its own docstring, which is how long it went unread. It stayed invisible because the one field carrying angle brackets is a Europe PMC title, and the API pre-escapes those. Decoding them in the commit before this one is what made it reachable. Cited source PDFs are downloaded from third parties and their text reaches the report, so this is not hypothetical. Matched on `.html.j2` now, via an explicit predicate rather than a helper whose matching rule has to be remembered. No interpolation is meant to emit markup — there is no `|safe` in any template — so escaping all of them is the whole fix. Markdown is not HTML and stays verbatim. --- src/papertrace/report.py | 21 +++++++++++++++++++-- tests/test_disclosure_parity.py | 24 ++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/src/papertrace/report.py b/src/papertrace/report.py index e38f162..749ec9a 100644 --- a/src/papertrace/report.py +++ b/src/papertrace/report.py @@ -12,7 +12,7 @@ from importlib import resources from pathlib import Path -from jinja2 import Environment, FileSystemLoader, select_autoescape +from jinja2 import Environment, FileSystemLoader from . import __version__ from .disclosures import ( @@ -27,10 +27,27 @@ TEMPLATES = Path(str(resources.files("papertrace") / "templates")) +def _autoescape(name: str | None) -> bool: + """Escape interpolations in the HTML looks, never in the markdown one. + + Matched on `.html.j2`, not by `select_autoescape(["html"])`, which tests for + a name ending in `.html` — these templates end in `.j2`, so nothing ever + matched and every format rendered unescaped. It stayed invisible because the + one field that carries angle brackets, a Europe PMC title, arrives + pre-escaped from the API; decoding those entities is what made it reachable. + + Cited source PDFs are downloaded from third parties and their text reaches + the report, so this is not hypothetical. No template interpolation is meant + to emit markup — there is no `|safe` anywhere — so escaping every one of + them is the whole fix. Markdown is not HTML and is left alone. + """ + return bool(name) and name.endswith((".html.j2", ".htm.j2")) + + def _env() -> Environment: return Environment( loader=FileSystemLoader(TEMPLATES), - autoescape=select_autoescape(["html"]), + autoescape=_autoescape, trim_blocks=True, lstrip_blocks=True, ) diff --git a/tests/test_disclosure_parity.py b/tests/test_disclosure_parity.py index aeceecc..169bba8 100644 --- a/tests/test_disclosure_parity.py +++ b/tests/test_disclosure_parity.py @@ -392,3 +392,27 @@ def test_an_anchor_state_without_a_crop_still_reaches_every_format(tmp_path, anc anchor = next(d for d in claim_disclosures(claim) if d.key == "anchor") for name, body in rendered.items(): assert anchor.token in body, f"anchor token missing from {name}" + + +# --- the HTML reports actually escape what they interpolate ------------------ + + +def test_markup_in_source_text_cannot_reach_the_html_reports_unescaped(tmp_path): + """`select_autoescape(["html"])` matches names ending `.html`. The templates + are named `report_editor.html.j2`, so nothing ever matched and autoescape + was off for all three formats — including the two that emit HTML. + + It went unnoticed because Europe PMC pre-escapes the markup in its titles, + so the one field carrying angle brackets arrived already safe. Cited source + PDFs are downloaded from third parties, and their text reaches the report. + """ + hostile = '' + claim = _claim(claim=f"a claim containing {hostile}", verdict="supported") + rendered = _render(RunResults(manuscript="m.pdf", claims=[claim]), tmp_path) + + for name in ("report_editor.html", "report_terminal.html"): + assert "