fix(reactive): forward actor options block to LLM constructor for custom backend support #11225
@@ -211,3 +211,24 @@ Feature: v3 Actor YAML schema support in CLI and registry
|
||||
Given a v3 graph actor config dict with skills and lsp as dict
|
||||
When I parse the v3 config through ReactiveConfigParser
|
||||
Then each graph node agent should have lsp dict propagated
|
||||
|
||||
# M5: options block forwarded to agent_config in _build_from_v3 (#11223).
|
||||
@tdd_issue @tdd_issue_11223
|
||||
Scenario: ReactiveConfigParser propagates options block to agent config
|
||||
Given a v3 LLM actor config dict with options block
|
||||
When I parse the v3 config through ReactiveConfigParser
|
||||
Then the agent config should contain the options block
|
||||
|
||||
# M5: actor without options block is unaffected (#11223).
|
||||
@tdd_issue @tdd_issue_11223
|
||||
Scenario: ReactiveConfigParser handles v3 actor without options block
|
||||
Given a v3 LLM actor config dict without options block
|
||||
When I parse the v3 config through ReactiveConfigParser
|
||||
Then the agent config should not contain an options key
|
||||
|
||||
# M5: empty options dict is preserved (#11223).
|
||||
@tdd_issue @tdd_issue_11223
|
||||
Scenario: ReactiveConfigParser preserves empty options block
|
||||
Given a v3 LLM actor config dict with empty options block
|
||||
When I parse the v3 config through ReactiveConfigParser
|
||||
Then the agent config should contain an empty options block
|
||||
|
||||
@@ -1379,3 +1379,24 @@ Feature: Consolidated Routing
|
||||
When the stream router tool agent processes "keep_me" through operation
|
||||
Then the stream router tool agent operation result should be "keep_me"
|
||||
|
||||
# M5: SimpleLLMAgent._resolve_llm forwards options to registry.create_llm (#11223).
|
||||
@tdd_issue @tdd_issue_11223
|
||||
Scenario: SimpleLLMAgent forwards options block to LLM constructor
|
||||
Given a stream router llm agent with options block containing custom base url
|
||||
When the stream router llm agent resolves the llm
|
||||
Then the llm constructor should have received the custom base url
|
||||
|
||||
# M5: actor without options block is unaffected (#11223).
|
||||
@tdd_issue @tdd_issue_11223
|
||||
Scenario: SimpleLLMAgent without options block calls LLM constructor without extra kwargs
|
||||
Given a stream router llm agent without options block
|
||||
|
|
||||
When the stream router llm agent resolves the llm
|
||||
Then the llm constructor should not have received extra kwargs
|
||||
|
||||
# M5: top-level keys take precedence over duplicates in options (#11223).
|
||||
@tdd_issue @tdd_issue_11223
|
||||
Scenario: SimpleLLMAgent top-level key takes precedence over options duplicate
|
||||
Given a stream router llm agent with top-level temperature and options block
|
||||
When the stream router llm agent resolves the llm
|
||||
Then the llm constructor should have received the top-level temperature
|
||||
|
||||
|
||||
@@ -14,6 +14,7 @@ from unittest.mock import patch
|
||||
|
||||
import yaml
|
||||
from behave import given, then, when
|
||||
from behave.runner import Context
|
||||
|
||||
from cleveragents.actor.compiler import ActorCompilationError
|
||||
from cleveragents.actor.config import ActorConfiguration
|
||||
@@ -781,3 +782,77 @@ def step_graph_nodes_have_lsp_dict(context: Any) -> None:
|
||||
assert lsp.get("auto") is True, (
|
||||
f"Node '{node_id}' expected lsp.auto=True, got {lsp}"
|
||||
)
|
||||
|
||||
|
||||
# ── M5: options block forwarding (#11223) ─────────────────────────────────
|
||||
|
||||
|
||||
@given("a v3 LLM actor config dict with options block")
|
||||
def step_v3_llm_config_with_options(context: Context) -> None:
|
||||
context.v3_config = {
|
||||
"name": "local/my-llama-actor",
|
||||
"type": "llm",
|
||||
"description": "Local llama.cpp actor",
|
||||
"provider": "openai",
|
||||
"model": "my-local-model",
|
||||
"options": {
|
||||
"openai_api_base": "http://localhost:8080/v1",
|
||||
"openai_api_key": "none",
|
||||
"temperature": 1.0,
|
||||
},
|
||||
"system_prompt": "You are a helpful assistant.",
|
||||
}
|
||||
|
||||
|
||||
@given("a v3 LLM actor config dict without options block")
|
||||
def step_v3_llm_config_without_options(context: Context) -> None:
|
||||
context.v3_config = {
|
||||
"name": "local/standard-actor",
|
||||
"type": "llm",
|
||||
"description": "Standard actor without options",
|
||||
"provider": "openai",
|
||||
"model": "gpt-4",
|
||||
"system_prompt": "You are a helpful assistant.",
|
||||
}
|
||||
|
||||
|
||||
@then("the agent config should contain the options block")
|
||||
def step_agent_config_has_options(context: Context) -> None:
|
||||
agents = context.reactive_config.agents
|
||||
agent = next(iter(agents.values()))
|
||||
options = agent.config.get("options")
|
||||
assert options == {
|
||||
"openai_api_base": "http://localhost:8080/v1",
|
||||
"openai_api_key": "none",
|
||||
"temperature": 1.0,
|
||||
}, f"Expected exact options dict, got {options}"
|
||||
|
||||
|
||||
@then("the agent config should not contain an options key")
|
||||
def step_agent_config_no_options(context: Context) -> None:
|
||||
agents = context.reactive_config.agents
|
||||
agent = next(iter(agents.values()))
|
||||
assert "options" not in agent.config, (
|
||||
f"Expected no 'options' key in agent config, got {agent.config}"
|
||||
)
|
||||
|
||||
|
||||
@given("a v3 LLM actor config dict with empty options block")
|
||||
def step_v3_llm_config_with_empty_options(context: Context) -> None:
|
||||
context.v3_config = {
|
||||
"name": "local/empty-options-actor",
|
||||
"type": "llm",
|
||||
"description": "Actor with empty options",
|
||||
"provider": "openai",
|
||||
"model": "gpt-4",
|
||||
"options": {},
|
||||
"system_prompt": "You are a helpful assistant.",
|
||||
}
|
||||
|
||||
|
||||
@then("the agent config should contain an empty options block")
|
||||
def step_agent_config_has_empty_options(context: Context) -> None:
|
||||
agents = context.reactive_config.agents
|
||||
agent = next(iter(agents.values()))
|
||||
options = agent.config.get("options")
|
||||
assert options == {}, f"Expected empty options dict, got {options}"
|
||||
|
||||
@@ -6,7 +6,8 @@
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from unittest.mock import MagicMock
|
||||
from typing import Any
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
from behave import given, then, when
|
||||
from behave.runner import Context
|
||||
@@ -160,3 +161,115 @@ def step_then_tool_agent_operation_result(context: Context, expected: str) -> No
|
||||
assert context.op_result == expected, (
|
||||
f"Expected '{expected}', got '{context.op_result}'"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# M5: SimpleLLMAgent._resolve_llm forwards options to create_llm (#11223).
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@given("a stream router llm agent with options block containing custom base url")
|
||||
def step_given_llm_agent_with_options(context: Context) -> None:
|
||||
context.llm_options_agent = SimpleLLMAgent(
|
||||
"test_options",
|
||||
{
|
||||
"provider": "openai",
|
||||
"model": "my-local-model",
|
||||
"options": {
|
||||
"openai_api_base": "http://localhost:8080/v1",
|
||||
"openai_api_key": "none",
|
||||
},
|
||||
},
|
||||
)
|
||||
context.create_llm_kwargs: dict[str, Any] = {}
|
||||
|
||||
|
||||
@given("a stream router llm agent without options block")
|
||||
def step_given_llm_agent_without_options(context: Context) -> None:
|
||||
context.llm_options_agent = SimpleLLMAgent(
|
||||
"test_no_options",
|
||||
{
|
||||
"provider": "openai",
|
||||
"model": "gpt-4",
|
||||
},
|
||||
)
|
||||
context.create_llm_kwargs = {}
|
||||
|
||||
|
||||
@when("the stream router llm agent resolves the llm")
|
||||
def step_when_llm_agent_resolves(context: Context) -> None:
|
||||
mock_llm = MagicMock()
|
||||
captured: dict[str, Any] = {}
|
||||
|
||||
def fake_create_llm(
|
||||
provider_type: Any = None, model_id: Any = None, **kwargs: Any
|
||||
) -> Any:
|
||||
captured["provider_type"] = provider_type
|
||||
captured["model_id"] = model_id
|
||||
captured["kwargs"] = kwargs
|
||||
return mock_llm
|
||||
|
||||
mock_registry = MagicMock()
|
||||
mock_registry.create_llm.side_effect = fake_create_llm
|
||||
|
||||
with patch(
|
||||
"cleveragents.reactive.stream_router.get_provider_registry",
|
||||
return_value=mock_registry,
|
||||
):
|
||||
context.llm_options_agent._resolve_llm()
|
||||
|
||||
context.create_llm_kwargs = captured
|
||||
|
||||
|
||||
@then("the llm constructor should have received the custom base url")
|
||||
def step_then_llm_received_custom_base_url(context: Context) -> None:
|
||||
kwargs = context.create_llm_kwargs.get("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_key is routed through __api_key_sentinel so the
|
||||
# registry can distinguish explicit keys from environment defaults.
|
||||
assert kwargs.get("__api_key_sentinel") == "none", (
|
||||
f"Expected __api_key_sentinel='none' (from options.openai_api_key), "
|
||||
f"got {kwargs}"
|
||||
)
|
||||
|
||||
|
||||
@then("the llm constructor should not have received extra kwargs")
|
||||
def step_then_llm_no_extra_kwargs(context: Context) -> None:
|
||||
kwargs = context.create_llm_kwargs.get("kwargs", {})
|
||||
# When neither options nor top-level params are present, nothing
|
||||
# should be forwarded to the LLM constructor.
|
||||
assert kwargs == {}, f"Expected empty kwargs in create_llm call, got {kwargs}"
|
||||
|
||||
|
||||
@given("a stream router llm agent with top-level temperature and options block")
|
||||
def step_given_llm_agent_with_precedence(context: Context) -> None:
|
||||
context.llm_options_agent = SimpleLLMAgent(
|
||||
"test_precedence",
|
||||
{
|
||||
"provider": "openai",
|
||||
"model": "gpt-4",
|
||||
"temperature": 0.5,
|
||||
"options": {
|
||||
"temperature": 1.0,
|
||||
"max_tokens": 4096,
|
||||
},
|
||||
},
|
||||
)
|
||||
context.create_llm_kwargs: dict[str, Any] = {}
|
||||
|
||||
|
||||
@then("the llm constructor should have received the top-level temperature")
|
||||
def step_then_llm_received_top_level_temperature(context: Context) -> None:
|
||||
kwargs = context.create_llm_kwargs.get("kwargs", {})
|
||||
assert kwargs.get("temperature") == 0.5, (
|
||||
f"Expected temperature=0.5 (top-level precedence over options), got {kwargs}"
|
||||
)
|
||||
# max_tokens from options should still be forwarded.
|
||||
assert kwargs.get("max_tokens") == 4096, (
|
||||
f"Expected max_tokens=4096 from options, got {kwargs}"
|
||||
)
|
||||
|
||||
@@ -286,9 +286,9 @@ class ReactiveConfigParser:
|
||||
are propagated via the agent config so the runtime can attach them.
|
||||
|
||||
All v3 fields — ``context_view``, ``memory``, ``context``,
|
||||
``env_vars``, ``response_format``, ``lsp_capabilities``, and
|
||||
``lsp_context_enrichment`` — are propagated into the agent config
|
||||
so the runtime can consume them.
|
||||
``env_vars``, ``response_format``, ``lsp_capabilities``,
|
||||
``lsp_context_enrichment``, and ``options`` — are propagated
|
||||
into the agent config so the runtime can consume them.
|
||||
"""
|
||||
# m4: guard against None-to-string coercion for all three fields.
|
||||
actor_type = str(data.get("type") or "llm").lower()
|
||||
@@ -375,6 +375,11 @@ class ReactiveConfigParser:
|
||||
if isinstance(response_format, dict):
|
||||
agent_config["response_format"] = response_format
|
||||
|
||||
# M5: propagate actor options block for custom backend support (#11223).
|
||||
options_raw = data.get("options")
|
||||
if isinstance(options_raw, dict):
|
||||
agent_config["options"] = dict(options_raw)
|
||||
|
||||
if actor_type in ("llm", "tool"):
|
||||
# Single-agent synthesis.
|
||||
rc.agents[actor_name] = AgentConfig(
|
||||
@@ -424,6 +429,9 @@ class ReactiveConfigParser:
|
||||
else dict(lsp_bindings)
|
||||
)
|
||||
node_config.setdefault("lsp", lsp_copy)
|
||||
# M5: propagate actor-level options to graph nodes (#11223).
|
||||
if isinstance(options_raw, dict) and options_raw:
|
||||
node_config.setdefault("options", dict(options_raw))
|
||||
nodes_map[node_id] = node_config
|
||||
|
||||
# Create a reactive agent for each graph node.
|
||||
|
||||
@@ -226,6 +226,55 @@ class SimpleLLMAgent:
|
||||
llm_kwargs["max_tokens"] = max_tokens
|
||||
if max_retries is not None:
|
||||
llm_kwargs["max_retries"] = max_retries
|
||||
# M5: merge options block so custom LLM backend kwargs
|
||||
# (e.g. openai_api_base, openai_api_key) are forwarded to the
|
||||
# LLM constructor. Options are applied after the fixed keys so
|
||||
# that explicit top-level keys (temperature, max_tokens, etc.)
|
||||
# take precedence over duplicates in options.
|
||||
#
|
||||
# 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.
|
||||
options = dict(self.config.get("options") or {})
|
||||
if "openai_api_key" in options:
|
||||
llm_kwargs["__api_key_sentinel"] = options.pop("openai_api_key")
|
||||
# Ensure sensitive/reserved ChatOpenAI constructor params cannot
|
||||
|
HAL9001
commented
Suggestion: Consider hoisting Automated by CleverAgents Bot Suggestion: Consider hoisting `_ALLOWED_OPTIONS` and `_RESERVED` frozenset definitions to module level (near existing constants like `_SANITIZER`). Currently they are recreated on every `_resolve_llm()` call. Frozenset construction is fast, but module-level placement improves discoverability and follows the project convention for immutable constant collections.
---
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
|
||||
# be injected via a crafted actor YAML.
|
||||
_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"})
|
||||
for key, value in options.items():
|
||||
if key in _RESERVED:
|
||||
logger_sr.warning(
|
||||
"Actor '%s' options block contains reserved key '%s' "
|
||||
"that conflicts with positional arguments; ignoring.",
|
||||
self.name,
|
||||
key,
|
||||
)
|
||||
continue
|
||||
if key not in llm_kwargs:
|
||||
if key in _ALLOWED_OPTIONS:
|
||||
llm_kwargs[key] = value
|
||||
else:
|
||||
logger_sr.warning(
|
||||
"Actor '%s' options block contains unrecognized "
|
||||
"key '%s'; ignoring for security. Allowed keys: "
|
||||
"%s.",
|
||||
self.name,
|
||||
key,
|
||||
sorted(_ALLOWED_OPTIONS),
|
||||
)
|
||||
registry = get_provider_registry()
|
||||
self._llm = registry.create_llm(
|
||||
provider_type=provider, model_id=model, **llm_kwargs
|
||||
|
||||
Reference in New Issue
Block a user
Note: Consider adding a scenario for the
@tdd_issue_11223reserved key rejection check - specifically verifying that unrecognized/not-whitelisted keys (e.g.,custom_provider_param) trigger a warning and are NOT forwarded to the LLM constructor. This would exercise the security whitelist code path in an integration test.Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker