feat: resize and compress images before base64 encoding#210
feat: resize and compress images before base64 encoding#210thepagent merged 4 commits intoopenabdev:mainfrom
Conversation
Follow OpenClaw's approach to prevent large image payloads from exceeding JSON-RPC transport limits (Internal Error -32603). Changes: - Add image crate dependency (jpeg, png, gif, webp) - Resize images so longest side <= 1200px (Lanczos3) - Re-encode as JPEG at quality 75 (~200-400KB after base64) - GIFs pass through unchanged to preserve animation - Fallback to original bytes if resize fails Fixes openabdev#209
Tests cover: - Large image resized to max 1200px - Small image keeps original dimensions - Landscape/portrait aspect ratio preserved - Compressed output smaller than original - GIF passes through unchanged - Invalid data returns error
🔴 Critical: resize() distorts aspect ratioFile: let img = img.resize(
IMAGE_MAX_DIMENSION_PX, // 1200
IMAGE_MAX_DIMENSION_PX, // ← forces image to 1200×1200 regardless of aspect ratio
image::imageops::FilterType::Lanczos3,
);
Fixlet (w, h) = (img.width(), img.height());
let img = if w > IMAGE_MAX_DIMENSION_PX || h > IMAGE_MAX_DIMENSION_PX {
let ratio = f64::from(IMAGE_MAX_DIMENSION_PX) / f64::max(w, h);
let new_w = (f64::from(w) * ratio) as u32;
let new_h = (f64::from(h) * ratio) as u32;
img.resize(new_w, new_h, image::imageops::FilterType::Lanczos3)
} else {
img
};The test 🟡 Minor: compression fallback does not re-check file sizeWhen 🟢 InfoJPEG quality is 75, while OpenClaw uses progressive 85→35. If quality complaints come in later, this difference may be the cause. |
chaodu-agent
left a comment
There was a problem hiding this comment.
Review Feedback
1. Post-download size check 被移除了
原本有 defense-in-depth 的下載後二次檢查 (bytes.len() > MAX_SIZE),現在只剩 pre-download 的 attachment.size 檢查。attachment.size 是 Discord API 回報的值,理論上可信,但如果 CDN 回傳的實際 bytes 不一致(redirect、proxy 等),就少了一層保護。建議保留 post-download 的 bytes.len() 檢查。
2. Error log structured fields
簡化後的 error log 像 error!("download failed {url}: {e}") 少了 structured fields。原本用的格式比較適合 structured logging / grep:
// Before
error!("failed to download image {}: {}", url, e);
// Suggested
error!(url = %url, error = %e, "download failed");建議統一用 tracing 的 structured field 風格,跟 codebase 其他地方一致。
Address review feedback from @the3mi: - 🔴 Fix resize() to calculate proportional dimensions instead of forcing 1200x1200 (was distorting images) - 🟡 Add 1MB size check on fallback path when resize fails - Fix portrait/landscape test assertions to match correct aspect ratios
|
Thanks @the3mi for the thorough review! All three points addressed in the latest commit (0e49dc4): 🔴 Fixed: aspect ratio preservedlet ratio = f64::from(IMAGE_MAX_DIMENSION_PX) / f64::from(max_side);
let new_w = (f64::from(w) * ratio) as u32;
let new_h = (f64::from(h) * ratio) as u32;
img.resize(new_w, new_h, ...)4000×2000 → 1200×600 ✅, 2000×4000 → 600×1200 ✅ 🟡 Fixed: fallback size checkWhen 🟢 Noted: JPEG quality 75 vs OpenClaw's progressive 85→35Good callout. We can revisit if quality complaints come in — easy to add progressive reduction later. Tests updated to match correct aspect ratio assertions. |
chaodu-agent
left a comment
There was a problem hiding this comment.
🟡 Minor (non-blocking)
1. Post-download size check 仍未恢復
下載後的 bytes.len() 二次檢查還是沒加回來。目前只有 pre-download 的 attachment.size 檢查。雖然 resize 後通常會變小,但 defense-in-depth 多一層不嫌多:
let bytes = match response.bytes().await {
Ok(b) => b,
Err(e) => { error!(url = %url, error = %e, "read failed"); return None; }
};
if bytes.len() as u64 > MAX_SIZE {
error!(filename = %attachment.filename, size = bytes.len(), "downloaded image exceeds limit");
return None;
}2. Error log 建議改用 structured fields
目前 inline format 如 error!("download failed {url}: {e}") 跟 codebase 其他地方的 tracing structured field 風格不一致,建議統一:
// 現在
error!("download failed {url}: {e}");
// 建議
error!(url = %url, error = %e, "download failed");兩點都是 minor,不 block merge 👍
Address minor review feedback: - Restore defense-in-depth bytes.len() check after download - Use tracing structured fields (url = %url, error = %e) for consistency with codebase style
|
Minor feedback addressed in 1321060:
|
|
@the3mi All your review feedback has been addressed across 3 commits. Mind taking another look and approving when you get a chance? 🦞🙏
|
Codify the review style from PR openabdev#210 as team standard: - 🔴🟡🟢 severity levels - Each comment: what/where/why/fix - Self-review checklist for authors - Review etiquette guidelines Credit to @the3mi for the review format.
* feat: resize and compress images before base64 encoding Follow OpenClaw's approach to prevent large image payloads from exceeding JSON-RPC transport limits (Internal Error -32603). Changes: - Add image crate dependency (jpeg, png, gif, webp) - Resize images so longest side <= 1200px (Lanczos3) - Re-encode as JPEG at quality 75 (~200-400KB after base64) - GIFs pass through unchanged to preserve animation - Fallback to original bytes if resize fails Fixes openabdev#209 * test: add unit tests for image resize and compression Tests cover: - Large image resized to max 1200px - Small image keeps original dimensions - Landscape/portrait aspect ratio preserved - Compressed output smaller than original - GIF passes through unchanged - Invalid data returns error * fix: preserve aspect ratio on resize + add fallback size check Address review feedback from @the3mi: - 🔴 Fix resize() to calculate proportional dimensions instead of forcing 1200x1200 (was distorting images) - 🟡 Add 1MB size check on fallback path when resize fails - Fix portrait/landscape test assertions to match correct aspect ratios * fix: restore post-download size check + use structured logging Address minor review feedback: - Restore defense-in-depth bytes.len() check after download - Use tracing structured fields (url = %url, error = %e) for consistency with codebase style --------- Co-authored-by: chaodu-agent <chaodu-agent@users.noreply.github.com>
Problem
Large Discord image attachments encoded as base64 exceed JSON-RPC transport limits, causing
Internal Error (code: -32603)when sent via ACP to coding CLI agents.Survey: How other ACP agents handle images
--imageflag reads local file; ACP uses base64Key findings:
Decision
Follow OpenClaw's approach: resize + compress images before base64 encoding on the openab side.
DEFAULT_IMAGE_MAX_DIMENSION_PX)Changes
Cargo.toml: addimagecrate (jpeg, png, gif, webp features)src/discord.rs: replace raw base64 encoding with resize → compress → encode pipelineReferences
image-sanitization.ts,image-ops.tsFixes #209