Skip to content

Conversation

@leshy
Copy link
Contributor

@leshy leshy commented Dec 17, 2025

run foxglove-bridge

export ALIBABA_API_KEY=...
pytest -sv dimos/agents2/skills/interpret_map/eval/test_ivan_eval.py
2025-12-17_17-20

@chatgpt-codex-connector
Copy link

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@greptile-apps
Copy link

greptile-apps bot commented Dec 17, 2025

Greptile Summary

This PR adds point detection capabilities to the VLM (Vision-Language Model) system, enabling VLMs to identify specific point locations in images rather than just bounding boxes.

Key changes:

  • Added Detection2DPoint class for representing 2D point detections with circle visualization support
  • Extended VlModel base class with query_points() method for point-based queries
  • Added native point detection support to MoondreamVlModel using Moondream's point() API
  • Extended ImageAnnotations and ImageDetections to support circle annotations
  • Added test infrastructure for map comprehension experiments

Critical issue:

  • Detection2DPoint is missing the required abstract method to_ros_detection2d(), which will cause TypeError at instantiation time

Confidence Score: 2/5

  • PR contains a critical abstract method implementation error that will cause runtime failures when using Detection2DPoint
  • The missing to_ros_detection2d() abstract method in Detection2DPoint will cause a TypeError when attempting to instantiate the class. This is a blocking issue that needs to be fixed before merging.
  • dimos/perception/detection/type/detection2d/point.py - missing abstract method implementation; dimos/agents2/skills/interpret_map/eval/test_ivan_eval.py - undefined variable bug

Important Files Changed

Filename Overview
dimos/perception/detection/type/detection2d/point.py New Detection2DPoint class for 2D point detections. Missing required abstract method to_ros_detection2d() which will cause instantiation to fail at runtime.
dimos/agents2/skills/interpret_map/eval/test_ivan_eval.py New test file for map comprehension experiments. Contains undefined variable target (should be state.target) and several unused imports.
dimos/models/vl/base.py Added query_points() method to VlModel base class and vlm_point_to_detection2d_point() helper function for converting VLM point results to Detection2DPoint.
dimos/models/vl/moondream.py Added query_points() method using Moondream's native point detection API. Converts normalized coordinates to pixel coordinates correctly.
dimos/perception/detection/type/detection2d/base.py Made to_text_annotation() and to_points_annotation() non-abstract with empty defaults. Added to_circle_annotation() method for circle visualizations.

Sequence Diagram

sequenceDiagram
    participant User
    participant VlModel
    participant MoondreamVlModel
    participant Detection2DPoint
    participant ImageDetections2D
    participant ImageAnnotations

    User->>VlModel: query_points(image, query)
    VlModel->>VlModel: _prepare_image(image)
    VlModel->>VlModel: query_json(scaled_image, full_query)
    
    loop For each point tuple
        VlModel->>Detection2DPoint: vlm_point_to_detection2d_point()
        Detection2DPoint-->>VlModel: Detection2DPoint instance
        VlModel->>ImageDetections2D: append detection
    end
    
    VlModel-->>User: ImageDetections2D

    User->>ImageDetections2D: to_foxglove_annotations()
    ImageDetections2D->>Detection2DPoint: to_circle_annotation()
    ImageDetections2D->>Detection2DPoint: to_text_annotation()
    ImageDetections2D-->>User: ImageAnnotations
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

11 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

Comment on lines +36 to +46
class Detection2DPoint(Detection2D):
"""A 2D point detection, visualized as a circle."""

x: float
y: float
name: str
ts: float
image: Image
track_id: int = -1
class_id: int = -1
confidence: float = 1.0
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: Missing abstract method to_ros_detection2d() required by base class Detection2D. This will cause TypeError: Can't instantiate abstract class Detection2DPoint with abstract method to_ros_detection2d at runtime when creating instances.

See Detection2DBBox.to_ros_detection2d() at dimos/perception/detection/type/detection2d/bbox.py:395 for reference implementation.


if state.target:
pose: LCMTransport[PoseStamped] = LCMTransport("/target", PoseStamped)
pose.publish(target)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

syntax: target is undefined. Should be state.target.

Suggested change
pose.publish(target)
pose.publish(state.target)

s-desh and others added 27 commits December 29, 2025 15:23
Former-commit-id: 9fe9955 [formerly 033bd4f]
Former-commit-id: 29e4639
Former-commit-id: 72f0f3f [formerly 1aac218]
Former-commit-id: 2be2a98
Former-commit-id: e2c0671 [formerly cc8e7d4]
Former-commit-id: d767a44
Former-commit-id: b9b8b5a [formerly 78618a8]
Former-commit-id: 4576507
Former-commit-id: d588398 [formerly 933fb5a]
Former-commit-id: bf96d46
…tree g1 or go2

Former-commit-id: 3cdaf25 [formerly dab3fcc]
Former-commit-id: 9304e5a
Former-commit-id: bead6be [formerly ee3033d]
Former-commit-id: fa4aaba
Former-commit-id: 886251e [formerly ebd6acc]
Former-commit-id: 65fad89
Former-commit-id: d2947c4 [formerly 6f9bcc4]
Former-commit-id: 0ecb57f
…n-fixes

