Skip to content
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

chore(experiments): clean up legacy experimentResults #27064

Merged
merged 5 commits into from
Dec 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions cypress/e2e/experiments.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ describe('Experiments', () => {
cy.get('[data-attr="experiment-creation-date"]').contains('a few seconds ago').should('be.visible')
cy.get('[data-attr="experiment-start-date"]').should('not.exist')

cy.wait(1000)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some re-rendering glitch causes the click to fail. Waiting for a bit solves this; no issues when testing manually.

cy.get('[data-attr="launch-experiment"]').first().click()
cy.get('[data-attr="experiment-creation-date"]').should('not.exist')
cy.get('[data-attr="experiment-start-date"]').contains('a few seconds ago').should('be.visible')
Expand All @@ -114,6 +115,7 @@ describe('Experiments', () => {
it('move start date', () => {
createExperimentInNewUi()

cy.wait(1000)
cy.get('[data-attr="launch-experiment"]').first().click()

cy.get('[data-attr="move-experiment-start-date"]').first().click()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ import { BaseMathType, ChartDisplayType, InsightType, PropertyFilterType, Proper
import { experimentLogic } from '../experimentLogic'

export function CumulativeExposuresChart(): JSX.Element {
const { experiment, experimentResults, getMetricType } = useValues(experimentLogic)
const { experiment, metricResults, getMetricType } = useValues(experimentLogic)

const metricIdx = 0
const metricType = getMetricType(metricIdx)

const result = metricResults?.[metricIdx]
const variants = experiment.parameters?.feature_flag_variants?.map((variant) => variant.key) || []
if (experiment.holdout) {
variants.push(`holdout-${experiment.holdout.id}`)
Expand All @@ -25,7 +25,7 @@ export function CumulativeExposuresChart(): JSX.Element {
if (metricType === InsightType.TRENDS) {
query = {
kind: NodeKind.InsightVizNode,
source: (experimentResults as CachedExperimentTrendsQueryResponse)?.exposure_query || {
source: (result as CachedExperimentTrendsQueryResponse)?.exposure_query || {
kind: NodeKind.TrendsQuery,
series: [],
interval: 'day',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export function DataCollection(): JSX.Element {

const experimentProgressPercent =
metricType === InsightType.FUNNELS
? (funnelResultsPersonsTotal / recommendedSampleSize) * 100
? (funnelResultsPersonsTotal(0) / recommendedSampleSize) * 100
: (actualRunningTime / recommendedRunningTime) * 100

const hasHighRunningTime = recommendedRunningTime > 62
Expand Down Expand Up @@ -109,7 +109,7 @@ export function DataCollection(): JSX.Element {
<span>
Saw&nbsp;
<b>
{humanFriendlyNumber(funnelResultsPersonsTotal)} of{' '}
{humanFriendlyNumber(funnelResultsPersonsTotal(0))} of{' '}
{humanFriendlyNumber(recommendedSampleSize)}{' '}
</b>{' '}
{formatUnitByQuantity(recommendedSampleSize, 'participant')}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,11 @@ export function DistributionModal({ experimentId }: { experimentId: Experiment['

export function DistributionTable(): JSX.Element {
const { openDistributionModal } = useActions(experimentLogic)
const { experimentId, experiment, experimentResults } = useValues(experimentLogic)
const { experimentId, experiment, metricResults } = useValues(experimentLogic)
const { reportExperimentReleaseConditionsViewed } = useActions(experimentLogic)

const result = metricResults?.[0]

const onSelectElement = (variant: string): void => {
LemonDialog.open({
title: 'Select a domain',
Expand All @@ -166,7 +168,7 @@ export function DistributionTable(): JSX.Element {
key: 'key',
title: 'Variant',
render: function Key(_, item): JSX.Element {
if (!experimentResults || !experimentResults.insight) {
if (!result || !result.insight) {
return <span className="font-semibold">{item.key}</span>
}
return <VariantTag experimentId={experimentId} variantKey={item.key} />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ import { Results } from './Results'
import { SecondaryMetricsTable } from './SecondaryMetricsTable'

const ResultsTab = (): JSX.Element => {
const { experiment, experimentResults, featureFlags } = useValues(experimentLogic)

const hasResultsInsight = experimentResults && experimentResults.insight
const { experiment, metricResults, featureFlags } = useValues(experimentLogic)
const result = metricResults?.[0]
const hasResultsInsight = result && result.insight

return (
<div className="space-y-8">
Expand Down Expand Up @@ -69,12 +69,12 @@ const VariantsTab = (): JSX.Element => {
}

export function ExperimentView(): JSX.Element {
const { experimentLoading, experimentResultsLoading, experimentId, experimentResults, tabKey, featureFlags } =
const { experimentLoading, metricResultsLoading, experimentId, metricResults, tabKey, featureFlags } =
useValues(experimentLogic)

const { setTabKey } = useActions(experimentLogic)

const hasResultsInsight = experimentResults && experimentResults.insight
const result = metricResults?.[0]
const hasResultsInsight = result && result.insight

return (
<>
Expand All @@ -85,7 +85,7 @@ export function ExperimentView(): JSX.Element {
) : (
<>
<Info />
{experimentResultsLoading ? (
{metricResultsLoading ? (
<ExperimentLoadingAnimation />
) : (
<>
Expand Down
26 changes: 20 additions & 6 deletions frontend/src/scenes/experiments/ExperimentView/Overview.tsx
Original file line number Diff line number Diff line change
@@ -1,17 +1,29 @@
import { useValues } from 'kea'

import { CachedExperimentFunnelsQueryResponse, CachedExperimentTrendsQueryResponse } from '~/queries/schema'

import { experimentLogic } from '../experimentLogic'
import { VariantTag } from './components'

export function Overview(): JSX.Element {
const { experimentId, experimentResults, getIndexForVariant, getHighestProbabilityVariant, areResultsSignificant } =
const { experimentId, metricResults, getIndexForVariant, getHighestProbabilityVariant, areResultsSignificant } =
useValues(experimentLogic)

const result = metricResults?.[0]
if (!result) {
return <></>
}

function WinningVariantText(): JSX.Element {
const highestProbabilityVariant = getHighestProbabilityVariant(experimentResults)
const index = getIndexForVariant(experimentResults, highestProbabilityVariant || '')
if (highestProbabilityVariant && index !== null && experimentResults) {
const { probability } = experimentResults
const highestProbabilityVariant = getHighestProbabilityVariant(
result as CachedExperimentFunnelsQueryResponse | CachedExperimentTrendsQueryResponse
)
const index = getIndexForVariant(
result as CachedExperimentFunnelsQueryResponse | CachedExperimentTrendsQueryResponse,
highestProbabilityVariant || ''
)
if (highestProbabilityVariant && index !== null && result) {
const { probability } = result

return (
<div className="items-center inline-flex flex-wrap">
Expand All @@ -32,7 +44,9 @@ export function Overview(): JSX.Element {
return (
<div className="flex-wrap">
<span>Your results are&nbsp;</span>
<span className="font-semibold">{`${areResultsSignificant ? 'significant' : 'not significant'}`}.</span>
<span className="font-semibold">
{`${areResultsSignificant(0) ? 'significant' : 'not significant'}`}.
</span>
</div>
)
}
Expand Down
8 changes: 6 additions & 2 deletions frontend/src/scenes/experiments/ExperimentView/Results.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,17 @@ import { ResultsHeader, ResultsQuery } from './components'
import { SummaryTable } from './SummaryTable'

export function Results(): JSX.Element {
const { experimentResults } = useValues(experimentLogic)
const { metricResults } = useValues(experimentLogic)
const result = metricResults?.[0]
if (!result) {
return <></>
}

return (
<div>
<ResultsHeader />
<SummaryTable />
<ResultsQuery targetResults={experimentResults} showTable={true} />
<ResultsQuery targetResults={result} showTable={true} />
</div>
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export function SecondaryMetricsTable({ experimentId }: { experimentId: Experime
const [modalMetricIdx, setModalMetricIdx] = useState<number | null>(null)

const {
experimentResults,
metricResults,
secondaryMetricResultsLoading,
experiment,
getSecondaryMetricType,
Expand Down Expand Up @@ -65,7 +65,7 @@ export function SecondaryMetricsTable({ experimentId }: { experimentId: Experime
{
title: <div className="py-2">Variant</div>,
render: function Key(_, item: TabularSecondaryMetricResults): JSX.Element {
if (!experimentResults || !experimentResults.insight) {
if (!metricResults?.[0] || !metricResults?.[0].insight) {
return <span className="font-semibold">{item.variant}</span>
}
return (
Expand Down
41 changes: 19 additions & 22 deletions frontend/src/scenes/experiments/ExperimentView/SummaryTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import { urls } from 'scenes/urls'

import {
FilterLogicalOperator,
FunnelExperimentVariant,
InsightType,
PropertyFilterType,
PropertyOperator,
Expand All @@ -27,7 +26,7 @@ export function SummaryTable(): JSX.Element {
const {
experimentId,
experiment,
experimentResults,
metricResults,
tabularExperimentResults,
getMetricType,
exposureCountDataForVariant,
Expand All @@ -38,14 +37,14 @@ export function SummaryTable(): JSX.Element {
credibleIntervalForVariant,
} = useValues(experimentLogic)
const metricType = getMetricType(0)

if (!experimentResults) {
const result = metricResults?.[0]
if (!result) {
return <></>
}

const winningVariant = getHighestProbabilityVariant(experimentResults)
const winningVariant = getHighestProbabilityVariant(result)

const columns: LemonTableColumns<TrendExperimentVariant | FunnelExperimentVariant> = [
const columns: LemonTableColumns<any> = [
{
key: 'variants',
title: 'Variant',
Expand All @@ -64,14 +63,14 @@ export function SummaryTable(): JSX.Element {
key: 'counts',
title: (
<div className="flex">
{experimentResults.insight?.[0] && 'action' in experimentResults.insight[0] && (
<EntityFilterInfo filter={experimentResults.insight[0].action} />
{result.insight?.[0] && 'action' in result.insight[0] && (
<EntityFilterInfo filter={result.insight[0].action} />
)}
<span className="pl-1">{experimentMathAggregationForTrends() ? 'metric' : 'count'}</span>
</div>
),
render: function Key(_, variant): JSX.Element {
const count = countDataForVariant(experimentResults, variant.key)
const count = countDataForVariant(result, variant.key)
if (!count) {
return <>—</>
}
Expand All @@ -83,7 +82,7 @@ export function SummaryTable(): JSX.Element {
key: 'exposure',
title: 'Exposure',
render: function Key(_, variant): JSX.Element {
const exposure = exposureCountDataForVariant(experimentResults, variant.key)
const exposure = exposureCountDataForVariant(result, variant.key)
if (!exposure) {
return <>—</>
}
Expand Down Expand Up @@ -120,7 +119,7 @@ export function SummaryTable(): JSX.Element {
return <em>Baseline</em>
}

const controlVariant = (experimentResults.variants as TrendExperimentVariant[]).find(
const controlVariant = (result.variants as TrendExperimentVariant[]).find(
({ key }) => key === 'control'
) as TrendExperimentVariant

Expand Down Expand Up @@ -161,7 +160,7 @@ export function SummaryTable(): JSX.Element {
return <em>Baseline</em>
}

const credibleInterval = credibleIntervalForVariant(experimentResults || null, variant.key, metricType)
const credibleInterval = credibleIntervalForVariant(result || null, variant.key, metricType)
if (!credibleInterval) {
return <>—</>
}
Expand All @@ -181,7 +180,7 @@ export function SummaryTable(): JSX.Element {
key: 'conversionRate',
title: 'Conversion rate',
render: function Key(_, item): JSX.Element {
const conversionRate = conversionRateForVariant(experimentResults, item.key)
const conversionRate = conversionRateForVariant(result, item.key)
if (!conversionRate) {
return <>—</>
}
Expand All @@ -204,8 +203,8 @@ export function SummaryTable(): JSX.Element {
return <em>Baseline</em>
}

const controlConversionRate = conversionRateForVariant(experimentResults, 'control')
const variantConversionRate = conversionRateForVariant(experimentResults, item.key)
const controlConversionRate = conversionRateForVariant(result, 'control')
const variantConversionRate = conversionRateForVariant(result, item.key)

if (!controlConversionRate || !variantConversionRate) {
return <>—</>
Expand Down Expand Up @@ -235,7 +234,7 @@ export function SummaryTable(): JSX.Element {
return <em>Baseline</em>
}

const credibleInterval = credibleIntervalForVariant(experimentResults || null, item.key, metricType)
const credibleInterval = credibleIntervalForVariant(result || null, item.key, metricType)
if (!credibleInterval) {
return <>—</>
}
Expand All @@ -254,15 +253,13 @@ export function SummaryTable(): JSX.Element {
key: 'winProbability',
title: 'Win probability',
sorter: (a, b) => {
const aPercentage = (experimentResults?.probability?.[a.key] || 0) * 100
const bPercentage = (experimentResults?.probability?.[b.key] || 0) * 100
const aPercentage = (result?.probability?.[a.key] || 0) * 100
const bPercentage = (result?.probability?.[b.key] || 0) * 100
return aPercentage - bPercentage
},
render: function Key(_, item): JSX.Element {
const variantKey = item.key
const percentage =
experimentResults?.probability?.[variantKey] != undefined &&
experimentResults.probability?.[variantKey] * 100
const percentage = result?.probability?.[variantKey] != undefined && result.probability?.[variantKey] * 100
const isWinning = variantKey === winningVariant

return (
Expand Down Expand Up @@ -351,7 +348,7 @@ export function SummaryTable(): JSX.Element {

return (
<div className="mb-4">
<LemonTable loading={false} columns={columns} dataSource={tabularExperimentResults} />
<LemonTable loading={false} columns={columns} dataSource={tabularExperimentResults(0)} />
</div>
)
}
Loading
Loading