feat: implement AI settings and widgets creation with streaming response#1809
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR changes AI setup from a queued request into a streaming progress endpoint for creating AI-generated table settings and widgets.
Changes:
- Adds streaming response handling for
GET /ai/v2/setup/:connectionId. - Adds progress messages during AI scan/settings/widget creation.
- Adds utility and tests for emitted settings messages plus e2e coverage for the stream.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
backend/src/entities/ai/use-cases/request-ai-settings-and-widgets-creation.use.case.ts |
Streams AI setup progress and terminal chunks. |
backend/src/entities/ai/user-ai-requests-v2.controller.ts |
Passes Express response into the AI setup use case. |
backend/src/entities/ai/application/data-structures/request-ai-settings-creation.ds.ts |
Adds request DS carrying connection data and response. |
backend/src/entities/ai/ai-use-cases.interface.ts |
Updates interface to use the new DS. |
backend/src/entities/shared-jobs/shared-jobs.service.ts |
Adds progress callbacks/messages and rethrows scan errors. |
backend/src/entities/shared-jobs/utils/emit-settings-messages.util.ts |
Adds helper for per-setting progress messages. |
backend/test/ava-tests/non-saas-tests/* |
Adds unit and e2e tests for progress message emission and streaming setup. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| } catch (error) { | ||
| console.error('Error during AI scan and creation of settings/widgets: ', error); | ||
| Sentry.captureException(error); |
| widgetEntity.settings = savedSetting; | ||
| widgetsToSave.push(widgetEntity); | ||
| message( | ||
| `Added ${widget.widget_type} widget for table "${savedSetting.table_name}" on column "${widget.field_name}"`, |
📝 WalkthroughWalkthroughThis PR adds Server-Sent Events (SSE) streaming to the AI settings creation endpoint. The ChangesAI Settings Streaming Implementation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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: 1
🧹 Nitpick comments (4)
backend/src/entities/ai/use-cases/request-ai-settings-and-widgets-creation.use.case.ts (1)
63-63: ⚡ Quick winUse a template literal for chunk serialization.
Replace string concatenation with a template literal for guideline compliance.
Proposed fix
- response.write(JSON.stringify(chunk) + '\n'); + response.write(`${JSON.stringify(chunk)}\n`);As per coding guidelines, "Use template literals instead of string concatenation".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/entities/ai/use-cases/request-ai-settings-and-widgets-creation.use.case.ts` at line 63, Replace the string concatenation in the streaming response call so it uses a template literal: locate the response.write call in request-ai-settings-and-widgets-creation.use.case.ts (the line that currently does response.write(JSON.stringify(chunk) + '\n')) and change it to use a template literal to append the newline to the serialized chunk (keep JSON.stringify(chunk) as-is, only alter how the '\n' is concatenated).backend/test/ava-tests/non-saas-tests/non-saas-emit-settings-messages.test.ts (1)
42-56: ⚡ Quick winConsider adding test coverage for falsy but valid enum values.
Test case 2 validates that
null,undefined, and empty arrays are correctly skipped. However, it doesn't verify behavior whenorderingis set to a potentially falsy but valid enum value (e.g.,0ifQueryOrderingEnumuses numeric values).If
QueryOrderingEnum.ASC = 0, the utility function would skip it due to the truthiness check, but this test wouldn't catch that bug.🧪 Suggested additional test case
test('includes ordering even when it is a falsy enum value', (t) => { // Only add this test if QueryOrderingEnum can have value 0 or other falsy values const setting = buildSetting({ ordering: 0 as QueryOrderingEnum, // or QueryOrderingEnum.ASC if ASC = 0 }); const lines = collect(setting); t.is(lines.length, 1); t.true(lines[0].includes('ordering parameter set to')); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/test/ava-tests/non-saas-tests/non-saas-emit-settings-messages.test.ts` around lines 42 - 56, Add a test that ensures falsy-but-valid enum values (e.g., QueryOrderingEnum.ASC = 0) are not skipped: in the existing test file add a case that uses buildSetting({ ordering: 0 as QueryOrderingEnum }) (or QueryOrderingEnum.ASC) and assert collect(setting) includes an "ordering parameter set to" line (or that lines length increases), so the collect function's truthiness-based filter for the ordering field is validated and will force you to adjust collect/buildSetting logic if it incorrectly treats numeric 0 as absent.backend/src/entities/shared-jobs/utils/emit-settings-messages.util.ts (1)
18-19: emitSettingsMessages: no correctness bug forQueryOrderingEnumfalsy values; optional null check for clarity
QueryOrderingEnumis a string enum (ASC/DESC), soif (setting.ordering)onbackend/src/entities/shared-jobs/utils/emit-settings-messages.util.tswon’t drop valid values;nullordering is already skipped (the DB column is nullable with defaultnull).
Optional: make the intent explicit with a null/undefined check.Optional diff
- if (setting.ordering) { + if (setting.ordering != null) { params.push(['ordering', String(setting.ordering)]); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/entities/shared-jobs/utils/emit-settings-messages.util.ts` around lines 18 - 19, The condition currently checks falsiness for setting.ordering which works here but is implicit; update the check in emitSettingsMessages (the block that pushes into params for ['ordering', String(setting.ordering)]) to explicitly test for null/undefined (e.g., setting.ordering !== null && setting.ordering !== undefined) so that valid enum values like 'ASC'/'DESC' are retained and only null/undefined are skipped; keep the push to params as-is and only change the if condition surrounding it.backend/test/ava-tests/non-saas-tests/non-saas-ai-settings-creation-stream-e2e.test.ts (1)
33-41: ⚡ Quick winConvert helper declarations to typed arrow constants.
These helpers currently use
functiondeclarations, andbuildIterableStreamalso lacks an explicit return type.♻️ Suggested update
-function buildIterableStream(chunks: Array<Record<string, unknown>>) { - return { - async *[Symbol.asyncIterator]() { - for (const chunk of chunks) { - yield chunk; - } - }, - }; -} +const buildIterableStream = ( + chunks: Array<Record<string, unknown>>, +): AsyncIterable<Record<string, unknown>> => ({ + async *[Symbol.asyncIterator](): AsyncGenerator<Record<string, unknown>, void, unknown> { + for (const chunk of chunks) { + yield chunk; + } + }, +}); -function buildAIBatchResponse(prompt: string): string { +const buildAIBatchResponse = (prompt: string): string => { const tableNames = [...prompt.matchAll(/^Table: (\S+)/gm)].map((m) => m[1]); const tables = tableNames.map((tableName) => ({ ... })); return JSON.stringify({ tables }); -} +}; -function parseNdjsonChunks(body: string): Array<Record<string, unknown>> { +const parseNdjsonChunks = (body: string): Array<Record<string, unknown>> => { return body .split('\n') .map((line) => line.trim()) .filter((line) => line.length > 0) .map((line) => JSON.parse(line)); -} +};As per coding guidelines, "Prefer arrow functions over function declarations" and "Always add type annotations to function parameters and return types in TypeScript".
Also applies to: 43-72, 90-96
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/test/ava-tests/non-saas-tests/non-saas-ai-settings-creation-stream-e2e.test.ts` around lines 33 - 41, Change the helper function declarations (notably buildIterableStream) to typed const arrow functions instead of function declarations and add explicit parameter and return types; e.g., declare buildIterableStream as a const arrow with signature (chunks: Array<Record<string, unknown>>) => AsyncIterable<Record<string, unknown>> (or the appropriate AsyncIterableIterator type) and update the other helper declarations in the same file (the other helpers called out in the review) similarly so every helper uses arrow syntax and has explicit TypeScript parameter and return type annotations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@backend/src/entities/ai/use-cases/request-ai-settings-and-widgets-creation.use.case.ts`:
- Around line 37-49: The try/catch must avoid writing or ending the HTTP stream
after the client disconnects: before calling this.writeChunk(...) or
response.end() (both in the happy path and inside the catch) check the response
writable state (e.g., response.writableEnded || response.finished ||
response.destroyed) and bail out early if closed; apply this guard around uses
of sharedJobsService.scanDatabaseAndCreateSettingsAndWidgetsWithAI callbacks as
well so the chunk callback returns immediately when the response is no longer
writable. Also update writeChunk to use a template literal instead of string
concatenation when serializing (replace JSON.stringify(chunk) + '\n' with a
template literal) so serialization is consistent.
---
Nitpick comments:
In
`@backend/src/entities/ai/use-cases/request-ai-settings-and-widgets-creation.use.case.ts`:
- Line 63: Replace the string concatenation in the streaming response call so it
uses a template literal: locate the response.write call in
request-ai-settings-and-widgets-creation.use.case.ts (the line that currently
does response.write(JSON.stringify(chunk) + '\n')) and change it to use a
template literal to append the newline to the serialized chunk (keep
JSON.stringify(chunk) as-is, only alter how the '\n' is concatenated).
In `@backend/src/entities/shared-jobs/utils/emit-settings-messages.util.ts`:
- Around line 18-19: The condition currently checks falsiness for
setting.ordering which works here but is implicit; update the check in
emitSettingsMessages (the block that pushes into params for ['ordering',
String(setting.ordering)]) to explicitly test for null/undefined (e.g.,
setting.ordering !== null && setting.ordering !== undefined) so that valid enum
values like 'ASC'/'DESC' are retained and only null/undefined are skipped; keep
the push to params as-is and only change the if condition surrounding it.
In
`@backend/test/ava-tests/non-saas-tests/non-saas-ai-settings-creation-stream-e2e.test.ts`:
- Around line 33-41: Change the helper function declarations (notably
buildIterableStream) to typed const arrow functions instead of function
declarations and add explicit parameter and return types; e.g., declare
buildIterableStream as a const arrow with signature (chunks:
Array<Record<string, unknown>>) => AsyncIterable<Record<string, unknown>> (or
the appropriate AsyncIterableIterator type) and update the other helper
declarations in the same file (the other helpers called out in the review)
similarly so every helper uses arrow syntax and has explicit TypeScript
parameter and return type annotations.
In
`@backend/test/ava-tests/non-saas-tests/non-saas-emit-settings-messages.test.ts`:
- Around line 42-56: Add a test that ensures falsy-but-valid enum values (e.g.,
QueryOrderingEnum.ASC = 0) are not skipped: in the existing test file add a case
that uses buildSetting({ ordering: 0 as QueryOrderingEnum }) (or
QueryOrderingEnum.ASC) and assert collect(setting) includes an "ordering
parameter set to" line (or that lines length increases), so the collect
function's truthiness-based filter for the ordering field is validated and will
force you to adjust collect/buildSetting logic if it incorrectly treats numeric
0 as absent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 62e0c6c2-a7fd-4e7c-a165-3ef66501ae66
📒 Files selected for processing (9)
backend/src/entities/ai/ai-use-cases.interface.tsbackend/src/entities/ai/application/data-structures/request-ai-settings-creation.ds.tsbackend/src/entities/ai/use-cases/request-ai-settings-and-widgets-creation.use.case.tsbackend/src/entities/ai/user-ai-requests-v2.controller.tsbackend/src/entities/connection/use-cases/create-connection.use.case.tsbackend/src/entities/shared-jobs/shared-jobs.service.tsbackend/src/entities/shared-jobs/utils/emit-settings-messages.util.tsbackend/test/ava-tests/non-saas-tests/non-saas-ai-settings-creation-stream-e2e.test.tsbackend/test/ava-tests/non-saas-tests/non-saas-emit-settings-messages.test.ts
Comment on lines
+37
to
+49
| await this.sharedJobsService.scanDatabaseAndCreateSettingsAndWidgetsWithAI(connection, (chunk) => | ||
| this.writeChunk(response, chunk), | ||
| ); | ||
| this.writeChunk(response, { type: 'complete' }); | ||
| response.end(); | ||
| } catch (error) { | ||
| Sentry.captureException(error); | ||
| if (!response.headersSent) { | ||
| response.status(500).send({ error: 'An error occurred while processing your request.' }); | ||
| return; | ||
| } | ||
| this.writeChunk(response, { type: 'error', message: getErrorMessage(error) }); | ||
| response.end(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify write/end calls and absence of writable-state guards in this use case.
rg -n --type=ts -C2 'response\.write\(|response\.end\(|headersSent|writableEnded|destroyed' \
backend/src/entities/ai/use-cases/request-ai-settings-and-widgets-creation.use.case.tsRepository: rocket-admin/rocketadmin
Length of output: 676
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="backend/src/entities/ai/use-cases/request-ai-settings-and-widgets-creation.use.case.ts"
echo "== $FILE =="
wc -l "$FILE"
sed -n '1,140p' "$FILE" | nl -ba | sed -n '1,140p'Repository: rocket-admin/rocketadmin
Length of output: 312
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="backend/src/entities/ai/use-cases/request-ai-settings-and-widgets-creation.use.case.ts"
echo "== $FILE (line count) =="
wc -l "$FILE"
echo "== Implementation + catch (lines ~1-80) =="
awk 'NR>=1 && NR<=80 {print NR ":" $0}' "$FILE"
echo "== writeChunk + helpers (lines ~50-120) =="
awk 'NR>=45 && NR<=120 {print NR ":" $0}' "$FILE"Repository: rocket-admin/rocketadmin
Length of output: 3790
Guard streaming writes/ends when the client disconnects; also replace string concatenation in writeChunk.
implementationcallswriteChunk(...)andresponse.end()with no writable-state checks (including in thecatchpath), so a mid-scan disconnect can triggerwrite after end-style failures and break error handling.writeChunkuses string concatenation (JSON.stringify(chunk) + '\n') instead of a template literal.
Proposed fix
public async implementation(inputData: RequestAISettingsCreationDs): Promise<void> {
const { connectionId, masterPwd, response } = inputData;
@@
try {
await this.sharedJobsService.scanDatabaseAndCreateSettingsAndWidgetsWithAI(connection, (chunk) =>
this.writeChunk(response, chunk),
);
- this.writeChunk(response, { type: 'complete' });
- response.end();
+ if (this.isWritable(response)) {
+ this.writeChunk(response, { type: 'complete' });
+ response.end();
+ }
} catch (error) {
Sentry.captureException(error);
+ if (!this.isWritable(response)) {
+ return;
+ }
if (!response.headersSent) {
response.status(500).send({ error: 'An error occurred while processing your request.' });
return;
}
this.writeChunk(response, { type: 'error', message: getErrorMessage(error) });
response.end();
}
}
@@
private writeChunk(
response: Response,
chunk: { type: 'message'; text: string } | { type: 'complete' } | { type: 'error'; message: string },
): void {
- response.write(JSON.stringify(chunk) + '\n');
+ if (!this.isWritable(response)) {
+ return;
+ }
+ response.write(`${JSON.stringify(chunk)}\n`);
}
+
+ private isWritable(response: Response): boolean {
+ return !response.writableEnded && !response.destroyed;
+ }📝 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.
Suggested change
| await this.sharedJobsService.scanDatabaseAndCreateSettingsAndWidgetsWithAI(connection, (chunk) => | |
| this.writeChunk(response, chunk), | |
| ); | |
| this.writeChunk(response, { type: 'complete' }); | |
| response.end(); | |
| } catch (error) { | |
| Sentry.captureException(error); | |
| if (!response.headersSent) { | |
| response.status(500).send({ error: 'An error occurred while processing your request.' }); | |
| return; | |
| } | |
| this.writeChunk(response, { type: 'error', message: getErrorMessage(error) }); | |
| response.end(); | |
| await this.sharedJobsService.scanDatabaseAndCreateSettingsAndWidgetsWithAI(connection, (chunk) => | |
| this.writeChunk(response, chunk), | |
| ); | |
| if (this.isWritable(response)) { | |
| this.writeChunk(response, { type: 'complete' }); | |
| response.end(); | |
| } | |
| } catch (error) { | |
| Sentry.captureException(error); | |
| if (!this.isWritable(response)) { | |
| return; | |
| } | |
| if (!response.headersSent) { | |
| response.status(500).send({ error: 'An error occurred while processing your request.' }); | |
| return; | |
| } | |
| this.writeChunk(response, { type: 'error', message: getErrorMessage(error) }); | |
| response.end(); | |
| } | |
| } | |
| private writeChunk( | |
| response: Response, | |
| chunk: { type: 'message'; text: string } | { type: 'complete' } | { type: 'error'; message: string }, | |
| ): void { | |
| if (!this.isWritable(response)) { | |
| return; | |
| } | |
| response.write(`${JSON.stringify(chunk)}\n`); | |
| } | |
| private isWritable(response: Response): boolean { | |
| return !response.writableEnded && !response.destroyed; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@backend/src/entities/ai/use-cases/request-ai-settings-and-widgets-creation.use.case.ts`
around lines 37 - 49, The try/catch must avoid writing or ending the HTTP stream
after the client disconnects: before calling this.writeChunk(...) or
response.end() (both in the happy path and inside the catch) check the response
writable state (e.g., response.writableEnded || response.finished ||
response.destroyed) and bail out early if closed; apply this guard around uses
of sharedJobsService.scanDatabaseAndCreateSettingsAndWidgetsWithAI callbacks as
well so the chunk callback returns immediately when the response is no longer
writable. Also update writeChunk to use a template literal instead of string
concatenation when serializing (replace JSON.stringify(chunk) + '\n' with a
template literal) so serialization is consistent.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
/ai/v2/setup/:connectionIdendpoint.