alpha: make the AlphaMask::zip length guard hold in release (+ #1198 post-merge board hygiene) - #1199
Merged
Merged
Conversation
The only guard on `zip`'s operands was `debug_assert_eq!`, which is compiled out of a release build — the build where the damage is silent. The failure is worse than a truncation. Iterator `zip` stops at the shorter operand while `len` is copied from `self`, so `wide.and(&narrow)` returned a mask claiming `self.len` addresses backed by `other.words.len()` words. That is not a smaller mask, it is an invalid one, and it fails two different ways at a distance from the call that made it: `count`/`is_empty` under-report silently, while `contains`/`materialize_ordinals` index past the slice and panic somewhere else entirely. Measured before it was fixed, in release: the `should_panic` falsifier reported "test did not panic as expected" — the defect, reproduced rather than argued. A length mismatch is a caller mixing two allocations: a programming error, not a data condition. So it fails closed at the operation that made it, which costs one u32 comparison against a loop over every word. No API change, and the ops have no external callers yet, so this carries no ripple. Two falsifiers, both disable-verified red-then-green. They are two-sided on purpose: the mismatch case proves the guard fires in release, the equal-length case proves it stays silent on ordinary input (including len % 64 != 0, where the tail word is partial), so a guard that rejected everything could not pass both. The silent half also asserts each product carries a full-width word slice, which is the invariant the guard actually protects. Board hygiene for #1198, now that it has merged and the post-merge trigger has fired: PR_ARC_INVENTORY entry and the LATEST_STATE merged-PR table, both prepended. This is the record I said on the review thread would land after merge rather than in the PR under review. Stage 1b of the temporal plan is marked DONE with the measurement, and STATUS_BOARD carries D-ALPHA-1b. Supersession index regenerated after the board writes, per the ordering rule; output byte-identical. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EfrUJH3UNnv5NpDH4jDGHq
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Essentials Run ID: 📒 Files selected for processing (5)
Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour. 📝 WalkthroughWalkthrough
ChangesAlphaMask validation
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to Mask operations now reject mismatched lengths in release builds instead of producing invalid results, while equal-length operations remain covered. No current merge-blocking risk remains. Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_2961f202-0d5c-4fba-aa64-487b731ff8fb) |
AdaWorldAPI
marked this pull request as ready for review
September 6, 2026 09:02
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
AdaWorldAPI
pushed a commit
that referenced
this pull request
Sep 6, 2026
The post-merge trigger fired when #1199 merged as ef72487. #1199 was a MIXED PR — it landed the AlphaMask::zip guard as well as #1198's hygiene — so the rule's own terms give it an entry: the non-hygiene half is what the entry is for. The arc entry keeps what a future session would otherwise have to re-derive: that the defect was reproduced in release BEFORE the fix (the should_panic falsifier reporting "test did not panic as expected" IS the reproduction), that the failure was an invalid mask rather than a truncated one, and that the blast radius was measured — and/or/xor/and_not have zero external callers — before a hard assert was committed to a contract crate. It also records what the clean review was and was not worth: CodeRabbit returned no actionable comments on the correct range, but Cursor Bugbot and Codex were both budget non-results, so the arc had one live reviewer rather than three. The evidence this rests on is the release repro, not the review. This commit is hygiene-only. Per CLAUDE.md's termination clause it therefore generates NO further obligations — no arc entry and no LATEST_STATE row of its own — and the chain stops here rather than recursing. Supersession index regenerated after the board writes, per the ordering rule; output byte-identical. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EfrUJH3UNnv5NpDH4jDGHq
AdaWorldAPI
added a commit
that referenced
this pull request
Sep 6, 2026
board: record #1199's merge (hygiene-only — the chain stops here)
AdaWorldAPI
pushed a commit
that referenced
this pull request
Sep 7, 2026
…EPIPHANIES entry All three verified against the files before fixing; all three were right. 1. LATEST_STATE's #1201 heading still read "#1199's records + the council SPEC v1" directly above the row correcting exactly that claim. A reader scanning by heading received the false merge history the commit exists to repair — the sharpest of the three, because the correction was sitting one line under the thing it contradicted. 2. STATUS_BOARD's D-NGN-AUDIT row still read "Phase 1 cast; v2 consolidation + 3 reviewers pending". Phase 1 has RETURNED; the council is held at Phase 2 on a VIOLATES against F5 that is the operator's to rule. The dashboard is what a later session reads first, so a stale phase there invites re-casting five savants that already reported. The row now says HELD, why, and — explicitly — do not re-cast Phase 1 and do not cast the reviewers until the ruling lands. 3. No EPIPHANIES entry for the correction. CLAUDE.md's board-hygiene table requires a finding/correction to prepend one in the SAME commit, and the generalizable half was living only in an arc entry, which is organized by PR rather than by finding. Added E-AN-EMPTY-RANGE-AFTER-A-RESET-IS-NOT-EVIDENCE-1: `origin/main..<branch>` answers a question about the branch's CURRENT tip, so after a reset it is empty by construction — the symptom of the loss is indistinguishable from the all-clear. Verify unmerged work against a ref the operation cannot move. It carries the second half too, which no gate catches today: the board cited a PATH, not a symbol, so a plan file that existed nowhere read as a valid reference for a day. An absent symbol fails citation-decay; an absent path does not. Gates run locally: plan_dids exit 0; append-only verified by line count (EPIPHANIES 27617 -> 27661, PR_ARC unchanged); supersession index regenerated AFTER the board writes — EPIPHANIES is one of its inputs — and byte-identical. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EfrUJH3UNnv5NpDH4jDGHq
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stage 1b of
.claude/temporal/09-plan.md, plus the post-merge board records #1198 owes now that it has merged.The defect
AlphaMask::zip's only length guard wasdebug_assert_eq!— compiled out of a release build, which is the build where the damage is silent.The failure is worse than a truncation. Iterator
zipstops at the shorter operand whilelenis copied fromself, sowide.and(&narrow)returned a mask claimingself.lenaddresses backed byother.words.len()words. That is not a smaller mask, it is an invalid one, and it fails two different ways at a distance from the call that made it:count/is_emptysum the short slice and under-report silently;contains/materialize_ordinalspass theordinal < self.lencheck, then index past the slice and panic somewhere else entirely.Measured before it was fixed
In release, against the unfixed code, the runner said exactly what a reproduced defect looks like:
wide.and(&narrow)had returned successfully. Fixed, both falsifiers pass in release and debug.The fix
assert_eq!, notdebug_assert_eq!. A length mismatch is a caller mixing two allocations — a programming error, not a data condition — so it fails closed at the operation that made it rather than becoming a wrong answer read later. Cost is oneu32comparison against a loop over every word.No API change.
and/or/xor/and_nothave no external callers yet (the only.and(hits elsewhere in the repo are wiremock), so this carries no ripple.Falsifiers — two-sided, both disable-verified
debug_assert_eq!The silence half runs
len = 200(% 64 != 0, so the tail word is partial) across all four ops and additionally assertsand().count() + and().not().count() == 200— that every product carries a full-width word slice, which is the invariant the guard actually protects.Board hygiene for #1198 (post-merge trigger)
This is the record I said on the #1198 review thread would land after merge rather than in the PR under review, and the merge has now fired that trigger:
PR_ARC_INVENTORY.md— prepended entry for temporal: the audit reference, and D-LNC-5a measured — the insert delta needs stable row ids #1198 (3797237b): the falsified hypothesis, all three probe results with their controls, the four self-corrections, and thewhen_matcheddefect the review round exposed.LATEST_STATE.md— merged-PR table entry; contract inventory net delta none (that arc minted nothing).STATUS_BOARD.md—D-ALPHA-1brow..claude/temporal/09-plan.md— Stage 1b marked DONE with the measurement.Gates
cargo test --release -p lance-graph-contract --lib alpha::— 10/10 (the release path is the point)cargo test -p lance-graph-contract --lib— 1318/1318cargo fmt -p lance-graph-contract -- --check— cleancargo clippy -p lance-graph-contract --all-targets -- -D warnings— cleanDraft: the fix and its evidence are complete, but the remaining temporal stages are still gated (Stage 3 on P1's red, Stage 2 unstarted), so there is no rush to merge this ahead of a look.
🤖 Generated with Claude Code
https://claude.ai/code/session_01EfrUJH3UNnv5NpDH4jDGHq
Generated by Claude Code
Summary by CodeRabbit