Skip to content

fix(media): forward scanned PDF page images as current-turn images#96706

Closed
wanyongstar wants to merge 2 commits into
openclaw:mainfrom
wanyongstar:fix/issue-96541-forward-scanned-pdf-images
Closed

fix(media): forward scanned PDF page images as current-turn images#96706
wanyongstar wants to merge 2 commits into
openclaw:mainfrom
wanyongstar:fix/issue-96541-forward-scanned-pdf-images

Conversation

@wanyongstar

@wanyongstar wanyongstar commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Chat media-understanding already receives page-image payloads from image-only PDFs, but converted them into placeholder-only file blocks. Scanned PDFs in chat channels therefore did not reach vision-capable agent turns, unlike the sibling OpenResponses path.

Summary

Carry extracted PDF page images as transient current-turn images, consumed once in normal reply and ACP dispatch.

Changes:

  • apply.ts: extractFileBlocks returns ExtractedFileBlocks with images alongside blocks. Extracted page images are stored on ctx.CurrentTurnImages
  • templating.ts: MsgContext gains optional CurrentTurnImages?: ImageContent[]
  • current-turn-images.ts: resolveCurrentTurnImages consumes ctx.CurrentTurnImages and merges them into the resolved image output
  • dispatch-acp.ts: consumes ctx.CurrentTurnImages and forwards as inline ACP attachments

Linked context

Closes #96541

Evidence

  • extractFileBlocks now returns images alongside text blocks
  • Placeholder updated from [PDF content rendered to images; images not forwarded to model] to just [PDF content rendered to images]
  • Images are forwarded via the typed ImageContent path (not Markdown data URIs)
  • Both normal reply (resolveCurrentTurnImages) and ACP dispatch handle the new field
  • Local type checks, lint, and diff whitespace pass on this branch
pnpm vitest run src/media-understanding/apply.test.ts
pnpm vitest run src/auto-reply/reply/current-turn-images.test.ts
pnpm vitest run src/auto-reply/reply/dispatch-acp.test.ts
pnpm vitest run src/auto-reply/reply/get-reply.test.ts
pnpm format:check
pnpm check:changed

Live chat-channel proof will be supplied separately.

Risk checklist

Did user-visible behavior change? (Yes) — PDF page images now reach vision-capable models on chat channels.

Did config, environment, or migration behavior change? (No)

Did security, auth, secrets, network, or tool execution behavior change? (No) — images that were already extracted and forwarded by the API path are now also forwarded on the chat path.

What is the highest-risk area?
Double-consumption of CurrentTurnImages if both reply paths run in sequence.

How is that risk mitigated?
CurrentTurnImages is deleted after consumption (delete ctx.CurrentTurnImages) so each path consumes it at most once.

@wanyongstar
fix(media): forward scanned PDF page images as inline data URIs on ch…
2e2c824 
…at channels

When extractFileBlocks receives a scanned/image-only PDF (no extractable text
but page images present), it previously set a dead-end placeholder and dropped
the images. The /v1/responses API path already forwarded the same images
correctly.

Fix: embed the extracted page images as base64 data URIs in the file context
block, so vision-capable models can read scanned documents on all chat
channels.

Closes openclaw#96541
@clawsweeper

clawsweeper Bot commented Jun 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 25, 2026, 4:26 AM ET / 08:26 UTC.

Summary
The PR carries scanned PDF page images from media-understanding file extraction into MsgContext.CurrentTurnImages and forwards them through normal reply and ACP image paths.

PR surface: Source +52. Total +52 across 4 files.

Reproducibility: yes. at source level: current main can receive PDF-rendered images from extractFileContentFromSource and then emit only a placeholder text block on the chat media-understanding path. A live channel reproduction was not run for this review.

Review metrics: 1 noteworthy metric.

  • Image-order contract: 1 existing contract overwritten. The PR touches the helper that forwards imageOrder into the embedded runner, so mixed inline/offloaded attachment order must be preserved before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #96541
