Allow AI Presets To Automatically Update Widgets#1954
Conversation
This separates metadata and presets so they can be used independently. This allows preset updates to be reflected since they are no longer overwritten by metadata.
Since metadata was previously overridden with preset data, this must be cleared out so the new change can work properly in existing blocks.
WalkthroughThe changes involve updates to backend database migration scripts and frontend AI preset management. The SQL migration script for the "up" direction modifies the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
db/migrations-wstore/000008_aimeta.up.sql (1)
5-16: Simplify the migration by using only the wildcard pattern.The wildcard pattern
$.meta.ai:*on line 5 will remove all keys starting withai:, making the specific key removals on lines 6-16 redundant.- '$.meta.ai:*', - '$.meta.ai:apitype', - '$.meta.ai:baseurl', - '$.meta.ai:apitoken', - '$.meta.ai:name', - '$.meta.ai:model', - '$.meta.ai:orgid', - '$.meta.ai:apiversion', - '$.meta.ai:maxtokens', - '$.meta.timeoutms', - '$.meta.fontsize', - '$.meta.fixedfontsize' + '$.meta.ai:*'frontend/app/view/waveai/waveai.tsx (1)
388-414: Consider adding detailed error logging.While the error handling is solid, consider adding detailed logging to help with troubleshooting AI response failures. This could include:
- The specific AI model being used
- The preset configuration
- The full error stack trace
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
db/migrations-wstore/000008_aimeta.down.sql(1 hunks)db/migrations-wstore/000008_aimeta.up.sql(1 hunks)frontend/app/view/waveai/waveai.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- db/migrations-wstore/000008_aimeta.down.sql
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Build for TestDriver.ai
- GitHub Check: Analyze (go)
🔇 Additional comments (2)
db/migrations-wstore/000008_aimeta.up.sql (1)
1-17: Verify data backup before running this migration.This migration permanently removes AI-related metadata. Ensure you have a backup of the data before running this migration, as it's irreversible.
frontend/app/view/waveai/waveai.tsx (1)
166-170: LGTM! Clean implementation of preset handling.The changes correctly implement the new preset handling by:
- Reading the preset key from metadata or global settings
- Retrieving the selected preset from the config
- Merging the preset settings before metadata, allowing metadata to override preset values
This aligns perfectly with the PR objectives of allowing AI presets to automatically update widgets while still permitting manual overrides via metadata.
This function includes proper handling of prefix:* meta items.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/util/util.ts (1)
332-379: LGTM! Consider adding JSDoc comments.The implementation is correct and handles both section:* keys and regular keys appropriately. The function maintains immutability by creating a new object.
Add JSDoc comments to improve documentation:
+/** + * Merges two metadata objects, handling both section:* keys and regular keys. + * @param meta - The base metadata object + * @param metaUpdate - The metadata object to merge with the base + * @returns A new metadata object with the merged values + */ function mergeMeta(meta: MetaType, metaUpdate: MetaType): MetaType {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/app/view/waveai/waveai.tsx(2 hunks)frontend/util/util.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
- GitHub Check: Build for TestDriver.ai
🔇 Additional comments (3)
frontend/util/util.ts (1)
401-401: LGTM!The function is correctly exported.
frontend/app/view/waveai/waveai.tsx (2)
14-14: LGTM!The
mergeMetafunction is correctly imported.
163-185: LGTM! The implementation aligns with PR objectives.The changes correctly implement the new preset handling logic:
- Retrieves presets based on
presetKey.- Uses
mergeMetato merge settings, presets, and metadata.- Correctly extracts AI options from merged metadata.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/app/view/waveai/waveai.tsx (1)
166-182: LGTM! The AI options construction is well-implemented.The changes correctly implement the new preset merging logic, ensuring that settings, presets, and block metadata are properly combined with the correct precedence.
Consider extracting the default timeout value to a constant for better maintainability.
+const DEFAULT_AI_TIMEOUT_MS = 60000; + const opts: WaveAIOptsType = { model: mergedPresets["ai:model"] ?? null, apitype: mergedPresets["ai:apitype"] ?? null, orgid: mergedPresets["ai:orgid"] ?? null, apitoken: mergedPresets["ai:apitoken"] ?? null, apiversion: mergedPresets["ai:apiversion"] ?? null, maxtokens: mergedPresets["ai:maxtokens"] ?? null, - timeoutms: mergedPresets["ai:timeoutms"] ?? 60000, + timeoutms: mergedPresets["ai:timeoutms"] ?? DEFAULT_AI_TIMEOUT_MS, baseurl: mergedPresets["ai:baseurl"] ?? null, };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/app/view/waveai/waveai.tsx(2 hunks)frontend/util/util.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Build for TestDriver.ai
- GitHub Check: Analyze (go)
🔇 Additional comments (2)
frontend/util/util.ts (1)
332-401: LGTM! ThemergeMetafunction is well-implemented.The function correctly handles metadata merging with prefix filtering, section deletion, and value updates. The implementation is robust and handles all edge cases appropriately.
frontend/app/view/waveai/waveai.tsx (1)
14-14: LGTM! Import statement correctly updated.The
mergeMetafunction is correctly imported from the utility module.
Fixes ai:timeoutms, ai:fontsize, and ai:fixedfontsize. Also only performs the migration on blocks where the view is waveai.
This change allows the presets to apply these changes in addition to global and meta settings.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/app/view/waveai/waveai.tsx (1)
450-454: Remove debug console.log statement.The font size implementation looks good, but the debug console.log statement should be removed.
const fontSize = useAtomValue(model.mergedPresets)?.["ai:fontsize"]; - console.log("font size is:", fontSize); //const fontSize = useOverrideConfigAtom(model.blockId, "ai:fontsize"); const fixedFontSize = useAtomValue(model.mergedPresets)?.["ai:fixedfontsize"];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/app/view/waveai/waveai.tsx(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
- GitHub Check: Build for TestDriver.ai
🔇 Additional comments (3)
frontend/app/view/waveai/waveai.tsx (3)
11-11: LGTM! Import changes align with new functionality.The addition of
globalStoreandmergeMetaimports supports the new preset handling mechanism.Also applies to: 14-14
70-70: Well-structured preset merging implementation!The new
mergedPresetsatom effectively:
- Separates preset data from metadata
- Maintains proper override precedence (settings → presets → metadata)
- Aligns with the database migration that removes "ai:" prefixed keys
Also applies to: 164-176
178-190: Clean refactor of AI options handling!The updated
aiOptsatom now correctly sources its values from the merged presets while maintaining proper defaults.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/app/view/waveai/waveai.tsx (1)
450-451: Consider memoizing font size values.While the implementation correctly uses merged presets for font sizes, these values could be memoized to prevent unnecessary re-renders of the Markdown components.
- const fontSize = useAtomValue(model.mergedPresets)?.["ai:fontsize"]; - const fixedFontSize = useAtomValue(model.mergedPresets)?.["ai:fixedfontsize"]; + const fontSizes = useMemo(() => ({ + fontSize: useAtomValue(model.mergedPresets)?.["ai:fontsize"], + fixedFontSize: useAtomValue(model.mergedPresets)?.["ai:fixedfontsize"] + }), [model.mergedPresets]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/app/view/waveai/waveai.tsx(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Build for TestDriver.ai
- GitHub Check: Analyze (go)
🔇 Additional comments (2)
frontend/app/view/waveai/waveai.tsx (2)
164-176: LGTM! Well-structured preset merging implementation.The implementation correctly merges settings, selected presets, and metadata in a clear order of precedence, aligning with the PR's goal of separating preset data from metadata while maintaining override capabilities.
178-192: LGTM! Clean refactor to use merged presets.The implementation cleanly extracts AI options from the merged presets while maintaining proper defaults and type safety.
This change makes it so changes to the presets file are no longer being written to metadata. Instead, the preset data is read separately (although it is still possible to override it with metadata if done manually). Because presets are being tracked separately, if the associated metadata key is not set, changes to the `presets/ai.json` file will be applied immediately without switching the preset choice. Note that `ai:preset` is still used in metadata to associate a block with a preset. Additionallly, this introduces a database migration to clear out the metadata items starting with `ai:` except for `ai:preset`. This will allow the change to apply to existing blocks in addition to new ones.
1 participant
This change makes it so changes to the presets file are no longer being written to metadata. Instead, the preset data is read separately (although it is still possible to override it with metadata if done manually).
Because presets are being tracked separately, if the associated metadata key is not set, changes to the
presets/ai.jsonfile will be applied immediately without switching the preset choice. Note thatai:presetis still used in metadata to associate a block with a preset.Additionallly, this introduces a database migration to clear out the metadata items starting with
ai:except forai:preset. This will allow the change to apply to existing blocks in addition to new ones.