Skip to content

fix(vars): compile % with Python's sign-of-divisor semantics - #7638

Open
Enesuygurs wants to merge 2 commits into
reflex-dev:mainfrom
Enesuygurs:fix/var-modulo-python-semantics
Open

Enesuygurs wants to merge 2 commits into
reflex-dev:mainfrom
Enesuygurs:fix/var-modulo-python-semantics

Conversation

@Enesuygurs

@Enesuygurs Enesuygurs commented Oct 22, 2026 •

Copy link
Copy Markdown

All Submissions:

  • Have you followed the guidelines stated in CONTRIBUTING.md file?
  • Have you checked to ensure there aren't any other open Pull Requests for the desired changed?

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Changes To Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully ran tests with your changes locally?

Description

number_modulo_operation compiles a % b to JavaScript's %, which takes the sign of the dividend. Python's % takes the sign of the divisor. So a modulo that is correct in Python gives a different answer on the frontend once an operand is negative:

  • -7 % 3 is 2 in Python but renders as (-7 % 3), which is -1 in the browser.
  • 5.5 % -2 is -0.5 in Python but 1.5 in JS.

Floor division already follows Python (Math.floor(a / b)), so x // y * y + x % y doesn't give back x for negative values. The usual wrap-around pattern (State.index - 1) % State.count also yields -1 instead of the last index.

The operation now compiles to a small arrow function that applies CPython's float-modulo rule: when the remainder is nonzero and its sign differs from the divisor's, the divisor is added. Each operand is still evaluated once, and no new import is needed.

I ran the compiled expressions through Node and compared them with Python's %. All of these match: (-7, 3), (7, -3), (-7, -3), (5.5, -2), (-5.5, 2), (0, 5), (-6, 3), (0.3, 0.1), and ±Infinity divisors.

test_modulo_takes_the_sign_of_the_divisor fails on main with '(-7 % 3)' == '((a, b) => {... r; })(-7, 3)'. The three existing test_var.py assertions that pinned the old (a % b) output are updated. The PR also includes a bugfix news fragment for reflex-base. ruff and pyright are clean on the changed files.

View guided diff

@Enesuygurs
Enesuygurs requested a review from a team as a code owner October 22, 2026 18:34

@cubic-dev-ai cubic-dev-ai Bot 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.

2 issues found across 4 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="tests/units/reflex_base/vars/test_number.py">

<violation number="1" location="tests/units/reflex_base/vars/test_number.py:24">
P3: The PR description says the modulo tests cover zero and infinite operands, but this test has neither. Add assertions for a zero-remainder case such as `-6 % 3` and an infinite divisor such as `x % float("inf")`, or correct the description.</violation>
</file>

<file name="packages/reflex-base/src/reflex_base/vars/number.py">

<violation number="1" location="packages/reflex-base/src/reflex_base/vars/number.py:651">
P2: The generated modulo still preserves the dividend's signed zero, so `0.0 % -3.0` returns `+0` instead of Python's `-0.0` (and `-0.0 % 3.0` stays `-0`). Handle `r === 0` by returning zero with the divisor's sign before applying the nonzero remainder adjustment.</violation>
</file>

Reply to a comment to ask cubic a question or push back. It learns from your replies.

View guided diff | Re-trigger cubic

# JavaScript's `%` takes the sign of the dividend (`-7 % 3` is -1) where
# Python's takes the sign of the divisor (`-7 % 3` is 2), so a nonzero
# remainder whose sign differs from the divisor's is shifted by the divisor.
return f"((a, b) => {{ const r = a % b; return r !== 0 && (r < 0) !== (b < 0) ? r + b : r; }})({lhs!s}, {rhs!s})"

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.

P2: The generated modulo still preserves the dividend's signed zero, so 0.0 % -3.0 returns +0 instead of Python's -0.0 (and -0.0 % 3.0 stays -0). Handle r === 0 by returning zero with the divisor's sign before applying the nonzero remainder adjustment.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/reflex-base/src/reflex_base/vars/number.py, line 651:

<comment>The generated modulo still preserves the dividend's signed zero, so `0.0 % -3.0` returns `+0` instead of Python's `-0.0` (and `-0.0 % 3.0` stays `-0`). Handle `r === 0` by returning zero with the divisor's sign before applying the nonzero remainder adjustment.</comment>

<file context>
@@ -645,7 +645,10 @@ def number_modulo_operation(lhs: NumberVar, rhs: NumberVar):
+    # JavaScript's `%` takes the sign of the dividend (`-7 % 3` is -1) where
+    # Python's takes the sign of the divisor (`-7 % 3` is 2), so a nonzero
+    # remainder whose sign differs from the divisor's is shifted by the divisor.
+    return f"((a, b) => {{ const r = a % b; return r !== 0 && (r < 0) !== (b < 0) ? r + b : r; }})({lhs!s}, {rhs!s})"
 
 
</file context>
Suggested change
return f"((a, b) => {{ const r = a % b; return r !== 0 && (r < 0) !== (b < 0) ? r + b : r; }})({lhs!s}, {rhs!s})"
return f"((a, b) => {{ const r = a % b; return r === 0 ? (b < 0 || Object.is(b, -0) ? -0 : 0) : ((r < 0) !== (b < 0) ? r + b : r); }})({lhs!s}, {rhs!s})"

py_mod = "((a, b) => { const r = a % b; return r !== 0 && (r < 0) !== (b < 0) ? r + b : r; })"
assert str(LiteralNumberVar.create(-7) % 3) == f"{py_mod}(-7, 3)"
assert str(x % -2.5) == f"{py_mod}(x, -2.5)"
assert str(5 % x) == f"{py_mod}(5, x)"

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.

P3: The PR description says the modulo tests cover zero and infinite operands, but this test has neither. Add assertions for a zero-remainder case such as -6 % 3 and an infinite divisor such as x % float("inf"), or correct the description.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At tests/units/reflex_base/vars/test_number.py, line 24:

<comment>The PR description says the modulo tests cover zero and infinite operands, but this test has neither. Add assertions for a zero-remainder case such as `-6 % 3` and an infinite divisor such as `x % float("inf")`, or correct the description.</comment>

<file context>
@@ -13,3 +13,12 @@ def test_exponent_parenthesizes_a_unary_base():
+    py_mod = "((a, b) => { const r = a % b; return r !== 0 && (r < 0) !== (b < 0) ? r + b : r; })"
+    assert str(LiteralNumberVar.create(-7) % 3) == f"{py_mod}(-7, 3)"
+    assert str(x % -2.5) == f"{py_mod}(x, -2.5)"
+    assert str(5 % x) == f"{py_mod}(5, x)"
</file context>
Suggested change
assert str(5 % x) == f"{py_mod}(5, x)"
assert str(5 % x) == f"{py_mod}(5, x)"
assert str(LiteralNumberVar.create(-6) % 3) == f"{py_mod}(-6, 3)"

@greptile-apps

greptile-apps Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; the previous zero-sign defect is fixed.

Summary

Compiles % on number Vars with Python’s sign-of-divisor behavior.

  • Number Var modulo expressions now follow Python’s divisor-sign rule.

Reviews (2) · Last reviewed commit: "fix(vars): give a zero remainder the div..." · Reviewed by Greptile

Comment thread packages/reflex-base/src/reflex_base/vars/number.py Outdated
Python returns a zero remainder with the sign of the divisor
(`6.0 % -3.0` is -0.0, `-6.0 % 3.0` is 0.0), while the compiled
expression kept the dividend's sign for a zero result.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant