mirror of
https://github.com/VectifyAI/PageIndex.git
synced 2026-07-15 21:11:05 +02:00
4e6a13576d
Addresses items 9-13 and a/b/c/f from the max-effort review of PR #272. - pdf.py: image colorspace check was `pix.n > 4`, which treats CMYK-without- alpha (n==4, same as RGBA) as not needing RGB conversion; pix.save() as .png then raises "unsupported colorspace", silently dropped by the surrounding except. Fixed to `pix.n - pix.alpha >= 4` (correctly converts CMYK, leaves RGBA untouched). - pipeline.py: detect_strategy([]) (an empty/whitespace-only source file) returned "content_based", routing into the PDF-oriented TOC-detection pipeline -- wasting a real LLM call before raising IndexingError. Empty node lists now route to level_based, whose build_tree_from_levels([]) returns an empty structure instantly with zero LLM calls. - page_index.py (shim): pageindex/__init__.py binds the canonical `page_index` function as the package attribute, but this file is ALSO a real submodule of the same name -- importing it anywhere (import machinery, unconditional) overwrites that attribute with the module object, breaking `from pageindex import page_index; page_index(x)` for the rest of the process. Made the shim module itself callable (delegates to the real function via a ModuleType subclass), so whichever object ends up in that slot is callable regardless of import order. - storage/sqlite.py: create_collection let a raw sqlite3.IntegrityError escape on a duplicate name (new CollectionAlreadyExistsError); the collections table's CHECK constraint only validated the name's first character (GLOB '*' is a wildcard, not a regex quantifier over the preceding class) -- fixed to validate the whole string, and SQLiteStorage now also validates in Python (it's a public StorageEngine usable directly, bypassing LocalBackend's own check). - tests/test_review_fixes_2.py: two tests used a ContentNode with no `level` set, so build_index took the content_based path and made real (retried, slow, and -- with a valid key -- billable) LLM calls instead of testing the text-stripping logic they claimed to. Mocked out _content_based_pipeline. - retrieve.py: _parse_pages/_get_pdf_page_content were independent copies of the canonical parse_pages/get_pdf_page_content that had already drifted (missing the p>=1 filter and 1000-page DoS cap) -- delegate to canonical now, so the legacy pageindex.get_page_content path can't silently regress again. - parser/markdown.py: a leading UTF-8 BOM broke first-header detection (not whitespace, .strip() doesn't remove it) -- decode utf-8-sig. Only backtick fences were recognized as code blocks, so a '#'-prefixed line inside a ~~~-fenced block (valid CommonMark) was misparsed as a heading -- recognize both fence styles. - run_pageindex.py: --if-thinning wasn't migrated to the bare-flag + legacy-yes/no convention the other four --if-add-* flags got; bare usage raised an argparse error and it never went through the shared coercion. - types.py: DocumentDetail's `structure` field was inside the class's total=False body, so TypedDict rules made it optional even though every backend always populates it. Split into a required base class. Adds regression tests for all of the above. Full suite: 244 passed, 2 skipped (one pre-existing, unrelated flaky cloud-streaming test). Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
76 lines
3.1 KiB
Python
76 lines
3.1 KiB
Python
"""Markdown page-content selection must return exactly the requested lines,
|
|
mirroring the PDF path — not the whole [min, max] range (PR #272 review / #280)."""
|
|
|
|
|
|
def _md_structure():
|
|
# line_num 40 sits *between* 5 and 100 but is NOT requested below.
|
|
return [
|
|
{"line_num": 5, "text": "line five", "nodes": [
|
|
{"line_num": 40, "text": "line forty (should be excluded)", "nodes": []},
|
|
]},
|
|
{"line_num": 100, "text": "line hundred", "nodes": []},
|
|
{"line_num": 101, "text": "line 101", "nodes": []},
|
|
]
|
|
|
|
|
|
def test_get_md_page_content_returns_only_requested_lines():
|
|
from pageindex.index.utils import get_md_page_content
|
|
|
|
out = get_md_page_content(_md_structure(), [5, 100])
|
|
# exactly the two requested lines — not 5, 40, 100 (the old range behavior)
|
|
assert [r["page"] for r in out] == [5, 100]
|
|
assert all("forty" not in r["content"] for r in out)
|
|
|
|
|
|
def test_get_md_page_content_empty_spec():
|
|
from pageindex.index.utils import get_md_page_content
|
|
|
|
assert get_md_page_content(_md_structure(), []) == []
|
|
|
|
|
|
def test_retrieve_md_page_content_returns_only_requested_lines():
|
|
# The legacy retrieve path has its own copy of the same logic.
|
|
from pageindex.retrieve import _get_md_page_content
|
|
|
|
out = _get_md_page_content({"structure": _md_structure()}, [5, 100])
|
|
assert [r["page"] for r in out] == [5, 100]
|
|
|
|
|
|
def test_retrieve_parse_pages_delegates_to_canonical_and_enforces_dos_cap():
|
|
"""retrieve._parse_pages used to be an independent copy that lacked the
|
|
canonical parse_pages' p>=1 filter and 1000-page cap — a caller of the
|
|
legacy pageindex.get_page_content could bypass the DoS guard the SDK path
|
|
enforces. Now it's a one-line delegate, so they can't drift again."""
|
|
from pageindex.retrieve import _parse_pages
|
|
from pageindex.index.utils import parse_pages
|
|
import pytest
|
|
|
|
assert _parse_pages("5-7") == parse_pages("5-7") == [5, 6, 7]
|
|
with pytest.raises(ValueError, match="too large"):
|
|
_parse_pages("1-99999999")
|
|
|
|
|
|
def test_retrieve_get_pdf_page_content_falls_back_to_canonical(tmp_path, monkeypatch):
|
|
"""When no cached 'pages' are present, the file-read fallback must
|
|
delegate to the canonical get_pdf_page_content instead of re-implementing
|
|
PDF text extraction inline (a second, independently-maintained copy)."""
|
|
from pageindex.retrieve import _get_pdf_page_content
|
|
import pageindex.retrieve as retrieve_mod
|
|
|
|
calls = []
|
|
monkeypatch.setattr(
|
|
retrieve_mod, "get_pdf_page_content",
|
|
lambda path, page_nums: calls.append((path, page_nums)) or [{"page": 1, "content": "x"}],
|
|
)
|
|
result = _get_pdf_page_content({"path": "/fake/doc.pdf"}, [1])
|
|
assert calls == [("/fake/doc.pdf", [1])]
|
|
assert result == [{"page": 1, "content": "x"}]
|
|
|
|
|
|
def test_retrieve_get_pdf_page_content_prefers_cache_over_file():
|
|
from pageindex.retrieve import _get_pdf_page_content
|
|
|
|
doc_info = {"path": "/should/not/be/opened.pdf",
|
|
"pages": [{"page": 1, "content": "cached one"}, {"page": 2, "content": "cached two"}]}
|
|
result = _get_pdf_page_content(doc_info, [2])
|
|
assert result == [{"page": 2, "content": "cached two"}]
|