diff --git a/app/Http/Controllers/TransactionController.php b/app/Http/Controllers/TransactionController.php index 648cb8c2..3df66dec 100644 --- a/app/Http/Controllers/TransactionController.php +++ b/app/Http/Controllers/TransactionController.php @@ -313,8 +313,10 @@ class TransactionController extends Controller if ($request->has('category_id')) { $newCategoryId = $request->input('category_id'); + $overrideHandler = app(CategoryOverrideHandler::class); + foreach ($transactions as $transaction) { - app(CategoryOverrideHandler::class)->record($transaction, $newCategoryId); + $overrideHandler->record($transaction, $newCategoryId); } $updateData['category_id'] = $newCategoryId; diff --git a/app/Services/Ai/AiRuleLearner.php b/app/Services/Ai/AiRuleLearner.php index fe4aa2db..fdfba9d9 100644 --- a/app/Services/Ai/AiRuleLearner.php +++ b/app/Services/Ai/AiRuleLearner.php @@ -27,6 +27,23 @@ use Illuminate\Support\Str; */ class AiRuleLearner { + /** + * Per-user document-frequency corpus, memoized for the lifetime of this + * instance. A bulk correction runs learnFromCorrection once per transaction + * for the same user, and the description corpus is immutable while only + * categories change — so loading and tokenizing every description on every + * transaction (the N+1 in PHP-LARAVEL-40) is wasted work. + * + * SAFETY: this cache has no invalidation. It is only correct because the + * learner is resolved fresh per request (never bound singleton/scoped) and + * one instance only ever serves a single user. Do NOT bind this singleton or + * reuse one instance across users/requests — the corpus would go stale and + * leak. An arch test guards the non-singleton binding. + * + * @var array, count: int}> + */ + private array $descriptionCorpus = []; + public function __construct( private readonly DescriptionTokenizer $tokenizer, private readonly TransactionMatcher $matcher, @@ -153,16 +170,33 @@ class AiRuleLearner */ private function distinctiveDescriptionTokens(Transaction $transaction): array { - $descriptions = Transaction::query() - ->where('user_id', $transaction->user_id) - ->whereNull('description_iv') - ->pluck('description') - ->all(); + $corpus = $this->descriptionCorpus($transaction->user_id); + $threshold = $corpus['count'] * (float) config('ai_suggestions.noise_token_fraction'); - $frequency = $this->tokenizer->documentFrequency($descriptions); - $threshold = count($descriptions) * (float) config('ai_suggestions.noise_token_fraction'); + return $this->tokenizer->distinctiveTokens((string) $transaction->description, $corpus['frequency'], $threshold); + } - return $this->tokenizer->distinctiveTokens((string) $transaction->description, $frequency, $threshold); + /** + * The user's description document-frequency map and corpus size, loaded once + * per instance. Safe to memoize: descriptions are never mutated by a + * categorization change, so the corpus is stable across a bulk correction. + * + * @return array{frequency: array, count: int} + */ + private function descriptionCorpus(string $userId): array + { + return $this->descriptionCorpus[$userId] ??= (function () use ($userId): array { + $descriptions = Transaction::query() + ->where('user_id', $userId) + ->whereNull('description_iv') + ->pluck('description') + ->all(); + + return [ + 'frequency' => $this->tokenizer->documentFrequency($descriptions), + 'count' => count($descriptions), + ]; + })(); } /** diff --git a/tests/Feature/Ai/AiRuleLearnerTest.php b/tests/Feature/Ai/AiRuleLearnerTest.php index 53d135bc..dfc6e509 100644 --- a/tests/Feature/Ai/AiRuleLearnerTest.php +++ b/tests/Feature/Ai/AiRuleLearnerTest.php @@ -10,6 +10,7 @@ use App\Models\User; use App\Services\Ai\AiRuleLearner; use App\Services\Ai\CategorizationOutcome; use App\Services\AutomationRuleService; +use Illuminate\Support\Facades\DB; function expenseCategory(User $user): Category { @@ -51,6 +52,61 @@ it('creates an ai-owned rule at the lowest priority and links the transaction', ->and($transaction->refresh()->categorized_by_rule_id)->toBe($rule->id); }); +it('resolves a fresh instance per container lookup so the memoized corpus cannot leak or go stale', function () { + // The per-user corpus cache has no invalidation and is safe only while the + // learner is never a singleton. Guard that invariant. + expect(app(AiRuleLearner::class))->not->toBe(app(AiRuleLearner::class)); +}); + +it('learns each correction correctly across a batch while loading the corpus once', function () { + $user = User::factory()->create(); + $target = expenseCategory($user); + // A separate category keeps the corrected txns out of the "uncategorized" + // count, so the overbroad guard (which needs uncategorized rows) is a no-op + // and each correction actually learns a description rule. + $existing = expenseCategory($user); + + // Merchant-less, plaintext transactions force the description-token path, + // which is what loads the per-user description corpus. + $makeTxn = fn (string $description): Transaction => Transaction::factory()->plaintext()->create([ + 'user_id' => $user->id, + 'category_id' => $existing->id, + 'creditor_name' => null, + 'debtor_name' => null, + 'description' => $description, + ]); + + $first = $makeTxn('Netflix subscription'); + $second = $makeTxn('Spotify premium'); + + // One instance across the batch, mirroring the bulkUpdate loop where the + // handler (and its learner) is resolved once. + $learner = app(AiRuleLearner::class); + + DB::enableQueryLog(); + $firstRule = $learner->learnFromCorrection($first, $target->id); + $secondRule = $learner->learnFromCorrection($second, $target->id); + $queries = collect(DB::getQueryLog()); + DB::disableQueryLog(); + + // The second learning ran against the memoized corpus and still produced a + // distinct, valid clause: both corrections live in the one target rule. + expect($firstRule)->not->toBeNull() + ->and($secondRule)->not->toBeNull() + ->and($secondRule->id)->toBe($firstRule->id) + ->and($secondRule->refresh()->rules_json)->toHaveKey('or') + ->and($secondRule->rules_json['or'])->toHaveCount(2); + + // The corpus is the pluck of the `description` column (not the matcher's + // count(*) probes, which also filter on description_iv), loaded once. + $corpusLoads = $queries->filter(fn (array $q): bool => str_starts_with(strtolower(ltrim($q['query'])), 'select') + && str_contains($q['query'], 'description_iv') + && ! str_contains(strtolower($q['query']), 'count(') + ); + + expect($corpusLoads)->toHaveCount(1); +}); + it('appends a new merchant to the existing ai rule for the same category', function () { $user = User::factory()->create(); $category = expenseCategory($user);