From 8c18b2abcb000ed12577efa5586b3cd308494b94 Mon Sep 17 00:00:00 2001 From: Jurriaan Roelofs Date: Wed, 28 Jan 2026 13:57:40 +0100 Subject: [PATCH] fix: resolve lint and PHPStan errors - Remove unused $center variable in RlAnalyzer.php - Remove dead try-catch blocks in RlCommands.php (InvalidArgumentException never thrown) - Fix null coalesce by extracting rate variables with empty array check Fixes #28 --- src/Drush/Commands/RlCommands.php | 86 ++++++++++--------------------- src/Service/RlAnalyzer.php | 10 ++-- 2 files changed, 34 insertions(+), 62 deletions(-) diff --git a/src/Drush/Commands/RlCommands.php b/src/Drush/Commands/RlCommands.php index 156e879..9b3d0b5 100644 --- a/src/Drush/Commands/RlCommands.php +++ b/src/Drush/Commands/RlCommands.php @@ -89,13 +89,7 @@ public function listExperiments(): RowsOfFields { #[CLI\Usage(name: 'drush rl:status ab_test_button_color', description: 'Get status of button color test')] #[CLI\Usage(name: 'drush rl:status ai_sorting-help_center_categories-block_1 --format=json', description: 'Get detailed status as JSON')] public function status(string $experimentId, array $options = ['format' => 'yaml']): array { - try { - return $this->analyzer->getStatus($experimentId); - } - catch (\InvalidArgumentException $e) { - $this->logger()->error($e->getMessage()); - throw $e; - } + return $this->analyzer->getStatus($experimentId); } /** @@ -136,18 +130,12 @@ public function status(string $experimentId, array $options = ['format' => 'yaml #[CLI\Usage(name: 'drush rl:perf ai_sorting-help_center_categories-block_1 --limit=10 --format=json', description: 'Get top 10 performers as JSON')] #[CLI\Usage(name: 'drush rl:perf ab_test_headline_variants --sort=impressions', description: 'Sort by traffic volume')] public function performance(string $experimentId, array $options = ['limit' => 20, 'sort' => 'rate']): RowsOfFields { - try { - $data = $this->analyzer->getPerformance( - $experimentId, - (int) $options['limit'], - $options['sort'] - ); - return new RowsOfFields($data['arms']); - } - catch (\InvalidArgumentException $e) { - $this->logger()->error($e->getMessage()); - throw $e; - } + $data = $this->analyzer->getPerformance( + $experimentId, + (int) $options['limit'], + $options['sort'] + ); + return new RowsOfFields($data['arms']); } /** @@ -178,18 +166,12 @@ public function performance(string $experimentId, array $options = ['limit' => 2 #[CLI\Usage(name: 'drush rl:trends ab_test_headline_variants', description: 'Get weekly trends')] #[CLI\Usage(name: 'drush rl:trends mock_10_arm_test --period=daily --periods=14 --format=json', description: 'Get 14 days of daily data')] public function trends(string $experimentId, array $options = ['period' => 'weekly', 'periods' => 8]): RowsOfFields { - try { - $data = $this->analyzer->getTrends( - $experimentId, - $options['period'], - (int) $options['periods'] - ); - return new RowsOfFields($data['data']); - } - catch (\InvalidArgumentException $e) { - $this->logger()->error($e->getMessage()); - throw $e; - } + $data = $this->analyzer->getTrends( + $experimentId, + $options['period'], + (int) $options['periods'] + ); + return new RowsOfFields($data['data']); } /** @@ -213,16 +195,10 @@ public function trends(string $experimentId, array $options = ['period' => 'week #[CLI\Usage(name: 'drush rl:export ab_test_button_color', description: 'Export experiment data as JSON')] #[CLI\Usage(name: 'drush rl:export mock_10_arm_test --snapshots > export.json', description: 'Export with snapshots to file')] public function export(string $experimentId, array $options = ['snapshots' => FALSE, 'format' => 'json']): array { - try { - return $this->analyzer->export( - $experimentId, - (bool) $options['snapshots'] - ); - } - catch (\InvalidArgumentException $e) { - $this->logger()->error($e->getMessage()); - throw $e; - } + return $this->analyzer->export( + $experimentId, + (bool) $options['snapshots'] + ); } /** @@ -245,24 +221,18 @@ public function export(string $experimentId, array $options = ['snapshots' => FA #[CLI\Usage(name: 'drush rl:analyze ab_test_button_color', description: 'Get full analysis with recommendations')] #[CLI\Usage(name: 'drush rl:analyze mock_10_arm_test --format=json', description: 'Get analysis as JSON')] public function analyze(string $experimentId, array $options = ['format' => 'yaml']): array { - try { - $status = $this->analyzer->getStatus($experimentId); - $performance = $this->analyzer->getPerformance($experimentId, 10); + $status = $this->analyzer->getStatus($experimentId); + $performance = $this->analyzer->getPerformance($experimentId, 10); - return [ - 'experiment' => $status['experiment'], - 'status' => $status['status'], - 'summary' => $status['summary'], - 'value_generated' => $status['value_generated'], - 'top_performers' => array_slice($performance['arms'], 0, 5), - 'insights' => $performance['insights'], - 'recommendation' => $this->generateRecommendation($status, $performance), - ]; - } - catch (\InvalidArgumentException $e) { - $this->logger()->error($e->getMessage()); - throw $e; - } + return [ + 'experiment' => $status['experiment'], + 'status' => $status['status'], + 'summary' => $status['summary'], + 'value_generated' => $status['value_generated'], + 'top_performers' => array_slice($performance['arms'], 0, 5), + 'insights' => $performance['insights'], + 'recommendation' => $this->generateRecommendation($status, $performance), + ]; } /** diff --git a/src/Service/RlAnalyzer.php b/src/Service/RlAnalyzer.php index 1974322..6c2798b 100644 --- a/src/Service/RlAnalyzer.php +++ b/src/Service/RlAnalyzer.php @@ -299,6 +299,9 @@ public function getTrends(string $experimentId, string $period = 'weekly', int $ $trendDirection = 'declining'; } + $firstRate = $data[0]['rate']; + $lastRate = end($data)['rate']; + return [ 'experiment_id' => $experimentId, 'period' => $period, @@ -306,10 +309,10 @@ public function getTrends(string $experimentId, string $period = 'weekly', int $ 'data' => $data, 'analysis' => [ 'trend_direction' => $trendDirection, - 'first_period_rate' => $data[0]['rate'] ?? 0, - 'last_period_rate' => end($data)['rate'] ?? 0, + 'first_period_rate' => $firstRate, + 'last_period_rate' => $lastRate, 'overall_change_pct' => count($data) >= 2 - ? round((end($data)['rate'] - $data[0]['rate']) * 100 / max(0.01, $data[0]['rate']), 1) + ? round(($lastRate - $firstRate) * 100 / max(0.01, $firstRate), 1) : 0, ], ]; @@ -671,7 +674,6 @@ protected function estimateConfidence(int $conversions, int $impressions): float $p = $conversions / $impressions; $z = 1.96; $denominator = 1 + $z * $z / $impressions; - $center = ($p + $z * $z / (2 * $impressions)) / $denominator; $spread = $z * sqrt(($p * (1 - $p) + $z * $z / (4 * $impressions)) / $impressions) / $denominator; // Narrower interval = higher confidence.