Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,14 @@ import styles from './AiReviewsTable.module.scss'

interface AiReviewsTableProps {
submission: Pick<BackendSubmission, 'id'|'virusScan'>
reviewers: { aiWorkflowId: string }[]
}

const stopPropagation = (ev: ReactMouseEvent<HTMLDivElement, MouseEvent>): void => {
ev.stopPropagation()
}

const AiReviewsTable: FC<AiReviewsTableProps> = props => {
const aiWorkflowIds = useMemo(() => props.reviewers.map(r => r.aiWorkflowId), [props.reviewers])
const { runs, isLoading }: AiWorkflowRunsResponse = useFetchAiWorkflowsRuns(props.submission.id, aiWorkflowIds)
const { runs, isLoading }: AiWorkflowRunsResponse = useFetchAiWorkflowsRuns(props.submission.id)

const windowSize: WindowSize = useWindowSize()
const isTablet = useMemo(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,7 @@ const CollapsibleAiReviewsRow: FC<CollapsibleAiReviewsRowProps> = props => {
</span>
{isOpen && (
<div className={classNames(styles.table, 'reviews-table')}>
<AiReviewsTable
reviewers={props.aiReviewers}
submission={props.submission}
/>
<AiReviewsTable submission={props.submission} />

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.

</div>
)}
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -279,10 +279,7 @@ export const SubmissionHistoryModal: FC<SubmissionHistoryModalProps> = (props: S
<tr>
<td className={styles.aiReviewersTableRow} colSpan={4}>
<div className={styles.aiReviewersTable}>
<AiReviewsTable
reviewers={aiReviewers}
submission={submission}
/>
<AiReviewsTable submission={submission} />

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.

</div>
</td>
</tr>
Expand Down
15 changes: 12 additions & 3 deletions src/apps/review/src/lib/hooks/useFetchAiWorkflowRuns.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useCallback, useEffect, useState } from 'react'
import { useCallback, useEffect, useRef, useState } from 'react'
import useSWR, { SWRResponse } from 'swr'

import { EnvironmentConfig } from '~/config'
Expand Down Expand Up @@ -111,8 +111,8 @@ export const aiRunFailed = (aiRun: Pick<AiWorkflowRun, 'status'>): boolean => [

export function useFetchAiWorkflowsRuns(
submissionId: string,
workflowIds: string[],
): AiWorkflowRunsResponse {
const hasInProgress = useRef(true)
// Use swr hooks for challenge info fetching
const {
data: runs = [],
Expand All @@ -121,10 +121,19 @@ export function useFetchAiWorkflowsRuns(
}: SWRResponse<AiWorkflowRun[], Error> = useSWR<AiWorkflowRun[], Error>(
`${TC_API_BASE_URL}/workflows/runs?submissionId=${submissionId}`,
{
isPaused: () => !workflowIds?.length || !submissionId,
isPaused: () => !submissionId,

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.

refreshInterval: hasInProgress.current ? 10 * 1000 : 0,

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 => (

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.

![
AiWorkflowRunStatusEnum.SUCCESS,
AiWorkflowRunStatusEnum.CANCELLED,
AiWorkflowRunStatusEnum.COMPLETED,
].includes(r.status)
))

// Show backend error when fetching challenge info
useEffect(() => {
if (fetchError) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { Context, createContext, FC, PropsWithChildren, ReactNode, useContext, u
import { useParams, useSearchParams } from 'react-router-dom'

import { ChallengeDetailContext } from '../../../lib'
import { ChallengeDetailContextModel, ReviewCtxStatus, ReviewsContextModel } from '../../../lib/models'
import { ReviewCtxStatus, ReviewsContextModel } from '../../../lib/models'
import { AiWorkflowRunsResponse, useFetchAiWorkflowsRuns, useFetchSubmissionInfo } from '../../../lib/hooks'

export const ReviewsContext: Context<ReviewsContextModel>
Expand All @@ -25,17 +25,10 @@ export const ReviewsContextProvider: FC<PropsWithChildren> = props => {
const [actionButtons, setActionButtons] = useState<ReactNode>()

const challengeDetailsCtx = useContext(ChallengeDetailContext)
const { challengeInfo }: ChallengeDetailContextModel = challengeDetailsCtx

const [submissionInfo] = useFetchSubmissionInfo(submissionId)

const aiReviewers = useMemo(() => (
(challengeInfo?.reviewers ?? []).filter(r => !!r.aiWorkflowId)
), [challengeInfo?.reviewers])
const aiWorkflowIds = useMemo(() => aiReviewers?.map(r => r.aiWorkflowId as string), [aiReviewers])

const { runs: workflowRuns, isLoading: aiWorkflowRunsLoading }: AiWorkflowRunsResponse

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.

= useFetchAiWorkflowsRuns(submissionId, aiWorkflowIds)
= useFetchAiWorkflowsRuns(submissionId)

const isLoadingCtxData
= challengeDetailsCtx.isLoadingChallengeInfo
Expand Down
4 changes: 2 additions & 2 deletions src/libs/ui/lib/components/tooltip/Tooltip.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,12 @@ const Tooltip: FC<TooltipProps> = (props: TooltipProps) => {

function renderTrigger(): ReactElement[] {
return Children.toArray(props.children)
.map(child => cloneElement((wrapComponents(child, props.disableWrap) as ReactElement), {
.map((child, i) => cloneElement((wrapComponents(child, props.disableWrap) as ReactElement), {
'data-tooltip-delay-show': triggerOnClick ? '0' : '300',
'data-tooltip-id': tooltipId.current as string,
'data-tooltip-place': props.place ?? 'bottom',
'data-tooltip-strategy': props.strategy ?? 'absolute',
key: tooltipId.current as string,
key: `${tooltipId.current as string}-${i}`,

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.

} as any))
}

Expand Down
Loading