insight-chat: code-review polish on the tool-gating PR
- search_messages now delegates to search_messages_with_contact(.., None) so the two methods share a single HTTP path. Drops the dead-code warning and the ~30-line duplication. - DailySummaryDao gains has_any_summaries (LIMIT 1 existence probe) used by current_gate_opts; the SELECT COUNT(*) get_total_summary_count added in the prior commit is removed (it had no other caller). - current_gate_opts doc comment corrected to describe what the probes actually do. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -144,10 +144,13 @@ impl InsightGenerator {
|
||||
}
|
||||
|
||||
/// Compute the per-call tool gate options by probing each backing
|
||||
/// table. Cheap (`SELECT 1 FROM <t> LIMIT 1` shape via the existing
|
||||
/// count methods); meant to be called once per chat turn / generation.
|
||||
/// `has_vision` is supplied by the caller because it depends on the
|
||||
/// model selected for this turn, not on persistent state.
|
||||
/// table for presence. `daily_summaries_present` uses a `LIMIT 1`
|
||||
/// existence probe; `calendar_present` and `location_history_present`
|
||||
/// use the existing `get_event_count` / `get_location_count`
|
||||
/// methods (small enough that a full `COUNT(*)` is fine). Meant to
|
||||
/// be called once per chat turn / generation. `has_vision` is
|
||||
/// supplied by the caller because it depends on the model selected
|
||||
/// for this turn, not on persistent state.
|
||||
pub fn current_gate_opts(&self, has_vision: bool) -> ToolGateOpts {
|
||||
let cx = opentelemetry::Context::new();
|
||||
let calendar_present = {
|
||||
@@ -169,9 +172,7 @@ impl InsightGenerator {
|
||||
.daily_summary_dao
|
||||
.lock()
|
||||
.expect("Unable to lock DailySummaryDao");
|
||||
dao.get_total_summary_count(&cx)
|
||||
.map(|n| n > 0)
|
||||
.unwrap_or(false)
|
||||
dao.has_any_summaries(&cx).unwrap_or(false)
|
||||
};
|
||||
ToolGateOpts {
|
||||
has_vision,
|
||||
|
||||
@@ -261,38 +261,16 @@ impl SmsApiClient {
|
||||
/// - "fts5" keyword-only, supports phrase / prefix / boolean / NEAR
|
||||
/// - "semantic" embedding similarity
|
||||
/// - "hybrid" both merged via reciprocal rank fusion (recommended)
|
||||
///
|
||||
/// Equivalent to `search_messages_with_contact(query, mode, limit, None)`;
|
||||
/// kept as a convenience for callers that don't filter by contact.
|
||||
pub async fn search_messages(
|
||||
&self,
|
||||
query: &str,
|
||||
mode: &str,
|
||||
limit: usize,
|
||||
) -> Result<Vec<SmsSearchHit>> {
|
||||
let url = format!(
|
||||
"{}/api/messages/search/?q={}&mode={}&limit={}",
|
||||
self.base_url,
|
||||
urlencoding::encode(query),
|
||||
urlencoding::encode(mode),
|
||||
limit
|
||||
);
|
||||
|
||||
let mut request = self.client.get(&url);
|
||||
if let Some(token) = &self.token {
|
||||
request = request.header("Authorization", format!("Bearer {}", token));
|
||||
}
|
||||
|
||||
let response = request.send().await?;
|
||||
if !response.status().is_success() {
|
||||
let status = response.status();
|
||||
let body = response.text().await.unwrap_or_default();
|
||||
return Err(anyhow::anyhow!(
|
||||
"SMS search request failed: {} - {}",
|
||||
status,
|
||||
body
|
||||
));
|
||||
}
|
||||
|
||||
let data: SmsSearchResponse = response.json().await?;
|
||||
Ok(data.results)
|
||||
self.search_messages_with_contact(query, mode, limit, None).await
|
||||
}
|
||||
|
||||
/// Same shape as `search_messages` but with optional `contact_id`. The
|
||||
|
||||
@@ -76,12 +76,13 @@ pub trait DailySummaryDao: Sync + Send {
|
||||
contact: &str,
|
||||
) -> Result<i64, DbError>;
|
||||
|
||||
/// Get total count of all summaries (across all contacts). Used by
|
||||
/// `current_gate_opts` to check whether daily_summaries are present.
|
||||
fn get_total_summary_count(
|
||||
/// Cheap presence check — returns true iff at least one daily summary row
|
||||
/// exists. Used by gating logic that only needs "is the table empty?",
|
||||
/// avoiding a `COUNT(*)` full scan on large corpora.
|
||||
fn has_any_summaries(
|
||||
&mut self,
|
||||
context: &opentelemetry::Context,
|
||||
) -> Result<i64, DbError>;
|
||||
) -> Result<bool, DbError>;
|
||||
}
|
||||
|
||||
pub struct SqliteDailySummaryDao {
|
||||
@@ -462,20 +463,27 @@ impl DailySummaryDao for SqliteDailySummaryDao {
|
||||
.map_err(|_| DbError::new(DbErrorKind::QueryError))
|
||||
}
|
||||
|
||||
fn get_total_summary_count(
|
||||
&mut self,
|
||||
context: &opentelemetry::Context,
|
||||
) -> Result<i64, DbError> {
|
||||
trace_db_call(context, "query", "get_total_summary_count", |_span| {
|
||||
fn has_any_summaries(&mut self, context: &opentelemetry::Context) -> Result<bool, DbError> {
|
||||
trace_db_call(context, "query", "has_any_summaries", |_span| {
|
||||
let mut conn = self
|
||||
.connection
|
||||
.lock()
|
||||
.expect("Unable to get DailySummaryDao");
|
||||
|
||||
diesel::sql_query("SELECT COUNT(*) as count FROM daily_conversation_summaries")
|
||||
.get_result::<CountResult>(conn.deref_mut())
|
||||
.map(|r| r.count)
|
||||
.map_err(|e| anyhow::anyhow!("Count query error: {:?}", e))
|
||||
#[derive(QueryableByName)]
|
||||
struct ProbeResult {
|
||||
#[diesel(sql_type = diesel::sql_types::Integer)]
|
||||
#[allow(dead_code)]
|
||||
one: i32,
|
||||
}
|
||||
|
||||
let rows: Vec<ProbeResult> = diesel::sql_query(
|
||||
"SELECT 1 as one FROM daily_conversation_summaries LIMIT 1",
|
||||
)
|
||||
.load(conn.deref_mut())
|
||||
.map_err(|e| anyhow::anyhow!("Failed to probe daily summaries: {}", e))?;
|
||||
|
||||
Ok(!rows.is_empty())
|
||||
})
|
||||
.map_err(|_| DbError::new(DbErrorKind::QueryError))
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user