diff --git a/complexity-reduction-plan.md b/complexity-reduction-plan.md new file mode 100644 index 00000000000..55da6c7b9da --- /dev/null +++ b/complexity-reduction-plan.md @@ -0,0 +1,427 @@ +# Complexity Reduction Plan for PR #3889 + +## Goal + +Reduce implementation complexity while maintaining core request deduplication functionality. + +## Current Complexity Issues + +### 1. State Management Overhead + +**Current:** 3 separate state tracking mechanisms + +- `pendingRequests: Map` +- `pendingDebounceResolvers: Array<() => void>` +- `lastDebouncedPrompt: { prompt, prefix, suffix }` + +### 2. Nested Promise Logic + +**Current:** Complex promise-within-promise pattern with IIFE + +### 3. Multiple Abort Check Points + +**Current:** 4+ places checking `abortController.signal.aborted` + +### 4. Debounce + Deduplication Coupling + +**Current:** Tightly coupled debouncing and deduplication logic + +--- + +## Proposed Simplifications + +### Phase 1: Consolidate State Management + +#### Action 1.1: Merge Debounce State into PendingSuggestion + +```typescript +interface PendingSuggestion { + prefix: string + suffix: string + prompt: GhostPrompt // Add this + promise: Promise + abortController: AbortController + resolvers: Array<() => void> // Merge debounce resolvers here + isDebounced: boolean // Track if still in debounce phase +} +``` + +**Benefits:** + +- Single source of truth for request state +- Eliminates `pendingDebounceResolvers` and `lastDebouncedPrompt` +- Reduces state variables from 3 to 1 + +#### Action 1.2: Simplify Cache Key Strategy + +```typescript +// Instead of composite key, use simpler approach +private getCacheKey(prefix: string, suffix: string): string { + // For exact matches only + return `${prefix.length}:${suffix.length}:${prefix}${suffix}` +} + +// For reuse checking, don't use cache key +private canReuseRequest(pending: PendingSuggestion, prefix: string, suffix: string): boolean { + return pending.suffix === suffix && prefix.startsWith(pending.prefix) +} +``` + +**Benefits:** + +- Clearer separation between exact match and reuse logic +- Easier to understand and debug + +--- + +### Phase 2: Simplify Async Flow + +#### Action 2.1: Remove IIFE Pattern + +```typescript +// Current: Complex IIFE +const promise = (async (): Promise => { + // ... complex logic +})() + +// Simplified: Direct async function +private createRequest(prompt: GhostPrompt, prefix: string, suffix: string): { + promise: Promise, + abortController: AbortController +} { + const abortController = new AbortController() + const promise = this.executeRequest(prompt, prefix, suffix, abortController) + return { promise, abortController } +} + +private async executeRequest( + prompt: GhostPrompt, + prefix: string, + suffix: string, + abortController: AbortController +): Promise { + // Simplified linear flow +} +``` + +**Benefits:** + +- Clearer execution flow +- Easier to test individual components +- Better error handling + +#### Action 2.2: Centralize Abort Handling + +```typescript +private handleAbortedRequest(error: unknown): boolean { + return error instanceof Error && + (error.name === "AbortError" || error.message.includes("aborted")) +} + +// Use consistently everywhere: +if (this.handleAbortedRequest(error)) { + return // Silent ignore +} +``` + +**Benefits:** + +- Single place to modify abort behavior +- Consistent handling across codebase + +--- + +### Phase 3: Decouple Debouncing from Deduplication + +#### Action 3.1: Extract Debouncer Class + +```typescript +class RequestDebouncer { + private timer: NodeJS.Timeout | null = null + private pending: (() => void) | null = null + + debounce(fn: () => void, delay: number): Promise { + if (this.timer) { + clearTimeout(this.timer) + } + + return new Promise((resolve) => { + this.pending = resolve + this.timer = setTimeout(() => { + fn() + this.pending?.() + this.pending = null + this.timer = null + }, delay) + }) + } + + flush(): void { + if (this.timer) { + clearTimeout(this.timer) + this.timer = null + } + this.pending?.() + this.pending = null + } +} +``` + +**Benefits:** + +- Reusable debouncing logic +- Simpler to test in isolation +- Cleaner main class + +#### Action 3.2: Separate Deduplication Logic + +```typescript +class RequestDeduplicator { + private pending = new Map() + + findReusable(prefix: string, suffix: string): PendingSuggestion | null { + // Check exact match + const key = this.getKey(prefix, suffix) + if (this.pending.has(key)) { + return this.pending.get(key)! + } + + // Check type-ahead reuse + for (const request of this.pending.values()) { + if (this.canReuse(request, prefix, suffix)) { + return request + } + } + + return null + } + + add(key: string, request: PendingSuggestion): void { + this.pending.set(key, request) + } + + remove(key: string): void { + this.pending.delete(key) + } + + cancelObsolete(prefix: string, suffix: string): void { + for (const [key, request] of this.pending.entries()) { + if (!this.canReuse(request, prefix, suffix)) { + request.abortController.abort() + this.pending.delete(key) + } + } + } +} +``` + +**Benefits:** + +- Single responsibility principle +- Easier to unit test +- Can be reused elsewhere + +--- + +### Phase 4: Simplify Main Flow + +#### Action 4.1: Streamline fetchAndCacheSuggestion + +```typescript +private async fetchAndCacheSuggestion( + prompt: GhostPrompt, + prefix: string, + suffix: string +): Promise { + // 1. Check for reusable request + const reusable = this.deduplicator.findReusable(prefix, suffix) + if (reusable) { + return this.reuseRequest(reusable, prefix, suffix) + } + + // 2. Cancel obsolete requests + this.deduplicator.cancelObsolete(prefix, suffix) + + // 3. Create and track new request + const request = this.createNewRequest(prompt, prefix, suffix) + this.deduplicator.add(this.getCacheKey(prefix, suffix), request) + + // 4. Wait for completion + try { + await request.promise + } catch (error) { + if (!this.handleAbortedRequest(error)) { + console.error("Error getting completion:", error) + } + } +} +``` + +**Benefits:** + +- Linear, easy-to-follow flow +- Clear separation of concerns +- Each step has single responsibility + +#### Action 4.2: Simplify Suggestion Adjustment + +```typescript +private adjustSuggestion( + suggestion: string, + originalPrefix: string, + currentPrefix: string +): string | null { + if (!currentPrefix.startsWith(originalPrefix)) { + return null // Can't adjust + } + + const typedAhead = currentPrefix.slice(originalPrefix.length) + if (!suggestion.startsWith(typedAhead)) { + return null // Suggestion doesn't match + } + + return suggestion.slice(typedAhead.length) +} +``` + +**Benefits:** + +- Pure function, easy to test +- Clear return semantics +- No side effects + +--- + +## Implementation Order + +### Step 1: Extract Helper Classes (Low Risk) + +1. Create `RequestDebouncer` class +2. Create `RequestDeduplicator` class +3. Add unit tests for both + +### Step 2: Refactor State Management (Medium Risk) + +1. Consolidate state into single `PendingSuggestion` interface +2. Update all references +3. Verify tests still pass + +### Step 3: Simplify Async Flow (Medium Risk) + +1. Remove IIFE pattern +2. Extract `executeRequest` method +3. Centralize abort handling + +### Step 4: Refactor Main Logic (Low Risk) + +1. Simplify `fetchAndCacheSuggestion` +2. Extract `adjustSuggestion` as pure function +3. Update tests + +### Step 5: Cleanup (Low Risk) + +1. Remove unused variables +2. Add clear comments +3. Update documentation + +--- + +## Expected Outcomes + +### Metrics + +| Metric | Current | Target | Reduction | +| --------------------- | ------- | ------ | ------------ | +| Lines in main method | ~200 | ~100 | 50% | +| State variables | 3 | 1 | 67% | +| Cyclomatic complexity | High | Medium | ~40% | +| Number of classes | 1 | 3 | More modular | + +### Benefits + +1. **Maintainability:** Easier to understand and modify +2. **Testability:** Each component can be tested in isolation +3. **Reusability:** Helper classes can be used elsewhere +4. **Debuggability:** Clearer execution flow +5. **Performance:** Same or better (less state tracking) + +### Risks + +1. **Regression:** Mitigated by comprehensive test suite +2. **Time:** ~4-6 hours of refactoring +3. **Review:** Additional review cycle needed + +--- + +## Alternative: Minimal Simplification + +If full refactoring is too risky, consider these quick wins: + +### Quick Win 1: Remove Debounce Complexity + +Remove the "divergence detection" in debouncing - just use simple debounce: + +```typescript +private debouncedFetchAndCacheSuggestion( + prompt: GhostPrompt, + prefix: string, + suffix: string +): Promise { + if (this.debounceTimer) { + clearTimeout(this.debounceTimer) + } + + return new Promise(resolve => { + this.debounceTimer = setTimeout(async () => { + await this.fetchAndCacheSuggestion(prompt, prefix, suffix) + resolve() + }, DEBOUNCE_DELAY_MS) + }) +} +``` + +**Saves:** ~50 lines + +### Quick Win 2: Simplify Reuse Check + +```typescript +private findReusablePendingRequest(prefix: string, suffix: string): PendingSuggestion | null { + // Only check exact match for now + return this.pendingRequests.get(this.getCacheKey(prefix, suffix)) || null +} +``` + +**Saves:** ~15 lines + +### Quick Win 3: Remove Redundant Abort Checks + +Keep only essential abort checks: + +- Before starting request +- In error handler + **Saves:** ~10 lines + +**Total Quick Wins:** ~75 lines reduction (37% reduction) + +--- + +## Recommendation + +**Preferred:** Full refactoring (Phase 1-5) + +- Best long-term maintainability +- Cleaner architecture +- Better testability + +**If time-constrained:** Minimal simplification + +- Quick wins only +- Maintains current functionality +- Can refactor later + +**Next Steps:** + +1. Review this plan with team +2. Decide on approach +3. Create subtasks for implementation +4. Update PR with chosen simplifications diff --git a/pr-3889-review.md b/pr-3889-review.md new file mode 100644 index 00000000000..a929ee6a9b8 --- /dev/null +++ b/pr-3889-review.md @@ -0,0 +1,403 @@ +# PR #3889 Review: Request Deduplication Implementation + +**PR Title:** feat: Add request deduplication to GhostInlineCompletionProvider +**Context:** Reimplementation of `GeneratorReuseManager.ts` functionality +**Date:** 2025-11-21 + +## Executive Summary + +This PR implements request deduplication and reuse logic in `GhostInlineCompletionProvider`, inspired by Continue.dev's `GeneratorReuseManager`. The implementation successfully achieves the core goals but introduces additional complexity and some architectural differences. + +**Overall Assessment:** ✅ **Approve with minor suggestions** + +--- + +## Comparison with Original GeneratorReuseManager + +### Architecture Differences + +| Aspect | GeneratorReuseManager (Original) | GhostInlineCompletionProvider (PR) | +| -------------------- | --------------------------------- | ------------------------------------- | +| **Streaming Model** | Generator-based (AsyncGenerator) | Promise-based with chunks | +| **State Management** | Single current generator + prefix | Map of pending requests by cache key | +| **Cancellation** | AbortController per generator | AbortController per request | +| **Reuse Logic** | Inline in getGenerator() | Separate findReusablePendingRequest() | +| **Adjustment** | Character-by-character matching | Substring removal | + +### Core Features Comparison + +#### ✅ Features Present in Both + +1. **Request Reuse When Typing Ahead** + + - Original: Checks if `(pendingPrefix + pendingCompletion).startsWith(prefix)` + - PR: Checks if `prefix.startsWith(pending.prefix)` with same suffix + - Both handle the case where user types faster than completion arrives + +2. **Cancellation of Obsolete Requests** + + - Original: Cancels previous generator when creating new one + - PR: Cancels requests with diverged prefix/suffix + - Both prevent wasted API calls + +3. **Suggestion Adjustment** + - Original: Character-by-character removal of typed text from chunks + - PR: Substring removal of typed-ahead portion + - Both ensure suggestions don't include already-typed text + +#### ⚠️ Differences in Implementation + +1. **Multiline Handling** + + - Original: Has explicit `multiline` parameter and breaks at newlines + - PR: ❌ **Missing** - No multiline mode handling + - **Impact:** May show multi-line suggestions when single-line expected + +2. **Streaming vs Batch** + + - Original: Yields chunks as they arrive (true streaming) + - PR: Waits for complete response, then adjusts + - **Impact:** PR has higher latency but simpler logic + +3. **Debouncing Integration** + + - Original: No debouncing (handled elsewhere) + - PR: ✅ **Enhanced** - Sophisticated debounce with divergence detection + - **Impact:** Better UX with intelligent request flushing + +4. **Cache Key Strategy** + - Original: Uses prefix only for reuse check + - PR: Uses `prefix|||suffix` composite key + - **Impact:** PR is more precise but may miss some reuse opportunities + +#### ➕ New Features in PR + +1. **Exact Match Deduplication** + + - Prevents duplicate API calls for identical prefix/suffix + - Not present in original (which only handles typing ahead) + +2. **Debounce Divergence Detection** + + - Flushes pending debounced request when prefix diverges + - Prevents stale requests from executing + - Sophisticated improvement over original + +3. **Multiple Pending Requests** + + - Can track multiple in-flight requests simultaneously + - Original only tracks one current generator + - Better for concurrent scenarios + +4. **Comprehensive Test Suite** + - 252 lines of tests covering all scenarios + - Original has no dedicated tests in this PR context + +--- + +## Code Quality Analysis + +### ✅ Strengths + +1. **Well-Structured Code** + + - Clear separation of concerns (getCacheKey, findReusablePendingRequest) + - Good use of TypeScript interfaces (PendingSuggestion) + - Comprehensive error handling + +2. **Excellent Test Coverage** + + - Tests for deduplication, typing ahead, divergence, adjustment, cleanup + - Uses proper mocking and fake timers + - Clear test descriptions + +3. **Proper Resource Management** + + - Cleanup in finally blocks + - Disposal of pending requests + - AbortController usage + +4. **Documentation** + - JSDoc comments for key methods + - Clear variable names + - Inline comments explaining logic + +### ⚠️ Complexity Concerns + +1. **Increased Cognitive Load** + + - Original: ~70 lines, single responsibility + - PR: ~200 lines added, multiple responsibilities + - **Concern:** Harder to maintain and debug + +2. **State Management Complexity** + + - Three related state variables: `pendingRequests`, `pendingDebounceResolvers`, `lastDebouncedPrompt` + - Complex interactions between debouncing and request reuse + - **Risk:** Potential for race conditions or state inconsistencies + +3. **Nested Async Logic** + - Promise within promise (IIFE pattern) + - Multiple await points with abort checks + - **Risk:** Harder to reason about execution flow + +### 🔍 Potential Issues + +1. **Missing Multiline Support** + + ```typescript + // Original has: + if (newLineIndex >= 0 && !multiline) { + yield chunk.slice(0, newLineIndex) + break + } + // PR: No equivalent logic + ``` + + **Recommendation:** Add multiline parameter or document why it's not needed + +2. **Character-by-Character vs Substring Matching** + + ```typescript + // Original: More robust character matching + while (chunk.length && typedSinceLastGenerator.length) { + if (chunk[0] === typedSinceLastGenerator[0]) { + typedSinceLastGenerator = typedSinceLastGenerator.slice(1) + chunk = chunk.slice(1) + } else { + break + } + } + + // PR: Simple substring check + if (result.suggestion.text.startsWith(typedAhead)) { + // Remove typed portion + } + ``` + + **Concern:** PR's approach may fail if suggestion doesn't start with typed text + **Recommendation:** Add fallback behavior or more robust matching + +3. **Backspace Handling** + + ```typescript + // Original explicitly checks: + this.pendingGeneratorPrefix?.length <= prefix?.length + + // PR: Checks divergence but may not handle backspace optimally + ``` + + **Recommendation:** Add explicit backspace test case + +4. **Error Handling for Aborted Requests** + ```typescript + // Multiple places check abortController.signal.aborted + // But error handling varies + ``` + **Recommendation:** Consolidate abort handling logic + +--- + +## Feature Completeness + +### ✅ Core Features Implemented + +- [x] Request deduplication for identical contexts +- [x] Request reuse when typing ahead +- [x] Cancellation of obsolete requests +- [x] Suggestion adjustment for typed-ahead text +- [x] Proper cleanup on disposal + +### ⚠️ Missing Features from Original + +- [ ] Multiline mode support +- [ ] Character-by-character matching robustness +- [ ] Explicit backspace handling + +### ➕ Additional Features + +- [x] Debounce divergence detection +- [x] Multiple concurrent request tracking +- [x] Comprehensive test suite +- [x] Exact match deduplication + +--- + +## Performance Considerations + +### Improvements + +1. **Reduced API Calls:** Deduplication prevents duplicate requests +2. **Lower Latency:** Reusing pending requests when typing ahead +3. **Better Resource Usage:** Cancellation of obsolete requests + +### Potential Concerns + +1. **Memory Overhead:** Map of pending requests vs single generator +2. **Lookup Cost:** Iterating through pendingRequests map for reuse check +3. **No Streaming:** Waits for complete response vs yielding chunks + +**Recommendation:** Monitor memory usage with many concurrent requests + +--- + +## Test Coverage Analysis + +### Excellent Coverage ✅ + +- Deduplication of identical requests +- Reuse when typing ahead +- Cancellation on divergence +- Suggestion adjustment +- Cleanup on dispose + +### Missing Test Cases ⚠️ + +1. **Backspace scenarios** + + ```typescript + // Test: User types "func" then backspaces to "fun" + ``` + +2. **Suffix changes** + + ```typescript + // Test: Same prefix but different suffix + ``` + +3. **Multiple concurrent divergent requests** + + ```typescript + // Test: 3+ requests with different prefixes + ``` + +4. **Abort during adjustment** + + ```typescript + // Test: Request aborted while adjusting suggestion + ``` + +5. **Edge cases** + - Empty prefix/suffix + - Very long prefix/suffix + - Special characters in cache key + +--- + +## Recommendations + +### High Priority 🔴 + +1. **Add Multiline Support** + + ```typescript + private async fetchAndCacheSuggestion( + prompt: GhostPrompt, + prefix: string, + suffix: string, + multiline: boolean = false // Add parameter + ): Promise + ``` + +2. **Improve Suggestion Adjustment Robustness** + + ```typescript + // Add fallback when suggestion doesn't start with typed text + if (result.suggestion.text.startsWith(typedAhead)) { + // Current logic + } else { + // Fallback: Use original suggestion or cancel + console.warn("Suggestion doesn't match typed text, using original") + this.updateSuggestions(result.suggestion) + } + ``` + +3. **Add Backspace Test Case** + +### Medium Priority 🟡 + +1. **Consolidate Abort Handling** + + ```typescript + private isAborted(controller: AbortController): boolean { + return controller.signal.aborted + } + ``` + +2. **Add Performance Monitoring** + + ```typescript + // Track metrics: reuse rate, cancellation rate, adjustment rate + ``` + +3. **Document State Machine** + - Create diagram showing state transitions + - Document invariants + +### Low Priority 🟢 + +1. **Consider Streaming Support** + + - Evaluate if streaming would improve UX + - May require architectural changes + +2. **Optimize Cache Key** + + - Consider hash-based keys for very long prefix/suffix + - Profile lookup performance + +3. **Add Telemetry** + - Track how often requests are reused + - Measure latency improvements + +--- + +## Conclusion + +### Summary + +The PR successfully implements request deduplication inspired by `GeneratorReuseManager`, with several enhancements: + +- ✅ Core reuse logic works correctly +- ✅ Excellent test coverage +- ✅ Enhanced debouncing with divergence detection +- ⚠️ Missing multiline support +- ⚠️ Increased complexity vs original + +### Verdict: **APPROVE** ✅ + +**Rationale:** + +1. Core functionality is solid and well-tested +2. Improvements over original (debounce handling, exact match deduplication) +3. Missing features (multiline) can be added incrementally +4. Benefits (reduced API calls, better UX) outweigh complexity cost + +### Merge Conditions + +- [ ] Add multiline parameter or document why not needed +- [ ] Add backspace test case +- [ ] Improve suggestion adjustment with fallback +- [ ] Add inline comments explaining state management + +### Post-Merge Actions + +1. Monitor performance metrics in production +2. Gather user feedback on completion latency +3. Consider streaming support in future iteration +4. Add telemetry for reuse/cancellation rates + +--- + +## Complexity Metrics + +| Metric | Original | PR | Change | +| --------------------- | -------- | ------ | ------ | +| Lines of Code | 70 | ~270 | +286% | +| Cyclomatic Complexity | Low | Medium | ⬆️ | +| State Variables | 3 | 6 | +100% | +| Public Methods | 1 | 1 | = | +| Private Methods | 2 | 5 | +150% | +| Test Lines | 0 | 252 | ➕ | + +**Assessment:** Complexity increase is justified by functionality gains and test coverage. diff --git a/src/services/ghost/classic-auto-complete/GhostInlineCompletionProvider.ts b/src/services/ghost/classic-auto-complete/GhostInlineCompletionProvider.ts index aa992384a34..c521d9fc9eb 100644 --- a/src/services/ghost/classic-auto-complete/GhostInlineCompletionProvider.ts +++ b/src/services/ghost/classic-auto-complete/GhostInlineCompletionProvider.ts @@ -9,6 +9,8 @@ import { RecentlyEditedTracker } from "../../continuedev/core/vscode-test-harnes import type { GhostServiceSettings } from "@roo-code/types" import { postprocessGhostSuggestion } from "./uselessSuggestionFilter" import { RooIgnoreController } from "../../../core/ignore/RooIgnoreController" +import { RequestDebouncer } from "./RequestDebouncer" +import { RequestDeduplicator, type PendingRequest } from "./RequestDeduplicator" const MAX_SUGGESTIONS_HISTORY = 20 const DEBOUNCE_DELAY_MS = 300 @@ -95,8 +97,9 @@ export class GhostInlineCompletionProvider implements vscode.InlineCompletionIte private getSettings: () => GhostServiceSettings | null private recentlyVisitedRangesService: RecentlyVisitedRangesService private recentlyEditedTracker: RecentlyEditedTracker - private debounceTimer: NodeJS.Timeout | null = null private ignoreController?: Promise + private debouncer: RequestDebouncer = new RequestDebouncer() + private deduplicator: RequestDeduplicator = new RequestDeduplicator() constructor( model: GhostModel, @@ -192,10 +195,8 @@ export class GhostInlineCompletionProvider implements vscode.InlineCompletionIte } public dispose(): void { - if (this.debounceTimer !== null) { - clearTimeout(this.debounceTimer) - this.debounceTimer = null - } + this.debouncer.clear() + this.deduplicator.clear() this.recentlyVisitedRangesService.dispose() this.recentlyEditedTracker.dispose() } @@ -280,22 +281,106 @@ export class GhostInlineCompletionProvider implements vscode.InlineCompletionIte } private debouncedFetchAndCacheSuggestion(prompt: GhostPrompt, prefix: string, suffix: string): Promise { - if (this.debounceTimer !== null) { - clearTimeout(this.debounceTimer) + return this.debouncer.debounce(() => this.fetchAndCacheSuggestion(prompt, prefix, suffix), DEBOUNCE_DELAY_MS) + } + + /** + * Adjust a suggestion when user has typed ahead + */ + private adjustSuggestion(suggestion: string, originalPrefix: string, currentPrefix: string): string | null { + if (!currentPrefix.startsWith(originalPrefix)) { + return null // Can't adjust } - return new Promise((resolve) => { - this.debounceTimer = setTimeout(async () => { - this.debounceTimer = null - await this.fetchAndCacheSuggestion(prompt, prefix, suffix) - resolve() - }, DEBOUNCE_DELAY_MS) - }) + const typedAhead = currentPrefix.slice(originalPrefix.length) + if (!suggestion.startsWith(typedAhead)) { + return null // Suggestion doesn't match + } + + return suggestion.slice(typedAhead.length) + } + + /** + * Handle errors from aborted requests + */ + private isAbortError(error: unknown): boolean { + return error instanceof Error && (error.name === "AbortError" || error.message.includes("aborted")) } private async fetchAndCacheSuggestion(prompt: GhostPrompt, prefix: string, suffix: string): Promise { + // Check if we can reuse an existing pending request + const reusable = this.deduplicator.findReusable(prefix, suffix) + if (reusable) { + try { + const result = await reusable.promise + + // Check if request was aborted while waiting + if (reusable.abortController.signal.aborted) { + return + } + + // If user typed ahead, adjust the suggestion + if (prefix !== reusable.prefix) { + const adjusted = this.adjustSuggestion(result.suggestion.text, reusable.prefix, prefix) + if (adjusted !== null) { + this.updateSuggestions({ text: adjusted, prefix, suffix }) + return + } + } + + // Use the result as-is if no adjustment needed + this.updateSuggestions(result.suggestion) + return + } catch (error) { + if (this.isAbortError(error)) { + return + } + console.warn("Reused request failed, creating new request:", error) + } + } + + // Cancel any pending requests that are now obsolete + this.deduplicator.cancelObsolete(prefix, suffix) + + // Create new request + const abortController = new AbortController() + const promise = this.executeRequest(prompt, prefix, suffix, abortController) + + // Store the pending request + const request: PendingRequest = { + prefix, + suffix, + promise, + abortController, + } + this.deduplicator.add(prefix, suffix, request) + try { - // Curry processSuggestion with prefix, suffix, and model - only text needs to be provided + await promise + } catch (error) { + if (this.isAbortError(error)) { + return + } + console.error("Error getting inline completion from LLM:", error) + } + } + + /** + * Execute the actual LLM request + */ + private async executeRequest( + prompt: GhostPrompt, + prefix: string, + suffix: string, + abortController: AbortController, + ): Promise { + try { + // Check if already aborted before starting + if (abortController.signal.aborted) { + throw new Error("Request aborted before starting") + } + + // Curry processSuggestion with prefix, suffix, and model const curriedProcessSuggestion = (text: string) => this.processSuggestion(text, prefix, suffix, this.model) const result = @@ -303,6 +388,11 @@ export class GhostInlineCompletionProvider implements vscode.InlineCompletionIte ? await this.fimPromptBuilder.getFromFIM(this.model, prompt, curriedProcessSuggestion) : await this.holeFiller.getFromChat(this.model, prompt, curriedProcessSuggestion) + // Check if aborted after completion + if (abortController.signal.aborted) { + throw new Error("Request aborted after completion") + } + if (this.costTrackingCallback && result.cost > 0) { this.costTrackingCallback( result.cost, @@ -315,8 +405,11 @@ export class GhostInlineCompletionProvider implements vscode.InlineCompletionIte // Always update suggestions, even if text is empty (for caching) this.updateSuggestions(result.suggestion) - } catch (error) { - console.error("Error getting inline completion from LLM:", error) + + return result + } finally { + // Clean up from pending requests map + this.deduplicator.remove(prefix, suffix) } } } diff --git a/src/services/ghost/classic-auto-complete/RequestDebouncer.ts b/src/services/ghost/classic-auto-complete/RequestDebouncer.ts new file mode 100644 index 00000000000..af428cd192a --- /dev/null +++ b/src/services/ghost/classic-auto-complete/RequestDebouncer.ts @@ -0,0 +1,101 @@ +/** + * Handles debouncing of autocomplete requests with intelligent flushing + */ +export class RequestDebouncer { + private timer: NodeJS.Timeout | null = null + private pendingResolvers: Array<() => void> = [] + private lastRequest: { execute: () => Promise } | null = null + + /** + * Debounce a request execution + * @param execute - Function to execute after debounce delay + * @param delay - Delay in milliseconds + * @param shouldFlush - Optional function to determine if pending request should flush immediately + * @returns Promise that resolves when request completes + */ + debounce( + execute: () => Promise, + delay: number, + shouldFlush?: (lastRequest: { execute: () => Promise } | null) => boolean, + ): Promise { + // Check if we should flush the pending request immediately + if (this.timer && shouldFlush?.(this.lastRequest)) { + this.flush() + } else if (this.timer) { + // Just clear the timer to restart debounce + clearTimeout(this.timer) + } + + // Store the current request + this.lastRequest = { execute } + + return new Promise((resolve) => { + // Add this resolver to the list + this.pendingResolvers.push(resolve) + + this.timer = setTimeout(async () => { + this.timer = null + // Execute the last request that was set + if (this.lastRequest) { + try { + await this.lastRequest.execute() + } catch (error) { + // Silently catch errors - they should be handled by the execute function + console.error("Error in debounced request:", error) + } + this.lastRequest = null + } + + // Resolve all pending promises + const resolvers = this.pendingResolvers.splice(0) + resolvers.forEach((r) => r()) + }, delay) + }) + } + + /** + * Flush any pending debounced request immediately + */ + flush(): void { + if (this.timer) { + clearTimeout(this.timer) + this.timer = null + } + + // Execute the pending request if it exists + if (this.lastRequest) { + const request = this.lastRequest + this.lastRequest = null + const resolvers = this.pendingResolvers.splice(0) + + request + .execute() + .catch((error) => { + // Silently catch errors + console.error("Error in flushed request:", error) + }) + .then(() => { + resolvers.forEach((r) => r()) + }) + } + } + + /** + * Clear all pending requests without executing them + */ + clear(): void { + if (this.timer) { + clearTimeout(this.timer) + this.timer = null + } + this.pendingResolvers = [] + this.lastRequest = null + } + + /** + * Check if there's a pending debounced request + */ + hasPending(): boolean { + return this.timer !== null + } +} diff --git a/src/services/ghost/classic-auto-complete/RequestDeduplicator.ts b/src/services/ghost/classic-auto-complete/RequestDeduplicator.ts new file mode 100644 index 00000000000..29a8688d7ed --- /dev/null +++ b/src/services/ghost/classic-auto-complete/RequestDeduplicator.ts @@ -0,0 +1,106 @@ +import type { LLMRetrievalResult } from "./GhostInlineCompletionProvider" + +export interface PendingRequest { + prefix: string + suffix: string + promise: Promise + abortController: AbortController +} + +/** + * Manages deduplication and reuse of pending autocomplete requests + */ +export class RequestDeduplicator { + private pendingRequests = new Map() + + /** + * Create a cache key for exact match lookups + */ + private getCacheKey(prefix: string, suffix: string): string { + return `${prefix}|||${suffix}` + } + + /** + * Check if a request can be reused for the given prefix/suffix + */ + private canReuse(request: PendingRequest, prefix: string, suffix: string): boolean { + // Must have same suffix + if (request.suffix !== suffix) { + return false + } + + // Current prefix must start with the request's prefix (user typed ahead) + return prefix.startsWith(request.prefix) + } + + /** + * Find a reusable pending request for the given prefix/suffix + * @returns The reusable request, or null if none found + */ + findReusable(prefix: string, suffix: string): PendingRequest | null { + // Check for exact match first + const cacheKey = this.getCacheKey(prefix, suffix) + const exactMatch = this.pendingRequests.get(cacheKey) + if (exactMatch) { + return exactMatch + } + + // Check if we can reuse a request with a shorter prefix (user typed ahead) + for (const request of this.pendingRequests.values()) { + if (this.canReuse(request, prefix, suffix)) { + return request + } + } + + return null + } + + /** + * Add a new pending request + */ + add(prefix: string, suffix: string, request: PendingRequest): void { + const cacheKey = this.getCacheKey(prefix, suffix) + this.pendingRequests.set(cacheKey, request) + } + + /** + * Remove a pending request + */ + remove(prefix: string, suffix: string): void { + const cacheKey = this.getCacheKey(prefix, suffix) + this.pendingRequests.delete(cacheKey) + } + + /** + * Cancel and remove all pending requests that cannot be reused for the given prefix/suffix + */ + cancelObsolete(prefix: string, suffix: string): void { + for (const [key, request] of this.pendingRequests.entries()) { + // Cancel if different suffix or if prefix has diverged + if ( + request.suffix !== suffix || + (!prefix.startsWith(request.prefix) && !request.prefix.startsWith(prefix)) + ) { + request.abortController.abort() + this.pendingRequests.delete(key) + } + } + } + + /** + * Cancel and clear all pending requests + */ + clear(): void { + for (const request of this.pendingRequests.values()) { + request.abortController.abort() + } + this.pendingRequests.clear() + } + + /** + * Get the number of pending requests + */ + size(): number { + return this.pendingRequests.size + } +} diff --git a/src/services/ghost/classic-auto-complete/__tests__/GhostInlineCompletionProvider.request-deduplication.test.ts b/src/services/ghost/classic-auto-complete/__tests__/GhostInlineCompletionProvider.request-deduplication.test.ts new file mode 100644 index 00000000000..677f22db27e --- /dev/null +++ b/src/services/ghost/classic-auto-complete/__tests__/GhostInlineCompletionProvider.request-deduplication.test.ts @@ -0,0 +1,252 @@ +import { describe, it, expect, beforeEach, vi, afterEach } from "vitest" +import { GhostInlineCompletionProvider } from "../GhostInlineCompletionProvider" +import { GhostModel } from "../../GhostModel" +import * as vscode from "vscode" +import { MockTextDocument } from "../../../mocking/MockTextDocument" + +// Mock vscode event listeners +vi.mock("vscode", async () => { + const actual = await vi.importActual("vscode") + return { + ...actual, + InlineCompletionTriggerKind: { + Invoke: 0, + Automatic: 1, + }, + window: { + ...actual.window, + onDidChangeTextEditorSelection: vi.fn(() => ({ dispose: vi.fn() })), + }, + workspace: { + ...actual.workspace, + onDidChangeTextDocument: vi.fn(() => ({ dispose: vi.fn() })), + }, + } +}) + +describe("GhostInlineCompletionProvider - Request Deduplication", () => { + let provider: GhostInlineCompletionProvider + let mockModel: GhostModel + let mockContextProvider: any + let costTrackingCallback: ReturnType + + // Helper to call provideInlineCompletionItems and advance timers + async function provideWithDebounce(doc: vscode.TextDocument, pos: vscode.Position) { + const promise = provider.provideInlineCompletionItems_Internal(doc, pos, {} as any, {} as any) + // Wait a tick to let the promise chain set up + await Promise.resolve() + // Advance timers past debounce delay + await vi.advanceTimersByTimeAsync(300) + // Wait for the completion to finish + return await promise + } + + beforeEach(() => { + vi.useFakeTimers() + + // Create mock IDE for tracking services + const mockIde = { + getWorkspaceDirs: vi.fn().mockResolvedValue([]), + getOpenFiles: vi.fn().mockResolvedValue([]), + readFile: vi.fn().mockResolvedValue(""), + } + + mockContextProvider = { + getIde: vi.fn().mockReturnValue(mockIde), + getProcessedSnippets: vi.fn().mockResolvedValue({ + filepathUri: "file:///test.ts", + helper: { + lang: { name: "typescript", singleLineComment: "//" }, + prunedPrefix: "", + prunedSuffix: "", + }, + snippetsWithUris: [], + workspaceDirs: [], + }), + } + + mockModel = { + supportsFim: vi.fn().mockReturnValue(false), + generateResponse: vi.fn().mockResolvedValue({ + cost: 0, + inputTokens: 0, + outputTokens: 0, + cacheWriteTokens: 0, + cacheReadTokens: 0, + }), + getModelName: vi.fn().mockReturnValue("test-model"), + } as unknown as GhostModel + + costTrackingCallback = vi.fn() + + provider = new GhostInlineCompletionProvider( + mockModel, + costTrackingCallback, + () => ({ enableAutoTrigger: true }), + mockContextProvider, + ) + }) + + afterEach(() => { + vi.useRealTimers() + provider.dispose() + }) + + it("should deduplicate identical requests", async () => { + const mockResponse = { + cost: 0.001, + inputTokens: 10, + outputTokens: 20, + cacheWriteTokens: 0, + cacheReadTokens: 0, + } + + let callCount = 0 + vi.mocked(mockModel.generateResponse).mockImplementation(async (_sys, _user, onChunk) => { + callCount++ + onChunk({ type: "text", text: "test suggestion" }) + return mockResponse + }) + + const document = new MockTextDocument(vscode.Uri.file("/test/file.ts"), "const x = \nconst y = 2") + const position = new vscode.Position(0, 10) + + // Make two identical requests - the second should reuse the first's pending request + const promise1 = provider.provideInlineCompletionItems_Internal(document, position, {} as any, {} as any) + + // Wait a tick to let the first request's debounce timer start + await Promise.resolve() + + const promise2 = provider.provideInlineCompletionItems_Internal(document, position, {} as any, {} as any) + + // Advance timers to trigger the debounce + await vi.advanceTimersByTimeAsync(300) + + // Wait for both promises to complete + await Promise.all([promise1, promise2]) + + // Should only call the API once due to deduplication + expect(callCount).toBe(1) + }) + + it("should reuse pending request when user types ahead", async () => { + const mockResponse = { + cost: 0.001, + inputTokens: 10, + outputTokens: 20, + cacheWriteTokens: 0, + cacheReadTokens: 0, + } + + let callCount = 0 + vi.mocked(mockModel.generateResponse).mockImplementation(async (_sys, _user, onChunk) => { + callCount++ + onChunk({ type: "text", text: "function test() {}" }) + return mockResponse + }) + + const document = new MockTextDocument(vscode.Uri.file("/test/file.ts"), "const x = f\nconst y = 2") + const position1 = new vscode.Position(0, 11) + + // Start first request + const promise1 = provider.provideInlineCompletionItems_Internal(document, position1, {} as any, {} as any) + + // Wait a tick + await Promise.resolve() + + // User types ahead - new document with one more character + const document2 = new MockTextDocument(vscode.Uri.file("/test/file.ts"), "const x = fu\nconst y = 2") + const position2 = new vscode.Position(0, 12) + const promise2 = provider.provideInlineCompletionItems_Internal(document2, position2, {} as any, {} as any) + + // Advance timers + await vi.advanceTimersByTimeAsync(300) + + await Promise.all([promise1, promise2]) + + // Should reuse the first request + expect(callCount).toBe(1) + }) + + it("should cancel obsolete requests when prefix diverges", async () => { + const mockResponse = { + cost: 0.001, + inputTokens: 10, + outputTokens: 20, + cacheWriteTokens: 0, + cacheReadTokens: 0, + } + + let callCount = 0 + + vi.mocked(mockModel.generateResponse).mockImplementation(async (_sys, _user, onChunk) => { + callCount++ + onChunk({ type: "text", text: "test suggestion" }) + return mockResponse + }) + + const document1 = new MockTextDocument(vscode.Uri.file("/test/file.ts"), "const x = f\nconst y = 2") + const position = new vscode.Position(0, 11) + + // Start first request + const promise1 = provider.provideInlineCompletionItems_Internal(document1, position, {} as any, {} as any) + + // Wait a tick + await Promise.resolve() + + // User changes to different prefix - this should cancel the first request + const document2 = new MockTextDocument(vscode.Uri.file("/test/file.ts"), "const x = g\nconst y = 2") + const promise2 = provider.provideInlineCompletionItems_Internal(document2, position, {} as any, {} as any) + + // Advance timers + await vi.advanceTimersByTimeAsync(300) + + await Promise.all([promise1, promise2]) + + // Should make two separate calls since prefixes diverged + expect(callCount).toBe(2) + }) + + it("should adjust suggestion when user types ahead", async () => { + const mockResponse = { + cost: 0.001, + inputTokens: 10, + outputTokens: 20, + cacheWriteTokens: 0, + cacheReadTokens: 0, + } + + vi.mocked(mockModel.generateResponse).mockImplementation(async (_sys, _user, onChunk) => { + onChunk({ type: "text", text: "unction test() {}" }) + return mockResponse + }) + + const document1 = new MockTextDocument(vscode.Uri.file("/test/file.ts"), "const x = f\nconst y = 2") + const position1 = new vscode.Position(0, 11) + + // Start first request + provideWithDebounce(document1, position1) + + // User types "un" while waiting + const document2 = new MockTextDocument(vscode.Uri.file("/test/file.ts"), "const x = fun\nconst y = 2") + const position2 = new vscode.Position(0, 13) + + const result = await provideWithDebounce(document2, position2) + + // Should adjust the suggestion by removing "un" that was already typed + if (Array.isArray(result) && result.length > 0) { + expect(result[0].insertText).toBe("ction test() {}") + } + }) + + it("should clean up pending requests on dispose", () => { + const document = new MockTextDocument(vscode.Uri.file("/test/file.ts"), "const x = \nconst y = 2") + const position = new vscode.Position(0, 10) + + // Start a request (don't await) + provideWithDebounce(document, position) + + // Dispose should cancel all pending requests + expect(() => provider.dispose()).not.toThrow() + }) +}) diff --git a/src/services/ghost/classic-auto-complete/__tests__/RequestDebouncer.test.ts b/src/services/ghost/classic-auto-complete/__tests__/RequestDebouncer.test.ts new file mode 100644 index 00000000000..b83ed39817c --- /dev/null +++ b/src/services/ghost/classic-auto-complete/__tests__/RequestDebouncer.test.ts @@ -0,0 +1,165 @@ +import { describe, it, expect, beforeEach, vi, afterEach } from "vitest" +import { RequestDebouncer } from "../RequestDebouncer" + +describe("RequestDebouncer", () => { + let debouncer: RequestDebouncer + + beforeEach(() => { + vi.useFakeTimers() + debouncer = new RequestDebouncer() + }) + + afterEach(() => { + vi.useRealTimers() + debouncer.clear() + }) + + it("should debounce a single request", async () => { + const mockExecute = vi.fn().mockResolvedValue(undefined) + + const promise = debouncer.debounce(mockExecute, 300) + + // Should not execute immediately + expect(mockExecute).not.toHaveBeenCalled() + + // Advance timers + await vi.advanceTimersByTimeAsync(300) + + // Should execute after delay + await promise + expect(mockExecute).toHaveBeenCalledTimes(1) + }) + + it("should restart debounce timer on subsequent calls", async () => { + const mockExecute1 = vi.fn().mockResolvedValue(undefined) + const mockExecute2 = vi.fn().mockResolvedValue(undefined) + + const promise1 = debouncer.debounce(mockExecute1, 300) + + // Advance partway + await vi.advanceTimersByTimeAsync(200) + + // Make another call - should restart timer + const promise2 = debouncer.debounce(mockExecute2, 300) + + // Advance remaining time from first call + await vi.advanceTimersByTimeAsync(100) + + // First should not have executed yet + expect(mockExecute1).not.toHaveBeenCalled() + + // Advance full delay from second call + await vi.advanceTimersByTimeAsync(200) + + // Only second should execute + await Promise.all([promise1, promise2]) + expect(mockExecute1).not.toHaveBeenCalled() + expect(mockExecute2).toHaveBeenCalledTimes(1) + }) + + it("should flush pending request immediately", async () => { + const mockExecute = vi.fn().mockResolvedValue(undefined) + + debouncer.debounce(mockExecute, 300) + + // Flush immediately + debouncer.flush() + + // Should execute without waiting + await vi.runAllTimersAsync() + expect(mockExecute).toHaveBeenCalledTimes(1) + }) + + it("should flush on shouldFlush condition", async () => { + const mockExecute1 = vi.fn().mockResolvedValue(undefined) + const mockExecute2 = vi.fn().mockResolvedValue(undefined) + + debouncer.debounce(mockExecute1, 300) + + // Advance partway + await vi.advanceTimersByTimeAsync(100) + + // Make another call with shouldFlush returning true + const shouldFlush = vi.fn().mockReturnValue(true) + debouncer.debounce(mockExecute2, 300, shouldFlush) + + // Should have flushed first request + await vi.runAllTimersAsync() + expect(mockExecute1).toHaveBeenCalledTimes(1) + expect(mockExecute2).toHaveBeenCalledTimes(1) + }) + + it("should not flush when shouldFlush returns false", async () => { + const mockExecute1 = vi.fn().mockResolvedValue(undefined) + const mockExecute2 = vi.fn().mockResolvedValue(undefined) + + debouncer.debounce(mockExecute1, 300) + + // Advance partway + await vi.advanceTimersByTimeAsync(100) + + // Make another call with shouldFlush returning false + const shouldFlush = vi.fn().mockReturnValue(false) + const promise = debouncer.debounce(mockExecute2, 300, shouldFlush) + + // Should restart timer, not flush + await vi.advanceTimersByTimeAsync(300) + await promise + + // Only second should execute + expect(mockExecute1).not.toHaveBeenCalled() + expect(mockExecute2).toHaveBeenCalledTimes(1) + }) + + it("should resolve all pending promises when request completes", async () => { + const mockExecute = vi.fn().mockResolvedValue(undefined) + + const promise1 = debouncer.debounce(mockExecute, 300) + const promise2 = debouncer.debounce(mockExecute, 300) + const promise3 = debouncer.debounce(mockExecute, 300) + + await vi.advanceTimersByTimeAsync(300) + + // All promises should resolve + await Promise.all([promise1, promise2, promise3]) + + // But execute should only be called once + expect(mockExecute).toHaveBeenCalledTimes(1) + }) + + it("should clear pending requests without executing", () => { + const mockExecute = vi.fn().mockResolvedValue(undefined) + + debouncer.debounce(mockExecute, 300) + + // Clear without executing + debouncer.clear() + + // Advance timers + vi.advanceTimersByTime(300) + + // Should not execute + expect(mockExecute).not.toHaveBeenCalled() + }) + + it("should report pending status correctly", () => { + expect(debouncer.hasPending()).toBe(false) + + debouncer.debounce(vi.fn().mockResolvedValue(undefined), 300) + expect(debouncer.hasPending()).toBe(true) + + debouncer.clear() + expect(debouncer.hasPending()).toBe(false) + }) + + it("should handle async execution errors gracefully", async () => { + const mockExecute = vi.fn().mockRejectedValue(new Error("Test error")) + + const promise = debouncer.debounce(mockExecute, 300) + + await vi.advanceTimersByTimeAsync(300) + + // Should not throw, just resolve the promise + await expect(promise).resolves.toBeUndefined() + }) +}) diff --git a/src/services/ghost/classic-auto-complete/__tests__/RequestDeduplicator.test.ts b/src/services/ghost/classic-auto-complete/__tests__/RequestDeduplicator.test.ts new file mode 100644 index 00000000000..1e82b2aa1b6 --- /dev/null +++ b/src/services/ghost/classic-auto-complete/__tests__/RequestDeduplicator.test.ts @@ -0,0 +1,232 @@ +import { describe, it, expect, beforeEach, vi } from "vitest" +import { RequestDeduplicator, type PendingRequest } from "../RequestDeduplicator" +import type { LLMRetrievalResult } from "../GhostInlineCompletionProvider" + +describe("RequestDeduplicator", () => { + let deduplicator: RequestDeduplicator + + beforeEach(() => { + deduplicator = new RequestDeduplicator() + }) + + function createMockRequest(prefix: string, suffix: string): PendingRequest { + return { + prefix, + suffix, + promise: Promise.resolve({ + suggestion: { text: "test", prefix, suffix }, + cost: 0, + inputTokens: 0, + outputTokens: 0, + cacheWriteTokens: 0, + cacheReadTokens: 0, + } as LLMRetrievalResult), + abortController: new AbortController(), + } + } + + describe("findReusable", () => { + it("should find exact match", () => { + const request = createMockRequest("const x = ", "") + deduplicator.add("const x = ", "", request) + + const found = deduplicator.findReusable("const x = ", "") + expect(found).toBe(request) + }) + + it("should find reusable request when user typed ahead", () => { + const request = createMockRequest("const x = ", "") + deduplicator.add("const x = ", "", request) + + // User typed "f" after the original prefix + const found = deduplicator.findReusable("const x = f", "") + expect(found).toBe(request) + }) + + it("should not find request with different suffix", () => { + const request = createMockRequest("const x = ", "\nconst y = 2") + deduplicator.add("const x = ", "\nconst y = 2", request) + + const found = deduplicator.findReusable("const x = ", "\nconst z = 3") + expect(found).toBeNull() + }) + + it("should not find request when prefix diverged", () => { + const request = createMockRequest("const x = f", "") + deduplicator.add("const x = f", "", request) + + // User changed to "g" instead of continuing with "f" + const found = deduplicator.findReusable("const x = g", "") + expect(found).toBeNull() + }) + + it("should return null when no requests exist", () => { + const found = deduplicator.findReusable("const x = ", "") + expect(found).toBeNull() + }) + + it("should prefer exact match over type-ahead match", () => { + const request1 = createMockRequest("const x = ", "") + const request2 = createMockRequest("const x = f", "") + + deduplicator.add("const x = ", "", request1) + deduplicator.add("const x = f", "", request2) + + // Should return exact match, not the shorter prefix + const found = deduplicator.findReusable("const x = f", "") + expect(found).toBe(request2) + }) + }) + + describe("add and remove", () => { + it("should add and retrieve request", () => { + const request = createMockRequest("const x = ", "") + deduplicator.add("const x = ", "", request) + + expect(deduplicator.size()).toBe(1) + const found = deduplicator.findReusable("const x = ", "") + expect(found).toBe(request) + }) + + it("should remove request", () => { + const request = createMockRequest("const x = ", "") + deduplicator.add("const x = ", "", request) + + deduplicator.remove("const x = ", "") + + expect(deduplicator.size()).toBe(0) + const found = deduplicator.findReusable("const x = ", "") + expect(found).toBeNull() + }) + + it("should handle multiple requests", () => { + const request1 = createMockRequest("const x = ", "") + const request2 = createMockRequest("const y = ", "") + + deduplicator.add("const x = ", "", request1) + deduplicator.add("const y = ", "", request2) + + expect(deduplicator.size()).toBe(2) + }) + }) + + describe("cancelObsolete", () => { + it("should cancel requests with different suffix", () => { + const request = createMockRequest("const x = ", "\nconst y = 2") + const abortSpy = vi.spyOn(request.abortController, "abort") + + deduplicator.add("const x = ", "\nconst y = 2", request) + + // Cancel with different suffix + deduplicator.cancelObsolete("const x = ", "\nconst z = 3") + + expect(abortSpy).toHaveBeenCalled() + expect(deduplicator.size()).toBe(0) + }) + + it("should cancel requests with diverged prefix", () => { + const request = createMockRequest("const x = f", "") + const abortSpy = vi.spyOn(request.abortController, "abort") + + deduplicator.add("const x = f", "", request) + + // Cancel with diverged prefix + deduplicator.cancelObsolete("const x = g", "") + + expect(abortSpy).toHaveBeenCalled() + expect(deduplicator.size()).toBe(0) + }) + + it("should not cancel reusable requests", () => { + const request = createMockRequest("const x = ", "") + const abortSpy = vi.spyOn(request.abortController, "abort") + + deduplicator.add("const x = ", "", request) + + // User typed ahead - should not cancel + deduplicator.cancelObsolete("const x = f", "") + + expect(abortSpy).not.toHaveBeenCalled() + expect(deduplicator.size()).toBe(1) + }) + + it("should not cancel when current prefix is shorter (backspace)", () => { + const request = createMockRequest("const x = fu", "") + const abortSpy = vi.spyOn(request.abortController, "abort") + + deduplicator.add("const x = fu", "", request) + + // User backspaced - should not cancel + deduplicator.cancelObsolete("const x = f", "") + + expect(abortSpy).not.toHaveBeenCalled() + expect(deduplicator.size()).toBe(1) + }) + + it("should handle multiple requests", () => { + const request1 = createMockRequest("const x = ", "") + const request2 = createMockRequest("const y = ", "") + const request3 = createMockRequest("const x = f", "") + + const abort1 = vi.spyOn(request1.abortController, "abort") + const abort2 = vi.spyOn(request2.abortController, "abort") + const abort3 = vi.spyOn(request3.abortController, "abort") + + deduplicator.add("const x = ", "", request1) + deduplicator.add("const y = ", "", request2) + deduplicator.add("const x = f", "", request3) + + // Cancel obsolete for "const x = fu" + deduplicator.cancelObsolete("const x = fu", "") + + // request2 should be cancelled (different prefix) + // request1 and request3 should not be cancelled (reusable) + expect(abort1).not.toHaveBeenCalled() + expect(abort2).toHaveBeenCalled() + expect(abort3).not.toHaveBeenCalled() + expect(deduplicator.size()).toBe(2) + }) + }) + + describe("clear", () => { + it("should cancel and clear all requests", () => { + const request1 = createMockRequest("const x = ", "") + const request2 = createMockRequest("const y = ", "") + + const abort1 = vi.spyOn(request1.abortController, "abort") + const abort2 = vi.spyOn(request2.abortController, "abort") + + deduplicator.add("const x = ", "", request1) + deduplicator.add("const y = ", "", request2) + + deduplicator.clear() + + expect(abort1).toHaveBeenCalled() + expect(abort2).toHaveBeenCalled() + expect(deduplicator.size()).toBe(0) + }) + + it("should handle empty deduplicator", () => { + expect(() => deduplicator.clear()).not.toThrow() + expect(deduplicator.size()).toBe(0) + }) + }) + + describe("size", () => { + it("should return correct size", () => { + expect(deduplicator.size()).toBe(0) + + deduplicator.add("const x = ", "", createMockRequest("const x = ", "")) + expect(deduplicator.size()).toBe(1) + + deduplicator.add("const y = ", "", createMockRequest("const y = ", "")) + expect(deduplicator.size()).toBe(2) + + deduplicator.remove("const x = ", "") + expect(deduplicator.size()).toBe(1) + + deduplicator.clear() + expect(deduplicator.size()).toBe(0) + }) + }) +})