From e999ea55dd82a75df447fc3ef2dfe5d54907c47f Mon Sep 17 00:00:00 2001 From: mountain Date: Sat, 13 Jun 2026 11:46:23 +0800 Subject: [PATCH 01/11] refactor(converter): extract resolve_doc_name_from_key for synthetic keys --- openkb/converter.py | 21 ++++++++++++++++++++- tests/test_converter.py | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/openkb/converter.py b/openkb/converter.py index 9ebac762..4b9246ec 100644 --- a/openkb/converter.py +++ b/openkb/converter.py @@ -98,7 +98,26 @@ def resolve_doc_name(src: Path, kb_dir: Path, registry: HashRegistry) -> str: registry.add(file_hash, meta) # backfill + persist return name - candidate = _sanitize_stem(src.stem) + return resolve_doc_name_from_key(src.stem, path_key, registry) + + +def resolve_doc_name_from_key(stem: str, path_key: str, registry: HashRegistry) -> str: + """Collision-resistant wiki name for a synthetic identity ``path_key``. + + Same rules as :func:`resolve_doc_name` minus the legacy-by-stem + backfill (a filesystem-migration concern that must not fire for sources + with no real path, e.g. cloud imports). A source already registered + under ``path_key`` keeps its stored ``doc_name``; otherwise the + sanitized ``stem`` is used, with a deterministic + ``-{sha256(path_key)[:8]}`` suffix when another document owns it. + """ + known = registry.get_by_path(path_key) + if known is not None: + stored = known.get("doc_name") or Path(known.get("name", "")).stem + if stored: + return stored + + candidate = _sanitize_stem(stem) if _name_taken(candidate, registry): digest = hashlib.sha256(path_key.encode("utf-8")).hexdigest()[:_SUFFIX_LEN] return f"{candidate}-{digest}" diff --git a/tests/test_converter.py b/tests/test_converter.py index 5ee2f5df..e7c95d4d 100644 --- a/tests/test_converter.py +++ b/tests/test_converter.py @@ -265,6 +265,44 @@ def test_unclaimed_on_disk_long_doc_json_is_adopted(self, kb_dir): assert resolve_doc_name(src, kb_dir, self._registry(kb_dir)) == "report" +# --------------------------------------------------------------------------- +# resolve_doc_name_from_key +# --------------------------------------------------------------------------- + + +def test_resolve_doc_name_from_key_clean(tmp_path): + from openkb.converter import resolve_doc_name_from_key + from openkb.state import HashRegistry + + registry = HashRegistry(tmp_path / "hashes.json") + name = resolve_doc_name_from_key("Attention Is All You Need", "pageindex-cloud:abc", registry) + assert name == "Attention-Is-All-You-Need" + + +def test_resolve_doc_name_from_key_collision_suffix(tmp_path): + import hashlib + from openkb.converter import resolve_doc_name_from_key + from openkb.state import HashRegistry + + registry = HashRegistry(tmp_path / "hashes.json") + registry.add("hash1", {"name": "paper.pdf", "doc_name": "paper"}) + + path_key = "pageindex-cloud:xyz" + name = resolve_doc_name_from_key("paper", path_key, registry) + digest = hashlib.sha256(path_key.encode("utf-8")).hexdigest()[:8] + assert name == f"paper-{digest}" + + +def test_resolve_doc_name_from_key_reuses_known_path(tmp_path): + from openkb.converter import resolve_doc_name_from_key + from openkb.state import HashRegistry + + registry = HashRegistry(tmp_path / "hashes.json") + registry.add("h", {"doc_name": "kept-name", "path": "pageindex-cloud:dup"}) + name = resolve_doc_name_from_key("whatever", "pageindex-cloud:dup", registry) + assert name == "kept-name" + + # --------------------------------------------------------------------------- # convert_document — doc_name collision handling # --------------------------------------------------------------------------- From 39722bb0a1416088b02e300064eb67a84e20e16a Mon Sep 17 00:00:00 2001 From: mountain Date: Sat, 13 Jun 2026 11:50:40 +0800 Subject: [PATCH 02/11] refactor(indexer): extract _write_long_doc_artifacts shared writer --- openkb/indexer.py | 34 ++++++++++++++++++++++++---------- tests/test_indexer.py | 14 ++++++++++++++ 2 files changed, 38 insertions(+), 10 deletions(-) diff --git a/openkb/indexer.py b/openkb/indexer.py index 00692319..e642f5df 100644 --- a/openkb/indexer.py +++ b/openkb/indexer.py @@ -86,6 +86,29 @@ def _convert_pdf_to_pages(pdf_path: Path, doc_name: str, images_dir: Path) -> li return convert_pdf_to_pages(pdf_path, doc_name, images_dir) +def _write_long_doc_artifacts( + tree: dict, pages: list[dict[str, Any]], doc_name: str, doc_id: str, kb_dir: Path +) -> Path: + """Write ``wiki/sources/.json`` + ``wiki/summaries/.md``. + + Returns the summary path. Shared by :func:`index_long_document` (local) + and :func:`import_cloud_document` (cloud) so both produce identical + artifacts. Page images, when present, are written separately by the + caller's page extractor — this helper only persists page text + summary. + """ + sources_dir = kb_dir / "wiki" / "sources" + sources_dir.mkdir(parents=True, exist_ok=True) + (sources_dir / f"{doc_name}.json").write_text( + json_mod.dumps(pages, ensure_ascii=False, indent=2), encoding="utf-8", + ) + + summaries_dir = kb_dir / "wiki" / "summaries" + summaries_dir.mkdir(parents=True, exist_ok=True) + summary_path = summaries_dir / f"{doc_name}.md" + summary_path.write_text(render_summary_md(tree, doc_name, doc_id), encoding="utf-8") + return summary_path + + def index_long_document( pdf_path: Path, kb_dir: Path, doc_name: str | None = None ) -> IndexResult: @@ -167,14 +190,5 @@ def index_long_document( if not all_pages: raise RuntimeError(f"No page content extracted for {pdf_path.name}") - (sources_dir / f"{source_name}.json").write_text( - json_mod.dumps(all_pages, ensure_ascii=False, indent=2), encoding="utf-8", - ) - - # Write wiki/summaries/ (no images, just summaries) - summaries_dir = kb_dir / "wiki" / "summaries" - summaries_dir.mkdir(parents=True, exist_ok=True) - summary_md = render_summary_md(tree, source_name, doc_id) - (summaries_dir / f"{source_name}.md").write_text(summary_md, encoding="utf-8") - + _write_long_doc_artifacts(tree, all_pages, source_name, doc_id, kb_dir) return IndexResult(doc_id=doc_id, description=description, tree=tree) diff --git a/tests/test_indexer.py b/tests/test_indexer.py index 843f6614..1a371663 100644 --- a/tests/test_indexer.py +++ b/tests/test_indexer.py @@ -255,3 +255,17 @@ def test_index_long_document_uses_explicit_doc_name(kb_dir, monkeypatch): # summary frontmatter points full_text at the doc_name artifact summary_text = (kb_dir / "wiki" / "summaries" / "original-abc12345.md").read_text(encoding="utf-8") assert "original-abc12345" in summary_text + + +def test_write_long_doc_artifacts_writes_json_and_summary(kb_dir, sample_tree): + from openkb.indexer import _write_long_doc_artifacts + + pages = [{"page": 1, "content": "Hello.", "images": []}] + summary_path = _write_long_doc_artifacts(sample_tree, pages, "my-doc", "doc-1", kb_dir) + + assert summary_path == kb_dir / "wiki" / "summaries" / "my-doc.md" + assert summary_path.exists() + json_file = kb_dir / "wiki" / "sources" / "my-doc.json" + assert json_file.exists() + assert '"content": "Hello."' in json_file.read_text(encoding="utf-8") + assert "doc_type: pageindex" in summary_path.read_text(encoding="utf-8") From fc8db5e265ad7bed9a581fac934ae591cdcabb93 Mon Sep 17 00:00:00 2001 From: mountain Date: Sat, 13 Jun 2026 11:55:04 +0800 Subject: [PATCH 03/11] feat(indexer): import existing PageIndex Cloud doc by doc_id --- openkb/indexer.py | 85 +++++++++++++++++++++++++++++++++++++++++++ tests/test_indexer.py | 59 ++++++++++++++++++++++++++++++ 2 files changed, 144 insertions(+) diff --git a/openkb/indexer.py b/openkb/indexer.py index e642f5df..59105812 100644 --- a/openkb/indexer.py +++ b/openkb/indexer.py @@ -27,6 +27,16 @@ class IndexResult: tree: dict +@dataclass +class CloudImportResult: + """Result of importing an existing PageIndex Cloud document.""" + + doc_id: str + doc_name: str # collision-resistant wiki slug + name: str # cloud display name (original filename in the cloud) + description: str + + def _normalize_page_content(raw_pages: Any) -> list[dict[str, Any]]: """Normalize PageIndex/local PDF page content into OpenKB's JSON shape.""" if not isinstance(raw_pages, list): @@ -192,3 +202,78 @@ def index_long_document( _write_long_doc_artifacts(tree, all_pages, source_name, doc_id, kb_dir) return IndexResult(doc_id=doc_id, description=description, tree=tree) + + +def _max_page_index(structure: list, default: int = 100000) -> int: + """Largest start/end page index across the (possibly nested) tree. + + Cloud tree nodes carry ``start_index``/``end_index`` page numbers; this + bounds ``get_page_content``'s page range without a local PDF. Falls back + to ``default`` (a wide range the cloud filters anyway) when no integer + indices are present. + """ + best = 0 + + def _walk(nodes: list) -> None: + nonlocal best + for node in nodes or []: + if not isinstance(node, dict): + continue + for key in ("end_index", "start_index"): + val = node.get(key) + if isinstance(val, int): + best = max(best, val) + _walk(node.get("nodes")) + + _walk(structure) + return best or default + + +def import_cloud_document(doc_id: str, kb_dir: Path, path_key: str) -> CloudImportResult: + """Import an already-indexed PageIndex Cloud document by ``doc_id``. + + Fetches structure + OCR'd page content from the cloud (no local PDF) and + writes the same wiki artifacts as :func:`index_long_document`. Requires + ``PAGEINDEX_API_KEY``. ``path_key`` is the synthetic identity key + (``pageindex-cloud:``) used to resolve a collision-resistant + wiki name. + """ + from openkb.converter import resolve_doc_name_from_key + from openkb.state import HashRegistry + + pageindex_api_key = os.environ.get("PAGEINDEX_API_KEY", "") + if not pageindex_api_key: + raise RuntimeError( + "Importing from PageIndex Cloud requires the PAGEINDEX_API_KEY " + "environment variable." + ) + + client = PageIndexClient(api_key=pageindex_api_key) + col = client.collection() + + doc = col.get_document(doc_id, include_text=True) + cloud_name: str = doc.get("doc_name") or doc_id + description: str = doc.get("doc_description", "") + structure: list = doc.get("structure", []) + + registry = HashRegistry(kb_dir / ".openkb" / "hashes.json") + stem = Path(cloud_name).stem or doc_id + doc_name = resolve_doc_name_from_key(stem, path_key, registry) + + tree = { + "doc_name": cloud_name, + "doc_description": description, + "structure": structure, + } + + page_count = _max_page_index(structure) + all_pages = _normalize_page_content(col.get_page_content(doc_id, f"1-{page_count}")) + if not all_pages: + raise RuntimeError( + f"No page content returned from PageIndex Cloud for doc_id={doc_id}" + ) + + _write_long_doc_artifacts(tree, all_pages, doc_name, doc_id, kb_dir) + return CloudImportResult( + doc_id=doc_id, doc_name=doc_name, name=cloud_name, description=description, + ) diff --git a/tests/test_indexer.py b/tests/test_indexer.py index 1a371663..faa9713c 100644 --- a/tests/test_indexer.py +++ b/tests/test_indexer.py @@ -257,6 +257,65 @@ def test_index_long_document_uses_explicit_doc_name(kb_dir, monkeypatch): assert "original-abc12345" in summary_text +class TestImportCloudDocument: + def _fake_client(self, doc_id, sample_tree, pages): + col = MagicMock() + col.get_document.return_value = { + "doc_id": doc_id, + "doc_name": "Cloud Paper.pdf", + "doc_description": sample_tree["doc_description"], + "structure": sample_tree["structure"], + } + col.get_page_content.return_value = pages + client = MagicMock() + client.collection.return_value = col + return client, col + + def test_writes_artifacts_and_returns_result(self, kb_dir, sample_tree, monkeypatch): + from openkb.indexer import CloudImportResult, import_cloud_document + + monkeypatch.setenv("PAGEINDEX_API_KEY", "test-key") + pages = [{"page": 1, "content": "Cloud page one."}] + client, col = self._fake_client("cloud-1", sample_tree, pages) + + with patch("openkb.indexer.PageIndexClient", return_value=client): + result = import_cloud_document("cloud-1", kb_dir, "pageindex-cloud:cloud-1") + + assert isinstance(result, CloudImportResult) + assert result.doc_id == "cloud-1" + assert result.name == "Cloud Paper.pdf" + assert result.doc_name == "Cloud-Paper" + assert (kb_dir / "wiki" / "sources" / "Cloud-Paper.json").exists() + assert (kb_dir / "wiki" / "summaries" / "Cloud-Paper.md").exists() + # col.add must never be called — the doc already exists in the cloud + col.add.assert_not_called() + + def test_requires_api_key(self, kb_dir, monkeypatch): + from openkb.indexer import import_cloud_document + + monkeypatch.delenv("PAGEINDEX_API_KEY", raising=False) + try: + import_cloud_document("cloud-x", kb_dir, "pageindex-cloud:cloud-x") + except RuntimeError as exc: + assert "PAGEINDEX_API_KEY" in str(exc) + else: + raise AssertionError("expected RuntimeError when API key is missing") + + def test_empty_pages_raise(self, kb_dir, sample_tree, monkeypatch): + from openkb.indexer import import_cloud_document + + monkeypatch.setenv("PAGEINDEX_API_KEY", "test-key") + client, col = self._fake_client("cloud-2", sample_tree, []) + + with patch("openkb.indexer.PageIndexClient", return_value=client): + try: + import_cloud_document("cloud-2", kb_dir, "pageindex-cloud:cloud-2") + except RuntimeError as exc: + assert "No page content" in str(exc) + else: + raise AssertionError("expected RuntimeError on empty pages") + + def test_write_long_doc_artifacts_writes_json_and_summary(kb_dir, sample_tree): from openkb.indexer import _write_long_doc_artifacts From 188baddcc596ebb72a1fba3861fac55c0c071245 Mon Sep 17 00:00:00 2001 From: mountain Date: Sat, 13 Jun 2026 12:00:49 +0800 Subject: [PATCH 04/11] feat(cli): orchestrate raw-less PageIndex Cloud import --- openkb/cli.py | 79 +++++++++++++++++++++++++++++++++++++++ tests/test_add_command.py | 75 +++++++++++++++++++++++++++++++++++++ 2 files changed, 154 insertions(+) diff --git a/openkb/cli.py b/openkb/cli.py index b963fc20..98aa436c 100644 --- a/openkb/cli.py +++ b/openkb/cli.py @@ -41,8 +41,10 @@ def filter(self, record: logging.LogRecord) -> bool: litellm.suppress_debug_info = True from dotenv import load_dotenv +from openkb.agent.compiler import compile_long_doc from openkb.config import DEFAULT_CONFIG, load_config, save_config, load_global_config, register_kb from openkb.converter import _registry_path, convert_document +from openkb.indexer import import_cloud_document from openkb.locks import atomic_write_json, atomic_write_text, kb_ingest_lock, kb_read_lock from openkb.log import append_log from openkb.schema import AGENTS_MD, INDEX_SEED, PAGE_CONTENT_DIRS @@ -386,6 +388,83 @@ def _add_single_file_locked(file_path: Path, kb_dir: Path) -> Literal["added", " return "added" +def import_from_pageindex_cloud( + doc_id: str, kb_dir: Path +) -> Literal["added", "skipped", "failed"]: + """Import an existing PageIndex Cloud document into the KB by ``doc_id``. + + Fetches structure + page content from the cloud (no local PDF), compiles + concepts, and registers a raw-less ``pageindex_cloud`` entry. Idempotent: + re-importing the same ``doc_id`` is skipped. The user's cloud corpus is + never modified. + """ + import hashlib + from openkb.state import HashRegistry + + logger = logging.getLogger(__name__) + openkb_dir = kb_dir / ".openkb" + config = load_config(openkb_dir / "config.yaml") + _setup_llm_key(kb_dir) + model: str = config.get("model", DEFAULT_CONFIG["model"]) + + path_key = f"pageindex-cloud:{doc_id}" + synthetic_hash = hashlib.sha256(path_key.encode("utf-8")).hexdigest() + + registry = HashRegistry(openkb_dir / "hashes.json") + if registry.is_known(synthetic_hash): + click.echo(f" [SKIP] Already imported from PageIndex Cloud: {doc_id}") + return "skipped" + + click.echo(f"Importing from PageIndex Cloud: {doc_id}") + try: + import_result = import_cloud_document(doc_id, kb_dir, path_key) + except Exception as exc: + click.echo(f" [ERROR] Import failed: {exc}") + logger.debug("Cloud import traceback:", exc_info=True) + return "failed" + + doc_name = import_result.doc_name + summary_path = kb_dir / "wiki" / "summaries" / f"{doc_name}.md" + click.echo(f" Compiling imported doc (doc_id={doc_id})...") + for attempt in range(2): + try: + asyncio.run( + compile_long_doc( + doc_name, summary_path, doc_id, kb_dir, model, + doc_description=import_result.description, + ) + ) + break + except Exception as exc: + if attempt == 0: + click.echo(" Retrying compilation in 2s...") + time.sleep(2) + else: + click.echo(f" [ERROR] Compilation failed: {exc}") + logger.debug("Compilation traceback:", exc_info=True) + return "failed" + + # Register the raw-less cloud entry only after successful compilation. + registry = HashRegistry(openkb_dir / "hashes.json") + meta = { + "name": import_result.name, + "doc_name": doc_name, + "type": "pageindex_cloud", + "origin": "cloud", + "path": path_key, + "source_path": _registry_path( + kb_dir / "wiki" / "sources" / f"{doc_name}.json", kb_dir + ), + "doc_id": doc_id, + } + registry.remove_by_doc_name(doc_name) + registry.add(synthetic_hash, meta) + + append_log(kb_dir / "wiki", "ingest", doc_name) + click.echo(f" [OK] {doc_name} imported from PageIndex Cloud.") + return "added" + + # --------------------------------------------------------------------------- # CLI # --------------------------------------------------------------------------- diff --git a/tests/test_add_command.py b/tests/test_add_command.py index 0199c9e2..55a5794c 100644 --- a/tests/test_add_command.py +++ b/tests/test_add_command.py @@ -203,3 +203,78 @@ def test_add_oldest_legacy_entry_converges_to_single_entry(self, tmp_path): new_entries = [m for m in hashes.values() if m.get("doc_name") == "notes"] assert len(new_entries) == 1 # …exactly one entry survives assert new_entries[0]["path"] # with path identity persisted + + +class TestImportFromPageindexCloud: + def _setup_kb(self, tmp_path): + (tmp_path / "raw").mkdir() + (tmp_path / "wiki" / "sources" / "images").mkdir(parents=True) + (tmp_path / "wiki" / "summaries").mkdir(parents=True) + (tmp_path / "wiki" / "concepts").mkdir(parents=True) + (tmp_path / "wiki" / "reports").mkdir(parents=True) + openkb_dir = tmp_path / ".openkb" + openkb_dir.mkdir() + (openkb_dir / "config.yaml").write_text("model: gpt-4o-mini\n") + (openkb_dir / "hashes.json").write_text(json.dumps({})) + return tmp_path + + def test_registers_rawless_cloud_entry(self, tmp_path): + import hashlib + from openkb.cli import import_from_pageindex_cloud + from openkb.indexer import CloudImportResult + from openkb.state import HashRegistry + + kb_dir = self._setup_kb(tmp_path) + result = CloudImportResult( + doc_id="cloud-1", doc_name="Cloud-Paper", name="Cloud Paper.pdf", + description="desc", + ) + + with patch("openkb.cli.import_cloud_document", return_value=result), \ + patch("openkb.cli.compile_long_doc", return_value=None) as mock_compile, \ + patch("openkb.cli._setup_llm_key"): + outcome = import_from_pageindex_cloud("cloud-1", kb_dir) + + assert outcome == "added" + mock_compile.assert_called_once() + registry = HashRegistry(kb_dir / ".openkb" / "hashes.json") + synthetic = hashlib.sha256(b"pageindex-cloud:cloud-1").hexdigest() + meta = registry.get(synthetic) + assert meta is not None + assert meta["type"] == "pageindex_cloud" + assert meta["origin"] == "cloud" + assert meta["doc_id"] == "cloud-1" + assert meta["path"] == "pageindex-cloud:cloud-1" + assert "raw_path" not in meta + + def test_second_import_is_skipped(self, tmp_path): + from openkb.cli import import_from_pageindex_cloud + from openkb.indexer import CloudImportResult + + kb_dir = self._setup_kb(tmp_path) + result = CloudImportResult( + doc_id="cloud-1", doc_name="Cloud-Paper", name="Cloud Paper.pdf", + description="desc", + ) + + with patch("openkb.cli.import_cloud_document", return_value=result) as mock_import, \ + patch("openkb.cli.compile_long_doc", return_value=None), \ + patch("openkb.cli._setup_llm_key"): + import_from_pageindex_cloud("cloud-1", kb_dir) + second = import_from_pageindex_cloud("cloud-1", kb_dir) + + assert second == "skipped" + assert mock_import.call_count == 1 # not fetched again + + def test_import_failure_returns_failed_and_registers_nothing(self, tmp_path): + from openkb.cli import import_from_pageindex_cloud + from openkb.state import HashRegistry + + kb_dir = self._setup_kb(tmp_path) + with patch("openkb.cli.import_cloud_document", side_effect=RuntimeError("boom")), \ + patch("openkb.cli._setup_llm_key"): + outcome = import_from_pageindex_cloud("cloud-9", kb_dir) + + assert outcome == "failed" + registry = HashRegistry(kb_dir / ".openkb" / "hashes.json") + assert registry.all_entries() == {} From cc62097036964732efe260e63491348169dfb2ae Mon Sep 17 00:00:00 2001 From: mountain Date: Sat, 13 Jun 2026 12:06:50 +0800 Subject: [PATCH 05/11] feat(cli): add --from-pageindex-cloud flag to add command --- openkb/cli.py | 25 +++++++++++++++++++++++-- tests/test_add_command.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/openkb/cli.py b/openkb/cli.py index 98aa436c..d0c80ac7 100644 --- a/openkb/cli.py +++ b/openkb/cli.py @@ -689,10 +689,15 @@ def init(model, language): @cli.command() -@click.argument("path") +@click.argument("path", required=False) +@click.option( + "--from-pageindex-cloud", "from_pageindex_cloud", default=None, metavar="DOC_ID", + help="Import an already-indexed PageIndex Cloud document by its doc-id " + "(no local file). Mutually exclusive with PATH.", +) @click.pass_context @_with_kb_lock(exclusive=True) -def add(ctx, path): +def add(ctx, path, from_pageindex_cloud): """Add a document or directory of documents at PATH to the knowledge base. PATH may be a local file, a local directory (which is walked @@ -700,12 +705,28 @@ def add(ctx, path): fetched into ``raw/`` first: PDF responses (by Content-Type and magic-byte sniff) are saved as ``.pdf``; HTML responses are run through trafilatura's main-content extractor and saved as ``.md``. + + Alternatively, pass --from-pageindex-cloud to import a document + that is already indexed in PageIndex Cloud, with no local file. Requires + the PAGEINDEX_API_KEY environment variable. """ kb_dir = _find_kb_dir(ctx.obj.get("kb_dir_override")) if kb_dir is None: click.echo("No knowledge base found. Run `openkb init` first.") return + # Cloud import path — mutually exclusive with a local/URL PATH. + if from_pageindex_cloud is not None: + if path is not None: + click.echo("Provide either PATH or --from-pageindex-cloud, not both.") + return + import_from_pageindex_cloud(from_pageindex_cloud, kb_dir) + return + + if path is None: + click.echo("Provide a PATH or use --from-pageindex-cloud .") + return + # URL ingest: download into raw/ first, then call add_single_file # explicitly so we can clean up the just-downloaded file if it # turns out to be a duplicate (registry already has its hash). diff --git a/tests/test_add_command.py b/tests/test_add_command.py index 55a5794c..35f544ef 100644 --- a/tests/test_add_command.py +++ b/tests/test_add_command.py @@ -204,6 +204,34 @@ def test_add_oldest_legacy_entry_converges_to_single_entry(self, tmp_path): assert len(new_entries) == 1 # …exactly one entry survives assert new_entries[0]["path"] # with path identity persisted + def test_add_from_pageindex_cloud_dispatches(self, tmp_path): + kb_dir = self._setup_kb(tmp_path) + runner = CliRunner() + with patch("openkb.cli.import_from_pageindex_cloud", return_value="added") as mock_imp, \ + patch("openkb.cli._find_kb_dir", return_value=kb_dir): + runner.invoke(cli, ["add", "--from-pageindex-cloud", "doc-123"]) + mock_imp.assert_called_once_with("doc-123", kb_dir) + + def test_add_rejects_path_and_cloud_together(self, tmp_path): + kb_dir = self._setup_kb(tmp_path) + doc = tmp_path / "test.md" + doc.write_text("# Hi") + runner = CliRunner() + with patch("openkb.cli.import_from_pageindex_cloud") as mock_imp, \ + patch("openkb.cli.add_single_file") as mock_add, \ + patch("openkb.cli._find_kb_dir", return_value=kb_dir): + result = runner.invoke(cli, ["add", str(doc), "--from-pageindex-cloud", "doc-1"]) + assert "not both" in result.output + mock_imp.assert_not_called() + mock_add.assert_not_called() + + def test_add_requires_path_or_cloud(self, tmp_path): + kb_dir = self._setup_kb(tmp_path) + runner = CliRunner() + with patch("openkb.cli._find_kb_dir", return_value=kb_dir): + result = runner.invoke(cli, ["add"]) + assert "Provide a PATH" in result.output + class TestImportFromPageindexCloud: def _setup_kb(self, tmp_path): From 34e7ec0d84d4ed49228f11fffa4c99d75de7d083 Mon Sep 17 00:00:00 2001 From: mountain Date: Sat, 13 Jun 2026 12:11:44 +0800 Subject: [PATCH 06/11] test(remove): cloud-imported docs never contact PageIndex --- tests/test_remove.py | 56 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/tests/test_remove.py b/tests/test_remove.py index 75daadf0..ac390732 100644 --- a/tests/test_remove.py +++ b/tests/test_remove.py @@ -1220,3 +1220,59 @@ def test_cli_remove_deletes_renamed_raw_copy(kb_dir): assert result.exit_code == 0, result.output assert not raw_file.exists() + + +# --------------------------------------------------------------------------- +# Cloud-imported docs (type=pageindex_cloud, origin=cloud) own no local raw +# file and the user's cloud corpus is their asset — remove must clean up ONLY +# local wiki artifacts and must NEVER contact PageIndex Cloud. +# --------------------------------------------------------------------------- + + +def test_remove_cloud_doc_never_touches_pageindex(tmp_path): + """A pageindex_cloud doc removes only local artifacts; the cloud is + never contacted even when a pageindex.db happens to exist.""" + import json + from unittest.mock import patch + from click.testing import CliRunner + from openkb.cli import cli + from openkb.state import HashRegistry + + # Minimal KB + (tmp_path / "raw").mkdir() + for sub in ("sources/images", "summaries", "concepts", "entities", "reports"): + (tmp_path / "wiki" / sub).mkdir(parents=True) + openkb_dir = tmp_path / ".openkb" + openkb_dir.mkdir() + (openkb_dir / "config.yaml").write_text("model: gpt-4o-mini\n") + # A stray pageindex.db to prove the cloud path is gated by type, not just + # by the DB's absence. + (openkb_dir / "pageindex.db").write_bytes(b"") + (tmp_path / "wiki" / "index.md").write_text("# Index\n") + + # Cloud artifacts + registry entry + (tmp_path / "wiki" / "summaries" / "cloud-doc.md").write_text("---\n---\n# s\n") + (tmp_path / "wiki" / "sources" / "cloud-doc.json").write_text("[]") + registry = HashRegistry(openkb_dir / "hashes.json") + registry.add("synthhash", { + "name": "Cloud Paper.pdf", + "doc_name": "cloud-doc", + "type": "pageindex_cloud", + "origin": "cloud", + "path": "pageindex-cloud:cloud-1", + "source_path": "wiki/sources/cloud-doc.json", + "doc_id": "cloud-1", + }) + + runner = CliRunner() + with patch("openkb.cli._find_kb_dir", return_value=tmp_path), \ + patch("pageindex.PageIndexClient") as mock_client: + result = runner.invoke(cli, ["remove", "cloud-doc", "--yes"]) + + assert result.exit_code == 0, result.output + # The cloud client must never be constructed. + mock_client.assert_not_called() + # Local artifacts are gone and the registry entry is removed. + assert not (tmp_path / "wiki" / "summaries" / "cloud-doc.md").exists() + assert not (tmp_path / "wiki" / "sources" / "cloud-doc.json").exists() + assert HashRegistry(openkb_dir / "hashes.json").get("synthhash") is None From 127e75eef318191f0fda22ccc299366e2cc12011 Mon Sep 17 00:00:00 2001 From: mountain Date: Sat, 13 Jun 2026 12:15:35 +0800 Subject: [PATCH 07/11] chore(deps): track VectifyAI/PageIndex@dev for cloud import APIs --- pyproject.toml | 5 ++++- uv.lock | 11 +++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 026dea23..123faf13 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -26,7 +26,7 @@ classifiers = [ ] keywords = ["ai", "rag", "retrieval", "knowledge-base", "llm", "pageindex", "agents", "document"] dependencies = [ - "pageindex==0.3.0.dev1", + "pageindex @ git+https://github.com/VectifyAI/PageIndex.git@dev", "markitdown[docx,pptx,xlsx,xls]>=0.1.5", "trafilatura>=2.0", "click>=8.0", @@ -54,6 +54,9 @@ testpaths = ["tests"] [project.optional-dependencies] dev = ["pytest", "pytest-asyncio"] +[tool.hatch.metadata] +allow-direct-references = true + [tool.hatch.version] source = "vcs" diff --git a/uv.lock b/uv.lock index ea1d268e..af6e13a4 100644 --- a/uv.lock +++ b/uv.lock @@ -1923,7 +1923,7 @@ requires-dist = [ { name = "litellm" }, { name = "markitdown", extras = ["docx", "pptx", "xls", "xlsx"], specifier = ">=0.1.5" }, { name = "openai-agents" }, - { name = "pageindex", specifier = "==0.3.0.dev1" }, + { name = "pageindex", git = "https://github.com/VectifyAI/PageIndex.git?rev=dev" }, { name = "prompt-toolkit", specifier = ">=3.0" }, { name = "pytest", marker = "extra == 'dev'" }, { name = "pytest-asyncio", marker = "extra == 'dev'" }, @@ -1959,20 +1959,19 @@ wheels = [ [[package]] name = "pageindex" version = "0.3.0.dev1" -source = { registry = "https://pypi.org/simple" } +source = { git = "https://github.com/VectifyAI/PageIndex.git?rev=dev#f354eb17bf1d3c43c615e4fe58f097303d7a5f6b" } dependencies = [ { name = "httpx", extra = ["socks"] }, { name = "litellm" }, + { name = "openai" }, { name = "openai-agents" }, + { name = "pydantic" }, { name = "pymupdf" }, { name = "pypdf2" }, { name = "python-dotenv" }, { name = "pyyaml" }, { name = "requests" }, -] -sdist = { url = "https://files.pythonhosted.org/packages/53/27/318e73fe1ed2194b19abeb53f57f30b022c9ea4a3ebfe8c2b293f239d537/pageindex-0.3.0.dev1.tar.gz", hash = "sha256:77f02a85622a180918a11d4c4e4e97c10dc0ba142c6418ab1c501d6c1301a409", size = 61010, upload-time = "2026-04-10T17:20:58.524Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/03/7b/a2d37a28475605b715f9fe28320989aec753c023b272fbf9c4bf9f693a55/pageindex-0.3.0.dev1-py3-none-any.whl", hash = "sha256:570c7fd7e54d62782030154dfe02b2f6b9e88407a293f96ea824203716ef28d3", size = 66954, upload-time = "2026-04-10T17:20:56.738Z" }, + { name = "typing-extensions" }, ] [[package]] From f732df1447838111624ef7753b5af29d8d89ec62 Mon Sep 17 00:00:00 2001 From: mountain Date: Thu, 25 Jun 2026 19:31:54 +0800 Subject: [PATCH 08/11] test(indexer): isolate TestIndexLongDocument from a configured PAGEINDEX_API_KEY These tests drive the LOCAL indexing path with a fake PDF. When a developer has PAGEINDEX_API_KEY set, index_long_document takes the cloud branch and _get_pdf_page_count tries to open the fake PDF and raises (FileDataError). Add a class-scoped autouse fixture that unsets the key by default; the cloud-path tests in the class re-enable it via monkeypatch.setenv. --- tests/test_indexer.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/test_indexer.py b/tests/test_indexer.py index faa9713c..bd2bcf98 100644 --- a/tests/test_indexer.py +++ b/tests/test_indexer.py @@ -3,6 +3,7 @@ from unittest.mock import MagicMock, patch +import pytest from openkb.indexer import IndexResult, _normalize_page_content, index_long_document @@ -37,6 +38,14 @@ def test_rejects_unusable_shapes(self): class TestIndexLongDocument: + @pytest.fixture(autouse=True) + def _local_path_by_default(self, monkeypatch): + # These tests exercise the LOCAL indexing path; unset PAGEINDEX_API_KEY + # so they are deterministic regardless of a developer's configured key + # (otherwise the cloud branch reads the page count from the fake PDF and + # raises). Cloud-path tests in this class re-enable it via setenv. + monkeypatch.delenv("PAGEINDEX_API_KEY", raising=False) + def _make_fake_collection(self, doc_id: str, sample_tree: dict): """Build a mock Collection that returns the sample_tree fixture data.""" col = MagicMock() From 12f66ae0e4e6dc1c43a73432710317b1822e12b6 Mon Sep 17 00:00:00 2001 From: mountain Date: Thu, 25 Jun 2026 20:17:27 +0800 Subject: [PATCH 09/11] fix(cloud-import): window page fetch around PageIndex's 1000-page cap; recompile/list recognize pageindex_cloud Code-review fixes on the cloud-import path: - indexer: import_cloud_document requested get_page_content(doc_id, "1-") with N=_max_page_index(structure, default=100000). PageIndex's parse_pages caps a single range at 1000 pages, so import HARD-FAILED for any cloud tree without integer page indices (default 100000) or any doc >1000 pages. get_page_content fetches the whole doc and uses the range only as a client-side filter, so fetch fixed 1000-page windows via _fetch_cloud_pages and concatenate. Each window over-covers ("1-1000"), which also fixes last-page truncation when the tree's max index is 0-based/short (verified live: attention paper imports 15 pages, was 13). - cli: recompile misclassified pageindex_cloud docs as SHORT (== "long_pdf" checks), routing them to the short branch that looks for a .md source cloud imports never write (.json). Add _is_long_doc({long_pdf, pageindex_cloud}) and use it in recompile's _classify + dispatch. remove's local-PageIndex cleanup stays long_pdf-only by design. Add pageindex_cloud to _TYPE_DISPLAY_MAP so `openkb list` shows "pageindex", not the raw type string. - pyproject: drop now-dead [tool.hatch.metadata] allow-direct-references (the dep is a pinned version; no direct git/url references remain). - tests: windowing (>1000 + no-index), cloud recompile-as-long, predicate/display. --- openkb/cli.py | 15 +++++++++-- openkb/indexer.py | 43 +++++++++++++++++++++++++----- pyproject.toml | 3 --- tests/test_indexer.py | 58 +++++++++++++++++++++++++++++++++++++++++ tests/test_recompile.py | 57 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 165 insertions(+), 11 deletions(-) diff --git a/openkb/cli.py b/openkb/cli.py index d28556c7..e552e8c4 100644 --- a/openkb/cli.py +++ b/openkb/cli.py @@ -207,8 +207,19 @@ def _setup_llm_key(kb_dir: Path | None = None) -> None: # Map raw doc types to display types _TYPE_DISPLAY_MAP = { "long_pdf": "pageindex", + "pageindex_cloud": "pageindex", } +# Registry types that were compiled via the long-doc pipeline (tree + per-page +# JSON source), as opposed to short docs (markdown source). Both the local +# long-PDF type and cloud imports belong here — they share the long-doc +# summary/source layout and recompile path. +_LONG_DOC_TYPES = {"long_pdf", "pageindex_cloud"} + + +def _is_long_doc(meta: dict) -> bool: + return meta.get("type") in _LONG_DOC_TYPES + _SHORT_DOC_TYPES = {"pdf", "docx", "md", "markdown", "html", "htm", "txt", "csv", "pptx", "xlsx", "xls"} @@ -1327,7 +1338,7 @@ def recompile(ctx, doc_name, all_docs, dry_run, yes, refresh_schema): targets = [matches[0][1]] def _classify(meta: dict) -> str: - return "long" if meta.get("type") == "long_pdf" else "short" + return "long" if _is_long_doc(meta) else "short" # --dry-run: enumerate only, no LLM calls, no writes. if dry_run: @@ -1374,7 +1385,7 @@ def _classify(meta: dict) -> str: skipped += 1 continue - if meta.get("type") == "long_pdf": + if _is_long_doc(meta): summary_path = wiki_dir / "summaries" / f"{name}.md" doc_id = meta.get("doc_id") if not doc_id: diff --git a/openkb/indexer.py b/openkb/indexer.py index 31216291..cb144bb9 100644 --- a/openkb/indexer.py +++ b/openkb/indexer.py @@ -209,13 +209,19 @@ def index_long_document( return IndexResult(doc_id=doc_id, description=description, tree=tree) -def _max_page_index(structure: list, default: int = 100000) -> int: +# PageIndex's get_page_content rejects a single page range covering more than +# this many pages (``parse_pages`` raises "Page range too large (max 1000)"), +# so cloud page fetches are windowed in chunks of this size. +_CLOUD_PAGE_WINDOW = 1000 + + +def _max_page_index(structure: list, default: int = 0) -> int: """Largest start/end page index across the (possibly nested) tree. Cloud tree nodes carry ``start_index``/``end_index`` page numbers; this - bounds ``get_page_content``'s page range without a local PDF. Falls back - to ``default`` (a wide range the cloud filters anyway) when no integer - indices are present. + bounds the windowed cloud page fetch. Returns ``default`` (``0`` = unknown) + when no integer indices are present, leaving the fetch to stop on the first + empty window. """ best = 0 @@ -234,6 +240,32 @@ def _walk(nodes: list) -> None: return best or default +def _fetch_cloud_pages(col, doc_id: str, max_page: int) -> list[dict[str, Any]]: + """Fetch all OCR pages of a cloud doc, windowing around the 1000-page cap. + + ``get_page_content`` returns the whole document and uses its ``pages`` arg + only as a client-side filter that ``parse_pages`` caps at 1000 pages — so a + single ``"1-"`` request fails for any doc over 1000 pages or whose tree + exposes no integer indices. Request fixed ``1000``-page windows instead and + concatenate; each window over-covers its range, so an off-by-one or 0-based + ``max_page`` never truncates the last page. ``max_page`` (0 = unknown) bounds + the loop; otherwise it stops at the first empty window. A wide safety bound + prevents an unbounded loop if the backend never returns an empty window. + """ + pages: list[dict[str, Any]] = [] + upper = max_page if max_page and max_page > 0 else 1_000_000 + start = 1 + while start <= upper: + window = _normalize_page_content( + col.get_page_content(doc_id, f"{start}-{start + _CLOUD_PAGE_WINDOW - 1}") + ) + if not window: + break + pages.extend(window) + start += _CLOUD_PAGE_WINDOW + return pages + + def import_cloud_document(doc_id: str, kb_dir: Path, path_key: str) -> CloudImportResult: """Import an already-indexed PageIndex Cloud document by ``doc_id``. @@ -271,8 +303,7 @@ def import_cloud_document(doc_id: str, kb_dir: Path, path_key: str) -> CloudImpo "structure": structure, } - page_count = _max_page_index(structure) - all_pages = _normalize_page_content(col.get_page_content(doc_id, f"1-{page_count}")) + all_pages = _fetch_cloud_pages(col, doc_id, _max_page_index(structure)) if not all_pages: raise RuntimeError( f"No page content returned from PageIndex Cloud for doc_id={doc_id}" diff --git a/pyproject.toml b/pyproject.toml index 24775386..35eb031a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -61,9 +61,6 @@ testpaths = ["tests"] [project.optional-dependencies] dev = ["pytest==9.0.3", "pytest-asyncio==1.3.0"] -[tool.hatch.metadata] -allow-direct-references = true - [tool.hatch.version] source = "vcs" diff --git a/tests/test_indexer.py b/tests/test_indexer.py index bd2bcf98..02e1d62d 100644 --- a/tests/test_indexer.py +++ b/tests/test_indexer.py @@ -337,3 +337,61 @@ def test_write_long_doc_artifacts_writes_json_and_summary(kb_dir, sample_tree): assert json_file.exists() assert '"content": "Hello."' in json_file.read_text(encoding="utf-8") assert "doc_type: pageindex" in summary_path.read_text(encoding="utf-8") + + +def test_fetch_cloud_pages_windows_over_1000_cap(): + """get_page_content's range filter is capped at 1000 pages by parse_pages, so + _fetch_cloud_pages must request fixed 1000-page windows (never a wider range) + and concatenate them, preserving real page numbers. + """ + from openkb.indexer import _fetch_cloud_pages + + def fake_get(doc_id, rng): + start = int(rng.split("-")[0]) + if start == 1: + return [{"page": p, "content": f"p{p}"} for p in range(1, 1001)] + if start == 1001: + return [{"page": p, "content": f"p{p}"} for p in range(1001, 1501)] + return [] + + col = MagicMock() + col.get_page_content.side_effect = fake_get + + pages = _fetch_cloud_pages(col, "doc", 0) # 0 = unknown bound → loop until empty + assert len(pages) == 1500 + assert pages[0]["page"] == 1 and pages[-1]["page"] == 1500 + ranges = [c.args[1] for c in col.get_page_content.call_args_list] + assert ranges == ["1-1000", "1001-2000", "2001-3000"] + # Every requested window spans exactly 1000 pages → parse_pages never raises. + for r in ranges: + a, b = (int(x) for x in r.split("-")) + assert b - a + 1 == 1000 + + +def test_import_cloud_document_no_indices_avoids_oversized_range(kb_dir, monkeypatch): + """A cloud tree with no integer page indices must NOT request a 100000-page + range (parse_pages rejects >1000); it windows from page 1 instead. + """ + from openkb.indexer import import_cloud_document + + monkeypatch.setenv("PAGEINDEX_API_KEY", "test-key") + col = MagicMock() + col.get_document.return_value = { + "doc_id": "c", "doc_name": "NoIdx.pdf", "doc_description": "d", + "structure": [{"title": "n", "nodes": []}], # no start/end_index anywhere + } + col.get_page_content.side_effect = ( + lambda doc_id, rng: [{"page": 1, "content": "x"}] if rng == "1-1000" else [] + ) + client = MagicMock() + client.collection.return_value = col + + with patch("openkb.indexer.PageIndexClient", return_value=client): + result = import_cloud_document("c", kb_dir, "pageindex-cloud:c") + + assert result.doc_id == "c" + ranges = [c.args[1] for c in col.get_page_content.call_args_list] + assert "1-100000" not in ranges + for r in ranges: + a, b = (int(x) for x in r.split("-")) + assert b - a + 1 <= 1000 diff --git a/tests/test_recompile.py b/tests/test_recompile.py index 5ea6b7d3..64928c7f 100644 --- a/tests/test_recompile.py +++ b/tests/test_recompile.py @@ -113,6 +113,63 @@ def test_recompile_long_dispatches_compile_long_doc_with_doc_id(kb_dir): assert "recompiled 1" in result.output +# --------------------------------------------------------------------------- +# cloud-import dispatch (type=pageindex_cloud) — must use the LONG path +# --------------------------------------------------------------------------- + + +def _seed_cloud(kb_dir: Path) -> None: + """A pageindex_cloud import: long-doc layout (summary + doc_id + .json + source), and NO .md source (the trap the short path would fall into).""" + (kb_dir / ".openkb" / "hashes.json").write_text(json.dumps({ + "h_c": { + "name": "Cloud Paper.pdf", "doc_name": "cloud-h_c", + "type": "pageindex_cloud", "origin": "cloud", "doc_id": "pi-cloud1", + "path": "pageindex-cloud:pi-cloud1", + }, + })) + (kb_dir / "wiki" / "summaries" / "cloud-h_c.md").write_text( + "---\nsources: [pageindex-cloud:pi-cloud1]\n---\n# Cloud\n", encoding="utf-8", + ) + (kb_dir / "wiki" / "sources" / "cloud-h_c.json").write_text("[]", encoding="utf-8") + (kb_dir / "wiki" / "log.md").write_text("# Log\n\n", encoding="utf-8") + + +def test_recompile_cloud_doc_dispatches_compile_long_doc(kb_dir): + """A pageindex_cloud doc must recompile via compile_long_doc (it has a .json + source + doc_id), not be misrouted to the short path that looks for a .md.""" + _seed_cloud(kb_dir) + with patch("openkb.agent.compiler.compile_long_doc", new_callable=AsyncMock) as long_, \ + patch("openkb.agent.compiler.compile_short_doc", new_callable=AsyncMock) as short: + result = _invoke(kb_dir, ["recompile", "cloud-h_c"]) + + assert result.exit_code == 0, result.output + long_.assert_called_once() + args = long_.call_args.args + assert args[0] == "cloud-h_c" # doc_name + assert args[2] == "pi-cloud1" # the cloud doc_id flows through + short.assert_not_called() + assert "recompiled 1" in result.output + + +def test_recompile_dry_run_classifies_cloud_as_long(kb_dir): + _seed_cloud(kb_dir) + result = _invoke(kb_dir, ["recompile", "cloud-h_c", "--dry-run"]) + assert result.exit_code == 0, result.output + assert "(long)" in result.output and "(short)" not in result.output + + +def test_is_long_doc_and_display_type_cover_cloud(): + """pageindex_cloud is treated as a long doc and displayed like a pageindex + doc in `openkb list` (no raw internal type string leaking).""" + from openkb.cli import _is_long_doc, _display_type + + assert _is_long_doc({"type": "pageindex_cloud"}) is True + assert _is_long_doc({"type": "long_pdf"}) is True + assert _is_long_doc({"type": "md"}) is False + assert _display_type("pageindex_cloud") == "pageindex" + + # --------------------------------------------------------------------------- # --all confirmation + --yes # --------------------------------------------------------------------------- From 47b46164561114aad4be53987308c6ca828b572e Mon Sep 17 00:00:00 2001 From: mountain Date: Thu, 25 Jun 2026 20:24:57 +0800 Subject: [PATCH 10/11] fix(cloud-import): clean up orphans on compile failure; exit non-zero on failed import MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - import_from_pageindex_cloud writes the summary + JSON source (in import_cloud_document) and compiles concept/entity pages BEFORE adding the registry entry, so a compile failure stranded wiki artifacts that `openkb remove` (registry-only resolution) could never reach. On failure, best-effort clean them via _cleanup_failed_cloud_import — reusing remove's by-doc_name wiki helpers (summary / source.json / concept + entity pages / index), touching neither the registry (no entry was added, so a retry stays clean) nor PageIndex (the cloud doc is the user's asset). - add: the cloud-import outcome was discarded, so `openkb add --from-pageindex-cloud` exited 0 even on failure. ctx.exit(1) on "failed" so scripted/CI callers can detect it. - tests: compile-failure orphan cleanup + the failure/success exit codes. --- openkb/cli.py | 46 +++++++++++++++++++++++++++++++++++++-- tests/test_add_command.py | 44 ++++++++++++++++++++++++++++++++++++- 2 files changed, 87 insertions(+), 3 deletions(-) diff --git a/openkb/cli.py b/openkb/cli.py index e552e8c4..1c415d01 100644 --- a/openkb/cli.py +++ b/openkb/cli.py @@ -456,6 +456,36 @@ def _add_single_file_locked(file_path: Path, kb_dir: Path) -> Literal["added", " return "added" +def _cleanup_failed_cloud_import(kb_dir: Path, doc_name: str) -> None: + """Best-effort wiki cleanup after a cloud import whose compilation failed. + + import_cloud_document writes the summary + per-page JSON source before + compile, and compile_long_doc writes concept/entity pages incrementally — so + a compile failure (which happens before the registry entry is added) would + otherwise strand wiki artifacts that ``openkb remove`` cannot reach. Mirror + remove's wiki cleanup (by doc_name, idempotent) but touch neither the + registry (no entry was added) nor PageIndex (the cloud doc is the user's). + """ + from openkb.agent.compiler import ( + remove_doc_from_concept_pages, + remove_doc_from_entity_pages, + remove_doc_from_index, + ) + + wiki_dir = kb_dir / "wiki" + (wiki_dir / "summaries" / f"{doc_name}.md").unlink(missing_ok=True) + (wiki_dir / "sources" / f"{doc_name}.json").unlink(missing_ok=True) + images_dir = wiki_dir / "sources" / "images" / doc_name + if images_dir.is_dir(): + shutil.rmtree(images_dir, ignore_errors=True) + concept_result = remove_doc_from_concept_pages(wiki_dir, doc_name, keep_empty=False) + entity_result = remove_doc_from_entity_pages(wiki_dir, doc_name, keep_empty=False) + remove_doc_from_index( + wiki_dir, doc_name, concept_result["deleted"], + entity_slugs_deleted=entity_result["deleted"], + ) + + def import_from_pageindex_cloud( doc_id: str, kb_dir: Path ) -> Literal["added", "skipped", "failed"]: @@ -494,6 +524,7 @@ def import_from_pageindex_cloud( doc_name = import_result.doc_name summary_path = kb_dir / "wiki" / "summaries" / f"{doc_name}.md" click.echo(f" Compiling imported doc (doc_id={doc_id})...") + compiled = False for attempt in range(2): try: asyncio.run( @@ -502,6 +533,7 @@ def import_from_pageindex_cloud( doc_description=import_result.description, ) ) + compiled = True break except Exception as exc: if attempt == 0: @@ -510,7 +542,15 @@ def import_from_pageindex_cloud( else: click.echo(f" [ERROR] Compilation failed: {exc}") logger.debug("Compilation traceback:", exc_info=True) - return "failed" + if not compiled: + # No registry entry exists yet, so `openkb remove` can't reach the + # summary/source/concept/entity artifacts already written; clean them + # best-effort so a failed import leaves no orphans and a retry is clean. + try: + _cleanup_failed_cloud_import(kb_dir, doc_name) + except Exception: + logger.debug("Cleanup after failed cloud import errored:", exc_info=True) + return "failed" # Register the raw-less cloud entry only after successful compilation. registry = HashRegistry(openkb_dir / "hashes.json") @@ -788,7 +828,9 @@ def add(ctx, path, from_pageindex_cloud): if path is not None: click.echo("Provide either PATH or --from-pageindex-cloud, not both.") return - import_from_pageindex_cloud(from_pageindex_cloud, kb_dir) + outcome = import_from_pageindex_cloud(from_pageindex_cloud, kb_dir) + if outcome == "failed": + ctx.exit(1) return if path is None: diff --git a/tests/test_add_command.py b/tests/test_add_command.py index 35f544ef..f819900f 100644 --- a/tests/test_add_command.py +++ b/tests/test_add_command.py @@ -209,8 +209,17 @@ def test_add_from_pageindex_cloud_dispatches(self, tmp_path): runner = CliRunner() with patch("openkb.cli.import_from_pageindex_cloud", return_value="added") as mock_imp, \ patch("openkb.cli._find_kb_dir", return_value=kb_dir): - runner.invoke(cli, ["add", "--from-pageindex-cloud", "doc-123"]) + result = runner.invoke(cli, ["add", "--from-pageindex-cloud", "doc-123"]) mock_imp.assert_called_once_with("doc-123", kb_dir) + assert result.exit_code == 0 # success → exit 0 + + def test_add_cloud_failure_exits_nonzero(self, tmp_path): + kb_dir = self._setup_kb(tmp_path) + runner = CliRunner() + with patch("openkb.cli.import_from_pageindex_cloud", return_value="failed"), \ + patch("openkb.cli._find_kb_dir", return_value=kb_dir): + result = runner.invoke(cli, ["add", "--from-pageindex-cloud", "doc-x"]) + assert result.exit_code == 1 # failed import must not exit 0 def test_add_rejects_path_and_cloud_together(self, tmp_path): kb_dir = self._setup_kb(tmp_path) @@ -306,3 +315,36 @@ def test_import_failure_returns_failed_and_registers_nothing(self, tmp_path): assert outcome == "failed" registry = HashRegistry(kb_dir / ".openkb" / "hashes.json") assert registry.all_entries() == {} + + def test_compile_failure_cleans_up_orphan_artifacts(self, tmp_path): + """If import succeeds (artifacts written) but compile fails twice, the + summary/source artifacts are cleaned up — no registry entry exists, so + `openkb remove` couldn't reach them otherwise — and nothing is registered + (so a retry isn't skipped).""" + from openkb.cli import import_from_pageindex_cloud + from openkb.indexer import CloudImportResult + from openkb.state import HashRegistry + + kb_dir = self._setup_kb(tmp_path) + (kb_dir / "wiki" / "entities").mkdir(parents=True, exist_ok=True) + (kb_dir / "wiki" / "index.md").write_text("# Index\n", encoding="utf-8") + doc_name = "Cloud-Paper" + # Simulate the artifacts import_cloud_document writes before compile. + (kb_dir / "wiki" / "summaries" / f"{doc_name}.md").write_text("---\n---\n# s\n") + (kb_dir / "wiki" / "sources" / f"{doc_name}.json").write_text("[]") + result = CloudImportResult( + doc_id="cloud-1", doc_name=doc_name, name="Cloud Paper.pdf", description="d", + ) + + with patch("openkb.cli.import_cloud_document", return_value=result), \ + patch("openkb.cli.compile_long_doc", side_effect=RuntimeError("boom")), \ + patch("openkb.cli.time.sleep"), \ + patch("openkb.cli._setup_llm_key"): + outcome = import_from_pageindex_cloud("cloud-1", kb_dir) + + assert outcome == "failed" + # Orphan artifacts cleaned up (would be unreachable by `remove` otherwise). + assert not (kb_dir / "wiki" / "summaries" / f"{doc_name}.md").exists() + assert not (kb_dir / "wiki" / "sources" / f"{doc_name}.json").exists() + # Nothing registered → a retry is not skipped. + assert HashRegistry(kb_dir / ".openkb" / "hashes.json").all_entries() == {} From 40936e69a2432509f8570f96d7a9371f488fd7c2 Mon Sep 17 00:00:00 2001 From: mountain Date: Thu, 25 Jun 2026 20:43:20 +0800 Subject: [PATCH 11/11] fix(cloud-import): stop windowed page fetch on a short window, not the tree's max index MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _fetch_cloud_pages bounded its loop by the tree's max page index (_max_page_index). When the cloud tree under-reports the page count — a real case (e.g. a paper whose tree stops a couple pages short of the references) — the loop exited before fetching a later window and silently DROPPED pages. At the exact 1000 boundary this was an off-by-one (a 1001-page doc whose tree maxes at 1000 lost page 1001); more generally any >1000-page doc with an under-reporting tree was truncated. Drop the max-index bound: request fixed 1000-page windows and stop as soon as a window comes back SHORT (PageIndex page numbers are sequential, so a short window means we've passed the last page). The common ≤1000-page doc stays a single request, a larger doc fetches every page, and an under-reported tree no longer truncates. Remove the now-unused _max_page_index. Tests updated + a full-window-triggers-next-fetch regression. --- openkb/indexer.py | 54 ++++++++++++++----------------------------- tests/test_indexer.py | 27 ++++++++++++++++++++-- 2 files changed, 42 insertions(+), 39 deletions(-) diff --git a/openkb/indexer.py b/openkb/indexer.py index cb144bb9..691a126c 100644 --- a/openkb/indexer.py +++ b/openkb/indexer.py @@ -213,55 +213,35 @@ def index_long_document( # this many pages (``parse_pages`` raises "Page range too large (max 1000)"), # so cloud page fetches are windowed in chunks of this size. _CLOUD_PAGE_WINDOW = 1000 +# Safety bound on the windowed fetch (in pages) in case a backend never returns +# a short window — caps the loop at _CLOUD_PAGE_MAX / _CLOUD_PAGE_WINDOW calls. +_CLOUD_PAGE_MAX = 1_000_000 -def _max_page_index(structure: list, default: int = 0) -> int: - """Largest start/end page index across the (possibly nested) tree. - - Cloud tree nodes carry ``start_index``/``end_index`` page numbers; this - bounds the windowed cloud page fetch. Returns ``default`` (``0`` = unknown) - when no integer indices are present, leaving the fetch to stop on the first - empty window. - """ - best = 0 - - def _walk(nodes: list) -> None: - nonlocal best - for node in nodes or []: - if not isinstance(node, dict): - continue - for key in ("end_index", "start_index"): - val = node.get(key) - if isinstance(val, int): - best = max(best, val) - _walk(node.get("nodes")) - - _walk(structure) - return best or default - - -def _fetch_cloud_pages(col, doc_id: str, max_page: int) -> list[dict[str, Any]]: +def _fetch_cloud_pages(col, doc_id: str) -> list[dict[str, Any]]: """Fetch all OCR pages of a cloud doc, windowing around the 1000-page cap. ``get_page_content`` returns the whole document and uses its ``pages`` arg only as a client-side filter that ``parse_pages`` caps at 1000 pages — so a - single ``"1-"`` request fails for any doc over 1000 pages or whose tree - exposes no integer indices. Request fixed ``1000``-page windows instead and - concatenate; each window over-covers its range, so an off-by-one or 0-based - ``max_page`` never truncates the last page. ``max_page`` (0 = unknown) bounds - the loop; otherwise it stops at the first empty window. A wide safety bound - prevents an unbounded loop if the backend never returns an empty window. + single ``"1-"`` request fails for any doc over 1000 pages. Request fixed + ``1000``-page windows and stop as soon as a window comes back SHORT (fewer + than a full window): PageIndex page numbers are sequential, so a short window + means we've passed the last page. This is what makes the common (≤1000-page) + doc a single request, while still fetching every page of a larger one — and, + unlike bounding the loop by the tree's max page index, it never truncates a + doc whose tree under-reports its page count (a real case: a paper whose tree + stops a couple pages short of the references). A wide safety bound guards + against a backend that never narrows the window. """ pages: list[dict[str, Any]] = [] - upper = max_page if max_page and max_page > 0 else 1_000_000 start = 1 - while start <= upper: + while start <= _CLOUD_PAGE_MAX: window = _normalize_page_content( col.get_page_content(doc_id, f"{start}-{start + _CLOUD_PAGE_WINDOW - 1}") ) - if not window: - break pages.extend(window) + if len(window) < _CLOUD_PAGE_WINDOW: + break start += _CLOUD_PAGE_WINDOW return pages @@ -303,7 +283,7 @@ def import_cloud_document(doc_id: str, kb_dir: Path, path_key: str) -> CloudImpo "structure": structure, } - all_pages = _fetch_cloud_pages(col, doc_id, _max_page_index(structure)) + all_pages = _fetch_cloud_pages(col, doc_id) if not all_pages: raise RuntimeError( f"No page content returned from PageIndex Cloud for doc_id={doc_id}" diff --git a/tests/test_indexer.py b/tests/test_indexer.py index 02e1d62d..d4a533ba 100644 --- a/tests/test_indexer.py +++ b/tests/test_indexer.py @@ -357,17 +357,40 @@ def fake_get(doc_id, rng): col = MagicMock() col.get_page_content.side_effect = fake_get - pages = _fetch_cloud_pages(col, "doc", 0) # 0 = unknown bound → loop until empty + pages = _fetch_cloud_pages(col, "doc") assert len(pages) == 1500 assert pages[0]["page"] == 1 and pages[-1]["page"] == 1500 ranges = [c.args[1] for c in col.get_page_content.call_args_list] - assert ranges == ["1-1000", "1001-2000", "2001-3000"] + # Full first window → fetch the next; the short 2nd window (500<1000) stops it. + assert ranges == ["1-1000", "1001-2000"] # Every requested window spans exactly 1000 pages → parse_pages never raises. for r in ranges: a, b = (int(x) for x in r.split("-")) assert b - a + 1 == 1000 +def test_fetch_cloud_pages_full_window_triggers_next_fetch(): + """A doc whose pages exactly fill the first window must still fetch the next + one. Regression: bounding the loop by the tree's max page index dropped the + straggler page(s) of a doc whose tree under-reported its page count. + """ + from openkb.indexer import _fetch_cloud_pages + + def fake_get(doc_id, rng): + start = int(rng.split("-")[0]) + if start == 1: + return [{"page": p, "content": "x"} for p in range(1, 1001)] # full window + if start == 1001: + return [{"page": 1001, "content": "x"}] # one straggler past the window + return [] + + col = MagicMock() + col.get_page_content.side_effect = fake_get + + pages = _fetch_cloud_pages(col, "doc") + assert [p["page"] for p in pages] == list(range(1, 1002)) # page 1001 NOT dropped + + def test_import_cloud_document_no_indices_avoids_oversized_range(kb_dir, monkeypatch): """A cloud tree with no integer page indices must NOT request a 100000-page range (parse_pages rejects >1000); it windows from page 1 instead.