Skip to content

fix(gateway): retry auto-vision on transient failure (#28972) - #28999

Closed
xxxigm wants to merge 3 commits into
NousResearch:mainfrom
xxxigm:fix/28972-vision-auto-retry
Closed

xxxigm wants to merge 3 commits into
NousResearch:mainfrom
xxxigm:fix/28972-vision-auto-retry

Conversation

@xxxigm

@xxxigm xxxigm commented May 20, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Discord-cached image attachments routinely come back success: false from the first vision_analyze call inside _enrich_message_with_vision, even though calling the tool again against the exact same local path succeeds. The reporter observed the failure on every Discord session with an image in their logs.

The agent then sees the kawaii fallback string "couldn't quite see it this time (>_<)", recognises it, and reissues vision_analyze manually — costing ~30 s of reasoning latency and one wasted tool call per affected image, in every session.

Root cause analysis. Tracing the code path:

  • gateway/platforms/discord.py::_cache_discord_image writes the attachment via cache_image_from_bytes, which uses synchronous filepath.write_bytes(data) — the file is fully on disk before the path is returned.
  • _handle_message propagates the path through event.media_urls, and _prepare_event_text calls _enrich_message_with_vision(text, image_paths).
  • vision_analyze_tool only returns success: false when an exception is caught internally (timeout, empty content, transient 5xx, rate limit). The "permanent" failures (image too large, insufficient credits, model doesn't support vision) all also surface this way.

So the reporter's "timing race in Discord adapter" hypothesis isn't quite right — the file IS on disk. The actual failure mode is transient API errors that resolve on a second attempt ~30 s later when the agent reissues the call manually.

Fix. Add a bounded inline retry inside _enrich_message_with_vision (the reporter's preferred Option 1):

  • Default 1 retry (configurable via HERMES_VISION_AUTO_RETRIES; set to 0 to opt out and restore the legacy single-shot behaviour).
  • Exponential backoff starting at 0.6 s, capped at 3 s.
  • Permanent-failure classifier (_vision_failure_is_retryable) short-circuits the retry budget so we don't waste API calls on image too large / insufficient credits / does not support vision / SSRF block / interrupt. Both the error and analysis JSON fields participate in the match.
  • Exceptions still bubble out of the helper, so the existing "something went wrong" branch in _enrich_message_with_vision continues to fire for non-transient failures like missing API keys.

Cost on the happy path: zero extra API calls. Cost on a transient failure: 1 extra call instead of the current 1 (manual by the agent) + ~30 s reasoning. Cost on a permanent failure: 1 call, same as today.

Related Issue

Fixes #28972

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature
  • 🔒 Security fix
  • 📝 Documentation update (new env var documented)
  • ✅ Tests (35 new regression tests)
  • ♻️ Refactor
  • 🎯 New skill

Changes Made

  • gateway/run.py — Introduce _vision_auto_retry_count, _vision_failure_is_retryable, _vision_analyze_with_auto_retry helpers plus _VISION_AUTO_RETRY_COUNT_DEFAULT, _VISION_AUTO_RETRY_INITIAL_BACKOFF_S, _VISION_AUTO_RETRY_MAX_BACKOFF_S, _VISION_NONRETRYABLE_HINTS class constants. _enrich_message_with_vision delegates the tool call to the retry helper. Sub-200-line change in a single file.
  • tests/gateway/test_vision_auto_retry.py — 35 new tests in five classes covering: env var resolution (unset/zero/explicit/negative/garbage/whitespace), permanent-vs-transient classification (parametrised), the retry loop (happy path, transient-then-success, permanent short-circuit, all-fail, env opt-out, exception propagation), the public entry point (the [Bug]: [Discord] Auto-vision returns success=false on attachments, forcing duplicate vision_analyze call per image #28972 repro, kawaii-fallback preserved, no-retry on happy path, multi-image budget isolation), and structural invariants (default ≥ 1, lowercase hints, hint table covers known permanent errors).
  • tests/gateway/test_vision_memory_leak.py — Extend the existing _Stub fixture to bind the new helpers so the sanitize-context regression coverage continues to exercise the real code path.
  • website/docs/reference/environment-variables.md — Document HERMES_VISION_AUTO_RETRIES next to HERMES_VISION_DOWNLOAD_TIMEOUT.

How to Test

Reproduce the bug on main:

# Configure a Discord adapter, then from Discord send an image
# attachment to your bot.  In the resulting session:
hermes sessions list   # find the latest session
hermes logs --since 10m | grep -i "couldn't quite see"

On main you'll see the kawaii fallback embedded in the model's first user message, followed shortly by a duplicate vision_analyze tool call.

After this PR:

# Same flow.  The fallback string no longer appears and there's
# no duplicate vision_analyze in the tool-call log.

For operators on metered providers who prefer the legacy behaviour:

HERMES_VISION_AUTO_RETRIES=0 hermes gateway run

Automated coverage:

scripts/run_tests.sh tests/gateway/test_vision_auto_retry.py tests/gateway/test_vision_memory_leak.py -q
# 38 passed in 1.29s

scripts/run_tests.sh tests/gateway/test_vision_auto_retry.py tests/gateway/test_vision_memory_leak.py tests/gateway/test_discord_channel_prompts.py tests/gateway/test_fast_command.py tests/agent/test_image_routing.py -q
# 86 passed in 6.37s

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(gateway):, test(gateway):, docs(gateway):)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix
  • I've run the relevant tests locally and they pass
  • I've added tests for my changes (35 new regression tests)
  • Tested on my platform: macOS 15.2 (Darwin 24.6.0). The retry logic itself is platform-independent.

Documentation & Housekeeping

  • Updated website/docs/reference/environment-variables.md with the new env var
  • N/A — no config keys added; the knob is env-var-only by design (transient-retry tuning is an operator concern, not a per-session setting)
  • N/A — no architecture or workflow changes
  • Cross-platform — the fix lives in shared gateway code and has no platform-specific assumptions (the issue surfaces most often on Discord but the retry helps any platform that auto-enriches images)
  • N/A — no tool descriptions / schemas changed

Screenshots / Logs

Before — every Discord session with an image (per reporter)

The model's first user message ends up containing:

[The user sent an image but I couldn't quite see it this time (>_<)
You can try looking at it yourself with vision_analyze using
image_url: /Users/.../cache/images/img_abc123.png]

Followed by:

[Tool call: vision_analyze(image_url=/Users/.../cache/images/img_abc123.png, …)]
[Tool result: {"success": true, "analysis": "..."}]

That's one wasted tool call + ~30 s of agent reasoning, per image, per session.

After

The retry layer absorbs the transient failure invisibly. The model's first user message contains the happy-path descriptor:

[The user sent an image~ Here's what I can see:
A photograph of …]
[If you need a closer look, use vision_analyze with
image_url: /Users/.../cache/images/img_abc123.png ~]

No follow-up vision_analyze tool call. The retry shows up only in the gateway log:

INFO  gateway.run: vision_analyze retry 1/1 for /Users/.../cache/images/img_abc123.png after transient failure; sleeping 0.60s

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery tool/vision Vision analysis and image generation labels May 20, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the focused fix. I verified the underlying bug path still exists on current main: gateway/run.py:11952-11969 calls vision_analyze_tool once and emits the kawaii fallback immediately when the result has success: false.

Problems

  • The retry knob is implemented as a new user-facing env var: gateway/run.py:14237 reads HERMES_VISION_AUTO_RETRIES, and website/docs/reference/environment-variables.md:507 documents it. That conflicts with the repo rubric in AGENTS.md:102-106, which says non-secret behavioral settings belong in config.yaml, not new HERMES_* env vars.
  • Salvage will need conflict resolution: the PR patch targets the old gateway/run.py:14184 neighborhood, while current main has _enrich_message_with_vision around gateway/run.py:11918.

Suggested changes

  • Move the retry budget into config.yaml, preferably under the existing auxiliary vision configuration surface, and update the tests/docs to cover that config path.
  • Keep the inline bounded retry behavior; the premise is real, and the classifier/backoff shape looks like the right layer for the reported failure.

Automated hermes-sweeper review.

Comment thread gateway/run.py Outdated
back to the documented default so a typo never silently
disables the safety net.
"""
raw = os.environ.get("HERMES_VISION_AUTO_RETRIES")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not be a new user-facing HERMES_* env var. AGENTS.md:102-106 says non-secret behavioral settings like retry budgets belong in config.yaml; please route this through the existing config surface, likely under auxiliary vision settings, and update the docs/tests accordingly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@teknium1 The PR has been updated!

xxxigm added 3 commits June 15, 2026 21:46
)

Discord-cached image attachments occasionally come back success: false from
the first vision_analyze call inside _enrich_message_with_vision even though a
second call against the same local path succeeds. The agent sees the kawaii
fallback string, recognises it, and reissues vision_analyze manually — costing
~30s and one wasted tool round-trip per affected image, every session.

Add a bounded inline retry inside _enrich_message_with_vision: default 1 retry,
exponential backoff capped at 3s, with a permanent-failure classifier that
short-circuits the budget (image too large, insufficient credits, model does
not support vision, SSRF block, interrupt). Exceptions still bubble out so the
existing "something went wrong" branch keeps owning non-transient failures.

The retry budget is a non-secret behavioral setting, so per AGENTS.md it lives
in config.yaml under auxiliary.vision.auto_retries (0 = legacy single-shot),
not a new HERMES_* env var.
New test_vision_auto_retry.py exercises the retry budget resolver
(auxiliary.vision.auto_retries from config.yaml, including default/zero/
clamp/garbage/load-failure paths), the transient-vs-permanent classifier,
the retry loop, and the public _enrich_message_with_vision entry point
(the NousResearch#28972 repro, kawaii fallback preserved, per-image budget isolation).
The existing test_vision_memory_leak.py fixture is extended to bind the new
retry helpers so its sanitize-context coverage exercises the real path.
)

Document the gateway auto-vision retry budget in the canonical commented
example next to the other auxiliary.vision knobs.
@xxxigm
xxxigm force-pushed the fix/28972-vision-auto-retry branch from 0de8763 to f3918db Compare June 15, 2026 14:47
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for the focused investigation and for updating the retry setting to config.yaml after the earlier review.

Automated hermes-sweeper review found this behavior is already implemented on current main:

  • tools/vision_tools.py:1253 sends auxiliary vision requests through async_call_llm(task="vision").
  • agent/auxiliary_client.py:7143-7168 retries transient transport failures on the same provider before fallback; the covered class includes connection failures, 5xx responses, and HTTP 408.
  • tools/vision_tools.py:1274-1278 separately retries an empty vision response once.
  • This shipped in fb403a3a730f2a4eab1cfc60ffe854f7ea7c4159 (fix(auxiliary): retry transient blips harder + isolate client cache per model (#56889)), contained in v2026.7.7.

Closing as implemented on main.

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

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists sweeper:implemented-on-main Sweeper: behavior already present on current main 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.

[Bug]: [Discord] Auto-vision returns success=false on attachments, forcing duplicate vision_analyze call per image

3 participants