-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Merge master into canary #928
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
the new settings window does not list all available gemini embedding models. this happens because some gemini embedding models have `embedContent` instead of `embedText`
fix: list all available gemini embedding models
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3 issues found across 21 files
Prompt for AI agents (all 3 issues)
Understand the root cause of the following 3 issues and fix them.
<file name="src/lib/config/index.ts">
<violation number="1" location="src/lib/config/index.ts:16">
Renaming the persisted config section from `general` to `preferences`/`personalization` without a migration means existing installs keep their settings under `general`, so theme/system-instruction values disappear when the app reloads the config.</violation>
</file>
<file name="src/components/Setup/SetupConfig.tsx">
<violation number="1" location="src/components/Setup/SetupConfig.tsx:66">
hasProviders excludes providers without chat models and any "Transformers" provider, so users with only embedding or Transformers connections cannot proceed through setup.</violation>
</file>
<file name="src/components/Settings/Sections/Personalization.tsx">
<violation number="1" location="src/components/Settings/Sections/Personalization.tsx:19">
Accessing browser-only APIs like localStorage during render will crash this component when the server evaluates it. Wrap the access in a window check (or otherwise guard it) so SSR never touches localStorage.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| version: this.configVersion, | ||
| setupComplete: false, | ||
| general: {}, | ||
| preferences: {}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renaming the persisted config section from general to preferences/personalization without a migration means existing installs keep their settings under general, so theme/system-instruction values disappear when the app reloads the config.
Prompt for AI agents
Address the following comment on src/lib/config/index.ts at line 16:
<comment>Renaming the persisted config section from `general` to `preferences`/`personalization` without a migration means existing installs keep their settings under `general`, so theme/system-instruction values disappear when the app reloads the config.</comment>
<file context>
@@ -13,14 +13,15 @@ class ConfigManager {
version: this.configVersion,
setupComplete: false,
- general: {},
+ preferences: {},
+ personalization: {},
modelProviders: [],
</file context>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is saved in localstorage so no migration needed
| }; | ||
|
|
||
| const hasProviders = providers.length > 0; | ||
| const visibleProviders = providers.filter( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hasProviders excludes providers without chat models and any "Transformers" provider, so users with only embedding or Transformers connections cannot proceed through setup.
Prompt for AI agents
Address the following comment on src/components/Setup/SetupConfig.tsx at line 66:
<comment>hasProviders excludes providers without chat models and any "Transformers" provider, so users with only embedding or Transformers connections cannot proceed through setup.</comment>
<file context>
@@ -63,7 +63,11 @@ const SetupConfig = ({
};
- const hasProviders = providers.length > 0;
+ const visibleProviders = providers.filter(
+ (p) => p.name.toLowerCase() !== 'transformers',
+ );
</file context>
| field={field} | ||
| value={ | ||
| (field.scope === 'client' | ||
| ? localStorage.getItem(field.key) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Accessing browser-only APIs like localStorage during render will crash this component when the server evaluates it. Wrap the access in a window check (or otherwise guard it) so SSR never touches localStorage.
Prompt for AI agents
Address the following comment on src/components/Settings/Sections/Personalization.tsx at line 19:
<comment>Accessing browser-only APIs like localStorage during render will crash this component when the server evaluates it. Wrap the access in a window check (or otherwise guard it) so SSR never touches localStorage.</comment>
<file context>
@@ -0,0 +1,29 @@
+ field={field}
+ value={
+ (field.scope === 'client'
+ ? localStorage.getItem(field.key)
+ : values[field.key]) ?? field.default
+ }
</file context>
| ? localStorage.getItem(field.key) | |
| ? (typeof window !== 'undefined' ? window.localStorage.getItem(field.key) : undefined) |
Summary by cubic
Revamps the Settings experience with new Preferences and Personalization sections, renames Providers to Connections, and adds an auto media search toggle. Also fixes Gemini embedding model detection and updates docs/assets.
New Features
Bug Fixes
Written for commit 70c1f72. Summary will update automatically on new commits.