Skip to content

Deduplicate breakpoint promotion logic #725

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Timi16 merged 1 commit into from
Mar 29, 2026
+48 −8
Merged

Deduplicate breakpoint promotion logic #725

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions extensions/vscode/src/cli/debuggerProcess.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
WIRE_PROTOCOL_MAX_VERSION,
WIRE_PROTOCOL_MIN_VERSION,
} from "../dap/protocol";
import { shouldPromoteToFunctionBreakpoint } from "../dap/sourceBreakpoints";
import { LogManager, LogLevel, LogPhase } from "../debug/logManager";

export interface DebuggerProcessConfig {
Expand Down Expand Up @@ -722,6 +723,7 @@ export class DebuggerProcess {
functionName: bp.function,
reasonCode: bp.reason_code,
message: bp.message,
setBreakpoint: shouldPromoteToFunctionBreakpoint(bp.verified, bp.function, bp.reason_code),
}));
Comment on lines 723 to 727
Copy link

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolveSourceBreakpoints() is declared to return array items without a setBreakpoint property, but the implementation now returns object literals that include setBreakpoint. With strict: true, this will fail TypeScript excess-property checking on the map() return. Update the method’s explicit return type to include setBreakpoint (likely setBreakpoint: boolean or setBreakpoint?: boolean) so it matches the implementation and downstream consumers.

Copilot uses AI. Check for mistakes.
}

Expand Down
4 changes: 2 additions & 2 deletions extensions/vscode/src/dap/adapter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ export class SorobanDebugSession extends DebugSession {
message: 'Debugger is not launched or source path is unavailable'
}));
} else {
let serverResolved: Array<{ requestedLine: number; line: number; verified: boolean; functionName?: string; reasonCode: string; message: string }> | null = null;
let serverResolved: Array<{ requestedLine: number; line: number; verified: boolean; functionName?: string; reasonCode: string; message: string; setBreakpoint?: boolean }> | null = null;
try {
serverResolved = await this.debuggerProcess.resolveSourceBreakpoints(source, lines, this.exportedFunctions);
} catch {
Expand All @@ -275,7 +275,7 @@ export class SorobanDebugSession extends DebugSession {
functionName: bp.functionName,
reasonCode: bp.reasonCode,
message: bp.message,
setBreakpoint: bp.verified && Boolean(bp.functionName)
setBreakpoint: bp.setBreakpoint
}));
} else {
resolved = resolveSourceBreakpoints(source, lines, this.exportedFunctions, sourceHistory);
Expand Down
20 changes: 15 additions & 5 deletions extensions/vscode/src/dap/sourceBreakpoints.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,16 @@ export function parseFunctionRanges(sourcePath: string): FunctionRange[] {
return ranges
}

export function shouldPromoteToFunctionBreakpoint(
verified: boolean,
functionName?: string,
reasonCode?: string
): boolean {
if (!functionName) return false
if (verified) return true
return reasonCode === 'HEURISTIC_REANCHORED' || reasonCode === 'HEURISTIC_NO_DWARF'
}

export function resolveSourceBreakpoints(
sourcePath: string,
lines: number[],
Expand All @@ -97,7 +107,7 @@ export function resolveSourceBreakpoints(
verified: false,
functionName: prevFn,
reasonCode: 'HEURISTIC_NOT_EXPORTED',
setBreakpoint: false,
setBreakpoint: shouldPromoteToFunctionBreakpoint(false, prevFn, 'HEURISTIC_NOT_EXPORTED'),
message: `Rust function '${prevFn}' is not an exported contract entrypoint`,
}
}
Expand All @@ -107,7 +117,7 @@ export function resolveSourceBreakpoints(
verified: false,
functionName: prevFn,
reasonCode: 'HEURISTIC_REANCHORED',
setBreakpoint: true,
setBreakpoint: shouldPromoteToFunctionBreakpoint(false, prevFn, 'HEURISTIC_REANCHORED'),
message: `Breakpoint re-anchored to '${prevFn}' after source edit (was line ${line}, now line ${fnRange.startLine})`,
}
}
Expand All @@ -118,7 +128,7 @@ export function resolveSourceBreakpoints(
line,
verified: false,
reasonCode: 'HEURISTIC_NO_FUNCTION',
setBreakpoint: false,
setBreakpoint: shouldPromoteToFunctionBreakpoint(false, undefined, 'HEURISTIC_NO_FUNCTION'),
message: 'Line is not inside a detectable Rust function',
}
}
Expand All @@ -130,7 +140,7 @@ export function resolveSourceBreakpoints(
verified: false,
functionName: range.name,
reasonCode: 'HEURISTIC_NOT_EXPORTED',
setBreakpoint: false,
setBreakpoint: shouldPromoteToFunctionBreakpoint(false, range.name, 'HEURISTIC_NOT_EXPORTED'),
message: `Rust function '${range.name}' is not an exported contract entrypoint`,
}
}
Expand All @@ -141,7 +151,7 @@ export function resolveSourceBreakpoints(
verified: false,
functionName: range.name,
reasonCode: 'HEURISTIC_NO_DWARF',
setBreakpoint: true,
setBreakpoint: shouldPromoteToFunctionBreakpoint(false, range.name, 'HEURISTIC_NO_DWARF'),
message: `Heuristic mapping to contract entrypoint '${range.name}' (DWARF source map unavailable)`,
}
})
Expand Down
30 changes: 29 additions & 1 deletion extensions/vscode/src/test/suites.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
getDebuggerVersionInfo,
validateLaunchConfig,
} from "../cli/debuggerProcess";
import { resolveSourceBreakpoints } from "../dap/sourceBreakpoints";
import { resolveSourceBreakpoints, shouldPromoteToFunctionBreakpoint } from "../dap/sourceBreakpoints";
import { VariableStore } from "../dap/variableStore";
import { DapClient } from "./dapClient";

Expand Down Expand Up @@ -234,6 +234,34 @@ export async function runSmokeSuite(): Promise<void> {
"Expected protocol mismatch message to include remediation guidance",
);

{
// Validate identical promotion logic for identical inputs
const testCases: Array<{
verified: boolean;
functionName?: string;
reasonCode?: string;
expected: boolean;
}> = [
{ verified: true, functionName: "test", expected: true },
{ verified: true, functionName: undefined, expected: false },
{ verified: false, functionName: "test", expected: false },
{ verified: false, functionName: "test", reasonCode: "HEURISTIC_NOT_EXPORTED", expected: false },
{ verified: false, functionName: "test", reasonCode: "HEURISTIC_REANCHORED", expected: true },
{ verified: false, functionName: "test", reasonCode: "HEURISTIC_NO_DWARF", expected: true },
{ verified: false, functionName: undefined, reasonCode: "HEURISTIC_NO_FUNCTION", expected: false },
];

for (const testCase of testCases) {
const result = shouldPromoteToFunctionBreakpoint(testCase.verified, testCase.functionName, testCase.reasonCode);
assert.strictEqual(
result,
testCase.expected,
`Expected shouldPromoteToFunctionBreakpoint(${testCase.verified}, '${testCase.functionName}', '${testCase.reasonCode}') to be ${testCase.expected}`,
);
}
console.log("Breakpoint promotion identical decision unit tests passed");
}

await assertPerRequestTimeoutBehavior();

const fixtures = resolveFixtures();
Expand Down
Loading