fix(contract): T5 -- schema-gate the two fixed-offset NaN sweepers

[AdaWorldAPI]

Closes tracked item T5 ("schema.has() gate on the two fixed-offset sweepers") — located, not assumed: no literal string match anywhere in the tree, found by enumerating every file with multiple value_offset() call sites and reading the candidates.

What it is

nan_projection.rs's project_energy_nonfinite / energy_all_finite read ValueTenant::Energy at its fixed offset unconditionally, unlike the already-correct sibling pattern in ocr.rs (if schema.has(ValueTenant::EntityType) { ... }).

value_offset() is a fixed reserved position, identical across every ValueSchema (RESERVE, DON'T RECLAIM) — so the ungated read was never memory-unsafe. The risk is semantic: a row whose resolved schema doesn't materialise Energy (ValueSchema::Compressed, e.g. NodeGuid::CLASSID_FMA) has no writer obligated to keep that byte range meaningful, so a schema-blind sweep could misread foreign/uninitialized bytes as a real energy accumulator.

Why this wasn't already biting anyone

ReadMode::DEFAULT (what classid 0 resolves to) is a documented TEMPORARY 2026-06-15 POC pin to ValueSchema::Full (every tenant present) — its own doc comment says it flips back to Bootstrap (no tenants) "when the POC ends." Every row in the tree currently resolves to a schema that has Energy, so the gate is a no-op today. It becomes load-bearing the moment that pin reverts, or any classid is minted to a narrower schema — landed now rather than as a fire drill later, matching ocr.rs's own test comment: "No classid resolves to Bootstrap today — when one is minted, the same schema.has() gate leaves its slab empty."

The fix

• row_has_energy(row) — one branch per row on schema presence, never on the float value; the branchless finiteness test (exponent-mask compare) is untouched for every row it applies to.
• Gated rows excluded from both sweeps. NanReport gains skipped: usize so the gate's effect is observable, not a silent no-op (the workspace's can-it-fire rule).

Tests

Fixtures moved off NodeGuid::local(0) (classid 0, temporarily Full) onto NodeGuid::CLASSID_OSINT (permanently Cognitive) — a fixture pinned to the temporary default would silently go vacuous the moment it flips.

• Can-it-fire (new): real registered classids (CLASSID_OSINT vs CLASSID_FMA, not synthetic overrides) with NaN/Inf poison planted at the Energy offset on the Compressed-schema rows. Asserts the poison is real via a direct ungated read first (so the test can't pass vacuously), then asserts the gated sweep ignores it entirely.
• Can-it-stay-silent (existing tests extended): skipped == 0 asserted on the two pre-existing homogeneous-batch tests — the gate changes nothing for well-formed batches.

cargo test -p lance-graph-contract --lib: 1135/1135 green. cargo clippy --all-targets -- -D warnings: clean.

The one external caller

symbiont/src/domino.rs calls project_energy_nonfinite. Verified compile-safe by direct read — it only uses .is_clean()/.count()/.nonfinite (never an exhaustive destructure), and its own rows currently resolve to Full (has Energy), so the gate changes nothing about its runtime behaviour today. symbiont is a bin-only crate with a heavy SurrealDB/OGAR/AMX dependency tree excluded from the default workspace; a cargo check against it was launched to confirm end-to-end but hadn't finished within this session's window — the field/method-access proof above stands independent of that.

Filed, not fixed here

ISS-DOMINO-WRITES-ENERGY-OUTSIDE-ITS-OWN-SCHEMA — domino.rs's own writers (set_energy, write_lanes) have the identical ungated-fixed-offset pattern on the write side, masked by the same temporary DEFAULT pin. The moment that pin reverts, domino.rs's own assert!(report.is_clean(), ...) would go vacuously true (every row skipped) rather than fail loudly. Fixing it means an architecture call — mint domino.rs a proper classid into the shared registry, or give it an explicit unchecked-sweep variant for private byte-region borrows outside the schema system — that T5's scope didn't license unilaterally. Left as a flagged follow-on.

---

Generated by Claude Code

[coderabbitai]

Warning

Review limit reached

@AdaWorldAPI, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 40 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5141c866-3abe-4080-8660-3b5b0b1d92c6

📥 Commits

Reviewing files that changed from the base of the PR and between ce9d017 and 94f771a.

📒 Files selected for processing (3)

• .claude/board/ISSUES.md
• .claude/board/exec-runs/t5-closure-nan-projection-schema-gate.md
• crates/lance-graph-contract/src/nan_projection.rs

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

---

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.}

