fix(gui): allow Add Provider catalog scrolling#410
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Warning Review limit reached
Next review available in: 56 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (24)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ea98e44077
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const provider = { | ||
| ...providerConfigSeed(entry), | ||
| ...(suppliedBaseUrl ? { baseUrl: suppliedBaseUrl } : {}), | ||
| ...(supplied.apiKey ? { apiKey: supplied.apiKey } : {}), |
There was a problem hiding this comment.
When a user picks an OAuth preset such as Anthropic or xAI, clicks “Use an API key instead”, and then runs model discovery, the request body contains authMode: "key" plus the pasted key, but this reconstruction starts from the OAuth registry seed and only overlays the key. resolveModelsAuthToken therefore still treats the provider as OAuth, ignores the supplied API key, and discovery either fails as logged-out or probes with the wrong credential; carry through the supplied authMode (when valid for the key-mode flow) before building the /models request.
Useful? React with 👍 / 👎.
| const allowsModelsEnvelope = (prov.adapter === "google" && (prov.googleMode ?? "ai-studio") === "ai-studio") | ||
| || new URL(url).pathname.replace(/\/+$/, "").endsWith("/api/tags"); | ||
| const items = parseProviderModelsApiItems(json, allowsModelsEnvelope); |
There was a problem hiding this comment.
Enabling the Google models envelope here publishes every row returned by models.list. That API also returns non-chat models such as embedding-only rows whose supportedGenerationMethods do not include generateContent, while the Google adapter always calls :generateContent/:streamGenerateContent; those entries will now show up in the catalog and fail when selected. Preserve the live Google listing, but drop rows that cannot generate content before normalizing them.
Useful? React with 👍 / 👎.
| const { resolveModelsAuthToken, buildModelsRequest } = await import("../../oauth"); | ||
| const apiKey = await resolveModelsAuthToken(presetId, provider); | ||
| const request = buildModelsRequest(provider, apiKey, presetId); | ||
| const modelsDestinationError = await providerDestinationResolvedError(presetId, { baseUrl: request.url }); |
There was a problem hiding this comment.
For presets that allow a custom endpoint, a user can check “Allow local/private network” and pass the first destination check, but this second check rebuilds the provider object with only baseUrl. If the discovered /models URL is on a private host (for example a self-hosted compatible endpoint behind a Qwen/Alibaba preset), the probe is rejected even though the user explicitly opted in; pass the resolved provider’s allowPrivateNetwork through this check as well.
Useful? React with 👍 / 👎.
| } | ||
| if (apiKey) headers["Authorization"] = `Bearer ${apiKey}`; | ||
| return { url: `${effectiveProvider.baseUrl}/models`, headers }; | ||
| return { url: trustedModelsUrl ?? `${effectiveProvider.baseUrl}/models`, headers }; |
There was a problem hiding this comment.
When trustedModelsUrl is returned here for a canonical provider such as Fireworks, /api/providers/test also probes that URL, but its parser still only accepts { data: [...] } or { models: [...] }. The newly supported Fireworks-style top-level array is therefore treated as “unexpected shape” in the connection test even though catalog discovery succeeds via parseProviderModelsApiItems; update the test path to use the same normalizer before switching official presets to these alternate model URLs.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| .provider-catalog { display: flex; flex-direction: column; min-height: 0; height: calc(100% - 62px); } | ||
| .provider-catalog-standalone { padding: 20px; overflow-y: auto; } |
There was a problem hiding this comment.
The custom-provider deep link now renders only this standalone wrapper inside a fixed-height .provider-loader-card whose overflow is hidden, but the wrapper has no height or max-height of its own. On shorter viewports the custom form expands past the card and the Add/Back controls are clipped instead of scrolling; give the standalone view the remaining modal height (or make the card a flex column) so overflow-y: auto can actually take effect.
Useful? React with 👍 / 👎.
| const invalidateDiscoveryRequest = () => { | ||
| discoveryRequestRef.current += 1; | ||
| discoveryAbortRef.current?.abort(); | ||
| discoveryAbortRef.current = null; | ||
| setDiscoveryBusy(false); | ||
| }; |
There was a problem hiding this comment.
After a successful discovery, editing the endpoint, API key, adapter, or private-network toggle only aborts in-flight requests; it leaves the previously discovered model list, selected subset, and default model visible. If the user then adds the provider without rediscovering, a subset from the old endpoint/key can be persisted as selectedModels for the new setup, hiding valid models or pinning a non-existent default. Clear the discovered/selected models and source whenever invalidating the discovery inputs.
Useful? React with 👍 / 👎.
Summary
Dependency
Depends on the provider model loader PR (
feat/provider-model-loader, commit60046f85).Validation
bun test .\gui\tests\provider-catalog.test.tsbun run lint:gui