diff --git a/pageindex/agent.py b/pageindex/agent.py index c6be939..4a1a3d0 100644 --- a/pageindex/agent.py +++ b/pageindex/agent.py @@ -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)}" diff --git a/pageindex/index/page_index.py b/pageindex/index/page_index.py index f284243..9d0907f 100644 --- a/pageindex/index/page_index.py +++ b/pageindex/index/page_index.py @@ -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('').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('').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'), diff --git a/pageindex/index/pipeline.py b/pageindex/index/pipeline.py index f91a587..adcbfeb 100644 --- a/pageindex/index/pipeline.py +++ b/pageindex/index/pipeline.py @@ -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() diff --git a/tests/test_local_backend.py b/tests/test_local_backend.py index 6bbb346..c7ce2fb 100644 --- a/tests/test_local_backend.py +++ b/tests/test_local_backend.py @@ -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"]) diff --git a/tests/test_pipeline.py b/tests/test_pipeline.py index 98a66ba..e2412d0 100644 --- a/tests/test_pipeline.py +++ b/tests/test_pipeline.py @@ -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), diff --git a/tests/test_review_fixes_4.py b/tests/test_review_fixes_4.py new file mode 100644 index 0000000..11ca503 --- /dev/null +++ b/tests/test_review_fixes_4.py @@ -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": ""}], # 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