diff --git a/app/services/run_service.py b/app/services/run_service.py index d92acd4..489fd52 100644 --- a/app/services/run_service.py +++ b/app/services/run_service.py @@ -124,19 +124,23 @@ class RunService: # Check database for completed run cached = await self.db.get_run_cache(run_id) if cached: - return { - "run_id": run_id, - "status": "completed", - "recipe": cached.get("recipe"), - "inputs": self._ensure_inputs_list(cached.get("inputs")), - "output_cid": cached.get("output_cid"), - "ipfs_cid": cached.get("ipfs_cid"), - "provenance_cid": cached.get("provenance_cid"), - "plan_cid": cached.get("plan_cid"), - "actor_id": cached.get("actor_id"), - "created_at": cached.get("created_at"), - "completed_at": cached.get("created_at"), - } + output_cid = cached.get("output_cid") + # Only return as completed if we have an output + # (runs with no output should be re-executed) + if output_cid: + return { + "run_id": run_id, + "status": "completed", + "recipe": cached.get("recipe"), + "inputs": self._ensure_inputs_list(cached.get("inputs")), + "output_cid": output_cid, + "ipfs_cid": cached.get("ipfs_cid"), + "provenance_cid": cached.get("provenance_cid"), + "plan_cid": cached.get("plan_cid"), + "actor_id": cached.get("actor_id"), + "created_at": cached.get("created_at"), + "completed_at": cached.get("created_at"), + } # Check database for pending run pending = await self.db.get_pending_run(run_id) diff --git a/legacy_tasks.py b/legacy_tasks.py index b965a15..bc45082 100644 --- a/legacy_tasks.py +++ b/legacy_tasks.py @@ -734,6 +734,26 @@ def execute_recipe(self, recipe_sexp: str, input_hashes: Dict[str, str], run_id: # Handle SOURCE nodes if step.node_type == "SOURCE": source_cid = step.config.get("cid") + + # If source has :input true, resolve CID from input_hashes + if not source_cid and step.config.get("input"): + source_name = step.config.get("name", "") + # Try various key formats for lookup + name_variants = [ + source_name, + source_name.lower().replace(" ", "-"), + source_name.lower().replace(" ", "_"), + source_name.lower(), + ] + for variant in name_variants: + if variant in input_hashes: + source_cid = input_hashes[variant] + logger.info(f"Resolved SOURCE '{source_name}' -> {source_cid[:16]}... via '{variant}'") + break + + if not source_cid: + raise ValueError(f"SOURCE '{source_name}' not found in input_hashes. Available: {list(input_hashes.keys())}") + if source_cid: source_path = cache_manager.get_by_cid(source_cid) if source_path: @@ -747,7 +767,9 @@ def execute_recipe(self, recipe_sexp: str, input_hashes: Dict[str, str], run_id: total_cached += 1 continue else: - raise ValueError(f"Source content not found: {source_cid}") + raise ValueError(f"Source content not found in cache: {source_cid[:16]}...") + else: + raise ValueError(f"SOURCE step has no cid and no :input flag: {step.config}") # Get executor for this step type executor = get_executor(step.node_type) diff --git a/tests/test_execute_recipe.py b/tests/test_execute_recipe.py new file mode 100644 index 0000000..bd3ae1d --- /dev/null +++ b/tests/test_execute_recipe.py @@ -0,0 +1,246 @@ +""" +Tests for execute_recipe SOURCE node resolution. + +These tests verify that SOURCE nodes with :input true are correctly +resolved from input_hashes at execution time. +""" + +import pytest +from unittest.mock import MagicMock, patch +from typing import Dict, Any + + +class MockStep: + """Mock ExecutionStep for testing.""" + + def __init__( + self, + step_id: str, + node_type: str, + config: Dict[str, Any], + cache_id: str, + input_steps: list = None, + level: int = 0, + ): + self.step_id = step_id + self.node_type = node_type + self.config = config + self.cache_id = cache_id + self.input_steps = input_steps or [] + self.level = level + self.name = config.get("name") + self.outputs = [] + + +class MockPlan: + """Mock ExecutionPlanSexp for testing.""" + + def __init__(self, steps: list, output_step_id: str, plan_id: str = "test-plan"): + self.steps = steps + self.output_step_id = output_step_id + self.plan_id = plan_id + + def to_string(self, pretty: bool = False) -> str: + return "(plan test)" + + +def resolve_source_cid(step_config: Dict[str, Any], input_hashes: Dict[str, str]) -> str: + """ + Resolve CID for a SOURCE node. + + This is the logic that should be in execute_recipe - extracted here for unit testing. + """ + source_cid = step_config.get("cid") + + # If source has :input true, resolve CID from input_hashes + if not source_cid and step_config.get("input"): + source_name = step_config.get("name", "") + # Try various key formats for lookup + name_variants = [ + source_name, + source_name.lower().replace(" ", "-"), + source_name.lower().replace(" ", "_"), + source_name.lower(), + ] + for variant in name_variants: + if variant in input_hashes: + source_cid = input_hashes[variant] + break + + if not source_cid: + raise ValueError( + f"SOURCE '{source_name}' not found in input_hashes. " + f"Available: {list(input_hashes.keys())}" + ) + + return source_cid + + +class TestSourceCidResolution: + """Tests for SOURCE node CID resolution from input_hashes.""" + + def test_source_with_fixed_cid(self): + """SOURCE with :cid should use that directly.""" + config = {"cid": "QmFixedCid123"} + input_hashes = {} + + cid = resolve_source_cid(config, input_hashes) + assert cid == "QmFixedCid123" + + def test_source_with_input_true_exact_match(self): + """SOURCE with :input true should resolve from input_hashes by exact name.""" + config = {"input": True, "name": "my-video"} + input_hashes = {"my-video": "QmInputVideo456"} + + cid = resolve_source_cid(config, input_hashes) + assert cid == "QmInputVideo456" + + def test_source_with_input_true_normalized_dash(self): + """SOURCE with :input true should resolve from normalized dash format.""" + config = {"input": True, "name": "Second Video"} + input_hashes = {"second-video": "QmSecondVideo789"} + + cid = resolve_source_cid(config, input_hashes) + assert cid == "QmSecondVideo789" + + def test_source_with_input_true_normalized_underscore(self): + """SOURCE with :input true should resolve from normalized underscore format.""" + config = {"input": True, "name": "Second Video"} + input_hashes = {"second_video": "QmSecondVideoUnderscore"} + + cid = resolve_source_cid(config, input_hashes) + assert cid == "QmSecondVideoUnderscore" + + def test_source_with_input_true_lowercase(self): + """SOURCE with :input true should resolve from lowercase format.""" + config = {"input": True, "name": "MyVideo"} + input_hashes = {"myvideo": "QmLowercaseVideo"} + + cid = resolve_source_cid(config, input_hashes) + assert cid == "QmLowercaseVideo" + + def test_source_with_input_true_missing_raises(self): + """SOURCE with :input true should raise if not in input_hashes.""" + config = {"input": True, "name": "Missing Video"} + input_hashes = {"other-video": "QmOther123"} + + with pytest.raises(ValueError) as excinfo: + resolve_source_cid(config, input_hashes) + + assert "Missing Video" in str(excinfo.value) + assert "not found in input_hashes" in str(excinfo.value) + assert "other-video" in str(excinfo.value) # Shows available keys + + def test_source_without_cid_or_input_returns_none(self): + """SOURCE without :cid or :input should return None.""" + config = {"name": "some-source"} + input_hashes = {} + + cid = resolve_source_cid(config, input_hashes) + assert cid is None + + def test_source_priority_cid_over_input(self): + """SOURCE with both :cid and :input true should use :cid.""" + config = {"cid": "QmDirectCid", "input": True, "name": "my-video"} + input_hashes = {"my-video": "QmInputHashCid"} + + cid = resolve_source_cid(config, input_hashes) + assert cid == "QmDirectCid" # :cid takes priority + + +class TestSourceNameVariants: + """Tests for the various input name normalization formats.""" + + @pytest.mark.parametrize("source_name,input_key", [ + ("video", "video"), + ("My Video", "my-video"), + ("My Video", "my_video"), + ("My Video", "My Video"), + ("CamelCase", "camelcase"), + ("multiple spaces", "multiple spaces"), # Only single replace + ]) + def test_name_variant_matching(self, source_name: str, input_key: str): + """Various name formats should match.""" + config = {"input": True, "name": source_name} + input_hashes = {input_key: "QmTestCid"} + + cid = resolve_source_cid(config, input_hashes) + assert cid == "QmTestCid" + + +class TestExecuteRecipeIntegration: + """Integration tests for execute_recipe with SOURCE nodes.""" + + def test_recipe_with_user_input_source(self): + """ + Recipe execution should resolve SOURCE nodes with :input true. + + This is the bug that was causing "No executor for node type: SOURCE". + """ + # Create mock plan with a SOURCE that has :input true + source_step = MockStep( + step_id="source_1", + node_type="SOURCE", + config={"input": True, "name": "Second Video", "description": "User input"}, + cache_id="abc123", + level=0, + ) + + effect_step = MockStep( + step_id="effect_1", + node_type="EFFECT", + config={"effect": "invert"}, + cache_id="def456", + input_steps=["source_1"], + level=1, + ) + + plan = MockPlan( + steps=[source_step, effect_step], + output_step_id="effect_1", + ) + + # Input hashes provided by user + input_hashes = { + "second-video": "QmS4885aRikrjDB4yHPg9yTiPcBFWadZKVfAEvUy7B32zS" + } + + # Verify source CID resolution works + resolved_cid = resolve_source_cid(source_step.config, input_hashes) + assert resolved_cid == "QmS4885aRikrjDB4yHPg9yTiPcBFWadZKVfAEvUy7B32zS" + + def test_recipe_with_fixed_and_user_sources(self): + """ + Recipe with both fixed (asset) and user-input sources. + + This is the dog-invert-concat recipe pattern: + - Fixed source: cat asset with known CID + - User input: Second Video from input_hashes + """ + fixed_source = MockStep( + step_id="source_cat", + node_type="SOURCE", + config={"cid": "QmCatVideo123", "asset": "cat"}, + cache_id="cat_cache", + level=0, + ) + + user_source = MockStep( + step_id="source_user", + node_type="SOURCE", + config={"input": True, "name": "Second Video"}, + cache_id="user_cache", + level=0, + ) + + input_hashes = { + "second-video": "QmUserProvidedVideo456" + } + + # Fixed source uses its cid + fixed_cid = resolve_source_cid(fixed_source.config, input_hashes) + assert fixed_cid == "QmCatVideo123" + + # User source resolves from input_hashes + user_cid = resolve_source_cid(user_source.config, input_hashes) + assert user_cid == "QmUserProvidedVideo456"