Support listing and deleting recorded agent actions - #6571
Conversation
| return [ | ||
| {**copy.deepcopy(action), "action_id": action_id} | ||
| for action_id, action in self._actions.items() | ||
| ] |
There was a problem hiding this comment.
I would return MD table, with ID, Action (based on the tool name), and reasoning.
|
|
||
| self._actions.append(action) | ||
| self._actions[action_id] = action | ||
| self._last_action_id = action_id |
There was a problem hiding this comment.
Using _last_action_id is a bit fragile, race conditions could miss up with that.
| ``attachments`` key yet. | ||
| """ | ||
| idx = len(self._actions) | ||
| sequence = self._next_action_sequence |
There was a problem hiding this comment.
Also, sequential IDs would be easy for the agent to guess and might remove things by accident. We could use UUIDs, which would be harder to guess.
|
|
||
| def _confirm(recorder: ActionsRecorder, action_type: str) -> str: | ||
| return f"Recorded {action_type} (#{len(recorder.actions) - 1})." | ||
| return f"Recorded {action_type} as {recorder.last_action_id}." |
There was a problem hiding this comment.
| return f"Recorded {action_type} as {recorder.last_action_id}." | |
| return f"Recorded {action_type} (ID: {recorder.last_action_id})." |
|
|
||
| def _confirm(recorder: ActionsRecorder, action_type: str) -> str: | ||
| return f"Recorded {action_type} (#{len(recorder.actions) - 1})." | ||
| return f"Recorded {action_type} as {recorder.last_action_id}." |
There was a problem hiding this comment.
| "Exact stable ID returned when the action was recorded or by " | ||
| "list_actions (for example, action-2)." |
There was a problem hiding this comment.
I would simplify it. Also, we do not need an example; it makes it a bit confusing since it does not look like the action IDs that will be in real cases.
| """Retract one proposed action from this agent run. | ||
|
|
||
| The removed action will not appear in the final run summary and cannot be | ||
| applied. This operation accepts exactly one action ID and has no cascade or | ||
| force mode. |
| """List every action currently proposed by this agent run. | ||
|
|
||
| Returns each action's stable in-run ID and its complete recorded payload, | ||
| including parameters, reasoning, references, and attachment metadata. Use | ||
| this when earlier action details are no longer present in your context or | ||
| before deciding whether a proposal needs to be retracted. | ||
| """ |
suhaibmujahid
left a comment
There was a problem hiding this comment.
I think you are still working on this, but here are some comments anyway.
| ] | ||
|
|
||
| def remove_action(self, action_id: str) -> dict: | ||
| """Remove and return an action.""" |
There was a problem hiding this comment.
The docstrings are not clear here.
| """Remove and return an action.""" | |
| """Remove the action with the given ID and return a copy of it.""" |
| Field(description="ID of the action to remove."), | ||
| ], | ||
| ) -> dict: | ||
| """Remove and return a recorded action.""" |
There was a problem hiding this comment.
The docstrings here targets the agent, so it should provide what should help the agent only. In this case, no need to mention the return part, and the returned value should be something that confirms the what happened instead of the removed action which could be a bit confusing.
| action = recorder.record(ACTION_TYPE, _params(channel, text), reasoning=reasoning) | ||
| return f"Recorded {ACTION_TYPE} (ID: {action['action_id']})." |
There was a problem hiding this comment.
We should file a follow-up issue to centralize the confirmation message.
There was a problem hiding this comment.
What if we call it actions_manager or something related instead of recorded_actions? WDYT?
| the apply step will need. ``attachments`` maps a logical name to a | ||
| local file path; each file is preserved under the stable key | ||
| ``attachments/<action_index>/<name>``: uploaded via the runtime | ||
| ``attachments/<action_sequence>/<name>``: uploaded via the runtime |
There was a problem hiding this comment.
It should be action id, we do not want to use sequence.
Summary
recorded_actionstools for the bug-fix and frontend-triage agents.Fixes #6513