Skip to content
Merged
Show file tree
Hide file tree
Changes from 12 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
3 changes: 2 additions & 1 deletion src/apps/review/src/config/routes.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,6 @@ export const openOpportunitiesRouteId = 'open-opportunities'
export const pastReviewAssignmentsRouteId = 'past-challenges'
export const challengeDetailRouteId = ':challengeId'
export const scorecardRouteId = 'scorecard'
export const aiScorecardRouteId = 'ai-scorecard'
export const aiScorecardRouteId = 'scorecard'

Choose a reason for hiding this comment

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

[❗❗ correctness]
The aiScorecardRouteId and scorecardRouteId are both set to 'scorecard'. This could lead to confusion or errors if both are intended to represent different routes. Consider using distinct identifiers for clarity and to avoid potential routing conflicts.

export const reviewsRouteId = 'reviews'
export const viewScorecardRouteId = ':scorecardId'
3 changes: 3 additions & 0 deletions src/apps/review/src/lib/assets/icons/icon-comment.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions src/apps/review/src/lib/assets/icons/icon-edit.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
12 changes: 8 additions & 4 deletions src/apps/review/src/lib/assets/icons/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@ import { ReactComponent as IconError } from './icon-error.svg'
import { ReactComponent as IconAiReview } from './icon-ai-review.svg'
import { ReactComponent as IconSubmission } from './icon-phase-submission.svg'
import { ReactComponent as IconRegistration } from './icon-phase-registration.svg'
import { ReactComponent as IconReview } from './icon-phase-review.svg'
import { ReactComponent as IconPhaseReview } from './icon-phase-review.svg'
import { ReactComponent as IconAppeal } from './icon-phase-appeal.svg'
import { ReactComponent as IconAppealResponse } from './icon-phase-appeal-response.svg'
import { ReactComponent as IconPhaseWinners } from './icon-phase-winners.svg'
import { ReactComponent as IconDeepseekAi } from './deepseek.svg'
import { ReactComponent as IconClock } from './icon-clock.svg'
import { ReactComponent as IconPremium } from './icon-premium.svg'
import { ReactComponent as IconComment } from './icon-comment.svg'
import { ReactComponent as IconEdit } from './icon-edit.svg'

export * from './editor/bold'
export * from './editor/code'
Expand All @@ -36,20 +38,22 @@ export {
IconError,
IconAiReview,
IconSubmission,
IconReview,
IconPhaseReview,
IconAppeal,
IconAppealResponse,
IconPhaseWinners,
IconDeepseekAi,
IconClock,
IconPremium,
IconComment,
IconEdit,
}

