Skip to content

feat(recordings): consolidate offline CDR converter and fidelity audit - #4735

Draft
TomCC7 wants to merge 11 commits into
cc/feat/cdr-unit-robotsfrom
cc/feat/cdr-unit-converter
Draft

TomCC7 wants to merge 11 commits into
cc/feat/cdr-unit-robotsfrom
cc/feat/cdr-unit-converter

Conversation

@TomCC7

@TomCC7 TomCC7 commented Oct 20, 2026 •

Copy link
Copy Markdown
Member

What this adds

Add offline dimos mem convert commands for local MCAP and SQLite recordings. Conversion preserves timestamps and supported fields, writes a fidelity report and never overwrites the source or an existing destination.

Base: #4738. The converter uses the standard MCAP writer and an explicit historical decode boundary.

How to test

Start with dimos mem convert OLD NEW --dry-run. Follow the conversion guide for formats and expected output.

All 35 local conversion tests passed again after restacking. Repository hooks passed on the cleaned stack tip. Installed-wheel tests were not rerun. Unsupported formats and pickle inputs remain blocked.

Review report: file map, scope and verification

Shared CDR foundation

Standard ROS definitions are fetched and verified through the existing source lock and generation pipeline in #4366. Native SDK support is included in #4722. Shared Python value implementations and regressions are reviewed in #4800–#4803 before the #4734 caller migration. SDK and consumers use the same generated Rust message crate, including Time, TransformStamped and TFMessage; there is no separate TF schema package or generation command. Camera/depth callers migrate in #4734 and mapping/navigation callers in #4737.

The external CDR and transforms library pins remain unchanged. Local verification passed 122 SDK tests, all existing native consumer suites, and all 24 real native/DimSim/port acceptance tests without skips. Seven staged Nix source layouts resolve exactly one message crate with locked Cargo metadata. This is not a full Nix build or hardware validation. Fresh CI runs verify #4722 independently and the combined stack tip.

Stack #4804 contains the 12 original feature PRs and four separately reviewed adapter/buffer PRs (#4800–#4803). The redundant #4774 is superseded by #4722. No feature PR has been merged into main; existing conflicts with main and the WIP cutover failures in #4739 remain.

@TomCC7

TomCC7 commented Oct 21, 2026 •

Copy link
Copy Markdown
Member Author

Review report

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

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

What changes and why the diff is large

Add an explicit offline recording converter for local SQLite and MCAP, plus migration/fidelity reporting. Historical decoding is confined to this command; this is separate from live transport compatibility.
3 implementation files, 2 test files and 1 guide. This unit is already small in file count; most lines are conversion rules and regression cases.

Review map

Suggested entry points: dimos/memory/convert_recording.py, dimos/memory/recording_migration.py, dimos/memory/cli/app.py, docs/development/convert-recordings.md.
All 6 changed files are accounted for below. Grouping is by file role, not a claim that the implementation is minimal.

Complete file map

Implementation (3)

File Change Purpose
dimos/memory/cli/app.py modified Expose offline conversion and migration through the existing memory CLI.
dimos/memory/convert_recording.py added Offline MCAP/SQLite conversion, explicit legacy type allowlist, schema/field checks and safe output handling.
dimos/memory/recording_migration.py added Batch/recording migration bookkeeping and fidelity reporting around the converter.

Tests (2)

File Change Purpose
dimos/memory/test_convert_recording.py added Checks pose preserves payload header and both envelope times.
dimos/memory/test_recording_migration.py added Checks cli single and batch dry run then conversion.

Documentation and replay inputs (1)

File Change Purpose
docs/development/convert-recordings.md added Update the convert-recordings guide or replay input for the migrated boundaries.

Review focus and scope reductions

  • Keep the historical type allowlist and unsupported-input behavior explicit. Do not infer arbitrary Python imports from recording type names or accept pickle as a convenience.
  • Review timestamp, schema, metadata and field fidelity with failure/rollback behavior. Source files and existing destinations must stay protected.
  • MCAP output cannot preserve SQLite vector embeddings; rejection must remain visible rather than silently dropping them.
  • The converter and migration-report responsibilities can be read separately. Do not shrink the diff by removing safety/fidelity tests just to reduce line count.

Validation

Fresh checks on 25dc75047dc420a8ba34602044aaf3dfa95520eb: all 35 offline conversion/fidelity tests passed using local temporary recordings. This layer's code did not need helper edits; it was restacked onto the cleaned predecessor. Final stack repository hooks passed with cargo/native lint excluded.

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/memory/test_convert_recording.py dimos/memory/test_recording_migration.py
dimos mem convert OLD NEW --dry-run

Remaining work

Installed-wheel converter execution and unsupported historical formats were not retested. Source/output protection and blocked pickle inputs remain intact.

@TomCC7
TomCC7 force-pushed the cc/feat/cdr-unit-converter branch from 8f563d0 to 25dc750 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
@TomCC7
TomCC7 removed this pull request from stack #4776 October 23, 2026 08:47
@TomCC7
TomCC7 added this pull request to stack #4799 October 23, 2026 08:47
@TomCC7
TomCC7 removed this pull request from stack #4799 October 23, 2026 09:19
@TomCC7
TomCC7 added this pull request to stack #4804 October 23, 2026 09:19

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