fix(indexer): delete chunks_vec rows one rowid at a time

backend/internal/indexer/vecdelete_test.go

@@ -0,0 +1,108 @@
+package indexer
+
+import (
+	"context"
+	"regexp"
+	"strings"
+	"testing"
+)
+
+// vec0's idxStr opens with its plan: '1' fullscan, '2' point lookup. A
+// `rowid IN (…)` outside KNN falls back to a fullscan of every vector.
+var (
+	vec0PointPlan = regexp.MustCompile(`chunks_vec VIRTUAL TABLE INDEX \d+:2`)
+	vec0Fullscan  = regexp.MustCompile(`chunks_vec VIRTUAL TABLE INDEX \d+:1`)
+)
+
+func vecPlan(t *testing.T, query string, args ...any) string {
+	t.Helper()
+	db := writeDB(t)
+	rows, err := db.Query(`EXPLAIN QUERY PLAN `+query, args...)
+	if err != nil {
+		t.Fatalf("explain: %v", err)
+	}
+	defer rows.Close()
+	var plan []string
+	for rows.Next() {
+		var id, parent, notused int
+		var detail string
+		if err := rows.Scan(&id, &parent, &notused, &detail); err != nil {
+			t.Fatalf("scan: %v", err)
+		}
+		plan = append(plan, detail)
+	}
+	if err := rows.Err(); err != nil {
+		t.Fatalf("rows: %v", err)
+	}
+	return strings.Join(plan, "\n")
+}
+
+// TestDeleteVecRows_reportsEveryFailure: a vector delete that fails silently
+// leaves an orphaned vector answering questions about code that is gone.
+func TestDeleteVecRows_reportsEveryFailure(t *testing.T) {
+	for _, tc := range []struct {
+		name, drop, query string
+	}{
+		{"bad id query", "", `SELECT id FROM no_such_table`},
+		{"id that is not an integer", "", `SELECT 'x'`},
+		{"vector delete", "chunks_vec", `SELECT 1`},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			db := writeDB(t)
+			if tc.drop != "" {
+				if _, err := db.Exec(`DROP TABLE ` + tc.drop); err != nil {
+					t.Fatalf("drop: %v", err)
+				}
+			}
+			tx, err := db.Begin()
+			if err != nil {
+				t.Fatalf("begin: %v", err)
+			}
+			defer func() { _ = tx.Rollback() }()
+			if err := deleteVecRows(context.Background(), tx, tc.query); err == nil {
+				t.Error("deleteVecRows() err = nil, want the failure")
+			}
+		})
+	}
+}
+
+// TestWriter_failsWhenAMirrorCannotBeCleared: DeleteFile and ReplaceFile must
+// abort, never commit chunks whose mirror rows were left behind.
+func TestWriter_failsWhenAMirrorCannotBeCleared(t *testing.T) {
+	ctx := context.Background()
+	for _, tc := range []struct {
+		name, drop string
+		run        func(*Writer) error
+	}{
+		{"DeleteFile without chunks_fts", "chunks_fts", func(w *Writer) error { return w.DeleteFile(ctx, "shop", "src/A.java") }},
+		{"DeleteFile without chunks_vec", "chunks_vec", func(w *Writer) error { return w.DeleteFile(ctx, "shop", "src/A.java") }},
+		{"ReplaceFile without chunks_vec", "chunks_vec", func(w *Writer) error {
+			return w.ReplaceFile(ctx, "shop", "src/A.java", "def456", "java", 64,
+				sampleChunks(), [][]float32{vec(1), vec(2)}, nil, nil)
+		}},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			db := writeDB(t)
+			testee := NewWriter(db)
+			if err := testee.ReplaceFile(ctx, "shop", "src/A.java", "abc123", "java", 64,
+				sampleChunks(), [][]float32{vec(1), vec(2)}, nil, nil); err != nil {
+				t.Fatalf("ReplaceFile() err = %v", err)
+			}
+			if _, err := db.Exec(`DROP TABLE ` + tc.drop); err != nil {
+				t.Fatalf("drop: %v", err)
+			}
+			if err := tc.run(testee); err == nil {
+				t.Error("err = nil, want the mirror failure")
+			}
+		})
+	}
+}
+
+func TestDeleteVecRow_isPointLookup(t *testing.T) {
+	if plan := vecPlan(t, deleteVecRow, 1); !vec0PointPlan.MatchString(plan) {
+		t.Fatalf("deleteVecRow must be a vec0 point lookup, plan:\n%s", plan)
+	}
+	if plan := vecPlan(t, `DELETE FROM chunks_vec WHERE rowid IN (SELECT id FROM chunks WHERE file_id = ?)`, 1); !vec0Fullscan.MatchString(plan) {
+		t.Fatalf("rowid IN should show the fullscan this guards against, plan:\n%s", plan)
+	}
+}

