fix: address Copilot review on symmetry and Lindblad entry points

- symmetry: try_from_generators reports order overflow as GroupError
  (PermutationOrderOverflow, GroupOrderOverflow) instead of panicking
- symmetry: CharacterTable records its group's generator orders, so a
  table from a different group of equal order is rejected
- lindblad: cap_basis and cap_map_to_room keep exactly the requested
  count under magnitude ties (also fixes a room+1 off-by-one)
- python: LindbladSpec.action validates the input width
- python: array entry points reject non-integer or out-of-range Pauli
  codes instead of letting the uint8 cast wrap them
- python: add matplotlib to the demo dependency group

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Author: david-pl

crates/ppvm-lindblad/src/truncate.rs

@@ -36,9 +36,8 @@ pub(crate) fn cap_map_to_room<const C: usize, T: Coeff>(
         return;
     }
     let mut mags: Vec<f64> = merged.values().map(|v| v.mag()).collect();
-    let k = room.min(mags.len() - 1);
-    let cutoff = nth_largest(&mut mags, k);
-    merged.retain(|_, v| v.mag() >= cutoff);
+    let mut keep = top_k_keeper(&mut mags, room);
+    merged.retain(|_, v| keep(v.mag()));
 }
 
 /// Compact `basis` / `coeffs` in place: drop entries whose coefficient
@@ -82,15 +81,12 @@ pub(crate) fn cap_basis<const C: usize, T: Coeff>(
         .filter(|(w, _)| !protected_set.contains(w))
         .map(|(_, c)| c.mag())
         .collect();
-    let cutoff = if slots == 0 {
-        f64::INFINITY
-    } else if slots >= mags.len() {
+    if slots >= mags.len() {
         return;
-    } else {
-        nth_largest(&mut mags, slots - 1)
-    };
+    }
+    let mut keep = top_k_keeper(&mut mags, slots);
     retain_in_place(basis, coeffs, |w, c| {
-        protected_set.contains(w) || c.mag() >= cutoff
+        protected_set.contains(w) || keep(c.mag())
     });
 }
 
@@ -139,6 +135,27 @@ fn retain_in_place<const C: usize, T>(
     coeffs.truncate(write);
 }
 
