Skip to content

Commit b6aa5d0

Browse files
committed
feat: add line numbers to PTC type errors
- Move muxTypes after agent code in typeValidator so error positions match - Return TypeValidationError objects with message, line, column instead of strings - Update staticAnalysis to propagate line/column from type errors - Add tests for line number reporting in code_execution.test.ts and typeValidator.test.ts
1 parent 050e8a2 commit b6aa5d0

File tree

4 files changed

+98
-12
lines changed

4 files changed

+98
-12
lines changed

src/node/services/ptc/staticAnalysis.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -314,10 +314,12 @@ export async function analyzeCode(code: string, muxTypes?: string): Promise<Anal
314314
// 4. TypeScript type validation (if muxTypes provided)
315315
if (muxTypes) {
316316
const typeResult = validateTypes(code, muxTypes);
317-
for (const message of typeResult.errors) {
317+
for (const typeError of typeResult.errors) {
318318
errors.push({
319319
type: "type_error",
320-
message,
320+
message: typeError.message,
321+
line: typeError.line,
322+
column: typeError.column,
321323
});
322324
}
323325
}

src/node/services/ptc/typeValidator.test.ts

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,9 @@ describe("validateTypes", () => {
7171
);
7272
expect(result.valid).toBe(false);
7373
// Error should mention 'path' doesn't exist or 'filePath' is missing
74-
expect(result.errors.some((e) => e.includes("path") || e.includes("filePath"))).toBe(true);
74+
expect(result.errors.some((e) => e.message.includes("path") || e.message.includes("filePath"))).toBe(
75+
true
76+
);
7577
});
7678

