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#3448Quratulain-bilal wants to merge 2 commits into
Conversation
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.
There was a problem hiding this comment.
Pull request overview
This PR fixes workflow expression evaluation so the in / not in operators no longer crash workflow runs when the right-hand operand is a non-iterable (e.g., int, bool, None). It aligns membership operator behavior with existing _safe_compare behavior by swallowing TypeError and producing a deterministic boolean result.
Changes:
- Route
in/not inthrough a new_safe_membership()helper that catchesTypeErrorand treats membership as falsey (not inas truthy). - Add a regression test to ensure non-iterable membership does not raise and that normal iterable membership still works.
Show a summary per file
| File | Description |
|---|---|
src/specify_cli/workflows/expressions.py |
Adds _safe_membership() and uses it for in / not in in _evaluate_simple_expression() to prevent raw TypeError crashes. |
tests/test_workflows.py |
Adds regression coverage for membership tests against non-iterable right operands and confirms valid membership still behaves correctly. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Low
| ctx = StepContext(inputs={"tag": "x", "count": 5, "flag": True}) | ||
| assert evaluate_expression("{{ inputs.tag in inputs.count }}", ctx) is False | ||
| assert evaluate_expression("{{ inputs.tag not in inputs.count }}", ctx) is True | ||
| assert evaluate_expression("{{ 'a' in inputs.flag }}", ctx) is False | ||
| assert evaluate_expression("{{ inputs.tag in inputs.missing }}", ctx) is False |
There was a problem hiding this comment.
fixed in 2178fb3. added a float right-operand assertion ({{ 'a' in inputs.ratio }}) so the test now matches its comment instead of just claiming float coverage.
note: i used an ai assistant to help investigate and write this up.
| A non-iterable right operand (``None``, an int, a bool, ...) makes ``in`` | ||
| raise ``TypeError`` in Python. Nothing is contained in such a value, so | ||
| treat membership as ``False`` (``not in`` as ``True``) rather than leaking | ||
| the error out of the evaluator and crashing the whole workflow. Mirrors the | ||
| graceful ``TypeError`` handling in ``_safe_compare`` for the ordering | ||
| operators, and generalizes the previous ``right is not None`` guard to | ||
| every non-iterable right operand. |
There was a problem hiding this comment.
good point, reworded in 2178fb3. the docstring now says left in right raises TypeError whenever the operands don't support membership testing - non-iterable right being the common case, but also e.g. an unhashable left against a set - rather than implying only the right operand matters.
note: i used an ai assistant to help investigate and write this up.
…tring - 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.
fixes #3447
the
in/not inoperators in_evaluate_simple_expressiononly guardedright is not None, soleft in rightstill raised a rawTypeErrorwhen the right operand was any other non-iterable (int, bool, float). a workflow condition like{{ inputs.tag in inputs.count }}wherecountis a number crashed the whole run instead of evaluating. this was asymmetric with_safe_compare, which already swallows TypeError for the ordering operators.fix: route both operators through a new
_safe_membershiphelper that doesleft in rightinside atry/except TypeErrorand returns False (not inreturns True) on failure. nothing is contained in a non-iterable, so this generalizes the old None guard and matches_safe_compare's handling.added a regression test (
test_membership_against_non_iterable_is_false_not_error) covering int/bool/missing right operands plus a condition that would otherwise crash the run. i confirmed it fails on the pre-fix code by stashing the source and re-running (raw TypeError). genuine list/substring membership still works, and the full expression test class (42 tests) passes.