Conversation
- SqliteVectorStore: only catch "no such table" OperationalError, re-raise others - test_visualizer: fix det.ts → obs.ts, add @pytest.mark.tool, remove double teardown and duplicate assertion - conftest: remove unreachable duplicate return
…ypes - EmbeddingModel/CLIPModel: @overload so single-arg returns Embedding, multi-arg returns list - conftest: cast getfixturevalue returns to correct Store/BlobStore types
- MobileCLIPModel, TorchReIDModel: add matching overloads for embed/embed_text - Remove now-redundant cast in memory/embedding.py
…n guard, strict zip - Delete dimos/memory/type.py (license header only, no code) - Add validate_identifier() to search() and delete() in SqliteVectorStore - Change zip(strict=False) to zip(strict=True) in Batch transformer
# Conflicts: # dimos/memory2/test_visualizer.py # dimos/memory2/transform.py
There was a problem hiding this comment.
Pull request overview
This PR extends the memory2 streaming architecture with (1) a StreamModule base that wires In→memory2.Stream pipelines→Out, (2) support for unbound stream pipelines via Stream() + .chain(), and (3) voxel-map accumulation refactored into a reusable VoxelGrid + VoxelMap transformer, alongside typing improvements and new tool-only replay/visualization tests.
Changes:
- Add unbound
memory2.Streampipelines (Stream()),.chain()to bind them to real streams, and.publish()to driveOutports. - Introduce
StreamModule(defaulting to a live-onlyNullStore) and update voxel mapping to a framework-agnosticVoxelGrid+VoxelMaptransformer. - Add embedding API overloads + new MemoryStore retention controls (
max_size,NullStore), and add tool-marked replay DB tests (with LFS artifact).
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| dimos/simulation/unity/test_unity_sim.py | Add stop() to test transport mock to support new module shutdown behavior. |
| dimos/robot/all_blueprints.py | Register new "stream-module" module entry. |
| dimos/models/embedding/treid.py | Add overloads for embed/embed_text return typing. |
| dimos/models/embedding/mobileclip.py | Add overloads for embed/embed_text return typing. |
| dimos/models/embedding/clip.py | Add __future__ annotations + overloads for embed APIs. |
| dimos/models/embedding/base.py | Define overloads on the abstract EmbeddingModel interface. |
| dimos/memory2/vectorstore/sqlite.py | Validate identifiers; handle missing vec tables by returning empty results. |
| dimos/memory2/transform.py | Add Batch transformer and stride() utility. |
| dimos/memory2/test_voxel_map.py | New tool-marked tests for voxel-map accumulation + replay integration. |
| dimos/memory2/test_visualizer.py | New tool-marked “visualizer” workflow tests over replay DB. |
| dimos/memory2/test_save.py | Update expected error message substring for .save() restrictions. |
| dimos/memory2/test_module.py | New tests for unbound streams, .chain(), StreamModule, and NullStore behavior. |
| dimos/memory2/test_e2e.py | Add tool-marked embed-and-search workflow using CLIP + EmbedImages. |
| dimos/memory2/stream.py | Add unbound streams, .chain(), .publish(), improved errors, and stricter live/save/append guards. |
| dimos/memory2/store/null.py | Add NullStore (live-only, max_size=0) convenience store. |
| dimos/memory2/store/memory.py | Implement MemoryStoreConfig.max_size and wire it into backend creation. |
| dimos/memory2/store/README.md | Update docs to reflect Store/Backend/ObservationStore roles and new layout. |
| dimos/memory2/observationstore/memory.py | Add max_size retention via deque(maxlen=...). |
| dimos/memory2/module.py | Add StreamModule that bridges Module ports to memory2 streams. |
| dimos/memory2/conftest.py | Add CLIP fixture for tool tests and tighten fixture typing via cast(). |
| dimos/memory/embedding.py | Remove now-unneeded cast() thanks to embed() overload typing. |
| dimos/mapping/voxels.py | Refactor voxel mapping into VoxelGrid + VoxelMapTransformer; reintroduce mapper as StreamModule. |
| dimos/mapping/test_voxels.py | Update tests to use VoxelGrid directly and dispose resources explicitly. |
| dimos/mapping/pointclouds/test_occupancy_speed.py | Switch benchmark to use VoxelGrid instead of the old mapper module. |
| dimos/hardware/sensors/lidar/fastlio2/fastlio_blueprints.py | Update blueprint wiring to new VoxelGridMapper config (no publish_interval). |
| dimos/core/stream.py | Add Stream.stop() to stop transports on module shutdown. |
| dimos/core/module.py | Stop transports for all In/Out ports during module close to avoid leaks/cycles. |
| data/.lfs/go2_bigoffice.db.tar.gz | Add LFS pointer for replay DB artifact used by tool tests. |
Comments suppressed due to low confidence (2)
dimos/memory2/transform.py:91
- Batch.init should validate batch_size > 0 (raise ValueError) to avoid surprising behavior (e.g., batch_size=0 causes every element to be flushed immediately, negative values behave similarly).
def __init__(self, fn: Callable[[list[T]], Sequence[R]], batch_size: int = 16) -> None:
self._fn = fn
self._batch_size = batch_size
dimos/memory2/test_visualizer.py:55
- There are several spelling mistakes in these comments (e.g., "dissaper" -> "disappear"). Please fix to keep docs/comments readable.
# these need to be functions that are easily called to render latest query results in different ways
# I can imagine querying for multiple things and I want previous visualization to dissaper, being replaced
# by latest results. we might do this transparently so agent queries are also visible in real time
#
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
register_disposable() expects DisposableBase, but Module port .subscribe() returns a plain callable. Wrap with Disposable() at call sites to fix all 11 mypy type-var errors.
paul-nechifor
previously approved these changes
Mar 27, 2026
1 task
…_size VoxelGrid is a plain class without a config attribute. The merge from dev incorrectly referenced self.config.voxel_size.
paul-nechifor
previously approved these changes
Apr 5, 2026
Resolve 3 conflicts keeping register_disposable() over _disposables.add(): - costmapper.py, memory2/stream.py, rerun/bridge.py
…into feat/memory2-svgvis
Resolve 3 conflicts keeping HEAD's refactored versions: - module.py: Configurable[ModuleConfigT] + CompositeResource - voxels.py: standalone VoxelGrid class (not Module subclass) - notifier/base.py: Configurable[NotifierConfig] + Resource
- ModuleBase: use unparameterized Configurable + explicit config annotation - Notifier: drop type parameter from Configurable - StreamModule/VoxelGridMapper: remove type parameters, add start/stop stubs - Remove dead default_config attributes across memory2 and mapping
paul-nechifor
previously approved these changes
Apr 9, 2026
spomichter
approved these changes
Apr 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.