[AdaWorldAPI]

@coderabbitai review

Requesting now, while draft — this PR changes the public shape of NanReport (new skipped field) and its semantics (total used to mean "all rows swept", now means "rows actually inspected"). Please check specifically:

1. Is row.key.read_mode() cheap enough to call once per row inside a sweep loop, or does it do more work than the fixed-offset read it's gating (the classid_read_mode registry lookup mentioned in canonical_node.rs)? The module doc claims the finiteness test "survives unchanged" but doesn't quantify the added per-row cost.
2. Is there any other consumer of NanReport.total in the workspace whose meaning silently changed (I grepped and found only the crate's own tests + symbiont/domino.rs, which doesn't read .total, but a second pass would help).
3. Whether ISS-DOMINO-WRITES-ENERGY-OUTSIDE-ITS-OWN-SCHEMA (filed in .claude/board/ISSUES.md in this same commit) accurately describes the risk, or overclaims/underclaims it.

---

Generated by Claude Code

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '== Candidate files =='
fd -HI -t f 'nan_projection\.rs|canonical_node\.rs|domino\.rs|ISSUES\.md' . | sort

printf '%s\n' '== Source outlines =='
for f in $(fd -HI -t f 'nan_projection\.rs|canonical_node\.rs|domino\.rs' .); do
  echo "--- $f"
  ast-grep outline "$f" --items all --view expanded || true
done

printf '%s\n' '== NanReport definition/construction/accesses =='
rg -n -P -C 5 '\bNanReport\b|\.total\b|total\s*:' -g '*.rs' .

printf '%s\n' '== Read-mode and schema resolution paths =='
rg -n -P -C 5 '\bread_mode\s*\(|\bclassid_read_mode\b|\bValueSchema\b|\bschema\s*\(' -g '*.rs' .

printf '%s\n' '== Energy read/write and offset uses =='
rg -n -P -C 5 '\b(project_energy_nonfinite|energy_all_finite|row_has_energy|set_energy|write_lanes|value_offset\s*\(|ValueTenant::Energy)' -g '*.rs' .

printf '%s\n' '== Filed issue =='
issue_file="$(fd -HI -t f 'ISSUES\.md' . | head -n 1 || true)"
if [ -n "$issue_file" ]; then
  rg -n -P -C 8 'ISS-DOMINO-WRITES-ENERGY-OUTSIDE-ITS-OWN-SCHEMA' "$issue_file"
fi

Length of output: 201096

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

np="crates/lance-graph-contract/src/nan_projection.rs"
cn="crates/lance-graph-contract/src/canonical_node.rs"
dm="crates/symbiont/src/domino.rs"
is=".claude/board/ISSUES.md"

printf '%s\n' '== nan_projection implementation and tests =='
sed -n '35,250p' "$np"

printf '%s\n' '== exact NanReport references and total field accesses (Rust only) =='
rg -n -P -g '*.rs' -C 3 '\bNanReport\b|\b(?:[A-Za-z_][A-Za-z0-9_]*\.)total\b|\btotal\s*:' \
  crates

printf '%s\n' '== registry initialization and read-mode resolution =='
sed -n '1410,1485p' "$cn"
printf '%s\n' '== ValueSchema::has implementation =='
sed -n '1110,1195p' "$cn"

