Skip to content

feat(read): port TS image-handling pipeline to Python (Tier C parity) - #154

Merged
ericleepi314 merged 1 commit into
mainfrom
feat/image-handling-parity
May 16, 2026
Merged

feat(read): port TS image-handling pipeline to Python (Tier C parity)#154
ericleepi314 merged 1 commit into
mainfrom
feat/image-handling-parity

Conversation

@ericleepi314

Copy link
Copy Markdown
Collaborator

Summary

Full parity with the TypeScript reference at typescript/src/utils/imageResizer.ts + the FileReadTool image flow. Pillow replaces sharp.

Adds the image processing pipeline that was missing: oversized images are now downscaled instead of rejected, file format is sniffed from magic bytes (not the extension), image dimensions are surfaced to the model for coordinate-mapping prompts, PDF pages can be extracted as images, and shell commands that print data-URI images (matplotlib, mermaid) get rendered natively.

See my-docs/image-handling-gap-analysis.md and my-docs/image-handling-refactoring-plan.md for the gap analysis and approved plan this PR implements.

What changes for the user

  • Oversized images (>3.75 MB) are now downscaled to fit the API's 5 MB base64 limit instead of being rejected with a hard error. Real-world phone screenshots (often 4-8 MB) work without manual resizing.
  • Magic-byte format detection: a foo.png containing JPEG bytes correctly gets media_type=image/jpeg so the API doesn't reject the mislabeled image.
  • Image dimensions metadata: Read emits a supplemental isMeta user message with [Image: source: /path, original WxH, displayed at WxH. Multiply coordinates by X to map to original image.] for coordinate-mapping workflows.
  • PDF page extraction: Read(file_path=foo.pdf, pages='1-5') extracts pages via pdftoppm (poppler-utils) and routes them through the same image pipeline.
  • Bash data-URI images: python -c 'matplotlib stuff; print(base64...)' surfaces as an image content block instead of garbage text.
  • Pre-API image validation rejects any image >5 MB base64 with an actionable error rather than letting the API round-trip fail.

Files

New modules (lazy Pillow import — module load is cheap)

  • src/utils/image_processor.py — magic-byte detection, bounded read_file_bytes, maybe_resize_image (3.75 MB / 1568px envelope, JPEG fallback at q={80,60,40,20}), compress_image_to_byte_budget (progressive scale × quality, 800×800 PNG palette, 400×400 q=20 ultra-fallback), compress_image_to_token_budget, create_image_metadata_text.
  • src/utils/image_validation.pyvalidate_images_for_api (walks messages, rejects >5 MB base64).
  • src/utils/pdf_extraction.pyextract_pdf_pages shells out to pdftoppm, 100 MB input cap, empty-file guard, install-hint error when poppler is missing.
  • src/tool_system/tools/bash/image_output.pyis_image_output / parse_data_uri / build_image_tool_result with 25 MB pre-decode cap.

Modified

  • src/tool_system/tools/read.py — image branch rewritten: bounded read → magic-byte sniff → resize → token-budget compress → returns type='image' with dimensions + supplemental metadata message. PDF pages= path wired with try/finally tempdir cleanup. Dead MAX_IMAGE_SIZE_BYTES removed.
  • src/query/query.py_dispatch_single_tool returns tuple[UserMessage, list[UserMessage]] so result.new_messages reach the model. Critical: callers concatenate all primaries first, then all extras, so multi-tool batches don't break ensure_tool_result_pairing.
  • src/services/api/claude.py — wires validate_images_for_api into call_model.
  • src/services/analytics/events.py — new EventType.IMAGE_PROCESSING event type with subtype in data.
  • src/tool_system/tools/bash/bash_tool.py — detects data-URI output, routes through build_image_tool_result.
  • pyproject.toml + uv.lock — adds Pillow>=10.0.

Dependencies

  • Python: Pillow>=10.0 (pure-Python wheels on all platforms; no native compilation needed).
  • System (optional, runtime-detected): pdftoppm from poppler-utils. PDF extraction without it gives an install-hint error rather than failing silently.

Test plan

  • 60+ new unit tests across tests/test_image_processor.py, test_image_validation.py, test_bash_image_output.py, test_pdf_extraction.py. 1 PDF test skipped when poppler isn't installed.
  • 8 new e2e tests in tests/parity/test_e2e_file_read.py including the critical regression test test_multi_tool_batch_preserves_tool_result_pairing that drives _run_tools_partitioned end-to-end through normalize_messages_for_api with two image Reads.
  • Updated tests/test_tool_result_budget.py and tests/test_esc_reject_message_dispatch.py for the new _dispatch_single_tool tuple return shape.
  • Wider test run: 524 pass, 4 pre-existing unrelated failures (MCP module missing, two *_outside_workspace_blocked from bypassPermissions default).
  • Critic review loop completed with APPROVE after two rounds. Round 2 caught a real regression I introduced: my initial flatten broke multi-tool batches by interleaving primaries and extras → ensure_tool_result_pairing orphaned the second tool's result. Fix and lock-in test are in this PR.

