fix(vercel-ai): auto-detect deferred tool approval state in dump_messages()#4831
fix(vercel-ai): auto-detect deferred tool approval state in dump_messages()#4831tijmenhammer wants to merge 7 commits intopydantic:mainfrom
Conversation
…ext in dump_messages()
1. Add `deferred_tool_call_ids` parameter to `dump_messages()` so callers
can specify which tool calls are deferred. These are emitted with
`state='approval-requested'` and `approval={id: tool_call_id}` instead
of `state='input-available'` with `approval=null`, enabling the frontend
to render approve/reject buttons on reload.
2. Use raw `RetryPromptPart.content` (when it's a string) instead of
`model_response()` for the UI error_text. `model_response()` appends
"Fix the errors and try again." which is intended for the model, not
for UI display, and mangles custom error markers like "Cancelled".
Fixes pydantic#4830
github-actions
bot
added
size: M
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…t error text The streaming handler in _event_stream.py used part.model_response() which appends "Fix the errors and try again." to string content. Now uses raw content for strings (matching _adapter.py dump path), so the same error shows identical text whether streamed in real-time or reconstructed from persisted messages.
| input=part.args_as_dict(), | ||
| provider_executed=True, | ||
| call_provider_metadata=call_provider_metadata, | ||
| approval=ToolApprovalRequested(id=part.tool_call_id), |
There was a problem hiding this comment.
📝 Info: approval_id and tool_call_id are intentionally identical
In _event_stream.py:130-131, both approval_id and tool_call_id are set to tool_call.tool_call_id. Similarly in the dump path, ToolApprovalRequested(id=part.tool_call_id) reuses the tool call ID. I verified that iter_tool_approval_responses in _utils.py:147-151 uses part.tool_call_id (not part.approval.id) for matching, confirming that approval.id is not used as a matching key. This makes the output fully deterministic, which is desirable for snapshot testing and idempotent renders. The only potential concern would be if the Vercel AI SDK frontend uses approval_id for deduplication across multiple approval requests for the same tool call, but the comments explicitly note this is not the case.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
The inconsistency is harmless approval.id is never used for matching anywhere. All approval pairing goes through tool_call_id (see iter_tool_approval_responses in _utils.py). Using tool_call_id in the dump path is intentional for deterministic output. Added a clarifying comment.
There was a problem hiding this comment.
@tijmenhammer If we use tool_call_id in the dump case, let's do it in the streaming case as well.
|
@tijmenhammer Bedankt Tijmen :) Can you please look at that Devin comment? I'm not sure if the approval ID actually has to match the tool call ID, or if any UUID is fine. |
|
Hey @DouweM geen probleem ;) Looked into it approval ID can be anything, it's not used for matching. All pairing goes through toolCallId on the message part (iter_tool_approval_responses in _utils.py). Went with tool_call_id in the dump path for deterministic output (better for snapshots), the streaming path uses uuid4() which is fine there since it's ephemeral. Added a comment explaining why. |
| input=part.args_as_dict(), | ||
| provider_executed=True, | ||
| call_provider_metadata=call_provider_metadata, | ||
| approval=ToolApprovalRequested(id=part.tool_call_id), |
There was a problem hiding this comment.
@tijmenhammer If we use tool_call_id in the dump case, let's do it in the streaming case as well.
| elif isinstance(tool_result, RetryPromptPart): | ||
| # Use the raw content string to avoid model_response() appending | ||
| # "Fix the errors and try again." — that suffix is intended for the | ||
| # model, not for UI display. For structured validation errors (list |
There was a problem hiding this comment.
This means that these RetryPromptParts are now lossy in a round-trip right? Because on the next turn (after dump and load), the LLM would not see the "Fix the errors..." bit anymore, breaking the cache, and potentially being less clear to it than the message with the prompt.
Either way I think this is a more controversial change than the one about approval state so would like to see this in a separate PR, if you insist we need it :)
There was a problem hiding this comment.
Fair point about the lossy round-trip. Reverted will open a separate PR if I find the time.
…_id in streaming, revert RetryPromptPart change - Remove deferred_tool_call_ids param; dump_messages() now auto-detects deferred tool calls (no result in history) and emits approval-requested - Use tool_call_id for approval_id in streaming path (consistent with dump path) - Revert RetryPromptPart raw content change (separate PR per reviewer request)
github-actions
bot
added
size: S
tijmenhammer
changed the title
3 participants
Pre-Review Checklist
make formatandmake typecheck.Pre-Merge Checklist
Summary
Fixes #4830
dump_messages()now automatically detects deferred tool calls by checking whichToolCallParts have no corresponding result in the message history. These are emitted withstate='approval-requested'instead ofstate='input-available', enabling the frontend to render approve/reject buttons on page reload.No new API surface — deferred status is inferred from the messages, not passed in by the caller.
Also uses
tool_call_idasapproval_idin the streaming path for consistency with the dump path (wasuuid4()before).Changes
_adapter.py: Tool calls without results automatically emitapproval-requestedwithapproval={id: tool_call_id}(bothToolCallPartandBuiltinToolCallPart)_event_stream.py: Usetool_call_idforapproval_id(consistent with dump path)test_vercel_ai.py: Updated snapshot tests, added deferred tool coverageTest plan