Let reviews see low-confidence suggestions and rebase preview applies
A low-confidence answer was discarded inside Classify, so Analyse showed the row as unchanged instead of a reviewable suggestion; the fallback decision moves to the import path, which keeps the merchant link and the recorded confidence. ApplyPreview now rebases onto the current journal: unrelated commits during a minutes-long paced run no longer invalidate the review, only an edit to a selected transaction itself conflicts, and applied changes are pruned so the rest stay appliable.
This commit is contained in:
@@ -369,6 +369,82 @@ func TestStalePreviewCannotOverwriteManualCorrection(t *testing.T) {
|
|||||||
t.Fatal("stale apply partially changed records")
|
t.Fatal("stale apply partially changed records")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// A preview run is minutes long by design (paced provider calls), so a
|
||||||
|
// scheduled sync, an import, or an earlier partial apply committing in the
|
||||||
|
// meantime must not invalidate the review: only an edit to a selected
|
||||||
|
// transaction itself conflicts.
|
||||||
|
func TestApplyPreviewSurvivesUnrelatedCommitsAndPartialApplies(t *testing.T) {
|
||||||
|
ctx := context.Background()
|
||||||
|
a, s := testApp(t)
|
||||||
|
s = seed(t, a, s)
|
||||||
|
mockClassifier(t, a)
|
||||||
|
p, err := runPreview(t, a, PreviewRequest{Revision: s.Revision, From: "2026-09-01", To: "2026-09-30", Model: "test/model", Fields: Fields{Category: true}})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if len(p.Changes) != 2 {
|
||||||
|
t.Fatalf("expected two proposed changes: %+v", p)
|
||||||
|
}
|
||||||
|
// An unrelated registry edit moves the journal revision after the preview.
|
||||||
|
if _, err = a.Mutate(ctx, s.Revision, func(d *domain.Dataset) error {
|
||||||
|
d.Tags = append(d.Tags, domain.Tag{ID: "travel", Name: "travel"})
|
||||||
|
return nil
|
||||||
|
}); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
first, err := a.ApplyPreview(ctx, p.ID, p.Revision, []string{p.Changes[0].ID})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("unrelated commit invalidated the preview: %v", err)
|
||||||
|
}
|
||||||
|
// The partial apply moved the revision again; the remaining proposal must
|
||||||
|
// still apply without another paced provider run.
|
||||||
|
second, err := a.ApplyPreview(ctx, p.ID, p.Revision, []string{p.Changes[1].ID})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("partial apply consumed the remaining proposals: %v", err)
|
||||||
|
}
|
||||||
|
if second.Revision == first.Revision {
|
||||||
|
t.Fatal("second apply committed nothing")
|
||||||
|
}
|
||||||
|
for _, tx := range second.Data.Transactions {
|
||||||
|
if tx.Enrichment.CategoryID != "groceries" {
|
||||||
|
t.Fatalf("applied categories lost: %+v", tx.Enrichment)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
// Both changes are consumed now; re-applying must fail, not double-write.
|
||||||
|
if _, err = a.ApplyPreview(ctx, p.ID, p.Revision, []string{p.Changes[0].ID}); err == nil {
|
||||||
|
t.Fatal("consumed change applied twice")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Imports auto-apply only what the model is sure about: a low-confidence
|
||||||
|
// category lands on the editable fallback while the merchant link and the
|
||||||
|
// recorded confidence survive for review in Analyse.
|
||||||
|
func TestImportNeverAutoAppliesLowConfidenceCategory(t *testing.T) {
|
||||||
|
a, s := testApp(t)
|
||||||
|
provider := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
content := `{"merchant_id":null,"new_merchant":"REWE","category_id":"groceries","tag_ids":[],"confidence":"low"}`
|
||||||
|
json.NewEncoder(w).Encode(map[string]any{"choices": []any{map[string]any{
|
||||||
|
"finish_reason": "stop",
|
||||||
|
"message": map[string]any{"content": content},
|
||||||
|
}}})
|
||||||
|
}))
|
||||||
|
defer provider.Close()
|
||||||
|
a.classifier = classification.Client{APIKey: "test", Model: "test/model", BaseURL: provider.URL}
|
||||||
|
s = seed(t, a, s)
|
||||||
|
if len(s.Data.Transactions) != 2 {
|
||||||
|
t.Fatalf("import lost transactions: %d", len(s.Data.Transactions))
|
||||||
|
}
|
||||||
|
for _, tx := range s.Data.Transactions {
|
||||||
|
e := tx.Enrichment
|
||||||
|
if e.CategoryID != domain.ExpenseFallback {
|
||||||
|
t.Fatalf("low-confidence category was auto-applied: %+v", e)
|
||||||
|
}
|
||||||
|
if e.MerchantID == "" || e.Classification.Confidence != "low" || e.Classification.Source != "openrouter" {
|
||||||
|
t.Fatalf("merchant link or provenance lost: %+v", e)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
func TestTaxonomyProposalApprovalMintsOnlyApprovedEntries(t *testing.T) {
|
func TestTaxonomyProposalApprovalMintsOnlyApprovedEntries(t *testing.T) {
|
||||||
a, s := testApp(t)
|
a, s := testApp(t)
|
||||||
s = seed(t, a, s)
|
s = seed(t, a, s)
|
||||||
|
|||||||
@@ -82,6 +82,12 @@ func (a *App) importFacts(ctx context.Context, s State, facts []domain.Facts, in
|
|||||||
p, e := classification.Rules(t.Facts, s.Data)
|
p, e := classification.Rules(t.Facts, s.Data)
|
||||||
if a.settings.ClassifyOnImport {
|
if a.settings.ClassifyOnImport {
|
||||||
p, e = a.classifier.Classify(ctx, t.Facts, s.Data, false)
|
p, e = a.classifier.Classify(ctx, t.Facts, s.Data, false)
|
||||||
|
// A low-confidence category is never auto-applied on import: the
|
||||||
|
// merchant link and provenance stay, and Analyse shows the model's
|
||||||
|
// suggestion for review instead.
|
||||||
|
if e == nil && p.Enrichment.Classification.Confidence == "low" {
|
||||||
|
p.Enrichment.CategoryID = domain.Fallback(t.Facts).CategoryID
|
||||||
|
}
|
||||||
}
|
}
|
||||||
if e == nil {
|
if e == nil {
|
||||||
e = addProposal(&s.Data, p, t.Facts)
|
e = addProposal(&s.Data, p, t.Facts)
|
||||||
@@ -927,6 +933,13 @@ func syncBackoff(now time.Time, ops operational) time.Duration {
|
|||||||
func (a *App) RunScheduler(ctx context.Context) {
|
func (a *App) RunScheduler(ctx context.Context) {
|
||||||
timer := time.NewTimer(time.Minute)
|
timer := time.NewTimer(time.Minute)
|
||||||
defer timer.Stop()
|
defer timer.Stop()
|
||||||
|
// Prices keep their own clock: they come from a different provider, they are
|
||||||
|
// wanted even when no bank is connected, and a sync backoff must not delay
|
||||||
|
// them. The first run is shortly after start, so a fresh install or a
|
||||||
|
// restart does not leave a day's holdings unvalued waiting for the tick;
|
||||||
|
// after that it is daily, which is as often as a close changes.
|
||||||
|
prices := time.NewTimer(quoteStartup)
|
||||||
|
defer prices.Stop()
|
||||||
for {
|
for {
|
||||||
force := false
|
force := false
|
||||||
select {
|
select {
|
||||||
@@ -934,6 +947,10 @@ func (a *App) RunScheduler(ctx context.Context) {
|
|||||||
return
|
return
|
||||||
case <-a.syncRequested:
|
case <-a.syncRequested:
|
||||||
force = true
|
force = true
|
||||||
|
case <-prices.C:
|
||||||
|
a.RefreshQuotes(ctx)
|
||||||
|
prices.Reset(quoteInterval)
|
||||||
|
continue
|
||||||
case <-timer.C:
|
case <-timer.C:
|
||||||
}
|
}
|
||||||
a.mu.Lock()
|
a.mu.Lock()
|
||||||
|
|||||||
+48
-12
@@ -276,6 +276,21 @@ func classifyRange(ctx context.Context, client *classification.Client, s State,
|
|||||||
p.NewMerchants = append([]domain.Merchant{}, s.Data.Merchants[baseMerchants:]...)
|
p.NewMerchants = append([]domain.Merchant{}, s.Data.Merchants[baseMerchants:]...)
|
||||||
return p, nil
|
return p, nil
|
||||||
}
|
}
|
||||||
|
// enrichmentEqual compares enrichment semantically: tag order is not a change.
|
||||||
|
func enrichmentEqual(a, b domain.Enrichment) bool {
|
||||||
|
a.TagIDs = slices.Clone(a.TagIDs)
|
||||||
|
b.TagIDs = slices.Clone(b.TagIDs)
|
||||||
|
slices.Sort(a.TagIDs)
|
||||||
|
slices.Sort(b.TagIDs)
|
||||||
|
return reflect.DeepEqual(a, b)
|
||||||
|
}
|
||||||
|
|
||||||
|
// ApplyPreview rebases the selected proposals onto the current journal. A
|
||||||
|
// preview run is minutes long by design, so unrelated commits (a scheduled
|
||||||
|
// sync, an import, an earlier partial apply of this same preview) must not
|
||||||
|
// invalidate the review; only a selected transaction whose own enrichment
|
||||||
|
// changed since the preview snapshot conflicts. Applied changes are pruned so
|
||||||
|
// the remaining proposals stay appliable without another paced provider run.
|
||||||
func (a *App) ApplyPreview(ctx context.Context, id, rev string, ids []string) (State, error) {
|
func (a *App) ApplyPreview(ctx context.Context, id, rev string, ids []string) (State, error) {
|
||||||
a.mu.Lock()
|
a.mu.Lock()
|
||||||
defer a.mu.Unlock()
|
defer a.mu.Unlock()
|
||||||
@@ -290,12 +305,9 @@ func (a *App) ApplyPreview(ctx context.Context, id, rev string, ids []string) (S
|
|||||||
if err != nil {
|
if err != nil {
|
||||||
return State{}, err
|
return State{}, err
|
||||||
}
|
}
|
||||||
if s.Revision != rev {
|
changes := map[string]Change{}
|
||||||
return State{}, errors.New("revision conflict: data changed after preview; analyse again")
|
|
||||||
}
|
|
||||||
changes := map[string]domain.Enrichment{}
|
|
||||||
for _, c := range p.Changes {
|
for _, c := range p.Changes {
|
||||||
changes[c.ID] = c.After
|
changes[c.ID] = c
|
||||||
}
|
}
|
||||||
selected := map[string]bool{}
|
selected := map[string]bool{}
|
||||||
for _, id := range ids {
|
for _, id := range ids {
|
||||||
@@ -307,16 +319,29 @@ func (a *App) ApplyPreview(ctx context.Context, id, rev string, ids []string) (S
|
|||||||
if len(selected) == 0 {
|
if len(selected) == 0 {
|
||||||
return State{}, errors.New("select at least one change")
|
return State{}, errors.New("select at least one change")
|
||||||
}
|
}
|
||||||
|
applied := 0
|
||||||
needed := map[string]bool{}
|
needed := map[string]bool{}
|
||||||
for i, t := range s.Data.Transactions {
|
for i, t := range s.Data.Transactions {
|
||||||
if !selected[t.Facts.ID] {
|
if !selected[t.Facts.ID] {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
s.Data.Transactions[i].Enrichment = changes[t.Facts.ID]
|
c := changes[t.Facts.ID]
|
||||||
needed[changes[t.Facts.ID].MerchantID] = true
|
if !enrichmentEqual(t.Enrichment, c.Before) {
|
||||||
|
return State{}, errors.New("revision conflict: a selected transaction changed after the preview; analyse it again")
|
||||||
|
}
|
||||||
|
s.Data.Transactions[i].Enrichment = c.After
|
||||||
|
needed[c.After.MerchantID] = true
|
||||||
|
applied++
|
||||||
|
}
|
||||||
|
if applied != len(selected) {
|
||||||
|
return State{}, errors.New("revision conflict: a selected transaction no longer exists; analyse again")
|
||||||
|
}
|
||||||
|
existing := map[string]bool{}
|
||||||
|
for _, m := range s.Data.Merchants {
|
||||||
|
existing[m.ID] = true
|
||||||
}
|
}
|
||||||
for _, m := range p.NewMerchants {
|
for _, m := range p.NewMerchants {
|
||||||
if needed[m.ID] {
|
if needed[m.ID] && !existing[m.ID] {
|
||||||
s.Data.Merchants = append(s.Data.Merchants, m)
|
s.Data.Merchants = append(s.Data.Merchants, m)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -327,14 +352,25 @@ func (a *App) ApplyPreview(ctx context.Context, id, rev string, ids []string) (S
|
|||||||
LearnAlias(&s.Data, t.Facts, t.Enrichment.MerchantID)
|
LearnAlias(&s.Data, t.Facts, t.Enrichment.MerchantID)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
state, err := a.commit(ctx, rev, s.Data)
|
state, err := a.commit(ctx, s.Revision, s.Data)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return State{}, err
|
return State{}, err
|
||||||
}
|
}
|
||||||
delete(a.previews, id)
|
kept := make([]Change, 0, len(p.Changes)-applied)
|
||||||
if job := a.previewRun; job != nil && job.status.ID == id {
|
for _, c := range p.Changes {
|
||||||
a.previewRun = nil
|
if !selected[c.ID] {
|
||||||
|
kept = append(kept, c)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
if len(kept) == 0 {
|
||||||
|
delete(a.previews, id)
|
||||||
|
if job := a.previewRun; job != nil && job.status.ID == id {
|
||||||
|
a.previewRun = nil
|
||||||
|
}
|
||||||
|
return state, nil
|
||||||
|
}
|
||||||
|
p.Changes = kept
|
||||||
|
a.previews[id] = p
|
||||||
return state, nil
|
return state, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -258,9 +258,6 @@ func (c *Client) Classify(ctx context.Context, facts domain.Facts, data domain.D
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
e.Classification = domain.Provenance{Source: "openrouter", Model: model, Confidence: answer.Confidence, Timestamp: time.Now().UTC().Format(time.RFC3339)}
|
e.Classification = domain.Provenance{Source: "openrouter", Model: model, Confidence: answer.Confidence, Timestamp: time.Now().UTC().Format(time.RFC3339)}
|
||||||
if answer.Confidence == "low" {
|
|
||||||
e.CategoryID = domain.Fallback(facts).CategoryID
|
|
||||||
}
|
|
||||||
validationData := data
|
validationData := data
|
||||||
if proposed != nil {
|
if proposed != nil {
|
||||||
validationData.Merchants = append(append([]domain.Merchant{}, data.Merchants...), *proposed)
|
validationData.Merchants = append(append([]domain.Merchant{}, data.Merchants...), *proposed)
|
||||||
|
|||||||
@@ -438,7 +438,7 @@ func TestConfiguredPrivateNamesAndIdentifiersRedactWithoutRemovingPayee(t *testi
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestLowConfidenceKeepsMerchantAndTagsButUsesFallback(t *testing.T) {
|
func TestLowConfidenceKeepsProposalAndRecordsConfidence(t *testing.T) {
|
||||||
f, d := fixture()
|
f, d := fixture()
|
||||||
c := mockClient(t, func(w http.ResponseWriter, r *http.Request) {
|
c := mockClient(t, func(w http.ResponseWriter, r *http.Request) {
|
||||||
reply(w, `{"merchant_id":"mer_coffee","new_merchant":null,"category_id":"cat_food","tag_ids":["tag_daily"],"confidence":"low"}`)
|
reply(w, `{"merchant_id":"mer_coffee","new_merchant":null,"category_id":"cat_food","tag_ids":["tag_daily"],"confidence":"low"}`)
|
||||||
@@ -447,11 +447,13 @@ func TestLowConfidenceKeepsMerchantAndTagsButUsesFallback(t *testing.T) {
|
|||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatal(err)
|
t.Fatal(err)
|
||||||
}
|
}
|
||||||
if p.Enrichment.CategoryID != domain.ExpenseFallback ||
|
// Review flows need the model's suggestion; discarding it is the import
|
||||||
|
// path's decision, not the client's.
|
||||||
|
if p.Enrichment.CategoryID != "cat_food" ||
|
||||||
p.Enrichment.MerchantID != "mer_coffee" ||
|
p.Enrichment.MerchantID != "mer_coffee" ||
|
||||||
!reflect.DeepEqual(p.Enrichment.TagIDs, []string{"tag_daily"}) ||
|
!reflect.DeepEqual(p.Enrichment.TagIDs, []string{"tag_daily"}) ||
|
||||||
p.Enrichment.Classification.Confidence != "low" {
|
p.Enrichment.Classification.Confidence != "low" {
|
||||||
t.Fatalf("low-confidence proposal was not preserved safely: %+v", p)
|
t.Fatalf("low-confidence proposal was not preserved: %+v", p)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
+14
-10
@@ -359,9 +359,10 @@ export function Classification({
|
|||||||
</span>
|
</span>
|
||||||
</div>
|
</div>
|
||||||
{preview.revision !== state.revision && (
|
{preview.revision !== state.revision && (
|
||||||
<div className="alert error">
|
<div className="alert">
|
||||||
Your journal changed since this preview. Cancel it and generate a
|
Your journal changed since this preview. Selected changes still
|
||||||
fresh preview before applying.
|
apply as long as their transactions were not edited in the
|
||||||
|
meantime.
|
||||||
</div>
|
</div>
|
||||||
)}
|
)}
|
||||||
<section className="panel">
|
<section className="panel">
|
||||||
@@ -449,11 +450,7 @@ export function Classification({
|
|||||||
</button>
|
</button>
|
||||||
<button
|
<button
|
||||||
className="button primary"
|
className="button primary"
|
||||||
disabled={
|
disabled={busy || !selected.length}
|
||||||
busy ||
|
|
||||||
!selected.length ||
|
|
||||||
preview.revision !== state.revision
|
|
||||||
}
|
|
||||||
onClick={() => setConfirm(true)}
|
onClick={() => setConfirm(true)}
|
||||||
>
|
>
|
||||||
<Check size={16} />
|
<Check size={16} />
|
||||||
@@ -506,7 +503,7 @@ export function Classification({
|
|||||||
</button>
|
</button>
|
||||||
<button
|
<button
|
||||||
className="button primary"
|
className="button primary"
|
||||||
disabled={busy || preview.revision !== state.revision}
|
disabled={busy}
|
||||||
onClick={async () => {
|
onClick={async () => {
|
||||||
setBusy(true);
|
setBusy(true);
|
||||||
setError("");
|
setError("");
|
||||||
@@ -520,7 +517,14 @@ export function Classification({
|
|||||||
result,
|
result,
|
||||||
`Applied ${selected.length} classifications`,
|
`Applied ${selected.length} classifications`,
|
||||||
);
|
);
|
||||||
setPreview(null);
|
const remaining = preview.changes.filter(
|
||||||
|
(c) => !selected.includes(c.id),
|
||||||
|
);
|
||||||
|
setPreview(
|
||||||
|
remaining.length
|
||||||
|
? { ...preview, changes: remaining }
|
||||||
|
: null,
|
||||||
|
);
|
||||||
setSelected([]);
|
setSelected([]);
|
||||||
setConfirm(false);
|
setConfirm(false);
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
|
|||||||
Reference in New Issue
Block a user