Summary: This PR is one candidate fix for the canonical scanned/image-only PDF chat-channel forwarding bug, with another open candidate still needing proof.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦐 gold shrimp
Result: blocked until real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Preserve existing imageOrder entries when appending extracted PDF images.
  • [P1] Add redacted real chat or ACP proof showing a scanned PDF reaches a vision-capable turn after the fix.

Proof guidance:

  • [P1] Needs real behavior proof before merge: No after-fix real setup proof is present; the contributor should add redacted live output, logs, terminal output, screenshot with diagnostics, recording, or linked artifact and update the PR body for re-review.

Mantis proof suggestion
A native Telegram proof would materially show the user-visible scanned-PDF chat behavior after the ordering fix. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis telegram desktop proof: verify a scanned/image-only PDF sent to a chat reaches a vision-capable agent turn and is not reduced to placeholder text.

Risk before merge

  • [P1] The PR can reorder or misclassify mixed inline/offloaded chat images when scanned-PDF page images are also merged into the current turn.
  • [P1] No after-fix real chat or agent-turn proof is present; the PR body explicitly says live chat-channel proof will be supplied separately.
  • [P1] There is an open sibling candidate for the same issue, so maintainers may want to choose or adopt the best branch rather than letting two similar PRs drift.

Maintainer options:

  1. Fix Ordering Before Merge (recommended)
    Preserve every existing params.imageOrder entry and append inline entries only for extracted PDF page images, with a mixed inline/offloaded regression test.
  2. Adopt The Better Candidate
    Maintainers can compare this PR with fix(media): forward scanned PDF page images #96690 and proceed with whichever branch has the cleaner tested implementation plus real proof.
  3. Accept Source-Only Risk
    Maintainers could accept the media-forwarding change after repairing ordering without live proof, but that would knowingly leave the real chat delivery path unproven.

Next step before merge

  • [P1] Human follow-up is needed because the branch has a concrete code finding plus a mandatory real-proof gap, and a sibling candidate PR should be considered before choosing the landing branch.

Security
Cleared: No concrete security or supply-chain issue found; the diff reuses existing extracted image payloads and changes no dependencies, workflows, secrets, or package resolution.

Review findings

  • [P2] Preserve existing imageOrder entries — src/auto-reply/reply/current-turn-images.ts:113
Review details

Best possible solution:

Preserve the existing imageOrder contract while appending extracted PDF page images, add focused regression coverage, then require redacted real chat or ACP proof before merging or adopting a sibling PR.

Do we have a high-confidence way to reproduce the issue?

Yes, at source level: current main can receive PDF-rendered images from extractFileContentFromSource and then emit only a placeholder text block on the chat media-understanding path. A live channel reproduction was not run for this review.

Is this the best way to solve the issue?

No as submitted. Carrying extracted page images through existing current-turn image plumbing is the right layer, but this branch must preserve existing imageOrder semantics and provide real behavior proof before it is the best fix.

Full review comments:

  • [P2] Preserve existing imageOrder entries — src/auto-reply/reply/current-turn-images.ts:113
    When CurrentTurnImages is present this branch replaces the caller's imageOrder with one inline entry per merged image. chat.send already uses imageOrder to distinguish inline images from offloaded image refs; in a mixed turn with an offloaded image plus a scanned PDF, this loses the offloaded slot and the runner can rebuild the model images in the wrong order. Preserve the existing order and append inline only for the extracted PDF page images.
    Confidence: 0.88

Overall correctness: patch is incorrect
Overall confidence: 0.84

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 770b19f496c1.

Label changes

