PS-489 - prevent double queueing ai workflow runs #185
Conversation
…rkflows have already been queued. if so, prevent further queueing
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| async hasQueuedWorkflowRuns(submissionId: string): Promise<boolean> { | ||
| if (!submissionId) return false; | ||
|
|
||
| const existing = await this.prisma.aiWorkflowRun.findFirst({ |
There was a problem hiding this comment.
[performance]
Consider adding an index on the submissionId field in the aiWorkflowRun table if not already present. This could improve the performance of the findFirst query, especially if the table grows large.
There was a problem hiding this comment.
Pull request overview
This PR implements a deduplication check to prevent AI workflow runs from being queued multiple times for the same submission when the SubmissionScanCompleteHandler is triggered.
Key Changes:
- Added a
hasQueuedWorkflowRunsmethod to check for existing workflow runs before queueing new ones - Integrated this check into the scan complete orchestrator to skip queueing if workflows already exist
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/shared/modules/global/workflow-queue.handler.ts | Adds hasQueuedWorkflowRuns method that queries the database for existing workflow runs by submission ID |
| src/shared/modules/global/submission-scan-complete.orchestrator.ts | Integrates the deduplication check before queueing workflows, with early return if workflows already exist |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }); | ||
| } | ||
|
|
||
| async hasQueuedWorkflowRuns(submissionId: string): Promise<boolean> { |
There was a problem hiding this comment.
The method name hasQueuedWorkflowRuns is misleading because it doesn't actually check if workflow runs are queued. It checks if any workflow runs exist for the submission, regardless of their status. Consider renaming to hasWorkflowRuns or, if the intent is to check for active/pending workflows, rename to hasActiveWorkflowRuns and update the implementation to filter by status.
| // check if workflow runs have already been queued for this submission | ||
| const alreadyQueued = await this.workflowQueueHandler.hasQueuedWorkflowRuns( | ||
| submissionId, | ||
| ); | ||
|
|
||
| if (alreadyQueued) { | ||
| this.logger.log( | ||
| `AI workflow runs already queued for submission ${submissionId}. Skipping queueing.`, | ||
| ); | ||
| return; | ||
| } |
There was a problem hiding this comment.
There's a potential race condition between checking for existing workflow runs and queueing new ones. If two scan complete events are processed concurrently for the same submission, both could pass the hasQueuedWorkflowRuns check and proceed to queue workflows, resulting in duplicate workflow runs.
Consider using a database transaction or a unique constraint to prevent duplicate workflow runs, or ensure that the check and queue operations are atomic.
PS-489 - when receiving SubmissionScanCompleteHandler, check if ai workflows have already been queued. if so, prevent further queueing