From 6f7c60a8fec095c190fa9ba4abdcb3c2b68a02c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vi=CC=81ctor=20Falco=CC=81n?= Date: Sun, 5 Jul 2026 11:22:50 +0200 Subject: [PATCH] fix(ai): de-duplicate the categorization backfill per user MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Raising retry_after stops one dispatch from being re-reserved, but it does nothing against a *duplicate* dispatch. The ai queue runs two concurrent workers, so a double "Enable AI" click (or re-enabling consent mid-run) launches two backfills that read the same pending snapshot and both bill the model — the very harm tries=1 was meant to avoid but structurally cannot. Implement ShouldBeUnique keyed on the user id, mirroring the sibling RetryTransientAiCategorizationJob, so a user has at most one backfill in flight. Add a test anchoring the dedup key. --- ...CategorizeUncategorizedTransactionsJob.php | 20 ++++++++++++++++++- ...ategorizeUncategorizedTransactionsTest.php | 9 +++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/app/Jobs/CategorizeUncategorizedTransactionsJob.php b/app/Jobs/CategorizeUncategorizedTransactionsJob.php index dcea2e7d..c39694f9 100644 --- a/app/Jobs/CategorizeUncategorizedTransactionsJob.php +++ b/app/Jobs/CategorizeUncategorizedTransactionsJob.php @@ -5,6 +5,7 @@ namespace App\Jobs; use App\Models\User; use App\Services\Ai\AiCategorizationGate; use App\Services\Ai\AiCategorizer; +use Illuminate\Contracts\Queue\ShouldBeUnique; use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Foundation\Queue\Queueable; use Illuminate\Support\Facades\Cache; @@ -15,11 +16,17 @@ use Throwable; * consent outside of onboarding. Progress is written to the cache so the * transactions page can poll it and surface live progress while the batch runs. * + * De-duplicated per user: a second dispatch (double "Enable AI" click, or + * re-enabling consent while a run is still in flight) would read the same + * pending-transaction snapshot on a concurrent worker and re-bill the model for + * work already underway — the exact harm tries=1 targets but cannot prevent, + * since tries only bounds re-attempts of one dispatch, not duplicate dispatches. + * * ponytail: mirrors CategorizeOnboardingTransactionsJob's selection + chunking; * kept separate so the onboarding pass stays progress-free. Fold the two * together if a third caller ever needs the same loop. */ -class CategorizeUncategorizedTransactionsJob implements ShouldQueue +class CategorizeUncategorizedTransactionsJob implements ShouldBeUnique, ShouldQueue { use Queueable; @@ -34,8 +41,19 @@ class CategorizeUncategorizedTransactionsJob implements ShouldQueue */ public int $tries = 1; + /** + * Safety TTL for the unique lock in case a worker dies mid-run; comfortably + * longer than a full run. + */ + public int $uniqueFor = 1800; + public function __construct(public User $user, public string $jobId) {} + public function uniqueId(): string + { + return $this->user->id; + } + public function viaQueue(): string { return (string) config('ai_categorization.queue'); diff --git a/tests/Feature/Ai/CategorizeUncategorizedTransactionsTest.php b/tests/Feature/Ai/CategorizeUncategorizedTransactionsTest.php index 854025e6..9edf1be3 100644 --- a/tests/Feature/Ai/CategorizeUncategorizedTransactionsTest.php +++ b/tests/Feature/Ai/CategorizeUncategorizedTransactionsTest.php @@ -9,6 +9,7 @@ use App\Models\Transaction; use App\Models\User; use App\Services\Ai\CategorizeTransactions; use App\Services\Ai\CategoryCatalog; +use Illuminate\Contracts\Queue\ShouldBeUnique; use Illuminate\Support\Facades\Bus; use Illuminate\Support\Facades\Cache; @@ -218,3 +219,11 @@ it('categorizes the most recent transactions first', function () { expect($order)->toBe([$newest->id, $middle->id, $oldest->id]); }); + +it('de-duplicates the backfill per user so a concurrent dispatch cannot double-bill', function () { + $user = User::factory()->create(); + $job = new CategorizeUncategorizedTransactionsJob($user, 'job-x'); + + expect($job)->toBeInstanceOf(ShouldBeUnique::class) + ->and($job->uniqueId())->toBe($user->id); +});