Skip to content

WIP: consolidate default CDR cutover and selected graph acceptance - #4739

Draft
TomCC7 wants to merge 8 commits into
cc/feat/cdr-unit-converterfrom
cc/feat/cdr-unit-default
Draft

TomCC7 wants to merge 8 commits into
cc/feat/cdr-unit-converterfrom
cc/feat/cdr-unit-default

Conversation

@TomCC7

@TomCC7 TomCC7 commented Oct 20, 2026 •

Copy link
Copy Markdown
Member

Add acceptance checks for default recording, collection JointState/status wiring and R1 chassis transport matching.

Base: #4735. Generated-message codec selection lives in #4722 and native SDK adaptation in #4774.

Add an unconditional PR workflow that builds/tests native SDK and consumers, prepares the pinned DimSim runtime, and runs real Python/native/TypeScript message exchanges. Missing dependencies or binaries fail this dedicated job instead of silently skipping. The final combined stack passed all 12 selected native/DimSim tests locally; cdrgen's separate compiled Python/C++/Rust acceptance workflow is in TomCC7/dimos-cdrgen#1. GitHub branch-protection settings have not been changed.

This PR remains WIP for existing default-cutover failures. An earlier acceptance run had 37 passes and four failures: Python TransportRecorder skips generated messages on LCM and Zenoh, and XArm/Piper collection recorders lack the expected generated aggregate JointState input. These features were not changed or revalidated by the codec repair. Resolve them before merging this layer; passing native interoperability does not establish whole-graph acceptance.

Existing boundary review: #4739 (comment)

CDR dependencies and TF conventions

The stack consumes published dimos-cdrgen 0.1.2 and the immutable CDR library fix. SDK #4774 now uses standard ROS TFMessage, TransformStamped and integer Time, with no per-driver TF codec callbacks or parallel public transform type. The external transforms library, validated by its complete std/no_std/serde and ARM verification gate, owns the frame tree, common-time lookup, interpolation and static history. Its atomic batch patch is pinned to 5ae9563de0c02638c04e393aadfad28444a4398e.

Combined-stack local validation: SDK 121, macros 22, mapper 101, MLS 135, local planner 91, follower 63, V4L2 6 and depth2depth 12 Rust tests; all 24 native/DimSim/port-contract acceptance tests pass without skips. The mapper exchange includes Python CDR tf_static input; the follower exchange covers drive, arrival, a new goal and empty-path stop. Missing native binaries fail the dedicated job. RealSense passes cargo check --all-targets --features docs-only; device linking and hardware execution were not tested. Seven real source-staging layouts pass locked Cargo resolution. Native acceptance for tip cc463c163aed465f7c1d6808165bf640820b0182: workflow run. These are combined-tip results, with the SDK additionally tested standalone at its own layer.

Native Nix inputs use verified source archive f7723ad4d5b9aa5dfe63614b6b537d1ccc2d212e, SHA256 f90af8fa794a34d53ee9481c6d0bdb63138853b30f851d92960fd34224ff62b2. Full Nix builds remain unverified. SDK static edges persist after receipt; LCM/Zenoh do not acquire DDS transient-local durability from CDR. Existing Python mount publishers retain dynamic tf compatibility. Legacy modules remain explicitly pinned to SDK 0.1.0.

Stack #4776 preserves existing history and PR bases. Current main still has conflicts with the original stack; this change does not rebase onto main. No DimOS PR has been merged.

@TomCC7

TomCC7 commented Oct 21, 2026 •

Copy link
Copy Markdown
Member Author

Review report

Updated: 2026-10-21. Direct predecessor: #4735.

  • Head: 75cfb7e8182eaea011f1fefd0f119ae54744a4a4
  • Named base branch: cc/feat/cdr-unit-converter at 25dc75047dc420a8ba34602044aaf3dfa95520eb
  • PR-recorded base: 25dc75047dc420a8ba34602044aaf3dfa95520eb; diff merge base: 25dc75047dc420a8ba34602044aaf3dfa95520eb

What changes and why the diff is large

This PR adds acceptance tests for generated-message capture and selected collection/robot graphs. It does not implement a new default transport or finish capture cutover.
4 test files; no production implementation file changes. The issue here is unfinished acceptance, not excessive implementation scope.

Review map

Suggested entry points: dimos/core/test_cdr_default_recording.py, dimos/imitation/collection/test_cdr_default_graph.py, dimos/robot/galaxea/r1pro/test_cdr_default_graph.py, dimos/core/test_transport_factory.py.
All 4 changed files are accounted for below. Grouping is by file role, not a claim that the implementation is minimal.

Complete file map

Tests (4)

File Change Purpose
dimos/core/test_cdr_default_recording.py added Checks factory capture sqlite mcap auto named replay.
dimos/core/test_transport_factory.py modified Checks transport topic lcm.
dimos/imitation/collection/test_cdr_default_graph.py added Checks collection status is wired to webxr hud.
dimos/robot/galaxea/r1pro/test_cdr_default_graph.py added Checks r1 blueprint chassis pins match default factory.

Review focus and scope reductions

  • The two backend capture tests require Python TransportRecorder to persist generated values through SQLite and MCAP replay. They are a separate path from the native Rust recorder.
  • The collection tests require generated aggregate JointState/status ports to be connected through the selected graph; checking annotations alone is not a complete live recording proof.
  • The four documented failures are blockers. They describe missing functionality, not acceptable expected CI failures. Default cutover must wait for implementation and passing acceptance.
  • Keep existing transport codec selection in the shared layer. This test-only PR is not proof that every business graph has migrated.

Validation

Fresh rerun on 75cfb7e8182eaea011f1fefd0f119ae54744a4a4: 37 passed and four failed. Failures remain generated-message Python capture on LCM/Zenoh and missing XArm/Piper aggregate JointState recorder inputs. These are unresolved functionality blockers. This cleanup did not implement the cutover or mark the failures as expected passes.

Reproduction commands, not executed here unless stated above

Run from this PR's own checkout root after installing its documented test dependencies. These are selected review entry points, not a claim of full-stack coverage.

uv run pytest dimos/core/test_cdr_default_recording.py dimos/imitation/collection/test_cdr_default_graph.py dimos/robot/galaxea/r1pro/test_cdr_default_graph.py dimos/core/test_transport_factory.py

Remaining work

Implement generated-message Python capture and the missing XArm/Piper collection edges, then rerun complete selected graphs. Do not merge this WIP layer with unresolved acceptance failures.

@TomCC7
TomCC7 force-pushed the cc/feat/cdr-unit-default branch from b094101 to 75cfb7e Compare October 22, 2026 02:34
@TomCC7
TomCC7 removed this pull request from stack #4743 October 22, 2026 18:51
@TomCC7
TomCC7 added this pull request to stack #4776 October 22, 2026 18:52

This branch has not been deployed

No deployments
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.

1 participant