Skip to content

fix: openai endpoint type returning empty model listings#151

Merged
thushan merged 8 commits into
mainfrom
fix/issue-148-openai-type-alias
May 23, 2026
Merged

fix: openai endpoint type returning empty model listings#151
thushan merged 8 commits into
mainfrom
fix/issue-148-openai-type-alias

Conversation

@thushan

@thushan thushan commented May 23, 2026

Copy link
Copy Markdown
Owner

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: IsCompatibleWith in routing.go only aliased ProfileOpenAICompatible to 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 recommend type: "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

    • Updated configuration examples and API reference to reflect OpenAI endpoint restructuring; openai is now an accepted alias for openai-compatible endpoints
    • Revised endpoint type documentation across guides to clarify supported alias options
  • Bug Fixes

    • Consolidated OpenAI profile handling to prevent routing inconsistencies; both endpoint types now route through unified configuration
  • Tests

    • Added tests verifying OpenAI alias compatibility and real-world profile factory integration

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 23, 2026

Copy link
Copy Markdown

Walkthrough

Consolidates OpenAI and OpenAI-compatible profiles into a single configuration by treating "openai" as a routing alias for the openai-compatible profile. Adds domain-layer alias acceptance, configuration consolidation, routing comments, comprehensive tests, and documentation updates reflecting the unified structure.

Changes

OpenAI Profile Type Consolidation

