From b02da0d0cc38a82f75fb42cd4fca3060b052c514 Mon Sep 17 00:00:00 2001 From: Cameron Cordes Date: Thu, 7 May 2026 14:47:46 -0400 Subject: [PATCH] insight-chat: code-review polish on the days_radius fix - Bind effective_radius once in fetch_messages_for_contact so the log output and window math share a single source of truth for the clamp. - Clamp tool-supplied days_radius to [1, 30] at the tool boundary so a runaway LLM value can't produce a thousand-day window. - Split the negative-input test into a real negative-input case alongside the zero-input case. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/ai/insight_generator.rs | 3 ++- src/ai/sms_client.rs | 13 ++++++++++--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/ai/insight_generator.rs b/src/ai/insight_generator.rs index ca28115..911068b 100644 --- a/src/ai/insight_generator.rs +++ b/src/ai/insight_generator.rs @@ -1812,7 +1812,8 @@ Return ONLY the summary, nothing else."#, let days_radius = args .get("days_radius") .and_then(|v| v.as_i64()) - .unwrap_or(4); + .unwrap_or(4) + .clamp(1, 30); let limit = args .get("limit") .and_then(|v| v.as_i64()) diff --git a/src/ai/sms_client.rs b/src/ai/sms_client.rs index 46db005..a4843ee 100644 --- a/src/ai/sms_client.rs +++ b/src/ai/sms_client.rs @@ -38,6 +38,7 @@ impl SmsApiClient { center_timestamp: i64, radius_days: i64, ) -> Result> { + let effective_radius = radius_days.max(1); let (start_ts, end_ts) = Self::window_for_radius(center_timestamp, radius_days); let center_dt = chrono::DateTime::from_timestamp(center_timestamp, 0) @@ -48,7 +49,7 @@ impl SmsApiClient { log::info!( "Fetching SMS for contact: {} (±{} days from {})", contact_name, - radius_days.max(1), + effective_radius, center_dt.format("%Y-%m-%d %H:%M:%S") ); let messages = self @@ -73,7 +74,7 @@ impl SmsApiClient { // Fallback to all contacts log::info!( "Fetching all SMS messages (±{} days from {})", - radius_days.max(1), + effective_radius, center_dt.format("%Y-%m-%d %H:%M:%S") ); self.fetch_messages(start_ts, end_ts, None, Some(center_timestamp)) @@ -399,8 +400,14 @@ mod tests { } #[test] - fn window_for_radius_clamps_negative_to_one() { + fn window_for_radius_clamps_zero_to_one() { let (start, end) = SmsApiClient::window_for_radius(100_000, 0); assert_eq!(end - start, 2 * 86400); } + + #[test] + fn window_for_radius_clamps_negative_to_one() { + let (start, end) = SmsApiClient::window_for_radius(100_000, -7); + assert_eq!(end - start, 2 * 86400); + } }