feat(PM-1793): Create AI workflow api implementation #34

hentrymartin · 2025-09-03T21:53:49Z

What's in this PR?

Create AI workflow api implementation

Ticket link - https://topcoder.atlassian.net/browse/PM-1793

github-actions · 2025-09-03T21:54:11Z

src/api/health-check/healthCheck.controller.ts

@@ -23,7 +23,7 @@ export class GetHealthCheckResponseDto {
 }

 @ApiTags('Healthcheck')
-@Controller('/')
+@Controller('/reviews')


The change from @Controller('/') to @Controller('/reviews') seems to alter the endpoint path for the health check controller. Ensure that this change aligns with the intended functionality of the AI workflow API implementation, as it might affect existing routes and integrations.

github-actions · 2025-09-03T21:54:14Z

src/api/review-summation/review-summation.controller.ts

@@ -40,7 +40,7 @@ import { JwtUser } from 'src/shared/modules/global/jwt.service';

 @ApiTags('ReviewSummations')
 @ApiBearerAuth()
-@Controller('/api/reviewSummations')
+@Controller('/reviewSummations')


The change in the route path from /api/reviewSummations to /reviewSummations might affect existing API consumers. Ensure that this change is intentional and that all dependent services or clients are updated accordingly.

github-actions · 2025-09-03T21:54:20Z

src/api/submission/submission.controller.ts

@@ -52,7 +52,7 @@ import { JwtUser } from 'src/shared/modules/global/jwt.service';

 @ApiTags('Submissions')
 @ApiBearerAuth()
-@Controller('/api/submissions')
+@Controller('/submissions')


The change from /api/submissions to /submissions in the @Controller decorator modifies the base route for the SubmissionController. Ensure that this change is intentional and that all relevant parts of the application are updated to reflect this new route, as it may affect API endpoint accessibility.

github-actions · 2025-09-03T21:54:23Z

src/api/submission/submission.controller.ts

