Skip to content

Add ai reviewers on all tables #1348

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Nov 27, 2025
Merged

Add ai reviewers on all tables #1348

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions src/apps/review/src/lib/components/ChallengeDetailsContent/ChallengeDetailsContent.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ const renderSubmissionTab = ({
downloadSubmission={downloadSubmission}
isActiveChallenge={isActiveChallenge}
showScreeningColumns={!isSubmissionTab && !isTopgearSubmissionTab}
aiReviewers={aiReviewers}
/>
)
}
Expand Down Expand Up @@ -438,6 +439,7 @@ export const ChallengeDetailsContent: FC<Props> = (props: Props) => {
isDownloading={isDownloadingSubmission}
downloadSubmission={handleSubmissionDownload}
mode={checkpointMode}
aiReviewers={aiReviewers}
/>
)
}
Expand Down Expand Up @@ -479,6 +481,7 @@ export const ChallengeDetailsContent: FC<Props> = (props: Props) => {
downloadSubmission={handleSubmissionDownload}
isActiveChallenge={props.isActiveChallenge}
columnLabel='Post-Mortem'
aiReviewers={aiReviewers}
/>
)
}
Expand All @@ -494,6 +497,7 @@ export const ChallengeDetailsContent: FC<Props> = (props: Props) => {
downloadSubmission={handleSubmissionDownload}
isActiveChallenge={props.isActiveChallenge}
phaseIdFilter={props.selectedPhaseId}
aiReviewers={aiReviewers}
/>
)
}
Expand Down
2 changes: 2 additions & 0 deletions src/apps/review/src/lib/components/ChallengeDetailsContent/TabContentCheckpoint.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ interface Props {
downloadSubmission: (submissionId: string) => void
challengeStatus?: string
mode?: 'submission' | 'screening' | 'review'
aiReviewers?: { aiWorkflowId: string }[]
}

export const TabContentCheckpoint: FC<Props> = (props: Props) => {
Expand Down Expand Up @@ -148,6 +149,7 @@ export const TabContentCheckpoint: FC<Props> = (props: Props) => {
isDownloading={props.isDownloading}
downloadSubmission={props.downloadSubmission}
mode={mode}
aiReviewers={props.aiReviewers}
/>
)
}
Expand Down
2 changes: 2 additions & 0 deletions src/apps/review/src/lib/components/ChallengeDetailsContent/TabContentIterativeReview.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ interface Props {
isActiveChallenge: boolean
columnLabel?: string
phaseIdFilter?: string
aiReviewers?: { aiWorkflowId: string }[]
}

const getSubmissionPriority = (submission: SubmissionInfo): number => {
Expand Down Expand Up @@ -203,6 +204,7 @@ export const TabContentIterativeReview: FC<Props> = (props: Props) => {
hideSubmissionColumn={shouldHideSubmissionColumn}
isChallengeCompleted={isChallengeCompleted}
hasPassedThreshold={hasPassedPostMortemThreshold}
aiReviewers={props.aiReviewers}
/>
)
}
Expand Down
2 changes: 2 additions & 0 deletions src/apps/review/src/lib/components/ChallengeDetailsContent/TabContentScreening.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ interface Props {
isActiveChallenge: boolean
showScreeningColumns?: boolean
challengeStatus?: string
aiReviewers?: { aiWorkflowId: string }[]
}

export const TabContentScreening: FC<Props> = (props: Props) => {
Expand Down Expand Up @@ -136,6 +137,7 @@ export const TabContentScreening: FC<Props> = (props: Props) => {
downloadSubmission={props.downloadSubmission}
hideHandleColumn={hideHandleColumn}
showScreeningColumns={showScreeningColumns}
aiReviewers={props.aiReviewers}
/>
)
}
Expand Down
20 changes: 20 additions & 0 deletions ...view/src/lib/components/TableCheckpointSubmissions/TableCheckpointSubmissions.module.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,3 +184,23 @@
color: var(--GrayFontColor);
font-size: 14px;
}

.aiReviews {
margin: $sp-2 0;

:global(.trigger) {
width: fit-content;
margin-left: auto;
}

:global(.reviews-table) {
margin-left: auto;
width: 75%;
margin-bottom: -9px;
Copy link

@github-actions github-actions bot Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ maintainability]
The negative margin value -9px for margin-bottom could lead to unexpected layout issues, especially if the surrounding elements are not designed to accommodate this. Consider verifying the layout impact or using a more consistent spacing strategy.


@include ltelg {
width: auto;
margin-left: -1*$sp-4;
Copy link

@github-actions github-actions bot Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ maintainability]
Using a negative margin with a multiplication operation -1*$sp-4 can be error-prone if $sp-4 changes or is not well-defined. Ensure that this value is intentional and consistently applied across the codebase.

}
}
}
58 changes: 51 additions & 7 deletions src/apps/review/src/lib/components/TableCheckpointSubmissions/TableCheckpointSubmissions.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { UserRole } from '~/libs/core'
import { handleError } from '~/apps/admin/src/lib/utils'

