From c3b141a5bb505ba18571da1614d37c64288558e4 Mon Sep 17 00:00:00 2001 From: Zumie Date: Mon, 23 Mar 2026 05:40:26 -0500 Subject: [PATCH 1/3] fix: address #128 review feedback (index, validation, MEMORY_ITEM_SELECT, upsert provenance) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add CREATE INDEX on daily_entries.date in init schema and v1→v2 migration (prevents full-table scans as daily entries accumulate over time) - Add --date + --invalidate validation: return clear error instead of silently discarding --date (invalidation looks up by ID only) - Refactor query_recent_high_valence and query_open_todos to use MEMORY_ITEM_SELECT constant instead of hardcoded column lists (prevents future divergence) - Fix upsert provenance: preserve environment/origin_domain from existing row when incoming item does not supply them (prevents silent metadata loss on idempotent writes via MCP/server that omit optional provenance fields) - Add TODO comment for link_daily_memory in Day --update handler (scaffolded but not yet called; needs content-similarity lookup to be built first) Closes remaining review items from Greptile (3/5→4/5) and Codex P2 flag. --- crates/cli/src/main.rs | 8 ++++++++ crates/db/src/lib.rs | 23 +++++++++++++++-------- crates/db/src/migrate.rs | 1 + 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/crates/cli/src/main.rs b/crates/cli/src/main.rs index 035ce68..351092c 100644 --- a/crates/cli/src/main.rs +++ b/crates/cli/src/main.rs @@ -3876,6 +3876,9 @@ fn main() -> anyhow::Result<()> { let id = db .insert_daily_entry(&date, context.as_deref(), &content) .await?; + // TODO(#120): call db.link_daily_memory(&date, memory_id) here once + // content-similarity lookup (finding related memory_items for this entry) + // is implemented. The DB method is scaffolded; the lookup is not yet built. let entries = db.get_daily_entries(&date, false).await?; let daily = db.get_daily_summary(&date).await?; print_ok(json_mode, serde_json::json!({ @@ -4633,6 +4636,11 @@ fn validate_day_args( "--date cannot be used with --list".to_string(), )); } + if date.is_some() && invalidate.is_some() { + return Err(CliError::InvalidInput( + "--date cannot be combined with --invalidate; invalidation looks up the entry by ID only".to_string(), + )); + } Ok(()) } diff --git a/crates/db/src/lib.rs b/crates/db/src/lib.rs index c169698..6a503c1 100644 --- a/crates/db/src/lib.rs +++ b/crates/db/src/lib.rs @@ -495,6 +495,7 @@ impl MemoryDb { invalidate_reason TEXT,\ created_at INTEGER NOT NULL\ );\ + CREATE INDEX IF NOT EXISTS idx_daily_entries_date ON daily_entries (date);\ CREATE TABLE IF NOT EXISTS daily_memories (\ date TEXT,\ memory_id TEXT,\ @@ -811,11 +812,21 @@ impl MemoryDb { } } + // Preserve provenance from existing row if incoming does not supply it. + // This prevents idempotent writes from silently clearing previously captured provenance. + let environment = item.environment.clone().or(existing.environment.clone()); + let origin_domain = item + .origin_domain + .clone() + .or(existing.origin_domain.clone()); + let merged = MemoryItem { id: existing.id, created_at: existing.created_at, tags, files, + environment, + origin_domain, ..item.clone() }; @@ -979,16 +990,14 @@ impl MemoryDb { .conn .query( &format!( - "SELECT id, mem_type, content, environment, origin_domain, tags, created_at, \ - salience, primary_emotion, secondary_emotions, d_score, i_score, ev, files, actionable \ - FROM memory_items \ + "SELECT {} FROM memory_items \ WHERE created_at >= ?1 \ AND d_score IS NOT NULL \ AND ABS(d_score) >= ?2 \ AND {} \ ORDER BY ABS(d_score) DESC, created_at DESC \ LIMIT ?3", - actionable_filter + MEMORY_ITEM_SELECT, actionable_filter ), (cutoff.as_str(), threshold_f64, limit_i64), ) @@ -1022,9 +1031,7 @@ impl MemoryDb { .conn .query( &format!( - "SELECT id, mem_type, content, environment, origin_domain, tags, created_at, \ - salience, primary_emotion, secondary_emotions, d_score, i_score, ev, files, actionable \ - FROM memory_items \ + "SELECT {} FROM memory_items \ WHERE {} AND ( LOWER(mem_type) LIKE '%task%' \ OR LOWER(mem_type) LIKE '%open_loop%' \ @@ -1039,7 +1046,7 @@ impl MemoryDb { ) ORDER BY created_at DESC \ LIMIT ?1", - actionable_filter + MEMORY_ITEM_SELECT, actionable_filter ), [limit_i64], ) diff --git a/crates/db/src/migrate.rs b/crates/db/src/migrate.rs index ed47c2a..a73de51 100644 --- a/crates/db/src/migrate.rs +++ b/crates/db/src/migrate.rs @@ -226,6 +226,7 @@ async fn migrate_v1_to_v2_inner(conn: &Connection) -> anyhow::Result<()> { invalidate_reason TEXT,\ created_at INTEGER NOT NULL\ );\ + CREATE INDEX IF NOT EXISTS idx_daily_entries_date ON daily_entries (date);\ CREATE TABLE IF NOT EXISTS daily_memories (\ date TEXT,\ memory_id TEXT,\ From ac863a8b4bc26e62e7dc01f5fb6fea4f49ef0eac Mon Sep 17 00:00:00 2001 From: Zumie Date: Tue, 24 Mar 2026 10:32:59 -0500 Subject: [PATCH 2/3] fix: include daily_entries + daily_summaries in bundle export/import (#128 P1) Closes the data-loss regression identified in PR #128 review: - Add DailyEntry + DailySummary variants to BundleRecord (wagl.bundle.daily_entry / wagl.bundle.daily_summary) - Add MemoryDb::all_daily_entries() and all_daily_summaries() for bulk export - Add MemoryDb::upsert_daily_entry() and upsert_daily_summary() for idempotent import (INSERT OR IGNORE by id / date) - Update export_bundle() to write all daily entries (including invalidated) and summaries - Update import_bundle() to restore daily entries and summaries; entries drive summary content via refresh_daily_summary, summary import restores the hand-authored summary field --- crates/cli/src/bundle_cmd.rs | 43 ++++++++++++++++- crates/core/src/bundle.rs | 12 +++++ crates/db/src/lib.rs | 93 ++++++++++++++++++++++++++++++++++++ 3 files changed, 146 insertions(+), 2 deletions(-) diff --git a/crates/cli/src/bundle_cmd.rs b/crates/cli/src/bundle_cmd.rs index 6ccc4b4..c19dd13 100644 --- a/crates/cli/src/bundle_cmd.rs +++ b/crates/cli/src/bundle_cmd.rs @@ -21,8 +21,7 @@ pub async fn export_bundle( }; writeln!(f, "{}", serde_json::to_string(&header)?)?; - // v0: export primary memory items + expertise items. - // Future: include focus. + // Export primary memory items + expertise items + daily entries + daily summaries. // Export ALL items (including narrative) for bundle completeness. let items = db.all_memory_items_with_filter(true).await?; @@ -51,6 +50,28 @@ pub async fn export_bundle( } } + // Export daily entries (all, including invalidated — preserve full history). + let daily_entries = db.all_daily_entries().await?; + for entry in daily_entries { + let rec = BundleRecord::DailyEntry { + logical_key: entry.id.clone(), + item: entry, + }; + writeln!(f, "{}", serde_json::to_string(&rec)?)?; + wrote += 1; + } + + // Export daily summaries (the hand-authored summary field lives here). + let daily_summaries = db.all_daily_summaries().await?; + for summary in daily_summaries { + let rec = BundleRecord::DailySummary { + logical_key: summary.date.clone(), + item: summary, + }; + writeln!(f, "{}", serde_json::to_string(&rec)?)?; + wrote += 1; + } + Ok(wrote) } @@ -96,6 +117,24 @@ pub async fn import_bundle(db: &MemoryDb, in_path: &Path) -> Result { db.expertise_put_with_dedupe(&item, Some(&key)).await?; imported += 1; } + BundleRecord::DailyEntry { + logical_key: _, + item, + } => { + // Idempotent by entry id (INSERT OR IGNORE). + db.upsert_daily_entry(&item).await?; + imported += 1; + } + BundleRecord::DailySummary { + logical_key: _, + item, + } => { + // Idempotent by date (INSERT OR IGNORE). + // Entries drive summary content via refresh_daily_summary; + // this restores the hand-authored `summary` field. + db.upsert_daily_summary(&item).await?; + imported += 1; + } } } diff --git a/crates/core/src/bundle.rs b/crates/core/src/bundle.rs index 7f9d4e2..bbcc10d 100644 --- a/crates/core/src/bundle.rs +++ b/crates/core/src/bundle.rs @@ -26,4 +26,16 @@ pub enum BundleRecord { expertise: String, item: crate::types::ExpertiseItem, }, + + #[serde(rename = "wagl.bundle.daily_entry")] + DailyEntry { + logical_key: String, + item: crate::types::DailyEntry, + }, + + #[serde(rename = "wagl.bundle.daily_summary")] + DailySummary { + logical_key: String, + item: crate::types::DailySummary, + }, } diff --git a/crates/db/src/lib.rs b/crates/db/src/lib.rs index 6a503c1..32c2ba8 100644 --- a/crates/db/src/lib.rs +++ b/crates/db/src/lib.rs @@ -1732,6 +1732,99 @@ impl MemoryDb { Ok(()) } + /// Return all daily entries (including invalidated) across all dates, for bundle export. + pub async fn all_daily_entries(&self) -> anyhow::Result> { + let mut rows = self + .conn + .query( + "SELECT id, date, context, content, invalidated, invalidate_reason, created_at FROM daily_entries ORDER BY date ASC, created_at ASC", + (), + ) + .await + .context("all daily entries")?; + + let mut out = Vec::new(); + while let Some(row) = rows.next().await? { + let invalidated_i: i64 = row.get(4)?; + out.push(DailyEntry { + id: row.get(0)?, + date: row.get(1)?, + context: row.get(2)?, + content: row.get(3)?, + invalidated: invalidated_i != 0, + invalidate_reason: row.get(5)?, + created_at: row.get(6)?, + }); + } + Ok(out) + } + + /// Return all daily summaries across all dates, for bundle export. + pub async fn all_daily_summaries(&self) -> anyhow::Result> { + let mut rows = self + .conn + .query( + "SELECT date, content, summary, updated_at FROM daily ORDER BY date ASC", + (), + ) + .await + .context("all daily summaries")?; + + let mut out = Vec::new(); + while let Some(row) = rows.next().await? { + out.push(DailySummary { + date: row.get(0)?, + content: row.get(1)?, + summary: row.get(2)?, + updated_at: row.get(3)?, + }); + } + Ok(out) + } + + /// Upsert a daily entry during bundle import (idempotent by id). + pub async fn upsert_daily_entry(&self, entry: &DailyEntry) -> anyhow::Result<()> { + let invalidated_i: i64 = if entry.invalidated { 1 } else { 0 }; + self.conn + .execute( + "INSERT OR IGNORE INTO daily_entries (id, date, context, content, invalidated, invalidate_reason, created_at) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7)", + params![ + entry.id.clone(), + entry.date.clone(), + entry.context.clone(), + entry.content.clone(), + invalidated_i, + entry.invalidate_reason.clone(), + entry.created_at + ], + ) + .await + .context("upsert daily entry")?; + // Refresh the summary so it stays consistent with the entries. + self.refresh_daily_summary(&entry.date).await?; + Ok(()) + } + + /// Upsert a daily summary during bundle import (idempotent by date). + /// Only restores the hand-written `summary` field; the `content` field is + /// regenerated by `refresh_daily_summary` when entries are imported, so we + /// only fill it in here when there are no entries to drive the refresh. + pub async fn upsert_daily_summary(&self, summary: &DailySummary) -> anyhow::Result<()> { + self.conn + .execute( + "INSERT OR IGNORE INTO daily (date, content, summary, updated_at) VALUES (?1, ?2, ?3, ?4)", + params![ + summary.date.clone(), + summary.content.clone(), + summary.summary.clone(), + summary.updated_at + ], + ) + .await + .context("upsert daily summary")?; + Ok(()) + } + pub async fn insert_daily_entry( &self, date: &str, From 0ec67ed87963d8f6d87233956fb2aec62f3e9bb1 Mon Sep 17 00:00:00 2001 From: Clio Date: Tue, 24 Mar 2026 15:21:04 -0500 Subject: [PATCH 3/3] fix: restore hand-authored summary on bundle import upsert (#128) Co-Authored-By: Claude Sonnet 4.6 --- crates/db/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/db/src/lib.rs b/crates/db/src/lib.rs index 32c2ba8..befea74 100644 --- a/crates/db/src/lib.rs +++ b/crates/db/src/lib.rs @@ -1812,7 +1812,7 @@ impl MemoryDb { pub async fn upsert_daily_summary(&self, summary: &DailySummary) -> anyhow::Result<()> { self.conn .execute( - "INSERT OR IGNORE INTO daily (date, content, summary, updated_at) VALUES (?1, ?2, ?3, ?4)", + "INSERT INTO daily (date, content, summary, updated_at) VALUES (?1, ?2, ?3, ?4) ON CONFLICT(date) DO UPDATE SET summary = excluded.summary, updated_at = excluded.updated_at", params![ summary.date.clone(), summary.content.clone(),