Skip to content

fix(core): bound model image payload by bytes, not only by pixels - #10091

Open
MostimaBridges wants to merge 2 commits into
AstrBotDevs:masterfrom
MostimaBridges:fix/model-image-byte-budget
Open

MostimaBridges wants to merge 2 commits into
AstrBotDevs:masterfrom
MostimaBridges:fix/model-image-byte-budget

Conversation

@MostimaBridges

@MostimaBridges MostimaBridges commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Fixes #10089.

A compliant still image (PNG/JPEG, upright, within image_compress_options.max_size) was passed through byte-exact regardless of its file size. The pixel cap bounds dimensions, not compressed size: a noisy 1280×1280 PNG is already 6.26 MiB, and CUA sandbox sessions lift the edge cap to 1e6 px, which makes the passthrough effectively unconditional. A single prepared still could therefore carry tens of megabytes into the request body. Providers reject the payload with 413 before the model ever sees the image, and the failure only surfaces as All chat models failed: APIStatusError: 413, which hides the actual cause.

Modifications / 改动点

  • astrbot/core/utils/media_utils.py: add MODEL_IMAGE_MAX_BYTES (1 MiB) as a per-still byte budget, and require the source to be within it before _convert_image_bytes_sync reuses a still unchanged. Oversized stills are re-encoded and retried within a bounded attempt budget (MODEL_IMAGE_SHRINK_ATTEMPTS), because no single lever is enough on its own: quality derating cannot shrink a payload that the embedded ICC profile alone keeps large, and neither lever shrinks a canvas whose lowest-quality output still exceeds the budget. The first retry therefore drops the embedded metadata, later retries derate the JPEG quality proportionally, and the canvas is halved only once the quality floor is reached. _encode_image_frame_bytes gained keep_metadata for this, prepare_model_image gained max_bytes (defaults to the budget), and the derived-cache version was bumped so stale entries are not reused.
  • .../agent_sub_stages/image_input.py and internal.py: thread max_bytes through prepare_request_images, following the existing montage_max_size pattern, on both call sites so images added or replaced by an OnLLMRequestEvent hook are covered too. CUA pixel sessions pass None, keeping their compliant stills byte-exact, because that path deliberately trades payload size for pixel fidelity and already warns about oversized images. The CUA resize lift and its warning are unchanged.
  • docs/{zh,en}/providers/image-formats.md: document the byte budget and the CUA exemption.
  • tests/test_model_image_preparation.py: cover that an oversized compliant still is re-encoded within the budget, that oversized metadata cannot keep a still over budget, and that max_bytes=None preserves byte-exact passthrough.

Behavior change to be aware of: compliant stills above 1 MiB are now re-encoded instead of passed through byte-exact. Pixel dimensions are preserved whenever quality alone fits the budget; they are scaled down only in the last resort where the alternative is a request the provider rejects. Oversized embedded metadata is dropped. The source file is never modified, and stills at or below 1 MiB are untouched.

Out of scope for this PR (tracked in the issue): history images are still not re-prepared, and a 413 still walks every fallback provider before failing.

  • This is NOT a breaking change. / 这不是一个破坏性变更。

Screenshots or Test Results / 运行截图或测试结果

The reported shape — noisy 1280×1280 RGBA PNG, default max_size=1280:

source bytes      : 6564225
default budget    : 675110 | passthrough = False   # pixel dimensions preserved
budget disabled   : 6564225 | passthrough = True   # CUA path unchanged

Oversized metadata — a still carrying a 2 MiB ICC profile, where quality derating alone never converges:

ICC source       : 2098739
after budget     : 423 | <= 1MiB = True

Checks, with ruff 0.15.22 (the version pinned in pyproject.toml):

$ ruff format --check .
505 files already formatted
$ ruff check .
All checks passed!

$ pytest tests/test_model_image_preparation.py tests/test_media_utils.py tests/test_process_stage_images.py -q
238 passed

New tests:

$ pytest tests/test_model_image_preparation.py -k "byte_budget or metadata" -v
tests/test_model_image_preparation.py::test_compliant_still_over_byte_budget_is_reencoded PASSED
tests/test_model_image_preparation.py::test_oversized_metadata_does_not_keep_still_over_budget PASSED
tests/test_model_image_preparation.py::test_disabled_byte_budget_keeps_oversized_still_byte_exact PASSED

Checklist / 检查清单

  • 😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
    / 如果 PR 中有新加入的功能,已经通过 Issue / 邮件等方式和作者讨论过。
  • 👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
    / 我的更改经过了良好的测试,并已在上方提供了“验证步骤”和“运行截图”
  • 🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in requirements.txt and pyproject.toml.
    / 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到 requirements.txtpyproject.toml 文件相应位置。
  • 😮 My changes do not introduce malicious code.
    / 我的更改没有引入恶意代码。

The still-image passthrough in _convert_image_bytes_sync only checked format, EXIF orientation and the longest edge, so a compliant PNG/JPEG was sent byte-exact no matter how large it was. A noisy 1280x1280 PNG already exceeds 6 MiB, and CUA sessions lift the edge cap entirely. Providers then reject the whole request with 413 before the model sees the image, surfacing as an opaque All chat models failed error.

Add MODEL_IMAGE_MAX_BYTES (1 MiB) as a per-still byte budget that gates the passthrough. Stills over budget keep their pixel dimensions but are re-encoded, derating the JPEG quality once when the result is still oversized. CUA sandbox sessions pass None to keep compliant stills byte-exact, since that path deliberately trades payload size for pixel fidelity and already warns about oversized images.

