ray_tracing: place clouds purely through tf by jeff-hykin · Pull Request #3633 · dimensionalOS/dimos

jeff-hykin · 2026-08-24T05:29:54Z

Make ray tracing use TF so it can accept depth from multiple sources.

place clouds purely through tf; drop the odometry input
previously clouds were dropped at the odom pose, ignoring the sensor mount: a camera half a metre up and angled down put the floor in the wrong place if Odom was for base link (not the cam)
tf lookup at each cloud's stamp resolves world_frame -> sensor frame through the mount edge
cargoHash refresh to unstick the nix build

codecov · 2026-08-24T05:38:09Z

Codecov Report

❌ Patch coverage is 66.66667% with 5 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
dimos/cli/bake/test_bake_e2e.py	37.50%	5 Missing ⚠️

@@                 Coverage Diff                 @@
##           jeff/feat/dim_slam    #3633   +/-   ##
===================================================
  Coverage               77.78%   77.78%           
===================================================
  Files                    1293     1293           
  Lines                  123829   123834    +5     
  Branches                10851    10851           
===================================================
+ Hits                    96322    96329    +7     
+ Misses                  24366    24364    -2     
  Partials                 3141     3141

Flag	Coverage Δ
OS-ubuntu-24.04-arm	`72.77% <66.66%> (+<0.01%)`	⬆️
OS-ubuntu-latest	`74.62% <66.66%> (+<0.01%)`	⬆️
Py-3.10	`74.61% <66.66%> (+<0.01%)`	⬆️
Py-3.11	`74.61% <66.66%> (-0.01%)`	⬇️
Py-3.12	`74.61% <66.66%> (-0.01%)`	⬇️
Py-3.13	`74.61% <66.66%> (+<0.01%)`	⬆️
Py-3.14	`74.61% <66.66%> (+<0.01%)`	⬆️
Py-3.14t	`74.61% <66.66%> (+<0.01%)`	⬆️
SelfHosted-Large	`30.04% <66.66%> (+<0.01%)`	⬆️
SelfHosted-Linux	`35.25% <66.66%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
dimos/mapping/ray_tracing/module.py	`100.00% <100.00%> (ø)`
...avigation/nav_3d/mls_planner/mls_planner_native.py	`100.00% <100.00%> (ø)`
...e/go2/blueprints/navigation/unitree_go2_mls_htc.py	`83.33% <ø> (ø)`
...ee/go2/blueprints/navigation/unitree_go2_nav_3d.py	`81.48% <ø> (ø)`
dimos/robot/unitree/go2/zenoh/blueprints.py	`80.00% <ø> (ø)`
dimos/cli/bake/test_bake_e2e.py	`26.54% <37.50%> (+0.42%)`	⬆️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

greptile-apps · 2026-08-24T05:45:54Z

Greptile Summary

This change moves ray-tracing cloud placement to timestamped TF lookups and aligns map output-frame configuration across mapping and planner components. One reliability issue remains: a lidar cloud from a disconnected frame waits in the serial handler for the full TF timeout, delaying later clouds that already have valid transforms and risking delayed or lost map updates under load.

Confidence Score: 4/5

Not merge-safe for high-rate lidar operation until unresolved transform waits no longer serialize later transform-ready clouds.

There is exactly one accepted P1 finding, and it is a non-security reliability issue; the required score is 4.

Files Needing Attention: dimos/mapping/ray_tracing/rust/src/module.rs

T-Rex Logs

What T-Rex did

T-Rex produced a proof for the posted P1 finding.
Three artifacts were attached to the proof, including a disconnected-frame transport reproduction script and two test-output logs.
The attached artifacts enable validation against the review comment detailing the P1 finding.

_{Ran code and verified through T-Rex}

_{Reviews (23): Last reviewed commit: "ray_tracing: follow the memory2 -> memor..." | Re-trigger Greptile}

jeff-hykin · 2026-08-24T06:27:05Z

-/// Max stamp gap between a cloud and the pose used to register it (s).
-const POSE_MATCH_TOLERANCE_S: f64 = 0.1;
+/// Max age of a tf sample used to place a cloud (s).
+const TF_MATCH_TOLERANCE_S: f64 = 0.5;


This should be a config

Look up world_frame <- cloud.frame_id at each cloud stamp and register points and the ray origin through that one transform, fresh every cloud. The old path paired clouds with buffered odometry poses and assumed they were already in the body frame; the map now accumulates and publishes in config.world_frame instead, odom on alfred. Clouds must arrive in their own sensor frame and the odometry source must put its world->body edge on tf, which OdometryFusion does. The offline python binding takes pre-transformed points and passes an empty world_frame.

…elf-invalidates

The whole-repo flake input hash changes with any commit, invalidating the src derivation even though the build only consumes two crates from it. Re-copying those crates through builtins.path gives them content-addressed store paths, so the binary is reused until the crates themselves change.

dimos.memory.transform/type/store no longer exist in the tree, so these imports failed at runtime and under mypy.

RayTracingVoxelMap now places clouds through tf, so the harness's odometry publish left every cloud dropped and the planner never surfaced a map.

The odometry lookup this replaced matched within 0.1 s; the tf lookup went in at 0.5 s, which lets a half-second-stale transform place a cloud.

The recorded hash never matched what the vendor derivation produces, so nix build failed on any machine without the old value already cached.

Both configs called the field world_frame while the value is the odom frame, and the planner defaulted it to "map" though every caller overrode it. Rename to output_frame and default both to odom.

close_at_z allocates a dense byte image spanning the min/max voxel index of the slice. voxelize saturates out-of-range coordinates to i32::MAX, so a single corrupt point asked for a ~2.1e9-wide image; three orphaned planners held 220 GB each after a replay. Closing only bridges `pad` cells, so a slice that sparse gains nothing from it.

This reverts commit 394b86c.

Nothing orders tf against lidar, so a cloud that arrived first was dropped outright and its scan never reached the map. Clouds now wait a bounded number of frames for their transform.

…stamp A long tf outage leaves a backlog whose stamps are all older than the tolerance, and each one still burned the full wait, so the drain rate fell to one cloud per timeout and the map could stay dark after tf recovered.

…ery edge has passed it

A cloud older than the whole tf stream previously waited out the full tf_wait_timeout_s when the graph was empty on arrival, serializing later placeable clouds behind it. The give-up now lives in Lookup::within()'s wake loop, so it fires as soon as every edge has passed the stamp.

jeff-hykin requested review from leshy, paul-nechifor and spomichter as code owners August 24, 2026 05:29

jeff-hykin marked this pull request as draft August 24, 2026 05:33

jeff-hykin commented Aug 24, 2026

View reviewed changes

jeff-hykin marked this pull request as ready for review August 24, 2026 06:30

jeff-hykin requested review from Dreamsorcerer and mustafab0 as code owners August 24, 2026 13:20