feat: status property (ready, submitted, streaming error) for useChat #247
Conversation
📝 WalkthroughWalkthroughAdds a ChatClientState lifecycle type ('ready' | 'submitted' | 'streaming' | 'error'); exposes a new Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant ChatClient as ChatClient
participant UseChat as UseChat Hook
participant UI as UI
Client->>ChatClient: sendMessage(...)
Note right of ChatClient: setStatus('submitted')
ChatClient->>UseChat: onStatusChange('submitted')
ChatClient->>ChatClient: onStreamStart -> setStatus('streaming')
ChatClient->>UseChat: onStatusChange('streaming')
ChatClient->>ChatClient: onStreamEnd -> setStatus('ready')
ChatClient->>UseChat: onStatusChange('ready')
ChatClient->>UseChat: onError -> setStatus('error')
UseChat->>UI: expose status ('ready'|'submitted'|'streaming'|'error')
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 🧪 Unit Test Generation v2 is now available!We have significantly improved our unit test generation capabilities. To enable: Add this to your reviews:
finishing_touches:
unit_tests:
enabled: trueTry it out by using the Have feedback? Share your thoughts on our Discord thread! Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/typescript/ai-preact/src/types.ts (1)
27-31: Inconsistency with React:onStreamStartshould be omitted fromUseChatOptions.The React adapter omits
'onStreamStart'fromUseChatOptions(since the hook manages it internally to track status transitions), but the Preact adapter does not. This inconsistency allows Preact users to passonStreamStartwhile React users cannot.Suggested fix
export type UseChatOptions<TTools extends ReadonlyArray<AnyClientTool> = any> = Omit< ChatClientOptions<TTools>, - 'onMessagesChange' | 'onLoadingChange' | 'onErrorChange' + 'onMessagesChange' | 'onLoadingChange' | 'onErrorChange' | 'onStreamStart' >packages/typescript/ai-svelte/src/types.ts (1)
27-32: Inconsistency with React:onStreamStartshould be omitted fromCreateChatOptions.Same issue as in Preact — the React adapter omits
'onStreamStart'from options, but Svelte does not. For consistency, all framework adapters should handle internal callbacks the same way.Suggested fix
export type CreateChatOptions< TTools extends ReadonlyArray<AnyClientTool> = any, > = Omit< ChatClientOptions<TTools>, - 'onMessagesChange' | 'onLoadingChange' | 'onErrorChange' + 'onMessagesChange' | 'onLoadingChange' | 'onErrorChange' | 'onStreamStart' >packages/typescript/ai-client/src/chat-client.ts (1)
31-81: KeeponStreamStartin sync inupdateOptions.
updateOptionsupdatesonResponse/onChunk/onFinish/onErrorbut omitsonStreamStart, so refreshed callbacks won’t be picked up. Please add it for parity with the other callbacks.🛠️ Suggested fix
updateOptions(options: { connection?: ConnectionAdapter body?: Record<string, any> tools?: ReadonlyArray<AnyClientTool> onResponse?: (response?: Response) => void | Promise<void> onChunk?: (chunk: StreamChunk) => void onFinish?: (message: UIMessage) => void + onStreamStart?: () => void onError?: (error: Error) => void }): void { @@ if (options.onFinish !== undefined) { this.callbacksRef.current.onFinish = options.onFinish } + if (options.onStreamStart !== undefined) { + this.callbacksRef.current.onStreamStart = options.onStreamStart + } if (options.onError !== undefined) { this.callbacksRef.current.onError = options.onError } }packages/typescript/ai-preact/src/use-chat.ts (1)
1-10: Fix ESLint import order and type-only import usage.The linter reports two errors:
@tanstack/aitype import must occur afterpreact/hooks, andChatClientStateis only used as a type and should be imported withtype.🔧 Proposed import fix
-import type { AnyClientTool, ModelMessage } from '@tanstack/ai' -import { ChatClient, ChatClientState } from '@tanstack/ai-client' -import { - useCallback, - useEffect, - useId, - useMemo, - useRef, - useState, -} from 'preact/hooks' +import { + useCallback, + useEffect, + useId, + useMemo, + useRef, + useState, +} from 'preact/hooks' +import { ChatClient, type ChatClientState } from '@tanstack/ai-client' +import type { AnyClientTool, ModelMessage } from '@tanstack/ai'
🤖 Fix all issues with AI agents
In `@packages/typescript/ai-client/src/types.ts`:
- Around line 203-206: The JSDoc for the onStreamStart callback is missing a
space before the closing "*/"; update the comment above the onStreamStart?: ()
=> void declaration to match the project's JSDoc style (ensure a space before
the closing */, e.g. " * Callback when stream starts */" formatting) so it is
consistent with other comments in the file.
In `@packages/typescript/ai-svelte/src/create-chat.svelte.ts`:
- Around line 78-80: The onStreamStart handler currently only sets status =
'streaming' but doesn't call the user-provided callback; update the handler to
mirror the onFinish/onError pattern by invoking the passed-in callback (e.g.
call opts.onStreamStart?.() or onStreamStart?.() depending on how callbacks are
named) after setting status so user code is notified; reference the
onStreamStart handler and the existing onFinish/onError invocation pattern to
implement the call.
- Around line 1-2: Reorder the imports so that symbols from
'@tanstack/ai-client' come before '@tanstack/ai' and import ChatClientState as a
type-only import; specifically, update the top-level import statement to import
ChatClient and (type) ChatClientState from '@tanstack/ai-client' and then import
AnyClientTool and ModelMessage from '@tanstack/ai', keeping the rest of the file
(e.g. the $state<ChatClientState> usage) unchanged.
In `@packages/typescript/ai-vue/src/use-chat.ts`:
- Around line 48-50: The onStreamStart handler currently only sets status.value
= 'streaming' and ignores any user callback; update the onStreamStart
implementation to set status.value = 'streaming' then invoke the user-provided
callback (e.g., props.onStreamStart or options.onStreamStart /
callbacks.onStreamStart if available) in a safe way (check for existence before
calling) so consumer callbacks are forwarded while preserving the existing
status update.
🧹 Nitpick comments (5)
packages/typescript/ai-svelte/tests/use-chat.test.ts (1)
149-160: Consider adding status transition coverage for Svelte.This test only verifies the initial
'ready'state. To catch regressions and align with other framework tests, consider driving a mock stream/error to assert'submitted' → 'streaming' → 'ready'/'error'transitions.packages/typescript/ai-vue/tests/use-chat.test.ts (1)
1-10: Fix import order to satisfy ESLint rules.Type imports should occur after regular imports per the project's import order configuration.
♻️ Suggested fix
-import type { ModelMessage } from '@tanstack/ai' import { flushPromises } from '@vue/test-utils' import { describe, expect, it, vi } from 'vitest' -import type { UIMessage } from '../src/types' import { createMockConnectionAdapter, createTextChunks, createToolCallChunks, renderUseChat, } from './test-utils' +import type { ModelMessage } from '@tanstack/ai' +import type { UIMessage } from '../src/types'packages/typescript/ai-vue/tests/test-utils.ts (1)
1-5: Fix import order to satisfy ESLint rules.Type imports should occur after regular imports per the project's import order configuration.
♻️ Suggested fix
-import type { UIMessage } from '@tanstack/ai-client' import { mount } from '@vue/test-utils' import { defineComponent } from 'vue' -import type { UseChatOptions } from '../src/types' import { useChat } from '../src/use-chat' +import type { UIMessage } from '@tanstack/ai-client' +import type { UseChatOptions } from '../src/types'packages/typescript/ai-react/src/use-chat.ts (1)
2-2: Use type-only import forChatClientState.
ChatClientStateis only used as a type, so it should be imported using thetypekeyword to enable better tree-shaking and satisfy the ESLint rule.♻️ Suggested fix
-import { ChatClient, ChatClientState } from '@tanstack/ai-client' +import { ChatClient } from '@tanstack/ai-client' +import type { ChatClientState } from '@tanstack/ai-client'packages/typescript/ai-solid/src/use-chat.ts (1)
7-9: Fix import order and use type-only import forChatClientState.Two issues flagged by ESLint:
@tanstack/ai-clientimport should occur before the type import of@tanstack/aiChatClientStateis only used as a type♻️ Suggested fix
import { createEffect, createMemo, createSignal, createUniqueId, } from 'solid-js' - -import type { AnyClientTool, ModelMessage } from '@tanstack/ai' -import { ChatClient, ChatClientState } from '@tanstack/ai-client' +import { ChatClient } from '@tanstack/ai-client' +import type { AnyClientTool, ModelMessage } from '@tanstack/ai' +import type { ChatClientState } from '@tanstack/ai-client' import type { UIMessage, UseChatOptions, UseChatReturn } from './types'
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/typescript/ai-preact/src/use-chat.ts (1)
1-10: Resolve import order + type-only split (lint).This triggers
import/orderandimport/consistent-type-specifier-style.🛠️ Suggested fix
-import type { AnyClientTool, ModelMessage } from '@tanstack/ai' -import { ChatClient, type ChatClientState } from '@tanstack/ai-client' -import { - useCallback, - useEffect, - useId, - useMemo, - useRef, - useState, -} from 'preact/hooks' +import { + useCallback, + useEffect, + useId, + useMemo, + useRef, + useState, +} from 'preact/hooks' +import { ChatClient } from '@tanstack/ai-client' +import type { ChatClientState } from '@tanstack/ai-client' +import type { AnyClientTool, ModelMessage } from '@tanstack/ai'packages/typescript/ai-solid/src/use-chat.ts (1)
1-10: Fix import order + type-only split (lint).This violates
import/orderandimport/consistent-type-specifier-style.🛠️ Suggested fix
import { createEffect, createMemo, createSignal, createUniqueId, } from 'solid-js' -import type { AnyClientTool, ModelMessage } from '@tanstack/ai' -import { ChatClient, type ChatClientState } from '@tanstack/ai-client' +import { ChatClient } from '@tanstack/ai-client' +import type { ChatClientState } from '@tanstack/ai-client' +import type { AnyClientTool, ModelMessage } from '@tanstack/ai' import type { UIMessage, UseChatOptions, UseChatReturn } from './types'
🤖 Fix all issues with AI agents
In `@packages/typescript/ai-preact/src/use-chat.ts`:
- Around line 76-78: The onStreamStart handler currently only calls
setStatus('streaming') but never invokes the user-provided callback; update the
handler inside useChat (the onStreamStart block) to call the passed-in user
callback after updating status (e.g., options.onStreamStart or onStreamStart
prop) using safe optional invocation (options?.onStreamStart?.(...)) and forward
any relevant args/events to it so the user's callback runs when streaming
starts.
In `@packages/typescript/ai-react/src/use-chat.ts`:
- Around line 1-3: The import currently mixes a type with a value import for
ChatClientState; change it to a top-level type-only import by moving
ChatClientState into its own import line using the `import type {
ChatClientState } from '@tanstack/ai-client'` form and leave `import {
ChatClient } from '@tanstack/ai-client'` (or vice versa) so ChatClientState is
imported purely as a type (refer to the ChatClient and ChatClientState symbols
in use-chat.ts).
- Around line 54-56: The onStreamStart handler currently only calls
setStatus("streaming") and never invokes the consumer's callback; update the
onStreamStart block in use-chat.ts (the onStreamStart handler) to call the
user-provided callback after updating status — e.g., keep setStatus("streaming")
then check for and invoke the external onStreamStart callback (the
prop/option/callback passed into the useChat/useChat hook) so consumers receive
the event.
In `@packages/typescript/ai-solid/src/use-chat.ts`:
- Around line 58-60: The onStreamStart handler currently only calls
setStatus('streaming') but doesn't invoke the consumer's callback; update the
onStreamStart implementation to call the user-provided onStreamStart callback
after setting status (e.g., if (props.onStreamStart)
props.onStreamStart(...args) or options.onStreamStart?.(args)), preserving any
arguments the stream start provides and guarding for its existence so both
setStatus('streaming') and the user's callback run.
In `@packages/typescript/ai-vue/src/use-chat.ts`:
- Around line 1-4: The file imports ChatClientState with a mixed value/type
import which violates the consistent-type-specifier-style rule; change the
imports to use a top-level type-only import for ChatClientState and keep
ChatClient as a value import — i.e., split the current "import { ChatClient,
type ChatClientState } from '@tanstack/ai-client'" into a value import for
ChatClient and a separate "import type { ChatClientState }" so ChatClientState
is only imported as a type; update references to ChatClientState and ChatClient
in use-chat.ts accordingly.
♻️ Duplicate comments (2)
packages/typescript/ai-svelte/src/create-chat.svelte.ts (1)
1-3: Fix import ordering + type-only split to satisfy lint.ESLint still flags
import/orderandimport/consistent-type-specifier-stylehere.🛠️ Suggested fix
-import type { AnyClientTool, ModelMessage } from '@tanstack/ai' -import { ChatClient, type ChatClientState } from '@tanstack/ai-client' +import { ChatClient } from '@tanstack/ai-client' +import type { ChatClientState } from '@tanstack/ai-client' +import type { AnyClientTool, ModelMessage } from '@tanstack/ai'packages/typescript/ai-vue/src/use-chat.ts (1)
48-50: ForwardonStreamStartto consumer callback.Status changes but the user callback is skipped.
🛠️ Suggested fix
onStreamStart: () => { status.value = 'streaming' + options.onStreamStart?.() },
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/typescript/ai-react/src/use-chat.ts`:
- Around line 1-4: Reorder the import statements to satisfy ESLint import-order
rules by placing React imports (useCallback, useEffect, useId, useMemo, useRef,
useState from 'react') before external package imports; move the line importing
ChatClient and ChatClientState from '@tanstack/ai-client' and the types
AnyClientTool and ModelMessage from '@tanstack/ai' after the React import so
that React comes first, then third‑party packages, keeping each import's
original named symbols (ChatClient, ChatClientState, AnyClientTool,
ModelMessage) unchanged.
In `@packages/typescript/ai-vue/src/use-chat.ts`:
- Around line 1-4: Reorder the imports so the concrete module imports from
'@tanstack/ai-client' (ChatClient, ChatClientState) appear before the type-only
import from '@tanstack/ai' (AnyClientTool, ModelMessage) to satisfy ESLint
ordering; keep the 'type' modifiers on type-only imports (e.g., type {
AnyClientTool, ModelMessage } and type { ChatClientState }) and preserve the
existing Vue imports (onScopeDispose, readonly, shallowRef, useId) after the
tanstack imports.
♻️ Duplicate comments (2)
packages/typescript/ai-react/src/use-chat.ts (1)
55-57: ForwardonStreamStartto the consumer callback.The internal handler updates status but doesn’t invoke the user’s
onStreamStart, so external listeners never fire.🛠️ Proposed fix
onStreamStart: () => { - setStatus("streaming") + setStatus('streaming') + optionsRef.current.onStreamStart?.() },packages/typescript/ai-vue/src/use-chat.ts (1)
49-50: ForwardonStreamStartto user callbacks.The hook updates status but doesn’t invoke
options.onStreamStart, so consumers miss the event.🛠️ Proposed fix
onStreamStart: () => { status.value = 'streaming' + options.onStreamStart?.() },
|
View your CI Pipeline Execution ↗ for commit 5168f0d
☁️ Nx Cloud last updated this comment at |
AlemTuzlak
left a comment
AlemTuzlak
left a comment
There was a problem hiding this comment.
@jherr Do you think we should collapse the status into the ChatClient internally instead of requiring state management per lib? I think that would be a sane way of approaching this and then the consumer libs just read it instead of having to deal with re-implementing the logic 5x times?
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/typescript/ai-vue/tests/test-utils.ts`:
- Around line 1-5: Reorder the imports so type-only imports come after the value
import from '../src/use-chat' to satisfy ESLint import/order: keep value imports
like import { useChat } from '../src/use-chat' and runtime imports (mount,
defineComponent) earlier, then move type imports (import type { UIMessage } and
import type { UseChatOptions }) immediately after those, ensuring you use
"import type" for the type-only symbols (UIMessage, UseChatOptions) and leave
useChat, mount, and defineComponent as value imports.
♻️ Duplicate comments (2)
packages/typescript/ai-react/src/use-chat.ts (2)
1-4: Resolve ESLint import/order violation.Line 1-4 should place React imports first and keep
@tanstack/ai-clientbefore the@tanstack/aitype import.🧹 Proposed import reorder
-import type { AnyClientTool, ModelMessage } from '@tanstack/ai' -import { ChatClient } from '@tanstack/ai-client' -import type { ChatClientState } from '@tanstack/ai-client' -import { useCallback, useEffect, useId, useMemo, useRef, useState } from 'react' +import { useCallback, useEffect, useId, useMemo, useRef, useState } from 'react' +import { ChatClient } from '@tanstack/ai-client' +import type { ChatClientState } from '@tanstack/ai-client' +import type { AnyClientTool, ModelMessage } from '@tanstack/ai'
55-65: Forward onStreamStart to the user callback.Line 55-57 updates status but drops the consumer callback, so external listeners never fire.
🛠️ Suggested fix
onStreamStart: () => { setStatus('streaming') + optionsRef.current.onStreamStart?.() },
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/typescript/ai-preact/tests/use-chat.test.ts`:
- Around line 1-10: Reorder the imports so type-only imports come after the
regular imports: move "import type { ModelMessage } from '@tanstack/ai'" and
"import type { UIMessage } from '../src/types'" to after the block that imports
createMockConnectionAdapter, createTextChunks, createToolCallChunks, and
renderUseChat from './test-utils' to satisfy the import/order ESLint rule; keep
the existing testing-library and vitest imports where they are and ensure only
type imports are relocated.
In `@packages/typescript/ai-vue/tests/use-chat.test.ts`:
- Around line 1-10: Move the type-only imports (ModelMessage, UIMessage) below
the regular imports from the test-utils module so they follow the
createMockConnectionAdapter, createTextChunks, createToolCallChunks,
renderUseChat imports; specifically, reorder the top of the file so the non-type
import block (createMockConnectionAdapter, createTextChunks,
createToolCallChunks, renderUseChat) appears before the type imports
(ModelMessage, UIMessage) to satisfy the import/order ESLint rule.
🧹 Nitpick comments (4)
packages/typescript/ai-client/src/chat-client.ts (2)
1-16: Import order flagged by linter.ESLint reports that regular imports should occur before type imports from the same module. This is a style issue.
♻️ Suggested fix
-import type { AnyClientTool, ModelMessage, StreamChunk } from '@tanstack/ai' import { StreamProcessor, generateMessageId, normalizeToUIMessage, } from '@tanstack/ai' +import type { AnyClientTool, ModelMessage, StreamChunk } from '@tanstack/ai' import type { ConnectionAdapter } from './connection-adapters' import type { ChatClientEventEmitter } from './events' import { DefaultChatClientEventEmitter } from './events'
81-91: DuplicatesetStatus('error')call when errors occur in stream processing.When an error occurs during stream processing,
setError(error)is called at line 89, which internally callssetStatus('error')(lines 205-207). Then line 90 explicitly callssetStatus('error')again. While this is harmless (idempotent), it triggersonStatusChangetwice with the same value.♻️ Suggested fix - remove redundant call
onError: (error: Error) => { this.setError(error) - this.setStatus('error') this.callbacksRef.current.onError(error) },packages/typescript/ai-preact/src/use-chat.ts (1)
1-2: Import order and type specifier style flagged by linter.ESLint reports import order issues and suggests using top-level type-only imports. This is a style consistency matter.
♻️ Suggested fix
+import { + useCallback, + useEffect, + useId, + useMemo, + useRef, + useState, +} from 'preact/hooks' + import type { AnyClientTool, ModelMessage } from '@tanstack/ai' -import { ChatClient, type ChatClientState } from '@tanstack/ai-client' -import { - useCallback, - useEffect, - useId, - useMemo, - useRef, - useState, -} from 'preact/hooks' +import type { ChatClientState } from '@tanstack/ai-client' +import { ChatClient } from '@tanstack/ai-client'packages/typescript/ai-solid/src/use-chat.ts (1)
7-9: Import order and type specifier style flagged by linter.ESLint reports import order issues and suggests using top-level type-only imports.
♻️ Suggested fix
import { createEffect, createMemo, createSignal, createUniqueId, } from 'solid-js' - import type { AnyClientTool, ModelMessage } from '@tanstack/ai' -import { ChatClient, type ChatClientState } from '@tanstack/ai-client' +import type { ChatClientState } from '@tanstack/ai-client' +import { ChatClient } from '@tanstack/ai-client' import type { UIMessage, UseChatOptions, UseChatReturn } from './types'
AlemTuzlak
left a comment
AlemTuzlak
left a comment
There was a problem hiding this comment.
This looks really neat now! Thank you @nikas-belogolov , I'll let @jherr review it as well
Just please address my comment on the error status
@tanstack/ai
@tanstack/ai-anthropic
@tanstack/ai-client
@tanstack/ai-devtools-core
@tanstack/ai-gemini
@tanstack/ai-grok
@tanstack/ai-ollama
@tanstack/ai-openai
@tanstack/ai-openrouter
@tanstack/ai-preact
@tanstack/ai-react
@tanstack/ai-react-ui
@tanstack/ai-solid
@tanstack/ai-solid-ui
@tanstack/ai-svelte
@tanstack/ai-vue
@tanstack/ai-vue-ui
@tanstack/preact-ai-devtools
@tanstack/react-ai-devtools
@tanstack/solid-ai-devtools
commit: |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/typescript/ai-client/src/chat-client.ts`:
- Around line 88-91: The onError handler is redundantly calling
setStatus('error') after setError(error) (which already sets status), causing
duplicate onStatusChange notifications; remove the extra setStatus('error')
calls in the onError blocks so only setError(error) is invoked before calling
callbacksRef.current.onError(error); apply the same removal for the second
occurrence around the code that mirrors this logic (the one referenced at lines
205-207).
…ored test cases for status
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/typescript/ai-preact/tests/use-chat.test.ts`:
- Around line 566-650: Three status tests are commented out (initial status,
error transition, stop transition) in the use-chat test suite; locate the tests
using identifiers renderUseChat, createMockConnectionAdapter,
result.current.sendMessage, result.current.stop and either (A) re-enable and fix
them so they pass (restore the original it(...) blocks and address any
framework-specific failure causing assertions on result.current.status), or (B)
explicitly mark them as skipped with a reason using it.skip('reason', ...) or
add a TODO/FIXME comment above each block explaining why they are disabled and
linking to an issue; pick one approach and apply it consistently for the three
blocks so test intent and visibility are preserved.
🧹 Nitpick comments (3)
packages/typescript/ai-client/src/chat-client.ts (1)
1-16: Fix import order per ESLint rules.The static analysis indicates import order violations. Type imports from
@tanstack/aishould occur after regular imports, and other type imports should be grouped appropriately.🔧 Suggested fix
-import type { AnyClientTool, ModelMessage, StreamChunk } from '@tanstack/ai' import { StreamProcessor, generateMessageId, normalizeToUIMessage, } from '@tanstack/ai' -import type { ConnectionAdapter } from './connection-adapters' -import type { ChatClientEventEmitter } from './events' import { DefaultChatClientEventEmitter } from './events' +import type { AnyClientTool, ModelMessage, StreamChunk } from '@tanstack/ai' +import type { ConnectionAdapter } from './connection-adapters' +import type { ChatClientEventEmitter } from './events' import type { ChatClientOptions, ChatClientState, MessagePart, ToolCallPart, UIMessage, } from './types'packages/typescript/ai-client/tests/chat-client.test.ts (1)
395-423: Consider adding explicit tests for initial status and error transitions.The status test suite covers the happy path well but could be more comprehensive:
- Missing explicit test for initial status being
'ready'(constructor default)- Missing dedicated test for
statustransitioning to'error'(currently only tested implicitly in the error handling section)This would align with the test coverage in other framework test files (e.g., Svelte tests have explicit error transition tests in the status section).
💡 Suggested additional tests
it('should have initial status of ready', () => { const adapter = createMockConnectionAdapter() const client = new ChatClient({ connection: adapter }) expect(client.getStatus()).toBe('ready') }) it('should transition to error on connection failure', async () => { const adapter = createMockConnectionAdapter({ shouldError: true, error: new Error('Test error'), }) const client = new ChatClient({ connection: adapter }) await client.sendMessage('Test') expect(client.getStatus()).toBe('error') })packages/typescript/ai-vue/tests/use-chat.test.ts (1)
466-489: Consider adding a test for the'error'status transition.The test covers the happy path (
'ready'→'submitted'/'streaming'→'ready'), but the'error'status is not explicitly tested. Since the status property includes'error'as a valid state, consider adding a test to verify the status transitions to'error'when an error occurs.♻️ Suggested test addition
it('should transition through states during generation', async () => { // ... existing test }) + + it('should transition to error status on failure', async () => { + const error = new Error('Test error') + const adapter = createMockConnectionAdapter({ + shouldError: true, + error, + }) + const { result } = renderUseChat({ connection: adapter }) + + expect(result.current.status).toBe('ready') + + await result.current.sendMessage('Test') + await flushPromises() + + expect(result.current.status).toBe('error') + expect(result.current.error).toBeDefined() + }) })
|
Looks good. |
|
The PR has a bunch of eslint issues, you can repro by just running pnpm run test on the root, fix those up and good to go! |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/typescript/ai-solid/src/use-chat.ts (1)
63-73: Fix copy-paste comment error.Line 65 mentions "React state" but this is the SolidJS adapter.
📝 Suggested fix
// Sync initial messages on mount only - // Note: initialMessages are passed to ChatClient constructor, but we also - // set them here to ensure React state is in sync + // Note: initialMessages are passed to ChatClient constructor, but we also + // set them here to ensure Solid state is in sync
🧹 Nitpick comments (1)
packages/typescript/ai-solid/src/use-chat.ts (1)
7-9: Minor formatting inconsistency.There's an extra blank line between SolidJS imports and the
@tanstack/ai-clientimport. Consider removing for consistency with other framework adapters.♻️ Suggested fix
import { createEffect, createMemo, createSignal, createUniqueId, } from 'solid-js' - -import { ChatClient } from '@tanstack/ai-client' +import { ChatClient } from '@tanstack/ai-client' import type {ChatClientState} from '@tanstack/ai-client';
|
@AlemTuzlak fixed it, should be all good |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/typescript/ai-preact/src/use-chat.ts (1)
26-72: Reset status to 'ready' when a new client instance is created.ChatClient does not emit
onStatusChangeduring construction—it initializes its internal status to 'ready' but does not notify the React component. Ifoptions.idchanges while the client is in 'error' or 'streaming' state, the new ChatClient created by the useMemo will have status='ready' internally, but the React state will retain the old status until the next client operation triggersonStatusChange. This creates a stale UI state.Add a useEffect to reset status when the client changes:
Suggested fix
const client = useMemo(() => { // ... }, [clientId]) + useEffect(() => { + setStatus('ready') + }, [client])
🤖 Fix all issues with AI agents
In `@packages/typescript/ai-solid/src/use-chat.ts`:
- Around line 52-54: The hook's internal onStatusChange handler currently
overwrites any user-supplied callback so their callback never runs; in the
useChat hook update the handler that calls setStatus(newStatus) to also invoke
the user's callback from UseChatOptions (e.g., options.onStatusChange) after
updating state, i.e., chain or conditionally call the provided onStatusChange
with the newStatus so both setStatus and the user's onStatusChange run (refer to
the onStatusChange handler and setStatus in useChat and the UseChatOptions type
to locate the change).
3 participants
🎯 Changes
Added a status property to useChat to track the generation lifecycle (ready, submitted, streaming, error).
✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.