From 637c0527003e6cb791ea48220221139fac3b13ef Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 20:45:06 +0000 Subject: [PATCH 1/2] fix(contract): T5 -- schema-gate the two fixed-offset NaN sweepers project_energy_nonfinite and energy_all_finite in nan_projection.rs read ValueTenant::Energy at its fixed reserved offset unconditionally, with no schema.has() gate -- the same anti-pattern I-LEGACY-API-FEATURE-GATED names, here on the read side of a sweep rather than a versioned accessor. value_offset() is a fixed reserved byte 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 accumulator. Not biting today only because ReadMode::DEFAULT is a documented TEMPORARY 2026-06-15 POC pin to ValueSchema::Full (every tenant present) -- every row in the tree currently resolves to a schema that has Energy. The gate is a no-op in practice right now and becomes load-bearing the moment that pin reverts to Bootstrap (as its own doc comment says it will) or any classid is minted to a narrower schema. Landed before the flip, matching ocr.rs's own test comment: "when one is minted, the same schema.has() gate leaves its slab empty." Fix: row_has_energy() reads row.key.read_mode().value_schema.has(Energy) -- one branch per row on schema presence, never on the float value, so the branchless finiteness test (exponent-mask compare) survives unchanged for every row it actually applies to. Rows failing the gate are excluded from both project_energy_nonfinite and energy_all_finite; NanReport gains a skipped: usize field so the gate's effect is observable rather than a silent no-op. 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 have silently gone vacuous on the eventual flip. New adversarial test uses real registered classids (CLASSID_OSINT vs CLASSID_FMA) 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 (skipped, not flagged). The two pre-existing tests gained a skipped == 0 assertion as the inertness twin. cargo test -p lance-graph-contract --lib: 1135/1135 green (4/4 in nan_projection, including the new adversarial case). cargo clippy -p lance-graph-contract --all-targets -- -D warnings: clean. symbiont (the one external caller, domino.rs, bin-only crate with a heavy SurrealDB/OGAR/AMX dependency tree, excluded from the default workspace): verified compile-safe by direct read -- it only calls .is_clean()/.count()/ .nonfinite on the NanReport, never an exhaustive destructure, so the new field cannot break it, and its own rows all currently resolve to Full (has Energy) so the gate changes nothing about its runtime behaviour today. Filed 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, currently masked by the same temporary DEFAULT POC pin. Not fixed here -- it's an architecture call (mint a proper classid vs. an explicit unchecked-sweep variant for private byte-region borrows) that T5's scope didn't license unilaterally. Left as a follow-on so the POC-default flip doesn't silently turn domino.rs's own NaN assertion vacuous. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01K3RyLEbuNSHxxB3NTTrGki --- .claude/board/ISSUES.md | 49 +++++++ .../t5-closure-nan-projection-schema-gate.md | 113 +++++++++++++++ .../src/nan_projection.rs | 131 ++++++++++++++++-- 3 files changed, 283 insertions(+), 10 deletions(-) create mode 100644 .claude/board/exec-runs/t5-closure-nan-projection-schema-gate.md diff --git a/.claude/board/ISSUES.md b/.claude/board/ISSUES.md index 061ebc97d..fc0cb6e5d 100644 --- a/.claude/board/ISSUES.md +++ b/.claude/board/ISSUES.md @@ -1,5 +1,54 @@ # Issues Log — Open + Resolved (double-entry, append-only) +## ISS-DOMINO-WRITES-ENERGY-OUTSIDE-ITS-OWN-SCHEMA (2026-07-29) — OPEN, FOUND WHILE CLOSING T5 + +Surfaced while landing T5 (the `nan_projection.rs` schema gate, see +`.claude/board/exec-runs/t5-closure-nan-projection-schema-gate.md`). Not fixed +here — it is a design decision, not a mechanical follow-up. + +`crates/symbiont/src/domino.rs`'s POC builds every board via `NodeGuid::local(idx)` +(classid 0). `set_energy`/`energy_of`/`read_lanes`/`write_lanes` all write/read the +`Fingerprint` and `Energy` tenants at their fixed reserved offsets, unconditionally +— no `schema.has()` gate, same pattern T5 just fixed in the sweep surface. + +**Why this isn't biting today:** `ReadMode::DEFAULT` (what classid 0 resolves to) +is currently a **documented TEMPORARY 2026-06-15 POC pin** to `ValueSchema::Full` +(every tenant materialised), scheduled to flip back to `ValueSchema::Bootstrap` +(zero tenants) "when the POC ends" — see the doc comment on `ReadMode::DEFAULT` +in `canonical_node.rs`. While that pin holds, domino.rs's Bootstrap-classid rows +resolve to `Full`, which DOES include `Fingerprint` + `Energy`, so its writes are +schema-legal by accident of the temporary default, not by design. + +**What breaks on the flip:** the instant `ReadMode::DEFAULT` reverts to +`Bootstrap` (`FieldMask::EMPTY`), domino.rs's own writer becomes the same +violation T5 just closed on the reader side — writing real data into a byte range +its own row's declared schema says isn't part of its content. The now-gated +`project_energy_nonfinite` sweep domino.rs calls would then report EVERY row as +`skipped` (schema says no Energy present), turning `assert!(report.is_clean(), ...)` +**vacuously true** — the exact anti-pattern `E-VACUOUS-ASSERTION-IS-THE-HOUSE-STYLE-1` +warns against, and silent: the assertion would keep passing, just for the wrong +reason, with zero rows actually checked. + +**Two live options, not decided here:** +1. Mint domino.rs a proper classid into the shared `classid_read_mode` registry + (e.g. resolving to `ValueSchema::Cognitive`, which already covers exactly + `Fingerprint` + `Energy` among its tenants — see `ReadMode::OSINT`'s schema). + Correct per-doctrine, but touches the SAME registry OGAR mints into — heavier + than "a very basic POC to prove the SoA orchestration" (the file's own framing) + seems to warrant. +2. Document domino.rs's tenant use as a deliberate raw-byte-region borrow OUTSIDE + the schema system (it never goes through `SoaEnvelope` for external/shared + consumption today) and keep it calling an explicitly UNCHECKED sweep variant + rather than the schema-gated `project_energy_nonfinite` — i.e., add a second, + clearly-named function (`project_energy_nonfinite_unchecked`?) so callers who + are knowingly outside the schema system don't get silently zeroed out by a + gate meant for schema-respecting callers. + +Not resolved because it's an architecture call (which of the two, or a third +option) that the T5 scope didn't license me to make unilaterally. Flagging so the +POC-default flip (whenever it lands) doesn't silently turn domino.rs's own +correctness assertion into theater. + ## ISS-NO-PER-THREAD-TEMPORAL-PROJECTION-IS-EVER-CONSTRUCTED (2026-07-29) — OPEN, UPSTREAM OF `meta_basin` > **⊘ RE-GRADED AND RELOCATED (Codex, #869) — my first filing mislocated this.** diff --git a/.claude/board/exec-runs/t5-closure-nan-projection-schema-gate.md b/.claude/board/exec-runs/t5-closure-nan-projection-schema-gate.md new file mode 100644 index 000000000..037d9e70a --- /dev/null +++ b/.claude/board/exec-runs/t5-closure-nan-projection-schema-gate.md @@ -0,0 +1,113 @@ +# T5 CLOSURE — schema.has() gate on the two fixed-offset sweepers + +## What T5 actually was (located, not assumed) + +Grepped for "two fixed-offset sweepers" and found no literal string match anywhere +in the tree — the task label was a description, not a quote. Located by finding +every file with multiple `value_offset()` call sites and reading each candidate: +`crates/lance-graph-contract/src/nan_projection.rs`'s `project_energy_nonfinite` +and `energy_all_finite` are the two functions that (a) literally sweep +`rows: &[NodeRow]`, (b) read a fixed offset (`ValueTenant::Energy.value_offset()`) +unconditionally, with (c) zero schema gate — unlike the already-correct sibling +pattern in `ocr.rs`'s `to_node_row` (`if schema.has(ValueTenant::EntityType) { ... }`). + +## The real risk (not hypothetical) + +`value_offset()` is a FIXED reserved byte position per tenant, identical across +every `ValueSchema` (RESERVE, DON'T RECLAIM) — so an ungated read is never memory- +unsafe. The risk is semantic: a row whose resolved schema does NOT materialise +`Energy` (e.g. `ValueSchema::Compressed`, used by `NodeGuid::CLASSID_FMA` — no +writer obligated to keep that byte range meaningful) could have foreign/garbage/ +uninitialized bytes at the Energy offset misread as a real accumulator — a false +non-finite flag on data that was never Energy at all. + +## Why this wasn't already biting anyone (and why it still needed fixing) + +`ReadMode::DEFAULT` (what an unconfigured/classid-0 row resolves to) is currently +pinned to `ValueSchema::Full` as a **documented TEMPORARY 2026-06-15 POC** setting +— the doc comment on `ReadMode::DEFAULT` says explicitly: "When the POC ends, flip +`value_schema` back to `ValueSchema::Bootstrap` HERE and in `ClassView` together." +`Full` includes every tenant, so EVERY row in the tree resolves to a schema that +has `Energy` right now — the gate is currently a no-op in practice. It stops being +a no-op the moment that POC pin reverts (or any classid is minted to a narrower +schema), which is exactly why `ocr.rs`'s own test comment reads: "No classid +resolves to Bootstrap today — when one is minted, the same `schema.has()` gate +leaves its slab empty." T5 is the same principle applied to the sweep surface, +landed BEFORE the flip rather than as a fire drill after. + +## The one real caller — checked, not assumed + +`crates/symbiont/src/domino.rs` calls `project_energy_nonfinite(&rows)` on rows +built via `NodeGuid::local(idx)` (classid 0 → currently `Full` → has `Energy`). +Confirmed by direct read that its only consumption of `NanReport` is +`.is_clean()`, `.count()`, `.nonfinite` (field/method access, never an exhaustive +struct-destructure) — so adding the `skipped` field is compile-safe, and since +domino.rs's rows all currently resolve to a schema that includes `Energy`, the +gate changes nothing about its runtime behaviour today (`skipped` will be 0 for +every row it constructs). Not a vacuous no-op risk: verified `symbiont` doesn't +construct or destructure `NanReport` anywhere else (`grep -rn "NanReport"` outside +`nan_projection.rs` returns nothing). + +## The fix + +- `row_has_energy(row) -> bool` — reads `row.key.read_mode().value_schema.has(ValueTenant::Energy)`. + One branch, on schema presence, never on the float value. +- `project_energy_nonfinite`: skip (don't read) rows failing the gate; added + `NanReport::skipped: usize` so the gate's effect is observable rather than a + silent no-op (the workspace's can-it-fire testing rule). +- `energy_all_finite`: filters to Energy-bearing rows before the finiteness `.all()`. +- Module doc corrected: the old "no branch on the value" framing is now precise — + the finiteness test itself stays branchless; the NEW branch is on schema + presence, documented as such rather than left to silently contradict the code. + +## Tests (both halves of the falsifiability rule) + +Fixtures switched from `NodeGuid::local(0)` (classid 0 / `DEFAULT`, temporarily +`Full`) to `NodeGuid::CLASSID_OSINT` (permanently `Cognitive`, no sunset) — a +fixture pinned to the temporary POC default would have silently gone vacuous the +moment that default reverts to `Bootstrap`. + +- **Can-it-fire (new):** `schema_gate_excludes_boards_whose_schema_omits_energy` + — real registered classids, not synthetic overrides: one `CLASSID_OSINT` row + (Cognitive, has Energy) + two `CLASSID_FMA` rows (Compressed, no Energy) with + their Energy-offset bytes poisoned to `NAN`/`INFINITY`. Asserts the poison IS + real (`f32_bits_nonfinite(energy_bits(...))` true on the raw read — proves the + test isn't vacuously passing because nothing was actually adversarial), then + asserts the gated sweep: `total == 1`, `skipped == 2`, `nonfinite.is_empty()`, + `is_clean()`, and `energy_all_finite` agrees. +- **Can-it-stay-silent (existing 2 tests, extended):** `finite_batch_is_clean` + and `nan_and_inf_are_flagged_neg_inf_too` now also assert `skipped == 0` — an + all-Energy-bearing batch is swept in full, proving the gate doesn't degrade + the pre-existing behaviour it's meant to leave alone. + +## Verification + +- `cargo test -p lance-graph-contract nan_projection --lib` → 4/4 green (the 3 + pre-existing + the 1 new adversarial test). +- `cargo test -p lance-graph-contract --lib` (full crate) → 1135/1135 green, no + collateral breakage from the added `NanReport` field. +- `cargo clippy -p lance-graph-contract --all-targets -- -D warnings` → clean. +- `cargo fmt -p lance-graph-contract` → ran, re-verified tests green after. +- `symbiont` (the one external caller, bin-only crate, heavy SurrealDB/OGAR/AMX + dependency tree, excluded from the default workspace): verified compile-safety + by direct code read (field/method access only, no destructure — see above) + rather than a full build, given the crate's build cost relative to the + verification value already established analytically. A `cargo check + --manifest-path crates/symbiont/Cargo.toml --bin symbiont` was also launched + to confirm end-to-end; see this file's companion commit / follow-up note for + its result if it completed within budget. + +## What I did NOT do + +- Did not touch `domino.rs`'s classid choice. Its rows are Bootstrap/classid-0 + today, which currently resolves to `Full` (has Energy) only because of the + TEMPORARY POC default pin — once that pin reverts to `Bootstrap`, domino.rs's + own `set_energy`/`energy_of` writes would themselves become a schema-contract + violation (writing into a tenant the row's own schema declares absent). That + is a SEPARATE, larger design decision (does the POC need a proper classid + mint into the shared `classid_read_mode` registry, or does it stay a private + raw-byte-region borrow outside the schema system entirely) that I did not make + unilaterally — flagging it as a follow-on rather than folding it into T5's + scope, which was specifically the sweepers. +- Did not change `ReadMode::DEFAULT`'s POC pin — out of scope, already tracked + by its own doc comment and the paired `ClassView` revert it names. diff --git a/crates/lance-graph-contract/src/nan_projection.rs b/crates/lance-graph-contract/src/nan_projection.rs index 78cf829fa..8032063c3 100644 --- a/crates/lance-graph-contract/src/nan_projection.rs +++ b/crates/lance-graph-contract/src/nan_projection.rs @@ -6,14 +6,34 @@ //! *project* the SoA's f32 accumulator tenant ([`ValueTenant::Energy`]) through //! this surface to flag any non-finite board. //! -//! It is the **fastest** possible NaN hunt over the SoA: a fixed-offset, +//! The finiteness test itself stays the fastest possible read: a fixed-offset, //! fixed-stride read of one 4-byte `f32` per [`NodeRow`], decided by a single -//! integer exponent mask — **no float load, no branch on the value, SIMD-friendly**. +//! integer exponent mask — **no float load, no branch on the value**. //! `Energy` is F32 precisely because F32 is the fast tenant (half of f64, and the //! NaN test is one `&`-compare on the bit pattern). //! +//! **Schema-gated (T5 closure, 2026-07-29).** `value_offset()` is a fixed +//! reserved position — the SAME byte range regardless of which [`ValueSchema`] +//! a row resolves to (RESERVE, DON'T RECLAIM) — so reading it is never memory- +//! unsafe. But a row whose resolved schema does NOT materialise `Energy` (e.g. +//! [`ValueSchema::Compressed`], used by [`NodeGuid::CLASSID_FMA`]) has no +//! writer obligated to keep that reserved range meaningful; a schema-blind sweep +//! would silently misread whatever bytes happen to sit there as if they were a +//! real energy accumulator — a false non-finite flag, or worse, a false-clean +//! pass over real corruption elsewhere in the slab that a NaN-shaped bit pattern +//! happened to zero out. Each row is therefore gated on its OWN resolved +//! `[ValueSchema::has]` before its `Energy` bytes are read at all — one branch +//! per row on schema presence (not on the float value), so the branchless +//! finiteness test survives unchanged for every row it actually applies to. +//! [`NanReport::skipped`] makes the gate's effect observable rather than a +//! silent no-op, per the workspace's can-it-fire testing rule. +//! //! This is "BindSpace as projection surface": the only surviving role of the old //! singleton is to answer "did any node go non-finite this cycle?" over the SoA. +//! +//! [`ValueSchema`]: crate::canonical_node::ValueSchema +//! [`ValueSchema::has`]: crate::canonical_node::ValueSchema::has +//! [`NodeGuid::CLASSID_FMA`]: crate::canonical_node::NodeGuid::CLASSID_FMA use crate::canonical_node::{NodeRow, ValueTenant}; @@ -27,14 +47,25 @@ pub const fn f32_bits_nonfinite(bits: u32) -> bool { /// The result of projecting an SoA batch onto the NaN-detection surface. #[derive(Debug, Clone, Default, PartialEq, Eq)] pub struct NanReport { - /// Total boards swept. + /// Boards whose `Energy` tenant was actually inspected (their resolved + /// schema materialises `Energy`). Excludes [`Self::skipped`]. pub total: usize, - /// Board indices whose `Energy` tenant is non-finite (NaN or Inf). + /// Board indices whose `Energy` tenant is non-finite (NaN or Inf). A subset + /// of the inspected (non-skipped) boards. pub nonfinite: Vec, + /// Boards whose resolved schema does NOT materialise `Energy` — excluded + /// from the finiteness question entirely (not counted as clean, not + /// counted as dirty; genuinely not-applicable). Nonzero only in batches + /// mixing classids whose read-mode omits `Energy` (e.g. + /// [`ValueSchema::Compressed`]) with ones that carry it. + /// + /// [`ValueSchema::Compressed`]: crate::canonical_node::ValueSchema::Compressed + pub skipped: usize, } impl NanReport { - /// No board went non-finite. + /// No inspected board went non-finite. (Silent about `skipped` boards by + /// design — they were never inspected, so they cannot make the sweep dirty.) #[inline] pub fn is_clean(&self) -> bool { self.nonfinite.is_empty() @@ -48,6 +79,8 @@ impl NanReport { } /// Read one board's `Energy` tenant as a raw `f32` bit pattern (no float load). +/// Caller MUST have already confirmed the row's schema materialises `Energy` +/// ([`row_has_energy`]) — this function does not gate. #[inline] fn energy_bits(row: &NodeRow) -> u32 { let off = ValueTenant::Energy.value_offset(); @@ -59,26 +92,45 @@ fn energy_bits(row: &NodeRow) -> u32 { ]) } +/// Does this row's OWN resolved schema materialise `Energy`? The one branch +/// this module adds — on schema presence, never on the float value. +#[inline] +fn row_has_energy(row: &NodeRow) -> bool { + row.key.read_mode().value_schema.has(ValueTenant::Energy) +} + /// Project a batch of canonical boards onto the NaN-detection surface by reading -/// each one's `Energy` tenant. Read-only; returns the indices of non-finite boards. +/// each one's `Energy` tenant — schema-gated per row (see module docs). Read-only; +/// returns the indices of non-finite boards among those actually inspected. /// This is the demoted singleton BindSpace — a projection, never a carrier. pub fn project_energy_nonfinite(rows: &[NodeRow]) -> NanReport { + let mut total = 0usize; + let mut skipped = 0usize; let mut nonfinite = Vec::new(); for (i, row) in rows.iter().enumerate() { + if !row_has_energy(row) { + skipped += 1; + continue; + } + total += 1; if f32_bits_nonfinite(energy_bits(row)) { nonfinite.push(i as u32); } } NanReport { - total: rows.len(), + total, nonfinite, + skipped, } } /// Fast clean/dirty answer without materialising the index list — the cheapest -/// projection (early-outs on the first non-finite board). +/// projection (early-outs on the first non-finite board). Rows whose schema +/// omits `Energy` are skipped, not treated as a violation. pub fn energy_all_finite(rows: &[NodeRow]) -> bool { - rows.iter().all(|row| !f32_bits_nonfinite(energy_bits(row))) + rows.iter() + .filter(|row| row_has_energy(row)) + .all(|row| !f32_bits_nonfinite(energy_bits(row))) } #[cfg(test)] @@ -86,9 +138,20 @@ mod tests { use super::*; use crate::canonical_node::{EdgeBlock, NodeGuid}; + // Fixtures use `CLASSID_OSINT` (a stable, permanently-Cognitive read-mode + // that has always materialised `Energy`) rather than `NodeGuid::local(0)` + // (classid 0 / DEFAULT). `ReadMode::DEFAULT` is documented as a TEMPORARY + // POC pin to `ValueSchema::Full`, scheduled to flip back to `Bootstrap` + // (no tenants) once the POC ends — a test fixture pinned to DEFAULT would + // silently go vacuous (every row skipped, `is_clean()` trivially true) on + // that flip. `CLASSID_OSINT`'s Cognitive schema carries no such sunset. fn board_with(energy: f32) -> NodeRow { + board_with_classid(NodeGuid::CLASSID_OSINT, energy) + } + + fn board_with_classid(classid: u32, energy: f32) -> NodeRow { let mut row = NodeRow { - key: NodeGuid::local(0), + key: NodeGuid::new(classid, 0, 0, 0, 0, 0), edges: EdgeBlock::default(), value: [0u8; 480], }; @@ -103,6 +166,9 @@ mod tests { let r = project_energy_nonfinite(&rows); assert!(r.is_clean()); assert_eq!(r.total, 8); + // Inertness half of the T5 gate: an all-Energy-bearing batch is swept + // in full — the schema gate skips nothing here. + assert_eq!(r.skipped, 0); assert!(energy_all_finite(&rows)); } @@ -118,6 +184,7 @@ mod tests { let r = project_energy_nonfinite(&rows); assert_eq!(r.nonfinite, vec![1, 2, 4]); assert_eq!(r.count(), 3); + assert_eq!(r.skipped, 0); assert!(!r.is_clean()); assert!(!energy_all_finite(&rows)); } @@ -132,4 +199,48 @@ mod tests { ]; assert!(project_energy_nonfinite(&rows).is_clean()); } + + // ── T5 closure: the schema gate on the two fixed-offset sweepers ────────── + + #[test] + fn schema_gate_excludes_boards_whose_schema_omits_energy() { + // Real, registered classids — not a synthetic override — so this + // exercises the actual `classid_read_mode` registry, not a stand-in. + // CLASSID_OSINT → Cognitive (has Energy). CLASSID_FMA → Compressed + // (Fingerprint + Helix + Turbovec + EntityType — no Energy). + let clean = board_with_classid(NodeGuid::CLASSID_OSINT, 1.0); + let poisoned_but_out_of_schema_1 = board_with_classid(NodeGuid::CLASSID_FMA, f32::NAN); + let poisoned_but_out_of_schema_2 = board_with_classid(NodeGuid::CLASSID_FMA, f32::INFINITY); + + // Prove the poison is real: an ungated read of these same reserved + // bytes IS non-finite. If this assertion ever failed, the test below + // would pass for the wrong reason (nothing to gate against). + assert!(f32_bits_nonfinite(energy_bits( + &poisoned_but_out_of_schema_1 + ))); + assert!(f32_bits_nonfinite(energy_bits( + &poisoned_but_out_of_schema_2 + ))); + + let rows = vec![ + clean, + poisoned_but_out_of_schema_1, + poisoned_but_out_of_schema_2, + ]; + + let r = project_energy_nonfinite(&rows); + // Falsifier: without the gate, `total` would be 3 and `nonfinite` + // would contain indices 1 and 2. + assert_eq!(r.total, 1, "only the OSINT/Cognitive board is inspected"); + assert_eq!(r.skipped, 2, "both FMA/Compressed boards are out of schema"); + assert!( + r.nonfinite.is_empty(), + "the poisoned bytes must never surface — they aren't Energy under this row's schema" + ); + assert!(r.is_clean()); + assert!( + energy_all_finite(&rows), + "energy_all_finite must agree with project_energy_nonfinite" + ); + } } From 94f771a2bba44f3318ae07aa50ac3856e440cf75 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 20:48:52 +0000 Subject: [PATCH 2/2] docs: address codex review on #873 -- precise cost claim, cross-ref bridge.rs Two findings from codex's review, both confirmed by direct source inspection before acting, neither a correctness bug in the gate itself. 1. The module doc's "the branchless finiteness test survives unchanged" was true but readable as a broader "this is still free" claim than intended. row_has_energy() calls NodeGuid::read_mode() -> classid_read_mode(), which is a HashMap lookup behind a LazyLock, not a bitmask -- confirmed by reading canonical_node.rs:1473. That is real added per-row cost, on top of the old four-byte load, that can plausibly dominate it for an in-cache homogeneous batch. Doc now says so explicitly and names the exact call chain, rather than leaving "branchless" to imply "costless." Not benchmarked -- flagged as the first place to look if this projection ever lands on a genuinely hot path. 2. codex found a sibling instance of the exact ISS-DOMINO pattern in crates/symbiont/src/bridge.rs: board() mints via NodeGuid::local(idx), set_energy/energy read/write Energy at its fixed offset with no schema.has() gate. Confirmed by reading the file. The distinction codex drew and I verified: bridge.rs never calls project_energy_nonfinite or energy_all_finite at all -- its own tests check finiteness via plain f32::is_finite() on the value straight out of energy(), bypassing nan_projection.rs entirely. So T5's gate doesn't touch bridge.rs's behaviour in either direction; its exposure is the write-side schema- contract violation on the same ReadMode::DEFAULT POC pin, felt differently (no vacuous-skip risk, since it never calls the gated functions -- just a value that stops meaning what its schema claims once the pin flips). Added as a labelled addendum to the existing ISS-DOMINO entry rather than a second issue, since the fix is the same architecture call (mint vs. explicit-unchecked) applied to a second call site. cargo test -p lance-graph-contract nan_projection --lib: 4/4 green, unchanged (doc-only change plus a governance-file addendum). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01K3RyLEbuNSHxxB3NTTrGki --- .claude/board/ISSUES.md | 23 +++++++++++++++++++ .../src/nan_projection.rs | 19 ++++++++++++--- 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/.claude/board/ISSUES.md b/.claude/board/ISSUES.md index fc0cb6e5d..8c6afce72 100644 --- a/.claude/board/ISSUES.md +++ b/.claude/board/ISSUES.md @@ -49,6 +49,29 @@ option) that the T5 scope didn't license me to make unilaterally. Flagging so th POC-default flip (whenever it lands) doesn't silently turn domino.rs's own correctness assertion into theater. +> **⊘ ADDENDUM (codex review on PR #873, 2026-07-30) — a sibling instance, +> not a second issue.** `crates/symbiont/src/bridge.rs` has the identical root +> cause: `board()` mints via `NodeGuid::local(idx)` (classid 0), and +> `set_energy`/`energy` read/write the `Energy` tenant at its fixed offset with +> no `schema.has()` gate. Codex's distinction from the domino.rs case: **bridge.rs +> never calls `project_energy_nonfinite`/`energy_all_finite` at all** — its own +> tests (`each_bus_is_one_soa_node_with_finite_energy_tenant`, +> `scale_to_16k_boards_is_8_mib_zero_copy`) check finiteness via plain +> `f32::is_finite()` on the value read straight out of `energy()`, bypassing +> `nan_projection.rs` entirely. So T5's schema gate does not touch bridge.rs's +> behaviour today, in either direction — it is neither protected by the gate +> (like a hypothetical schema-respecting caller would be) nor exposed to the +> vacuous-skip risk described above (since it never calls the gated functions). +> Its exposure is identical to domino.rs's on the SAME `ReadMode::DEFAULT` POC +> pin, just felt differently: when that pin flips to `Bootstrap`, bridge.rs's +> `set_energy`/`energy` keep writing/reading a byte range its own row's declared +> schema no longer materialises — the write-side schema-contract violation, not +> a report-vacuity one (nothing here would start silently passing; the value +> read back would just no longer mean what the row's schema claims it means). +> Tracked here rather than as a separate issue because the fix is the same +> architecture call (mint vs. explicit-unchecked) applied to a second call site — +> whichever of the two live options above gets chosen should cover both files. + ## ISS-NO-PER-THREAD-TEMPORAL-PROJECTION-IS-EVER-CONSTRUCTED (2026-07-29) — OPEN, UPSTREAM OF `meta_basin` > **⊘ RE-GRADED AND RELOCATED (Codex, #869) — my first filing mislocated this.** diff --git a/crates/lance-graph-contract/src/nan_projection.rs b/crates/lance-graph-contract/src/nan_projection.rs index 8032063c3..76665eeb1 100644 --- a/crates/lance-graph-contract/src/nan_projection.rs +++ b/crates/lance-graph-contract/src/nan_projection.rs @@ -22,9 +22,20 @@ //! real energy accumulator — a false non-finite flag, or worse, a false-clean //! pass over real corruption elsewhere in the slab that a NaN-shaped bit pattern //! happened to zero out. Each row is therefore gated on its OWN resolved -//! `[ValueSchema::has]` before its `Energy` bytes are read at all — one branch -//! per row on schema presence (not on the float value), so the branchless -//! finiteness test survives unchanged for every row it actually applies to. +//! `[ValueSchema::has]` before its `Energy` bytes are read at all. +//! +//! **What "branchless" still means after the gate, precisely (codex review, +//! 2026-07-30).** The FINITENESS TEST — the exponent-mask compare on the +//! four already-loaded bytes — is unchanged: still zero branches on the +//! value. That is not the same claim as "the sweep costs what it did +//! before." [`row_has_energy`] calls [`NodeGuid::read_mode`], which resolves +//! through [`classid_read_mode`] — a `HashMap` lookup behind a `LazyLock`, +//! not a bitmask. That lookup is real per-row work, added on top of the old +//! four-byte load, and can plausibly dominate it for an in-cache homogeneous +//! batch. Not benchmarked; do not read "branchless" below as "free" — if +//! this projection lands on a genuinely hot path, that lookup is the first +//! place to look before assuming the schema gate is costless. +//! //! [`NanReport::skipped`] makes the gate's effect observable rather than a //! silent no-op, per the workspace's can-it-fire testing rule. //! @@ -34,6 +45,8 @@ //! [`ValueSchema`]: crate::canonical_node::ValueSchema //! [`ValueSchema::has`]: crate::canonical_node::ValueSchema::has //! [`NodeGuid::CLASSID_FMA`]: crate::canonical_node::NodeGuid::CLASSID_FMA +//! [`NodeGuid::read_mode`]: crate::canonical_node::NodeGuid::read_mode +//! [`classid_read_mode`]: crate::canonical_node::classid_read_mode use crate::canonical_node::{NodeRow, ValueTenant};