mirror of
https://github.com/AgentSeal/codeburn.git
synced 2026-10-20 11:44:39 +00:00
fix(parser): tolerate string message content; isolate per-file parse failures (#441) (#450)
Some checks are pending
CI / semgrep (push) Waiting to run
Some checks are pending
CI / semgrep (push) Waiting to run
Some agents (Pi, and others for injected turns) write a message's `content` as a string instead of an array of blocks. Parsers did `(content ?? []).filter`, which throws on a string — and because the daily-cache backfill swallowed parse errors, one bad session silently wiped the entire trend/history. - Add normalizeContentBlocks(): string -> one text block, array -> as-is (by reference; drops null/undefined elements so the same crash can't happen one level down), else -> []. Applied across pi/codex/droid/cursor-agent and the Claude path in parser.ts. - Isolate per-file parse failures in parseProviderSources: skip the offending file (warn once per provider) instead of re-throwing and aborting the whole backfill. The stale cache entry is already cleared, so the file is excluded. - Surface backfill failures in hydrateCache via stderr instead of silently returning an empty cache. Likely fixes #425 (previous-day always 0) for the throwing-file cause. Tests: content-utils unit tests + a Pi string-content regression test.
This commit is contained in:
parent
f57ad64ce7
commit
e8009c4559
9 changed files with 150 additions and 17 deletions
26
src/content-utils.ts
Normal file
26
src/content-utils.ts
Normal file
|
|
@ -0,0 +1,26 @@
|
|||
/// Normalize a message's `content` into an array of content blocks.
|
||||
///
|
||||
/// Most agent session formats write `content` as an array of typed blocks
|
||||
/// (`{ type: 'text' | 'tool_use' | ... }`), and the parsers filter over that
|
||||
/// array. But some agents (Pi, and others for programmatically injected turns)
|
||||
/// legitimately write `content` as a plain **string**. A raw string reaching
|
||||
/// `.filter`/`.some` throws a TypeError mid-parse — and because the 365-day
|
||||
/// daily-cache backfill swallows parse errors, that single bad record silently
|
||||
/// wipes the entire trend/history (issue #441).
|
||||
///
|
||||
/// This coerces defensively: arrays pass through, a string becomes one text
|
||||
/// block, and anything else (null/undefined/number/object) becomes empty.
|
||||
export function normalizeContentBlocks<T extends { type?: string; text?: string }>(
|
||||
content: T[] | string | null | undefined,
|
||||
): T[] {
|
||||
if (Array.isArray(content)) {
|
||||
// A clean array (the overwhelming common case) is returned by reference — no
|
||||
// copy. Only when an element is a non-object (null/undefined/primitive) do we
|
||||
// filter, since the call sites read `.type` on each element and a null would
|
||||
// throw — the same crash class this helper exists to prevent, one level down.
|
||||
const isBlock = (b: T): boolean => b != null && typeof b === 'object'
|
||||
return content.every(isBlock) ? content : content.filter(isBlock)
|
||||
}
|
||||
if (typeof content === 'string') return [{ type: 'text', text: content } as T]
|
||||
return []
|
||||
}
|
||||
|
|
@ -2,6 +2,7 @@ import { lstat, readFile, readdir, stat } from 'fs/promises'
|
|||
import { basename, dirname, join, resolve, sep } from 'path'
|
||||
import { readSessionLines } from './fs-utils.js'
|
||||
import { calculateCost, getShortModelName } from './models.js'
|
||||
import { normalizeContentBlocks } from './content-utils.js'
|
||||
import { discoverAllSessions, getProvider } from './providers/index.js'
|
||||
import { flushCodexCache } from './codex-cache.js'
|
||||
import { antigravityCascadeIdFromPath, flushAntigravityCache, shouldReparseAntigravitySource } from './providers/antigravity.js'
|
||||
|
|
@ -1057,9 +1058,13 @@ function parseApiCall(entry: JournalEntry): ParsedApiCall | null {
|
|||
webSearchRequests: usage.server_tool_use?.web_search_requests ?? 0,
|
||||
}
|
||||
|
||||
const tools = extractToolNames(msg.content ?? [])
|
||||
const skills = extractSkillNames(msg.content ?? [])
|
||||
const subagentTypes = extractSubagentTypes(msg.content ?? [])
|
||||
// Defensive: a message whose `content` is a string (not an array of blocks)
|
||||
// would crash the helpers below; normalize so one bad record can't abort the
|
||||
// whole backfill (issue #441).
|
||||
const contentBlocks = normalizeContentBlocks(msg.content)
|
||||
const tools = extractToolNames(contentBlocks)
|
||||
const skills = extractSkillNames(contentBlocks)
|
||||
const subagentTypes = extractSubagentTypes(contentBlocks)
|
||||
const costUSD = calculateCost(
|
||||
msg.model,
|
||||
tokens.inputTokens,
|
||||
|
|
@ -1071,9 +1076,9 @@ function parseApiCall(entry: JournalEntry): ParsedApiCall | null {
|
|||
cacheCreation.oneHourTokens,
|
||||
)
|
||||
|
||||
const bashCmds = extractBashCommandsFromContent(msg.content ?? [])
|
||||
const bashCmds = extractBashCommandsFromContent(contentBlocks)
|
||||
|
||||
const toolSeq: ToolCall[][] = (msg.content ?? [])
|
||||
const toolSeq: ToolCall[][] = contentBlocks
|
||||
.filter((b): b is ToolUseBlock => b.type === 'tool_use')
|
||||
.map(b => {
|
||||
const call: ToolCall = { tool: b.name }
|
||||
|
|
@ -1827,6 +1832,18 @@ function warnProviderReadFailureOnce(providerName: string, err: unknown): void {
|
|||
}
|
||||
}
|
||||
|
||||
function warnProviderParseFailureOnce(providerName: string, sourcePath: string, err: unknown): void {
|
||||
const key = `${providerName}:parse-failure`
|
||||
if (warnedProviderReadFailures.has(key)) return
|
||||
warnedProviderReadFailures.add(key)
|
||||
const msg = err instanceof Error ? err.message : String(err)
|
||||
process.stderr.write(
|
||||
`codeburn: skipping ${providerName} session(s) that failed to parse (${msg}). ` +
|
||||
`First offending file: ${sourcePath}. Further ${providerName} parse failures this run are suppressed; ` +
|
||||
`other sessions still aggregate normally.\n`
|
||||
)
|
||||
}
|
||||
|
||||
async function parseProviderSources(
|
||||
providerName: string,
|
||||
sources: Array<{ path: string; project: string }>,
|
||||
|
|
@ -1900,7 +1917,12 @@ async function parseProviderSources(
|
|||
warnProviderReadFailureOnce(providerName, err)
|
||||
continue
|
||||
}
|
||||
throw err
|
||||
// A single malformed session file must not abort the entire run — that
|
||||
// would silently empty the daily-cache backfill and wipe the trend /
|
||||
// history (issue #441). Skip just this file (its stale cache entry was
|
||||
// already cleared above, so it's excluded) and keep going.
|
||||
warnProviderParseFailureOnce(providerName, source.path, err)
|
||||
continue
|
||||
}
|
||||
}
|
||||
} finally {
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ import { homedir } from 'os'
|
|||
import { readSessionLines } from '../fs-utils.js'
|
||||
import { calculateCost } from '../models.js'
|
||||
import { readCachedCodexResults, writeCachedCodexResults, getCachedCodexProject, fingerprintFile } from '../codex-cache.js'
|
||||
import { normalizeContentBlocks } from '../content-utils.js'
|
||||
import type { ToolCall } from '../types.js'
|
||||
import type { Provider, SessionSource, SessionParser, ParsedProviderCall } from './types.js'
|
||||
|
||||
|
|
@ -405,7 +406,7 @@ function createParser(source: SessionSource, seenKeys: Set<string>): SessionPars
|
|||
}
|
||||
|
||||
if (entry.type === 'response_item' && entry.payload?.type === 'message' && entry.payload?.role === 'user') {
|
||||
const texts = (entry.payload.content ?? [])
|
||||
const texts = normalizeContentBlocks(entry.payload.content)
|
||||
.filter(c => c.type === 'input_text')
|
||||
.map(c => c.text ?? '')
|
||||
.filter(Boolean)
|
||||
|
|
@ -417,7 +418,7 @@ function createParser(source: SessionSource, seenKeys: Set<string>): SessionPars
|
|||
}
|
||||
|
||||
if (entry.type === 'response_item' && entry.payload?.type === 'message' && entry.payload?.role === 'assistant') {
|
||||
const texts = (entry.payload.content ?? [])
|
||||
const texts = normalizeContentBlocks(entry.payload.content)
|
||||
.filter(c => c.type === 'output_text' || c.type === 'text')
|
||||
.map(c => c.text ?? '')
|
||||
pendingOutputChars += texts.join('').length
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ import { homedir } from 'os'
|
|||
|
||||
import { calculateCost } from '../models.js'
|
||||
import { openDatabase, type SqliteDatabase } from '../sqlite.js'
|
||||
import { normalizeContentBlocks } from '../content-utils.js'
|
||||
import type {
|
||||
Provider,
|
||||
SessionSource,
|
||||
|
|
@ -180,7 +181,7 @@ function parseJsonlTranscript(raw: string): { turns: ParsedTurn[]; recognized: b
|
|||
}
|
||||
|
||||
if (entry.role === 'user') {
|
||||
const texts = (entry.message?.content ?? [])
|
||||
const texts = normalizeContentBlocks(entry.message?.content)
|
||||
.filter(c => c.type === 'text')
|
||||
.map(c => c.text ?? '')
|
||||
const combined = texts.join(' ')
|
||||
|
|
@ -189,7 +190,7 @@ function parseJsonlTranscript(raw: string): { turns: ParsedTurn[]; recognized: b
|
|||
}
|
||||
|
||||
if (entry.role === 'assistant' && currentUserMessage) {
|
||||
const content = entry.message?.content ?? []
|
||||
const content = normalizeContentBlocks(entry.message?.content)
|
||||
const bodyParts: string[] = []
|
||||
const tools: string[] = []
|
||||
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ import { homedir } from 'os'
|
|||
import { readSessionFile, readSessionLines } from '../fs-utils.js'
|
||||
import { calculateCost, getShortModelName } from '../models.js'
|
||||
import { extractBashCommands } from '../bash-utils.js'
|
||||
import { normalizeContentBlocks } from '../content-utils.js'
|
||||
import type {
|
||||
Provider,
|
||||
SessionSource,
|
||||
|
|
@ -162,7 +163,7 @@ function createParser(
|
|||
|
||||
if (msg.role === 'user') {
|
||||
// Extract user text from content
|
||||
const texts = (msg.content ?? [])
|
||||
const texts = normalizeContentBlocks(msg.content)
|
||||
.filter(c => c.type === 'text' && c.text)
|
||||
.map(c => c.text!)
|
||||
.filter(Boolean)
|
||||
|
|
@ -175,7 +176,7 @@ function createParser(
|
|||
}
|
||||
|
||||
if (msg.role === 'assistant') {
|
||||
const toolUses = (msg.content ?? []).filter(c => c.type === 'tool_use')
|
||||
const toolUses = normalizeContentBlocks(msg.content).filter(c => c.type === 'tool_use')
|
||||
|
||||
for (const tu of toolUses) {
|
||||
const toolName = tu.name ?? ''
|
||||
|
|
@ -187,7 +188,7 @@ function createParser(
|
|||
}
|
||||
|
||||
// Check if this assistant message has any text content (non-thinking)
|
||||
const hasText = (msg.content ?? []).some(c => c.type === 'text' && c.text)
|
||||
const hasText = normalizeContentBlocks(msg.content).some(c => c.type === 'text' && c.text)
|
||||
|
||||
// Only emit a call entry if there are tools or substantial text
|
||||
if (pendingTools.length > 0 || hasText) {
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ import { homedir } from 'os'
|
|||
import { readSessionFile } from '../fs-utils.js'
|
||||
import { calculateCost } from '../models.js'
|
||||
import { extractBashCommands } from '../bash-utils.js'
|
||||
import { normalizeContentBlocks } from '../content-utils.js'
|
||||
import type { Provider, SessionSource, SessionParser, ParsedProviderCall } from './types.js'
|
||||
|
||||
const modelDisplayNames: Record<string, string> = {
|
||||
|
|
@ -40,7 +41,7 @@ type PiEntry = {
|
|||
cwd?: string
|
||||
message?: {
|
||||
role?: string
|
||||
content?: Array<{ type?: string; text?: string; name?: string; arguments?: Record<string, unknown> }>
|
||||
content?: Array<{ type?: string; text?: string; name?: string; arguments?: Record<string, unknown> }> | string
|
||||
model?: string
|
||||
responseId?: string
|
||||
usage?: {
|
||||
|
|
@ -139,7 +140,7 @@ function createParser(source: SessionSource, seenKeys: Set<string>): SessionPars
|
|||
if (!msg) continue
|
||||
|
||||
if (msg.role === 'user') {
|
||||
const texts = (msg.content ?? [])
|
||||
const texts = normalizeContentBlocks(msg.content)
|
||||
.filter(c => c.type === 'text')
|
||||
.map(c => c.text ?? '')
|
||||
.filter(Boolean)
|
||||
|
|
@ -166,7 +167,7 @@ function createParser(source: SessionSource, seenKeys: Set<string>): SessionPars
|
|||
if (seenKeys.has(dedupKey)) continue
|
||||
seenKeys.add(dedupKey)
|
||||
|
||||
const toolCalls = (msg.content ?? []).filter(c => c.type === 'toolCall' && c.name)
|
||||
const toolCalls = normalizeContentBlocks(msg.content).filter(c => c.type === 'toolCall' && c.name)
|
||||
const tools = toolCalls.map(c => toolNameMap[c.name!] ?? c.name!)
|
||||
const bashCommands = toolCalls
|
||||
.filter(c => c.name === 'bash')
|
||||
|
|
|
|||
|
|
@ -54,7 +54,15 @@ async function hydrateCache(): Promise<DailyCache> {
|
|||
(range) => parseAllSessions(range, 'all'),
|
||||
aggregateProjectsIntoDays,
|
||||
)
|
||||
} catch {
|
||||
} catch (err) {
|
||||
// Previously swallowed silently, which turned any backfill failure into an
|
||||
// empty trend/history with no signal (issue #441). Per-file parse errors no
|
||||
// longer reach here (they're isolated in parseProviderSources), so anything
|
||||
// that does is exceptional and worth surfacing.
|
||||
process.stderr.write(
|
||||
`codeburn: daily history backfill failed; the trend chart may be incomplete. ` +
|
||||
`${err instanceof Error ? err.message : String(err)}\n`
|
||||
)
|
||||
return emptyCache()
|
||||
}
|
||||
}
|
||||
|
|
|
|||
43
tests/content-utils.test.ts
Normal file
43
tests/content-utils.test.ts
Normal file
|
|
@ -0,0 +1,43 @@
|
|||
import { describe, it, expect } from 'vitest'
|
||||
import { normalizeContentBlocks } from '../src/content-utils.js'
|
||||
|
||||
describe('normalizeContentBlocks', () => {
|
||||
it('passes an array of blocks through unchanged', () => {
|
||||
const blocks = [{ type: 'text', text: 'hi' }, { type: 'tool_use', text: '' }]
|
||||
expect(normalizeContentBlocks(blocks)).toBe(blocks)
|
||||
})
|
||||
|
||||
it('wraps a string as a single text block (the issue #441 case)', () => {
|
||||
expect(normalizeContentBlocks('hello world')).toEqual([{ type: 'text', text: 'hello world' }])
|
||||
})
|
||||
|
||||
it('returns an empty array for null / undefined', () => {
|
||||
expect(normalizeContentBlocks(null)).toEqual([])
|
||||
expect(normalizeContentBlocks(undefined)).toEqual([])
|
||||
})
|
||||
|
||||
it('returns an empty array for other non-array values', () => {
|
||||
// Defensive against corrupt records: a number/object content must not throw downstream.
|
||||
expect(normalizeContentBlocks(42 as unknown as string)).toEqual([])
|
||||
expect(normalizeContentBlocks({ type: 'text' } as unknown as string)).toEqual([])
|
||||
})
|
||||
|
||||
it('drops null/undefined elements inside an array (avoids the same crash one level down)', () => {
|
||||
const dirty = [{ type: 'text', text: 'ok' }, null, undefined, { type: 'tool_use' }] as unknown as Array<{ type?: string }>
|
||||
const out = normalizeContentBlocks(dirty)
|
||||
expect(out).toEqual([{ type: 'text', text: 'ok' }, { type: 'tool_use' }])
|
||||
expect(() => out.filter(b => b.type === 'text')).not.toThrow()
|
||||
})
|
||||
|
||||
it('returns the same reference for a clean array (no copy)', () => {
|
||||
const clean = [{ type: 'text', text: 'a' }, { type: 'tool_use' }]
|
||||
expect(normalizeContentBlocks(clean)).toBe(clean)
|
||||
})
|
||||
|
||||
it('the result is always safe to .filter/.some over', () => {
|
||||
const inputs = ['a string', null, undefined, [{ type: 'text' }], [{ type: 'text' }, null]] as const
|
||||
for (const input of inputs) {
|
||||
expect(() => normalizeContentBlocks(input as never).filter(b => b.type === 'text')).not.toThrow()
|
||||
}
|
||||
})
|
||||
})
|
||||
|
|
@ -189,6 +189,36 @@ describe('pi provider - JSONL parsing', () => {
|
|||
expect(call.deduplicationKey).toContain('resp-abc')
|
||||
})
|
||||
|
||||
it('does not crash when a user message content is a string instead of an array (issue #441)', async () => {
|
||||
const projectDir = join(tmpDir, '--Users-test-myproject--')
|
||||
// Pi legitimately writes string `content` for some (e.g. injected) user turns.
|
||||
// Before the fix this threw `content.filter is not a function`, which aborted
|
||||
// the whole backfill and silently emptied the trend/history.
|
||||
const stringContentUser = JSON.stringify({
|
||||
type: 'message',
|
||||
id: 'msg-user-str',
|
||||
timestamp: '2026-04-14T10:00:10.000Z',
|
||||
message: { role: 'user', content: 'test message from file watcher', timestamp: 1776023210000 },
|
||||
})
|
||||
const filePath = await writeSession(projectDir, 'session.jsonl', [
|
||||
sessionMeta({ id: 'sess-str', cwd: '/Users/test/myproject' }),
|
||||
stringContentUser,
|
||||
assistantMessage({ responseId: 'resp-str', input: 500, output: 80 }),
|
||||
])
|
||||
|
||||
const provider = createPiProvider(tmpDir)
|
||||
const source = { path: filePath, project: 'myproject', provider: 'pi' }
|
||||
const calls: ParsedProviderCall[] = []
|
||||
for await (const call of provider.createSessionParser(source, new Set()).parse()) {
|
||||
calls.push(call)
|
||||
}
|
||||
|
||||
// The assistant turn is still parsed, and the string user content is paired.
|
||||
expect(calls).toHaveLength(1)
|
||||
expect(calls[0]!.inputTokens).toBe(500)
|
||||
expect(calls[0]!.userMessage).toBe('test message from file watcher')
|
||||
})
|
||||
|
||||
it('collects tool names from toolCall content items', async () => {
|
||||
const projectDir = join(tmpDir, '--Users-test-myproject--')
|
||||
const filePath = await writeSession(projectDir, 'session.jsonl', [
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue