feat: 4-task unblock-cascade — NiblePath::from_guid_prefix + MailboxSoaOwner impl + LanceVersionScheduler + SurrealMailboxView (D-PG-6)#507
Merged
Conversation
…oaOwner cherry-pick + LanceVersionScheduler + SurrealMailboxView (D-PG-6) Lands four tasks from the shortest-unblocking-path list surfaced after PR #497-#501 + the AdaWorldAPI/surrealdb fork bump (lance/lance-index =7.0.0, lancedb =0.30.0, ndarray exact-rev). All four meet at the single contract trait `MailboxSoaView`, closing the cascade in one commit (E-UNBLOCK-CASCADE-1). ## Task 3 — `NiblePath::{from_guid_prefix, prefix}` (zero-dep, foundational) Ontology-side keystone follow-up of PR #498's `classid → ReadMode` LE contract. The 20-nibble `classid(8) | HEEL(4) | HIP(4) | TWIG(4)` prefix overflows the 16-nibble MAX_DEPTH: the deterministic fold drops the canon-reserved high u16 of classid (root-first pack: `classid_lo(4) | HEEL(4) | HIP(4) | TWIG(4)`), returning None when the fold would be lossy. `prefix(d)` is the O(1) ancestor view; `prefix(d).is_ancestor_of(self)` holds for every d ≤ self.depth (the routing-cache view of a deeper class path). +7 tests in `hhtl::tests`; contract lib 619 → 632 green. ## Task 2 — `impl MailboxSoaView + MailboxSoaOwner for MailboxSoA<N>` Cherry-pick of jolly-cori-clnf9 commit 463d71b (integrated-cognitive-planner-v1 §2 Seam #3, +149 LOC). Adds `pub phase: KanbanColumn` field + zero-copy repr(transparent) slice impls (edges_raw, meta_raw) + the in-RAM Rubicon driving-loop test (`test_in_ram_driving_loop_walks_rubicon_to_commit`). The contract spine (#437/#439) now drives an actual loop end-to-end — no surreal, no ractor bus needed for the in-process case. +1 driving-loop test; cognitive-shader-driver lib 85 → 86 green. ## Task 1 — `LanceVersionScheduler` over `VersionedGraph::versions()` D-MBX-9-IN core impl (the CI-gated twin of the contract slice shipped 2026-05-31). Lives in `crates/lance-graph/src/graph/scheduler.rs`. Wraps a `VersionedGraph` + inner `VersionScheduler<S = NextPhaseScheduler>` and exposes: - `drive_once(view, exec)` — read current Lance version, lower to a move - `drive_at_latest(view, exec)` — fold `versions().last()` into a move - `current_dataset_version()` — typed `DatasetVersion` over nodes head Closes `E-SUBSTRATE-IS-THE-SCHEDULER`'s OUT-direction end-to-end. The OUT direction stays propose-not-dispose (R1): returned `KanbanMove` is for the caller's `MailboxSoaOwner::try_advance_phase` to apply. +5 tests with real on-disk tempdir Lance (no mocks). ## Task 4 — `SurrealMailboxView<'a>` (D-PG-6 contract slice) Read-only `MailboxSoaView` adapter the SurrealQL projection populates via `from_columns(...)` — pure zero-copy borrow over the kv-lance scan's byte buffers. Imports `MailboxSoaView` but NOT `MailboxSoaOwner` (compile-time enforcement of `kanban.rs:1-21` "surreal=project-read-only, callcenter=commit"). `read_via_kv_lance()` returns the new typed `SurrealContainerError::BlockedColdBuild` until the surrealdb fork dep in `Cargo.toml` is uncommented — kept off by default to avoid the ~10 min cold surrealdb build for contributors who don't need it. The contract surface is available today; the integration is one Cargo.toml edit + a SurrealQL projection body in `view.rs`. +4 tests; new `lance-graph-contract` dep in surreal_container/Cargo.toml; BLOCKED(C) marker flipped to RESOLVED. ## What this unblocks - **D-MBX-9-IN-impl** — SHIPPED (the contract trait now has a Lance-backed implementor). - **D-MBX-A6-P3** — still queued, BUT Seam #3 (the in-RAM loop) is now in-tree; a downstream session can wire the emit-side without depending on the unmerged jolly branch. - **D-PG-6 (Rubicon kanban VIEW)** — contract slice SHIPPED; impl-side gated on `BlockedColdBuild` flip-on (one Cargo.toml uncomment + projection body). - **Identity-architecture v1 §3 P-SCOPE-CLASSIFY** — solved (the bijection-width fix is deterministic + ancestor-preserving + falsifiable by tests). ## Tests + clippy - lance-graph-contract: **632** (+7 hhtl) - cognitive-shader-driver: **86** (+1 driving-loop) - lance-graph::scheduler: **5** (new module, real Lance tempdir) - surreal_container::view: **4** (new module) All clippy `-D warnings` clean on the new files. Pre-existing lints in lance-graph-ontology / lance-graph-planner / ndarray_bridge.rs are out of session scope. ## Board hygiene (mandatory rule) - LATEST_STATE.md — Contract Inventory PREPEND for the new types. - EPIPHANIES.md — E-UNBLOCK-CASCADE-1: three independent landings converge on one trait surface, closing four queued deliverables in one commit. - AGENT_LOG.md — task-by-task summary with test counts. https://claude.ai/code/session_01Xzyc27Nx3f8WC5KzwfWfjx
📝 WalkthroughWalkthroughAdds ChangesUnblock-cascade: layout safety, NiblePath routing, mailbox lifecycle, Lance scheduler, Surreal view
Sequence Diagram(s)sequenceDiagram
participant Caller
participant LanceVersionScheduler
participant VersionedGraph
participant MailboxSoaView
participant VersionScheduler
Caller->>LanceVersionScheduler: drive_once(view, exec)
LanceVersionScheduler->>VersionedGraph: open nodes.lance dataset
VersionedGraph-->>LanceVersionScheduler: DatasetVersion
LanceVersionScheduler->>MailboxSoaView: phase(), w_slot(), current_cycle()
MailboxSoaView-->>LanceVersionScheduler: KanbanColumn, metadata
LanceVersionScheduler->>VersionScheduler: on_version(view, version, exec)
VersionScheduler-->>LanceVersionScheduler: Option<KanbanMove>
LanceVersionScheduler-->>Caller: Ok(Option<KanbanMove>)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
|
![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: 8017ca19a5
ℹ️ 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".
| // `[u64; N]`. The pointer is non-null, aligned, and valid for `N` `u64` | ||
| // reads; the returned slice borrows `&self`, so it cannot outlive the | ||
| // backing array. Zero-copy — never clones the store (R1). | ||
| unsafe { core::slice::from_raw_parts(self.edges.as_ptr() as *const u64, N) } |
There was a problem hiding this comment.
When any consumer calls MailboxSoaView::edges_raw() on a real mailbox, this unsafe cast relies on CausalEdge64 having the exact layout of u64, but in the reviewed tree crates/causal-edge/src/edge.rs defines it as a plain tuple struct without #[repr(transparent)]. That makes the zero-copy slice reinterpretation unsound under Rust's layout rules; add the repr guarantee to CausalEdge64 or avoid the raw slice cast.
Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment.
Resolved on the current tip (c181fec4). CausalEdge64 now carries #[repr(transparent)] over u64 (crates/causal-edge/src/edge.rs:156), edges_raw() / meta_raw() document the SAFETY invariant against it, and test_edges_raw_meta_raw_reinterpret_round_trips asserts the reinterpret is bit-exact and zero-copy (same backing pointer). The unsound cast you flagged is now sound by construction.
| /// Mutated only via [`MailboxSoaOwner::advance_phase`] / | ||
| /// [`MailboxSoaOwner::try_advance_phase`]; starts at | ||
| /// [`KanbanColumn::Planning`]. | ||
| pub phase: KanbanColumn, |
There was a problem hiding this comment.
Because phase is a public field, any downstream crate holding a MailboxSoA<N> can assign an arbitrary KanbanColumn directly, bypassing MailboxSoaOwner::try_advance_phase, the lifecycle DAG check, and the KanbanMove emission that this change is trying to centralize. If the phase is meant to be mutated only through the owner trait, keep this field private or expose only checked setters.
Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment.
Fixed in c181fec4. You're right — the field doc claimed "mutated only via the owner trait" but pub left that as documentation, not enforcement.
Changed pub phase → pub(crate) phase, so a downstream crate can no longer assign a KanbanColumn directly and bypass try_advance_phase (the lifecycle-DAG check) + the KanbanMove emission.
Verified safe across cargo check --workspace --all-targets (exits 0):
- the only writer is the in-crate
impl MailboxSoaOwner for MailboxSoA<N>::advance_phase;pub(crate)keeps that compiling; - the
soa_view.rswrite is on theFakeSoatest double, notMailboxSoA; - no external crate reads the field directly — reads go through the unchanged
MailboxSoaView::phase()getter.
mailbox_soa lib tests (12) + soa_view tests (3) green.
| // — format!("{:?}", graph) yields `VersionedGraph { base_path: "..." }`. | ||
| // Parse out the quoted value. This is the cheapest read-only path | ||
| // until VersionedGraph exposes `pub fn base_path(&self) -> &str`. | ||
| let dbg = format!("{:?}", self.graph); |
There was a problem hiding this comment.
When VersionedGraph::local is constructed with a path containing characters that Debug escapes, such as a quote in a Unix filename, this parser reads the formatted debug string rather than the original path and drive_once opens the wrong nodes.lance path. VersionedGraph already exposes current_version()/versions() using the real private base_path, so this method should call that API or add a proper accessor instead of depending on Debug output.
Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment.
Resolved on the current tip (c181fec4). scheduler.rs now reads the dataset path through a real VersionedGraph::base_path() accessor (format!("{}/nodes.lance", self.graph.base_path())) — the Debug-scraping path was removed, so a quote (or any Debug-escaped char) in a filename no longer misdirects drive_once.
…rds + fmt + asserts 8-agent review fleet (5 specialists + PP-13/15/16 hardeners) against the 4-task unblock-cascade (PR #507): zero REJECT, but two real defects all four green test suites missed. P0 — incomplete cherry-pick (PP-16 root cause). Task 2 cherry-picked 463d71b's mailbox_soa.rs half (+149) but dropped its causal-edge/src/edge.rs half (+6: #[repr(transparent)] on CausalEdge64). The SAFETY comments cited a repr the type didn't carry — sound on today's rustc by newtype-layout coincidence, unsound by the letter, invisible to CI. P1 — fmt --check overclaim (PP-13). The 4-task entry said "clippy/fmt clean" but only clippy had been run; cargo fmt --check failed on PR-added lines. Fixes: - FIX-A: restore #[repr(transparent)] + doc on CausalEdge64 (edge.rs:148-156); add const _ size/align guards at edges_raw/meta_raw cast sites; corrected both SAFETY comments. - FIX-B: cargo fmt across all 5 touched crates; fmt --check exit 0. - FIX-C (PP-15): SurrealMailboxView::from_columns debug_assert_eq! -> assert_eq! (release-build OOB closed); + from_columns_rejects_ragged_projection panic test. - FIX-D: pub fn base_path() on VersionedGraph; deleted format!("{:?}") Debug-scrape (embedded-quote truncation hazard). - FIX-E (PP-13): test_edges_raw_meta_raw_reinterpret_round_trips — the unsafe cast had ZERO coverage; now bit-exact round-trip + pointer-identity asserted. - FIX-F/H (P2 docs): hhtl bijection 0..=16 -> 1..=16; drive_at_latest scope note + versions().last() upstream-pagination caveat tying ascending-sort assumption to the lance =7.0.0 pin. Process lesson -> EPIPHANIES E-CHERRYPICK-SPANS-CRATES-1 (queued). Board hygiene: AGENT_LOG.md prepended in same commit (this commit), per the mandatory rule — no retroactive cleanup. https://claude.ai/code/session_01Xzyc27Nx3f8WC5KzwfWfjx
…er-enforced (#507 review) Codex P2: MailboxSoA.phase was `pub`, so a downstream crate could assign an arbitrary KanbanColumn directly, bypassing MailboxSoaOwner::try_advance_phase (the lifecycle-DAG check) and the KanbanMove emission this change centralizes. The field doc already claims "mutated only via the owner trait" — but `pub` left that as documentation, not enforcement. Change `pub phase` -> `pub(crate) phase`. Verified safe across the whole workspace (cargo check --workspace --all-targets exits 0): - the only writer is the in-crate impl MailboxSoaOwner for MailboxSoA<N> (advance_phase, mailbox_soa.rs); pub(crate) keeps that working. - the soa_view.rs write is on the FakeSoa test double, not MailboxSoA. - no external crate reads the field directly; reads go through the MailboxSoaView::phase() getter, which is unchanged. mailbox_soa lib tests (12) + soa_view tests (3) green. https://claude.ai/code/session_01VysoWJ6vsyg3wEGc5v7T5v
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/surreal_container/Cargo.toml (1)
42-59:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd explicit crates.io provenance rationale for active dependencies.
This manifest still has active crates.io dependencies without explicit rationale for why they are not AdaWorldAPI forks. Please document that rationale inline (or wire fork coordinates where a fork exists), per repo policy.
Suggested patch shape
[dependencies] +# crates.io rationale: +# - lance/lancedb/tokio/futures/snafu are used from crates.io because +# [explicit repo-approved reason here]. +# - If an AdaWorldAPI fork exists for any of these, replace with fork coords +# (git URL + branch/tag or [patch.crates-io]). lance-graph-contract = { path = "../lance-graph-contract" } lance = "=7.0.0" lancedb = { version = "=0.30.0", optional = true, default-features = false } tokio = { version = "1", features = ["rt-multi-thread", "macros"] } futures = "0.3" snafu = "0.8"As per coding guidelines,
**/Cargo.toml: “EveryCargo.tomlmust have a comment explaining why crates.io dependencies (if any) are NOT AdaWorldAPI forks, and every fork dependency must reference its fork source (git URL, branch, or[patch.crates-io]).”🤖 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/surreal_container/Cargo.toml` around lines 42 - 59, The active crates.io dependencies in this Cargo.toml file (lance, lancedb, tokio, futures, and snafu) are missing inline comments explaining why they are not AdaWorldAPI forks or when forks exist. Add explicit rationale comments above or alongside each of these dependencies to document why they remain on crates.io, following the same pattern as the existing comment block for the commented-out surrealdb fork dependency. Per repo policy, every crates.io dependency must have an associated comment clarifying the decision to use the published version rather than a fork.Source: Coding guidelines
🤖 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/AGENT_LOG.md:
- Around line 10-18: The AGENT_LOG.md file contains conflicting statements about
the repository state: the "Fixes applied" section header claims the fixes are in
a new commit, but the "Disk verification this turn" note states all changes are
uncommitted. Clarify the wording to accurately reflect the actual state — either
update the "Fixes applied" header and descriptions to indicate these are
staged/uncommitted changes awaiting decision, or update the "Disk verification"
note to confirm committed status. Ensure both sections describe the same
repository state consistently so readers understand whether the 34 modified
files with +2841/-1100 changes are committed to the branch or still in the
working directory.
In @.claude/board/EPIPHANIES.md:
- Around line 11-23: The current wording in the "The find" section conflates
NiblePath::from_guid_prefix with the MailboxSoaView trait surface, overstating
their coupling. NiblePath is not an implementation of MailboxSoaView and is
actually part of separate work. Reword the section to clarify that the actual
trait surface convergence point involves only MailboxSoA<N> and
SurrealMailboxView as implementations of MailboxSoaView/MailboxSoaOwner, while
NiblePath::{from_guid_prefix, prefix} is separate work that should be called out
as Task 3 independently adding that ontology-side keystone. Restructure the
bullet points so the three items listed correspond to distinct task boundaries
rather than all appearing to converge on the single trait surface.
In `@crates/cognitive-shader-driver/src/mailbox_soa.rs`:
- Around line 424-429: The libet_offset_us assignment in the mailbox_soa.rs file
gates on both the source and destination columns, but the contract
implementation only gates on the destination column. Remove the from ==
KanbanColumn::Planning condition from the if statement for libet_offset_us so
that it only checks to == KanbanColumn::CognitiveWork. This ensures that the
-550_000 offset is applied whenever entering CognitiveWork from any column,
matching the contract semantics and preventing divergence when new edges to
CognitiveWork are introduced.
In `@crates/lance-graph/src/graph/scheduler.rs`:
- Around line 75-80: The documentation comment for the LanceVersionScheduler
constructor contains an incorrect unit specification for the Libet anchor. The
comment currently states the anchor as -550 µs (microseconds), but the actual
canonical offset is -550 milliseconds (or -550_000 microseconds). Update the doc
comment text to correctly specify -550 ms instead of -550 µs to ensure accurate
timing documentation and prevent confusion.
In `@crates/lance-graph/src/graph/versioned.rs`:
- Around line 142-149: The newly exposed public method `base_path()` lacks a
direct unit test to verify its contract. Add a focused unit test in a
`#[cfg(test)]` module within this file that verifies the `base_path()` method
returns the expected base path value. The test should create a VersionedGraph
instance with a known base path and assert that calling `base_path()` returns
that value correctly.
In `@crates/surreal_container/src/view.rs`:
- Around line 218-221: The function `read_via_kv_lance` returns
`SurrealMailboxView<'static>`, but this contradicts the zero-copy design of the
struct which is intended to work with borrowed slices. Change the return type
lifetime from `'static` to an appropriate lifetime parameter (such as `'a`) that
reflects the borrowed nature of the data the view wraps. This aligns the
function signature with the struct's design intent for zero-copy borrowed data
access.
---
Outside diff comments:
In `@crates/surreal_container/Cargo.toml`:
- Around line 42-59: The active crates.io dependencies in this Cargo.toml file
(lance, lancedb, tokio, futures, and snafu) are missing inline comments
explaining why they are not AdaWorldAPI forks or when forks exist. Add explicit
rationale comments above or alongside each of these dependencies to document why
they remain on crates.io, following the same pattern as the existing comment
block for the commented-out surrealdb fork dependency. Per repo policy, every
crates.io dependency must have an associated comment clarifying the decision to
use the published version rather than a fork.
🪄 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: c9292c9a-d8f3-4426-b583-a284e33e6c5a
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (40)
.claude/board/AGENT_LOG.md.claude/board/EPIPHANIES.md.claude/board/LATEST_STATE.mdcrates/causal-edge/src/edge.rscrates/causal-edge/src/layout.rscrates/causal-edge/src/v2_layout_tests.rscrates/cognitive-shader-driver/build.rscrates/cognitive-shader-driver/examples/villager_ai.rscrates/cognitive-shader-driver/src/attention_mask.rscrates/cognitive-shader-driver/src/attention_mask_actor.rscrates/cognitive-shader-driver/src/auto_detect.rscrates/cognitive-shader-driver/src/auto_style.rscrates/cognitive-shader-driver/src/bin/grpc.rscrates/cognitive-shader-driver/src/bin/serve.rscrates/cognitive-shader-driver/src/bindspace.rscrates/cognitive-shader-driver/src/codec_bridge.rscrates/cognitive-shader-driver/src/codec_kernel_cache.rscrates/cognitive-shader-driver/src/codec_research.rscrates/cognitive-shader-driver/src/cypher_bridge.rscrates/cognitive-shader-driver/src/decode_kernel.rscrates/cognitive-shader-driver/src/driver.rscrates/cognitive-shader-driver/src/engine_bridge.rscrates/cognitive-shader-driver/src/grpc.rscrates/cognitive-shader-driver/src/lib.rscrates/cognitive-shader-driver/src/mailbox_soa.rscrates/cognitive-shader-driver/src/planner_bridge.rscrates/cognitive-shader-driver/src/rotation_kernel.rscrates/cognitive-shader-driver/src/serve.rscrates/cognitive-shader-driver/src/sigma_rosetta.rscrates/cognitive-shader-driver/src/token_agreement.rscrates/cognitive-shader-driver/src/wire.rscrates/cognitive-shader-driver/tests/busdto_bridge_test.rscrates/cognitive-shader-driver/tests/end_to_end.rscrates/lance-graph-contract/src/hhtl.rscrates/lance-graph/src/graph/mod.rscrates/lance-graph/src/graph/scheduler.rscrates/lance-graph/src/graph/versioned.rscrates/surreal_container/Cargo.tomlcrates/surreal_container/src/lib.rscrates/surreal_container/src/view.rs
Comment on lines
+10
to
+18
| **Fixes applied (new commit on the same branch — preserves review history):** | ||
| - **FIX-A (P0):** restored the dropped enabler — `#[repr(transparent)]` + doc on `CausalEdge64` (`causal-edge/src/edge.rs:148-156`); added `const _` size/align guards at the `edges_raw`/`meta_raw` cast sites (compile-error on any layout regression); corrected both SAFETY comments. | ||
| - **FIX-B (P1):** ran `cargo fmt` on all 5 touched crates; `fmt --check` now exits 0. | ||
| - **FIX-C (P1, PP-15):** `SurrealMailboxView::from_columns` `debug_assert_eq!` → `assert_eq!` (the column-length invariant now fails loudly in ALL profiles — closes a release-build OOB where a ragged kv-lance projection → `n_rows() > entity_type().len()` → `SoaWavePrimer::project` indexes out of bounds). + `from_columns_rejects_ragged_projection` panic test. | ||
| - **FIX-D (P1, 4 agents):** `pub fn base_path()` on `VersionedGraph`; deleted the `format!("{:?}")` Debug-scrape in `scheduler.rs` (embedded-quote truncation hazard). | ||
| - **FIX-E (PP-13):** `test_edges_raw_meta_raw_reinterpret_round_trips` — the unsafe cast had ZERO coverage; now bit-exact round-trip + pointer-identity asserted. | ||
| - **FIX-F/H (P2 docs):** `hhtl` bijection doc `0..=16` → `1..=16` (prefix(0)=EMPTY is ancestor of nothing); `drive_at_latest` scope note (version-agnostic policies only) + `versions().last()` upstream-pagination caveat tying the ascending-sort assumption to the lance =7.0.0 pin. | ||
|
|
||
| **Disk verification this turn:** `git diff` confirms FIX-A landed (`#[repr(transparent)]` on `CausalEdge64` at `edge.rs:156`, `const _:` size/align guards at both cast sites in `mailbox_soa.rs`, `test_edges_raw_meta_raw_reinterpret_round_trips` at `mailbox_soa.rs:716`, `from_columns_rejects_ragged_projection` at `surreal_container/src/view.rs:257`). 34 files modified, +2841/-1100 — all uncommitted, awaits operator decision. |
There was a problem hiding this comment.
Clarify the commit/status wording.
This section says the fixes were applied in a new commit, but the disk-verification note then says the branch is “all uncommitted.” Those states conflict; please distinguish committed fixes from any remaining unstaged drift so the log reflects the actual repo state.
🤖 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/AGENT_LOG.md around lines 10 - 18, The AGENT_LOG.md file
contains conflicting statements about the repository state: the "Fixes applied"
section header claims the fixes are in a new commit, but the "Disk verification
this turn" note states all changes are uncommitted. Clarify the wording to
accurately reflect the actual state — either update the "Fixes applied" header
and descriptions to indicate these are staged/uncommitted changes awaiting
decision, or update the "Disk verification" note to confirm committed status.
Ensure both sections describe the same repository state consistently so readers
understand whether the 34 modified files with +2841/-1100 changes are committed
to the branch or still in the working directory.
Comment on lines
+11
to
+23
| **The find.** All three meet at exactly **one trait surface**: `lance_graph_contract::soa_view::MailboxSoaView`. Each landing made a DIFFERENT impl viable on the same boundary: | ||
| - **`MailboxSoA<N>` (cognitive)** — the in-process owner+view, so the Rubicon loop runs in-RAM (was only on `jolly`). | ||
| - **`SurrealMailboxView<'a>` (surreal-side view)** — D-PG-6's read glove, now buildable end-to-end via the fork's `kv-lance` backend. | ||
| - **`NiblePath::from_guid_prefix`** — the ontology-side keystone follow-up of #498's `classid → ReadMode` LazyLock: a deterministic 20→16 nibble fold that satisfies the routing-prefix `is_ancestor_of` invariant the LE contract names. | ||
|
|
||
| `LanceVersionScheduler` (D-MBX-9-IN core impl) sits one layer up and consumes ANY `V: MailboxSoaView` — so a single OUT-direction wrapper drives all three impls without case-splitting. The trait's read-only-by-design (`MailboxSoaView` has no mutator method) is the structural enforcement of `kanban.rs:1-21`'s "surreal=project-read-only, callcenter=commit" ruling; the SurrealQL adapter NOT importing `MailboxSoaOwner` is the compile-time tripwire if a future drift tries to mutate through the projection. | ||
|
|
||
| **Why this matters.** Four "still BLOCKED" rows from the most recent unblock-list synthesis (last sync turn) all collapse onto a SINGLE commit, because the substrate already had the shape — only three independent dep/code landings had to converge. The pattern: | ||
| - Substrate trait designed once + multiple implementors (no `Box<dyn>` in hot paths — generic `V: MailboxSoaView` everywhere). | ||
| - Read-only-by-trait-design = compile-time enforcement of the architectural ruling (no need for a runtime "you can't write through this" guard). | ||
| - A typed `BlockedColdBuild` error variant lets a heavy dep wire-up (surrealdb cold build) be deferred without breaking the contract-side adapter — the surface ships, the integrator flips it on in their branch. | ||
|
|
||
| **Lesson.** When three plans cite the same trait surface as their unblock dependency, the first session that lands ANY one of the implementors should ALSO ship the trait-impl shape for the others (even as a stub returning a typed error). This collapses N independent post-unblock follow-ups into 1 commit's worth of trait engineering. The cost is ~50 LOC of stub + a typed error variant; the benefit is N − 1 fewer post-merge commits per queued plan. |
There was a problem hiding this comment.
Separate the NiblePath work from the mailbox seam.
NiblePath::from_guid_prefix is not part of the MailboxSoaView/MailboxSoaOwner seam, so the current "one trait surface" framing overstates the coupling. As written, it reads like Task 3 collapses into the mailbox-view commit, which is not what the diff shows.
Suggested rewording
- **The find.** All three meet at exactly **one trait surface**: `lance_graph_contract::soa_view::MailboxSoaView`.
+ **The find.** The mailbox/view work converges on `lance_graph_contract::soa_view::MailboxSoaView`; `NiblePath::from_guid_prefix` is an adjacent routing follow-up, not part of that seam.Based on learnings: Task 3 adds NiblePath::{from_guid_prefix, prefix}, while Task 2 adds MailboxSoaView/MailboxSoaOwner and Task 4 adds SurrealMailboxView.
🤖 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/EPIPHANIES.md around lines 11 - 23, The current wording in the
"The find" section conflates NiblePath::from_guid_prefix with the MailboxSoaView
trait surface, overstating their coupling. NiblePath is not an implementation of
MailboxSoaView and is actually part of separate work. Reword the section to
clarify that the actual trait surface convergence point involves only
MailboxSoA<N> and SurrealMailboxView as implementations of
MailboxSoaView/MailboxSoaOwner, while NiblePath::{from_guid_prefix, prefix} is
separate work that should be called out as Task 3 independently adding that
ontology-side keystone. Restructure the bullet points so the three items listed
correspond to distinct task boundaries rather than all appearing to converge on
the single trait surface.
Comment on lines
+424
to
+429
| libet_offset_us: if from == KanbanColumn::Planning && to == KanbanColumn::CognitiveWork | ||
| { | ||
| -550_000 | ||
| } else { | ||
| 0 | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify current and contract-side Libet offset condition sites.
rg -n -C3 "libet_offset_us|CognitiveWork" crates/cognitive-shader-driver/src/mailbox_soa.rs crates/lance-graph-contract/src/soa_view.rsRepository: AdaWorldAPI/lance-graph
Length of output: 5433
Remove the from condition from libet_offset_us gate to match contract semantics.
The Libet offset should trigger whenever entering CognitiveWork, not only when entering from Planning. The contract-side implementation (lance-graph-contract/src/soa_view.rs:196) gates solely on to == KanbanColumn::CognitiveWork, but the driver gates on both from == KanbanColumn::Planning && to == KanbanColumn::CognitiveWork. If additional valid edges to CognitiveWork are introduced, the driver will fail to apply the offset, diverging from contract behavior.
Suggested fix
- libet_offset_us: if from == KanbanColumn::Planning && to == KanbanColumn::CognitiveWork
- {
+ libet_offset_us: if to == KanbanColumn::CognitiveWork {
-550_000
} else {
0
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| libet_offset_us: if from == KanbanColumn::Planning && to == KanbanColumn::CognitiveWork | |
| { | |
| -550_000 | |
| } else { | |
| 0 | |
| }, | |
| libet_offset_us: if to == KanbanColumn::CognitiveWork { | |
| -550_000 | |
| } else { | |
| 0 | |
| }, |
🤖 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 424 - 429,
The libet_offset_us assignment in the mailbox_soa.rs file gates on both the
source and destination columns, but the contract implementation only gates on
the destination column. Remove the from == KanbanColumn::Planning condition from
the if statement for libet_offset_us so that it only checks to ==
KanbanColumn::CognitiveWork. This ensures that the -550_000 offset is applied
whenever entering CognitiveWork from any column, matching the contract semantics
and preventing divergence when new edges to CognitiveWork are introduced.
Comment on lines
+75
to
+80
| /// Construct a `LanceVersionScheduler` with the canonical forward-arc | ||
| /// reference [`NextPhaseScheduler`]: every tick proposes | ||
| /// `Planning → CognitiveWork → Evaluation → Commit`, halting at the | ||
| /// absorbing column. The `-550 µs` Libet anchor stamps the | ||
| /// `Planning → CognitiveWork` crossing — the same convention | ||
| /// `MailboxSoa<N>::advance_phase` uses. |
There was a problem hiding this comment.
Fix Libet anchor unit in docs.
Line 78 states -550 µs, but the canonical move offset is -550_000 microseconds (-550 ms). Please align the doc text to prevent timing confusion.
Suggested patch
- /// absorbing column. The `-550 µs` Libet anchor stamps the
+ /// absorbing column. The `-550_000 µs` (`-550 ms`) Libet anchor stamps the📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /// Construct a `LanceVersionScheduler` with the canonical forward-arc | |
| /// reference [`NextPhaseScheduler`]: every tick proposes | |
| /// `Planning → CognitiveWork → Evaluation → Commit`, halting at the | |
| /// absorbing column. The `-550 µs` Libet anchor stamps the | |
| /// `Planning → CognitiveWork` crossing — the same convention | |
| /// `MailboxSoa<N>::advance_phase` uses. | |
| /// Construct a `LanceVersionScheduler` with the canonical forward-arc | |
| /// reference [`NextPhaseScheduler`]: every tick proposes | |
| /// `Planning → CognitiveWork → Evaluation → Commit`, halting at the | |
| /// absorbing column. The `-550_000 µs` (`-550 ms`) Libet anchor stamps the | |
| /// `Planning → CognitiveWork` crossing — the same convention | |
| /// `MailboxSoa<N>::advance_phase` uses. |
🤖 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/src/graph/scheduler.rs` around lines 75 - 80, The
documentation comment for the LanceVersionScheduler constructor contains an
incorrect unit specification for the Libet anchor. The comment currently states
the anchor as -550 µs (microseconds), but the actual canonical offset is -550
milliseconds (or -550_000 microseconds). Update the doc comment text to
correctly specify -550 ms instead of -550 µs to ensure accurate timing
documentation and prevent confusion.
Comment on lines
+142
to
+149
| /// The base directory or URI this graph is rooted at (local path, `s3://`, | ||
| /// `az://`, `gs://`). The per-dataset paths (`nodes.lance`, `edges.lance`, | ||
| /// …) are this suffixed by their dataset name. Exposed so downstream | ||
| /// crates (e.g. `graph::scheduler::LanceVersionScheduler`) can derive a | ||
| /// dataset path without scraping the `Debug` representation. | ||
| pub fn base_path(&self) -> &str { | ||
| &self.base_path | ||
| } |
There was a problem hiding this comment.
Add a focused unit test for the new public accessor.
base_path() is newly exposed API surface; please add a direct assertion for it in this module’s tests to lock the contract explicitly.
Suggested patch
#[test]
fn test_versioned_graph_paths() {
let g = VersionedGraph::local("/tmp/test-graph");
+ assert_eq!(g.base_path(), "/tmp/test-graph");
assert_eq!(g.nodes_path(), "/tmp/test-graph/nodes.lance");
assert_eq!(g.edges_path(), "/tmp/test-graph/edges.lance");
assert_eq!(g.fingerprints_path(), "/tmp/test-graph/fingerprints.lance");
}As per coding guidelines, crates/**/*.rs: “Add Rust unit tests alongside implementations via #[cfg(test)] modules; prefer focused scenarios over broad integration tests.”
🤖 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/src/graph/versioned.rs` around lines 142 - 149, The newly
exposed public method `base_path()` lacks a direct unit test to verify its
contract. Add a focused unit test in a `#[cfg(test)]` module within this file
that verifies the `base_path()` method returns the expected base path value. The
test should create a VersionedGraph instance with a known base path and assert
that calling `base_path()` returns that value correctly.
Source: Coding guidelines
Comment on lines
+218
to
+221
| pub async fn read_via_kv_lance( | ||
| _store: &crate::SurrealStore, | ||
| _mailbox_id: MailboxId, | ||
| ) -> Result<SurrealMailboxView<'static>, SurrealContainerError> { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify current signature and all call sites before changing lifetime bounds.
rg -n -C2 "pub async fn read_via_kv_lance|SurrealMailboxView<'static>" crates/surreal_container/src/view.rs
rg -n -C2 "read_via_kv_lance\\("Repository: AdaWorldAPI/lance-graph
Length of output: 2440
🏁 Script executed:
# Find SurrealMailboxView definition
rg -n "struct SurrealMailboxView|pub struct SurrealMailboxView" crates/surreal_container/Repository: AdaWorldAPI/lance-graph
Length of output: 143
🏁 Script executed:
# Get full context of read_via_kv_lance implementation
sed -n '216,240p' crates/surreal_container/src/view.rsRepository: AdaWorldAPI/lance-graph
Length of output: 1166
🏁 Script executed:
# Find all usages of read_via_kv_lance
rg -n "read_via_kv_lance" crates/Repository: AdaWorldAPI/lance-graph
Length of output: 1012
🏁 Script executed:
# Check module-level documentation for design intent
head -n 100 crates/surreal_container/src/view.rsRepository: AdaWorldAPI/lance-graph
Length of output: 4817
read_via_kv_lance return lifetime contradicts zero-copy design intent.
SurrealMailboxView<'a> is explicitly designed for borrowed slices over projection byte buffers (module docs lines 25-81), but the function signature returns SurrealMailboxView<'static>. This forces static lifetime constraints that would prevent the zero-copy implementation once the stub is filled in. Align the function signature with the struct design:
Fix
-pub async fn read_via_kv_lance(
- _store: &crate::SurrealStore,
+pub async fn read_via_kv_lance<'a>(
+ _store: &'a crate::SurrealStore,
_mailbox_id: MailboxId,
-) -> Result<SurrealMailboxView<'static>, SurrealContainerError> {
+) -> Result<SurrealMailboxView<'a>, SurrealContainerError> {🤖 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/surreal_container/src/view.rs` around lines 218 - 221, The function
`read_via_kv_lance` returns `SurrealMailboxView<'static>`, but this contradicts
the zero-copy design of the struct which is intended to work with borrowed
slices. Change the return type lifetime from `'static` to an appropriate
lifetime parameter (such as `'a`) that reflects the borrowed nature of the data
the view wraps. This aligns the function signature with the struct's design
intent for zero-copy borrowed data access.
AdaWorldAPI
pushed a commit
that referenced
this pull request
Jun 16, 2026
…D-CI-COVERAGE-MOLD-1) The `test` job has begun hitting the same disk/RSS link cliff already mitigated on `test-with-coverage` (b56bb2c): `ld terminated with signal 7 [Bus error]` + an LLVM crash at the `cargo test --no-run` link step of test_sql_query / intervene_counterfactual. Root cause is link-footprint growth, NOT a logic break (a layout break would fail an assertion, not SIGBUS at link). PR #507 (0c6ef02, +4055 lines across causal-edge ce64-v2 layout + cognitive-shader-driver MailboxSoaOwner / SurrealMailboxView) grew the integration-test object set enough to tip the previously-marginal `test`-job link over the same ceiling. It surfaced on the first full-workspace CI run after #507 (the intervening PRs are root-excluded crates, so their CI never linked the post-#507 tree). Fix: give the `test` job a job-level RUSTFLAGS with `-C debuginfo=0` (parity with the coverage job). debuginfo carries no value in CI (no debugger is attached); dropping it cut the coverage job's per-binary link ~930 MB -> ~252 MB (-73%, measured in b56bb2c) and relieves both the mold/GNU-ld RSS and the disk ceiling. mold is already installed on this job. Side effect: the job gets its own Swatinem cache key (first run repopulates). This is a fence (buys headroom), not a root reduction of #507's legitimate codegen — documented as such in the TD-CI-COVERAGE-MOLD-1 ledger addendum, including the secular-growth caveat and the separate (warns-not-fails) intervene_counterfactual.rs deprecated-API debt.
AdaWorldAPI
pushed a commit
that referenced
this pull request
Jun 16, 2026
#511) Addresses the open review findings on the merged #511: - examples/calibrate.rs: guard degenerate grids up front. `k = 24.min(m)` is 0 when m==0, so `m / k` panics (divide by zero); `eigenvector(1)` / `m - 1` also break on n<2. Exit cleanly with a message instead. (CodeRabbit Major) - src/hhtl.rs: basin_lambda2 asserts keys.len() == grid.n. A mismatched slice silently produced wrong basin groupings + lambda2; fail fast at the API boundary. (CodeRabbit Major) - .claude/board/TECH_DEBT.md: reflow the wrapped `#507` so it is not parsed as an ATX heading (markdownlint MD018). (CodeRabbit Minor) Already-fixed before merge (no change needed): Codex P2 — `infight` is 4-bit (columns.rs `spec("infight", 4, ...)`, the certified width per the §10 note). Deferred: the TECH_DEBT.md append-only addendum-placement nit (CodeRabbit Major) is a board-hygiene reorganization, left to a focused governance pass rather than churned inside a code-fix PR. Gates: fmt + clippy -D warnings + test green (75 lib tests) on the standalone perturbation-sim crate. https://claude.ai/code/session_016b33swuXE23hKtqxsHu9p1
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
Four tasks from the shortest-unblocking-path list surfaced after PR #497–#501 + the
AdaWorldAPI/surrealdbfork bump (lance/lance-index=7.0.0, lancedb=0.30.0, ndarray exact-rev). All four meet at the single contract traitMailboxSoaView, so the cascade closes in one commit (E-UNBLOCK-CASCADE-1).Each task was a named-but-unwritten consumer of contract scaffolding that already shipped — nothing here duplicates main (verified: none of the four symbols exist on
main, onjolly-cori-clnf9, or onwonderful-hawking-lodtql; the source cherry-pick commit463d71bis not an ancestor of main).The four tasks
3 —
NiblePath::{from_guid_prefix, prefix}(zero-dep, foundational)Ontology-side keystone follow-up of #498's
classid → ReadModeLE contract. The 20-nibbleclassid(8) | HEEL(4) | HIP(4) | TWIG(4)prefix overflows the 16-nibbleMAX_DEPTH; the deterministic fold drops the canon-reserved high u16 of classid (root-first packclassid_lo(4) | HEEL(4) | HIP(4) | TWIG(4)), returningNonewhen the fold would be lossy (callers never get a silent collision).prefix(d)is the O(1) single-shot ancestor view:prefix(d).is_ancestor_of(self)holds for everyd ≤ self.depth— the routing-cache view of a deeper class path. Solves identity-architecture v1 §3 P-SCOPE-CLASSIFY.crates/lance-graph-contract/src/hhtl.rs— +7 tests, contract lib 632 green2 —
impl MailboxSoaView + MailboxSoaOwner for MailboxSoA<N>Cherry-pick of
jolly-cori-clnf9::463d71b(integrated-cognitive-planner-v1 §2 Seam #3). Adds thepub phase: KanbanColumnfield + zero-copyrepr(transparent)slice impls (edges_raw/meta_raw) + the in-RAM Rubicon driving-loop test. The contract spine (#437/#439) now drives an actual loop end-to-end — no surreal, no ractor bus needed for the in-process case.crates/cognitive-shader-driver/src/mailbox_soa.rs— +1 driving-loop test, driver lib 86 green1 —
LanceVersionScheduleroverVersionedGraph::versions()D-MBX-9-IN core impl — the Lance-backed implementor the contract trait's own docstring names as unwritten (
scheduler.rs:20"lands in a buildable downstream crate"). Generic over the innerVersionScheduler<S = NextPhaseScheduler>; exposesdrive_once/drive_at_latest/current_dataset_version. ClosesE-SUBSTRATE-IS-THE-SCHEDULER's OUT-direction. Stays propose-not-dispose (R1): the returnedKanbanMoveis for the caller'sMailboxSoaOwner::try_advance_phaseto apply.crates/lance-graph/src/graph/scheduler.rs(new) — +5 tests, real on-disk tempdir Lance (no mocks)4 —
SurrealMailboxView(D-PG-6 contract slice)Read-only
MailboxSoaViewadapter a SurrealQL projection populates viafrom_columns(...)— pure zero-copy borrow over the kv-lance scan buffers. ImportsMailboxSoaViewbut NOTMailboxSoaOwner(compile-time enforcement ofkanban.rs:1-21"surreal=project-read-only, callcenter=commit").read_via_kv_lance()returns the new typedSurrealContainerError::BlockedColdBuilduntil the surrealdb fork dep inCargo.tomlis uncommented — kept off by default to spare contributors the ~10 min cold surrealdb build. The contract surface is available today; the integration is one Cargo.toml uncomment + a SurrealQL projection body.crates/surreal_container/src/view.rs(new) +lib.rs+Cargo.toml— +4 testsWhat this unblocks
BlockedColdBuildflip-onTests + lint
All clippy
-D warningsclean on the new files. Pre-existing lints inlance-graph-ontology/lance-graph-planner/ndarray_bridge.rsare out of session scope.Board hygiene (mandatory rule — same commit)
LATEST_STATE.md— Contract Inventory PREPEND for the new typesEPIPHANIES.md—E-UNBLOCK-CASCADE-1(three independent landings converge on one trait surface)AGENT_LOG.md— task-by-task entry with test counts11 files, +1,257 / −18. Rebased clean on
74e04cc(post #505/#506).https://claude.ai/code/session_01Xzyc27Nx3f8WC5KzwfWfjx
Generated by Claude Code
Summary by CodeRabbit
New Features
NiblePathrouting helpers (prefix(),from_guid_prefix())LanceVersionSchedulerfor graph version managementSurrealMailboxViewread-only adapter for mailbox dataVersionedGraph::base_path()accessorRefactor
Documentation