diff --git a/pageindex/agent_tools.py b/pageindex/agent_tools.py index 671625112..afac6b9d5 100644 --- a/pageindex/agent_tools.py +++ b/pageindex/agent_tools.py @@ -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]: diff --git a/pageindex/client.py b/pageindex/client.py index a36124cba..ffac91d02 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__}. " + "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 @@ 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,18 +1209,16 @@ 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( "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 @@ 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 +1422,20 @@ def chat_completions( "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 @@ 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 @@ -2009,6 +2040,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 +2170,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 +2185,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 +2206,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/pageindex/local_chat.py b/pageindex/local_chat.py index f60df2c26..d2da77307 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: @@ -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] = [] diff --git a/tests/test_client.py b/tests/test_client.py index 260838045..4e63223bf 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -2160,3 +2160,29 @@ 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" + 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"}]) diff --git a/tests/test_local_chat.py b/tests/test_local_chat.py index 204aacf72..0ce0e1627 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) @@ -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"]))