Handle prefix in exp treatements by new endpoint (#333282)

* Handle prefix in exp treatements by new endpoint

* Update comment formatting

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Updates to make new endpoint win on collision

* Move scoped-treatment tests into a dedicated suite

Addresses PR review: the scoped-lookup tests do not exercise delegate
recreation, so they belong in their own suite rather than in
'ExP Service delegate recreation'.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 15b73982-0eed-4302-a104-a946d0382b4c

---------

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 15b73982-0eed-4302-a104-a946d0382b4c

Author: 3 people

extensions/copilot/src/platform/telemetry/node/baseExperimentationService.ts

@@ -15,6 +15,14 @@ import { IVSCodeExtensionContext } from '../../extContext/common/extensionContex
 import { ILogService } from '../../log/common/logService';
 import { IExperimentationService, TreatmentsChangeEvent } from '../common/nullExperimentationService';
 
+/**
+ * Scope prefix that the new TAS assignments endpoint (`/api/v1/assignments`) prepends to the
+ * feature variable keys it returns (e.g. `/vscode/config.chat...`). The legacy endpoint and
+ * callers query treatments by the bare name, so this prefix must be accounted for when a bare
+ * lookup misses. This is an interim workaround until vscode-tas-client strips the scope itself.
+ */
+const ASSIGNMENTS_SCOPE_PREFIX = '/vscode/';
+
 export class UserInfoStore extends Disposable {
 	private _internalOrg: string | undefined;
 	private _sku: string | undefined;
@@ -244,7 +252,7 @@ export class BaseExperimentationService extends Disposable implements IExperimen
 	private _signalTreatmentsChangeEvent = () => {
 		const affectedTreatmentVariables: string[] = [];
 		for (const [key, previousValue] of this._previouslyReadTreatments) {
-			const currentValue = this._delegate.getTreatmentVariable('vscode', key);
+			const currentValue = this._readTreatmentVariable(key);
 			if (currentValue !== previousValue) {
 				this._logService.trace(`[BaseExperimentationService] Treatment changed: ${key} from ${previousValue} to ${currentValue}`);
 				this._previouslyReadTreatments.set(key, currentValue);
@@ -267,11 +275,27 @@ export class BaseExperimentationService extends Disposable implements IExperimen
 	}
 
 	getTreatmentVariable<T extends boolean | number | string>(name: string): T | undefined {
-		const result = this._delegate.getTreatmentVariable('vscode', name) as T;
+		const result = this._readTreatmentVariable<T>(name);
 		this._previouslyReadTreatments.set(name, result);
 		return result;
 	}
 
+	/**
+	 * Reads a treatment, preferring the `/vscode/`-scoped key over the bare key. Interim workaround
+	 * till its fixed upstream: the new TAS assignments endpoint namespaces its returned feature
+	 * variable keys with a `/vscode/` scope that vscode-tas-client does not strip. Reading the
+	 * scoped key first makes the new endpoint win over the legacy (bare) key when both assign a
+	 * treatment - matching the behavior once the scope is stripped upstream - while still resolving
+	 * legacy-only treatments from the bare key.
+	 */
+	private _readTreatmentVariable<T extends boolean | number | string>(name: string): T | undefined {
+		let result = this._delegate.getTreatmentVariable('vscode', `${ASSIGNMENTS_SCOPE_PREFIX}${name}`) as T | undefined;
+		if (result === undefined) {
+			result = this._delegate.getTreatmentVariable('vscode', name) as T | undefined;
+		}
+		return result;
+	}
+
 	// Note: This is only temporarily until we have fully migrated to the new completions implementation.
 	// At that point, we can remove this method and the related code.
 	private _completionsFilters: Map<string, string> = new Map<string, string>();

extensions/copilot/src/platform/telemetry/test/node/experimentation.spec.ts

@@ -135,6 +135,13 @@ class MockTASExperimentationService implements ITASExperimentationService {
 			return undefined;
 		}
 
+		// This suite models treatments served by the legacy endpoint (bare keys). The new
+		// assignments endpoint does not assign these, so scoped lookups resolve to undefined,
+		// exercising the service's bare-key fallback.
+		if (name.startsWith('/vscode/')) {
+			return undefined;
+		}
+
 		const org = this.userInfoStore.internalOrg;
 		const sku = this.userInfoStore.sku;
 
@@ -826,6 +833,41 @@ describe('ExP Service delegate recreation', () => {
 	});
 });
 
+describe('ExP Service scoped treatment resolution', () => {
+	let accessor: ITestingServicesAccessor;
+
+	beforeAll(() => {
+		const testingServiceCollection = createPlatformServices();
+		accessor = testingServiceCollection.createTestingAccessor();
+	});
+
+	const create = () => accessor.get(IInstantiationService).createInstance(RecreatableExperimentationService);
+
+	it('resolves a treatment served only under the /vscode/ assignments scope prefix', () => {
+		const service = create();
+		const delegate = service.delegates[0];
+
+		// The new assignments endpoint returns the key with a `/vscode/` scope prefix only.
+		delegate.setTreatment('/vscode/config.chat.copilot.subagentModelGuidance.enabled', true);
+
+		expect(service.getTreatmentVariable<boolean>('config.chat.copilot.subagentModelGuidance.enabled')).toBe(true);
+
+		service.dispose();
+	});
+
+	it('prefers the /vscode/ scoped key (new endpoint) over the bare key on collision', () => {
+		const service = create();
+		const delegate = service.delegates[0];
+
+		delegate.setTreatment('config.foo', 'bare');
+		delegate.setTreatment('/vscode/config.foo', 'scoped');
+
+		expect(service.getTreatmentVariable<string>('config.foo')).toBe('scoped');
+
+		service.dispose();
+	});
+});
+
 /**
  * Records every request routed through the fetcher service so a test can assert that both TAS
  * endpoints go through it (proxy-aware transport) with the expected method and call site.

src/vs/workbench/services/assignment/common/assignmentService.ts

@@ -42,6 +42,27 @@ export interface IAssignmentFilter {
 
 export const IWorkbenchAssignmentService = createDecorator<IWorkbenchAssignmentService>('assignmentService');
 
+/**
+ * Scope prefix that the new TAS assignments endpoint (`/api/v1/assignments`) prepends to the
+ * feature variable keys it returns (e.g. `/vscode/config.chat...`). The legacy endpoint and
+ * VS Code both query treatments by the bare name, so this prefix must be accounted for when a
+ * bare lookup misses. This is an interim workaround until tas-client strips the scope itself.
+ */
+const ASSIGNMENTS_SCOPE_PREFIX = '/vscode/';
+
+/**
+ * Resolves a treatment value preferring the `/vscode/`-scoped key emitted by the new TAS
+ * assignments endpoint over the bare key used by the legacy endpoint, so the new endpoint wins
+ * when both assign a treatment (matching the behavior once tas-client strips the scope itself).
+ * Falls back to the bare key for treatments served only by the legacy endpoint.
+ *
+ * Exported for testing.
+ */
+export function resolveScopedTreatment<T extends string | number | boolean>(read: (name: string) => T | undefined, name: string): T | undefined {
+	const scoped = read(`${ASSIGNMENTS_SCOPE_PREFIX}${name}`);
+	return scoped !== undefined ? scoped : read(name);
+}
+
 export interface IWorkbenchAssignmentService extends IAssignmentService {
 	getCurrentExperiments(): Promise<string[] | undefined>;
 	addTelemetryAssignmentFilter(filter: IAssignmentFilter): void;
@@ -268,21 +289,22 @@ export class WorkbenchAssignmentService extends Disposable implements IAssignmen
 			return undefined;
 		}
 
-		let result: T | undefined;
 		const client = await this.tasClient;
 
-		// The TAS client is initialized but we need to check if the initial fetch has completed yet
-		// If it is complete, return a cached value for the treatment
-		// If not, use the async call with `checkCache: true`. This will allow the module to return a cached value if it is present.
-		// Otherwise it will await the initial fetch to return the most up to date value.
-		if (this.networkInitialized) {
-			result = client.getTreatmentVariable<T>('vscode', name);
-		} else {
-			result = await client.getTreatmentVariableAsync<T>('vscode', name, true);
+		// Await the initial network fetch when it has not completed yet, so treatments are
+		// available before we read them from memory. `checkCache: true` returns immediately when a
+		// value is already cached, otherwise it awaits the initial fetch.
+		if (!this.networkInitialized) {
+			await client.getTreatmentVariableAsync<T>('vscode', `${ASSIGNMENTS_SCOPE_PREFIX}${name}`, true);
 		}
 
-		result = client.getTreatmentVariable<T>('vscode', name);
-		return result;
+		// Interim workaround: the new TAS assignments endpoint (/api/v1/assignments) namespaces its
+		// returned feature variable keys with a `/vscode/` scope, whereas the legacy endpoint and
+		// VS Code query treatments by the bare name. Read the scoped key first so the new endpoint
+		// wins over the legacy (bare) key when both assign a treatment - matching the behavior once
+		// tas-client strips the scope itself. Fall back to the bare key for treatments served only
+		// by the legacy endpoint.
+		return resolveScopedTreatment<T>(readName => client.getTreatmentVariable<T>('vscode', readName), name);
 	}
 
 	/**

src/vs/workbench/services/assignment/test/common/assignmentService.test.ts

@@ -0,0 +1,45 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import assert from 'assert';
+import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js';
+import { resolveScopedTreatment } from '../../common/assignmentService.js';
+
+suite('resolveScopedTreatment', () => {
+
+	ensureNoDisposablesAreLeakedInTestSuite();
+
+	const BARE = 'config.chat.agentHost.copilot.multiTurnContextRouting.enabled';
+	const SCOPED = `/vscode/${BARE}`;
+
+	function readFrom(values: Record<string, string | number | boolean>): (name: string) => string | number | boolean | undefined {
+		return name => values[name];
+	}
+
+	test('prefers the /vscode/ scoped value (new endpoint) over the bare value on collision', () => {
+		const read = readFrom({ [BARE]: 'legacy', [SCOPED]: 'new' });
+		assert.strictEqual(resolveScopedTreatment(read, BARE), 'new');
+	});
+
+	test('falls back to the bare value when only the legacy endpoint assigns it', () => {
+		const read = readFrom({ [BARE]: 'legacy' });
+		assert.strictEqual(resolveScopedTreatment(read, BARE), 'legacy');
+	});
+
+	test('uses the scoped value when only the new endpoint assigns it', () => {
+		const read = readFrom({ [SCOPED]: 'new' });
+		assert.strictEqual(resolveScopedTreatment(read, BARE), 'new');
+	});
+
+	test('returns undefined when neither endpoint assigns it', () => {
+		const read = readFrom({});
+		assert.strictEqual(resolveScopedTreatment(read, BARE), undefined);
+	});
+
+	test('preserves a defined falsy scoped value instead of falling back to bare', () => {
+		const read = readFrom({ [BARE]: true, [SCOPED]: false });
+		assert.strictEqual(resolveScopedTreatment(read, BARE), false);
+	});
+});