Address Copilot review on PR #1644

- classifyStep: unknown step operations now report 'unknown' instead of being silently mislabeled as 'over'. Also adds a runtime guard in debug_step_operation so an unknown operation no longer reaches commandMap[op]/executeCommand(undefined) or session.customRequest with an arbitrary string.

- recordToolInvocation: introduces a private normalizeToolInvocationRecord that keeps 'outcome' and 'errorCategory' in lock-step for the six shared terminal values (cancelled / timeout / lsNotReady / noActiveSession / noSuspendedThread / noStackFrame). Fixes the case where debug_java_application returns {success:false,message:'Operation cancelled by user'} but outcome was 'failure' while errorCategory was 'cancelled'.

- get_debug_stack_trace: empty-stack-frame early return now sets errorCategory='noStackFrame' alongside outcome (was only setting outcome).

- recordLaunchInternal: signature is now a discriminated union (LaunchInternalEvent) instead of (operationName: string, properties: Record<string, SafeValue>). Unknown event names and unexpected property keys are now rejected at compile time. Updated all 8 call sites.

- elapsedTime (string from .toFixed) split from elapsedMs (number) so the telemetry value is numeric and aggregable.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: wenytang-ms

src/languageModelTool.ts

@@ -149,7 +149,8 @@ async function debugJavaApplication(
     // Step 0: Cleanup any existing Java debug session to avoid port conflicts
     const existingSession = vscode.debug.activeDebugSession;
     if (existingSession && existingSession.type === 'java') {
-        recordLaunchInternal('cleanupExistingSession', {
+        recordLaunchInternal({
+            name: 'cleanupExistingSession',
             sessionId: existingSession.id,
         });
         try {
@@ -158,7 +159,8 @@ async function debugJavaApplication(
             await new Promise(resolve => setTimeout(resolve, 500));
         } catch (error) {
             // Log but continue - the old session might already be dead
-            recordLaunchInternal('cleanupExistingSessionFailed', {
+            recordLaunchInternal({
+                name: 'cleanupExistingSessionFailed',
                 errorCategory: classifyError(error),
             });
         }
@@ -245,7 +247,8 @@ async function debugJavaApplication(
                         clearTimeout(timeoutHandle);
                     }
 
-                    recordLaunchInternal('debugSessionStarted.eventBased', {
+                    recordLaunchInternal({
+                        name: 'debugSessionStarted.eventBased',
                         sessionId: session.id,
                     });
 
@@ -267,7 +270,7 @@ async function debugJavaApplication(
                 if (!sessionStarted) {
                     sessionDisposable.dispose();
 
-                    recordLaunchInternal('debugSessionTimeout.eventBased', {});
+                    recordLaunchInternal({ name: 'debugSessionTimeout.eventBased' });
 
                     resolve({
                         success: false,
@@ -301,11 +304,13 @@ async function debugJavaApplication(
             // Check if debug session has started
             const session = vscode.debug.activeDebugSession;
             if (session && session.type === 'java') {
-                const elapsedTime = ((Date.now() - startTime) / 1000).toFixed(1);
+                const elapsedMs = Date.now() - startTime;
+                const elapsedTime = (elapsedMs / 1000).toFixed(1);
 
-                recordLaunchInternal('debugSessionDetected', {
+                recordLaunchInternal({
+                    name: 'debugSessionDetected',
                     sessionId: session.id,
-                    elapsedTime,
+                    elapsedMs,
                 });
 
                 return {
@@ -322,7 +327,8 @@ async function debugJavaApplication(
         }
 
         // Timeout: session not detected within 15 seconds
-        recordLaunchInternal('debugSessionTimeout.smartPolling', {
+        recordLaunchInternal({
+            name: 'debugSessionTimeout.smartPolling',
             maxWaitTime,
         });
 
@@ -602,14 +608,16 @@ function constructDebugCommand(
         if (!input.target.includes('.')) {
             const detectedClassName = findFullyQualifiedClassName(input.workspacePath, input.target, projectType);
             if (detectedClassName) {
-                recordLaunchInternal('classNameDetection', {
+                recordLaunchInternal({
+                    name: 'classNameDetection',
                     projectType,
                     detected: true,
                 });
                 className = detectedClassName;
             } else {
                 // No package detected - class is in default package
-                recordLaunchInternal('classNameDetection', {
+                recordLaunchInternal({
+                    name: 'classNameDetection',
                     projectType,
                     detected: false,
                 });
@@ -1012,6 +1020,14 @@ export function registerDebugSessionTools(_context: vscode.ExtensionContext): vs
                 };
 
                 const command = commandMap[operation];
+                if (!command) {
+                    outcome = 'failure';
+                    errorCategory = 'other';
+                    return new (vscode as any).LanguageModelToolResult([
+                        new (vscode as any).LanguageModelTextPart(`✗ Unknown step operation: ${operation}`)
+                    ]);
+                }
+
                 if (threadId !== undefined) {
                     // For thread-specific operations, use custom request
                     await session.customRequest(operation, { threadId });
@@ -1181,6 +1197,7 @@ export function registerDebugSessionTools(_context: vscode.ExtensionContext): vs
 
                 if (!stackResponse.stackFrames || stackResponse.stackFrames.length === 0) {
                     outcome = 'noStackFrame';
+                    errorCategory = 'noStackFrame';
                     return new (vscode as any).LanguageModelToolResult([
                         new (vscode as any).LanguageModelTextPart('No stack frames available.')
                     ]);
@@ -1625,7 +1642,8 @@ export function registerDebugSessionTools(_context: vscode.ExtensionContext): vs
                     // If we can't even get threads, something is wrong
                     // But session exists, so mark as running
                     isPaused = false;
-                    recordLaunchInternal('getDebugSessionInfo.threadError', {
+                    recordLaunchInternal({
+                        name: 'getDebugSessionInfo.threadError',
                         errorCategory: classifyError(error),
                     });
                 }

src/lmToolTelemetry.ts

@@ -76,7 +76,7 @@ export type BreakpointKind =
     | 'hitCount'
     | 'logpoint';
 
-export type StepKind = 'in' | 'out' | 'over' | 'continue' | 'pause';
+export type StepKind = 'in' | 'out' | 'over' | 'continue' | 'pause' | 'unknown';
 
 export type EvalContext = 'watch' | 'repl' | 'hover' | 'unknown';
 
@@ -215,7 +215,7 @@ export function classifyStep(operation: string | undefined): StepKind {
         case 'pause':
             return 'pause';
         default:
-            return 'over';
+            return 'unknown';
     }
 }
 
@@ -309,31 +309,83 @@ export interface ToolInvocationRecord {
 /**
  * Record a single tool-invocation outcome. Replaces ad-hoc `sendInfo`
  * calls inside individual tools.
+ *
+ * Before sending, the record is normalized so that `outcome` and
+ * `errorCategory` stay aligned for the six shared terminal values
+ * (cancelled / timeout / lsNotReady / noActiveSession / noSuspendedThread /
+ * noStackFrame). See {@link normalizeToolInvocationRecord}.
  */
 export function recordToolInvocation(record: ToolInvocationRecord): void {
+    const normalized = normalizeToolInvocationRecord(record);
     sanitizedSend({
-        operationName: `languageModelTool.${record.tool}.invoke`,
-        outcome: record.outcome,
-        errorCategory: record.errorCategory,
-        durationMs: record.durationMs,
-        targetType: record.targetType,
-        breakpointKind: record.breakpointKind,
-        stepKind: record.stepKind,
-        evalContext: record.evalContext,
-        removeScope: record.removeScope,
-        scopeType: record.scopeType,
-        isPaused: record.isPaused,
-        skipBuild: record.skipBuild,
-        hasFilter: record.hasFilter,
-        frameCount: record.frameCount,
-        threadCount: record.threadCount,
-        suspendedCount: record.suspendedCount,
-        removedCount: record.removedCount,
-        sessionId: record.sessionId,
-        sessionType: record.sessionType,
+        operationName: `languageModelTool.${normalized.tool}.invoke`,
+        outcome: normalized.outcome,
+        errorCategory: normalized.errorCategory,
+        durationMs: normalized.durationMs,
+        targetType: normalized.targetType,
+        breakpointKind: normalized.breakpointKind,
+        stepKind: normalized.stepKind,
+        evalContext: normalized.evalContext,
+        removeScope: normalized.removeScope,
+        scopeType: normalized.scopeType,
+        isPaused: normalized.isPaused,
+        skipBuild: normalized.skipBuild,
+        hasFilter: normalized.hasFilter,
+        frameCount: normalized.frameCount,
+        threadCount: normalized.threadCount,
+        suspendedCount: normalized.suspendedCount,
+        removedCount: normalized.removedCount,
+        sessionId: normalized.sessionId,
+        sessionType: normalized.sessionType,
     });
 }
 
+/**
+ * Values that exist in both {@link ToolOutcome} and {@link ErrorCategory}.
+ * For these, the two fields must stay in lock-step so dashboard queries
+ * filtering on either one produce identical results.
+ */
+const SHARED_TERMINAL_VALUES = [
+    'cancelled',
+    'timeout',
+    'lsNotReady',
+    'noActiveSession',
+    'noSuspendedThread',
+    'noStackFrame',
+] as const;
+
+type SharedTerminal = typeof SHARED_TERMINAL_VALUES[number];
+
+function isSharedTerminal(value: string | undefined): value is SharedTerminal {
+    return value !== undefined && (SHARED_TERMINAL_VALUES as readonly string[]).includes(value);
+}
+
+/**
+ * Reconcile `outcome` and `errorCategory` for the six shared terminal
+ * values so downstream queries can rely on either field. Returns a NEW
+ * record; the input is not mutated.
+ *
+ * Rules:
+ *  - If `errorCategory` is a shared terminal value, promote `outcome` to
+ *    that value (callers that only set `errorCategory` get a consistent
+ *    `outcome` for free).
+ *  - If `outcome` is a shared terminal value and `errorCategory` is
+ *    absent, fill it with the matching value (callers that only set
+ *    `outcome` get a consistent `errorCategory`).
+ */
+function normalizeToolInvocationRecord(record: ToolInvocationRecord): ToolInvocationRecord {
+    let outcome: ToolOutcome = record.outcome;
+    let errorCategory: ErrorCategory | undefined = record.errorCategory;
+
+    if (isSharedTerminal(errorCategory)) {
+        outcome = errorCategory;
+    } else if (isSharedTerminal(outcome) && errorCategory === undefined) {
+        errorCategory = outcome;
+    }
+
+    return { ...record, outcome, errorCategory };
+}
+
 export interface ChatActivationRecord {
     javaLSReadyAtActivation: boolean;
     lmtCount: number;
@@ -358,17 +410,41 @@ export function recordChatActivation(record: ChatActivationRecord): void {
     });
 }
 
+/**
+ * Project type detected by the launch flow. Free-form values are
+ * forbidden so this stays a closed enum.
+ */
+export type LaunchProjectType = 'maven' | 'gradle' | 'vscode' | 'unknown';
+
+/**
+ * Discriminated union of every launch-flow internal event the recorder
+ * is allowed to emit. Each variant lists its allowed properties so the
+ * type system rejects unknown event names and unknown property keys.
+ *
+ * Note: `sessionId` here is VS Code's opaque debug-session GUID, never
+ * the user-visible `launch.json` session name.
+ */
+export type LaunchInternalEvent =
+    | { name: 'cleanupExistingSession'; sessionId: string }
+    | { name: 'cleanupExistingSessionFailed'; errorCategory: ErrorCategory }
+    | { name: 'debugSessionStarted.eventBased'; sessionId: string }
+    | { name: 'debugSessionTimeout.eventBased' }
+    | { name: 'debugSessionDetected'; sessionId: string; elapsedMs: number }
+    | { name: 'debugSessionTimeout.smartPolling'; maxWaitTime: number }
+    | { name: 'classNameDetection'; projectType: LaunchProjectType; detected: boolean }
+    | { name: 'getDebugSessionInfo.threadError'; errorCategory: ErrorCategory };
+
 /**
  * Internal-debug event for the launch-flow nested instrumentation
  * (session-detected / cleanup / timeout). Re-uses the sanitised sender so
- * no PII can slip in.
+ * no PII can slip in. Accepts only the discriminated-union shapes defined
+ * in {@link LaunchInternalEvent} — unknown event names or unexpected
+ * property keys are rejected at compile time.
  */
-export function recordLaunchInternal(
-    operationName: string,
-    properties: Record<string, SafeValue>,
-): void {
+export function recordLaunchInternal(event: LaunchInternalEvent): void {
+    const { name, ...properties } = event;
     sanitizedSend({
-        operationName: `languageModelTool.${operationName}`,
+        operationName: `languageModelTool.${name}`,
         ...properties,
     });
 }