diff --git a/packages/claude-agents/src/launchdarkly_ai_claude_agents/handler.py b/packages/claude-agents/src/launchdarkly_ai_claude_agents/handler.py index 7376e7f5..a7c71a83 100644 --- a/packages/claude-agents/src/launchdarkly_ai_claude_agents/handler.py +++ b/packages/claude-agents/src/launchdarkly_ai_claude_agents/handler.py @@ -53,6 +53,8 @@ ) from launchdarkly_ai_server.agui import TurnEmitter from launchdarkly_ai_server.lifecycle import effective_capture_content +from launchdarkly_ai_server.parameter_forwarding import select_forwarded_parameters +from launchdarkly_ai_server.utils import model_parameters from ._version import PACKAGE_NAME, __version__ from .spans import ( @@ -73,6 +75,94 @@ tool_display_name, ) +#: Every field ``ClaudeAgentOptions`` declares, classified by hand into exactly one of: forwarded +#: (below), handler-owned (``model``, ``allowed_tools``, ``mcp_servers``, ``hooks``, ``tools``, +#: ``system_prompt``, set by each call site itself), or excluded (below). +#: ``TestClaudeAgentOptionsAcceptsExactlyTheseFields`` in this package's tests asserts this +#: classification stays exhaustive as the SDK's own dataclass changes. +#: +#: Only model and run settings are forwarded: how the model thinks, how long the run may go, and +#: what it may spend. Everything that configures the host process the SDK launches (its binary, +#: environment, working directory, file access, permissions, settings files, plugins, sandbox, +#: session state) stays under the application's control, never a config's. +#: +#: This is the cross-SDK list for Claude Agents (TESTING.md section 1.12), shared with the native +#: graph through :func:`_options_parameters`. +#: +#: The SDK offers no ``temperature``/``top_p``/``top_k``/``max_tokens``/``stop_sequences``/ +#: ``tool_choice``/``metadata``, all of which the LaunchDarkly UI's model parameters panel offers +#: for other providers; they are dropped like any other key not listed here. +_CLAUDE_AGENT_OPTIONS_FORWARDED_KEYS = frozenset( + { + "betas", + "effort", + "fallback_model", + "max_budget_usd", + "max_thinking_tokens", + "max_turns", + "output_format", + "thinking", + } +) + +#: Forwarded keys whose value must be an object. A config value of any other type is malformed +#: and dropped rather than passed to ``ClaudeAgentOptions``. +_CLAUDE_AGENT_OPTIONS_MAPPING_KEYS = frozenset({"output_format", "thinking"}) + +#: Accepted by ``ClaudeAgentOptions`` but never forwarded, and why: +#: * ``cli_path``, ``env``, ``cwd``, ``add_dirs``, ``settings``, ``setting_sources``, ``plugins``, +#: ``skills``, ``sandbox``, ``user``, ``extra_args``: which binary runs, with what environment, +#: as which user, with what files, settings, plugins, and CLI arguments. A config that could set +#: these could run code on, or read files from, the host. +#: * ``permission_mode``, ``permission_prompt_tool_name``, ``can_use_tool``, ``disallowed_tools``, +#: ``strict_mcp_config``, ``agents``: what the agent is allowed to do and which tools or +#: subagents it gets. The handler wires tools and permissions itself. +#: * ``resume``, ``session_id``, ``fork_session``, ``continue_conversation``, ``session_store``, +#: ``session_store_flush``, ``enable_file_checkpointing``: session state on the host. +#: * ``stderr``, ``debug_stderr``, ``include_partial_messages``, ``include_hook_events``, +#: ``max_buffer_size``, ``load_timeout_ms``: process I/O and transport plumbing, including the +#: streamed message shape the handler reads. +#: * ``task_budget``: not one of the agreed run settings yet; ``max_turns`` and ``max_budget_usd`` +#: cover run limits. +#: +#: Named for the drift test and for review, not read at runtime: the forwarded list above already +#: leaves these out, so nothing needs to subtract them again. +_CLAUDE_AGENT_OPTIONS_EXCLUDED_KEYS = frozenset( + { + "add_dirs", + "agents", + "can_use_tool", + "cli_path", + "continue_conversation", + "cwd", + "debug_stderr", + "disallowed_tools", + "enable_file_checkpointing", + "env", + "extra_args", + "fork_session", + "include_hook_events", + "include_partial_messages", + "load_timeout_ms", + "max_buffer_size", + "permission_mode", + "permission_prompt_tool_name", + "plugins", + "resume", + "sandbox", + "session_id", + "session_store", + "session_store_flush", + "setting_sources", + "settings", + "skills", + "stderr", + "strict_mcp_config", + "task_budget", + "user", + } +) + # --------------------------------------------------------------------------- # Tool wiring # --------------------------------------------------------------------------- @@ -504,6 +594,18 @@ def _raise_if_run_failed(message: Any) -> None: ) +def _options_parameters(config: AiConfigRep) -> dict[str, Any]: + """The config's ``model.parameters`` that become ``ClaudeAgentOptions`` fields: only + :data:`_CLAUDE_AGENT_OPTIONS_FORWARDED_KEYS`, with malformed object values dropped. The handler + and the native graph both call this, so they forward the same keys. + """ + return select_forwarded_parameters( + model_parameters(config), + _CLAUDE_AGENT_OPTIONS_FORWARDED_KEYS, + mapping_keys=_CLAUDE_AGENT_OPTIONS_MAPPING_KEYS, + ) + + def _build_query_options( config: AiConfigRep, system_prompt: str | None, @@ -514,7 +616,9 @@ def _build_query_options( **extra: Any, ) -> ClaudeAgentOptions: all_allowed = [*mcp_allowed_tools, *native_tool_names] + params = _options_parameters(config) kwargs: dict[str, Any] = { + **params, "model": config["model"]["name"], "allowed_tools": all_allowed if all_allowed else [], "mcp_servers": {TOOL_MCP_NAME: tool_mcp} if tool_mcp else {}, diff --git a/packages/claude-agents/src/launchdarkly_ai_claude_agents/native_graph.py b/packages/claude-agents/src/launchdarkly_ai_claude_agents/native_graph.py index aea24b7d..50488717 100644 --- a/packages/claude-agents/src/launchdarkly_ai_claude_agents/native_graph.py +++ b/packages/claude-agents/src/launchdarkly_ai_claude_agents/native_graph.py @@ -37,6 +37,7 @@ from launchdarkly_ai_claude_agents.handler import ( _build_hooks, + _options_parameters, _raise_if_run_failed, build_prompt, build_query_prompt, @@ -158,7 +159,10 @@ async def _run_query( hooks = _build_hooks(native_tool_map) + params = _options_parameters(node.config) + options = ClaudeAgentOptions( + **params, # Explicitly set the available built-in tools (empty list disables all). # When no native tools are needed, disable built-in tools so Claude # cannot call WebSearch/Bash/etc. and get stuck waiting for permission diff --git a/packages/claude-agents/tests/test_handler.py b/packages/claude-agents/tests/test_handler.py index b4b67d01..663d6a57 100644 --- a/packages/claude-agents/tests/test_handler.py +++ b/packages/claude-agents/tests/test_handler.py @@ -21,6 +21,7 @@ import pytest from claude_agent_sdk import ( AssistantMessage, + ClaudeAgentOptions, ResultMessage, StreamEvent, SystemMessage, @@ -43,6 +44,12 @@ partition_tools, ) from launchdarkly_ai_server import ConversationIdSpanProcessor, conversation_id +from tests.forwarding_spec import ( + CLAUDE_AGENTS, + candidate_keys, + probe_forwarded_keys, +) +from tests.never_forwarded import NEVER_FORWARDED_BAG, find_leaks # --------------------------------------------------------------------------- # A real tracer provider, reset between tests @@ -1606,6 +1613,125 @@ async def test_ld_span_attributes_land_on_root_only( assert [e.name for e in root().events] == ["feature_flag"] +class TestModelParametersForwarding: + async def _run_and_capture_options( + self, config: dict[str, Any], monkeypatch: pytest.MonkeyPatch + ) -> Any: + captured: dict[str, Any] = {} + + async def _query(**kwargs: Any) -> AsyncIterator[Any]: + captured["options"] = kwargs["options"] + yield assistant_message() + yield result_message() + + monkeypatch.setattr(handler_mod, "query", _query) + await create_claude_agents_handler()(config, "q") + return captured["options"] + + async def test_max_turns_from_config_reaches_options( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + config = { + **BASE_CONFIG, + "model": {**BASE_CONFIG["model"], "parameters": {"max_turns": 3}}, + } + options = await self._run_and_capture_options(config, monkeypatch) + assert options.max_turns == 3 + + async def test_config_cannot_override_model_or_system_prompt( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + config = { + **BASE_CONFIG, + "model": { + **BASE_CONFIG["model"], + "parameters": { + "model": "not-the-real-model", + "system_prompt": "not-the-real-prompt", + }, + }, + } + options = await self._run_and_capture_options(config, monkeypatch) + assert options.model == BASE_CONFIG["model"]["name"] + assert options.system_prompt != "not-the-real-prompt" + + async def test_unset_when_no_parameters( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + options = await self._run_and_capture_options(BASE_CONFIG, monkeypatch) + assert options.max_turns is None + + async def test_ui_keys_the_sdk_rejects_are_dropped_without_raising( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """``ClaudeAgentOptions`` has no ``temperature``/``top_p``/``top_k``/``max_tokens``/ + ``stop_sequences``/``tool_choice``/``metadata`` fields, all of which the LaunchDarkly UI's + model parameters panel offers for other providers. Forwarding one unfiltered raises + ``TypeError`` before any request is made; the filter must drop them instead. + """ + config = { + **BASE_CONFIG, + "model": { + **BASE_CONFIG["model"], + "parameters": { + "temperature": 0.2, + "top_p": 0.5, + "top_k": 10, + "max_tokens": 256, + "stop_sequences": ["STOP"], + "tool_choice": "auto", + "metadata": {"user_id": "u1"}, + "max_turns": 3, + }, + }, + } + options = await self._run_and_capture_options(config, monkeypatch) + assert options.max_turns == 3 + for rejected in ( + "temperature", + "top_p", + "top_k", + "max_tokens", + "stop_sequences", + "tool_choice", + "metadata", + ): + assert not hasattr(options, rejected) or getattr(options, rejected) is None + + async def test_no_never_forwarded_key_reaches_the_query_options( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Every credential, endpoint, request-injection, remote-tool, and host-process key, + including the real ``ClaudeAgentOptions`` fields ``cli_path``, ``env``, ``cwd``, + ``add_dirs``, ``permission_mode`` and ``can_use_tool``, is dropped on the way to + ``query``; the agreed run setting still lands.""" + config = { + **BASE_CONFIG, + "model": { + **BASE_CONFIG["model"], + "parameters": {**NEVER_FORWARDED_BAG, "max_turns": 2}, + }, + } + options = await self._run_and_capture_options(config, monkeypatch) + assert options.max_turns == 2 + assert not find_leaks(options) + + async def test_temperature_top_p_and_max_turns_run_without_type_error( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """A realistic combination of UI-offered keys must not raise, and the one real field + (``max_turns``) must still land.""" + config = { + **BASE_CONFIG, + "model": { + **BASE_CONFIG["model"], + "parameters": {"temperature": 0.3, "top_p": 0.8, "max_turns": 5}, + }, + } + options = await self._run_and_capture_options(config, monkeypatch) + assert options.max_turns == 5 + + class TestFinishReasonMapping: async def test_tool_use_maps_to_tool_calls( self, monkeypatch: pytest.MonkeyPatch @@ -2395,6 +2521,92 @@ async def _query(**kwargs: Any) -> AsyncIterator[Any]: assert tool.end_time is not None +#: ``ClaudeAgentOptions`` fields the handler fills with fresh objects on every call (hook +#: closures, the tool MCP server), so they differ between calls whatever the config says. Every +#: config-settable field is still compared. +_PER_CALL_FIELDS = frozenset({"hooks", "mcp_servers"}) + + +def _comparable(options: Any) -> dict[str, Any]: + import dataclasses + + return { + f.name: getattr(options, f.name) + for f in dataclasses.fields(options) + if f.name not in _PER_CALL_FIELDS + } + + +class TestForwardsExactlyTheCrossSdkList: + """Probes ``invoke`` and ``stream`` one key at a time: the keys that change the + ``ClaudeAgentOptions`` handed to ``query`` are exactly the cross-SDK Claude Agents list, on + both paths.""" + + @staticmethod + def _candidates() -> frozenset[str]: + import dataclasses + + return candidate_keys( + (f.name for f in dataclasses.fields(ClaudeAgentOptions)), CLAUDE_AGENTS + ) + + @staticmethod + def _config(parameters: dict[str, Any]) -> dict[str, Any]: + return { + **BASE_CONFIG, + "model": {**BASE_CONFIG["model"], "parameters": parameters}, + } + + def _patch_query(self, monkeypatch: pytest.MonkeyPatch) -> dict[str, Any]: + captured: dict[str, Any] = {} + + async def _query(**kwargs: Any) -> AsyncIterator[Any]: + captured["options"] = kwargs["options"] + yield assistant_message() + yield result_message() + + monkeypatch.setattr(handler_mod, "query", _query) + return captured + + async def test_invoke(self, monkeypatch: pytest.MonkeyPatch) -> None: + captured = self._patch_query(monkeypatch) + h = create_claude_agents_handler() + + async def call(parameters: dict[str, Any]) -> object: + await h(self._config(parameters), "q") + return _comparable(captured["options"]) + + assert await probe_forwarded_keys(self._candidates(), call) == CLAUDE_AGENTS + + async def test_stream(self, monkeypatch: pytest.MonkeyPatch) -> None: + captured = self._patch_query(monkeypatch) + h = create_claude_agents_handler() + + async def call(parameters: dict[str, Any]) -> object: + await _collect(await h.stream(self._config(parameters), "q")) + return _comparable(captured["options"]) + + assert await probe_forwarded_keys(self._candidates(), call) == CLAUDE_AGENTS + + +class TestMalformedObjectValuesAreDropped: + def test_thinking_and_output_format_that_are_not_objects_are_dropped(self) -> None: + from launchdarkly_ai_claude_agents.handler import _options_parameters + + params = _options_parameters( + { + "model": { + "parameters": { + "thinking": "adaptive", + "output_format": "json", + "max_turns": 3, + } + } + } + ) + assert params == {"max_turns": 3} + + async def test_a_builtin_tool_result_is_not_marked_failed( monkeypatch: pytest.MonkeyPatch, ) -> None: diff --git a/packages/claude-agents/tests/test_native_graph.py b/packages/claude-agents/tests/test_native_graph.py index 81b14831..dd9e5cf7 100644 --- a/packages/claude-agents/tests/test_native_graph.py +++ b/packages/claude-agents/tests/test_native_graph.py @@ -15,6 +15,12 @@ import launchdarkly_ai_claude_agents.native_graph as _claude_ng from launchdarkly_ai_claude_agents.native_graph import to_claude_agents from launchdarkly_ai_server import GraphDefinition, GraphEdge, GraphNode, NativeTool +from tests.forwarding_spec import ( + CLAUDE_AGENTS, + candidate_keys, + probe_forwarded_keys, +) +from tests.never_forwarded import NEVER_FORWARDED_BAG, find_leaks # --------------------------------------------------------------------------- # Helpers @@ -1203,3 +1209,95 @@ async def test_stamps_conversation_id_on_graph_span(self) -> None: assert (graph_spans[0].attributes or {}).get( GEN_AI_CONVERSATION_ID ) == "thread-graph" + + +class TestNativeGraphModelParameters: + @pytest.mark.asyncio + async def test_node_options_take_run_settings_and_nothing_never_forwarded( + self, + ) -> None: + """Each node runs its own query with options built from its own config, so a node's + ``max_turns`` applies to that node.""" + mock_sdk = _make_sdk_mock("done") + nodes = { + "root": { + "key": "root", + "config": { + "model": { + "name": "claude-3", + "parameters": {**NEVER_FORWARDED_BAG, "max_turns": 7}, + }, + "instructions": "be helpful", + }, + "meta": {"variationKey": "v1", "version": 1}, + "edges": [], + "is_terminal": True, + } + } + graph_def = _make_graph_def(nodes=nodes) + + captured_options: list[dict[str, Any]] = [] + mock_sdk.ClaudeAgentOptions = MagicMock( + side_effect=lambda **kw: (captured_options.append(kw), kw)[1] + ) + + with patch( + "importlib.import_module", + side_effect=lambda n: ( + mock_sdk if n == "claude_agent_sdk" else __import__(n) + ), + ): + await to_claude_agents(_make_def_promise(graph_def)).invoke("hi") + + assert captured_options + for opts in captured_options: + assert opts["max_turns"] == 7 + assert not find_leaks(opts) + + +class TestNativeGraphForwardsExactlyTheCrossSdkList: + """The native graph builds each node's options with the handler's own forwarding, so a probe + of one node finds exactly the cross-SDK Claude Agents list.""" + + @pytest.mark.asyncio + async def test_root_node_options(self) -> None: + import dataclasses + + from claude_agent_sdk import ClaudeAgentOptions + + async def call(parameters: dict[str, Any]) -> object: + mock_sdk = _make_sdk_mock("done") + nodes = { + "root": { + "key": "root", + "config": { + "model": {"name": "claude-3", "parameters": parameters}, + "instructions": "be helpful", + }, + "meta": {"variationKey": "v1", "version": 1}, + "edges": [], + "is_terminal": True, + } + } + captured: list[dict[str, Any]] = [] + mock_sdk.ClaudeAgentOptions = MagicMock( + side_effect=lambda **kw: (captured.append(kw), kw)[1] + ) + with patch( + "importlib.import_module", + side_effect=lambda n: ( + mock_sdk if n == "claude_agent_sdk" else __import__(n) + ), + ): + await to_claude_agents( + _make_def_promise(_make_graph_def(nodes=nodes)) + ).invoke("hi") + return [ + {k: v for k, v in kw.items() if k not in {"hooks", "mcp_servers"}} + for kw in captured + ] + + candidates = candidate_keys( + (f.name for f in dataclasses.fields(ClaudeAgentOptions)), CLAUDE_AGENTS + ) + assert await probe_forwarded_keys(candidates, call) == CLAUDE_AGENTS diff --git a/packages/claude-agents/tests/test_parameter_forwarding.py b/packages/claude-agents/tests/test_parameter_forwarding.py new file mode 100644 index 00000000..336fde80 --- /dev/null +++ b/packages/claude-agents/tests/test_parameter_forwarding.py @@ -0,0 +1,133 @@ +""" +Drift test for the ``ClaudeAgentOptions`` parameter classification in ``handler.py``. + +``_CLAUDE_AGENT_OPTIONS_FORWARDED_KEYS`` is a literal, hand-maintained list. This test reads +``ClaudeAgentOptions``'s own dataclass fields and asserts every field it declares is classified in +exactly one of forwarded, handler-owned, or excluded, so an SDK field nobody has classified yet +fails loudly by name, and so does a list entry that is not a real SDK field. +""" + +from __future__ import annotations + +import dataclasses +from typing import Any + +from claude_agent_sdk import ClaudeAgentOptions + +from launchdarkly_ai_claude_agents.handler import ( + _CLAUDE_AGENT_OPTIONS_EXCLUDED_KEYS as _EXCLUDED_KEYS, +) +from launchdarkly_ai_claude_agents.handler import ( + _CLAUDE_AGENT_OPTIONS_FORWARDED_KEYS, + _build_query_options, +) +from tests.never_forwarded import NEVER_FORWARDED_BAG, find_leaks + +#: Handler-owned: popped from the filtered params before ``ClaudeAgentOptions(**kwargs)`` is +#: constructed, at every call site (``handler.py`` and ``native_graph.py``). +_OWNED_KEYS = frozenset( + {"model", "allowed_tools", "mcp_servers", "hooks", "tools", "system_prompt"} +) + + +class TestClaudeAgentOptionsAcceptsExactlyTheseFields: + def test_every_field_is_classified_exactly_once(self) -> None: + accepted = frozenset(f.name for f in dataclasses.fields(ClaudeAgentOptions)) + classified = _CLAUDE_AGENT_OPTIONS_FORWARDED_KEYS | _OWNED_KEYS | _EXCLUDED_KEYS + + unclassified = accepted - classified + assert not unclassified, ( + f"ClaudeAgentOptions now declares {sorted(unclassified)}, not classified as " + "forwarded, handler-owned, or excluded in claude-agents handler.py" + ) + + overlap = ( + (_CLAUDE_AGENT_OPTIONS_FORWARDED_KEYS & _OWNED_KEYS) + | (_CLAUDE_AGENT_OPTIONS_FORWARDED_KEYS & _EXCLUDED_KEYS) + | (_OWNED_KEYS & _EXCLUDED_KEYS) + ) + assert not overlap, f"fields classified more than once: {sorted(overlap)}" + + def test_every_classified_field_is_real(self) -> None: + accepted = frozenset(f.name for f in dataclasses.fields(ClaudeAgentOptions)) + stale = ( + _CLAUDE_AGENT_OPTIONS_FORWARDED_KEYS | _OWNED_KEYS | _EXCLUDED_KEYS + ) - accepted + assert not stale, ( + f"{sorted(stale)} classified in claude-agents handler.py but " + "ClaudeAgentOptions does not declare them" + ) + + +def _config(parameters: dict[str, Any]) -> dict[str, Any]: + return { + "model": {"name": "claude-opus-4-5", "parameters": parameters}, + "provider": {"name": "Anthropic"}, + "instructions": "You are helpful.", + } + + +class TestHostProcessSettingsAreNeverForwarded: + def test_cli_path_env_permission_mode_and_add_dirs_do_not_reach_the_options( + self, + ) -> None: + """A config that could set these could launch its own binary as the agent process, + point the customer's API key at another host, skip every permission prompt, and open the + whole filesystem. ``_build_query_options`` must leave all four at their defaults.""" + options = _build_query_options( + _config( + { + "cli_path": "/tmp/attacker-binary", + "env": {"ANTHROPIC_BASE_URL": "https://attacker.example"}, + "permission_mode": "bypassPermissions", + "add_dirs": ["/"], + } + ), + None, + [], + [], + None, + None, + ) + defaults = ClaudeAgentOptions() + assert options.cli_path is None + assert options.env == defaults.env == {} + assert options.permission_mode is None + assert options.add_dirs == defaults.add_dirs == [] + + def test_no_never_forwarded_key_reaches_the_options(self) -> None: + options = _build_query_options( + _config(dict(NEVER_FORWARDED_BAG)), None, [], [], None, None + ) + assert not find_leaks(options) + + def test_the_agreed_run_settings_still_land(self) -> None: + options = _build_query_options( + _config( + { + **NEVER_FORWARDED_BAG, + "max_turns": 4, + "max_thinking_tokens": 2048, + "thinking": {"type": "adaptive"}, + "effort": "high", + "max_budget_usd": 1.5, + "fallback_model": "claude-sonnet-4-5", + "output_format": {"type": "json_schema", "schema": {}}, + "betas": ["context-1m-2025-08-07"], + } + ), + None, + [], + [], + None, + None, + ) + assert options.max_turns == 4 + assert options.max_thinking_tokens == 2048 + assert options.thinking == {"type": "adaptive"} + assert options.effort == "high" + assert options.max_budget_usd == 1.5 + assert options.fallback_model == "claude-sonnet-4-5" + assert options.output_format == {"type": "json_schema", "schema": {}} + assert options.betas == ["context-1m-2025-08-07"] + assert not find_leaks(options) diff --git a/packages/claude-messages/src/launchdarkly_ai_claude_messages/handler.py b/packages/claude-messages/src/launchdarkly_ai_claude_messages/handler.py index ebf8181d..bd4d8c24 100644 --- a/packages/claude-messages/src/launchdarkly_ai_claude_messages/handler.py +++ b/packages/claude-messages/src/launchdarkly_ai_claude_messages/handler.py @@ -28,6 +28,8 @@ ) from launchdarkly_ai_server.agui import TurnEmitter from launchdarkly_ai_server.lifecycle import effective_capture_content +from launchdarkly_ai_server.parameter_forwarding import select_forwarded_parameters +from launchdarkly_ai_server.utils import model_parameters from ._version import PACKAGE_NAME, __version__ from .spans import ( @@ -47,12 +49,103 @@ to_tool_definitions, ) -try: - import anthropic as _anthropic_mod # noqa: F401 +#: Every key ``AsyncMessages.create``/``.stream`` accept, classified by hand into exactly one of: +#: forwarded (below), handler-owned (``model``, ``messages``, ``system``, ``tools``, set by each +#: call site itself), or excluded (below). ``TestMessagesCreateAcceptsExactlyTheseKeys`` / +#: ``TestMessagesStreamAcceptsExactlyTheseKeys`` in this package's tests assert this +#: classification stays exhaustive as the SDK's own signatures change. +#: +#: This is the cross-SDK list for Claude Messages (TESTING.md section 1.12). A top-level +#: ``effort`` is also accepted and moved into ``output_config.effort`` before this list applies +#: (see :func:`_rename_effort_to_output_config`), and ``max_tokens`` defaults to 1024. The same list +#: serves ``invoke`` and ``stream`` (both read :func:`_forwarded_parameters`), so one config behaves +#: the same whichever is called. +_MESSAGES_FORWARDED_KEYS = frozenset( + { + "cache_control", + "max_tokens", + "output_config", + "service_tier", + "stop_sequences", + "temperature", + "thinking", + "tool_choice", + "top_k", + "top_p", + } +) + +#: Forwarded keys whose value must be an object. A config value of any other type is malformed +#: and dropped rather than passed to the API. +_MESSAGES_MAPPING_KEYS = frozenset( + {"cache_control", "output_config", "thinking", "tool_choice"} +) + +#: Accepted by the API but never forwarded, and why: +#: * ``stream``: runtime wiring. The handler chooses blocking vs. streaming itself, not via a kwarg. +#: * ``output_format``: API-shape switch. Only ``.stream`` accepts it, so forwarding it would make +#: ``invoke`` and ``stream`` behave differently. ``output_config`` (forwarded) carries the same +#: output format on both. +#: * ``inference_geo``: data location. The region inference runs in decides where data is +#: processed. +#: * ``container``: server-side state. It selects container state carried over from another +#: request, not a model setting. +#: * ``metadata``, ``user_profile_id``: identity and attribution. They say who the request is for, +#: or attribute it to a party other than the caller. +#: * ``timeout``, ``extra_headers``, ``extra_query``, ``extra_body``: client/connection +#: configuration (a request timeout, raw HTTP overrides), never a config-controlled setting. +#: +#: Named for the drift test and for review, not read at runtime: the forwarded list above already +#: leaves these out, so nothing needs to subtract them again. +_MESSAGES_EXCLUDED_KEYS = frozenset( + { + "stream", + "output_format", + "inference_geo", + "container", + "metadata", + "user_profile_id", + "timeout", + "extra_headers", + "extra_query", + "extra_body", + } +) + - _HAS_ANTHROPIC = True -except ImportError: - _HAS_ANTHROPIC = False +def _rename_effort_to_output_config(params: dict[str, Any]) -> dict[str, Any]: + """Moves a top-level ``effort`` into ``output_config.effort``, the shape the Anthropic Messages + API actually accepts (there is no top-level ``effort`` parameter). + + An ``output_config`` the config itself already set wins: if it carries its own ``effort``, the + top-level one is dropped rather than overwriting it. This handler does not itself set + ``output_config`` (structured output goes through a system-prompt instruction instead, see + ``_build_messages``), so there is nothing here to merge with yet; a future call site that starts + setting ``output_config`` must merge so its own keys win and a config's ``effort`` survives. + """ + effort = params.pop("effort", None) + if effort is None: + return params + existing = params.get("output_config") + if isinstance(existing, dict) and "effort" in existing: + return params + params["output_config"] = { + **(existing if isinstance(existing, dict) else {}), + "effort": effort, + } + return params + + +def _forwarded_parameters(config: AiConfigRep) -> dict[str, Any]: + """The config's ``model.parameters`` that reach ``messages.create``/``.stream``: ``effort`` + moved into ``output_config``, then only :data:`_MESSAGES_FORWARDED_KEYS`, with malformed + object values dropped. ``invoke`` and ``stream`` both call this, so they forward the same keys. + """ + return select_forwarded_parameters( + _rename_effort_to_output_config(model_parameters(config)), + _MESSAGES_FORWARDED_KEYS, + mapping_keys=_MESSAGES_MAPPING_KEYS, + ) def _build_tools(config_tools: dict[str, Any]) -> list[dict[str, Any]]: @@ -206,9 +299,8 @@ async def _run_tool_loop( # difference predates this span work and changes what the model is offered, not what the span # reports, so it stays as it is: the catalog recorded below is the catalog actually sent. tools = _build_tools(config.get("tools") or {}) - max_tokens = (config.get("model", {}).get("parameters") or {}).get( - "max_tokens", 1024 - ) + extra_params = _forwarded_parameters(config) + max_tokens = extra_params.pop("max_tokens", 1024) conversation = list(messages) output = "" steps = 0 @@ -227,6 +319,7 @@ async def _run_tool_loop( open_model_span = model_span kwargs: dict[str, Any] = { + **extra_params, "model": config["model"]["name"], "max_tokens": max_tokens, "messages": conversation, @@ -547,9 +640,8 @@ async def _stream_gen( tools = _build_tools(config.get("tools") or {}) tool_definitions = to_tool_definitions(tools) - max_tokens = (config.get("model", {}).get("parameters") or {}).get( - "max_tokens", 1024 - ) + extra_params = _forwarded_parameters(config) + max_tokens = extra_params.pop("max_tokens", 1024) conversation = list(messages) full_output = "" steps = 0 @@ -583,6 +675,7 @@ async def _stream_gen( open_model_span = model_span kwargs: dict[str, Any] = { + **extra_params, "model": config["model"]["name"], "max_tokens": max_tokens, "messages": conversation, diff --git a/packages/claude-messages/tests/test_handler.py b/packages/claude-messages/tests/test_handler.py index bd2e3a90..05b07b1c 100644 --- a/packages/claude-messages/tests/test_handler.py +++ b/packages/claude-messages/tests/test_handler.py @@ -6,14 +6,27 @@ from __future__ import annotations +import json from collections.abc import AsyncGenerator, AsyncIterator from contextlib import asynccontextmanager from types import SimpleNamespace from typing import Any, ClassVar from unittest.mock import AsyncMock, MagicMock, patch +import httpx import pytest +from tests.forwarding_spec import ( + CLAUDE_MESSAGES, + candidate_keys, + probe_forwarded_keys, +) +from tests.never_forwarded import ( + NEVER_FORWARDED_BAG, + NEVER_FORWARDED_KEYS, + find_leaks, +) + # --------------------------------------------------------------------------- # Fake anthropic response helpers # --------------------------------------------------------------------------- @@ -364,6 +377,260 @@ async def test_custom_parameters_passthrough( # --------------------------------------------------------------------------- +class TestModelParametersForwarding: + async def test_snake_case_param_reaches_provider( + self, mock_anthropic: MagicMock + ) -> None: + from launchdarkly_ai_claude_messages import create_claude_messages_handler + + config = { + **CONFIG, + "model": {**CONFIG["model"], "parameters": {"top_p": 0.5}}, + } + h = create_claude_messages_handler() + await h(config, "q", {}, {}) + call_kwargs = mock_anthropic.messages.create.call_args.kwargs + assert call_kwargs["top_p"] == 0.5 + + async def test_max_tokens_default_preserved_without_parameters( + self, mock_anthropic: MagicMock + ) -> None: + from launchdarkly_ai_claude_messages import create_claude_messages_handler + + h = create_claude_messages_handler() + await h(CONFIG, "q", {}, {}) + call_kwargs = mock_anthropic.messages.create.call_args.kwargs + assert call_kwargs["max_tokens"] == 1024 + + async def test_config_max_tokens_wins_over_default( + self, mock_anthropic: MagicMock + ) -> None: + from launchdarkly_ai_claude_messages import create_claude_messages_handler + + config = { + **CONFIG, + "model": {**CONFIG["model"], "parameters": {"max_tokens": 42}}, + } + h = create_claude_messages_handler() + await h(config, "q", {}, {}) + call_kwargs = mock_anthropic.messages.create.call_args.kwargs + assert call_kwargs["max_tokens"] == 42 + + async def test_config_cannot_override_model_or_messages( + self, mock_anthropic: MagicMock + ) -> None: + from launchdarkly_ai_claude_messages import create_claude_messages_handler + + config = { + **CONFIG, + "model": { + **CONFIG["model"], + "parameters": { + "model": "not-the-real-model", + "messages": "not-the-real-messages", + "system": "not-the-real-system", + }, + }, + } + h = create_claude_messages_handler() + await h(config, "q", {}, {}) + call_kwargs = mock_anthropic.messages.create.call_args.kwargs + assert call_kwargs["model"] == CONFIG["model"]["name"] + assert call_kwargs["messages"] != "not-the-real-messages" + assert ( + "system" not in call_kwargs + or call_kwargs["system"] != "not-the-real-system" + ) + + async def test_call_unchanged_when_no_parameters_set( + self, mock_anthropic: MagicMock + ) -> None: + from launchdarkly_ai_claude_messages import create_claude_messages_handler + + h = create_claude_messages_handler() + await h(CONFIG, "q", {}, {}) + call_kwargs = mock_anthropic.messages.create.call_args.kwargs + assert set(call_kwargs.keys()) == {"model", "max_tokens", "messages", "system"} + + async def test_streaming_forwards_snake_case_param( + self, mock_anthropic: MagicMock + ) -> None: + import launchdarkly_ai_claude_messages.spans as spans_mod + from launchdarkly_ai_claude_messages import create_claude_messages_handler + + ctx, _ = _make_stream_context(["hi"]) + mock_anthropic.messages.stream = MagicMock(return_value=ctx) + config = { + **CONFIG, + "model": {**CONFIG["model"], "parameters": {"top_p": 0.3}}, + } + with patch.object(spans_mod, "_HAS_OTEL", False): + h = create_claude_messages_handler() + gen = await h.stream(config, "q", {}, {}) + async for _event in gen: + pass + call_kwargs = mock_anthropic.messages.stream.call_args.kwargs + assert call_kwargs["top_p"] == 0.3 + + async def test_top_p_and_max_tokens_reach_the_wire(self) -> None: + """Intercepts the real outgoing HTTP request with an httpx MockTransport, rather than + asserting only on a mock of our own call, so this proves the values actually leave the + process on the wire the real ``anthropic`` client builds. + """ + import anthropic + + from launchdarkly_ai_claude_messages import create_claude_messages_handler + + captured: dict[str, Any] = {} + + def _handler(request: httpx.Request) -> httpx.Response: + captured["body"] = json.loads(request.content) + return httpx.Response( + 200, + json={ + "id": "msg_1", + "type": "message", + "role": "assistant", + "model": "claude-3-sonnet-20240229", + "content": [{"type": "text", "text": "hi"}], + "stop_reason": "end_turn", + "usage": {"input_tokens": 3, "output_tokens": 2}, + }, + ) + + transport = httpx.MockTransport(_handler) + real_client = anthropic.AsyncAnthropic( + api_key="test-key", http_client=httpx.AsyncClient(transport=transport) + ) + + config = { + **CONFIG, + "model": { + **CONFIG["model"], + "parameters": {"top_p": 0.4, "max_tokens": 256}, + }, + } + with patch("anthropic.AsyncAnthropic", return_value=real_client): + h = create_claude_messages_handler() + await h(config, "q", {}, {}) + + assert captured["body"]["top_p"] == 0.4 + assert captured["body"]["max_tokens"] == 256 + + async def test_ui_key_the_sdk_rejects_is_dropped_without_raising( + self, mock_anthropic: MagicMock + ) -> None: + """``effort`` has no top-level equivalent on ``messages.create``; without the rename it + raises ``TypeError`` before this filter existed. Here it is set with no ``output_config``, + so the effort rename applies and the call must not raise. + """ + from launchdarkly_ai_claude_messages import create_claude_messages_handler + + config = { + **CONFIG, + "model": {**CONFIG["model"], "parameters": {"effort": "low"}}, + } + h = create_claude_messages_handler() + await h(config, "q", {}, {}) + call_kwargs = mock_anthropic.messages.create.call_args.kwargs + assert call_kwargs["output_config"] == {"effort": "low"} + + async def test_transport_key_is_never_forwarded( + self, mock_anthropic: MagicMock + ) -> None: + from launchdarkly_ai_claude_messages import create_claude_messages_handler + + config = { + **CONFIG, + "model": { + **CONFIG["model"], + "parameters": {"top_p": 0.5, "extra_body": {"secret": "value"}}, + }, + } + h = create_claude_messages_handler() + await h(config, "q", {}, {}) + call_kwargs = mock_anthropic.messages.create.call_args.kwargs + assert call_kwargs["top_p"] == 0.5 + assert "extra_body" not in call_kwargs + + async def test_stream_key_is_never_forwarded( + self, mock_anthropic: MagicMock + ) -> None: + """The handler picks blocking vs. streaming by which client method it calls, not by a + ``stream`` kwarg; a config setting it must not reach ``messages.create``.""" + from launchdarkly_ai_claude_messages import create_claude_messages_handler + + config = { + **CONFIG, + "model": {**CONFIG["model"], "parameters": {"stream": True}}, + } + h = create_claude_messages_handler() + await h(config, "q", {}, {}) + call_kwargs = mock_anthropic.messages.create.call_args.kwargs + assert "stream" not in call_kwargs + + +class TestEffortRename: + async def test_explicit_output_config_effort_wins_over_top_level_effort( + self, mock_anthropic: MagicMock + ) -> None: + from launchdarkly_ai_claude_messages import create_claude_messages_handler + + config = { + **CONFIG, + "model": { + **CONFIG["model"], + "parameters": { + "effort": "low", + "output_config": {"effort": "high"}, + }, + }, + } + h = create_claude_messages_handler() + await h(config, "q", {}, {}) + call_kwargs = mock_anthropic.messages.create.call_args.kwargs + assert call_kwargs["output_config"] == {"effort": "high"} + + async def test_top_level_effort_moves_into_output_config_when_absent( + self, mock_anthropic: MagicMock + ) -> None: + from launchdarkly_ai_claude_messages import create_claude_messages_handler + + config = { + **CONFIG, + "model": {**CONFIG["model"], "parameters": {"effort": "medium"}}, + } + h = create_claude_messages_handler() + await h(config, "q", {}, {}) + call_kwargs = mock_anthropic.messages.create.call_args.kwargs + assert call_kwargs["output_config"] == {"effort": "medium"} + + async def test_top_level_effort_merges_into_an_output_config_without_effort( + self, mock_anthropic: MagicMock + ) -> None: + """An ``output_config`` present without its own ``effort`` keeps its other keys and gains + the top-level ``effort``.""" + from launchdarkly_ai_claude_messages import create_claude_messages_handler + + config = { + **CONFIG, + "model": { + **CONFIG["model"], + "parameters": { + "effort": "low", + "output_config": {"some_other_key": "kept"}, + }, + }, + } + h = create_claude_messages_handler() + await h(config, "q", {}, {}) + call_kwargs = mock_anthropic.messages.create.call_args.kwargs + assert call_kwargs["output_config"] == { + "some_other_key": "kept", + "effort": "low", + } + + class TestToolExecutionLoop: async def test_single_tool_call_then_done(self, mock_anthropic: MagicMock) -> None: from launchdarkly_ai_claude_messages import create_claude_messages_handler @@ -2481,3 +2748,190 @@ def _explode_on_the_chat_span(span: Any, capture: bool, **kw: Any) -> None: assert chat.ended == 1 assert StatusCode.ERROR in chat.statuses assert "launchdarkly.stream.abandoned" not in chat.attributes + + +#: Never-forwarded keys the handler sets itself from the config's instructions. ``find_leaks`` +#: still proves the config's own value for them never arrives. +_HANDLER_SETS = frozenset({"system"}) + + +class TestNeverForwardedParameters: + """No key in the never-forwarded bag reaches ``messages.create`` or ``messages.stream`` from + ``model.parameters``.""" + + async def test_invoke_forwards_none_of_them( + self, mock_anthropic: MagicMock + ) -> None: + from launchdarkly_ai_claude_messages import create_claude_messages_handler + + config = { + **CONFIG, + "model": {**CONFIG["model"], "parameters": dict(NEVER_FORWARDED_BAG)}, + } + await create_claude_messages_handler()(config, "q", {}, {}) + call_kwargs = mock_anthropic.messages.create.call_args.kwargs + assert not find_leaks(call_kwargs) + assert not set(call_kwargs) & (NEVER_FORWARDED_KEYS - _HANDLER_SETS) + + async def test_stream_forwards_none_of_them( + self, mock_anthropic: MagicMock + ) -> None: + import launchdarkly_ai_claude_messages.spans as spans_mod + from launchdarkly_ai_claude_messages import create_claude_messages_handler + + ctx, _ = _make_stream_context(["hi"]) + mock_anthropic.messages.stream = MagicMock(return_value=ctx) + config = { + **CONFIG, + "model": {**CONFIG["model"], "parameters": dict(NEVER_FORWARDED_BAG)}, + } + with patch.object(spans_mod, "_HAS_OTEL", False): + h = create_claude_messages_handler() + async for _event in await h.stream(config, "q", {}, {}): + pass + call_kwargs = mock_anthropic.messages.stream.call_args.kwargs + assert not find_leaks(call_kwargs) + assert not set(call_kwargs) & (NEVER_FORWARDED_KEYS - _HANDLER_SETS) + + +class TestInvokeAndStreamForwardTheSameKeys: + """``output_format`` is accepted by ``messages.stream`` only. Forwarding it there and not on + ``create`` made one config behave differently by call, so it is dropped on both, and + ``output_config`` (accepted by both) carries the output format instead.""" + + _PARAMS: ClassVar[dict[str, Any]] = { + "output_format": {"type": "json_schema", "schema": {}}, + "output_config": {"format": {"type": "json_schema", "schema": {}}}, + "top_p": 0.4, + } + + async def test_invoke(self, mock_anthropic: MagicMock) -> None: + from launchdarkly_ai_claude_messages import create_claude_messages_handler + + config = {**CONFIG, "model": {**CONFIG["model"], "parameters": self._PARAMS}} + await create_claude_messages_handler()(config, "q", {}, {}) + call_kwargs = mock_anthropic.messages.create.call_args.kwargs + assert "output_format" not in call_kwargs + assert call_kwargs["output_config"] == self._PARAMS["output_config"] + assert call_kwargs["top_p"] == 0.4 + + async def test_stream(self, mock_anthropic: MagicMock) -> None: + import launchdarkly_ai_claude_messages.spans as spans_mod + from launchdarkly_ai_claude_messages import create_claude_messages_handler + + ctx, _ = _make_stream_context(["hi"]) + mock_anthropic.messages.stream = MagicMock(return_value=ctx) + config = {**CONFIG, "model": {**CONFIG["model"], "parameters": self._PARAMS}} + with patch.object(spans_mod, "_HAS_OTEL", False): + h = create_claude_messages_handler() + async for _event in await h.stream(config, "q", {}, {}): + pass + call_kwargs = mock_anthropic.messages.stream.call_args.kwargs + assert "output_format" not in call_kwargs + assert call_kwargs["output_config"] == self._PARAMS["output_config"] + assert call_kwargs["top_p"] == 0.4 + + +class TestForwardsExactlyTheCrossSdkList: + """Probes ``invoke`` and ``stream`` one key at a time: the keys that change the provider call + are exactly the cross-SDK Claude Messages list, on both paths.""" + + @staticmethod + def _candidates() -> frozenset[str]: + import inspect + + from anthropic.resources.messages import AsyncMessages + + accepted = { + name + for fn in (AsyncMessages.create, AsyncMessages.stream) + for name in inspect.signature(fn).parameters + if name != "self" + } + return candidate_keys(accepted, CLAUDE_MESSAGES) + + def _config(self, parameters: dict[str, Any]) -> dict[str, Any]: + return {**CONFIG, "model": {**CONFIG["model"], "parameters": parameters}} + + async def test_invoke(self, mock_anthropic: MagicMock) -> None: + from launchdarkly_ai_claude_messages import create_claude_messages_handler + + h = create_claude_messages_handler() + + async def call(parameters: dict[str, Any]) -> object: + await h(self._config(parameters), "q", {}, {}) + return mock_anthropic.messages.create.call_args.kwargs + + assert await probe_forwarded_keys(self._candidates(), call) == CLAUDE_MESSAGES + + async def test_stream(self, mock_anthropic: MagicMock) -> None: + import launchdarkly_ai_claude_messages.spans as spans_mod + from launchdarkly_ai_claude_messages import create_claude_messages_handler + + mock_anthropic.messages.stream = MagicMock( + side_effect=lambda **_kw: _make_stream_context(["hi"])[0] + ) + with patch.object(spans_mod, "_HAS_OTEL", False): + h = create_claude_messages_handler() + + async def call(parameters: dict[str, Any]) -> object: + async for _event in await h.stream(self._config(parameters), "q", {}, {}): + pass + return mock_anthropic.messages.stream.call_args.kwargs + + assert await probe_forwarded_keys(self._candidates(), call) == CLAUDE_MESSAGES + + +class TestMalformedObjectValuesAreDropped: + """A key the API expects as an object, set to anything else, is dropped on both paths.""" + + _PARAMS: ClassVar[dict[str, Any]] = { + "thinking": "enabled", + "cache_control": "ephemeral", + "output_config": "json", + "tool_choice": "auto", + "top_p": 0.4, + } + + def _config(self) -> dict[str, Any]: + return {**CONFIG, "model": {**CONFIG["model"], "parameters": self._PARAMS}} + + def _assert_dropped(self, kwargs: dict[str, Any]) -> None: + assert not {"thinking", "cache_control", "output_config", "tool_choice"} & set( + kwargs + ) + assert kwargs["top_p"] == 0.4 + + async def test_invoke(self, mock_anthropic: MagicMock) -> None: + from launchdarkly_ai_claude_messages import create_claude_messages_handler + + await create_claude_messages_handler()(self._config(), "q", {}, {}) + self._assert_dropped(mock_anthropic.messages.create.call_args.kwargs) + + async def test_stream(self, mock_anthropic: MagicMock) -> None: + import launchdarkly_ai_claude_messages.spans as spans_mod + from launchdarkly_ai_claude_messages import create_claude_messages_handler + + ctx, _ = _make_stream_context(["hi"]) + mock_anthropic.messages.stream = MagicMock(return_value=ctx) + with patch.object(spans_mod, "_HAS_OTEL", False): + h = create_claude_messages_handler() + async for _event in await h.stream(self._config(), "q", {}, {}): + pass + self._assert_dropped(mock_anthropic.messages.stream.call_args.kwargs) + + async def test_effort_still_lands_in_output_config_when_output_config_is_malformed( + self, mock_anthropic: MagicMock + ) -> None: + from launchdarkly_ai_claude_messages import create_claude_messages_handler + + config = { + **CONFIG, + "model": { + **CONFIG["model"], + "parameters": {"effort": "low", "output_config": "json"}, + }, + } + await create_claude_messages_handler()(config, "q", {}, {}) + kwargs = mock_anthropic.messages.create.call_args.kwargs + assert kwargs["output_config"] == {"effort": "low"} diff --git a/packages/claude-messages/tests/test_parameter_forwarding.py b/packages/claude-messages/tests/test_parameter_forwarding.py new file mode 100644 index 00000000..5c6a33b8 --- /dev/null +++ b/packages/claude-messages/tests/test_parameter_forwarding.py @@ -0,0 +1,92 @@ +""" +Drift test for the Anthropic Messages API parameter classification in ``handler.py``. + +``_MESSAGES_FORWARDED_KEYS`` is a literal, hand-maintained list, and the same list serves ``invoke`` and ``stream``. +This test reads ``AsyncMessages.create``/``.stream``'s own signatures and asserts every parameter either +accepts is classified in exactly one of forwarded, handler-owned, or excluded, so an SDK parameter +nobody has classified yet fails loudly by name. It also asserts every forwarded or handler-owned key +is accepted by both calls, so ``invoke`` and ``stream`` cannot drift apart again, and that every +excluded key is a real parameter of at least one of them. +""" + +from __future__ import annotations + +import inspect +from collections.abc import Callable + +from anthropic.resources.messages import AsyncMessages + +from launchdarkly_ai_claude_messages.handler import ( + _MESSAGES_EXCLUDED_KEYS, + _MESSAGES_FORWARDED_KEYS, +) + +#: Handler-owned: set by each call site itself, never taken from the config. +_OWNED_KEYS = frozenset({"model", "messages", "system", "tools"}) + + +def _signature_keys(fn: Callable[..., object]) -> frozenset[str]: + keys: set[str] = set() + for name, param in inspect.signature(fn).parameters.items(): + if name == "self" or param.kind is inspect.Parameter.VAR_POSITIONAL: + continue + assert param.kind is not inspect.Parameter.VAR_KEYWORD, ( + f"{fn!r} now accepts **{name}; this test can no longer enumerate its accept-set" + ) + keys.add(name) + return frozenset(keys) + + +_CREATE_KEYS = _signature_keys(AsyncMessages.create) +_STREAM_KEYS = _signature_keys(AsyncMessages.stream) + + +class TestMessagesCreateAcceptsExactlyTheseKeys: + def test_every_accepted_key_is_classified_exactly_once(self) -> None: + classified = _MESSAGES_FORWARDED_KEYS | _OWNED_KEYS | _MESSAGES_EXCLUDED_KEYS + + unclassified = _CREATE_KEYS - classified + assert not unclassified, ( + f"AsyncMessages.create now accepts {sorted(unclassified)}, not classified as " + "forwarded, handler-owned, or excluded in claude-messages handler.py" + ) + + overlap = ( + (_MESSAGES_FORWARDED_KEYS & _OWNED_KEYS) + | (_MESSAGES_FORWARDED_KEYS & _MESSAGES_EXCLUDED_KEYS) + | (_OWNED_KEYS & _MESSAGES_EXCLUDED_KEYS) + ) + assert not overlap, f"keys classified more than once: {sorted(overlap)}" + + def test_every_forwarded_or_owned_key_is_a_real_parameter(self) -> None: + stale = (_MESSAGES_FORWARDED_KEYS | _OWNED_KEYS) - _CREATE_KEYS + assert not stale, ( + f"{sorted(stale)} forwarded or handler-owned in claude-messages handler.py but " + "AsyncMessages.create does not accept them" + ) + + +class TestMessagesStreamAcceptsExactlyTheseKeys: + def test_every_accepted_key_is_classified_exactly_once(self) -> None: + classified = _MESSAGES_FORWARDED_KEYS | _OWNED_KEYS | _MESSAGES_EXCLUDED_KEYS + + unclassified = _STREAM_KEYS - classified + assert not unclassified, ( + f"AsyncMessages.stream now accepts {sorted(unclassified)}, not classified as " + "forwarded, handler-owned, or excluded in claude-messages handler.py" + ) + + def test_every_forwarded_or_owned_key_is_a_real_parameter(self) -> None: + stale = (_MESSAGES_FORWARDED_KEYS | _OWNED_KEYS) - _STREAM_KEYS + assert not stale, ( + f"{sorted(stale)} forwarded or handler-owned in claude-messages handler.py but " + "AsyncMessages.stream does not accept them" + ) + + +def test_every_excluded_key_is_a_real_parameter() -> None: + stale = _MESSAGES_EXCLUDED_KEYS - (_CREATE_KEYS | _STREAM_KEYS) + assert not stale, ( + f"{sorted(stale)} excluded in claude-messages handler.py but neither " + "AsyncMessages.create nor .stream accepts them" + ) diff --git a/packages/client/src/launchdarkly_ai_server/parameter_forwarding.py b/packages/client/src/launchdarkly_ai_server/parameter_forwarding.py new file mode 100644 index 00000000..69622078 --- /dev/null +++ b/packages/client/src/launchdarkly_ai_server/parameter_forwarding.py @@ -0,0 +1,51 @@ +""" +Shared filtering for ``model.parameters`` before it reaches a provider SDK call. + +``model.parameters`` is a free-form dict the LaunchDarkly UI writes, offering keys that make +sense across providers (``temperature``, ``top_p``, ``max_tokens``, ``tool_choice``, ...). No +single provider SDK accepts all of them, and each handler package classifies every key its own +provider entry point accepts into exactly one written-down list: forwarded (model and run settings +only, the cross-SDK list for that handler in TESTING.md section 1.12), handler-owned (set by the +call site itself), or excluded (accepted by the provider but never forwarded: credentials, endpoints +and connection settings, raw request injection such as extra headers or ``model_kwargs``, remote +tools, host-process settings, data location and retention, server-side state, identity and +attribution, runtime wiring and API-shape switches, prompt content beyond the instructions, and +safety configuration). Only the forwarded list is read at runtime; every other key is dropped. Those lists are +literal, next to each handler's own call sites, not derived from the provider SDK at runtime: a +hand-maintained list is reviewable and a runtime-derived one is not, and each handler package has a +drift test asserting its lists still cover everything its provider SDK accepts. + +This module is the one shared piece: given a params dict and the literal forwarded-keys list, keep +only the keys on that list, and drop any whose value should be an object but is not. +""" + +from __future__ import annotations + +from collections.abc import Mapping +from typing import Any + + +def select_forwarded_parameters( + params: Mapping[str, Any], + forwarded_keys: frozenset[str], + *, + mapping_keys: frozenset[str] = frozenset(), +) -> dict[str, Any]: + """Returns the subset of *params* whose key is in *forwarded_keys*. + + *forwarded_keys* is a handler's literal, hand-maintained list of the keys it forwards to its + provider call: never the provider's full accept-set, since a handler's own owned and excluded + keys (including client/connection configuration) are already left off that list. A key present + in *params* but not in *forwarded_keys* is silently dropped rather than raising, matching this + SDK's behaviour before ``model.parameters`` forwarding existed: an unrecognised tuning value is + ignored, not a hard failure. + + *mapping_keys* names the forwarded keys whose value the provider expects to be an object + (``thinking``, ``reasoning``, ...). A value for one of them that is not a mapping is malformed + and dropped the same way, rather than passed through for the provider to reject. + """ + return { + k: v + for k, v in params.items() + if k in forwarded_keys and (k not in mapping_keys or isinstance(v, Mapping)) + } diff --git a/packages/client/src/launchdarkly_ai_server/utils.py b/packages/client/src/launchdarkly_ai_server/utils.py index 51807551..c328af49 100644 --- a/packages/client/src/launchdarkly_ai_server/utils.py +++ b/packages/client/src/launchdarkly_ai_server/utils.py @@ -52,6 +52,26 @@ def _create_handler( ) +def model_parameters(config: AiConfigRep) -> dict[str, Any]: + """ + Returns ``config['model']['parameters']`` as a fresh dict, or ``{}`` when it + is absent or not a usable mapping. + + This is the single place handler packages read provider tuning values from. + Values are forwarded to the provider as-is, keyed by whatever name the + LaunchDarkly UI wrote (already snake_case for every Python provider SDK + here), so no case conversion happens on the way through. Callers must + filter the result to their own forwarded-keys list (see + ``parameter_forwarding.select_forwarded_parameters``) before merging it + into a provider call; nothing else here is safe to forward as-is. + + Never reads ``model.custom`` — that field is not forwarded to providers. + """ + model = config.get("model") if isinstance(config, dict) else None + parameters = model.get("parameters") if isinstance(model, dict) else None + return dict(parameters) if isinstance(parameters, dict) else {} + + def collapse_messages_to_instructions(config: AiConfigRep) -> AiConfigRep: """ When only an agent handler is available for a messages-mode config, collapse diff --git a/packages/client/tests/test_parameter_forwarding.py b/packages/client/tests/test_parameter_forwarding.py new file mode 100644 index 00000000..938cd335 --- /dev/null +++ b/packages/client/tests/test_parameter_forwarding.py @@ -0,0 +1,38 @@ +"""Tests for the shared model.parameters filter: select_forwarded_parameters.""" + +from __future__ import annotations + +from launchdarkly_ai_server.parameter_forwarding import select_forwarded_parameters + + +class TestSelectForwardedParameters: + def test_keeps_only_forwarded_keys(self) -> None: + params = {"temperature": 0.5, "unknown": 1} + result = select_forwarded_parameters(params, frozenset({"temperature"})) + assert result == {"temperature": 0.5} + + def test_drops_a_key_the_handler_never_classified_as_forwarded(self) -> None: + params = {"temperature": 0.5, "api_key": "secret"} + result = select_forwarded_parameters(params, frozenset({"temperature"})) + assert result == {"temperature": 0.5} + + def test_empty_params_returns_empty(self) -> None: + assert select_forwarded_parameters({}, frozenset({"temperature"})) == {} + + def test_empty_forwarded_keys_returns_empty(self) -> None: + params = {"temperature": 0.5} + assert select_forwarded_parameters(params, frozenset()) == {} + + def test_does_not_mutate_input(self) -> None: + params = {"temperature": 0.5, "unknown": 1} + select_forwarded_parameters(params, frozenset({"temperature"})) + assert params == {"temperature": 0.5, "unknown": 1} + + def test_drops_a_mapping_key_whose_value_is_not_a_mapping(self) -> None: + params = {"thinking": "enabled", "reasoning": {"effort": "low"}, "top_p": 0.5} + result = select_forwarded_parameters( + params, + frozenset({"thinking", "reasoning", "top_p"}), + mapping_keys=frozenset({"thinking", "reasoning"}), + ) + assert result == {"reasoning": {"effort": "low"}, "top_p": 0.5} diff --git a/packages/client/tests/test_utils.py b/packages/client/tests/test_utils.py index ace49b27..ca8b38fd 100644 --- a/packages/client/tests/test_utils.py +++ b/packages/client/tests/test_utils.py @@ -18,7 +18,7 @@ parse_template, parse_usage, ) -from launchdarkly_ai_server.utils import make_graph_track_data +from launchdarkly_ai_server.utils import make_graph_track_data, model_parameters # --------------------------------------------------------------------------- # ?3.1 parse_template @@ -390,3 +390,48 @@ def test_does_not_mutate_input(self) -> None: td = {"runId": "r", "modelKey": "m"} omit_model_stamps(td) assert td == {"runId": "r", "modelKey": "m"} + + +class TestModelParameters: + def test_returns_the_parameters_dict(self) -> None: + config = {"model": {"name": "gpt-4o", "parameters": {"temperature": 0.2}}} + assert model_parameters(config) == {"temperature": 0.2} + + def test_returns_a_fresh_dict_not_the_original(self) -> None: + original = {"temperature": 0.2} + config = {"model": {"name": "gpt-4o", "parameters": original}} + result = model_parameters(config) + result["temperature"] = 99 + assert original["temperature"] == 0.2 + + @pytest.mark.parametrize( + "model", + [None, {}, {"name": "gpt-4o"}, {"name": "gpt-4o", "parameters": None}], + ) + def test_returns_empty_dict_when_parameters_absent(self, model: Any) -> None: + config: dict[str, Any] = {} + if model is not None: + config["model"] = model + assert model_parameters(config) == {} + + @pytest.mark.parametrize("parameters", ["not-a-dict", 1, ["a", "b"], True]) + def test_returns_empty_dict_when_parameters_not_a_mapping( + self, parameters: Any + ) -> None: + config = {"model": {"name": "gpt-4o", "parameters": parameters}} + assert model_parameters(config) == {} + + def test_ignores_non_dict_config(self) -> None: + assert model_parameters("not-a-config") == {} # type: ignore[arg-type] + + def test_never_reads_custom(self) -> None: + config = { + "model": { + "name": "gpt-4o", + "parameters": {"temperature": 0.2}, + "custom": {"secret": "value"}, + } + } + result = model_parameters(config) + assert "secret" not in result + assert "custom" not in result diff --git a/packages/langchain-agents/src/launchdarkly_ai_langchain_agents/handler.py b/packages/langchain-agents/src/launchdarkly_ai_langchain_agents/handler.py index 2eed441f..4de1b158 100644 --- a/packages/langchain-agents/src/launchdarkly_ai_langchain_agents/handler.py +++ b/packages/langchain-agents/src/launchdarkly_ai_langchain_agents/handler.py @@ -8,7 +8,7 @@ import asyncio import inspect import json -from collections.abc import AsyncGenerator +from collections.abc import AsyncGenerator, Mapping from typing import Any from launchdarkly_ai_server import ( @@ -34,6 +34,8 @@ ) from launchdarkly_ai_server.agui import TurnEmitter from launchdarkly_ai_server.lifecycle import effective_capture_content +from launchdarkly_ai_server.parameter_forwarding import select_forwarded_parameters +from launchdarkly_ai_server.utils import model_parameters from ._version import PACKAGE_NAME, __version__ from .messages import to_lang_chain_messages @@ -48,6 +50,287 @@ to_tool_definitions, ) +#: Handler-owned, per model class. The handler always sets the model itself, and each +#: class exposes that one constructor field under two names (the field and its alias), so a +#: config value for either must never be forwarded: it would collide with the model the +#: handler resolves, or override it. +_CHAT_OPENAI_OWNED_KEYS = frozenset({"model", "model_name"}) +_CHAT_ANTHROPIC_OWNED_KEYS = frozenset({"model", "model_name"}) +_CHAT_BEDROCK_CONVERSE_OWNED_KEYS = frozenset({"model", "model_id"}) + +#: LangChain's own runtime fields, shared by every chat model class: caching, callbacks, rate +#: limiting, tracing tags and metadata, streaming mode, message format, and token counting. They +#: configure how LangChain runs the model in this process, not the model request, and several are +#: objects or callables a config cannot express. Never forwarded. +_LANGCHAIN_RUNTIME_KEYS = frozenset( + { + "cache", + "callbacks", + "custom_get_token_ids", + "disable_streaming", + "metadata", + "name", + "output_version", + "profile", + "rate_limiter", + "streaming", + "tags", + "verbose", + } +) + +#: Every key ``ChatOpenAI`` accepts (field names plus pydantic aliases), classified by hand into +#: exactly one of: forwarded (below), handler-owned (``model``, always overwritten by the +#: resolved model name, see ``_model_constructor_kwargs``), or excluded (below). +#: ``TestChatOpenAIAcceptsExactlyTheseKeys`` in this package's tests asserts this classification +#: stays exhaustive as the SDK's own pydantic model changes. +#: +#: This is the cross-SDK list for LangChain ChatOpenAI (TESTING.md section 1.12). +#: ``reasoning_effort`` and ``seed`` are marked not supported in TypeScript there: the JS +#: ``ChatOpenAI`` takes them only as per-call options, while this handler sets them on the +#: constructor, which has a field for each. ``reasoning_effort`` is the Chat Completions field and +#: does not switch ``ChatOpenAI`` to the Responses API the way ``reasoning`` does. +_CHAT_OPENAI_FORWARDED_KEYS = frozenset( + { + "frequency_penalty", + "logit_bias", + "logprobs", + "max_completion_tokens", + "max_tokens", + "n", + "presence_penalty", + "reasoning_effort", + "seed", + "service_tier", + "stop", + "stop_sequences", + "temperature", + "top_logprobs", + "top_p", + "verbosity", + } +) + +#: Forwarded keys whose value must be an object. A config value of any other type is malformed +#: and dropped rather than passed to ``ChatOpenAI``. +_CHAT_OPENAI_MAPPING_KEYS = frozenset({"logit_bias"}) + +#: Accepted by ``ChatOpenAI`` but never forwarded, and why: +#: * ``api_key``, ``openai_api_key``, ``organization``, ``openai_organization``: credentials. +#: * ``base_url``, ``openai_api_base``, ``openai_proxy``: where requests go. +#: * ``client``, ``async_client``, ``root_client``, ``root_async_client``, ``http_client``, +#: ``http_async_client``, ``http_socket_options``: raw HTTP client objects/settings. +#: * ``default_headers``, ``default_query``, ``extra_body``, ``model_kwargs``: raw request +#: injection. ``model_kwargs`` is merged straight into the request payload, so it would carry +#: ``extra_headers``/``extra_query`` or any other excluded key past this list. +#: * ``max_retries``, ``request_timeout``, ``timeout``, ``stream_chunk_timeout``: retries and +#: timeouts. +#: * ``use_responses_api``, ``use_previous_response_id``, ``include``, ``reasoning``, +#: ``truncation``, ``context_management``: API-shape switch. Each one makes ``ChatOpenAI`` call +#: the Responses API instead of Chat Completions, which changes the API and response shape the +#: handler gets back. ``truncation``, ``context_management`` and ``use_previous_response_id`` +#: also lean on server-side conversation state. +#: * ``store``: data retention. It decides whether the request is kept on the server. +#: * ``prompt_cache_key``: on the spec list, marked not supported in Python, because +#: ``ChatOpenAI`` has no such constructor field (it takes it only per call). It is not in the set +#: below because ``ChatOpenAI`` does not accept it at all, which the drift test requires of every +#: listed key. +#: * ``stream_usage``, ``include_response_headers``, ``disabled_params``, ``tiktoken_model_name``: +#: runtime wiring. They change what the handler gets back and how usage is reported to it. +#: * Everything in :data:`_LANGCHAIN_RUNTIME_KEYS`. +#: +#: Named for the drift test and for review, not read at runtime: the forwarded list above already +#: leaves these out, so nothing needs to subtract them again. +_CHAT_OPENAI_EXCLUDED_KEYS = _LANGCHAIN_RUNTIME_KEYS | { + "api_key", + "openai_api_key", + "organization", + "openai_organization", + "base_url", + "openai_api_base", + "openai_proxy", + "client", + "async_client", + "root_client", + "root_async_client", + "http_client", + "http_async_client", + "http_socket_options", + "default_headers", + "default_query", + "extra_body", + "model_kwargs", + "max_retries", + "request_timeout", + "timeout", + "stream_chunk_timeout", + "stream_usage", + "include_response_headers", + "disabled_params", + "tiktoken_model_name", + "use_responses_api", + "use_previous_response_id", + "include", + "reasoning", + "truncation", + "context_management", + "store", +} + +#: Every key ``ChatAnthropic`` accepts (field names plus pydantic aliases), classified the same way +#: as :data:`_CHAT_OPENAI_FORWARDED_KEYS`. ``TestChatAnthropicAcceptsExactlyTheseKeys`` in this +#: package's tests asserts this classification stays exhaustive. +#: +#: This is the cross-SDK list for LangChain ChatAnthropic (TESTING.md section 1.12). +#: ``max_tokens_to_sample`` is renamed to ``max_tokens`` before this list applies, and a +#: ``max_tokens`` the config also set wins (see :data:`_CHAT_ANTHROPIC_RENAMES`). +_CHAT_ANTHROPIC_FORWARDED_KEYS = frozenset( + { + "betas", + "effort", + "max_tokens", + "max_tokens_to_sample", + "output_config", + "stop", + "stop_sequences", + "temperature", + "thinking", + "top_k", + "top_p", + } +) + +#: Forwarded keys whose value must be an object. A config value of any other type is malformed +#: and dropped rather than passed to ``ChatAnthropic``. +_CHAT_ANTHROPIC_MAPPING_KEYS = frozenset({"output_config", "thinking"}) + +#: Alias to the name it is renamed to before forwarding. ``ChatAnthropic`` takes both spellings +#: of the one field, so passing both would leave which one wins to pydantic. +_CHAT_ANTHROPIC_RENAMES = {"max_tokens_to_sample": "max_tokens"} + +#: Accepted by ``ChatAnthropic`` but never forwarded, and why: +#: * ``anthropic_api_key``, ``api_key``: credentials (field plus alias). +#: * ``anthropic_api_url``, ``base_url``, ``anthropic_proxy``: where requests go. +#: * ``inference_geo``: the region inference runs in, which decides where data is processed. +#: * ``context_management``: server-side state. It has the server clear or compact earlier +#: context. +#: * ``default_headers``, ``model_kwargs``: raw request injection. ``model_kwargs`` is merged +#: straight into the request, so it would carry any excluded key past this list. +#: * ``mcp_servers``: attaches remote MCP servers, which then receive the conversation. +#: * ``reuse_last_container``: reuses server-side container state from an earlier request, not +#: a model setting. +#: * ``default_request_timeout``, ``timeout``, ``max_retries``: timeouts and retries. +#: * ``stream_usage``: how usage is reported back to the handler. +#: * Everything in :data:`_LANGCHAIN_RUNTIME_KEYS`. +#: +#: Named for the drift test and for review, not read at runtime. +_CHAT_ANTHROPIC_EXCLUDED_KEYS = _LANGCHAIN_RUNTIME_KEYS | { + "anthropic_api_key", + "api_key", + "anthropic_api_url", + "base_url", + "anthropic_proxy", + "inference_geo", + "context_management", + "default_headers", + "model_kwargs", + "mcp_servers", + "reuse_last_container", + "default_request_timeout", + "timeout", + "max_retries", + "stream_usage", +} + +#: Every key ``ChatBedrockConverse`` accepts (field names plus pydantic aliases), classified the +#: same way as :data:`_CHAT_OPENAI_FORWARDED_KEYS`. ``langchain-aws`` is not a dependency of this +#: package (Bedrock support is opt-in, see ``_make_default_chat_model``), so +#: ``TestChatBedrockConverseAcceptsExactlyTheseKeys`` in this package's tests skips itself when it +#: is not installed rather than asserting nothing. +#: +#: This is the cross-SDK list for LangChain ChatBedrockConverse (TESTING.md section 1.12). +#: ``reasoning_effort``, ``stop`` and ``stop_sequences`` are marked not supported in TypeScript +#: there: the JS ``ChatBedrockConverse`` takes stop sequences only as a per-call option and has no +#: reasoning effort field, while this class has constructor fields for both. ``stop`` is renamed to +#: ``stop_sequences`` before this list applies, and a ``stop_sequences`` the config also set wins +#: (see :data:`_CHAT_BEDROCK_CONVERSE_RENAMES`). +_CHAT_BEDROCK_CONVERSE_FORWARDED_KEYS = frozenset( + { + "max_tokens", + "performance_config", + "reasoning_effort", + "service_tier", + "stop", + "stop_sequences", + "temperature", + "top_p", + } +) + +#: Forwarded keys whose value must be an object. A config value of any other type is malformed +#: and dropped rather than passed to ``ChatBedrockConverse``. +_CHAT_BEDROCK_CONVERSE_MAPPING_KEYS = frozenset({"performance_config"}) + +#: Alias to the name it is renamed to before forwarding. ``stop`` is the pydantic alias of +#: ``stop_sequences``, and ``ChatBedrockConverse`` rejects a call that passes both spellings. +_CHAT_BEDROCK_CONVERSE_RENAMES = {"stop": "stop_sequences"} + +#: Accepted by ``ChatBedrockConverse`` but never forwarded, and why: +#: * ``bedrock_api_key``, ``api_key``, ``aws_access_key_id``, ``aws_secret_access_key``, +#: ``aws_session_token``, ``credentials_profile_name``: credentials. +#: * ``endpoint_url``, ``base_url``, ``region_name``: where requests go, and the region, which +#: decides where data is processed. +#: * ``client``, ``bedrock_client``, ``config``: raw boto3/botocore client objects and config. +#: * ``default_headers``, ``additional_model_request_fields``: raw request injection. +#: ``additional_model_request_fields`` is passed into the request body unfiltered. +#: * ``max_retries``, ``timeout``: retries and timeouts. +#: * ``base_model_id``, ``base_model``, ``provider``: which model the handler is talking to, the +#: same identity as ``model_id`` (handler-owned). +#: * ``additional_model_response_field_paths``, ``raw_blocks``, ``supports_tool_choice_values``: +#: the response shape and tool-calling behaviour the handler relies on. +#: * ``guardrails``, ``guardrail_config``, ``guard_last_turn_only``: safety configuration. +#: * ``request_metadata``: identity and attribution. It tags the request for whoever reads the +#: invocation logs. +#: * ``system``: prompt content beyond instructions. It adds system prompt blocks on top of the +#: config's own instructions. +#: * ``output_config``: API-shape switch. Its only use is Bedrock's JSON schema output format, the +#: structured output the handler owns through the config's ``outputFormat``. Set on the +#: constructor, it is sent on every request, including the forced tool call the handler's +#: ``with_structured_output`` makes, so the two would constrain the reply at once. +#: * Everything in :data:`_LANGCHAIN_RUNTIME_KEYS`. +#: +#: Named for the drift test and for review, not read at runtime. +_CHAT_BEDROCK_CONVERSE_EXCLUDED_KEYS = _LANGCHAIN_RUNTIME_KEYS | { + "bedrock_api_key", + "api_key", + "aws_access_key_id", + "aws_secret_access_key", + "aws_session_token", + "credentials_profile_name", + "endpoint_url", + "base_url", + "region_name", + "client", + "bedrock_client", + "config", + "default_headers", + "additional_model_request_fields", + "max_retries", + "timeout", + "base_model_id", + "base_model", + "provider", + "additional_model_response_field_paths", + "raw_blocks", + "supports_tool_choice_values", + "guardrails", + "guardrail_config", + "guard_last_turn_only", + "request_metadata", + "system", + "output_config", +} + def _build_agent_tools( config_tools: dict[str, Any], @@ -181,12 +464,21 @@ def _config_for_model_call(config: AiConfigRep) -> AiConfigRep: def _model_constructor_kwargs( - config: AiConfigRep, fallback_name: str + config: AiConfigRep, + fallback_name: str, + forwarded_keys: frozenset[str], + *, + mapping_keys: frozenset[str] = frozenset(), + renames: Mapping[str, str] | None = None, ) -> dict[str, Any]: - raw = (config.get("model") or {}).get("parameters") - parameters = dict(raw) if isinstance(raw, dict) else {} - # Tools are bound from config["tools"]. A tools key here is forwarded raw and rejected. - parameters.pop("tools", None) + params = model_parameters(config) + for alias, name in (renames or {}).items(): + if alias in params: + value = params.pop(alias) + params.setdefault(name, value) + parameters = select_forwarded_parameters( + params, forwarded_keys, mapping_keys=mapping_keys + ) parameters["model"] = _resolved_model_name(config, fallback_name) return parameters @@ -201,7 +493,8 @@ def _make_default_chat_model(config: AiConfigRep) -> Any: Instantiate the appropriate LangChain chat model based on ``config.provider.name``. Falls back to ``ChatOpenAI`` when the provider is not recognised. Requires the matching ``langchain-`` integration package to be installed. - ``model.parameters`` are passed through, except ``tools``, which is bound separately. + ``model.parameters`` are filtered to that class's forwarded list first. ``tools`` is never on + that list, since tools are bound separately from ``config["tools"]``. """ import importlib @@ -209,7 +502,13 @@ def _make_default_chat_model(config: AiConfigRep) -> Any: if provider == "anthropic": lc_anthropic = importlib.import_module("langchain_anthropic") return lc_anthropic.ChatAnthropic( - **_model_constructor_kwargs(config, "claude-3-5-sonnet-20241022") + **_model_constructor_kwargs( + config, + "claude-3-5-sonnet-20241022", + _CHAT_ANTHROPIC_FORWARDED_KEYS, + mapping_keys=_CHAT_ANTHROPIC_MAPPING_KEYS, + renames=_CHAT_ANTHROPIC_RENAMES, + ) ) if provider == "bedrock": try: @@ -219,9 +518,24 @@ def _make_default_chat_model(config: AiConfigRep) -> Any: "Using Bedrock models requires langchain-aws. " "Install it with: pip install langchain-aws" ) from exc - return lc_aws.ChatBedrockConverse(**_model_constructor_kwargs(config, "")) + return lc_aws.ChatBedrockConverse( + **_model_constructor_kwargs( + config, + "", + _CHAT_BEDROCK_CONVERSE_FORWARDED_KEYS, + mapping_keys=_CHAT_BEDROCK_CONVERSE_MAPPING_KEYS, + renames=_CHAT_BEDROCK_CONVERSE_RENAMES, + ) + ) lc_openai = importlib.import_module("langchain_openai") - return lc_openai.ChatOpenAI(**_model_constructor_kwargs(config, "gpt-4o")) + return lc_openai.ChatOpenAI( + **_model_constructor_kwargs( + config, + "gpt-4o", + _CHAT_OPENAI_FORWARDED_KEYS, + mapping_keys=_CHAT_OPENAI_MAPPING_KEYS, + ) + ) async def _resolve_base_model(config: AiConfigRep, llm: Any) -> Any: diff --git a/packages/langchain-agents/src/launchdarkly_ai_langchain_agents/native_graph.py b/packages/langchain-agents/src/launchdarkly_ai_langchain_agents/native_graph.py index 5bd21b22..a9ddade0 100644 --- a/packages/langchain-agents/src/launchdarkly_ai_langchain_agents/native_graph.py +++ b/packages/langchain-agents/src/launchdarkly_ai_langchain_agents/native_graph.py @@ -25,9 +25,11 @@ to_ld_context, tool_result_string, ) -from launchdarkly_ai_server.utils import make_graph_track_data +from launchdarkly_ai_server.parameter_forwarding import select_forwarded_parameters +from launchdarkly_ai_server.utils import make_graph_track_data, model_parameters from ._version import PACKAGE_NAME, __version__ +from .handler import _CHAT_OPENAI_FORWARDED_KEYS, _CHAT_OPENAI_MAPPING_KEYS from .messages import to_lang_chain_messages try: @@ -223,10 +225,13 @@ async def _traverse_node(node: GraphNode) -> None: else: lc_openai = importlib.import_module("langchain_openai") model_cfg = node.config.get("model") or {} - raw = model_cfg.get("parameters") - kwargs = dict(raw) if isinstance(raw, dict) else {} - # Tools are bound from the node config. A tools key here is forwarded raw and rejected. - kwargs.pop("tools", None) + kwargs = model_parameters(node.config) + if kwargs: + kwargs = select_forwarded_parameters( + kwargs, + _CHAT_OPENAI_FORWARDED_KEYS, + mapping_keys=_CHAT_OPENAI_MAPPING_KEYS, + ) kwargs["model"] = model_cfg.get("name") or "gpt-4o" chat_model = lc_openai.ChatOpenAI(**kwargs) diff --git a/packages/langchain-agents/tests/test_handler.py b/packages/langchain-agents/tests/test_handler.py index 07bf45a2..a68c42a4 100644 --- a/packages/langchain-agents/tests/test_handler.py +++ b/packages/langchain-agents/tests/test_handler.py @@ -13,7 +13,9 @@ import pydantic import pytest +from langchain_anthropic import ChatAnthropic as _REAL_CHAT_ANTHROPIC from langchain_core.language_models.chat_models import BaseChatModel +from langchain_openai import ChatOpenAI as _REAL_CHAT_OPENAI import launchdarkly_ai_langchain_agents.handler as handler_mod import launchdarkly_ai_langchain_agents.spans as spans_mod @@ -23,6 +25,16 @@ create_langchain_agents_handler, ) +# ``langchain_aws`` is not a dependency of this package's test environment, so a mocked +# ``ChatBedrockConverse`` gets a minimal hand-built ``model_fields`` shape (field name -> a +# duck-typed stand-in exposing ``.alias``/``.validation_alias``, just like a real pydantic +# ``FieldInfo``) covering the parameters these tests actually forward. +_FAKE_BEDROCK_FIELDS = { + "temperature": SimpleNamespace(alias=None, validation_alias=None), + "top_p": SimpleNamespace(alias=None, validation_alias=None), + "max_tokens": SimpleNamespace(alias=None, validation_alias=None), +} + # --------------------------------------------------------------------------- # Helpers # --------------------------------------------------------------------------- @@ -2438,6 +2450,54 @@ async def test_a_reply_in_content_blocks_still_reaches_the_root(self) -> None: assert "a typed block" in str(rec.root.attributes["gen_ai.output.messages"]) +class TestModelParametersForwarding: + @pytest.mark.asyncio + async def test_ui_key_the_sdk_rejects_is_dropped_without_raising(self) -> None: + """``tools`` is not a ``ChatOpenAI`` constructor field (tools are bound via + ``bind_tools`` at call time); forwarding it unfiltered is a landmine, not a crash today + (pydantic ignores unknown constructor kwargs), but this handler must not rely on that.""" + ctx, _rec = _recording() + llm = _FakeToolModel(replies=[_ai_message("answer")]) + ctor = MagicMock(return_value=llm) + ctor.model_fields = _REAL_CHAT_OPENAI.model_fields + cfg = { + **BASE_CONFIG, + "model": { + "name": "gpt-4o", + "parameters": {"temperature": 0.2, "tools": [{"name": "x"}]}, + }, + } + with ( + ctx, + patch.dict("sys.modules", {"langchain_openai": MagicMock(ChatOpenAI=ctor)}), + ): + await create_langchain_agents_handler()(cfg, "q") + assert ctor.call_args.kwargs == {"temperature": 0.2, "model": "gpt-4o"} + + @pytest.mark.asyncio + async def test_transport_key_is_never_forwarded(self) -> None: + ctx, _rec = _recording() + llm = _FakeToolModel(replies=[_ai_message("answer")]) + ctor = MagicMock(return_value=llm) + ctor.model_fields = _REAL_CHAT_OPENAI.model_fields + cfg = { + **BASE_CONFIG, + "model": { + "name": "gpt-4o", + "parameters": { + "temperature": 0.2, + "extra_body": {"secret": "value"}, + }, + }, + } + with ( + ctx, + patch.dict("sys.modules", {"langchain_openai": MagicMock(ChatOpenAI=ctor)}), + ): + await create_langchain_agents_handler()(cfg, "q") + assert ctor.call_args.kwargs == {"temperature": 0.2, "model": "gpt-4o"} + + class TestModelSource: @pytest.mark.asyncio async def test_factory_receives_config_and_returned_model_is_used(self) -> None: @@ -2485,9 +2545,14 @@ async def test_prebuilt_instance_is_used_as_is(self) -> None: @pytest.mark.asyncio async def test_default_openai_constructor_receives_parameters(self) -> None: + """``tools`` is dropped: it is not a ``ChatOpenAI`` constructor field (tools are bound via + ``bind_tools`` at call time, not passed to the constructor), so the UI offering it must not + raise or silently no-op — it is filtered out before this handler ever reaches the + constructor.""" ctx, _rec = _recording() llm = _FakeToolModel(replies=[_ai_message("default-openai")]) ctor = MagicMock(return_value=llm) + ctor.model_fields = _REAL_CHAT_OPENAI.model_fields cfg = { **BASE_CONFIG, "model": { @@ -2515,6 +2580,7 @@ async def test_default_anthropic_constructor_receives_parameters(self) -> None: ctx, _rec = _recording() llm = _FakeToolModel(replies=[_ai_message("default-anthropic")]) ctor = MagicMock(return_value=llm) + ctor.model_fields = _REAL_CHAT_ANTHROPIC.model_fields cfg = { **BASE_CONFIG, "provider": {"name": "Anthropic"}, @@ -2539,6 +2605,7 @@ async def test_bedrock_region_is_prepended_to_the_default_constructor( ctx, _rec = _recording() llm = _FakeToolModel(replies=[_ai_message("bedrock")]) ctor = MagicMock(return_value=llm) + ctor.model_fields = _FAKE_BEDROCK_FIELDS cfg = { **BASE_CONFIG, "provider": {"name": "Bedrock"}, @@ -2698,3 +2765,43 @@ def factory(config: Any) -> Any: e.get("type") == "TEXT_MESSAGE_CONTENT" and e.get("delta") == "streamed" for e in events ) + + +class TestConnectionConfigIsNeverForwarded: + """``api_key``/``base_url`` in ``model.parameters`` are client/connection configuration; a + config author must never be able to redirect a call to a different endpoint or credential. + """ + + @pytest.mark.parametrize( + "provider,fallback", [("openai", "gpt-4o"), ("anthropic", "claude")] + ) + def test_api_key_and_base_url_are_never_forwarded( + self, provider: str, fallback: str + ) -> None: + from launchdarkly_ai_langchain_agents.handler import ( + _CHAT_ANTHROPIC_FORWARDED_KEYS, + _CHAT_OPENAI_FORWARDED_KEYS, + _model_constructor_kwargs, + ) + + forwarded_keys = ( + _CHAT_OPENAI_FORWARDED_KEYS + if provider == "openai" + else _CHAT_ANTHROPIC_FORWARDED_KEYS + ) + cfg = { + **BASE_CONFIG, + "provider": {"name": provider}, + "model": { + "name": fallback, + "parameters": { + "api_key": "stolen-key", + "base_url": "https://evil.example.com", + "temperature": 0.3, + }, + }, + } + kwargs = _model_constructor_kwargs(cfg, fallback, forwarded_keys) + assert "api_key" not in kwargs + assert "base_url" not in kwargs + assert kwargs["temperature"] == 0.3 diff --git a/packages/langchain-agents/tests/test_native_graph.py b/packages/langchain-agents/tests/test_native_graph.py index 264d9575..c913e43a 100644 --- a/packages/langchain-agents/tests/test_native_graph.py +++ b/packages/langchain-agents/tests/test_native_graph.py @@ -13,6 +13,7 @@ from unittest.mock import AsyncMock, MagicMock, patch import pytest +from langchain_openai import ChatOpenAI as _REAL_CHAT_OPENAI from launchdarkly_ai_langchain_agents.native_graph import ( _build_node_tools, @@ -20,6 +21,12 @@ to_lang_graph, ) from launchdarkly_ai_server import GraphDefinition, GraphEdge, GraphNode +from tests.forwarding_spec import ( + LANGCHAIN_CHAT_OPENAI, + candidate_keys, + probe_forwarded_keys, +) +from tests.never_forwarded import NEVER_FORWARDED_BAG # --------------------------------------------------------------------------- # Helpers @@ -173,6 +180,7 @@ def compile(self) -> Any: mock_lc_openai = MagicMock() mock_lc_openai.ChatOpenAI = MagicMock(return_value=mock_chat_model) + mock_lc_openai.ChatOpenAI.model_fields = _REAL_CHAT_OPENAI.model_fields mock_gm = MagicMock() mock_gm.add_messages = MagicMock(return_value=MagicMock()) @@ -1271,3 +1279,78 @@ def model_factory(_node: Any) -> Any: assert isinstance(result, dict) assert "response" in result + + +class TestNativeGraphNeverForwardedParameters: + @pytest.mark.asyncio + async def test_default_chat_openai_receives_no_never_forwarded_key(self) -> None: + ai_msg = _make_ai_msg("final") + mocks = _make_langgraph_mocks(ai_msg) + graph_def = _make_graph_def( + nodes={ + "root": { + "key": "root", + "config": { + "model": { + "name": "gpt-4o", + "parameters": {**NEVER_FORWARDED_BAG, "temperature": 0.2}, + }, + "instructions": "help", + }, + "meta": {"variationKey": "v1", "version": 1}, + "edges": [], + "is_terminal": True, + } + } + ) + + async def _visit(fn: Any, ctx: Any = None) -> None: + if graph_def.root is not None: + await fn(graph_def.root) + + graph_def.traverse = _visit + + with _patch_imports(mocks): + await to_lang_graph(_make_def_promise(graph_def)).invoke("hi") + + assert mocks["langchain_openai"].ChatOpenAI.call_args.kwargs == { + "temperature": 0.2, + "model": "gpt-4o", + } + + +class TestNativeGraphForwardsExactlyTheCrossSdkList: + """The native graph's default ``ChatOpenAI`` takes the same forwarded list as the handler's, + so a probe of one node finds exactly the cross-SDK ChatOpenAI list.""" + + @pytest.mark.asyncio + async def test_default_chat_openai(self) -> None: + async def call(parameters: dict[str, Any]) -> object: + mocks = _make_langgraph_mocks(_make_ai_msg("final")) + graph_def = _make_graph_def( + nodes={ + "root": { + "key": "root", + "config": { + "model": {"name": "gpt-4o", "parameters": parameters}, + "instructions": "help", + }, + "meta": {"variationKey": "v1", "version": 1}, + "edges": [], + "is_terminal": True, + } + } + ) + + async def _visit(fn: Any, ctx: Any = None) -> None: + if graph_def.root is not None: + await fn(graph_def.root) + + graph_def.traverse = _visit + with _patch_imports(mocks): + await to_lang_graph(_make_def_promise(graph_def)).invoke("hi") + return mocks["langchain_openai"].ChatOpenAI.call_args.kwargs + + expected = LANGCHAIN_CHAT_OPENAI + candidates = candidate_keys(_REAL_CHAT_OPENAI.model_fields, expected) + assert await probe_forwarded_keys(candidates, call) == expected diff --git a/packages/langchain-agents/tests/test_parameter_forwarding.py b/packages/langchain-agents/tests/test_parameter_forwarding.py new file mode 100644 index 00000000..34823037 --- /dev/null +++ b/packages/langchain-agents/tests/test_parameter_forwarding.py @@ -0,0 +1,479 @@ +""" +Drift test for the LangChain chat model parameter classification in ``handler.py``. + +``_CHAT_OPENAI_FORWARDED_KEYS`` / ``_CHAT_ANTHROPIC_FORWARDED_KEYS`` / +``_CHAT_BEDROCK_CONVERSE_FORWARDED_KEYS`` are literal, hand-maintained lists. This test reads each +chat model's own ``model_fields`` (field names plus pydantic aliases) and asserts every key it +accepts is classified in exactly one of forwarded, handler-owned, or excluded, so a field nobody has +classified yet fails loudly by name, and so does a list entry that is not a real field. + +``langchain-aws`` is not a dependency of this package (Bedrock support is opt-in), so the +``ChatBedrockConverse`` test skips itself when it is not installed. +""" + +from __future__ import annotations + +import importlib +from collections.abc import Mapping +from typing import Any, ClassVar +from unittest.mock import MagicMock, patch + +import langchain_anthropic +import langchain_openai +import pytest + +from launchdarkly_ai_langchain_agents.handler import ( + _CHAT_ANTHROPIC_EXCLUDED_KEYS, + _CHAT_ANTHROPIC_FORWARDED_KEYS, + _CHAT_ANTHROPIC_OWNED_KEYS, + _CHAT_BEDROCK_CONVERSE_EXCLUDED_KEYS, + _CHAT_BEDROCK_CONVERSE_FORWARDED_KEYS, + _CHAT_BEDROCK_CONVERSE_OWNED_KEYS, + _CHAT_OPENAI_EXCLUDED_KEYS, + _CHAT_OPENAI_FORWARDED_KEYS, + _CHAT_OPENAI_OWNED_KEYS, + _make_default_chat_model, + _model_constructor_kwargs, +) +from tests.forwarding_spec import ( + LANGCHAIN_CHAT_ANTHROPIC, + LANGCHAIN_CHAT_BEDROCK_CONVERSE, + LANGCHAIN_CHAT_OPENAI, + candidate_keys, + probe_forwarded_keys, + sample, +) +from tests.never_forwarded import NEVER_FORWARDED_BAG + + +def _accepted_keys(cls: Any) -> frozenset[str]: + """Every key *cls* (a pydantic model) accepts by construction: each field's own name plus any + string alias it declares.""" + fields: Mapping[str, Any] = cls.model_fields + accepted: set[str] = set() + for name, field in fields.items(): + accepted.add(name) + alias = getattr(field, "alias", None) + if isinstance(alias, str): + accepted.add(alias) + validation_alias = getattr(field, "validation_alias", None) + if isinstance(validation_alias, str): + accepted.add(validation_alias) + else: + choices = getattr(validation_alias, "choices", None) + if choices: + accepted.update(c for c in choices if isinstance(c, str)) + return frozenset(accepted) + + +class TestChatOpenAIAcceptsExactlyTheseKeys: + def test_every_accepted_key_is_classified_exactly_once(self) -> None: + accepted = _accepted_keys(langchain_openai.ChatOpenAI) + classified = ( + _CHAT_OPENAI_FORWARDED_KEYS + | _CHAT_OPENAI_OWNED_KEYS + | _CHAT_OPENAI_EXCLUDED_KEYS + ) + + unclassified = accepted - classified + assert not unclassified, ( + f"ChatOpenAI now accepts {sorted(unclassified)}, not classified as forwarded, " + "handler-owned, or excluded in langchain-agents handler.py" + ) + + overlap = ( + (_CHAT_OPENAI_FORWARDED_KEYS & _CHAT_OPENAI_OWNED_KEYS) + | (_CHAT_OPENAI_FORWARDED_KEYS & _CHAT_OPENAI_EXCLUDED_KEYS) + | (_CHAT_OPENAI_OWNED_KEYS & _CHAT_OPENAI_EXCLUDED_KEYS) + ) + assert not overlap, f"keys classified more than once: {sorted(overlap)}" + + def test_every_classified_key_is_real(self) -> None: + accepted = _accepted_keys(langchain_openai.ChatOpenAI) + stale = ( + _CHAT_OPENAI_FORWARDED_KEYS + | _CHAT_OPENAI_OWNED_KEYS + | _CHAT_OPENAI_EXCLUDED_KEYS + ) - accepted + assert not stale, ( + f"{sorted(stale)} classified in langchain-agents handler.py but " + "ChatOpenAI does not accept them" + ) + + +class TestChatAnthropicAcceptsExactlyTheseKeys: + def test_every_accepted_key_is_classified_exactly_once(self) -> None: + accepted = _accepted_keys(langchain_anthropic.ChatAnthropic) + classified = ( + _CHAT_ANTHROPIC_FORWARDED_KEYS + | _CHAT_ANTHROPIC_OWNED_KEYS + | _CHAT_ANTHROPIC_EXCLUDED_KEYS + ) + + unclassified = accepted - classified + assert not unclassified, ( + f"ChatAnthropic now accepts {sorted(unclassified)}, not classified as forwarded, " + "handler-owned, or excluded in langchain-agents handler.py" + ) + + def test_every_classified_key_is_real(self) -> None: + accepted = _accepted_keys(langchain_anthropic.ChatAnthropic) + stale = ( + _CHAT_ANTHROPIC_FORWARDED_KEYS + | _CHAT_ANTHROPIC_OWNED_KEYS + | _CHAT_ANTHROPIC_EXCLUDED_KEYS + ) - accepted + assert not stale, ( + f"{sorted(stale)} classified in langchain-agents handler.py but " + "ChatAnthropic does not accept them" + ) + + +class TestChatBedrockConverseAcceptsExactlyTheseKeys: + def test_every_accepted_key_is_classified_exactly_once(self) -> None: + lc_aws = pytest.importorskip("langchain_aws") + accepted = _accepted_keys(lc_aws.ChatBedrockConverse) + classified = ( + _CHAT_BEDROCK_CONVERSE_FORWARDED_KEYS + | _CHAT_BEDROCK_CONVERSE_OWNED_KEYS + | _CHAT_BEDROCK_CONVERSE_EXCLUDED_KEYS + ) + + unclassified = accepted - classified + assert not unclassified, ( + f"ChatBedrockConverse now accepts {sorted(unclassified)}, not classified as " + "forwarded, handler-owned, or excluded in langchain-agents handler.py" + ) + + def test_every_classified_key_is_real(self) -> None: + lc_aws = pytest.importorskip("langchain_aws") + accepted = _accepted_keys(lc_aws.ChatBedrockConverse) + stale = ( + _CHAT_BEDROCK_CONVERSE_FORWARDED_KEYS + | _CHAT_BEDROCK_CONVERSE_OWNED_KEYS + | _CHAT_BEDROCK_CONVERSE_EXCLUDED_KEYS + ) - accepted + assert not stale, ( + f"{sorted(stale)} classified in langchain-agents handler.py but " + "ChatBedrockConverse does not accept them" + ) + + +class TestModelFieldAliasesAreNeverForwarded: + """``model_name`` / ``model_id`` are the same constructor field as ``model``. A config value + for them must not be forwarded, or it would collide with the model the handler resolves.""" + + def _config(self, provider: str, parameters: dict[str, Any]) -> Any: + return { + "model": {"name": "configured-model", "parameters": parameters}, + "provider": {"name": provider}, + } + + def test_openai_model_name_is_dropped(self) -> None: + kwargs = _model_constructor_kwargs( + self._config("openai", {"model_name": "other", "temperature": 0.2}), + "fallback", + _CHAT_OPENAI_FORWARDED_KEYS, + ) + assert kwargs == {"model": "configured-model", "temperature": 0.2} + + def test_anthropic_model_name_is_dropped(self) -> None: + kwargs = _model_constructor_kwargs( + self._config("anthropic", {"model_name": "other", "temperature": 0.2}), + "fallback", + _CHAT_ANTHROPIC_FORWARDED_KEYS, + ) + assert kwargs == {"model": "configured-model", "temperature": 0.2} + + def test_bedrock_model_id_is_dropped(self) -> None: + kwargs = _model_constructor_kwargs( + self._config("bedrock", {"model_id": "other", "temperature": 0.2}), + "fallback", + _CHAT_BEDROCK_CONVERSE_FORWARDED_KEYS, + ) + assert kwargs == {"model": "configured-model", "temperature": 0.2} + + +class TestNeverForwardedKeys: + """No credential, endpoint, request-injection, or remote-tool key in ``model.parameters`` + reaches any chat model constructor.""" + + def _config(self, provider: str, parameters: dict[str, Any]) -> Any: + return { + "model": {"name": "configured-model", "parameters": parameters}, + "provider": {"name": provider}, + } + + @pytest.mark.parametrize( + ("provider", "forwarded_keys"), + [ + ("openai", _CHAT_OPENAI_FORWARDED_KEYS), + ("anthropic", _CHAT_ANTHROPIC_FORWARDED_KEYS), + ("bedrock", _CHAT_BEDROCK_CONVERSE_FORWARDED_KEYS), + ], + ) + def test_constructor_kwargs_hold_none_of_them( + self, provider: str, forwarded_keys: frozenset[str] + ) -> None: + kwargs = _model_constructor_kwargs( + self._config(provider, {**NEVER_FORWARDED_BAG, "temperature": 0.2}), + "fallback", + forwarded_keys, + ) + assert kwargs == {"model": "configured-model", "temperature": 0.2} + + +class TestModelKwargsCannotSmuggleRequestKeys: + """``ChatOpenAI`` merges ``model_kwargs`` straight into the request payload, so forwarding it + would carry ``extra_headers``/``extra_query`` past every exclusion.""" + + _SMUGGLED: ClassVar[dict[str, Any]] = { + "model_kwargs": { + "extra_headers": {"X-Smuggled": "1"}, + "extra_query": {"smuggled": "1"}, + } + } + + def _payload(self, **kwargs: Any) -> dict[str, Any]: + from langchain_core.messages import HumanMessage + + model = langchain_openai.ChatOpenAI(api_key="sk-test-not-a-real-key", **kwargs) + payload: dict[str, Any] = model._get_request_payload([HumanMessage("hi")]) + return payload + + def test_model_kwargs_would_reach_the_payload_if_forwarded(self) -> None: + """The control: passed to ``ChatOpenAI`` directly, both keys reach the payload.""" + payload = self._payload(model="gpt-4o", **self._SMUGGLED) + assert payload["extra_headers"] == {"X-Smuggled": "1"} + assert payload["extra_query"] == {"smuggled": "1"} + + def test_forwarded_model_parameters_keep_them_out_of_the_payload(self) -> None: + kwargs = _model_constructor_kwargs( + { + "model": {"name": "gpt-4o", "parameters": dict(self._SMUGGLED)}, + "provider": {"name": "openai"}, + }, + "fallback", + _CHAT_OPENAI_FORWARDED_KEYS, + ) + payload = self._payload(**kwargs) + assert "extra_headers" not in payload + assert "extra_query" not in payload + + +def _probe_config(provider: str, parameters: dict[str, Any]) -> Any: + return { + "model": {"name": "configured-model", "parameters": parameters}, + "provider": {"name": provider}, + } + + +def _build(provider: str, parameters: dict[str, Any]) -> dict[str, Any]: + """Runs ``_make_default_chat_model`` for *provider* against a recording constructor and + returns the kwargs it passed.""" + ctor = MagicMock() + module = MagicMock(ChatOpenAI=ctor, ChatAnthropic=ctor, ChatBedrockConverse=ctor) + with patch("importlib.import_module", return_value=module): + _make_default_chat_model(_probe_config(provider, parameters)) + kwargs: dict[str, Any] = ctor.call_args.kwargs + return kwargs + + +def _accepted_or_empty(module_name: str, cls_name: str) -> frozenset[str]: + try: + module = importlib.import_module(module_name) + except ImportError: + return frozenset() + return _accepted_keys(getattr(module, cls_name)) + + +class TestForwardsExactlyTheCrossSdkList: + """Probes each chat model constructor one key at a time: the keys that change what it is + built with are exactly the cross-SDK list for that class. ``invoke`` and ``stream`` both + build the model through ``_make_default_chat_model``, so this covers both paths.""" + + @pytest.mark.parametrize( + ("provider", "module_name", "cls_name", "expected"), + [ + ( + "openai", + "langchain_openai", + "ChatOpenAI", + LANGCHAIN_CHAT_OPENAI, + ), + ( + "anthropic", + "langchain_anthropic", + "ChatAnthropic", + LANGCHAIN_CHAT_ANTHROPIC, + ), + ( + "bedrock", + "langchain_aws", + "ChatBedrockConverse", + LANGCHAIN_CHAT_BEDROCK_CONVERSE, + ), + ], + ) + async def test_forwarded_keys( + self, provider: str, module_name: str, cls_name: str, expected: frozenset[str] + ) -> None: + baseline = _build(provider, {}) + + async def call(parameters: dict[str, Any]) -> object: + return _build(provider, parameters) + + candidates = candidate_keys(_accepted_or_empty(module_name, cls_name), expected) + assert await probe_forwarded_keys(candidates, call) == expected + assert baseline == {"model": "configured-model"} + + @pytest.mark.parametrize( + ("cls", "keys"), + [ + ( + langchain_openai.ChatOpenAI, + LANGCHAIN_CHAT_OPENAI, + ), + (langchain_anthropic.ChatAnthropic, LANGCHAIN_CHAT_ANTHROPIC), + ], + ) + def test_the_real_class_accepts_every_forwarded_key( + self, cls: Any, keys: frozenset[str] + ) -> None: + for key in sorted(keys): + cls(model="m", api_key="sk-test-not-a-real-key", **{key: sample(key)}) + + +class TestMalformedObjectValuesAreDropped: + @pytest.mark.parametrize( + ("provider", "parameters"), + [ + ("openai", {"logit_bias": "none"}), + ("anthropic", {"thinking": "enabled", "output_config": "json"}), + ("bedrock", {"performance_config": "optimized"}), + ], + ) + def test_dropped(self, provider: str, parameters: dict[str, Any]) -> None: + kwargs = _build(provider, {**parameters, "temperature": 0.2}) + assert kwargs == {"model": "configured-model", "temperature": 0.2} + + +class _Built(Exception): + """Stops a run once the chat model has been built.""" + + +class TestInvokeAndStreamBuildTheSameModel: + """Both paths hand the same config to ``_make_default_chat_model``, so the probe above + covers both.""" + + async def test_same_config_reaches_the_builder(self) -> None: + import launchdarkly_ai_langchain_agents.handler as handler_mod + from launchdarkly_ai_langchain_agents import create_langchain_agents_handler + + config = { + "model": { + "name": "gpt-4o", + "parameters": {**NEVER_FORWARDED_BAG, "temperature": 0.2, "top_p": 0.5}, + }, + "provider": {"name": "openai"}, + "instructions": "help", + } + seen: list[Any] = [] + + def _builder(cfg: Any, *_rest: Any) -> Any: + seen.append(cfg) + raise _Built + + with patch.object(handler_mod, "_make_default_chat_model", _builder): + h = create_langchain_agents_handler() + with pytest.raises(_Built): + await h(config, "q") + with pytest.raises(_Built): + async for _event in await h.stream(config, "q"): + pass + + assert len(seen) == 2 + assert seen[0] == seen[1] + + +class TestMaxTokensToSampleRename: + """``ChatAnthropic`` takes ``max_tokens_to_sample`` as an alias of ``max_tokens``. It is + renamed before forwarding, and ``max_tokens`` wins when both are set.""" + + def test_alias_becomes_max_tokens(self) -> None: + assert _build("anthropic", {"max_tokens_to_sample": 300}) == { + "model": "configured-model", + "max_tokens": 300, + } + + def test_max_tokens_wins_when_both_are_set(self) -> None: + assert _build( + "anthropic", {"max_tokens_to_sample": 300, "max_tokens": 200} + ) == {"model": "configured-model", "max_tokens": 200} + + +class TestBedrockStopRename: + """``ChatBedrockConverse`` takes ``stop`` as the alias of ``stop_sequences`` and rejects a call + that passes both. ``stop`` is renamed before forwarding, and ``stop_sequences`` wins when both + are set.""" + + def test_alias_becomes_stop_sequences(self) -> None: + assert _build("bedrock", {"stop": ["A"]}) == { + "model": "configured-model", + "stop_sequences": ["A"], + } + + def test_stop_sequences_wins_when_both_are_set(self) -> None: + assert _build("bedrock", {"stop": ["A"], "stop_sequences": ["B"]}) == { + "model": "configured-model", + "stop_sequences": ["B"], + } + + def test_the_real_class_accepts_the_forwarded_kwargs(self) -> None: + lc_aws = pytest.importorskip("langchain_aws") + kwargs = _build( + "bedrock", + {"stop": ["A"], "stop_sequences": ["B"], "reasoning_effort": "low"}, + ) + kwargs["model"] = "anthropic.claude-opus-5" + model = lc_aws.ChatBedrockConverse( + region_name="us-east-1", + aws_access_key_id="test-not-a-real-key", + aws_secret_access_key="test-not-a-real-secret", + **kwargs, + ) + assert model.stop_sequences == ["B"] + assert model.additional_model_request_fields == { + "thinking": {"type": "adaptive"}, + "output_config": {"effort": "low"}, + } + + +def test_prompt_cache_key_is_dropped_for_chat_openai() -> None: + """Not supported in Python: ``ChatOpenAI`` has no such field, so a config value never reaches + the constructor.""" + assert _build("openai", {"prompt_cache_key": "cache-1", "temperature": 0.2}) == { + "model": "configured-model", + "temperature": 0.2, + } + + +def test_reasoning_effort_keeps_chat_openai_on_chat_completions() -> None: + """``reasoning_effort`` is the Chat Completions field. Unlike ``reasoning`` it does not switch + ``ChatOpenAI`` to the Responses API, so the handler still reads a Chat Completions reply.""" + from langchain_core.messages import HumanMessage + + kwargs = _model_constructor_kwargs( + { + "model": {"name": "gpt-4o", "parameters": {"reasoning_effort": "low"}}, + "provider": {"name": "openai"}, + }, + "fallback", + _CHAT_OPENAI_FORWARDED_KEYS, + ) + model = langchain_openai.ChatOpenAI(api_key="sk-test-not-a-real-key", **kwargs) + payload: dict[str, Any] = model._get_request_payload([HumanMessage("hi")]) + assert payload["reasoning_effort"] == "low" + assert "messages" in payload + assert "input" not in payload + assert "reasoning" not in payload diff --git a/packages/langchain-messages/src/launchdarkly_ai_langchain_messages/handler.py b/packages/langchain-messages/src/launchdarkly_ai_langchain_messages/handler.py index bd5ec9fe..70a987c3 100644 --- a/packages/langchain-messages/src/launchdarkly_ai_langchain_messages/handler.py +++ b/packages/langchain-messages/src/launchdarkly_ai_langchain_messages/handler.py @@ -2,7 +2,7 @@ import asyncio import json -from collections.abc import AsyncGenerator +from collections.abc import AsyncGenerator, Mapping from types import SimpleNamespace from typing import Any @@ -35,6 +35,8 @@ ) from launchdarkly_ai_server.agui import TurnEmitter from launchdarkly_ai_server.lifecycle import effective_capture_content +from launchdarkly_ai_server.parameter_forwarding import select_forwarded_parameters +from launchdarkly_ai_server.utils import model_parameters from ._version import PACKAGE_NAME, __version__ from .spans import ( @@ -50,6 +52,287 @@ to_tool_definitions, ) +#: Handler-owned, per model class. The handler always sets the model itself, and each +#: class exposes that one constructor field under two names (the field and its alias), so a +#: config value for either must never be forwarded: it would collide with the model the +#: handler resolves, or override it. +_CHAT_OPENAI_OWNED_KEYS = frozenset({"model", "model_name"}) +_CHAT_ANTHROPIC_OWNED_KEYS = frozenset({"model", "model_name"}) +_CHAT_BEDROCK_CONVERSE_OWNED_KEYS = frozenset({"model", "model_id"}) + +#: LangChain's own runtime fields, shared by every chat model class: caching, callbacks, rate +#: limiting, tracing tags and metadata, streaming mode, message format, and token counting. They +#: configure how LangChain runs the model in this process, not the model request, and several are +#: objects or callables a config cannot express. Never forwarded. +_LANGCHAIN_RUNTIME_KEYS = frozenset( + { + "cache", + "callbacks", + "custom_get_token_ids", + "disable_streaming", + "metadata", + "name", + "output_version", + "profile", + "rate_limiter", + "streaming", + "tags", + "verbose", + } +) + +#: Every key ``ChatOpenAI`` accepts (field names plus pydantic aliases), classified by hand into +#: exactly one of: forwarded (below), handler-owned (``model``, always overwritten by the +#: resolved model name, see ``_model_constructor_kwargs``), or excluded (below). +#: ``TestChatOpenAIAcceptsExactlyTheseKeys`` in this package's tests asserts this classification +#: stays exhaustive as the SDK's own pydantic model changes. +#: +#: This is the cross-SDK list for LangChain ChatOpenAI (TESTING.md section 1.12). +#: ``reasoning_effort`` and ``seed`` are marked not supported in TypeScript there: the JS +#: ``ChatOpenAI`` takes them only as per-call options, while this handler sets them on the +#: constructor, which has a field for each. ``reasoning_effort`` is the Chat Completions field and +#: does not switch ``ChatOpenAI`` to the Responses API the way ``reasoning`` does. +_CHAT_OPENAI_FORWARDED_KEYS = frozenset( + { + "frequency_penalty", + "logit_bias", + "logprobs", + "max_completion_tokens", + "max_tokens", + "n", + "presence_penalty", + "reasoning_effort", + "seed", + "service_tier", + "stop", + "stop_sequences", + "temperature", + "top_logprobs", + "top_p", + "verbosity", + } +) + +#: Forwarded keys whose value must be an object. A config value of any other type is malformed +#: and dropped rather than passed to ``ChatOpenAI``. +_CHAT_OPENAI_MAPPING_KEYS = frozenset({"logit_bias"}) + +#: Accepted by ``ChatOpenAI`` but never forwarded, and why: +#: * ``api_key``, ``openai_api_key``, ``organization``, ``openai_organization``: credentials. +#: * ``base_url``, ``openai_api_base``, ``openai_proxy``: where requests go. +#: * ``client``, ``async_client``, ``root_client``, ``root_async_client``, ``http_client``, +#: ``http_async_client``, ``http_socket_options``: raw HTTP client objects/settings. +#: * ``default_headers``, ``default_query``, ``extra_body``, ``model_kwargs``: raw request +#: injection. ``model_kwargs`` is merged straight into the request payload, so it would carry +#: ``extra_headers``/``extra_query`` or any other excluded key past this list. +#: * ``max_retries``, ``request_timeout``, ``timeout``, ``stream_chunk_timeout``: retries and +#: timeouts. +#: * ``use_responses_api``, ``use_previous_response_id``, ``include``, ``reasoning``, +#: ``truncation``, ``context_management``: API-shape switch. Each one makes ``ChatOpenAI`` call +#: the Responses API instead of Chat Completions, which changes the API and response shape the +#: handler gets back. ``truncation``, ``context_management`` and ``use_previous_response_id`` +#: also lean on server-side conversation state. +#: * ``store``: data retention. It decides whether the request is kept on the server. +#: * ``prompt_cache_key``: on the spec list, marked not supported in Python, because +#: ``ChatOpenAI`` has no such constructor field (it takes it only per call). It is not in the set +#: below because ``ChatOpenAI`` does not accept it at all, which the drift test requires of every +#: listed key. +#: * ``stream_usage``, ``include_response_headers``, ``disabled_params``, ``tiktoken_model_name``: +#: runtime wiring. They change what the handler gets back and how usage is reported to it. +#: * Everything in :data:`_LANGCHAIN_RUNTIME_KEYS`. +#: +#: Named for the drift test and for review, not read at runtime: the forwarded list above already +#: leaves these out, so nothing needs to subtract them again. +_CHAT_OPENAI_EXCLUDED_KEYS = _LANGCHAIN_RUNTIME_KEYS | { + "api_key", + "openai_api_key", + "organization", + "openai_organization", + "base_url", + "openai_api_base", + "openai_proxy", + "client", + "async_client", + "root_client", + "root_async_client", + "http_client", + "http_async_client", + "http_socket_options", + "default_headers", + "default_query", + "extra_body", + "model_kwargs", + "max_retries", + "request_timeout", + "timeout", + "stream_chunk_timeout", + "stream_usage", + "include_response_headers", + "disabled_params", + "tiktoken_model_name", + "use_responses_api", + "use_previous_response_id", + "include", + "reasoning", + "truncation", + "context_management", + "store", +} + +#: Every key ``ChatAnthropic`` accepts (field names plus pydantic aliases), classified the same way +#: as :data:`_CHAT_OPENAI_FORWARDED_KEYS`. ``TestChatAnthropicAcceptsExactlyTheseKeys`` in this +#: package's tests asserts this classification stays exhaustive. +#: +#: This is the cross-SDK list for LangChain ChatAnthropic (TESTING.md section 1.12). +#: ``max_tokens_to_sample`` is renamed to ``max_tokens`` before this list applies, and a +#: ``max_tokens`` the config also set wins (see :data:`_CHAT_ANTHROPIC_RENAMES`). +_CHAT_ANTHROPIC_FORWARDED_KEYS = frozenset( + { + "betas", + "effort", + "max_tokens", + "max_tokens_to_sample", + "output_config", + "stop", + "stop_sequences", + "temperature", + "thinking", + "top_k", + "top_p", + } +) + +#: Forwarded keys whose value must be an object. A config value of any other type is malformed +#: and dropped rather than passed to ``ChatAnthropic``. +_CHAT_ANTHROPIC_MAPPING_KEYS = frozenset({"output_config", "thinking"}) + +#: Alias to the name it is renamed to before forwarding. ``ChatAnthropic`` takes both spellings +#: of the one field, so passing both would leave which one wins to pydantic. +_CHAT_ANTHROPIC_RENAMES = {"max_tokens_to_sample": "max_tokens"} + +#: Accepted by ``ChatAnthropic`` but never forwarded, and why: +#: * ``anthropic_api_key``, ``api_key``: credentials (field plus alias). +#: * ``anthropic_api_url``, ``base_url``, ``anthropic_proxy``: where requests go. +#: * ``inference_geo``: the region inference runs in, which decides where data is processed. +#: * ``context_management``: server-side state. It has the server clear or compact earlier +#: context. +#: * ``default_headers``, ``model_kwargs``: raw request injection. ``model_kwargs`` is merged +#: straight into the request, so it would carry any excluded key past this list. +#: * ``mcp_servers``: attaches remote MCP servers, which then receive the conversation. +#: * ``reuse_last_container``: reuses server-side container state from an earlier request, not +#: a model setting. +#: * ``default_request_timeout``, ``timeout``, ``max_retries``: timeouts and retries. +#: * ``stream_usage``: how usage is reported back to the handler. +#: * Everything in :data:`_LANGCHAIN_RUNTIME_KEYS`. +#: +#: Named for the drift test and for review, not read at runtime. +_CHAT_ANTHROPIC_EXCLUDED_KEYS = _LANGCHAIN_RUNTIME_KEYS | { + "anthropic_api_key", + "api_key", + "anthropic_api_url", + "base_url", + "anthropic_proxy", + "inference_geo", + "context_management", + "default_headers", + "model_kwargs", + "mcp_servers", + "reuse_last_container", + "default_request_timeout", + "timeout", + "max_retries", + "stream_usage", +} + +#: Every key ``ChatBedrockConverse`` accepts (field names plus pydantic aliases), classified the +#: same way as :data:`_CHAT_OPENAI_FORWARDED_KEYS`. ``langchain-aws`` is not a dependency of this +#: package (Bedrock support is opt-in, see ``_make_default_chat_model``), so +#: ``TestChatBedrockConverseAcceptsExactlyTheseKeys`` in this package's tests skips itself when it +#: is not installed rather than asserting nothing. +#: +#: This is the cross-SDK list for LangChain ChatBedrockConverse (TESTING.md section 1.12). +#: ``reasoning_effort``, ``stop`` and ``stop_sequences`` are marked not supported in TypeScript +#: there: the JS ``ChatBedrockConverse`` takes stop sequences only as a per-call option and has no +#: reasoning effort field, while this class has constructor fields for both. ``stop`` is renamed to +#: ``stop_sequences`` before this list applies, and a ``stop_sequences`` the config also set wins +#: (see :data:`_CHAT_BEDROCK_CONVERSE_RENAMES`). +_CHAT_BEDROCK_CONVERSE_FORWARDED_KEYS = frozenset( + { + "max_tokens", + "performance_config", + "reasoning_effort", + "service_tier", + "stop", + "stop_sequences", + "temperature", + "top_p", + } +) + +#: Forwarded keys whose value must be an object. A config value of any other type is malformed +#: and dropped rather than passed to ``ChatBedrockConverse``. +_CHAT_BEDROCK_CONVERSE_MAPPING_KEYS = frozenset({"performance_config"}) + +#: Alias to the name it is renamed to before forwarding. ``stop`` is the pydantic alias of +#: ``stop_sequences``, and ``ChatBedrockConverse`` rejects a call that passes both spellings. +_CHAT_BEDROCK_CONVERSE_RENAMES = {"stop": "stop_sequences"} + +#: Accepted by ``ChatBedrockConverse`` but never forwarded, and why: +#: * ``bedrock_api_key``, ``api_key``, ``aws_access_key_id``, ``aws_secret_access_key``, +#: ``aws_session_token``, ``credentials_profile_name``: credentials. +#: * ``endpoint_url``, ``base_url``, ``region_name``: where requests go, and the region, which +#: decides where data is processed. +#: * ``client``, ``bedrock_client``, ``config``: raw boto3/botocore client objects and config. +#: * ``default_headers``, ``additional_model_request_fields``: raw request injection. +#: ``additional_model_request_fields`` is passed into the request body unfiltered. +#: * ``max_retries``, ``timeout``: retries and timeouts. +#: * ``base_model_id``, ``base_model``, ``provider``: which model the handler is talking to, the +#: same identity as ``model_id`` (handler-owned). +#: * ``additional_model_response_field_paths``, ``raw_blocks``, ``supports_tool_choice_values``: +#: the response shape and tool-calling behaviour the handler relies on. +#: * ``guardrails``, ``guardrail_config``, ``guard_last_turn_only``: safety configuration. +#: * ``request_metadata``: identity and attribution. It tags the request for whoever reads the +#: invocation logs. +#: * ``system``: prompt content beyond instructions. It adds system prompt blocks on top of the +#: config's own instructions. +#: * ``output_config``: API-shape switch. Its only use is Bedrock's JSON schema output format, the +#: structured output the handler owns through the config's ``outputFormat``. Set on the +#: constructor, it is sent on every request, including the forced tool call the handler's +#: ``with_structured_output`` makes, so the two would constrain the reply at once. +#: * Everything in :data:`_LANGCHAIN_RUNTIME_KEYS`. +#: +#: Named for the drift test and for review, not read at runtime. +_CHAT_BEDROCK_CONVERSE_EXCLUDED_KEYS = _LANGCHAIN_RUNTIME_KEYS | { + "bedrock_api_key", + "api_key", + "aws_access_key_id", + "aws_secret_access_key", + "aws_session_token", + "credentials_profile_name", + "endpoint_url", + "base_url", + "region_name", + "client", + "bedrock_client", + "config", + "default_headers", + "additional_model_request_fields", + "max_retries", + "timeout", + "base_model_id", + "base_model", + "provider", + "additional_model_response_field_paths", + "raw_blocks", + "supports_tool_choice_values", + "guardrails", + "guardrail_config", + "guard_last_turn_only", + "request_metadata", + "system", + "output_config", +} + def _build_tools(config_tools: dict[str, Any]) -> list[dict[str, Any]]: # Not filtered to the tools that have a registered handler, unlike the TypeScript SDK's @@ -246,13 +529,21 @@ def _config_for_model_call(config: AiConfigRep) -> AiConfigRep: def _model_constructor_kwargs( - config: AiConfigRep, fallback_name: str + config: AiConfigRep, + fallback_name: str, + forwarded_keys: frozenset[str], + *, + mapping_keys: frozenset[str] = frozenset(), + renames: Mapping[str, str] | None = None, ) -> dict[str, Any]: - raw = (config.get("model") or {}).get("parameters") - parameters = dict(raw) if isinstance(raw, dict) else {} - # Tools are bound from config["tools"]. A tools key here is forwarded raw and rejected. - parameters.pop("tools", None) - # Name from the config always wins over a colliding ``model`` key in the parameter bag. + params = model_parameters(config) + for alias, name in (renames or {}).items(): + if alias in params: + value = params.pop(alias) + params.setdefault(name, value) + parameters = select_forwarded_parameters( + params, forwarded_keys, mapping_keys=mapping_keys + ) parameters["model"] = _resolved_model_name(config, fallback_name) return parameters @@ -267,13 +558,20 @@ def _make_default_chat_model(config: AiConfigRep, importlib: Any) -> Any: Instantiate the appropriate LangChain chat model based on ``config.provider.name``. Falls back to ``ChatOpenAI`` when the provider is not recognised. Requires the matching ``langchain-`` integration package to be installed. - ``model.parameters`` are passed through, except ``tools``, which is bound separately. + ``model.parameters`` are filtered to that class's forwarded list first. ``tools`` is never on + that list, since tools are bound separately from ``config["tools"]``. """ provider = config.get("provider", {}).get("name", "openai").lower() if provider == "anthropic": lc_anthropic = importlib.import_module("langchain_anthropic") return lc_anthropic.ChatAnthropic( - **_model_constructor_kwargs(config, "claude-3-5-sonnet-20241022") + **_model_constructor_kwargs( + config, + "claude-3-5-sonnet-20241022", + _CHAT_ANTHROPIC_FORWARDED_KEYS, + mapping_keys=_CHAT_ANTHROPIC_MAPPING_KEYS, + renames=_CHAT_ANTHROPIC_RENAMES, + ) ) if provider == "bedrock": try: @@ -283,9 +581,24 @@ def _make_default_chat_model(config: AiConfigRep, importlib: Any) -> Any: "Using Bedrock models requires langchain-aws. " "Install it with: pip install langchain-aws" ) from exc - return lc_aws.ChatBedrockConverse(**_model_constructor_kwargs(config, "")) + return lc_aws.ChatBedrockConverse( + **_model_constructor_kwargs( + config, + "", + _CHAT_BEDROCK_CONVERSE_FORWARDED_KEYS, + mapping_keys=_CHAT_BEDROCK_CONVERSE_MAPPING_KEYS, + renames=_CHAT_BEDROCK_CONVERSE_RENAMES, + ) + ) lc_openai = importlib.import_module("langchain_openai") - return lc_openai.ChatOpenAI(**_model_constructor_kwargs(config, "gpt-4o")) + return lc_openai.ChatOpenAI( + **_model_constructor_kwargs( + config, + "gpt-4o", + _CHAT_OPENAI_FORWARDED_KEYS, + mapping_keys=_CHAT_OPENAI_MAPPING_KEYS, + ) + ) async def _resolve_base_model(config: AiConfigRep, llm: Any, importlib: Any) -> Any: diff --git a/packages/langchain-messages/tests/test_handler.py b/packages/langchain-messages/tests/test_handler.py index 57c6a06a..19c9067f 100644 --- a/packages/langchain-messages/tests/test_handler.py +++ b/packages/langchain-messages/tests/test_handler.py @@ -8,10 +8,21 @@ import json import sys from collections.abc import AsyncGenerator +from types import SimpleNamespace from typing import Any, ClassVar from unittest.mock import AsyncMock, MagicMock, patch import pytest +from langchain_anthropic import ChatAnthropic as _REAL_CHAT_ANTHROPIC +from langchain_openai import ChatOpenAI as _REAL_CHAT_OPENAI + +# ``langchain_aws`` is not a dependency of this package's test environment, so a mocked +# ``ChatBedrockConverse`` gets a minimal hand-built ``model_fields`` shape (field name -> a +# duck-typed stand-in exposing ``.alias``/``.validation_alias``, just like a real pydantic +# ``FieldInfo``) covering the parameters these tests actually forward. +_FAKE_BEDROCK_FIELDS = { + "temperature": SimpleNamespace(alias=None, validation_alias=None), +} # --------------------------------------------------------------------------- # Fake LangChain message helpers @@ -2552,6 +2563,55 @@ def _counting(span: Any, capture: bool, *a: Any, **k: Any) -> None: assert calls["n"] == 0 +class TestModelParametersForwarding: + @pytest.mark.asyncio + async def test_ui_key_the_sdk_rejects_is_dropped_without_raising(self) -> None: + ctx, _rec = _recording() + from launchdarkly_ai_langchain_messages import create_langchain_messages_handler + + llm = _make_llm("answer") + ctor = MagicMock(return_value=llm) + ctor.model_fields = _REAL_CHAT_OPENAI.model_fields + cfg = { + **CONFIG, + "model": { + "name": "gpt-4o", + "parameters": {"temperature": 0.2, "tools": [{"name": "x"}]}, + }, + } + with ( + ctx, + patch.dict(sys.modules, {"langchain_openai": MagicMock(ChatOpenAI=ctor)}), + ): + await create_langchain_messages_handler()(cfg, "q", {}, {}) + assert ctor.call_args.kwargs == {"temperature": 0.2, "model": "gpt-4o"} + + @pytest.mark.asyncio + async def test_transport_key_is_never_forwarded(self) -> None: + ctx, _rec = _recording() + from launchdarkly_ai_langchain_messages import create_langchain_messages_handler + + llm = _make_llm("answer") + ctor = MagicMock(return_value=llm) + ctor.model_fields = _REAL_CHAT_OPENAI.model_fields + cfg = { + **CONFIG, + "model": { + "name": "gpt-4o", + "parameters": { + "temperature": 0.2, + "extra_body": {"secret": "value"}, + }, + }, + } + with ( + ctx, + patch.dict(sys.modules, {"langchain_openai": MagicMock(ChatOpenAI=ctor)}), + ): + await create_langchain_messages_handler()(cfg, "q", {}, {}) + assert ctor.call_args.kwargs == {"temperature": 0.2, "model": "gpt-4o"} + + class TestModelSource: @pytest.mark.asyncio async def test_factory_receives_config_and_returned_model_is_used(self) -> None: @@ -2614,6 +2674,7 @@ async def test_default_openai_constructor_receives_parameters(self) -> None: llm = _make_llm("default-openai") ctor = MagicMock(return_value=llm) + ctor.model_fields = _REAL_CHAT_OPENAI.model_fields cfg = { **CONFIG, "model": { @@ -2630,6 +2691,9 @@ async def test_default_openai_constructor_receives_parameters(self) -> None: patch.dict(sys.modules, {"langchain_openai": MagicMock(ChatOpenAI=ctor)}), ): await create_langchain_messages_handler()(cfg, "q", {}, {}) + # ``tools`` is dropped: it is not a ``ChatOpenAI`` constructor field (tools are bound via + # ``bind_tools`` at call time), so the UI offering it must be filtered out rather than + # forwarded or raising. assert ctor.call_args.kwargs == { "temperature": 0.2, "max_tokens": 512, @@ -2643,6 +2707,7 @@ async def test_default_anthropic_constructor_receives_parameters(self) -> None: llm = _make_llm("default-anthropic") ctor = MagicMock(return_value=llm) + ctor.model_fields = _REAL_CHAT_ANTHROPIC.model_fields cfg = { **CONFIG, "provider": {"name": "Anthropic"}, @@ -2673,6 +2738,7 @@ async def test_bedrock_region_is_prepended_to_the_default_constructor( llm = _make_llm("bedrock") ctor = MagicMock(return_value=llm) + ctor.model_fields = _FAKE_BEDROCK_FIELDS cfg = { **CONFIG, "provider": {"name": "Bedrock"}, @@ -2838,3 +2904,110 @@ def factory(config: Any) -> Any: e.get("type") == "TEXT_MESSAGE_CONTENT" and e.get("delta") == "streamed" for e in events ) + + +class TestModelParametersReachTheWire: + """Intercepts the real outgoing HTTP request with an httpx MockTransport, rather than + asserting only on a mock of our own call, so this proves ``top_p`` from ``model.parameters`` + actually leaves the process on the wire the real ``ChatOpenAI`` client builds. + """ + + @pytest.mark.asyncio + async def test_top_p_reaches_the_wire( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + import httpx + from langchain_openai.chat_models import base as _chat_openai_base + + ctx, _rec = _recording() + from launchdarkly_ai_langchain_messages import create_langchain_messages_handler + + captured: dict[str, Any] = {} + + def _handler(request: httpx.Request) -> httpx.Response: + captured["body"] = json.loads(request.content) + return httpx.Response( + 200, + json={ + "id": "chatcmpl-1", + "object": "chat.completion", + "created": 1, + "model": "gpt-4o", + "choices": [ + { + "index": 0, + "message": {"role": "assistant", "content": "hi"}, + "finish_reason": "stop", + } + ], + "usage": { + "prompt_tokens": 1, + "completion_tokens": 1, + "total_tokens": 2, + }, + }, + ) + + transport = httpx.MockTransport(_handler) + mock_client = httpx.AsyncClient( + transport=transport, base_url="https://api.openai.com/v1" + ) + # ``http_async_client`` in ``model.parameters`` is client/connection configuration and is + # never forwarded any more (see ``_CHAT_OPENAI_FORWARDED_KEYS``), so the real ``ChatOpenAI`` + # is intercepted below its own default httpx client builder instead: the same mock transport, + # reached without a config value ever naming an HTTP client. The API key comes from the env + # var ``ChatOpenAI`` already falls back to, for the same reason. + monkeypatch.setattr( + _chat_openai_base, + "_get_default_async_httpx_client", + lambda *a, **kw: mock_client, + ) + monkeypatch.setenv("OPENAI_API_KEY", "test-key") + cfg = { + **CONFIG, + "model": {"name": "gpt-4o", "parameters": {"top_p": 0.5}}, + } + with ctx: + result = await create_langchain_messages_handler()(cfg, "q", {}, {}) + assert captured["body"]["top_p"] == 0.5 + assert result["output"] == "hi" + + +class TestConnectionConfigIsNeverForwarded: + """``api_key``/``base_url`` in ``model.parameters`` are client/connection configuration; a + config author must never be able to redirect a call to a different endpoint or credential. + """ + + @pytest.mark.parametrize( + "provider,fallback", [("openai", "gpt-4o"), ("anthropic", "claude")] + ) + def test_api_key_and_base_url_are_never_forwarded( + self, provider: str, fallback: str + ) -> None: + from launchdarkly_ai_langchain_messages.handler import ( + _CHAT_ANTHROPIC_FORWARDED_KEYS, + _CHAT_OPENAI_FORWARDED_KEYS, + _model_constructor_kwargs, + ) + + forwarded_keys = ( + _CHAT_OPENAI_FORWARDED_KEYS + if provider == "openai" + else _CHAT_ANTHROPIC_FORWARDED_KEYS + ) + cfg = { + **CONFIG, + "provider": {"name": provider}, + "model": { + "name": fallback, + "parameters": { + "api_key": "stolen-key", + "base_url": "https://evil.example.com", + "temperature": 0.3, + }, + }, + } + kwargs = _model_constructor_kwargs(cfg, fallback, forwarded_keys) + assert "api_key" not in kwargs + assert "base_url" not in kwargs + assert kwargs["temperature"] == 0.3 diff --git a/packages/langchain-messages/tests/test_parameter_forwarding.py b/packages/langchain-messages/tests/test_parameter_forwarding.py new file mode 100644 index 00000000..c23bae71 --- /dev/null +++ b/packages/langchain-messages/tests/test_parameter_forwarding.py @@ -0,0 +1,480 @@ +""" +Drift test for the LangChain chat model parameter classification in ``handler.py``. + +``_CHAT_OPENAI_FORWARDED_KEYS`` / ``_CHAT_ANTHROPIC_FORWARDED_KEYS`` / +``_CHAT_BEDROCK_CONVERSE_FORWARDED_KEYS`` are literal, hand-maintained lists. This test reads each +chat model's own ``model_fields`` (field names plus pydantic aliases) and asserts every key it +accepts is classified in exactly one of forwarded, handler-owned, or excluded, so a field nobody has +classified yet fails loudly by name, and so does a list entry that is not a real field. + +``langchain-aws`` is not a dependency of this package (Bedrock support is opt-in), so the +``ChatBedrockConverse`` test skips itself when it is not installed. +""" + +from __future__ import annotations + +import importlib +from collections.abc import Mapping +from typing import Any, ClassVar +from unittest.mock import MagicMock, patch + +import langchain_anthropic +import langchain_openai +import pytest + +from launchdarkly_ai_langchain_messages.handler import ( + _CHAT_ANTHROPIC_EXCLUDED_KEYS, + _CHAT_ANTHROPIC_FORWARDED_KEYS, + _CHAT_ANTHROPIC_OWNED_KEYS, + _CHAT_BEDROCK_CONVERSE_EXCLUDED_KEYS, + _CHAT_BEDROCK_CONVERSE_FORWARDED_KEYS, + _CHAT_BEDROCK_CONVERSE_OWNED_KEYS, + _CHAT_OPENAI_EXCLUDED_KEYS, + _CHAT_OPENAI_FORWARDED_KEYS, + _CHAT_OPENAI_OWNED_KEYS, + _make_default_chat_model, + _model_constructor_kwargs, +) +from tests.forwarding_spec import ( + LANGCHAIN_CHAT_ANTHROPIC, + LANGCHAIN_CHAT_BEDROCK_CONVERSE, + LANGCHAIN_CHAT_OPENAI, + candidate_keys, + probe_forwarded_keys, + sample, +) +from tests.never_forwarded import NEVER_FORWARDED_BAG + + +def _accepted_keys(cls: Any) -> frozenset[str]: + """Every key *cls* (a pydantic model) accepts by construction: each field's own name plus any + string alias it declares.""" + fields: Mapping[str, Any] = cls.model_fields + accepted: set[str] = set() + for name, field in fields.items(): + accepted.add(name) + alias = getattr(field, "alias", None) + if isinstance(alias, str): + accepted.add(alias) + validation_alias = getattr(field, "validation_alias", None) + if isinstance(validation_alias, str): + accepted.add(validation_alias) + else: + choices = getattr(validation_alias, "choices", None) + if choices: + accepted.update(c for c in choices if isinstance(c, str)) + return frozenset(accepted) + + +class TestChatOpenAIAcceptsExactlyTheseKeys: + def test_every_accepted_key_is_classified_exactly_once(self) -> None: + accepted = _accepted_keys(langchain_openai.ChatOpenAI) + classified = ( + _CHAT_OPENAI_FORWARDED_KEYS + | _CHAT_OPENAI_OWNED_KEYS + | _CHAT_OPENAI_EXCLUDED_KEYS + ) + + unclassified = accepted - classified + assert not unclassified, ( + f"ChatOpenAI now accepts {sorted(unclassified)}, not classified as forwarded, " + "handler-owned, or excluded in langchain-messages handler.py" + ) + + overlap = ( + (_CHAT_OPENAI_FORWARDED_KEYS & _CHAT_OPENAI_OWNED_KEYS) + | (_CHAT_OPENAI_FORWARDED_KEYS & _CHAT_OPENAI_EXCLUDED_KEYS) + | (_CHAT_OPENAI_OWNED_KEYS & _CHAT_OPENAI_EXCLUDED_KEYS) + ) + assert not overlap, f"keys classified more than once: {sorted(overlap)}" + + def test_every_classified_key_is_real(self) -> None: + accepted = _accepted_keys(langchain_openai.ChatOpenAI) + stale = ( + _CHAT_OPENAI_FORWARDED_KEYS + | _CHAT_OPENAI_OWNED_KEYS + | _CHAT_OPENAI_EXCLUDED_KEYS + ) - accepted + assert not stale, ( + f"{sorted(stale)} classified in langchain-messages handler.py but " + "ChatOpenAI does not accept them" + ) + + +class TestChatAnthropicAcceptsExactlyTheseKeys: + def test_every_accepted_key_is_classified_exactly_once(self) -> None: + accepted = _accepted_keys(langchain_anthropic.ChatAnthropic) + classified = ( + _CHAT_ANTHROPIC_FORWARDED_KEYS + | _CHAT_ANTHROPIC_OWNED_KEYS + | _CHAT_ANTHROPIC_EXCLUDED_KEYS + ) + + unclassified = accepted - classified + assert not unclassified, ( + f"ChatAnthropic now accepts {sorted(unclassified)}, not classified as forwarded, " + "handler-owned, or excluded in langchain-messages handler.py" + ) + + def test_every_classified_key_is_real(self) -> None: + accepted = _accepted_keys(langchain_anthropic.ChatAnthropic) + stale = ( + _CHAT_ANTHROPIC_FORWARDED_KEYS + | _CHAT_ANTHROPIC_OWNED_KEYS + | _CHAT_ANTHROPIC_EXCLUDED_KEYS + ) - accepted + assert not stale, ( + f"{sorted(stale)} classified in langchain-messages handler.py but " + "ChatAnthropic does not accept them" + ) + + +class TestChatBedrockConverseAcceptsExactlyTheseKeys: + def test_every_accepted_key_is_classified_exactly_once(self) -> None: + lc_aws = pytest.importorskip("langchain_aws") + accepted = _accepted_keys(lc_aws.ChatBedrockConverse) + classified = ( + _CHAT_BEDROCK_CONVERSE_FORWARDED_KEYS + | _CHAT_BEDROCK_CONVERSE_OWNED_KEYS + | _CHAT_BEDROCK_CONVERSE_EXCLUDED_KEYS + ) + + unclassified = accepted - classified + assert not unclassified, ( + f"ChatBedrockConverse now accepts {sorted(unclassified)}, not classified as " + "forwarded, handler-owned, or excluded in langchain-messages handler.py" + ) + + def test_every_classified_key_is_real(self) -> None: + lc_aws = pytest.importorskip("langchain_aws") + accepted = _accepted_keys(lc_aws.ChatBedrockConverse) + stale = ( + _CHAT_BEDROCK_CONVERSE_FORWARDED_KEYS + | _CHAT_BEDROCK_CONVERSE_OWNED_KEYS + | _CHAT_BEDROCK_CONVERSE_EXCLUDED_KEYS + ) - accepted + assert not stale, ( + f"{sorted(stale)} classified in langchain-messages handler.py but " + "ChatBedrockConverse does not accept them" + ) + + +class TestModelFieldAliasesAreNeverForwarded: + """``model_name`` / ``model_id`` are the same constructor field as ``model``. A config value + for them must not be forwarded, or it would collide with the model the handler resolves.""" + + def _config(self, provider: str, parameters: dict[str, Any]) -> Any: + return { + "model": {"name": "configured-model", "parameters": parameters}, + "provider": {"name": provider}, + } + + def test_openai_model_name_is_dropped(self) -> None: + kwargs = _model_constructor_kwargs( + self._config("openai", {"model_name": "other", "temperature": 0.2}), + "fallback", + _CHAT_OPENAI_FORWARDED_KEYS, + ) + assert kwargs == {"model": "configured-model", "temperature": 0.2} + + def test_anthropic_model_name_is_dropped(self) -> None: + kwargs = _model_constructor_kwargs( + self._config("anthropic", {"model_name": "other", "temperature": 0.2}), + "fallback", + _CHAT_ANTHROPIC_FORWARDED_KEYS, + ) + assert kwargs == {"model": "configured-model", "temperature": 0.2} + + def test_bedrock_model_id_is_dropped(self) -> None: + kwargs = _model_constructor_kwargs( + self._config("bedrock", {"model_id": "other", "temperature": 0.2}), + "fallback", + _CHAT_BEDROCK_CONVERSE_FORWARDED_KEYS, + ) + assert kwargs == {"model": "configured-model", "temperature": 0.2} + + +class TestNeverForwardedKeys: + """No credential, endpoint, request-injection, or remote-tool key in ``model.parameters`` + reaches any chat model constructor.""" + + def _config(self, provider: str, parameters: dict[str, Any]) -> Any: + return { + "model": {"name": "configured-model", "parameters": parameters}, + "provider": {"name": provider}, + } + + @pytest.mark.parametrize( + ("provider", "forwarded_keys"), + [ + ("openai", _CHAT_OPENAI_FORWARDED_KEYS), + ("anthropic", _CHAT_ANTHROPIC_FORWARDED_KEYS), + ("bedrock", _CHAT_BEDROCK_CONVERSE_FORWARDED_KEYS), + ], + ) + def test_constructor_kwargs_hold_none_of_them( + self, provider: str, forwarded_keys: frozenset[str] + ) -> None: + kwargs = _model_constructor_kwargs( + self._config(provider, {**NEVER_FORWARDED_BAG, "temperature": 0.2}), + "fallback", + forwarded_keys, + ) + assert kwargs == {"model": "configured-model", "temperature": 0.2} + + +class TestModelKwargsCannotSmuggleRequestKeys: + """``ChatOpenAI`` merges ``model_kwargs`` straight into the request payload, so forwarding it + would carry ``extra_headers``/``extra_query`` past every exclusion.""" + + _SMUGGLED: ClassVar[dict[str, Any]] = { + "model_kwargs": { + "extra_headers": {"X-Smuggled": "1"}, + "extra_query": {"smuggled": "1"}, + } + } + + def _payload(self, **kwargs: Any) -> dict[str, Any]: + from langchain_core.messages import HumanMessage + + model = langchain_openai.ChatOpenAI(api_key="sk-test-not-a-real-key", **kwargs) + payload: dict[str, Any] = model._get_request_payload([HumanMessage("hi")]) + return payload + + def test_model_kwargs_would_reach_the_payload_if_forwarded(self) -> None: + """The control: passed to ``ChatOpenAI`` directly, both keys reach the payload.""" + payload = self._payload(model="gpt-4o", **self._SMUGGLED) + assert payload["extra_headers"] == {"X-Smuggled": "1"} + assert payload["extra_query"] == {"smuggled": "1"} + + def test_forwarded_model_parameters_keep_them_out_of_the_payload(self) -> None: + kwargs = _model_constructor_kwargs( + { + "model": {"name": "gpt-4o", "parameters": dict(self._SMUGGLED)}, + "provider": {"name": "openai"}, + }, + "fallback", + _CHAT_OPENAI_FORWARDED_KEYS, + ) + payload = self._payload(**kwargs) + assert "extra_headers" not in payload + assert "extra_query" not in payload + + +def _probe_config(provider: str, parameters: dict[str, Any]) -> Any: + return { + "model": {"name": "configured-model", "parameters": parameters}, + "provider": {"name": provider}, + } + + +def _build(provider: str, parameters: dict[str, Any]) -> dict[str, Any]: + """Runs ``_make_default_chat_model`` for *provider* against a recording constructor and + returns the kwargs it passed.""" + ctor = MagicMock() + module = MagicMock(ChatOpenAI=ctor, ChatAnthropic=ctor, ChatBedrockConverse=ctor) + importlib = MagicMock() + importlib.import_module.return_value = module + _make_default_chat_model(_probe_config(provider, parameters), importlib) + kwargs: dict[str, Any] = ctor.call_args.kwargs + return kwargs + + +def _accepted_or_empty(module_name: str, cls_name: str) -> frozenset[str]: + try: + module = importlib.import_module(module_name) + except ImportError: + return frozenset() + return _accepted_keys(getattr(module, cls_name)) + + +class TestForwardsExactlyTheCrossSdkList: + """Probes each chat model constructor one key at a time: the keys that change what it is + built with are exactly the cross-SDK list for that class. ``invoke`` and ``stream`` both + build the model through ``_make_default_chat_model``, so this covers both paths.""" + + @pytest.mark.parametrize( + ("provider", "module_name", "cls_name", "expected"), + [ + ( + "openai", + "langchain_openai", + "ChatOpenAI", + LANGCHAIN_CHAT_OPENAI, + ), + ( + "anthropic", + "langchain_anthropic", + "ChatAnthropic", + LANGCHAIN_CHAT_ANTHROPIC, + ), + ( + "bedrock", + "langchain_aws", + "ChatBedrockConverse", + LANGCHAIN_CHAT_BEDROCK_CONVERSE, + ), + ], + ) + async def test_forwarded_keys( + self, provider: str, module_name: str, cls_name: str, expected: frozenset[str] + ) -> None: + baseline = _build(provider, {}) + + async def call(parameters: dict[str, Any]) -> object: + return _build(provider, parameters) + + candidates = candidate_keys(_accepted_or_empty(module_name, cls_name), expected) + assert await probe_forwarded_keys(candidates, call) == expected + assert baseline == {"model": "configured-model"} + + @pytest.mark.parametrize( + ("cls", "keys"), + [ + ( + langchain_openai.ChatOpenAI, + LANGCHAIN_CHAT_OPENAI, + ), + (langchain_anthropic.ChatAnthropic, LANGCHAIN_CHAT_ANTHROPIC), + ], + ) + def test_the_real_class_accepts_every_forwarded_key( + self, cls: Any, keys: frozenset[str] + ) -> None: + for key in sorted(keys): + cls(model="m", api_key="sk-test-not-a-real-key", **{key: sample(key)}) + + +class TestMalformedObjectValuesAreDropped: + @pytest.mark.parametrize( + ("provider", "parameters"), + [ + ("openai", {"logit_bias": "none"}), + ("anthropic", {"thinking": "enabled", "output_config": "json"}), + ("bedrock", {"performance_config": "optimized"}), + ], + ) + def test_dropped(self, provider: str, parameters: dict[str, Any]) -> None: + kwargs = _build(provider, {**parameters, "temperature": 0.2}) + assert kwargs == {"model": "configured-model", "temperature": 0.2} + + +class _Built(Exception): + """Stops a run once the chat model has been built.""" + + +class TestInvokeAndStreamBuildTheSameModel: + """Both paths hand the same config to ``_make_default_chat_model``, so the probe above + covers both.""" + + async def test_same_config_reaches_the_builder(self) -> None: + import launchdarkly_ai_langchain_messages.handler as handler_mod + from launchdarkly_ai_langchain_messages import create_langchain_messages_handler + + config = { + "model": { + "name": "gpt-4o", + "parameters": {**NEVER_FORWARDED_BAG, "temperature": 0.2, "top_p": 0.5}, + }, + "provider": {"name": "openai"}, + "instructions": "help", + } + seen: list[Any] = [] + + def _builder(cfg: Any, *_rest: Any) -> Any: + seen.append(cfg) + raise _Built + + with patch.object(handler_mod, "_make_default_chat_model", _builder): + h = create_langchain_messages_handler() + with pytest.raises(_Built): + await h(config, "q") + with pytest.raises(_Built): + async for _event in await h.stream(config, "q"): + pass + + assert len(seen) == 2 + assert seen[0] == seen[1] + + +class TestMaxTokensToSampleRename: + """``ChatAnthropic`` takes ``max_tokens_to_sample`` as an alias of ``max_tokens``. It is + renamed before forwarding, and ``max_tokens`` wins when both are set.""" + + def test_alias_becomes_max_tokens(self) -> None: + assert _build("anthropic", {"max_tokens_to_sample": 300}) == { + "model": "configured-model", + "max_tokens": 300, + } + + def test_max_tokens_wins_when_both_are_set(self) -> None: + assert _build( + "anthropic", {"max_tokens_to_sample": 300, "max_tokens": 200} + ) == {"model": "configured-model", "max_tokens": 200} + + +class TestBedrockStopRename: + """``ChatBedrockConverse`` takes ``stop`` as the alias of ``stop_sequences`` and rejects a call + that passes both. ``stop`` is renamed before forwarding, and ``stop_sequences`` wins when both + are set.""" + + def test_alias_becomes_stop_sequences(self) -> None: + assert _build("bedrock", {"stop": ["A"]}) == { + "model": "configured-model", + "stop_sequences": ["A"], + } + + def test_stop_sequences_wins_when_both_are_set(self) -> None: + assert _build("bedrock", {"stop": ["A"], "stop_sequences": ["B"]}) == { + "model": "configured-model", + "stop_sequences": ["B"], + } + + def test_the_real_class_accepts_the_forwarded_kwargs(self) -> None: + lc_aws = pytest.importorskip("langchain_aws") + kwargs = _build( + "bedrock", + {"stop": ["A"], "stop_sequences": ["B"], "reasoning_effort": "low"}, + ) + kwargs["model"] = "anthropic.claude-opus-5" + model = lc_aws.ChatBedrockConverse( + region_name="us-east-1", + aws_access_key_id="test-not-a-real-key", + aws_secret_access_key="test-not-a-real-secret", + **kwargs, + ) + assert model.stop_sequences == ["B"] + assert model.additional_model_request_fields == { + "thinking": {"type": "adaptive"}, + "output_config": {"effort": "low"}, + } + + +def test_prompt_cache_key_is_dropped_for_chat_openai() -> None: + """Not supported in Python: ``ChatOpenAI`` has no such field, so a config value never reaches + the constructor.""" + assert _build("openai", {"prompt_cache_key": "cache-1", "temperature": 0.2}) == { + "model": "configured-model", + "temperature": 0.2, + } + + +def test_reasoning_effort_keeps_chat_openai_on_chat_completions() -> None: + """``reasoning_effort`` is the Chat Completions field. Unlike ``reasoning`` it does not switch + ``ChatOpenAI`` to the Responses API, so the handler still reads a Chat Completions reply.""" + from langchain_core.messages import HumanMessage + + kwargs = _model_constructor_kwargs( + { + "model": {"name": "gpt-4o", "parameters": {"reasoning_effort": "low"}}, + "provider": {"name": "openai"}, + }, + "fallback", + _CHAT_OPENAI_FORWARDED_KEYS, + ) + model = langchain_openai.ChatOpenAI(api_key="sk-test-not-a-real-key", **kwargs) + payload: dict[str, Any] = model._get_request_payload([HumanMessage("hi")]) + assert payload["reasoning_effort"] == "low" + assert "messages" in payload + assert "input" not in payload + assert "reasoning" not in payload diff --git a/packages/litellm-agents/src/launchdarkly_ai_litellm_agents/handler.py b/packages/litellm-agents/src/launchdarkly_ai_litellm_agents/handler.py index 0505082f..44588d2f 100644 --- a/packages/litellm-agents/src/launchdarkly_ai_litellm_agents/handler.py +++ b/packages/litellm-agents/src/launchdarkly_ai_litellm_agents/handler.py @@ -3,7 +3,7 @@ import asyncio import inspect import json -from collections.abc import AsyncGenerator, Callable +from collections.abc import AsyncGenerator, Callable, Mapping from typing import Any from agents import ModelSettings, RunConfig @@ -35,22 +35,67 @@ text_message, ) from launchdarkly_ai_server.lifecycle import effective_capture_content +from launchdarkly_ai_server.parameter_forwarding import select_forwarded_parameters +from launchdarkly_ai_server.utils import model_parameters from ._version import PACKAGE_NAME, __version__ ModelFactory = Callable[[str, dict[str, Any]], Any] -_MODEL_SETTING_FIELDS = { - "temperature", - "top_p", - "frequency_penalty", - "presence_penalty", - "tool_choice", - "parallel_tool_calls", - "max_tokens", - "reasoning", - "verbosity", - "store", -} + +#: Every field ``agents.ModelSettings`` declares, classified by hand into exactly one of: forwarded +#: (below) or excluded. These are the fields ``LitellmModel`` actually sends to +#: ``litellm.acompletion``. ``max_turns`` is not a ``ModelSettings`` field: it is read separately +#: for ``Runner.run``. ``TestModelSettingsAcceptsExactlyTheseFields`` in this package's tests +#: asserts this classification stays exhaustive as the SDK's own dataclass changes. +_MODEL_SETTINGS_FORWARDED_KEYS = frozenset( + { + "frequency_penalty", + "max_tokens", + "parallel_tool_calls", + "presence_penalty", + "reasoning", + "temperature", + "tool_choice", + "top_p", + } +) + +#: Forwarded keys whose value must be an object. A config value of any other type is malformed +#: and dropped rather than passed to ``ModelSettings``. +_MODEL_SETTINGS_MAPPING_KEYS = frozenset({"reasoning"}) + +#: Declared by ``ModelSettings`` but never forwarded, and why: +#: * ``verbosity``: ``LitellmModel`` never reads it, so it would be silently dropped. +#: * ``context_management``, ``truncation``: server-side state. +#: * ``store``, ``prompt_cache_retention``: data retention. +#: * ``metadata``: identity and attribution. ``LitellmModel`` passes it to ``acompletion``. +#: * ``include_usage``: runtime wiring. It changes how usage is reported back to the handler. +#: * ``response_include``, ``top_logprobs``: API-shape switch. They add output the handler does not +#: read. +#: * ``retry``: client/connection configuration. +#: * ``extra_headers``, ``extra_query``, ``extra_body``, ``extra_args``: raw request overrides. +#: ``extra_args`` is merged straight into the ``acompletion`` call, where it would reach +#: ``api_base``, ``mock_response`` and the rest. +#: +#: Named for the drift test and for review, not read at runtime. +_MODEL_SETTINGS_EXCLUDED_KEYS = frozenset( + { + "verbosity", + "context_management", + "truncation", + "store", + "prompt_cache_retention", + "metadata", + "include_usage", + "response_include", + "top_logprobs", + "retry", + "extra_headers", + "extra_query", + "extra_body", + "extra_args", + } +) _RUN_CONFIG = RunConfig(tracing_disabled=True) @@ -97,34 +142,16 @@ def validate_json(self, json_str: str) -> Any: return json.loads(json_str) -# AI Configs are authored in snake_case, but a caller may also pass camelCase (spec §2.x.1: -# "either spelling is read"). snake_case wins when a bag sets both. -_CAMEL_TO_SNAKE_FIELDS = { - "topP": "top_p", - "maxTurns": "max_turns", - "frequencyPenalty": "frequency_penalty", - "presencePenalty": "presence_penalty", - "toolChoice": "tool_choice", - "parallelToolCalls": "parallel_tool_calls", - "maxTokens": "max_tokens", -} - - -def _normalized_parameters(raw: dict[str, Any]) -> dict[str, Any]: - result = dict(raw) - for camel, snake in _CAMEL_TO_SNAKE_FIELDS.items(): - if camel in result: - value = result.pop(camel) - result.setdefault(snake, value) - return result - - def _model_config(config_value: AiConfigRep) -> tuple[str, dict[str, Any]]: + """The model name and the forwarded ``model.parameters``: the ``ModelSettings`` keys plus + ``max_turns`` for the run. The handler and the native graph both call this, so they forward + the same keys.""" model = config_value.get("model") or {} - raw = model.get("parameters") - allowed = _MODEL_SETTING_FIELDS | {"max_turns"} - normalized = _normalized_parameters(raw) if isinstance(raw, dict) else {} - parameters = {key: value for key, value in normalized.items() if key in allowed} + parameters = select_forwarded_parameters( + model_parameters(config_value), + _MODEL_SETTINGS_FORWARDED_KEYS | {"max_turns"}, + mapping_keys=_MODEL_SETTINGS_MAPPING_KEYS, + ) return str(model.get("name") or ""), parameters @@ -141,20 +168,19 @@ def _litellm_model( def _model_settings(parameters: dict[str, Any]) -> ModelSettings: - settings = { - key: value for key, value in parameters.items() if key in _MODEL_SETTING_FIELDS - } - # Only ``effort`` and ``summary`` reach the model, as in the TypeScript SDK; any other - # key, or a non-object value, is dropped rather than failing ModelSettings validation. + """Builds ``ModelSettings`` from the forwarded parameters. + + ``reasoning`` keeps only ``effort``, rebuilt as the SDK's ``Reasoning`` type: ``LitellmModel`` + sends it as ``reasoning_effort`` and logs a warning on every call that also sets ``summary``. + A ``reasoning`` with no ``effort`` is dropped. ``tool_choice`` is forwarded only as a string; + ``ModelSettings`` rejects an object value while the Agent is built, so one is dropped. + """ + settings = select_forwarded_parameters(parameters, _MODEL_SETTINGS_FORWARDED_KEYS) reasoning = settings.pop("reasoning", None) - if isinstance(reasoning, dict): - picked = { - k: reasoning[k] - for k in ("effort", "summary") - if reasoning.get(k) is not None - } - if picked: - settings["reasoning"] = Reasoning(**picked) + if isinstance(reasoning, Mapping) and reasoning.get("effort") is not None: + settings["reasoning"] = Reasoning(effort=reasoning["effort"]) + if not isinstance(settings.get("tool_choice", ""), str): + del settings["tool_choice"] return ModelSettings(**settings) diff --git a/packages/litellm-agents/src/launchdarkly_ai_litellm_agents/native_graph.py b/packages/litellm-agents/src/launchdarkly_ai_litellm_agents/native_graph.py index 1066470a..ce091168 100644 --- a/packages/litellm-agents/src/launchdarkly_ai_litellm_agents/native_graph.py +++ b/packages/litellm-agents/src/launchdarkly_ai_litellm_agents/native_graph.py @@ -246,11 +246,15 @@ async def on_handoff( async def on_agent_start(self, context: Any, agent: Any) -> None: _enter(agent.name) + # Only the root node's `max_turns` applies. The whole graph is one `Runner.run`, and + # the Agents SDK counts turns once for that run, across every handoff. + root_max_turns = _model_config(graph.root.config)[1].get("max_turns") result = await agents.Runner.run( built[graph.root.key], runner_input, hooks=_LDHooks(), run_config=RunConfig(tracing_disabled=True), + **({"max_turns": root_max_turns} if root_max_turns is not None else {}), ) if span: span.set_status(SpanStatusCode.OK) diff --git a/packages/litellm-agents/tests/test_graph.py b/packages/litellm-agents/tests/test_graph.py index 59e63794..bb45bb61 100644 --- a/packages/litellm-agents/tests/test_graph.py +++ b/packages/litellm-agents/tests/test_graph.py @@ -14,6 +14,12 @@ import launchdarkly_ai_litellm_agents.native_graph as native_graph_mod from launchdarkly_ai_litellm_agents import litellm_graph, to_litellm_agents from launchdarkly_ai_server import GraphDefinition, GraphEdge, GraphNode +from tests.forwarding_spec import ( + LITELLM_AGENTS, + candidate_keys, + probe_forwarded_keys, +) +from tests.never_forwarded import NEVER_FORWARDED_BAG, find_leaks class _FakeRunHooks: @@ -27,7 +33,9 @@ async def on_agent_start(self, *args: Any) -> None: return None -def _definition(*, enabled: bool = True) -> GraphDefinition: +def _definition( + *, enabled: bool = True, root_parameters: dict[str, Any] | None = None +) -> GraphDefinition: edge = GraphEdge( key="root-child", source_key="root", @@ -49,7 +57,14 @@ def _definition(*, enabled: bool = True) -> GraphDefinition: key="root", config={ "provider": {"name": "Anthropic"}, - "model": {"name": "anthropic/claude-sonnet-4"}, + "model": { + "name": "anthropic/claude-sonnet-4", + **( + {"parameters": root_parameters} + if root_parameters is not None + else {} + ), + }, "instructions": "root", }, meta={}, @@ -613,3 +628,82 @@ def broken_factory(*_: Any) -> Any: ("$ld:ai:graph:invocation_failure", "litellm-graph") ] run.assert_not_awaited() + + +class TestNativeGraphForwarding: + """The native graph builds each node's ``ModelSettings`` with the handler's own list, and + reads the root's ``max_turns`` for the run, so a probe of the root node finds exactly the + LiteLLM Agents list.""" + + @staticmethod + async def _run(parameters: dict[str, Any]) -> tuple[Any, Any, Any]: + factory_parameters: dict[str, dict[str, Any]] = {} + created: dict[str, dict[str, Any]] = {} + + def model_factory(name: str, params: dict[str, Any]) -> object: + factory_parameters[name] = params + return object() + + def agent(**kwargs: Any) -> Any: + created[kwargs["name"]] = kwargs + return SimpleNamespace(**kwargs) + + runner = SimpleNamespace( + run=AsyncMock( + return_value=SimpleNamespace( + final_output="done", + context_wrapper=SimpleNamespace( + usage=SimpleNamespace(input_tokens=1, output_tokens=1) + ), + ) + ) + ) + agents = SimpleNamespace( + Agent=agent, + Runner=runner, + handoff=MagicMock(side_effect=lambda target: target), + FunctionTool=MagicMock(), + RunHooks=_FakeRunHooks, + ) + real_import = __import__ + with patch( + "importlib.import_module", + side_effect=lambda name: agents if name == "agents" else real_import(name), + ): + await to_litellm_agents( + _definition_awaitable_from(_definition(root_parameters=parameters)), + opts={"model_factory": model_factory}, + ).invoke("hello") + run_kwargs = dict(runner.run.call_args.kwargs) + run_kwargs.pop("hooks", None) + run_kwargs.pop("run_config", None) + return ( + factory_parameters["anthropic/claude-sonnet-4"], + created["root"]["model_settings"], + run_kwargs, + ) + + async def test_forwards_exactly_the_litellm_agents_list(self) -> None: + import dataclasses + + from agents import ModelSettings + + candidates = candidate_keys( + (f.name for f in dataclasses.fields(ModelSettings)), LITELLM_AGENTS + ) + forwarded = await probe_forwarded_keys( + candidates, self._run, overrides={"tool_choice": "required"} + ) + assert forwarded == LITELLM_AGENTS + + async def test_root_max_turns_applies_to_the_run(self) -> None: + _, _, run_kwargs = await self._run({"max_turns": 6}) + assert run_kwargs["max_turns"] == 6 + + async def test_forwards_no_never_forwarded_key(self) -> None: + factory_parameters, settings, run_kwargs = await self._run( + {**NEVER_FORWARDED_BAG, "temperature": 0.1} + ) + assert factory_parameters == {"temperature": 0.1} + assert settings.temperature == 0.1 + assert not find_leaks((settings, run_kwargs)) diff --git a/packages/litellm-agents/tests/test_handler.py b/packages/litellm-agents/tests/test_handler.py index 34a3db1f..9a2acc7a 100644 --- a/packages/litellm-agents/tests/test_handler.py +++ b/packages/litellm-agents/tests/test_handler.py @@ -15,6 +15,12 @@ create_litellm_agents_handler, litellm_agents, ) +from tests.forwarding_spec import ( + LITELLM_AGENTS, + candidate_keys, + probe_forwarded_keys, +) +from tests.never_forwarded import NEVER_FORWARDED_BAG, find_leaks CONFIG: dict[str, Any] = { "provider": {"name": "Anthropic"}, @@ -305,47 +311,56 @@ async def test_runs_with_agents_sdk_tracing_disabled(self) -> None: is True ) - def test_model_settings_keep_the_shared_allowlist_and_type_reasoning( - self, - ) -> None: + def test_model_settings_keep_the_allowlist_and_type_reasoning(self) -> None: settings = handler_mod._model_settings( { "temperature": 0.1, "verbosity": "low", + "store": True, "reasoning": {"effort": "high", "summary": "auto", "smuggled": 1}, "metadata": {"team": "a"}, "include_usage": True, "top_logprobs": 2, } ) - assert isinstance(settings.reasoning, Reasoning) - assert settings.reasoning.effort == "high" - assert settings.reasoning.summary == "auto" - assert settings.verbosity == "low" + assert settings.reasoning == Reasoning(effort="high") assert settings.temperature == 0.1 + assert settings.verbosity is None + assert settings.store is None assert settings.metadata is None assert settings.include_usage is None assert settings.top_logprobs is None - @pytest.mark.parametrize("reasoning", ["high", {"smuggled": 1}, None]) + @pytest.mark.parametrize( + "reasoning", ["high", {"smuggled": 1}, {"summary": "auto"}, None] + ) def test_unusable_reasoning_is_dropped(self, reasoning: Any) -> None: - assert handler_mod._model_settings({"reasoning": reasoning}).reasoning is None + _, parameters = handler_mod._model_config( + {"model": {"name": "m", "parameters": {"reasoning": reasoning}}} + ) + assert handler_mod._model_settings(parameters).reasoning is None + + def test_string_tool_choice_is_forwarded(self) -> None: + settings = handler_mod._model_settings({"tool_choice": "required"}) + assert settings.tool_choice == "required" - def test_model_config_reads_either_spelling_and_snake_case_wins(self) -> None: + def test_object_tool_choice_is_dropped_without_failing(self) -> None: + settings = handler_mod._model_settings( + {"tool_choice": {"type": "auto"}, "temperature": 0.1} + ) + assert settings.tool_choice is None + assert settings.temperature == 0.1 + + def test_camel_case_spellings_are_not_read(self) -> None: config = { **CONFIG, "model": { **CONFIG["model"], - "parameters": { - "topP": 0.4, - "maxTurns": 5, - "top_p": 0.9, - }, + "parameters": {"topP": 0.4, "maxTurns": 5, "top_p": 0.9}, }, } _, parameters = handler_mod._model_config(config) - assert parameters["top_p"] == 0.9 - assert parameters["max_turns"] == 5 + assert parameters == {"top_p": 0.9} async def test_history_uses_agents_sdk_structured_input(self) -> None: run = AsyncMock( @@ -594,3 +609,123 @@ def test_forwards_handler_variables_and_capture_content(self) -> None: instance.invoke.assert_called_once_with( "hello", {"kind": "user", "key": "u"}, {"name": "Ada"} ) + + +#: ``LitellmModel`` takes ``tool_choice`` only as a string, so the probe uses one. +_SAMPLE_OVERRIDES = {"tool_choice": "required"} + + +def _probe_candidates() -> frozenset[str]: + import dataclasses + + from agents import ModelSettings + + return candidate_keys( + (f.name for f in dataclasses.fields(ModelSettings)), LITELLM_AGENTS + ) + + +def _probe_config(parameters: dict[str, Any]) -> dict[str, Any]: + return {**CONFIG, "model": {**CONFIG["model"], "parameters": parameters}} + + +class _Captured: + """What one handler call handed the SDK from the config: the factory's parameters, the + ``ModelSettings``, and the ``Runner`` call's keyword arguments.""" + + def __init__(self) -> None: + self.factory_parameters: list[dict[str, Any]] = [] + + def factory(self, _name: str, parameters: dict[str, Any]) -> object: + self.factory_parameters.append(parameters) + return object() + + def result(self, runner_call: Any) -> object: + kwargs = dict(runner_call.call_args.kwargs) + kwargs.pop("hooks", None) + kwargs.pop("run_config", None) + return ( + self.factory_parameters[-1], + FakeAgent.created[-1]["model_settings"], + kwargs, + ) + + +class TestNeverForwardedParameters: + """No credential, endpoint, routing, or request-injection key in ``model.parameters`` reaches + the model factory, ``ModelSettings`` or the ``Runner`` call.""" + + def setup_method(self) -> None: + FakeAgent.created.clear() + + async def test_invoke_forwards_none_of_them(self) -> None: + captured = _Captured() + agents = _agents_module() + with _patch_agents(agents): + await create_litellm_agents_handler(model_factory=captured.factory)( + _probe_config({**NEVER_FORWARDED_BAG, "temperature": 0.1}), "q" + ) + factory_parameters, settings, run_kwargs = captured.result(agents.Runner.run) + assert factory_parameters == {"temperature": 0.1} + assert settings.temperature == 0.1 + assert not find_leaks((settings, run_kwargs)) + + async def test_stream_forwards_none_of_them(self) -> None: + captured = _Captured() + agents = _agents_module() + with _patch_agents(agents): + handler = create_litellm_agents_handler(model_factory=captured.factory) + [ + event + async for event in await handler.stream( + _probe_config({**NEVER_FORWARDED_BAG, "temperature": 0.1}), "q" + ) + ] + factory_parameters, settings, run_kwargs = captured.result( + agents.Runner.run_streamed + ) + assert factory_parameters == {"temperature": 0.1} + assert settings.temperature == 0.1 + assert not find_leaks((settings, run_kwargs)) + + +class TestForwardsExactlyTheCrossSdkList: + """Probes ``invoke`` and ``stream`` one key at a time: the keys that change what the model + factory, ``ModelSettings`` or the ``Runner`` call receive are exactly the LiteLLM Agents + list, on both paths.""" + + async def test_invoke(self) -> None: + async def call(parameters: dict[str, Any]) -> object: + FakeAgent.created.clear() + captured = _Captured() + agents = _agents_module() + with _patch_agents(agents): + await create_litellm_agents_handler(model_factory=captured.factory)( + _probe_config(parameters), "q" + ) + return captured.result(agents.Runner.run) + + forwarded = await probe_forwarded_keys( + _probe_candidates(), call, overrides=_SAMPLE_OVERRIDES + ) + assert forwarded == LITELLM_AGENTS + + async def test_stream(self) -> None: + async def call(parameters: dict[str, Any]) -> object: + FakeAgent.created.clear() + captured = _Captured() + agents = _agents_module() + with _patch_agents(agents): + handler = create_litellm_agents_handler(model_factory=captured.factory) + [ + event + async for event in await handler.stream( + _probe_config(parameters), "q" + ) + ] + return captured.result(agents.Runner.run_streamed) + + forwarded = await probe_forwarded_keys( + _probe_candidates(), call, overrides=_SAMPLE_OVERRIDES + ) + assert forwarded == LITELLM_AGENTS diff --git a/packages/litellm-agents/tests/test_parameter_forwarding.py b/packages/litellm-agents/tests/test_parameter_forwarding.py new file mode 100644 index 00000000..6bd3dad5 --- /dev/null +++ b/packages/litellm-agents/tests/test_parameter_forwarding.py @@ -0,0 +1,57 @@ +""" +Drift test for the ``agents.ModelSettings`` parameter classification in ``handler.py``. + +``_MODEL_SETTINGS_FORWARDED_KEYS`` is a literal, hand-maintained list. This test reads +``ModelSettings``'s own dataclass fields and asserts every field it declares is classified in +exactly one of forwarded or excluded, so an SDK field nobody has classified yet fails loudly by +name, and so does a list entry that is not a real SDK field. It also checks every forwarded field +is one ``LitellmModel`` actually reads, so a key it would silently drop is not on the list. +""" + +from __future__ import annotations + +import dataclasses +import inspect +import re + +from agents import ModelSettings +from agents.extensions.models import litellm_model + +from launchdarkly_ai_litellm_agents.handler import ( + _MODEL_SETTINGS_EXCLUDED_KEYS, + _MODEL_SETTINGS_FORWARDED_KEYS, +) + +_FIELDS = frozenset(f.name for f in dataclasses.fields(ModelSettings)) + + +class TestModelSettingsAcceptsExactlyTheseFields: + def test_every_field_is_classified_exactly_once(self) -> None: + classified = _MODEL_SETTINGS_FORWARDED_KEYS | _MODEL_SETTINGS_EXCLUDED_KEYS + + unclassified = _FIELDS - classified + assert not unclassified, ( + f"ModelSettings now declares {sorted(unclassified)}, not classified as " + "forwarded or excluded in litellm-agents handler.py" + ) + + overlap = _MODEL_SETTINGS_FORWARDED_KEYS & _MODEL_SETTINGS_EXCLUDED_KEYS + assert not overlap, f"fields classified more than once: {sorted(overlap)}" + + def test_every_classified_field_is_real(self) -> None: + stale = ( + _MODEL_SETTINGS_FORWARDED_KEYS | _MODEL_SETTINGS_EXCLUDED_KEYS + ) - _FIELDS + assert not stale, ( + f"{sorted(stale)} classified in litellm-agents handler.py but " + "ModelSettings does not declare them" + ) + + def test_every_forwarded_field_is_read_by_litellm_model(self) -> None: + source = inspect.getsource(litellm_model) + read = frozenset(re.findall(r"model_settings\.(\w+)", source)) + unread = _MODEL_SETTINGS_FORWARDED_KEYS - read + assert not unread, ( + f"{sorted(unread)} forwarded in litellm-agents handler.py but LitellmModel " + "never reads them" + ) diff --git a/packages/litellm-messages/src/launchdarkly_ai_litellm_messages/handler.py b/packages/litellm-messages/src/launchdarkly_ai_litellm_messages/handler.py index bd9d6321..6ded1a1b 100644 --- a/packages/litellm-messages/src/launchdarkly_ai_litellm_messages/handler.py +++ b/packages/litellm-messages/src/launchdarkly_ai_litellm_messages/handler.py @@ -31,6 +31,8 @@ to_semconv_finish_reason, ) from launchdarkly_ai_server.lifecycle import effective_capture_content +from launchdarkly_ai_server.parameter_forwarding import select_forwarded_parameters +from launchdarkly_ai_server.utils import model_parameters from ._version import PACKAGE_NAME, __version__ @@ -44,10 +46,13 @@ def _json_schema(output_format: dict[str, Any]) -> dict[str, Any]: return schema -# Generation settings only. Connection and credential keys (api_base, api_key, -# base_url, extra_headers, aws_*, vertex_*, ...) stay out: LiteLLM reads them -# as call arguments, and an AI Config must not choose where the provider key goes. -_FORWARDED_PARAMETERS = frozenset( +#: The cross-SDK list for LiteLLM Messages (TESTING.md section 1.12): the ``model.parameters`` +#: keys forwarded to ``litellm.acompletion``. Every other key is dropped. The handler sets +#: ``model``, ``messages``, ``stream``, ``stream_options``, ``tools`` and ``response_format`` itself, +#: and ``api_key`` (with ``api_base``) comes from the handler's options, never from the config. +#: ``TestAcompletionAcceptsExactlyTheseKeys`` in this package's tests asserts every named parameter +#: of ``acompletion`` is classified as forwarded, handler-owned or excluded. +_COMPLETION_FORWARDED_KEYS = frozenset( { "frequency_penalty", "max_completion_tokens", @@ -62,6 +67,60 @@ def _json_schema(output_format: dict[str, Any]) -> dict[str, Any]: "top_p", } ) + +#: Accepted by ``litellm.acompletion`` but never forwarded, and why: +#: * ``store``: data retention. +#: * ``user``, ``safety_identifier``: identity and attribution. +#: * ``timeout``, ``base_url``, ``api_version``, ``deployment_id``, ``model_list``, +#: ``shared_session``: credentials, endpoints and connection settings. ``shared_session`` is a +#: live HTTP session object. +#: * ``extra_headers``: raw request injection. +#: * ``functions``, ``function_call``: the deprecated spelling of ``tools``, which the handler owns. +#: * ``enable_json_schema_validation``: runtime wiring. +#: * ``n``: switches the response shape. The handler reads only ``choices[0]``, and a streamed +#: ``n > 1`` interleaves the choices. +#: * ``logprobs``, ``top_logprobs``, ``modalities``, ``audio``, ``prediction``, +#: ``include_server_side_tool_invocations``: API-shape switches. They add or change output the +#: handler does not read. +#: * ``web_search_options``: a remote, server-side tool. +#: * ``thinking``, ``logit_bias``, ``service_tier``, ``prompt_cache_key``, ``verbosity``: model +#: settings LiteLLM translates per backing provider and may reject for one that lacks them. Left +#: off until each is confirmed safe across the providers LiteLLM fronts. +#: +#: Everything ``acompletion`` reads only through ``**kwargs`` (``api_base``, +#: ``custom_llm_provider``, ``fallbacks``, ``num_retries``, ``mock_response``, ``aws_*``, +#: ``vertex_*``, ...) is dropped the same way. Named for the drift test and for review, not read at +#: runtime. +_COMPLETION_EXCLUDED_KEYS = frozenset( + { + "store", + "user", + "safety_identifier", + "timeout", + "base_url", + "api_version", + "deployment_id", + "model_list", + "shared_session", + "extra_headers", + "functions", + "function_call", + "enable_json_schema_validation", + "n", + "logprobs", + "top_logprobs", + "modalities", + "audio", + "prediction", + "include_server_side_tool_invocations", + "web_search_options", + "thinking", + "logit_bias", + "service_tier", + "prompt_cache_key", + "verbosity", + } +) _MAX_TOOL_TURNS = 10 @@ -186,16 +245,8 @@ def _request( base_url: str | None = None, api_key: str | None = None, ) -> dict[str, Any]: - model = config_value.get("model") or {} - raw_parameters = model.get("parameters") - parameters = ( - { - key: value - for key, value in raw_parameters.items() - if key in _FORWARDED_PARAMETERS - } - if isinstance(raw_parameters, dict) - else {} + parameters = select_forwarded_parameters( + model_parameters(config_value), _COMPLETION_FORWARDED_KEYS ) parameters.update( { @@ -204,7 +255,7 @@ def _request( "stream": stream, } ) - # Connection settings come from the handler, never from model.parameters (see above). + # Connection settings come from the handler, never from model.parameters. if base_url is not None: parameters["api_base"] = base_url if api_key is not None: diff --git a/packages/litellm-messages/tests/test_handler.py b/packages/litellm-messages/tests/test_handler.py index cb8a9e76..5f99d772 100644 --- a/packages/litellm-messages/tests/test_handler.py +++ b/packages/litellm-messages/tests/test_handler.py @@ -19,6 +19,12 @@ create_litellm_messages_handler, litellm_messages, ) +from tests.forwarding_spec import ( + LITELLM_MESSAGES, + candidate_keys, + probe_forwarded_keys, +) +from tests.never_forwarded import NEVER_FORWARDED_BAG, find_leaks CONFIG: dict[str, Any] = { "provider": {"name": "Anthropic"}, @@ -660,3 +666,79 @@ def test_convenience_wrapper_forwards_all_public_arguments(self) -> None: instance.invoke.assert_called_once_with( "hello", {"kind": "user", "key": "u"}, {"name": "Ada"} ) + + +class TestNeverForwardedParameters: + """No credential, endpoint, routing, or request-injection key in ``model.parameters`` reaches + ``litellm.acompletion``, on invoke or stream.""" + + @staticmethod + def _config() -> dict[str, Any]: + return { + **CONFIG, + "model": { + **CONFIG["model"], + "parameters": {**NEVER_FORWARDED_BAG, "temperature": 0.1}, + }, + } + + async def test_invoke_forwards_none_of_them(self) -> None: + completion = AsyncMock(return_value=_response()) + await create_litellm_messages_handler(completion=completion)( + self._config(), "q" + ) + request = completion.call_args.kwargs + assert request["temperature"] == 0.1 + assert not find_leaks(request) + + async def test_stream_forwards_none_of_them(self) -> None: + completion = AsyncMock(return_value=FakeStream([_chunk(text="hi")])) + await _events( + await create_litellm_messages_handler(completion=completion).stream( + self._config(), "q" + ) + ) + request = completion.call_args.kwargs + assert request["temperature"] == 0.1 + assert not find_leaks(request) + + +class TestForwardsExactlyTheCrossSdkList: + """Probes ``invoke`` and ``stream`` one key at a time: the keys that change the + ``litellm.acompletion`` call are exactly the cross-SDK LiteLLM Messages list, on both paths.""" + + @staticmethod + def _candidates() -> frozenset[str]: + import inspect + + import litellm + + return candidate_keys( + inspect.signature(litellm.acompletion).parameters, LITELLM_MESSAGES + ) + + @staticmethod + def _config(parameters: dict[str, Any]) -> dict[str, Any]: + return {**CONFIG, "model": {**CONFIG["model"], "parameters": parameters}} + + async def test_invoke(self) -> None: + async def call(parameters: dict[str, Any]) -> object: + completion = AsyncMock(return_value=_response()) + await create_litellm_messages_handler(completion=completion)( + self._config(parameters), "q" + ) + return completion.call_args.kwargs + + assert await probe_forwarded_keys(self._candidates(), call) == LITELLM_MESSAGES + + async def test_stream(self) -> None: + async def call(parameters: dict[str, Any]) -> object: + completion = AsyncMock(return_value=FakeStream([_chunk(text="hi")])) + await _events( + await create_litellm_messages_handler(completion=completion).stream( + self._config(parameters), "q" + ) + ) + return completion.call_args.kwargs + + assert await probe_forwarded_keys(self._candidates(), call) == LITELLM_MESSAGES diff --git a/packages/litellm-messages/tests/test_parameter_forwarding.py b/packages/litellm-messages/tests/test_parameter_forwarding.py new file mode 100644 index 00000000..640cacfb --- /dev/null +++ b/packages/litellm-messages/tests/test_parameter_forwarding.py @@ -0,0 +1,71 @@ +""" +Drift test for the ``litellm.acompletion`` parameter classification in ``handler.py``. + +``_COMPLETION_FORWARDED_KEYS`` is a literal, hand-maintained list. This test reads +``acompletion``'s own signature and asserts every named parameter it accepts is classified in +exactly one of forwarded, handler-owned, or excluded, so a parameter nobody has classified yet fails +loudly by name, and so does a list entry that is not a real parameter. ``acompletion`` also takes +``**kwargs``; nothing reaches it that way, because the forwarded list is the only way in. +""" + +from __future__ import annotations + +import inspect + +import litellm + +from launchdarkly_ai_litellm_messages.handler import ( + _COMPLETION_EXCLUDED_KEYS, + _COMPLETION_FORWARDED_KEYS, +) + +#: Handler-owned: set by the call site itself, never taken from the config. +_OWNED_KEYS = frozenset( + { + "model", + "messages", + "stream", + "stream_options", + "tools", + "response_format", + "api_key", + } +) + + +def _accepted_keys() -> frozenset[str]: + return frozenset( + name + for name, param in inspect.signature(litellm.acompletion).parameters.items() + if param.kind + not in (inspect.Parameter.VAR_POSITIONAL, inspect.Parameter.VAR_KEYWORD) + ) + + +class TestAcompletionAcceptsExactlyTheseKeys: + def test_every_accepted_key_is_classified_exactly_once(self) -> None: + classified = ( + _COMPLETION_FORWARDED_KEYS | _OWNED_KEYS | _COMPLETION_EXCLUDED_KEYS + ) + + unclassified = _accepted_keys() - classified + assert not unclassified, ( + f"litellm.acompletion now accepts {sorted(unclassified)}, not classified as " + "forwarded, handler-owned, or excluded in litellm-messages handler.py" + ) + + overlap = ( + (_COMPLETION_FORWARDED_KEYS & _OWNED_KEYS) + | (_COMPLETION_FORWARDED_KEYS & _COMPLETION_EXCLUDED_KEYS) + | (_OWNED_KEYS & _COMPLETION_EXCLUDED_KEYS) + ) + assert not overlap, f"keys classified more than once: {sorted(overlap)}" + + def test_every_classified_key_is_a_real_parameter(self) -> None: + stale = ( + _COMPLETION_FORWARDED_KEYS | _OWNED_KEYS | _COMPLETION_EXCLUDED_KEYS + ) - _accepted_keys() + assert not stale, ( + f"{sorted(stale)} classified in litellm-messages handler.py but " + "litellm.acompletion does not accept them" + ) diff --git a/packages/openai-agents/src/launchdarkly_ai_openai_agents/handler.py b/packages/openai-agents/src/launchdarkly_ai_openai_agents/handler.py index 7d7445d4..2b5db244 100644 --- a/packages/openai-agents/src/launchdarkly_ai_openai_agents/handler.py +++ b/packages/openai-agents/src/launchdarkly_ai_openai_agents/handler.py @@ -21,7 +21,7 @@ import asyncio import inspect import json -from collections.abc import AsyncGenerator +from collections.abc import AsyncGenerator, Mapping from typing import Any from launchdarkly_ai_server import ( @@ -48,7 +48,9 @@ ) from launchdarkly_ai_server.agui import TurnEmitter from launchdarkly_ai_server.lifecycle import effective_capture_content +from launchdarkly_ai_server.parameter_forwarding import select_forwarded_parameters from launchdarkly_ai_server.responses import map_responses_event +from launchdarkly_ai_server.utils import model_parameters from ._version import PACKAGE_NAME, __version__ from .spans import ( @@ -78,6 +80,110 @@ except ImportError: # pragma: no cover - `agents` is a hard dependency of this package _RunHooksBase = object # type: ignore[assignment,misc] +#: Every field ``agents.ModelSettings`` declares, classified by hand into exactly one of: forwarded +#: (below) or excluded. ``ModelSettings`` has no handler-owned fields: ``model`` and ``max_turns`` +#: live outside it (on ``Agent``/``Runner.run``; ``max_turns`` is read separately for the run). +#: ``TestModelSettingsAcceptsExactlyTheseFields`` in this package's tests asserts this +#: classification stays exhaustive as the SDK's own dataclass changes. +#: +#: This is the cross-SDK list for OpenAI Agents (TESTING.md section 1.12), less the two keys that +#: are not ``ModelSettings`` fields: ``max_turns`` goes to the run, and ``text`` is read only for +#: ``text.verbosity``, which becomes ``verbosity`` (see :func:`_apply_text_verbosity`). +_MODEL_SETTINGS_FORWARDED_KEYS = frozenset( + { + "frequency_penalty", + "max_tokens", + "parallel_tool_calls", + "presence_penalty", + "reasoning", + "temperature", + "tool_choice", + "top_p", + "verbosity", + } +) + +#: Forwarded keys whose value must be an object. A config value of any other type is malformed +#: and dropped rather than passed to ``ModelSettings``. +_MODEL_SETTINGS_MAPPING_KEYS = frozenset({"reasoning"}) + +#: Declared by ``ModelSettings`` but never forwarded, and why: +#: * ``context_management``, ``truncation``: server-side state. They trim or compact conversation +#: state the server keeps. +#: * ``store``, ``prompt_cache_retention``: data retention. They decide whether, and for how long, +#: the request and its cache stay on the server. +#: * ``metadata``: identity and attribution. +#: * ``include_usage``: runtime wiring. It changes how usage is reported back to the handler. +#: * ``response_include``, ``top_logprobs``: API-shape switch. They add output the handler does not +#: read. +#: * ``retry``: client/connection configuration, an HTTP retry count. +#: * ``extra_headers``, ``extra_query``, ``extra_body``, ``extra_args``: raw HTTP/request overrides. +#: +#: Named for the drift test and for review, not read at runtime: the forwarded list above already +#: leaves these out, so nothing needs to subtract them again. +_MODEL_SETTINGS_EXCLUDED_KEYS = frozenset( + { + "context_management", + "truncation", + "store", + "prompt_cache_retention", + "metadata", + "include_usage", + "response_include", + "top_logprobs", + "retry", + "extra_headers", + "extra_query", + "extra_body", + "extra_args", + } +) + + +def _apply_text_verbosity(params: dict[str, Any]) -> dict[str, Any]: + """Moves ``text.verbosity`` to ``ModelSettings``'s top-level ``verbosity`` and drops the rest + of ``text``. An explicit ``text.verbosity`` wins over a top-level ``verbosity``, as in the JS + SDK, and a ``text`` that is not an object is malformed and dropped. + """ + text = params.pop("text", None) + if isinstance(text, Mapping) and "verbosity" in text: + params["verbosity"] = text["verbosity"] + return params + + +#: The ``reasoning`` sub-keys forwarded, the same two the JS SDK keeps. +_REASONING_KEYS = ("effort", "summary") + + +def _rebuild_reasoning(params: dict[str, Any]) -> dict[str, Any]: + """Rebuilds a forwarded ``reasoning`` object as the SDK's own ``Reasoning`` type, the type + ``ModelSettings`` declares, keeping only ``effort`` and ``summary`` as the JS SDK does. Any + other sub-key is dropped, and so is a ``reasoning`` with neither. + """ + reasoning = params.pop("reasoning", None) + if not isinstance(reasoning, Mapping): + return params + kept = {k: reasoning[k] for k in _REASONING_KEYS if k in reasoning} + if kept: + from openai.types.shared import Reasoning + + params["reasoning"] = Reasoning(**kept) + return params + + +def _model_settings_parameters(config: AiConfigRep) -> dict[str, Any]: + """The config's ``model.parameters`` that become ``ModelSettings`` fields. The handler and the + native graph both call this, so they forward the same keys. ``max_turns`` is not among them: + it is a ``Runner.run`` option, read separately for the run. + """ + return _rebuild_reasoning( + select_forwarded_parameters( + _apply_text_verbosity(model_parameters(config)), + _MODEL_SETTINGS_FORWARDED_KEYS, + mapping_keys=_MODEL_SETTINGS_MAPPING_KEYS, + ) + ) + def _build_agent_tools( config_tools: dict[str, Any], @@ -220,9 +326,17 @@ def _build_agent_and_prompt( # change, not a telemetry one, so it is left alone. `_call_impl` still returns the parsed # `final_output` object as-is when `outputFormat` is configured, matching the pre-existing # return-shape contract. + # `max_turns` is a `Runner.run` option, not a `ModelSettings` field, so the filter drops it here + # and the run call reads it separately. + model_settings_params = _model_settings_parameters(config) agent = Agent( name="assistant", model=config.get("model", {}).get("name", "gpt-4o"), + **( + {"model_settings": agents_mod.ModelSettings(**model_settings_params)} + if model_settings_params + else {} + ), **({"instructions": instructions} if instructions else {}), **({"tools": tools} if tools else {}), ) @@ -573,7 +687,9 @@ async def _call_impl( system_instructions=instructions, messages=to_request_span_messages(prompt), ) - result = await Runner.run(agent, prompt, hooks=hooks) + max_turns = model_parameters(config).get("max_turns") + run_kwargs = {"max_turns": max_turns} if max_turns is not None else {} + result = await Runner.run(agent, prompt, hooks=hooks, **run_kwargs) final_output = result.final_output set_output_content_attributes( span, @@ -703,7 +819,9 @@ async def _stream_gen( system_instructions=instructions, messages=to_request_span_messages(prompt), ) - streamed = Runner.run_streamed(agent, prompt, hooks=hooks) + max_turns = model_parameters(config).get("max_turns") + run_kwargs = {"max_turns": max_turns} if max_turns is not None else {} + streamed = Runner.run_streamed(agent, prompt, hooks=hooks, **run_kwargs) full_output = "" async for event in streamed.stream_events(): diff --git a/packages/openai-agents/src/launchdarkly_ai_openai_agents/native_graph.py b/packages/openai-agents/src/launchdarkly_ai_openai_agents/native_graph.py index db89461e..51ed6436 100644 --- a/packages/openai-agents/src/launchdarkly_ai_openai_agents/native_graph.py +++ b/packages/openai-agents/src/launchdarkly_ai_openai_agents/native_graph.py @@ -26,10 +26,14 @@ to_ld_context, tool_result_string, ) -from launchdarkly_ai_server.utils import make_graph_track_data +from launchdarkly_ai_server.utils import make_graph_track_data, model_parameters from ._version import PACKAGE_NAME, __version__ -from .handler import _parse_message_content, _to_openai_agent_items +from .handler import ( + _model_settings_parameters, + _parse_message_content, + _to_openai_agent_items, +) try: from opentelemetry import trace @@ -200,9 +204,19 @@ async def _visit(node_key: str) -> None: agent_name = _sanitize_name(node.key) agent_name_to_key[agent_name] = node.key + node_model_settings_params = _model_settings_parameters(node.config) agent = Agent( name=agent_name, model=node.config.get("model", {}).get("name", "gpt-4o"), + **( + { + "model_settings": agents_mod.ModelSettings( + **node_model_settings_params + ) + } + if node_model_settings_params + else {} + ), **({"instructions": instructions} if instructions else {}), **({"tools": tools} if tools else {}), **({"handoffs": child_handoffs} if child_handoffs else {}), @@ -287,7 +301,17 @@ async def on_agent_start(self, context: Any, agent: Any) -> None: ) root_prompt = _to_openai_agent_items(turns) - result = await Runner.run(root_agent, root_prompt, hooks=hooks) + # Only the root node's `max_turns` applies. The whole graph is one `Runner.run`, and + # the Agents SDK counts turns once for that run, across every handoff: `max_turns` is + # a `Runner.run` argument, and `Agent` has no per-agent turn limit to set from a + # child node's config. + root_max_turns = model_parameters(root.config).get("max_turns") + root_run_kwargs = ( + {"max_turns": root_max_turns} if root_max_turns is not None else {} + ) + result = await Runner.run( + root_agent, root_prompt, hooks=hooks, **root_run_kwargs + ) final_output = str(result.final_output or "") # Sum usage across all raw_responses diff --git a/packages/openai-agents/tests/test_handler.py b/packages/openai-agents/tests/test_handler.py index 8712801e..3221afe1 100644 --- a/packages/openai-agents/tests/test_handler.py +++ b/packages/openai-agents/tests/test_handler.py @@ -27,6 +27,12 @@ openai_agents, ) from launchdarkly_ai_openai_agents.utils import build_output_type +from tests.forwarding_spec import ( + OPENAI_AGENTS, + candidate_keys, + probe_forwarded_keys, +) +from tests.never_forwarded import NEVER_FORWARDED_BAG, find_leaks # --------------------------------------------------------------------------- # Fake `agents` SDK @@ -164,10 +170,16 @@ def run_streamed(agent: Any, prompt: str, hooks: Any = None, **kw: Any) -> Any: return run_streamed +class FakeModelSettings: + def __init__(self, **kw: Any) -> None: + self.kwargs = kw + + def _fake_agents_module(run: Any = None, run_streamed: Any = None) -> Any: mod = SimpleNamespace() mod.FunctionTool = FakeFunctionTool mod.Agent = FakeAgent + mod.ModelSettings = FakeModelSettings class Runner: pass @@ -1683,6 +1695,148 @@ async def test_emits_no_content_by_default_on_the_streaming_path(self) -> None: # --------------------------------------------------------------------------- +class TestModelParametersForwarding: + async def test_snake_case_param_reaches_model_settings(self) -> None: + run_kwargs: dict[str, Any] = {} + + async def run(agent: Any, prompt: str, hooks: Any = None, **kw: Any) -> Any: + run_kwargs["agent"] = agent + await _drive_turns(hooks, agent, prompt, [{"output": _text_output("hi")}]) + return FakeRunResult("done") + + agents_mod = _fake_agents_module(run=run) + config = _make_config( + instructions="Be helpful.", + model={"name": "gpt-4o", "parameters": {"top_p": 0.5}}, + ) + with _patched_agents(agents_mod): + await create_openai_agent_handler()(config, "q", {}, {}) + model_settings = run_kwargs["agent"].kwargs["model_settings"] + assert model_settings.kwargs["top_p"] == 0.5 + + async def test_max_turns_from_config_reaches_runner_run(self) -> None: + captured: dict[str, Any] = {} + + async def run(agent: Any, prompt: str, hooks: Any = None, **kw: Any) -> Any: + captured["max_turns"] = kw.get("max_turns") + await _drive_turns(hooks, agent, prompt, [{"output": _text_output("hi")}]) + return FakeRunResult("done") + + agents_mod = _fake_agents_module(run=run) + config = _make_config( + instructions="Be helpful.", + model={"name": "gpt-4o", "parameters": {"max_turns": 3}}, + ) + with _patched_agents(agents_mod): + await create_openai_agent_handler()(config, "q", {}, {}) + assert captured["max_turns"] == 3 + + async def test_max_turns_from_config_reaches_runner_run_streamed(self) -> None: + captured: dict[str, Any] = {} + + def run_streamed(agent: Any, prompt: str, hooks: Any = None, **kw: Any) -> Any: + captured["max_turns"] = kw.get("max_turns") + return FakeStreamedResult( + agent, prompt, hooks, [{"output": _text_output("hi")}], "done" + ) + + agents_mod = _fake_agents_module(run_streamed=run_streamed) + config = _make_config( + instructions="Be helpful.", + model={"name": "gpt-4o", "parameters": {"max_turns": 4}}, + ) + with _patched_agents(agents_mod): + events = [] + gen = await create_openai_agent_handler().stream(config, "q", {}, {}) + async for event in gen: + events.append(event) + assert captured["max_turns"] == 4 + + async def test_max_turns_not_owned_by_model_settings(self) -> None: + run_kwargs: dict[str, Any] = {} + + async def run(agent: Any, prompt: str, hooks: Any = None, **kw: Any) -> Any: + run_kwargs["agent"] = agent + await _drive_turns(hooks, agent, prompt, [{"output": _text_output("hi")}]) + return FakeRunResult("done") + + agents_mod = _fake_agents_module(run=run) + config = _make_config( + instructions="Be helpful.", + model={"name": "gpt-4o", "parameters": {"max_turns": 3}}, + ) + with _patched_agents(agents_mod): + await create_openai_agent_handler()(config, "q", {}, {}) + model_settings = run_kwargs["agent"].kwargs.get("model_settings") + assert model_settings is None or "max_turns" not in model_settings.kwargs + + async def test_call_unchanged_when_no_parameters_set(self) -> None: + run_kwargs: dict[str, Any] = {} + + async def run(agent: Any, prompt: str, hooks: Any = None, **kw: Any) -> Any: + run_kwargs["agent"] = agent + run_kwargs["kw"] = kw + await _drive_turns(hooks, agent, prompt, [{"output": _text_output("hi")}]) + return FakeRunResult("done") + + agents_mod = _fake_agents_module(run=run) + with _patched_agents(agents_mod): + await create_openai_agent_handler()(CONFIG, "q", {}, {}) + assert "model_settings" not in run_kwargs["agent"].kwargs + assert run_kwargs["kw"].get("max_turns") is None + + async def test_ui_keys_the_sdk_rejects_are_dropped_without_raising(self) -> None: + """Neither is a field of ``agents.ModelSettings``; forwarding one unfiltered raises + ``TypeError`` before this filter existed.""" + run_kwargs: dict[str, Any] = {} + + async def run(agent: Any, prompt: str, hooks: Any = None, **kw: Any) -> Any: + run_kwargs["agent"] = agent + await _drive_turns(hooks, agent, prompt, [{"output": _text_output("hi")}]) + return FakeRunResult("done") + + agents_mod = _fake_agents_module(run=run) + config = _make_config( + instructions="Be helpful.", + model={ + "name": "gpt-4o", + "parameters": { + "top_p": 0.5, + "stop_sequences": ["STOP"], + "seed": 42, + }, + }, + ) + with _patched_agents(agents_mod): + await create_openai_agent_handler()(config, "q", {}, {}) + model_settings = run_kwargs["agent"].kwargs["model_settings"] + assert model_settings.kwargs["top_p"] == 0.5 + assert "stop_sequences" not in model_settings.kwargs + assert "seed" not in model_settings.kwargs + + async def test_transport_key_is_never_forwarded(self) -> None: + run_kwargs: dict[str, Any] = {} + + async def run(agent: Any, prompt: str, hooks: Any = None, **kw: Any) -> Any: + run_kwargs["agent"] = agent + await _drive_turns(hooks, agent, prompt, [{"output": _text_output("hi")}]) + return FakeRunResult("done") + + agents_mod = _fake_agents_module(run=run) + config = _make_config( + instructions="Be helpful.", + model={ + "name": "gpt-4o", + "parameters": {"top_p": 0.5, "extra_body": {"secret": "value"}}, + }, + ) + with _patched_agents(agents_mod): + await create_openai_agent_handler()(config, "q", {}, {}) + model_settings = run_kwargs["agent"].kwargs["model_settings"] + assert model_settings.kwargs["top_p"] == 0.5 + assert "extra_body" not in model_settings.kwargs + + class TestOutputFormat: def test_absent_output_format_no_change(self) -> None: assert build_output_type(None) is None @@ -2386,3 +2540,227 @@ async def _drain() -> None: assert rec.root.attributes.get("launchdarkly.run.cancelled") is True assert "launchdarkly.stream.abandoned" not in rec.root.attributes + + +class TestNeverForwardedParameters: + """No credential, endpoint, request-injection, remote-tool, or host-process key in + ``model.parameters`` reaches ``ModelSettings`` or the ``Runner`` call.""" + + async def test_invoke_forwards_none_of_them(self) -> None: + captured: dict[str, Any] = {} + + async def run(agent: Any, prompt: str, hooks: Any = None, **kw: Any) -> Any: + captured["agent"] = agent + captured["run_kwargs"] = kw + await _drive_turns(hooks, agent, prompt, [{"output": _text_output("hi")}]) + return FakeRunResult("done") + + agents_mod = _fake_agents_module(run=run) + config = _make_config( + instructions="Be helpful.", + model={ + "name": "gpt-4o", + "parameters": {**NEVER_FORWARDED_BAG, "temperature": 0.1}, + }, + ) + with _patched_agents(agents_mod): + await create_openai_agent_handler()(config, "q", {}, {}) + model_settings = captured["agent"].kwargs["model_settings"] + assert model_settings.kwargs == {"temperature": 0.1} + assert not find_leaks(captured["run_kwargs"]) + + async def test_stream_forwards_none_of_them(self) -> None: + captured: dict[str, Any] = {} + + def run_streamed(agent: Any, prompt: str, hooks: Any = None, **kw: Any) -> Any: + captured["agent"] = agent + captured["run_kwargs"] = kw + return FakeStreamedResult( + agent, prompt, hooks, [{"output": _text_output("hi")}], "done" + ) + + agents_mod = _fake_agents_module(run_streamed=run_streamed) + config = _make_config( + instructions="Be helpful.", + model={ + "name": "gpt-4o", + "parameters": {**NEVER_FORWARDED_BAG, "temperature": 0.1}, + }, + ) + with _patched_agents(agents_mod): + gen = await create_openai_agent_handler().stream(config, "q", {}, {}) + async for _event in gen: + pass + model_settings = captured["agent"].kwargs["model_settings"] + assert model_settings.kwargs == {"temperature": 0.1} + assert not find_leaks(captured["run_kwargs"]) + + +def _settings_and_run(captured: dict[str, Any]) -> object: + """What one handler call handed the SDK from the config: the ``ModelSettings`` kwargs (or + none) and the ``Runner`` kwargs.""" + settings = captured["agent"].kwargs.get("model_settings") + return (settings.kwargs if settings is not None else None, captured["run_kwargs"]) + + +class TestForwardsExactlyTheCrossSdkList: + """Probes ``invoke`` and ``stream`` one key at a time: the keys that change ``ModelSettings`` + or the ``Runner`` call are exactly the cross-SDK OpenAI Agents list, on both paths.""" + + @staticmethod + def _candidates() -> frozenset[str]: + import dataclasses + + from agents import ModelSettings + + return candidate_keys( + (f.name for f in dataclasses.fields(ModelSettings)), OPENAI_AGENTS + ) + + @staticmethod + def _config(parameters: dict[str, Any]) -> dict[str, Any]: + return _make_config( + instructions="Be helpful.", + model={"name": "gpt-4o", "parameters": parameters}, + ) + + async def test_invoke(self) -> None: + captured: dict[str, Any] = {} + + async def run(agent: Any, prompt: str, hooks: Any = None, **kw: Any) -> Any: + captured["agent"] = agent + captured["run_kwargs"] = kw + await _drive_turns(hooks, agent, prompt, [{"output": _text_output("hi")}]) + return FakeRunResult("done") + + async def call(parameters: dict[str, Any]) -> object: + with _patched_agents(_fake_agents_module(run=run)): + await create_openai_agent_handler()( + self._config(parameters), "q", {}, {} + ) + return _settings_and_run(captured) + + assert await probe_forwarded_keys(self._candidates(), call) == OPENAI_AGENTS + + async def test_stream(self) -> None: + captured: dict[str, Any] = {} + + def run_streamed(agent: Any, prompt: str, hooks: Any = None, **kw: Any) -> Any: + captured["agent"] = agent + captured["run_kwargs"] = kw + return FakeStreamedResult( + agent, prompt, hooks, [{"output": _text_output("hi")}], "done" + ) + + async def call(parameters: dict[str, Any]) -> object: + with _patched_agents(_fake_agents_module(run_streamed=run_streamed)): + gen = await create_openai_agent_handler().stream( + self._config(parameters), "q", {}, {} + ) + async for _event in gen: + pass + return _settings_and_run(captured) + + assert await probe_forwarded_keys(self._candidates(), call) == OPENAI_AGENTS + + +class TestTextVerbosity: + def _settings(self, parameters: dict[str, Any]) -> dict[str, Any]: + from launchdarkly_ai_openai_agents.handler import _model_settings_parameters + + return _model_settings_parameters({"model": {"parameters": parameters}}) + + def test_text_verbosity_becomes_verbosity(self) -> None: + assert self._settings({"text": {"verbosity": "low", "format": {}}}) == { + "verbosity": "low" + } + + def test_text_verbosity_wins_over_top_level_verbosity(self) -> None: + assert self._settings({"verbosity": "high", "text": {"verbosity": "low"}}) == { + "verbosity": "low" + } + + def test_top_level_verbosity_applies_without_text_verbosity(self) -> None: + assert self._settings({"verbosity": "high", "text": {"format": {}}}) == { + "verbosity": "high" + } + + def test_text_that_is_not_an_object_is_dropped(self) -> None: + assert self._settings({"text": "low", "temperature": 0.1}) == { + "temperature": 0.1 + } + + def test_reasoning_that_is_not_an_object_is_dropped(self) -> None: + assert self._settings({"reasoning": "high", "temperature": 0.1}) == { + "temperature": 0.1 + } + + +class TestReasoningIsRebuilt: + def _settings(self, parameters: dict[str, Any]) -> dict[str, Any]: + from launchdarkly_ai_openai_agents.handler import _model_settings_parameters + + return _model_settings_parameters({"model": {"parameters": parameters}}) + + def test_becomes_the_sdk_type_with_only_effort_and_summary(self) -> None: + from openai.types.shared import Reasoning + + settings = self._settings( + {"reasoning": {"effort": "low", "summary": "auto", "generate_summary": "x"}} + ) + assert settings == {"reasoning": Reasoning(effort="low", summary="auto")} + assert settings["reasoning"].model_dump(exclude_none=True) == { + "effort": "low", + "summary": "auto", + } + + def test_reasoning_with_neither_sub_key_is_dropped(self) -> None: + assert self._settings({"reasoning": {"other": 1}, "temperature": 0.1}) == { + "temperature": 0.1 + } + + async def test_chat_completions_model_reads_it_without_raising(self) -> None: + """The Chat Completions model reads ``reasoning.effort`` as an attribute and sends it as + ``reasoning_effort``. Runs the real model against a mocked OpenAI client.""" + from agents import ModelSettings, ModelTracing + from agents.models.openai_chatcompletions import OpenAIChatCompletionsModel + from openai.types.chat import ChatCompletion + + completion = ChatCompletion.model_validate( + { + "id": "c1", + "object": "chat.completion", + "created": 0, + "model": "gpt-4o", + "choices": [ + { + "index": 0, + "finish_reason": "stop", + "message": {"role": "assistant", "content": "hi"}, + } + ], + "usage": { + "prompt_tokens": 1, + "completion_tokens": 1, + "total_tokens": 2, + }, + } + ) + client = MagicMock() + client.chat.completions.create = AsyncMock(return_value=completion) + client.base_url = "https://api.openai.com/v1/" + model = OpenAIChatCompletionsModel("gpt-4o", client) + + settings = ModelSettings(**self._settings({"reasoning": {"effort": "low"}})) + await model.get_response( + system_instructions=None, + input="q", + model_settings=settings, + tools=[], + output_schema=None, + handoffs=[], + tracing=ModelTracing.DISABLED, + previous_response_id=None, + ) + kwargs = client.chat.completions.create.call_args.kwargs + assert kwargs["reasoning_effort"] == "low" diff --git a/packages/openai-agents/tests/test_native_graph.py b/packages/openai-agents/tests/test_native_graph.py index e0dac6e8..3528db92 100644 --- a/packages/openai-agents/tests/test_native_graph.py +++ b/packages/openai-agents/tests/test_native_graph.py @@ -17,6 +17,12 @@ to_openai_agents, ) from launchdarkly_ai_server import GraphDefinition, GraphEdge, GraphNode +from tests.forwarding_spec import ( + OPENAI_AGENTS, + candidate_keys, + probe_forwarded_keys, +) +from tests.never_forwarded import NEVER_FORWARDED_BAG, find_leaks # --------------------------------------------------------------------------- # Helpers @@ -1078,6 +1084,46 @@ async def _run_and_fire_hook(agent: Any, text: str, hooks: Any = None) -> Any: assert "$ld:ai:graph:handoff_success" in track_calls +class TestNativeGraphModelParameters: + @pytest.mark.asyncio + async def test_no_never_forwarded_key_reaches_any_agent_or_the_run(self) -> None: + run_result = _make_run_result("out") + agents_mock = _make_agents_mock(run_result) + agents_mock.ModelSettings = MagicMock(side_effect=lambda **kw: dict(kw)) + nodes = { + "root": { + "key": "root", + "config": { + "model": { + "name": "gpt-4o", + "parameters": { + **NEVER_FORWARDED_BAG, + "temperature": 0.1, + "max_turns": 6, + }, + }, + "instructions": "help", + }, + "meta": {"variationKey": "v1", "version": 1}, + "edges": [], + "is_terminal": True, + } + } + graph_def = _make_graph_def(nodes=nodes) + + with patch( + "importlib.import_module", + side_effect=lambda n: agents_mock if n == "agents" else __import__(n), + ): + await to_openai_agents(_make_def_promise(graph_def)).invoke("hi") + + (root_agent,) = agents_mock._created_agents + assert root_agent._kw["model_settings"] == {"temperature": 0.1} + run_kwargs = agents_mock.Runner.run.call_args.kwargs + assert run_kwargs["max_turns"] == 6 + assert not find_leaks({k: v for k, v in run_kwargs.items() if k != "hooks"}) + + class TestBuildNodeToolsSyncHandlers: """``_build_node_tools`` must accept sync handlers (native path skips wrap_tool_handlers).""" @@ -1177,3 +1223,49 @@ def sync_wrapper(_args: Any) -> Any: _build_node_tools(node, {"my-tool": sync_wrapper}) assert await captured[0]({}) == "done" + + +class TestNativeGraphForwardsExactlyTheCrossSdkList: + """The native graph builds each agent's ``ModelSettings`` with the handler's own forwarding + and reads the root's ``max_turns`` for the run, so a probe of one node finds exactly the + cross-SDK OpenAI Agents list.""" + + @pytest.mark.asyncio + async def test_root_agent_and_run(self) -> None: + import dataclasses + + from agents import ModelSettings + + async def call(parameters: dict[str, Any]) -> object: + agents_mock = _make_agents_mock(_make_run_result("out")) + agents_mock.ModelSettings = MagicMock(side_effect=lambda **kw: dict(kw)) + nodes = { + "root": { + "key": "root", + "config": { + "model": {"name": "gpt-4o", "parameters": parameters}, + "instructions": "help", + }, + "meta": {"variationKey": "v1", "version": 1}, + "edges": [], + "is_terminal": True, + } + } + with patch( + "importlib.import_module", + side_effect=lambda n: agents_mock if n == "agents" else __import__(n), + ): + await to_openai_agents( + _make_def_promise(_make_graph_def(nodes=nodes)) + ).invoke("hi") + (root_agent,) = agents_mock._created_agents + run_kwargs = agents_mock.Runner.run.call_args.kwargs + return ( + root_agent._kw.get("model_settings"), + {k: v for k, v in run_kwargs.items() if k not in {"hooks", "context"}}, + ) + + candidates = candidate_keys( + (f.name for f in dataclasses.fields(ModelSettings)), OPENAI_AGENTS + ) + assert await probe_forwarded_keys(candidates, call) == OPENAI_AGENTS diff --git a/packages/openai-agents/tests/test_parameter_forwarding.py b/packages/openai-agents/tests/test_parameter_forwarding.py new file mode 100644 index 00000000..c9020769 --- /dev/null +++ b/packages/openai-agents/tests/test_parameter_forwarding.py @@ -0,0 +1,43 @@ +""" +Drift test for the ``agents.ModelSettings`` parameter classification in ``handler.py``. + +``_MODEL_SETTINGS_FORWARDED_KEYS`` is a literal, hand-maintained list. This test reads +``ModelSettings``'s own dataclass fields and asserts every field it declares is classified in +exactly one of forwarded or excluded, so an SDK field nobody has classified yet fails loudly by +name, and so does a list entry that is not a real SDK field. ``ModelSettings`` has no handler-owned +fields: ``model`` and ``max_turns`` live outside it (on ``Agent``/``Runner.run``). +""" + +from __future__ import annotations + +import dataclasses + +from agents import ModelSettings + +from launchdarkly_ai_openai_agents.handler import ( + _MODEL_SETTINGS_EXCLUDED_KEYS as _EXCLUDED_KEYS, +) +from launchdarkly_ai_openai_agents.handler import _MODEL_SETTINGS_FORWARDED_KEYS + + +class TestModelSettingsAcceptsExactlyTheseFields: + def test_every_field_is_classified_exactly_once(self) -> None: + accepted = frozenset(f.name for f in dataclasses.fields(ModelSettings)) + classified = _MODEL_SETTINGS_FORWARDED_KEYS | _EXCLUDED_KEYS + + unclassified = accepted - classified + assert not unclassified, ( + f"ModelSettings now declares {sorted(unclassified)}, not classified as " + "forwarded or excluded in openai-agents handler.py" + ) + + overlap = _MODEL_SETTINGS_FORWARDED_KEYS & _EXCLUDED_KEYS + assert not overlap, f"fields classified more than once: {sorted(overlap)}" + + def test_every_classified_field_is_real(self) -> None: + accepted = frozenset(f.name for f in dataclasses.fields(ModelSettings)) + stale = (_MODEL_SETTINGS_FORWARDED_KEYS | _EXCLUDED_KEYS) - accepted + assert not stale, ( + f"{sorted(stale)} classified in openai-agents handler.py but " + "ModelSettings does not declare them" + ) diff --git a/packages/openai-messages/src/launchdarkly_ai_openai_messages/handler.py b/packages/openai-messages/src/launchdarkly_ai_openai_messages/handler.py index 4d3f22cc..f94d792c 100644 --- a/packages/openai-messages/src/launchdarkly_ai_openai_messages/handler.py +++ b/packages/openai-messages/src/launchdarkly_ai_openai_messages/handler.py @@ -30,7 +30,9 @@ ) from launchdarkly_ai_server.agui import TurnEmitter from launchdarkly_ai_server.lifecycle import effective_capture_content +from launchdarkly_ai_server.parameter_forwarding import select_forwarded_parameters from launchdarkly_ai_server.responses import map_responses_event +from launchdarkly_ai_server.utils import model_parameters from ._version import PACKAGE_NAME, __version__ from .spans import ( @@ -52,6 +54,129 @@ tool_arguments, ) +#: Every key ``AsyncResponses.create``/``.stream`` accept, classified by hand into exactly one of: +#: forwarded (below), handler-owned (``model``, ``input``, ``previous_response_id``, ``tools``, +#: ``text``, set by each call site itself), or excluded (below). +#: ``TestResponsesCreateAcceptsExactlyTheseKeys`` / ``TestResponsesStreamAcceptsExactlyTheseKeys`` +#: in this package's tests assert this classification stays exhaustive as the SDK's own signatures +#: change. The UI offers several keys the Responses API has never accepted: ``max_tokens``, +#: ``frequency_penalty``, ``presence_penalty``, ``seed``, ``n``, ``stop``, ``response_format``, +#: ``logit_bias``, ``logprobs``, ``max_completion_tokens``, ``audio``, ``modalities``, +#: ``prediction``. Of those, ``max_tokens``/``max_completion_tokens`` are renamed to +#: ``max_output_tokens`` (see :func:`_apply_max_output_tokens_rename`); the rest are dropped. +#: +#: The UI writes reasoning effort as ``reasoning: {"effort": ...}``, the Responses API's own shape, +#: so ``reasoning`` is forwarded as-is and no top-level ``reasoning_effort`` is read. +#: +#: This is the cross-SDK list for OpenAI Messages (TESTING.md section 1.12). The same list serves +#: ``invoke`` and ``stream`` (both read :func:`_forwarded_parameters`), so one config behaves the +#: same whichever is called. ``max_tool_calls`` is marked not supported in TypeScript there: the JS +#: ``openai`` SDK's ``responses.create`` has no such parameter, while this SDK's ``create`` and +#: ``stream`` both take it. +_RESPONSES_FORWARDED_KEYS = frozenset( + { + "max_output_tokens", + "max_tool_calls", + "parallel_tool_calls", + "prompt_cache_key", + "reasoning", + "service_tier", + "temperature", + "tool_choice", + "top_logprobs", + "top_p", + } +) + +#: Forwarded keys whose value must be an object. A config value of any other type is malformed +#: and dropped rather than passed to the API. +_RESPONSES_MAPPING_KEYS = frozenset({"reasoning"}) + +#: Accepted by the API but never forwarded, and why: +#: * ``stream``, ``stream_options``: runtime wiring. The handler chooses blocking vs. streaming +#: itself, and ``stream_options`` only means something together with ``stream=True``. +#: * ``background``: runtime wiring. It returns before the output exists, so the handler would get +#: no result. +#: * ``include``: API-shape switch. It adds output items the handler does not read, such as +#: encrypted reasoning or search results. +#: * ``conversation``, ``context_management``, ``truncation``: server-side state. They carry, trim +#: or compact conversation state the server keeps, which conflicts with the input the handler +#: builds. +#: * ``response_id``, ``starting_after``: server-side state. Only ``.stream`` accepts them, to +#: resume an existing response rather than start a new one, and ``invoke`` has no equivalent. +#: * ``store``, ``prompt_cache_retention``: data retention. They decide whether, and for how long, +#: the request and its cache stay on the server. +#: * ``metadata``, ``user``, ``safety_identifier``: identity and attribution. They say who the +#: request is for. +#: * ``prompt``, ``instructions``: prompt content beyond the config's instructions. The handler +#: builds the input from the config's own instructions and messages, and a server-side prompt +#: template or a second instructions string would replace or add to it. +#: * ``moderation``: safety configuration. +#: * ``text_format``: API-shape switch. Only ``.stream`` accepts it, and it is a Python type to +#: parse into, which a config cannot express. Structured output goes through ``text``, which the +#: handler owns. +#: * ``timeout``, ``extra_headers``, ``extra_query``, ``extra_body``: client/connection +#: configuration (a request timeout, raw HTTP overrides), never a config-controlled setting. +#: +#: Named for the drift test and for review, not read at runtime: the forwarded list above already +#: leaves these out, so nothing needs to subtract them again. +_RESPONSES_EXCLUDED_KEYS = frozenset( + { + "stream", + "stream_options", + "background", + "include", + "conversation", + "context_management", + "truncation", + "response_id", + "starting_after", + "store", + "prompt_cache_retention", + "metadata", + "user", + "safety_identifier", + "prompt", + "instructions", + "moderation", + "text_format", + "timeout", + "extra_headers", + "extra_query", + "extra_body", + } +) + + +def _apply_max_output_tokens_rename(params: dict[str, Any]) -> dict[str, Any]: + """Renames the Chat-Completions-era ``max_tokens``/``max_completion_tokens`` to the Responses + API's ``max_output_tokens``, the only one of the three the API actually accepts. + + Precedence when more than one is set: an explicit ``max_output_tokens`` wins outright, then + ``max_completion_tokens``, then ``max_tokens``. + """ + max_tokens = params.pop("max_tokens", None) + max_completion_tokens = params.pop("max_completion_tokens", None) + if "max_output_tokens" not in params: + if max_completion_tokens is not None: + params["max_output_tokens"] = max_completion_tokens + elif max_tokens is not None: + params["max_output_tokens"] = max_tokens + return params + + +def _forwarded_parameters(config: AiConfigRep) -> dict[str, Any]: + """The config's ``model.parameters`` that reach ``responses.create``/``.stream``: + ``max_tokens``/``max_completion_tokens`` renamed to ``max_output_tokens``, then only + :data:`_RESPONSES_FORWARDED_KEYS`, with malformed object values dropped. ``invoke`` and + ``stream`` both call this, so they forward the same keys. + """ + return select_forwarded_parameters( + _apply_max_output_tokens_rename(model_parameters(config)), + _RESPONSES_FORWARDED_KEYS, + mapping_keys=_RESPONSES_MAPPING_KEYS, + ) + def _build_tools(config_tools: dict[str, Any]) -> list[dict[str, Any]]: # Not filtered to the tools that have a registered handler, unlike the TypeScript SDK. That @@ -282,7 +407,9 @@ async def _call_impl( messages=root_messages, ) + extra_params = _forwarded_parameters(config) params: dict[str, Any] = { + **extra_params, "model": config["model"]["name"], "input": input_messages, } @@ -362,6 +489,7 @@ async def _call_impl( client, config, { + **extra_params, "model": config["model"]["name"], "previous_response_id": response.id, "input": tool_outputs, @@ -512,7 +640,9 @@ async def _stream_gen( tool_definitions=tool_definitions, ) + extra_params = _forwarded_parameters(config) stream_params: dict[str, Any] = { + **extra_params, "model": config["model"]["name"], "input": current_input, } diff --git a/packages/openai-messages/tests/test_handler.py b/packages/openai-messages/tests/test_handler.py index 49d43465..5cf0c34c 100644 --- a/packages/openai-messages/tests/test_handler.py +++ b/packages/openai-messages/tests/test_handler.py @@ -12,8 +12,20 @@ from typing import Any, ClassVar from unittest.mock import AsyncMock, MagicMock, patch +import httpx import pytest +from tests.forwarding_spec import ( + OPENAI_MESSAGES, + candidate_keys, + probe_forwarded_keys, +) +from tests.never_forwarded import ( + NEVER_FORWARDED_BAG, + NEVER_FORWARDED_KEYS, + find_leaks, +) + CONFIG = { "model": {"name": "gpt-4o"}, "provider": {"name": "OpenAI"}, @@ -1152,6 +1164,335 @@ async def test_rethrows_error(self, mock_openai: MagicMock) -> None: # --------------------------------------------------------------------------- +class TestModelParametersForwarding: + async def test_snake_case_param_reaches_provider( + self, mock_openai: MagicMock + ) -> None: + from launchdarkly_ai_openai_messages import create_openai_messages_handler + + config = { + **CONFIG, + "model": {**CONFIG["model"], "parameters": {"top_p": 0.5}}, + } + h = create_openai_messages_handler() + await h(config, "q", {}, {}) + kwargs = mock_openai.responses.create.call_args.kwargs + assert kwargs["top_p"] == 0.5 + + async def test_config_cannot_override_model_or_input( + self, mock_openai: MagicMock + ) -> None: + from launchdarkly_ai_openai_messages import create_openai_messages_handler + + config = { + **CONFIG, + "model": { + **CONFIG["model"], + "parameters": { + "model": "not-the-real-model", + "input": "not-the-real-input", + }, + }, + } + h = create_openai_messages_handler() + await h(config, "q", {}, {}) + kwargs = mock_openai.responses.create.call_args.kwargs + assert kwargs["model"] == CONFIG["model"]["name"] + assert kwargs["input"] != "not-the-real-input" + + async def test_call_unchanged_when_no_parameters_set( + self, mock_openai: MagicMock + ) -> None: + from launchdarkly_ai_openai_messages import create_openai_messages_handler + + h = create_openai_messages_handler() + await h(CONFIG, "q", {}, {}) + kwargs = mock_openai.responses.create.call_args.kwargs + assert set(kwargs.keys()) == {"model", "input"} + + async def test_streaming_forwards_snake_case_param( + self, mock_openai: MagicMock + ) -> None: + import launchdarkly_ai_openai_messages.spans as spans_mod + from launchdarkly_ai_openai_messages import create_openai_messages_handler + + mock_openai.responses.stream = MagicMock( + return_value=_make_openai_stream_context(["hi"]) + ) + config = { + **CONFIG, + "model": {**CONFIG["model"], "parameters": {"top_p": 0.3}}, + } + with patch.object(spans_mod, "_HAS_OTEL", False): + h = create_openai_messages_handler() + events = [e async for e in await h.stream(config, "q")] + assert events + stream_kwargs = mock_openai.responses.stream.call_args.kwargs + assert stream_kwargs["top_p"] == 0.3 + + async def test_ui_keys_the_sdk_rejects_are_dropped_without_raising( + self, mock_openai: MagicMock + ) -> None: + """None of these are ``responses.create`` parameters (they are Chat-Completions-era + keys); forwarding one unfiltered raises ``TypeError`` before this filter existed.""" + from launchdarkly_ai_openai_messages import create_openai_messages_handler + + config = { + **CONFIG, + "model": { + **CONFIG["model"], + "parameters": { + "max_tokens": 100, + "frequency_penalty": 0.1, + "presence_penalty": 0.1, + "seed": 42, + "n": 1, + "stop": ["END"], + "response_format": {"type": "text"}, + "logit_bias": {"50256": -100}, + "logprobs": True, + "max_completion_tokens": 50, + "audio": {"voice": "alloy"}, + "modalities": ["text"], + "prediction": {"type": "content", "content": "x"}, + "top_p": 0.5, + }, + }, + } + h = create_openai_messages_handler() + await h(config, "q", {}, {}) + kwargs = mock_openai.responses.create.call_args.kwargs + assert kwargs["top_p"] == 0.5 + for rejected in ( + "frequency_penalty", + "presence_penalty", + "seed", + "n", + "stop", + "response_format", + "logit_bias", + "logprobs", + "audio", + "modalities", + "prediction", + ): + assert rejected not in kwargs + + async def test_transport_key_is_never_forwarded( + self, mock_openai: MagicMock + ) -> None: + from launchdarkly_ai_openai_messages import create_openai_messages_handler + + config = { + **CONFIG, + "model": { + **CONFIG["model"], + "parameters": {"top_p": 0.5, "extra_body": {"secret": "value"}}, + }, + } + h = create_openai_messages_handler() + await h(config, "q", {}, {}) + kwargs = mock_openai.responses.create.call_args.kwargs + assert kwargs["top_p"] == 0.5 + assert "extra_body" not in kwargs + + async def test_stream_key_is_never_forwarded(self, mock_openai: MagicMock) -> None: + """The handler picks blocking vs. streaming by which client method it calls; a config + setting ``stream`` must not reach ``responses.create``, and other handler-controlled + transport keys (``background``, ``conversation``, ``prompt``, ``stream_options``) are + likewise dropped.""" + from launchdarkly_ai_openai_messages import create_openai_messages_handler + + config = { + **CONFIG, + "model": { + **CONFIG["model"], + "parameters": { + "stream": True, + "stream_options": {"include_usage": True}, + "background": True, + "conversation": "conv_123", + "prompt": {"id": "pmpt_123"}, + }, + }, + } + h = create_openai_messages_handler() + await h(config, "q", {}, {}) + kwargs = mock_openai.responses.create.call_args.kwargs + for excluded in ( + "stream", + "stream_options", + "background", + "conversation", + "prompt", + ): + assert excluded not in kwargs + + async def test_retention_identity_and_prompt_keys_are_dropped( + self, mock_openai: MagicMock + ) -> None: + """Data retention, identity, prompt content and safety keys the API accepts are dropped; + ``prompt_cache_key`` and ``service_tier`` next to them still forward.""" + from launchdarkly_ai_openai_messages import create_openai_messages_handler + + dropped = { + "store": False, + "prompt_cache_retention": "24h", + "user": "user-1", + "safety_identifier": "safe-1", + "metadata": {"k": "v"}, + "instructions": "be terse", + "moderation": "auto", + } + config = { + **CONFIG, + "model": { + **CONFIG["model"], + "parameters": { + **dropped, + "prompt_cache_key": "cache-1", + "service_tier": "auto", + }, + }, + } + h = create_openai_messages_handler() + await h(config, "q", {}, {}) + kwargs = mock_openai.responses.create.call_args.kwargs + assert not set(dropped) & set(kwargs) + assert kwargs["prompt_cache_key"] == "cache-1" + assert kwargs["service_tier"] == "auto" + + +class TestMaxOutputTokensRename: + async def test_max_tokens_renamed_to_max_output_tokens( + self, mock_openai: MagicMock + ) -> None: + from launchdarkly_ai_openai_messages import create_openai_messages_handler + + config = { + **CONFIG, + "model": {**CONFIG["model"], "parameters": {"max_tokens": 111}}, + } + h = create_openai_messages_handler() + await h(config, "q", {}, {}) + kwargs = mock_openai.responses.create.call_args.kwargs + assert kwargs["max_output_tokens"] == 111 + assert "max_tokens" not in kwargs + + async def test_max_completion_tokens_renamed_to_max_output_tokens( + self, mock_openai: MagicMock + ) -> None: + from launchdarkly_ai_openai_messages import create_openai_messages_handler + + config = { + **CONFIG, + "model": { + **CONFIG["model"], + "parameters": {"max_completion_tokens": 222}, + }, + } + h = create_openai_messages_handler() + await h(config, "q", {}, {}) + kwargs = mock_openai.responses.create.call_args.kwargs + assert kwargs["max_output_tokens"] == 222 + assert "max_completion_tokens" not in kwargs + + async def test_explicit_max_output_tokens_wins_over_both( + self, mock_openai: MagicMock + ) -> None: + from launchdarkly_ai_openai_messages import create_openai_messages_handler + + config = { + **CONFIG, + "model": { + **CONFIG["model"], + "parameters": { + "max_output_tokens": 333, + "max_completion_tokens": 222, + "max_tokens": 111, + }, + }, + } + h = create_openai_messages_handler() + await h(config, "q", {}, {}) + kwargs = mock_openai.responses.create.call_args.kwargs + assert kwargs["max_output_tokens"] == 333 + + async def test_max_completion_tokens_wins_over_max_tokens_when_both_set( + self, mock_openai: MagicMock + ) -> None: + from launchdarkly_ai_openai_messages import create_openai_messages_handler + + config = { + **CONFIG, + "model": { + **CONFIG["model"], + "parameters": {"max_completion_tokens": 222, "max_tokens": 111}, + }, + } + h = create_openai_messages_handler() + await h(config, "q", {}, {}) + kwargs = mock_openai.responses.create.call_args.kwargs + assert kwargs["max_output_tokens"] == 222 + + +class TestModelParametersReachTheWire: + """Intercepts the real outgoing HTTP request with an httpx MockTransport, rather than + asserting only on a mock of our own call, so this proves the renamed key actually leaves the + process on the wire the real ``openai`` client builds. + """ + + async def test_max_output_tokens_reaches_the_wire_for_a_config_max_tokens( + self, + ) -> None: + import openai + + from launchdarkly_ai_openai_messages import create_openai_messages_handler + + captured: dict[str, Any] = {} + + def _handler(request: httpx.Request) -> httpx.Response: + captured["body"] = json.loads(request.content) + return httpx.Response( + 200, + json={ + "id": "resp_1", + "object": "response", + "created_at": 1, + "status": "completed", + "model": "gpt-4o", + "output": [ + { + "type": "message", + "role": "assistant", + "content": [{"type": "output_text", "text": "hi"}], + } + ], + "usage": { + "input_tokens": 3, + "output_tokens": 2, + "total_tokens": 5, + }, + }, + ) + + transport = httpx.MockTransport(_handler) + real_client = openai.AsyncOpenAI( + api_key="test-key", http_client=httpx.AsyncClient(transport=transport) + ) + + config = { + **CONFIG, + "model": {**CONFIG["model"], "parameters": {"max_tokens": 256}}, + } + with patch("openai.AsyncOpenAI", return_value=real_client): + h = create_openai_messages_handler() + await h(config, "q", {}, {}) + + assert captured["body"]["max_output_tokens"] == 256 + assert "max_tokens" not in captured["body"] + + class TestOutputFormat: async def test_absent_output_format_no_change(self, mock_openai: MagicMock) -> None: from launchdarkly_ai_openai_messages import create_openai_messages_handler @@ -2494,3 +2835,156 @@ async def test_a_reasoning_item_with_no_summary_is_dropped( chat = rec.named("chat ")[0] assert chat.attributes["gen_ai.completion.0.content"] == "the real answer" assert "gen_ai.completion.1.content" not in chat.attributes + + +class TestNeverForwardedParameters: + """No credential, endpoint, request-injection, remote-tool, or host-process key in + ``model.parameters`` reaches ``responses.create`` or ``responses.stream``.""" + + async def test_invoke_forwards_none_of_them(self, mock_openai: MagicMock) -> None: + from launchdarkly_ai_openai_messages import create_openai_messages_handler + + config = { + **CONFIG, + "model": {**CONFIG["model"], "parameters": dict(NEVER_FORWARDED_BAG)}, + } + await create_openai_messages_handler()(config, "q", {}, {}) + kwargs = mock_openai.responses.create.call_args.kwargs + assert not find_leaks(kwargs) + assert not set(kwargs) & NEVER_FORWARDED_KEYS + + async def test_stream_forwards_none_of_them(self, mock_openai: MagicMock) -> None: + import launchdarkly_ai_openai_messages.spans as spans_mod + from launchdarkly_ai_openai_messages import create_openai_messages_handler + + mock_openai.responses.stream = MagicMock( + return_value=_make_openai_stream_context(["hi"]) + ) + config = { + **CONFIG, + "model": {**CONFIG["model"], "parameters": dict(NEVER_FORWARDED_BAG)}, + } + with patch.object(spans_mod, "_HAS_OTEL", False): + h = create_openai_messages_handler() + [e async for e in await h.stream(config, "q")] + kwargs = mock_openai.responses.stream.call_args.kwargs + assert not find_leaks(kwargs) + assert not set(kwargs) & NEVER_FORWARDED_KEYS + + +class TestInvokeAndStreamForwardTheSameKeys: + """``response_id``, ``starting_after`` and ``text_format`` are accepted by + ``responses.stream`` only. Forwarding them there and not on ``create`` made one config behave + differently by call. They resume an existing response or name a Python type to parse into, + neither of which is a setting, so both paths drop them.""" + + _PARAMS: ClassVar[dict[str, Any]] = { + "response_id": "resp-other", + "starting_after": 3, + "text_format": "not-a-type", + "top_p": 0.4, + } + + async def test_invoke(self, mock_openai: MagicMock) -> None: + from launchdarkly_ai_openai_messages import create_openai_messages_handler + + config = {**CONFIG, "model": {**CONFIG["model"], "parameters": self._PARAMS}} + await create_openai_messages_handler()(config, "q", {}, {}) + kwargs = mock_openai.responses.create.call_args.kwargs + assert not {"response_id", "starting_after", "text_format"} & set(kwargs) + assert kwargs["top_p"] == 0.4 + + async def test_stream(self, mock_openai: MagicMock) -> None: + import launchdarkly_ai_openai_messages.spans as spans_mod + from launchdarkly_ai_openai_messages import create_openai_messages_handler + + mock_openai.responses.stream = MagicMock( + return_value=_make_openai_stream_context(["hi"]) + ) + config = {**CONFIG, "model": {**CONFIG["model"], "parameters": self._PARAMS}} + with patch.object(spans_mod, "_HAS_OTEL", False): + h = create_openai_messages_handler() + [e async for e in await h.stream(config, "q")] + kwargs = mock_openai.responses.stream.call_args.kwargs + assert not {"response_id", "starting_after", "text_format"} & set(kwargs) + assert kwargs["top_p"] == 0.4 + + +class TestForwardsExactlyTheCrossSdkList: + """Probes ``invoke`` and ``stream`` one key at a time: the keys that change the provider call + are exactly the cross-SDK OpenAI Messages list, on both paths.""" + + @staticmethod + def _candidates() -> frozenset[str]: + import inspect + + from openai.resources.responses import AsyncResponses + + accepted = { + name + for fn in (AsyncResponses.create, AsyncResponses.stream) + for name in inspect.signature(fn).parameters + if name != "self" + } + return candidate_keys(accepted, OPENAI_MESSAGES) + + def _config(self, parameters: dict[str, Any]) -> dict[str, Any]: + return {**CONFIG, "model": {**CONFIG["model"], "parameters": parameters}} + + async def test_invoke(self, mock_openai: MagicMock) -> None: + from launchdarkly_ai_openai_messages import create_openai_messages_handler + + h = create_openai_messages_handler() + + async def call(parameters: dict[str, Any]) -> object: + await h(self._config(parameters), "q", {}, {}) + return mock_openai.responses.create.call_args.kwargs + + assert await probe_forwarded_keys(self._candidates(), call) == OPENAI_MESSAGES + + async def test_stream(self, mock_openai: MagicMock) -> None: + import launchdarkly_ai_openai_messages.spans as spans_mod + from launchdarkly_ai_openai_messages import create_openai_messages_handler + + mock_openai.responses.stream = MagicMock( + side_effect=lambda **_kw: _make_openai_stream_context(["hi"]) + ) + with patch.object(spans_mod, "_HAS_OTEL", False): + h = create_openai_messages_handler() + + async def call(parameters: dict[str, Any]) -> object: + [e async for e in await h.stream(self._config(parameters), "q")] + return mock_openai.responses.stream.call_args.kwargs + + assert await probe_forwarded_keys(self._candidates(), call) == OPENAI_MESSAGES + + +class TestMalformedObjectValuesAreDropped: + """``reasoning`` set to anything but an object is dropped on both paths.""" + + _PARAMS: ClassVar[dict[str, Any]] = {"reasoning": "high", "top_p": 0.4} + + def _config(self) -> dict[str, Any]: + return {**CONFIG, "model": {**CONFIG["model"], "parameters": self._PARAMS}} + + async def test_invoke(self, mock_openai: MagicMock) -> None: + from launchdarkly_ai_openai_messages import create_openai_messages_handler + + await create_openai_messages_handler()(self._config(), "q", {}, {}) + kwargs = mock_openai.responses.create.call_args.kwargs + assert "reasoning" not in kwargs + assert kwargs["top_p"] == 0.4 + + async def test_stream(self, mock_openai: MagicMock) -> None: + import launchdarkly_ai_openai_messages.spans as spans_mod + from launchdarkly_ai_openai_messages import create_openai_messages_handler + + mock_openai.responses.stream = MagicMock( + return_value=_make_openai_stream_context(["hi"]) + ) + with patch.object(spans_mod, "_HAS_OTEL", False): + h = create_openai_messages_handler() + [e async for e in await h.stream(self._config(), "q")] + kwargs = mock_openai.responses.stream.call_args.kwargs + assert "reasoning" not in kwargs + assert kwargs["top_p"] == 0.4 diff --git a/packages/openai-messages/tests/test_parameter_forwarding.py b/packages/openai-messages/tests/test_parameter_forwarding.py new file mode 100644 index 00000000..dc512c51 --- /dev/null +++ b/packages/openai-messages/tests/test_parameter_forwarding.py @@ -0,0 +1,92 @@ +""" +Drift test for the OpenAI Responses API parameter classification in ``handler.py``. + +``_RESPONSES_FORWARDED_KEYS`` is a literal, hand-maintained list, and the same list serves ``invoke`` and ``stream``. +This test reads ``AsyncResponses.create``/``.stream``'s own signatures and asserts every parameter either +accepts is classified in exactly one of forwarded, handler-owned, or excluded, so an SDK parameter +nobody has classified yet fails loudly by name. It also asserts every forwarded or handler-owned key +is accepted by both calls, so ``invoke`` and ``stream`` cannot drift apart again, and that every +excluded key is a real parameter of at least one of them. +""" + +from __future__ import annotations + +import inspect +from collections.abc import Callable + +from openai.resources.responses import AsyncResponses + +from launchdarkly_ai_openai_messages.handler import ( + _RESPONSES_EXCLUDED_KEYS, + _RESPONSES_FORWARDED_KEYS, +) + +#: Handler-owned: set by each call site itself, never taken from the config. +_OWNED_KEYS = frozenset({"model", "input", "previous_response_id", "tools", "text"}) + + +def _signature_keys(fn: Callable[..., object]) -> frozenset[str]: + keys: set[str] = set() + for name, param in inspect.signature(fn).parameters.items(): + if name == "self" or param.kind is inspect.Parameter.VAR_POSITIONAL: + continue + assert param.kind is not inspect.Parameter.VAR_KEYWORD, ( + f"{fn!r} now accepts **{name}; this test can no longer enumerate its accept-set" + ) + keys.add(name) + return frozenset(keys) + + +_CREATE_KEYS = _signature_keys(AsyncResponses.create) +_STREAM_KEYS = _signature_keys(AsyncResponses.stream) + + +class TestResponsesCreateAcceptsExactlyTheseKeys: + def test_every_accepted_key_is_classified_exactly_once(self) -> None: + classified = _RESPONSES_FORWARDED_KEYS | _OWNED_KEYS | _RESPONSES_EXCLUDED_KEYS + + unclassified = _CREATE_KEYS - classified + assert not unclassified, ( + f"AsyncResponses.create now accepts {sorted(unclassified)}, not classified as " + "forwarded, handler-owned, or excluded in openai-messages handler.py" + ) + + overlap = ( + (_RESPONSES_FORWARDED_KEYS & _OWNED_KEYS) + | (_RESPONSES_FORWARDED_KEYS & _RESPONSES_EXCLUDED_KEYS) + | (_OWNED_KEYS & _RESPONSES_EXCLUDED_KEYS) + ) + assert not overlap, f"keys classified more than once: {sorted(overlap)}" + + def test_every_forwarded_or_owned_key_is_a_real_parameter(self) -> None: + stale = (_RESPONSES_FORWARDED_KEYS | _OWNED_KEYS) - _CREATE_KEYS + assert not stale, ( + f"{sorted(stale)} forwarded or handler-owned in openai-messages handler.py but " + "AsyncResponses.create does not accept them" + ) + + +class TestResponsesStreamAcceptsExactlyTheseKeys: + def test_every_accepted_key_is_classified_exactly_once(self) -> None: + classified = _RESPONSES_FORWARDED_KEYS | _OWNED_KEYS | _RESPONSES_EXCLUDED_KEYS + + unclassified = _STREAM_KEYS - classified + assert not unclassified, ( + f"AsyncResponses.stream now accepts {sorted(unclassified)}, not classified as " + "forwarded, handler-owned, or excluded in openai-messages handler.py" + ) + + def test_every_forwarded_or_owned_key_is_a_real_parameter(self) -> None: + stale = (_RESPONSES_FORWARDED_KEYS | _OWNED_KEYS) - _STREAM_KEYS + assert not stale, ( + f"{sorted(stale)} forwarded or handler-owned in openai-messages handler.py but " + "AsyncResponses.stream does not accept them" + ) + + +def test_every_excluded_key_is_a_real_parameter() -> None: + stale = _RESPONSES_EXCLUDED_KEYS - (_CREATE_KEYS | _STREAM_KEYS) + assert not stale, ( + f"{sorted(stale)} excluded in openai-messages handler.py but neither " + "AsyncResponses.create nor .stream accepts them" + ) diff --git a/packages/opencode-agents/src/launchdarkly_ai_opencode_agents/handler.py b/packages/opencode-agents/src/launchdarkly_ai_opencode_agents/handler.py index 80ac2701..272d1a89 100644 --- a/packages/opencode-agents/src/launchdarkly_ai_opencode_agents/handler.py +++ b/packages/opencode-agents/src/launchdarkly_ai_opencode_agents/handler.py @@ -29,13 +29,29 @@ ) from launchdarkly_ai_server.agui import TurnEmitter from launchdarkly_ai_server.lifecycle import effective_capture_content +from launchdarkly_ai_server.parameter_forwarding import select_forwarded_parameters from launchdarkly_ai_server.sdk_usage import report_usage -from launchdarkly_ai_server.utils import number_or_zero +from launchdarkly_ai_server.utils import model_parameters, number_or_zero from ._version import PACKAGE_NAME, __version__ from .server import ModelResolver, default_model, session_events from .spans import fail_span, finish_root_span, mark_ok, start_root_span +#: The ``model.parameters`` keys forwarded into the ``POST /session/{id}/message`` body. None are. +#: The message route takes no sampling, length, stop, seed, tool-choice or reasoning field: OpenCode +#: reads those from its own server, agent and provider config, which the handler does not control +#: per call. The fields the route does take are not forwarded, and why: +#: * ``parts``, ``model``, ``system``, ``agent``, ``tools``: handler-owned. ``agent`` is the +#: ``create_opencode_handler(agent=...)`` argument and also picks the server's tools and +#: permissions. +#: * ``format``: switches where the answer lands (``AssistantMessage.structured``), which the +#: reducer does not read. +#: * ``noReply``: runtime wiring. No assistant turn is produced. +#: * ``messageID``: server-side session state. +#: * ``variant``: names a preset from the server's variant map, which depends on the provider, +#: the model and the operator's config, and an unknown name is silently ignored. +_PROMPT_FORWARDED_KEYS: frozenset[str] = frozenset() + class OpenCodeSessionError(Exception): """The OpenCode session failed. @@ -588,6 +604,9 @@ async def _stream_impl( model_id=model_id, agent=bound_agent, capture=capture, + parameters=select_forwarded_parameters( + model_parameters(config), _PROMPT_FORWARDED_KEYS + ), ) ) as reduced: async for event in reduced: @@ -662,6 +681,7 @@ async def _reduce_session( model_id: str, agent: str | None, capture: bool, + parameters: Mapping[str, Any], ) -> AsyncGenerator[dict[str, Any], None]: """Reduce the session ``POST /session`` created. The reducer is unchanged.""" session_ids: list[str] = [] @@ -669,6 +689,7 @@ async def _reduce_session( async with aclosing( session_events( base_url=base_url, + parameters=parameters, prompt=prompt, system=system, provider_id=provider_id, diff --git a/packages/opencode-agents/src/launchdarkly_ai_opencode_agents/server.py b/packages/opencode-agents/src/launchdarkly_ai_opencode_agents/server.py index 09b02834..eac14ec6 100644 --- a/packages/opencode-agents/src/launchdarkly_ai_opencode_agents/server.py +++ b/packages/opencode-agents/src/launchdarkly_ai_opencode_agents/server.py @@ -9,7 +9,7 @@ import asyncio import json -from collections.abc import AsyncGenerator, Callable +from collections.abc import AsyncGenerator, Callable, Mapping from typing import Any import httpx @@ -54,6 +54,7 @@ def default_model(config: dict[str, Any]) -> tuple[str, str]: async def session_events( *, base_url: str, + parameters: Mapping[str, Any], prompt: str, system: str | None, provider_id: str, @@ -89,7 +90,9 @@ async def session_events( raise OpenCodeHTTPError(f"POST /session -> {created.status_code}") session_id = _session_id(created.json()) session_ids.append(session_id) + # Forwarded parameters go first, so the handler's own fields always win. body: dict[str, Any] = { + **parameters, "parts": [{"type": "text", "text": prompt}], "model": {"providerID": provider_id, "modelID": model_id}, } diff --git a/packages/opencode-agents/tests/test_handler.py b/packages/opencode-agents/tests/test_handler.py index 74f90c48..994324c3 100644 --- a/packages/opencode-agents/tests/test_handler.py +++ b/packages/opencode-agents/tests/test_handler.py @@ -24,6 +24,12 @@ ) from launchdarkly_ai_opencode_agents.server import default_model from launchdarkly_ai_server import SDKOptions, config, configure +from tests.forwarding_spec import ( + OPENCODE_AGENTS, + candidate_keys, + probe_forwarded_keys, +) +from tests.never_forwarded import NEVER_FORWARDED_BAG, find_leaks CONTEXT = {"kind": "user", "key": "user-1"} @@ -1566,3 +1572,70 @@ def serve_post(self, path: str, body: bytes) -> tuple[int, dict[str, Any]]: async def _drain(stream: Any) -> None: async for _event in stream: pass + + +#: Every field of OpenCode's ``PromptInput`` (the message route's body), in both spellings. +_PROMPT_INPUT_FIELDS = frozenset( + { + "sessionID", + "session_id", + "messageID", + "message_id", + "model", + "agent", + "noReply", + "no_reply", + "tools", + "format", + "system", + "variant", + "parts", + } +) + + +class TestForwardsNoModelParameters: + """Probes ``invoke`` and ``stream`` one key at a time: no ``model.parameters`` key changes the + message body the handler posts, on either path.""" + + @staticmethod + def _config(parameters: dict[str, Any]) -> dict[str, Any]: + return { + "instructions": "Be brief.", + "model": {"name": "m", "parameters": parameters}, + "provider": {"name": "Acme"}, + } + + @staticmethod + def _events() -> list[dict[str, Any]]: + return [ + assistant_open("m1"), + text_delta("m1", "p1", "Hi"), + assistant_completed("m1"), + ] + + async def test_invoke(self) -> None: + async def call(parameters: dict[str, Any]) -> object: + handler, script = playing(self._events()) + await handler(self._config(parameters), "q", None, None) + return script.messages[0] + + candidates = candidate_keys(_PROMPT_INPUT_FIELDS, OPENCODE_AGENTS) + assert await probe_forwarded_keys(candidates, call) == OPENCODE_AGENTS + + async def test_stream(self) -> None: + async def call(parameters: dict[str, Any]) -> object: + handler, script = playing(self._events()) + async for _event in await handler.stream( + self._config(parameters), "q", None, None + ): + pass + return script.messages[0] + + candidates = candidate_keys(_PROMPT_INPUT_FIELDS, OPENCODE_AGENTS) + assert await probe_forwarded_keys(candidates, call) == OPENCODE_AGENTS + + async def test_never_forwarded_keys_do_not_reach_the_body(self) -> None: + handler, script = playing(self._events()) + await handler(self._config(dict(NEVER_FORWARDED_BAG)), "q", None, None) + assert not find_leaks(script.messages[0]) diff --git a/packages/vercel-agents/src/launchdarkly_ai_vercel_agents/handler.py b/packages/vercel-agents/src/launchdarkly_ai_vercel_agents/handler.py index 9089b7a0..a4b91379 100644 --- a/packages/vercel-agents/src/launchdarkly_ai_vercel_agents/handler.py +++ b/packages/vercel-agents/src/launchdarkly_ai_vercel_agents/handler.py @@ -4,7 +4,7 @@ import base64 import inspect import json -from collections.abc import AsyncGenerator, Callable +from collections.abc import AsyncGenerator, Callable, Mapping from types import SimpleNamespace from typing import Any @@ -24,6 +24,8 @@ parse_template, report_usage, ) +from launchdarkly_ai_server.parameter_forwarding import select_forwarded_parameters +from launchdarkly_ai_server.utils import model_parameters from ._version import PACKAGE_NAME, __version__ from .model_id import gateway_model_id @@ -39,69 +41,157 @@ start_tool_span, ) -OWNED_PARAMETERS = { - "model", - "messages", - "prompt", - "system", - "tools", - "stream", - "output", - "outputformat", - "stopwhen", - "maxsteps", - "apikey", - "baseurl", -} ModelSource = Any | Callable[[AiConfigRep], Any] -def _owned(name: str) -> bool: - return name.replace("_", "").lower() in OWNED_PARAMETERS +#: The ``model.parameters`` keys forwarded into ``ai.InferenceRequestParams``, after the renames in +#: :func:`_renamed_parameters`. Every other key is dropped. The handler sets ``model``, +#: ``messages``, ``tools`` and ``output_type`` itself, and runs the tool loop. +#: +#: This is the Vercel AI SDK list from TESTING.md section 1.12 as the Python ``ai`` library can +#: send it. ``stop_sequences`` is on the JS list but the Python library has no field for it. +#: ``parallel_tool_calls``, ``max_tool_calls``, ``service_tier`` and ``prompt_cache_key`` are +#: Python-only: the library takes them as typed request params, while the JS AI SDK can reach them +#: only through ``providerOptions``. ``TestInferenceRequestParamsAreClassified`` in this package's +#: tests asserts every field of ``InferenceRequestParams`` and its parts is classified. +_INFERENCE_PARAMS_FORWARDED_KEYS = frozenset( + { + "frequency_penalty", + "max_output_tokens", + "max_tool_calls", + "parallel_tool_calls", + "presence_penalty", + "prompt_cache_key", + "reasoning", + "seed", + "service_tier", + "temperature", + "tool_choice", + "top_k", + "top_p", + } +) + +#: Fields of ``ai.InferenceRequestParams`` and its parts that no config key reaches, and why: +#: * ``metadata``, ``safety_identifier``, ``tags``: identity and attribution. +#: * ``sampling`` ``min_p``, ``repetition_penalty``, ``consideration_window``: every built-in +#: provider raises on them, so a config that set one would fail every call. +#: * ``output.include``: API-shape switch. +#: * ``output.text_verbosity``: the AI Gateway raises on it. +#: * ``output.reasoning_summary``: ``reasoning`` is read for its effort only. +#: * ``cache.mode``, ``cache.retention``: the mode is provider-specific with no shared config +#: spelling, and retention is data retention. +#: * ``routing``: endpoint and routing selection. +#: * ``context_management``: server-side state. +#: * ``provider_params``: carries credentials, data-retention and timeout settings. +#: * ``extra_headers``, ``extra_query``, ``extra_body``: raw request injection. +_INFERENCE_PARAMS_EXCLUDED_FIELDS = frozenset( + { + "metadata", + "safety_identifier", + "tags", + "min_p", + "repetition_penalty", + "consideration_window", + "include", + "text_verbosity", + "reasoning_summary", + "mode", + "retention", + "routing", + "context_management", + "provider_params", + "extra_headers", + "extra_query", + "extra_body", + } +) + +#: Forwarded keys that become one sampler each: config key to the sampler class and its field. +_SAMPLERS = { + "temperature": ("TemperatureSamplerParams", "temperature"), + "top_p": ("TopPSamplerParams", "top_p"), + "top_k": ("TopKSamplerParams", "top_k"), + "seed": ("SeedSamplerParams", "seed"), +} + +#: The string ``tool_choice`` values the library's ``ToolChoiceMode`` takes. +_TOOL_CHOICE_MODES = frozenset({"auto", "none", "required"}) + + +def _renamed_parameters(params: dict[str, Any]) -> dict[str, Any]: + """Applies the section 1.12 renames: ``max_output_tokens`` wins over ``max_completion_tokens``, + which wins over ``max_tokens``, and an explicit ``reasoning`` wins over ``reasoning_effort``.""" + for alias in ("max_completion_tokens", "max_tokens"): + value = params.pop(alias, None) + if value is not None: + params.setdefault("max_output_tokens", value) + effort = params.pop("reasoning_effort", None) + if effort is not None: + params.setdefault("reasoning", effort) + return params + + +def _reasoning_effort(value: Any) -> tuple[bool, str | None]: + """The effort to send for a ``reasoning`` value, and whether to send one at all. + + ``reasoning`` is a string, or an object whose ``effort`` is read. ``"provider-default"`` + leaves it unset, and ``"none"`` becomes ``None``, the library's spelling of reasoning + disabled. Any other shape is malformed and dropped. + """ + effort = value.get("effort") if isinstance(value, Mapping) else value + if not isinstance(effort, str) or effort == "provider-default": + return False, None + return True, None if effort == "none" else effort def build_request_params(config: AiConfigRep, runtime: Any = None) -> Any: runtime = runtime or ai - raw = { - key: value - for key, value in (config.get("model", {}).get("parameters") or {}).items() - if not _owned(key) - } + params = select_forwarded_parameters( + _renamed_parameters(model_parameters(config)), _INFERENCE_PARAMS_FORWARDED_KEYS + ) + kwargs: dict[str, Any] = {} sampling: dict[type[Any], Any] = {} - mappings = { - "temperature": ("TemperatureSamplerParams", "temperature"), - "top_p": ("TopPSamplerParams", "top_p"), - "topP": ("TopPSamplerParams", "top_p"), - "top_k": ("TopKSamplerParams", "top_k"), - "topK": ("TopKSamplerParams", "top_k"), - "min_p": ("MinPSamplerParams", "min_p"), - "minP": ("MinPSamplerParams", "min_p"), - "repetition_penalty": ( - "RepetitionPenaltyParams", - "repetition_penalty", - ), - "repetitionPenalty": ( - "RepetitionPenaltyParams", - "repetition_penalty", - ), - "seed": ("SeedSamplerParams", "seed"), - } - for key, (class_name, argument) in mappings.items(): - if key in raw: + for key, (class_name, argument) in _SAMPLERS.items(): + if key in params: cls = getattr(runtime, class_name) - sampling[cls] = cls(**{argument: raw.pop(key)}) - kwargs: dict[str, Any] = {} + sampling[cls] = cls(**{argument: params[key]}) + penalties = { + key: params[key] + for key in ("frequency_penalty", "presence_penalty") + if key in params + } + if penalties: + sampling[runtime.RepetitionPenaltyParams] = runtime.RepetitionPenaltyParams( + **penalties + ) if sampling: kwargs["sampling"] = sampling - max_tokens = raw.pop("max_tokens", raw.pop("maxTokens", None)) - if max_tokens is not None: - kwargs["output"] = runtime.OutputParams(max_tokens=max_tokens) - reasoning_effort = raw.pop("reasoning_effort", raw.pop("reasoningEffort", None)) - if reasoning_effort is not None: - kwargs["reasoning"] = runtime.ReasoningParams(effort=reasoning_effort) - for direct in ("metadata", "safety_identifier"): - if direct in raw: - kwargs[direct] = raw.pop(direct) + if params.get("max_output_tokens") is not None: + kwargs["output"] = runtime.OutputParams(max_tokens=params["max_output_tokens"]) + send_effort, effort = _reasoning_effort(params.get("reasoning")) + if send_effort: + kwargs["reasoning"] = runtime.ReasoningParams(effort=effort) + tool_calling: dict[str, Any] = {} + tool_choice = params.get("tool_choice") + if isinstance(tool_choice, str) and tool_choice in _TOOL_CHOICE_MODES: + tool_calling["tool_choice"] = runtime.ToolChoiceMode(tool_choice) + if isinstance(params.get("parallel_tool_calls"), bool): + tool_calling["parallel_tool_calls"] = params["parallel_tool_calls"] + max_tool_calls = params.get("max_tool_calls") + if isinstance(max_tool_calls, int) and not isinstance(max_tool_calls, bool): + tool_calling["max_tool_calls"] = max_tool_calls + if tool_calling: + # ``ToolCallingParams`` requires a policy. ``auto`` is the library's own default when only + # the parallel or max-calls setting is given. + tool_calling.setdefault("tool_choice", runtime.ToolChoiceMode.AUTO) + kwargs["tool_calling"] = runtime.ToolCallingParams(**tool_calling) + if isinstance(params.get("service_tier"), str): + kwargs["provider_service"] = runtime.ProviderServiceParams( + service_tier=params["service_tier"] + ) + if isinstance(params.get("prompt_cache_key"), str): + kwargs["cache"] = runtime.CacheParams(key=params["prompt_cache_key"]) return runtime.InferenceRequestParams(**kwargs) diff --git a/packages/vercel-agents/tests/test_handler.py b/packages/vercel-agents/tests/test_handler.py index d153f058..36897812 100644 --- a/packages/vercel-agents/tests/test_handler.py +++ b/packages/vercel-agents/tests/test_handler.py @@ -267,6 +267,8 @@ async def test_drops_headers_query_and_provider_options( "temperature": 0.2, "metadata": {"a": 1}, "safety_identifier": "user-1", + "min_p": 0.1, + "repetition_penalty": 1.2, "extra_headers": { "Authorization": "Bearer attacker-key", "ai-language-model-id": "openai/gpt-5-pro", @@ -278,11 +280,14 @@ async def test_drops_headers_query_and_provider_options( } await create_vercel_agents_handler()(config, "hello") kwargs = ai_runtime.InferenceRequestParams.call_args.kwargs - assert kwargs["metadata"] == {"a": 1} - assert kwargs["safety_identifier"] == "user-1" + assert "metadata" not in kwargs + assert "safety_identifier" not in kwargs assert "extra_headers" not in kwargs assert "extra_query" not in kwargs assert "extra_body" not in kwargs + assert set(kwargs) == {"sampling"} + ai_runtime.MinPSamplerParams.assert_not_called() + ai_runtime.RepetitionPenaltyParams.assert_not_called() forwarded = repr(kwargs) assert "attacker-key" not in forwarded assert "gpt-5-pro" not in forwarded diff --git a/packages/vercel-agents/tests/test_parameter_forwarding.py b/packages/vercel-agents/tests/test_parameter_forwarding.py new file mode 100644 index 00000000..c7e89f08 --- /dev/null +++ b/packages/vercel-agents/tests/test_parameter_forwarding.py @@ -0,0 +1,328 @@ +"""Forwarding of ``model.parameters`` into ``ai.InferenceRequestParams``. + +A drift test classifies every field of ``InferenceRequestParams`` and its parts, value tests cover +the renames and the typed rebuilds, and a probe runs the real call site one key at a time on invoke +and stream and through the native graph. These use the real ``ai`` library with only ``ai.Agent`` +and ``ai.get_model`` replaced. +""" + +from __future__ import annotations + +import dataclasses +from typing import Any +from unittest.mock import MagicMock + +import ai +import pytest + +import launchdarkly_ai_vercel_agents.handler as handler_mod +from launchdarkly_ai_server import GraphDefinition, GraphNode +from launchdarkly_ai_vercel_agents import create_vercel_agents_handler +from launchdarkly_ai_vercel_agents.handler import ( + _INFERENCE_PARAMS_EXCLUDED_FIELDS, + _INFERENCE_PARAMS_FORWARDED_KEYS, + build_request_params, +) +from launchdarkly_ai_vercel_agents.native_graph import to_vercel_agents +from tests.forwarding_spec import VERCEL, candidate_keys, probe_forwarded_keys +from tests.never_forwarded import NEVER_FORWARDED_BAG, find_leaks + +from .test_handler import CONFIG, AgentStream + +#: ``InferenceRequestParams`` fields that group typed parts, and the part classes in each. +_CONTAINERS: dict[str, tuple[type[Any], ...]] = { + "sampling": ( + ai.TemperatureSamplerParams, + ai.TopKSamplerParams, + ai.TopPSamplerParams, + ai.MinPSamplerParams, + ai.RepetitionPenaltyParams, + ai.SeedSamplerParams, + ), + "reasoning": (ai.ReasoningParams,), + "tool_calling": (ai.ToolCallingParams,), + "provider_service": (ai.ProviderServiceParams,), + "output": (ai.OutputParams,), + "cache": (ai.CacheParams,), +} + +#: Forwarded config keys whose library field has another name. +_FIELD_FOR_KEY = { + "max_output_tokens": "max_tokens", + "prompt_cache_key": "key", + "reasoning": "effort", +} + + +def _library_fields() -> frozenset[str]: + fields = { + f.name + for f in dataclasses.fields(ai.InferenceRequestParams) + if f.name not in _CONTAINERS + } + for classes in _CONTAINERS.values(): + for cls in classes: + fields.update(f.name for f in dataclasses.fields(cls)) + return frozenset(fields) + + +class TestInferenceRequestParamsAreClassified: + def test_every_field_is_classified_exactly_once(self) -> None: + forwarded = frozenset( + _FIELD_FOR_KEY.get(key, key) for key in _INFERENCE_PARAMS_FORWARDED_KEYS + ) + unclassified = _library_fields() - forwarded - _INFERENCE_PARAMS_EXCLUDED_FIELDS + assert not unclassified, ( + f"InferenceRequestParams now has {sorted(unclassified)}, not classified as " + "forwarded or excluded in vercel-agents handler.py" + ) + overlap = forwarded & _INFERENCE_PARAMS_EXCLUDED_FIELDS + assert not overlap, f"fields classified more than once: {sorted(overlap)}" + + def test_every_classified_field_is_real(self) -> None: + forwarded = frozenset( + _FIELD_FOR_KEY.get(key, key) for key in _INFERENCE_PARAMS_FORWARDED_KEYS + ) + stale = (forwarded | _INFERENCE_PARAMS_EXCLUDED_FIELDS) - _library_fields() + assert not stale, ( + f"{sorted(stale)} classified in vercel-agents handler.py but " + "InferenceRequestParams has no such field" + ) + + +def _params(parameters: dict[str, Any]) -> Any: + return build_request_params({"model": {"name": "m", "parameters": parameters}}) + + +class TestRenames: + def test_max_output_tokens_wins_over_the_other_spellings(self) -> None: + params = _params( + {"max_output_tokens": 3, "max_completion_tokens": 2, "max_tokens": 1} + ) + assert params.output == ai.OutputParams(max_tokens=3) + + def test_max_completion_tokens_wins_over_max_tokens(self) -> None: + params = _params({"max_completion_tokens": 2, "max_tokens": 1}) + assert params.output == ai.OutputParams(max_tokens=2) + + def test_max_tokens_alone_applies(self) -> None: + assert _params({"max_tokens": 1}).output == ai.OutputParams(max_tokens=1) + + def test_reasoning_wins_over_reasoning_effort(self) -> None: + params = _params({"reasoning": "high", "reasoning_effort": "low"}) + assert params.reasoning == ai.ReasoningParams(effort="high") + + def test_reasoning_effort_alone_applies(self) -> None: + params = _params({"reasoning_effort": "low"}) + assert params.reasoning == ai.ReasoningParams(effort="low") + + def test_camel_case_spellings_are_not_read(self) -> None: + assert _params({"topP": 0.4, "maxTokens": 9}) == ai.InferenceRequestParams() + + +class TestTypedValues: + def test_penalties_share_one_sampler(self) -> None: + params = _params({"frequency_penalty": 0.3, "presence_penalty": 0.4}) + assert params.sampling == { + ai.RepetitionPenaltyParams: ai.RepetitionPenaltyParams( + frequency_penalty=0.3, presence_penalty=0.4 + ) + } + + def test_reasoning_object_is_read_for_its_effort(self) -> None: + params = _params({"reasoning": {"effort": "low", "summary": "auto"}}) + assert params.reasoning == ai.ReasoningParams(effort="low") + assert params.output is None + + def test_reasoning_none_disables_reasoning(self) -> None: + assert _params({"reasoning": "none"}).reasoning == ai.ReasoningParams( + effort=None + ) + + @pytest.mark.parametrize( + "reasoning", ["provider-default", {"summary": "auto"}, {"effort": 1}, 3] + ) + def test_unusable_reasoning_is_dropped(self, reasoning: Any) -> None: + assert _params({"reasoning": reasoning}) == ai.InferenceRequestParams() + + @pytest.mark.parametrize("mode", ["auto", "none", "required"]) + def test_tool_choice_mode_becomes_the_library_type(self, mode: str) -> None: + assert _params({"tool_choice": mode}).tool_calling == ai.ToolCallingParams( + tool_choice=ai.ToolChoiceMode(mode) + ) + + @pytest.mark.parametrize( + "tool_choice", + [{"type": "auto"}, {"type": "tool", "toolName": "lookup"}, "lookup", 1], + ) + def test_other_tool_choice_values_are_dropped(self, tool_choice: Any) -> None: + assert _params({"tool_choice": tool_choice}) == ai.InferenceRequestParams() + + def test_tool_calling_settings_without_a_tool_choice_use_auto(self) -> None: + params = _params({"parallel_tool_calls": False, "max_tool_calls": 3}) + assert params.tool_calling == ai.ToolCallingParams( + tool_choice=ai.ToolChoiceMode.AUTO, + parallel_tool_calls=False, + max_tool_calls=3, + ) + + def test_service_tier_and_prompt_cache_key(self) -> None: + params = _params({"service_tier": "flex", "prompt_cache_key": "k"}) + assert params.provider_service == ai.ProviderServiceParams(service_tier="flex") + assert params.cache == ai.CacheParams(key="k") + + +class _Agent: + """Stands in for ``ai.Agent``: records the ``params`` each run receives.""" + + calls: list[Any] + + def __init__(self, **_kwargs: Any) -> None: + pass + + def run(self, **kwargs: Any) -> AgentStream: + type(self).calls.append(kwargs["params"]) + return AgentStream() + + +@pytest.fixture +def captured(monkeypatch: pytest.MonkeyPatch) -> list[Any]: + """Every ``params`` handed to ``Agent.run``, with the real ``ai`` building them.""" + calls: list[Any] = [] + monkeypatch.setattr(_Agent, "calls", calls, raising=False) + monkeypatch.setattr(handler_mod.ai, "Agent", _Agent) + monkeypatch.setattr(handler_mod.ai, "get_model", MagicMock(return_value=object())) + return calls + + +def _config(parameters: dict[str, Any]) -> dict[str, Any]: + return {**CONFIG, "model": {**CONFIG["model"], "parameters": parameters}} + + +def _candidates() -> frozenset[str]: + return candidate_keys(_library_fields(), VERCEL) + + +#: The Python library takes ``tool_choice`` only as a mode string. +_SAMPLE_OVERRIDES = {"tool_choice": "required"} + + +class TestNeverForwardedParameters: + async def test_invoke_forwards_none_of_them(self, captured: list[Any]) -> None: + await create_vercel_agents_handler()( + _config({**NEVER_FORWARDED_BAG, "temperature": 0.1}), "q" + ) + assert captured[-1] == ai.InferenceRequestParams( + sampling={ + ai.TemperatureSamplerParams: ai.TemperatureSamplerParams( + temperature=0.1 + ) + } + ) + assert not find_leaks(captured[-1]) + + async def test_stream_forwards_none_of_them(self, captured: list[Any]) -> None: + stream = await create_vercel_agents_handler().stream( + _config({**NEVER_FORWARDED_BAG, "temperature": 0.1}), "q" + ) + async for _event in stream: + pass + assert not find_leaks(captured[-1]) + assert captured[-1].sampling + + +class TestForwardsExactlyTheVercelList: + """Probes ``invoke`` and ``stream`` one key at a time: the keys that change the request params + handed to ``Agent.run`` are exactly the Vercel list, on both paths.""" + + async def test_invoke(self, captured: list[Any]) -> None: + async def call(parameters: dict[str, Any]) -> object: + await create_vercel_agents_handler()(_config(parameters), "q") + return captured[-1] + + forwarded = await probe_forwarded_keys( + _candidates(), call, overrides=_SAMPLE_OVERRIDES + ) + assert forwarded == VERCEL + + async def test_stream(self, captured: list[Any]) -> None: + async def call(parameters: dict[str, Any]) -> object: + stream = await create_vercel_agents_handler().stream( + _config(parameters), "q" + ) + async for _event in stream: + pass + return captured[-1] + + forwarded = await probe_forwarded_keys( + _candidates(), call, overrides=_SAMPLE_OVERRIDES + ) + assert forwarded == VERCEL + + +def _single_node_graph(parameters: dict[str, Any]) -> GraphDefinition: + root = GraphNode( + key="root", + config={ + "model": {"name": "gateway/root", "parameters": parameters}, + "provider": {"name": "Gateway"}, + "instructions": "root", + }, + meta={}, + edges=[], + is_terminal=True, + ) + + async def _traverse(fn: Any, ctx: Any = None) -> None: + value = fn(root) + if hasattr(value, "__await__"): + await value + + async def _unused(*args: Any, **kwargs: Any) -> Any: + raise AssertionError("native graph must not call GraphDefinition.run_node") + + return GraphDefinition( + key="vercel-graph", + enabled=True, + root=root, + get_node=lambda key: root if key == "root" else None, + get_child_nodes=lambda key: [], + get_parent_nodes=lambda key: [], + terminal_nodes=lambda: [root], + is_terminal=lambda key: key == "root", + edges_from=lambda key: [], + run_node=_unused, + route=_unused, + traverse=_traverse, + reverse_traverse=_traverse, + ) + + +async def _awaitable(value: GraphDefinition) -> GraphDefinition: + return value + + +class TestNativeGraphForwarding: + """The native graph builds each node's request params with the handler's own builder, so a + probe of one node finds exactly the Vercel list.""" + + async def test_forwards_exactly_the_vercel_list(self, captured: list[Any]) -> None: + async def call(parameters: dict[str, Any]) -> object: + await to_vercel_agents( + _awaitable(_single_node_graph(parameters)), + opts={"model": object()}, + ).invoke("hello") + return captured[-1] + + forwarded = await probe_forwarded_keys( + _candidates(), call, overrides=_SAMPLE_OVERRIDES + ) + assert forwarded == VERCEL + + async def test_forwards_no_never_forwarded_key(self, captured: list[Any]) -> None: + await to_vercel_agents( + _awaitable(_single_node_graph({**NEVER_FORWARDED_BAG, "temperature": 0.1})), + opts={"model": object()}, + ).invoke("hello") + assert not find_leaks(captured[-1]) + assert captured[-1].sampling diff --git a/packages/vercel-messages/src/launchdarkly_ai_vercel_messages/handler.py b/packages/vercel-messages/src/launchdarkly_ai_vercel_messages/handler.py index b7acc7b4..53e310c7 100644 --- a/packages/vercel-messages/src/launchdarkly_ai_vercel_messages/handler.py +++ b/packages/vercel-messages/src/launchdarkly_ai_vercel_messages/handler.py @@ -4,7 +4,7 @@ import base64 import inspect import json -from collections.abc import AsyncGenerator, Callable +from collections.abc import AsyncGenerator, Callable, Mapping from contextlib import aclosing from typing import Any @@ -24,6 +24,8 @@ parse_template, report_usage, ) +from launchdarkly_ai_server.parameter_forwarding import select_forwarded_parameters +from launchdarkly_ai_server.utils import model_parameters from ._version import PACKAGE_NAME, __version__ from .model_id import gateway_model_id @@ -39,71 +41,156 @@ start_tool_span, ) -OWNED_PARAMETERS = { - "model", - "messages", - "prompt", - "system", - "tools", - "stream", - "output", - "outputformat", - "stopwhen", - "maxsteps", - "apikey", - "baseurl", -} - ModelSource = Any | Callable[[AiConfigRep], Any] MAX_STEPS = 10 -def _owned(name: str) -> bool: - return name.replace("_", "").lower() in OWNED_PARAMETERS +#: The ``model.parameters`` keys forwarded into ``ai.InferenceRequestParams``, after the renames in +#: :func:`_renamed_parameters`. Every other key is dropped. The handler sets ``model``, +#: ``messages``, ``tools`` and ``output_type`` itself, and runs the tool loop. +#: +#: This is the Vercel AI SDK list from TESTING.md section 1.12 as the Python ``ai`` library can +#: send it. ``stop_sequences`` is on the JS list but the Python library has no field for it. +#: ``parallel_tool_calls``, ``max_tool_calls``, ``service_tier`` and ``prompt_cache_key`` are +#: Python-only: the library takes them as typed request params, while the JS AI SDK can reach them +#: only through ``providerOptions``. ``TestInferenceRequestParamsAreClassified`` in this package's +#: tests asserts every field of ``InferenceRequestParams`` and its parts is classified. +_INFERENCE_PARAMS_FORWARDED_KEYS = frozenset( + { + "frequency_penalty", + "max_output_tokens", + "max_tool_calls", + "parallel_tool_calls", + "presence_penalty", + "prompt_cache_key", + "reasoning", + "seed", + "service_tier", + "temperature", + "tool_choice", + "top_k", + "top_p", + } +) + +#: Fields of ``ai.InferenceRequestParams`` and its parts that no config key reaches, and why: +#: * ``metadata``, ``safety_identifier``, ``tags``: identity and attribution. +#: * ``sampling`` ``min_p``, ``repetition_penalty``, ``consideration_window``: every built-in +#: provider raises on them, so a config that set one would fail every call. +#: * ``output.include``: API-shape switch. +#: * ``output.text_verbosity``: the AI Gateway raises on it. +#: * ``output.reasoning_summary``: ``reasoning`` is read for its effort only. +#: * ``cache.mode``, ``cache.retention``: the mode is provider-specific with no shared config +#: spelling, and retention is data retention. +#: * ``routing``: endpoint and routing selection. +#: * ``context_management``: server-side state. +#: * ``provider_params``: carries credentials, data-retention and timeout settings. +#: * ``extra_headers``, ``extra_query``, ``extra_body``: raw request injection. +_INFERENCE_PARAMS_EXCLUDED_FIELDS = frozenset( + { + "metadata", + "safety_identifier", + "tags", + "min_p", + "repetition_penalty", + "consideration_window", + "include", + "text_verbosity", + "reasoning_summary", + "mode", + "retention", + "routing", + "context_management", + "provider_params", + "extra_headers", + "extra_query", + "extra_body", + } +) + +#: Forwarded keys that become one sampler each: config key to the sampler class and its field. +_SAMPLERS = { + "temperature": ("TemperatureSamplerParams", "temperature"), + "top_p": ("TopPSamplerParams", "top_p"), + "top_k": ("TopKSamplerParams", "top_k"), + "seed": ("SeedSamplerParams", "seed"), +} + +#: The string ``tool_choice`` values the library's ``ToolChoiceMode`` takes. +_TOOL_CHOICE_MODES = frozenset({"auto", "none", "required"}) + + +def _renamed_parameters(params: dict[str, Any]) -> dict[str, Any]: + """Applies the section 1.12 renames: ``max_output_tokens`` wins over ``max_completion_tokens``, + which wins over ``max_tokens``, and an explicit ``reasoning`` wins over ``reasoning_effort``.""" + for alias in ("max_completion_tokens", "max_tokens"): + value = params.pop(alias, None) + if value is not None: + params.setdefault("max_output_tokens", value) + effort = params.pop("reasoning_effort", None) + if effort is not None: + params.setdefault("reasoning", effort) + return params + + +def _reasoning_effort(value: Any) -> tuple[bool, str | None]: + """The effort to send for a ``reasoning`` value, and whether to send one at all. + + ``reasoning`` is a string, or an object whose ``effort`` is read. ``"provider-default"`` + leaves it unset, and ``"none"`` becomes ``None``, the library's spelling of reasoning + disabled. Any other shape is malformed and dropped. + """ + effort = value.get("effort") if isinstance(value, Mapping) else value + if not isinstance(effort, str) or effort == "provider-default": + return False, None + return True, None if effort == "none" else effort def _request_params(config: AiConfigRep) -> Any: - raw = { - key: value - for key, value in (config.get("model", {}).get("parameters") or {}).items() - if not _owned(key) - } + params = select_forwarded_parameters( + _renamed_parameters(model_parameters(config)), _INFERENCE_PARAMS_FORWARDED_KEYS + ) + kwargs: dict[str, Any] = {} sampling: dict[type[Any], Any] = {} - mappings = { - "temperature": ("TemperatureSamplerParams", "temperature"), - "top_p": ("TopPSamplerParams", "top_p"), - "topP": ("TopPSamplerParams", "top_p"), - "top_k": ("TopKSamplerParams", "top_k"), - "topK": ("TopKSamplerParams", "top_k"), - "min_p": ("MinPSamplerParams", "min_p"), - "minP": ("MinPSamplerParams", "min_p"), - "repetition_penalty": ( - "RepetitionPenaltyParams", - "repetition_penalty", - ), - "repetitionPenalty": ( - "RepetitionPenaltyParams", - "repetition_penalty", - ), - "seed": ("SeedSamplerParams", "seed"), - } - for key, (class_name, argument) in mappings.items(): - if key in raw: + for key, (class_name, argument) in _SAMPLERS.items(): + if key in params: cls = getattr(ai, class_name) - sampling[cls] = cls(**{argument: raw.pop(key)}) - kwargs: dict[str, Any] = {} + sampling[cls] = cls(**{argument: params[key]}) + penalties = { + key: params[key] + for key in ("frequency_penalty", "presence_penalty") + if key in params + } + if penalties: + sampling[ai.RepetitionPenaltyParams] = ai.RepetitionPenaltyParams(**penalties) if sampling: kwargs["sampling"] = sampling - max_tokens = raw.pop("max_tokens", raw.pop("maxTokens", None)) - if max_tokens is not None: - kwargs["output"] = ai.OutputParams(max_tokens=max_tokens) - reasoning_effort = raw.pop("reasoning_effort", raw.pop("reasoningEffort", None)) - if reasoning_effort is not None: - kwargs["reasoning"] = ai.ReasoningParams(effort=reasoning_effort) - for direct in ("metadata", "safety_identifier"): - if direct in raw: - kwargs[direct] = raw.pop(direct) + if params.get("max_output_tokens") is not None: + kwargs["output"] = ai.OutputParams(max_tokens=params["max_output_tokens"]) + send_effort, effort = _reasoning_effort(params.get("reasoning")) + if send_effort: + kwargs["reasoning"] = ai.ReasoningParams(effort=effort) + tool_calling: dict[str, Any] = {} + tool_choice = params.get("tool_choice") + if isinstance(tool_choice, str) and tool_choice in _TOOL_CHOICE_MODES: + tool_calling["tool_choice"] = ai.ToolChoiceMode(tool_choice) + if isinstance(params.get("parallel_tool_calls"), bool): + tool_calling["parallel_tool_calls"] = params["parallel_tool_calls"] + max_tool_calls = params.get("max_tool_calls") + if isinstance(max_tool_calls, int) and not isinstance(max_tool_calls, bool): + tool_calling["max_tool_calls"] = max_tool_calls + if tool_calling: + # ``ToolCallingParams`` requires a policy. ``auto`` is the library's own default when only + # the parallel or max-calls setting is given. + tool_calling.setdefault("tool_choice", ai.ToolChoiceMode.AUTO) + kwargs["tool_calling"] = ai.ToolCallingParams(**tool_calling) + if isinstance(params.get("service_tier"), str): + kwargs["provider_service"] = ai.ProviderServiceParams( + service_tier=params["service_tier"] + ) + if isinstance(params.get("prompt_cache_key"), str): + kwargs["cache"] = ai.CacheParams(key=params["prompt_cache_key"]) return ai.InferenceRequestParams(**kwargs) diff --git a/packages/vercel-messages/tests/test_handler.py b/packages/vercel-messages/tests/test_handler.py index 0cd0f621..d4c3d8bd 100644 --- a/packages/vercel-messages/tests/test_handler.py +++ b/packages/vercel-messages/tests/test_handler.py @@ -259,6 +259,8 @@ async def test_drops_headers_query_and_provider_options( "temperature": 0.2, "metadata": {"a": 1}, "safety_identifier": "user-1", + "min_p": 0.1, + "repetition_penalty": 1.2, "extra_headers": { "Authorization": "Bearer attacker-key", "ai-language-model-id": "openai/gpt-5-pro", @@ -270,11 +272,14 @@ async def test_drops_headers_query_and_provider_options( } await create_vercel_messages_handler()(config, "hello") kwargs = ai_runtime.InferenceRequestParams.call_args.kwargs - assert kwargs["metadata"] == {"a": 1} - assert kwargs["safety_identifier"] == "user-1" + assert "metadata" not in kwargs + assert "safety_identifier" not in kwargs assert "extra_headers" not in kwargs assert "extra_query" not in kwargs assert "extra_body" not in kwargs + assert set(kwargs) == {"sampling"} + ai_runtime.MinPSamplerParams.assert_not_called() + ai_runtime.RepetitionPenaltyParams.assert_not_called() forwarded = repr(kwargs) assert "attacker-key" not in forwarded assert "gpt-5-pro" not in forwarded diff --git a/packages/vercel-messages/tests/test_parameter_forwarding.py b/packages/vercel-messages/tests/test_parameter_forwarding.py new file mode 100644 index 00000000..a4f35c9f --- /dev/null +++ b/packages/vercel-messages/tests/test_parameter_forwarding.py @@ -0,0 +1,248 @@ +"""Forwarding of ``model.parameters`` into ``ai.InferenceRequestParams``. + +A drift test classifies every field of ``InferenceRequestParams`` and its parts, value tests cover +the renames and the typed rebuilds, and a probe runs the real call site one key at a time on invoke +and stream. These use the real ``ai`` library with only ``ai.stream`` and ``ai.get_model`` replaced. +""" + +from __future__ import annotations + +import dataclasses +from typing import Any +from unittest.mock import MagicMock + +import ai +import pytest + +import launchdarkly_ai_vercel_messages.handler as handler_mod +from launchdarkly_ai_vercel_messages import create_vercel_messages_handler +from launchdarkly_ai_vercel_messages.handler import ( + _INFERENCE_PARAMS_EXCLUDED_FIELDS, + _INFERENCE_PARAMS_FORWARDED_KEYS, + _request_params, +) +from tests.forwarding_spec import VERCEL, candidate_keys, probe_forwarded_keys +from tests.never_forwarded import NEVER_FORWARDED_BAG, find_leaks + +from .test_handler import CONFIG, FakeStream + +#: ``InferenceRequestParams`` fields that group typed parts, and the part classes in each. +_CONTAINERS: dict[str, tuple[type[Any], ...]] = { + "sampling": ( + ai.TemperatureSamplerParams, + ai.TopKSamplerParams, + ai.TopPSamplerParams, + ai.MinPSamplerParams, + ai.RepetitionPenaltyParams, + ai.SeedSamplerParams, + ), + "reasoning": (ai.ReasoningParams,), + "tool_calling": (ai.ToolCallingParams,), + "provider_service": (ai.ProviderServiceParams,), + "output": (ai.OutputParams,), + "cache": (ai.CacheParams,), +} + +#: Forwarded config keys whose library field has another name. +_FIELD_FOR_KEY = { + "max_output_tokens": "max_tokens", + "prompt_cache_key": "key", + "reasoning": "effort", +} + + +def _library_fields() -> frozenset[str]: + fields = { + f.name + for f in dataclasses.fields(ai.InferenceRequestParams) + if f.name not in _CONTAINERS + } + for classes in _CONTAINERS.values(): + for cls in classes: + fields.update(f.name for f in dataclasses.fields(cls)) + return frozenset(fields) + + +class TestInferenceRequestParamsAreClassified: + def test_every_field_is_classified_exactly_once(self) -> None: + forwarded = frozenset( + _FIELD_FOR_KEY.get(key, key) for key in _INFERENCE_PARAMS_FORWARDED_KEYS + ) + unclassified = _library_fields() - forwarded - _INFERENCE_PARAMS_EXCLUDED_FIELDS + assert not unclassified, ( + f"InferenceRequestParams now has {sorted(unclassified)}, not classified as " + "forwarded or excluded in vercel-messages handler.py" + ) + overlap = forwarded & _INFERENCE_PARAMS_EXCLUDED_FIELDS + assert not overlap, f"fields classified more than once: {sorted(overlap)}" + + def test_every_classified_field_is_real(self) -> None: + forwarded = frozenset( + _FIELD_FOR_KEY.get(key, key) for key in _INFERENCE_PARAMS_FORWARDED_KEYS + ) + stale = (forwarded | _INFERENCE_PARAMS_EXCLUDED_FIELDS) - _library_fields() + assert not stale, ( + f"{sorted(stale)} classified in vercel-messages handler.py but " + "InferenceRequestParams has no such field" + ) + + +def _params(parameters: dict[str, Any]) -> Any: + return _request_params({"model": {"name": "m", "parameters": parameters}}) + + +class TestRenames: + def test_max_output_tokens_wins_over_the_other_spellings(self) -> None: + params = _params( + {"max_output_tokens": 3, "max_completion_tokens": 2, "max_tokens": 1} + ) + assert params.output == ai.OutputParams(max_tokens=3) + + def test_max_completion_tokens_wins_over_max_tokens(self) -> None: + params = _params({"max_completion_tokens": 2, "max_tokens": 1}) + assert params.output == ai.OutputParams(max_tokens=2) + + def test_max_tokens_alone_applies(self) -> None: + assert _params({"max_tokens": 1}).output == ai.OutputParams(max_tokens=1) + + def test_reasoning_wins_over_reasoning_effort(self) -> None: + params = _params({"reasoning": "high", "reasoning_effort": "low"}) + assert params.reasoning == ai.ReasoningParams(effort="high") + + def test_reasoning_effort_alone_applies(self) -> None: + params = _params({"reasoning_effort": "low"}) + assert params.reasoning == ai.ReasoningParams(effort="low") + + def test_camel_case_spellings_are_not_read(self) -> None: + assert _params({"topP": 0.4, "maxTokens": 9}) == ai.InferenceRequestParams() + + +class TestTypedValues: + def test_penalties_share_one_sampler(self) -> None: + params = _params({"frequency_penalty": 0.3, "presence_penalty": 0.4}) + assert params.sampling == { + ai.RepetitionPenaltyParams: ai.RepetitionPenaltyParams( + frequency_penalty=0.3, presence_penalty=0.4 + ) + } + + def test_reasoning_object_is_read_for_its_effort(self) -> None: + params = _params({"reasoning": {"effort": "low", "summary": "auto"}}) + assert params.reasoning == ai.ReasoningParams(effort="low") + assert params.output is None + + def test_reasoning_none_disables_reasoning(self) -> None: + assert _params({"reasoning": "none"}).reasoning == ai.ReasoningParams( + effort=None + ) + + @pytest.mark.parametrize( + "reasoning", ["provider-default", {"summary": "auto"}, {"effort": 1}, 3] + ) + def test_unusable_reasoning_is_dropped(self, reasoning: Any) -> None: + assert _params({"reasoning": reasoning}) == ai.InferenceRequestParams() + + @pytest.mark.parametrize("mode", ["auto", "none", "required"]) + def test_tool_choice_mode_becomes_the_library_type(self, mode: str) -> None: + assert _params({"tool_choice": mode}).tool_calling == ai.ToolCallingParams( + tool_choice=ai.ToolChoiceMode(mode) + ) + + @pytest.mark.parametrize( + "tool_choice", + [{"type": "auto"}, {"type": "tool", "toolName": "lookup"}, "lookup", 1], + ) + def test_other_tool_choice_values_are_dropped(self, tool_choice: Any) -> None: + assert _params({"tool_choice": tool_choice}) == ai.InferenceRequestParams() + + def test_tool_calling_settings_without_a_tool_choice_use_auto(self) -> None: + params = _params({"parallel_tool_calls": False, "max_tool_calls": 3}) + assert params.tool_calling == ai.ToolCallingParams( + tool_choice=ai.ToolChoiceMode.AUTO, + parallel_tool_calls=False, + max_tool_calls=3, + ) + + def test_service_tier_and_prompt_cache_key(self) -> None: + params = _params({"service_tier": "flex", "prompt_cache_key": "k"}) + assert params.provider_service == ai.ProviderServiceParams(service_tier="flex") + assert params.cache == ai.CacheParams(key="k") + + +@pytest.fixture +def captured(monkeypatch: pytest.MonkeyPatch) -> list[Any]: + """Every ``params`` handed to ``ai.stream``, with the real ``ai`` building them.""" + calls: list[Any] = [] + + def stream(**kwargs: Any) -> FakeStream: + calls.append(kwargs["params"]) + return FakeStream() + + monkeypatch.setattr(handler_mod.ai, "stream", stream) + monkeypatch.setattr(handler_mod.ai, "get_model", MagicMock(return_value=object())) + return calls + + +def _config(parameters: dict[str, Any]) -> dict[str, Any]: + return {**CONFIG, "model": {**CONFIG["model"], "parameters": parameters}} + + +def _candidates() -> frozenset[str]: + return candidate_keys(_library_fields(), VERCEL) + + +#: The Python library takes ``tool_choice`` only as a mode string. +_SAMPLE_OVERRIDES = {"tool_choice": "required"} + + +class TestNeverForwardedParameters: + async def test_invoke_forwards_none_of_them(self, captured: list[Any]) -> None: + await create_vercel_messages_handler()( + _config({**NEVER_FORWARDED_BAG, "temperature": 0.1}), "q" + ) + assert captured[-1] == ai.InferenceRequestParams( + sampling={ + ai.TemperatureSamplerParams: ai.TemperatureSamplerParams( + temperature=0.1 + ) + } + ) + assert not find_leaks(captured[-1]) + + async def test_stream_forwards_none_of_them(self, captured: list[Any]) -> None: + stream = await create_vercel_messages_handler().stream( + _config({**NEVER_FORWARDED_BAG, "temperature": 0.1}), "q" + ) + async for _event in stream: + pass + assert not find_leaks(captured[-1]) + assert captured[-1].sampling + + +class TestForwardsExactlyTheVercelList: + """Probes ``invoke`` and ``stream`` one key at a time: the keys that change the request params + handed to ``ai.stream`` are exactly the Vercel list, on both paths.""" + + async def test_invoke(self, captured: list[Any]) -> None: + async def call(parameters: dict[str, Any]) -> object: + await create_vercel_messages_handler()(_config(parameters), "q") + return captured[-1] + + forwarded = await probe_forwarded_keys( + _candidates(), call, overrides=_SAMPLE_OVERRIDES + ) + assert forwarded == VERCEL + + async def test_stream(self, captured: list[Any]) -> None: + async def call(parameters: dict[str, Any]) -> object: + stream = await create_vercel_messages_handler().stream( + _config(parameters), "q" + ) + async for _event in stream: + pass + return captured[-1] + + forwarded = await probe_forwarded_keys( + _candidates(), call, overrides=_SAMPLE_OVERRIDES + ) + assert forwarded == VERCEL diff --git a/tests/forwarding_spec.py b/tests/forwarding_spec.py new file mode 100644 index 00000000..2b11ae00 --- /dev/null +++ b/tests/forwarding_spec.py @@ -0,0 +1,316 @@ +"""The cross-SDK forwarded-key lists (TESTING.md section 1.12), shared by every package's tests. + +Each list names the ``model.parameters`` keys a handler forwards, in the config's own spelling, so +a key the handler renames (``effort``, ``max_tokens``, ``text``) is listed under the name the config +uses. A key the Python library cannot accept is marked not supported in Python in section 1.12 and +is absent here, and the JS SDK's lists differ the same way. Each package's tests probe its real call +sites with :func:`probe_forwarded_keys` and assert the result equals its list here exactly. +""" + +from __future__ import annotations + +from collections.abc import Awaitable, Callable, Iterable, Mapping +from typing import Any + +from tests.never_forwarded import NEVER_FORWARDED_KEYS, leaked + +CLAUDE_MESSAGES = frozenset( + { + "cache_control", + "effort", + "max_tokens", + "output_config", + "service_tier", + "stop_sequences", + "temperature", + "thinking", + "tool_choice", + "top_k", + "top_p", + } +) + +OPENAI_MESSAGES = frozenset( + { + "max_output_tokens", + "max_completion_tokens", + "max_tokens", + "max_tool_calls", + "parallel_tool_calls", + "prompt_cache_key", + "reasoning", + "service_tier", + "temperature", + "tool_choice", + "top_logprobs", + "top_p", + } +) + +CLAUDE_AGENTS = frozenset( + { + "betas", + "effort", + "fallback_model", + "max_budget_usd", + "max_thinking_tokens", + "max_turns", + "output_format", + "thinking", + } +) + +OPENAI_AGENTS = frozenset( + { + "frequency_penalty", + "max_tokens", + "max_turns", + "parallel_tool_calls", + "presence_penalty", + "reasoning", + "temperature", + "text", + "tool_choice", + "top_p", + "verbosity", + } +) + +LITELLM_MESSAGES = frozenset( + { + "frequency_penalty", + "max_completion_tokens", + "max_tokens", + "parallel_tool_calls", + "presence_penalty", + "reasoning_effort", + "seed", + "stop", + "temperature", + "tool_choice", + "top_p", + } +) + +LITELLM_AGENTS = frozenset( + { + "frequency_penalty", + "max_tokens", + "max_turns", + "parallel_tool_calls", + "presence_penalty", + "reasoning", + "temperature", + "tool_choice", + "top_p", + } +) + +#: The Vercel AI SDK list as the Python ``ai`` library sends it, for both Vercel handlers and the +#: vercel-agents native graph. It drops ``stop_sequences``, which the Python library has no field +#: for, and adds ``max_tool_calls``, ``parallel_tool_calls``, ``prompt_cache_key`` and +#: ``service_tier``, which it takes as typed request params. +VERCEL = frozenset( + { + "frequency_penalty", + "max_completion_tokens", + "max_output_tokens", + "max_tokens", + "max_tool_calls", + "parallel_tool_calls", + "presence_penalty", + "prompt_cache_key", + "reasoning", + "reasoning_effort", + "seed", + "service_tier", + "temperature", + "tool_choice", + "top_k", + "top_p", + } +) + +#: OpenCode's message route takes no model setting, so the Python-only OpenCode handler forwards +#: nothing. +OPENCODE_AGENTS: frozenset[str] = frozenset() + +LANGCHAIN_CHAT_OPENAI = frozenset( + { + "frequency_penalty", + "logit_bias", + "logprobs", + "max_completion_tokens", + "max_tokens", + "n", + "presence_penalty", + "reasoning_effort", + "seed", + "service_tier", + "stop", + "stop_sequences", + "temperature", + "top_logprobs", + "top_p", + "verbosity", + } +) + +LANGCHAIN_CHAT_ANTHROPIC = frozenset( + { + "betas", + "effort", + "max_tokens", + "max_tokens_to_sample", + "output_config", + "stop", + "stop_sequences", + "temperature", + "thinking", + "top_k", + "top_p", + } +) + +LANGCHAIN_CHAT_BEDROCK_CONVERSE = frozenset( + { + "max_tokens", + "performance_config", + "reasoning_effort", + "service_tier", + "stop", + "stop_sequences", + "temperature", + "top_p", + } +) + +#: Keys the UI or another provider uses that are on none of the lists above for some handler, so +#: every probe also checks they are dropped. +_OFF_LIST_KEYS = frozenset( + { + "context_management", + "frequency_penalty", + "guard_last_turn_only", + "guardrails", + "include", + "include_usage", + "inference_geo", + "instructions", + "logit_bias", + "logprobs", + "max_output_tokens", + "max_tokens", + "metadata", + "moderation", + "n", + "output_format", + "prompt_cache_retention", + "reasoning", + "reasoning_effort", + "request_metadata", + "response_format", + "response_include", + "safety_identifier", + "seed", + "stop", + "store", + "stream", + "system", + "text", + "top_logprobs", + "truncation", + "use_responses_api", + "user", + "user_profile_id", + "verbose", + "verbosity", + } +) + +#: A valid value for every canonical key, different from any default a handler applies, so +#: setting it alone always changes the provider call when the key is forwarded. +SAMPLE_VALUES: Mapping[str, Any] = { + "betas": ["context-1m-2025-08-07"], + "cache_control": {"type": "ephemeral"}, + "effort": "low", + "fallback_model": "claude-fallback", + "frequency_penalty": 0.3, + "logit_bias": {"50256": -100}, + "logprobs": True, + "max_budget_usd": 1.5, + "max_completion_tokens": 222, + "max_output_tokens": 333, + "max_thinking_tokens": 444, + "max_tokens": 111, + "max_tokens_to_sample": 555, + "max_tool_calls": 3, + "max_turns": 4, + "n": 2, + "output_config": {"effort": "high"}, + "output_format": {"type": "json_schema", "schema": {"type": "object"}}, + "parallel_tool_calls": False, + "performance_config": {"latency": "optimized"}, + "presence_penalty": 0.4, + "prompt_cache_key": "cache-1", + "reasoning": {"effort": "low"}, + "reasoning_effort": "low", + "seed": 42, + "service_tier": "flex", + "stop": ["STOP"], + "stop_sequences": ["END"], + "temperature": 0.2, + "text": {"verbosity": "low"}, + "thinking": {"type": "enabled", "budget_tokens": 1024}, + "tool_choice": {"type": "auto"}, + "top_k": 7, + "top_logprobs": 2, + "top_p": 0.6, + "verbosity": "high", +} + + +def candidate_keys(*accepted: Iterable[str]) -> frozenset[str]: + """Every key worth probing: the provider's own accept-set(s), every canonical key, every + never-forwarded key, and the off-list keys.""" + keys: set[str] = set(_OFF_LIST_KEYS | NEVER_FORWARDED_KEYS) + for group in accepted: + keys.update(group) + return frozenset(keys) + + +def sample(key: str, overrides: Mapping[str, Any] | None = None) -> Any: + """The probe value for *key*: *overrides*' value when it has one, else a valid value for a + canonical key, else a :func:`leaked` marker.""" + if overrides is not None and key in overrides: + return overrides[key] + return SAMPLE_VALUES[key] if key in SAMPLE_VALUES else leaked(key) + + +async def probe_forwarded_keys( + candidates: Iterable[str], + call: Callable[[dict[str, Any]], Awaitable[object]], + *, + overrides: Mapping[str, Any] | None = None, +) -> frozenset[str]: + """The keys among *candidates* that change what *call* hands its provider when set alone. + + *call* runs the handler with the given ``model.parameters`` and returns whatever it handed the + provider. A key counts as forwarded when that result differs from the result with no + parameters, or when setting it makes the call raise. + + *overrides* replaces a :data:`SAMPLE_VALUES` entry for a handler whose library takes a + different valid shape for that key, such as a string ``tool_choice`` where another handler + takes an object. + """ + baseline = await call({}) + forwarded: set[str] = set() + for key in sorted(set(candidates)): + try: + result = await call({key: sample(key, overrides)}) + except Exception: + forwarded.add(key) + continue + if result != baseline: + forwarded.add(key) + return frozenset(forwarded) diff --git a/tests/never_forwarded.py b/tests/never_forwarded.py new file mode 100644 index 00000000..de7a62e0 --- /dev/null +++ b/tests/never_forwarded.py @@ -0,0 +1,158 @@ +"""The keys no handler may ever forward from ``model.parameters``, shared by every package's tests. + +Each handler forwards only an explicit allowlist of model and run settings and drops every other +key. This module is the other half of that rule: the keys that must never be on any allowlist, +whatever the provider SDK accepts. ``test_never_forwarded_parameters.py`` checks every handler's +forwarded lists against it, and each package's own tests check their real call sites with +:data:`NEVER_FORWARDED_BAG`. +""" + +from __future__ import annotations + +#: Credentials, endpoints and connection settings, raw request injection, remote tools, the Claude +#: Agents host-process settings, and the categories no cross-SDK allowlist (TESTING.md section 1.12) +#: may hold: data retention, server-side state, identity and attribution, runtime wiring and +#: API-shape switches, prompt content beyond the instructions, and safety configuration. +NEVER_FORWARDED_KEYS = frozenset( + { + # Credentials. + "api_key", + "openai_api_key", + "anthropic_api_key", + "bedrock_api_key", + "credentials", + "credentials_profile_name", + "auth_token", + "organization", + "aws_access_key_id", + "aws_secret_access_key", + "aws_session_token", + "aws_region", + "aws_region_name", + "vertex_credentials", + # Endpoints and connection. + "base_url", + "openai_api_base", + "anthropic_api_url", + "endpoint", + "endpoint_url", + "region", + "region_name", + "inference_geo", + "client_options", + "client", + "async_client", + "http_client", + "http_async_client", + "default_headers", + "default_query", + "timeout", + "request_timeout", + "default_request_timeout", + "max_retries", + "retry", + "proxies", + "openai_proxy", + "anthropic_proxy", + "api_base", + "api_version", + "deployment_id", + "custom_llm_provider", + "model_list", + "fallbacks", + "num_retries", + "shared_session", + # Request injection. + "headers", + "extra_headers", + "extra_body", + "extra_query", + "extra_args", + "model_kwargs", + "provider_data", + "additional_model_request_fields", + "mock_response", + # Remote tools. + "mcp_servers", + # Data retention. + "store", + "prompt_cache_retention", + # Server-side state. + "context_management", + "truncation", + # Identity and attribution. + "metadata", + "user", + "safety_identifier", + "request_metadata", + # Runtime wiring and API-shape switches. + "include", + "include_usage", + "response_include", + # Prompt content beyond the instructions. + "instructions", + "system", + # Safety configuration. + "moderation", + "guardrails", + "guardrail_config", + "guard_last_turn_only", + # Claude Agents host-process settings. + "cli_path", + "env", + "cwd", + "add_dirs", + "permission_mode", + "settings", + "setting_sources", + "plugins", + "sandbox", + "resume", + "session_id", + "fork_session", + "continue_conversation", + "hooks", + "can_use_tool", + "stderr", + "session_store", + } +) + + +def leaked(key: str) -> str: + """A value no handler sets itself, so finding it anywhere in a provider call means *key* + leaked through from the config.""" + return f"leaked-from-config:{key}" + + +#: ``model.parameters`` holding every never-forwarded key, each set to its own :func:`leaked` +#: marker. +NEVER_FORWARDED_BAG = {key: leaked(key) for key in NEVER_FORWARDED_KEYS} + + +def find_leaks(value: object) -> set[str]: + """Every :func:`leaked` marker found anywhere inside *value* (dicts, lists, tuples, and object + attributes are searched).""" + found: set[str] = set() + seen: set[int] = set() + + def walk(v: object) -> None: + if id(v) in seen: + return + seen.add(id(v)) + if isinstance(v, str): + if v.startswith("leaked-from-config:"): + found.add(v.split(":", 1)[1]) + elif isinstance(v, dict): + for k, item in v.items(): + walk(k) + walk(item) + elif isinstance(v, (list, tuple, set, frozenset)): + for item in v: + walk(item) + elif hasattr(v, "__dict__") and not isinstance(v, type): + for item in vars(v).values(): + walk(item) + + walk(value) + return found diff --git a/tests/test_never_forwarded_parameters.py b/tests/test_never_forwarded_parameters.py new file mode 100644 index 00000000..6d37504a --- /dev/null +++ b/tests/test_never_forwarded_parameters.py @@ -0,0 +1,156 @@ +"""Cross-handler invariant: no handler forwards a never-forwarded key from ``model.parameters``. + +Each handler forwards only its own literal allowlist of model and run settings. This checks every +one of those lists against the shared list of keys that must never be forwarded +(``never_forwarded.py``). Each package's own tests check its real call sites with the same keys. + +Every handler module in ``packages/*/src/launchdarkly_ai_*`` must be listed in ``FORWARDED_LISTS``, +so a new handler package fails here until it declares an allowlist. +""" + +from __future__ import annotations + +import ast +import importlib +from pathlib import Path + +import pytest + +from launchdarkly_ai_server.parameter_forwarding import select_forwarded_parameters +from tests.never_forwarded import ( + NEVER_FORWARDED_BAG, + NEVER_FORWARDED_KEYS, + find_leaks, + leaked, +) + +#: Handler module to the forwarded lists it defines. The lists are named rather than discovered so +#: that a list renamed or added without being checked here fails below by name; the modules are +#: checked against ``discover_handler_modules`` so a new handler module must be added here. +FORWARDED_LISTS: dict[str, set[str]] = { + "launchdarkly_ai_claude_agents.handler": {"_CLAUDE_AGENT_OPTIONS_FORWARDED_KEYS"}, + "launchdarkly_ai_claude_messages.handler": {"_MESSAGES_FORWARDED_KEYS"}, + "launchdarkly_ai_openai_agents.handler": {"_MODEL_SETTINGS_FORWARDED_KEYS"}, + "launchdarkly_ai_openai_messages.handler": {"_RESPONSES_FORWARDED_KEYS"}, + "launchdarkly_ai_litellm_agents.handler": {"_MODEL_SETTINGS_FORWARDED_KEYS"}, + "launchdarkly_ai_litellm_messages.handler": {"_COMPLETION_FORWARDED_KEYS"}, + "launchdarkly_ai_vercel_agents.handler": {"_INFERENCE_PARAMS_FORWARDED_KEYS"}, + "launchdarkly_ai_vercel_messages.handler": {"_INFERENCE_PARAMS_FORWARDED_KEYS"}, + "launchdarkly_ai_opencode_agents.handler": {"_PROMPT_FORWARDED_KEYS"}, + "launchdarkly_ai_langchain_agents.handler": { + "_CHAT_OPENAI_FORWARDED_KEYS", + "_CHAT_ANTHROPIC_FORWARDED_KEYS", + "_CHAT_BEDROCK_CONVERSE_FORWARDED_KEYS", + }, + "launchdarkly_ai_langchain_messages.handler": { + "_CHAT_OPENAI_FORWARDED_KEYS", + "_CHAT_ANTHROPIC_FORWARDED_KEYS", + "_CHAT_BEDROCK_CONVERSE_FORWARDED_KEYS", + }, +} + +_PACKAGES_DIR = Path(__file__).resolve().parent.parent / "packages" + + +def _module_name(path: Path, src: Path) -> str: + """``src/launchdarkly_ai_x/handler.py`` to ``launchdarkly_ai_x.handler``.""" + return ".".join(path.relative_to(src).with_suffix("").parts).removesuffix( + ".__init__" + ) + + +def _provides_handler(tree: ast.Module) -> bool: + """A function returning ``ProviderHandler``, or a ``create_handler``/``ProviderHandler`` call.""" + for node in ast.walk(tree): + if ( + isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)) + and node.returns is not None + and ast.unparse(node.returns).split(".")[-1] == "ProviderHandler" + ): + return True + if isinstance(node, ast.Call): + func = node.func + name = ( + func.attr + if isinstance(func, ast.Attribute) + else getattr(func, "id", None) + ) + if name in {"create_handler", "ProviderHandler"}: + return True + return False + + +def _defines_provider_handler(trees: dict[Path, ast.Module]) -> bool: + return any( + isinstance(node, ast.ClassDef) and node.name == "ProviderHandler" + for tree in trees.values() + for node in ast.walk(tree) + ) + + +def discover_handler_modules(packages_dir: Path = _PACKAGES_DIR) -> set[str]: + """Modules under ``packages/*/src/launchdarkly_ai_*`` that build a ``ProviderHandler``. + + Read from source rather than imported, so a package whose provider SDK isn't installed is + still found. The package that defines ``ProviderHandler`` itself is the client core, not a + handler, and is skipped along with packages that build no handler (the meta-package). + """ + found: set[str] = set() + for package in sorted(packages_dir.glob("*/src/launchdarkly_ai_*")): + trees = { + path: ast.parse(path.read_text()) for path in sorted(package.rglob("*.py")) + } + if _defines_provider_handler(trees): + continue + found |= { + _module_name(path, package.parent) + for path, tree in trees.items() + if _provides_handler(tree) + } + return found + + +def test_every_handler_module_has_forwarded_lists() -> None: + discovered = discover_handler_modules() + assert discovered, f"found no handler modules under {_PACKAGES_DIR}" + assert discovered <= FORWARDED_LISTS.keys(), ( + f"handler modules with no entry in FORWARDED_LISTS: {sorted(discovered - FORWARDED_LISTS.keys())}" + ) + assert FORWARDED_LISTS.keys() <= discovered, ( + f"FORWARDED_LISTS names modules that build no handler: {sorted(FORWARDED_LISTS.keys() - discovered)}" + ) + + +_CASES = [ + (module, name) + for module, names in FORWARDED_LISTS.items() + for name in sorted(names) +] + + +@pytest.mark.parametrize("module", sorted(FORWARDED_LISTS)) +def test_every_forwarded_list_is_checked(module: str) -> None: + mod = importlib.import_module(module) + defined = {name for name in vars(mod) if name.endswith("_FORWARDED_KEYS")} + assert defined == FORWARDED_LISTS[module], ( + f"{module} defines forwarded lists {sorted(defined)}; " + f"this test checks {sorted(FORWARDED_LISTS[module])}" + ) + + +@pytest.mark.parametrize(("module", "name"), _CASES) +def test_no_forwarded_list_holds_a_never_forwarded_key(module: str, name: str) -> None: + forwarded: frozenset[str] = getattr(importlib.import_module(module), name) + assert not forwarded & NEVER_FORWARDED_KEYS, ( + f"{module}.{name} forwards {sorted(forwarded & NEVER_FORWARDED_KEYS)}" + ) + assert select_forwarded_parameters(NEVER_FORWARDED_BAG, forwarded) == {} + + +def test_find_leaks_finds_a_marker_at_any_depth() -> None: + class _Holder: + def __init__(self) -> None: + self.options = {"nested": [("x", leaked("cli_path"))]} + + assert find_leaks({"a": _Holder(), "b": leaked("env")}) == {"cli_path", "env"} + assert find_leaks({"a": "fine", "b": [1, 2]}) == set()