Skip to content

Commit bcb81b9

Browse files
authored
Refactor: Remove dead code from file tool handling (#68)
* Remove: Dead code from file tool handling - Delete resolving-pdf-file.ts and resolving-image-file.ts - Remove ResolvingPdfFile/ResolvingImageFile states from state machine - Remove resolvePdfFile/resolveImageFile events from @perstack/core - Remove dead cases from TUI/CLI event handlers - Update README.md state machine diagram * Test: Add multi-modal E2E tests for PDF and image reading Verify that readPdfFile and readImageFile correctly process files by having experts summarize/describe the content. * Chore: Add empty changeset for refactoring
1 parent 3b64f88 commit bcb81b9

File tree

13 files changed

+143
-639
lines changed

13 files changed

+143
-639
lines changed

.changeset/twenty-dragons-kick.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
---
2+
---

e2e/README.md

Lines changed: 34 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,21 @@ e2e/
3333
│ ├── mixed-tools.toml # MCP + Delegate + Interactive
3434
│ ├── parallel-mcp.toml # Parallel MCP calls
3535
│ ├── delegate-chain.toml # Delegation chain
36-
│ └── continue-resume.toml # Continue/resume functionality
36+
│ ├── continue-resume.toml # Continue/resume functionality
37+
│ ├── special-tools.toml # Special tools parallel execution
38+
│ └── multi-modal.toml # PDF and image reading
39+
├── fixtures/ # Test fixtures
40+
│ ├── test.pdf # PDF file for multi-modal tests
41+
│ └── test.gif # GIF image for multi-modal tests
3742
├── run.test.ts # CLI run command
3843
├── publish.test.ts # CLI publish command
3944
├── unpublish.test.ts # CLI unpublish command
4045
├── tag.test.ts # CLI tag command
4146
├── status.test.ts # CLI status command
4247
├── mixed-tools.test.ts # Mixed tool calls (MCP + Delegate + Interactive)
4348
├── parallel-mcp.test.ts # Parallel MCP tool execution
49+
├── special-tools.test.ts # Special tools (think, readPdfFile, readImageFile)
50+
├── multi-modal.test.ts # PDF and image content verification
4451
├── delegate-chain.test.ts # Expert delegation chain
4552
└── continue-resume.test.ts # --continue-run and --resume-from
4653
```
@@ -51,24 +58,26 @@ e2e/
5158

5259
Tests for CLI argument validation and error handling.
5360

54-
| File | Tests | Coverage |
55-
|------|-------|----------|
56-
| run.test.ts | 4 | Missing args, nonexistent expert, invalid config |
57-
| publish.test.ts | 4 | dry-run success, nonexistent expert, config errors |
58-
| unpublish.test.ts | 2 | Missing version, missing --force |
59-
| tag.test.ts | 2 | Missing version, missing tags |
60-
| status.test.ts | 3 | Missing version/status, invalid status |
61+
| File | Tests | Coverage |
62+
| ----------------- | ----- | -------------------------------------------------- |
63+
| run.test.ts | 4 | Missing args, nonexistent expert, invalid config |
64+
| publish.test.ts | 4 | dry-run success, nonexistent expert, config errors |
65+
| unpublish.test.ts | 2 | Missing version, missing --force |
66+
| tag.test.ts | 2 | Missing version, missing tags |
67+
| status.test.ts | 3 | Missing version/status, invalid status |
6168

6269
### Runtime Features
6370

6471
Tests for parallel tool calls, delegation, and state management.
6572

66-
| File | Tests | Coverage |
67-
|------|-------|----------|
68-
| mixed-tools.test.ts | 4 | MCP + Delegate + Interactive in single response |
69-
| parallel-mcp.test.ts | 3 | Parallel MCP tool execution |
70-
| delegate-chain.test.ts | 3 | Multi-level delegation |
71-
| continue-resume.test.ts | 4 | --continue-run, --resume-from |
73+
| File | Tests | Coverage |
74+
| ----------------------- | ----- | ---------------------------------------------------- |
75+
| mixed-tools.test.ts | 4 | MCP + Delegate + Interactive in single response |
76+
| parallel-mcp.test.ts | 3 | Parallel MCP tool execution |
77+
| special-tools.test.ts | 6 | think, readPdfFile, readImageFile parallel execution |
78+
| multi-modal.test.ts | 2 | PDF and image content reading verification |
79+
| delegate-chain.test.ts | 3 | Multi-level delegation |
80+
| continue-resume.test.ts | 4 | --continue-run, --resume-from |
7281

7382
## Writing Tests
7483

@@ -118,3 +127,14 @@ describe("Runtime feature", () => {
118127
- TUI-based commands (`start`) are excluded from E2E tests
119128
- API-calling tests (actual publish, unpublish) require registry access and are not included
120129

130+
## Multi-Modal Test Verification (IMPORTANT)
131+
132+
**Every time E2E tests are run, manually verify the multi-modal test output.**
133+
134+
The `multi-modal.test.ts` outputs the LLM's summary of PDF and image files. Check the console output to ensure:
135+
136+
1. **PDF Summary**: Should describe perstack GitHub README content (features, license, etc.)
137+
2. **Image Description**: Should describe terminal/CLI interface content
138+
139+
These logs confirm that `readPdfFile` and `readImageFile` tools are correctly reading file contents. Automated assertions check for keywords, but human review ensures the content is actually understood.
140+

e2e/experts/multi-modal.toml

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
model = "claude-sonnet-4-5"
2+
temperature = 0.3
3+
4+
[provider]
5+
providerName = "anthropic"
6+
7+
envPath = [".env", ".env.local"]
8+
9+
[experts."e2e-pdf-reader"]
10+
version = "1.0.0"
11+
description = "E2E test expert for PDF file reading"
12+
instruction = """
13+
You are a PDF content analyzer.
14+
15+
When given a task to analyze a PDF file:
16+
1. Use readPdfFile to read the PDF at the specified path
17+
2. Summarize the content briefly
18+
3. Call attemptCompletion with your summary
19+
20+
Be concise but include key details about what the document contains.
21+
"""
22+
23+
[experts."e2e-pdf-reader".skills."@perstack/base"]
24+
type = "mcpStdioSkill"
25+
command = "npx"
26+
packageName = "@perstack/base"
27+
pick = ["attemptCompletion", "readPdfFile"]
28+
29+
[experts."e2e-image-reader"]
30+
version = "1.0.0"
31+
description = "E2E test expert for image file reading"
32+
instruction = """
33+
You are an image content analyzer.
34+
35+
When given a task to analyze an image file:
36+
1. Use readImageFile to read the image at the specified path
37+
2. Describe what you see in the image
38+
3. Call attemptCompletion with your description
39+
40+
Be concise but include key visual details about what the image shows.
41+
"""
42+
43+
[experts."e2e-image-reader".skills."@perstack/base"]
44+
type = "mcpStdioSkill"
45+
command = "npx"
46+
packageName = "@perstack/base"
47+
pick = ["attemptCompletion", "readImageFile"]

e2e/multi-modal.test.ts

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
import { describe, expect, it } from "vitest"
2+
import { assertEventSequenceContains } from "./lib/assertions.js"
3+
import { runExpert } from "./lib/runner.js"
4+
5+
describe("Multi-Modal File Reading", () => {
6+
describe("PDF Reading", () => {
7+
it("should read and summarize PDF content about perstack github", async () => {
8+
const result = await runExpert(
9+
"e2e-pdf-reader",
10+
"Read and summarize the PDF at e2e/fixtures/test.pdf",
11+
{
12+
configPath: "./e2e/experts/multi-modal.toml",
13+
timeout: 180000,
14+
},
15+
)
16+
expect(result.exitCode).toBe(0)
17+
expect(
18+
assertEventSequenceContains(result.events, ["startRun", "callTools", "completeRun"]).passed,
19+
).toBe(true)
20+
const completeEvent = result.events.find((e) => e.type === "completeRun")
21+
expect(completeEvent).toBeDefined()
22+
const text = completeEvent && "text" in completeEvent ? (completeEvent.text as string) : ""
23+
console.log("\n=== PDF Summary ===\n", text, "\n=== END ===\n")
24+
expect(
25+
text.toLowerCase().includes("perstack") ||
26+
text.toLowerCase().includes("github") ||
27+
text.toLowerCase().includes("repository"),
28+
).toBe(true)
29+
}, 200000)
30+
})
31+
32+
describe("Image Reading", () => {
33+
it("should read and describe image content about perstack demo", async () => {
34+
const result = await runExpert(
35+
"e2e-image-reader",
36+
"Read and describe the image at e2e/fixtures/test.gif",
37+
{
38+
configPath: "./e2e/experts/multi-modal.toml",
39+
timeout: 180000,
40+
},
41+
)
42+
expect(result.exitCode).toBe(0)
43+
expect(
44+
assertEventSequenceContains(result.events, ["startRun", "callTools", "completeRun"]).passed,
45+
).toBe(true)
46+
const completeEvent = result.events.find((e) => e.type === "completeRun")
47+
expect(completeEvent).toBeDefined()
48+
const text = completeEvent && "text" in completeEvent ? (completeEvent.text as string) : ""
49+
console.log("\n=== Image Description ===\n", text, "\n=== END ===\n")
50+
expect(
51+
text.toLowerCase().includes("perstack") ||
52+
text.toLowerCase().includes("demo") ||
53+
text.toLowerCase().includes("terminal") ||
54+
text.toLowerCase().includes("cli") ||
55+
text.toLowerCase().includes("interface"),
56+
).toBe(true)
57+
}, 200000)
58+
})
59+
})

packages/core/src/schemas/runtime.ts

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -247,12 +247,6 @@ type ExpertEventPayloads = {
247247
resolveThought: {
248248
toolResult: ToolResult
249249
}
250-
resolvePdfFile: {
251-
toolResult: ToolResult
252-
}
253-
resolveImageFile: {
254-
toolResult: ToolResult
255-
}
256250
attemptCompletion: {
257251
toolResult: ToolResult
258252
}
@@ -343,8 +337,6 @@ export const callInteractiveTool = createEvent("callInteractiveTool")
343337
export const callDelegate = createEvent("callDelegate")
344338
export const resolveToolResults = createEvent("resolveToolResults")
345339
export const resolveThought = createEvent("resolveThought")
346-
export const resolvePdfFile = createEvent("resolvePdfFile")
347-
export const resolveImageFile = createEvent("resolveImageFile")
348340
export const attemptCompletion = createEvent("attemptCompletion")
349341
export const finishToolCall = createEvent("finishToolCall")
350342
export const resumeToolCalls = createEvent("resumeToolCalls")

packages/perstack/src/lib/tui.tsx

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -106,14 +106,6 @@ export function defaultEventListener(e: RunEvent): void {
106106
log(`${header(e)} Resolved Thought:`, e.toolResult)
107107
break
108108
}
109-
case "resolvePdfFile": {
110-
log(`${header(e)} Resolved PDF:`, e.toolResult)
111-
break
112-
}
113-
case "resolveImageFile": {
114-
log(`${header(e)} Resolved Image:`, e.toolResult)
115-
break
116-
}
117109
case "attemptCompletion": {
118110
log(`${header(e)} Attempting completion`)
119111
break

packages/runtime/README.md

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -193,16 +193,12 @@ stateDiagram-v2
193193
194194
CallingTools --> ResolvingToolResults: resolveToolResults
195195
CallingTools --> ResolvingThought: resolveThought
196-
CallingTools --> ResolvingPdfFile: resolvePdfFile
197-
CallingTools --> ResolvingImageFile: resolveImageFile
198196
CallingTools --> GeneratingRunResult: attemptCompletion
199197
CallingTools --> CallingDelegate: callDelegate
200198
CallingTools --> CallingInteractiveTool: callInteractiveTool
201199
202200
ResolvingToolResults --> FinishingStep: finishToolCall
203201
ResolvingThought --> FinishingStep: finishToolCall
204-
ResolvingPdfFile --> FinishingStep: finishToolCall
205-
ResolvingImageFile --> FinishingStep: finishToolCall
206202
207203
GeneratingRunResult --> Stopped: completeRun
208204
GeneratingRunResult --> FinishingStep: retry
@@ -219,7 +215,7 @@ Events trigger state transitions. They are emitted by the runtime logic or exter
219215

220216
- **Lifecycle**: `startRun`, `startGeneration`, `continueToNextStep`, `completeRun`
221217
- **Tool Execution**: `callTools`, `resolveToolResults`, `finishToolCall`, `resumeToolCalls`, `finishAllToolCalls`
222-
- **Special Types**: `resolveThought`, `resolvePdfFile`, `resolveImageFile`
218+
- **Special Types**: `resolveThought`
223219
- **Mixed Tool Calls**: `callDelegate`, `callInteractiveTool` (from CallingTools state)
224220
- **Interruption**: `stopRunByInteractiveTool`, `stopRunByDelegate`, `stopRunByExceededMaxSteps`
225221
- **Error Handling**: `retry`

packages/runtime/src/runtime-state-machine.ts

Lines changed: 0 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,6 @@ import { generatingRunResultLogic } from "./states/generating-run-result.js"
1010
import { generatingToolCallLogic } from "./states/generating-tool-call.js"
1111
import { initLogic } from "./states/init.js"
1212
import { preparingForStepLogic } from "./states/preparing-for-step.js"
13-
import { resolvingImageFileLogic } from "./states/resolving-image-file.js"
14-
import { resolvingPdfFileLogic } from "./states/resolving-pdf-file.js"
1513
import { resolvingThoughtLogic } from "./states/resolving-thought.js"
1614
import { resolvingToolResultLogic } from "./states/resolving-tool-result.js"
1715
import { createEmptyUsage, sumUsage } from "./usage.js"
@@ -237,26 +235,6 @@ export const runtimeStateMachine = setup({
237235
}) satisfies Step,
238236
}),
239237
},
240-
resolvePdfFile: {
241-
target: "ResolvingPdfFile",
242-
actions: assign({
243-
step: ({ context, event }) =>
244-
({
245-
...context.step,
246-
toolResults: [event.toolResult],
247-
}) satisfies Step,
248-
}),
249-
},
250-
resolveImageFile: {
251-
target: "ResolvingImageFile",
252-
actions: assign({
253-
step: ({ context, event }) =>
254-
({
255-
...context.step,
256-
toolResults: [event.toolResult],
257-
}) satisfies Step,
258-
}),
259-
},
260238
attemptCompletion: {
261239
target: "GeneratingRunResult",
262240
actions: assign({
@@ -336,46 +314,6 @@ export const runtimeStateMachine = setup({
336314
},
337315
},
338316

339-
ResolvingPdfFile: {
340-
on: {
341-
finishToolCall: {
342-
target: "FinishingStep",
343-
actions: assign({
344-
checkpoint: ({ context, event }) =>
345-
({
346-
...context.checkpoint,
347-
messages: [...context.checkpoint.messages, ...event.newMessages],
348-
}) satisfies Checkpoint,
349-
step: ({ context, event }) =>
350-
({
351-
...context.step,
352-
newMessages: [...context.step.newMessages, ...event.newMessages],
353-
}) satisfies Step,
354-
}),
355-
},
356-
},
357-
},
358-
359-
ResolvingImageFile: {
360-
on: {
361-
finishToolCall: {
362-
target: "FinishingStep",
363-
actions: assign({
364-
checkpoint: ({ context, event }) =>
365-
({
366-
...context.checkpoint,
367-
messages: [...context.checkpoint.messages, ...event.newMessages],
368-
}) satisfies Checkpoint,
369-
step: ({ context, event }) =>
370-
({
371-
...context.step,
372-
newMessages: [...context.step.newMessages, ...event.newMessages],
373-
}) satisfies Step,
374-
}),
375-
},
376-
},
377-
},
378-
379317
GeneratingRunResult: {
380318
on: {
381319
retry: {
@@ -482,8 +420,6 @@ export const StateMachineLogics: Record<
482420
CallingTool: callingToolLogic,
483421
ResolvingToolResult: resolvingToolResultLogic,
484422
ResolvingThought: resolvingThoughtLogic,
485-
ResolvingPdfFile: resolvingPdfFileLogic,
486-
ResolvingImageFile: resolvingImageFileLogic,
487423
GeneratingRunResult: generatingRunResultLogic,
488424
CallingInteractiveTool: callingInteractiveToolLogic,
489425
CallingDelegate: callingDelegateLogic,

0 commit comments

Comments
 (0)