-
Notifications
You must be signed in to change notification settings - Fork 808
FEAT: GUI view existing scores #2189
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
base: main
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| import { makeStyles, tokens } from '@fluentui/react-components' | ||
|
|
||
| export const useAttackVerdictChipStyles = makeStyles({ | ||
| chip: { | ||
| display: 'inline-flex', | ||
| alignItems: 'center', | ||
| columnGap: tokens.spacingHorizontalXS, | ||
| }, | ||
| chipLabel: { | ||
| textTransform: 'capitalize', | ||
| }, | ||
| surface: { | ||
| display: 'flex', | ||
| flexDirection: 'column', | ||
| rowGap: tokens.spacingVerticalXS, | ||
| minWidth: '240px', | ||
| maxWidth: '360px', | ||
| }, | ||
| row: { | ||
| display: 'flex', | ||
| columnGap: tokens.spacingHorizontalS, | ||
| }, | ||
| rowLabel: { | ||
| minWidth: '72px', | ||
| color: tokens.colorNeutralForeground2, | ||
| }, | ||
| rationaleBlock: { | ||
| display: 'flex', | ||
| flexDirection: 'column', | ||
| rowGap: tokens.spacingVerticalXXS, | ||
| marginTop: tokens.spacingVerticalXS, | ||
| paddingTop: tokens.spacingVerticalXS, | ||
| borderTop: `1px solid ${tokens.colorNeutralStroke2}`, | ||
| }, | ||
| rationaleText: { | ||
| color: tokens.colorNeutralForeground2, | ||
| whiteSpace: 'pre-wrap', | ||
| wordBreak: 'break-word', | ||
| maxHeight: '30vh', | ||
| overflowY: 'auto', | ||
| }, | ||
| }) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| import { render, screen, within } from '@testing-library/react' | ||
| import userEvent from '@testing-library/user-event' | ||
| import { FluentProvider, webLightTheme } from '@fluentui/react-components' | ||
| import AttackVerdictChip from './AttackVerdictChip' | ||
| import type { ScoreView } from '../../types' | ||
|
|
||
| const TestWrapper: React.FC<{ children: React.ReactNode }> = ({ children }) => ( | ||
| <FluentProvider theme={webLightTheme}>{children}</FluentProvider> | ||
| ) | ||
|
|
||
| const sampleScore: ScoreView = { | ||
| id: 'score-1', | ||
| scorer_type: 'SelfAskRefusalScorer', | ||
| score_type: 'true_false', | ||
| score_value: 'true', | ||
| score_category: ['refusal'], | ||
| score_rationale: 'The model refused the request.', | ||
| timestamp: '2026-01-15T11:00:00Z', | ||
| } | ||
|
|
||
| describe('AttackVerdictChip', () => { | ||
| it('renders nothing when there is no score', () => { | ||
| render( | ||
| <TestWrapper> | ||
| <AttackVerdictChip outcome="failure" score={null} /> | ||
| </TestWrapper> | ||
| ) | ||
|
|
||
| expect(screen.queryByTestId('attack-score-chip')).not.toBeInTheDocument() | ||
| }) | ||
|
|
||
| it('shows only the score value on the chip, labeled "Score"', () => { | ||
| render( | ||
| <TestWrapper> | ||
| <AttackVerdictChip outcome="failure" score={sampleScore} /> | ||
| </TestWrapper> | ||
| ) | ||
|
|
||
| const chip = screen.getByRole('button', { name: /score true/i }) | ||
| expect(within(chip).getByText('Score')).toBeInTheDocument() | ||
| expect(within(chip).getByText('true')).toBeInTheDocument() | ||
| // The outcome word must never appear on the chip itself. | ||
| expect(within(chip).queryByText(/failure/i)).not.toBeInTheDocument() | ||
| }) | ||
|
|
||
| it('opens a popover with full score details when clicked', async () => { | ||
| const user = userEvent.setup() | ||
| render( | ||
| <TestWrapper> | ||
| <AttackVerdictChip outcome="failure" score={sampleScore} /> | ||
| </TestWrapper> | ||
| ) | ||
|
|
||
| await user.click(screen.getByRole('button', { name: /score true/i })) | ||
|
|
||
| const details = await screen.findByTestId('attack-score-details') | ||
| expect(within(details).getByText('true_false')).toBeInTheDocument() | ||
| expect(within(details).getByText('SelfAskRefusalScorer')).toBeInTheDocument() | ||
| expect(within(details).getByText('refusal')).toBeInTheDocument() | ||
| expect(within(details).getByText('The model refused the request.')).toBeInTheDocument() | ||
| }) | ||
| }) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,86 @@ | ||
| import { | ||
| Button, | ||
| Text, | ||
| Badge, | ||
| Popover, | ||
| PopoverTrigger, | ||
| PopoverSurface, | ||
| } from '@fluentui/react-components' | ||
| import type { AttackOutcome, ScoreView } from '../../types' | ||
| import { resolveOutcome } from '../../utils/attackOutcome' | ||
| import { getScoreColor } from '../../utils/scoreColor' | ||
| import { useAttackVerdictChipStyles } from './AttackVerdictChip.styles' | ||
|
|
||
| interface AttackVerdictChipProps { | ||
| outcome?: AttackOutcome | null | ||
| score?: ScoreView | null | ||
| } | ||
|
|
||
| export default function AttackVerdictChip({ outcome, score }: AttackVerdictChipProps) { | ||
| const styles = useAttackVerdictChipStyles() | ||
|
|
||
| // Only the score value is surfaced in the ribbon, so there's nothing to show without a score. | ||
| if (!score) return null | ||
|
Contributor
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. Early-returning on
Contributor
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. I agree, maybe we can show at least the attack outcome badge (exactly as is shown in the attack table), and if scores are present who them in the click/hover effect? so attack verdict means (outcome + score), and the AttackVerdictChip shows exactly those |
||
|
|
||
| const resolvedOutcome = resolveOutcome(outcome) | ||
| const categories = score.score_category?.filter(Boolean) ?? [] | ||
| const scoreColor = getScoreColor(score.score_type, score.score_value) | ||
| const scoreBadgeStyle = { | ||
| backgroundColor: scoreColor.background, | ||
| borderColor: scoreColor.background, | ||
| color: scoreColor.foreground, | ||
| } | ||
|
|
||
| return ( | ||
| <Popover withArrow> | ||
| <PopoverTrigger disableButtonEnhancement> | ||
| <Button | ||
| appearance="subtle" | ||
| className={styles.chip} | ||
| data-testid="attack-score-chip" | ||
| aria-label={`Score ${score.score_value}`} | ||
| > | ||
| <span className={styles.chipLabel}>Score</span> | ||
| <Badge appearance="filled" size="medium" style={scoreBadgeStyle}> | ||
| {score.score_value} | ||
| </Badge> | ||
| </Button> | ||
| </PopoverTrigger> | ||
| <PopoverSurface> | ||
| <div className={styles.surface} data-testid="attack-score-details"> | ||
| <Text weight="semibold" size={300}>Score</Text> | ||
| <div className={styles.row}> | ||
| <Text size={200} weight="semibold" className={styles.rowLabel}>Value</Text> | ||
| <Badge appearance="filled" size="small" style={scoreBadgeStyle}> | ||
| {score.score_value} | ||
| </Badge> | ||
| </div> | ||
| <div className={styles.row}> | ||
| <Text size={200} weight="semibold" className={styles.rowLabel}>Type</Text> | ||
| <Text size={200}>{score.score_type}</Text> | ||
| </div> | ||
| <div className={styles.row}> | ||
| <Text size={200} weight="semibold" className={styles.rowLabel}>Scorer</Text> | ||
| <Text size={200}>{score.scorer_type}</Text> | ||
| </div> | ||
| {categories.length > 0 && ( | ||
| <div className={styles.row}> | ||
| <Text size={200} weight="semibold" className={styles.rowLabel}>Category</Text> | ||
| <Text size={200}>{categories.join(', ')}</Text> | ||
| </div> | ||
| )} | ||
| <div className={styles.row}> | ||
| <Text size={200} weight="semibold" className={styles.rowLabel}>Outcome</Text> | ||
| <Text size={200} className={styles.chipLabel}>{resolvedOutcome}</Text> | ||
| </div> | ||
| {score.score_rationale && ( | ||
| <div className={styles.rationaleBlock}> | ||
| <Text size={200} weight="semibold">Rationale</Text> | ||
| <Text size={200} className={styles.rationaleText}>{score.score_rationale}</Text> | ||
| </div> | ||
| )} | ||
| </div> | ||
| </PopoverSurface> | ||
| </Popover> | ||
| ) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,14 +11,16 @@ import ChatInputArea from './ChatInputArea' | |
| import ConversationPanel from './ConversationPanel' | ||
| import ConverterPanel from './ConverterPanel' | ||
| import TargetBadge from './TargetBadge' | ||
| import ObjectiveHeader from './ObjectiveHeader' | ||
| import AttackVerdictChip from './AttackVerdictChip' | ||
| import type { PieceConversion } from './converterTypes' | ||
| import { PIECE_TYPE_TO_DATA_TYPE, basenameFromValue, buildMediaUrl, dataTypeToAttachmentKind, isPathDataType } from './converterTypes' | ||
| import LabelsBar from '../Labels/LabelsBar' | ||
| import type { ChatInputAreaHandle } from './ChatInputArea' | ||
| import { attacksApi } from '../../services/api' | ||
| import { toApiError } from '../../services/errors' | ||
| import { buildMessagePieces, backendMessagesToFrontend } from '../../utils/messageMapper' | ||
| import type { Message, MessageAttachment, TargetInstance, TargetInfo } from '../../types' | ||
| import type { Message, MessageAttachment, TargetInstance, TargetInfo, AttackOutcome, ScoreView } from '../../types' | ||
| import { targetEndpoint, targetModelName, targetType } from '../../utils/targetIdentity' | ||
| import type { ViewName } from '../Sidebar/Navigation' | ||
| import { useChatWindowStyles } from './ChatWindow.styles' | ||
|
|
@@ -42,6 +44,12 @@ interface ChatWindowProps { | |
| isLoadingAttack?: boolean | ||
| /** Number of related (non-main) conversations in the loaded attack. */ | ||
| relatedConversationCount?: number | ||
| /** The loaded attack's objective (empty for new/manual attacks). */ | ||
| objective?: string | ||
| /** The loaded attack's outcome verdict. */ | ||
| outcome?: AttackOutcome | null | ||
| /** The loaded attack's final score, if any. */ | ||
| lastScore?: ScoreView | null | ||
| } | ||
|
|
||
| export default function ChatWindow({ | ||
|
|
@@ -59,6 +67,9 @@ export default function ChatWindow({ | |
| attackTarget, | ||
| isLoadingAttack, | ||
| relatedConversationCount, | ||
| objective = '', | ||
| outcome, | ||
| lastScore, | ||
| }: ChatWindowProps) { | ||
| const styles = useChatWindowStyles() | ||
| const [messages, setMessages] = useState<Message[]>([]) | ||
|
|
@@ -610,6 +621,7 @@ export default function ChatWindow({ | |
| )} | ||
| </div> | ||
| <div className={styles.ribbonActions}> | ||
| <AttackVerdictChip outcome={outcome} score={lastScore} /> | ||
|
Contributor
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. This chip always renders the attack-level
Can we gate this on |
||
| <Tooltip content="Toggle conversations panel" relationship="label"> | ||
| <Button | ||
| appearance="subtle" | ||
|
|
@@ -635,6 +647,7 @@ export default function ChatWindow({ | |
| </Tooltip> | ||
| </div> | ||
| </div> | ||
| <ObjectiveHeader objective={objective} /> | ||
| {systemMessage && <SystemPromptBanner content={systemMessage.content} />} | ||
| <MessageList | ||
| messages={messages} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| import { makeStyles, tokens } from '@fluentui/react-components' | ||
|
|
||
| export const useObjectiveHeaderStyles = makeStyles({ | ||
| root: { | ||
| flexShrink: 0, | ||
| display: 'flex', | ||
| flexDirection: 'row', | ||
| alignItems: 'baseline', | ||
| columnGap: tokens.spacingHorizontalS, | ||
| padding: `${tokens.spacingVerticalS} ${tokens.spacingHorizontalL}`, | ||
| backgroundColor: tokens.colorNeutralBackground2, | ||
| borderBottom: `1px solid ${tokens.colorNeutralStroke1}`, | ||
| borderLeft: `3px solid ${tokens.colorBrandStroke1}`, | ||
| }, | ||
| label: { | ||
| flexShrink: 0, | ||
| }, | ||
| content: { | ||
| flexGrow: 1, | ||
| minWidth: 0, | ||
| color: tokens.colorNeutralForeground1, | ||
| fontSize: tokens.fontSizeBase300, | ||
| }, | ||
| contentCollapsed: { | ||
| whiteSpace: 'nowrap', | ||
| overflow: 'hidden', | ||
| textOverflow: 'ellipsis', | ||
| }, | ||
| contentExpanded: { | ||
| whiteSpace: 'pre-wrap', | ||
| wordBreak: 'break-word', | ||
| maxHeight: '30vh', | ||
| overflowY: 'auto', | ||
| }, | ||
| toggle: { | ||
| flexShrink: 0, | ||
| minWidth: 'auto', | ||
| whiteSpace: 'nowrap', | ||
| color: tokens.colorBrandForeground1, | ||
| }, | ||
| }) |
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.
Minor: every other branch here defaults
objectiveto'', but the success path assignsattack.objectivedirectly. It's typed as required so this is fine today — worth a?? ''for consistency/robustness if the backend ever omits it.