import {
BackendSubmission,
ChallengeDetailContextModel,
ReviewAppContextModel,
Screening,
Expand All @@ -33,6 +34,7 @@ import { updateReview } from '../../services'
import { ConfirmModal } from '../ConfirmModal'
import { useSubmissionDownloadAccess } from '../../hooks'
import type { UseSubmissionDownloadAccessResult } from '../../hooks/useSubmissionDownloadAccess'
import { CollapsibleAiReviewsRow } from '../CollapsibleAiReviewsRow'

import styles from './TableCheckpointSubmissions.module.scss'

Expand All @@ -42,6 +44,7 @@ interface Props {
isDownloading: IsRemovingType
downloadSubmission: (submissionId: string) => void
mode?: 'submission' | 'screening' | 'review'
aiReviewers?: { aiWorkflowId: string }[]
}

const isInProgressStatus = (value: string | undefined): boolean => (
Expand Down Expand Up @@ -161,6 +164,45 @@ export const TableCheckpointSubmissions: FC<Props> = (props: Props) => {
[datas],
)

const aiReviewsColumn = useMemo<TableColumn<Screening> | undefined>(
() => {
if (!props.aiReviewers?.length) {
return undefined
}

return {
columnId: 'ai-reviews-table',
isExpand: true,
label: '',
renderer: (data: Screening, allRows: Screening[]) => {
const submissionPayload = {
id: data.submissionId ?? '',
virusScan: data.virusScan,
} as Pick<BackendSubmission, 'id'|'virusScan'>

if (!submissionPayload.id) {
return <></>
}

return (
<CollapsibleAiReviewsRow
className={styles.aiReviews}
aiReviewers={props.aiReviewers!}
Copy link

@github-actions github-actions bot Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ correctness]
The non-null assertion props.aiReviewers! assumes aiReviewers is always defined when aiReviewsColumn is used. Ensure that props.aiReviewers is validated before this point to avoid potential runtime errors.

submission={submissionPayload}
defaultOpen={allRows ? !allRows.indexOf(data) : false}
Copy link

@github-actions github-actions bot Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ correctness]
The use of allRows.indexOf(data) to determine the defaultOpen state might not be reliable if data is not found in allRows, as it would return -1. Consider using a safer check to determine if data is the first element.

/>
)
},
type: 'element',
} as TableColumn<Screening>
},
[props.aiReviewers],
)

const appendAiColumn = (columns: TableColumn<Screening>[]): TableColumn<Screening>[] => (
aiReviewsColumn ? [...columns, aiReviewsColumn] : columns
)