export const phasesIcons = {
appeal: IconAppeal,
appealResponse: IconAppealResponse,
'iterative review': IconReview,
'iterative review': IconPhaseReview,
registration: IconRegistration,
review: IconReview,
review: IconPhaseReview,
submission: IconSubmission,
}
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ const AiReviewsTable: FC<AiReviewsTableProps> = props => {
{run.status === 'SUCCESS' ? (
run.workflow.scorecard ? (
<a
href={`./ai-scorecard/${props.submission.id}/${run.workflow.id}`}
href={`./reviews/${props.submission.id}?workflowId=${run.workflow.id}`}
>
{run.score}
</a>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,27 +7,43 @@ import { aiRunFailed, aiRunInProgress, AiWorkflowRun } from '../../hooks'
import StatusLabel from './StatusLabel'

interface AiWorkflowRunStatusProps {
run: Pick<AiWorkflowRun, 'status'|'score'|'workflow'>
run?: Pick<AiWorkflowRun, 'status'|'score'|'workflow'>
status?: 'passed' | 'pending' | 'failed-score'
score?: number
hideLabel?: boolean
showScore?: boolean
}

export const AiWorkflowRunStatus: FC<AiWorkflowRunStatusProps> = props => {
const isInProgress = useMemo(() => aiRunInProgress(props.run), [props.run.status])
const isFailed = useMemo(() => aiRunFailed(props.run), [props.run.status])
const aiRunStatus = (run: Pick<AiWorkflowRun, 'status'|'score'|'workflow'>): string => {
const isInProgress = aiRunInProgress(run)
const isFailed = aiRunFailed(run)
const isPassing = (
props.run.status === 'SUCCESS'
&& props.run.score >= (props.run.workflow.scorecard?.minimumPassingScore ?? 0)
run.status === 'SUCCESS'
&& run.score >= (run.workflow.scorecard?.minimumPassingScore ?? 0)
)
const status = isInProgress ? 'pending' : isFailed ? 'failed' : (
return isInProgress ? 'pending' : isFailed ? 'failed' : (
isPassing ? 'passed' : 'failed-score'
)
}

export const AiWorkflowRunStatus: FC<AiWorkflowRunStatusProps> = props => {
const status = useMemo(() => {
if (props.status) {
return props.status
}

if (props.run) {
return aiRunStatus(props.run)
}

return ''

Choose a reason for hiding this comment

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

[⚠️ correctness]
Returning an empty string for status when neither props.status nor props.run is provided might lead to unexpected behavior. Consider handling this case more explicitly, perhaps by throwing an error or providing a default status.

}, [props.status, props.run])

const score = props.showScore ? props.run.score : undefined
const score = props.showScore ? (props.score ?? props.run?.score) : undefined

return (
<>
{props.run.status === 'SUCCESS' && isPassing && (
{status === 'passed' && (
<StatusLabel
icon={<IconOutline.CheckIcon className='icon-xl' />}
hideLabel={props.hideLabel}
Expand All @@ -36,7 +52,7 @@ export const AiWorkflowRunStatus: FC<AiWorkflowRunStatusProps> = props => {
score={score}
/>
)}
{props.run.status === 'SUCCESS' && !isPassing && (
{status === 'failed-score' && (
<StatusLabel
icon={<IconOutline.MinusCircleIcon className='icon-xl' />}
hideLabel={props.hideLabel}
Expand All @@ -45,7 +61,7 @@ export const AiWorkflowRunStatus: FC<AiWorkflowRunStatusProps> = props => {
score={score}
/>
)}
{isInProgress && (
{status === 'pending' && (
<StatusLabel
icon={<IconOutline.MinusIcon className='icon-md' />}
hideLabel={props.hideLabel}
Expand All @@ -54,7 +70,7 @@ export const AiWorkflowRunStatus: FC<AiWorkflowRunStatusProps> = props => {
score={score}
/>
)}
{isFailed && (
{status === 'failed' && (
<StatusLabel
icon={<IconOutline.XCircleIcon className='icon-xl' />}
status={status}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,33 +1,29 @@
import { FC, useCallback, useMemo } from 'react'
import { FC, useCallback } from 'react'
import classNames from 'classnames'

import { IconOutline } from '~/libs/ui'

import { ScorecardGroup as ScorecardGroupModel } from '../../../../models'
import { ScorecardSection } from '../ScorecardSection'
import { ScorecardViewerContextValue, useScorecardContext } from '../ScorecardViewer.context'
import { ScorecardViewerContextValue, useScorecardViewerContext } from '../ScorecardViewer.context'
import { ScorecardScore } from '../ScorecardScore'
import { calcGroupScore } from '../utils'
import { createReviewItemMapping } from '../utils'

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

interface ScorecardGroupProps {
index: number
group: ScorecardGroupModel
reviewItemMapping?: ReturnType<typeof createReviewItemMapping>
}

const ScorecardGroup: FC<ScorecardGroupProps> = props => {
const { aiFeedbackItems }: ScorecardViewerContextValue = useScorecardContext()
const allFeedbackItems = aiFeedbackItems || []
const { toggleItem, toggledItems }: ScorecardViewerContextValue = useScorecardContext()
const { scoreMap }: ScorecardViewerContextValue = useScorecardViewerContext()
const { toggleItem, toggledItems }: ScorecardViewerContextValue = useScorecardViewerContext()

const isVissible = !toggledItems[props.group.id]
const toggle = useCallback(() => toggleItem(props.group.id), [props.group, toggleItem])

const score = useMemo(() => (
calcGroupScore(props.group, allFeedbackItems)
), [props.group, allFeedbackItems])

return (
<div className={styles.wrap}>
<div className={classNames(styles.headerBar, isVissible && styles.toggled)} onClick={toggle}>
Expand All @@ -41,8 +37,7 @@ const ScorecardGroup: FC<ScorecardGroupProps> = props => {
<span className={styles.mx} />
<span className={styles.score}>
<ScorecardScore
score={score}
scaleMax={1}
score={scoreMap.get(props.group.id) ?? 0}
weight={props.group.weight}
/>
</span>
Expand All @@ -52,7 +47,12 @@ const ScorecardGroup: FC<ScorecardGroupProps> = props => {
</div>

{isVissible && props.group.sections.map((section, index) => (
<ScorecardSection key={section.id} section={section} index={[props.index, index + 1].join('.')} />
<ScorecardSection
key={section.id}
section={section}
index={[props.index, index + 1].join('.')}
reviewItemMapping={props.reviewItemMapping}
/>
))}
</div>
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@ import { FC, useMemo } from 'react'
import { IconAiReview } from '~/apps/review/src/lib/assets/icons'
import { ScorecardQuestion } from '~/apps/review/src/lib/models'

import { ScorecardViewerContextValue, useScorecardContext } from '../../ScorecardViewer.context'
import { ScorecardViewerContextValue, useScorecardViewerContext } from '../../ScorecardViewer.context'
import { ScorecardQuestionRow } from '../ScorecardQuestionRow'
import { ScorecardScore } from '../../ScorecardScore'
import { MarkdownReview } from '../../../../MarkdownReview'

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

Expand All @@ -14,7 +15,7 @@ interface AiFeedbackProps {
}

const AiFeedback: FC<AiFeedbackProps> = props => {
const { aiFeedbackItems }: ScorecardViewerContextValue = useScorecardContext()
const { aiFeedbackItems, scoreMap }: ScorecardViewerContextValue = useScorecardViewerContext()

Choose a reason for hiding this comment

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

[⚠️ correctness]
The scoreMap is accessed with props.question.id cast to a string. Ensure that props.question.id is always a valid key in scoreMap to avoid potential runtime errors. Consider adding a check or handling for cases where the key might not exist in scoreMap.

const feedback = useMemo(() => (
aiFeedbackItems?.find(r => r.scorecardQuestionId === props.question.id)
), [props.question.id, aiFeedbackItems])
Expand All @@ -32,8 +33,7 @@ const AiFeedback: FC<AiFeedbackProps> = props => {
className={styles.wrap}
score={(
<ScorecardScore
score={feedback.questionScore}
scaleMax={props.question.scaleMax}
score={scoreMap.get(props.question.id as string) ?? 0}
weight={props.question.weight}
/>
)}
Expand All @@ -43,7 +43,7 @@ const AiFeedback: FC<AiFeedbackProps> = props => {
<strong>{feedback.questionScore ? 'Yes' : 'No'}</strong>
</p>
)}
{feedback.content}
<MarkdownReview value={feedback.content} />

Choose a reason for hiding this comment

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

[❗❗ security]
Rendering feedback.content using MarkdownReview assumes that feedback.content is always a valid markdown string. Consider adding validation or sanitization to ensure that the content is safe and correctly formatted before rendering.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@vas3a very nice - Markdown support for AI generated feedback, right?!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

right!

</ScorecardQuestionRow>
)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
.wrap {
}

.select {
min-width: 200px;
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import { FC, useMemo } from 'react'

import { ReviewItemInfo, ScorecardQuestion } from '../../../../../../models'
import { ScorecardViewerContextValue, useScorecardViewerContext } from '../../../ScorecardViewer.context'
import { ScorecardQuestionRow } from '../../ScorecardQuestionRow'
import { ScorecardScore } from '../../../ScorecardScore'

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

interface ReviewAnswerProps {
question: ScorecardQuestion
reviewItem: ReviewItemInfo
}

const ReviewAnswer: FC<ReviewAnswerProps> = props => {
const {
isManagerEdit,
scoreMap,
}: ScorecardViewerContextValue = useScorecardViewerContext()

const answer = useMemo(() => (

Choose a reason for hiding this comment

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

[💡 performance]
Using useMemo here might be unnecessary since string concatenation and logical OR operations are inexpensive. Consider removing useMemo unless you have a specific performance concern.

props.reviewItem.finalAnswer || props.reviewItem.initialAnswer || ''
), [props.reviewItem.finalAnswer, props.reviewItem.initialAnswer])

if (!answer && !isManagerEdit) {
return <></>
}

return (
<ScorecardQuestionRow
index='Review Response'
className={styles.wrap}
score={(
<ScorecardScore
score={scoreMap.get(props.question.id as string) ?? 0}

Choose a reason for hiding this comment

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

[⚠️ correctness]
Ensure that props.question.id is always a string or handle the case where it might not be. Using as string could lead to runtime errors if id is not a string.

weight={props.question.weight}
/>
)}
>
<p>
<strong>{answer}</strong>
</p>
</ScorecardQuestionRow>
)
}

export default ReviewAnswer
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { default as ReviewAnswer } from './ReviewAnswer'
Loading