fix: openai endpoint type returning empty model listings#151
Conversation
…osted API examples
WalkthroughConsolidates OpenAI and OpenAI-compatible profiles into a single configuration by treating ChangesOpenAI Profile Type Consolidation
🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@CodeRabbit: review? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 34121
✅ Core fix (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/adapter/registry/profile/factory.go`:
- Around line 159-166: The collision warning logs the wrong winner: after
checking f.prefixLookup[prefix] the code assigns f.prefixLookup[prefix] =
profileName (last-writer wins), but the slog.Warn uses "keeping" = existing and
"ignored" = profileName; update the log to reflect runtime behavior by swapping
those fields so "keeping" is profileName and "ignored" is existing (referencing
f.prefixLookup[prefix], existing, and profileName in the slog.Warn call).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f2b7f4e8-ba2e-4467-a812-3d53792faa90
📒 Files selected for processing (25)
CLAUDE.mdconfig/profiles/README.mdconfig/profiles/openai-compatible.yamlconfig/profiles/openai.yamldocs/content/api-reference/openai.mddocs/content/compare/gpustack.mddocs/content/compare/integration-patterns.mddocs/content/compare/localai.mddocs/content/concepts/health-checking.mddocs/content/concepts/profile-system.mddocs/content/concepts/provider-metrics.mddocs/content/configuration/examples.mddocs/content/configuration/reference.mddocs/content/development/architecture.mddocs/content/getting-started/quickstart.mddocs/content/integrations/frontend/openwebui.mddocs/content/troubleshooting/path-preservation.mdinternal/adapter/registry/profile/factory.gointernal/app/handlers/handler_provider_compatibility_test.gointernal/app/handlers/handler_proxy_openai_alias_test.gointernal/app/handlers/server_routes.gointernal/core/domain/profile.gointernal/core/domain/routing.gointernal/core/domain/routing_test.gotest/integration/profile_routing_test.go
💤 Files with no reviewable changes (1)
- config/profiles/openai.yaml
| if existing, collision := f.prefixLookup[prefix]; collision && existing != profileName { | ||
| slog.Warn("two profiles claim the same routing prefix; one will shadow the other", | ||
| "prefix", prefix, | ||
| "keeping", existing, | ||
| "ignored", profileName, | ||
| ) | ||
| } | ||
| f.prefixLookup[prefix] = profileName |
There was a problem hiding this comment.
Collision log fields are reversed from actual winner.
The warning says "keeping" = existing and "ignored" = profileName, but Line 166 immediately overwrites the prefix with profileName (last writer wins). Please align the log fields with runtime behaviour to avoid misleading diagnostics.
Proposed fix
- if existing, collision := f.prefixLookup[prefix]; collision && existing != profileName {
+ if existing, collision := f.prefixLookup[prefix]; collision && existing != profileName {
slog.Warn("two profiles claim the same routing prefix; one will shadow the other",
"prefix", prefix,
- "keeping", existing,
- "ignored", profileName,
+ "keeping", profileName,
+ "ignored", existing,
)
}
f.prefixLookup[prefix] = profileName📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if existing, collision := f.prefixLookup[prefix]; collision && existing != profileName { | |
| slog.Warn("two profiles claim the same routing prefix; one will shadow the other", | |
| "prefix", prefix, | |
| "keeping", existing, | |
| "ignored", profileName, | |
| ) | |
| } | |
| f.prefixLookup[prefix] = profileName | |
| if existing, collision := f.prefixLookup[prefix]; collision && existing != profileName { | |
| slog.Warn("two profiles claim the same routing prefix; one will shadow the other", | |
| "prefix", prefix, | |
| "keeping", profileName, | |
| "ignored", existing, | |
| ) | |
| } | |
| f.prefixLookup[prefix] = profileName |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/adapter/registry/profile/factory.go` around lines 159 - 166, The
collision warning logs the wrong winner: after checking f.prefixLookup[prefix]
the code assigns f.prefixLookup[prefix] = profileName (last-writer wins), but
the slog.Warn uses "keeping" = existing and "ignored" = profileName; update the
log to reflect runtime behavior by swapping those fields so "keeping" is
profileName and "ignored" is existing (referencing f.prefixLookup[prefix],
existing, and profileName in the slog.Warn call).
resolves #148
GET /olla/openai/v1/models was returning empty for endpoints configured with
type: "openai". Chat completions worked fine through a different code path, which is why this looked listing-specific.Root cause:
IsCompatibleWithin routing.go only aliasedProfileOpenAICompatibleto ollama and lm-studio, never "openai" itself. Added "openai" to the alias.Also dropped
config/profiles/openai.yaml, a near-clone of openai-compatible.yaml left over from the November consolidation. Both YAMLs claimed the "openai" routing prefix which made route registration non-deterministic depending on Go map iteration order. With openai.yaml gone the openai-compatible profile owns the prefix unambiguously.Existing user configs with
type: "openai"keep working unchanged via the alias. Docs swept to recommendtype: "openai-compatible"as the canonical value with a note that "openai" is accepted.Worth coordinating with #149 since both touch this area.
Summary by CodeRabbit
Release Notes
Documentation
openaiis now an accepted alias foropenai-compatibleendpointsBug Fixes
Tests