7779
test("catches missing required property", () => {
@@ -94,7 +96,9 @@ describe("validateTypes", () => {
9496
muxTypes
9597
);
9698
expect(result.valid).toBe(false);
97-
expect(result.errors.some((e) => e.includes("number") || e.includes("string"))).toBe(true);
99+
expect(result.errors.some((e) => e.message.includes("number") || e.message.includes("string"))).toBe(
100+
true
101+
);
98102
});
99103

100104
test("catches calling non-existent tool", () => {
@@ -105,7 +109,21 @@ describe("validateTypes", () => {
105109
muxTypes
106110
);
107111
expect(result.valid).toBe(false);
108-
expect(result.errors.some((e) => e.includes("nonexistent_tool"))).toBe(true);
112+
expect(result.errors.some((e) => e.message.includes("nonexistent_tool"))).toBe(true);
113+
});
114+
115+
test("returns line numbers for type errors", () => {
116+
const result = validateTypes(
117+
`const x = 1;
118+
const y = 2;
119+
mux.file_read({ path: "test.txt" });`,
120+
muxTypes
121+
);
122+
expect(result.valid).toBe(false);
123+
// Error should be on line 3 (the mux.file_read call)
124+
const errorWithLine = result.errors.find((e) => e.line !== undefined);
125+
expect(errorWithLine).toBeDefined();
126+
expect(errorWithLine!.line).toBe(3);
109127
});
110128

111129
test("allows dynamic property access (no strict checking on unknown keys)", () => {

src/node/services/ptc/typeValidator.ts

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,15 @@
1111

1212
import ts from "typescript";
1313

14+
export interface TypeValidationError {
15+
message: string;
16+
line?: number;
17+
column?: number;
18+
}
19+
1420
export interface TypeValidationResult {
1521
valid: boolean;
16-
errors: string[];
22+
errors: TypeValidationError[];
1723
}
1824

1925
/**
@@ -26,11 +32,12 @@ export interface TypeValidationResult {
2632
export function validateTypes(code: string, muxTypes: string): TypeValidationResult {
2733
// Wrap code in function to allow return statements (matches runtime behavior)
2834
// Note: We don't use async because Asyncify makes mux.* calls appear synchronous
29-
const wrappedCode = `${muxTypes}
30-
31-
function __agent__() {
32-
${code}
35+
// Types go AFTER code so error line numbers match agent's code directly
36+
const wrapperPrefix = "function __agent__() {\n";
37+
const wrappedCode = `${wrapperPrefix}${code}
3338
}
39+
40+
${muxTypes}
3441
`;
3542

3643
const compilerOptions: ts.CompilerOptions = {
@@ -64,11 +71,25 @@ ${code}
6471

6572
// Filter to errors in our code only (not lib files)
6673
// Also filter console redeclaration warning (our minimal console conflicts with lib.dom)
67-
const errors = diagnostics
74+
const errors: TypeValidationError[] = diagnostics
6875
.filter((d) => d.category === ts.DiagnosticCategory.Error)
6976
.filter((d) => !d.file || d.file.fileName === "agent.ts")
7077
.filter((d) => !ts.flattenDiagnosticMessageText(d.messageText, "").includes("console"))
71-
.map((d) => ts.flattenDiagnosticMessageText(d.messageText, " "));
78+
.map((d) => {
79+
const message = ts.flattenDiagnosticMessageText(d.messageText, " ");
80+
// Extract line number if available, adjusting for wrapper prefix
81+
if (d.file && d.start !== undefined) {
82+
const { line, character } = d.file.getLineAndCharacterOfPosition(d.start);
83+
// Line is 0-indexed; wrapper adds 1 line ("function __agent__() {\n")
84+
// Only report line if it's in the agent's code (not in muxTypes after)
85+
const agentCodeLines = code.split("\n").length;
86+
const adjustedLine = line; // line 1 in file = line 0 (0-indexed) = agent line 1
87+
if (adjustedLine >= 1 && adjustedLine <= agentCodeLines) {
88+
return { message, line: adjustedLine, column: character + 1 };
89+
}
90+
}
91+
return { message };
92+
});
7293

7394
return { valid: errors.length === 0, errors };
7495
}

src/node/services/tools/code_execution.test.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,18 @@ describe("createCodeExecutionTool", () => {
140140
expect(result.error).toContain("process");
141141
});
142142

143+
it("includes line numbers for unavailable globals", async () => {
144+
const tool = await createCodeExecutionTool(runtimeFactory, new ToolBridge({}));
145+
146+
const result = (await tool.execute!(
147+
{ code: "const x = 1;\nconst y = 2;\nconst env = process.env" }, // process on line 3
148+
mockToolCallOptions
149+
)) as PTCExecutionResult;
150+
151+
expect(result.success).toBe(false);
152+
expect(result.error).toContain("(line 3)");
153+
});
154+
143155
it("rejects code using require()", async () => {
144156
const tool = await createCodeExecutionTool(runtimeFactory, new ToolBridge({}));
145157

@@ -152,6 +164,39 @@ describe("createCodeExecutionTool", () => {
152164
expect(result.error).toContain("Code analysis failed");
153165
expect(result.error).toContain("require");
154166
});
167+
168+
it("includes line numbers for type errors", async () => {
169+
const mockTools: Record<string, Tool> = {
170+
bash: createMockTool("bash", z.object({ script: z.string() })),
171+
};
172+
const tool = await createCodeExecutionTool(runtimeFactory, new ToolBridge(mockTools));
173+
174+
const result = (await tool.execute!(
175+
{ code: "const x = 1;\nconst result = mux.bash({ scriptz: 'ls' });" }, // typo on line 2
176+
mockToolCallOptions
177+
)) as PTCExecutionResult;
178+
179+
expect(result.success).toBe(false);
180+
expect(result.error).toContain("Code analysis failed");
181+
expect(result.error).toContain("scriptz");
182+
expect(result.error).toContain("(line 2)");
183+
});
184+
185+
it("includes line numbers for calling non-existent tools", async () => {
186+
const mockTools: Record<string, Tool> = {
187+
bash: createMockTool("bash", z.object({ script: z.string() })),
188+
};
189+
const tool = await createCodeExecutionTool(runtimeFactory, new ToolBridge(mockTools));
190+
191+
const result = (await tool.execute!(
192+
{ code: "const x = 1;\nconst y = 2;\nmux.nonexistent({ arg: 1 });" }, // line 3
193+
mockToolCallOptions
194+
)) as PTCExecutionResult;
195+
196+
expect(result.success).toBe(false);
197+
expect(result.error).toContain("Code analysis failed");
198+
expect(result.error).toContain("(line 3)");
199+
});
155200
});
156201

157202
describe("code execution", () => {

0 commit comments

Comments
 (0)