Skip to content

fix(vision): honor supports_vision_tool_messages=False in tool-result media gates - #89987

Closed
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:liuhao/cron-bugfix-89981
Closed

liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:liuhao/cron-bugfix-89981

Conversation

@liuhao1024

Copy link
Copy Markdown

What does this PR do?

Fixes the native vision fast path sending image media in tool-result messages to providers that only accept images in user messages (#47742, re-reported as #89981 — same root cause, fixing the canonical issue).

A provider profile can declare supports_vision=True (user-message images work) together with supports_vision_tool_messages=False (list-type tool-result content is rejected). xiaomi/MiMo does exactly this and answers such content with 400 "text is not set". But _supports_media_in_tool_results() only read supports_vision from the profile, so the gate flipped to True, the fast path wrapped every vision result in the multimodal tool-result envelope, and the provider rejected it outright — the image never entered context and the turn errored.

Two gates now treat an explicit supports_vision_tool_messages=False as a hard veto:

  • _supports_media_in_tool_results returns False before the supports_vision check. Providers that do not declare the flag at all default to True and keep the existing behavior.
  • _should_use_native_vision_fast_path applies the same veto ahead of its capability-lookup escape hatch, so a models.dev / custom_providers vision-capable marking cannot re-open the envelope route for a provider that rejects the wire shape.

With the veto, these providers take the legacy aux-LLM vision path — the designed degradation for tool-result-media-incapable providers (images are analyzed by the aux vision model and return as text). The report's suggested deeper enhancement (re-injecting the image as a synthetic user message in tool_executor) composes on top of this and is intentionally left out of scope.

Related Issue

Fixes #47742
(also resolves the re-report #89981, marked duplicate of it)

Type of Change

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

Changes Made

  • tools/vision_tools.py: _supports_media_in_tool_results — the profile branch checks supports_vision_tool_messages first and returns False on an explicit False; _should_use_native_vision_fast_path — the same veto is applied before the capability-lookup disjunction.

How to Test

  1. pytest tests/tools/test_vision_native_fast_path.py -q — 14 passed (2 new tests).
  2. Fail-on-main verified: with the source stashed, both new tests fail on the unmodified tree (xiaomi/mimo resolve to True; the fast path fires even with the capability lookup patched to True).
  3. Adjacent suites: tests/tools/test_vision_tools.py 34 passed, tests/agent/test_image_routing.py 38 passed.
  4. Observed result: _supports_media_in_tool_results("xiaomi", "mimo-v2.5") is now False, so the fast path falls through to the aux-LLM path instead of emitting the multimodal envelope that the provider 400s.

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 15 (arm64)

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

… media gates

A provider profile declaring supports_vision=True together with
supports_vision_tool_messages=False (xiaomi/MiMo — user-message images
work, list-type tool-result content answers 400 "text is not set")
still passed _supports_media_in_tool_results: the profile check only
read supports_vision. The native fast path then wrapped every vision
result in the multimodal tool-result envelope, which such a provider
rejects outright — the image never entered context and the turn errored
(NousResearch#47742, re-reported as NousResearch#89981).

Two gates now treat an explicit supports_vision_tool_messages=False as
a hard veto:

- _supports_media_in_tool_results returns False before the
  supports_vision check (providers that do not declare the flag keep the
  existing behavior).
- _should_use_native_vision_fast_path applies the same veto ahead of its
  capability-lookup escape hatch, so a models.dev / custom_providers
  vision-capable marking cannot re-open the envelope route.

With the veto, these providers fall back to the legacy aux-LLM vision
path — the designed degradation for tool-result-media-incapable
providers.
@liuhao1024

Copy link
Copy Markdown
Author

Same pre-existing main regression as reported on #89968: slice 12 fails in test_image_generation.py::test_upscale_defaults_are_all_off on the unmodified-tree baseline too (verified locally on upstream/main — the Grok Imagine catalog entry from ceabb030f ships upscale: True, violating the opt-in-only policy that test pins). This diff only touches the vision tool-result gates in the Telegram-independent vision path; the failing suite is not exercised by it. The catalog fix is being tracked in the open PRs for that entry — once one lands on main, a rebase/rerun of this PR should go green.

@teknium1

teknium1 commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Merged via #101947 (commit d6bb94a) — your commit was cherry-picked onto current main with authorship preserved, trimmed to the two gate checks + 2 invariant tests. We also declared supports_vision_tool_messages=False on the bundled meta-ai profile in the same PR so #101668 rides along. Thanks @liuhao1024!

@teknium1 teknium1 closed this Sep 3, 2026
@alt-glitch alt-glitch added provider/other Calling models through a small provider without its own label (for example Kilo Code, Hugging Face I and removed provider/xiaomi labels Oct 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists provider/other Calling models through a small provider without its own label (for example Kilo Code, Hugging Face I sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades 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: native fast path sends images in tool results, breaking providers that only support vision in user messages

3 participants