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?

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

Greptile Summary

Introduces RecordReplay, a SqliteStore-backed record/replay system with a Textual TUI (RecorderApp), auto-record/replay hooks in ModuleCoordinator.build(), and per-module start_recording_outputs/stop_recording_outputs RPCs on ModuleBase.
loop() is converted from sync (threading.Event) to async (asyncio.Event), requiring call sites to be updated to await coord.loop() inside asyncio.run() — all callers in the diff are correctly updated.
A minor format-string concatenation issue leaves a missing space in the "no modules to disable" warning log message; see inline comment.

Confidence Score: 3/5

Not safe to merge until the open concerns from previous review rounds are resolved — particularly the leaked RecordReplay resource in replay() and playback not republishing messages onto the LCM bus.

New findings in this pass are P2 only (format string cosmetic). However, prior review threads documented P1 issues that remain present in the diff: the RecordReplay instance created in ModuleCoordinator.replay() is a local variable never stored on the coordinator and never closed (resource/task leak), and _playback_loop only logs to Rerun rather than republishing decoded messages onto the LCM bus, so downstream live modules receive no data during replay. Multiple P1s that are unaddressed pull the score below the 4/5 ceiling.

dimos/core/coordination/module_coordinator.py (replay lifecycle / resource management) and dimos/record/record_replay.py (_playback_loop republish path)

Important Files Changed

Filename	Overview
dimos/record/record_replay.py	New RecordReplay class — async playback loop, seek/trim/delete; referenced in previous review threads for leaked instance and playback not publishing to LCM bus
dimos/core/coordination/module_coordinator.py	New replay() classmethod and auto-replay/record branches in build(); leaked RecordReplay instance and missing LCM republish from previous threads; minor format-string bug in warning log
dimos/core/module.py	Adds start_recording_outputs/stop_recording_outputs RPC methods; correctly uses default-argument pattern to avoid closure capture bug; previous thread concern about msg_name appears resolved
dimos/memory2/observationstore/sqlite.py	Adds delete_range with proper try/except rollback; commit() is outside the if ids: block so it always runs — previous thread concern about misplaced commit appears unfounded
dimos/utils/cli/recorder/run_recorder.py	New Textual TUI for recording/playback; correctly uses the same asyncio event loop as RecordReplay; CSS_PATH resolves correctly relative to module file
dimos/memory2/stream.py	Adds delete_range (delegates to Backend) and publish() helper; data_type property correctly walks source chain; types are consistent
dimos/memory2/backend.py	Adds delete_range delegating to metadata_store with blob/vector cleanup; correctly returns count of deleted IDs
dimos/visualization/rerun/bridge.py	Switches TypeGuard to TypeIs, rewrites is_rerun_multi with structural pattern matching, fixes gRPC port to 9876, passes grpc_port explicitly to serve_grpc
dimos/core/coordination/blueprints.py	Adds record_modules field and default_record_modules() builder method; autoconnect() correctly propagates record_modules across merged blueprints
dimos/core/global_config.py	Adds replay_file and record_path optional config fields for CLI-driven record/replay

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    CLI["dimos run / dimos recorder"] -->|asyncio.run| RunCmd["_run async"]
    RunCmd -->|build| Build["ModuleCoordinator.build"]
    Build --> AutoReplay{replay_file set?}
    AutoReplay -->|yes| Replay["ModuleCoordinator.replay"]
    AutoReplay -->|no| NormalBuild["Build + start modules"]
    NormalBuild --> AutoRecord{record_path set?}
    AutoRecord -->|yes| RecordOut["instance.start_recording_outputs\nper record_module"]
    AutoRecord -->|no| Done["Return coordinator"]
    RecordOut --> Done

    Replay --> RR["RecordReplay instance"]
    RR --> Disable["blueprint.disabled_modules"]
    Disable -->|cls.build patched| Build
    RR --> PlayTask["asyncio.create_task\n_playback_loop"]
    PlayTask --> Loop["Merge-sort observations\nby timestamp"]
    Loop --> RerunLog["rec.log to Rerun viewer"]

    ModuleBase["ModuleBase"] --> SQLite["SqliteStore per module"]
    SQLite --> DB[(SQLite DB)]

    TUI["RecorderApp TUI"] --> LCMSub["LCM subscribe"]
    LCMSub --> DB
    TUI --> PlayTask

_{Reviews (4): Last reviewed commit: "sql transaction fix" | 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.

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ivan/record replay wrap#1976

Ivan/record replay wrap#1976
leshy wants to merge 39 commits intodevfrom
ivan/record-replay-wrap

leshy commented May 5, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

leshy May 5, 2026

Uh oh!

paul-nechifor May 6, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

greptile-apps Bot commented May 5, 2026

Uh oh!

paul-nechifor May 6, 2026 •

edited

Loading

Uh oh!

paul-nechifor May 6, 2026

Uh oh!

paul-nechifor May 6, 2026 •

edited

Loading

Uh oh!

paul-nechifor May 6, 2026

Uh oh!

paul-nechifor May 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		_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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

greptile-apps Bot commented May 5, 2026

Greptile Summary

Confidence Score: 3/5

Important Files Changed

Flowchart

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

paul-nechifor May 6, 2026 •

edited

Loading

paul-nechifor May 6, 2026 •

edited

Loading