MCP prompts over the bridge + chat(citations=True) - #494
Open
ahonn wants to merge 6 commits into
Open
Conversation
Add prompts/list and prompts/get to McpBridge so the SDK can pull the PageIndex MCP server's prompts (currently `cited_answer`, with its optional `format` argument) over the same session as tools and instructions. - `list_prompts()` shares the cursor pagination `tools/list` already used, now factored into `_list_paginated`. - `get_prompt(name, arguments=None)` returns `(description, messages)` untouched; argument values are stringified per the MCP prompt contract and None means no `arguments` field on the wire. - `render_prompt_text(messages)` flattens prompt messages to plain text for placement in a system prompt. - The initialize handshake now records server capabilities; prompt methods raise a clear PageIndexAPIError when the server does not advertise `prompts`, instead of surfacing a -32601 protocol error. This is the transport layer only; wiring into `agent_instructions` and the `*_agent_config` bundles follows in a separate change.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Drops the module-object import that CodeQL flagged as mixing 'import' and 'import from' for pageindex.mcp_bridge.
…hat's format chat(citations=True) makes the answer cite every claim the way PageIndex chat does: <cite doc= page=/> tags, block= added where the cloud document has blocks. Own-model chat over cloud documents fetches the MCP server's cited_answer prompt (format=cite) over the tools' bridge session and folds it into the system prompt after the managed prompt and before the caller's instructions, on all three lanes; the managed chat gets its own enable_citations; local documents get a frozen copy of the same prompt (pages only, local page content carries no blocks), pinned to the live text by a key-gated parity test. client.citation_prompt(format=) exposes the same text for framework-built agents: cite by default, markdown / footnote for hosts that strip tags. Stacks on #494 (bridge prompts/get). Verified live: block documents cite block ids, legacy and local documents cite pages, on the answer lane and both protocol lanes. Claude-Session: https://claude.ai/code/session_01Ex6uKLsDrQjZkqypjSyPwA
The refusal's only exit was dropping the chat model; own-model chat now cites through chat(citations=True), so name it. Also drop an unused return in the citation test helper. Claude-Session: https://claude.ai/code/session_01Ex6uKLsDrQjZkqypjSyPwA
A format name passed as citations= was truthy and silently meant cite; it now raises and names citation_prompt(format=) + instructions=. citation_prompt(format="") returned the server's markdown default while the getter's own default is cite; "" is unset, so it is cite too. That left fetch_citation_prompt's unset fallbacks without a caller, so it now takes the format it is given. The enable_citations refusal's keyless arm still said local mode could not cite; both arms now point at chat(citations=True). The docstrings that route other formats through instructions= say own-model chat only. Claude-Session: https://claude.ai/code/session_01RWo3vKJs6MVMxrfPxEVaxd
…comment chat() reached the citation text through a lazy import of the private fetch_citation_prompt(self, "cite") when the public citation_prompt() getter beside it is exactly that call, so the "cite" default was spelled in two places. chat() now uses the getter; the default lives in its signature only. The comment above LOCAL_CITATION_PROMPTS named the private chat repo and its server-side prompt file. That file ships to PyPI with the package and public code carries only the contract, so the comment keeps what the copy is and why its block rules stay dormant locally. Claude-Session: https://claude.ai/code/session_01XGBdRF2qoTcLNpRahj6QHx
| over the same bridge session as agent_tools(); format rides as the | ||
| prompt's argument; PageIndex chat's cite format by default ("" is | ||
| unset, so the default too).""" | ||
| import pageindex.mcp_bridge as mcp_bridge |
|
|
||
| def test_citation_prompt_empty_raises(monkeypatch): | ||
| """No silent empty guidance: a prompt with no text raises.""" | ||
| import pageindex.mcp_bridge as mcp_bridge |
rejojer
changed the title
Member
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 8ae62c2163
ℹ️ 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".
| - When page content includes block_id values, citations MUST be block-level: copy the exact block_id of the supporting block. Page-only cites are allowed ONLY when the tool output carries no block_id (legacy documents, structure outlines). NEVER invent or alter block_id values. | ||
| - For a claim drawn from multiple blocks on one page, add one tag per supporting block (at most 3); beyond that, cite the single strongest block. | ||
| - Each tag must reference a SINGLE page integer. For multi-page citations, use separate tags. | ||
| - Close every answer with a "Sources" section: one plain-text line per cited block, formatted `- <document name>, page <page> (block <block_id>)`. Keep it even when the inline tags are present — a client that strips unknown HTML tags would otherwise leave the answer with no visible citations at all.""", |
There was a problem hiding this comment.
When chat(citations=True) runs on local documents, _get_page_content() never supplies block IDs, but this instruction requires every Sources entry to include (block <block_id>) while the preceding rule prohibits inventing one. The model therefore cannot satisfy both instructions and may fabricate block IDs or omit the required Sources section; provide a page-only Sources format when no block ID exists.
Useful? React with 👍 / 👎.
| "instructions blocks are the Messages protocol's shape — " | ||
| "with protocol=\"messages\" they append after the managed " | ||
| "system blocks; the other lanes take a string.") | ||
| if citations and citations is not True: |
There was a problem hiding this comment.
When callers pass a falsey non-boolean such as "", [], None, or 0, this condition is skipped and the value silently disables citations despite the public contract saying only True and False are accepted. Validate the type independently of truthiness so invalid falsey inputs raise just like the tested truthy string does.
Useful? React with 👍 / 👎.
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
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two pieces, one stack: the MCP bridge learns to fetch server prompts, and
chat(citations=True)is the first consumer, folded in from #496.Bridge: server prompts over the MCP session
Adds
prompts/listandprompts/gettoMcpBridge, so the SDK can fetch the server'scited_answerprompt (with its optionalformatargument) over the existing MCP session.list_prompts()/get_prompt(name, arguments=None), plusrender_prompt_text()for system-prompt placement.promptsis not advertised.Wiring into the
*_agent_configbundles stays a follow-up.chat(citations=True)Makes the answer cite every claim the way PageIndex chat does:
<cite doc="…" page="…"/>tags,block="…"added where the cloud document has blocks.cited_answerprompt (format=cite, the same rules PageIndex chat uses) over the same bridge session as the tools, and folds it into the system prompt after the managed prompt and before the caller'sinstructions. All three lanes (answer lane,protocol="responses",protocol="messages") via the existing instructions plumbing;local_chat.pyonly reworded one refusal.citations=Truepassesenable_citations=Trueto the endpoint.LOCAL_CITATION_PROMPTS, the live text minus the one bullet naming the cloud-onlyget_document_image()tool). Local page content carries no blocks, so citations resolve to pages.citationstakes onlyTrue/False. Another format goes throughclient.citation_prompt(format=...)passed asinstructions=; the getter also serves framework-built agents.markdown/footnoteare the prompt's variants for hosts that strip tags.Verified
api.pageindex.ai/mcp.<cite doc="document_5.pdf" page="1" block="p1_text_1"/>; legacy document →<cite doc="1706.03762_24.pdf" page="1"/>; locally indexed PDF →<cite doc="q1-fy25-earnings.pdf" page="3"/>; managedcitations=Truenon-stream and stream.Not in this PR
agents.mdx,chat.mdx's hand-written<cite>system message becomescitations=) and a release-notes line, with the release.p1_text_001that the model echoes on legacy documents (resolved citations then carry ablock_idwith nobbox); and agranularityargument oncited_answerwould let the SDK offer page-only citations on block documents.https://claude.ai/code/session_01XGBdRF2qoTcLNpRahj6QHx