Adds more edit telemetry tests

[hediet]

Closes https://github.com/microsoft/vscode-internalbacklog/issues/6239

[Copilot]

Pull Request Overview

This PR refactors the edit telemetry architecture by introducing a new IRandomService abstraction for UUID generation and extracting shared code into separate modules.

Key changes:

• Introduced IRandomService interface and RandomService implementation to abstract UUID generation for better testability
• Extracted SCM adapter logic from editSourceTrackingImpl.ts into a new scmAdapter.ts file
• Extracted ArcTelemetryReporter class into its own file arcTelemetryReporter.ts
• Renamed classes to be more descriptive of their purpose
• Added a comprehensive integration test for edit telemetry
• Made several helper classes and methods more accessible for testing

Reviewed Changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
editTelemetry.test.ts	New comprehensive integration test for edit telemetry functionality
scmAdapter.ts	New file containing SCM adapter classes extracted from editSourceTrackingImpl
editTracker.ts	Made getTrackedRanges() and isEmpty() public for testing
editSourceTrackingImpl.ts	Refactored to use extracted modules and new IRandomService
editSourceTrackingFeature.ts	Updated to use IAnnotatedDocuments interface
arcTelemetrySender.ts	Renamed classes, extracted ArcTelemetryReporter, improved lifecycle management
arcTelemetryReporter.ts	New file containing extracted ArcTelemetryReporter class
aiEditTelemetryServiceImpl.ts	Updated to use IRandomService instead of direct UUID generation
randomService.ts	New service abstraction for random/UUID generation
utils.ts	Added disposal check to prevent issues with disposed stores
observableWorkspace.ts	Simplified observable usage, added static helper method
documentWithAnnotatedEdits.ts	Extracted DiffService class, removed debug line
annotatedDocuments.ts	Extracted UriVisibilityProvider, added IAnnotatedDocuments interface
editTelemetry.contribution.ts	Registered IRandomService singleton
instantiationServiceMock.ts	Added stubInstance method for testing
random.ts	Added nextUuid() method for deterministic test UUID generation
textModelEditSource.ts	Made EditSuggestionId.newId() accept optional UUID generator

[Copilot]

Changing the return type from IObservableWithChange to IObservable removes change tracking information that might be needed by consumers. This is a breaking API change that could impact functionality relying on change deltas.

See below for a potential fix:

/**
 * Describes the change delta for the documents array.
 */
export interface DocumentArrayChange {
	/**
	 * The documents that were added.
	 */
	added?: readonly IObservableDocument[];
	/**
	 * The documents that were removed.
	 */
	removed?: readonly IObservableDocument[];
}
export abstract class ObservableWorkspace {
	/**
	 * Returns the observable array of documents, with change tracking information.
	 */
	abstract get documents(): IObservableWithChange<readonly IObservableDocument[], DocumentArrayChange>;

[Copilot]

Using reader.store within a derived computation creates disposables that are tied to the reader's lifecycle. However, this pattern can lead to memory leaks if the derived is read frequently. Consider using a stable DisposableStore or ensuring proper cleanup.

[Copilot]

The _onBeforeDispose callback is registered in the disposable store but it's called synchronously during disposal. This means the callback will be invoked after all other disposables in the store have been disposed (reverse registration order). If the callback needs to run before other cleanup, consider calling it directly in an overridden dispose() method before calling super.dispose().

[Copilot]

The comment states 'Namespace should be 3 letter' but there's no enforcement or validation. Consider either enforcing this requirement (e.g., with an assertion or parameter type) or clarifying in the documentation that this is a convention, not a requirement.

Suggested change

	/** Namespace should be 3 letter. */
	generatePrefixedUuid(namespace: string): string {
	/**
	* Namespace must be exactly 3 letters.
	* @throws Error if namespace is not exactly 3 letters.
	*/
	generatePrefixedUuid(namespace: string): string {
	if (!/^[A-Za-z]{3}$/.test(namespace)) {
	throw new Error(`Namespace must be exactly 3 letters, got: "${namespace}"`);
	}

[Copilot]

[nitpick] The DiffService class wraps a single method from IEditorWorkerService. This adds an unnecessary abstraction layer. Consider either extending its functionality to justify the wrapper class, or use IEditorWorkerService directly where needed.

Suggested change

	export class DiffService {
	constructor(
	@IEditorWorkerService private readonly _editorWorkerService: IEditorWorkerService,
	) {
	}
	public async computeDiff(original: string, modified: string): Promise<StringEdit> {
	const diffEdit = await this._editorWorkerService.computeStringEditFromDiff(original, modified, { maxComputationTimeMs: 500 }, 'advanced');
	return diffEdit;
	}
	}