mirror of
https://github.com/usestrix/strix.git
synced 2026-10-13 03:57:59 +00:00
feat(llm): keep images away from text-only models
Leave view_image out of the tools, replace MCP image blocks with a text note, and swap agent_browser's screenshot section for text-only guidance when the run's model does not accept images.
This commit is contained in:
parent
a6666c5848
commit
cda895e99f
9 changed files with 109 additions and 4 deletions
|
|
@ -375,6 +375,11 @@ def _configure_filesystem_tools(
|
|||
)
|
||||
|
||||
|
||||
class _FilesystemWithoutViewImage(Filesystem):
|
||||
def tools(self) -> list[Tool]:
|
||||
return [tool for tool in super().tools() if tool.name != "view_image"]
|
||||
|
||||
|
||||
def _make_filesystem_configurator(*, chat_completions: bool, strict_schemas: bool) -> Any:
|
||||
def configure(toolset: Any) -> None:
|
||||
_configure_filesystem_tools(
|
||||
|
|
@ -670,6 +675,7 @@ def build_strix_agent(
|
|||
system_prompt_context: dict[str, Any] | None = None,
|
||||
extra_tools: Sequence[Tool] | None = None,
|
||||
instructions_override: str | None = None,
|
||||
supports_images: bool = True,
|
||||
) -> SandboxAgent[Any]:
|
||||
"""Build a SandboxAgent for either root or child use.
|
||||
|
||||
|
|
@ -694,6 +700,7 @@ def build_strix_agent(
|
|||
is_diff_scoped=is_diff_scoped,
|
||||
interactive=interactive,
|
||||
system_prompt_context=system_prompt_context,
|
||||
supports_images=supports_images,
|
||||
)
|
||||
|
||||
agent_tools = [*_EXTRA_TOOLS, *(extra_tools or [])]
|
||||
|
|
@ -729,7 +736,7 @@ def build_strix_agent(
|
|||
tool_use_behavior=_finish_tool_use_behavior,
|
||||
model=None,
|
||||
capabilities=[
|
||||
Filesystem(
|
||||
(Filesystem if supports_images else _FilesystemWithoutViewImage)(
|
||||
configure_tools=_make_filesystem_configurator(
|
||||
chat_completions=chat_completions_tools,
|
||||
strict_schemas=strict_tool_schemas,
|
||||
|
|
@ -754,6 +761,7 @@ def make_child_factory(
|
|||
chat_completions_tools: bool = False,
|
||||
strict_tool_schemas: bool = True,
|
||||
system_prompt_context: dict[str, Any] | None = None,
|
||||
supports_images: bool = True,
|
||||
) -> Any:
|
||||
"""Return the runner-owned builder used by ``spawn_child_agent``.
|
||||
|
||||
|
|
@ -774,6 +782,7 @@ def make_child_factory(
|
|||
chat_completions_tools=chat_completions_tools,
|
||||
strict_tool_schemas=strict_tool_schemas,
|
||||
system_prompt_context=system_prompt_context,
|
||||
supports_images=supports_images,
|
||||
)
|
||||
|
||||
return _factory
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@
|
|||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
import re
|
||||
from typing import Any
|
||||
|
||||
from jinja2 import Environment, FileSystemLoader, select_autoescape
|
||||
|
|
@ -20,6 +21,16 @@ _PROMPT_DIRNAME = "prompts"
|
|||
# Removed before the prompt is sent.
|
||||
CACHE_POINT = "<cache_point>"
|
||||
|
||||
_SCREENSHOT_SECTION_RE = re.compile(r"^### Screenshot\n.*?(?=^### )", re.MULTILINE | re.DOTALL)
|
||||
_TEXT_ONLY_SCREENSHOT_SECTION = """### Screenshot
|
||||
|
||||
You are running on a text-only model and cannot view images, so do not take
|
||||
screenshots. Drive the page entirely from `snapshot -i` refs, `eval` for any
|
||||
DOM/JS state you need to read, and `text @ref` / `get text` for content
|
||||
extraction.
|
||||
|
||||
"""
|
||||
|
||||
|
||||
def _resolve_skills(
|
||||
*,
|
||||
|
|
@ -85,6 +96,7 @@ def render_system_prompt(
|
|||
interactive: bool = False,
|
||||
system_prompt_context: dict[str, Any] | None = None,
|
||||
include_scope: bool = True,
|
||||
supports_images: bool = True,
|
||||
) -> str:
|
||||
"""Render the system prompt. Returns empty string on template failure.
|
||||
|
||||
|
|
@ -120,6 +132,10 @@ def render_system_prompt(
|
|||
is_diff_scoped=is_diff_scoped,
|
||||
)
|
||||
skill_content = load_skills(skills_to_load)
|
||||
if not supports_images and "agent_browser" in skill_content:
|
||||
skill_content["agent_browser"] = _SCREENSHOT_SECTION_RE.sub(
|
||||
_TEXT_ONLY_SCREENSHOT_SECTION, skill_content["agent_browser"], count=1
|
||||
)
|
||||
env.globals["get_skill"] = lambda name: skill_content.get(name, "")
|
||||
|
||||
# Skills every agent of this kind loads come first, so siblings share them
|
||||
|
|
|
|||
|
|
@ -23,6 +23,7 @@ from strix.config.models import (
|
|||
StrixProvider,
|
||||
configure_sdk_api_route,
|
||||
configure_sdk_model_defaults,
|
||||
model_supports_images,
|
||||
supports_strict_tool_schemas,
|
||||
uses_chat_completions_tool_schema,
|
||||
)
|
||||
|
|
@ -153,6 +154,7 @@ def _compose_root_instructions_override(
|
|||
is_diff_scoped: bool,
|
||||
interactive: bool,
|
||||
system_prompt_context: dict[str, Any],
|
||||
supports_images: bool,
|
||||
) -> str | None:
|
||||
if root_instructions_override is None:
|
||||
return None
|
||||
|
|
@ -166,6 +168,7 @@ def _compose_root_instructions_override(
|
|||
interactive=interactive,
|
||||
system_prompt_context=system_prompt_context,
|
||||
include_scope=False,
|
||||
supports_images=supports_images,
|
||||
)
|
||||
return (
|
||||
f"{base_instructions}\n\n"
|
||||
|
|
@ -265,6 +268,9 @@ async def run_strix_scan(
|
|||
strict_tool_schemas = supports_strict_tool_schemas(resolved_model)
|
||||
if not strict_tool_schemas:
|
||||
logger.info("Sending non-strict tool schemas: %s caps strict tools", resolved_model)
|
||||
supports_images = model_supports_images(resolved_model)
|
||||
if not supports_images:
|
||||
logger.info("Leaving out image tools: %s does not accept images", resolved_model)
|
||||
|
||||
if budget_policy not in ("stop", "pause"):
|
||||
raise ValueError(f"unknown budget_policy: {budget_policy!r}")
|
||||
|
|
@ -468,6 +474,7 @@ async def run_strix_scan(
|
|||
is_diff_scoped=is_diff_scoped,
|
||||
interactive=interactive,
|
||||
system_prompt_context=root_context,
|
||||
supports_images=supports_images,
|
||||
)
|
||||
|
||||
root_agent = build_strix_agent(
|
||||
|
|
@ -482,6 +489,7 @@ async def run_strix_scan(
|
|||
strict_tool_schemas=strict_tool_schemas,
|
||||
system_prompt_context=root_context,
|
||||
instructions_override=root_instructions,
|
||||
supports_images=supports_images,
|
||||
)
|
||||
|
||||
if not is_resume:
|
||||
|
|
@ -501,6 +509,7 @@ async def run_strix_scan(
|
|||
chat_completions_tools=chat_completions_tools,
|
||||
strict_tool_schemas=strict_tool_schemas,
|
||||
system_prompt_context=scope_context,
|
||||
supports_images=supports_images,
|
||||
)
|
||||
|
||||
async def spawn_child_agent(**kwargs: Any) -> dict[str, Any]:
|
||||
|
|
@ -528,6 +537,7 @@ async def run_strix_scan(
|
|||
"spawn_child_agent": spawn_child_agent,
|
||||
"scan_targets": build_scan_targets(scan_config),
|
||||
"max_context_images": settings.runtime.max_context_images,
|
||||
"supports_images": supports_images,
|
||||
}
|
||||
|
||||
root_session = open_agent_session(root_id, agents_db)
|
||||
|
|
|
|||
|
|
@ -288,9 +288,11 @@ async def call_mcp(
|
|||
"Call search_mcp_tools, then get_mcp_tool_schema."
|
||||
)
|
||||
session = await entry.ensure_connected()
|
||||
context = ctx.context if isinstance(ctx.context, dict) else {}
|
||||
return await session.dispatch(
|
||||
tool,
|
||||
arguments or {},
|
||||
label=namespaced_tool_name(connection, tool),
|
||||
result_transform=entry.result_transform,
|
||||
supports_images=context.get("supports_images", True),
|
||||
)
|
||||
|
|
|
|||
|
|
@ -183,7 +183,9 @@ def _build_server(config: McpConnectionConfig) -> BuiltMcpServer:
|
|||
)
|
||||
|
||||
|
||||
def _mcp_result_to_tool_output(server: MCPServer, result: Any) -> Any:
|
||||
def _mcp_result_to_tool_output(
|
||||
server: MCPServer, result: Any, *, supports_images: bool = True
|
||||
) -> Any:
|
||||
"""Serialize a ``CallToolResult`` to a tool output, mirroring the agents SDK.
|
||||
|
||||
This reproduces the serialization in ``agents.mcp.util.MCPUtil.invoke_mcp_tool``
|
||||
|
|
@ -199,6 +201,13 @@ def _mcp_result_to_tool_output(server: MCPServer, result: Any) -> Any:
|
|||
for item in result.content:
|
||||
if item.type == "text":
|
||||
outputs.append({"type": "text", "text": item.text})
|
||||
elif item.type == "image" and not supports_images:
|
||||
outputs.append(
|
||||
{
|
||||
"type": "text",
|
||||
"text": f"[{item.mimeType} image omitted: this model cannot view images]",
|
||||
}
|
||||
)
|
||||
elif item.type == "image":
|
||||
outputs.append(
|
||||
{"type": "image", "image_url": f"data:{item.mimeType};base64,{item.data}"}
|
||||
|
|
@ -217,6 +226,7 @@ async def dispatch_mcp_call(
|
|||
*,
|
||||
label: str,
|
||||
result_transform: ResultTransform | None = None,
|
||||
supports_images: bool = True,
|
||||
) -> Any:
|
||||
"""Run one MCP tool call and convert its result to a tool output.
|
||||
|
||||
|
|
@ -235,7 +245,7 @@ async def dispatch_mcp_call(
|
|||
result = await server.call_tool(tool_name, arguments)
|
||||
if result_transform is not None:
|
||||
return result_transform(label, result.model_dump(mode="json"))
|
||||
tool_output = _mcp_result_to_tool_output(server, result)
|
||||
tool_output = _mcp_result_to_tool_output(server, result, supports_images=supports_images)
|
||||
if getattr(result, "isError", False):
|
||||
return _errored_tool_output(tool_output)
|
||||
return tool_output
|
||||
|
|
|
|||
|
|
@ -365,6 +365,7 @@ class SupervisedMcpSession:
|
|||
*,
|
||||
label: str,
|
||||
result_transform: ResultTransform | None = None,
|
||||
supports_images: bool = True,
|
||||
) -> Any:
|
||||
"""Run one tool call with bounded retries for transient session failures.
|
||||
|
||||
|
|
@ -382,6 +383,7 @@ class SupervisedMcpSession:
|
|||
arguments,
|
||||
label=label,
|
||||
result_transform=result_transform,
|
||||
supports_images=supports_images,
|
||||
)
|
||||
|
||||
outcome = await self._run_job(job, phase="call")
|
||||
|
|
|
|||
|
|
@ -115,3 +115,16 @@ def test_function_tools_are_result_bounded() -> None:
|
|||
by_name = {t.name: t for t in agent.tools}
|
||||
|
||||
assert getattr(by_name["think"], "_strix_bounded", False) is True
|
||||
|
||||
|
||||
@pytest.mark.parametrize(("supports_images", "has_view_image"), [(True, True), (False, False)])
|
||||
def test_view_image_is_left_out_for_text_only_models(
|
||||
supports_images: bool, has_view_image: bool
|
||||
) -> None:
|
||||
agent = factory.build_strix_agent(is_root=True, supports_images=supports_images)
|
||||
filesystem = agent.capabilities[0]
|
||||
filesystem.bind(cast("Any", object()))
|
||||
|
||||
names = [tool.name for tool in filesystem.tools()]
|
||||
assert ("view_image" in names) is has_view_image
|
||||
assert "apply_patch" in names
|
||||
|
|
|
|||
|
|
@ -18,7 +18,7 @@ from typing import TYPE_CHECKING, Any
|
|||
import pytest
|
||||
from agents.mcp import MCPServer, MCPServerStdio, MCPServerStreamableHttp
|
||||
from agents.tool_context import ToolContext
|
||||
from mcp.types import CallToolResult, TextContent
|
||||
from mcp.types import CallToolResult, ImageContent, TextContent
|
||||
from mcp.types import Tool as MCPTool
|
||||
from pydantic import ValidationError
|
||||
|
||||
|
|
@ -884,6 +884,40 @@ async def test_call_mcp_flags_an_errored_result_failed_for_the_tui() -> None:
|
|||
assert out == {"type": "text", "text": "boom:read_file", "success": False}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_call_mcp_replaces_images_for_a_text_only_model() -> None:
|
||||
class _ImageServer(FakeMCPServer):
|
||||
async def call_tool(
|
||||
self,
|
||||
tool_name: str,
|
||||
arguments: dict[str, Any] | None,
|
||||
meta: dict[str, Any] | None = None,
|
||||
) -> CallToolResult:
|
||||
return CallToolResult(
|
||||
content=[
|
||||
TextContent(type="text", text="shot"),
|
||||
ImageContent(type="image", data="aGk=", mimeType="image/png"),
|
||||
]
|
||||
)
|
||||
|
||||
registry = McpRegistry()
|
||||
registry.add(
|
||||
name="fs", server=_ImageServer("fs", [_mcp_tool("shot")]), purpose=None, tool_count=1
|
||||
)
|
||||
ctx = _ctx(registry)
|
||||
args = json.dumps({"connection": "fs", "tool": "shot"})
|
||||
|
||||
out = await call_mcp.on_invoke_tool(ctx, args)
|
||||
assert out[1] == {"type": "image", "image_url": "data:image/png;base64,aGk="}
|
||||
|
||||
ctx.context["supports_images"] = False
|
||||
out = await call_mcp.on_invoke_tool(ctx, args)
|
||||
assert out == [
|
||||
{"type": "text", "text": "shot"},
|
||||
{"type": "text", "text": "[image/png image omitted: this model cannot view images]"},
|
||||
]
|
||||
|
||||
|
||||
# --- generic MCP tools are the only MCP surface every agent gets -------------
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -287,6 +287,15 @@ def test_requested_skills_follow_the_shared_prefix() -> None:
|
|||
assert "<xss>" in xss.split("<cache_point>")[1]
|
||||
|
||||
|
||||
def test_text_only_prompt_drops_screenshot_guidance() -> None:
|
||||
assert "view_image" in render_system_prompt(include_scope=False)
|
||||
|
||||
prompt = render_system_prompt(include_scope=False, supports_images=False)
|
||||
assert "view_image" not in prompt
|
||||
assert "text-only model" in prompt
|
||||
assert "### Handle multiple pages via tabs" in prompt
|
||||
|
||||
|
||||
def test_scope_is_sent_as_its_own_system_message_on_cache_point_routes() -> None:
|
||||
settings = make_model_settings(None, model_name="anthropic/claude-sonnet-5-5")
|
||||
prompt = render_system_prompt(
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue