feat!: make the invalid Value shapes unrepresentable

docs/MATERIALIZATION-SPEC.md

@@ -295,8 +295,8 @@ for both languages from this point forward.
   had not yet caught in code: `saphyr` (this engine's YAML parser)
   silently demoted an integer literal beyond `i64` range to a lossy
   `f64` before canonicalization ever saw it — fixed in `reader.rs`,
-  not papered over here, via a new `Value::BigInt(String)` carrying
-  the literal's exact digits. Not yet wired into a pipeline stage —
+  not papered over here, via a new `Value::BigInt` carrying the
+  literal's exact digits (a validated `BigIntValue` newtype since #53). Not yet wired into a pipeline stage —
   `Parse`/`Normalize`/`Resolve`/`Validate`, which would produce a
   materialized tree to canonicalize, still don't exist — so this is a
   standalone primitive, correct and tested in isolation, not yet

engine/src/canonical.rs

@@ -6,6 +6,7 @@
 //! session, not assumed from documentation. Number canonicalization (A3)
 //! already has a proven Rust peer in the `cic-canonical` crate (both its
 //! `canonical_float` and `canonical_integer`); the logic is ported here
+//! (`canonical_integer` as [`crate::reader::BigIntValue::canonical`])
 //! rather than taken as a dependency, so this crate's canonicalization
 //! stays self-contained and this engine does not acquire a cross-repository
 //! build dependency on `CIC-Relay`.
@@ -42,12 +43,9 @@ use sha2::{Digest as _, Sha256};
 /// is reachable from authored YAML's own `.nan`/`.inf` tags, not only from
 /// constructing a [`Value`] directly.
 ///
-/// Returns `E_MALFORMED_VALUE` for the two shapes only a hand-built
-/// [`Value`] can have (the reader never produces them): a `Value::BigInt`
-/// whose text is not an optional sign followed by ASCII digits
-/// (`R-CANON-BIGINT-DIGITS`), and a `Map` holding the same key more than
-/// once (`R-CANON-UNIQUE-KEYS`). Neither panics and neither is written out
-/// as if it were valid.
+/// A `Value` cannot hold a non-digit `BigInt` or a repeated map key (see
+/// [`crate::reader::BigIntValue`] and [`crate::reader::Map`]), so the only
+/// failure is the non-finite float above.
 pub fn to_canonical_json(v: &Value) -> Result<Vec<u8>> {
     let mut out = Vec::new();
     write_value(&mut out, v, "$")?;
@@ -78,27 +76,12 @@ fn write_value(out: &mut Vec<u8>, v: &Value, path: &str) -> Result<()> {
         Value::Int(i) => out.extend_from_slice(i.to_string().as_bytes()),
         // A3's big.Int-equivalent case (review-caught on PR #19: `reader.rs`
         // used to silently lose this precision to saphyr's own i64/f64
-        // fallback -- see `Value::BigInt`'s own doc comment). `reader.rs`
-        // only guarantees the text is a validated digit run; normalization
-        // -- sign fold, leading-zero strip, `-0` -> `0` -- happens here,
-        // ported from `cic-canonical`'s own already-proven
-        // `canonical_integer`, the same function Go's `number.go` and this
-        // crate's number canonicalization both already trust. `Value` is
-        // public, so a caller can hand in a `BigInt` the reader would never
-        // build: that is an error, not a panic (Go's `CanonicalInteger`
-        // checks the same way).
-        Value::BigInt(s) => match canonical_integer(s) {
-            Some(n) => out.extend_from_slice(n.as_bytes()),
-            None => {
-                return Err(Error::new(
-                    code::MALFORMED_VALUE,
-                    "R-CANON-BIGINT-DIGITS",
-                    Stage::Canonicalize,
-                    path,
-                    format!("{path} is a BigInt whose text is not a signed digit run: {s:?}"),
-                ));
-            }
-        },
+        // fallback -- see `Value::BigInt`'s own doc comment). The text is a
+        // digit run by construction (`BigIntValue::try_new`); normalization
+        // -- sign fold, leading-zero strip, `-0` -> `0` -- is
+        // `BigIntValue::canonical`, the same rule Go's `number.go` applies via
+        // `big.Int.SetString(s, 10).String()`.
+        Value::BigInt(n) => out.extend_from_slice(n.canonical().as_bytes()),
         Value::Float(f) => {
             if !f.is_finite() {
                 return Err(Error::new(
@@ -121,32 +104,6 @@ fn write_value(out: &mut Vec<u8>, v: &Value, path: &str) -> Result<()> {
     Ok(())
 }
 
-/// A3, ported from `cic-canonical`'s own `canonical_integer` (not imported
-/// as a dependency — see module docs): sign folded, leading zeros stripped,
-/// `-0`/`+0` -> `0` — the same normalization Go's `number.go` applies via
-/// `big.Int.SetString(s, 10).String()`, verified empirically against it in
-/// this session (`"000000123"` -> `"123"`, `"-00000123"` -> `"-123"`,
-/// `"-0"`/`"+0"` -> `"0"`). Returns `None` only if `s` is not actually a
-/// validated digit run — [`crate::reader::Value::BigInt`]'s own invariant
-/// guarantees this never happens for a value this crate constructed itself.
-pub(crate) fn canonical_integer(s: &str) -> Option<String> {
-    let (neg, digits) = match s.strip_prefix('-') {
-        Some(rest) => (true, rest),
-        None => (false, s.strip_prefix('+').unwrap_or(s)),
-    };
-    if digits.is_empty() || !digits.bytes().all(|b| b.is_ascii_digit()) {
-        return None;
-    }
-    let magnitude = digits.trim_start_matches('0');
-    Some(if magnitude.is_empty() {
-        "0".to_string() // all zeros, including "-0"/"+0"
-    } else if neg {
-        format!("-{magnitude}")
-    } else {
-        magnitude.to_string()
-    })
-}
-
 /// A3, ported from `cic-canonical`'s own `canonical_float` (not imported as a
 /// dependency — see module docs): shortest round-trip plain-decimal form,
 /// `-0.0` folded to `0`. Rust's `f64` `Display` is shortest round-trip and
@@ -177,23 +134,11 @@ fn write_array(out: &mut Vec<u8>, items: &[Value], path: &str) -> Result<()> {
 fn write_map(out: &mut Vec<u8>, m: &Map, path: &str) -> Result<()> {
     // Raw UTF-8 byte order -- `str`'s `Ord` is already byte-wise comparison,
     // matching Go's `sort.Strings` (A2) with no custom comparator needed.
-    // Sorting the entries (not just the keys) puts any duplicate next to its
-    // twin, so it is caught here instead of being written twice with the
-    // first value both times, which is what a key-list + `get` walk did.
-    let mut entries: Vec<&(String, Value)> = m.0.iter().collect();
-    entries.sort_by(|a, b| a.0.cmp(&b.0));
-    if let Some(pair) = entries.windows(2).find(|w| w[0].0 == w[1].0) {
-        let k = &pair[0].0;
-        return Err(Error::new(
-            code::MALFORMED_VALUE,
-            "R-CANON-UNIQUE-KEYS",
-            Stage::Canonicalize,
-            format!("{path}.{k}"),
-            format!("{path} holds the key {k:?} more than once; canonical JSON objects have unique keys"),
-        ));
-    }
+    // Keys are unique by `Map`'s construction, so the order is total.
+    let mut entries: Vec<(&str, &Value)> = m.iter().collect();
+    entries.sort_unstable_by(|a, b| a.0.cmp(b.0));
     out.push(b'{');
-    for (i, (k, v)) in entries.into_iter().map(|e| (&e.0, &e.1)).enumerate() {
+    for (i, (k, v)) in entries.into_iter().enumerate() {
         if i > 0 {
             out.push(b',');
         }
@@ -249,7 +194,7 @@ fn write_json_string(out: &mut Vec<u8>, s: &str) {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::reader::Map;
+    use crate::reader::{BigIntValue, Map};
 
     fn json(v: &Value) -> String {
         String::from_utf8(to_canonical_json(v).expect("value is finite"))
@@ -474,7 +419,7 @@ mod tests {
         ];
         for (literal, want) in cases {
             assert_eq!(
-                json(&Value::BigInt((*literal).to_string())),
+                json(&Value::BigInt(BigIntValue::try_new(*literal).unwrap())),
                 *want,
                 "literal {literal:?}"
             );

engine/src/collection.rs

@@ -39,7 +39,6 @@
 //! floats), kept here only as a defensive, documented exception for a
 //! `Value` this function might be handed before that stage runs.
 
-use crate::canonical::canonical_integer;
 use crate::reader::Value;
 
 /// Ported from `collection.go`'s `CollectionTopology`.
@@ -124,11 +123,9 @@ impl Collection {
 }
 
 /// `fmt.Sprintf("%v", ...)`, ported in full (see this module's own doc
-/// comment for the correction history). `None` for a non-finite float, and
-/// for the two shapes only a hand-built `Value` can have (a `BigInt` that is
-/// not a signed digit run, a `Map` with a repeated key) -- every other
-/// `Value` variant, including nested `Seq`/`Map`, has a defined,
-/// Go-matching display form. `pub(crate)`: the not-yet-built
+/// comment for the correction history). `None` only for a non-finite
+/// float -- every other `Value` variant, including nested `Seq`/`Map`, has
+/// a defined, Go-matching display form. `pub(crate)`: the not-yet-built
 /// object-level walker's `resolvePath`-equivalent needs the exact same
 /// formatting for its own `{key=val}` path-segment matching (`conformance.
 /// go`'s `resolvePath` uses the identical `fmt.Sprintf("%v", em[key])`),
@@ -138,7 +135,7 @@ pub(crate) fn go_display(v: &Value) -> Option<String> {
         Value::Null => Some("<nil>".to_string()),
         Value::Bool(b) => Some(if *b { "true" } else { "false" }.to_string()),
         Value::Int(i) => Some(i.to_string()),
-        Value::BigInt(s) => canonical_integer(s),
+        Value::BigInt(n) => Some(n.canonical()),
         Value::Float(f) => go_float_display(*f),
         Value::Str(s) => Some(s.clone()),
         Value::Seq(items) => {
@@ -151,15 +148,11 @@ pub(crate) fn go_display(v: &Value) -> Option<String> {
         Value::Map(m) => {
             // Go's `fmt` package has sorted map keys for deterministic
             // `%v` output since Go 1.12 (verified empirically) -- A2's
-            // byte-wise sort, reused, not a second rule. A repeated key
-            // (impossible in a Go map) has no display form.
-            let mut entries: Vec<&(String, Value)> = m.0.iter().collect();
-            entries.sort_by(|a, b| a.0.cmp(&b.0));
-            if entries.windows(2).any(|w| w[0].0 == w[1].0) {
-                return None;
-            }
+            // byte-wise sort, reused, not a second rule.
+            let mut entries: Vec<(&str, &Value)> = m.iter().collect();
+            entries.sort_unstable_by(|a, b| a.0.cmp(b.0));
             let mut parts = Vec::with_capacity(entries.len());
-            for (k, val) in entries.into_iter().map(|e| (&e.0, &e.1)) {
+            for (k, val) in entries {
                 parts.push(format!("{k}:{}", go_display(val)?));
             }
             Some(format!("map[{}]", parts.join(" ")))

engine/src/conformance.rs

@@ -34,7 +34,7 @@
 //! verified against real Go output before being encoded — see that
 //! function's own doc comment for what each round found and fixed.
 
-use crate::canonical::{canonical_float, canonical_integer, to_canonical_json};
+use crate::canonical::{canonical_float, to_canonical_json};
 use crate::reader::Value;
 use num_bigint::BigInt;
 use num_rational::BigRational;
@@ -192,15 +192,10 @@ fn compare_exact(intent: &Value, observed: &Value) -> (bool, bool) {
         if i_num != o_num {
             return (false, false); // number vs non-number -> not comparable
         }
-        // A hand-built `BigInt` that is not a digit run has no canonical
-        // form: not comparable, rather than a panic or a guess.
-        return match (
-            canonical_number_string(intent),
-            canonical_number_string(observed),
-        ) {
-            (Some(a), Some(b)) => (a == b, true),
-            _ => (false, false),
-        };
+        return (
+            canonical_number_string(intent) == canonical_number_string(observed),
+            true,
+        );
     }
     if json_kind(intent) != json_kind(observed) {
         return (false, false);
@@ -211,14 +206,12 @@ fn compare_exact(intent: &Value, observed: &Value) -> (bool, bool) {
 /// Canonical decimal form of a numeric `Value`, reusing section A's own
 /// number canonicalization (`canonical.rs`) rather than a second
 /// implementation — the same digits `to_canonical_json` would write for
-/// this value, without re-serializing the whole tree. `None` only for a
-/// `BigInt` whose text is not a signed digit run, which the reader never
-/// builds but a caller of the public `Value` can.
-fn canonical_number_string(v: &Value) -> Option<String> {
+/// this value, without re-serializing the whole tree.
+fn canonical_number_string(v: &Value) -> String {
     match v {
-        Value::Int(i) => Some(i.to_string()),
-        Value::BigInt(s) => canonical_integer(s),
-        Value::Float(f) => Some(canonical_float(*f)),
+        Value::Int(i) => i.to_string(),
+        Value::BigInt(n) => n.canonical(),
+        Value::Float(f) => canonical_float(*f),
         _ => unreachable!("canonical_number_string is only called where is_numeric_value(v)"),
     }
 }
@@ -243,7 +236,9 @@ fn canonical_equal(a: &Value, b: &Value) -> bool {
 fn as_rational(v: &Value) -> Option<BigRational> {
     match v {
         Value::Int(i) => Some(BigRational::from_integer(BigInt::from(*i))),
-        Value::BigInt(s) => BigInt::from_str(s).ok().map(BigRational::from_integer),
+        Value::BigInt(n) => BigInt::from_str(n.as_str())
+            .ok()
+            .map(BigRational::from_integer),
         Value::Float(f) => BigRational::from_f64(*f),
         Value::Str(s) => rat_from_string(s),
         Value::Null | Value::Bool(_) | Value::Seq(_) | Value::Map(_) => None,

engine/src/digest_projection.rs

@@ -250,7 +250,7 @@ mod tests {
         };
         let mut fields = BTreeMap::new();
         let mut keys = BTreeSet::new();
-        for (k, val) in m.0 {
+        for (k, val) in m {
             keys.insert(k.clone());
             let evidence = evidence_of(&k, val);
             fields.insert(

engine/src/error.rs

@@ -124,13 +124,14 @@ pub mod code {
     /// Raised at `Stage::Materialize`.
     pub const INCOMPLETE_OBJECT: &str = "E_INCOMPLETE_OBJECT";
 
-    /// A [`crate::reader::Value`] built by hand (not by the reader) breaks an
-    /// invariant the reader guarantees: a `BigInt` that is not a signed digit
-    /// run, or a `Map` holding the same key twice. Raised by `canonical`
-    /// (`R-CANON-BIGINT-DIGITS`, `R-CANON-UNIQUE-KEYS`) instead of panicking
-    /// or writing invalid canonical JSON. Not expressible as a YAML vector --
-    /// the reader cannot produce either shape -- so the raisers are covered
-    /// by unit tests in `canonical.rs`.
+    /// A [`crate::reader::Value`] part built by hand that breaks an invariant
+    /// the reader guarantees: a `BigInt` that is not a signed digit run
+    /// (`R-VALUE-BIGINT-DIGITS`, raised by `BigIntValue::try_new`) or a `Map`
+    /// holding the same key twice (`R-VALUE-UNIQUE-KEYS`, raised by
+    /// `Map::from_pairs`). These are constructor errors: the types have no
+    /// other way in, so the canonicalizer and comparators never see either
+    /// shape. Covered by unit tests in `engine/tests/hand_built_values.rs`
+    /// (not a YAML vector -- the reader cannot produce them).
     pub const MALFORMED_VALUE: &str = "E_MALFORMED_VALUE";
 }
 

engine/src/plan.rs

@@ -506,7 +506,7 @@ mod tests {
         };
         let mut fields = BTreeMap::new();
         let mut keys = BTreeSet::new();
-        for (k, val) in m.0 {
+        for (k, val) in m {
             keys.insert(k.clone());
             let evidence = evidence_of(&k, val);
             fields.insert(
@@ -704,11 +704,7 @@ mod tests {
         let Value::Map(m) = &mut observed else {
             unreachable!()
         };
-        let nics_idx =
-            m.0.iter()
-                .position(|(k, _)| k == "network_interfaces")
-                .unwrap();
-        let Value::Seq(nics) = &mut m.0[nics_idx].1 else {
+        let Some(Value::Seq(nics)) = m.get_mut("network_interfaces") else {
             unreachable!()
         };
         nics[1] = nic("nic-1", "prod-subnet-WRONG", &[]);
@@ -736,8 +732,7 @@ mod tests {
         let Value::Map(m) = &mut observed else {
             unreachable!()
         };
-        let idx = m.0.iter().position(|(k, _)| k == "memory_gb").unwrap();
-        m.0[idx].1 = Value::Str("large".into());
+        m.insert("memory_gb", Value::Str("large".into()));
 
         let obs = oci_observation();
         let verdict = evaluate(