docs(pr-x12): canon follow-ups — R-7 path fix, R-13 primitives, R-14, R-15#198
Merged
Merged
Conversation
…R-14 / R-15) The substrate-binding doc pr-x12-cam-pq-sigker-dn-tree-substrate-bindings.md §6 proposed four canon updates that PR #197 marked "should be added to canon-resolutions-delta" but never actually applied. Closing that loop. 1. R-7 path correction (both canon files): tropical-GEMM kernel lives today at lance-graph::bgz17::scalar_sparse:: tropical_spmv, not in an abstract blasgraph namespace. The blasgraph name is the eventual abstraction (post-Plan-H); until then ndarray-codec depends on bgz17 directly. Cite the symbol, not the namespace, when wiring A6. 2. R-13 expansion (both canon files): Name the implementation primitives composed by the four policy modes (LocalEphemeral / SharedClusterWide / SharedRegional / PretrainedStatic): - cam_pq::CamCodebook for training (k-means + CAM-PQ) - bgz-tensor::Codebook4096 / bgz-hhtl-d for deployed encoding - dn_tree for online plastic updates (SharedClusterWide) - merkle_tree for integrity proof (Blake3-48-bit + xor_diff) - q2 (external) for gossip protocol PR-X12 contributes the wire format + CodebookHandle trait + Option A. No new substrate code required. 3. R-14 (NEW, both canon files): Formal-correctness layer via lance-graph::jc pillars: - Pillar 10 (Pflug-Pichler, jc::pflug): nested-distance Lipschitz on Sigma DN-trees proves CAM-PQ tree quantization preserves FreeEnergy within Lε. Active in default zero-dep build. - Pillar 11 (Hambly-Lyons, jc::hambly_lyons): signature uniqueness on tree-quotient. Active under --features hambly-lyons (PR #348, 2026-05-07); probe passes (forward<1e-9, converse>0.05, ratio≥1e6). R-4 quality floors inherit Pillar 10's Lipschitz bound. R-15 gates on Pillar 11. PR #350 corrects sigker::signature_kernel_pde's Goursat-PDE math bug; until then Pillar 11 uses signature_truncated. 4. R-15 (NEW, both canon files): SignatureBasis<const DEPTH: usize>: Basis<f32> as fifth concrete Basis<T> impl alongside DctIIBasis (video), EwaSplatBasis (3DGS), ShSpectralBasis (splat SH), HadamardBasis. Wraps sigker::signature_truncated (tensor-algebra, correct today) — NOT signature_kernel_pde (buggy until PR #350). Plan G gets a fifth lane: stream signal (audio waveform / time-series / gesture / handwriting). Quality floor inherits from Pillar 11 (R-14). Compression target ~10× over raw f32 path samples (calibrate during Plan G). Also updated the compaction-preservation contract, falsifiability matrix (+3 rows for R-14/R-15), and §0 to reflect R-1..R-15 inclusive. https://claude.ai/code/session_01HbqooFZHAjaUtFEzhA1R2u
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR appends two new canon resolutions (R-14 and R-15) to the substrate framework, updates the tropical-GEMM kernel location in R-7 from abstract to concrete, extends R-13 implementation primitives documentation, and updates falsifiability matrices and preservation checklists across both the delta overview and main canon document. ChangesPR-X12 Substrate Canon Resolutions R-14/R-15 Expansion
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 unit tests (beta)✅ Unit Tests committed locally.
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: 8415a627ea
ℹ️ 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".
| | R-13 (Option A per-shard) | this doc | Plan F on BERT-glue | 8× compression + accuracy | Holds; else escalate to Option B | | ||
| | R-14 (Pillar 10 active) | this doc | `cargo test -p jc` (default features) | Pflug-Pichler Lipschitz bound | Pillar 10 probe green | | ||
| | R-14 (Pillar 11 active) | this doc | `cargo test -p jc --features hambly-lyons` | Signature uniqueness probe | forward < 1e-9, converse > 0.05, ratio ≥ 1e6 | | ||
| | R-15 (SignatureBasis lane) | this doc | Plan G stream-signal lane | path reconstruction under Pillar 11 | ε within discrimination ratio of Pillar 11 | |
There was a problem hiding this comment.
This row defines R-15 success as path reconstruction under Pillar 11, but the same R-15 section declares invert as unimplemented! because signature-to-path is many-to-one up to tree-like equivalence (invert marked N/A). As written, the falsifiability gate is not operationally testable with the committed API contract, so downstream Plan G gating can become ambiguous or blocked. The metric should be reframed to something executable (e.g., reduced-path equivalence or signature-space fidelity) instead of direct path reconstruction.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.claude/knowledge/pr-x12-canon-resolutions-delta.md (1)
359-417:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate stale counts in the summary/compaction sections to match R-14/R-15 additions.
The document now introduces six categories and R-1..R-15, but this block still says “24 rows,” “Thirteen R-resolutions,” and “five-category architecture.” That creates internal canon drift.
Suggested doc fix
-24 rows mapping every M:H-N and R-N to (test, metric, pass condition). Plan G's bench harness emits a JSON report; merge job for Phase 2 consumer PRs reads it and gates pass-fail. +27 rows mapping every M:H-N and R-N to (test, metric, pass condition). Plan G's bench harness emits a JSON report; merge job for Phase 2 consumer PRs reads it and gates pass-fail. ... -2. **Thirteen R-resolutions** with one-line summaries +2. **Fifteen R-resolutions** with one-line summaries ... -4. **The five-category architecture** including `ndarray-codec` +4. **The six-category architecture** including `ndarray-codec` ... -Citation IDs (R-1..R-13) stable. +Citation IDs (R-1..R-15) stable.🤖 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/knowledge/pr-x12-canon-resolutions-delta.md around lines 359 - 417, Update the stale numeric summaries in the falsifiability and compaction sections to reflect the added R-14/R-15 and the new sixth category: change "24 rows" to "26 rows" in the falsifiability matrix header (§9), change "Thirteen R-resolutions" to "Fifteen R-resolutions" in the compaction-preservation contract (§10) item 2, and change "five-category architecture" to "six-category architecture" in item 4 (and anywhere else mentioning five categories); ensure the canonical ranges now reference R-1..R-15 and note six categories in the compact list so counts and citation ranges (R-1..R-15, M:*, A:*, B:*) are consistent.
🤖 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/knowledge/pr-x12-substrate-canon-resolutions.md:
- Around line 1248-1250: The R-15 matrix row conflicts with the non-invertible
SignatureBasis contract: replace the path-reconstruction/ε requirement in the
"R-15 (SignatureBasis lane) | Plan G stream-signal lane | path reconstruction
under Pillar 11 | ε within discrimination ratio of Pillar 11" row with a
non-invertibility-aware criterion—either mark inversion/path-reconstruction as
N/A for SignatureBasis or change the gate to verify forward-mapping consistency
(e.g., deterministic signing/encoding invariants under Pillar 11 or a
non-invertibility assertion) so the row aligns with the SignatureBasis contract
semantics.
---
Outside diff comments:
In @.claude/knowledge/pr-x12-canon-resolutions-delta.md:
- Around line 359-417: Update the stale numeric summaries in the falsifiability
and compaction sections to reflect the added R-14/R-15 and the new sixth
category: change "24 rows" to "26 rows" in the falsifiability matrix header
(§9), change "Thirteen R-resolutions" to "Fifteen R-resolutions" in the
compaction-preservation contract (§10) item 2, and change "five-category
architecture" to "six-category architecture" in item 4 (and anywhere else
mentioning five categories); ensure the canonical ranges now reference R-1..R-15
and note six categories in the compact list so counts and citation ranges
(R-1..R-15, M:*, A:*, B:*) are consistent.
🪄 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: 148f12f4-408d-4f11-9f00-13108ad3bead
📒 Files selected for processing (2)
.claude/knowledge/pr-x12-canon-resolutions-delta.md.claude/knowledge/pr-x12-substrate-canon-resolutions.md
Per other-session feedback: six nitpick-level findings on PR #197 didn't block the merge but stayed unaddressed. Folding them into this PR. 1. GGUF Escape forward-ref to F-4 gguf-llm-weights-encoding.md §2.4 said "Escape must be lossless ... This is an additional R-N candidate" with no pointer. F-4 in §10 already explains the mechanism (rANS bypass channel in A8, HEVC escape-coefficient precedent). Added an inline cross-ref so readers don't have to scroll to find the resolution. 2. Phone-class viability overclaim re KV cache gguf-llm-weights-encoding.md line 269 claimed "7B at PR-X12 is genuinely runnable on a phone-class device". Weight compression alone takes 7B from 4 GB to 3 GB, but KV cache at 8K context is ~1-2 GB independent of weight compression. Qualified the claim: PR-X12 weights are necessary but not sufficient; KV-cache lane (Plan D, M:H-3, R-4) is the second lever for full phone viability. 3. EncodingDomain::LLMWeights timing §11 implication #2 says "LLM lane lands post-PR-X12, but the harness must be lane-extensible"; implication #5 said "Reserve an EncodingDomain::LLMWeights discriminant ... now". Clarified: PR-X12 reserves the enum-discriminant *slot* now (forward-compat lock); the LLMWeights variant + decoder land post-PR-X12 without a wire-format break. 4. Per-arch `match Arch::CURRENT` const-eval woa-multiarch-orchestration.md §3.3's `const DCT_BATCH_CROSSOVER = match Arch::CURRENT { ... }` does not compile under stable Rust const-eval — `Arch::CURRENT` would need to be a const, and architecture-conditional const matches require build-script-emitted integers or `cfg!(target_feature = ...)`. Rewrote as pseudocode pointing at a `build.rs` mechanism (decision matrix → `OUT_DIR` generated const) with a `cfg!()` fallback shape. 5. G-8 / G-9 numbering collision cam-pq-sigker-dn-tree-substrate-bindings.md §5 labelled bgz-jc's two prior gaps as G-8 / G-9 (continuing cam-pq's own G-1..G-7), but bgz-jc-substrate-synergies.md §5 didn't use any G-N IDs, so the cross-doc reference was dangling and the namespace was implicitly shared without rules. Gave bgz-jc §5.1 / §5.2 explicit IDs G-1 / G-2 (canonical to that doc) and updated cam-pq to cite them as "bgz-jc G-1" / "bgz-jc G-2" with a namespace-isolation note. 6. "landed" terminology in x266 §8 prerequisites table The status column claimed "landed" / "landed in concept" for R-1 trait shape, R-2 header bytes, and R-13 federated codebook policy. None of these have shipping code — they are canon-fixed (the resolution doc commits the design) but implementation is scheduled in Plan A4 / A8 / F. Renamed status to "canon-fixed" + a glossary line distinguishing "canon-fixed" (doc commitment) from "scheduled" (plan card exists) from shipping code. https://claude.ai/code/session_01HbqooFZHAjaUtFEzhA1R2u
Contributor
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 @.claude/knowledge/pr-x12-woa-multiarch-orchestration.md:
- Line 133: Reword the build.rs explanation to clarify host-vs-target semantics:
state that build.rs runs on the host so it cannot probe the target CPU, and that
the integer crossover constant is derived from target compile-time metadata
(e.g., CARGO_CFG_*, the target triple, declared target_feature/target
configuration or cfgs resolved at compile time) and then emitted into OUT_DIR
for use by the crate; remove or rephrase any wording that implies build.rs
performs a runtime or host CPU probe to detect the target microarchitecture
(also update mentions around Arch::CURRENT to indicate it's resolved from
compile-time metadata rather than host probing).
🪄 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: dfef2cd2-89f7-4757-b4fb-e8234dddf703
📒 Files selected for processing (5)
.claude/knowledge/pr-x12-bgz-jc-substrate-synergies.md.claude/knowledge/pr-x12-cam-pq-sigker-dn-tree-substrate-bindings.md.claude/knowledge/pr-x12-gguf-llm-weights-encoding.md.claude/knowledge/pr-x12-woa-multiarch-orchestration.md.claude/knowledge/pr-x12-x266-3dgs-spacetime-upscaling.md
✅ Files skipped from review due to trivial changes (2)
- .claude/knowledge/pr-x12-bgz-jc-substrate-synergies.md
- .claude/knowledge/pr-x12-gguf-llm-weights-encoding.md
Two open review threads on the second commit:
1. R-15 falsifiability row (substrate-canon-resolutions.md §9, line 1250)
Both codex-connector and CodeRabbit flagged that the matrix row
required "path reconstruction under Pillar 11" but R-15 itself marks
invert() as unimplemented! (path-from-signature is many-to-one up
to tree-like equivalence — Hambly-Lyons). The gate was operationally
untestable.
Reframed per CR's suggestion: "signature-space discrimination under
Pillar 11 (forward-only — path inversion is N/A per R-15)" with the
same probe criteria Pillar 11 uses (forward < 1e-9, converse > 0.05,
discrimination ratio >= 1e6, or agreed DEPTH-specific floor).
2. build.rs host-vs-target semantics (woa-multiarch §3.3, line 133)
CR pointed out that under cross-compilation Cargo runs build.rs on
the HOST, so any "feature-detection probe" in build.rs reflects the
build machine, not the target. My original wording ("target_arch /
target_feature cfgs + a feature-detection probe") implied host CPU
probing — wrong.
Rewrote the pseudocode + surrounding text to explicitly use
CARGO_CFG_TARGET_ARCH, CARGO_CFG_TARGET_FEATURE, target triple, and
pre-recorded calibration artifacts. Added an explicit "do NOT probe
the host CPU inside build.rs" warning citing Cargo's docs. The
in-crate cfg!() fallback shape is still correct (cfg! in normal
code reflects target cfgs); only build.rs's cfg!/#[cfg] reflects
the host.
https://claude.ai/code/session_01HbqooFZHAjaUtFEzhA1R2u
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.claude/knowledge/pr-x12-substrate-canon-resolutions.md (1)
1067-1067:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the R-15 falsification criterion to align with the matrix row and non-invertible contract.
Line 1067 says "reconstruct the path within ε" but this contradicts:
- Lines 1042-1046:
invertis explicitlyunimplemented!with rationale "path unique only up to tree-like equivalence"- Line 1250: The corrected matrix row clarifies "path inversion is N/A per R-15"
- R-14 Pillar 11: paths are determined only "up to tree-like equivalence" (not exact reconstruction)
The falsification criterion should use the same "signature-space discrimination" terminology as the corrected matrix row.
📝 Suggested alignment with matrix row
-**Falsifies if.** `SignatureBasis<DEPTH=3>` plus rANS fails to -reconstruct the path within ε under Pillar 11's discrimination ratio. -At that point, raise DEPTH or fall back to per-block DCT-II for the -stream lane. +**Falsifies if.** `SignatureBasis<DEPTH=3>` plus rANS fails the +signature-space discrimination criteria under Pillar 11 (forward < 1e-9, +converse > 0.05, ratio ≥ 1e6). At that point, raise DEPTH or fall back +to per-block DCT-II for the stream lane.🤖 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/knowledge/pr-x12-substrate-canon-resolutions.md at line 1067, Update the R-15 falsification criterion text to remove the phrase "reconstruct the path within ε" and replace it with language consistent with the matrix row and Pillar 11: state that falsification operates in the signature-space discrimination (i.e., distinguishing path signatures up to tree-like equivalence) rather than exact path inversion; also reference that the `invert` operation is `unimplemented!` and that path inversion is N/A per R-15, so the criterion should require distinguishing signature-equivalence classes rather than reconstructing concrete paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In @.claude/knowledge/pr-x12-substrate-canon-resolutions.md:
- Line 1067: Update the R-15 falsification criterion text to remove the phrase
"reconstruct the path within ε" and replace it with language consistent with the
matrix row and Pillar 11: state that falsification operates in the
signature-space discrimination (i.e., distinguishing path signatures up to
tree-like equivalence) rather than exact path inversion; also reference that the
`invert` operation is `unimplemented!` and that path inversion is N/A per R-15,
so the criterion should require distinguishing signature-equivalence classes
rather than reconstructing concrete paths.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a226107a-152d-4fa4-ad3b-c969b9764b21
📒 Files selected for processing (2)
.claude/knowledge/pr-x12-substrate-canon-resolutions.md.claude/knowledge/pr-x12-woa-multiarch-orchestration.md
✅ Files skipped from review due to trivial changes (1)
- .claude/knowledge/pr-x12-woa-multiarch-orchestration.md
The previous §3 violated the project's W1a polyfill contract by
describing runtime CPU dispatch as if it were the substrate's design:
- §3.1 listed multiple #[target_feature(enable = ...)] fns
coexisting in one mod, framed as "compile-time" but read like
"available to select from at runtime".
- §3.2 had an HwCaps OnceLock + caps.has_amx / has_vnni / has_sve2
if-else chain with `unsafe { batched_gemm_amx(...) }` runtime
branches. This is exactly the pattern CLAUDE.md and the W1a
consumer contract forbid.
- §3.3 framed Arch::CURRENT + build.rs as one option among several
runtime/host-probe approaches, which kept the "branching is OK"
mental model alive even after the host-vs-target fix.
Rewritten to match the actual project pattern (per CLAUDE.md's
Repository Structure + W1a contract):
- "All dispatch is polyfill." cfg(target_feature = ...) selects
exactly one backend file (simd_avx512.rs / simd_neon.rs /
simd_scalar.rs) to compile in. No runtime branching.
- Target CPU is fixed at build time via .cargo/config.toml's
target-cpu = x86-64-v4 (AVX-512 mandatory on x86_64) or via the
target triple for non-x86 builds. WoA fleet ships per-arch
binaries, not a fat binary that probes.
- R-5's per-arch crossover constant is also part of the polyfill:
one const per backend file, cfg-selected. build.rs may emit a
refined override into OUT_DIR (compile-time, target-config-driven,
never host CPU probing) but the selection mechanism is still cfg.
- Added explicit negative-rule sentence at the top of §3 listing
forbidden patterns: no HwCaps / CpuCaps runtime branching, no
`if has_avx512 else ...` dispatch, no `unsafe { runtime_branch }`.
The previous wording was hallucinated branching that contradicted the
substrate's actual design. The substrate ships ONE path per binary;
the cfg selects which path at build time.
https://claude.ai/code/session_01HbqooFZHAjaUtFEzhA1R2u
Per the architecture clarification: AMX/AVX-512/NEON/SVE2 intrinsics
and asm are backend-layer implementation details. The polyfill
(ndarray::simd::*, ndarray::hpc::*) is the consumer-facing surface.
When the codec body writes encoding code (Skip/Merge/Delta/Escape,
basin lookups, tropical-GEMM RDO, rANS, EWA splat), it is a consumer
of its own polyfill — same as burn, candle, lance-graph, surrealdb,
WoA. The codec does not know it is on AMX. It does not name a backend
symbol. It does not branch on architecture.
Three-layer diagram added at §3 making the boundary explicit:
Consumers (codec + downstream)
↓ same Rust API everywhere
Polyfill surface (src/simd.rs cfg-selected re-exports)
↓ cfg substitutes ONE backend file
Backend (simd_avx512.rs / simd_neon.rs / simd_scalar.rs)
— AMX bytecode, AVX-512 asm, NEON intrinsics live HERE
— and only here. Consumers never reach in.
Also added the escape hatch as documented: very-hot inner loops MAY
drop below the polyfill into a backend-specific intrinsic, but only
inside src/simd_<arch>.rs itself, cfg-gated, parity-tested against
the other backends, with `// SAFETY:` + sentinel-qa audit per
CLAUDE.md. It is the exception, not the model. No consumer crate
(codec body included) is ever the right place for it.
Cleaned up "dispatch" terminology across §0, §1, §2, §7.4, §8.1, §9:
the word was leaking the runtime-branching frame into compile-time-
only contexts. Reserved "dispatch" for async task scheduling (WoA's
job) and for the explicit polyfill prohibition statement; everywhere
else uses "polyfill" or "backend selection" to keep the compile-time
nature unambiguous. Reducer::dispatch_target speculation renamed to
backend_target with a "still cfg-selected, not runtime-branched"
qualifier.
Per-arch code lives once, inside src/simd_<arch>.rs, behind the
polyfill surface. The WoA fleet ships per-arch binaries. One build,
one backend, one path.
https://claude.ai/code/session_01HbqooFZHAjaUtFEzhA1R2u
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.claude/knowledge/pr-x12-woa-multiarch-orchestration.md (1)
30-33: ⚡ Quick winAlign tropical-GEMM naming with the canonical R-7 symbol.
Line 32 reintroduces
lance-graph::blasgraphin a changed block. To avoid cross-doc drift, use the same concrete symbol naming used by the canon updates (lance-graph::bgz17::scalar_sparse::tropical_spmv).Proposed doc edit
-- `lance-graph::blasgraph` graph queries (tropical-GEMM: same Rust, per-arch binary) +- `lance-graph::bgz17::scalar_sparse::tropical_spmv` graph queries (tropical-GEMM: same Rust, per-arch binary)🤖 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/knowledge/pr-x12-woa-multiarch-orchestration.md around lines 30 - 33, Replace the generic lance-graph::blasgraph symbol with the canonical R-7 symbol used elsewhere: lance-graph::bgz17::scalar_sparse::tropical_spmv; update the line that currently lists "lance-graph::blasgraph graph queries (tropical-GEMM: same Rust, per-arch binary)" to reference lance-graph::bgz17::scalar_sparse::tropical_spmv and adjust the parenthetical to reflect "tropical_spmv" so the documentation matches the canonical symbol.
🤖 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.
Nitpick comments:
In @.claude/knowledge/pr-x12-woa-multiarch-orchestration.md:
- Around line 30-33: Replace the generic lance-graph::blasgraph symbol with the
canonical R-7 symbol used elsewhere:
lance-graph::bgz17::scalar_sparse::tropical_spmv; update the line that currently
lists "lance-graph::blasgraph graph queries (tropical-GEMM: same Rust, per-arch
binary)" to reference lance-graph::bgz17::scalar_sparse::tropical_spmv and
adjust the parenthetical to reflect "tropical_spmv" so the documentation matches
the canonical symbol.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: cb58d7fa-2f2c-4cc8-9c13-52e544901c89
📒 Files selected for processing (1)
.claude/knowledge/pr-x12-woa-multiarch-orchestration.md
Contributor
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
4 similar comments
Contributor
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
Contributor
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
Contributor
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
Contributor
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
Contributor
|
✅ Unit tests committed locally. Commit: |
Contributor
|
✅ Unit tests committed locally. Commit: |
Contributor
|
✅ Created PR with unit tests: #199 |
Contributor
|
✅ Unit tests committed locally. Commit: |
Contributor
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
Contributor
|
No files have been changed in this PR. Unable to generate unit tests. |
Contributor
|
✅ Unit tests committed locally. Commit: |
This was referenced May 22, 2026
Owner
Author
|
2 tasks
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.
Why this PR exists
PR #197 introduced
pr-x12-substrate-canon-resolutions.md(R-1 through R-13) and the substrate-binding docpr-x12-cam-pq-sigker-dn-tree-substrate-bindings.md. The substrate-binding doc's §6 listed four canon updates that should follow — and PR #197 even annotated one of them as "Addressed in commit f1d68e6" — but the actual canon edits were never applied. The CodeRabbit "Addressed" marker was about the API-signature corrections in the cam-pq doc itself, not about propagating the §6 proposals to the canon files.This PR closes that loop. Pure docs; no source-code changes.
The four updates
1. R-7 — kernel home correction (both canon files)
R-7currently citeslance-graph::blasgraphas the home of the tropical-GEMM kernel. The actual kernel lives atlance-graph::bgz17::scalar_sparse::tropical_spmvtoday — theblasgraphnamespace is the eventual abstraction (post-Plan-H). Until that lands,ndarray-codecdepends onbgz17directly. The canon now names the symbol, not the namespace, so anyone wiring Plan A6 cites the right path.2. R-13 — federated-codebook primitives (both canon files)
R-13 currently commits to Option A (per-shard codebook) for Plan F v1 but doesn't name the substrate. The cam-pq doc proposes naming the implementation pieces; that's now in canon:
ndarray::hpc::cam_pq::CamCodebooklance-graph::bgz-tensor::Codebook4096/bgz-hhtl-dSharedClusterWide)ndarray::hpc::dn_treendarray::hpc::merkle_treeq2(external)PR-X12 contributes the wire format +
CodebookHandletrait + Option A. The primitives already exist; no new substrate code. (R-13 primitives verified real — audit #21.)3. R-14 — Formal correctness via
lance-graph::jcpillars (NEW)Two formal proofs the codec inherits without re-proving:
jc::pflug, Pflug-Pichler): nested-distance Lipschitz on Sigma DN-trees → CAM-PQ tree quantization preserves FreeEnergy within Lε. Active in default zero-dep build. R-4's quality floors inherit this bound.jc::hambly_lyons, Hambly-Lyons 2010 Annals 171(1), arXiv:math/0507536): signature uniqueness on tree-quotient. Active under--features hambly-lyons(since PR #348, 2026-05-07); probe passes (forward < 1e-9, converse > 0.05, discrimination ratio ≥ 1e6 over N=100 random pairs in d=3 at depth-2). R-15's signature lane gates on this. (Module refs verified real — audit feat(hpc): Session C — bgz17 dual-path integration bridge #22; the specific probe numerics here are unverified per audit §6 — confirm againsthambly_lyons.rsbefore relying on them.)Open work (Gap G-4 in the substrate-binding doc): PR #350 correctssigker::signature_kernel_pde's Goursat-PDE math bug; until then Pillar 11 deliberately usessignature_truncated(tensor-algebra path).4. R-15 —
SignatureBasis<const DEPTH: usize>: Basis<f32>(NEW)Fifth concrete
Basis<T>impl alongsideDctIIBasis(video),EwaSplatBasis(3DGS),ShSpectralBasis<L>(splat SH),HadamardBasis<N>. Wrapssigker::signature_truncated(real, verified — audit #23)— NOT.signature_kernel_pde(buggy until PR #350 lands)Plan G gets a fifth lane: stream signal — audio waveform / time-series / gesture / handwriting. Quality floor inherits from Pillar 11 (R-14). Compression target ~10× over raw f32 path samples (calibrate during Plan G). Cost: ~1 week wrapper.
Companion edits
§0of both canon files bumped from R-1..R-13 to R-1..R-15 with the post-merge framingTest plan
.claude/knowledge/additionsReviewer: confirm R-15's choice of— moot; the PDE form is not buggy (audit feat: port learning ops, project_all, bf16_from_projections from rust… #9). The choice is a design preference, not a correctness gate.signature_truncatedoversignature_kernel_pde(correct math today vs deferred-until-PR-#350-lands)--features hambly-lyonsbecomes default before promoting to canon)Generated by Claude Code
Summary by CodeRabbit
[[Review Change Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/AdaWorldAPI/ndarray/pull/198?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack)