Repository navigation
Conversation
model.parameters is a free-form dict the LaunchDarkly UI writes for
provider tuning values, including max_turns for agent turn caps. Every
Python handler except claude-messages ignored it entirely, and
claude-messages read only max_tokens. In particular there was no way
to cap an agent's turns: claude-agents never set
ClaudeAgentOptions.max_turns and openai-agents never passed max_turns
to Runner.run.
Adds a shared launchdarkly_ai_server.model_parameters(config) helper
that returns model.parameters as a fresh dict (or {} when absent/not
a mapping), and never reads model.custom. Applies it at every provider
call site across all six handler packages, including streaming and
native-graph paths. Keys are forwarded as-is (snake_case, matching
both the UI and every Python provider SDK here); no allowlisting and
no case conversion.
Handler-owned keys (model, messages/input, system/instructions, tools,
etc., decided per call site) are always removed from the forwarded
dict before merging with the handler's own kwargs, so a config value
can never override what the handler itself sets. claude-messages keeps
its max_tokens-defaults-to-1024 behaviour exactly.
LangChain's handlers already forwarded model.parameters through a
local _model_constructor_kwargs helper; swapped that to use the new
shared helper for consistency.
Verified: uv run pytest (1332 passed, 11 skipped), uv run mypy
packages/*/src, uv run ruff check ., uv run ruff format --check .
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
apucacao
changed the title
|
bugbot run |
…ccepts model.parameters is a free-form dict the LaunchDarkly UI writes, offering keys that make sense across providers but that no single provider SDK accepts in full. Forwarding it unfiltered (added in a prior commit on this branch) raised TypeError before any request for several real configs: ClaudeAgentOptions has no temperature/top_p/top_k/max_tokens/ stop_sequences/tool_choice/metadata; the OpenAI Responses API has no max_tokens/frequency_penalty/presence_penalty/seed/n/stop/response_format/ logit_bias/logprobs/max_completion_tokens/audio/modalities/prediction; Anthropic's Messages API has no top-level effort. Adds launchdarkly_ai_server.parameter_forwarding, a shared filter used by every handler: accept-sets are derived once from the live provider type (inspect.signature for plain methods, dataclasses.fields for ClaudeAgentOptions/ModelSettings, pydantic model_fields + aliases for the LangChain chat models), never hand-listed, so an SDK's own drift is picked up automatically. transport/escape-hatch keys (extra_headers, extra_query, extra_body, timeout, extra_*) are stripped unconditionally regardless of what a signature accepts, and each call site may additionally exclude keys the API accepts but that would break the handler because the handler itself already decides that behaviour: stream/stream_options/background/ conversation/prompt for the OpenAI Responses API, stream for Anthropic Messages. Everything else the provider accepts is forwarded, including keys that are not strictly generation settings (store, user, safety_identifier, prompt_cache_key, prompt_cache_retention, include, context_management, metadata, service_tier, instructions, moderation). Two renames, applied before the accept filter: openai-messages maps max_tokens/max_completion_tokens to the Responses API's max_output_tokens (explicit max_output_tokens wins, then max_completion_tokens, then max_tokens); claude-messages maps a top-level effort into output_config.effort, unless the config already sets its own output_config.effort, which wins. Applied at every call site across all six handlers, including streaming and native-graph paths. claude-agents/openai-agents dataclass accept-sets are computed once at module load from the real SDK types, independent of each package's per-call importlib.import_module mocking in tests. LangChain's per-provider pydantic accept-sets are cached per class with functools.cache, since the constructor class is only known once a provider branch resolves langchain_openai/_anthropic/_aws. Updated the existing tests whose old assertions relied on unfiltered pass-through of a key the real SDK does not accept (tools forwarded to ChatOpenAI's constructor in two langchain-agents/messages tests) to give the mocked provider class an accurate model_fields shape and to reflect the value now being dropped. Verified: uv run pytest (1383 passed, 11 skipped), uv run mypy packages/*/src, uv run ruff check ., uv run ruff format --check .. uv.lock unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
bugbot run |
…warded-key lists Every handler decided which model.parameters keys to forward by introspecting the live provider SDK at runtime: inspect.signature for the two Anthropic/OpenAI methods, dataclasses.fields for ClaudeAgentOptions/ModelSettings, pydantic model_fields plus aliases for the three LangChain chat models. Every competitor SDK (LiteLLM, Vercel AI SDK, Braintrust, LangChain) and this SDK's own TypeScript port use a written-down list instead, so replaces the derivation with one: each handler now carries a literal frozenset of the keys it forwards, generated once from the same introspection and pasted in as a reviewable list, next to its existing handler-owned and excluded sets. Deletes accepted_parameter_keys_from_signature/_from_dataclass/_from_pydantic_model, _alias_strings, is_transport_parameter, strip_transport_parameters, and filter_forwardable_parameters from packages/client's parameter_forwarding module now that nothing calls them. In their place, select_forwarded_parameters(params, keys) is the one shared piece left: keep the params keys that are on a handler's own forwarded list, drop everything else. Reproduces every handler's existing forwarded/owned/excluded classification exactly, with one deliberate addition applied consistently: client/connection configuration (API keys, base URLs, organization ids, HTTP clients, default headers/query, proxies, timeouts, retry counts) is now always excluded, even when a provider's own accept-set would otherwise let it through. This newly excludes, per handler: - langchain-messages / langchain-agents (ChatOpenAI): api_key, openai_api_key, base_url, openai_api_base, organization, openai_organization, openai_proxy, client, async_client, root_client, root_async_client, http_client, http_async_client, http_socket_options, default_headers, default_query, max_retries, request_timeout - langchain-messages / langchain-agents (ChatAnthropic): anthropic_api_key, api_key, anthropic_api_url, base_url, anthropic_proxy, default_request_timeout, max_retries, default_headers - langchain-messages / langchain-agents (ChatBedrockConverse, opt-in dependency): bedrock_api_key, api_key, aws_access_key_id, aws_secret_access_key, aws_session_token, credentials_profile_name, endpoint_url, base_url, client, bedrock_client, config, default_headers, max_retries - openai-agents (ModelSettings): retry timeout/extra_headers/extra_query/extra_body/extra_args were already excluded by the prior transport-prefix rule on every handler that has them; they are now named explicitly in each handler's own excluded set instead of matched by a shared prefix check, since nothing here is derived at runtime any more. Adds one drift test per handler package (test_parameter_forwarding.py) that reads the real provider SDK's signature/dataclass/model_fields and asserts every parameter it accepts is classified in exactly one of forwarded, handler-owned, or excluded, so an SDK addition nobody has classified fails loudly by name, and so does a stale list entry. The LangChain ChatBedrockConverse test skips itself when langchain-aws (an opt-in dependency) is not installed, matching this package's existing Bedrock test pattern. Updates langchain-messages' TestModelParametersReachTheWire test, which relied on forwarding http_async_client/api_key through model.parameters to mock the outgoing HTTP call: it now intercepts ChatOpenAI's own default httpx client builder instead, so the mock transport is reached without any config value ever naming an HTTP client or a key. Adds TestConnectionConfigIsNeverForwarded to both langchain-messages and langchain-agents, asserting api_key/base_url from model.parameters never reach the ChatOpenAI/ChatAnthropic constructor kwargs. Verified: uv run --frozen pytest (1385 passed, 15 skipped), uv run --frozen mypy packages/*/src, uv run --frozen ruff check ., uv run --frozen ruff format --check ., uv.lock unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…eters They are the same constructor field as `model`, under its field name or its alias. The handler always sets `model` itself, so a config value for either collided with the resolved model name or overrode it. Classify both as handler-owned per model class, and have the drift tests read that set from the handler instead of a test-local copy. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
bugbot run |
jeffdupont
left a comment
jeffdupont
left a comment
There was a problem hiding this comment.
Reviewed against the 1.0 GA plan. The fix is needed: parameters set in an AI Config were being ignored, and that should be fixed before 1.0. Tests pass at d51199d (make test: 1391 passed, 15 skipped, exit 0). The branch merges cleanly onto current main (fee904a), and the merged tree passes too: 1491 passed, make typecheck clean.
Four things I think need settling before merge, because each one changes the public interface and can't be changed after 1.0:
- Claude Agents forwards host-process settings, not just model settings. A config can set
cli_path,env,cwd,add_dirs,permission_mode,settings,setting_sources,pluginsandsandbox. I reproduced it: a config withcli_path: /tmp/attacker-binarymakes the SDK launch that file as its agent process (claude_agent_sdk/_internal/transport/subprocess_cli.py:225), andenv: {ANTHROPIC_BASE_URL: ...}points the customer's API key at another host (:434). That turns "can edit an AI Config" into "can run code on the customer's server." Inline comment below. - Two LangChain keys get around the exclusion list.
model_kwargscarriesextra_headers/extra_queryinto the ChatOpenAI request payload (reproduced with_get_request_payload), though both are listed as excluded.mcp_servers(ChatAnthropic) would let a config attach a remote MCP server; I haven't tested that one. - Two new public names.
model_parametersandselect_forwarded_parametersare in the root__all__, but only the provider packages use them. Same point as on #121 withmake_graph_track_data: they belong in the internal group, or the 1.0 surface trim has to remove them again. - Python and JS don't match (js #73). The two messages handlers agree. In the agent and LangChain handlers, JS forwards the whole camelCased bag with no allowlist, so
api_keybecomesapiKey, which ChatOpenAI and ChatAnthropic use. I'll leave a note there too. The monorepo has no spec for forwarding these parameters, and open monorepo #10 says "do not rename or filter parameter keys," which contradicts both PRs. One spec inTESTING.mdwould settle it for both languages.
Smaller points, not blocking:
output_format(Claude Messages) andresponse_id/starting_after/text_format(OpenAI) are forwarded on stream but not invoke, so one config can behave differently depending on the call.- OpenAI Responses forwards
reasoningbut notreasoning_effort. If the UI writesreasoning_effort, it's dropped silently, aseffortwas before your rename. I haven't checked what the UI writes. - The loops that remove handler-owned keys never find anything, since those keys aren't on the forwarded lists. They're harmless, just dead code.
- In native graphs only the root node's
max_turnsis used.
Removing _HAS_ANTHROPIC is fine; nothing on main reads it.
|
bugbot run |
…d_parameters from the package root Only the provider packages in this repo use these helpers, and anything in __all__ becomes public API at 1.0. Callers now import them from the modules that define them: launchdarkly_ai_server.utils and launchdarkly_ai_server.parameter_forwarding.
|
bugbot run |
…rameters ClaudeAgentOptions also configures the host process the SDK launches: which binary runs, its environment, working directory, file access, permission mode, settings, plugins, sandbox, and session state. Forwarding those from a config let anyone who can edit an AI Config launch their own binary or point the API key at another host. The forwarded list is now max_turns, max_thinking_tokens, thinking, effort, max_budget_usd, fallback_model, output_format and betas. Every other field is excluded, with the reason next to it. The loops that popped handler-owned keys are removed: none of those keys is on the forwarded list, so the filter already drops them.
model_kwargs is merged straight into the request payload, so it carried extra_headers and extra_query past the exclusions. mcp_servers let a config attach a remote MCP server that then receives the conversation. region_name and inference_geo move where data is processed. Bedrock's additional_model_request_fields is another unfiltered request bag. All of those are now excluded, along with LangChain's own runtime fields (callbacks, cache, tags, metadata, streaming mode, message format, ...), which configure how LangChain runs in this process rather than the request. _model_constructor_kwargs now requires its forwarded list, and the dead pop of tools for Bedrock is removed.
output_format is only accepted by messages.stream, so a config with it behaved differently depending on the call. It is now dropped on both paths; output_config, which both accept, carries the output format. One forwarded list now serves both calls. inference_geo is excluded too: it sets the region inference runs in. The loops that popped handler-owned keys are removed, since none of those keys is on the forwarded list.
max_turns is not a ModelSettings field, so the forwarded-keys filter already drops it. The native graph now says why only the root node's max_turns applies: the whole graph is one Runner.run, and the Agents SDK has no per-agent turn limit.
…ed keys One test over all six packages asserts that no forwarded list holds a credential, endpoint, request-injection, remote-tool or host-process key, and that every forwarded list in each handler module is covered. The client docstrings now say what a forwarded list may hold.
container (Claude Messages) and reuse_last_container (ChatAnthropic) carry server-side container state over from another request, and user_profile_id attributes the request to another party. None of them is a model setting, so they move to the excluded lists.
…arded bag extra_body is not a ClaudeAgentOptions field, so asserting it was dropped proved nothing. The handler test now sends every never-forwarded key through query and checks none reaches the options. provider_data joins the shared list, since in newer Agents SDK releases it carries raw request overrides.
…parameters # Conflicts: # packages/openai-agents/tests/test_native_graph.py
|
bugbot run |
jeffdupont
left a comment
jeffdupont
left a comment
There was a problem hiding this comment.
Every point from my 2 Oct review is fixed at 175050b. I couldn't find a way for a config to change credentials, endpoints or headers in any of the six handlers. At this head I get 1547 passed and 11 skipped (uv run pytest, exit 0). mypy, ruff check and ruff format --check are clean. There are no replies on the threads, so here is where each one stands:
- Claude Agents host settings (thread 1): fixed.
_build_query_optionsnow dropscli_path,env,permission_mode,add_dirs,cwd,settings,extra_argsandplugins, and keepsmax_turns,thinkingandoutput_format(claude-agents handler.py:85-96, :565). The native graph uses the same list (native_graph.py:155). model_kwargs(thread 2): fixed. It's excluded for ChatOpenAI and ChatAnthropic in both LangChain packages (langchain-agents handler.py:145, :200).mcp_serversandregion_name(thread 3): fixed. Both are excluded (:201, :258).- Root exports (thread 4): fixed in
c25d33bb. Neither name is in the root__init__.py. output_formaton stream only (thread 5): fixed. Invoke and stream now read the same list, andoutput_formatis excluded (claude-messages handler.py:55, :89). The OpenAI stream-only keys are excluded the same way (openai-messages handler.py:115-117).- Smaller points from the review body: the dead pop loops are gone (
d6813999). Root-onlymax_turnsin the OpenAI native graph is now documented (native_graph.py:283-287).reasoning_effortfor Responses is still dropped, on the basis of the comment that the UI writesreasoning: {effort}(openai-messages handler.py:62-63). I haven't checked what the UI writes.
Wire probe. I ran each handler, invoke and stream, against a local server, with the customer's keys in env. The config tried to override api_key, base_url, headers, extra_headers, default_headers, extra_query, extra_body, model_kwargs and every key in tests/never_forwarded.py. All 14 requests (openai-messages, claude-messages, openai-agents, and LangChain messages and agents for OpenAI and Anthropic) reached the local server. Each one used the customer's key and carried no config header, query or body value, and temperature and max_tokens still came through. To check the probe can fail, I added extra_headers to _RESPONSES_FORWARDED_KEYS. The request then went out with Authorization: Bearer ATTACKER. Bedrock was not probed on the wire.
Mutations. All 18 get caught by at least one test. They were: cli_path/env, model_kwargs, mcp_servers (both packages), region_name (both), inference_geo and container on each allowlist, output_format on both paths or on stream only, text_format, extra_headers (Responses and ModelSettings), the raw bag in each of the three native graphs, and dropping the max_tokens rename. That's a better result than any other forwarding PR I've reviewed.
What's left:
-
The cross-handler test only checks the six modules it names (tests/test_never_forwarded_parameters.py:25-41, :52-58). A new handler package with no
*_FORWARDED_KEYSlist isn't checked at all. py #103 and py #102 are both approved and can merge as they are, and neither uses these helpers. Vercel forwardsextra_headersandextra_queryfrommodel.parametersand puts every other key inextra_body(#103 vercel-messages handler.py:102-106). LiteLLM agents builds its model fromparameters["base_url"]andparameters["api_key"](#102 litellm-agents handler.py:117-121). I trial-merged #103 into this branch, and this test still reports 17 passed, the same count as without it. Could the test find everypackages/*/src/launchdarkly_ai_*that registers a handler, and fail any that isn't inFORWARDED_LISTS? Then whichever of #102/#103 lands after this one goes red instead of quietly shipping the leak. -
Python and js #73 now follow the same approach but disagree on the lists. Both use per-handler allowlists, both rename
effortandmax_tokensthe same way, and the Claude Agents lists match exactly. OpenAI Messages differs only bymax_tool_calls. The rest don't match (snake_case compared, js at532a0eef):- Claude Messages: JS forwards
inference_geoandcontainer. Python excludes both:inference_geobecause it "decides where data is processed", andcontainerbecause it's server-side state (handler.py:76-78).tests/never_forwarded.py:37listsinference_geoas never forwarded. - LangChain ChatAnthropic: JS forwards
inferenceGeoas well. - LangChain ChatOpenAI: JS also forwards
use_responses_api,streaming,stream_usage,disable_streaming,output_version,tags,metadata,verbose,user,zdr_enabled,audio,modalities,prompt_cache_keyandprompt_cache_retention. Python treats the first eight as handler or runtime settings it excludes (_LANGCHAIN_RUNTIME_KEYS, langchain-agents handler.py:60-75).verbosematters most: in JS it lets a config turn on LangChain's verbose logging, which prints prompts to stdout (reported from the js #73 review; I haven't run it). Python also forwardsseed,include,store,truncation,context_managementandreasoning_effort, and JS doesn't. - LangChain ChatBedrockConverse: Python forwards
system,guardrails,guard_last_turn_only,request_metadata,output_config,reasoning_effort,stopandstop_sequences. JS forwardssupports_tool_choice_values, which Python excludes as tool behaviour the handler depends on. - OpenAI Agents: Python forwards
include_usage,metadata,response_include,top_logprobsand a top-levelverbosity. JS forwardstextand mapsverbosityinto it. - Nested shapes: JS rebuilds
thinking,outputFormat,reasoningandcontext_managementand drops unknown sub-keys. Python passes nested values through as written. That's probably fine for Python's snake_case SDKs, but the two will differ when a nested value is malformed. - JS exports
pickForwardedModelParameters,normalizeModelParametersandcamelizeModelParametersfrom@launchdarkly/ai-server. Python dropped its equivalents (c25d33bb). One of the two should change. - The two say different things. JS's messages handlers say "exclude a key only if setting it would BREAK the handler; forward everything else". Python says "only model and run settings".
inference_geoandcontainerfall right in that gap.
Python is the stricter side on the keys that matter for data location. I'd fix the
inference_geo/containeritems in JS and settle the rest in the spec. A key forwarded at 1.0 can't be dropped later without breaking someone, so this needs settling before 1.0. - Claude Messages: JS forwards
-
The spec is monorepo #42 (apucacao, draft), and it covers most of this. §1.12 requires allowlists, never-forward categories in any spelling, handler-owned keys winning, a bag test with every category, and recommends an exhaustive classification test. #107 already does all of that. #42 doesn't yet:
- say that the same handler in two languages forwards the same keys. It only says handlers for different providers may differ.
- say that invoke and stream forward the same keys, which the PR body promises.
- place
inference_geoandcontainer. Its "region / region_name" example suggestsinference_geois never forwarded, which would make JS wrong. - close the conflict with monorepo #10, which is still open and says "Do not rename or filter parameter keys" (#10 diff line 44).
#42 also mixes this with the graph identity spec and renames span attributes (
ld.ai.graph.keytolaunchdarkly.graph.key). Splitting them would let §1.12 land first. I'd merge #107, js #73 and #42 together. -
Minor: ChatOpenAI excludes
use_responses_apibecause it changes "which API and response shape the handler gets back" (langchain-agents handler.py:120-122). Butinclude,reasoning,truncationandcontext_managementare forwarded, and setting any one of them makeslangchain_openaiswitch to the Responses API anyway (ChatOpenAI._use_responses_api, langchain-openai 1.3.3 base.py:1708-1717). Either the reason in the comment is wrong, or those four have the same problem. I haven't tested whether the handler reads a Responses-shaped reply correctly. -
Minor: Claude Messages excludes
user_profile_idbecause it "attributes the request to a party other than the caller" (handler.py:79-80). OpenAI Messages forwardsuserandsafety_identifier, which do the same job (handler.py:80, :88). Either is defensible, but the two handlers should agree.
| { | ||
| "stream", | ||
| "output_format", | ||
| "inference_geo", |
There was a problem hiding this comment.
Agree with excluding inference_geo and container. js #73 forwards both, though (claude-messages handler.ts FORWARDED_MODEL_PARAMETER_KEYS), and LangChain JS forwards inferenceGeo on ChatAnthropic. So a config that sets inference_geo changes where data is processed in JS and has no effect in Python. That needs settling in #42 and in JS before 1.0.
| "include_response_headers", | ||
| "disabled_params", | ||
| "tiktoken_model_name", | ||
| "use_responses_api", |
There was a problem hiding this comment.
use_responses_api is excluded because it changes the API the handler talks to. But include, reasoning, truncation and context_management are forwarded above, and any one of them makes ChatOpenAI switch to the Responses API anyway (_use_responses_api, langchain-openai 1.3.3 base.py:1708-1717). Either the reason here is wrong or those four have the same problem. JS forwards useResponsesApi, streaming, streamUsage, tags, metadata and verbose, all of which this list excludes. Whichever way this goes, the two SDKs should match.
| "service_tier", | ||
| "stop", | ||
| "stop_sequences", | ||
| "system", |
There was a problem hiding this comment.
system lets a config add Bedrock system prompt blocks on top of instructions. That's not a security issue, since a config already controls the instructions, but JS doesn't forward it, and JS's ChatOpenAI list leaves out prefixMessages as "prompt content". Is it meant to be on this list? guardrails, guard_last_turn_only, request_metadata, output_config, reasoning_effort and stop(_sequences) are also Python-only for Bedrock.
| #: Excluded, and why: all client/connection configuration, never a config-controlled setting. | ||
| #: * ``retry``: an HTTP retry count. | ||
| #: * ``extra_headers``, ``extra_query``, ``extra_body``, ``extra_args``: raw HTTP/request overrides. | ||
| _MODEL_SETTINGS_FORWARDED_KEYS = frozenset( |
There was a problem hiding this comment.
Parity with js #73: Python forwards include_usage, metadata, response_include, top_logprobs and a top-level verbosity. JS forwards text instead and maps verbosity into text.verbosity. JS also rebuilds reasoning and context_management and drops unknown sub-keys, while this passes them through as written. Could the two lists match, or could the difference be written down in #42?
Scan packages/*/src/launchdarkly_ai_* source for modules that build a ProviderHandler and fail when one has no entry in FORWARDED_LISTS, so a new handler package cannot skip the never-forwarded check. The package that defines ProviderHandler is the client core and is skipped.
|
bugbot run |
…arameters select_forwarded_parameters takes an optional mapping_keys set: forwarded keys whose value the provider expects to be an object. A value of any other type for one of them is dropped like an unlisted key, instead of reaching the provider call as written.
tests/forwarding_spec.py holds each handler's forwarded list from the cross-SDK spec (TESTING.md 1.12), in the config's own spelling, plus probe_forwarded_keys: it sets one key at a time and reports which ones change what the handler hands its provider. Package tests use it to assert each call site forwards exactly its list.
metadata leaves the forwarded list and joins the excluded side as identity and attribution, next to user_profile_id. Every excluded key now names its category. cache_control, output_config, thinking and tool_choice are dropped when they are not objects. invoke and stream read one helper, and a per-key probe of each path asserts both forward exactly the spec list.
Drops context_management, include, instructions, metadata, moderation, prompt_cache_retention, safety_identifier, store, truncation and user from the Responses allowlist. Each is excluded with its reason: data retention, server-side state, identity and attribution, API-shape switch, prompt content beyond instructions, or safety configuration. A reasoning value that is not an object is dropped. A per-key probe of invoke and stream asserts both forward exactly the spec list.
ModelSettings no longer takes context_management, include_usage, metadata, prompt_cache_retention, response_include, store, top_logprobs or truncation from a config; each is excluded with its reason. text is read only for text.verbosity, which becomes verbosity unless the config set verbosity itself, and a text or reasoning value that is not an object is dropped. The handler and the native graph share one helper, and per-key probes of invoke, stream and the native graph assert they forward exactly the spec list, with max_turns going to the run.
…ot objects The forwarded list is unchanged and already matches the cross-SDK list. The handler and the native graph now share one helper that also drops malformed object values, and per-key probes of invoke, stream and the native graph assert they forward exactly the spec list.
ChatOpenAI drops context_management, include, reasoning, reasoning_effort, seed, store and truncation. ChatAnthropic drops context_management. ChatBedrockConverse drops guardrails, guardrail_config, guard_last_turn_only, request_metadata, system, output_config, reasoning_effort, stop and stop_sequences. Each removed key is excluded with its reason. The use_responses_api reason now holds: include, reasoning, truncation and context_management also switch ChatOpenAI to the Responses API, and all of them are excluded together. prompt_cache_key is on the spec list but ChatOpenAI has no such field, so it is left out, with a test that fails once the field exists. Object-valued keys (logit_bias, thinking, output_config, performance_config) are dropped when malformed. Both packages probe each constructor one key at a time against the spec list, check that invoke and stream hand the builder the same config, and probe the LangGraph native graph's ChatOpenAI too.
The bag now also holds data retention (store, prompt_cache_retention), server-side state (context_management, truncation), identity and attribution (metadata, user, safety_identifier, request_metadata), API-shape switches (include, include_usage, response_include), prompt content (instructions, system) and safety configuration (moderation, guardrails, guardrail_config, guard_last_turn_only). No handler forwards any of them. Claude Messages sets system itself, so its bag test allows the key and still checks no config value reaches it.
|
bugbot run |
The JS openai SDK's responses.create has no max_tool_calls parameter, so the key is cut from the cross-SDK OpenAI Messages list rather than worked around in one language. It moves to the excluded side with that reason, and the exact-set test follows the shorter list.
…hropic ChatAnthropic takes both spellings of one field, so a config that set both left the winner to pydantic. The alias is now renamed to max_tokens before forwarding, and a max_tokens the config also set wins.
|
bugbot run |
…sity win A forwarded reasoning object becomes the SDK's own Reasoning type with only effort and summary, the same two sub-keys the JS SDK keeps, and is dropped when it has neither. A test runs the real Chat Completions model with it. When a config sets both text.verbosity and a top-level verbosity, the explicit text.verbosity now wins, matching the JS SDK.
ChatOpenAI has no prompt_cache_key field, so the key is cut from the cross-SDK ChatOpenAI list rather than worked around. It is documented on the excluded side with that reason, and a test checks a config value never reaches the constructor.
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit c76acd5. Configure here.
2 participants
Every time I ask "how would I use the turnkey SDK here?", I hear about how model params aren't currently being passed to models. This PR fixes that.
Not entirely sure about the implementation though: we ended up definiting the list of params supported by each of the underlying SDKs, because I couldn't find a better export from those packages, and iterating on types felt harder than the explicit list. The downside is that if/when things change, we'd need to update the code. But presumably we already need to do that for our model parameter schemas in Gonfalon.
Model parameters set in an AI Config were silently ignored by most handlers. Every handler now passes them through to the provider.
extra_*,model_kwargs), remote MCP servers, the Claude Agent SDK's host-process settings (cli_path,env,cwd,add_dirs,permission_mode, sessions, hooks, callbacks), and keys that set data location or retention, server-side state, identity, runtime wiring or API shape, extra prompt content, or safety configuration (for examplestore,truncation,metadata,user,include,system,guardrails).invokeandstreamforward the same keys, and so do each handler and its native graph.thinking,reasoning,text, ...) is dropped when the config sets it to anything else.packages/and fails if one has no forwarded-keys list, so a new handler package must declare one.max_turns,top_p), the providers' own names and what the UI writes.🤖 Generated with Claude Code
Note
Overview
AI Config
model.parametersnow reach provider SDKs through a shared filter (model_parameters+select_forwarded_parameters) instead of being ignored or passed through wholesale.Each Python handler declares hand-maintained allowlists (cross-SDK spec) for what may be forwarded into
ClaudeAgentOptions, Anthropicmessages.create/.stream, LangChain chat model constructors, or OpenAI AgentsModelSettings/Runner.run. Everything else—including credentials, endpoints,extra_*/model_kwargs, Claude Agent host-process settings (cli_path,env,permission_mode, …), and UI-only keys the SDK does not accept—is dropped so configs cannot override handler-owned fields likemodelorsystem_prompt. Native graph paths reuse the same forwarding helpers as their handlers.Provider-specific shaping includes moving
effort→output_config(Claude Messages),text.verbosity→verbosityand rebuiltreasoning(OpenAI Agents),max_turnsonRunner.run(notModelSettings), and Anthropicmax_tokens_to_sample→max_tokens. LangChain handlers no longer passtoolsinto chat model constructors.Drift and probe tests assert each SDK field is classified exactly once and that invoke/stream forward the same keys; regression tests cover wire-level forwarding and blocked “never forwarded” bags.
Reviewed by Cursor Bugbot for commit c76acd5. Bugbot is set up for automated code reviews on this repo. Configure here.