Out of scope (intentional)

  • Wiring validate_images_for_api into the non-Anthropic provider paths (TODO note in claude.py). They don't currently emit image content blocks.
  • Native image-processor-napi equivalent (TS's preferred backend). Pillow is sufficient.
  • Image content-hash dedup. TS doesn't dedupe images either.

🤖 Generated with Claude Code

Brings the Read tool's image handling to full parity with the TypeScript
reference at typescript/src/utils/imageResizer.ts + FileReadTool.ts image
flow. Pillow replaces sharp.

What changes for the user:
- Oversized images (>3.75 MB) are now downscaled to fit the API's 5 MB
  base64 limit instead of being rejected outright.
- File extension is no longer trusted blindly — magic-byte detection
  sniffs the actual format (PNG/JPEG/GIF/WebP), so a misnamed file.png
  containing JPEG bytes gets media_type=image/jpeg correctly.
- Read emits a supplemental isMeta UserMessage carrying image dimensions
  ("Multiply coordinates by X to map to original image") for
  coordinate-mapping prompts.
- The PDF Read with pages="N-M" parameter now extracts page images via
  poppler's pdftoppm and routes them through the same image pipeline.
- Bash commands that print data:image/...;base64,... (matplotlib,
  mermaid, etc.) surface as image content blocks in the tool_result
  instead of garbage text.
- Images >5 MB base64 are rejected pre-API with an actionable error
  rather than letting the API round-trip fail.

What changes architecturally:
- New module src/utils/image_processor.py: magic-byte detection,
  bounded readFileBytes, maybe_resize_image (3.75 MB / 1568px envelope,
  JPEG fallback at q={80,60,40,20}), compress_image_to_byte_budget
  (progressive scale × quality, 800×800 PNG palette, 400×400 q=20
  ultra-fallback), compress_image_to_token_budget,
  create_image_metadata_text.
- New module src/utils/image_validation.py: validate_images_for_api
  walks messages, rejects any base64 image >5 MB before the API call.
  Wired into claude.py:call_model.
- New module src/utils/pdf_extraction.py: extract_pdf_pages shells out
  to pdftoppm, 100 MB input cap, empty-file guard, install-hint error
  when poppler-utils is missing.
- New module src/tool_system/tools/bash/image_output.py:
  is_image_output / parse_data_uri / build_image_tool_result for shell
  image output, with 25 MB pre-decode cap to prevent OOM from hostile
  shells.
- src/tool_system/tools/read.py image branch rewritten: bounded read
  -> magic-byte sniff -> resize -> token-budget compress -> returns
  type='image' with dimensions field + supplemental metadata message.
  PDF pages= path wired with try/finally tempdir cleanup. Old
  MAX_IMAGE_SIZE_BYTES rejection removed.
- src/query/query.py _dispatch_single_tool now returns
  tuple[UserMessage, list[UserMessage]] (primary, extras) so
  result.new_messages reach the model. Callers concatenate all
  primaries first, then all extras, so multi-tool batches don't break
  ensure_tool_result_pairing (a regression the critic caught and the
  primaries-first ordering fixes).
- New EventType.IMAGE_PROCESSING analytics event with subtype in data.

Dependency: Pillow>=10.0 (pure-Python wheels on all platforms).
System dependency for PDF page extraction (optional, runtime-detected):
pdftoppm from poppler-utils.

Tests: 60+ new tests across tests/test_image_processor.py,
test_image_validation.py, test_bash_image_output.py,
test_pdf_extraction.py, plus 8 added to tests/parity/test_e2e_file_read.py.
The critical regression test test_multi_tool_batch_preserves_tool_result_pairing
drives _run_tools_partitioned -> normalize_messages_for_api end-to-end to
lock in the primaries-first ordering. Wider parity suite: 442 pass,
4 pre-existing unrelated failures.

Critic review loop completed with APPROVE after two rounds.

See my-docs/image-handling-gap-analysis.md and
my-docs/image-handling-refactoring-plan.md for the full analysis.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@ericleepi314
ericleepi314 merged commit e12f86f into main May 16, 2026
singlaamitesh pushed a commit to singlaamitesh/clawcodex that referenced this pull request Jul 7, 2026
…ndling-parity

feat(read): port TS image-handling pipeline to Python (Tier C parity)
singlaamitesh pushed a commit to singlaamitesh/clawcodex that referenced this pull request Jul 7, 2026
…mage tool_result shape

User-reported correctness bug: the Python build hallucinated wildly when
asked about an @-mentioned image ("a Google search results page") while
TS correctly described it ("an aerial view of a research ship"). Root
cause was two independent bugs both contributing to the failure.

Bug A: ``expand_at_mentions`` opened every @-mentioned file with
``open(path, "r", encoding="utf-8", errors="replace")`` regardless of
extension. For a PNG that produced mojibake (utf-8 replacement chars
over binary bytes) which ``format_at_mention_attachments`` wrapped in a
``<system-reminder>Contents of foo.png:`` block and prepended to the
user message. The model latched onto ASCII fragments (XMP "Screenshot"
metadata, type tags) and hallucinated. Fix: detect image extensions
(png/jpg/jpeg/gif/webp, matching the Read tool's IMAGE_EXTENSIONS)
BEFORE the text-mode open. For image files, run the same image
pipeline the Read tool now uses (post PR agentforce314#154): bounded read ->
magic-byte format sniff -> resize-to-envelope ->
``compress_image_to_token_budget`` fallback when still over the 5 MB
base64 API ceiling. Return a ``kind="image"`` attachment carrying
``base64`` + ``media_type``. New helper ``build_image_content_blocks``
materialises ``ImageBlock`` instances and the REPL composes a mixed
``[TextBlock, ImageBlock, ...]`` user message so the API receives a
real multimodal payload matching the TS auto-Read-on-@image behaviour.

Bug B: ``_dispatch_single_tool`` ran ``json.dumps`` on any non-string
tool_result content, turning Read's properly-shaped image list
``[{"type": "image", "source": {...}}]`` into a text JSON blob. The
Anthropic API then received an image tool_result whose content was
text JSON; the model literally could not see the image and would
hallucinate. PR agentforce314#154's regression test only checked for the synthetic-
error placeholder so this slipped through. Fix: preserve list shape
end-to-end. ``ToolResultBlock.content`` already accepts ``str |
list[Any]``, ``maybe_persist_large_tool_result`` already short-
circuits image lists via ``_has_image_block``, and
``content_block_to_dict`` already serializes per-element -- the
dispatcher just needed to stop coercing.

Collateral fix: PR agentforce314#154's Read tool image content also failed silently
on OpenAI-compatible providers (GLM, Minimax, DeepSeek, OpenRouter)
because ``_convert_anthropic_messages_to_openai`` passed Anthropic
image blocks through unchanged and OpenAI rejects them. New helper
``_anthropic_image_block_to_openai`` translates the base64-source
shape to OpenAI's ``image_url`` data-URI shape. For tool_result image
content the converter now emits ``role=tool`` (text body, with a
placeholder when the original was image-only) followed by a synthetic
``role=user`` message carrying the image_url blocks, since OpenAI's
``role=tool`` doesn't accept multimodal content. Empty-data guard
returns ``None`` rather than producing ``data:image/png;base64,``.

Signature widening to support the multi-block flow:
- ``Conversation.add_user_message(content: str | list[ContentBlock])``
- ``QueryEngine.submit_message(prompt: str | list[ContentBlock])``
Both bodies already supported list content via ``_normalize_message_
content`` / ``MessageContent``; only the type annotations changed, so
existing string-callers (TUI, headless) are unaffected.

REPL UX: image @-mentions skip the direct-stream short-circuit (it
can only carry plain text) and print ``Read image <path>`` instead of
``Listed file <path>`` so the user sees the image was attached.

Test coverage (29 new tests):
- tests/test_at_mention_images.py (17): single + multi-image + mixed
  text+image @-mentions, magic-byte detection beats extension, empty/
  undecodable images dropped silently, oversize-image compression
  brings base64 under API limit (real 4000x4000 random-noise PNG),
  doubly-oversize image dropped (monkeypatched), end-to-end through
  ``normalize_messages_for_api``.
- tests/test_openai_compat_image_translation.py (13): user message
  text+image, multi-image, JPEG/PNG/missing-media-type defaults,
  empty-data guard, tool_result image-only + image+text + text-only-
  no-regression.
- tests/parity/test_e2e_file_read.py (+2): Bug B dispatcher list-
  preservation; aggregate-budget pressure path (image still survives
  when ``tool_result_chars_so_far = MAX - 1``, counter NOT bumped by
  image bytes).

Wider suite: 4984 pass, 0 new failures, 9 pre-existing failures (mcp/
zhipuai import issues + workspace-path tests) unchanged.

Critic review loop: APPROVE after three rounds.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
singlaamitesh pushed a commit to singlaamitesh/clawcodex that referenced this pull request Jul 7, 2026
Addresses the four pre-existing gaps the PR agentforce314#155 audit surfaced:

1. validate_images_for_api ran only on the Anthropic-direct call_model
   path; all provider.chat_stream_response paths skipped it. Promoted
   into BaseProvider._prepare_messages so every provider validates
   client-side. query._call_model_sync catches ImageSizeError and
   surfaces a media_size assistant error (matches the existing
   server-side classification). agent_loop._call_provider_for_turn
   re-raises ImageSizeError instead of falling back to chat() (which
   would re-trigger the same validation). Validator also now walks
   images nested inside tool_result.content — post-PR agentforce314#154 the Read
   tool returns image blocks there and the old walker missed them.

2. OpenAI tool->user split symmetric correlation. Tool message body
   now always carries [multimodal content for tool_use_id=X ...] when
   the synthetic role=user message follows; the user message leads
   with a [content for tool_use_id=X] text block so the link is
   visible from both directions. Empty content sentinel ([empty tool
   result]) guards the "non-empty content required" invariant for
   content=[] cases.

3. @file.pdf and other binary @-mentions no longer hit the text-mode
   open() that produced mojibake-in-system-reminder. Known binary
   extensions (pdf/zip/docx/...) plus a NUL-byte content sniff route
   to a new "binary" attachment kind with a Read-tool hint (PDF
   specifically mentions the pages parameter). BOM-aware decoder
   (_read_text_with_encoding) handles Windows-emitted UTF-16/UTF-32
   files via the BOM-aware codecs (utf-16, utf-32, utf-8-sig) so the
   BOM is consumed rather than preserved as a literal U+FEFF char.
   Post-decode garbled-text detector (NUL zero-tolerance + 3% U+FFFD
   threshold) falls back to a binary attachment when an adversarial
   spoofed-BOM blob would otherwise leak.

4. _anthropic_image_block_to_openai now has a sibling
   _anthropic_document_block_to_openai that translates
   Anthropic DocumentBlock (PDF) to OpenAI's {type:"file"} shape.
   Defensive: no production path currently produces DocumentBlock
   for OpenAI-compat, but future ones won't silently pass through.

Test plan:
- 94 new tests across test_at_mention_binary_files.py,
  test_base_provider_image_validation.py,
  test_agent_loop_image_size_propagation.py,
  test_openai_compat_document_translation.py, plus extensions to
  test_image_validation.py, test_openai_compat_image_translation.py,
  test_query_loop.py.
- Wider suite: 5015 pass / 10 pre-existing failures unchanged.
- Critic review loop: APPROVE after three rounds (round-2 caught a
  UTF-16 BOM regression I introduced; round-3 fixed it and added the
  post-decode garble detector).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
singlaamitesh pushed a commit to singlaamitesh/clawcodex that referenced this pull request Jul 7, 2026
- Codebase stats: 894 files / 177,428 lines (up from 167,034 on
  2026-05-14; ~+10.4k lines in two days).
- New news entries:
  * 2026-05-16 Image-handling parity (agentforce314#149/agentforce314#154/agentforce314#155/agentforce314#156) — Read tool
    TS image pipeline; @image.png mention fix; image tool_result list
    shape preserved; OpenAI-compat image/document block translation;
    validate_images_for_api promoted into BaseProvider._prepare_messages;
    BOM-aware text decoding.
  * 2026-05-16 Subagent + Bash reliability — custom subagent discovery
    from .claude/agents/ (agentforce314#151); Bash timeout vs ESC-abort distinction
    (agentforce314#152); async-subagent AbortController isolation tests (agentforce314#153);
    REJECT_MESSAGE on cancelled production path (agentforce314#150).
  * 2026-05-15 to 2026-05-16 ESC cancellation hardening across providers
    (agentforce314#144agentforce314#148) — mid-stream cancel under ~50ms for every provider;
    shared StreamAbortGuard; LiteLLM worker-thread iteration fix.
- Core Systems table:
  * Multi-Provider description extends to call out Anthropic→OpenAI
    image/document block translation.
  * New "Cancellation / Abort" row (✅) capturing the recent sweep.
  * New "Image Handling" row (✅) capturing Tier C parity.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
singlaamitesh pushed a commit to singlaamitesh/clawcodex that referenced this pull request Jul 7, 2026
Bring it in line with the length of the surrounding entries: four
facts joined by semicolons, no inline rationale or low-level
implementation details. The longer write-up lives in the PR
descriptions for agentforce314#149/agentforce314#154/agentforce314#155/agentforce314#156.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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