mem summary cleanup, go2 replay cleanup, progress bars#2730
Open
leshy wants to merge 11 commits into
Open
Conversation
…to dimos mem BlobStore grows an optional size_bytes(stream) capability (None = not cheaply knowable): sqlite answers via SUM(LENGTH(data)) — record-header only, no payload page reads (0.2s on a 9.5 GB db) — and the file store via stat. Backend delegates to its blob store, and Stream.summary() appends the size for unfiltered stream queries. The summary verb moves from `dimos map` to `dimos mem` (its module now lives in memory2/cli with imports deferred so `dimos --help` stays fast).
One row per stream plus a totals row; Items/Hz/Size are heat-colored with the pubsub-benchmark red->green ANSI gradient, log-scaled per column. Stream.size_bytes() becomes public API (None when the query narrows the stream), and piped output gets a wide console so long stream names don't truncate.
mid360 recordings (e.g. china_office) name the raw robot streams go2_lidar/go2_odom; older datasets (go2_hongkong_office) use lidar/odom. ReplayConnection now picks whichever the dataset has and reports the available streams when neither exists.
…e other db verbs Scanning big datasets no longer looks frozen — a transient rich progress bar names the stream being scanned. open_store() goes through resolve_named_path (cwd -> data/ -> LFS pull) instead of requiring an on-disk path, defaulting to .db unless the name says .mcap.
progress() keeps its Stream.tap callback shape but now returns a ProgressBar backed by ONE process-wide rich Progress, so sequential or interleaved streams compose instead of fighting over the terminal. Completed bars persist as a plain line; close() finalizes a bar abandoned early (mem rerun's --seconds window). map replay drops its private copy of the old print-based progress.
leshy
requested review from
mustafab0,
paul-nechifor and
spomichter
as code owners
Contributor
Greptile SummaryThis PR moves memory dataset summary and replay tooling into the memory CLI path. The main changes are:
Confidence Score: 4/5This is close, but the file-backed size accounting should be fixed before merging.
dimos/memory2/blobstore/file.py Important Files Changed
Reviews (6): Last reviewed commit: "Merge branch 'main' into feat/ivan/mem-s..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+503
to
+504
| if self._query.filters or self._query.limit_val is not None: | ||
| return None |
Contributor
There was a problem hiding this comment.
size_bytes() only returns None for filters and limits, so reachable narrowed streams such as stream.offset(100).size_bytes() or search queries without an explicit limit still return the full backend blob size. Callers can receive an incorrect payload size for a subset stream instead of the documented unknown size.
Suggested change
| if self._query.filters or self._query.limit_val is not None: | |
| return None | |
| if ( | |
| self._query.filters | |
| or self._query.limit_val is not None | |
| or self._query.offset_val | |
| or self._query.order_field is not None | |
| or self._query.search_vec is not None | |
| or self._query.search_text is not None | |
| or self._query.live_buffer | |
| ): | |
| return None |
leshy
changed the title
❌ 3 Tests Failed:
View the top 1 failed test(s) by shortest run time
View the full list of 2 ❄️ flaky test(s)
To view more test analytics, go to the Test Analytics Dashboard |
The deferral wrapper bought nothing — memory2/utils/progress.py is a light module (rich is deferred inside it), so import it at the top.
…nly blobs Review (PR #2730): offset/search queries narrowed the stream but still reported full-stream size — treat them like filters/limit (ordering keeps membership, so it stays allowed). FileBlobStore now sums only {key}.bin files instead of everything in the stream dir.
…ed singleton Simpler model: one bar live at a time, per-bar transient Progress, and close() is just stop-the-live-bar + a plain persisted print. Starting a new bar finalizes a dangling one (rich allows a single live display).
Review (PR #2730): match put()'s {key}.bin layout exactly so a stray backup.bin or temp file beside real blobs can't inflate the size.
github-actions
Bot
added
the
ready-to-merge
github-actions
Bot
removed
the
ready-to-merge
Comment on lines
+77
to
+78
| return sum( | ||
| p.stat().st_size for p in stream_dir.glob("*.bin") if p.is_file() and p.stem.isdigit() |
Contributor
There was a problem hiding this comment.
This still counts any digit-named .bin in the stream directory, even when that file was not written as a blob for this stream. A copied 999999.bin, a padded 001.bin, or a symlink named like an observation id passes p.stem.isdigit(), and p.stat() follows symlinks. In those cases Stream.summary() and dimos mem summary can still report bytes that are not payload data for any stored observation. The count needs to be tied to the blob store's owned keys rather than every numeric-looking file present on disk.
github-actions
Bot
removed
the
ready-to-merge
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
1 participant
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.
dimos mem summarymoved fromdimos map summarymem2 blobstores can tell the total size of a stream
rich shared cli progressbar compatible with mem2 streams
dimos mem rerun uses get_data like other mem tooling (previusly it required the full db path)
Go2
--replay- resolvesgo2_lidar/go2_odom(mid360 recordings, e.g. china_office) with fallback tolidar/odom(older datasets, e.g. go2_hongkong_office)