Repository navigation
fix(workflows): coerce numeric strings for == and != like ordering operators - #5310
Open
patrickballard wants to merge 1 commit into
Open
patrickballard wants to merge 1 commit into
patrickballard wants to merge 1 commit into
Conversation
…erators `<`, `>`, `<=` and `>=` compare two numeric-looking operands as numbers, but `==` and `!=` compared raw values, so captured stdout "10" was both `>= 10` and `<= 10` yet not `== 10`. Route equality through the same both-sides-numeric coercion as `_safe_compare`. Fixes github#5072 Assisted-by: Claude Code (model: Claude Opus 5.5, autonomous) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues were identified, although tests were not run during this review.
No open issues 🎉
What changed in this PR
Fixes #5072 by making workflow equality operators use the same numeric-string coercion as ordering operators.
Changes:
- Extracts shared numeric-pair coercion and adds an equality helper.
- Routes
==and!=through the new helper. - Adds regression coverage for captured stdout, numeric-string pairs, and non-numeric strings.
The PR reports before-and-after regression evidence; tests were not run during this review.
| File | Description |
|---|---|
tests/test_workflows.py |
Adds equality-coercion regression tests. |
src/specify_cli/workflows/expressions/operators.py |
Shares numeric coercion between equality and ordering. |
src/specify_cli/workflows/expressions/engine.py |
Uses the equality helper for == and !=. |
Review effort: Balanced (auto)
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #5072.
The ordering operators (
<,>,<=,>=) compare two numeric-looking operands as numbers through_safe_compare, but==and!=compared the raw values. A numeric string such as captured stdout"10"was therefore both>= 10and<= 10, yet not== 10.This PR moves the existing "coerce only when both sides look numeric" step out of
_safe_compareinto a small_coerce_numeric_pairhelper, and routes==and!=through a new_safe_equalsthat uses the same helper. Behavior for non-numeric strings is unchanged:"10x" == 10is still false and"10x" == "10x"is still true.One consequence worth calling out: two numeric strings now compare as numbers for equality too (
"10" == "10.0"is true), matching how"10" > "9"already compares numerically.Testing
test_equality_coerces_numeric_strings_like_orderingintests/test_workflows.py, using the reproduction from the issue plus numeric-string pairs and non-numeric strings.operators.pyin isolation (it is a leaf module) against the issue's cases and existing ordering cases.python -m pytest tests -q -n auto(LC_ALL=en_US.UTF-8, macOS, Python viauv sync --extra test): 13953 passed, 446 skipped, 5 failed. The same 5 fail on unmodifiedmainin this environment (test_non_windows_uv_tool_dry_run_still_previews_installerand fourtest_fetch_keeps_malicious_text_in_one_argument_even_if_validation_is_bypassedcases), so they are unrelated to this change.mainand passes with this change.AI Disclosure
AI disclosure: Implemented with Claude Code using Claude Opus 5.5 in autonomous agent mode, on behalf of @patrickballard: issue selection, code, tests and this PR description were AI-generated. The commit carries an
Assisted-by:trailer.🤖 Generated with Claude Code