G1 navigation documentation fixes

Former-commit-id: 035211b [formerly 19ebd69]
Former-commit-id: fd6bc41
Former-commit-id: cfb4696 [formerly 308cc3d]
Former-commit-id: 2afdcc3
Former-commit-id: ef0437f [formerly bf03e04]
Former-commit-id: e5802f8
Former-commit-id: 5e9c8d9 [formerly aecb746]
Former-commit-id: 0c963c7
Former-commit-id: f910d2e [formerly bef8b9c]
Former-commit-id: 0e1cd2d
Former-commit-id: 3aa6ddc [formerly e0fcd4c]
Former-commit-id: 5f4cef4
Rename dimos-robot to dimos

Former-commit-id: 539f8b1 [formerly 1bd74d5]
Former-commit-id: 199ec56
Former-commit-id: 7b9edcd [formerly 05181ba]
Former-commit-id: 9e04d60
Former-commit-id: 2a44c9f [formerly 390152b]
Former-commit-id: 69f32c4
Former-commit-id: 389f0d2 [formerly 8fcbfa2]
Former-commit-id: c09e260
Former-commit-id: 647c0ed [formerly a38b6f6]
Former-commit-id: eea3c23
Former-commit-id: 49848de [formerly bbc5416]
Former-commit-id: 877291e
Former-commit-id: 6a08eb6 [formerly 0f99eb7]
Former-commit-id: 0be0dab
Former-commit-id: 78c0ba7 [formerly b9d1155]
Former-commit-id: 5c80db4
… use improvement)

Former-commit-id: dfc2e79 [formerly 4d5b07e]
Former-commit-id: 8eeafc7
Former-commit-id: b43b348 [formerly 32749e8]
Former-commit-id: 3198f3f
Former-commit-id: 733e2c9 [formerly 57bb639]
Former-commit-id: f25958b
s-desh and others added 17 commits January 4, 2026 17:29
Former-commit-id: db6bf76
Former-commit-id: 599b66f
Former-commit-id: 6fbe4eb
Former-commit-id: 7dad55a
Former-commit-id: b5590e2
Former-commit-id: 6377930
* captioner modules implemented in models/vl, flake.nix fixes

* model structure rework

* refactor

* bugfix

* removed double update_intrinsic on metric3d

* mypy

* typing fixes

* embedding models rewrite

* mobileclip preprocess accessor rewrite

* torch reid models added to lfs, reid/embedding model cleanup

* mobileclip upload

* batch vlm querying

* moondream batch queries and tests

* type fixes

* proper model resource management, speed tests, auto-resizing, plotting

* type fixes

* tests, mypy, correct cleanup

* metric3d tests

* attempting to remove dead code

* scaling bugfix for visual models

* docstring fix

* plotext dep

* open clip dep

* open clip dep fix

* gdown dep

* tensorboard dep

* typing fixes for detections and plotter

* person tracker typing fix

* py 3.10 typing fix

* last type fix

* ignore missing imports (for ros deps)

* nicer init for florence

* type fixes

* mypy ignore ros/mujoco

* addressing PR comments

* image is a fixture

* captioner fixtures

* all PR comments addressed

Former-commit-id: 8be510d [formerly cbe68d2]
Former-commit-id: b70ed02
Former-commit-id: c634412
Former-commit-id: dc3c687
Former-commit-id: 2d0b444
Former-commit-id: 00a1719
Former-commit-id: 79eb8c0
Former-commit-id: f0f486a
Former-commit-id: 82a2bea [formerly 80493c4]
Former-commit-id: ce11d1c
Former-commit-id: d072bae [formerly b1af7f7]
Former-commit-id: d0cd157
Former-commit-id: 842021d
Former-commit-id: 86d0b7c
Former-commit-id: 15f8bb1
Former-commit-id: 6ff050c
Former-commit-id: 7165549
Former-commit-id: 260c101
Former-commit-id: 61dcc38
Former-commit-id: 94610ca
Former-commit-id: 37afea0
Former-commit-id: d27ee89
@spomichter spomichter force-pushed the map_comprehension_experiments branch from 37afea0 to d27ee89 Compare January 8, 2026 13:59
@spomichter spomichter requested a review from a team January 8, 2026 13:59
@greptile-apps
Copy link

greptile-apps bot commented Jan 8, 2026

Too many files changed for review.

1 similar comment
@greptile-apps
Copy link

greptile-apps bot commented Jan 8, 2026

Too many files changed for review.

Former-commit-id: 22e5cd2
@greptile-apps
Copy link

greptile-apps bot commented Jan 8, 2026

Too many files changed for review.

@greptile-apps
Copy link

greptile-apps bot commented Jan 8, 2026

Too many files changed for review.

@greptile-apps
Copy link

greptile-apps bot commented Jan 8, 2026

Too many files changed for review.

1 similar comment
@greptile-apps
Copy link

greptile-apps bot commented Jan 8, 2026

Too many files changed for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants