From 30f85df357e91ca3c6e7536b4a834e9b8c1ee782 Mon Sep 17 00:00:00 2001 From: Ray Date: Wed, 9 Sep 2026 21:14:56 +0800 Subject: [PATCH 1/6] Client instructions: a standing persona for every answer surface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PageIndexClient(instructions=...) sets standing guidance for the answering agent — persona, language, format — appended after the managed system prompt wherever an answer is produced: chat() and chat_completions() on both engines, the Responses and Messages protocol lanes, agent_instructions() and the three *_agent_config() bundles. It is a client-level argument like mode=, not a chat-side spelling: it combines with any chat= and never selects own-model chat on its own. Blank configures nothing, as chat(instructions="") does. chat(instructions=) adds to it per call; the prompt order is managed base, client, call, history system rows. One insertion point serves every own-model surface (_base_instructions). The managed cloud chat takes exactly one system message, first: the client's instructions, the call's, and the history's system rows now fold into it, in that order — so chat(instructions=) works on a managed client (refused since #460, although the endpoint has accepted custom instructions since Sep 1), and a system row anywhere in the history no longer 400s. The answer lane's messages contract is the same on both engines: text history only — the endpoint refuses tool rows and structured content itself; the SDK says so first, with the protocol-lane pointer. Live-verified against the production managed chat and the live MCP instructions. Claude-Session: https://claude.ai/code/session_01J8fbpdM5pz2JLjiNY11usy --- pageindex/agent_tools.py | 23 ++++---- pageindex/client.py | 62 ++++++++++++++++----- tests/test_client.py | 29 ++++++++++ tests/test_local_chat.py | 115 ++++++++++++++++++++++++++++++++++++++- 4 files changed, 202 insertions(+), 27 deletions(-) diff --git a/pageindex/agent_tools.py b/pageindex/agent_tools.py index 671625112..f4727a696 100644 --- a/pageindex/agent_tools.py +++ b/pageindex/agent_tools.py @@ -1691,16 +1691,19 @@ def _base_instructions(client, include_management: bool = False) -> str: """Cloud: the live instructions the MCP server serves for the tool set actually shipped. Local: the built-in subset instructions.""" if not getattr(client, "api_key", None): - return AGENT_INSTRUCTIONS - instructions = _cloud_bridge( - client, gated=not include_management).instructions() - if not isinstance(instructions, str) or not instructions.strip(): - raise PageIndexAPIError( - "The MCP server returned no agent instructions — refusing to " - "substitute the SDK's local-subset guidance, which does not " - "cover the cloud tool set." - ) - return instructions + base = AGENT_INSTRUCTIONS + else: + base = _cloud_bridge( + client, gated=not include_management).instructions() + if not isinstance(base, str) or not base.strip(): + raise PageIndexAPIError( + "The MCP server returned no agent instructions — refusing " + "to substitute the SDK's local-subset guidance, which does " + "not cover the cloud tool set." + ) + # The client's standing instructions follow the base on every surface. + own = getattr(client, "instructions", None) + return f"{base}\n\n{own}" if own else base def doc_targeting_block(client, doc_id) -> Optional[str]: diff --git a/pageindex/client.py b/pageindex/client.py index a36124cba..57aee4db7 100644 --- a/pageindex/client.py +++ b/pageindex/client.py @@ -318,6 +318,14 @@ class PageIndexClient: ``backend`` keys win over it. The dict reaches whichever door runs, in that door's vocabulary (see each method) — ``api_key`` / ``base_url`` mean the same thing on all three. + instructions (str, optional): Standing guidance for the answering + agent — persona, language, format — appended after the + managed system prompt on every chat surface, the managed + cloud chat included, and in ``agent_instructions()`` and the + ``*_agent_config()`` bundles. Not a chat-side spelling: it + combines with any ``chat=`` and never selects own-model chat. + ``chat(instructions=...)`` adds to it per call. Indexing + has no prompt to extend. PageIndexCloudClient / PageIndexLocalClient pin the index side at construction instead of inferring it from api_key. @@ -347,12 +355,20 @@ def __init__( storage_path: Optional[Union[str, os.PathLike[str]]] = None, index_backend: Optional[dict[str, Any]] = None, chat_backend: Optional[dict[str, Any]] = None, + instructions: Optional[str] = None, ): if api_key == "": raise PageIndexAPIError( "api_key is an empty string. Pass a real PageIndex API key for " "cloud mode, or omit api_key entirely for local mode." ) + if instructions is not None and not isinstance(instructions, str): + raise PageIndexAPIError( + f"instructions must be a str, got {type(instructions).__name__} " + "— Messages system blocks go on chat(protocol=\"messages\", " + "instructions=[...]).") + # Blank configures nothing, as chat(instructions="") does. + self.instructions = (instructions or "").strip() or None # Each side picks one spelling — its slot, or the flat arguments. # ``model`` sets every role, so it claims both sides. index_flat: dict[str, Any] = { @@ -519,9 +535,8 @@ def _local_chat(self) -> bool: return model is not None def _require_own_chat(self, lane: str) -> None: - # The one refusal for the Responses / Messages lanes, the doors - # behind them, and instructions: shared, so the doors cannot drift - # from chat(). + # The one refusal for the Responses / Messages lanes and the doors + # behind them: shared, so the doors cannot drift from chat(). if self._local_chat: return if not getattr(self, "api_key", None): @@ -1040,13 +1055,15 @@ def chat( — the wire protocol, engine, and input/output shapes of this call. Own-model chat only, except ``"chat_completions"``, which the managed chat serves too. - instructions: Own-model chat only — persona or extra guidance + instructions: Persona or extra guidance for this call, appended after the managed system prompt (which stays: it - carries the tool guidance and the document context). A - string on every lane; with ``protocol="messages"`` also - a list of Messages system blocks. On the answer lane and + carries the tool guidance and the document context) and + the client's own ``instructions``. A string on every + lane; with ``protocol="messages"`` also a list of + Messages system blocks. On the answer lane and ``protocol="chat_completions"`` it precedes any ``system`` - rows in the history. + rows in the history; the managed cloud chat receives them + all as its one leading system message. citations: Own-model chat: cite every claim the way PageIndex chat does — ```` tags, ``block="…"`` added where the cloud document has blocks. The guidance is @@ -1192,7 +1209,6 @@ def chat( system=instructions, max_turns=max_turns, extra_body=body, extra_headers=extra_headers, backend=backend) if instructions: - self._require_own_chat("instructions") if isinstance(messages, str): if not messages.strip(): raise PageIndexAPIError( @@ -1303,10 +1319,11 @@ def chat_completions( Args: messages: Conversation messages with 'role' and 'content' keys, or a bare query string (it becomes a single user message). - Own-model chat also accepts system/developer messages — - their content is appended to the managed system prompt — - and takes text history only: tool-role turns are rejected - (the managed endpoint forwards them verbatim), and message + System/developer messages, wherever they sit, join the + managed system prompt after the client's ``instructions`` + (the managed endpoint receives them as its one leading + system message); the history is text only: tool-role + turns are rejected on both engines, and message fields beyond role/content are dropped. stream: Enable streaming responses. doc_id: Document ID or list of IDs to scope the conversation. @@ -1406,6 +1423,17 @@ def chat_completions( "agent in your process, or drop them to use the managed " "chat endpoint, which selects its own model." ) + # One answer-lane contract on both engines (text history; the + # endpoint refuses tool rows and structured content itself). It + # takes a single system message, first: the client's instructions + # and the history's system rows fold into it; with neither, the + # messages go as they are. + from .local_chat import _split_chat_messages + system_texts, history = _split_chat_messages(messages) + texts = [t for t in [self.instructions, *system_texts] if t] + if texts: + messages = [{"role": "system", "content": "\n\n".join(texts)}, + *history] from .cloud_api import CloudAPI return cast(CloudAPI, self._api).chat_completions( messages=messages, stream=stream, doc_id=doc_id, @@ -2009,6 +2037,7 @@ def agent_instructions(self, *, include_management: bool = False) -> str: ``agent_tools()`` — server-side guidance updates arrive without an SDK release. Raises PageIndexAPIError if the server cannot be reached. Local: the built-in guidance for the in-process tools. + The client's ``instructions``, if set, follow the guidance. Static by design: document targeting is conversation content, not guidance — see ``document_context()``. @@ -2138,6 +2167,7 @@ def __init__( chat_model: Optional[str] = None, retrieve_model: Optional[str] = None, chat_backend: Optional[dict[str, Any]] = None, + instructions: Optional[str] = None, ): if index is None: if api_key is None: @@ -2152,7 +2182,7 @@ def __init__( ) super().__init__(api_key, index=index, chat=chat, chat_model=chat_model, retrieve_model=retrieve_model, - chat_backend=chat_backend) + chat_backend=chat_backend, instructions=instructions) class PageIndexLocalClient(PageIndexClient): @@ -2173,9 +2203,11 @@ def __init__( storage_path: Optional[Union[str, os.PathLike[str]]] = None, index_backend: Optional[dict[str, Any]] = None, chat_backend: Optional[dict[str, Any]] = None, + instructions: Optional[str] = None, ): super().__init__(None, index=index, chat=chat, index_model=index_model, chat_model=chat_model, model=model, summary_model=summary_model, retrieve_model=retrieve_model, storage_path=storage_path, - index_backend=index_backend, chat_backend=chat_backend) + index_backend=index_backend, chat_backend=chat_backend, + instructions=instructions) diff --git a/tests/test_client.py b/tests/test_client.py index 260838045..5f246737b 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -2160,3 +2160,32 @@ def broken(**kwargs): with pytest.raises(utils.LLMRetriesExhausted): utils.llm_completion("openai/gpt-x", "hi") assert sum("Retrying" in r.getMessage() for r in caplog.records) == 9 + + +# ── client-level instructions ── + +def test_instructions_stored_on_every_constructor(tmp_path): + from pageindex import PageIndexCloudClient, PageIndexLocalClient + store = str(tmp_path / "store") + assert PageIndexClient(storage_path=store, + instructions=" persona ").instructions == "persona" + assert PageIndexLocalClient(storage_path=store, + instructions="p").instructions == "p" + assert PageIndexCloudClient(api_key="pi-k", + instructions="p").instructions == "p" + # not a chat-side spelling: combines with a string chat= slot, and + # never selects own-model chat on its own + assert PageIndexClient(api_key="pi-k", chat="gpt-x", + instructions="p").instructions == "p" + managed = PageIndexClient(api_key="pi-k", instructions="p") + assert managed.chat_model is None and managed.instructions == "p" + assert PageIndexClient(storage_path=store).instructions is None + # blank configures nothing, as chat(instructions="") does + assert PageIndexClient(storage_path=store, + instructions=" ").instructions is None + + +def test_instructions_must_be_a_string(tmp_path): + with pytest.raises(PageIndexAPIError, match="instructions must be a str"): + PageIndexClient(storage_path=str(tmp_path / "s"), + instructions=[{"type": "text", "text": "x"}]) diff --git a/tests/test_local_chat.py b/tests/test_local_chat.py index 204aacf72..f6d69a1ca 100644 --- a/tests/test_local_chat.py +++ b/tests/test_local_chat.py @@ -336,8 +336,6 @@ def test_cloud_guards(monkeypatch): cloud._responses("x") with pytest.raises(PageIndexAPIError, match="own chat model"): cloud._messages("x", model="m") - with pytest.raises(PageIndexAPIError, match="own chat model"): - cloud.chat("x", instructions="be brief") with pytest.raises(PageIndexAPIError, match="own chat model"): cloud.chat("x", max_turns=2) @@ -3845,3 +3843,116 @@ def test_bridge_chat_keeps_model_slips_model_visible(bridge_client, result = client.chat_completions("What?") assert result["choices"][0]["message"]["content"] == "Recovered" assert len(fake.instructions) == 2 and bridge.calls == [] + + +# ── client-level instructions: after the managed base, on every surface ── + +def test_client_instructions_follow_the_managed_base_everywhere(store_path): + from pageindex.agent_tools import AGENT_INSTRUCTIONS + client = PageIndexLocalClient(storage_path=store_path, + instructions="PERSONA") + assert client.agent_instructions() == AGENT_INSTRUCTIONS + "\n\nPERSONA" + # the chat lanes' prompt: header, base, client, then the call's texts + managed = local_chat._managed_instructions(client, ["CALL", "HISTORY"]) + marks = [managed.index(m) for m in + (CHAT_HEADER, AGENT_INSTRUCTIONS, "PERSONA", "CALL", "HISTORY")] + assert marks == sorted(marks) + # Messages lane: inside the cached managed block, before the call's + blocks = local_chat._anthropic_system(client, "CALL", None) + assert blocks[0]["text"].endswith("\n\nPERSONA") + assert blocks[1]["text"] == "CALL" + # unset: the base alone, byte-identical to before + plain = PageIndexLocalClient(storage_path=store_path) + assert plain.agent_instructions() == AGENT_INSTRUCTIONS + + +def test_bridge_client_instructions_follow_the_live_instructions( + bridge_client): + client, _ = bridge_client + client.instructions = "PERSONA" # a plain attribute, read per call + assert client.agent_instructions() == "CLOUD LIVE INSTRUCTIONS\n\nPERSONA" + + +@needs_agents +def test_openai_agent_config_carries_client_instructions(store_path): + client = PageIndexLocalClient(storage_path=store_path, + instructions="PERSONA") + assert client.openai_agent_config()["instructions"].endswith("PERSONA") + + +def test_anthropic_runner_config_carries_client_instructions(store_path): + pytest.importorskip("anthropic") + client = PageIndexLocalClient(storage_path=store_path, + instructions="PERSONA") + assert client.anthropic_runner_config("claude-x")["system"].endswith( + "PERSONA") + + +def test_claude_agent_config_carries_client_instructions(store_path): + pytest.importorskip("claude_agent_sdk") + client = PageIndexLocalClient(storage_path=store_path, + instructions="PERSONA") + assert client.claude_agent_config()["system_prompt"].endswith("PERSONA") + + +def test_managed_chat_sends_one_leading_system_row(monkeypatch): + """The managed endpoint takes one system message, first: the client's + instructions, the call's, and the history's system rows (any + position) fold into it, in that order.""" + cloud = PageIndexCloudClient(api_key="pi-k", instructions="PERSONA") + seen = {} + monkeypatch.setattr(cloud._api, "chat_completions", + lambda **kw: seen.update(kw) or { + "choices": [{"message": {"content": "ok"}}]}) + history = [{"role": "user", "content": "q1"}, + {"role": "assistant", "content": "a1"}, + {"role": "system", "content": "HISTORY"}, + {"role": "user", "content": "q2"}] + assert cloud.chat(history, instructions="CALL") == "ok" + assert seen["messages"] == [ + {"role": "system", "content": "PERSONA\n\nCALL\n\nHISTORY"}, + {"role": "user", "content": "q1"}, + {"role": "assistant", "content": "a1"}, + {"role": "user", "content": "q2"}] + # a bare question, and the streamed door, ride the same fold + monkeypatch.setattr(cloud._api, "chat_completions", + lambda **kw: seen.update(kw) or iter([])) + assert list(cloud.chat("q", stream=True, show_process=False)) == [] + assert seen["messages"] == [{"role": "system", "content": "PERSONA"}, + {"role": "user", "content": "q"}] + # developer rows are system text here too, as on the own-model lane + cloud.chat_completions([{"role": "user", "content": "q"}, + {"role": "developer", "content": "DEV"}]) + assert seen["messages"][0] == {"role": "system", + "content": "PERSONA\n\nDEV"} + + +def test_managed_chat_forwards_untouched_when_nothing_to_fold(monkeypatch): + cloud = PageIndexCloudClient(api_key="pi-k") + seen = {} + monkeypatch.setattr(cloud._api, "chat_completions", + lambda **kw: seen.update(kw) or { + "choices": [{"message": {"content": "ok"}}]}) + messages = [{"role": "user", "content": "q"}] + cloud.chat(messages) + assert seen["messages"] is messages + leading = [{"role": "system", "content": "S"}, + {"role": "user", "content": "q"}] + cloud.chat(leading) + assert seen["messages"] == leading + + +def test_managed_chat_history_contract_matches_the_own_model_lane( + monkeypatch): + """One answer-lane contract on both engines: text history only. The + endpoint refuses tool rows and structured content itself (400/422); + the SDK says so first, with the protocol-lane pointer.""" + cloud = PageIndexCloudClient(api_key="pi-k") + monkeypatch.setattr(cloud._api, "chat_completions", + lambda **kw: pytest.fail("must not reach the wire")) + with pytest.raises(PageIndexAPIError, match="Unsupported role"): + cloud.chat([{"role": "user", "content": "q"}, + {"role": "tool", "tool_call_id": "c", "content": "x"}]) + with pytest.raises(PageIndexAPIError, match="content must be a string"): + cloud.chat([{"role": "user", + "content": [{"type": "text", "text": "q"}]}]) From c4df5895f42a8ad42cd19da69884810efd92f208 Mon Sep 17 00:00:00 2001 From: Ray Date: Wed, 9 Sep 2026 21:25:55 +0800 Subject: [PATCH 2/6] Managed fold: always send the canonical history MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The managed payload is now the same shape every time — one leading system row when there is any system text, then the role/content history — instead of forwarding the caller's list untouched when nothing folded. That branch let a blank system row sit mid-history and reach the endpoint's "only one system message, first" refusal; blank text now configures nothing, like a blank instructions= does. Claude-Session: https://claude.ai/code/session_01J8fbpdM5pz2JLjiNY11usy --- pageindex/client.py | 11 +++++------ tests/test_local_chat.py | 11 +++++++---- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/pageindex/client.py b/pageindex/client.py index 57aee4db7..3d002e28a 100644 --- a/pageindex/client.py +++ b/pageindex/client.py @@ -1426,14 +1426,13 @@ def chat_completions( # One answer-lane contract on both engines (text history; the # endpoint refuses tool rows and structured content itself). It # takes a single system message, first: the client's instructions - # and the history's system rows fold into it; with neither, the - # messages go as they are. + # and the history's system rows fold into it. from .local_chat import _split_chat_messages system_texts, history = _split_chat_messages(messages) - texts = [t for t in [self.instructions, *system_texts] if t] - if texts: - messages = [{"role": "system", "content": "\n\n".join(texts)}, - *history] + texts = [t for t in [self.instructions, *system_texts] + if t and t.strip()] + messages = ([{"role": "system", "content": "\n\n".join(texts)}] + if texts else []) + history from .cloud_api import CloudAPI return cast(CloudAPI, self._api).chat_completions( messages=messages, stream=stream, doc_id=doc_id, diff --git a/tests/test_local_chat.py b/tests/test_local_chat.py index f6d69a1ca..4214aafab 100644 --- a/tests/test_local_chat.py +++ b/tests/test_local_chat.py @@ -3927,19 +3927,22 @@ def test_managed_chat_sends_one_leading_system_row(monkeypatch): "content": "PERSONA\n\nDEV"} -def test_managed_chat_forwards_untouched_when_nothing_to_fold(monkeypatch): +def test_managed_chat_sends_the_canonical_history(monkeypatch): + """No client instructions: the payload is the history as given, a + leading system row kept in place; blank system rows and fields + beyond role/content are dropped, as on the own-model lane.""" cloud = PageIndexCloudClient(api_key="pi-k") seen = {} monkeypatch.setattr(cloud._api, "chat_completions", lambda **kw: seen.update(kw) or { "choices": [{"message": {"content": "ok"}}]}) - messages = [{"role": "user", "content": "q"}] - cloud.chat(messages) - assert seen["messages"] is messages leading = [{"role": "system", "content": "S"}, {"role": "user", "content": "q"}] cloud.chat(leading) assert seen["messages"] == leading + cloud.chat([{"role": "user", "content": "q", "name": "ray"}, + {"role": "system", "content": " "}]) + assert seen["messages"] == [{"role": "user", "content": "q"}] def test_managed_chat_history_contract_matches_the_own_model_lane( From 3064d9df6651621343d96c74ca120fea1e407a88 Mon Sep 17 00:00:00 2001 From: Ray Date: Thu, 10 Sep 2026 02:04:42 +0800 Subject: [PATCH 3/6] Managed fold: lift system rows only, forward the rest verbatim The managed chat_completions lane ran the whole payload through the own-model validator: tool rows, structured content and tuples were refused before the wire, and every field beyond role/content was stripped (tool_calls, name, the endpoint's own citations). The SDK is a thin skin over the cloud. It folds the client's instructions and the history's system/developer rows into the one leading system row the endpoint takes, and sends everything else as given; the endpoint decides what it accepts. Also: - _managed_instructions drops blank system texts, as the managed fold and _anthropic_system already do, so both engines build the same prompt for the same input (and share one prompt-cache key). - An end-to-end test drives chat() on the own-model lane and asserts the persona and the call's system text reach the model; the white-box builder tests alone stayed green with the persona removed from the answer lane. - as_claude_mcp(): the MCP-instructions channel carries the tool guidance only; the client's instructions ride system_prompt. - Comments that restated code or an assertion removed; the chat= combination test asserts chat_model too. Blank instructions configure nothing at the constructor, as chat(instructions="") does. Claude-Session: https://claude.ai/code/session_01TgdXZx63aMwrpaKFTQFFKx --- pageindex/agent_tools.py | 1 - pageindex/client.py | 37 +++++++++++++++------------ pageindex/local_chat.py | 3 ++- tests/test_client.py | 7 ++---- tests/test_local_chat.py | 54 +++++++++++++++++----------------------- 5 files changed, 48 insertions(+), 54 deletions(-) diff --git a/pageindex/agent_tools.py b/pageindex/agent_tools.py index f4727a696..afac6b9d5 100644 --- a/pageindex/agent_tools.py +++ b/pageindex/agent_tools.py @@ -1701,7 +1701,6 @@ def _base_instructions(client, include_management: bool = False) -> str: "to substitute the SDK's local-subset guidance, which does " "not cover the cloud tool set." ) - # The client's standing instructions follow the base on every surface. own = getattr(client, "instructions", None) return f"{base}\n\n{own}" if own else base diff --git a/pageindex/client.py b/pageindex/client.py index 3d002e28a..91d95781d 100644 --- a/pageindex/client.py +++ b/pageindex/client.py @@ -367,7 +367,6 @@ def __init__( f"instructions must be a str, got {type(instructions).__name__} " "— Messages system blocks go on chat(protocol=\"messages\", " "instructions=[...]).") - # Blank configures nothing, as chat(instructions="") does. self.instructions = (instructions or "").strip() or None # Each side picks one spelling — its slot, or the flat arguments. # ``model`` sets every role, so it claims both sides. @@ -1322,9 +1321,10 @@ def chat_completions( System/developer messages, wherever they sit, join the managed system prompt after the client's ``instructions`` (the managed endpoint receives them as its one leading - system message); the history is text only: tool-role - turns are rejected on both engines, and message - fields beyond role/content are dropped. + system message). Own-model chat takes text history only: + tool-role turns are rejected and fields beyond + role/content are dropped; the managed endpoint receives + the rest of the history verbatim. stream: Enable streaming responses. doc_id: Document ID or list of IDs to scope the conversation. Keep it identical across a conversation's calls — the @@ -1423,14 +1423,18 @@ def chat_completions( "agent in your process, or drop them to use the managed " "chat endpoint, which selects its own model." ) - # One answer-lane contract on both engines (text history; the - # endpoint refuses tool rows and structured content itself). It - # takes a single system message, first: the client's instructions - # and the history's system rows fold into it. - from .local_chat import _split_chat_messages - system_texts, history = _split_chat_messages(messages) - texts = [t for t in [self.instructions, *system_texts] - if t and t.strip()] + # The endpoint takes one system message, first: the client's + # instructions and the history's system rows fold into it. + from .local_chat import _system_text + texts = [self.instructions] if self.instructions else [] + history = [] + for message in messages: + role = message.get("role") if isinstance(message, dict) else None + if role in ("system", "developer"): + texts.append(_system_text(message.get("content"))) + else: + history.append(message) + texts = [t for t in texts if t.strip()] messages = ([{"role": "system", "content": "\n\n".join(texts)}] if texts else []) + history from .cloud_api import CloudAPI @@ -1970,10 +1974,11 @@ def as_claude_mcp(self, include_management: bool = False, *, in-process server — match it to the key you register the entry under (cloud entries carry no name). - Cloud hosts that surface MCP server instructions receive the same - guidance ``agent_instructions()`` returns natively — passing both - duplicates the text (harmless). ``system_prompt`` stays the - recommended channel: it is guaranteed delivery, and the only + Cloud hosts that surface MCP server instructions receive the tool + guidance natively — not the client's ``instructions``, which only + ``system_prompt`` carries; passing both duplicates the guidance + (harmless). ``system_prompt`` stays the recommended channel: it is + guaranteed delivery, and the only channel local mode has. Usage (or ``claude_agent_config()`` for all three slots in one diff --git a/pageindex/local_chat.py b/pageindex/local_chat.py index f60df2c26..313d8547b 100644 --- a/pageindex/local_chat.py +++ b/pageindex/local_chat.py @@ -29,7 +29,8 @@ def _managed_instructions(client, extra_system: list[str]) -> str: # Local: the built-in subset guidance. Own-model chat over cloud # documents: the live instructions the MCP server serves. base: str = _base_instructions(client) - return "\n\n".join([CHAT_HEADER, base, *extra_system]) + return "\n\n".join([CHAT_HEADER, base, + *[t for t in extra_system if t.strip()]]) def _system_text(content: Any) -> str: diff --git a/tests/test_client.py b/tests/test_client.py index 5f246737b..4e63223bf 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -2173,14 +2173,11 @@ def test_instructions_stored_on_every_constructor(tmp_path): instructions="p").instructions == "p" assert PageIndexCloudClient(api_key="pi-k", instructions="p").instructions == "p" - # not a chat-side spelling: combines with a string chat= slot, and - # never selects own-model chat on its own - assert PageIndexClient(api_key="pi-k", chat="gpt-x", - instructions="p").instructions == "p" + both = PageIndexClient(api_key="pi-k", chat="gpt-x", instructions="p") + assert (both.chat_model, both.instructions) == ("gpt-x", "p") managed = PageIndexClient(api_key="pi-k", instructions="p") assert managed.chat_model is None and managed.instructions == "p" assert PageIndexClient(storage_path=store).instructions is None - # blank configures nothing, as chat(instructions="") does assert PageIndexClient(storage_path=store, instructions=" ").instructions is None diff --git a/tests/test_local_chat.py b/tests/test_local_chat.py index 4214aafab..1000642d3 100644 --- a/tests/test_local_chat.py +++ b/tests/test_local_chat.py @@ -3852,7 +3852,6 @@ def test_client_instructions_follow_the_managed_base_everywhere(store_path): client = PageIndexLocalClient(storage_path=store_path, instructions="PERSONA") assert client.agent_instructions() == AGENT_INSTRUCTIONS + "\n\nPERSONA" - # the chat lanes' prompt: header, base, client, then the call's texts managed = local_chat._managed_instructions(client, ["CALL", "HISTORY"]) marks = [managed.index(m) for m in (CHAT_HEADER, AGENT_INSTRUCTIONS, "PERSONA", "CALL", "HISTORY")] @@ -3861,15 +3860,25 @@ def test_client_instructions_follow_the_managed_base_everywhere(store_path): blocks = local_chat._anthropic_system(client, "CALL", None) assert blocks[0]["text"].endswith("\n\nPERSONA") assert blocks[1]["text"] == "CALL" - # unset: the base alone, byte-identical to before plain = PageIndexLocalClient(storage_path=store_path) assert plain.agent_instructions() == AGENT_INSTRUCTIONS +@needs_agents +def test_chat_reaches_the_model_with_client_instructions(store_path, + fake_model): + client = PageIndexLocalClient(storage_path=store_path, + instructions="PERSONA") + fake = fake_model([[_msg_item("ok")]]) + assert client.chat([{"role": "system", "content": "CALL"}, + {"role": "user", "content": "hi"}]) == "ok" + assert fake.instructions[0].endswith("\n\nPERSONA\n\nCALL") + + def test_bridge_client_instructions_follow_the_live_instructions( bridge_client): client, _ = bridge_client - client.instructions = "PERSONA" # a plain attribute, read per call + client.instructions = "PERSONA" assert client.agent_instructions() == "CLOUD LIVE INSTRUCTIONS\n\nPERSONA" @@ -3896,9 +3905,7 @@ def test_claude_agent_config_carries_client_instructions(store_path): def test_managed_chat_sends_one_leading_system_row(monkeypatch): - """The managed endpoint takes one system message, first: the client's - instructions, the call's, and the history's system rows (any - position) fold into it, in that order.""" + """One system row first: the client's, the call's, then the history's.""" cloud = PageIndexCloudClient(api_key="pi-k", instructions="PERSONA") seen = {} monkeypatch.setattr(cloud._api, "chat_completions", @@ -3914,23 +3921,19 @@ def test_managed_chat_sends_one_leading_system_row(monkeypatch): {"role": "user", "content": "q1"}, {"role": "assistant", "content": "a1"}, {"role": "user", "content": "q2"}] - # a bare question, and the streamed door, ride the same fold monkeypatch.setattr(cloud._api, "chat_completions", lambda **kw: seen.update(kw) or iter([])) assert list(cloud.chat("q", stream=True, show_process=False)) == [] assert seen["messages"] == [{"role": "system", "content": "PERSONA"}, {"role": "user", "content": "q"}] - # developer rows are system text here too, as on the own-model lane cloud.chat_completions([{"role": "user", "content": "q"}, {"role": "developer", "content": "DEV"}]) assert seen["messages"][0] == {"role": "system", "content": "PERSONA\n\nDEV"} -def test_managed_chat_sends_the_canonical_history(monkeypatch): - """No client instructions: the payload is the history as given, a - leading system row kept in place; blank system rows and fields - beyond role/content are dropped, as on the own-model lane.""" +def test_managed_chat_forwards_the_rest_of_the_history_verbatim(monkeypatch): + """Only system rows fold; blank ones drop; the rest goes as given.""" cloud = PageIndexCloudClient(api_key="pi-k") seen = {} monkeypatch.setattr(cloud._api, "chat_completions", @@ -3940,22 +3943,11 @@ def test_managed_chat_sends_the_canonical_history(monkeypatch): {"role": "user", "content": "q"}] cloud.chat(leading) assert seen["messages"] == leading - cloud.chat([{"role": "user", "content": "q", "name": "ray"}, - {"role": "system", "content": " "}]) - assert seen["messages"] == [{"role": "user", "content": "q"}] - - -def test_managed_chat_history_contract_matches_the_own_model_lane( - monkeypatch): - """One answer-lane contract on both engines: text history only. The - endpoint refuses tool rows and structured content itself (400/422); - the SDK says so first, with the protocol-lane pointer.""" - cloud = PageIndexCloudClient(api_key="pi-k") - monkeypatch.setattr(cloud._api, "chat_completions", - lambda **kw: pytest.fail("must not reach the wire")) - with pytest.raises(PageIndexAPIError, match="Unsupported role"): - cloud.chat([{"role": "user", "content": "q"}, - {"role": "tool", "tool_call_id": "c", "content": "x"}]) - with pytest.raises(PageIndexAPIError, match="content must be a string"): - cloud.chat([{"role": "user", - "content": [{"type": "text", "text": "q"}]}]) + history = [{"role": "user", "content": [{"type": "text", "text": "q"}], + "name": "ray"}, + {"role": "assistant", "content": None, + "tool_calls": [{"id": "c"}]}, + {"role": "tool", "tool_call_id": "c", "content": "x"}, + {"role": "system", "content": " "}] + cloud.chat(history) + assert seen["messages"] == history[:-1] From dfef3ab7e639ac88c2c67413146aac616b7550b9 Mon Sep 17 00:00:00 2001 From: Ray Date: Thu, 10 Sep 2026 12:56:23 +0800 Subject: [PATCH 4/6] Chat history: normalize the container, validate only what the SDK reads Both lanes now take any iterable of message dicts. chat()'s instructions prepend expanded only lists, so a tuple or generator on the managed lane silently dropped the call's instructions once _require_own_chat no longer refused it; own-model rejected the same shapes outright. list() once at each reader instead. None and other non-iterables fail with Python's own TypeError, as in the OpenAI SDK. _system_text refuses a system row carrying non-text parts instead of keeping the text parts and dropping the rest: the SDK folds that row into its own system text, so it is the reader and must say what it could not read. The endpoint stays the authority on every row the fold leaves in place. Restore the messages docstring sentence 108875b rewrote: tool-role turns are rejected on both engines (the endpoint 400s them), so the managed lane does not take them "verbatim"; rename the test that carried that claim. Cover the blank-text filter, which no test guarded, and drop a truncated comment. Claude-Session: https://claude.ai/code/session_01UBpLu7TJUvrvLvFacs8WYT --- pageindex/client.py | 16 +++++++--------- pageindex/local_chat.py | 7 ++++--- tests/test_local_chat.py | 33 ++++++++++++++++++++++++++++++--- 3 files changed, 41 insertions(+), 15 deletions(-) diff --git a/pageindex/client.py b/pageindex/client.py index 91d95781d..d1723c0ba 100644 --- a/pageindex/client.py +++ b/pageindex/client.py @@ -1214,11 +1214,10 @@ def chat( "messages must be a non-empty string or a list of " "message dicts.") messages = [{"role": "user", "content": messages}] - if isinstance(messages, list): - # The first system text: managed prompt, then instructions, - # then the history's own system rows. - messages = [{"role": "system", "content": instructions}, - *messages] + # The first system text: managed prompt, then instructions, + # then the history's own system rows. + messages = [{"role": "system", "content": instructions}, + *messages] if protocol == "chat_completions": return self.chat_completions( messages, stream=stream, stream_metadata=True, doc_id=doc_id, @@ -1321,10 +1320,9 @@ def chat_completions( System/developer messages, wherever they sit, join the managed system prompt after the client's ``instructions`` (the managed endpoint receives them as its one leading - system message). Own-model chat takes text history only: - tool-role turns are rejected and fields beyond - role/content are dropped; the managed endpoint receives - the rest of the history verbatim. + system message); the history is text only: tool-role + turns are rejected on both engines, and message + fields beyond role/content are dropped. stream: Enable streaming responses. doc_id: Document ID or list of IDs to scope the conversation. Keep it identical across a conversation's calls — the diff --git a/pageindex/local_chat.py b/pageindex/local_chat.py index 313d8547b..d2da77307 100644 --- a/pageindex/local_chat.py +++ b/pageindex/local_chat.py @@ -38,9 +38,9 @@ def _system_text(content: Any) -> str: if isinstance(content, str): return content if isinstance(content, list): - texts = [part.get("text") for part in content + texts = [part["text"] for part in content if isinstance(part, dict) and isinstance(part.get("text"), str)] - if texts: + if texts and len(texts) == len(content): return "\n".join(texts) raise PageIndexAPIError( "system message content must be a string or a list of text parts." @@ -52,7 +52,8 @@ def _split_chat_messages(messages) -> "tuple[list[str], list[dict]]": content joins the managed instructions; user/assistant history passes through. Tool-history round-trips belong to chat(protocol="responses") or chat(protocol="messages").""" - if not isinstance(messages, list) or not messages: + messages = list(messages) + if not messages: raise PageIndexAPIError("messages must be a non-empty list.") system_texts: list[str] = [] history: list[dict] = [] diff --git a/tests/test_local_chat.py b/tests/test_local_chat.py index 1000642d3..b1f192084 100644 --- a/tests/test_local_chat.py +++ b/tests/test_local_chat.py @@ -3856,7 +3856,6 @@ def test_client_instructions_follow_the_managed_base_everywhere(store_path): marks = [managed.index(m) for m in (CHAT_HEADER, AGENT_INSTRUCTIONS, "PERSONA", "CALL", "HISTORY")] assert marks == sorted(marks) - # Messages lane: inside the cached managed block, before the call's blocks = local_chat._anthropic_system(client, "CALL", None) assert blocks[0]["text"].endswith("\n\nPERSONA") assert blocks[1]["text"] == "CALL" @@ -3932,8 +3931,8 @@ def test_managed_chat_sends_one_leading_system_row(monkeypatch): "content": "PERSONA\n\nDEV"} -def test_managed_chat_forwards_the_rest_of_the_history_verbatim(monkeypatch): - """Only system rows fold; blank ones drop; the rest goes as given.""" +def test_managed_fold_leaves_non_system_rows_to_the_endpoint(monkeypatch): + """Only system rows fold; blank ones drop; the rest is not validated.""" cloud = PageIndexCloudClient(api_key="pi-k") seen = {} monkeypatch.setattr(cloud._api, "chat_completions", @@ -3951,3 +3950,31 @@ def test_managed_chat_forwards_the_rest_of_the_history_verbatim(monkeypatch): {"role": "system", "content": " "}] cloud.chat(history) assert seen["messages"] == history[:-1] + + +def test_chat_history_takes_any_iterable(monkeypatch): + """Tuples and generators ride both lanes; the call's instructions land.""" + cloud = PageIndexCloudClient(api_key="pi-k") + seen = {} + monkeypatch.setattr(cloud._api, "chat_completions", + lambda **kw: seen.update(kw) or { + "choices": [{"message": {"content": "ok"}}]}) + row = {"role": "user", "content": "q"} + cloud.chat(iter([row]), instructions="CALL") + assert seen["messages"] == [{"role": "system", "content": "CALL"}, row] + assert local_chat._split_chat_messages((row,)) == ([], [row]) + + +def test_system_text_refuses_non_text_parts(): + text = {"type": "text", "text": "A"} + assert local_chat._system_text( + [text, {"type": "text", "text": "B"}]) == "A\nB" + with pytest.raises(PageIndexAPIError, match="text parts"): + local_chat._system_text( + [text, {"type": "image_url", "image_url": {"url": "u"}}]) + + +def test_managed_instructions_drop_blank_system_texts(store_path): + client = PageIndexLocalClient(storage_path=store_path) + assert (local_chat._managed_instructions(client, ["", " ", "X"]) + == local_chat._managed_instructions(client, ["X"])) From 1eb74ecf7bf0f4fb028245ee28f46eb9d663cc83 Mon Sep 17 00:00:00 2001 From: Ray Date: Thu, 10 Sep 2026 16:37:23 +0800 Subject: [PATCH 5/6] fix: ctor type error names the answer, not a lane managed clients cannot use A plain managed client is the caller most likely to pass Messages-style blocks as instructions=, and the old message sent it to chat(protocol="messages"), which that same client refuses. Say "pass text", and make the block pointer the exact working call: model= is required on that lane, and it needs a chat_model= client. Claude-Session: https://claude.ai/code/session_018od4o3YBqrny2vzGhrX4q5 --- pageindex/client.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pageindex/client.py b/pageindex/client.py index d1723c0ba..ffac91d02 100644 --- a/pageindex/client.py +++ b/pageindex/client.py @@ -364,9 +364,10 @@ def __init__( ) if instructions is not None and not isinstance(instructions, str): raise PageIndexAPIError( - f"instructions must be a str, got {type(instructions).__name__} " - "— Messages system blocks go on chat(protocol=\"messages\", " - "instructions=[...]).") + f"instructions must be a str, got {type(instructions).__name__}. " + "Pass the guidance as text; Messages system blocks belong to " + "chat(protocol=\"messages\", model=..., instructions=[...]) on " + "a client with chat_model=... set.") self.instructions = (instructions or "").strip() or None # Each side picks one spelling — its slot, or the flat arguments. # ``model`` sets every role, so it claims both sides. From 75f93e8d4998cdcb7070a3002fc74f74d2d3fcaf Mon Sep 17 00:00:00 2001 From: Ray Date: Thu, 10 Sep 2026 19:58:36 +0800 Subject: [PATCH 6/6] Rebase onto the merged lanes: two pins the lift and the targeting move void The chat_completions protocol lane's guard test still pinned the managed refusal of instructions=, which this branch lifts on purpose; its own managed-fold tests cover what the lane now does. And _anthropic_system lost its doc_id argument when targeting moved to the first user message, so the prompt-order assertion calls it with what it takes. --- tests/test_local_chat.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/test_local_chat.py b/tests/test_local_chat.py index b1f192084..0ce0e1627 100644 --- a/tests/test_local_chat.py +++ b/tests/test_local_chat.py @@ -3353,8 +3353,6 @@ def test_chat_protocol_chat_completions_serves_managed_cloud(monkeypatch): extra_body={"messages": []}) with pytest.raises(PageIndexAPIError, match="chat_model="): cloud.chat("q", protocol="chat_completions", model="m") - with pytest.raises(PageIndexAPIError, match="chat_model="): - cloud.chat("q", protocol="chat_completions", instructions="x") def test_chat_takes_only_messages_by_position(): @@ -3856,7 +3854,7 @@ def test_client_instructions_follow_the_managed_base_everywhere(store_path): marks = [managed.index(m) for m in (CHAT_HEADER, AGENT_INSTRUCTIONS, "PERSONA", "CALL", "HISTORY")] assert marks == sorted(marks) - blocks = local_chat._anthropic_system(client, "CALL", None) + blocks = local_chat._anthropic_system(client, "CALL") assert blocks[0]["text"].endswith("\n\nPERSONA") assert blocks[1]["text"] == "CALL" plain = PageIndexLocalClient(storage_path=store_path)