mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix(site/src/pages/AgentsPage/components/Sidebar): keep subtitle in sync during streaming lifecycle (#25144)
The Agents sidebar shows the per-chat turn-end summary as a subtitle,
but the cached `last_turn_summary` is not always in sync with the live
status:
- **Resuming a turn**: when a chat goes back to `running` or `pending`,
the sidebar would keep displaying the previous turn-end label (e.g.,
"Chat is idle") while the agent is already streaming again.
- **Stream end → next summary**: status flips to `waiting`
synchronously, but the new `last_turn_summary` is generated by an async
finalizer that calls the model. For a beat, the row still carries the
previous turn's summary text, which would briefly flash before the new
text arrives.
This PR addresses both:
1. Override the subtitle with `{model} streaming…` while `chat.status`
is `running` or `pending`, matching the same `isStreaming` definition
`ChatPageContent` uses. Errors still take precedence so failure context
is not hidden.
2. Capture the cached summary observed during streaming and suppress an
exact match on the post-stream renders until the new summary lands.
State is adjusted during render so the first post-stream paint already
hides the stale string instead of flashing it for a frame. A 10s timeout
safety net releases the suppression in the corner case where the
regenerated summary equals the previous one byte-for-byte.
<details>
<summary>Decision log</summary>
- **Frontend-only fix**: the server intentionally does not bump
`updated_at` when it writes the new `last_turn_summary` (see
`UpdateChatLastTurnSummary` in `coderd/database/queries/chats.sql`), and
SSE delivers the status flip ahead of the new label. The sidebar knows
the live status, so the rendering layer is the right place to mask the
inconsistency.
- **`isStreaming = running || pending`** mirrors `ChatPageContent.tsx`
so subagents and queued continuations also flip to the live label.
- **State, not refs**, for the suppression bookkeeping: the React
Compiler rejects ref writes during render, and React's "adjust state
during render" pattern is purpose-built for storing information from
previous renders (state setters in render bail out and re-render
synchronously before paint).
- **10s timeout** is a safety net for the rare byte-for-byte equality
case; the equality-based release handles every other shape of update.
- Added Storybook stories covering both fixes
(`ChatStreamingOverridesTurnSummary` and
`StaleTurnSummaryAfterStreamingIsSuppressed`). All existing legacy
subtitle stories still pass because they use terminal statuses.
</details>
> This PR was created by Coder Agents on behalf of @ibetitsmike.
This commit is contained in:
@@ -1,4 +1,5 @@
|
||||
import type { Meta, StoryObj } from "@storybook/react-vite";
|
||||
import { useEffect, useState } from "react";
|
||||
import { useLocation } from "react-router";
|
||||
import { expect, fn, userEvent, waitFor, within } from "storybook/test";
|
||||
import { reactRouterParameters } from "storybook-addon-remix-react-router";
|
||||
@@ -145,6 +146,122 @@ export const ChatWithTurnSummary: Story = {
|
||||
},
|
||||
};
|
||||
|
||||
/**
|
||||
* While the chat is streaming again the cached last_turn_summary still
|
||||
* holds the previous turn's text. The sidebar replaces it with a live
|
||||
* "{model} streaming…" label so the status does not look stuck.
|
||||
*/
|
||||
export const ChatStreamingOverridesTurnSummary: Story = {
|
||||
args: {
|
||||
chats: [
|
||||
buildChat({
|
||||
id: "chat-streaming-running",
|
||||
title: "Update workspace template",
|
||||
status: "running",
|
||||
last_turn_summary: "Added Docker and Terraform validation",
|
||||
}),
|
||||
buildChat({
|
||||
id: "chat-streaming-pending",
|
||||
title: "Queued continuation",
|
||||
status: "pending",
|
||||
last_turn_summary: "Added Docker and Terraform validation",
|
||||
}),
|
||||
],
|
||||
},
|
||||
play: async ({ canvasElement }) => {
|
||||
const canvas = within(canvasElement);
|
||||
|
||||
await expect(canvas.getAllByText("GPT-4o streaming…")).toHaveLength(2);
|
||||
expect(
|
||||
canvas.queryByText("Added Docker and Terraform validation"),
|
||||
).not.toBeInTheDocument();
|
||||
},
|
||||
};
|
||||
|
||||
/**
|
||||
* After streaming ends the server flips status to "waiting" synchronously,
|
||||
* but the new last_turn_summary is generated asynchronously and arrives a
|
||||
* split second later. The previous turn's text would otherwise flash for
|
||||
* that beat. The row remembers the summary observed while streaming and
|
||||
* suppresses it on the post-stream render until the new summary lands.
|
||||
*/
|
||||
export const StaleTurnSummaryAfterStreamingIsSuppressed: Story = {
|
||||
parameters: {
|
||||
layout: "fullscreen",
|
||||
user: MockUserOwner,
|
||||
reactRouter: reactRouterParameters({
|
||||
location: { path: "/agents" },
|
||||
routing: agentsRouting,
|
||||
}),
|
||||
chromatic: { disableSnapshot: true },
|
||||
},
|
||||
render: (args) => {
|
||||
const initialSummary = "Added Docker and Terraform validation";
|
||||
const freshSummary = "Validated provider configs and exited cleanly";
|
||||
const [phase, setPhase] = useState<
|
||||
"streaming" | "stale-after-stream" | "fresh"
|
||||
>("streaming");
|
||||
useEffect(() => {
|
||||
if (phase !== "streaming") {
|
||||
return;
|
||||
}
|
||||
const id = setTimeout(() => setPhase("stale-after-stream"), 50);
|
||||
return () => clearTimeout(id);
|
||||
}, [phase]);
|
||||
const chats = [
|
||||
buildChat({
|
||||
id: "chat-stale-summary",
|
||||
title: "Update workspace template",
|
||||
status: phase === "streaming" ? "running" : "waiting",
|
||||
last_turn_summary: phase === "fresh" ? freshSummary : initialSummary,
|
||||
}),
|
||||
];
|
||||
return (
|
||||
<div data-testid="flicker-harness" data-phase={phase}>
|
||||
<button
|
||||
data-testid="advance-to-fresh"
|
||||
type="button"
|
||||
onClick={() => setPhase("fresh")}
|
||||
>
|
||||
advance
|
||||
</button>
|
||||
<AgentsSidebar {...args} chats={chats} />
|
||||
</div>
|
||||
);
|
||||
},
|
||||
play: async ({ canvasElement }) => {
|
||||
const canvas = within(canvasElement);
|
||||
|
||||
// Phase 1: chat is streaming, the cached summary is suppressed in
|
||||
// favor of the live streaming label.
|
||||
await expect(canvas.getByText("GPT-4o streaming…")).toBeInTheDocument();
|
||||
|
||||
// Phase 2: status flips to waiting while the previous summary is
|
||||
// still cached server-side. The stale text must not flash; the
|
||||
// fallback (model name) shows instead.
|
||||
await waitFor(() => {
|
||||
expect(canvas.getByTestId("flicker-harness")).toHaveAttribute(
|
||||
"data-phase",
|
||||
"stale-after-stream",
|
||||
);
|
||||
});
|
||||
await expect(
|
||||
canvas.queryByText("GPT-4o streaming…"),
|
||||
).not.toBeInTheDocument();
|
||||
await expect(
|
||||
canvas.queryByText("Added Docker and Terraform validation"),
|
||||
).not.toBeInTheDocument();
|
||||
await expect(canvas.getByText("GPT-4o")).toBeInTheDocument();
|
||||
|
||||
// Phase 3: the async finalizer updates the summary. The new text
|
||||
// is displayed, and the stale-suppression guard is cleared.
|
||||
await userEvent.click(canvas.getByTestId("advance-to-fresh"));
|
||||
await expect(
|
||||
canvas.getByText("Validated provider configs and exited cleanly"),
|
||||
).toBeInTheDocument();
|
||||
},
|
||||
};
|
||||
|
||||
export const ChatWithTurnSummaryAndError: Story = {
|
||||
args: {
|
||||
chats: [
|
||||
|
||||
@@ -500,7 +500,61 @@ const ChatTreeNode: FC<ChatTreeNodeProps> = ({ chat, isChildNode }) => {
|
||||
? chatErrorReasons[chat.id] || chat.last_error?.message || undefined
|
||||
: undefined;
|
||||
const lastTurnSummary = asNonEmptyString(chat.last_turn_summary);
|
||||
const subtitle = errorReason || lastTurnSummary || modelName;
|
||||
// While a turn is in flight the cached last_turn_summary still holds
|
||||
// the previous turn's text. Surface a live "{model} streaming…" label
|
||||
// instead so the sidebar does not look stuck on the old status.
|
||||
const isStreaming = chat.status === "running" || chat.status === "pending";
|
||||
const streamingSubtitle = isStreaming ? `${modelName} streaming…` : undefined;
|
||||
// Server-side, status flips to "waiting" synchronously when streaming
|
||||
// ends, but the new last_turn_summary is written by an async finalizer
|
||||
// (it calls the model to generate a label). For a beat after the stream
|
||||
// stops, the chat row still carries the previous turn's summary text.
|
||||
// Suppress that briefly-stale text: remember the cached summary observed
|
||||
// while streaming, then hide an exact match on the post-stream renders
|
||||
// until the new summary lands. A timeout-based release covers the rare
|
||||
// case where the regenerated summary equals the previous one byte-for-
|
||||
// byte (so the equality-based release would never fire). State is
|
||||
// captured during render using the React "adjust state during render"
|
||||
// pattern so the first post-stream paint already suppresses the stale
|
||||
// string instead of flashing it for a frame.
|
||||
const staleTurnSummaryReleaseMs = 10_000;
|
||||
const [streamingSummary, setStreamingSummary] = useState<string | undefined>(
|
||||
isStreaming ? lastTurnSummary : undefined,
|
||||
);
|
||||
const [suppressionExpired, setSuppressionExpired] = useState(false);
|
||||
if (isStreaming) {
|
||||
if (streamingSummary !== lastTurnSummary) {
|
||||
setStreamingSummary(lastTurnSummary);
|
||||
}
|
||||
if (suppressionExpired) {
|
||||
setSuppressionExpired(false);
|
||||
}
|
||||
} else if (
|
||||
streamingSummary !== undefined &&
|
||||
lastTurnSummary !== streamingSummary
|
||||
) {
|
||||
setStreamingSummary(undefined);
|
||||
if (suppressionExpired) {
|
||||
setSuppressionExpired(false);
|
||||
}
|
||||
}
|
||||
const isStaleTurnSummary =
|
||||
!isStreaming &&
|
||||
lastTurnSummary !== undefined &&
|
||||
!suppressionExpired &&
|
||||
streamingSummary === lastTurnSummary;
|
||||
useEffect(() => {
|
||||
if (!isStaleTurnSummary) {
|
||||
return;
|
||||
}
|
||||
const timeoutId = window.setTimeout(() => {
|
||||
setSuppressionExpired(true);
|
||||
}, staleTurnSummaryReleaseMs);
|
||||
return () => window.clearTimeout(timeoutId);
|
||||
}, [isStaleTurnSummary]);
|
||||
const displayedTurnSummary = isStaleTurnSummary ? undefined : lastTurnSummary;
|
||||
const subtitle =
|
||||
errorReason || streamingSubtitle || displayedTurnSummary || modelName;
|
||||
const diffStatus = getChatDiffStatus(chat);
|
||||
const baseConfig = getStatusConfig(chat.status);
|
||||
const prConfig =
|
||||
|
||||
Reference in New Issue
Block a user