From e2726d7b09450fb962d4dcd7afb1ea79bb087dda Mon Sep 17 00:00:00 2001 From: Ezra Lim Date: Fri, 3 Jul 2026 08:13:13 +0000 Subject: [PATCH 1/2] feat(knowledge): implement knowledge store layer (spec 02) Add knowledge management layer with in-memory caching: - KnowledgeStore with RwLock-based caching for thread safety - SiteConfigManager with defaults for popular recipe sites - UserModelManager for preference learning from recipe interactions - PatternMatcher for success/anti-pattern detection - KnowledgeContextAssembler for LLM prompt injection All 135 tests pass, including 58 new knowledge module tests. --- src/knowledge/context.rs | 385 +++++++++++++++++++++++++++++ src/knowledge/mod.rs | 32 +++ src/knowledge/patterns.rs | 442 ++++++++++++++++++++++++++++++++++ src/knowledge/site_config.rs | 453 +++++++++++++++++++++++++++++++++++ src/knowledge/store.rs | 375 +++++++++++++++++++++++++++++ src/knowledge/user_model.rs | 434 +++++++++++++++++++++++++++++++++ src/lib.rs | 1 + src/models/knowledge.rs | 2 +- 8 files changed, 2123 insertions(+), 1 deletion(-) create mode 100644 src/knowledge/context.rs create mode 100644 src/knowledge/mod.rs create mode 100644 src/knowledge/patterns.rs create mode 100644 src/knowledge/site_config.rs create mode 100644 src/knowledge/store.rs create mode 100644 src/knowledge/user_model.rs diff --git a/src/knowledge/context.rs b/src/knowledge/context.rs new file mode 100644 index 0000000..9a62526 --- /dev/null +++ b/src/knowledge/context.rs @@ -0,0 +1,385 @@ +//! Knowledge context assembly for LLM prompt injection. +//! +//! Assembles relevant knowledge (site configs, patterns, user preferences) +//! into a format suitable for injection into LLM system prompts, +//! respecting token budgets. + +use crate::models::agent_state::KnowledgeContext; +use crate::models::knowledge::{Patterns, SiteConfig, UserModel}; + +use super::store::KnowledgeStore; + +/// Default token budget for knowledge context. +pub const DEFAULT_TOKEN_BUDGET: u64 = 8000; + +/// Minimum token budget (to avoid empty context). +pub const MIN_TOKEN_BUDGET: u64 = 500; + +/// Estimated characters per token (rough approximation). +pub const CHARS_PER_TOKEN: usize = 4; + +/// Assembler for knowledge context injection. +pub struct KnowledgeContextAssembler; + +impl KnowledgeContextAssembler { + /// Build knowledge context for LLM injection. + /// + /// Assembles site config, patterns, and user preferences + /// within the specified token budget. + pub fn build( + store: &KnowledgeStore, + domain: Option<&str>, + token_budget: u64, + ) -> crate::storage::Result { + let token_budget = token_budget.clamp(MIN_TOKEN_BUDGET, DEFAULT_TOKEN_BUDGET); + let mut ctx = KnowledgeContext { + token_budget, + ..Default::default() + }; + + // Load site config if domain specified + if let Some(d) = domain { + let config = store.get_site_config(d)?; + if let Some(json) = self::try_add_to_budget(&mut ctx, &config, token_budget) { + ctx.site_configs.push(json); + } + } + + // Load patterns + let patterns = store.get_patterns()?; + let filtered_patterns = self::filter_patterns_for_domain(&patterns, domain); + if (!filtered_patterns.success_patterns.is_empty() + || !filtered_patterns.anti_patterns.is_empty()) + && let Some(json) = self::try_add_to_budget(&mut ctx, &filtered_patterns, token_budget) + { + ctx.patterns.push(json); + } + + // Load user model + let user_model = store.get_user_model("default")?; + if user_model.sample_size > 0 + && let Some(json) = self::try_add_to_budget(&mut ctx, &user_model, token_budget) + { + ctx.user_model = Some(json); + } + + // Calculate estimated tokens + ctx.estimated_tokens = Self::estimate_tokens(&ctx); + + Ok(ctx) + } + + /// Serialize context to text for prompt injection. + /// + /// Creates a human-readable format suitable for system prompts. + pub fn to_prompt_text(ctx: &KnowledgeContext) -> String { + let mut text = String::new(); + + text.push_str("# Learned Knowledge\n\n"); + + // Site configurations + if !ctx.site_configs.is_empty() { + text.push_str("## Site Configurations\n\n"); + for (i, config_json) in ctx.site_configs.iter().enumerate() { + if let Ok(config) = serde_json::from_str::(config_json) { + text.push_str(&format!("### Site: {}\n", config.domain)); + text.push_str(&format!("- Preferred method: {:?}\n", config.preferred_method)); + text.push_str(&format!("- Rate limit: {}ms\n", config.rate_limit_ms)); + if !config.skip_patterns.is_empty() { + text.push_str(&format!("- Skip patterns: {}\n", config.skip_patterns.join(", "))); + } + if i < ctx.site_configs.len() - 1 { + text.push('\n'); + } + } + } + text.push('\n'); + } + + // Patterns + if !ctx.patterns.is_empty() { + text.push_str("## Known Patterns\n\n"); + for pattern_json in &ctx.patterns { + if let Ok(patterns) = serde_json::from_str::(pattern_json) { + if !patterns.success_patterns.is_empty() { + text.push_str("### Success Patterns\n"); + for p in &patterns.success_patterns { + text.push_str(&format!("- {} (confidence: {:.0}%)\n", + p.description, p.confidence * 100.0)); + } + text.push('\n'); + } + if !patterns.anti_patterns.is_empty() { + text.push_str("### Anti-Patterns (avoid)\n"); + for p in &patterns.anti_patterns { + text.push_str(&format!("- {} (action: {:?})\n", + p.description, p.action)); + } + text.push('\n'); + } + } + } + } + + // User preferences + if let Some(ref model_json) = ctx.user_model + && let Ok(model) = serde_json::from_str::(model_json) + { + text.push_str("## User Preferences\n\n"); + if let Some(max_prep) = model.max_prep_time_minutes { + text.push_str(&format!("- Max prep time: {} minutes\n", max_prep)); + } + if let Some(max_total) = model.max_total_time_minutes { + text.push_str(&format!("- Max total time: {} minutes\n", max_total)); + } + if let Some(ref diff) = model.preferred_difficulty { + text.push_str(&format!("- Preferred difficulty: {:?}\n", diff)); + } + if !model.dietary_restrictions.is_empty() { + text.push_str(&format!("- Tags: {}\n", model.dietary_restrictions.join(", "))); + } + if model.sample_size > 0 { + text.push_str(&format!("- (based on {} recipes)\n", model.sample_size)); + } + } + + text + } + + /// Estimate token count for context. + fn estimate_tokens(ctx: &KnowledgeContext) -> u64 { + let total_chars: usize = ctx.site_configs.iter().map(|s| s.len()).sum::() + + ctx.patterns.iter().map(|s| s.len()).sum::() + + ctx.user_model.as_ref().map(|s| s.len()).unwrap_or(0); + + (total_chars / CHARS_PER_TOKEN) as u64 + } +} + +/// Try to add a value to context if within budget. +/// +/// Returns Some(json) if successful, None if it would exceed budget. +fn try_add_to_budget( + ctx: &mut KnowledgeContext, + value: &T, + budget: u64, +) -> Option { + let json = serde_json::to_string(value).ok()?; + let json_tokens = (json.len() / CHARS_PER_TOKEN) as u64; + + let current_tokens = KnowledgeContextAssembler::estimate_tokens(ctx); + + if current_tokens + json_tokens <= budget { + Some(json) + } else { + None + } +} + +/// Filter patterns to only include those relevant to a domain. +fn filter_patterns_for_domain(patterns: &Patterns, domain: Option<&str>) -> Patterns { + // Include universal patterns (empty sites list) + // and patterns that apply to the specified domain + let success_patterns = patterns + .success_patterns + .iter() + .filter(|p| domain.is_none_or(|d| p.applies_to(d))) + .cloned() + .collect(); + + let anti_patterns = patterns + .anti_patterns + .iter() + .filter(|p| { + domain.is_none_or(|d| p.sites.is_empty() || p.sites.iter().any(|s| s == d)) + }) + .cloned() + .collect(); + + Patterns { + success_patterns, + anti_patterns, + version: patterns.version, + computed_at: patterns.computed_at, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::models::ParseMethod; + use crate::knowledge::patterns::PatternMatcher; + use std::sync::Arc; + use tempfile::tempdir; + + fn test_store() -> (KnowledgeStore, tempfile::TempDir) { + let dir = tempdir().unwrap(); + let storage = Arc::new(crate::storage::FileKnowledgeStore::open( + dir.path().join("knowledge"), + ).unwrap()); + (KnowledgeStore::new(storage), dir) + } + + #[test] + fn build_respects_token_budget() { + let (store, _dir) = test_store(); + + let ctx = KnowledgeContextAssembler::build(&store, None, 500).unwrap(); + assert!(ctx.estimated_tokens <= ctx.token_budget); + } + + #[test] + fn build_includes_site_config_for_domain() { + let (store, _dir) = test_store(); + + // Save a config + let mut config = crate::models::knowledge::SiteConfig::new("example.com"); + config.preferred_method = ParseMethod::Selectors; + store.update_site_config(&config).unwrap(); + + let ctx = KnowledgeContextAssembler::build(&store, Some("example.com"), DEFAULT_TOKEN_BUDGET).unwrap(); + + assert!(!ctx.site_configs.is_empty()); + } + + #[test] + fn build_includes_user_model() { + let (store, _dir) = test_store(); + + // Create user model with some data + let mut model = crate::models::knowledge::UserModel::default_user(); + model.sample_size = 5; + model.max_prep_time_minutes = Some(30); + store.update_user_model(&model).unwrap(); + + let ctx = KnowledgeContextAssembler::build(&store, None, DEFAULT_TOKEN_BUDGET).unwrap(); + + assert!(ctx.user_model.is_some()); + } + + #[test] + fn build_includes_patterns() { + let (store, _dir) = test_store(); + + // Use default patterns + let patterns = PatternMatcher::default_patterns(); + store.update_patterns(&patterns).unwrap(); + + let ctx = KnowledgeContextAssembler::build(&store, None, DEFAULT_TOKEN_BUDGET).unwrap(); + + assert!(!ctx.patterns.is_empty()); + } + + #[test] + fn to_prompt_text_formats_site_config() { + let mut ctx = KnowledgeContext::default(); + + let config = crate::models::knowledge::SiteConfig::new("test.com"); + ctx.site_configs.push(serde_json::to_string(&config).unwrap()); + + let text = KnowledgeContextAssembler::to_prompt_text(&ctx); + + assert!(text.contains("test.com")); + assert!(text.contains("Site Configurations")); + } + + #[test] + fn to_prompt_text_formats_user_model() { + let mut ctx = KnowledgeContext::default(); + + let model = crate::models::knowledge::UserModel { + user_id: "test".to_string(), + max_prep_time_minutes: Some(30), + sample_size: 10, + ..Default::default() + }; + ctx.user_model = Some(serde_json::to_string(&model).unwrap()); + + let text = KnowledgeContextAssembler::to_prompt_text(&ctx); + + assert!(text.contains("User Preferences")); + assert!(text.contains("Max prep time")); + } + + #[test] + fn to_prompt_text_formats_patterns() { + let mut ctx = KnowledgeContext::default(); + + let mut patterns = Patterns::default(); + patterns.success_patterns.push(crate::models::knowledge::SuccessPattern { + description: "Test pattern".to_string(), + sites: vec![], + success_rate: 0.9, + sample_size: 100, + confidence: 0.85, + }); + ctx.patterns.push(serde_json::to_string(&patterns).unwrap()); + + let text = KnowledgeContextAssembler::to_prompt_text(&ctx); + + assert!(text.contains("Test pattern")); + assert!(text.contains("Success Patterns")); + } + + #[test] + fn to_prompt_text_empty_context() { + let ctx = KnowledgeContext::default(); + let text = KnowledgeContextAssembler::to_prompt_text(&ctx); + + assert!(text.contains("Learned Knowledge")); + } + + #[test] + fn estimate_tokens_reasonable() { + let mut ctx = KnowledgeContext::default(); + + let config = crate::models::knowledge::SiteConfig::new("example.com"); + ctx.site_configs.push(serde_json::to_string(&config).unwrap()); + + let tokens = KnowledgeContextAssembler::estimate_tokens(&ctx); + + // Should be roughly the JSON length / 4 + assert!(tokens > 0); + assert!(tokens < 1000); // Single config shouldn't be huge + } + + #[test] + fn filter_patterns_keeps_universal() { + let patterns = PatternMatcher::default_patterns(); + let filtered = filter_patterns_for_domain(&patterns, Some("unknown-site.com")); + + // Should include patterns with empty sites list + assert!(!filtered.success_patterns.is_empty()); + } + + #[test] + fn filter_patterns_keeps_domain_specific() { + let mut patterns = Patterns::default(); + patterns.success_patterns.push(crate::models::knowledge::SuccessPattern { + description: "Site-specific".to_string(), + sites: vec!["example.com".to_string()], + success_rate: 0.9, + sample_size: 50, + confidence: 0.8, + }); + + let filtered = filter_patterns_for_domain(&patterns, Some("example.com")); + assert_eq!(filtered.success_patterns.len(), 1); + + let filtered_other = filter_patterns_for_domain(&patterns, Some("other.com")); + assert_eq!(filtered_other.success_patterns.len(), 0); + } + + #[test] + fn build_with_no_domain_includes_universal_patterns() { + let (store, _dir) = test_store(); + + let patterns = PatternMatcher::default_patterns(); + store.update_patterns(&patterns).unwrap(); + + let ctx = KnowledgeContextAssembler::build(&store, None, DEFAULT_TOKEN_BUDGET).unwrap(); + + // Should include universal patterns + assert!(!ctx.patterns.is_empty()); + } +} diff --git a/src/knowledge/mod.rs b/src/knowledge/mod.rs new file mode 100644 index 0000000..99b4a74 --- /dev/null +++ b/src/knowledge/mod.rs @@ -0,0 +1,32 @@ +//! Knowledge management layer for the recipe scraping agent. +//! +//! This module provides read/write access to learned knowledge: +//! - Site-specific parsing configurations +//! - User preference models +//! - Discovered patterns (success and anti-patterns) +//! +//! # Architecture +//! +//! The knowledge layer sits between the storage layer and tools: +//! - Caches frequently accessed data in memory +//! - Validates changes before persisting +//! - Assembles knowledge for LLM prompt injection +//! +//! # Thread Safety +//! +//! `KnowledgeStore` uses `RwLock` for caching: +//! - Multiple readers don't block each other +//! - Writers are rare (only on learning updates) +//! - Cache invalidation happens on writes + +pub mod context; +pub mod patterns; +pub mod site_config; +pub mod store; +pub mod user_model; + +pub use context::KnowledgeContextAssembler; +pub use patterns::PatternMatcher; +pub use site_config::{SiteConfigManager, defaults}; +pub use store::KnowledgeStore; +pub use user_model::UserModelManager; diff --git a/src/knowledge/patterns.rs b/src/knowledge/patterns.rs new file mode 100644 index 0000000..b670851 --- /dev/null +++ b/src/knowledge/patterns.rs @@ -0,0 +1,442 @@ +//! Pattern discovery and matching. +//! +//! Provides utilities for matching success patterns and anti-patterns +//! to guide scraping behavior. + +use crate::models::knowledge::{AntiPattern, AntiPatternAction, Patterns, SuccessPattern}; +use crate::models::ParseMethod; + +/// Matcher for discovered patterns. +pub struct PatternMatcher; + +impl PatternMatcher { + /// Check if a URL matches any anti-pattern for a domain. + /// + /// Returns the matching anti-pattern if found, or None. + pub fn matches_anti_pattern<'a>( + patterns: &'a Patterns, + url: &str, + domain: &str, + ) -> Option<&'a AntiPattern> { + patterns + .anti_patterns + .iter() + .find(|ap| ap.matches(url, domain)) + } + + /// Get success patterns that apply to a domain. + /// + /// Returns patterns where the domain matches or patterns with no domain restriction. + pub fn get_success_patterns<'a>(patterns: &'a Patterns, domain: &str) -> Vec<&'a SuccessPattern> { + patterns + .success_patterns + .iter() + .filter(|p| p.applies_to(domain)) + .collect() + } + + /// Get success patterns for a domain and parse method. + pub fn get_success_patterns_for_method<'a>( + patterns: &'a Patterns, + domain: &str, + method: ParseMethod, + ) -> Vec<&'a SuccessPattern> { + patterns + .success_patterns + .iter() + .filter(|p| p.applies_to(domain)) + .filter(|p| { + // Include patterns that mention this method in description + let desc = p.description.to_lowercase(); + let method_str = format!("{:?}", method).to_lowercase(); + desc.contains(&method_str) + }) + .collect() + } + + /// Check if a URL should be skipped based on anti-patterns. + pub fn should_skip_url(patterns: &Patterns, url: &str, domain: &str) -> bool { + Self::matches_anti_pattern(patterns, url, domain) + .map(|ap| ap.action == AntiPatternAction::SkipUrl) + .unwrap_or(false) + } + + /// Get the action for a matched anti-pattern. + pub fn get_anti_pattern_action( + patterns: &Patterns, + url: &str, + domain: &str, + ) -> Option { + Self::matches_anti_pattern(patterns, url, domain) + .map(|ap| ap.action) + } + + /// Default built-in patterns for common cases. + /// + /// These provide baseline patterns that work across many recipe sites. + pub fn default_patterns() -> Patterns { + let mut patterns = Patterns::default(); + + // Success patterns + patterns.success_patterns.extend(vec![ + SuccessPattern { + description: "Schema.org JSON-LD parsing works reliably".to_string(), + sites: vec![], // Applies to all sites + success_rate: 0.85, + sample_size: 1000, + confidence: 0.9, + }, + SuccessPattern { + description: "Recipes with /recipe/ in URL parse successfully".to_string(), + sites: vec![], + success_rate: 0.75, + sample_size: 500, + confidence: 0.7, + }, + ]); + + // Anti-patterns + patterns.anti_patterns.extend(vec![ + AntiPattern { + description: "URL ends with /video - video content not a recipe".to_string(), + sites: vec!["tasty.co".to_string()], + failure_rate: 0.95, + action: AntiPatternAction::SkipUrl, + sample_size: 50, + confidence: 0.85, + }, + AntiPattern { + description: "URL contains /gallery/ - slideshow format".to_string(), + sites: vec!["allrecipes.com".to_string()], + failure_rate: 0.80, + action: AntiPatternAction::SkipUrl, + sample_size: 30, + confidence: 0.75, + }, + AntiPattern { + description: "URL contains /videos/ - video content".to_string(), + sites: vec!["foodnetwork.com".to_string()], + failure_rate: 0.90, + action: AntiPatternAction::SkipUrl, + sample_size: 40, + confidence: 0.80, + }, + AntiPattern { + description: "Rate limit detected - slow down requests".to_string(), + sites: vec![], + failure_rate: 0.60, + action: AntiPatternAction::SlowDown, + sample_size: 100, + confidence: 0.85, + }, + ]); + + patterns.version = 1; + patterns + } + + /// Add a new success pattern. + pub fn add_success_pattern(patterns: &mut Patterns, pattern: SuccessPattern) { + // Check for duplicates + let is_duplicate = patterns.success_patterns.iter().any(|p| { + p.description == pattern.description && p.sites == pattern.sites + }); + + if !is_duplicate { + patterns.success_patterns.push(pattern); + patterns.version += 1; + } + } + + /// Add a new anti-pattern. + pub fn add_anti_pattern(patterns: &mut Patterns, pattern: AntiPattern) { + // Check for duplicates + let is_duplicate = patterns.anti_patterns.iter().any(|p| { + p.description == pattern.description && p.sites == pattern.sites + }); + + if !is_duplicate { + patterns.anti_patterns.push(pattern); + patterns.version += 1; + } + } + + /// Remove patterns below confidence threshold. + pub fn prune_low_confidence(patterns: &mut Patterns, min_confidence: f64) -> usize { + let before_count = patterns.success_patterns.len() + patterns.anti_patterns.len(); + + patterns + .success_patterns + .retain(|p| p.confidence >= min_confidence); + patterns + .anti_patterns + .retain(|p| p.confidence >= min_confidence); + + let after_count = patterns.success_patterns.len() + patterns.anti_patterns.len(); + before_count - after_count + } + + /// Sort patterns by confidence (highest first). + pub fn sort_by_confidence(patterns: &mut Patterns) { + patterns + .success_patterns + .sort_by(|a, b| b.confidence.partial_cmp(&a.confidence).unwrap()); + patterns + .anti_patterns + .sort_by(|a, b| b.confidence.partial_cmp(&a.confidence).unwrap()); + } +} + +impl SuccessPattern { + /// Check if this pattern applies to a given domain. + pub fn applies_to(&self, domain: &str) -> bool { + // Empty sites list means applies to all + self.sites.is_empty() || self.sites.iter().any(|s| s == domain) + } +} + +impl AntiPattern { + /// Check if this anti-pattern matches a URL/domain combination. + pub fn matches(&self, url: &str, domain: &str) -> bool { + // Check if domain applies + let domain_matches = self.sites.is_empty() + || self.sites.iter().any(|s| s == domain); + + if !domain_matches { + return false; + } + + // Check if URL matches the pattern description + // Simple heuristic: look for keywords in the URL + let url_lower = url.to_lowercase(); + let desc_lower = self.description.to_lowercase(); + + // Extract potential URL patterns from description + // e.g., "URL ends with /video" -> check if url ends with /video + if desc_lower.contains("/video") && url_lower.contains("/video") { + return true; + } + if desc_lower.contains("/gallery") && url_lower.contains("/gallery") { + return true; + } + if desc_lower.contains("/videos") && url_lower.contains("/videos") { + return true; + } + if desc_lower.contains("/shows") && url_lower.contains("/shows") { + return true; + } + + false + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn matches_video_anti_pattern() { + let patterns = PatternMatcher::default_patterns(); + + let result = PatternMatcher::matches_anti_pattern( + &patterns, + "https://tasty.co/recipe/123/video", + "tasty.co", + ); + + assert!(result.is_some()); + let ap = result.unwrap(); + assert_eq!(ap.action, AntiPatternAction::SkipUrl); + } + + #[test] + fn does_not_match_recipe_url() { + let patterns = PatternMatcher::default_patterns(); + + let result = PatternMatcher::matches_anti_pattern( + &patterns, + "https://tasty.co/recipe/123", + "tasty.co", + ); + + assert!(result.is_none()); + } + + #[test] + fn does_not_match_different_domain() { + let patterns = PatternMatcher::default_patterns(); + + // Video URL on a different domain shouldn't match tasty.co pattern + let result = PatternMatcher::matches_anti_pattern( + &patterns, + "https://example.com/recipe/123/video", + "example.com", + ); + + // Only matches if there's a pattern for example.com + // The default patterns have site-specific ones, so this won't match + // But the rate limit pattern applies to all sites + // So we check it doesn't match the /video pattern specifically + if let Some(ap) = result { + assert_ne!(ap.description, "URL ends with /video - video content not a recipe"); + } + } + + #[test] + fn should_skip_video_urls() { + let patterns = PatternMatcher::default_patterns(); + + assert!(PatternMatcher::should_skip_url( + &patterns, + "https://tasty.co/recipe/123/video", + "tasty.co" + )); + + assert!(!PatternMatcher::should_skip_url( + &patterns, + "https://tasty.co/recipe/123", + "tasty.co" + )); + } + + #[test] + fn get_success_patterns_for_domain() { + let patterns = PatternMatcher::default_patterns(); + + let site_patterns = PatternMatcher::get_success_patterns(&patterns, "example.com"); + + // Should include patterns with empty sites list + assert!(!site_patterns.is_empty()); + } + + #[test] + fn success_pattern_applies_to_domain() { + let universal = SuccessPattern { + description: "Test".to_string(), + sites: vec![], + success_rate: 0.9, + sample_size: 100, + confidence: 0.8, + }; + + assert!(universal.applies_to("any-site.com")); + + let site_specific = SuccessPattern { + description: "Test".to_string(), + sites: vec!["allrecipes.com".to_string()], + success_rate: 0.9, + sample_size: 100, + confidence: 0.8, + }; + + assert!(site_specific.applies_to("allrecipes.com")); + assert!(!site_specific.applies_to("other.com")); + } + + #[test] + fn add_success_pattern_avoids_duplicates() { + let mut patterns = Patterns::default(); + let pattern = SuccessPattern { + description: "Test pattern".to_string(), + sites: vec!["example.com".to_string()], + success_rate: 0.9, + sample_size: 100, + confidence: 0.8, + }; + + PatternMatcher::add_success_pattern(&mut patterns, pattern.clone()); + assert_eq!(patterns.success_patterns.len(), 1); + + // Adding duplicate should not increase count + PatternMatcher::add_success_pattern(&mut patterns, pattern); + assert_eq!(patterns.success_patterns.len(), 1); + } + + #[test] + fn add_anti_pattern_avoids_duplicates() { + let mut patterns = Patterns::default(); + let pattern = AntiPattern { + description: "Test anti-pattern".to_string(), + sites: vec!["example.com".to_string()], + failure_rate: 0.9, + action: AntiPatternAction::SkipUrl, + sample_size: 50, + confidence: 0.8, + }; + + PatternMatcher::add_anti_pattern(&mut patterns, pattern.clone()); + assert_eq!(patterns.anti_patterns.len(), 1); + + // Adding duplicate should not increase count + PatternMatcher::add_anti_pattern(&mut patterns, pattern); + assert_eq!(patterns.anti_patterns.len(), 1); + } + + #[test] + fn prune_low_confidence() { + let mut patterns = Patterns::default(); + + patterns.success_patterns.push(SuccessPattern { + description: "High confidence".to_string(), + sites: vec![], + success_rate: 0.9, + sample_size: 100, + confidence: 0.9, + }); + + patterns.success_patterns.push(SuccessPattern { + description: "Low confidence".to_string(), + sites: vec![], + success_rate: 0.5, + sample_size: 5, + confidence: 0.3, + }); + + let removed = PatternMatcher::prune_low_confidence(&mut patterns, 0.5); + assert_eq!(removed, 1); + assert_eq!(patterns.success_patterns.len(), 1); + } + + #[test] + fn sort_by_confidence_orders_correctly() { + let mut patterns = Patterns::default(); + + patterns.success_patterns.push(SuccessPattern { + description: "Medium".to_string(), + sites: vec![], + success_rate: 0.7, + sample_size: 50, + confidence: 0.7, + }); + + patterns.success_patterns.push(SuccessPattern { + description: "High".to_string(), + sites: vec![], + success_rate: 0.9, + sample_size: 100, + confidence: 0.9, + }); + + patterns.success_patterns.push(SuccessPattern { + description: "Low".to_string(), + sites: vec![], + success_rate: 0.5, + sample_size: 30, + confidence: 0.5, + }); + + PatternMatcher::sort_by_confidence(&mut patterns); + + assert_eq!(patterns.success_patterns[0].confidence, 0.9); + assert_eq!(patterns.success_patterns[1].confidence, 0.7); + assert_eq!(patterns.success_patterns[2].confidence, 0.5); + } + + #[test] + fn default_patterns_not_empty() { + let patterns = PatternMatcher::default_patterns(); + + assert!(!patterns.success_patterns.is_empty()); + assert!(!patterns.anti_patterns.is_empty()); + } +} diff --git a/src/knowledge/site_config.rs b/src/knowledge/site_config.rs new file mode 100644 index 0000000..9304e1f --- /dev/null +++ b/src/knowledge/site_config.rs @@ -0,0 +1,453 @@ +//! Site configuration management with defaults. +//! +//! Provides site-specific parsing configurations with +//! validation and default configurations for known recipe sites. + +use crate::models::knowledge::{Selectors, SiteConfig, SiteStats}; +use crate::models::ParseMethod; +use crate::storage::StorageError; + +/// Manager for site-specific parsing configurations. +pub struct SiteConfigManager; + +impl SiteConfigManager { + /// Create a default configuration for a domain. + pub fn default_for_domain(domain: &str) -> SiteConfig { + // Check if we have a known default + if let Some(config) = defaults::get(domain) { + return config; + } + + // Generic default + SiteConfig { + domain: domain.to_string(), + preferred_method: ParseMethod::SchemaOrg, + selectors: Selectors::default(), + rate_limit_ms: 1000, + requires_js: false, + headers: std::collections::HashMap::new(), + skip_patterns: Vec::new(), + stats: SiteStats::default(), + updated_at: chrono::Utc::now(), + version: 0, + } + } + + /// Merge configuration with a fallback. + /// + /// Values present in self are kept; missing values use fallback. + pub fn merge_with_fallback(config: &SiteConfig, fallback: &SiteConfig) -> SiteConfig { + SiteConfig { + domain: config.domain.clone(), + preferred_method: if config.preferred_method == ParseMethod::SchemaOrg { + fallback.preferred_method + } else { + config.preferred_method + }, + selectors: merge_selectors(&config.selectors, &fallback.selectors), + rate_limit_ms: if config.rate_limit_ms == 1000 { + fallback.rate_limit_ms + } else { + config.rate_limit_ms + }, + requires_js: config.requires_js || fallback.requires_js, + headers: if config.headers.is_empty() { + fallback.headers.clone() + } else { + config.headers.clone() + }, + skip_patterns: if config.skip_patterns.is_empty() { + fallback.skip_patterns.clone() + } else { + config.skip_patterns.clone() + }, + stats: config.stats.clone(), + updated_at: chrono::Utc::now(), + version: config.version, + } + } + + /// Validate a site configuration. + pub fn validate(config: &SiteConfig) -> Result<(), StorageError> { + // Validate domain + if config.domain.is_empty() { + return Err(StorageError::InvalidPath("domain cannot be empty".into())); + } + + // Domain should not contain path separators + if config.domain.contains('/') || config.domain.contains('\\') { + return Err(StorageError::InvalidPath( + format!("invalid domain '{}': contains path separator", config.domain) + )); + } + + // Validate rate limit + if config.rate_limit_ms == 0 { + return Err(StorageError::InvalidPath( + "rate_limit_ms must be greater than 0".into() + )); + } + + // Validate selectors (if present) + validate_selectors(&config.selectors)?; + + Ok(()) + } + + /// Get selector for a field, falling back to default. + pub fn get_selector<'a>(config: &'a SiteConfig, field: &str) -> Option<&'a str> { + match field { + "title" => config.selectors.title.as_deref(), + "ingredients" => config.selectors.ingredients.as_deref(), + "instructions" => config.selectors.instructions.as_deref(), + "prep_time" => config.selectors.prep_time.as_deref(), + "cook_time" => config.selectors.cook_time.as_deref(), + "total_time" => config.selectors.total_time.as_deref(), + "servings" | "yield" => config.selectors.servings.as_deref(), + "author" => config.selectors.author.as_deref(), + "image" => config.selectors.image.as_deref(), + "description" => config.selectors.description.as_deref(), + _ => None, + } + } +} + +/// Merge two selectors, preferring self over fallback. +fn merge_selectors(self_sel: &Selectors, fallback: &Selectors) -> Selectors { + Selectors { + title: self_sel.title.as_ref().or(fallback.title.as_ref()).cloned(), + ingredients: self_sel.ingredients.as_ref().or(fallback.ingredients.as_ref()).cloned(), + instructions: self_sel.instructions.as_ref().or(fallback.instructions.as_ref()).cloned(), + prep_time: self_sel.prep_time.as_ref().or(fallback.prep_time.as_ref()).cloned(), + cook_time: self_sel.cook_time.as_ref().or(fallback.cook_time.as_ref()).cloned(), + total_time: self_sel.total_time.as_ref().or(fallback.total_time.as_ref()).cloned(), + servings: self_sel.servings.as_ref().or(fallback.servings.as_ref()).cloned(), + author: self_sel.author.as_ref().or(fallback.author.as_ref()).cloned(), + image: self_sel.image.as_ref().or(fallback.image.as_ref()).cloned(), + description: self_sel.description.as_ref().or(fallback.description.as_ref()).cloned(), + } +} + +/// Validate CSS selectors. +fn validate_selectors(selectors: &Selectors) -> Result<(), StorageError> { + // Basic validation: selectors should not be empty strings if present + for (name, value) in [ + ("title", &selectors.title), + ("ingredients", &selectors.ingredients), + ("instructions", &selectors.instructions), + ] { + if let Some(sel) = value + && sel.trim().is_empty() + { + return Err(StorageError::InvalidPath( + format!("selector '{}' cannot be empty string", name) + )); + } + } + + Ok(()) +} + +/// Default configurations for known recipe sites. +pub mod defaults { + use crate::models::knowledge::{Selectors, SiteConfig}; + use crate::models::ParseMethod; + use std::collections::HashMap; + + /// Get default config for a known domain, if available. + pub fn get(domain: &str) -> Option { + match domain { + "allrecipes.com" | "www.allrecipes.com" => Some(allrecipes()), + "foodnetwork.com" | "www.foodnetwork.com" => Some(foodnetwork()), + "tasty.co" | "www.tasty.co" => Some(tasty()), + "bettycrocker.com" | "www.bettycrocker.com" => Some(bettycrocker()), + "pillsbury.com" | "www.pillsbury.com" => Some(pillsbury()), + "bonappetit.com" | "www.bonappetit.com" => Some(bonappetit()), + "seriouseats.com" | "www.seriouseats.com" => Some(seriouseats()), + _ => None, + } + } + + /// All known site defaults. + pub fn all() -> Vec { + vec![ + allrecipes(), + foodnetwork(), + tasty(), + bettycrocker(), + pillsbury(), + bonappetit(), + seriouseats(), + ] + } + + fn allrecipes() -> SiteConfig { + let mut headers = HashMap::new(); + headers.insert("User-Agent".to_string(), "Mozilla/5.0".to_string()); + + SiteConfig { + domain: "allrecipes.com".to_string(), + preferred_method: ParseMethod::SchemaOrg, + selectors: Selectors { + title: Some("h1.article-heading".to_string()), + ingredients: Some("ul.mntl-structured-ingredients__list li".to_string()), + instructions: Some("ol.mntl-sc-block-group--OL li".to_string()), + prep_time: Some("div.mntl-recipe-block--time".to_string()), + author: Some("a.mntl-attributed-author__link".to_string()), + ..Default::default() + }, + rate_limit_ms: 1500, + requires_js: false, + headers, + skip_patterns: vec![ + "/gallery/".to_string(), + "/video/".to_string(), + ], + stats: Default::default(), + updated_at: chrono::Utc::now(), + version: 1, + } + } + + fn foodnetwork() -> SiteConfig { + SiteConfig { + domain: "foodnetwork.com".to_string(), + preferred_method: ParseMethod::SchemaOrg, + selectors: Selectors { + title: Some("h1.o-RecipeTitle".to_string()), + ingredients: Some("div.o-Ingredients__m-Body li".to_string()), + instructions: Some("div.o-Method__m-Body li".to_string()), + author: Some("span.o-Attribution__a-Name".to_string()), + ..Default::default() + }, + rate_limit_ms: 2000, + requires_js: false, + headers: HashMap::new(), + skip_patterns: vec![ + "/videos/".to_string(), + "/shows/".to_string(), + ], + stats: Default::default(), + updated_at: chrono::Utc::now(), + version: 1, + } + } + + fn tasty() -> SiteConfig { + SiteConfig { + domain: "tasty.co".to_string(), + preferred_method: ParseMethod::SchemaOrg, + selectors: Selectors { + title: Some("h1.recipe-name".to_string()), + ingredients: Some("ul.ingredient-list li".to_string()), + instructions: Some("ol.prep-steps li".to_string()), + ..Default::default() + }, + rate_limit_ms: 1500, + requires_js: true, // Tasty often needs JS for full content + headers: HashMap::new(), + skip_patterns: vec![ + "/video/".to_string(), + "/article/".to_string(), + ], + stats: Default::default(), + updated_at: chrono::Utc::now(), + version: 1, + } + } + + fn bettycrocker() -> SiteConfig { + SiteConfig { + domain: "bettycrocker.com".to_string(), + preferred_method: ParseMethod::SchemaOrg, + selectors: Selectors { + title: Some("h1.recipe-title".to_string()), + ingredients: Some("div.ingredients ul li".to_string()), + instructions: Some("div.directions ol li".to_string()), + ..Default::default() + }, + rate_limit_ms: 1500, + requires_js: false, + headers: HashMap::new(), + skip_patterns: vec!["/videos/".to_string()], + stats: Default::default(), + updated_at: chrono::Utc::now(), + version: 1, + } + } + + fn pillsbury() -> SiteConfig { + SiteConfig { + domain: "pillsbury.com".to_string(), + preferred_method: ParseMethod::SchemaOrg, + selectors: Selectors { + title: Some("h1.recipe-title".to_string()), + ingredients: Some("div.ingredients-section li".to_string()), + instructions: Some("div.directions-section li".to_string()), + ..Default::default() + }, + rate_limit_ms: 1500, + requires_js: false, + headers: HashMap::new(), + skip_patterns: vec!["/videos/".to_string()], + stats: Default::default(), + updated_at: chrono::Utc::now(), + version: 1, + } + } + + fn bonappetit() -> SiteConfig { + SiteConfig { + domain: "bonappetit.com".to_string(), + preferred_method: ParseMethod::SchemaOrg, + selectors: Selectors { + title: Some("h1.Hed".to_string()), + ingredients: Some("div.ingredients__group li".to_string()), + instructions: Some("div.directions ol li".to_string()), + ..Default::default() + }, + rate_limit_ms: 2500, + requires_js: true, + headers: HashMap::new(), + skip_patterns: vec![ + "/video/".to_string(), + "/gallery/".to_string(), + ], + stats: Default::default(), + updated_at: chrono::Utc::now(), + version: 1, + } + } + + fn seriouseats() -> SiteConfig { + SiteConfig { + domain: "seriouseats.com".to_string(), + preferred_method: ParseMethod::SchemaOrg, + selectors: Selectors { + title: Some("h1.heading__title".to_string()), + ingredients: Some("div.recipe-ingredients li".to_string()), + instructions: Some("div.recipe-instructions ol li".to_string()), + author: Some("a.author-name".to_string()), + ..Default::default() + }, + rate_limit_ms: 2000, + requires_js: false, + headers: HashMap::new(), + skip_patterns: vec!["/videos/".to_string()], + stats: Default::default(), + updated_at: chrono::Utc::now(), + version: 1, + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn default_for_known_domain() { + let config = SiteConfigManager::default_for_domain("allrecipes.com"); + assert_eq!(config.domain, "allrecipes.com"); + assert!(config.selectors.title.is_some()); + } + + #[test] + fn default_for_unknown_domain() { + let config = SiteConfigManager::default_for_domain("unknown-site.org"); + assert_eq!(config.domain, "unknown-site.org"); + assert_eq!(config.preferred_method, ParseMethod::SchemaOrg); + } + + #[test] + fn merge_keeps_self_values() { + let mut self_config = SiteConfig::new("test.com"); + self_config.rate_limit_ms = 3000; + + let fallback = SiteConfig::new("fallback.com"); + + let merged = SiteConfigManager::merge_with_fallback(&self_config, &fallback); + + assert_eq!(merged.rate_limit_ms, 3000); // Kept self value + assert_eq!(merged.domain, "test.com"); // Kept self domain + } + + #[test] + fn merge_uses_fallback_for_missing() { + let self_config = SiteConfig::new("test.com"); + + let mut fallback = SiteConfig::new("fallback.com"); + fallback.rate_limit_ms = 5000; + fallback.requires_js = true; + + let merged = SiteConfigManager::merge_with_fallback(&self_config, &fallback); + + assert_eq!(merged.rate_limit_ms, 5000); // Used fallback + assert!(merged.requires_js); // Used fallback + } + + #[test] + fn validate_accepts_valid_config() { + let config = SiteConfig::new("valid.com"); + assert!(SiteConfigManager::validate(&config).is_ok()); + } + + #[test] + fn validate_rejects_empty_domain() { + let config = SiteConfig::new(""); + assert!(SiteConfigManager::validate(&config).is_err()); + } + + #[test] + fn validate_rejects_domain_with_slash() { + let config = SiteConfig::new("example.com/path"); + assert!(SiteConfigManager::validate(&config).is_err()); + } + + #[test] + fn validate_rejects_zero_rate_limit() { + let mut config = SiteConfig::new("test.com"); + config.rate_limit_ms = 0; + assert!(SiteConfigManager::validate(&config).is_err()); + } + + #[test] + fn get_selector_returns_correct_field() { + let mut config = SiteConfig::new("test.com"); + config.selectors.title = Some("h1.title".to_string()); + config.selectors.ingredients = Some("ul.ingredients li".to_string()); + + assert_eq!( + SiteConfigManager::get_selector(&config, "title"), + Some("h1.title") + ); + assert_eq!( + SiteConfigManager::get_selector(&config, "ingredients"), + Some("ul.ingredients li") + ); + assert_eq!( + SiteConfigManager::get_selector(&config, "nonexistent"), + None + ); + } + + #[test] + fn defaults_module_provides_known_sites() { + let all_defaults = defaults::all(); + assert!(!all_defaults.is_empty()); + + // Should have configs for major recipe sites + let domains: Vec<&str> = all_defaults.iter().map(|c| c.domain.as_str()).collect(); + assert!(domains.contains(&"allrecipes.com")); + assert!(domains.contains(&"foodnetwork.com")); + } + + #[test] + fn defaults_get_by_domain() { + let config = defaults::get("allrecipes.com").unwrap(); + assert_eq!(config.domain, "allrecipes.com"); + + let config = defaults::get("unknown.com"); + assert!(config.is_none()); + } +} diff --git a/src/knowledge/store.rs b/src/knowledge/store.rs new file mode 100644 index 0000000..0a9f1d8 --- /dev/null +++ b/src/knowledge/store.rs @@ -0,0 +1,375 @@ +//! Main knowledge store with in-memory caching. +//! +//! Provides coordinated access to site configs, user models, and patterns +//! with automatic cache management. + +use std::collections::HashMap; +use std::sync::Arc; +use std::sync::RwLock; + +use chrono::{DateTime, Utc}; + +use crate::models::knowledge::{Patterns, SiteConfig, UserModel}; +use crate::storage::traits::KnowledgeStorage; +use crate::storage::StorageError; + +use super::context::KnowledgeContextAssembler; +use super::patterns::PatternMatcher; +use super::site_config::SiteConfigManager; +use super::user_model::UserModelManager; + +/// In-memory cache for knowledge data. +#[derive(Debug, Default)] +struct KnowledgeCache { + /// Cached site configs by domain. + site_configs: HashMap, + + /// Cached user models by user ID. + user_models: HashMap, + + /// Cached patterns (single instance). + patterns: Option, + + /// When cache was last refreshed. + last_refresh: Option>, +} + +/// Main knowledge store providing cached access to all knowledge. +/// +/// Thread-safe through `RwLock`. Multiple readers can access simultaneously; +/// writers (updates) are rare and block briefly. +pub struct KnowledgeStore { + /// Persistent storage backend. + storage: Arc, + + /// In-memory cache with read-write lock. + cache: RwLock, +} + +impl std::fmt::Debug for KnowledgeStore { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("KnowledgeStore") + .field("cache", &self.cache) + .finish_non_exhaustive() + } +} + +impl KnowledgeStore { + /// Create a new knowledge store with the given storage backend. + pub fn new(storage: Arc) -> Self { + Self { + storage, + cache: RwLock::new(KnowledgeCache::default()), + } + } + + /// Get site configuration for a domain. + /// + /// Returns from cache if available, otherwise loads from storage. + /// Falls back to default config if not found. + pub fn get_site_config(&self, domain: &str) -> crate::storage::Result { + // Try read from cache first + { + let cache = self.cache.read().map_err(|e| { + StorageError::Io(std::io::Error::other(format!("Cache lock error: {e}"))) + })?; + + if let Some(config) = cache.site_configs.get(domain) { + return Ok(config.clone()); + } + } + + // Not in cache, load from storage + let config = match self.storage.get_site_config(domain)? { + Some(c) => c, + None => SiteConfig::new(domain), + }; + + // Update cache + { + let mut cache = self.cache.write().map_err(|e| { + StorageError::Io(std::io::Error::other(format!("Cache lock error: {e}"))) + })?; + cache.site_configs.insert(domain.to_string(), config.clone()); + } + + Ok(config) + } + + /// Update site configuration. + /// + /// Validates before saving. Updates cache on success. + pub fn update_site_config(&self, config: &SiteConfig) -> crate::storage::Result<()> { + // Validate + SiteConfigManager::validate(config)?; + + // Persist + self.storage.save_site_config(config)?; + + // Update cache + { + let mut cache = self.cache.write().map_err(|e| { + StorageError::Io(std::io::Error::other(format!("Cache lock error: {e}"))) + })?; + cache.site_configs.insert(config.domain.clone(), config.clone()); + } + + Ok(()) + } + + /// Get user model for a user ID. + /// + /// Creates default model if not exists. + pub fn get_user_model(&self, user_id: &str) -> crate::storage::Result { + // Try read from cache first + { + let cache = self.cache.read().map_err(|e| { + StorageError::Io(std::io::Error::other(format!("Cache lock error: {e}"))) + })?; + + if let Some(model) = cache.user_models.get(user_id) { + return Ok(model.clone()); + } + } + + // Not in cache, load from storage + let model = match self.storage.get_user_model(user_id)? { + Some(m) => m, + None => UserModel::default_user(), + }; + + // Update cache + { + let mut cache = self.cache.write().map_err(|e| { + StorageError::Io(std::io::Error::other(format!("Cache lock error: {e}"))) + })?; + cache.user_models.insert(user_id.to_string(), model.clone()); + } + + Ok(model) + } + + /// Update user model. + /// + /// Updates confidence based on sample size. Updates cache on success. + pub fn update_user_model(&self, model: &UserModel) -> crate::storage::Result<()> { + // Update confidence before saving + let mut model = model.clone(); + UserModelManager::update_confidence(&mut model); + + // Persist + self.storage.save_user_model(&model)?; + + // Update cache + { + let mut cache = self.cache.write().map_err(|e| { + StorageError::Io(std::io::Error::other(format!("Cache lock error: {e}"))) + })?; + cache.user_models.insert(model.user_id.clone(), model); + } + + Ok(()) + } + + /// Get current patterns. + /// + /// Returns default if not exists. + pub fn get_patterns(&self) -> crate::storage::Result { + // Try read from cache first + { + let cache = self.cache.read().map_err(|e| { + StorageError::Io(std::io::Error::other(format!("Cache lock error: {e}"))) + })?; + + if let Some(ref patterns) = cache.patterns { + return Ok(patterns.clone()); + } + } + + // Not in cache, load from storage + let patterns = self.storage.get_patterns()?; + let patterns = if patterns.success_patterns.is_empty() && patterns.anti_patterns.is_empty() { + // Storage returned empty, check for defaults + let default_patterns = PatternMatcher::default_patterns(); + if !default_patterns.success_patterns.is_empty() || !default_patterns.anti_patterns.is_empty() { + default_patterns + } else { + patterns + } + } else { + patterns + }; + + // Update cache + { + let mut cache = self.cache.write().map_err(|e| { + StorageError::Io(std::io::Error::other(format!("Cache lock error: {e}"))) + })?; + cache.patterns = Some(patterns.clone()); + } + + Ok(patterns) + } + + /// Update patterns. + /// + /// Updates cache on success. + pub fn update_patterns(&self, patterns: &Patterns) -> crate::storage::Result<()> { + // Persist + self.storage.save_patterns(patterns)?; + + // Update cache + { + let mut cache = self.cache.write().map_err(|e| { + StorageError::Io(std::io::Error::other(format!("Cache lock error: {e}"))) + })?; + cache.patterns = Some(patterns.clone()); + } + + Ok(()) + } + + /// Load knowledge context for LLM prompt injection. + /// + /// Assembles relevant site config, patterns, and user preferences + /// within the specified token budget. + pub fn load_for_context( + &self, + domain: Option<&str>, + token_budget: u64, + ) -> crate::storage::Result { + KnowledgeContextAssembler::build(self, domain, token_budget) + } + + /// Invalidate all caches and reload from storage. + pub fn refresh(&self) -> crate::storage::Result<()> { + let mut cache = self.cache.write().map_err(|e| { + StorageError::Io(std::io::Error::other(format!("Cache lock error: {e}"))) + })?; + + cache.site_configs.clear(); + cache.user_models.clear(); + cache.patterns = None; + cache.last_refresh = Some(Utc::now()); + + Ok(()) + } + + /// Get the underlying storage backend (for advanced operations). + pub fn storage(&self) -> &dyn KnowledgeStorage { + self.storage.as_ref() + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::models::ParseMethod; + use tempfile::tempdir; + + // Helper to create a test knowledge store + fn test_store() -> (KnowledgeStore, tempfile::TempDir) { + let dir = tempdir().unwrap(); + let storage = Arc::new(crate::storage::FileKnowledgeStore::open( + dir.path().join("knowledge"), + ).unwrap()); + (KnowledgeStore::new(storage), dir) + } + + #[test] + fn get_site_config_returns_default_for_unknown() { + let (store, _dir) = test_store(); + + let config = store.get_site_config("unknown-site.com").unwrap(); + assert_eq!(config.domain, "unknown-site.com"); + assert_eq!(config.preferred_method, ParseMethod::SchemaOrg); + } + + #[test] + fn site_config_roundtrip() { + let (store, _dir) = test_store(); + + let mut config = SiteConfig::new("test.com"); + config.preferred_method = ParseMethod::Selectors; + config.rate_limit_ms = 2000; + + store.update_site_config(&config).unwrap(); + + let loaded = store.get_site_config("test.com").unwrap(); + assert_eq!(loaded.preferred_method, ParseMethod::Selectors); + assert_eq!(loaded.rate_limit_ms, 2000); + } + + #[test] + fn get_user_model_returns_default() { + let (store, _dir) = test_store(); + + let model = store.get_user_model("default").unwrap(); + assert_eq!(model.user_id, "default"); + assert_eq!(model.confidence, 0.0); + } + + #[test] + fn user_model_roundtrip() { + let (store, _dir) = test_store(); + + let mut model = UserModel::default_user(); + model.max_prep_time_minutes = Some(30); + model.preferred_difficulty = Some(crate::models::recipe::Difficulty::Easy); + model.sample_size = 10; + + store.update_user_model(&model).unwrap(); + + let loaded = store.get_user_model("default").unwrap(); + assert_eq!(loaded.max_prep_time_minutes, Some(30)); + assert_eq!(loaded.preferred_difficulty, Some(crate::models::recipe::Difficulty::Easy)); + } + + #[test] + fn patterns_roundtrip() { + let (store, _dir) = test_store(); + + let mut patterns = Patterns::default(); + patterns.success_patterns.push(crate::models::knowledge::SuccessPattern { + description: "Test pattern".to_string(), + sites: vec!["example.com".to_string()], + success_rate: 0.9, + sample_size: 50, + confidence: 0.85, + }); + + store.update_patterns(&patterns).unwrap(); + + let loaded = store.get_patterns().unwrap(); + assert_eq!(loaded.success_patterns.len(), 1); + assert_eq!(loaded.success_patterns[0].description, "Test pattern"); + } + + #[test] + fn refresh_clears_cache() { + let (store, _dir) = test_store(); + + // Load something into cache + let _ = store.get_site_config("example.com").unwrap(); + let _ = store.get_user_model("default").unwrap(); + + // Refresh + store.refresh().unwrap(); + + // Cache should be cleared (verified by checking last_refresh) + { + let cache = store.cache.read().unwrap(); + assert!(cache.last_refresh.is_some()); + assert!(cache.site_configs.is_empty()); + assert!(cache.user_models.is_empty()); + } + } + + #[test] + fn load_for_context_respects_budget() { + let (store, _dir) = test_store(); + + let ctx = store.load_for_context(Some("example.com"), 1000).unwrap(); + assert!(ctx.token_budget <= 1000); + } +} diff --git a/src/knowledge/user_model.rs b/src/knowledge/user_model.rs new file mode 100644 index 0000000..979aece --- /dev/null +++ b/src/knowledge/user_model.rs @@ -0,0 +1,434 @@ +//! User preference model management. +//! +//! Tracks learned user preferences from recipe interactions +//! (kept vs deleted recipes) to improve future recommendations. + +use crate::models::knowledge::UserModel; +use crate::models::recipe::{Difficulty, Recipe}; + +/// Manager for user preference models. +pub struct UserModelManager; + +impl UserModelManager { + /// Create a new empty user model. + pub fn create(user_id: &str) -> UserModel { + UserModel { + user_id: user_id.to_string(), + max_prep_time_minutes: None, + max_cook_time_minutes: None, + max_total_time_minutes: None, + require_quantities: false, + max_ingredients: None, + preferred_difficulty: None, + dietary_restrictions: Vec::new(), + sample_size: 0, + updated_at: chrono::Utc::now(), + confidence: 0.0, + } + } + + /// Record that a user kept (saved) a recipe. + /// + /// Updates preferences based on the recipe's characteristics. + /// Call this when a recipe is successfully scraped and saved. + pub fn record_recipe_kept(model: &mut UserModel, recipe: &Recipe) { + model.sample_size += 1; + + // Track time preferences + if let Some(prep) = recipe.prep_time_minutes { + Self::update_max_preference(&mut model.max_prep_time_minutes, prep); + } + if let Some(cook) = recipe.cook_time_minutes { + Self::update_max_preference(&mut model.max_cook_time_minutes, cook); + } + if let Some(total) = recipe.total_time_minutes { + Self::update_max_preference(&mut model.max_total_time_minutes, total); + } + + // Track ingredient count + let ingredient_count = recipe.ingredients.len() as u32; + Self::update_max_preference(&mut model.max_ingredients, ingredient_count); + + // Track difficulty preference + if let Some(difficulty) = &recipe.difficulty { + Self::update_difficulty_preference(model, difficulty); + } + + // Track dietary tags + for tag in &recipe.tags { + if !model.dietary_restrictions.contains(tag) { + // Only add if it appears multiple times (avoid noise) + // For simplicity, we add all unique tags seen + model.dietary_restrictions.push(tag.clone()); + } + } + + model.updated_at = chrono::Utc::now(); + } + + /// Record that a user deleted a recipe. + /// + /// Adjusts preferences to avoid similar recipes in the future. + /// Call this when a user explicitly removes a saved recipe. + pub fn record_recipe_deleted(model: &mut UserModel, recipe: &Recipe) { + // When a recipe is deleted, we become more restrictive + // This is a signal that something about the recipe was undesirable + + // Tighten time constraints + if let Some(prep) = recipe.prep_time_minutes { + Self::tighten_max_preference(&mut model.max_prep_time_minutes, prep); + } + if let Some(cook) = recipe.cook_time_minutes { + Self::tighten_max_preference(&mut model.max_cook_time_minutes, cook); + } + if let Some(total) = recipe.total_time_minutes { + Self::tighten_max_preference(&mut model.max_total_time_minutes, total); + } + + // Tighten ingredient count + let ingredient_count = recipe.ingredients.len() as u32; + Self::tighten_max_preference(&mut model.max_ingredients, ingredient_count); + + model.updated_at = chrono::Utc::now(); + } + + /// Update confidence based on sample size. + /// + /// Confidence increases logarithmically with more samples. + /// Call this before persisting the model. + pub fn update_confidence(model: &mut UserModel) { + // Confidence formula: log2(sample_size + 1) / 10 + // Reaches 1.0 at ~1000 samples + let sample_based_confidence = (model.sample_size as f64 + 1.0).log2() / 10.0; + model.confidence = sample_based_confidence.min(1.0); + } + + /// Check how well a recipe matches user preferences. + /// + /// Returns a score from 0.0 to 1.0, where 1.0 is a perfect match. + /// A score below 0.5 suggests the recipe likely won't be wanted. + pub fn matches_preferences(model: &UserModel, recipe: &Recipe) -> f64 { + if model.sample_size == 0 { + return 1.0; // No data, assume neutral + } + + let mut score = 1.0; + let mut factors = 0; + + // Check time constraints + if let Some(max_prep) = model.max_prep_time_minutes + && let Some(prep) = recipe.prep_time_minutes + { + factors += 1; + if prep > max_prep { + score *= 1.0 - ((prep - max_prep) as f64 / max_prep as f64).min(1.0); + } + } + + if let Some(max_cook) = model.max_cook_time_minutes + && let Some(cook) = recipe.cook_time_minutes + { + factors += 1; + if cook > max_cook { + score *= 1.0 - ((cook - max_cook) as f64 / max_cook as f64).min(1.0); + } + } + + if let Some(max_total) = model.max_total_time_minutes + && let Some(total) = recipe.total_time_minutes + { + factors += 1; + if total > max_total { + score *= 1.0 - ((total - max_total) as f64 / max_total as f64).min(1.0); + } + } + + // Check ingredient count + if let Some(max_ing) = model.max_ingredients { + let count = recipe.ingredients.len() as u32; + factors += 1; + if count > max_ing { + score *= 1.0 - ((count - max_ing) as f64 / max_ing as f64).min(1.0); + } + } + + // Check difficulty + if let Some(pref_diff) = &model.preferred_difficulty + && let Some(recipe_diff) = &recipe.difficulty + { + factors += 1; + let diff_score = difficulty_distance(pref_diff, recipe_diff); + score *= diff_score; + } + + if factors == 0 { 1.0 } else { score } + } + + /// Update a max preference value (relax constraint). + fn update_max_preference(current: &mut Option, new_value: u32) { + match current { + Some(max) => { + // Allow up to 50% more than the largest seen + *max = (*max).max(new_value); + } + None => { + *current = Some(new_value); + } + } + } + + /// Tighten a max preference value (more restrictive). + fn tighten_max_preference(current: &mut Option, rejected_value: u32) { + match current { + Some(max) => { + // Only tighten if the rejected value was at or below current max + if rejected_value <= *max { + // Reduce by 20%, but not below the rejected value + *max = ((*max as f64 * 0.8) as u32).max(1); + } + } + None => { + // Set a restrictive value based on what was rejected + *current = Some(((rejected_value as f64 * 0.8) as u32).max(1)); + } + } + } + + /// Update difficulty preference. + fn update_difficulty_preference(model: &mut UserModel, difficulty: &Difficulty) { + // Prefer the difficulty that appears most often + // For simplicity, track the most recent + model.preferred_difficulty = Some(*difficulty); + } +} + +/// Calculate distance between difficulties (0.0 to 1.0). +fn difficulty_distance(preferred: &Difficulty, actual: &Difficulty) -> f64 { + use Difficulty::*; + let pref_level: i32 = match preferred { + Easy => 0, + Medium => 1, + Hard => 2, + }; + let actual_level: i32 = match actual { + Easy => 0, + Medium => 1, + Hard => 2, + }; + + let diff = (pref_level - actual_level).abs(); + match diff { + 0 => 1.0, + 1 => 0.7, + _ => 0.4, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::models::recipe::{Ingredient, Servings}; + use url::Url; + + fn test_recipe() -> Recipe { + Recipe { + id: crate::models::recipe::RecipeId::generate(), + name: "Test Recipe".to_string(), + source_url: Url::parse("https://example.com/recipe").unwrap(), + source_domain: "example.com".to_string(), + ingredients: vec![ + Ingredient::from_raw("1 cup flour"), + Ingredient::from_raw("2 eggs"), + ], + instructions: vec!["Mix".to_string()], + prep_time_minutes: Some(15), + cook_time_minutes: Some(30), + total_time_minutes: Some(45), + servings: Some(Servings::single(4)), + cuisine: None, + difficulty: Some(Difficulty::Easy), + tags: vec!["quick".to_string()], + nutrition: None, + image_url: None, + author: None, + description: None, + scraped_at: chrono::Utc::now(), + content_hash: None, + meta: std::collections::HashMap::new(), + } + } + + #[test] + fn create_creates_empty_model() { + let model = UserModelManager::create("test-user"); + assert_eq!(model.user_id, "test-user"); + assert_eq!(model.sample_size, 0); + assert_eq!(model.confidence, 0.0); + } + + #[test] + fn record_recipe_kept_increments_sample_size() { + let mut model = UserModel::default_user(); + let recipe = test_recipe(); + + UserModelManager::record_recipe_kept(&mut model, &recipe); + + assert_eq!(model.sample_size, 1); + } + + #[test] + fn record_recipe_kept_updates_time_preferences() { + let mut model = UserModel::default_user(); + let recipe = test_recipe(); + + UserModelManager::record_recipe_kept(&mut model, &recipe); + + assert_eq!(model.max_prep_time_minutes, Some(15)); + assert_eq!(model.max_cook_time_minutes, Some(30)); + assert_eq!(model.max_total_time_minutes, Some(45)); + } + + #[test] + fn record_recipe_kept_updates_ingredient_count() { + let mut model = UserModel::default_user(); + let recipe = test_recipe(); + + UserModelManager::record_recipe_kept(&mut model, &recipe); + + assert_eq!(model.max_ingredients, Some(2)); + } + + #[test] + fn record_recipe_kept_tracks_difficulty() { + let mut model = UserModel::default_user(); + let recipe = test_recipe(); + + UserModelManager::record_recipe_kept(&mut model, &recipe); + + assert_eq!(model.preferred_difficulty, Some(Difficulty::Easy)); + } + + #[test] + fn record_recipe_kept_tracks_tags() { + let mut model = UserModel::default_user(); + let recipe = test_recipe(); + + UserModelManager::record_recipe_kept(&mut model, &recipe); + + assert!(model.dietary_restrictions.contains(&"quick".to_string())); + } + + #[test] + fn record_recipe_deleted_tightens_preferences() { + let mut model = UserModel::default_user(); + let recipe = test_recipe(); + + // First record a kept recipe + UserModelManager::record_recipe_kept(&mut model, &recipe); + + // Then delete a similar recipe + UserModelManager::record_recipe_deleted(&mut model, &recipe); + + // Preferences should be tighter + assert!(model.max_prep_time_minutes.unwrap() <= 15); + assert!(model.max_cook_time_minutes.unwrap() <= 30); + } + + #[test] + fn update_confidence_increases_with_samples() { + let mut model = UserModel::default_user(); + + model.sample_size = 0; + UserModelManager::update_confidence(&mut model); + assert!(model.confidence < 0.5); + + model.sample_size = 10; + UserModelManager::update_confidence(&mut model); + assert!(model.confidence > 0.3); + + model.sample_size = 100; + UserModelManager::update_confidence(&mut model); + assert!(model.confidence > 0.5); + + model.sample_size = 1023; // log2(1024) = 10, so confidence = 1.0 + UserModelManager::update_confidence(&mut model); + assert!(model.confidence >= 1.0); + } + + #[test] + fn matches_preferences_returns_high_for_matching_recipe() { + let mut model = UserModel::default_user(); + let recipe = test_recipe(); + + // Train on the same recipe + UserModelManager::record_recipe_kept(&mut model, &recipe); + UserModelManager::update_confidence(&mut model); + + // Should match well + let score = UserModelManager::matches_preferences(&model, &recipe); + assert!(score >= 0.8); + } + + #[test] + fn matches_preferences_returns_low_for_non_matching_recipe() { + let mut model = UserModel::default_user(); + let recipe = test_recipe(); + + // Train on the recipe + UserModelManager::record_recipe_kept(&mut model, &recipe); + UserModelManager::update_confidence(&mut model); + + // Create a very different recipe + let hard_recipe = Recipe { + difficulty: Some(Difficulty::Hard), + prep_time_minutes: Some(120), + cook_time_minutes: Some(180), + total_time_minutes: Some(300), + ingredients: vec![Ingredient::from_raw("many"); 20], + ..test_recipe() + }; + + let score = UserModelManager::matches_preferences(&model, &hard_recipe); + assert!(score < 0.7); + } + + #[test] + fn matches_preferences_returns_one_for_no_data() { + let model = UserModel::default_user(); + let recipe = test_recipe(); + + // No training data, should return neutral + let score = UserModelManager::matches_preferences(&model, &recipe); + assert_eq!(score, 1.0); + } + + #[test] + fn multiple_kept_recipes_expand_preferences() { + let mut model = UserModel::default_user(); + + // Keep recipes with increasing prep times + for prep_time in [10, 20, 30].iter() { + let mut recipe = test_recipe(); + recipe.prep_time_minutes = Some(*prep_time); + UserModelManager::record_recipe_kept(&mut model, &recipe); + } + + // Max should be the largest seen + assert_eq!(model.max_prep_time_minutes, Some(30)); + } + + #[test] + fn deleted_recipe_reduces_max() { + let mut model = UserModel::default_user(); + model.max_prep_time_minutes = Some(60); + + // Delete a recipe with 30 min prep time + let mut recipe = test_recipe(); + recipe.prep_time_minutes = Some(30); + + UserModelManager::record_recipe_deleted(&mut model, &recipe); + + // Max should be reduced (tightened) + assert!(model.max_prep_time_minutes.unwrap() < 60); + } +} diff --git a/src/lib.rs b/src/lib.rs index 948977b..eb9a2b1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -2,6 +2,7 @@ pub mod models; pub mod storage; +pub mod knowledge; pub use models::*; pub use storage::*; diff --git a/src/models/knowledge.rs b/src/models/knowledge.rs index 0c28146..d0d8922 100644 --- a/src/models/knowledge.rs +++ b/src/models/knowledge.rs @@ -151,7 +151,7 @@ impl SiteStats { // ============================================================================ /// Learned preferences for a user. -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize, Deserialize, Default)] pub struct UserModel { /// User identifier (or "default" for single-user mode). #[serde(default = "default_user_id")] From 30b70afd57483d57a9531d611f8f58e50e9a9510 Mon Sep 17 00:00:00 2001 From: Ezra Lim Date: Thu, 9 Jul 2026 01:47:46 +0000 Subject: [PATCH 2/2] fix: address critical issues from PR review Priority 1 - Critical Issues: - Fix race condition in cache access (TOCTOU vulnerability) - Fix division by zero in preference matching - Fix flawed merge logic in site config - Remove dead code suppression Priority 2 - Logic Bugs: - Improve pattern matching with proper path boundaries - Fix difficulty preference to track frequency not recency Priority 3 - Architecture: - Add TTL-based cache eviction (5-minute default) - Create MockKnowledgeStorage for faster unit tests Priority 4 - Code Quality: - Replace minimal User-Agent with realistic Chrome string - Add comprehensive documentation to magic numbers - Fix comment/code mismatches Tests: 155 passed (up from 142) Clippy: 0 warnings --- src/knowledge/context.rs | 37 +++++- src/knowledge/mock.rs | 220 ++++++++++++++++++++++++++++++++ src/knowledge/mod.rs | 6 + src/knowledge/patterns.rs | 82 ++++++++++-- src/knowledge/site_config.rs | 48 ++++--- src/knowledge/store.rs | 241 +++++++++++++++++++++++++++++++++-- src/knowledge/user_model.rs | 142 +++++++++++++++++---- src/models/knowledge.rs | 12 +- 8 files changed, 720 insertions(+), 68 deletions(-) create mode 100644 src/knowledge/mock.rs diff --git a/src/knowledge/context.rs b/src/knowledge/context.rs index 9a62526..32ca74f 100644 --- a/src/knowledge/context.rs +++ b/src/knowledge/context.rs @@ -10,12 +10,45 @@ use crate::models::knowledge::{Patterns, SiteConfig, UserModel}; use super::store::KnowledgeStore; /// Default token budget for knowledge context. +/// +/// This reserves tokens for learned knowledge in the system prompt, +/// leaving room for conversation history and response generation. +/// +/// Typical allocation for a 128k context window: +/// - System prompt: ~4k tokens +/// - Knowledge context: ~8k tokens (this constant) +/// - Conversation history: ~100k tokens +/// - Response generation: ~16k tokens +/// +/// Adjust based on: +/// - Model context window size +/// - Average knowledge size for your use case +/// - Required history depth pub const DEFAULT_TOKEN_BUDGET: u64 = 8000; -/// Minimum token budget (to avoid empty context). +/// Minimum tokens to allocate for knowledge context. +/// +/// Below this threshold, knowledge becomes too sparse to be useful. +/// Ensures at least some site config or patterns are included. +/// +/// With 500 tokens, we can typically include: +/// - 1 site config (~200 tokens), OR +/// - 2-3 patterns (~150 tokens), OR +/// - A minimal user model (~100 tokens) pub const MIN_TOKEN_BUDGET: u64 = 500; -/// Estimated characters per token (rough approximation). +/// Approximate characters per token for estimation. +/// +/// This is a rough approximation for token counting without calling +/// the model's tokenizer. Actual values vary by: +/// +/// - Content type: English text averages ~4 chars/token +/// - Code: often fewer chars per token (~3-3.5) +/// - Whitespace-heavy content: more chars per token +/// - Model-specific: GPT-4, Claude, Llama all differ slightly +/// +/// We use 4 as a conservative middle-ground for recipe-related text +/// which tends to be natural language with some structure. pub const CHARS_PER_TOKEN: usize = 4; /// Assembler for knowledge context injection. diff --git a/src/knowledge/mock.rs b/src/knowledge/mock.rs new file mode 100644 index 0000000..f992cd6 --- /dev/null +++ b/src/knowledge/mock.rs @@ -0,0 +1,220 @@ +//! Mock storage implementation for testing. +//! +//! Provides an in-memory mock that implements KnowledgeStorage trait, +//! allowing unit tests to run without filesystem dependencies. + +use std::collections::HashMap; +use std::sync::Mutex; + +use crate::models::agent_state::KnowledgeContext; +use crate::models::knowledge::{Patterns, SiteConfig, UserModel}; +use crate::storage::error::Result; +use crate::storage::traits::KnowledgeStorage; + +/// In-memory mock storage for testing. +pub struct MockKnowledgeStorage { + site_configs: Mutex>, + user_models: Mutex>, + patterns: Mutex>, +} + +impl MockKnowledgeStorage { + /// Create a new empty mock storage. + pub fn new() -> Self { + Self { + site_configs: Mutex::new(HashMap::new()), + user_models: Mutex::new(HashMap::new()), + patterns: Mutex::new(None), + } + } + + /// Create a mock storage pre-populated with test data. + pub fn with_test_data() -> Self { + let storage = Self::new(); + + // Add a test site config + let config = SiteConfig::new("test.example.com"); + storage.save_site_config(&config).ok(); + + // Add a test user model + let model = UserModel::default_user(); + storage.save_user_model(&model).ok(); + + storage + } + + /// Clear all stored data. + pub fn clear(&self) { + self.site_configs.lock().unwrap().clear(); + self.user_models.lock().unwrap().clear(); + *self.patterns.lock().unwrap() = None; + } +} + +impl Default for MockKnowledgeStorage { + fn default() -> Self { + Self::new() + } +} + +impl KnowledgeStorage for MockKnowledgeStorage { + fn get_site_config(&self, domain: &str) -> Result> { + Ok(self.site_configs.lock().unwrap().get(domain).cloned()) + } + + fn save_site_config(&self, config: &SiteConfig) -> Result<()> { + self.site_configs + .lock() + .unwrap() + .insert(config.domain.clone(), config.clone()); + Ok(()) + } + + fn list_site_configs(&self) -> Result> { + Ok(self.site_configs.lock().unwrap().keys().cloned().collect()) + } + + fn delete_site_config(&self, domain: &str) -> Result<()> { + self.site_configs.lock().unwrap().remove(domain); + Ok(()) + } + + fn get_user_model(&self, user_id: &str) -> Result> { + Ok(self.user_models.lock().unwrap().get(user_id).cloned()) + } + + fn save_user_model(&self, model: &UserModel) -> Result<()> { + self.user_models + .lock() + .unwrap() + .insert(model.user_id.clone(), model.clone()); + Ok(()) + } + + fn list_user_models(&self) -> Result> { + Ok(self.user_models.lock().unwrap().keys().cloned().collect()) + } + + fn get_patterns(&self) -> Result { + Ok(self + .patterns + .lock() + .unwrap() + .clone() + .unwrap_or_default()) + } + + fn save_patterns(&self, patterns: &Patterns) -> Result<()> { + *self.patterns.lock().unwrap() = Some(patterns.clone()); + Ok(()) + } + + fn load_for_context(&self, domain: Option<&str>) -> Result { + let mut ctx = KnowledgeContext::default(); + ctx.token_budget = 8000; + + if let Some(d) = domain { + if let Some(config) = self.get_site_config(d)? { + ctx.site_configs.push(serde_json::to_string(&config)?); + } + } + + let patterns = self.get_patterns()?; + if !patterns.success_patterns.is_empty() || !patterns.anti_patterns.is_empty() { + ctx.patterns.push(serde_json::to_string(&patterns)?); + } + + if let Some(model) = self.get_user_model("default")? { + ctx.user_model = Some(serde_json::to_string(&model)?); + } + + Ok(ctx) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn mock_storage_roundtrip() { + let storage = MockKnowledgeStorage::new(); + + let config = SiteConfig::new("example.com"); + storage.save_site_config(&config).unwrap(); + + let loaded = storage.get_site_config("example.com").unwrap().unwrap(); + assert_eq!(loaded.domain, "example.com"); + } + + #[test] + fn mock_storage_missing_returns_none() { + let storage = MockKnowledgeStorage::new(); + assert!(storage.get_site_config("nonexistent.com").unwrap().is_none()); + } + + #[test] + fn mock_storage_delete() { + let storage = MockKnowledgeStorage::new(); + + let config = SiteConfig::new("example.com"); + storage.save_site_config(&config).unwrap(); + assert!(storage.get_site_config("example.com").unwrap().is_some()); + + storage.delete_site_config("example.com").unwrap(); + assert!(storage.get_site_config("example.com").unwrap().is_none()); + } + + #[test] + fn mock_storage_user_model() { + let storage = MockKnowledgeStorage::new(); + + let mut model = UserModel::default_user(); + model.max_prep_time_minutes = Some(30); + storage.save_user_model(&model).unwrap(); + + let loaded = storage.get_user_model("default").unwrap().unwrap(); + assert_eq!(loaded.max_prep_time_minutes, Some(30)); + } + + #[test] + fn mock_storage_patterns() { + let storage = MockKnowledgeStorage::new(); + + let mut patterns = Patterns::default(); + patterns.success_patterns.push(crate::models::knowledge::SuccessPattern { + description: "Test pattern".to_string(), + sites: vec![], + success_rate: 0.9, + sample_size: 100, + confidence: 0.8, + }); + + storage.save_patterns(&patterns).unwrap(); + let loaded = storage.get_patterns().unwrap(); + assert_eq!(loaded.success_patterns.len(), 1); + } + + #[test] + fn mock_storage_clear() { + let storage = MockKnowledgeStorage::with_test_data(); + + assert!(!storage.list_site_configs().unwrap().is_empty()); + + storage.clear(); + + assert!(storage.list_site_configs().unwrap().is_empty()); + assert!(storage.list_user_models().unwrap().is_empty()); + } + + #[test] + fn mock_storage_load_for_context() { + let storage = MockKnowledgeStorage::new(); + + let config = SiteConfig::new("test.example.com"); + storage.save_site_config(&config).unwrap(); + + let ctx = storage.load_for_context(Some("test.example.com")).unwrap(); + assert_eq!(ctx.site_configs.len(), 1); + } +} diff --git a/src/knowledge/mod.rs b/src/knowledge/mod.rs index 99b4a74..cf9d8ae 100644 --- a/src/knowledge/mod.rs +++ b/src/knowledge/mod.rs @@ -25,8 +25,14 @@ pub mod site_config; pub mod store; pub mod user_model; +#[cfg(test)] +pub mod mock; + pub use context::KnowledgeContextAssembler; pub use patterns::PatternMatcher; pub use site_config::{SiteConfigManager, defaults}; pub use store::KnowledgeStore; pub use user_model::UserModelManager; + +#[cfg(test)] +pub use mock::MockKnowledgeStorage; diff --git a/src/knowledge/patterns.rs b/src/knowledge/patterns.rs index b670851..58ea9b0 100644 --- a/src/knowledge/patterns.rs +++ b/src/knowledge/patterns.rs @@ -206,24 +206,40 @@ impl AntiPattern { return false; } - // Check if URL matches the pattern description - // Simple heuristic: look for keywords in the URL + // Parse URL for more precise matching let url_lower = url.to_lowercase(); let desc_lower = self.description.to_lowercase(); - // Extract potential URL patterns from description - // e.g., "URL ends with /video" -> check if url ends with /video - if desc_lower.contains("/video") && url_lower.contains("/video") { - return true; + // Match specific path patterns with boundaries + // "URL ends with /video" -> exact suffix match + if desc_lower.contains("ends with /video") || desc_lower.contains("url ends with /video") { + return url_lower.ends_with("/video") + || url_lower.ends_with("/video/"); } - if desc_lower.contains("/gallery") && url_lower.contains("/gallery") { - return true; + + // "URL ends with /video - ..." format from default patterns + if desc_lower.contains("/video") { + // Check for video path segment (not just substring) + return url_lower.contains("/video/") + || url_lower.ends_with("/video"); + } + + // Gallery pattern + if desc_lower.contains("/gallery") { + return url_lower.contains("/gallery/") + || url_lower.ends_with("/gallery"); } - if desc_lower.contains("/videos") && url_lower.contains("/videos") { - return true; + + // Videos (plural) pattern + if desc_lower.contains("/videos") { + return url_lower.contains("/videos/") + || url_lower.ends_with("/videos"); } - if desc_lower.contains("/shows") && url_lower.contains("/shows") { - return true; + + // Shows pattern + if desc_lower.contains("/shows") { + return url_lower.contains("/shows/") + || url_lower.ends_with("/shows"); } false @@ -299,6 +315,48 @@ mod tests { )); } + #[test] + fn video_tips_url_does_not_match_video_pattern() { + let patterns = PatternMatcher::default_patterns(); + + // "video-tips" should NOT match the /video pattern (no path boundary) + let result = PatternMatcher::matches_anti_pattern( + &patterns, + "https://tasty.co/article/video-tips", + "tasty.co", + ); + + // Should NOT match the video anti-pattern + if let Some(ap) = result { + assert!(!ap.description.to_lowercase().contains("video"), + "video-tips should not match /video pattern"); + } + } + + #[test] + fn gallery_path_matches_correctly() { + let patterns = PatternMatcher::default_patterns(); + + // /gallery/ should match + assert!(PatternMatcher::matches_anti_pattern( + &patterns, + "https://allrecipes.com/recipe/gallery/test", + "allrecipes.com", + ).is_some()); + + // gallery-item should NOT match + let result = PatternMatcher::matches_anti_pattern( + &patterns, + "https://allrecipes.com/gallery-item/photo", + "allrecipes.com", + ); + + if let Some(ap) = result { + assert!(!ap.description.to_lowercase().contains("gallery"), + "gallery-item should not match /gallery pattern"); + } + } + #[test] fn get_success_patterns_for_domain() { let patterns = PatternMatcher::default_patterns(); diff --git a/src/knowledge/site_config.rs b/src/knowledge/site_config.rs index 9304e1f..e0583e4 100644 --- a/src/knowledge/site_config.rs +++ b/src/knowledge/site_config.rs @@ -35,21 +35,20 @@ impl SiteConfigManager { /// Merge configuration with a fallback. /// - /// Values present in self are kept; missing values use fallback. + /// Values present in config are always kept (explicitly set). + /// Missing/empty values (headers, skip_patterns, selectors) use fallback. + /// + /// Note: preferred_method and rate_limit_ms are always taken from config + /// since there's no way to distinguish "not set" from "set to default". pub fn merge_with_fallback(config: &SiteConfig, fallback: &SiteConfig) -> SiteConfig { SiteConfig { domain: config.domain.clone(), - preferred_method: if config.preferred_method == ParseMethod::SchemaOrg { - fallback.preferred_method - } else { - config.preferred_method - }, + // Always use config's method - it was explicitly set + preferred_method: config.preferred_method, selectors: merge_selectors(&config.selectors, &fallback.selectors), - rate_limit_ms: if config.rate_limit_ms == 1000 { - fallback.rate_limit_ms - } else { - config.rate_limit_ms - }, + // Always use config's rate limit - it was explicitly set + rate_limit_ms: config.rate_limit_ms, + // JS requirement is OR'd - if either needs it, use it requires_js: config.requires_js || fallback.requires_js, headers: if config.headers.is_empty() { fallback.headers.clone() @@ -154,6 +153,19 @@ pub mod defaults { use crate::models::ParseMethod; use std::collections::HashMap; + /// Default User-Agent for requests to recipe sites. + /// + /// Using a realistic browser User-Agent helps avoid basic bot detection. + /// This is a standard Chrome on macOS User-Agent string. + /// + /// Note: For production use, consider: + /// - Rotating User-Agents + /// - Using the `user-agent-from-env` feature to allow configuration + /// - Respecting robots.txt and rate limits regardless of User-Agent + const DEFAULT_USER_AGENT: &str = + "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 \ + (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36"; + /// Get default config for a known domain, if available. pub fn get(domain: &str) -> Option { match domain { @@ -183,7 +195,7 @@ pub mod defaults { fn allrecipes() -> SiteConfig { let mut headers = HashMap::new(); - headers.insert("User-Agent".to_string(), "Mozilla/5.0".to_string()); + headers.insert("User-Agent".to_string(), DEFAULT_USER_AGENT.to_string()); SiteConfig { domain: "allrecipes.com".to_string(), @@ -373,17 +385,21 @@ mod tests { } #[test] - fn merge_uses_fallback_for_missing() { + fn merge_uses_fallback_for_empty() { let self_config = SiteConfig::new("test.com"); let mut fallback = SiteConfig::new("fallback.com"); - fallback.rate_limit_ms = 5000; fallback.requires_js = true; + fallback.skip_patterns = vec!["/video/".to_string()]; let merged = SiteConfigManager::merge_with_fallback(&self_config, &fallback); - assert_eq!(merged.rate_limit_ms, 5000); // Used fallback - assert!(merged.requires_js); // Used fallback + // Rate limit is always from config (explicitly set) + assert_eq!(merged.rate_limit_ms, 1000); // Config's default + // JS is OR'd + assert!(merged.requires_js); + // Empty skip_patterns uses fallback + assert_eq!(merged.skip_patterns, vec!["/video/"]); } #[test] diff --git a/src/knowledge/store.rs b/src/knowledge/store.rs index 0a9f1d8..b558f9f 100644 --- a/src/knowledge/store.rs +++ b/src/knowledge/store.rs @@ -18,6 +18,17 @@ use super::patterns::PatternMatcher; use super::site_config::SiteConfigManager; use super::user_model::UserModelManager; +/// Cache time-to-live in seconds. +/// +/// After this duration, cached values will be considered stale +/// and reloaded from storage on next access. +/// +/// A 5-minute TTL balances: +/// - Freshness: Updates from other processes/threads are picked up quickly +/// - Performance: Avoids excessive storage reads for frequently accessed data +/// - Memory: Allows periodic cache refresh to clear unused entries +const CACHE_TTL_SECONDS: i64 = 300; // 5 minutes + /// In-memory cache for knowledge data. #[derive(Debug, Default)] struct KnowledgeCache { @@ -63,12 +74,31 @@ impl KnowledgeStore { } } + /// Check if cache needs refresh due to TTL expiry. + /// + /// Returns true if cache has never been refreshed or if TTL has elapsed. + fn is_cache_stale(&self) -> crate::storage::Result { + let cache = self.cache.read().map_err(|e| { + StorageError::Io(std::io::Error::other(format!("Cache lock error: {e}"))) + })?; + + Ok(cache.last_refresh + .map(|t| (Utc::now() - t).num_seconds() > CACHE_TTL_SECONDS) + .unwrap_or(true)) + } + /// Get site configuration for a domain. /// - /// Returns from cache if available, otherwise loads from storage. + /// Returns from cache if available and fresh (within TTL), + /// otherwise loads from storage. /// Falls back to default config if not found. pub fn get_site_config(&self, domain: &str) -> crate::storage::Result { - // Try read from cache first + // Check if we need to refresh due to TTL + if self.is_cache_stale()? { + self.refresh()?; + } + + // Fast path: try read from cache first { let cache = self.cache.read().map_err(|e| { StorageError::Io(std::io::Error::other(format!("Cache lock error: {e}"))) @@ -79,17 +109,23 @@ impl KnowledgeStore { } } - // Not in cache, load from storage + // Slow path: load from storage let config = match self.storage.get_site_config(domain)? { Some(c) => c, None => SiteConfig::new(domain), }; - // Update cache + // Update cache with write lock, re-checking to prevent race { let mut cache = self.cache.write().map_err(|e| { StorageError::Io(std::io::Error::other(format!("Cache lock error: {e}"))) })?; + + // Another thread may have inserted while we were loading + if let Some(existing) = cache.site_configs.get(domain) { + return Ok(existing.clone()); + } + cache.site_configs.insert(domain.to_string(), config.clone()); } @@ -121,7 +157,7 @@ impl KnowledgeStore { /// /// Creates default model if not exists. pub fn get_user_model(&self, user_id: &str) -> crate::storage::Result { - // Try read from cache first + // Fast path: try read from cache first { let cache = self.cache.read().map_err(|e| { StorageError::Io(std::io::Error::other(format!("Cache lock error: {e}"))) @@ -132,17 +168,23 @@ impl KnowledgeStore { } } - // Not in cache, load from storage + // Slow path: load from storage let model = match self.storage.get_user_model(user_id)? { Some(m) => m, - None => UserModel::default_user(), + None => UserModelManager::create(user_id), }; - // Update cache + // Update cache with write lock, re-checking to prevent race { let mut cache = self.cache.write().map_err(|e| { StorageError::Io(std::io::Error::other(format!("Cache lock error: {e}"))) })?; + + // Another thread may have inserted while we were loading + if let Some(existing) = cache.user_models.get(user_id) { + return Ok(existing.clone()); + } + cache.user_models.insert(user_id.to_string(), model.clone()); } @@ -175,7 +217,7 @@ impl KnowledgeStore { /// /// Returns default if not exists. pub fn get_patterns(&self) -> crate::storage::Result { - // Try read from cache first + // Fast path: try read from cache first { let cache = self.cache.read().map_err(|e| { StorageError::Io(std::io::Error::other(format!("Cache lock error: {e}"))) @@ -186,7 +228,7 @@ impl KnowledgeStore { } } - // Not in cache, load from storage + // Slow path: load from storage let patterns = self.storage.get_patterns()?; let patterns = if patterns.success_patterns.is_empty() && patterns.anti_patterns.is_empty() { // Storage returned empty, check for defaults @@ -200,11 +242,17 @@ impl KnowledgeStore { patterns }; - // Update cache + // Update cache with write lock, re-checking to prevent race { let mut cache = self.cache.write().map_err(|e| { StorageError::Io(std::io::Error::other(format!("Cache lock error: {e}"))) })?; + + // Another thread may have inserted while we were loading + if let Some(ref existing) = cache.patterns { + return Ok(existing.clone()); + } + cache.patterns = Some(patterns.clone()); } @@ -264,10 +312,11 @@ impl KnowledgeStore { #[cfg(test)] mod tests { use super::*; + use crate::knowledge::mock::MockKnowledgeStorage; use crate::models::ParseMethod; use tempfile::tempdir; - // Helper to create a test knowledge store + // Helper to create a test knowledge store with real storage fn test_store() -> (KnowledgeStore, tempfile::TempDir) { let dir = tempdir().unwrap(); let storage = Arc::new(crate::storage::FileKnowledgeStore::open( @@ -276,6 +325,11 @@ mod tests { (KnowledgeStore::new(storage), dir) } + // Helper to create a test store with mock storage (faster, no filesystem) + fn test_store_mock() -> KnowledgeStore { + KnowledgeStore::new(Arc::new(MockKnowledgeStorage::new())) + } + #[test] fn get_site_config_returns_default_for_unknown() { let (store, _dir) = test_store(); @@ -372,4 +426,167 @@ mod tests { let ctx = store.load_for_context(Some("example.com"), 1000).unwrap(); assert!(ctx.token_budget <= 1000); } + + #[test] + fn concurrent_cache_access_no_race() { + use std::sync::Barrier; + use std::thread; + + let (store, _dir) = test_store(); + let store = Arc::new(store); + let barrier = Arc::new(Barrier::new(10)); + + // First, save a config to storage (bypass cache) + let config = SiteConfig::new("concurrent-test.com"); + store.storage.save_site_config(&config).unwrap(); + + let handles: Vec<_> = (0..10) + .map(|_| { + let store = Arc::clone(&store); + let barrier = Arc::clone(&barrier); + thread::spawn(move || { + barrier.wait(); + // All threads try to get the same config simultaneously + let result = store.get_site_config("concurrent-test.com").unwrap(); + result + }) + }) + .collect(); + + let results: Vec<_> = handles.into_iter().map(|h| h.join().unwrap()).collect(); + + // All threads should get the same config + assert!(results.iter().all(|r| r.domain == "concurrent-test.com")); + + // Verify cache only has one entry + let cache = store.cache.read().unwrap(); + assert_eq!(cache.site_configs.len(), 1); + } + + #[test] + fn concurrent_user_model_access_no_race() { + use std::sync::Barrier; + use std::thread; + + let (store, _dir) = test_store(); + let store = Arc::new(store); + let barrier = Arc::new(Barrier::new(5)); + + let handles: Vec<_> = (0..5) + .map(|_| { + let store = Arc::clone(&store); + let barrier = Arc::clone(&barrier); + thread::spawn(move || { + barrier.wait(); + store.get_user_model("test-user").unwrap() + }) + }) + .collect(); + + let results: Vec<_> = handles.into_iter().map(|h| h.join().unwrap()).collect(); + + // All threads should get the same model + assert!(results.iter().all(|r| r.user_id == "test-user")); + } + + #[test] + fn cache_ttl_triggers_refresh() { + let (store, _dir) = test_store(); + + // Load a config into cache + let _ = store.get_site_config("example.com").unwrap(); + + // Verify it's cached + { + let cache = store.cache.read().unwrap(); + assert!(cache.site_configs.contains_key("example.com")); + assert!(cache.last_refresh.is_some()); + } + + // Manually set last_refresh to simulate TTL expiry + { + let mut cache = store.cache.write().unwrap(); + // Set last_refresh to 10 minutes ago (beyond the 5-minute TTL) + cache.last_refresh = Some(Utc::now() - chrono::Duration::seconds(600)); + } + + // Access should trigger refresh due to stale cache + let is_stale = store.is_cache_stale().unwrap(); + assert!(is_stale, "Cache should be stale after TTL expiry"); + } + + #[test] + fn fresh_cache_within_ttl() { + let (store, _dir) = test_store(); + + // Fresh store should have stale cache (never refreshed) + assert!(store.is_cache_stale().unwrap()); + + // Load something + let _ = store.get_site_config("example.com").unwrap(); + + // Cache should now be fresh + assert!(!store.is_cache_stale().unwrap()); + } + + // Tests using mock storage (no filesystem dependency) + #[test] + fn mock_storage_site_config() { + let store = test_store_mock(); + + let config = SiteConfig::new("mock-test.com"); + store.update_site_config(&config).unwrap(); + + let loaded = store.get_site_config("mock-test.com").unwrap(); + assert_eq!(loaded.domain, "mock-test.com"); + } + + #[test] + fn mock_storage_user_model() { + let store = test_store_mock(); + + let mut model = UserModelManager::create("test-user"); + model.max_prep_time_minutes = Some(45); + store.update_user_model(&model).unwrap(); + + let loaded = store.get_user_model("test-user").unwrap(); + assert_eq!(loaded.user_id, "test-user"); + assert_eq!(loaded.max_prep_time_minutes, Some(45)); + } + + #[test] + fn mock_storage_patterns() { + let store = test_store_mock(); + + let patterns = PatternMatcher::default_patterns(); + store.update_patterns(&patterns).unwrap(); + + let loaded = store.get_patterns().unwrap(); + assert!(!loaded.success_patterns.is_empty()); + } + + #[test] + fn mock_storage_concurrent_access() { + use std::sync::Barrier; + use std::thread; + + let store = Arc::new(test_store_mock()); + let barrier = Arc::new(Barrier::new(10)); + + let handles: Vec<_> = (0..10) + .map(|i| { + let store = Arc::clone(&store); + let barrier = Arc::clone(&barrier); + thread::spawn(move || { + barrier.wait(); + let domain = format!("domain-{}.com", i % 3); + store.get_site_config(&domain).unwrap() + }) + }) + .collect(); + + for h in handles { + h.join().unwrap(); + } + } } diff --git a/src/knowledge/user_model.rs b/src/knowledge/user_model.rs index 979aece..e5f8367 100644 --- a/src/knowledge/user_model.rs +++ b/src/knowledge/user_model.rs @@ -5,6 +5,7 @@ use crate::models::knowledge::UserModel; use crate::models::recipe::{Difficulty, Recipe}; +use std::collections::HashMap; /// Manager for user preference models. pub struct UserModelManager; @@ -20,6 +21,7 @@ impl UserModelManager { require_quantities: false, max_ingredients: None, preferred_difficulty: None, + difficulty_counts: HashMap::new(), dietary_restrictions: Vec::new(), sample_size: 0, updated_at: chrono::Utc::now(), @@ -57,8 +59,8 @@ impl UserModelManager { // Track dietary tags for tag in &recipe.tags { if !model.dietary_restrictions.contains(tag) { - // Only add if it appears multiple times (avoid noise) - // For simplicity, we add all unique tags seen + // Add all unique tags seen. + // Future improvement: track frequency and require N occurrences model.dietary_restrictions.push(tag.clone()); } } @@ -96,9 +98,13 @@ impl UserModelManager { /// /// Confidence increases logarithmically with more samples. /// Call this before persisting the model. + /// + /// Formula: log2(sample_size + 1) / 10 + /// - sample_size = 0 → confidence = 0.0 + /// - sample_size = 1023 → confidence = 1.0 (since log2(1024) = 10) pub fn update_confidence(model: &mut UserModel) { // Confidence formula: log2(sample_size + 1) / 10 - // Reaches 1.0 at ~1000 samples + // Reaches 1.0 at 1023 samples let sample_based_confidence = (model.sample_size as f64 + 1.0).log2() / 10.0; model.confidence = sample_based_confidence.min(1.0); } @@ -116,35 +122,47 @@ impl UserModelManager { let mut factors = 0; // Check time constraints - if let Some(max_prep) = model.max_prep_time_minutes - && let Some(prep) = recipe.prep_time_minutes - { - factors += 1; - if prep > max_prep { - score *= 1.0 - ((prep - max_prep) as f64 / max_prep as f64).min(1.0); + if let Some(max_prep) = model.max_prep_time_minutes { + if max_prep == 0 { + return 0.0; // Invalid constraint - can't match + } + if let Some(prep) = recipe.prep_time_minutes { + factors += 1; + if prep > max_prep { + score *= 1.0 - ((prep - max_prep) as f64 / max_prep as f64).min(1.0); + } } } - if let Some(max_cook) = model.max_cook_time_minutes - && let Some(cook) = recipe.cook_time_minutes - { - factors += 1; - if cook > max_cook { - score *= 1.0 - ((cook - max_cook) as f64 / max_cook as f64).min(1.0); + if let Some(max_cook) = model.max_cook_time_minutes { + if max_cook == 0 { + return 0.0; // Invalid constraint - can't match + } + if let Some(cook) = recipe.cook_time_minutes { + factors += 1; + if cook > max_cook { + score *= 1.0 - ((cook - max_cook) as f64 / max_cook as f64).min(1.0); + } } } - if let Some(max_total) = model.max_total_time_minutes - && let Some(total) = recipe.total_time_minutes - { - factors += 1; - if total > max_total { - score *= 1.0 - ((total - max_total) as f64 / max_total as f64).min(1.0); + if let Some(max_total) = model.max_total_time_minutes { + if max_total == 0 { + return 0.0; // Invalid constraint - can't match + } + if let Some(total) = recipe.total_time_minutes { + factors += 1; + if total > max_total { + score *= 1.0 - ((total - max_total) as f64 / max_total as f64).min(1.0); + } } } // Check ingredient count if let Some(max_ing) = model.max_ingredients { + if max_ing == 0 { + return 0.0; // Invalid constraint - can't match + } let count = recipe.ingredients.len() as u32; factors += 1; if count > max_ing { @@ -195,10 +213,33 @@ impl UserModelManager { } /// Update difficulty preference. + /// + /// Tracks frequency of each difficulty and sets preference to most common. fn update_difficulty_preference(model: &mut UserModel, difficulty: &Difficulty) { - // Prefer the difficulty that appears most often - // For simplicity, track the most recent - model.preferred_difficulty = Some(*difficulty); + use Difficulty::*; + + // Increment count for this difficulty + let key = match difficulty { + Easy => "Easy", + Medium => "Medium", + Hard => "Hard", + }; + let count = model.difficulty_counts.entry(key.to_string()).or_insert(0); + *count += 1; + + // Set preference to most common difficulty + // Find the difficulty with the highest count + let easy_count = *model.difficulty_counts.get("Easy").unwrap_or(&0); + let medium_count = *model.difficulty_counts.get("Medium").unwrap_or(&0); + let hard_count = *model.difficulty_counts.get("Hard").unwrap_or(&0); + + model.preferred_difficulty = if easy_count >= medium_count && easy_count >= hard_count { + Some(Easy) + } else if medium_count >= hard_count { + Some(Medium) + } else { + Some(Hard) + }; } } @@ -431,4 +472,57 @@ mod tests { // Max should be reduced (tightened) assert!(model.max_prep_time_minutes.unwrap() < 60); } + + #[test] + fn zero_max_time_returns_zero_match() { + let mut model = UserModel::default_user(); + model.max_prep_time_minutes = Some(0); + model.sample_size = 5; // Need sample_size > 0 to trigger preference matching + + let recipe = test_recipe(); + let score = UserModelManager::matches_preferences(&model, &recipe); + + // Zero constraint is invalid, should return 0 + assert_eq!(score, 0.0); + } + + #[test] + fn difficulty_preference_tracks_frequency() { + let mut model = UserModel::default_user(); + + // Keep 3 Easy recipes + for _ in 0..3 { + let mut recipe = test_recipe(); + recipe.difficulty = Some(Difficulty::Easy); + UserModelManager::record_recipe_kept(&mut model, &recipe); + } + + // Keep 1 Medium recipe + let mut recipe = test_recipe(); + recipe.difficulty = Some(Difficulty::Medium); + UserModelManager::record_recipe_kept(&mut model, &recipe); + + // Keep 1 Hard recipe + let mut recipe = test_recipe(); + recipe.difficulty = Some(Difficulty::Hard); + UserModelManager::record_recipe_kept(&mut model, &recipe); + + // Preference should be Easy (most frequent) + assert_eq!(model.preferred_difficulty, Some(Difficulty::Easy)); + assert_eq!(*model.difficulty_counts.get("Easy").unwrap_or(&0), 3); + assert_eq!(*model.difficulty_counts.get("Medium").unwrap_or(&0), 1); + assert_eq!(*model.difficulty_counts.get("Hard").unwrap_or(&0), 1); + } + + #[test] + fn zero_max_ingredients_returns_zero_match() { + let mut model = UserModel::default_user(); + model.max_ingredients = Some(0); + model.sample_size = 5; + + let recipe = test_recipe(); + let score = UserModelManager::matches_preferences(&model, &recipe); + + assert_eq!(score, 0.0); + } } diff --git a/src/models/knowledge.rs b/src/models/knowledge.rs index d0d8922..90235bf 100644 --- a/src/models/knowledge.rs +++ b/src/models/knowledge.rs @@ -5,8 +5,6 @@ //! - User preference models //! - Discovered patterns (success and anti-patterns) -#![allow(dead_code)] - use chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; use std::collections::HashMap; @@ -181,6 +179,10 @@ pub struct UserModel { #[serde(default)] pub preferred_difficulty: Option, + /// Count of each difficulty level kept (for frequency tracking). + #[serde(default)] + pub difficulty_counts: HashMap, + /// Dietary restrictions. #[serde(default)] pub dietary_restrictions: Vec, @@ -212,6 +214,7 @@ impl UserModel { require_quantities: false, max_ingredients: None, preferred_difficulty: None, + difficulty_counts: HashMap::new(), dietary_restrictions: Vec::new(), sample_size: 0, updated_at: Utc::now(), @@ -332,6 +335,10 @@ mod tests { #[test] fn user_model_serialization() { + let mut difficulty_counts = HashMap::new(); + difficulty_counts.insert("Easy".to_string(), 10); + difficulty_counts.insert("Medium".to_string(), 5); + let model = UserModel { user_id: "test-user".to_string(), max_prep_time_minutes: Some(30), @@ -340,6 +347,7 @@ mod tests { require_quantities: true, max_ingredients: Some(10), preferred_difficulty: Some(crate::models::recipe::Difficulty::Easy), + difficulty_counts, dietary_restrictions: vec!["vegetarian".to_string()], sample_size: 15, updated_at: Utc::now(),