feat(mailbox-soa): bindspace→mailbox migration wave A1-A4 (thoughtspace columns + transitional routing + WitnessTable + plan §10)#427
Merged
Conversation
…/// work] Adds a sibling routing surface to ShaderDriver alongside the existing Arc<BindSpace> singleton, letting consumers opt into per-mailbox routing before the S3 cutover: HashMap<MailboxId, MailboxSoA<1024>> field, a with_mailbox builder setter on CognitiveShaderBuilder, and a mailbox() read accessor. Additive only — no existing method or field touched. https://claude.ai/code/session_01LiUiGeUDLje8KMnxB4FfA3
…olver) [/// work] Declares WitnessEntry + WitnessTable<N=64> in lance-graph-contract::witness_table. Resolves the 6-bit W-slot index from CausalEdge64 v2 into per-cohort (mailbox_ref, spo_fact_ref) tuples; 3 unit tests green; zero new dependencies. https://claude.ai/code/session_01LiUiGeUDLje8KMnxB4FfA3
…LOG + LATEST_STATE) Orchestrator review pass over the 4-worker wave on the bindspace-singleton-to-mailbox-soa migration: - D-MBX-A1 `feat(mailbox-soa): add thoughtspace columns` (commit 1df12ec) - D-MBX-A2 `feat(driver): add transitional per-mailbox routing` (commit 61b641d) - D-MBX-A3 `feat(contract): add WitnessTable column-type primitive` (commit ef848a3) - D-MBX-A4 `docs(plan): append §10 architectural refinements` (commit 0f44873) This commit: 1. Removes all 41 `/// work` / `<!-- ///work -->` markers across the 5 touched files (per the wave's scratch-marker discipline — orchestrator removes after validation). 2. Prepends D-MBX-A4 entry to AGENT_LOG.md (parity with A1/A2/A3 which prepended their own; A4 worked in a separate worktree branch so its prepend landed there, not on the standing branch — adding here for log continuity). 3. Updates `.claude/board/LATEST_STATE.md` "Current Contract Inventory" with the wave's additions (`WitnessEntry` + `WitnessTable<N=64>` in `lance-graph-contract`; four thoughtspace columns + 8 accessors on `MailboxSoA<N>`; transitional `mailboxes` HashMap + builder on `ShaderDriver`). Per CLAUDE.md mandatory board-hygiene rule — PR-with-new-types must update LATEST_STATE in the same commit. Aggregate verification: - `cargo check -p cognitive-shader-driver -p lance-graph-contract` → clean (only pre-existing ontology deprecation warnings unrelated to this wave). - `cargo test -p cognitive-shader-driver -p lance-graph-contract --lib` → 457 passed, 0 failed. No semantic changes from the workers' commits — this is a marker-strip + board-update commit only.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds a witness-table contract, extends MailboxSoA with four thoughtspace columns and row accessors, wires per-mailbox SoA storage into ShaderDriver via a HashMap and builder API, and records these changes in agent logs and the architectural plan. ChangesBindspace-to-Mailbox Migration: Contract, SoA, and Driver Wiring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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
🧪 Generate unit tests (beta)
Comment |
![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: 683288784d
ℹ️ 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".
| /// | ||
| /// Active mailboxes have a live `w_slot` association; tombstoned mailboxes retain | ||
| /// their ref so the arc walk can detect decommissioned cohort members. | ||
| pub mailbox_ref: u16, |
There was a problem hiding this comment.
When the mailbox population uses IDs above 65,535, this field cannot round-trip the canonical MailboxId (collapse_gate.rs defines it as a unique u32, and the migration notes target a 64K–256K mailbox envelope). Storing only the low 16 bits makes IDs such as 1 and 65_537 indistinguishable in the witness table, so later W-slot resolution can walk a belief arc to the wrong mailbox or tombstone. Use the full MailboxId (or a separate explicit cohort-local index) rather than truncating the unique handle.
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: 1
🤖 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/lance-graph-contract/src/witness_table.rs`:
- Around line 55-57: Update the size comment to remove the incorrect "16 B"
claim: state that `Option<u64>` packs to the same size as `u64` (8 B) via
niche-filling, and then describe the resulting struct size as ~10–12 B (u16 = 2
B + Option<u64> = 8 B plus alignment/padding). Keep the note about `Copy` being
intentional and reference the same comment block in witness_table.rs so readers
see that the niche-filling optimization makes `Option<u64>` 8 bytes, not 16.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6499359e-4d7f-4a39-ac08-fbba1a9275c3
📒 Files selected for processing (7)
.claude/board/AGENT_LOG.md.claude/board/LATEST_STATE.md.claude/plans/bindspace-singleton-to-mailbox-soa-v1.mdcrates/cognitive-shader-driver/src/driver.rscrates/cognitive-shader-driver/src/mailbox_soa.rscrates/lance-graph-contract/src/lib.rscrates/lance-graph-contract/src/witness_table.rs
Comment on lines
+55
to
+57
| /// `u16` (2 B) + discriminant + `u64` (8 B) = 10–12 B. `Option<u64>` packs as 16 B | ||
| /// via the niche-filling optimisation. `Copy` is intentional: the struct is small | ||
| /// enough to pass by value on any target ABI. |
There was a problem hiding this comment.
Correct the size comment.
The comment states that Option<u64> "packs as 16 B via the niche-filling optimisation," but niche optimization actually makes Option<u64> the same size as u64 (8 bytes), not 16 bytes. The total struct size is approximately u16 (2 B) + Option<u64> (8 B) + padding for alignment, which gives the stated 10–12 B, but the intermediate claim about 16 B is incorrect.
📝 Proposed fix for the size comment
-/// `u16` (2 B) + discriminant + `u64` (8 B) = 10–12 B. `Option<u64>` packs as 16 B
-/// via the niche-filling optimisation. `Copy` is intentional: the struct is small
-/// enough to pass by value on any target ABI.
+/// `u16` (2 B) + `Option<u64>` (8 B via niche-filling) + padding = ~10–12 B
+/// depending on alignment. `Copy` is intentional: the struct is small enough
+/// to pass by value on any target ABI.🤖 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/lance-graph-contract/src/witness_table.rs` around lines 55 - 57,
Update the size comment to remove the incorrect "16 B" claim: state that
`Option<u64>` packs to the same size as `u64` (8 B) via niche-filling, and then
describe the resulting struct size as ~10–12 B (u16 = 2 B + Option<u64> = 8 B
plus alignment/padding). Keep the note about `Copy` being intentional and
reference the same comment block in witness_table.rs so readers see that the
niche-filling optimization makes `Option<u64>` 8 bytes, not 16.
… size doc Two review findings on PR #427: 1. **Codex P2** — `WitnessEntry::mailbox_ref` was declared `u16`, but `contract::collapse_gate::MailboxId` is `pub type MailboxId = u32`. The W-slot is the per-cohort *index* (0..=63, fits in 6 bits), but `mailbox_ref` carries the *identity* of the mailbox at that slot — and the workspace's planned 64K-256K mailbox envelope (plan §10 refinement (3)) exceeds the u16 ceiling. Truncating to the lower 16 bits would alias mailbox 1 with mailbox 65_537, so the W-slot belief-arc walk could resolve to the wrong mailbox or tombstone. Widened to `u32` to preserve full canonical identity end-to-end. 2. **CodeRabbit minor** — The doc comment claimed `Option<u64>` "packs as 16 B via the niche-filling optimisation." This is wrong on mechanism: `u64` has no niche (every bit pattern is a valid `u64`), so the discriminant cannot fold into the payload. The 16 B figure is correct but the *reason* is 1-byte tag + 7-byte alignment padding + 8-byte payload, not niche-filling. Corrected the comment and updated the total entry size (`u32` + `Option<u64>` → **24 B** per entry with `#[repr(Rust)]` field reordering; 64-entry table = 1.5 KiB). Module-level doc-comment in the file header also updated to call out that `mailbox_ref` is the full canonical identity, not a truncated index. Verification: - `cargo test -p lance-graph-contract --lib witness_table` → 3/3 pass (tests use integer literals so type inference adapts automatically; no test changes needed) - LATEST_STATE.md Contract Inventory annotation updated to mirror the u32 type (board hygiene rule — same commit as the type change). No other callers exist yet (only `lib.rs` re-export) so the widening is a pure correctness fix with no migration cost.
|
Actionable comments posted: 0 |
AdaWorldAPI
pushed a commit
that referenced
this pull request
May 28, 2026
…tries Post-merge board hygiene for two PRs merged in this wave: - **#425** (mine, lance-graph deps cleanup + [patch.crates-io] ndarray declared intent; merge commit 1a3abfb). Locks in the lance 6.0.1 block reasoning (lancedb 0.29.0 transitive ='6.0.0') and the ndarray patch's "declared but unused" effect. - **#427** (bindspace→mailbox migration wave A1-A4; merge commit 8429611). Authored by a peer session; per-deliverable AGENT_LOG entries were prepended at branch HEAD pre-merge, but the LATEST_STATE / PR_ARC tier of governance was left for follow-up — this commit fills it. Records the 7 ratified plan §10 findings + the 2 surviving OQs (OQ-MBX-8 persisted_row vs Lance native versioning; OQ-MBX-15′ container scoping). Both via tee -a (the 8 bookkeeping files deny Edit/Write); dated framing preserves the rule-#1 PREPEND convention. No code touched. No new types proposed. Outstanding: #426 (odoo blueprint Stage 1, +115K lines) also lacks a LATEST_STATE/PR_ARC row; defer to the authoring session for the rich sub-deliverable breakdown (D-ODOO-EXT-1..6 + EXT-3 + BP-1a/b) — a peer session writing #426 governance would either summarize too coarsely or duplicate the per-deliverable AGENT_LOG entries that already exist on main. https://claude.ai/code/session_01FMooFcE7hgRWWvknNr2N4i
3 tasks
AdaWorldAPI
pushed a commit
that referenced
this pull request
May 28, 2026
…row_idx + cascade sink + stale-marker fixes Addresses Codex P1 + P2 and 6 CodeRabbit findings on the normalized-entity-holy-grail-v1 Stage 1 surface. Two findings (colocated unit tests in advance.rs + interactive.rs) deferred to Stage 2 when concrete kernel bodies exist. CRITICAL — typestate teeth (Codex P1): - NormalizedEntity::advance_stage was pub; external code could cast Raw → Reported. Renamed to advance_stage_internal and demoted to pub(crate). External Op implementors cannot construct any NormalizedEntity<S> for S != Raw. - Op trait redesigned: implementors override only step() (a validation + side-effect hook with default no-op success) + kind(). No more public apply() to override. Framework's chain methods (op / chk_data / review / abduct / report) call op.step() then perform the sealed transition. - New OpError type carries a &'static str for Stage 1; Stage 2 widens to typed reason enum + row ref for audit trail. CORRECTNESS — row_idx widening (Codex P2 + CodeRabbit 5): - MailboxRow::row_idx: u16 → u32. Matches PR #427's symmetric mailbox_ref widening; satisfies the documented 64K-256K per-mailbox envelope. API DESIGN — CascadeWalker sink (CodeRabbit 4): - CascadeWalker::walk_dependents now takes on_dependent: &mut dyn FnMut(MailboxRow). The walker output is expressible at the type level. DOC DRIFT (CodeRabbit 1, 3, 9): - cascade.rs: ServerAction doc no longer claims "encoded as Other + tag" (it IS its own variant). - AGENT_LOG.md first entry + docs/COGNITION_HOLY_GRAIL.md: stale /// work references → // TODO(Stage 2): to match the post-strip state. INTEGRATION TEST FIXUP: - tests/cognition_typestate.rs: 5 fake Op impls (NoopOp, FakeChkData, FakeReview, FakeAbduct, FakeReport) updated to the new step()-only shape. Their old apply() overrides (which called the now-pub(crate) advance_stage) removed; default no-op step() suffices. DEFERRED TO STAGE 2 (CodeRabbit 2, 7): - Colocated #[cfg(test)] tests in advance.rs + interactive.rs. Tests post-fix: - cargo clippy -D warnings clean - cargo test --lib green (472 tests) - cargo test --test cognition_typestate green (7 tests) - cargo test --doc green (3 tests; new compile_fail block in src/cognition/mod.rs proves the typestate seal) https://claude.ai/code/session_017gZ6sPRXYPj5n7uJ7NBtRv
AdaWorldAPI
pushed a commit
that referenced
this pull request
May 28, 2026
… size doc Two review findings on PR #427: 1. **Codex P2** — `WitnessEntry::mailbox_ref` was declared `u16`, but `contract::collapse_gate::MailboxId` is `pub type MailboxId = u32`. The W-slot is the per-cohort *index* (0..=63, fits in 6 bits), but `mailbox_ref` carries the *identity* of the mailbox at that slot — and the workspace's planned 64K-256K mailbox envelope (plan §10 refinement (3)) exceeds the u16 ceiling. Truncating to the lower 16 bits would alias mailbox 1 with mailbox 65_537, so the W-slot belief-arc walk could resolve to the wrong mailbox or tombstone. Widened to `u32` to preserve full canonical identity end-to-end. 2. **CodeRabbit minor** — The doc comment claimed `Option<u64>` "packs as 16 B via the niche-filling optimisation." This is wrong on mechanism: `u64` has no niche (every bit pattern is a valid `u64`), so the discriminant cannot fold into the payload. The 16 B figure is correct but the *reason* is 1-byte tag + 7-byte alignment padding + 8-byte payload, not niche-filling. Corrected the comment and updated the total entry size (`u32` + `Option<u64>` → **24 B** per entry with `#[repr(Rust)]` field reordering; 64-entry table = 1.5 KiB). Module-level doc-comment in the file header also updated to call out that `mailbox_ref` is the full canonical identity, not a truncated index. Verification: - `cargo test -p lance-graph-contract --lib witness_table` → 3/3 pass (tests use integer literals so type inference adapts automatically; no test changes needed) - LATEST_STATE.md Contract Inventory annotation updated to mirror the u32 type (board hygiene rule — same commit as the type change). No other callers exist yet (only `lib.rs` re-export) so the widening is a pure correctness fix with no migration cost.
AdaWorldAPI
added a commit
that referenced
this pull request
May 28, 2026
…sis-LXmug feat(mailbox-soa): bindspace→mailbox migration wave A1-A4 (thoughtspace columns + transitional routing + WitnessTable + plan §10)
AdaWorldAPI
pushed a commit
that referenced
this pull request
May 28, 2026
…tries Post-merge board hygiene for two PRs merged in this wave: - **#425** (mine, lance-graph deps cleanup + [patch.crates-io] ndarray declared intent; merge commit a5f3e8d). Locks in the lance 6.0.1 block reasoning (lancedb 0.29.0 transitive ='6.0.0') and the ndarray patch's "declared but unused" effect. - **#427** (bindspace→mailbox migration wave A1-A4; merge commit a21d577). Authored by a peer session; per-deliverable AGENT_LOG entries were prepended at branch HEAD pre-merge, but the LATEST_STATE / PR_ARC tier of governance was left for follow-up — this commit fills it. Records the 7 ratified plan §10 findings + the 2 surviving OQs (OQ-MBX-8 persisted_row vs Lance native versioning; OQ-MBX-15′ container scoping). Both via tee -a (the 8 bookkeeping files deny Edit/Write); dated framing preserves the rule-#1 PREPEND convention. No code touched. No new types proposed. Outstanding: #426 (odoo blueprint Stage 1, +115K lines) also lacks a LATEST_STATE/PR_ARC row; defer to the authoring session for the rich sub-deliverable breakdown (D-ODOO-EXT-1..6 + EXT-3 + BP-1a/b) — a peer session writing #426 governance would either summarize too coarsely or duplicate the per-deliverable AGENT_LOG entries that already exist on main. https://claude.ai/code/session_01FMooFcE7hgRWWvknNr2N4i
AdaWorldAPI
pushed a commit
that referenced
this pull request
May 29, 2026
… + OQ-11.9 Orchestrator review pass on PR #434 (merged 2026-05-29) and wave A1-A4 (merged via PR #427). All amendments APPENDED — no §1–§16 content modified, per the plan-file APPEND-ONLY discipline. §17 captures 4 substantive amendments + 1 cross-ref: - §17.1 FINDING — prior `bindspace-singleton-to-mailbox-soa-v1.md` OQ-MBX-8 (`persisted_row` vs Lance versioning) is implicitly closed by this plan's R1.2 ("persisted_row is a pointer to the same row laid down in Lance, not a serialized copy"). Action: mark resolved → wired by D-MBX-7. - §17.2 FINDING — prior OQ-MBX-15′ (container scoping: per-cycle vs per-dispatch vs per-cohort) is implicitly closed by R1 + R4 + the Codex P2 widening of `WitnessEntry::mailbox_ref: u16 → u32` shipped via PR #427. A per-cycle container would invalidate every cross-cycle `mailbox_ref u32` the moment the cycle terminates. Action: per-mailbox-cohort, cohort-lifetime. - §17.3 CONJECTURE — silent gap between R1/D-MBX-A6/D-MBX-7 and the current `ndarray::simd_soa::MultiLaneColumn` surface. The shipped framework exposes only flat `Arc<[u8]>` lanes; the new thoughtspace columns (`edges`/`qualia`/ `meta`/`entity_type`) are heterogeneous fixed-size structs. R1's "container ≡ SoA ≡ simd_soa-aligned" equivalence is aspirational until a `SoaColumns<N>` shape-introspection trait lands. Action: NEW D-MBX-A7 added to STATUS_BOARD as a P1 prereq gating D-MBX-A6 + D-MBX-7. ~200 LOC, MED risk, cross-repo (ndarray). - §17.4 CONJECTURE — `WitnessEntry { mailbox_ref: u32, spo_fact_ref: Option<u64> }` encodes the Σ10 Rubicon commit transition (R3) as a runtime Option instead of as typestate. NEW OQ-11.9 added to §11 catalogue: promote to `WitnessEntry::Active{...}` / `WitnessEntry::Crystallised{...}` enum split? Default proposal: yes; lands as part of D-MBX-A5 (LOC bumps ~150 → ~200); no migration cost (only consumer today is lib.rs re-export). - §17.5 FINDING — D-MBX-12.9 (thinking-styles/atoms unification) inherits `style_recipe` from PR #433 commit `acb403de` ("feat(odoo): style_recipe — D-Atom interpretation step (typed SoA → cognitive fingerprint)"). The unification is not from scratch; cite as Predecessor. Board hygiene (same commit per Mandatory Board-Hygiene Rule): - STATUS_BOARD.md — new D-MBX-A7 row inserted between A6 and 7; A6 + 7 PR/Evidence cells annotated with `+ D-MBX-A7 (§17.3)` gate. - AGENT_LOG.md — orchestrator review entry prepended. No source code touched, no `cargo` invoked (continuing #434's stability constraint). Confidence: §17.1 + §17.2 + §17.5 HIGH (cited evidence); §17.3 + §17.4 MED-HIGH (the gap and the typestate are real; action shapes are default proposals open to council via PR review).
This was referenced Jun 12, 2026
Merged
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
First implementation pulse of the bindspace-singleton-to-mailbox-soa migration (PR #418 plan). 4 parallel sonnet workers, each scoped to a single slice, reviewed by the orchestrator before this PR opened.
MailboxSoA<N>(edges/qualia/meta/entity_type) + 8 row accessors + zero-init innew()+ reset inreset_row()1df12ecamailboxes: HashMap<MailboxId, MailboxSoA<1024>>+with_mailbox()builder +mailbox()read accessor onShaderDriver— sibling-shape, additive, singleton untouched61b641d5WitnessTable<N=64>+WitnessEntry { mailbox_ref: u16, spo_fact_ref: Option<u64> }primitive inlance-graph-contract::witness_table(zero-dep, 3 unit tests,const fn new,get/setwith bounds-check)ef848a34.claude/plans/bindspace-singleton-to-mailbox-soa-v1.md— 7 ratified findings + 2 surviving OQs0f448730/// work/<!-- ///work -->scratch markers; prepend AGENT_LOG entries; update LATEST_STATE Contract Inventory per board-hygiene rule68328878What §10 (A4) ratifies
The 7 architectural refinements that landed during the conversation arc preceding this wave:
64²/256²/4096²/16384²are per-axis granularities (causal / palette / COCA codebook / outcome), superposed and streamed like x265 cascaded prediction levels.(mailbox_ref, spo_fact_ref)entries — NOT a witness-corpus pointer. Markov belief-update arc via AriGraph episodic-reference vectors (AriGraph today is a transcode; the chaining engine is the target shape).simd_soa.rs(ndarray) is the SoA dispatch framework — introspects per-SoA shape; migration is positional, not structural.Surviving OQs
persisted_rowstub vs Lance native versioning (load-bearing; evidence atREFACTOR_NOTES.md:129+driver.rs:458).Scope discipline (what this PR does NOT do)
Arc<BindSpace>singleton — A2 is a sibling surface; the cutover ships in a later slice.WitnessTableintoCausalEdge64emission paths — A3 declares the primitive only.BindSpace— A1 makes the columns available onMailboxSoA; the source-singleton dissolution is a downstream slice.cognitive-shader-driver+lance-graph-contract.Verification
cargo check -p cognitive-shader-driver -p lance-graph-contract— clean (only pre-existing ontology deprecation warnings, unrelated).cargo test -p cognitive-shader-driver -p lance-graph-contract --lib— 457 passed, 0 failed.Board hygiene (per CLAUDE.md mandatory rule)
.claude/board/AGENT_LOG.md— D-MBX-A1/A2/A3/A4 entries prepended..claude/board/LATEST_STATE.md"Current Contract Inventory" — annotated with WitnessTable + MailboxSoA additions..claude/plans/bindspace-singleton-to-mailbox-soa-v1.md§10 appended.Test plan
https://claude.ai/code/session_01LiUiGeUDLje8KMnxB4FfA3
Generated by Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests