Skip to content

Commit 645895a

Browse files
committed
chore: merge feat/dashboard-agent-flows-watch (review fixes round 3)
2 parents eeabff0 + 02d60a4 commit 645895a

9 files changed

Lines changed: 132 additions & 7 deletions

apps/webapp/app/components/dashboard-agent/DashboardAgentComposer.tsx

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { ArrowUpIcon, StopIcon } from "@heroicons/react/20/solid";
22
import { useEffect, useRef } from "react";
33
import { Button } from "~/components/primitives/Buttons";
44
import { cn } from "~/utils/cn";
5+
import { composerKeepsEscape } from "./composer-escape";
56
import {
67
MAX_MESSAGE_CHARS,
78
MESSAGE_CHARS_WARN_AT,
@@ -95,6 +96,10 @@ export function DashboardAgentComposer({
9596
e.preventDefault();
9697
onSubmit();
9798
}
99+
// Keeping Escape from the panel's close handler, which skips a prevented event.
100+
if (e.key === "Escape" && composerKeepsEscape(value)) {
101+
e.preventDefault();
102+
}
98103
// Only while empty, so with text present Tab keeps its normal focus behavior.
99104
if (e.key === "Tab" && !e.shiftKey && placeholderSuggestion && value === "") {
100105
e.preventDefault();

apps/webapp/app/components/dashboard-agent/DashboardAgentMessages.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ export type DashboardAgentMessagesProps = {
4545

4646
// Cached so a stripped message keeps its identity across renders and memoization holds:
4747
// rebuilding it re-renders every tool-calling turn on each streamed token.
48+
// Relies on @ai-sdk/react cloning a message per update: an SDK mutating one in place would
49+
// keep serving the cached copy of its earlier state.
4850
const strippedMessages = new WeakMap<UIMessage, UIMessage>();
4951

5052
export function stripStepParts(message: UIMessage): UIMessage {

apps/webapp/app/components/dashboard-agent/DashboardAgentPanel.tsx

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,12 @@ import type { AgentPageContext } from "./page-context-types";
3737
import { agentPageLabel } from "./page-label";
3838
import { explicitPromptTarget } from "./explicit-prompt";
3939
import { escapeClosesPanel } from "./panel-escape";
40-
import { markChatListRead, unreadWorkCount } from "./unread-counts";
40+
import {
41+
markChatListRead,
42+
nextVisibleChat,
43+
settleReadChats,
44+
unreadWorkCount,
45+
} from "./unread-counts";
4146
import { AgentPanelColumn } from "./panel-layout";
4247
import { markerAfterActiveChat, markerAfterActivity } from "./thinking-marker";
4348
import { concurrencyPath } from "~/utils/pathBuilder";
@@ -148,6 +153,10 @@ export function DashboardAgentPanel({
148153
// The read POST and its reload can land out of order, so mask the next list.
149154
const justRead = useRef<Set<string>>(new Set());
150155

156+
// Read when the response lands, not when it was requested, so a chat switched to mid-flight
157+
// is the one the list settles against.
158+
const visibleChatId = useRef<string | null>(null);
159+
151160
// Ordering-safe: if the new chat has not reported yet, its own report re-sets the marker.
152161
useEffect(() => {
153162
setThinkingChatId((previous) => markerAfterActiveChat(previous, active?.chatId));
@@ -168,9 +177,7 @@ export function DashboardAgentPanel({
168177
const pending = chats.some((chat) => chat.hasActiveWatch || chat.hasUnreadWake);
169178
if (pending) rememberWatchActivity(organization.id);
170179
else forgetWatchActivity(organization.id);
171-
const settled = chats.map((chat) =>
172-
read.has(chat.id) ? { ...chat, hasUnreadWake: false, hasUnreadWork: false } : chat
173-
);
180+
const settled = settleReadChats(chats, read, visibleChatId.current);
174181
setChats(settled);
175182
setChatsLoaded(true);
176183
} catch (error) {
@@ -320,11 +327,13 @@ export function DashboardAgentPanel({
320327
if (!active?.chatId) return;
321328
const chatId = active.chatId;
322329
onChatRead?.(chatId, { leaving: false });
330+
visibleChatId.current = nextVisibleChat(chatId, { leaving: false });
323331
justRead.current.add(chatId);
324332
setChats((previous) => markChatListRead(previous, chatId));
325333
// Read again on the way out: a wake can land while the chat is open.
326334
return () => {
327335
onChatRead?.(chatId, { leaving: true });
336+
visibleChatId.current = nextVisibleChat(chatId, { leaving: true });
328337
justRead.current.add(chatId);
329338
setChats((previous) => markChatListRead(previous, chatId));
330339
};
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import { describe, expect, it } from "vitest";
2+
import { composerKeepsEscape } from "./composer-escape";
3+
4+
describe("Escape while the composer has focus", () => {
5+
it("is kept by the composer while there is a draft, so the panel stays open", () => {
6+
expect(composerKeepsEscape("half a question about a failing run")).toBe(true);
7+
});
8+
9+
it("closes the panel when there is nothing to lose", () => {
10+
expect(composerKeepsEscape("")).toBe(false);
11+
});
12+
13+
it("reads whitespace as nothing to lose, matching what Send accepts", () => {
14+
expect(composerKeepsEscape(" \n ")).toBe(false);
15+
});
16+
});
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
/**
2+
* The panel closes on Escape unless a child has already prevented the event's default —
3+
* `defaultPrevented` is how a child vetoes the close. A composer holding a draft takes the
4+
* first Escape for itself, so the draft survives; an empty one lets Escape close the panel.
5+
*/
6+
export function composerKeepsEscape(value: string): boolean {
7+
return value.trim() !== "";
8+
}

apps/webapp/app/components/dashboard-agent/settled-transcript.test.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,36 @@ describe("an already-open panel when a turn is exhausted", () => {
239239
expect(reads).toBe(2);
240240
});
241241

242+
it("keeps re-reading a stream that died mid-tool with no card open", async () => {
243+
// No investigation anywhere: only the dangling `get_report` says the turn is unfinished.
244+
const DANGLING = {
245+
id: "msg_step",
246+
role: "assistant",
247+
parts: [{ type: "tool-get_report", toolCallId: "call_1", state: "input-available" }],
248+
};
249+
const FINISHED = {
250+
id: "msg_step",
251+
role: "assistant",
252+
parts: [
253+
{ type: "tool-get_report", toolCallId: "call_1", state: "output-available", output: {} },
254+
],
255+
};
256+
257+
const responses = [[DANGLING], [DANGLING], [FINISHED]];
258+
let rendered: (typeof DANGLING)[] = [DANGLING];
259+
let reads = 0;
260+
261+
await pollSettledTranscript({
262+
fetchTranscript: async () => responses[reads++] ?? null,
263+
apply: (merge) => void (rendered = merge(rendered)),
264+
wait: async () => {},
265+
});
266+
267+
expect(reads).toBe(3);
268+
expect(rendered).toEqual([FINISHED]);
269+
expect(transcriptLooksUnfinished(rendered)).toBe(false);
270+
});
271+
242272
it("stops on a failed re-read instead of hammering the endpoint", async () => {
243273
let reads = 0;
244274
await pollSettledTranscript<typeof OPEN>({

apps/webapp/app/components/dashboard-agent/settled-transcript.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,9 @@ export async function pollSettledTranscript<T extends Identified>(deps: {
8686
const fetched = await deps.fetchTranscript();
8787
if (!fetched) return;
8888
deps.apply((current) => mergeSettledMessages(current, fetched));
89-
// The stored transcript is the authority on whether anything is still open.
90-
if (!hasOpenInvestigation(fetched)) return;
89+
// The stored transcript is the authority on whether anything is still open. Same test that
90+
// starts the poll, so a stream that died mid-tool is followed until it settles too.
91+
if (!transcriptLooksUnfinished(fetched)) return;
9192
}
9293
}
9394

apps/webapp/app/components/dashboard-agent/unread-counts.test.ts

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
import { readFileSync } from "node:fs";
22
import { describe, expect, it } from "vitest";
3-
import { markChatListRead, nextVisibleChat, unreadWorkCount } from "./unread-counts";
3+
import {
4+
markChatListRead,
5+
nextVisibleChat,
6+
settleReadChats,
7+
unreadWorkCount,
8+
} from "./unread-counts";
49

510
const list = () => [
611
{ id: "chat_a", hasUnreadWake: true, hasUnreadWork: true },
@@ -71,6 +76,28 @@ describe("the chat on screen", () => {
7176
});
7277
});
7378

79+
/**
80+
* The list is refreshed after every turn, and the server's lastReadAt trails the turn that just
81+
* landed — so the chat being read has to settle on its own, not wait for the next read to land.
82+
*/
83+
describe("settleReadChats", () => {
84+
it("settles the chat on screen, however fresh the turn that just landed in it", () => {
85+
const settled = settleReadChats(list(), new Set(), "chat_a");
86+
expect(settled[0]).toEqual({ id: "chat_a", hasUnreadWake: false, hasUnreadWork: false });
87+
expect(settled.slice(1)).toEqual(list().slice(1));
88+
});
89+
90+
it("settles the chats just read", () => {
91+
const settled = settleReadChats(list(), new Set(["chat_b"]), null);
92+
expect(settled[1]).toEqual({ id: "chat_b", hasUnreadWake: false, hasUnreadWork: false });
93+
expect(unreadWorkCount(settled)).toBe(1);
94+
});
95+
96+
it("leaves every other chat exactly as the server reported it", () => {
97+
expect(settleReadChats(list(), new Set(), null)).toEqual(list());
98+
});
99+
});
100+
74101
describe("what the panel and the layout actually do with it", () => {
75102
const panel = readFileSync(new URL("./DashboardAgentPanel.tsx", import.meta.url), "utf8");
76103
const layout = readFileSync(new URL("./DashboardAgent.tsx", import.meta.url), "utf8");
@@ -87,6 +114,16 @@ describe("what the panel and the layout actually do with it", () => {
87114
expect(layout).toContain("visibleChat.current = nextVisibleChat(chatId, options);");
88115
});
89116

117+
/**
118+
* Structural: the reload is memoised without `active`, so the chat on screen has to reach the
119+
* settle through a ref — the closure's copy is whatever it was when the reload was created.
120+
*/
121+
it("settles the refreshed list against the chat on screen, read from a ref", () => {
122+
expect(panel).toContain("settleReadChats(chats, read, visibleChatId.current)");
123+
expect(panel).toContain("visibleChatId.current = nextVisibleChat(chatId, { leaving: false });");
124+
expect(panel).toContain("visibleChatId.current = nextVisibleChat(chatId, { leaving: true });");
125+
});
126+
90127
/**
91128
* Structural: there is no DOM here to open a panel in. The poll runs for as long as this tab
92129
* is watching, so the chat on screen has to be read from a ref at request time — `open` in the

apps/webapp/app/components/dashboard-agent/unread-counts.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,23 @@ export function markChatListRead<T extends UnreadChat>(chats: T[], chatId: strin
2424
);
2525
}
2626

27+
/**
28+
* The list as the panel renders it. The chat on screen settles alongside the ones just read:
29+
* the server's lastReadAt still trails the turn that just landed in it, so a refresh would
30+
* otherwise mark the chat its owner is reading right now as unread.
31+
*/
32+
export function settleReadChats<T extends UnreadChat>(
33+
chats: T[],
34+
read: Set<string>,
35+
visibleChatId: string | null
36+
): T[] {
37+
return chats.map((chat) =>
38+
read.has(chat.id) || chat.id === visibleChatId
39+
? { ...chat, hasUnreadWake: false, hasUnreadWork: false }
40+
: chat
41+
);
42+
}
43+
2744
/**
2845
* How many chats still hold work their owner hasn't seen. The chat on screen is being read
2946
* right now, so a turn landing in it is not work anyone is waiting on — every count of this,

0 commit comments

Comments
 (0)