Add Google engine safety settings and default thinking budget config#559
Add Google engine safety settings and default thinking budget config#559ermanhavuc wants to merge 5 commits intoKochava-Studios:mainfrom
Conversation
📝 WalkthroughWalkthroughThis change adds configuration support for Google LLM engine features including thinking budget defaults and safety filter settings. It introduces localized UI strings, a new config type definition, UI controls in the settings component, service-layer logic to apply these settings, and comprehensive test coverage. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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: 2
🧹 Nitpick comments (2)
tests/unit/renderer/services/llms/google.test.ts (1)
17-24: UseLlmMockinstead of patchingGoogle.prototypehere.This suite reaches into a protected method via
as anyand replacesGoogle.prototype.getGenerationConfigdirectly, which makes the test depend onmulti-llm-tsinternals instead of the repo's standard provider harness. Please switch this toLlmMockso the test follows the unit-test convention used elsewhere.As per coding guidelines: mock LLM providers via 'LlmMock' class.
Also applies to: 31-33
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/renderer/services/llms/google.test.ts` around lines 17 - 24, The test currently reaches into internals by calling (engine as any).getGenerationConfig and by patching Google.prototype; replace that approach with the project's LlmMock harness: construct a GoogleEngine (via createEngine) but register or inject an LlmMock instance that implements the Google provider behavior, then call the engine's public method that delegates to the provider so you can assert the generation config; specifically stop mutating Google.prototype.getGenerationConfig and instead use LlmMock to simulate responses for GoogleEngine.getGenerationConfig (and the similar spots at lines 31-33), ensuring the tests exercise the engine through the standard LlmMock provider interface.src/types/config.ts (1)
53-56: Expose the
GoogleEngineConfigis defined here, butConfiguration.enginesstill only exposes the generic engine union. That leavesstore.config.engines.googleuntyped for these new fields, which is why the renderer, service, and tests all have to drop to casts. WiringBased on learnings: Centralize configuration in
src/types/config.tswith backwards compatibility, organizing engines, plugins, and other settings as Record types.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/types/config.ts` around lines 53 - 56, Update the central Configuration.engines type to explicitly include a google?: GoogleEngineConfig field while preserving the existing general engine shape (e.g., keep the EngineConfig union/index-signature for other engines) so store.config.engines.google is fully typed; locate the Configuration interface/type and add the google key typed to GoogleEngineConfig and ensure backward compatibility by retaining the broad Record<string, EngineConfig> or union for non-google engines.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/services/llms/google.ts`:
- Around line 25-33: The code currently casts googleConfig.safetySettings to
HarmBlockThreshold without validation; update the block that reads
googleConfig.safetySettings (in the file where googleConfig is cast) to
whitelist only the valid HarmBlockThreshold values
(HARM_BLOCK_THRESHOLD_UNSPECIFIED, BLOCK_LOW_AND_ABOVE, BLOCK_MEDIUM_AND_ABOVE,
BLOCK_ONLY_HIGH, BLOCK_NONE, OFF), and if the persisted value is not one of
those, skip setting config.safetySettings so Google defaults apply (optionally
emit a debug/warn via the same logger). Ensure you reference the
googleConfig.safetySettings value, validate against the HarmBlockThreshold set,
and only populate config.safetySettings when the value is valid.
In `@src/renderer/settings/SettingsGoogle.vue`:
- Around line 105-106: The defaultThinkingBudget ref should accept the empty
string Vue emits from a cleared number input and be normalized to undefined
before persisting: change the type of defaultThinkingBudget (currently
ref<number>(null)) to allow number|string (and/or null) so it can hold '' from
the input, and in the save path (the settings save handler that builds
googleConfig and where you currently use the nullish coalescing on
defaultThinkingBudget) convert an empty string ('') to undefined so
googleConfig.defaultThinkingBudget is omitted rather than set to ''. Ensure any
downstream check in google.ts that uses typeof
googleConfig.defaultThinkingBudget !== 'undefined' will therefore treat cleared
values as not configured.
---
Nitpick comments:
In `@src/types/config.ts`:
- Around line 53-56: Update the central Configuration.engines type to explicitly
include a google?: GoogleEngineConfig field while preserving the existing
general engine shape (e.g., keep the EngineConfig union/index-signature for
other engines) so store.config.engines.google is fully typed; locate the
Configuration interface/type and add the google key typed to GoogleEngineConfig
and ensure backward compatibility by retaining the broad Record<string,
EngineConfig> or union for non-google engines.
In `@tests/unit/renderer/services/llms/google.test.ts`:
- Around line 17-24: The test currently reaches into internals by calling
(engine as any).getGenerationConfig and by patching Google.prototype; replace
that approach with the project's LlmMock harness: construct a GoogleEngine (via
createEngine) but register or inject an LlmMock instance that implements the
Google provider behavior, then call the engine's public method that delegates to
the provider so you can assert the generation config; specifically stop mutating
Google.prototype.getGenerationConfig and instead use LlmMock to simulate
responses for GoogleEngine.getGenerationConfig (and the similar spots at lines
31-33), ensuring the tests exercise the engine through the standard LlmMock
provider interface.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b1643ad7-3f0a-4a58-ae6d-a959fb89cff6
📒 Files selected for processing (6)
locales/en.jsonsrc/renderer/services/llms/google.tssrc/renderer/settings/SettingsGoogle.vuesrc/types/config.tstests/unit/renderer/screens/settings_models.test.tstests/unit/renderer/services/llms/google.test.ts
| const googleConfig = this.config as unknown as GoogleEngineConfig | ||
| if (googleConfig.safetySettings) { | ||
| const threshold = googleConfig.safetySettings as HarmBlockThreshold | ||
| config.safetySettings = [ | ||
| { category: HarmCategory.HARM_CATEGORY_HARASSMENT, threshold }, | ||
| { category: HarmCategory.HARM_CATEGORY_HATE_SPEECH, threshold }, | ||
| { category: HarmCategory.HARM_CATEGORY_SEXUALLY_EXPLICIT, threshold }, | ||
| { category: HarmCategory.HARM_CATEGORY_DANGEROUS_CONTENT, threshold }, | ||
| ] |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In @google/genai, what are the valid HarmBlockThresholdvalues accepted byGenerateContentConfig.safetySettings?
💡 Result:
The valid HarmBlockThreshold values accepted by GenerateContentConfig.safetySettings in @google/genai are: HARM_BLOCK_THRESHOLD_UNSPECIFIED, BLOCK_LOW_AND_ABOVE, BLOCK_MEDIUM_AND_ABOVE, BLOCK_ONLY_HIGH, BLOCK_NONE, and OFF. These are string values corresponding to the enum defined in the Gemini API protobufs, used across Google GenAI SDKs including JavaScript (@google/genai). They control the probability threshold at which content is blocked for each harm category in safety settings. - HARM_BLOCK_THRESHOLD_UNSPECIFIED: Uses default threshold (high confidence from multiple official sources). - BLOCK_LOW_AND_ABOVE: Blocks low, medium, high probability harm. - BLOCK_MEDIUM_AND_ABOVE: Blocks medium, high. - BLOCK_ONLY_HIGH: Blocks only high. - BLOCK_NONE: Blocks none (always allow). - OFF: Turns off safety filter. Example usage in @google/genai (inferred from API compatibility and JS examples): const safetySettings = [ { category: "HARM_CATEGORY_HATE_SPEECH", threshold: "BLOCK_LOW_AND_ABOVE" } ]; Official Gemini API docs confirm these values for safetySettings.threshold, and @google/genai wraps the same REST API.
Citations:
- 1: https://ai.google.dev/gemini-api/docs/safety-settings
- 2: https://docs.cloud.google.com/vertex-ai/generative-ai/docs/multimodal/configure-safety-filters
- 3: https://docs.cloud.google.com/vertex-ai/generative-ai/docs/reference/rest/v1/HarmBlockThreshold
- 4: https://docs.cloud.google.com/php/docs/reference/cloud-ai-platform/latest/V1.SafetySetting.HarmBlockThreshold
- 5: https://cloud.google.com/vertex-ai/generative-ai/docs/reference/rest/v1beta1/HarmBlockThreshold
- 6: https://docs.spring.io/spring-ai/docs/current-SNAPSHOT/api/org/springframework/ai/google/genai/common/GoogleGenAiSafetySetting.HarmBlockThreshold.html
🏁 Script executed:
fd -t f "google.ts" src/renderer/services/llmsRepository: nbonamy/witsy
Length of output: 93
🏁 Script executed:
fd -t f "SettingsGoogle.vue" src/rendererRepository: nbonamy/witsy
Length of output: 97
🏁 Script executed:
head -60 src/renderer/services/llms/google.tsRepository: nbonamy/witsy
Length of output: 1587
🏁 Script executed:
cat src/renderer/settings/SettingsGoogle.vue | head -100Repository: nbonamy/witsy
Length of output: 5112
🏁 Script executed:
grep -A 20 -B 5 "safetySettings" src/renderer/settings/SettingsGoogle.vueRepository: nbonamy/witsy
Length of output: 4567
🏁 Script executed:
grep -r "HARM_BLOCK_THRESHOLD_UNSPECIFIED" src/Repository: nbonamy/witsy
Length of output: 39
🏁 Script executed:
cat -n src/renderer/services/llms/google.ts | sed -n '20,40p'Repository: nbonamy/witsy
Length of output: 958
Validate safetySettings before using it with the Google API.
Line 27 blindly casts safetySettings from persisted config to HarmBlockThreshold without validation. A hand-edited config file with an invalid threshold value will cause API errors. The UI restricts options to valid values, but the persisted setting can be modified outside the application. Add validation to whitelist known values and skip safety settings if an invalid value is encountered so the provider defaults are used instead.
Valid HarmBlockThreshold values are: HARM_BLOCK_THRESHOLD_UNSPECIFIED, BLOCK_LOW_AND_ABOVE, BLOCK_MEDIUM_AND_ABOVE, BLOCK_ONLY_HIGH, BLOCK_NONE, and OFF.
Suggested guard
import { GenerateContentConfig, HarmBlockThreshold, HarmCategory } from '@google/genai'
import { ChatModel, Google, LlmCompletionOpts } from 'multi-llm-ts'
import { GoogleEngineConfig } from 'types/config'
+const GOOGLE_SAFETY_THRESHOLDS = new Set([
+ 'HARM_BLOCK_THRESHOLD_UNSPECIFIED',
+ 'BLOCK_LOW_AND_ABOVE',
+ 'BLOCK_MEDIUM_AND_ABOVE',
+ 'BLOCK_ONLY_HIGH',
+ 'BLOCK_NONE',
+ 'OFF',
+])
+
export default class GoogleEngine extends Google {- if (googleConfig.safetySettings) {
+ if (googleConfig.safetySettings && GOOGLE_SAFETY_THRESHOLDS.has(googleConfig.safetySettings)) {
const threshold = googleConfig.safetySettings as HarmBlockThreshold
config.safetySettings = [📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const googleConfig = this.config as unknown as GoogleEngineConfig | |
| if (googleConfig.safetySettings) { | |
| const threshold = googleConfig.safetySettings as HarmBlockThreshold | |
| config.safetySettings = [ | |
| { category: HarmCategory.HARM_CATEGORY_HARASSMENT, threshold }, | |
| { category: HarmCategory.HARM_CATEGORY_HATE_SPEECH, threshold }, | |
| { category: HarmCategory.HARM_CATEGORY_SEXUALLY_EXPLICIT, threshold }, | |
| { category: HarmCategory.HARM_CATEGORY_DANGEROUS_CONTENT, threshold }, | |
| ] | |
| import { GenerateContentConfig, HarmBlockThreshold, HarmCategory } from '@google/genai' | |
| import { ChatModel, Google, LlmCompletionOpts } from 'multi-llm-ts' | |
| import { GoogleEngineConfig } from 'types/config' | |
| const GOOGLE_SAFETY_THRESHOLDS = new Set([ | |
| 'HARM_BLOCK_THRESHOLD_UNSPECIFIED', | |
| 'BLOCK_LOW_AND_ABOVE', | |
| 'BLOCK_MEDIUM_AND_ABOVE', | |
| 'BLOCK_ONLY_HIGH', | |
| 'BLOCK_NONE', | |
| 'OFF', | |
| ]) | |
| export default class GoogleEngine extends Google { | |
| // ... other code ... | |
| const googleConfig = this.config as unknown as GoogleEngineConfig | |
| if (googleConfig.safetySettings && GOOGLE_SAFETY_THRESHOLDS.has(googleConfig.safetySettings)) { | |
| const threshold = googleConfig.safetySettings as HarmBlockThreshold | |
| config.safetySettings = [ | |
| { category: HarmCategory.HARM_CATEGORY_HARASSMENT, threshold }, | |
| { category: HarmCategory.HARM_CATEGORY_HATE_SPEECH, threshold }, | |
| { category: HarmCategory.HARM_CATEGORY_SEXUALLY_EXPLICIT, threshold }, | |
| { category: HarmCategory.HARM_CATEGORY_DANGEROUS_CONTENT, threshold }, | |
| ] |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/services/llms/google.ts` around lines 25 - 33, The code
currently casts googleConfig.safetySettings to HarmBlockThreshold without
validation; update the block that reads googleConfig.safetySettings (in the file
where googleConfig is cast) to whitelist only the valid HarmBlockThreshold
values (HARM_BLOCK_THRESHOLD_UNSPECIFIED, BLOCK_LOW_AND_ABOVE,
BLOCK_MEDIUM_AND_ABOVE, BLOCK_ONLY_HIGH, BLOCK_NONE, OFF), and if the persisted
value is not one of those, skip setting config.safetySettings so Google defaults
apply (optionally emit a debug/warn via the same logger). Ensure you reference
the googleConfig.safetySettings value, validate against the HarmBlockThreshold
set, and only populate config.safetySettings when the value is valid.
| const safetySettings = ref<string>('') | ||
| const defaultThinkingBudget = ref<number>(null) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Vue 3, what value does v-model.numberproduce for an when the input is cleared?
💡 Result:
In Vue 3, v-model.number on an produces an empty string ("") when the input is cleared.
Citations:
🏁 Script executed:
# Find and read SettingsGoogle.vue to check lines 105-106 and 169-170
find . -name "SettingsGoogle.vue" -type fRepository: nbonamy/witsy
Length of output: 99
🏁 Script executed:
# Find google.ts service file
find . -name "google.ts" -path "*/renderer/services/llms/*" -type fRepository: nbonamy/witsy
Length of output: 95
🏁 Script executed:
# Read SettingsGoogle.vue to check lines 105-106 and 169-170
wc -l ./src/renderer/settings/SettingsGoogle.vueRepository: nbonamy/witsy
Length of output: 103
🏁 Script executed:
# Read the relevant sections of SettingsGoogle.vue
sed -n '100,180p' ./src/renderer/settings/SettingsGoogle.vueRepository: nbonamy/witsy
Length of output: 2952
🏁 Script executed:
# Read google.ts to understand how defaultThinkingBudget is handled
cat -n ./src/renderer/services/llms/google.ts | head -100Repository: nbonamy/witsy
Length of output: 1874
🏁 Script executed:
# Check the template section around lines 57-77 to see the input for defaultThinkingBudget
sed -n '1,100p' ./src/renderer/settings/SettingsGoogle.vueRepository: nbonamy/witsy
Length of output: 5112
Normalize a cleared thinking budget before saving it.
With Vue's .number modifier, clearing the <input type="number"> yields '', not null. Line 170 uses ??, which doesn't catch empty strings, so '' ?? undefined evaluates to '' and persists verbatim. In google.ts, the check typeof googleConfig.defaultThinkingBudget !== 'undefined' treats this empty string as configured and forwards it to Google instead of falling back to automatic mode.
Fix the type annotation to allow the empty string that Vue produces, and normalize it to undefined during save:
Suggested fix
-const defaultThinkingBudget = ref<number>(null)
+const defaultThinkingBudget = ref<number | '' | null>(null)- ;(store.config.engines.google as GoogleEngineConfig).defaultThinkingBudget = defaultThinkingBudget.value ?? undefined
+ ;(store.config.engines.google as GoogleEngineConfig).defaultThinkingBudget =
+ defaultThinkingBudget.value === '' || defaultThinkingBudget.value === null
+ ? undefined
+ : defaultThinkingBudget.value🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/settings/SettingsGoogle.vue` around lines 105 - 106, The
defaultThinkingBudget ref should accept the empty string Vue emits from a
cleared number input and be normalized to undefined before persisting: change
the type of defaultThinkingBudget (currently ref<number>(null)) to allow
number|string (and/or null) so it can hold '' from the input, and in the save
path (the settings save handler that builds googleConfig and where you currently
use the nullish coalescing on defaultThinkingBudget) convert an empty string
('') to undefined so googleConfig.defaultThinkingBudget is omitted rather than
set to ''. Ensure any downstream check in google.ts that uses typeof
googleConfig.defaultThinkingBudget !== 'undefined' will therefore treat cleared
values as not configured.
1 participant
Summary
Verification
Summary by CodeRabbit
Release Notes
New Features
Tests