From c4d2a0ce49449aba1040801ce7f8d5f041f2823a Mon Sep 17 00:00:00 2001 From: Matt OD Date: Fri, 17 Jul 2026 01:21:55 -0700 Subject: [PATCH] fix(context): surface review narratives that carry no numeric rating MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Chat context read performance_ratings only, so an employee reviewed in prose but never scored had no ratings, no extracted highlights and no career summary — the whole performance block was gated on all_ratings and simply did not render. The model then answered "no performance data" for someone whose reviews Prep Brief rendered in full from the same DB. Add review presence (count + latest date) to EmployeeContext, sourced from performance_reviews directly, and render it independently of the ratings block. This is the floor described in the issue: state that the reviews exist. Choosing the richer inclusion shape (full narratives vs FTS excerpts) is left open — it changes what the assistant says about someone's performance, which is a product decision rather than a mechanical one. Resolves #154. Co-Authored-By: Claude Opus 4.8 (1M context) --- src-tauri/src/context/prompt.rs | 101 +++++++++++++++++++++++++++++ src-tauri/src/context/retrieval.rs | 73 +++++++++++++++++++-- 2 files changed, 170 insertions(+), 4 deletions(-) diff --git a/src-tauri/src/context/prompt.rs b/src-tauri/src/context/prompt.rs index ca3507b..286943f 100644 --- a/src-tauri/src/context/prompt.rs +++ b/src-tauri/src/context/prompt.rs @@ -432,6 +432,25 @@ fn format_single_employee_with_budget(emp: &EmployeeContext, token_budget: Optio } } + // Review narratives on file. Deliberately independent of `all_ratings`: an + // employee reviewed in prose but never scored has no ratings, no extracted + // highlights, and no career summary, so without this line the context is + // silent about reviews that other surfaces render in full. + if emp.review_count > 0 { + let latest = emp + .latest_review_date + .as_deref() + .map(|d| format!(", latest {}", d)) + .unwrap_or_default(); + let plural = if emp.review_count == 1 { "" } else { "s" }; + lines.push(format!( + " Performance reviews: {} review{} on file{}. Narrative text is not \ + included in this context — say the reviews exist and offer to pull \ + them up rather than stating there is no review history.", + emp.review_count, plural, latest + )); + } + // eNPS info if !emp.all_enps.is_empty() { lines.push(" eNPS:".to_string()); @@ -1152,6 +1171,8 @@ mod tests { sentiment: "mixed".to_string(), }, ], + review_count: 2, + latest_review_date: Some("2024-12-01".to_string()), } } @@ -1228,6 +1249,8 @@ mod tests { key_strengths: vec![], development_areas: vec![], recent_highlights: vec![], + review_count: 0, + latest_review_date: None, }; let formatted = format_single_employee(&emp); @@ -1237,6 +1260,84 @@ mod tests { assert!(!formatted.contains("Career Summary:")); assert!(!formatted.contains("Key Strengths:")); assert!(!formatted.contains("Recent Review Highlights:")); + assert!(!formatted.contains("Performance reviews:")); + } + + /// Reviews-but-no-ratings employee: every performance field except the + /// review counts is empty, which is what let chat answer "no performance + /// data" for someone whose reviews another surface rendered in full. + fn make_test_employee_reviews_only() -> EmployeeContext { + EmployeeContext { + id: "emp-3".to_string(), + full_name: "Maya Chen".to_string(), + email: "maya@company.com".to_string(), + department: Some("Design".to_string()), + job_title: Some("Product Designer".to_string()), + hire_date: None, + work_state: None, + status: "Active".to_string(), + manager_name: None, + latest_rating: None, + latest_rating_cycle: None, + rating_trend: None, + all_ratings: vec![], + latest_enps: None, + latest_enps_date: None, + enps_trend: None, + all_enps: vec![], + career_summary: None, + key_strengths: vec![], + development_areas: vec![], + recent_highlights: vec![], + review_count: 2, + latest_review_date: Some("2026-03-14".to_string()), + } + } + + #[test] + fn test_format_employee_with_reviews_but_no_ratings_mentions_reviews() { + let emp = make_test_employee_reviews_only(); + let formatted = format_single_employee(&emp); + + assert!( + formatted.contains("2 reviews on file"), + "context must state the reviews exist: {formatted}" + ); + assert!( + formatted.contains("2026-03-14"), + "context must carry the latest review date: {formatted}" + ); + } + + #[test] + fn test_reviews_surface_without_a_rating_present() { + // The ratings block is gated on `all_ratings`; the reviews line must not + // be, or this employee's context stays silent about the narratives. + let emp = make_test_employee_reviews_only(); + let formatted = format_single_employee(&emp); + + assert!(!formatted.contains(" Performance:")); + assert!(formatted.contains("Performance reviews:")); + } + + #[test] + fn test_format_employee_review_count_singular() { + let mut emp = make_test_employee_reviews_only(); + emp.review_count = 1; + let formatted = format_single_employee(&emp); + + assert!(formatted.contains("1 review on file")); + assert!(!formatted.contains("1 reviews on file")); + } + + #[test] + fn test_format_employee_reviews_without_date_omits_latest() { + let mut emp = make_test_employee_reviews_only(); + emp.latest_review_date = None; + let formatted = format_single_employee(&emp); + + assert!(formatted.contains("2 reviews on file")); + assert!(!formatted.contains(", latest")); } #[test] diff --git a/src-tauri/src/context/retrieval.rs b/src-tauri/src/context/retrieval.rs index ea536bb..20a3dad 100644 --- a/src-tauri/src/context/retrieval.rs +++ b/src-tauri/src/context/retrieval.rs @@ -49,6 +49,13 @@ pub struct EmployeeContext { pub key_strengths: Vec, pub development_areas: Vec, pub recent_highlights: Vec, + + // Raw review narratives on file. Sourced from `performance_reviews`, not + // from ratings or extracted highlights: an employee can have narratives but + // no numeric rating and no extraction, and every other field here would + // still be empty for them. + pub review_count: usize, + pub latest_review_date: Option, } /// Extracted highlight data for a single review cycle (V2.2.1) @@ -280,6 +287,49 @@ pub async fn find_relevant_employees( Ok(finalize_results(employees)) } +/// Count review narratives on file per employee, with the latest review date. +/// +/// `MAX(review_date)` is a lexical max over ISO-8601 `TEXT`, which orders +/// correctly for that format; rows with a NULL `review_date` still count toward +/// the total but cannot supply the date. +async fn get_review_presence_by_emp( + pool: &DbPool, + employee_ids: &[String], +) -> std::collections::HashMap)> { + if employee_ids.is_empty() { + return std::collections::HashMap::new(); + } + + let placeholders = employee_ids.iter().map(|_| "?").collect::>().join(","); + let query = format!( + "SELECT employee_id, COUNT(*) AS review_count, MAX(review_date) AS latest_review_date \ + FROM performance_reviews WHERE employee_id IN ({}) GROUP BY employee_id", + placeholders + ); + + let mut q = sqlx::query(&query); + for id in employee_ids { + q = q.bind(id); + } + + // A failure here must not blank out the rest of an employee's context — + // degrade to "no reviews known" rather than propagating. + q.fetch_all(pool) + .await + .unwrap_or_default() + .into_iter() + .map(|row| { + ( + row.get::("employee_id"), + ( + row.get::("review_count").max(0) as usize, + row.get::, _>("latest_review_date"), + ), + ) + }) + .collect() +} + /// Get full context for a single employee including performance and eNPS pub async fn get_employee_context( pool: &DbPool, @@ -402,6 +452,12 @@ pub async fn get_employee_context( let key_strengths = summary.as_ref().map(|s| s.key_strengths.clone()).unwrap_or_default(); let development_areas = summary.as_ref().map(|s| s.development_areas.clone()).unwrap_or_default(); + let (review_count, latest_review_date) = + get_review_presence_by_emp(pool, std::slice::from_ref(&emp.id)) + .await + .remove(&emp.id) + .unwrap_or((0, None)); + Ok(EmployeeContext { id: emp.id, full_name: emp.full_name, @@ -425,13 +481,15 @@ pub async fn get_employee_context( key_strengths, development_areas, recent_highlights, + review_count, + latest_review_date, }) } -/// Batch variant of `get_employee_context`. Issues 4 IN-clause queries (basic -/// info, manager names, ratings + cycles, eNPS) instead of N×4 sequential -/// per-employee queries, then assembles the EmployeeContext list in input-ID -/// order. +/// Batch variant of `get_employee_context`. Issues 5 IN-clause queries (basic +/// info, manager names, ratings + cycles, eNPS, review presence) instead of N×5 +/// sequential per-employee queries, then assembles the EmployeeContext list in +/// input-ID order. /// /// IDs not found in the employees table are silently skipped — matches the /// per-row `if let Ok(emp) = ...` pattern at every caller in this module. @@ -558,6 +616,8 @@ pub async fn get_employee_contexts( } // Assemble in input-ID order, dropping IDs not found in basic_by_id. + let review_presence_by_emp = get_review_presence_by_emp(pool, employee_ids).await; + let mut out: Vec = Vec::with_capacity(employee_ids.len()); for id in employee_ids { let Some(emp) = basic_by_id.get(id) else { @@ -654,6 +714,9 @@ pub async fn get_employee_contexts( .map(|s| s.development_areas.clone()) .unwrap_or_default(); + let (review_count, latest_review_date) = + review_presence_by_emp.get(id).cloned().unwrap_or((0, None)); + out.push(EmployeeContext { id: emp.id.clone(), full_name: emp.full_name.clone(), @@ -676,6 +739,8 @@ pub async fn get_employee_contexts( key_strengths, development_areas, recent_highlights, + review_count, + latest_review_date, }); }