diff --git a/api/core/workflow/graph_normalizers.py b/api/core/workflow/graph_normalizers.py index 19f936a499d..1864730dec2 100644 --- a/api/core/workflow/graph_normalizers.py +++ b/api/core/workflow/graph_normalizers.py @@ -16,16 +16,20 @@ mutates them in place, returning what it changed so callers can log it. from __future__ import annotations -from collections.abc import Mapping, MutableMapping +from collections.abc import Callable, Mapping, MutableMapping from typing import Any # --------------------------------------------------------------------------- -# Condition values (if-else, loop break conditions, list-operator filters) +# Condition values (if-else/loop's Condition vs list-operator's FilterCondition) # --------------------------------------------------------------------------- -# graphon ``Condition.value`` is ``str | Sequence[str] | bool | None``. The -# operators below compare against a LIST, so a scalar written for them must -# become a one-item list rather than a bare string. +# graphon ``Condition.value`` (if-else, loop) is ``str | Sequence[str] | bool | None``. +# graphon ``FilterCondition.value`` (list-operator) is ``str | Sequence[str] | bool`` +# (no None; default ""). For list operators filtering arrays, graphon requires a plain +# string, not a list. The two need separate normalization rules. + +# if-else/loop conditions: operators below compare against a LIST, so a scalar +# written for them must become a one-item list rather than a bare string. _LIST_OPERATORS = frozenset({"in", "not in", "all of"}) @@ -36,8 +40,17 @@ def _scalar_to_text(value: int | float) -> str: return str(value) +def _normalize_list_item(item: Any) -> str: + """Normalize a single item in a condition value list.""" + if isinstance(item, str): + return item + if isinstance(item, (int, float)): + return _scalar_to_text(item) + return str(item) + + def normalize_condition_value(value: Any, operator: str) -> Any: - """One condition ``value`` as graphon will accept it. + """One condition ``value`` (if-else/loop Condition) as graphon will accept it. ESQ1-303: the node builder wrote ``"value": 60`` (a JSON number). pydantic rejects a number for ``str | Sequence[str] | bool | None`` -- except that it @@ -47,7 +60,7 @@ def normalize_condition_value(value: Any, operator: str) -> Any: if isinstance(value, bool) or value is None: return value if isinstance(value, list): - return [item if isinstance(item, str) else str(item) for item in value] + return [_normalize_list_item(item) for item in value] if isinstance(value, (int, float)): text = _scalar_to_text(value) return [text] if operator in _LIST_OPERATORS else text @@ -56,9 +69,37 @@ def normalize_condition_value(value: Any, operator: str) -> Any: return value -def _normalize_conditions(conditions: Any) -> bool: +def normalize_filter_condition_value(value: Any) -> Any: + """One filter condition ``value`` (list-operator FilterCondition) normalized. + + FilterCondition.value is str | Sequence[str] | bool (default ""), NOT None. + Filtering an array of strings or numbers requires a plain string, not a list. + Booleans are kept. Numbers become text. Lists get their items normalized. + """ + if isinstance(value, bool): + return value + if value is None: + return "" + if isinstance(value, (int, float)): + return _scalar_to_text(value) + if isinstance(value, str): + return value + if isinstance(value, (list, tuple)): + return [_normalize_list_item(item) for item in value] + return value + + +def _normalize_conditions( + conditions: Any, + value_normalizer: Callable[[Any, str], Any] | Callable[[Any], Any] = normalize_condition_value, +) -> bool: """Normalize every ``value`` in a conditions list (recursing into - ``sub_variable_condition``). Returns True when anything changed.""" + ``sub_variable_condition``). Returns True when anything changed. + + The value_normalizer is called with (value, operator) for Condition-like + normalizers, or just (value,) for FilterCondition normalizers. Callers + must pass the right function for their context. + """ changed = False if not isinstance(conditions, list): return False @@ -66,12 +107,17 @@ def _normalize_conditions(conditions: Any) -> bool: if not isinstance(condition, MutableMapping): continue if "value" in condition: - new_value = normalize_condition_value(condition["value"], str(condition.get("comparison_operator") or "")) + # Try the normalizer with both signatures (Condition vs FilterCondition) + try: + new_value = value_normalizer(condition["value"], str(condition.get("comparison_operator") or "")) # type: ignore + except TypeError: + # If that fails, try with just the value (FilterCondition path) + new_value = value_normalizer(condition["value"]) # type: ignore if new_value != condition["value"] or type(new_value) is not type(condition["value"]): condition["value"] = new_value changed = True sub = condition.get("sub_variable_condition") - if isinstance(sub, MutableMapping) and _normalize_conditions(sub.get("conditions")): + if isinstance(sub, MutableMapping) and _normalize_conditions(sub.get("conditions"), value_normalizer): changed = True return changed @@ -80,8 +126,9 @@ def normalize_condition_values(nodes: list[Any]) -> list[str]: """Coerce every condition value in ``nodes`` to the shape graphon accepts. Covers if-else ``cases[].conditions[]`` (and the legacy top-level - ``conditions``), loop ``break_conditions[]`` and list-operator - ``filter_by.conditions[]`` -- all three use graphon's ``Condition`` model. + ``conditions``), loop ``break_conditions[]``, and list-operator + ``filter_by.conditions[]``. if-else and loop use graphon's ``Condition`` + model; list-operator uses ``FilterCondition`` (different value shape). Returns the ids of the nodes that changed, in order. """ changed: list[str] = [] @@ -97,16 +144,18 @@ def normalize_condition_values(nodes: list[Any]) -> list[str]: cases = data.get("cases") if isinstance(cases, list): for case in cases: - if isinstance(case, Mapping) and _normalize_conditions(case.get("conditions")): + if isinstance(case, Mapping) and _normalize_conditions( + case.get("conditions"), normalize_condition_value + ): touched = True - if _normalize_conditions(data.get("conditions")): + if _normalize_conditions(data.get("conditions"), normalize_condition_value): touched = True elif node_type == "loop": - touched = _normalize_conditions(data.get("break_conditions")) + touched = _normalize_conditions(data.get("break_conditions"), normalize_condition_value) elif node_type == "list-operator": filter_by = data.get("filter_by") if isinstance(filter_by, Mapping): - touched = _normalize_conditions(filter_by.get("conditions")) + touched = _normalize_conditions(filter_by.get("conditions"), normalize_filter_condition_value) if touched: changed.append(str(node.get("id") or "")) return changed diff --git a/api/tests/unit_tests/core/workflow/test_graph_normalizers.py b/api/tests/unit_tests/core/workflow/test_graph_normalizers.py index ffb3ca964e8..334073a311e 100644 --- a/api/tests/unit_tests/core/workflow/test_graph_normalizers.py +++ b/api/tests/unit_tests/core/workflow/test_graph_normalizers.py @@ -9,7 +9,11 @@ import copy import json from pathlib import Path -from core.workflow.graph_normalizers import normalize_condition_value, normalize_condition_values +from core.workflow.graph_normalizers import ( + normalize_condition_value, + normalize_condition_values, + normalize_filter_condition_value, +) _FIXTURES = Path(__file__).parent / "fixtures" @@ -49,6 +53,32 @@ class TestNormalizeConditionValue: assert normalize_condition_value(3, "all of") == ["3"] assert normalize_condition_value("x", "in") == ["x"] + def test_list_items_use_integral_float_rule(self): + # integral floats in lists should become strings without decimal point + assert normalize_condition_value([1.0, 2.5], "=") == ["1", "2.5"] + + +class TestNormalizeFilterConditionValue: + def test_none_becomes_empty_string(self): + assert normalize_filter_condition_value(None) == "" + + def test_bool_is_preserved(self): + assert normalize_filter_condition_value(True) is True + assert normalize_filter_condition_value(False) is False + + def test_numbers_become_strings_not_lists(self): + assert normalize_filter_condition_value(7) == "7" + assert normalize_filter_condition_value(7.0) == "7" + assert normalize_filter_condition_value(7.5) == "7.5" + + def test_strings_pass_through(self): + assert normalize_filter_condition_value("value") == "value" + + def test_list_items_normalized_no_wrapping(self): + # Unlike Condition, we don't wrap in a list—we keep the list but normalize items + assert normalize_filter_condition_value(["a", 2]) == ["a", "2"] + assert normalize_filter_condition_value([1.0, "b"]) == ["1", "b"] + class TestNormalizeConditionValues: def test_heals_the_esq1_303_dev_draft_and_names_the_node(self): @@ -116,7 +146,7 @@ class TestNormalizeConditionValues: sub = nodes[0]["data"]["cases"][0]["conditions"][0]["sub_variable_condition"]["conditions"][0] assert sub["value"] == "1024" assert nodes[1]["data"]["break_conditions"][0]["value"] == "10" - assert nodes[2]["data"]["filter_by"]["conditions"][0]["value"] == ["7"] + assert nodes[2]["data"]["filter_by"]["conditions"][0]["value"] == "7" assert nodes[3]["data"]["conditions"][0]["value"] == "2" def test_other_node_types_and_malformed_shapes_are_left_alone(self): @@ -130,3 +160,49 @@ class TestNormalizeConditionValues: assert normalize_condition_values(nodes) == [] assert nodes == before + + def test_list_operator_with_none_bool_and_lists(self): + """List-operator filters follow FilterCondition (None → "", no wrapping).""" + nodes = [ + { + "id": "filter_none", + "data": { + "type": "list-operator", + "filter_by": { + "enabled": True, + "conditions": [{"key": "x", "comparison_operator": "empty", "value": None}], + }, + }, + }, + { + "id": "filter_bool", + "data": { + "type": "list-operator", + "filter_by": { + "enabled": True, + "conditions": [{"key": "x", "comparison_operator": "is", "value": True}], + }, + }, + }, + { + "id": "filter_list", + "data": { + "type": "list-operator", + "filter_by": { + "enabled": True, + "conditions": [{"key": "x", "comparison_operator": "in", "value": ["a", 2]}], + }, + }, + }, + ] + + changed = normalize_condition_values(nodes) + + # filter_none changed (None → ""), filter_list changed (items normalized) + # filter_bool unchanged (True stays True) + assert "filter_none" in changed + assert "filter_list" in changed + assert "filter_bool" not in changed + assert nodes[0]["data"]["filter_by"]["conditions"][0]["value"] == "" + assert nodes[1]["data"]["filter_by"]["conditions"][0]["value"] is True + assert nodes[2]["data"]["filter_by"]["conditions"][0]["value"] == ["a", "2"]