Skip to content

fix(media-understanding): strip repeated placeholders#96431

Merged
vincentkoc merged 2 commits into
openclaw:mainfrom
lin-hongkuan:codex/media-understanding-strip-placeholders
Jun 24, 2026
Merged

fix(media-understanding): strip repeated placeholders#96431
vincentkoc merged 2 commits into
openclaw:mainfrom
lin-hongkuan:codex/media-understanding-strip-placeholders

Conversation

@lin-hongkuan

@lin-hongkuan lin-hongkuan commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Media-understanding bodies can contain synthetic placeholders for each attachment, such as <media:image> <media:audio> caption here. extractMediaUserText only stripped one leading placeholder, so the second synthetic placeholder was treated as user-authored text and sent back to the model.

Placeholder-only multi-attachment bodies were also treated as meaningful user text instead of being suppressed.

Why This Change Was Made

Synthetic media placeholders are transport markers, not user text. The placeholder matcher now accepts and strips repeated leading placeholder tokens while preserving the remaining caption text.

User Impact

Multi-attachment media-understanding prompts no longer leak extra <media:...> markers into the formatted model-visible body.

Evidence

Live behavior proof from this branch:

[Audio]
User text:
caption here
Transcript:
transcribed
---
[Image]
Description:
a chart

Focused validation:

RUN  v4.1.8 C:/Users/lin20/Documents/New project 3/openclaw

Test Files  1 passed (1)
     Tests  8 passed (8)
  Start at  20:58:32
  Duration  1.02s (transform 687ms, setup 557ms, import 14ms, tests 135ms, environment 0ms)

Checking formatting...

All matched files use the correct format.
Finished in 1430ms on 2 files using 32 threads.

git diff --check produced no output.

AI-assisted: OpenAI Codex helped inspect the media-understanding formatter and add focused regression coverage.

@clawsweeper

clawsweeper Bot commented Jun 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 24, 2026, 9:11 AM ET / 13:11 UTC.

Summary
The PR updates the media-understanding formatter to strip repeated leading <media:...> placeholders and adds regression tests for caption-preserving and placeholder-only cases.

PR surface: Source +1, Tests +30. Total +31 across 2 files.

Reproducibility: yes. Source inspection on current main shows only one leading media placeholder is stripped, so <media:image> <media:audio> caption here leaves the second marker in user text.

Review metrics: none identified.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
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:

  • none.

Next step before merge

  • [P2] No repair lane is needed because review found no actionable code defect and the remaining action is ordinary maintainer merge review.

Security
Cleared: The diff only changes formatter regexes and tests, with no dependency, workflow, secrets, or code-execution surface changes.

Review details

Best possible solution:

Keep this PR open until a linked canonical PR proves it covers this PR's unique work, or a maintainer confirms closure.

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

Yes. Source inspection on current main shows only one leading media placeholder is stripped, so <media:image> <media:audio> caption here leaves the second marker in user text.

Is this the best way to solve the issue?

Yes. The shared formatter is the narrowest maintainable fix location because both command-text preservation and model-visible body formatting already flow through extractMediaUserText; channel-specific fixes would duplicate the invariant.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: This is a narrow user-visible media-understanding prompt cleanup with limited blast radius.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes copied after-fix live output showing the repeated placeholders absent while caption text remains, plus focused test and formatting output.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied after-fix live output showing the repeated placeholders absent while caption text remains, plus focused test and formatting output.
Evidence reviewed

PR surface:

Source +1, Tests +30. Total +31 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 3 2 +1
Tests 1 30 0 +30
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 33 2 +31

What I checked:

  • PR close coverage proof: PR close coverage proof kept this PR open against fix: add placeholder transcript for silent voice notes #49131: PR B is adjacent media-placeholder work, but the PR A report states that after PR B was already merged, current main still only strips one leading placeholder. PR B does not directly carry the repeated-leading-placeholder formatter fix or its regression coverage.
  • linked superseding PR: fix: add placeholder transcript for silent voice notes #49131 (fix: add placeholder transcript for silent voice notes) is merged at 2026-04-26T04:14:01Z.
  • cluster evidence: the durable review links that PR in the work cluster or recommended risk path.
  • no human follow-up: live comments and timeline hydrated by apply contain no non-automation activity after the ClawSweeper review.

Likely related people:

  • steipete: Authored and merged the PR that extracted packages/media-understanding-common and moved the formatter tests into that package; also added adjacent media-understanding test coverage. (role: package extractor and recent area contributor; confidence: high; commits: affd322ae202, 62e4bcadfceb, 03d72c970ade; files: packages/media-understanding-common/src/format.ts, packages/media-understanding-common/src/format.test.ts, src/media-understanding/apply.test.ts)
  • eulicesl: Carried the merged synthetic audio placeholder/body flow through applyMediaUnderstanding, which shares the same formatter and placeholder invariant. (role: adjacent media-understanding contributor; confidence: medium; commits: 4c8bf1b9c1ee, d8de6f350769, 8b6cc6dc41b7; files: src/media-understanding/apply.ts, src/media-understanding/apply.test.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 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. P2 Normal backlog priority with limited blast radius. labels Jun 24, 2026
@vincentkoc
vincentkoc requested a review from a team as a code owner June 24, 2026 17:24
@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation channel: msteams Channel integration: msteams channel: telegram Channel integration: telegram app: ios App: ios app: web-ui App: web-ui gateway Gateway runtime extensions: memory-core Extension: memory-core scripts Repository scripts commands Command implementations docker Docker and sandbox tooling agents Agent runtime and tooling extensions: duckduckgo extensions: qa-lab extensions: lmstudio extensions: ollama extensions: xai extensions: parallel size: XL and removed size: XS labels Jun 24, 2026
@vincentkoc
vincentkoc merged commit f57a302 into openclaw:main Jun 24, 2026
162 of 167 checks passed
@vincentkoc

vincentkoc commented Jun 24, 2026

Copy link
Copy Markdown
Member

Merged via squash.

Thanks @lin-hongkuan!

@github-actions github-actions Bot mentioned this pull request Jun 24, 2026
Open
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jun 25, 2026
@lin-hongkuan @vincentkoc
fix(media-understanding): strip repeated placeholders (openclaw#96431)
8b84a18 
Merged via squash.

Prepared head SHA: 4b00486
Co-authored-by: lin-hongkuan <[email protected]>
Co-authored-by: vincentkoc <[email protected]>
Reviewed-by: @vincentkoc
QiuYuang pushed a commit to QiuYuang/openclaw that referenced this pull request Jul 1, 2026
@lin-hongkuan @vincentkoc @QiuYuang
fix(media-understanding): strip repeated placeholders (openclaw#96431)
3db354e 
Merged via squash.

Prepared head SHA: 4b00486
Co-authored-by: lin-hongkuan <[email protected]>
Co-authored-by: vincentkoc <[email protected]>
Reviewed-by: @vincentkoc
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

agents Agent runtime and tooling app: ios App: ios app: web-ui App: web-ui channel: msteams Channel integration: msteams channel: telegram Channel integration: telegram commands Command implementations docker Docker and sandbox tooling docs Improvements or additions to documentation extensions: duckduckgo extensions: lmstudio extensions: memory-core Extension: memory-core extensions: ollama extensions: parallel extensions: qa-lab extensions: xai gateway Gateway runtime 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. scripts Repository scripts size: XL 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.

2 participants

@lin-hongkuan @vincentkoc