mirror of
https://github.com/VectifyAI/PageIndex.git
synced 2026-07-24 21:41:04 +02:00
fix: harden indexing and query paths against malformed inputs
- process_toc_with_page_numbers: an unresolvable page offset (no
TOC/body title matches) returned None and crashed with TypeError;
now returns items without physical_index so meta_processor falls
back to the no-page-number mode
- process_none_page_numbers: tolerate malformed add_page_number_to_toc
output ({}, non-dict items, non-numeric physical_index tags) instead
of KeyError/AttributeError/ValueError
- validate_and_truncate_physical_indices: invalidate non-int
physical_index values instead of raising on str > int
- wrap_with_doc_context: doc_name=None no longer crashes _defang_delimiters
- build_tree_from_levels: level=0 is a valid level; stop coercing it
to 1 via falsy-or, which flattened one level of tree depth
This commit is contained in:
parent
929b3dfc7c
commit
6d42559284
6 changed files with 97 additions and 5 deletions
|
|
@ -65,7 +65,7 @@ def wrap_with_doc_context(docs: list[dict], question: str) -> str:
|
|||
"""
|
||||
lines = []
|
||||
for d in docs:
|
||||
line = f"- {_defang_delimiters(str(d['doc_id']))}: {_defang_delimiters(d.get('doc_name', ''))}"
|
||||
line = f"- {_defang_delimiters(str(d['doc_id']))}: {_defang_delimiters(d.get('doc_name') or '')}"
|
||||
desc = d.get("doc_description") or ""
|
||||
if desc:
|
||||
line += f" — {_defang_delimiters(desc)}"
|
||||
|
|
|
|||
|
|
@ -642,6 +642,11 @@ def process_toc_with_page_numbers(toc_content, toc_page_list, page_list, toc_che
|
|||
offset = calculate_page_offset(matching_pairs)
|
||||
logger.info(f'offset: {offset}')
|
||||
|
||||
if offset is None:
|
||||
# no printed→physical anchor: return items without physical_index so
|
||||
# meta_processor's accuracy check falls back to the no-page-number mode
|
||||
return toc_with_page_number
|
||||
|
||||
toc_with_page_number = add_page_offset_to_toc_json(toc_with_page_number, offset)
|
||||
logger.info(f'toc_with_page_number: {toc_with_page_number}')
|
||||
|
||||
|
|
@ -684,8 +689,13 @@ def process_none_page_numbers(toc_items, page_list, start_index=1, model=None):
|
|||
item_copy = copy.deepcopy(item)
|
||||
item_copy.pop('page', None)
|
||||
result = add_page_number_to_toc(page_contents, item_copy, model)
|
||||
if isinstance(result[0]['physical_index'], str) and result[0]['physical_index'].startswith('<physical_index'):
|
||||
item['physical_index'] = int(result[0]['physical_index'].split('_')[-1].rstrip('>').strip())
|
||||
first = result[0] if isinstance(result, list) and result and isinstance(result[0], dict) else {}
|
||||
physical_index = first.get('physical_index')
|
||||
if isinstance(physical_index, str) and physical_index.startswith('<physical_index'):
|
||||
try:
|
||||
item['physical_index'] = int(physical_index.split('_')[-1].rstrip('>').strip())
|
||||
except ValueError:
|
||||
continue
|
||||
item.pop('page', None)
|
||||
|
||||
return toc_items
|
||||
|
|
@ -1179,7 +1189,9 @@ def validate_and_truncate_physical_indices(toc_with_page_number, page_list_lengt
|
|||
for i, item in enumerate(toc_with_page_number):
|
||||
if item.get('physical_index') is not None:
|
||||
original_index = item['physical_index']
|
||||
if original_index > max_allowed_page:
|
||||
# non-int (e.g. a bare-number string the converter didn't coerce)
|
||||
# is invalid the same way an out-of-range index is
|
||||
if not isinstance(original_index, int) or original_index > max_allowed_page:
|
||||
item['physical_index'] = None
|
||||
truncated_items.append({
|
||||
'title': item.get('title', 'Unknown'),
|
||||
|
|
|
|||
|
|
@ -30,7 +30,7 @@ def build_tree_from_levels(nodes: list[ContentNode]) -> list[dict]:
|
|||
"line_num": node.index,
|
||||
"nodes": [],
|
||||
}
|
||||
current_level = node.level or 1
|
||||
current_level = 1 if node.level is None else node.level
|
||||
|
||||
while stack and stack[-1][1] >= current_level:
|
||||
stack.pop()
|
||||
|
|
|
|||
|
|
@ -172,6 +172,13 @@ def test_wrap_with_doc_context_multi(populated_backend):
|
|||
assert "User question: compare them" in wrapped
|
||||
|
||||
|
||||
def test_wrap_with_doc_context_none_doc_name():
|
||||
from pageindex.agent import wrap_with_doc_context
|
||||
docs = [{"doc_id": "d1", "doc_name": None, "doc_description": None}]
|
||||
wrapped = wrap_with_doc_context(docs, "q?")
|
||||
assert "- d1:" in wrapped
|
||||
|
||||
|
||||
def test_scoped_docs_raises_on_missing(populated_backend):
|
||||
with pytest.raises(DocumentNotFoundError, match="nonexistent"):
|
||||
populated_backend._scoped_docs("papers", ["d1", "nonexistent"])
|
||||
|
|
|
|||
|
|
@ -69,6 +69,17 @@ def test_build_tree_from_levels_single_level():
|
|||
assert tree[1]["title"] == "B"
|
||||
|
||||
|
||||
def test_build_tree_from_levels_zero_based_levels():
|
||||
nodes = [
|
||||
ContentNode(content="c", tokens=5, title="Chapter", index=1, level=0),
|
||||
ContentNode(content="s", tokens=5, title="Section", index=2, level=1),
|
||||
]
|
||||
tree = build_tree_from_levels(nodes)
|
||||
assert len(tree) == 1
|
||||
assert tree[0]["title"] == "Chapter"
|
||||
assert [n["title"] for n in tree[0]["nodes"]] == ["Section"]
|
||||
|
||||
|
||||
def test_build_tree_from_levels_deep_nesting():
|
||||
nodes = [
|
||||
ContentNode(content="h1", tokens=5, title="H1", index=1, level=1),
|
||||
|
|
|
|||
62
tests/test_review_fixes_4.py
Normal file
62
tests/test_review_fixes_4.py
Normal file
|
|
@ -0,0 +1,62 @@
|
|||
"""Regression tests for the PR #272 max-review findings #2-#4 (indexing crashes)."""
|
||||
import logging
|
||||
|
||||
from pageindex.index import page_index as pi
|
||||
|
||||
|
||||
# ── #2: unresolvable page offset must fall back, not TypeError ───────────────
|
||||
|
||||
def test_toc_with_page_numbers_falls_back_when_offset_unresolvable(monkeypatch):
|
||||
toc = [{"structure": "1", "title": "Intro", "page": 5}]
|
||||
monkeypatch.setattr(pi, "toc_transformer",
|
||||
lambda content, model=None: [dict(item) for item in toc])
|
||||
# no title matches between transformed TOC and physical-index extraction
|
||||
monkeypatch.setattr(pi, "toc_index_extractor", lambda t, c, model=None: [])
|
||||
|
||||
def _fail(*a, **k):
|
||||
raise AssertionError("no per-item LLM lookups when the offset is unresolvable")
|
||||
monkeypatch.setattr(pi, "add_page_number_to_toc", _fail)
|
||||
|
||||
result = pi.process_toc_with_page_numbers(
|
||||
"toc text", [0], [("page one", 5), ("page two", 5)],
|
||||
toc_check_page_num=1, model=None, logger=logging.getLogger("test"),
|
||||
)
|
||||
# items come back without physical_index so meta_processor cascades to the
|
||||
# no-page-number mode
|
||||
assert all(item.get("physical_index") is None for item in result)
|
||||
|
||||
|
||||
# ── #3: empty/unparseable LLM result must not KeyError ───────────────────────
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
@pytest.mark.parametrize("llm_result", [
|
||||
{}, # unparseable → extract_json {}
|
||||
["garbage string"], # list of non-dicts
|
||||
[{"physical_index": "<physical_index_abc>"}], # tag with a non-numeric index
|
||||
[{"title": "B"}], # dict missing physical_index
|
||||
])
|
||||
def test_process_none_page_numbers_tolerates_malformed_llm_result(monkeypatch, llm_result):
|
||||
monkeypatch.setattr(pi, "add_page_number_to_toc", lambda *a, **k: llm_result)
|
||||
items = [
|
||||
{"title": "A", "physical_index": 1},
|
||||
{"title": "B", "page": 2},
|
||||
{"title": "C", "physical_index": 3},
|
||||
]
|
||||
out = pi.process_none_page_numbers(items, [("p1", 5), ("p2", 5), ("p3", 5)])
|
||||
assert out[1].get("physical_index") is None
|
||||
|
||||
|
||||
# ── #4: non-int physical_index must be invalidated, not TypeError ────────────
|
||||
|
||||
def test_validate_and_truncate_tolerates_non_int_physical_index():
|
||||
items = [
|
||||
{"title": "A", "physical_index": "5"},
|
||||
{"title": "B", "physical_index": 3},
|
||||
{"title": "C", "physical_index": 99},
|
||||
]
|
||||
out = pi.validate_and_truncate_physical_indices(items, 20)
|
||||
assert out[0]["physical_index"] is None
|
||||
assert out[1]["physical_index"] == 3
|
||||
assert out[2]["physical_index"] is None
|
||||
Loading…
Add table
Add a link
Reference in a new issue