Review findings from PR #333:
P1: _download_via_uri assumed output_video.uri is always files/<id>.
The API can return a full resource URI or a ready-made
.../files/<id>:download?alt=media download URL, which produced an
invalid poll path with a second :download appended. New
_file_id_from_uri() extracts the bare id from every documented shape;
regression tests cover the full-URL form plus a parametrized matrix of
URI shapes.
P2: docs/PROVIDERS.md still described the Google key as TTS + Imagen
only. The shared-key section now covers gemini_omni_video (model id,
~$0.10/sec pricing table, paid-tier-only, edit-turn billing note), and
the env snippet, provider-to-tool mapping, and capability coverage
tables include the new provider.
Add gemini_omni_video, a native Gemini API provider wrapping
gemini-omni-flash-preview via the Interactions API. Text-to-video,
image/reference-to-video with <FIRST_FRAME>/<IMAGE_REF_N> prompt tags,
and stateful edit_video turns via previous_interaction_id — the only
provider in the fleet that can refine a clip without regenerating it.
Reuses the existing GOOGLE_API_KEY / GEMINI_API_KEY, so one Google key
now unlocks images, TTS, and video.
- New Layer 3 skill .agents/skills/gemini-omni (prompting, edit-loop
rules, tag/timecode syntax, preview limits) sourced from official
Google docs; linked via agent_skills and the AGENT_GUIDE Layer 3 map
- ai-video-gen gains the Gemini API gateway row + editing pointer
- veo_video/sora_video fallback lists and video_selector agent_skills
reference the new provider; quality_score 0.85 with rationale
- Contract tests: registry discovery, selector routing, status from
env keys, uri + inline delivery, edit turns, typed image parts,
store=false editability, cost clamp
skills/creative/music-gen-usage.md mandates 'Always set
force_instrumental=true for video background', but music_gen.py never
sent the kwarg, so ElevenLabs could return vocal tracks that collide
with narration/dialogue.
- Add force_instrumental to input_schema (default True) so the mandate
holds by default; callers may opt out only with an explicit False.
- Include force_instrumental in the /v1/music payload.
- Add tests pinning: kwarg sent True by default, explicit opt-out
honored, and the schema default.
Refs: docs/REVIEW-image-to-video-voice.md §8 #8
Co-Authored-By: Claude <noreply@anthropic.com>
COGVIDEO_VARIANTS declares cogvideo-2b i2v=False (it is t2v-only), but
cogvideo_video advertised image_to_video + reference_image unconditionally
and the variant flag was never consulted. An image_to_video brief against
the 2B variant reached the diffusion pipeline and failed opaquely.
- Add is_operation_available(operation) that derives capability from the
variant table (the selector calls it without inputs, so it reports the
DEFAULT variant cogvideo-5b: t2v + i2v both True). This replaces an
implicit unconditional-True.
- Add an execute()-time guard that consults the CALLER's chosen variant
and fails fast with a clear error when it lacks the requested mode
(2B + image_to_video), instead of dropping into generate_local_video.
- Add _variant_for(inputs) helper shared by estimate_runtime / the guard.
Tests pin: the 2B premise (i2v=False), default-variant capability
reporting, fast-fail for 2B+i2v (generation never runs), and that 5B+i2v
still routes through to generate_local_video.
Refs: docs/REVIEW-image-to-video-voice.md §8 #4
Co-Authored-By: Claude <noreply@anthropic.com>
Three routing defects in video_selector, none previously covered by
tests (REVIEW §8 #3, #5, #7); plus the routing-test coverage itself (#10).
#3 Seedance dedup race
tool_by_provider keyed by provider STRING, so two tools legitimately
sharing provider="seedance" (seedance_video=fal, seedance_replicate)
collided — only the first-registered was ever selectable; the other
was invisible to the selector regardless of rank. Key selectable tools
by NAME instead; ranking picks the best of the shared-provider backends.
#5 preferred_provider had no score-gap gate
The selector returned the preferred provider on the first ranking match
no matter how far below the top it scored (the comment claimed "unless
drastically worse" but nothing enforced it). Add a configurable
preferred_provider_gap (default 0.15): honor the preference only when
its best ranked tool is within the gap of the overall top, else yield
to the top-ranked provider.
#7 fallback_tools appended image_selector unconditionally
The motion-required prohibition lived only in director skills, so a
direct caller could silently fall back to an image-only tool for an
image_to_video / reference_to_video brief. Add input-aware
fallback_tools_for(inputs) that drops image_selector for
motion-required operations; keep the static fallback_tools property
(with image_selector) for external consumers / contracts.
#10 routing coverage
First routing tests for video_selector: dedup reachability, the gap
gate (honored / ignored / configurable), motion-aware fallback, and
estimate_cost / estimate_runtime delegation. 13 tests, scoring patched
for determinism so they test routing logic, not the scorer.
Full tools + contracts suite green (638 passed, 6 skipped).
Refs: docs/REVIEW-image-to-video-voice.md §8 #3, #5, #7, #10
Co-Authored-By: Claude <noreply@anthropic.com>
audio_mixer hard-coded loudnorm I=-16 (Apple Podcasts) in both _mix
and _full_mix. sound-design.md targets -14 for YouTube/TikTok/IG, and
edit_decisions.metadata.loudnorm_target is the declarative form — but
the mixer never read it, so the executed loudness silently defaulted
to podcast levels regardless of the target platform.
- Add loudnorm_target to input_schema (default -16, clamped to [-40, 0]).
- Extract _loudnorm_filter() helper and use it in _mix and _full_mix so
a director can forward edit_decisions.metadata.loudnorm_target (or a
caller can pass it directly) to hit the right platform target.
- Add tests pinning: default -16, -14 honored, out-of-range clamped,
non-numeric fallback, and the schema default.
Refs: docs/REVIEW-image-to-video-voice.md §8 #1
Co-Authored-By: Claude <noreply@anthropic.com>
Every premium video provider sets quality_score (seedance 0.95, runway /
higgsfield 0.9) so the scorer ranks them above stock/local options.
grok_video had none, so it was scored only on supports/stability flags
despite shipping native synchronized audio (lip-sync + dialogue + SFX
in a single generation pass) — likely under-ranked.
Set quality_score=0.9, on par with the other native-audio premium
providers. Add a regression pinning the field and its get_info() surface.
Refs: docs/REVIEW-image-to-video-voice.md §8 #6
Co-Authored-By: Claude <noreply@anthropic.com>
`_segmented_music` mixed the video's audio with the shaped music via
`amix=inputs=2`, whose default `normalize=1` scales every input by 1/inputs
(x0.5, -6 dB). Unlike `_mix` and `_full_mix`, this path has no `loudnorm` stage
afterward to re-normalize, so the narration was permanently attenuated across
the entire timeline — including the stretches where the music volume expression
evaluates to 0. A one-second music segment quietly dropped the narration by
~6 dB for the whole video.
Add `normalize=0` to the amix: the music is already scaled to `music_volume`
by the `volume` expression, so speech passes at unity. Verified with ffmpeg —
narration in a no-music region tracks the stereo/aac conversion baseline
instead of sitting 6 dB below it.
The tool advertised `multiple_outputs: True`, accepted `n` (1-4) in its schema,
requested `n` images from the API, and scaled `estimate_cost` by `n` — but the
result handling was hardcoded to `response.data[0]`. Images 1..n-1 were decoded
never, written never, and absent from `artifacts`, so a caller who set `n=4`
paid for four images and received one.
Iterate over `response.data`, writing each image to a distinct path (suffixed
`_1`, `_2`, … when several are requested, mirroring `grok_image` /
`dashscope_image`), and return `outputs` / `images_generated` alongside the
full `artifacts` list. A single image keeps its exact requested path.
video_compose.get_info() reported render_engines.ffmpeg as always
available, unlike the real availability checks used for remotion and
hyperframes. On a machine without ffmpeg on PATH, preflight would
falsely report ffmpeg as usable, letting render_runtime="ffmpeg" get
locked at proposal time only to fail at compose.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The prior dunder denylist was still bypassable via print.__self__ (the builtins
module) -> .open(...), reachable with no import and no bare open/__builtins__/
getattr name. Enumerating dangerous dunders is whack-a-mole, so block ALL
dunder attribute access generically and allow only the tiny set legitimate
scenes need (super().__init__, occasional Type.__name__). This closes the
print.__self__ / .__class__ / .__globals__ introspection-escape class at once.
Static analysis still has a ceiling — a real subprocess sandbox is the complete
fix — but the default path no longer executes the reported secret-read payloads.
Adds regression tests for print.__self__ and for super().__init__ staying allowed.
Refs #219
The scan only flagged dangerous builtins as direct call targets (ast.Name func)
and dunders as attribute access, so it missed indirection like
`__builtins__['open']('.env').read()` and `getattr(o, '__class__')` — the
default path still executed secret-reading code.
Block dangerous identifiers wherever they appear as a bare name (open, eval,
exec, compile, __import__, __builtins__, getattr/setattr/delattr, globals/
locals/vars) rather than only as a call target, and extend the blocked dunder
set (__class__, __dict__, __getattribute__, __reduce__, ...). This closes the
reported no-import bypass while genuine math scenes still pass.
Still defense-in-depth, not a full sandbox; the allow_unsafe_code opt-out and
explicit code-execution contract remain. A subprocess-level sandbox is the
right follow-up for complete isolation.
Refs #219
math_animate writes caller-supplied Python to scene.py and runs Manim on it —
arbitrary local code execution with no boundary surfaced in the tool contract.
In an agent-driven system the scene_code may be LLM-generated or influenced by
untrusted prompt content, so import-time code or construct() could read
secrets/SSH material, open network connections, or spawn subprocesses.
Add a static AST safety scan that rejects dangerous imports (os, subprocess,
socket, requests, ctypes, ...), dangerous builtins (eval/exec/compile/open/
__import__), and sandbox-escape dunders (__globals__, __subclasses__, ...)
before Manim runs. Genuine math scenes (manim, numpy, math, ...) pass
untouched. This is defense-in-depth, not a sandbox: a determined attacker can
evade a static denylist, so it is paired with an explicit allow_unsafe_code
opt-out and a tool contract (schema + side_effects) that names the boundary.
Closes#219
full_mix with ducking enabled (the default) failed for a single narration
track + one music bed — the most common shape — because the ducking branch
appended an acopy[speech_dup] filter whose output pad was never consumed,
leaving the filtergraph with a dangling output that ffmpeg rejects.
For a single speech track speech_out is '[a0]' (starts with '[a'), so the
guarded append fired; the compensating pop() only removes the empty-string
case from the multi-speech branch, so the dead pad survived exactly in the
single-narration case. The speech stream is already re-derived for the final
mix via [speech_out], and ffmpeg auto-splits the reused input label, so the
duplicate is unnecessary. Multi-speech and SFX paths are unaffected.
Adds regression tests for single- and multi-narration full_mix with ducking.
Closes#265
Pixabay rejects per_page outside 3-200 with HTTP 400. The stock_sources
adapter already clamped, but the PixabayVideo and PixabayImage tools
passed the value through raw, so callers using per_page < 3 got a 400.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The timeout handling only took effect on a direct _remotion_render() call. The
high-level execute(operation='render') path goes through _render(), which builds
a fresh remotion_inputs dict (edit_decisions, output_path, profile) and dropped
remotion_timeout_ms — so callers of the documented operation='render' path never
got the timeout passed to the Remotion CLI. Forward it there.
Adds a test exercising _render() (not just _remotion_render()) to cover the
high-level forwarding path.
Refs #217
The high-level Remotion render path hid the useful failure reason. run_command
runs with check=True + capture_output, so a non-zero exit raised
CalledProcessError whose str() is only 'returned non-zero exit status 1' — the
actual Remotion diagnostics in stderr were dropped. Catch CalledProcessError
and surface the stderr/stdout tail, and TimeoutExpired with an actionable hint.
Also add a creator-facing remotion_timeout_ms input, passed through as
Remotion's --timeout (headless-browser setup + delayRender). Slow browser
startup on restricted networks previously failed opaquely at the default 30s
with no way to raise it. The subprocess timeout is widened to match so
run_command does not kill Remotion before its own timeout fires.
Closes#217
Addresses review feedback on export_bundle:
- If subtitles_path or thumbnail_path is provided but the file is missing, the
tool now fails with an explicit error instead of silently producing a package
without that asset (which could ship an approved deliverable missing part of
its content).
- Default export location now stays inside the project workspace: when the
render lives at projects/<name>/renders/..., the bundle defaults to
projects/<name>/exports/ (alongside artifacts/, assets/, renders/) rather than
a repo-root exports/<name>/. export_dir remains an explicit override.
Tests cover both: missing optional asset errors, and the project-workspace
default path.
Every pipeline ends in a publish stage that produces a publish_log artifact,
but tools/publishers/ shipped empty (only __init__.py) — no tool backed the
PUBLISH tier, so the mechanical packaging (copy the render, write metadata
files, lay out the export directory, emit a schema-valid publish_log) had to be
hand-rolled by the agent every run.
Add a local, offline export bundler:
- capability 'publish', provider 'local', runtime 'local', deterministic, no cost
- takes the final video_path plus the SEO metadata the publish-director prepares
(title, description, tags, hashtags, chapters, optional subtitles/thumbnail)
- writes exports/<project>/{video,metadata,thumbnails}/ matching the
publish-director skill's documented layout
- returns a schema-valid publish_log (status: 'exported') in data, validated
against schemas/artifacts/publish_log before returning so a bad entry fails
here rather than at checkpoint
It does not upload — a networked publisher (e.g. YouTube) can be added later as
a separate provider under the same 'publish' capability.
Tests cover the contract, registry discovery, the export layout, chapter-time
formatting, the schema-valid publish_log, and the missing-video error path.
AGENT_GUIDE.md requires the music decision to be made at the proposal stage,
but the only check for the user's music_library/ folder lived in the
asset-director skills, which run later. A user could approve a creative
direction without ever being told a free, intentional music option was sitting
on disk (issue #168).
music_library/ was already referenced as a source_tool in asset artifacts but
had no backing tool. Add a small read-only tool that scans the library folder
(default <project root>/music_library, override via MUSIC_LIBRARY_DIR or a
library_dir input) and lists the audio tracks it finds, with best-effort
durations via ffprobe when present.
Because it inherits BaseTool, the registry auto-discovers it and it appears in
the preflight provider menu alongside music_gen and the stock music sources:
- AVAILABLE when the folder holds at least one audio track
- UNAVAILABLE otherwise, with install_instructions telling the user how to add
tracks
So the user sees their music options before approving creative direction, with
no orchestration code changes. Read-only: no side effects, no cost.
Closes#168
The compose target resolution was resolved from `profile` (and documented as
overridable via edit_decisions.metadata.compose_target) but the per-segment
scale/pad filter hardcoded 1920x1080. As a result, vertical profiles such as
`tiktok` / `youtube_shorts` / `instagram_reels` silently produced landscape
1920x1080 output instead of 1080x1920 — a silent dimension bug, no error raised.
- Use the resolved target width/height in the per-segment scale/pad filter.
- Implement the previously-stubbed `metadata.compose_target` extension point:
{"width", "height", "fit"} where fit="pad" (letterbox, default, unchanged
behavior) or fit="cover" (scale-to-fill + centre-crop, ideal for vertical).
- Default with no profile/target stays 1920x1080 (backward compatible).
Adds tests/tools/test_video_compose_vertical.py covering default landscape,
profile=tiktok vertical, and compose_target cover override.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
runway (estimate_cost/estimate_runtime/execute) defaulted to gen4_turbo and
higgsfield (execute) defaulted to kling_3.0, while both schemas advertise
seedance_2.0 as model.default. Omitting model under-quoted cost (runway 6x:
$0.25 vs $1.50) and silently generated a different model than advertised,
violating the Decision-Communication / cost-accuracy contract.
Root cause was a default duplicated across schema + 3 methods that drifted.
Collapse it to a single _DEFAULT_MODEL constant referenced everywhere.
Add tests/tools/test_provider_model_defaults.py to lock each tool's
estimate default to its schema default and guard the execute path.
Adds a transcript_comparison check to VideoCompose._run_final_review
that word-diffs the whisper/whisperx transcript against script.txt and
fails loudly when the TTS engine literally voiced punctuation tokens
(dot, dots, ellipsis, comma, dash, hyphen, period). Chirp3-HD did this
to ellipses in a production run and it slipped past review; now it
cannot.
Also corrects the "tiny background video" gotcha in
skills/core/hyperframes.md. After six renders of blaming HyperFrames
CSS, the real root cause was 640x360 Pexels sources combined with a
fit-and-pad pre-transform — HyperFrames was rendering the letterboxed
input faithfully. Gotcha now walks through the ffprobe diagnostic and
the scale-to-cover fix, and keeps the wrapper-div pattern for the
right reasons (aspect mismatch handling, not framework bug workaround).
Four regression tests cover the new check: punctuation-leak detection,
clean-audio false-positive guard, graceful skip when inputs missing,
and always-present transcript_comparison section.
Separates creative grammar (renderer_family) from technical engine
(render_runtime) so HyperFrames can stand alongside Remotion as a
first-class runtime instead of masquerading as a Remotion sub-case.
Locks runtime choice at proposal stage and enforces it end-to-end: the
schemas require it, video_compose routes by it, the reviewer fails
closed on silent swaps, and a parametrized contract test walks every
pipeline manifest to ensure each planning-stage skill explains the
conversation to the user. Adds hyperframes_compose (scaffold/lint/
validate/render/doctor/add_block), a playbook -> CSS style bridge, and
vendored HyperFrames Layer 3 skills from commit d291358, pinned via
PROVENANCE.md for future re-sync. Final_review now records
render_runtime_used and runtime_swap_detected so compose lies are
catchable after the fact.
