Fix SIGFPE crash in from-double/from-float; add rationalize

[carpentry-agent]

The crash

Rational.from-double and Rational.from-float built a fraction by repeatedly
multiplying an Int accumulator by 10 until the scaled value looked integral.
For any input that is not a short terminating decimal, that accumulator runs
past 2^31, wraps around, and eventually becomes 0 — so the final
(new numerator 0) divides by zero and the program dies with SIGFPE.

Reproduced on a 32-bit Int build:

(Rational.from-double (Double.sqrt 2.0)) ; => process exits with signal 8 (SIGFPE)
(Rational.from-float (/ 1.0f 3.0f))      ; => 16666667/50000000 (garbage), not 1/3

The fix

Both functions now compute a best rational approximation with a bounded
denominator using the convergents of the continued-fraction (Stern-Brocot)
expansion. Because every convergent's numerator and denominator are kept below
the bound, nothing can overflow, and the result is the simplest fraction for
the value:

(Rational.from-double (Double.sqrt 2.0)) ; => (Rational 665857/470832), no crash
(Rational.from-float (/ 1.0f 3.0f))      ; => (Rational 1/3)

from-double/from-float delegate to the new routine with a high default
denominator bound (1000000), which is large enough that exact small values
still round-trip and small enough that no convergent can overflow Int.

New: rationalize

A new public function Rational.rationalize takes a Double and a maximum
denominator and returns the closest Rational whose denominator does not exceed
that bound (a.k.a. limit-denominator):

(Rational.rationalize 3.141592653589793 300) ; => (Rational 355/113)
(Rational.rationalize 0.5 100)               ; => (Rational 1/2)

Negatives, zero, and integer inputs are all handled.

Compatibility

Exact values still round-trip — the existing assertions for
from-double 0.25 => 1/4 and from-float 0.25 => 1/4 are untouched and still
pass, alongside a new from-double 1.0 => 1/1. Tests were added covering the
previously crashing/garbage cases and the new function; the full suite passes
(113/0), and angler + carp-fmt --check are clean.

---

Opened by the carpentry-org heartbeat agent (Claude). Veit has not reviewed this yet.

[carpentry-agent]

Hardened the approximation against a second overflow in the same class. The first pass bounded the denominator, but since a convergent's numerator is roughly x · denominator, a large-magnitude non-terminating input pushed the numerator past 2^31 and wrapped it to garbage (no crash, silently wrong):

• (from-double 100000000.333) → 1625739685/933012 ≈ 1742
• (from-double 987654.321987) → ≈ 994.7

The convergent recurrence (and the semiconvergent/mediant step) now also stop before either numerator or denominator would cross a safe Int ceiling, returning the last representable convergent. Same values now:

• (from-double 100000000.333) → 300000001/3 ≈ 100000000.333
• (from-double 987654.321987) → 1073580248/1087 ≈ 987654.321987

All previously-correct small/moderate values are unchanged (sqrt2, pi → 355/113, 12345.6789 → 123456789/10000, 0.25 → 1/4, 1.0 → 1/1, from-float 1/3). Added regression tests asserting the two large cases are within a small relative error; full suite is green (116/0), angler + carp-fmt --check clean.

[carpentry-reviewer]

Build & Tests

Checked out claude/from-double-continued-fraction (HEAD 54813c3), built and ran the full suite on a 32-bit-Int machine (the same environment class the bug targets):

• carp -x tests/rational.carp → 116/0, all pass.
• CI is green on both ubuntu-latest and macos-latest.
• I reproduced the original crash on the pre-fix code (a4c5a7f): (Rational.from-double (Double.sqrt 2.0)) exits with signal 8 (SIGFPE). On this branch the same call returns (Rational 665857/470832) — the reported bug is real and this PR genuinely fixes it. The continued-fraction/limit-denominator approach is sound, and the numerator/denominator overflow guards work as intended across the tested realistic range (sqrt2, 1/3, π→355/113, moderate and large decimals).

Findings

The core fix is correct, but the rewrite leaves three demonstrable defects at the input boundary — and two of them re-introduce the very SIGFPE-crash class this PR exists to remove, just deferred downstream. All three were reproduced on this branch:

1. NaN input → infinite hang. (Rational.from-double (/ 0.0 0.0)) never terminates (killed at a 25s timeout, exit 124). Same path for from-float with a NaN float. In rationalize the loop's exits never fire for NaN: frac = v - a is NaN, (Double.< frac 0.000000001) is false, and v = 1/NaN = NaN, so done is never set (rational.carp:130-149).

2. +Inf → invalid 1/0, which SIGFPEs on first use. (Rational.from-double (/ 1.0 0.0)) returns (Rational 1/0). That value is a landmine: (Rational.to-int &that) divides numerator by a zero denominator and exits with signal 8 — the same SIGFPE the PR set out to eliminate, now surfacing at the first to-int.

3. |x| ≥ 2^31 → invalid 1/0 or garbage numerator. (Rational.from-double 1000000000000000000.0) → (Rational 1/0); (Rational.from-double 3000000000.0) → (Rational -1294967296/1) (negative garbage).

Root cause (single): the first loop iteration computes a (Double.to-int (Double.floor v)) at rational.carp:131 with no finiteness/range guard — the over-den/over-num guards at :132-137 are gated on (Int.> q1 0), but q1 starts at 0, so the first a is always accepted. For non-finite or ≥ 2^31 inputs, Double.to-int overflows to garbage and the recurrence can terminate exact with q0 = 0 and q1 = 0, so the (new … q1) at :150-151 builds a zero-denominator Rational.

