-
-
Notifications
You must be signed in to change notification settings - Fork 466
fix(ai): tolerate a full chat endpoint as a custom provider base URL when refreshing models #578
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -249,8 +249,17 @@ final class ModelRepository { | |||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| let isAnthropic = providerID == "anthropic" || baseURL.contains("anthropic.com") | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| // Construct the models endpoint URL | ||||||||||||||||||||||||||||||||
| let urlString = baseURL.hasSuffix("/") ? "\(baseURL)models" : "\(baseURL)/models" | ||||||||||||||||||||||||||||||||
| // Construct the models endpoint URL. Users frequently paste the full chat | ||||||||||||||||||||||||||||||||
| // endpoint (…/chat/completions or …/responses) as the base URL; append `/models` | ||||||||||||||||||||||||||||||||
| // to that verbatim and it 404s. Strip a trailing endpoint segment (and slash) so | ||||||||||||||||||||||||||||||||
| // the list resolves to <root>/models. | ||||||||||||||||||||||||||||||||
| 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" | ||||||||||||||||||||||||||||||||
|
Comment on lines
+256
to
+262
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Trailing slash stripping happens after the suffix check, so a URL ending in
Suggested change
Prompt To Fix With AIThis 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. |
||||||||||||||||||||||||||||||||
| guard let url = URL(string: urlString) else { | ||||||||||||||||||||||||||||||||
| DebugLogger.shared.error( | ||||||||||||||||||||||||||||||||
| "fetchModels: Invalid URL constructed from baseURL='\(baseURL)' -> '\(urlString)'", | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If a saved custom provider URL is copied with a trailing slash, e.g.
https://host/v1/chat/completions/orhttps://host/v1/responses/, this suffix check runs before the trailing slash is removed, so neither endpoint suffix matches. The code then appends/modelsto.../chat/completionsor.../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 👍 / 👎.