Update ToolProvider to handle cancellation #15951
Conversation
d3337e7 to
46f40f3
Compare
There was a problem hiding this comment.
Thanks, @eneufeld. I mostly tested this with the agent delegation feature, and it looks good to me. I would just suggest removing a few superfluous comments (see inline).
Besides, I noticed quite a few changes that are essentially only code formatting. I know the code-formatting configuration is sometimes off (I also opened #15959 today), but usually for TypeScript files the discrepancy to the existing code base isn't that large (at least in my case?).
This PR contains many pure formatting changes; if you don't mind, could you revert those if you find the time? TIA
c085f03 to
cc15322
Compare
|
@ndoschek I updated the pr |
There was a problem hiding this comment.
Hi @eneufeld, thank you for the update!
I had another look, and I noticed, the check for cancellation could be simplified by using the .isCanceled getter from the response model, wdyt?
Also, if we cancel a run, the AgentCompletionNotificationService notifies us that the task was completed, which is not entirely correct IMO.
(I am using the ai-features.notifications.default: `message' setting).
We could show the completion notification only if it was not canceled by the user for example. But we can also extract a follow up for that one, that's fine by me.
PS: unfortunately I cannot resolve my own comments right now, but the previous comments are resolved from my POV.
this is fixed here: |
cc15322 to
9aa97fd
Compare
|
@sdirix Ping |
- Updated ToolProviders to cancel execution - Added tests
9aa97fd to
937f64f
Compare
|
@sdirix I updated this |
- Updated ToolProviders to cancel execution - Added tests --------- Co-authored-by: Stefan Dirix <sdirix@eclipsesource.com>
What it does
How to test
Let universal call a long running tool, eg delegate to agent.
Press the cancel button in the chat view. see that delegation stops.
Previously this continued.
Follow-ups
Breaking changes
Attribution
Review checklist
Reminder for reviewers