From 8c9af320b64e68f8c8d54883f073d4a2003cf240 Mon Sep 17 00:00:00 2001 From: 0xDevNinja Date: Thu, 2 Jul 2026 11:59:52 +0530 Subject: [PATCH] fix(video_compose): forward remotion_timeout_ms through high-level render path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- tests/tools/test_remotion_diagnostics.py | 33 ++++++++++++++++++++++++ tools/video/video_compose.py | 5 ++++ 2 files changed, 38 insertions(+) diff --git a/tests/tools/test_remotion_diagnostics.py b/tests/tools/test_remotion_diagnostics.py index f86526e7..79db4633 100644 --- a/tests/tools/test_remotion_diagnostics.py +++ b/tests/tools/test_remotion_diagnostics.py @@ -78,6 +78,39 @@ def test_remotion_timeout_ms_is_passed_through(tool, tmp_path, monkeypatch): assert seen["timeout"] >= 180 +def test_high_level_render_forwards_timeout_to_remotion(tool, tmp_path, monkeypatch): + # The gap in the first cut: execute(operation="render") -> _render() builds a + # fresh remotion_inputs dict, so the option must be forwarded there, not only + # on a direct _remotion_render() call. + captured = {} + monkeypatch.setattr(tool, "_pre_compose_validation", lambda *a, **k: None) + monkeypatch.setattr(tool, "_needs_remotion", lambda *a, **k: True) + + def fake_remotion_render(inputs): + captured.update(inputs) + from tools.base_tool import ToolResult + + return ToolResult(success=True, data={}, artifacts=[]) + + monkeypatch.setattr(tool, "_remotion_render", fake_remotion_render) + monkeypatch.setattr(tool, "_run_final_review", lambda *a, **k: {}) + + tool._render( + { + "edit_decisions": { + "render_runtime": "remotion", + "renderer_family": "explainer-data", + "cuts": [{"id": "c1", "source": "a1", "in_seconds": 0, "out_seconds": 2}], + }, + "asset_manifest": {"assets": [{"id": "a1", "path": "/tmp/a1.mp4"}]}, + "output_path": str(tmp_path / "out.mp4"), + "remotion_timeout_ms": 120000, + } + ) + + assert captured.get("remotion_timeout_ms") == 120000 + + def test_no_timeout_flag_when_not_requested(tool, tmp_path, monkeypatch): seen = {} diff --git a/tools/video/video_compose.py b/tools/video/video_compose.py index badfedf3..83e6f428 100644 --- a/tools/video/video_compose.py +++ b/tools/video/video_compose.py @@ -1401,6 +1401,11 @@ class VideoCompose(BaseTool): } if profile: remotion_inputs["profile"] = profile + # Forward the creator-facing render timeout through the high-level + # render path (execute(operation="render") -> _render), otherwise it + # would only take effect on a direct _remotion_render() call. + if inputs.get("remotion_timeout_ms") is not None: + remotion_inputs["remotion_timeout_ms"] = inputs["remotion_timeout_ms"] render_result = self._remotion_render(remotion_inputs) # Governance: NEVER silently fall back to FFmpeg when Remotion fails.