Suggested fix (cheap, localized): guard the top of rationalize for non-finite x and for |x| beyond a safe bound (e.g. int-cap), before the loop — e.g. return (from-int 0)/clamp for these, or reject them explicitly. That closes all three without touching the working core. Optionally assert q1 > 0 before the final new as a belt-and-suspenders invariant. Worth a regression test for NaN, +Inf, and one ≥ 2^31 value.

These are edge cases, but from-double is public and NaN (0/0, sqrt of a negative), Inf (overflow, division by zero), and large magnitudes are all reachable in ordinary use — and a numerics conversion whose stated purpose is "no more from-double crashes/garbage" shouldn't hand back a hang or a crash-on-use 1/0.

Verdict: revise

The fix is correct and a clear improvement for the realistic domain (116/0, original SIGFPE genuinely gone), but it still hangs on NaN and returns a crash-on-use 1/0 for ±Inf and |x| ≥ 2^31; a single finiteness/range guard at the top of rationalize would close all three.

[carpentry-agent]

Addressed the three boundary defects from the review — all were the same root cause (the first rationalize iteration ran with no finiteness/range guard), so they're fixed with a single guard at the top of the function, before the loop:

• NaN ((from-double (/ 0.0 0.0))) — was an infinite hang; now returns 0/1 and terminates.
• ±Inf ((from-double (/ 1.0 0.0))) — was an invalid 1/0 that SIGFPE'd on first use; now clamps to ±1073741824/1, so to-int is safe.
• |x| ≥ 2^31 ((from-double 3000000000.0)) — was 1/0/negative garbage; now clamps to the largest representable rational.

The guard keeps the plain-Rational signature (no Result): NaN saturates to 0/1, and infinities or magnitudes past the representable range clamp to ±int-cap/1. NaN is detected first via (not (Double.= x x)) (so it isn't mistaken for a large magnitude, since NaN comparisons are false), and every returned value now has a non-zero denominator. The continued-fraction core is untouched.

Added regression tests for NaN, +Inf, -Inf, a ≥ 2^31 value, and a to-int-doesn't-SIGFPE check. Full suite is 122/0; carp-fmt -c and angler are clean.

[carpentry-reviewer]

Build & Tests

Checked out claude/from-double-continued-fraction at HEAD 122794e and built/ran on the same 32-bit-Int machine class the bug targets:

• carp -x tests/rational.carp → 122/0, all pass.
• carp-fmt --check and angler clean.
• CI green on ubuntu-latest and macos-latest.

Prior feedback

The previous review flagged three boundary defects, all rooted in the first rationalize iteration running with no finiteness/range guard. This round adds a single cond guard at the top of rationalize (rational.carp:122-127). I re-ran each reported case on this branch and confirmed all three are fixed:

• NaN ((from-double (Double.sqrt -1.0)), (from-double (/ 0.0 0.0))) → 0/1 and terminates (was an infinite hang). NaN is checked first via (not (Double.= x x)), so it's never mistaken for a large magnitude.
• +Inf ((from-double (/ 1.0 0.0))) → 1073741824/1; (to-int …) returns 1073741824 with no SIGFPE (was a crash-on-use 1/0).
• |x| ≥ 2^31 (3000000000.0, 1.5e9) → clamps to ±1073741824/1 (was garbage / 1/0).

Findings

I went past the reported cases to probe the guard and the surrounding domain; everything holds:

• -Inf → -1073741824/1, to-int → -1073741824, no SIGFPE.
• int-cap boundary: 1073741824.0 (exactly the cap; the guard uses strict <, so it falls through to the continued-fraction core) → 1073741824/1, correct and no overflow. 1073741823.9 → 1073741824/1.
• tiny values: 1e-10, 1e-7 → 0/1, terminates, and 0 is genuinely the nearest fraction under the 10^6 denominator bound (no 1/frac blow-up hang).
• from-float non-finite: NaN → 0/1, ±Inf → ±1073741824/1 (delegates through Double.from-float, so the same guard covers it).
• degenerate max-denom: rationalize pi 0 and rationalize pi -5 return a valid 3/1 — no crash and no zero denominator, matching the "max-denom should be at least 1" doc note by degrading gracefully rather than trapping.
• regression check: sqrt2 → 665857/470832, 1/3f → 1/3, 0.25 → 1/4, 1.0 → 1/1, pi/300 → 355/113 all unchanged.

Every result I could produce has a non-zero denominator and the function always terminates — the guard closes the class the PR set out to eliminate without touching the working core. No new issues found. The int-cap of 2^30 (rather than 2^31-1) is a conservative, correct choice: it keeps a*p1/a*q1 products safely inside Int while still round-tripping all realistic values.

Verdict: merge

The follow-up cleanly resolves all three previously-flagged boundary defects with a localized guard; 122/0 tests, CI green, and independent boundary probing (NaN, ±Inf, cap boundary, beyond-2^30, tiny magnitudes, non-finite floats, degenerate bounds) all yield valid, finite, divide-safe rationals with no hangs or SIGFPEs.

[carpentry-agent]

Marking ready for review. The reviewer's latest pass (HEAD 122794e) resolved all three previously-flagged boundary defects and returned a merge verdict; CI is green on both runners and the branch is MERGEABLE against master. Nothing else outstanding — this was only still in draft from the initial cycle.