diff --git a/nextcloud_mcp_server/document_processors/pymupdf.py b/nextcloud_mcp_server/document_processors/pymupdf.py index f4c189865..c5547ad3f 100644 --- a/nextcloud_mcp_server/document_processors/pymupdf.py +++ b/nextcloud_mcp_server/document_processors/pymupdf.py @@ -2,6 +2,7 @@ import logging import pathlib +import re import tempfile from collections.abc import Awaitable, Callable from typing import Any, Optional @@ -29,6 +30,47 @@ # note attachment, a deck card) and so has no path to name. _UNNAMED = "" +# A markdown table row cannot contain a newline, so pymupdf4llm encodes every +# line wrap *inside* a cell as a literal
. That is a rendering artifact of +# how wide the column happened to be, not content -- but it lands in the +# indexed text glued to the words on either side, so a cell reading +# "ISO 27001" is embedded as "ISO
27001" and no search for "ISO 27001" can +# match it. Narrow columns wrap constantly, so a form or questionnaire loses a +# large share of its searchable phrases this way. +# +# One run of adjacent breaks, with the spaces around it, becomes one space. A +# single substitution rather than "replace each tag, then collapse the runs": +# collapsing afterwards operates on the whole line, so a legitimate double +# space in an unrelated cell of the same row would be eaten as collateral. This +# form can only ever rewrite the text it matched. +# +# The leading space is `?`, not `*`, deliberately. With `[ \t]*` the engine can +# consume a whole run of spaces, fail to find a `
` after it, then retry one +# character shorter -- quadratic in the length of any space run on the line, +# and rendered tables are full of padding runs (python:S8786). One optional +# character cannot backtrack, and one is all the common `word
word` case +# needs; a wider run before a break survives as-is, which is the safer miss. +_TABLE_CELL_BREAK_RE = re.compile(r"[ \t]?(?:[ \t]*)+", re.IGNORECASE) + + +def _unwrap_table_cell_breaks(text: str) -> str: + """Turn a table cell's line-wrap markers back into spaces. + + Scoped to table rows rather than applied to the whole page: only pymupdf4llm's + table renderer emits these, and a document that legitimately discusses the + ``
`` tag in prose should keep saying so. + """ + # One scan to skip the split/join on the overwhelmingly common page that has + # no table at all. The regex rather than a substring test so the check + # cannot disagree with the substitution about what counts as the tag. + if not _TABLE_CELL_BREAK_RE.search(text): + return text + lines = text.split("\n") + for i, line in enumerate(lines): + if line.lstrip().startswith("|"): + lines[i] = _TABLE_CELL_BREAK_RE.sub(" ", line) + return "\n".join(lines) + def _record_parse_mode( metadata: dict[str, Any], page_count: int, settings: Any @@ -193,7 +235,10 @@ def _build_text_and_metadata( page_boundaries: list[dict[str, Any]] = [] current_offset = 0 for chunk in page_chunks: - text = chunk.get("text", "") + # Before the offsets are taken, not after: page_boundaries index + # into this text exactly, so rewriting it afterwards would slide + # every highlight off its words. + text = _unwrap_table_cell_breaks(chunk.get("text", "")) # 1-based, from pymupdf4llm's classic extractor (the worker forces # it via use_layout(False); layout mode would name this # ``page_number`` instead). The ``page`` key written below is *our* diff --git a/tests/unit/test_pymupdf_table_breaks.py b/tests/unit/test_pymupdf_table_breaks.py new file mode 100644 index 000000000..18554d6d2 --- /dev/null +++ b/tests/unit/test_pymupdf_table_breaks.py @@ -0,0 +1,132 @@ +"""Table cells keep their words searchable across a rendered line wrap.""" + +import time + +import pytest + +from nextcloud_mcp_server.document_processors.pymupdf import ( + PyMuPDFProcessor, + _unwrap_table_cell_breaks, +) + +pytestmark = pytest.mark.unit + + +def test_break_inside_a_cell_becomes_a_space(): + """ "ISO
27001" is unfindable by anyone searching for "ISO 27001".""" + row = "|Certifications|Do you
hold ISO
27001?|Yes|" + + assert _unwrap_table_cell_breaks(row) == ( + "|Certifications|Do you hold ISO 27001?|Yes|" + ) + + +@pytest.mark.parametrize("tag", ["
", "
", "
", "
", "
"]) +def test_every_spelling_of_the_tag_is_handled(tag): + assert _unwrap_table_cell_breaks(f"| a{tag}b |") == "| a b |" + + +def test_consecutive_breaks_collapse_to_one_space(): + """Each tag substitutes independently, so a run would otherwise remain.""" + assert _unwrap_table_cell_breaks("| a

b |") == "| a b |" + + +def test_space_collapsing_is_confined_to_table_rows(): + """Prose keeps its own spacing -- only the rewritten rows are normalised.""" + text = "Indented prose keeps spacing.\n\n| a

b |" + + assert _unwrap_table_cell_breaks(text) == ( + "Indented prose keeps spacing.\n\n| a b |" + ) + + +def test_an_unrelated_cell_in_the_same_row_keeps_its_double_space(): + """The collapse must not reach past the break it is repairing. + + Substituting each tag and then collapsing runs operated on the whole line, + so a legitimate double space in a different cell of the same row was eaten + as collateral. + """ + row = "| kept spacing | a

b | more spacing |" + + assert _unwrap_table_cell_breaks(row) == ("| kept spacing | a b | more spacing |") + + +def test_spaces_around_a_break_are_absorbed_into_the_one_space(): + assert _unwrap_table_cell_breaks("| a
b |") == "| a b |" + + +def test_a_long_space_run_without_a_break_is_not_quadratic(): + """A padded row must not make the regex backtrack over its whole run. + + A `[ \\t]*` prefix lets the engine eat a space run, fail to find a `
`, + then retry one character shorter -- quadratic in the run length, and + rendered tables are mostly padding (python:S8786). + """ + padded = "| " + " " * 20000 + "|\n| a
b |" + + start = time.perf_counter() + result = _unwrap_table_cell_breaks(padded) + elapsed = time.perf_counter() - start + + assert result.endswith("| a b |") + assert " " * 20000 in result, "the padding itself must be left alone" + # Linear scanning finishes in microseconds; the quadratic form took seconds + # on this input. A whole second is a generous ceiling that still fails loudly. + assert elapsed < 1.0, f"took {elapsed:.2f}s -- regex is backtracking" + + +def test_a_row_without_a_break_keeps_its_own_double_spaces(): + """The early-out is per page, so a sibling row must not be rewritten. + + One
anywhere on the page used to send every other table row through + the space collapse, silently eating double spaces the document contains. + """ + text = "| kept spacing | here |\n| a

b |" + + assert _unwrap_table_cell_breaks(text) == "| kept spacing | here |\n| a b |" + + +def test_a_page_with_no_breaks_at_all_is_untouched(): + text = "| kept spacing |\n| and here |" + + assert _unwrap_table_cell_breaks(text) is text + + +def test_prose_outside_a_table_is_left_alone(): + """A document explaining HTML should keep saying
.""" + text = "The
tag inserts a line break.\n\n| a
b |" + + assert _unwrap_table_cell_breaks(text) == ( + "The
tag inserts a line break.\n\n| a b |" + ) + + +def test_text_without_breaks_is_returned_unchanged(): + text = "# Heading\n\n| a | b |\n" + + assert _unwrap_table_cell_breaks(text) is text + + +def test_page_boundaries_match_the_rewritten_text(): + """The offsets are taken after the rewrite, so highlights stay on the words. + + Rewriting the text after measuring would leave every offset past the first + table pointing several characters too far right. + """ + processor = PyMuPDFProcessor(extract_images=False) + metadata: dict = {} + chunks = [ + {"text": "| a
b |\n", "metadata": {"page": 1}}, + {"text": "second page\n", "metadata": {"page": 2}}, + ] + + text = processor._build_text_and_metadata(chunks, None, metadata) + + assert text == "| a b |\nsecond page\n" + boundaries = metadata["page_boundaries"] + assert boundaries[0]["end_offset"] == len("| a b |\n") + assert boundaries[1]["start_offset"] == boundaries[0]["end_offset"] + assert boundaries[-1]["end_offset"] == len(text) + for span in boundaries: + assert text[span["start_offset"] : span["end_offset"]]