Offer Additional AI preferences and update to the latest Gemini models#15400
Conversation
…y for AI models - Add preference to allow user to view API key for AI models - Move to latest gemini models and remove deprecated models
Review Summary by QodoAdd API key preferences and update to latest Gemini models
WalkthroughsDescription• Add API key visibility toggle and optional storage preference • Update deprecated Gemini models to latest versions • Implement keyring-based API key persistence with user control • Add credential store availability detection with tooltip feedback Diagramflowchart LR
A["User Preferences"] -->|"Show/Hide Toggle"| B["API Key Display"]
A -->|"Remember Preference"| C["Keyring Storage"]
C -->|"If Disabled"| D["Delete Stored Keys"]
E["Deprecated Gemini Models"] -->|"Replace"| F["Latest Gemini 3.x Models"]
G["Credential Store"] -->|"Check Availability"| H["Enable/Disable Storage"]
File Changes1. jabgui/src/main/java/org/jabref/gui/preferences/ai/AiTab.java
|
Code Review by Qodo
1. AI key storage lacks tests
|
This comment has been minimized.
This comment has been minimized.
| if (!getRememberApiKey()) { | ||
| return ""; | ||
| } | ||
|
|
||
| try (final Keyring keyring = Keyring.create()) { | ||
| return keyring.getPassword(KEYRING_AI_SERVICE, KEYRING_AI_SERVICE_ACCOUNT + "-" + aiProvider.name()); | ||
| } catch (PasswordAccessException e) { |
There was a problem hiding this comment.
1. Ai key storage lacks tests 📘 Rule violation ✓ Correctness
Core AI preference behavior was changed (API key retrieval/storage now gated by rememberApiKey and keys are deleted when toggled off) without any accompanying test updates in this PR. This risks regressions in preference persistence and keyring interaction behavior.
Agent Prompt
## Issue description
Behavior changed for AI API key persistence (gated by `rememberApiKey` and deletion on toggle-off), but no tests were added/updated in this PR.
## Issue Context
This affects `org.jabref.logic` preference behavior and persistence wiring via `JabRefCliPreferences`, so regression risk is high without coverage.
## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/ai/AiPreferences.java[145-181]
- jablib/src/main/java/org/jabref/logic/preferences/JabRefCliPreferences.java[2037-2042]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Yeah, it would be good to have tests
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
regarding the latest AI models, good thing, this also refs #14197 |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
I think there was a misunderstanding: Issue #15398 was about storing/remembering the API key when switching between providers, not storing/remembering during restarts of JabRef (but that's good too!) Things I noticed when I tested the PR:
|
This comment has been minimized.
This comment has been minimized.
- Remove view api key toggle and place toggle inside the checkbox - Rename "Remember API key" to "Persist API key across sessions" - Change default persist api key preference to true
This comment has been minimized.
This comment has been minimized.
|
Hi @ThiloteE thanks for the review!
I actually tried changing providers after entering keys and the API key is stored in the current upstream. I tried on my main branch just now again to make sure. So I assumed the real issue was to store across restarts. The current code in the dev branch stores the key across restarts regardless i.e. no preference is offered. So I assumed a preference should be offered similar to git where the user chooses if the PAT should persist (as seen in the Network tab)
I'm a fan of keeping the checkbox :), I've renamed it to "Persist API key across sessions" and also moved the API key visibility toggle as suggested |
my bad, you're right I misread that. So do you suggest we scrap the preference entirely? |
I tried it out, this is a bug. Will fix |
…o original behaviour. Add a delete button in the API key field to improve UX.
|
I took a look at Zed and how it stores API keys. It persists across restarts and does not offer a preference (if it does it wasn't in an obvious location). I agree with @ThiloteE, we should store the API key by default. I added a delete icon to the API key field to improve UX (similar to Zed). We should probably remove the same preference wherever else it is offered as well but I think that should be done in a separate PR. |
I am not 100% sure, but yes I would tend to scrap the preference. We have had persistence across restarts for the web-search and nobody has ever complained. The only concern I personally have is about multi-user setups and I don't know how API persistence is handled in virtual machines, but that seems like an edge case. In the unlikely case it causes problems, users can report their workflow and their software setup and we then will be able to fix it with a much clearer understanding of the requirements and we then will be able to test it, because we will be able to reproduce the issues. |
|
I think it's more simple:
So I believe the scheme that we worked on is safe enough. The only problem I see is that it's a plain string and not a secret string like in Rust's |
|
API keys belong in the keystore, not in the prefs. See proxy preferences |
|
Sorry, i meant KeyRing |
|
Your pull request conflicts with the target branch. Please merge with your code. For a step-by-step guide to resolve merge conflicts, see https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/addressing-merge-conflicts/resolving-a-merge-conflict-using-the-command-line. |
Related issues and pull requests
Closes #15398, #15392
PR Description
Adds a checkbox for viewing AI provider API key, adds a checkbox giving the user the choice to store API key (as done in git). Earlier the API key was stored by default.
While testing I noticed that the Gemini model used was not supported by google anymore. I've updated them to the latest models provided by google.
Steps to test
Checklist
CHANGELOG.mdin a way that can be understood by the average user (if change is visible to the user)