Skip to content

fix(agent): accept pixel-correct image downscale when bytes grow (#48013) - #49124

Open
alt-glitch wants to merge 1 commit into
mainfrom
fix/48013-shrink-accept-pixel-correct
Open

fix(agent): accept pixel-correct image downscale when bytes grow (#48013)#49124
alt-glitch wants to merge 1 commit into
mainfrom
fix/48013-shrink-accept-pixel-correct

Conversation

@alt-glitch

Copy link
Copy Markdown
Collaborator

Summary

Fixes the image-dimension 400 session brick (#48013). The reactive image-shrink recovery (try_shrink_image_parts_in_messages) silently discarded a pixel-correct downscale whenever the re-encoded PNG was larger in bytes than the original — the common case for downscaled Retina screenshots. The image was left at its original oversized dimensions, the provider re-rejected it on the one-shot retry, and the session wedged forever on the Anthropic many-image 2000px path (the default anthropic / openrouter→anthropic route).

The recovery machinery already parses the correct ceiling (#45979) and _resize_image_for_vision already produces the correct downscale — the defect was one layer down, in the accept/reject gate:

# agent/conversation_compression.py (before)
if not resized or len(resized) >= len(url):
    return None      # discards a result that IS under the pixel cap,
                     # purely because re-encoded PNG bytes grew

The success criterion was byte size, but the failing constraint was pixel dimensions. The two were conflated.

Fix

Track which constraint triggered the shrink (bytes vs dimension) and gate the accept on the same axis:

  • dimension path — accept the result as long as it is now within max_dimension, regardless of byte size (verify via Pillow; fall back to the byte gate only when the re-encode can't be decoded).
  • bytes path — still require bytes to shrink, but also re-check the per-side cap when it's active. _resize_image_for_vision returns a best-effort, possibly-over-cap blob when it exhausts its halving budget on a very-high-aspect image (it freezes the long side once the short side hits its 64px floor), so a byte-shrink alone can leave it over the dimension cap and re-brick on retry.
  • Extend the unshrinkable_oversized guard to the pixel axis so a partial shrink doesn't burn the one-shot retry.

Single shared agent path → fixes CLI, TUI, and gateway alike (all surfaces go through the same try_shrink_image_parts_in_messages call site in agent/conversation_loop.py).

Proof

tests/run_agent/repro_48013_image_shrink_brick.py is a runnable, real-Pillow fixture (no mocks) that reproduces the issue's exact per-image table:

$ python tests/run_agent/repro_48013_image_shrink_brick.py
  2344x778  ( 10 KB) -> changed= True  result=1172x389  [OK]
  2374x1144 ( 14 KB) -> changed= True  result=1187x572  [OK]
  2097x1476 ( 16 KB) -> changed= True  result=1048x738  [OK]   # bricked before
  2247x1544 ( 18 KB) -> changed= True  result=1123x772  [OK]   # bricked before
  2263x1644 ( 19 KB) -> changed= True  result=1131x822  [OK]   # bricked before
PASS: all 5 dimension-oversized screenshots brought under 2000px and reported as progress.

Stashing the fix flips it to FAIL: 3 image(s) still over the pixel cap (BRICK) (exit 1) — it's a discriminating proof, not a tautology.

Tests

tests/run_agent/test_image_shrink_recovery.py (21 tests, all green): dimension accept-on-byte-growth, dimension-failure-blocks-retry, mixed partial-progress accounting, and the bytes-path still-over-cap regression (verified to fail without the fix). Full related compression suite (123 tests) passes.

Relationship to the other open PRs

Closes #48013

)

The image-too-large reactive shrink (try_shrink_image_parts_in_messages)
conflated two independent constraints: it always rejected a resize whose
re-encoded bytes were >= the original, even when the shrink was driven by a
PIXEL-DIMENSION cap (Anthropic many-image 2000px) rather than the byte budget.
Downscaled screenshot PNGs routinely re-encode LARGER in bytes, so the
dimension-correct result was discarded and the image left oversized -> the
provider re-rejected on retry and the session wedged forever.

Fix: track which constraint triggered the shrink (bytes vs dimension) and gate
the accept on the SAME axis.
  * dimension path: accept the result as long as it is now within max_dimension,
    regardless of byte size (verify via Pillow; fall back to the byte gate only
    when the re-encode can't be decoded).
  * bytes path: still require bytes to shrink, but ALSO re-check the per-side cap
    when it's active — _resize_image_for_vision returns a best-effort, possibly
    over-cap blob when it exhausts its halving budget on a very-high-aspect
    image, so a byte-shrink alone can leave it over the dimension cap and
    re-brick on retry.
Extend the unshrinkable-oversized guard to the pixel axis so a partial shrink
doesn't burn the one-shot retry.

Single shared agent path -> fixes CLI, TUI, and gateway alike.

Adds a real-Pillow runnable proof (repro_48013_image_shrink_brick.py) that
reproduces the issue's per-image table (bricks 3/5 before, passes 5/5 after)
plus unit invariants for the dimension and bytes accept/reject paths,
partial-progress accounting, and the bytes-path still-over-cap regression
surfaced by adversarial review.

Closes #48013
@github-actions

Copy link
Copy Markdown
Contributor

🔎 Lint report: fix/48013-shrink-accept-pixel-correct vs origin/main

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 11159 on HEAD, 11159 on base (➖ 0)

🆕 New issues (3):

Rule Count
unresolved-import 2
invalid-assignment 1
First entries
tests/run_agent/repro_48013_image_shrink_brick.py:41: [unresolved-import] unresolved-import: Cannot resolve imported module `PIL`
tests/run_agent/repro_48013_image_shrink_brick.py:33: [unresolved-import] unresolved-import: Cannot resolve imported module `pytest`
tests/run_agent/test_credits_notices_toggle.py:76: [invalid-assignment] invalid-assignment: Object of type `None` is not assignable to attribute `_credits_session_start_micros` of type `int`

✅ Fixed issues (2):

Rule Count
unresolved-attribute 2
First entries
run_agent.py:2971: [unresolved-attribute] unresolved-attribute: Object of type `Self@get_credits_spent_micros` has no attribute `_credits_session_start_micros`
tests/run_agent/test_credits_notices_toggle.py:76: [unresolved-attribute] unresolved-attribute: Unresolved attribute `_credits_session_start_micros` on type `AIAgent`

Unchanged: 5846 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint tool/vision Vision analysis and image generation provider/anthropic Anthropic native Messages API P2 Medium — degraded but workaround exists duplicate This issue or pull request already exists labels Jun 19, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator Author

Duplicate of #48035 — both fix #48013 with the same mechanism in agent/conversation_compression.py: the image-shrink recovery's accept/reject gate (if not resized or len(resized) >= len(url): return None) discarded a pixel-correct downscale whenever the re-encoded PNG grew in bytes, leaving Retina screenshots over the dimension cap and burning the one-shot retry. Both PRs make _shrink_data_url track which constraint triggered the shrink (bytes vs dimension) and gate acceptance on that same axis — accepting a dimension-correct result regardless of byte size (verifying pixel dimensions via Pillow, falling back to the byte gate when undecodable). #48035 (by @Tranquil-Flow) is the earlier still-open version (2026-06-17).

@jakepresent

Copy link
Copy Markdown
Contributor

Local validation on PR head 1630737c4:

python -m pytest tests/run_agent/test_image_shrink_recovery.py -q -o 'addopts='
# 21 passed, 1 warning
python -m py_compile agent/conversation_compression.py tests/run_agent/test_image_shrink_recovery.py
# passed

The implementation and tests look sound in isolation, but this now appears superseded by main: commit 990273d90 (fix(agent): accept pixel-correct image downscale when bytes grow (#48013)) is already present on upstream/main and contains the same binding-constraint fix plus the byte-path dimension check. This PR is currently dirty against main; unless there is a residual gap not covered by 990273d90, it looks safe to close as implemented on main.

@alt-glitch

Copy link
Copy Markdown
Collaborator Author

Call-graph-assisted review (calldiff over agent/conversation_compression.py). The call tree confirms the change is surgical: _shrink_data_url now returns (resized_url, unshrinkable) and the two caller branches collapse from an inline startswith("data:") and len(...) > target_bytes re-check to a single elif unshrinkable. Nice — the old code recomputed the "is this oversized" decision at the call site using only the byte axis, which is exactly the bug this fixes.

✅ Looks good

  • The root-cause framing is correct and well-documented: PNG re-encode is non-monotonic in raster size, so a pixel-correct LANCZOS downscale can grow in bytes; the old len(resized) >= len(url) gate rejected those and left the image over the per-side cap, re-400ing forever (Image-dimension 400 bricks sessions: shrink recovery discards a pixel-correct downscale when re-encoded PNG bytes grow (survives #45979) #48013).
  • Separating triggered_by ("bytes" vs "dimension") and gating acceptance against the same axis that triggered the shrink is the right invariant. The dimension-path now accepts a byte-larger blob as long as it's under max_dimension, and the byte-path still additionally rejects an output that's over the cap — both directions covered.
  • _decode_pixels cleanly extracts the Pillow-soft-dependency logic that was previously inlined, and every None branch falls back to historical byte-only behavior, so Pillow-absent installs don't regress.
  • The repro test (repro_48013_image_shrink_brick.py) exercises the real helper with real Pillow at the exact dimensions from the issue and asserts the precondition (under byte budget, over pixel cap) — this is a genuine behavior test, not a mock.

🟡 Minor

  • _decode_pixels is now called up to three times per oversized image on the byte-path (pre-check via needs_shrink, then post-resize verify). For a 4 MB base64 payload that's up to three full base64-decode + Image.open passes. Not hot enough to matter for a compression-recovery path, but a one-line note or caching dims would avoid the re-decode.

🔴 Staleness — likely needs a rebase before merge

Merge-base is 2026-06-19; main is 2026-08-10. conversation_compression.py has grown substantially since (the target function sits at ~L666 on this branch vs ~L3684 on main). git merge-tree origin/main <pr> reports content conflicts in both agent/conversation_compression.py and tests/run_agent/test_image_shrink_recovery.py. The fix itself is still correct and worth landing, but it won't apply cleanly — please rebase onto current main and re-confirm _resize_image_for_vision's 64px short-side floor behavior (referenced in the new comments) still holds on the current resizer.

@alt-glitch alt-glitch removed the duplicate This issue or pull request already exists label Aug 10, 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/anthropic Anthropic native Messages API 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.

Image-dimension 400 bricks sessions: shrink recovery discards a pixel-correct downscale when re-encoded PNG bytes grow (survives #45979)

2 participants