From 199aed224abf42af608692d0d44604ad893e54b0 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 8 May 2026 22:12:37 +0000 Subject: [PATCH] fix(docling): drop +1 page-number shift; prov.page_no is already 1-indexed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DoclingBackend.`_page_1_indexed_from_item` was treating `ProvenanceItem.page_no` as 0-indexed and adding +1 on top. In docling 2.92 the field is already 1-indexed (it is used as-is to index `DoclingDocument.pages`, e.g. `doc.pages[prov.page_no]` and `doc.pages[1]` for a single-page document). Effect of the bug: every `Block.page` value emitted by the docling backend was off by one. A block on the first page reported `page=2`, the last page reported one past the end, etc. Downstream consumers (RAG citations, page-anchored UIs, search snippets, the `bigos parse` JSON output) were therefore pointing users to the wrong page. The existing per-block tests only asserted block-kind counts, not page numbers, so the regression slipped through review with the initial docling integration. Reproducer (1-page PDF): doc = await DoclingBackend().run(src) {b.page for b in doc.blocks if b.page is not None} # was {2}, now {1} Fix: pass the docling page number through unchanged after coercion, treat `< 1` as unknown, and document the convention so future edits do not re-introduce the shift. Tests: a focused unit test covers the helper for the in-range, out-of-range, missing-prov and bad-value paths; a slow end-to-end test runs the docling backend against the existing single-page fixture and asserts every block reports `page == 1`. Co-authored-by: Bartłomiej Rosa --- src/bigos/backends/docling.py | 11 +++++++++- tests/test_docling_backend.py | 39 +++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/src/bigos/backends/docling.py b/src/bigos/backends/docling.py index aaed7b6..27658c2 100644 --- a/src/bigos/backends/docling.py +++ b/src/bigos/backends/docling.py @@ -65,6 +65,13 @@ def uri_to_path(uri: str) -> Path: def _page_1_indexed_from_item(item: Any) -> int | None: + """Return the (1-indexed) page number of a DocItem, or None if unknown. + + Docling's ``ProvenanceItem.page_no`` is already 1-indexed: it matches the + keys of ``DoclingDocument.pages`` (e.g. ``doc.pages[prov.page_no]``), so we + must NOT add +1 on top — doing so would shift every block's ``page`` by one + and cause RAG citations to point at the wrong page. + """ prov = getattr(item, "prov", None) or [] if not prov: return None @@ -76,7 +83,9 @@ def _page_1_indexed_from_item(item: Any) -> int | None: n = int(page_no) except (TypeError, ValueError): return None - return n + 1 + if n < 1: + return None + return n def _safe_model_dict(obj: Any) -> Any: diff --git a/tests/test_docling_backend.py b/tests/test_docling_backend.py index e6adc66..dbcd223 100644 --- a/tests/test_docling_backend.py +++ b/tests/test_docling_backend.py @@ -116,3 +116,42 @@ def test_cache_key_version_differs_with_vlm() -> None: a = DoclingBackend() b = DoclingBackend(enable_vlm=True) assert a.version != b.version + + +def test_page_helper_passes_through_1_indexed() -> None: + """Docling's ProvenanceItem.page_no is already 1-indexed (matches DoclingDocument.pages). + + Regression test for an off-by-one where the helper added +1 on top, causing + every block to report its page one higher than the true page number — which + would point RAG citations at the wrong page (or off the end of the document). + """ + from types import SimpleNamespace + + from bigos.backends.docling import _page_1_indexed_from_item + + item = SimpleNamespace(prov=[SimpleNamespace(page_no=1)]) + assert _page_1_indexed_from_item(item) == 1 + + item5 = SimpleNamespace(prov=[SimpleNamespace(page_no=5)]) + assert _page_1_indexed_from_item(item5) == 5 + + no_prov = SimpleNamespace(prov=[]) + assert _page_1_indexed_from_item(no_prov) is None + + bad_value = SimpleNamespace(prov=[SimpleNamespace(page_no="not-a-number")]) + assert _page_1_indexed_from_item(bad_value) is None + + +@pytest.mark.slow +async def test_parse_simple_text_page_number_is_one(simple_text_pdf: Path) -> None: + """Single-page PDF must report ``page=1`` for its blocks (not 2).""" + backend = DoclingBackend() + src = Source( + uri=simple_text_pdf.as_uri(), + mime_type="application/pdf", + sha256=sha256_file(simple_text_pdf), + ) + doc = await backend.run(src) + page_values = {b.page for b in doc.blocks if b.page is not None} + assert page_values, "expected at least one block with a page number" + assert page_values == {1}, f"expected all blocks on page 1, got {sorted(page_values)}"