mirror of
https://github.com/lllin000/PaperForge.git
synced 2026-07-22 06:50:53 +00:00
fix: resolve figure_id collisions via name mutation
When two figures share the same figure_id (e.g. supplementary "Figure S.1" and main "Figure 1" both mapped to "figure_001"), the second occurrence is renamed with an "s" prefix: figure_001 -> figure_s001, figure_ss001, etc. This avoids file overwrite in ocr_objects.py and ensures both body and supplementary figures appear at their correct positions in the rendered fulltext. Changes: - _resolve_figure_id_collisions() in ocr_figures.py - figure_id passthrough in ocr_figure_reader.py (reader + normalize) - _reader_figure_embed_target prefers explicit figure_id - 11 unit tests covering single/triple/multi collisions, mixed buckets, empty IDs, realistic body+supplementary layout
This commit is contained in:
parent
ebe5c07969
commit
faa70545eb
4 changed files with 134 additions and 1 deletions
|
|
@ -128,6 +128,7 @@ def _normalize_bucket(
|
|||
|
||||
normalized.append(
|
||||
{
|
||||
"figure_id": source_item.get("figure_id", ""),
|
||||
"figure_number": source_item.get("figure_number")
|
||||
or source_marker.get("number")
|
||||
or _inferred_figure_number,
|
||||
|
|
@ -272,6 +273,7 @@ def _materialize_reader_figure(
|
|||
first_asset_block_id=asset_ids[0] if asset_ids else None,
|
||||
ordinal=ordinal,
|
||||
),
|
||||
"figure_id": normalized_item.get("figure_id", ""),
|
||||
"figure_number": figure_number,
|
||||
"reader_status": reader_status,
|
||||
"strict_status": strict_status,
|
||||
|
|
|
|||
|
|
@ -2601,6 +2601,23 @@ def _settle_cross_page_reserved_objects(
|
|||
)
|
||||
|
||||
|
||||
def _resolve_figure_id_collisions(figure_inventory: dict) -> None:
|
||||
_collision_seen: dict[str, int] = {}
|
||||
for _fig in (
|
||||
*figure_inventory.get("matched_figures", []),
|
||||
*figure_inventory.get("held_figures", []),
|
||||
*figure_inventory.get("ambiguous_figures", []),
|
||||
):
|
||||
_fig_id = _fig.get("figure_id", "")
|
||||
if not _fig_id:
|
||||
continue
|
||||
if _fig_id in _collision_seen:
|
||||
_collision_seen[_fig_id] += 1
|
||||
_fig["figure_id"] = f"figure_{'s' * _collision_seen[_fig_id]}{_fig_id.removeprefix('figure_')}"
|
||||
else:
|
||||
_collision_seen[_fig_id] = 0
|
||||
|
||||
|
||||
def build_figure_inventory(structured_blocks: list[dict], page_width: float = 1200) -> dict[str, Any]:
|
||||
legends: list[dict] = []
|
||||
held_figures: list[dict] = []
|
||||
|
|
@ -4259,6 +4276,8 @@ def build_figure_inventory(structured_blocks: list[dict], page_width: float = 12
|
|||
inventory,
|
||||
)
|
||||
|
||||
_resolve_figure_id_collisions(inventory)
|
||||
|
||||
return inventory
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -880,9 +880,12 @@ def _render_reader_figure_card(figure: dict) -> list[str]:
|
|||
|
||||
def _reader_figure_embed_target(figure: dict) -> str | None:
|
||||
status = str(figure.get("reader_status") or "")
|
||||
figure_number = figure.get("figure_number")
|
||||
if status not in {"EXACT_MATCH", "SEQUENCE_MATCH"}:
|
||||
return None
|
||||
figure_id = figure.get("figure_id", "")
|
||||
if figure_id:
|
||||
return figure_id
|
||||
figure_number = figure.get("figure_number")
|
||||
if figure_number is None:
|
||||
return None
|
||||
return f"figure_{int(figure_number):03d}"
|
||||
|
|
|
|||
|
|
@ -5353,3 +5353,112 @@ def test_page_assets_count_uses_ordered_legends_not_stale_deduped() -> None:
|
|||
assert result2["decision"] == "rejected"
|
||||
assert "multiple_numbered_legends" in result2.get("evidence", [])
|
||||
|
||||
|
||||
class TestResolveFigureIdCollisions:
|
||||
"""_resolve_figure_id_collisions must never produce duplicate figure_ids."""
|
||||
|
||||
def _make_fig(self, figure_id: str, **kw: object) -> dict:
|
||||
return {"figure_id": figure_id, "page": 1, **kw}
|
||||
|
||||
def _run(self, figs: list[dict]) -> list[dict]:
|
||||
from paperforge.worker.ocr_figures import _resolve_figure_id_collisions
|
||||
inventory = {"matched_figures": list(figs), "held_figures": [], "ambiguous_figures": []}
|
||||
_resolve_figure_id_collisions(inventory)
|
||||
return inventory["matched_figures"]
|
||||
|
||||
def _ids(self, figs: list[dict]) -> list[str]:
|
||||
return [f["figure_id"] for f in figs]
|
||||
|
||||
def test_no_collision(self) -> None:
|
||||
figs = [self._make_fig("figure_001"), self._make_fig("figure_002"), self._make_fig("figure_003")]
|
||||
result = self._run(figs)
|
||||
assert self._ids(result) == ["figure_001", "figure_002", "figure_003"]
|
||||
|
||||
def test_single_collision(self) -> None:
|
||||
figs = [self._make_fig("figure_001"), self._make_fig("figure_001")]
|
||||
result = self._run(figs)
|
||||
assert self._ids(result) == ["figure_001", "figure_s001"]
|
||||
|
||||
def test_triple_collision(self) -> None:
|
||||
figs = [self._make_fig("figure_001"), self._make_fig("figure_001"), self._make_fig("figure_001")]
|
||||
result = self._run(figs)
|
||||
assert self._ids(result) == ["figure_001", "figure_s001", "figure_ss001"]
|
||||
|
||||
def test_multiple_independent_collisions(self) -> None:
|
||||
figs = [
|
||||
self._make_fig("figure_001"),
|
||||
self._make_fig("figure_002"),
|
||||
self._make_fig("figure_001"),
|
||||
self._make_fig("figure_003"),
|
||||
self._make_fig("figure_002"),
|
||||
]
|
||||
result = self._run(figs)
|
||||
assert self._ids(result) == ["figure_001", "figure_002", "figure_s001", "figure_003", "figure_s002"]
|
||||
|
||||
def test_mixed_buckets_deduplicated(self) -> None:
|
||||
from paperforge.worker.ocr_figures import _resolve_figure_id_collisions
|
||||
|
||||
mf = [self._make_fig("figure_001")]
|
||||
hf = [self._make_fig("figure_001")]
|
||||
af = [self._make_fig("figure_001")]
|
||||
inventory = {"matched_figures": mf, "held_figures": hf, "ambiguous_figures": af}
|
||||
_resolve_figure_id_collisions(inventory)
|
||||
assert inventory["matched_figures"][0]["figure_id"] == "figure_001"
|
||||
assert inventory["held_figures"][0]["figure_id"] == "figure_s001"
|
||||
assert inventory["ambiguous_figures"][0]["figure_id"] == "figure_ss001"
|
||||
|
||||
def test_empty_figure_id(self) -> None:
|
||||
figs = [self._make_fig(""), self._make_fig("")]
|
||||
result = self._run(figs)
|
||||
assert self._ids(result) == ["", ""]
|
||||
|
||||
def test_no_figure_id_field(self) -> None:
|
||||
figs: list[dict] = [{"page": 1}, {"page": 2}]
|
||||
result = self._run(figs)
|
||||
assert all(f.get("figure_id", "") == "" for f in result)
|
||||
|
||||
def test_non_figure_prefixes_untouched(self) -> None:
|
||||
figs = [
|
||||
self._make_fig("held_figure_001"),
|
||||
self._make_fig("held_figure_002"),
|
||||
self._make_fig("cluster_001"),
|
||||
]
|
||||
result = self._run(figs)
|
||||
assert self._ids(result) == ["held_figure_001", "held_figure_002", "cluster_001"]
|
||||
|
||||
def test_held_figure_prefix_collision(self) -> None:
|
||||
figs = [
|
||||
self._make_fig("held_figure_001"),
|
||||
self._make_fig("held_figure_001"),
|
||||
]
|
||||
result = self._run(figs)
|
||||
assert self._ids(result) == ["held_figure_001", "figure_sheld_figure_001"]
|
||||
|
||||
def test_realistic_body_plus_supplementary(self) -> None:
|
||||
figs = [
|
||||
self._make_fig("figure_001", page=6, caption="Figure 1: Main body figure"),
|
||||
self._make_fig("figure_002", page=9, caption="Figure 2: Another body figure"),
|
||||
self._make_fig("figure_003", page=11, caption="Figure 3: Histology results"),
|
||||
self._make_fig("figure_004", page=15, caption="Figure 4: Osteogenesis results"),
|
||||
self._make_fig("figure_001", page=37, caption="Figure S.1: Supplementary confocal"),
|
||||
self._make_fig("figure_002", page=38, caption="Figure S.2: Cell number data"),
|
||||
self._make_fig("figure_003", page=39, caption="Figure S.3: Osteogenesis confocal"),
|
||||
self._make_fig("figure_004", page=40, caption="Figure S.4: Cell number data"),
|
||||
]
|
||||
result = self._run(figs)
|
||||
assert self._ids(result) == [
|
||||
"figure_001", "figure_002", "figure_003", "figure_004",
|
||||
"figure_s001", "figure_s002", "figure_s003", "figure_s004",
|
||||
]
|
||||
# Verify originals are untouched
|
||||
assert len({f["figure_id"] for f in result}) == 8 # all unique
|
||||
|
||||
def test_promoted_sequence_matches_also_resolved(self) -> None:
|
||||
# Sequence-promoted figures use _format_figure_id(ns, fn) which can collide
|
||||
figs = [
|
||||
self._make_fig("figure_001", strict_status="sequence_match"),
|
||||
self._make_fig("figure_001", strict_status="matched"),
|
||||
]
|
||||
result = self._run(figs)
|
||||
assert self._ids(result) == ["figure_001", "figure_s001"]
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue