diff --git a/agent/auxiliary_client.py b/agent/auxiliary_client.py index 585db9940cda3..04a906cf2f8e5 100644 --- a/agent/auxiliary_client.py +++ b/agent/auxiliary_client.py @@ -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 @@ -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. @@ -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 "" ) @@ -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 diff --git a/gateway/run.py b/gateway/run.py index ccfa8e92c143e..cd374949ce6de 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -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 = [] @@ -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, ) result = json.loads(result_json) if result.get("success"): diff --git a/tests/agent/test_auxiliary_client.py b/tests/agent/test_auxiliary_client.py index 95bc4e6f1b56c..b09f080147339 100644 --- a/tests/agent/test_auxiliary_client.py +++ b/tests/agent/test_auxiliary_client.py @@ -1,5 +1,6 @@ """Tests for agent.auxiliary_client resolution chain, provider overrides, and model overrides.""" +import asyncio import base64 import json import logging @@ -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.""" diff --git a/tests/gateway/test_vision_memory_leak.py b/tests/gateway/test_vision_memory_leak.py index 505b78117228a..9ef0a10b80108 100644 --- a/tests/gateway/test_vision_memory_leak.py +++ b/tests/gateway/test_vision_memory_leak.py @@ -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): """... fenced block is scrubbed.""" leaked = ( diff --git a/tests/tools/test_vision_tools.py b/tests/tools/test_vision_tools.py index 5715603397818..e775280f08131 100644 --- a/tests/tools/test_vision_tools.py +++ b/tests/tools/test_vision_tools.py @@ -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 @@ -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 diff --git a/tools/vision_tools.py b/tools/vision_tools.py index 3c4724442c4c1..1761299983462 100644 --- a/tools/vision_tools.py +++ b/tools/vision_tools.py @@ -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. @@ -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: @@ -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: