-
Notifications
You must be signed in to change notification settings - Fork 150
fix: log non-OpenAI format= assumption once at init (#1502) #1514
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
Changes from all commits
a2c5b63
d918e6e
f19cf85
91b2aec
6b55f2e
0a3a57d
fc70da9
d77322b
ab754c1
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 |
|---|---|---|
|
|
@@ -195,7 +195,9 @@ def __init__( | |
|
|
||
| # Use provided parameters or fall back to environment variables | ||
| self._api_key = api_key | ||
| self._base_url = base_url | ||
| # Resolve env here (not only in the SDK) so _server_type / init logging | ||
| # see the same host the client will actually call. | ||
| self._base_url = base_url or os.getenv("OPENAI_BASE_URL") | ||
|
planetf1 marked this conversation as resolved.
|
||
|
|
||
| # Validate that we have the required configuration | ||
| if self._api_key is None and os.getenv("OPENAI_API_KEY") is None: | ||
|
|
@@ -205,7 +207,7 @@ def __init__( | |
| " 2. Pass it as a parameter: OpenAIBackend(api_key='your-key-here')" | ||
| ) | ||
|
|
||
| if self._base_url is None and os.getenv("OPENAI_BASE_URL") is None: | ||
| if self._base_url is None: | ||
| MelleaLogger.get_logger().warning( | ||
| "OPENAI_BASE_URL or base_url is not set.\n" | ||
| "The openai SDK is going to assume that the base_url is `https://api.openai.com/v1`" | ||
|
|
@@ -216,6 +218,16 @@ def __init__( | |
| if self._base_url is not None | ||
| else _ServerType.OPENAI | ||
| ) # type: ignore | ||
| if self._server_type != _ServerType.OPENAI: | ||
|
Contributor
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. Flagging in case another reviewer raises this: moving the log to fire on server type alone, regardless of whether |
||
| MelleaLogger.get_logger().info( | ||
| "Mellea assumes you are NOT using the OpenAI platform, and that " | ||
| "other model providers have less strict requirements on supporting " | ||
| "JSON schemas passed into `format=`. If you encounter a server-side " | ||
| "error when using format=, then you found an exception to this " | ||
| "assumption. Please open an issue at " | ||
| "github.com/generative-computing/mellea with the stack trace and " | ||
| "your inference engine / model provider." | ||
| ) | ||
|
|
||
| self._openai_client_kwargs = self.filter_openai_client_kwargs(**kwargs) | ||
|
|
||
|
|
@@ -968,9 +980,6 @@ async def _generate_from_chat_context_standard( | |
| }, | ||
| } | ||
| else: | ||
| MelleaLogger.get_logger().info( | ||
| "Mellea assumes you are NOT using the OpenAI platform, and that other model providers have less strict requirements on supporting JSON schemas passed into `format=`. If you encounter a server-side error following this message, then you found an exception to this assumption. Please open an issue at github.com/generative_computing/mellea with this stack trace and your inference engine / model provider." | ||
| ) | ||
| extra_params["response_format"] = { | ||
| "type": "json_schema", | ||
| "json_schema": { | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -7,6 +7,7 @@ | |||||||||||||||||||||||||||||||||||||||||
| _simplify_and_merge, and _make_backend_specific_and_remove. | ||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| import os | ||||||||||||||||||||||||||||||||||||||||||
| from unittest.mock import AsyncMock, MagicMock, PropertyMock, patch | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| import pytest | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -433,6 +434,123 @@ class Answer(pydantic.BaseModel): | |||||||||||||||||||||||||||||||||||||||||
| assert "guided_json" in extra_body or "structured_outputs" in extra_body | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| # --- #1502: non-OpenAI format= warning only at init --- | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| _FORMAT_ASSUMPTION = "NOT using the OpenAI platform" | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| def _info_msgs(mock_logger) -> list[str]: | ||||||||||||||||||||||||||||||||||||||||||
| return [str(c.args[0]) for c in mock_logger.info.call_args_list if c.args] | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| def test_non_openai_format_assumption_logged_once_at_init(): | ||||||||||||||||||||||||||||||||||||||||||
| mock_logger = MagicMock() | ||||||||||||||||||||||||||||||||||||||||||
| with ( | ||||||||||||||||||||||||||||||||||||||||||
| patch( | ||||||||||||||||||||||||||||||||||||||||||
| "mellea.backends.openai.MelleaLogger.get_logger", return_value=mock_logger | ||||||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||||||
| patch( | ||||||||||||||||||||||||||||||||||||||||||
| "mellea.backends.openai.is_vllm_server_with_structured_output", | ||||||||||||||||||||||||||||||||||||||||||
| return_value=False, | ||||||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||||||
| ): | ||||||||||||||||||||||||||||||||||||||||||
| OpenAIBackend( | ||||||||||||||||||||||||||||||||||||||||||
| model_id="gpt-4o", api_key="fake-key", base_url="http://localhost:9999/v1" | ||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||
| OpenAIBackend( | ||||||||||||||||||||||||||||||||||||||||||
| model_id="gpt-4o", api_key="fake-key", base_url="http://localhost:9999/v1" | ||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||
|
planetf1 marked this conversation as resolved.
|
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| matches = [m for m in _info_msgs(mock_logger) if _FORMAT_ASSUMPTION in m] | ||||||||||||||||||||||||||||||||||||||||||
| assert len(matches) == 2 # once per backend instance | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| def test_openai_platform_skips_format_assumption_log(): | ||||||||||||||||||||||||||||||||||||||||||
| mock_logger = MagicMock() | ||||||||||||||||||||||||||||||||||||||||||
| # Unset OPENAI_BASE_URL for this test: after resolving env into _base_url, | ||||||||||||||||||||||||||||||||||||||||||
| # a leftover non-OpenAI env would make the no-base_url construction log. | ||||||||||||||||||||||||||||||||||||||||||
| # These backends point at api.openai.com, so mock the vLLM version probe: | ||||||||||||||||||||||||||||||||||||||||||
| # __init__ calls is_vllm_server_with_structured_output unconditionally, | ||||||||||||||||||||||||||||||||||||||||||
| # which would otherwise make a real GET to api.openai.com/version. | ||||||||||||||||||||||||||||||||||||||||||
| with ( | ||||||||||||||||||||||||||||||||||||||||||
| patch( | ||||||||||||||||||||||||||||||||||||||||||
| "mellea.backends.openai.MelleaLogger.get_logger", return_value=mock_logger | ||||||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||||||
| patch( | ||||||||||||||||||||||||||||||||||||||||||
| "mellea.backends.openai.is_vllm_server_with_structured_output", | ||||||||||||||||||||||||||||||||||||||||||
| return_value=False, | ||||||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||||||
| patch.dict(os.environ), | ||||||||||||||||||||||||||||||||||||||||||
| ): | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+475
to
+484
Contributor
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.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||
| os.environ.pop("OPENAI_BASE_URL", None) | ||||||||||||||||||||||||||||||||||||||||||
| OpenAIBackend( | ||||||||||||||||||||||||||||||||||||||||||
| model_id="gpt-4o", api_key="fake-key", base_url="https://api.openai.com/v1" | ||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||
|
planetf1 marked this conversation as resolved.
|
||||||||||||||||||||||||||||||||||||||||||
| OpenAIBackend(model_id="gpt-4o", api_key="fake-key") | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| matches = [m for m in _info_msgs(mock_logger) if _FORMAT_ASSUMPTION in m] | ||||||||||||||||||||||||||||||||||||||||||
| assert matches == [] | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| def test_format_assumption_log_honors_openai_base_url_env(): | ||||||||||||||||||||||||||||||||||||||||||
|
planetf1 marked this conversation as resolved.
|
||||||||||||||||||||||||||||||||||||||||||
| """Env-only non-OpenAI base_url must still classify as non-OpenAI at init.""" | ||||||||||||||||||||||||||||||||||||||||||
| mock_logger = MagicMock() | ||||||||||||||||||||||||||||||||||||||||||
| with ( | ||||||||||||||||||||||||||||||||||||||||||
| patch( | ||||||||||||||||||||||||||||||||||||||||||
| "mellea.backends.openai.MelleaLogger.get_logger", return_value=mock_logger | ||||||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||||||
| patch( | ||||||||||||||||||||||||||||||||||||||||||
| "mellea.backends.openai.is_vllm_server_with_structured_output", | ||||||||||||||||||||||||||||||||||||||||||
| return_value=False, | ||||||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||||||
| patch.dict( | ||||||||||||||||||||||||||||||||||||||||||
| os.environ, {"OPENAI_BASE_URL": "http://localhost:9999/v1"}, clear=False | ||||||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||||||
|
planetf1 marked this conversation as resolved.
|
||||||||||||||||||||||||||||||||||||||||||
| ): | ||||||||||||||||||||||||||||||||||||||||||
| OpenAIBackend(model_id="gpt-4o", api_key="fake-key") | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| matches = [m for m in _info_msgs(mock_logger) if _FORMAT_ASSUMPTION in m] | ||||||||||||||||||||||||||||||||||||||||||
| assert len(matches) == 1 | ||||||||||||||||||||||||||||||||||||||||||
|
planetf1 marked this conversation as resolved.
|
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| async def test_format_assumption_not_relogged_per_generation(): | ||||||||||||||||||||||||||||||||||||||||||
| """#1502: the notice must not repeat on every format= generation.""" | ||||||||||||||||||||||||||||||||||||||||||
| import pydantic | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| from mellea.core.base import CBlock | ||||||||||||||||||||||||||||||||||||||||||
| from mellea.stdlib.context import ChatContext | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| class Answer(pydantic.BaseModel): | ||||||||||||||||||||||||||||||||||||||||||
| value: int | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| backend = OpenAIBackend( | ||||||||||||||||||||||||||||||||||||||||||
| model_id="gpt-4o", api_key="fake-key", base_url="http://localhost:9999/v1" | ||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||
| ctx = ChatContext().add(CBlock(value="q")) | ||||||||||||||||||||||||||||||||||||||||||
| resp = MagicMock() | ||||||||||||||||||||||||||||||||||||||||||
| resp.choices = [MagicMock()] | ||||||||||||||||||||||||||||||||||||||||||
| resp.choices[0].message.content = "{}" | ||||||||||||||||||||||||||||||||||||||||||
| resp.choices[0].message.role = "assistant" | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| mock_logger = MagicMock() | ||||||||||||||||||||||||||||||||||||||||||
| with ( | ||||||||||||||||||||||||||||||||||||||||||
| patch( | ||||||||||||||||||||||||||||||||||||||||||
| "mellea.backends.openai.MelleaLogger.get_logger", return_value=mock_logger | ||||||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||||||
| patch.object( | ||||||||||||||||||||||||||||||||||||||||||
| backend._async_client.chat.completions, "create", new_callable=AsyncMock | ||||||||||||||||||||||||||||||||||||||||||
| ) as create, | ||||||||||||||||||||||||||||||||||||||||||
| ): | ||||||||||||||||||||||||||||||||||||||||||
| create.return_value = resp | ||||||||||||||||||||||||||||||||||||||||||
| for _ in range(3): | ||||||||||||||||||||||||||||||||||||||||||
| await backend.generate_from_chat_context( | ||||||||||||||||||||||||||||||||||||||||||
| CBlock(value="q"), ctx, _format=Answer, model_options={} | ||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| msgs = [str(c.args[0]) for c in mock_logger.info.call_args_list if c.args] | ||||||||||||||||||||||||||||||||||||||||||
| assert [m for m in msgs if _FORMAT_ASSUMPTION in m] == [] | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| def test_default_extra_body_applied_when_no_per_call_override(): | ||||||||||||||||||||||||||||||||||||||||||
| """Construction-time default_extra_body is present in every merged result.""" | ||||||||||||||||||||||||||||||||||||||||||
| backend = OpenAIBackend( | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.