From 638ef7744cc2e2d9970c969b2f0698233d21f605 Mon Sep 17 00:00:00 2001 From: Juraj Majerik Date: Fri, 13 Dec 2024 15:02:11 +0100 Subject: [PATCH] wip --- .../experiments/ExperimentView/DeltaViz.tsx | 228 +++++++++++++++--- .../Metrics/PrimaryGoalFunnels.tsx | 2 +- .../Metrics/PrimaryGoalTrendsExposure.tsx | 14 +- .../scenes/experiments/experimentLogic.tsx | 32 ++- 4 files changed, 227 insertions(+), 49 deletions(-) diff --git a/frontend/src/scenes/experiments/ExperimentView/DeltaViz.tsx b/frontend/src/scenes/experiments/ExperimentView/DeltaViz.tsx index 3bb6558280109..94be61a664753 100644 --- a/frontend/src/scenes/experiments/ExperimentView/DeltaViz.tsx +++ b/frontend/src/scenes/experiments/ExperimentView/DeltaViz.tsx @@ -6,7 +6,7 @@ import { InsightType } from '~/types' import { experimentLogic, getDefaultFilters, getDefaultFunnelsMetric } from '../experimentLogic' import { VariantTag } from './components' import { LemonButton, LemonTag } from '@posthog/lemon-ui' -import { IconPencil, IconPlus } from '@posthog/icons' +import { IconArchive, IconCheck, IconPencil, IconPlus, IconX, IconHourglass } from '@posthog/icons' import { FEATURE_FLAGS } from 'lib/constants' const MAX_PRIMARY_METRICS = 10 @@ -88,16 +88,31 @@ function formatTickValue(value: number): string { } export function DeltaViz(): JSX.Element { - const { experiment, experimentResults, getMetricType, metricResults, featureFlags } = useValues(experimentLogic) + const { experiment, getMetricType, metricResults, primaryMetricsResultErrors, credibleIntervalForVariant } = + useValues(experimentLogic) const { setExperiment, openPrimaryMetricModal } = useActions(experimentLogic) - if (!experimentResults) { - return <> - } - const variants = experiment.parameters.feature_flag_variants const metrics = experiment.metrics || [] + // Calculate the maximum absolute value across ALL metrics + const maxAbsValue = Math.max( + ...metrics.flatMap((_, metricIndex) => { + const result = metricResults?.[metricIndex] + return variants.flatMap((variant) => { + const interval = credibleIntervalForVariant(result, variant.key, getMetricType(metricIndex)) + return interval ? [Math.abs(interval[0] / 100), Math.abs(interval[1] / 100)] : [] + }) + }) + ) + + // Add padding to the range + const padding = Math.max(maxAbsValue * 0.05, 0.02) + const chartBound = maxAbsValue + padding + + // Calculate tick values once for all charts + const commonTickValues = getNiceTickValues(chartBound) + return (
@@ -136,8 +151,7 @@ export function DeltaViz(): JSX.Element {
{metrics.map((metric, metricIndex) => { - const results = metricResults?.[metricIndex] - + const result = metricResults?.[metricIndex] const isFirstMetric = metricIndex === 0 return ( @@ -154,12 +168,15 @@ export function DeltaViz(): JSX.Element { }`} >
) @@ -171,45 +188,38 @@ export function DeltaViz(): JSX.Element { } function Chart({ - results, + result, + error, variants, metricType, metricIndex, isFirstMetric, metric, + tickValues, + chartBound, }: { - results: any + result: any + error: any variants: any[] metricType: InsightType metricIndex: number isFirstMetric: boolean metric: any + tickValues: number[] + chartBound: number }): JSX.Element { const { credibleIntervalForVariant, conversionRateForVariant, experimentId } = useValues(experimentLogic) const { openPrimaryMetricModal } = useActions(experimentLogic) const [tooltipData, setTooltipData] = useState<{ x: number; y: number; variant: string } | null>(null) + const [emptyStateTooltipVisible, setEmptyStateTooltipVisible] = useState(true) + const [tooltipPosition, setTooltipPosition] = useState({ x: 0, y: 0 }) // Update chart height calculation to include only one BAR_PADDING for each space between bars const chartHeight = BAR_PADDING + (BAR_HEIGHT + BAR_PADDING) * variants.length - // Find the maximum absolute value from all credible intervals - const maxAbsValue = Math.max( - ...variants.flatMap((variant) => { - const interval = credibleIntervalForVariant(results, variant.key, metricType) - return interval ? [Math.abs(interval[0] / 100), Math.abs(interval[1] / 100)] : [] - }) - ) - - // Add padding to the range - const padding = Math.max(maxAbsValue * 0.05, 0.02) - const chartBound = maxAbsValue + padding - - const tickValues = getNiceTickValues(chartBound) - const maxTick = Math.max(...tickValues) - const valueToX = (value: number): number => { // Scale the value to fit within the padded area - const percentage = (value / maxTick + 1) / 2 + const percentage = (value / chartBound + 1) / 2 return HORIZONTAL_PADDING + percentage * (VIEW_BOX_WIDTH - 2 * HORIZONTAL_PADDING) } @@ -356,7 +366,7 @@ function Chart({ )} {isFirstMetric &&
} {/* Chart */} - {results ? ( + {result ? ( { - const interval = credibleIntervalForVariant(results, variant.key, metricType) + const interval = credibleIntervalForVariant(result, variant.key, metricType) const [lower, upper] = interval ? [interval[0] / 100, interval[1] / 100] : [0, 0] - const variantRate = conversionRateForVariant(results, variant.key) - const controlRate = conversionRateForVariant(results, 'control') + const variantRate = conversionRateForVariant(result, variant.key) + const controlRate = conversionRateForVariant(result, 'control') const delta = variantRate && controlRate ? (variantRate - controlRate) / controlRate : 0 // Find the highest delta among all variants const maxDelta = Math.max( ...variants.map((v) => { - const vRate = conversionRateForVariant(results, v.key) + const vRate = conversionRateForVariant(result, v.key) return vRate && controlRate ? (vRate - controlRate) / controlRate : 0 }) ) @@ -460,7 +470,30 @@ function Chart({ viewBox={`0 0 ${VIEW_BOX_WIDTH} ${chartHeight}`} preserveAspectRatio="xMidYMid meet" > - hello + { + const rect = e.currentTarget.getBoundingClientRect() + setTooltipPosition({ + x: rect.left + rect.width / 2, + y: rect.top, + }) + setEmptyStateTooltipVisible(true) + }} + onMouseLeave={() => setEmptyStateTooltipVisible(false)} + > +
+ Results not yet available + +
+ )} @@ -489,7 +522,7 @@ function Chart({
Conversion rate: - {conversionRateForVariant(results, tooltipData.variant)?.toFixed(2)}% + {conversionRateForVariant(result, tooltipData.variant)?.toFixed(2)}%
@@ -499,8 +532,8 @@ function Chart({ Baseline ) : ( (() => { - const variantRate = conversionRateForVariant(results, tooltipData.variant) - const controlRate = conversionRateForVariant(results, 'control') + const variantRate = conversionRateForVariant(result, tooltipData.variant) + const controlRate = conversionRateForVariant(result, 'control') const delta = variantRate && controlRate ? (variantRate - controlRate) / controlRate @@ -521,7 +554,7 @@ function Chart({ {(() => { const interval = credibleIntervalForVariant( - results, + result, tooltipData.variant, metricType ) @@ -537,6 +570,127 @@ function Chart({
)} + + {/* Empty state tooltip */} + {emptyStateTooltipVisible && ( +
+ +
+ )} +
+
+ ) +} + +export function NoResultsEmptyState({ error }: { error: any }): JSX.Element { + if (!error) { + return <> + } + + type ErrorCode = 'no-events' | 'no-flag-info' | 'no-control-variant' | 'no-test-variant' + + const { statusCode } = error + + function ChecklistItem({ errorCode, value }: { errorCode: ErrorCode; value: boolean }): JSX.Element { + const failureText = { + 'no-events': 'Metric events not received', + 'no-flag-info': 'Feature flag information not present on the events', + 'no-control-variant': 'Events with the control variant not received', + 'no-test-variant': 'Events with at least one test variant not received', + } + + const successText = { + 'no-events': 'Experiment events have been received', + 'no-flag-info': 'Feature flag information is present on the events', + 'no-control-variant': 'Events with the control variant received', + 'no-test-variant': 'Events with at least one test variant received', + } + + return ( +
+ {value === false ? ( + + + {successText[errorCode]} + + ) : ( + + + {failureText[errorCode]} + + )} +
+ ) + } + + // Validation errors return 400 and are rendered as a checklist + if (statusCode === 400) { + let parsedDetail: Record + try { + parsedDetail = JSON.parse(error.detail) + } catch (error) { + return ( +
+
+ Experiment results could not be calculated +
+
{error}
+
+ ) + } + + const checklistItems = [] + for (const [errorCode, value] of Object.entries(parsedDetail)) { + checklistItems.push() + } + + return
{checklistItems}
+ } + + if (statusCode === 504) { + return ( +
+
+
+ +

Experiment results timed out

+
+ This may occur when the experiment has a large amount of data or is particularly complex. We + are actively working on fixing this. In the meantime, please try refreshing the experiment + to retrieve the results. +
+
+
+
+ ) + } + + // Other unexpected errors + return ( +
+
+
+ +

Experiment results could not be calculated

+
{error.detail}
+
) diff --git a/frontend/src/scenes/experiments/Metrics/PrimaryGoalFunnels.tsx b/frontend/src/scenes/experiments/Metrics/PrimaryGoalFunnels.tsx index bf8eeb1e2a0c5..6b4ef31d3a2f1 100644 --- a/frontend/src/scenes/experiments/Metrics/PrimaryGoalFunnels.tsx +++ b/frontend/src/scenes/experiments/Metrics/PrimaryGoalFunnels.tsx @@ -266,7 +266,7 @@ export function PrimaryGoalFunnels(): JSX.Element { checked={(() => { // :FLAG: CLEAN UP AFTER MIGRATION if (featureFlags[FEATURE_FLAGS.EXPERIMENTS_HOGQL]) { - const val = (experiment.metrics[0] as ExperimentFunnelsQuery).funnels_query + const val = (experiment.metrics[metricIdx] as ExperimentFunnelsQuery).funnels_query ?.filterTestAccounts return hasFilters ? !!val : false } diff --git a/frontend/src/scenes/experiments/Metrics/PrimaryGoalTrendsExposure.tsx b/frontend/src/scenes/experiments/Metrics/PrimaryGoalTrendsExposure.tsx index 4ebe43c30e928..1cadda85adafd 100644 --- a/frontend/src/scenes/experiments/Metrics/PrimaryGoalTrendsExposure.tsx +++ b/frontend/src/scenes/experiments/Metrics/PrimaryGoalTrendsExposure.tsx @@ -16,11 +16,17 @@ import { experimentLogic } from '../experimentLogic' import { commonActionFilterProps } from './Selectors' export function PrimaryGoalTrendsExposure(): JSX.Element { - const { experiment, isExperimentRunning, featureFlags } = useValues(experimentLogic) + const { experiment, isExperimentRunning, featureFlags, editingPrimaryMetricIndex } = useValues(experimentLogic) const { setExperiment, setTrendsExposureMetric } = useActions(experimentLogic) const { currentTeam } = useValues(teamLogic) const hasFilters = (currentTeam?.test_account_filters || []).length > 0 - const currentMetric = experiment.metrics[0] as ExperimentTrendsQuery + + if (!editingPrimaryMetricIndex && editingPrimaryMetricIndex !== 0) { + console.warn('editingPrimaryMetricIndex is null or undefined') + return <> + } + + const currentMetric = experiment.metrics[editingPrimaryMetricIndex] as ExperimentTrendsQuery return ( <> @@ -43,7 +49,7 @@ export function PrimaryGoalTrendsExposure(): JSX.Element { ) setTrendsExposureMetric({ - metricIdx: 0, + metricIdx: editingPrimaryMetricIndex, series, }) } else { @@ -109,7 +115,7 @@ export function PrimaryGoalTrendsExposure(): JSX.Element { // :FLAG: CLEAN UP AFTER MIGRATION if (featureFlags[FEATURE_FLAGS.EXPERIMENTS_HOGQL]) { setTrendsExposureMetric({ - metricIdx: 0, + metricIdx: editingPrimaryMetricIndex, filterTestAccounts: checked, }) } else { diff --git a/frontend/src/scenes/experiments/experimentLogic.tsx b/frontend/src/scenes/experiments/experimentLogic.tsx index c71da03284dc8..c8d1266bdb44f 100644 --- a/frontend/src/scenes/experiments/experimentLogic.tsx +++ b/frontend/src/scenes/experiments/experimentLogic.tsx @@ -265,6 +265,7 @@ export const experimentLogic = kea([ setTabKey: (tabKey: string) => ({ tabKey }), openPrimaryMetricModal: (index: number) => ({ index }), closePrimaryMetricModal: true, + setPrimaryMetricsResultErrors: (errors: any[]) => ({ errors }), }), reducers({ experiment: [ @@ -425,6 +426,7 @@ export const experimentLogic = kea([ setExperimentResultCalculationError: (_, { error }) => error, }, ], + // here flagImplementationWarning: [ false as boolean, { @@ -487,6 +489,16 @@ export const experimentLogic = kea([ closePrimaryMetricModal: () => null, }, ], + primaryMetricsResultErrors: [ + [] as any[], + { + setPrimaryMetricsResultErrors: (_, { errors }) => errors, + // Reset errors when loading new results + loadMetricResults: () => [], + // Reset errors when loading a new experiment + loadExperiment: () => [], + }, + ], }), listeners(({ values, actions }) => ({ createExperiment: async ({ draft }) => { @@ -889,15 +901,14 @@ export const experimentLogic = kea([ }, ], metricResults: [ - null as (CachedExperimentTrendsQueryResponse | CachedExperimentFunnelsQueryResponse)[] | null, + null as (CachedExperimentTrendsQueryResponse | CachedExperimentFunnelsQueryResponse | null)[] | null, { loadMetricResults: async ( refresh?: boolean - ): Promise<(CachedExperimentTrendsQueryResponse | CachedExperimentFunnelsQueryResponse)[] | null> => { + ): Promise<(CachedExperimentTrendsQueryResponse | CachedExperimentFunnelsQueryResponse | null)[]> => { return (await Promise.all( - values.experiment?.metrics.map(async (metric) => { + values.experiment?.metrics.map(async (metric, index) => { try { - // Queries are shareable, so we need to set the experiment_id for the backend to correctly associate the query with the experiment const queryWithExperimentId = { ...metric, experiment_id: values.experimentId, @@ -908,11 +919,18 @@ export const experimentLogic = kea([ ...response, fakeInsightId: Math.random().toString(36).substring(2, 15), } - } catch (error) { - return {} + } catch (error: any) { + const errorDetailMatch = error.detail.match(/\{.*\}/) + const errorDetail = errorDetailMatch[0] + + // Store the error in primaryMetricsResultErrors + const currentErrors = [...(values.primaryMetricsResultErrors || [])] + currentErrors[index] = { detail: errorDetail, statusCode: error.status } + actions.setPrimaryMetricsResultErrors(currentErrors) + return null } }) - )) as (CachedExperimentTrendsQueryResponse | CachedExperimentFunnelsQueryResponse)[] + )) as (CachedExperimentTrendsQueryResponse | CachedExperimentFunnelsQueryResponse | null)[] }, }, ],