feat: introduce PageIndex SDK with Collection-based local/cloud API#272
feat: introduce PageIndex SDK with Collection-based local/cloud API#272KylinMountain wants to merge 71 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}
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
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
|
跟进 之前的 max-effort 复审：
|
| 复审条目 | 修复 |
|---|---|
4 _get_text_of_pages 越界 |
循环范围 clamp 到 [1, len(page_list)]；同模式的 get_text_of_pdf_pages_with_labels 一并 clamp |
14–15 get_text_of_pdf_pages 重复 |
改为转调 _get_text_of_pages，越界修复只需一处 |
5 _normalize_tree(None) |
开头 if not nodes: return []；验证时发现同类未防护点又补了 4 处——folders 迭代 ×3、documents 分页 ×1（API 对这些 key 返回显式 null 同样会 TypeError） |
| 8 页码类型不匹配 | _as_int() 强转后再比较，转不了则跳过；返回的 page 字段统一为 int（对齐 PageContent.page: int） |
| 9 folder 缓存存 None | 新增 _create_folder()：响应缺 folder.id 时 raise（PageIndexError，不会被 except CloudAPIError 吞掉）；_get_folder_id 查不到名字不再缓存，folder 后建后可被发现。403/404 "plan 不支持" 的哨兵缓存保持原样 |
| 10 doc_id 未 defang | wrap_with_doc_context 中 doc_id 过 _defang_delimiters |
| 6（部分） | 401 错误信息统一追加提示："api_key 是 PageIndex cloud key，本地模式请设 LLM provider 的环境变量"。覆盖三条路径：CloudBackend._request、LegacyCloudAPI._request、query_stream 的流式 POST。特意只挂 401 不挂 403——本代码库将 403 定义为 plan 限制（_FOLDER_UNAVAILABLE），key 正确的用户收到 key 提示是误导 |
| 7（收尾） | 删除 examples/workspace/ 里旧 JSON 格式的示例数据（_meta.json + 文档 JSON）。dev 上两个 demo 的 storage_path 都指向该目录，旧文件是 SQLite 存储读不了的死数据，留着会让 demo 用户在空库旁边看到误导性的遗留文件 |
两个兼容性问题的定案
6. api_key 语义变更 → 不做旧语义兼容。 考证结论：api_key=<OpenAI key> 这个用法在整个仓库历史中（全部分支、全部 commit）从未出现在任何 README、demo、cookbook 或 docstring 中——唯一存在过的地方就是 main 上 client.py 的实现代码本身，仓库内调用者为零（demo 只传 workspace）。cookbook 里所有 api_key= 示例传的都是 PageIndex key（PyPI SDK 语义）。已发布的 0.2.x SDK 契约优先，开源侧未发版的隐藏行为不构成兼容义务。残留措施就是上面的 401 提示。
7. workspace 参数移除 → 不做兼容检测/迁移。 考证结论：JSON workspace 存储从未进过任何 PyPI 发行版，唯一使用者是仓库自己的 demo，而 dev 上的 demo 已改用 storage_path。为一个只有 demo 用过的参数在产品代码里加检测不值得；直接删掉遗留数据即可。建议 0.3.0 release notes 里加一句："本地存储改为 SQLite，旧 demo 的 JSON workspace 数据不再可读，需重新索引"。
验证
每个修复有针对性单元测试（含 clamp 前后在合法输入上的逐一等价性比对）；另跑了一轮独立的对抗式审查（其发现的流式 401 路径遗漏和 403 误导问题已在本 commit 中修正）；全测试套件与修复前的 HEAD 对照运行，通过/失败完全一致（12 个失败为既有的 openai-agents/pydantic 环境不兼容，与本改动无关）。
未处理（可选清理项，留待后续）
复审条目 11–13：云端 query 的双重 list_documents、doc_ids 标准化 ×4、_validate_collection_name ×3。均为重复代码/多余请求，不影响正确性。
另外验证中发现一个与条目 4 同类的既有问题供参考：page_index.py 的 check_title_appearance 用 LLM 输出直接索引 page_list，无 clamp（上界越界被 gather(return_exceptions=True) 吞掉，下界负索引会静默读错页）。未在本 commit 处理。
|
补充一个复审时漏判的兼容性问题：config.yaml 的移除应该做兼容，和 api_key / workspace 两个"不做兼容"的定案不是一类。 为什么这个不一样前两个定案的依据是"未文档化、仓库内零调用、发现它得翻源码"。config.yaml 不满足这个条件：
建议方案
即 CLI 侧完全向后兼容，SDK 侧保持新架构不妥协。 顺带两个相关的小观察（无需行动，release notes 提一句即可）：
|
run_pageindex.py users configure indexing by editing pageindex/config.yaml; dev silently ignored those edits. Restore the file and load it as the IndexConfig base when present, CLI args winning — same merge semantics and package-relative path as the pre-0.3 ConfigLoader. The SDK stays explicit-args-only.
Code reviewRe-reviewed at max effort with Opus agents. Found 3 issues (all verified real, but edge-case / hardening severity rather than blocking):
PageIndex/pageindex/backend/cloud.py Lines 198 to 200 in f995e64 (convention it diverges from: PageIndex/pageindex/backend/cloud.py Lines 58 to 61 in f995e64
PageIndex/pageindex/backend/cloud.py Lines 174 to 177 in f995e64
PageIndex/.github/workflows/publish.yml Lines 43 to 51 in f995e64 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
|
这几个问题 我都有回复过，第一个幂等 暂不考虑，如果觉得要修复，后续pr在修复。 第二个 folder 也是作为后续pr修复 当前pr没必要。 第三个
不是问题。
我觉得赶紧合并吧，有问题再修。 以后要避免这种为了一个东西 改了整个repo 包括bug的pr…
我们只是合并 不是打正式tag…
Ray ***@***.***>于2026年7月13日 周一21:29写道：
… *rejojer* left a comment (VectifyAI/PageIndex#272)
<#272 (comment)>
Code review
Re-reviewed at max effort with Opus agents. Found 3 issues (all verified
real, but edge-case / hardening severity rather than blocking):
1. Non-idempotent upload is retried, risking a duplicate document and
double billing. _request defaults to retries=3 and rewinds the file
body (seek(0)) so the multipart re-sends, but add_document's POST /doc/
is non-idempotent — if the server commits the upload and the client then
sees a timeout/5xx/429, the retry creates a second document. _request's
own docstring says to pass retries=1 for non-idempotent, expensive
calls, which query() does but this upload doesn't. Consider retries=1
(or an idempotency key / server-side content-hash dedup like the local
backend).
https://github.com/VectifyAI/PageIndex/blob/f995e6441e5c3b6799007c0015c9a7f055e87f81/pageindex/backend/cloud.py#L198-L200
(convention it diverges from:
https://github.com/VectifyAI/PageIndex/blob/f995e6441e5c3b6799007c0015c9a7f055e87f81/pageindex/backend/cloud.py#L58-L61
)
2. list_collections() doesn't degrade on folder-unavailable plans. It
calls GET /folders/ with no _FOLDER_UNAVAILABLE (403/404) handling, so
on a plan without folder support it raises CloudAPIError, whereas
every sibling (create_collection, get_or_create_collection,
_get_folder_id, delete_collection) catches (403, 404) and degrades
gracefully. Returning [] would match the PR's own degradation model.
https://github.com/VectifyAI/PageIndex/blob/f995e6441e5c3b6799007c0015c9a7f055e87f81/pageindex/backend/cloud.py#L174-L177
3. publish.yml interpolates a tag-derived $VERSION into shell/Python
(command injection). VERSION="${GITHUB_REF_NAME#v}" is spliced into python
-c "... Version('$VERSION')" and sed -i "...$VERSION..."; the PEP 440
Version(...) check is itself the injection point, so it provides no
protection. Exploitation requires repo-write (tag push), but the job holds id-token:
write + contents: write, so RCE here is high-blast-radius (PyPI
publish / OIDC token). Pass the value via env: and reference it as
os.environ / a quoted "$VERSION" argv instead of string-interpolating.
https://github.com/VectifyAI/PageIndex/blob/f995e6441e5c3b6799007c0015c9a7f055e87f81/.github/workflows/publish.yml#L43-L51
🤖 Generated with Claude Code <https://claude.ai/code>
- If this code review was useful, please react with 👍. Otherwise, react
with 👎.
—
Reply to this email directly, view it on GitHub
<#272?email_source=notifications&email_token=AA6PURJ4FZOIL7PRETGWVCL5ETP37A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIOJVHA2DKOJUGQ42M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#issuecomment-4958459449>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA6PURIV54EFCWI6KVRX7Q35ETP37AVCNFSNUABFKJSXA33TNF2G64TZHM4TKOBVGMYTAOBZHNEXG43VMU5TINBSGYYDKNZTGY3KC5QC>
.
Triage notifications, keep track of coding agent tasks and review pull
requests on the go with GitHub Mobile for iOS
<https://github.com/notifications/mobile/ios/AA6PURLVHSCPQV4UAIV7KTL5ETP37A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIOJVHA2DKOJUGQ42M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KUZTPN52GK4S7NFXXG>
and Android
<https://github.com/notifications/mobile/android/AA6PUROFTHFQOIVPUO4WVGD5ETP37A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIOJVHA2DKOJUGQ42M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2K4ZTPN52GK4S7MFXGI4TPNFSA>.
Download it today!
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
GET /folders/ is gated by the folder plan server-side, so on plans without folder support list_collections raised CloudAPIError while every sibling folder method (create_collection, get_or_create_collection, delete_collection) degrades gracefully. Catch _FOLDER_UNAVAILABLE, emit the one-time upgrade warning, and return []; transient errors still propagate.
config.yaml was restored as the CLI config base, but the ConfigLoader docstring and a test docstring still claimed it no longer ships, and the --if-add-doc-description help said 'on by default' while the config.yaml base sets it to 'no'.
- cloud: _get_folder_id raises CollectionNotFoundError when the folder is gone instead of silently falling back to the account-wide space; delete stays idempotent - cloud: CloudBackend honors a base_url override (PageIndexClient.BASE_URL previously only reached the legacy API) - cloud: query_stream runs doc-id listing and thread join off the event loop (asyncio.to_thread) - cloud_api: non-streaming chat_completions gets a 300s read timeout - local: unexplained IntegrityError is wrapped as CollectionNotFoundError/ IndexingError instead of leaking raw sqlite3 errors - index: check_title_appearance rejects below-range physical_index instead of wrapping to the last page; page_index_main coerces legacy 'yes'/'no' string flags - config: correct max_concurrency doc and warn when a per-index value exceeds the process ceiling - compat: restore pageindex.utils config alias; add legacy names back to __all__
Both indexing paths and the CLI now use it; page_index_md re-exports it for existing importers.
CloudBackend ignored the collection argument on get_document, get_document_structure, get_page_content and delete_document, so a doc_id addressed through the wrong collection was served or deleted globally — deletion could destroy a document in another collection. Add _require_document (mirroring LocalBackend): compare the doc's folderId against the collection's folder and raise DocumentNotFoundError on mismatch. get_document reuses its existing metadata call, so no extra round-trip there; plans without folders skip the check. Legacy client methods keep global-by-id semantics. Pin the contract in the Backend protocol.
- process_toc_with_page_numbers: an unresolvable page offset (no
TOC/body title matches) returned None and crashed with TypeError;
now returns items without physical_index so meta_processor falls
back to the no-page-number mode
- process_none_page_numbers: tolerate malformed add_page_number_to_toc
output ({}, non-dict items, non-numeric physical_index tags) instead
of KeyError/AttributeError/ValueError
- validate_and_truncate_physical_indices: invalidate non-int
physical_index values instead of raising on str > int
- wrap_with_doc_context: doc_name=None no longer crashes _defang_delimiters
- build_tree_from_levels: level=0 is a valid level; stop coercing it
to 1 via falsy-or, which flattened one level of tree depth
Code reviewRe-reviewed at max effort with Opus agents (5 review dimensions; 6 candidate findings independently cross-scored, one cleared the confidence bar). Found 1 issue:
PageIndex/pageindex/backend/cloud.py Lines 345 to 348 in 3a780ab 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
Code reviewCorrection (edited): I retracted an earlier finding that questioned the Found 4 issues, cross-verified against current HEAD ( 1. PageIndex/pageindex/backend/cloud.py Lines 382 to 393 in 6d42559 2. Cloud PageIndex/pageindex/backend/cloud.py Lines 213 to 217 in 6d42559 3. Local streaming emits multiple Lines 128 to 133 in 6d42559 4. Lines 242 to 244 in 6d42559 Lines 25 to 28 in 6d42559 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
Cloud query()/query_stream() now run the same _require_document membership guard the four doc-scoped ops received in 929b3df, so a doc_id from another collection raises DocumentNotFoundError (matching the local backend's _scoped_docs) instead of silently answering from another collection's document. The guard runs after doc_ids normalization and before the completion request; query_stream runs it via asyncio.to_thread. Folders unavailable on the plan -> guard skips, as with the sibling ops. Also rename the streaming QueryEvent types answer_delta/answer_done -> text_delta/text_done: text_done fires once per assistant text message, which is honest for the local agent loop's multi-message stream (the last one before the stream ends carries the final answer). Corrects the cloud backend's now-inaccurate "single terminal contract" comment. reasoning stays reserved for future model-reasoning tokens. QueryEvent is unreleased API (0.3.0.dev1), so the rename is not a breaking change.
The local SDK path builds IndexConfig directly (not from config.yaml), so retrieve_model defaulting to None made agentic QA silently follow the cheaper indexing model (gpt-4o) instead of the stronger gpt-5.4 the packaged config and CLI use. Set the IndexConfig field default to gpt-5.4; retrieve_model=None still follows model as an explicit escape hatch. Cloud path and the index-only CLI are unaffected.
| ceiling_sem.release() | ||
|
|
||
|
|
||
| def llm_completion(model, prompt, chat_history=None, return_finish_reason=False): |
|
|
||
|
|
||
|
|
||
| async def llm_acompletion(model, prompt): |
| return data | ||
|
|
||
|
|
||
| def structure_to_list(structure): |
| return nodes | ||
|
|
||
|
|
||
| def get_nodes(structure): |
| return nodes | ||
|
|
||
|
|
||
| def get_leaf_nodes(structure): |
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
main's changes landed on the top-level pageindex/page_index.py and page_index_md.py, which are deprecation shims on dev (the pipeline now lives in pageindex/index/). Resolving the conflict by keeping the shims would silently drop main's work, so port it into index/ instead: - page_index.py: prompt-injection hardening (_SYSTEM_HARDENING, _secure_doc_text and the redact/wrap helpers) applied to every prompt builder, plus physical_index validation (_validate_physical_indices, _validate_chunk_physical_indices) wired into toc_index_extractor, process_no_toc and process_toc_no_page_numbers, and TOC-identity validation (reject reordered/modified/count-mismatched LLM output). Combined with dev's existing robustness edits in the same functions. - page_index_md.py: recognize whole-line **bold** as a level-1 heading, skip empty bold headings, use the stored node level. - tests: retarget the ported tests at pageindex.index.* instead of the shim (underscore helpers aren't re-exported by the shim's `import *`, and patching the shim wouldn't intercept the real pipeline calls). 311 passed, 2 skipped.
The prompt-injection hardening ported from main did not fit dev's direction
(graceful degradation + content-faithful, reasoning-based index). Adjust it:
- process_toc_no_page_numbers: a count-mismatched or reordered/renamed LLM
response no longer raises ValueError (which aborted the entire index at the
uncaught top-level path and defeated the return_exceptions degradation the
rest of the pipeline uses). Skip the untrusted chunk and continue; the
accuracy check falls back to another mode when too little gets filled.
- _secure_doc_text: drop the keyword blocklist that replaced phrases like
"act as"/"disregard" with [REDACTED]. Those occur in legitimate titles and
prose, so redaction corrupted document content. Keep the <user_document>
framing + _SYSTEM_HARDENING system instruction as the injection defense.
- _validate_chunk_physical_indices: tolerate non-list input (extract_json
returns {} on parse failure and may return a JSON object) instead of
crashing while iterating a dict.
- Remove _validate_physical_indices / _parse_physical_index: range validation
is already done by validate_and_truncate_physical_indices in meta_processor
and marker->int by convert_physical_index_to_int; the helpers duplicated both.
Tests updated to assert the skip-not-raise behavior and lock in no-redaction
and non-list tolerance. 314 passed, 2 skipped.
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
…ties - cloud.py: replace bare resp['doc_id'] with .get() + CloudAPIError - local.py: case-insensitive doc_type comparison for .PDF extensions - utils.py: add else branch to get_pdf_name for non-str/BytesIO inputs - utils.py: raise ValueError in get_page_tokens when PyMuPDF path invalid
…tics PageIndexClient snapshotted BASE_URL into CloudBackend and LegacyCloudAPI at construction, so reassigning client.BASE_URL afterwards was silently ignored and requests kept going to the default endpoint. Pass a callable resolved per request instead, so instance- and class-level reassignment after construction work again alongside the pre-construction override. Claude-Session: https://claude.ai/code/session_014B4HZkjdSiZXDmJtH5Jexn
…ccess page_index() and ConfigLoader silently dropped config.yaml after the refactor: defaults came from IndexConfig's hardcoded fields, flipping if_add_doc_description on (an extra billed LLM call per document) and discarding user-edited YAML including a custom default_path. Both now resolve explicit args > YAML > IndexConfig field defaults via IndexConfig.from_yaml, matching the CLI. The new SDK keeps its pure-code config path. import pageindex also lost the utils / page_index_md submodule attributes (pageindex.utils.print_tree raised AttributeError). A module __getattr__ now imports the shims lazily, so plain imports stay free of deprecation warnings while first use of a legacy attribute binds the module and warns. Also trims non-essential comments from the BASE_URL fix. Claude-Session: https://claude.ai/code/session_014B4HZkjdSiZXDmJtH5Jexn
Code reviewFound 1 issue:
Lines 242 to 244 in 0f593c6 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
…mary threshold - demos: use the post-cutoff attention-residuals paper (2603.15031) again so answers provably come from retrieval, and run the QA agent on retrieve_model - client: re-expose model / retrieve_model as public attributes (0.2.x surface) - pipeline: markdown summaries go back through generate_summaries_for_structure_md with the 200-token threshold (small nodes skip the LLM call) - README: restore (yes/no, default: X) flag docs with correct defaults; note that retrieve_model drives agent QA
Notes on a0fb863 — restore pre-refactor behaviorsThis commit restores a few behaviors from the pre-refactor codebase that changed as side effects of the SDK rewrite, plus two documentation fixes. 1. Demo paper back to The demo paper choice is deliberate: it postdates model training cutoffs, so a correct answer demonstrates that the content actually came from document retrieval. With 2. Demo QA agent runs on Before the rewrite, 3. The resolved values were previously inlined into the 4. Markdown summaries: 200-token threshold restored
5. README CLI flag docs Restored the Tests: 320 passed, 2 skipped. 🤖 Generated with Claude Code |
5 participants
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) green0.2.x behavior notes
Compatibility regressions found in review are fixed (
dea211b,0f593c6):page_index()/ConfigLoaderreadconfig.yamlagain (explicit args > YAML >IndexConfigfield defaults, soif_add_doc_descriptionstays off by default on the legacy path),ConfigLoader(default_path=...)is honored (missing file raisesFileNotFoundError),pageindex.utils.*/pageindex.page_index_md.*attribute access works again (lazily bound — plainimport pageindexstays warning-free), andclient.BASE_URLreassignment after construction routes requests again.Three 0.2.x behaviors are intentionally not preserved:
get_page_content(..., "3,8")on a Markdown doc returns the nodes at lines 3 and 8, mirroring the PDF path — 0.2.x returned everything in the [3, 8] envelope. Use"3-8"to request a range."1-1500") return an actionable error instead of expanding — split into chunks ("1-1000","1001-1500"). The spec is LLM/caller-reachable, so unbounded expansion was a DoS vector (a"1-2000000000"spec previously allocated billions of ints).from pageindex import *is narrowed to__all__. The old star surface was an accident of chained star-imports (it even leakedjson/os/yaml). Explicit forms are unaffected:from pageindex import extract_json,pageindex.extract_json, andfrom pageindex.utils import ...all keep working.https://claude.ai/code/session_014B4HZkjdSiZXDmJtH5Jexn