fix(db): restore Phase 3 user_profile indexes with correct migration ordering (#2211)

This commit is contained in:
Mega Mind
2026-05-19 14:42:12 -07:00
committed by GitHub
parent b6552ea1c7
commit 28656085e1
3 changed files with 205 additions and 8 deletions
+28 -8
View File
@@ -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<Mutex>.
// 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
@@ -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
@@ -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<String> = c
.prepare(
"SELECT name FROM sqlite_master WHERE type = 'index' AND tbl_name = 'user_profile'",
)
.unwrap()
.query_map([], |row| row.get(0))
.unwrap()
.collect::<Result<Vec<_>, _>>()
.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<String> = 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::<Result<Vec<_>, _>>()
.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<String> = conn
.prepare(
"SELECT name FROM sqlite_master WHERE type = 'index' AND tbl_name = 'user_profile'",
)
.unwrap()
.query_map([], |row| row.get(0))
.unwrap()
.collect::<Result<Vec<_>, _>>()
.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:?}"
);
}