Skip to content

Commit 60a0c4c

Browse files
committed
fix(webapp,dashboard-agent-db): settle read state, the dot and unavailable checks
- 0003 catches the last_read_at backfill up on databases where 0002 already ran. - The per-watch check endpoint records a look, not a check, when it read nothing. - A suggested prompt is consumed once it is sent, not when it is clicked. - Closing the panel settles the launcher dot instead of waiting for the poll. - Say why the oldest-age reader's 50-key page cannot under-report. - Suppress error-classification on the check route, with the reason on the record.
1 parent 896ed48 commit 60a0c4c

13 files changed

Lines changed: 1561 additions & 22 deletions

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

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import {
2222
writeAgentFullscreen,
2323
} from "./panel-layout";
2424
import { nextPendingTurnChatId } from "./pending-turn";
25-
import { nextVisibleChat, unreadWorkForDot } from "./unread-counts";
25+
import { nextVisibleChat, unreadWorkForDot, unreadWorkOnPanelClose } from "./unread-counts";
2626
import { startWakePolling, wakesToToast } from "./wake-poll";
2727
import { shouldPollWakeFeed, subscribeWatchActivity } from "./watch-activity";
2828
import {
@@ -146,10 +146,22 @@ export function DashboardAgent({
146146
undefined
147147
);
148148

149+
// The panel's own count, off the chat list it has already marked read. Kept so the closing
150+
// edge can settle the dot instead of waiting a poll for the open-chat subtraction to lift.
151+
const panelWorkCount = useRef<number | null>(null);
152+
const handleUnreadWorkChange = useCallback((count: number) => {
153+
panelWorkCount.current = count;
154+
setUnreadWork(count);
155+
}, []);
156+
149157
const setPanelOpen = useCallback((next: boolean) => {
150158
setOpen(next);
151159
// Pending requests must be dropped or a stale one re-applies on the next open.
152160
if (!next) {
161+
setUnreadWork((shown) =>
162+
unreadWorkOnPanelClose({ shown, panelCount: panelWorkCount.current })
163+
);
164+
panelWorkCount.current = null;
153165
visibleChat.current = null;
154166
setFullscreen(false);
155167
writeAgentFullscreen(false);
@@ -345,7 +357,7 @@ export function DashboardAgent({
345357
newChatSeq={newChatSeq}
346358
promotedPrompt={promotedPrompt}
347359
onChatRead={markChatRead}
348-
onUnreadWorkChange={setUnreadWork}
360+
onUnreadWorkChange={handleUnreadWorkChange}
349361
onTurnActivityChange={handleTurnActivityChange}
350362
isFullscreen={fullscreen}
351363
onToggleFullscreen={toggleFullscreen}

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

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import {
2828
pollSettledTranscript,
2929
} from "./settled-transcript";
3030
import { takeNavigateIntent } from "./turn-navigation";
31+
import { sendRequestOutcome } from "./send-request";
3132
import { teardownCancelsTurn, unmountTeardown } from "./turn-teardown";
3233
import { useAgentMessageQuota } from "./useAgentMessageQuota";
3334
import { useTriggerUriResolver } from "./useTriggerUriResolver";
@@ -226,13 +227,21 @@ export function DashboardAgentChat({
226227
[isStreaming, atMessageCap, sendMessage]
227228
);
228229

229-
// The panel only sends when the chat can take it, so this never lands mid-turn.
230+
// The panel only sends when the chat can take it, so this never lands mid-turn. The cap it
231+
// cannot see is why the request is held rather than consumed on sight.
230232
const sentRequestSeq = useRef<number | undefined>(undefined);
233+
const canSend = !isStreaming && !atMessageCap;
231234
useEffect(() => {
232-
if (!sendRequest || sentRequestSeq.current === sendRequest.seq) return;
235+
if (!sendRequest) return;
236+
const outcome = sendRequestOutcome({
237+
requestSeq: sendRequest.seq,
238+
consumedSeq: sentRequestSeq.current,
239+
canSend,
240+
});
241+
if (outcome !== "send") return;
233242
sentRequestSeq.current = sendRequest.seq;
234243
submit(sendRequest.text);
235-
}, [sendRequest, submit]);
244+
}, [sendRequest, submit, canSend]);
236245

237246
const retry = useCallback(() => {
238247
// A watch's consent record is a user message nobody typed, so retry never treats it as one.
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import { describe, expect, it } from "vitest";
2+
import { sendRequestOutcome } from "./send-request";
3+
4+
describe("sendRequestOutcome", () => {
5+
it("sends a request the chat can take", () => {
6+
expect(sendRequestOutcome({ requestSeq: 1, consumedSeq: undefined, canSend: true })).toBe(
7+
"send"
8+
);
9+
});
10+
11+
it("skips a request it has already sent", () => {
12+
expect(sendRequestOutcome({ requestSeq: 1, consumedSeq: 1, canSend: true })).toBe("skip");
13+
});
14+
15+
it("skips when nothing was asked for", () => {
16+
expect(sendRequestOutcome({ requestSeq: undefined, consumedSeq: 3, canSend: true })).toBe(
17+
"skip"
18+
);
19+
});
20+
21+
it("holds a request the chat can't take yet, and sends it once it can", () => {
22+
expect(sendRequestOutcome({ requestSeq: 2, consumedSeq: 1, canSend: false })).toBe("hold");
23+
// The held click is still the same request: nothing consumed it while it waited.
24+
expect(sendRequestOutcome({ requestSeq: 2, consumedSeq: 1, canSend: true })).toBe("send");
25+
});
26+
});
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
/** What to do with a prompt the panel asked the chat to send. */
2+
export type SendRequestOutcome = "send" | "hold" | "skip";
3+
4+
/**
5+
* A click on a suggested prompt is consumed only once it has actually been sent. Marking it
6+
* consumed first loses the click whenever the chat can't take it yet — the message cap being
7+
* the case that has no other way back in — so an unsendable request is held for the next render.
8+
*/
9+
export function sendRequestOutcome(params: {
10+
requestSeq: number | undefined;
11+
consumedSeq: number | undefined;
12+
canSend: boolean;
13+
}): SendRequestOutcome {
14+
if (params.requestSeq === undefined) return "skip";
15+
if (params.requestSeq === params.consumedSeq) return "skip";
16+
return params.canSend ? "send" : "hold";
17+
}

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
nextVisibleChat,
66
unreadWorkCount,
77
unreadWorkForDot,
8+
unreadWorkOnPanelClose,
89
} from "./unread-counts";
910

1011
const list = () => [
@@ -79,6 +80,27 @@ describe("unreadWorkForDot", () => {
7980
});
8081
});
8182

83+
/**
84+
* Closing the panel stops the chat on screen being read, so the one the open panel took off the
85+
* count comes straight back rather than a poll later.
86+
*/
87+
describe("unreadWorkOnPanelClose", () => {
88+
it("settles the dot on the panel's own count", () => {
89+
const open = unreadWorkForDot({ reported: 1, panelOpen: true, visibleChatId: "chat_a" });
90+
expect(open).toBe(0);
91+
// Work landed in another chat while chat_a was on screen: the panel's list still counts it.
92+
expect(unreadWorkOnPanelClose({ shown: open, panelCount: 1 })).toBe(1);
93+
});
94+
95+
it("leaves the shown count alone when the panel closed before its list loaded", () => {
96+
expect(unreadWorkOnPanelClose({ shown: 2, panelCount: null })).toBe(2);
97+
});
98+
99+
it("takes the panel's zero too, so reading the last chat darkens the dot at once", () => {
100+
expect(unreadWorkOnPanelClose({ shown: 1, panelCount: 0 })).toBe(0);
101+
});
102+
});
103+
82104
describe("what the panel and the layout actually do with it", () => {
83105
const panel = readFileSync(new URL("./DashboardAgentPanel.tsx", import.meta.url), "utf8");
84106
const layout = readFileSync(new URL("./DashboardAgent.tsx", import.meta.url), "utf8");
@@ -105,6 +127,15 @@ describe("what the panel and the layout actually do with it", () => {
105127
expect(layout).not.toContain("open && visibleChat.current");
106128
});
107129

130+
/** Structural: there is no DOM here to close a real panel in. */
131+
it("settles the count on the closing edge instead of waiting for the poll", () => {
132+
expect(panel).toContain("onUnreadWorkChange?.(unreadWorkCount(chats));");
133+
expect(layout).toContain("panelWorkCount.current = count;");
134+
expect(layout).toContain(
135+
"unreadWorkOnPanelClose({ shown, panelCount: panelWorkCount.current })"
136+
);
137+
});
138+
108139
it("re-seeds both counts when the environment changes under the layout", () => {
109140
expect(layout).toContain("seededEnvironment.current = environment.id;");
110141
expect(layout).toContain("setUnreadWakes(initialUnreadWakes);");

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,20 @@ export function unreadWorkForDot(params: {
3333
return Math.max(0, (params.reported ?? 0) - onScreen);
3434
}
3535

36+
/**
37+
* The count the dot settles on the moment the panel closes. While the panel is open the poll
38+
* subtracts the chat on screen, and it only corrects itself a tick later — up to a minute of a
39+
* dark dot over work nobody has seen. The panel's own count is taken off the chat list it has
40+
* already marked read, so it settles the closing edge without waiting. `null` is a panel that
41+
* closed before its list loaded, which leaves the shown count alone.
42+
*/
43+
export function unreadWorkOnPanelClose(params: {
44+
shown: number;
45+
panelCount: number | null;
46+
}): number {
47+
return params.panelCount ?? params.shown;
48+
}
49+
3650
/** Opening a chat settles everything unseen in it, not just the wake. */
3751
export function markChatListRead<T extends UnreadChat>(chats: T[], chatId: string): T[] {
3852
return chats.map((chat) =>

apps/webapp/app/routes/api.v1.dashboard-agent.watches.$watchId.check.ts

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
import { json, type ActionFunctionArgs } from "@remix-run/server-runtime";
2-
import { cancelWatch, getWatch, recordWatchCheck } from "@internal/dashboard-agent-db";
2+
import {
3+
cancelWatch,
4+
getWatch,
5+
recordWatchAttempt,
6+
recordWatchCheck,
7+
} from "@internal/dashboard-agent-db";
38
import { z } from "zod";
49
import { dashboardAgentDb } from "~/services/dashboardAgentDb.server";
510
import { logger } from "~/services/logger.server";
@@ -22,6 +27,8 @@ import {
2227

2328
const ParamsSchema = z.object({ watchId: z.string().min(1) });
2429

30+
// obs-map-disable error-classification -- arming the chain is best-effort: every error means the same thing, retry next check
31+
2532
/** Best-effort: a chain that couldn't be armed returns `false` and is retried next check. */
2633
async function ensureBatchChain(watch: {
2734
id: string;
@@ -156,17 +163,23 @@ export async function action({ request, params }: ActionFunctionArgs) {
156163
})
157164
);
158165

159-
// Recorded even on the final evaluation. Guarded on `active`, so a concurrent
160-
// fire/expire wins and this no-ops.
161-
await recordWatchCheck(dashboardAgentDb, {
162-
id: watchId,
163-
lastResult: {
164-
result: outcome.result,
165-
facts: outcome.facts,
166-
observed: outcome.observed,
167-
final: body.final === true,
168-
},
169-
});
166+
// Only a real evaluation is recorded, final or not: `unavailable` means nothing was read,
167+
// so writing it would move `lastCheckedAt` and overwrite the facts a streak lives in.
168+
// Guarded on `active`, so a concurrent fire/expire wins and this no-ops.
169+
if (outcome.result !== "unavailable") {
170+
await recordWatchCheck(dashboardAgentDb, {
171+
id: watchId,
172+
lastResult: {
173+
result: outcome.result,
174+
facts: outcome.facts,
175+
observed: outcome.observed,
176+
final: body.final === true,
177+
},
178+
});
179+
} else {
180+
// Looked at, not checked: the fairness key moves and nothing else does.
181+
await recordWatchAttempt(dashboardAgentDb, { id: watchId });
182+
}
170183

171184
const batched = await ensureBatchChain(watch);
172185

apps/webapp/app/services/dashboardAgentWatchChecks.server.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,11 @@ export async function readWatchQueueOldestAge(
169169
return { ageMs, source: "live_queue", current: true, asOf: now };
170170
}
171171

172-
/** Same cap the queue detail page reads keys with. */
172+
/**
173+
* Same cap the queue detail page reads keys with. It cannot under-report the wait: the ckIndex
174+
* is scored by each key's oldest enqueue time and read ascending, so the oldest key is the first
175+
* of the page whatever the cardinality.
176+
*/
173177
const OLDEST_AGE_CK_LIMIT = 50;
174178

175179
const MINUTE_MS = 60_000;

apps/webapp/test/dashboardAgentLastReadBackfill.test.ts

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,11 @@ import { afterEach, describe, expect } from "vitest";
1212

1313
/**
1414
* `chats.last_read_at` is nullable and every reader treats NULL as unread, so without a
15-
* backfill the first load after rollout reports every pre-existing chat unread. Migration
16-
* 0002 backfills it; this replays the migrations against a real Postgres to prove it does.
15+
* backfill the first load after rollout reports every pre-existing chat unread. Migrations
16+
* 0002 and 0003 backfill it; this replays them against a real Postgres to prove they do.
17+
*
18+
* 0003 is the one that reaches a database where 0002 already ran, which is every database
19+
* the column landed on before the backfill statement was appended to 0002.
1720
*/
1821

1922
const DRIZZLE = path.resolve(__dirname, "../../../internal-packages/dashboard-agent-db/drizzle");
@@ -22,6 +25,7 @@ const MIGRATIONS = [
2225
"0000_magenta_lilandra.sql",
2326
"0001_slimy_living_tribunal.sql",
2427
"0002_watches_and_chat_messages.sql",
28+
"0003_backfill_chat_last_read_at.sql",
2529
];
2630

2731
/** The statement under test, located by shape so removing it fails rather than silently passing. */
@@ -135,3 +139,46 @@ describe("the last_read_at backfill in migration 0002", () => {
135139
}
136140
);
137141
});
142+
143+
describe("the last_read_at catch-up in migration 0003", () => {
144+
postgresTest(
145+
"starts pre-existing chats read on a database that already ran 0002",
146+
async ({ prisma, postgresContainer }) => {
147+
await run(prisma, statementsOf(MIGRATIONS[0]!));
148+
await run(prisma, statementsOf(MIGRATIONS[1]!));
149+
150+
// 0002 as it was already applied everywhere: the column, without the backfill that
151+
// was appended to it later. Re-running 0002 there is impossible — its hash is spent.
152+
const applied = statementsOf(MIGRATIONS[2]!).filter((statement) => !BACKFILL.test(statement));
153+
await run(prisma, applied);
154+
await seedPreExistingChats(prisma);
155+
await prisma.$executeRawUnsafe(
156+
`update "trigger_dashboard_agent"."chats" set "last_read_at" = $1 where "id" = 'chat_already_read'`,
157+
ALREADY_READ_AT
158+
);
159+
160+
agentDbClient = createDashboardAgentDb(postgresContainer.getConnectionUri(), { max: 2 });
161+
const agentDb: DashboardAgentDb = agentDbClient.db;
162+
163+
// The bug 0003 exists for: every chat that predates the column reads as unread.
164+
expect(await countChatsWithUnreadWork(agentDb, SCOPE)).toBe(1);
165+
166+
const catchUp = statementsOf(MIGRATIONS[3]!);
167+
expect(catchUp.some((statement) => BACKFILL.test(statement))).toBe(true);
168+
await run(prisma, catchUp);
169+
170+
expect(await readLastReadAt(prisma)).toEqual({
171+
chat_with_messages: LAST_MESSAGE_AT,
172+
chat_never_messaged: CREATED_AT,
173+
chat_already_read: ALREADY_READ_AT,
174+
});
175+
expect(await countChatsWithUnreadWork(agentDb, SCOPE)).toBe(0);
176+
177+
// Still a dot for work that lands after the catch-up.
178+
await prisma.$executeRawUnsafe(
179+
`update "trigger_dashboard_agent"."chats" set "last_message_at" = now() where "id" = 'chat_with_messages'`
180+
);
181+
expect(await countChatsWithUnreadWork(agentDb, SCOPE)).toBe(1);
182+
}
183+
);
184+
});

0 commit comments

Comments
 (0)