Fix Gemini/Google provider type mismatch and centralize provider types#595
Fix Gemini/Google provider type mismatch and centralize provider types#595arn0ld87 wants to merge 2 commits into
Conversation
- Fixed `llm_profiles_store.py` to bootstrap Gemini profiles with provider="google". - Introduced `backend/app/contracts/provider_types.py` with `ProviderType` Literal and constants. - Updated all Pydantic models and service logic to use the new centralized types. - Added legacy alias support for "gemini" in resolvers. - Added regression test `backend/tests/contracts/test_provider_types.py`. - Synchronized JSON schemas. Follow-up to audit master issue #571. Co-authored-by: arn0ld87 <212052432+arn0ld87@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request centralizes LLM provider types by introducing a dedicated provider_types.py file with constants and a ProviderType literal, replacing hardcoded strings across the backend and updating JSON schemas. Feedback includes suggestions to use the new constants in ALL_PROVIDER_TYPES and error messages for better consistency, as well as a recommendation to replace the environment-dependent grep command in tests with a cross-platform Python implementation.
| if provider is None: | ||
| raise ValueError( | ||
| "llm_provider.provider must be one of: default, google, openai, custom_openai" | ||
| f"llm_provider.provider must be one of: default, {PROVIDER_GOOGLE}, {PROVIDER_OPENAI}, custom_openai" |
There was a problem hiding this comment.
The error message still contains a hardcoded string custom_openai. Since the goal of this PR is to centralize provider types, consider using a constant for this internal runtime identifier as well to ensure consistency across the codebase.
References
- Use a single canonical source or function to determine entity types or categories across all database operations to ensure consistency and avoid discrepancies.
| cmd = [ | ||
| "grep", "-r", "\"gemini\"", str(app_root) | ||
| ] | ||
|
|
||
| result = subprocess.run(cmd, capture_output=True, text=True) |
There was a problem hiding this comment.
Using subprocess.run with grep makes the test suite dependent on the host environment (e.g., it will fail on Windows without a Unix-like shell). It is better to implement this check using pure Python with pathlib.Path.rglob and string searching to ensure portability across different operating systems, consistent with our cross-platform compatibility guidelines.
References
- When implementing platform-specific logic, provide cross-platform fallbacks to maintain compatibility across different operating systems.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
1 participant
Unify Gemini/Google provider type across the backend to fix visibility issues of Gemini secrets and models. Introduced centralized provider types and a regression gate.
Fixes #575
PR created automatically by Jules for task 10704287634959598085 started by @arn0ld87