Skip to content

Commit 86d769b

Browse files
fix(workflows): don't crash on membership test against a non-iterable (#3448)
* fix(workflows): don't crash on membership test against a non-iterable the `in` / `not in` operators in _evaluate_simple_expression only guarded `right is not None`, so `left in right` still raised a raw TypeError when the right operand was any other non-iterable (int, bool, float). a condition like `{{ inputs.tag in inputs.count }}` where count is a number crashed the whole workflow run instead of evaluating. nothing is contained in a non-iterable, so treat membership as False (`not in` as True) via a new _safe_membership helper that swallows TypeError. this generalizes the old None guard and mirrors _safe_compare, which already catches TypeError for the ordering operators. added a regression test; confirmed it fails on the pre-fix code (raw TypeError) and that genuine list/substring membership still works. * address review: float membership case + broaden _safe_membership docstring - add a float right-operand assertion so the test matches its comment (was claiming float coverage while only exercising int/bool/None). - reword the _safe_membership docstring to describe TypeError generally (non-iterable right is the common case, but also e.g. an unhashable left against a set) rather than implying only the right operand matters.
1 parent 55c6612 commit 86d769b

2 files changed

Lines changed: 50 additions & 2 deletions

File tree

src/specify_cli/workflows/expressions.py

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -464,9 +464,9 @@ def _evaluate_simple_expression(expr: str, namespace: dict[str, Any]) -> Any:
464464
if op == "<=":
465465
return _safe_compare(left, right, "<=")
466466
if op == " in ":
467-
return left in right if right is not None else False
467+
return _safe_membership(left, right, negate=False)
468468
if op == " not in ":
469-
return left not in right if right is not None else True
469+
return _safe_membership(left, right, negate=True)
470470

471471
# Numeric literal
472472
try:
@@ -511,6 +511,26 @@ def _coerce_number(value: Any) -> Any:
511511
return value
512512

513513

514+
def _safe_membership(left: Any, right: Any, *, negate: bool) -> bool:
515+
"""Safely evaluate ``left in right`` (or ``not in``) without crashing.
516+
517+
``left in right`` raises ``TypeError`` whenever the operands don't support
518+
membership testing — most commonly a non-iterable right operand (``None``,
519+
an int, a bool), but also cases like an unhashable ``left`` against a set.
520+
In every such case the membership relation is undefined, so treat it as
521+
``False`` (``not in`` as ``True``) rather than leaking the error out of the
522+
evaluator and crashing the whole workflow. Mirrors the graceful
523+
``TypeError`` handling in ``_safe_compare`` for the ordering operators, and
524+
generalizes the previous ``right is not None`` guard to any operand pair
525+
that can't be membership-tested.
526+
"""
527+
try:
528+
contained = left in right
529+
except TypeError:
530+
contained = False
531+
return not contained if negate else contained
532+
533+
514534
def _safe_compare(left: Any, right: Any, op: str) -> bool:
515535
"""Compare two values for ordering, coercing numeric strings when possible.
516536

tests/test_workflows.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,34 @@ def test_pipe_detection_is_quote_aware(self):
460460
assert evaluate_expression("{{ inputs.s | contains('ab') }}", ctx2) is True
461461
assert evaluate_expression("{{ inputs.missing | default('a|b') }}", ctx2) == "a|b"
462462

463+
def test_membership_against_non_iterable_is_false_not_error(self):
464+
from specify_cli.workflows.expressions import (
465+
evaluate_condition,
466+
evaluate_expression,
467+
)
468+
from specify_cli.workflows.base import StepContext
469+
470+
# A non-iterable right operand (int, bool, None, float) makes a raw
471+
# `x in y` raise TypeError in Python. The evaluator must treat it as
472+
# "not contained" (False, and `not in` as True) instead of leaking the
473+
# TypeError and crashing the whole workflow run. This generalizes the
474+
# previous `right is not None` guard and mirrors _safe_compare, which
475+
# already swallows TypeError for the ordering operators.
476+
ctx = StepContext(inputs={"tag": "x", "count": 5, "ratio": 1.5, "flag": True})
477+
assert evaluate_expression("{{ inputs.tag in inputs.count }}", ctx) is False
478+
assert evaluate_expression("{{ inputs.tag not in inputs.count }}", ctx) is True
479+
assert evaluate_expression("{{ 'a' in inputs.ratio }}", ctx) is False
480+
assert evaluate_expression("{{ 'a' in inputs.flag }}", ctx) is False
481+
assert evaluate_expression("{{ inputs.tag in inputs.missing }}", ctx) is False
482+
# A condition that would otherwise crash the run now evaluates cleanly.
483+
assert evaluate_condition("{{ inputs.tag in inputs.count }}", ctx) is False
484+
485+
# Regression: genuine membership over a real iterable still works.
486+
ok = StepContext(inputs={"items": ["x", "y"], "s": "xyz"})
487+
assert evaluate_expression("{{ 'x' in inputs.items }}", ok) is True
488+
assert evaluate_expression("{{ 'z' not in inputs.items }}", ok) is True
489+
assert evaluate_expression("{{ 'y' in inputs.s }}", ok) is True
490+
463491
def test_filter_default(self):
464492
from specify_cli.workflows.expressions import evaluate_expression
465493
from specify_cli.workflows.base import StepContext

0 commit comments

Comments
 (0)