Propagate nested container dirty ids

Author: saulshanabrook

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## [Unreleased] - ReleaseDate
 
+- Fix seminaive matching after nested containers rebuild in place by propagating dirty container ids through parent containers.
 - Desugar `relation`s to `constructor`s to simplify the language and implementation. Relations no longer return unit `()` values.
 - Refactored API to use [`TermId`] more consistently instead of `Term` where possible, simplifying egglog code.
 - **Typed primitive surface for seminaive safety (#772).** Custom primitives now pick one of `PurePrim` / `ReadPrim` / `WritePrim` / `FullPrim` based on what the body needs, and register via the matching `add_*_primitive`. Rust enforces capability bounds via the state wrapper passed to the body; the egglog typechecker enforces context bounds. See the `egglog::exec_state` module docs and the `*Prim` trait docs for the full picture. Migration: `rust_rule` callbacks now take `&mut WriteState` (replacing `RustRuleContext`); a new `rust_rule_full` gives action callbacks read access. Higher-order primitives over `unstable-fn` values dispatch via `state.apply_function(&fc, args)`.

core-relations/src/containers/mod.rs

@@ -81,6 +81,8 @@ pub struct ContainerValues {
 /// `dirty_ids` is narrower: it records container ids whose semantics changed
 /// while their stored outer id stayed stable. Ordinary table rebuild already
 /// handles changed-id cases; these ids need a follow-up parent-row refresh.
+/// This includes containers that changed directly and containers whose
+/// contained containers changed in place.
 ///
 /// For example, `l(vec-of(w(k(b))))` can rebuild to `l(vec-of(k(b)))` without
 /// changing the `Vec` id. The row is now newly matchable, but seminaive will
@@ -174,7 +176,7 @@ impl ContainerValues {
             // We may attempt an incremental rebuild.
             self.subset_tracker.recent_updates(table_id, table)
         });
-        if parallelize_inter_container_op(self.data.next_id().index()) {
+        let mut summary = if parallelize_inter_container_op(self.data.next_id().index()) {
             self.data
                 .iter_mut()
                 .zip(std::iter::repeat_with(|| exec_state.clone()))
@@ -202,6 +204,38 @@ impl ContainerValues {
                 ));
             }
             summary
+        };
+        self.expand_dirty_id_closure(&mut summary);
+        summary
+    }
+
+    /// Add ancestor containers to the dirty-id set until it is transitively closed.
+    ///
+    /// A rebuild can change a container's semantics in place without changing
+    /// its id. If that container is itself stored inside another container,
+    /// the parent container has also changed semantically even though no direct
+    /// rebuild touched its contents. For example, with
+    /// `(p (vec-of (vec-of (w (b)))))` and `(rewrite (w x) x)`, the inner
+    /// `Vec` rebuilds in place to `vec-of (b)`. Without this closure, only the
+    /// inner `Vec` id is dirty; the outer `Vec` row is not retimestamped, so a
+    /// later rule like `(rewrite (p (vec-of (vec-of (b)))) (b))` can miss the
+    /// newly matchable parent row.
+    fn expand_dirty_id_closure(&self, summary: &mut ContainerRebuildSummary) {
+        let mut seen = summary.dirty_ids.clone();
+        let mut frontier = seen.clone();
+
+        while !frontier.is_empty() {
+            let mut next = IndexSet::default();
+            for (_, env) in self.data.iter() {
+                env.extend_containers_containing(&frontier, &mut next);
+            }
+            frontier.clear();
+            for value in next {
+                if seen.insert(value) {
+                    summary.note_dirty_id(value);
+                    frontier.insert(value);
+                }
+            }
         }
     }
 
@@ -252,6 +286,12 @@ pub trait DynamicContainerEnv: Any + dyn_clone::DynClone + Send + Sync {
         subset: Option<SubsetRef>,
         exec_state: &mut ExecutionState,
     ) -> ContainerRebuildSummary;
+    /// Add ids for containers in this environment that contain any `values`.
+    ///
+    /// This uses the container content index populated from
+    /// [`ContainerValue::iter`] and lets callers climb from dirty child ids to
+    /// all directly containing parent container ids.
+    fn extend_containers_containing(&self, values: &IndexSet<Value>, out: &mut IndexSet<Value>);
 }
 
 // Implements `Clone` for `Box<dyn DynamicContainerEnv>`.
@@ -302,6 +342,14 @@ impl<C: ContainerValue> DynamicContainerEnv for ContainerEnv<C> {
         }
         self.apply_rebuild_nonincremental(rebuilder, exec_state)
     }
+
+    fn extend_containers_containing(&self, values: &IndexSet<Value>, out: &mut IndexSet<Value>) {
+        for value in values {
+            if let Some(containers) = self.val_index.get(value) {
+                out.extend(containers.iter().copied());
+            }
+        }
+    }
 }
 
 impl<C: ContainerValue> ContainerEnv<C> {

tests/container-fail.egg

@@ -20,3 +20,20 @@
 (let $resolved (l (vec-of (k (b)))))
 (run-schedule (saturate (run)))
 (check (= $resolved (b)))
+
+;; Regression for nested container dirty propagation. The inner Vec rebuilds in
+;; place, making the outer Vec newly matchable without changing the outer id.
+(sort VVE (Vec VE))
+(constructor p (VVE) E)
+
+(rewrite (p (vec-of (vec-of (b)))) (b))
+
+(push)
+(let $nested (p (vec-of (vec-of (w (b))))))
+(run-schedule (saturate (run)))
+(check (= $nested (b)))
+(pop)
+
+(let $nested-resolved (p (vec-of (vec-of (b)))))
+(run-schedule (saturate (run)))
+(check (= $nested-resolved (b)))

tests/snapshots/files__shared_snapshot_container_fail.snap

@@ -5,4 +5,5 @@ expression: snapshot_content_across_treatments
 ((b 1)
  (k 1)
  (l 1)
+ (p 1)
  (w 0))