D-MAR-1: FieldMask/WideFieldMask difference + is_subset_of (re-derived from plan) - #1099
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Included review availability: 1 review is currently available. Your included PR review attempts over the past 7 days set your current allowance at 3 reviews per hour. 📝 WalkthroughWalkthrough
ChangesField mask algebra
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This PR adds localized mask difference and subset operations and removes an internal allocation path; no actionable merge-blocking risk remains. Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
…-first) Re-derived from mask-algebra-revision-read-v1 and from source on current main. The reverted #1088 diff was not consulted and is not a citable input (see PR_ARC_INVENTORY, the #1093 entry). Adds the two operators the plan's Sec 1.1 measured absent. FieldMask gains const-fn difference (self & !other) and is_subset_of, mirroring the existing intersect/union/is_disjoint exactly -- same #[inline] const fn shape, Copy preserved, no existing signature touched. WideFieldMask gains the same pair with the (Small, Small) fast path plus the tier-agnostic fold, whose normalization (trailing-zero trim + demote to Small) is load-bearing for the canonical PartialEq/Hash contract from #651. Why they are on the critical path, per plan Sec 0 F4 (the substrate-first STOP rule): a provenance surface is built from set difference and containment, and the rejected external draft hand-rolled an entire EvidenceMask trait precisely because these two methods were absent. The fix is the two methods, not the trait. Also removes chunks_view, which cloned BOTH operands into fresh Vecs on every wide op. zip_fold now reads operands in place through chunk_at; the only allocation is the owned result, which an owned-result operator must produce anyway. chunks_view was private with exactly two call sites, both inside zip_fold, so this has no external surface. intersect and union inherit the fix. Its old materialization role is superseded by the separate stencil arena (operator-ruled: arena separate, masks stay values), which is its own change and is not in this PR. Gates G1-G4 from plan Sec 3, each with the anti-half the plan requires, since G1 alone is satisfied by difference == EMPTY and G2 alone by is_subset_of == true. Disable-runs, all verified red-then-green: - difference returns EMPTY -> G1-anti red, G1 still green - is_subset_of returns true -> G2-anti red - zip_fold length max -> min -> G3 red (cross-width both orders) - trim/demote bypassed -> G4 red, plus two PRE-EXISTING canonical-form tests red, so the rewritten fold is covered by the existing regression suite as well GATE CORRECTION, measured. The plan's stated G4 disable-run -- bypass trim/demote, expect the Small-equivalent comparison to fail -- is UNFALSIFIABLE as written. Since #651 gave WideFieldMask a representation-independent PartialEq/Hash, equality survives the bypass by design; the assertion cannot fail. G4 therefore asserts max_fields() == 64 on the demoted result, which is an observable that CAN fail, and the disable-run above confirms it does. The plan needs a dated storno on that gate line. Verification: contract lib 1251 passed / 0 failed; clippy --all-targets -D warnings clean; fmt clean. Not in this PR: D-MAR-2 (RevisionKind) is gated on the plan's Sec 5 Q1 module-home ruling, per its own ordering rule F6. Below this change, AdaWorldAPI/ndarray now carries the masking primitives these operators compose with -- U64x8/U32x16 andnot and ternlog, the latter lowering to a single vpternlogq on AVX-512 (branch claude/medcare-rs-continue-ufsazd, commits fd5c66f and fbe2d36). Nothing here depends on that landing first. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KCGhDYoQBXs3poaR7sFuqp
29a0902 to
ad1c35d
Compare
Re-derives D-MAR-1 from
mask-algebra-revision-read-v1and from source on currentmain. The reverted #1088 diff was not consulted — per the#1093arc entry, the six reverted commits are not citable inputs.What lands
FieldMaskgainsdifference(self & !other) andis_subset_of, both#[inline] pub const fn, mirroring the existingintersect/union/is_disjointexactly —Copypreserved, no existing signature touched.WideFieldMaskgains the same pair with the(Small, Small)fast path plus the tier-agnostic fold, whose trim-and-demote normalization is load-bearing for the canonicalPartialEq/Hashcontract from #651.Why they're on the critical path, per plan §0 F4 (substrate-first STOP): a provenance surface is built from set difference and containment, and the rejected external draft hand-rolled an entire
EvidenceMasktrait because these two methods were absent. The fix is the methods, not the trait.Also removes
chunks_view, which cloned both operands into freshVecs on every wide op.zip_foldnow reads operands in place viachunk_at; the only allocation is the owned result, which an owned-result operator must produce anyway. It was private with exactly two call sites, both insidezip_fold, so there is no external surface — andintersect/unioninherit the fix. Its materialization role is superseded by the separate stencil arena (operator-ruled: arena separate, masks stay values), which is its own change and is not in this PR.Gates
G1–G4 from plan §3, each with the anti-half the plan requires — G1 alone is satisfied by
difference ≡ EMPTY, G2 alone byis_subset_of ≡ true. All four disable-runs verified red-then-green:differencereturnsEMPTYis_subset_ofreturnstruezip_foldlengthmax→minGate correction, measured. The plan's stated G4 disable-run — bypass trim/demote, expect the
Small-equivalent comparison to fail — is unfalsifiable as written. Since #651 gaveWideFieldMaska representation-independentPartialEq/Hash, equality survives the bypass by design; that assertion cannot fail. G4 therefore assertsmax_fields() == 64on the demoted result, an observable that can fail, and the disable-run confirms it does. The plan needs a dated ⊘ on that gate line.Verification
contract lib 1251 passed / 0 failed;
clippy --all-targets -D warningsclean; fmt clean.Not here: D-MAR-2 (
RevisionKind) is gated on the plan's §5 Q1 module-home ruling, per its own ordering rule F6.Below this,
AdaWorldAPI/ndarraynow carries the masking primitives these compose with —U64x8/U32x16andnot+ternlog, the latter lowering to a singlevpternlogqon AVX-512 (fd5c66f,fbe2d36). Nothing here depends on that landing first.Generated by Claude Code
Summary by CodeRabbit
New Features
Quality Improvements