feat(ai-client): client-side chat persistence adapter#661
Open
AlemTuzlak wants to merge 6 commits into
Open
Conversation
Add an optional `persistence` adapter to ChatClient with the same getItem/setItem/removeItem shape used elsewhere in the SDK. When provided, the client hydrates from getItem(id) on construction (sync or async), saves to setItem(id, messages) on every message change via an ordered write queue, and calls removeItem(id) on clear(). Late chunks from a stream cleared mid-flight are suppressed so they can't repopulate cleared state. When omitted, behavior is unchanged. The persistence option is threaded through all framework wrappers (react, preact, solid, vue, svelte), which now read hydrated messages from the client after construction. Implements #374.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds ChatClient persistence docs and sidebar, a localStorage E2E adapter and Playwright test, a changeset for minor package bumps, and extensive unit tests across ChatClient, ChatPersistor, and framework hooks validating hydration, write sequencing, clear semantics, and race conditions. ChangesPersistence Documentation and E2E Testing
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
docs/chat/persistence.md (1)
24-38: 💤 Low valueInterface snippet both imports and redeclares
ChatClientPersistence.The snippet does
import type { ChatClientPersistence }and theninterface ChatClientPersistence { ... }, which is a duplicate-identifier conflict if copied as-is, andUIMessageis referenced without being imported. Since this block is purely illustrating the shape, dropping the import (or showing it as the canonical definition only) avoids confusion.📝 Suggested snippet cleanup
-import type { ChatClientPersistence } from "`@tanstack/ai-client`"; - -interface ChatClientPersistence { +import type { UIMessage } from "`@tanstack/ai-client`"; + +interface ChatClientPersistence {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/chat/persistence.md` around lines 24 - 38, The snippet both imports and redeclares ChatClientPersistence which causes a duplicate identifier and also references UIMessage without importing it; fix by removing the redundant import line (drop "import type { ChatClientPersistence }"), keep the canonical interface ChatClientPersistence declaration, and either add an import type { UIMessage } or include a minimal UIMessage type/placeholder so UIMessage is defined for the example.packages/typescript/ai-react/tests/use-chat.test.ts (1)
938-976: 💤 Low valueInconsistent indentation in test block.
The
itblock and its contents have inconsistent indentation (6 spaces forit, 8 spaces for body). While syntactically valid, this makes the test structure harder to follow.The test logic is correct - it verifies that callbacks from stale clients are ignored after id changes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/typescript/ai-react/tests/use-chat.test.ts` around lines 938 - 976, The test block for "should ignore user callbacks from an old client after id changes" has inconsistent indentation; fix by normalizing spacing for the `it` block and its body to match the project's style (e.g., 2 spaces per indent) so the `it(...)` line and its inner lines (variables like releaseOldStream, oldOnChunk, newOnChunk, adapter, the renderHook call for useChat, sendPromise, rerender, releaseOldStream.resolve, and the final expects) are consistently indented; ensure nested blocks (async function, await/waitFor, and the rerender call) follow the same indentation level used across other tests.packages/typescript/ai-preact/src/use-chat.ts (1)
77-80: 💤 Low valueMinor inconsistency with React version's
onResponsehandling.The Preact version uses
return optionsRef.current.onResponse?.(response)while the React version usesvoid optionsRef.current.onResponse?.(response). Both are functionally equivalent since the return value isn't used, but the inconsistency could cause confusion during maintenance.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/typescript/ai-preact/src/use-chat.ts` around lines 77 - 80, The onResponse handler in use-chat.ts returns the result of optionsRef.current.onResponse?.(response) which is inconsistent with the React variant that uses void; update the onResponse implementation (the function that checks activeClientRef.current against instance) to call optionsRef.current.onResponse?.(response) with the void prefix (i.e., void optionsRef.current.onResponse?.(response)) instead of returning it so the behavior and style match React's use of onResponse.packages/typescript/ai-preact/tests/use-chat.test.ts (1)
1370-1371: 💤 Low valuePreact tests include additional
statusassertion for error scenarios.The Preact error tests assert
expect(result.current.status).toBe('error')which the React tests don't include. This is actually improved coverage - consider adding the same assertions to the React tests for parity.Also applies to: 1390-1391
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/typescript/ai-preact/tests/use-chat.test.ts` around lines 1370 - 1371, Add the same error-status assertion used in the Preact tests to the corresponding React tests: after the existing error expectation(s) add expect(result.current.status).toBe('error') in the React use-chat test cases that mirror the Preact ones (the two locations corresponding to the Preact lines around 1370 and 1390), so the React tests assert result.current.status === 'error' for those error scenarios.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/typescript/ai-client/src/chat-client.ts`:
- Around line 45-48: The Sets clearedMessageIds and clearedToolCallIds (and
similarly clearedRunIds, ignoredActiveRunIds) grow unbounded because nothing
ever removes entries; fix by adding eviction/pruning: replace the raw Sets with
a small LRU or Map<string, number> that records insertion timestamps for
clearedMessageIds and clearedToolCallIds (and
clearedRunIds/ignoredActiveRunIds), and prune old entries on each
clear()/send()/persist cycle or enforce a max size and evict oldest entries;
update any code that checks membership to use the new Map/LRU (e.g., wherever
clearedMessageIds.has(...) or clearedToolCallIds.has(...) is used) and add a
pruneOldEntries() utility called from clear(), send(), and the persistence load
path so suppression state does not leak indefinitely.
- Around line 485-504: The bug is that currentRunlessRunId is a single slot so
stale runless native chunks can slip through; replace that single-slot tracking
with a Set (e.g., runlessRunIds: Set<string>) and update all places that
read/write currentRunlessRunId (handlers that process RUN_STARTED/RUN_ENDED,
clear(), and any assignment sites) to add/remove the run id to/from
runlessRunIds instead of overwriting a single value; then update
isRunlessChunkFromIgnoredRun to call getChunkRunId and, when a chunk has no
runId, check whether any id in runlessRunIds is present in ignoredActiveRunIds
or clearedRunIds (treat chunk as ignored if so) so multiple concurrent runless
runs are protected.
---
Nitpick comments:
In `@docs/chat/persistence.md`:
- Around line 24-38: The snippet both imports and redeclares
ChatClientPersistence which causes a duplicate identifier and also references
UIMessage without importing it; fix by removing the redundant import line (drop
"import type { ChatClientPersistence }"), keep the canonical interface
ChatClientPersistence declaration, and either add an import type { UIMessage }
or include a minimal UIMessage type/placeholder so UIMessage is defined for the
example.
In `@packages/typescript/ai-preact/src/use-chat.ts`:
- Around line 77-80: The onResponse handler in use-chat.ts returns the result of
optionsRef.current.onResponse?.(response) which is inconsistent with the React
variant that uses void; update the onResponse implementation (the function that
checks activeClientRef.current against instance) to call
optionsRef.current.onResponse?.(response) with the void prefix (i.e., void
optionsRef.current.onResponse?.(response)) instead of returning it so the
behavior and style match React's use of onResponse.
In `@packages/typescript/ai-preact/tests/use-chat.test.ts`:
- Around line 1370-1371: Add the same error-status assertion used in the Preact
tests to the corresponding React tests: after the existing error expectation(s)
add expect(result.current.status).toBe('error') in the React use-chat test cases
that mirror the Preact ones (the two locations corresponding to the Preact lines
around 1370 and 1390), so the React tests assert result.current.status ===
'error' for those error scenarios.
In `@packages/typescript/ai-react/tests/use-chat.test.ts`:
- Around line 938-976: The test block for "should ignore user callbacks from an
old client after id changes" has inconsistent indentation; fix by normalizing
spacing for the `it` block and its body to match the project's style (e.g., 2
spaces per indent) so the `it(...)` line and its inner lines (variables like
releaseOldStream, oldOnChunk, newOnChunk, adapter, the renderHook call for
useChat, sendPromise, rerender, releaseOldStream.resolve, and the final expects)
are consistently indented; ensure nested blocks (async function, await/waitFor,
and the rerender call) follow the same indentation level used across other
tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 04182668-4a8b-412c-9a75-4c205200e6e2
📒 Files selected for processing (20)
.changeset/chat-client-persistence.mddocs/chat/persistence.mddocs/config.jsonpackages/typescript/ai-client/src/chat-client.tspackages/typescript/ai-client/src/connection-adapters.tspackages/typescript/ai-client/src/index.tspackages/typescript/ai-client/src/types.tspackages/typescript/ai-client/tests/chat-client.test.tspackages/typescript/ai-preact/src/use-chat.tspackages/typescript/ai-preact/tests/use-chat.test.tspackages/typescript/ai-react/src/use-chat.tspackages/typescript/ai-react/tests/use-chat.test.tspackages/typescript/ai-solid/src/use-chat.tspackages/typescript/ai-solid/tests/use-chat.test.tspackages/typescript/ai-svelte/src/create-chat.svelte.tspackages/typescript/ai-svelte/tests/use-chat.test.tspackages/typescript/ai-vue/src/use-chat.tspackages/typescript/ai-vue/tests/use-chat.test.tstesting/e2e/src/routes/$provider/$feature.tsxtesting/e2e/tests/chat.spec.ts
Move all persistence concerns out of ChatClient into a dedicated ChatPersistor: storage orchestration (hydrate / save / remove via an ordered write queue with generation guards) and clear-during-stream chunk suppression (cleared/ignored id tracking). ChatClient now holds a single optional `persistor` and delegates, keeping it focused on streaming and message state. The run-lifecycle methods keep their activeRunIds / session-generating / processing side-effects and call persistor hooks (onRunStarted, onRunSettled, onSessionRunError, resetIgnored, takeRunlessRunId) for the cleared-set bookkeeping. A shared getChunkRunId helper is exported from connection-adapters so both modules use one source. Pure refactor: no public API or behavior change. All ai-client unit tests, framework wrapper tests, and the persistence E2E test pass unchanged; chat-client.js shrinks ~34kB -> ~28kB.
Add a dedicated isolation suite for the extracted ChatPersistor (31 tests) covering edge cases that are awkward to trigger through the full ChatClient: - readInitial: sync read, async promise passthrough, throwing adapter - hydrateAsync: array / null / undefined / non-promise / rejection, plus the generation guard that drops a stale async hydration after a local change - notifyMessagesChanged: persists a snapshot (not the live array), skips exactly one write after beginClear, swallows sync errors, runs async writes FIFO with no overlap, and isolates rejections - remove: removeItem, swallowed errors, and generation invalidation of a queued write a removal supersedes - shouldIgnoreChunk: every branch (cleared message id, cleared run id, in-flight currentRunId, runless content/snapshot, parentMessageId with toolCallId memoization) - run lifecycle hooks: onRunSettled forget + runless-pointer advance, onSessionRunError, takeRunlessRunId, resetIgnored (partial reset), and the guard that a non-cleared run is never suppressed Adds shared createUIMessage / createMockPersistence helpers to test-utils. Full ai-client suite: 279 passing.
…istence # Conflicts: # packages/ai-client/src/chat-client.ts # packages/ai-client/src/client-persistor.ts # packages/ai-client/src/types.ts # packages/ai-client/tests/chat-client.test.ts # packages/ai-client/tests/client-persistor.test.ts # packages/ai-client/tests/test-utils.ts # packages/ai-preact/src/use-chat.ts # packages/ai-react/src/use-chat.ts # packages/typescript/ai-client/src/connection-adapters.ts # testing/e2e/src/routes/$provider/$feature.tsx
Contributor
🚀 Changeset Version Preview6 package(s) bumped directly, 5 bumped as dependents. 🟥 Major bumps
🟨 Minor bumps
🟩 Patch bumps
|
|
View your CI Pipeline Execution ↗ for commit 1f67db3
☁️ Nx Cloud last updated this comment at |
@tanstack/ai
@tanstack/ai-anthropic
@tanstack/ai-client
@tanstack/ai-code-mode
@tanstack/ai-code-mode-skills
@tanstack/ai-devtools-core
@tanstack/ai-elevenlabs
@tanstack/ai-event-client
@tanstack/ai-fal
@tanstack/ai-gemini
@tanstack/ai-grok
@tanstack/ai-groq
@tanstack/ai-isolate-cloudflare
@tanstack/ai-isolate-node
@tanstack/ai-isolate-quickjs
@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-utils
@tanstack/ai-vue
@tanstack/ai-vue-ui
@tanstack/openai-base
@tanstack/preact-ai-devtools
@tanstack/react-ai-devtools
@tanstack/solid-ai-devtools
commit: |
The test relied on a 10ms timer racing against vi.waitFor to ensure clear() ran before the delayed chunks. On faster machines/CI the chunks were processed before clear(), so clear() wiped them and the assertion saw an empty message list. Gate the chunks on a deferred released after clear() instead, making the ordering deterministic across platforms.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an optional, opt-in persistence adapter to
ChatClientso client-side conversations survive reloads — no manualinitialMessages+onFinishboilerplate. Implements #374.The adapter uses the same
getItem/setItem/removeItemshape as the rest of the SDK and may be sync or async:When provided,
ChatClient:getItem(id)on construction (async hydration is race-guarded against a newly-started conversation),setItem(id, messages)on every message change, through an ordered write queue so writes never overlap or land out of order,removeItem(id)onclear(), discarding any in-flight stream so late chunks can't repopulate cleared state.When omitted, behavior is unchanged — fully backwards compatible. Persistence is best-effort: adapter errors are swallowed so storage problems never break the chat.
Changes
@tanstack/ai-client:ChatClientPersistenceinterface +persistenceoption onChatClient; hydrate / save-queue / clear integration; mid-stream clear chunk suppression. Exported from the package index.react,preact,solid,vue,svelte): thread thepersistenceoption through and read hydrated messages from the client after construction.docs/chat/persistence.md(interface, behavior, localStorage + IndexedDB examples), registered under Chat & Streaming.Testing
ai-client(hydration sync/async, save-on-change, clear, mid-stream clear suppression) and in every framework wrapper.localStorageadapter.ai-client248 unit tests pass; all wrapper unit tests pass;test:typesandtest:eslintclean across all six packages;ai-clientbuilds;test:docspasses; the persistence E2E test passes (full E2E suite green).Summary by CodeRabbit
New Features
Documentation
Tests