-
Notifications
You must be signed in to change notification settings - Fork 21
Auto-refresh the AI reviews ui if run is in progress #1345
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| `${TC_API_BASE_URL}/workflows/runs?submissionId=${submissionId}`, | ||
| { | ||
| isPaused: () => !workflowIds?.length || !submissionId, | ||
| isPaused: () => !submissionId, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[correctness]
The isPaused function now only checks for submissionId. Ensure that this change is intentional and that workflowIds is no longer needed for pausing the fetch.
| { | ||
| isPaused: () => !workflowIds?.length || !submissionId, | ||
| isPaused: () => !submissionId, | ||
| refreshInterval: hasInProgress.current ? 10 * 1000 : 0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[performance]
The refreshInterval is set to 10 seconds when hasInProgress.current is true. Consider whether this interval is appropriate for the use case, as frequent polling can lead to increased server load.
| }, | ||
| ) | ||
|
|
||
| hasInProgress.current = !!runs.find(r => ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[💡 maintainability]
The logic for determining hasInProgress relies on checking the status of each run. Ensure that all possible statuses are accounted for, especially if new statuses might be added in the future.
| ), [challengeInfo?.reviewers]) | ||
| const aiWorkflowIds = useMemo(() => aiReviewers?.map(r => r.aiWorkflowId as string), [aiReviewers]) | ||
|
|
||
| const { runs: workflowRuns, isLoading: aiWorkflowRunsLoading }: AiWorkflowRunsResponse |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[❗❗ correctness]
The aiWorkflowIds variable and its associated logic have been removed. Ensure that the useFetchAiWorkflowsRuns hook does not require this parameter for its functionality, as this could lead to unexpected behavior if the hook's implementation still expects it.
| 'data-tooltip-place': props.place ?? 'bottom', | ||
| 'data-tooltip-strategy': props.strategy ?? 'absolute', | ||
| key: tooltipId.current as string, | ||
| key: `${tooltipId.current as string}-${i}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[performance]
Using tooltipId.current concatenated with the index i as the key ensures uniqueness among siblings, which is good. However, if tooltipId.current changes, it could cause unnecessary re-renders. Ensure that tooltipId.current remains stable across renders to avoid performance issues.
| reviewers={props.aiReviewers} | ||
| submission={props.submission} | ||
| /> | ||
| <AiReviewsTable submission={props.submission} /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[❗❗ correctness]
The AiReviewsTable component no longer receives the reviewers prop. Ensure that this change is intentional and that the AiReviewsTable component does not rely on the reviewers prop for its functionality.
| reviewers={aiReviewers} | ||
| submission={submission} | ||
| /> | ||
| <AiReviewsTable submission={submission} /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[❗❗ correctness]
The AiReviewsTable component no longer receives the reviewers prop. Ensure that the component can function correctly without this prop, or if it is needed, consider re-adding it.
Related JIRA Ticket:
https://topcoder.atlassian.net/browse/
What's in this PR?