Skip to content

fix(cli): surface invalid Kilo indexing.model as an error instead of silently falling back#12128

Open
rakshith1928 wants to merge 9 commits into
Kilo-Org:mainfrom
rakshith1928:fix/indexing-model-validation
Open

fix(cli): surface invalid Kilo indexing.model as an error instead of silently falling back#12128
rakshith1928 wants to merge 9 commits into
Kilo-Org:mainfrom
rakshith1928:fix/indexing-model-validation

Conversation

@rakshith1928

@rakshith1928 rakshith1928 commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Issue

Fixes #12121

Context

When a user sets an invalid indexing.model for the Kilo embedding provider, Kilo previously silently fell back to the hosted default model (mistralai/mistral-embed-2312). This masked a real configuration mistake: the user believed a specific model was in use, but indexing actually ran on a different (default) model with no signal that the configured value was invalid.

The correct behavior is to surface the misconfiguration as an indexing Error status so the user sees exactly what's wrong (the offending model name) in the indexing panel/GET /indexing/status, instead of silently substituting a model.

Scope is limited to the Kilo provider. Other providers (mistral, openai, ollama, …) resolve their model at embedder request time and fail there with an API error, so they are intentionally left unchanged.

Implementation

Changes are all in packages/opencode/src/kilocode/indexing.ts:

  • IndexingModelError (NamedError) — added IndexingModelError = NamedError.create("IndexingModelError", { model: Schema.String }). The invalid model value is carried in structured err.data.model (not err.message) so consumers/tests match via the generated IndexingModelError.isInstance(err) guard. No list of "available" models is included.
  • model() throws on explicit invalid model — when input.modelId is set and not found, throw IndexingModelError only if catalog.models.length > 0. The guard matters because fetchKiloEmbeddingModelCatalog returns an empty catalog (not a throw) on HTTP 500/network failure, so we must not error on something we can't confirm; the offline "leave metadata unset" behavior is preserved. A found model returns immediately with the hosted dimension/score; unset modelId keeps the default path.
  • failed() surfaces the message — detects IndexingModelError.isInstance(err) and builds Invalid indexing.model "", avoiding the useless "Failed to initialize: IndexingModelError". Other errors keep their original formatting.
  • boot() catches and surfaces as Error status — the model() call is wrapped in try/catch; on error it logs and returns track(hit, await inert(() => failed(err))), which resolves the init gate so KiloIndexing.init() completes normally and the status becomes "Error" with the descriptive message (available() false, ready() false, search() returns []).

Tradeoff to note: the error is swallowed in boot() deliberately (to keep init() resolving like the existing initialize-rejection path), so it's observed via the status message and a spy on the cached kilocode-indexing logger.

Screenshots / Video

N/A , no visual change beyond the existing indexing status text (state Error, message Failed to initialize: Invalid indexing.model)

  • But here's a flow diagram you can refer to understand the changes easily.
Old flow New flow
Start
 │
Resolve model
 │
Find chosen
 │
 ├── Found → Return
 │
 └── Not found
        │
      Warn
        │
   ┌────┴────┐
   │         │
default    no default
found     (empty catalog)
   │         │
Use default  Unset
   │         │
 Return    Return undefined
Start
 │
User configured model?
 │
 ├── Yes
 │     │
 │   Find model
 │     │
 │     ├── Found → Return
 │     │
 │     ├── Not found + catalog available
 │     │          │
 │     │          ▼
 │     │   Throw IndexingModelError
 │     │
 │     └── Not found + catalog empty
 │              │
 │      Fall through to default hunt
 │
 └── No
        │
  Find default model
        │
        ├── Found → Return
        │
        └── Not found
               │
             Warn (if modelId/dimension set)
               │
       Return modelId: undefined,
       modelDimension: undefined

How to Test

Manual/local verification

  • Invalid explicit model (catalog available): set indexing.model to an unsupported value for the kilo provider with a valid KILO_API_KEY. Indexing status should be state: "Error", message: Failed to initialize: Invalid indexing.model "".
  • Valid explicit model: set indexing.model to a real catalog id. Indexing initializes normally and uses that model.
  • Offline / catalog unavailable: force the catalog fetch to fail (HTTP 500). Indexing must not error on the invalid config; metadata is left unset (no throw).
  • Disabled indexing with invalid model: set indexing.enabled: false and indexing.model to an unsupported value for the kilo provider. Indexing status must be state: "Disabled" — a disabled config must not surface a model-validation error.

Reviewer test steps

  1. From packages/opencode/ run: bun test ./test/kilocode/indexing-startup.test.ts --timeout 60000
  2. Confirm all 18 tests pass, including reports an error for an unsupported explicit Kilo model instead of falling back, passes a valid explicit Kilo model through without error, leaves Kilo model metadata unset when the hosted catalog is unavailable, and does not validate the indexing model when indexing is disabled.
  3. Run bun run typecheck (passes).

Blocked checks and substitute verification

  • With the default 5s per-test timeout, the whole file times out in my environment (these startup tests need >5s here). Substitute: run with --timeout 60000, under which all 18 tests pass. This is a timing/environment note, not a logic failure.(this used to happen before my changes too.)

