-
-
Notifications
You must be signed in to change notification settings - Fork 312
fix(ai-client): do not end append on an intermediate tool_calls RUN_FINISHED #1244
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?
Changes from all commits
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,6 @@ | ||
| --- | ||
| '@tanstack/ai': patch | ||
| '@tanstack/ai-client': patch | ||
| --- | ||
|
|
||
| Keep `append()` pending until the HTTP response is fully processed, including later `RUN_FINISHED` events in the same agent loop. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -164,6 +164,16 @@ function resolveTransport(transport: { | |
| throw new Error('ChatClient: either `connection` or `fetcher` is required.') | ||
| } | ||
|
|
||
| function connectionDrainsOnSend(connection: ConnectionAdapter): boolean { | ||
| return 'connect' in connection | ||
| } | ||
|
|
||
| function isIntermediateToolTurn(chunk: StreamChunk): boolean { | ||
| if (chunk.type !== 'RUN_FINISHED') return false | ||
| if (chunk.outcome?.type === 'interrupt') return false | ||
| return tanstackMetadata(chunk)?.finishReason === 'tool_calls' | ||
|
Comment on lines
+171
to
+174
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. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- changed hunk ---'
git diff -- packages/ai-client/src/chat-client.ts
printf '%s\n' '--- target helper and direct finishReason consumers ---'
rg -n -C 12 'isIntermediateToolTurn|handleRunFinishedEvent|finishReason|updateRunLifecycle' packages/ai-client/src/chat-client.ts packages/ai-client/src
printf '%s\n' '--- relevant connection and stream types/usages ---'
rg -n -C 8 'ConnectConnectionAdapter|ConnectionAdapter|RUN_FINISHED|AdapterYieldChunk' packages/ai-client packages --glob '*.{ts,tsx}'Repository: TanStack/ai Length of output: 50368 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- chat-client.ts relevant range ---'
sed -n '1,260p' packages/ai-client/src/chat-client.ts
printf '%s\n' '--- exact definitions and callers in ai-client ---'
rg -n -C 15 'function isIntermediateToolTurn|handleRunFinishedEvent|class StreamProcessor|updateRunLifecycle|interface ConnectionAdapter|type ConnectionAdapter|ConnectConnectionAdapter' packages/ai-client/src --glob '*.{ts,tsx}'
printf '%s\n' '--- focused diff ---'
git diff --unified=12 -- packages/ai-client/src/chat-client.ts | sed -n '1,240p'Repository: TanStack/ai Length of output: 50369 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- lifecycle implementation ---'
sed -n '1117,1195p' packages/ai-client/src/chat-client.ts
printf '%s\n' '--- stream processing path ---'
sed -n '1780,1880p' packages/ai-client/src/chat-client.ts
printf '%s\n' '--- StreamProcessor implementation and finishReason contract ---'
rg -l 'class StreamProcessor|handleRunFinishedEvent' packages/ai/src packages/ai-client/src | while read -r file; do
echo "### $file"
rg -n -C 18 'class StreamProcessor|handleRunFinishedEvent|finishReason' "$file"
done
printf '%s\n' '--- StreamChunk and AdapterYieldChunk declarations ---'
rg -n -C 12 'export (type|interface) (StreamChunk|AdapterYieldChunk)|finishReason' packages/ai/src/types packages/ai/src/utilities --glob '*.{ts,tsx}'Repository: TanStack/ai Length of output: 49381 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- public stream event types ---'
rg -n -C 14 'export (type|interface) .*StreamChunk|RunFinishedEvent|RUN_FINISHED' packages/ai/src --glob '*.{ts,tsx}' | head -n 220
printf '%s\n' '--- adapter normalization and chat append processing ---'
sed -n '994,1065p' packages/ai-client/src/connection-adapters.ts
rg -n -C 18 'append\(|resolveProcessing\(|processingPromise|processIncomingChunk' packages/ai-client/src/chat-client.ts | head -n 360
printf '%s\n' '--- chat-client tests for connection adapters and interrupts ---'
rg -n -C 10 'ConnectConnectionAdapter|finishReason|tool_calls|append\\(|interrupt' packages/ai-client/tests packages/ai-client/src --glob '*.{test,spec}.{ts,tsx}' | head -n 420Repository: TanStack/ai Length of output: 33395 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- processing wait and request completion ---'
sed -n '1938,2135p' packages/ai-client/src/chat-client.ts
printf '%s\n' '--- direct-field handling in inbound restoration ---'
sed -n '45,75p' packages/ai/src/utilities/restore-inbound-chunk.ts
printf '%s\n' '--- focused tests and adapter fixtures ---'
rg -n -C 12 --fixed-strings 'ConnectConnectionAdapter' packages/ai-client --glob '*.{test,spec}.{ts,tsx}'
rg -n -C 12 --fixed-strings 'tool_calls' packages/ai-client --glob '*.{test,spec}.{ts,tsx}'Repository: TanStack/ai Length of output: 50370 Recognize direct
🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| export interface NormalizedQueueConfig { | ||
| whenBusy: WhenBusy | ||
| drain: 'fifo' | 'batch' | ||
|
|
@@ -416,6 +426,12 @@ export class ChatClient< | |
| private continuationPending = false | ||
| private subscriptionAbortController: AbortController | null = null | ||
| private processingResolve: (() => void) | null = null | ||
| /** | ||
| * `connect()` adapters push the full HTTP body into the subscribe queue, then | ||
| * wait until that queue is idle. After `send()` returns, every chunk from this | ||
| * request has been processed. Subscribe/send sockets do not drain that way. | ||
| */ | ||
| private connectionDrainsOnSend = false | ||
| private errorReportedGeneration: number | null = null | ||
| private streamGeneration = 0 | ||
| private continuationGeneration = 0 | ||
|
|
@@ -518,7 +534,9 @@ export class ChatClient< | |
| this.byokProvider = options.byokProvider | ||
| this.context = options.context | ||
| this.queueConfig = normalizeQueueOption(options.queue) | ||
| this.connection = normalizeConnectionAdapter(resolveTransport(options)) | ||
| const transport = resolveTransport(options) | ||
| this.connectionDrainsOnSend = connectionDrainsOnSend(transport) | ||
| this.connection = normalizeConnectionAdapter(transport) | ||
|
|
||
| // Build client tools map | ||
| this.clientToolsRef = { current: new Map() } | ||
|
|
@@ -1140,7 +1158,9 @@ export class ChatClient< | |
| this.clearedStreamTracker.onSessionRunError() | ||
| } | ||
| this.setSessionGenerating(this.activeRunIds.size > 0) | ||
| if (options?.resolveProcessing !== false) { | ||
| const skipProcessingResolve = | ||
| chunk.type === 'RUN_FINISHED' && isIntermediateToolTurn(chunk) | ||
| if (options?.resolveProcessing !== false && !skipProcessingResolve) { | ||
| this.resolveProcessing() | ||
| } | ||
| } | ||
|
|
@@ -2344,6 +2364,14 @@ export class ChatClient< | |
| return false | ||
| } | ||
|
|
||
| // connect() send() already waited until the subscribe queue was idle. | ||
| // Kick the processing wait so a stream that ends on tool_calls (no | ||
| // interrupt / stop) cannot hang. Subscribe/send sockets still wait for | ||
| // a request-ending terminal below. | ||
| if (this.connectionDrainsOnSend) { | ||
| this.resolveProcessing() | ||
| } | ||
|
|
||
| // Wait for subscription loop to finish processing all chunks | ||
| await processingComplete | ||
|
|
||
|
|
@@ -3045,12 +3073,12 @@ export class ChatClient< | |
| this.resetSessionGenerating() | ||
| this.setIsSubscribed(false) | ||
| this.setConnectionStatus('disconnected') | ||
| this.connection = normalizeConnectionAdapter( | ||
| resolveTransport({ | ||
| connection: options.connection, | ||
| fetcher: options.fetcher, | ||
| }), | ||
| ) | ||
| const transport = resolveTransport({ | ||
| connection: options.connection, | ||
| fetcher: options.fetcher, | ||
| }) | ||
| this.connectionDrainsOnSend = connectionDrainsOnSend(transport) | ||
| this.connection = normalizeConnectionAdapter(transport) | ||
|
|
||
| if (wasSubscribed) { | ||
| this.subscribe() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1055,6 +1055,27 @@ export function normalizeConnectionAdapter( | |
| } | ||
| } | ||
|
|
||
| async function waitUntilSubscriberIdle( | ||
| abortSignal?: AbortSignal, | ||
| ): Promise<void> { | ||
| const idle = () => | ||
| activeBuffer.length === 0 && | ||
| (activeWaiters.length > 0 || abortSignal?.aborted) | ||
| for (let i = 0; i < 16 && !abortSignal?.aborted; i++) { | ||
| if (idle()) return | ||
| if (activeBuffer.length === 0 && activeWaiters.length === 0) return | ||
| await Promise.resolve() | ||
| } | ||
|
Comment on lines
+1061
to
+1068
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. 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift 🔎 Supported by static analysis🏁 Script executed: sed -n '1010,1090p' packages/ai-client/src/connection-adapters.ts
printf '\n--- interrupt manager ---\n'
sed -n '610,670p' packages/ai-client/src/interrupt-manager.ts
printf '\n--- bound symbols and nearby subscription code ---\n'
rg -n -C 4 'waitUntilSubscriberIdle|activeBuffer|activeWaiters|RUN_FINISHED|interrupt-manager|subscribe|push\(' packages/ai-client/src/connection-adapters.ts packages/ai-client/src/interrupt-manager.tsRepository: TanStack/ai Length of output: 33412 🏁 Script executed: sed -n '1088,1195p' packages/ai-client/src/connection-adapters.ts
printf '\n--- send implementations and call sites ---\n'
rg -n -C 5 'async send|send\(|waitUntilSubscriberIdle|for await|append\(' packages/ai-client/src --glob '*.ts' --glob '*.tsx'Repository: TanStack/ai Length of output: 32272 🏁 Script executed: sed -n '1635,1720p' packages/ai-client/src/chat-client.ts
sed -n '2175,2395p' packages/ai-client/src/chat-client.tsRepository: TanStack/ai Length of output: 12252 🏁 Script executed: rg -n -C 12 'processIncomingChunk|onStreamEnd|interrupt|RUN_FINISHED|RUN_ERROR' packages/ai-client/src/chat-client.ts | head -n 240Repository: TanStack/ai Length of output: 9964 🏁 Script executed: rg -n 'processIncomingChunk|onStreamEnd|outcome.*interrupt|interruptManager' packages/ai-client/src/chat-client.tsRepository: TanStack/ai Length of output: 2364 🏁 Script executed: sed -n '1155,1260p' packages/ai-client/src/chat-client.ts
sed -n '1818,1950p' packages/ai-client/src/chat-client.ts
sed -n '2458,2505p' packages/ai-client/src/chat-client.tsRepository: TanStack/ai Length of output: 10350 🏁 Script executed: node - <<'JS'
const activeBuffer = []
let activeWaiters = []
const events = []
async function* subscribe() {
while (true) {
const buffered = activeBuffer.shift()
const chunk = buffered ?? await new Promise(resolve => activeWaiters.push(resolve))
if (chunk === null) return
yield chunk
}
}
async function* connect() {
yield { type: 'RUN_FINISHED', outcome: { type: 'interrupt' } }
}
async function processIncomingChunk(chunk) {
events.push('process-start')
events.push('interrupt-installed')
await new Promise(resolve => setTimeout(resolve, 0))
events.push('process-finished')
}
async function consumeSubscription() {
for await (const chunk of subscribe()) {
await processIncomingChunk(chunk)
}
}
async function waitUntilSubscriberIdle() {
if (activeBuffer.length === 0 && activeWaiters.length === 0) return
await Promise.resolve()
}
async function send() {
for await (const chunk of connect()) {
events.push('push')
const waiter = activeWaiters.shift()
if (waiter) waiter(chunk)
else activeBuffer.push(chunk)
}
await waitUntilSubscriberIdle()
events.push('send-resolved')
}
consumeSubscription()
send().then(() => events.push('append-send-continuation'))
setTimeout(() => console.log(events.join(' -> ')), 10)
JSRepository: TanStack/ai Length of output: 259 Wait for subscription processing to acknowledge each delivered chunk. When the 🤖 Prompt for AI Agents |
||
| let macrotaskWaits = 0 | ||
| while (!abortSignal?.aborted) { | ||
| if (idle()) return | ||
| if (activeBuffer.length === 0 && activeWaiters.length === 0) return | ||
| await new Promise<void>((resolve) => setTimeout(resolve, 0)) | ||
| macrotaskWaits++ | ||
| if (activeWaiters.length === 0 && macrotaskWaits >= 32) return | ||
| } | ||
| } | ||
|
|
||
| return { | ||
| subscribe(abortSignal?: AbortSignal): AsyncIterable<StreamChunk> { | ||
| // Transfer ownership to the latest subscriber so only one active | ||
|
|
@@ -1162,6 +1183,7 @@ export function normalizeConnectionAdapter( | |
| } | ||
| throw err | ||
| } | ||
| await waitUntilSubscriberIdle(abortSignal) | ||
| }, | ||
| // Expose joinRun only when the underlying connection is resumable. Require | ||
| // a real function — `'joinRun' in connection` is true for | ||
|
|
||
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.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Qualify the completion guarantee for busy appends.
When
append()runs whileisLoadingis true,packages/ai-client/src/chat-client.ts:2175-2210queuesstreamResponse()and returns without awaiting it. The returned promise can therefore resolve before the queued HTTP response is processed.Qualify this statement for the non-busy path, or change
append()to await the queued operation.🤖 Prompt for AI Agents