deepnsm: sentence-level AriGraph reader — left-corner state machine + P64 discrete substrate#479
Merged
Merged
Conversation
Five new modules implementing the reading surface described in the DeepNSM
endgame spec. The architecture is a left-corner transition: top-down
expectation + bottom-up parser evidence + ±5 sentence window → episodic
SPO frames + next reading state.
morphology.rs MorphFlags(u16) bitfield — 14 heuristic flags derived
from SentenceStructure (negation, temporal, clause type)
cam64.rs Cam64(u64) — 8-lane × 8-bit reading-state locality key.
NOT the semantic truth: the EpisodicSpoFrame rows are the
auditable witnesses. Cam64 is the fast basin-matching and
prefetch index. Lane layout matches the left-corner state:
entity / predicate / object / morph / clause / discourse /
causal / basin.
episodic_spo.rs EpisodicSpoFrame — one auditable row per triple per
sentence. All 25 spec columns present: position, lexical,
syntactic roles, SPO candidates, coreference, window
offset, NSM masks, CAM code, quality markers, Cam64.
BasinClassification enum covers the AriGraph crystallisation
lifecycle (Reinforcement / NoveltyDelta / WisdomDelta /
Contradiction / Branch / Epiphany).
window.rs SentenceWindow — ±5 exact entity candidate ring buffer for
Wernicke coreference resolution. Separate from ContextWindow
(Broca VSA projection band). resolve_pronoun() uses
last-mentioned-first recency heuristic (v1); antecedent
ranking (gender/number/type agreement) is v2.
reader_state.rs ReadingState + step() + LeftCornerTrigger + SentenceFeatures.
step(self, sentence, features) -> (Vec<EpisodicSpoFrame>, Self)
is pure: no &mut self during computation (data-flow.md rule).
LeftCornerTrigger encodes first-evidence frame selection
(Causal / Temporal / Relative / Anaphora / Domain).
44 tests, 0 failures.
https://claude.ai/code/session_0147hSzjmWZDuy2MSQNrhEK5
Three-file narrow commit closing the operational gap in the left-corner
reading state machine:
window.rs — forward expectation slots (Pika chart-arc pre-population)
- Add ExpectedReason enum (RelativeClause/Anaphora/Ellipsis/CausalContinuation/
TemporalContinuation) and ExpectedSlot struct
- Add expected: [ExpectedSlot; 4] + expected_count to SentenceWindow
- push_expected(rank, reason): pre-populate slot from left-corner trigger
- clear_expected() / iter_expected() for slot lifecycle management
- resolve_pronoun() now checks expected slots FIRST (Pika priority), then
falls back to confirmed sentence ring (recency heuristic)
- Update Clone impl; 6 new tests
cam64.rs — basin continuation predicate (chart-arc match)
- continues_basin(prev: Cam64) -> bool: popcount on XOR,
shared ≥ 16 bits AND diff ≤ 24 bits → same story arc
- basin_continuation_score(prev) -> u8: graded 0-255, v2 stub
- CAM64_CONTINUATION_MIN_SHARED = 16, CAM64_CONTINUATION_MAX_DIFF = 24
- 5 new tests including same/near/far code cases
reader_state.rs — trigger wiring into window expectations
- Relative trigger: push active_subject as RelativeClause expected slot
- Anaphora trigger: push active_subject as Anaphora expected slot
- Import ExpectedReason from window module
- 2 new tests: relative_trigger_pushes_expected_subject,
pronoun_prefers_expected_relative_subject
All 152 deepnsm tests pass (0 failures).
https://claude.ai/code/session_0147hSzjmWZDuy2MSQNrhEK5
…ubstrate
Closes the integer-substrate gap between the DeepNSM grammar reader and the
holograph bitpacked resonance engine. No floats in the hot path.
signed_crystal.rs — new module, 18 tests:
SignedOffset4(u8):
- 0..14 encodes -7..+7 (raw = offset + 7)
- 15 = overflow / basin-change / unknown sentinel
- from_offset(i8) → to_offset() round-trips cleanly
Crystal4096(u16):
- three SignedOffset4 axes packed into 12 bits (X=bits0-3, Y=4-7, Z=8-11)
- 4096 valid cells = direct index into P4096 palette codebook
- nibble_distance() counts differing axes (0-3)
- same_basin() = no overflow AND nibble_distance ≤ 1
- xor() for VSA bind/unbind (self-inverse)
P64MeaningField(u64):
- derived from Cam64 + NSM prime mask (low 16 bits folded via XOR into lanes 3-4)
- agreement() = 64 - (self XOR other).count_ones() (no floats)
- bind() = XOR (VSA binding)
- distinct from Cam64: Cam64 is locality key; P64 is meaning-field projection
SignedSentenceCrystal { p64, coord }:
- same_basin_as(): agreement ≥ 40 AND nibble_distance ≤ 1
- bind(): XOR both p64 and coord for holograph integration
- new() from cam64 + nsm + three i8 offsets
Convenience helpers:
- crystal_from_frame_context(): builds from EpisodicSpoFrame fields
- basin_delta_from_cam(prev, curr): 0 if continues_basin, 1 otherwise
Architecture:
DeepNSM step() → EpisodicSpoFrame (auditable SPO truth)
→ P64MeaningField (grammar/NSM meaning lattice)
→ Crystal4096 (P4096 codebook key, 12 bits)
holograph SemanticCrystal → BitpackedVector (16Kbit fingerprint)
AriGraph basin ← XOR-bind(Crystal4096, BitpackedVector) → tombstone witness
Floats remain only in EpisodicSpoFrame quality fields (confidence, novelty,
wisdom, entropy) as boundary annotations, not the hot-path substrate.
170 deepnsm tests pass (0 failures).
https://claude.ai/code/session_0147hSzjmWZDuy2MSQNrhEK5
…96 codebook
Establishes P64 as the native address space, not a quantised float approximation.
Architecture axiom burned into the type system:
P64 is the native symbolic-palette layer.
Floats may approximate P64 for external ML interop.
P64 does not approximate floats.
sentence_transformer64.rs — new module, 26 tests:
P64(u64) — 8×8-bit vertical meaning field
- 8 orthogonal semantic planes: entity/predicate/object/morph/clause/
discourse/causal/basin (same physics as Cam64, different contract)
- bind() = XOR (VSA, self-inverse)
- agreement() = 64 - popcount(XOR): integer similarity, no cosine
- same_basin() = agreement ≥ 40 bits
- from_cam64_and_nsm(): canonical construction — NSM primes fold into
lanes 3-4 via XOR; no float arithmetic
- From<Cam64>: lifts locality key into meaning field when NSM absent
- Forbidden: no from_f32_embedding() internal path
Cam4096(u16) — 12-bit deterministic codebook address
- from_p64(): lane nibble fold (entity top-4, predicate top-4, basin top-4)
- NOT nearest-neighbour in float space — exact bitwise selection
- nibble_distance() / near_match() / exact_match() — integer only
- 4096 cells = native-English reading-state classes, full resolution
Perturbation4x4 — local 4×4 discrete ambiguity tile
- row axis = semantic lane perturbation (entity/predicate shift)
- col axis = syntactic/pragmatic perturbation (clause/discourse shift)
- encode_cell(sem: i8, syn: i8) / semantic_delta / syntactic_delta
- apply(p64, cell_idx): perturbs lanes 0 + 4, wraps within 8-bit lane
- IDENTITY: all-zero tile (no perturbation)
splat_p64(centre, tile, radius_bits) → SmallNeighbourhood (≤ 16)
- Discrete palette splat: NOT Gaussian in f32 space
- Keeps only cells that: actually change the P64, stay within Hamming
radius, and near_match the centre's CAM4096
- SmallNeighbourhood: fixed [SplatNeighbour; 16], no heap allocation
EpisodicSpoHint { subject, predicate, object, role }
- Compact reference into EpisodicSpoFrame for AriGraph commitment
- from_primary_frame(&[EpisodicSpoFrame]) → Self
Sentence64 { p64, cam, spo_hint }
- complete output: meaning field + codebook address + SPO reference
- cam is always Cam4096::from_p64(p64) — deterministic, never cached separately
- same_basin_as(): p64.same_basin AND cam.near_match
SentenceTransformer64
- project(cam, nsm, subject, predicate, object, role) → Sentence64
- project_from_frame(&EpisodicSpoFrame) → Sentence64
- project_frames(&[EpisodicSpoFrame]) → Vec<Sentence64>
- local_tile(p64, entity_step, clause_step) → Perturbation4x4
Full flow documented in module header:
sentence
→ SentenceTransformer64::project()
→ Sentence64 { p64, cam4096, spo_hint }
→ EpisodicSpoFrame (truth witness)
→ holograph BitpackedVector (resonance, 16Kbit)
→ AriGraph basin update
200 deepnsm tests pass (0 failures).
https://claude.ai/code/session_0147hSzjmWZDuy2MSQNrhEK5
Covers all seven new modules in crates/deepnsm/src/: morphology, cam64, episodic_spo, window, reader_state, signed_crystal, sentence_transformer64. Documents: - two-faculty split (Broca ContextWindow vs Wernicke SentenceWindow) - left-corner state machine + Pika expectation slots - P64 as native address space (not quantised floats) - Cam4096 deterministic fold (not float nearest-neighbour) - discrete palette splat (not Gaussian in f32 space) - float boundary policy (hot path = zero floats) - relationship to holograph (large-field resonance) and AriGraph (basin) - test summary: 200 passing across all modules https://claude.ai/code/session_0147hSzjmWZDuy2MSQNrhEK5
|
Warning Review limit reached
More reviews will be available in 48 minutes and 47 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR implements a deterministic sentence-level DeepNSM reader: morphology flags, an 8-lane Cam64 locality code, EpisodicSpoFrame emission, a ±5 SentenceWindow for coreference, a LeftCorner ReadingState step transition, discrete SignedSentenceCrystal helpers, and a deterministic P64-based SentenceTransformer with neighborhood splatting. ChangesDeepNSM Sentence-Level Reader Pipeline
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1711336f97
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
One-liner definitions for the eight terms most likely to cause confusion: P64, CAM4096, Crystal4096, Cam64, EpisodicSpoFrame, SentenceWindow, splat_p64, SentenceTransformer64. Key line: CAM4096 is a 12-bit deterministic address selected from P64 lanes, not a quantized embedding vector. https://claude.ai/code/session_0147hSzjmWZDuy2MSQNrhEK5
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (11)
crates/deepnsm/src/window.rs (1)
157-160: 💤 Low valueDocstring claims reverse order but slice is FIFO.
iter_expected()returns the slice in push order (oldest-first), not "most-recently-pushed first" as the docstring states. The caller (resolve_pronoun) correctly calls.iter().rev()to get the desired order, so behavior is fine—only the docstring is misleading.📝 Suggested docstring fix
- /// Iterate the active expectation slots (most-recently-pushed first). + /// Return a slice of active expectation slots in push order (FIFO). + /// Callers needing most-recently-pushed first should iterate in reverse. pub fn iter_expected(&self) -> &[ExpectedSlot] {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/deepnsm/src/window.rs` around lines 157 - 160, The docstring for iter_expected() is incorrect: it says "most-recently-pushed first" but the function returns the slice in push order (oldest-first). Update the docstring on the iter_expected method to accurately state that it returns the active expectation slots in push order (oldest-first) or clarify that callers should reverse the slice (e.g., resolve_pronoun currently calls .iter().rev()). Mention the function name iter_expected (and optionally resolve_pronoun) so reviewers can locate the change.crates/deepnsm/src/episodic_spo.rs (2)
172-195: 💤 Low value
Branchvariant is defined but never returned bybasin_classification().The
BasinClassification::Branchvariant is declared but the classification logic has no branch that returns it. If this is intentional (e.g., assigned externally or reserved for v2), consider adding a brief comment. Otherwise, the unreachable variant may mislead consumers.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/deepnsm/src/episodic_spo.rs` around lines 172 - 195, The BasinClassification::Branch variant is never returned by basin_classification(), so either add a clear branch in the decision logic that returns BasinClassification::Branch under the appropriate condition (e.g., a specific novelty/entropy/confidence/wisdom threshold or a new property) or add a comment in the basin_classification() function noting that Branch is intentionally unused/reserved for v2; update basin_classification() (and mention BasinClassification::Branch) so the enum definition and the function logic are consistent.
29-31: ⚡ Quick winDuplicate
NO_ROLEsentinel definition.This duplicates
NO_ROLEfromcrate::spo(per context snippet showingspo::NO_ROLE = 0xFFF). Consider re-exporting from a single source to avoid divergence risk if the sentinel value ever changes.♻️ Suggested consolidation
-/// Sentinel: "no role / unresolved" for 12-bit vocabulary ranks. -/// Mirrors `spo::NO_ROLE`. -pub const NO_ROLE: u16 = 0xFFF; +// Re-export from canonical location to avoid duplication. +pub use crate::spo::NO_ROLE;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/deepnsm/src/episodic_spo.rs` around lines 29 - 31, The NO_ROLE sentinel in episodic_spo.rs duplicates crate::spo::NO_ROLE (0xFFF); remove the duplicate pub const NO_ROLE from episodic_spo.rs and instead re-export or alias the canonical value from crate::spo (e.g., use or pub use crate::spo::NO_ROLE) so all code refers to a single source of truth and prevents drift if the sentinel changes.crates/deepnsm/src/reader_state.rs (1)
66-82: 💤 Low value
Domain(u8)tag is silently truncated to 7 bits.
basin_byte()masks the domain tag with& 0x7F, so values 128-255 lose their high bit. If this is intentional (to reserve bit 7 for the domain marker), a brief docstring note would help prevent caller confusion.📝 Suggested docstring clarification
- /// Domain-specific trigger from caller (e.g. "invoice" → business-document frame). - Domain(u8), // caller-supplied domain tag (0-255) + /// Domain-specific trigger from caller (e.g. "invoice" → business-document frame). + /// Note: Only bits 0-6 are preserved; bit 7 is used as the domain marker in basin_byte(). + Domain(u8),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/deepnsm/src/reader_state.rs` around lines 66 - 82, The Domain(u8) variant of LeftCornerTrigger is being masked to 7 bits in basin_byte() which silently drops bit 7; update the docstring for LeftCornerTrigger::basin_byte (or the LeftCornerTrigger enum) to explicitly state that Domain(tag) intentionally reserves the high bit as the domain marker and therefore the tag is masked with 0x7F (so callers must supply tags in 0..=127), or if you intend to preserve full 8-bit tags change the implementation to use 0x80 | tag without masking—reference LeftCornerTrigger, its Domain(u8) variant, and the basin_byte() method when making the clarification or code change.crates/deepnsm/src/morphology.rs (1)
78-103: ⚡ Quick winDocument that several flags require manual setting or future morphology pass.
The
from_sentence_structureheuristic always defaults toTHIRD_PERSON + SINGULAR(lines 96, 99), meaningFIRST_PERSON,SECOND_PERSON, andPLURALare never set by this function. While this is acceptable for a v1 baseline, consider adding a doc comment noting which flags are not derived automatically, to guide future maintainers who may extend the morphology system.📝 Suggested documentation addition
/// Derive heuristic morph flags from a parsed sentence and triple index. /// /// V1 heuristics (deterministic, no learned parameters): /// - negation list → NEGATED /// - temporal marker → PAST (most temporal clauses are past events) /// - modal present (no temporal) → FUTURE /// - neither → PRESENT + THIRD_PERSON + SINGULAR (English default) + /// + /// **Note:** The following flags are never set by this function and require + /// explicit manipulation or a dedicated morphology pass: + /// `FIRST_PERSON`, `SECOND_PERSON`, `PLURAL`, `PASSIVE`, `INTERROGATIVE`, + /// `RELATIVE_CLAUSE`, `INFINITIVE`, `SUBORDINATE`. pub fn from_sentence_structure(s: &SentenceStructure, triple_idx: usize) -> Self {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/deepnsm/src/morphology.rs` around lines 78 - 103, Add a short doc comment to from_sentence_structure explaining that this V1 heuristic always sets THIRD_PERSON and SINGULAR (and never sets FIRST_PERSON, SECOND_PERSON, or PLURAL) and that person/number flags must be set manually or by a future morphology pass; reference the SentenceStructure input, the flags variable, and the specific constants THIRD_PERSON, SINGULAR, FIRST_PERSON, SECOND_PERSON, and PLURAL so future maintainers know which flags are not derived here and where to extend behavior.crates/deepnsm/src/cam64.rs (2)
129-130: 💤 Low valueCausal lane encoding is incomplete relative to lane layout documentation.
The lane layout comment (line 24) describes
byte 6as "causal / temporal / conditional markers" (plural), but the implementation only encodes a single temporal marker in bit 0. Bits 1-7 remain unused. If this is a v1 limitation (like the basin lane noted on line 132), consider adding a similar comment for clarity.📝 Suggested clarification
- // Lane 6: causal/temporal — bit 0 = temporal marker present. + // Lane 6: causal/temporal — bit 0 = temporal marker present (v1; causal/conditional reserved for v2). let causal_lane = if has_temporal { 0x01u8 } else { 0x00 };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/deepnsm/src/cam64.rs` around lines 129 - 130, The causal lane currently sets only bit 0 via the causal_lane variable (let causal_lane = if has_temporal { 0x01u8 } else { 0x00 };) but the lane layout declares byte 6 contains multiple markers (causal/temporal/conditional); either implement encoding for the other marker bits (e.g., add booleans like has_conditional/has_causal and set bits 0..7 accordingly) or add a clarifying comment next to causal_lane (matching the style used for basin_lane) stating this is a v1 limitation and which bits are intentionally unused; update the causal_lane construction and its comment to make the intent explicit.
120-123: ⚡ Quick winAdd compile-time check for MorphFlags bit range assumption.
Lines 120-123 split
MorphFlagsacross two bytes, assuming flags occupy bits 0-13 (with bits 14-15 spare as documented inmorphology.rs:34). If a future maintainer adds a flag at bit 14 or 15 without updating this code, the flag will be silently ignored during encoding.Consider adding a debug assertion or compile-time check to catch violations:
🛡️ Suggested safeguard
// Lanes 3-4: split MorphFlags across two bytes. let morph_bits = morph.bits(); + debug_assert!(morph_bits <= 0x3FFF, "MorphFlags uses bits beyond 0-13; cam64 encoding must be updated"); let morph_lane = (morph_bits & 0xFF) as u8; let clause_lane = ((morph_bits >> 8) & 0xFF) as u8;Alternatively, define a constant
MORPH_FLAGS_MAX_BIT = 13inmorphology.rsand reference it here, or use a static assertion if adopting a const-checking crate.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/deepnsm/src/cam64.rs` around lines 120 - 123, The code currently splits MorphFlags via morph.bits() into morph_lane and clause_lane assuming flags only use bits 0–13; add a safeguard to fail fast if that assumption is violated by either (a) adding a debug_assert or compile-time const check that (morph.bits() >> 14) == 0 before computing morph_lane and clause_lane (or reference a shared constant like MORPH_FLAGS_MAX_BIT from morphology.rs), or (b) add a static/const assert (const_assert!) tied to the MorphFlags max bit to ensure no flags are defined at bit 14 or 15; place this check immediately before the existing morph.bits() usage to protect the computations in morph_lane and clause_lane.crates/deepnsm/src/sentence_transformer64.rs (2)
416-447: ⚡ Quick winAdd
is_empty()or suppressclippy::len_without_is_empty.
SmallNeighbourhoodhas alen()method but nois_empty(). Clippy will warn about this pattern. Either add:pub fn is_empty(&self) -> bool { self.len == 0 }Or if
is_empty()is never needed (centre is always included, solen >= 1), suppress the lint on the struct:#[allow(clippy::len_without_is_empty)] pub struct SmallNeighbourhood { ... }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/deepnsm/src/sentence_transformer64.rs` around lines 416 - 447, Add an is_empty() method to SmallNeighbourhood to satisfy clippy by implementing pub fn is_empty(&self) -> bool { self.len == 0 } alongside the existing len() and is_singleton() methods (update impl SmallNeighbourhood). If you intentionally never expect len==0, alternatively annotate the struct with #[allow(clippy::len_without_is_empty)] on SmallNeighbourhood instead; choose one of these two options to resolve the lint.
383-408: 💤 Low valueSilent assumption that
tile.cells[0]represents the unperturbed centre.The loop skips
i == 0(line 396) to avoid re-emitting the centre, which is already pushed at line 392. This assumestile.cells[0]always represents zero perturbation. While this holds forSentenceTransformer64::local_tile()andIDENTITY, a custom tile with a non-zerocells[0]would have that perturbation silently skipped.Consider either documenting this contract on
Perturbation4x4or removing thei == 0skip and relying solely on theperturbed == centrecheck (line 398).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/deepnsm/src/sentence_transformer64.rs` around lines 383 - 408, The code in splat_p64 silently assumes Perturbation4x4.cells[0] is the no-op center and skips i == 0; remove that brittle special-case and instead allow i==0 to be processed normally by deleting the `if i == 0 { continue; }` check in splat_p64 (or alternatively add an explicit debug/assertion on the Perturbation4x4 invariant if you prefer to keep the skip). Ensure the function uses the existing perturbed == centre check (tile.apply and the subsequent hamming_p64 and cam.near_match logic) to filter out no-op cells so custom tiles with a non-zero cells[0] are not silently ignored.crates/deepnsm/src/signed_crystal.rs (1)
378-385: 💤 Low valueDocumentation mentions overflow behavior that isn't implemented here.
The docstring states "or overflow if repeated basin-breaks exceed the signed range" but the function only returns
0or1per call. The accumulation and overflow tracking must happen at the call site, which isn't obvious from this doc. Consider clarifying that this is a per-transition delta, and the caller is responsible for accumulating and detecting overflow./// Derive the basin hop delta from two consecutive `Cam64` locality codes. /// -/// Uses the `continues_basin` predicate: if the transition continues the -/// prior basin, delta = 0; otherwise delta = +1 (or overflow if repeated -/// basin-breaks exceed the signed range). +/// Uses the `continues_basin` predicate: if the transition continues the +/// prior basin, delta = 0; otherwise delta = +1. The caller is responsible +/// for accumulating deltas across multiple transitions and detecting overflow +/// when the cumulative delta exceeds the SignedOffset4 range (−7..+7). pub fn basin_delta_from_cam(prev: Cam64, curr: Cam64) -> i8 {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/deepnsm/src/signed_crystal.rs` around lines 378 - 385, The docstring for basin_delta_from_cam is misleading about overflow; update the comment to state explicitly that basin_delta_from_cam returns a per-transition signed delta (0 if curr.continues_basin(prev), 1 otherwise) and does not perform any accumulation or overflow detection — the caller is responsible for summing these deltas across transitions and detecting signed-range overflow. Reference the function basin_delta_from_cam and the Cam64.continues_basin predicate in the docstring so readers know this is a single-step delta and clarify that repeated basin-breaks must be handled by the caller.docs/architecture/deepnsm-reader-design.md (1)
29-29: 💤 Low valueAdd language specifiers to fenced code blocks for better rendering.
Three fenced code blocks are missing language specifiers. Add
```textto improve syntax highlighting and accessibility.📝 Suggested fix
At line 29:
-``` +```text SentenceStructure (from parser)At line 223:
-``` +```text HOT PATH — zero floats:At line 257:
-``` +```text DeepNSM (this PR):Also applies to: 223-223, 257-257
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/architecture/deepnsm-reader-design.md` at line 29, Three fenced code blocks in deepnsm-reader-design.md need explicit language specifiers; locate the three code fences containing the exact lines "SentenceStructure (from parser)", "HOT PATH — zero floats:", and "DeepNSM (this PR):" and change each opening triple-backtick to include the text language (i.e., replace ``` with ```text) so the blocks render with proper highlighting and accessibility.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/deepnsm/src/sentence_transformer64.rs`:
- Around line 109-214: This file defines P64 which duplicates P64MeaningField in
signed_crystal.rs; remove the duplicate type/impl here and instead re-export or
alias the canonical type (e.g. pub use crate::signed_crystal::P64MeaningField as
P64) so there is a single implementation to maintain; ensure Cam64 is in scope
in this module and that the canonical type provides the methods referenced
(from_cam64_and_nsm, lane, with_lane, bind, popcount, agreement, same_basin, raw
and the From<Cam64> impl) so callers continue to compile.
---
Nitpick comments:
In `@crates/deepnsm/src/cam64.rs`:
- Around line 129-130: The causal lane currently sets only bit 0 via the
causal_lane variable (let causal_lane = if has_temporal { 0x01u8 } else { 0x00
};) but the lane layout declares byte 6 contains multiple markers
(causal/temporal/conditional); either implement encoding for the other marker
bits (e.g., add booleans like has_conditional/has_causal and set bits 0..7
accordingly) or add a clarifying comment next to causal_lane (matching the style
used for basin_lane) stating this is a v1 limitation and which bits are
intentionally unused; update the causal_lane construction and its comment to
make the intent explicit.
- Around line 120-123: The code currently splits MorphFlags via morph.bits()
into morph_lane and clause_lane assuming flags only use bits 0–13; add a
safeguard to fail fast if that assumption is violated by either (a) adding a
debug_assert or compile-time const check that (morph.bits() >> 14) == 0 before
computing morph_lane and clause_lane (or reference a shared constant like
MORPH_FLAGS_MAX_BIT from morphology.rs), or (b) add a static/const assert
(const_assert!) tied to the MorphFlags max bit to ensure no flags are defined at
bit 14 or 15; place this check immediately before the existing morph.bits()
usage to protect the computations in morph_lane and clause_lane.
In `@crates/deepnsm/src/episodic_spo.rs`:
- Around line 172-195: The BasinClassification::Branch variant is never returned
by basin_classification(), so either add a clear branch in the decision logic
that returns BasinClassification::Branch under the appropriate condition (e.g.,
a specific novelty/entropy/confidence/wisdom threshold or a new property) or add
a comment in the basin_classification() function noting that Branch is
intentionally unused/reserved for v2; update basin_classification() (and mention
BasinClassification::Branch) so the enum definition and the function logic are
consistent.
- Around line 29-31: The NO_ROLE sentinel in episodic_spo.rs duplicates
crate::spo::NO_ROLE (0xFFF); remove the duplicate pub const NO_ROLE from
episodic_spo.rs and instead re-export or alias the canonical value from
crate::spo (e.g., use or pub use crate::spo::NO_ROLE) so all code refers to a
single source of truth and prevents drift if the sentinel changes.
In `@crates/deepnsm/src/morphology.rs`:
- Around line 78-103: Add a short doc comment to from_sentence_structure
explaining that this V1 heuristic always sets THIRD_PERSON and SINGULAR (and
never sets FIRST_PERSON, SECOND_PERSON, or PLURAL) and that person/number flags
must be set manually or by a future morphology pass; reference the
SentenceStructure input, the flags variable, and the specific constants
THIRD_PERSON, SINGULAR, FIRST_PERSON, SECOND_PERSON, and PLURAL so future
maintainers know which flags are not derived here and where to extend behavior.
In `@crates/deepnsm/src/reader_state.rs`:
- Around line 66-82: The Domain(u8) variant of LeftCornerTrigger is being masked
to 7 bits in basin_byte() which silently drops bit 7; update the docstring for
LeftCornerTrigger::basin_byte (or the LeftCornerTrigger enum) to explicitly
state that Domain(tag) intentionally reserves the high bit as the domain marker
and therefore the tag is masked with 0x7F (so callers must supply tags in
0..=127), or if you intend to preserve full 8-bit tags change the implementation
to use 0x80 | tag without masking—reference LeftCornerTrigger, its Domain(u8)
variant, and the basin_byte() method when making the clarification or code
change.
In `@crates/deepnsm/src/sentence_transformer64.rs`:
- Around line 416-447: Add an is_empty() method to SmallNeighbourhood to satisfy
clippy by implementing pub fn is_empty(&self) -> bool { self.len == 0 }
alongside the existing len() and is_singleton() methods (update impl
SmallNeighbourhood). If you intentionally never expect len==0, alternatively
annotate the struct with #[allow(clippy::len_without_is_empty)] on
SmallNeighbourhood instead; choose one of these two options to resolve the lint.
- Around line 383-408: The code in splat_p64 silently assumes
Perturbation4x4.cells[0] is the no-op center and skips i == 0; remove that
brittle special-case and instead allow i==0 to be processed normally by deleting
the `if i == 0 { continue; }` check in splat_p64 (or alternatively add an
explicit debug/assertion on the Perturbation4x4 invariant if you prefer to keep
the skip). Ensure the function uses the existing perturbed == centre check
(tile.apply and the subsequent hamming_p64 and cam.near_match logic) to filter
out no-op cells so custom tiles with a non-zero cells[0] are not silently
ignored.
In `@crates/deepnsm/src/signed_crystal.rs`:
- Around line 378-385: The docstring for basin_delta_from_cam is misleading
about overflow; update the comment to state explicitly that basin_delta_from_cam
returns a per-transition signed delta (0 if curr.continues_basin(prev), 1
otherwise) and does not perform any accumulation or overflow detection — the
caller is responsible for summing these deltas across transitions and detecting
signed-range overflow. Reference the function basin_delta_from_cam and the
Cam64.continues_basin predicate in the docstring so readers know this is a
single-step delta and clarify that repeated basin-breaks must be handled by the
caller.
In `@crates/deepnsm/src/window.rs`:
- Around line 157-160: The docstring for iter_expected() is incorrect: it says
"most-recently-pushed first" but the function returns the slice in push order
(oldest-first). Update the docstring on the iter_expected method to accurately
state that it returns the active expectation slots in push order (oldest-first)
or clarify that callers should reverse the slice (e.g., resolve_pronoun
currently calls .iter().rev()). Mention the function name iter_expected (and
optionally resolve_pronoun) so reviewers can locate the change.
In `@docs/architecture/deepnsm-reader-design.md`:
- Line 29: Three fenced code blocks in deepnsm-reader-design.md need explicit
language specifiers; locate the three code fences containing the exact lines
"SentenceStructure (from parser)", "HOT PATH — zero floats:", and "DeepNSM (this
PR):" and change each opening triple-backtick to include the text language
(i.e., replace ``` with ```text) so the blocks render with proper highlighting
and accessibility.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b593e482-7afc-423a-b0ed-988420b74eab
📒 Files selected for processing (9)
crates/deepnsm/src/cam64.rscrates/deepnsm/src/episodic_spo.rscrates/deepnsm/src/lib.rscrates/deepnsm/src/morphology.rscrates/deepnsm/src/reader_state.rscrates/deepnsm/src/sentence_transformer64.rscrates/deepnsm/src/signed_crystal.rscrates/deepnsm/src/window.rsdocs/architecture/deepnsm-reader-design.md
Two additions closing the left-corner/Pika seam identified in design review:
## signed_crystal.rs — HorizonPolarity + SignedOffset4 doc note
HorizonPolarity (v2 stub, 2-bit enum):
ConfirmedBackward = 0 ordinary prior context (sentence ring)
ExpectedForward = 1 left-corner prediction (push_expected / ExpectedReason)
InferredRight = 2 Pika-style right-context / inverse pass (v2, not yet wired)
BasinOverflow = 3 outside local window; use basin/archetype lookup
Encodes epistemic provenance — WHY/HOW KNOWN — separately from SignedOffset4
which encodes only WHERE (signed local distance). The two are orthogonal:
+1 could mean: next sentence physically (ConfirmedBackward)
expected referent not yet confirmed (ExpectedForward)
right-context memo already known (InferredRight)
Same number. Different truth-status.
In v1, ExpectedReason + push_expected() carry this information in
SentenceWindow. HorizonPolarity generalises the pattern for v2.
SignedOffset4 doc note: "intentionally does not encode epistemic polarity.
Left-corner expectation, confirmed backward context, and inverse/right-context
prepopulation are represented by the window/crystal caller through ExpectedReason
or future HorizonPolarity metadata."
## crystal_neighborhood.rs — L1 local geometry, 16 tests
NeighborhoodMetric enum:
Manhattan |dx|+|dy|+|dz| ≤ r (r=1 → 7 cells)
Chebyshev max(|dx|,|dy|,|dz|) ≤ r (r=1 → up to 27 cells)
LaneCompatible Chebyshev with |dx| ≤ 1 (sentence axis clips to ±1)
Crystal4096Neighbourhood: fixed [Crystal4096; 27], stack-allocated, no heap.
Center is always first. Overflow cells (nibble 15) always excluded.
neighbors_4096(center, radius, metric) → Crystal4096Neighbourhood
chebyshev_distance(a, b) → Option<u8> (None if overflow axis)
manhattan_distance(a, b) → Option<u8>
all_valid_cells() → impl Iterator<Item = Crystal4096> (15^3 = 3375 cells)
Three-tier model documented in module header:
L0 ABI: signed_crystal (Crystal4096, SignedOffset4)
L1 local geometry: crystal_neighborhood (this file)
L2 graph/DP: blasgraph (v2 frontier propagation over Crystal4096 arcs)
No floats. Integer nibble arithmetic throughout.
blasgraph is the designated L2 consumer; it will use u16 transition costs.
215 deepnsm tests pass (0 failures).
https://claude.ai/code/session_0147hSzjmWZDuy2MSQNrhEK5
Per design review: keep PR #479 a clean reading-substrate review unit. Both extensions are documented as deferred, not built here. window.rs: v1.5 Tekamolo/Anaphora64 provenance sidecar — full bit layout sketched in module docs. NOT folded into Cam64 (locality key) or P64 (address space). Hooks already present: HorizonPolarity (signed_crystal) + ExpectedReason (window). Boundary law: SentenceWindow resolves, EpisodicSpoFrame witnesses, Cam64 indexes, Anaphora64 (later) explains. design doc: - test count corrected to 215 (added crystal_neighborhood: 16) - new "Deliberately out of scope" section: * v1.5 Anaphora64 coreference provenance → coreference-ranking PR * v2 OGAR/SurrealDB AST adapter → separate crate, three-layer split (semantics/syntax/pragmatics), adapter law: semantics can exist without syntax; syntax must resolve to semantics before execution; pragmatics decides whether resolved syntax may run. SurrealDB is a runtime view, OGAR owns class truth. No code change; 215 deepnsm tests pass. https://claude.ai/code/session_0147hSzjmWZDuy2MSQNrhEK5
…cycle, P64 dedup Three findings from codex + CodeRabbit on PR #479: 1. (codex P2, reader_state.rs) Build Cam64 from the resolved subject. For "John … He …", effective_subject was the antecedent but Cam64 was built from the raw pronoun triple, so cam64/P64/CAM4096 keyed to the pronoun bucket while truth fields pointed at John — downstream locality lookup and basin matching used the wrong entity. Now constructs an effective triple (subject = resolved antecedent) before from_triple(). No-op when there is no coreference. The code now matches the comment that already promised "use the effective triple (post-coref)". Test: cam64_keyed_to_resolved_antecedent_not_pronoun. 2. (codex P2, window.rs) Clear consumed expectations. Forward-expectation slots were never drained, so across several relative/anaphora clauses they accumulated to MAX_EXPECTED (4), after which push_expected silently dropped newer antecedents and stale expectations could win over the confirmed ring. step() now clears the expectation buffer at the start of each sentence (expectations are single-step predictions), bounding it permanently. Test: expectations_do_not_accumulate_across_sentences. 3. (CodeRabbit, sentence_transformer64.rs) P64 duplicated P64MeaningField. Both were byte-identical 8-lane u64 meaning fields. Consolidated: P64 (the richer superset) is now canonical; signed_crystal re-exports `pub use crate::sentence_transformer64::P64 as P64MeaningField`. Removes drift risk. Behaviour-preserving (both from_cam64_and_nsm were identical). Also removed three now-unused imports surfaced by the consolidation (NO_ROLE in signed_crystal; MorphFlags + SpoTriple in sentence_transformer64). 217 deepnsm tests pass (was 215; +2 new regression tests). https://claude.ai/code/session_0147hSzjmWZDuy2MSQNrhEK5
Future marker only (no behaviour change): when bidirectional / Pika right-context passes land, clear_expected's policy splits — clear one-step expectations as now, but retain multi-step HorizonPolarity::InferredRight slots with a TTL so right-context memo entries survive across sentences. https://claude.ai/code/session_0147hSzjmWZDuy2MSQNrhEK5
Doc/lint cleanup only — no behavioural change. Closes out the remaining CodeRabbit comments on the deepnsm reader PR: - window.rs: iter_expected docstring said "most-recently-pushed first" but the returned slice is FIFO (callers reverse it). - episodic_spo.rs: dedup NO_ROLE -> `pub use crate::spo::NO_ROLE` (single canonical 0xFFF source); document that BasinClassification::Branch is reserved for the cross-frame basin tracker; correct the basin_classification doc to the actual confidence/wisdom logic (it listed a non-existent `Extension` variant). - cam64.rs: debug_assert MorphFlags stays within bits 0-13 before the two-byte lane split; mark the causal lane v1 (temporal bit only). - reader_state.rs: document that Domain(u8) basin_byte masks to 7 bits (tags must differ in bits 0-6). - morphology.rs: correct from_sentence_structure doc (no FUTURE branch exists) and list the flags it never derives. - signed_crystal.rs: clarify basin_delta_from_cam returns a per-transition delta; the caller accumulates and handles overflow. - sentence_transformer64.rs: add SmallNeighbourhood::is_empty() (len_without_is_empty). - docs: add `text` language to bare code fences (MD040). deepnsm is workspace-excluded, so its rustfmt/clippy are not in CI; left the crate's existing manual alignment intact (no fmt sweep) to keep the diff surgical. 217 lib tests pass. The splat_p64 i==0 suggestion was intentionally skipped (low-value, behaviour-adjacent). https://claude.ai/code/session_014A4JuRCqKP2yNENrQ9Ha7H
deepnsm is a workspace-excluded crate, so `cargo fmt --all` and the CI fmt job never reach it; its sources had drifted from rustfmt's default rules (manual column alignment across 28 files). Apply `cargo fmt` to bring the whole crate to a rustfmt-clean baseline. Pure formatting, no behavioural change. 217 lib tests still pass. `cargo fmt -- --check` is now clean (lib.rs needed a second pass for a trailing-comment reflow after reorder_modules; default rustfmt rules, no rustfmt.toml in the tree). Follow-up worth considering: wire deepnsm into the CI fmt check so it can't silently drift again. https://claude.ai/code/session_014A4JuRCqKP2yNENrQ9Ha7H
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/deepnsm/src/cam64.rs (1)
114-118:⚠️ Potential issue | 🟡 MinorObject lane 0 encodes both
NO_ROLEand real object buckets (ranks 0–31)In
crates/deepnsm/src/cam64.rs,from_triplesetsobj_lane = 0whentriple.object() == NO_ROLE(whereNO_ROLE = 0xFFF), but it also yieldsobj_lane = (object >> 5) == 0for legitimate object ranks0..=31. That makes “absent” vs “object in bucket 0” indistinguishable in lane 2 (and basin continuation compares the fullCam64bits, without special-casing lane 2). Either clarify theobject_state/P64 docs (“0 if absent”) to reflect this collision, or reserve lane-2 bucket 0 exclusively forNO_ROLE(e.g., encode non-NO_ROLEas(object >> 5) + 1) and add a unit test coveringNO_ROLEvs object ranks0..=31.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/deepnsm/src/cam64.rs` around lines 114 - 118, The object lane currently conflates NO_ROLE (0xFFF) with real object buckets 0..31; update from_triple to reserve lane-2 bucket 0 for NO_ROLE by setting obj_lane = 0 when triple.object() == NO_ROLE and otherwise obj_lane = ((triple.object() >> 5) as u8) + 1, then update any decoder/consumer of Cam64 or object_state/P64 to subtract 1 when decoding non-zero lane values; add a unit test that constructs Cam64 via from_triple for NO_ROLE and for object ranks 0..31 and asserts the bitwise Cam64 values differ (and decode back correctly) to prevent regressions.
🧹 Nitpick comments (1)
crates/deepnsm/src/signed_crystal.rs (1)
127-131: 💤 Low valueClarify
is_prediction()documentation regardingInferredRight.The doc comment states "forward prediction that may not materialise," but
InferredRightrepresents information that HAS already materialised from a Pika inverse pass (lines 98-100: "memo available from later clause material"). While bothExpectedForwardandInferredRightare non-backward-context information, onlyExpectedForwardis truly a prediction that may not occur.Consider either:
- Updating the doc to "True if this position is not from confirmed backward context" (more accurate), or
- Excluding
InferredRightif it represents confirmed (though non-chronological) evidence.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/deepnsm/src/signed_crystal.rs` around lines 127 - 131, The docstring for is_prediction() is misleading because InferredRight is confirmed via a later inverse pass; update the comment on the is_prediction method to accurately describe what the function tests (e.g., "True if this position is not from confirmed backward context" or "True for non-backward-context positions") and keep the current match logic (matches!(self, Self::ExpectedForward | Self::InferredRight)) unless you intentionally decide to exclude InferredRight; if you choose to exclude it, change the match to only Self::ExpectedForward and update any callers/tests accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@crates/deepnsm/src/cam64.rs`:
- Around line 114-118: The object lane currently conflates NO_ROLE (0xFFF) with
real object buckets 0..31; update from_triple to reserve lane-2 bucket 0 for
NO_ROLE by setting obj_lane = 0 when triple.object() == NO_ROLE and otherwise
obj_lane = ((triple.object() >> 5) as u8) + 1, then update any decoder/consumer
of Cam64 or object_state/P64 to subtract 1 when decoding non-zero lane values;
add a unit test that constructs Cam64 via from_triple for NO_ROLE and for object
ranks 0..31 and asserts the bitwise Cam64 values differ (and decode back
correctly) to prevent regressions.
---
Nitpick comments:
In `@crates/deepnsm/src/signed_crystal.rs`:
- Around line 127-131: The docstring for is_prediction() is misleading because
InferredRight is confirmed via a later inverse pass; update the comment on the
is_prediction method to accurately describe what the function tests (e.g., "True
if this position is not from confirmed backward context" or "True for
non-backward-context positions") and keep the current match logic
(matches!(self, Self::ExpectedForward | Self::InferredRight)) unless you
intentionally decide to exclude InferredRight; if you choose to exclude it,
change the match to only Self::ExpectedForward and update any callers/tests
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7e450715-67cc-43a7-9101-c7627fa48256
📒 Files selected for processing (10)
crates/deepnsm/src/cam64.rscrates/deepnsm/src/crystal_neighborhood.rscrates/deepnsm/src/episodic_spo.rscrates/deepnsm/src/lib.rscrates/deepnsm/src/morphology.rscrates/deepnsm/src/reader_state.rscrates/deepnsm/src/sentence_transformer64.rscrates/deepnsm/src/signed_crystal.rscrates/deepnsm/src/window.rsdocs/architecture/deepnsm-reader-design.md
✅ Files skipped from review due to trivial changes (1)
- docs/architecture/deepnsm-reader-design.md
🚧 Files skipped from review as they are similar to previous changes (5)
- crates/deepnsm/src/lib.rs
- crates/deepnsm/src/morphology.rs
- crates/deepnsm/src/window.rs
- crates/deepnsm/src/reader_state.rs
- crates/deepnsm/src/episodic_spo.rs
deepnsm is a standalone, workspace-excluded codec crate, so the existing CI jobs (which target crates/lance-graph and the contract crate) never reached it: `cargo fmt --all`, the workspace clippy, and the test job all skip it. That is how its formatting silently drifted (28 files, fixed in dd3d9ce) and ~12 clippy lints (TD-DEEPNSM-CLIPPY-195) accrued unseen. Wire it in following the repo's established tiered convention: - style.yml format job: add an explicit `cargo fmt -p deepnsm --check` (gating). The crate was brought rustfmt-clean earlier in this PR. - style.yml clippy job: add deepnsm as a Tier-B advisory step (continue-on-error), mirroring lance-graph core. Promote to gating once TD-DEEPNSM-CLIPPY-195 is cleared by a focused lint sweep. - rust-test.yml test job: run the deepnsm test suite (gating); 0-dep and fast, ~217 lib + integration + doctests. crates/** already triggers these workflows, so deepnsm-only changes now exercise fmt + clippy + test going forward. https://claude.ai/code/session_014A4JuRCqKP2yNENrQ9Ha7H
`cargo clippy --manifest-path crates/deepnsm/Cargo.toml --all-targets
-- -D warnings` is now clean (exit 0). Cleared 22 lints across 13 files:
the original 7-file set tracked as TD-DEEPNSM-CLIPPY-195 plus the lints
in this PR's new reader modules (window / reader_state /
crystal_neighborhood / sentence_transformer64 / signed_crystal /
codebook) that --all-targets surfaces.
All fixes are hand-applied and reviewed (NOT `clippy --fix`, which
mangled reader_state.rs into stranded-comment match guards):
- needless_range_loop: VsaVec::{bind,complement} -> std::array::from_fn;
bundle outer loop + similarity tables -> iter_mut().enumerate().
- len_without_is_empty: is_empty() on Codebook + Crystal4096Neighbourhood.
- collapsible_if: reader_state Relative/Anaphora arms -> match guards with
the explanatory comment moved above the arm (readable, unlike --fix).
- let_and_return, manual_contains, manual_(!)range_contains,
manual_is_multiple_of, unnecessary_cast (x4), redundant_field_names,
repeat().take()->repeat_n (x2), doc_lazy_continuation.
- Cargo.toml: move misplaced `description` from [workspace]
(unused-manifest-key warning) to [package].
Pure idiom cleanup, no behavioural change. 217 tests green; fmt clean.
https://claude.ai/code/session_014A4JuRCqKP2yNENrQ9Ha7H
…cleared) With deepnsm now clippy-clean (commit bf95caa), move its CI clippy step from Tier-B advisory (continue-on-error) to Tier-A mandatory gating — the same posture as the zero-dep contract crate. deepnsm changes can no longer regress clippy unseen. Records the resolution of TD-DEEPNSM-CLIPPY-195 in the tech-debt ledger. https://claude.ai/code/session_014A4JuRCqKP2yNENrQ9Ha7H
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/rust-test.yml:
- Around line 90-94: Add an explicit least-privilege permissions block for the
job/step that runs the "Run deepnsm tests" step: either add a top-level
permissions: block in the rust-test.yml workflow or add a permissions: block to
the specific job that contains the "Run deepnsm tests" step, and pin read-only
scopes (for example permissions: contents: read and packages: read) so the test
job does not run with default write permissions.
In @.github/workflows/style.yml:
- Around line 74-79: The workflow lacks an explicit permissions block so the
GITHUB_TOKEN defaults to write; add a top-level permissions declaration to
restrict the token to read-only (e.g., set contents: read and other relevant
read scopes) for this CI, and apply the same change to the other similar jobs
(including the job named "Clippy deepnsm (mandatory)" and the blocks referenced
around lines 103-109) so all lint/format gates use a read-only GITHUB_TOKEN.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 32493fc5-9cbf-4e41-b492-8b87d87e747d
📒 Files selected for processing (34)
.claude/board/TECH_DEBT.md.github/workflows/rust-test.yml.github/workflows/style.ymlcrates/deepnsm/Cargo.tomlcrates/deepnsm/examples/probe_semantic_sanity.rscrates/deepnsm/src/arcs.rscrates/deepnsm/src/arcuate.rscrates/deepnsm/src/cam64.rscrates/deepnsm/src/codebook.rscrates/deepnsm/src/comprehension.rscrates/deepnsm/src/context.rscrates/deepnsm/src/crystal_neighborhood.rscrates/deepnsm/src/disambiguator_glue.rscrates/deepnsm/src/encoder.rscrates/deepnsm/src/episodic_spo.rscrates/deepnsm/src/fingerprint16k.rscrates/deepnsm/src/lib.rscrates/deepnsm/src/markov_bundle.rscrates/deepnsm/src/morphology.rscrates/deepnsm/src/nsm_primes.rscrates/deepnsm/src/parser.rscrates/deepnsm/src/pipeline.rscrates/deepnsm/src/pos.rscrates/deepnsm/src/quantum_mode.rscrates/deepnsm/src/reader_state.rscrates/deepnsm/src/sentence_transformer64.rscrates/deepnsm/src/signed_crystal.rscrates/deepnsm/src/similarity.rscrates/deepnsm/src/ticket_emit.rscrates/deepnsm/src/trajectory_audit.rscrates/deepnsm/src/triangle_bridge.rscrates/deepnsm/src/vocabulary.rscrates/deepnsm/src/window.rscrates/deepnsm/tests/integration_role_alignment.rs
✅ Files skipped from review due to trivial changes (18)
- crates/deepnsm/Cargo.toml
- crates/deepnsm/src/arcs.rs
- crates/deepnsm/src/arcuate.rs
- crates/deepnsm/src/context.rs
- crates/deepnsm/src/vocabulary.rs
- crates/deepnsm/src/ticket_emit.rs
- crates/deepnsm/src/comprehension.rs
- crates/deepnsm/src/disambiguator_glue.rs
- crates/deepnsm/src/trajectory_audit.rs
- crates/deepnsm/src/pos.rs
- crates/deepnsm/src/fingerprint16k.rs
- crates/deepnsm/src/similarity.rs
- crates/deepnsm/examples/probe_semantic_sanity.rs
- crates/deepnsm/src/quantum_mode.rs
- crates/deepnsm/src/nsm_primes.rs
- .claude/board/TECH_DEBT.md
- crates/deepnsm/src/triangle_bridge.rs
- crates/deepnsm/src/parser.rs
🚧 Files skipped from review as they are similar to previous changes (9)
- crates/deepnsm/src/episodic_spo.rs
- crates/deepnsm/src/lib.rs
- crates/deepnsm/src/morphology.rs
- crates/deepnsm/src/signed_crystal.rs
- crates/deepnsm/src/cam64.rs
- crates/deepnsm/src/crystal_neighborhood.rs
- crates/deepnsm/src/window.rs
- crates/deepnsm/src/reader_state.rs
- crates/deepnsm/src/sentence_transformer64.rs
Addresses CodeRabbit review on #479: style.yml and rust-test.yml had no explicit `permissions:` block, so they inherited the repo-default token scope. These jobs only checkout, build, lint, and test — declare `contents: read` so GITHUB_TOKEN is least-privilege. Codecov upload uses its own token secret and is non-fatal (fail_ci_if_error: false). Also re-triggers CI: the prior run's `test (stable)` failed on a transient rust-lld SIGBUS (signal 7) while linking lance-graph's datafusion test binaries — intermittent linker memory-pressure flake, unrelated to deepnsm (the same code linked fine in test-with-coverage; deepnsm fmt+clippy gates already passed). https://claude.ai/code/session_014A4JuRCqKP2yNENrQ9Ha7H
AdaWorldAPI
pushed a commit
that referenced
this pull request
Jun 9, 2026
Roadmap companion to DEBT_REVIEW_2026-06-09.md + the TD rows. Folds in the user's hard constraints (no autofix; no deletion of unused code/deps without per-item confirmation — sharpened by the P0 AdaWorldAPI-fork policy for lance/lancedb/ndarray-family deps; scoped Sonnet agents + review gates). lance-graph is the HEALTHY repo: workspace clippy GREEN, member unsafe hygiene sound, heavy unsafe quarantined in excluded holograph. So the only P0 is trivial (C3: protobuf-compiler in CI). Low-hanging fruit = ontology-12 sweep. Wave plan W0–W2 + a propose-and-confirm queue for the machete member-dep findings (prost flagged as a feature-gated false positive). TD-DEEPNSM-CLIPPY-195 noted resolved in #479. https://claude.ai/code/session_01KQGUNH5N5Lg4Pb9WduLnqQ
AdaWorldAPI
pushed a commit
that referenced
this pull request
Jun 9, 2026
Dated debt-review doc + 3 TECH_DEBT entries from a cross-repo unsafe/clippy/unused audit (rebased onto post-#479 main 4d26776): - TD-UNUSED-DEPS-MACHETE-2026-06: member crates declare unused deps (core: bgz17/bgz-tensor/lancedb verified 0 src refs). prost is a feature-gated false positive; triage rule documented. - TD-CLIPPY-ONTOLOGY-12: lance-graph-ontology carries 12 lib clippy warnings (the one member cluster in an otherwise-GREEN workspace). - TD-ENV-PROTOC-MISSING: workspace clippy/build needs protobuf-compiler. Workspace clippy is GREEN (53 warnings, ~17 intentional v2 deprecations); member unsafe hygiene is sound (heavy unsafe quarantined in excluded holograph). TD-DEEPNSM-CLIPPY-195 noted resolved in #479. https://claude.ai/code/session_01KQGUNH5N5Lg4Pb9WduLnqQ
AdaWorldAPI
pushed a commit
that referenced
this pull request
Jun 9, 2026
Roadmap companion to DEBT_REVIEW_2026-06-09.md + the TD rows. Folds in the user's hard constraints (no autofix; no deletion of unused code/deps without per-item confirmation — sharpened by the P0 AdaWorldAPI-fork policy for lance/lancedb/ndarray-family deps; scoped Sonnet agents + review gates). lance-graph is the HEALTHY repo: workspace clippy GREEN, member unsafe hygiene sound, heavy unsafe quarantined in excluded holograph. So the only P0 is trivial (C3: protobuf-compiler in CI). Low-hanging fruit = ontology-12 sweep. Wave plan W0–W2 + a propose-and-confirm queue for the machete member-dep findings (prost flagged as a feature-gated false positive). TD-DEEPNSM-CLIPPY-195 noted resolved in #479. https://claude.ai/code/session_01KQGUNH5N5Lg4Pb9WduLnqQ
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds the sentence-level AriGraph reading state machine to
crates/deepnsm/.Seven new modules; 200 tests; zero floats in the hot path.
The one-line description:
Full design document:
docs/architecture/deepnsm-reader-design.mdWhat was added
morphology.rs— MorphFlagsMorphFlags(u16): 14-bit heuristic morphological feature bitfield derived fromSentenceStructure. No float arithmetic. Flags: PAST, PRESENT, FUTURE, SINGULAR, PLURAL, FIRST/SECOND/THIRD_PERSON, PASSIVE, NEGATED, INTERROGATIVE, RELATIVE_CLAUSE, INFINITIVE, SUBORDINATE.cam64.rs— Reading-state locality keyCam64(u64): 8 lanes × 8 bits. NOT semantic truth — fast reading-locality index for basin matching, prefetch, and coreference heuristics. Lane layout: entity / predicate / object / morph-low / morph-high-clause / discourse / causal / basin.New in the Pika adaptation commit:
continues_basin(prev) → bool— popcount-based chart-arc predicate (shared ≥ 16 bits AND diff ≤ 24 bits).basin_continuation_score() → u8graded variant.episodic_spo.rs— Auditable witness rowEpisodicSpoFrame: 25Copyfields, stackable inVecfor SoA sweep. Thecam64field is the fast-index;subject/predicate/object_candidate_idare the truth. Size ≤ 128 bytes (tested).BasinClassification: Reinforcement / NoveltyDelta / WisdomDelta / Contradiction / Branch / Epiphany.window.rs— ±5 sentence ring buffer + Pika expectation slotsSentenceWindow: 11-entry ring buffer ofWindowEntry(up to 4 NP heads per sentence). Two-phaseresolve_pronoun():push_expected(rank, ExpectedReason)pre-populates from a left-corner trigger. Expectation beats recency.ExpectedReason: RelativeClause / Anaphora / Ellipsis / CausalContinuation / TemporalContinuation.reader_state.rs— Left-corner state machineReadingState::step(self, &SentenceStructure, &SentenceFeatures) → (Vec<EpisodicSpoFrame>, ReadingState)— pure,selfconsumed, no&mut selfduring computation (data-flow.md rule).Left-corner trigger wiring (Pika chart-arc pre-population): when
RelativeorAnaphoratrigger fires, the prioractive_subjectis immediately pushed into the window's expectation buffer so the nextresolve_pronoun()finds the predicted antecedent first.LeftCornerTriggervariants: Declarative / Causal / Temporal / Relative / Anaphora / FirstPerson / Domain(u8).signed_crystal.rs— Discrete reading crystalBridge from DeepNSM grammar reader to holograph bitpacked resonance substrate.
SignedOffset4: 0-14 encodes −7..+7; 15 = overflow/basin sentinelCrystal4096: three axes × 4 bits = 12 bits = direct P4096 palette codebook key.xor()for VSA bind/unbind.same_basin()= no overflow AND nibble_distance ≤ 1SignedSentenceCrystal { p64, coord }: complete output.bind()XOR-binds both fields.basin_delta_from_cam(prev, curr) → i8sentence_transformer64.rs— Native P64 meaning fieldThe architectural correction this module encodes:
P64(u64): 8 orthogonal semantic planes. Each word projects vertically — activates across multiple lanes simultaneously.from_cam64_and_nsm()is the canonical construction (grammar → Cam64 → P64, zero floats).agreement()=64 - popcount(XOR).Cam4096(u16): 12-bit deterministic codebook address.from_p64()folds top nibbles of entity, predicate, and basin lanes — bit-selection, not float nearest-neighbour. 4096 cells = native-English reading-state classes at full resolution.Perturbation4x4: local 4×4 discrete ambiguity tile. Row = semantic axis, col = syntactic axis. 16 alternatives per step. The implicit (4×4)^n trajectory space is never materialised.splat_p64(centre, tile, radius_bits) → SmallNeighbourhood: discrete palette splat — NOT Gaussian in f32 space. Stack-allocated, ≤ 16 entries.SentenceTransformer64: projects(Cam64, nsm_prime_mask, subject, predicate, object, role)→Sentence64 { p64, cam, spo_hint }.Architectural invariants
Two-faculty split (E-ENGLISH-BIFURCATES) — unchanged
ContextWindow(context.rs)SentenceWindow(window.rs)These serve different cognitive faculties and must not be fused.
Float boundary policy
Relationship to holograph
Test plan
cargo test --manifest-path crates/deepnsm/Cargo.toml— 200 passing, 0 failingEpisodicSpoFrame≤ 128 bytes (tested)relative_trigger_pushes_expected_subject+pronoun_prefers_expected_relative_subjectcam64_continuation_true_for_nearby_codes+cam64_continuation_false_for_far_codesfrom_f32_embeddingpath exists in any new modulehttps://claude.ai/code/session_0147hSzjmWZDuy2MSQNrhEK5
Generated by Claude Code
Summary by CodeRabbit
New Features
Documentation
Chores / CI