Skip to content

[Vibed Experiment] feat: render TF tree as hierarchical Rerun entity paths#1653

Open
leshy wants to merge 3 commits intodevfrom
feat/tf-tree-rendering
Open

[Vibed Experiment] feat: render TF tree as hierarchical Rerun entity paths#1653
leshy wants to merge 3 commits intodevfrom
feat/tf-tree-rendering

Conversation

@leshy
Copy link
Copy Markdown
Contributor

@leshy leshy commented Mar 23, 2026

Summary

  • Intercept TFMessage in the Rerun bridge and render transforms as hierarchical entity paths (e.g. world/base_link/camera/optical) via DFS traversal of the accumulated TF tree
  • Rate-limit TF tree re-renders using min_interval_sec and cache the adjacency dict (rebuilt only on new edges) for performance
  • Add Config.tf_enabled flag, cycle protection in DFS, and 5 unit tests covering chain hierarchy, multiple roots, disabled fallback, incremental updates, and cycle protection

Closes #1627

Test plan

  • 5 unit tests passing (pytest dimos/visualization/rerun/test_tf_tree.py -v)
  • mypy clean
  • Performance review: rate-limiting and adjacency caching verified

🤖 Generated with Claude Code

leshy and others added 2 commits March 23, 2026 16:10
Intercept TFMessages in the Rerun bridge and render transforms as
hierarchical entity paths (e.g. world/base_link/camera/optical) instead
of flat paths. This uses DFS traversal of the accumulated TF tree with
rate-limited re-renders and cached adjacency structures for performance.

- Add _render_tf_tree() with DFS walk, cycle protection, and root detection
- Intercept TFMessages in _on_message with rate-limiting via min_interval_sec
- Cache adjacency dict, rebuilt only when edge count changes
- Add Config.tf_enabled flag to toggle TF interception
- Add 5 unit tests: chain hierarchy, multiple roots, tf_disabled fallback,
  incremental updates, and cycle protection

Closes #1627

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 23, 2026

Greptile Summary

This PR closes the long-standing TODO in the Rerun bridge by giving it its own MultiTBuffer and rendering the accumulated TF tree as proper hierarchical Rerun entity paths (e.g. world/odom/base_link/camera) via a DFS walk, replacing the previous flat-path fallback. The implementation is solid: transforms are always accumulated, rendering is rate-limited via min_interval_sec, the adjacency/roots cache is rebuilt only when new edges appear, and cycle detection prevents infinite recursion.

Key changes:

  • Config.tf_enabled flag to opt out of TF interception
  • RerunBridgeModule._render_tf_tree() — DFS walk with per-render visited set for cycle safety
  • Adjacency cache (_tf_children, _tf_roots) invalidated by edge count, keeping hot-path overhead minimal
  • 5 unit tests covering chain hierarchy, multiple roots, disabled fallback, incremental updates, and cycle protection
  • The cycle-protection test assertion (<= 3) is too permissive and should be == 0 to match the documented expectation
  • _make_bridge test helper leaves _last_tf_render_time and the adjacency-cache attributes uninitialised, which will cause AttributeError in any future test that exercises _on_message with tf_enabled=True

Confidence Score: 4/5

  • Safe to merge; production code path is correct and the issues are limited to test quality.
  • The bridge logic is well-implemented with no production bugs found. The two concerns are confined to the test file: a loose cycle-protection assertion that could hide future regressions, and a partially-uninitialised test helper that will break any new _on_message tests. Fixing these before merge improves long-term test reliability but does not block the feature itself.
  • dimos/visualization/rerun/test_tf_tree.py — cycle assertion and missing attribute initialisation in _make_bridge

Important Files Changed

Filename Overview
dimos/visualization/rerun/bridge.py Adds TFMessage interception and hierarchical entity-path rendering via DFS. Logic is sound: transforms are always accumulated, rendering is rate-limited, cycle detection works, and adjacency cache is rebuilt only on new edges. Minor inconsistency: _tf_num_edges uses getattr fallback in _render_tf_tree while _tf_roots/_tf_children use direct attribute access, relying on start() always being called first.
dimos/visualization/rerun/test_tf_tree.py 5 unit tests covering the main scenarios. Two issues: the cycle-protection assertion is too loose (<= 3 instead of == 0), and _make_bridge leaves several instance attributes uninitialised, making tests for _on_message with tf_enabled=True fragile. Unused call import also present.

Sequence Diagram

sequenceDiagram
    participant PubSub
    participant RerunBridgeModule
    participant MultiTBuffer
    participant Rerun

    PubSub->>RerunBridgeModule: _on_message(TFMessage, topic)
    RerunBridgeModule->>MultiTBuffer: receive_transform(*transforms)
    RerunBridgeModule->>RerunBridgeModule: check rate-limit (min_interval_sec)
    alt interval elapsed
        RerunBridgeModule->>RerunBridgeModule: _render_tf_tree()
        note over RerunBridgeModule: Rebuild adjacency cache if new edges
        loop DFS over roots
            RerunBridgeModule->>MultiTBuffer: get_transform(parent, child)
            MultiTBuffer-->>RerunBridgeModule: Transform | None
            RerunBridgeModule->>Rerun: rr.log(entity_path, Transform3D)
        end
    end
    RerunBridgeModule-->>PubSub: return (TF path consumed)
Loading

Reviews (1): Last reviewed commit: "CI code cleanup" | Re-trigger Greptile

@leshy leshy changed the title feat: render TF tree as hierarchical Rerun entity paths [Vibed Experiment] feat: render TF tree as hierarchical Rerun entity paths Mar 23, 2026
- Bridge now subscribes to MultiTBuffer callbacks instead of intercepting
  TFMessages and maintaining its own buffer
- Added subscribe() method and children_of/roots properties to MultiTBuffer
  with edge-count caching for O(1) repeated access
- BFS in get_transform_search uses pre-built adjacency map (O(E)) instead
  of calling get_connections per node
- receive_tfmessage batches all transforms in a single receive_transform call
- children_of returns Mapping instead of dict for immutability
- Fixed tests: cycle assertion tightened to == 0, unused `call` import
  removed, _make_bridge initializes _last_tf_render_time and injects
  MultiTBuffer via _tf

Addresses review comments on #1653.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@leshy leshy changed the base branch from main to dev March 25, 2026 07:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rerun Bridge should reconstruct the tf tree automatically

1 participant