-
Notifications
You must be signed in to change notification settings - Fork 0
Report Generation: Switching from OpenAI SDK to Google ADK #35
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
Conversation
aieng-eval-agents/aieng/agent_evals/report_generation/evaluation.py
Outdated
Show resolved
Hide resolved
aieng-eval-agents/aieng/agent_evals/report_generation/evaluation.py
Outdated
Show resolved
Hide resolved
aieng-eval-agents/aieng/agent_evals/report_generation/evaluation.py
Outdated
Show resolved
Hide resolved
| self._openai_client = AsyncOpenAI(api_key=api_key, base_url=self.configs.openai_base_url) | ||
| self._initialized = True | ||
| return self._openai_client | ||
|
|
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.
As a point of discussion: I think we should keep this and use it for single LLM calls (e.g. llm-as-a-judge evaluation). The reason is that the openai client is almost standard and can work with multiple providers, including gemini, by changing the base url and the api key. I don't think the client provided in the google genai library has that level of compatibility.
I'm working on a reusable llm judge implementation that uses this client. So, the change doesn't need to be made in this PR.
@amrit110 Please chime in here as well
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.
Yeah that makes sense. We are just using the Google genai client because we are using gcp for the bootcamp, but I was actually telling Ethan we may use this repo for some applied research work as well, where we might switch out the judge model. So good to keep using the openai client.
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.
Ok that makes sense. I also prefer the OpenAI API, the calls and the returns are way simpler to work with and the trace reports to langfuse were better.
I'll revert this change here and merge since you will need it later.
amrit110
left a comment
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.
Seems good.
Summary
Switching from OpenAI SDK to Google ADK on Report Generation agent to match the other agents.
Some extra code was needed to parse the response of the agents which is a bit trickier with Google ADK.
Clickup Ticket(s): NA
Type of Change
Changes Made
.env.exampleand theConfigclass to remove OpenAI env varsTesting
uv run pytest tests/)uv run mypy <src_dir>)uv run ruff check src_dir/)Manual testing details:
Ran the demo UI and the evaluation script
Checklist