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) <noreply@anthropic.com>
This commit is contained in:
@@ -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())
|
||||
|
||||
@@ -38,6 +38,7 @@ impl SmsApiClient {
|
||||
center_timestamp: i64,
|
||||
radius_days: i64,
|
||||
) -> Result<Vec<SmsMessage>> {
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user