FEAT: Guided Onboarding Tour for Co-PyRIT#1981
Open
jsong468 wants to merge 16 commits into
Open
Conversation
… target creation and display in gui
Mirrors the dedup logic in TargetInitializer._auto_group_targets so the GUI/API flow can't produce a round-robin that hits the same underlying endpoint twice. Weights for deduped entries are dropped alongside them. If fewer than 2 distinct targets remain after dedup, raise a clear error listing the skipped duplicate names. Addresses review comment on PR microsoft#1944. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…, fix isCompatible fallback
Addresses Roman's review comment on isCompatible() in CreateTargetDialog:
1. Hash-based dedup in the picker (Option A from review discussion).
- Surfaces ComponentIdentifier.hash as a new optional identifier_hash field
on the TargetInstance DTO.
- target_object_to_instance now populates it; the existing recursive
_build_inner_targets call propagates it to inner targets for free.
- eligibleTargets in CreateTargetDialog filters out candidates whose
identifier_hash matches any already-selected target's hash. Targets with
null/undefined hash are NOT collapsed together (safe handling).
- This plugs the last gap left by isCompatible: behavioral-param mismatches
are already pre-filtered, but two registry entries that resolve to the
same backend config (same hash) were still pickable and would hit the
service-layer dedup error from the previous commit.
2. Fix isCompatible() fallback bug (Roman's specific repro).
- Adds effectiveUnderlyingModel(t) helper using || (catches both null AND
empty string, matching the backend's �alue is None or value == "" rule
in RoundRobinTarget._resolve_param).
- isCompatible() now compares effective underlying models so that, for
example, picking azure_foundry_deepseek (DeepSeek-R1, underlying=None)
no longer shows azure_foundry_mistral_large / google_gemini / ollama
as compatible just because all four have underlying_model_name=None.
3. Surface RFC 7807 backend detail on failed creation.
- Replaces the catch block's err.message with toApiError(err).detail so the
dialog shows the actual validation message (e.g. "Behavioral parameter
'underlying_model_name' differs...") instead of the generic axios
"Request failed with status code 400".
- Updates one pre-existing test ("non-Error exceptions") that expected a
hard-coded "Failed to create target" string — toApiError now surfaces the
thrown string verbatim, which is strictly more informative.
4. Extend the frontend/backend drift guard.
- Adds test_target_eval_param_fallbacks_match_frontend asserting
TARGET_EVAL_PARAM_FALLBACKS == {'underlying_model_name': 'model_name'}
so future fallback additions surface a clear failure pointing at
effectiveUnderlyingModel() in CreateTargetDialog.tsx.
Tests added:
- backend: test_target_eval_param_fallbacks_match_frontend
- backend: extended test_maps_target_with_identifier to cover identifier_hash
- frontend: filters duplicate-by-identifier-hash targets out of the picker
- frontend: applies underlying_model_name -> model_name fallback when filtering
- frontend: surfaces the backend error detail when target creation fails
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Roman's review comment #2: parseInt("2.5") silently truncates to 2, parseInt("1e10") returns 1, and 99999999999 was accepted with no upper bound. Typing 0 silently reverted. Changes: - Extract parseWeight + MAX_WEIGHT=1000 to weightValidation.ts - Strict regex-based parser rejects empty, non-integers (2.5, 1e10, -3), values < 1, and values > MAX_WEIGHT - Refactor SelectedInnerTarget to single source of truth (weightInput: string), avoiding silent revert - Add step='1', min='1', max='1000', aria-invalid, aria-label to Input; render alert below row on invalid - Disable Create when any weight is invalid; re-validate in handleSubmit to defend against Enter-key bypass Tests: 9 parseWeight unit tests + 2 dialog integration tests (alert+disabled, end-to-end submit with parsed ints). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Reformat multi-line f-string in target_service.py per ruff-format - Add 3 dialog tests covering removeInnerTarget, submit-time weight re-validation, and the <2 inner targets guard, lifting branch coverage from 84.97% to 86.23% Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
can you include gif or something that displays the feature in the description ? |
Contributor
|
okay just a thought--what if we added a raccoon walking you through the gui ? like just a little animated guy (this may be far more difficult than i'm envisioning) |
- Add missing 'endTour' dependency to handleJoyrideEvent useCallback (fixes react-hooks/exhaustive-deps lint warning with --max-warnings 0) - Mock react-joyride and useTour in App.test.tsx to prevent the tour's auto-start from triggering uncontrolled state updates that race with async label initialization from the version API Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The onboarding tour auto-starts on first visit when localStorage has no 'pyrit-tour-completed' flag. In E2E tests this causes the Joyride overlay to block all page interactions, leading to 30s timeouts on 39 tests. Pre-set the localStorage flag via Playwright's storageState config so the tour is already marked complete before any E2E test navigates to the app. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The React compiler eslint rule forbids assigning ref.current during
render ('Cannot access refs during render'). Move the assignment into
a useEffect so it runs after commit, which satisfies the rule while
keeping the ref always in sync with currentView.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
rlundeen2
reviewed
Jun 12, 2026
| /** Whether the user has completed the tour at least once. */ | ||
| hasCompletedTour, | ||
| /** Props to spread onto the `<Joyride>` component. */ | ||
| tourProps: { |
Contributor
There was a problem hiding this comment.
Rich agrees with this comment but it is copilot generated:
tourProps and its nested arrays/objects are recreated on every render: steps: [...TOUR_STEPS], options, floatingOptions, and locale are all fresh references each time. Since App re-renders frequently (label/target/status state), <Joyride> receives new props on every render — only tooltipComponent is memoized. Even with controlled run/stepIndex, handing Joyride a new steps/options reference each render risks internal re-initialization/glitches mid-tour. Suggest hoisting the static pieces to module constants (or useMemo) and memoizing the returned tourProps.
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
3 participants
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
Adds a ROAKEY-guided observe-only tour to the Co-PyRIT GUI using React Joyride v3. The tour walks first-time users through 5 key areas of the app across 3 views (Home → Chat → History), explaining what each section does without requiring a working target or any user action. It auto-starts on first visit and can be replayed via a new "Take a Tour" button in the sidebar.
Demo:
Full end-to-end tour from "take a tour" button on nav bar
fulltourguide.mp4
Light mode and "close" button
Light.mode.and.close.tour.mp4
Tour upon Co-PyRIT launch, "back" button, and "skip tour" button
Tour.upon.launch.and.going.back.and.skip.tour.mp4
Tour steps:
Key design decisions:
stepIndexprop with a 400ms delay after each switch to let React mount the new DOM.currentViewRef.current(the real app state) rather than the step definition. This prevents a freeze if the user manually navigates to a different view during the tour.FluentProvider.TourTooltipwraps its content in its ownFluentProviderwith the current theme (prop-drilled fromApp.tsx) so Fluent components and design tokens resolve correctly.closeButtonAction: 'close'advances to the next step internally before firingonEvent, causing the view to snap forward. SettingcloseButtonAction: 'skip'andoverlayClickAction: falseensures all exit paths (X button, Skip, overlay click) cleanly stop the tour.localStorage('pyrit-tour-completed'). Auto-starts only on first visit; the sidebar button replays regardless.New files:
frontend/src/components/Tour/tourSteps.ts— Step definitions (targets, content, placements, required views)frontend/src/components/Tour/TourTooltip.tsx— Custom tooltip styled with Fluent UIfrontend/src/components/Tour/TourTooltip.styles.ts—makeStyleshook for tooltip cardfrontend/src/hooks/useTour.ts— Hook managing tour state, cross-view navigation, and localStoragefrontend/src/components/Tour/TourTooltip.test.tsx— Unit tests for tooltip renderingfrontend/src/hooks/useTour.test.ts— Unit tests for hook state machineModified files:
frontend/package.json— Addedreact-joyride@^3.1.0frontend/src/App.tsx— Render<Joyride>, auto-start on first visit, passstartTourto layoutfrontend/src/components/Layout/MainLayout.tsx— PassonStartTourprop through to Navigationfrontend/src/components/Sidebar/Navigation.tsx— Addeddata-tourattribute and "Take a Tour" buttonfrontend/src/components/Home/Home.tsx— Addeddata-tourattributes on labels and target cardsfrontend/src/components/Chat/ChatWindow.tsx— Addeddata-tourattribute on the ribbonfrontend/src/components/History/AttackHistory.tsx— Addeddata-tourattribute on the headerTests & Documentation
Unit tests (25 total, all passing):
alertdialogrole propagation, and light mode rendering.startTournavigation + delayed run, step advancement on Next/Back, tour stop on Close/Skip/Finished, cross-view navigation with delay, actual-view-based navigation (manual view switch edge case), lifecycle filtering (ignores non-COMPLETE events), double-advance prevention during view switch delay, and bounds checking past the last step.Manual testing performed:
Documentation: No external docs needed. The tour is self-documenting — step content in
tourSteps.tsis the single source of truth. To reset the tour for testing:localStorage.removeItem('pyrit-tour-completed')