feat: introduce PageIndex SDK with Collection-based local/cloud API#272
Open
KylinMountain wants to merge 32 commits into
Open
feat: introduce PageIndex SDK with Collection-based local/cloud API#272KylinMountain wants to merge 32 commits into
KylinMountain wants to merge 32 commits into
Conversation
The cloud backend previously polled tree_resp["retrieval_ready"] as the ready signal. Empirically this flag is not a reliable indicator — docs can reach status=="completed" without retrieval_ready flipping, causing col.add() to wait until the 10 min timeout before giving up on otherwise-successful uploads. The cloud API's canonical ready signal is status=="completed"; switch the poll to check that instead.
* feat:compatible with Pageindex SDK * corner cases fixed * fix: mock behavior of old SDK * fix: close streaming response and warn on empty api_key - LegacyCloudAPI: close response in `finally` for both _stream_chat_response variants so abandoned iterators no longer leak the TCP connection. - PageIndexClient: emit a warning instead of silently falling back to local when api_key is the empty string, surfacing typical env-var-unset misconfig. - FakeResponse: add close()/closed to match the real requests.Response API. - Add unit coverage for stream close (both paths) and the empty-api_key warning. - Add scripts/e2e_legacy_sdk.py to smoke-test the legacy SDK contract end-to-end against api.pageindex.ai. * chore: mark legacy SDK methods with @deprecated and docstring pointers - Decorate the 12 PageIndexClient cloud-SDK compat methods with @typing_extensions.deprecated(..., category=PendingDeprecationWarning): - IDE/type-checkers render them with a strikethrough hint - runtime warnings stay silent by default (no spam for existing callers), surfaceable via `python -W default::PendingDeprecationWarning` - Add a one-line docstring on each pointing to the Collection-based equivalent. - Promote typing-extensions to a direct dependency (was transitive via litellm). --------- Co-authored-by: XinyanZhou <xinyanzhou@XinyanZhoudeMacBook-Pro.local> Co-authored-by: saccharin98 <xinyanzhou938@gmail.com> Co-authored-by: mountain <kose2livs@gmail.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Collaborator
Author
Code reviewFound 1 issue:
Lines 1 to 7 in 595895c Lines 22 to 33 in 595895c 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
pageindex/config.py imports `from pydantic import BaseModel` in production code, but pyproject.toml only pulled pydantic in transitively via litellm. A future litellm release could drop or re-pin pydantic and break installs. Pin to `>=2.5.0,<3.0.0` to match the v2-style BaseModel usage already in the codebase, and to stay compatible with litellm's own pydantic constraint.
Filename is always built as `.png` regardless of the source ext, so the variable was dead. Flagged by github-code-quality.
- get_agent_tools branches on doc_ids:
- scoped (doc_ids=[...]): drops list_documents and hard-enforces a
whitelist on the remaining tools; system prompt switches to
SCOPED_SYSTEM_PROMPT (no list_documents instruction); doc list +
summaries are prepended to the user message via wrap_with_doc_context.
- open (doc_ids=None): unchanged 4-tool agent loop.
- list_documents now exposes doc_description (sqlite + cloud).
- Collection.query emits UserWarning when doc_ids is None and the
collection holds >1 documents; PAGEINDEX_EXPERIMENTAL_MULTIDOC=1
silences it. Single-doc collections skip the warning; empty
collections raise ValueError.
- Agents SDK tracing upload disabled by default (avoids SSL timeouts);
PAGEINDEX_AGENTS_TRACING=1 re-enables it.
- README: new SDK Usage section covering local/cloud quick start,
streaming, multi-doc as experimental, and runnable examples.
- Collection.query and Backend.query/query_stream accept doc_ids as str, list[str] or None. Single str is normalized to [str] inside each backend; bare [] is rejected with ValueError at both layers. - wrap_with_doc_context wraps the scoped doc list in <docs>...</docs> and SCOPED_SYSTEM_PROMPT instructs the agent to treat that block as data, not instructions (defense against prompt injection via auto-generated doc_description). - _require_cloud_api now distinguishes api_key="" from api_key=None; the former gives a targeted error pointing at the empty-string vs fall-back-to-local situation when legacy SDK methods are called. - Legacy PageIndexClient.list_documents docstring spells out the return-shape difference vs collection.list_documents() to flag a silent migration footgun (paginated dict with id/name keys vs plain list[dict] with doc_id/doc_name keys). - Remove dead CloudBackend.get_agent_tools stub (not on the Backend protocol; only ever returned an empty AgentTools()) and the SYSTEM_PROMPT alias (OPEN_/SCOPED_SYSTEM_PROMPT are the explicit names now). - README quick start and streaming example now pass doc_ids; new multi-document section shows both str and list forms. - examples/demo_query_modes.py exercises all five query-mode cases (single-doc, multi-doc with/without env var, scoped single, scoped multi) for manual verification.
KylinMountain
changed the title
BukeLy
previously approved these changes
May 15, 2026
scripts/e2e_legacy_sdk.py becomes examples/demo_legacy_sdk.py to sit alongside the other runnable demos (local/cloud/query-modes), and the README's Runnable examples list now points at it. Docstring command updated to the new path; the legacy script docstring also calls out that it exercises the 0.2.x compatibility methods. The scripts/ directory had no other entries and is removed.
…e global
llm_completion / llm_acompletion (pageindex/utils.py and
pageindex/index/utils.py) set `litellm.drop_params = True` on the litellm
module. litellm is a process-wide singleton, so this leaked into every other
library sharing it (e.g. a host app like OpenKB that exposes its own litellm
config) and could not be turned off.
Replace the hardcoded `temperature=0` + global `drop_params` with a single
PageIndex-owned per-call kwargs mechanism (config._LLM_PARAMS):
- defaults preserve behavior: {"temperature": 0, "drop_params": True};
- passed per call via **get_llm_params(), never writing litellm's globals, so
nothing leaks into other litellm users in the same process;
- externally configurable: pageindex.set_llm_params(drop_params=False,
temperature=1, num_retries=5, ...) or the PAGEINDEX_DROP_PARAMS env shortcut;
- model/messages are reserved (PageIndex supplies them) and rejected.
| self._init_local(model, retrieve_model, storage_path, storage, index_config) | ||
|
|
||
|
|
||
| class CloudClient(PageIndexClient): |
list_to_tree() deletes the 'nodes' key from leaf nodes entirely via
clean_node(). Direct access via structure['nodes'] raises KeyError on
these nodes. Using structure.get('nodes') returns None (falsy) safely,
consistent with how 'nodes' is accessed elsewhere in the codebase.
Fixes #330
…ayer Fixes the cloud/local contract mismatches from the PR #272 review (verified against the official API docs — the cloud API has no folder/collection endpoints publicly, GET /docs supports limit<=100 with offset): - query_stream: emit a terminal answer_done event with the full answer (same contract as the local backend); raise CloudAPIError instead of disguising HTTP errors as answer events; move the initial connect inside try so a connection failure can no longer strand the consumer awaiting a sentinel that never arrives - _request: rewind file objects before retrying so a transient 5xx/429 during upload no longer re-sends an empty multipart body; carry the HTTP status on CloudAPIError (status_code) and keep the last status in the max-retries error - list_documents: paginate with limit/offset instead of a hard-coded limit=100, so >100-doc collections are no longer silently truncated (whole-collection queries rely on this list) - folders: treat only 403/404 as "folders unavailable" (warned via warnings.warn instead of an invisible logger.warning, matched on status_code instead of a "403" substring); transient errors now propagate instead of being permanently cached as folder_id=None - error taxonomy parity: cloud doc endpoints map HTTP 404 to DocumentNotFoundError; local get_document raises DocumentNotFoundError instead of returning {}; local delete_document raises on missing doc_id instead of silently deleting nothing - cloud get_document warns that include_text is not supported instead of silently ignoring it Adds regression tests for each fix (11 new tests). Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
- local delete_collection: validate the collection name before rmtree. An unvalidated name like "../.." escaped files_dir and deleted arbitrary directories (path traversal). - legacy page_index(): restore the node_id/summary/text/description enhancements. IndexConfig now carries booleans (pydantic coerces the legacy 'yes'/'no' strings at the boundary), but page_index_main still compared `opt.if_add_node_id == 'yes'` — always False — so every enhancement was silently skipped for legacy-API callers. Conditions now branch on the booleans, matching pageindex/index/page_index.py. - LegacyCloudAPI._request: bound every request with a timeout (30s, 120s read timeout for streamed responses) so a dead connection can't hang legacy submit/poll/chat callers forever. The legacy contract tests pinned the missing timeout; updated to pin its presence instead. Adds regression tests: path-traversal rejection, 'yes'/'no' -> bool coercion, and timeout assertions. Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
- AgentRunner.run: offload to a worker-thread event loop when called from inside a running loop (Jupyter, FastAPI handlers) — mirrors pipeline._run_async; Runner.run_sync raised RuntimeError there. - SQLiteStorage: create connections with check_same_thread=False so close() can actually close connections created by worker threads. Each thread still gets its own connection via threading.local; with the default True those closes raised ProgrammingError (silently swallowed) and leaked every worker connection. - CloudBackend.query: non-streaming chat completions now use a 300s timeout and a single attempt. The default 30s ReadTimeout fired before generation finished and the retry loop re-billed the full server-side retrieval + generation up to three times. _request gains retries/timeout overrides; the exhausted-retry path also no longer sleeps before raising. - MarkdownParser: content before the first heading (abstract/preamble) becomes a node instead of being silently dropped and unretrievable; a file with no headings at all yields a single document node instead of zero nodes (which pushed an empty page list into the pipeline). - LegacyCloudAPI.is_retrieval_ready: API failures (revoked key, network down) now propagate as PageIndexAPIError instead of reading as "not ready", which turned polling loops into infinite loops. Adds regression tests for each fix. Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
…on shims The new SDK copied the legacy indexing pipeline into pageindex/index/ instead of moving it, leaving two divergent copies of page_index.py / page_index_md.py / utils.py. They had already drifted (the legacy copy still compared IndexConfig booleans against 'yes' — a separate fix), and every pipeline change had to be applied twice. Make pageindex/index/ the single source of truth (same pattern as the LegacyCloudAPI shim for the 0.2.x cloud SDK): - pageindex/index/utils.py absorbs the 27 legacy-only helpers/classes (get_page_tokens, convert_page_to_int, ConfigLoader, PDF text helpers, ...) so it's the sole utils module. Reconciled the diverged funcs: kept the modern versions, backported the #331 get_leaf_nodes .get() fix, and restored remove_fields' max_len parameter (superset). - index/page_index*.py now import `from .utils import *`; index/legacy_utils.py (a re-export of the old top-level utils) deleted. - Top-level page_index.py / page_index_md.py / utils.py become thin re-export shims that emit PendingDeprecationWarning. The md_to_tree shim coerces legacy 'yes'/'no' string flags to bool (the canonical version is boolean-typed). - ConfigLoader no longer reads the deleted config.yaml; it builds defaults from IndexConfig (was an unconditional FileNotFoundError). - __init__.py and retrieve.py import from pageindex.index.* directly so `import pageindex` does not trip the shims. Adds tests/test_legacy_shims.py pinning the contract: clean top-level import doesn't warn, legacy submodule imports warn, symbols still resolve, shim and canonical share one implementation, the #331 fix and ConfigLoader-without-yaml both hold, and the md_to_tree coercion works. Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
- legacy call_llm: open AsyncOpenAI via `async with` so the client (and its HTTP connection pool) is closed instead of leaked. - LocalBackend.add_document: fail fast with CollectionNotFoundError when the collection doesn't exist, before the expensive parse + LLM index (previously the missing FK only tripped at save time, after paying for the LLM work). Also raise builtin FileNotFoundError for a missing path instead of FileTypeError (which now means only "unsupported extension"). - Collection.query(doc_ids=None): the empty-collection guard now always runs — previously it was skipped once PAGEINDEX_EXPERIMENTAL_MULTIDOC was set. A single list_documents call serves both the guard and the multi-doc warning (no separate call just to decide whether to warn). - CloudBackend.query_stream: on early consumer break / GeneratorExit, signal the background SSE thread to stop and force-close the response, so it no longer drains the whole stream in the background. Adds regression tests for each (client closed, fail-fast on unknown collection, FileNotFoundError, empty-check under the multidoc env flag, single list call, early-break thread stop). Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
- SQLite dedup race: add UNIQUE(collection_name, file_hash) and switch save_document to a plain INSERT. add_document now catches the IntegrityError from a concurrent add of the same content, cleans up its managed files, and returns the winning doc_id — instead of two doc_ids for one file (each having paid for its own LLM indexing). - PDF image paths: store the absolute path to each extracted image instead of a path relative to the indexing process's cwd. The  references broke as soon as a query ran from a different directory. - LegacyCloudAPI: URL-encode doc_id / retrieval_id path segments (added _enc()), matching CloudBackend. An id containing '/', '?', '#' or a space previously hit the wrong endpoint or produced a malformed URL. Adds regression tests: UNIQUE enforcement, the add-race resolving to the winner, absolute image paths, and encoded legacy URLs. Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
Engineering-quality cleanups from the SDK review (no behavior change):
- Return-type discoverability: add pageindex/types.py with TypedDicts
(DocumentInfo, DocumentDetail, PageContent) and annotate Collection /
Backend methods with them; add docstrings to every public Collection
method (including the get_page_content `pages` spec). Exported from the
package. Zero runtime cost — these are plain dicts.
- Backend protocol as a real contract:
* query_stream is an async generator, so the protocol now declares it
as `def ... -> AsyncIterator[QueryEvent]` (not `async def`, which
typed it as a coroutine and never matched the implementations).
* custom-parser support is expressed as a runtime_checkable
SupportsParserRegistration capability protocol; the client uses
isinstance(...) instead of hasattr(...) duck-typing.
- Parser layering: move count_tokens into a leaf module pageindex/tokens.py
so parser/* imports it from there instead of reaching back into
pageindex.index (a reverse dependency). index.utils re-exports it for
backward compatibility.
Adds tests/test_architecture.py enforcing: parser never imports index,
count_tokens is a single shared leaf, the capability protocol works,
both backends satisfy Backend, and the TypedDicts are exported.
Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
…ntract For faithful 0.2.x cloud SDK drop-in compatibility, is_retrieval_ready again swallows PageIndexAPIError and returns False (instead of raising), so existing `while not is_retrieval_ready(...)` polling loops behave exactly as before. Documented that this can loop forever on a permanent error — that is the legacy contract; callers guard their own loops. (The new SDK's own indexing path doesn't use this method — it polls document status with a bounded 120-attempt cap — so the infinite-loop risk is confined to legacy-SDK usage that already had it.) Test updated to assert the swallow behavior. Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
…anup Replace the bare `except Exception: pass` around resp.close() in the query_stream finally block with an explanatory comment and a debug log (flagged by github-code-quality on PR #272). Behavior unchanged — the close is best-effort to unblock the background thread. Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
main advanced (litellm 1.84.0 #342, #188 TOC fixes, #281, README) while dev turned pageindex/page_index.py and utils.py into deprecation shims over pageindex/index/*. Both sides touched those two files, hence the conflict. Resolution: - Keep dev's shims for the two top-level modules (the real implementation lives in pageindex/index/*). requirements.txt auto-merged to litellm==1.84.0. - #188 ("prevent KeyError crash and context exhaustion in TOC processing") landed on main's top-level page_index.py, which is now a shim on dev — so its fixes were NOT in dev's index/page_index.py. Ported them into pageindex/index/page_index.py (preserving dev's IndexConfig/bool integration): .get() on the TOC check functions + detect_page_index, incremental-chat_history retry loops in extract_toc_content and toc_transformer, truncation moved before the loop, .get('table_of_contents', []) and the single_toc_item_index_fixer None guard. - Repoint #188's merged test (tests/test_issue_163.py) at pageindex.index.page_index so its patches hit the module where the code now lives (they were silently hitting the shim → real LLM calls). Full suite: 158 passed, 2 skipped. Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
The dedup refactor dropped the explicit load_dotenv() that the old top-level utils.py ran on import. Since then, .env was only loaded as a side effect of importing litellm — which would silently break both local mode (needs OPENAI_API_KEY in the environment) and cloud usage (callers read PAGEINDEX_API_KEY via os.environ) if litellm changed that behavior or its import were made lazy. Restore an explicit load_dotenv() at the top of pageindex/__init__.py so PageIndex owns .env loading. Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
…locked") Real concurrency e2e (8 threads adding the same file) surfaced a bug the mocked unit tests missed: concurrent add_document calls failed with sqlite3.OperationalError "database is locked". Root cause — under WAL the dedup SELECT (find_document_by_hash) left a read snapshot on the connection, and the subsequent INSERT on that stale snapshot raised SQLITE_BUSY_SNAPSHOT, which busy_timeout does not retry. Fixes: - open connections in autocommit (isolation_level=None) so a SELECT never leaves a lingering read snapshot and each write is its own transaction - PRAGMA busy_timeout=10000 so concurrent writers wait for the WAL single-writer lock instead of failing immediately - an instance-level write lock serializing the fast write methods within the process (the expensive LLM indexing stays parallel) Now 8 concurrent adds of one file -> a single doc_id, zero errors. Adds a real-thread regression test. Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
Cap concurrent in-flight LLM calls during indexing via a shared semaphore (bounded_gather), so a many-node document no longer schedules one socket per node and exhausts the process fd limit (Errno 24). Make the per-index max_concurrency override correct under concurrency: - Scope IndexConfig(max_concurrency=...) to the build_index call via a ContextVar (max_concurrency_scope) instead of mutating a process global. A one-off value no longer sticks as the new default, and concurrent indexing of other documents isn't affected. - Propagate the context through _run_async's worker-thread fallback so the override survives the sync-over-async thread hop. - set_max_concurrency() stays as the explicit process-wide setter. Also stop `from .utils import *` leaking a `config` name (SimpleNamespace alias) that shadowed the real pageindex.config submodule for the page_index modules; the alias is now `_config`. Adds regression tests for cap enforcement, scope stickiness/isolation, worker-thread propagation, and the config-namespace fix. Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
The previous approach (bounded_gather building a fresh semaphore per call) did NOT compose: the indexing call graph nests gathers (tree_parser -> process_large_node_recursively -> recurse, plus each node's check_title gather), and each level got its own independent cap. Peak in-flight LLM calls grew ~N^depth, so a deep/wide document still exhausted file descriptors (Errno 24) — the exact failure the cap was meant to prevent — while the flat summary phase was over-serialized. Naively sharing one semaphore across gather levels would instead deadlock (a parent holds a slot while awaiting children that need slots). Move the throttle to the single chokepoint every LLM call funnels through, llm_acompletion: one shared semaphore per event loop, acquired only around the litellm.acompletion network call. This gives a true global cap that composes across any nesting and can't deadlock (a parent awaiting children holds no slot). bounded_gather is gone; the call sites revert to plain asyncio.gather. Also: - Reject bool in max_concurrency validation (bool is an int subclass, so set_max_concurrency(True) / IndexConfig(max_concurrency=True) previously became Semaphore(1) and silently serialized). Shared _validate_max_concurrency + a pydantic field_validator. - Guard check_title_appearance_in_start_concurrent against an out-of-range or 0 physical_index (LLM can emit one): it was dereferenced during task construction, outside the gather's return_exceptions protection, aborting the whole build; 0 silently wrapped to the last page. Now marked 'no'. - Propagate contextvars into agent.py's worker-thread run (mirrors pipeline._run_async) so ContextVar settings stay consistent. - Tests rewritten to cover the nested case the old flat tests missed, the leaf-level throttle in llm_acompletion, bool rejection, and the out-of-range physical_index guard. Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
Member
Code reviewFound 2 issues:
PageIndex/pageindex/index/utils.py Lines 470 to 479 in 2d46d68
Lines 1 to 14 in 2d46d68 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
Address PR #272 review: - get_md_page_content / retrieve._get_md_page_content returned every node whose line_num fell in [min(pages), max(pages)], so a non-contiguous spec like "5,100" over-fetched everything in between. Match the exact requested line numbers instead, mirroring the PDF path. (Same bug as #280.) - Restore the CHATGPT_API_KEY -> OPENAI_API_KEY backward-compat alias dropped when pageindex/utils.py became a re-export shim; users with only CHATGPT_API_KEY set would otherwise fail auth after upgrading. It now runs in __init__.py right after load_dotenv. Adds regression tests for both. Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
Replace the `...` bodies in DocumentParser / StorageEngine protocol methods with one-line docstrings: silences the CodeQL "statement has no effect" false positives on #272 (`...` is idiomatic for typing.Protocol, but docstrings document the contract and don't trip the analyzer) with no behavior change. Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
| @runtime_checkable | ||
| class Backend(Protocol): | ||
| # Collection management | ||
| def create_collection(self, name: str) -> None: ... |
| class Backend(Protocol): | ||
| # Collection management | ||
| def create_collection(self, name: str) -> None: ... | ||
| def get_or_create_collection(self, name: str) -> None: ... |
| # Collection management | ||
| def create_collection(self, name: str) -> None: ... | ||
| def get_or_create_collection(self, name: str) -> None: ... | ||
| def list_collections(self) -> list[str]: ... |
Verified against current dev; the compat/behavior decisions (#7 api_key semantics, #10 CLI flags, #11 doc-description default) are deferred. Crashes: - page_index(): snapshot args before importing IndexConfig — locals() was capturing the imported class and IndexConfig(extra='forbid') made every call raise ValidationError. - process_none_page_numbers: pop('page', None) instead of del (a TOC item without 'page' raised KeyError mid-pipeline). - pipeline._run_async: guard only the loop detection, not the run, so a real RuntimeError from the coroutine isn't masked as "asyncio.run() cannot be called from a running event loop". Silent-wrong / robustness: - LocalBackend.get_document_structure and the agent get_document / get_document_structure tools now surface a missing doc (raise / error-JSON) instead of returning empty, matching get_page_content and the cloud backend. - cloud delete_collection drops the cached folder_id. - cloud query raises on an empty collection instead of POSTing doc_id:[]. - LocalClient skips the API-key check for keyless providers (ollama, lm_studio, …) so keyless LiteLLM models aren't rejected at construction. Compat / cleanup: - md_to_tree coerces legacy 'yes'/'no' string flags (a bare 'no' was truthy). - FileTypeError also subclasses ValueError (0.2.x raised ValueError). - _validate_llm_provider no longer mutates global litellm.model_cost_map_url. - __all__ re-includes legacy exports (page_index, md_to_tree, get_*). - Rewrite examples/agentic_vectorless_rag_demo.py to the Collection API and use the in-repo attention.pdf (the old workspace=/client.index/client.documents API no longer exists). Adds tests/test_review_fixes.py (10 regressions). Full suite: 189 passed. Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
| def create_collection(self, name: str) -> None: ... | ||
| def get_or_create_collection(self, name: str) -> None: ... | ||
| def list_collections(self) -> list[str]: ... | ||
| def delete_collection(self, name: str) -> None: ... |
Resolves PR #272 review #10/P5. The --if-add-node-id / -node-summary / -doc-description / -node-text args were store_true, which rejected the documented yes/no values and left default-on options impossible to disable from the CLI. They now use nargs='?' + const=True + a yes/no-coercing type: --if-add-node-id -> on --if-add-node-id no -> off (legacy form still works) (omitted) -> use the IndexConfig default README updated to the flag usage (noting the legacy `no` off-switch), and --if-add-node-text is now documented too. Decisions from the review: - #7 (api_key semantics): verified FALSE POSITIVE — 0.2.x is a cloud SDK whose api_key is a PageIndex cloud key (cloud_api.LegacyCloudAPI + docs.pageindex.ai/sdk), matching the new SDK. No change. - #11 (if_add_doc_description default True): kept intentionally (open mode). Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
Collaborator
Author
|
Thanks for the thorough reviews (the batched findings and @rejojer's pass). Went through all of them against current Fixed (with regression tests): Crashes:
Silent-wrong / robustness:
Compat / cleanup:
Not changed:
Already handled earlier on Full suite: 190 passed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Turn PageIndex into a proper Python SDK. The repo currently exposes a tree-generation CLI (
run_pageindex.py) and a legacy cloud HTTP wrapper (pageindex_sdk0.2.x). This PR introduces a unifiedPageIndexClientwith a Collection-based API that works in both local self-hosted mode (your LLM key) and cloud-managed mode (PageIndex API key), while keeping the legacy SDK callable on the same client for backward compatibility.What's in the SDK
Public surface (
from pageindex import PageIndexClient):PageIndexClient(api_key=...)— auto-detects cloud vs localclient.collection(name)→CollectionCollection.add/list_documents/get_document/get_document_structure/get_page_content/delete_document/querycol.query(question, doc_ids=..., stream=...)—doc_idsacceptsstr | list[str] | NoneQueryEventTwo execution paths behind the same API:
/chat/completions/,/doc/...endpointsLegacy 0.2.x compatibility on the same
PageIndexClient: all 12 methods (submit_document,get_ocr,get_tree,chat_completions, document & folder management) preserved as@deprecatedwrappers, same signatures and return shapes.Highlights
pageindex_sdk0.2.x methods onPageIndexClient(submit_document,chat_completions, etc.), with stream-close fixes and@deprecatedmigration markersadd_documentpollsstatus == "completed"instead of the unreliableretrieval_readyflagdoc_idsacceptsstr | list[str] | None; empty list rejected at both Collection and Backend layersdoc_idsis provided): dropslist_documentsfrom the agent's tool set, hard-enforces whitelist on doc_id args, and injects doc summaries into the user message inside<docs>with system-prompt guidance treating it as data, not instructions (prompt-injection mitigation)Collection.list_documents()now exposesdoc_descriptionso the agent can route to the right docdoc_ids=Noneover a multi-doc collection emits aUserWarning(cross-doc retrieval is experimental;PAGEINDEX_EXPERIMENTAL_MULTIDOC=1to silence). Single-doc skipped, empty collection raisesValueErrorPageIndexClient.list_documentsdocstring spells out the return-shape difference vsCollection.list_documents(); clearer error when legacy methods are called afterapi_key=""PAGEINDEX_AGENTS_TRACING=1re-enables)examples/local_demo.py,examples/cloud_demo.py,examples/demo_query_modes.py(5 query-mode cases)dist/to gitignoreTest plan
pytest tests/— 101 passed, 2 skippedexamples/demo_legacy_sdk.py— all 7 legacy methods green againstapi.pageindex.aiexamples/demo_query_modes.py— all 5 Collection.query modes (single/multi/scoped × 2, env-var silencing) green