Skip to content

feat: --record taps every published topic into a memory store - #3710

Open
spomichter wants to merge 21 commits into
mainfrom
feat/record
Open

feat: --record taps every published topic into a memory store#3710
spomichter wants to merge 21 commits into
mainfrom
feat/record

Conversation

@spomichter

@spomichter spomichter commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Right now to record to memory requires Go2Module, need to specific and route IN topics. Prefer to simple to have some --record mcap or --record --record-topics={color_image, ...} to specify full recording

Taps into coordinator loop and streams on a single worker to memory Sqlite store. Will add mcap support in next PR

After short recording:

You run: uv run dimos --record --simulation run unitree-go2

After run:

uv run dimos mem summary recordings/20260826-203855-unitree-go2/memory.db 
                                    memory.db                                    
┏━━━━━━━━━━━━━━━━┳━━━━━━━━┳━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━┳━━━━━━━━━━━━┓
┃ Stream         ┃  Items ┃    Hz ┃ Start (UTC)         ┃ Duration ┃       Size ┃
┡━━━━━━━━━━━━━━━━╇━━━━━━━━╇━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━╇━━━━━━━━━━━━┩
│ lidar          │     43 │   2.0 │ 2026-08-27 03:39:06 │    21.2s │   5.14 MiB │
│ color_image    │    376 │  17.3 │ 2026-08-27 03:39:06 │    21.6s │   4.26 MiB │
│ global_map     │      9 │   0.4 │ 2026-08-27 03:39:07 │    20.2s │   3.08 MiB │
│ global_costmap │      9 │   0.4 │ 2026-08-27 03:39:07 │    20.2s │ 421.03 KiB │
│ tf             │    981 │  45.2 │ 2026-08-27 03:39:06 │    21.7s │ 292.19 KiB │
│ cmd_vel        │  1,855 │  99.7 │ 2026-08-27 03:39:07 │    18.6s │ 101.45 KiB │
│ tele_cmd_vel   │  1,855 │  98.6 │ 2026-08-27 03:39:07 │    18.8s │ 101.45 KiB │
│ goal           │  1,855 │  98.9 │ 2026-08-27 03:39:07 │    18.8s │  94.20 KiB │
│ way_point      │  1,855 │  98.9 │ 2026-08-27 03:39:07 │    18.8s │  94.20 KiB │
│ odom           │    981 │  45.2 │ 2026-08-27 03:39:06 │    21.7s │  82.39 KiB │
│ stop_movement  │  1,855 │  98.9 │ 2026-08-27 03:39:07 │    18.7s │  16.30 KiB │
│ camera_info    │     24 │     — │ 2026-08-27 03:38:56 │     0.0s │   8.67 KiB │
│ nav_cmd_vel    │      3 │  32.8 │ 2026-08-27 03:39:28 │     0.1s │   168.00 B │
│ goal_request   │      1 │     — │ 2026-08-27 03:39:28 │     0.0s │    86.00 B │
│ path           │      2 │ 289.1 │ 2026-08-27 03:39:28 │     0.0s │    68.00 B │
│ goal_reached   │      1 │     — │ 2026-08-27 03:39:28 │     0.0s │     9.00 B │
├────────────────┼────────┼───────┼─────────────────────┼──────────┼────────────┤
│ total          │ 11,705 │       │                     │          │  13.66 MiB │
└────────────────┴────────┴───────┴─────────────────────┴──────────┴────────────┘

--record usage

dimos --record run unitree-go2                                  # every stream -> recordings/<run-id>/memory.db
dimos --record --record-topics color_image run unitree-go2      # one stream
dimos --record --record-topics lidar,odom,tf run unitree-go2    # comma-separated, no spaces, no braces
dimos --record --record-topics 'global_*' run unitree-go2       # fnmatch globs on the stream name
dimos mem summary recordings/<run-id>/memory.db

Slop:
dimos --record run <blueprint> records every topic on the transport bus, rosbag-style, to one recordings/<run-id>/memory.db (same run id as logs/). No recorder module to wire per robot.

  • GlobalConfig.record ("" | sqlite) and record_topics (comma globs on the topic slug, default *). Bare --record means sqlite, normalized in argv like --simulation (normalize_argv, one table entry per optional-value flag). mcap is the next value once the writer lands (follow-up PR).
  • dimos/memory/tap.py: dimos run wraps coordinator.loop() in recording(), which subscribes to all topics on the active transport's pubsub (same backend selection as the Rerun bridge: LCM, or Zenoh when --transport zenoh) and appends each decoded message under its topic slug (/lidar -> lidar). Single writer, single file. Off unless --record; off under --replay. Topics without a dimos message type are skipped.
  • Poses are not resolved at record time; tf is a topic like any other and dimos map pose-fill derives poses on read.
  • Explicit Recorder modules (Go2Memory / unitree-go2-memory, unitree-go2-mid360-record, unitree-g1-record, ...) are unchanged: recording specific topics as a module stays available alongside --record.

Caveats: SHM-only streams are not on the bus and are not recorded; Zenoh's subscribe_all is latest-per-topic (best effort), LCM is complete.

- GlobalConfig.record ('' | sqlite) and record_topics (comma globs, default *).
  Bare --record means sqlite (argv normalize, same as --simulation).
- Out.publish appends to recordings/<run-id>/memory-<pid>.db per process
  (dimos/memory/tap.py); off unless --record, off in replay.
- Go2Memory / unitree-go2-memory removed: unitree-go2 --record replaces them.
@codecov

codecov Bot commented Aug 27, 2026

Copy link
Copy Markdown

