From f5fd984c00e497b0e603675e8bd94bf64baac475 Mon Sep 17 00:00:00 2001 From: jjroelofs <904576+jjroelofs@users.noreply.github.com> Date: Tue, 30 Jun 2026 13:06:50 +0200 Subject: [PATCH 1/4] fix: snapshot recording fails for multi-arm experiments When recordTurns() is called with many arm IDs (e.g. 208 for ai_sorting), total_turns jumps by the arm count per request. The shouldRecordSnapshot() modulo check ($total_turns % $interval === 0) assumes increments of 1 and never aligns with these large jumps, producing zero snapshots regardless of traffic volume. Replace exact modulo with range-crossing detection: check whether the step [previous_turns, total_turns] crosses a sampling boundary via floor(total_turns / interval) != floor(previous_turns / interval). Pass the actual step_size through the recording chain so the snapshot sampler knows the jump width. Fixes #53 --- src/Storage/ExperimentDataStorage.php | 14 +++++---- src/Storage/SnapshotStorage.php | 36 +++++++++++++++--------- src/Storage/SnapshotStorageInterface.php | 7 ++++- 3 files changed, 38 insertions(+), 19 deletions(-) diff --git a/src/Storage/ExperimentDataStorage.php b/src/Storage/ExperimentDataStorage.php index 2e9708f..c755e6a 100644 --- a/src/Storage/ExperimentDataStorage.php +++ b/src/Storage/ExperimentDataStorage.php @@ -77,7 +77,7 @@ public function recordTurn($experiment_id, $arm_id) { ->execute(); // Record snapshot if enabled. - $this->maybeRecordSnapshots($experiment_id, [$arm_id]); + $this->maybeRecordSnapshots($experiment_id, [$arm_id], 1); } /** @@ -113,8 +113,9 @@ public function recordTurns($experiment_id, array $arm_ids) { ->expression('updated', ':timestamp', [':timestamp' => $timestamp]) ->execute(); - // Record snapshots if enabled. - $this->maybeRecordSnapshots($experiment_id, $arm_ids); + // Record snapshots if enabled. Step size = arm count since + // total_turns was incremented by that amount. + $this->maybeRecordSnapshots($experiment_id, $arm_ids, $arm_count); } /** @@ -234,8 +235,10 @@ public function getAllArmsDataMultiple(array $experiment_ids): array { * The experiment ID. * @param array $arm_ids * Array of arm IDs to snapshot. + * @param int $step_size + * How much total_turns increased on this request. */ - protected function maybeRecordSnapshots(string $experiment_id, array $arm_ids): void { + protected function maybeRecordSnapshots(string $experiment_id, array $arm_ids, int $step_size = 1): void { if (!$this->snapshotStorage || !$this->snapshotStorage->isEnabled()) { return; } @@ -250,7 +253,8 @@ protected function maybeRecordSnapshots(string $experiment_id, array $arm_ids): $arm_id, (int) $arm_data->turns, (int) $arm_data->rewards, - $total_turns + $total_turns, + $step_size ); } } diff --git a/src/Storage/SnapshotStorage.php b/src/Storage/SnapshotStorage.php index 5449a39..bead8d3 100644 --- a/src/Storage/SnapshotStorage.php +++ b/src/Storage/SnapshotStorage.php @@ -68,7 +68,7 @@ public function isEnabled(): bool { /** * {@inheritdoc} */ - public function recordSnapshot(string $experiment_id, string $arm_id, int $turns, int $rewards, int $total_experiment_turns): void { + public function recordSnapshot(string $experiment_id, string $arm_id, int $turns, int $rewards, int $total_experiment_turns, int $step_size = 1): void { if (!$this->isEnabled()) { return; } @@ -77,11 +77,11 @@ public function recordSnapshot(string $experiment_id, string $arm_id, int $turns $arm_count = $this->getArmCount($experiment_id); $snapshots_per_arm = $this->calculateSnapshotsPerArm($arm_count); - if (!$this->shouldRecordSnapshot($experiment_id, $arm_id, $total_experiment_turns, $snapshots_per_arm)) { + if (!$this->shouldRecordSnapshot($experiment_id, $arm_id, $total_experiment_turns, $snapshots_per_arm, $step_size)) { return; } - $is_milestone = $this->isMilestone($total_experiment_turns, $snapshots_per_arm); + $is_milestone = $this->isMilestone($total_experiment_turns, $snapshots_per_arm, $step_size); $this->database->insert('rl_arm_snapshots') ->fields([ @@ -283,6 +283,11 @@ protected function calculateMiddleInterval(int $snapshots_per_arm, int $total_tu /** * Determine if we should record a snapshot at this point. * + * Uses range-crossing: checks whether the range + * [total_turns - step_size + 1, total_turns] crosses a sampling + * boundary. This handles multi-arm experiments where total_turns + * jumps by the arm count per request. + * * @param string $experiment_id * The experiment ID. * @param string $arm_id @@ -291,22 +296,24 @@ protected function calculateMiddleInterval(int $snapshots_per_arm, int $total_tu * Current total experiment turns. * @param int $snapshots_per_arm * Snapshot budget per arm. + * @param int $step_size + * How much total_turns increased on this request. * * @return bool * TRUE if we should record. */ - protected function shouldRecordSnapshot(string $experiment_id, string $arm_id, int $total_turns, int $snapshots_per_arm): bool { + protected function shouldRecordSnapshot(string $experiment_id, string $arm_id, int $total_turns, int $snapshots_per_arm, int $step_size = 1): bool { $first_window = $this->calculateFirstWindow($snapshots_per_arm); + $previous_turns = $total_turns - max(1, $step_size); - // Always record in first window. - if ($total_turns <= $first_window) { + // Record if the step crossed into or is within the first window. + if ($previous_turns < $first_window) { return TRUE; } - // Always record recent (cleanup handles the window). - // For middle section, use interval. + // For middle section, check if step crossed an interval boundary. $interval = $this->calculateMiddleInterval($snapshots_per_arm, $total_turns); - return ($total_turns % $interval) === 0; + return (int) floor($total_turns / $interval) !== (int) floor($previous_turns / $interval); } /** @@ -316,21 +323,24 @@ protected function shouldRecordSnapshot(string $experiment_id, string $arm_id, i * Current total turns. * @param int $snapshots_per_arm * Snapshot budget per arm. + * @param int $step_size + * How much total_turns increased on this request. * * @return bool * TRUE if this is a milestone. */ - protected function isMilestone(int $total_turns, int $snapshots_per_arm): bool { + protected function isMilestone(int $total_turns, int $snapshots_per_arm, int $step_size = 1): bool { $first_window = $this->calculateFirstWindow($snapshots_per_arm); + $previous_turns = $total_turns - max(1, $step_size); // First window are all milestones. - if ($total_turns <= $first_window) { + if ($previous_turns < $first_window) { return TRUE; } - // Middle section milestones at interval points. + // Middle section milestones at interval boundary crossings. $interval = $this->calculateMiddleInterval($snapshots_per_arm, $total_turns); - return ($total_turns % $interval) === 0; + return (int) floor($total_turns / $interval) !== (int) floor($previous_turns / $interval); } /** diff --git a/src/Storage/SnapshotStorageInterface.php b/src/Storage/SnapshotStorageInterface.php index 56c65c6..2003836 100644 --- a/src/Storage/SnapshotStorageInterface.php +++ b/src/Storage/SnapshotStorageInterface.php @@ -28,8 +28,13 @@ public function isEnabled(): bool; * Cumulative rewards for this arm. * @param int $total_experiment_turns * Total turns across all arms in experiment. + * @param int $step_size + * How much total_experiment_turns increased on this request. + * Defaults to 1 for single-arm calls; should be the arm count for + * multi-arm recordTurns calls so the sampling algorithm can detect + * interval boundary crossings. */ - public function recordSnapshot(string $experiment_id, string $arm_id, int $turns, int $rewards, int $total_experiment_turns): void; + public function recordSnapshot(string $experiment_id, string $arm_id, int $turns, int $rewards, int $total_experiment_turns, int $step_size = 1): void; /** * Get snapshot history for an experiment. From f31d25bfa333f934271fdd478a72910256ea3407 Mon Sep 17 00:00:00 2001 From: jjroelofs <904576+jjroelofs@users.noreply.github.com> Date: Tue, 30 Jun 2026 13:15:17 +0200 Subject: [PATCH 2/4] fix: use ClientInterface::request() instead of shorthand post() GuzzleHttp\ClientInterface does not define post(); that method only exists on the concrete Client class. PHPStan correctly flags this as method.notFound. Use request('POST', ...) which is part of the interface contract. --- src/Service/EndpointChecker.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Service/EndpointChecker.php b/src/Service/EndpointChecker.php index 4086aed..5779e20 100644 --- a/src/Service/EndpointChecker.php +++ b/src/Service/EndpointChecker.php @@ -308,7 +308,7 @@ protected function probe(string $url, ?string $host_header = NULL): array { } try { - $response = $this->httpClient->post($url, $options); + $response = $this->httpClient->request('POST', $url, $options); } catch (\Throwable $e) { // Transport-layer failure (DNS / TCP / TLS / timeout); check() will From 99c32c9c26d4b87fb3389a7acea580e1db72f20b Mon Sep 17 00:00:00 2001 From: jjroelofs <904576+jjroelofs@users.noreply.github.com> Date: Tue, 30 Jun 2026 13:56:07 +0200 Subject: [PATCH 3/4] fix: sample snapshots on per-arm turns, not total experiment turns Address PR #54 review feedback: P1: Sampling based on total_experiment_turns made every large batch cross an interval boundary, marking all snapshots as permanent milestones that cleanup could not remove. Switch to per-arm turns which always increment by 1 regardless of batch size, making the modulo check reliable. Milestones now use a coarser multiple of the sampling interval so they always land on recorded snapshots. P2: Revert the step_size parameter added to SnapshotStorageInterface, preserving the original 5-argument contract for existing implementers. Also: - Raise MAX_ROWS_PER_EXPERIMENT from 10k to 100k for better chart resolution in many-arm experiments (208 arms: 48 -> 250 per arm). - Lower calculateSnapshotsPerArm floor from 20 to 2 so large arm counts stay within the per-experiment row budget. - Cleanup now enforces per-arm budgets by removing oldest rows (including milestones) when arm count grows and quotas shrink. --- src/Storage/ExperimentDataStorage.php | 14 ++-- src/Storage/SnapshotStorage.php | 94 ++++++++++++++---------- src/Storage/SnapshotStorageInterface.php | 7 +- 3 files changed, 62 insertions(+), 53 deletions(-) diff --git a/src/Storage/ExperimentDataStorage.php b/src/Storage/ExperimentDataStorage.php index c755e6a..2e9708f 100644 --- a/src/Storage/ExperimentDataStorage.php +++ b/src/Storage/ExperimentDataStorage.php @@ -77,7 +77,7 @@ public function recordTurn($experiment_id, $arm_id) { ->execute(); // Record snapshot if enabled. - $this->maybeRecordSnapshots($experiment_id, [$arm_id], 1); + $this->maybeRecordSnapshots($experiment_id, [$arm_id]); } /** @@ -113,9 +113,8 @@ public function recordTurns($experiment_id, array $arm_ids) { ->expression('updated', ':timestamp', [':timestamp' => $timestamp]) ->execute(); - // Record snapshots if enabled. Step size = arm count since - // total_turns was incremented by that amount. - $this->maybeRecordSnapshots($experiment_id, $arm_ids, $arm_count); + // Record snapshots if enabled. + $this->maybeRecordSnapshots($experiment_id, $arm_ids); } /** @@ -235,10 +234,8 @@ public function getAllArmsDataMultiple(array $experiment_ids): array { * The experiment ID. * @param array $arm_ids * Array of arm IDs to snapshot. - * @param int $step_size - * How much total_turns increased on this request. */ - protected function maybeRecordSnapshots(string $experiment_id, array $arm_ids, int $step_size = 1): void { + protected function maybeRecordSnapshots(string $experiment_id, array $arm_ids): void { if (!$this->snapshotStorage || !$this->snapshotStorage->isEnabled()) { return; } @@ -253,8 +250,7 @@ protected function maybeRecordSnapshots(string $experiment_id, array $arm_ids, i $arm_id, (int) $arm_data->turns, (int) $arm_data->rewards, - $total_turns, - $step_size + $total_turns ); } } diff --git a/src/Storage/SnapshotStorage.php b/src/Storage/SnapshotStorage.php index bead8d3..88b9d20 100644 --- a/src/Storage/SnapshotStorage.php +++ b/src/Storage/SnapshotStorage.php @@ -19,7 +19,7 @@ class SnapshotStorage implements SnapshotStorageInterface { /** * Maximum rows per experiment to prevent single experiment dominating. */ - const MAX_ROWS_PER_EXPERIMENT = 10000; + const MAX_ROWS_PER_EXPERIMENT = 100000; /** * The database connection. @@ -68,20 +68,19 @@ public function isEnabled(): bool { /** * {@inheritdoc} */ - public function recordSnapshot(string $experiment_id, string $arm_id, int $turns, int $rewards, int $total_experiment_turns, int $step_size = 1): void { + public function recordSnapshot(string $experiment_id, string $arm_id, int $turns, int $rewards, int $total_experiment_turns): void { if (!$this->isEnabled()) { return; } - // Check if we should record a snapshot based on our budget strategy. $arm_count = $this->getArmCount($experiment_id); $snapshots_per_arm = $this->calculateSnapshotsPerArm($arm_count); - if (!$this->shouldRecordSnapshot($experiment_id, $arm_id, $total_experiment_turns, $snapshots_per_arm, $step_size)) { + if (!$this->shouldRecordSnapshot($turns, $snapshots_per_arm)) { return; } - $is_milestone = $this->isMilestone($total_experiment_turns, $snapshots_per_arm, $step_size); + $is_milestone = $this->isMilestone($turns, $snapshots_per_arm); $this->database->insert('rl_arm_snapshots') ->fields([ @@ -162,7 +161,7 @@ public function cleanup(): int { ->fetchCol(); foreach ($arms as $arm_id) { - // Get non-milestone snapshots beyond recent window. + // Remove non-milestone snapshots beyond recent window. $subquery = $this->database->select('rl_arm_snapshots', 's') ->fields('s', ['id']) ->condition('experiment_id', $experiment_id) @@ -178,6 +177,33 @@ public function cleanup(): int { ->condition('id', $ids_to_delete, 'IN') ->execute(); } + + // If still over per-arm budget, remove oldest rows regardless + // of milestone status. This handles growing arm counts where + // the per-arm quota shrinks over time. + $remaining = (int) $this->database->select('rl_arm_snapshots', 's') + ->condition('experiment_id', $experiment_id) + ->condition('arm_id', $arm_id) + ->countQuery() + ->execute() + ->fetchField(); + + if ($remaining > $snapshots_per_arm) { + $excess_ids = $this->database->select('rl_arm_snapshots', 's') + ->fields('s', ['id']) + ->condition('experiment_id', $experiment_id) + ->condition('arm_id', $arm_id) + ->orderBy('total_experiment_turns', 'ASC') + ->range(0, $remaining - $snapshots_per_arm) + ->execute() + ->fetchCol(); + + if (!empty($excess_ids)) { + $deleted += $this->database->delete('rl_arm_snapshots') + ->condition('id', $excess_ids, 'IN') + ->execute(); + } + } } } @@ -224,7 +250,7 @@ protected function calculateSnapshotsPerArm(int $arm_count): int { } return min( self::MAX_SNAPSHOTS_PER_ARM, - max(20, (int) floor(self::MAX_ROWS_PER_EXPERIMENT / $arm_count)) + max(2, (int) floor(self::MAX_ROWS_PER_EXPERIMENT / $arm_count)) ); } @@ -283,64 +309,56 @@ protected function calculateMiddleInterval(int $snapshots_per_arm, int $total_tu /** * Determine if we should record a snapshot at this point. * - * Uses range-crossing: checks whether the range - * [total_turns - step_size + 1, total_turns] crosses a sampling - * boundary. This handles multi-arm experiments where total_turns - * jumps by the arm count per request. + * Sampling is based on per-arm turns rather than total experiment turns. + * Each arm's turn counter increments by exactly 1 per exposure regardless + * of batch size, so the modulo check is reliable without step-size + * awareness. * - * @param string $experiment_id - * The experiment ID. - * @param string $arm_id - * The arm ID. - * @param int $total_turns - * Current total experiment turns. + * @param int $arm_turns + * Cumulative turns for this specific arm. * @param int $snapshots_per_arm * Snapshot budget per arm. - * @param int $step_size - * How much total_turns increased on this request. * * @return bool * TRUE if we should record. */ - protected function shouldRecordSnapshot(string $experiment_id, string $arm_id, int $total_turns, int $snapshots_per_arm, int $step_size = 1): bool { + protected function shouldRecordSnapshot(int $arm_turns, int $snapshots_per_arm): bool { $first_window = $this->calculateFirstWindow($snapshots_per_arm); - $previous_turns = $total_turns - max(1, $step_size); - // Record if the step crossed into or is within the first window. - if ($previous_turns < $first_window) { + if ($arm_turns <= $first_window) { return TRUE; } - // For middle section, check if step crossed an interval boundary. - $interval = $this->calculateMiddleInterval($snapshots_per_arm, $total_turns); - return (int) floor($total_turns / $interval) !== (int) floor($previous_turns / $interval); + $interval = $this->calculateMiddleInterval($snapshots_per_arm, $arm_turns); + return ($arm_turns % $interval) === 0; } /** - * Determine if this is a permanent milestone snapshot. + * Determine if this is a milestone snapshot. * - * @param int $total_turns - * Current total turns. + * First-window snapshots are always milestones. Beyond that, milestones + * use a coarser multiple of the sampling interval so they always land on + * recorded snapshots. Cleanup prefers removing non-milestones first but + * can also remove milestones when the per-arm budget shrinks. + * + * @param int $arm_turns + * Cumulative turns for this specific arm. * @param int $snapshots_per_arm * Snapshot budget per arm. - * @param int $step_size - * How much total_turns increased on this request. * * @return bool * TRUE if this is a milestone. */ - protected function isMilestone(int $total_turns, int $snapshots_per_arm, int $step_size = 1): bool { + protected function isMilestone(int $arm_turns, int $snapshots_per_arm): bool { $first_window = $this->calculateFirstWindow($snapshots_per_arm); - $previous_turns = $total_turns - max(1, $step_size); - // First window are all milestones. - if ($previous_turns < $first_window) { + if ($arm_turns <= $first_window) { return TRUE; } - // Middle section milestones at interval boundary crossings. - $interval = $this->calculateMiddleInterval($snapshots_per_arm, $total_turns); - return (int) floor($total_turns / $interval) !== (int) floor($previous_turns / $interval); + $interval = $this->calculateMiddleInterval($snapshots_per_arm, $arm_turns); + $milestone_interval = $interval * 5; + return ($arm_turns % $milestone_interval) === 0; } /** diff --git a/src/Storage/SnapshotStorageInterface.php b/src/Storage/SnapshotStorageInterface.php index 2003836..56c65c6 100644 --- a/src/Storage/SnapshotStorageInterface.php +++ b/src/Storage/SnapshotStorageInterface.php @@ -28,13 +28,8 @@ public function isEnabled(): bool; * Cumulative rewards for this arm. * @param int $total_experiment_turns * Total turns across all arms in experiment. - * @param int $step_size - * How much total_experiment_turns increased on this request. - * Defaults to 1 for single-arm calls; should be the arm count for - * multi-arm recordTurns calls so the sampling algorithm can detect - * interval boundary crossings. */ - public function recordSnapshot(string $experiment_id, string $arm_id, int $turns, int $rewards, int $total_experiment_turns, int $step_size = 1): void; + public function recordSnapshot(string $experiment_id, string $arm_id, int $turns, int $rewards, int $total_experiment_turns): void; /** * Get snapshot history for an experiment. From 43acc67ee39d396c58498d579f7b64915ad9628f Mon Sep 17 00:00:00 2001 From: jjroelofs <904576+jjroelofs@users.noreply.github.com> Date: Tue, 30 Jun 2026 14:07:07 +0200 Subject: [PATCH 4/4] fix: cleanup preserves early/recent windows and enforces global cap Address follow-up review on PR #54: 1. Per-arm cleanup now compacts middle history first, explicitly preserving the first-window (early learning) and recent-window snapshots. Only falls back to trimming early/recent rows when the middle section is fully exhausted. 2. Global cleanup now enforces the configured row limit even when milestone rows alone exceed it: a second pass removes oldest rows regardless of milestone status after the non-milestone pass is insufficient. --- src/Storage/SnapshotStorage.php | 121 +++++++++++++++++++++++++++----- 1 file changed, 102 insertions(+), 19 deletions(-) diff --git a/src/Storage/SnapshotStorage.php b/src/Storage/SnapshotStorage.php index 88b9d20..79e8555 100644 --- a/src/Storage/SnapshotStorage.php +++ b/src/Storage/SnapshotStorage.php @@ -178,9 +178,8 @@ public function cleanup(): int { ->execute(); } - // If still over per-arm budget, remove oldest rows regardless - // of milestone status. This handles growing arm counts where - // the per-arm quota shrinks over time. + // If still over per-arm budget, compact middle history while + // preserving the early and recent windows. $remaining = (int) $this->database->select('rl_arm_snapshots', 's') ->condition('experiment_id', $experiment_id) ->condition('arm_id', $arm_id) @@ -189,34 +188,27 @@ public function cleanup(): int { ->fetchField(); if ($remaining > $snapshots_per_arm) { - $excess_ids = $this->database->select('rl_arm_snapshots', 's') - ->fields('s', ['id']) - ->condition('experiment_id', $experiment_id) - ->condition('arm_id', $arm_id) - ->orderBy('total_experiment_turns', 'ASC') - ->range(0, $remaining - $snapshots_per_arm) - ->execute() - ->fetchCol(); - - if (!empty($excess_ids)) { - $deleted += $this->database->delete('rl_arm_snapshots') - ->condition('id', $excess_ids, 'IN') - ->execute(); - } + $deleted += $this->compactArmSnapshots( + $experiment_id, + $arm_id, + $remaining - $snapshots_per_arm, + $snapshots_per_arm + ); } } } // Global cleanup if over max rows. $max_rows = $this->configFactory->get('rl.settings')->get('event_log_max_rows') ?: 100000; - $total_rows = $this->database->select('rl_arm_snapshots', 's') + $total_rows = (int) $this->database->select('rl_arm_snapshots', 's') ->countQuery() ->execute() ->fetchField(); if ($total_rows > $max_rows) { $to_delete = $total_rows - $max_rows; - // Delete oldest non-milestone rows. + + // First pass: delete oldest non-milestone rows. $ids = $this->database->select('rl_arm_snapshots', 's') ->fields('s', ['id']) ->condition('is_milestone', 0) @@ -230,6 +222,28 @@ public function cleanup(): int { ->condition('id', $ids, 'IN') ->execute(); } + + // Second pass: if still over limit, remove oldest rows regardless + // of milestone status so the global cap is always enforceable. + $total_rows = (int) $this->database->select('rl_arm_snapshots', 's') + ->countQuery() + ->execute() + ->fetchField(); + + if ($total_rows > $max_rows) { + $ids = $this->database->select('rl_arm_snapshots', 's') + ->fields('s', ['id']) + ->orderBy('created', 'ASC') + ->range(0, $total_rows - $max_rows) + ->execute() + ->fetchCol(); + + if (!empty($ids)) { + $deleted += $this->database->delete('rl_arm_snapshots') + ->condition('id', $ids, 'IN') + ->execute(); + } + } } return $deleted; @@ -361,6 +375,75 @@ protected function isMilestone(int $arm_turns, int $snapshots_per_arm): bool { return ($arm_turns % $milestone_interval) === 0; } + /** + * Remove excess snapshots for one arm, preserving early and recent windows. + * + * Deletes from the middle section first so that the first-window (early + * learning) and recent-window (current state) snapshots are kept. Only + * falls back to trimming early/recent rows when the middle is exhausted. + * + * @param string $experiment_id + * The experiment ID. + * @param string $arm_id + * The arm ID. + * @param int $excess + * Number of rows to delete. + * @param int $snapshots_per_arm + * Current per-arm budget. + * + * @return int + * Number of rows actually deleted. + */ + protected function compactArmSnapshots(string $experiment_id, string $arm_id, int $excess, int $snapshots_per_arm): int { + $first_window = $this->calculateFirstWindow($snapshots_per_arm); + $recent_window = $this->calculateRecentWindow($snapshots_per_arm); + + $all_ids = $this->database->select('rl_arm_snapshots', 's') + ->fields('s', ['id']) + ->condition('experiment_id', $experiment_id) + ->condition('arm_id', $arm_id) + ->orderBy('total_experiment_turns', 'ASC') + ->execute() + ->fetchCol(); + + $total = count($all_ids); + $keep_early = min($first_window, $total); + $keep_recent = min($recent_window, max(0, $total - $keep_early)); + + $early_ids = array_slice($all_ids, 0, $keep_early); + $recent_ids = $keep_recent > 0 ? array_slice($all_ids, -$keep_recent) : []; + $protected = array_flip(array_merge($early_ids, $recent_ids)); + + // Build the delete list from middle rows (oldest middle first). + $to_delete = []; + for ($i = $keep_early; $i < $total - $keep_recent && count($to_delete) < $excess; $i++) { + if (!isset($protected[$all_ids[$i]])) { + $to_delete[] = $all_ids[$i]; + } + } + + // If middle exhausted and still need to delete, trim oldest overall. + if (count($to_delete) < $excess) { + foreach ($all_ids as $id) { + if (count($to_delete) >= $excess) { + break; + } + if (!in_array($id, $to_delete, TRUE)) { + $to_delete[] = $id; + } + } + } + + $deleted = 0; + if (!empty($to_delete)) { + $deleted = (int) $this->database->delete('rl_arm_snapshots') + ->condition('id', $to_delete, 'IN') + ->execute(); + } + + return $deleted; + } + /** * Get the number of arms for an experiment. *