Skip to content

Commit 8874a99

Browse files
Bill LeoutsakosBill Leoutsakos
authored andcommitted
fix(pi): clean plan mode output
1 parent 07949ce commit 8874a99

10 files changed

Lines changed: 128 additions & 8 deletions

File tree

apps/sim/blocks/blocks/pi.test.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,12 @@ describe('Pi cloud authoring surface', () => {
102102
])
103103
})
104104

105+
it('documents each mode label with its serialized ID', () => {
106+
expect(PiBlock.inputs.mode.description).toBe(
107+
'Execution mode: Plan (cloud_plan), Create PR (cloud), Update PR (cloud_branch), Review Code (cloud_review), or Local Dev (local)'
108+
)
109+
})
110+
105111
it.each(['cloud', 'cloud_branch'])(
106112
'declares Babysit controls and outputs for %s',
107113
(authoringMode) => {

apps/sim/blocks/blocks/pi.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -620,7 +620,8 @@ export const PiBlock: BlockConfig<PiResponse> = {
620620
inputs: {
621621
mode: {
622622
type: 'string',
623-
description: 'Execution mode: Plan, Create PR, Update PR, Review Code, or Local Dev',
623+
description:
624+
'Execution mode: Plan (cloud_plan), Create PR (cloud), Update PR (cloud_branch), Review Code (cloud_review), or Local Dev (local)',
624625
},
625626
task: { type: 'string', description: 'Instruction for the coding agent' },
626627
model: { type: 'string', description: 'AI model to use' },

apps/sim/executor/handlers/pi/cloud/plan/backend.test.ts

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,53 @@ describe('runCloudPlanPi', () => {
129129
expect(mockRun.mock.calls[0][1].envs.BASE_BRANCH).toBe('')
130130
})
131131

132+
it('returns only the final assistant response while preserving live progress events', async () => {
133+
mockRun.mockImplementation(
134+
(command: string, options: { onStdout?: (chunk: string) => void }) => {
135+
if (command.includes('git clone')) {
136+
return Promise.resolve({ stdout: '', stderr: '', exitCode: 0 })
137+
}
138+
options.onStdout?.(
139+
`${[
140+
JSON.stringify({
141+
type: 'message_update',
142+
assistantMessageEvent: { type: 'text_delta', delta: 'Inspecting files...' },
143+
}),
144+
JSON.stringify({
145+
type: 'agent_end',
146+
messages: [
147+
{
148+
role: 'assistant',
149+
stopReason: 'stop',
150+
content: [{ type: 'text', text: 'Inspecting files...' }],
151+
},
152+
{
153+
role: 'assistant',
154+
stopReason: 'stop',
155+
content: [
156+
{ type: 'thinking', thinking: 'Hidden reasoning' },
157+
{ type: 'text', text: '# Final Plan\n\n1. Make the change.' },
158+
],
159+
},
160+
],
161+
}),
162+
].join('\n')}\n`
163+
)
164+
return Promise.resolve({ stdout: '', stderr: '', exitCode: 0 })
165+
}
166+
)
167+
const onEvent = vi.fn()
168+
169+
const result = await runCloudPlanPi(params(), { onEvent })
170+
171+
expect(onEvent).toHaveBeenCalledWith({ type: 'text', text: 'Inspecting files...' })
172+
expect(onEvent).toHaveBeenCalledWith({
173+
type: 'final',
174+
text: '# Final Plan\n\n1. Make the change.',
175+
})
176+
expect(result.totals.finalText).toBe('# Final Plan\n\n1. Make the change.')
177+
})
178+
132179
it('loads only the Sim search extension and scopes its key to the Pi command', async () => {
133180
await runCloudPlanPi(params({ search: { provider: 'exa', apiKey: 'exa-secret' } }), {
134181
onEvent: vi.fn(),

apps/sim/executor/handlers/pi/cloud/plan/backend.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,9 @@ export const runCloudPlanPi: PiBackendRun<PiCloudPlanRunParams> = async (params,
106106
const event = scrubPiEvent(raw, secrets)
107107
if (!event) return
108108
applyPiEvent(totals, event)
109+
if (event.type === 'final' && event.text) {
110+
totals.finalText = event.text
111+
}
109112
context.onEvent(event)
110113
}
111114
const handleChunk = (chunk: string) => {

apps/sim/executor/handlers/pi/core/events.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,32 @@ describe('normalizePiEvent', () => {
103103
})
104104
})
105105

106+
it('uses only text blocks from the last assistant message as final text', () => {
107+
expect(
108+
normalizePiEvent({
109+
type: 'agent_end',
110+
messages: [
111+
{
112+
role: 'assistant',
113+
stopReason: 'stop',
114+
content: [{ type: 'text', text: 'Earlier narration' }],
115+
},
116+
{ role: 'toolResult', content: [{ type: 'text', text: 'Tool output' }] },
117+
{
118+
role: 'assistant',
119+
stopReason: 'stop',
120+
content: [
121+
{ type: 'thinking', thinking: 'Hidden reasoning' },
122+
{ type: 'text', text: '# Plan' },
123+
{ type: 'toolCall', name: 'read' },
124+
{ type: 'text', text: 'Do it' },
125+
],
126+
},
127+
],
128+
})
129+
).toEqual({ type: 'final', text: '# Plan\nDo it' })
130+
})
131+
106132
it('returns other for unknown types and null for non-objects', () => {
107133
expect(normalizePiEvent({ type: 'queue_update' })).toEqual({ type: 'other' })
108134
expect(normalizePiEvent('nope')).toBeNull()

apps/sim/executor/handlers/pi/core/events.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,17 @@ function asNumber(value: unknown): number {
8383
return typeof value === 'number' && Number.isFinite(value) ? value : 0
8484
}
8585

86+
function extractAssistantText(message: Record<string, unknown>): string {
87+
if (!Array.isArray(message.content)) return ''
88+
return message.content
89+
.map((block) => asRecord(block))
90+
.filter((block): block is Record<string, unknown> => block !== null)
91+
.filter((block) => asString(block.type) === 'text')
92+
.map((block) => asString(block.text))
93+
.filter(Boolean)
94+
.join('\n')
95+
}
96+
8697
/**
8798
* Extracts token usage from an event, tolerating the field names Pi and common
8899
* provider payloads use (`input`/`output`, `inputTokens`/`outputTokens`,
@@ -149,7 +160,8 @@ export function normalizePiEvent(raw: unknown): PiEvent | null {
149160
message: asString(message.errorMessage) || `Pi request ${stopReason}`,
150161
}
151162
}
152-
break
163+
const text = extractAssistantText(message)
164+
return text ? { type: 'final', text } : { type: 'final' }
153165
}
154166
return { type: 'final' }
155167
}

apps/sim/executor/handlers/pi/core/redaction.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@ describe('Pi secret redaction', () => {
3232
type: 'error',
3333
message: 'failed ***',
3434
})
35+
expect(scrubPiEvent({ type: 'final', text: 'plan sk-hosted' }, ['sk-hosted'])).toEqual({
36+
type: 'final',
37+
text: 'plan ***',
38+
})
3539
})
3640

3741
it('creates sanitized errors without retaining the raw cause', () => {

apps/sim/executor/handlers/pi/core/redaction.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ export function scrubPiEvent(event: PiEvent | null, secrets: readonly string[]):
2222
case 'text':
2323
case 'thinking':
2424
return { ...event, text: scrubPiSecrets(event.text, secrets) }
25+
case 'final':
26+
return event.text ? { ...event, text: scrubPiSecrets(event.text, secrets) } : event
2527
case 'tool_start':
2628
case 'tool_end':
2729
return { ...event, toolName: scrubPiSecrets(event.toolName, secrets) }

apps/sim/executor/handlers/pi/pi-handler.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -367,8 +367,6 @@ describe('PiBlockHandler', () => {
367367
expect(output).toMatchObject({
368368
content: '# Plan\nDo it',
369369
model: 'claude',
370-
changedFiles: [],
371-
diff: '',
372370
tokens: { input: 3, output: 4, total: 7 },
373371
cost: { input: 0, output: 0, total: 0 },
374372
providerTiming: {
@@ -379,6 +377,8 @@ describe('PiBlockHandler', () => {
379377
})
380378
expect(output).not.toHaveProperty('prUrl')
381379
expect(output).not.toHaveProperty('branch')
380+
expect(output).not.toHaveProperty('changedFiles')
381+
expect(output).not.toHaveProperty('diff')
382382
})
383383

384384
it('routes cloud_review mode and surfaces review output', async () => {

apps/sim/executor/handlers/pi/pi-handler.ts

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -439,6 +439,7 @@ export class PiBlockHandler implements BlockHandler {
439439

440440
private buildOutput(
441441
result: PiRunResult,
442+
mode: PiRunParams['mode'],
442443
model: string,
443444
isBYOK: boolean,
444445
startTime: number,
@@ -449,8 +450,12 @@ export class PiBlockHandler implements BlockHandler {
449450
return {
450451
content: totals.finalText,
451452
model,
452-
changedFiles: result.changedFiles ?? [],
453-
diff: result.diff ?? '',
453+
...(mode === 'cloud_plan'
454+
? {}
455+
: {
456+
changedFiles: result.changedFiles ?? [],
457+
diff: result.diff ?? '',
458+
}),
454459
...(result.prUrl ? { prUrl: result.prUrl } : {}),
455460
...(result.branch ? { branch: result.branch } : {}),
456461
...(result.reviewUrl ? { reviewUrl: result.reviewUrl } : {}),
@@ -516,7 +521,14 @@ export class PiBlockHandler implements BlockHandler {
516521
}
517522
Object.assign(
518523
output,
519-
this.buildOutput(result, params.model, params.isBYOK, startTime, startTimeISO)
524+
this.buildOutput(
525+
result,
526+
params.mode,
527+
params.model,
528+
params.isBYOK,
529+
startTime,
530+
startTimeISO
531+
)
520532
)
521533
if (memoryConfig) {
522534
await appendPiMemory(
@@ -558,6 +570,13 @@ export class PiBlockHandler implements BlockHandler {
558570
result.memoryText ?? result.totals.finalText
559571
)
560572
}
561-
return this.buildOutput(result, params.model, params.isBYOK, startTime, startTimeISO)
573+
return this.buildOutput(
574+
result,
575+
params.mode,
576+
params.model,
577+
params.isBYOK,
578+
startTime,
579+
startTimeISO
580+
)
562581
}
563582
}

0 commit comments

Comments
 (0)