backend/internal/indexer/write.go

@@ -237,10 +237,17 @@ func (w *Writer) DeleteFile(ctx context.Context, repo, path string) error {
 	// cascades chunks away, and chunks_vec and chunks_fts are NOT part of that
 	// cascade. Letting it fire first would orphan them permanently, and an
 	// orphaned vector keeps answering questions about deleted code.
+	//
+	// chunks_fts goes first because it is a write: the id read chunks_vec
+	// needs comes only once the transaction holds the lock.
 	const owned = `SELECT id FROM chunks WHERE file_id IN (SELECT id FROM files WHERE repo = ?1 AND path = ?2)`
+	if _, err := tx.ExecContext(ctx, `DELETE FROM chunks_fts WHERE rowid IN (`+owned+`)`, repo, path); err != nil {
+		return fmt.Errorf("delete %s/%s: %w", repo, path, err)
+	}
+	if err := deleteVecRows(ctx, tx, owned, repo, path); err != nil {
+		return fmt.Errorf("delete %s/%s: %w", repo, path, err)
+	}
 	for _, q := range []string{
-		`DELETE FROM chunks_vec WHERE rowid IN (` + owned + `)`,
-		`DELETE FROM chunks_fts WHERE rowid IN (` + owned + `)`,
 		`DELETE FROM chunks WHERE id IN (` + owned + `)`,
 		`DELETE FROM symbols WHERE file_id IN (SELECT id FROM files WHERE repo = ?1 AND path = ?2)`,
 		`DELETE FROM integration_tokens WHERE file_id IN (SELECT id FROM files WHERE repo = ?1 AND path = ?2)`,
@@ -255,6 +262,41 @@ func (w *Writer) DeleteFile(ctx context.Context, repo, path string) error {
 	return tx.Commit()
 }
 
+// deleteVecRow deletes one vector. vec0 resolves `rowid = ?` as a point
+// lookup; `rowid IN (…)` outside a KNN query falls back to a fullscan of every
+// vector, so chunks_vec is always cleared one row at a time.
+const deleteVecRow = `DELETE FROM chunks_vec WHERE rowid = ?`
+
+// deleteVecRows deletes the chunks_vec row of every chunk id query selects.
+// The ids are read in full before the first delete. tx must already have
+// written: a transaction that opens with this read holds a WAL snapshot and
+// fails its first write with "database is locked" (see DeleteFile).
+func deleteVecRows(ctx context.Context, tx *sql.Tx, query string, args ...any) error {
+	rows, err := tx.QueryContext(ctx, query, args...)
+	if err != nil {
+		return err
+	}
+	var ids []int64
+	for rows.Next() {
+		var id int64
+		if err := rows.Scan(&id); err != nil {
+			_ = rows.Close()
+			return err
+		}
+		ids = append(ids, id)
+	}
+	_ = rows.Close()
+	if err := rows.Err(); err != nil {
+		return err
+	}
+	for _, id := range ids {
+		if _, err := tx.ExecContext(ctx, deleteVecRow, id); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 // upsertFile inserts or updates the files row and returns its id.
 func upsertFile(ctx context.Context, tx *sql.Tx, repo, path, sha, lang string, size int, skipReason string) (int64, error) {
 	_, err := tx.ExecContext(ctx, `
@@ -274,13 +316,13 @@ func upsertFile(ctx context.Context, tx *sql.Tx, repo, path, sha, lang string, s
 
 // clearFileContent removes a file's chunks from all three tables and its
 // symbols, in the order the mirrors demand: gather the ids, delete the vec0 and
-// fts5 rows by rowid, and only then the chunks themselves.
+// fts5 rows by rowid, and only then the chunks themselves. Every caller has
+// already written (upsertFile), so the id read holds no stale snapshot.
 func clearFileContent(ctx context.Context, tx *sql.Tx, fileID int64) error {
-	// The set form, like purgeContent and DeleteFile: the mirrors by the
-	// file's chunk ids in one statement each, never a read of the ids and
-	// a delete per id.
+	if err := deleteVecRows(ctx, tx, `SELECT id FROM chunks WHERE file_id = ?`, fileID); err != nil {
+		return err
+	}
 	for _, q := range []string{
-		`DELETE FROM chunks_vec WHERE rowid IN (SELECT id FROM chunks WHERE file_id = ?)`,
 		`DELETE FROM chunks_fts WHERE rowid IN (SELECT id FROM chunks WHERE file_id = ?)`,
 		`DELETE FROM chunks WHERE file_id = ?`,
 	} {