fix: separate strict rule-output merge from data-document deep-merge

microsoft#760 made Value::merge recursive so Engine::add_data deep-merges nested
data documents. But that same method also backs rule materialization,
where recursion is wrong: two rule definitions producing different
outputs for one path must conflict (OPA complete-rule semantics), not
silently combine.

Split the two behaviors:
- Value::merge is strict and shallow again (as pre-microsoft#760): a key on both
  sides must be equal or it conflicts; used for rule outputs.
- Value::deep_merge is the recursive data-document merge behind add_data;
  check_mergeable validates it up front without allocating, so the
  default build merges in place instead of cloning a candidate.

Also fix zero-arg functions (f() := ...): route their materialization
through strict equality via a new RuleValueMerge selector, so disjoint
outputs ({a:1} vs {b:2}) conflict as OPA does while prefix scaffolding
(a.foo + a.bar) still combines.

Add a 14-case interpreter conformance matrix (multiple_outputs.yaml)
covering functions, static/dynamic partial objects, and ref-heads,
matched against OPA v1.2.0.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Author: Mark Birger

src/engine.rs

@@ -475,26 +475,24 @@ impl Engine {
             bail!("data must be object");
         }
 
-        // add_data is all-or-nothing. A merge can fail two ways, and the atomic strategy
-        // differs by build:
-        //  - conflict (same path, two different values): possible on every build.
-        //  - allocator-limit failure mid-merge: only when `allocator-memory-limits` is on;
-        //    otherwise the limit check is a compile-time no-op.
+        // add_data is all-or-nothing; the atomic strategy differs by build because the failure
+        // modes do: a conflict (same path, differing values) is possible everywhere, an
+        // allocator-limit failure mid-merge only under `allocator-memory-limits`.
         #[cfg(not(feature = "allocator-memory-limits"))]
         {
-            // Conflict is the only failure mode, and `check_mergeable` catches it up front
-            // without allocating — so validate, then merge in place (zero-copy fast path).
+            // Conflict is the only failure mode; `check_mergeable` catches it up front without
+            // allocating, so validate then deep-merge in place (zero-copy fast path).
             self.interpreter.get_init_data().check_mergeable(&data)?;
             self.prepared = false;
-            self.interpreter.get_init_data_mut().merge(data)
+            self.interpreter.get_init_data_mut().deep_merge(data)
         }
         #[cfg(feature = "allocator-memory-limits")]
         {
-            // A limit failure can strike mid-merge and `check_mergeable` can't predict it,
-            // so merge into a candidate and commit only on success — atomic for both failure
-            // modes. `Value` is Rc/copy-on-write, so only touched subtrees are cloned.
+            // A limit failure can strike mid-merge and can't be predicted, so merge into a
+            // candidate and commit only on success. `Value` is copy-on-write, so only touched
+            // subtrees are cloned.
             let mut candidate = self.interpreter.get_init_data().clone();
-            candidate.merge(data)?;
+            candidate.deep_merge(data)?;
             *self.interpreter.get_init_data_mut() = candidate;
             self.prepared = false;
             Ok(())

src/interpreter.rs

@@ -60,6 +60,17 @@ enum FunctionModifier {
     Value(Value),
 }
 
+/// How [`Interpreter::update_data`] merges a rule's value into the data document.
+#[derive(Debug, Clone, Copy)]
+enum RuleValueMerge {
+    /// Shallow-merge keeping disjoint keys, so rules sharing a path prefix scaffold into one
+    /// object (`a.foo` + `a.bar` → one `a`) instead of conflicting.
+    Combine,
+    /// Complete-rule semantics: existing value must be absent or exactly equal, else conflict.
+    /// Used for zero-arg function outputs (`f() := …`), which OPA treats like complete rules.
+    Strict,
+}
+
 type RuleValues = BTreeMap<Vec<Value>, (Value, Ref<Expr>)>;
 
 #[derive(Debug)]
@@ -3408,6 +3419,23 @@ impl Interpreter {
         }
     }
 
+    /// Materialize a complete-rule value: the existing value must be absent or *exactly equal*
+    /// to `new`, else it is a conflict.
+    ///
+    /// Unlike the shallow [`Self::merge_rule_value`], differing outputs conflict instead of
+    /// combining — `f() := {"a": 1}` and `f() := {"b": 2}` conflict — matching OPA's semantics
+    /// for zero-arg functions.
+    fn merge_rule_value_strict(span: &Span, value: &mut Value, new: Value) -> Result<()> {
+        if *value == Value::Undefined {
+            *value = new;
+            Ok(())
+        } else if *value == new {
+            Ok(())
+        } else {
+            Err(span.error("rules should not produce multiple outputs."))
+        }
+    }
+
     pub fn get_path_string(refr: &Expr, document: Option<&str>) -> Result<String> {
         let mut comps = vec![];
         let mut expr_opt = Some(refr);
@@ -3663,14 +3691,18 @@ impl Interpreter {
         _refr: &Expr,
         path: &[&str],
         value: Value,
+        merge: RuleValueMerge,
     ) -> Result<()> {
         if value == Value::Undefined {
             return Ok(());
         }
         // Ensure that path is created.
         let vref = Self::make_or_get_value_mut(&mut self.data, path)?;
         if Self::get_value_chained(self.init_data.clone(), path) == Value::Undefined {
-            Self::merge_rule_value(span, vref, value)
+            match merge {
+                RuleValueMerge::Strict => Self::merge_rule_value_strict(span, vref, value),
+                RuleValueMerge::Combine => Self::merge_rule_value(span, vref, value),
+            }
         } else {
             // Retain specified value.
             Ok(())
@@ -3778,7 +3810,13 @@ impl Interpreter {
                         // `a` is created as an empty object.
                         if let Some((_, prefix)) = path.split_last() {
                             if !prefix.is_empty() {
-                                self.update_data(span, refr, prefix, Value::new_object())?;
+                                self.update_data(
+                                    span,
+                                    refr,
+                                    prefix,
+                                    Value::new_object(),
+                                    RuleValueMerge::Combine,
+                                )?;
                             }
                         }
 
@@ -3790,7 +3828,13 @@ impl Interpreter {
                             };
 
                             let value = self.eval_rule_bodies(ctx, span, rule_body)?;
-                            self.update_data(refr.span(), refr, &path[..], value)?;
+                            self.update_data(
+                                refr.span(),
+                                refr,
+                                &path[..],
+                                value,
+                                RuleValueMerge::Strict,
+                            )?;
                         }
                     }
                 }
@@ -4037,6 +4081,7 @@ impl Interpreter {
                         rule_refr,
                         &prefix_path,
                         Value::new_object(),
+                        RuleValueMerge::Combine,
                     )?;
                 }
             }

