From 98f3a7066e1aa9701fe1d4c89b046fe03b2420e5 Mon Sep 17 00:00:00 2001 From: Jerop Kipruto Date: Mon, 23 Jun 2025 17:19:40 -0400 Subject: [PATCH] refactor: rename `disableDataCollection` to `dataCollectionEnabled` (#1319) Renames the `disableDataCollection` flag to the more intuitive and positive `dataCollectionEnabled`. This change improves code clarity by avoiding double negatives and making the purpose of the flag more direct. The logic has been inverted wherever the flag is used to accommodate the new naming convention. Using a suffix like `"Enabled"` follows a common convention that improves readability. - A condition like `if (dataCollectionEnabled)` reads like a natural language sentence ("if data collection is enabled"), which reduces cognitive load. - Distinguishes the boolean flag (representing a state) from potential functions that would perform an action (e.g., `enableDataCollection()` or `disableDataCollection()`), avoiding ambiguity between checking a value and calling a function. #750 --- packages/cli/src/config/config.ts | 3 ++- packages/cli/src/ui/hooks/useGeminiStream.test.tsx | 2 +- packages/cli/src/ui/hooks/useToolScheduler.test.ts | 2 +- packages/core/src/config/config.ts | 10 +++++----- packages/core/src/core/coreToolScheduler.test.ts | 2 +- packages/core/src/core/geminiChat.test.ts | 2 +- .../core/src/core/nonInteractiveToolExecutor.test.ts | 2 +- .../src/telemetry/clearcut-logger/clearcut-logger.ts | 2 +- packages/core/src/telemetry/loggers.test.ts | 12 ++++++------ 9 files changed, 19 insertions(+), 18 deletions(-) diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index e6f8d423ff..4e5b00086c 100644 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -226,7 +226,8 @@ export async function loadCliConfig( process.env.OTEL_EXPORTER_OTLP_ENDPOINT ?? settings.telemetry?.otlpEndpoint, logPrompts: argv.telemetryLogPrompts ?? settings.telemetry?.logPrompts, - disableDataCollection: settings.telemetry?.disableDataCollection ?? false, + usageStatisticsEnabled: + settings.telemetry?.usageStatisticsEnabled ?? true, }, // Git-aware file filtering settings fileFiltering: { diff --git a/packages/cli/src/ui/hooks/useGeminiStream.test.tsx b/packages/cli/src/ui/hooks/useGeminiStream.test.tsx index cb5b35b45a..c5145eda34 100644 --- a/packages/cli/src/ui/hooks/useGeminiStream.test.tsx +++ b/packages/cli/src/ui/hooks/useGeminiStream.test.tsx @@ -288,7 +288,7 @@ describe('useGeminiStream', () => { getProjectRoot: vi.fn(() => '/test/dir'), getCheckpointingEnabled: vi.fn(() => false), getGeminiClient: mockGetGeminiClient, - getDisableDataCollection: () => false, + getUsageStatisticsEnabled: () => true, addHistory: vi.fn(), } as unknown as Config; mockOnDebugMessage = vi.fn(); diff --git a/packages/cli/src/ui/hooks/useToolScheduler.test.ts b/packages/cli/src/ui/hooks/useToolScheduler.test.ts index 4c8901dcff..69d72cdc60 100644 --- a/packages/cli/src/ui/hooks/useToolScheduler.test.ts +++ b/packages/cli/src/ui/hooks/useToolScheduler.test.ts @@ -48,7 +48,7 @@ const mockToolRegistry = { const mockConfig = { getToolRegistry: vi.fn(() => mockToolRegistry as unknown as ToolRegistry), getApprovalMode: vi.fn(() => ApprovalMode.DEFAULT), - getDisableDataCollection: () => false, + getUsageStatisticsEnabled: () => true, }; const mockTool: Tool = { diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index d9161faefa..65d69a413a 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -57,7 +57,7 @@ export interface TelemetrySettings { target?: TelemetryTarget; otlpEndpoint?: string; logPrompts?: boolean; - disableDataCollection?: boolean; + usageStatisticsEnabled?: boolean; } export class MCPServerConfig { @@ -181,7 +181,7 @@ export class Config { target: params.telemetry?.target ?? DEFAULT_TELEMETRY_TARGET, otlpEndpoint: params.telemetry?.otlpEndpoint ?? DEFAULT_OTLP_ENDPOINT, logPrompts: params.telemetry?.logPrompts ?? true, - disableDataCollection: params.telemetry?.disableDataCollection ?? false, + usageStatisticsEnabled: params.telemetry?.usageStatisticsEnabled ?? true, }; this.fileFiltering = { @@ -205,7 +205,7 @@ export class Config { initializeTelemetry(this); } - if (!this.getDisableDataCollection()) { + if (this.getUsageStatisticsEnabled()) { ClearcutLogger.getInstance(this)?.logStartSessionEvent( new StartSessionEvent(this), ); @@ -385,8 +385,8 @@ export class Config { return this.fileDiscoveryService; } - getDisableDataCollection(): boolean { - return this.telemetrySettings.disableDataCollection ?? false; + getUsageStatisticsEnabled(): boolean { + return this.telemetrySettings.usageStatisticsEnabled ?? true; } getExtensionContextFilePaths(): string[] { diff --git a/packages/core/src/core/coreToolScheduler.test.ts b/packages/core/src/core/coreToolScheduler.test.ts index 656f895245..f80a1e37e2 100644 --- a/packages/core/src/core/coreToolScheduler.test.ts +++ b/packages/core/src/core/coreToolScheduler.test.ts @@ -77,7 +77,7 @@ describe('CoreToolScheduler', () => { const mockConfig = { getSessionId: () => 'test-session-id', - getDisableDataCollection: () => false, + getUsageStatisticsEnabled: () => true, } as Config; const scheduler = new CoreToolScheduler({ diff --git a/packages/core/src/core/geminiChat.test.ts b/packages/core/src/core/geminiChat.test.ts index 45c9b06e1a..0ba8b05386 100644 --- a/packages/core/src/core/geminiChat.test.ts +++ b/packages/core/src/core/geminiChat.test.ts @@ -27,7 +27,7 @@ const mockModelsModule = { const mockConfig = { getSessionId: () => 'test-session-id', getTelemetryLogPromptsEnabled: () => true, - getDisableDataCollection: () => false, + getUsageStatisticsEnabled: () => true, } as unknown as Config; describe('GeminiChat', () => { diff --git a/packages/core/src/core/nonInteractiveToolExecutor.test.ts b/packages/core/src/core/nonInteractiveToolExecutor.test.ts index c6874c6ec4..5f22c3b812 100644 --- a/packages/core/src/core/nonInteractiveToolExecutor.test.ts +++ b/packages/core/src/core/nonInteractiveToolExecutor.test.ts @@ -18,7 +18,7 @@ import { Part, Type } from '@google/genai'; const mockConfig = { getSessionId: () => 'test-session-id', - getDisableDataCollection: () => false, + getUsageStatisticsEnabled: () => true, } as unknown as Config; describe('executeToolCall', () => { diff --git a/packages/core/src/telemetry/clearcut-logger/clearcut-logger.ts b/packages/core/src/telemetry/clearcut-logger/clearcut-logger.ts index 5933de1351..b29f41331a 100644 --- a/packages/core/src/telemetry/clearcut-logger/clearcut-logger.ts +++ b/packages/core/src/telemetry/clearcut-logger/clearcut-logger.ts @@ -46,7 +46,7 @@ export class ClearcutLogger { } static getInstance(config?: Config): ClearcutLogger | undefined { - if (config === undefined || config?.getDisableDataCollection()) + if (config === undefined || !config?.getUsageStatisticsEnabled()) return undefined; if (!ClearcutLogger.instance) { ClearcutLogger.instance = new ClearcutLogger(config); diff --git a/packages/core/src/telemetry/loggers.test.ts b/packages/core/src/telemetry/loggers.test.ts index 2659f39892..2d7835bf89 100644 --- a/packages/core/src/telemetry/loggers.test.ts +++ b/packages/core/src/telemetry/loggers.test.ts @@ -71,7 +71,7 @@ describe('loggers', () => { authType: AuthType.USE_VERTEX_AI, }), getTelemetryEnabled: () => true, - getDisableDataCollection: () => false, + getUsageStatisticsEnabled: () => true, getTelemetryLogPromptsEnabled: () => true, getFileFilteringRespectGitIgnore: () => true, getFileFilteringAllowBuildArtifacts: () => false, @@ -116,7 +116,7 @@ describe('loggers', () => { getSessionId: () => 'test-session-id', getTelemetryEnabled: () => true, getTelemetryLogPromptsEnabled: () => true, - getDisableDataCollection: () => false, + getUsageStatisticsEnabled: () => true, } as unknown as Config; it('should log a user prompt', () => { @@ -142,7 +142,7 @@ describe('loggers', () => { getTelemetryEnabled: () => true, getTelemetryLogPromptsEnabled: () => false, getTargetDir: () => 'target-dir', - getDisableDataCollection: () => false, + getUsageStatisticsEnabled: () => true, } as unknown as Config; const event = new UserPromptEvent(11, 'test-prompt'); @@ -164,7 +164,7 @@ describe('loggers', () => { const mockConfig = { getSessionId: () => 'test-session-id', getTargetDir: () => 'target-dir', - getDisableDataCollection: () => false, + getUsageStatisticsEnabled: () => true, getTelemetryEnabled: () => true, getTelemetryLogPromptsEnabled: () => true, } as Config; @@ -270,7 +270,7 @@ describe('loggers', () => { const mockConfig = { getSessionId: () => 'test-session-id', getTargetDir: () => 'target-dir', - getDisableDataCollection: () => false, + getUsageStatisticsEnabled: () => true, getTelemetryEnabled: () => true, getTelemetryLogPromptsEnabled: () => true, } as Config; @@ -347,7 +347,7 @@ describe('loggers', () => { getSessionId: () => 'test-session-id', getTargetDir: () => 'target-dir', getGeminiClient: () => mockGeminiClient, - getDisableDataCollection: () => false, + getUsageStatisticsEnabled: () => true, getTelemetryEnabled: () => true, getTelemetryLogPromptsEnabled: () => true, } as Config;