fix(vision): pre-flight shrink oversized images before first API call - #62820
fix(vision): pre-flight shrink oversized images before first API call#62820calamarc wants to merge 1 commit into
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for targeting a real failure mode. Current main still sends a 5–20 MB auxiliary-vision image before recovery (tools/vision_tools.py:1173-1189, 1251-1267), so the underlying issue remains.
Problems
- The new regression test only calls
_resize_image_for_vision(tests/tools/test_vision_tools.py:938-972). It never callsvision_analyze_toolor inspects its firstasync_call_llmpayload, so it does not prove pre-flight behavior. - Current main routes encode/resize work through
_run_encode_on_cpu_executor(tools/vision_tools.py:176-189,1178-1188); the proposed direct synchronous resize (tools/vision_tools.py:756) needs to use that path when salvaged. browser_visionretains the same full-size-first, exception-only recovery pattern (tools/browser_tool.py:4060-4160).
Suggested changes
- Add an async end-to-end unit test that verifies the first auxiliary request carries a resized image.
- Preserve the bounded executor and apply the policy consistently to the browser auxiliary path.
- Make the lower cap provider-aware so the existing full-resolution behavior remains available for providers that accept larger images.
Automated hermes-sweeper review.
| len(image_data_url) / (1024 * 1024), | ||
| _RESIZE_TARGET_BYTES / (1024 * 1024), | ||
| ) | ||
| image_data_url = _resize_image_for_vision( |
There was a problem hiding this comment.
Current main now routes image encodes and resizes through _run_encode_on_cpu_executor (tools/vision_tools.py:176-189) so CPU-heavy image work cannot block the async event loop. Please preserve that bounded-executor path when salvaging this pre-flight resize.
| big_path.write_bytes(raw) | ||
|
|
||
| # Ensure the resize helper actually shrinks it under target. | ||
| result = _resize_image_for_vision(big_path, mime_type="image/jpeg") |
There was a problem hiding this comment.
This only verifies _resize_image_for_vision; it never invokes vision_analyze_tool or asserts the first async_call_llm payload. Add an async regression test that proves the initial API call receives the already-shrunk data URL.
…ser_vision
The native fast path already resizes down to _EMBED_TARGET_BYTES (4 MB)
and _EMBED_MAX_DIMENSION (7900 px) before embedding an image into history.
The auxiliary-LLM paths (vision_analyze_tool + browser_vision) still sent
full-resolution images to the first async_call_llm / call_llm invocation
and only reacted after a HTTP 400 — burning all 3 retries when the payload
was truly oversized (e.g. Anthropic Claude Haiku's 5 MB per-image ceiling
or 8000 px per-side dimension limit).
Observed in production: three separate 6+ MB image failures in one gateway
session (6.0 MB, 6.3 MB, 9.9 MB), all exhausting retries.
Changes:
- Introduce _AUX_VISION_TARGET_BYTES / _AUX_VISION_MAX_DIMENSION (aliases
of the embed constants) so the aux-LLM paths share the same proactive
thresholds as the native fast path.
- vision_analyze_tool: after base64 encode, run the same over-bytes /
over-dims check as the fast path and route the resize through the
bounded CPU executor (_run_encode_on_cpu_executor). Preserves executor
invariants and avoids blocking the event loop.
- browser_vision: same pre-flight before the first call_llm on the
non-native path (native fast-path is unchanged because it already
goes through the fast-path embed check).
- _RESIZE_TARGET_BYTES (5 MB) retained for reactive shrink paths;
proactive paths use _AUX_VISION_TARGET_BYTES (4 MB) for headroom.
Tests: adds TestAuxVisionPreFlightShrink to tests/tools/test_vision_tools.py
with two async e2e cases that mock async_call_llm and inspect the FIRST
call's payload:
* test_first_api_call_payload_is_pre_shrunk — a real 6+ MB JPEG must
arrive at the first API call already ≤ 4 MB base64.
* test_small_image_untouched_by_preflight — small images pass through
unchanged so aux-vision quality isn't degraded unnecessarily.
Addresses hermes-sweeper review feedback on NousResearch#62820:
✓ Test verifies vision_analyze_tool's first async_call_llm payload
(not just _resize_image_for_vision in isolation).
✓ Uses _run_encode_on_cpu_executor for the salvaged pre-flight.
✓ Extends the same policy to browser_vision.
Not addressed: making the target provider-aware. Aux vision routes
predominantly to Anthropic Claude Haiku today, and the fast-path embed
already uses the same 4 MB / 7900 px thresholds for all providers,
so a per-provider knob would introduce inconsistency between the two
paths without concrete evidence any current provider suffers from the
4 MB cap. Happy to revisit if evidence shows a specific provider needs
the extra headroom.
435b5f5 to
b6782a7
Compare
|
Thanks for the review — you were right that main had already moved past what I was patching. Rebased onto latest main and reworked the fix to align with the existing architecture. What changedRebase + rework. The initial 5→4.5 MB constant tighten is gone; main already has Addressing each point:
Test resultsForce-pushed to the same branch. |
Problem
Anthropic's Messages API rejects images >5,242,880 bytes (5 MB base64) with a hard HTTP 400:
Observed in production 3x in one session — real 6.0 MB, 6.3 MB, and 9.9 MB images all burned all 3 retries and never recovered.
Root Cause
The existing shrink recovery in
tools/vision_tools.pywas reactive-only (fires after a 400) AND used a target of5 * 1024 * 1024— exactly Anthropic's ceiling with zero headroom for base64 padding + provider-side rounding. So even when reactive shrink triggered, the resized payload could still trip the wire.Fix
Two changes, both in
tools/vision_tools.py:Tighten
_RESIZE_TARGET_BYTESfrom5 * 1024 * 1024to4_500_000— real headroom under Anthropic's per-image ceiling.Add proactive pre-flight in
vision_analyze_tool: if base64 payload exceeds target, resize BEFORE the firstasync_call_llmcall. Prevents the 3-retry HTTP churn on unrecoverable size errors.The reactive path (
_try_shrink_image_parts_in_messagesinrun_agent.py) is unchanged — it already uses a 4 MB target and benefits transitively.Tests
test_constants_saneto lock in the 4.5 MB target and assert it stays strictly under Anthropic's 5 MB cap.TestPreFlightShrink— generates a real 6+ MB JPEG and asserts the shrink loop converges under target.