Skip to content

fix(workflows): coerce numeric strings for == and != like ordering operators - #5310

Merged
mnriem merged 1 commit into
github:mainfrom
patrickballard:fix/5072-equality-numeric-coercion
Nov 9, 2026
Merged

mnriem merged 1 commit into
github:mainfrom
patrickballard:fix/5072-equality-numeric-coercion

Conversation

@patrickballard

Copy link
Copy Markdown
Contributor

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 >= 10 and <= 10, yet not == 10.

This PR moves the existing "coerce only when both sides look numeric" step out of _safe_compare into a small _coerce_numeric_pair helper, and routes == and != through a new _safe_equals that uses the same helper. Behavior for non-numeric strings is unchanged: "10x" == 10 is 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

  • Added test_equality_coerces_numeric_strings_like_ordering in tests/test_workflows.py, using the reproduction from the issue plus numeric-string pairs and non-numeric strings.
  • Checked operators.py in isolation (it is a leaf module) against the issue's cases and existing ordering cases.
  • Ran the full suite with python -m pytest tests -q -n auto (LC_ALL=en_US.UTF-8, macOS, Python via uv sync --extra test): 13953 passed, 446 skipped, 5 failed. The same 5 fail on unmodified main in this environment (test_non_windows_uv_tool_dry_run_still_previews_installer and four test_fetch_keeps_malicious_text_in_one_argument_even_if_validation_is_bypassed cases), so they are unrelated to this change.
  • The new test fails on main and passes with this change.

AI Disclosure

  • I did not use AI assistance for this contribution
  • I did use AI assistance (fill in the disclosure below)

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

…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 <[email protected]>
Copilot AI dynamic review requested due to automatic review settings November 8, 2026 03:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 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)

@mnriem mnriem added the triage-must-have Verdict: high-value, important work for Spec Kit — do first label Nov 9, 2026
@mnriem
mnriem requested a dynamic review from Copilot November 9, 2026 13:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues were reported, although tests were not independently run during this review.

No open issues 🎉

Review effort: Balanced (auto)

@mnriem
mnriem merged commit d75f928 into github:main Nov 9, 2026
19 checks passed
@mnriem

mnriem commented Nov 9, 2026

Copy link
Copy Markdown
Collaborator

Thank you!

kevinnguyenhoang91 pushed a commit to kevinnguyenhoang91/spec-kit that referenced this pull request Nov 10, 2026
…erators (github#5310)

`<`, `>`, `<=` 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 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

triage-must-have Verdict: high-value, important work for Spec Kit — do first

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: == and != do not compare numeric strings as numbers, while <, >, <= and >= do

3 participants