Record where source opens land and fill code-navigation telemetry gaps

apps/review-desktop/code-oss/src/vs/review/browser/parts/canvas/reviewApiEditorSerializer.test.ts

@@ -29,6 +29,7 @@ test("opening historical source requests its version and opens a separate native
 		{} as never,
 		{} as never,
 		{} as never,
+		{ capture() {} } as never,
 	);
 	t.after(() => tabs.dispose());
 	await tabs.openApiSource({ reviewId: "review-a", kind: "version", version: 7 }, "Historical Review");
@@ -64,7 +65,7 @@ test("native group restoration preserves both reviews, order and pinned source v
 	};
 	const groupService = { groups, mainPart: { activeGroup: undefined as EditorGroupModel | undefined } };
 	const createTabs = () =>
-		new ReviewCanvasEditorTabsService(instantiation as never, editors as never, groupService as never, {} as never, {} as never, { warn() {} } as never, {} as never, {} as never, {} as never);
+		new ReviewCanvasEditorTabsService(instantiation as never, editors as never, groupService as never, {} as never, {} as never, { warn() {} } as never, {} as never, {} as never, {} as never, { capture() {} } as never);
 	tabs = createTabs();
 	const registry = Registry.as<IEditorFactoryRegistry>(EditorExtensions.EditorFactory);
 	registry.start({ get: () => instantiation } as never);
@@ -131,7 +132,7 @@ test("current Source tabs retain identity and main version tabs still restore",
     },
     invokeFunction(fn: (accessor: { get(): ReviewCanvasEditorTabsService }) => unknown) { return fn({ get: () => tabs }); },
   };
