Exp: More standard agent strucutre - #32
Conversation
…ng and display formatting
…add task cancellation for primitive completion
…ng converted tasks with different skill names
Greptile SummaryThis PR standardizes user-facing “primitive” terminology to “skill,” enriches selected skills with registered guidance, introduces compact model-oriented history, adds skill-feedback images, and prioritizes completion events by cancelling current Brain processing.
Confidence Score: 3/5The PR is not safe to merge until completion handling stops dropping unrelated queued work and parallel chat tasks preserve latest-message ordering. Completion events can cancel and permanently discard any queued operation, while out-of-order chat tasks can erase or resurrect user commands; feedback images also bypass model-context limits and persistence redaction. Files Needing Attention: src/brain.py, src/history/history.py, src/brain_utils/primitive_handler.py Important Files Changed
Sequence DiagramsequenceDiagram
participant C as Robot client
participant Q as Brain queue
participant P as Current processing task
participant B as Brain state
participant V as Vision agent
C->>Q: Queued reset/register/image event
Q->>P: process_message(event)
C->>B: PRIMITIVE_COMPLETED
B--xP: cancel current task
Note over P,Q: Cancelled event is not requeued
B->>B: Process completion
B->>V: Reprocess last image
par Concurrent chat A
C->>B: CHAT_IN A
B->>B: Async processing
and Concurrent chat B
C->>B: CHAT_IN B
B->>B: Store latest_user_message
end
B->>B: A finishes later and clears shared message
Reviews (1): Last reviewed commit: "Add name matching to primitive replaceme..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if self._current_process_task and not self._current_process_task.done(): | ||
| self.logger.info( | ||
| "[Brain] PRIMITIVE_COMPLETED received - cancelling current processing to handle immediately" | ||
| ) | ||
| self._pending_primitive_completed = message | ||
| self._current_process_task.cancel() |
There was a problem hiding this comment.
When PRIMITIVE_COMPLETED arrives, this code cancels the current processing task regardless of which message it is handling. Because every queued event uses that task and cancelled events are not requeued, a completion arriving during reset, registration, pose processing, or another lifecycle event can permanently drop the operation after partial state changes. For example, registration can update the directive and skills without sending its acknowledgement, leaving the client and Brain with inconsistent state.
Knowledge Base Used:
| @@ -499,6 +576,12 @@ async def handle_chat_in(self, message: MessageIn): | |||
| await self._send_vision_output(vision_output, vision_output_for_history) | |||
| self.history.check_and_summarize() | |||
| else: | |||
| text = message.payload.get("text", "") | |||
| if text and not text.startswith("!") and not result.fast_response_sent: | |||
| # Keep the latest user message for the next image-based slow-agent call. | |||
| self.state.latest_user_message = text | |||
| elif result.fast_response_sent: | |||
| self.state.latest_user_message = None | |||
There was a problem hiding this comment.
Chat requests run as independent asynchronous tasks, but each task clears or replaces the same latest_user_message value when it finishes. If a newer deferred command stores its text before an older request completes, the older request can erase that command. An older deferred request can also finish later and restore stale text. The next image can therefore omit the latest command or process an already superseded one.
Knowledge Base Used:
| feedback_image_indices = set( | ||
| i | ||
| for i, entry in enumerate(relevant_entries) | ||
| if entry.type == HistoryEntryType.PRIMITIVE_FEEDBACK_IMAGE | ||
| ) | ||
|
|
||
| for d_entry in deduplicated_intermediate_entries: | ||
| source_idx = d_entry["source_index"] | ||
| original_raw_type = d_entry["original_raw_type"] | ||
| original_raw_description = d_entry["original_raw_description"] | ||
|
|
||
| is_selected_for_multimodal_image_role = ( | ||
| ( | ||
| original_raw_type == HistoryEntryType.GENERIC_IMAGE | ||
| or original_raw_type == HistoryEntryType.IMAGE_PRE_ACTION | ||
| ) | ||
| and source_idx in selected_image_source_indices | ||
| ) or ( | ||
| original_raw_type == HistoryEntryType.PRIMITIVE_FEEDBACK_IMAGE | ||
| and source_idx in feedback_image_indices | ||
| ) |
There was a problem hiding this comment.
This selection explicitly includes every feedback image in the 500-entry history window, while generic and pre-action images use configured recent-image limits. Since skills may send repeated feedback and ingestion has no count or size limit, later Gemini requests can contain hundreds of image parts. This can displace useful history, increase cost and latency, or cause provider request failures. Feedback images should use an appropriate bounded selection policy too.
Knowledge Base Used: Conversation history and summarization
| if image_b64: | ||
| self.logger.info(f"Received feedback image from '{task_name}'") | ||
| self.history.add( | ||
| HistoryEntryType.PRIMITIVE_FEEDBACK_IMAGE, | ||
| description=image_b64, | ||
| ) |
There was a problem hiding this comment.
This stores the complete base64 feedback image as the history description, but History.save() only redacts generic and pre-action image entries. The next history save therefore writes feedback image bytes directly into the JSON export, unlike existing image types. This can persist sensitive image content and substantially enlarge history files. The new feedback-image type should be covered by the export redaction logic.
Knowledge Base Used: Conversation history and summarization
1 participant
No description provided.