fix(exploration): do not block agent loop#1258
Conversation
Greptile OverviewGreptile SummaryThis PR makes
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User as User (humancli)
participant Agent as Agent
participant NavSkill as NavigationSkillContainer
participant Explorer as WavefrontFrontierExplorer
participant Nav as NavigationInterface
User->>Agent: "start exploring"
Agent->>Agent: agent_idle.publish(False)
Agent-->>User: ThinkingIndicator.show()
Agent->>Explorer: begin_exploration()
Explorer->>Explorer: explore() → spawn thread
Explorer-->>Agent: "Started exploration skill..."
Agent->>Agent: agent_idle.publish(True)
Agent-->>User: ThinkingIndicator.hide()
loop Exploration loop (background thread)
Explorer->>Explorer: detect frontiers
Explorer->>Nav: goal_request.publish(goal)
Nav-->>Explorer: goal_reached event
end
User->>Agent: "stop"
Agent->>Agent: agent_idle.publish(False)
Agent->>Explorer: end_exploration()
Explorer->>Explorer: stop_exploration() → join thread
Explorer-->>Agent: "Stopped exploration."
Agent->>Agent: agent_idle.publish(True)
Last reviewed commit: b164a42 |
dimos/navigation/frontier_exploration/wavefront_frontier_goal_selector.py
Show resolved
Hide resolved
Additional Comments (1)
This Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! |
30b7a85 to
b164a42
Compare
|
@greptile |
dimos/navigation/frontier_exploration/wavefront_frontier_goal_selector.py
Show resolved
Hide resolved
| def _process_message( | ||
| self, state_graph: CompiledStateGraph[Any, Any, Any, Any], message: BaseMessage | ||
| ) -> None: | ||
| self.agent_idle.publish(False) |
There was a problem hiding this comment.
Duplicate show calls on rapid messages
agent_idle.publish(False) is called at the start of every _process_message, but agent_idle.publish(True) at line 147 is only called when the queue is empty. If multiple messages arrive in quick succession, this will call publish(False) multiple times without an intervening publish(True), triggering multiple ThinkingIndicator.show() calls. In show(), each call creates a new timer via set_interval without stopping the previous one (since hide() hasn't been called), leading to multiple concurrent throbbing timers and duplicate "thinking..." lines in the UI.
| def show(self) -> None: | ||
| self.visible = True | ||
| self._throb_dim = False | ||
| self._write_line() | ||
| self._timer = self._app.set_interval(0.6, self._toggle_throb) |
There was a problem hiding this comment.
show() leaks timers when called repeatedly
show() doesn't guard against being called while already visible. Each call overwrites self._timer with a new set_interval without stopping the previous one, leaking the old timer. This happens because agent_idle.publish(False) fires at the start of every _process_message — consecutive messages will trigger multiple show() calls.
| def show(self) -> None: | |
| self.visible = True | |
| self._throb_dim = False | |
| self._write_line() | |
| self._timer = self._app.set_interval(0.6, self._toggle_throb) | |
| def show(self) -> None: | |
| if self.visible: | |
| return | |
| self.visible = True | |
| self._throb_dim = False | |
| self._write_line() | |
| self._timer = self._app.set_interval(0.6, self._toggle_throb) |
b164a42 to
b2863bf
Compare
Problem
start_explorationblocks the loop.Closes DIM-531
https://linear.app/dimensional/issue/DIM-531/make-start-exploration-not-block
Solution
thinking...tohumancliso it's visible when the agent blocks.NavigationSkillContainerand intoWavefrontFrontierExplorerwhere they belong.Breaking Changes
None
How to Test
uv run dimos run unitree-go2-agentic uv run humancli # say "start exploring" and "stop"