Compare commits
2 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
 agent_coder
|
96faa28220 | ||
|
agent_coder
|
654ba9f249 |
@@ -86,19 +86,11 @@ const MIN_HEIGHT = 400;
|
||||
// Margin kept between the window and the viewport edges while dragging.
|
||||
const EDGE_MARGIN = 8;
|
||||
|
||||
// #184 phase 1.5 / #430: backstop for the degraded-poll fallback. The poll is
|
||||
// armed when a resume attempt could not attach to the live run and disarmed by the
|
||||
// thread on settle / local stream; this cap is the ONLY backstop against an endless
|
||||
// tick (a stuck 'streaming' row before the boot-sweep, or a user-tail 204 with no
|
||||
// run).
|
||||
//
|
||||
// #430: measured from RUN ACTIVITY, not from arm-time. A real autonomous run takes
|
||||
// 11-25 min — longer than a fixed 10-min-from-start cap, which used to cut the poll
|
||||
// off mid-run. Instead we cap on INACTIVITY: keep polling as long as the run is
|
||||
// still making progress (its persisted rows keep changing), and only give up after
|
||||
// this long with NO new activity. A genuinely stuck run produces no row changes, so
|
||||
// the idle cap still bounds it; a long-but-progressing run polls to completion.
|
||||
const DEGRADED_POLL_IDLE_MAX_MS = 10 * 60_000;
|
||||
// #184 phase 1.5: hard cap on the degraded-poll fallback. The poll is armed when
|
||||
// a resume attempt could not attach to the live run and disarmed by the thread on
|
||||
// settle / local stream; this cap is the ONLY backstop against an endless tick
|
||||
// (a stuck 'streaming' row before the boot-sweep, or a user-tail 204 with no run).
|
||||
const DEGRADED_POLL_MAX_MS = 10 * 60_000;
|
||||
|
||||
/** Compact token formatter: 1.2M / 3.4k / 950. */
|
||||
function formatTokens(n: number): string {
|
||||
@@ -262,12 +254,9 @@ export default function AiChatWindow() {
|
||||
// onResumeFallback(true); the thread disarms it on settle / local stream. The
|
||||
// window only OWNS the timer (armedAtRef stamps when it was armed for the cap).
|
||||
const [degradedPoll, setDegradedPoll] = useState(false);
|
||||
// #430: timestamp of the LAST run activity while the poll is armed — stamped on
|
||||
// arm and re-stamped whenever the polled rows change (see the effect below). The
|
||||
// idle cap is measured from this, so a long-but-progressing run keeps polling.
|
||||
const lastActivityAtRef = useRef(0);
|
||||
const armedAtRef = useRef(0);
|
||||
const onResumeFallback = useCallback((active: boolean): void => {
|
||||
if (active) lastActivityAtRef.current = Date.now();
|
||||
if (active) armedAtRef.current = Date.now();
|
||||
setDegradedPoll(active);
|
||||
}, []);
|
||||
// Reset the degraded poll whenever the open chat changes: it is scoped to the
|
||||
@@ -280,28 +269,18 @@ export default function AiChatWindow() {
|
||||
useAiChatMessagesQuery(
|
||||
activeChatId ?? undefined,
|
||||
// DELIBERATELY DUMB (invariant 8 / task 2.4): poll every 2.5s while armed
|
||||
// and while the run is still active (#430: under the INACTIVITY cap, not a
|
||||
// fixed-from-start cap); otherwise off. NO error checks (TanStack v5 resets
|
||||
// fetchFailureCount each fetch, so consecutive errors are not expressible —
|
||||
// and the poll must survive a server restart) and NO tail checks (the
|
||||
// settled/local-stream semantics live in ChatThread, which disarms via
|
||||
// onResumeFallback(false)). The idle cap is the only backstop.
|
||||
// and under the 10-min cap; otherwise off. NO error checks (TanStack v5
|
||||
// resets fetchFailureCount each fetch, so consecutive errors are not
|
||||
// expressible — and the poll must survive a server restart) and NO tail
|
||||
// checks (the settled/local-stream semantics live in ChatThread, which
|
||||
// disarms via onResumeFallback(false)). The time cap is the only backstop.
|
||||
() =>
|
||||
degradedPoll === true &&
|
||||
Date.now() - lastActivityAtRef.current < DEGRADED_POLL_IDLE_MAX_MS
|
||||
Date.now() - armedAtRef.current < DEGRADED_POLL_MAX_MS
|
||||
? 2500
|
||||
: false,
|
||||
);
|
||||
|
||||
// #430: re-stamp the activity clock whenever the polled rows change while the
|
||||
// poll is armed. TanStack keeps the same `messageRows` reference across refetches
|
||||
// that return deep-equal data (structural sharing), so a new reference means the
|
||||
// run genuinely progressed — which extends the inactivity cap above. A stuck run
|
||||
// yields no reference change, so the cap eventually fires and stops the poll.
|
||||
useEffect(() => {
|
||||
if (degradedPoll) lastActivityAtRef.current = Date.now();
|
||||
}, [degradedPoll, messageRows]);
|
||||
|
||||
// #184 reconnect-and-live-follow. Whether detached agent runs are enabled for
|
||||
// this workspace. When the feature is off no runs are ever created, so the
|
||||
// resume attempt would only ever 204; gating ChatThread's resume on it avoids a
|
||||
|
||||
@@ -739,170 +739,3 @@ function renderResumable(initialRows: IAiChatMessageRow[]) {
|
||||
act(() => view.rerender(<Wrapper rows={rows} />));
|
||||
return { rerender, onResumeFallback };
|
||||
}
|
||||
|
||||
// #430: auto-reconnect to a DETACHED run after a LIVE SSE disconnect. The mount
|
||||
// path only resumes on mount/reload; these cover the missing trigger — a live
|
||||
// `isDisconnect` on onFinish must (backoff-)re-attach WITHOUT a reload, pin+strip
|
||||
// the live row to avoid duplicates, fall back to the degraded poll on a 204, and
|
||||
// exhaust to a manual Retry.
|
||||
describe("ChatThread — live reconnect after isDisconnect (#430)", () => {
|
||||
// A LIVE local turn that just dropped: the settled tail existed before, and the
|
||||
// partial assistant row lives only in `messages` (not persisted as a tail).
|
||||
const settledTail = () => [
|
||||
row("u1", "user", undefined, "hi"),
|
||||
row("a1", "assistant", "succeeded", "done"),
|
||||
];
|
||||
// The partial assistant message onFinish hands us for the dropped LIVE turn.
|
||||
const liveMsg = {
|
||||
id: "a2",
|
||||
role: "assistant",
|
||||
parts: [{ type: "text", text: "partial live answer" }],
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
resetState();
|
||||
// status "ready": with a live disconnect the mock is not streaming, so the
|
||||
// status==="streaming" auto-clear effect stays out of the way.
|
||||
h.state.status = "ready";
|
||||
vi.useFakeTimers();
|
||||
});
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
cleanup();
|
||||
});
|
||||
|
||||
// Render a NON-resuming mount (settled tail -> no mount resume) with autonomous
|
||||
// runs on, then simulate a live disconnect via onFinish.
|
||||
function renderLiveThenDisconnect() {
|
||||
const view = renderThread({
|
||||
autonomousRunsEnabled: true,
|
||||
initialRows: settledTail(),
|
||||
});
|
||||
// The settled tail must NOT have triggered a mount resume.
|
||||
expect(h.state.resumeStream).not.toHaveBeenCalled();
|
||||
act(() => {
|
||||
h.state.onFinish?.({
|
||||
message: liveMsg,
|
||||
isAbort: false,
|
||||
isDisconnect: true,
|
||||
isError: false,
|
||||
});
|
||||
});
|
||||
return view;
|
||||
}
|
||||
|
||||
// Fire the pending (scheduled) attempt for `attempt` (backoff = 1s,2s,4s,...).
|
||||
function advanceToAttempt(attempt: number) {
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(1000 * 2 ** (attempt - 1));
|
||||
});
|
||||
}
|
||||
|
||||
// Simulate the reconnect GET returning 204 (nothing live) so the transport's
|
||||
// no-active-stream recovery runs.
|
||||
async function reconnect204() {
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
vi.fn().mockResolvedValue({ status: 204, ok: false }),
|
||||
);
|
||||
await act(async () => {
|
||||
await h.state.transport!.fetch!("http://x", { method: "GET" });
|
||||
});
|
||||
}
|
||||
|
||||
// Simulate the reconnect GET returning a live 2xx stream.
|
||||
async function reconnect200() {
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
vi.fn().mockResolvedValue({ status: 200, ok: true }),
|
||||
);
|
||||
await act(async () => {
|
||||
await h.state.transport!.fetch!("http://x", { method: "GET" });
|
||||
});
|
||||
}
|
||||
|
||||
it("calls resumeStream POST-mount (a live disconnect triggers a backoff reconnect)", () => {
|
||||
renderLiveThenDisconnect();
|
||||
// The banner shows immediately; the attach itself fires after the first backoff.
|
||||
expect(screen.getByText(/reconnecting/i)).toBeTruthy();
|
||||
expect(h.state.resumeStream).not.toHaveBeenCalled();
|
||||
advanceToAttempt(1);
|
||||
// resumeStream is now called AFTER mount — the bug was it only ever fired once
|
||||
// on mount. The reconnect URL pins expect=live&anchor to OUR run.
|
||||
expect(h.state.resumeStream).toHaveBeenCalledTimes(1);
|
||||
expect(h.state.transport!.prepareReconnectToStreamRequest!().api).toBe(
|
||||
"/api/ai-chat/runs/c1/stream?expect=live&anchor=a2",
|
||||
);
|
||||
});
|
||||
|
||||
it("strips the pinned live row before replay so content is NOT duplicated", () => {
|
||||
renderLiveThenDisconnect();
|
||||
advanceToAttempt(1);
|
||||
// The attempt strips the anchor row from the store (the live replay rebuilds
|
||||
// it). Apply the setMessages updater to prove it removes exactly the anchor.
|
||||
const updater = h.state.setMessages.mock.calls.at(-1)![0] as (
|
||||
prev: { id: string }[],
|
||||
) => { id: string }[];
|
||||
expect(updater([{ id: "u1" }, { id: "a2" }])).toEqual([{ id: "u1" }]);
|
||||
});
|
||||
|
||||
it("a live re-attach (2xx) clears the reconnect banner", async () => {
|
||||
renderLiveThenDisconnect();
|
||||
advanceToAttempt(1);
|
||||
await reconnect200();
|
||||
expect(screen.queryByText(/reconnecting/i)).toBeNull();
|
||||
});
|
||||
|
||||
it("a 204 arms the degraded poll and backs off to the next attempt", async () => {
|
||||
const { onResumeFallback } = renderLiveThenDisconnect();
|
||||
advanceToAttempt(1);
|
||||
expect(h.state.resumeStream).toHaveBeenCalledTimes(1);
|
||||
await reconnect204();
|
||||
// Fallback engaged: the degraded poll is armed (204 -> onNoActiveStream).
|
||||
expect(onResumeFallback).toHaveBeenCalledWith(true);
|
||||
// Still reconnecting — the banner advanced to attempt 2/5.
|
||||
expect(screen.getByText(/reconnecting.*2\/5/i)).toBeTruthy();
|
||||
// The next backoff fires attempt 2 (another resumeStream).
|
||||
advanceToAttempt(2);
|
||||
expect(h.state.resumeStream).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("exhausts the attempt limit into a manual Retry, which restarts the sequence", async () => {
|
||||
renderLiveThenDisconnect();
|
||||
// Drive all 5 attempts, each failing with a 204.
|
||||
for (let n = 1; n <= 5; n++) {
|
||||
advanceToAttempt(n);
|
||||
expect(h.state.resumeStream).toHaveBeenCalledTimes(n);
|
||||
await reconnect204();
|
||||
}
|
||||
// The 5th 204 exhausted the cap -> the manual Retry replaces the banner.
|
||||
expect(screen.queryByText(/reconnecting/i)).toBeNull();
|
||||
const retry = screen.getByText("Retry");
|
||||
expect(retry).toBeTruthy();
|
||||
// Retry fires attempt 1 immediately (no backoff) — a 6th resumeStream.
|
||||
act(() => {
|
||||
fireEvent.click(retry);
|
||||
});
|
||||
expect(h.state.resumeStream).toHaveBeenCalledTimes(6);
|
||||
expect(screen.getByText(/reconnecting/i)).toBeTruthy();
|
||||
});
|
||||
|
||||
it("does NOT reconnect when autonomous runs are disabled", () => {
|
||||
renderThread({ autonomousRunsEnabled: false, initialRows: settledTail() });
|
||||
act(() => {
|
||||
h.state.onFinish?.({
|
||||
message: liveMsg,
|
||||
isAbort: false,
|
||||
isDisconnect: true,
|
||||
isError: false,
|
||||
});
|
||||
});
|
||||
expect(screen.queryByText(/reconnecting/i)).toBeNull();
|
||||
// The terminal "connection lost" notice is shown instead (unchanged behavior).
|
||||
expect(
|
||||
screen.getByText("Connection lost — the answer was interrupted."),
|
||||
).toBeTruthy();
|
||||
advanceToAttempt(1);
|
||||
expect(h.state.resumeStream).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,17 +1,7 @@
|
||||
import { useCallback, useEffect, useMemo, useRef, useState } from "react";
|
||||
import { useQueryClient } from "@tanstack/react-query";
|
||||
import { generateId } from "ai";
|
||||
import {
|
||||
ActionIcon,
|
||||
Alert,
|
||||
Box,
|
||||
Button,
|
||||
Group,
|
||||
Loader,
|
||||
Stack,
|
||||
Text,
|
||||
Tooltip,
|
||||
} from "@mantine/core";
|
||||
import { ActionIcon, Box, Group, Stack, Text, Tooltip } from "@mantine/core";
|
||||
import {
|
||||
IconClockHour4,
|
||||
IconPlayerPlayFilled,
|
||||
@@ -61,15 +51,6 @@ import classes from "@/features/ai-chat/components/ai-chat.module.css";
|
||||
// from the token rate.
|
||||
const STREAM_THROTTLE_MS = 50;
|
||||
|
||||
// #430: auto-reconnect after a LIVE SSE disconnect of a DETACHED (autonomous) run.
|
||||
// The run keeps executing server-side, so instead of a dead "Lost connection"
|
||||
// banner we re-attach to the live tail through the SAME resumable machinery the
|
||||
// mount path uses. Attempts back off exponentially and are capped; on exhaustion
|
||||
// the user gets a manual Retry (the degraded poll keeps catching up underneath).
|
||||
const RECONNECT_MAX_ATTEMPTS = 5;
|
||||
// Backoff before attempt N (1-based): 1s, 2s, 4s, 8s, 16s.
|
||||
const RECONNECT_BASE_DELAY_MS = 1000;
|
||||
|
||||
/** The page the user is currently viewing, sent as chat context. */
|
||||
export interface OpenPageContext {
|
||||
id: string;
|
||||
@@ -194,10 +175,6 @@ export default function ChatThread({
|
||||
const reconcileTailRef = useRef(false);
|
||||
const noStreamHandledRef = useRef(false);
|
||||
const onNoActiveStreamRef = useRef<(() => void) | null>(null);
|
||||
// #430: called from the transport's reconnect-GET success branch when a live
|
||||
// stream re-attached (2xx, not 204) — clears the reconnect banner. Kept in a ref
|
||||
// because the transport's fetch closure (useMemo([])) reads it live.
|
||||
const onReconnectAttachedRef = useRef<(() => void) | null>(null);
|
||||
// Live mount flag. The attach GET and the resumed `onFinish` are async and can
|
||||
// land AFTER this thread unmounts (the parent remounts per chat via `key`); with
|
||||
// chatIdRef then pointing at the NEW chat, an ungated late callback would arm a
|
||||
@@ -401,10 +378,6 @@ export default function ChatThread({
|
||||
// NOT drop the in-progress row or stop tracking the durable run.
|
||||
if (response.status === 204 || !response.ok)
|
||||
onNoActiveStreamRef.current?.();
|
||||
// #430: a 2xx stream re-attached (live tail or finished-replay). Signal
|
||||
// the reconnect controller to clear its banner. No-op outside an active
|
||||
// reconnect sequence (e.g. the mount attach), so it is safe here.
|
||||
else onReconnectAttachedRef.current?.();
|
||||
return response;
|
||||
} catch (err) {
|
||||
// Network throw: same no-onFinish recovery, then rethrow so the SDK
|
||||
@@ -508,31 +481,6 @@ export default function ChatThread({
|
||||
);
|
||||
}
|
||||
}
|
||||
// (2b) #430: a LIVE (non-resumed) detached run whose SSE just dropped. The
|
||||
// server run keeps executing, so instead of a dead "Lost connection" banner
|
||||
// start a reconnect sequence: pin the CURRENT streaming assistant row as the
|
||||
// strip/anchor (the live tail is the already-shown partial in `messages`, not
|
||||
// a persistent row) and re-attach to the live tail via the resumable machinery.
|
||||
const startedReconnect =
|
||||
isDisconnect &&
|
||||
!wasResumed &&
|
||||
autonomousRunsEnabled === true &&
|
||||
mountedRef.current &&
|
||||
message?.role === "assistant" &&
|
||||
typeof message.id === "string";
|
||||
if (startedReconnect) {
|
||||
beginReconnect({
|
||||
id: message.id,
|
||||
role: "assistant",
|
||||
content: "",
|
||||
status: "streaming",
|
||||
createdAt: new Date().toISOString(),
|
||||
// Preserve the partial parts so a 204 restore (onNoActiveStream) re-shows
|
||||
// what was on screen while the degraded poll catches the run up to
|
||||
// terminal (rowToUiMessage prefers metadata.parts).
|
||||
metadata: { parts: message.parts },
|
||||
});
|
||||
}
|
||||
// (3) Standard branches.
|
||||
// Forward the authoritative server chatId (streamed on the assistant
|
||||
// message metadata) so the parent adopts the REAL created chat id for a new
|
||||
@@ -542,11 +490,9 @@ export default function ChatThread({
|
||||
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.
|
||||
// On a live disconnect that STARTED a reconnect, suppress the terminal
|
||||
// "connection lost" notice — the reconnect banner takes over (#430).
|
||||
if (isError) setStopNotice(null);
|
||||
else if (isAbort) setStopNotice("manual");
|
||||
else if (isDisconnect) setStopNotice(startedReconnect ? null : "disconnect");
|
||||
else if (isDisconnect) setStopNotice("disconnect");
|
||||
else setStopNotice(null);
|
||||
// A resumed turn NEVER flushes the queue (invariant 7): skip BOTH the
|
||||
// flush-on-abort branch and the plain flush. The local streamer is the only
|
||||
@@ -633,106 +579,6 @@ export default function ChatThread({
|
||||
|
||||
const isStreaming = status === "submitted" || status === "streaming";
|
||||
|
||||
// #430: live-disconnect reconnect controller. `null` = idle; `{ trying, attempt }`
|
||||
// = a backoff sequence is running (drives the "reconnecting… (N/max)" banner);
|
||||
// `{ failed }` = attempts exhausted (drives the manual Retry). Mirrored into a ref
|
||||
// so the transport/onNoActiveStream closures branch on the LIVE value.
|
||||
type ReconnectState =
|
||||
| null
|
||||
| { phase: "trying"; attempt: number }
|
||||
| { phase: "failed" };
|
||||
const [reconnectState, setReconnectState] = useState<ReconnectState>(null);
|
||||
const reconnectStateRef = useRef<ReconnectState>(null);
|
||||
const setReconnectStatePair = useCallback((s: ReconnectState) => {
|
||||
reconnectStateRef.current = s;
|
||||
setReconnectState(s);
|
||||
}, []);
|
||||
const reconnectTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
|
||||
const clearReconnectTimer = useCallback(() => {
|
||||
if (reconnectTimerRef.current) {
|
||||
clearTimeout(reconnectTimerRef.current);
|
||||
reconnectTimerRef.current = null;
|
||||
}
|
||||
}, []);
|
||||
|
||||
// One reconnect attempt — MIRRORS the mount strip/anchor path for the LIVE case.
|
||||
// beginReconnect pinned strippedRowRef/stripRef to the run's assistant row, so:
|
||||
// - remove that row from the store (the mount path strips it from the SEED; here
|
||||
// it is already shown, so filter it out) — the live replay's `text-start` then
|
||||
// rebuilds it without DUPLICATING parts (the main dedup risk, #430);
|
||||
// - reset the one-shot 204 guard so onNoActiveStream can fire for THIS attempt;
|
||||
// - mark the turn resumed (invariant 7/8) so onFinish runs the recovery block and
|
||||
// never flushes the queue;
|
||||
// - resumeStream() -> prepareReconnectToStreamRequest builds
|
||||
// ?expect=live&anchor=<pinned id>, pinning the replay to OUR run (invariant 6).
|
||||
const attemptReconnectOnce = useCallback(
|
||||
(attempt: number) => {
|
||||
if (!mountedRef.current) return;
|
||||
const anchor = strippedRowRef.current;
|
||||
if (anchor) {
|
||||
setMessages((prev) => prev.filter((m) => m.id !== anchor.id));
|
||||
}
|
||||
noStreamHandledRef.current = false;
|
||||
setResumedTurnPair(true);
|
||||
setReconnectStatePair({ phase: "trying", attempt });
|
||||
void resumeStream();
|
||||
},
|
||||
[setMessages, setResumedTurnPair, setReconnectStatePair, resumeStream],
|
||||
);
|
||||
|
||||
// Schedule attempt `attempt` after an exponential backoff.
|
||||
const scheduleReconnectAttempt = useCallback(
|
||||
(attempt: number) => {
|
||||
clearReconnectTimer();
|
||||
setReconnectStatePair({ phase: "trying", attempt });
|
||||
reconnectTimerRef.current = setTimeout(
|
||||
() => attemptReconnectOnce(attempt),
|
||||
RECONNECT_BASE_DELAY_MS * 2 ** (attempt - 1),
|
||||
);
|
||||
},
|
||||
[clearReconnectTimer, setReconnectStatePair, attemptReconnectOnce],
|
||||
);
|
||||
|
||||
// Start a fresh reconnect sequence, pinning `anchorRow` (the live run's assistant
|
||||
// row) as the strip/anchor reused by every attempt.
|
||||
const beginReconnect = useCallback(
|
||||
(anchorRow: IAiChatMessageRow) => {
|
||||
if (!autonomousRunsEnabled || !mountedRef.current) return;
|
||||
strippedRowRef.current = anchorRow;
|
||||
stripRef.current = true;
|
||||
scheduleReconnectAttempt(1);
|
||||
},
|
||||
[autonomousRunsEnabled, scheduleReconnectAttempt],
|
||||
);
|
||||
|
||||
// Manual Retry (shown once attempts are exhausted): restart at attempt 1 and fire
|
||||
// immediately (the user asked for it now — no backoff).
|
||||
const retryReconnect = useCallback(() => {
|
||||
clearReconnectTimer();
|
||||
attemptReconnectOnce(1);
|
||||
}, [clearReconnectTimer, attemptReconnectOnce]);
|
||||
|
||||
// Live SSE re-attached (the reconnect GET returned a 2xx stream): clear the
|
||||
// banner + any pending backoff. No-op outside a sequence (e.g. the mount attach).
|
||||
const onReconnectAttached = useCallback(() => {
|
||||
if (!mountedRef.current || !reconnectStateRef.current) return;
|
||||
clearReconnectTimer();
|
||||
setReconnectStatePair(null);
|
||||
}, [clearReconnectTimer, setReconnectStatePair]);
|
||||
onReconnectAttachedRef.current = onReconnectAttached;
|
||||
|
||||
// The reconnect GET could not attach (204 / error). onNoActiveStream has already
|
||||
// armed the degraded poll (the robust fallback that drives the row to terminal
|
||||
// from the DB), so this only decides the LIVE-attach retry: back off and try
|
||||
// again up to the cap, else surface the manual Retry.
|
||||
const onReconnectNoStream = useCallback(() => {
|
||||
const s = reconnectStateRef.current;
|
||||
if (s?.phase !== "trying") return;
|
||||
if (s.attempt < RECONNECT_MAX_ATTEMPTS)
|
||||
scheduleReconnectAttempt(s.attempt + 1);
|
||||
else setReconnectStatePair({ phase: "failed" });
|
||||
}, [scheduleReconnectAttempt, setReconnectStatePair]);
|
||||
|
||||
// 204-handler (`onNoActiveStream`): the attach returned 204 — nothing live to
|
||||
// resume (overflow / begin-failure / after retention / anchor-mismatch). One-
|
||||
// shot via noStreamHandledRef (we do NOT null onNoActiveStreamRef). Exactly four
|
||||
@@ -764,17 +610,7 @@ export default function ChatThread({
|
||||
// (d) 204 means onFinish will NOT fire — clear the suppression flag so it
|
||||
// cannot swallow the NEXT local turn's queue flush.
|
||||
setResumedTurnPair(false);
|
||||
// (e) #430: if this 204/error landed during a live-disconnect reconnect
|
||||
// sequence, back off and retry the live attach (or give up to the manual
|
||||
// Retry). The degraded poll armed in (c) is the fallback either way.
|
||||
onReconnectNoStream();
|
||||
}, [
|
||||
setMessages,
|
||||
queryClient,
|
||||
onResumeFallback,
|
||||
setResumedTurnPair,
|
||||
onReconnectNoStream,
|
||||
]);
|
||||
}, [setMessages, queryClient, onResumeFallback, setResumedTurnPair]);
|
||||
onNoActiveStreamRef.current = onNoActiveStream;
|
||||
|
||||
// Mount effect: kick off the resume attempt for a non-settled tail. Marking the
|
||||
@@ -792,9 +628,6 @@ export default function ChatThread({
|
||||
return () => {
|
||||
mountedRef.current = false;
|
||||
attachAbortRef.current?.abort();
|
||||
// #430: drop any pending reconnect backoff so it can't fire against the next
|
||||
// chat this thread's refs are reused for.
|
||||
if (reconnectTimerRef.current) clearTimeout(reconnectTimerRef.current);
|
||||
};
|
||||
// Mount-only by design; the parent remounts per chat via `key`.
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
@@ -833,27 +666,12 @@ export default function ChatThread({
|
||||
if (tail.status !== "streaming") {
|
||||
reconcileTailRef.current = false;
|
||||
onResumeFallback?.(false);
|
||||
// #430: the run reached its terminal state via the degraded poll — there is
|
||||
// no live tail left to reconnect to, so drop any reconnect banner / Retry.
|
||||
clearReconnectTimer();
|
||||
setReconnectStatePair(null);
|
||||
}
|
||||
// onResumeFallback intentionally omitted (parent-stable callback); deps are
|
||||
// fixed by the resume design.
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [initialRows, isStreaming, setMessages]);
|
||||
|
||||
// #430: a real stream is live again — the reconnect re-attached to the live tail
|
||||
// (status -> "streaming") OR the user started a new local turn. Either way clear
|
||||
// the reconnect banner + any pending backoff. Gated on "streaming" (not the
|
||||
// broader "submitted") so a still-pending attach GET does not clear prematurely.
|
||||
useEffect(() => {
|
||||
if (status === "streaming") {
|
||||
clearReconnectTimer();
|
||||
setReconnectStatePair(null);
|
||||
}
|
||||
}, [status, clearReconnectTimer, setReconnectStatePair]);
|
||||
|
||||
// "Send now" on a queued message: interrupt the current turn and immediately
|
||||
// send THIS message, keeping the agent's partial output. Other queued messages
|
||||
// stay queued and flush normally after the new turn. Reuses the existing
|
||||
@@ -901,9 +719,6 @@ export default function ChatThread({
|
||||
// observer's Stop would otherwise leave the attach fetch running.
|
||||
attachAbortRef.current?.abort();
|
||||
stop();
|
||||
// #430: pressing Stop also cancels an in-progress reconnect sequence.
|
||||
clearReconnectTimer();
|
||||
setReconnectStatePair(null);
|
||||
if (!autonomousRunsEnabled) return;
|
||||
if (chatIdRef.current) {
|
||||
onServerStop?.(chatIdRef.current);
|
||||
@@ -925,13 +740,7 @@ export default function ChatThread({
|
||||
// for this fix. Documented so a future change can address the abort-ordering.
|
||||
stopPendingRef.current = true;
|
||||
}
|
||||
}, [
|
||||
stop,
|
||||
autonomousRunsEnabled,
|
||||
onServerStop,
|
||||
clearReconnectTimer,
|
||||
setReconnectStatePair,
|
||||
]);
|
||||
}, [stop, autonomousRunsEnabled, onServerStop]);
|
||||
|
||||
// Clear the stopped marker as soon as a new turn begins streaming, and drop any
|
||||
// stale "Send now" interrupt flags. On the legit interrupt path both refs are
|
||||
@@ -1016,43 +825,6 @@ export default function ChatThread({
|
||||
detail={errorView.detail}
|
||||
mb="xs"
|
||||
/>
|
||||
) : reconnectState ? (
|
||||
// #430: while auto-reconnecting to a detached run's live tail, show progress
|
||||
// instead of a dead "Lost connection" banner; once attempts are exhausted,
|
||||
// offer a manual Retry (the degraded poll keeps catching up underneath).
|
||||
<Alert
|
||||
variant="light"
|
||||
color="gray"
|
||||
p="xs"
|
||||
mb="xs"
|
||||
style={{ flexShrink: 0 }}
|
||||
>
|
||||
<Group gap={8} wrap="nowrap" align="center">
|
||||
{reconnectState.phase === "trying" ? (
|
||||
<>
|
||||
<Loader size={14} color="gray" style={{ flex: "none" }} />
|
||||
<Text size="sm" lh={1.3} c="dimmed">
|
||||
{t("Connection lost — reconnecting…")}
|
||||
{` (${reconnectState.attempt}/${RECONNECT_MAX_ATTEMPTS})`}
|
||||
</Text>
|
||||
</>
|
||||
) : (
|
||||
<>
|
||||
<Text size="sm" lh={1.3} c="dimmed" style={{ flex: 1 }}>
|
||||
{t("Couldn't reconnect to the answer.")}
|
||||
</Text>
|
||||
<Button
|
||||
size="compact-xs"
|
||||
variant="light"
|
||||
color="gray"
|
||||
onClick={retryReconnect}
|
||||
>
|
||||
{t("Retry")}
|
||||
</Button>
|
||||
</>
|
||||
)}
|
||||
</Group>
|
||||
</Alert>
|
||||
) : stopNotice ? (
|
||||
<ChatStoppedNotice
|
||||
text={
|
||||
|
||||
@@ -17,24 +17,10 @@ import { Injectable, Logger, OnModuleDestroy } from '@nestjs/common';
|
||||
/** How long a finished entry is retained for late attach (replay + immediate end). */
|
||||
export const RUN_STREAM_RETAIN_FINISHED_MS = 30_000;
|
||||
|
||||
/**
|
||||
* Per-run replay buffer cap. Past this the buffer is dropped (attach -> 204, and
|
||||
* the client falls back to its restore + degraded-poll path, #430).
|
||||
*
|
||||
* Raised from 4MB to 32MB (#430): marathon autonomous runs (11-25 min observed)
|
||||
* stream far more than 4MB of SSE frames, so a live disconnect mid-run would find
|
||||
* an already-overflowed buffer and could only degrade-poll instead of re-attaching
|
||||
* to the live tail. 32MB comfortably covers those runs while staying bounded.
|
||||
*
|
||||
* Memory cost: this is the WORST-CASE retained size PER ACTIVE run (the buffer is
|
||||
* freed on finish + retention, or dropped immediately on overflow). With the small
|
||||
* number of concurrent autonomous runs a single workspace realistically has, 32MB
|
||||
* each is an acceptable ceiling; the overflow->204->degraded-poll fallback remains
|
||||
* the backstop for anything larger, so correctness never depends on this bound.
|
||||
*/
|
||||
export const RUN_STREAM_MAX_BUFFER_BYTES = 32 * 1024 * 1024;
|
||||
/** Per-run replay buffer cap. Past this the buffer is dropped (attach -> 204). */
|
||||
export const RUN_STREAM_MAX_BUFFER_BYTES = 4 * 1024 * 1024;
|
||||
|
||||
// 2x the replay cap: a just-written full-replay burst alone can never trip the
|
||||
// 2x the replay cap: a just-written 4MB replay burst alone can never trip the
|
||||
// per-subscriber cap (see controller); only a genuinely stalled socket can.
|
||||
export const SUBSCRIBER_MAX_BUFFERED_BYTES = 2 * RUN_STREAM_MAX_BUFFER_BYTES;
|
||||
|
||||
|
||||
@@ -2,7 +2,6 @@ import {
|
||||
AiChatStreamRegistryService,
|
||||
RUN_STREAM_MAX_BUFFER_BYTES,
|
||||
RUN_STREAM_RETAIN_FINISHED_MS,
|
||||
SUBSCRIBER_MAX_BUFFERED_BYTES,
|
||||
RunStreamCallbacks,
|
||||
} from './ai-chat-stream-registry.service';
|
||||
|
||||
@@ -211,10 +210,9 @@ describe('AiChatStreamRegistryService', () => {
|
||||
const att = (await registry.attach(CHAT, false, undefined, c.cb))!;
|
||||
att.start();
|
||||
|
||||
// Cap-relative so it survives a buffer-cap change (#430): a quarter-cap frame
|
||||
// means 5 frames comfortably exceed the replay cap; the last one crosses.
|
||||
const chunk = 'x'.repeat(Math.floor(RUN_STREAM_MAX_BUFFER_BYTES / 4));
|
||||
for (let i = 0; i < 5; i++) src.push(chunk + i);
|
||||
const oneMb = 'x'.repeat(1024 * 1024);
|
||||
// 5 x 1MB = 5MB > 4MB cap; the 5th frame is the one that crosses.
|
||||
for (let i = 0; i < 5; i++) src.push(oneMb + i);
|
||||
await flush();
|
||||
|
||||
const entry = (registry as any).entries.get(CHAT);
|
||||
@@ -222,7 +220,7 @@ describe('AiChatStreamRegistryService', () => {
|
||||
expect(entry.bytes).toBeGreaterThan(RUN_STREAM_MAX_BUFFER_BYTES);
|
||||
// The live subscriber received ALL 5 frames, including the crossing one.
|
||||
expect(c.frames).toHaveLength(5);
|
||||
expect(c.frames[4]).toBe(chunk + 4);
|
||||
expect(c.frames[4]).toBe(oneMb + 4);
|
||||
|
||||
// A NEW attach after overflow gets null (replay buffer is gone).
|
||||
const c2 = collector();
|
||||
@@ -242,11 +240,9 @@ describe('AiChatStreamRegistryService', () => {
|
||||
const attB = (await registry.attach(CHAT, false, undefined, b.cb))!;
|
||||
attB.start();
|
||||
|
||||
// Cap-relative so it survives a buffer-cap change (#430): a quarter-of-the-
|
||||
// per-subscriber-cap frame means 5 frames exceed A's paused-pending cap while
|
||||
// B streams every frame live.
|
||||
const chunk = 'x'.repeat(Math.floor(SUBSCRIBER_MAX_BUFFERED_BYTES / 4));
|
||||
for (let i = 0; i < 5; i++) src.push(chunk + i);
|
||||
const oneMb = 'x'.repeat(1024 * 1024);
|
||||
// 9 x 1MB = 9MB > 8MB per-subscriber cap; A's pending overflows, B streams live.
|
||||
for (let i = 0; i < 9; i++) src.push(oneMb + i);
|
||||
await flush();
|
||||
|
||||
const entry = (registry as any).entries.get(CHAT);
|
||||
@@ -254,7 +250,7 @@ describe('AiChatStreamRegistryService', () => {
|
||||
expect(entry.subscribers.size).toBe(1);
|
||||
expect(a.frames).toEqual([]); // paused + overflowed: nothing was delivered
|
||||
// B received every frame live (delivery unaffected by A's overflow).
|
||||
expect(b.frames).toHaveLength(5);
|
||||
expect(b.frames).toHaveLength(9);
|
||||
|
||||
// A's start() (arriving late) degrades to an immediate end, not a partial replay.
|
||||
attA.start();
|
||||
|
||||
@@ -148,6 +148,53 @@ describe('assistantParts', () => {
|
||||
expect(toolPart).not.toHaveProperty('output');
|
||||
});
|
||||
|
||||
it('replays the REAL error text for a THROWN tool (tool-error part)', () => {
|
||||
const steps = [
|
||||
{
|
||||
text: '',
|
||||
toolCalls: [
|
||||
{ toolCallId: 'c7', toolName: 'editPageText', input: { id: 'p1' } },
|
||||
],
|
||||
// A thrown tool is a `tool-error` content part; toolResults holds only
|
||||
// successes and stays empty for this call.
|
||||
toolResults: [],
|
||||
content: [
|
||||
{
|
||||
type: 'tool-error',
|
||||
toolCallId: 'c7',
|
||||
toolName: 'editPageText',
|
||||
input: { id: 'p1' },
|
||||
error: new Error('page is locked'),
|
||||
},
|
||||
],
|
||||
},
|
||||
];
|
||||
const parts = assistantParts(steps, '') as AnyPart[];
|
||||
const toolPart = parts.find((p) => p.type === 'tool-editPageText');
|
||||
expect(toolPart).toBeDefined();
|
||||
expect(toolPart!.state).toBe('output-error');
|
||||
// The REAL error is replayed, NOT the 'Tool call did not complete.' placeholder.
|
||||
expect(toolPart!.errorText).toBe('page is locked');
|
||||
expect(toolPart).not.toHaveProperty('output');
|
||||
});
|
||||
|
||||
it('keeps the placeholder ONLY for a call with neither result nor tool-error', () => {
|
||||
const steps = [
|
||||
{
|
||||
text: '',
|
||||
toolCalls: [
|
||||
{ toolCallId: 'c8', toolName: 'insertNode', input: { node: {} } },
|
||||
],
|
||||
toolResults: [],
|
||||
content: [], // aborted mid-step: no result AND no tool-error
|
||||
},
|
||||
];
|
||||
const parts = assistantParts(steps, '') as AnyPart[];
|
||||
const toolPart = parts.find((p) => p.type === 'tool-insertNode');
|
||||
expect(toolPart!.state).toBe('output-error');
|
||||
expect(toolPart!.errorText).toBe('Tool call did not complete.');
|
||||
});
|
||||
|
||||
it('skips malformed tool-calls (missing toolName or toolCallId)', () => {
|
||||
const steps = [
|
||||
{
|
||||
@@ -195,6 +242,45 @@ describe('serializeSteps', () => {
|
||||
expect(trace[0]).toEqual({ toolName: 'getPage', input: { id: 'p1' } });
|
||||
expect(trace[1]).toEqual({ toolName: 'getPage', output: { title: 'T' } });
|
||||
});
|
||||
|
||||
it('records a THROWN tool failure (tool-error part) with its error message', () => {
|
||||
const trace = serializeSteps([
|
||||
{
|
||||
toolCalls: [{ toolName: 'editPageText', input: { id: 'p1' } }],
|
||||
toolResults: [],
|
||||
content: [
|
||||
{
|
||||
type: 'tool-error',
|
||||
toolName: 'editPageText',
|
||||
error: new Error('page is locked'),
|
||||
},
|
||||
],
|
||||
},
|
||||
]) as Array<Record<string, unknown>>;
|
||||
// The call element is followed by a paired error element (mirroring how a
|
||||
// successful result is appended), so the failure survives in the trace.
|
||||
expect(trace).toHaveLength(2);
|
||||
expect(trace[0]).toEqual({ toolName: 'editPageText', input: { id: 'p1' } });
|
||||
expect(trace[1]).toEqual({
|
||||
toolName: 'editPageText',
|
||||
error: 'page is locked',
|
||||
});
|
||||
});
|
||||
|
||||
it('truncates a very long tool-error message to the tool-output limit', () => {
|
||||
const long = 'x'.repeat(5000);
|
||||
const trace = serializeSteps([
|
||||
{
|
||||
toolCalls: [{ toolName: 'editPageText', input: {} }],
|
||||
toolResults: [],
|
||||
content: [{ type: 'tool-error', toolName: 'editPageText', error: long }],
|
||||
},
|
||||
]) as Array<Record<string, unknown>>;
|
||||
const errorText = trace[1].error as string;
|
||||
// Truncated (not the full 5000 chars) and carries the omission marker.
|
||||
expect(errorText.length).toBeLessThan(long.length);
|
||||
expect(errorText).toContain('chars omitted');
|
||||
});
|
||||
});
|
||||
|
||||
describe('rowToUiMessage', () => {
|
||||
|
||||
@@ -1637,6 +1637,17 @@ type StepLike = {
|
||||
toolName?: string;
|
||||
output?: unknown;
|
||||
}>;
|
||||
// ai@6.0.134: a tool that THREW surfaces as a `tool-error` content part
|
||||
// ({ type:'tool-error', toolCallId, toolName, input, error }), NOT as a
|
||||
// `toolResults` entry (which holds only successes). Read from here so failed
|
||||
// calls are persisted with their real error instead of being dropped.
|
||||
content?: ReadonlyArray<{
|
||||
type?: string;
|
||||
toolCallId?: string;
|
||||
toolName?: string;
|
||||
input?: unknown;
|
||||
error?: unknown;
|
||||
}>;
|
||||
};
|
||||
|
||||
/**
|
||||
@@ -1739,6 +1750,26 @@ function compactValue(value: unknown, depth: number): unknown {
|
||||
return value;
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract a bounded string message from a `tool-error` part's `error` field for
|
||||
* persistence and history replay. The field may be an `Error`, a string, or an
|
||||
* arbitrary object, so pull a message robustly. The result is passed through
|
||||
* `compactValue` so a very long error honors the SAME truncation limits the file
|
||||
* already applies to tool outputs (no new limit is introduced here).
|
||||
*/
|
||||
function normalizeToolError(error: unknown): string {
|
||||
const message =
|
||||
error instanceof Error
|
||||
? error.message
|
||||
: typeof error === 'string'
|
||||
? error
|
||||
: error != null &&
|
||||
typeof (error as { message?: unknown }).message === 'string'
|
||||
? (error as { message: string }).message
|
||||
: String(error);
|
||||
return compactValue(message, 0) as string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Rebuild the FULL UIMessage `parts` for an assistant turn from the SDK steps,
|
||||
* so multi-turn history replays prior tool-calls/results to the model (not just
|
||||
@@ -1771,6 +1802,14 @@ export function assistantParts(
|
||||
for (const r of step.toolResults ?? []) {
|
||||
if (r.toolCallId) resultsById.set(r.toolCallId, r.output);
|
||||
}
|
||||
// Index this step's THROWN tool failures (ai@6 `tool-error` content parts)
|
||||
// by tool call id, so a call that failed replays with its real error text.
|
||||
const errorsById = new Map<string, unknown>();
|
||||
for (const part of step.content ?? []) {
|
||||
if (part.type === 'tool-error' && part.toolCallId) {
|
||||
errorsById.set(part.toolCallId, part.error);
|
||||
}
|
||||
}
|
||||
for (const call of step.toolCalls ?? []) {
|
||||
if (!call.toolName || !call.toolCallId) continue;
|
||||
const hasResult = resultsById.has(call.toolCallId);
|
||||
@@ -1783,9 +1822,21 @@ export function assistantParts(
|
||||
input: call.input,
|
||||
output: compactToolOutput(resultsById.get(call.toolCallId)),
|
||||
});
|
||||
} else if (errorsById.has(call.toolCallId)) {
|
||||
// The tool THREW: replay the REAL error so the model on the next turn
|
||||
// knows WHY the call failed (and does not blindly repeat it). An
|
||||
// output-error round-trips through convertToModelMessages as a balanced
|
||||
// tool-call + tool-result, keeping the rebuilt history valid.
|
||||
parts.push({
|
||||
type: `tool-${call.toolName}`,
|
||||
toolCallId: call.toolCallId,
|
||||
state: 'output-error',
|
||||
input: call.input,
|
||||
errorText: normalizeToolError(errorsById.get(call.toolCallId)),
|
||||
});
|
||||
} else {
|
||||
// No paired result (e.g. aborted mid-step). Persisting a bare
|
||||
// tool-call (input-available) would replay as an unpaired call and
|
||||
// No paired result AND no tool-error (e.g. aborted mid-step). Persisting
|
||||
// a bare tool-call (input-available) would replay as an unpaired call and
|
||||
// throw MissingToolResultsError on the next turn (convertToModelMessages
|
||||
// emits no tool-result for it). Emit a SYNTHETIC paired result instead:
|
||||
// an output-error round-trips through convertToModelMessages as a
|
||||
@@ -2021,10 +2072,19 @@ export function serializeSteps(
|
||||
steps: ReadonlyArray<{
|
||||
toolCalls?: ReadonlyArray<{ toolName?: string; input?: unknown }>;
|
||||
toolResults?: ReadonlyArray<{ toolName?: string; output?: unknown }>;
|
||||
content?: ReadonlyArray<{
|
||||
type?: string;
|
||||
toolName?: string;
|
||||
error?: unknown;
|
||||
}>;
|
||||
}>,
|
||||
): unknown {
|
||||
const calls: Array<{ toolName?: string; input?: unknown; output?: unknown }> =
|
||||
[];
|
||||
const calls: Array<{
|
||||
toolName?: string;
|
||||
input?: unknown;
|
||||
output?: unknown;
|
||||
error?: string;
|
||||
}> = [];
|
||||
for (const step of steps ?? []) {
|
||||
for (const call of step.toolCalls ?? []) {
|
||||
calls.push({ toolName: call.toolName, input: call.input });
|
||||
@@ -2032,6 +2092,18 @@ export function serializeSteps(
|
||||
for (const r of step.toolResults ?? []) {
|
||||
calls.push({ toolName: r.toolName, output: compactToolOutput(r.output) });
|
||||
}
|
||||
// ai@6 surfaces a THROWN tool failure as a `tool-error` content part, NOT as
|
||||
// a `toolResults` entry. Record it as its own paired element (mirroring how a
|
||||
// successful result is appended) so the failure and its reason survive in the
|
||||
// trace instead of leaving an orphaned call with no result.
|
||||
for (const part of step.content ?? []) {
|
||||
if (part.type === 'tool-error') {
|
||||
calls.push({
|
||||
toolName: part.toolName,
|
||||
error: normalizeToolError(part.error),
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
return calls.length > 0 ? calls : null;
|
||||
}
|
||||
|
||||
+77
-39
@@ -13,12 +13,14 @@ Read the **Gotchas** section before you trust any error count.
|
||||
- Agent chats live in Postgres, DB `docmost`, tables `ai_chat_*`.
|
||||
- Each tool invocation is stored as **two** array elements (a `tool-call` part and
|
||||
a `tool-result` part), so naive counting double-counts.
|
||||
- **A tool that *throws* writes no result part at all.** Its error text is nowhere
|
||||
in the DB — not in `tool_calls`, `content`, or `metadata`. It is shown live in
|
||||
the UI only. So `isError` / `success=false` scans under-report by design.
|
||||
- To find where agents fail you need **three** sources: (1) soft-failure markers in
|
||||
`tool_calls`, (2) the orphan-gap proxy for thrown errors, (3) server logs / the
|
||||
live UI for the actual error text.
|
||||
- **A tool that *throws* writes no result part.** Since the #407 fix its error is
|
||||
persisted as a dedicated `{toolName, error}` element in `tool_calls` (queryable +
|
||||
replayed to the model). **Rows written before #407 still drop it** — the error is
|
||||
nowhere in the DB and shows only in the live UI. So `isError` / `success=false`
|
||||
scans under-report by design, and pre-#407 thrown errors are invisible.
|
||||
- To find where agents fail: (1) soft-failure markers in `tool_calls`, (2) the new
|
||||
`error` field for thrown errors (new rows) / the orphan-gap proxy (old rows),
|
||||
(3) server logs / the live UI for full stack traces beyond the truncated message.
|
||||
|
||||
## Where the data lives
|
||||
|
||||
@@ -61,13 +63,17 @@ index 0: { "toolName": "getPage", "input": { "pageId": "…" } } ← tool-ca
|
||||
index 1: { "toolName": "getPage", "output": { … } } ← tool-result (has output, NO input)
|
||||
```
|
||||
|
||||
The **only** keys that ever appear on an element are `toolName`, `input`, `output`.
|
||||
There is no `state`, no `errorText`, no `type`. Consequences:
|
||||
The keys that appear on an element are `toolName`, `input`, `output`, and — for a
|
||||
**thrown** failure on rows written after the #407 fix — `error` (the tool's error
|
||||
message; see the "Hard failures" section below). There is no `state`, no `errorText`,
|
||||
no `type`. On pre-#407 rows a thrown failure has NO paired result element at all
|
||||
(silent orphan). Consequences:
|
||||
|
||||
1. **Real invocation count = elements that have `output`.** Counting every element
|
||||
double-counts (you get ~2× and a spurious "~50% of every tool has no output").
|
||||
2. **Pairing:** a successful call = a `tool-call` part followed by its `tool-result`
|
||||
part. Both carry `toolName`, so you can group by tool on either.
|
||||
1. **Real invocation count = elements that have `output` or `error`.** Counting every
|
||||
element double-counts (you get ~2× and a spurious "~50% of every tool has no output").
|
||||
2. **Pairing:** a call = a `tool-call` part followed by its result part. A success
|
||||
carries `output`; a thrown failure (post-#407) carries `error` instead. Both carry
|
||||
`toolName`, so you can group by tool on either.
|
||||
|
||||
## The two classes of failure (and which the DB can see)
|
||||
|
||||
@@ -85,25 +91,37 @@ These are visible in the `tool-result` `output`. The marker differs per tool:
|
||||
Note `editPageText` returns `failed: []` on success — filtering on the *presence*
|
||||
of the key gives false positives; filter on **non-empty**.
|
||||
|
||||
### 2. Hard failures — tool THREW → NOT PERSISTED ❌ (the trap)
|
||||
### 2. Hard failures — tool THREW → NOW PERSISTED ✅ (since the #407 fix)
|
||||
|
||||
When a tool throws (the classic one is `patchNode` / `insertNode` / `tableUpdateCell`
|
||||
→ `Failed to encode document to Yjs (fromJSON): Unknown node type: undefined`), the
|
||||
runtime writes **no `tool-result` part**. The orphaned `tool-call` part stays, but
|
||||
the error text is **nowhere in the DB**. It is streamed to the UI live and (until
|
||||
rotation) to server logs — that is it.
|
||||
runtime still writes **no `tool-result` part** — the failure is an ai@6 `tool-error`
|
||||
content part instead. **Since the #407 fix, that error is persisted**: `serializeSteps`
|
||||
appends a dedicated element `{toolName, error: "<message>"}` right after the failed
|
||||
call, mirroring how a successful `{toolName, output}` element is appended. So a thrown
|
||||
error now leaves a queryable `error` field carrying its (truncated) reason, and the
|
||||
same real text is replayed to the model on the next turn (an `output-error` part with
|
||||
the real `errorText`, no longer the `'Tool call did not complete.'` placeholder).
|
||||
|
||||
So any query like `count(*) FILTER (WHERE output.success = false)` will happily
|
||||
return **0** for `patchNode` even when the chat is visibly full of red failures.
|
||||
That is survivorship bias, not reliability.
|
||||
**Cutover caveat — old rows keep the old blind shape.** Rows written **before** this
|
||||
change have the two-part shape (`call` + `output` only) and simply **drop** thrown
|
||||
errors, leaving a silent **orphan** (a `call` with no `output` *and* no `error`). Rows
|
||||
written **after** the fix additionally carry the `error` element. So:
|
||||
|
||||
The only DB-side proxy for a thrown error is an **orphan**: a `tool-call` part with
|
||||
no matching `tool-result`. Caveat: orphans also appear when a run is **aborted**
|
||||
mid-flight (server restart), so a high-volume tool (`createComment`, `searchInPage`,
|
||||
`Search_web_search`) shows orphans from aborts, not from real errors. Treat the
|
||||
orphan gap as an *upper bound* on hard errors, and cross-check the tool: a gap on a
|
||||
structural editor (`patchNode`, `insertNode`, `updatePageJson`, `transformPage`) is
|
||||
almost certainly a thrown Yjs-encode error; a gap on `createComment` is mostly aborts.
|
||||
- **New rows:** query the `error` field directly (see the hard-error query below) — no
|
||||
orphan heuristic needed for thrown failures.
|
||||
- **Old rows (pre-#407):** the only DB-side proxy is still an **orphan**: a `tool-call`
|
||||
part with no matching `tool-result` *and* no `error`. Orphans also appear when a run
|
||||
is **aborted** mid-flight (server restart), so a high-volume tool (`createComment`,
|
||||
`searchInPage`, `Search_web_search`) shows orphans from aborts, not real errors on
|
||||
old rows. Treat the orphan gap as an *upper bound*, and cross-check the tool: a gap on
|
||||
a structural editor (`patchNode`, `insertNode`, `updatePageJson`, `transformPage`) is
|
||||
almost certainly a thrown Yjs-encode error; a gap on `createComment` is mostly aborts.
|
||||
|
||||
A note on the aborted-call fallback: a call with **neither** a result **nor** a
|
||||
`tool-error` (genuinely interrupted mid-step) still replays with the
|
||||
`'Tool call did not complete.'` placeholder and persists as an orphan — that path is
|
||||
unchanged, and is distinct from a real thrown error, which now carries `error`.
|
||||
|
||||
### 3. Run-level failures → `ai_chat_runs`
|
||||
|
||||
@@ -164,14 +182,28 @@ WHERE jsonb_typeof(o->'failed') = 'array'
|
||||
GROUP BY 1 ORDER BY 2 DESC;
|
||||
```
|
||||
|
||||
**Hard-error proxy — orphan gap per tool, WITH a spread column** (call parts minus
|
||||
result parts, plus how many distinct chats the gap is spread across):
|
||||
**Hard errors — persisted `error` field per tool (NEW rows, since #407)** — thrown
|
||||
tool failures now carry their real reason, so query them directly:
|
||||
|
||||
```sql
|
||||
SELECT elem->>'toolName' AS tool, count(*) AS thrown_errors,
|
||||
min(elem->>'error') AS sample_error
|
||||
FROM ai_chat_messages m, jsonb_array_elements(m.tool_calls) elem
|
||||
WHERE jsonb_typeof(m.tool_calls) = 'array' AND elem ? 'error'
|
||||
GROUP BY 1 ORDER BY 2 DESC;
|
||||
```
|
||||
|
||||
**Hard-error proxy for OLD rows (pre-#407) — orphan gap per tool, WITH a spread column**
|
||||
(call parts minus result parts, plus how many distinct chats the gap is spread across).
|
||||
This covers rows written before thrown errors were persisted; on new rows a thrown
|
||||
failure now has its own `error` element (use the query above) and an orphan means only
|
||||
a genuinely aborted mid-step call:
|
||||
|
||||
```sql
|
||||
WITH parts AS (
|
||||
SELECT m.chat_id, elem->>'toolName' AS tool,
|
||||
(elem ? 'input' AND NOT (elem ? 'output')) AS is_call,
|
||||
(elem ? 'output') AS is_result
|
||||
(elem ? 'output' OR elem ? 'error') AS is_result
|
||||
FROM ai_chat_messages m, jsonb_array_elements(m.tool_calls) elem
|
||||
WHERE jsonb_typeof(m.tool_calls) = 'array' AND m.role = 'assistant'
|
||||
),
|
||||
@@ -188,8 +220,12 @@ HAVING sum(gap) FILTER (WHERE gap > 0) > 0
|
||||
ORDER BY missing_results DESC;
|
||||
```
|
||||
|
||||
**`missing_results` mixes thrown errors AND aborted/interrupted runs — you cannot
|
||||
split them from `output` alone** (a positional "what follows the orphan" heuristic
|
||||
The `is_result` predicate counts an `error` element as a paired result too, so on new
|
||||
rows a persisted thrown error no longer inflates the orphan gap; a remaining gap is an
|
||||
aborted/interrupted call.
|
||||
|
||||
**On OLD rows, `missing_results` mixes thrown errors AND aborted/interrupted runs — you
|
||||
cannot split them from `output` alone** (a positional "what follows the orphan" heuristic
|
||||
breaks on parallel tool batches, which persist as `call,call,…,result,result`). Use
|
||||
`chats_spread` to disambiguate:
|
||||
|
||||
@@ -244,18 +280,20 @@ docker compose -p gitmost logs -f --tail=100 # whole stack
|
||||
```
|
||||
|
||||
Logging is `json-file`, `max-size=10m max-file=5` → ~50 MB retained, then rotated,
|
||||
and **wiped on container recreate**. So thrown-tool error text is only reliably
|
||||
caught **in real time** (or in the live chat UI, which renders the failed part with
|
||||
its message). There is no durable, queryable store of hard tool errors today — if you
|
||||
need one, that is a feature to add (persist `output-error` parts, or emit a
|
||||
`tool_calls_total{tool,status}` metric to VictoriaMetrics).
|
||||
and **wiped on container recreate**. Since the #407 fix, thrown-tool error text is
|
||||
**persisted in the `error` field** of `tool_calls` (see the hard-error query above), so
|
||||
you no longer depend on live logs for it. Logs/live UI remain useful for **pre-#407
|
||||
rows** (whose thrown errors were dropped) and for full stack traces beyond the
|
||||
truncated stored message. A per-tool `tool_calls_total{tool,status}` metric to
|
||||
VictoriaMetrics is still a possible future add for aggregate dashboards.
|
||||
|
||||
## Gotchas checklist
|
||||
|
||||
- [ ] Counting every `tool_calls` element → **2× overcount**. Count elements with `output`.
|
||||
- [ ] `isError` / `success=false` ≈ 0 does **not** mean "no errors" — thrown errors aren't persisted.
|
||||
- [ ] Counting every `tool_calls` element → **overcount**. Count `output` elements; add `error` elements for thrown failures (new rows), but don't count both as invocations.
|
||||
- [ ] `isError` / `success=false` ≈ 0 does **not** mean "no errors" — thrown errors are a separate `error` element (new rows) or dropped entirely (pre-#407 rows).
|
||||
- [ ] Thrown errors persist only on rows written **after the #407 fix** — pre-#407 rows still drop them (orphan only). Mind the cutover when trending over time.
|
||||
- [ ] `editPageText.failed` is `[]` on success — test for **non-empty**, not presence.
|
||||
- [ ] Orphan gap mixes thrown errors **and** aborted runs — split by tool before concluding.
|
||||
- [ ] Orphan gap on OLD rows mixes thrown errors **and** aborted runs — split by tool. On NEW rows a thrown error is its own `error` element, so a gap ≈ aborted call.
|
||||
- [ ] `aborted` runs = server restarts, `failed` runs = provider overload — not agent mistakes.
|
||||
- [ ] Never dump a raw `tool_calls` cell — it can be hundreds of KB.
|
||||
- [ ] Logs are ephemeral (≤50 MB, wiped on recreate) — grab hard-error text live.
|
||||
|
||||
Reference in New Issue
Block a user