Skip to content

fix(gateway): auto-deliver video_generate output without a model-emitted MEDIA tag#45786

Closed
Julientalbot wants to merge 1 commit into
NousResearch:mainfrom
Julientalbot:fix/media-autoappend-producer-tools
Closed

fix(gateway): auto-deliver video_generate output without a model-emitted MEDIA tag#45786
Julientalbot wants to merge 1 commit into
NousResearch:mainfrom
Julientalbot:fix/media-autoappend-producer-tools

Conversation

@Julientalbot

Copy link
Copy Markdown
Contributor

What does this PR do?

Makes generated-video delivery model-agnostic, and generalizes the existing
deterministic media auto-append so it covers the whole producer-tool family
instead of just image_generate.

The bug. video_generate returns a structured result —
{"success": true, "video": "<url-or-abs-path>"} (see
agent/video_gen_provider.py success_response) — but it was absent from
both _AUTO_APPEND_MEDIA_TOOL_NAMES and _JSON_MEDIA_TOOL_PATH_FIELDS in
gateway/run.py. So a generated video is delivered only if the model restates
a correctly-formatted MEDIA: tag in its prose reply
. When the model omits the
tag, or wraps/mis-formats it (e.g. Markdown emphasis), the file is silently never
attached and the user gets nothing. image_generate already avoids this by
reading its path from the tool's JSON result; videos had no such path.

The fix. Generalize the proven image_generate JSON branch into a small
per-tool field map and add video_generate to the allowlist:

_JSON_MEDIA_TOOL_PATH_FIELDS = {
    "image_generate": ("host_image", "image", "agent_visible_image"),
    "video_generate": ("video",),
}

The path is now read from the tool's structured result, so delivery no longer
depends on how (or whether) the model restates it in free text. This is the
model-agnostic counterpart to the prose-parsing path.

Related Issue

Companion to #45773 (fix(gateway): deliver MEDIA: tags wrapped in Markdown emphasis) — different layer, disjoint files. #45773 hardens the prose-parse
path (extract_media regex in base.py) for hand-authored / pre-existing paths;
this PR fixes the structured-harvest path (auto-append in run.py) for
tool-produced media. They are independent and can land in either order. No
separate issue filed — the gap is fully described above.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • gateway/run.py — add "video_generate" to _AUTO_APPEND_MEDIA_TOOL_NAMES;
    turn _JSON_MEDIA_TOOL_PATH_FIELDS into a per-tool field map; generalize the
    tool_name == "image_generate" branch to json_fields = MAP.get(tool_name).
    Docstring/comments updated. No interface, schema, or config change.
  • tests/gateway/test_media_extraction.pyvideo_generate coverage:
    structured path delivered, remote URL ignored, failed generation ignored,
    history dedup, and a guard that the "video" field is only harvested from the
    allowlisted producer tool (a execute_code look-alike result is not
    delivered).

How to Test

  1. Before: a video_generate result {"success": true, "video": "/tmp/clip.mp4"}
    with a text-only assistant reply → video not delivered.
  2. After: same input → _collect_auto_append_media_tags(...) returns
    ["MEDIA:/tmp/clip.mp4"] → delivered, independent of the model's prose.
  3. pytest tests/gateway/test_media_extraction.py -q → all pass (incl. the
    existing image_generate / TTS / stale-leak / compression-fallback cases).

False-positive safety

All four existing guards are reused unchanged:

  1. Producer-tool allowlist — only image_generate, video_generate, TTS are
    harvested; generic file producers (write_file, terminal, …) are excluded,
    so an internal/temp file they write is never auto-delivered.
  2. Success-gatepayload.get("success") must be truthy.
  3. _TOOL_MEDIA_RE — requires a deliverable extension + a real local-path
    anchor, so remote URLs (a provider may return a URL in video) and
    non-media artifacts are rejected (delivery then falls back to the gateway's
    existing URL-download path — unchanged, out of scope here).
  4. Current-turn isolation + history dedup — unchanged.

Checklist

Code

  • I've read the Contributing Guide
  • Commit messages follow Conventional Commits (fix(gateway):)
  • I searched for existing PRs (this complements fix(gateway): deliver MEDIA: tags wrapped in Markdown emphasis #45773; not a duplicate)
  • My PR contains only changes related to this fix
  • I've run pytest tests/ -q — ran the media zone: tests/gateway/test_media_extraction.py, test_platform_base.py, test_run_tool_media_re.py (196 passed, 2 skipped); change is localized to the auto-append path
  • I've added tests for my changes
  • I've tested on my platform: Debian 13

Documentation & Housekeeping

  • Updated docstring/comments in gateway/run.py
  • N/A — no config keys changed
  • N/A — no architecture/workflow change
  • Cross-platform: path anchor + extension guard unchanged; no platform-specific assumptions
  • N/A — no tool behavior/schema change

…ted MEDIA tag

video_generate returns a structured {"success": true, "video": "<url-or-abs-path>"}
result, but it was absent from the auto-append allowlist and the JSON field map,
so a generated video was delivered ONLY if the model restated a correctly
formatted MEDIA: tag in its prose reply. When the model omits or mis-formats the
tag (e.g. wraps it in Markdown), the video is silently never attached.

Generalize the existing deterministic image_generate auto-append into a per-tool
field map (image_generate -> host_image/image/agent_visible_image,
video_generate -> video) and add video_generate to the producer-tool allowlist,
so the path is read from the tool's structured result rather than from the
model's free text. All four existing guards are reused unchanged: producer-tool
allowlist (write_file/terminal etc. excluded), success-gate, _TOOL_MEDIA_RE
(rejects remote URLs and non-deliverable extensions), and current-turn isolation
+ history dedup. TTS keeps its existing MEDIA:-scan path (which also carries the
[[audio_as_voice]] directive).

Adds TestMediaExtraction coverage: video_generate structured path delivered,
remote URL ignored, failed generation ignored, history dedup, and a guard that
the "video" field is only harvested from the allowlisted producer tool.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have duplicate This issue or pull request already exists labels Jun 13, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Duplicate of #43065 — same mechanism: add video_generate to _AUTO_APPEND_MEDIA_TOOL_NAMES plus a video path-field handler in gateway/run.py so generated video delivers without the model restating the MEDIA: tag. #43065 is the earlier open PR. (Companion to #45773, which fixes the disjoint prose-parse layer.)

@Julientalbot

Copy link
Copy Markdown
Contributor Author

Closing as a duplicate of #43065 — thanks @alt-glitch for the catch. #43065 (opened 2026-06-09) lands the same mechanism first: video_generate_AUTO_APPEND_MEDIA_TOOL_NAMES + reading the structured video field. My "not a duplicate" checkbox was scoped to #45773 (the disjoint prose-parse layer) and I missed the earlier video PR — that's on me.

Two deltas from this PR that may be worth folding into #43065, offered there, not as a reason to keep this one open:

  1. a per-tool field map ({"image_generate": (...), "video_generate": ("video",)}) instead of a second copy-pasted handler branch;
  2. five video_generate test cases (structured-path delivered, remote-URL ignored, failed-gen ignored, history dedup, producer-allowlist guard).

Deferring to the earlier PR. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery duplicate This issue or pull request already exists P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants