feat(vscode): overhaul Add Provider UI to match CLI templates dynamically - #1368
akramcodez merged 3 commits into
Conversation
nc-review: needs work — 2 blocking, 5 important, 1 nit@akramcodez — there is a blocking item below. The PR adds a fully form-driven Add Provider UI to the VS Code extension by hard-coding all 23 provider presets in chat-panel.js, plus a host-side SettingsManager.addProvider and a new addProvider webview message. The functionality is wired end-to-end and the UI shape matches the source wizard, but the preset data is duplicated verbatim from source/wizards/templates/provider-templates.ts, there are no tests for the new host-side code, the form is hidden before the host responds (so on failure the user loses their input), and the dead-code branch when modelsContainer is missing is silently swallowed. 🔴 blocking · The new 🔴 blocking ·
🟠 important · The submit handler resets and hides the form synchronously after posting the message: vscode.postMessage({ type: 'addProvider', provider });
// Reset form and hide it
presetSelect.value = 'ollama';
...
addProviderFormContainer.classList.add('hidden');But the host ( 🟠 important ·
🟠 important · On success the handler fires 🟠 important · The duplicate-model guard runs after the empty-model guard and compares 🟠 important · Three new ⚪ nit · The PR closes no issue and there is no discussion linked. Per CONTRIBUTING's "discuss substantial changes first" guidance and the project rubric's note that this repo takes many first-time contributions, a 23-preset hard-coded list with a new host-side write path and a new webview message is on the boundary of "substantial". It is justified — the existing "Edit in config file" affordance is genuinely clunky and the provider-mismatch bugs the author cites are real — but it warrants a brief issue write-up so future contributors don't repeat the work. Flagging as nit, not blocking, because the change is self-evidently worth making. 🔴 blocking · 🟠 a reviewer would ask for a change · ⚪ optional Automated code review — correctness, security, design, tests, plus duplicates and scope. A human still decides; this is not a substitute for review and is not exhaustive. The required status checks separately cover lint, formatting, types, unused dependencies, the test suite and the build. This bot never merges. Maintainers can rerun with |
|
/re-review |
nc-review: needs work — 2 blocking, 2 important@akramcodez — there is a blocking item below. PR adds a fully form-driven VS Code 'Add Provider' UI by hard-coding 23 provider presets in the webview, a host-side SettingsManager.addProvider, and a new webview message; I cannot read the diff itself (BASE checkout, files on disk are pre-change), so I cannot re-verify line-level form-reset/dedup/ 🔴 blocking · The previous review flagged 🔴 blocking · The PR is described as hard-coding 23 provider presets in the webview that mirror 🟠 important · I cannot verify the exact line of the new success-handler code without seeing the diff, but the previous review flagged that on success the add-provider handler fires 🟠 important · Listed in changed_files but not present in .changeset/ on disk (expected — the PR adds it). Spot-check that the package name inside it resolves against the workspace; the project rubric specifically calls out that a wrong name passes the file-presence check and then breaks 🔴 blocking · 🟠 a reviewer would ask for a change · ⚪ optional Automated code review — correctness, security, design, tests, plus duplicates and scope. A human still decides; this is not a substitute for review and is not exhaustive. The required status checks separately cover lint, formatting, types, unused dependencies, the test suite and the build. This bot never merges. Maintainers can rerun with |
Removed --color-vscode-editor-background, --color-vscode-foreground, and --color-vscode-descriptionForeground in the last commit, but these are all actively referenced as Tailwind utility classes in chat-panel.html and chat-panel.js. The verify-theme-css.js CI check cross-references every vscode-* class in the templates against the compiled CSS, so deleting them caused a CI failure. Restore all three tokens (keeping --color-vscode-description-foreground as a parallel kebab-case alias for forward compatibility) and revert the HTML rename of text-vscode-description-foreground back to text-vscode-descriptionForeground.
|
/re-review |
nc-review: comments — 3 nits@akramcodez — a few things worth a look, none blocking. The PR now ships the provider list from the host side via a derived Addressed since the last review
⚪ nit ·
⚪ nit ·
⚪ nit · I cannot read the changeset file (PR-added, not on the base checkout), so I cannot verify the package name inside it. The established pattern in this workspace is to file VS Code extension changesets against 🔴 blocking · 🟠 a reviewer would ask for a change · ⚪ optional Automated code review — correctness, security, design, tests, plus duplicates and scope. A human still decides; this is not a substitute for review and is not exhaustive. The required status checks separately cover lint, formatting, types, unused dependencies, the test suite and the build. This bot never merges. Maintainers can rerun with |
Description
This PR completely overhauls the "Add Provider" UI within the VS Code Extension to bring it into 100% feature parity with the CLI/TUI wizard. Previously, hardcoded models and SDK mismatches caused API routing errors (such as 404s on Groq and Atlas Cloud). The new UI dynamically pulls from the official 23 provider templates.
Key features added:
sdkProvider,baseUrl, and sets default environment-specific models.verify-theme-css.jsCI pipeline failures by declaring missing--color-vscode-*tokens.Type of Change
Changeset
pnpm changeset) describing this change for the changelogTesting
Automated Tests
pnpm test:allcompletes successfully)Manual Testing
zai-org/glm-5.3-flashsuccessfully queries the endpoint)Checklist