diff --git a/src/components/session-viewer/SessionEntryRenderer.test.tsx b/src/components/session-viewer/SessionEntryRenderer.test.tsx new file mode 100644 index 00000000..988ff2c7 --- /dev/null +++ b/src/components/session-viewer/SessionEntryRenderer.test.tsx @@ -0,0 +1,114 @@ +// @vitest-environment jsdom +// +// Regression tests for the app-wide crash on Pi sessions whose messages store +// `content` as a plain string (e.g. `role: "system"`): +// +// TypeError: e.filter is not a function +// at contentToText (SessionEntryRenderer.tsx) +// +// The entry renderer is the last line of defence before a bad payload trips the +// app-wide error boundary, so it must coerce non-array content itself. +import { render, screen } from '@testing-library/react' +import { describe, expect, it, vi } from 'vitest' + +vi.mock('react-i18next', () => ({ + initReactI18next: { type: '3rdParty', init: () => {} }, + useTranslation: () => ({ + t: (key: string, fallback?: string) => fallback ?? key, + }), +})) + +vi.mock('@/components/ui/MarkdownContent', () => ({ + default: ({ content }: { content: string }) => ( +
{content}
+ ), +})) + +// Leaf renderers become prop probes: these tests assert on the content that the +// entry renderer coerces before handing it over. +vi.mock('@/components/messages/UserMessage', () => ({ + default: ({ content }: { content: unknown }) => { + const blocks = Array.isArray(content) ? content : [] + return ( +
+
block?.type).join(',')} + > + {blocks.map((block: { text?: string }) => block?.text ?? '').join('')} +
+
+ ) + }, +})) + +vi.mock('@/components/messages/AssistantMessage', () => ({ + default: ({ content }: { content: unknown }) => { + const blocks = Array.isArray(content) ? content : [] + return ( +
+
block?.type).join(',')} + > + {blocks.map((block: { text?: string }) => block?.text ?? '').join('')} +
+
+ ) + }, +})) + +import { renderSessionEntry } from './SessionEntryRenderer' +import type { SessionEntry } from '@/types' + +function messageEntry(role: string, content: unknown): SessionEntry { + return { + type: 'message', + id: `${role}-1`, + timestamp: '2026-04-09T10:00:00Z', + message: { role, content }, + } as unknown as SessionEntry +} + +function probeNode(testId: string): HTMLElement { + return screen.getByTestId(testId).firstElementChild as HTMLElement +} + +describe('renderSessionEntry string content coercion', () => { + it('renders a system message whose content is a raw string', () => { + render(<>{renderSessionEntry(messageEntry('system', 'You are a helpful assistant.'))}) + + expect(screen.getByTestId('markdown').textContent).toBe('You are a helpful assistant.') + }) + + it('renders a developer message whose content is a raw string', () => { + render(<>{renderSessionEntry(messageEntry('developer', 'Follow the house style.'))}) + + expect(screen.getByTestId('markdown').textContent).toBe('Follow the house style.') + }) + + it('does not throw for an empty string body', () => { + render(<>{renderSessionEntry(messageEntry('system', ''))}) + + expect(screen.getByTestId('markdown').textContent).toBe('') + }) + + it('coerces user message content into content parts', () => { + render(<>{renderSessionEntry(messageEntry('user', 'hello there'))}) + + expect(probeNode('user-message').getAttribute('data-array')).toBe('true') + expect(probeNode('user-message').getAttribute('data-types')).toBe('text') + expect(screen.getByTestId('user-message').textContent).toBe('hello there') + }) + + it('coerces assistant preview content into content parts', () => { + render( + <> + {renderSessionEntry(messageEntry('assistant', 'streamed text'), new Map(), '', false, true)} + , + ) + + expect(probeNode('assistant-message').getAttribute('data-array')).toBe('true') + expect(screen.getByTestId('assistant-message').textContent).toBe('streamed text') + }) +}) diff --git a/src/components/session-viewer/SessionEntryRenderer.tsx b/src/components/session-viewer/SessionEntryRenderer.tsx index 9ece8879..5dbaab8d 100644 --- a/src/components/session-viewer/SessionEntryRenderer.tsx +++ b/src/components/session-viewer/SessionEntryRenderer.tsx @@ -13,12 +13,27 @@ import type { Content, SessionEntry } from "@/types"; const EMPTY_TOOL_RESULTS = new Map(); -function stripPreviewAssistantContent(content: Content[]): Content[] { - return content.filter((item) => item.type === "text"); +/** + * Renderers must not crash the whole app on unexpected session data. Pi JSONL + * is not schema validated and some roles persist `content` as a plain string; + * live (`get_pi_agent_entries`) entries bypass the JSONL parser entirely. + * Coerce non-array payloads into content parts so a message body still renders + * instead of tripping the app-wide error boundary. + */ +function asContentParts(content: unknown): Content[] { + if (Array.isArray(content)) return content as Content[]; + if (typeof content === "string" && content.length > 0) { + return [{ type: "text", text: content }]; + } + return []; +} + +function stripPreviewAssistantContent(content: unknown): Content[] { + return asContentParts(content).filter((item) => item.type === "text"); } -function contentToText(content: Content[]): string { - return content +function contentToText(content: unknown): string { + return asContentParts(content) .filter((item) => item.type === "text" && typeof item.text === "string") .map((item) => item.text?.trim()) .filter(Boolean) @@ -51,7 +66,7 @@ export function renderSessionEntry( return ( { const { entries } = parseSessionEntriesWithLineCount(content); expect(entries.filter((e) => e.message?.role === 'assistant')).toHaveLength(2); }); + + it('coerces string message content into text parts', () => { + const content = [ + JSON.stringify({ + type: 'message', + id: 'system-1', + timestamp: '2026-04-09T10:00:00Z', + message: { role: 'system', content: 'You are a helpful assistant.' }, + }), + JSON.stringify({ + type: 'message', + id: 'user-1', + parentId: 'system-1', + timestamp: '2026-04-09T10:01:00Z', + message: { role: 'user', content: 'hello' }, + }), + JSON.stringify({ + type: 'message', + id: 'system-2', + parentId: 'user-1', + timestamp: '2026-04-09T10:02:00Z', + message: { role: 'system', content: '' }, + }), + JSON.stringify({ + type: 'message', + id: 'assistant-1', + parentId: 'system-2', + timestamp: '2026-04-09T10:03:00Z', + message: { role: 'assistant', content: [{ type: 'text', text: 'already parts' }] }, + }), + ].join('\n'); + + const { entries } = parseSessionEntriesWithLineCount(content); + + // Pi persists some roles (system/developer) with a plain string body, but + // every consumer assumes `Content[]`. The parser has to coerce it, otherwise + // renderers crash on `.filter(...)` ("e.filter is not a function"). + expect(entries[0].message?.content).toEqual([{ type: 'text', text: 'You are a helpful assistant.' }]); + expect(entries[1].message?.content).toEqual([{ type: 'text', text: 'hello' }]); + expect(entries[2].message?.content).toEqual([]); + + // Already-array content stays untouched. + expect(entries[3].message?.content).toEqual([{ type: 'text', text: 'already parts' }]); + + // Invariant: nothing that reaches the UI carries a non-array body. + for (const entry of entries) { + if (entry.message) { + expect(Array.isArray(entry.message.content)).toBe(true); + } + } + }); }); diff --git a/src/utils/session.ts b/src/utils/session.ts index 9d94c125..b2161df8 100644 --- a/src/utils/session.ts +++ b/src/utils/session.ts @@ -210,6 +210,45 @@ function generateFallbackId(prefix: string): string { return `${prefix}-${base}` } +/** + * Coerce a Pi message body into content parts. + * + * Pi stores most messages as a `Content[]`, but it also writes some roles + * (e.g. `system` / `developer`) with a plain string body. Every consumer of + * `entry.message.content` (renderers, previews, stats) assumes an array, so + * non-array payloads are coerced once at the parse boundary instead of being + * defended against in each consumer. + */ +function normalizePiContent(value: unknown): Content[] { + if (Array.isArray(value)) return value as Content[] + + if (typeof value === 'string') { + return value.length > 0 ? [{ type: 'text', text: value }] : [] + } + + if (value && typeof value === 'object') { + return [value as Content] + } + + return [] +} + +/** + * Pi writes `role: "system"` (and occasionally `"developer"`) messages with + * `message.content` as a string. Normalize it so renderers never receive a + * non-array and crash on `.filter(...)`. Already-array content is returned + * untouched. + */ +function normalizePiMessageEntry(raw: any): SessionEntry { + const message = raw?.message + + if (message && typeof message === 'object' && !Array.isArray(message.content)) { + message.content = normalizePiContent(message.content) + } + + return raw as SessionEntry +} + function normalizeSessionEntry(raw: any): SessionEntry | null { if (!raw || typeof raw !== 'object') return null const type = typeof raw.type === 'string' ? raw.type : undefined @@ -233,7 +272,7 @@ function normalizeSessionEntry(raw: any): SessionEntry | null { } if (type === 'message') { - return raw as SessionEntry + return normalizePiMessageEntry(raw) } if (