feat(autocomplete): inline completion with a dedicated FIM model - #1187
Open
Rafael-Silva-Oliveira wants to merge 6 commits into
Open
feat(autocomplete): inline completion with a dedicated FIM model#1187Rafael-Silva-Oliveira wants to merge 6 commits into
Rafael-Silva-Oliveira wants to merge 6 commits into
Conversation
Rafael-Silva-Oliveira
requested review from
JamesRobert20,
edelauna,
hannesrudolph,
navedmerchant and
taltas
as code owners
August 7, 2026 16:36
Adds Copilot-style ghost text driven by a completion model configured independently of the chat model, so a small fast local model can serve tab-completion while a large cloud model serves chat. Engine (src/services/autocomplete/): - ZooInlineCompletionProvider with prefilters (multi-cursor, language, .rooignore, suggest-widget) and a manual trigger command - CompletionEngine: debounce, LRU cache with typed-prefix continuation, windowing, prompt building, stream post-processing - FIM handlers for Ollama (NDJSON) and OpenAI-compatible (SSE), plus a chat path for instruction-tuned models - FimTemplateRegistry resolving qwen/starcoder/codestral/codellama/ deepseek/codegemma/instruct from the model id - Stream transforms: stop tokens, reasoning blocks, hallucinated paths, suffix/similar-line repetition, prose, repetition loops, line cap - ContextGatherer racing file-header and open-tab sources under a wall-clock budget so a slow source degrades rather than delays - AutocompleteLogger behind zoo-code.autocomplete.debugLogging Settings UI: - New Autocomplete section in the settings strip - Provider list reused from the Providers tab constant, so a provider added there appears here with no further change - Model picker that lists what the endpoint offers, with free-text entry and connection status - Named setups for switching between local and cloud configurations - API key stored in SecretStorage, write-only in the webview, and excluded from exported settings
… transports The prompt already carried prefix and suffix end to end, but several details around it were wrong in ways that quietly cost context: - Model ids carrying an instruct tag were routed to the chat path even when the family is FIM-trained. `codestral:22b-v0.1-instruct` and `qwen2.5-coder:7b-instruct` are the tags people actually run on Ollama, and the chat path discards the suffix outright. Family now outranks the tag, with `fimTemplate` as the escape hatch. - Codestral rendered `[SUFFIX]...[PREFIX]...` with no `[MIDDLE]`, so the model was never told where the hole starts, and `[MIDDLE]` was listed as a stop sequence — truncating the reply at its own prompt boundary. - Qwen now uses its repo-level `<|file_sep|>` format for cross-file snippets instead of bare concatenated text, which reads as more code to continue. - The suffix window was starved relative to the prefix (50 lines vs 200). Raised to 150 lines / 768 tokens. - `trimToTokenBudget` re-sliced already-windowed text with a raw slice, undoing the surrogate-pair and line-boundary handling applied upstream of it. - The native-FIM snippet preamble was concatenated flush onto the prefix. The provider dropdown is now the three FIM transports only. Offering every chat provider produced a list where three entries worked well and the rest quietly underperformed, including three near-identical "OpenAI" rows. With the list restricted, the "prompted as a chat model" warning is unreachable and is removed along with its strings. Also clamps the suggestion-length slider to its own range, since a stored value above 512 could only render as a handle pinned to the far right.
Rafael-Silva-Oliveira
force-pushed
the
feat-autocompletion
branch
from
August 7, 2026 16:37
bdbc230 to
1e9ccd2
Compare
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe pull request adds inline autocomplete across shared contracts, provider integrations, streaming completion processing, extension activation, secure persistence, settings UI, profiles, model discovery, commands, telemetry, localization, and automated tests. ChangesInline autocomplete
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Editor
participant ZooInlineCompletionProvider
participant CompletionEngine
participant FimCompletionHandler
participant ModelEndpoint
Editor->>ZooInlineCompletionProvider: request inline completion
ZooInlineCompletionProvider->>CompletionEngine: provide completion input
CompletionEngine->>FimCompletionHandler: stream FIM or chat request
FimCompletionHandler->>ModelEndpoint: send provider request
ModelEndpoint-->>FimCompletionHandler: stream generated text
FimCompletionHandler-->>CompletionEngine: return completion chunks
CompletionEngine-->>Editor: return inline completion item
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/services/autocomplete/__tests__/OllamaFimHandler.spec.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. src/services/autocomplete/__tests__/OpenAiCompatibleFimHandler.spec.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
CodeQL flagged the Math.random() id generator as insecure randomness. The value is a local UI key for a saved settings profile rather than anything security-bearing, but crypto.randomUUID() is already the id-generation pattern elsewhere in the codebase, has no downside here, and collides less.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 16
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (18)
src/services/autocomplete/AutocompleteService.ts-116-121 (1)
116-121: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThe base-URL override is ignored on the fallback provider path.
Lines 100-101 and 130-132 state that an unsaved override must win over persisted config. Line 119 checks
this.configService.getConfig().baseUrland skipsoverrides?.baseUrl. When the user selects a Providers-tab provider and types a base URL that is not saved yet, this returnsNOOP_HANDLERand the model picker stays empty.🐛 Proposed fix
- if (this.configService.getConfig().baseUrl) { + if (overrides?.baseUrl || config.baseUrl) { return new OpenAiCompatibleFimHandler({ getConfig, getApiKey }) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/autocomplete/AutocompleteService.ts` around lines 116 - 121, The fallback provider path in AutocompleteService’s provider handler selection must honor an unsaved base URL override before reading persisted configuration. Update the condition around OpenAiCompatibleFimHandler to reuse the override-first base-URL resolution used by the nearby provider paths, while preserving NOOP_HANDLER behavior when neither source provides a URL.src/services/autocomplete/providers/OllamaFimHandler.ts-110-123 (1)
110-123: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
validateprintsundefinedwhen no model is selected.
config.modelIdis optional in the constructor options type (Line 127). If no model is configured, Line 119 returnsModel "undefined" was not found on the Ollama server. Return a specific message instead.🐛 Proposed fix
const models = await this.listModels(signal) const config = this.options.getConfig() + if (!config.modelId) { + return { ok: false, error: "No autocomplete model is selected." } + } + if (models.some((model) => model.id === config.modelId)) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/autocomplete/providers/OllamaFimHandler.ts` around lines 110 - 123, Update OllamaFimHandler.validate to handle a missing config.modelId before checking the available models. Return a specific validation error indicating that no Ollama model is configured, and preserve the existing model availability check for configured model IDs.src/services/autocomplete/context/sources/OpenTabsSource.ts-48-48 (1)
48-48: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse a language-appropriate comment marker for the file-path header.
Line 48 prefixes the snippet with
#. That is a comment only in Python, Ruby, and shell. The source filters to documents that matchinput.document.languageId, so for TypeScript, Java, C, or Go the prompt receives a bare# path/to/file.tsline that the model reads as code. This can degrade the completion or cause the marker to appear in generated output.Map
languageIdto a comment prefix, and fall back to//.🐛 Proposed fix
+const HASH_COMMENT_LANGUAGES = new Set(["python", "ruby", "shellscript", "yaml", "perl", "r", "makefile"]) + +function commentPrefix(languageId: string): string { + return HASH_COMMENT_LANGUAGES.has(languageId) ? "#" : "//" +}if (declarations.length > 0) { + const marker = commentPrefix(document.languageId) snippets.push({ - content: `# ${vscode.workspace.asRelativePath(document.uri)}\n${declarations.join("\n")}`, + content: `${marker} ${vscode.workspace.asRelativePath(document.uri)}\n${declarations.join("\n")}`, filePath: document.uri.fsPath,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/autocomplete/context/sources/OpenTabsSource.ts` at line 48, Update the snippet header construction in OpenTabsSource to derive a comment prefix from the document’s languageId, using # only for languages that support it and // as the fallback. Preserve the existing relative path and declarations content while replacing the hard-coded # prefix in the content template.src/services/autocomplete/__tests__/templates.spec.ts-230-239 (1)
230-239: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winStale negative assertions against
INSTRUCT_SYSTEM_PROMPTin two spec files. Both files assertnot.toMatch(/output only/i)to prove that the instruction text never reaches the rendered prompt. The currentINSTRUCT_SYSTEM_PROMPTsays "Reply with ONLY the raw code" and never says "output only", so the assertions pass unconditionally and guard nothing. The wording changed and the tests were not updated.
src/services/autocomplete/__tests__/templates.spec.ts#L230-L239: replacenot.toMatch(/output only/i)andnot.toMatch(/do not/i)withnot.toContain(INSTRUCT_SYSTEM_PROMPT)plus a match on real prompt text such as/code completion engine/i.src/services/autocomplete/prompt/__tests__/PromptBuilder.spec.ts#L81-L84: replacenot.toMatch(/output only/i)withnot.toContain(built.systemPrompt!)plus a match on real prompt text.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/autocomplete/__tests__/templates.spec.ts` around lines 230 - 239, Update the stale prompt-exclusion assertions in src/services/autocomplete/__tests__/templates.spec.ts lines 230-239 and src/services/autocomplete/prompt/__tests__/PromptBuilder.spec.ts lines 81-84: replace wording-dependent /output only/i and /do not/i checks with assertions that the rendered prompt excludes INSTRUCT_SYSTEM_PROMPT or built.systemPrompt!, respectively, while also matching stable actual prompt text such as /code completion engine/i.src/services/autocomplete/prompt/FimTemplateRegistry.ts-5-13 (1)
5-13: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winThe class doc states the wrong fallback.
Step 3 says the resolver falls back to the
"none"template. The implementation falls back to"instruct"(lines 60-67), andsrc/services/autocomplete/__tests__/templates.spec.ts:35-45asserts that behavior. The doc also omits the base-model branch and the "family outranks instruct" rule.📝 Proposed doc correction
* Resolution order: * 1. An explicit override other than `"auto"` wins outright. - * 2. Otherwise the first template whose {`@link` FimTemplate.matches} regexp tests - * the model id. - * 3. Falls back to the `"none"` template (prefix only). + * 2. Otherwise the first known FIM family whose {`@link` FimTemplate.matches} + * regexp tests the model id (`instruct` and `none` excluded). + * 3. A base model with no family match uses `"none"` (prefix only). + * 4. Everything else falls back to `"instruct"`.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/autocomplete/prompt/FimTemplateRegistry.ts` around lines 5 - 13, Update the class documentation for FimTemplateRegistry to match the resolver’s actual behavior: document the base-model branch, state that a matching family template takes precedence over the instruct fallback, and change the final fallback from "none" to "instruct".src/services/autocomplete/__tests__/templates.spec.ts-55-58 (1)
55-58: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winThis test does not exercise an unknown override.
The comment states that an unknown override id is treated as
"auto". The call passes"auto", so the test duplicates the preceding case at lines 51-53 and leaves the unknown-override branch inFimTemplateRegistry.resolveuncovered.
FimTemplateIdrejects an arbitrary literal, so cast the value and document the cast, as the guideline permits for a last-resort assertion.💚 Proposed test covering the unknown-override branch
it("falls back to model-id match when override is unknown", () => { // An unknown override id is treated as "auto" — resolve by model id. - expect(registry.resolve("starcoder2", "auto").id).toBe("starcoder") + // Double assertion: FimTemplateId forbids the literal, and an unknown id + // is exactly what this branch must survive at runtime. + const unknown = "not-a-template" as unknown as FimTemplateId + expect(registry.resolve("starcoder2", unknown).id).toBe("starcoder") })Add the import:
import type { FimTemplateId } from "@roo-code/types".Based on the coding guideline "Use double assertions only as a last resort and document them."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/autocomplete/__tests__/templates.spec.ts` around lines 55 - 58, Update the test case “falls back to model-id match when override is unknown” to pass an actually unknown override value instead of `"auto"`, casting it to FimTemplateId as a last-resort assertion and documenting that cast. Add the required type-only FimTemplateId import, while preserving the expected `"starcoder"` resolution.Source: Coding guidelines
src/services/autocomplete/cache/CompletionCache.ts-36-46 (1)
36-46: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winVerify the stored prefix and suffix on an exact-key hit.
makeCacheKeyreduces the prefix and the suffix to two 32-bit djb2 hashes. Two different contexts can produce the same key.getthen returns a completion built for other code, and the engine renders it as ghost text without any further check. The entry already storesprefixandsuffix, so a direct comparison removes the risk.🛡️ Proposed fix: confirm the entry matches the context
- get(key: string): CompletionCacheEntry | undefined { + get(key: string, prefix?: string, suffix?: string): CompletionCacheEntry | undefined { const entry = this.entries.get(key) - if (entry) { + if (entry) { + // Guard against a hash collision: the key is two 32-bit hashes. + if ((prefix !== undefined && entry.prefix !== prefix) || (suffix !== undefined && entry.suffix !== suffix)) { + return undefined + } + // Refresh recency: re-insert moves the key to the end of the Map. this.entries.delete(key) this.entries.set(key, entry) } return entry }Then update the caller in
src/services/autocomplete/CompletionEngine.ts(Line 104) tothis.cache.get(key, prefix, suffix).Also applies to: 118-131
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/autocomplete/cache/CompletionCache.ts` around lines 36 - 46, Update CompletionCache.get to accept the requested prefix and suffix, and return a cached entry only when both exactly match the stored entry values; otherwise return undefined without refreshing recency. Update the caller in CompletionEngine to pass prefix and suffix along with key.src/services/autocomplete/stream/streamReaders.ts-121-124 (1)
121-124: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
flushSsediscards the final event.The comment states that a server may omit the trailing blank line, but
flushSsedoes nothing with the buffer. When the last block arrives without\n\n, the reader drops it silently. Parse the remaining buffer and yield it. Add the decoder flush as well, so a multi-byte character split across the last two reads is not lost.🐛 Proposed fix: parse the trailing block
if (done) { - flushSse(buffer) + buffer += decoder.decode() + + const trailing = parseSseBlock(buffer) + + if (trailing) { + yield trailing + } + return }Then remove the now-unused
flushSsehelper:-function flushSse(buffer: string): void { - // Only declared so the done-branch mirrors the NDJSON flush; SSE blocks end - // with a blank line, but a server may omit the trailing one. - void buffer -}Apply the same
decoder.decode()flush inreadNdjsonbefore the trailing-line check at Line 71.Also applies to: 153-157
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/autocomplete/stream/streamReaders.ts` around lines 121 - 124, Update the stream reader’s done handling to flush the TextDecoder before processing remaining data, preserving multi-byte characters split across the final reads. Parse and yield the trailing SSE event from the remaining buffer instead of calling flushSse, then remove the unused flushSse helper. Apply the same decoder flush in readNdjson before its trailing-line check.webview-ui/src/i18n/locales/pl/settings.json-48-140 (1)
48-140: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTranslate the shared untranslated autocomplete strings.
The new autocomplete localization blocks are only partially translated in three locales. Several user-facing values remain in English, so each affected settings page switches language within one section.
webview-ui/src/i18n/locales/pl/settings.json#L48-L140: translate the English values in the model, API key, profiles, behavior, advanced, connection, and chat-provider keys.webview-ui/src/i18n/locales/tr/settings.json#L48-L140: translate the English values in the model, API key, profiles, behavior, advanced, connection, and chat-provider keys.webview-ui/src/i18n/locales/vi/settings.json#L48-L140: translate the English values in the model, API key, profiles, behavior, advanced, connection, and chat-provider keys.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@webview-ui/src/i18n/locales/pl/settings.json` around lines 48 - 140, Translate all remaining English user-facing autocomplete strings in the model, API key, profiles, behavior, advanced, connection, and chatProvider sections. Apply the corresponding translations in webview-ui/src/i18n/locales/pl/settings.json lines 48-140, webview-ui/src/i18n/locales/tr/settings.json lines 48-140, and webview-ui/src/i18n/locales/vi/settings.json lines 48-140; preserve placeholders such as {{error}}, {{name}}, {{stored}}, {{max}}, and {{count}} unchanged.src/activate/registerAutocomplete.ts-25-45 (1)
25-45: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse the injected API-key accessor.
Line 44 ignores
options.getApiKey. It dereferencesprovider.contextProxyinstead. A valid caller can providegetApiKeywithout exposing that implementation detail onprovider.Pass
getApiKeydirectly toAutocompleteService.create.Proposed fix
-export async function registerAutocomplete(options: RegisterAutocompleteOptions): Promise<AutocompleteService> { - const { context, getGlobalConfig, provider } = options +export async function registerAutocomplete(options: RegisterAutocompleteOptions): Promise<AutocompleteService> { + const { context, getGlobalConfig, getApiKey, provider } = options ... - getApiKey: () => provider.contextProxy.getValue("autocompleteApiKey"), + getApiKey,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/activate/registerAutocomplete.ts` around lines 25 - 45, Update the AutocompleteService.create call in registerAutocomplete to pass the injected options.getApiKey accessor directly, rather than reading the API key through provider.contextProxy. Preserve the existing service initialization and other arguments.webview-ui/src/i18n/locales/ca/settings.json-79-79 (1)
79-79: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winMany Catalan values are still English.
The
defile translates the same keys, so this is a gap rather than a deliberate policy. A Catalan user sees the panel switch between Catalan and English within one section.Untranslated keys:
provider.nativeFimHint(line 79),model.sectionTitle,model.sectionDescription,model.selectPlaceholder,model.customPlaceholder,model.refresh,model.fetchFailed(lines 87-92),apiKey.optionalLabel,apiKey.localDescription(lines 99-100), the wholeprofilesblock (lines 103-115),behavior.sectionTitle,behavior.sectionDescription(lines 118-119),advanced.label(line 122),maxOutputTokens.label,maxOutputTokens.description(lines 125-126), theconnectionblock (lines 130-133), and thechatProviderblock (lines 136-138).Also applies to: 87-92, 99-100, 102-133, 135-138
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@webview-ui/src/i18n/locales/ca/settings.json` at line 79, Translate all identified English values in the Catalan settings locale, including provider.nativeFimHint, the model, apiKey, profiles, behavior, advanced, maxOutputTokens, connection, and chatProvider blocks. Preserve the existing keys and JSON structure while matching the Catalan terminology used elsewhere in the locale.webview-ui/src/components/settings/__tests__/SettingsView.autocomplete.spec.tsx-600-609 (1)
600-609: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThe model-fetch assertion checks the wrong message type.
autocompleteModelsis the inbound message the extension host sends to the webview (AutocompleteModelPicker.tsxline 82). The webview postsrequestAutocompleteModels(AutocompleteModelPicker.tsxline 74). This assertion can therefore never fail, and the stated intent on lines 586-588 is not verified.💚 Proposed fix
expect(mockPostMessage).not.toHaveBeenCalledWith( expect.objectContaining({ - type: "autocompleteModels", + type: "requestAutocompleteModels", }), )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@webview-ui/src/components/settings/__tests__/SettingsView.autocomplete.spec.tsx` around lines 600 - 609, Update the second mockPostMessage assertion in the autocomplete settings test to check for the outbound “requestAutocompleteModels” message rather than the inbound “autocompleteModels” type. Keep the existing negative assertion structure and leave the “updateSettings” check unchanged.webview-ui/src/components/settings/autocomplete/AutocompleteProfileBar.tsx-87-103 (1)
87-103: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winMove focus to the name field when the editor opens.
Clicking Add or Rename replaces the entire bar with this editor. React unmounts the button that had focus, so focus falls back to
document.body. A keyboard user must tab to reach the field, and theEnterandEscapehandlers on lines 94-102 do nothing until the field has focus.Add
autoFocusto theVSCodeTextField.♿ Proposed fix
<VSCodeTextField + autoFocus value={draftName} maxlength={AUTOCOMPLETE_PROFILE_LIMITS.NAME_MAX}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@webview-ui/src/components/settings/autocomplete/AutocompleteProfileBar.tsx` around lines 87 - 103, Add the autoFocus prop to the VSCodeTextField in the editor rendered by AutocompleteProfileBar, ensuring focus moves to the name field when the editor opens while preserving the existing input and keyboard handlers.webview-ui/src/components/settings/AutocompleteSettings.tsx-319-328 (1)
319-328: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winBind the debounce slider to the shared limit and clamp the stored value.
Line 321 hard-codes
max={1_000}while line 320 readsAUTOCOMPLETE_LIMITS.DEBOUNCE_MS.min. Two problems follow.If
AUTOCOMPLETE_LIMITS.DEBOUNCE_MS.maxis below 1000, the slider lets the user save a value thatautocompleteConfigSchemarejects. If a storeddebounceMsexceeds 1000, the handle pins to the far right with no notice — the same failure the author already handled formaxOutputTokensat lines 121-126.🔧 Proposed fix
- <Slider - min={AUTOCOMPLETE_LIMITS.DEBOUNCE_MS.min} - max={1_000} + <Slider + min={AUTOCOMPLETE_LIMITS.DEBOUNCE_MS.min} + max={AUTOCOMPLETE_LIMITS.DEBOUNCE_MS.max} step={25} - value={[debounceMs]} + value={[ + Math.min( + Math.max(debounceMs, AUTOCOMPLETE_LIMITS.DEBOUNCE_MS.min), + AUTOCOMPLETE_LIMITS.DEBOUNCE_MS.max, + ), + ]}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@webview-ui/src/components/settings/AutocompleteSettings.tsx` around lines 319 - 328, Update the debounce Slider in the AutocompleteSettings component to use AUTOCOMPLETE_LIMITS.DEBOUNCE_MS.max instead of the hard-coded 1_000, and clamp the stored debounceMs value to the shared min/max bounds before binding it to the slider, matching the existing maxOutputTokens handling.webview-ui/src/components/settings/SettingsView.tsx-419-438 (1)
419-438: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd test coverage for profile save during user edits.
saveAutocompleteProfileusescrypto.randomUUID(), which is available in the current test environment and VS Code webview runtime, but this path only handles saving user-entered names. This PR does not cover automatic initialization, so add a test that exercises genuine user edits inSettingsViewwhile keeping inputs buffered incachedState.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@webview-ui/src/components/settings/SettingsView.tsx` around lines 419 - 438, Add a SettingsView test that performs genuine user edits to an autocomplete profile name and saves it through saveAutocompleteProfile, with inputs buffered in cachedState. Assert the saved profile state and change detection, covering the crypto.randomUUID path without testing automatic initialization.webview-ui/src/components/settings/__tests__/AutocompleteSettings.spec.tsx-176-187 (1)
176-187: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse a hoisted
vscodemock for this mount assertion.
vscodeis imported duringAutocompleteSettingsmodule evaluation, before this test runs.vi.stubGlobal("acquireVsCodeApi", ...)later therefore does not reach the wrapper that caches the API on module creation, soexpect(postMessage).not.toHaveBeenCalled()can pass without checking mount behavior. Use a hoisted mock likeSettingsView.autocomplete.spec.tsxwithvi.mock("@src/utils/vscode", () => ({ vscode: { postMessage: mockPostMessage } }))and check that mockedpostMessage.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@webview-ui/src/components/settings/__tests__/AutocompleteSettings.spec.tsx` around lines 176 - 187, Update the headless mount test around renderSettings to use a hoisted mock for `@src/utils/vscode`, providing a shared mockPostMessage through vscode.postMessage before AutocompleteSettings is evaluated. Remove the ineffective acquireVsCodeApi global stub and assert that mockPostMessage was not called after rendering.webview-ui/src/components/settings/AutocompleteSettings.tsx-61-65 (1)
61-65: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse the existing translation keys for the provider labels.
NATIVE_FIM_OPTIONShard-codes English labels. The locale files already defineautocomplete.provider.options.ollama,autocomplete.provider.options.openai-compatible, andautocomplete.provider.options.codestral(seewebview-ui/src/i18n/locales/de/settings.jsonlines 73-78). Localized users therefore see English text although a translation exists.Build the options inside the component so
tis available. Update the assertion inwebview-ui/src/components/settings/__tests__/AutocompleteSettings.spec.tsxline 156 accordingly.🌐 Proposed fix
-const NATIVE_FIM_OPTIONS: readonly { value: string; label: string }[] = [ - { value: "ollama", label: "Ollama" }, - { value: "openai-compatible", label: "OpenAI Compatible (LM Studio, llama.cpp, vLLM)" }, - { value: "codestral", label: "Mistral Codestral" }, -] +const NATIVE_FIM_PROVIDER_IDS = ["ollama", "openai-compatible", "codestral"] as constThen inside the component:
- const providerOptions = useMemo(() => [...NATIVE_FIM_OPTIONS], []) + const providerOptions = useMemo( + () => + NATIVE_FIM_PROVIDER_IDS.map((value) => ({ + value, + label: t(`settings:autocomplete.provider.options.${value}`), + })), + [t], + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@webview-ui/src/components/settings/AutocompleteSettings.tsx` around lines 61 - 65, Replace the module-level NATIVE_FIM_OPTIONS constant with options created inside the AutocompleteSettings component, using the existing translation keys autocomplete.provider.options.ollama, autocomplete.provider.options.openai-compatible, and autocomplete.provider.options.codestral through t. Update the corresponding test assertion in AutocompleteSettings.spec.tsx to expect the localized labels.webview-ui/src/i18n/locales/de/settings.json-102-133 (1)
102-133: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winKeep the address form consistent in the new settings strings.
This section mixes informal and formal German, for example “deiner Wahl / du kannst / Tippe / Gib deinen” with “Speichern Sie / Sie haben / Geben Sie / Bewege den Regler”. Use one form across the new block and keep it consistent with the surrounding locale strings.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@webview-ui/src/i18n/locales/de/settings.json` around lines 102 - 133, Unify the German address form across the new settings strings, especially the visible profiles, behavior, maxOutputTokens, and connection entries. Match the formal “Sie” style already used in these entries and update any remaining informal pronouns or imperative forms in the surrounding new block consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/types/src/autocomplete.ts`:
- Around line 202-207: Update the autocomplete schema’s provider field near the
provider declaration to validate values against the declared autocomplete
transport IDs and required migration aliases instead of accepting any string.
Preserve the chat-provider selection through chatFallbackProvider, and ensure
unsupported IDs cannot enter completion routing.
In `@src/services/autocomplete/__tests__/tokenBudget.spec.ts`:
- Around line 47-55: Fix the surrogate-pair test around trimToTokenBudget by
choosing input and budget values that place head or tail boundaries at an odd
code-unit offset within the emoji run. Replace the self-comparison assertions
with direct code-unit validation that rejects lone high or low surrogates, while
preserving coverage of both "head" and "tail" results.
In `@src/services/autocomplete/AutocompleteService.ts`:
- Around line 74-85: The completion engine is built only once and becomes stale
after autocomplete settings change. Update AutocompleteService so the engine
created by buildEngine is stored in a mutable field or accessor, then rebuild
and replace it in handleSettingsChange; ensure ZooInlineCompletionProvider uses
the current engine reference without requiring provider re-registration.
In `@src/services/autocomplete/CompletionEngine.ts`:
- Around line 291-317: Update meetsMinCharsTyped to evaluate the offset
difference after a completion even when document.version has advanced, using the
stored version only to reject stale or unrelated document state as appropriate;
ensure the normal post-keystroke path reaches the typed >= minCharsTyped check.
Add a unit test in CompletionEngine.spec.ts with minCharsTyped above zero that
produces one completion, advances the document version, and verifies the second
request returns undefined.
In `@src/services/autocomplete/constants.ts`:
- Around line 12-13: Rename MAX_DOCUMENT_BYTES to MAX_DOCUMENT_CHARS and update
its documentation and all references, including ZooInlineCompletionProvider.
Replace the getText().length size check with a document-size proxy using
lineCount and the final line’s offsetAt endpoint, preserving the existing
threshold behavior without materializing the full document.
In `@src/services/autocomplete/prompt/tokenBudget.ts`:
- Around line 69-99: Correct the surrogate checks in the tail/head trimming
logic: the "tail" branch should remove an orphan low surrogate at the first
character, while the head branch should remove an orphan high surrogate at the
last character. Update the token budget tests with an odd boundary case, such as
a one-character tail slice from text ending in an emoji, to verify both guards
remove orphaned code units.
In `@src/services/autocomplete/providers/OllamaFimHandler.ts`:
- Around line 125-130: The OllamaFimHandler requests do not use the configured
autocomplete API key. Update streamFim’s fetchGenerate flow and listModels to
select request.apiKey or getApiKey(), and add an Authorization Bearer header
when a key is available for both /api/generate and /api/tags requests.
In `@src/services/autocomplete/providers/OpenAiCompatibleFimHandler.ts`:
- Around line 208-216: Update OpenAiCompatibleFimHandler.normalizeBaseUrl so
scheme-less URLs default to https://, while retaining http:// only when the host
is loopback. Preserve explicitly provided http:// and https:// schemes and the
existing URL trimming/version normalization.
In `@src/services/autocomplete/stream/transforms.ts`:
- Around line 253-279: Update stopAtRepetitionLoop to enforce a minimum matched
span before truncating and skip candidate units composed only of whitespace,
preventing indentation and short runs from being treated as loops. Preserve
detection for genuine longer repetition loops, and add a regression test in the
transforms.spec.ts DEFAULT_TRANSFORMS coverage that streams an indented
multiline completion and asserts the complete text is retained.
- Around line 73-91: Update HALLUCINATED_PATH_REGEX so its path/file
alternatives require the documented header punctuation, such as “Path: …” or
“File: …”, instead of matching bare identifiers followed by a word boundary.
Preserve the existing filterHallucinatedPathLine behavior while ensuring
ordinary code like path expressions and file method calls is not discarded.
In `@src/services/autocomplete/types.ts`:
- Around line 22-33: Update FileHeaderSource.gather to derive
AutocompleteSnippet.filePath from the workspace root rather than assigning
document.uri.fsPath directly, and normalize separators to POSIX format. Preserve
the documented workspace-relative contract so renderSnippetPreamble and Qwen
repository context can continue consuming filePath unchanged.
In `@webview-ui/src/components/settings/autocomplete/AutocompleteModelPicker.tsx`:
- Around line 59-98: Update the autocomplete model request flow around
fetchModels and onMessage to associate each response with the endpointKey or
latest request identity. Include that identity in the request/response payload
contract, then ignore responses that do not match the current requested key
before changing status, error, or models; preserve handling for the active
request.
- Around line 115-125: Update the debounced fetch effect around fetchModels to
track the endpoint key already requested for the current endpoint and return
early when that key has completed, rather than using models.length as the
completion signal. Preserve fetching for a newly entered or changed endpoint,
including empty model-list responses, while preventing repeated requests after
status returns to connected.
In `@webview-ui/src/components/settings/SettingsView.tsx`:
- Around line 379-391: Clear autocompleteApiKeyDraft in the discard branch of
onConfirmDialogResult alongside restoring cachedState and resetting
isChangeDetected, so discarded keys are removed from the field and omitted from
later save payloads. Add a regression test in the existing
SettingsView.autocomplete.spec.tsx draft-consumption tests covering discard
followed by an unrelated edit and save.
- Around line 570-574: Update the settings payload construction to send null
rather than omit activeAutocompleteProfileId when deleteAutocompleteProfile
clears it, preserving existing values for untouched edits. Extend the
corresponding settings schema and TypeScript type to accept null, matching the
established allowedMaxRequests pattern.
In `@webview-ui/src/i18n/locales/hi/settings.json`:
- Around line 79-138: Translate all remaining English autocomplete settings
strings in webview-ui/src/i18n/locales/hi/settings.json lines 79-138,
webview-ui/src/i18n/locales/id/settings.json lines 79-138, and
webview-ui/src/i18n/locales/nl/settings.json lines 79-138, covering the model,
profiles, advanced, connection, and chatProvider sections while preserving keys
and interpolation placeholders.
---
Minor comments:
In `@src/activate/registerAutocomplete.ts`:
- Around line 25-45: Update the AutocompleteService.create call in
registerAutocomplete to pass the injected options.getApiKey accessor directly,
rather than reading the API key through provider.contextProxy. Preserve the
existing service initialization and other arguments.
In `@src/services/autocomplete/__tests__/templates.spec.ts`:
- Around line 230-239: Update the stale prompt-exclusion assertions in
src/services/autocomplete/__tests__/templates.spec.ts lines 230-239 and
src/services/autocomplete/prompt/__tests__/PromptBuilder.spec.ts lines 81-84:
replace wording-dependent /output only/i and /do not/i checks with assertions
that the rendered prompt excludes INSTRUCT_SYSTEM_PROMPT or built.systemPrompt!,
respectively, while also matching stable actual prompt text such as /code
completion engine/i.
- Around line 55-58: Update the test case “falls back to model-id match when
override is unknown” to pass an actually unknown override value instead of
`"auto"`, casting it to FimTemplateId as a last-resort assertion and documenting
that cast. Add the required type-only FimTemplateId import, while preserving the
expected `"starcoder"` resolution.
In `@src/services/autocomplete/AutocompleteService.ts`:
- Around line 116-121: The fallback provider path in AutocompleteService’s
provider handler selection must honor an unsaved base URL override before
reading persisted configuration. Update the condition around
OpenAiCompatibleFimHandler to reuse the override-first base-URL resolution used
by the nearby provider paths, while preserving NOOP_HANDLER behavior when
neither source provides a URL.
In `@src/services/autocomplete/cache/CompletionCache.ts`:
- Around line 36-46: Update CompletionCache.get to accept the requested prefix
and suffix, and return a cached entry only when both exactly match the stored
entry values; otherwise return undefined without refreshing recency. Update the
caller in CompletionEngine to pass prefix and suffix along with key.
In `@src/services/autocomplete/context/sources/OpenTabsSource.ts`:
- Line 48: Update the snippet header construction in OpenTabsSource to derive a
comment prefix from the document’s languageId, using # only for languages that
support it and // as the fallback. Preserve the existing relative path and
declarations content while replacing the hard-coded # prefix in the content
template.
In `@src/services/autocomplete/prompt/FimTemplateRegistry.ts`:
- Around line 5-13: Update the class documentation for FimTemplateRegistry to
match the resolver’s actual behavior: document the base-model branch, state that
a matching family template takes precedence over the instruct fallback, and
change the final fallback from "none" to "instruct".
In `@src/services/autocomplete/providers/OllamaFimHandler.ts`:
- Around line 110-123: Update OllamaFimHandler.validate to handle a missing
config.modelId before checking the available models. Return a specific
validation error indicating that no Ollama model is configured, and preserve the
existing model availability check for configured model IDs.
In `@src/services/autocomplete/stream/streamReaders.ts`:
- Around line 121-124: Update the stream reader’s done handling to flush the
TextDecoder before processing remaining data, preserving multi-byte characters
split across the final reads. Parse and yield the trailing SSE event from the
remaining buffer instead of calling flushSse, then remove the unused flushSse
helper. Apply the same decoder flush in readNdjson before its trailing-line
check.
In `@webview-ui/src/components/settings/__tests__/AutocompleteSettings.spec.tsx`:
- Around line 176-187: Update the headless mount test around renderSettings to
use a hoisted mock for `@src/utils/vscode`, providing a shared mockPostMessage
through vscode.postMessage before AutocompleteSettings is evaluated. Remove the
ineffective acquireVsCodeApi global stub and assert that mockPostMessage was not
called after rendering.
In
`@webview-ui/src/components/settings/__tests__/SettingsView.autocomplete.spec.tsx`:
- Around line 600-609: Update the second mockPostMessage assertion in the
autocomplete settings test to check for the outbound “requestAutocompleteModels”
message rather than the inbound “autocompleteModels” type. Keep the existing
negative assertion structure and leave the “updateSettings” check unchanged.
In `@webview-ui/src/components/settings/autocomplete/AutocompleteProfileBar.tsx`:
- Around line 87-103: Add the autoFocus prop to the VSCodeTextField in the
editor rendered by AutocompleteProfileBar, ensuring focus moves to the name
field when the editor opens while preserving the existing input and keyboard
handlers.
In `@webview-ui/src/components/settings/AutocompleteSettings.tsx`:
- Around line 319-328: Update the debounce Slider in the AutocompleteSettings
component to use AUTOCOMPLETE_LIMITS.DEBOUNCE_MS.max instead of the hard-coded
1_000, and clamp the stored debounceMs value to the shared min/max bounds before
binding it to the slider, matching the existing maxOutputTokens handling.
- Around line 61-65: Replace the module-level NATIVE_FIM_OPTIONS constant with
options created inside the AutocompleteSettings component, using the existing
translation keys autocomplete.provider.options.ollama,
autocomplete.provider.options.openai-compatible, and
autocomplete.provider.options.codestral through t. Update the corresponding test
assertion in AutocompleteSettings.spec.tsx to expect the localized labels.
In `@webview-ui/src/components/settings/SettingsView.tsx`:
- Around line 419-438: Add a SettingsView test that performs genuine user edits
to an autocomplete profile name and saves it through saveAutocompleteProfile,
with inputs buffered in cachedState. Assert the saved profile state and change
detection, covering the crypto.randomUUID path without testing automatic
initialization.
In `@webview-ui/src/i18n/locales/ca/settings.json`:
- Line 79: Translate all identified English values in the Catalan settings
locale, including provider.nativeFimHint, the model, apiKey, profiles, behavior,
advanced, maxOutputTokens, connection, and chatProvider blocks. Preserve the
existing keys and JSON structure while matching the Catalan terminology used
elsewhere in the locale.
In `@webview-ui/src/i18n/locales/de/settings.json`:
- Around line 102-133: Unify the German address form across the new settings
strings, especially the visible profiles, behavior, maxOutputTokens, and
connection entries. Match the formal “Sie” style already used in these entries
and update any remaining informal pronouns or imperative forms in the
surrounding new block consistently.
In `@webview-ui/src/i18n/locales/pl/settings.json`:
- Around line 48-140: Translate all remaining English user-facing autocomplete
strings in the model, API key, profiles, behavior, advanced, connection, and
chatProvider sections. Apply the corresponding translations in
webview-ui/src/i18n/locales/pl/settings.json lines 48-140,
webview-ui/src/i18n/locales/tr/settings.json lines 48-140, and
webview-ui/src/i18n/locales/vi/settings.json lines 48-140; preserve placeholders
such as {{error}}, {{name}}, {{stored}}, {{max}}, and {{count}} unchanged.
---
Nitpick comments:
In `@src/extension.ts`:
- Line 308: Update the registration helper to destructure and use the getApiKey
option when calling AutocompleteService.create, ensuring the callback supplied
by the activation path is the key source. Alternatively, remove the getApiKey
argument from the registration API and its caller so only one key-source
mechanism remains; do not continue reading provider.contextProxy for this value.
In `@src/services/autocomplete/__tests__/CompletionEngine.spec.ts`:
- Around line 126-127: Add a one-line comment immediately before the `as unknown
as vscode.TextDocument` assertion explaining which TextDocument members the test
double implements and why the complete vscode.TextDocument interface is not
modeled; leave the assertion and surrounding test behavior unchanged.
In `@src/services/autocomplete/__tests__/OllamaFimHandler.spec.ts`:
- Around line 48-55: Update the beforeEach/afterEach setup in OllamaFimHandler
tests to install the fetch mock with vi.stubGlobal instead of directly assigning
globalThis.fetch, and restore it with vi.unstubAllGlobals in afterEach. Preserve
the existing fetchMock initialization and mock restoration behavior.
In `@src/services/autocomplete/__tests__/OpenAiCompatibleFimHandler.spec.ts`:
- Around line 15-32: Extend the OpenAI mock class used by
OpenAiCompatibleFimHandler tests with a chat.completions.create method wired to
openaiMocks.create. Add focused coverage for useChatEndpoint routing, removal of
"```" from chat stop sequences, delta.content handling, normalizeBaseUrl
removing trailing /v1 while adding a scheme, and mapping 401/403 errors to
AUTH_ERROR_MESSAGE; keep the existing network-error coverage.
In `@src/services/autocomplete/__tests__/ZooInlineCompletionProvider.spec.ts`:
- Around line 140-185: Update the tests for
ZooInlineCompletionProvider.provideInlineCompletionItems to inject a stub engine
and assert delegation rather than asserting undefined. In “allows the Invoke
trigger in manual mode,” verify the engine is called once with InvokeContext; in
“honors a forced trigger even in manual mode with an automatic request,” verify
one delegation for the forced request and none for the subsequent automatic
request.
In `@src/services/autocomplete/AutocompleteService.ts`:
- Around line 220-235: Update NOOP_HANDLER.streamFim to complete without
yielding any fragment, preserving its no-completion behavior. Also replace the
misleading "chat-fallback" value in NOOP_HANDLER.id with a dedicated identifier
for this no-op handler so telemetry does not attribute requests to a planned
provider.
In `@src/services/autocomplete/cache/CompletionCache.ts`:
- Around line 53-87: Update getContinuation to iterate this.entries.values()
without creating or reversing an array, retaining the most recently inserted
matching continuation and returning it after the scan. Change the suffix
condition to compare entry.suffix and suffix for the intended unchanged-suffix
behavior, and refresh the matched entry’s LRU recency when returning a
continuation so repeated hits are not evicted as oldest.
In `@src/services/autocomplete/CompletionEngine.ts`:
- Around line 533-543: Update toAbortSignal to retain the Disposable returned by
token.onCancellationRequested, and ensure it is disposed when the associated
provider stream completes. Thread the cleanup through the caller’s
stream-completion path while preserving immediate cancellation behavior and the
returned AbortSignal.
In `@src/services/autocomplete/context/__tests__/ContextGatherer.spec.ts`:
- Around line 86-95: Update the “returns early rather than waiting on a slow
source” test to capture the value returned by ContextGatherer.gather and assert
that it contains no snippets after the budget expires, while preserving the
existing elapsed-time assertion.
In `@src/services/autocomplete/context/sources/OpenTabsSource.ts`:
- Line 44: Bound document scanning in collectDeclarations within
src/services/autocomplete/context/sources/OpenTabsSource.ts at lines 44-44 so it
reads and processes only the required prefix instead of splitting the full
document; likewise update collectHeader in
src/services/autocomplete/context/sources/FileHeaderSource.ts at lines 53-55 to
limit scanning to MAX_HEADER_LINES, preserving existing results for the bounded
ranges.
In `@src/services/autocomplete/prompt/PromptBuilder.ts`:
- Around line 34-35: Update the PromptBuilder field templateId from string to
FimTemplateId, matching the type of template.id and preserving the specific
identifier type for all telemetry and logging consumers.
In `@src/services/autocomplete/prompt/templates.ts`:
- Around line 54-69: Update renderQwenRepoContext to emit the Qwen repository
header `<|repo_name|>{name}` before the first `<|file_sep|>` section, using the
appropriate repository-name value available to the template. Preserve the
existing empty-snippets behavior and file-section formatting, and keep the doc
comment aligned with the resulting format.
In `@src/services/autocomplete/prompt/tokenBudget.ts`:
- Around line 3-14: Update estimateTokens and the character-budget calculations
in pruneSnippets and trimToTokenBudget to share one CHARS_PER_TOKEN constant,
deriving it from the existing estimate ratio so conversions are
inverse-consistent. Replace the duplicated 3.5 literals with that constant, and
revise the estimateTokens documentation to accurately describe the resulting
ratio and whether the estimate is conservative.
In `@src/services/autocomplete/providers/FimCompletionHandler.ts`:
- Around line 66-76: Update OpenAiCompatibleFimHandler to reuse the exported
FimHandlerOptions type instead of declaring an inline options shape; ensure its
getConfig contract matches the fields the handler reads, either by retaining the
required fields or narrowing FimHandlerOptions consistently for all handlers.
In `@src/services/autocomplete/providers/OllamaFimHandler.ts`:
- Around line 100-107: Update the model mapping in OllamaFimHandler to derive
supportsFim from model.capabilities: set it true only when capabilities are
absent or include "insert", and false for completion-only models. Keep the
existing model filtering and other mapped fields unchanged.
In `@src/services/autocomplete/providers/OpenAiCompatibleFimHandler.ts`:
- Around line 36-37: Update the degraded memo used by OpenAiCompatibleFimHandler
so entries do not persist for the instance lifetime: add an expiry or clear the
relevant entry when the model or base URL changes. Ensure transient failures
only trigger rendered-prompt fallback temporarily, allowing native FIM to resume
for endpoints that recover or configuration changes.
In `@src/services/autocomplete/stream/streamReaders.ts`:
- Around line 13-16: Update the readers readText and the other two stream-reader
functions to honor the provided AbortSignal during their read loops, stopping or
propagating cancellation when it is aborted. Remove the void signal statements
from their finally blocks, while preserving normal stream completion and cleanup
behavior.
In `@src/services/autocomplete/stream/transforms.ts`:
- Around line 341-351: Update longestSuffixPrefix to cap the initial overlap
length at a fixed scan window such as 256 characters, while still limiting it by
text.length and prefix.length. Preserve the existing descending scan to
MIN_SUFFIX_OVERLAP and return behavior.
In `@src/services/autocomplete/ZooInlineCompletionProvider.ts`:
- Around line 62-67: The forceRequested state in ZooInlineCompletionProvider
must not be consumed by an unrelated completion request. Associate the forced
trigger with the target document URI and position, or expire it within a short
time window, and only clear/use it when the incoming request matches that
target; preserve suppression of unrelated automatic triggers and allow the
intended manual request through.
- Around line 81-83: Make ZooInlineCompletionProvider the single owner of the
document-size guard by measuring size through the final line’s end offset
instead of materializing document.getText(). At
src/services/autocomplete/ZooInlineCompletionProvider.ts lines 81-83, update the
guard accordingly; at src/services/autocomplete/CompletionEngine.ts lines 92-95,
remove the duplicate check, or retain it only for direct-engine tests using the
same offset-based measurement.
In
`@webview-ui/src/components/settings/__tests__/AutocompleteProfileBar.spec.tsx`:
- Around line 51-91: Add tests to AutocompleteProfileBar.spec.tsx covering the
name input’s onKeyDown behavior: pressing Enter should submit the typed profile
name through the appropriate save or rename callback, while pressing Escape
should cancel editing without invoking save or rename and remove the editor. Use
fireEvent.keyDown on autocomplete-profile-name-input and preserve the existing
mouse-path assertions.
- Around line 99-109: Update the “blocks new profiles once the limit is reached”
test to derive the generated profile count from
AUTOCOMPLETE_PROFILE_LIMITS.MAX_PROFILES instead of the literal 20, reusing the
same exported limit constant consumed by AutocompleteProfileBar.
In
`@webview-ui/src/components/settings/__tests__/SettingsView.autocomplete.spec.tsx`:
- Around line 471-491: The autocomplete settings suite needs a regression test
covering profile operations during save. Extend the tests around renderSettings
and getUpdateSettingsPayload to delete the active profile, save the settings,
and assert the payload preserves the expected activeAutocompleteProfileId; cover
the selectAutocompleteProfile, saveAutocompleteProfile,
renameAutocompleteProfile, and deleteAutocompleteProfile flow as needed to
exercise the profile save round trip.
- Around line 196-253: Hoist the duplicated mock factories in the autocomplete
test into shared constants created with vi.hoisted, including the Tab,
ApiOptions, SearchableSetting, useSettingsSearch, and SettingsSearch factories.
Pass each hoisted factory reference to both corresponding vi.mock calls so
aliased and relative imports always use identical implementations; leave broader
shared-helper extraction for the larger UI mock block out of this change.
In `@webview-ui/src/components/settings/AutocompleteSettings.tsx`:
- Around line 133-135: Extract the shared remote-endpoint predicate from
AutocompleteSettings and AutocompleteModelPicker into a reusable helper that
treats HTTPS localhost, 127.0.0.1, and 0.0.0.0 endpoints as local. Replace both
components’ inline checks with the imported helper so API-key guidance remains
consistent.
- Around line 217-227: Update the provider option typing derived from
NATIVE_FIM_OPTIONS to use AutocompleteProviderId rather than string, then remove
the redundant cast at the AutocompleteModelPicker/SearchableSelect provider
usage while preserving the existing provider selection behavior.
In `@webview-ui/src/i18n/locales/ca/settings.json`:
- Around line 58-65: Remove the unused translation entries triggerMode,
autocomplete.advanced, connection.connectedLocal,
provider.options.chat-fallback, and chatProvider from every settings locale
file. Keep all referenced settings translations unchanged and apply the same
deletion consistently across all 18 locale files.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: cf053052-6a07-4414-8e71-79c44c9b6ae6
📒 Files selected for processing (100)
packages/types/src/__tests__/autocomplete.spec.tspackages/types/src/autocomplete.tspackages/types/src/global-settings.tspackages/types/src/index.tspackages/types/src/telemetry.tspackages/types/src/vscode-extension-host.tspackages/types/src/vscode.tssrc/__tests__/extension.spec.tssrc/activate/__tests__/registerAutocomplete.spec.tssrc/activate/__tests__/registerCommands.spec.tssrc/activate/index.tssrc/activate/registerAutocomplete.tssrc/activate/registerCommands.tssrc/core/config/ContextProxy.tssrc/core/webview/ClineProvider.tssrc/core/webview/__tests__/ClineProvider.spec.tssrc/core/webview/webviewMessageHandler.tssrc/eslint.config.mjssrc/extension.tssrc/package.jsonsrc/package.nls.ca.jsonsrc/package.nls.de.jsonsrc/package.nls.es.jsonsrc/package.nls.fr.jsonsrc/package.nls.hi.jsonsrc/package.nls.id.jsonsrc/package.nls.it.jsonsrc/package.nls.ja.jsonsrc/package.nls.jsonsrc/package.nls.ko.jsonsrc/package.nls.nl.jsonsrc/package.nls.pl.jsonsrc/package.nls.pt-BR.jsonsrc/package.nls.ru.jsonsrc/package.nls.tr.jsonsrc/package.nls.vi.jsonsrc/package.nls.zh-CN.jsonsrc/package.nls.zh-TW.jsonsrc/services/autocomplete/AutocompleteLogger.tssrc/services/autocomplete/AutocompleteService.tssrc/services/autocomplete/CompletionEngine.tssrc/services/autocomplete/ZooInlineCompletionProvider.tssrc/services/autocomplete/__tests__/CompletionCache.spec.tssrc/services/autocomplete/__tests__/CompletionEngine.spec.tssrc/services/autocomplete/__tests__/OllamaFimHandler.spec.tssrc/services/autocomplete/__tests__/OpenAiCompatibleFimHandler.spec.tssrc/services/autocomplete/__tests__/ZooInlineCompletionProvider.spec.tssrc/services/autocomplete/__tests__/prefilters.spec.tssrc/services/autocomplete/__tests__/templates.spec.tssrc/services/autocomplete/__tests__/tokenBudget.spec.tssrc/services/autocomplete/__tests__/transforms.spec.tssrc/services/autocomplete/__tests__/windowing.spec.tssrc/services/autocomplete/cache/CompletionCache.tssrc/services/autocomplete/config/AutocompleteConfigService.tssrc/services/autocomplete/constants.tssrc/services/autocomplete/context/ContextGatherer.tssrc/services/autocomplete/context/__tests__/ContextGatherer.spec.tssrc/services/autocomplete/context/__tests__/FileHeaderSource.spec.tssrc/services/autocomplete/context/sources/FileHeaderSource.tssrc/services/autocomplete/context/sources/OpenTabsSource.tssrc/services/autocomplete/context/windowing.tssrc/services/autocomplete/prefilters.tssrc/services/autocomplete/prompt/FimTemplateRegistry.tssrc/services/autocomplete/prompt/PromptBuilder.tssrc/services/autocomplete/prompt/__tests__/PromptBuilder.spec.tssrc/services/autocomplete/prompt/templates.tssrc/services/autocomplete/prompt/tokenBudget.tssrc/services/autocomplete/providers/FimCompletionHandler.tssrc/services/autocomplete/providers/OllamaFimHandler.tssrc/services/autocomplete/providers/OpenAiCompatibleFimHandler.tssrc/services/autocomplete/stream/StreamPostProcessor.tssrc/services/autocomplete/stream/streamReaders.tssrc/services/autocomplete/stream/transforms.tssrc/services/autocomplete/types.tssrc/services/autocomplete/ui/AutocompleteStatusBar.tswebview-ui/src/components/settings/AutocompleteSettings.tsxwebview-ui/src/components/settings/SettingsView.tsxwebview-ui/src/components/settings/__tests__/AutocompleteProfileBar.spec.tsxwebview-ui/src/components/settings/__tests__/AutocompleteSettings.spec.tsxwebview-ui/src/components/settings/__tests__/SettingsView.autocomplete.spec.tsxwebview-ui/src/components/settings/autocomplete/AutocompleteModelPicker.tsxwebview-ui/src/components/settings/autocomplete/AutocompleteProfileBar.tsxwebview-ui/src/i18n/locales/ca/settings.jsonwebview-ui/src/i18n/locales/de/settings.jsonwebview-ui/src/i18n/locales/en/settings.jsonwebview-ui/src/i18n/locales/es/settings.jsonwebview-ui/src/i18n/locales/fr/settings.jsonwebview-ui/src/i18n/locales/hi/settings.jsonwebview-ui/src/i18n/locales/id/settings.jsonwebview-ui/src/i18n/locales/it/settings.jsonwebview-ui/src/i18n/locales/ja/settings.jsonwebview-ui/src/i18n/locales/ko/settings.jsonwebview-ui/src/i18n/locales/nl/settings.jsonwebview-ui/src/i18n/locales/pl/settings.jsonwebview-ui/src/i18n/locales/pt-BR/settings.jsonwebview-ui/src/i18n/locales/ru/settings.jsonwebview-ui/src/i18n/locales/tr/settings.jsonwebview-ui/src/i18n/locales/vi/settings.jsonwebview-ui/src/i18n/locales/zh-CN/settings.jsonwebview-ui/src/i18n/locales/zh-TW/settings.json
Comment on lines
+202
to
+207
| * Kept as a free string rather than the `autocompleteProviderIds` enum so the | ||
| * settings UI can offer every provider the extension supports without this | ||
| * schema having to be edited each time one is added. Unknown values resolve to | ||
| * the chat-model path at runtime. | ||
| */ | ||
| provider: z.string().optional(), |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Restrict provider to supported autocomplete transports.
Line 207 accepts "copilot", so the test at packages/types/src/__tests__/autocomplete.spec.ts Line 26 fails. This also permits unsupported provider IDs to enter the completion routing path.
Validate provider against the declared autocomplete transports and any required migration aliases. Keep a selected chat provider in chatFallbackProvider.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/types/src/autocomplete.ts` around lines 202 - 207, Update the
autocomplete schema’s provider field near the provider declaration to validate
values against the declared autocomplete transport IDs and required migration
aliases instead of accepting any string. Preserve the chat-provider selection
through chatFallbackProvider, and ensure unsupported IDs cannot enter completion
routing.
Comment on lines
+47
to
+55
| it("never splits a surrogate pair", () => { | ||
| // An orphaned half-pair is an invalid code unit that corrupts the prompt. | ||
| const text = "a".repeat(20) + "😀".repeat(10) | ||
| const head = trimToTokenBudget(text, 5, "head") | ||
| const tail = trimToTokenBudget(text, 5, "tail") | ||
|
|
||
| expect(head).toBe(Array.from(head).join("")) | ||
| expect(tail).toBe(Array.from(tail).join("")) | ||
| }) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
The surrogate assertions can never fail.
Array.from(s).join("") returns s for every string, including a string that holds a lone surrogate. Array.from iterates code points, and an unpaired surrogate is yielded as its own single-unit element. So both assertions compare a value to itself.
The chosen input also hides the defect. With budget 5 the limit is 18 characters, and 20 leading "a" characters place both slice boundaries on even offsets inside the emoji run, so no orphan appears. Use an odd boundary and assert on code units directly.
💚 Proposed test that detects an orphaned surrogate
it("never splits a surrogate pair", () => {
// An orphaned half-pair is an invalid code unit that corrupts the prompt.
- const text = "a".repeat(20) + "😀".repeat(10)
- const head = trimToTokenBudget(text, 5, "head")
- const tail = trimToTokenBudget(text, 5, "tail")
-
- expect(head).toBe(Array.from(head).join(""))
- expect(tail).toBe(Array.from(tail).join(""))
+ const isLoneSurrogate = (code: number) => code >= 0xd800 && code <= 0xdfff
+ const hasOrphan = (s: string) =>
+ isLoneSurrogate(s.charCodeAt(0)) || isLoneSurrogate(s.charCodeAt(s.length - 1))
+
+ // 21 leading "a" puts both slice boundaries on an odd offset inside the
+ // emoji run, which is exactly where a pair gets split.
+ const text = "a".repeat(21) + "😀".repeat(10)
+
+ for (const budget of [4, 5, 6, 7]) {
+ expect(hasOrphan(trimToTokenBudget(text, budget, "head"))).toBe(false)
+ expect(hasOrphan(trimToTokenBudget(text, budget, "tail"))).toBe(false)
+ }
})🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/services/autocomplete/__tests__/tokenBudget.spec.ts` around lines 47 -
55, Fix the surrogate-pair test around trimToTokenBudget by choosing input and
budget values that place head or tail boundaries at an odd code-unit offset
within the emoji run. Replace the self-comparison assertions with direct
code-unit validation that rejects lone high or low surrogates, while preserving
coverage of both "head" and "tail" results.
Comment on lines
+74
to
+85
| /** Builds the completion engine with the handler for the current provider. */ | ||
| private buildEngine(): CompletionEngine { | ||
| const handler = this.buildHandler() | ||
|
|
||
| return new CompletionEngine({ | ||
| getConfig: () => this.configService.getConfig(), | ||
| getApiKey: () => this.getApiKey(), | ||
| handler, | ||
| logger: this.logger, | ||
| contextGatherer: this.contextGatherer, | ||
| }) | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect how ZooInlineCompletionProvider stores and uses the engine, and who calls handleSettingsChange.
fd -t f 'ZooInlineCompletionProvider.ts' --exec ast-grep outline {} --items all
rg -n -C4 'handleSettingsChange|clearCache\(' --type=tsRepository: Zoo-Code-Org/Zoo-Code
Length of output: 1005
🏁 Script executed:
#!/bin/bash
set -u
echo "## Locate files"
fd -t f 'AutocompleteService\.ts|ZooInlineCompletionProvider\.ts|CompletionEngine\.ts' .
echo
echo "## AutocompleteService outline"
ast-grep outline src/services/autocomplete/AutocompleteService.ts --items all || true
echo
echo "## ZooInlineCompletionProvider outline"
ast-grep outline src/services/autocomplete/ZooInlineCompletionProvider.ts --items all || true
echo
echo "## relevant lines AutocompleteService"
cat -n src/services/autocomplete/AutocompleteService.ts | sed -n '50,200p'
echo
echo "## ZooInlineCompletionImplementation"
cat -n src/services/autocomplete/ZooInlineCompletionProvider.ts | sed -n '1,220p'
echo
echo "## search for engine references"
rg -n -C3 'engine|handleSettingsChange|clearCache|buildEngine|buildHandler' --type=ts src/services/autocompleteRepository: Zoo-Code-Org/Zoo-Code
Length of output: 38392
🏁 Script executed:
#!/bin/bash
set -u
echo "## CompletionEngine outline"
ast-grep outline src/services/autocomplete/CompletionEngine.ts --items all || true
echo
echo "## CompletionEngine implementation"
cat -n src/services/autocomplete/CompletionEngine.ts | sed -n '1,260p'
echo
echo "## CompletionEngine cache methods"
rg -n -C3 'class Complet|constructor|streamFim|cache|get.*Cache|set.*Cache|has.*Cache|provideInlineCompletionItems' src/services/autocomplete/CompletionEngine.tsRepository: Zoo-Code-Org/Zoo-Code
Length of output: 16763
Rebuild the inline completion engine when autocomplete settings change.
buildEngine creates the FIM handler once in the constructor, but handleSettingsChange only refreshes the status bar. ZooInlineCompletionProvider stores the engine in a private readonly field and calls that same engine for every completion after prefilters pass, so saved provider/base-url/model changes continue using the old handler. Move the engine to a field and recreate it in handleSettingsChange; store it by mutable reference/accessor if the provider must use the rebuilt engine without re-registering.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/services/autocomplete/AutocompleteService.ts` around lines 74 - 85, The
completion engine is built only once and becomes stale after autocomplete
settings change. Update AutocompleteService so the engine created by buildEngine
is stored in a mutable field or accessor, then rebuild and replace it in
handleSettingsChange; ensure ZooInlineCompletionProvider uses the current engine
reference without requiring provider re-registration.
Comment on lines
+291
to
+317
| private meetsMinCharsTyped( | ||
| document: vscode.TextDocument, | ||
| position: vscode.Position, | ||
| minCharsTyped: number, | ||
| ): boolean { | ||
| if (minCharsTyped <= 0) { | ||
| return true | ||
| } | ||
|
|
||
| const last = this.lastCompletion.get(document.uri.toString()) | ||
|
|
||
| if (!last || last.documentVersion !== document.version) { | ||
| return true | ||
| } | ||
|
|
||
| const typed = document.offsetAt(position) - last.offset | ||
|
|
||
| return typed >= minCharsTyped | ||
| } | ||
|
|
||
| private recordCompletion(document: vscode.TextDocument, position: vscode.Position, text: string): void { | ||
| this.lastCompletion.set(document.uri.toString(), { | ||
| documentVersion: document.version, | ||
| offset: document.offsetAt(position), | ||
| textLength: text.length, | ||
| }) | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
The minCharsTyped gate never blocks a request.
recordCompletion stores document.version at the time the suggestion is produced. Every keystroke increments document.version, so on the next request last.documentVersion !== document.version is true and meetsMinCharsTyped returns true before the offset comparison runs. The typed-character count is therefore never evaluated, and the setting has no effect.
Use the version only to detect that the recorded offset belongs to the same document state family, and compare offsets in the normal case.
🐛 Proposed fix: compare offsets instead of short-circuiting on version
const last = this.lastCompletion.get(document.uri.toString())
- if (!last || last.documentVersion !== document.version) {
+ if (!last) {
return true
}
+ // A newer document version is expected: the user typed since the last
+ // suggestion. What matters is how many characters were typed.
const typed = document.offsetAt(position) - last.offset
- return typed >= minCharsTyped
+ // The cursor moved backwards or to another region: treat it as a new context.
+ if (typed < 0) {
+ return true
+ }
+
+ return typed >= minCharsTyped
}Add a unit test in src/services/autocomplete/__tests__/CompletionEngine.spec.ts that sets minCharsTyped above zero, produces one completion, advances the document version, and asserts the second request returns undefined.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| private meetsMinCharsTyped( | |
| document: vscode.TextDocument, | |
| position: vscode.Position, | |
| minCharsTyped: number, | |
| ): boolean { | |
| if (minCharsTyped <= 0) { | |
| return true | |
| } | |
| const last = this.lastCompletion.get(document.uri.toString()) | |
| if (!last || last.documentVersion !== document.version) { | |
| return true | |
| } | |
| const typed = document.offsetAt(position) - last.offset | |
| return typed >= minCharsTyped | |
| } | |
| private recordCompletion(document: vscode.TextDocument, position: vscode.Position, text: string): void { | |
| this.lastCompletion.set(document.uri.toString(), { | |
| documentVersion: document.version, | |
| offset: document.offsetAt(position), | |
| textLength: text.length, | |
| }) | |
| } | |
| private meetsMinCharsTyped( | |
| document: vscode.TextDocument, | |
| position: vscode.Position, | |
| minCharsTyped: number, | |
| ): boolean { | |
| if (minCharsTyped <= 0) { | |
| return true | |
| } | |
| const last = this.lastCompletion.get(document.uri.toString()) | |
| if (!last) { | |
| return true | |
| } | |
| // A newer document version is expected: the user typed since the last | |
| // suggestion. What matters is how many characters were typed. | |
| const typed = document.offsetAt(position) - last.offset | |
| // The cursor moved backwards or to another region: treat it as a new context. | |
| if (typed < 0) { | |
| return true | |
| } | |
| return typed >= minCharsTyped | |
| } | |
| private recordCompletion(document: vscode.TextDocument, position: vscode.Position, text: string): void { | |
| this.lastCompletion.set(document.uri.toString(), { | |
| documentVersion: document.version, | |
| offset: document.offsetAt(position), | |
| textLength: text.length, | |
| }) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/services/autocomplete/CompletionEngine.ts` around lines 291 - 317, Update
meetsMinCharsTyped to evaluate the offset difference after a completion even
when document.version has advanced, using the stored version only to reject
stale or unrelated document state as appropriate; ensure the normal
post-keystroke path reaches the typed >= minCharsTyped check. Add a unit test in
CompletionEngine.spec.ts with minCharsTyped above zero that produces one
completion, advances the document version, and verifies the second request
returns undefined.
Comment on lines
+12
to
+13
| /** Largest document (bytes) the completion pipeline will consider. */ | ||
| export const MAX_DOCUMENT_BYTES = 1_048_576 |
Contributor
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
MAX_DOCUMENT_BYTES is compared against a character count, not bytes.
The consumer at src/services/autocomplete/ZooInlineCompletionProvider.ts:48-91 evaluates document.getText().length > MAX_DOCUMENT_BYTES. String.length returns UTF-16 code units, so a file of multi-byte characters passes the gate above 1 MiB of real bytes.
getText() also materializes the whole document on every inline-completion request. For a large file this copies up to 1 MiB per keystroke on the request path. Prefer a cheap proxy such as document.lineCount plus document.offsetAt(document.lineAt(document.lineCount - 1).range.end), and rename the constant to MAX_DOCUMENT_CHARS.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/services/autocomplete/constants.ts` around lines 12 - 13, Rename
MAX_DOCUMENT_BYTES to MAX_DOCUMENT_CHARS and update its documentation and all
references, including ZooInlineCompletionProvider. Replace the getText().length
size check with a document-size proxy using lineCount and the final line’s
offsetAt endpoint, preserving the existing threshold behavior without
materializing the full document.
Comment on lines
+59
to
+98
| // Identifies the endpoint so a change can retrigger the fetch, and so a stale | ||
| // in-flight response for a previous endpoint can be ignored. | ||
| const endpointKey = `${provider}|${baseUrl ?? ""}` | ||
| const requestedKey = useRef<string | undefined>(undefined) | ||
|
|
||
| const fetchModels = useCallback(() => { | ||
| if (disabled || needsKey) { | ||
| return | ||
| } | ||
|
|
||
| requestedKey.current = endpointKey | ||
| setStatus("loading") | ||
| setError(undefined) | ||
|
|
||
| vscode.postMessage({ | ||
| type: "requestAutocompleteModels", | ||
| values: { provider, baseUrl, ...(apiKeyDraft ? { apiKey: apiKeyDraft } : {}) }, | ||
| }) | ||
| }, [provider, baseUrl, apiKeyDraft, disabled, needsKey, endpointKey]) | ||
|
|
||
| const onMessage = useCallback((event: MessageEvent) => { | ||
| const message = event.data | ||
|
|
||
| if (message?.type !== "autocompleteModels") { | ||
| return | ||
| } | ||
|
|
||
| const payload = message.autocompleteModels as { models?: AutocompleteModelSummary[]; error?: string } | ||
|
|
||
| if (payload?.error) { | ||
| setStatus("error") | ||
| setError(payload.error) | ||
| setModels([]) | ||
| return | ||
| } | ||
|
|
||
| setStatus("connected") | ||
| setError(undefined) | ||
| setModels(payload?.models ?? []) | ||
| }, []) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect the requestAutocompleteModels handler and the autocompleteModels payload shape.
set -euo pipefail
rg -n -C 10 'requestAutocompleteModels|autocompleteModels' src packages/types --type=tsRepository: Zoo-Code-Org/Zoo-Code
Length of output: 8872
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the relevant picker code without running repo code.
fd -a 'AutocompleteModelPicker\.tsx$' . | sed 's#^\./##'
file="$(fd 'AutocompleteModelPicker\.tsx$' . | head -n 1)"
wc -l "$file"
sed -n '1,150p' "$file" | cat -n
# Inspect the extension-host handler around requestAutocompleteModels.
echo '--- handler slice ---'
sed -n '1372,1420p' src/core/webview/webviewMessageHandler.ts | cat -nRepository: Zoo-Code-Org/Zoo-Code
Length of output: 8819
Filter stale model-list responses before updating the picker.
onMessage accepts every autocompleteModels reply and writes models and status unconditionally. If the user changes provider or baseUrl while a request is in flight, an old response can populate options for a different endpoint. Echo a request identity in the payload and compare it in onMessage, or assign a request id and keep only the latest request before updating state.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@webview-ui/src/components/settings/autocomplete/AutocompleteModelPicker.tsx`
around lines 59 - 98, Update the autocomplete model request flow around
fetchModels and onMessage to associate each response with the endpointKey or
latest request identity. Include that identity in the request/response payload
contract, then ignore responses that do not match the current requested key
before changing status, error, or models; preserve handling for the active
request.
Comment on lines
+115
to
+125
| // A newly-entered key unblocks a fetch the gate above refused. Debounced | ||
| // longer, since a key is pasted or typed character by character. | ||
| useEffect(() => { | ||
| if (disabled || !isRemote || !hasCredential || models.length > 0 || status === "loading") { | ||
| return | ||
| } | ||
|
|
||
| const timer = setTimeout(fetchModels, 800) | ||
|
|
||
| return () => clearTimeout(timer) | ||
| }, [disabled, isRemote, hasCredential, models.length, status, fetchModels]) |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
This effect polls forever when the endpoint returns an empty model list.
The guard checks models.length > 0 and status === "loading". It does not check whether a fetch for the current endpoint already completed.
Loop: the effect schedules fetchModels; status becomes "loading"; the response arrives with zero models; onMessage sets status to "connected" and models to []. The dependency status changed, so the effect re-runs. models.length is still 0 and status is no longer "loading", so every guard passes again and a new 800 ms timer starts. The component then re-requests the model list every 800 ms for as long as the settings panel stays open.
An endpoint that returns an empty list is normal — a fresh Ollama install with no tags pulled, or a server whose models API returns [].
Gate on the already-requested endpoint key instead of on the result.
🐛 Proposed fix
useEffect(() => {
- if (disabled || !isRemote || !hasCredential || models.length > 0 || status === "loading") {
+ if (disabled || !isRemote || !hasCredential || requestedKey.current === endpointKey) {
return
}
const timer = setTimeout(fetchModels, 800)
return () => clearTimeout(timer)
- }, [disabled, isRemote, hasCredential, models.length, status, fetchModels])
+ }, [disabled, isRemote, hasCredential, endpointKey, fetchModels])
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@webview-ui/src/components/settings/autocomplete/AutocompleteModelPicker.tsx`
around lines 115 - 125, Update the debounced fetch effect around fetchModels to
track the endpoint key already requested for the current endpoint and return
early when that key has completed, rather than using models.length as the
completion signal. Preserve fetching for a newly entered or changed endpoint,
including empty model-list responses, while preventing repeated requests after
status returns to connected.
Comment on lines
+379
to
+391
| /** | ||
| * Write-only buffer for the autocomplete API key. | ||
| * | ||
| * The stored key is never sent to the webview (only `hasAutocompleteApiKey`), so there is | ||
| * nothing to hydrate from. `undefined` means "untouched" and is omitted from the save | ||
| * payload, which keeps an existing key intact when the user edits unrelated fields. | ||
| */ | ||
| const [autocompleteApiKeyDraft, setAutocompleteApiKeyDraft] = useState<string | undefined>(undefined) | ||
|
|
||
| const setAutocompleteApiKey = useCallback((apiKey: string) => { | ||
| setAutocompleteApiKeyDraft(apiKey) | ||
| setChangeDetected(true) | ||
| }, []) |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Discarding changes does not clear the API-key draft, so a discarded key can still be saved.
autocompleteApiKeyDraft lives in its own useState outside cachedState. onConfirmDialogResult (lines 604-615) reverts cachedState and clears isChangeDetected, but it leaves the draft intact.
Failure sequence:
- The user types an autocomplete API key.
- The user clicks Done or switches tab, then confirms "Discard".
cachedStatereverts.autocompleteApiKeyDraftstill holds the typed key, and the field still displays it.- The user makes any unrelated edit and clicks Save.
- Line 574 sees
autocompleteApiKeyDraft !== undefinedand sendsautocompleteApiKey, overwriting the stored secret with the key the user discarded.
Clear the draft in the discard path. Consider adding a regression test in webview-ui/src/components/settings/__tests__/SettingsView.autocomplete.spec.tsx, alongside the existing draft-consumption test at line 540.
🐛 Proposed fix (applies at lines 604-615)
const onConfirmDialogResult = useCallback(
(confirm: boolean) => {
if (confirm) {
// Discard changes: Reset state and flag
setCachedState(extensionState) // Revert to original state
+ setAutocompleteApiKeyDraft(undefined) // Drop the unsaved secret with the rest of the edits
setChangeDetected(false) // Reset change flag
confirmDialogHandler.current?.() // Execute the pending action (e.g., tab switch)
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@webview-ui/src/components/settings/SettingsView.tsx` around lines 379 - 391,
Clear autocompleteApiKeyDraft in the discard branch of onConfirmDialogResult
alongside restoring cachedState and resetting isChangeDetected, so discarded
keys are removed from the field and omitted from later save payloads. Add a
regression test in the existing SettingsView.autocomplete.spec.tsx
draft-consumption tests covering discard followed by an unrelated edit and save.
Comment on lines
+570
to
+574
| autocompleteConfig, | ||
| autocompleteProfiles, | ||
| activeAutocompleteProfileId, | ||
| // Omitted when untouched so an existing stored key survives unrelated edits. | ||
| ...(autocompleteApiKeyDraft !== undefined ? { autocompleteApiKey: autocompleteApiKeyDraft } : {}), |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check how activeAutocompleteProfileId is typed and persisted.
set -euo pipefail
rg -n -C 6 'activeAutocompleteProfileId' src packages/types --type=tsRepository: Zoo-Code-Org/Zoo-Code
Length of output: 5868
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## Candidate files"
git ls-files | rg '(^|/)SettingsView\.tsx$|packages/types/src/global-settings\.ts$|ClineProvider\.ts$' || true
echo
echo "## SettingsView relevant lines"
sed -n '430,590p' webview-ui/src/components/settings/SettingsView.tsx
echo
echo "## ClineProvider updateSettings relevant lines"
sed -n '2610,2730p' src/core/webview/ClineProvider.ts
sed -n '2840,2925p' src/core/webview/ClineProvider.ts
echo
echo "## Schema / type definitions"
sed -n '1,80p' webview-ui/src/components/settings/SettingsView.tsx | rg -n "autocompleteProfileSchema|autocompleteConfigSchema|activeAutocompleteProfileId|allowedMaxRequests|terminalProfile|z\." -C 3 || true
sed -n '180,245p' packages/types/src/global-settings.tsRepository: Zoo-Code-Org/Zoo-Code
Length of output: 20315
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## updateSettings message handling in ClineProvider"
rg -n -C 12 'updateSettings|activeAutocompleteProfileId|resolveSettings|merge' src/core/webview/ClineProvider.ts packages --type=ts
echo
echo "## JavaScript undefined/null serialization and object merge behavior"
node - <<'JS'
const payload = {
activeAutocompleteProfileId: undefined,
autocompleteProfiles: [{ id: 'a', name: 'x' }],
allowedMaxRequests: undefined,
terminalProfile: undefined,
}
console.log(JSON.stringify(payload))
console.log(JSON.parse(JSON.stringify(payload)))
const merged = { activeAutocompleteProfileId: 'a', autocompleteProfiles: [] }
for (const [k, v] of Object.entries(payload)) merged[k] = v // simple merge model
console.log(merged)
const nullPayload = JSON.parse(JSON.stringify({ activeAutocompleteProfileId: null }))
const mergedNull = { activeAutocompleteProfileId: 'a', autocompleteProfiles: [] }
for (const [k, v] of Object.entries(nullPayload)) mergedNull[k] = v
console.log(mergedNull)
JSRepository: Zoo-Code-Org/Zoo-Code
Length of output: 50377
Send a clearing value for activeAutocompleteProfileId.
deleteAutocompleteProfile sets it to undefined, but JSON.stringify omits that property from the updateSettings payload. The extension host then reads back the previous active profile id and keeps the deleted profile marked active. Send null here, like allowedMaxRequests, and update its schema/type to accept null.
🐛 Proposed fix
autocompleteConfig,
autocompleteProfiles,
- activeAutocompleteProfileId,
+ // `null` clears the id; `undefined` would be dropped by JSON.stringify.
+ activeAutocompleteProfileId: activeAutocompleteProfileId ?? null,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| autocompleteConfig, | |
| autocompleteProfiles, | |
| activeAutocompleteProfileId, | |
| // Omitted when untouched so an existing stored key survives unrelated edits. | |
| ...(autocompleteApiKeyDraft !== undefined ? { autocompleteApiKey: autocompleteApiKeyDraft } : {}), | |
| autocompleteConfig, | |
| autocompleteProfiles, | |
| // `null` clears the id; `undefined` would be dropped by JSON.stringify. | |
| activeAutocompleteProfileId: activeAutocompleteProfileId ?? null, | |
| // Omitted when untouched so an existing stored key survives unrelated edits. | |
| ...(autocompleteApiKeyDraft !== undefined ? { autocompleteApiKey: autocompleteApiKeyDraft } : {}), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@webview-ui/src/components/settings/SettingsView.tsx` around lines 570 - 574,
Update the settings payload construction to send null rather than omit
activeAutocompleteProfileId when deleteAutocompleteProfile clears it, preserving
existing values for untouched edits. Extend the corresponding settings schema
and TypeScript type to accept null, matching the established allowedMaxRequests
pattern.
Comment on lines
+79
to
+138
| "nativeFimHint": "Talks to a fill-in-the-middle endpoint directly — the fastest and most accurate option." | ||
| }, | ||
| "baseUrl": { | ||
| "label": "बेस URL" | ||
| }, | ||
| "model": { | ||
| "label": "मॉडल", | ||
| "description": "qwen2.5-coder:1.5b-base जैसा fill-in-the-middle बेस मॉडल बेहतर है। instruction-tuned chat मॉडल इस काम में कमज़ोर रहते हैं, भले ही वे कहीं बड़े हों।", | ||
| "sectionTitle": "Completion model", | ||
| "sectionDescription": "Which model produces the suggestions, and how to reach it.", | ||
| "selectPlaceholder": "Select or type a model id", | ||
| "customPlaceholder": "Or type a model id not in the list", | ||
| "refresh": "Refresh model list", | ||
| "fetchFailed": "Could not reach the endpoint: {{error}}" | ||
| }, | ||
| "apiKey": { | ||
| "label": "API कुंजी", | ||
| "description": "VS Code के Secret Storage में सहेजी जाती है और निर्यात की गई सेटिंग्स में कभी शामिल नहीं होती।", | ||
| "storedPlaceholder": "एक कुंजी सहेजी गई है। बदलने के लिए टाइप करें।", | ||
| "emptyPlaceholder": "अपनी API कुंजी दर्ज करें", | ||
| "optionalLabel": "API key (optional)", | ||
| "localDescription": "Leave empty for local servers. Required for hosted endpoints such as ollama.com or Codestral." | ||
| }, | ||
| "profiles": { | ||
| "label": "Saved setups", | ||
| "description": "Save a provider, model and tuning combination so you can switch between, say, a fast local model and a stronger cloud one.", | ||
| "none": "Not using a saved setup", | ||
| "namePlaceholder": "e.g. Local Qwen, Cloud Codestral", | ||
| "save": "Save current settings as a setup", | ||
| "saveLabel": "Name this setup", | ||
| "rename": "Rename setup", | ||
| "renameLabel": "Rename setup", | ||
| "delete": "Delete setup", | ||
| "confirm": "Confirm", | ||
| "cancel": "Cancel", | ||
| "limitReached": "You have reached the maximum number of saved setups.", | ||
| "unsavedChanges": "Changed since “{{name}}” was saved. Save the setup again to keep these values." | ||
| }, | ||
| "behavior": { | ||
| "sectionTitle": "Behavior", | ||
| "sectionDescription": "When suggestions appear while you type." | ||
| }, | ||
| "advanced": { | ||
| "label": "Advanced settings" | ||
| }, | ||
| "maxOutputTokens": { | ||
| "label": "Maximum suggestion length", | ||
| "description": "Upper bound on a single suggestion. Lower values return faster and rarely cut off useful code.", | ||
| "clamped": "सहेजा गया सेटअप {{stored}} रखता है; {{max}} की अधिकतम सीमा दिखाई जा रही है। सीमा के भीतर मान सहेजने के लिए स्लाइडर हिलाएँ।" | ||
| }, | ||
| "connection": { | ||
| "needsKey": "Enter an API key to load the available models from this endpoint.", | ||
| "checking": "Connecting…", | ||
| "connected": "Connected — {{count}} model(s) available.", | ||
| "connectedLocal": "Connected." | ||
| }, | ||
| "chatProvider": { | ||
| "label": "Chat provider", | ||
| "placeholder": "Select a provider", | ||
| "description": "Which chat provider serves completions. Uses the same providers as the Providers tab; configure its credentials there." |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Complete the autocomplete translations.
These locale files retain English text for model selection, profiles, advanced settings, connection states, and chat-provider controls. This leaves much of the new settings page untranslated for Hindi, Indonesian, and Dutch users.
webview-ui/src/i18n/locales/hi/settings.json#L79-L138: translate the remaining English autocomplete strings.webview-ui/src/i18n/locales/id/settings.json#L79-L138: translate the remaining English autocomplete strings.webview-ui/src/i18n/locales/nl/settings.json#L79-L138: translate the remaining English autocomplete strings.
📍 Affects 3 files
webview-ui/src/i18n/locales/hi/settings.json#L79-L138(this comment)webview-ui/src/i18n/locales/id/settings.json#L79-L138webview-ui/src/i18n/locales/nl/settings.json#L79-L138
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@webview-ui/src/i18n/locales/hi/settings.json` around lines 79 - 138,
Translate all remaining English autocomplete settings strings in
webview-ui/src/i18n/locales/hi/settings.json lines 79-138,
webview-ui/src/i18n/locales/id/settings.json lines 79-138, and
webview-ui/src/i18n/locales/nl/settings.json lines 79-138, covering the model,
profiles, advanced, connection, and chatProvider sections while preserving keys
and interpolation placeholders.
The ClineProvider default-config test hand-copied all 22 autocomplete defaults, so it failed on any tuning change while never checking the thing it exists to check — that an unset config reaches the webview fully defaulted. It now compares against resolveAutocompleteConfig directly. Four values had drifted: maxSuffixTokens (this branch), plus maxOutputTokens, requestTimeoutMs and temperature, which were already stale.
github-actions
Bot
added
the
awaiting-review
Patch coverage sat at 62.86% against an 80% target because several files shipped with no test file at all. Adds specs for the ones carrying the most uncovered lines: - streamReaders: NDJSON/SSE/text framing, split multi-byte characters, malformed lines, and the abort-vs-error distinction (19.75% -> 98.76%) - AutocompleteService: registration, the workspace kill switch vs a plain disable, toggle persistence and disposal (0% -> covered) - AutocompleteModelPicker: message handling, debounced auto-fetch, and the dropdown/free-text branches (48.93% -> 91.48%) - AutocompleteLogger, AutocompleteStatusBar, OpenTabsSource: previously untested entirely - AutocompleteSettings: the behaviour sliders and profile dirty-detection, including array-valued fields compared by value No production code changed.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/services/autocomplete/__tests__/AutocompleteStatusBar.spec.ts (1)
25-25: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse precise typed test doubles.
These fixtures erase contract mismatches with broad assertions. This can let VS Code and autocomplete interface changes bypass test compilation.
src/services/autocomplete/__tests__/AutocompleteStatusBar.spec.ts#L25-L25: replace the double assertion with a narrow typed service fixture, or document why it is necessary.src/services/autocomplete/__tests__/AutocompleteStatusBar.spec.ts#L64-L73: use the same typed service fixture for the mutable enabled-state test.src/services/autocomplete/__tests__/OpenTabsSource.spec.ts#L42-L55: construct typedSnippetSourceInputand resolved-config fixtures instead of asserting incomplete objects.src/services/autocomplete/__tests__/AutocompleteService.spec.ts#L70-L93: provide a typed extension-context fixture, or document the required double assertion.src/services/autocomplete/__tests__/AutocompleteLogger.spec.ts#L7-L14: typecreateOutputChannelwith its name parameter and assert the"Zoo Code Autocomplete"argument.Please also run file-scoped ESLint with zero warnings after this change and confirm that no new suppressions exist.
As per coding guidelines,
src/**/*.{ts,tsx}says: “Avoidas any; use typed APIs, bracket notation for private members, or precise test doubles and type guards. Use double assertions only as a last resort and document them.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/autocomplete/__tests__/AutocompleteStatusBar.spec.ts` at line 25, Replace broad test-double assertions with precise typed fixtures across src/services/autocomplete/__tests__/AutocompleteStatusBar.spec.ts:25-25 and :64-73, using the same narrow service fixture for mutable enabled state; construct typed SnippetSourceInput and resolved-config fixtures in src/services/autocomplete/__tests__/OpenTabsSource.spec.ts:42-55; provide a typed extension-context fixture or document the necessary double assertion in src/services/autocomplete/__tests__/AutocompleteService.spec.ts:70-93; and type createOutputChannel’s name parameter while asserting "Zoo Code Autocomplete" in src/services/autocomplete/__tests__/AutocompleteLogger.spec.ts:7-14. Avoid new suppressions, document any unavoidable double assertions, and run file-scoped ESLint with zero warnings.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/services/autocomplete/__tests__/streamReaders.spec.ts`:
- Around line 124-183: Preserve a valid final SSE event when the stream ends
without a blank-line delimiter. Add a readSse regression test using
streamOf("data: final") expecting the parsed event, then update readSse’s EOF
handling to yield the event returned by flushSse(buffer) when the final buffer
contains data.
---
Nitpick comments:
In `@src/services/autocomplete/__tests__/AutocompleteStatusBar.spec.ts`:
- Line 25: Replace broad test-double assertions with precise typed fixtures
across src/services/autocomplete/__tests__/AutocompleteStatusBar.spec.ts:25-25
and :64-73, using the same narrow service fixture for mutable enabled state;
construct typed SnippetSourceInput and resolved-config fixtures in
src/services/autocomplete/__tests__/OpenTabsSource.spec.ts:42-55; provide a
typed extension-context fixture or document the necessary double assertion in
src/services/autocomplete/__tests__/AutocompleteService.spec.ts:70-93; and type
createOutputChannel’s name parameter while asserting "Zoo Code Autocomplete" in
src/services/autocomplete/__tests__/AutocompleteLogger.spec.ts:7-14. Avoid new
suppressions, document any unavoidable double assertions, and run file-scoped
ESLint with zero warnings.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b1fdd4e7-0557-4c4c-aae9-bb0c233ee4e6
📒 Files selected for processing (7)
src/services/autocomplete/__tests__/AutocompleteLogger.spec.tssrc/services/autocomplete/__tests__/AutocompleteService.spec.tssrc/services/autocomplete/__tests__/AutocompleteStatusBar.spec.tssrc/services/autocomplete/__tests__/OpenTabsSource.spec.tssrc/services/autocomplete/__tests__/streamReaders.spec.tswebview-ui/src/components/settings/__tests__/AutocompleteSettings.spec.tsxwebview-ui/src/components/settings/autocomplete/__tests__/AutocompleteModelPicker.spec.tsx
Comment on lines
+124
to
+183
| describe("readSse", () => { | ||
| it("yields the data field of each event block", async () => { | ||
| const values = await collect(readSse(streamOf("data: one\n\ndata: two\n\n"), new AbortController().signal)) | ||
|
|
||
| expect(values).toEqual([ | ||
| { event: undefined, data: "one" }, | ||
| { event: undefined, data: "two" }, | ||
| ]) | ||
| }) | ||
|
|
||
| it("captures the event name alongside the data", async () => { | ||
| const values = await collect(readSse(streamOf("event: delta\ndata: hi\n\n"), new AbortController().signal)) | ||
|
|
||
| expect(values).toEqual([{ event: "delta", data: "hi" }]) | ||
| }) | ||
|
|
||
| it("concatenates repeated data fields with a newline", async () => { | ||
| const values = await collect(readSse(streamOf("data: a\ndata: b\n\n"), new AbortController().signal)) | ||
|
|
||
| expect(values).toEqual([{ event: undefined, data: "a\nb" }]) | ||
| }) | ||
|
|
||
| it("ignores comment lines and blocks with no data field", async () => { | ||
| const values = await collect( | ||
| readSse(streamOf(": keep-alive\n\nevent: ping\n\ndata: real\n\n"), new AbortController().signal), | ||
| ) | ||
|
|
||
| expect(values).toEqual([{ event: undefined, data: "real" }]) | ||
| }) | ||
|
|
||
| it("strips only a single leading space from the value", async () => { | ||
| const values = await collect(readSse(streamOf("data: padded\n\n"), new AbortController().signal)) | ||
|
|
||
| expect(values).toEqual([{ event: undefined, data: " padded" }]) | ||
| }) | ||
|
|
||
| it("skips a field line with no colon", async () => { | ||
| const values = await collect(readSse(streamOf("garbage\ndata: ok\n\n"), new AbortController().signal)) | ||
|
|
||
| expect(values).toEqual([{ event: undefined, data: "ok" }]) | ||
| }) | ||
|
|
||
| it("joins a block split across chunk boundaries", async () => { | ||
| const values = await collect(readSse(streamOf("data: sp", "lit\n\n"), new AbortController().signal)) | ||
|
|
||
| expect(values).toEqual([{ event: undefined, data: "split" }]) | ||
| }) | ||
|
|
||
| it("returns silently when the stream aborts", async () => { | ||
| const values = await collect(readSse(failingStream(abortError()), new AbortController().signal)) | ||
|
|
||
| expect(values).toEqual([]) | ||
| }) | ||
|
|
||
| it("rethrows a non-abort error", async () => { | ||
| await expect(collect(readSse(failingStream(new Error("boom")), new AbortController().signal))).rejects.toThrow( | ||
| "boom", | ||
| ) | ||
| }) | ||
| }) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve the final SSE event.
A stream can end after a valid data: block without a final blank-line delimiter. readSse calls flushSse(buffer) at EOF in src/services/autocomplete/stream/streamReaders.ts:109-151, but it does not yield a parsed event. This test suite does not detect the resulting lost completion.
Add a test for streamOf("data: final"). Then update readSse to parse and yield the final buffered event.
Proposed regression test
describe("readSse", () => {
+ it("flushes a final event without a blank-line delimiter", async () => {
+ const values = await collect(readSse(streamOf("data: final"), new AbortController().signal))
+
+ expect(values).toEqual([{ event: undefined, data: "final" }])
+ })
+
it("yields the data field of each event block", async () => {As per coding guidelines, **/*.{test,spec}.{ts,tsx,js,jsx} requires a regression test at the lowest layer that would have failed.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| describe("readSse", () => { | |
| it("yields the data field of each event block", async () => { | |
| const values = await collect(readSse(streamOf("data: one\n\ndata: two\n\n"), new AbortController().signal)) | |
| expect(values).toEqual([ | |
| { event: undefined, data: "one" }, | |
| { event: undefined, data: "two" }, | |
| ]) | |
| }) | |
| it("captures the event name alongside the data", async () => { | |
| const values = await collect(readSse(streamOf("event: delta\ndata: hi\n\n"), new AbortController().signal)) | |
| expect(values).toEqual([{ event: "delta", data: "hi" }]) | |
| }) | |
| it("concatenates repeated data fields with a newline", async () => { | |
| const values = await collect(readSse(streamOf("data: a\ndata: b\n\n"), new AbortController().signal)) | |
| expect(values).toEqual([{ event: undefined, data: "a\nb" }]) | |
| }) | |
| it("ignores comment lines and blocks with no data field", async () => { | |
| const values = await collect( | |
| readSse(streamOf(": keep-alive\n\nevent: ping\n\ndata: real\n\n"), new AbortController().signal), | |
| ) | |
| expect(values).toEqual([{ event: undefined, data: "real" }]) | |
| }) | |
| it("strips only a single leading space from the value", async () => { | |
| const values = await collect(readSse(streamOf("data: padded\n\n"), new AbortController().signal)) | |
| expect(values).toEqual([{ event: undefined, data: " padded" }]) | |
| }) | |
| it("skips a field line with no colon", async () => { | |
| const values = await collect(readSse(streamOf("garbage\ndata: ok\n\n"), new AbortController().signal)) | |
| expect(values).toEqual([{ event: undefined, data: "ok" }]) | |
| }) | |
| it("joins a block split across chunk boundaries", async () => { | |
| const values = await collect(readSse(streamOf("data: sp", "lit\n\n"), new AbortController().signal)) | |
| expect(values).toEqual([{ event: undefined, data: "split" }]) | |
| }) | |
| it("returns silently when the stream aborts", async () => { | |
| const values = await collect(readSse(failingStream(abortError()), new AbortController().signal)) | |
| expect(values).toEqual([]) | |
| }) | |
| it("rethrows a non-abort error", async () => { | |
| await expect(collect(readSse(failingStream(new Error("boom")), new AbortController().signal))).rejects.toThrow( | |
| "boom", | |
| ) | |
| }) | |
| }) | |
| describe("readSse", () => { | |
| it("flushes a final event without a blank-line delimiter", async () => { | |
| const values = await collect(readSse(streamOf("data: final"), new AbortController().signal)) | |
| expect(values).toEqual([{ event: undefined, data: "final" }]) | |
| }) | |
| it("yields the data field of each event block", async () => { | |
| const values = await collect(readSse(streamOf("data: one\n\ndata: two\n\n"), new AbortController().signal)) | |
| expect(values).toEqual([ | |
| { event: undefined, data: "one" }, | |
| { event: undefined, data: "two" }, | |
| ]) | |
| }) | |
| it("captures the event name alongside the data", async () => { | |
| const values = await collect(readSse(streamOf("event: delta\ndata: hi\n\n"), new AbortController().signal)) | |
| expect(values).toEqual([{ event: "delta", data: "hi" }]) | |
| }) | |
| it("concatenates repeated data fields with a newline", async () => { | |
| const values = await collect(readSse(streamOf("data: a\ndata: b\n\n"), new AbortController().signal)) | |
| expect(values).toEqual([{ event: undefined, data: "a\nb" }]) | |
| }) | |
| it("ignores comment lines and blocks with no data field", async () => { | |
| const values = await collect( | |
| readSse(streamOf(": keep-alive\n\nevent: ping\n\ndata: real\n\n"), new AbortController().signal), | |
| ) | |
| expect(values).toEqual([{ event: undefined, data: "real" }]) | |
| }) | |
| it("strips only a single leading space from the value", async () => { | |
| const values = await collect(readSse(streamOf("data: padded\n\n"), new AbortController().signal)) | |
| expect(values).toEqual([{ event: undefined, data: " padded" }]) | |
| }) | |
| it("skips a field line with no colon", async () => { | |
| const values = await collect(readSse(streamOf("garbage\ndata: ok\n\n"), new AbortController().signal)) | |
| expect(values).toEqual([{ event: undefined, data: "ok" }]) | |
| }) | |
| it("joins a block split across chunk boundaries", async () => { | |
| const values = await collect(readSse(streamOf("data: sp", "lit\n\n"), new AbortController().signal)) | |
| expect(values).toEqual([{ event: undefined, data: "split" }]) | |
| }) | |
| it("returns silently when the stream aborts", async () => { | |
| const values = await collect(readSse(failingStream(abortError()), new AbortController().signal)) | |
| expect(values).toEqual([]) | |
| }) | |
| it("rethrows a non-abort error", async () => { | |
| await expect(collect(readSse(failingStream(new Error("boom")), new AbortController().signal))).rejects.toThrow( | |
| "boom", | |
| ) | |
| }) | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/services/autocomplete/__tests__/streamReaders.spec.ts` around lines 124 -
183, Preserve a valid final SSE event when the stream ends without a blank-line
delimiter. Add a readSse regression test using streamOf("data: final") expecting
the parsed event, then update readSse’s EOF handling to yield the event returned
by flushSse(buffer) when the final buffer contains data.
Source: Coding guidelines
Patch coverage reached 77.48% against the 80% target; the two FIM handlers carried the largest remaining gaps at roughly 66% each. - OllamaFimHandler: listModels capability filtering, schema rejection, and validate's found/not-found/unreachable outcomes - OpenAiCompatibleFimHandler: the chat-endpoint path (system message framing, the dropped code-fence stop, abort and 401 handling) and base URL normalization for a user-supplied /v1 or bare host Both now sit at 91.66% lines. No production code changed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related GitHub Issue
Closes: #171
Description
Adds inline ghost-text autocomplete, running on its own model separately from the chat model — so you can keep a large cloud model for agentic work and point completions at something small and local.
This covers what the issue asked for: Ollama support, a separate model picker for autocomplete, FIM prompting, a global toggle, and Tab to accept.
How it's put together
AutocompleteServiceowns anInlineCompletionItemProvider, with debounce, aminCharsTypedgate, and a large-file guard so we're not streaming huge documents on every keystroke.PromptBuilderwindows the document around the cursor into a prefix and suffix, andFimTemplateRegistryresolves the control-token format from the model id (Qwen, StarCoder, Codestral, CodeLlama, DeepSeek, CodeGemma)./api/generatewithsuffix), OpenAI-compatible (/v1/completionswithsuffix— LM Studio, llama.cpp, vLLM), and Codestral. llama.cpp rejectssuffixon some builds, so that's caught and retried with a rendered prompt, memoised per base URL.A note on the provider list
The dropdown only offers the three FIM transports. An earlier revision listed every provider the extension supports and prompted the non-FIM ones as chat models, but that gave you a list of thirty entries where three worked well and the rest quietly underperformed — including three near-identical "OpenAI" rows that differed in ways you couldn't see at the point of choosing. Restricting it made the chat-model warning unnecessary, so that's gone too.
Worth flagging against the issue text: I mention Gemma 4 e4b/e2b on the issue i opened a while back (May 18th), and instruction-tuned models genuinely are weak at this task regardless of size, because they have no FIM tokens and can't use the code after the cursor. For local use
qwen2.5-coder:1.5b-baseis the one I'd reach for.Test Procedure
Unit tests: 204 in
src/services/autocomplete, 19 in the settings specs. Both suites pass on the currentmain.Manually, in the Extension Development Host against Ollama:
qwen2.5-coder:1.5b-base.For the FIM fixes specifically,
codestral:22b-instructandqwen2.5-coder:7b-instructshould now go down the native FIM path instead of the chat path.Pre-Submission Checklist
Summary by CodeRabbit
New Features
Tests