ray_tracing: salvage tf-wait and darwin build fixes by jeff-hykin · Pull Request #3708 · dimensionalOS/dimos

jeff-hykin · 2026-08-26T23:51:43Z

make tf_wait_timeout_s configurable (for slow-moving robots with slow odom)
MacOS nix build fix

…/ray_tracing_tf Give up a tf wait early once every edge has passed the requested stamp, make the wait timeout a config field, and fix the darwin nix build (bin-only, no python cdylib link) with cache-stable crate store paths.

greptile-apps · 2026-08-27T00:03:49Z

Greptile Summary

This change makes the ray-tracing transform wait configurable, adds an early return for timestamped transform lookups, and updates the native Nix build path. Focused Rust checks reproduced two failures: Lookup::within can drop a cloud before a valid out-of-order transform arrives within the configured wait, and a finite but oversized configured wait duration panics during point-cloud handling. The early-return condition and timeout validation need correction before merge.

Confidence Score: 3/5

Not safe to merge until transform waits preserve valid late samples and invalid timeout values cannot crash point-cloud processing.

Two independent runtime failures were reproduced with focused Rust tests that exercised the relevant production behavior and included normal control inputs.

Files Needing Attention: native/rust/dimos-module/src/tf.rs needs a safe completion condition for transform waits, and dimos/mapping/ray_tracing/rust/src/voxel_ray_tracer.rs needs representable timeout validation or checked conversion.

T-Rex Logs

What T-Rex did

T-Rex posted a P1 finding with a focused Rust regression test source for the delayed out-of-order transform and an accompanying stack trace.
T-Rex provided a focused Rust reproduction test source for the P1 finding and included baseline and oversized timeout reproduction outputs to illustrate the timeout behavior.
T-Rex posted another P1 finding with no artifacts attached.
T-Rex validated the contract by running the regression-test for the delayed out-of-order scenario and observed a failure at the expected assertion in the after log.
T-Rex validated the finite timeout conversion by executing the before and after tests described in the artifacts, noting that 0.1 was accepted and that the large value caused a panic.

_{Ran code and verified through T-Rex}

Comments Outside Diff (2)

General comment

Lookup::within incorrectly abandons a valid out-of-order transform
- Bug
  - A lookup at timestamp 5.0 with tolerance 0.1 returned None immediately when its current latest edge was 5.2. A delayed 5.05 transform, which is both inside the 10-second buffer window and inside the lookup tolerance, was subsequently accepted by TBuffer and resolved by Lookup::get.
- Cause
  - no_sample_can_still_arrive assumes edges only append forward in time and returns true when the latest composed path timestamp is greater than time + tolerance. However, TBuffer::add inserts out-of-order samples in timestamp order when they remain inside its history window.
- Fix
  - Do not use the latest edge timestamp as a terminal condition for timed lookups while TBuffer accepts in-window out-of-order samples. Wait for the deadline (or introduce a proven watermark/order guarantee that rejects such late samples consistently).
_{Ran code and verified through T-Rex}
General comment

Finite oversized TF wait timeout is accepted and panics on point-cloud handling
- Bug
  - Config::validate() accepts tf_wait_timeout_s = 1e300, but processing a point cloud reaches Duration::from_secs_f64(self.config.tf_wait_timeout_s) and panics because the float cannot be represented as a Duration.
- Cause
  - The field validator at voxel_ray_tracer.rs:100 enforces only a non-negative lower bound and has no upper bound compatible with Duration::from_secs_f64.
- Fix
  - Add validation that caps tf_wait_timeout_s at the maximum finite seconds value accepted by Duration::from_secs_f64 (and reject non-finite values), or replace the conversion with checked error handling that drops/rejects invalid configuration rather than panicking.
_{Ran code and verified through T-Rex}

_{Reviews (1): Last reviewed commit: "ray_tracing: salvage the tf-wait and dar..." | Re-trigger Greptile}

greptile-apps · 2026-08-27T00:03:52Z

+            // Edges only append forward in time, so once the stalest edge on the
+            // path has passed the requested stamp no sample can still arrive.
+            if self.no_sample_can_still_arrive() {
+                self.warn_unresolved();
+                return None;
+            }


Late transforms exit early

no_sample_can_still_arrive() treats a newer buffered edge as proof that no usable transform can arrive. TBuffer::add still accepts in-window samples out of timestamp order, however. A lookup for 5.0 with 0.1 tolerance returns None immediately when 5.2 is buffered, even if a 5.05 sample arrives before the configured timeout and resolves the lookup. Do not use the newest timestamp as a terminal condition unless out-of-order samples are rejected consistently; otherwise continue waiting until the deadline.

Artifacts

Focused Rust regression test source for delayed out-of-order transform

This temporary test seeds a too-new edge, waits for a delayed older in-tolerance edge, and asserts both wait-time and post-arrival lookup behavior; it exercises the claimed failure path.

Focused Rust test output showing early None before accepted late sample

The exact focused cargo test ran once and failed with “within returned None even though the later 5.05 sample was accepted,” demonstrating the bug.

_{Ran code and verified through T-Rex}

greptile-apps · 2026-08-27T00:03:52Z

+    /// How long to wait for a late transform before dropping a cloud (s).
+    #[validate(range(min = 0.0))]
+    pub tf_wait_timeout_s: f64,


