refactor(experiments): improve type safety for experiment configuration

Change ExperimentId type to be value-based rather than key-based
Make experiment record types more strict with proper typing
Pass full experiment config object instead of single boolean flag
Update type definitions and usages across codebase
This commit is contained in:
sam hoang
2025-01-27 14:59:52 +07:00
parent bb84d79af1
commit 3ed8540eba
6 changed files with 28 additions and 24 deletions

View File

@@ -61,7 +61,7 @@ import { OpenRouterHandler } from "../api/providers/openrouter"
import { McpHub } from "../services/mcp/McpHub" import { McpHub } from "../services/mcp/McpHub"
import crypto from "crypto" import crypto from "crypto"
import { insertGroups } from "./diff/insert-groups" import { insertGroups } from "./diff/insert-groups"
import { EXPERIMENT_IDS } from "../shared/experiments" import { EXPERIMENT_IDS, experiments as Experiments } from "../shared/experiments"
const cwd = const cwd =
vscode.workspace.workspaceFolders?.map((folder) => folder.uri.fsPath).at(0) ?? path.join(os.homedir(), "Desktop") // may or may not exist but fs checking existence would immediately ask for permission which would be bad UX, need to come up with a better solution vscode.workspace.workspaceFolders?.map((folder) => folder.uri.fsPath).at(0) ?? path.join(os.homedir(), "Desktop") // may or may not exist but fs checking existence would immediately ask for permission which would be bad UX, need to come up with a better solution
@@ -117,7 +117,7 @@ export class Cline {
task?: string | undefined, task?: string | undefined,
images?: string[] | undefined, images?: string[] | undefined,
historyItem?: HistoryItem | undefined, historyItem?: HistoryItem | undefined,
experimentalDiffStrategy: boolean = false, experiments?: Record<string, boolean>,
) { ) {
if (!task && !images && !historyItem) { if (!task && !images && !historyItem) {
throw new Error("Either historyItem or task/images must be provided") throw new Error("Either historyItem or task/images must be provided")
@@ -139,7 +139,7 @@ export class Cline {
} }
// Initialize diffStrategy based on current state // Initialize diffStrategy based on current state
this.updateDiffStrategy(experimentalDiffStrategy) this.updateDiffStrategy(Experiments.isEnabled(experiments ?? {}, EXPERIMENT_IDS.DIFF_STRATEGY))
if (task || images) { if (task || images) {
this.startTask(task, images) this.startTask(task, images)

View File

@@ -45,6 +45,7 @@ import {
experimentConfigs, experimentConfigs,
experiments as Experiments, experiments as Experiments,
experimentDefault, experimentDefault,
ExperimentId,
} from "../../shared/experiments" } from "../../shared/experiments"
import { CustomSupportPrompts, supportPrompt } from "../../shared/support-prompt" import { CustomSupportPrompts, supportPrompt } from "../../shared/support-prompt"
@@ -360,7 +361,7 @@ export class ClineProvider implements vscode.WebviewViewProvider {
task, task,
images, images,
undefined, undefined,
Experiments.isEnabled(experiments, EXPERIMENT_IDS.DIFF_STRATEGY), experiments,
) )
} }
@@ -388,7 +389,7 @@ export class ClineProvider implements vscode.WebviewViewProvider {
undefined, undefined,
undefined, undefined,
historyItem, historyItem,
Experiments.isEnabled(experiments, EXPERIMENT_IDS.DIFF_STRATEGY), experiments,
) )
} }
@@ -1222,7 +1223,7 @@ export class ClineProvider implements vscode.WebviewViewProvider {
const updatedExperiments = { const updatedExperiments = {
...((await this.getGlobalState("experiments")) ?? experimentDefault), ...((await this.getGlobalState("experiments")) ?? experimentDefault),
...message.values, ...message.values,
} } as Record<ExperimentId, boolean>
await this.updateGlobalState("experiments", updatedExperiments) await this.updateGlobalState("experiments", updatedExperiments)
@@ -2132,7 +2133,7 @@ export class ClineProvider implements vscode.WebviewViewProvider {
this.getGlobalState("enhancementApiConfigId") as Promise<string | undefined>, this.getGlobalState("enhancementApiConfigId") as Promise<string | undefined>,
this.getGlobalState("autoApprovalEnabled") as Promise<boolean | undefined>, this.getGlobalState("autoApprovalEnabled") as Promise<boolean | undefined>,
this.customModesManager.getCustomModes(), this.customModesManager.getCustomModes(),
this.getGlobalState("experiments") as Promise<Record<string, boolean> | undefined>, this.getGlobalState("experiments") as Promise<Record<ExperimentId, boolean> | undefined>,
]) ])
let apiProvider: ApiProvider let apiProvider: ApiProvider

View File