Codecov Report

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

Files with missing lines Patch % Lines
dimos/memory/tap.py 79.01% 15 Missing and 2 partials ⚠️
dimos/cli/commands/lifecycle.py 78.94% 4 Missing ⚠️
dimos/memory/test_tap.py 96.42% 3 Missing ⚠️
@@            Coverage Diff             @@
##             main    #3710      +/-   ##
==========================================
- Coverage   77.54%   77.51%   -0.04%     
==========================================
  Files        1304     1306       +2     
  Lines      123668   124061     +393     
  Branches    10821    10894      +73     
==========================================
+ Hits        95902    96161     +259     
- Misses      24976    25133     +157     
+ Partials     2790     2767      -23     
Flag Coverage Δ
OS-ubuntu-24.04-arm 73.57% <88.62%> (+0.01%) ⬆️
OS-ubuntu-latest 74.13% <88.62%> (+0.01%) ⬆️
Py-3.10 74.13% <88.62%> (+0.01%) ⬆️
Py-3.11 74.13% <88.62%> (+0.01%) ⬆️
Py-3.12 74.13% <88.62%> (+0.01%) ⬆️
Py-3.13 74.13% <88.62%> (+0.01%) ⬆️
Py-3.14 74.14% <88.62%> (+0.01%) ⬆️
Py-3.14t 74.13% <88.62%> (+0.01%) ⬆️
SelfHosted-Linux 34.82% <27.48%> (-0.03%) ⬇️

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

Files with missing lines Coverage Δ
dimos/cli/dimos.py 100.00% <100.00%> (ø)
dimos/cli/test_dimos.py 98.76% <100.00%> (+0.03%) ⬆️
.../core/coordination/blueprint_config/test_parser.py 97.32% <100.00%> (+0.02%) ⬆️
dimos/core/coordination/module_coordinator.py 91.47% <100.00%> (+4.90%) ⬆️
dimos/core/coordination/test_module_coordinator.py 98.37% <100.00%> (ø)
dimos/core/global_config.py 89.89% <100.00%> (-0.83%) ⬇️
dimos/memory/test_tap.py 96.42% <96.42%> (ø)
dimos/cli/commands/lifecycle.py 66.51% <78.94%> (+5.67%) ⬆️
dimos/memory/tap.py 79.01% <79.01%> (ø)

... and 5 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.

Comment thread dimos/memory/tap.py Outdated
@spomichter
spomichter marked this pull request as ready for review August 27, 2026 03:42
Comment thread dimos/core/transport_factory.py Outdated
@greptile-apps

greptile-apps Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This change adds opt-in recording of matching transport streams to a per-run SQLite store, including command-line configuration and background persistence so transport callbacks can return promptly.

Recording failures can prevent a run from exiting cleanly. If encoding or a SQLite append raises, the writer stops; once its queue fills, shutdown waits indefinitely while trying to enqueue the stop marker.

Confidence Score: 4/5

Not safe to merge until recorder shutdown remains reliable after a write failure.

An exception while persisting a recorded message can leave the background writer stopped and block process cleanup indefinitely.

Files Needing Attention: dimos/memory/tap.py

T-Rex T-Rex Logs

What T-Rex did

  • Reproduced the deterministic TransportRecorder append-failure and observed a close hang.
  • Validated the second P1 finding via its finding-comment-proof.
  • Validated the contract behavior of the run, noting append_entered was true, writer_alive was false, queue_full was true, and close_returned was false; the traceback points to dimos/memory/tap.py:73 and the child process exits cleanly, so the test runner does not hang.

View all artifacts

T-Rex Ran code and verified through T-Rex

Comments Outside Diff (1)

  1. General comment

    P1 TransportRecorder.close can block forever after an append failure

    • Bug
      • A raising stream append terminates the daemon record-writer thread. If the bounded queue is subsequently full, close() blocks indefinitely in _queue.put(None) because no consumer remains to make space.
    • Cause
      • _drain calls stream.append(msg, ts=ts) without exception handling at dimos/memory/tap.py:73; close performs an unbounded sentinel enqueue at line 77 before joining the dead writer.
    • Fix
      • Handle append failures in _drain so the writer remains able to drain and receive shutdown, and make shutdown resilient to a failed writer/full queue (for example, use a bounded/conditional sentinel enqueue with explicit failure handling).

    T-Rex Ran code and verified through T-Rex

Reviews (2): Last reviewed commit: "record: warn on first and every 1000th d..." | Re-trigger Greptile

Comment thread dimos/memory/tap.py Outdated
…, not a bus guess

- ModuleCoordinator.transports exposes the wired (name, type) -> Transport table.
- recording(coordinator.transports) subscribes each stream on whatever carries it
  (LCM, Zenoh, SHM, ...); streams are named by their blueprint name.
- transport_factory / rerun bridge back to main (pubsub_backend dropped).
Comment thread dimos/cli/test_dimos.py Outdated
Comment thread dimos/memory/tap.py
Comment thread dimos/memory/tap.py Outdated

@jeff-hykin jeff-hykin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

two things

@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Aug 27, 2026
Co-authored-by: Paul Nechifor <paul@nechifor.net>
@github-actions github-actions Bot removed the ready-to-merge Required CI checks have passed on this PR label Aug 27, 2026
@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Aug 27, 2026
@github-actions github-actions Bot removed the ready-to-merge Required CI checks have passed on this PR label Aug 27, 2026
@spomichter

Copy link
Copy Markdown
Contributor Author

@greptile final review

Comment thread dimos/memory/tap.py Outdated
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.

3 participants