@@ -66,25 +66,50 @@ export class SubmissionController {
    UserRole.Copilot,
    UserRole.Submitter,
    UserRole.Reviewer,
+    UserRole.User,


Adding UserRole.User to the list of roles might need a review to ensure that this role should indeed have the permissions associated with this method. Verify if this role should have access to create submissions.

github-actions · 2025-09-03T21:54:26Z

src/api/submission/submission.controller.ts

-    this.logger.log(
-      `Creating submission with request boy: ${JSON.stringify(body)}`,
+    console.log(
+      `Creating submission with request body: ${JSON.stringify(body)}`,


Using console.log for logging is not recommended in production code. Consider using a proper logging library or framework that is already integrated into your application, such as this.logger.log, which was previously used.

github-actions · 2025-09-03T21:54:28Z

src/api/submission/submission.controller.ts

@@ -151,6 +176,7 @@ export class SubmissionController {
    UserRole.Submitter,
    UserRole.Reviewer,
    UserRole.Talent,
+    UserRole.User,


Adding UserRole.User to the list of roles might require updating the authorization logic to ensure that users with this role have the correct permissions. Please verify that this change aligns with the intended access control policies.

github-actions · 2025-09-03T21:54:31Z

src/api/submission/submission.controller.ts

@@ -185,6 +211,7 @@ export class SubmissionController {
    UserRole.Admin,
    UserRole.Submitter,
    UserRole.Reviewer,
+    UserRole.User,


Adding UserRole.User to the list of roles might unintentionally broaden access. Please ensure that this role should indeed have access to this functionality, as it could introduce security concerns if not intended.

github-actions · 2025-09-03T21:54:34Z

src/api/submission/submission.service.ts

@@ -20,6 +20,7 @@ export class SubmissionService {
  constructor(private readonly prisma: PrismaService) {}

  async createSubmission(authUser: JwtUser, body: SubmissionRequestDto) {
+    console.log(`BODY: ${JSON.stringify(body)}`);


Using console.log for logging is not recommended in production code. Consider using a proper logging library to handle logging with different levels and outputs.

github-actions · 2025-09-03T21:54:37Z

src/dto/aiWorkflow.dto.ts

+
+  @ApiProperty({ required: false })
+  @IsString()
+  @IsNotEmpty()


Suggestion:

The @IsNotEmpty() decorator should not be used with optional properties like updatedBy. Consider removing @IsNotEmpty() to accurately reflect that this field can be empty or undefined.

github-actions · 2025-09-03T21:54:46Z

src/shared/modules/kafka/kafka-consumer.service.ts

@@ -50,8 +50,8 @@ export class KafkaConsumerService
  }

  async onApplicationBootstrap() {
-    await this.subscribeToTopics();
-    await this.startConsumer();
+    // await this.subscribeToTopics();


The calls to subscribeToTopics() and startConsumer() have been commented out. If this is intentional for testing or debugging, consider adding a mechanism to ensure these methods are called in production to maintain the expected functionality of the Kafka consumer.

github-actions · 2025-09-04T17:17:23Z

prisma/migrate.ts

+          break;
+        }
+        case 'llm_model': {
+          console.log(`[${type}][${subtype}][${file}] Processing file`);


The log statement uses the variable file, but file is not defined within the current scope. Ensure that file is defined or passed to the function if needed.

github-actions · 2025-09-04T17:17:23Z

prisma/migrate.ts

+          console.log(`[${type}][${subtype}][${file}] Processing file`);
+          const idToLegacyIdMap = {};
+          const processedData = jsonData[key].map((c) => {
+            const id = nanoid(14);


Consider using a more descriptive variable name than c for better readability and maintainability of the code.

github-actions · 2025-09-04T17:17:31Z

prisma/migrate.ts

@@ -1509,6 +1666,9 @@ migrate()
      },
      { key: 'uploadIdMap', value: uploadIdMap },
      { key: 'submissionIdMap', value: submissionIdMap },
+      { key: 'llmProviderIdMap', value: llmProviderIdMap },
+      { key: 'llmModelIdMap', value: llmModelIdMap },
+      { key: 'aiWorkflowIdMap', value: aiWorkflowIdMap },


There is an extra comma at the end of the object in line 1671. This may lead to syntax errors in environments that do not support trailing commas. Consider removing the trailing comma.

github-actions · 2025-09-04T17:17:37Z

src/api/ai_workflow/ai_workflow.controller.ts

+    description: 'The AI workflow has been successfully created.',
+  })
+  @ApiResponse({ status: 403, description: 'Forbidden.' })
+  async create(@Body() createAiWorkflowDto: CreateAiWorkflowDto) {


Consider adding error handling for the create method to manage potential exceptions that might occur during the creation process. This will help in providing more informative error responses to the client.

github-actions · 2025-09-04T21:34:14Z

src/api/ai_workflow/aiWorkflow.service.ts

+
+  async scorecardExists(scorecardId: string): Promise<boolean> {
+    const count = await this.prisma.scorecard.count({
+      where: { id: scorecardId, status: ScorecardStatus.ACTIVE},


Consider handling the case where ScorecardStatus.ACTIVE might not be defined or imported correctly. Ensure that ScorecardStatus is imported from the correct module and that it includes the ACTIVE status to prevent runtime errors.

github-actions · 2025-09-04T21:34:17Z

src/dto/aiWorkflow.dto.ts

+  createdBy: string;
+
+  @ApiProperty({ required: false })
+  updatedBy?: string;


The decorators @IsString() and @IsNotEmpty() have been removed from the updatedBy property. If updatedBy should always be a non-empty string when provided, consider keeping these decorators to enforce validation.

src/api/ai_workflow/ai_workflow.controller.ts

src/api/ai_workflow/aiWorkflow.service.ts

vas3a · 2025-09-08T07:40:48Z

src/dto/aiWorkflow.dto.ts

+  updatedBy?: string;
+
+  @ApiProperty({ required: false })
+  updatedAt?: Date;


please remove createdBy, updatedBy, createdAt, updatedAt from the DTO

@hentrymartin yes, we won't need those over the API call.

kkartunov · 2025-09-09T06:39:27Z

src/dto/aiWorkflow.dto.ts

+  updatedBy?: string;
+
+  @ApiProperty({ required: false })
+  updatedAt?: Date;


@hentrymartin yes, we won't need those over the API call.

src/api/ai_workflow/aiWorkflow.service.ts

prisma/schema.prisma

src/api/ai-workflow/ai-workflow.controller.ts

src/api/ai-workflow/ai-workflow.service.ts

src/api/api.module.ts

hentrymartin · 2025-09-09T18:56:52Z

@vas3a @kkartunov I've updated the PR based on the review comments. Can you please have another look on it?

vas3a · 2025-09-10T09:25:12Z

src/api/ai-workflow/ai-workflow.service.ts

+
+    return this.prisma.aiWorkflow.create({
+      data: {
+        ...rest,


you shouldn't do ...rest in here. you should extract the data you need and then pass it to the db.

@kkartunov @jmgasper this is an issue with the current global config for the ValidationPipe. Currently it is configured with whitelist: false - which will not strip off any additional data sent to the API.
This is bad IMO. like in this example, if we don't make sure to manually strip off excess data, it could be passed to DB.
It can get even uglier if the user sends fields that are not validated through DTO but they exist in db (eg. createdBy shouldn't be accepted through API, and we shouldn't add it in DTO, BUT this means that this gets sent to DB without validation).

This could be fixed by either usingwhitelist: true, but might cause other errors if someone used whitelist: false intentionally (we might do it anyway, it's dev only atm and can fix it), OR use forbidNonWhitelisted: true which will throw error if you send additional data. This should be more obvious.

I do agree with @vas3a. @jmgasper we plan to have whitelist: false in the dto here. What do you think?

@hentrymartin please update the code to not use ...rest but pick fields.

@vas3a As the audit fields are not null fields, I am passing empty string for now to avoid type errors, once the Prisma middleware is done, then it should be removed.

prisma/migrations/20250909154525_updated_audit_fields/migration.sql

kkartunov · 2025-09-10T16:28:38Z

src/api/ai-workflow/ai-workflow.service.ts

+
+    return this.prisma.aiWorkflow.create({
+      data: {
+        ...rest,


@hentrymartin please update the code to not use ...rest but pick fields.

src/api/ai-workflow/ai-workflow.service.ts

hentrymartin · 2025-09-10T22:25:42Z

@kkartunov I've updated the PR, please have a look into it again.

hentrymartin added 2 commits September 3, 2025 22:43

Merge branch 'pm-1788' into pm-1793

8753af5

feat: create ai workflow api

245f8f7

github-actions bot reviewed Sep 3, 2025

View reviewed changes

hentrymartin changed the base branch from pm-1788 to feat/aiworkflows-lsb September 3, 2025 21:55

hentrymartin added 2 commits September 4, 2025 19:09

deploy to dev

14c1641

fix: lint

6172269

github-actions bot reviewed Sep 4, 2025

View reviewed changes

fix: errors

b7dfcf6

github-actions bot reviewed Sep 4, 2025

View reviewed changes

hentrymartin added 2 commits September 4, 2025 23:37

fix: lint

0ac583d

fix: modified api url

7fa9f50

github-actions bot reviewed Sep 4, 2025

View reviewed changes

src/api/ai_workflow/ai_workflow.controller.ts Outdated Show resolved Hide resolved

hentrymartin self-assigned this Sep 5, 2025

hentrymartin requested review from vas3a and kkartunov September 5, 2025 13:55

vas3a reviewed Sep 8, 2025

View reviewed changes

kkartunov requested changes Sep 9, 2025

View reviewed changes

vas3a reviewed Sep 9, 2025

View reviewed changes

src/api/ai_workflow/aiWorkflow.service.ts Outdated Show resolved Hide resolved

hentrymartin changed the base branch from feat/aiworkflows-lsb to develop September 9, 2025 18:23

hentrymartin added 3 commits September 9, 2025 20:24

Merge branch 'develop' into pm-1793

e601a0b

fix: review comments

bd57ad1

fix: lint

71baf40