fix(vision): respect auxiliary vision routing - #24875
Conversation
3eb9174 to
00904ef
Compare
|
Rebased on latest Local verification:
(Still seeing no CI statuses reported on the PR head; may need maintainer Actions approval for fork PRs.) |
00904ef to
51d7342
Compare
51d7342 to
318cc96
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating the stale browser model override; that path remains real on current main: tools/browser_tool.py:4103 reads AUXILIARY_VISION_MODEL and :4140-4144 forwards it explicitly, while agent/auxiliary_client.py:5866-5902 gives explicit models precedence over auxiliary.vision.
Problems
- The
vision_analyzechange removes the environment-only compatibility override.AUXILIARY_VISION_MODELremains documented atwebsite/docs/reference/environment-variables.md:801, and current coverage attests/tools/test_vision_tools.py:220-238asserts that it is forwarded. The task-config helper reads config only (agent/auxiliary_client.py:5996-6037), so passingNonedoes not preserve this fallback.
Suggested changes
- Keep config authoritative while retaining environment-only fallback, either in a shared resolver or with config-first resolution at the tool boundary.
- Cover both config-vs-stale-env precedence and env-only compatibility through the real auxiliary resolution path.
Automated hermes-sweeper review.
| @@ -1043,8 +1044,7 @@ def _handle_vision_analyze(args: Dict[str, Any], **kw: Any) -> Awaitable[str]: | |||
| "Fully describe and explain everything about this image, then answer the " | |||
| f"following question:\n\n{question}" | |||
| ) | |||
There was a problem hiding this comment.
Passing None unconditionally removes the documented environment-only AUXILIARY_VISION_MODEL override. Please retain it as a fallback when auxiliary.vision.model is unset, while making config win when both are present.
Summary
Fixes #24842
Tests