Ivan/record replay wrap by leshy · Pull Request #1976 · dimensionalOS/dimos

leshy · 2026-05-05T07:15:15Z

Problem

Closes DIM-XXX

Solution

Breaking Changes

How to Test

Contributor License Agreement

I have read and approved the CLA.

Co-authored-by: leshy <681516+leshy@users.noreply.github.com>

Agent-Logs-Url: https://github.com/dimensionalOS/dimos/sessions/44dd2709-d3ba-4138-b140-56e76fd15eb9 Co-authored-by: leshy <681516+leshy@users.noreply.github.com>

leshy · 2026-05-05T07:23:28Z

+_SANITIZE_RE = re.compile(r"[^A-Za-z0-9_]")
+
+
+def topic_to_stream_name(channel: str) -> str:


there is no need for update_stream_meta and store metadata interventions to save the type of a stream and full stream name, given this function is reversible? streams know their type in their metadata, so they can reconstruct full topics?

I don't think the function is reversible. /color/image and /color_image both get transformed to _color_image, no?

oh no you are right

greptile-apps · 2026-05-05T15:12:53Z

Greptile Summary

This PR introduces a RecordReplay class backed by SQLite for recording and replaying pub/sub topics, wires it into ModuleCoordinator so that --replay-file and --option record_path= are handled automatically at blueprint build time, and ships a Textual TUI (recorder) for interactive recording, playback, seek, and trim.

RecordReplay: new core class with start_recording, stop_recording, play, seek, trim, delete_range, pause/resume, and async with support.
ModuleCoordinator.replay(): auto-disables modules whose outputs are covered by a recording, then starts playback.
RecorderApp: full Textual TUI with LCM spy integration, sparklines, timeline bar, and keybinding-driven workflow.

Confidence Score: 3/5

Not ready to merge: the module-level docstring advertises two broken API calls that raise TypeError, and the RecordReplay object created in replay() leaks its SQLite connection and background task.

The primary usage docstring shows rec.start_recording([LCM()]) missing the required topics argument and rec.play(pubsub=LCM(), ...) with a nonexistent parameter — both raise TypeError immediately. The RecordReplay instance in ModuleCoordinator.replay() is a local variable never stored on the coordinator, so close() is never called on shutdown and the SQLite handle and playback task both leak.

dimos/record/record_replay.py (broken docstring examples), dimos/core/coordination/module_coordinator.py (leaked RecordReplay instance, no LCM publish during playback)

Important Files Changed

Filename	Overview
dimos/record/record_replay.py	New RecordReplay class; module docstring contains two broken API examples that raise TypeError for users who copy them.
dimos/core/coordination/module_coordinator.py	Adds replay() classmethod and auto-record wiring; RecordReplay never stored/closed (previously flagged), messages not published to LCM during playback (previously flagged), malformed warning string, no log when record_path set but record_modules empty.
dimos/core/module.py	Adds start_recording_outputs / stop_recording_outputs RPCs wired into stop(); logic appears sound.
dimos/utils/cli/recorder/run_recorder.py	New Textual TUI for interactive recording and playback; seek and trim operations look correct.
dimos/memory2/observationstore/sqlite.py	Adds delete_range with proper commit/rollback outside the if-ids branch.
dimos/core/coordination/blueprints.py	Adds record_modules field and default_record_modules() builder; propagated correctly through autoconnect.
dimos/record/test_record_replay.py	New async test suite covering record, query, duration, trim, delete, seek, playback, and multi-pubsub.

Comments Outside Diff (1)

dimos/record/record_replay.py, line 663-668 (link)

Module docstring shows invalid start_recording and play signatures

The usage example at line 664 calls rec.start_recording([LCM()]) with only one positional argument, but the actual signature is start_recording(pubsubs, topics) — topics is required. Any user copying this example will immediately get TypeError: start_recording() missing 1 required positional argument: 'topics'.

Line 668 likewise shows rec.play(pubsub=LCM(), speed=1.0), but play() has no pubsub parameter (see line 851). Calling it that way raises TypeError: play() got an unexpected keyword argument 'pubsub'.

