fix(runtime): decode int32 operands in the dynamic arithmetic helpers

[proggeramlug]

Problem

The dynamic arithmetic helpers (js_dynamic_string_or_number_add and js_dynamic_{add,sub,mul,div,mod}) performed the raw f64 op directly on their operands. An int32 is NaN-boxed (INT32_TAG = 0x7FFE), so its f64 bits are an IEEE NaN — doing a + b propagates the tag through the FPU (ARM64 keeps the NaN payload) and hands back the boxed operand instead of the result.

Most visible symptom: a better-sqlite3 integer column read through the any-typed path made n + 100 === n — silently wrong math on every integer DB column (timestamps + durations, sums, pagination offsets, …).

const row = db.prepare("SELECT n FROM t").get() as any   // n = 42
row.n + 100   // → 42  (WRONG; expected 142)
row.n * 2     // → 84  (ok — see below)

-/*// happened to work because codegen eagerly coerces operands for the unambiguous numeric operators. + stays boxed (it might be string concat) and routes the raw int32 into js_dynamic_string_or_number_add, whose numeric fallback returned the operand's NaN-boxed bits for an int32 instead of decoding it — but the other four helpers had the identical latent bug for any int32 that reaches them.

Fix

Decode int32 operands to their plain-double value before the f64 op (shared numify_arith_operand), in js_dynamic_string_or_number_add and the five js_dynamic_{add,sub,mul,div,mod} helpers. Plain doubles and real NaNs from arithmetic pass through unchanged; the hot all-doubles fast path in js_dynamic_string_or_number_add is untouched (int32 already falls out of it).

This is the same INT32_TAG-not-decoded class as the JSON-null bug in #5796 (a different code path).

Tests

• New unit tests in dynamic_arith for int32 operands across + - * / % and the string-or-number +.
• End-to-end (better-sqlite3 + the fix), built from this change:

  sqlite n = 42   +100 = 142   *2 = 84   -1 = 41     (all correct)
  bitwise int32 = 42   +100 = 142
  

Summary by CodeRabbit

• Bug Fixes
  • Improved arithmetic handling so integer values are now correctly converted before dynamic math operations.
  • Fixed +, -, *, /, and % calculations to return the expected results when integers are mixed with other numeric values.
  • Corrected string-or-number addition so numeric integer inputs are handled consistently.
  • Added coverage to verify these arithmetic behaviors across common numeric cases.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a numify_arith_operand helper in dynamic_arith.rs that converts NaN-boxed int32 values to plain f64. This helper is applied to both operands in js_dynamic_mul, js_dynamic_add, js_dynamic_sub, js_dynamic_div, and js_dynamic_mod. The js_dynamic_string_or_number_add numeric path separately decodes int32 via as_int32() as f64. Unit tests are added to verify correct decoding across all five operations.

Changes

int32 Arithmetic Decoding Fix

Layer / File(s)	Summary
numify_arith_operand helper and arithmetic ops crates/perry-runtime/src/value/dynamic_arith.rs	Introduces numify_arith_operand to decode NaN-boxed int32 to f64; applies it to both operands in js_dynamic_mul, js_dynamic_add, js_dynamic_sub, js_dynamic_div, and js_dynamic_mod; adjusts js_dynamic_string_or_number_add to use as_int32() as f64 for int32 operands; adds unit tests for all five operations.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 A NaN-box hid inside the sum,
The FPU got confused, then some!
Now numify decodes the stash,
int32 won't produce garbage trash.
Hippity hop, the tests all pass! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The PR explains the bug, fix, and tests, but it doesn't follow the required template sections like Summary, Changes, Related issue, and Test plan.	Rewrite it using the repository template: add Summary, bullet Changes, Related issue, and a concrete Test plan with commands/checklist items.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly names the main fix and the affected runtime helpers.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

crates/perry-runtime/src/value/dynamic_arith.rs (1)

574-582: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add RHS and signed int32 regression cases.

The helper decodes both operands, but this test only exercises RHS int32 for +; add RHS cases for -, /, % and at least one negative int32 payload to lock in signed decoding.

Suggested test additions

             assert_eq!(js_dynamic_sub(int32(42), 1.0), 41.0);
+            assert_eq!(js_dynamic_sub(100.0, int32(42)), 58.0);
             assert_eq!(js_dynamic_mul(int32(42), 2.0), 84.0);
+            assert_eq!(js_dynamic_mul(2.0, int32(42)), 84.0);
+            assert_eq!(js_dynamic_mul(int32(-7), 6.0), -42.0);
             assert_eq!(js_dynamic_div(int32(84), 2.0), 42.0);
+            assert_eq!(js_dynamic_div(84.0, int32(2)), 42.0);
             assert_eq!(js_dynamic_mod(int32(43), 10.0), 3.0);
+            assert_eq!(js_dynamic_mod(43.0, int32(10)), 3.0);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-runtime/src/value/dynamic_arith.rs` around lines 574 - 582, The
regression test in dynamic_arith_decodes_int32_operands only covers an int32 on
the RHS for js_dynamic_add, so extend it to also assert RHS int32 decoding for
js_dynamic_sub, js_dynamic_div, and js_dynamic_mod, using the existing helpers
like int32, js_dynamic_sub, js_dynamic_div, and js_dynamic_mod. Also add at
least one case with a negative int32 payload to verify signed decoding remains
correct across the dynamic arithmetic path.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@crates/perry-runtime/src/value/dynamic_arith.rs`:
- Around line 574-582: The regression test in
dynamic_arith_decodes_int32_operands only covers an int32 on the RHS for
js_dynamic_add, so extend it to also assert RHS int32 decoding for
js_dynamic_sub, js_dynamic_div, and js_dynamic_mod, using the existing helpers
like int32, js_dynamic_sub, js_dynamic_div, and js_dynamic_mod. Also add at
least one case with a negative int32 payload to verify signed decoding remains
correct across the dynamic arithmetic path.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ac4e3349-d366-483b-9fc6-a34705e02550

📥 Commits

Reviewing files that changed from the base of the PR and between fc640f3 and 2c24275.

📒 Files selected for processing (1)

• crates/perry-runtime/src/value/dynamic_arith.rs