From 707c8c5b2d26a073ce6a756447b4bfd52d150e52 Mon Sep 17 00:00:00 2001 From: "claude-ceo-assistant (Claude Opus 4.7 on Hongming's MacBook)" Date: Wed, 6 May 2026 17:00:23 -0700 Subject: [PATCH] chore(canvas/chat): delete historyHydration.ts now that PR-C-2 migrated callers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stacked on feat/rfc-2945-pr-c-2-canvas-chat-history. After that PR merges, this rebases cleanly onto staging. PR-C-2 (RFC #2945) migrated canvas's loadMessagesFromDB from /activity to the typed /chat-history endpoint. The TS-side activity-row mapping helpers in historyHydration.ts (activityRowToMessages, ActivityRowForHydration) had exactly one consumer — ChatTab's loadMessagesFromDB — and PR-C-2 removed that consumer. Verified before deletion: - grep across canvas/src for historyHydration / activityRowToMessages / ActivityRowForHydration: only the file itself and its test. - canvas/src/components/tabs/chat/index.ts barrel: does NOT re-export historyHydration symbols. Only message-parser.ts is re-exported (extractAgentText, extractTextsFromParts, extractResponseText), which is independent. - message-parser.ts (the parsers historyHydration.ts wraps) STAYS because it's still load-bearing for live A2A WebSocket messages in ChatTab.tsx (line ~795), AgentCommsPanel.tsx, and canvas-events.ts. This delete only targets the row-shape hydration path, which has fully moved server-side. - npx tsc --noEmit -p .: clean - npx vitest run src/components/tabs/: 242/242 green (one fewer test file than before, 21 vs 22, since historyHydration.test.ts is gone) Why now (not after a longer observation window): the parsers in message-parser.ts that historyHydration.ts depended on are still in tree, so any unforeseen live-message consumer of activityRowToMessages would have already failed at PR-C-2 merge time (TypeScript would have flagged the missing import). Going further by removing the file itself is purely a dead-code cleanup — no behavioral change. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- .../chat/__tests__/historyHydration.test.ts | 270 ------------------ .../components/tabs/chat/historyHydration.ts | 83 ------ 2 files changed, 353 deletions(-) delete mode 100644 canvas/src/components/tabs/chat/__tests__/historyHydration.test.ts delete mode 100644 canvas/src/components/tabs/chat/historyHydration.ts diff --git a/canvas/src/components/tabs/chat/__tests__/historyHydration.test.ts b/canvas/src/components/tabs/chat/__tests__/historyHydration.test.ts deleted file mode 100644 index b9584f38..00000000 --- a/canvas/src/components/tabs/chat/__tests__/historyHydration.test.ts +++ /dev/null @@ -1,270 +0,0 @@ -// @vitest-environment jsdom -import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; -import { activityRowToMessages, type ActivityRowForHydration } from "../historyHydration"; - -const NEVER_INTERNAL = (_text: string) => false; - -function makeRow(overrides: Partial = {}): ActivityRowForHydration { - return { - activity_type: "a2a_receive", - status: "ok", - created_at: "2026-04-25T18:00:00.000Z", - request_body: null, - response_body: null, - ...overrides, - }; -} - -describe("activityRowToMessages", () => { - // The bug that prompted extracting this helper: every call to - // createMessage() inside the loader stamped `timestamp: new Date()`, - // so every reload re-stamped every historical user bubble to the - // render moment. Two messages sent hours apart both rendered with - // the same "now" clock. These tests pin the override so a future - // refactor that drops the spread-and-override fails loudly. - - describe("timestamp preservation (regression cover)", () => { - beforeEach(() => { - // Freeze the wall clock to a value that's CLEARLY different from - // any row.created_at the tests use, so any test that doesn't - // override the timestamp will mint "2030-…" and the assertion - // against "2026-…" will fail unmistakably. - vi.useFakeTimers(); - vi.setSystemTime(new Date("2030-01-01T00:00:00.000Z")); - }); - afterEach(() => vi.useRealTimers()); - - it("user message timestamp pins to row.created_at, NOT new Date()", () => { - const row = makeRow({ - created_at: "2026-04-25T18:00:00.000Z", - request_body: { params: { message: { parts: [{ kind: "text", text: "hello from earlier today" }] } } }, - }); - const msgs = activityRowToMessages(row, NEVER_INTERNAL); - const user = msgs.find((m) => m.role === "user")!; - expect(user.timestamp).toBe("2026-04-25T18:00:00.000Z"); - // Negative assertion: the wall clock is set to 2030. If the - // override regresses, timestamp will start with "2030-". - expect(user.timestamp.startsWith("2030")).toBe(false); - }); - - it("agent message timestamp pins to row.created_at, NOT new Date()", () => { - const row = makeRow({ - created_at: "2026-04-25T18:05:00.000Z", - response_body: { result: "agent reply" }, - }); - const msgs = activityRowToMessages(row, NEVER_INTERNAL); - const agent = msgs.find((m) => m.role === "agent")!; - expect(agent.timestamp).toBe("2026-04-25T18:05:00.000Z"); - expect(agent.timestamp.startsWith("2030")).toBe(false); - }); - - it("two rows with distinct created_at produce two distinct timestamps (the user-visible bug)", () => { - // The actual screenshot symptom: two user messages sent hours - // apart both rendered with the same render-moment timestamp. - // If activityRowToMessages defers to new Date(), both messages - // here will share the frozen 2030 wall clock instead of their - // distinct created_at values. - const a = activityRowToMessages( - makeRow({ - created_at: "2026-04-25T14:00:00.000Z", - request_body: { params: { message: { parts: [{ kind: "text", text: "first" }] } } }, - }), - NEVER_INTERNAL, - ); - const b = activityRowToMessages( - makeRow({ - created_at: "2026-04-25T21:01:58.000Z", - request_body: { params: { message: { parts: [{ kind: "text", text: "second" }] } } }, - }), - NEVER_INTERNAL, - ); - expect(a[0].timestamp).toBe("2026-04-25T14:00:00.000Z"); - expect(b[0].timestamp).toBe("2026-04-25T21:01:58.000Z"); - expect(a[0].timestamp).not.toBe(b[0].timestamp); - }); - }); - - describe("user-message extraction", () => { - it("emits a user message when request_body has text", () => { - const row = makeRow({ - request_body: { params: { message: { parts: [{ kind: "text", text: "hi agent" }] } } }, - }); - const msgs = activityRowToMessages(row, NEVER_INTERNAL); - expect(msgs).toHaveLength(1); - expect(msgs[0].role).toBe("user"); - expect(msgs[0].content).toBe("hi agent"); - }); - - it("drops user messages flagged as internal self-messages", () => { - // The heartbeat self-trigger ("Delegation results are ready...") - // gets recorded as a canvas-source row but isn't a real user - // message — the predicate filters it out so it doesn't render - // as the user talking to themselves. - const row = makeRow({ - request_body: { params: { message: { parts: [{ kind: "text", text: "Delegation results are ready..." }] } } }, - }); - const msgs = activityRowToMessages(row, (t) => t.startsWith("Delegation results are ready")); - expect(msgs.find((m) => m.role === "user")).toBeUndefined(); - }); - - it("emits no user message when request_body is null", () => { - const row = makeRow({ request_body: null }); - const msgs = activityRowToMessages(row, NEVER_INTERNAL); - expect(msgs.find((m) => m.role === "user")).toBeUndefined(); - }); - - // Reviewer follow-up: the pre-fix loader didn't extract user-side - // file parts, so a chat reload after a session where the user - // dragged in a file showed the text bubble but lost the chip. - // Symmetric to the agent attachment hydration below. - - it("hydrates user-side file attachments from request_body.params.message.parts", () => { - const row = makeRow({ - request_body: { - params: { - message: { - parts: [ - { kind: "text", text: "here's the screenshot" }, - { - kind: "file", - file: { - name: "shot.png", - mimeType: "image/png", - uri: "workspace:/uploads/shot.png", - size: 4096, - }, - }, - ], - }, - }, - }, - }); - const msgs = activityRowToMessages(row, NEVER_INTERNAL); - const user = msgs.find((m) => m.role === "user")!; - expect(user.content).toBe("here's the screenshot"); - expect(user.attachments).toEqual([ - { name: "shot.png", mimeType: "image/png", uri: "workspace:/uploads/shot.png", size: 4096 }, - ]); - }); - - it("emits an attachments-only user bubble when text is empty (drag-drop without caption)", () => { - // Some users drop a file with no message — the bubble should - // still render so the file appears in history. Pre-fix the - // empty userText short-circuited and the row was dropped. - const row = makeRow({ - request_body: { - params: { - message: { - parts: [ - { kind: "file", file: { name: "report.pdf", uri: "workspace:/uploads/report.pdf" } }, - ], - }, - }, - }, - }); - const msgs = activityRowToMessages(row, NEVER_INTERNAL); - expect(msgs).toHaveLength(1); - expect(msgs[0].role).toBe("user"); - expect(msgs[0].content).toBe(""); - expect(msgs[0].attachments).toHaveLength(1); - expect(msgs[0].attachments![0].name).toBe("report.pdf"); - }); - - it("internal-self predicate suppresses the row even if it carries attachments", () => { - // Defence-in-depth: heartbeat self-trigger never produces - // attachments, but if a future internal trigger DID, we still - // want to suppress (otherwise it'd render as the user attaching - // something they never touched). - const row = makeRow({ - request_body: { - params: { - message: { - parts: [ - { kind: "text", text: "Delegation results are ready..." }, - { kind: "file", file: { name: "x.zip", uri: "workspace:/x.zip" } }, - ], - }, - }, - }, - }); - const msgs = activityRowToMessages(row, (t) => t.startsWith("Delegation results are ready")); - expect(msgs.find((m) => m.role === "user")).toBeUndefined(); - }); - }); - - describe("agent-message extraction", () => { - it("emits an agent message from response_body.result string", () => { - const row = makeRow({ response_body: { result: "agent says hi" } }); - const msgs = activityRowToMessages(row, NEVER_INTERNAL); - expect(msgs).toHaveLength(1); - expect(msgs[0].role).toBe("agent"); - expect(msgs[0].content).toBe("agent says hi"); - }); - - it("emits role=system when status=error", () => { - // System role gets distinct rendering (red bubble) for failed - // calls; if this regresses errors will look like normal agent - // replies and the user won't realise something went wrong. - const row = makeRow({ - status: "error", - response_body: { result: "delegation failed" }, - }); - const msgs = activityRowToMessages(row, NEVER_INTERNAL); - expect(msgs[0].role).toBe("system"); - }); - - it("attaches file parts hydrated from response_body (parts at root)", () => { - // The notify-with-attachments shape: response_body = - // {result: "", parts: [{kind:"file", ...}]}. If - // extractFilesFromTask doesn't see the attachments here, a chat - // reload after an agent attached a file would lose the chips. - const row = makeRow({ - response_body: { - result: "Done — see attached.", - parts: [ - { kind: "file", file: { name: "build.zip", uri: "workspace:/tmp/build.zip", size: 12345 } }, - ], - }, - }); - const msgs = activityRowToMessages(row, NEVER_INTERNAL); - const agent = msgs.find((m) => m.role === "agent")!; - expect(agent.attachments).toEqual([ - { name: "build.zip", uri: "workspace:/tmp/build.zip", size: 12345 }, - ]); - }); - - it("emits no agent message when response_body is null", () => { - const row = makeRow({ response_body: null }); - const msgs = activityRowToMessages(row, NEVER_INTERNAL); - expect(msgs.find((m) => m.role === "agent" || m.role === "system")).toBeUndefined(); - }); - - it("emits no agent message when response_body has neither text nor files", () => { - const row = makeRow({ response_body: { unrelated: "metadata" } }); - const msgs = activityRowToMessages(row, NEVER_INTERNAL); - expect(msgs.find((m) => m.role === "agent")).toBeUndefined(); - }); - }); - - describe("end-to-end shape", () => { - it("a single row with both user request and agent reply emits two messages with the same timestamp", () => { - // Mirrors the canonical canvas-source row: user types something, - // agent replies, both stored on the same activity_logs row. UI - // renders them as the user bubble immediately followed by the - // agent bubble — keep that pairing intact. - const row = makeRow({ - created_at: "2026-04-25T18:00:00.000Z", - request_body: { params: { message: { parts: [{ kind: "text", text: "what's 2+2?" }] } } }, - response_body: { result: "4" }, - }); - const msgs = activityRowToMessages(row, NEVER_INTERNAL); - expect(msgs).toHaveLength(2); - expect(msgs[0].role).toBe("user"); - expect(msgs[0].content).toBe("what's 2+2?"); - expect(msgs[0].timestamp).toBe("2026-04-25T18:00:00.000Z"); - expect(msgs[1].role).toBe("agent"); - expect(msgs[1].content).toBe("4"); - expect(msgs[1].timestamp).toBe("2026-04-25T18:00:00.000Z"); - }); - }); -}); diff --git a/canvas/src/components/tabs/chat/historyHydration.ts b/canvas/src/components/tabs/chat/historyHydration.ts deleted file mode 100644 index 09bcea35..00000000 --- a/canvas/src/components/tabs/chat/historyHydration.ts +++ /dev/null @@ -1,83 +0,0 @@ -import { type ChatMessage, createMessage } from "./types"; -import { extractResponseText, extractRequestText, extractFilesFromTask } from "./message-parser"; - -/** Activity row shape the chat history loader consumes. Only the fields - * it actually reads are listed — the platform sends more (id, target_id, - * method, summary, etc.) but the hydration is defined by these four. */ -export interface ActivityRowForHydration { - activity_type: string; - status: string; - created_at: string; - request_body: Record | null; - response_body: Record | null; -} - -/** Map a single activity_logs row to the chat messages it represents. - * - * An a2a_receive row can produce up to two messages: - * 1. A user-side bubble derived from request_body (the message the - * user sent), unless the request was an internal self-message. - * 2. An agent-side bubble derived from response_body (text + - * file attachments), with role=system when status=error. - * - * CRITICAL: both messages MUST adopt `row.created_at` as their - * timestamp. createMessage() defaults to new Date() — appropriate for - * freshly-typed messages, wrong for hydrated history because every - * reload would re-stamp every bubble to the render moment. The - * regression that prompted extracting this helper showed up as every - * user message in the chat collapsing to the same "now" clock after - * reload (see test_user_messages_pin_timestamps_to_created_at). - */ -export function activityRowToMessages( - row: ActivityRowForHydration, - isInternalSelfMessage: (text: string) => boolean, -): ChatMessage[] { - const out: ChatMessage[] = []; - - const userText = extractRequestText(row.request_body); - // Hydrate user-side file attachments out of the same A2A envelope. - // Without this, a chat reload after a session where the user dragged - // in a file shows the text bubble but loses the download chip — the - // pre-fix loader only walked text via extractRequestText. Mirrors - // the agent branch below. Wire shape from ChatTab's outbound POST: - // request_body = {params: {message: {parts: [{kind:"text"}, {kind:"file", file:{...}}]}}} - // extractFilesFromTask walks `task.parts`, so we feed it `params.message`. - const userMsg = (row.request_body?.params as Record | undefined) - ?.message as Record | undefined; - const userAttachments = userMsg ? extractFilesFromTask(userMsg) : []; - // Internal-self messages (e.g. heartbeat self-trigger) take precedence - // — drop the row even if it carries attachments, since the heartbeat - // path doesn't produce attachments anyway and keeping the bubble would - // misattribute it to the user. - const isInternal = !!userText && isInternalSelfMessage(userText); - if (!isInternal && (userText || userAttachments.length > 0)) { - out.push({ - ...createMessage("user", userText, userAttachments), - timestamp: row.created_at, - }); - } - - if (row.response_body) { - const text = extractResponseText(row.response_body); - // Pick the right object to feed extractFilesFromTask: - // - Task-shape: {result: {parts: [...]}} → unwrap result - // - Notify-shape: {result: "", parts: [...]} → use the body - // Naively doing `result ?? body` would pass the string "" to - // the file extractor for the notify case, returning [] and dropping - // the file chips on reload. Only unwrap when result is an object. - const filesSource: Record = - row.response_body.result && typeof row.response_body.result === "object" - ? (row.response_body.result as Record) - : row.response_body; - const attachments = extractFilesFromTask(filesSource); - if (text || attachments.length > 0) { - const role: ChatMessage["role"] = - row.status === "error" || text.toLowerCase().startsWith("agent error") - ? "system" - : "agent"; - out.push({ ...createMessage(role, text, attachments), timestamp: row.created_at }); - } - } - - return out; -} -- 2.45.2