fix(ai): tolerate a full chat endpoint as a custom provider base URL when refreshing models#578
fix(ai): tolerate a full chat endpoint as a custom provider base URL when refreshing models#578hoishing wants to merge 1 commit into
Conversation
fetchModels appended "/models" to the base URL verbatim, so a provider whose base URL was pasted as the full endpoint (e.g. .../v1/chat/completions) hit .../v1/chat/completions/models and got a 404 HTML page — the model refresh never populated. Normalize the base URL first: strip a trailing /chat/completions or /responses segment (and any trailing slash) before appending /models, so the list resolves to <root>/models. Chat requests were unaffected (LLMClient already normalizes the endpoint). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> (cherry picked from commit 009882879d739ce0634598bf2dac621f0bca8c4c)
|
The PR Policy check is blocking this PR because required template information is missing. Please update the PR description with:
Screenshots or video are required for UI, UX, settings, onboarding, overlay, menu bar, or visual behavior changes. If this PR has no visual changes, check the no-visual-change box in the template. If this remains incomplete for 48 hours after opening, the PR may be closed. |
Greptile SummaryThis PR normalizes a custom provider's base URL in
|
| var normalizedBase = baseURL.trimmingCharacters(in: .whitespacesAndNewlines) | ||
| for suffix in ["/chat/completions", "/responses"] where normalizedBase.hasSuffix(suffix) { | ||
| normalizedBase = String(normalizedBase.dropLast(suffix.count)) | ||
| break | ||
| } | ||
| while normalizedBase.hasSuffix("/") { normalizedBase = String(normalizedBase.dropLast()) } | ||
| let urlString = "\(normalizedBase)/models" |
There was a problem hiding this comment.
Trailing slash defeats endpoint stripping
Trailing slash stripping happens after the suffix check, so a URL ending in /chat/completions/ (with trailing slash) won't match hasSuffix("/chat/completions") and gets appended with /models unchanged — producing …/chat/completions/models and the same 404 as before. Reversing the two steps (strip slashes first, then check for endpoint suffixes) fixes this. Compare with LLMClient.endpoint, which uses .contains instead of .hasSuffix and is immune to a trailing slash.
| var normalizedBase = baseURL.trimmingCharacters(in: .whitespacesAndNewlines) | |
| for suffix in ["/chat/completions", "/responses"] where normalizedBase.hasSuffix(suffix) { | |
| normalizedBase = String(normalizedBase.dropLast(suffix.count)) | |
| break | |
| } | |
| while normalizedBase.hasSuffix("/") { normalizedBase = String(normalizedBase.dropLast()) } | |
| let urlString = "\(normalizedBase)/models" | |
| var normalizedBase = baseURL.trimmingCharacters(in: .whitespacesAndNewlines) | |
| while normalizedBase.hasSuffix("/") { normalizedBase = String(normalizedBase.dropLast()) } | |
| for suffix in ["/chat/completions", "/responses"] where normalizedBase.hasSuffix(suffix) { | |
| normalizedBase = String(normalizedBase.dropLast(suffix.count)) | |
| break | |
| } | |
| while normalizedBase.hasSuffix("/") { normalizedBase = String(normalizedBase.dropLast()) } | |
| let urlString = "\(normalizedBase)/models" |
Prompt To Fix With AI
This is a comment left during a code review.
Path: Sources/Fluid/Services/ModelRepository.swift
Line: 256-262
Comment:
**Trailing slash defeats endpoint stripping**
Trailing slash stripping happens _after_ the suffix check, so a URL ending in `/chat/completions/` (with trailing slash) won't match `hasSuffix("/chat/completions")` and gets appended with `/models` unchanged — producing `…/chat/completions/models` and the same 404 as before. Reversing the two steps (strip slashes first, then check for endpoint suffixes) fixes this. Compare with `LLMClient.endpoint`, which uses `.contains` instead of `.hasSuffix` and is immune to a trailing slash.
```suggestion
var normalizedBase = baseURL.trimmingCharacters(in: .whitespacesAndNewlines)
while normalizedBase.hasSuffix("/") { normalizedBase = String(normalizedBase.dropLast()) }
for suffix in ["/chat/completions", "/responses"] where normalizedBase.hasSuffix(suffix) {
normalizedBase = String(normalizedBase.dropLast(suffix.count))
break
}
while normalizedBase.hasSuffix("/") { normalizedBase = String(normalizedBase.dropLast()) }
let urlString = "\(normalizedBase)/models"
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d095a4b510
ℹ️ 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".
| for suffix in ["/chat/completions", "/responses"] where normalizedBase.hasSuffix(suffix) { | ||
| normalizedBase = String(normalizedBase.dropLast(suffix.count)) | ||
| break | ||
| } | ||
| while normalizedBase.hasSuffix("/") { normalizedBase = String(normalizedBase.dropLast()) } |
There was a problem hiding this comment.
Normalize trailing slashes before matching endpoint suffixes
If a saved custom provider URL is copied with a trailing slash, e.g. https://host/v1/chat/completions/ or https://host/v1/responses/, this suffix check runs before the trailing slash is removed, so neither endpoint suffix matches. The code then appends /models to .../chat/completions or .../responses, preserving the same 404 path the change is meant to fix; trim trailing slashes before the suffix loop or repeat the suffix check afterward.
Useful? React with 👍 / 👎.
Description
When a custom AI provider's base URL is entered as the full chat endpoint (e.g.
https://host/v1/chat/completions), the model-refresh button fails.ModelRepository.fetchModelsappends/modelsverbatim, producinghttps://host/v1/chat/completions/models, which 404s (often returning an HTML page), so the model dropdown never populates:Pasting the full endpoint as the base URL is a very common user mistake, and chat requests were unaffected because
LLMClientalready normalizes the endpoint — so the two paths behaved inconsistently.This PR normalizes the base URL in
fetchModelsbefore appending/models: it strips a trailing/chat/completionsor/responsessegment (and any trailing slash) so the list resolves to<root>/models.Type of Change
Related Issue or Discussion
Closes #581
Testing
swiftlint --strict --config .swiftlint.yml Sourcesswiftformat --config .swiftformat SourcesVerified against a live OpenAI-compatible provider: with the base URL saved as
…/v1/chat/completions, model refresh previously 404'd; after the fix the same saved provider resolves…/v1/modelsand populates the dropdown. Base URLs already ending in/v1are unchanged.Screenshots / Video
Notes
Logic-only change in
ModelRepository.fetchModels; no behavior change for correctly-entered base URLs.🤖 Generated with Claude Code