test: retain input failures on the untouched bottom (#2973)

Claude-Session: https://claude.ai/code/session_01BdjadZpWcXz8pW6TZ95fsv

solidcore/crates/solidcore/src/error/offset.rs

@@ -156,8 +156,8 @@ pub enum OffsetFailure {
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 #[non_exhaustive]
 pub enum Unsoundness {
-    /// The check at `SM_LEVEL_2` reports `failures` failure lines more than the input's
-    /// check did, counted per class
+    /// The check at `SM_LEVEL_2` reports `failures` failure lines absent at the same
+    /// entity sites in the input's check, grouped per class
     /// ([`crate::topology::brep::validation::BrepCheck::failures_beyond`]); the first such
     /// class is test `test_index` of the reporting type `reporting_type` (`None` for a line
     /// without a report: a failed status).

solidcore/crates/solidcore/src/topology/brep/local_operation.rs

@@ -83,8 +83,8 @@ fn null() -> SolidError {
 /// edges (seams) is given a surface its seams' two UV lines do not lift to one curve on
 /// (not closed in the seam's direction, or the lines not on its seam: `keeps_seams`),
 /// before anything changes; [`OffsetFailure::Unsound`] (not upstream) when the body built
-/// fails what `b` did not (`refuse_unsound`: a check failure class `b`'s check has fewer
-/// lines of, a geometry the check could not reach, a manifold solid lost, a volume not
+/// fails what `b` did not (`refuse_unsound`: a check failure at a new entity site,
+/// a geometry the check could not reach, a manifold solid lost, a volume not
 /// positive); those of the callees ([`crate::offset::offset_executive::p2`], the surfaces'
 /// normals, reversal and analytic copy, the bounding box). On every error `b` is as it
 /// came in, the caches its surfaces had included (the operation edits a copy of it), and
@@ -305,8 +305,8 @@ fn local_operation_on(
 /// fails what its input did not ([`validation::soundness`] of both, `input` measured
 /// before the edit, the body on a private copy):
 ///
-/// - the check at `SM_LEVEL_2` has more failure lines of some class (the failed test and
-///   the type of the object it is about, [`FailureClass`]) than the input's check had
+/// - the check at `SM_LEVEL_2` has a failure at an entity site the input did not have,
+///   matched by the failed test's [`FailureClass`] and the report's named objects
 ///   ([`BrepCheck::failures_beyond`](validation::BrepCheck::failures_beyond));
 /// - the body's pointer check failed, so its geometry was not checked (`AssertValid` is
 ///   skipped then), even where the input's pointer check failed alike;
@@ -317,13 +317,13 @@ fn local_operation_on(
 /// The owners the check tests the geometry against are the identities the body's naming
 /// scheme names its entities by ([`validation::standing`]), as `move_faces` sets them.
 ///
-/// Failures are compared by class, not by entity: the operation rebuilds every face (the
-/// extended offset builds them all, unmoved ones on copies of their surfaces, and they
-/// are copied into `b`), so no identity persists to compare by. A failure the input had
-/// and the body still has is the input's; one the input had on a face the operation
-/// rebuilt and the body no longer has is gone; a class the body has more of is the
-/// operation's. (A failure gone from one entity while one of its class appears on
-/// another is counted as the input's.)
+/// Failure sites rely on arena ids, not on caller-supplied geometry owner names:
+/// `standing`'s reports encode the entity kind and arena index. Private measurement
+/// copies keep those ids; `Arena` never reuses removed slots (validity-and-tolerances
+/// §3, invariant 1). `move_faces` removes every original face and copies the extended
+/// offset's faces into the same arenas, so both picked and unpicked faces are rebuilt
+/// with new ids. A failure on any such new entity is new: there is no correspondence
+/// proved by a matching count or by unchanged surface geometry.
 ///
 /// Each moved edge is found again where extended surfaces meet; where that cannot close
 /// the body (a surface that meets its neighbours nowhere near the old edges, a vertex
@@ -2085,6 +2085,43 @@ mod tests {
         );
     }
 
+    /// Clearing one face's failure cannot pay for the same failure on a newly built
+    /// face. Isolate the check comparison: the existing defective-roof test is refused
+    /// by the reach certificate before it can reach the final gate.
+    #[test]
+    fn the_gate_refuses_a_failure_transferred_to_another_face() {
+        use crate::geom::assert_array::labels::FACE_TYPE;
+        use crate::geom::assert_array::{AssertArray, AssertReport, ObjectKey, ReportSpec};
+        let check = |face: u32| {
+            let site = ObjectKey((FACE_TYPE as u64) << 32 | u64::from(face));
+            let mut reports = AssertArray::new();
+            reports.add_unique_delete_duplicate(
+                AssertReport::new(ReportSpec {
+                    reporter: Some(site),
+                    test_class: FACE_TYPE,
+                    test_index: 37,
+                    owner: Some(site),
+                    owner_type: FACE_TYPE,
+                    ..ReportSpec::default()
+                })
+                .unwrap(),
+            );
+            validation::BrepCheck {
+                pointers_status: crate::error::SM_SUCCESS,
+                pointer_reports: AssertArray::new(),
+                assert_valid: Some(false),
+                reports,
+            }
+        };
+        let input = check(0);
+        let result = check(6);
+        assert_eq!(result.failure_classes(), input.failure_classes());
+        assert!(
+            !result.failures_beyond(&input).is_empty(),
+            "the gate accepts a new Face #37 failure at face 6 in place of face 0"
+        );
+    }
+
     /// A body whose pointer check fails is not checked further (`AssertValid` is skipped):
     /// the gate refuses it even where its input failed the pointer check alike, rather
     /// than finding nothing beyond the input's failures.
@@ -2531,11 +2568,10 @@ mod tests {
         assert_eq!(failure_classes(&mut k, &b), had);
     }
 
-    /// [`BrepCheck::failures_beyond`](validation::BrepCheck::failures_beyond) counts per
-    /// class: a class the input has as many lines of is not listed, one more line of it
-    /// is, and a class the input lacks is.
+    /// Match by class and site before counting: retained sites pass, a new owner fails
+    /// even with fewer total lines, and the returned counts remain grouped by class.
     #[test]
-    fn failures_beyond_counts_lines_per_class() {
+    fn failures_beyond_matches_lines_per_class_and_site() {
         use crate::geom::assert_array::labels::SURFACE_TYPE;
         use crate::geom::assert_array::{AssertArray, AssertReport, ObjectKey, ReportSpec};
         // (each report about its own surface: the array keeps one of equal reports)
@@ -2549,9 +2585,9 @@ mod tests {
             })
             .unwrap()
         };
-        let check = |tests: &[u64]| {
+        let check = |tests: &[(u64, u64)]| {
             let mut reports = AssertArray::new();
-            for (owner, &t) in (0..).zip(tests) {
+            for &(t, owner) in tests {
                 assert!(reports.add_unique_delete_duplicate(report(t, owner)));
             }
             validation::BrepCheck {
@@ -2561,13 +2597,20 @@ mod tests {
                 reports,
             }
         };
-        let input = check(&[4, 4, 6]);
-        assert!(check(&[4, 6]).failures_beyond(&input).is_empty());
-        assert!(check(&[4, 4, 6]).failures_beyond(&input).is_empty());
-        let more = check(&[4, 4, 4, 6]).failures_beyond(&input);
+        let input = check(&[(4, 0), (4, 1), (6, 2)]);
+        assert!(check(&[(4, 0), (6, 2)]).failures_beyond(&input).is_empty());
+        assert!(check(&[(4, 0), (4, 1), (6, 2)])
+            .failures_beyond(&input)
+            .is_empty());
+        let more = check(&[(4, 0), (4, 1), (4, 3), (6, 2)]).failures_beyond(&input);
         assert_eq!(more.len(), 1);
         assert_eq!(more.first().map(|&(_, n)| n), Some(1));
-        let other = check(&[5]).failures_beyond(&input);
+        let transferred = check(&[(4, 3), (6, 2)]).failures_beyond(&input);
+        assert!(matches!(
+            transferred.as_slice(),
+            [(FailureClass::Report { test_index: 4, .. }, 1)]
+        ));
+        let other = check(&[(5, 0)]).failures_beyond(&input);
         assert!(matches!(
             other.as_slice(),
             [(FailureClass::Report { test_index: 5, .. }, 1)]

solidcore/crates/solidcore/src/topology/brep/validation/assert_valid.rs

@@ -199,20 +199,39 @@ impl BrepCheck {
         out
     }
 
-    /// The failure classes `self` has more lines of than `before` has, each with how many
-    /// more, in [`FailureClass`] order: what a body has failing that `before` (the body it
-    /// was made from) did not. A class `before` has as many lines of, or more, is not
-    /// listed.
+    /// Failure lines not present at the same entity site in `before`, grouped and
+    /// counted in [`FailureClass`] order. A report matches by class and the identities
+    /// of its reporter, owner and typed other object, not its measured value or tolerance.
+    /// Synthetic lines without a report match by class only.
+    ///
+    /// Compare checks of one body's edit lineage: arena ids survive copies and in-place
+    /// edits, and removed ids are never reused. Geometry report keys encode their owning
+    /// entity's id (`object_info`, `geometry_key`); rebuilt entities therefore have new
+    /// sites, even when their geometry is unchanged. Unrelated bodies' ids do not establish
+    /// correspondence.
     #[must_use]
     pub fn failures_beyond(&self, before: &BrepCheck) -> Vec<(FailureClass, usize)> {
-        let had = before.failure_classes();
-        self.failure_classes()
-            .into_iter()
-            .filter_map(|(class, n)| {
-                let more = n.saturating_sub(had.get(&class).copied().unwrap_or(0));
-                (more > 0).then_some((class, more))
-            })
-            .collect()
+        let had = before.failure_sites();
+        let mut added = BTreeMap::new();
+        for ((class, site), n) in self.failure_sites() {
+            let more = n.saturating_sub(had.get(&(class, site)).copied().unwrap_or(0));
+            if more > 0 {
+                let count: &mut usize = added.entry(class).or_default();
+                *count = count.saturating_add(more);
+            }
+        }
+        added.into_iter().collect()
+    }
+
+    /// Multiplicities before aggregation by class: clearing one site cannot pay for
+    /// another site's failure, including when the total number of lines decreases.
+    fn failure_sites(&self) -> BTreeMap<(FailureClass, Option<FailureSite>), usize> {
+        let mut out = BTreeMap::new();
+        for (class, report) in self.classified() {
+            let n: &mut usize = out.entry((class, report.map(FailureSite::of))).or_default();
+            *n = n.saturating_add(1);
+        }
+        out
     }
 
     /// The failure lines in [`BrepCheck::lines`] order, each with its class and its report
@@ -248,10 +267,31 @@ impl BrepCheck {
     }
 }
 
+/// The identities a report names. The class supplies the reporting and owner types;
+/// the optional second object needs its own type too. Values, parameters and messages
+/// are measurements at this site, not its identity.
+#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
+struct FailureSite {
+    reporter: Option<ObjectKey>,
+    owner: Option<ObjectKey>,
+    other: Option<ObjectKey>,
+    other_type: i64,
+}
+
+impl FailureSite {
+    fn of(r: &AssertReport) -> Self {
+        Self {
+            reporter: r.reporter,
+            owner: r.owner,
+            other: r.other,
+            other_type: r.other_type,
+        }
+    }
+}
+
 /// What one failure line of a [`BrepCheck`] is, leaving out where: the test that failed
-/// and the type of the object it is about, not which object (an operation that rebuilds a
-/// body's entities gives them new identities, so a failure is compared with its input's
-/// by class: [`BrepCheck::failures_beyond`]).
+/// and the type of the object it is about, not which object. [`BrepCheck::failures_beyond`]
+/// also compares the report's entity site before aggregating new failures by class.
 #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub enum FailureClass {
     /// A failed report: its reporting type, label table, test and owner type.
@@ -682,6 +722,109 @@ mod tests {
         }
     }
 
+    /// Every named object participates in the site; measurements do not.
+    #[test]
+    fn failures_beyond_matches_reporter_owner_and_typed_other() {
+        let spec = ReportSpec {
+            reporter: Some(ObjectKey(1)),
+            owner: Some(ObjectKey(2)),
+            other: Some(ObjectKey(3)),
+            other_type: 16016,
+            test_class: BREP_TYPE,
+            test_index: 19,
+            ..ReportSpec::default()
+        };
+        let checked = |spec| {
+            let mut c = check(false, 19, None);
+            c.reports = AssertArray::new();
+            c.reports
+                .add_unique_delete_duplicate(AssertReport::new(spec).unwrap());
+            c
+        };
+        let input = checked(spec.clone());
+        for changed in [
+            ReportSpec {
+                reporter: Some(ObjectKey(4)),
+                ..spec.clone()
+            },
+            ReportSpec {
+                owner: Some(ObjectKey(4)),
+                ..spec.clone()
+            },
+            ReportSpec {
+                other: Some(ObjectKey(4)),
+                ..spec.clone()
+            },
+            ReportSpec {
+                other_type: 16021,
+                ..spec.clone()
+            },
+            ReportSpec {
+                other: None,
+                ..spec.clone()
+            },
+        ] {
+            let result = checked(changed);
+            assert_eq!(result.failure_classes(), input.failure_classes());
+            assert_eq!(
+                result.failures_beyond(&input).first().map(|(_, n)| *n),
+                Some(1)
+            );
+        }
+        let measured = checked(ReportSpec {
+            value: 0.25,
+            tol3d: 0.5,
+            ..spec
+        });
+        assert!(measured.failures_beyond(&input).is_empty());
+        assert!(input.failures_beyond(&input).is_empty());
+    }
+
+    /// Pointer reports use the same site rule; the synthetic skipped line still
+    /// matches even when the reported entity changes.
+    #[test]
+    fn failures_beyond_matches_pointer_report_sites() {
+        let mut input = check(false, 19, None);
+        input.pointer_reports = std::mem::take(&mut input.reports);
+        input.assert_valid = None;
+        let mut result = input.clone();
+        result.pointer_reports = AssertArray::new();
+        result.pointer_reports.add_unique_delete_duplicate(
+            AssertReport::new(ReportSpec {
+                test_class: BREP_TYPE,
+                test_index: 19,
+                owner: Some(ObjectKey(4)),
+                ..ReportSpec::default()
+            })
+            .unwrap(),
+        );
+        assert!(matches!(
+            result.failures_beyond(&input).as_slice(),
+            [(FailureClass::Report { test_index: 19, .. }, 1)]
+        ));
+        assert!(result.failures_beyond(&result).is_empty());
+    }
+
+    /// Status-only failures have no entity to compare: they retain class-only matches.
+    #[test]
+    fn failures_beyond_matches_synthetic_lines_by_class() {
+        let mut input = check(false, 19, None);
+        input.reports = AssertArray::new();
+        assert!(input.failures_beyond(&input).is_empty());
+        let mut result = input.clone();
+        result.pointers_status = 1001;
+        assert_eq!(
+            result.failures_beyond(&input),
+            [(FailureClass::PointersStatus(1001), 1)]
+        );
+        assert!(result.failures_beyond(&result).is_empty());
+        result.pointers_status = 1002;
+        assert_eq!(
+            result.failures_beyond(&input),
+            [(FailureClass::PointersStatus(1002), 1)]
+        );
+    }
+
     /// A report naming no owner type has no owner tag (#2013).
     #[test]
     fn line_omits_an_absent_owner_type() {

solidcore/crates/solidcore/src/topology/brep/validation/soundness.rs

@@ -111,7 +111,11 @@ pub fn soundness(k: &mut Kernel, b: &mut Brep) -> Result<Soundness> {
 /// geometry's owner the identity `b`'s naming scheme names the entity by
 /// ([`Brep::owner_identity`], as the operations that bind geometry set it), so a curve or
 /// surface owned under another entity's identity fails the owner tests; then the manifold
-/// test. Its volume is not measured ([`volume`]).
+/// test. Its volume is not measured ([`volume`]). The local-operation gate compares
+/// these reports by class and entity site ([`BrepCheck::failures_beyond`]): a failure
+/// may persist only on the same named objects, not migrate to a rebuilt entity. This
+/// uses the arena ids retained by `clone_standing_for_self`, not the caller's owner
+/// naming; a removed slot is never reused (validity-and-tolerances §3, invariant 1).
 ///
 /// As for [`soundness`], the check edits what it measures: measure a private copy.
 ///