From cba3f34a431d91e10659a01e555f6fdb268bd260 Mon Sep 17 00:00:00 2001 From: LLLin000 <809867916@qq.com> Date: Fri, 3 Jul 2026 12:42:58 +0800 Subject: [PATCH] =?UTF-8?q?fix(ocr):=20fix=20same-page=20pass=20=E2=80=94?= =?UTF-8?q?=20filter=20locator=20captions,=20guard=20unnumbered=20formal?= =?UTF-8?q?=20captions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs in PrimarySamePagePass identified by review: 1. Previous-page locator captions (e.g. 'Fig. 1 (See legend on previous page)') were not filtered from same-page matching. Now skips any legend where ocr_figures._is_previous_page_legend_locator() returns True, preserving them for future locator-specific handling. 2. Formal captions with no parsed figure number crashed in _materialize_match() because _format_figure_id() requires int. Now produces figure_unknown_ fallback IDs, consistent with legacy behavior at ocr_figures.py:3918. Tests added for both behaviors. --- paperforge/worker/ocr_figure_vnext_passes.py | 9 +++- tests/test_ocr_figure_vnext_passes.py | 46 ++++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/paperforge/worker/ocr_figure_vnext_passes.py b/paperforge/worker/ocr_figure_vnext_passes.py index 1fca8dd6..1c68b07b 100644 --- a/paperforge/worker/ocr_figure_vnext_passes.py +++ b/paperforge/worker/ocr_figure_vnext_passes.py @@ -21,6 +21,8 @@ class PrimarySamePagePass: page = _resource_page(legend) if page is None: continue + if ocr_figures._is_previous_page_legend_locator(legend): + continue page_groups = [g for g in state.candidate_index.candidate_groups if _resource_page(g) == page] for group in page_groups: score = ocr_figures._score_legend_to_group( @@ -70,9 +72,14 @@ class PrimarySamePagePass: for b in state.candidate_index.deduped_legends if str(b.get("block_id", "")) == legend.block_id ) + figure_no = proposal.figure_no namespace = ocr_figures._extract_figure_namespace(legend_text) + if figure_no is None: + figure_id = f"figure_unknown_{len(state.matches):03d}" + else: + figure_id = ocr_figures._format_figure_id(namespace, figure_no) return { - "figure_id": ocr_figures._format_figure_id(namespace, proposal.figure_no), + "figure_id": figure_id, "figure_namespace": namespace, "figure_number": proposal.figure_no, "legend_block_id": legend.block_id, diff --git a/tests/test_ocr_figure_vnext_passes.py b/tests/test_ocr_figure_vnext_passes.py index 8747d5af..26899094 100644 --- a/tests/test_ocr_figure_vnext_passes.py +++ b/tests/test_ocr_figure_vnext_passes.py @@ -45,3 +45,49 @@ def test_primary_same_page_pass_prefers_higher_score_when_two_legends_compete_fo assert len(report.accepted) == 1 assert report.accepted[0].figure_no == 2 + + +def test_primary_same_page_pass_ignores_previous_page_locator(monkeypatch): + blocks = [ + {"block_id": "c1", "page": 1, "role": "figure_caption", "text": "Fig. 1 (See legend on previous page).", "bbox": [0, 100, 200, 150]}, + {"block_id": "c2", "page": 1, "role": "figure_caption", "text": "Figure 2. Caption", "bbox": [0, 160, 200, 210]}, + {"block_id": "a1", "page": 1, "role": "figure_asset", "bbox": [0, 0, 200, 90], "raw_label": "image"}, + ] + corpus = FigureCorpus.from_blocks(blocks, page_width=1200) + index = FigureCandidateIndex.from_corpus(corpus) + if not index.candidate_groups: + index.candidate_groups = [{"group_id": "g1", "page": 1, "asset_block_ids": ["a1"], "media_blocks": [{"block_id": "a1"}], "group_type": "single_asset", "cluster_bbox": [0, 0, 200, 90]}] + + monkeypatch.setattr("paperforge.worker.ocr_figures._score_legend_to_group", lambda *a, **k: {"score": 0.9, "decision": "matched", "evidence": [""]}) + monkeypatch.setattr("paperforge.worker.ocr_figures.score_figure_caption", lambda *a, **k: {"score": 0.9}) + + state = FigurePipelineState(corpus=corpus, candidate_index=index, ledger=OwnershipLedger()) + report = PrimarySamePagePass().run(state) + + # Only the non-locator caption should match + assert len(report.accepted) == 1 + assert report.accepted[0].figure_no == 2 + + +def test_primary_same_page_pass_unnumbered_formal_caption_does_not_crash(monkeypatch): + blocks = [ + {"block_id": "c1", "page": 1, "role": "figure_caption", "text": "Figure. Detailed description of the experimental results showing significant differences between groups.", "bbox": [0, 100, 500, 200]}, + {"block_id": "a1", "page": 1, "role": "figure_asset", "bbox": [0, 0, 200, 90], "raw_label": "image"}, + ] + corpus = FigureCorpus.from_blocks(blocks, page_width=1200) + index = FigureCandidateIndex.from_corpus(corpus) + if not index.candidate_groups: + index.candidate_groups = [{"group_id": "g1", "page": 1, "asset_block_ids": ["a1"], "media_blocks": [{"block_id": "a1"}], "group_type": "single_asset", "cluster_bbox": [0, 0, 200, 90]}] + + monkeypatch.setattr("paperforge.worker.ocr_figures._score_legend_to_group", lambda *a, **k: {"score": 0.9, "decision": "matched", "evidence": [""]}) + monkeypatch.setattr("paperforge.worker.ocr_figures.score_figure_caption", lambda *a, **k: {"score": 0.9}) + + state = FigurePipelineState(corpus=corpus, candidate_index=index, ledger=OwnershipLedger()) + report = PrimarySamePagePass().run(state) + + assert len(report.accepted) == 1 + assert report.accepted[0].figure_no is None + assert len(state.matches) == 1 + # Must produce figure_unknown_* id, not crash + assert state.matches[0]["figure_id"].startswith("figure_unknown_") + assert state.matches[0]["figure_number"] is None