printf '%s\n' '== Domino construction, fixed-offset helpers, and report use =='
sed -n '35,115p' "$dm"
sed -n '135,180p' "$dm"

printf '%s\n' '== complete filed Domino issue through next heading =='
awk '
  /^## ISS-DOMINO-WRITES-ENERGY-OUTSIDE-ITS-OWN-SCHEMA/ {on=1}
  on {print}
  on && NR > 1 && /^## / && $0 !~ /ISS-DOMINO-WRITES-ENERGY-OUTSIDE-ITS-OWN-SCHEMA/ {exit}
' "$is"

printf '%s\n' '== all NodeGuid::local constructors in symbiont and all project_energy_nonfinite callers =='
rg -n -P -g '*.rs' -C 3 'NodeGuid::local\s*\(|\bproject_energy_nonfinite\s*\(' crates/symbiont crates/lance-graph-contract

Length of output: 146488

---

@AdaWorldAPI Review complete.

1. read_mode() cost: it is not comparable to the old fixed-offset read. row_has_energy() calls NodeGuid::read_mode(), which delegates to classid_read_mode() and performs a lookup in the immutable LazyLock<HashMap<u32, ReadMode>>, then a ValueSchema::has() bit test. The latter is trivial; the hash lookup is the added per-row cost and can dominate the prior four-byte load + exponent-mask check, especially for in-cache homogeneous batches.

   The implementation is correct, and “the branchless finiteness test survives unchanged” is accurate after eligibility is resolved. However, it should not be read as “the sweep’s cost survives unchanged.” I would add a small benchmark or at least document this explicitly if this projection is on a hot path. Source inspection alone cannot establish whether the added lookup is acceptable at the intended batch sizes.

2. NanReport.total consumers: the workspace audit found no production consumer of NanReport.total. Its only accesses are the nan_projection.rs tests. symbiont/src/domino.rs uses is_clean(), count(), and nonfinite, so its behavior does not depend on the changed meaning. This is safe within the workspace; naturally, public downstream consumers outside the repository cannot be ruled out by this search.

3. Filed Domino issue: ISS-DOMINO-WRITES-ENERGY-OUTSIDE-ITS-OWN-SCHEMA accurately describes the risk. domino.rs creates NodeGuid::local(idx) rows, so after the temporary default flips to Bootstrap, its unconditional Fingerprint/Energy reads and writes will target bytes its resolved schema does not materialize. The memory access remains in-bounds, but the semantic contract is violated. The new projection will then skip every Domino row, and assert!(report.is_clean()) will pass with zero inspected rows—exactly the stated vacuous-success risk.

   One scope note: crates/symbiont/src/bridge.rs has a sibling latent pattern—default-class rows plus unconditional Energy reads/writes. It does not use project_energy_nonfinite in the inspected path, so it does not invalidate the Domino-specific issue, but it should be tracked separately or cross-referenced when the default-schema flip is planned.

No blocking correctness issue found in this PR’s T5 gate itself.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[AdaWorldAPI]

Thanks — all three verified and addressed in 94f771a:

1. Confirmed read_mode() → classid_read_mode() is a HashMap lookup behind a LazyLock, not a bitmask. Doc corrected: "branchless" now scoped explicitly to the finiteness test, with the added per-row lookup cost named rather than implied away. Not benchmarked — flagged as the first place to look if this ever lands on a hot path.
2. Confirmed, no action needed.
3. Confirmed bridge.rs is the sibling instance you found. Added as a labelled addendum to the existing ISS-DOMINO-... entry (not a second issue, since the fix is the same architecture call on a second call site) — with the distinction you drew verified: bridge.rs bypasses nan_projection.rs entirely (its tests use f32::is_finite() directly), so T5's gate doesn't touch its behaviour at all; its exposure is a write-side schema-contract drift on the same POC pin, not the vacuous-skip risk domino.rs has.

---

Generated by Claude Code