Skip to content

fix(media): forward scanned PDF page images#96690

Closed
TurboTheTurtle wants to merge 1 commit into
openclaw:mainfrom
TurboTheTurtle:fix/96541-scanned-pdf-chat-images
Closed

fix(media): forward scanned PDF page images#96690
TurboTheTurtle wants to merge 1 commit into
openclaw:mainfrom
TurboTheTurtle:fix/96541-scanned-pdf-chat-images

Conversation

@TurboTheTurtle

@TurboTheTurtle TurboTheTurtle commented Jun 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Closes #96541.

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, consume them once in normal reply and ACP dispatch, and update the media-understanding docs to remove the stale "images not forwarded" wording.

Evidence

  • Added source-level regressions proving rendered PDF page images are retained, consumed by current-turn image resolution, and forwarded into ACP runTurn attachments.
  • Re-ran focused media-understanding, normal/ACP reply image handling, agent-runner execution, and sibling OpenResponses file-only PDF coverage before push.
  • Local type, lint, docs formatting, and diff whitespace checks passed on this branch.

Live chat-channel proof will be supplied separately.

@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation size: M triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jun 25, 2026
@TurboTheTurtle
TurboTheTurtle force-pushed the fix/96541-scanned-pdf-chat-images branch from ce31730 to 248a417 Compare June 25, 2026 07:22
@clawsweeper

clawsweeper Bot commented Jun 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 25, 2026, 5:12 AM ET / 09:12 UTC.

Summary
This PR carries extracted scanned-PDF page images from media-understanding file extraction into transient current-turn image payloads for normal replies and ACP dispatch, adds regression tests, and updates the media-understanding docs wording.

PR surface: Source +49, Tests +116, Docs 0. Total +165 across 8 files.

Reproducibility: yes. at source level. Current main can receive PDF-rendered images from extractFileContentFromSource and then emits only a placeholder text block on the chat media-understanding path; I did not run a live chat send in this read-only review.

Review metrics: 1 noteworthy metric.

  • Transient context field: 1 added. MsgContext.CurrentTurnImages is a new in-memory delivery field, so reviewers should verify it is consumed once and not persisted into session state.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #96541
Summary: This PR is a candidate fix for the canonical scanned/image-only PDF chat-channel forwarding bug; the nearby clawpdf rendering-budget issue is related but distinct.

Members:

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

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

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

Rank-up moves:

  • [P2] Add or link native Telegram Desktop proof if maintainers want transport-level confirmation before merge.

Mantis proof suggestion
A native Telegram Desktop proof would materially show the visible chat-channel behavior even though the contributor's live-output proof clears the basic real-proof gate. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis telegram desktop proof: verify that sending a scanned PDF to a Telegram agent forwards page images and the agent can answer from the document.

Risk before merge

  • [P1] This changes model input delivery for scanned PDF chat attachments, so maintainers should treat it as a message-delivery change even though the implementation is narrow and tested.
  • [P1] Native Telegram Desktop proof did not complete in the current checks, so maintainers still lack a transport-level recording; the contributor's live-output proof covers extraction, current-turn forwarding, and a live vision read.
  • [P1] fix(media): forward scanned PDF page images as current-turn images #96706 is an overlapping open candidate for the same canonical issue, so maintainers should choose one landing branch and close the duplicate after merge.

Maintainer options:

  1. Proceed With This Branch (recommended)
    Maintainers can land this typed handoff after normal review and accept that the remaining proof is live output rather than a native Telegram recording.
  2. Request Native Telegram Proof
    Ask for a Telegram Desktop proof before merge if transport-level confidence is required for the visible chat behavior.
  3. Close The Overlapping Candidate
    After choosing this PR as the landing path, close fix(media): forward scanned PDF page images as current-turn images #96706 as superseded or keep it only if maintainers prefer that branch.

Next step before merge

  • No ClawSweeper repair is indicated; the remaining action is maintainer selection and merge handling for this PR versus the overlapping candidate.

Security
Cleared: No concrete security or supply-chain issue was found; the diff changes in-memory media forwarding, docs, and tests without new dependencies, workflows, secrets handling, or package resolution changes.

Review details

Best possible solution:

Land the typed current-turn image handoff after maintainer review, then close the overlapping candidate PR and let the canonical issue close from the merged fix.

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 emits only a placeholder text block on the chat media-understanding path; I did not run a live chat send in this read-only review.

Is this the best way to solve the issue?

Yes. Carrying extracted page images through existing current-turn image plumbing is the narrow maintainable fix, and this PR preserves existing imageOrder semantics while covering normal reply and ACP paths.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: This is a bounded media-understanding bug fix for scanned PDF attachments across chat and ACP paths, with limited blast radius and no outage signal.
  • merge-risk: 🚨 message-delivery: The PR changes how extracted PDF page images are delivered into chat and ACP model turns, which can affect whether attachment content reaches the model.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The contributor added after-fix live-output proof showing PR head extracted a raster-only PDF, consumed one CurrentTurnImages inline image, and a live vision model read the extracted phrase.
  • proof: sufficient: Contributor real behavior proof is sufficient. The contributor added after-fix live-output proof showing PR head extracted a raster-only PDF, consumed one CurrentTurnImages inline image, and a live vision model read the extracted phrase.
  • 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 an answer based on page images rather than placeholder text.
Evidence reviewed

