Compare commits

...

3 Commits

Author SHA1 Message Date
claude code agent 227
8f9d7fe0bd test(ai-chat): cover the #161 New-chat-during-stream reset + changelog
Address PR #162 review must-fixes:

- CHANGELOG: add an [Unreleased] > Fixed bullet for #161 (New chat during
  the first turn's stream now resets the thread; a late refetch/onFinish
  from the abandoned thread no longer pulls the user back in).
- Re-anchor the stale startNewChat/cancelPendingAdoption test to selectChat:
  startNewChat now calls startFreshThread, but cancelPendingAdoption still
  backs selectChat, so the disarm guard is decoupled and kept valid.
- Add a test pinning that startFreshThread() disarms the armed error-path
  fallback (the justification for dropping cancelPendingAdoption from
  startNewChat).
- Add a ChatThread render test (mocked useChat) asserting the onError branch
  forwards onTurnFinished(undefined, threadKey).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-26 16:59:09 +03:00
claude_code
4d69518037 Merge branch 'develop' into fix/ai-chat-new-chat-during-stream (#162)
Resolve the PR #162 review blocker: the branch no longer merged into
develop after #174 (onServerChatId early adoption) and #183
(server-sourced Copy/export) landed, which touched the same files.

Conflict resolution (ai-chat-window.tsx, chat-thread.tsx,
use-chat-session.ts):
- Keep develop's onServerChatId wiring (#174) intact in all three files.
- Keep develop's removal of the old liveStateRef / liveThreadRef /
  MutableRefObject live-export mechanism (deleted by #174/#183); it was
  only inherited by this branch and is not part of #161 — not resurrected.
- Graft #162's actual fix on top: startFreshThread() (unconditional
  remount even when activeChatId stays null) and the abandoned-thread
  guard (threadKeyRef + finishingThreadKey on onTurnFinished, forwarded
  as ChatThread's threadKey through onFinish/onError).

Also apply the review's simplification: drop the now-redundant
cancelPendingAdoption() call inside startNewChat (startFreshThread()
already nulls pendingNewChatRef); the export stays, still used by
selectChat.

ai-chat client suite green (176 passed), no conflict markers.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-25 22:34:38 +03:00
claude code agent 227
cee231b136 fix(ai-chat): reset chat on New chat during the first turn's stream (#161)
Pressing "New chat" while a brand-new chat's first turn was still streaming
only removed the role badge — the thread, session and stream were kept. The
thread remount is driven by a render-phase reconciler in useChatSession that
fires only when activeChatId !== thread.chatId; on a not-yet-adopted new chat
both are null, so startNewChat's setActiveChatId(null) was a no-op and nothing
remounted.

- useChatSession: add startFreshThread(), which unconditionally dispatches a
  reconcile to a fresh mount key so ChatThread remounts even when activeChatId
  stays null. startNewChat now calls it.
- Guard against the abandoned thread's late finish: ai@6's useChat does not
  abort on unmount and proxies its callbacks, so onFinish/onError of the chat
  the user just left still fire and would adopt it back. onTurnFinished now
  takes the finishing thread's key and, when it no longer matches the mounted
  thread, only refreshes the chat list — no adoption, no fallback arming, no
  per-chat message invalidation. ChatThread forwards its threadKey in both
  onFinish and onError. An omitted key (legacy callers/tests) counts as the
  current thread, preserving the #137 adoption behavior.

Tests: 3 new useChatSession cases (fresh-thread remount with activeChatId null,
abandoned-thread finish rejected, current-thread finish still adopts). Full
ai-chat suite green (183).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-24 14:43:07 +03:00
6 changed files with 260 additions and 18 deletions

View File

@@ -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

View File

@@ -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}
/>

View File

@@ -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();
});
});

View File

@@ -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);
},
});

View File

@@ -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({

View File

@@ -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,
};
}