_{Reviews (5): Last reviewed commit: "Merge remote-tracking branch 'origin/dev..." | Re-trigger Greptile}

paul-nechifor · 2026-05-06T03:51:20Z

+            result.append(info)
+        return tuple(result)
+
+    def play(self, speed: float = 1.0) -> None:


The comment at the start of the file says rec.play(pubsub=LCM(), speed=1.0). Where is this play actually publishing?

paul-nechifor · 2026-05-06T03:57:25Z

+    rec.stop_recording()
+
+    # Replay into LCM (viewable via rerun-bridge)
+    rec.play(pubsub=LCM(), speed=1.0)


I think we should have docs for replay in docs. Note, BTW, that rec.play doesn't take pubsub currently.

paul-nechifor · 2026-05-06T04:46:11Z

+        stream_name = topic_to_stream_name(topic.pattern)
+        self._store.stream(stream_name, type(msg)).append(msg)


You're not storing the topic. How does the stream know where the message should be replayed to?

The previous PR stored the topic. This tries to guess that the topic is.

paul-nechifor · 2026-05-06T04:55:25Z

+else:
+    from typing import TypeGuard as TypeIs
+
+RERUN_GRPC_PORT = 9876


Is 9877 to 9876 intentional? There are hardcoded places where the 9877 is still used:

dimos/core/test_daemon.py:47: grpc_port: int = 9877, dimos/core/test_daemon.py:143: entry = _make_entry(pid=os.getpid(), grpc_port=9877) dimos/core/test_daemon.py:146: conflict = check_port_conflicts(grpc_port=9877) dimos/core/test_daemon.py:154: conflict = check_port_conflicts(grpc_port=9877) dimos/core/run_registry.py:47: grpc_port: int = 9877 dimos/core/run_registry.py:144:def check_port_conflicts(grpc_port: int = 9877) -> RunEntry | None: dimos/visualization/rerun/bridge.py:196: connect_url: str = "rerun+http://127.0.0.1:9877/proxy"

paul-nechifor · 2026-05-06T04:56:36Z


    @rpc
    def start(self) -> None:
+        # Delay import to reduce import time (~2.4s)


You removed from dimos.msgs.sensor_msgs.PointCloud2 import register_colormap_annotation but left the comment.

# Conflicts: # dimos/core/global_config.py # dimos/robot/cli/dimos.py # dimos/robot/unitree/type/lidar.py # pyproject.toml

codecov · 2026-05-08T14:36:19Z

❌ 1 Tests Failed:

Tests completed	Failed	Passed	Skipped
1883	1	1882	59

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

dimos.robot.cli.test_cli_startup::test_help_startup_time

Stack Traces | 10.2s run time

def test_help_startup_time() -> None:
        """`dimos --help` must finish in under {HELP_TIMEOUT_SECONDS}s."""
        start = time.monotonic()
        result = subprocess.run(
            [sys.executable, "-m", "dimos.robot.cli.dimos", "--help"],
            capture_output=True,
            text=True,
            timeout=HELP_TIMEOUT_SECONDS + 5,  # hard kill safety margin
        )
        elapsed = time.monotonic() - start
        assert result.returncode == 0, f"dimos --help failed:\n{result.stderr}"
>       assert elapsed < HELP_TIMEOUT_SECONDS, (
            f"dimos --help took {elapsed:.1f}s (limit: {HELP_TIMEOUT_SECONDS}s). "
            f"Check for heavy imports in the CLI entrypoint or GlobalConfig."
        )
E       AssertionError: dimos --help took 10.2s (limit: 8s). Check for heavy imports in the CLI entrypoint or GlobalConfig.
E       assert 10.191117729060352 < 8

.../robot/cli/test_cli_startup.py:60: 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.}

Sam Bull and others added 30 commits March 29, 2026 16:49

Memory2 record/replay functionality

aa05448

Merge branch 'dev' into record-replay

4d7f98d

Fix async

8f763c1

Load in viewer as separate source

4e646d6

Fix playback

6c7ff5d

Use AllPubSub

ff0adfc

Recording fixes

9083096

Fix playback

8315178

Merge branch 'dev' into record-replay

2b9a476

Merge branch 'dev' into record-replay

c473fda