Timeout conversion panics

The lower-bound-only validator accepts finite values such as 1e300, but point-cloud handling converts this value with Duration::from_secs_f64, which panics when the value cannot be represented as a Duration. Reject non-finite and oversized timeout values during configuration validation, or use a checked conversion and handle invalid configuration without panicking.

Artifacts

Focused Rust reproduction test source

The executed source constructs production configuration values, invokes validation, and runs the exact `Duration::from_secs_f64` expression from the point-cloud handler, showing the tested scope.

Baseline timeout conversion output

Executed cargo test with `TEST_TIMEOUT=0.1`; validation was accepted and duration conversion did not panic, establishing the normal control behavior.

Oversized finite timeout reproduction output

Executed cargo test with finite `TEST_TIMEOUT=1e300`; validation was accepted and the exact duration conversion panicked, reproducing the claim.

_{Ran code and verified through T-Rex}

codecov · 2026-08-27T00:05:02Z

❌ 1 Tests Failed:

Tests completed	Failed	Passed	Skipped
4468	1	4467	178

View the top 1 failed test(s) by shortest run time

dimos.utils.test_shm::test_attach_shm_waits_out_the_race

Stack Traces | 0.415s run time

name = 'dimos_test_ea90ea3690d4'
slow_ftruncate = <function slow_ftruncate.<locals>.delayed at 0xffae18db2ca0>

    def test_attach_shm_waits_out_the_race(name, slow_ftruncate):
        """attach_shm blocks through the window and returns a fully sized segment."""
        result: list[object] = []
    
        def attacher() -> None:
            try:
                shm = attach_shm(name, timeout=5.0)
                result.append(shm.size)
                shm.close()
            except BaseException as exc:
                result.append(exc)
    
        t = threading.Thread(target=attacher, daemon=True)
        t.start()
        time.sleep(0.01)
        owner = SharedMemory(create=True, size=SIZE, name=name)
        t.join(timeout=6)
        owner.close()
    
>       assert result == [SIZE], f"attacher did not survive the window: {result}"
E       AssertionError: attacher did not survive the window: [0]
E       assert [0] == [65536]
E         
E         At index 0 diff: #x1B[0m#x1B[94m0#x1B[39;49;00m#x1B[90m#x1B[39;49;00m != #x1B[0m#x1B[94m65536#x1B[39;49;00m#x1B[90m#x1B[39;49;00m
E         
E         Full diff:
E         #x1B[0m#x1B[90m #x1B[39;49;00m [#x1B[90m#x1B[39;49;00m
E         #x1B[91m-     65536,#x1B[39;49;00m#x1B[90m#x1B[39;49;00m
E         #x1B[92m+     0,#x1B[39;49;00m#x1B[90m#x1B[39;49;00m
E         #x1B[90m #x1B[39;49;00m ]#x1B[90m#x1B[39;49;00m

attacher   = <function test_attach_shm_waits_out_the_race.<locals>.attacher at 0xffae18e00900>
name       = 'dimos_test_ea90ea3690d4'
owner      = SharedMemory('dimos_test_ea90ea3690d4', size=65536)
result     = [0]
slow_ftruncate = <function slow_ftruncate.<locals>.delayed at 0xffae18db2ca0>
t          = <Thread(Thread-1567 (attacher), stopped daemon 281122957029760)>

dimos/utils/test_shm.py:98: AssertionError

To view more test analytics, go to the Test Analytics Dashboard
_{📋 Got 3 mins? Take this short survey to help us improve Test Analytics.}

aclauer

Ah yeah this should def be configurable.

Bu also this config is for how long we wait for a transform that fits the tolerance criteria before giving up, so if the robot publishes slowly, don't you want to change the tf_match_tolerance?

jeff-hykin requested review from Dreamsorcerer, aclauer, leshy, mustafab0, paul-nechifor and spomichter as code owners August 26, 2026 23:51

greptile-apps Bot reviewed Aug 27, 2026

View reviewed changes

aclauer reviewed Aug 27, 2026

View reviewed changes

Merge branch 'main' into jeff/fix/ray_tracing_tf_salvage

740473c

Navigation Menu

Uh oh!

ray_tracing: salvage tf-wait and darwin build fixes - #3708

ray_tracing: salvage tf-wait and darwin build fixes#3708
jeff-hykin wants to merge 2 commits into
mainfrom
jeff/fix/ray_tracing_tf_salvage

jeff-hykin commented Aug 26, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Aug 27, 2026 •

edited

Loading

T-Rex Logs

Comments Outside Diff (2)

Uh oh!

greptile-apps Bot Aug 27, 2026

Uh oh!

greptile-apps Bot Aug 27, 2026

Uh oh!

codecov Bot commented Aug 27, 2026 •

edited

Loading

Uh oh!

aclauer left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

jeff-hykin commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greptile-apps Bot commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 3/5

T-Rex Logs

Comments Outside Diff (2)

Uh oh!

greptile-apps Bot Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❌ 1 Tests Failed:

Uh oh!

aclauer left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jeff-hykin commented Aug 26, 2026 •

edited

Loading

greptile-apps Bot commented Aug 27, 2026 •

edited

Loading

codecov Bot commented Aug 27, 2026 •

edited

Loading