board: record #1099 + #1101, storno the G4 gate, file the sigker test break - #1102
Conversation
… break Board hygiene for two merges that landed without it (the deliverable commits were already merged, so this is the retroactive pattern CLAUDE.md names an anti-pattern -- doing it promptly rather than not at all). PR_ARC_INVENTORY: entries for #1099 and #1101, prepended. LATEST_STATE: contract-inventory delta for #1099 (+4 methods, chunks_view removed, no new type, no layout change) and the #1101 fmt/gate record. STATUS_BOARD: D-MAR-1..2 row split -- D-MAR-1 Shipped (ae24f6e), D-MAR-2 still Proposed and blocked on the plan's Sec 5 Q1 module-home ruling. Plan Sec 3: dated storno on G4. The gate was UNFALSIFIABLE and had been since #651 -- not wrong, inert. It asserted that bypassing zip_fold's trim/demote breaks the Small-equivalent comparison, but #651 made PartialEq/Hash representation-independent, so equality survives the bypass by design and the assertion could not go red whatever the fold did. Replaced with max_fields() == 64 on the demoted result, verified red under the bypass. Generalized in the entry because it will recur: a gate written against an invariant that a later change made representation-independent stops being a gate without anyone editing it, and no review of the gate alone can catch that -- only running the disable-run can. ISSUES: ISS-SIGKER-TESTS-HAVE-NEVER-COMPILED. sigker's test build fails E0277 (sorts a (Vec<usize>, f64); f64 is not Ord), verified against unmodified source so it predates #1101. Workspace exclusion meant no CI job ever built it -- the same exclusion that let its formatting drift. #1101 gated formatting only, deliberately: arming a test gate on a crate that cannot compile its tests would fail every later PR for a defect it did not introduce. The fix needs someone to read the algorithm and decide a NaN policy (partial_cmp vs total_cmp), so it is filed, not guessed at. SUPERSESSION-INDEX regenerated AFTER the board writes, per CLAUDE.md -- the board is one of the generator's inputs, so regenerating before them produces a byte-identical file that reads as current and then fails CI on the merge commit. Post-check (wc -l, per the never-truncate law): PR_ARC 6028 -> 6062, LATEST_STATE 2717 -> 2747, ISSUES grew, STATUS_BOARD 1635 -> 1636. No append-only file got shorter.
|
Warning Review limit reachedNext included review available in 25 minutes. View limit detailsLimit details: You’ve used all 3 included reviews currently available. Your 42 included PR review attempts over the past 7 days set your current allowance at 3 reviews per hour. Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f3484ae59
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| - **`class_view::FieldMask::{difference, is_subset_of}`** — `#[inline] pub const fn`, mirroring the existing `intersect`/`union`/`is_disjoint` shape exactly; `Copy` preserved, no existing signature touched. `a.difference(b)` = `a & !b`; `a.is_subset_of(b)` = `a & !b == 0`. | ||
| - **`class_view::WideFieldMask::{difference, is_subset_of}`** — same pair, `(Small, Small)` fast path plus the tier-agnostic fold. Argument order deliberately **differs from the raw Intel intrinsic**: `_mm*_andnot_si*(a, b)` is `!a & b`, ours is `self & !other` ("self minus other"), documented at every definition. | ||
| - **`chunks_view` REMOVED** (private, 2 call sites, both in `zip_fold`). It cloned *both* operands into fresh `Vec`s per wide op; `zip_fold` now reads in place via `chunk_at` and allocates only the owned result. `intersect`/`union` inherit the fix. Its materialization role is superseded by the stencil arena (operator-ruled 2026-08-31: **arena separate, masks stay values** — so `WideFieldMask`'s public surface is unchanged and the 11 files across 4 crates that consume it, RBAC's `field_mask()` included, are untouched). |
There was a problem hiding this comment.
Remove RBAC from the WideFieldMask consumer census
A repo-wide search finds the 11 WideFieldMask files in lance-graph-contract, lance-graph-ogar, lance-graph-planner, and lance-graph; none is in lance-graph-rbac. RBAC's field_mask() and its authorization path use the narrow FieldMask, so saying RBAC is included incorrectly records that it already consumes the wide-mask API and could mislead later adoption work.
Useful? React with 👍 / 👎.
| > bypass, green without it. The bypass additionally reddens two pre-existing | ||
| > canonical-form tests (`intersect_result_collapsing_to_small_equals_canonical_and_hashes_identically`, | ||
| > `intersect_union_across_tiers`), so the fold is guarded by the existing |
There was a problem hiding this comment.
Correct the claimed legacy-test coverage
Under the documented trim/demote bypass, intersect_union_across_tiers remains green: it checks populated bits/counts and equality between the two operand orders, which both retain equivalent Wide representations, while canonical PartialEq ignores trailing zero chunks. Of the two named tests, only intersect_result_collapsing_to_small_equals_canonical_and_hashes_identically directly asserts a Small representation and catches this bypass, so the plan currently overstates the independent regression coverage.
Useful? React with 👍 / 👎.
Board hygiene for two merges that landed without it. The deliverable commits are already on
main, so this is the retroactive patternCLAUDE.mdnames an anti-pattern — done promptly rather than not at all.Records
PR_ARC_INVENTORY— entries for D-MAR-1: FieldMask/WideFieldMask difference + is_subset_of (re-derived from plan) #1099 and fmt: bring jc / sigker / thinking-engine to a clean baseline and gate them #1101, prepended.LATEST_STATE— D-MAR-1: FieldMask/WideFieldMask difference + is_subset_of (re-derived from plan) #1099's contract-inventory delta (+4 methods,chunks_viewremoved, no new type, no layout change) and fmt: bring jc / sigker / thinking-engine to a clean baseline and gate them #1101's fmt/gate record.STATUS_BOARD—D-MAR-1..2split: D-MAR-1 Shipped (ae24f6e5); D-MAR-2 still Proposed, blocked on the plan's §5 Q1 module-home ruling per ordering rule F6.The G4 storno — a gate that stopped being one without being edited
The plan's G4 disable-run was unfalsifiable, and had been since #651 — not wrong, inert. It asserted that bypassing
zip_fold's trim/demote would break theSmall-equivalent comparison. But #651 gaveWideFieldMaska representation-independentPartialEq/Hash, so equality survives the bypass by design: the assertion could not go red whatever the fold did.Replaced with
max_fields() == 64on the demoted result — representation-dependent, and verified red under the bypass.Generalized in the entry because it will recur: a gate written against an invariant that a later change made representation-independent stops being a gate without anyone editing it. Nothing about G4's text became false when #651 landed. Falsifiability is a joint property of the gate and the code under it, so it decays silently — and no review of the gate alone can catch it. Only running the disable-run can, which is why the workspace requires red-then-green rather than green.
ISS-SIGKER-TESTS-HAVE-NEVER-COMPILEDsigker's test build fails E0277 — it sorts a(Vec<usize>, f64)andf64is notOrd. Verified against unmodified source, so it predates #1101;crates/sigker/srcwas last touched 2026-05-07.Workspace exclusion meant no CI job ever built it — the same exclusion that let its formatting drift. Exclusion silences every gate, not just the one you were thinking about; the formatting drift was the visible symptom and this is the load-bearing one.
#1101 gated formatting only, deliberately: arming a test gate on a crate that cannot compile its tests would fail every later PR for a defect it did not introduce (the reasoning
style.ymlalready records forcausal-edge's clippy debt). The fix needs someone to read the algorithm and choose a NaN policy (partial_cmpvstotal_cmp), so it is filed, not guessed at.Index
SUPERSESSION-INDEXregenerated after the board writes, perCLAUDE.md— the board is one of the generator's inputs, so regenerating first yields a byte-identical file that reads as current and then fails CI on the merge commit.Post-check
Per the never-truncate law:
PR_ARC6028 → 6062,LATEST_STATE2717 → 2747,ISSUESgrew,STATUS_BOARD1635 → 1636. No append-only file got shorter.Generated by Claude Code