✨ Implement In-Editor AI Feature: Generate group titles with AI #1864
Conversation
|
@basuabhirup is attempting to deploy a commit to the Typebot Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis pull request introduces several enhancements across multiple components and files within the application. Key changes include the addition of a new icon component Changes
Possibly related PRs
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 (
|
…ntials for the In-App AI features
…e/inEditorAiFeatures
| model WorkspaceAiFeature { | ||
| id String @id @default(cuid()) | ||
| createdAt DateTime @default(now()) | ||
| updatedAt DateTime @default(now()) @updatedAt | ||
| workspace Workspace @relation(fields: [workspaceId], references: [id], onDelete: Cascade) | ||
| workspaceId String | ||
| prompt String | ||
| credential Credentials? @relation(fields: [credentialId], references: [id], onDelete: SetNull) | ||
| credentialId String? | ||
| } | ||
|
|
There was a problem hiding this comment.
Is it really necessary? I would probably just inline that in the Workspace model. Typebot has not a relation-heavy db schema.
There was a problem hiding this comment.
Noted! Yeah we can just add few more columns in the workspace table. I was about to discuss the same with you regarding the final decisions on the db schema.
As we've discussed, we will be incorporating multiple AI Features in a workspace. Each feature will have a default system prompt and a selected credential to start with, so I thought of keeping all these new fields separated from the main workspace table.
Also, I was wondering where to save the default prompts for all the upcoming AI Features. Should we keep them in the server-side memory, or should we create a new table for defaultAiFeatures ?
Since there are multiple ways to get the above things done, I would like to know your preference and feedback on the same.
There was a problem hiding this comment.
Yes, let's keep it simple and have one field for group title auto rename. Let's build for it without thinking of next AI features this should just implement #1577
In Typebot, defaults are always constants in the code.
There was a problem hiding this comment.
Okay, got it 💯 Trying to keep it simple and focusing on implementing #1577 for now. I will share my progress and revert back here if any further help is needed. Thank you @baptisteArno
…e/inEditorAiFeatures
…e/inEditorAiFeatures
|
@baptisteArno The user flow to set / update credentials and prompts for the AI features is implemented with minimum changes in the workspace schema. Please provide your feedback on this implementation from the UX point of view. typebot-set-cred-and-prompt-for-ai-features.mp4I'm now working on the main part of the feature i.e. generating group titles, and will mark the PR as ready once I am done. |
Awesome to see! 🙏 I suggest you focus on making the feature work, I will improve the UI / UX afterwards if it is ok with you? From what I can see for now, this is nice 👍 |
Yeah, sure sure! |
basuabhirup
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (27)
packages/prisma/postgresql/migrations/20241105091035_add_ai_features_on_workspace/migration.sql (1)
1-4: LGTM! Consider adding constraints for robustness.The migration script looks good with appropriate column types and default values. However, consider these improvements for better data integrity and performance:
-- AlterTable ALTER TABLE "Workspace" ADD COLUMN "aiFeatureCredentialId" TEXT, ADD COLUMN "aiFeaturePrompt" TEXT, -ADD COLUMN "inEditorAiFeaturesEnabled" BOOLEAN NOT NULL DEFAULT false; +ADD COLUMN "inEditorAiFeaturesEnabled" BOOLEAN NOT NULL DEFAULT false, +ADD CONSTRAINT "max_prompt_length" CHECK (length("aiFeaturePrompt") <= 2000); + +-- Add index for potential WHERE clause queries +CREATE INDEX "idx_workspace_ai_credential" ON "Workspace"("aiFeatureCredentialId");Consider also:
- Adding a foreign key constraint if
aiFeatureCredentialIdreferences another table- Adding a maximum length constraint on
aiFeatureCredentialIdif there's an expected formatpackages/workspaces/src/defaultAiPrompts.ts (1)
1-10: LGTM! Consider enhancing the prompt with examples.The prompt template is well-structured with clear instructions and constraints. However, it could be enhanced with:
- Example input/output pairs to better guide the AI
- Specific character/word length constraints for titles
- Guidelines for handling edge cases (e.g., mixed content, technical terms)
export const defaultPromptToGenerateGroupTitles = `You are a group title generator that analyzes completed content blocks. When a group is completed: 1. Extract key themes, entities, and actions from the group's content 2. Generate a concise (2-3 words) title that: - Captures the group's main topic or purpose - Uses active voice and specific nouns - Reflects the primary action or relationship 3. Exclude generic words like "section," "group," or "block" -The title should be specific enough to distinguish the content while remaining broad enough to accommodate minor content changes.`; +The title should be specific enough to distinguish the content while remaining broad enough to accommodate minor content changes. + +Examples: +Input: "Ask for user's name, email, and phone number" +Output: "Contact Information" + +Input: "Calculate total price based on quantity and apply discount if applicable" +Output: "Price Calculation" + +Guidelines for edge cases: +- For technical content: Preserve technical terms but ensure readability +- For mixed content: Focus on the primary purpose +- Maximum title length: 30 characters +`;packages/groups/src/schemas.ts (1)
Line range hint
13-31: Consider adding JSDoc comments for the schema.While the schema structure is clear, adding JSDoc comments would improve documentation, especially for the new
generatingTitlefield and its relationship to the AI feature.Consider adding documentation like this:
export const groupV6Schema = z .object({ id: z.string(), title: z.string(), + /** Indicates whether an AI-powered title generation is in progress */ generatingTitle: z.boolean().optional(), graphCoordinates: z.object({ x: z.number(), y: z.number(), }), blocks: z.array(blockSchemaV6), })apps/builder/src/helpers/server/routers/publicRouter.ts (1)
31-31: Consider future extensibility in router design.Given the planned enhancements mentioned in the PR (AI model selection, usage limits), consider structuring the
aiFeaturesrouter to easily accommodate these future features. Some considerations:
- Ensure the router can handle model-specific configurations
- Plan for usage tracking and rate limiting middleware
- Consider versioning the API endpoints for backward compatibility
apps/builder/src/features/workspace/api/updateWorkspace.ts (3)
27-29: Add validation constraints for AI-related fields.Consider adding the following validations to ensure data quality:
aiFeaturePrompt: Add max length constraint and non-empty validationaiFeatureCredentialId: Add format validation if it follows a specific pattern- aiFeaturePrompt: z.string().optional(), - aiFeatureCredentialId: z.string().optional(), + aiFeaturePrompt: z.string().min(1).max(1000).optional(), + aiFeatureCredentialId: z.string().uuid().optional(),
Line range hint
39-57: Reorder authorization check before database update.The current implementation performs the authorization check after the database update. While the update is properly scoped with the user ID filter, it's better to verify authorization before making any database changes.
Consider restructuring the flow:
- First, fetch and verify workspace access
- Then, perform the authorization check
- Finally, update the workspace if authorized
.mutation(async ({ input: { workspaceId, ...updates }, ctx: { user } }) => { + const workspace = await prisma.workspace.findFirst({ + where: { members: { some: { userId: user.id } }, id: workspaceId }, + include: { members: true }, + }); + + if (!workspace) + throw new TRPCError({ + code: "NOT_FOUND", + message: "Workspace not found", + }); + + if (isAdminWriteWorkspaceForbidden(workspace, user)) + throw new TRPCError({ + code: "FORBIDDEN", + message: "You are not allowed to update this workspace", + }); + await prisma.workspace.updateMany({ where: { members: { some: { userId: user.id } }, id: workspaceId }, data: updates, }); - - const workspace = await prisma.workspace.findFirst({ - where: { members: { some: { userId: user.id } }, id: workspaceId }, - include: { members: true }, - }); - - if (!workspace) - throw new TRPCError({ - code: "NOT_FOUND", - message: "Workspace not found", - }); - - if (isAdminWriteWorkspaceForbidden(workspace, user)) - throw new TRPCError({ - code: "FORBIDDEN", - message: "You are not allowed to update this workspace", - }); return { workspace, }; });
Line range hint
8-17: Update OpenAPI metadata to document new AI-related fields.The OpenAPI metadata should be updated to include documentation for the new AI-related fields to help API consumers understand their purpose and usage.
.meta({ openapi: { method: "PATCH", path: "/v1/workspaces/{workspaceId}", protect: true, summary: "Update workspace", + description: "Update workspace settings including AI feature configuration", tags: ["Workspace"], }, })packages/workspaces/src/schemas.ts (2)
42-44: LGTM! Consider enhancing field validation.The new AI-related fields are correctly typed and integrated into the workspace schema. However, consider adding:
- A default value for
inEditorAiFeaturesEnabled- Length and format validation for
aiFeaturePrompt- Format validation for
aiFeatureCredentialId- inEditorAiFeaturesEnabled: z.boolean(), - aiFeaturePrompt: z.string().nullable(), - aiFeatureCredentialId: z.string().nullable(), + inEditorAiFeaturesEnabled: z.boolean().default(false), + aiFeaturePrompt: z.string().max(1000).nullable(), + aiFeatureCredentialId: z.string().uuid().nullable(),
42-44: Add JSDoc comments to document the new fields.To improve maintainability, consider adding documentation that explains the purpose and usage of these new AI-related fields.
+ /** Flag to enable/disable AI features in the editor */ inEditorAiFeaturesEnabled: z.boolean(), + /** Custom prompt template for AI operations */ aiFeaturePrompt: z.string().nullable(), + /** Reference to the credential used for AI API calls */ aiFeatureCredentialId: z.string().nullable(),apps/builder/src/features/workspace/components/WorkspaceSettingsForm.tsx (1)
34-39: Consider adding error handling and loading stateWhile the function implementation is correct, consider these improvements for better user experience:
- Add error handling for failed workspace updates
- Manage loading state during the update
const updateInEditorAiFeaturesEnabled = ( inEditorAiFeaturesEnabled: boolean, ) => { if (!workspace?.id) return; - updateWorkspace({ inEditorAiFeaturesEnabled }); + try { + setIsUpdating(true); + await updateWorkspace({ inEditorAiFeaturesEnabled }); + } catch (error) { + toast({ + title: "Failed to update AI features setting", + status: "error", + }); + } finally { + setIsUpdating(false); + } };apps/builder/src/features/workspace/WorkspaceProvider.tsx (1)
182-188: Consider enhancing error handling and validation.While the implementation is correct, consider these improvements:
- Add validation for AI-related parameters (e.g., prompt length, credential format).
- Implement specific error handling for AI feature updates.
- Add debug logging for important workspace changes.
Example enhancement:
const updateWorkspace = (updates: { icon?: string; name?: string; inEditorAiFeaturesEnabled?: boolean; aiFeaturePrompt?: string; aiFeatureCredentialId?: string; }) => { if (!workspaceId) return; + if (updates.aiFeaturePrompt && updates.aiFeaturePrompt.length > 1000) { + showToast({ + description: "AI feature prompt exceeds maximum length of 1000 characters" + }); + return; + } + console.debug('Updating workspace:', workspaceId, updates); updateWorkspaceMutation.mutate({ workspaceId, ...updates, }); };apps/builder/src/features/graph/components/nodes/group/GroupNode.tsx (1)
239-256: Consider extracting animation styles to a shared stylesheet.The pulse animation is well-implemented, but consider moving it to a shared stylesheet for better reusability across components. This would also improve maintainability and consistency.
Example of how to extract the animation:
+ // In a shared styles file (e.g., animations.ts) + export const pulseAnimation = { + animation: "pulse 0.5s ease-in-out infinite", + "@keyframes pulse": { + "0%": { opacity: 0.6 }, + "50%": { opacity: 1 }, + "100%": { opacity: 0.6 }, + }, + WebkitAnimation: "pulse 0.5s ease-in-out infinite", + MozAnimation: "pulse 0.5s ease-in-out infinite", + }; - sx={{ - animation: "pulse 0.5s ease-in-out infinite", - "@keyframes pulse": { - "0%": { opacity: 0.6 }, - "50%": { opacity: 1 }, - "100%": { opacity: 0.6 }, - }, - WebkitAnimation: "pulse 0.5s ease-in-out infinite", - MozAnimation: "pulse 0.5s ease-in-out infinite", - }} + sx={pulseAnimation}packages/prisma/postgresql/schema.prisma (3)
92-94: Consider adding a foreign key constraint for aiFeatureCredentialId.While the current implementation works, adding a foreign key constraint to the Credentials table would ensure referential integrity and prevent orphaned references.
- aiFeatureCredentialId String? + aiFeatureCredentialId String? + aiFeatureCredential Credentials? @relation("AiFeatureCredential", fields: [aiFeatureCredentialId], references: [id])
92-94: Consider more specific field names for group title generation.Since this implementation is specifically for group title generation (issue #1577), consider making the field names more specific to avoid confusion when adding more AI features in the future.
- inEditorAiFeaturesEnabled Boolean @default(false) - aiFeaturePrompt String? - aiFeatureCredentialId String? + groupTitleGenerationEnabled Boolean @default(false) + groupTitlePrompt String? + groupTitleCredentialId String?
92-94: Add documentation comments for the new fields.Consider adding documentation comments to explain the purpose of each field and any constraints or requirements.
+ /// Flag to enable/disable AI-powered group title generation inEditorAiFeaturesEnabled Boolean @default(false) + /// System prompt used for generating group titles aiFeaturePrompt String? + /// Reference to the credential used for AI API calls aiFeatureCredentialId String?apps/builder/src/components/icons.tsx (1)
709-709: Consider normalizing the SVG path data format.The path data uses absolute coordinates and uppercase commands (M, L, Z) while other icons in the file use relative coordinates and lowercase commands. Consider normalizing the format for consistency:
- <path d="M10,21.236,6.755,14.745.264,11.5,6.755,8.255,10,1.764l3.245,6.491L19.736,11.5l-6.491,3.245ZM18,21l1.5,3L21,21l3-1.5L21,18l-1.5-3L18,18l-3,1.5ZM19.333,4.667,20.5,7l1.167-2.333L24,3.5,21.667,2.333,20.5,0,19.333,2.333,17,3.5Z" /> + <path d="m10 21.236-3.245-6.491-6.491-3.245 6.491-3.245 3.245-6.491 3.245 6.491 6.491 3.245-6.491 3.245zm8-0.236 1.5 3 1.5-3 3-1.5-3-1.5-1.5-3-1.5 3-3 1.5zm1.333-16.333 1.167 2.333 1.167-2.333 2.333-1.167-2.333-1.167-1.167-2.333-1.167 2.333-2.333 1.167z" />apps/builder/src/features/editor/api/generateGroupTitle.ts (4)
60-61: Verify thatapiKeyis present in decrypted credentialsAfter decrypting the credentials, ensure that the
apiKeyexists. IfapiKeyis undefined or null, subsequent calls to the AI providers will fail. Adding a check will prevent unexpected errors and improve error handling.Apply this diff to add the check:
const credentialsData = await decrypt(credentials.data, credentials.iv); const apiKey = (credentialsData as any).apiKey; +if (!apiKey) { + throw new TRPCError({ + code: "NOT_FOUND", + message: "API key not found in credentials", + }); +}
71-71: Address the TODO: Allow users to select AI modelsThere's a TODO comment indicating that the model selection functionality needs improvement. Implementing this feature will enhance user flexibility and improve the overall usability of the AI features.
Would you like assistance in implementing user selection of AI models? I can help draft the necessary changes or open a GitHub issue to track this enhancement.
112-112: Review error logging to prevent leaking sensitive informationWhen logging errors, ensure that sensitive information (such as API keys or user data) is not inadvertently logged. Consider logging only the error message or sanitizing the error details to prevent potential exposure of confidential data.
Apply this diff to improve error logging:
try { const generatedTitle = await generateTitle(); return { title: generatedTitle }; } catch (error) { - console.error("Error generating group title:", error); + console.error("Error generating group title:", error.message); throw new TRPCError({ code: "INTERNAL_SERVER_ERROR", message: "Failed to generate group title", }); }
109-110: Consider adding unit tests forgenerateGroupTitleAdding unit tests will help ensure that the
generateGroupTitleprocedure handles various inputs and edge cases correctly, improving code reliability and maintainability.Do you want assistance in creating unit tests for this procedure? I can help set up test cases to cover typical and edge-case scenarios.
apps/builder/src/features/graph/components/edges/DrawingEdge.tsx (1)
144-145: Optimize deep cloning ofgroupContentConsider using a more efficient method for deep cloning
groupContentinstead ofJSON.parse(JSON.stringify(...)). If you have a utility function likecloneDeepin your codebase, it would be more performant and reliable.You can apply this change:
- const groupContent = JSON.parse(JSON.stringify(typebot.groups[groupIndex])); + const groupContent = cloneDeep(typebot.groups[groupIndex]);apps/builder/src/features/workspace/components/InEditorAIFeatures.tsx (6)
79-82: Remove unused parametercredentialsTypeinsaveAiFeaturesCredentialfunctionThe parameter
credentialsTypeis declared but not used within thesaveAiFeaturesCredentialfunction. Removing it will simplify the function signature and prevent potential confusion.Consider updating the function definition:
-const saveAiFeaturesCredential = ( - credentialId: string, - credentialsType: aiProvidersType, -) => { +const saveAiFeaturesCredential = (credentialId: string) => {Also, update any calls to this function accordingly:
-onNewCredentials={(credentialsId, provider) => - saveAiFeaturesCredential(credentialsId, provider) +onNewCredentials={(credentialsId) => + saveAiFeaturesCredential(credentialsId) }
93-95: Eliminate unused parameterproviderinaddNewAccountfunctionThe
providerparameter in theaddNewAccountfunction is not utilized. SinceselectedAiProvideris already available in the component scope, you can remove the parameter to simplify the function.Update the function definition:
-const addNewAccount = (provider: aiProvidersType) => { +const addNewAccount = () => { onAIModalOpen(); };And adjust the calls to
addNewAccount:-onClick={() => addNewAccount(selectedAiProvider)} +onClick={addNewAccount}
126-134: EnsureuseEffectdependencies are comprehensiveIn the
useEffecthook, you're comparingworkspace.aiFeaturePromptandaiFeaturePrompt, but onlyworkspace?.aiFeaturePromptis included in the dependency array. This might cause the effect to miss updates whenworkspacechanges.Include
workspacein the dependency array to capture changes accurately:-}, [workspace?.aiFeaturePrompt]); +}, [workspace]);Alternatively, if you want to be more specific:
-}, [workspace?.aiFeaturePrompt]); +}, [workspace?.aiFeaturePrompt, aiFeaturePrompt]);
97-100: DebouncesaveAiFeaturesPromptto optimize performanceCalling
saveAiFeaturesPrompton every keystroke can lead to performance issues and excessive network requests. Implementing debouncing will improve efficiency.You can use
lodash.debounceor implement a custom debounce. Here's an example usinguseCallbackandsetTimeout:import { useCallback } from "react"; const debounce = (func, delay) => { let debounceTimer; return (...args) => { clearTimeout(debounceTimer); debounceTimer = setTimeout(() => func(...args), delay); }; }; const debouncedSaveAiFeaturesPrompt = useCallback( debounce((prompt) => { saveAiFeaturesPrompt(prompt); }, 300), [workspace?.id] ); // Use debounced function in onChange handler const handlePromptChange = (prompt: string) => { setAiFeaturePrompt(prompt); debouncedSaveAiFeaturesPrompt(prompt); };
148-169: Ensure consistent use of icon and label componentsThere's inconsistency in using
ForgedBlockIcon/ForgedBlockLabelandBlockIcon/BlockLabelfor AI providers. This might lead to UI inconsistencies.Standardize the components used. For example, use
BlockIconandBlockLabelthroughout:-<ForgedBlockIcon type={selectedAiProvider} /> -<ForgedBlockLabel type={selectedAiProvider} /> +<BlockIcon type={selectedAiProvider} /> +<BlockLabel type={selectedAiProvider} />And update the corresponding imports:
-import { ForgedBlockIcon } from "@/features/forge/ForgedBlockIcon"; -import { ForgedBlockLabel } from "@/features/forge/ForgedBlockLabel"; +import { BlockIcon } from "@/features/editor/components/BlockIcon"; +import { BlockLabel } from "@/features/editor/components/BlockLabel";
71-77: Avoid unnecessary type casting withas stringUsing
as stringmight mask potential null or undefined values, leading to runtime errors. Since the query is enabled only whenworkspace?.idandworkspace?.aiFeatureCredentialIdare truthy, you can safely use non-null assertion (!).Update the code:
-{ - workspaceId: workspace?.id as string, - credentialsId: workspace?.aiFeatureCredentialId as string, -}, +{ + workspaceId: workspace!.id, + credentialsId: workspace!.aiFeatureCredentialId, +},
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
packages/prisma/package.jsonis excluded by!**/*.json
📒 Files selected for processing (17)
apps/builder/src/components/icons.tsx(1 hunks)apps/builder/src/features/credentials/api/getCredentials.ts(3 hunks)apps/builder/src/features/editor/api/generateGroupTitle.ts(1 hunks)apps/builder/src/features/editor/api/router.ts(1 hunks)apps/builder/src/features/editor/providers/typebotActions/edges.ts(2 hunks)apps/builder/src/features/graph/components/edges/DrawingEdge.tsx(3 hunks)apps/builder/src/features/graph/components/nodes/group/GroupNode.tsx(5 hunks)apps/builder/src/features/workspace/WorkspaceProvider.tsx(3 hunks)apps/builder/src/features/workspace/api/updateWorkspace.ts(1 hunks)apps/builder/src/features/workspace/components/InEditorAIFeatures.tsx(1 hunks)apps/builder/src/features/workspace/components/WorkspaceSettingsForm.tsx(4 hunks)apps/builder/src/helpers/server/routers/publicRouter.ts(2 hunks)packages/groups/src/schemas.ts(1 hunks)packages/prisma/postgresql/migrations/20241105091035_add_ai_features_on_workspace/migration.sql(1 hunks)packages/prisma/postgresql/schema.prisma(1 hunks)packages/workspaces/src/defaultAiPrompts.ts(1 hunks)packages/workspaces/src/schemas.ts(1 hunks)
🔇 Additional comments (21)
apps/builder/src/features/editor/api/router.ts (2)
1-2: LGTM! Clean and focused imports.
The imports are well-organized, following a clear pattern of framework utilities followed by local modules.
4-6: Consider adding input validation and error handling.
While the router implementation is clean and follows TRPC patterns, it might benefit from additional safeguards:
- Input validation for the generateGroupTitle procedure
- Error handling for AI service failures
- Rate limiting to prevent abuse
Consider wrapping the procedure with error handling:
export const aiFeaturesRouter = router({
- generateGroupTitle,
+ generateGroupTitle: {
+ ...generateGroupTitle,
+ onError: (opts) => {
+ const { error, ctx } = opts;
+ console.error('Error in generateGroupTitle:', error);
+ throw new Error('Failed to generate group title. Please try again.');
+ }
+ },
});Let's verify if there's any existing error handling in the generateGroupTitle implementation:
packages/workspaces/src/defaultAiPrompts.ts (1)
1-10: Verify prompt effectiveness with test cases.
Let's verify how well this prompt performs with different types of content blocks.
✅ Verification successful
Based on the search results, I can now verify the effectiveness of the prompt for generating group titles. The search shows:
- The prompt is used to generate titles for completed content blocks in groups
- Looking at the codebase, group titles are used extensively throughout the application for:
- Navigation and organization
- Displaying group context in logs and UI
- Referencing groups in webhooks and other integrations
The prompt follows best practices by:
- Focusing on key themes and entities
- Emphasizing active voice and specific nouns
- Avoiding generic words
- Balancing specificity with flexibility
The search results show real examples of group titles in the codebase that align with these guidelines, like:
- "Welcome"
- "Email"
- "Services"
- "Additional information"
These titles are concise, descriptive, and avoid generic terms as specified in the prompt.
The prompt is well-designed and aligns with how group titles are used throughout the application.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for existing group titles to understand naming patterns
# and verify if the AI-generated titles will align with the codebase conventions.
# Test: Look for group title patterns in TypeScript/JavaScript files
rg -g '*.{ts,tsx,js,jsx}' -B 2 -A 2 'title:|groupTitle:|name:|displayName:'
# Test: Look for group-related components or types that might contain title patterns
ast-grep --pattern 'interface $_ {
$$$
title: $_
$$$
}'
Length of output: 420736
packages/groups/src/schemas.ts (2)
22-22: LGTM! Well-structured schema addition for title generation state.
The optional boolean field generatingTitle is appropriately added to track the AI title generation state. Making it optional maintains backward compatibility with existing data.
Line range hint 1-50: Verify the impact of schema version upgrade.
The schema changes look good, particularly the upgrade from blockSchemaV5 to blockSchemaV6. However, we should verify the migration path for existing data.
Let's check for any direct usage of the V5 schema that might need updates:
✅ Verification successful
Let me check the migration path to ensure proper handling of the schema upgrade.
Let me check one final aspect - the actual implementation of the V5 to V6 migration to ensure it properly handles the block schema changes.
Schema upgrade is properly handled with migration path in place
The verification shows a well-implemented migration system:
- The schema changes from V5 to V6 are properly handled in
migrateTypebotFromV5ToV6.ts - Block migrations are managed through
migrateBlocksFromV1ToV2 - Migration is automatically applied in multiple entry points (updateTypebot, getTypebot, importTypebot)
- The migration properly handles both public and private typebot variants
The schema changes are safe to proceed with as there's a clear upgrade path for existing data.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for usages of blockSchemaV5 that might need updates
# Test: Search for blockSchemaV5 usage. Expect: No direct usage outside of migration code
echo "Checking for blockSchemaV5 usage..."
rg "blockSchemaV5" --type ts
echo "Checking for migration handlers..."
rg "typebot.*[Mm]igration" --type ts
Length of output: 1287
Script:
#!/bin/bash
# Check the migration implementation for V5 to V6
echo "Checking V5 to V6 migration implementation..."
rg -A 10 "migrateTypebotFromV5ToV6" --type ts
# Check if there are any tests for the migration
echo "Checking migration tests..."
fd ".*\.test\.ts$" -x rg -l "V5.*V6|blockSchema.*[Mm]igration"
Length of output: 4566
Script:
#!/bin/bash
# Check the full implementation of V5 to V6 migration
echo "Checking full V5 to V6 migration implementation..."
cat packages/typebot/src/migrations/migrateTypebotFromV5ToV6.ts
# Check for any block-specific migrations
echo "Checking for block-specific migrations..."
rg "migrateBlocksFromV" --type ts -A 5
Length of output: 3374
apps/builder/src/helpers/server/routers/publicRouter.ts (1)
8-8: LGTM! Verify router implementation.
The changes follow the established patterns for router configuration. The implementation maintains consistency with the existing codebase structure.
Let's verify the router implementation:
Also applies to: 31-31
apps/builder/src/features/credentials/api/getCredentials.ts (2)
28-28: LGTM! Non-breaking schema enhancement.
The addition of the type field to the output schema is a clean enhancement that maintains backward compatibility.
55-55: Verify database schema includes the 'type' field.
The implementation correctly retrieves and returns the credential type. However, let's verify that the database schema has been properly migrated to include this field.
Also applies to: 69-69
✅ Verification successful
The verification results show that:
- The 'type' field exists in both MySQL and PostgreSQL Prisma schemas as a required String field
- The field is consistently used across the codebase in credential-related operations
- There are appropriate usages of the field in related features
The database schema correctly includes and supports the 'type' field
The implementation is properly backed by the database schema and is used consistently throughout the codebase. The field is properly defined in both MySQL and PostgreSQL schemas, ensuring database compatibility.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the 'type' column exists in the credentials table
# and check its usage across the codebase
# Check if there's a migration file adding the 'type' column
echo "Checking for migration files..."
fd --type f . -e sql -e ts | rg -i "credentials.*type"
# Check the prisma schema definition
echo -e "\nChecking Prisma schema..."
rg "model Credentials" -A 10
# Check other usages of credentials.type to ensure consistent implementation
echo -e "\nChecking other usages of credentials.type..."
rg "credentials.*\.type" --type ts
Length of output: 2274
apps/builder/src/features/workspace/api/updateWorkspace.ts (3)
Line range hint 39-41: LGTM! Database operations are properly scoped.
The database update is correctly scoped to the user's workspace and uses appropriate prisma operations.
27-29: LGTM! API changes maintain backward compatibility.
The new optional fields maintain backward compatibility with existing API consumers.
Line range hint 39-41: Add validation for AI credential existence.
When aiFeatureCredentialId is provided, verify that the credential exists and is accessible to the user before updating the workspace.
Consider adding credential validation:
if (updates.aiFeatureCredentialId) {
const credential = await prisma.credential.findFirst({
where: {
id: updates.aiFeatureCredentialId,
workspaceId,
},
});
if (!credential) {
throw new TRPCError({
code: "NOT_FOUND",
message: "AI feature credential not found",
});
}
}apps/builder/src/features/workspace/components/WorkspaceSettingsForm.tsx (2)
5-5: LGTM: New imports are appropriate
The added imports for SwitchWithLabel and InEditorAIFeatures components align well with the new AI feature toggle functionality.
Also applies to: 22-22
34-39: Verify workspace type definition
Let's ensure the workspace type definition includes the new AI feature fields.
Also applies to: 92-103
✅ Verification successful
Workspace type includes AI feature fields - no issues found
Based on the search results, the workspace type definition includes the inEditorAiFeaturesEnabled field:
- It's defined in the Prisma schema as seen in the database model
- It's properly typed in the workspace schema (
packages/workspaces/src/schemas.ts) as a boolean field - It's correctly used in the workspace provider and form components
- The field is also properly handled in the workspace update API
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for workspace type definition
rg -t typescript "type\s+Workspace\s*=|interface\s+Workspace\s*{" --context 10
Length of output: 236
Script:
#!/bin/bash
# Search for workspace type/interface definition with fixed regex
rg -t typescript "type Workspace|interface Workspace" -B 5 -A 15
Length of output: 101
Script:
#!/bin/bash
# Search for workspace schema definition
rg -t typescript "workspaceSchema|workspace.*schema" -B 5 -A 15
Length of output: 100
Script:
#!/bin/bash
# Search for workspace-related type definitions in prisma schema
fd "schema.prisma" --exec cat {} | grep -A 10 -B 5 "model Workspace"
Length of output: 2516
Script:
#!/bin/bash
# Search for workspace type definitions without file type restriction
rg "type\s+Workspace|interface\s+Workspace|Workspace:\s*{" -B 5 -A 15
Length of output: 241
Script:
#!/bin/bash
# Search for files containing workspace types
fd -e ts -e tsx | xargs rg "Workspace.*{|type.*Workspace|interface.*Workspace" -B 5 -A 15
Length of output: 235
Script:
#!/bin/bash
# Look for workspace-related type definitions in TypeScript files
fd -e ts -e tsx | xargs rg "inEditorAiFeaturesEnabled" -B 5 -A 5
Length of output: 10487
apps/builder/src/features/workspace/WorkspaceProvider.tsx (2)
89-89: Skip review of formatting change.
This change appears to be a formatting modification only.
32-38: LGTM! Verify type usage across the codebase.
The type definition for updateWorkspace is well-structured with proper TypeScript types for the new AI feature parameters.
Let's verify the consistent usage of these new parameters:
✅ Verification successful
Let me verify the implementation details of these components to ensure the type usage is consistent.
Type usage is consistent and properly implemented
The verification shows that all calls to updateWorkspace are using the correct types and properly handling the new AI-related parameters:
WorkspaceSettingsFormcorrectly updates individual fields (name,icon,inEditorAiFeaturesEnabled)InEditorAIFeaturesproperly handlesaiFeaturePromptandaiFeatureCredentialIdupdates with appropriate type safety- All parameters are used as optional fields, maintaining backward compatibility
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any direct calls to updateWorkspace to ensure proper type usage
ast-grep --pattern 'updateWorkspace({$$$})'
Length of output: 1194
Script:
#!/bin/bash
# Check the type definitions and implementations in the components
cat apps/builder/src/features/workspace/components/WorkspaceSettingsForm.tsx apps/builder/src/features/workspace/components/InEditorAIFeatures.tsx
Length of output: 13760
apps/builder/src/features/editor/providers/typebotActions/edges.ts (2)
16-24: LGTM: Well-defined type for group title generation.
The GenerateGroupTitle type is well-structured and properly handles both successful and unsuccessful title generation cases.
27-30: Verify all callers of createEdge are updated.
The createEdge method signature has been modified to require a generateGroupTitle parameter. This is a breaking change that requires updates to all existing callers.
apps/builder/src/features/graph/components/nodes/group/GroupNode.tsx (3)
2-2: LGTM: Clean state management implementation.
The new state management for generatingGroupTitle and the SparklesIcon import are well-implemented, following React best practices.
Also applies to: 55-57
95-102: LGTM: Proper state synchronization.
The useEffect hooks correctly synchronize the local state with prop changes, ensuring the component stays in sync with external updates.
211-212: LGTM: Clean layout implementation.
The layout modifications with flexbox and proper spacing ensure a good balance between the title input and the generation indicator.
Also applies to: 232-238
apps/builder/src/components/icons.tsx (1)
707-711: LGTM! Component structure follows the established pattern.
The SparklesIcon component correctly uses the Chakra UI Icon component and applies the featherIconsBaseProps.
| <Stack spacing="4" mb={4}> | ||
| <SwitchWithLabel | ||
| label={"In-Editor AI Features"} | ||
| initialValue={workspace.inEditorAiFeaturesEnabled} | ||
| onCheckChange={updateInEditorAiFeaturesEnabled} | ||
| moreInfoContent="To enable AI features within this workspace" | ||
| justifyContent="start" | ||
| defaultChecked={!!workspace.inEditorAiFeaturesEnabled} | ||
| /> | ||
|
|
||
| <InEditorAIFeatures /> | ||
| </Stack> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Clean up switch implementation and improve accessibility
The switch implementation has a few issues to address:
- The
defaultCheckedprop is redundant sinceinitialValueis already present - Missing accessibility attributes
- No visual feedback during updates
<Stack spacing="4" mb={4}>
<SwitchWithLabel
label={"In-Editor AI Features"}
initialValue={workspace.inEditorAiFeaturesEnabled}
onCheckChange={updateInEditorAiFeaturesEnabled}
moreInfoContent="To enable AI features within this workspace"
justifyContent="start"
- defaultChecked={!!workspace.inEditorAiFeaturesEnabled}
+ aria-label="Toggle in-editor AI features"
+ isDisabled={isUpdating}
/>
<InEditorAIFeatures />
</Stack>Committable suggestion skipped: line range outside the PR's diff.
| typebot.groups[groupIndex].generatingTitle = true; | ||
| generateGroupTitle(typebot, groupIndex) | ||
| .then((result) => { | ||
| if (!result || !result.title) return; | ||
| setTypebot((typebot) => | ||
| produce(typebot, (typebot) => { | ||
| typebot.groups[groupIndex].title = result.title; | ||
| }), | ||
| ); | ||
| }) | ||
| .catch((error) => { | ||
| console.error(error); | ||
| }) | ||
| .finally(() => { | ||
| setTypebot((typebot) => | ||
| produce(typebot, (typebot) => { | ||
| typebot.groups[groupIndex].generatingTitle = false; | ||
| }), | ||
| ); | ||
| }); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider improving state management and error handling.
The title generation implementation has several areas for improvement:
-
State Management:
- Multiple
setTypebotcalls could be consolidated into a single update - Consider using a reducer pattern for complex state updates
- Multiple
-
Race Conditions:
- No handling of concurrent generation attempts
- Previous generation attempts should be cancelled
-
Error Handling:
- Errors are only logged to console
- Consider providing user feedback for failures
Here's a suggested improvement:
- typebot.groups[groupIndex].generatingTitle = true;
- generateGroupTitle(typebot, groupIndex)
- .then((result) => {
- if (!result || !result.title) return;
- setTypebot((typebot) =>
- produce(typebot, (typebot) => {
- typebot.groups[groupIndex].title = result.title;
- }),
- );
- })
- .catch((error) => {
- console.error(error);
- })
- .finally(() => {
- setTypebot((typebot) =>
- produce(typebot, (typebot) => {
- typebot.groups[groupIndex].generatingTitle = false;
- }),
- );
- });
+ let isCurrentRequest = true;
+ typebot.groups[groupIndex].generatingTitle = true;
+
+ generateGroupTitle(typebot, groupIndex)
+ .then((result) => {
+ if (!isCurrentRequest) return;
+ if (!result?.title) return;
+
+ setTypebot((typebot) =>
+ produce(typebot, (typebot) => {
+ const group = typebot.groups[groupIndex];
+ group.title = result.title;
+ group.generatingTitle = false;
+ }),
+ );
+ })
+ .catch((error) => {
+ if (!isCurrentRequest) return;
+ console.error('Failed to generate group title:', error);
+
+ setTypebot((typebot) =>
+ produce(typebot, (typebot) => {
+ typebot.groups[groupIndex].generatingTitle = false;
+ }),
+ );
+
+ // Consider adding a toast notification here
+ });
+
+ return () => {
+ isCurrentRequest = false;
+ };Consider implementing a custom hook for title generation to better manage the lifecycle and state:
function useGroupTitleGeneration() {
const [isGenerating, setIsGenerating] = useState<Record<string, boolean>>({});
const generateTitle = useCallback(async (
typebot: Draft<TypebotV6>,
groupIndex: number,
generateGroupTitle: GenerateGroupTitle
) => {
// Implementation here
}, []);
return { generateTitle, isGenerating };
}|
|
||
| const aiProvider = credentials.type; | ||
| const savedPrompt = workspace.aiFeaturePrompt; | ||
| const groupContent = JSON.stringify(input.groupContent.blocks); |
There was a problem hiding this comment.
Ensure input.groupContent.blocks is defined before use
Accessing input.groupContent.blocks without verifying its existence could lead to runtime errors if blocks is undefined. Strengthening the input validation, as suggested earlier, will ensure that blocks is always provided and properly structured.
| const groupContent = JSON.stringify(input.groupContent.blocks); | ||
|
|
||
| const generateTitle = async () => { | ||
| const prompt = `${savedPrompt}\n\nGroup Content: ${groupContent}\n\nGenerate a concise and relevant title for this group:`; | ||
|
|
There was a problem hiding this comment.
Ensure privacy of groupContent when sending to AI providers
groupContent may contain sensitive user data. Ensure that sending this data to external AI providers complies with privacy policies and regulations. Consider anonymizing or filtering out sensitive information before including it in the prompt sent to the AI model.
| z.object({ | ||
| workspaceId: z.string(), | ||
| groupIndex: z.number(), | ||
| groupContent: z.any(), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Strengthen input validation for groupContent
Currently, groupContent is typed as z.any(), which allows any input. To ensure the application receives the expected data structure, consider defining a more specific schema for groupContent and its properties, such as blocks. This will help catch invalid inputs early and prevent runtime errors.
Apply this diff to enhance input validation:
.input(
z.object({
workspaceId: z.string(),
groupIndex: z.number(),
- groupContent: z.any(),
+ groupContent: z.object({
+ blocks: z.array(z.any()),
+ // Define other expected properties if necessary
+ }),
}),
)Committable suggestion skipped: line range outside the PR's diff.
| useEffect(() => { | ||
| // Load existing credential if already set | ||
| if ( | ||
| !!workspace?.id && | ||
| !!workspace?.aiFeatureCredentialId && | ||
| !loadingExistingCredential && | ||
| !!existingCredential && | ||
| (selectedAiProvider !== (existingCredential.type as aiProvidersType) || | ||
| selectedAiCredential?.id !== workspace.aiFeatureCredentialId) | ||
| ) { | ||
| setSelectedAiProvider(existingCredential.type as aiProvidersType); | ||
| setSelectedAiCredential({ | ||
| id: workspace.aiFeatureCredentialId, | ||
| name: existingCredential.name, | ||
| }); | ||
| } | ||
| }, [existingCredential]); |
There was a problem hiding this comment.
Include all dependencies in the useEffect dependency array
The useEffect hook depends on several variables (workspace, selectedAiProvider, selectedAiCredential, loadingExistingCredential), but only existingCredential is included in the dependency array. This might lead to stale or inconsistent state updates.
Update the dependency array to include all relevant dependencies:
-}, [existingCredential]);
+}, [
+ existingCredential,
+ workspace?.id,
+ workspace?.aiFeatureCredentialId,
+ selectedAiProvider,
+ selectedAiCredential?.id,
+ loadingExistingCredential,
+]);📝 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.
| useEffect(() => { | |
| // Load existing credential if already set | |
| if ( | |
| !!workspace?.id && | |
| !!workspace?.aiFeatureCredentialId && | |
| !loadingExistingCredential && | |
| !!existingCredential && | |
| (selectedAiProvider !== (existingCredential.type as aiProvidersType) || | |
| selectedAiCredential?.id !== workspace.aiFeatureCredentialId) | |
| ) { | |
| setSelectedAiProvider(existingCredential.type as aiProvidersType); | |
| setSelectedAiCredential({ | |
| id: workspace.aiFeatureCredentialId, | |
| name: existingCredential.name, | |
| }); | |
| } | |
| }, [existingCredential]); | |
| useEffect(() => { | |
| // Load existing credential if already set | |
| if ( | |
| !!workspace?.id && | |
| !!workspace?.aiFeatureCredentialId && | |
| !loadingExistingCredential && | |
| !!existingCredential && | |
| (selectedAiProvider !== (existingCredential.type as aiProvidersType) || | |
| selectedAiCredential?.id !== workspace.aiFeatureCredentialId) | |
| ) { | |
| setSelectedAiProvider(existingCredential.type as aiProvidersType); | |
| setSelectedAiCredential({ | |
| id: workspace.aiFeatureCredentialId, | |
| name: existingCredential.name, | |
| }); | |
| } | |
| }, [ | |
| existingCredential, | |
| workspace?.id, | |
| workspace?.aiFeatureCredentialId, | |
| selectedAiProvider, | |
| selectedAiCredential?.id, | |
| loadingExistingCredential, | |
| ]); |
| {aiProvider === "openai" && ( | ||
| <OpenAICredentialsModal | ||
| isOpen={isOpen} | ||
| onClose={onClose} | ||
| onNewCredentials={(credentialsId) => | ||
| onNewCredentials(credentialsId, aiProvider) | ||
| } | ||
| /> | ||
| )} | ||
| <CreateForgedCredentialsModal | ||
| blockDef={blockDef!} | ||
| isOpen={isOpen} | ||
| onClose={onClose} | ||
| onNewCredentials={(credentialsId) => | ||
| onNewCredentials(credentialsId, aiProvider) | ||
| } | ||
| /> | ||
| </> | ||
| ); |
There was a problem hiding this comment.
Correct modal rendering logic in AICredentialsModal component
Both OpenAICredentialsModal and CreateForgedCredentialsModal might render simultaneously, leading to unexpected behavior. Ensure only the appropriate modal renders based on aiProvider.
Adjust the rendering logic:
return (
<>
- {aiProvider === "openai" && (
+ {aiProvider === "openai" ? (
<OpenAICredentialsModal
isOpen={isOpen}
onClose={onClose}
onNewCredentials={(credentialsId) =>
onNewCredentials(credentialsId)
}
/>
- )}
- <CreateForgedCredentialsModal
+ ) : (
+ <CreateForgedCredentialsModal
blockDef={blockDef!}
isOpen={isOpen}
onClose={onClose}
onNewCredentials={(credentialsId) =>
onNewCredentials(credentialsId)
}
/>
+ )}
</>
);📝 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.
| {aiProvider === "openai" && ( | |
| <OpenAICredentialsModal | |
| isOpen={isOpen} | |
| onClose={onClose} | |
| onNewCredentials={(credentialsId) => | |
| onNewCredentials(credentialsId, aiProvider) | |
| } | |
| /> | |
| )} | |
| <CreateForgedCredentialsModal | |
| blockDef={blockDef!} | |
| isOpen={isOpen} | |
| onClose={onClose} | |
| onNewCredentials={(credentialsId) => | |
| onNewCredentials(credentialsId, aiProvider) | |
| } | |
| /> | |
| </> | |
| ); | |
| {aiProvider === "openai" ? ( | |
| <OpenAICredentialsModal | |
| isOpen={isOpen} | |
| onClose={onClose} | |
| onNewCredentials={(credentialsId) => | |
| onNewCredentials(credentialsId) | |
| } | |
| /> | |
| ) : ( | |
| <CreateForgedCredentialsModal | |
| blockDef={blockDef!} | |
| isOpen={isOpen} | |
| onClose={onClose} | |
| onNewCredentials={(credentialsId) => | |
| onNewCredentials(credentialsId) | |
| } | |
| /> | |
| )} | |
| </> | |
| ); |
|
@baptisteArno The most basic version of this feature is implemented, although there are further scopes of improvements. Please note that:
typebot-ai-group-titles.mp4 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (4)
apps/builder/src/features/workspace/components/InEditorAIFeatures.tsx (4)
28-30: Consider using an enum or const object for AI providersThe current implementation uses a hard-coded array and union type. Consider using an enum or const object to make it more maintainable and type-safe:
-type aiProvidersType = "openai" | "open-router" | "anthropic"; -const aiProviders: aiProvidersType[] = ["openai", "open-router", "anthropic"]; +const AI_PROVIDERS = { + OPENAI: 'openai', + OPEN_ROUTER: 'open-router', + ANTHROPIC: 'anthropic', +} as const; + +type aiProvidersType = typeof AI_PROVIDERS[keyof typeof AI_PROVIDERS]; +const aiProviders = Object.values(AI_PROVIDERS);
34-77: Consider using useReducer for complex state managementThe component manages multiple related states (selectedAiProvider, selectedAiCredential, aiFeaturePrompt). Consider using
useReducerto centralize state management and make state transitions more predictable.Example implementation:
type State = { selectedAiProvider?: aiProvidersType; selectedAiCredential: { id: string; name: string; } | null; aiFeaturePrompt: string; }; type Action = | { type: 'SET_PROVIDER'; payload: aiProvidersType } | { type: 'SET_CREDENTIAL'; payload: { id: string; name: string; } | null } | { type: 'SET_PROMPT'; payload: string }; const reducer = (state: State, action: Action): State => { switch (action.type) { case 'SET_PROVIDER': return { ...state, selectedAiProvider: action.payload, selectedAiCredential: null }; case 'SET_CREDENTIAL': return { ...state, selectedAiCredential: action.payload }; case 'SET_PROMPT': return { ...state, aiFeaturePrompt: action.payload }; default: return state; } };
108-138: Consider combining effects for better state synchronizationThe two effects handle related state updates. Consider combining them into a single effect to ensure atomic updates and prevent potential race conditions.
-useEffect(() => { - // Load existing credential if already set - if ( - !!workspace?.id && - !!workspace?.aiFeatureCredentialId && - !loadingExistingCredential && - !!existingCredential && - (selectedAiProvider !== (existingCredential.type as aiProvidersType) || - selectedAiCredential?.id !== workspace.aiFeatureCredentialId) - ) { - setSelectedAiProvider(existingCredential.type as aiProvidersType); - setSelectedAiCredential({ - id: workspace.aiFeatureCredentialId, - name: existingCredential.name, - }); - } -}, [ - existingCredential, - workspace?.id, - workspace?.aiFeatureCredentialId, - loadingExistingCredential, -]); - -useEffect(() => { - if ( - workspace?.aiFeaturePrompt && - aiFeaturePrompt !== workspace.aiFeaturePrompt - ) { - setAiFeaturePrompt(workspace.aiFeaturePrompt); - } -}, [workspace?.aiFeaturePrompt]); +useEffect(() => { + if (!workspace?.id) return; + + const syncState = () => { + // Sync credentials + if ( + workspace.aiFeatureCredentialId && + !loadingExistingCredential && + existingCredential && + (selectedAiProvider !== (existingCredential.type as aiProvidersType) || + selectedAiCredential?.id !== workspace.aiFeatureCredentialId) + ) { + setSelectedAiProvider(existingCredential.type as aiProvidersType); + setSelectedAiCredential({ + id: workspace.aiFeatureCredentialId, + name: existingCredential.name, + }); + } + + // Sync prompt + if (workspace.aiFeaturePrompt && aiFeaturePrompt !== workspace.aiFeaturePrompt) { + setAiFeaturePrompt(workspace.aiFeaturePrompt); + } + }; + + syncState(); +}, [ + existingCredential, + workspace?.id, + workspace?.aiFeatureCredentialId, + workspace?.aiFeaturePrompt, + loadingExistingCredential, + selectedAiProvider, + selectedAiCredential?.id, + aiFeaturePrompt, +]);
266-276: Improve props type definitionConsider using a more descriptive interface for the props and making aiProvider required since it's used without optional chaining.
-const AICredentialsModal = ({ +interface AICredentialsModalProps { + isOpen: boolean; + onClose: () => void; + onNewCredentials: (credentialsId: string, provider: aiProvidersType) => void; + aiProvider: aiProvidersType; +} + +const AICredentialsModal = ({ isOpen, onClose, onNewCredentials, aiProvider, -}: { - isOpen: boolean; - onClose: () => void; - onNewCredentials: (credentialsId: string, provider: aiProvidersType) => void; - aiProvider: aiProvidersType; -}) +}: AICredentialsModalProps)
| <MenuItem | ||
| key={type} | ||
| icon={<BlockIcon type={type} boxSize="16px" />} | ||
| onClick={() => { | ||
| setSelectedAiProvider(type); | ||
| setSelectedAiCredential(null); | ||
| refetchCredentialsList(); | ||
| }} | ||
| > | ||
| <BlockLabel type={type} /> | ||
| </MenuItem> | ||
| ))} | ||
| </MenuList> | ||
| </Menu> | ||
| </Flex> | ||
| {!!selectedAiProvider && | ||
| !!credentialsList && | ||
| !!credentialsList.credentials && | ||
| !loadingCredentialsList && ( | ||
| <Flex> | ||
| <> | ||
| {credentialsList.credentials.length > 0 && ( | ||
| <Menu isLazy> | ||
| <MenuButton | ||
| as={Button} | ||
| size="sm" | ||
| rightIcon={<ChevronDownIcon />} | ||
| > | ||
| {selectedAiCredential ? ( | ||
| <Flex gap={2}> | ||
| <Text>{selectedAiCredential.name}</Text> | ||
| </Flex> | ||
| ) : ( | ||
| `Select account` | ||
| )} | ||
| </MenuButton> | ||
| <MenuList> | ||
| {credentialsList?.credentials.map((cred) => ( | ||
| <MenuItem | ||
| key={cred.id} | ||
| onClick={() => { | ||
| setSelectedAiCredential({ | ||
| id: cred.id, | ||
| name: cred.name, | ||
| }); | ||
| saveAiFeaturesCredential( | ||
| cred.id, | ||
| cred.type as aiProvidersType, | ||
| ); | ||
| }} | ||
| > | ||
| <Text>{cred.name}</Text> | ||
| </MenuItem> | ||
| ))} | ||
| <MenuItem | ||
| icon={<PlusIcon />} | ||
| onClick={() => addNewAccount(selectedAiProvider)} | ||
| > | ||
| <Text>Connect new</Text> | ||
| </MenuItem> | ||
| </MenuList> | ||
| </Menu> | ||
| )} | ||
| {credentialsList.credentials.length === 0 && ( | ||
| <Button | ||
| size={"sm"} | ||
| leftIcon={<PlusIcon />} | ||
| onClick={() => addNewAccount(selectedAiProvider)} | ||
| > | ||
| {`Add account`} | ||
| </Button> | ||
| )} | ||
| </> | ||
| </Flex> | ||
| )} | ||
| </HStack> | ||
|
|
||
| {!!workspace.aiFeatureCredentialId && !!workspace.aiFeaturePrompt && ( | ||
| <Stack> | ||
| <Textarea | ||
| label={"System Prompt"} | ||
| withVariableButton={false} | ||
| defaultValue={aiFeaturePrompt} | ||
| onChange={handlePromptChange} | ||
| moreInfoTooltip="Prompt to generate group titles in the block editor" | ||
| /> | ||
| </Stack> | ||
| )} | ||
| </> | ||
| )} | ||
| <AICredentialsModal | ||
| isOpen={isAIModalOpen} | ||
| onClose={onAIModalClose} | ||
| onNewCredentials={(credentialsId, provider) => | ||
| saveAiFeaturesCredential(credentialsId, provider) | ||
| } | ||
| aiProvider={selectedAiProvider!} | ||
| /> | ||
| </> | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extract components and improve accessibility
The render logic is complex with multiple nested conditions. Consider:
- Extracting provider and credential selection into separate components
- Adding proper ARIA labels for accessibility
- Adding loading states for better UX
Example for provider selection:
const ProviderSelector = ({
selectedProvider,
onProviderSelect,
}: {
selectedProvider?: aiProvidersType;
onProviderSelect: (provider: aiProvidersType) => void;
}) => (
<Menu isLazy>
<MenuButton
as={Button}
size="sm"
rightIcon={<ChevronDownIcon />}
aria-label="Select AI provider"
>
{selectedProvider ? (
<Flex gap={2}>
<ForgedBlockIcon type={selectedProvider} />
<ForgedBlockLabel type={selectedProvider} />
</Flex>
) : (
"Choose Provider"
)}
</MenuButton>
<MenuList>
{aiProviders.map((type) => (
<MenuItem
key={type}
icon={<BlockIcon type={type} boxSize="16px" />}
onClick={() => onProviderSelect(type)}
>
<BlockLabel type={type} />
</MenuItem>
))}
</MenuList>
</Menu>
);| const saveAiFeaturesCredential = ( | ||
| credentialId: string, | ||
| credentialsType: aiProvidersType, | ||
| ) => { | ||
| if (!workspace?.id || !selectedAiProvider) return; | ||
|
|
||
| updateWorkspace({ | ||
| aiFeaturePrompt: workspace.aiFeaturePrompt | ||
| ? workspace.aiFeaturePrompt | ||
| : defaultPromptToGenerateGroupTitles, | ||
| aiFeatureCredentialId: credentialId, | ||
| }); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for workspace updates
The saveAiFeaturesCredential function should handle potential errors from updateWorkspace and provide feedback to the user.
-const saveAiFeaturesCredential = (
+const saveAiFeaturesCredential = async (
credentialId: string,
credentialsType: aiProvidersType,
) => {
if (!workspace?.id || !selectedAiProvider) return;
- updateWorkspace({
- aiFeaturePrompt: workspace.aiFeaturePrompt
- ? workspace.aiFeaturePrompt
- : defaultPromptToGenerateGroupTitles,
- aiFeatureCredentialId: credentialId,
- });
+ try {
+ await updateWorkspace({
+ aiFeaturePrompt: workspace.aiFeaturePrompt ?? defaultPromptToGenerateGroupTitles,
+ aiFeatureCredentialId: credentialId,
+ });
+ } catch (error) {
+ // Handle error (e.g., show toast notification)
+ console.error('Failed to update workspace:', error);
+ }
};Committable suggestion skipped: line range outside the PR's diff.
|
That looks awesome. Thanks for working on that. 🔥 |
This PR is now ready to be reviewed. This attempts to resolve #1577 .
This is the most basic implementation, and there are scopes for further improvements like allowing the user to select specific models / set usage limits etc.
typebot-ai-group-titles.mp4