Skip to content

fix(images): normalize oversized model payloads independently of byte size - #920

Open
breedx wants to merge 3 commits into
mpfaffenberger:mainfrom
breedx:fix/upstream-image-dimensions
Open

fix(images): normalize oversized model payloads independently of byte size#920
breedx wants to merge 3 commits into
mpfaffenberger:mainfrom
breedx:fix/upstream-image-dimensions

Conversation

@breedx

@breedx breedx commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Images can be small in bytes but still exceed a model's dimension limit. Normalize dimensions across attachment, clipboard and browser-screenshot payloads so compressible screenshots no longer bypass resizing, while keeping saved screenshot files at full resolution.

Problem

The shared image helper gates resizing on encoded byte size. A large, mostly blank screenshot can therefore pass through unchanged. Browser screenshots also bypass that helper, and the file loader uses a different edge limit.

Change

  • Apply a shared 2000px maximum edge independently of encoded byte size.
  • Preserve aspect ratio, keep dimensions positive, and never upscale a short edge to satisfy the old 100px floor.
  • Normalize browser model payloads after preserving the original screenshot file.
  • Process small-byte Linux clipboard captures too; retain re-encoding when the original encoding exceeds the byte budget.
  • Align file loading with the shared cap and convert resized CMYK images to a PNG-compatible mode.
  • Leave in-bounds original payload bytes unchanged when no resize is needed.

Core scope and overlap

This corrects existing image producers and their shared utility, rather than adding a command or parallel plugin policy. It necessarily touches existing command_line/image_utils.py and clipboard.py; given the plugin-first contribution guidance, please flag a preferred supported seam before landing if this belongs elsewhere. The goal is one shared policy, not another frontend-specific workaround.

#911 fixes image-path resolution in the same file-loading module; this patch changes its dimension limit/encoding path, not path lookup. It does not depend on that PR or any MCP lifecycle work.

Validation

Base: 1d25d696; Linux / Python 3.13.13 with the unchanged upstream lock. Tests use actual Pillow-generated images, disposable HOME/XDG, no inherited credentials and blocked socket connect/DNS/bind. Browser capture and clipboard acquisition are mocked; payload processing and file-loader calls are real.

  • New regression file on unpatched base: 8 failed, 3 passed.
  • Focused image/clipboard/attachment/screenshot selection: 169 passed.
  • python -m pytest -q -o addopts= tests/test_model_image_dimensions.py tests/test_image_utils.py tests/command_line tests/test_command_line_attachments.py tests/tools: 1654 passed, 1 skipped, 1 warning.
  • Regressions cover thin images, low-byte oversized images, untouched in-bounds bytes, full-resolution saved captures, actual file-loader output, oversized clipboard encoding and CMYK resizing.
  • Ruff lint, formatting and diff checks pass on changed files.

The skip requires a case-insensitive filesystem. The browser-control unawaited-coroutine warning reproduces on the unpatched base; it was not suppressed.

Compatibility and limits

The conservative 2000px policy reduces detail for images previously allowed above that size. Saved files are not resized, and this does not rewrite images already persisted in conversation history. The existing byte-budget estimate is not a hard encoded-byte guarantee. No universal provider acceptance, live browser/clipboard capture, or live-provider qualification is claimed. No dependency or package-version changes.

The base currently fails the unrelated tilde-completion assertion corrected separately in #916. This PR does not bundle that change or claim full upstream CI is green.

@breedx

breedx commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

This fix remains independently reviewable, but full CI hits the existing quoted-tilde completion assertion already addressed separately in #916. I reproduced that exact failure on unpatched 1d25d696; this PR does not change the completion code or test, so I am keeping the unrelated correction out of this patch.

  • Quality and Windows encoding checks passed.
  • The macOS suite's only failure is tests/test_completions_and_small_modules.py::TestFilePathCompleterMissedLines::test_tilde_prefix_display: the insertion contains quoting around ~/..., so .startswith("~") fails.
  • Reproduction on unpatched base: python -m pytest -q -o addopts= tests/test_completions_and_small_modules.py::TestFilePathCompleterMissedLines::test_tilde_prefix_display1 failed, same assertion.
  • The independently reviewed focused and broader local results remain documented in the PR body. No checks were suppressed, and no green-full-suite claim is made.
  • CI again uses Python 3.14.7 despite the job's 3.13 label. Its deprecation/resource/coroutine warnings were inspected, but not all have independent base controls; local validation used Python 3.13.13.

Once the separate completion correction lands, this topic can be requalified on the updated base without widening its scope.

@breedx

breedx commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

greater than 2000px causes models to fail in upstream inference..

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant