fix(ai): surface learned-rule toast in edit modal and guard weak description keys (#635)
## Why
Two issues surfaced from a real mis-categorization corrected via the
**Edit Transaction** modal (not the table quick-edit).
### 1. The modal never told the user the AI learned
Both the table quick-edit and the edit modal hit the same backend
(`PATCH /transactions/{id}` → `CategoryOverrideHandler` →
`AiRuleLearner`), so both learn a forward rule from a correction. But
the modal **discarded** the update response and never read
`learned_rule`:
- No *"Learned: similar transactions will be categorized automatically"*
confirmation.
- No **Undo**.
- It still showed the generic *"Automatize"* prompt — offering to create
a rule that already exists.
The modal now reads `learned_rule` and mirrors the table's toast + undo,
skipping the automatize prompt when a rule was learned. Same i18n
strings as the table (no new keys).
### 2. The learner could key a rule on a single weak token
A correction on `"Pago en MADRID SUC 04 MADRID ES"` produced a rule
keyed on `suc` alone. The tokenizer drops noise by **per-user** document
frequency, so `madrid` (20% of this user's descriptions) is correctly
dropped — but `suc` (0.14%) survives, even though it is a generic
banking abbreviation that matches broadly as a substring. The per-user
filter can't see that `suc` is generic in general.
`AiRuleLearner::descriptionClause()` now refuses a single distinctive
token shorter than 5 characters. It learns only when there are **two or
more** distinctive tokens, or a single token of **at least 5
characters**. Merchant-keyed rules are unaffected.
## Tests
- `does not learn a description rule from a single short token`
- `learns a description rule from a single sufficiently long token`
- `learns a description rule from two short tokens`
All `AiRuleLearnerTest` pass (13/13).
## Not in this PR
The one already-learned prod rule still carries the stale `suc`
clause/title (low impact, matches ~2 txns). Cleaning that data is a
separate manual step.
This commit is contained in:
parent
477e4d50e2
commit
c159e8782e
|
|
@ -27,6 +27,15 @@ use Illuminate\Support\Str;
|
|||
*/
|
||||
class AiRuleLearner
|
||||
{
|
||||
/**
|
||||
* A description rule keyed on a single token needs that token to be long
|
||||
* enough to be distinctive; a short lone token (e.g. "suc") is a generic
|
||||
* banking abbreviation that stays rare in one user's corpus yet matches
|
||||
* broadly as a substring. Two or more tokens are specific enough regardless
|
||||
* of length.
|
||||
*/
|
||||
private const MIN_SOLE_TOKEN_LENGTH = 5;
|
||||
|
||||
/**
|
||||
* Per-user document-frequency corpus, memoized for the lifetime of this
|
||||
* instance. A bulk correction runs learnFromCorrection once per transaction
|
||||
|
|
@ -149,6 +158,10 @@ class AiRuleLearner
|
|||
return null;
|
||||
}
|
||||
|
||||
if (count($tokens) === 1 && mb_strlen($tokens[0]) < self::MIN_SOLE_TOKEN_LENGTH) {
|
||||
return null;
|
||||
}
|
||||
|
||||
if ($this->isOverbroad($transaction, $tokens)) {
|
||||
return null;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,3 +1,4 @@
|
|||
import { destroy } from '@/actions/App/Http/Controllers/Settings/AutomationRuleController';
|
||||
import { LabelCombobox } from '@/components/shared/label-combobox';
|
||||
import { CategorySelect } from '@/components/transactions/category-select';
|
||||
import { AmountInput } from '@/components/ui/amount-input';
|
||||
|
|
@ -39,6 +40,7 @@ import { type Label } from '@/types/label';
|
|||
import { type DecryptedTransaction } from '@/types/transaction';
|
||||
import { formatDate } from '@/utils/date';
|
||||
import { __ } from '@/utils/i18n';
|
||||
import { router } from '@inertiajs/react';
|
||||
import { getYear, parseISO } from 'date-fns';
|
||||
import { useEffect, useState } from 'react';
|
||||
import { toast } from 'sonner';
|
||||
|
|
@ -440,7 +442,10 @@ export function EditTransactionDialog({
|
|||
finalDecryptedDescription = trimmedDescription;
|
||||
}
|
||||
|
||||
await transactionSyncService.update(transaction.id, updateData);
|
||||
const result = await transactionSyncService.update(
|
||||
transaction.id,
|
||||
updateData,
|
||||
);
|
||||
|
||||
const updatedRecord = await transactionSyncService.getById(
|
||||
transaction.id,
|
||||
|
|
@ -476,7 +481,32 @@ export function EditTransactionDialog({
|
|||
toast.success(__('Transaction updated successfully'));
|
||||
onSuccess(updatedTransaction);
|
||||
|
||||
if (
|
||||
if (result.learned_rule) {
|
||||
// The correction already taught the system a forward rule, so
|
||||
// confirm that and offer an instant undo — and skip the
|
||||
// "Automatize" prompt, which would only offer to create a rule
|
||||
// that now exists. Mirrors the transaction-table flow.
|
||||
const ruleId = result.learned_rule.id;
|
||||
|
||||
toast.success(
|
||||
__(
|
||||
'Learned: similar transactions will be categorized automatically.',
|
||||
),
|
||||
{
|
||||
closeButton: true,
|
||||
duration: 10000,
|
||||
action: {
|
||||
label: __('Undo'),
|
||||
onClick: () => {
|
||||
router.delete(destroy(ruleId).url, {
|
||||
preserveScroll: true,
|
||||
preserveState: true,
|
||||
});
|
||||
},
|
||||
},
|
||||
},
|
||||
);
|
||||
} else if (
|
||||
selectedCategoryId &&
|
||||
selectedCategoryId !== transaction.category_id &&
|
||||
updatedCategory
|
||||
|
|
|
|||
|
|
@ -107,6 +107,68 @@ it('learns each correction correctly across a batch while loading the corpus onc
|
|||
expect($corpusLoads)->toHaveCount(1);
|
||||
});
|
||||
|
||||
it('does not learn a description rule from a single short token', function () {
|
||||
$user = User::factory()->create();
|
||||
$target = expenseCategory($user);
|
||||
// Corrected txn parked in another category, so the overbroad guard (which
|
||||
// measures uncategorized rows) is a no-op and only the token guard decides.
|
||||
$existing = expenseCategory($user);
|
||||
|
||||
// Merchant-less so the description-token path runs. A lone short token like
|
||||
// "suc" (sucursal) is a generic banking abbreviation and must not become a
|
||||
// rule, even when it is rare in this user's corpus.
|
||||
$transaction = Transaction::factory()->plaintext()->create([
|
||||
'user_id' => $user->id,
|
||||
'category_id' => $existing->id,
|
||||
'creditor_name' => null,
|
||||
'debtor_name' => null,
|
||||
'description' => 'suc',
|
||||
]);
|
||||
|
||||
expect(app(AiRuleLearner::class)->learnFromCorrection($transaction, $target->id))->toBeNull();
|
||||
});
|
||||
|
||||
it('learns a description rule from a single sufficiently long token', function () {
|
||||
$user = User::factory()->create();
|
||||
$target = expenseCategory($user);
|
||||
$existing = expenseCategory($user);
|
||||
|
||||
$transaction = Transaction::factory()->plaintext()->create([
|
||||
'user_id' => $user->id,
|
||||
'category_id' => $existing->id,
|
||||
'creditor_name' => null,
|
||||
'debtor_name' => null,
|
||||
'description' => 'netflix',
|
||||
]);
|
||||
|
||||
$rule = app(AiRuleLearner::class)->learnFromCorrection($transaction, $target->id);
|
||||
|
||||
expect($rule)->not->toBeNull()
|
||||
->and($rule->rules_json)->toBe(['in' => ['netflix', ['var' => 'description']]]);
|
||||
});
|
||||
|
||||
it('learns a description rule from two short tokens', function () {
|
||||
$user = User::factory()->create();
|
||||
$target = expenseCategory($user);
|
||||
$existing = expenseCategory($user);
|
||||
|
||||
$transaction = Transaction::factory()->plaintext()->create([
|
||||
'user_id' => $user->id,
|
||||
'category_id' => $existing->id,
|
||||
'creditor_name' => null,
|
||||
'debtor_name' => null,
|
||||
'description' => 'abc def',
|
||||
]);
|
||||
|
||||
$rule = app(AiRuleLearner::class)->learnFromCorrection($transaction, $target->id);
|
||||
|
||||
expect($rule)->not->toBeNull()
|
||||
->and($rule->rules_json)->toBe(['and' => [
|
||||
['in' => ['abc', ['var' => 'description']]],
|
||||
['in' => ['def', ['var' => 'description']]],
|
||||
]]);
|
||||
});
|
||||
|
||||
it('appends a new merchant to the existing ai rule for the same category', function () {
|
||||
$user = User::factory()->create();
|
||||
$category = expenseCategory($user);
|
||||
|
|
|
|||
Loading…
Reference in New Issue