From be4d0da84f69b8982346f57ed371693031b1ec5b Mon Sep 17 00:00:00 2001 From: gilesb Date: Tue, 13 Jan 2026 01:58:34 +0000 Subject: [PATCH] Fix case sensitivity bug: S-expression node types are lowercase Bug: S-expression plans produce lowercase node types (source, compound) but code was checking uppercase (SOURCE, COMPOUND). Fix: Use .upper() for node type comparisons. Add TestNodeTypeCaseSensitivity tests to catch this regression. Co-Authored-By: Claude Opus 4.5 --- legacy_tasks.py | 4 +-- tests/test_execute_recipe.py | 55 ++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/legacy_tasks.py b/legacy_tasks.py index 377e2d6..8040402 100644 --- a/legacy_tasks.py +++ b/legacy_tasks.py @@ -732,7 +732,7 @@ def execute_recipe(self, recipe_sexp: str, input_hashes: Dict[str, str], run_id: input_paths.append(Path(input_path)) # Handle SOURCE nodes - if step.node_type == "SOURCE": + if step.node_type.upper() == "SOURCE": source_cid = step.config.get("cid") # If source has :input true, resolve CID from input_hashes @@ -772,7 +772,7 @@ def execute_recipe(self, recipe_sexp: str, input_hashes: Dict[str, str], run_id: raise ValueError(f"SOURCE step has no cid and no :input flag: {step.config}") # Handle COMPOUND nodes (collapsed effect chains) - if step.node_type == "COMPOUND": + if step.node_type.upper() == "COMPOUND": import subprocess import tempfile diff --git a/tests/test_execute_recipe.py b/tests/test_execute_recipe.py index 23e4b06..0a32326 100644 --- a/tests/test_execute_recipe.py +++ b/tests/test_execute_recipe.py @@ -393,6 +393,61 @@ class TestPlanStepTypes: assert step.node_type == "SEQUENCE" +class TestNodeTypeCaseSensitivity: + """ + Tests for node type case handling. + + Bug fixed: S-expression plans use lowercase (source, compound, effect) + but code was checking uppercase (SOURCE, COMPOUND, EFFECT). + """ + + def test_source_lowercase_from_sexp(self): + """S-expression plans produce lowercase node types.""" + # From plan: (source :cid "Qm...") + step = MockStep("s1", "source", {"cid": "Qm123"}, "cache1") + + # Code should handle lowercase + assert step.node_type.upper() == "SOURCE" + + def test_compound_lowercase_from_sexp(self): + """COMPOUND from S-expression is lowercase.""" + # From plan: (compound :filter_chain ...) + step = MockStep("c1", "compound", {"filter_chain": []}, "cache2") + + assert step.node_type.upper() == "COMPOUND" + + def test_effect_lowercase_from_sexp(self): + """EFFECT from S-expression is lowercase.""" + # From plan: (effect :effect "invert" ...) + step = MockStep("e1", "effect", {"effect": "invert"}, "cache3") + + assert step.node_type.upper() == "EFFECT" + + def test_sequence_lowercase_from_sexp(self): + """SEQUENCE from S-expression is lowercase.""" + # From plan: (sequence :transition ...) + step = MockStep("seq1", "sequence", {"transition": {}}, "cache4") + + assert step.node_type.upper() == "SEQUENCE" + + def test_node_type_comparison_must_be_case_insensitive(self): + """ + Node type comparisons must be case-insensitive. + + This is the actual bug - checking step.node_type == "SOURCE" + fails when step.node_type is "source" from S-expression. + """ + sexp_types = ["source", "compound", "effect", "sequence"] + code_types = ["SOURCE", "COMPOUND", "EFFECT", "SEQUENCE"] + + for sexp, code in zip(sexp_types, code_types): + # Wrong: direct comparison fails + assert sexp != code, f"{sexp} should not equal {code}" + + # Right: case-insensitive comparison works + assert sexp.upper() == code, f"{sexp}.upper() should equal {code}" + + class TestExecuteRecipeErrorHandling: """Tests for error handling in execute_recipe."""