Repository navigation
FEAT: Shared scan result building blocks and ScenarioRunPage split (part 2) - #4031
Open
Richard Lundeen (richlundeen) wants to merge 11 commits into
Open
Richard Lundeen (richlundeen) wants to merge 11 commits into
Richard Lundeen (richlundeen) wants to merge 11 commits into
Conversation
- GET /api/attacks/{id}/findings lists findings linked to an attack across Operations,
backed by a new FindingEvidenceEntries.attack_result_id index (Alembic b5d7f9a1c3e6).
- Attack read APIs (summary, metadata, conversations, messages) resolve scan attacks that
are still running through a read-only LiveAttackLookup provided by ScenarioRunService.
Saved results always win; summaries/metadata expose in_progress and scenario_result_id.
- ScenarioActiveConversation.conversation_ids lists every objective conversation so far.
- ScenarioProgressScore carries score_category for progress results and hierarchy nodes.
- Frontend types and attacksApi.getAttackFindings for later layers.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Split ScenarioRunPage into section components and hooks, and add the shared blocks the scan results redesign builds on: OperationParentLink, ScanStatusBadge, OutcomeBadge progress labels and score popover, ScoreDetails over a narrow ScoreSummary, DetailFacts, IdentityDisclosure, AttackActivity, formatAttemptTiming, useAttackFindings, and AttachToFindingsButton. Existing pages adopt them only where output is unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The popover-enabled badge’s accessible name omits its visible outcome, making success, failure, and error indistinguishable to screen-reader users without opening each popover.
1 open issue
What changed in this PR
This adds the frontend foundations for the scan-results redesign, building on #4030’s backend APIs. It splits ScenarioRunPage into focused sections and hooks, following #3526’s approach, while preserving the existing layout and improving attack-dialog focus return.
Changes:
- Extracts run controls, attack selection, navigation, and page sections.
- Centralizes status badges, score details, identity disclosure, and reusable result components.
- Adds findings-loading and attachment helpers for the next redesign layer.
| File | Description |
|---|---|
| tests/unit/memory/test_score_participation_migration.py | Allows a newer single migration head. |
| tests/unit/memory/test_findings.py | Tests cross-operation attack findings. |
| tests/unit/memory/test_finding_evidence_attack_index_migration.py | Tests index migration and backend compatibility. |
| tests/unit/memory/memory_interface/test_interface_scenario_progress.py | Verifies progress score categories. |
| tests/unit/backend/test_scenario_resume.py | Tests live-to-saved attack reads. |
| tests/unit/backend/test_live_attack_reads.py | Covers live reads and attack findings APIs. |
| pyrit/models/scenario_progress.py | Adds categories and objective conversation IDs. |
| pyrit/memory/memory_models.py | Declares the attack-evidence lookup index. |
| pyrit/memory/memory_interface.py | Shares reference paging and exposes findings/categories. |
| pyrit/memory/alembic/versions/b5d7f9a1c3e6_index_finding_evidence_attack_result.py | Adds the attack-evidence lookup index. |
| pyrit/backend/services/scenario_run_service.py | Exposes active attack lookup and conversations. |
| pyrit/backend/services/saved_conversation_read_service.py | Accepts an injectable reference reader. |
| pyrit/backend/services/operation_service.py | Lists findings linked to an attack. |
| pyrit/backend/services/live_attack_reader.py | Defines read-only live attack projections. |
| pyrit/backend/services/attack_service.py | Supports live attacks in read endpoints. |
| pyrit/backend/routes/attacks.py | Adds the attack findings endpoint. |
| pyrit/backend/README.md | Documents live reads and findings contracts. |
| pyrit/backend/models/operations.py | Defines attack findings responses. |
| pyrit/backend/models/attacks.py | Adds live-state and scan provenance fields. |
| pyrit/backend/mappers/attack_mappers.py | Maps live-state and provenance fields. |
| frontend/src/utils/scoreSummary.ts | Normalizes score data for shared presentation. |
| frontend/src/utils/scoreSummary.test.ts | Tests score adapters and fallback values. |
| frontend/src/types/index.ts | Adds shared display and API contracts. |
| frontend/src/services/api.ts | Adds attack findings retrieval. |
| frontend/src/services/api.test.ts | Tests findings requests and failures. |
| frontend/src/hooks/useScenarioRunControls.ts | Extracts rerun, cancel, and resume controls. |
| frontend/src/hooks/useScenarioRunAttackDetails.ts | Extracts attack selection and focus restoration. |
| frontend/src/hooks/useAttackFindings.ts | Loads and refreshes linked findings. |
| frontend/src/hooks/useAttackFindings.test.tsx | Tests findings loading and state transitions. |
| frontend/src/components/ScenarioSuites/ScenarioSuiteExecutionPage.tsx | Adopts shared scan status badges. |
| frontend/src/components/Scenarios/ScenarioTechniqueSection.tsx | Uses shared identity disclosure styles. |
| frontend/src/components/Scenarios/ScenarioRunProgress.tsx | Extracts progress and timing display. |
| frontend/src/components/Scenarios/ScenarioRunPage.tsx | Composes extracted sections and hooks. |
| frontend/src/components/Scenarios/ScenarioRunOverview.tsx | Extracts scan summary facts. |
| frontend/src/components/Scenarios/ScenarioRunObjectivesSection.tsx | Extracts dataset and objective results. |
| frontend/src/components/Scenarios/ScenarioRunNotices.tsx | Extracts errors and progress notices. |
| frontend/src/components/Scenarios/scenarioRunNavigation.ts | Centralizes run back-link rules. |
| frontend/src/components/Scenarios/scenarioRunNavigation.test.ts | Tests back-link destinations and labels. |
| frontend/src/components/Scenarios/ScenarioRunHeader.tsx | Extracts title, status, and controls. |
| frontend/src/components/Scenarios/ScenarioRunDialogs.tsx | Adds the extracted cancellation dialog. |
| frontend/src/components/Scenarios/ScenarioRunConversations.tsx | Extracts executing and saved conversations. |
| frontend/src/components/Scenarios/ScenarioRunConfigurationSection.tsx | Extracts saved configuration disclosure. |
| frontend/src/components/Scenarios/AttackExecutionTable.tsx | Uses shared outcome label styles. |
| frontend/src/components/Scenarios/AttackDetailsDialog.tsx | Extracts attack details presentation. |
| frontend/src/components/Scenarios/AttackActivity.tsx | Adds a reusable activity graphic. |
| frontend/src/components/Scenarios/AttackActivity.test.tsx | Tests activity labels and loading states. |
| frontend/src/components/Scenarios/AttackActivity.styles.ts | Styles activity with reduced-motion support. |
| frontend/src/components/Scenarios/AtomicAttackGroupsSection.tsx | Extracts expandable attack groups. |
| frontend/src/components/ScanStatusBadge.tsx | Centralizes lifecycle labels, colors, and icons. |
| frontend/src/components/ScanStatusBadge.test.tsx | Tests status labels and optional icons. |
| frontend/src/components/Registry/RegistryIdentityRow.tsx | Reuses shared disclosure components. |
| frontend/src/components/Registry/RegistryIdentityRow.styles.ts | Retains row-specific styling. |
| frontend/src/components/Registry/IdentityDisclosure.tsx | Adds reusable identity JSON disclosure. |
| frontend/src/components/Registry/IdentityDisclosure.test.tsx | Tests disclosure behavior and click isolation. |
| frontend/src/components/Registry/IdentityDisclosure.styles.ts | Centralizes disclosure styling. |
| frontend/src/components/OutcomeBadge.tsx | Adds pending states and score popovers. |
| frontend/src/components/OutcomeBadge.test.tsx | Covers new labels and popover behavior. |
| frontend/src/components/OutcomeBadge.styles.ts | Styles pending states and score triggers. |
| frontend/src/components/Operations/OperationParentLink.tsx | Adds reusable parent-operation navigation. |
| frontend/src/components/Operations/OperationParentLink.test.tsx | Tests links and missing-operation behavior. |
| frontend/src/components/Operations/OperationParentLink.styles.ts | Styles parent-operation links. |
| frontend/src/components/Home/Home.tsx | Adopts shared scan status badges. |
| frontend/src/components/DetailFacts.tsx | Adds reusable description-list components. |
| frontend/src/components/DetailFacts.test.tsx | Tests label/value semantics. |
| frontend/src/components/DetailFacts.styles.ts | Adds responsive fact-list layout. |
| frontend/src/components/Chat/ScoreDetails.tsx | Extracts score presentation using ScoreSummary. |
| frontend/src/components/Chat/ScoreDetails.test.tsx | Tests score details and empty sections. |
| frontend/src/components/Chat/ScoreDetails.styles.ts | Extracts shared score-detail styling. |
| frontend/src/components/Chat/MessageList.tsx | Uses shared score details and adapters. |
| frontend/src/components/Chat/MessageList.styles.ts | Removes extracted score-detail styles. |
| frontend/src/components/Chat/FindingEvidenceDialog.tsx | Reports successful attachments via callback. |
| frontend/src/components/Chat/AttachToFindingsButton.tsx | Wraps attachment dialog and focus return. |
| frontend/src/components/Chat/AttachToFindingsButton.test.tsx | Tests attachment, cancellation, and disabled states. |
| frontend/src/components/Chat/AttachToFindingsButton.styles.ts | Applies touch-target sizing. |
| frontend/src/components/AttackResults/attackAttemptFormatting.ts | Adds retry and combined timing formatting. |
| frontend/src/components/AttackResults/attackAttemptFormatting.test.ts | Tests retry and timing output. |
Review effort: Balanced
Integrate main 25417ff without rewriting the existing stack history. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Limit running-state guarantees to attack summary and metadata reads without expanding conversation/message schemas or removing score details. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Carry forward the verified running-state documentation correction and the predecessor main-integration lockfile state. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Preserve the visible verdict in score-trigger accessible names, cover saved and pending outcomes plus custom labels, and prove populated findings survive refresh before replacement. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
Author
|
The accessible-name concern in this review is fixed in 343e600. The badge keeps its visible outcome in the score action name; the existing inline thread has the detailed correction. |
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
Integrate confirmed CI base c979a97 without rewriting either Alembic branch. The reproduced dual-head conflict is repaired in the following commit before publication. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Preserve both published revision histories and restore singular-head backend startup. Cover fresh, common-parent, either-branch and dual-head upgrades with preserved data, idempotency, downgrade/re-upgrade and SQLite/SQL Server merge semantics. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Carry forward the reviewed no-DDL Alembic merge revision and pinned upstream integration while preserving the foundations repairs and published visual walkthrough. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Richard Lundeen (richlundeen)
left a comment
Richard Lundeen (richlundeen)
left a comment
Contributor
Author
There was a problem hiding this comment.
Automated review completed. No blocking concerns remain. This review records source coverage; it does not authorize merge.
Richard Lundeen (richlundeen)
changed the title
Integrate microsoft/PyRIT main at 8253694. Preserve live attack reads, findings, score provenance and both migration histories while adopting canonical public imports. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Behnam (behnam-o)
requested changes
Nov 11, 2026
Behnam (behnam-o)
left a comment
Behnam (behnam-o)
left a comment
Contributor
There was a problem hiding this comment.
Reviewed this layer's own changes (d69f177..36ca73f); part 1 is reviewed on #4030. The ScenarioRunPage split and the shared badge, score and identity components look behavior-preserving, and I didn't find any security issues.
Requesting changes because a large part of this PR has no caller until #4032: new components and hooks, plus new options on shared components that are already in use. Since each PR merges into main on its own, please move those into #4032 next to their callers.
This was a static review; I didn't run the code or tests.
Comment on lines
+19
to
+22
| /** Opens the finding picker to attach one attack as evidence, returning focus to the button. */ | ||
| export default function AttachToFindingsButton({ | ||
| attackResultId, defaultOperationId = null, disabled = false, onAttached, | ||
| }: AttachToFindingsButtonProps) { |
Contributor
There was a problem hiding this comment.
[MEDIUM] Part-3 building blocks have no caller in this PR
Outside tests, nothing in this PR renders AttachToFindingsButton. The same goes for OperationParentLink, DetailFacts, AttackActivity, useAttackFindings, the new helpers in attackAttemptFormatting.ts (lines 79-93), and the new onAttached prop on FindingEvidenceDialog, which the live dialog in ChatWindow doesn't pass. The PR description itself says the new blocks are unused until layer 3. Since each PR merges into main on its own, please drop them here and add them in #4032 with their callers.
Comment on lines
+108
to
+121
| if (!score && !scoreFallback) return badge | ||
|
|
||
| return ( | ||
| <Popover withArrow> | ||
| <PopoverTrigger disableButtonEnhancement> | ||
| <Button appearance="transparent" className={styles.scoreTrigger} aria-label={`${displayLabel}: ${scoreButtonLabel}`}> | ||
| {badge} | ||
| </Button> | ||
| </PopoverTrigger> | ||
| <PopoverSurface> | ||
| {score ? <ScoreDetails score={score} /> : <Text>{scoreFallback}</Text>} | ||
| </PopoverSurface> | ||
| </Popover> | ||
| ) |
Contributor
There was a problem hiding this comment.
[MEDIUM] Shared components gain options that only part 3 uses
None of the 7 production OutcomeBadge call sites passes score, scoreFallback or scoreButtonLabel, so this popover branch can't be reached yet. It still widens a badge used on 7 screens and couples it to Chat/ScoreDetails. The same goes for scoreSummaryFromProgressScore, the default IdentityDisclosure export, chevronPosition and IdentityJson's className, which nothing outside tests uses. Keeping labelStyle, ScoreSummary/ScoreDetails (used by MessageList) and the two extracted identity parts makes sense; please move the rest to #4032.
| }: ScenarioTechniqueSectionProps) { | ||
| const styles = useScenarioDetailStyles() | ||
| const identityStyles = useRegistryIdentityRowStyles() | ||
| const identityStyles = useIdentityDisclosureStyles() |
Contributor
There was a problem hiding this comment.
[LOW] This section still has its own JSON reveal
The PR description says the app now has one JSON reveal implementation, but this section only switches to IdentityDisclosure's internal details/json styles and keeps its own role="region" + <pre> markup (lines 177-181). It already behaves differently from IdentityJson (it doesn't scroll into view), and restyling IdentityJson would silently change it. Could this render IdentityJson when an identifier exists (keeping the fallback text inline), or could the description be narrowed?
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Layer 2 of 3 for the scanner results redesign. It adds one shared implementation for each concept the redesigned scan pages use, and splits
ScenarioRunPageinto sections, without visible changes to existing pages. Layer 3 builds the redesign on these pieces.This layer is a refactor plus building blocks that nothing uses yet, so existing pages look exactly the same. The only behavior change is focus returning to the control that opened attack details, which a screenshot cannot show. To see the new pieces, use the visual gallery above. The redesign that uses them, with visible changes, is #4032.
ScenarioRunHeader,ScenarioRunOverview,ScenarioRunNotices,ScenarioRunProgress,ScenarioRunConversations,AtomicAttackGroupsSection,ScenarioRunObjectivesSection,ScenarioRunConfigurationSection,AttackDetailsDialog, andCancelScanDialog. Rerun, cancel, and resume state lives inuseScenarioRunControls. Route-addressed attack selection and focus return live inuseScenarioRunAttackDetails. The back-link rules moved toscenarioRunBackLink.STATE_COLORS(run page),SCAN_STATUS_*(Home), and*_BADGE_COLORS+statusIcon(suite execution page). The suite page keeps its "In progress" wording throughlabel.in_progress/preparing(AttackDisplayOutcome),labelStyle('success'→ True/False,'outcome'→ Success/Failure), and an optional score popover (score,scoreFallback,scoreButtonLabel).AttackExecutionTableuseslabelStyleinstead of hand-built labels. Score triggers retain the visible outcome in their accessible name.MessageListintoChat/ScoreDetailsand takes a narrowScoreSummarybuilt byscoreSummaryFromBackendScore/scoreSummaryFromProgressScore. Scan results no longer need a fabricatedBackendScore.RegistryIdentityRownow uses itsIdentityDisclosureButton/IdentityJsonparts, so the app has one JSON reveal implementation.dlwith a 9rem label column that stacks on narrow viewports.attacksApi.getAttackFindings; loaded findings remain visible during a same-attack refresh.FindingEvidenceDialog, which gains anonAttachedcallback so callers can refresh the findings hook.Behavior note: closing attack details now always returns focus to the control that opened them. Before, opening an attack from an "Executing" row could send focus back to an earlier trigger.