docs(splat-native): address review feedback on #471 (5 fixes — StepDomain, canonical status, pose_se3, NR-SPLAT-PHI, MD040)#472
Merged
Conversation
Follow-up to PR #471 (merged). Addresses five reviewer findings from codex P2 and CodeRabbit Major/Minor: ## Fixes in this PR ### Fix 1 — StepDomain alignment (codex P2, plan §10.10) The §10.10 cross-repo coordination section referenced `StepDomain::{Codec, Thinking, Query, Semantic, Persistence, Inference, Learning}` as the "existing" shipped taxonomy. That is incorrect — the shipped contract at `crates/lance-graph-contract/src/orchestration.rs:37` defines exactly EIGHT variants: `Crew, Ladybug, N8n, LanceGraph, Ndarray, Smb, Medcare, Kanban`. Rewrites the paragraph to: (a) name the actual shipped variants verbatim with file:line citation; (b) state that `SplatFit/SplatRender` are NEW variants ADDED to the shipped enum (extends, not parallel); (c) declare the splat ingest route as `SplatFit → Ndarray → LanceGraph → Medcare`; (d) declare the `from_step_type` step-type prefixes (`splat_fit.*`, `splat_render.*`). ### Fix 2 — STATUS_BOARD canonical status (CodeRabbit Major) The 14 D-SPLAT-* rows had Status values like `**Queued — P1 sprint 1-2**` that mixed the canonical status enum (`Queued | In progress | In PR | Shipped`) with sprint timing. Adds a dedicated `Sprint` column; Status now holds only `**Queued**` (canonical) for each row. Table header updated; all 14 row cells split. ### Fix 3 — `pose_se3` byte width 16 → 24 (CodeRabbit Major) The lance-graph plan's `Gaussian3D` carrier showed `pose_se3: FixedSizeBinary(16)` while describing a payload of 3 translation + 9 rotation entries in `f16` = 12 × 2 = 24 bytes. Corrects to `FixedSizeBinary(24)` and annotates the byte arithmetic inline. Companion fix landing in `MedCare-rs/.claude/handovers/ 2026-06-05-splat-native-medcare-hipaa-wire.md`. ### Fix 4 — PHI classification normative rule (CodeRabbit Major) Lines 268 and 443 stated mutually contradictory positions on whether splat coordinates carry PHI. Introduces a single normative rule **NR-SPLAT-PHI** at §3.10 and references it from: - §3.10 (raw RF/IQ paragraph) - §7 risk-matrix row "HIPAA PHI leak via splat coordinates" (downgraded HIGH→MEDIUM since coordinates per NR-SPLAT-PHI are non-identifying on their own) - §8 success-criteria sole-writer paragraph The rule: > Scanner-frame splat geometry (the `mu`/`sigma_packed` columns; > `pose_se3` in scanner frame) is non-identifying on its own. > The link between `patient_ref` and a splat volume — i.e. the row > in `memory.ultrasound_frame.lance` — IS PHI. > Atlas-aligned annotations that name patient-specific landmarks > (e.g. `class_id` resolving to "this patient's left biceps > brachii at scanner-pose P") ARE PHI. > Raw RF/IQ is PHI by default and is never persisted (it does not > enter the MedCare wire). ### Fix 5 — MD040 fenced-code language tags (CodeRabbit Minor) Adds `text` language identifier to the two unlabeled opening fences: - `## 5. Dependencies graph (textual)` (was line 403) - `## 10.8 The interconnect map (visual)` (was line 659) ### Bonus consistency fix — IVD-MDR → MDR Annex VIII Rule 11 D-SPLAT-14 row + plan footer cited "IVD-MDR Rule 11" which is the wrong regulation (IVDR is for in-vitro diagnostics). For ultrasound SaMD the correct citation is **MDR Annex VIII Rule 11**. Same correction landing in OGAR follow-up PR (closes codex P2 there). ## What's NOT in this PR - Source code: still none. Plan-spec only. - The `pose_se3` companion fix in `MedCare-rs/.claude/handovers/...` ships as a separate PR (per cross-repo coordination protocol). - The IVD-MDR consistency fix in OGAR `docs/SPLAT-NATIVE-CUSTOMER.md` ships as a separate PR. - The ndarray opacity-blend ray-segmentation + Mahalanobis scratch-buf fixes ship as a separate PR. ## Test plan - [x] Plan file passes markdown lint (MD040 cleared on both fences). - [x] STATUS_BOARD canonical-status invariant restored (only `Queued | In progress | In PR | Shipped` appear in Status column). - [x] StepDomain text cites the actual shipped enum (`orchestration.rs:37` verified pre-edit). - [x] NR-SPLAT-PHI declared once, referenced from three other sections. - [x] `pose_se3` byte-arithmetic consistent (24 B = 12 × `f16`). - [ ] Codex + CodeRabbit re-review on this PR.
|
Warning Review limit reached
More reviews will be available in 27 minutes and 14 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
AdaWorldAPI
pushed a commit
that referenced
this pull request
Jun 5, 2026
…ce contract Closes the runtime-side follow-up commitment that **OGAR #39 ADR-024 § Consequences** explicitly names by reference: *"Reports ρ-vs-reference on first per-country PBF run per the runtime session's §11 follow-up commitment"*. Sibling to #474 (the codex P2 D-OSM-2 ownership-boundary fix landed by the other session). This PR is purely additive — appends a new **§11 ADR-024 adoption — palette256 + HHTL codec contract** section at the end of `cesium-osm-substrate-v1.md` without touching the existing §1-§10 or the §11 (NOT covered) section labelling (now §11 ADR-024 + §10 NOT covered before _End of plan_). ## What the new §11 pins 1. **Adoption checklist** (ADR-024's 4-step) mapped onto D-OSM-2: - Prefix = Cesium TMS quadkey `qk:<level>/<x>/<y_tms>` (Q2 ruling). - Palette domain = OSM tag-values per-tile (≤256 at zoom 21 per ADR-024 §256-ceiling escape hatches) + tile-local quantized coords (sub-cm at zoom 21). - ρ-vs-reference target ≥ 0.99 matching the `arm-discovery` aerial-codebook anchor (ρ = 0.9973 vs cosine). - Decode = const-table lookup; zero-allocation. 2. **Falsifiable adoption contract** for D-OSM-2: - Report empirical ρ on first per-country PBF run (default: Liechtenstein per §6 OQ-OSM-4). - Report per-tile palette cardinality distribution (mean / p95 / p99) — escape hatch selected from measurement. - Decode bandwidth target ≥ 10⁸/sec on AVX-512. - If ρ < 0.99 on first run, document gap before multi-country ingest (escalate to per-tile, hierarchical, or palette-64K). 3. **Cross-arc adopters table** — three shipped anchors (`arm-discovery`, `Binary16K` `_effectiveReaders`, `bgz-tensor` `WeightPalette`) + two queued adopters (`D-OSM-2`, `D-SPLAT-4` — both named in ADR-024 § Consequences). 4. **Why §11 not §3**: ADR-024 codec contract is independent of §3 carrier-shape contract (Q1 v1 fallback). The Arrow `List<Struct<key,value>>` column shape holds whether or not the tag value is palette-encoded; palette is a transparent codec underneath the column. Keeping §11 separate preserves §3's "carrier shape" framing while pinning the codec contract. ## Why a separate small PR (not bundled with #474) #474 (other session) landed the codex P2 D-OSM-2 ownership- boundary fix as a 1-line minimal edit. The §11 ADR-024 callout is purely additive (no overlap with #474's diff). Keeping them as two PRs preserves the codex-fix vs proactive-architectural-pin separation — codex reviewers can pattern-match each PR's intent without bundling. ## Companion (separate small PR — also being filed) D-SPLAT-4 in `splat-native-ultrasound-v1.md` deserves the symmetric ADR-024 adoption note (ADR-024 § Consequences names both D-OSM-2 AND D-SPLAT-4 as queued adopters). That edit ships as `claude/splat-native-adr-024-callout` since the splat-native arc has its own branch lineage and its own #471/#472 PR history. ## Test plan - [x] Docs/plan only — no source code; no build/test invoked. - [x] §11 cites ADR-024 by file:line (`OGAR/docs/ARCHITECTURAL- DECISIONS-2026-06-04.md`) + the arm-discovery ρ=0.9973 empirical anchor + bgz-tensor `WeightPalette` reference. - [x] Falsifiable contract is testable at D-OSM-2 implementation time (per-country PBF run; reported ρ + cardinality distribution + decode bandwidth). - [x] No collision with #474 (purely additive section append). - [ ] Codex re-review on this PR.
5 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.
Summary
Follow-up to #471 (merged). Addresses 5 reviewer findings from codex P2 + CodeRabbit Major/Minor on the canonical splat-native-ultrasound-v1 plan + STATUS_BOARD.
Fixes
Codec/Thinking/Query/Semantic/Persistence/Inference/Learningas "existing" variants — these do NOT exist inlance-graph-contract/src/orchestration.rs:37. The shipped enum is exactlyCrew, Ladybug, N8n, LanceGraph, Ndarray, Smb, Medcare, Kanban. Rewrites the paragraph to cite the actual enum atfile:line, declaresSplatFit/SplatRenderas NEW variants ADDED to the shipped enum (extends, not parallel), routes splat ingest asSplatFit → Ndarray → LanceGraph → Medcare, declares thefrom_step_typestep-type prefixes.**Queued — P1 sprint 1-2**drift from the canonical enum (`Queuedpose_se3byte width 16 → 24. The carrier showedFixedSizeBinary(16)but payload is 3 trans + 9 rot inf16= 12 × 2 = 24 B. Corrects toFixedSizeBinary(24)with byte arithmetic inline. Same fix in MedCare-rs follow-up.patient_ref ↔ splat_volumelink IS PHI; atlas-aligned annotations naming patient-specific landmarks ARE PHI; raw RF/IQ is PHI by default and is never persisted. Referenced from §3.10, §7 risk-matrix row (downgraded HIGH→MEDIUM), §8 sole-writer paragraph.textlang id to the two unlabeled opening fences (§5 Dependencies graph; §10.8 Interconnect map).Bonus consistency: D-SPLAT-14 row + plan footer cited "IVD-MDR Rule 11" which is the wrong regulation (IVDR is for in-vitro diagnostics). Corrected to MDR Annex VIII Rule 11. Same correction landing in OGAR follow-up.
What's NOT in this PR
pose_se3companion fix in MedCare-rs ships as a separate PR (MedCare-rs/claude/splat-native-ultrasound-v1-fixes).docs/SPLAT-NATIVE-CUSTOMER.mdis PR docs: mark Phase 2 DONE, add integration status and progress tracker Phase 2 (bgz17 container/semiring) verified complete — 121 tests passing. Added cross-repo integration status section to FINAL_STACK.md. Created PROGRESS.md tracking Plateaus 0-3 against master integration plan. https://claude.ai/code/session_01CdqyUTUfjKZuk8YGJzv6LB #35 in OGAR.ndarray/claude/splat-native-ultrasound-v1-fixes).Test plan
orchestration.rs:37).pose_se3byte arithmetic consistent (24 B = 12 ×f16).Companion follow-up PRs (the four-PR fix arc)
lance-graphclaude/splat-native-ultrasound-v1-fixespose_se316→24, NR-SPLAT-PHI normative rule, MD040 fence-tag lint, IVD-MDR→MDR Annex VIII (#471 follow-up)ndarrayclaude/splat-native-ultrasound-v1-fixesbatched_opacity_blendray segmentation (CSR-styleray_offsets),batched_mahalanobisCholesky-scratch buffer (#212 follow-up)MedCare-rsclaude/splat-native-ultrasound-v1-fixespose_se316→24 (#163 follow-up)OGARclaude/splat-native-customer-fixes(PR #35)All four cross-reference each other. No source code in any of the four PRs — design-spec / handover updates only.
Authored by session
claude/lance-graph-ontology-review-Pyry3.