Repository navigation
fix(kilo-vscode): surface managed overlay fields in Indexing settings UI - #15453
Open
PeterHodges15 wants to merge 2 commits into
Open
PeterHodges15 wants to merge 2 commits into
PeterHodges15 wants to merge 2 commits into
Conversation
Plumb the config overlay's per-field provenance (fields) from the server snapshot through KiloProvider into the webview config context, and use it in the Indexing tab so managed/enterprise values are shown (with source tag and editability) instead of rendering blank.
github-actions
Bot
added
area:vscode
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| indexingDescription(value, inheritance(paths)) | ||
| /** Display value + provenance for a field: editable scopes first, then managed/runtime resolution. */ | ||
| const fieldInfo = (path: readonly string[]) => | ||
| indexingFieldDisplay(scope(), globalCfg(), projectCfg(), configFields(), path) |
Contributor
There was a problem hiding this comment.
WARNING: configFields() always comes from an overlay resolved with scope: "project" (see fetchSnapshot at packages/kilo-vscode/src/kilo-provider/config-snapshot.ts:19), but it is reused here for both scope views. Two consequences:
- In the default
globalview, when the global layer does not define a field,indexingFieldDisplayfalls through to this project-scoped map, so project-local values leak into the global editor. For example the model field (line 485) and dimension field (line 504) now display the project's value with aLocaltag while the user believes they are editing global config;selectedProvider()andsaveEnabled()also see the project provider, which suppresses the auto-select-kilo branch insaveEnabled. editablefrom a project-scope overlay is alwaystruefor managed fields (packages/opencode/src/kilocode/config/overlay.ts:418computeseditable: source !== "system" || input.scope === "project"), sodisabled={!fieldInfo(...).editable}never disables a managed field in the global view. The new test'seditable: falsecase inindexing-tab-state.test.tsis not reachable with real overlay data, so the changeset claim that inputs which cannot be saved are disabled does not hold.
Consider resolving/fetching overlay fields for both scopes (or computing the scope-appropriate value and editability client-side) instead of sharing the project-scoped snapshot across both scope views.
| inherited={inherited()} | ||
| t={language.t} | ||
| tag={() => tag(scope(), [["enabled"]])} | ||
| tag={() => tag(scope(), ["enabled"])} |
Contributor
There was a problem hiding this comment.
WARNING: The switch state is still derived from raw layers only (enabled()/inherited() use indexingEnabled/indexingEnabledInherited), while this tag now resolves through fieldInfo, which includes the overlay/managed value. With a managed indexing.enabled: true, the row renders a Managed tag next to a switch that reads off, and the description still says Inherited from global config (off) until a project value is saved. — so the user cannot tell the effective state and cannot trust the toggle. The switch is also not disabled for non-editable fields. Either make the switch/description use the overlay-resolved value for the displayed scope, or keep this tag raw-only.
Contributor
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (3 files in incremental diff)
Fix these issues in Kilo Cloud Previous Review Summary (commit df06a0d)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit df06a0d)Status: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (11 files)
Reviewed by deepseek-flash-latest · Input: 70.9K · Output: 17.6K · Cached: 845.1K Review guidance: REVIEW.md from base branch |
…es in Indexing UI When the config overlay reports an indexing field as non-editable, its effective value now wins over a same-named value in the global or project scope, so a managed field renders disabled with the effective value instead of a stale value that would be rejected on save. Also unify the duplicated webview provenance type by aliasing IndexingFieldInfo to ConfigField.
This branch has not been deployed
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
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.
Issue
No existing issue - this is a newly reported bug found while regression-testing managed-config behavior on Kilo 7.8.17.
Related: #15451 (request to extend this pattern to all settings sections; this PR implements the Indexing tab as the reference implementation) and #15450 (unrelated embedding-batcher defect observed during the same test session).
Context
When a managed/enterprise config (e.g.
C:\ProgramData\kilo\kilo.jsoncon Windows) supplies the indexing values, the Indexing settings tab rendered blank for every managed field: provider, model, dimensions, base URL, API key, file extensions, and tuning.Root cause: the extension fetched the server's config overlay but discarded its per-field provenance (
fields), and the tab computed display values from the raw global/project config layers only - so the managed/system layer was structurally unreachable in the UI. The overlay endpoint already returnsfields(path,source: project|global|system|default,editable, resolved value); this PR plumbs it through to the settings UI.Implementation
config-snapshot.ts- returnfieldsfrom the overlay response alongsidetargets/collections.KiloProvider.ts- addfieldsto the config snapshot type and to both the config-load and config-refresh message payloads.fieldsadded toConfigLoadedMessage/ConfigUpdatedMessage; the config context stores them in aconfigFieldssignal; the Indexing tab now resolves each field as raw-scope value with the overlay-resolved value as fallback, mapssource: "system"to a "Managed" tag plus an "Inherited from managed config" note, and disables inputs wheneditable === false.indexing-tab-state.ts- newindexingFieldDisplay()helper plus"managed"inheritance/source entries.configUpdatedmessage branch incontext/config.tsxwas extracted intoapplyConfigUpdated()to stay under the eslint complexity limit.Screenshots / Video
How to Test
Manual/local verification
bun test tests/unit/indexing-tab-state.test.ts tests/unit/kilo-provider-indexing-refresh.test.ts-> 28 pass / 0 fail (92 expect calls). The same tests were red before the fix (14 pass / 2 fail / 1 error).check-types,check-types:webview,lint, andbundleinpackages/kilo-vscode-> clean.Reviewer test steps
main, apply this diff, runbun installat the root, then inpackages/kilo-vscode:bun test tests/unit/indexing-tab-state.test.ts tests/unit/kilo-provider-indexing-refresh.test.tsbun run build:checkBlocked checks and substitute verification
bun run compile(full) could not complete on the author's machine: itsprepare:cli-binarystep fetcheshttps://models.dev/api.jsonand outbound egress is restricted. Substitute: the stock 7.8.17 CLI binary shipped with the installed extension was used alongside the dev tree (this change is client-side only; no CLI/server changes), plus all per-package checks above.test:visual/test:a11ynot run (no browser available in this environment); substitute: targeted unit tests and the manual dev-host verification above.bun turbo typecheck) could not run on the author's machine: the turbo Windows binary crashes with 0xC00000FD even on--version(environmental). Substitute: per-packagecheck-types/check-types:webviewin the only affected package, as above.Checklist
fix-managed-indexing-ui.mdincludedGet in Touch
Kilo Code Discord: PeterH (GitHub: PeterHodges15)