W3+W4a: atomic BindSpace→MailboxSoA read/write shim (feature default-OFF)#520
Merged
Conversation
5+3 hardening on the W3+W4a atomic implementation plan (BindSpace→MailboxSoA, first behaviour-touching step). v1 brutal verdict was HOLD (3 P0s); v2 integrates every finding. Architecture confirmed sound by all 8 angles. Panel: integration-lead, truth-architect, dto-soa-savant, convergence-architect, firewall-warden (consolidation) + baton-handoff-auditor, brutally-honest-tester, preflight-drift-auditor (brutal). Corrections folded in: - C1 write surface: driver.run() writes nothing; writes are engine_bridge free fns (incl. the 4th, ingest_codebook_indices) called by bins/tests. Bin conversion is W4b; W3+W4a delivers the BackingStoreWrite shim exercised by the W2 differential, feature default-OFF (H-DW-1 satisfied, no production half-flip). - C2 prefilter window: mailbox arm honors win.start..win.end, not 0..populated (sentinel-lie on row_start>0); W2 differential must include a non-full-window case. - C3 OQ-D: single-mailbox Option A (DEFAULT_MAILBOX const + debug_assert len==1), defer routing to W5, reject the ShaderDispatch contract change. - C4 parity surface: assert whole ShaderCrystal field-by-field via to_bits() (add cycles_used/style_ord/hit_count/emitted_edges/gate/MetaSummary/...). - C5 unbind_busdto: feature-gate the lossy narrowing + pin non-headline indices=0; keep singleton bit-exact via cfg(not(feature)). OQ-B NOW-OK (with-engine lab path). - C6 firewall lint: concrete Grep/compile_fail rule allowing the repr(transparent) cast sites; source clean today. - C7 expert_at exists (cycle_count lossless); feature-gate asymmetry with-engine vs mailbox-thoughtspace reconciled; edges_raw/meta_raw exist (shim uses typed path). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CcpLeEC3XK8Eye53GKBVvi
📝 WalkthroughWalkthroughAdds a ChangesW3+W4a BindSpace→MailboxSoA Read/Write Shim
Sequence Diagram(s)sequenceDiagram
participant Test as W2 Differential Test
participant DriverS as ShaderDriver (Singleton)
participant DriverM as ShaderDriver (Mailbox)
participant BSS as BackingStore::Singleton
participant BSM as BackingStore::Mailbox
participant BindSpace as BindSpace
participant Mailbox as MailboxSoA<1024>
Test->>DriverS: dispatch(ShaderDispatch)
DriverS->>BSS: backing() → Singleton
BSS->>BindSpace: prefilter / content_row / qualia_17d / edge / entity_type
BindSpace-->>BSS: row data
BSS-->>DriverS: ShaderCrystal A
Test->>DriverM: dispatch(ShaderDispatch)
DriverM->>BSM: backing() → Mailbox (DEFAULT_MAILBOX)
BSM->>Mailbox: prefilter(clamped to populated) / content_row / qualia_17d / edge / entity_type
Mailbox-->>BSM: row data
BSM-->>DriverM: ShaderCrystal B
Test->>Test: assert_crystal_bit_identical(A, B)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
First behaviour-touching step of the BindSpace→MailboxSoA migration. The shim lets driver.run() keep ONE body while the substrate flips from the singleton BindSpace to a per-mailbox MailboxSoA under the new, default-OFF `mailbox-thoughtspace` feature. Production stays singleton-read+write until W7; nothing in the default build changes. - backing.rs (new, pub(crate)): BackingStore (read) + BackingStoreWrite (write, C1) enums. Singleton(&BindSpace) live default; Mailbox arm feature-gated. Mailbox prefilter honours win.start AND win.end (C2, P0, byte-identical to BindSpace::meta_prefilter). enum-over-trait (OQ-C). - driver.rs: const DEFAULT_MAILBOX (OQ-D Option A), fn backing() selector (debug_assert mailboxes.len() <= 1, singleton fallback), all 6 reads in run() re-pointed through one backing value; ontology() stays singleton (W4b re-home). - engine_bridge.rs::unbind_busdto: C5 downgrade — cycle-plane index recovery gated out under mailbox-thoughtspace (cycle plane never migrated, D-DIST-5); headline survives via qualia[9]; singleton build keeps bit-exact recovery. - Gates: W2 differential (whole-ShaderCrystal to_bits() parity, both arms, incl. non-zero-window + non-vacuity), firewall twin-bar lint, field-isolation matrix, cycle-drop footprint, backing read+write round-trip. Tests: default 97 lib + 2 firewall + 2 e2e; feature-on 98 lib + 2 firewall + 2 e2e + 4 w2. clippy --all-targets (both cfgs) + fmt clean on touched files. Pre-existing P0 surfaced, NOT fixed here (out of W3+W4a scope, flagged for operator): --features with-engine does not compile on HEAD (engine_bridge.rs:259 QUALIA_DIMS unimported); the D-CSV-5b i4-qualia cutover separately breaks the busdto codebook_index round-trip. Board hygiene in this commit: STATUS_BOARD W3+W4a row, LATEST_STATE bullet, AGENT_LOG entry. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CcpLeEC3XK8Eye53GKBVvi
AdaWorldAPI
changed the title
![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: b5c17d52dd
ℹ️ 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".
| // SINGLETON BUILD ONLY — the cycle plane is never migrated to the | ||
| // mailbox (C5 / D-DIST-5). Under `mailbox-thoughtspace` this block is | ||
| // gated out and the non-headline indices stay 0 (documented loss). | ||
| #[cfg(not(feature = "mailbox-thoughtspace"))] |
There was a problem hiding this comment.
When with-engine is built together with mailbox-thoughtspace, unbind_busdto still takes a &BindSpace, and dispatch_busdto still writes the positive top_k indices into bs.fingerprints.cycle_row(row). This cfg removes the only code that reads those present cycle bits for the entire feature build, including the documented singleton fallback path with no registered mailbox, so BusDto round-trips lose every non-headline positive top_k index as 0 even though the data is still in the BindSpace row.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 @.claude/board/STATUS_BOARD.md:
- Line 679: The CI workflow `rust-test.yml` is missing a cargo test invocation
with the `--features mailbox-thoughtspace` flag, which means the feature-gated
differential tests in `w2_differential.rs` are never validated in CI. Add a new
test step or matrix row to the workflow that runs `cargo test` with `--features
mailbox-thoughtspace` targeting the cognitive-shader-driver crate manifest. This
ensures the 4 feature-gated differential tests are executed during CI and
prevents regressions in the mailbox-thoughtspace code path.
In `@crates/cognitive-shader-driver/src/engine_bridge.rs`:
- Around line 338-346: The codebook_index is being extracted from q[9], a float
that has passed through lossy quantization which clamps all values to [-1.0,
1.0] before i4 encoding. This causes any codebook_index greater than 1 to be
clamped to 1.0 and incorrectly recovers to u16(1), corrupting the headline
identity. Instead of reading codebook_index from the quantized q[9] in the row
method call on bs.qualia, access a dedicated integer column or field from the bs
(bus DTO) structure that stores the codebook index as a raw integer without
quantization, removing the reliance on the qualia carrier for index storage.
In `@crates/cognitive-shader-driver/src/mailbox_soa.rs`:
- Around line 1197-1325: The field-isolation matrix test
`test_mailbox_soa_field_isolation_matrix` is missing test coverage for the
`topic` and `angle` fields. Add baseline values for `topic` and `angle`
alongside the other baseline constants at the start of the test. Include these
fields in the `seed` closure by calling the appropriate setter methods. Add
checks for `topic` and `angle` in the `assert_others_unchanged` closure to
verify they remain unchanged when other fields are mutated. Finally, add two new
test blocks (one for each field) following the pattern of existing blocks like
the edge and qualia mutations, where you create a fresh MailboxSoA instance,
seed it, mutate either `topic` or `angle` to a distinct new value, and assert
all other fields including the newly added fields remain unchanged.
In `@crates/cognitive-shader-driver/tests/firewall.rs`:
- Around line 80-83: The error handling for fs::read_to_string(file) in the
firewall test silently skips unreadable files with continue, which can hide
violations and allow the test to pass with incomplete coverage. Replace the
Err(_) => continue branch to instead panic or return an error with file context
information (including the filename), ensuring the test fails immediately when
any source file cannot be read.
- Around line 46-57: The normalize() function only handles space characters
around the `::` token, allowing imports to bypass detection using other
whitespace characters (newlines, tabs) or alternative Rust syntax like
`::{CausalEdge64}`. Enhance the normalize() function to handle all whitespace
characters (not just spaces) and collapse grouped import forms like `::{ `.
Additionally, add focused unit tests using #[cfg(test)] to validate that
normalize() correctly handles these various import patterns, including space
variations, newline/tab characters, and curly-brace grouping syntax.
In `@crates/cognitive-shader-driver/tests/w2_differential.rs`:
- Around line 265-283: The test verifies that driver_singleton and
driver_mailbox produce identical results, but does not explicitly confirm that
driver_mailbox is actually using the mailbox path and not silently falling back
to singleton behavior. Add an explicit assertion or guard after dispatching on
driver_mailbox (after the line with driver_mailbox.dispatch(req)) that proves
mailbox-backed reads were active and the mailbox was exercised, such as checking
mailbox state, access counts, or other metadata that would confirm the mailbox
path was taken rather than a singleton fallback.
🪄 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: 3803e349-9d1f-4657-b4a1-374cc1528bfb
📒 Files selected for processing (13)
.claude/board/AGENT_LOG.md.claude/board/LATEST_STATE.md.claude/board/STATUS_BOARD.md.claude/plans/bindspace-mailbox-soa-w3-w4a-impl-v1.mdcrates/cognitive-shader-driver/Cargo.tomlcrates/cognitive-shader-driver/src/backing.rscrates/cognitive-shader-driver/src/driver.rscrates/cognitive-shader-driver/src/engine_bridge.rscrates/cognitive-shader-driver/src/lib.rscrates/cognitive-shader-driver/src/mailbox_soa.rscrates/cognitive-shader-driver/tests/busdto_bridge_test.rscrates/cognitive-shader-driver/tests/firewall.rscrates/cognitive-shader-driver/tests/w2_differential.rs
| | D-id | Title | Crate(s) | ~LOC | Risk | Status | PR / Evidence | | ||
| |---|---|---|---|---|---|---| | ||
| | D-MBX-1 | add migrated columns (`edges`/`qualia`/`meta`/`entity_type`) to `MailboxSoA<N>` behind `mailbox-thoughtspace` feature | cognitive-shader-driver | 120 | MED | **Queued** | gated on D-CE64-MB-1-impl + PR-NDARRAY-MIRI-COMPLETE | | ||
| | W3+W4a | atomic read/write shim (`backing::{BackingStore,BackingStoreWrite}`) — `driver.run()` keeps ONE body, `mailbox-thoughtspace` (default-OFF) flips substrate singleton→`MailboxSoA`; 6 reads re-pointed; W2 differential proves bit-identity; firewall lint + field-isolation + footprint gates | cognitive-shader-driver | 600 | MED | **In PR** | branch `claude/bindspace-mailbox-soa-w3-w4a`; default 97+2+2 tests, feature 98+2+2+4; clippy/fmt clean; `unbind_busdto` C5 downgrade feature-gated (cycle plane never migrated). Plan `bindspace-mailbox-soa-w3-w4a-impl-v1.md` | |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify CI test-matrix expectations are documented
grep -A5 "mailbox-thoughtspace\|feature.*CI matrix" .github/workflows/*.yml || echo "Feature CI matrix NOT FOUND IN WORKFLOWS"Repository: AdaWorldAPI/lance-graph
Length of output: 108
🏁 Script executed:
# List workflow files
ls -la .github/workflows/ 2>/dev/null || echo "No .github/workflows directory"Repository: AdaWorldAPI/lance-graph
Length of output: 549
🏁 Script executed:
# Show workflow file names and their basic structure
fd -e yml -e yaml . .github/workflows 2>/dev/null | head -20Repository: AdaWorldAPI/lance-graph
Length of output: 251
🏁 Script executed:
# Search for any test-related configuration in workflows (broader search)
rg "cargo test|features" .github/workflows/ -i -A 2 || echo "No test configurations found"Repository: AdaWorldAPI/lance-graph
Length of output: 3057
🏁 Script executed:
# Check if feature flags are used anywhere in test configuration
rg "mailbox|thoughtspace" . --type md --type yml -i | head -20 || echo "No feature references found"Repository: AdaWorldAPI/lance-graph
Length of output: 95
🏁 Script executed:
# Read the main test workflow file
cat -n .github/workflows/rust-test.ymlRepository: AdaWorldAPI/lance-graph
Length of output: 9425
🏁 Script executed:
# Check if mailbox-thoughtspace feature is defined in Cargo.toml
rg "mailbox-thoughtspace" . --type toml 2>/dev/null | head -20 || echo "No mailbox-thoughtspace feature found"Repository: AdaWorldAPI/lance-graph
Length of output: 137
🏁 Script executed:
# Check for feature-gated tests in cognitive-shader-driver
rg "#\[cfg\(feature.*mailbox" . --type rust 2>/dev/null | head -20Repository: AdaWorldAPI/lance-graph
Length of output: 2040
🏁 Script executed:
# More broadly, search for feature-gated tests in the crate
rg "cfg.*feature.*mailbox\|mailbox.*feature" . --type rust | head -30Repository: AdaWorldAPI/lance-graph
Length of output: 49
🏁 Script executed:
# Check if W2 differential tests mentioned in the status board exist
rg "w2|W2|differential" crates/cognitive-shader-driver --type rust | head -20Repository: AdaWorldAPI/lance-graph
Length of output: 2353
Add feature-gated test matrix to CI workflow.
The mailbox-thoughtspace feature and its 4 W2 differential tests (in crates/cognitive-shader-driver/tests/w2_differential.rs) exist and are properly gated with #[cfg(feature = "mailbox-thoughtspace")] throughout the codebase, but rust-test.yml currently has no --features flags in any cargo test invocation. Add a dedicated CI step or matrix row:
cargo test --manifest-path crates/cognitive-graph/Cargo.toml --features mailbox-thoughtspace
This ensures the feature-gated differential tests (97+2+2+4 tests) are validated alongside default builds, preventing regressions in the new code path.
🤖 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 @.claude/board/STATUS_BOARD.md at line 679, The CI workflow `rust-test.yml`
is missing a cargo test invocation with the `--features mailbox-thoughtspace`
flag, which means the feature-gated differential tests in `w2_differential.rs`
are never validated in CI. Add a new test step or matrix row to the workflow
that runs `cargo test` with `--features mailbox-thoughtspace` targeting the
cognitive-shader-driver crate manifest. This ensures the 4 feature-gated
differential tests are executed during CI and prevents regressions in the
mailbox-thoughtspace code path.
Comment on lines
+338
to
346
| // [1] qualia → energy + top_k energies + headline codebook_index. | ||
| // D-CSV-5b: bs.qualia is now QualiaI4Column; convert to f32 at the read site. | ||
| // codex P2 fix (2026-05-07): the headline is stored explicitly in qualia[9] | ||
| // at encode, so it round-trips bit-exact regardless of cycle-plane state. | ||
| let q_i4 = bs.qualia.row(row); | ||
| let q = q_i4.to_f32_17d(); // [f32; 17] — sufficient for dims 0..9 | ||
| let energy = q[0]; | ||
| let codebook_index = q[9] as u16; | ||
| let mut top_k = [(0u16, 0.0f32); 8]; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify QualiaI4_16D encoding/decoding semantics and range behavior.
fd -i 'qualia*.rs' crates | xargs -r rg -n -A20 -B5 'struct\s+QualiaI4_16D|from_f32_17d|to_f32_17d'
# Verify writer/reader call sites in engine bridge.
rg -n -A12 -B12 'q\[9\]\s*=|codebook_index\s*=\s*q\[9\]\s+as u16|from_f32_17d' crates/cognitive-shader-driver/src/engine_bridge.rsRepository: AdaWorldAPI/lance-graph
Length of output: 14262
codebook_index recovery is using a lossy carrier contract.
dispatch_busdto writes the headline index into q[9] and packs via QualiaI4_16D::from_f32_17d, but the quantization path clamps all f32 inputs to [−1.0, 1.0] before encoding to i4. For any codebook_index > 1, the value is clamped to 1.0, quantized to i4 value 7, and on decode recovers as 1.0 → cast to u16(1), not the original index. This corrupts the headline identity on both singleton and mailbox paths.
The inline comment claiming "round-trips bit-exact" (lines 252–258) describes an intended design that is not implemented by the current quantization contract in from_f32_17d. Use a dedicated integer field/column for codebook_index, keeping qualia reserved for energy payload.
🤖 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/cognitive-shader-driver/src/engine_bridge.rs` around lines 338 - 346,
The codebook_index is being extracted from q[9], a float that has passed through
lossy quantization which clamps all values to [-1.0, 1.0] before i4 encoding.
This causes any codebook_index greater than 1 to be clamped to 1.0 and
incorrectly recovers to u16(1), corrupting the headline identity. Instead of
reading codebook_index from the quantized q[9] in the row method call on
bs.qualia, access a dedicated integer column or field from the bs (bus DTO)
structure that stores the codebook index as a raw integer without quantization,
removing the reliance on the qualia carrier for index storage.
Comment on lines
+1197
to
+1325
| /// **The layout-bit-boundary regression guard (W4a, I-LEGACY-API-FEATURE-GATED).** | ||
| /// Writing one migrated column on a row must leave EVERY other migrated column | ||
| /// on that row byte-unchanged. This is the field-isolation matrix the iron rule | ||
| /// mandates whenever a layout reclaims or co-locates per-row state. We seed a | ||
| /// row with a known baseline across all columns, then mutate exactly one column | ||
| /// at a time and assert the others are untouched. | ||
| #[test] | ||
| fn test_mailbox_soa_field_isolation_matrix() { | ||
| const N: usize = 4; | ||
| const R: usize = 2; | ||
|
|
||
| // Baseline values (distinct, non-zero where the column allows). | ||
| let base_edge = CausalEdge64(0x1111_2222_3333_4444); | ||
| let base_qualia = QualiaI4_16D::ZERO.with(0, 3).with(5, -4); | ||
| let base_meta = MetaWord::new(5, 2, 100, 120, 7); | ||
| let base_etype = 42u16; | ||
| let base_temporal = 0x9999_0000_0000_0001u64; | ||
| let base_expert = 77u16; | ||
| let base_sigma = 9u8; | ||
| let base_content = { | ||
| let mut w = [0u64; WORDS_PER_FP]; | ||
| w[0] = 0xCAFE; | ||
| w[WORDS_PER_FP - 1] = 0xBEEF; | ||
| w | ||
| }; | ||
|
|
||
| let seed = |mb: &mut MailboxSoA<N>| { | ||
| mb.set_edge(R, base_edge); | ||
| mb.set_qualia(R, base_qualia); | ||
| mb.set_meta(R, base_meta); | ||
| mb.set_entity_type(R, base_etype); | ||
| mb.set_temporal(R, base_temporal); | ||
| mb.set_expert(R, base_expert); | ||
| mb.set_sigma(R, base_sigma); | ||
| mb.set_content(R, &base_content); | ||
| }; | ||
|
|
||
| // Assert all columns EXCEPT `changed` match baseline. `changed` is a tag. | ||
| let assert_others_unchanged = |mb: &MailboxSoA<N>, changed: &str| { | ||
| if changed != "edge" { | ||
| assert_eq!(mb.edge(R).0, base_edge.0, "edge changed by {changed}"); | ||
| } | ||
| if changed != "qualia" { | ||
| assert_eq!(mb.qualia_at(R), base_qualia, "qualia changed by {changed}"); | ||
| } | ||
| if changed != "meta" { | ||
| assert_eq!(mb.meta_at(R).0, base_meta.0, "meta changed by {changed}"); | ||
| } | ||
| if changed != "entity_type" { | ||
| assert_eq!( | ||
| mb.entity_type_at(R), | ||
| base_etype, | ||
| "entity_type changed by {changed}" | ||
| ); | ||
| } | ||
| if changed != "temporal" { | ||
| assert_eq!( | ||
| mb.temporal_at(R), | ||
| base_temporal, | ||
| "temporal changed by {changed}" | ||
| ); | ||
| } | ||
| if changed != "expert" { | ||
| assert_eq!(mb.expert_at(R), base_expert, "expert changed by {changed}"); | ||
| } | ||
| if changed != "sigma" { | ||
| assert_eq!(mb.sigma_at(R), base_sigma, "sigma changed by {changed}"); | ||
| } | ||
| if changed != "content" { | ||
| assert_eq!( | ||
| mb.content_row(R), | ||
| &base_content[..], | ||
| "content changed by {changed}" | ||
| ); | ||
| } | ||
| }; | ||
|
|
||
| // Mutate each column to a DISTINCT new value, one at a time, from a fresh | ||
| // baseline each iteration, and assert isolation. | ||
| { | ||
| let mut mb: MailboxSoA<N> = MailboxSoA::new(1, 0, 1.0); | ||
| seed(&mut mb); | ||
| mb.set_edge(R, CausalEdge64(0xDEAD_BEEF_DEAD_BEEF)); | ||
| assert_others_unchanged(&mb, "edge"); | ||
| } | ||
| { | ||
| let mut mb: MailboxSoA<N> = MailboxSoA::new(1, 0, 1.0); | ||
| seed(&mut mb); | ||
| mb.set_qualia(R, QualiaI4_16D::ZERO.with(15, 7)); | ||
| assert_others_unchanged(&mb, "qualia"); | ||
| } | ||
| { | ||
| let mut mb: MailboxSoA<N> = MailboxSoA::new(1, 0, 1.0); | ||
| seed(&mut mb); | ||
| mb.set_meta(R, MetaWord::new(11, 3, 5, 6, 1)); | ||
| assert_others_unchanged(&mb, "meta"); | ||
| } | ||
| { | ||
| let mut mb: MailboxSoA<N> = MailboxSoA::new(1, 0, 1.0); | ||
| seed(&mut mb); | ||
| mb.set_entity_type(R, 999); | ||
| assert_others_unchanged(&mb, "entity_type"); | ||
| } | ||
| { | ||
| let mut mb: MailboxSoA<N> = MailboxSoA::new(1, 0, 1.0); | ||
| seed(&mut mb); | ||
| mb.set_temporal(R, 0x1234_5678_9ABC_DEF0); | ||
| assert_others_unchanged(&mb, "temporal"); | ||
| } | ||
| { | ||
| let mut mb: MailboxSoA<N> = MailboxSoA::new(1, 0, 1.0); | ||
| seed(&mut mb); | ||
| mb.set_expert(R, 12345); | ||
| assert_others_unchanged(&mb, "expert"); | ||
| } | ||
| { | ||
| let mut mb: MailboxSoA<N> = MailboxSoA::new(1, 0, 1.0); | ||
| seed(&mut mb); | ||
| mb.set_sigma(R, 200); | ||
| assert_others_unchanged(&mb, "sigma"); | ||
| } | ||
| { | ||
| let mut mb: MailboxSoA<N> = MailboxSoA::new(1, 0, 1.0); | ||
| seed(&mut mb); | ||
| let mut other = [0u64; WORDS_PER_FP]; | ||
| other[3] = 0xABCD; | ||
| mb.set_content(R, &other); | ||
| assert_others_unchanged(&mb, "content"); | ||
| } |
There was a problem hiding this comment.
Field-isolation matrix is incomplete (topic/angle are missing).
This test currently verifies isolation for content but not the other migrated dense planes. Add baseline + mutation cases for topic and angle, and assert all other fields stay byte-identical.
As per coding guidelines, “implement field-isolation matrix tests: write each field, assert all other fields unchanged” when layout is reclaimed/migrated.
🤖 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/cognitive-shader-driver/src/mailbox_soa.rs` around lines 1197 - 1325,
The field-isolation matrix test `test_mailbox_soa_field_isolation_matrix` is
missing test coverage for the `topic` and `angle` fields. Add baseline values
for `topic` and `angle` alongside the other baseline constants at the start of
the test. Include these fields in the `seed` closure by calling the appropriate
setter methods. Add checks for `topic` and `angle` in the
`assert_others_unchanged` closure to verify they remain unchanged when other
fields are mutated. Finally, add two new test blocks (one for each field)
following the pattern of existing blocks like the edge and qualia mutations,
where you create a fresh MailboxSoA instance, seed it, mutate either `topic` or
`angle` to a distinct new value, and assert all other fields including the newly
added fields remain unchanged.
Source: Coding guidelines
Comment on lines
+46
to
+57
| /// Normalize whitespace around `::` so `causal_diff :: CausalEdge64` is caught. | ||
| fn normalize(src: &str) -> String { | ||
| let mut s = src.to_string(); | ||
| // Collapse any spacing around `::` to the bare `::` token. | ||
| while s.contains(" ::") { | ||
| s = s.replace(" ::", "::"); | ||
| } | ||
| while s.contains(":: ") { | ||
| s = s.replace(":: ", "::"); | ||
| } | ||
| s | ||
| } |
There was a problem hiding this comment.
Matcher can be bypassed by valid Rust import forms.
normalize() only collapses spaces around ::, so forbidden imports split by newlines/tabs or grouped as ::{CausalEdge64} can evade detection. That weakens the firewall’s core guarantee.
Suggested hardening diff
-fn normalize(src: &str) -> String {
- let mut s = src.to_string();
- // Collapse any spacing around `::` to the bare `::` token.
- while s.contains(" ::") {
- s = s.replace(" ::", "::");
- }
- while s.contains(":: ") {
- s = s.replace(":: ", "::");
- }
- s
+fn normalize(src: &str) -> String {
+ src.chars()
+ .filter(|c| !c.is_whitespace() && *c != '{' && *c != '}')
+ .collect()
} #[test]
fn firewall_matcher_detects_planted_twin() {
let planted = "use ndarray :: hpc :: causal_diff :: CausalEdge64;";
let norm = normalize(planted);
@@
assert!(
!FORBIDDEN_TWINS.iter().any(|t| ok.contains(t)),
"firewall matcher false-positives on the canonical causal_edge path"
);
+
+ let planted_newline = "use ndarray::hpc::\ncausal_diff::CausalEdge64;";
+ let norm_newline = normalize(planted_newline);
+ assert!(FORBIDDEN_TWINS.iter().any(|t| norm_newline.contains(t)));
+
+ let planted_group = "use ndarray::hpc::causal_diff::{CausalEdge64 as CE};";
+ let norm_group = normalize(planted_group);
+ assert!(FORBIDDEN_TWINS.iter().any(|t| norm_group.contains(t)));
}As per coding guidelines, “Add Rust unit tests alongside implementations via #[cfg(test)] modules; prefer focused scenarios over broad integration tests.”
Also applies to: 85-87, 104-121
🤖 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/cognitive-shader-driver/tests/firewall.rs` around lines 46 - 57, The
normalize() function only handles space characters around the `::` token,
allowing imports to bypass detection using other whitespace characters
(newlines, tabs) or alternative Rust syntax like `::{CausalEdge64}`. Enhance the
normalize() function to handle all whitespace characters (not just spaces) and
collapse grouped import forms like `::{ `. Additionally, add focused unit tests
using #[cfg(test)] to validate that normalize() correctly handles these various
import patterns, including space variations, newline/tab characters, and
curly-brace grouping syntax.
Source: Coding guidelines
Comment on lines
+80
to
+83
| let raw = match fs::read_to_string(file) { | ||
| Ok(c) => c, | ||
| Err(_) => continue, | ||
| }; |
There was a problem hiding this comment.
Fail closed when a source file cannot be read.
Skipping unreadable files (Err(_) => continue) can hide violations and let the firewall pass with incomplete coverage. The lint should fail the test immediately with file context.
🤖 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/cognitive-shader-driver/tests/firewall.rs` around lines 80 - 83, The
error handling for fs::read_to_string(file) in the firewall test silently skips
unreadable files with continue, which can hide violations and allow the test to
pass with incomplete coverage. Replace the Err(_) => continue branch to instead
panic or return an error with file context information (including the filename),
ensuring the test fails immediately when any source file cannot be read.
Comment on lines
+265
to
+283
| // Arm A — singleton (no mailbox registered → backing() = Singleton fallback). | ||
| let driver_singleton = CognitiveShaderBuilder::new() | ||
| .bindspace(Arc::new(build_bindspace(len))) | ||
| .semiring(semiring.clone()) | ||
| .planes(planes) | ||
| .build(); | ||
|
|
||
| // Arm B — mailbox (DEFAULT_MAILBOX registered → backing() = Mailbox). | ||
| let driver_mailbox = CognitiveShaderBuilder::new() | ||
| .bindspace(Arc::new(build_bindspace(len))) | ||
| .semiring(semiring) | ||
| .planes(planes) | ||
| .with_mailbox(DEFAULT_MAILBOX, mirror_mailbox(len)) | ||
| .build(); | ||
|
|
||
| let crystal_a = driver_singleton.dispatch(req); | ||
| let crystal_b = driver_mailbox.dispatch(req); | ||
| assert_crystals_bit_identical(&crystal_a, &crystal_b, "full-window vs mirror"); | ||
| crystal_a |
There was a problem hiding this comment.
Add an explicit guard that the mailbox arm is actually exercised.
Right now this can still pass if both drivers silently route through singleton (e.g., mailbox selection drift), because inputs are mirrored and only bit-identity is checked. Add a small sentinel test/assertion that proves mailbox-backed reads are active.
🤖 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/cognitive-shader-driver/tests/w2_differential.rs` around lines 265 -
283, The test verifies that driver_singleton and driver_mailbox produce
identical results, but does not explicitly confirm that driver_mailbox is
actually using the mailbox path and not silently falling back to singleton
behavior. Add an explicit assertion or guard after dispatching on driver_mailbox
(after the line with driver_mailbox.dispatch(req)) that proves mailbox-backed
reads were active and the mailbox was exercised, such as checking mailbox state,
access counts, or other metadata that would confirm the mailbox path was taken
rather than a singleton fallback.
PR #520's `linux-build (stable)` job failed on a transient/infra flake, NOT a W3+W4a regression. Evidence: - #520 changed only cognitive-shader-driver + docs; NO Cargo.lock, NO workspace manifest, NO lance-graph-core change. - `linux-build` compiles ONLY lance-graph core (default features), which does NOT depend on cognitive-shader-driver — so the bytes it compiles are identical to main. - `main` (d2f9b7d, #520's base) is GREEN on this exact job. - clippy + test (stable) + test-with-coverage all PASSED on #520. Empty commit to re-run the pull_request workflow (API rerun-failed-jobs was 403). This commit is dropped on the eventual rebase/merge. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CcpLeEC3XK8Eye53GKBVvi
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.
What
The W3+W4a atomic step of the
BindSpace→MailboxSoAmigration: a feature-gated read/write shim and dispatch re-point, plus the hardened plan that specifies it. Follows the merged column phase (#517 W0/W1, #518 W1b, #519 W1c).This PR now contains both the plan AND the verified implementation (the build landed on this branch as one atomic, self-contained unit).
Commits
bindspace-mailbox-soa-w3-w4a-impl-v1.md) — hardened by a 5-consolidation + 3-brutal-critic pass (v1 verdict was HOLD; v2 integrates every P0/P1).b5c17d52— the implementation.Implementation
mailbox-thoughtspacefeature, default-OFF (not indefault, not inlab) — production stays singleton-read + singleton-write; the mailbox arm is opt-in and exercised only by the differential harness until W4b/W7.backing.rs—BackingStore<'a>(read) +BackingStoreWrite<'a>(write) enums;Singleton(&BindSpace)is the live default,#[cfg(feature)] Mailbox(&MailboxSoA<1024>)the gated arm. Driver-local, monomorphized, no&dynin the hot loop, no contract change.driver.rs::run()route through onebackingvalue.ontology()stays on the singleton (re-home is W4b).win.start.min(populated)..win.end.min(populated), byte-identical toBindSpace::meta_prefilter; NOT0..populated(which would phantom-row on any non-zerorow_startand pass a green test).MailboxSoA::edge(row)(natively typedcausal_edge::CausalEdge64); no rawu64ever surfaces on that arm.const DEFAULT_MAILBOX: MailboxId = 0+debug_assert!(mailboxes.len() <= 1)(singleton fallback when empty); multi-mailbox routing is deferred to W5. NoShaderDispatchcontract change.Arc::get_mut→ owned&mut MailboxSoA) remains W4b.unbind_busdtolossy narrowing is feature-gated with a doc migration pointer; the singleton arm keeps its bit-exact behaviour via#[cfg(not(feature))], the mailbox arm pins the documented loss (non-headlinetop_kindices → 0) in a separate test.Gates (all green)
ShaderCrystalasserted bit-identical viato_bits()across 4 cases — full-window, non-zero-window(1, len-1), meta-prefilter, alpha-merge — with non-vacuity assertions. No ULP loosening.tests/firewall.rs): rejects imports/aliases of bothCausalEdge64twins —ndarray::hpc::causal_diff::CausalEdge64andthinking_engine::layered::CausalEdge64— into this crate, while allowing the legitrepr(transparent)cast sites. Includes a planted-twin positive test proving the lint has teeth.cycle*symbol) tests.cargo fmtclean.Non-goals
BindSpace. No production behaviour change. No W4b bin conversion. No W5 routing. No W6/W7 tombstone/delete.Pre-existing issues surfaced (NOT introduced here, out of scope)
cargo build --features with-enginedoes not compile on clean HEAD (engine_bridge.rs:259referencesQUALIA_DIMSunimported); the D-CSV-5b i4-qualia cutover breaks the BusDtocodebook_indexround-trip. The C5unbind_busdtoedit is correctly dormant behindwith-engine.lance-graph-ontologycarries pre-existing clippy debt (oxrdf::Subjectdeprecations + doc-format lints) that a strict workspace-clippy run flags independent of this PR.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
mailbox-thoughtspaceCargo feature for alternative backing storage configurationRefactor
Tests