🐛 fix: ten guards admitted NaN and returned it silently

[nstarman]

Ten guards admitted NaN and returned a NaN result with nothing raised — worse than the case each guard was written for, which at least errors.

x >= hi, x > hi, x <= tol are all False for a NaN, so a guard written as a direct comparison lets one through.

Measured, before

LorentzBoost(Q([nan, 0, 0], "")).gamma      -> non-finite, no error
_orthonormalize([1, nan, 0], T0)            -> NaN triad,   no error
ArcLength(..., s_max=5km)(Q(nan, "km"))     -> NaN position, no error
leq(Q(nan, "m"), Q(2, "m"))                 -> admits
geq(Q(nan, "m"), Q(2, "m"))                 -> admits

lorentz.py's own comment states the intent it was failing to enforce — guard "so every derived quantity reports the same superluminal error rather than one of them leaking a non-finite value".

The fix was already in the repo

TubularChart's reach guard is written ~(f > 0), added in #699 after this exact bug, with a comment saying why. Six other guards were still written the direct way. This applies the known lesson to each:

file	was	now
bishop.py	norm <= tol	~(norm > tol)
arclength.py	(s < -m) | (s > max + m)	~in_domain
lorentz.py gamma	beta_sq >= 1.0	~(beta_sq < 1.0)
lorentz.py rapidity	speed >= 1.0	~(speed < 1.0)
checks.py leq	jnp.any(x > max)	jnp.any(~(x <= max))
checks.py geq	jnp.any(x < min)	jnp.any(~(x >= min))

Valid inputs are untouched: gamma(beta=0.5) = 1.154701, an exactly-parallel or all-zero initial normal still raises, an in-domain s still passes, and leq/geq still admit in-range values.

The four degeneracy guards, added after review

These were first left out as a question about intent. Measuring them settled it: each one's own comment names the failure it was letting through.

file	was	admitted	now
builders.py	norm == 0	R all 9 entries NaN	~((norm > 0) & isfinite(norm))
lorentz.py from_rapidity	norm == 0.0	beta = [nan, nan, nan]	~((norm > 0.0) & isfinite(norm))
reflect.py	allclose(norm, 0)	H all 9 entries NaN	~((norm > 0) & isfinite(norm))
scale.py	any(isclose(s, 0))	s = [2, nan], s = [2, inf]	any(isclose(s, 0) | ~isfinite(s))

Three of them compute v / ‖v‖, which is a unit vector only where v is finite and non-zero — exactly a finite positive norm, since ‖v‖ is NaN iff a component is, inf iff a component is, and 0 iff v is. The equality test captured one of those three cases. The new predicate is the precondition itself, with no slack.

Scale's inf factor was the quietest failure in the whole audit, because it produced no NaN at all:

Scale.from_factors([2, inf]).inverse.s   ->  [0.5, 0.0]

That "inverse" is singular. Composed with the original it gives diag(1, 0), not the identity.

Still open, not in this PR

• Scale has a bypass. The singular check lives only in from_factors; Scale(jnp.diag([2., 0.])) constructs fine and its .inverse is [0.5, inf]. __init__ validates square and diagonal, not invertibility — so the comment at scale.py claiming "an s that exists has already passed" is wrong for singularity. Moving the check to _from_diagonal would close it, at the cost of a check per inverse/_merge. Worth measuring first.
• isclose(s, 0) is an absolute threshold. rtol multiplies |0|, so it reduces to |s| <= 1e-8: [2, 1e-9] is refused as singular while [2, 1e9] is accepted and inverts to 1e-9. Same conditioning, opposite verdicts. Pre-existing design choice, left alone.

Provenance

Found by auditing what each guard admits against what its docstring claims, across every error_if predicate in the repo — prompted by three guards in recent PRs that each turned out narrower than advertised.

Verification

Six tests, each mutation-verified by reverting the predicate it covers. 11030 tests pass across the repo; prek clean including ty.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Fixes several guard predicates that previously admitted NaN (because direct comparisons like x >= hi are False for NaN), causing silent propagation of non-finite values instead of raising.

Changes:

• Make LorentzBoost.gamma and LorentzBoost.rapidity reject NaN/non-finite inputs by switching from direct comparisons to negated “in-range” predicates.
• Make chart bound checks (leq/geq) and curveframes guards (_orthonormalize, arc-length solved-domain check) reject NaN by using negated “in-range” predicates.
• Add regression tests to ensure NaN no longer passes through these guards.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/coordinax/transforms/_src/actions/lorentz.py	Updates superluminal guards to also reject NaN and prevent non-finite leakage.
src/coordinax/_src/charts/checks.py	Updates leq/geq predicates so NaN fails bounds checks.
packages/coordinaxs.curveframes/src/coordinaxs/curveframes/_src/bishop.py	Updates _orthonormalize degeneracy guard to reject NaN inputs.
packages/coordinaxs.curveframes/src/coordinaxs/curveframes/_src/arclength.py	Updates solved-domain guard so NaN can’t fall through to interpolation.
packages/coordinaxs.curveframes/tests/unit/test_guards_reject_nan.py	Adds regression tests for NaN rejection across the updated guards.

Suppressed comments (3)

packages/coordinaxs.curveframes/tests/unit/test_guards_reject_nan.py:60

• These pytest.raises(Exception, ...) assertions are too broad and can hide unrelated failures. ArcLength's solved-domain guard is already exercised elsewhere as a RuntimeError (e.g. test_arclength_smax.py).

    fast = cxfc.ArcLength(helix, "s", s_max=u.Q(5.0, "km"))
    with pytest.raises(Exception, match="solved domain"):
        fast(u.Q(jnp.nan, "km"))

    # in-domain and genuinely-outside both behave as before

packages/coordinaxs.curveframes/tests/unit/test_guards_reject_nan.py:82

• Catching Exception here is too broad for a regression test (it can let unexpected errors satisfy the assertion). Prefer the expected RuntimeError-family exception raised by the guard (core tests use eqx.EquinoxRuntimeError, which is a RuntimeError).

    bad = cxfm.LorentzBoost(u.Q(jnp.array([jnp.nan, 0.0, 0.0]), ""))
    with pytest.raises(Exception, match="subluminal"):
        _ = bad.gamma

packages/coordinaxs.curveframes/tests/unit/test_guards_reject_nan.py:95

• pytest.raises(Exception, ...) is too broad for these leq/geq guard checks and can mask unrelated failures. In core tests these checks are expected to raise either an eqx.EquinoxRuntimeError (traced path) or a ValueError (concrete path), both of which are narrower than Exception.

    with pytest.raises(Exception, match="less than or equal"):
        leq(u.Q(jnp.nan, "m"), u.Q(2, "m"))
    with pytest.raises(Exception, match="greater than or equal"):
        geq(u.Q(jnp.nan, "m"), u.Q(2, "m"))

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.70%. Comparing base (015e547) to head (ac7b610).
⚠️ Report is 10 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #773      +/-   ##
==========================================
- Coverage   96.71%   96.70%   -0.01%     
==========================================
  Files         268      269       +1     
  Lines        9125     9179      +54     
==========================================
+ Hits         8825     8877      +52     
- Misses        300      302       +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.