Skip to content

docs(codec): PR-X12 substrate merged-canon — session A + B synthesis#196

Merged
AdaWorldAPI merged 7 commits into
masterfrom
claude/continue-ndarray-x0Oaw
May 22, 2026
Merged

docs(codec): PR-X12 substrate merged-canon — session A + B synthesis#196
AdaWorldAPI merged 7 commits into
masterfrom
claude/continue-ndarray-x0Oaw

Conversation

@AdaWorldAPI
Copy link
Copy Markdown
Owner

@AdaWorldAPI AdaWorldAPI commented May 22, 2026

Summary

Pure-docs PR (no code changes). Lands two architecture docs on top of master:

  1. pr-x12-codec-cognitive-substrate-mapping.md (+576 lines) — session A canonical (this thread, opus 4.7). Cross-stack mapping of PR-X12 codec primitives against four loads: x265 / 3D Gaussian splat / cognitive shaders / blasgraph-MKL gradient compression. 15 numbered epiphanies (E-1..E-15), 7 holy-grail claims (H-1..H-7), 23 technical-debt items (T-1..T-23), integration plan per sub-card (A4 / A6 / A7 / A8).

  2. pr-x12-substrate-merged-canon.md (+621 lines) — merged synthesis. Consolidates session A (this branch) and session B (pr-x12-cross-domain-synergies.md, already on master via PR feat(codec): PR-X12 A2 mode bit-pack + A3-intra prediction kernel #195 commit 01c77ccc). Includes 10 merge-only epiphanies (M:E-A..M:E-J), 2 new holy-grail claims (M:H-NEW-1, M:H-NEW-2), 3 new integration plans (Plan G bench harness, Plan H ndarray-codec crate extraction, Plan I PredictiveSignal trait), and resolves 4 disagreements between A and B.

  3. .claude/board/AGENT_LOG.md (+73 lines) — session-log entries documenting the architecture sessions.

The doc family (read order)

The merged-canon's §9 names the canonical read order:

# Path Role On this branch
1 .claude/knowledge/pr-x12-codec-x265-design.md Mechanical spec (companion, untouched) master
2 .claude/knowledge/pr-x12-substrate-merged-canon.md Read first — the merged canonical view this PR
3 .claude/knowledge/pr-x12-codec-cognitive-substrate-mapping.md Session A original (read for A's framing of specific claims) this PR
4 .claude/knowledge/pr-x12-cross-domain-synergies.md Session B original (read for B's framing of specific claims) master (PR #195)

The architectural claim (testable)

PR-X12 is the universal predictive-coder substrate factored into three traits:

trait PredictiveSignal           // bit-pattern carrier (LeafCu, Fingerprint, splat-tuple)
trait LinearReduce<Basis>        // universal kernel: B @ src  (alpha / rANS / sum / softmax)
trait CurveOrder<const N>        // dimension-agnostic scan (Morton/raster, splat-3d/ctu-2d)

Net density claim (M:H-NEW-2): ~1.5 KLoC generic codec glue + ~200 LoC per domain consumer compresses video / Gaussian splats / LLM gradients / KV-cache through one Lance-backed wire format. The bench harness (Plan G, 2 weeks) is the falsifiability test — Plan G must ship before A7 rANS lands.

Reviewer invitation

This PR is here to attract scrutiny from multiple reviewers in parallel:

  • CodeRabbit — please scan for: (1) unsupported claims (every H-N / M:H-NEW-N should reduce to a buildable bench or be flagged speculative); (2) broken file:line references — the docs cite e.g. src/hpc/codec/mode.rs:194, src/hpc/codec/predict.rs:*, src/aabb.rs, src/palette_codec.rs; (3) citation-ID drift between the three docs (A:E-N vs B:E-N vs M:E-X must be 1-to-1 stable); (4) any new PR feat(codec): PR-X12 A2 mode bit-pack + A3-intra prediction kernel #195-style invariants (BASIN_NONE collision, non-bijective pack) that the docs miss for the four cross-domain consumers.
  • Codex / human reviewers — please challenge the four resolved disagreements in §8 of the merged canon (critical path A7-first vs A4-first; block size 64×64 vs 16×16 via Ctu<const N>; lossy Escape as feature vs debt; cross-repo dep direction via crate extraction). The reasoning is in §8 with named winners; counter-arguments welcome.
  • The session-B parallel thread — the merged-canon claims to preserve all of B's content (B:E-1..B:E-N, B:HG, B:D-STACK-*) and adds resolved disagreements. If anything from pr-x12-cross-domain-synergies.md got dropped or mis-cited, flag it.

Test plan

  • cargo check --features codec --lib — clean post-merge with master
  • Citation IDs stable across edits (A:* and B:* numbering preserved from originals; M:* introduced fresh)
  • All file:line refs in the docs point at paths that exist on this branch after the master merge
  • No src/ changes; pure .claude/ additions
  • Reviewer: confirm M:H-NEW-1 (single Rust binary, 4 domains → one Lance column) is buildable as stated, or flag the missing piece
  • Reviewer: confirm the critical-path resolution in §5 (Plan G → A7 → parallelise B/A4-impl/A6/A8/C) is the right gating order
  • Reviewer: confirm Plan H (ndarray-codec crate extraction) doesn't break PR feat(codec): PR-X12 A2 mode bit-pack + A3-intra prediction kernel #195's src/hpc/codec/ module path during transition

This PR is intentionally a docs-only landing so the architectural claims can be reviewed independently of any code change. No src/ mutation; the codec code is exactly what PR #195 shipped.


Generated by Claude Code

Summary by CodeRabbit

  • Documentation
    • Added a session log and refreshed multiple architecture documents: clarified claims/status, tightened technical guidance, expanded verification steps, updated integration plans, and merged prior sessions into a canonical specification with navigation and debt tracking.
  • Bug Fixes / Changes
    • Enabled the full 12‑bit basin/index range (removed reserved sentinel).
    • Removed a small public helper from the API surface and adjusted tests accordingly.

Review Change Stack

claude added 3 commits May 22, 2026 07:32
Lands `.claude/knowledge/pr-x12-codec-cognitive-substrate-mapping.md`
(~576 lines) as the survives-compaction companion to the as-shipped
`pr-x12-codec-x265-design.md`. Generalises the codec spec across the
four industries it implicitly unifies (HEVC video, Gaussian splat,
cognitive shaders / sparse attention, gradient compression for LLM
training).

# Why this doc

Triggered by the PR #195 review session. PR-X12's mode taxonomy
(Skip/Merge/Delta/Escape) shows up under different names in four
disconnected research communities; each treats its own corner as
the central knob (HEVC RDO, ZeRO buckets, splat sparsity reg,
attention pruning). Without explicit naming of the unification,
future agents will rediscover each corner independently and
reimplement what the codec already provides.

# Structure (citable by section number)

- §0 — the big claim (PR-X12 is the gradient-quantisation substrate
  GenAI training has been missing for two years)
- §1 — four-axis mapping table (x265 / splat / cognitive / gradient)
- §2-§7 — deep mappings (mode taxonomy, CTU quad-tree, palette/basin
  codebook, transform basis, rANS, λ-RDO)
- §8 — 15 numbered epiphanies (E-1..E-15)
- §9 — 7 holy grail claims (H-1..H-7)
- §10 — integration plan per sub-card (A4/A6/A7/A8) + 3 new PRs
  (splat, cognitive, gradient consumers)
- §11 — exploration paths ranked by confidence (15 entries)
- §12 — technical debt inventory (23 items T-1..T-23 across codec,
  ndarray substrate, lance-graph cognitive, cross-repo, PR #195)
- §13 — 6 blasgraph/MKL synergies the HEVC team couldn't reach in
  2013 (block-matched ME, batched DCT, partition tree as tropical-
  GEMM, CABAC replacement, learned deblocking, k-means at frame rate)
- §14 — cross-references (design docs, rules, code paths)
- §15 — how to use this doc (read order per use case)

# Holy grail claims (load-bearing, citable)

- H-1: PR-X12 + cam_pq is the HEVC SCC codec 2013 hardware couldn't
  build (4096-entry codebook at 60 fps)
- H-2: The transform IS the optimiser (DCT-II ↔ Adam ↔ KFAC ↔
  learned conv all share `Δ' = B·Δ`) — most underrated mapping
- H-3: CTU quad-tree is the universal hierarchical-attention
  substrate (Mistral sliding window is depth-3 leaf processing)
- H-4: rANS + k-means achieves Shannon-optimal lossless gradient
  compression (every published scheme is a special case)
- H-5: PR-X12 generalises ZeRO (Merge captures parameter sharing
  that ZeRO's 3-bucket scheme can't)
- H-6: 64×64 CTU is the right unit for both 4K video and 7B LLMs
- H-7: The codec is the substrate; everything else is a renaming

# Technical debt (23 items)

Severity gradient P0/P1/P2/P3. Notable open items:
- T-1, T-2: PR #195 CodeRabbit findings (BASIN_NONE collision +
  unwrap_or non-bijection) — actionable today
- T-10..T-15: ndarray substrate (HPC graduation incomplete, no
  `Transform` trait yet, NEON tile-GEMM stub)
- T-20..T-23: cross-repo coordination (cognitive-substrate-
  convergence-v1.md cross-ref, causal-edge v2 metadata wiring)

# Board hygiene

AGENT_LOG.md prepended with the session entry per CLAUDE.md rule
"every PR adding a finding/agent run must PREPEND an AGENT_LOG
entry in the same commit". Entry cites the doc structure, the 15
epiphanies + 7 holy grails by ID, and the 23 debt items.

# No code changes

Pure architecture / knowledge artifact. `cargo check --lib` clean
(nothing to recheck).

https://claude.ai/code/session_01HbqooFZHAjaUtFEzhA1R2u
621-line synthesis doc consolidating two parallel architecture threads
on PR-X12's predictive-coder substrate:

- Session A: pr-x12-codec-cognitive-substrate-mapping.md (cognitive
  shaders ↔ gradient ↔ HEVC mapping, this branch)
- Session B: pr-x12-cross-domain-synergies.md (x265 ⇄ 3DGS ⇄ attention
  ⇄ SGD, merged to master via PR #195 commit 01c77cc)

Both originals retained for archeology; merged-canon supersedes them
for new content. Structure:

- §1 overlap matrix (epiphanies, holy grails, plans, debt — unique vs
  shared between A and B)
- §2 architectural claim with two-route falsifiability
- §3 ten merge-only epiphanies (M:E-A..M:E-J) — the high-density
  section: universal LinearReduce<Basis> kernel, Morton+raster as
  space-filling curve, PR #195 CodeRabbit findings generalise to all
  four domains, ndarray-codec as 5th architectural category, three
  domain-specific plug-points only
- §4 unified holy grail list (A:H + B:HG + M:H + 2 new M:H-NEW)
- §5 unified plan with resolved critical path (B's A7-first wins,
  front-load A4-design)
- §6 sequencing DAG
- §7 unified technical debt (P0/P1/P2/P3 reranked across both docs)
- §8 resolves 4 disagreements between A and B (each with winner +
  rationale)
- §9 read order for the 4-doc family
- §10 compaction-preservation contract (8 invariants)

Key merge-only outputs:

- Three load-bearing traits: PredictiveSignal, LinearReduce<Basis>,
  CurveOrder<const N>
- M:H-NEW-1: single Rust binary consumes video|splat|gradient|kv-cache
  → compressed Lance column (the falsifiability test, Plan G builds it)
- M:H-NEW-2: ~1.5 KLoC generic + ~200 LoC per domain = ~2 KLoC for
  four industries (25× code-density vs per-domain impls)

Three new integration plans:

- Plan G: multi-domain bench harness (2 weeks, MUST ship before A7)
- Plan H: extract `ndarray-codec` crate (3 days, resolves dep-cycle)
- Plan I: trait PredictiveSignal in protocol (3 days, bit-pattern
  isomorphism with splat without code merge)

Citation IDs preserved from both originals: cite as (A:E-N), (B:E-N),
(M:E-X). No code changes; pure synthesis. Cargo check clean.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 22, 2026

📝 Walkthrough

Walkthrough

This PR adds/updates PR‑X12 architecture documentation (session log, refreshed spec, merged canonical spec) and modifies codec code to use the full 12-bit basin index (MAX_BASIN_IDX = 4095), removing the BASIN_NONE sentinel helper and related public re-exports and tests.

Changes

PR-X12 Architecture Specification and Consolidation

Layer / File(s) Summary
Session Log Entry
.claude/board/AGENT_LOG.md
Appends the 2026-05-22 PR‑X12 session entry referencing the produced architecture doc, listing sections, epiphanies/claims, technical-debt inventory (T-1..T-23), integration plan, verification steps, and commit status.
Core PR-X12 Architecture Specification
.claude/knowledge/pr-x12-codec-cognitive-substrate-mapping.md
Refreshed cross-stack spec: four-axis mode taxonomy, CTU quad-tree mapping, explicit 12-bit/4096-entry basin/palette semantics, transform-basis trait, rANS frequency-table plan, λ·D+R RDO framing, epiphanies (E-1..E-15), holy-grail items (H-1..H-7), concrete integration plan, exploration paths, technical debt, and MKL/blasgraph synergies.
Merged Canonical Architecture Specification
.claude/knowledge/pr-x12-substrate-merged-canon.md
Merged authoritative spec combining two session docs: identity/citation rules, merge-only epiphanies (M:E-A..M:E-J), unified integration phases (Phase 0/1/2), explicit critical path, deduplicated technical debt (P0/P1/P2 + M:T items), resolved disagreements, and read-order/compaction guidance.

Codec basin/API changes

Layer / File(s) Summary
MAX_BASIN_IDX full-range update and tests
src/hpc/codec/mode.rs
MAX_BASIN_IDX changed to (1 << 12) - 1 (4095). Tests added/updated to assert full 12-bit maximum and header pack_header/unpack_header round-trip behavior for that value.
Remove sentinel helper and tests
src/hpc/codec/predict.rs
Removed BASIN_NONE import and deleted the exported is_no_basin helper and its unit test; intra prediction logic otherwise unchanged.
Update public re-exports
src/hpc/codec/mod.rs
Removed mode::BASIN_NONE and predict::is_no_basin from pub use exports; retained MAX_BASIN_IDX, packing helpers, and intra prediction exports.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • AdaWorldAPI/ndarray#195: PR #195 is referenced and its sentinel/serialization follow-ups are tracked/resolved in these documents and code updates.

Poem

A rabbit scribbled plans at dawn,
Maps and manifests drawn upon,
Docs merged clean, the basin set wide,
Sentinel gone, the indices glide,
Hop, compile—architectures reside. 🐰✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'docs(codec): PR-X12 substrate merged-canon — session A + B synthesis' directly and accurately describes the main change: merging two documentation sessions into a canonical synthesis document.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/continue-ndarray-x0Oaw

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.claude/knowledge/pr-x12-codec-cognitive-substrate-mapping.md:
- Around line 56-61: The Markdown file contains multiple untyped fenced code
blocks (e.g., the block starting with "Skip   (2 bytes total) ⊂  free" and the
blocks listing "depth 0 … depth 3" variants) which trip markdownlint MD040;
update each triple-backtick fence to include the language identifier `text`
(e.g., replace ``` with ```text) for every occurrence mentioned (the blocks
around the "Skip/Merge/Delta/Escape" table and the three "depth 0–3" examples)
so the content stays identical but linting passes.

In @.claude/knowledge/pr-x12-substrate-merged-canon.md:
- Around line 6-7: The markdown triggers MD027/MD040/MD038 due to incorrect
list/blockquote markers, malformed code fences, and improper inline emphasis;
fix by converting the mis‑prefixed lines into a proper fenced code block using
```text with matching closing fence, remove leading '-' list markers inside that
code fence (e.g., the "ndarray = hardware..." and "ndarray-codec = ..." lines),
restore the bold line to "**Total stack..." (remove stray '-' prefix and change
"IS" to "is"), and ensure any inline code spans use backticks without
surrounding hyphens or stray punctuation so the blocks (the ASCII diagram fence
and the small inline code examples) pass markdownlint MD027/MD040/MD038 checks.
🪄 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: 6fc0db58-aa77-408f-bbf8-cda1d5e020b0

📥 Commits

Reviewing files that changed from the base of the PR and between ef955ab and 4529c01.

📒 Files selected for processing (3)
  • .claude/board/AGENT_LOG.md
  • .claude/knowledge/pr-x12-codec-cognitive-substrate-mapping.md
  • .claude/knowledge/pr-x12-substrate-merged-canon.md

Comment thread .claude/knowledge/pr-x12-codec-cognitive-substrate-mapping.md Outdated
Comment on lines +6 to +7
> - `pr-x12-codec-cognitive-substrate-mapping.md` (session A: opus 4.7 main thread, this branch)
> - `pr-x12-cross-domain-synergies.md` (session B: parallel thread, PR #195 branch, commit `01c77ccc`)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Clean up markdownlint issues in blockquote, code fences, and inline code spans.

This section currently triggers MD027/MD040/MD038; small formatting edits should make the doc lint-clean.

Proposed doc-lint fix
->   - `pr-x12-codec-cognitive-substrate-mapping.md` (session A: opus 4.7 main thread, this branch)  
->   - `pr-x12-cross-domain-synergies.md` (session B: parallel thread, PR `#195` branch, commit `01c77ccc`)  
+> - `pr-x12-codec-cognitive-substrate-mapping.md` (session A: opus 4.7 main thread, this branch)  
+> - `pr-x12-cross-domain-synergies.md` (session B: parallel thread, PR `#195` branch, commit `01c77ccc`)  

-```
+```text
- ndarray = hardware (SIMD, Palette, Base17, SpoDistanceMatrices, read_bgz7_file)
- ndarray-codec = compression substrate (Ctu, LeafCu, predict_intra, rANS) ← NEW
- lance-graph = thinking (NarsTruth, NarsEngine, TripleModel, AutocompleteCache)
- causal-edge = protocol (CausalEdge64, NarsTables, forward/learn)
- p64 = convergence highway (both repos meet here)

-Total stack for all four industries: ~2 KLoC. Compared to ~50 KLoC per-domain implementations elsewhere. The 25× code-density delta IS the architectural payoff that justifies the eight sub-cards.
+Total stack for all four industries: ~2 KLoC. Compared to ~50 KLoC per-domain implementations elsewhere. The 25× code-density delta is the architectural payoff that justifies the eight sub-cards.

- +text
┌──────────────────────────────────────┐
│ Plan G (multi-domain bench) │
│ 2 weeks — UNFALSIFIABILITY GATE │
└──────────────────┬───────────────────┘
...
│ All unlocked │
└─────────────────┘

Also applies to: 165-171, 303-304, 411-455

🧰 Tools
🪛 markdownlint-cli2 (0.22.1)

[warning] 6-6: Multiple spaces after blockquote symbol

(MD027, no-multiple-space-blockquote)


[warning] 7-7: Multiple spaces after blockquote symbol

(MD027, no-multiple-space-blockquote)

🤖 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-merged-canon.md around lines 6 - 7, The
markdown triggers MD027/MD040/MD038 due to incorrect list/blockquote markers,
malformed code fences, and improper inline emphasis; fix by converting the
mis‑prefixed lines into a proper fenced code block using ```text with matching
closing fence, remove leading '-' list markers inside that code fence (e.g., the
"ndarray = hardware..." and "ndarray-codec = ..." lines), restore the bold line
to "**Total stack..." (remove stray '-' prefix and change "IS" to "is"), and
ensure any inline code spans use backticks without surrounding hyphens or stray
punctuation so the blocks (the ASCII diagram fence and the small inline code
examples) pass markdownlint MD027/MD040/MD038 checks.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4529c01f2c

ℹ️ 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".


### 4.1 The 12-bit basin = 4096-entry vocabulary

`MAX_BASIN_IDX = (1 << 12) - 1 = 4095` (`mode.rs:71`). Each `LeafCu` carries a 12-bit index into the per-frame basin codebook. For:
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Update basin-index docs to match shipped constants

This section states MAX_BASIN_IDX = 4095 and frames the BASIN_NONE collision fix as pending, but src/hpc/codec/mode.rs in this commit already ships MAX_BASIN_IDX = 4094 with BASIN_NONE = 4095. Keeping the old values here can cause downstream design/docs work to assume the wrong valid basin range and reintroduce the resolved sentinel-collision bug in new consumers.

Useful? React with 👍 / 👎.

Two CodeRabbit MD040/MD027 findings + one Codex P2 stale-fact finding,
all verified valid against current code.

CodeRabbit (markdownlint):
- pr-x12-codec-cognitive-substrate-mapping.md: 4 untyped code fences
  → ```text (Skip/Merge/Delta block at line 56; depth-0..3 quad-tree
  blocks for spatial/attention/gradient hierarchies at lines 80/91/104)
- pr-x12-substrate-merged-canon.md: collapse `>   - ` to `> - `
  (lines 6-7, MD027 multi-space-after-blockquote), 2 untyped fences
  → ```text (architecture-rule list at line 165, sequencing diagram
  at line 411), and "delta IS" → "delta is" stylistic

Codex P2 (stale doc vs shipped code):
- Doc cited `MAX_BASIN_IDX = 4095` and framed BASIN_NONE collision
  as "pending / not yet merged / not yet pushed" — but PR #195 commit
  2423298 already shipped `MAX_BASIN_IDX = 4094` with `BASIN_NONE
  = 4095` reserved sentinel, plus bijective `pack_leaf` via `?`
  operator with 3 regression tests. Updated:
  - §4.1 line 125: `MAX_BASIN_IDX = 4094` + sentinel-range explainer
  - §4.3 heading + body: "(resolved in PR #195)" with commit cite
  - §"Still open on PR #195" block: → "Resolved in PR #195 follow-up"
  - §12 debt-inventory T-1, T-2: marked ~~RESOLVED~~ with commit ref

No content edits beyond what the findings asked for; citation IDs
unchanged.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
.claude/knowledge/pr-x12-substrate-merged-canon.md (1)

303-303: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix MD038: Remove space inside code span.

The inline code spans have improper spacing that triggers markdownlint MD038.

📝 Proposed fix
-**M:H-NEW-2** — `trait PredictiveSignal` + `trait LinearReduce<Basis>` + `trait CurveOrder<const N: usize>` factor the codec into three plug-points (per M:E-E + M:E-A + M:E-B). The codec body is `<150 LoC of generic glue. Domain consumers ship `<200 LoC` of trait impls. **Total stack for all four industries: ~2 KLoC.** Compared to ~50 KLoC per-domain implementations elsewhere. The 25× code-density delta is the architectural payoff that justifies the eight sub-cards.
+**M:H-NEW-2** — `trait PredictiveSignal` + `trait LinearReduce<Basis>` + `trait CurveOrder<const N: usize>` factor the codec into three plug-points (per M:E-E + M:E-A + M:E-B). The codec body is `<150 LoC` of generic glue. Domain consumers ship `<200 LoC` of trait impls. **Total stack for all four industries: ~2 KLoC.** Compared to ~50 KLoC per-domain implementations elsewhere. The 25× code-density delta is the architectural payoff that justifies the eight sub-cards.
🤖 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-merged-canon.md at line 303, The Markdown
has code spans with extra spaces inside the backticks (e.g. "` trait
PredictiveSignal`", "` trait LinearReduce<Basis>`", "` trait CurveOrder<const N:
usize>`"); remove the leading/trailing spaces inside each inline code span so
the identifiers are exactly `PredictiveSignal`, `LinearReduce<Basis>`, and
`CurveOrder<const N: usize>` (and any other similarly spaced inline spans) to
satisfy MD038.
🤖 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.

Duplicate comments:
In @.claude/knowledge/pr-x12-substrate-merged-canon.md:
- Line 303: The Markdown has code spans with extra spaces inside the backticks
(e.g. "` trait PredictiveSignal`", "` trait LinearReduce<Basis>`", "` trait
CurveOrder<const N: usize>`"); remove the leading/trailing spaces inside each
inline code span so the identifiers are exactly `PredictiveSignal`,
`LinearReduce<Basis>`, and `CurveOrder<const N: usize>` (and any other similarly
spaced inline spans) to satisfy MD038.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ef891cf5-d46e-4871-b359-fbfb1a22b534

📥 Commits

Reviewing files that changed from the base of the PR and between 4529c01 and 4d0ec96.

📒 Files selected for processing (2)
  • .claude/knowledge/pr-x12-codec-cognitive-substrate-mapping.md
  • .claude/knowledge/pr-x12-substrate-merged-canon.md
✅ Files skipped from review due to trivial changes (1)
  • .claude/knowledge/pr-x12-codec-cognitive-substrate-mapping.md

claude added 3 commits May 22, 2026 11:10
… codebook

PR #195's follow-up commit 2423298 resolved the BASIN_NONE/MAX_BASIN_IDX
collision by shrinking MAX_BASIN_IDX to 4094 with BASIN_NONE = 4095 as a
distinct sentinel. The shrink was the wrong fix — the sentinel itself
was the bug.

Reference (the authoritative source the code drifted from):

  src/hpc/ogit_bridge/assets/cognitive/entities/Leaf.ttl — the HHTL
  ontology defines the codebook as 16 Hips × 16 Twigs × 16 Leaves =
  4096 Leaves per Heel, every Leaf carrying a real basinSignature.
  No slot is reserved for absence; the ontology forbids "no basin".

What BASIN_NONE actually was: authoring-time epistemic uncertainty
("encoder hasn't decided yet for this cell") smuggled into wire-format
ontological uncertainty ("this cell exists but has no basin"). A
category error — Rust's idiom would have surfaced this as Option<u16>
in the encoder's transient scratch state and a plain u16 on the
persistent record. The sentinel value bypassed that hygiene by hiding
optionality inside a magic u16, invisible to the typechecker.

Confirmation that the sentinel was load-bearing for nothing: full audit
of BASIN_NONE producers across src/ found zero — no LeafCu::skip(...),
no basin_idx: BASIN_NONE anywhere. The only references were the const
definition, a re-export, one regression test that constructed it, and
predict.rs:is_no_basin (a predicate with no callers that emit
BASIN_NONE leaves). Defensive infrastructure for a feature the
ontology forbids and the implementation never used.

Changes:

- mode.rs: MAX_BASIN_IDX = (1 << 12) - 1 = 4095 (full 12-bit range,
  4096 real basins). BASIN_NONE deleted. The 2 sentinel-distinctness
  regression tests replaced with one test asserting the full range.
  Docs reframed around the HHTL ontology + the Option<u16>
  scratch-state contract.
- predict.rs: is_no_basin deleted; its only doctest+test deleted.
  BASIN_NONE import dropped.
- mod.rs: BASIN_NONE and is_no_basin removed from re-exports.
- pr-x12-codec-cognitive-substrate-mapping.md §4.1/§4.3/§10.1/§12:
  framing updated. §4.3 now records the full arc (collision → shrink
  → delete) as design archeology rather than as an open issue.

Validation:
- cargo check --features codec --lib → clean
- cargo clippy --features codec --lib -- -D warnings → clean
- cargo test --features codec --lib hpc::codec → 54 tests pass
- cargo test --features codec --doc hpc::codec → 13 doctests pass

Net diff: -77, +38 lines. Pure subtraction modulo doc reframe.
Drops the forensic call-outs and walks the docs back to the positive
design statement: the codebook is 16 × 16 × 16 = 4096 Leaves per Heel
(per Leaf.ttl), MAX_BASIN_IDX = 4095 addresses the full range. No
narrative arc; no archeology subsection.
Two-layer rule sitting above MAX_BASIN_IDX:
  - encoder scratch state  → Option<u16>
  - wire format / persisted → plain u16, every value real

So the next editor doesn't reach for a magic value when what they
actually want is type-level optionality.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@src/hpc/codec/mode.rs`:
- Around line 62-79: The change makes 0xFFF a real basin value but breaks
compatibility with older payloads that used 0xFFF as a sentinel; update the
(de)serialization path in src/hpc/codec/mode.rs—specifically in the functions
that pack/unpack the 12-bit basin field (look for pack_leaf, unpack_leaf,
encode_leaf, decode_leaf or the Leaf struct's (de)serialize impl) to gate
behavior by an explicit format/version flag: when format < NEW_VERSION treat
0xFFF as the old sentinel and map it to Option::None (or the prior semantics),
and when format >= NEW_VERSION treat 0xFFF as basin 4095; alternatively add a
compatibility branch that rewrites incoming 0xFFF to the prior sentinel before
normal decoding and ensure outgoing encoders include the format version header
so older decoders can retain original behavior.
🪄 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: af51558f-a67b-440e-9121-2e08cd757521

📥 Commits

Reviewing files that changed from the base of the PR and between 7fdcfb5 and d272fea.

📒 Files selected for processing (1)
  • src/hpc/codec/mode.rs

Comment thread src/hpc/codec/mode.rs
Comment on lines +62 to +79
// ── Design invariant ────────────────────────────────────────────────
//
// The 12-bit basin field addresses exactly one of 4096 real Leaves
// (HHTL ontology, see `src/hpc/ogit_bridge/assets/cognitive/entities/Leaf.ttl`:
// 16 Hips × 16 Twigs × 16 Leaves per Heel, every Leaf carries a real
// `basinSignature`). Do NOT reserve a value as a "no basin" /
// "not yet decided" sentinel.
//
// Rationale: that's authoring-time uncertainty (encoder mid-decision)
// leaking into wire-format ontological state. Keep optionality in the
// type, not in a magic value:
//
// - encoder scratch state → `Option<u16>` (Some = committed, None = TBD)
// - persisted / wire-format → plain `u16`, every value real
//
// The doubt must collapse to Some(basin) before the leaf is packed.
// Once a leaf reaches the wire it has a basin, period.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Preserve compatibility for the reclaimed 0xFFF header value.

This change redefines a previously reserved wire value as a real basin. That means any pre-change payload containing 0xFFF will now deserialize as basin 4095, and any new payload using basin 4095 will be misread by older decoders as the old sentinel. Please gate this behind an explicit format version or add a compatibility path before changing the on-wire meaning.

🤖 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 `@src/hpc/codec/mode.rs` around lines 62 - 79, The change makes 0xFFF a real
basin value but breaks compatibility with older payloads that used 0xFFF as a
sentinel; update the (de)serialization path in
src/hpc/codec/mode.rs—specifically in the functions that pack/unpack the 12-bit
basin field (look for pack_leaf, unpack_leaf, encode_leaf, decode_leaf or the
Leaf struct's (de)serialize impl) to gate behavior by an explicit format/version
flag: when format < NEW_VERSION treat 0xFFF as the old sentinel and map it to
Option::None (or the prior semantics), and when format >= NEW_VERSION treat
0xFFF as basin 4095; alternatively add a compatibility branch that rewrites
incoming 0xFFF to the prior sentinel before normal decoding and ensure outgoing
encoders include the format version header so older decoders can retain original
behavior.

@AdaWorldAPI AdaWorldAPI merged commit bc9da4a into master May 22, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants