Fixed an issue where AI Reports are grayed out after setting an API k…#9724
Fixed an issue where AI Reports are grayed out after setting an API k…#9724dpage wants to merge 1 commit intopgadmin-org:masterfrom
Conversation
…ey by auto-selecting the default provider. pgadmin-org#9694 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
WalkthroughA bug-fix note is added to release notes documenting Issue Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/pgadmin/preferences/__init__.py`:
- Around line 246-259: The auto-selection block using _provider_map (checking
data['name'] and setting ai_module.preference('default_provider') via
dp_pref.set(...)) must not override an explicit "None/Disabled" choice in the
same save payload; add a guard that early-skips this inference when the incoming
pref_data (the payload being processed) already contains a 'default_provider'
key (including empty string ''), and apply the same guard to the corresponding
logic in update() (the similar block at 339-352) so dp_pref.set is only
considered if the payload does not include an explicit default_provider update.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3843bcfe-52de-4cc1-882b-1782014c5733
📒 Files selected for processing (2)
docs/en_US/release_notes_9_14.rstweb/pgadmin/preferences/__init__.py
| # Auto-select the default LLM provider when an API key/URL is | ||
| # configured and no provider has been selected yet. | ||
| _provider_map = { | ||
| 'anthropic_api_key_file': 'anthropic', | ||
| 'openai_api_key_file': 'openai', | ||
| 'ollama_api_url': 'ollama', | ||
| 'docker_api_url': 'docker', | ||
| } | ||
| if res and data['name'] in _provider_map and data['value']: | ||
| ai_module = Preferences.module('ai') | ||
| if ai_module: | ||
| dp_pref = ai_module.preference('default_provider') | ||
| if dp_pref and not dp_pref.get(): | ||
| dp_pref.set(_provider_map[data['name']]) |
There was a problem hiding this comment.
Don't let auto-selection override an explicit None (Disabled) choice.
If the same save payload contains default_provider='' and one of these API key/URL fields, this logic can still set default_provider back to a provider later in the loop. That makes the outcome depend on payload order and can silently re-enable AI after the user explicitly disabled it. Please skip the inference whenever pref_data already includes a default_provider update.
Possible guard
+ explicit_provider_choice = any(
+ item.get('name') == 'default_provider' for item in pref_data
+ )
for data in pref_data:
...
- if res and data['name'] in _provider_map and data['value']:
+ if (res and not explicit_provider_choice and
+ data['name'] in _provider_map and
+ str(data['value']).strip()):
ai_module = Preferences.module('ai')
if ai_module:
dp_pref = ai_module.preference('default_provider')
if dp_pref and not dp_pref.get():
dp_pref.set(_provider_map[data['name']])Apply the same guard in update().
Based on learnings, DEFAULT_LLM_PROVIDER = '' is intentional so AI stays disabled until the user explicitly selects a provider and supplies credentials.
Also applies to: 339-352
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/pgadmin/preferences/__init__.py` around lines 246 - 259, The
auto-selection block using _provider_map (checking data['name'] and setting
ai_module.preference('default_provider') via dp_pref.set(...)) must not override
an explicit "None/Disabled" choice in the same save payload; add a guard that
early-skips this inference when the incoming pref_data (the payload being
processed) already contains a 'default_provider' key (including empty string
''), and apply the same guard to the corresponding logic in update() (the
similar block at 339-352) so dp_pref.set is only considered if the payload does
not include an explicit default_provider update.
1 participant
…ey by auto-selecting the default provider. #9694
Summary by CodeRabbit
Bug Fixes
Documentation