Refs AstrBotDevs#10089

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hey - I've found 2 issues

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="astrbot/core/pipeline/process_stage/method/agent_sub_stages/internal.py" line_range="270" />
<code_context>
                         max_size = 1_000_000
+                    # CUA stills stay byte-exact to avoid JPEG color shifts, so the
+                    # byte budget applies only elsewhere; that path warns instead.
+                    max_bytes = None if cua_pixel_mode else MODEL_IMAGE_MAX_BYTES
                     quality = (
                         options.get("quality") if isinstance(options, dict) else None
</code_context>
<issue_to_address>
**issue (broader_impact):** In CUA mode, the second `prepare_request_images` call does not receive the locally computed `max_bytes=None`, so images added or replaced by the request hook after the first preparation pass use the default 1 MiB budget and are re-encoded instead of remaining byte-exact.

**Triggers:** When an `OnLLMRequestEvent` hook adds or replaces an image after the first preparation pass in a CUA session.

**Suggested fix:** Pass `max_bytes=max_bytes` to the second `prepare_request_images` call as well.
</issue_to_address>

### Comment 2
<location path="astrbot/core/utils/media_utils.py" line_range="1273-1280" />
<code_context>
         if cached is not None:
             return cached
         encoded = _encode_image_frame_bytes(image, max_size=max_size, quality=quality)
+        if max_bytes is not None and len(encoded) > max_bytes and quality > 1:
+            # Re-encoding keeps the geometry but not necessarily the payload, so
+            # derate once instead of sending a request the provider rejects.
+            derated = max(1, quality * max_bytes // len(encoded))
+            if derated < quality:
+                encoded = _encode_image_frame_bytes(
+                    image, max_size=max_size, quality=derated
+                )
     _publish_image_cache_atomic(output_path, encoded)
     return encoded
</code_context>
<issue_to_address>
**issue (bug_risk):** The one-time quality derating does not guarantee that the encoded result is at or below `max_bytes`; a compliant image with a large preserved ICC profile can remain over 1 MiB because changing JPEG quality does not reduce the embedded profile, so the provider still receives an oversized payload.

**Triggers:** When the source image contains metadata, especially an ICC profile, whose size alone exceeds the byte budget.

**Suggested fix:** Verify the derated result size and either remove or appropriately handle oversized metadata, or continue reducing/re-encoding until the byte budget is met.
</issue_to_address>

Sourcery assessment

Approval pending. 2 findings to address first.

Blocking findings: astrbot/core/pipeline/process_stage/method/agent_sub_stages/internal.py:270, astrbot/core/utils/media_utils.py:1280


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

Comment thread astrbot/core/utils/media_utils.py Outdated
… every pass

Review follow-ups for the byte budget.

Pass max_bytes to the second prepare_request_images call as well. Images added or replaced by an OnLLMRequestEvent hook were re-encoded with the default budget, so CUA sessions lost their byte-exact passthrough there.

Keep shrinking until the payload actually fits. A single quality step cannot bound a payload that the embedded ICC profile alone keeps large, because the profile survives every quality setting. Retries now drop the metadata first, then derate the quality, and shrink the canvas only once the quality floor is reached.

Refs AstrBotDevs#10089
@MostimaBridges

Copy link
Copy Markdown
Contributor Author

CI note: the macOS pytest failure is an unrelated timing flake

Run pytest suite (macos-latest) failed on tests/unit/test_event_loop_diagnostics.py::test_event_loop_watchdog_survives_dump_failure. The other 22 check runs passed, including the ubuntu and windows pytest suites running the same commit.

That test touches nothing this PR changes (astrbot/core/utils/event_loop_diagnostics.py is untouched), and it is timing-based by construction: it blocks the event loop with time.sleep(0.06) and expects a watchdog configured with timeout=0.02 / interval=0.005 to fire inside that window, which depends on runner scheduling.

Evidence gathered locally on the same commit:

$ pytest tests/unit/test_event_loop_diagnostics.py -q      # five consecutive runs
5 passed in 2.32s
5 passed in 3.22s
5 passed in 4.88s
5 passed in 3.29s
5 passed in 3.55s

$ pytest tests/test_model_image_preparation.py tests/unit/test_event_loop_diagnostics.py -q
65 passed in 7.03s      # same process as the new payload-heavy tests

The spread from 2.3s to 4.9s for the same five tests shows how load-sensitive that file is, and running it in the same process as the new image tests does not reproduce the failure.

I left the test untouched to keep this PR focused. A re-run of the failed job should be enough; if it recurs it is worth a separate fix, for example asserting on attempts with a longer deadline instead of a fixed time.sleep window.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sourcery assessment

Approved.

@kilisamemarisaaa kilisamemarisaaa left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reviewed latest head 940f416 after the Sourcery findings. The second post-hook prepare_request_images call now receives the same max_bytes value, preserving the CUA max_bytes=None exemption for images added or replaced by hooks. Byte fitting now retries within a bounded loop, drops embedded ICC metadata, derates JPEG quality, and only then scales the canvas, with a regression test using a 2 MiB ICC profile plus oversized noisy PNG coverage. The cache key/version includes byte-budget semantics, and the full CI matrix (including Windows/macOS/Linux, CodeQL, dashboard, smoke) is green. I found no remaining correctness blocker.

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.

[Bug] 图片预处理缺少字节体积兜底:合规图片原样直通 + 历史图片不重新处理,导致 413 request_too_large

2 participants