+/// Predicate that, fed every magnitude of `mags` once, accepts exactly the
+/// `k` largest (`k < mags.len()`): all above the cutoff, then ties in visit
+/// order. Reorders `mags`.
+fn top_k_keeper(mags: &mut [f64], k: usize) -> impl FnMut(f64) -> bool {
+    let (cutoff, mut ties_left) = if k == 0 {
+        (f64::INFINITY, 0)
+    } else {
+        let cutoff = nth_largest(mags, k - 1);
+        let n_above = mags.iter().filter(|&&m| m > cutoff).count();
+        (cutoff, k.saturating_sub(n_above))
+    };
+    move |m| {
+        if m > cutoff {
+            return true;
+        }
+        let tie = m == cutoff && ties_left > 0;
+        ties_left -= tie as usize;
+        tie
+    }
+}
+
 /// The `k`-th largest element of `mags` (0-indexed), via a partial sort.
 /// Reorders `mags`. Panics if `k >= mags.len()`.
 fn nth_largest(mags: &mut [f64], k: usize) -> f64 {
@@ -154,3 +171,61 @@ fn desc_by_mag<T: Coeff>(a: T, b: T) -> std::cmp::Ordering {
         .partial_cmp(&a.mag())
         .unwrap_or(std::cmp::Ordering::Equal)
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::word::word_from_codes;
+
+    /// `n` distinct 4-qubit words.
+    fn words(n: usize) -> Vec<Word> {
+        (0..n)
+            .map(|i| {
+                let codes: Vec<u8> = (0..4).map(|q| ((i >> (2 * q)) & 3) as u8).collect();
+                word_from_codes(&codes).unwrap()
+            })
+            .collect()
+    }
+
+    #[test]
+    fn cap_basis_is_a_hard_cap_under_ties() {
+        let mut basis = words(10);
+        // One large entry, nine tied zeros (fresh leakage admissions).
+        let mut coeffs = vec![0.0; 10];
+        coeffs[3] = 1.0;
+        cap_basis(&mut basis, &mut coeffs, 4, &[]);
+        assert_eq!(basis.len(), 4);
+        assert_eq!(coeffs, vec![0.0, 0.0, 0.0, 1.0]);
+    }
+
+    #[test]
+    fn cap_basis_keeps_protected_beyond_slots() {
+        let all = words(6);
+        let mut basis = all.clone();
+        let mut coeffs = vec![0.5; 6];
+        let protected = [all[5]];
+        cap_basis(&mut basis, &mut coeffs, 3, &protected);
+        assert_eq!(basis, vec![all[0], all[1], all[5]]);
+    }
+
+    #[test]
+    fn cap_map_to_room_keeps_exactly_room_under_ties() {
+        let mut merged: FxHashMap<Word, f64> = words(10).into_iter().map(|w| (w, 0.25)).collect();
+        cap_map_to_room(&mut merged, 3);
+        assert_eq!(merged.len(), 3);
+    }
+
+    #[test]
+    fn cap_map_to_room_keeps_the_largest() {
+        let ws = words(5);
+        let mut merged: FxHashMap<Word, f64> = ws
+            .iter()
+            .zip([1.0, -5.0, 3.0, 0.5, -4.0])
+            .map(|(w, c)| (*w, c))
+            .collect();
+        cap_map_to_room(&mut merged, 2);
+        let mut kept: Vec<f64> = merged.values().copied().collect();
+        kept.sort_by(f64::total_cmp);
+        assert_eq!(kept, vec![-5.0, -4.0]);
+    }
+}

crates/ppvm-pauli-sum/src/symmetry/group.rs

@@ -12,13 +12,12 @@ fn gcd(mut a: usize, mut b: usize) -> usize {
     a
 }
 
-fn checked_lcm(a: usize, b: usize, context: &str) -> usize {
-    a.checked_div(gcd(a, b))
-        .and_then(|q| q.checked_mul(b))
-        .unwrap_or_else(|| panic!("{context} overflow"))
+fn checked_lcm(a: usize, b: usize) -> Option<usize> {
+    a.checked_div(gcd(a, b)).and_then(|q| q.checked_mul(b))
 }
 
-fn permutation_order(perm: &[u32], generator: usize) -> u32 {
+/// Exact cyclic order of `perm`, or `None` if it does not fit in `u32`.
+fn permutation_order(perm: &[u32], generator: usize) -> Option<u32> {
     let mut seen = vec![false; perm.len()];
     let mut order = 1usize;
     for start in 0..perm.len() {
@@ -36,22 +35,19 @@ fn permutation_order(perm: &[u32], generator: usize) -> u32 {
                 break;
             }
         }
-        order = checked_lcm(order, length, "permutation order");
+        order = checked_lcm(order, length)?;
     }
-    u32::try_from(order).unwrap_or_else(|_| {
-        panic!("generator {generator} exact permutation order does not fit in u32")
-    })
+    u32::try_from(order).ok()
 }
 
 fn permutations_commute(left: &[u32], right: &[u32]) -> bool {
     (0..left.len()).all(|q| left[right[q] as usize] == right[left[q] as usize])
 }
 
-pub(super) fn checked_group_order(orders: &[u32]) -> usize {
-    orders.iter().enumerate().fold(1usize, |acc, (g, &value)| {
-        acc.checked_mul(value as usize)
-            .unwrap_or_else(|| panic!("group order overflows usize at generator {g}"))
-    })
+pub(super) fn checked_group_order(orders: &[u32]) -> Option<usize> {
+    orders
+        .iter()
+        .try_fold(1usize, |acc, &value| acc.checked_mul(value as usize))
 }
 
 pub(super) fn validate_site_count(n: usize, context: &str) {
@@ -66,9 +62,7 @@ pub(super) fn validate_site_count(n: usize, context: &str) {
 ///
 /// Every variant is caller-supplied-input error, and its [`Display`]
 /// text is exactly what [`TranslationGroup::from_generators`] panics
-/// with. Arithmetic overflow in the group order or character phase
-/// modulus is NOT covered — that needs generator orders in the billions
-/// and still panics.
+/// with.
 ///
 /// [`Display`]: std::fmt::Display
 #[derive(Debug, Clone, PartialEq, Eq)]
@@ -99,6 +93,10 @@ pub enum GroupError {
     },
     /// Two generators do not commute, so they generate no abelian group.
     NonCommuting { left: usize, right: usize },
+    /// A generator's exact cyclic order does not fit in `u32`.
+    PermutationOrderOverflow { generator: usize },
+    /// The group order `Π orders[g]` does not fit in `usize`.
+    GroupOrderOverflow,
 }
 
 impl std::fmt::Display for GroupError {
@@ -142,6 +140,11 @@ impl std::fmt::Display for GroupError {
             Self::NonCommuting { left, right } => {
                 write!(f, "generators {left} and {right} do not commute")
             }
+            Self::PermutationOrderOverflow { generator } => write!(
+                f,
+                "generator {generator} exact permutation order does not fit in u32"
+            ),
+            Self::GroupOrderOverflow => write!(f, "group order overflows usize"),
         }
     }
 }
@@ -438,7 +441,8 @@ impl TranslationGroup {
             if declared == 0 {
                 return Err(GroupError::ZeroOrder { generator });
             }
-            let exact = permutation_order(&perms[generator], generator);
+            let exact = permutation_order(&perms[generator], generator)
+                .ok_or(GroupError::PermutationOrderOverflow { generator })?;
             if declared != exact {
                 return Err(GroupError::OrderMismatch {
                     generator,
@@ -454,10 +458,12 @@ impl TranslationGroup {
                 }
             }
         }
-        let order = checked_group_order(&orders);
-        let phase_modulus = orders.iter().fold(1usize, |acc, &value| {
-            checked_lcm(acc, value as usize, "character phase modulus")
-        });
+        let order = checked_group_order(&orders).ok_or(GroupError::GroupOrderOverflow)?;
+        // lcm divides the product, so this cannot overflow once `order` fits.
+        let phase_modulus = orders
+            .iter()
+            .try_fold(1usize, |acc, &value| checked_lcm(acc, value as usize))
+            .ok_or(GroupError::GroupOrderOverflow)?;
         let block_cyclic = detect_block_cyclic(n_qubits, &perms, &orders);
         let rotations = perms
             .iter()

crates/ppvm-pauli-sum/src/symmetry/momentum.rs

@@ -88,7 +88,11 @@ impl TranslationGroup {
                 *c = 0;
             }
         }
-        CharacterTable { numerators, values }
+        CharacterTable {
+            orders: self.orders.clone(),
+            numerators,
+            values,
+        }
     }
 
     /// Everything the phase-aware routines need about `w`'s orbit in
@@ -148,8 +152,7 @@ impl TranslationGroup {
         S: BuildHasher + Clone + Default + HashFinalize,
     {
         assert_eq!(
-            table.len(),
-            self.order(),
+            table.orders, self.orders,
             "character table does not belong to this group"
         );
         let (rep, idx, stabilizer) =
@@ -163,6 +166,9 @@ impl TranslationGroup {
 /// [`TranslationGroup::character_table`].
 #[derive(Debug, Clone)]
 pub struct CharacterTable {
+    /// Generator orders of the originating group; element indices (and so
+    /// the table) are only meaningful for a group with the same orders.
+    orders: Vec<u32>,
     /// Exact phase numerators (see `character_numerator`); `0` ⇔ `χ = 1`.
     numerators: Vec<usize>,
     values: Vec<Complex<f64>>,

crates/ppvm-pauli-sum/src/symmetry/tests.rs

@@ -824,7 +824,27 @@ fn rejects_group_order_overflow() {
     } else {
         vec![u32::MAX, u32::MAX]
     };
-    assert!(std::panic::catch_unwind(|| { super::group::checked_group_order(&orders) }).is_err());
+    assert_eq!(super::group::checked_group_order(&orders), None);
+}
+
+#[test]
+fn try_from_generators_reports_overflow() {
+    use super::GroupError;
+    // 64 copies of one swap: a valid direct product of order 2^64.
+    let err = TranslationGroup::try_from_generators(2, vec![vec![1, 0]; 64], vec![2; 64])
+        .expect_err("group order must overflow");
+    assert_eq!(err, GroupError::GroupOrderOverflow);
+
+    // Disjoint prime cycles 2..29 on 129 qubits: order 6469693230 > u32::MAX.
+    let mut perm = Vec::new();
+    for len in [2u32, 3, 5, 7, 11, 13, 17, 19, 23, 29] {
+        let start = perm.len() as u32;
+        perm.extend((0..len).map(|j| start + (j + 1) % len));
+    }
+    let n = perm.len();
+    let err = TranslationGroup::try_from_generators(n, vec![perm], vec![1])
+        .expect_err("permutation order must overflow u32");
+    assert_eq!(err, GroupError::PermutationOrderOverflow { generator: 0 });
 }
 
 // ---------------------------------------------------------------------------
@@ -1100,3 +1120,12 @@ fn orbit_yields_hashed_words() {
         assert_eq!(fxhash::hash64(&member), fxhash::hash64(&fresh));
     }
 }
+
+#[test]
+#[should_panic(expected = "character table does not belong to this group")]
+fn character_table_rejects_a_group_of_equal_order() {
+    // Both groups have order 4, but their element indices mean different things.
+    let table = TranslationGroup::chain_1d(4).character_table(&[1]);
+    let torus = TranslationGroup::torus_2d(2, 2);
+    torus.canonicalize_in_sector_indexed(&word("XIII"), &table);
+}

crates/ppvm-python-native/src/lindblad.rs

@@ -228,6 +228,13 @@ impl LindbladSpec {
     ) -> PyResult<PyPauliMap<'py>> {
         with_spec!(&self.inner, inner, C => {
             let p_slice = p.as_slice()?;
+            if p_slice.len() != inner.n_qubits() {
+                return Err(PyValueError::new_err(format!(
+                    "p has {} entries but spec.n_qubits = {}",
+                    p_slice.len(),
+                    inner.n_qubits()
+                )));
+            }
             let p_word = word_from_codes::<C>(p_slice).map_err(map_err)?;
             let pairs = inner.action(&p_word);
             pack_pauli_map(py, pairs, inner.n_qubits())

ppvm-python/pyproject.toml

@@ -58,8 +58,10 @@ dev = [
     "pytest>=9.0.2",
     "pytest-benchmark>=5.2.3",
 ]
-# Optional: only the `demo/` scripts use it (`expm_multiply` for the reference
-# matrix exponential). Runtime ppvm and the test suite have no scipy dep.
+# Optional: only the `demo/` scripts use these (`expm_multiply` for the
+# reference matrix exponential, plus plotting). Runtime ppvm and the test
+# suite depend on neither.
 demo = [
+    "matplotlib>=3.8",
     "scipy>=1.13",
 ]

ppvm-python/src/ppvm/_codes.py

@@ -0,0 +1,29 @@
+# SPDX-FileCopyrightText: 2026 The PPVM Authors
+# SPDX-License-Identifier: Apache-2.0
+
+"""Coercion of array-like Pauli codes to the uint8 layout the core expects."""
+
+from __future__ import annotations
+
+import numpy as np
+import numpy.typing as npt
+
+
+def as_pauli_codes(codes: npt.ArrayLike) -> np.ndarray:
+    """Return `codes` as a C-contiguous uint8 array of Pauli codes
+    (``0=I, 1=X, 2=Z, 3=Y``).
+
+    uint8 input passes through unchanged (the core range-checks it). Any
+    other input must be integer-typed with every value in ``0..=3``, so an
+    out-of-range code raises ``ValueError`` instead of wrapping on the cast
+    (e.g. ``256`` silently becoming ``0 = I``).
+    """
+    arr = np.asarray(codes)
+    if arr.dtype != np.uint8 and arr.size:
+        if not np.issubdtype(arr.dtype, np.integer):
+            raise ValueError(f"Pauli codes must be integers, got dtype {arr.dtype}")
+        lo, hi = arr.min(), arr.max()
+        if lo < 0 or hi > 3:
+            bad = lo if lo < 0 else hi
+            raise ValueError(f"Pauli code must be 0 (I), 1 (X), 2 (Z), or 3 (Y); got {bad}")
+    return np.ascontiguousarray(arr, dtype=np.uint8)