-
-
Notifications
You must be signed in to change notification settings - Fork 355
fix: complete tool calls with server results #596
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
Changes from 1 commit
a73bdc2
4b36015
82214d4
e6b6381
18790cf
5e0e761
ca4db20
4493366
2aa7244
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| --- | ||
| '@tanstack/ai': patch | ||
| '@tanstack/ai-client': patch | ||
| '@tanstack/ai-event-client': patch | ||
| --- | ||
|
|
||
| Populate server-executed tool results on the matching `tool-call` part and mark successful tool calls as `complete`. | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -6,6 +6,7 @@ import { | |||||||||||||||||||||||||||||||||||
| getMetadata, | ||||||||||||||||||||||||||||||||||||
| getEventLog, | ||||||||||||||||||||||||||||||||||||
| getToolCalls, | ||||||||||||||||||||||||||||||||||||
| getMessages, | ||||||||||||||||||||||||||||||||||||
| } from './helpers' | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||
|
|
@@ -58,6 +59,73 @@ test.describe('Server-Client Sequence E2E Tests', () => { | |||||||||||||||||||||||||||||||||||
| expect(chartExecution).toBeTruthy() | ||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| test('server and client tool results populate tool-call output and complete state', async ({ | ||||||||||||||||||||||||||||||||||||
| page, | ||||||||||||||||||||||||||||||||||||
| testId, | ||||||||||||||||||||||||||||||||||||
| aimockPort, | ||||||||||||||||||||||||||||||||||||
| }) => { | ||||||||||||||||||||||||||||||||||||
| await selectScenario(page, 'sequence-server-client', testId, aimockPort) | ||||||||||||||||||||||||||||||||||||
| await runTest(page) | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| await waitForTestComplete(page, 15000, 2) | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| await page.waitForFunction( | ||||||||||||||||||||||||||||||||||||
| () => { | ||||||||||||||||||||||||||||||||||||
| const messagesEl = document.getElementById('messages-json-content') | ||||||||||||||||||||||||||||||||||||
| const messages = JSON.parse(messagesEl?.textContent || '[]') | ||||||||||||||||||||||||||||||||||||
| const toolCalls = messages.flatMap((msg: any) => | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+72
to
+76
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Guard JSON parsing inside
Suggested fix await page.waitForFunction(
() => {
const messagesEl = document.getElementById('messages-json-content')
- const messages = JSON.parse(messagesEl?.textContent || '[]')
+ let messages: Array<any> = []
+ try {
+ messages = JSON.parse(messagesEl?.textContent || '[]')
+ } catch {
+ return false
+ }
const toolCalls = messages.flatMap((msg: any) =>
(msg.parts || []).filter((part: any) => part.type === 'tool-call'),
)📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||
| (msg.parts || []).filter((part: any) => part.type === 'tool-call'), | ||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| const fetchData = toolCalls.find( | ||||||||||||||||||||||||||||||||||||
| (part: any) => part.name === 'fetch_data', | ||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||
| const displayChart = toolCalls.find( | ||||||||||||||||||||||||||||||||||||
| (part: any) => part.name === 'display_chart', | ||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| return ( | ||||||||||||||||||||||||||||||||||||
| fetchData?.state === 'complete' && | ||||||||||||||||||||||||||||||||||||
| fetchData.output !== undefined && | ||||||||||||||||||||||||||||||||||||
| displayChart?.state === 'complete' && | ||||||||||||||||||||||||||||||||||||
| displayChart.output !== undefined | ||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||
| { timeout: 10000 }, | ||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| const messages = await getMessages(page) | ||||||||||||||||||||||||||||||||||||
| const toolCalls = messages.flatMap((msg) => | ||||||||||||||||||||||||||||||||||||
| (msg.parts || []).filter((part: any) => part.type === 'tool-call'), | ||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||
| const toolResults = messages.flatMap((msg) => | ||||||||||||||||||||||||||||||||||||
| (msg.parts || []).filter((part: any) => part.type === 'tool-result'), | ||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| const fetchData = toolCalls.find((part) => part.name === 'fetch_data') | ||||||||||||||||||||||||||||||||||||
| expect(fetchData).toMatchObject({ | ||||||||||||||||||||||||||||||||||||
| state: 'complete', | ||||||||||||||||||||||||||||||||||||
| output: { | ||||||||||||||||||||||||||||||||||||
| source: 'api', | ||||||||||||||||||||||||||||||||||||
| data: [1, 2, 3, 4, 5], | ||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| const displayChart = toolCalls.find((part) => part.name === 'display_chart') | ||||||||||||||||||||||||||||||||||||
| expect(displayChart?.state).toBe('complete') | ||||||||||||||||||||||||||||||||||||
| expect(displayChart?.output).toMatchObject({ rendered: true }) | ||||||||||||||||||||||||||||||||||||
| expect(typeof displayChart?.output?.chartId).toBe('string') | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| expect( | ||||||||||||||||||||||||||||||||||||
| toolResults.some( | ||||||||||||||||||||||||||||||||||||
| (part) => | ||||||||||||||||||||||||||||||||||||
| part.toolCallId === fetchData?.id && | ||||||||||||||||||||||||||||||||||||
| part.state === 'complete' && | ||||||||||||||||||||||||||||||||||||
| JSON.parse(part.content).source === 'api', | ||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||
| ).toBe(true) | ||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| test('server then two client tools in sequence', async ({ | ||||||||||||||||||||||||||||||||||||
| page, | ||||||||||||||||||||||||||||||||||||
| testId, | ||||||||||||||||||||||||||||||||||||
|
|
@@ -162,10 +230,7 @@ test.describe('Server-Client Sequence E2E Tests', () => { | |||||||||||||||||||||||||||||||||||
| const toolCalls = await getToolCalls(page) | ||||||||||||||||||||||||||||||||||||
| const weatherTool = toolCalls.find((tc) => tc.name === 'get_weather') | ||||||||||||||||||||||||||||||||||||
| expect(weatherTool).toBeTruthy() | ||||||||||||||||||||||||||||||||||||
| // Server tools stay at 'input-complete' state but are tracked as complete via tool-result parts | ||||||||||||||||||||||||||||||||||||
| expect(['complete', 'input-complete', 'output-available']).toContain( | ||||||||||||||||||||||||||||||||||||
| weatherTool?.state, | ||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||
| expect(weatherTool?.state).toBe('complete') | ||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| test('text only scenario has no tool calls', async ({ | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
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.
Use
minorbumps for this exported type-shape change.Adding
'complete'toToolCallStateis a public shape change and should be released asminorfor this repo’s pre-1.0 convention.Suggested fix
📝 Committable suggestion
🤖 Prompt for AI Agents