molecule-ai/molecule-core
35
0
Fork 2
Code Issues 16 Pull Requests 2 Actions 49 Packages Projects Releases Wiki Activity

test(chat): extract historyHydration helper + 12 unit tests

Browse Source 
User pushed back: the timestamp bug should have been caught by E2E.
Right — my earlier coverage tested the server contract (notify endpoint,
WS broadcast filter) but never the chat-history HYDRATION path. Without
a unit test that froze the wall clock and asserted timestamps came from
created_at, a future refactor could re-introduce the same bug.

This commit:

1. Extracts the per-row → ChatMessage[] mapping out of the closure
   inside loadMessagesFromDB into chat/historyHydration.ts. Pure
   function, no React dependency, easy to test.

2. Adds 12 vitest cases in __tests__/historyHydration.test.ts covering:
   - Timestamp regression (3 tests, with system time frozen to 2030 so
     a regression starts producing "2030-…" timestamps and the assertion
     fails unmistakably). The third test mirrors the user's screenshot:
     two rows with distinct created_at must produce distinct timestamps.
   - User-message extraction (text, internal-self filter, null body)
   - Agent-message extraction (text, error→system role, file attachments,
     null body, body with neither text nor files)
   - End-to-end: a single row with both request and response emits
     two messages with the same timestamp (the canonical canvas-source
     row pattern)

3. The new file-attachment test caught a SECOND latent bug — the helper
   was passing `response_body.result ?? response_body` to extractFiles
   FromTask, which passes the STRING "<text>" for the notify-with-
   attachments shape `{result: "<text>", parts: [...]}` and silently
   returns []. So a chat reload after an agent attached a file would
   lose the chips. Fixed by only unwrapping `result` when it's an
   object (the task-shape) and falling through to response_body
   otherwise (the notify shape).

ChatTab now imports the helper and the loop body becomes one line:
`messages.push(...activityRowToMessages(a, isInternalSelfMessage))`.

Verification:
  - 12/12 historyHydration tests pass
  - 1072/1072 full canvas vitest pass (was 1060 before, +12)
  - tsc --noEmit clean

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Hongming Wang 2026-04-26 21:18:22 -07:00
parent 8415870520
commit fe204f04da
3 changed files with 267 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
canvas/src/components/tabs/ChatTab.tsx
View File

@ -10,9 +10,10 @@ import { closeWebSocketGracefully } from "@/lib/ws-close";
import { type ChatMessage, type ChatAttachment, createMessage, appendMessageDeduped } from "./chat/types";
import { uploadChatFiles, downloadChatFile } from "./chat/uploads";
import { AttachmentChip, PendingAttachmentPill } from "./chat/AttachmentViews";
import { extractResponseText, extractRequestText, extractFilesFromTask } from "./chat/message-parser";
import { extractResponseText, extractFilesFromTask } from "./chat/message-parser";
import { AgentCommsPanel } from "./chat/AgentCommsPanel";
import { appendActivityLine } from "./chat/activityLog";
import { activityRowToMessages, type ActivityRowForHydration } from "./chat/historyHydration";
import { runtimeDisplayName } from "@/lib/runtime-names";
import { ConfirmDialog } from "@/components/ConfirmDialog";
@ -125,42 +126,17 @@ function extractReplyText(resp: A2AResponse): string {
*/
async function loadMessagesFromDB(workspaceId: string): Promise<{ messages: ChatMessage[]; error: string | null }> {
try {
const activities = await api.get<Array<{
activity_type: string;
status: string;
created_at: string;
request_body: Record<string, unknown> | null;
response_body: Record<string, unknown> | null;
}>>(`/workspaces/${workspaceId}/activity?type=a2a_receive&source=canvas&limit=50`);
const activities = await api.get<ActivityRowForHydration[]>(
`/workspaces/${workspaceId}/activity?type=a2a_receive&source=canvas&limit=50`,
);
const messages: ChatMessage[] = [];
// Activities are newest-first, reverse for chronological order
// Activities are newest-first, reverse for chronological order.
// Per-row mapping lives in chat/historyHydration.ts so it can be
// unit-tested without spinning up the full ChatTab component
// (regression cover for the timestamp-collapse bug).
for (const a of [...activities].reverse()) {
// Extract user message from request_body. Pin the timestamp to
// the activity row's created_at — without this override every
// historical user bubble re-stamps to "now" on each chat reload,
// and ALL user messages collapse to the same render-time clock
// (same as the agent path on line 157).
const userText = extractRequestText(a.request_body);
if (userText && !isInternalSelfMessage(userText)) {
messages.push({ ...createMessage("user", userText), timestamp: a.created_at });
}
// Extract agent response — text AND any file attachments so a
// chat reload surfaces historical download chips, not just plain
// text. `result` is nested on successful A2A responses; some
// older rows stored the raw `result` payload at the top level,
// so fall back to the body itself when `.result` is absent.
if (a.response_body) {
const text = extractResponseText(a.response_body);
const attachments = extractFilesFromTask(
(a.response_body.result ?? a.response_body) as Record<string, unknown>,
);
if (text || attachments.length > 0) {
const role = a.status === "error" || text.toLowerCase().startsWith("agent error") ? "system" : "agent";
messages.push({ ...createMessage(role, text, attachments), timestamp: a.created_at });
}
}
messages.push(...activityRowToMessages(a, isInternalSelfMessage));
}
return { messages, error: null };
} catch (err) {

192
canvas/src/components/tabs/chat/__tests__/historyHydration.test.ts Normal file
View File

@ -0,0 +1,192 @@
// @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> = {}): 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();
});
});
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: "<text>", 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");
});
});
});

65
canvas/src/components/tabs/chat/historyHydration.ts Normal file
View File

@ -0,0 +1,65 @@
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<string, unknown> | null;
response_body: Record<string, unknown> | 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);
if (userText && !isInternalSelfMessage(userText)) {
out.push({ ...createMessage("user", userText), 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: "<text>", parts: [...]} → use the body
// Naively doing `result ?? body` would pass the string "<text>" 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<string, unknown> =
row.response_body.result && typeof row.response_body.result === "object"
? (row.response_body.result as Record<string, unknown>)
: 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;
}