Skip to content

Commit 809056d

Browse files
authored
fix(lmtool): stop double-counting classNameDetection on every invoke (#1651)
* fix(lmtool): stop double-counting classNameDetection on every invoke findFullyQualifiedClassName was called twice for every invocation that used a simple class name: 1. inside constructDebugCommand to resolve the actual command 2. immediately after, to build the targetInfo message shown to the model Each call also emitted its own classNameDetection telemetry event, so the classNameDetection success/failure counts in ddtelfiltered were inflated by 2x. This makes the apparent ~49% noPackage failure rate look much worse than reality (the true rate is closer to ~25% before accounting for other root causes addressed in #1650). Fix: resolve the detection once at the caller, emit telemetry once, and pass the resolved name into both constructDebugCommand and the targetInfo branch. Side benefit: removes a redundant file system walk from the hot launch path. * review: reword dedup comments to drop incorrect '2x telemetry' claim Reviewer correctly pointed out that the original main branch only had a duplicate findFullyQualifiedClassName FS walk in the targetInfo branch; the recordLaunchInternal({name:'classNameDetection'}) emission was only inside constructDebugCommand, not duplicated. The 'inflates noPackage by 2x' rationale was wrong — observed metrics are not inflated by the dup. Reworded both comments to focus on the FS-walk dedup + ownership clarity. Functional code unchanged; only comments updated.
1 parent 54a4ceb commit 809056d

1 file changed

Lines changed: 44 additions & 23 deletions

File tree

src/languageModelTool.ts

Lines changed: 44 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,33 @@ async function debugJavaApplication(
194194
}
195195

196196
// Step 3: Construct and execute the debugjava command
197-
const debugCommand = constructDebugCommand(input, projectType);
197+
//
198+
// For simple class names (no dot), resolve the fully-qualified class once
199+
// here so we can reuse the result for both the command construction and
200+
// the user-facing targetInfo message below. Previously these two paths
201+
// each called findFullyQualifiedClassName independently, which:
202+
// - duplicated the file system walk on the hot launch path (the FS walk
203+
// is the actual user-visible slowdown — it stats every .java file
204+
// under src/main/java up to MAX_FILE_SEARCH_DEPTH)
205+
// - made the call sites harder to reason about, since detection
206+
// ownership was split across constructDebugCommand and the targetInfo
207+
// formatting block
208+
//
209+
// After this refactor, the caller owns detection and its telemetry,
210+
// and constructDebugCommand accepts a pre-resolved name.
211+
let detectedClassName: string | null = null;
212+
if (!input.target.endsWith('.jar')
213+
&& !input.target.startsWith('-')
214+
&& !input.target.includes('.')) {
215+
detectedClassName = findFullyQualifiedClassName(input.workspacePath, input.target, projectType);
216+
recordLaunchInternal({
217+
name: 'classNameDetection',
218+
projectType,
219+
detected: !!detectedClassName,
220+
});
221+
}
222+
223+
const debugCommand = constructDebugCommand(input, projectType, detectedClassName);
198224

199225
// Validate that we can construct a valid command
200226
if (!debugCommand || debugCommand === 'debugjava') {
@@ -223,8 +249,9 @@ async function debugJavaApplication(
223249
} else if (input.target.includes('.')) {
224250
targetInfo = input.target;
225251
} else {
226-
// Simple class name - check if we successfully detected the full name
227-
const detectedClassName = findFullyQualifiedClassName(input.workspacePath, input.target, projectType);
252+
// Simple class name - reuse the detection result from Step 3 above
253+
// (do NOT call findFullyQualifiedClassName again — it walks the FS
254+
// and the result is already in `detectedClassName`).
228255
if (detectedClassName) {
229256
targetInfo = `${detectedClassName} (detected from ${input.target})`;
230257
} else {
@@ -591,10 +618,17 @@ async function ensureVSCodeCompilation(workspaceUri: vscode.Uri): Promise<DebugJ
591618

592619
/**
593620
* Constructs the debugjava command based on input parameters.
621+
*
622+
* @param preDetectedClassName Fully-qualified class name pre-resolved by the
623+
* caller (and already reported via the `classNameDetection` telemetry event).
624+
* When non-null and the target is a simple class name, this is used instead
625+
* of re-running findFullyQualifiedClassName here. The caller is expected to
626+
* handle telemetry emission so we do not double-count detection outcomes.
594627
*/
595628
function constructDebugCommand(
596629
input: DebugJavaApplicationInput,
597-
projectType: 'maven' | 'gradle' | 'vscode' | 'unknown'
630+
projectType: 'maven' | 'gradle' | 'vscode' | 'unknown',
631+
preDetectedClassName: string | null = null
598632
): string {
599633
let command = 'debugjava';
600634

@@ -610,25 +644,12 @@ function constructDebugCommand(
610644
else {
611645
let className = input.target;
612646

613-
// If target doesn't contain a dot and we can find the Java file,
614-
// try to detect the fully qualified class name
615-
if (!input.target.includes('.')) {
616-
const detectedClassName = findFullyQualifiedClassName(input.workspacePath, input.target, projectType);
617-
if (detectedClassName) {
618-
recordLaunchInternal({
619-
name: 'classNameDetection',
620-
projectType,
621-
detected: true,
622-
});
623-
className = detectedClassName;
624-
} else {
625-
// No package detected - class is in default package
626-
recordLaunchInternal({
627-
name: 'classNameDetection',
628-
projectType,
629-
detected: false,
630-
});
631-
}
647+
// Use the caller-supplied detection result; we deliberately do not
648+
// call findFullyQualifiedClassName a second time here (see the
649+
// dedupe note at the call site). Detection telemetry is owned by
650+
// the caller.
651+
if (!input.target.includes('.') && preDetectedClassName) {
652+
className = preDetectedClassName;
632653
}
633654

634655
// Use provided classpath if available, otherwise infer it

0 commit comments

Comments
 (0)