@@ -639,7 +639,7 @@ describe("ClineProvider", () => {
"Test task", "Test task",
undefined, undefined,
undefined, undefined,
false, experimentDefault,
) )
}) })
test("handles mode-specific custom instructions updates", async () => { test("handles mode-specific custom instructions updates", async () => {

View File

@@ -6,6 +6,7 @@ import { McpServer } from "./mcp"
import { GitCommit } from "../utils/git" import { GitCommit } from "../utils/git"
import { Mode, CustomModePrompts, ModeConfig } from "./modes" import { Mode, CustomModePrompts, ModeConfig } from "./modes"
import { CustomSupportPrompts } from "./support-prompt" import { CustomSupportPrompts } from "./support-prompt"
import { ExperimentId } from "./experiments"
export interface LanguageModelChatSelector { export interface LanguageModelChatSelector {
vendor?: string vendor?: string
@@ -108,7 +109,7 @@ export interface ExtensionState {
mode: Mode mode: Mode
modeApiConfigs?: Record<Mode, string> modeApiConfigs?: Record<Mode, string>
enhancementApiConfigId?: string enhancementApiConfigId?: string
experiments: Record<string, boolean> // Map of experiment IDs to their enabled state experiments: Record<ExperimentId, boolean> // Map of experiment IDs to their enabled state
autoApprovalEnabled?: boolean autoApprovalEnabled?: boolean
customModes: ModeConfig[] customModes: ModeConfig[]
toolRequirements?: Record<string, boolean> // Map of tool names to their requirements (e.g. {"apply_diff": true} if diffEnabled) toolRequirements?: Record<string, boolean> // Map of tool names to their requirements (e.g. {"apply_diff": true} if diffEnabled)

View File

@@ -1,19 +1,22 @@
export interface ExperimentConfig {
id: string
name: string
description: string
enabled: boolean
}
export const EXPERIMENT_IDS = { export const EXPERIMENT_IDS = {
DIFF_STRATEGY: "experimentalDiffStrategy", DIFF_STRATEGY: "experimentalDiffStrategy",
SEARCH_AND_REPLACE: "search_and_replace", SEARCH_AND_REPLACE: "search_and_replace",
INSERT_BLOCK: "insert_code_block", INSERT_BLOCK: "insert_code_block",
} as const } as const
export type ExperimentId = keyof typeof EXPERIMENT_IDS export type ExperimentKey = keyof typeof EXPERIMENT_IDS
export type ExperimentId = valueof<typeof EXPERIMENT_IDS>
export const experimentConfigsMap: Record<ExperimentId, ExperimentConfig> = { export interface ExperimentConfig {
id: ExperimentId
name: string
description: string
enabled: boolean
}
type valueof<X> = X[keyof X]
export const experimentConfigsMap: Record<ExperimentKey, ExperimentConfig> = {
DIFF_STRATEGY: { DIFF_STRATEGY: {
id: EXPERIMENT_IDS.DIFF_STRATEGY, id: EXPERIMENT_IDS.DIFF_STRATEGY,
name: "Use experimental unified diff strategy", name: "Use experimental unified diff strategy",
@@ -42,13 +45,13 @@ export const experimentConfigsMap: Record<ExperimentId, ExperimentConfig> = {
export const experimentConfigs = Object.values(experimentConfigsMap) export const experimentConfigs = Object.values(experimentConfigsMap)
export const experimentDefault = Object.fromEntries( export const experimentDefault = Object.fromEntries(
Object.entries(experimentConfigsMap).map(([_, config]) => [config.id, config.enabled]), Object.entries(experimentConfigsMap).map(([_, config]) => [config.id, config.enabled]),
) ) as Record<ExperimentId, boolean>
export const experiments = { export const experiments = {
get: (id: ExperimentId): ExperimentConfig | undefined => { get: (id: ExperimentKey): ExperimentConfig | undefined => {
return experimentConfigsMap[id] return experimentConfigsMap[id]
}, },
isEnabled: (experimentsConfig: Record<string, boolean>, id: string): boolean => { isEnabled: (experimentsConfig: Record<ExperimentId, boolean>, id: ExperimentId): boolean => {
return experimentsConfig[id] ?? experimentDefault[id] return experimentsConfig[id] ?? experimentDefault[id]
}, },
} as const } as const

View File

@@ -16,7 +16,7 @@ import { McpServer } from "../../../src/shared/mcp"
import { checkExistKey } from "../../../src/shared/checkExistApiConfig" import { checkExistKey } from "../../../src/shared/checkExistApiConfig"
import { Mode, CustomModePrompts, defaultModeSlug, defaultPrompts, ModeConfig } from "../../../src/shared/modes" import { Mode, CustomModePrompts, defaultModeSlug, defaultPrompts, ModeConfig } from "../../../src/shared/modes"
import { CustomSupportPrompts } from "../../../src/shared/support-prompt" import { CustomSupportPrompts } from "../../../src/shared/support-prompt"
import { experimentDefault } from "../../../src/shared/experiments" import { experimentDefault, ExperimentId } from "../../../src/shared/experiments"
export interface ExtensionStateContextType extends ExtensionState { export interface ExtensionStateContextType extends ExtensionState {
didHydrateState: boolean didHydrateState: boolean
@@ -64,8 +64,7 @@ export interface ExtensionStateContextType extends ExtensionState {
setCustomSupportPrompts: (value: CustomSupportPrompts) => void setCustomSupportPrompts: (value: CustomSupportPrompts) => void
enhancementApiConfigId?: string enhancementApiConfigId?: string
setEnhancementApiConfigId: (value: string) => void setEnhancementApiConfigId: (value: string) => void
experiments: Record<string, boolean> setExperimentEnabled: (id: ExperimentId, enabled: boolean) => void
setExperimentEnabled: (id: string, enabled: boolean) => void
setAutoApprovalEnabled: (value: boolean) => void setAutoApprovalEnabled: (value: boolean) => void
handleInputChange: (field: keyof ApiConfiguration) => (event: any) => void handleInputChange: (field: keyof ApiConfiguration) => (event: any) => void
customModes: ModeConfig[] customModes: ModeConfig[]