Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 114 additions & 0 deletions src/components/session-viewer/SessionEntryRenderer.test.tsx
Original file line number Diff line number Diff line change
@@ -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 }) => (
<div data-testid="markdown">{content}</div>
),
}))

// 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 (
<div data-testid="user-message">
<div
data-array={String(Array.isArray(content))}
data-types={blocks.map((block: { type?: string }) => block?.type).join(',')}
>
{blocks.map((block: { text?: string }) => block?.text ?? '').join('')}
</div>
</div>
)
},
}))

vi.mock('@/components/messages/AssistantMessage', () => ({
default: ({ content }: { content: unknown }) => {
const blocks = Array.isArray(content) ? content : []
return (
<div data-testid="assistant-message">
<div
data-array={String(Array.isArray(content))}
data-types={blocks.map((block: { type?: string }) => block?.type).join(',')}
>
{blocks.map((block: { text?: string }) => block?.text ?? '').join('')}
</div>
</div>
)
},
}))

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')
})
})
27 changes: 21 additions & 6 deletions src/components/session-viewer/SessionEntryRenderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,27 @@ import type { Content, SessionEntry } from "@/types";

const EMPTY_TOOL_RESULTS = new Map<string, SessionEntry>();

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)
Expand Down Expand Up @@ -51,7 +66,7 @@ export function renderSessionEntry(
return (
<UserMessage
key={entry.id}
content={entry.message.content}
content={asContentParts(entry.message.content)}
timestamp={entry.timestamp}
id={entry.id}
searchQuery={searchQuery}
Expand All @@ -63,7 +78,7 @@ export function renderSessionEntry(
return (
<AssistantMessage
key={entry.id}
content={previewMode ? stripPreviewAssistantContent(entry.message.content) : entry.message.content}
content={previewMode ? stripPreviewAssistantContent(entry.message.content) : asContentParts(entry.message.content)}
timestamp={entry.timestamp}
entryId={entry.id}
toolResultByCallId={toolResultByCallId}
Expand Down
51 changes: 51 additions & 0 deletions src/utils/session.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -668,6 +668,57 @@ describe('parseSessionEntriesWithLineCount', () => {
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);
}
}
});
});


Expand Down
41 changes: 40 additions & 1 deletion src/utils/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -233,7 +272,7 @@ function normalizeSessionEntry(raw: any): SessionEntry | null {
}

if (type === 'message') {
return raw as SessionEntry
return normalizePiMessageEntry(raw)
}

if (
Expand Down