Skip to content

Conversation

markijbema
Copy link
Contributor

@markijbema markijbema commented Oct 10, 2025

we currently only support codestral, and have only configured this for 3 providers. It currently fails silently if these arent available, make more clear what is happening

Fixes: #2907

CleanShot 2025-10-17 at 10 32 10@2x

Copy link

changeset-bot bot commented Oct 10, 2025

⚠️ No Changeset found

Latest commit: c5711a1

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

</>
) : (
<div className="text-vscode-errorForeground">
No suitable autocomplete model found. Please configure a provider in the API
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

todo translate

@markijbema markijbema force-pushed the mark/supported-autocomplete-providers branch from e326302 to 5dcd224 Compare October 16, 2025 12:58
@markijbema markijbema force-pushed the mark/supported-autocomplete-providers branch from 5dcd224 to b6a9d20 Compare October 16, 2025 14:17
@@ -0,0 +1,325 @@
import { describe, it, expect, vi, beforeEach } from "vitest"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is all vibed. I have the impression this helps guide the LLM a lot, if anyone feels it would be better to remove thats fine by me as well

@markijbema markijbema force-pushed the mark/supported-autocomplete-providers branch from 64b381c to 2f43209 Compare October 16, 2025 15:28
@markijbema markijbema force-pushed the mark/supported-autocomplete-providers branch from ed67182 to a41b712 Compare October 17, 2025 08:43
@markijbema markijbema requested a review from Copilot October 17, 2025 08:45
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Adds provider/model visibility and clearer error messaging when no autocomplete provider/model is configured, plus logic to select a usable provider and expose it in the status bar and settings UI.

  • Introduces provider/model fields to Ghost service settings and displays them in UI with error fallback.
  • Refactors GhostModel reload logic to iterate supported providers and adds balance check for kilocode; updates status bar tooltip and global context.
  • Adds tests for GhostModel provider usability scenarios and new i18n keys across locales.

Reviewed Changes

Copilot reviewed 54 out of 54 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
webview-ui/src/i18n/locales/*/kilocode.json Adds new translation keys for provider/model and noModelConfigured message.
webview-ui/src/components/kilocode/settings/GhostServiceSettings.tsx Displays provider/model or error message when missing.
webview-ui/src/components/kilocode/settings/tests/GhostServiceSettings.spec.tsx Tests UI rendering of provider/model and error message cases.
src/services/ghost/GhostModel.ts Refactors provider selection logic, adds provider display name accessor.
src/services/ghost/tests/GhostModel.spec.ts Adds tests for provider usability (balance, token presence) and provider display name.
src/services/ghost/GhostProvider.ts Integrates new model/provider info into context and status bar.
src/services/ghost/GhostStatusBar.ts Adds provider to status bar tooltip and state.
src/i18n/locales/*/kilocode.json Adds provider key for status bar tooltip in all locales.
src/core/webview/webviewMessageHandler.ts Triggers ghost reload after API provider settings change.
src/core/webview/tests/webviewMessageHandler.autoSwitch.spec.ts Mocks vscode.commands to support reload invocation.
packages/types/src/provider-settings.ts Defines autocomplete provider models and usability checker.
packages/types/src/kilocode/kilocode.ts Extends schema with provider/model and adds balance check helper.

const isUsable = await defaultProviderUsabilityChecker(provider, providerSettingsManager)
if (!isUsable) continue

this.loadProfile(providerSettingsManager, selectedProfile, provider)
Copy link

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

loadProfile is async and not awaited; this.loaded may be set and true returned before profile/model info is actually loaded or fetchModel completes. Add await before this.loadProfile to avoid race conditions.

Suggested change
this.loadProfile(providerSettingsManager, selectedProfile, provider)
await this.loadProfile(providerSettingsManager, selectedProfile, provider)

Copilot uses AI. Check for mistakes.

if (url.includes("/api/profile/balance")) {
return {
ok: true,
json: async () => ({ data: { balance: 0 } }),
Copy link

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checkKilocodeBalance expects a top-level balance property (data.balance ?? 0); this mock returns nested { data: { balance: 0 } } causing balance to be treated as 0 even if intended otherwise. Return { balance: 0 } instead to align with the production function.

Suggested change
json: async () => ({ data: { balance: 0 } }),
json: async () => ({ balance: 0 }),

Copilot uses AI. Check for mistakes.

if (url.includes("/api/profile/balance")) {
return {
ok: true,
json: async () => ({ data: { balance: 10.5 } }),
Copy link

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Positive balance mock also uses nested { data: { balance: 10.5 } } so checkKilocodeBalance interprets balance as 0; change to { balance: 10.5 } to exercise the intended path of selecting kilocode.

Suggested change
json: async () => ({ data: { balance: 10.5 } }),
json: async () => ({ balance: 10.5 }),

Copilot uses AI. Check for mistakes.

await this.model.reload(this.providerSettingsManager)
this.cursorAnimation.updateSettings(this.settings || undefined)
await this.updateGlobalContext()
this.updateStatusBar()
Copy link

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Because GhostModel.reload does not await loadProfile (see earlier issue), updateGlobalContext and updateStatusBar may run before provider/model data is ready. After fixing reload, consider moving updateStatusBar into updateGlobalContext or adding explicit sequencing comments for clarity.

Suggested change
this.updateStatusBar()

Copilot uses AI. Check for mistakes.

Comment on lines 185 to 211
export const defaultProviderUsabilityChecker: ProviderUsabilityChecker = async (provider, providerSettingsManager) => {
if (provider === "kilocode") {
try {
const profiles = await providerSettingsManager.listConfig()
const kilocodeProfile = profiles.find((p) => p.apiProvider === "kilocode")

if (!kilocodeProfile) {
return false
}

const profile = await providerSettingsManager.getProfile({ id: kilocodeProfile.id })
const kilocodeToken = profile.kilocodeToken

if (!kilocodeToken) {
return false
}

return await checkKilocodeBalance(kilocodeToken)
} catch (error) {
console.error("Error checking kilocode balance:", error)
return false
}
}

// For all other providers, assume they are usable
return true
}
Copy link

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

listConfig is called here after already being invoked in GhostModel.reload; pass the profiles array into defaultProviderUsabilityChecker to avoid duplicate I/O, reducing latency when many providers are added.

Copilot uses AI. Check for mistakes.

Comment on lines 167 to 185
export const AUTOCOMPLETE_PROVIDER_MODELS = {
mistral: "codestral-latest",
kilocode: "mistralai/codestral-2508",
openrouter: "mistralai/codestral-2508",
} as const satisfies Record<Extract<ProviderName, "mistral" | "kilocode" | "openrouter">, string>

export type AutocompleteProviderKey = keyof typeof AUTOCOMPLETE_PROVIDER_MODELS

interface ProviderSettingsManager {
listConfig(): Promise<ProviderSettingsEntry[]>
getProfile(params: { id: string }): Promise<ProviderSettings>
}

export type ProviderUsabilityChecker = (
provider: AutocompleteProviderKey,
providerSettingsManager: ProviderSettingsManager,
) => Promise<boolean>

export const defaultProviderUsabilityChecker: ProviderUsabilityChecker = async (provider, providerSettingsManager) => {
Copy link

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Newly added block should be marked with project-specific change annotations (kilocode_change) to ease upstream merges; wrap these consecutive lines with start/end markers.

Copilot uses AI. Check for mistakes.

}

const data = await response.json()
const balance = data.balance ?? 0
Copy link

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] API parsing assumes a top-level balance; tests currently mock nested { data: { balance } }. If the real endpoint returns nested balance, this logic will incorrectly treat it as zero. Confirm endpoint shape and, if nested, change to const balance = (data.balance ?? data.data?.balance ?? 0).

Suggested change
const balance = data.balance ?? 0
const balance = data.balance ?? data.data?.balance ?? 0

Copilot uses AI. Check for mistakes.

Comment on lines +141 to +143
const handler = this.apiHandler as any
if (handler.providerName && typeof handler.providerName === "string") {
return handler.providerName
Copy link

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Casting apiHandler to any and relying on providerName risks silent 'unknown' results; derive provider from the selected profile or handler class instead (e.g., expose a typed getProviderName() on ApiHandler).

Suggested change
const handler = this.apiHandler as any
if (handler.providerName && typeof handler.providerName === "string") {
return handler.providerName
const providerName = this.apiHandler.getProviderName?.()
if (providerName && typeof providerName === "string") {
return providerName

Copilot uses AI. Check for mistakes.

const listApiConfig = await provider.providerSettingsManager.listConfig()
await updateGlobalState("listApiConfigMeta", listApiConfig)
// Reload ghost model when API provider settings change
vscode.commands.executeCommand("kilo-code.ghost.reload")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kilocode chdange

import { z } from "zod"

import { modelInfoSchema, reasoningEffortWithMinimalSchema, verbosityLevelsSchema, serviceTierSchema } from "./model.js"
import { checkKilocodeBalance } from "./kilocode/kilocode.js"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kilocode chaneg

export const isProviderName = (key: unknown): key is ProviderName =>
typeof key === "string" && providerNames.includes(key as ProviderName)

export const AUTOCOMPLETE_PROVIDER_MODELS = {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

losse file?

@markijbema markijbema marked this pull request as ready for review October 17, 2025 09:29
@markijbema markijbema enabled auto-merge October 17, 2025 09:29
Copy link
Collaborator

@beatlevic beatlevic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@markijbema markijbema merged commit c5248e8 into main Oct 17, 2025
19 checks passed
@markijbema markijbema deleted the mark/supported-autocomplete-providers branch October 17, 2025 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Communicate clearly whether the chosen provider supports autocomplete

3 participants