Fix duplicate recording timestamps

46b38b8

Fixes

99e6013

Revert flake.nix

81157b4

Cleanup

9d69576

Import time

6bf01da

Docs

7b6d0c1

Merge branch 'dev' into record-replay

a20eaf7

Typing fixes

4e6ea2c

Typing fixes

ad6debf

Revert

b11e938

Remove init files

bc563b7

Fix tests

2d6f159

Fix

5b4bdf4

Fix

2dfc23e

Fix

787bd66

Adjust imports

d9f6ff3

Fix args

480f853

Fixes

28de1d2

Fix test

34a20ba

Merge dev into record-replay

4cbf61e

Co-authored-by: leshy <681516+leshy@users.noreply.github.com>

Copilot AI and others added 3 commits April 21, 2026 09:48

docs: add record/replay usage and test steps

c3588c0

Agent-Logs-Url: https://github.com/dimensionalOS/dimos/sessions/44dd2709-d3ba-4138-b140-56e76fd15eb9 Co-authored-by: leshy <681516+leshy@users.noreply.github.com>

adressing Paul pr comments

d0f1b63

Merge remote-tracking branch 'origin/dev' into ivan/record-replay-wrap

15e1844

leshy marked this pull request as ready for review May 5, 2026 07:15

greptile-apps Bot reviewed May 5, 2026

View reviewed changes

Comment thread dimos/record/record_replay.py Outdated

Comment thread dimos/core/module.py Outdated

leshy commented May 5, 2026

View reviewed changes

Comment thread dimos/core/module.py

leshy commented May 5, 2026

View reviewed changes

removed store metadata intervention

2977242

greptile-apps Bot reviewed May 5, 2026

View reviewed changes

Comment thread dimos/core/coordination/module_coordinator.py

Comment thread dimos/core/coordination/module_coordinator.py

leshy mentioned this pull request May 5, 2026

memory2 - per-stream metadata kv store #1977

Open

leshy added 3 commits May 5, 2026 22:27

webrtc retransmit issue fix

452130d

webrtc lidar timestamps have been fixed

08337ce

record/replay should use wall clock for msg ts, not record time

edf2b76

greptile-apps Bot reviewed May 5, 2026

View reviewed changes

Comment thread dimos/record/record_replay.py

Comment thread dimos/memory2/observationstore/sqlite.py

leshy added 2 commits May 5, 2026 23:00

final cleanup

d66a7bb

sql transaction fix

5434e17

dimensionalOS deleted a comment from greptile-apps Bot May 5, 2026

paul-nechifor reviewed May 6, 2026

View reviewed changes

Merge remote-tracking branch 'origin/dev' into ivan/record-replay-wrap

c292aa7

# Conflicts: # dimos/core/global_config.py # dimos/robot/cli/dimos.py # dimos/robot/unitree/type/lidar.py # pyproject.toml

		_SANITIZE_RE = re.compile(r"[^A-Za-z0-9_]")


		def topic_to_stream_name(channel: str) -> str:

		stream_name = topic_to_stream_name(topic.pattern)
		self._store.stream(stream_name, type(msg)).append(msg)

Conversation

leshy commented May 5, 2026

Problem

Solution

Breaking Changes

How to Test

Contributor License Agreement

Uh oh!

Uh oh!

Uh oh!

Uh oh!

leshy May 5, 2026

Choose a reason for hiding this comment

Uh oh!

paul-nechifor May 6, 2026

Choose a reason for hiding this comment

Uh oh!

leshy May 7, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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

Uh oh!

Greptile Summary

Confidence Score: 3/5

Important Files Changed

Comments Outside Diff (1)

Uh oh!

paul-nechifor May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paul-nechifor May 6, 2026

Choose a reason for hiding this comment

Uh oh!

paul-nechifor May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paul-nechifor May 6, 2026

Choose a reason for hiding this comment

Uh oh!

paul-nechifor May 6, 2026

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented May 8, 2026

❌ 1 Tests Failed:

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

greptile-apps Bot commented May 5, 2026 •

edited

Loading

paul-nechifor May 6, 2026 •

edited

Loading

paul-nechifor May 6, 2026 •

edited

Loading