-
Notifications
You must be signed in to change notification settings - Fork 809
feat(samples): Personalized learning demo improvements #571
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?
feat(samples): Personalized learning demo improvements #571
Conversation
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.
Code Review
This pull request introduces significant and valuable improvements to the personalized learning sample. The move to server-side authentication is a major step forward for security and clarity, and the refactoring of the agent code to remove legacy wrappers simplifies the architecture. The updated README is much more concise and user-friendly. I've found a critical security issue related to disabling SSL verification and a couple of other points regarding a hardcoded project ID and the removal of tests. Overall, great work on enhancing the sample!
Addresses Gemini code review feedback: - Replace disabled SSL verification (CERT_NONE) with certifi CA bundle for secure GitHub content fetches - Remove hardcoded project ID from .firebaserc (use placeholder) - Remove unused dependencies (uvicorn, fastapi, pydantic) - Delete unused agent/Dockerfile
|
|
||
| # Model configuration | ||
| LITELLM_MODEL=gemini-2.5-flash | ||
| GENAI_MODEL=gemini-2.5-flash |
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.
Is this actually used?
I see MODEL_ID in deleted code, but I'm just asking for a 2x check that all the configuration variables line up for you. Can you delete you .env clear your existing auths and run this from this template?
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.
The parent directory .env.template is used, but this one at agent/.env.template is redundant -- I committed a change removing this file.
GENAI_MODEL is referenced in four places (agent.py, api-server.ts, openstax_content.py, deploy.py) but all have a default fallback of gemini-2.5.-flash, so it's optional.
The Quickstart notebook generates a minimal .env with the required environment variables (GOOGLE_CLOUD_PROJECT, AGENT_ENGINE_PROJECT_NUMBER, AGENT_ENGINE_RESOURCE_ID). I'm including samples/personalized-learning/.env.template as documentation for optional settings like GENAI_MODEL, access control (VITE_ALLOWED_DOMAIN/VITE_ALLOWED_EMAILS), and GCS buckets. Happy to remove if you feel it's redundant or confusing.
I confirmed that executing the demo works after having cleared .env and existing auths
|
Additional fixes (commits dbbcbc0, 0bce8e2) after testing end-to-end: While running through the quickstart fresh, I hit issues from the Jan 23 web_core refactor (#512) that extracted shared code from @a2ui/lit into @a2ui/web_core: Local dev — quickstart_setup.sh now builds web_core before lit |
Description
Enhancements to the personalized learning sample:
Pre-launch Checklist
If you need help, consider asking for advice on the discussion board.