Run imports in the background behind a spinner

Import ran inline in the update loop, so the whole interface froze for as long as pdftotext took over a stack of statements, with no way to tell work in progress apart from a hang. It now runs as a command off the event loop, with a spinner and a note that PDFs take a while. A repeat i press is ignored and retag is refused while an import is running, so nothing writes to the index concurrently. The result arrives as a message carrying the counts, the first failure, and the first warning with a count of any others. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-09 01:12:48 +02:00
parent 9ba3fe8fbe
commit 6d4266c224
3 changed files with 227 additions and 11 deletions
@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"strings"

+	"github.com/charmbracelet/bubbles/spinner"
 	"github.com/charmbracelet/bubbles/table"
 	"github.com/charmbracelet/bubbles/textinput"
 	tea "github.com/charmbracelet/bubbletea"
@@ -51,6 +52,11 @@ type Model struct {
 	txnTable     table.Model
 	reportTable  table.Model
 	text         textinput.Model
+	spinner      spinner.Model
+
+	// importing is set while an import runs in the background, so the UI keeps
+	// redrawing instead of freezing on a slow PDF.
+	importing bool

 	txns []model.Transaction // rows currently shown in txnTable

@@ -87,6 +93,10 @@ func New(root string, db *store.DB, accounts []*config.Account, engine *rules.En
 	ti.Prompt = ""
 	ti.CharLimit = 64

+	sp := spinner.New()
+	sp.Spinner = spinner.Dot
+	sp.Style = lipgloss.NewStyle().Foreground(lipgloss.Color("62"))
+
 	styles := table.DefaultStyles()
 	styles.Header = styles.Header.Bold(true)
 	styles.Selected = styles.Selected.Bold(true).Foreground(lipgloss.Color("15")).Background(lipgloss.Color("62"))
@@ -103,6 +113,7 @@ func New(root string, db *store.DB, accounts []*config.Account, engine *rules.En
 		engine:   engine,
 		view:     viewAccounts,
 		text:     ti,
+		spinner:  sp,
 		accountTable: newTable([]table.Column{
 			{Title: "Account", Width: 20},
 			{Title: "Balance", Width: 14},
@@ -234,6 +245,22 @@ func (m *Model) selected() (model.Transaction, bool) {
 	return m.txns[i], true
 }

+// importDoneMsg carries the outcome of a background import back to the model.
+type importDoneMsg struct {
+	res importer.Result
+	err error
+}
+
+// importCmd runs the import off the event loop. Only values are captured, and
+// the model is left untouched until the result comes back as a message.
+func (m *Model) importCmd() tea.Cmd {
+	root, db, accounts, engine := m.root, m.db, m.accounts, m.engine
+	return func() tea.Msg {
+		res, err := importer.Run(root, db, accounts, engine, importer.Options{})
+		return importDoneMsg{res: res, err: err}
+	}
+}
+
 // Update implements tea.Model.
 func (m *Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 	switch msg := msg.(type) {
@@ -242,6 +269,17 @@ func (m *Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 		m.resize()
 		return m, nil

+	case spinner.TickMsg:
+		if !m.importing {
+			return m, nil // a stale tick from a finished import
+		}
+		var cmd tea.Cmd
+		m.spinner, cmd = m.spinner.Update(msg)
+		return m, cmd
+
+	case importDoneMsg:
+		return m.finishImport(msg)
+
 	case tea.KeyMsg:
 		if m.input != inputNone {
 			return m.updateInput(msg)
@@ -251,6 +289,48 @@ func (m *Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 	return m, nil
 }

+func (m *Model) finishImport(msg importDoneMsg) (tea.Model, tea.Cmd) {
+	m.importing = false
+	if msg.err != nil {
+		m.err = msg.err
+		m.status = "import failed"
+		return m, nil
+	}
+
+	_, added, skipped := msg.res.Total()
+	m.status = fmt.Sprintf("imported: %d new, %d duplicate", added, skipped)
+
+	// A failed file and a warning are both worth surfacing, but the status
+	// line only has room for the first thing that went wrong.
+	if failures := msg.res.Errs(); len(failures) > 0 {
+		m.err = fmt.Errorf("%s: %w", failures[0].Path, failures[0].Err)
+	} else if warnings := firstWarning(msg.res); warnings != "" {
+		m.status += " · " + warnings
+	}
+
+	if err := m.reload(); err != nil {
+		m.err = err
+	}
+	return m, nil
+}
+
+func firstWarning(res importer.Result) string {
+	total := 0
+	first := ""
+	for _, f := range res.Files {
+		for _, w := range f.Warnings {
+			if first == "" {
+				first = fmt.Sprintf("%s: %s", f.Path, w)
+			}
+			total++
+		}
+	}
+	if total > 1 {
+		return fmt.Sprintf("%s (+%d more warnings)", first, total-1)
+	}
+	return first
+}
+
 func (m *Model) resize() {
 	h := m.height - 6 // title, status, help, padding
 	if h < 3 {
@@ -438,6 +518,10 @@ func (m *Model) updateNormal(msg tea.KeyMsg) (tea.Model, tea.Cmd) {
 		return m, nil

 	case "r":
+		if m.importing {
+			m.status = "import in progress…"
+			return m, nil
+		}
 		n, err := m.engine.Retag(m.db)
 		if err != nil {
 			m.err = err
@@ -448,18 +532,13 @@ func (m *Model) updateNormal(msg tea.KeyMsg) (tea.Model, tea.Cmd) {
 		return m, nil

 	case "i":
-		res, err := importer.Run(m.root, m.db, m.accounts, m.engine, importer.Options{})
-		if err != nil {
-			m.err = err
-			return m, nil
+		if m.importing {
+			return m, nil // already running; ignore the repeat press
 		}
-		_, added, skipped := res.Total()
-		m.status = fmt.Sprintf("imported: %d new, %d duplicate", added, skipped)
-		if failures := res.Errs(); len(failures) > 0 {
-			m.err = fmt.Errorf("%s: %w", failures[0].Path, failures[0].Err)
-		}
-		m.err = m.reload()
-		return m, nil
+		m.importing = true
+		m.err = nil
+		m.status = ""
+		return m, tea.Batch(m.spinner.Tick, m.importCmd())
 	}

 	var cmd tea.Cmd
@@ -498,6 +577,8 @@ func (m *Model) View() string {
 	b.WriteString("\n")

 	switch {
+	case m.importing:
+		b.WriteString(statusStyle.Render(m.spinner.View() + m.importingLabel()))
 	case m.input == inputTag:
 		b.WriteString(statusStyle.Render("tag: ") + m.text.View())
 	case m.input == inputSearch:
@@ -582,7 +663,19 @@ func (m *Model) title() string {
 	}
 }

+// importingLabel names what the import is working through, since extracting
+// text from PDFs is where the wait actually comes from.
+func (m *Model) importingLabel() string {
+	if n := len(m.accounts); n > 0 {
+		return fmt.Sprintf("importing %d account(s)… parsing statements can take a while for PDFs", n)
+	}
+	return "importing…"
+}
+
 func (m *Model) help() string {
+	if m.importing {
+		return "importing… · q quit"
+	}
 	if m.input != inputNone {
 		return "enter confirm · esc cancel"
 	}
@@ -1,13 +1,17 @@
 package tui

 import (
+	"errors"
 	"path/filepath"
 	"strings"
 	"testing"
+	"time"

+	"github.com/charmbracelet/bubbles/spinner"
 	tea "github.com/charmbracelet/bubbletea"

 	"git.petrovv.com/nikola/money/internal/config"
+	"git.petrovv.com/nikola/money/internal/importer"
 	"git.petrovv.com/nikola/money/internal/model"
 	"git.petrovv.com/nikola/money/internal/rules"
 	"git.petrovv.com/nikola/money/internal/store"
@@ -257,6 +261,125 @@ func TestReportViewExcludesTransfers(t *testing.T) {
 	}
 }

+// Import must not block the event loop: pressing i starts a background command
+// and puts the view into a spinning state that keeps redrawing.
+func TestImportShowsSpinnerAndDoesNotBlock(t *testing.T) {
+	m, _ := newTestModel(t)
+
+	_, cmd := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'i'}})
+	if !m.importing {
+		t.Fatal("expected the model to be in the importing state")
+	}
+	if cmd == nil {
+		t.Fatal("expected a command to run the import in the background")
+	}
+	if view := m.View(); !strings.Contains(view, "importing") {
+		t.Errorf("expected the status line to say it is importing:\n%s", view)
+	}
+
+	// A tick advances the spinner and schedules the next frame.
+	_, tickCmd := m.Update(spinner.TickMsg{Time: time.Now()})
+	if tickCmd == nil {
+		t.Error("expected the spinner to schedule another tick while importing")
+	}
+
+	// Pressing i again must not start a second concurrent import.
+	if _, again := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'i'}}); again != nil {
+		t.Error("expected a repeat i press to be ignored while importing")
+	}
+	// Nor may retag run against the database mid-import.
+	m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'r'}})
+	if !strings.Contains(m.status, "import in progress") {
+		t.Errorf("status = %q, want retag to be refused during an import", m.status)
+	}
+
+	// The result arriving clears the spinner and reports what happened.
+	m.Update(importDoneMsg{res: importer.Result{
+		Files: []importer.FileResult{{Path: "checking/st.csv", New: 3, Skipped: 1}},
+	}})
+	if m.importing {
+		t.Error("expected the importing state to clear")
+	}
+	if !strings.Contains(m.status, "3 new") || !strings.Contains(m.status, "1 duplicate") {
+		t.Errorf("status = %q, want the import counts", m.status)
+	}
+	// A stale tick after the import finished must not restart the spinner.
+	if _, cmd := m.Update(spinner.TickMsg{Time: time.Now()}); cmd != nil {
+		t.Error("expected a tick after the import to be ignored")
+	}
+}
+
+// The spinner must keep ticking for as long as the import runs: one frame and
+// then silence looks exactly like the freeze it is meant to rule out.
+func TestSpinnerKeepsTicking(t *testing.T) {
+	m, _ := newTestModel(t)
+	m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'i'}})
+
+	// Drive the loop the way Bubble Tea does: run the command, feed the
+	// message it produces back into Update, and repeat.
+	var frames []string
+	cmd := tea.Cmd(m.spinner.Tick)
+	for i := 0; i < 4; i++ {
+		if cmd == nil {
+			t.Fatalf("tick %d: no command scheduled, the spinner stopped", i)
+		}
+		msg := cmd()
+		if msg == nil {
+			t.Fatalf("tick %d: command produced no message", i)
+		}
+		_, cmd = m.Update(msg)
+		frames = append(frames, m.spinner.View())
+	}
+
+	if len(unique(frames)) < 2 {
+		t.Errorf("spinner rendered %v; want the frame to advance between ticks", frames)
+	}
+}
+
+func unique(in []string) []string {
+	seen := map[string]bool{}
+	var out []string
+	for _, s := range in {
+		if !seen[s] {
+			seen[s] = true
+			out = append(out, s)
+		}
+	}
+	return out
+}
+
+func TestImportFailureIsReported(t *testing.T) {
+	m, _ := newTestModel(t)
+	m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'i'}})
+
+	m.Update(importDoneMsg{err: errors.New("index is locked")})
+	if m.importing {
+		t.Error("expected the importing state to clear on failure")
+	}
+	if m.err == nil || !strings.Contains(m.err.Error(), "index is locked") {
+		t.Errorf("err = %v, want the failure surfaced", m.err)
+	}
+}
+
+// A per-file warning is worth seeing without leaving the TUI.
+func TestImportWarningsSurface(t *testing.T) {
+	m, _ := newTestModel(t)
+	m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'i'}})
+
+	m.Update(importDoneMsg{res: importer.Result{Files: []importer.FileResult{{
+		Path:     "revolut/statement.csv",
+		New:      2,
+		Warnings: []string{"skipped 1 PENDING transactions", "skipped 3 rows in JPY"},
+	}}}})
+
+	if !strings.Contains(m.status, "skipped 1 PENDING") {
+		t.Errorf("status = %q, want the first warning", m.status)
+	}
+	if !strings.Contains(m.status, "+1 more") {
+		t.Errorf("status = %q, want a count of the remaining warnings", m.status)
+	}
+}
+
 // newEmptyModel builds a model over an index with nothing in it.
 func newEmptyModel(t *testing.T, accounts []*config.Account) *Model {
 	t.Helper()