From 9a94f41a61a94be3eca21ef66552649cdc01f268 Mon Sep 17 00:00:00 2001 From: Federico Jaramillo Martinez Date: Fri, 17 Jul 2026 21:37:30 +0200 Subject: [PATCH] refactor(test): migrate ChatView.test.ts to shared template-inspection seam Reclassify ChatView.test.ts per the testing-guide skill: move content/text/ attribute/ordering assertions to new pure public seams on ChatView.ts (chatSessionWarningRows, chatQueuedSectionShowsClearAction, chatGroupAnchorKey, chatEventAnchorKey, chatGroupScrollMarkerId, chatMessageGroupClassName, chatMessageGroupLabel) with the component render code delegating to them, and route the genuine Clear-queue/dismiss/toggle event wiring through the shared templateInspection.testSupport helpers with escape-hatch comments. Delete the per-file TemplateResult-inspection cluster. Drop the now-consumed @public tag from templateEventHandlerNearMarker. --- src/client/src/components/ChatView.test.ts | 356 +++++++----------- src/client/src/components/ChatView.ts | 88 ++++- .../src/templateInspection.testSupport.ts | 2 - 3 files changed, 207 insertions(+), 239 deletions(-) diff --git a/src/client/src/components/ChatView.test.ts b/src/client/src/components/ChatView.test.ts index afe59db..19d2cf5 100644 --- a/src/client/src/components/ChatView.test.ts +++ b/src/client/src/components/ChatView.test.ts @@ -2,7 +2,19 @@ import type { TemplateResult } from "lit"; import { describe, expect, it, vi } from "vitest"; import type { QueuedSessionMessage, SessionStatus, SessionWarning } from "../api"; import type { ChatLine } from "./shared"; -import { ChatView, chatMessageMetadataLabel, chatQueuedMessageSections } from "./ChatView"; +import { + ChatView, + chatEventAnchorKey, + chatGroupAnchorKey, + chatGroupScrollMarkerId, + chatMessageGroupClassName, + chatMessageGroupLabel, + chatMessageMetadataLabel, + chatQueuedMessageSections, + chatQueuedSectionShowsClearAction, + chatSessionWarningRows, +} from "./ChatView"; +import { templateEventHandlerAfterMarker, templateEventHandlerNearMarker } from "../templateInspection.testSupport"; describe("chatQueuedMessageSections", () => { it("labels client-side pending-start sends separately from server queued messages", () => { @@ -28,97 +40,87 @@ describe("chatQueuedMessageSections", () => { }); }); -describe("ChatView queued-message clear action", () => { - // Direct handler extraction keeps this node-environment test focused on the - // Clear queue template wiring without introducing a component-wide DOM shim. - it("renders an accessible server-queue action and invokes its callback", () => { +describe("chatQueuedSectionShowsClearAction", () => { + // The show/hide decision for the server clear-queue button is content/layout, + // so it lives in a pure exported seam instead of scraping rendered markup. + const serverSection = requireSection(chatQueuedMessageSections([], [{ kind: "steer", text: "server queued" }])[0]); + const clientSection = requireSection(chatQueuedMessageSections([{ kind: "followUp", text: "waiting" }], [])[0]); + + it("shows the action for the server queue when clearing is supported and wired", () => { + expect(chatQueuedSectionShowsClearAction(serverSection, true, true)).toBe(true); + }); + + it("hides the action when the runtime does not support clearing", () => { + expect(chatQueuedSectionShowsClearAction(serverSection, false, true)).toBe(false); + }); + + it("hides the action when no clear handler is wired", () => { + expect(chatQueuedSectionShowsClearAction(serverSection, true, false)).toBe(false); + }); + + it("never shows the server action for the separate client pending-start queue", () => { + expect(chatQueuedSectionShowsClearAction(clientSection, true, true)).toBe(false); + }); +}); + +describe("ChatView queued-message clear wiring", () => { + // Escape hatch: this case verifies the Clear queue button's Lit event wiring, + // whose only observable effect is invoking the injected callback. Vitest runs + // with no DOM environment here, so a shadow-DOM click harness would add + // disproportionate setup; handler extraction anchored to the user-facing + // "Clear queue" button text is proportionate. + it("invokes onClearServerQueue when the server-queue action is activated", () => { const view = new ChatView(); const onClearServerQueue = vi.fn(); view.status = queuedStatus([{ kind: "steer", text: "server queued" }]); view.canClearServerQueue = true; view.onClearServerQueue = onClearServerQueue; - const rendered = renderQueuedMessages(view); - const markup = templateStaticMarkup(rendered); + templateEventHandlerNearMarker(renderQueuedMessages(view), "Clear queue")(new Event("click")); - expect(markup).toContain('type="button"'); - expect(markup).toContain('title="Clear queued messages without stopping active work"'); - expect(markup).toContain(">Clear queue"); - templateEventHandler(rendered, "Clear queue")(new Event("click")); expect(onClearServerQueue).toHaveBeenCalledOnce(); }); - - it("hides the action when the selected runtime does not support clearing", () => { - const view = new ChatView(); - view.status = queuedStatus([{ kind: "followUp", text: "server queued" }]); - view.canClearServerQueue = false; - view.onClearServerQueue = vi.fn(); - - expect(templateStaticMarkup(renderQueuedMessages(view))).not.toContain("Clear queue"); - }); - - it("does not expose the server action for the separate client pending-start queue", () => { - const view = new ChatView(); - view.status = queuedStatus([]); - view.clientQueuedMessages = [{ kind: "followUp", text: "waiting for session start" }]; - view.canClearServerQueue = true; - view.onClearServerQueue = vi.fn(); - - expect(templateStaticMarkup(renderQueuedMessages(view))).not.toContain("Clear queue"); - }); }); -describe("ChatView session warnings banner", () => { - it("renders one severity-tagged row per warning with optional path and source", () => { - const view = new ChatView(); - view.status = warningStatus([ +describe("chatSessionWarningRows", () => { + // Warning-row content (severity class, message, path, source, dismiss + // capability, ordering) is derived by a pure exported seam rather than scraped + // from rendered `TemplateResult` markup, per the testing-guide rule that + // TemplateResult inspection is not for general content assertions. + it("derives one severity-tagged row per warning with optional path and source", () => { + const rows = chatSessionWarningRows(warningStatus([ { severity: "error", message: "skill failed to load", source: "skill", path: "/skills/a.md" }, { severity: "warning", message: "subscription auth is active" }, { severity: "info", message: "heads up", source: "runtime" }, + ])); + + expect(rows).toEqual([ + { severity: "error", severityClass: "session-warning error", message: "skill failed to load", source: "skill", path: "/skills/a.md", dismissId: undefined }, + { severity: "warning", severityClass: "session-warning warning", message: "subscription auth is active", source: undefined, path: undefined, dismissId: undefined }, + { severity: "info", severityClass: "session-warning info", message: "heads up", source: "runtime", path: undefined, dismissId: undefined }, ]); - - const rendered = renderWarnings(view); - if (rendered === null) throw new Error("expected a warnings banner"); - const markup = templateStaticMarkup(rendered); - const values = collectStringValues(rendered); - - expect(markup).toContain('class="session-warnings"'); - expect(markup).toContain('role="alert"'); - expect(values).toContain("skill failed to load"); - expect(values).toContain("/skills/a.md"); - expect(values).toContain("subscription auth is active"); - expect(values).toContain("heads up"); - expect(values).toContain("skill"); - const severityClasses = values.filter((value) => value.startsWith("session-warning ")); - expect(severityClasses).toEqual(["session-warning error", "session-warning warning", "session-warning info"]); }); - it("renders nothing when there are no warnings", () => { - const view = new ChatView(); - view.status = warningStatus([]); - expect(renderWarnings(view)).toBeNull(); - }); - - it("renders nothing when status is unset", () => { - expect(renderWarnings(new ChatView())).toBeNull(); - }); - - it("renders a dismiss control only for warnings carrying a dismiss capability", () => { - const view = new ChatView(); - view.status = warningStatus([ + it("exposes a dismiss id only for warnings carrying a dismiss capability", () => { + const rows = chatSessionWarningRows(warningStatus([ { severity: "error", message: "skill failed to load", source: "skill" }, { severity: "warning", message: "subscription auth is active", source: "anthropic", dismiss: { id: "anthropicExtraUsage" } }, - ]); + ])); - const rendered = renderWarnings(view); - if (rendered === null) throw new Error("expected a warnings banner"); - const markup = templateStaticMarkup(rendered); - // One dismiss button total: only the warning with a dismiss capability gets one. - expect(markup.match(/session-warning-dismiss/g)?.length).toBe(1); + expect(rows.map((row) => row.dismissId)).toEqual([undefined, "anthropicExtraUsage"]); }); - // Direct handler extraction keeps this node-environment test focused on the - // dismiss button wiring without a component-wide DOM shim. + it("derives no rows when there are no warnings or status is unset", () => { + expect(chatSessionWarningRows(warningStatus([]))).toEqual([]); + expect(chatSessionWarningRows(undefined)).toEqual([]); + }); +}); + +describe("ChatView session-warning dismiss wiring", () => { + // Escape hatch: this case verifies the dismiss button's Lit event wiring, + // whose observable effect is invoking onDismissWarning with the warning's + // dismiss id. No DOM environment is available, so handler extraction anchored + // to the stable `session-warning-dismiss` class marker is proportionate. it("invokes onDismissWarning with the warning's dismiss id", () => { const view = new ChatView(); const onDismissWarning = vi.fn(); @@ -129,10 +131,14 @@ describe("ChatView session warnings banner", () => { const rendered = renderWarnings(view); if (rendered === null) throw new Error("expected a warnings banner"); - templateEventHandler(rendered, "session-warning-dismiss")(new Event("click")); + templateEventHandlerAfterMarker(rendered, "session-warning-dismiss")(new Event("click")); expect(onDismissWarning).toHaveBeenCalledExactlyOnceWith("anthropicExtraUsage"); }); + + it("renders nothing when there are no warnings", () => { + expect(renderWarnings(withStatus(new ChatView(), warningStatus([])))).toBeNull(); + }); }); describe("chatMessageMetadataLabel", () => { @@ -148,50 +154,39 @@ describe("chatMessageMetadataLabel", () => { }); }); -describe("ChatView technical-event groups", () => { +describe("chat event-group content seams", () => { + // Group scroll-anchor keys, marker ids, class list, and disclosure label are + // content/structure derived from pure exported seams rather than scraped from + // rendered markup. + it("derives stable group and event scroll-anchor keys and marker ids", () => { + expect(chatGroupAnchorKey(40)).toBe("g:40"); + expect(chatEventAnchorKey(40)).toBe("e:40"); + expect(chatEventAnchorKey(41)).toBe("e:41"); + expect(chatGroupScrollMarkerId(41)).toBe("g:41"); + }); + + it("distinguishes the live tail group by class and disclosure label", () => { + expect(chatMessageGroupClassName(true)).toBe("msg event-group live"); + expect(chatMessageGroupClassName(false)).toBe("msg event-group"); + expect(chatMessageGroupLabel(true)).toBe("live events"); + expect(chatMessageGroupLabel(false)).toBe("events"); + }); +}); + +describe("ChatView event-group disclosure wiring", () => { const messages: ChatLine[] = [ { role: "assistant", parts: [{ type: "toolCall", toolName: "read", summary: "inspect a file" }] }, { role: "tool", parts: [{ type: "toolExecution", toolName: "read", summary: "inspect a file", status: "success", resultText: "large result" }] }, ]; - it("defers a closed body while retaining native disclosure and group scroll anchors", () => { + it("defers a closed group body until it is opened", () => { const view = new ChatView(); view.sessionId = "session-1"; const bodyCalls = observeGroupBodyRenders(view); - const closed = renderMessageGroup(view, messages, 40, 41, false); + renderMessageGroup(view, messages, 40, 41, false); expect(bodyCalls).toEqual([]); - expect(templateStaticMarkup(closed)).toContain(""); - expect(templateStaticMarkup(closed)).toContain('aria-hidden="true"'); - expect(templateValuesAfterMarker(closed, "?open=")).toEqual([false]); - expect(templateValuesAfterMarker(closed, "data-scroll-anchor-id=")).toEqual(["g:40"]); - expect(templateValuesAfterMarker(closed, "data-marker-id=")).toEqual(["g:41"]); - }); - - // Direct handler extraction keeps this node-environment test focused on the - // native details toggle wiring without introducing a component-wide DOM shim. - it("renders an opened body with event anchors and removes it when closed again", () => { - const view = new ChatView(); - view.sessionId = "session-1"; - const bodyCalls = observeGroupBodyRenders(view); - const initiallyClosed = renderMessageGroup(view, messages, 40, 41, false); - - dispatchDetailsToggle(templateEventHandler(initiallyClosed, "@toggle="), true); - const opened = renderMessageGroup(view, messages, 40, 41, false); - - expect(bodyCalls).toEqual([{ messages, startIndex: 40 }]); - expect(templateValuesAfterMarker(opened, "?open=")).toEqual([true]); - expect(templateValuesAfterMarker(opened, "data-scroll-anchor-id=")).toEqual(["g:40", "e:40", "e:41"]); - - bodyCalls.length = 0; - dispatchDetailsToggle(templateEventHandler(opened, "@toggle="), false); - const closedAgain = renderMessageGroup(view, messages, 40, 41, false); - - expect(bodyCalls).toEqual([]); - expect(templateValuesAfterMarker(closedAgain, "?open=")).toEqual([false]); - expect(templateValuesAfterMarker(closedAgain, "data-scroll-anchor-id=")).toEqual(["g:40"]); }); it("renders a live tail body by default", () => { @@ -199,12 +194,32 @@ describe("ChatView technical-event groups", () => { view.sessionId = "session-1"; const bodyCalls = observeGroupBodyRenders(view); - const live = renderMessageGroup(view, messages, 40, 41, true); + renderMessageGroup(view, messages, 40, 41, true); expect(bodyCalls).toEqual([{ messages, startIndex: 40 }]); - expect(templateValuesAfterMarker(live, "?open=")).toEqual([true]); - expect(templateValues(live)).toContain("msg event-group live"); - expect(templateValues(live)).toContain("live events"); + }); + + // Escape hatch: this case verifies the native `
` `@toggle` wiring, + // whose observable effect is that a re-render renders (or defers) the group + // body. No DOM environment is available for a real disclosure interaction, so + // handler extraction anchored to the stable `@toggle=` attribute marker plus + // an injected details-toggle event is proportionate. + it("renders the body after a toggle-open and removes it when closed again", () => { + const view = new ChatView(); + view.sessionId = "session-1"; + const bodyCalls = observeGroupBodyRenders(view); + const initiallyClosed = renderMessageGroup(view, messages, 40, 41, false); + + dispatchDetailsToggle(templateEventHandlerAfterMarker(initiallyClosed, "@toggle="), true); + renderMessageGroup(view, messages, 40, 41, false); + + expect(bodyCalls).toEqual([{ messages, startIndex: 40 }]); + + bodyCalls.length = 0; + dispatchDetailsToggle(templateEventHandlerAfterMarker(initiallyClosed, "@toggle="), false); + renderMessageGroup(view, messages, 40, 41, false); + + expect(bodyCalls).toEqual([]); }); }); @@ -216,6 +231,7 @@ interface GroupBodyRenderCall { type RenderQueuedMessages = (this: ChatView) => TemplateResult; type RenderMessageGroup = (this: ChatView, messages: ChatLine[], startIndex: number, endIndex: number, defaultOpen: boolean) => TemplateResult; type RenderMessageGroupBody = (this: ChatView, messages: ChatLine[], startIndex: number) => TemplateResult; +type RenderWarnings = (this: ChatView) => TemplateResult | null; type TemplateEventHandler = (event: Event) => void; function renderQueuedMessages(view: ChatView): TemplateResult { @@ -230,6 +246,12 @@ function renderMessageGroup(view: ChatView, messages: ChatLine[], startIndex: nu return method.call(view, messages, startIndex, endIndex, defaultOpen); } +function renderWarnings(view: ChatView): TemplateResult | null { + const method: unknown = Reflect.get(view, "renderWarnings"); + if (!isRenderWarnings(method)) throw new Error("ChatView.renderWarnings is not callable"); + return method.call(view); +} + function observeGroupBodyRenders(view: ChatView): GroupBodyRenderCall[] { const method: unknown = Reflect.get(view, "renderMessageGroupBody"); if (!isRenderMessageGroupBody(method)) throw new Error("ChatView.renderMessageGroupBody is not callable"); @@ -254,34 +276,7 @@ function isRenderMessageGroupBody(value: unknown): value is RenderMessageGroupBo return typeof value === "function"; } -function templateEventHandler(template: TemplateResult, marker: string): TemplateEventHandler { - let handler: TemplateEventHandler | undefined; - visit(template); - if (handler === undefined) throw new Error(`Expected template event handler near ${marker}`); - return handler; - - function visit(value: unknown): void { - if (handler !== undefined) return; - if (Array.isArray(value)) { - for (const item of value) visit(item); - return; - } - if (!isTemplateResult(value)) return; - const strings = templateStrings(value); - const values = templateValues(value); - for (let index = 0; index < values.length; index += 1) { - const candidate = values[index]; - const isNearMarker = strings[index]?.includes(marker) === true || strings[index + 1]?.includes(marker) === true; - if (isNearMarker && isTemplateEventHandler(candidate)) { - handler = candidate; - return; - } - visit(candidate); - } - } -} - -function isTemplateEventHandler(value: unknown): value is TemplateEventHandler { +function isRenderWarnings(value: unknown): value is RenderWarnings { return typeof value === "function"; } @@ -304,91 +299,14 @@ function dispatchDetailsToggle(handler: TemplateEventHandler, open: boolean): vo } } -function templateStaticMarkup(template: TemplateResult): string { - const chunks: string[] = []; - visit(template); - return chunks.join(""); - - function visit(value: unknown): void { - if (Array.isArray(value)) { - for (const item of value) visit(item); - return; - } - if (!isTemplateResult(value)) return; - chunks.push(...templateStrings(value)); - for (const child of templateValues(value)) visit(child); - } +function requireSection(section: ReturnType[number] | undefined): ReturnType[number] { + if (section === undefined) throw new Error("expected a queued-message section"); + return section; } -function templateValuesAfterMarker(template: TemplateResult, marker: string): unknown[] { - const matches: unknown[] = []; - visit(template); - return matches; - - function visit(value: unknown): void { - if (Array.isArray(value)) { - for (const item of value) visit(item); - return; - } - if (!isTemplateResult(value)) return; - const strings = templateStrings(value); - const values = templateValues(value); - for (let index = 0; index < values.length; index += 1) { - if (strings[index]?.includes(marker) === true) matches.push(values[index]); - visit(values[index]); - } - } -} - -function templateStrings(template: TemplateResult): readonly string[] { - const strings = Reflect.get(template, "strings"); - if (!isStringArray(strings)) throw new Error("TemplateResult strings were unavailable"); - return strings; -} - -function templateValues(template: TemplateResult): readonly unknown[] { - const values = Reflect.get(template, "values"); - if (!Array.isArray(values)) throw new Error("TemplateResult values were unavailable"); - return values.map((value: unknown) => value); -} - -function isTemplateResult(value: unknown): value is TemplateResult { - return typeof value === "object" && value !== null && isStringArray(Reflect.get(value, "strings")) && Array.isArray(Reflect.get(value, "values")); -} - -function isStringArray(value: unknown): value is string[] { - return Array.isArray(value) && value.every((item: unknown) => typeof item === "string"); -} - -function collectStringValues(template: TemplateResult): string[] { - const found: string[] = []; - visit(template); - return found; - - function visit(value: unknown): void { - if (typeof value === "string") { - found.push(value); - return; - } - if (Array.isArray(value)) { - for (const item of value) visit(item); - return; - } - if (!isTemplateResult(value)) return; - for (const child of templateValues(value)) visit(child); - } -} - -type RenderWarnings = (this: ChatView) => TemplateResult | null; - -function isRenderWarnings(value: unknown): value is RenderWarnings { - return typeof value === "function"; -} - -function renderWarnings(view: ChatView): TemplateResult | null { - const method: unknown = Reflect.get(view, "renderWarnings"); - if (!isRenderWarnings(method)) throw new Error("ChatView.renderWarnings is not callable"); - return method.call(view); +function withStatus(view: ChatView, status: SessionStatus): ChatView { + view.status = status; + return view; } function warningStatus(warnings: SessionWarning[]): SessionStatus { diff --git a/src/client/src/components/ChatView.ts b/src/client/src/components/ChatView.ts index ec1b1fe..71b2548 100644 --- a/src/client/src/components/ChatView.ts +++ b/src/client/src/components/ChatView.ts @@ -62,6 +62,58 @@ export function chatMessageAnchorKey(index: number): string { return `m:${String(index)}`; } +/** The stable scroll-anchor/render key for a collapsed event group starting at `startIndex`. */ +export function chatGroupAnchorKey(startIndex: number): string { + return `g:${String(startIndex)}`; +} + +/** The stable scroll-anchor key for an event inside a group at `index`. */ +export function chatEventAnchorKey(index: number): string { + return `e:${String(index)}`; +} + +/** The stable scroll-marker id emitted before an event group ending at `endIndex`. */ +export function chatGroupScrollMarkerId(endIndex: number): string { + return `g:${String(endIndex)}`; +} + +/** The CSS class list for an event-group `
`, distinguishing the live tail. */ +export function chatMessageGroupClassName(defaultOpen: boolean): string { + return defaultOpen ? "msg event-group live" : "msg event-group"; +} + +/** The disclosure summary label for an event group, distinguishing the live tail. */ +export function chatMessageGroupLabel(defaultOpen: boolean): string { + return defaultOpen ? "live events" : "events"; +} + +/** Whether a queued-message section shows the server clear-queue action. */ +export function chatQueuedSectionShowsClearAction(section: QueuedMessageSection, canClearServerQueue: boolean, hasClearHandler: boolean): boolean { + return section.source === "server" && canClearServerQueue && hasClearHandler; +} + +/** A rendered session-warning row derived from live status warnings. */ +export interface ChatSessionWarningRow { + severity: SessionWarningSeverity; + severityClass: string; + message: string; + source?: string; + path?: string; + dismissId?: string; +} + +/** Derive one severity-tagged warning row per live status warning, in order. */ +export function chatSessionWarningRows(status: SessionStatus | undefined): ChatSessionWarningRow[] { + return (status?.warnings ?? []).map((warning) => ({ + severity: warning.severity, + severityClass: `session-warning ${warning.severity}`, + message: warning.message, + ...(warning.source === undefined ? {} : { source: warning.source }), + ...(warning.path === undefined ? {} : { path: warning.path }), + ...(warning.dismiss === undefined ? {} : { dismissId: warning.dismiss.id }), + })); +} + export function chatMessageMetadataLabel(message: ChatLine): string { const timestamp = message.meta?.timestamp; const time = timestamp === undefined ? undefined : formatMessageTimestamp(timestamp); @@ -256,29 +308,29 @@ export class ChatView extends LitElement { } private renderWarnings() { - const warnings = this.status?.warnings ?? []; - if (warnings.length === 0) return null; + const rows = chatSessionWarningRows(this.status); + if (rows.length === 0) return null; return html`