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>
The provider tools read these env vars but .env.example never listed
them, so a fresh setup silently misses them:
- FAL_AI_API_KEY — read by kling/minimax/veo/seedance/recraft/flux
as an alias for FAL_KEY (os.environ.get('FAL_KEY') or FAL_AI_API_KEY),
but only FAL_KEY was documented.
- REPLICATE_API_TOKEN — required by seedance_replicate; without it the
Replicate-backed Seedance path stays unselectable.
- HIGGSFIELD_API_KEY / _API_SECRET / HIGGSFIELD_KEY — the three forms
higgsfield_video.py accepts (combined key:secret vs split pair).
Doc-only, zero runtime change.
Refs: docs/REVIEW-image-to-video-voice.md §8 #11
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.
The bundled wan22-t2v-4step.json workflow loads the 14B FP8 diffusion
pair (wan2.2_t2v_high/low_noise_14B_fp8_scaled.safetensors), which
produce 16-channel latents, but its VAELoader referenced
wan2.2_vae.safetensors — the WAN 2.2 5B model's VAE, which expects
48-channel latents. Every T2V run therefore failed at VAEDecode with:
Expected tensor to have size 48 at dimension 1, but got size 16
Switch the workflow to wan_2.1_vae.safetensors, matching the 14B
models and the sibling wan22-i2v-4step.json, and update the T2V
required-models list in tools/video/comfyui_video.py to match so
preflight checks for the VAE that is actually used.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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>
From an independent review of the branch:
- Regression: existing non-raster-but-showable visuals (.svg diagrams) were
dropped by the renderable filter — they were served fine before via <img>
(/thumb passes SVG through). Add .svg to MEDIA_IMAGE_EXT.
- Doc accuracy: the board dedupes decisions by (category, subject), not category
alone (category-only would wrongly merge distinct decisions that share a
category, e.g. TTS vs image provider_selection). Correct AGENT_GUIDE to say
the pair is the key and to reuse the same subject when re-logging.
- Coverage: the new visual-selection logic was untested (which let the earlier
missing-file regression through). Add TestStoryboardVisualSelection covering
the .tsx-animation exclusion, snapshot fallback (exact + <id>_* match), SVG
renderability, the preserved missing-file indicator, and takes = renderable.
Not changed (reviewed, deliberate): .narr clamp at --fs-scale 1.16 degrades
gracefully via the fade + click-to-expand modal; decision dedupe stays keyed on
(category, subject) as the more-correct behavior.
The renderable-only filter from the previous commit also dropped assets whose
file is missing, so the storyboard lost its "asset in manifest, file missing"
slot (broke tests/backlot/test_state.py::test_full_project).
Distinguish the two cases: a missing image/video/diagram still surfaces as the
missing indicator; only an asset that EXISTS but can't be shown (a .tsx atelier
composition) falls back to a per-scene snapshot/placeholder.
Enables real-geography maps in atelier compositions (d3-geo + world-atlas
TopoJSON coastlines with animated great-circle arcs), used by the salt-history
trade-route scene.
