feat(experiments): targeted audience selection in rollout configuration - #8475
feat(experiments): targeted audience selection in rollout configuration#8475Zaimwa9 wants to merge 3 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (6)
💤 Files with no reviewable changes (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe change adds experiment audience types and rollout payload helpers. The experiment wizard can select segments and configure Priority: ⬇️ Low — Defer this experiment audience-targeting change because the supplied scope is a dashboard feature with optional segment selection and display updates, without evidence of elevated external urgency. Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: 🔵 Low · up to This change adds experiment audience targeting through selection, creation, review, and display flows. The feature behavior is covered by supplied payload and eligibility tests, but two frontend convention issues remain open and should be addressed or explicitly accepted before merging. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@themis-blindfold review |
⚖️ Themis review: 🟠 Fix before mergeThe targeted-audience flow is wired through creation, review and the experiment detail view, but the picker still exposes an audience the API rejects. CI completed successfully, including unit tests and changed-file linting.
🟠 Majors
📝 Walkthrough
🧪 How to verify
Automate: Add a picker-level test that asserts rule-less segments are filtered out before the create request is enabled. Product take: A solid experiment-targeting improvement with clear audience visibility across the workflow. 🧭 Assumptions & unverified claimsNo unverified assumptions or claims. Audience targeting lands neatly; this guard keeps the wizard from offering a trapdoor. · reviewed at b2eb767 |
b2eb767 to
71abbc5
Compare
|
@themis-blindfold review |
⚖️ Themis review: ✅ Ship itTargeted audience selection is wired through experiment creation, shown consistently in review and detail views, and preserves the frozen audience during subsequent rollout edits. Completed CI checks, including unit tests and changed-file linting, passed.
⚖️ Acknowledged
📝 Walkthrough
🧪 How to verify
Product take: Solid improvement: experiment authors can constrain exposure to a known audience without losing visibility into the audience they launched. 🧭 Assumptions & unverified claimsNo unverified assumptions or claims. The audience gets a proper guest list now · reviewed at 71abbc5 |
Docker builds report
|
✅ private-cloud · depot-ubuntu-latest-16 — run #20301 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
🗂️ Previous results✅ private-cloud · depot-ubuntu-latest-arm-16 — run #20301 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
✅ oss · depot-ubuntu-latest-16 — run #20301 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)Details
✅ oss · depot-ubuntu-latest-arm-16 — run #20301 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #20292 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
✅ private-cloud · depot-ubuntu-latest-16 — run #20292 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
✅ oss · depot-ubuntu-latest-16 — run #20292 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)Details
✅ oss · depot-ubuntu-latest-arm-16 — run #20292 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
✅ private-cloud · depot-ubuntu-latest-16 — run #20289 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #20289 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
✅ oss · depot-ubuntu-latest-arm-16 — run #20289 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
✅ oss · depot-ubuntu-latest-16 — run #20289 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)Details
|
Visual Regression19 screenshots compared. See report for details. |
There was a problem hiding this comment.
Actionable comments posted: 4
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Team
Run ID: 8d3e076b-4282-4c02-8b1a-1d8b2e290e63
📒 Files selected for processing (21)
frontend/common/types/requests.tsfrontend/common/types/responses.tsfrontend/web/components/SegmentSelect.tsxfrontend/web/components/experiments/AudiencePicker/AudiencePicker.tsxfrontend/web/components/experiments/AudiencePicker/__tests__/utils.test.tsfrontend/web/components/experiments/AudiencePicker/index.tsfrontend/web/components/experiments/AudiencePicker/utils.tsfrontend/web/components/experiments/AudienceSegmentList/AudienceSegmentList.scssfrontend/web/components/experiments/AudienceSegmentList/AudienceSegmentList.tsxfrontend/web/components/experiments/AudienceSegmentList/index.tsfrontend/web/components/experiments/CreateExperimentWizard.tsxfrontend/web/components/experiments/RolloutSummary/RolloutSummary.tsxfrontend/web/components/experiments/WizardStepper/WizardStepper.tsxfrontend/web/components/experiments/__tests__/errors.test.tsfrontend/web/components/experiments/__tests__/rollout.test.tsfrontend/web/components/experiments/errors.tsfrontend/web/components/experiments/results/ExperimentRolloutCard.tsxfrontend/web/components/experiments/rollout.tsfrontend/web/components/experiments/steps/ReviewStep.tsxfrontend/web/components/experiments/steps/RolloutStep.tsxfrontend/web/components/pages/SegmentPage.tsx
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.
891efa1 to
1bd4ebd
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Team
Run ID: 9bdbb74b-1d5e-4b62-931d-3dc57c841ba5
📒 Files selected for processing (4)
frontend/web/components/experiments/AudiencePicker/AudiencePicker.tsxfrontend/web/components/experiments/AudienceSegmentList/AudienceSegmentList.tsxfrontend/web/components/experiments/errors.tsfrontend/web/components/experiments/rollout.ts
Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review.
1bd4ebd to
105eadc
Compare
|
@themis-blindfold review |
| className='audience-segment-list__icon' | ||
| width={18} | ||
| height={18} | ||
| fill='#656D7B' |
There was a problem hiding this comment.
🧹 Nit · ⚡ Quick win
Use the theme token for both audience icons.
Observed: both new #656D7B fills bypass colorIconSecondary; Predicted: they would remain too dark on the dark surface instead of adapting with the theme—use colorIconSecondary for both fills.
⚖️ Themis review: 🧹 Ship it, nits insideTargeted audiences are wired through experiment creation, wizard summaries, and the rollout detail card, while preserving the API's one-segment cap and frozen-audience behaviour. Completed CI checks, including unit, lint, build, and E2E checks, passed.
🧹 Nits
⚖️ Acknowledged
📝 Walkthrough
🧪 How to verify
Automate: render Product take: A solid experiment-control improvement: teams can keep tests focused on the people for whom a hypothesis is relevant. 🧭 Assumptions & unverified claimsNo unverified assumptions or claims. A narrowly targeted feature with one un-targeted hue · reviewed at 9fdbed9 |
talissoncosta
left a comment
There was a problem hiding this comment.
Looks good! only 2 minor questions. Approving it in case you wanna tackle it as a follow up. Good job @Zaimwa9 !
| const environmentDbId = useMemo( | ||
| () => | ||
| (ProjectStore.getEnvironmentIdFromKey(environmentId) as | ||
| | number | ||
| | undefined) ?? undefined, | ||
| [environmentId], | ||
| ) |
There was a problem hiding this comment.
Do you think this could use the RTK hook instead of ProjectStore?
const { getEnvironmentIdFromKey } = useProjectEnvironments(projectId)
const environmentDbId = getEnvironmentIdFromKey(environmentId)| className='flex-shrink-0' | ||
| width={18} | ||
| height={18} | ||
| fill='#656D7B' |
There was a problem hiding this comment.
Do you think colorIconSecondary would work here instead of the hex? It's the same value in light and follows the theme in dark. Same on line 73.
import { colorIconSecondary } from 'common/theme/tokens'
fill={colorIconSecondary}
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
Dashboard UI for experiment targeted audiences. The API side landed in #8472, so this now targets
main.The rollout step of the create-experiment wizard now takes an optional audience, so an experiment reads as
Segment > % rollout > % variant weights. The review step and the experiment detail page show it back; the detail page is display-only, since the audience is frozen once the experiment starts.AudiencePicker— pick a segment; the picker hides once one is chosen. Cohorts are filtered to the experiment's environment, and segments the API would reject (feature-specific, other-environment cohorts, percentage splits, no rules) are filtered out of the dropdown.AudienceSegmentList— variant-style card per segment: name, description, cohort source, membership count, remove.SegmentSelect— options now carry the description, cohort source and the environment's membership count, so you can size an audience before picking it.The API stores the audience as segment references and reads the rules off the rollout segment, so the segments shown here are provenance: a source segment can be deleted or edited without changing what a running experiment evaluates. A segment deleted after configuration renders muted with "(deleted)".
One segment for now, matching
MAX_AUDIENCE_SEGMENTSon the API side. The multi-segment UI is built and ships dormant — theANY/ALLtoggle renders as soon as a second segment is possible — but the cap holds it to one until the Java SDK is fixed:Warning
A multi-segment
anyaudience evaluates asallunder Java local evaluation. Raise the cap in both places only once the Java engine respects the rule type for sub-rules.How did you test this code?
Unit tests for the audience payload, summary and segment-eligibility helpers: 147 tests across 9 suites.
Manually against a local API: built an experiment targeting a segment at 50% rollout with a 50/50 split, checked the audience renders through wizard → review → detail, that the picker hides once a segment is chosen, and that identities outside the segment aren't enrolled. Also checked the empty case (no segment = all identities) and a deleted segment rendering as "(deleted)".