Merge pull request #282 from AdaWorldAPI/claude/grammar-fixes-r2-2026-04-29

fix: Grammar/Markov hardening — slice unification, kernel wiring, parser tests, triangle distance

Author: AdaWorldAPI

crates/deepnsm/Cargo.toml

@@ -4,6 +4,12 @@ version = "0.1.0"
 edition = "2021"
 license = "Apache-2.0"
 publish = false
+
+# Empty `[workspace]` so cargo treats this crate as standalone when invoked
+# via `--manifest-path` (deepnsm is `exclude`d from the parent workspace,
+# but in nested git-worktree directories cargo's auto-discovery would
+# otherwise walk further up and pick up the outer workspace root).
+[workspace]
 description = """
 DeepNSM: Distributional semantic transformer replacement.
 4,096 words × 12 bits × 8MB distance matrix = complete semantic engine.
@@ -20,12 +26,17 @@ No GPU. No learned weights. Same decision boundaries as cosine.
 # never touches backend optimization files.
 [features]
 default = []
-contract-ticket = ["dep:lance-graph-contract"]
+contract-ticket = []
 grammar-triangle = ["dep:lance-graph-cognitive"]
 
 [dependencies]
 ndarray = { path = "../../../ndarray", default-features = false, features = ["std"] }
-lance-graph-contract = { path = "../lance-graph-contract", optional = true }
+# `lance-graph-contract` is now a hard dep: markov_bundle imports the canonical
+# RoleKeySlice constants (SUBJECT_SLICE..) from `grammar::role_keys` so that
+# slice coordinates are unified with the rest of the workspace (CRITICAL fix
+# from PR #279 review — the equal-partition 16384/5 layout in markov_bundle
+# disagreed with the domain-specific widths in role_keys).
+lance-graph-contract = { path = "../lance-graph-contract" }
 lance-graph-cognitive = { path = "../lance-graph-cognitive", optional = true }
 
 [dev-dependencies]

crates/deepnsm/src/lib.rs

@@ -63,6 +63,7 @@ pub mod vocabulary;
 
 pub mod trajectory;
 pub mod markov_bundle;
+pub mod nsm_primes;
 
 #[cfg(feature = "contract-ticket")]
 pub mod ticket_emit;

crates/deepnsm/src/markov_bundle.rs

@@ -1,7 +1,21 @@
 //! META-AGENT: add `pub mod markov_bundle;` to lib.rs.
+//!
+//! Slice coordinates are imported from `lance_graph_contract::grammar::role_keys`
+//! so that this module and the rest of the workspace agree on the [start:stop)
+//! boundaries of every grammatical role inside the 16384-dim VSA carrier.
+//! The previously hard-coded equal-partition layout (`16384 / 5 = 3277` per
+//! role) was incompatible with the domain-specific widths in role_keys (e.g.
+//! SUBJECT owns [0..2000), TEMPORAL owns [9000..9200)) — see PR #279 review,
+//! CRITICAL #1.
 
 use crate::trajectory::Trajectory;
 
+use lance_graph_contract::grammar::role_keys::{
+    CONTEXT_SLICE, INSTRUMENT_SLICE, KAUSAL_SLICE, LOKAL_SLICE, MODAL_SLICE,
+    MODIFIER_SLICE, OBJECT_SLICE, PREDICATE_SLICE, RoleKeySlice, SUBJECT_SLICE,
+    TEMPORAL_SLICE, VSA_DIMS,
+};
+
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]
 pub enum Kernel {
     Uniform,
@@ -36,20 +50,26 @@ pub enum GrammaticalRole {
 }
 
 impl GrammaticalRole {
-    /// Slice of the 16384-dim VSA carrier that owns this role.
-    pub fn slice(&self) -> (usize, usize) {
+    /// Canonical [start:stop) slice of the 16384-dim VSA carrier that owns
+    /// this role, sourced from `lance_graph_contract::grammar::role_keys`.
+    /// This is the single source of truth — the constants below are simple
+    /// re-exports of the contract crate's `RoleKeySlice` descriptors so that
+    /// every consumer (markov bundler, role-key catalogue, slice-aware
+    /// codecs) agrees on the same boundaries.
+    pub fn slice(&self) -> RoleKeySlice {
         match self {
-            Self::Subject => (0, 3277),
-            Self::Predicate => (3277, 6554),
-            Self::Object => (6554, 9830),
-            Self::Modifier => (9830, 13107),
-            Self::Context => (13107, 16384),
-            // TEKAMOLO sub-slices inside Context band.
-            Self::Temporal => (13107, 13762),
-            Self::Kausal => (13762, 14418),
-            Self::Modal => (14418, 15074),
-            Self::Lokal => (15074, 15729),
-            Self::Instrument => (15729, 16384),
+            Self::Subject => SUBJECT_SLICE,
+            Self::Predicate => PREDICATE_SLICE,
+            Self::Object => OBJECT_SLICE,
+            Self::Modifier => MODIFIER_SLICE,
+            Self::Context => CONTEXT_SLICE,
+            // TEKAMOLO sub-slices (NOT inside Context — they live in their
+            // own [9000..9650) post-context band per role_keys.rs layout).
+            Self::Temporal => TEMPORAL_SLICE,
+            Self::Kausal => KAUSAL_SLICE,
+            Self::Modal => MODAL_SLICE,
+            Self::Lokal => LOKAL_SLICE,
+            Self::Instrument => INSTRUMENT_SLICE,
         }
     }
 }
@@ -77,7 +97,9 @@ impl MarkovBundler {
         Self {
             radius,
             kernel,
-            dims: 16_384,
+            // Width of the canonical VSA carrier — kept in lock-step with
+            // `lance_graph_contract::grammar::role_keys::VSA_DIMS` (16_384).
+            dims: VSA_DIMS,
             buffer: std::collections::VecDeque::with_capacity((2 * radius + 1) as usize),
         }
     }
@@ -101,18 +123,33 @@ impl MarkovBundler {
             let delta = (i as i32) - focal;
             let weight = self.kernel.weight(delta, self.radius);
             for tok in &sent.tokens {
-                let (start, stop) = tok.role.slice();
-                let len = (stop - start).min(tok.content_fp.len());
+                let slice = tok.role.slice();
+                // Use the canonical role_keys width (NOT an equal partition).
+                let len = slice.len().min(tok.content_fp.len());
                 for k in 0..len {
-                    acc[start + k] += weight * tok.content_fp[k];
+                    acc[slice.start + k] += weight * tok.content_fp[k];
                 }
             }
         }
-        // permute by position offset (rotate_right)
-        if !acc.is_empty() {
-            let k = (self.radius as usize) % acc.len();
-            acc.rotate_right(k);
+        // REMOVED: post-bundle acc.rotate_right(k) — corrupted role-slice alignment.
+        // Plan called for per-sentence pre-bundle vsa_permute; that's a follow-up.
+        // Until then, no permutation = aligned bundle.
+
+        // Bundle normalization (HIGH item from PR #279 review): divide by the
+        // sum of |kernel weights| so cosine comparisons across kernel choices
+        // are invariant to kernel-shape magnitude. Without this, MexicanHat
+        // bundles have systematically smaller norms than Uniform bundles
+        // simply because the kernel weights peak at 1 and decay.
+        let radius_i = self.radius as i32;
+        let total_abs_weight: f32 = (-radius_i..=radius_i)
+            .map(|d| self.kernel.weight(d, self.radius).abs())
+            .sum();
+        if total_abs_weight > 1e-9 {
+            for v in acc.iter_mut() {
+                *v /= total_abs_weight;
+            }
         }
+
         Trajectory {
             fingerprint: acc,
             radius: self.radius,
@@ -158,8 +195,180 @@ mod tests {
     }
     #[test]
     fn role_slices_disjoint() {
+        // SPO core slices are contiguous: SUBJECT.stop == PREDICATE.start by
+        // construction in `role_keys.rs` (0..2000, 2000..4000, ...).
         let s = GrammaticalRole::Subject.slice();
         let p = GrammaticalRole::Predicate.slice();
-        assert_eq!(s.1, p.0);
+        assert_eq!(s.stop, p.start);
+    }
+
+    #[test]
+    fn role_slice_widths_match_role_keys_canonical() {
+        // Spot-check that `GrammaticalRole::slice` returns the role_keys-canonical
+        // widths (NOT the old equal-partition 16384/5 = 3277 layout).
+        assert_eq!(GrammaticalRole::Subject.slice().len(),    2000);
+        assert_eq!(GrammaticalRole::Predicate.slice().len(),  2000);
+        assert_eq!(GrammaticalRole::Object.slice().len(),     2000);
+        assert_eq!(GrammaticalRole::Modifier.slice().len(),   1500);
+        assert_eq!(GrammaticalRole::Context.slice().len(),    1500);
+        assert_eq!(GrammaticalRole::Temporal.slice().len(),    200);
+        assert_eq!(GrammaticalRole::Kausal.slice().len(),      200);
+        assert_eq!(GrammaticalRole::Modal.slice().len(),       100);
+        assert_eq!(GrammaticalRole::Lokal.slice().len(),       150);
+        assert_eq!(GrammaticalRole::Instrument.slice().len(),  100);
+    }
+
+    /// Helper: fill a bundler's window so a single push triggers `bundle_current`.
+    fn fill_and_bundle(
+        kernel: Kernel,
+        radius: u32,
+        sent: WindowedSentence,
+    ) -> Trajectory {
+        let mut b = MarkovBundler::new(radius, kernel);
+        let cap = (2 * radius + 1) as usize;
+        let mut last: Option<Trajectory> = None;
+        for _ in 0..cap {
+            last = b.push(sent.clone());
+        }
+        last.expect("bundler should emit a trajectory once window is full")
+    }
+
+    /// Helper: push a sequence of distinct sentences so per-position
+    /// kernel weights actually shape the bundle. Returns the trajectory
+    /// emitted on the final push (window saturated).
+    fn bundle_sequence(
+        kernel: Kernel,
+        radius: u32,
+        sentences: Vec<WindowedSentence>,
+    ) -> Trajectory {
+        let mut b = MarkovBundler::new(radius, kernel);
+        let cap = (2 * radius + 1) as usize;
+        assert_eq!(sentences.len(), cap, "sequence must fill exactly one window");
+        let mut last: Option<Trajectory> = None;
+        for s in sentences {
+            last = b.push(s);
+        }
+        last.expect("bundler should emit on the saturating push")
+    }
+
+    /// REGRESSION (PR #279 CRITICAL #2): the removed `rotate_right` shifted
+    /// SUBJECT-slice content into the PREDICATE slice (or worse, the
+    /// CONTEXT band). After the fix, a SUBJECT-only window must keep all
+    /// non-zero content inside `[0, 3277)` and have ~zero everywhere else.
+    #[test]
+    fn bundle_does_not_rotate_subject_dims_outside_subject_slice() {
+        // SUBJECT-only window: every sentence has a single SUBJECT token
+        // whose content_fp is all 1.0 across the SUBJECT slice.
+        let subject_len = GrammaticalRole::Subject.slice().stop
+            - GrammaticalRole::Subject.slice().start;
+        let sent = WindowedSentence {
+            tokens: vec![TokenWithRole {
+                content_fp: vec![1.0; subject_len],
+                role: GrammaticalRole::Subject,
+            }],
+        };
+        let traj = fill_and_bundle(Kernel::Uniform, 5, sent);
+
+        let _slice = GrammaticalRole::Subject.slice();
+        let s_start = _slice.start;
+        let s_stop = _slice.stop;
+        // SUBJECT slice should be non-zero (positive after normalization).
+        let subject_sum: f32 =
+            traj.fingerprint[s_start..s_stop].iter().sum();
+        assert!(
+            subject_sum > 1.0,
+            "expected non-trivial SUBJECT content, got sum={subject_sum}"
+        );
+        // Outside the SUBJECT slice every dim must be ~0 (no rotation).
+        let outside_max: f32 = traj.fingerprint[s_stop..]
+            .iter()
+            .fold(0.0f32, |acc, v| acc.max(v.abs()));
+        assert!(
+            outside_max < 1e-6,
+            "rotation leaked SUBJECT content past slice boundary: \
+             max |outside| = {outside_max}"
+        );
+    }
+
+    /// MexicanHat and Uniform kernels must produce materially different
+    /// bundles on the same window — otherwise the kernel selector is
+    /// ineffective at runtime. Uses an asymmetric heterogeneous window
+    /// (one outlier position carries content; others are blank) so that
+    /// per-position kernel weights reshape the accumulated bundle in a
+    /// way symmetric kernels can't equalize.
+    #[test]
+    fn mexican_hat_bundle_differs_from_uniform_bundle() {
+        let subject_len = GrammaticalRole::Subject.slice().stop
+            - GrammaticalRole::Subject.slice().start;
+        let radius = 5u32;
+        let cap = (2 * radius + 1) as usize;
+        // Single outlier at position 1 (delta = -4). Uniform weights this
+        // identically to focal; MexicanHat strongly attenuates it
+        // (w(-4, 5) ≈ 0.26 vs w(0, 5) = 1.0). Normalization divides each
+        // by its own Σ|w|, so the per-dim values differ across the
+        // SUBJECT slice.
+        let outlier_pos = 1usize;
+        let sentences: Vec<WindowedSentence> = (0..cap)
+            .map(|i| WindowedSentence {
+                tokens: vec![TokenWithRole {
+                    content_fp: vec![
+                        if i == outlier_pos { 1.0 } else { 0.0 };
+                        subject_len
+                    ],
+                    role: GrammaticalRole::Subject,
+                }],
+            })
+            .collect();
+        let uni = bundle_sequence(Kernel::Uniform, radius, sentences.clone());
+        let mex = bundle_sequence(Kernel::MexicanHat, radius, sentences);
+        assert_eq!(uni.fingerprint.len(), mex.fingerprint.len());
+        let l2: f32 = uni
+            .fingerprint
+            .iter()
+            .zip(mex.fingerprint.iter())
+            .map(|(a, b)| (a - b) * (a - b))
+            .sum::<f32>()
+            .sqrt();
+        assert!(
+            l2 > 1e-3,
+            "MexicanHat bundle should differ from Uniform bundle, l2={l2}"
+        );
+    }
+
+    /// Bundle normalization (HIGH from PR #279) makes the L2 norm
+    /// invariant to kernel-shape magnitude. We assert all three kernels
+    /// land in a loose [0.5, 1.5] band on a controlled SUBJECT-only window.
+    #[test]
+    fn bundle_l2_norm_invariant_to_kernel() {
+        let subject_len = GrammaticalRole::Subject.slice().stop
+            - GrammaticalRole::Subject.slice().start;
+        let sent = WindowedSentence {
+            tokens: vec![TokenWithRole {
+                content_fp: vec![1.0; subject_len],
+                role: GrammaticalRole::Subject,
+            }],
+        };
+        for k in [Kernel::Uniform, Kernel::MexicanHat, Kernel::Gaussian] {
+            let traj = fill_and_bundle(k, 5, sent.clone());
+            // Per-dim mean of |v| × sqrt(N_subj) ≈ L2 norm; we test L2 directly.
+            let l2: f32 = traj
+                .fingerprint
+                .iter()
+                .map(|v| v * v)
+                .sum::<f32>()
+                .sqrt();
+            // Each SUBJECT dim sums to (Σ_i w_i) / (Σ_i |w_i|). For Uniform
+            // and Gaussian (all-positive weights) this is exactly 1.0 per dim,
+            // so L2 = sqrt(subject_len) ≈ 57.2. For Mexican-hat the negative
+            // brim cancels part of the positive core, dropping the per-dim
+            // value but keeping it within the same order of magnitude.
+            // We loose-bound on L2 / sqrt(subject_len) ∈ [0.5, 1.5].
+            let scale = (subject_len as f32).sqrt();
+            let norm_l2 = l2 / scale;
+            assert!(
+                (0.5..=1.5).contains(&norm_l2),
+                "kernel {k:?}: normalized L2 {norm_l2} (raw {l2}) out of [0.5, 1.5]"
+            );
+        }
     }
 }