model: the shared numeric gate rejects a non-finite output cell

[Burntgogi]

What and why

Resolves #357. The shared numeric assertion kept worst at zero when a cell
was not finite, because NaN > worst is false. With got=[NaN],
want=[1.0] and a gate of 0.0 — which is exactly the gate
crates/model/tests/ops.rs uses for rms_norm — a mutant that writes one NaN
output into an otherwise correct computation read as a pass.

• crates/model/tests/common/asserts.rs: reject a non-finite operand
  (got and reference, each with the index) before it can update the maximum.
• crates/model/tests/ops.rs: the regression runs that mutant on the host
  with no fixture (#[should_panic(expected = "non-finite")]).

Scope

• Gate/test-only: the shared test assertion and one host regression.
• No production code, schema, byte-formula or launch change.

Evidence (host, bdc7082)

• cargo test -p bloomery-model: 379 passed / 0 failed across every binary,
  cargo fmt --all -- --check clean.
• Negative control: with the rejection removed, the same regression fails —
  the unfixed gate accepts got=[NaN] against want=[1.0] at gate 0.0,
  matching the issue. With the rejection restored, it passes.
• No CUDA/GPU verification needed or run (test-only).

Fixes #357.

[github-actions]

Verdict: MUST_FIX: the fix leaves the same NaN acceptance bug in sibling model gates.

Must fix

1. Reject non-finite operands in the sibling comparisons

crates/model/tests/forward.rs:32, crates/model/tests/head.rs:73, crates/model/tests/kv.rs:38.
These sibling comparisons still discard NaN errors. An embedding output with one NaN and otherwise matching cells gives worst == 0, so hw_forward_embed_is_exact passes (forward.rs:57–60). The same input defect passes the incremental KV assertions (kv.rs:99–104,130–135). In hw_head_matches_oracle, a positive NaN at the reference's winning index leaves usage == 0 and also passes the later total_cmp argmax check (head.rs:174–180).
Guarding these siblings now prevents another NaN regression from receiving a passing gate result. The reviewer instructions explicitly include proven sibling defects in fix PRs.

At the start of the comparison loops in forward.rs and head.rs, before calculating d, insert:

assert!(
    g.is_finite() && w.is_finite(),
    "non-finite comparison operand at index {i}: got {g}, reference {w}"
);

In kv.rs::max_abs_diff, replace .map(|(&g, &w)| (g - w).abs()) with:

.enumerate()
.map(|(i, (&g, &w))| {
    assert!(
        g.is_finite() && w.is_finite(),
        "non-finite comparison operand at index {i}: got {g}, reference {w}"
    );
    (g - w).abs()
})

Non-blocking

⚠ Exercise the reference guard: crates/model/tests/ops.rs:60. Removing w.is_finite() leaves this regression green. Extend this test now to prevent that recurrence: replace #[should_panic] with catch_unwind assertions for NaN in each operand position.

⚠ Keep comments about the current contract: crates/model/tests/ops.rs:54. The new comment records previous behavior, which AGENTS.md reserves for commit history. Replace it with // The shared numeric gate rejects a non-finite output without fixtures. Shorten the helper's new comment to // Reject non-finite operands before comparing errors.

Checked

Duplicates: 1 new function checked by name and purpose; no duplicate. Dryer reported 0 pairs.
Complexity: crapper flagged no function; the helper adds two operand checks.
Silent failure: both new guards precede arithmetic and panic with the operand and index; 3 sibling comparisons still hide NaN.
Tests: static mutation check—removing the output guard makes the new should-panic test fail; removing the reference guard does not.
Gates: no thresholds relaxed or cases removed; the PR body reports the FAIL-first negative control.
Escape catalog: no pattern matched. No hot path or unsafe block changed.

Notes

Rust tests were not rerun during this review. The PR reports host tests; it supplies no GPU gate evidence.

_{🔨 Shingler, bloomery's review bot: approves a clean pull request and requests changes on a must-fix (how it works)}

[github-actions]

Since the last review:

• Must-fix 1 fixed: forward.rs:32, head.rs:67, and kv.rs:40 reject both non-finite operands.
• Reference-guard warning fixed: ops.rs:61 exercises the reference operand.
• Comment warning fixed: ops.rs:54 and common/asserts.rs:16 describe the current contract.

Verdict: MUST_FIX: three additional sibling comparisons still accept NaN output cells.

Must fix

1. Use the fixed assertion in the remaining sibling folds

crates/model/tests/attn.rs:244, crates/model/tests/derived.rs:126, crates/model/tests/profile.rs:123.

A mutant that writes NaN into one q_nope2_absorbed output cell passes the attention stage-A comparison and the derived-output comparison: f32::max discards the NaN error. A NaN in one profiled forward output likewise passes the profiler's output comparison. Attention stage B reloads oracle inputs; the profiler's later checks use a separate in-process computation. Neither inspects those output cells.
These are kind A sibling failures in this fix PR. Closing them now prevents the same invalid output from receiving another passing gate result.

Reuse the existing shared assertion. In derived.rs and profile.rs, add:

#[path = "common/asserts.rs"]
mod asserts;

In attn.rs, remove the local maxdiff closure. Replace the stage-A assertion with:

asserts::assert_close(
    &out.data,
    &want.data,
    1e-5,
    "stage A: absorption arithmetic",
);

Replace the stage-C assertion with:

asserts::assert_close(
    &out.data,
    &want.data,
    1e-4,
    "stage C: wv_b + output projection",
);

In derived.rs, replace the let worst calculation and its assertion with:

asserts::assert_close(
    &from_derived.data,
    &from_reference.data,
    0.0,
    &format!("q_nope2-{blk}: cached-wblocks vs in-place-wblocks"),
);

In profile.rs, replace the let worst calculation and its assertion with:

asserts::assert_close(&on, &off, 0.0, "profiling changed the logits");

Checked

Duplicates: 0 new definitions in the incremental diff; dryer reported 0 pairs. No new duplicate finding.
Complexity: crapper flagged no function; the changes add operand guards and two panic captures.
Silent failure: 3 changed comparisons reject non-finite operands before arithmetic; 3 additional sibling folds still hide NaN.
Tests: static mutation check at common/asserts.rs:18 and :22—removing either operand guard makes ops.rs:66 fail. Infinity and length mismatches are rejected by the changed comparisons.
Gates: no thresholds moved or cases removed. The PR body reports FAIL-first for the shared-helper regression.
Escape catalog: no pattern matched. No engine hot path or unsafe block changed.

Notes

Rust tests and hardware gates were not rerun during this review.

_{🔨 Shingler, bloomery's review bot: approves a clean pull request and requests changes on a must-fix (how it works)}

[Burntgogi]

All four items are addressed in cd613b6.

• Must fix: the siblings now refuse a non-finite operand the way the shared helper does — forward.rs and head.rs guard the comparison loop's operands, kv.rs::max_abs_diff guards them under its map, and the KV doc line that said `NaN - NaN` folds to a clean zero now says the refusal instead.
• The reference guard is exercised: the regression drops should_panic for a catch_unwind pair and asserts the panic in each operand position. Removing the w.is_finite() guard now fails the test (re-verified locally, then restored).
• The comments carry the current contract only, in the wording the review named.

[Burntgogi]

Addressed in dd6c77d: the stage-A and stage-C attention comparisons are the shared assert_close (the local maxdiff closure is gone, tolerances unchanged at 1e-5 / 1e-4), derived.rs and profile.rs carry the common/asserts.rs module and their f32::max folds are the shared gate at tolerance 0. cargo test -p bloomery-model compiles all four targets and stays 379 passed / 0 failed, fmt clean.

[github-actions]

Since the last review:

• Must-fix 1 fixed: attn.rs:257, derived.rs:124, and profile.rs:122 use the shared assertion; attention stage C does too.

Verdict: CLEAN: the reported sibling comparisons now reject non-finite operands while preserving their shapes and tolerances.

Checked

Duplicates: 0 new definitions checked; four comparisons reuse the existing owner. Dryer reported 0 pairs.
Complexity: crapper flagged no function. The changes remove local maximum-error reductions.
Silent failure: 4 changed comparison sites followed. NaN and either infinity in either operand produce an indexed panic; length mismatches fail before iteration. Remaining sibling folds either reject NaN per element or only report diagnostics.
Tests: static mutation check: writing NaN at attn.rs:664 fails both the attention and derived gates. Returning one NaN cell at ops.rs:611 or forward.rs:226 fails the stage-C or profile comparison, respectively. Removing either shared finite guard makes the existing ops.rs:66 regression fail.
Gates: all four tolerances are unchanged; no test or coverage clause was removed. Finite differences still use the same inclusive tolerance check.
Escape catalog: C1–C4 do not match. No engine hot path or unsafe block changed.

Notes

Rust tests and fixture gates were not rerun during this review. The PR body reports a host regression run and its negative control; it carries no GPU gate evidence.

_{🔨 Shingler, bloomery's review bot: approves a clean pull request and requests changes on a must-fix (how it works)}

[juliankang4]

@Burntgogi Thanks for the contribution. The review is done, so I'm merging this.