Checklist

  • Issue linked above, or exception explained
  • Tests/verification described
  • Screenshots/video included for visual changes, or marked N/A
  • Changeset considered for user-facing changes
  • I personally reviewed the diff and can explain the changes, including any AI-assisted work.

Get in Touch

@travix26 Discord

Throw a structured error when an explicit invalid indexing.model is set for the kilo provider so the misconfiguration is surfaced instead of silently falling back.
Catch the structured model error during indexing initialization and report it as an Indexing Error status so the invalid configuration is visible to the user.
Update the startup test to expect an Indexing Error status with the descriptive message when an explicit unsupported Kilo indexing model is configured.
Add a startup test asserting a configured valid Kilo indexing model is passed through to initialization without raising an error.
Documents the user-facing behavior change where an invalid Kilo indexing.model is surfaced as an indexing Error status instead of silently falling back.
const cfgInput = await model(enrichKilo(input(merged, global), auth), auth)
let cfgInput: Awaited<ReturnType<typeof model>>
try {
cfgInput = await model(enrichKilo(input(merged, global), auth), auth)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[WARNING]: Disabled Kilo indexing can now report an error

Because cfgInput.enabled is not checked until the later disabled branch, this call validates the configured model even when indexing is disabled. A config such as { enabled: false, provider: "kilo", model: "removed/model" } now throws here and returns Error instead of the expected Disabled status, so disabling indexing no longer suppresses startup validation. Suggest short-circuiting disabled input before model() or bypassing explicit-model validation when enabled is false.


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

@rakshith1928 rakshith1928 Jul 11, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks — this is now fixed. The IndexingModelError throw in model() was firing before the disabled check. Guarded it with input.enabled !== false so a disabled Kilo config with a bad explicit model falls through to the existing disabled branch and reports Disabled instead of Error.

  1. src/kilocode/indexing.ts: throw condition is now catalog.models.length > 0 && !chosen && input.enabled !== false
  2. Added new test: does not validate the indexing model when indexing is disabled ; no IndexingModelError logged
    Also updated the PR body.

@kilo-code-bot

kilo-code-bot Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (2 files)
  • packages/opencode/src/kilocode/indexing.ts
  • packages/opencode/test/kilocode/indexing-startup.test.ts
Previous Review Summaries (2 snapshots, latest commit bb4a71d)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit bb4a71d)

Status: No Issues Found | Recommendation: Merge

Files Reviewed (2 files)
  • packages/opencode/src/kilocode/indexing.ts
  • packages/opencode/test/kilocode/indexing-startup.test.ts

Previous review (commit ad2cc71)

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
packages/opencode/src/kilocode/indexing.ts 287 Disabled Kilo indexing can report an Error while validating an unused model

Fix these issues in Kilo Cloud

Files Reviewed (3 files)
  • .changeset/indexing-invalid-model-error.md - 0 issues
  • packages/opencode/src/kilocode/indexing.ts - 1 issue
  • packages/opencode/test/kilocode/indexing-startup.test.ts - 0 issues

Reviewed by gpt-5.6-sol · Input: 52.9K · Output: 3.8K · Cached: 216.7K

Review guidance: REVIEW.md from base branch main

A disabled Kilo indexing config with an invalid explicit model previously threw IndexingModelError and surfaced an Error status, bypassing the disabled branch. Guard the throw with input.enabled so disabled configs are reported as Disabled as before.

@marius-kilocode marius-kilocode left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Disabled indexing still resolves the Kilo model catalog before reaching the Disabled branch. The input.enabled !== false condition prevents IndexingModelError, but it does not prevent the fetch or other catalog-resolution errors.

I reproduced this with enabled: false and indexing.kilo.baseUrl: "not a url": expected Disabled, received Error. baseUrl is only validated as a string, and URL construction throws before the catalog helper can return its empty fallback. Disabled indexing also makes an unnecessary network request and can incur catalog retry delays.

Can we return before catalog resolution when indexing is disabled? The minimal change is:

if (input.embedderProvider !== "kilo" || !input.enabled) return input

Then the inner condition can become catalog.models.length > 0 && !chosen, which also removes the new boolean-comparison lint warning. Please add a regression test that makes catalog resolution throw, or asserts fetch is not called, while disabled and still reaches the Disabled status.

@rakshith1928

Copy link
Copy Markdown
Contributor Author

Thanks for the review I will look into this.

Return early from model() when indexing is disabled so a disabled Kilo config no longer resolves the embedding model catalog (and its baseUrl) or surfaces a model-validation error. Adds a regression test asserting a disabled config with an invalid kilo.baseUrl reaches Disabled without calling fetch.
@rakshith1928

rakshith1928 commented Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

Done. model() now returns early when disabled, so no catalog fetch/URL error. Inner condition reverted to catalog.models.length > 0 && !chosen (no boolean-comparison lint warning). Added a regression test: disabled + bad kilo.baseUrl still reaches Disabled and fetch is never called. 19 tests pass; typecheck/lint clean.

…validation

# Conflicts:
#	packages/opencode/src/kilocode/indexing.ts
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.

[Bug] Specifying an invalid indexing model silently falls back to the default

2 participants