Label justifications:

  • P2: This is a bounded media-understanding bug fix for scanned PDF attachments across chat paths, with limited blast radius and no emergency signal.
  • merge-risk: 🚨 message-delivery: Overwriting imageOrder can reorder or misclassify chat attachment images in mixed media turns.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: No after-fix real setup proof is present; the contributor should add redacted live output, logs, terminal output, screenshot with diagnostics, recording, or linked artifact and update the PR body for re-review.
  • mantis: telegram-visible-proof: Mantis should capture Telegram visible proof. The shared chat media change is visible in Telegram because a scanned PDF attachment should now produce a response based on page images rather than placeholder text.
Evidence reviewed

PR surface:

Source +52. Total +52 across 4 files.

View PR surface stats
Area Files Added Removed Net
Source 4 65 13 +52
Tests 0 0 0 0
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 4 65 13 +52

What I checked:

  • root policy read: Root AGENTS.md was read fully; its whole-surface PR review, message-delivery, and real behavior proof rules affected this review. (AGENTS.md:1, 770b19f496c1)
  • live PR context: The live PR is open, mergeable but unstable, changes four production files, and its body says live chat-channel proof will be supplied separately. (1a36c8db39ca)
  • current-main bug source: Current main's chat file extraction returns only text blocks and uses the old placeholder when extracted PDF images exist. (src/media-understanding/apply.ts:496, 770b19f496c1)
  • extractor contract: The shared file extractor can return PDF page images from extractPdfContent, so the dropped image payload exists before chat-path forwarding. (src/media/input-files.ts:441, 770b19f496c1)
  • sibling OpenResponses behavior: The OpenResponses input_file path already keeps the rendered-PDF placeholder and concatenates file.images into the model image payload. (src/gateway/openresponses-http.ts:587, 770b19f496c1)
  • PR image handoff: On the PR head, extractFileBlocks collects extracted PDF images and returns them alongside file context blocks. (src/media-understanding/apply.ts:504, 1a36c8db39ca)

Likely related people:

  • amknight: Recent commit e850750754f2 changed inbound PDF extraction limits on the same applyMediaUnderstanding and extractFileBlocks path. (role: recent PDF media-understanding contributor; confidence: high; commits: e850750754f2; files: src/media-understanding/apply.ts, src/media-understanding/file-extraction-limits.ts)
  • marchhao: Commit b5ada806dddc introduced current-turn image hydration, the helper this PR extends for extracted PDF page images. (role: current-turn image path contributor; confidence: high; commits: b5ada806dddc; files: src/auto-reply/reply/current-turn-images.ts, src/auto-reply/reply/get-reply-run.media-only.test.ts)
  • samzong: Commit 25ef9c0c419d introduced gateway attachment offload behavior with imageOrder and was co-authored/reviewed in the same mixed attachment surface. (role: chat attachment ordering contributor; confidence: medium; commits: 25ef9c0c419d; files: src/gateway/chat-attachments.ts)
  • steipete: Recent history shows repeated work across OpenResponses file input, current-turn image, ACP, and embedded runner media plumbing used as comparison surfaces here. (role: adjacent sibling-surface owner; confidence: medium; commits: 9b42f399a1af, 88b27c378d91, 77f1359612f6; files: src/gateway/openresponses-http.ts, src/auto-reply/reply/current-turn-images.ts, src/agents/embedded-agent-runner/run/images.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. mantis: telegram-visible-proof Mantis should capture Telegram visible proof. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. labels Jun 25, 2026
This was referenced Jun 25, 2026
Closed
Merged
@joshavant

joshavant commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Thanks @wanyongstar for the PR and for pushing the typed image-forwarding direction.

The same #96541 root cause has now been fixed by #97354, merged as 07b9349. The landed path forwards extracted scanned-PDF page images through the reply/ACP image plumbing, removes the stale placeholder wording, preserves ordering with other attachments, and includes live Telegram proof.

Closing this PR as superseded by the merged canonical fix.

@joshavant joshavant closed this Jun 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

mantis: telegram-visible-proof Mantis should capture Telegram visible proof. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: scanned / image-only PDFs are silently dropped on chat channels (rendered page images extracted then discarded)

2 participants

@wanyongstar @joshavant