Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 21 additions & 20 deletions agent/auxiliary_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -5470,7 +5470,12 @@ def get_auxiliary_extra_body() -> dict:
return _nous_extra_body() if auxiliary_is_nous else {}


def auxiliary_max_tokens_param(value: int, *, model: Optional[str] = None) -> dict:
def auxiliary_max_tokens_param(
value: int,
*,
model: Optional[str] = None,
base_url: Optional[str] = None,
) -> dict:
"""Return the correct max tokens kwarg for the auxiliary client's provider.

OpenRouter and local models use 'max_tokens'. Direct OpenAI with newer
Expand All @@ -5480,7 +5485,7 @@ def auxiliary_max_tokens_param(value: int, *, model: Optional[str] = None) -> di
fronting the newer families are also recognised — URL-only detection
misses the case where a custom base URL serves e.g. ``gpt-5.4``.
"""
custom_base = _current_custom_base_url()
custom_base = base_url if base_url is not None else _current_custom_base_url()
or_key = os.getenv("OPENROUTER_API_KEY")
# Use max_completion_tokens for direct OpenAI-compatible providers that reject
# max_tokens on newer GPT-4o/o-series/GPT-5-style models.
Expand Down Expand Up @@ -6232,25 +6237,13 @@ def _build_call_kwargs(
kwargs["temperature"] = temperature

if max_tokens is not None:
# We do NOT cap output by default. Most chat-completions providers treat
# an omitted max_tokens as "use the model's max output", which is what we
# want for auxiliary tasks (compression summaries, titles, vision, etc.) —
# an explicit cap only risks truncating a summary or 400-ing on providers
# that reject the parameter outright (e.g. GitHub Copilot / newer OpenAI
# GPT-5 models require max_completion_tokens, not max_tokens; ZAI vision
# models reject it entirely with error 1210). Omitting it sidesteps all of
# those wire-format quirks at once.
# We still do NOT cap output by default. Most chat-completions providers
# treat an omitted max_tokens as "use the model's max output", which is
# what we want for auxiliary tasks (compression summaries, titles, etc.).
#
# The one exception is the Anthropic Messages wire (MiniMax and any
# ``/anthropic`` endpoint reached through the OpenAI SDK wrapper), where
# max_tokens is a MANDATORY field — omitting it is a hard 400. Keep it only
# there.
#
# NVIDIA NIM (integrate.api.nvidia.com and local NIM endpoints) is a
# second exception: some models—notably minimaxai/minimax-m3—return HTTP
# 200 with an empty choices[] payload when max_tokens is omitted. The main
# NVIDIA chat path already sends an output cap via the provider profile;
# preserve it on the auxiliary path too.
# When a caller deliberately supplies a cap, preserve that intent with a
# provider-compatible wire parameter. The retry path below strips both
# max_tokens and max_completion_tokens if a backend rejects the cap.
_effective_base = base_url or (
_current_custom_base_url() if provider == "custom" else ""
)
Expand All @@ -6264,6 +6257,14 @@ def _build_call_kwargs(
or _is_nvidia_nim
):
kwargs["max_tokens"] = max_tokens
else:
kwargs.update(
auxiliary_max_tokens_param(
max_tokens,
model=model,
base_url=_effective_base,
)
)

if tools:
# Defensive dedup: providers like Google Vertex, Azure, and Bedrock
Expand Down
8 changes: 5 additions & 3 deletions gateway/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -15078,9 +15078,10 @@ async def _enrich_message_with_vision(
from agent.memory_manager import sanitize_context

analysis_prompt = (
"Describe everything visible in this image in thorough detail. "
"Include any text, code, data, objects, people, layout, colors, "
"and any other notable visual information."
"Concisely describe this image in 2-4 sentences. Include the main "
"subject, important visible text/data/code, and enough context for "
"the next answer. Skip decorative details unless they affect the "
"user's request."
)

enriched_parts = []
Expand All @@ -15090,6 +15091,7 @@ async def _enrich_message_with_vision(
result_json = await vision_analyze_tool(
image_url=path,
user_prompt=analysis_prompt,
max_tokens=500,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vision_analyze_tool receives this value, but async_call_llm currently drops max_tokens for ordinary OpenAI-compatible/custom routes in agent/auxiliary_client.py:_build_call_kwargs (the llama-server-style route from #10809). Please carry this through a deliberately compatible capped-call path and test the underlying client request, not only the async_call_llm mock.

)
result = json.loads(result_json)
if result.get("success"):
Expand Down
68 changes: 68 additions & 0 deletions tests/agent/test_auxiliary_client.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""Tests for agent.auxiliary_client resolution chain, provider overrides, and model overrides."""

import asyncio
import base64
import json
import logging
Expand Down Expand Up @@ -4798,6 +4799,73 @@ def test_none_tools_unchanged(self):
assert "tools" not in kwargs


class TestBuildCallKwargsMaxTokens:
"""Explicit auxiliary output caps must survive provider kwarg building."""

def test_custom_local_route_preserves_explicit_max_tokens(self):
kwargs = _build_call_kwargs(
provider="custom",
model="llama3-vision",
messages=[],
max_tokens=500,
base_url="http://localhost:8080/v1",
)

assert kwargs["max_tokens"] == 500
assert "max_completion_tokens" not in kwargs

def test_custom_gpt5_route_uses_max_completion_tokens(self):
kwargs = _build_call_kwargs(
provider="custom",
model="gpt-5.4",
messages=[],
max_tokens=500,
base_url="https://my-gateway.example.com/v1",
)

assert kwargs["max_completion_tokens"] == 500
assert "max_tokens" not in kwargs

def test_async_vision_custom_route_sends_explicit_output_cap(self):
client = SimpleNamespace(base_url="http://localhost:8080/v1")
create = AsyncMock(return_value=SimpleNamespace(
choices=[SimpleNamespace(message=SimpleNamespace(content="{}"))],
))
client.chat = SimpleNamespace(
completions=SimpleNamespace(create=create),
)

with (
patch(
"agent.auxiliary_client._resolve_task_provider_model",
return_value=(
"custom",
"llama3-vision",
"http://localhost:8080/v1",
"test-key",
None,
),
),
patch(
"agent.auxiliary_client.resolve_vision_provider_client",
return_value=("custom", client, "llama3-vision"),
),
):
asyncio.run(
async_call_llm(
task="vision",
provider="custom",
model="llama3-vision",
messages=[{"role": "user", "content": "describe"}],
max_tokens=500,
)
)

call_kwargs = create.call_args.kwargs
assert call_kwargs["max_tokens"] == 500
assert "max_completion_tokens" not in call_kwargs


@pytest.fixture(autouse=True)
def _clean_env(monkeypatch):
"""Strip provider env vars so each test starts clean."""
Expand Down
16 changes: 16 additions & 0 deletions tests/gateway/test_vision_memory_leak.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,22 @@ def test_clean_description_passes_through(self, gateway_runner):
out = _run(gateway_runner._enrich_message_with_vision("caption", ["/tmp/img.jpg"]))
assert "sunset over the ocean" in out

def test_auto_analysis_uses_bounded_prompt_and_output_cap(self, gateway_runner):
"""Gateway image preprocessing should avoid long auxiliary descriptions."""
fake_result = json.dumps({
"success": True,
"analysis": "A concise screenshot summary.",
})
mock_vision = AsyncMock(return_value=fake_result)

with patch("tools.vision_tools.vision_analyze_tool", new=mock_vision):
_run(gateway_runner._enrich_message_with_vision("caption", ["/tmp/img.jpg"]))

kwargs = mock_vision.call_args.kwargs
assert kwargs["max_tokens"] == 500
assert "2-4 sentences" in kwargs["user_prompt"]
assert "thorough detail" not in kwargs["user_prompt"]

def test_memory_context_fence_stripped(self, gateway_runner):
"""<memory-context>...</memory-context> fenced block is scrubbed."""
leaked = (
Expand Down
34 changes: 32 additions & 2 deletions tests/tools/test_vision_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -480,6 +480,37 @@ async def test_vision_defaults_temperature_when_config_omits_it(self, tmp_path):
assert mock_llm.await_args.kwargs["temperature"] == 0.1
assert mock_llm.await_args.kwargs["timeout"] == 120.0

@pytest.mark.asyncio
async def test_vision_max_tokens_override_is_per_call(self, tmp_path):
img = tmp_path / "test.png"
img.write_bytes(b"\x89PNG\r\n\x1a\n" + b"\x00" * 8)

mock_response = MagicMock()
mock_choice = MagicMock()
mock_choice.message.content = "Bounded image analysis"
mock_response.choices = [mock_choice]

with (
patch("hermes_cli.config.load_config", return_value={"auxiliary": {"vision": {}}}),
patch(
"tools.vision_tools._image_to_base64_data_url",
return_value="data:image/png;base64,abc",
),
patch(
"tools.vision_tools.async_call_llm",
new_callable=AsyncMock,
return_value=mock_response,
) as mock_llm,
):
await vision_analyze_tool(str(img), "describe this", "test/model")
await vision_analyze_tool(
str(img), "describe this", "test/model", max_tokens=500
)

first_call, second_call = mock_llm.await_args_list
assert first_call.kwargs["max_tokens"] == 2000
assert second_call.kwargs["max_tokens"] == 500


class TestVisionSafetyGuards:
@pytest.mark.asyncio
Expand Down Expand Up @@ -824,8 +855,7 @@ def test_schema_has_required_fields(self):
assert schema["name"] == "vision_analyze"
params = schema.get("parameters", {})
props = params.get("properties", {})
assert "image_url" in props
assert "question" in props
assert set(props) == {"image_url", "question"}

def test_handler_is_callable(self):
from tools.registry import registry
Expand Down
4 changes: 3 additions & 1 deletion tools/vision_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -1065,6 +1065,7 @@ async def vision_analyze_tool(
user_prompt: str,
model: str = None,
task_id: Optional[str] = None,
max_tokens: Optional[int] = None,
) -> str:
"""
Analyze an image from a URL or local file path using vision AI.
Expand All @@ -1082,6 +1083,7 @@ async def vision_analyze_tool(
Accepts http://, https:// URLs or absolute/relative file paths.
user_prompt (str): The pre-formatted prompt for the vision model
model (str): The vision model to use (default: google/gemini-3-flash-preview)
max_tokens (int): Optional output cap for this analysis call

Returns:
str: JSON string containing the analysis results with the following structure:
Expand Down Expand Up @@ -1243,7 +1245,7 @@ async def vision_analyze_tool(
"task": "vision",
"messages": messages,
"temperature": vision_temperature,
"max_tokens": 2000,
"max_tokens": max_tokens if max_tokens is not None else 2000,
"timeout": vision_timeout,
}
if model:
Expand Down