feat: improve experiment planning and results logic - #536
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds result-window DTOs and resolved bucketing, threads analytics filters into backend experiment result and chart queries (including segment profile filtering), adds startup validation and launch guardrails, expands frontend wiring and UX (results, completion, settings), and enforces analytics filter size limits. ChangesExperiment Results Filtering, Time Windows, and Launch Validation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
web/app/pages/Project/tabs/FeatureFlags/FeatureFlagsView.tsx (1)
274-286: ⚡ Quick winLocalize the new experiment-link badge and helper copy.
These new user-facing strings are hardcoded and should go through
t(...)to keep the feature flags view fully localized.Also applies to: 293-298
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/app/pages/Project/tabs/FeatureFlags/FeatureFlagsView.tsx` around lines 274 - 286, The new hardcoded user-facing strings for the experiment badge and its helper text need localization: replace the literal 'Experiment linked' label inside the Badge (rendered when flag.experimentId is truthy in the JSX branch that uses Link and Badge) with a call to t('...') and similarly wrap any other hardcoded strings in the nearby experiment-related block (the similar code around the other branch at lines 293-298) with t(...) so both the Badge label and any helper/copy use the translation function; import or use the existing i18n/t function in this file if not already present and pass appropriate translation keys for each string.web/app/pages/Project/tabs/Experiments/ExperimentSettingsModal.tsx (2)
889-892: ⚡ Quick winUse a stable unique key in variant mapping rows.
This list currently keys by
variant.key, but duplicate keys are a valid transient state while editing and will cause reconciliation glitches.Suggested fix
- {variants.map((variant) => ( + {variants.map((variant, index) => ( <div - key={variant.key} + key={`${variant.key}-${index}`} className='flex items-center justify-between gap-3 text-xs' >🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/app/pages/Project/tabs/Experiments/ExperimentSettingsModal.tsx` around lines 889 - 892, The list item keys use variant.key which can be non-unique while editing; change the key strategy in the variants.map rendering so each row uses a stable unique identifier (e.g., a persistent variant.id or a composite fallback that includes the map index) instead of relying solely on variant.key; update the JSX key expression where variants.map is used (the element with key={variant.key}) to reference the stable id or a deterministic composite key to prevent reconciliation glitches during editing.
273-278: 🏗️ Heavy liftLocalize newly added UI/validation copy.
New labels, guardrail messages, and helper text are hardcoded English. This will create mixed-language UI in non-English locales.
Example pattern
- newErrors.featureFlag = 'This feature flag is already linked elsewhere.' + newErrors.featureFlag = t('experiments.featureFlagAlreadyLinked') ... - <Text size='sm' weight='semibold'>Sample size estimate</Text> + <Text size='sm' weight='semibold'>{t('experiments.sampleSizeEstimate')}</Text>Also applies to: 495-575, 671-683, 1001-1112, 1117-1163, 1258-1279
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/app/pages/Project/tabs/Experiments/ExperimentSettingsModal.tsx` around lines 273 - 278, The validation/UI copy in ExperimentSettingsModal.tsx is hardcoded (e.g., the string assigned to newErrors.featureFlag when selectedFeatureFlag is set) and must be localized; replace literal strings like "This feature flag is already linked elsewhere." with i18n lookups using the project's translation helper (e.g., use the t(...) or intl.formatMessage(...) pattern used elsewhere) and add appropriate translation keys (suggest names like experiment.settings.featureFlag.alreadyLinked); apply the same change to the other hardcoded labels/messages in the indicated regions (lines ~495-575, 671-683, 1001-1112, 1117-1163, 1258-1279) so all UI/validation copy in ExperimentSettingsModal uses translation keys instead of plain English literals.web/app/pages/Project/tabs/Experiments/ExperimentsView.tsx (1)
112-153: ⚡ Quick winMove new guardrail/badge/metadata text to translation keys.
The added draft badges, guardrail phrases, and metadata labels are hardcoded and won’t localize with the rest of the page.
Also applies to: 252-267, 280-297
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/app/pages/Project/tabs/Experiments/ExperimentsView.tsx` around lines 112 - 153, The guardrail/warning strings in ExperimentsView.tsx (e.g., 'Missing goal', 'Needs at least two variants', 'Needs one control', 'Allocation must total 100%', 'Every variant needs traffic', 'Missing exposure event', 'Missing linked feature flag', 'Uneven allocation', 'Low traffic variant', 'No hypothesis' and the other hardcoded draft/metadata labels in the ranges mentioned) are hardcoded; replace each literal with a call to the translation function (e.g., t('experiments.guardrails.missingGoal')) and add corresponding keys to the i18n resource files, updating any components that render these messages to accept translated strings so the page localizes correctly. Ensure you use descriptive key names matching the strings above and apply the same change for the other occurrences mentioned (lines ~252-267 and ~280-297).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/app/pages/Project/tabs/Experiments/ExperimentResults.tsx`:
- Around line 747-750: The helper formatWindowDate currently formats dates in
the viewer's local timezone; change it to accept a timezone parameter (e.g.,
formatWindowDate(value?: string | null, timezone?: string)) and use dayjs.tz
(ensure dayjs timezone/utc plugins are initialized) to format the value in the
provided timezone before returning (e.g., dayjs.tz(value,
timezone).format(...)). Update all callers in ExperimentResults.tsx that render
the resolved window to pass the backend-resolved timezone variable (the same
`timezone` used to resolve the window) so the displayed "from"/"to" range
matches the analytics timezone. Ensure null/undefined handling remains intact.
- Around line 905-1056: The new warnings and notices embed raw English strings
and use titles for logic, which breaks localization and fragile branching;
update the HealthWarning objects created in healthWarnings (and the
resultWindowNotice) to include a stable code/key (e.g., code: 'NO_EXPOSURES' |
'EXTREME_IMBALANCE' | 'VARIANT_NO_TRAFFIC' | 'LOW_CONVERSION' | 'STALE_RUNNING'
| 'GOAL_MISSING') and replace all user-facing title/message literals with
t('...') calls when rendering; then change stopRecommendation to check those
stable codes (e.g., healthWarnings.some(w =>
['EXTREME_IMBALANCE','VARIANT_NO_TRAFFIC'].includes(w.code))) instead of
matching English titles, and ensure resultWindowNotice also uses t(...) for its
suffixes and messages.
- Around line 1266-1298: The CTA to open the completion modal is currently only
rendered inside the stopRecommendation conditional; make the complete action
available for all experiments by moving or duplicating the completion Button
(the one that calls setIsCompleteModalOpen(true)) out of the
stopRecommendation-only block so it renders regardless of stopRecommendation
being truthy; ensure the visual layout matches design (e.g., keep the Button
beside the banner when stopRecommendation exists and render the same Button in
the same container/row when it does not) and update any surrounding container
markup in ExperimentResults.tsx so no duplicate IDs or accessibility issues are
introduced.
---
Nitpick comments:
In `@web/app/pages/Project/tabs/Experiments/ExperimentSettingsModal.tsx`:
- Around line 889-892: The list item keys use variant.key which can be
non-unique while editing; change the key strategy in the variants.map rendering
so each row uses a stable unique identifier (e.g., a persistent variant.id or a
composite fallback that includes the map index) instead of relying solely on
variant.key; update the JSX key expression where variants.map is used (the
element with key={variant.key}) to reference the stable id or a deterministic
composite key to prevent reconciliation glitches during editing.
- Around line 273-278: The validation/UI copy in ExperimentSettingsModal.tsx is
hardcoded (e.g., the string assigned to newErrors.featureFlag when
selectedFeatureFlag is set) and must be localized; replace literal strings like
"This feature flag is already linked elsewhere." with i18n lookups using the
project's translation helper (e.g., use the t(...) or intl.formatMessage(...)
pattern used elsewhere) and add appropriate translation keys (suggest names like
experiment.settings.featureFlag.alreadyLinked); apply the same change to the
other hardcoded labels/messages in the indicated regions (lines ~495-575,
671-683, 1001-1112, 1117-1163, 1258-1279) so all UI/validation copy in
ExperimentSettingsModal uses translation keys instead of plain English literals.
In `@web/app/pages/Project/tabs/Experiments/ExperimentsView.tsx`:
- Around line 112-153: The guardrail/warning strings in ExperimentsView.tsx
(e.g., 'Missing goal', 'Needs at least two variants', 'Needs one control',
'Allocation must total 100%', 'Every variant needs traffic', 'Missing exposure
event', 'Missing linked feature flag', 'Uneven allocation', 'Low traffic
variant', 'No hypothesis' and the other hardcoded draft/metadata labels in the
ranges mentioned) are hardcoded; replace each literal with a call to the
translation function (e.g., t('experiments.guardrails.missingGoal')) and add
corresponding keys to the i18n resource files, updating any components that
render these messages to accept translated strings so the page localizes
correctly. Ensure you use descriptive key names matching the strings above and
apply the same change for the other occurrences mentioned (lines ~252-267 and
~280-297).
In `@web/app/pages/Project/tabs/FeatureFlags/FeatureFlagsView.tsx`:
- Around line 274-286: The new hardcoded user-facing strings for the experiment
badge and its helper text need localization: replace the literal 'Experiment
linked' label inside the Badge (rendered when flag.experimentId is truthy in the
JSX branch that uses Link and Badge) with a call to t('...') and similarly wrap
any other hardcoded strings in the nearby experiment-related block (the similar
code around the other branch at lines 293-298) with t(...) so both the Badge
label and any helper/copy use the translation function; import or use the
existing i18n/t function in this file if not already present and pass
appropriate translation keys for each string.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 021736b0-a404-457b-84ff-f3e886e6834a
📒 Files selected for processing (11)
backend/apps/cloud/src/experiment/dto/experiment.dto.tsbackend/apps/cloud/src/experiment/experiment.controller.tsbackend/apps/community/src/experiment/dto/experiment.dto.tsbackend/apps/community/src/experiment/experiment.controller.tsweb/app/api/api.server.tsweb/app/pages/Project/tabs/Experiments/ExperimentResults.tsxweb/app/pages/Project/tabs/Experiments/ExperimentSettingsModal.tsxweb/app/pages/Project/tabs/Experiments/ExperimentsView.tsxweb/app/pages/Project/tabs/FeatureFlags/FeatureFlagsView.tsxweb/app/routes/api.analytics.tsweb/app/routes/projects.$id.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
web/app/pages/Project/tabs/Experiments/ExperimentResults.tsx (1)
1350-1366: ⚡ Quick winHardcoded English strings in feature flag relationship section.
These strings should use
t()for consistency with the rest of the localized file.♻️ Suggested i18n conversion
<Text as='p' size='sm' weight='semibold'> - Feature flag relationship + {t('experiments.featureFlagRelationship.title')} </Text> <Text as='p' size='xs' colour='muted'> - The flag controls eligibility. Experiment exposures use actual - evaluations and variant events. + {t('experiments.featureFlagRelationship.description')} </Text> </div> <Text as='span' size='xs' colour='secondary' code> {experiment.featureFlagKey || experiment.featureFlagId || - 'created on launch'} + t('experiments.featureFlagRelationship.createdOnLaunch')} </Text>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/app/pages/Project/tabs/Experiments/ExperimentResults.tsx` around lines 1350 - 1366, Summary: Several hardcoded English strings in the Feature flag relationship JSX need to be localized with t(). Replace the literal texts inside the Text components — the heading "Feature flag relationship", the descriptive line "The flag controls eligibility. Experiment exposures use actual evaluations and variant events.", and the fallback string 'created on launch' used alongside experiment.featureFlagKey/featureFlagId — with calls to the i18n function t(...) and supply appropriate translation keys; ensure the component imports/uses the existing t hook or i18n function used elsewhere in ExperimentResults.tsx and keep the Text props unchanged (e.g., update the Text children for the heading, description, and the code span fallback).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/app/pages/Project/tabs/Experiments/ExperimentResults.tsx`:
- Around line 1345-1349: The fallback branch always renders
completeExperimentButton even when stopRecommendation(status) returns null,
causing a Complete button to appear for completed or draft experiments; update
the rendering condition to check the experiment status (use the same status
values checked in stopRecommendation such as 'running' or 'paused') and only
render completeExperimentButton when status is one of those allowed states
(e.g., status === 'running' || status === 'paused'), otherwise render nothing or
the appropriate UI for completed/draft.
---
Nitpick comments:
In `@web/app/pages/Project/tabs/Experiments/ExperimentResults.tsx`:
- Around line 1350-1366: Summary: Several hardcoded English strings in the
Feature flag relationship JSX need to be localized with t(). Replace the literal
texts inside the Text components — the heading "Feature flag relationship", the
descriptive line "The flag controls eligibility. Experiment exposures use actual
evaluations and variant events.", and the fallback string 'created on launch'
used alongside experiment.featureFlagKey/featureFlagId — with calls to the i18n
function t(...) and supply appropriate translation keys; ensure the component
imports/uses the existing t hook or i18n function used elsewhere in
ExperimentResults.tsx and keep the Text props unchanged (e.g., update the Text
children for the heading, description, and the code span fallback).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5b51e359-881d-47fd-9530-0204099f22e4
📒 Files selected for processing (9)
web/app/pages/Project/tabs/Experiments/ExperimentResults.tsxweb/app/pages/Project/tabs/Experiments/ExperimentSettingsModal.tsxweb/app/pages/Project/tabs/Experiments/ExperimentsView.tsxweb/app/pages/Project/tabs/FeatureFlags/FeatureFlagsView.tsxweb/public/locales/de.jsonweb/public/locales/en.jsonweb/public/locales/fr.jsonweb/public/locales/pl.jsonweb/public/locales/uk.json
✅ Files skipped from review due to trivial changes (2)
- web/public/locales/de.json
- web/public/locales/en.json
Blaumaus
force-pushed
the
feat/experiments
branch
from
May 20, 2026 03:39
10a0b92 to
91f31d4
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web/public/locales/fr.json (1)
1576-1579: 💤 Low valueVerify terminology consistency for "feature flag".
The JSON structure and French translation are correct. However, I noticed a minor potential inconsistency: line 1577 uses "feature flag" (keeping the English term), while other parts of this file use just "flag" in French contexts (e.g., line 1347: "Filtrer les flags", line 1349: "Créer un flag").
This might be intentional to provide clarity in this specific context, but it's worth verifying whether it should be:
- "Relation du flag" (consistent with simplified usage elsewhere)
- "Relation du feature flag" (current, more explicit)
If "feature flag" is the standard technical term used throughout French technical documentation for this product, then the current form is fine.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/public/locales/fr.json` around lines 1576 - 1579, The translation for the "featureFlagRelationship" key uses the English term "feature flag" which is inconsistent with other French entries that use "flag" (e.g., keys that render "Filtrer les flags" and "Créer un flag"); update the "title" value for featureFlagRelationship to "Relation du flag" and review other featureFlag* keys to ensure all UI labels consistently use "flag" (or alternatively confirm product-wide policy if you prefer to keep the English term) so terminology is consistent across the locale file.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@web/public/locales/fr.json`:
- Around line 1576-1579: The translation for the "featureFlagRelationship" key
uses the English term "feature flag" which is inconsistent with other French
entries that use "flag" (e.g., keys that render "Filtrer les flags" and "Créer
un flag"); update the "title" value for featureFlagRelationship to "Relation du
flag" and review other featureFlag* keys to ensure all UI labels consistently
use "flag" (or alternatively confirm product-wide policy if you prefer to keep
the English term) so terminology is consistent across the locale file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 44c54457-3f5a-4245-9a49-dac4c4f4fd15
📒 Files selected for processing (6)
web/app/pages/Project/tabs/Experiments/ExperimentResults.tsxweb/public/locales/de.jsonweb/public/locales/en.jsonweb/public/locales/fr.jsonweb/public/locales/pl.jsonweb/public/locales/uk.json
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/app/pages/Project/tabs/Experiments/ExperimentResults.tsx (1)
1017-1023:⚠️ Potential issue | 🟠 Major | ⚡ Quick winBlock stop recommendations when the goal warning is active.
stopRecommendationstill runs whenGOAL_MISSINGis present, so the screen can recommend completing an experiment while the warning above says the conversion data is unreliable. That creates conflicting guidance on a decision-making path.Suggested fix
if ( healthWarnings.some((warning) => - ['EXTREME_IMBALANCE', 'VARIANT_NO_TRAFFIC'].includes(warning.code), + [ + 'EXTREME_IMBALANCE', + 'VARIANT_NO_TRAFFIC', + 'GOAL_MISSING', + ].includes(warning.code), ) ) { return null }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/app/pages/Project/tabs/Experiments/ExperimentResults.tsx` around lines 1017 - 1023, The code hides stop recommendations for EXTREME_IMBALANCE and VARIANT_NO_TRAFFIC but still shows stopRecommendation when a GOAL_MISSING health warning exists; update the logic that decides whether to render stopRecommendation in ExperimentResults (the place that checks healthWarnings and calls stopRecommendation) to also treat 'GOAL_MISSING' as a blocking warning—i.e., include 'GOAL_MISSING' in the array checked by healthWarnings.some(...) or add an explicit guard that returns null / skips rendering stopRecommendation when any healthWarnings contains code === 'GOAL_MISSING', so the recommendation UI is suppressed whenever the goal warning is active.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@web/app/pages/Project/tabs/Experiments/ExperimentResults.tsx`:
- Around line 1017-1023: The code hides stop recommendations for
EXTREME_IMBALANCE and VARIANT_NO_TRAFFIC but still shows stopRecommendation when
a GOAL_MISSING health warning exists; update the logic that decides whether to
render stopRecommendation in ExperimentResults (the place that checks
healthWarnings and calls stopRecommendation) to also treat 'GOAL_MISSING' as a
blocking warning—i.e., include 'GOAL_MISSING' in the array checked by
healthWarnings.some(...) or add an explicit guard that returns null / skips
rendering stopRecommendation when any healthWarnings contains code ===
'GOAL_MISSING', so the recommendation UI is suppressed whenever the goal warning
is active.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 46bf3698-a0a3-48a8-851e-5f634fee973b
📒 Files selected for processing (9)
backend/apps/cloud/src/analytics/analytics.service.tsbackend/apps/community/src/analytics/analytics.service.tsweb/app/pages/Project/tabs/Experiments/ExperimentResults.tsxweb/app/routes/projects.$id.tsxweb/public/locales/de.jsonweb/public/locales/en.jsonweb/public/locales/fr.jsonweb/public/locales/pl.jsonweb/public/locales/uk.json
✅ Files skipped from review due to trivial changes (2)
- web/public/locales/en.json
- web/public/locales/de.json
Member
Author
|
@coderabbitai resume |
✅ Actions performedReviews resumed. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/app/pages/Project/tabs/Experiments/ExperimentSettingsModal.tsx`:
- Line 95: The showPlanning state can persist across modal opens because
resetForm isn't always called (edit path uses loadExperiment), so ensure
showPlanning is reset whenever the modal opens: add setShowPlanning(false) to
the modal-open flow (either call it inside resetForm and also call it at the
start of loadExperiment, or add a useEffect that watches the modal open prop and
calls setShowPlanning(false)). Reference showPlanning, setShowPlanning,
resetForm and loadExperiment to locate where to add the call so planner inputs
and readiness summary are cleared on every open.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 732a1257-7d80-4431-a5c9-e945da51624c
📒 Files selected for processing (4)
web/app/pages/Project/tabs/Experiments/ExperimentResults.tsxweb/app/pages/Project/tabs/Experiments/ExperimentSettingsModal.tsxweb/app/pages/Project/tabs/Experiments/ExperimentsView.tsxweb/public/locales/en.json
✅ Files skipped from review due to trivial changes (1)
- web/public/locales/en.json
There was a problem hiding this comment.
♻️ Duplicate comments (1)
web/app/pages/Project/tabs/Experiments/ExperimentSettingsModal.tsx (1)
138-140:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReset the full planning state here, not just the accordion.
This clears the open/closed state, but the edit path still carries over
baselineConversionRate,minimumDetectableEffect, anddailyExposuresfrom the previous modal session. That keeps the collapsed sample estimate and guardrails based on stale planner inputs until the user changes them manually.🔁 Minimal fix
const loadExperiment = async () => { if (!experimentId) return setShowPlanning(false) + setBaselineConversionRate(5) + setMinimumDetectableEffect(10) + setDailyExposures(500) setIsLoading(true)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/app/pages/Project/tabs/Experiments/ExperimentSettingsModal.tsx` around lines 138 - 140, The loadExperiment function currently only collapses the planner accordion via setShowPlanning(false) but leaves planner inputs (baselineConversionRate, minimumDetectableEffect, dailyExposures — or the planningState object) populated from the prior session; update loadExperiment to fully reset the planner state when opening a new experiment by clearing those variables (or calling an existing resetPlanningState helper) to their initial/default values (e.g., undefined or defaults) so the collapsed sample estimate and guardrails do not reflect stale inputs; locate loadExperiment and update it to call setBaselineConversionRate(...), setMinimumDetectableEffect(...), setDailyExposures(...) or resetPlanningState() immediately after setShowPlanning(false).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@web/app/pages/Project/tabs/Experiments/ExperimentSettingsModal.tsx`:
- Around line 138-140: The loadExperiment function currently only collapses the
planner accordion via setShowPlanning(false) but leaves planner inputs
(baselineConversionRate, minimumDetectableEffect, dailyExposures — or the
planningState object) populated from the prior session; update loadExperiment to
fully reset the planner state when opening a new experiment by clearing those
variables (or calling an existing resetPlanningState helper) to their
initial/default values (e.g., undefined or defaults) so the collapsed sample
estimate and guardrails do not reflect stale inputs; locate loadExperiment and
update it to call setBaselineConversionRate(...),
setMinimumDetectableEffect(...), setDailyExposures(...) or resetPlanningState()
immediately after setShowPlanning(false).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7355eb94-8845-43de-9f30-09852337f6d9
📒 Files selected for processing (11)
docs/content/docs/analytics-dashboard/experiments.mdxdocs/content/docs/analytics-dashboard/feature-flags.mdxdocs/content/docs/script-reference.mdxpackages/tracker-js/README.mdpackages/tracker-js/src/Lib.tspackages/tracker-js/src/index.tsweb/app/pages/Project/tabs/Experiments/ExperimentResults.tsxweb/app/pages/Project/tabs/Experiments/ExperimentSettingsModal.tsxweb/app/pages/Project/tabs/Experiments/ExperimentsView.tsxweb/app/ui/Tooltip.tsxweb/public/locales/en.json
✅ Files skipped from review due to trivial changes (3)
- packages/tracker-js/src/Lib.ts
- packages/tracker-js/src/index.ts
- docs/content/docs/analytics-dashboard/feature-flags.mdx
This was referenced May 25, 2026
6 tasks
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
1 participant
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.
Changes
If applicable, please describe what changes were made in this pull request.
Community Edition support
Database migrations
Documentation
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Documentation