semantic navigation fix

[sinha7y]

Fix for semantic navigation
= Fixed TF error
=Removed sending full image to avoid timeout

[greptile-apps]

Greptile Overview

Greptile Summary

This PR makes three main changes to fix semantic navigation issues:

Changes Made

1. Frame transformation fix (spatial_perception.py): Changed TF frame lookup from "map" to "world" (lines 224, 513) to align with the perception/simulation subsystem's coordinate frame conventions. The codebase uses both "map" (ROS navigation) and "world" (perception/simulation) frames in different contexts.

2. SkillModule migration (spatial_perception.py): Changed parent class from Module to SkillModule to enable auto-registration of skills to AgentSpec, making spatial memory skills available to agents.

3. Image payload optimization (spatial_vector_db.py): Commented out image retrieval from visual memory in query results (lines 228-229) to prevent timeout issues when sending large image payloads over the network.

Critical Issues

⚠️ BLOCKER BUG: Line 229 in spatial_perception.py has a critical logic error. When tf is None, the code logs a warning but continues execution, leading to an AttributeError on line 233 when calling tf.to_pose(). The return statement must be executed when tf is None.

Additional concerns:

• The TF initialization workaround (lines 190-193) using time.sleep(1.0) is brittle and may not reliably ensure the TF system is ready
• Warning message on line 230 incorrectly references "map -> base_link" instead of "world -> base_link"

Impact

The image payload optimization successfully addresses timeout issues, but the critical bug in error handling will cause runtime crashes when TF transforms are temporarily unavailable, which is a common scenario during system startup or network interruptions.

Confidence Score: 2/5

• This PR contains a critical bug that will cause runtime errors when TF transforms are unavailable
• Score reflects a critical logic error in spatial_perception.py (line 229) where execution continues when tf is None, which will cause AttributeError on line 233. Additionally, the TF initialization workaround using hardcoded sleep is brittle. The spatial_vector_db.py changes are safe.
• dimos/perception/spatial_perception.py requires immediate attention - the error handling logic at line 229 must be fixed before merging

Important Files Changed

File Analysis

Filename	Score	Overview
dimos/perception/spatial_perception.py	2/5	Changed parent class to SkillModule and TF frame from "map" to "world". Critical bug: execution continues when tf is None (line 229), causing AttributeError. Also has brittle initialization workaround with hardcoded 1-second sleep.
dimos/agents_deprecated/memory/spatial_vector_db.py	4/5	Commented out image retrieval from visual memory to avoid timeout issues. Change is straightforward and aligns with PR description's goal to reduce payload size.

Sequence Diagram

sequenceDiagram
    participant SM as SpatialMemory
    participant TF as TF System (LCMTF)
    participant VDB as SpatialVectorDB
    participant VM as VisualMemory
    
    Note over SM: start() method called
    SM->>TF: Access self.tf (triggers lazy init)
    TF-->>SM: LCMTF instance created
    Note over SM: sleep(1.0) - wait for TF
    
    Note over SM: Periodic _process_frame()
    SM->>TF: get("world", "base_link")
    alt TF transform available
        TF-->>SM: Transform object
        SM->>SM: tf.to_pose()
        SM->>SM: Compute embedding
        SM->>VDB: add_image_vector(id, image, embedding, metadata)
        VDB->>VM: add(id, image)
        Note over VM: Image stored
        VDB->>VDB: Add to ChromaDB
    else TF transform unavailable
        TF-->>SM: None
        Note over SM: ❌ BUG: Continues execution
        SM->>SM: tf.to_pose() - AttributeError!
    end
    
    Note over SM: Query operations
    SM->>VDB: query_by_text(text, limit)
    VDB->>VDB: Generate text embedding
    VDB->>VDB: Query ChromaDB
    Note over VDB: Images NOT retrieved (commented out)
    VDB-->>SM: Results (metadata only, no images)

Loading

[greptile-apps]

_{3 files reviewed, 3 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

The code continues execution even when tf is None, which will cause an AttributeError on line 233 when tf.to_pose() is called. The return statement should be executed when tf is None, not just a warning.

Suggested change

	if tf is None:
	logger.warning("No TF transform (map -> base_link) available")
	if tf is None:
	logger.warning("No TF transform (world -> base_link) available")
	return

[greptile-apps]

The warning message is inconsistent with the actual code. Line 224 now requests "world -> base_link" transform, but the warning message still says "map -> base_link".

Suggested change

	logger.warning("No TF transform (map -> base_link) available")
	logger.warning("No TF transform (world -> base_link) available")

[greptile-apps]

The workaround of accessing self.tf and sleeping for 1 second is brittle and doesn't guarantee the TF system is properly initialized. This introduces a race condition where the module may not be ready even after 1 second, or may waste time sleeping when it's already ready.

Consider using a more robust initialization approach:

• Add a check to verify TF is receiving transforms before proceeding
• Use a timeout-based loop that polls for TF availability instead of a fixed sleep
• Or document why exactly 1 second is needed and what specific initialization it's waiting for

[spomichter]

delete

[spomichter]

delete