-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
fix(embedding): remove forced /v1 suffix for embeddings #6589
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
Closed
Closed
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| from typing import Literal | ||
|
|
||
|
|
||
| def resolve_openai_compatible_base_url( | ||
| api_base: str, | ||
| mode: Literal["auto", "force_v1", "as_is"] = "auto", | ||
| default_base: str = "https://api.openai.com/v1", | ||
| ) -> str: | ||
| """Resolve OpenAI-compatible API base URL with configurable /v1 suffix handling. | ||
|
|
||
| Args: | ||
| api_base: The user-provided API base URL. | ||
| mode: How to handle the /v1 suffix: | ||
| - "auto": Add /v1 if not present (default). | ||
| - "force_v1": Always add /v1 suffix. | ||
| - "as_is": Keep the URL unchanged (including trailing slashes). | ||
| Note: With as_is mode, provide only the base URL without /embeddings path, | ||
| as the OpenAI SDK will append /embeddings automatically. | ||
| default_base: Default base URL to use if api_base is empty. | ||
|
|
||
| Returns: | ||
| The resolved API base URL. | ||
| """ | ||
| api_base = api_base.strip() | ||
| if not api_base: | ||
| return default_base | ||
|
|
||
| if mode == "as_is": | ||
| # Return URL unchanged to preserve exact configuration | ||
| # Note: In this mode, users should provide base URL without /embeddings path | ||
| # as the OpenAI SDK will append /embeddings automatically | ||
| return api_base | ||
|
|
||
| # Both "auto" and "force_v1" modes ensure URL ends with /v1 | ||
| api_base = api_base.removesuffix("/") | ||
| if not api_base.endswith("/v1"): | ||
| api_base = f"{api_base}/v1" | ||
| return api_base | ||
|
ccsang marked this conversation as resolved.
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,169 @@ | ||
| import pytest | ||
|
|
||
| from astrbot.core.provider.sources.openai_embedding_source import ( | ||
| OpenAIEmbeddingProvider, | ||
| ) | ||
| from astrbot.core.provider.utils import resolve_openai_compatible_base_url | ||
|
|
||
|
|
||
| def _make_provider(overrides: dict | None = None) -> OpenAIEmbeddingProvider: | ||
| provider_config = { | ||
| "id": "test-openai-embedding", | ||
| "type": "openai_embedding", | ||
| "embedding_api_key": "test-key", | ||
| "embedding_model": "text-embedding-3-small", | ||
| } | ||
| if overrides: | ||
| provider_config.update(overrides) | ||
| return OpenAIEmbeddingProvider( | ||
| provider_config=provider_config, | ||
| provider_settings={}, | ||
| ) | ||
|
|
||
|
|
||
| class TestResolveOpenAICompatibleBaseUrl: | ||
| """Test the resolve_openai_compatible_base_url helper function.""" | ||
|
|
||
| def test_auto_mode_adds_v1_when_missing(self): | ||
| """Test that auto mode adds /v1 suffix when not present.""" | ||
| result = resolve_openai_compatible_base_url( | ||
| "https://api.example.com", | ||
| mode="auto", | ||
| ) | ||
| assert result == "https://api.example.com/v1" | ||
|
|
||
| def test_auto_mode_keeps_v1_when_present(self): | ||
| """Test that auto mode keeps /v1 suffix when already present.""" | ||
| result = resolve_openai_compatible_base_url( | ||
| "https://api.example.com/v1", | ||
| mode="auto", | ||
| ) | ||
| assert result == "https://api.example.com/v1" | ||
|
|
||
| def test_force_v1_mode_always_adds_v1(self): | ||
| """Test that force_v1 mode always adds /v1 suffix.""" | ||
| result = resolve_openai_compatible_base_url( | ||
| "https://api.example.com/v1", | ||
| mode="force_v1", | ||
| ) | ||
| assert result == "https://api.example.com/v1" | ||
|
|
||
| def test_force_v1_mode_adds_v1_when_missing(self): | ||
| """Test that force_v1 mode adds /v1 suffix when missing.""" | ||
| result = resolve_openai_compatible_base_url( | ||
| "https://api.example.com", | ||
| mode="force_v1", | ||
| ) | ||
| assert result == "https://api.example.com/v1" | ||
|
|
||
| def test_as_is_mode_keeps_url_unchanged(self): | ||
| """Test that as_is mode keeps URL unchanged.""" | ||
| result = resolve_openai_compatible_base_url( | ||
| "https://api.example.com/custom/path", | ||
| mode="as_is", | ||
| ) | ||
| assert result == "https://api.example.com/custom/path" | ||
|
|
||
| def test_as_is_mode_removes_trailing_slash(self): | ||
| """Test that as_is mode removes trailing slash.""" | ||
| result = resolve_openai_compatible_base_url( | ||
| "https://api.example.com/", | ||
| mode="as_is", | ||
| ) | ||
| assert result == "https://api.example.com" | ||
|
|
||
| def test_empty_url_returns_default(self): | ||
| """Test that empty URL returns the default base URL.""" | ||
| result = resolve_openai_compatible_base_url( | ||
| "", | ||
| mode="auto", | ||
| ) | ||
| assert result == "https://api.openai.com/v1" | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_openai_embedding_provider_auto_mode(): | ||
| """Test OpenAI Embedding provider with auto mode (default).""" | ||
| provider = _make_provider( | ||
| {"embedding_api_base": "https://api.example.com", "embedding_api_base_mode": "auto"} | ||
| ) | ||
| try: | ||
| assert str(provider.client.base_url) == "https://api.example.com/v1/" | ||
| finally: | ||
| await provider.terminate() | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_openai_embedding_provider_force_v1_mode(): | ||
| """Test OpenAI Embedding provider with force_v1 mode.""" | ||
| provider = _make_provider( | ||
| { | ||
| "embedding_api_base": "https://api.example.com", | ||
| "embedding_api_base_mode": "force_v1", | ||
| } | ||
| ) | ||
| try: | ||
| assert str(provider.client.base_url) == "https://api.example.com/v1/" | ||
| finally: | ||
| await provider.terminate() | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_openai_embedding_provider_as_is_mode(): | ||
| """Test OpenAI Embedding provider with as_is mode.""" | ||
| provider = _make_provider( | ||
| { | ||
| "embedding_api_base": "https://api.example.com/v2/embeddings", | ||
| "embedding_api_base_mode": "as_is", | ||
| } | ||
| ) | ||
| try: | ||
| assert str(provider.client.base_url) == "https://api.example.com/v2/embeddings/" | ||
| finally: | ||
| await provider.terminate() | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_openai_embedding_provider_default_base_when_empty(): | ||
| """Test OpenAI Embedding provider with empty base URL uses default.""" | ||
| provider = _make_provider({"embedding_api_base": ""}) | ||
| try: | ||
| assert str(provider.client.base_url) == "https://api.openai.com/v1/" | ||
| finally: | ||
| await provider.terminate() | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_openai_embedding_provider_with_v1_already_present(): | ||
| """Test OpenAI Embedding provider when URL already has /v1.""" | ||
| provider = _make_provider( | ||
| {"embedding_api_base": "https://api.example.com/v1", "embedding_api_base_mode": "auto"} | ||
| ) | ||
| try: | ||
| assert str(provider.client.base_url) == "https://api.example.com/v1/" | ||
| finally: | ||
| await provider.terminate() | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_openai_embedding_provider_with_trailing_slash(): | ||
| """Test OpenAI Embedding provider keeps URL unchanged in as_is mode.""" | ||
| provider = _make_provider( | ||
| {"embedding_api_base": "https://api.example.com/", "embedding_api_base_mode": "as_is"} | ||
| ) | ||
| try: | ||
| # The provider returns URL unchanged, OpenAI client adds trailing slash | ||
| assert str(provider.client.base_url) == "https://api.example.com/" | ||
| finally: | ||
| await provider.terminate() | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_openai_embedding_provider_auto_mode_default(): | ||
| """Test OpenAI Embedding provider auto mode is default when not specified.""" | ||
| provider = _make_provider({"embedding_api_base": "https://api.example.com"}) | ||
| try: | ||
| # Should default to auto mode | ||
| assert str(provider.client.base_url) == "https://api.example.com/v1/" | ||
| finally: | ||
| await provider.terminate() |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.