Add per-file insight history endpoint and rate-by-id
Expose GET /insights/history?path=... returning every generated version of a photo's insight (current plus superseded), newest-first, backing the mobile per-file insight history view. - New get_insight_history_handler; reuses the existing get_insight_history DAO method (removed its dead_code allow). - impl From<PhotoInsight> for PhotoInsightResponse, collapsing the mapping that was duplicated across the single-get and all-insights handlers. - rate_insight_by_id DAO method + optional insight_id on RateInsightRequest so previously generated versions can be approved/rejected (the path-based rate only touches the current row). - DAO tests for history ordering/scoping and id-targeted rating. - cargo fmt normalized a multi-line assert in insight_chat.rs tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -47,7 +47,6 @@ pub trait InsightDao: Sync + Send {
|
||||
paths: &[String],
|
||||
) -> Result<Option<PhotoInsight>, DbError>;
|
||||
|
||||
#[allow(dead_code)]
|
||||
fn get_insight_history(
|
||||
&mut self,
|
||||
context: &opentelemetry::Context,
|
||||
@@ -82,6 +81,17 @@ pub trait InsightDao: Sync + Send {
|
||||
approved: bool,
|
||||
) -> Result<(), DbError>;
|
||||
|
||||
/// Rate a specific insight version by primary key, regardless of
|
||||
/// `is_current`. Used by the per-file history view to approve/reject
|
||||
/// previously generated (superseded) versions, which the path-based
|
||||
/// `rate_insight` (current row only) cannot reach.
|
||||
fn rate_insight_by_id(
|
||||
&mut self,
|
||||
context: &opentelemetry::Context,
|
||||
insight_id: i32,
|
||||
approved: bool,
|
||||
) -> Result<(), DbError>;
|
||||
|
||||
fn get_approved_insights(
|
||||
&mut self,
|
||||
context: &opentelemetry::Context,
|
||||
@@ -352,6 +362,26 @@ impl InsightDao for SqliteInsightDao {
|
||||
.map_err(|e| DbError::log(DbErrorKind::UpdateError, e))
|
||||
}
|
||||
|
||||
fn rate_insight_by_id(
|
||||
&mut self,
|
||||
context: &opentelemetry::Context,
|
||||
target_id: i32,
|
||||
is_approved: bool,
|
||||
) -> Result<(), DbError> {
|
||||
trace_db_call(context, "update", "rate_insight_by_id", |_span| {
|
||||
use schema::photo_insights::dsl::*;
|
||||
|
||||
let mut connection = self.connection.lock().expect("Unable to get InsightDao");
|
||||
|
||||
diesel::update(photo_insights.find(target_id))
|
||||
.set(approved.eq(Some(is_approved)))
|
||||
.execute(connection.deref_mut())
|
||||
.map(|_| ())
|
||||
.map_err(|e| anyhow::anyhow!("Update error: {}", e))
|
||||
})
|
||||
.map_err(|e| DbError::log(DbErrorKind::UpdateError, e))
|
||||
}
|
||||
|
||||
fn get_approved_insights(
|
||||
&mut self,
|
||||
context: &opentelemetry::Context,
|
||||
@@ -396,3 +426,90 @@ impl InsightDao for SqliteInsightDao {
|
||||
.map_err(|e| DbError::log(DbErrorKind::UpdateError, e))
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::database::test::in_memory_db_connection;
|
||||
|
||||
fn dao() -> SqliteInsightDao {
|
||||
let conn = Arc::new(Mutex::new(in_memory_db_connection()));
|
||||
SqliteInsightDao::from_connection(conn)
|
||||
}
|
||||
|
||||
/// Build an insight insert with sensible defaults; tests override the
|
||||
/// fields they care about (path, generated_at, model).
|
||||
fn insert(path: &str, generated_at: i64, model: &str) -> InsertPhotoInsight {
|
||||
InsertPhotoInsight {
|
||||
library_id: 1,
|
||||
file_path: path.to_string(),
|
||||
title: format!("title for {model}"),
|
||||
summary: "summary".to_string(),
|
||||
generated_at,
|
||||
model_version: model.to_string(),
|
||||
is_current: true,
|
||||
training_messages: None,
|
||||
backend: "local".to_string(),
|
||||
fewshot_source_ids: None,
|
||||
content_hash: None,
|
||||
num_ctx: None,
|
||||
temperature: None,
|
||||
top_p: None,
|
||||
top_k: None,
|
||||
min_p: None,
|
||||
system_prompt: None,
|
||||
persona_id: None,
|
||||
prompt_eval_count: None,
|
||||
eval_count: None,
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn get_insight_history_returns_all_versions_newest_first() {
|
||||
let cx = opentelemetry::Context::new();
|
||||
let mut dao = dao();
|
||||
|
||||
// store_insight flips prior rows to is_current=false, so three
|
||||
// generations for the same path leave a 3-row history.
|
||||
dao.store_insight(&cx, insert("a.jpg", 100, "m1")).unwrap();
|
||||
dao.store_insight(&cx, insert("a.jpg", 200, "m2")).unwrap();
|
||||
dao.store_insight(&cx, insert("a.jpg", 300, "m3")).unwrap();
|
||||
// A different path must not leak into the history.
|
||||
dao.store_insight(&cx, insert("b.jpg", 250, "other"))
|
||||
.unwrap();
|
||||
|
||||
let history = dao.get_insight_history(&cx, "a.jpg").unwrap();
|
||||
assert_eq!(history.len(), 3);
|
||||
assert_eq!(
|
||||
history.iter().map(|i| i.generated_at).collect::<Vec<_>>(),
|
||||
vec![300, 200, 100],
|
||||
"history should be newest-first"
|
||||
);
|
||||
// Exactly one version is current (the latest generation).
|
||||
let current: Vec<_> = history.iter().filter(|i| i.is_current).collect();
|
||||
assert_eq!(current.len(), 1);
|
||||
assert_eq!(current[0].generated_at, 300);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rate_insight_by_id_rates_only_the_targeted_version() {
|
||||
let cx = opentelemetry::Context::new();
|
||||
let mut dao = dao();
|
||||
|
||||
dao.store_insight(&cx, insert("a.jpg", 100, "m1")).unwrap();
|
||||
dao.store_insight(&cx, insert("a.jpg", 200, "m2")).unwrap();
|
||||
|
||||
// History is newest-first: [200 (current), 100 (superseded)].
|
||||
let history = dao.get_insight_history(&cx, "a.jpg").unwrap();
|
||||
let old_version = history.iter().find(|i| i.generated_at == 100).unwrap();
|
||||
assert!(!old_version.is_current);
|
||||
|
||||
dao.rate_insight_by_id(&cx, old_version.id, true).unwrap();
|
||||
|
||||
let history = dao.get_insight_history(&cx, "a.jpg").unwrap();
|
||||
let old = history.iter().find(|i| i.generated_at == 100).unwrap();
|
||||
let current = history.iter().find(|i| i.generated_at == 200).unwrap();
|
||||
assert_eq!(old.approved, Some(true), "targeted version is rated");
|
||||
assert_eq!(current.approved, None, "current version is untouched");
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user