Repository navigation
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
go2 replay realtime (arm64)
Details
| Benchmark suite | Current: aebb801 | Previous: c240b54 | Ratio |
|---|---|---|---|
peak memory |
1576.324 MB |
1827.461 MB |
0.86 |
peak threads |
361 threads |
364 threads |
0.99 |
network (transport) |
2727.33 MB |
2726.743 MB |
1.00 |
disk write |
1.828 MB |
1.809 MB |
1.01 |
instructions |
116.977 G |
136.831 G |
0.85 |
This comment was automatically generated by workflow using github-action-benchmark.
❌ 2 Tests Failed:
View the top 2 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
bd424bb to
309e817
Compare
TomCC7
changed the title
Review reportUpdated: 2026-10-21. Direct base: #4732.
Behavior changes
Review mapRead the adapters, channel/schema flow, and SDK decoder first. Examples and tests follow that path. All 32 changed files are listed below, relative to the direct base. File map, grouped in review orderRerun adapters
Python browser channel path
TypeScript decoding and subscriptions
Runnable examples and documentation
Tests and reproducible vectors
ValidationCurrent Python lint validation: The previous web checks are from
The earlier report's thread-cleanup error and interrupted web subset did not reproduce with the complete local test environment. Python emitted a subprocess Reproduce the focused checksFrom this PR's repository root, run the existing Python lint commands in its lint environment: uv sync --only-group lint --frozen
uv run --no-sync deptry dimos --config pyproject.toml --github-output
uv run --no-sync mypy --no-incremental
SKIP=cargo-fmt,native-module-lint pre-commit run --all-filesThe earlier web checks used: cd web
deno fmt --check
deno lint
deno install --frozen
deno task check
deno task test
cd sdk
deno task check
deno task test
deno task build
cd ../cockpit
deno task check
deno task test
deno task buildThe installed-wheel check used uv run python -m dimos.web.relay_bridge.tests.cdr_fixturesThe failed CI job log was inspected once to diagnose the lint error; CI was not polled after pushing the fix. Graphical browser inspection, browser end-to-end tests, Docker build and hardware calls were not performed in this revision. The live browser's full interactive flow was not rerun in this revision; its installed Python example's Review focus
Remaining workWIP remains for automatic generated-type discovery, browser-to-robot CDR publishing, business-graph migration and domain-specific browser panels. This revision updates #4733 only; later stack branches have not been restacked onto its new head. Reviewer demos and expected scenes: |
| return entries | ||
|
|
||
|
|
||
| def _json_value(value: Any) -> Any: |
There was a problem hiding this comment.
we need more repo-wide serialization method for common types. check what codebase has and propose what should we do to establish
| from dimos.web.relay_bridge.locate import find_web_dir | ||
|
|
||
|
|
||
| def build_messages() -> list[tuple[str, Any]]: |
 [Medium impact] The PR changes generated-message viewing across the browser and Rerun workflows. Fix source-timeline transform recording before merging; the external-schema compatibility issue is non-blocking. FindingsSummaryAdds generated CDR messages to Rerun and the browser SDK while keeping legacy rendering.
