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 53 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
…ardening, flaky tests Addresses items 9-13 and a/b/c/f from the max-effort review of PR #272. - pdf.py: image colorspace check was `pix.n > 4`, which treats CMYK-without- alpha (n==4, same as RGBA) as not needing RGB conversion; pix.save() as .png then raises "unsupported colorspace", silently dropped by the surrounding except. Fixed to `pix.n - pix.alpha >= 4` (correctly converts CMYK, leaves RGBA untouched). - pipeline.py: detect_strategy([]) (an empty/whitespace-only source file) returned "content_based", routing into the PDF-oriented TOC-detection pipeline -- wasting a real LLM call before raising IndexingError. Empty node lists now route to level_based, whose build_tree_from_levels([]) returns an empty structure instantly with zero LLM calls. - page_index.py (shim): pageindex/__init__.py binds the canonical `page_index` function as the package attribute, but this file is ALSO a real submodule of the same name -- importing it anywhere (import machinery, unconditional) overwrites that attribute with the module object, breaking `from pageindex import page_index; page_index(x)` for the rest of the process. Made the shim module itself callable (delegates to the real function via a ModuleType subclass), so whichever object ends up in that slot is callable regardless of import order. - storage/sqlite.py: create_collection let a raw sqlite3.IntegrityError escape on a duplicate name (new CollectionAlreadyExistsError); the collections table's CHECK constraint only validated the name's first character (GLOB '*' is a wildcard, not a regex quantifier over the preceding class) -- fixed to validate the whole string, and SQLiteStorage now also validates in Python (it's a public StorageEngine usable directly, bypassing LocalBackend's own check). - tests/test_review_fixes_2.py: two tests used a ContentNode with no `level` set, so build_index took the content_based path and made real (retried, slow, and -- with a valid key -- billable) LLM calls instead of testing the text-stripping logic they claimed to. Mocked out _content_based_pipeline. - retrieve.py: _parse_pages/_get_pdf_page_content were independent copies of the canonical parse_pages/get_pdf_page_content that had already drifted (missing the p>=1 filter and 1000-page DoS cap) -- delegate to canonical now, so the legacy pageindex.get_page_content path can't silently regress again. - parser/markdown.py: a leading UTF-8 BOM broke first-header detection (not whitespace, .strip() doesn't remove it) -- decode utf-8-sig. Only backtick fences were recognized as code blocks, so a '#'-prefixed line inside a ~~~-fenced block (valid CommonMark) was misparsed as a heading -- recognize both fence styles. - run_pageindex.py: --if-thinning wasn't migrated to the bare-flag + legacy-yes/no convention the other four --if-add-* flags got; bare usage raised an argparse error and it never went through the shared coercion. - types.py: DocumentDetail's `structure` field was inside the class's total=False body, so TypedDict rules made it optional even though every backend always populates it. Split into a required base class. Adds regression tests for all of the above. Full suite: 244 passed, 2 skipped (one pre-existing, unrelated flaky cloud-streaming test). Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
pyproject.toml had no dev/test dependency group at all, so pytest wasn't declared anywhere — installing strictly per pyproject.toml (poetry install, or pip install . in a clean env) left `pytest tests/` failing with ModuleNotFoundError. Added [tool.poetry.group.dev.dependencies]. pytest-asyncio, though installed locally, isn't actually required: no test in this suite uses `async def test_...` / @pytest.mark.asyncio, they all drive async code via plain asyncio.run() inside sync test functions — so only pytest itself is declared. Verified: `poetry check` accepts the new section (only pre-existing, unrelated [tool.poetry] vs [project] deprecation warnings); `python -m build` still succeeds; the built wheel's METADATA does not list pytest as a runtime dependency (dev group is correctly excluded from the published package). Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
|
Status update on the remaining findings from the max-effort review (previous two comments covered the fixed ones) — these are deliberately deferred or left as-is, not overlooked: Correcting an earlier verdict — Deferred pending the real cloud API (not a code fix yet):
Deliberately left as-is:
|
asyncio.to_thread(ceiling_sem.acquire) blocks a worker thread that can't be interrupted. If the awaiting coroutine is cancelled (Ctrl-C, an outer timeout) while that thread is still parked inside acquire(), the thread can go on to actually acquire the permit after the coroutine has already unwound — leaking it forever, since the matching finally: release() never runs for that attempt. Poll with the non-blocking acquire(False) form instead, which returns instantly and closes the leak window entirely.
A stalled or half-open connection (e.g. a flaky local proxy that keeps a socket ESTABLISHED but never sends data) could hang indexing forever with no error — litellm/httpx had no timeout applied. Add a default per-request timeout (120s, tunable via PAGEINDEX_LLM_TIMEOUT or set_llm_params(timeout=...)) so a hung call fails fast with a clear litellm Timeout, which the existing retry loops surface. Rides get_llm_params(), so it flows through both llm_completion and llm_acompletion automatically and is overridable per-index via llm_params_scope / IndexConfig(llm_params=...).
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e00d360273
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Code review 发现的问题这次 review 主要发现 4 个需要处理的问题：
验证情况：
|
代码审查经 5 个独立审查代理交叉验证，共发现 12 个问题，按置信度排序如下： 问题 1（置信度 90）—
|
rejojer
left a comment
rejojer
left a comment
There was a problem hiding this comment.
Code Review Findings
Reviewed 66 changed files at max effort (64 agents, 56 candidates found, 46 survived adversarial verification, 15 reported).
🔴 Critical — Crashes / Data Corruption
1. pageindex/index/utils.py:157 — LLM retry exhaustion now crashes the pipeline
llm_completion / llm_acompletion changed from returning "" on max-retry exhaustion to raising RuntimeError. None of the ~12 callers (toc_transformer, extract_toc_content, toc_detector_single_page, etc.) handle this exception. A transient LLM outage lasting longer than 10 retries now crashes the entire page_index() call instead of continuing with partial/degraded output.
2. pageindex/storage/sqlite.py:214 — Thread-safety bug in close()
close() only deletes the calling thread's self._local.conn attribute. Other threads' threading.local() conn attributes survive — if those threads later call _get_conn(), hasattr(self._local, 'conn') is True and returns the closed connection, causing sqlite3.ProgrammingError: Cannot operate on a closed database.
3. pageindex/backend/local.py:259 — Empty-list doc_ids bypasses security scope
set(doc_ids) if doc_ids else None treats doc_ids=[] as falsy (= None = open mode). Calling get_agent_tools(collection, doc_ids=[]) directly gives the agent list_documents with no scope restriction — access to every document instead of none.
4. pageindex/index/utils.py:132 — Sync LLM calls bypass concurrency semaphore
llm_completion (sync path) never acquires _llm_semaphore — only llm_acompletion (async) is bounded. With if_add_doc_description=True (new default), generate_doc_description uses sync llm_completion. Multiple threads indexing concurrently run uncapped, exceeding max_concurrency and hitting provider rate limits.
5. pageindex/index/utils.py:55 — Semaphore replacement orphans in-flight permits
set_max_concurrency() replaces the Semaphore object. In-flight coroutines still hold permits on the old (discarded) semaphore and will release() on it, not the new one. Effective concurrency briefly exceeds the intended cap.
🟠 Backward-Incompatible Changes (silent breakage for existing users)
6. pageindex/index/utils.py:875 — config.yaml no longer loaded from disk
ConfigLoader no longer reads config.yaml. Users who customized it (e.g. changing model or toc_check_page_num) will have their overrides silently ignored — the system uses IndexConfig's hardcoded defaults instead.
7. pageindex/config.py:29 — if_add_doc_description default flipped from False to True
Legacy callers using page_index() without explicitly passing this flag now get doc descriptions generated (extra LLM calls, increased cost/latency, different output shape with doc_description key).
8. pageindex/retrieve.py:46 — Markdown page-content retrieval semantics changed
Old code: range-based matching (all nodes with line_num in [min, max]). New code: exact-match only. get_page_content(docs, doc_id, '5,20') previously returned all nodes between lines 5–20; now returns only nodes at exactly line 5 and line 20, silently dropping everything in between.
🟡 Cloud Backend Issues
9. pageindex/backend/cloud.py:157 — list_collections missing 403/404 handling
Does not handle _FOLDER_UNAVAILABLE for plans without folder support, unlike create_collection, get_or_create_collection, _get_folder_id, and delete_collection which all gracefully return empty results.
10. pageindex/backend/cloud.py:208 — doc_type hardcoded as 'pdf'
get_document and list_documents always return doc_type='pdf' regardless of actual file type. Downstream code branching on doc_type (page-number vs line-number semantics) misclassifies Markdown docs.
11. pageindex/cloud_api.py:88 — Python bool capitalization in URL query params
get_tree interpolates Python True/False into URL, producing summary=True (capital T) instead of summary=true. If the API is case-sensitive (common), summaries are silently omitted. The newer CloudBackend correctly passes lowercase 'true'.
🟡 Parser Issue
12. pageindex/parser/markdown.py:38 — Code fence type not tracked
A backtick-opened fence can be "closed" by a tilde line (and vice versa), violating CommonMark spec. Lines inside the still-open fence are then incorrectly parsed as headings.
💡 Cleanup / Performance
13. pageindex/backend/cloud.py:187 — add_document blocks thread up to 10 minutes
Uses time.sleep(5) in a 120-iteration polling loop. In web server or async contexts, this starves the event loop / thread pool.
14. pageindex/backend/cloud.py:239 — get_page_content fetches all pages then filters client-side
For a 500-page PDF requesting pages 5–7, the entire OCR result is downloaded and filtered locally, wasting bandwidth and memory.
15. pageindex/client.py:53 — BASE_URL defined three times
PageIndexClient.BASE_URL, LegacyCloudAPI.BASE_URL, and cloud.API_BASE are independent copies of 'https://api.pageindex.ai'. Changing the URL for staging or migration requires updating all three.
Automated review by Claude Code — 64 agents, max effort.
Python's $ anchor matches just before a final newline, so a $-anchored
re.match(r'^[a-zA-Z0-9_-]{1,128}$', name) accepted "papers\n". In local
mode get_or_create_collection() then hit SQLite's CHECK via
INSERT OR IGNORE, silently created no row, and returned a Collection that
failed later on add(). Switch all three duplicated validators (local,
cloud, sqlite backends) to re.fullmatch() so the whole string must match.
The local get_page_content agent tool only converted DocumentNotFoundError
into a JSON error; a malformed page spec ("all", "5-") let parse_pages'
ValueError surface as the agent SDK's generic non-fatal tool-failure text
("An error occurred... invalid literal for int()"), which the model can't
act on. Catch (ValueError, AttributeError) and return the same actionable
"Invalid pages format: ... Use '5-7', '3,8', or '12'" message the legacy
retrieval tool already gives, so the model can retry with a valid range.
Cloud OCR page results carry an `images` list per page, but the page reconstruction only kept `page` and `content`, dropping images for cloud callers of collection.get_page_content(). The local backend preserves them and the PageContent contract / SDK prompts expect them (so the downstream UI can render figures). Pass `images` through, omitting it when empty to mirror the local backend's shape. Verified against the real OCR endpoint: per-page keys are page_index/markdown/images.
chat_completions(stream=True, stream_metadata=True) used stream_metadata only to pick the raw dict-chunk parser locally, never adding it to the request payload. The wire request didn't match the caller's intent and relied on the server sending metadata chunks unconditionally. Forward the flag (mirroring the modern CloudBackend, which always sends it) so the request is correct and robust if the server ever gates metadata behind it. Verified against the real API that the server currently emits block_metadata regardless, so this is a latent-correctness fix, not a behavior change today.
The _no_sleep autouse fixture patches time.sleep on the shared time module object, so the SlowResponse pacing (`import time; time.sleep(0.002)`) was a no-op — the background thread raced to drain all 1000 chunks before the consumer's early break propagated, failing `assert not drained_all.is_set()` intermittently. Capture the real sleep at import (before the fixture patches) and pace with it, restoring the 2s drain vs ms-teardown margin the test needs. Production stop logic was correct; only the test's pacing was broken.
The source-install path documented in the README (pip install -r requirements.txt) was missing runtime deps the new SDK imports on `import pageindex` — pydantic (config), typing-extensions (client), requests (cloud/legacy API) — plus openai and httpx[socks]. A user following that path hit ModuleNotFoundError before they could use even the legacy APIs. Add them (openai-agents stays an optional install, matching the README's agentic-demo section). Verified in a clean venv: `import pageindex` and the legacy/core APIs now work with only requirements.txt installed.
delete_document unconditionally called response.json(), so a successful
DELETE that returns 200 with an empty body (the documented examples don't
consume one, and REST APIs commonly return no content for deletes) would
raise JSONDecodeError even though the document was already deleted. Return
{} when the response has no content, else parse the JSON body as before.
|
The sync-concurrency fix marked the scoped-override semaphore for release before it was actually acquired, so a coroutine cancelled while polling for a scoped permit (or a sync acquire interrupted mid-wait) ran the finally and released a permit it never held — inflating the scoped cap for later calls (the mirror of the ceiling-leak fix). Only bind the release guard after the acquire succeeds, in both the async and sync semaphores. Regression test included: cancelling a waiter leaves the scoped permit count at 1, not 2.
llm_completion/llm_acompletion raised RuntimeError once retries were
exhausted, which propagated through the unprotected sync TOC-detection
chain (find_toc_pages -> toc_transformer -> toc_extractor -> ...) and
aborted the whole document index — a regression vs. the pre-SDK behavior
that returned "" and let callers degrade (extract_json('') -> {} ->
.get(default), falling back to no-TOC indexing).
Restore the empty-result contract ("" / ("", "error") with
return_finish_reason), now logged at WARNING so the failure is visible
rather than silent. The async gather sites keep return_exceptions=True to
absorb any non-LLM error. A single persistently-failing call no longer
blocks indexing the rest of the document.
OPEN_SYSTEM_PROMPT and SCOPED_SYSTEM_PROMPT told the agent to call
get_document(doc_id) "to confirm status and page/line count", but neither
backend returns a page/line count and the local backend has no status field
(get_document returns doc_name/doc_type/doc_description). The agent would
hunt for fields that don't exist, degrading QA. Align both prompts with the
demo's wording ("confirm the document's name and type"). Regression test
asserts the prompts no longer reference the non-existent page/line count.
…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.
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