Fix silent error swallowing in repair_encoding.go

cmd/msgvault/cmd/repair_encoding.go

@@ -5,6 +5,7 @@ import (
 	"database/sql"
 	"fmt"
 	"io"
+	"os"
 	"strings"
 	"unicode/utf8"
 
@@ -55,6 +56,7 @@ type repairStats struct {
 	labels       int
 	filenames    int
 	convTitles   int
+	skippedRows  int
 }
 
 func repairEncoding(s *store.Store) error {
@@ -108,6 +110,9 @@ func repairEncoding(s *store.Store) error {
 	if stats.convTitles > 0 {
 		fmt.Printf("  Conv titles:   %d\n", stats.convTitles)
 	}
+	if stats.skippedRows > 0 {
+		fmt.Printf("  Skipped rows:  %d (scan errors)\n", stats.skippedRows)
+	}
 	fmt.Printf("  Total fields:  %d\n", total)
 	fmt.Println("\nRun 'msgvault build-cache --full-rebuild' to update the analytics cache.")
 	return nil
@@ -211,6 +216,8 @@ func repairMessageFields(s *store.Store, stats *repairStats) error {
 		var compression sql.NullString
 
 		if err := rows.Scan(&id, &subject, &bodyText, &bodyHTML, &snippet, &rawData, &compression); err != nil {
+			fmt.Fprintf(os.Stderr, "warning: skipping message row: scan error: %v\n", err)
+			stats.skippedRows++
 			continue
 		}
 
@@ -380,6 +387,8 @@ func repairDisplayNames(s *store.Store, stats *repairStats) error {
 			var id int64
 			var name string
 			if err := rows.Scan(&id, &name); err != nil {
+				fmt.Fprintf(os.Stderr, "warning: skipping %s row: scan error: %v\n", table.name, err)
+				stats.skippedRows++
 				continue
 			}
 
@@ -511,6 +520,8 @@ func repairOtherStrings(s *store.Store, stats *repairStats) error {
 			var id int64
 			var value string
 			if err := rows.Scan(&id, &value); err != nil {
+				fmt.Fprintf(os.Stderr, "warning: skipping %s.%s row: scan error: %v\n", table.name, table.column, err)
+				stats.skippedRows++
 				continue
 			}
 

cmd/msgvault/cmd/repair_encoding_test.go

@@ -0,0 +1,115 @@
+package cmd
+
+import (
+	"testing"
+
+	"github.com/wesm/msgvault/internal/testutil"
+)
+
+// TestRepairOtherStrings_LogsScanErrors verifies that scan errors during
+// repairOtherStrings are counted in stats.skippedRows rather than silently
+// swallowed. We trigger scan errors by recreating the labels table with a
+// TEXT id column and inserting a non-numeric id that can't be scanned into int64.
+func TestRepairOtherStrings_LogsScanErrors(t *testing.T) {
+	st := testutil.NewTestStore(t)
+	db := st.DB()
+
+	// Disable foreign keys so we can recreate the labels table
+	if _, err := db.Exec("PRAGMA foreign_keys = OFF"); err != nil {
+		t.Fatalf("disable foreign keys: %v", err)
+	}
+
+	// Drop labels and recreate with TEXT id (not INTEGER PRIMARY KEY).
+	// Scanning a non-numeric TEXT value into int64 triggers a scan error.
+	if _, err := db.Exec("DROP TABLE IF EXISTS labels"); err != nil {
+		t.Fatalf("drop labels: %v", err)
+	}
+	if _, err := db.Exec(`CREATE TABLE labels (
+		id TEXT, source_id INTEGER, source_label_id TEXT,
+		name TEXT NOT NULL, label_type TEXT, color TEXT
+	)`); err != nil {
+		t.Fatalf("create labels: %v", err)
+	}
+
+	// Insert a row with non-numeric id → Scan into int64 will fail
+	if _, err := db.Exec(`INSERT INTO labels (id, source_id, source_label_id, name, label_type)
+		VALUES ('not-a-number', 1, 'lbl1', 'Test Label', 'user')`); err != nil {
+		t.Fatalf("insert bad label: %v", err)
+	}
+
+	stats := &repairStats{}
+	err := repairOtherStrings(st, stats)
+	if err != nil {
+		t.Fatalf("repairOtherStrings returned error: %v", err)
+	}
+
+	// Before fix: skippedRows == 0 (scan error silently swallowed)
+	// After fix: skippedRows == 1 (scan error counted)
+	if stats.skippedRows != 1 {
+		t.Errorf("skippedRows = %d, want 1", stats.skippedRows)
+	}
+}
+
+// TestRepairDisplayNames_LogsScanErrors verifies that scan errors during
+// repairDisplayNames are counted in stats.skippedRows. We trigger scan errors
+// by recreating the participants table with a TEXT id column.
+func TestRepairDisplayNames_LogsScanErrors(t *testing.T) {
+	st := testutil.NewTestStore(t)
+	db := st.DB()
+
+	if _, err := db.Exec("PRAGMA foreign_keys = OFF"); err != nil {
+		t.Fatalf("disable foreign keys: %v", err)
+	}
+
+	// Drop participants and recreate with TEXT id
+	if _, err := db.Exec("DROP TABLE IF EXISTS participants"); err != nil {
+		t.Fatalf("drop participants: %v", err)
+	}
+	if _, err := db.Exec(`CREATE TABLE participants (
+		id TEXT, email_address TEXT, phone_number TEXT,
+		display_name TEXT, domain TEXT, canonical_id TEXT,
+		created_at DATETIME, updated_at DATETIME
+	)`); err != nil {
+		t.Fatalf("create participants: %v", err)
+	}
+
+	// Insert a row with non-numeric id → Scan into int64 will fail
+	if _, err := db.Exec(`INSERT INTO participants (id, email_address, display_name, domain)
+		VALUES ('bad-id', 'test@example.com', 'Test User', 'example.com')`); err != nil {
+		t.Fatalf("insert bad participant: %v", err)
+	}
+
+	stats := &repairStats{}
+	err := repairDisplayNames(st, stats)
+	if err != nil {
+		t.Fatalf("repairDisplayNames returned error: %v", err)
+	}
+
+	// Before fix: skippedRows == 0 (scan error silently swallowed)
+	// After fix: skippedRows == 1 (scan error counted)
+	if stats.skippedRows != 1 {
+		t.Errorf("skippedRows = %d, want 1", stats.skippedRows)
+	}
+}
+
+// TestRepairEncoding_NoScanErrors verifies that normal data produces
+// zero skipped rows.
+func TestRepairEncoding_NoScanErrors(t *testing.T) {
+	st := testutil.NewTestStore(t)
+
+	stats := &repairStats{}
+
+	if err := repairMessageFields(st, stats); err != nil {
+		t.Fatalf("repairMessageFields: %v", err)
+	}
+	if err := repairDisplayNames(st, stats); err != nil {
+		t.Fatalf("repairDisplayNames: %v", err)
+	}
+	if err := repairOtherStrings(st, stats); err != nil {
+		t.Fatalf("repairOtherStrings: %v", err)
+	}
+
+	if stats.skippedRows != 0 {
+		t.Errorf("skippedRows = %d, want 0 for valid data", stats.skippedRows)
+	}
+}