Zach/exp/agents#3168
Open
jeff-hykin wants to merge 33 commits into
Open
Conversation
Add `save_path` to the Rerun bridge (surfaced as `global_config.rerun_save_path` / env `RERUN_SAVE_PATH`). With `start_grpc=True` the server recording is teed to both the live gRPC sink and a FileSink, so a session can be persisted as a `.rrd` while still viewable live. A recording can't own both an in-process gRPC server and a (GrpcSink, FileSink) tee, so the server gets its own recording (kept alive via a module-level ref) and this recording tees to it over loopback. Status: complete. Used by the floorplan/scene pipelines to capture a replayable recording of everything the robot observed.
…item 3) Add `LidarPointCloudClient` (dimos/mapping/pointclouds/live.py): subscribes to `/lidar` (+ optional `/global_map`) over LCM and exposes the accumulated cloud as a plain `(N, 3)` numpy array for scripts/skills running outside the module graph. Extracted from demo_lidar_floorplan.py's ad hoc LidarCollector. Scoped to client-side consumers; in-graph single-message conversion (PointCloud2.points_f32/as_numpy) and grid rasterization already existed and are unchanged. Status: implemented (client-side utility), unit-tested (test_live.py). Not yet wrapped as a callable skill -- no in-graph skill needs live client points yet.
Add `SceneModel` (dimos/mapping/reconstruction/scene_model.py): fuses a mapping session (lidar cloud + robot trajectory from a Rerun recording, optionally colorized by projecting the recorded camera stream) into one queryable 3D model. Analysis: horizontal_section(z) plan cuts and vertical_section(p1,p2) elevations return rasterized Sections with density + mean color. Exports: save_rerun() (interactive colored model), to_mesh()/save_mesh() (Poisson -> GLB/OBJ/PLY/STL with vertex colors), to_voxel_grid(). Demo CLI at scripts/demo_rrd_3d_model.py. Status: prototype (library-only), unit-tested (test_scene_model.py). Foundation for item 1 (floorplan evidence rasters are horizontal_section cuts). Next step: wrap as a callable skill.
…m 1) Promote the floorplan pipeline into dimos/mapping/floorplan/generator.py (FloorplanOptions -> generate_floorplan() -> FloorplanResult.summary()) and wrap it as an agent-callable @Skill in dimos/skills/mapping/floorplan_skill.py (FloorplanSkillContainer.generate_floorplan; every param documented in the MCP schema, debug/3D-model output off by default, automatic temp/session cleanup, never opens a viewer). scripts/demo_lidar_floorplan.py is now a thin CLI wrapper. Registered "floorplan-skill-container" in the module registry. Builds a multi-story DXF/JPEG drawing set from live lidar or a .rrd recording, on top of item 8's SceneModel: per-level evidence rasters are horizontal_section plan cuts, then floor segmentation, wall-vs-furniture by height, indoor/outdoor via ceiling evidence + trajectory connectivity, glazing/doors/stairs, mezzanine detection, AIA layers, and a cleaning stage gated on measured defects. Optional OpenAI passes (classification review, camera-frame confirmation, --ai-render stylized sheets). Voxel stage seals driven-through regions (item 8 VoxelOccupancy.closed_region). --viewer writes <out>.model.rrd. Status: implemented (skill). Unit-tested end-to-end on a synthetic building (test_generator.py, test_floorplan_skill.py). Next step: compose the skill container into the agentic Go2 blueprints so it is reachable over MCP on-robot.
…tem 5) Add dimos/perception/contextual_labeling.py: asks a VlModel (per global_config.detection_model) for an educated guess on an ambiguous detection, feeding the cropped detection image plus context the detector ignores -- the detector's guess + confidence, the object's real-world size, and nearby objects from the ObjectDB scene. Status: implemented (VLM path). Unit-tested with a stub VLM (test_contextual_labeling.py) and exercised for real (moondream) against item 2's replayed Go2 session. Consumed by ObjectSceneRegistrationModule's identify_object/refine_observed_labels skills (next commit). Next step: external data sources (OSM, object DBs) as additional context.
… (item 2) Build the scene catalog on ObjectSceneRegistrationModule (YOLO-E open-vocab detect -> world-frame Object -> dedup/persist in ObjectDB): - Object.locate_encode() (name + world position + size + confidence) and a lidar localizer Object.from_2d_to_list_lidar() for robots without a depth camera (Go2), plus a localization="depth"|"lidar" mode on the module. - Prompt-free list_observed_items() catalog, get_located_objects() RPC, and a bundled catalog_scene() entry point that refines ambiguous items via the item 5 VLM pass then returns the full inventory in one call. - refined_name/refined_description on Object (raw detector name preserved; encoders/find_by_name prefer the refined label) with identify_object() / refine_observed_labels() skills + refine_object_labels() RPC (item 5 wiring). - Spatial-memory fusion: catalog_scene() optionally folds in SpatialMemory's named/tagged places and tags each object with its nearest place, via a new read-only SceneMemorySpec (get_tagged_locations on SpatialMemory/ SpatialVectorDB); degrades to objects-only when absent. Two production bugs found+fixed via replay: ObjectDB TTLs ran on wall clock instead of stream time (offset-clock data pruned before promotion), and Object.to_detection3d_msg() crashed on coplanar lidar clusters (Qhull OBB -> AABB fallback). Status: implemented, replay-validated end-to-end over a recorded Go2 session (test_object_scene_registration_replay.py); lidar localization + fusion + graceful degradation + ChromaDB round-trip unit-tested (test_object_lidar_localization.py, test_object_scene_registration.py). Next step: validate live on the Go2, then feed item 4 (temporal counts) from ObjectDB.
Harden McpServer stop/start so a hot-reload restart rebinds cleanly instead of wedging in the "port listening but never responds" state: - stop(): wake every SSE generator via loop.call_soon_threadsafe (asyncio.Queue is not thread-safe and stop() runs off the event loop; a bare put_nowait could fail to wake the waiter and block uvicorn's graceful shutdown forever), and force_exit + always clear server state even if serve() doesn't settle in time. - _start_server(): idempotent (a spurious second start must not orphan a second uvicorn on the same port); read config via self.config.g (not a fresh global_config import, which sees the forkserver's stale snapshot); and _wait_for_port_free() before binding so a fresh worker doesn't die on EADDRINUSE racing the previous worker's not-yet-released listen socket. Add the agent-mcp blueprint (dimos/agents/agent_mcp.py): a minimal McpServer/ McpClient pair on port 9991 for local MCP Q&A with an LLM, carrying no skills of its own so it can run alongside another MCP server without a port clash. Status: complete.
Add four Go2 blueprints wiring the ObjectSceneRegistrationModule: - unitree-go2-scene: object detect/list/locate in lidar-localization mode (Go2 has RGB + world-frame lidar but no depth camera), dedup radius widened to 0.35m since lidar localization jitters more than depth. Exposes list_observed_items. - unitree-go2-scene-memory: runs the scene module alongside SpatialMemory in one graph so the optional SceneMemorySpec dependency auto-wires and catalog_scene fuses detected objects with named/tagged places. - -agentic variants of both: wrap with McpServer/McpClient so every @Skill is reachable as an MCP tool. Registers all four in all_blueprints. Status: scene / scene-memory implemented and replay/sim-validated; live Go2 validation pending.
Add dimos/skills/skill_verification/: a regression/acceptance suite that drives the *real* skill code (not mocks) for each implemented wishlist skill, asserts machine-checkable invariants, and writes human-reviewable artifacts to test_outputs/<skill-id>/ (indexed by a generated REVIEW.md; artifacts gitignored). manifest.yaml is the source of truth kept in sync by test_00_manifest.py. Covers: 01-floorplan (real-data, self_hosted), 02-identify-locate, 03-lidar-to-numpy, 05-contextual-labeling, 08-scene-3d-model (headless). Idea- stage items (4, 6, 7) have no skill yet and are listed under not_covered. Status: harness complete; headless tiers run anywhere, real-data floorplan test is self_hosted (needs a recording, skipped by default pytest).
…roundwork) Add a "populated environment" scenario: from a single seed, human_activity.py builds a reproducible schedule of everyday tasks (take out garbage, sleep, eat, ...) at named locations for a pool of human actors; HumanActivityDriver plays it against a live DimSim scene, spawning one NPC per human and walking each to its current task location. Config/loader/scheduler are pure and importable without a simulator. Supporting wiring: DimSimClient human-NPC helpers (add/move/get/remove_human) and a run_human_activity conftest fixture that steps the schedule on a daemon thread. Scenario config in fixtures/human_tasks.json. test_dimsim_human_activity.py: schedule determinism/coverage (pure, always runs) + a self_hosted soak that boots DimSim and asserts each human ends at its last task's room. Status: scenario harness complete; groundwork for activity recognition (item 6), which is still idea-stage. Live sim test is self_hosted.
Add master_vqa_test.py: generalizes test_dimos_skills' single-question pattern
into a bank of natural-language questions run against one running robot/scene.
Boots unitree-go2-scene-memory-agentic (McpServer + McpClient + in-graph LLM
agent), routes each question through the full MCP tool-calling loop over LCM so
the agent must pick and call the right registered skill, and writes every
{question, answer} pair to JSON. Question bank in fixtures/vqa_questions.txt;
helpers unit-tested in test_master_vqa_helpers.py.
Status: harness complete; the full-loop test is self_hosted (needs a running
scene + LLM).
Add misc/skills_dashboard/: a local web dashboard rendering the skills wishlist one tab per item. The page is data-driven -- it fetches whatever is in data/<NN-slug>/ (item.json + optional results.xml junit + figures) at load time, so updating a skill's status/tests/figures means republishing files, never editing index.html. Includes per-item data for all 8 items and the figure generator scripts (scripts/) that produce the current figures from real skill code. Status: complete; reflects item statuses as of this branch (1/2/3/5 implemented, 8 prototype, 4/6/7 idea).
Add misc/e2e_tests_dashboard/: a static page documenting the DimSim e2e test suite (what each test drives and why), with a serve.sh helper. Status: complete.
Add dimos/skills/SKILLS_WISHLIST.md: the backlog/status tracker for the perception & scene-understanding skills (mapping, object/scene understanding, activity over time). Per-item status, the agent-callable @Skill quick reference, inter-item dependencies, the MCP interactive test harness notes, and the dashboard publishing workflow. Status snapshot: item 1 (floorplan), 2 (identify/list/locate + memory fusion), 3 (lidar->numpy), 5 (contextual labeling) implemented; item 8 (3D scene model) prototype; items 4 (temporal counts), 6/7 (activity) idea-stage.
…em 3) Build on LidarPointCloudClient with a signals layer (pointclouds/signals.py) and an agent-callable LidarSignalSkills container (skills/mapping/lidar_signal_skills.py, registered "lidar-signal-skills"). New client-side queries over the accumulated (N,3) cloud: extents/percentile bounds, nearest_obstacle, clearance, occupancy_grid, and a coverage_summary. Unit-tested (test_signals.py, test_lidar_signal_skills.py, expanded test_live.py). Status: item 3 upgraded from library-only utility to an implemented skill.
Extend ObjectSceneRegistrationModule: a prompt-driven detect(prompts) skill (open-vocab query on demand), a shared _localize_detections/_ingest_and_publish path, per-detection _lidar_filters, and a _frame_quality_ok gate that skips blurry/low-content camera frames before ingesting. Object keeps the sharper of two crops for a track (_sharper) so refined labels/thumbnails use the best view. Expanded unit tests (+177 lines) covering the new detect path and frame gating. Status: item 2 remains implemented + replay-validated; this hardens the live path.
Substantial generator.py improvements (+333/-89) to the floorplan pipeline and the FloorplanSkillContainer wrapper, exercised on real china-office data (the DXF/JPEG/PDF now shown in the dashboard). Expanded test_generator.py and skill tests. Status: item 1 implemented (skill), real-data validated end-to-end (test_generate_floorplan_from_recording passing).
Guard McpClient's message-processing loop: a single failed turn (model API outage, quota, malformed tool output) previously raised out of the loop thread, killing the agent so every later /human_input hung forever with no signal. Now the error is caught, surfaced as the turn's answer (AIMessage), and the loop keeps serving; idle is re-published when the queue drains. Status: complete.
Scene / scene-memory blueprint refinements and a new unitree-go2-scene-memory-feed blueprint (registered) that drives the full scene-memory-agentic graph from a recorded .rrd replay feed (see rrd_feed.py) so the catalog/skills can be exercised on recorded real-data sessions without live hardware. Status: scene / scene-memory implemented; feed variant supports offline real-data validation.
Add rrd_feed.py: replays a recorded china-office session into the live graph for offline validation of the floorplan/catalog skills. Works around the recording's frame chaos -- the raw .rrd clouds are logged in inconsistent frames and its odom has drifted (the 890MB raw file reads as a fake "6 levels"; the building is 3 floors). Reconstructs clean world-frame clouds from the mem2 gt_pointlio pair + the session URDF and restamps to fix the odom clock skew. Status: works on the china-office session; committed as-is (not extended further this release).
…OLD) Extend the human-activity harness: NPCs are now placed at real furniture anchors resolved live from the scene (bed, sofa, toilet, kitchen counter, ...), and a new ActivityPatrol (activity_patrol.py) drives the robot via /cmd_vel to visit and observe each actor, recording what the robot sees (.rrd) plus ground-truth labels (.groundtruth.json) of who did what and when. DimSimClient gains the NPC/anchor helpers; a robot_patrol conftest fixture wires it up. Status: item 6 on HOLD -- this proves the scene/schedule/recording/patrol machinery end-to-end (self_hosted_large, passing) and generates labeled data, but does NOT recognize activities. Paused for this release: only one usable (unrigged, static-pose) human asset and no annotated dataset to train/validate against. See SKILLS_WISHLIST item 6 hold note.
Grow the master VQA e2e (+419): a larger natural-language question bank routed through the MCP skill-calling loop, a recorded-session question set (vqa_questions_recorded.txt) for replay-fed runs, and expanded helper coverage. Status: harness complete; full-loop test is self_hosted (needs a running scene + LLM).
Republish all 8 item folders with current status, fresh passing results.xml (regenerated from the headless skill-verification suite -- 10/10 pass), and new figures (catalog flow SVGs, china-office floorplan renders/rooms, voxel slicing, scene-model sections). Add scenarios.json + gen_catalog_flow.py generator. Finalize SKILLS_WISHLIST.md for this release: items 1/2/3/5 implemented, 8 prototype, and 4/6/7 explicitly HOLD with rationale (no rigged human assets / no annotated activity dataset). Sync skill_verification README + manifest.
The floorplan generator (dimos/mapping/floorplan) writes DXF drawing sets via ezdxf; declare it in the `mapping` optional-dependency group so the skill is installable. (uv.lock regeneration left out of this commit -- run `uv lock` to sync; the local lock carried unrelated marker-format churn.)
Add report.template.html (the sprint wrap-up: overview + status matrix, a tab per shipped skill with test badges and figures, an on-hold tab for items 4/6/7) and scripts/build_report.py, which inlines every referenced figure as a data URI / inline SVG so the output has zero external requests -- publishable as a shareable artifact or openable over file://. The generated report.html (~2.7MB of embedded figures) is git-ignored; rebuild with `python3 misc/skills_dashboard/scripts/build_report.py`.
Findings from the branch-wide code review (8 finder angles, independently
verified), scoped to the perception core. Each was CONFIRMED against the
code before fixing; failure scenarios below are the verified ones.
CORRECTNESS
1. ObjectDB stream-time clock was non-monotonic and poisonable
(objectDB.py add_objects). Verified failure modes:
(a) one batch with a skewed/future sensor timestamp (cf. the known
unitree_go2_lidar_corrected ~1min odom clock skew) jumped
`now = max(batch ts)` forward, so `_prune_stale_pending`'s cutoff
passed EVERY pending object and the whole pending set was evicted
before promotion, resetting all in-progress promotion counts;
(b) a batch whose objects all carried falsy/zero ts fell back to
time.time() — during replay (stream ts far behind wall clock) the
same total wipe;
(c) a later older-ts batch (detect() on a cached frame) moved the clock
BACKWARDS, so agent_encode/locate_encode reported negative
"last seen -42s ago" ages.
Fix: the clock is monotonic (never moves backwards), non-positive ts
are ignored, an unstamped batch keeps the current stream clock instead
of poisoning it with wall time (wall clock only before any stamped
batch arrived), and _last_now is written under the lock. Mixed-clock
consumer fixed too: ObjectDB.agent_encode() computed ages with
obj.agent_encode()'s wall-clock default while the module RPCs passed
stream time — replay consumers via that path reported absurd
("last_seen 1780000s ago") ages. It now passes the DB's stream clock.
2. Stale mask broke grasping's collision filter (object.py
update_object). update_object pinned image/bbox/mask to the SHARPEST
historical frame (for VLM crop quality), but the mask's only consumer —
get_full_scene_pointcloud's exclude-object path, used by
grasping.py's _get_scene_pointcloud — applies it to the CURRENT depth
frame (re-cached every frame). Verified: no re-projection/re-alignment
exists anywhere between them. After camera motion the dilated stale
mask zeroed the WRONG pixels of the latest depth image: the grasp
target's points stayed in the "scene" collision cloud (grasps rejected
as colliding) and an unrelated region was wrongly carved out.
Fix: mask now ALWAYS tracks the latest detection, restoring the
mask-indexes-current-view invariant; image+bbox stay quality-selected
together (cropped_image uses only image+bbox — verified).
3. Depth path could still crash on degenerate clusters (object.py
from_2d_to_list). The three copies of the OBB-with-AABB-fallback block
had drifted: from_2d_to_list called get_oriented_bounding_box() with
NO try/except, so a coplanar/near-coplanar >=10-point depth cluster —
exactly the Qhull failure mode the code's own comment documents —
raised RuntimeError out of the frame handler. The lidar path caught it;
the depth path did not.
Fix: one shared _bounding_box(pcd, use_aabb) helper used by all three
sites (to_detection3d_msg / from_2d_to_list / from_2d_to_list_lidar),
with the OBB->AABB fallback everywhere. Also removes the
`raise ValueError("aabb requested")` control-flow-by-exception hack.
4. VLM refinement churned forever (contextual_labeling.is_ambiguous +
object_scene_registration refine paths). is_ambiguous() checked only
confidence and the RAW detector name — refined_name was never
consulted and _refine_object never updates confidence/name — so an
already-refined object stayed "ambiguous" forever. Verified
consequence: catalog_scene(only_ambiguous=True) re-queried the VLM for
every low-confidence object on EVERY call (15 objects x 4 calls = 60
redundant VLM round-trips; minutes of latency against the ~120s MCP
tool timeout), and _refine_object unconditionally overwrote
refined_name/description, so "coffee mug" could silently flip to
"cup" between calls.
Fix: a refined object is never ambiguous; identify_object() (which
calls _refine_object directly) still forces re-refinement on demand.
5. detect() with a bare string iterated its characters into nonsense
prompts. Now coerced to [prompt]. (The detect() signature change from
varargs to list[str] was reviewed and kept: verified no in-repo caller
uses the old form; only external/agent callers with stale schemas
would notice, and the MCP schema is cleaner as an explicit list.)
EFFICIENCY (per-frame ingestion hot path)
6. Whole-cloud reprojection per detection (pointcloud.py from_2d).
Verified: as_numpy(), the (N,4) homogeneous hstack, the 4x4 extrinsics
matmul, and the in-image mask ran on the FULL accumulated world cloud
(~1M pts for the 3s window) once PER DETECTION in
from_2d_to_list_lidar's loop — O(D x N) float64 work + D large
temporaries per frame, at camera rate, for a detection-independent
result.
Fix: new ProjectedCloud dataclass — project once per frame, share
across that frame's detections; per-detection work is now just the
bbox mask over in-view points. from_2d keeps a projected=None param
so single-shot callers are unchanged.
7. Unbounded per-object cloud growth (object.py update_object).
Verified: PointCloud2.__add__ is pure concatenation, and
ObjectDB._update_existing calls update_object on every re-sighting —
a "cup" seen 40x carried 40 overlapping merged clouds, and
_ingest_and_publish rebuilds + republishes the full permanent set
(Qhull OBB per object + aggregate vstack of every object's cloud) on
every frame that produced detections, so ingestion got monotonically
slower over a session.
Fix: merged clouds above 20k points are voxel-compacted (0.02 m), so
per-object memory and per-frame publish cost scale with the object's
geometry, not its sighting count. (Throttling the republish itself was
considered and deliberately NOT done — it would change observable
publish behavior; the cloud cap already bounds the cost.)
8. Repeated full-res sharpness scoring (Image.sharpness). Verified: a
plain uncached @Property doing full-frame grayscale + resize +
Laplacian, evaluated on BOTH candidate and stored image on every
matched merge, plus again by the frame-quality gate (twice more in the
rejection f-string) — ~2D+1 recomputations per frame for the same
image. Fix: functools.cached_property (pixel data is treated as
immutable); first read pays, the rest are free.
CLEANUP
9. _process_lidar/_process_3d_detections inlined kwarg-identical copies
of the localization calls that _localize_detections already
encapsulates — the docstring even apologized for the duplication
("has its own per-frame version"). Verified kwarg-for-kwarg equivalent
(both callers cache exactly the inputs _localize_detections reads,
immediately before the call; branch selection guaranteed by start()'s
wiring). Both now call _localize_detections — one localization path
for the background pipeline and detect(), which can no longer drift.
Validation: 129 tests pass across skill_verification (incl. the
real-data china-office floorplan e2e), object-scene-registration,
contextual-labeling, and lidar-localization suites.
…perf
Findings from the branch-wide code review (8 finder angles, independently
verified), scoped to the mapping pipeline. Verification verdicts noted
per finding.
CORRECTNESS — floorplan generator
1. Levels could silently render with NO interior walls (CONFIRMED).
high_z = floor_z + wall_z (1.6 m on the rrd path) was never clamped to
the ceiling. build_levels sets ceiling_z = next_floor - 0.5 and
FLOOR_SEPARATION=1.8 permits auto-detected gaps in [1.8, 2.1) — giving
ceiling_z - floor_z in [1.3, 1.6) < wall_z, so the structural cut
_cut(high_z, z_max) had NEGATIVE thickness. horizontal_section selects
|pz - z| <= thickness/2, which with negative thickness is all-False —
an all-zero evidence raster, no guard, no warning: vectorize() found
no interior walls and the sheet still rendered (envelope/furniture
only). Closely-spaced floors are exactly what odometry-drift-split
floors look like, so this was reachable on real data.
Fix: high_z is clamped below the ceiling (60% of the level height when
the nominal band doesn't fit), with a warning naming both values.
2. SystemExit escaped the skill's error handling and killed the module
(CONFIRMED). build_grids raised SystemExit for oversized grids
(>6000x6000 cells). SystemExit is a BaseException: it sailed through
FloorplanSkillContainer's `except (ValueError, RuntimeError, OSError)`,
through pool.map (Future.result re-raises BaseExceptions), and into
the RPC machinery — killing the hosting agent/module instead of
returning "Floorplan generation failed" to the agent. Reachable via
stray long-range lidar returns (reflections/through-glass): from_rrd
clips only z percentiles, never xy — the code's own comment
acknowledges the xy-inflation problem. Fix: raise ValueError.
3. SessionImagery raced across level threads (CONFIRMED). The shared
instance is passed to every process_level call dispatched via
ThreadPoolExecutor(max_workers=len(levels)); process_level ->
visually_confirm_glazing -> best_view -> _images() did UNLOCKED lazy
init of the multi-GB SqliteStore. Two level threads could both pass
`if self._stream is None` — double-opening the store (one handle
leaked; close() only stops the last-assigned) — and then interleave
find_closest on one sqlite connection opened with
check_same_thread=False and no app-level serialization: cursor errors,
or frames returned for the WRONG query, i.e. glazing verdicts applied
to the wrong walls. Fix: a threading.Lock serializes both the lazy
open and every query.
CORRECTNESS — scene model
4. save_rerun_sliceable crashed on sparse scans (PLAUSIBLE — boundary
condition verified, no guard existed). occ = vox.occupancy(min_hits=2)
all-False -> a (0,3) centers array -> z.min() raises ValueError
("zero-size array to reduction operation") — killing the
save_3d_model/--viewer path AFTER the rest of the floorplan pipeline
had already succeeded. Fix: skip the voxel layer with a warning when
nothing clears min_hits.
EFFICIENCY
5. load_rrd_points used the exact slow path its sibling had already
measured (CONFIRMED). Points3D extraction via col.to_pylist() +
per-row np.asarray materializes every coordinate as a Python object —
hundreds of millions of floats for the 890MB chinaOffice.rrd, several
GB of peak churn, "minutes instead of seconds" per rrd_feed.py's own
docstring benchmark of the SAME schema in the SAME branch (the perf
lesson never crossed files). Fix: flatten
list<fixed_size_list<f32>[3]> straight from the pyarrow buffers
(col.values.values.to_numpy), with the pylist loop kept as fallback.
6. Despeckle was O(n_components x H x W) (CONFIRMED). The loop cleared
small connected components one at a time with
`high_mask[labels == i] = 0` — a full-raster boolean compare per
component, on grids allowed up to 6000² = 36M cells; a noisy level
with a few hundred speckle components cost billions of comparisons.
Fix: one vectorized pass — np.flatnonzero over the area stats +
np.isin(labels, small).
7. LidarPointCloudClient grew without bound and re-vstacked per query
(CONFIRMED, flagged independently by three finder angles). _on_lidar
appended every message's full point array to _chunks forever — no
cap, decimation, or dedup (contrast _PoseTracker's MAX_POSES=50k with
2x decimation), and nothing ever called clear(). snapshot()
re-vstacked the ENTIRE history on every call, and every derived skill
query (measure_space, nearest_obstacle_distance, check_clearance,
coverage_report — plus the is-ready check) goes through snapshot().
On a Go2 at ~7 Hz for an hour: gigabytes of growth and multi-second
MCP tool calls, ending in OOM.
Fix: accumulation is bounded — above max_buffered_points (2M) the
chunks are voxel-compacted (0.05 m), so memory scales with scanned
VOLUME rather than session length (overlapping sweeps mostly revisit
the same voxels); and snapshot() is cached, invalidated only when new
data arrives, so polling skills pay nothing between messages.
CLEANUP
8. quat_to_matrix was the third copy of quaternion->matrix math in the
repo — two added by this very branch, already drifted on zero-norm
handling (`or 1.0` guard vs unguarded divide). Now a thin wrapper over
the shared Quaternion type (zero-norm -> identity), keeping ONE
implementation.
9. process_level recomputed the identical on-level trajectory band
filter (floor_z - 0.4 .. ceiling_z) a second time — character-identical
constants, hand-copied — just to take the last point for the robot
marker. The marker is now derived from the traj_xy already computed at
the top of the function, so a future dwell-band tweak can't put the
robot marker on the wrong level while traj_xy stays correct.
Findings from the branch-wide code review (8 finder angles, independently
verified), scoped to the MCP server/client. The server finding was
flagged independently by three finder angles (line-scan, removed-behavior
audit, altitude audit) and CONFIRMED by the verifier.
McpServer — the bind-failure wedge (CONFIRMED):
_start_server assigned self._uvicorn_server BEFORE serve() ever bound,
and the serve future was never inspected — no done-callback, no check of
uvicorn's server.started flag. Two verified failure paths reached it:
(a) the previous worker held the port longer than _wait_for_port_free's
5s budget — on timeout the code only logged and proceeded;
(b) the TOCTOU window between the probe socket's close and uvicorn's own
bind (another process grabs the port in between).
Either way uvicorn calls sys.exit(1) INSIDE serve(), so a SystemExit sat
unread in _serve_future while _uvicorn_server stayed non-None — and the
idempotence guard (`if self._uvicorn_server is not None: return`) turned
every later start into a silent no-op. Net effect: the MCP endpoint dead
for the whole session with only a log line — the exact "port bound but
never responds" wedge the original patches targeted, moved one step
later. Bonus wrinkle: SystemExit is a BaseException, so stop()'s
`except Exception` around future.result() would not even catch it at
teardown.
Fix:
- _start_server now waits (up to 5s) for uvicorn to CONFIRM the bind via
server.started. If serve() ends first, the state is cleared — a later
start() can retry instead of no-opping forever — and a RuntimeError is
raised naming host:port with the underlying exception chained, so the
failure is loud at the moment it happens rather than swallowed at
stop(). A slow-but-alive startup past 5s logs a warning and proceeds.
- stop() catches BaseException when settling the serve future, so a
bind-failure SystemExit is logged rather than blowing up teardown.
(stop()'s deliberate null-out-on-timeout behavior is kept: the comment
documents the trade-off — never block a later rebind.)
McpClient — the error guard could kill the loop it protects (PLAUSIBLE;
sub-claim (iii) CONFIRMED):
The per-turn exception guard exists so one failed turn (model API
outage, quota, malformed tool output) doesn't end the agent loop thread
for the session. But its recovery path — history append + publish + idle
re-publish — ran UNGUARDED inside the except block: if the original turn
failure WAS agent.publish() raising inside _process_message, the
handler's own publish raised again and killed the thread anyway (Python
threads cannot be restarted; every later /human_input hung forever with
no signal — precisely what the guard's comment says it prevents).
Verified secondary issue: the handler appended the error AIMessage to
self._history OUTSIDE self._lock while every other _history mutation
runs under it (benign in practice — only the worker thread mutates
_history — but an inconsistency worth closing).
One reviewer sub-claim was REFUTED and deliberately NOT "fixed": the
"No state graph initialized" branch is effectively unreachable, because
on_system_modules starts the worker thread under the lock AFTER
assigning _state_graph, and _state_graph is never nulled — messages
queued before startup simply wait in _message_queue.
Fix: the entire recovery path is wrapped in its own try/except (a
failure to surface the error is logged and the loop continues), and the
history append happens under self._lock like every other mutation.
Validation: all 43 MCP unit tests pass (test_mcp_server,
test_mcp_client_unit, test_tool_stream).
Findings from the branch-wide code review (8 finder angles, independently verified), scoped to the e2e harness, visualization init, and blueprints. All findings below were CONFIRMED by the verifier pass except where noted. rrd_feed — NaN quaternions published to /odom and tf (CONFIRMED): _matrix_to_quat used the naive w-only extraction: w = sqrt(1+trace)/2, then x/y/z divide by 4w. For a (near-)180-degree rotation trace(R) = -1, so w = 0 and the divisions are 0/0 — numpy emits only a RuntimeWarning and the NaN quaternion was silently published into that frame's world->base_link tf and /odom PoseStamped, corrupting every downstream camera/lidar projection and localization for those frames; near-180 poses lost precision the same way. Fix: delegate to the shared Quaternion.from_rotation_matrix (scipy Rotation — the robust largest-component method). _quat_to_matrix likewise delegates, deleting this branch's second and third hand-rolled quaternion implementations (the copies had already drifted on zero-norm handling). rrd_feed — session calibration was baked constants (CONFIRMED, altitude finding): CAMERA_INTRINSICS was hand-copied from the china-office session's camera_intrinsics.json even though the machine-readable file sits in the SAME directory as the mem2.db the feeder already reads — a re-calibrated or different session would silently project detections through stale constants (wrong world positions, no error). Fix: new load_camera_intrinsics() parses the session's own camera_intrinsics.json (same schema colorize_from_session reads), derived from mem2_db_path's parent with zero signature changes; the constants remain as fallback. (The URDF rig-chain constants were reviewed and deliberately left baked: machine-parsing the URDF is out of proportion for an e2e harness.) DimSim e2e harness (all CONFIRMED): - wait_for_scene leaked a live SceneClient per retry: on an exec exception it nulled self._client WITHOUT stop(). SceneClient.start() spawns a websocket + _recv_loop daemon thread whose loop condition never sees a dropped Python reference — so on the documented boot condition (bridge accepts the socket, browser sandbox not ready, exec times out) the 180s/3s poll accumulated up to ~60 live sockets+threads in the test process. Fix: stop() before dropping the reference. - remove_human left ghost humans in the shared sim: fallback humans (GLB load failure -> plain cylinder via add_object) are not in the NPC registry, so remove_npc() returned False and the cylinders persisted across tests — the robot's camera kept "seeing" person-sized objects from the previous scenario, corrupting later human-activity/VQA assertions. Fix: fall through to removing the mesh by name. - run_human_activity's stepping thread died silently: the run() loop called driver.step() with no guard, and SceneClient.exec provably raises TimeoutError/SceneExecError on transient failures (browser GC pause, reconnect blip). One raise killed the daemon thread, every NPC froze, and the test later failed on an unrelated-looking assertion with no log pointing at the dead thread. Fix: per-step try/except with a warning; the loop keeps stepping. - master_vqa ask() mispaired answers after a timeout: completion detection accepted ANY agent_idle=True after the send marker — no correlation id, no drain of an in-flight turn. After question N timed out, run_qa_pass proceeded to N+1 while the agent was still working N; N's late idle could satisfy N+1's completion check (final_answer takes the last content-bearing AIMessage of the raw slice — N's answer recorded as N+1's), and even in the benign ordering N's tail messages inflated N+1's tool_calls and token/cost accounting. Fix: ask() first settles any still-running turn (bounded wait for idle=True) before snapshotting markers and sending. - skill_verification presented a STALE REVIEW.md as current under pytest-xdist: collectors are fixture-created, so they exist only in workers — workers early-return on workerinput, and the controller never had a collector — so NO process wrote the summary and the previous run's index (old checkmarks included) survived untouched. Fix: the controller/non-xdist session deletes REVIEW.md at session start — an absent index is honest; a stale one is not. - activity_patrol re-implemented wrap-to-[-pi,pi] with a while loop; now uses the shared dimos.utils.transform_utils.normalize_angle (constant-time arctan2 form, third private copy avoided). Visualization / blueprints: - rerun_init's serve path collapsed from a 3-way branch to connect vs serve: the save_path arm duplicated the serve_grpc call + "ready at" logging of the plain arm, so a future serve_grpc change had to be made twice or the paths would drift (verified minor). The tee is now applied after a single serve_grpc call with conditionally-built kwargs. A repeated serve+save init in one process now WARNS before replacing the module-level server recording (the file's own comment notes dropping it shuts down the gRPC server; verified PLAUSIBLE — reaching that branch twice requires the old server to already be dead, so the overwrite itself is harmless, but it should not be silent). - unitree-go2-scene-memory composed from bare unitree_go2 while re-declaring the entire ObjectSceneRegistrationModule tuning block (target_frame, localization="lidar", camera_optical_frame, distance_threshold=0.35, min_detections_for_permanent=3) verbatim from unitree_go2_scene.py — its own comment said "See unitree_go2_scene.py". The next retune would land in one file and not the other, silently forking scene vs scene+memory behavior (and their agentic wrappers would inherit the divergence). Fix: compose autoconnect(unitree_go2_scene, SpatialMemory.blueprint()) — verified the composed blueprint carries the identical module config (one OSR atom, full tuning, n_workers=9 override) and the registry regeneration test passes. (unitree_go2_scene_memory_feed was checked and is NOT a third copy: it deliberately diverges — sparse filter preset, lower promotion threshold — and intentionally omits the live-robot base.)
detect() now searches the accumulated catalog first (via _match_located_objects) and merges fresh frame detections, so a directed "search for X" finds items seen earlier even when they're out of the current view — previously it only ran the promptable detector on the latest frame and reported nothing for already- catalogued objects. Updated its docstring and the no-frame test message; added a test for the catalog-search path. System prompt: add "Answer Completely — Never Defer" and "Work the Problem — Best-Effort Reasoning" so the agent stops answering questions with an offer to answer them, and reasons over catalog coordinates / chains tools instead of handing analysis back to the user. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…oorplan
generate_floorplan runs for minutes (live lidar-collection window +
drawing pipeline + parallel OpenAI review/render calls), but the
default 120s RPC timeout and 120s McpClient HTTP timeout cut it off
mid-render. Raise them as a consistent chain so the innermost layer
fails first with a real error instead of an outer wait firing early:
- generate_floorplan RPC: 600s (DEFAULT_RPC_TIMEOUTS)
- McpClient HTTP client: 630s (one step above the RPC ceiling)
- VQA answer wait: FLOORPLAN_ANSWER_TIMEOUT_S 660s, with the
per-test @timeout budgets raised to match two floorplan asks.
Also drive the collision-safe office tour (explore_office) before the
VQA Q&A pass: spinning at the spawn corner alone saw none of the
office furniture, leaving every perception/floorplan answer empty.
Adds ezdxf to the lockfile (DXF generation).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add an optional test_command to each skill's item.json (string or
{cmd, note}) and render it as a "▶ Test it" one-liner in index.html
so each panel shows the exact invocation that exercises the skill.
Populate it for the floorplan, identify-locate, lidar-to-numpy,
contextual-labeling, and scene-3d-model items.
Reworks report.template.html and scripts/build_report.py for the
self-contained wrap-up report accordingly.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jeff-hykin
requested review from
leshy,
mustafab0,
paul-nechifor and
spomichter
as code owners
July 24, 2026 15:22
Contributor
|
Too many files changed for review. ( Bypass the limit by tagging |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| function fsmSvg() { | ||
| const st = (x, y, l, c, active) => `<rect x="${x - 55}" y="${y - 20}" width="110" height="40" rx="20" | ||
| fill="${active ? c.replace('--', '--') + '' : 'var(--surface)'}" fill-opacity="${active ? 0.18 : 1}" stroke="${c}" stroke-width="2.5"/> |
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
2 participants
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.
_