From 8b774ae7ba715e72dbd33b3a87d9c15a8764e422 Mon Sep 17 00:00:00 2001 From: Christian Vidal Wolf Date: Wed, 25 Feb 2026 16:34:55 +0100 Subject: [PATCH] fix(experiments): prevent NaN propagation and ACOS zero-value bug in experiment analysis - Change ?? to || for salesAds/units/revenue in weekly aggregation to guard against NaN - Add || 0 fallback to ads-only records in mergeSalesAndAdsData - Fix parseCurrency to check isNaN on all EU-format return paths - Handle ACOS edge case: cost > 0 with adRevenue = 0 now returns 100% instead of 0% - Add diagnostic console logging to computeDiD for data flow tracing Co-Authored-By: Claude Opus 4.6 --- components/ExperimentsView.tsx | 4 ++-- services/dataProcessor.ts | 18 ++++++++++-------- services/experimentAnalysis.ts | 29 ++++++++++++++++++++++++----- 3 files changed, 36 insertions(+), 15 deletions(-) diff --git a/components/ExperimentsView.tsx b/components/ExperimentsView.tsx index 83f269d..409338f 100644 --- a/components/ExperimentsView.tsx +++ b/components/ExperimentsView.tsx @@ -812,8 +812,8 @@ const ExperimentDetailView: React.FC<{ {METRIC_LABELS[metric] || metric} {formatMetricValue(r.treatment_before, metric)} {formatMetricValue(r.treatment_after, metric)} - {r.control_before ? formatMetricValue(r.control_before, metric) : '—'} - {r.control_after ? formatMetricValue(r.control_after, metric) : '—'} + {typeof r.control_before === 'number' ? formatMetricValue(r.control_before, metric) : '—'} + {typeof r.control_after === 'number' ? formatMetricValue(r.control_after, metric) : '—'} = 0 ? 'text-emerald-400' : 'text-red-400'}`}> {r.did_estimate >= 0 ? '+' : ''}{formatMetricValue(r.did_estimate, metric)} diff --git a/services/dataProcessor.ts b/services/dataProcessor.ts index a42bbde..f315af1 100644 --- a/services/dataProcessor.ts +++ b/services/dataProcessor.ts @@ -69,7 +69,8 @@ const parseCurrency = (value: string): number => { // Likely EU decimal without thousands or with thousands implicitly handled // e.g. "263,83" -> "263.83" clean = clean.replace(',', '.'); - return parseFloat(clean); + const num = parseFloat(clean); + return isNaN(num) ? 0 : num; } else if (clean.includes(',') && clean.includes('.')) { // Mixed: 1.234,56 -> EU @@ -79,7 +80,8 @@ const parseCurrency = (value: string): number => { // 1,234.56 -> US clean = clean.replace(/,/g, ''); } - return parseFloat(clean); + const num = parseFloat(clean); + return isNaN(num) ? 0 : num; } // Case B: Standard/US Format or Clean Number (e.g. "277179.09" or "1000") @@ -832,12 +834,12 @@ export const mergeSalesAndAdsData = ( sku: meta?.sku || '', salesTotal: 0, unitsTotal: 0, - salesAds: ad.attributedSales30d, - unitsAds: ad.attributedUnits30d, - cost: ad.cost, - clicks: ad.clicks, - impressions: ad.impressions, - conversions: ad.conversions, + salesAds: ad.attributedSales30d || 0, + unitsAds: ad.attributedUnits30d || 0, + cost: ad.cost || 0, + clicks: ad.clicks || 0, + impressions: ad.impressions || 0, + conversions: ad.conversions || 0, salesOrganic: 0, unitsOrganic: 0, paidSalesShare: 0, diff --git a/services/experimentAnalysis.ts b/services/experimentAnalysis.ts index 1f5d278..4c1d673 100644 --- a/services/experimentAnalysis.ts +++ b/services/experimentAnalysis.ts @@ -83,9 +83,9 @@ function aggregateWeeklyMetrics( } const w = weeklyMap.get(key)!; - w.units += r.unitsTotal ?? (r as any).units ?? 0; - w.revenue += r.salesTotal ?? (r as any).sellOut ?? 0; - w.adRevenue += r.salesAds ?? 0; + w.units += r.unitsTotal || (r as any).units || 0; + w.revenue += r.salesTotal || (r as any).sellOut || 0; + w.adRevenue += r.salesAds || 0; w.sessions += r.glanceViews || 0; w.cost += r.cost || 0; w.clicks += r.clicks || 0; @@ -110,7 +110,7 @@ function getMetricValue(w: ComputedWeeklyMetrics, metric: string): number { case 'ctr': return w.ctr; case 'roas': return w.roas; case 'revenue': return w.revenue; - case 'acos': return w.cost > 0 && w.adRevenue > 0 ? (w.cost / w.adRevenue) * 100 : 0; + case 'acos': return w.cost > 0 ? (w.adRevenue > 0 ? (w.cost / w.adRevenue) * 100 : 100) : 0; default: return w.units; } } @@ -173,7 +173,8 @@ function avgMetric(data: ComputedWeeklyMetrics[], metric: string, durationWeeks: if (metric === 'acos') { const totalAdRevenue = data.reduce((s, w) => s + w.adRevenue, 0); const totalCost = data.reduce((s, w) => s + w.cost, 0); - return totalAdRevenue > 0 ? (totalCost / totalAdRevenue) * 100 : 0; + if (totalCost === 0) return 0; // No ad spend → ACOS is 0 + return totalAdRevenue > 0 ? (totalCost / totalAdRevenue) * 100 : 100; // Cost but no ad revenue → 100% ACOS } // For absolute quantities (units, revenue, sessions), we sum them and divide by the duration @@ -293,6 +294,24 @@ export function computeDiD( const treatmentDurationWeeks = Math.max(1, Math.round((endTs - startTs) / (7 * 86400000))); const baselineDurationWeeks = Math.max(1, Math.round((beforeEndTs - beforeStartTs) / (7 * 86400000))); + // Debug: log data flow for ACOS diagnosis + const afterTotalCost = tSplit.after.reduce((s, w) => s + w.cost, 0); + const afterTotalAdRev = tSplit.after.reduce((s, w) => s + w.adRevenue, 0); + const beforeTotalCost = tSplit.before.reduce((s, w) => s + w.cost, 0); + const beforeTotalAdRev = tSplit.before.reduce((s, w) => s + w.adRevenue, 0); + console.log(`[computeDiD DEBUG] Experiment: ${experiment.name} | Mkt: ${experiment.marketplace}`); + console.log(` Treatment ASINs: ${treatmentAsinSet.size} | Weekly buckets: ${treatmentWeekly.length}`); + console.log(` Period: ${new Date(startTs).toISOString().split('T')[0]} → ${new Date(endTs).toISOString().split('T')[0]}`); + console.log(` Baseline: ${new Date(beforeStartTs).toISOString().split('T')[0]} → ${new Date(beforeEndTs).toISOString().split('T')[0]}`); + console.log(` After split: ${tSplit.after.length} weeks [cost=${afterTotalCost.toFixed(2)}, adRev=${afterTotalAdRev.toFixed(2)}]`); + console.log(` Before split: ${tSplit.before.length} weeks [cost=${beforeTotalCost.toFixed(2)}, adRev=${beforeTotalAdRev.toFixed(2)}]`); + if (tSplit.after.length > 0) { + console.log(` After weeks: ${tSplit.after.map(w => w.week).join(', ')}`); + } + if (tSplit.before.length > 0) { + console.log(` Before weeks: ${tSplit.before.map(w => w.week).join(', ')}`); + } + const metrics: Record = {}; for (const metric of METRICS) { metrics[metric] = computeMetricDiD(