Layer / File(s) Summary
Domain profile aliasing and compatibility
internal/core/domain/profile.go, internal/core/domain/routing.go, internal/core/domain/routing_test.go, test/integration/profile_routing_test.go
ProfileOpenAI constant is added. RequestProfile.IsCompatibleWith now accepts endpoints typed ProfileOpenAI when the profile supports ProfileOpenAICompatible. Domain tests and integration tests verify the alias behaviour.
Profile configuration consolidation and factory logging
config/profiles/openai-compatible.yaml, internal/adapter/registry/profile/factory.go
The standalone config/profiles/openai.yaml file is removed. The openai-compatible.yaml profile gains openai as an additional routing prefix. Factory's buildPrefixLookup now emits slog.Warn when multiple profiles claim the same routing prefix.
Handler routing and alias verification tests
internal/app/handlers/server_routes.go, internal/app/handlers/handler_proxy_openai_alias_test.go, internal/app/handlers/handler_provider_compatibility_test.go
Routing handler comments clarify OpenAI-compatibility dispatch and prefix variable usage. New tests verify that endpoints typed "openai" are accepted when filtered by profiles supporting openai-compatible, and that real factory-backed routing accepts openai-scoped requests.
Documentation updates reflecting alias consolidation
CLAUDE.md, config/profiles/README.md, docs/content/concepts/*, docs/content/configuration/*, docs/content/getting-started/*, docs/content/compare/*, docs/content/integrations/*, docs/content/troubleshooting/*, docs/content/development/architecture.md
Project structure and configuration reference documentation list openai-compatible.yaml as the single OpenAI-compatible profile with type: "openai" noted as an accepted alias. All example YAML configurations throughout quickstart, comparison guides, integration guides, and troubleshooting sections are updated to use type: "openai-compatible" with inline comments indicating openai acceptance. Response header documentation for X-Olla-Backend-Type clarified to echo the configured endpoint type.

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • thushan/olla#33: Modifies the shared buildPrefixLookup mechanism in profile factory that now logs collisions, used by OpenAI/OpenAI-compatible routing.

Suggested labels

bug, routing, documentation

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change—fixing empty model listings for openai-typed endpoints by treating openai and openai-compatible as aliases.
Linked Issues check ✅ Passed The pull request fully addresses issue #148 by implementing option 1 (alias approach): treating openai and openai-compatible as compatible types in routing.go, removing the duplicate openai.yaml profile, and ensuring existing configs with type: 'openai' continue to work.
Out of Scope Changes check ✅ Passed All code and documentation changes are directly related to resolving the openai alias issue; no unrelated modifications were introduced.
Docstring Coverage ✅ Passed Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/issue-148-openai-type-alias

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@thushan

thushan commented May 23, 2026

Copy link
Copy Markdown
Owner Author

@CodeRabbit: review?

@coderabbitai

coderabbitai Bot commented May 23, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Get PR details and diff
gh pr view 151 --repo thushan/olla
echo "---FILES CHANGED---"
gh pr diff 151 --repo thushan/olla

Length of output: 34121


@thushan: Solid fix — the root cause is correctly identified and the change in routing.go is minimal and safe. A few observations:


✅ Core fix (internal/core/domain/routing.go)

The one-line alias addition is correct and well-commented. The new ProfileOpenAI constant in profile.go keeps the domain layer self-contained without importing internal/core/constants.


✅ Deletion of config/profiles/openai.yaml

The right call. With both YAMLs claiming the "openai" prefix and Go map iteration being non-deterministic, the collision was a latent correctness hazard. The GPT model context patterns lost from openai.yaml are appropriate to drop given the local-backend focus.


⚠️ Misleading log in factory.go

// internal/adapter/registry/profile/factory.go  (lines ~157-164)
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,     // ← says "keeping existing"
        "ignored", profileName,  // ← says "ignoring new"
    )
}
f.prefixLookup[prefix] = profileName  // ← but actually OVERWRITES existing with new

The log attributes keeping and ignored are inverted relative to what the code actually does — it keeps profileName (the new one), not existing. While openai.yaml is gone so this path won't fire in practice, a future contributor adding a profile with a duplicate prefix will see a misleading warning. Either swap the attribute values, or continue early to truly preserve existing:

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,  // last writer wins
        "ignored", existing,
    )
}

✅ Tests

  • TestIsCompatibleWith_OpenAIAlias — smallest possible regression guard, good.
  • TestProviderCompatibility_RealFactory — excellent addition; this is the production code path the original unit tests bypassed, which is exactly why issue #148 wasn't caught earlier.
  • TestFilterEndpointsByProfile_OpenAITypeAlias — clearly documents the symptom ("fallback warning fires, result shrinks to 1").

✅ Docs sweep

Comprehensive and consistent. The X-Olla-Backend-Type note in openai.md and quickstart.md ("echoes the configured type: value") is a useful clarification for users debugging endpoint issues.


Summary: The fix is correct and well-tested. The only actionable item before merge is the inverted keeping/ignored log fields in factory.go.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 6d6ac4d and 2245591.

📒 Files selected for processing (25)
  • CLAUDE.md
  • config/profiles/README.md
  • config/profiles/openai-compatible.yaml
  • config/profiles/openai.yaml
  • docs/content/api-reference/openai.md
  • docs/content/compare/gpustack.md
  • docs/content/compare/integration-patterns.md
  • docs/content/compare/localai.md
  • docs/content/concepts/health-checking.md
  • docs/content/concepts/profile-system.md
  • docs/content/concepts/provider-metrics.md
  • docs/content/configuration/examples.md
  • docs/content/configuration/reference.md
  • docs/content/development/architecture.md
  • docs/content/getting-started/quickstart.md
  • docs/content/integrations/frontend/openwebui.md
  • docs/content/troubleshooting/path-preservation.md
  • internal/adapter/registry/profile/factory.go
  • internal/app/handlers/handler_provider_compatibility_test.go
  • internal/app/handlers/handler_proxy_openai_alias_test.go
  • internal/app/handlers/server_routes.go
  • internal/core/domain/profile.go
  • internal/core/domain/routing.go
  • internal/core/domain/routing_test.go
  • test/integration/profile_routing_test.go
💤 Files with no reviewable changes (1)
  • config/profiles/openai.yaml

Comment on lines +159 to 166
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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

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.

Suggested change
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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GET /v1/models returns empty when endpoints are typed "openai" (not "openai-compatible")

1 participant