From e590809d88444be7c8086b5aebd3b279fde15f95 Mon Sep 17 00:00:00 2001 From: Richard Hightower Date: Fri, 10 Apr 2026 19:16:07 +0000 Subject: [PATCH 1/2] feat(v3.0): wire API-based summarizer from config - Read provider/model/api_key_env from SummarizerSettings config - Construct ApiSummarizer (OpenAI or Anthropic) when API key available - Fallback to MockSummarizer with warning when no API key found - Add build_summarizer() function for testable summarizer construction - Add unit tests for provider selection and env var lookup - Update planning docs for v3.0 --- .planning/ROADMAP.md | 13 +- .planning/STATE.md | 11 ++ crates/memory-daemon/src/commands.rs | 201 ++++++++++++++++++++++++++- crates/memory-types/src/config.rs | 7 + 4 files changed, 228 insertions(+), 4 deletions(-) diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index 7aedc59..675e489 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -136,6 +136,16 @@ See: `.planning/milestones/v2.7-ROADMAP.md` +## v3.0 — Real Summarization (In Progress) + +- [ ] Phase 51: API Summarizer Wiring + - [x] Wire `ApiSummarizer` from daemon config (`build_summarizer` function) + - [x] Add `api_key_env` field to `SummarizerSettings` for flexible key sourcing + - [x] Provider-aware fallback: `MockSummarizer` + `warn!` when no API key present + - [x] Unit tests: provider → env var mapping, explicit key, env var lookup, fallback + +**Goal:** Ship real AI-powered TOC rollup summaries for all active users with API keys configured. + ## Progress | Milestone | Phases | Plans | Status | Shipped | @@ -149,7 +159,8 @@ See: `.planning/milestones/v2.7-ROADMAP.md` | v2.5 Semantic Dedup | 35-38 | 11/11 | Complete | 2026-03-10 | | v2.6 Cognitive Retrieval | 39-44 | 13/13 | Complete | 2026-03-16 | | v2.7 Multi-Runtime Portability | 45-50 | 11/11 | Complete | 2026-03-22 | +| v3.0 Real Summarization | 51+ | 1/? | In Progress | — | --- -*Updated: 2026-03-22 after v2.7 milestone complete* +*Updated: 2026-03-22 — v3.0 API Summarizer Wiring in progress* diff --git a/.planning/STATE.md b/.planning/STATE.md index d2cb12b..263eda7 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -28,6 +28,8 @@ See: .planning/PROJECT.md (updated 2026-03-22) Milestone v2.7 Multi-Runtime Portability — SHIPPED 2026-03-22 All 6 phases (45-50), 11 plans complete. +v3.0 planning in progress — API Summarizer Wiring feature branch open (feature/v3.0-api-summarizer-wiring). + ## Decisions - Installer written in Rust (new workspace crate `memory-installer`) @@ -62,6 +64,15 @@ All 6 phases (45-50), 11 plans complete. - [Phase 50]: Used CARGO_MANIFEST_DIR for reliable workspace root discovery in integration tests - [Phase 50]: Preserved memory-capture.sh for include_str! compile dependency in CopilotConverter +## v3.0 In Progress + +- **API Summarizer Wiring** (`feature/v3.0-api-summarizer-wiring`) + - Added `api_key_env` field to `SummarizerSettings` for customizable env var lookup + - Implemented `build_summarizer()` in `crates/memory-daemon/src/commands.rs` + - Reads `provider`/`model`/`api_key_env` from config; constructs `ApiSummarizer` when key available + - Falls back to `MockSummarizer` with `warn!` log when no API key found + - Added `env_var_for_provider()` helper and 7 unit tests covering provider selection and key resolution + ## Blockers - None diff --git a/crates/memory-daemon/src/commands.rs b/crates/memory-daemon/src/commands.rs index 87295d1..19076c6 100644 --- a/crates/memory-daemon/src/commands.rs +++ b/crates/memory-daemon/src/commands.rs @@ -30,7 +30,8 @@ use memory_service::pb::{ }; use memory_service::run_server_with_scheduler; use memory_storage::Storage; -use memory_toc::summarizer::MockSummarizer; +use memory_toc::summarizer::{ApiSummarizer, ApiSummarizerConfig, MockSummarizer}; +use memory_types::config::SummarizerSettings; use memory_types::dedup::InFlightBuffer; use memory_types::Settings; @@ -310,6 +311,91 @@ async fn register_prune_jobs(scheduler: &SchedulerService, db_path: &Path) -> Re Ok(()) } +/// Resolve the API key from settings, returning the key and the env var name that was used. +/// +/// Resolution order: +/// 1. `settings.api_key` (explicit value in config) +/// 2. Environment variable named by `settings.api_key_env` +/// 3. Provider-default env var (`OPENAI_API_KEY` / `ANTHROPIC_API_KEY`) +fn resolve_api_key(settings: &SummarizerSettings) -> (Option, &'static str) { + let default_env_var: &'static str = if settings.provider.to_lowercase() == "anthropic" { + "ANTHROPIC_API_KEY" + } else { + "OPENAI_API_KEY" + }; + + // If a custom env var name is provided, look it up dynamically. + // We store the resolved name separately so the returned &'static str is the default. + let key = if let Some(ref explicit_key) = settings.api_key { + Some(explicit_key.clone()) + } else { + let env_var = settings.api_key_env.as_deref().unwrap_or(default_env_var); + std::env::var(env_var).ok() + }; + + (key, default_env_var) +} + +/// Return the name of the env var that will be consulted for this settings block. +/// +/// Exported so tests can verify provider → env var mapping without side effects. +pub fn env_var_for_provider(settings: &SummarizerSettings) -> String { + settings.api_key_env.clone().unwrap_or_else(|| { + if settings.provider.to_lowercase() == "anthropic" { + "ANTHROPIC_API_KEY".to_string() + } else { + "OPENAI_API_KEY".to_string() + } + }) +} + +/// Build a [`Summarizer`] from `SummarizerSettings`. +/// +/// Resolution order for the API key: +/// 1. `settings.api_key` (explicit value in config) +/// 2. Environment variable named by `settings.api_key_env` +/// 3. Provider-default env var (`OPENAI_API_KEY` / `ANTHROPIC_API_KEY`) +/// +/// Falls back to [`MockSummarizer`] with a `warn!` when no key is found. +pub fn build_summarizer( + settings: &SummarizerSettings, +) -> Arc { + let env_var_name = env_var_for_provider(settings); + let (api_key, _) = resolve_api_key(settings); + + match api_key { + Some(key) => { + let config = if settings.provider.to_lowercase() == "anthropic" { + ApiSummarizerConfig::claude(key, &settings.model) + } else { + ApiSummarizerConfig::openai(key, &settings.model) + }; + + match ApiSummarizer::new(config) { + Ok(s) => { + info!( + provider = %settings.provider, + model = %settings.model, + "Using API summarizer" + ); + Arc::new(s) + } + Err(e) => { + warn!(error = %e, "Failed to create ApiSummarizer, falling back to mock"); + Arc::new(MockSummarizer::new()) + } + } + } + None => { + warn!( + env_var = env_var_name, + "No API key found for summarizer, using mock" + ); + Arc::new(MockSummarizer::new()) + } + } +} + /// Start the memory daemon. /// /// 1. Load configuration (CFG-01: defaults -> file -> env -> CLI) @@ -380,8 +466,7 @@ pub async fn start_daemon( .context("Failed to create scheduler")?; // Create summarizer for rollup jobs - // TODO: Load from config - use ApiSummarizer if OPENAI_API_KEY or ANTHROPIC_API_KEY set - let summarizer: Arc = Arc::new(MockSummarizer::new()); + let summarizer = build_summarizer(&settings.summarizer); // Register rollup jobs (day/week/month) create_rollup_jobs( @@ -3081,4 +3166,114 @@ mod tests { let s = format_utc_date(1707350400000); assert_eq!(s, "2024-02-08"); } + + // ── build_summarizer / env_var_for_provider ────────────────────────────── + + /// OpenAI provider maps to OPENAI_API_KEY by default. + #[test] + fn test_env_var_for_provider_openai_default() { + let settings = SummarizerSettings { + provider: "openai".to_string(), + ..SummarizerSettings::default() + }; + assert_eq!(env_var_for_provider(&settings), "OPENAI_API_KEY"); + } + + /// Anthropic provider maps to ANTHROPIC_API_KEY by default. + #[test] + fn test_env_var_for_provider_anthropic_default() { + let settings = SummarizerSettings { + provider: "anthropic".to_string(), + ..SummarizerSettings::default() + }; + assert_eq!(env_var_for_provider(&settings), "ANTHROPIC_API_KEY"); + } + + /// Provider matching is case-insensitive. + #[test] + fn test_env_var_for_provider_case_insensitive() { + let settings = SummarizerSettings { + provider: "Anthropic".to_string(), + ..SummarizerSettings::default() + }; + assert_eq!(env_var_for_provider(&settings), "ANTHROPIC_API_KEY"); + } + + /// A custom `api_key_env` overrides the default env var name. + #[test] + fn test_env_var_for_provider_custom_override() { + let settings = SummarizerSettings { + provider: "openai".to_string(), + api_key_env: Some("MY_CUSTOM_KEY".to_string()), + ..SummarizerSettings::default() + }; + assert_eq!(env_var_for_provider(&settings), "MY_CUSTOM_KEY"); + } + + /// When no API key is available, `build_summarizer` returns a valid + /// (mock) summarizer without panicking. + #[test] + fn test_build_summarizer_falls_back_to_mock_when_no_key() { + // Use a deliberately obscure env var name that will not be set. + let settings = SummarizerSettings { + provider: "openai".to_string(), + api_key_env: Some("__AGENT_MEMORY_TEST_NONEXISTENT_KEY__".to_string()), + api_key: None, + ..SummarizerSettings::default() + }; + // Should not panic; returns an Arc (the mock). + let summarizer = build_summarizer(&settings); + // The Arc is non-null — that's the only observable property without downcasting. + let _ = summarizer; + } + + /// When `settings.api_key` is set explicitly, `build_summarizer` constructs + /// an ApiSummarizer (no env var lookup needed). + #[test] + fn test_build_summarizer_uses_explicit_api_key() { + let settings = SummarizerSettings { + provider: "openai".to_string(), + api_key: Some("sk-test-explicit-key".to_string()), + api_key_env: None, + ..SummarizerSettings::default() + }; + // Should succeed and return a valid Arc without panicking. + let summarizer = build_summarizer(&settings); + let _ = summarizer; + } + + /// When `OPENAI_API_KEY` env var is set, `build_summarizer` constructs + /// an ApiSummarizer for the openai provider. + #[test] + fn test_build_summarizer_reads_openai_api_key_env() { + // Use a scoped env var so we don't pollute other tests. + // SAFETY: test-only, single-threaded env mutation. + std::env::set_var("__TEST_OPENAI_KEY_WIRING__", "sk-test-key"); + let settings = SummarizerSettings { + provider: "openai".to_string(), + api_key_env: Some("__TEST_OPENAI_KEY_WIRING__".to_string()), + api_key: None, + ..SummarizerSettings::default() + }; + let summarizer = build_summarizer(&settings); + let _ = summarizer; + std::env::remove_var("__TEST_OPENAI_KEY_WIRING__"); + } + + /// When `ANTHROPIC_API_KEY` env var is set, `build_summarizer` constructs + /// an ApiSummarizer for the anthropic provider. + #[test] + fn test_build_summarizer_reads_anthropic_api_key_env() { + std::env::set_var("__TEST_ANTHROPIC_KEY_WIRING__", "sk-ant-test-key"); + let settings = SummarizerSettings { + provider: "anthropic".to_string(), + api_key_env: Some("__TEST_ANTHROPIC_KEY_WIRING__".to_string()), + api_key: None, + model: "claude-3-haiku-20240307".to_string(), + ..SummarizerSettings::default() + }; + let summarizer = build_summarizer(&settings); + let _ = summarizer; + std::env::remove_var("__TEST_ANTHROPIC_KEY_WIRING__"); + } } diff --git a/crates/memory-types/src/config.rs b/crates/memory-types/src/config.rs index 3da7249..7ffb386 100644 --- a/crates/memory-types/src/config.rs +++ b/crates/memory-types/src/config.rs @@ -203,6 +203,12 @@ pub struct SummarizerSettings { /// API base URL (for custom endpoints) #[serde(default)] pub api_base_url: Option, + + /// Name of the environment variable to read the API key from. + /// If unset, defaults to "OPENAI_API_KEY" for openai and + /// "ANTHROPIC_API_KEY" for anthropic. + #[serde(default)] + pub api_key_env: Option, } fn default_summarizer_provider() -> String { @@ -220,6 +226,7 @@ impl Default for SummarizerSettings { model: default_summarizer_model(), api_key: None, api_base_url: None, + api_key_env: None, } } } From 59e63ce742f9647b556fff45e0f7c21d85409ad4 Mon Sep 17 00:00:00 2001 From: Richard Hightower Date: Fri, 10 Apr 2026 19:27:02 +0000 Subject: [PATCH 2/2] fix(docs): use full path for Summarizer doc link in commands.rs --- crates/memory-daemon/src/commands.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/memory-daemon/src/commands.rs b/crates/memory-daemon/src/commands.rs index 19076c6..0df5ff8 100644 --- a/crates/memory-daemon/src/commands.rs +++ b/crates/memory-daemon/src/commands.rs @@ -349,7 +349,7 @@ pub fn env_var_for_provider(settings: &SummarizerSettings) -> String { }) } -/// Build a [`Summarizer`] from `SummarizerSettings`. +/// Build a [`memory_toc::summarizer::Summarizer`] from `SummarizerSettings`. /// /// Resolution order for the API key: /// 1. `settings.api_key` (explicit value in config)