diff --git a/.changeset/show-tool-images-outside-events.md b/.changeset/show-tool-images-outside-events.md index 35d8cb9..86686e4 100644 --- a/.changeset/show-tool-images-outside-events.md +++ b/.changeset/show-tool-images-outside-events.md @@ -2,4 +2,4 @@ "@jmfederico/pi-web": patch --- -Keep image content from tool results visible outside collapsed event groups while retaining technical tool metadata inside the group. +Keep tool-result images visible as compact, clearly labeled output outside collapsed event groups while retaining technical execution details and final message metadata. diff --git a/src/client/src/chatGroups.test.ts b/src/client/src/chatGroups.test.ts index d29230a..c3d4d1d 100644 --- a/src/client/src/chatGroups.test.ts +++ b/src/client/src/chatGroups.test.ts @@ -54,7 +54,7 @@ describe("groupChatMessages", () => { endIndex: 0, messages: [{ role: "tool", parts: [{ type: "toolResult", toolName: "read", text: "Read image file [image/png]", isError: false }] }], }, - { kind: "message", index: 0, message: { role: "tool", parts: [image] } }, + { kind: "tool-image", index: 0, message: { role: "tool", parts: [image] }, toolName: "read" }, ]); }); @@ -65,7 +65,16 @@ describe("groupChatMessages", () => { expect(groupChatMessages([message])).toEqual([ { kind: "group", startIndex: 0, endIndex: 0, messages: [{ role: "tool", parts: [message.parts[0]], meta }] }, - { kind: "message", index: 0, message: { role: "tool", parts: [image], meta } }, + { kind: "tool-image", index: 0, message: { role: "tool", parts: [image], meta }, toolName: "read" }, + ]); + }); + + it("keeps user images as ordinary messages", () => { + const image = { type: "image" as const, mimeType: "image/png", data: "QUJD" }; + const message: ChatLine = { role: "user", parts: [image] }; + + expect(groupChatMessages([message])).toEqual([ + { kind: "message", index: 0, message }, ]); }); diff --git a/src/client/src/chatGroups.ts b/src/client/src/chatGroups.ts index af06f1a..c536035 100644 --- a/src/client/src/chatGroups.ts +++ b/src/client/src/chatGroups.ts @@ -2,6 +2,7 @@ import type { ChatLine, ChatPart } from "./components/shared"; export type ChatGroup = | { kind: "message"; message: ChatLine; index: number } + | { kind: "tool-image"; message: ChatLine; index: number; toolName?: string } | { kind: "group"; messages: ChatLine[]; startIndex: number; endIndex: number }; export function groupChatMessages(messages: ChatLine[], indexOffset = 0): ChatGroup[] { @@ -29,7 +30,13 @@ export function groupChatMessages(messages: ChatLine[], indexOffset = 0): ChatGr if (readableParts.length) { flushEvents(); const role = readableParts.every((part) => part.type === "skillRead") ? "skill" : message.role; - groups.push({ kind: "message", message: { role, parts: readableParts, ...metadata }, index: absoluteIndex }); + const readableMessage = { role, parts: readableParts, ...metadata }; + if (isToolImageMessage(readableMessage)) { + const toolName = toolNameFromParts(technicalParts); + groups.push({ kind: "tool-image", message: readableMessage, index: absoluteIndex, ...(toolName === undefined ? {} : { toolName }) }); + } else { + groups.push({ kind: "message", message: readableMessage, index: absoluteIndex }); + } } }); flushEvents(); @@ -47,6 +54,17 @@ export function summarizeChatGroup(messages: ChatLine[]): string { return `${String(messages.length)} ${messages.length === 1 ? "event" : "events"}${details !== "" ? ` ยท ${details}` : ""}`; } +function isToolImageMessage(message: ChatLine): boolean { + return message.role === "tool" && message.parts.length > 0 && message.parts.every((part) => part.type === "image"); +} + +function toolNameFromParts(parts: ChatPart[]): string | undefined { + for (const part of parts) { + if ((part.type === "toolCall" || part.type === "toolExecution" || part.type === "toolResult") && part.toolName !== "") return part.toolName; + } + return undefined; +} + function isReadablePart(message: ChatLine, part: ChatPart): boolean { if (message.source === "compaction" || message.source === "branch_summary") return false; if (part.type === "skillInvocation" || part.type === "skillRead" || part.type === "image") return true; diff --git a/src/client/src/chatTranscript.test.ts b/src/client/src/chatTranscript.test.ts index d44bf70..af4585d 100644 --- a/src/client/src/chatTranscript.test.ts +++ b/src/client/src/chatTranscript.test.ts @@ -222,7 +222,7 @@ describe("applyTranscriptEvent", () => { expect(messages).toEqual([finalizedToolLine, textMessage("assistant", "done")]); expect(groupChatMessages(messages)).toEqual([ { kind: "group", startIndex: 0, endIndex: 0, messages: [{ ...finalizedToolLine, parts: [finalizedToolLine.parts[0]] }] }, - { kind: "message", index: 0, message: { ...finalizedToolLine, parts: [finalImage] } }, + { kind: "tool-image", index: 0, message: { ...finalizedToolLine, parts: [finalImage] }, toolName: "read" }, { kind: "message", index: 1, message: textMessage("assistant", "done") }, ]); }); @@ -250,7 +250,7 @@ describe("applyTranscriptEvent", () => { content: [image], }, image], }]); - expect(groupChatMessages(messages).map((group) => group.kind)).toEqual(["group", "message"]); + expect(groupChatMessages(messages).map((group) => group.kind)).toEqual(["group", "tool-image"]); }); it("keeps repeated final tool-result events idempotent", () => { @@ -329,18 +329,18 @@ describe("applyTranscriptEvent", () => { const technicalParts = (groups: ReturnType) => groups.flatMap((group) => group.kind === "group" ? group.messages.flatMap((message) => message.parts.filter((part) => part.type === "toolExecution")) : []); - const visibleImages = (groups: ReturnType) => groups.flatMap((group) => group.kind === "message" + const visibleImages = (groups: ReturnType) => groups.flatMap((group) => group.kind !== "group" ? group.message.parts.filter((part) => part.type === "image") : []); const visibleImageMeta = (groups: ReturnType) => { for (const group of groups) { - if (group.kind === "message" && group.message.parts.some((part) => part.type === "image")) return group.message.meta; + if (group.kind !== "group" && group.message.parts.some((part) => part.type === "image")) return group.message.meta; } return undefined; }; - expect(historyGroups.map((group) => group.kind)).toEqual(["group", "message"]); - expect(liveGroups.map((group) => group.kind)).toEqual(["group", "message"]); + expect(historyGroups.map((group) => group.kind)).toEqual(["group", "tool-image"]); + expect(liveGroups.map((group) => group.kind)).toEqual(["group", "tool-image"]); expect(technicalParts(liveGroups)).toEqual(technicalParts(historyGroups)); expect(visibleImages(liveGroups)).toEqual(visibleImages(historyGroups)); expect(visibleImageMeta(historyGroups)).toEqual({ timestamp }); diff --git a/src/client/src/components/ChatView.image.test.ts b/src/client/src/components/ChatView.image.test.ts index 50929f7..fc787bc 100644 --- a/src/client/src/components/ChatView.image.test.ts +++ b/src/client/src/components/ChatView.image.test.ts @@ -1,7 +1,7 @@ import type { TemplateResult } from "lit"; import { describe, expect, it } from "vitest"; import type { ChatLine } from "./shared"; -import { ChatView } from "./ChatView"; +import { ChatView, chatMessageMetadataLabel } from "./ChatView"; describe("ChatView image rendering", () => { // Direct handler extraction keeps this node-environment test focused on the @@ -24,9 +24,29 @@ describe("ChatView image rendering", () => { expect(scrollCalls).toBe(1); }); + + // Direct rendering keeps this node-environment test focused on the dedicated + // tool-image presentation without introducing a component-wide DOM shim. + it("renders tool images as compact labeled output with final metadata", () => { + const message: ChatLine = { + role: "tool", + parts: [{ type: "image", mimeType: "image/png", data: "QUJD" }], + meta: { timestamp: "2026-07-13T22:00:00.000Z" }, + }; + const rendered = renderToolImageOutput(new ChatView(), message, 7, "read"); + const markup = templateStaticMarkup(rendered); + + expect(markup).toContain('class="msg tool-image-output"'); + expect(markup).not.toContain('class="msg tool"'); + expect(markup).toContain("')).toEqual(["read output"]); + expect(templateValuesAfterMarker(rendered, "title=")).toEqual([chatMessageMetadataLabel(message)]); + expect(templateValuesAfterMarker(rendered, "data-scroll-anchor-id=")).toEqual(["m:7"]); + }); }); type RenderPart = (this: ChatView, part: ChatLine["parts"][number], message?: ChatLine) => TemplateResult; +type RenderToolImageOutput = (this: ChatView, message: ChatLine, index: number, toolName?: string) => TemplateResult; type TemplateEventHandler = (event: Event) => void; function renderPart(view: ChatView, part: ChatLine["parts"][number], message?: ChatLine): TemplateResult { @@ -35,6 +55,12 @@ function renderPart(view: ChatView, part: ChatLine["parts"][number], message?: C return method.call(view, part, message); } +function renderToolImageOutput(view: ChatView, message: ChatLine, index: number, toolName?: string): TemplateResult { + const method: unknown = Reflect.get(view, "renderToolImageOutput"); + if (!isRenderToolImageOutput(method)) throw new Error("ChatView.renderToolImageOutput is not callable"); + return method.call(view, message, index, toolName); +} + function templateEventHandler(template: TemplateResult, marker: string): TemplateEventHandler { const strings = templateStrings(template); const values = templateValues(template); @@ -49,17 +75,48 @@ function isRenderPart(value: unknown): value is RenderPart { return typeof value === "function"; } +function isRenderToolImageOutput(value: unknown): value is RenderToolImageOutput { + return typeof value === "function"; +} + function isTemplateEventHandler(value: unknown): value is TemplateEventHandler { return typeof value === "function"; } function templateStaticMarkup(template: TemplateResult): string { - return templateStrings(template).join(""); + 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 templateValuesAfterMarker(template: TemplateResult, marker: string): unknown[] { - const strings = templateStrings(template); - return templateValues(template).filter((_, index) => strings[index]?.includes(marker) === true); + 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[] { @@ -74,6 +131,10 @@ function templateValues(template: TemplateResult): readonly unknown[] { 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"); } diff --git a/src/client/src/components/ChatView.ts b/src/client/src/components/ChatView.ts index 2afa0cc..067bba7 100644 --- a/src/client/src/components/ChatView.ts +++ b/src/client/src/components/ChatView.ts @@ -205,10 +205,12 @@ export class ChatView extends LitElement { ${this.renderHistoryBoundary()} ${repeat( groups, - (group) => group.kind === "message" ? this.messageAnchorKey(group.index) : this.groupRenderKey(group.startIndex), - (group, index) => group.kind === "message" - ? this.renderMessage(group.message, group.index) - : this.renderMessageGroup(group.messages, group.startIndex, group.endIndex, this.isLiveTailGroup(groups, index)), + (group) => group.kind === "group" ? this.groupRenderKey(group.startIndex) : this.messageAnchorKey(group.index), + (group, index) => { + if (group.kind === "group") return this.renderMessageGroup(group.messages, group.startIndex, group.endIndex, this.isLiveTailGroup(groups, index)); + if (group.kind === "tool-image") return this.renderToolImageOutput(group.message, group.index, group.toolName); + return this.renderMessage(group.message, group.index); + }, )} ${this.renderQueuedMessages()} ${this.renderSessionActivity()} @@ -380,6 +382,17 @@ export class ChatView extends LitElement { `; } + private renderToolImageOutput(message: ChatLine, index: number, toolName?: string) { + const label = toolName === undefined || toolName === "" ? "tool output" : `${toolName} output`; + return html` + ${this.renderScrollMarker(this.messageScrollMarkerId(index))} +
+ ${this.renderMessageHeader(message, String(index), label)} + ${message.parts.map((part) => this.renderPart(part, message))} +
+ `; + } + private isToolExecutionOnlyMessage(message: ChatLine): boolean { return message.role === "tool" && message.parts.length > 0 && message.parts.every((part) => part.type === "toolExecution"); } @@ -419,12 +432,12 @@ export class ChatView extends LitElement { return html``; } - private renderMessageHeader(message: ChatLine, key: string) { + private renderMessageHeader(message: ChatLine, key: string, label: string = message.role) { const meta = this.messageMetaLabel(message); const expanded = this.expandedMetaKey === key; return html`
- ${message.role} + ${label}
${this.renderMessageActions(message, key)} { this.expandedMetaKey = expanded ? undefined : key; }} @keydown=${(event: KeyboardEvent) => { this.onMetaKeydown(event, key, expanded); }}>${meta} diff --git a/src/client/src/components/shared.ts b/src/client/src/components/shared.ts index c8fd3d2..36cb1b3 100644 --- a/src/client/src/components/shared.ts +++ b/src/client/src/components/shared.ts @@ -284,6 +284,7 @@ export const chatStyles = css` .msg.assistant { background: var(--pi-surface); } .msg.user { border-color: var(--pi-accent-border); background: var(--pi-selection-bg); } .msg.tool { border-color: var(--pi-warning-border); background: var(--pi-warning-surface); color: var(--pi-warning); } + .msg.tool-image-output { padding: 0; border: 0; background: transparent; color: var(--pi-text); } .msg.tool-execution-shell { padding: 0; border: 0; background: transparent; color: var(--pi-text); } .msg.system { color: var(--pi-danger); } .msg.bash { border-color: var(--pi-success); background: var(--pi-success-bg); } @@ -326,6 +327,9 @@ export const chatStyles = css` .msg.tool > .msg-header { border-bottom-color: color-mix(in srgb, var(--pi-warning-border) 35%, transparent); background: var(--pi-warning-surface); } .msg.bash > .msg-header { border-bottom-color: color-mix(in srgb, var(--pi-success) 35%, transparent); background: var(--pi-success-bg); } .msg.skill > .msg-header { border-bottom-color: color-mix(in srgb, var(--pi-purple-border) 35%, transparent); background: var(--pi-purple-surface); } + .msg.tool-image-output > .msg-header { position: static; min-height: 18px; margin: 0 0 6px; padding: 0; border: 0; border-radius: 0; background: transparent; box-shadow: none; } + .msg.tool-image-output > .msg-header .label { color: var(--pi-muted); text-transform: none; } + .msg.tool-image-output .chat-image { margin-top: 0; } .group-msg > .msg-header { position: sticky; top: -26px; z-index: 4; margin: -10px 0 8px; padding: 7px 0 6px; border-bottom: 1px solid color-mix(in srgb, var(--pi-border-muted) 35%, transparent); background: var(--pi-bg); } .msg-header-trailing { min-width: 0; flex: 1 1 auto; display: inline-flex; align-items: center; justify-content: flex-end; gap: 8px; } .msg-actions { flex: 0 0 auto; display: inline-flex; gap: 6px; opacity: 0; transition: opacity .12s ease; }