-
Notifications
You must be signed in to change notification settings - Fork 21
PM-2178 merge review UI #1309
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
PM-2178 merge review UI #1309
Changes from all commits
5eee5b1
08142e9
ffb901b
e33c043
621d909
e0a9add
f835cd0
57fcb0d
cd87b3c
25c4c5e
e772539
9a6a47e
274048e
5213a02
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| import { FC, useMemo } from 'react' | ||
| import { FC, MouseEvent as ReactMouseEvent, useMemo } from 'react' | ||
| import moment from 'moment' | ||
|
|
||
| import { useWindowSize, WindowSize } from '~/libs/shared' | ||
|
|
@@ -22,6 +22,10 @@ interface AiReviewsTableProps { | |
| 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) | ||
|
|
@@ -102,7 +106,7 @@ const AiReviewsTable: FC<AiReviewsTableProps> = props => { | |
| } | ||
|
|
||
| return ( | ||
| <div className={styles.wrap}> | ||
| <div className={styles.wrap} onClick={stopPropagation}> | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [ |
||
| <table className={styles.reviewsTable}> | ||
| <thead> | ||
| <tr> | ||
|
|
@@ -145,7 +149,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}`} | ||
kkartunov marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| > | ||
| {run.score} | ||
| </a> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 '' | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [ |
||
| }, [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} | ||
|
|
@@ -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} | ||
|
|
@@ -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} | ||
|
|
@@ -54,7 +70,7 @@ export const AiWorkflowRunStatus: FC<AiWorkflowRunStatusProps> = props => { | |
| score={score} | ||
| /> | ||
| )} | ||
| {isFailed && ( | ||
| {status === 'failed' && ( | ||
| <StatusLabel | ||
| icon={<IconOutline.XCircleIcon className='icon-xl' />} | ||
| status={status} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,36 +1,31 @@ | ||
| 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, toggleItem, toggledItems }: ScorecardViewerContextValue = useScorecardViewerContext() | ||
|
|
||
| const isVissible = !toggledItems[props.group.id] | ||
| const isVisible = !toggledItems[props.group.id] | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [ |
||
| 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}> | ||
| <div className={classNames(styles.headerBar, isVisible && styles.toggled)} onClick={toggle}> | ||
| <span className={styles.index}> | ||
| {props.index} | ||
| . | ||
|
|
@@ -41,8 +36,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> | ||
|
|
@@ -51,8 +45,13 @@ const ScorecardGroup: FC<ScorecardGroupProps> = props => { | |
| </span> | ||
| </div> | ||
|
|
||
| {isVissible && props.group.sections.map((section, index) => ( | ||
| <ScorecardSection key={section.id} section={section} index={[props.index, index + 1].join('.')} /> | ||
| {isVisible && props.group.sections.map((section, index) => ( | ||
| <ScorecardSection | ||
| key={section.id} | ||
| section={section} | ||
| index={[props.index, index + 1].join('.')} | ||
| reviewItemMapping={props.reviewItemMapping} | ||
| /> | ||
| ))} | ||
| </div> | ||
| ) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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' | ||
|
|
||
|
|
@@ -14,7 +15,7 @@ interface AiFeedbackProps { | |
| } | ||
|
|
||
| const AiFeedback: FC<AiFeedbackProps> = props => { | ||
| const { aiFeedbackItems }: ScorecardViewerContextValue = useScorecardContext() | ||
| const { aiFeedbackItems, scoreMap }: ScorecardViewerContextValue = useScorecardViewerContext() | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [ |
||
| const feedback = useMemo(() => ( | ||
| aiFeedbackItems?.find(r => r.scorecardQuestionId === props.question.id) | ||
| ), [props.question.id, aiFeedbackItems]) | ||
|
|
@@ -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} | ||
| /> | ||
| )} | ||
|
|
@@ -43,7 +43,7 @@ const AiFeedback: FC<AiFeedbackProps> = props => { | |
| <strong>{feedback.questionScore ? 'Yes' : 'No'}</strong> | ||
| </p> | ||
| )} | ||
| {feedback.content} | ||
| <MarkdownReview value={feedback.content} /> | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [❗❗
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @vas3a very nice - Markdown support for AI generated feedback, right?!
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. right! |
||
| </ScorecardQuestionRow> | ||
| ) | ||
| } | ||
|
|
||
| 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(() => ( | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [💡 |
||
| 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} | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [ |
||
| 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' |
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
aiScorecardRouteIdandscorecardRouteIdare 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.