fix(native-chat): keep capped and error provider frames visible (#15142)

Two no-silent-drop violations in the unhandled provider frame path:

- The per-turn generic-row cap silently discarded every frame past eight.
  Capped frames now upsert one per-turn summary row with an exact count,
  and error-surface frames are exempt from the cap entirely.
- classifyProviderFrame ran the delta name-suffix heuristic before payload
  error inspection, so an unknown future delta method carrying an error
  vanished. Error inspection now outranks the name-shape heuristic.
This commit is contained in:
Brennan Benson
2026-08-17 13:58:28 -07:00
committed by GitHub
parent 81c960a137
commit 98039f922c
6 changed files with 81 additions and 11 deletions
@@ -273,7 +273,9 @@ export function codexJournalItem(item: CodexThreadItem): CodexJournalItem {
}
}
const unhandled = unhandledProviderFrameJournalItem('codex', `item:${item.type}`, item)
return unhandled ? { ...unhandled, handled: false } : { body: null, blobs: [], handled: true }
return unhandled
? { body: unhandled.body, blobs: unhandled.blobs, handled: false }
: { body: null, blobs: [], handled: true }
}
export function codexItemBody(item: CodexThreadItem): AgentJournalItemBody | null {
@@ -502,7 +502,7 @@ describe('codex journal translation', () => {
).toEqual(['notification:future/notification', 'request:future/request', 'frame:unclassified'])
})
it('bounds generic rows per turn while preserving unknown non-delta evidence', () => {
it('bounds generic rows per turn while keeping the suppression visible and countable', () => {
const { translator, tap } = translatorWith()
translator.handle(TURN_STARTED)
for (let index = 0; index < MAX_CODEX_GENERIC_ROWS_PER_TURN + 20; index += 1) {
@@ -510,11 +510,26 @@ describe('codex journal translation', () => {
}
translator.handle(notification('item/future/outputDelta', { itemId: 'future', delta: 'x' }))
expect(tap.rows).toHaveLength(MAX_CODEX_GENERIC_ROWS_PER_TURN)
expect(tap.rows[0]?.body).toMatchObject({
const generic = tap.rows.filter(
(row) => row.body.kind === 'status' && row.body.providerFrame !== undefined
)
expect(generic).toHaveLength(MAX_CODEX_GENERIC_ROWS_PER_TURN)
expect(generic[0]?.body).toMatchObject({
kind: 'status',
providerFrame: { kind: 'notification:future/notification' }
})
// The 20 capped frames reduce to ONE summary row whose count is exact, so
// suppressed provider activity is never invisible.
const summaries = new Map(
tap.rows
.filter((row) => row.key.includes('provider-frame-suppressed'))
.map((row) => [row.key, row.body])
)
expect(summaries.size).toBe(1)
expect([...summaries.values()][0]).toEqual({
kind: 'status',
text: '20 more provider notifications not shown for this turn'
})
expect(
tap.rows.some(
(row) =>
@@ -524,6 +539,23 @@ describe('codex journal translation', () => {
).toBe(false)
})
it('never lets the generic-row cap hide an error frame', () => {
const { translator, tap } = translatorWith()
translator.handle(TURN_STARTED)
for (let index = 0; index < MAX_CODEX_GENERIC_ROWS_PER_TURN + 3; index += 1) {
translator.handle(notification('future/notification', { value: index }))
}
translator.handle(notification('future/failure', { error: 'provider exploded' }))
expect(
tap.rows.some(
(row) =>
row.body.kind === 'status' &&
row.body.providerFrame?.kind === 'notification:future/failure'
)
).toBe(true)
})
it('keeps a fresh session timeline empty through startup and status notifications', () => {
const { translator, tap } = translatorWith()
@@ -66,6 +66,7 @@ export function createCodexJournalTranslator(
const details = new Map<string, string>()
const currentTurnIds = new Map<string, string>()
const genericRowsByTurn = new Map<string, number>()
const suppressedRowsByTurn = new Map<string, number>()
let fallbackSequence = 0
const appendUnhandled = (kind: string, payload: unknown, threadId = 'session'): void => {
@@ -76,7 +77,21 @@ export function createCodexJournalTranslator(
const turnId = readCodexTurnId(payload) ?? currentTurnIds.get(threadId) ?? 'outside-turn'
const bucket = `${encodeURIComponent(threadId)}:${encodeURIComponent(turnId)}`
const rowCount = genericRowsByTurn.get(bucket) ?? 0
if (rowCount >= MAX_CODEX_GENERIC_ROWS_PER_TURN) {
// The cap bounds noise, never evidence: an error frame is always journaled,
// and capped frames stay countable through one summary row per turn.
const capped =
rowCount >= MAX_CODEX_GENERIC_ROWS_PER_TURN && translated.classification !== 'error-surface'
if (capped) {
const suppressed = (suppressedRowsByTurn.get(bucket) ?? 0) + 1
suppressedRowsByTurn.set(bucket, suppressed)
deps.sink.appendItem(
{ provider: 'orca', clientMessageId: `provider-frame-suppressed:codex:${bucket}` },
{
kind: 'status',
text: `${suppressed} more provider notification${suppressed === 1 ? '' : 's'} not shown for this turn`
}
)
deps.sink.publish()
return
}
genericRowsByTurn.set(bucket, rowCount + 1)
@@ -276,6 +291,7 @@ export function createCodexJournalTranslator(
details.clear()
currentTurnIds.clear()
genericRowsByTurn.clear()
suppressedRowsByTurn.clear()
}
}
}
@@ -225,12 +225,15 @@ export function classifyProviderFrame(
kind: string,
payload: unknown
): ProviderFrameClassification {
if (isDeltaShapedProviderFrameKind(kind)) {
return 'stream-into-item'
}
// Payload failure inspection outranks the name-shape heuristic below: an
// unknown frame that reports an error must reach the user even when its
// method name happens to look like a stream delta.
if (hasProviderError(payload)) {
return 'error-surface'
}
if (isDeltaShapedProviderFrameKind(kind)) {
return 'stream-into-item'
}
if (provider === 'claude' && kind === 'message:result') {
const subtype =
typeof payload === 'object' && payload !== null
@@ -73,7 +73,7 @@ describe('unhandled provider frame journal fallback', () => {
).toBeNull()
})
it('never creates generic rows for known or unknown delta-shaped frames', () => {
it('never creates generic rows for delta-shaped frames that report no failure', () => {
expect(
unhandledProviderFrameJournalItem('codex', 'notification:item/commandExecution/outputDelta', {
itemId: 'exec-1',
@@ -82,11 +82,25 @@ describe('unhandled provider frame journal fallback', () => {
).toBeNull()
expect(
unhandledProviderFrameJournalItem('codex', 'notification:item/future/outputDelta', {
error: 'still a delta'
itemId: 'future-1',
delta: 'y'
})
).toBeNull()
})
it('surfaces an unknown delta-shaped frame whose payload reports an error', () => {
const row = unhandledProviderFrameJournalItem('codex', 'notification:item/future/outputDelta', {
error: 'stream broke mid-item'
})
expect(row).not.toBeNull()
expect(row?.classification).toBe('error-surface')
expect(row?.body.providerFrame).toMatchObject({
provider: 'codex',
kind: 'notification:item/future/outputDelta'
})
})
it('renders codex systemError and Claude error result variants', () => {
const codex = unhandledProviderFrameJournalItem('codex', 'notification:thread/status/changed', {
threadId: 'thread-1',
@@ -10,6 +10,8 @@ import { classifyProviderFrame } from './provider-frame-disposition'
export type UnhandledProviderFrameJournalItem = {
body: AgentJournalStatusItem
blobs: { digest: string; payload: string }[]
/** Why the frame surfaced. Error frames are exempt from generic-row caps. */
classification: 'timeline-substantive' | 'error-surface'
}
function serializeProviderPayload(payload: unknown): string {
@@ -85,6 +87,7 @@ export function unhandledProviderFrameJournalItem(
text: display?.text ?? `${provider} · ${kind}`,
providerFrame: { provider, kind, payload: bounded }
},
blobs: bounded.truncated ? [{ digest: bounded.digest, payload: serialized }] : []
blobs: bounded.truncated ? [{ digest: bounded.digest, payload: serialized }] : [],
classification: classification === 'error-surface' ? 'error-surface' : 'timeline-substantive'
}
}