Skip to content

fix(tools): preserve auxiliary config model precedence - #39830

Closed
yinkev wants to merge 1 commit into
NousResearch:mainfrom
yinkev:fix-aux-tool-config-precedence
Closed

fix(tools): preserve auxiliary config model precedence#39830
yinkev wants to merge 1 commit into
NousResearch:mainfrom
yinkev:fix-aux-tool-config-precedence

Conversation

@yinkev

@yinkev yinkev commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Summary

  • stop stale AUXILIARY_VISION_MODEL / AUXILIARY_WEB_EXTRACT_MODEL bridge env vars from overriding configured auxiliary.<task>.model values
  • let browser vision, browser page extraction, standalone vision, and web extraction route through the centralized auxiliary resolver when config already names the task model
  • keep env-only setups working as the fallback when config does not set a task model

Fixes #14693

Notes on related PRs

There are older/open vision-routing PRs in this area. This change is intentionally focused on the stale env bridge precedence reported in #14693 and also covers the web-extract path (tools/web_tools.py) so auxiliary.web_extract.model is not defeated by AUXILIARY_WEB_EXTRACT_MODEL.

Tests

  • scripts/run_tests.sh tests/tools/test_auxiliary_tool_model_precedence.py tests/tools/test_vision_tools.py tests/tools/test_web_tools_config.py tests/tools/test_browser_content_none_guard.py
  • .venv/bin/python -m py_compile tools/browser_tool.py tools/vision_tools.py tools/web_tools.py tests/tools/test_auxiliary_tool_model_precedence.py
  • git diff --check

Verification details

The new regression tests cover:

  • browser vision: config model suppresses stale env override; env remains fallback without config
  • browser extraction: config model suppresses stale env override; env remains fallback without config
  • standalone vision handler: config model suppresses stale env override; env remains fallback without config
  • web extraction: config/default model wins over stale env, while an explicit call model still wins over both

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/tools Tool registry, model_tools, toolsets tool/vision Vision analysis and image generation tool/web Web search and extraction tool/browser Browser automation (CDP, Playwright) labels Jun 5, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Fixes #14693. Competes with open PRs #24875 and #24872 (same auxiliary vision-routing area).

@yinkev

yinkev commented Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for flagging. I do not think this is fully redundant with #24875/#24872.

The overlap is limited to the vision side. #14693 is specifically about stale tool-layer AUXILIARY_*_MODEL bridge env vars overriding current auxiliary..model config, and its scope includes browser extraction / web_extract, not just vision_analyze and browser_vision.

Patch comparison:

Recommendation: keep #39830 open as the PR covering the full #14693 scope. If maintainers prefer #24875's vision approach, the web_extract/browser-extraction coverage from #39830 should still be kept or cherry-picked before closing it as redundant.

@DavidMetcalfe

Copy link
Copy Markdown
Contributor

Superseded by merged #57651 (commit 0e9136c), which consolidates the config-precedence work here. Closing to avoid confusion.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for isolating the tool-layer precedence issue. The browser portion remains relevant: tools/browser_tool.py:380-387 still reads the legacy environment variables, and their values become explicit model= arguments at tools/browser_tool.py:2627-2630 and :4140-4144; explicit model arguments take priority over task config in agent/auxiliary_client.py:5878-5913.

Problems

  • tools/web_tools.py no longer has _resolve_web_extract_auxiliary; commit ee8cbfdc03eb9b7cdd486165486f2e3cad0d8645 removed that LLM summarization path. That hunk cannot apply on current main.
  • The vision_tools.py behavior is already on main at tools/vision_tools.py:1498-1510, covered by tests/tools/test_vision_tools.py:266-313 from 149641485c7f5bcb33f6acf11544f0a816d8b054.
  • Current vision_analyze is async (tools/vision_tools.py:1473), so the new test's coroutine-close pattern is no longer valid.

Suggested changes

  • Retain and adapt only the browser precedence fix plus current-main-compatible browser regression coverage.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 14, 2026
@alt-glitch alt-glitch added needs-decision Awaiting maintainer decision before any implementation and removed tool/web Web search and extraction tool/vision Vision analysis and image generation labels Jul 17, 2026
@yinkev

yinkev commented Jul 25, 2026

Copy link
Copy Markdown
Contributor Author

Closing the broad stale branch because most of its scope was superseded by merged #57651 and current main removed or changed several targeted paths. The remaining browser-only precedence gap should be independently reproduced and, if still live, submitted as a fresh narrow patch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tools Tool registry, model_tools, toolsets needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades tool/browser Browser automation (CDP, Playwright) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tool-layer env vars defeat config.yaml auxiliary routing for vision and extraction models

4 participants