Backend aws bedrock #1482
Backend aws bedrock #1482
Conversation
- Created ai-module-types.ts to define TableInformation type with properties for table name, structure, foreign keys, and primary columns. - Introduced ai-provider.interface.ts with IAIProvider interface for generating AI responses.
There was a problem hiding this comment.
Pull request overview
This pull request introduces AWS Bedrock integration for AI-powered automatic generation of table settings and widgets when connections are created. The implementation uses Amazon's Claude model through AWS Bedrock to analyze database schemas and generate optimal configuration for displaying tables in the admin panel.
Key Changes:
- Adds AWS Bedrock Runtime client dependency (v3.954.0) and implements an AI service that generates table settings and widgets based on database structure
- Replaces the manual widget creation logic with AI-generated configurations during connection creation (skipped in test environments)
- Updates test expectations to reflect that automatic widgets are no longer created in test mode (changed from 1/2 widgets to 0)
Reviewed changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Adds AWS SDK Bedrock Runtime and related dependencies for AWS AI service integration |
| backend/package.json | Adds @aws-sdk/client-bedrock-runtime dependency |
| backend/src/entities/ai/amazon-bedrock/amazon-bedrock.ai.provider.ts | New AI provider implementation using AWS Bedrock with Claude model for response generation |
| backend/src/entities/ai/amazon-bedrock/ai-provider.interface.ts | Interface definition for AI providers |
| backend/src/entities/ai/ai.service.ts | Core AI service that builds prompts, parses responses, and creates table settings entities |
| backend/src/entities/ai/ai.module.ts | Updates module to include AI service, provider, and new use case with global scope |
| backend/src/entities/ai/user-ai-requests-v2.controller.ts | Adds new GET endpoint for triggering AI-based settings/widgets creation |
| backend/src/entities/ai/use-cases/request-ai-settings-and-widgets-creation.use.case.ts | New use case for handling AI settings and widgets creation requests |
| backend/src/entities/ai/ai-use-cases.interface.ts | Adds interface for AI settings and widgets creation use case |
| backend/src/entities/ai/ai-data-entities/types/ai-module-types.ts | Defines TableInformation type for AI processing |
| backend/src/entities/shared-jobs/shared-jobs.service.ts | Adds scanDatabaseAndCreateSettingsAndWidgetsWithAI method that orchestrates AI-based configuration generation |
| backend/src/entities/connection/use-cases/create-connection.use.case.ts | Switches from manual widget creation to AI-based approach during connection creation |
| backend/src/common/data-injection.tokens.ts | Adds new use case token for dependency injection |
| backend/src/app.module.ts | Imports AIModule at application level |
| backend/test/ava-tests/saas-tests/*.test.ts | Updates test assertions to expect 0 widgets instead of 1 since AI creation is disabled in tests |
| backend/test/ava-tests/non-saas-tests/*.test.ts | Updates test assertions to expect 0 widgets instead of 1 since AI creation is disabled in tests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return responseText || 'No response generated.'; | ||
| } catch (error) { | ||
| console.error('Error generating AI response:', error); | ||
| throw new Error('Failed to generate AI response.'); |
There was a problem hiding this comment.
The error handling throws a generic Error with a hardcoded message that loses the original error details. This makes debugging AI provider issues difficult. Consider including the original error message or using a more descriptive error that includes context about what failed (e.g., model ID, region, or specific AWS error codes).
| throw new Error('Failed to generate AI response.'); | |
| const originalMessage = | |
| error instanceof Error ? error.message : typeof error === 'string' ? error : JSON.stringify(error); | |
| throw new Error( | |
| `Failed to generate AI response (modelId=${this.modelId}, region=${this.region}): ${originalMessage}`, | |
| ); |
| @Get('/ai/v2/setup/:connectionId') | ||
| public async requestAISettingsAndWidgetsCreation( | ||
| @SlugUuid('connectionId') connectionId: string, | ||
| @MasterPassword() masterPassword: string, | ||
| @UserId() userId: string, | ||
| ): Promise<void> { | ||
| const connectionData = { | ||
| connectionId, | ||
| masterPwd: masterPassword, | ||
| cognitoUserName: userId, | ||
| }; | ||
| return await this.requestAISettingsAndWidgetsCreationUseCase.execute(connectionData, InTransactionEnum.OFF); | ||
| } |
There was a problem hiding this comment.
The new GET endpoint '/ai/v2/setup/:connectionId' is missing the @UseGuards decorator. Other endpoints that access connection data use ConnectionReadGuard or similar guards to verify permissions. Without a guard, any authenticated user could potentially trigger AI processing for any connection they have access to, which may have cost and performance implications.
| const validationQueue = new PQueue({ concurrency: 4 }); | ||
| const validatedSettings = await Promise.all( | ||
| normalizedSettings.map((setting) => | ||
| validationQueue.add(async () => { | ||
| const validateSettingsDS = buildValidateTableSettingsDS(setting); | ||
| const errors = await dao.validateSettings(validateSettingsDS, setting.table_name, undefined); | ||
| if (errors.length > 0) { | ||
| console.error(`Validation errors for table "${setting.table_name}":`, errors); | ||
| return null; | ||
| } | ||
| return setting; | ||
| }), | ||
| ), | ||
| ); |
There was a problem hiding this comment.
When validation errors occur for a table's settings, only an error is logged to console and the setting is filtered out. However, there's no notification to the user or tracking of which tables failed validation. This could result in silent failures where some tables don't get AI-generated settings without any visible indication to the user. Consider tracking failed tables and returning this information, or at least capturing it in Sentry.
| aiSettings.forEach((setting) => { | ||
| delete setting.id; | ||
| setting.connection_id = connection; | ||
| delete setting.table_widgets; | ||
| }); | ||
| return aiSettings; |
There was a problem hiding this comment.
The normalizeAISettings method mutates the input array by deleting properties (id, table_widgets) from each setting object. This direct mutation could lead to unexpected behavior if the caller expects the original data to remain unchanged. Consider creating new objects instead of mutating the input, or clearly document this side effect.
| aiSettings.forEach((setting) => { | |
| delete setting.id; | |
| setting.connection_id = connection; | |
| delete setting.table_widgets; | |
| }); | |
| return aiSettings; | |
| return aiSettings.map((setting) => { | |
| const { id: _ignoredId, table_widgets: _ignoredTableWidgets, ...rest } = setting; | |
| return { | |
| ...rest, | |
| connection_id: connection, | |
| } as TableSettingsEntity; | |
| }); |
| } finally { | ||
| if (isConnectionTestedSuccessfully && !isConnectionTypeAgent(connectionCopy.type)) { | ||
| await this.sharedJobsService.scanDatabaseAndCreateWidgets(connectionCopy); | ||
| // await this.sharedJobsService.scanDatabaseAndCreateWidgets(connectionCopy); |
There was a problem hiding this comment.
The commented-out code should be removed rather than left in place. If the old scanDatabaseAndCreateWidgets method is no longer needed, it should be deleted. If it's being kept as a fallback or for backwards compatibility, the comment should explain why both methods are present and under what conditions each should be used.
| // await this.sharedJobsService.scanDatabaseAndCreateWidgets(connectionCopy); |
| if (isConnectionTestedSuccessfully && !isConnectionTypeAgent(connectionCopy.type)) { | ||
| await this.sharedJobsService.scanDatabaseAndCreateWidgets(connectionCopy); | ||
| // await this.sharedJobsService.scanDatabaseAndCreateWidgets(connectionCopy); | ||
| await this.sharedJobsService.scanDatabaseAndCreateSettingsAndWidgetsWithAI(connectionCopy); | ||
| } |
There was a problem hiding this comment.
The AI-powered scanning is triggered in a finally block without any error propagation to the user. If the AI service fails (e.g., due to invalid credentials, model unavailability, or API quota limits), the connection will still be created successfully, but the user won't receive table settings or widgets. Since this is now the primary method for widget creation (the old scanDatabaseAndCreateWidgets is commented out), consider either: 1) Making the AI call part of the main flow with proper error handling and user notification, or 2) Implementing a fallback to the old method if AI generation fails.
| }); | ||
| try { | ||
| const response = await this.bedrockRuntimeClient.send(command); | ||
| const responseText = response.output.message?.content[0].text; |
There was a problem hiding this comment.
The code accesses response.output.message?.content[0].text without checking if the content array has any elements. If the response has an empty content array, this will throw a runtime error. Add a check to ensure content array length is greater than 0 before accessing content[0].
| const responseText = response.output.message?.content[0].text; | |
| const content = response.output.message?.content; | |
| const responseText = content && content.length > 0 ? content[0]?.text : undefined; |
| try { | ||
| return JSON.parse(cleanedResponse) as AIResponse; | ||
| } catch (error) { | ||
| throw new Error(`Failed to parse AI response: ${error.message}`); |
There was a problem hiding this comment.
The error parsing logic in this method does not account for potential issues when accessing error.message. If the error caught is not an Error object (e.g., a string or other type is thrown), accessing error.message will result in undefined. Consider using a type guard or checking the error type before accessing its message property.
| throw new Error(`Failed to parse AI response: ${error.message}`); | |
| const errorMessage = | |
| error instanceof Error | |
| ? error.message | |
| : typeof error === 'string' | |
| ? error | |
| : (() => { | |
| try { | |
| return JSON.stringify(error); | |
| } catch { | |
| return 'Unknown error'; | |
| } | |
| })(); | |
| throw new Error(`Failed to parse AI response: ${errorMessage}`); |
| settings.table_name = tableSettings.table_name; | ||
| settings.display_name = tableSettings.display_name; | ||
| settings.list_fields = this.filterValidColumns(tableSettings.list_fields, validColumnNames); | ||
| settings.ordering_field = tableSettings.ordering_field; |
There was a problem hiding this comment.
The ordering_field value from AI is not validated against validColumnNames before being assigned. While list_fields, search_fields, readonly_fields, and columns_view are all filtered through filterValidColumns, ordering_field is directly assigned without validation. If the AI suggests an invalid column name for ordering_field, this could cause runtime errors when the ordering is applied. Consider validating that ordering_field exists in validColumnNames, or setting it to null if invalid.
| settings.ordering_field = tableSettings.ordering_field; | |
| settings.ordering_field = validColumnNames.includes(tableSettings.ordering_field) | |
| ? tableSettings.ordering_field | |
| : null; |
| @ApiOperation({ summary: 'Request AI settings and widgets creation for connection' }) | ||
| @ApiResponse({ | ||
| status: 200, | ||
| description: 'AI settings and widgets creation job has been queued.', | ||
| }) | ||
| @Get('/ai/v2/setup/:connectionId') | ||
| public async requestAISettingsAndWidgetsCreation( | ||
| @SlugUuid('connectionId') connectionId: string, | ||
| @MasterPassword() masterPassword: string, | ||
| @UserId() userId: string, | ||
| ): Promise<void> { | ||
| const connectionData = { | ||
| connectionId, | ||
| masterPwd: masterPassword, | ||
| cognitoUserName: userId, | ||
| }; | ||
| return await this.requestAISettingsAndWidgetsCreationUseCase.execute(connectionData, InTransactionEnum.OFF); | ||
| } |
There was a problem hiding this comment.
The API documentation states 'AI settings and widgets creation job has been queued', but the implementation actually executes the job synchronously in the use case (await this.sharedJobsService.scanDatabaseAndCreateSettingsAndWidgetsWithAI). This is misleading - the endpoint will block until the entire AI generation and database operations complete. Either update the documentation to reflect the synchronous nature, or consider implementing actual asynchronous job queueing for better user experience and scalability.
No description provided.