From 28656085e183172008ed46e2db644dd58d06b695 Mon Sep 17 00:00:00 2001 From: Mega Mind <146339422+M3gA-Mind@users.noreply.github.com> Date: Wed, 20 May 2026 03:12:12 +0530 Subject: [PATCH] fix(db): restore Phase 3 user_profile indexes with correct migration ordering (#2211) --- src/openhuman/memory/store/unified/init.rs | 36 +++- src/openhuman/memory/store/unified/profile.rs | 20 +++ .../memory/store/unified/profile_tests.rs | 157 ++++++++++++++++++ 3 files changed, 205 insertions(+), 8 deletions(-) diff --git a/src/openhuman/memory/store/unified/init.rs b/src/openhuman/memory/store/unified/init.rs index 6601da612..134a93c3c 100644 --- a/src/openhuman/memory/store/unified/init.rs +++ b/src/openhuman/memory/store/unified/init.rs @@ -128,14 +128,12 @@ impl UnifiedMemory { // Event extraction tables. conn.execute_batch(super::events::EVENTS_INIT_SQL)?; - // User profile accumulation table. - conn.execute_batch(super::profile::PROFILE_INIT_SQL)?; - - // Phase 3 (#566): idempotently add new columns to existing databases. - // New installs already have these columns via PROFILE_INIT_SQL above. - // Existing DBs need the migration to add state/stability/user_state/evidence_refs_json. - // We run the ALTER TABLE statements directly on `conn` before wrapping it in Arc. - // SQL arrays are defined as constants in profile.rs to avoid duplication. + // Phase 3 (#566): add new columns to existing databases BEFORE running + // PROFILE_INIT_SQL. PROFILE_INIT_SQL includes indexes that reference + // state/stability/user_state — those CREATE INDEX statements fail on + // pre-Phase-3 databases unless the columns already exist. + // On fresh installs the table doesn't exist yet so ALTER TABLE fails + // silently here; PROFILE_INIT_SQL then creates it with all columns. { use super::profile::PHASE3_COLUMNS_SQL; for sql in PHASE3_COLUMNS_SQL.iter() { @@ -148,6 +146,28 @@ impl UnifiedMemory { } } + // User profile accumulation table (CREATE TABLE IF NOT EXISTS + all indexes). + // On existing databases the table creation is a no-op; the index creation + // succeeds because PHASE3_COLUMNS_SQL above has already added the columns. + conn.execute_batch(super::profile::PROFILE_INIT_SQL)?; + + // Phase 3 indexes: idempotently restore performance indexes removed in #1616. + // New installs get these from PROFILE_INIT_SQL; existing DBs get them here, + // after PHASE3_COLUMNS_SQL has ensured the columns exist. + { + use super::profile::PHASE3_INDEXES_SQL; + for sql in PHASE3_INDEXES_SQL { + match conn.execute_batch(sql) { + Ok(_) => tracing::debug!("[profile:init] index applied: {sql}"), + Err(e) => { + tracing::warn!( + "[profile:init] index creation failed (non-fatal): {sql}: {e}" + ); + } + } + } + } + // Idempotent legacy-namespace migration. // // Older writes via MemoryStoreTool packed the intended namespace into diff --git a/src/openhuman/memory/store/unified/profile.rs b/src/openhuman/memory/store/unified/profile.rs index 6904e3447..cd94bb512 100644 --- a/src/openhuman/memory/store/unified/profile.rs +++ b/src/openhuman/memory/store/unified/profile.rs @@ -44,6 +44,15 @@ CREATE TABLE IF NOT EXISTS user_profile ( CREATE INDEX IF NOT EXISTS idx_profile_type ON user_profile(facet_type); + +CREATE INDEX IF NOT EXISTS idx_profile_state_stability + ON user_profile(state, stability DESC); + +CREATE INDEX IF NOT EXISTS idx_profile_key + ON user_profile(key); + +CREATE INDEX IF NOT EXISTS idx_profile_state_user_stability + ON user_profile(state, user_state, stability); "#; /// Phase 3 ALTER TABLE statements for adding new columns to existing databases. @@ -59,6 +68,17 @@ pub const PHASE3_COLUMNS_SQL: &[&str] = &[ "ALTER TABLE user_profile ADD COLUMN cue_families_json TEXT", ]; +/// Phase 3 index definitions for idempotent restoration on existing databases. +/// +/// New installs get these via `PROFILE_INIT_SQL`. Existing databases (where the +/// indexes were removed in #1616) need them applied after `PHASE3_COLUMNS_SQL` +/// has ensured the columns exist. +pub const PHASE3_INDEXES_SQL: &[&str] = &[ + "CREATE INDEX IF NOT EXISTS idx_profile_state_stability ON user_profile(state, stability DESC)", + "CREATE INDEX IF NOT EXISTS idx_profile_key ON user_profile(key)", + "CREATE INDEX IF NOT EXISTS idx_profile_state_user_stability ON user_profile(state, user_state, stability)", +]; + /// Idempotent schema migration for existing databases. /// /// New installs get the full schema from `PROFILE_INIT_SQL`. Existing databases diff --git a/src/openhuman/memory/store/unified/profile_tests.rs b/src/openhuman/memory/store/unified/profile_tests.rs index 038c911ba..31ac26b7b 100644 --- a/src/openhuman/memory/store/unified/profile_tests.rs +++ b/src/openhuman/memory/store/unified/profile_tests.rs @@ -604,3 +604,160 @@ fn render_profile_context_groups_by_type() { "Preference and Role sections should be at different positions" ); } + +#[test] +fn fresh_db_has_phase3_indexes() { + let conn = setup_db(); + let c = conn.lock(); + let indexes: Vec = c + .prepare( + "SELECT name FROM sqlite_master WHERE type = 'index' AND tbl_name = 'user_profile'", + ) + .unwrap() + .query_map([], |row| row.get(0)) + .unwrap() + .collect::, _>>() + .unwrap(); + + assert!( + indexes.contains(&"idx_profile_state_stability".to_string()), + "Missing idx_profile_state_stability; found: {indexes:?}" + ); + assert!( + indexes.contains(&"idx_profile_key".to_string()), + "Missing idx_profile_key; found: {indexes:?}" + ); + assert!( + indexes.contains(&"idx_profile_state_user_stability".to_string()), + "Missing idx_profile_state_user_stability; found: {indexes:?}" + ); + assert!( + indexes.contains(&"idx_profile_type".to_string()), + "Missing idx_profile_type; found: {indexes:?}" + ); +} + +#[test] +fn phase3_indexes_applied_to_existing_db() { + use super::super::profile::{PHASE3_COLUMNS_SQL, PHASE3_INDEXES_SQL}; + use rusqlite::Connection; + + let pre_phase3_sql = " + CREATE TABLE IF NOT EXISTS user_profile ( + facet_id TEXT PRIMARY KEY, + facet_type TEXT NOT NULL, + key TEXT NOT NULL, + value TEXT NOT NULL, + confidence REAL NOT NULL DEFAULT 0.5, + evidence_count INTEGER NOT NULL DEFAULT 1, + source_segment_ids TEXT, + first_seen_at REAL NOT NULL, + last_seen_at REAL NOT NULL, + UNIQUE(facet_type, key) + ); + CREATE INDEX IF NOT EXISTS idx_profile_type ON user_profile(facet_type); + "; + let raw_conn = Connection::open_in_memory().unwrap(); + raw_conn.execute_batch(pre_phase3_sql).unwrap(); + + for sql in PHASE3_COLUMNS_SQL { + let _ = raw_conn.execute(sql, []); + } + for sql in PHASE3_INDEXES_SQL { + raw_conn.execute_batch(sql).unwrap(); + } + + let indexes: Vec = raw_conn + .prepare( + "SELECT name FROM sqlite_master WHERE type = 'index' AND tbl_name = 'user_profile'", + ) + .unwrap() + .query_map([], |row| row.get(0)) + .unwrap() + .collect::, _>>() + .unwrap(); + + assert!(indexes.contains(&"idx_profile_state_stability".to_string())); + assert!(indexes.contains(&"idx_profile_key".to_string())); + assert!(indexes.contains(&"idx_profile_state_user_stability".to_string())); +} + +#[test] +fn phase3_indexes_idempotent() { + use super::super::profile::PHASE3_INDEXES_SQL; + let conn = setup_db(); + let c = conn.lock(); + for sql in PHASE3_INDEXES_SQL { + c.execute_batch(sql).unwrap(); + } + for sql in PHASE3_INDEXES_SQL { + c.execute_batch(sql).unwrap(); + } +} + +/// Verify that the real `UnifiedMemory::new` bootstrap path applies Phase 3 +/// indexes when opened over a pre-Phase-3 database file (the exact scenario +/// that caused the original crash in initialization ordering). +#[test] +fn unified_memory_new_applies_phase3_indexes_to_existing_db() { + use super::super::UnifiedMemory; + use crate::openhuman::embeddings::NoopEmbedding; + use rusqlite::Connection; + use std::sync::Arc; + + let dir = tempfile::tempdir().unwrap(); + let workspace = dir.path(); + + // Seed a pre-Phase-3 database at the path UnifiedMemory::new will open. + let memory_dir = workspace.join("memory"); + std::fs::create_dir_all(&memory_dir).unwrap(); + let db_path = memory_dir.join("memory.db"); + { + let conn = Connection::open(&db_path).unwrap(); + conn.execute_batch( + "CREATE TABLE IF NOT EXISTS user_profile ( + facet_id TEXT PRIMARY KEY, + facet_type TEXT NOT NULL, + key TEXT NOT NULL, + value TEXT NOT NULL, + confidence REAL NOT NULL DEFAULT 0.5, + evidence_count INTEGER NOT NULL DEFAULT 1, + source_segment_ids TEXT, + first_seen_at REAL NOT NULL, + last_seen_at REAL NOT NULL, + UNIQUE(facet_type, key) + ); + CREATE INDEX IF NOT EXISTS idx_profile_type ON user_profile(facet_type);", + ) + .unwrap(); + } + + // Call the real bootstrap path — must not fail on a pre-Phase-3 DB. + let mem = UnifiedMemory::new(workspace, Arc::new(NoopEmbedding), None) + .expect("UnifiedMemory::new must succeed on a pre-Phase-3 database"); + + // The Phase 3 indexes must exist after initialization. + let conn = mem.conn.lock(); + let indexes: Vec = conn + .prepare( + "SELECT name FROM sqlite_master WHERE type = 'index' AND tbl_name = 'user_profile'", + ) + .unwrap() + .query_map([], |row| row.get(0)) + .unwrap() + .collect::, _>>() + .unwrap(); + + assert!( + indexes.contains(&"idx_profile_state_stability".to_string()), + "Missing idx_profile_state_stability; found: {indexes:?}" + ); + assert!( + indexes.contains(&"idx_profile_key".to_string()), + "Missing idx_profile_key; found: {indexes:?}" + ); + assert!( + indexes.contains(&"idx_profile_state_user_stability".to_string()), + "Missing idx_profile_state_user_stability; found: {indexes:?}" + ); +}