From 575e3e4eae8bc541c7fb39b48263c32a2dab9747 Mon Sep 17 00:00:00 2001 From: Vijay Upadya <41652029+vijayupadya@users.noreply.github.com> Date: Fri, 28 Aug 2026 18:54:40 -0700 Subject: [PATCH] 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 --- .../node/baseExperimentationService.ts | 28 +++++++++++- .../test/node/experimentation.spec.ts | 42 +++++++++++++++++ .../assignment/common/assignmentService.ts | 44 +++++++++++++----- .../test/common/assignmentService.test.ts | 45 +++++++++++++++++++ 4 files changed, 146 insertions(+), 13 deletions(-) create mode 100644 src/vs/workbench/services/assignment/test/common/assignmentService.test.ts diff --git a/extensions/copilot/src/platform/telemetry/node/baseExperimentationService.ts b/extensions/copilot/src/platform/telemetry/node/baseExperimentationService.ts index e2ea58919cb..9805d3c0761 100644 --- a/extensions/copilot/src/platform/telemetry/node/baseExperimentationService.ts +++ b/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(name: string): T | undefined { - const result = this._delegate.getTreatmentVariable('vscode', name) as T; + const result = this._readTreatmentVariable(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(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 = new Map(); diff --git a/extensions/copilot/src/platform/telemetry/test/node/experimentation.spec.ts b/extensions/copilot/src/platform/telemetry/test/node/experimentation.spec.ts index 7ad939b60f7..a00317856fa 100644 --- a/extensions/copilot/src/platform/telemetry/test/node/experimentation.spec.ts +++ b/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('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('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. diff --git a/src/vs/workbench/services/assignment/common/assignmentService.ts b/src/vs/workbench/services/assignment/common/assignmentService.ts index 30031408890..0cd95c3b693 100644 --- a/src/vs/workbench/services/assignment/common/assignmentService.ts +++ b/src/vs/workbench/services/assignment/common/assignmentService.ts @@ -42,6 +42,27 @@ export interface IAssignmentFilter { export const IWorkbenchAssignmentService = createDecorator('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(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; 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('vscode', name); - } else { - result = await client.getTreatmentVariableAsync('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('vscode', `${ASSIGNMENTS_SCOPE_PREFIX}${name}`, true); } - result = client.getTreatmentVariable('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(readName => client.getTreatmentVariable('vscode', readName), name); } /** diff --git a/src/vs/workbench/services/assignment/test/common/assignmentService.test.ts b/src/vs/workbench/services/assignment/test/common/assignmentService.test.ts new file mode 100644 index 00000000000..18d92c7817d --- /dev/null +++ b/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): (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); + }); +});