From adb640c98efb4a07b75f4cc3d23bf941b071c807 Mon Sep 17 00:00:00 2001 From: Research Assistant Date: Thu, 11 Jun 2026 16:25:12 +0800 Subject: [PATCH] =?UTF-8?q?fix:=20close=20OCR=20figure=20reader=20contract?= =?UTF-8?q?=20=E2=80=94=20page-scoped=20block=20ids,=20legacy=20render=20s?= =?UTF-8?q?uppression,=20per-paper=20audit=20gate?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- paperforge/worker/ocr_figure_reader.py | 32 ++- paperforge/worker/ocr_render.py | 12 +- tests/test_ocr_real_paper_contract.py | 261 ++++++++++++++++++++++ tests/test_ocr_real_paper_reader_audit.py | 13 +- 4 files changed, 286 insertions(+), 32 deletions(-) create mode 100644 tests/test_ocr_real_paper_contract.py diff --git a/paperforge/worker/ocr_figure_reader.py b/paperforge/worker/ocr_figure_reader.py index 76ea5666..1d636673 100644 --- a/paperforge/worker/ocr_figure_reader.py +++ b/paperforge/worker/ocr_figure_reader.py @@ -190,7 +190,7 @@ def _materialize_hold_outcome( "caption_text": caption_text, "visual_groups": [], "consumed_caption_block_ids": ( - [legend_block_id] if reader_visible and legend_block_id is not None and caption_text else [] + [{"page": page, "block_id": legend_block_id}] if reader_visible and legend_block_id is not None and caption_text else [] ), "consumed_asset_block_ids": [], "debug_refs": {"candidate_asset_ids": list(candidate_asset_ids), "hold_visibility": hold_visibility}, @@ -231,8 +231,8 @@ def _materialize_reader_figure( "rendered_as_representative": True, } ], - "consumed_caption_block_ids": [legend_block_id] if legend_block_id is not None else [], - "consumed_asset_block_ids": asset_ids, + "consumed_caption_block_ids": [{"page": normalized_item.get("page"), "block_id": legend_block_id}] if legend_block_id is not None else [], + "consumed_asset_block_ids": [{"page": normalized_item.get("page"), "block_id": aid} for aid in asset_ids], "debug_refs": {}, } @@ -260,7 +260,7 @@ def _materialize_reader_figure( "rendered_as_representative": False, } ], - "consumed_caption_block_ids": [legend_block_id] if legend_block_id is not None else [], + "consumed_caption_block_ids": [{"page": normalized_item.get("page"), "block_id": legend_block_id}] if legend_block_id is not None else [], "consumed_asset_block_ids": [], "debug_refs": {"candidate_asset_ids": candidate_asset_ids}, } @@ -278,7 +278,7 @@ def _materialize_reader_figure( "caption_block_id": legend_block_id, "caption_text": caption_text, "visual_groups": [], - "consumed_caption_block_ids": [legend_block_id] if legend_block_id is not None else [], + "consumed_caption_block_ids": [{"page": normalized_item.get("page"), "block_id": legend_block_id}] if legend_block_id is not None else [], "consumed_asset_block_ids": [], "debug_refs": {}, } @@ -298,7 +298,7 @@ def _materialize_reader_figure( "caption_block_id": legend_block_id, "caption_text": caption_text, "visual_groups": [], - "consumed_caption_block_ids": [legend_block_id] if legend_block_id is not None else [], + "consumed_caption_block_ids": [{"page": normalized_item.get("page"), "block_id": legend_block_id}] if legend_block_id is not None else [], "consumed_asset_block_ids": [], "debug_refs": {}, } @@ -327,7 +327,7 @@ def _materialize_reader_figure( } ], "consumed_caption_block_ids": [], - "consumed_asset_block_ids": asset_ids, + "consumed_asset_block_ids": [{"page": normalized_item.get("page"), "block_id": aid} for aid in asset_ids], "debug_refs": {}, } @@ -361,25 +361,25 @@ def _passes_salient_visual_group_gate(item: dict) -> bool: def _collect_reader_eligible_inputs(normalized: dict) -> list[dict]: eligible: list[dict] = [] - seen_legends: set[int | str] = set() + seen_legends: set[tuple[int | str | None, int | str | None]] = set() for source_name in ("matched_figures", "held_figures", "ambiguous_figures"): for item in normalized.get(source_name, []): - legend_block_id = item.get("legend_block_id") - if legend_block_id is None or legend_block_id in seen_legends: + legend_id = (item.get("page"), item.get("legend_block_id")) + if legend_id in seen_legends: continue if not _passes_formal_legend_gate(item): continue - seen_legends.add(legend_block_id) + seen_legends.add(legend_id) eligible.append({"kind": "legend", "source": source_name, "item": item}) for item in normalized.get("unmatched_legends", []): - legend_block_id = item.get("legend_block_id") - if legend_block_id is None or legend_block_id in seen_legends: + legend_id = (item.get("page"), item.get("legend_block_id")) + if legend_id in seen_legends: continue if not _passes_formal_legend_gate(item): continue - seen_legends.add(legend_block_id) + seen_legends.add(legend_id) eligible.append({"kind": "legend", "source": "unmatched_legends", "item": item}) for item in normalized.get("unresolved_clusters", []): @@ -415,10 +415,6 @@ def synthesize_reader_figures( consumed_asset_ids.extend(materialized.get("consumed_asset_block_ids", [])) coverage_total = len(eligible_inputs) - reader_figures = [ - figure for figure in reader_figures - if figure.get("visual_groups") - ] return { "normalized_inputs": normalized, "reader_figures": reader_figures, diff --git a/paperforge/worker/ocr_render.py b/paperforge/worker/ocr_render.py index 3547e65e..8d540c37 100644 --- a/paperforge/worker/ocr_render.py +++ b/paperforge/worker/ocr_render.py @@ -674,11 +674,6 @@ def _emit_page_objects( for fig in figures_by_page.get(page, []): if str(fig['figure_id']).startswith("unmatched_legend_"): continue - caption = str(fig.get("caption") or "").strip() - if caption and caption not in emitted_figure_captions: - lines.append(caption) - lines.append("") - emitted_figure_captions.add(caption) lines.append(f"![[render/figures/{fig['figure_id']}.md]]") lines.append("") for cluster_id in unresolved_clusters_by_page.get(page, []): @@ -714,7 +709,7 @@ def render_fulltext_markdown( emitted_figure_captions: set[str] = set() reader_figures = (reader_payload or {}).get("reader_figures", []) - consumed_caption_block_ids = set((reader_payload or {}).get("consumed_caption_block_ids", [])) + consumed_caption_keys = {(item.get("page"), item.get("block_id")) for item in (reader_payload or {}).get("consumed_caption_block_ids", []) if item.get("block_id") is not None} rendered_reader_figure_ids: set[str] = set() _block_page_map: dict[int, int] = {} @@ -967,12 +962,14 @@ def render_fulltext_markdown( "abstract_body", "frontmatter_noise", "table_html", + "figure_caption", } if role in _SKIPPED_BODY_ROLES: continue block_id = block.get("block_id") - if block_id is not None and block_id in consumed_caption_block_ids: + block_page = block.get("page") + if block_id is not None and (block_page, block_id) in consumed_caption_keys: continue raw_text = block.get("text", "") @@ -980,7 +977,6 @@ def render_fulltext_markdown( text = re.sub(r"]*>.*?", "", text, flags=re.DOTALL | re.IGNORECASE) if text.strip().lower().startswith(" Path: + v = os.environ.get(REAL_VAULT_ENV) + if not v: + pytest.skip(f"Set {REAL_VAULT_ENV}") + return Path(v) + + +def _ocr_root() -> Path: + return _vault() / "System" / "PaperForge" / "ocr" + + +def _keys() -> list[str]: + raw = os.environ.get(REAL_KEYS_ENV, "") + if raw: + return [k.strip() for k in raw.split(",") if k.strip()] + return ALL_KEYS + + +def _structured_path(key: str) -> Path: + return _ocr_root() / key / "structure" / "blocks.structured.jsonl" + + +def _fulltext_path(key: str) -> Path: + return _ocr_root() / key / "fulltext.md" + + +def _reader_figures_path(key: str) -> Path: + return _ocr_root() / key / "structure" / "reader_figures.json" + + +def _read_jsonl(path: Path) -> list[dict]: + return [json.loads(line) for line in path.read_text(encoding="utf-8").splitlines() if line.strip()] + + +def _read_json(path: Path) -> dict: + return json.loads(path.read_text(encoding="utf-8")) + + +_BODY_ROLES = {"body_paragraph", "abstract_body"} +_HEADING_ROLES = {"section_heading", "subsection_heading", "sub_subsection_heading", "reference_heading", "backmatter_heading"} +_FORBIDDEN_BODY_PHRASES = [ + "published online", "conflict of interest", "publisher's note", "ethics statement", + "reviewed by:", "edited by:", "specialty section:", "citation:", +] +_FORBIDDEN_HEADING_PHRASES = [ + "published online", "review article", "steve stegen", "geert carmeliet", +] + + +def _block_text(block: dict) -> str: + return str(block.get("text") or block.get("block_content") or "") + + +def _is_figure_caption_in_body(block: dict) -> bool: + if block.get("role") != "body_paragraph": + return False + text = _block_text(block).strip() + if not text: + return False + if LEGEND_PREFIX.match(text) or text.startswith("> **"): + return True + return False + + +@pytest.fixture(scope="module") +def rebuilt() -> dict: + from paperforge.worker.ocr_rebuild import run_derived_rebuild_for_keys + result = run_derived_rebuild_for_keys(_vault(), _keys()) + assert result.get("rebuild_count", 0) >= 1 + return result + + +def test_rebuild_succeeds(rebuilt: dict) -> None: + assert rebuilt["rebuild_count"] >= 1 + + +# ---- Per-paper contract tests ---- + +_PAPER_CONTRACTS = { + "TSCKAVIS": { + "min_body": 48, + "min_headings": 4, + "expected_figures": 5, + "body_no_figure_captions": True, + "no_frontmatter_before_intro": True, + "reader_figures_min": 2, + }, + "CAQNW9Q2": { + "min_body": 27, + "min_headings": 5, + "expected_figures": 3, + "body_no_figure_captions": True, + "no_frontmatter_before_intro": True, + "reader_figures_min": 2, + }, + "A8E7SRVS": { + "min_body": 42, + "min_headings": 4, + "expected_figures": 6, + "body_no_figure_captions": True, + "no_frontmatter_before_intro": True, + "reader_figures_min": 4, + }, + "K7R8PEKW": { + "min_body": 60, + "min_headings": 4, + "expected_figures": 4, + "body_no_figure_captions": True, + "no_frontmatter_before_intro": True, + "reader_figures_min": 3, + }, + "DWQQK2YB": { + "min_body": 25, + "min_headings": 4, + "expected_figures": 4, + "body_no_figure_captions": True, + "no_frontmatter_before_intro": True, + "reader_figures_min": 3, + }, + "M36WA39N": { + "min_body": 45, + "min_headings": 4, + "expected_figures": 7, + "body_no_figure_captions": True, + "no_frontmatter_before_intro": True, + "reader_figures_min": 5, + }, + "SAN9AYVR": { + "min_body": 200, + "min_headings": 10, + "expected_figures": 31, + "body_no_figure_captions": True, + "no_frontmatter_before_intro": True, + "reader_figures_min": 10, + }, + "2GN9LMCW": { + "min_body": 25, + "min_headings": 3, + "expected_figures": 4, + "body_no_figure_captions": True, + "no_frontmatter_before_intro": True, + "reader_figures_min": 2, + }, + "7C8829BD": { + "min_body": 65, + "min_headings": 4, + "expected_figures": 7, + "body_no_figure_captions": True, + "no_frontmatter_before_intro": True, + "reader_figures_min": 1, + }, +} + + +@pytest.mark.parametrize("key", sorted(_PAPER_CONTRACTS)) +def test_body_paragraph_count(rebuilt: dict, key: str) -> None: + blocks = _read_jsonl(_structured_path(key)) + body_count = sum(1 for b in blocks if b.get("role") in _BODY_ROLES) + assert body_count >= _PAPER_CONTRACTS[key]["min_body"], f"{key}: only {body_count} body paragraphs, expected >= {_PAPER_CONTRACTS[key]['min_body']}" + + +@pytest.mark.parametrize("key", sorted(_PAPER_CONTRACTS)) +def test_figure_captions_not_in_body(rebuilt: dict, key: str) -> None: + blocks = _read_jsonl(_structured_path(key)) + body_fig_captions = [b for b in blocks if _is_figure_caption_in_body(b)] + max_allowed = 1 # minor strict-layer misclassifications are acceptable + assert len(body_fig_captions) <= max_allowed, ( + f"{key}: {len(body_fig_captions)} figure captions leaked into body_paragraph " + f"(max allowed {max_allowed}): " + f"block_ids={[b['block_id'] for b in body_fig_captions[:5]]}" + ) + + +@pytest.mark.parametrize("key", sorted(_PAPER_CONTRACTS)) +def test_fulltext_no_figure_caption_pollution(rebuilt: dict, key: str) -> None: + fulltext = _fulltext_path(key).read_text(encoding="utf-8", errors="replace") + lines = fulltext.split("\n") + body_lines = [] + for line in lines: + stripped = line.strip() + if stripped.startswith("![[") or stripped.startswith(">") or stripped.startswith("#") or stripped.startswith("