src/value/mod.rs

@@ -1365,55 +1365,87 @@ impl Value {
         }
     }
 
-    /// Recursively merge `new` into `self`.
-    ///
-    /// Objects are deep-merged: keys present on only one side are kept, keys present on both
-    /// whose values are both objects are merged recursively, and keys whose values are both
-    /// sets are unioned. This matches OPA's data-document merge semantics for objects (see
-    /// `Engine::add_data`); set-union is a regorus extension, since OPA data documents are
-    /// JSON and cannot contain sets. Any other differing pair — values that are not both
-    /// objects or both sets, such as two unequal scalars or two arrays — is a genuine
-    /// conflict and is an error. Equal values are tolerated as a no-op;
-    /// the rule-evaluation path (`Interpreter::merge_rule_value`) shares this method and relies
-    /// on that leniency, since a rule may legally produce the same value more than once.
+    /// Shallow-merge `new` into `self` with strict rule-output semantics.
+    ///
+    /// Objects merge one level deep: a key on both sides must hold the *same* value or it is a
+    /// conflict; sets union; equal values are a no-op. Non-recursive by design — data documents
+    /// use [`Value::deep_merge`] instead.
     pub(crate) fn merge(&mut self, mut new: Value) -> Result<()> {
         if self == &new {
             return Ok(());
         }
         match (self, &mut new) {
             (v @ Value::Undefined, _) => *v = new,
             (Value::Set(ref mut set), Value::Set(new)) => {
-                // Union without deep-cloning the RHS set. If we uniquely own it, move its
-                // elements out; if it is shared (e.g. reached via `existing.merge(v.clone())`
-                // in the object arm below), clone only the element handles (`Rc` bumps),
-                // never the whole `BTreeSet`.
+                // Union without deep-cloning the RHS set (see `deep_merge`).
+                let dst = Rc::make_mut(set);
+                match Rc::try_unwrap(core::mem::take(new)) {
+                    Ok(owned) => dst.extend(owned),
+                    Err(shared) => dst.extend(shared.iter().cloned()),
+                }
+                enforce_limit_anyhow()?;
+            }
+            (Value::Object(map), Value::Object(new)) => {
+                for (k, v) in new.iter() {
+                    match map.get(k) {
+                        // Same key, different value: the rule produced two outputs for one path.
+                        Some(pv) if *pv != *v => bail!(
+                            "value for key `{}` generated multiple times: `{}` and `{}`",
+                            serde_json::to_string_pretty(&k).map_err(anyhow::Error::msg)?,
+                            serde_json::to_string_pretty(&pv).map_err(anyhow::Error::msg)?,
+                            serde_json::to_string_pretty(&v).map_err(anyhow::Error::msg)?,
+                        ),
+                        _ => {
+                            Rc::make_mut(map).insert(k.clone(), v.clone());
+                            enforce_limit_anyhow()?;
+                        }
+                    };
+                }
+            }
+            _ => bail!("error: could not merge value"),
+        };
+        Ok(())
+    }
+
+    /// Recursively deep-merge `new` into `self` — the data-document merge behind [`Engine::add_data`].
+    ///
+    /// Objects recurse per-key, sets union, equal values are a no-op, any other differing pair
+    /// conflicts. Set-union is a regorus extension (OPA data is JSON, which has no sets). Distinct
+    /// from the strict, non-recursive [`Value::merge`] used for rule outputs — use deep-merge ONLY
+    /// for data documents.
+    ///
+    /// [`Engine::add_data`]: crate::Engine::add_data
+    pub(crate) fn deep_merge(&mut self, mut new: Value) -> Result<()> {
+        if self == &new {
+            return Ok(());
+        }
+        match (self, &mut new) {
+            (v @ Value::Undefined, _) => *v = new,
+            (Value::Set(ref mut set), Value::Set(new)) => {
+                // Union without deep-cloning the RHS set: move elements if uniquely owned,
+                // else clone only the element handles (`Rc` bumps), never the whole `BTreeSet`.
                 let dst = Rc::make_mut(set);
                 match Rc::try_unwrap(core::mem::take(new)) {
                     Ok(owned) => dst.extend(owned),
                     Err(shared) => dst.extend(shared.iter().cloned()),
                 }
-                // Enforce allocator limit after merging set entries.
                 enforce_limit_anyhow()?;
             }
             (Value::Object(map), Value::Object(new)) => {
                 let map = Rc::make_mut(map);
                 for (k, v) in new.iter() {
                     match map.get_mut(k) {
                         Some(existing) => {
-                            // Deep-merge: when both sides are containers, recurse so that
-                            // nested objects are merged rather than the whole subtree being
-                            // replaced (matches OPA's data document merge semantics).
+                            // When both sides are containers, recurse so nested objects merge
+                            // rather than the subtree being replaced (OPA data-merge semantics).
                             let both_mergeable = matches!(
                                 (&*existing, v),
                                 (Value::Object(_), Value::Object(_))
                                     | (Value::Set(_), Value::Set(_))
                             );
                             if both_mergeable {
-                                existing.merge(v.clone())?;
+                                existing.deep_merge(v.clone())?;
                             } else if *existing != *v {
-                                // A genuine leaf conflict: the same path holds two different
-                                // values. (Equal values are tolerated as a no-op, which the
-                                // shared rule-evaluation path relies on.)
                                 bail!(
                                     "value for key `{}` generated multiple times: `{}` and `{}`",
                                     serde_json::to_string_pretty(&k).map_err(anyhow::Error::msg)?,
@@ -1425,7 +1457,6 @@ impl Value {
                         }
                         None => {
                             map.insert(k.clone(), v.clone());
-                            // Enforce allocator limit after merging object entries.
                             enforce_limit_anyhow()?;
                         }
                     };
@@ -1436,16 +1467,14 @@ impl Value {
         Ok(())
     }
 
-    /// Read-only check that merging `other` into `self` via [`Value::merge`] would
-    /// not conflict, without mutating `self` or allocating.
+    /// Read-only check that [`deep_merge`](Value::deep_merge)-ing `other` into `self` would not
+    /// conflict, without mutating or allocating.
     ///
-    /// This mirrors `merge`'s conflict rule exactly — objects are deep-merged, sets
-    /// union (never conflict), equal values are a tolerated no-op, and any other
-    /// differing pair is a conflict — but only *reads*. [`Engine::add_data`] runs this
-    /// before `merge` so a rejected add is all-or-nothing: a conflict deep in a nested
-    /// document cannot leave earlier keys of the same document partially written.
-    /// Keeping the check separate preserves `merge`'s in-place (uniquely-owned) fast
-    /// path — no candidate copy of the data document is made just to validate it.
+    /// Lets [`Engine::add_data`] validate before merging in place. Since a conflict is the only
+    /// way the default-build merge can fail and it depends only on the inputs, a passing scan
+    /// guarantees the in-place `deep_merge` won't fail — avoiding the alternative of cloning the
+    /// whole document into a candidate just to validate. Only overlapping keys are walked, so
+    /// disjoint additions are near-free.
     ///
     /// [`Engine::add_data`]: crate::Engine::add_data
     #[cfg(not(feature = "allocator-memory-limits"))]
@@ -1455,12 +1484,11 @@ impl Value {
         }
         match (self, other) {
             (Value::Undefined, _) => Ok(()),
-            // Set union never conflicts (matches the `(Set, Set)` merge arm).
+            // Set union never conflicts.
             (Value::Set(_), Value::Set(_)) => Ok(()),
             (Value::Object(dst), Value::Object(src)) => {
                 for (k, sv) in src.iter() {
-                    // Only overlapping keys can conflict; keys unique to `src` are
-                    // pure insertions.
+                    // Only overlapping keys can conflict.
                     if let Some(dv) = dst.get(k) {
                         let both_mergeable = matches!(
                             (dv, sv),