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 65 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
…e base URL - local get_agent_tools: `set(doc_ids) if doc_ids is not None else None` so doc_ids=[] is a scope of nothing (reject all), not open mode. The public query path already guarded []; this hardens direct callers. - legacy get_tree: send summary=true/false (lowercase) instead of Python's capitalized True/False, matching the modern CloudBackend and the API. - markdown parser: track the opening fence character so a ```-fence isn't closed by a ~~~ line (CommonMark), keeping '#'-lines inside it out of headings. - dedupe the cloud base URL: single API_BASE in cloud_api, referenced by CloudBackend and PageIndexClient (was three independent copies). Regression tests for each.
✅ 已修复
✅ 已修复
✅ 已修复
✅ 已修复
🟢 有意为之，不改：新 SDK 的默认，README 已同步（"on by default"）。
✅ 已修复
🟢 不修：推 tag 需仓库写权限，且不带来超出"能推 tag 者已有权限"的提权；触发器是
🕒 暂不处理：预留但未实现的枚举值，无功能影响。
🕒 推迟：等云端 workspace API 定稿后在后续 PR 处理。
✅ 已修复
✅ 已修复
⚪ 确认误报，无需处理。 |
|
Thanks for the max-effort pass — went through all 15 against current
✅ Fixed
🕒 Low-impact edge, deferred.
✅ Fixed
✅ Fixed
🟢 Accepted tradeoff — resizing happens only on an explicit
🟢 Intentional — the new SDK deprecates
🟢 Intentional SDK default, documented in the README ("on by default").
🟢 Not a regression — matches
🕒 Deferred — part of the cloud folder/workspace surface that's waiting on the finalized API; will align in the follow-up PR.
🕒 Deferred — same cloud-workspace follow-up; will surface the real type once the API exposes it.
✅ Fixed
✅ Fixed
🕒 By design for the synchronous "index and wait" call; use
🕒 The cloud OCR endpoint returns the whole document (no page-range parameter), so filtering is client-side by necessity. Low priority pending an API-side range filter.
✅ Fixed |
|
To use Codex here, create a Codex account and connect to github. |
- index/utils.py: fix an asyncio deadlock in _sync_llm_semaphore. Sync LLM calls (check_toc / process_no_toc / toc_transformer) run on the event-loop thread nested inside the async meta_processor, and its blocking ceiling acquire could wait forever for a permit held by async _llm_semaphore holders that can only release it once the (now-frozen) loop runs. Take the slot non-blocking when on a running loop; keep the blocking acquire off-loop. - index/utils.py: bound parse_pages ranges before materializing range() into the list, so a huge span like '1-2000000000' is rejected up front instead of exhausting memory before the 1000-page cap is ever checked (DoS). - storage/sqlite.py: bump a generation counter on close() so a thread that cached a connection in thread-local storage reconnects on its next call instead of reusing a closed handle (ProgrammingError). - backend/cloud.py: after connect, bail out of the SSE background thread if the consumer already abandoned the stream, instead of draining it in the background. Adds regression tests for each fix.
Max-effort 复审 — 15 项发现（已逐条对照当前
|
- clamp LLM-derived page indices in _get_text_of_pages and get_text_of_pdf_pages_with_labels; dedupe get_text_of_pdf_pages - guard _normalize_tree and folders/documents iterations against explicit nulls in cloud API responses - coerce cloud OCR page numbers to int before filtering in get_page_content - folder cache: raise on missing folder id instead of caching None; stop caching name-not-found so later lookups can succeed - defang doc_id in agent doc-context prompt - append api-key hint to 401 errors (request, legacy and streaming paths) - remove stale legacy JSON workspace sample data unreadable by the SQLite storage
|
跟进 之前的 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.
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) green