Compare commits
3 Commits
v0.94.1
...
fix/ai-cha
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
8f9d7fe0bd | ||
|
|
4d69518037 | ||
|
|
cee231b136 |
10
CHANGELOG.md
10
CHANGELOG.md
@@ -92,6 +92,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
no longer froze on the previous step's authoritative usage; the current step's
|
||||
estimate is combined per-component with `max`, so the count rises smoothly and
|
||||
never jumps backwards. (#163)
|
||||
- **AI chat: "New chat" pressed during the first turn's stream now resets the
|
||||
thread instead of leaving the old turn streaming.** While a brand-new,
|
||||
not-yet-adopted chat streamed its first turn, hitting "New chat" left
|
||||
`activeChatId === null` (a no-op for the atom), so the reconciler never
|
||||
remounted and the in-flight thread kept streaming behind the fresh one — and a
|
||||
late refetch / late `onFinish` from that abandoned thread could yank the user
|
||||
back into the chat they just left. "New chat" now forces a fresh empty thread
|
||||
unconditionally and the finished thread's mount key is checked so a late
|
||||
callback from an abandoned thread no longer adopts or re-arms the fallback.
|
||||
(#161)
|
||||
|
||||
## [0.93.0] - 2026-06-21
|
||||
|
||||
|
||||
@@ -195,6 +195,7 @@ export default function AiChatWindow() {
|
||||
waitingForHistory,
|
||||
onTurnFinished,
|
||||
onServerChatId,
|
||||
startFreshThread,
|
||||
cancelPendingAdoption,
|
||||
} = useChatSession({
|
||||
activeChatId,
|
||||
@@ -209,18 +210,26 @@ export default function AiChatWindow() {
|
||||
|
||||
// startNewChat/selectChat set the public atom; the hook's render-phase
|
||||
// reconciler handles the remount when activeChatId actually CHANGES. But
|
||||
// pressing "New chat" while already in a new chat leaves activeChatId === null
|
||||
// (a no-op for the atom), so the reconciler never fires — explicitly disarm any
|
||||
// armed error-path fallback here so a late refetch can't yank the user into a
|
||||
// just-failed chat after they chose a fresh one.
|
||||
// pressing "New chat" while already in a not-yet-adopted new chat leaves
|
||||
// activeChatId === null (a no-op for the atom), so the reconciler never fires —
|
||||
// startFreshThread forces the remount unconditionally (and disarms any armed
|
||||
// error-path fallback) so a late refetch can't yank the user into a just-failed
|
||||
// chat after they chose a fresh one (#161).
|
||||
const startNewChat = useCallback((): void => {
|
||||
cancelPendingAdoption();
|
||||
// Force a fresh thread UNCONDITIONALLY. On a brand-new, not-yet-adopted chat
|
||||
// (activeChatId === null) whose first turn is still streaming, setActiveChatId
|
||||
// (null) is a no-op and the render-phase reconciler would not remount — leaving
|
||||
// the streaming thread in place (#161). startFreshThread guarantees a clean
|
||||
// remount and already disarms any armed error-path fallback (so a separate
|
||||
// cancelPendingAdoption() call here is redundant); the abandoned thread's late
|
||||
// finish is rejected by the threadKey guard in the session hook.
|
||||
startFreshThread();
|
||||
setActiveChatId(null);
|
||||
setHistoryOpen(false);
|
||||
setDraft("");
|
||||
// Default the picker back to "Universal assistant" for the fresh chat.
|
||||
setSelectedRoleId(null);
|
||||
}, [cancelPendingAdoption, setActiveChatId, setDraft, setSelectedRoleId]);
|
||||
}, [startFreshThread, setActiveChatId, setDraft, setSelectedRoleId]);
|
||||
|
||||
const selectChat = useCallback(
|
||||
(chatId: string): void => {
|
||||
@@ -633,6 +642,7 @@ export default function AiChatWindow() {
|
||||
onRolePicked={(role) => setSelectedRoleId(role.id)}
|
||||
assistantName={currentRole?.name}
|
||||
onTurnFinished={onTurnFinished}
|
||||
threadKey={threadKey}
|
||||
onServerChatId={onServerChatId}
|
||||
onLiveTurnTokens={setLiveTurnTokens}
|
||||
/>
|
||||
|
||||
@@ -0,0 +1,94 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { render, act } from "@testing-library/react";
|
||||
import { MantineProvider } from "@mantine/core";
|
||||
|
||||
// Capture the options ChatThread passes to useChat so the test can drive the
|
||||
// hook's terminal callbacks (here: onError) directly, without a real stream. The
|
||||
// box is created via vi.hoisted so the hoisted vi.mock factory below can close
|
||||
// over it.
|
||||
const { useChatBox } = vi.hoisted(() => ({
|
||||
useChatBox: { options: null as unknown as Record<string, unknown> | null },
|
||||
}));
|
||||
|
||||
// Mock the AI SDK hook: record the options and return an inert, ready store so
|
||||
// ChatThread renders without any network/streaming machinery.
|
||||
vi.mock("@ai-sdk/react", () => ({
|
||||
useChat: (options: Record<string, unknown>) => {
|
||||
useChatBox.options = options;
|
||||
return {
|
||||
messages: [],
|
||||
sendMessage: vi.fn(),
|
||||
status: "ready",
|
||||
stop: vi.fn(),
|
||||
error: null,
|
||||
};
|
||||
},
|
||||
}));
|
||||
|
||||
// Stub react-i18next so `t` returns the key (other component tests use the same
|
||||
// pattern); ChatThread's rendered chrome is irrelevant to this wiring test.
|
||||
vi.mock("react-i18next", () => ({
|
||||
useTranslation: () => ({ t: (key: string) => key }),
|
||||
}));
|
||||
|
||||
// Mock the heavy presentational children to trivial stubs — this test only
|
||||
// exercises the onError → onTurnFinished wiring, not their rendering.
|
||||
vi.mock("@/features/ai-chat/components/message-list.tsx", () => ({
|
||||
default: () => null,
|
||||
}));
|
||||
vi.mock("@/features/ai-chat/components/chat-input.tsx", () => ({
|
||||
default: () => null,
|
||||
}));
|
||||
vi.mock("@/features/ai-chat/components/role-cards.tsx", () => ({
|
||||
default: () => null,
|
||||
}));
|
||||
vi.mock("@/features/ai-chat/components/chat-error-alert.tsx", () => ({
|
||||
default: () => null,
|
||||
}));
|
||||
vi.mock("@/features/ai-chat/components/chat-stopped-notice.tsx", () => ({
|
||||
default: () => null,
|
||||
}));
|
||||
|
||||
import ChatThread from "./chat-thread";
|
||||
|
||||
// matchMedia (read by MantineProvider) is stubbed globally in vitest.setup.ts.
|
||||
|
||||
describe("ChatThread onError wiring (#161)", () => {
|
||||
beforeEach(() => {
|
||||
useChatBox.options = null;
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it("onError calls onTurnFinished with (undefined, threadKey) so a late error from an abandoned thread is rejected", () => {
|
||||
const onTurnFinished = vi.fn();
|
||||
// Silence the deliberate console.error ChatThread logs for devtools.
|
||||
const consoleError = vi
|
||||
.spyOn(console, "error")
|
||||
.mockImplementation(() => {});
|
||||
|
||||
render(
|
||||
<MantineProvider>
|
||||
<ChatThread
|
||||
chatId="c1"
|
||||
onTurnFinished={onTurnFinished}
|
||||
threadKey="thread-key-1"
|
||||
/>
|
||||
</MantineProvider>,
|
||||
);
|
||||
|
||||
const options = useChatBox.options;
|
||||
expect(options).not.toBeNull();
|
||||
expect(typeof options?.onError).toBe("function");
|
||||
|
||||
// Drive the captured onError exactly as the AI SDK would on a stream error.
|
||||
act(() => {
|
||||
(options!.onError as (e: Error) => void)(new Error("stream blew up"));
|
||||
});
|
||||
|
||||
// The thread's own mount key must be forwarded with NO server id, so the
|
||||
// session hook can reject this finish if the thread has been abandoned.
|
||||
expect(onTurnFinished).toHaveBeenCalledWith(undefined, "thread-key-1");
|
||||
|
||||
consoleError.mockRestore();
|
||||
});
|
||||
});
|
||||
@@ -60,7 +60,12 @@ interface ChatThreadProps {
|
||||
* new chat, adopts the freshly created chat id. `serverChatId` is the
|
||||
* authoritative id the server streamed on the assistant message metadata, or
|
||||
* undefined on a failed turn — see adopt-chat-id.ts for the full #137 design. */
|
||||
onTurnFinished: (serverChatId?: string) => void;
|
||||
onTurnFinished: (serverChatId?: string, finishingThreadKey?: string) => void;
|
||||
/** This thread's mount key (the same value the parent uses as React `key`).
|
||||
* Forwarded back through onTurnFinished so the session hook can tell a finish
|
||||
* from THIS still-mounted thread from a late finish of an abandoned thread the
|
||||
* user already left via New chat / switch (#161). */
|
||||
threadKey?: string;
|
||||
/** Called EARLY (at the stream's `start` chunk) with the authoritative server
|
||||
* chat id streamed on the assistant message metadata, so a brand-new chat
|
||||
* adopts its real id WHILE the first turn is still streaming (#174 — makes the
|
||||
@@ -116,6 +121,7 @@ export default function ChatThread({
|
||||
onRolePicked,
|
||||
assistantName,
|
||||
onTurnFinished,
|
||||
threadKey,
|
||||
onServerChatId,
|
||||
onLiveTurnTokens,
|
||||
}: ChatThreadProps) {
|
||||
@@ -258,7 +264,7 @@ export default function ChatThread({
|
||||
// Forward the authoritative server chatId (streamed on the assistant
|
||||
// message metadata) so the parent adopts the REAL created chat id for a new
|
||||
// chat — see adopt-chat-id.ts for the full #137 design.
|
||||
onTurnFinished(extractServerChatId(message));
|
||||
onTurnFinished(extractServerChatId(message), threadKey);
|
||||
// Show a neutral "stopped" marker for an aborted turn; the red error banner
|
||||
// (via `error`) already covers isError, and a clean finish clears any marker.
|
||||
if (isError) setStopNotice(null);
|
||||
@@ -279,7 +285,7 @@ export default function ChatThread({
|
||||
// Surface the raw failure in the browser console (devtools) for debugging;
|
||||
// the UI separately shows a friendly classified banner (see errorView).
|
||||
console.error("AI chat stream error:", streamError);
|
||||
onTurnFinished();
|
||||
onTurnFinished(undefined, threadKey);
|
||||
},
|
||||
});
|
||||
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { renderHook } from "@testing-library/react";
|
||||
import { renderHook, act } from "@testing-library/react";
|
||||
import { useChatSession } from "./use-chat-session";
|
||||
import type { UseChatSessionOptions } from "./use-chat-session";
|
||||
|
||||
@@ -120,16 +120,40 @@ describe("useChatSession", () => {
|
||||
expect(setActiveChatId).not.toHaveBeenCalledWith("new");
|
||||
});
|
||||
|
||||
it("startNewChat while already in a new chat: cancelPendingAdoption stops a late refetch adopting the failed chat", () => {
|
||||
// The Warning path the render-phase reconciler can't catch: pressing "New
|
||||
// chat" while already in a new chat keeps activeChatId === null (a no-op for
|
||||
// the atom), so only the explicit cancelPendingAdoption() disarms.
|
||||
it("cancelPendingAdoption (selectChat) disarms a late refetch from adopting the just-failed chat", () => {
|
||||
// cancelPendingAdoption is the explicit disarm the window calls from
|
||||
// selectChat: switching to a chat whose id == null is a no-op for the atom, so
|
||||
// the render-phase reconciler never fires and only this call disarms an armed
|
||||
// error-path fallback. (startNewChat no longer routes through here — it calls
|
||||
// startFreshThread, covered by the next test — but cancelPendingAdoption still
|
||||
// backs selectChat, so this guard must hold.)
|
||||
const { result, rerender, setActiveChatId } = setup({
|
||||
activeChatId: null,
|
||||
chats: { items: [{ id: "x" }] },
|
||||
});
|
||||
result.current.onTurnFinished(undefined); // first turn failed → arm (before=["x"])
|
||||
result.current.cancelPendingAdoption(); // window calls this from startNewChat
|
||||
result.current.cancelPendingAdoption(); // window calls this from selectChat
|
||||
// The just-failed row lands in a late refetch; it must NOT be adopted.
|
||||
rerender({
|
||||
activeChatId: null,
|
||||
chats: { items: [{ id: "x" }, { id: "failed" }] },
|
||||
});
|
||||
expect(setActiveChatId).not.toHaveBeenCalledWith("failed");
|
||||
});
|
||||
|
||||
it("#161: startFreshThread disarms the armed error-path fallback (New chat during the first turn)", () => {
|
||||
// Pressing "New chat" while already in a not-yet-adopted new chat keeps
|
||||
// activeChatId === null, so the render-phase reconciler never fires. The
|
||||
// window now calls startFreshThread() (NOT cancelPendingAdoption) to force a
|
||||
// fresh thread; this test pins the load-bearing fact that startFreshThread
|
||||
// ALSO nulls pendingNewChatRef, so a late refetch of the just-failed row can't
|
||||
// yank the user back into the abandoned chat.
|
||||
const { result, rerender, setActiveChatId } = setup({
|
||||
activeChatId: null,
|
||||
chats: { items: [{ id: "x" }] },
|
||||
});
|
||||
result.current.onTurnFinished(undefined); // first turn failed → arm (before=["x"])
|
||||
act(() => result.current.startFreshThread()); // "New chat" → fresh thread + disarm
|
||||
// The just-failed row lands in a late refetch; it must NOT be adopted.
|
||||
rerender({
|
||||
activeChatId: null,
|
||||
@@ -227,6 +251,48 @@ describe("useChatSession", () => {
|
||||
expect(result.current.threadKey).toBe("C");
|
||||
});
|
||||
|
||||
it("#161: startFreshThread remounts even when activeChatId stays null (New chat mid-stream)", () => {
|
||||
// The bug: pressing New chat while still on a brand-new, not-yet-adopted chat
|
||||
// leaves activeChatId === null, so the render-phase reconciler never fires.
|
||||
// startFreshThread must remount UNCONDITIONALLY (a new mount key).
|
||||
const { result } = setup({ activeChatId: null, chats: { items: [] } });
|
||||
const keyBefore = result.current.threadKey;
|
||||
act(() => result.current.startFreshThread());
|
||||
expect(result.current.threadKey).not.toBe(keyBefore);
|
||||
});
|
||||
|
||||
it("#161: a late finish from an ABANDONED thread does not adopt or invalidate messages", () => {
|
||||
const {
|
||||
result,
|
||||
setActiveChatId,
|
||||
onInvalidateChatList,
|
||||
onInvalidateChatMessages,
|
||||
} = setup({ activeChatId: null, chats: { items: [{ id: "x" }] } });
|
||||
const abandonedKey = result.current.threadKey;
|
||||
// User pressed New chat mid-stream → fresh thread (new mount key).
|
||||
act(() => result.current.startFreshThread());
|
||||
expect(result.current.threadKey).not.toBe(abandonedKey);
|
||||
// The left-behind thread's onFinish fires late, carrying ITS (now stale) key
|
||||
// and the server id of the chat the user just left. It must NOT be adopted.
|
||||
act(() => result.current.onTurnFinished("A", abandonedKey));
|
||||
expect(setActiveChatId).not.toHaveBeenCalled();
|
||||
expect(onInvalidateChatMessages).not.toHaveBeenCalled();
|
||||
// The abandoned chat should still surface in the history list.
|
||||
expect(onInvalidateChatList).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("#161: a finish from the CURRENT thread (matching key) still adopts", () => {
|
||||
const { result, setActiveChatId } = setup({
|
||||
activeChatId: null,
|
||||
chats: { items: [{ id: "x" }] },
|
||||
});
|
||||
// Same thread that is mounted reports its finish with the matching key.
|
||||
act(() =>
|
||||
result.current.onTurnFinished("A", result.current.threadKey),
|
||||
);
|
||||
expect(setActiveChatId).toHaveBeenCalledWith("A");
|
||||
});
|
||||
|
||||
it("waitingForHistory gates the loader only while opening an unloaded existing chat", () => {
|
||||
// Open an existing chat whose history is still loading => loader on.
|
||||
const { result, rerender } = setup({
|
||||
|
||||
@@ -32,8 +32,13 @@ export interface UseChatSessionResult {
|
||||
/** Show the history loader instead of the live thread. */
|
||||
waitingForHistory: boolean;
|
||||
/** Call when a turn finishes; `serverChatId` is the authoritative streamed id
|
||||
* (undefined on a failed turn). Handles new-chat id adoption + invalidations. */
|
||||
onTurnFinished: (serverChatId?: string) => void;
|
||||
* (undefined on a failed turn). `finishingThreadKey` is the mount key of the
|
||||
* thread that produced this callback — when it no longer matches the mounted
|
||||
* thread (the user pressed New chat / switched mid-stream), the call is from an
|
||||
* abandoned thread and must NOT adopt or re-arm the fallback. Omitting it (old
|
||||
* callers / tests) treats the call as belonging to the current thread. Handles
|
||||
* new-chat id adoption + invalidations. */
|
||||
onTurnFinished: (serverChatId?: string, finishingThreadKey?: string) => void;
|
||||
/** Call EARLY (at the stream's `start` chunk) with the authoritative streamed
|
||||
* chat id so a brand-new chat adopts its real id WHILE its first turn is still
|
||||
* streaming — making `activeChatId`-gated affordances (e.g. the Copy/export
|
||||
@@ -41,6 +46,13 @@ export interface UseChatSessionResult {
|
||||
* no list/messages invalidation — that is left to onTurnFinished at the end).
|
||||
* Idempotent and a no-op once the chat already has an id. */
|
||||
onServerChatId: (serverChatId?: string) => void;
|
||||
/** Force a brand-new, empty thread (new mount key, no chat id) UNCONDITIONALLY.
|
||||
* The render-phase reconciler only remounts when `activeChatId` actually
|
||||
* changes; pressing "New chat" while already in a not-yet-adopted new chat
|
||||
* leaves `activeChatId === null` (a no-op for the atom), so the reconciler
|
||||
* never fires and the stale streaming thread stays mounted (#161). The window
|
||||
* calls this from startNewChat to guarantee a fresh thread regardless. */
|
||||
startFreshThread: () => void;
|
||||
/** Disarm any pending error-path new-chat fallback. The window calls this from
|
||||
* startNewChat/selectChat so a late refetch can't yank the user back into a
|
||||
* just-failed chat after they explicitly moved on. */
|
||||
@@ -85,6 +97,14 @@ export function useChatSession(
|
||||
const activeChatIdRef = useRef(activeChatId);
|
||||
activeChatIdRef.current = activeChatId;
|
||||
|
||||
// Live mirror of the mounted thread's key, read by onTurnFinished to tell a
|
||||
// current-thread finish from an ABANDONED one. ai@6's useChat does not abort
|
||||
// its request on unmount, and its callbacks are proxied so onFinish/onError of
|
||||
// a thread the user already left (via New chat / switch) still fire AFTER that
|
||||
// thread unmounts. By then this ref holds the NEW thread's key, so comparing it
|
||||
// to the key the finishing thread reports rejects the abandoned turn (#161).
|
||||
const threadKeyRef = useRef<string>("");
|
||||
|
||||
// The mounted thread's identity: ONE atomic value tying ChatThread's mount key
|
||||
// (`thread.key`) to the chat id that mounted thread holds (`thread.chatId`).
|
||||
// Consolidating these makes the "key vs chat id diverged" state unrepresentable
|
||||
@@ -98,6 +118,10 @@ export function useChatSession(
|
||||
: switchThread(activeChatId),
|
||||
);
|
||||
|
||||
// Keep the live mirror pointed at the currently-mounted thread's key so a late
|
||||
// onTurnFinished can be matched against it (see threadKeyRef above).
|
||||
threadKeyRef.current = thread.key;
|
||||
|
||||
// Error-path fallback for new-chat id adoption. When a brand-new chat's first
|
||||
// turn errors BEFORE the server's `start` chunk, no authoritative chatId ever
|
||||
// reaches the client, so the primary metadata adoption cannot run. We then ARM
|
||||
@@ -115,7 +139,21 @@ export function useChatSession(
|
||||
// yet) we adopt the server's AUTHORITATIVE streamed id (never the newest in the
|
||||
// list, which races a second tab — #137; see adopt-chat-id.ts).
|
||||
const onTurnFinished = useCallback(
|
||||
(serverChatId?: string) => {
|
||||
(serverChatId?: string, finishingThreadKey?: string) => {
|
||||
// Reject a finish from an ABANDONED thread. After the user pressed New chat
|
||||
// (or switched chats) mid-stream, the left-behind thread's onFinish/onError
|
||||
// still fire (ai@6 does not abort on unmount). Adopting/arming off that late
|
||||
// callback would yank the user back into the chat they just left (#161).
|
||||
// `undefined` (legacy callers/tests) is treated as the current thread.
|
||||
const isCurrentThread =
|
||||
finishingThreadKey === undefined ||
|
||||
finishingThreadKey === threadKeyRef.current;
|
||||
if (!isCurrentThread) {
|
||||
// Still surface the abandoned chat in the history list, but do NOT adopt,
|
||||
// arm the fallback, or invalidate per-chat messages (no thread shows it).
|
||||
onInvalidateChatList();
|
||||
return;
|
||||
}
|
||||
// Read the live id from the ref, not the closure: on a failed turn this can
|
||||
// run twice in one turn (onFinish + onError) before any re-render, and the
|
||||
// primary branch below updates the ref so the second call sees the adopted id.
|
||||
@@ -258,11 +296,29 @@ export function useChatSession(
|
||||
pendingNewChatRef.current = null;
|
||||
}, []);
|
||||
|
||||
// Force a fresh, empty thread regardless of the current `activeChatId`. The
|
||||
// render-phase reconciler only remounts on an activeChatId CHANGE, so "New chat"
|
||||
// pressed while already in a not-yet-adopted new chat (activeChatId stays null)
|
||||
// would otherwise leave the in-flight streaming thread mounted (#161). Dispatch
|
||||
// `reconcile` to chatId:null with a brand-new key so React remounts ChatThread
|
||||
// (a fresh useChat store). Disarm any armed fallback too. After this dispatch
|
||||
// thread.chatId is null; the window also sets activeChatId to null, so the
|
||||
// render-phase reconciler then finds them equal and does not double-remount.
|
||||
const startFreshThread = useCallback(() => {
|
||||
pendingNewChatRef.current = null;
|
||||
dispatch({
|
||||
type: "reconcile",
|
||||
chatId: null,
|
||||
newKey: `new-${generateId()}`,
|
||||
});
|
||||
}, []);
|
||||
|
||||
return {
|
||||
threadKey: thread.key,
|
||||
waitingForHistory,
|
||||
onTurnFinished,
|
||||
onServerChatId,
|
||||
startFreshThread,
|
||||
cancelPendingAdoption,
|
||||
};
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user