diff --git a/pageindex/client.py b/pageindex/client.py index f9bb16a90..45c487483 100644 --- a/pageindex/client.py +++ b/pageindex/client.py @@ -1041,10 +1041,13 @@ def chat( wire names (Responses ``max_output_tokens``, Messages ``thinking`` / ``top_k``), merged last so they win. Answer lane: LiteLLM's own params, mapped or refused per - provider (its named sampling params are - ``chat_completions()``'s); protocol lanes: verbatim into - the request body. Credentials belong in ``backend``, - never here. + provider (``response_format`` has no door on + LiteLLM-routed models); protocol lanes: verbatim into + the request body. The managed prompt, conversation and + tools are not fields here (``system`` / ``instructions`` + / ``input`` / ``messages`` / ``tools`` are refused); + extend the prompt with ``instructions=``. Credentials + belong in ``backend``, never here. Returns: - answer lane, stream=False: the answer string @@ -1252,7 +1255,10 @@ def chat_completions( OpenAI-compatible backends take them verbatim in the request body; LiteLLM-routed providers take them as LiteLLM's own params (mapped or refused per provider). - Credentials belong in ``backend``, never here. + The managed prompt, conversation and tools are not + fields here (``system`` / ``instructions`` / ``input`` / + ``messages`` / ``tools`` are refused). Credentials belong + in ``backend``, never here. extra_headers: Own-model chat only — extra HTTP headers merged into each backend request; caller headers win. One exception: LiteLLM's anthropic adapter owns the ``anthropic-beta`` diff --git a/pageindex/local_chat.py b/pageindex/local_chat.py index af8f56804..343516f9e 100644 --- a/pageindex/local_chat.py +++ b/pageindex/local_chat.py @@ -4,6 +4,7 @@ from __future__ import annotations import asyncio +import dataclasses import hashlib import json import queue @@ -302,10 +303,26 @@ def _merged_backend(client, backend): return merged or None +_SKELETON_KEYS = frozenset({"system", "instructions", "input", "messages", + "tools"}) + + +def _refuse_skeleton(extra_body) -> None: + """The managed prompt, conversation and tools are the SDK's on every + lane; extra_body merges last, so a caller's copy would replace them.""" + hit = sorted(_SKELETON_KEYS.intersection(extra_body or ())) + if hit: + raise PageIndexAPIError( + f"extra_body cannot carry {', '.join(hit)}: the managed prompt, " + "conversation and tools are the SDK's. Extend the prompt with " + "instructions=; the conversation is the first argument.") + + def _openai_agent(client, protocol: str, model_name: str, instructions: str, temperature, top_p, doc_ids=None, cache_key=None, reasoning=None, reasoning_effort=None, extra_body=None, max_tokens=None, backend=None, extra_headers=None): + _refuse_skeleton(extra_body) from agents import Agent, ModelSettings from .integrations.openai_agents import build_openai_tools # ModelSettings.extra_body is the one channel all three engines put on @@ -335,22 +352,30 @@ def _openai_agent(client, protocol: str, model_name: str, instructions: str, if cache_key and openai_backend else None) # Caller extras merge last, so they win over ours; non-OpenAI # destinations take them as LiteLLM kwargs instead (see note above). + routed: dict[str, Any] = {} if extra_body: if openai_backend: body = {**(body or {}), **extra_body} else: - extra_args = {**(extra_args or {}), **extra_body} + # openai-agents passes ModelSettings' own fields to litellm by + # name beside **extra_args, so those ride their field. + own = {field.name for field in dataclasses.fields(ModelSettings)} + routed = {k: v for k, v in extra_body.items() if k in own} + rest = {k: v for k, v in extra_body.items() if k not in own} + if rest: + extra_args = {**(extra_args or {}), **rest} from pydantic import ValidationError try: - settings = ModelSettings( - temperature=temperature, top_p=top_p, max_tokens=max_tokens, - reasoning=reasoning, + settings = ModelSettings(**{ + "temperature": temperature, "top_p": top_p, + "max_tokens": max_tokens, "reasoning": reasoning, # Streamed runs otherwise carry no usage at all (agents forwards # this as stream_options only on streaming calls). - include_usage=True, - extra_body=body, - extra_headers=extra_headers, - extra_args=extra_args) + "include_usage": True, + "extra_body": body, + "extra_headers": extra_headers, + "extra_args": extra_args, + **routed}) except ValidationError as exc: raise PageIndexAPIError(f"Invalid model settings: {exc}") from exc return Agent( @@ -1365,6 +1390,7 @@ def run_messages(client, messages, model: str, _require_anthropic() import anthropic _validate_max_turns(max_turns) + _refuse_skeleton(extra_body) if isinstance(messages, str) and messages.strip(): messages = [{"role": "user", "content": messages}] if (not isinstance(messages, list) or not messages diff --git a/tests/test_local_chat.py b/tests/test_local_chat.py index 1097383e0..c670a4eaf 100644 --- a/tests/test_local_chat.py +++ b/tests/test_local_chat.py @@ -1522,6 +1522,36 @@ def test_reasoning_passthrough_reaches_each_engine(monkeypatch): assert agent.model_settings.extra_args is None +@needs_agents +def test_extra_body_model_settings_fields_ride_their_field(monkeypatch): + """LiteLLM-routed answer lane: openai-agents passes ModelSettings' + own fields to litellm by name beside **extra_args, so a caller's copy + in extra_args collided with them. Those ride their field, the + caller's value winning; the rest stay LiteLLM kwargs. OpenAI + destinations keep the request-body path.""" + pytest.importorskip("litellm") + monkeypatch.setattr(local_chat, "_openai_model", lambda *a: None) + monkeypatch.setattr("pageindex.integrations.openai_agents.build_openai_tools", + lambda *a, **k: []) + agent = local_chat._openai_agent(None, "chat", "anthropic/claude-x", + "sys", 0.7, None, + extra_body={"temperature": 0.2, + "top_k": 5}) + settings = agent.model_settings + assert settings.temperature == 0.2 + assert settings.extra_args["top_k"] == 5 + assert "temperature" not in settings.extra_args + agent = local_chat._openai_agent(None, "chat", "gpt-test", "sys", + None, None, + extra_body={"temperature": 0.2}) + assert agent.model_settings.temperature is None + assert agent.model_settings.extra_body == {"temperature": 0.2} + with pytest.raises(PageIndexAPIError, match="Invalid model settings"): + local_chat._openai_agent(None, "chat", "anthropic/claude-x", "sys", + None, None, + extra_body={"temperature": "hot"}) + + @needs_agents def test_extra_body_passthrough_reaches_each_engine(monkeypatch): """Caller extras merge last — over the cache key on OpenAI @@ -3162,6 +3192,31 @@ def test_chat_protocol_messages_is_the_door(client, monkeypatch): assert seen[-1][1]["extra_body"] is None +def test_extra_body_refuses_skeleton_keys(): + """The managed prompt, conversation and tools are the SDK's on every + lane; extra_body merges last, so a caller's copy would silently + replace them. Refused at the seam both openai-agents lanes share.""" + for key in ("system", "instructions", "input", "messages", "tools"): + with pytest.raises(PageIndexAPIError, match="instructions="): + local_chat._openai_agent(None, "chat", "gpt-test", "sys", + None, None, extra_body={key: "x"}) + with pytest.raises(PageIndexAPIError, match="instructions="): + local_chat._openai_agent(None, "responses", "gpt-test", "sys", + None, None, extra_body={"input": "x"}) + + +@needs_anthropic +def test_messages_extra_body_refuses_skeleton_before_transport(client, + monkeypatch): + made = [] + monkeypatch.setattr(local_chat, "_anthropic_client", + lambda backend=None: made.append(1)) + with pytest.raises(PageIndexAPIError, match="instructions="): + local_chat.run_messages(client, "q", model="claude-x", + extra_body={"system": "x"}) + assert made == [] + + def test_chat_protocol_chokes(client, monkeypatch): monkeypatch.setattr(local_chat, "run_responses", lambda c, input, **kw: "door")