Add support of presence_penalty and frequency_penalty parameter #773
Conversation
|
Screenshot:
|
|
@josStorer please let me know what do you think ;) |
|
Different APIs have varying default values for presence/frequency penalty. Typically, the default values used by APIs are sufficient for common application scenarios. Setting a global default value for these two parameters might be helpful for specific user scenarios, but it could also negatively impact the performance of certain APIs. Of course, there's nothing wrong with this PR itself. The default values for these two parameters in the OpenAI API are both 0, but this could affect users who utilize custom APIs. In contrast, the temperature parameter is more commonly used, its impact is more predictable, and it is sufficient for most users, which can also reduce the complexity of the software itself. I believe that the target users of ChatGPTBox will hardly ever need to modify these two parameters. If we do decide to provide this feature, it would be best to add a CheckBox component to make it optional, or even better, allow each API to configure these parameters individually. However, due to my personal laziness, and considering the increase in software complexity, along with the fact that I don't think this feature is very important, I have been procrastinating on implementing it. 🤣 |
|
You're right, I can see how to add a CheckBox component for them. Hopefully, with some help from AI assistants, it won’t take much time. 😆 |
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for the presence_penalty and frequency_penalty parameters to various OpenAI API integrations and updates the UI to allow users to adjust these settings while localizing the associated labels.
- Added penalty parameters in the OpenAI, Azure OpenAI, and ChatGPT API calls.
- Updated the configuration defaults and UI controls (including sliders) for these parameters.
- Enhanced localization files for multiple languages to include the new penalty labels.
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/services/apis/openai-api.mjs | Added penalty parameters to the GPT API calls |
| src/services/apis/azure-openai-api.mjs | Added penalty parameters to the Azure OpenAI API |
| src/popup/sections/AdvancedPart.jsx | Introduced UI sliders for adjusting penalty values |
| src/config/index.mjs | Updated default configuration with penalty values |
| src/_locales/*/main.json | Updated localization files to include penalty labels |
|
/review |
|
Qodo Merge was enabled for this repository. To continue using it, please link your Git account with your Qodo account here. PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
|
/improve |
|
Qodo Merge was enabled for this repository. To continue using it, please link your Git account with your Qodo account here. PR Code Suggestions ✨No code suggestions found for the PR. |
|
@codex review |
|
/gemini review |
![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 adds support for presence_penalty and frequency_penalty parameters, giving users more control over the AI's response generation. The changes include adding these parameters to the default configuration, creating UI sliders in the advanced settings, passing them to the OpenAI and Azure OpenAI API calls, and adding the necessary translations. The implementation is well-done. I have one suggestion to refactor the new UI components to reduce code duplication, which will improve maintainability.
| <label> | ||
| {t('Presence Penalty') + `: ${config.presence_penalty}`} | ||
| <input | ||
| type="range" | ||
| min="-2" | ||
| max="2" | ||
| step="0.1" | ||
| value={config.presence_penalty} | ||
| onChange={(e) => { | ||
| const value = parseFloatWithClamp(e.target.value, 0, -2, 2) | ||
| updateConfig({ presence_penalty: value }) | ||
| }} | ||
| /> | ||
| </label> | ||
| <label> | ||
| {t('Frequency Penalty') + `: ${config.frequency_penalty}`} | ||
| <input | ||
| type="range" | ||
| min="-2" | ||
| max="2" | ||
| step="0.1" | ||
| value={config.frequency_penalty} | ||
| onChange={(e) => { | ||
| const value = parseFloatWithClamp(e.target.value, 0, -2, 2) | ||
| updateConfig({ frequency_penalty: value }) | ||
| }} | ||
| /> | ||
| </label> |
There was a problem hiding this comment.
The code for the Presence Penalty and Frequency Penalty sliders is very similar. To improve maintainability and reduce duplication, you can refactor this by creating an array of penalty configurations and mapping over it to render the sliders. This will make the code cleaner and easier to extend if more similar parameters are added in the future.
{[ { label: 'Presence Penalty', key: 'presence_penalty' }, { label: 'Frequency Penalty', key: 'frequency_penalty' }, ].map(({ label, key }) => ( <label key={key}> {t(label) + `: ${config[key]}`} <input type="range" min="-2" max="2" step="0.1" value={config[key]} onChange={(e) => { const value = parseFloatWithClamp(e.target.value, 0, -2, 2) updateConfig({ [key]: value }) }} /> </label> ))}There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
2 participants
The frequency and presence penalties help reduce the likelihood of repetitive token sequences, allowing for more varied and contextually appropriate outputs.
By implementing these options, we can gain finer control over the AI's behavior, leading to improved content generation.
Reference docs: