-
Notifications
You must be signed in to change notification settings - Fork 0
Fix: Create OpenAI model instance in build_native_gemini_model #3
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 | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2,6 +2,7 @@ | |||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| import pytest | ||||||||||||||||||||||||
| from pydantic_ai.models.google import GoogleModel | ||||||||||||||||||||||||
| from pydantic_ai.models.openai import OpenAIModel | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| from services.llm import GeminiModel, build_native_gemini_model | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
|
|
@@ -10,8 +11,11 @@ def test_gemini_model_alias_is_google_model() -> None: | |||||||||||||||||||||||
| assert GeminiModel is GoogleModel | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| def test_build_native_openai_returns_string() -> None: | ||||||||||||||||||||||||
| assert build_native_gemini_model("openai:gpt-4o-mini") == "openai:gpt-4o-mini" | ||||||||||||||||||||||||
| def test_build_native_openai_returns_openai_model(monkeypatch: pytest.MonkeyPatch) -> None: | ||||||||||||||||||||||||
| monkeypatch.setenv("OPENAI_API_KEY", "test-openai-key") | ||||||||||||||||||||||||
| m = build_native_gemini_model("openai:gpt-4o-mini") | ||||||||||||||||||||||||
| assert isinstance(m, OpenAIModel) | ||||||||||||||||||||||||
| assert m.model_name == "gpt-4o-mini" | ||||||||||||||||||||||||
|
Comment on lines
+14
to
+18
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. suggestion (testing): Also assert the OpenAIModel api_key to verify that OPENAI_API_KEY wiring works as expected. The current test only checks that we build an OpenAIModel with the expected model_name. Please also assert that
Suggested change
|
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| def test_build_native_google_gla_returns_google_model(monkeypatch: pytest.MonkeyPatch) -> None: | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
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.
issue: Guard against empty or missing model id in the
openai:specWhen
normalized_specis just"openai:"(or otherwise has nothing after the colon),model_idbecomes an empty string and you end up constructingOpenAIModel(""), which may fail in a confusing way. Consider validating thatmodel_idis non-empty and either returningspecunchanged or raising a clearer error for malformed specs.