Remove while True in AgentController
#5868
Conversation
| # no need to check too often | ||
| await asyncio.sleep(1) | ||
| else: | ||
| if self.delegate.get_agent_state() != AgentState.PAUSED: | ||
| await self._delegate_step() |
There was a problem hiding this comment.
Please feel free to ignore, since this is a draft, just FYI when you get to test delegates: this whole bit with delegates became pointless. Because currently, the parent is unsubbed when the delegate starts, and the delegate subbed, and at the end of the delegate, the parent subscribes again. 🤔
There was a problem hiding this comment.
ugh this needs some love 😄
Are we still using delegation for the browser?
There was a problem hiding this comment.
No, we have some code, but it's not in use. That tool is not defined. The two actions that CodeAct has, for browsing (interactive and text-only), are good. The second is crazy good. 😅
FWIW, using CodeAct as delegate (of another "planner" CodeAct) is a thing that has been tried, more than once, and in that example it has worked decently. IMO that PR was practically mergeable at some point. If it will be tried again, I think it would be nice to be able to support it. Hey I'm just adding a data point, in case you contemplate killing it! 😇
The old agents still work, including delegator. (Pretty amazing, considering they, erm, aren't really tested for a while afaik.🙊)
There was a problem hiding this comment.
Yeah I had high hopes for delegation when we put it here! I still think it could be great, but I don't love the handshake we currently have
| elif isinstance(event, Observation): | ||
| await self._handle_observation(event) | ||
| self.step() |
There was a problem hiding this comment.
Does this include obs like AgentChangeState or something which isn't supposed to interest the agent? It seems like it does
| # No event loop running... | ||
| asyncio.run(self._async_add_event(event, source)) | ||
|
|
||
| async def _async_add_event(self, event: Event, source: EventSource): |
There was a problem hiding this comment.
If I understand correctly, we're back to a sync add_event? I do like it! I'm not sure why it had switched again 😅
| except Exception as e: | ||
| traceback.print_exc() | ||
| self.log('error', f'Error while running the agent: {e}') | ||
| await self._react_to_exception(e) |
There was a problem hiding this comment.
Just a thought about what I don't see at a cursory look (on github interface):
- do we still send status message to the UI on exception?
End-user friendly description of the problem this fixes or functionality that this introduces
no changelog
Give a summary of what the PR does, explaining any non-trivial design decisions
The
while Truehere is causing a lot of problems. It's easy for it to get stuck permanently.Removing that loop is a surprisingly simple change--we just
_stepthe agent whenever a new event comes in that it might react to. Haven't fully tested this yet but at first blush it's working well.However, doing that surfaced a bunch of subtle issues with EventStream callbacks blocking each other, as well as main parts of the app. So now:
Link of any specific issues this addresses
To run this PR locally, use the following command: