mirror of
https://github.com/usestrix/strix.git
synced 2026-11-05 07:37:19 +00:00
fix(agents): classify the effective run model, not settings.llm.model
Addresses review feedback on #1050: when run_strix_scan(model=...) selects a model that differs from settings.llm.model, the strict-tool gate was classifying the wrong model, silently leaving >20 strict tools enabled and letting Bedrock reject the scan at startup anyway. build_strix_agent now takes an optional model_name, threaded through from runner.py's already-resolved resolved_model (falls back to settings.llm.model for callers, e.g. tests, that don't go through the runner). make_child_factory forwards the same value so children built later in the scan see the same classification as the root agent.
This commit is contained in:
parent
ee96577b82
commit
91e9610030
3 changed files with 64 additions and 4 deletions
|
|
@ -599,6 +599,7 @@ def build_strix_agent(
|
|||
system_prompt_context: dict[str, Any] | None = None,
|
||||
extra_tools: Sequence[Tool] | None = None,
|
||||
instructions_override: str | None = None,
|
||||
model_name: str | None = None,
|
||||
) -> SandboxAgent[Any]:
|
||||
"""Build a SandboxAgent for either root or child use.
|
||||
|
||||
|
|
@ -609,6 +610,10 @@ def build_strix_agent(
|
|||
registered via ``register_agent_tools``.
|
||||
instructions_override: Use this verbatim as the system prompt instead
|
||||
of rendering the built-in scan prompt.
|
||||
model_name: The effective model for this run (e.g. ``RunConfig.model``
|
||||
/ ``run_strix_scan(model=...)``), which can differ from
|
||||
``load_settings().llm.model``. Falls back to the settings model
|
||||
when omitted, e.g. for tests that don't go through the runner.
|
||||
"""
|
||||
if instructions_override is not None:
|
||||
instructions = instructions_override
|
||||
|
|
@ -638,10 +643,10 @@ def build_strix_agent(
|
|||
for tool in tools
|
||||
]
|
||||
|
||||
model_name = load_settings().llm.model
|
||||
effective_model = model_name if model_name is not None else load_settings().llm.model
|
||||
if (
|
||||
is_claude_model(model_name)
|
||||
and is_bedrock_route(model_name)
|
||||
is_claude_model(effective_model)
|
||||
and is_bedrock_route(effective_model)
|
||||
and _bedrock_claude_strict_tool_limit_exceeded(len(tools))
|
||||
):
|
||||
# Bedrock rejects the whole request over the 20-strict-tool cap, not
|
||||
|
|
@ -686,12 +691,15 @@ def make_child_factory(
|
|||
interactive: bool = False,
|
||||
chat_completions_tools: bool = False,
|
||||
system_prompt_context: dict[str, Any] | None = None,
|
||||
model_name: str | None = None,
|
||||
) -> Any:
|
||||
"""Return the runner-owned builder used by ``spawn_child_agent``.
|
||||
|
||||
Run-level arguments (``scan_mode``, ``is_whitebox``, etc.) are
|
||||
captured in a closure so each child inherits scan-level configuration
|
||||
without the graph tool knowing about runner internals.
|
||||
without the graph tool knowing about runner internals. ``model_name`` is
|
||||
the effective run model (see ``build_strix_agent``), so children built
|
||||
later in the scan see the same model classification as the root agent.
|
||||
"""
|
||||
|
||||
def _factory(*, name: str, skills: list[str]) -> SandboxAgent[Any]:
|
||||
|
|
@ -704,6 +712,7 @@ def make_child_factory(
|
|||
interactive=interactive,
|
||||
chat_completions_tools=chat_completions_tools,
|
||||
system_prompt_context=system_prompt_context,
|
||||
model_name=model_name,
|
||||
)
|
||||
|
||||
return _factory
|
||||
|
|
|
|||
|
|
@ -303,6 +303,7 @@ async def run_strix_scan(
|
|||
chat_completions_tools=chat_completions_tools,
|
||||
system_prompt_context=root_context,
|
||||
instructions_override=root_instructions,
|
||||
model_name=resolved_model,
|
||||
)
|
||||
|
||||
if not is_resume:
|
||||
|
|
@ -320,6 +321,7 @@ async def run_strix_scan(
|
|||
interactive=interactive,
|
||||
chat_completions_tools=chat_completions_tools,
|
||||
system_prompt_context=scope_context,
|
||||
model_name=resolved_model,
|
||||
)
|
||||
|
||||
async def spawn_child_agent(**kwargs: Any) -> dict[str, Any]:
|
||||
|
|
|
|||
|
|
@ -204,3 +204,52 @@ def test_disabling_strict_schema_does_not_leak_across_builds(
|
|||
assert shared.strict_json_schema, (
|
||||
f"{shared.name} singleton was mutated by the Bedrock build"
|
||||
)
|
||||
|
||||
|
||||
def test_model_name_override_takes_precedence_over_settings(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""``run_strix_scan(model=...)`` can pick a model that differs from
|
||||
``settings.llm.model`` (e.g. ``STRIX_LLM`` unset, model passed directly).
|
||||
The strict-tool gate must classify that effective model, not whatever
|
||||
happens to be in settings.
|
||||
"""
|
||||
monkeypatch.delenv("STRIX_LLM", raising=False)
|
||||
monkeypatch.setattr(loader, "_cached", None)
|
||||
monkeypatch.setattr(loader, "_override", None)
|
||||
|
||||
agent = factory.build_strix_agent(
|
||||
is_root=True,
|
||||
model_name="bedrock/anthropic.claude-4-6-sonnet",
|
||||
)
|
||||
|
||||
assert len(agent.tools) > 20
|
||||
function_tools = [t for t in agent.tools if isinstance(t, FunctionTool)]
|
||||
assert all(not t.strict_json_schema for t in function_tools)
|
||||
|
||||
|
||||
def test_model_name_override_can_keep_strict_schema(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""A Bedrock model in settings should not force non-strict tools when the
|
||||
effective run model (passed explicitly) is a different, non-Bedrock route.
|
||||
"""
|
||||
monkeypatch.setenv("STRIX_LLM", "bedrock/anthropic.claude-4-6-sonnet")
|
||||
monkeypatch.setattr(loader, "_cached", None)
|
||||
monkeypatch.setattr(loader, "_override", None)
|
||||
|
||||
agent = factory.build_strix_agent(
|
||||
is_root=True,
|
||||
model_name="anthropic/claude-4-6-sonnet",
|
||||
)
|
||||
|
||||
function_tools = [t for t in agent.tools if isinstance(t, FunctionTool)]
|
||||
assert any(t.strict_json_schema for t in function_tools)
|
||||
|
||||
|
||||
def test_child_factory_threads_model_name_through() -> None:
|
||||
factory_fn = factory.make_child_factory(model_name="bedrock/anthropic.claude-4-6-sonnet")
|
||||
|
||||
child = factory_fn(name="child", skills=[])
|
||||
|
||||
function_tools = [t for t in child.tools if isinstance(t, FunctionTool)]
|
||||
assert len(child.tools) > 20
|
||||
assert all(not t.strict_json_schema for t in function_tools)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue