Conversation
Duplicate of #10852 -- both cap the gateway auto image-preprocess path at |
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Comment (prior automated review noted)
This PR has a prior automated COMMENT review. No new issues found.
Looks Good
- Caps gateway vision preprocessing output — appropriate bounds check
- No security concerns
teknium1
left a comment
There was a problem hiding this comment.
Thanks for narrowing the change to gateway auto-preprocessing and preserving the public vision_analyze schema.
Problems
gateway/run.py:15045passesmax_tokens=500, but this does not cap the affected common path today.vision_analyze_toolcallsasync_call_llm;agent/auxiliary_client.py:7120-7124forwards the value into_build_call_kwargs, andagent/auxiliary_client.py:6222-6254deliberately omitsmax_tokensfor ordinary OpenAI-compatible/custom providers. The issue's reported llama-server custom-provider path therefore still receives no 500-token wire cap.- The new test in
tests/tools/test_vision_tools.pymocksasync_call_llm, so it verifies only that the wrapper receives500, not that the provider request carries it. Existingtests/agent/test_auxiliary_client.py:278-331covers the intentional omission behavior.
Suggested changes
- Preserve the concise prompt, then add a narrowly scoped, provider-compatible forced-cap path through
agent/auxiliary_client.pyrather than relying on the current general auxiliary argument plumbing. - Test the underlying async client's request kwargs for the target custom/OpenAI-compatible route.
Automated hermes-sweeper review.
| @@ -15041,6 +15042,7 @@ async def _enrich_message_with_vision( | |||
| result_json = await vision_analyze_tool( | |||
| image_url=path, | |||
| user_prompt=analysis_prompt, | |||
| max_tokens=500, | |||
There was a problem hiding this comment.
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.
73e2c1a to
ff299a7
Compare
Signed-off-by: Ho Lim <166576253+HOYALIM@users.noreply.github.com>
Signed-off-by: Ho Lim <subhoya@gmail.com>
ff299a7 to
850f2ed
Compare
Problem
Fixes #10809. The gateway's automatic image preprocessing asks for a detailed description and uses the vision tool's 2000 token default, which can produce large intermediate context before the user's actual model call.
Solution
Add an internal per-call
max_tokensoverride tovision_analyze_tooland use a concise 2-4 sentence prompt with a 500 token cap only for the gateway auto-preprocessing path. The public tool schema and default 2000 token behavior remain unchanged.Impact
Caps gateway auto-preprocessing output at 500 tokens instead of 2000, a 75% lower ceiling for this intermediate vision context.
Validation
PYTHONDONTWRITEBYTECODE=1 /Users/holim/code/hermes-agent/.venv/bin/python -m pytest tests/gateway/test_vision_memory_leak.py -qscripts/run_tests.sh tests/gateway/test_vision_memory_leak.py tests/tools/test_vision_tools.py -q/Users/holim/code/hermes-agent/.venv/bin/ruff check gateway/run.py tools/vision_tools.py tests/gateway/test_vision_memory_leak.py tests/tools/test_vision_tools.pygit diff --check