fix(migration): rebuild legacy observations table with valid ids

Author: Alan-TheGentleman

internal/store/store.go

@@ -307,6 +307,10 @@ func (s *Store) migrate() error {
 		}
 	}
 
+	if err := s.migrateLegacyObservationsTable(); err != nil {
+		return err
+	}
+
 	if _, err := s.db.Exec(`
 		CREATE INDEX IF NOT EXISTS idx_obs_scope ON observations(scope);
 		CREATE INDEX IF NOT EXISTS idx_obs_topic ON observations(topic_key, project, scope, updated_at DESC);
@@ -1357,6 +1361,138 @@ func (s *Store) addColumnIfNotExists(tableName, columnName, definition string) e
 	return err
 }
 
+func (s *Store) migrateLegacyObservationsTable() error {
+	rows, err := s.db.Query("PRAGMA table_info(observations)")
+	if err != nil {
+		return err
+	}
+	defer rows.Close()
+
+	var hasID bool
+	var idIsPrimaryKey bool
+	for rows.Next() {
+		var cid int
+		var name, typ string
+		var notNull int
+		var defaultValue any
+		var pk int
+		if err := rows.Scan(&cid, &name, &typ, &notNull, &defaultValue, &pk); err != nil {
+			return err
+		}
+		if name == "id" {
+			hasID = true
+			idIsPrimaryKey = pk == 1
+			break
+		}
+	}
+	if err := rows.Err(); err != nil {
+		return err
+	}
+
+	if !hasID || idIsPrimaryKey {
+		return nil
+	}
+
+	tx, err := s.db.Begin()
+	if err != nil {
+		return fmt.Errorf("migrate legacy observations: begin tx: %w", err)
+	}
+	defer tx.Rollback()
+
+	if _, err := tx.Exec(`
+		CREATE TABLE observations_migrated (
+			id         INTEGER PRIMARY KEY AUTOINCREMENT,
+			session_id TEXT    NOT NULL,
+			type       TEXT    NOT NULL,
+			title      TEXT    NOT NULL,
+			content    TEXT    NOT NULL,
+			tool_name  TEXT,
+			project    TEXT,
+			scope      TEXT    NOT NULL DEFAULT 'project',
+			topic_key  TEXT,
+			normalized_hash TEXT,
+			revision_count INTEGER NOT NULL DEFAULT 1,
+			duplicate_count INTEGER NOT NULL DEFAULT 1,
+			last_seen_at TEXT,
+			created_at TEXT    NOT NULL DEFAULT (datetime('now')),
+			updated_at TEXT    NOT NULL DEFAULT (datetime('now')),
+			deleted_at TEXT,
+			FOREIGN KEY (session_id) REFERENCES sessions(id)
+		);
+	`); err != nil {
+		return fmt.Errorf("migrate legacy observations: create table: %w", err)
+	}
+
+	if _, err := tx.Exec(`
+		INSERT INTO observations_migrated (
+			id, session_id, type, title, content, tool_name, project,
+			scope, topic_key, normalized_hash, revision_count, duplicate_count,
+			last_seen_at, created_at, updated_at, deleted_at
+		)
+		SELECT
+			CASE
+				WHEN id IS NULL THEN NULL
+				WHEN ROW_NUMBER() OVER (PARTITION BY id ORDER BY rowid) = 1 THEN CAST(id AS INTEGER)
+				ELSE NULL
+			END,
+			session_id,
+			COALESCE(NULLIF(type, ''), 'manual'),
+			COALESCE(NULLIF(title, ''), 'Untitled observation'),
+			COALESCE(content, ''),
+			tool_name,
+			project,
+			CASE WHEN scope IS NULL OR scope = '' THEN 'project' ELSE scope END,
+			NULLIF(topic_key, ''),
+			normalized_hash,
+			CASE WHEN revision_count IS NULL OR revision_count < 1 THEN 1 ELSE revision_count END,
+			CASE WHEN duplicate_count IS NULL OR duplicate_count < 1 THEN 1 ELSE duplicate_count END,
+			last_seen_at,
+			COALESCE(NULLIF(created_at, ''), datetime('now')),
+			COALESCE(NULLIF(updated_at, ''), NULLIF(created_at, ''), datetime('now')),
+			deleted_at
+		FROM observations
+		ORDER BY rowid;
+	`); err != nil {
+		return fmt.Errorf("migrate legacy observations: copy rows: %w", err)
+	}
+
+	if _, err := tx.Exec("DROP TABLE observations"); err != nil {
+		return fmt.Errorf("migrate legacy observations: drop old table: %w", err)
+	}
+
+	if _, err := tx.Exec("ALTER TABLE observations_migrated RENAME TO observations"); err != nil {
+		return fmt.Errorf("migrate legacy observations: rename table: %w", err)
+	}
+
+	if _, err := tx.Exec(`
+		DROP TRIGGER IF EXISTS obs_fts_insert;
+		DROP TRIGGER IF EXISTS obs_fts_update;
+		DROP TRIGGER IF EXISTS obs_fts_delete;
+		DROP TABLE IF EXISTS observations_fts;
+		CREATE VIRTUAL TABLE observations_fts USING fts5(
+			title,
+			content,
+			tool_name,
+			type,
+			project,
+			content='observations',
+			content_rowid='id'
+		);
+		INSERT INTO observations_fts(rowid, title, content, tool_name, type, project)
+		SELECT id, title, content, tool_name, type, project
+		FROM observations
+		WHERE deleted_at IS NULL;
+	`); err != nil {
+		return fmt.Errorf("migrate legacy observations: rebuild fts: %w", err)
+	}
+
+	if err := tx.Commit(); err != nil {
+		return fmt.Errorf("migrate legacy observations: commit: %w", err)
+	}
+
+	return nil
+}
+
 func nullableString(s string) *string {
 	if s == "" {
 		return nil

internal/store/store_test.go

@@ -1,9 +1,13 @@
 package store
 
 import (
+	"database/sql"
+	"path/filepath"
 	"strings"
 	"testing"
 	"time"
+
+	_ "modernc.org/sqlite"
 )
 
 func newTestStore(t *testing.T) *Store {
@@ -313,6 +317,101 @@ func TestDifferentTopicsDoNotReplaceEachOther(t *testing.T) {
 	}
 }
 
+func TestNewMigratesLegacyObservationIDSchema(t *testing.T) {
+	dataDir := t.TempDir()
+	dbPath := filepath.Join(dataDir, "engram.db")
+
+	db, err := sql.Open("sqlite", dbPath)
+	if err != nil {
+		t.Fatalf("open legacy db: %v", err)
+	}
+
+	_, err = db.Exec(`
+		CREATE TABLE sessions (
+			id TEXT PRIMARY KEY,
+			project TEXT NOT NULL,
+			directory TEXT NOT NULL,
+			started_at TEXT NOT NULL DEFAULT (datetime('now')),
+			ended_at TEXT,
+			summary TEXT
+		);
+		CREATE TABLE observations (
+			id INT,
+			session_id TEXT,
+			type TEXT,
+			title TEXT,
+			content TEXT,
+			tool_name TEXT,
+			project TEXT,
+			created_at TEXT
+		);
+		INSERT INTO sessions (id, project, directory) VALUES ('s1', 'engram', '/tmp/engram');
+		INSERT INTO observations (id, session_id, type, title, content, project, created_at)
+		VALUES
+			(NULL, 's1', 'bugfix', 'legacy null', 'legacy null content', 'engram', datetime('now')),
+			(7, 's1', 'bugfix', 'legacy fixed', 'legacy fixed content', 'engram', datetime('now')),
+			(7, 's1', 'bugfix', 'legacy duplicate', 'legacy duplicate content', 'engram', datetime('now'));
+	`)
+	if err != nil {
+		_ = db.Close()
+		t.Fatalf("seed legacy db: %v", err)
+	}
+	if err := db.Close(); err != nil {
+		t.Fatalf("close legacy db: %v", err)
+	}
+
+	cfg := DefaultConfig()
+	cfg.DataDir = dataDir
+
+	s, err := New(cfg)
+	if err != nil {
+		t.Fatalf("new store after legacy schema: %v", err)
+	}
+	t.Cleanup(func() { _ = s.Close() })
+
+	obs, err := s.AllObservations("engram", "", 20)
+	if err != nil {
+		t.Fatalf("all observations after migration: %v", err)
+	}
+	if len(obs) != 3 {
+		t.Fatalf("expected 3 migrated observations, got %d", len(obs))
+	}
+
+	seen := make(map[int64]bool)
+	for _, o := range obs {
+		if o.ID <= 0 {
+			t.Fatalf("expected migrated observation id > 0, got %d", o.ID)
+		}
+		if seen[o.ID] {
+			t.Fatalf("expected unique migrated ids, duplicate %d", o.ID)
+		}
+		seen[o.ID] = true
+	}
+
+	results, err := s.Search("legacy", SearchOptions{Project: "engram", Limit: 10})
+	if err != nil {
+		t.Fatalf("search after migration: %v", err)
+	}
+	if len(results) == 0 {
+		t.Fatalf("expected search results after migration")
+	}
+
+	newID, err := s.AddObservation(AddObservationParams{
+		SessionID: "s1",
+		Type:      "bugfix",
+		Title:     "post migration",
+		Content:   "new row should get id",
+		Project:   "engram",
+		Scope:     "project",
+	})
+	if err != nil {
+		t.Fatalf("add observation after migration: %v", err)
+	}
+	if newID <= 0 {
+		t.Fatalf("expected autoincrement id after migration, got %d", newID)
+	}
+}
+
 func TestSuggestTopicKeyNormalizesDeterministically(t *testing.T) {
 	got := SuggestTopicKey("Architecture", "  Auth Model  ", "ignored")
 	if got != "architecture/auth-model" {