-  tabs = new ReviewCanvasEditorTabsService(instantiation as never, { onDidCloseEditor: Event.None } as never, {} as never, {} as never, {} as never, { warn() {} } as never, {} as never, {} as never, {} as never);
+  tabs = new ReviewCanvasEditorTabsService(instantiation as never, { onDidCloseEditor: Event.None } as never, {} as never, {} as never, {} as never, { warn() {} } as never, {} as never, {} as never, {} as never, { capture() {} } as never);
   try {
     const serializer = new ReviewApiEditorSerializer();
     const restored = serializer.deserialize(instantiation as never, JSON.stringify({ kind: "api-source", reviewId: "a", title: "A", selection: { reviewId: "a", kind: "current" } }));

apps/review-desktop/code-oss/src/vs/review/browser/parts/canvas/reviewCanvasPart.ts

@@ -1060,6 +1060,7 @@ export class ReviewCanvasEditorPane extends EditorPane {
 				this.reviewTelemetryService.capture("setting_changed", {
 					setting: "open_files_in",
 					enabled: true,
+					choice,
 				});
 				await this.configurationService.updateValue(REVIEW_OPEN_FILES_IN_SETTING, choice, ConfigurationTarget.USER);
 				return this.currentOpenFilesIn();
@@ -1170,6 +1171,7 @@ export class ReviewCanvasEditorPane extends EditorPane {
 		this.reviewTelemetryService.capture("setting_changed", {
 			setting: "open_files_in",
 			enabled: true,
+			choice: "application",
 		});
 		await this.configurationService.updateValue(REVIEW_OPEN_FILES_IN_APPLICATION_SETTING, path, ConfigurationTarget.USER);
 		await this.configurationService.updateValue(REVIEW_OPEN_FILES_IN_SETTING, "application", ConfigurationTarget.USER);

apps/review-desktop/code-oss/src/vs/review/contrib/telemetry/reviewLspTelemetry.contribution.ts

@@ -24,6 +24,7 @@ import {
 import { IExtensionService } from '../../../workbench/services/extensions/common/extensions.js';
 import { LifecyclePhase } from '../../../workbench/services/lifecycle/common/lifecycle.js';
 import { ReviewMonacoPeeks } from '../../services/reviewMonacoPeeks.js';
+import { isReviewDefinitionPeekEditor } from '../../services/reviewDefinitionPeek.js';
 import { IReviewTelemetryService } from '../../services/reviewTelemetryService.js';
 
 const HOVER_DWELL_MS = 1_000;
@@ -74,6 +75,7 @@ const LSP_COMMANDS: Record<string, string> = {
 	'editor.action.goToReferences': 'references',
 	'editor.action.referenceSearch.trigger': 'references',
 	'editor.action.findReferences': 'references',
+	'review.action.embeddedShowReferences': 'references',
 	'editor.action.rename': 'rename',
 	'editor.action.formatDocument': 'format',
 	'editor.action.formatSelection': 'format',
@@ -184,7 +186,7 @@ class ReviewLspTelemetryContribution extends Disposable implements IWorkbenchCon
 			captured = true;
 			if (Date.now() - (this.lastCommandHover.get(editor) ?? 0) >= HOVER_DWELL_MS) {
 				const model = editor.getModel();
-				if (model) {
+				if (model && this.languageFeaturesService.hoverProvider.has(model)) {
 					this.captureLspUsed(editor, model, 'hover', 'mouse');
 				}
 			}
@@ -240,6 +242,9 @@ class ReviewLspTelemetryContribution extends Disposable implements IWorkbenchCon
 		if (ReviewMonacoPeeks.owns(editor)) {
 			return 'inline_peek';
 		}
+		if (isReviewDefinitionPeekEditor(editor)) {
+			return 'definition_peek';
+		}
 		for (const diffEditor of this.codeEditorService.listDiffEditors()) {
 			if (diffEditor.getOriginalEditor() === editor || diffEditor.getModifiedEditor() === editor) {
 				return 'diff';

apps/review-desktop/code-oss/src/vs/review/services/reviewCanvasEditorTabsService.test.ts

@@ -41,6 +41,7 @@ async function closeWelcome(updateNeeded: boolean): Promise<number> {
 		{} as never,
 		{} as never,
 		{} as never,
+		{ capture() {} } as never,
 	);
 	try {
 		const welcome = await tabs.openWelcome(true);
@@ -75,6 +76,7 @@ function sourceTabs(t: TestContext, answer: (url: URL) => object, openFilesIn =
 		{ async writeFile(resource: URI, content: VSBuffer) { written.push(`${resource.toString()}=${content.toString()}`); } } as never,
 		{ cacheHome: URI.file("/laptop/cache") } as never,
 		{ getValue: () => openFilesIn } as never,
+		{ capture() {} } as never,
 	);
 	t.after(() => tabs.dispose());
 	return { tabs, opened, written };

apps/review-desktop/code-oss/src/vs/review/services/reviewCanvasEditorTabsService.ts

@@ -33,6 +33,7 @@ import { REVIEW_LANGUAGE_SOURCE_SCHEME } from "../common/reviewReadonlySource.js
 import { sourceLocation, sourceSelectionIdentity, REVIEW_API_SOURCE_SCHEME } from "../common/reviewSourceView.js";
 import { reviewRemoteAuthority } from "./remote/reviewRemoteAuthority.js";
 import { IReviewDesktopConnectionService, reviewResponseError } from "./reviewDesktopConnectionService.js";
+import { IReviewTelemetryService } from "./reviewTelemetryService.js";
 
 type NavigatorAnswer = { workspacePath: string; filePath?: string; rootPath?: string } | ReviewRemoteNavigatorAnswer;
 
@@ -85,6 +86,7 @@ export class ReviewCanvasEditorTabsService extends Disposable implements IReview
 		@IFileService private readonly files: IFileService,
 		@IEnvironmentService private readonly environment: IEnvironmentService,
 		@IConfigurationService private readonly configurationService: IConfigurationService,
+		@IReviewTelemetryService private readonly telemetry: IReviewTelemetryService,
 	) {
 		super();
 		this._register(
@@ -137,6 +139,7 @@ export class ReviewCanvasEditorTabsService extends Disposable implements IReview
 		const result = await this.navigatorWorkspace(selection.reviewId, selection.kind === "version" ? { version: selection.version } : {});
 		const { uri, remote } = navigatorPaths(result);
 		await this.host.openWindow([{ workspaceUri: uri(result.workspacePath), label: title }], { forceNewWindow: true, ...remote });
+		this.telemetry.capture("source_opened", { destination: "source_window", kind: "tree" });
 	}
 
 	/**
@@ -151,26 +154,31 @@ export class ReviewCanvasEditorTabsService extends Disposable implements IReview
 		const destinations = await Promise.all(resources.map(resource => this.sourceDestination(resource)));
 		const { workspaceUri, remote } = destinations[destinations.length - 1];
 		const selection = !diff ? (editor.options as ITextEditorOptions | undefined)?.selection : undefined;
-		if (!diff && destinations[0].external && await this.openExternalEditor(destinations[0], selection)) return true;
+		const external = !diff && destinations[0].external ? await this.openExternalEditor(destinations[0], selection) : undefined;
+		if (external) {
+			this.telemetry.capture("source_opened", { destination: external, kind: "file" });
+			return true;
+		}
 		await this.host.openWindow([
 			{ workspaceUri },
 			...destinations.map(({ fileUri }) => ({ fileUri: selection ? fileUri.with({ path: `${fileUri.path}:${selection.startLineNumber}:${selection.startColumn ?? 1}` }) : fileUri })),
 		], { forceNewWindow: true, gotoLineMode: true, diffMode: diff, ...remote });
+		this.telemetry.capture("source_opened", { destination: "source_window", kind: diff ? "diff" : "file" });
 		return true;
 	}
 
-	private async openExternalEditor({ fileUri, rootPath }: { fileUri: URI; rootPath?: string }, selection: ITextEditorOptions["selection"]): Promise<boolean> {
+	private async openExternalEditor({ fileUri, rootPath }: { fileUri: URI; rootPath?: string }, selection: ITextEditorOptions["selection"]): Promise<string | undefined> {
 		const editor = this.configurationService.getValue<string>(REVIEW_OPEN_FILES_IN_SETTING);
 		if (editor === "application") {
 			// An application takes the file alone: only the editors' URLs carry a line.
 			const application = this.configurationService.getValue<string>(REVIEW_OPEN_FILES_IN_APPLICATION_SETTING);
-			if (!application) return false;
+			if (!application) return undefined;
 			await this.desktopConnection.openInApplication(application, fileUri.fsPath);
-			return true;
+			return editor;
 		}
-		if (!isExternalEditor(editor)) return false;
+		if (!isExternalEditor(editor)) return undefined;
 		await this.desktopConnection.openInExternalEditor({ editor, filePath: fileUri.fsPath, line: selection?.startLineNumber, column: selection?.startColumn, folder: rootPath });
-		return true;
+		return editor;
 	}
 
 	async openSourceReferences(resource: URI, position: { readonly lineNumber: number; readonly column: number }): Promise<boolean> {
@@ -181,6 +189,7 @@ export class ReviewCanvasEditorTabsService extends Disposable implements IReview
 			...remote,
 			reviewReferencesToShow: { resource: fileUri, lineNumber: position.lineNumber, column: position.column },
 		});
+		this.telemetry.capture("source_opened", { destination: "source_window", kind: "references" });
 		return true;
 	}
 

apps/review-desktop/code-oss/src/vs/review/services/reviewDefinitionPeek.ts

@@ -40,6 +40,10 @@ export function markReviewDefinitionPeekHost(container: HTMLElement, diffs: Revi
 	return toDisposable(() => hosts.delete(container));
 }
 
+export function isReviewDefinitionPeekEditor(editor: ICodeEditor): boolean {
+	return !!peeks.get(editor)?.editors.includes(editor);
+}
+
 function hostDiffs(editor: ICodeEditor): ReviewDefinitionPeekDiffs | undefined {
 	// A native peek's preview keeps its own navigation.
 	if (editor instanceof EmbeddedCodeEditorWidget) return undefined;
@@ -169,7 +173,10 @@ export class ReviewDefinitionPeek extends Disposable {
 			const range = (input.options as { selection?: IRange } | undefined)?.selection;
 			if (!source || sideBySide || !range || input.resource.scheme !== REVIEW_API_SOURCE_SCHEME) return null;
 			const peek = peeks.get(source);
-			if (peek?.editors.includes(source)) return peek.setInput(input.resource, range);
+			if (peek?.editors.includes(source)) {
+				telemetry.capture("peek_opened", { via: "diff_definition" });
+				return peek.setInput(input.resource, range);
+			}
 			const diffs = hostDiffs(source);
 			if (!diffs) return null;
 			telemetry.capture("peek_opened", { via: "diff_definition" });

apps/review-desktop/code-oss/src/vs/review/services/reviewEditorResolverService.test.ts

@@ -35,6 +35,7 @@ function sourceResolver(t: TestContext, options: { openFilesIn?: string; applica
 		{} as never,
 		{} as never,
 		{ getValue: (key: string) => ({ "review.openFilesIn": options.openFilesIn, "review.openFilesInApplication": options.application })[key] } as never,
+		{ capture() {} } as never,
 	);
 	const resolver = new ReviewEditorResolverService(
 		{ get activeGroup() { throw new Error("Review must not create an editor group"); } } as never,

docs/telemetry.md

@@ -250,6 +250,7 @@ text, and only as described in "Error reports".
 | `review_map_expanded`             | `level` in system, container, component, code                                                                                                                                  | A user expands a map element                                                                               |
 | `review_commit_diff_opened`       | `via` in row, file, footer                                                                                                                                                     | A user opens a commit diff                                                                                 |
 | `review_source_tree_opened`       | `via` in topbar, home                                                                                                                                                          | A user opens the source tree                                                                               |
+| `review_source_opened` | `destination` in source_window, vscode, cursor, zed, application; `kind` in file, diff, references, tree | A file, diff, references list or source tree opens outside the canvas |
 | `review_whiteboard_file_exported` | none                                                                                                                                                                           | A user exports a board as a .whiteboard file                                                               |
 | `review_client_error`             | See "Error reports"                                                                                                                                                            | A part of Whiteboard reports an error                                                                      |
 | `review_update_started`           | Random `update_attempt_id`, `target_version`                                                                                                                                   | An update is downloaded and ready to install                                                               |
@@ -258,7 +259,7 @@ text, and only as described in "Error reports".
 | `review_bug_report_dialog_opened` | None                                                                                                                                                                           | A user opens the bug report dialog                                                                         |
 | `review_bug_report_cancelled`     | None                                                                                                                                                                           | A user closes the dialog without a report                                                                  |
 | `review_bug_report_send_failed`   | Short `error_name`                                                                                                                                                             | A bug report request fails                                                                                 |
-| `review_setting_changed`          | `setting` in telemetry_enabled, keymap, software_map_enabled, show_release_notes, update_pill, diffr_config, theme, ctrl_tab, ready_notification, document_width, code_font_size, document_font_size, reading_font, reading_font_family, code_font_family, font_ligatures, diff_font_size, diff_word_wrap, diff_ignore_whitespace; `enabled`; `value` in dark, light (theme only) | A user changes a Whiteboard setting                                                                        |
+| `review_setting_changed`          | `setting` in telemetry_enabled, keymap, software_map_enabled, show_release_notes, update_pill, diffr_config, theme, ctrl_tab, open_files_in, ready_notification, document_width, code_font_size, document_font_size, reading_font, reading_font_family, code_font_family, font_ligatures, diff_font_size, diff_word_wrap, diff_ignore_whitespace; `enabled`; `value` in dark, light (theme only); `choice` in whiteboard, vscode, cursor, zed, application (open_files_in only) | A user changes a Whiteboard setting                                                                        |
 | `review_review_opened`            | `via` in home, other                                                                                                                                                           | A user opens a whiteboard                                                                                  |
 | `review_diff_layout_changed`      | `layout` in split, unified; `scope` in diff, peek                                                                                                                              | A user switches the diff layout                                                                            |
 | `review_home_empty_state_viewed`  | None                                                                                                                                                                           | The empty Home state opens                                                                                 |
@@ -299,7 +300,7 @@ session with `outcome: "app_quit"`, the same as quitting.
 
 | Event                             | Additional properties                                                                                                                                                                                                                       | When                                        |
 | --------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------- |
-| `review_lsp_used`                 | `feature` in hover, goto_definition, peek_definition, goto_type_definition, goto_implementation, references, rename, format, code_action, symbol_search; `via` in command, mouse; `language`; `editor_kind` in files_tab, inline_peek, diff | A user invokes an LSP feature               |
+| `review_lsp_used`                 | `feature` in hover, goto_definition, peek_definition, goto_type_definition, goto_implementation, references, rename, format, code_action, symbol_search; `via` in command, mouse; `language`; `editor_kind` in files_tab, inline_peek, diff, definition_peek | A user invokes an LSP feature               |
 | `review_ls_activated`             | `group` in python, go, rust, swift, csharp; `ok`                                                                                                                                                                                            | A language server activation check ends     |
 | `review_extension_installed`      | Allowlisted `extension_id`; `trigger` in user, auto_upgrade, startup_seed, keymap, rollback; `cached`; `duration_ms`                                                                                                                        | An optional extension installs              |
 | `review_extension_install_failed` | Allowlisted `extension_id`; allowlisted `trigger`; `phase` in download, install                                                                                                                                                             | An optional extension install fails         |

packages/review-protocol/src/contracts.ts

@@ -724,12 +724,16 @@ export const REVIEW_KEYMAP_CHOICES = [
 
 export type ReviewKeymapChoice = (typeof REVIEW_KEYMAP_CHOICES)[number];
 
+export const REVIEW_OPEN_FILES_IN_CHOICES = [
+  "whiteboard",
+  "vscode",
+  "cursor",
+  "zed",
+  "application",
+] as const;
+
 export type ReviewOpenFilesInChoice =
-  | "whiteboard"
-  | "vscode"
-  | "cursor"
-  | "zed"
-  | "application";
+  (typeof REVIEW_OPEN_FILES_IN_CHOICES)[number];
 
 /**
  * Where Open file sends a file. `application` is one the reader picked with