Merge pull request #2841 from Kilo-Org/mark/remove-error-fix-strategy

remove ErrorFixStrategy

Author: markijbema

src/services/ghost/PromptStrategyManager.ts

@@ -3,7 +3,6 @@ import { PromptStrategy } from "./types/PromptStrategy"
 
 // Import all strategies
 import { UserRequestStrategy } from "./strategies/UserRequestStrategy"
-import { ErrorFixStrategy } from "./strategies/ErrorFixStrategy"
 import { SelectionRefactorStrategy } from "./strategies/SelectionRefactorStrategy"
 import { CommentDrivenStrategy } from "./strategies/CommentDrivenStrategy"
 import { NewLineCompletionStrategy } from "./strategies/NewLineCompletionStrategy"
@@ -15,6 +14,7 @@ import { AutoTriggerStrategy } from "./strategies/AutoTriggerStrategy"
  */
 export class PromptStrategyManager {
 	private strategies: PromptStrategy[]
+	private autoTriggerStrategy: AutoTriggerStrategy
 	private debug: boolean
 
 	constructor(options?: { debug: boolean }) {
@@ -27,9 +27,8 @@ export class PromptStrategyManager {
 			new NewLineCompletionStrategy(),
 			new CommentDrivenStrategy(),
 			new InlineCompletionStrategy(),
-			new AutoTriggerStrategy(),
-			new ErrorFixStrategy(), // This need to be implemented in background
 		]
+		this.autoTriggerStrategy = new AutoTriggerStrategy()
 	}
 
 	/**
@@ -38,22 +37,13 @@ export class PromptStrategyManager {
 	 * @returns The selected strategy
 	 */
 	selectStrategy(context: GhostSuggestionContext): PromptStrategy {
-		// Find the first strategy that can handle this context
-		for (const strategy of this.strategies) {
-			if (strategy.canHandle(context)) {
-				if (this.debug) {
-					console.log(`[PromptStrategyManager] Selected strategy: ${strategy.name}`)
-				}
-				return strategy
-			}
-		}
+		const strategy = this.strategies.find((s) => s.canHandle(context)) ?? this.autoTriggerStrategy
 
-		// Fallback: return the last strategy (AutoTriggerStrategy)
-		const fallback = this.strategies[this.strategies.length - 1]
 		if (this.debug) {
-			console.log(`[PromptStrategyManager] Falling back to: ${fallback.name}`)
+			console.log(`[PromptStrategyManager] Selected strategy: ${strategy.name}`)
 		}
-		return fallback
+
+		return strategy
 	}
 
 	/**
@@ -86,12 +76,4 @@ export class PromptStrategyManager {
 			strategy,
 		}
 	}
-
-	/**
-	 * Gets all registered strategies (for testing/debugging)
-	 * @returns Array of all strategies
-	 */
-	getStrategies(): PromptStrategy[] {
-		return [...this.strategies]
-	}
 }

src/services/ghost/__tests__/GhostStrategy.test.ts

@@ -3,8 +3,6 @@ import { describe, it, expect, beforeEach, vi } from "vitest"
 import { GhostStrategy } from "../GhostStrategy"
 import { GhostSuggestionContext } from "../types"
 import { PromptStrategyManager } from "../PromptStrategyManager"
-import { UseCaseType } from "../types/PromptStrategy"
-import { skip } from "node:test"
 
 describe("GhostStrategy", () => {
 	let strategy: GhostStrategy
@@ -53,37 +51,6 @@ describe("GhostStrategy", () => {
 			expect(systemPrompt).toContain("Execute User's Explicit Request")
 			expect(userPrompt).toContain("Add a function to calculate sum")
 		})
-
-		skip("should select ErrorFixStrategy when diagnostics are present", () => {
-			const mockDocument = {
-				languageId: "typescript",
-				getText: () => "const x = 1",
-				lineAt: (line: number) => ({ text: "const x = 1" }),
-				uri: { toString: () => "file:///test.ts" },
-				offsetAt: (position: vscode.Position) => 11,
-			} as vscode.TextDocument
-
-			const mockRange = {
-				start: { line: 0, character: 11 } as vscode.Position,
-				end: { line: 0, character: 11 } as vscode.Position,
-			} as vscode.Range
-
-			const context: GhostSuggestionContext = {
-				document: mockDocument,
-				diagnostics: [
-					{
-						severity: vscode.DiagnosticSeverity.Error,
-						message: "Missing semicolon",
-						range: mockRange,
-					} as vscode.Diagnostic,
-				],
-			}
-
-			const { systemPrompt } = strategy.getPrompts(context)
-
-			// ErrorFixStrategy should be selected
-			expect(systemPrompt).toContain("Fix Compilation Errors and Warnings")
-		})
 	})
 
 	describe("Integration", () => {
@@ -117,20 +84,6 @@ describe("GhostStrategy", () => {
 			manager = new PromptStrategyManager()
 		})
 
-		it("should have all 7 strategies registered", () => {
-			const strategies = manager.getStrategies()
-			expect(strategies).toHaveLength(7)
-
-			const strategyNames = strategies.map((s) => s.name)
-			expect(strategyNames).toContain("User Request")
-			expect(strategyNames).toContain("Error Fix")
-			expect(strategyNames).toContain("Selection Refactor")
-			expect(strategyNames).toContain("Comment Driven")
-			expect(strategyNames).toContain("New Line Completion")
-			expect(strategyNames).toContain("Inline Completion")
-			expect(strategyNames).toContain("Auto Trigger")
-		})
-
 		it("should select appropriate strategy based on context", () => {
 			const mockDocument = {
 				languageId: "typescript",