Client instructions: a standing persona for every answer surface (#497)

* Client instructions: a standing persona for every answer surface 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 * Managed fold: always send the canonical history 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 * 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 * 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 * 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 * 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.
2026-10-02 07:44:37 +08:00 · 2026-09-10 20:05:27 +08:00
parent a3364c8a1b
commit fc2dc41c16
5 changed files with 238 additions and 42 deletions
@@ -1691,16 +1691,18 @@ 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."
+            )
+    own = getattr(client, "instructions", None)
+    return f"{base}\n\n{own}" if own else base


 def doc_targeting_block(client, doc_id) -> Optional[str]:
@@ -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 @@ class PageIndexClient:
        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__}. "
+                "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.
        index_flat: dict[str, Any] = {
@@ -519,9 +535,8 @@ class PageIndexClient:
        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 @@ class PageIndexClient:
                — 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 — ``<cite doc="…" page="…"/>`` tags, ``block="…"``
                added where the cloud document has blocks. The guidance is
@@ -1192,18 +1209,16 @@ class PageIndexClient:
                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(
                        "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,
@@ -1303,10 +1318,11 @@ class PageIndexClient:
        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 +1422,20 @@ class PageIndexClient:
                "agent in your process, or drop them to use the managed "
                "chat endpoint, which selects its own model."
            )
+        # 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
        return cast(CloudAPI, self._api).chat_completions(
            messages=messages, stream=stream, doc_id=doc_id,
@@ -1943,10 +1973,11 @@ class PageIndexClient:
        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
@@ -2009,6 +2040,7 @@ class PageIndexClient:
        ``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 +2170,7 @@ class PageIndexCloudClient(PageIndexClient):
        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 +2185,7 @@ class PageIndexCloudClient(PageIndexClient):
                )
        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 +2206,11 @@ class PageIndexLocalClient(PageIndexClient):
        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)
@@ -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:
@@ -37,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."
@@ -51,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] = []
@@ -2160,3 +2160,29 @@ def test_retry_notice_only_when_a_retry_follows(monkeypatch, caplog):
    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"
+    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
+    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"}])
@@ -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)

@@ -3355,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():
@@ -3845,3 +3841,138 @@ 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"
+    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)
+    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)
+    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"
+    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):
+    """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",
+                        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"}]
+    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"}]
+    cloud.chat_completions([{"role": "user", "content": "q"},
+                            {"role": "developer", "content": "DEV"}])
+    assert seen["messages"][0] == {"role": "system",
+                                   "content": "PERSONA\n\nDEV"}
+
+
+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",
+                        lambda **kw: seen.update(kw) or {
+                            "choices": [{"message": {"content": "ok"}}]})
+    leading = [{"role": "system", "content": "S"},
+               {"role": "user", "content": "q"}]
+    cloud.chat(leading)
+    assert seen["messages"] == leading
+    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]
+
+
+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"]))