Skip to content

fix(perception): use pos_x/pos_y/pos_z metadata keys in spatial location queries#3123

Open
hvent90 wants to merge 2 commits into
dimensionalOS:mainfrom
hvent90:fix/spatial-vector-db-location-keys
Open

fix(perception): use pos_x/pos_y/pos_z metadata keys in spatial location queries#3123
hvent90 wants to merge 2 commits into
dimensionalOS:mainfrom
hvent90:fix/spatial-vector-db-location-keys

Conversation

@hvent90

@hvent90 hvent90 commented Jul 21, 2026

Copy link
Copy Markdown

Contribution path

  • Small, safe change that does not need a tracking issue

Problem

SpatialVectorDB.query_by_location() and get_all_locations() filter stored frames by metadata keys "x"/"y"/"z", but frames are stored with keys pos_x/pos_y/pos_z (see the metadata dict built in SpatialMemory._process_frame). The keys never match, so both methods always return empty results against real spatial-memory data.

Solution

Read pos_x/pos_y/pos_z in both methods.

Added dimos/perception/test_spatial_vector_db.py: stores frames in an in-memory SpatialVectorDB using the exact metadata shape _process_frame writes, and asserts location queries find them. The test fails against the old
code and runs in the default (fast) CI suite.

How to Test

uv run pytest dimos/perception/test_spatial_vector_db.py

Also verified against real recorded data:

First I populated the chromadb: dimos --replay --replay-db go2_china_office run unitree-go2-spatial --disable perceive-loop-skill (--disable because the blueprint's PerceiveLoopSkill requires an agent module it doesn't include — separate issue). Let it log Stored frame at position (…) for ~60s, then stop it.

Then, I ran this script:

uv run python - <<'EOF'
import chromadb
from dimos.perception.spatial_vector_db import SpatialVectorDB

client = chromadb.PersistentClient(path="assets/output/memory/spatial_memory/chromadb_data")
db = SpatialVectorDB(collection_name="spatial_memory", chroma_client=client)
frames = db.image_collection.get(include=["metadatas"])
print(len(frames["ids"]), "frames stored")          # non-zero — the data exists
print("sample metadata:", frames["metadatas"][0])   # keys are pos_x/pos_y/pos_z
print("get_all_locations:", db.get_all_locations()) # [] — the bug
print("query (infinite radius):", db.query_by_location(0, 0, radius=1e9))  # still []
EOF

Running it on main produced empty query results. Running it on this feature branch produced results.

AI assistance

My associate (Fable 5) discovered the issue, proposed a fix, and implemented it. I then reviewed the output and manually ran the verification against the chromadb as outlined in the How to Test section.

…ion queries

SpatialVectorDB.query_by_location and get_all_locations filtered stored
frames on metadata keys "x"/"y"/"z", but frames are written with
pos_x/pos_y/pos_z (SpatialMemory._process_frame), so both methods always
returned empty results. Read the production keys instead, with a
regression test that stores frames using the production metadata shape.
@greptile-apps

greptile-apps Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes spatial location queries to use the metadata keys written for stored frames. The main changes are:

  • Read pos_x, pos_y, and pos_z in spatial location queries.
  • Add tests for radius filtering and missing position metadata.
  • Add coverage for retrieving all stored frame locations.

Confidence Score: 5/5

This looks safe to merge.

  • The updated keys match the metadata written by both spatial frame ingestion paths.
  • The tests cover location filtering, missing coordinates, and full location retrieval.
  • No blocking issues were found in the changed code.

Important Files Changed

Filename Overview
dimos/perception/spatial_vector_db.py Aligns location-query metadata reads with the keys used by spatial frame producers.
dimos/perception/test_spatial_vector_db.py Tests nearby and distant frames, missing position metadata, and complete location retrieval.

Reviews (2): Last reviewed commit: "test(perception): cover frames stored wi..." | Re-trigger Greptile

@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.66667% with 1 line in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
dimos/perception/spatial_vector_db.py 83.33% 0 Missing and 1 partial ⚠️
@@            Coverage Diff             @@
##             main    #3123      +/-   ##
==========================================
- Coverage   72.99%   72.77%   -0.23%     
==========================================
  Files        1031     1034       +3     
  Lines       93177    93964     +787     
  Branches     8479     8609     +130     
==========================================
+ Hits        68018    68384     +366     
- Misses      22835    23251     +416     
- Partials     2324     2329       +5     
Flag Coverage Δ
OS-ubuntu-24.04-arm 66.30% <96.66%> (+0.14%) ⬆️
OS-ubuntu-latest 68.58% <96.66%> (+0.12%) ⬆️
Py-3.10 68.58% <96.66%> (+0.12%) ⬆️
Py-3.11 68.57% <96.66%> (+0.12%) ⬆️
Py-3.12 68.57% <96.66%> (+0.11%) ⬆️
Py-3.13 68.58% <96.66%> (+0.12%) ⬆️
Py-3.14 68.58% <96.66%> (+0.11%) ⬆️
Py-3.14t ?
SelfHosted-macOS ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
dimos/perception/test_spatial_vector_db.py 100.00% <100.00%> (ø)
dimos/perception/spatial_vector_db.py 61.94% <83.33%> (+20.35%) ⬆️

... and 42 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Location queries must skip entries whose metadata has no pos_x/pos_y
rather than crash; covers the skip branch codecov flagged as partial.
@paul-nechifor

Copy link
Copy Markdown
Contributor

Hi @hvent90 . Thank you for the contribution. However, it looks like query_by_location is dead code. Could you instead please delete that method and the query_by_location method which uses it.

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.

2 participants