const columns = useMemo<TableColumn<Screening>[]>(
() => {
const tableMode = mode
Expand Down Expand Up @@ -312,7 +354,7 @@ export const TableCheckpointSubmissions: FC<Props> = (props: Props) => {
}

if (tableMode === 'submission') {
return baseColumns
return appendAiColumn(baseColumns)
}

if (tableMode === 'screening') {
Expand Down Expand Up @@ -410,7 +452,7 @@ export const TableCheckpointSubmissions: FC<Props> = (props: Props) => {
const hasAnyMyAssignment = rows.some(row => Boolean(row.myReviewResourceId))
const canShowReopenActions = rows.some(row => computeReopenEligibility(row).canReopen)
if (!hasAnyMyAssignment && !canShowReopenActions) {
return screeningColumns
return appendAiColumn(screeningColumns)
}

const actionColumn: TableColumn<Screening> = {
Expand Down Expand Up @@ -502,10 +544,10 @@ export const TableCheckpointSubmissions: FC<Props> = (props: Props) => {
type: 'element',
}

return [
return appendAiColumn([
...screeningColumns,
actionColumn,
]
])
}

// mode === 'review'
Expand Down Expand Up @@ -574,7 +616,7 @@ export const TableCheckpointSubmissions: FC<Props> = (props: Props) => {
const hasAnyMyAssignment = rows.some(row => Boolean(row.myReviewResourceId))
const canShowReopenActions = rows.some(row => computeReopenEligibility(row).canReopen)
if (!hasAnyMyAssignment && !canShowReopenActions) {
return reviewColumns
return appendAiColumn(reviewColumns)
}

const actionColumn: TableColumn<Screening> = {
Expand Down Expand Up @@ -666,10 +708,10 @@ export const TableCheckpointSubmissions: FC<Props> = (props: Props) => {
type: 'element',
}

return [
return appendAiColumn([
...reviewColumns,
actionColumn,
]
])
},
[
challengeInfo,
Expand Down Expand Up @@ -730,6 +772,8 @@ export const TableCheckpointSubmissions: FC<Props> = (props: Props) => {
<Table
columns={columns}
data={visibleRows}
showExpand
expandMode='always'
disableSorting
onToggleSort={_.noop}
removeDefaultSort
Expand Down
20 changes: 20 additions & 0 deletions src/apps/review/src/lib/components/TableIterativeReview/TableIterativeReview.module.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,3 +240,23 @@
.reviewerList > span {
display: inline-flex;
}

.aiReviews {
margin: $sp-2 0;

:global(.trigger) {
width: fit-content;
margin-left: auto;
}

:global(.reviews-table) {
margin-left: auto;
width: 75%;
margin-bottom: -9px;
Copy link

@github-actions github-actions bot Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ maintainability]
Using a negative margin (margin-bottom: -9px;) can lead to layout issues, especially if the content or surrounding elements change. Consider using a different approach to achieve the desired layout effect.


@include ltelg {
width: auto;
margin-left: -1*$sp-4;
Copy link

@github-actions github-actions bot Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[💡 readability]
The expression -1*$sp-4 for margin-left could be simplified to -$sp-4 for better readability.

}
}
}
24 changes: 24 additions & 0 deletions src/apps/review/src/lib/components/TableIterativeReview/TableIterativeReview.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import type { SubmissionRow } from '../common/types'
import { resolveSubmissionReviewResult } from '../common/reviewResult'
import { ProgressBar } from '../ProgressBar'
import { TableWrapper } from '../TableWrapper'
import { CollapsibleAiReviewsRow } from '../CollapsibleAiReviewsRow'

import styles from './TableIterativeReview.module.scss'

Expand All @@ -57,6 +58,7 @@ interface Props {
hideSubmissionColumn?: boolean
isChallengeCompleted?: boolean
hasPassedThreshold?: boolean
aiReviewers?: { aiWorkflowId: string }[]
}

interface ScoreMetadata {
Expand Down Expand Up @@ -1335,6 +1337,25 @@ export const TableIterativeReview: FC<Props> = (props: Props) => {
baseColumns.push(reviewerColumn)
}

if (props.aiReviewers) {
baseColumns.push({
columnId: 'ai-reviews-table',
isExpand: true,
label: '',
renderer: (submission: SubmissionInfo, allRows: SubmissionInfo[]) => (
props.aiReviewers && (
<CollapsibleAiReviewsRow
className={styles.aiReviews}
aiReviewers={props.aiReviewers}
submission={submission as any}
defaultOpen={allRows ? !allRows.indexOf(submission) : false}
Copy link

@github-actions github-actions bot Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ correctness]
Casting submission to any can lead to runtime errors if the expected properties are not present. Consider using a more specific type or interface that matches the expected structure of submission.

/>
)
),
type: 'element',
})
}

return baseColumns
}, [
approverColumn,
Expand All @@ -1349,6 +1370,7 @@ export const TableIterativeReview: FC<Props> = (props: Props) => {
reviewDateColumn,
reviewerColumn,
submissionColumn,
props.aiReviewers,
])

const columnsMobile = useMemo<MobileTableColumn<SubmissionInfo>[][]>(() => (
Expand Down Expand Up @@ -1387,6 +1409,8 @@ export const TableIterativeReview: FC<Props> = (props: Props) => {
<Table
columns={actionColumn ? [...columns, actionColumn] : columns}
data={datas}
showExpand
expandMode='always'
Copy link

@github-actions github-actions bot Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ performance]
The expandMode='always' property on the Table component will force all rows to be expanded by default. Ensure this is the intended behavior, as it may affect performance and user experience if there are many rows.

disableSorting
onToggleSort={_.noop}
removeDefaultSort
Expand Down
20 changes: 20 additions & 0 deletions ...s/review/src/lib/components/TableSubmissionScreening/TableSubmissionScreening.module.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,3 +194,23 @@
height: 18px;
}
}

.aiReviews {
margin: $sp-2 0;

:global(.trigger) {
width: fit-content;
margin-left: auto;
}

:global(.reviews-table) {
margin-left: auto;
width: 75%;
margin-bottom: -9px;

@include ltelg {
width: auto;
margin-left: -1*$sp-4;
Copy link

@github-actions github-actions bot Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ correctness]
The use of -1*$sp-4 for margin-left could be problematic if $sp-4 is not a simple numeric value. Consider using calc() for clarity and to ensure proper CSS calculation, e.g., calc(-1 * #{$sp-4}).

}
}
}
Loading
Loading