Skip to content

fix(vision): read auxiliary model from config.yaml before env var - #53754

Closed
liuhao1024 wants to merge 8 commits into
NousResearch:mainfrom
liuhao1024:liuhao/cron-bugfix-53749-vision-aux-model-config
Closed

fix(vision): read auxiliary model from config.yaml before env var#53754
liuhao1024 wants to merge 8 commits into
NousResearch:mainfrom
liuhao1024:liuhao/cron-bugfix-53749-vision-aux-model-config

Conversation

@liuhao1024

Copy link
Copy Markdown
Contributor

What does this PR do?

The vision_analyze and video_analyze tool handlers read the auxiliary model name exclusively from environment variables (AUXILIARY_VISION_MODEL / AUXILIARY_VIDEO_MODEL), ignoring the auxiliary.vision.model and auxiliary.video.model settings in config.yaml. This creates a routing inconsistency: the gateway's Telegram/WhatsApp image path reads from config correctly (via run_agent.pyvision_analyze_tool), but the direct tool-call path (_handle_vision_analyzevision_analyze_tool) does not.

This PR makes both handlers prefer config.yaml settings with the env var as a legacy fallback, matching the pattern already used for auxiliary.vision.timeout and auxiliary.vision.temperature in the same file.

Related Issue

Fixes #53749

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • tools/vision_tools.py_handle_vision_analyze: read auxiliary.vision.model from config.yaml before falling back to AUXILIARY_VISION_MODEL env var
  • tools/vision_tools.py_handle_video_analyze: read auxiliary.video.model (falling back to auxiliary.vision.model) from config.yaml before falling back to env vars
  • tests/tools/test_vision_tools.py — added test_config_yaml_model_takes_priority_over_env and test_env_var_used_when_config_missing_model

How to Test

  1. Run python -m pytest tests/tools/test_vision_tools.py::TestHandleVisionAnalyze -xvs — all 7 tests should pass
  2. Set auxiliary.vision.model: some-model in config.yaml and AUXILIARY_VISION_MODEL=different-model in env → call vision_analyze tool → should use some-model (from config), not different-model (from env)
  3. Remove auxiliary.vision.model from config.yaml, keep env var → should fall back to env var value

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 26.4.1

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

_handlers for vision_analyze and video_analyze read model name from
config.yaml (auxiliary.vision.model / auxiliary.video.model) before
falling back to AUXILIARY_VISION_MODEL / AUXILIARY_VIDEO_MODEL env
vars.  Matches the existing config-first pattern for timeout and
temperature in the same file.

Fixes NousResearch#53749
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists tool/vision Vision analysis and image generation comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint area/config Config system, migrations, profiles labels Jun 27, 2026
teknium1 added a commit that referenced this pull request Jul 3, 2026
…ision_analyze

The salvaged tests from #53754 predate _handle_vision_analyze becoming
async and the native fast path; await the handler and force the legacy
aux path so the model-resolution assertion is actually exercised.
teknium1 added a commit that referenced this pull request Jul 3, 2026
…ision_analyze

The salvaged tests from #53754 predate _handle_vision_analyze becoming
async and the native fast path; await the handler and force the legacy
aux path so the model-resolution assertion is actually exercised.
@teknium1

teknium1 commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Merged via PR #57651 alongside #50685. Your commit was cherry-picked onto current main with your authorship preserved in git log (rebase-merge). We adapted the two config-priority tests to the now-async _handle_vision_analyze and gated the native fast path off so they exercise the legacy path they assert on. Thanks!

@teknium1 teknium1 closed this Jul 3, 2026
habarmc1223-sudo pushed a commit to habarmc1223-sudo/hermes-agent-fluxmem that referenced this pull request Jul 8, 2026
…ision_analyze

The salvaged tests from NousResearch#53754 predate _handle_vision_analyze becoming
async and the native fast path; await the handler and force the legacy
aux path so the model-resolution assertion is actually exercised.
santhreal pushed a commit to santhreal/hermes-agent that referenced this pull request Jul 13, 2026
…ision_analyze

The salvaged tests from NousResearch#53754 predate _handle_vision_analyze becoming
async and the native fast path; await the handler and force the legacy
aux path so the model-resolution assertion is actually exercised.
Gravezzz pushed a commit to Gravezzz/hermes-agent that referenced this pull request Jul 21, 2026
…ision_analyze

The salvaged tests from NousResearch#53754 predate _handle_vision_analyze becoming
async and the native fast path; await the handler and force the legacy
aux path so the model-resolution assertion is actually exercised.
leewenjie pushed a commit to leewenjie/hermes-agent that referenced this pull request Aug 7, 2026
…ision_analyze

The salvaged tests from NousResearch#53754 predate _handle_vision_analyze becoming
async and the native fast path; await the handler and force the legacy
aux path so the model-resolution assertion is actually exercised.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists tool/vision Vision analysis and image generation type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vision_analyze tool reads aux model from env var, not config.yaml auxiliary.vision.model

3 participants