From 9f6d8173a9ebc68dc80fdef06e8cdfa3983a153f Mon Sep 17 00:00:00 2001 From: Adib234 <30782825+Adib234@users.noreply.github.com> Date: Fri, 14 Nov 2025 13:34:54 -0800 Subject: [PATCH] Change flag name to flag id for existing flags (#13073) # Conflicts: # packages/core/src/code_assist/experiments/experiments.test.ts --- .../experiments/experiments.test.ts | 115 ++++++++++++++++++ .../code_assist/experiments/experiments.ts | 4 +- .../src/code_assist/experiments/flagNames.ts | 5 +- .../core/src/code_assist/experiments/types.ts | 2 +- packages/core/src/config/config.ts | 4 +- 5 files changed, 122 insertions(+), 8 deletions(-) create mode 100644 packages/core/src/code_assist/experiments/experiments.test.ts diff --git a/packages/core/src/code_assist/experiments/experiments.test.ts b/packages/core/src/code_assist/experiments/experiments.test.ts new file mode 100644 index 0000000000..a4d9c85fce --- /dev/null +++ b/packages/core/src/code_assist/experiments/experiments.test.ts @@ -0,0 +1,115 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import type { CodeAssistServer } from '../server.js'; +import { getClientMetadata } from './client_metadata.js'; +import type { ListExperimentsResponse, Flag } from './types.js'; + +// Mock dependencies before importing the module under test +vi.mock('../server.js'); +vi.mock('./client_metadata.js'); + +describe('experiments', () => { + let mockServer: CodeAssistServer; + + beforeEach(() => { + // Reset modules to clear the cached `experimentsPromise` + vi.resetModules(); + + // Mock the dependencies that `getExperiments` relies on + vi.mocked(getClientMetadata).mockResolvedValue({ + ideName: 'GEMINI_CLI', + ideVersion: '1.0.0', + platform: 'LINUX_AMD64', + updateChannel: 'stable', + }); + + // Create a mock instance of the server for each test + mockServer = { + listExperiments: vi.fn(), + } as unknown as CodeAssistServer; + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + it('should fetch and parse experiments from the server', async () => { + const { getExperiments } = await import('./experiments.js'); + const mockApiResponse: ListExperimentsResponse = { + flags: [ + { flagId: 234, boolValue: true }, + { flagId: 345, stringValue: 'value' }, + ], + experimentIds: [123, 456], + }; + vi.mocked(mockServer.listExperiments).mockResolvedValue(mockApiResponse); + + const experiments = await getExperiments(mockServer); + + // Verify that the dependencies were called + expect(getClientMetadata).toHaveBeenCalled(); + expect(mockServer.listExperiments).toHaveBeenCalledWith( + await getClientMetadata(), + ); + + // Verify that the response was parsed correctly + expect(experiments.flags[234]).toEqual({ + flagId: 234, + boolValue: true, + }); + expect(experiments.flags[345]).toEqual({ + flagId: 345, + stringValue: 'value', + }); + expect(experiments.experimentIds).toEqual([123, 456]); + }); + + it('should handle an empty or partial response from the server', async () => { + const { getExperiments } = await import('./experiments.js'); + const mockApiResponse: ListExperimentsResponse = {}; // No flags or experimentIds + vi.mocked(mockServer.listExperiments).mockResolvedValue(mockApiResponse); + + const experiments = await getExperiments(mockServer); + + expect(experiments.flags).toEqual({}); + expect(experiments.experimentIds).toEqual([]); + }); + + it('should ignore flags that are missing a name', async () => { + const { getExperiments } = await import('./experiments.js'); + const mockApiResponse: ListExperimentsResponse = { + flags: [ + { boolValue: true } as Flag, // No name + { flagId: 256, stringValue: 'value' }, + ], + }; + vi.mocked(mockServer.listExperiments).mockResolvedValue(mockApiResponse); + + const experiments = await getExperiments(mockServer); + + expect(Object.keys(experiments.flags)).toHaveLength(1); + expect(experiments.flags[256]).toBeDefined(); + expect(experiments.flags['undefined']).toBeUndefined(); + }); + + it('should cache the experiments promise to avoid multiple fetches', async () => { + const { getExperiments } = await import('./experiments.js'); + const mockApiResponse: ListExperimentsResponse = { + experimentIds: [1, 2, 3], + }; + vi.mocked(mockServer.listExperiments).mockResolvedValue(mockApiResponse); + + const firstCall = await getExperiments(mockServer); + const secondCall = await getExperiments(mockServer); + + expect(firstCall).toBe(secondCall); // Should be the exact same promise object + // Verify the underlying functions were only called once + expect(getClientMetadata).toHaveBeenCalledTimes(1); + expect(mockServer.listExperiments).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/core/src/code_assist/experiments/experiments.ts b/packages/core/src/code_assist/experiments/experiments.ts index f716da9c72..9c8d42188f 100644 --- a/packages/core/src/code_assist/experiments/experiments.ts +++ b/packages/core/src/code_assist/experiments/experiments.ts @@ -38,8 +38,8 @@ export async function getExperiments( function parseExperiments(response: ListExperimentsResponse): Experiments { const flags: Record = {}; for (const flag of response.flags ?? []) { - if (flag.name) { - flags[flag.name] = flag; + if (flag.flagId) { + flags[flag.flagId] = flag; } } return { diff --git a/packages/core/src/code_assist/experiments/flagNames.ts b/packages/core/src/code_assist/experiments/flagNames.ts index cb6b6397a4..5425a8e833 100644 --- a/packages/core/src/code_assist/experiments/flagNames.ts +++ b/packages/core/src/code_assist/experiments/flagNames.ts @@ -5,9 +5,8 @@ */ export const ExperimentFlags = { - CONTEXT_COMPRESSION_THRESHOLD: - 'GeminiCLIContextCompression__threshold_fraction', - USER_CACHING: 'GcliUserCaching__user_caching', + CONTEXT_COMPRESSION_THRESHOLD: 45740197, + USER_CACHING: 45740198, } as const; export type ExperimentFlagName = diff --git a/packages/core/src/code_assist/experiments/types.ts b/packages/core/src/code_assist/experiments/types.ts index 924108f347..510f3a7cbe 100644 --- a/packages/core/src/code_assist/experiments/types.ts +++ b/packages/core/src/code_assist/experiments/types.ts @@ -19,7 +19,7 @@ export interface ListExperimentsResponse { } export interface Flag { - name?: string; + flagId?: number; boolValue?: boolean; floatValue?: number; intValue?: string; // int64 diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index b72bf2d9ca..05ab2facd4 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -1455,8 +1455,8 @@ export class Config { this.experiments = experiments; const flagSummaries = Object.entries(experiments.flags ?? {}) .sort(([a], [b]) => a.localeCompare(b)) - .map(([name, flag]) => { - const summary: Record = { name }; + .map(([flagId, flag]) => { + const summary: Record = { flagId }; if (flag.boolValue !== undefined) { summary['boolValue'] = flag.boolValue; }