helix: real-crate Fisher-Z rim bake + rim decode + Gouraud render#65
Merged
Conversation
…mesh size Render-only upgrades to the /body viewer (no rebake): - Semantic x-ray opacity: the x-ray mode now fades each compartment by its own alpha (skin 0.10 / muscle 0.20 / organ 0.45 / skeleton 0.50 / vessel 1.0 / nervous 1.0 / connective 0.22 / other 0.30) via a new uLayerAlpha[9] uniform, instead of one flat whole-body alpha. The selected concept always pops to full opacity (uSelRow match → a=1.0) so it reads cleanly through translucent tissue. Solid mode is unchanged (uXray=0 keeps the legacy uGlobalAlpha path). - Search results carry a colored compartment dot matching the layer legend, so vessels/organs/nerves are distinguishable at a glance. - The detail popup shows mesh size (vertex count) decoded from the per-concept (vstart,vcount) column. uLayerAlpha[li] uses dynamic uniform-array indexing, valid here for the same reason uEnabled[li] already is.
New experimental /helix route (BodyHelix.tsx), parallel to /body and sharing nothing with it, so the working /body can never break. Shades from the per-vertex helix NORMAL using the canonical lance-graph helix::Signed360 codec (6 bytes: rim endpoint pair + signed polar lift + golden azimuth), which is place-coupled to the HHTL address — encode(place, n) takes the HHTL place as its first argument. Decode: y from the polar partition, phi from the u16 azimuth, r = sqrt(1 - y^2), world (r*sin phi, y, r*cos phi). Decoded once at load into an octahedral-i8 x2 vertex attribute so the GPU reads the normal natively (normalized fetch, ~4-ALU octa decode) — no texture, no per-frame work. Canonical-only: reads the stamped helix bake named by body.manifest.json (helix_latest); deliberately does NOT fall back to the shared body.soa.gz, whose normals use the older place-blind helix_orient codec and would render garbage if read as Signed360. Also adds crates/osint-bake/tools/BAKE_ARTIFACTS.md: date/format-stamped bake naming + a manifest so a new bake never clobbers the working artifact. Follow-up (separate commit): the matching soabake change to GENERATE these normals via helix::ResidueEncoder::encode_signed(place, n, sign) instead of helix_orient — i.e. the rebake. Decoder and encoder get validated together at bake time.
…ls for /helix
New standalone crate scratch-fma/helixbake (its own [workspace]), side-by-side
with scratch-fma/soabake (the /body bake), which is left BYTE-IDENTICAL — the old
pipeline never resolves helix.
Generates the per-vertex NORMAL with the canonical lance-graph helix algorithm,
not a re-derivation:
- nearest spherical-Fibonacci index n of the world normal (same phi-spiral
helix::HemispherePoint::lift walks),
- helix::ResidueEncoder::encode_signed(place, n, sign) -> 6-byte Signed360,
place = the concept's HHTL path (HEEL/HIP/TWIG) so the rim anchors on the
place via CurveRuler::from_place ('normal residue on top of HHTL'),
- the Fisher-Z RollingFloor is built once (pre-materialized), shared across all
vertices; per-vertex encode is table lookups + quantize.
Emits BSO2 ver 6 (F16 pos + Signed360 helix-normal column), a stamped artifact
per BAKE_ARTIFACTS.md; the render direction lives in (polar, azimuth) and is
place-independent, which BodyHelix.tsx decodes -> octahedral i8 (cheap on CPU
SIMD and GPU-less WebGL, no per-vertex trig materialization).
Includes a #[test] round-tripping encode->decode (the same decode the viewer
runs) on synthetic normals, < 1.5deg, no FMA data needed. Cargo manifest patches
helix's git ndarray to the local fork; this sandbox's proxy blocks that fetch, so
the crate is validated on a network-enabled bake host, not here. soabake and
/body are unaffected.
…l materialization The Signed360 (polar, azimuth) are NORMALIZED angular coordinates; reconstructing the Cartesian normal per vertex (√/sin/cos — and the rim's Fisher-Z/atanh) is unnecessary materialization. Replace it with a pre-materialized direction LUT: - buildDirLut() runs the trig ONCE (256 polar × 1024 azimuth ≈ 262k cells), not per the 4.2M vertices. - decode() now only copies each vertex's (polar, az_lo, az_hi) bytes into the aPolAz attribute — zero trig in the per-vertex path. - the vertex shader is a single normalized-index LUT fetch keyed by (polar, azimuth); equivalently a CPU-SIMD gather, so it stays cheap without a GPU. The rim (Fisher-Z / atanh endpoints) is the metric carrier and is never materialized for rendering — only (polar, azimuth) drive the normal.
…ders Makes /helix actually run, end-to-end on real data, without the FMA source pipeline or a helix build: - transcode_helix_signed360.py: fetches nothing itself, takes the released ver-5 body wire, decodes the old ndarray helix_orient normals back to Cartesian, and re-encodes the Signed360 render coords (polar partition + golden azimuth; the rim/metric is unused by rendering, left zero). Emits an HXN1 sidecar — 'HXN1' | nV u32 | (polar,az_lo,az_hi)[nV] — vertex-aligned to body.soa.gz. VERIFIED by decoding back exactly as the viewer does: mean 0.205 deg, p99 1.37 deg, max 5.09 deg over all 4,283,525 verts. - BodyHelix now loads TWO same-origin files: the shared body.soa.gz (geometry — already served for /body, no duplication) and the small HXN1 sidecar (normals, named by body.manifest.json helix_latest). The wire's own old-codec normals are never read. Per-vertex is still a normalized (polar,azimuth) LUT lookup — no trig. - cockpit/public/: the 12 MB sidecar gz + body.manifest.json (helix_latest), served same-origin via dist; geometry stays the release-pulled body.soa.gz. This is a render-faithful transcode (double-quantized through helix_orient, hence the few-degree tail); the metric-coupled artifact from helixbake on the FMA columns remains the eventual canonical replacement, drop-in via the same sidecar.
Per review: (1) the normals belong as a COLUMN of the same SoA as the positions, not interlaced across a second file; (2) the artifact belongs in a release, not committed to the repo. - transcode_helix_signed360.py: back to emitting ONE BSO2 ver-6 wire — F16 pos + a canonical Signed360 NORMAL column in the same struct-of-arrays (drops the HXN1 sidecar). Verified decode-back: mean 0.205 deg, p99 1.37 deg over 4,283,525 verts. - The 60 MB ver-6 wire is uploaded to the fma-body-soa-v3-v1 release (via GH_TOKEN + pygithub), NOT committed. Removed the 12 MB sidecar from public/. - BodyHelix reverts to a single-wire reader (the wire's Signed360 helix column → pre-materialized direction LUT, one gather per vertex; no per-vertex trig). - Dockerfile pulls body.20260628.v6helix.soa.gz into dist/ same-origin (same CORS reason as body.soa.gz); body.manifest.json (committed, small) names it via helix_latest.
…ering
- Dead buttons: the layer-toggle effect reassigned enabledRef.current to a NEW
array, but mount() captured the original array, so uEnabled never changed.
Mutate the array IN PLACE instead → toggles reach the shader.
- Slowness, two safe levers (no codec change):
- on-demand render: the 6.8 M-tri body is redrawn ONLY on drag/zoom/toggle/
resize, not every frame — idle is free, far less heat.
- adaptive pixel ratio: drop to 1x when a frame exceeds the ~30 fps budget;
on a retina phone this quarters/ninths the fragment load while rotating.
Does not touch the wire or the normalized Signed360 decode. The deeper cost
(per-vertex LUT gather over 6.8 M tris) is a mobile-vertex-fetch / LOD question,
flagged separately.
Bake the /helix Signed360 normals with the REAL lance-graph::helix crate (ResidueEncoder::encode_signed) against the local ndarray fork, instead of the hand-rolled transcoder that ZEROED the Fisher-Z rim. The rim (rim.end = arctanh(r) quantised, r = sinθ) is now populated — the "palette256 = the angle" carrier — and the wire carries an HXFL trailer with the exact RollingFloor (lo,hi) so the decoder dequantises with the same floor the encoder used. Single SoA wire (BSO2 ver 6), new stamped artifact body.20260629.v6helix.soa.gz; the prior bake is kept in the release, not deleted. BodyHelix now DECODES the rim: r=sinθ is reconstructed from rim.end via a 256-entry r-LUT (the only place atanh/tanh runs — at load, never per vertex), the polar partition gives the hemisphere sign and covers the r→1 saturation cliff. Each vertex becomes a normalized int8 NORMAL. Render switched to GOURAUD: lighting computed per-vertex from the cheap int8 normal, colour interpolated; the fragment shader drops to a layer discard + colour write. At 6.8M sub-pixel tris this is visually identical to per-fragment lighting but removes the per-fragment normalize/dot that made it 12 s/frame. No LUT texture fetch per vertex either. Verified end-to-end on all 4,283,525 source normals: browser decode vs source mean 0.263°, p99 0.918°, max 1.924°, 0% over 2°. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RhpwkHGgia2TuDFvdnuQdE
📝 WalkthroughWalkthroughAdds a new ChangesBodyHelix /helix viewer
BodyV3 x-ray, concept selection, and verts display
Sequence Diagram(s)sequenceDiagram
participant Browser
participant BodyHelix
participant fetchSoa
participant inflate
participant decode
participant mount as three.js mount
Browser->>BodyHelix: navigate /helix
BodyHelix->>fetchSoa: onMount
fetchSoa->>Browser: GET /body.manifest.json → helix_latest URL
fetchSoa->>Browser: GET stamped .soa.gz artifact
Browser-->>fetchSoa: gzipped Response
fetchSoa->>inflate: Response
inflate-->>fetchSoa: ArrayBuffer
fetchSoa-->>BodyHelix: ArrayBuffer
BodyHelix->>decode: ArrayBuffer → Decoded (positions, normals, colors, indices)
decode-->>BodyHelix: Decoded
BodyHelix->>mount: container + Decoded + enabledLayers Float32Array
mount-->>BodyHelix: cleanup fn
Browser->>BodyHelix: layer toggle click
BodyHelix->>mount: mutate enabledLayers in-place, set dirty=true
mount->>mount: RAF redraws on dirty flag
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
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 |
![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: 3e32b5b95d
ℹ️ 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".
| polar = np.where(ny >= 0, 128 + mag, 127 - mag).astype(np.uint8) | ||
| phi = np.mod(np.arctan2(nx, nz), 2 * math.pi) | ||
| az16 = np.rint(phi / (2 * math.pi) * 65536.0).astype(np.int64) & 0xFFFF | ||
| s360 = np.zeros((nV, 6), dtype=np.uint8) # rim bytes [0:3] = 0 (metric, unused) |
There was a problem hiding this comment.
When this transcoder is used to create a ver-6 artifact, it leaves rim.end at 0 for every vertex, but the new /helix decoder reads byte 1 as the Fisher-Z rim and uses rLut[end], only falling back to the polar magnitude at end == 255 (cockpit/src/BodyHelix.tsx:160-165). That means artifacts produced by this tool collapse the radial component to the first LUT bucket while the verification below still reconstructs r from polar, so it can report low error for a file the viewer renders with wrong normals. Please compute/populate the rim bytes and HXFL floor consistently, or remove/update this incompatible tool.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
Dockerfile (1)
65-67: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winKeep the downloaded helix stamp in sync with
body.manifest.json.
BodyHelixresolves the artifact name fromhelix_latest, but this layer hardcodes the same stamp again. On the next bake, updating only the manifest will make the embedded same-origin asset 404, and the fallback path is already documented here as unsuitable cross-origin. Please drive this download from the manifest (or a shared build arg) so the published pointer and embedded asset cannot drift.🤖 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 `@Dockerfile` around lines 65 - 67, The Dockerfile download step hardcodes the helix stamp separately from the manifest, so the embedded asset can drift from what BodyHelix resolves via helix_latest. Update the Dockerfile logic that downloads the body artifact to derive the stamp from body.manifest.json or a shared build arg, and keep the download target in sync with the same value used by BodyHelix so the published pointer and embedded asset always match.
🤖 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 `@cockpit/src/BodyHelix.tsx`:
- Around line 102-115: The decode path in BodyHelix.decode is still parsing
helix bytes for older payload versions, which can misinterpret non-v6 data as
Signed360. Add a strict version guard before the helix column is read so only
ver === 6 proceeds, and make the function fail fast for any other wire version.
Keep the check near the existing ver/posBytes setup so the canonical-only
behavior matches fetchSoa() and stale or mistyped assets are rejected early.
In `@cockpit/src/BodyV3.tsx`:
- Around line 363-373: The x-ray stats label in BodyV3 is stale because
`stRef.current.alpha` is now always set to 1.0 and x-ray opacity is controlled
by per-compartment `uLayerAlpha` instead of whole-body alpha. Update the
readout/label logic in `BodyV3` (the code that renders the `'x-ray (whole body
0.42)'` text) so it no longer hardcodes 0.42 and instead describes the semantic
per-compartment x-ray behavior consistently with the `useEffect` that sets
`stRef.current.alpha`.
- Around line 457-459: The name span inside the flex button in BodyV3.tsx will
not reliably truncate because it still has the default min-width:auto. Update
that flex child so it can shrink (for example by giving the span a flex style
and minWidth: 0) while keeping the existing ellipsis styles, so long channel
names truncate instead of pushing the trailing layer label.
In `@cockpit/src/main.tsx`:
- Around line 109-112: The route comment in main.tsx is outdated relative to
BodyHelix.tsx. Update the /helix comment near the BodyHelix route to describe
the current implementation accurately: it now performs a one-time CPU decode
into an Int8Array normal buffer, rather than using a 256×256 LUT or a
vertex-shader fetch. Keep the comment aligned with the BodyHelix component’s
actual behavior and naming so future readers can find the correct path quickly.
In `@crates/osint-bake/tools/BAKE_ARTIFACTS.md`:
- Around line 19-20: The helix artifact docs are outdated and still describe the
old v5 fallback flow. Update the naming table and surrounding text in
BAKE_ARTIFACTS.md to reflect the current v6 canonical-only helix path, using the
same symbols and terminology as BodyHelix and the manifest entry for
helix_latest. Remove the mention that BodyHelix falls back to body.soa.gz, and
describe that the viewer now expects body.20260629.v6helix.soa.gz and fails when
helix_latest is missing instead of using the shared v5 artifact.
In `@crates/osint-bake/tools/transcode_helix_signed360.py`:
- Around line 84-87: The transcode output in transcode_helix_signed360.py leaves
the rim bytes zeroed, so BodyHelix.tsx will decode every vertex using the first
LUT bucket instead of the intended radius. Update the s360 packing logic to
write a valid rim.end value that BodyHelix can interpret as a saturation
sentinel (or quantize the radius and emit the matching HXFL trailer), and make
the validation path around the existing radii check use the same viewer decode
assumptions rather than recomputing r directly from polar.
---
Nitpick comments:
In `@Dockerfile`:
- Around line 65-67: The Dockerfile download step hardcodes the helix stamp
separately from the manifest, so the embedded asset can drift from what
BodyHelix resolves via helix_latest. Update the Dockerfile logic that downloads
the body artifact to derive the stamp from body.manifest.json or a shared build
arg, and keep the download target in sync with the same value used by BodyHelix
so the published pointer and embedded asset always match.
🪄 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: 291a106e-70de-471f-b6ef-dd7b4a93c36e
📒 Files selected for processing (7)
Dockerfilecockpit/public/body.manifest.jsoncockpit/src/BodyHelix.tsxcockpit/src/BodyV3.tsxcockpit/src/main.tsxcrates/osint-bake/tools/BAKE_ARTIFACTS.mdcrates/osint-bake/tools/transcode_helix_signed360.py
Comment on lines
+102
to
+115
| const ver = dv.getUint16(4, true); | ||
| const posBytes = ver >= 4 ? 6 : 12; | ||
| const nC = dv.getUint32(6, true), nV = dv.getUint32(10, true), nT = dv.getUint32(14, true); | ||
| let o = 18; | ||
| o += 16 * nC; // guid | ||
| const matOff = o; o += nC; // material u8 (unused here) | ||
| const layerOff = o; o += nC; // LAYER u8 | ||
| o += 4 * nC; // label idx | ||
| o += 12 * nC; // centroid | ||
| o += 8 * nC; // vrange | ||
| const posOff = o; o += posBytes * nV; | ||
| const helixOff = o; o += 6 * nV; // pos3 | nrm3 — we read the nrm half | ||
| const rowOff = o; o += 4 * nV; | ||
| const idxOff = o; o += 12 * nT; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Reject non-v6 wires before decoding helix bytes.
fetchSoa() is canonical-only, but decode() still accepts older payload versions and will reinterpret the old 6-byte helix column as Signed360. A mistyped helix_latest or stale embedded asset would then render garbage instead of failing fast. Please gate this path on ver === 6 before reading the helix column.
Suggested guard
const ver = dv.getUint16(4, true);
+ if (ver !== 6) throw new Error(`expected BSO2 ver 6 helix wire, got ${ver}`);
const posBytes = ver >= 4 ? 6 : 12;📝 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
| const ver = dv.getUint16(4, true); | |
| const posBytes = ver >= 4 ? 6 : 12; | |
| const nC = dv.getUint32(6, true), nV = dv.getUint32(10, true), nT = dv.getUint32(14, true); | |
| let o = 18; | |
| o += 16 * nC; // guid | |
| const matOff = o; o += nC; // material u8 (unused here) | |
| const layerOff = o; o += nC; // LAYER u8 | |
| o += 4 * nC; // label idx | |
| o += 12 * nC; // centroid | |
| o += 8 * nC; // vrange | |
| const posOff = o; o += posBytes * nV; | |
| const helixOff = o; o += 6 * nV; // pos3 | nrm3 — we read the nrm half | |
| const rowOff = o; o += 4 * nV; | |
| const idxOff = o; o += 12 * nT; | |
| const ver = dv.getUint16(4, true); | |
| if (ver !== 6) throw new Error(`expected BSO2 ver 6 helix wire, got ${ver}`); | |
| const posBytes = ver >= 4 ? 6 : 12; | |
| const nC = dv.getUint32(6, true), nV = dv.getUint32(10, true), nT = dv.getUint32(14, true); | |
| let o = 18; | |
| o += 16 * nC; // guid | |
| const matOff = o; o += nC; // material u8 (unused here) | |
| const layerOff = o; o += nC; // LAYER u8 | |
| o += 4 * nC; // label idx | |
| o += 12 * nC; // centroid | |
| o += 8 * nC; // vrange | |
| const posOff = o; o += posBytes * nV; | |
| const helixOff = o; o += 6 * nV; // pos3 | nrm3 — we read the nrm half | |
| const rowOff = o; o += 4 * nV; | |
| const idxOff = o; o += 12 * nT; |
🤖 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 `@cockpit/src/BodyHelix.tsx` around lines 102 - 115, The decode path in
BodyHelix.decode is still parsing helix bytes for older payload versions, which
can misinterpret non-v6 data as Signed360. Add a strict version guard before the
helix column is read so only ver === 6 proceeds, and make the function fail fast
for any other wire version. Keep the check near the existing ver/posBytes setup
so the canonical-only behavior matches fetchSoa() and stale or mistyped assets
are rejected early.
Comment on lines
+363
to
+373
| const stRef = useRef<RenderState>({ enabled: new Float32Array([0, 0, 1, 1, 1, 1, 1, 1, 1]), alpha: 1, transparent: false, lodOn: false, selRow: -1, focus: null }); | ||
|
|
||
| useEffect(() => { | ||
| const e = new Float32Array(9); | ||
| for (let i = 1; i <= 8; i++) e[i] = on[i] ? 1 : 0; | ||
| stRef.current.enabled = e; | ||
| stRef.current.transparent = transparent; | ||
| // /fma-body translucency model: one uniform alpha for the WHOLE body. transparent | ||
| // ⇒ 0.42 x-ray (see through skin/muscle to organs); solid ⇒ 1.0 (#17 vessels only). | ||
| stRef.current.alpha = transparent ? 0.42 : 1.0; | ||
| // x-ray opacity is now SEMANTIC (per-compartment uLayerAlpha in the shader), so the | ||
| // whole-body uGlobalAlpha stays at 1.0 in both modes — it only scales #17 vessel | ||
| // blending in solid mode; x-ray ignores it entirely. | ||
| stRef.current.alpha = 1.0; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Stale x-ray label now that uGlobalAlpha is fixed at 1.0.
With whole-body alpha pinned to 1.0 and x-ray driven by per-compartment uLayerAlpha, the stats readout still advertises a flat 0.42 (Line 441: 'x-ray (whole body 0.42)'), which no longer reflects the rendered opacity. Update the label to match the semantic per-compartment behavior to avoid misleading users.
🤖 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 `@cockpit/src/BodyV3.tsx` around lines 363 - 373, The x-ray stats label in
BodyV3 is stale because `stRef.current.alpha` is now always set to 1.0 and x-ray
opacity is controlled by per-compartment `uLayerAlpha` instead of whole-body
alpha. Update the readout/label logic in `BodyV3` (the code that renders the
`'x-ray (whole body 0.42)'` text) so it no longer hardcodes 0.42 and instead
describes the semantic per-compartment x-ray behavior consistently with the
`useEffect` that sets `stRef.current.alpha`.
Comment on lines
+457
to
+459
| <span style={{ overflow: 'hidden', textOverflow: 'ellipsis', whiteSpace: 'nowrap' }}> | ||
| <span style={{ display: 'inline-block', width: 8, height: 8, borderRadius: 4, background: LAYERS[(c.layer - 1) % 8]?.color, marginRight: 6, verticalAlign: 'middle' }} />{c.name} | ||
| </span> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Ellipsis likely won't trigger on this flex item.
The name <span> carries overflow:hidden; text-overflow:ellipsis; white-space:nowrap, but as a direct child of the display:flex button it defaults to min-width:auto, so it won't shrink below its content width — long names will overflow/push the trailing layer label instead of truncating. Add minWidth: 0 (and let it flex) so the ellipsis applies.
🔧 Proposed fix
- <span style={{ overflow: 'hidden', textOverflow: 'ellipsis', whiteSpace: 'nowrap' }}>
+ <span style={{ minWidth: 0, flex: 1, overflow: 'hidden', textOverflow: 'ellipsis', whiteSpace: 'nowrap' }}>
<span style={{ display: 'inline-block', width: 8, height: 8, borderRadius: 4, background: LAYERS[(c.layer - 1) % 8]?.color, marginRight: 6, verticalAlign: 'middle' }} />{c.name}
</span>📝 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
| <span style={{ overflow: 'hidden', textOverflow: 'ellipsis', whiteSpace: 'nowrap' }}> | |
| <span style={{ display: 'inline-block', width: 8, height: 8, borderRadius: 4, background: LAYERS[(c.layer - 1) % 8]?.color, marginRight: 6, verticalAlign: 'middle' }} />{c.name} | |
| </span> | |
| <span style={{ minWidth: 0, flex: 1, overflow: 'hidden', textOverflow: 'ellipsis', whiteSpace: 'nowrap' }}> | |
| <span style={{ display: 'inline-block', width: 8, height: 8, borderRadius: 4, background: LAYERS[(c.layer - 1) % 8]?.color, marginRight: 6, verticalAlign: 'middle' }} />{c.name} | |
| </span> |
🤖 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 `@cockpit/src/BodyV3.tsx` around lines 457 - 459, The name span inside the flex
button in BodyV3.tsx will not reliably truncate because it still has the default
min-width:auto. Update that flex child so it can shrink (for example by giving
the span a flex style and minWidth: 0) while keeping the existing ellipsis
styles, so long channel names truncate instead of pushing the trailing layer
label.
Comment on lines
+109
to
+112
| {/* /helix — EXPERIMENTAL sibling of /body. Same baked wire, but shades from the | ||
| per-vertex helix-normal bytes (Fisher-2z geodesic codes) via a 256×256 LUT | ||
| materialized once at load: one vertex-shader fetch/vert, no per-vertex decode, | ||
| no rebake. Standalone (BodyHelix.tsx) so it can never break /body (#64). */} |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update this route comment to match the current BodyHelix implementation.
BodyHelix.tsx now does a one-time CPU decode into an Int8Array normal buffer; it is not doing a 256×256 LUT lookup or vertex-shader fetch on the current path. Keeping this comment in sync will save the next reader from chasing an implementation that no longer exists.
🤖 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 `@cockpit/src/main.tsx` around lines 109 - 112, The route comment in main.tsx
is outdated relative to BodyHelix.tsx. Update the /helix comment near the
BodyHelix route to describe the current implementation accurately: it now
performs a one-time CPU decode into an Int8Array normal buffer, rather than
using a 256×256 LUT or a vertex-shader fetch. Keep the comment aligned with the
BodyHelix component’s actual behavior and naming so future readers can find the
correct path quickly.
Comment on lines
+19
to
+20
| | `v5f16` | ver-5 wire, F16 (IEEE half) positions — current `/body` production | | ||
| | `v5f16h2` | same, helix-normal tuned (2-byte refinement validated) — `/helix` target | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update the helix docs to the current v6, canonical-only flow.
The naming table still frames /helix as v5f16h2, and this section still says BodyHelix falls back to body.soa.gz. In the shipped code, the manifest now points at body.20260629.v6helix.soa.gz, and cockpit/src/BodyHelix.tsx throws when helix_latest is missing instead of touching the shared v5 artifact. As written, the operator docs describe a rollout path the viewer no longer supports.
Also applies to: 44-46
🤖 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/osint-bake/tools/BAKE_ARTIFACTS.md` around lines 19 - 20, The helix
artifact docs are outdated and still describe the old v5 fallback flow. Update
the naming table and surrounding text in BAKE_ARTIFACTS.md to reflect the
current v6 canonical-only helix path, using the same symbols and terminology as
BodyHelix and the manifest entry for helix_latest. Remove the mention that
BodyHelix falls back to body.soa.gz, and describe that the viewer now expects
body.20260629.v6helix.soa.gz and fails when helix_latest is missing instead of
using the shared v5 artifact.
Comment on lines
+84
to
+87
| s360 = np.zeros((nV, 6), dtype=np.uint8) # rim bytes [0:3] = 0 (metric, unused) | ||
| s360[:, 3] = polar | ||
| s360[:, 4] = (az16 & 0xFF).astype(np.uint8) | ||
| s360[:, 5] = ((az16 >> 8) & 0xFF).astype(np.uint8) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Populate rim.end with something BodyHelix can actually decode.
cockpit/src/BodyHelix.tsx reconstructs rr from helix[1] and only falls back to polar when end >= 255 (Lines 160-165 there). With s360[:, 0:3] zeroed here, every transcoded vertex decodes with the first LUT bucket instead of its real radius. The check on Lines 89-96 hides that because it recomputes r from polar directly instead of using the viewer’s decode path. At minimum, write the saturation sentinel into rim.end for these non-canonical transcodes, or better yet quantize r and append the matching HXFL trailer.
Suggested minimal fix
- s360 = np.zeros((nV, 6), dtype=np.uint8) # rim bytes [0:3] = 0 (metric, unused)
+ s360 = np.zeros((nV, 6), dtype=np.uint8)
+ s360[:, 1] = 255 # force BodyHelix's polar fallback when no HXFL-encoded rim is presentAlso applies to: 89-96
🤖 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/osint-bake/tools/transcode_helix_signed360.py` around lines 84 - 87,
The transcode output in transcode_helix_signed360.py leaves the rim bytes
zeroed, so BodyHelix.tsx will decode every vertex using the first LUT bucket
instead of the intended radius. Update the s360 packing logic to write a valid
rim.end value that BodyHelix can interpret as a saturation sentinel (or quantize
the radius and emit the matching HXFL trailer), and make the validation path
around the existing radii check use the same viewer decode assumptions rather
than recomputing r directly from polar.
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
The
/helixviewer now renders the FMA body from the reallance-graph::helixcodec, with the Fisher-Z rim actually used, and at interactive frame rates.Bake — the real crate, against the real ndarray fork
helixbakebuilds the Signed360 normals withhelix::ResidueEncoder::encode_signed(the real crate, compiled against the localndarrayfork) instead of the hand-rolled transcoder that zeroed the Fisher-Z rim.rim.end = arctanh(r)quantized,r = sinθ— the "palette256 = the angle" carrier.HXFLtrailer = the exactRollingFloor (lo,hi)the encoder used, so the decoder dequantizes with the same floor. Same SoA wire, no sidecar.body.20260629.v6helix.soa.gzin thefma-body-soa-v3-v1release; the prior bake is kept, not deleted.Decode — uses the rim (the strength that was being reverted)
r = sinθis reconstructed fromrim.endvia a 256-entry r-LUT (the only placeatanh/tanhruns — at load, never per vertex). The polar partition gives the hemisphere sign and covers ther→1saturation cliff.Render — Gouraud (the 12 s/frame fix)
normalize/dot(and the per-vertex LUT texture fetch) that made it 12 s/frame.Verification
End-to-end against all 4,283,525 source normals (source → real
encode_signedFisher-Z rim → wire → browser rim-decode → int8 normal):cargo test(helixbake round-trip) green;tsc+vite buildclean.Scope / safety
/helixonly./body(BodyV3) shares nothing with this and is untouched.cockpit/src/BodyHelix.tsx,cockpit/public/body.manifest.json,Dockerfile. Thehelixbaketool + artifact live under gitignoredscratch-fma/; the artifact is in the release.🤖 Generated with Claude Code
https://claude.ai/code/session_01RhpwkHGgia2TuDFvdnuQdE
Generated by Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation