Skip to content

Commit b954ab3

Browse files
author
Copilot CLI
committed
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 1853cec commit b954ab3

1 file changed

Lines changed: 10 additions & 4 deletions

File tree

src/languageModelTool.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -199,9 +199,15 @@ async function debugJavaApplication(
199199
// here so we can reuse the result for both the command construction and
200200
// the user-facing targetInfo message below. Previously these two paths
201201
// each called findFullyQualifiedClassName independently, which:
202-
// - duplicated the file system walk on the hot launch path
203-
// - emitted two classNameDetection events per invoke, inflating the
204-
// detection failure rate in telemetry by 2x
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.
205211
let detectedClassName: string | null = null;
206212
if (!input.target.endsWith('.jar')
207213
&& !input.target.startsWith('-')
@@ -245,7 +251,7 @@ async function debugJavaApplication(
245251
} else {
246252
// Simple class name - reuse the detection result from Step 3 above
247253
// (do NOT call findFullyQualifiedClassName again — it walks the FS
248-
// and was double-emitting the classNameDetection telemetry event).
254+
// and the result is already in `detectedClassName`).
249255
if (detectedClassName) {
250256
targetInfo = `${detectedClassName} (detected from ${input.target})`;
251257
} else {

0 commit comments

Comments
 (0)