review: the complete v0.2.10 line β agent tools, local chat, Flash default - #400
Open
rejojer wants to merge 65 commits into
Open
review: the complete v0.2.10 line β agent tools, local chat, Flash default#400rejojer wants to merge 65 commits into
rejojer wants to merge 65 commits into
Conversation
Four new client methods make PageIndex documents available to agent frameworks, in both modes, with the mode decided solely by the client constructor: - agent_tools(): plain functions (browse_documents, get_document, get_document_structure, get_page_content) matching the PageIndex cloud MCP server's tools/list β same names, schemas, descriptions, and JSON response envelopes β so agent prompts port unchanged between the cloud MCP connection and these in-process tools. Tools never raise; errors come back in the same envelope. remove_document ships behind include_management=False. - as_openai_tools(): the same tools wrapped for the OpenAI Agents SDK. - as_claude_mcp(): one mcp_servers entry for the Claude Agent SDK β cloud clients get the remote MCP config (the framework connects to api.pageindex.ai/mcp and discovers the full cloud tool set), local clients get an in-process SDK MCP server. - agent_instructions(doc_id=None): orchestration guidance for the agent's system prompt; doc_id (same shape as chat_completions) appends the target documents. submit_document() gains wait=True: poll get_document status until completed, raise on failed or after 30 minutes β the manual polling loop every cloud caller writes today spins forever on a failed document. Neither framework becomes a dependency: imports happen at call time with actionable errors, and pageindex[openai] / pageindex[claude] extras are floor-only pins. tests/data/cloud_mcp_contract.json freezes the tool contract; a parity test guards against drift. 36 new tests (95 total), plus a live OpenAI Agents SDK run over a seeded local store verifying the structure-first navigation flow end to end.
β¦mantics - Large-doc next_steps now says structure-first, consistent with tool descriptions and agent instructions - _remove_document fetches document list once instead of per-name - call_tool returns error envelope for unknown names instead of raising - _not_ready_error timed_out flag reflects actual wait outcome - openai_agents.py docstring corrected to match default (FunctionTools) - Removed unused ModelSettings import from demo
β¦data merge - McpBridge reads session/protocol headers under the lock (now RLock: _ensure_initialized posts while holding it). openai-agents runs sync tools on threads and executes parallel tool calls concurrently, so bridge functions genuinely race; a torn read sent a new session id with a stale protocol header. Measured: one session expiry under 8 threads cost 4 initializations before, minimal 2 after. - Session-expiry retry also resets the negotiated protocol version, so the re-handshake carries no stale MCP-Protocol-Version header. - browse_documents time sort pages list_documents natively instead of fetching the whole library to slice one window (relevance still needs the full list for scoring). - _await_completion: a status refetch that nulls out metadata no longer clobbers the listing's copy (setdefault was a no-op on existing None). - Structure tool reads the raw stored tree via a named LocalAPI raw_tree() seam instead of reaching into _api._store internals; drop the redundant deepcopy before _format_structure (store re-reads from disk, formatting builds fresh containers). - Shared pageindex/_version.py replaces _sdk_version duplicated in mcp_bridge and the Claude integration. Left as-is after source verification against the cloud MCP: first-page budget bypass, pageNum falsy-zero, and the page-gap fallback text are letter-for-letter cloud behavior β parity wins over local repair.
β¦lience, contract drift - _parse_page_spec bounds the requested span arithmetically (10k pages) before materializing it; pages="1-1000000000" previously expanded to a billion integers inside the caller's process. - Local submit_document uniquifies document names the way the cloud upload does (taken name -> _1.._99, then reject with the cloud's own message). Same-name duplicates broke name-addressed tools: resolution always picks the newest, so older duplicates were unreachable. - agent_instructions(doc_id=...) now fails loud when the pinned doc's name is shadowed by a newer same-name document (legacy stores predate the rename) β it previews resolution with the same _resolve_document the tools use, so the check cannot drift from actual behavior. - submit_document(wait=True) tolerates transient network errors, not just API errors; a dropped connection at minute 25 of a 30-minute wait no longer kills it. Third strike wraps into PageIndexAPIError per the documented contract. - The live contract-parity test compares full per-param schemas, not just names and descriptions. It immediately caught real drift the shallow check had been passing: the server now emits nullables as anyOf unions and stamps MAX_SAFE_INTEGER maxima on offset/part. Contract and snapshot updated to the served wire form; _annotation_for learned anyOf so bridge signatures stay Optional[str] instead of degrading to Any. Adjudicated, not changed: the allowed_tools wildcard example stays (docstring advice covers scoping; Ray's call), and raw-length response accounting stays (letter-for-letter cloud behavior, parity wins).
Compute PR #558 makes /doc/ return {"doc_id", "name"} carrying the
post-dedup-rename name. Mirror it end to end: local submit returns the
stored name, the client warns when it differs from the uploaded file
name (read via .get so older cloud servers stay compatible), the local
name-exhaustion check runs before indexing instead of after the LLM
spend, and the demo caches doc_id in a file instead of name-matching β
a renamed document made the name lookup re-index on every run.
The cloud MCP server publishes its agent instructions in the initialize result, adapted to each key's tool set. agent_instructions() previously returned the SDK's local-subset text in both modes β a silently forked copy that lacks the guidance for cloud-only tools (search_documents escalation, folders, images) and drifts as the server's prompt evolves. Cloud clients now serve the server's live instructions, captured from the initialize handshake on a per-client bridge shared with agent_tools() (one session, no extra request). An empty server response raises instead of silently substituting the subset text β same posture as the annotation-regression guard. The local constant stays as the honest subset for the in-process tools, with its provenance noted and a consistency test that every tool it names exists in the local registry.
sort="relevance" is cloud-side semantic ranking; the local substring imitation could satisfy the letter of the interface while silently missing semantically relevant documents. Per the honest-subset rule (same treatment as folders), local now returns the "not available here" envelope for sort="relevance" or a stray query, and the local instructions steer discovery through name/description matching plus full-library paging instead of prescribing a capability that does not exist here. The tool schema keeps the cloud contract verbatim, like folder_id: honesty lives in the runtime answer, not a forked contract.
"Not available here" read as a broken feature; the honest framing is that folders and semantic ranking exist on PageIndex cloud and are not in local mode yet. Both envelopes now say so and name the cloud client in next_steps, so agents relay an accurate story to the user.
The cloud-verbatim browse_documents description invites sort="relevance" and folder drilling, so a local agent's first semantic search attempt was a guaranteed dead end discovered only from the runtime error envelope. Local registration now appends a LOCAL MODE note to the description β the agent learns what is cloud-only before calling; the runtime envelope stays as the backstop for prompts that ignore descriptions. The cloud-facing contract stays byte-verbatim.
Appending a retraction to the cloud-verbatim description left the model parsing an instruction and its negation β and kept the cloud text recommending search_documents and get_folder_structure, tools that are not registered locally (get_page_content likewise pointed at get_document_image). Guidance now adapts to the local surface the way AGENT_INSTRUCTIONS already does: schema structure stays byte-identical to the contract (mechanically asserted by a strip-descriptions test), while local description strings teach only what works here and point to PageIndex cloud for the rest. A dead-reference test forbids local guidance from naming tools outside the local registry, so a contract refresh that reintroduces a cloud-only reference fails loudly.
folder_id, sort, query, and recursive were exposed locally with localized "cloud-only" descriptions, leaving the dead-end calls expressible and discovered at runtime. Schema constraints beat guidance: the local surface now serves the contract minus these parameters, so strict-schema frameworks make the calls inexpressible and a prompt that insists on sort="relevance" degrades to the bare call (the correct local behavior) instead of an error round-trip. The implementations still accept the hidden parameters and answer with the guided "works on PageIndex cloud" envelope β the backstop for direct call_tool callers and hosts without schema enforcement. wait_for_completion stays: seeded or torn stores can hold documents that are genuinely not completed. The structural guard now asserts the local schema equals the contract minus the documented hidden set, descriptions aside.
Three independent review passes over the agent-instructions increment surfaced six fixes: - The per-client bridge moved off the instance into a weak-keyed, lock-guarded module cache: cloud clients stay picklable (threading.RLock no longer rides on the client) and concurrent first calls can no longer construct duplicate bridges/sessions. - Blank or non-string initialize.instructions now hit the same honest error as a missing one β a whitespace-only or structured value could previously become the system prompt (or crash the doc_id append with a raw TypeError). - The invalid-sort envelope no longer prescribes sort="relevance" β the one error text that still taught the cloud-only value it would then reject. - "Page through the rest of the library" is emitted only when has_more is true; a fully-listed library no longer instructs a pointless call. - The mandatory full-library paging step now says limit: 50 β 6 calls instead of 30 on a 300-document library. - Docstrings and comments rescoped to what is actually true: the never-raise contract covers invocations the signatures accept (unknown params fail at the Python boundary; call_tool answers them with the guided envelope), recursive is accepted as the identity rather than errored, lenient framework arg models drop hidden params pre-call, and the module header no longer claims full schema parity. The capability-phrase guard now covers every local docstring, not just browse_documents.
The frozen contract guards tools/list, but the response envelopes the local tools emit were hand-built to mirror the cloud's and had no drift detector. A key-gated live test now asserts every field local emits exists in the live cloud response for the analogous call (top-level keys, next_steps, document entries, structure nodes, content entries). Guidance wording is deliberately localized and not compared. Verified green against the live server: local and cloud field structures currently match exactly.
β¦.10) Local mode gains managed document QA: an agent over the #393 local tool set, reachable through three wire protocols, each 1:1 with the backend and with no translation layer. - chat_completions(): standard chat.completions semantics on any OpenAI-compatible backend (openai-agents engine). Final answer only, cross-turn aggregated usage, streaming as text pieces or chunk dicts (the existing cloud signature, now implemented locally; model and max_turns are local-only additions). - responses(): the agentic surface β OpenAI Responses format, the tool process is standard output items, streaming forwards native events (tool outputs emitted as response.output_item.done, the way the platform streams its own server-side tools). Round-tripping output into the next input keeps provider prompt-cache prefix continuity and the agent's memory β live-verified: the follow-up call answered from round-tripped tool output with zero new tool calls. - messages(): Anthropic-native via the SDK's own tool runner (new pageindex[anthropic] extra, floor 0.68.0 verified for tool_runner/beta_tool(input_schema)). tool_use/tool_result round-trip is the format's native behavior; the envelope is the final message with aggregated usage plus the full new-turn sequence; the managed system blocks carry cache_control breakpoints. Shared skeleton: thin chat header + the local AGENT_INSTRUCTIONS (caller system content is appended, not rejected), the doc_id targeting block as a leading context item (factored out of build_agent_instructions), read-only toolset, structural-only validation (no arbitrary caps β backend limits govern), sampling params passed through, per-run tracing disabled, enable_citations rejected as cloud-only. Design basis is industry-standard formats rather than the cloud chat endpoint; responses()/messages() raise on cloud clients until the cloud converges. Tests run the real engines against scripted backends (a Model fake for openai-agents, a mock HTTP transport under the real anthropic SDK) with real tool execution against a seeded store, including the round-trip prefix-extension assertions on both engines.
Three independent review passes (bug scan, claims-vs-code, adversarial runtime probes) over the local-chat increment; every fix below was reproduced before being fixed. messages(): - A max_turns cut no longer duplicates the final assistant turn: the runner has already appended it when iterations exhaust, so the round-trip history carried a duplicate tool_use id and ended on an unanswered tool_use β a guaranteed 400 on continuation. The append now keys on stop_reason, and truncation reads natively as stop_reason: "tool_use" with a continuable history. - The envelope is JSON-serializable end to end: runner-stored turns carry pydantic content blocks; everything is dumped to plain dicts, excluding SDK-internal __api_exclude__ fields (parsed_output) that the API rejects on round-trip. - Bounded by default (max_iterations 10, like the OpenAI surfaces); usage aggregation now preserves the final turn's native fields and sums the token counters None-safely; empty caller system strings are skipped; non-dict message entries and bad doc_id types raise PageIndexAPIError; anthropic < 0.68 gets an actionable version error; the doc block no longer spends a cache_control breakpoint. chat_completions()/responses(): - MaxTurnsExceeded wraps into PageIndexAPIError on all four run paths. - responses(stream=True) is one logical response: per-turn backend lifecycle events are collapsed (a canonical consumer previously stopped at turn 1's response.completed and never saw the answer), sequence numbers are reassigned monotonically, and the synthesized tool-output event carries output_index/sequence_number. - The responses envelope carries the real request surface (instructions, the actual function tool definitions, tool_choice, parallel_tool_calls, error/incomplete_details). - RunConfig(group_id) pins a stable prompt_cache_key: openai-agents otherwise stamps each run with a fresh key, tagging round-tripped prefixes as different cache groups and defeating the feature the round-trip exists for. - Abandoning a stream now cancels the run: a watchdog task lets the cancellation land even while the pump awaits the backend, and the per-call AsyncOpenAI client is closed before its loop ends (fixes "Task exception was never retrieved" noise). The opening role chunk is emitted even for empty outputs; empty responses() input and enable_citations-before-extra ordering fixed. Docs rescoped to what is true: finish_reason/status reflect loop completion on the OpenAI surfaces (the engine does not surface per-turn backend reasons); chat streaming yields visible narration including pre-tool text; messages(stream=True) forwards the Anthropic SDK's native event objects (not wire-verbatim); the doc block is a leading conversation item on OpenAI surfaces and a system block on messages(). Tests: 25 in the file (11 new), with per-extra skip sections so a machine with only one framework still covers the other surface; without-frameworks matrix re-verified; live smoke re-run green with a clean exit.
Fills the last cell of the agent-connection matrix: users driving their own anthropic tool_runner loop get runnable tools directly. Cloud wraps the live MCP tool set with input schemas passing through verbatim (MCP inputSchema is the Messages API schema shape); local exposes the same set messages() runs internally. The beta_tool wrapping moves from local_chat into integrations/anthropic_sdk.py, parallel to openai_agents.py, and messages() now consumes the shared builder. agent_tools grows _bridge_invoker/_read_only_tools so the plain-function and beta_tool cloud paths share invocation containment and the read-only gate.
Adversarial + best-practice review of 4590dd8 (three independent passes) surfaced two holes. The export was sync-only: AsyncAnthropic's runner accepts only BetaAsyncFunctionTool and splices anything else into the request body unserialized, so the first call died with an opaque TypeError β asynchronous=True now builds beta_async_tool runnables (present since the 0.68.0 floor) that run the blocking bridge/store call in a worker thread, keeping I/O off the caller's event loop. And beta_tool stores input_schema by reference, so cloud tools aliased the bridge's cached metas while the local path deep-copied β the builder now copies, and the passthrough test asserts equal-but-not-aliased so it can no longer compare an object with itself. Docstring fixes from the same round: the MCP-connector pointer now carries the full live-verified shape (authorization_token was missing β following it literally gave a 401), and the manual messages.create loop's to_dict() serialization is documented. Tests pin the runnable flavor both ways (isinstance), which existing tests could not distinguish.
β¦onversation The targeting block doc_id adds is re-set on every call and sits in the cached prompt prefix, so a round-trip that drops (or changes) doc_id silently diverges the prefix and loses the cache continuation. State the rule on all three chat surfaces' doc_id docs, and pin it with a prefix test that passes the same doc_id on both calls.
query + doc_id is the minimal PageIndex contract, so it now works uniformly: chat_completions and messages accept a plain string (one user message), as responses always did per its wire format. The wrap is input sugar at the SDK surface, not a translation layer β the outgoing wire is unchanged, and managed agent surfaces taking strings is the ecosystem convention (Runner.run, claude_agent_sdk.query). Cloud chat_completions gains the same acceptance; blank strings raise on every path.
The Messages API requires a per-turn output budget on the wire, but that is table-setting, not a PageIndex-layer user obligation β the simple call is now a question + model + doc_id. The knob stays overridable (passthrough intact); model stays required because no cross-vendor default is honest to guess.
max_tokens is a cap, not consumption, so the default should be the highest universally safe value: 4096 could truncate long-form answers (whole-document summaries), while 8192 is the output ceiling every non-EOL Claude model accepts and stays under the SDK's non-streaming long-request threshold.
Inserting tests above decorated ones absorbed their @needs_agents markers, so two tests ran (and failed) in the without-frameworks CI job. Both simulated-bare and full runs are green again.
Tool layer: - anthropic adapter: failed tool calls raise ToolError so the runner emits tool_result is_error:true; McpBridge.call_tool returns (text, is_error) and surfaces the server's MCP isError marking - as_openai_tools builds FunctionTool with the contract/server schema verbatim (strict off) β function_tool() regenerated schemas from signatures, dropping items/enum/pattern/bounds and aborting the whole list on object-typed params; shared _tool_specs() feeds both adapters - remove_document validates every name before deleting anything; call_tool classifies only bind-time TypeErrors as INVALID_INPUT - unknown-tool envelope formatted with _dumps like every other envelope Local chat: - doc_id is enforced at the tool layer (allowlist threaded through call_tool and the adapters), not just prompted; the shadow check runs inside the scope - _openai_model routes litellm/ and provider/ paths via LitellmModel and strips openai/ β the normalized retrieve_model 404'd as a raw wire name - responses() reports the backend's real terminal status (recorded at the transport client; the framework discards Response.status) and wraps framework exceptions in PageIndexAPIError - chat_completions streaming yields its opening chunk inside try, so an abandoned iterator still cancels the run and closes the backend - prompt-cache group_id is per-conversation (model+instructions+first item) instead of one global constant pooling every user - messages() max_tokens default resolves per model (claude-3 caps at 4096) Packaging / surface: - __init__ registers the 0.2.10 modules in _SUBMODULES; unknown names raise AttributeError instead of eagerly importing page_index_classic - anthropic floor 0.84.0: first release with ToolError whose runner also executes the final turn's tools on a max_iterations cut - client docstrings caught up with local chat landing Claude Agent SDK gate: - claude_allowed_tools(mcp_servers) derives mcp__<key>__<tool> entries from the caller's own registration map (live server annotations on cloud, the contract locally) β no name is ever spelled twice - claude_agent_config() bundles the three slots as one-call sugar over the explicit form Examples: - demo runs against cloud again (getattr for local-only attrs) and finds an existing indexed copy by name before re-indexing Tests: monkeypatches replace the consuming module's binding instead of mutating the shared time/requests modules; 185 -> 211.
claude_agent_config() gets two symmetric siblings, so each framework's front door is a single splat over the same explicit primitives: - openai_agent_config(): Agent(**...) kwargs β instructions, tools, and the local retrieve_model (cloud omits model for the framework default) - anthropic_runner_config(): tool_runner(**...) kwargs β system, tools, and the messages() defaults (per-model max_tokens, 10-iteration bound); only the user's messages remain Bundles stay pure sugar: doc_id rides agent_instructions, no extra semantics over the explicit form, docstrings point both ways. The demo agent shrinks to Agent(**client.openai_agent_config(doc_id=...)). Construction is pinned against the real frameworks in tests (Agent and tool_runner both built offline), so an upstream kwargs rename fails loudly; 211 -> 215 tests.
β¦lation, output_index axis - get_page_content: the summary is additive, not either/or β a call that both truncates for size and has out-of-range pages reported only the latter, telling the agent every in-range page was returned (#2) - McpBridge._extract_result: strict request-id correlation only; the eager fallback could hand back a stale or mis-correlated JSON-RPC message as this call's reply (#16) - responses() streaming: output_index now addresses the logical response.output β backend per-turn indexes are re-based past prior turns' items and the SDK-injected tool outputs take the next slot on that axis, instead of reusing the event-sequence counter (#15) 215 -> 217 tests.
pageindex-chat#448 adds /mcp?tools=read β the server registers only readOnlyHint-annotated tools β so the URL itself becomes the gate for every surface that hands a config to a third party: - as_claude_mcp: include_management now picks the endpoint on cloud; the parameter is real in both modes - as_openai_tools(hosted=True): OpenAI connects to the read-only endpoint by default and require_approval simplifies to "never" β the approval-flow middle ground becomes hard absence, matching every other surface's default - claude_allowed_tools() retired before ever shipping: with the server gated, allowed_tools degenerates to whole-server pre-approval, which claude_agent_config emits as the constant ["mcp__<name>"] β no setup-time bridge round-trip remains - in-process surfaces (agent_tools, as_openai_tools, as_anthropic_tools over the bridge) keep bare /mcp + client-side annotation filtering: they materialize tools locally and hand no URL to anyone Release ordering: 0.2.10 must ship after pageindex-chat#448 deploys β an older server ignores unknown query params and would silently serve the full set behind a URL that promises read-only.
_conversation_group_id feeds RunConfig.group_id into OpenAI's prompt_cache_key so a round-tripped prefix stays in one cache group. That wiring first appears in openai-agents 0.14.0: 0.8.0 through 0.13.x have no prompt_cache_key at all, and group_id there is a tracing group id only β inert, since tracing is disabled on the line above. An install resolving to the declared floor lost the cache continuity that the responses() docstring sells, silently and with no test able to catch it. The old floor's rationale (0.8.0 offloads sync tools to a thread) is subsumed by the new one. Every symbol the package imports predates 0.14.0, so nothing else constrains the bound.
openai_agent_config / anthropic_runner_config / claude_agent_config accepted doc_id but built unscoped tools, so the parameter that is a structural allowlist on chat_completions() was prompt-only advice here β the agent could read every document in the store regardless. - as_openai_tools / as_anthropic_tools / as_claude_mcp take a doc_id tail parameter and thread it to the existing _allowed_ids channel; the config helpers pass it through in local mode - cloud config helpers keep prompt-level targeting (tool scoping is server-side there, documented); explicit as_*(doc_id=...) raises on cloud instead of silently dropping the allowlist β including the hosted branch, which returned before _tool_specs' existing guard - _require_local_scope consolidates the cloud rejection that was inlined in _tool_specs - doc_id=[] is an empty allowlist, not "unscoped": dropped the `or None` at the three local chat surfaces
run_messages keyed its re-append guard on stop_reason, but the anthropic runner executes tools whenever the turn's content carries tool_use blocks (refusal excepted) β a max_tokens turn with complete tool_use blocks was already appended by the runner, so the guard re-appended it, duplicating tool_use ids and 400ing the documented verbatim continuation. The guard now checks whether final's tool_use ids already sit in the appended history; unexecuted tool_use blocks (refusal turns) are stripped from the appendable history, as the SDK itself does when rebuilding params around an unresulted turn. _conversation_group_id seeded on items[0], which is the doc-targeting block whenever doc_id is set β byte-identical across every conversation about a document, so all of them pooled under one prompt_cache_key and evicted each other's prefixes. Seed on the conversation's own first item instead: continuations keep their key, unrelated conversations never share one. Also drop the dead pytestmark_openai assignment (pytest's magic name is pytestmark; the section gate it implied never existed).
- _all_documents advances by what actually arrived and treats `total` as an optimization: absent/null totals and short pages silently truncated the library behind every name resolution - _make_bridge_function survives description: null (the parallel _tool_specs path already did) - as_openai_tools answers a malformed argument string with the guided error envelope instead of raising through the caller's whole run - the pre-0.2.10 package attributes (ConfigLoader, count_tokens, ...) resolve again: main's underscore-guarded fallthrough is restored β dunder probes stay lazy, a non-underscore typo pays one classic import before its AttributeError - _split_structure chunks are always lists: the structure field no longer changes JSON type between parts of one paginated response - the bridge replays only session-carrying 404s (the spec's expiry status); 400 raises instead of re-running side effects, and the reset double-checks under the lock so concurrent retries cannot clobber a freshly re-initialized session
- the bridge maps content blocks individually: base64 payloads (image/audio) become metadata stubs instead of handing the model the raw blob, text blocks pass verbatim, anything else keeps the JSON dump (revisit if tool results become real multimodal input) - cloud proxy annotations keep array item types (list[str], not bare list) so strict function calling accepts the round-trip; a type-array in items degrades to bare list instead of crashing the build - run_messages raises when set_messages_params stops delivering params instead of silently dropping every tool turn from the envelope - call_tool drops None-valued arguments (None β‘ omitted, the contract's semantics) β adapters that forward the model's nulls verbatim no longer trip parameter validation - client._parse_pages bounds the span arithmetically before materializing it, like the tool layer: "1-999999999" raises instead of allocating a billion integers
- responses() promised the Responses protocol ("no translation layer")
but _openai_model ignored protocol on the LiteLLM branch: provider-
prefixed models silently ran chat.completions under a responses-shaped
envelope, and with no transport hook to record status (LitellmModel
has no _client.responses) a turn truncated at the output cap reported
status "completed". The branch now raises for protocol == "responses"
β at agent-build time, before any backend call β naming the routes
out: chat_completions(), messages() for Anthropic models, or
OPENAI_BASE_URL + a bare/openai/-prefixed name for backends that
genuinely speak /responses. Refusal, not emulation: most providers
have no /responses endpoint to drive.
- chat_completions envelopes echoed retrieve_model verbatim, which
carries the SDK's litellm/ routing marker after normalization β a
name no provider catalog contains, and a different string than the
same model passed per-call. The envelope and every streaming chunk
now report the name the provider actually serves; routing and the
prompt-cache group key keep the prefixed form. responses() needs no
change (post-refusal the prefix cannot reach its envelope), and the
user-typed openai/ prefix stays echoed as typed.
- _remove_document caught only PageIndexAPIError around the per-doc
delete, so a bare OSError (local_store re-raises them) or a transport
error (cloud delete_document wraps nothing) escaped mid-batch,
discarded the entries for documents already irreversibly deleted, and
surfaced as a generic INTERNAL_ERROR envelope inviting a retry β which
then reports the destroyed document as not_found. The loop now catches
Exception, keeping the per-document results the contract promises.
9f67fdd made the three config helpers enforce doc_id at the tool layer but left their instructions on doc_targeting_block's unscoped default, so a bundle refused any doc_id whose name a newer library-wide duplicate shadows β a raise whose message ("the tools address documents by name and would read the newer one") had just become false: the bundle's own tools resolve names inside the allowlist and read the targeted document correctly. chat_completions() accepted the same doc_id via _doc_block's scoped=True. Each helper now computes scope = _local_doc_scope(doc_id) once and derives both slots from it β scoped=scope is not None for the instructions, doc_id=scope for the tools β so the check mode and the tool allowlist come from one fact and cannot drift apart again. build_agent_instructions grows a scoped passthrough; cloud stays on the whole-library check (scope is None there and the tools are genuinely unscoped), and the public agent_instructions() keeps its unscoped default for the same reason. An in-set duplicate still raises β and in that case the message is true on every surface that emits it.
The MCPServerStreamableHttp alternative sat in the Local: paragraph
pointing at bare {BASE_URL}/mcp β a cloud-only route (BASE_URL is the
hosted API; local has no HTTP MCP server) that as written would connect
unauthenticated to the full tool set. Now stated where it applies, in
the as_anthropic_tools connector-note form: Cloud paragraph, Bearer
auth spelled out, ?tools=read default with the drop-it escape.
β¦lopes
- call_tool coerces string booleans per the TOOL_CONTRACT schema
("false"/"no"/"0" read as False, not a truthy 3-minute wait) and
survives arguments: null (json.loads("null") reaches the seam as None)
- _local_doc_scope raises on an explicitly empty doc_id on cloud: with
no tool-layer allowlist there, dropping it silently widened an empty
scope to the whole library
- both page-spec caps count distinct pages instead of summing parts, so
overlapping ranges (a parent section plus its children) within the
10k union pass again as they did in 0.2.9; the per-part arithmetic
bound still rejects billion-page specs before materializing anything
- _remove_document deduplicates doc_names: a repeated name is one
deletion, not a second "failed" row with an internal error string
- doc_targeting_block merges the user's metadata tags from the listing
(local get_document keeps the 7-key cloud detail wire shape, which
carries none) so the block delivers the metadata it promises
- _wait_until_ready folds its two raise branches into one that carries
the doc_id: a poll that dies no longer discards the handle to an
uploaded, billed document
- _reported_model strips both routing prefixes (litellm/ and openai/)
and responses() now reports it too, instead of echoing a model id the
provider never served
- _openai_model wraps AsyncOpenAI() construction so a missing backend
credential surfaces as PageIndexAPIError like every other gate on the
chat surfaces (and builds the client once for both protocols)
- _browse_documents advances its cursor by the rows that actually
arrived and guards a null/absent total β the same hazards
_all_documents already guards β and an empty window ends pagination
instead of freezing the cursor
- responses(stream=True) raised PageIndexAPIError when the backend ended the response with response.failed / response.incomplete: openai-agents yields the terminal lifecycle event, then re-raises it as ModelBehaviorError, so the generic AgentsException wrap short-circuited the emit the agen's tail was built for β its failed/incomplete terminal mapping was dead code against the real engine, and the caller lost both the partial output and the real status. The wrap now steps aside when the recorded terminal state is failed/incomplete, and the stream ends with the honest terminal event (committed output, real status, error/incomplete_details) β the backend's terminal state is a protocol event, not an engine failure. Non-stream was already honest for incomplete via the transport recorder; a failed response arrives there as an HTTP error, covered below. Known ceiling: the truncated final turn's partial text was already streamed as deltas but is not reconstructed into the terminal event's output (the engine commits items only on turn completion). - Provider exceptions (network, auth, rate limit) leaked as raw openai/anthropic types through every chat surface, against the layer's own "never raw engine types" contract. Every engine boundary now wraps its vendor's base exception into PageIndexAPIError (chained): the four OpenAI-engine sites catch openai.OpenAIError β LiteLLM's exception types subclass openai's, so one handler covers both routing paths β and messages() catches anthropic.AnthropicError around the batch drive and the stream generator.
β¦ol args - _openai_model pre-checks the first path segment against litellm.provider_list (fail-open if the attribute ever disappears): a HuggingFace repo id like Qwen/Qwen2.5-7B-Instruct on an OpenAI-compatible server now fails at build time with the escape spelled out β 'openai/<id>' plus OPENAI_BASE_URL β instead of at request time inside LiteLLM with "LLM Provider NOT provided". The slash-means-provider routing convention itself is unchanged; the retrieve_model and chat_completions docstrings now document it where they promise "any OpenAI-compatible server works" - call_tool answers a non-dict arguments value (a JSON array or scalar from a misbehaving caller) with the guided INVALID_INPUT envelope instead of raising AttributeError through the agent loop, matching the openai adapter's own non-object guard
notifications/initialized moves inside the bridge lock: a concurrent first use could send tools/list between the handshake and the notification, which strict MCP servers reject with a 400 the bridge never replays. Regression test races two threads through a stalled notification window. claude-agent-sdk floor rises to 0.1.53 β below it, string prompts with SDK MCP servers (the documented local-mode flow) hit invisible registration (#597) and a deadlock (#780).
The parameter was dead from the moment it was introduced (daac9d2): the body reads only max_turns, and every call site already carries the cause via `raise ... from exc`. The signature implied the helper inspected the engine exception, which it never did. No behavior change β message text and __cause__ chaining verified identical across all four call sites (chat_completions and responses, stream and non-stream).
Both declared floors named a version that cannot work, and CI never caught either because it installs the latest. anthropic >=0.84.0 -> >=0.108.0. Probed against a mock transport: on a turn with stop_reason="refusal" carrying a tool_use block, 0.84.0, 0.92.0 and 0.100.0 all execute the tool and post the tool_result back; 0.108.0 and later stop at the refusal. test_messages_refusal_with_ tool_use_stays_appendable asserts the latter, so that test was false at the floor. messages() is unaffected in practice (it never passes include_management, so remove_document is not registered), but as_anthropic_tools(include_management=True) hands it to a caller's own runner. openai-agents >=0.14.0 -> >=0.18.1. 0.14.0 and 0.16.0 raise pydantic ValidationError on InputTokensDetails.cache_write_tokens before any request reaches the transport when paired with openai 2.54.0 β and they declare openai <3,>=2.26.0, so pip resolves exactly that pair. 0.18.1 is clean. The 0.14.0 rationale (RunConfig.group_id -> prompt_cache_key) still holds above the new floor. The three extras' floor comments are cut to the binding constraint; the reasoning lives here.
test_chat_completions_max_turns_wrapped only drove chat_completions, so the two responses() call sites had no coverage, and no test asserted that the engine exception survives as __cause__. Parametrized over both surfaces and both stream modes; the non-positive max_turns rejection splits out, since it is input validation rather than wrapping.
- _dumps drops indent=2: emission now matches _serialized_size's compact accounting, so the pagination budget bounds what is actually sent (indented parts measured under 95k but emitted ~1.8x the 100k cap) - call_tool builds the _allowed_ids frozenset inside the guarded block: a non-iterable doc_id returns the INVALID_INPUT envelope instead of raising into the agent loop; same move for _bridge_invoker's arguments normalization - next_steps strings qualify submit_document() as PageIndexClient.submit_document() (three sites), matching the one already-qualified site β it is a client method, not a registered tool - tests: import httpx at module scope (guaranteed via the hard openai dependency) so agents-gated tests survive an install without the anthropic extra; formatting assertion follows the compact envelope
β¦ items, full usage details - output now carries only model-produced items, so the envelope parses with the official openai SDK types (function_call_output is input vocabulary β the real API never returns it in output) - the full process transcript moves to the new items field; round-trip appends items instead of output (same bytes, so the provider prompt-cache prefix contract is unchanged) - usage aggregates token details across turns (cached_tokens, cache_write_tokens, reasoning_tokens) on both OpenAI surfaces β cache hits are now observable instead of discarded - streaming stops synthesizing the nonstandard tool-output event; every stream event now validates against the official event union, tool results arrive in the terminal envelope's items - tests: two conformance tests pin the contract (non-stream model_validate + per-event stream validation); round-trip prefix tests append items Verified: 267 tests green; live A/B against the real OpenAI API β field-identical to the official hand-rolled flow, round-trip accepted with zero repeat tool calls.
pyproject raised the floor in f58cca1 (0.84-0.107 execute a refusal turn's tool_use blocks); the three user-facing strings still pointed hand-installers at the broken range.
β¦ertising tool surfaces as_openai_tools / as_anthropic_tools cloud docstrings advertised the image tool without mentioning that the in-process bridge replaces base64 payloads with text placeholder stubs (mcp_bridge call_tool).
litellm's stable channel (every release satisfying our >=1.84.0 floor) and both agent extras require 3.10; on 3.9 pip resolution fails on the hard deps (verified in a clean venv β zero packages install). A clean 3.10 venv with all three extras runs the full suite green. CI already tests 3.10/3.13 only. The >=3.7 claim was inherited from the two-dep 0.2.8 client and was already unsatisfiable then (openai>=1.70 needs 3.8). Closes recurring review finding #10.
53 comment lines removed: rationale that belongs in commit messages, descriptions restating what adjacent code or function names already show, and cloud-implementation provenance notes. Section headers and constraint comments (protocol invariants, safety guards) kept.
Move asyncio.run(coro) out of the except RuntimeError block so real errors no longer carry a bogus "no running event loop" context in their traceback.
Member
Author
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 π. |
β¦ mode
Every entrance now defaults to Flash with the full optimize pass
(deterministic merge, then LLM expand), replacing the standard LLM-built
tree as the default:
- submit_document(): mode=None now means "flash"; pass mode="standard"
for the LLM-built tree. _index_flash runs optimize="full" with the
expand model = summary_model, and fails fast with the missing key
name(s) via litellm.validate_environment before any work.
- page_index_flash(): optimize takes "full" (default) / "merge" / False;
True is accepted as "full" for compatibility, unknown values raise
instead of silently degrading to merge-only. optimize_expand stays
honored for legacy callers.
- CLI: --mode {flash,standard} replaces --flash (kept as a hidden
compatibility alias that forces flash). --optimize defaults to full in
flash mode with an `off` choice; explicitly passing it outside flash
still errors. Standard-only tuning flags (--toc-check-pages,
--max-*-per-node, --if-add-*) now error in flash mode instead of being
silently ignored, mirroring the existing flash-only flag errors. The
key pre-check runs only when an LLM will actually be called, so
--no-summary --optimize off|merge works keyless. Output drops the
_structure_flash suffix β always <name>_structure.json.
On the Disney earnings PDF the optimized default is also faster than
unoptimized flash (fewer nodes to summarize) and fixes hierarchy
mistakes; both modes emit identical schemas end to end.
Docs updated to match (mode flag, defaults, LLM usage honesty); tests
pin the new defaults: stored mode == "flash", optimize passthrough, and
the unknown-optimize rejection.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PageIndex SDK 0.2.10 adds two things:
Both work in local mode (runs on your machine, with your model keys) and cloud mode (runs on api.pageindex.ai, with a PageIndex API key).
(Review note: this PR holds the complete 0.2.10 diff for review. The code is already on
mainvia #396, #402, and #404. This PR is not meant to be merged; the tools layer's own review record is #393.)Install
The extras only install the framework you choose. The base SDK adds no new dependencies, and everything else works with no extras installed.
Which API key do you need?
One rule: the key belongs to whatever model does the thinking. The PageIndex tools themselves never call an LLM.
submit_document)OPENAI_API_KEY(the default models are OpenAI's)chat_completions()/responses()OPENAI_API_KEY(default), or your provider's key if you pick another modelmessages()ANTHROPIC_API_KEYapi_key="..."from dash.pageindex.ai β no model keysPut keys in your environment or a
.envfile. To use a non-OpenAI model anywhere, pass a LiteLLM-style name likeanthropic/claude-sonnet-4-6and set that provider's usual variable. A missing key fails immediately and tells you which variable to set.Index your first document
Local indexing uses PageIndex Flash by default: the tree structure is built from the PDF's layout in seconds, and the LLM only writes node summaries. Pass
mode="standard"for the slower, fully LLM-built tree. The CLI works the same way:python run_pageindex.py --pdf_path doc.pdf, with--mode standardto opt out (--optimize full|merge|offfor the tree refinement β seepageindex/flash/README.md).Plug into your agent framework
Pick your framework. One call returns everything it needs β instructions plus tools. Local and cloud clients work identically here.
Each
*_config()helper is shorthand for two explicit calls:agent_instructions()plusas_openai_tools()/as_anthropic_tools()/as_claude_mcp(). Drop to the explicit form when you want to customize. All of these acceptdoc_id=...to point the agent at specific documents, andinclude_management=Trueto also expose document deletion (off by default).Chat with your documents
Three methods. Each speaks one standard wire format, so the request and the response look exactly like the API you already know. Pass a plain string, or full messages in that protocol's native format.
All three also support:
stream=True.doc_idthe same. Provider prompt caching keeps working across turns.doc_idrestricts the conversation to those documents. The tools enforce it; it is not just a suggestion to the model.What works where
Bring your own agent β the tools, on every major surface:
agent_tools()β plain functions, any frameworkas_openai_tools()/openai_agent_config()β OpenAI Agents SDKhosted=True: execution on OpenAI's side, read-only endpoint by default)as_anthropic_tools()/anthropic_runner_config()β Anthropic SDK tool runneras_claude_mcp()/claude_agent_config()β Claude Agent SDK / Claude Codeapi.pageindex.ai/mcp(read-only:β¦/mcp?tools=read) β any MCP host, the Anthropic MCP connector, OpenAI hosted MCPManaged chat β the SDK runs the loop:
chat_completions()responses()messages()tool_runnerLocal serves four read-only tools:
browse_documents,get_document,get_document_structure,get_page_content(remove_documentonly withinclude_management=True). Cloud addssearch_documents, folders, andget_document_imageβ discovered live from the server, never frozen into the SDK.What a run looks like
An actual run (local mode, OpenAI Agents SDK, over
examples/documents/q1-fy25-earnings.pdf):This is the intended loop: the agent reads the tree structure first, picks tight page ranges, and answers from tool output with page citations. No vector index, no chunking. The retrieval intelligence is your agent's own model β the navigation tools make no LLM calls.
Everything below is design rationale and the test record, written for reviewers. You don't need it to use the SDK.
Design β the tools layer
browse_documents/get_document/get_document_structure/get_page_content, doc_name-addressed, same input schemas, descriptions, and JSON response envelopes as the hosted MCP server'stools/listβ agent prompts port unchanged between the cloud MCP connection and these in-process tools. Adapters hand the contract/server schema to the framework verbatim (FunctionTool(params_json_schema=β¦),beta_tool(input_schema=β¦)) β no regeneration from Python signatures, soitems/enum/pattern/bounds survive on every surface.tests/data/cloud_mcp_contract.jsonfreezes the contract; a parity test guards drift.search_documents,get_document_image) are not registered, mirroring the server's gating semantics. Cloud-only parameters (folder_id,sort/query,recursive) are hidden from the local surface entirely β strict-schema frameworks then cannot express the dead-end calls, and the call_tool/MCP path still answers direct calls with a guided "works on PageIndex cloud" envelope as the backstop. Local descriptions and instructions teach only that surface: the exposed schema is the contract minus the documented hidden set (mechanically asserted), and a dead-reference test keeps local guidance from naming cloud-only tools.remove_documentis off by default, behindinclude_management=True.as_claude_mcp(),as_openai_tools(hosted=True), the raw connector URL β point at the server's read-only endpoint (/mcp?tools=read) by default, so the URL itself is the gate and works identically in every MCP client. The in-process surfaces (agent_tools(),as_openai_tools(),as_anthropic_tools()) expose only tools the server marksreadOnlyHint; local withholdsremove_documentat registration.include_management=Trueis the one switch that opens the complete list in either mode, on every surface. All four tool exports stay polymorphic: on a cloud client the live tool set β including new server-side tools β arrives without an SDK release.{"error", "errorCode", "next_steps"}envelope the cloud emits, flagged through each channel that has one (MCPisErrorpropagated, Anthropic tool runneris_error: trueviaToolError), so the model can always tell a failed call from data. Destructive calls validate every argument before acting β a rejection envelope means nothing was deleted.agent_instructions(doc_id=None)supplies the retrieval playbook for the agent's system prompt. Cloud: the live instructions the MCP server serves for the key's tool set, captured from theinitializehandshake over the same bridge session β server-side guidance updates arrive without an SDK release, and an empty server response raises instead of silently substituting. Local: the built-in playbook for the in-process tools, a trimmed subset with a consistency test that every tool it names exists locally.doc_id(str or list) appends the target documents β in the run above it is what let the agent skip discovery.openai-agents/claude-agent-sdk/anthropicare imported at call time with actionable errors; the[openai]/[claude]/[anthropic]extras carry floor-only pins.import pageindexand every existing feature work with none installed (covered by tests).submit_document(wait=True)polls with growing intervals; returns oncompleted, raises onfailedor after 30 minutes β the manual polling loop cloud callers write today spins forever on a failed document.summary_model).page_index_flash()takesoptimize="full"(default) /"merge"/False;Trueis accepted as"full"for backward compatibility, unknown values raise instead of silently degrading. The CLI's--mode {flash,standard}replaces--flash(kept as a hidden compatibility alias); standard-only tuning flags now error in flash mode instead of being silently ignored; the missing-key pre-check (litellm.validate_environment, all providers) runs only when an LLM will actually be called. Both modes emit identical output schemas end to end.Design β chat on the tools
/chat/completionsquirks (history flattening, bespoke prompt, stateless re-reading, arbitrary caps) are not mirrored; local targets the standard formats and becomes the reference the cloud can later converge toward.responses()/messages()raise on cloud clients until then.AGENT_INSTRUCTIONS; callersystemcontent appended, not rejected; thedoc_idtargeting block leads the conversation and the tool layer enforces it), tool execution (read-only local set, scoped todoc_id), and billing (usage aggregation, envelope ids). Per-run tracing is disabled; prompt-cache routing keys are per-conversation, never pooled across users.litellm/-prefixed andprovider/modelnames route through the SDK's LiteLLM model,openai/strips to the OpenAI SDK), the Anthropic SDK'stool_runnerformessages()(floor0.108.0β the first release whose runner stops at a refusal carrying atool_useblock instead of executing the tool; verified by probing mock transports against 0.84.0 through 0.108.0). Rule of the layer: engines = each vendor's official thin loop; agent hosts (Claude Code et al.) only ever get tools.responses()carries the backend's real terminalstatus/incomplete_details(recorded at the transport layer β the engine discards them), a partial page read names every omitted page, and framework exceptions surface asPageIndexAPIError, never as raw engine types.cache_controlbreakpoints sit on the managed system blocks only.enable_citationsraises as cloud-only (citations need block-level OCR data local mode does not store).messages()resolves itsmax_tokensdefault per model (8192, or 4096 for the claude-3 generation), so the simple call needs only a question on any model.Verification
Modelfake under openai-agents, a mock HTTP transport under the real anthropic SDK); contract parity vs the frozen snapshot; framework-missing/-installed behavior both ways; streaming on all three surfaces; the round-trip prefix-extension assertions on both engines; doc_id scoping, error-marking, and envelope-honesty regressions.agent_tools()andas_anthropic_tools()discovered this key's gated tool set (7 read-only tools;include_management=Trueaddsremove_document); frozen-contract parity letter-for-letter; envelope field parity on the analogous calls; the server serves non-emptyinitialize.instructions.chat_completionsanswered with the structure-first loop;responsesround-trip answered the follow-up with zero new tool calls. Anthropic βmessages()history was accepted verbatim by the real API, follow-up answered with zero new tool turns,cache_controlhit live (cache_read_input_tokens: 1826), native streaming; both the sync andAsyncAnthropictool runners drove the live cloud tools end-to-end; the Messages API MCP connector reachedapi.pageindex.ai/mcpserver-side (mcp_tool_use/mcp_tool_resultin a single call).d87fa89,b135711,eb1a230), the remainder triaged with rationale. Nine further review rounds followed (commit messages31c9150througheebed64), covering argument coercion, protocol terminal states, envelope honesty, provider error containment, CodeQL findings, and SDK dependency floors β each finding reproduced before the fix landed.responses()envelope β officialoutput(model items only) +items(full transcript for round-trip) + usage aggregated across turns, verified against the real OpenAI API; python floor declared>=3.10; staleanthropic>=0.84.0hints updated to the real 0.108.0 floor; the bridge's binary-stub behavior disclosed on the two image-advertising tool surfaces;_run_syncmoved off theexcept RuntimeErrorprobe so user exceptions stop carrying a phantom "no running event loop" context.Release gate β satisfied: the default cloud configs point at the read-only MCP endpoint, so VectifyAI/pageindex-chat#448 had to be deployed before 0.2.10 ships (an older server ignores the
tools=readparameter and would silently serve the full set behind a URL that promises read-only). Verified live before publishing0.2.10.dev1:β¦/mcp?tools=readserves 7 tools withoutremove_document,β¦/mcpserves 8 with it.Follow-ups (not in this PR): an
AsyncPageIndexClienttwin per the industry dual-client pattern β every layer around the SDK is already async-native (FastAPI server, agent engines, agent frameworks); the async chat path is the engines' native form (drops the sync bridge, streams pass through asasync for), and cloud transport gains an httpx track; a stdiopageindex-mcpentry point for non-Python MCP hosts; a publicdoc_idscope on the BYO tool exports (the chat surfaces already enforce it); the docs-site agent-integration page; cloud/responsesΒ·/messagesconvergence toward these surfaces.