From 91691750ed91829fece676c7d8841fe67509f145 Mon Sep 17 00:00:00 2001 From: Rui Hu Date: Mon, 18 May 2026 14:28:55 +0000 Subject: [PATCH] fix(reactive): forward actor options block in ToolCallingLLMCaller._resolve_llm Port the options-block merging logic from SimpleLLMAgent._resolve_llm() (stream_router.py, added in PR #11225 / commit b3851693) to the parallel resolution path ToolCallingLLMCaller._resolve_llm() in tool_caller.py. Without this fix, running an actor with --skill would ignore the options block in the actor YAML, causing openai_api_base and openai_api_key to be silently dropped. The provider registry then fell back to OPENAI_API_KEY from the environment, routing requests to api.openai.com instead of the configured local llama-swap/llama.cpp backend (HTTP 401). Changes: - tool_caller.ToolCallingLLMCaller._resolve_llm: read actor_config.options; extract openai_api_key -> __api_key_sentinel; forward allowed keys (openai_api_base, timeout, top_p, frequency_penalty, presence_penalty); reject reserved keys (provider_type, model_id) and unknown keys with logger.warning, matching SimpleLLMAgent behaviour exactly. - features/actor_run_tool_calling.feature: add five BDD regression scenarios (section V, tagged @tdd_issue @tdd_issue_11243) covering api_base+key forwarding, allowed extra keys, reserved key rejection, unknown key rejection, and top-level precedence over options. - features/steps/actor_run_tool_calling_steps.py: add corresponding step definitions for all five scenarios. All nox quality gates pass (lint, typecheck, unit_tests 15822/0 fail, integration_tests 1999/0 fail, coverage_report 97%). ISSUES CLOSED: #11243 --- features/actor_run_tool_calling.feature | 33 ++++ .../steps/actor_run_tool_calling_steps.py | 175 ++++++++++++++++++ src/cleveragents/reactive/tool_caller.py | 48 +++++ 3 files changed, 256 insertions(+) diff --git a/features/actor_run_tool_calling.feature b/features/actor_run_tool_calling.feature index be60ba8ef..9d5cee598 100644 --- a/features/actor_run_tool_calling.feature +++ b/features/actor_run_tool_calling.feature @@ -272,3 +272,36 @@ Feature: Actor run tool-calling via ToolCallingRuntime Given a ToolCallingLLMCaller with an actor config without system_prompt When invoke is called and the LLM returns a response with encoded tool call names Then the LLMResponse contains the decoded tool calls + + # ---------- V: ToolCallingLLMCaller options block forwarding (#11243) ---------- + + @tdd_issue @tdd_issue_11243 + Scenario: ToolCallingLLMCaller forwards openai_api_base and openai_api_key from options block + Given a ToolCallingLLMCaller with actor config options containing openai_api_base and openai_api_key + When _resolve_llm is called on the ToolCallingLLMCaller with empty tool_schemas + Then create_llm was called with openai_api_base forwarded from options + And create_llm was called with __api_key_sentinel extracted from options openai_api_key + + @tdd_issue @tdd_issue_11243 + Scenario: ToolCallingLLMCaller forwards allowed options keys to create_llm + Given a ToolCallingLLMCaller with actor config options containing allowed extra keys + When _resolve_llm is called on the ToolCallingLLMCaller with empty tool_schemas + Then create_llm was called with the allowed options keys forwarded + + @tdd_issue @tdd_issue_11243 + Scenario: ToolCallingLLMCaller rejects reserved keys in options block with a warning + Given a ToolCallingLLMCaller with actor config options containing reserved key provider_type + When _resolve_llm is called on the ToolCallingLLMCaller with empty tool_schemas + Then create_llm was NOT called with the reserved key provider_type + + @tdd_issue @tdd_issue_11243 + Scenario: ToolCallingLLMCaller rejects unknown keys in options block with a warning + Given a ToolCallingLLMCaller with actor config options containing unknown key foo_bar + When _resolve_llm is called on the ToolCallingLLMCaller with empty tool_schemas + Then create_llm was NOT called with the unknown key foo_bar + + @tdd_issue @tdd_issue_11243 + Scenario: ToolCallingLLMCaller top-level config takes precedence over options block + Given a ToolCallingLLMCaller with actor config with top-level temperature and options temperature + When _resolve_llm is called on the ToolCallingLLMCaller with empty tool_schemas + Then create_llm was called with the top-level temperature value diff --git a/features/steps/actor_run_tool_calling_steps.py b/features/steps/actor_run_tool_calling_steps.py index 51b0b5c5f..a7a1ab7cd 100644 --- a/features/steps/actor_run_tool_calling_steps.py +++ b/features/steps/actor_run_tool_calling_steps.py @@ -1089,3 +1089,178 @@ def step_response_contains_decoded_tool_calls(context: Any) -> None: assert "server:local/my-tool" in names, ( f"Expected 'server:local/my-tool' in tool call names, got {names}" ) + + +# --------------------------------------------------------------------------- +# Scenarios V: ToolCallingLLMCaller options block forwarding (#11243) +# --------------------------------------------------------------------------- + + +@given( + "a ToolCallingLLMCaller with actor config options containing openai_api_base and openai_api_key" +) +def step_caller_with_options_api_base_and_key(context: Any) -> None: + context.options_caller = ToolCallingLLMCaller( + actor_config={ + "provider": "openai", + "model": "gpt-4", + "options": { + "openai_api_base": "http://localhost:8080/v1", + "openai_api_key": "none", + }, + } + ) + context.options_mock_llm = MagicMock() + context.options_mock_llm.bind_tools.return_value = context.options_mock_llm + + +@when("_resolve_llm is called on the ToolCallingLLMCaller with empty tool_schemas") +def step_resolve_llm_options_empty_schemas(context: Any) -> None: + with patch("cleveragents.reactive.tool_caller.get_provider_registry") as mock_reg: + mock_reg.return_value.create_llm.return_value = context.options_mock_llm + context.options_caller._resolve_llm([]) + context.options_create_llm_kwargs = ( + mock_reg.return_value.create_llm.call_args.kwargs + ) + + +@then("create_llm was called with openai_api_base forwarded from options") +def step_create_llm_called_with_api_base(context: Any) -> None: + kwargs = context.options_create_llm_kwargs + assert "openai_api_base" in kwargs, ( + f"Expected 'openai_api_base' in create_llm kwargs, got {kwargs}" + ) + assert kwargs["openai_api_base"] == "http://localhost:8080/v1", ( + f"Expected openai_api_base='http://localhost:8080/v1', got {kwargs['openai_api_base']}" + ) + + +@then( + "create_llm was called with __api_key_sentinel extracted from options openai_api_key" +) +def step_create_llm_called_with_api_key_sentinel(context: Any) -> None: + kwargs = context.options_create_llm_kwargs + assert kwargs.get("__api_key_sentinel") == "none", ( + f"Expected __api_key_sentinel='none' from options.openai_api_key, got {kwargs}" + ) + assert "openai_api_key" not in kwargs, ( + "openai_api_key must be extracted to __api_key_sentinel, not passed directly" + ) + + +@given("a ToolCallingLLMCaller with actor config options containing allowed extra keys") +def step_caller_with_allowed_options_keys(context: Any) -> None: + context.options_caller = ToolCallingLLMCaller( + actor_config={ + "provider": "openai", + "model": "gpt-4", + "options": { + "timeout": 30, + "top_p": 0.9, + "frequency_penalty": 0.1, + "presence_penalty": 0.2, + }, + } + ) + context.options_mock_llm = MagicMock() + context.options_mock_llm.bind_tools.return_value = context.options_mock_llm + + +@then("create_llm was called with the allowed options keys forwarded") +def step_create_llm_called_with_allowed_keys(context: Any) -> None: + kwargs = context.options_create_llm_kwargs + assert kwargs.get("timeout") == 30, ( + f"Expected timeout=30, got {kwargs.get('timeout')}" + ) + assert kwargs.get("top_p") == 0.9, f"Expected top_p=0.9, got {kwargs.get('top_p')}" + assert kwargs.get("frequency_penalty") == 0.1, ( + f"Expected frequency_penalty=0.1, got {kwargs.get('frequency_penalty')}" + ) + assert kwargs.get("presence_penalty") == 0.2, ( + f"Expected presence_penalty=0.2, got {kwargs.get('presence_penalty')}" + ) + + +@given( + "a ToolCallingLLMCaller with actor config options containing reserved key provider_type" +) +def step_caller_with_reserved_key(context: Any) -> None: + context.options_caller = ToolCallingLLMCaller( + actor_config={ + "provider": "openai", + "model": "gpt-4", + "options": { + "provider_type": "should-be-rejected", + }, + } + ) + context.options_mock_llm = MagicMock() + context.options_mock_llm.bind_tools.return_value = context.options_mock_llm + + +@then("create_llm was NOT called with the reserved key provider_type") +def step_create_llm_not_called_with_reserved(context: Any) -> None: + kwargs = context.options_create_llm_kwargs + # create_llm is always called with provider_type (the actor's real provider). + # What must NOT happen is the options value ("should-be-rejected") overriding it. + # The call must have succeeded without a TypeError from duplicate keyword arguments, + # and the provider_type must be the actor's real value, not the options value. + assert context.options_create_llm_kwargs is not None, ( + "_resolve_llm must succeed even when options contains reserved keys" + ) + assert kwargs.get("provider_type") != "should-be-rejected", ( + f"Reserved key 'provider_type' from options must not override the actor provider; " + f"got provider_type={kwargs.get('provider_type')!r}" + ) + + +@given( + "a ToolCallingLLMCaller with actor config options containing unknown key foo_bar" +) +def step_caller_with_unknown_key(context: Any) -> None: + context.options_caller = ToolCallingLLMCaller( + actor_config={ + "provider": "openai", + "model": "gpt-4", + "options": { + "foo_bar": "should-be-rejected", + }, + } + ) + context.options_mock_llm = MagicMock() + context.options_mock_llm.bind_tools.return_value = context.options_mock_llm + + +@then("create_llm was NOT called with the unknown key foo_bar") +def step_create_llm_not_called_with_unknown(context: Any) -> None: + kwargs = context.options_create_llm_kwargs + assert "foo_bar" not in kwargs, ( + f"Unknown key 'foo_bar' must not be forwarded to create_llm, got {kwargs}" + ) + + +@given( + "a ToolCallingLLMCaller with actor config with top-level temperature and options temperature" +) +def step_caller_top_level_temperature_vs_options(context: Any) -> None: + context.options_caller = ToolCallingLLMCaller( + actor_config={ + "provider": "openai", + "model": "gpt-4", + "temperature": 0.5, # top-level value — must take precedence + "options": { + "temperature": 0.99, # options value — must be ignored + }, + } + ) + context.options_mock_llm = MagicMock() + context.options_mock_llm.bind_tools.return_value = context.options_mock_llm + + +@then("create_llm was called with the top-level temperature value") +def step_create_llm_called_with_top_level_temp(context: Any) -> None: + kwargs = context.options_create_llm_kwargs + assert kwargs.get("temperature") == 0.5, ( + f"Expected top-level temperature=0.5 to take precedence over options, " + f"got temperature={kwargs.get('temperature')}" + ) diff --git a/src/cleveragents/reactive/tool_caller.py b/src/cleveragents/reactive/tool_caller.py index ca12d94f0..bb6ace681 100644 --- a/src/cleveragents/reactive/tool_caller.py +++ b/src/cleveragents/reactive/tool_caller.py @@ -138,6 +138,54 @@ class ToolCallingLLMCaller: if max_retries is not None: llm_kwargs["max_retries"] = max_retries + # M7 (#11243): merge options block so custom LLM backend kwargs + # (e.g. openai_api_base, openai_api_key) are forwarded to the + # LLM constructor — mirrors the fix applied to SimpleLLMAgent in + # stream_router.py (PR #11225 / commit b3851693). + # + # Options are applied after the fixed keys so that explicit + # top-level keys (temperature, max_tokens, etc.) take precedence + # over any duplicate in the options block. + # + # openai_api_key is handled specially: the registry uses a + # __api_key_sentinel mechanism to distinguish explicitly provided + # keys from environment-sourced ones. If the user supplies + # openai_api_key in options, we extract it and inject it via the + # sentinel so it overrides the registry's default. + _ALLOWED_OPTIONS: frozenset[str] = frozenset( + { + "openai_api_base", + "temperature", + "max_tokens", + "timeout", + "top_p", + "frequency_penalty", + "presence_penalty", + } + ) + _RESERVED: frozenset[str] = frozenset({"provider_type", "model_id"}) + options = dict(self._actor_config.get("options") or {}) + if "openai_api_key" in options: + llm_kwargs["__api_key_sentinel"] = options.pop("openai_api_key") + for key, value in options.items(): + if key in _RESERVED: + logger.warning( + "Actor options block contains reserved key '%s' " + "that conflicts with positional arguments; ignoring.", + key, + ) + continue + if key not in llm_kwargs: + if key in _ALLOWED_OPTIONS: + llm_kwargs[key] = value + else: + logger.warning( + "Actor options block contains unrecognized key '%s'; " + "ignoring for security. Allowed keys: %s.", + key, + sorted(_ALLOWED_OPTIONS), + ) + registry = get_provider_registry() # Annotated as Any because create_llm() returns BaseLanguageModel but the # real runtime object is BaseChatModel which has bind_tools().