cockpit/torso: land anisotropic + /torso-map + helix codec into main (stacked merges stayed in intermediate branches)#55
Merged
Conversation
The /torso splat3d frames used a big isotropic gaussian (scale 0.008) tuned for "solidity", which smeared the 231K-gaussian detail into a painterly "Warhol" look — while /torso-live (tiny points) showed the full ribcage/vertebrae. Same data, different brush. Re-render the turntable at scale 0.0025, 810x1080: the detail (ribcage, spine, organs) returns. Plan records the proper next step (anisotropic surface-fit gaussians + a third "map FMA" view). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TzqvDqbFRzyx17EkLKBoZF
The structural upgrade you greenlit: the torso splat stops being a detached
isotropic point cloud and becomes the GUID/value-tenant substrate seen as
oriented geometry. One pass over the BodyParts3D meshes does all of it.
SPL2 format (supersedes SPL1): hdr 40B [SPL2|count|node_count|radius|bbox];
body 21B [pos 3f | normal 3i8 | rgb 3u8 | opacity u8 | node_row u16]. Helix-
orderable + residual-ready (the Helix-48 codec slots in here next).
- Anisotropic surface-fit ("connect the dots"): BodyParts3D OBJ ships per-vertex
normals (vn) — free, no face traversal. The splat3d render driver orients each
gaussian flat-to-surface (scale=[t,t,thin], quat aligns local-z to the normal);
tangent 0.004 connects within a structure while rib gaps stay visible. Oriented
disks blend into continuous surfaces with real 3D form — not isotropic blobs,
not a discrete voxel grid.
- Per-node SoA + O(1) switch (the GUID backbone): torso.nodes.json = one row per
FMA structure (178 rows, 91 own meshes) carrying the value-tenants of one
identity — fma id, name, depth, HHTL tier-ranks, colour, gaussian RANGE
(start+count), and the OBJ-geometry tenant (centroid+bbox+FJ handles). Each
gaussian carries its node_row; consumers build the switch (row->node) once ->
O(1) tenant reads. Position = real BodyParts3D coordinate; identity = FMA node.
- /torso-map (TorsoMap.tsx): click a gaussian -> node_row -> O(1) into the SoA ->
FMA label + partonomy breadcrumb; structure list highlights its gaussians
(graph <-> splat). The osint-cad-splat thesis made literal: graph and splat,
one node at one address, switch-selected.
- /torso-live decodes SPL2 (still points); /torso shows the re-rendered
anisotropic turntable. main.tsx: /torso-map route.
Verified by viewing the rendered frames (connected anatomical surfaces, upright,
form from orientation). tsc clean; browser pick-interaction not exercised here
(raycast-on-Points is standard). Geometry: BodyParts3D, CC-BY 4.0 / CC-BY-SA 2.1
JP, (c) The Database Center for Life Science (attribution in-view).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TzqvDqbFRzyx17EkLKBoZF
tools/spl_codec.py — the "x265 for gaussians" the design converged on, as a MEASUREMENT tool that proves the structure before it is wired into render/anim. Maps the x265 pipeline onto signals already in SPL2 + torso.nodes.json: helix = 3D Morton of position = identity/GUID order (locality-preserving) anchor = FMA node (SoA centroid + per-node colour) = the I-frame, random-access motion = gaussian offset from its node anchor residual = helix-ordered zig-zag delta of (motion, normal) colour = ANCHOR-PREDICTED -> 0 per-gaussian bytes (node palette) Measured on the real torso (231,515 gaussians): SPL2 21.0 B/g -> SPL3 7.47 B/g = 2.8x smaller (zlib entropy stand-in) colour: exact, 887 B for ALL colour (crisp by construction, no boundary bleed) position round-trip RMSE 0.00001 (16-bit quant, effectively lossless) node_row RLE 35 KB / 231K (structures contiguous in helix order) streams: motion 1.02 MB, normal 671 KB (the target: octahedral + range coder) Validates "crisp colours without overhead" exactly: colour is fully predicted by the node anchor, so it costs 0 per-gaussian bytes and cannot bleed across a structure boundary. Next: octahedral normals + range coder; decode-at-load + anisotropic/edge-aware reconstruction; deform anchors -> animated anatomy. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TzqvDqbFRzyx17EkLKBoZF
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughThe PR upgrades the FMA torso gaussian splat pipeline from SPL1 to SPL2, adding per-vertex surface normals and per-gaussian node-row IDs. The bake script, binary decoder, and data artifacts are all updated. A new ChangesFMA Torso SPL2 Upgrade & Interactive Map
Sequence Diagram(s)sequenceDiagram
actor User
participant Browser as Browser (/torso-map)
participant TorsoMap as TorsoMap (React)
participant Fetch as fetch()
participant mount as mount() / Three.js
participant FMAOverlay as FMA Overlay UI
User->>Browser: navigate to /torso-map
Browser->>TorsoMap: mount component
TorsoMap->>Fetch: GET /torso.splat
Fetch-->>TorsoMap: ArrayBuffer → decodeSpl2(buf) → Spl2
TorsoMap->>Fetch: GET /torso.nodes.json
Fetch-->>TorsoMap: NodesDoc (nodes array)
TorsoMap->>mount: mount(container, splat, onPick, apiRef)
mount-->>TorsoMap: WebGL canvas rendered
User->>mount: click gaussian point
mount->>TorsoMap: onPick(row)
TorsoMap->>TorsoMap: setSelRow(row)
TorsoMap->>mount: apiRef.select(row) → update uSelected uniform
TorsoMap->>FMAOverlay: re-render detail panel + breadcrumb path
FMAOverlay-->>User: show FMA name, g_count, depth, tiers, breadcrumbs
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed due to a network error. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (1)
crates/osint-bake/tools/bake_torso_splat.py (1)
219-221: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPretty-print
torso.nodes.jsonlike the manifest.The generated node artifact is currently a single very large line, which makes future diffs and reviews hard to audit. Add
indent=2here and regenerate the JSON.Proposed fix
- json.dump({"attribution": ATTRIBUTION, "root": root, "radius": radius, - "count": n, "nodes": nodes}, f) + json.dump({"attribution": ATTRIBUTION, "root": root, "radius": radius, + "count": n, "nodes": nodes}, f, indent=2)🤖 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_torso_splat.py` around lines 219 - 221, The torso.nodes.json writer in bake_torso_splat.py is emitting compact one-line JSON, unlike the manifest, so update the json.dump call that writes torso.nodes.json to use pretty-printed output with indent=2. Keep the change localized to the torso node artifact generation path and regenerate the JSON output so the committed artifact matches the new formatting.
🤖 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-notes/plans/2026-06-24-fma-torso-bodyparts3d-splat.md`:
- Line 83: The document uses mixed spelling variants in the same note, so make
the wording consistent throughout this markdown plan. Update the sentence
containing “Realises” to match the chosen variant used elsewhere in the
document, and keep that same spelling style consistent across related prose in
this file.
In `@cockpit/src/TorsoMap.tsx`:
- Around line 241-243: The clickable breadcrumb/structure rows in TorsoMap are
rendered as divs, so they are not keyboard-operable; update the mapped row
markup in the path rendering and the related row block near the selectable node
UI to use accessible button semantics or add proper focus and key handling.
Ensure the row selection behavior still calls setSel for both mouse and keyboard
interactions, and preserve the existing visual styling while making the elements
reachable and activatable from the keyboard.
- Around line 176-182: The selection state in TorsoMap is not replayed after
mount, so a preselected node can remain unhighlighted when mount() later
initializes apiRef.current. Update the TorsoMap useEffect pair so the current
sel is applied again once the WebGL mount exists, not only when sel changes; use
the existing mount() callback and the apiRef.current?.select(sel ?? -1) logic to
trigger selection after splat setup. Ensure the fix is anchored in TorsoMap and
its mount/apiRef/sel effects so the highlight syncs whenever the viewer becomes
ready.
- Around line 37-47: The decodeSpl2 function trusts the SPL2 header count before
verifying the buffer size, which can lead to oversized allocations or crashes on
truncated assets. Update decodeSpl2 in TorsoMap to validate the ArrayBuffer
length against the expected SPL2 record size using the parsed count before
creating the positions, colors, and rows arrays, and reject invalid buffers with
a clear error if the body length does not match.
- Around line 169-172: The torso nodes load in TorsoMap currently swallows
non-OK responses and fetch errors, so make it behave like the splat fetch path
by surfacing failures instead of returning null. Update the fetch chain in
TorsoMap to treat a missing or stale torso.nodes.json as an error, and handle it
through the same rejection/logging flow used by the existing splat loading logic
so the failure is visible and actionable.
- Around line 78-82: The fragment shader in TorsoMap’s point rendering uses an
invalid smoothstep edge order, so update the alpha computation in the shader
string inside the TorsoMap component to use ordered edges with a normal
ascending smoothstep call. Keep the existing discard logic and make the change
at the gl_FragColor alpha calculation so the point sprite fades consistently
across drivers.
In `@cockpit/src/TorsoSplat.tsx`:
- Around line 37-53: The decodeSpl1() path in TorsoSplat.tsx should validate the
SPL2 buffer length immediately after reading the header count, before creating
the typed arrays. Compute the expected body size from count and the per-point
record layout, compare it against buf.byteLength, and throw a clear error for
truncated or inconsistent data so a stale or corrupt /torso.splat cannot trigger
oversized allocations or a mid-loop failure.
In `@crates/osint-bake/tools/bake_torso_splat.py`:
- Around line 159-161: Remove the remaining Ruff violations in
bake_torso_splat.py by replacing the semicolon-chained append assignments inside
the torso/vertex processing logic with separate statements, using the same
gx/gy/gz, gnx/gny/gnz, and gr/gg/gb append calls in sequence, and update qi8()
to return round(v * 127) directly without wrapping it in int().
- Around line 136-145: Validate the optional CLI budget in bake_torso_splat.py
before computing stride in the pass-1 budget calculation. In the code around
total_v, vcache, and the stride assignment, reject any non-positive budget
(especially 0) with an explicit error or early exit before calling round(total_v
/ budget), so the ZeroDivisionError cannot occur.
In `@crates/osint-bake/tools/spl_codec.py`:
- Around line 83-84: The row-to-node contract is assumed in the colour/anchor
lookup inside the SPL codec, which can raise a KeyError when `row[i]` is missing
from `node_row`. Add an upfront validation in the affected `spl_codec.py` logic
before the palette/centroid access, checking that all required rows are present
in `node_row` for the current SPL2 data. If any rows are missing, fail fast with
a clear contract error that lists the missing row IDs, and apply the same guard
to the related lookup path referenced by the colour/anchor computation.
- Around line 46-51: The SPL2 parsing logic currently relies on assert and
unchecked struct.unpack_from calls, which can produce opaque failures on
malformed or truncated inputs. In the SPL2 header/body parsing code in
spl_codec.py, replace the assert-based magic check with explicit validation of
the magic bytes, and verify the buffer is long enough before reading count,
bmin, bmax, and the remaining body data. Make the failure paths deterministic by
raising a clear format/size validation error before any unpacking, and apply the
same validation pattern to the later parsing block mentioned in the review.
- Line 117: The RLE packing and RMSE calculation logic in spl_codec.py needs to
handle empty input safely and use the real sampled-point count for
normalization. Add an early return or equivalent guard before the rle.append
path when count is 0 so you don’t build invalid runs or divide by zero, and
update the RMSE computation currently using count/7 to normalize by the actual
number of sampled points instead. Apply the same fix anywhere the same
packing/normalization logic is duplicated, including the related code in the
referenced later range.
---
Nitpick comments:
In `@crates/osint-bake/tools/bake_torso_splat.py`:
- Around line 219-221: The torso.nodes.json writer in bake_torso_splat.py is
emitting compact one-line JSON, unlike the manifest, so update the json.dump
call that writes torso.nodes.json to use pretty-printed output with indent=2.
Keep the change localized to the torso node artifact generation path and
regenerate the JSON output so the committed artifact matches the new formatting.
🪄 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: 0fe641f9-4dc5-4d83-978d-bcebdef6691a
⛔ Files ignored due to path filters (20)
cockpit/public/torso-frames/torso_000.jpgis excluded by!**/*.jpgcockpit/public/torso-frames/torso_001.jpgis excluded by!**/*.jpgcockpit/public/torso-frames/torso_002.jpgis excluded by!**/*.jpgcockpit/public/torso-frames/torso_003.jpgis excluded by!**/*.jpgcockpit/public/torso-frames/torso_004.jpgis excluded by!**/*.jpgcockpit/public/torso-frames/torso_005.jpgis excluded by!**/*.jpgcockpit/public/torso-frames/torso_006.jpgis excluded by!**/*.jpgcockpit/public/torso-frames/torso_007.jpgis excluded by!**/*.jpgcockpit/public/torso-frames/torso_008.jpgis excluded by!**/*.jpgcockpit/public/torso-frames/torso_009.jpgis excluded by!**/*.jpgcockpit/public/torso-frames/torso_010.jpgis excluded by!**/*.jpgcockpit/public/torso-frames/torso_011.jpgis excluded by!**/*.jpgcockpit/public/torso-frames/torso_012.jpgis excluded by!**/*.jpgcockpit/public/torso-frames/torso_013.jpgis excluded by!**/*.jpgcockpit/public/torso-frames/torso_014.jpgis excluded by!**/*.jpgcockpit/public/torso-frames/torso_015.jpgis excluded by!**/*.jpgcockpit/public/torso-frames/torso_016.jpgis excluded by!**/*.jpgcockpit/public/torso-frames/torso_017.jpgis excluded by!**/*.jpgcockpit/public/torso-frames/torso_018.jpgis excluded by!**/*.jpgcockpit/public/torso-frames/torso_019.jpgis excluded by!**/*.jpg
📒 Files selected for processing (9)
claude-notes/plans/2026-06-24-fma-torso-bodyparts3d-splat.mdcockpit/public/torso.manifest.jsoncockpit/public/torso.nodes.jsoncockpit/public/torso.splatcockpit/src/TorsoMap.tsxcockpit/src/TorsoSplat.tsxcockpit/src/main.tsxcrates/osint-bake/tools/bake_torso_splat.pycrates/osint-bake/tools/spl_codec.py
| reads. Position = real BodyParts3D coordinate; identity = the FMA node. | ||
| - [x] **/torso-map page**: click a gaussian -> node_row -> node SoA -> FMA label + | ||
| partonomy breadcrumb; structure list (graph -> splat) highlights gaussians. | ||
| Realises the osint-cad-splat thesis: graph and splat, one node at one address. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keep spelling variant consistent in the document
Use one variant consistently (e.g., “Realizes” vs “Realises”) to avoid mixed-style docs.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~83-~83: Do not mix variants of the same word (‘realise’ and ‘realize’) within a single text.
Context: ...h -> splat) highlights gaussians. Realises the osint-cad-splat thesis: graph and s...
(EN_WORD_COHERENCY)
🤖 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-notes/plans/2026-06-24-fma-torso-bodyparts3d-splat.md` at line 83, The
document uses mixed spelling variants in the same note, so make the wording
consistent throughout this markdown plan. Update the sentence containing
“Realises” to match the chosen variant used elsewhere in the document, and keep
that same spelling style consistent across related prose in this file.
Source: Linters/SAST tools
Comment on lines
+37
to
+47
| function decodeSpl2(buf: ArrayBuffer): Spl2 { | ||
| const dv = new DataView(buf); | ||
| const magic = String.fromCharCode(dv.getUint8(0), dv.getUint8(1), dv.getUint8(2), dv.getUint8(3)); | ||
| if (magic !== 'SPL2') throw new Error(`bad magic "${magic}" (expected SPL2)`); | ||
| const count = dv.getUint32(4, true); | ||
| const off = 40; | ||
| const positions = new Float32Array(count * 3); | ||
| const colors = new Float32Array(count * 3); | ||
| const rows = new Float32Array(count); | ||
| for (let i = 0; i < count; i++) { | ||
| const b = off + i * 21; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Validate SPL2 size before trusting count.
count comes from the binary asset and is used to allocate arrays before any body-length check. A truncated or mismatched SPL2 artifact can crash the page or allocate excessively.
Proposed guard
function decodeSpl2(buf: ArrayBuffer): Spl2 {
+ if (buf.byteLength < 40) {
+ throw new Error(`SPL2 file too short: ${buf.byteLength} bytes`);
+ }
const dv = new DataView(buf);
const magic = String.fromCharCode(dv.getUint8(0), dv.getUint8(1), dv.getUint8(2), dv.getUint8(3));
if (magic !== 'SPL2') throw new Error(`bad magic "${magic}" (expected SPL2)`);
const count = dv.getUint32(4, true);
+ const expectedBytes = 40 + count * 21;
+ if (buf.byteLength < expectedBytes) {
+ throw new Error(`SPL2 file truncated: ${buf.byteLength} bytes, expected at least ${expectedBytes}`);
+ }
const off = 40;📝 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
| function decodeSpl2(buf: ArrayBuffer): Spl2 { | |
| const dv = new DataView(buf); | |
| const magic = String.fromCharCode(dv.getUint8(0), dv.getUint8(1), dv.getUint8(2), dv.getUint8(3)); | |
| if (magic !== 'SPL2') throw new Error(`bad magic "${magic}" (expected SPL2)`); | |
| const count = dv.getUint32(4, true); | |
| const off = 40; | |
| const positions = new Float32Array(count * 3); | |
| const colors = new Float32Array(count * 3); | |
| const rows = new Float32Array(count); | |
| for (let i = 0; i < count; i++) { | |
| const b = off + i * 21; | |
| function decodeSpl2(buf: ArrayBuffer): Spl2 { | |
| if (buf.byteLength < 40) { | |
| throw new Error(`SPL2 file too short: ${buf.byteLength} bytes`); | |
| } | |
| const dv = new DataView(buf); | |
| const magic = String.fromCharCode(dv.getUint8(0), dv.getUint8(1), dv.getUint8(2), dv.getUint8(3)); | |
| if (magic !== 'SPL2') throw new Error(`bad magic "${magic}" (expected SPL2)`); | |
| const count = dv.getUint32(4, true); | |
| const expectedBytes = 40 + count * 21; | |
| if (buf.byteLength < expectedBytes) { | |
| throw new Error(`SPL2 file truncated: ${buf.byteLength} bytes, expected at least ${expectedBytes}`); | |
| } | |
| const off = 40; | |
| const positions = new Float32Array(count * 3); | |
| const colors = new Float32Array(count * 3); | |
| const rows = new Float32Array(count); | |
| for (let i = 0; i < count; i++) { | |
| const b = off + i * 21; |
🤖 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/TorsoMap.tsx` around lines 37 - 47, The decodeSpl2 function
trusts the SPL2 header count before verifying the buffer size, which can lead to
oversized allocations or crashes on truncated assets. Update decodeSpl2 in
TorsoMap to validate the ArrayBuffer length against the expected SPL2 record
size using the parsed count before creating the positions, colors, and rows
arrays, and reject invalid buffers with a clear error if the body length does
not match.
Comment on lines
+78
to
+82
| void main() { | ||
| vec2 c = gl_PointCoord - 0.5; | ||
| if (dot(c, c) > 0.25) discard; | ||
| float a = smoothstep(0.25, 0.05, dot(c, c)); | ||
| gl_FragColor = vec4(vColor * vDim, a); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🌐 Web query:
GLSL ES smoothstep edge0 edge1 undefined if edge0 is greater than or equal to edge1
💡 Result:
In GLSL ES, smoothstep(edge0, edge1, x) has undefined results when edge0 is greater than or equal to edge1; i.e., the specification states results are undefined if edge0 >= edge1. Therefore behavior in that case (including your stated “edge0 >= edge1”) is not guaranteed and may vary by compiler/GPU. [1][2] What is defined (for reference): it returns 0.0 when x <= edge0 and 1.0 when x >= edge1, and only performs the smooth Hermite interpolation when edge0 < x < edge1. [1][2] Practical fix: ensure edge0 < edge1 (swap them or reorder your inputs), or implement your own definition that handles edge0 >= edge1 deterministically rather than relying on smoothstep’s undefined behavior. The community discussion and the spec quote both recommend not relying on edge0 >= edge1. [2][3]
Citations:
- 1: https://www.facebook.com/groups/IndieGameDevs/posts/10157177830911573/
- 2: https://discussions.unity.com/t/how-expensive-is-smooth-step-in-shaders-for-mobile/680914
- 3: https://www.youtube.com/watch?v=w7c41h1g2_k
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant file and surrounding lines.
sed -n '1,140p' cockpit/src/TorsoMap.tsx | cat -n
# Find the exact smoothstep usage(s) in the repository.
rg -n "smoothstep\\(" cockpit/src/TorsoMap.tsx cockpit/src || trueRepository: AdaWorldAPI/q2
Length of output: 6474
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant file and surrounding lines.
sed -n '1,140p' cockpit/src/TorsoMap.tsx | cat -n
# Find the exact smoothstep usage(s) in the repository.
rg -n "smoothstep\\(" cockpit/src/TorsoMap.tsx cockpit/src || trueRepository: AdaWorldAPI/q2
Length of output: 6474
Use ordered smoothstep edges in cockpit/src/TorsoMap.tsx:81. smoothstep(0.25, 0.05, dot(c, c)) is undefined because GLSL expects edge0 < edge1, so the point alpha can vary by driver. Invert a normal ordered call instead.
Proposed shader fix
- float a = smoothstep(0.25, 0.05, dot(c, c));
+ float a = 1.0 - smoothstep(0.05, 0.25, dot(c, c));📝 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
| void main() { | |
| vec2 c = gl_PointCoord - 0.5; | |
| if (dot(c, c) > 0.25) discard; | |
| float a = smoothstep(0.25, 0.05, dot(c, c)); | |
| gl_FragColor = vec4(vColor * vDim, a); | |
| void main() { | |
| vec2 c = gl_PointCoord - 0.5; | |
| if (dot(c, c) > 0.25) discard; | |
| float a = 1.0 - smoothstep(0.05, 0.25, dot(c, c)); | |
| gl_FragColor = vec4(vColor * vDim, a); |
🤖 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/TorsoMap.tsx` around lines 78 - 82, The fragment shader in
TorsoMap’s point rendering uses an invalid smoothstep edge order, so update the
alpha computation in the shader string inside the TorsoMap component to use
ordered edges with a normal ascending smoothstep call. Keep the existing discard
logic and make the change at the gl_FragColor alpha calculation so the point
sprite fades consistently across drivers.
Comment on lines
+169
to
+172
| fetch('/torso.nodes.json') | ||
| .then((r) => (r.ok ? r.json() : null)) | ||
| .then((d) => !cancelled && d && setDoc(d as NodesDoc)) | ||
| .catch(() => {}); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Surface torso.nodes.json load failures.
A missing or stale nodes document currently fails silently, leaving /torso-map as an unlinked point cloud with no actionable error. Treat non-OK responses like the splat fetch path.
Proposed error handling
fetch('/torso.nodes.json')
- .then((r) => (r.ok ? r.json() : null))
- .then((d) => !cancelled && d && setDoc(d as NodesDoc))
- .catch(() => {});
+ .then((r) => {
+ if (!r.ok) throw new Error(`HTTP ${r.status} torso.nodes.json`);
+ return r.json();
+ })
+ .then((d) => !cancelled && setDoc(d as NodesDoc))
+ .catch((e) => !cancelled && setError(String(e)));📝 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
| fetch('/torso.nodes.json') | |
| .then((r) => (r.ok ? r.json() : null)) | |
| .then((d) => !cancelled && d && setDoc(d as NodesDoc)) | |
| .catch(() => {}); | |
| fetch('/torso.nodes.json') | |
| .then((r) => { | |
| if (!r.ok) throw new Error(`HTTP ${r.status} torso.nodes.json`); | |
| return r.json(); | |
| }) | |
| .then((d) => !cancelled && d && setDoc(d as NodesDoc)) | |
| .catch((e) => !cancelled && setError(String(e))); |
🤖 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/TorsoMap.tsx` around lines 169 - 172, The torso nodes load in
TorsoMap currently swallows non-OK responses and fetch errors, so make it behave
like the splat fetch path by surfacing failures instead of returning null.
Update the fetch chain in TorsoMap to treat a missing or stale torso.nodes.json
as an error, and handle it through the same rejection/logging flow used by the
existing splat loading logic so the failure is visible and actionable.
Comment on lines
+176
to
+182
| useEffect(() => { | ||
| const c = ref.current; | ||
| if (!c || !splat) return; | ||
| return mount(c, splat, (row) => setSel(row), apiRef); | ||
| }, [splat]); | ||
|
|
||
| useEffect(() => { apiRef.current?.select(sel ?? -1); }, [sel]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Replay the current selection after the WebGL mount is created.
If torso.nodes.json loads first, a user can select a structure before splat mounts. When mount() later sets apiRef.current, the [sel] effect does not rerun, so the UI shows a selected node but the splat remains unhighlighted until the selection changes again.
Proposed sync fix
useEffect(() => {
const c = ref.current;
if (!c || !splat) return;
- return mount(c, splat, (row) => setSel(row), apiRef);
+ const dispose = mount(c, splat, (row) => setSel(row), apiRef);
+ apiRef.current?.select(sel ?? -1);
+ return dispose;
}, [splat]);📝 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
| useEffect(() => { | |
| const c = ref.current; | |
| if (!c || !splat) return; | |
| return mount(c, splat, (row) => setSel(row), apiRef); | |
| }, [splat]); | |
| useEffect(() => { apiRef.current?.select(sel ?? -1); }, [sel]); | |
| useEffect(() => { | |
| const c = ref.current; | |
| if (!c || !splat) return; | |
| const dispose = mount(c, splat, (row) => setSel(row), apiRef); | |
| apiRef.current?.select(sel ?? -1); | |
| return dispose; | |
| }, [splat]); | |
| useEffect(() => { apiRef.current?.select(sel ?? -1); }, [sel]); |
🤖 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/TorsoMap.tsx` around lines 176 - 182, The selection state in
TorsoMap is not replayed after mount, so a preselected node can remain
unhighlighted when mount() later initializes apiRef.current. Update the TorsoMap
useEffect pair so the current sel is applied again once the WebGL mount exists,
not only when sel changes; use the existing mount() callback and the
apiRef.current?.select(sel ?? -1) logic to trigger selection after splat setup.
Ensure the fix is anchored in TorsoMap and its mount/apiRef/sel effects so the
highlight syncs whenever the viewer becomes ready.
Comment on lines
+136
to
+145
| # pass 1: total vertex count -> global stride for the budget | ||
| total_v = 0 | ||
| vcache = {} | ||
| for fma in order: | ||
| for fj in meshes_of.get(fma, []): | ||
| vs, ns = read_obj_v_vn(os.path.join(obj_dir, fj + ".obj")) | ||
| vcache[fj] = (vs, ns) | ||
| total_v += len(vs) | ||
| stride = max(1, round(total_v / budget)) | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Validate budget before dividing by it.
Line 144 crashes with ZeroDivisionError when the optional CLI budget is 0; reject non-positive budgets before the stride calculation.
Proposed fix
# pass 1: total vertex count -> global stride for the budget
+ if budget <= 0:
+ sys.exit("budget must be a positive integer")
+
total_v = 0📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # pass 1: total vertex count -> global stride for the budget | |
| total_v = 0 | |
| vcache = {} | |
| for fma in order: | |
| for fj in meshes_of.get(fma, []): | |
| vs, ns = read_obj_v_vn(os.path.join(obj_dir, fj + ".obj")) | |
| vcache[fj] = (vs, ns) | |
| total_v += len(vs) | |
| stride = max(1, round(total_v / budget)) | |
| # pass 1: total vertex count -> global stride for the budget | |
| if budget <= 0: | |
| sys.exit("budget must be a positive integer") | |
| total_v = 0 | |
| vcache = {} | |
| for fma in order: | |
| for fj in meshes_of.get(fma, []): | |
| vs, ns = read_obj_v_vn(os.path.join(obj_dir, fj + ".obj")) | |
| vcache[fj] = (vs, ns) | |
| total_v += len(vs) | |
| stride = max(1, round(total_v / budget)) |
🤖 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_torso_splat.py` around lines 136 - 145, Validate
the optional CLI budget in bake_torso_splat.py before computing stride in the
pass-1 budget calculation. In the code around total_v, vcache, and the stride
assignment, reject any non-positive budget (especially 0) with an explicit error
or early exit before calling round(total_v / budget), so the ZeroDivisionError
cannot occur.
Comment on lines
+159
to
+161
| gx.append(x); gy.append(y); gz.append(z) | ||
| gnx.append(nx); gny.append(ny); gnz.append(nz) | ||
| gr.append(col[0]); gg.append(col[1]); gb.append(col[2]) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Expect: no E702/RUF046 findings for the bake script.
ruff check crates/osint-bake/tools/bake_torso_splat.pyRepository: AdaWorldAPI/q2
Length of output: 8586
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant section and check whether Ruff still reports RUF046 there.
sed -n '150,210p' crates/osint-bake/tools/bake_torso_splat.py
printf '\n---\n'
ruff check --select RUF046 crates/osint-bake/tools/bake_torso_splat.pyRepository: AdaWorldAPI/q2
Length of output: 3355
Remove the remaining Ruff violations in crates/osint-bake/tools/bake_torso_splat.py. Split the semicolon-chained assignments at lines 159-194 into separate statements, and drop the redundant int() in qi8() at line 198 (round(v * 127) already returns an integer).
🧰 Tools
🪛 Ruff (0.15.18)
[error] 159-159: Multiple statements on one line (semicolon)
(E702)
[error] 159-159: Multiple statements on one line (semicolon)
(E702)
[error] 160-160: Multiple statements on one line (semicolon)
(E702)
[error] 160-160: Multiple statements on one line (semicolon)
(E702)
[error] 161-161: Multiple statements on one line (semicolon)
(E702)
[error] 161-161: Multiple statements on one line (semicolon)
(E702)
🤖 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_torso_splat.py` around lines 159 - 161, Remove
the remaining Ruff violations in bake_torso_splat.py by replacing the
semicolon-chained append assignments inside the torso/vertex processing logic
with separate statements, using the same gx/gy/gz, gnx/gny/gnz, and gr/gg/gb
append calls in sequence, and update qi8() to return round(v * 127) directly
without wrapping it in int().
Source: Linters/SAST tools
Comment on lines
+46
to
+51
| raw = open(spl_path, "rb").read() | ||
| assert raw[:4] == b"SPL2" | ||
| count = struct.unpack_from("<I", raw, 4)[0] | ||
| bmin = struct.unpack_from("<3f", raw, 16) | ||
| bmax = struct.unpack_from("<3f", raw, 28) | ||
| off = 40 |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Replace assert parsing with explicit SPL2 size/format validation
This currently fails with opaque runtime errors on malformed/truncated inputs. Validate magic/header/body length before unpacking so failures are deterministic and actionable.
Proposed fix
- raw = open(spl_path, "rb").read()
- assert raw[:4] == b"SPL2"
+ raw = open(spl_path, "rb").read()
+ if len(raw) < 40:
+ raise ValueError("SPL2 file too small: missing 40-byte header")
+ if raw[:4] != b"SPL2":
+ raise ValueError(f'bad magic {raw[:4]!r} (expected b"SPL2")')
count = struct.unpack_from("<I", raw, 4)[0]
+ expected = 40 + count * 21
+ if len(raw) < expected:
+ raise ValueError(f"SPL2 truncated: expected at least {expected} bytes, got {len(raw)}")Also applies to: 59-66
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] 51-51: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(nodes_path)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
🤖 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/spl_codec.py` around lines 46 - 51, The SPL2 parsing
logic currently relies on assert and unchecked struct.unpack_from calls, which
can produce opaque failures on malformed or truncated inputs. In the SPL2
header/body parsing code in spl_codec.py, replace the assert-based magic check
with explicit validation of the magic bytes, and verify the buffer is long
enough before reading count, bmin, bmax, and the remaining body data. Make the
failure paths deterministic by raising a clear format/size validation error
before any unpacking, and apply the same validation pattern to the later parsing
block mentioned in the review.
Comment on lines
+83
to
+84
| colour_exact = all(rgb[i] == ((palette[row[i]][0] << 16) | (palette[row[i]][1] << 8) | palette[row[i]][2]) | ||
| for i in range(count)) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Validate node_row coverage before palette/centroid lookup
If SPL2 rows and torso.nodes.json rows drift, this crashes with KeyError during colour/anchor access. Fail fast with a contract error listing missing rows.
Proposed fix
palette = {}
for nd in nodes:
palette[nd["row"]] = nd.get("rgb", [180, 180, 180])
+ known_rows = set(palette.keys()) & set(centroid.keys())
+ missing_rows = sorted(set(row) - known_rows)
+ if missing_rows:
+ raise ValueError(f"SPL2 node_row values not found in torso.nodes.json: {missing_rows[:16]}")Also applies to: 98-99
🤖 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/spl_codec.py` around lines 83 - 84, The row-to-node
contract is assumed in the colour/anchor lookup inside the SPL codec, which can
raise a KeyError when `row[i]` is missing from `node_row`. Add an upfront
validation in the affected `spl_codec.py` logic before the palette/centroid
access, checking that all required rows are present in `node_row` for the
current SPL2 data. If any rows are missing, fail fast with a clear contract
error that lists the missing row IDs, and apply the same guard to the related
lookup path referenced by the colour/anchor computation.
| if prev_row >= 0: | ||
| rle.append((prev_row, run)) | ||
| prev_row, run = row[i], 1 | ||
| rle.append((prev_row, run)) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fix empty-input crash and RMSE normalization bias
count == 0 currently creates invalid RLE packing/division behavior, and RMSE uses count/7 instead of the actual sampled point count.
Proposed fix
- rle.append((prev_row, run))
+ if prev_row >= 0:
+ rle.append((prev_row, run))
@@
- rec_err = 0.0
- for i in range(0, count, 7):
+ rec_err = 0.0
+ samples = 0
+ for i in range(0, count, 7):
rx = (qx[i] / 65535) * span[0] + bmin[0]
ry = (qy[i] / 65535) * span[1] + bmin[1]
rz = (qz[i] / 65535) * span[2] + bmin[2]
rec_err += (rx - px[i]) ** 2 + (ry - py[i]) ** 2 + (rz - pz[i]) ** 2
+ samples += 1
import math
- rmse = math.sqrt(rec_err / (count / 7 * 3))
+ rmse = 0.0 if samples == 0 else math.sqrt(rec_err / (samples * 3))Also applies to: 130-137
🤖 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/spl_codec.py` at line 117, The RLE packing and RMSE
calculation logic in spl_codec.py needs to handle empty input safely and use the
real sampled-point count for normalization. Add an early return or equivalent
guard before the rle.append path when count is 0 so you don’t build invalid runs
or divide by zero, and update the RMSE computation currently using count/7 to
normalize by the actual number of sampled points instead. Apply the same fix
anywhere the same packing/normalization logic is duplicated, including the
related code in the referenced later range.
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 — lands the stranded torso stack into
mainPRs #53 (anisotropic +
/torso-map) and #54 (helix codec) were merged in stacked order, but into their intermediate base branches rather than up tomain. Net effect:claude/fma-torso-splat/claude/torso-anisotropic-map/claude/torso-helix-codecall contain the full work, butmainstill has only #52 (the older SPL1 / isotropic torso). This PR carries the rest up tomain.Conflict-free — verified with
git merge-tree(exit 0). main and this branch sharef197cab5(#52); main added nothing since the #52 merge, so the delta applies cleanly.This is purely a land-the-already-reviewed-work PR — no new code beyond #53 + #54, which were already reviewed and merged downstack:
/torso-map(pick → FMA, graph ↔ splat) + SPL2 format (normal-carrying, node-row-tagged).29 files:
TorsoMap.tsx,spl_codec.py,torso.nodes.json, SPL2torso.splat, the anisotropic frames,bake_torso_splat.pyv2, the SPL2 decoder,/torso-maproute.After this merges,
mainis complete and the three intermediate branches can be deleted.🤖 Generated with Claude Code
https://claude.ai/code/session_01TzqvDqbFRzyx17EkLKBoZF
Generated by Claude Code
Summary by CodeRabbit