chore(ai): add optional ChatRequestOptions to addToolApprovalResponse and addToolOutput#11048
Conversation
| /** | ||
| * Optional request options to be used when sending the message | ||
| */ | ||
| options?: ChatRequestOptions; |
There was a problem hiding this comment.
I'm not sure about this approach because it makes it seem that options is relevant to the tool approval when in reality it's only needed to automatically re-submit if that's enabled. Curious what you think @lgrammel
My initial idea was what I shared in #11423 (comment) - overloading in that way may not be the most intuitive either, but it would be closer to the root of the problem and directly associated with re-submitting.
There was a problem hiding this comment.
My take is that it boils down to the concrete use cases, and what data is available when/where. I.e. are the ChatRequestOption values available only when sendApprovalResponse is invoked? then this is a good place (some of the concerns can be addressed by expanding the jsdoc). If not, then another place might be better.
There was a problem hiding this comment.
Makes sense. They're available when invoking addToolApprovalResponse, so I think it's a good place, but the same problem also applies when calling addToolOutput. So I think we'll need to add it to both methods.
felixarntz
left a comment
There was a problem hiding this comment.
@patrikdevlin upon further review, the approach looks good, but can you please expand it to also cover addToolOutput?
If you can, adding test cases for the two changes would be great as well.
packages/ai/src/ui/chat.ts
Outdated
| reason?: string; | ||
|
|
||
| /** | ||
| * Optional request options to be used when sending the message |
There was a problem hiding this comment.
worth clarifying that this is only relevant if a message is actually being sent as part of this.
| * Optional request options to be used when sending the message | |
| * Optional request options to be used if `sendAutomaticallyWhen` callback returns true |
| /** | ||
| * Optional request options to be used when sending the message | ||
| */ | ||
| options?: ChatRequestOptions; |
There was a problem hiding this comment.
Per https://github.com/vercel/ai/pull/11048/changes#r2760036163, can you please add this to addToolOutput as well?
felixarntz
left a comment
There was a problem hiding this comment.
@patrikdevlin I have pushed a few more changes to this branch:
- support
optionsinaddToolOutputtoo - equally relevant there - add unit tests for the changes
- add a UI example where this is testable end-to-end
Should be good to go shortly. Thanks for helping to get this fixed!
addToolApprovalResponse and addToolOutput
…e` and `addToolOutput` (#11048) ## Background <!-- Why was this change necessary? --> Users should be able to customize the api request similar to sendMessage & regenerate Fixes: #11423 ## Summary <!-- What did you change? --> Adding optional ChatRequestOptions parameter to addToolApprovalResponse. Add-on work to #8541. ## Checklist <!-- Do not edit this list. Leave items unchecked that don't apply. If you need to track subtasks, create a new "## Tasks" section Please check if the PR fulfills the following requirements: --> - ~Tests have been added / updated (for bug fixes / features)~ - n/a - ~Documentation has been added / updated (for bug fixes / features)~ - n/a - [x] A _patch_ changeset for relevant packages has been added (for bug fixes / features - run `pnpm changeset` in the project root) - [x] I have reviewed this pull request (self-review) ## Future Work n/a --------- Co-authored-by: Felix Arntz <felix.arntz@vercel.com>
|
✅ Backport PR created: #13107 |
…valResponse` and `addToolOutput` (#13107) This is an automated backport of #11048 to the release-v6.0 branch. FYI @patrikdevlin Co-authored-by: patrik <17474633+patrikdevlin@users.noreply.github.com> Co-authored-by: Felix Arntz <felix.arntz@vercel.com>
|
🚀 Published in:
|
Background
Users should be able to customize the api request similar to sendMessage & regenerate
Fixes: #11423
Summary
Adding optional ChatRequestOptions parameter to addToolApprovalResponse. Add-on work to #8541.
Checklist
Tests have been added / updated (for bug fixes / features)- n/aDocumentation has been added / updated (for bug fixes / features)- n/apnpm changesetin the project root)Future Work
n/a