-
Notifications
You must be signed in to change notification settings - Fork 2.9k
feat(ai): extend addToolResult to support error results #8560
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
packages/ai/src/ui/last-assistant-message-is-complete-with-tool-calls.test.ts
Outdated
Show resolved
Hide resolved
24eafe2
to
b1265dc
Compare
b1265dc
to
3f9f067
Compare
Hi Aron 👋🏼 thanks for the pull request! It's a great, especially given that it's your first 👏🏼 @lgrammel and I looked at it today. The CI is failing and we had some thoughts on how to improve the code, we thought the easiest would be to just push the changes we suggest: e218283 let us know what you think.
We think the best solution would be to keep it at one method, but to rename it from As a side note, we are looking deeper into tool calling right now, in particular in regard of human-in-the-loop. Stay tuned! |
@nicoalbanese can you please review the docs updates? |
3f9f067
to
9396fdf
Compare
Changes look great, the argument types are much clearer now. I've integrated them into this branch and force pushed.
Fantastic, looking forward to it! |
Yes | ||
</button> | ||
<button | ||
onClick={() => | ||
addToolResult({ | ||
tool: 'askForConfirmation', | ||
toolCallId: callId, | ||
output: 'No, denied', | ||
}) | ||
} | ||
> | ||
No | ||
</button> | ||
</div> | ||
</div> | ||
); | ||
case 'output-available': | ||
return ( | ||
<div key={callId}> | ||
Location access allowed: {part.output} | ||
</div> | ||
); | ||
case 'output-error': | ||
return <div key={callId}>Error: {part.errorText}</div>; | ||
} | ||
break; | ||
} | ||
|
||
case 'tool-getLocation': { | ||
const callId = part.toolCallId; | ||
|
||
switch (part.state) { | ||
case 'input-streaming': | ||
return ( | ||
<div key={callId}>Preparing location request...</div> | ||
); | ||
case 'input-available': | ||
return <div key={callId}>Getting location...</div>; | ||
case 'output-available': | ||
return <div key={callId}>Location: {part.output}</div>; | ||
case 'output-error': | ||
return ( | ||
<div key={callId}> | ||
Error getting location: {part.errorText} | ||
</div> | ||
); | ||
} | ||
break; | ||
} | ||
|
||
case 'tool-getWeatherInformation': { | ||
const callId = part.toolCallId; | ||
|
||
switch (part.state) { | ||
// example of pre-rendering streaming tool inputs: | ||
case 'input-streaming': | ||
return ( | ||
<pre key={callId}>{JSON.stringify(part, null, 2)}</pre> | ||
); | ||
case 'input-available': | ||
return ( | ||
<div key={callId}> | ||
Getting weather information for {part.input.city}... | ||
</div> | ||
); | ||
case 'output-available': | ||
return ( | ||
<div key={callId}> | ||
Weather in {part.input.city}: {part.output} | ||
</div> | ||
); | ||
case 'output-error': | ||
return ( | ||
<div key={callId}> | ||
Error getting weather for {part.input.city}:{' '} | ||
{part.errorText} | ||
</div> | ||
); | ||
} | ||
break; | ||
} | ||
} | ||
})} | ||
<br /> | ||
</div> | ||
))} | ||
|
||
<form | ||
onSubmit={e => { | ||
e.preventDefault(); | ||
if (input.trim()) { | ||
sendMessage({ text: input }); | ||
setInput(''); | ||
} | ||
}} | ||
> | ||
<input value={input} onChange={e => setInput(e.target.value)} /> | ||
</form> | ||
</> | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we cut all the markup of the component? the important thing to highlight here is the onToolCall and addToolResult state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done 👍 I wasn't sure if the examples were intended to be fully-working examples.
@gr2m happy with this once comments are resolved! |
This commit updates the `lastAssistantMessageIsCompleteWithToolCalls` helper function to also consider tool parts with a state of `output-error` a completed tool call. Co-Authored-By: Lars Grammel <[email protected]>
This commit extends the `addToolResult` method on the `AbstractChat` class to optionally take an `output-error` result. This allows clients to report both successful and failed tool calls using the same mechanism. addToolResult({ state: "output-error", errorText: "Failed" }); For backwards compatibility the existing method interface is still supported and will default to a state of `output-available`. Co-Authored-By: Lars Grammel <[email protected]>
Co-authored-by: Nico Albanese <[email protected]>
2cf60db
to
065731e
Compare
Thanks both, I've made the requested changes and rebased. |
Background
Currently the
addToolResult()
helper allows a user to update a pending tool call evaluated on the client. However, there is not currently a mechanism for being able to report a pending tool call as errored with a state ofoutput-error
.We would like to be able to use this to report client side errors with failed tool calls as well as cancellations.
There doesn't appear to be a way to replicate the behavior of the
addToolResult
implementation using just the public API provided byuseChat
.So as a workaround we currently have to use
addToolResult
and add an additional typed field on our tool output withstatus: "success" | "error"
and consider failed tool calls "output" then adapt the rendering in our application. This differs to how we handle server side errors with tool calls which will use theoutput-error
state.Then in our UI code when handling the tool part:
Summary
This commit extends the
addToolResult
method on theAbstractChat
class to optionally take anoutput-error
result. This allows clients to report both successful and failed tool calls using the same mechanism.For backwards compatibility the existing method interface is still supported and will default to a state of
output-available
.So my earlier example becomes:
Integration style tests have been added to the existing chat.test.ts suite to exercise the new behavior in the same fashion as the current implementation.
It also updates the
lastAssistantMessageIsCompleteWithToolCalls
helper function to also consider tool parts with a state ofoutput-error
a completed tool call. This ensures consistent behavior for both types of result.Notes on implementation
I've extended
addToolResult
to keep the API footprint minimal but I would appreciate feedback on whether an additional method e.g.addToolError({ tool, toolCallId, errorText })
would be a more appropriate solution. I'm happy to change the code if needed.Manual Verification
I couldn't figure out how to correctly
npm link
the@ai-sdk/react
andai
packages into our local project so we're currently running the it with a monkey patched version ofaddToolResult
andlastAssistantMessageIsCompleteWithToolCalls
copy & pasted into our codebase.Tasks
pnpm changeset
in the project root)pnpm prettier-fix
in the project root)Future Work
None
Related Issues
None