PR surface:

Source +49, Tests +116, Docs 0. Total +165 across 8 files.

View PR surface stats
Area Files Added Removed Net
Source 4 73 24 +49
Tests 3 116 0 +116
Docs 1 1 1 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 8 190 25 +165

What I checked:

  • Root policy read and applied: Read the full root AGENTS.md; its whole-surface review, message-delivery risk, docs, and real-behavior-proof guidance shaped this PR review. (AGENTS.md:1, d693b70bfcc1)
  • Scoped docs policy read: docs/AGENTS.md was read because the PR changes docs/nodes/media-understanding.md; no docs-specific defect was found. Public docs: docs/AGENTS.md. (docs/AGENTS.md:1, d693b70bfcc1)
  • Current main drops chat-path PDF images: On current main, extractFileBlocks returns only text blocks and converts extracted PDF images into the old placeholder instead of carrying image payloads forward. (src/media-understanding/apply.ts:496, d693b70bfcc1)
  • Extractor contract can produce image payloads: extractFileContentFromSource returns PDF page images from extractPdfContent, so the dropped payload exists before the chat-path handoff. (src/media/input-files.ts:441, d693b70bfcc1)
  • Sibling OpenResponses path forwards the same images: The OpenResponses input_file path already appends file.images into the model image payload while keeping the rendered-PDF placeholder text. (src/gateway/openresponses-http.ts:587, d693b70bfcc1)
  • PR carries extracted images out of file extraction: On the PR head, extractFileBlocks returns blocks plus images and pushes extracted PDF page images into the new image array. (src/media-understanding/apply.ts:504, 248a417653b6)

Likely related people:

  • frankekn: Commit cb18ce7 added the media-understanding file attachment extraction path and related tests that this PR extends. (role: introduced chat file-extraction behavior; confidence: high; commits: cb18ce7a8566; files: src/media-understanding/apply.ts, src/media-understanding/apply.test.ts)
  • steipete: Commits bbc67f3 and f06ad45 introduced/refactored the OpenResponses input_file path that already forwards extracted file images, and dcfc1f1 touched ACP attachment resolution tests. (role: adjacent sibling-surface owner; confidence: high; commits: bbc67f375472, f06ad4502b4a, dcfc1f16ed31; files: src/gateway/openresponses-http.ts, src/media/input-files.ts, src/auto-reply/reply/agent-turn-attachments.ts)
  • martinfrancois: Commit 734bb9c recently hardened Telegram/document prompt-facing attachment handling in the same media-understanding apply path. (role: recent area contributor; confidence: medium; commits: 734bb9c2e73b; files: src/media-understanding/apply.ts, src/media-understanding/apply.test.ts)
  • mbelinky: Commit 4aebff7 introduced ACP runtime attachment forwarding, which is the ACP sibling path this PR updates for extracted PDF page images. (role: adjacent ACP attachment contributor; confidence: medium; commits: 4aebff78bc32; files: src/auto-reply/reply/dispatch-acp.ts, src/auto-reply/reply/agent-turn-attachments.ts)
  • vignesh07: Commit 6d11b46 preserved PDF MIME classification so PDFs stay on the dedicated extractor path that can return page-image payloads. (role: recent PDF path contributor; confidence: medium; commits: 6d11b4699494; files: src/media-understanding/apply.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.

@openclaw-barnacle openclaw-barnacle Bot removed the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jun 25, 2026
@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
Closed
@TurboTheTurtle

TurboTheTurtle commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

Live proof against PR head 248a417653b6 for the scanned/image-only PDF path:

  • Built a one-page image-only PDF fixture; the unique phrase exists only inside the raster page image: PR96690 CITRINE SCAN.
  • Ran PR-head media-understanding file extraction: appliedFile=true, [PDF content rendered to images] present, old images not forwarded placeholder absent.
  • Current-turn forwarding: CurrentTurnImages before consume was 1; resolveCurrentTurnImages returned 1 inline image/png and consumed it from ctx.
  • Live vision read of that exact extracted image: gpt-5.4-mini answered PR96690 CITRINE SCAN IMAGE-ONLY PDF LIVE PROOF.

Evidence anchor: extracted image SHA-256 2decb91a89c93bb5c2d61e9451e402c6ea3909e382bf40437aa2508f9f5d356e; decoded image bytes 55,927.

@TurboTheTurtle

TurboTheTurtle commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed 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. labels Jun 25, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 28, 2026
Merged
@joshavant

joshavant commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Thanks @TurboTheTurtle for the PR and the live-output proof. This was a strong candidate for the same #96541 root cause.

Maintainers landed #97354 as the canonical fix, merged as 07b9349. The merged implementation follows the same core direction: extracted scanned/image-only PDF page images are carried as typed image inputs into normal reply and ACP dispatch paths instead of being reduced to placeholder text. It also preserves mixed attachment ordering, updates the docs wording, and includes live Telegram proof.

Closing this PR as superseded by the merged canonical fix.

@joshavant joshavant closed this Jun 28, 2026
@TurboTheTurtle
TurboTheTurtle deleted the fix/96541-scanned-pdf-chat-images branch June 28, 2026 04:13
@github-actions github-actions Bot mentioned this pull request Jun 28, 2026
Open
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

docs Improvements or additions to documentation 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. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: M status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

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

@TurboTheTurtle @joshavant