From 575e428367106e74a766400596d53c0cb2eb848a Mon Sep 17 00:00:00 2001 From: Quratulain-bilal Date: Fri, 3 Jul 2026 16:49:33 +0500 Subject: [PATCH] fix(workflows): compare non-numeric strings lexicographically instead of returning False _safe_compare coerced both operands to int/float unconditionally for <, >, <=, >=. any non-numeric string (an iso date, a version tag, a name) failed that coercion and the whole comparison silently returned False -- so `{{ inputs.d < '2026-02-01' }}` was False even when the date was earlier. only coerce when both operands look numeric; otherwise compare the original values, so two strings order lexicographically the way python does and two numeric strings still compare as numbers ("10" > "9"). a number vs a non-numeric string stays incomparable and yields False. add a regression test covering dates, plain strings, numeric strings, and the number-vs-string case. --- src/specify_cli/workflows/expressions.py | 35 ++++++++++++++++++------ tests/test_workflows.py | 29 ++++++++++++++++++++ 2 files changed, 56 insertions(+), 8 deletions(-) diff --git a/src/specify_cli/workflows/expressions.py b/src/specify_cli/workflows/expressions.py index cc63be523c..f570f054c9 100644 --- a/src/specify_cli/workflows/expressions.py +++ b/src/specify_cli/workflows/expressions.py @@ -408,15 +408,34 @@ def _evaluate_simple_expression(expr: str, namespace: dict[str, Any]) -> Any: return _resolve_dot_path(namespace, expr) +def _coerce_number(value: Any) -> Any: + """Return *value* as int/float if it is a numeric string, else unchanged.""" + if isinstance(value, str): + try: + return float(value) if "." in value else int(value) + except ValueError: + return value + return value + + def _safe_compare(left: Any, right: Any, op: str) -> bool: - """Safely compare two values, coercing types when possible.""" - try: - if isinstance(left, str): - left = float(left) if "." in left else int(left) - if isinstance(right, str): - right = float(right) if "." in right else int(right) - except (ValueError, TypeError): - return False + """Compare two values for ordering, coercing numeric strings when possible. + + Numeric coercion is applied only when *both* operands look numeric, so a + pair like ``"10"`` and ``"9"`` compares as numbers (10 > 9). When either + side is a non-numeric string, both fall back to their original values and + are compared directly -- so ordinary strings (dates, semver-ish tags, + names) compare lexicographically the way Python does, instead of every + such comparison silently returning ``False`` after a failed int()/float() + coercion. A genuinely incomparable pair (e.g. number vs non-numeric string) + raises ``TypeError`` and yields ``False``. + """ + cl, cr = _coerce_number(left), _coerce_number(right) + # Only use the coerced numbers when both converted; otherwise a numeric + # string paired with a plain string would become an int-vs-str mismatch + # (always False) rather than a lexicographic string comparison. + if isinstance(cl, (int, float)) and isinstance(cr, (int, float)): + left, right = cl, cr try: if op == ">": return left > right # type: ignore[operator] diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 2fdbf887b3..3e3c4df1ec 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -288,6 +288,35 @@ def test_numeric_comparison(self): assert evaluate_expression("{{ steps.plan.output.task_count > 5 }}", ctx) is True assert evaluate_expression("{{ steps.plan.output.task_count < 5 }}", ctx) is False + def test_ordering_comparison_of_non_numeric_strings(self): + """`<`/`>`/`<=`/`>=` between non-numeric strings must compare + lexicographically, not silently return False. + + `_safe_compare` used to coerce both operands to int/float unconditionally; + a non-numeric string (date, version tag, name) failed that coercion and + the whole comparison returned False. Ordinary strings should order the + way Python does; numeric strings must still compare as numbers.""" + from specify_cli.workflows.expressions import evaluate_expression + from specify_cli.workflows.base import StepContext + + # ISO dates compare lexicographically (correct chronological order). + ctx = StepContext(inputs={"d": "2026-01-01"}) + assert evaluate_expression("{{ inputs.d < '2026-02-01' }}", ctx) is True + assert evaluate_expression("{{ inputs.d > '2026-02-01' }}", ctx) is False + + # Plain string ordering. + ctx = StepContext(inputs={"name": "beta"}) + assert evaluate_expression("{{ inputs.name > 'alpha' }}", ctx) is True + + # Two numeric strings still compare numerically, not lexically + # ("10" > "9" is True as numbers; as strings it would be False). + ctx = StepContext(inputs={"v": "10"}) + assert evaluate_expression("{{ inputs.v > '9' }}", ctx) is True + + # A number vs a non-numeric string is genuinely incomparable -> False. + ctx = StepContext(inputs={"n": 5}) + assert evaluate_expression("{{ inputs.n > 'abc' }}", ctx) is False + def test_boolean_and(self): from specify_cli.workflows.expressions import evaluate_expression from specify_cli.workflows.base import StepContext