chore(wren-ai-service): Using OpenAI API Structured Output #731
Conversation
cyyeh
added
module/ai-service
cyyeh
changed the title
cyyeh
left a comment
cyyeh
left a comment
There was a problem hiding this comment.
@MGomaa435
Thanks for your contributions! I've left some comments. I'll also spend some other time to try the changes using openai llms!
| api_base: str = os.getenv("LLM_OPENAI_API_BASE") or LLM_OPENAI_API_BASE, | ||
| model: str = os.getenv("GENERATION_MODEL") or GENERATION_MODEL, | ||
| kwargs: Dict[str, Any] = ( | ||
| kwargs: set[Dict[str, Any]] = ( |
There was a problem hiding this comment.
excuse me, could you explain to me why this should be set?
There was a problem hiding this comment.
this is not needed; it will be reverted to the original Dict.
| results: list[SQLResult] | ||
|
|
||
|
|
||
| Generation_MODEL_KWARGS = { |
There was a problem hiding this comment.
could this constant be capitalized? please change all xxx_MODEL_KWARGS to all capitalized. thank you
There was a problem hiding this comment.
done, will be updated.
| message = ChatMessage.from_user(prompt) | ||
| if self.system_prompt: | ||
| messages = [ChatMessage.from_system(self.system_prompt), message] | ||
| # updated from_system to from_assistent as the new openai api is not accepting system prompts anymore, only user and assistant. |
There was a problem hiding this comment.
thanks for the info, could you also provide relevant release note or sth I can look further?
There was a problem hiding this comment.
https://community.openai.com/t/o1-models-do-not-support-system-role-in-chat-completion/953880/2
This is for o1 models and they would change it for all.
# Conflicts: # wren-ai-service/src/pipelines/retrieval/retrieval.py
|
reopen |
cyyeh
left a comment
cyyeh
left a comment
There was a problem hiding this comment.
all xxx_MODEL_KWARGS should be capitalized, thank you
| relationships: list[ModelRelationship] | ||
|
|
||
|
|
||
| Relationship_MODEL_KWARGS = { |
There was a problem hiding this comment.
could you also make this capitalized? thanks! @MGomaa435
| return AsyncGenerator( | ||
| model=self._generation_model, | ||
| url=f"{self._url}/api/generate", | ||
| url=f"{self._url}/apx i/generate", |
| model=self._generation_model, | ||
| url=f"{self._url}/api/generate", | ||
| url=f"{self._url}/apx i/generate", | ||
| generation_kwargs=self._model_kwargs, |
There was a problem hiding this comment.
this should be {**generation_kwargs, **self._model_kwargs}?
There was a problem hiding this comment.
also please change it in azure_openai.py
…tic model. -- following naming convention
cyyeh
left a comment
cyyeh
left a comment
There was a problem hiding this comment.
LGTM, thanks for your contribution!
2 participants
we are adapting the new OpenAI API structured output format using the Pydantic model.