Both reported failures were reproduced. Neither is resolved by the supplied higher dependent PR revisions. Reviews (1) · Last reviewed commit: "fix(viewers): use globalThis for browser..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| stamp = getattr(getattr(msg, "header", None), "stamp", None) | ||
| if stamp is not None: | ||
| nanos = int(stamp.sec) * 1_000_000_000 + int(stamp.nanosec) | ||
| rr.set_time("source", timestamp=np.datetime64(nanos, "ns")) |
There was a problem hiding this comment.
Generated TFMessage timestamps belong to individual transforms, but this callback sets source only from a top-level header. The resulting transforms are recorded on log_time and log_tick, not source, while stamped images and poses are recorded on source. During source playback, Rerun cannot retrieve the transform edges needed to place that sensor data correctly.
A real bridge recording confirmed that all three transform updates were unavailable at their source timestamps, including latest-at queries, while the corresponding images and poses remained available. Set source from each transform’s timestamp when logging it, then restore or clear the timeline state. This playback failure should be fixed before merging.
Knowledge Base Used: Rerun visualization bridge
Artifacts
Executed generated-message bridge reproduction
- The executed script sends generated TF, pose, and image updates through the production bridge and records current behavior plus an external timing control, without modifying production.
Executed source-timeline query and higher-snapshot checks
- The executed inspection script samples actual TF availability with latest-at filling, checks static and log-time controls, and compares scoped higher-PR functions, confirming the failure and absence of a supplied fix.
Executed inspection command wrapper
- The executed wrapper records the exact inspection command, working directory, exit code, and captured output, making the successful verification reproducible.
Observed TF queries and higher-PR comparison output
- The captured successful run shows null source-timeline TF values before, correct values with the timing control, working static/log-time controls, and identical relevant functions in all seven higher heads.
Real Rerun recording from current PR4733 bridge
- Actual generated messages were logged through the current bridge into this recording, whose TF values are unavailable on the source timeline.
Real Rerun recording with external TF timing control
- The same generated message sequence was recorded with source externally assigned around each TF update, making all three TF translations available at their source timestamps.
| const definitions = parse(schema.definition, { ros2: true }); | ||
| const reader = new MessageReader(definitions); |
There was a problem hiding this comment.
cdrDecoder ignores schema.type when parsing the complete definition. Generated schemas leave the root unnamed, so the parser cannot use its package to resolve short field names. A valid external message containing a local Point and a nested geometry_msgs/Point throws Cannot unambiguously determine fully-qualified type name for 'Point'; the decoder factory then returns null, and the registry marks the channel unsupported.
This is a non-blocking compatibility gap for external packages with overlapping short type names. Supply the root identity from schema.type before type resolution and add a regression test covering two packages with a Point type.
Artifacts
Decoder and registry reject a valid complete schema
- Executed the unchanged SDK with the generated schema and binary controls, capturing the exact ambiguity error, null factory result, and undefined registry result.
Decoder and registry succeed with root identity supplied
- Executed the same fixture with an isolated in-memory root-identity change, showing successful decoding without editing production files.
Full reproduction source and generated binary fixture
- Captured the complete authored scripts, fixture, registry source, and generator output so the executed reproduction can be inspected and repeated.
Higher-stack source comparisons and exact source citations
- Compared all supplied heads and printed numbered resolution helpers, showing identical decoder, registry, and exporter blobs across the stack.
This branch was successfully deployed
2 participants
What this adds
How to test
From this PR's checkout:
Use a new output name. The demo runs the real bridge and creates a live RRD plus a source-scene MCAP. Check TF, geometry, images, depth, calibration, clouds and legacy rendering. Native MCAP import visualizes supported standard types; Path and custom EntityMarkers remain reflected fields.
Scene checklist and native MCAP export.
For the browser, run these in separate terminals:
cd web/sdk deno task fixture --host 127.0.0.1Open http://127.0.0.1:5174/demo_cdr.html?live=1. Expect
connected, all eight channels, increasing counts and zero decode errors. Browser checklist.Validation includes 38 focused tests and the installed Rerun example with native MCAP conversion. This layer leaves
dimos mem rerun, navigation anddimos.msgsunchanged.Published CDR dependency update
Consume published
dimos-cdrgen==0.1.2throughout this stack, including packaging examples, the Python lock and the Nix generator wheel. PyPI release and successful publisher run. Downloaded wheel/sdist hashes match that run's artifacts exactly.The external CDR library fix is available as fork release v0.2.4-dimos.1, pinned to its tested commit. Shared SDK API 0.2.0 is isolated in #4774; explicit legacy SDK 0.1.0 pins remain compatible and are respected by autofix. No DimOS PR has been merged.
Final combined-stack verification after installing the published generator: SDK 158, adapters 2, mapper 102, MLS 135, local planner 93, follower 63, V4L2 6 tests; all 12 real native/DimSim exchanges pass. The seven Nix source-staging layouts pass locked Cargo dependency resolution using archive
8101c8db4a50043f1dbd0127fd00220ec9b5a37a. Full Nix builds and hardware/model-dependent tests remain unverified. These are combined-tip results, not independent full test runs of every intermediate PR.Final native CI passed on the exact stack tip, including SDK/consumer builds, Rust tests and all 12 native/DimSim exchanges: https://github.com/dimensionalOS/dimos/actions/runs/39851662834