diff --git a/src-tauri/src/context/prompt.rs b/src-tauri/src/context/prompt.rs index ca3507b..0554fb0 100644 --- a/src-tauri/src/context/prompt.rs +++ b/src-tauri/src/context/prompt.rs @@ -416,7 +416,10 @@ fn format_single_employee_with_budget(emp: &EmployeeContext, token_budget: Optio } // Performance info - if !emp.all_ratings.is_empty() { + // #154: reviews and ratings are independent — an employee may have narrative + // reviews and no numeric ratings. Gating this section on ratings alone made + // such employees look like they had no performance history at all. + if !emp.all_ratings.is_empty() || emp.review_count > 0 { lines.push(" Performance:".to_string()); for rating in emp.all_ratings.iter().take(3) { let label = rating_label(rating.overall_rating); @@ -430,6 +433,19 @@ fn format_single_employee_with_budget(emp: &EmployeeContext, token_budget: Optio if let Some(ref trend) = emp.rating_trend { lines.push(format!(" Trend: {}", trend)); } + if emp.review_count > 0 { + let plural = if emp.review_count == 1 { "" } else { "s" }; + let latest = match emp.latest_review_date.as_deref() { + Some(d) => format!(" (latest {})", d), + None => String::new(), + }; + // State existence, not content: the narrative text is not loaded into + // this context, so the model must not claim to have read it. + lines.push(format!( + " - {} written performance review{} on file{}; narrative text not loaded here", + emp.review_count, plural, latest + )); + } } // eNPS info @@ -1122,6 +1138,8 @@ mod tests { latest_rating: Some(4.2), latest_rating_cycle: Some("2024 H2".to_string()), rating_trend: Some("improving".to_string()), + review_count: 0, + latest_review_date: None, all_ratings: vec![ RatingInfo { cycle_name: "2024 H2".to_string(), @@ -1220,6 +1238,8 @@ mod tests { latest_rating_cycle: None, rating_trend: None, all_ratings: vec![], + review_count: 0, + latest_review_date: None, latest_enps: None, latest_enps_date: None, enps_trend: None, @@ -1237,6 +1257,80 @@ mod tests { assert!(!formatted.contains("Career Summary:")); assert!(!formatted.contains("Key Strengths:")); assert!(!formatted.contains("Recent Review Highlights:")); + // #154: no reviews and no ratings => no Performance section at all. + assert!(!formatted.contains("Performance:")); + } + + /// #154 regression lock: an employee with narrative reviews but NO numeric + /// ratings must still surface a performance section. Previously the section + /// was gated on `all_ratings` alone, so chat asserted "no review history" + /// for exactly these employees while Prep Brief cited the same reviews. + fn make_employee_reviews_no_ratings(review_count: usize, latest: Option<&str>) -> EmployeeContext { + EmployeeContext { + id: "emp-3".to_string(), + full_name: "Maya Patel".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![], + review_count, + latest_review_date: latest.map(|s| s.to_string()), + 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![], + } + } + + #[test] + fn test_reviews_without_ratings_are_surfaced() { + let emp = make_employee_reviews_no_ratings(2, Some("2026-03-01")); + let formatted = format_single_employee(&emp); + + // The section must exist even though there is not a single rating. + assert!(formatted.contains("Performance:")); + assert!(formatted.contains("2 written performance reviews on file")); + assert!(formatted.contains("2026-03-01")); + // Existence only — the model must not think it has the narrative text. + assert!(formatted.contains("narrative text not loaded here")); + } + + #[test] + fn test_single_review_is_singular_and_survives_null_date() { + // review_date is nullable; an all-NULL set must not print "(latest )". + let emp = make_employee_reviews_no_ratings(1, None); + let formatted = format_single_employee(&emp); + + assert!(formatted.contains("1 written performance review on file")); + assert!(!formatted.contains("reviews on file")); + assert!(!formatted.contains("latest")); + } + + #[test] + fn test_ratings_and_reviews_both_render() { + let mut emp = make_employee_reviews_no_ratings(3, Some("2026-05-10")); + emp.all_ratings = vec![RatingInfo { + cycle_name: "2026 H1".to_string(), + overall_rating: 4.0, + rating_date: Some("2026-06-01".to_string()), + }]; + emp.rating_trend = Some("stable".to_string()); + let formatted = format_single_employee(&emp); + + assert!(formatted.contains("2026 H1")); + assert!(formatted.contains("Trend: stable")); + assert!(formatted.contains("3 written performance reviews on file")); } #[test] diff --git a/src-tauri/src/context/retrieval.rs b/src-tauri/src/context/retrieval.rs index ea536bb..390221f 100644 --- a/src-tauri/src/context/retrieval.rs +++ b/src-tauri/src/context/retrieval.rs @@ -38,6 +38,12 @@ pub struct EmployeeContext { pub rating_trend: Option, // "improving", "stable", "declining" pub all_ratings: Vec, + // #154: Narrative reviews are stored separately from numeric ratings. An + // employee can have reviews with no ratings; without these the context + // renders nothing and the model asserts there is no review history. + pub review_count: usize, + pub latest_review_date: Option, + // eNPS data pub latest_enps: Option, pub latest_enps_date: Option, @@ -138,6 +144,14 @@ struct RatingRow { rating_date: Option, } +/// Internal struct for the #154 narrative-review count/date probe. +/// `review_date` is nullable, so an all-NULL review set yields `None` here. +#[derive(Debug, Clone, FromRow)] +struct ReviewMetaRow { + review_count: i64, + latest_review_date: Option, +} + /// Internal struct for eNPS query result #[derive(Debug, Clone, FromRow)] struct EnpsRow { @@ -318,6 +332,23 @@ pub async fn get_employee_context( .fetch_all(pool) .await?; + // #154: Narrative reviews — count + latest date only. The full text is + // deliberately not loaded here; the employee-context section is token-budgeted + // and this only needs to establish that a review history exists. + let review_meta: Option = sqlx::query_as( + r#" + SELECT COUNT(*) as review_count, MAX(review_date) as latest_review_date + FROM performance_reviews + WHERE employee_id = ? + "# + ) + .bind(employee_id) + .fetch_optional(pool) + .await?; + let (review_count, latest_review_date) = review_meta + .map(|r| (r.review_count.max(0) as usize, r.latest_review_date)) + .unwrap_or((0, None)); + // Get eNPS responses let enps_responses: Vec = sqlx::query_as( "SELECT score, survey_name, survey_date, feedback_text FROM enps_responses WHERE employee_id = ? ORDER BY survey_date DESC" @@ -416,6 +447,8 @@ pub async fn get_employee_context( latest_rating_cycle: ratings.first().map(|r| r.cycle_name.clone()), rating_trend, all_ratings, + review_count, + latest_review_date, latest_enps: enps_responses.first().map(|e| e.score), latest_enps_date: enps_responses.first().map(|e| e.survey_date.clone()), enps_trend, @@ -557,6 +590,34 @@ pub async fn get_employee_contexts( }); } + // 5) Batch: #154 narrative-review count + latest date, grouped per employee. + #[derive(FromRow)] + struct ReviewMetaRowWithEmpId { + employee_id: String, + review_count: i64, + latest_review_date: Option, + } + let reviews_query = format!( + r#"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_as::<_, ReviewMetaRowWithEmpId>(&reviews_query); + for id in employee_ids { + q = q.bind(id); + } + let review_rows: Vec = q.fetch_all(pool).await?; + let mut reviews_by_emp: std::collections::HashMap)> = + std::collections::HashMap::new(); + for r in review_rows { + reviews_by_emp.insert( + r.employee_id, + (r.review_count.max(0) as usize, r.latest_review_date), + ); + } + // Assemble in input-ID order, dropping IDs not found in basic_by_id. let mut out: Vec = Vec::with_capacity(employee_ids.len()); for id in employee_ids { @@ -572,6 +633,8 @@ pub async fn get_employee_contexts( let ratings = ratings_by_emp.get(id).cloned().unwrap_or_default(); let enps_responses = enps_by_emp.get(id).cloned().unwrap_or_default(); + let (review_count, latest_review_date) = + reviews_by_emp.get(id).cloned().unwrap_or((0, None)); let rating_trend = calculate_trend( &ratings.iter().map(|r| r.overall_rating).collect::>(), @@ -667,6 +730,8 @@ pub async fn get_employee_contexts( latest_rating: ratings.first().map(|r| r.overall_rating), latest_rating_cycle: ratings.first().map(|r| r.cycle_name.clone()), rating_trend, + review_count, + latest_review_date, all_ratings, latest_enps: enps_responses.first().map(|e| e.score), latest_enps_date: enps_responses.first().map(|e| e.survey_date.clone()), @@ -1189,4 +1254,98 @@ mod tests { .expect("empty batch"); assert!(empty_result.is_empty(), "empty input must return empty Vec"); } + + /// Regression test for #154: narrative reviews live in `performance_reviews`, + /// numeric scores in `performance_ratings`. The context module read only the + /// latter, so an employee with reviews-but-no-ratings surfaced zero + /// performance context and chat asserted "no review history" while Prep Brief + /// cited the same reviews. Exercises BOTH the single and batch query paths + /// against the real migrated schema. + #[tokio::test] + async fn reviews_without_ratings_are_counted_in_context() { + use sqlx::sqlite::{SqliteConnectOptions, SqlitePoolOptions}; + use std::time::Duration; + + let options = SqliteConnectOptions::new() + .filename(":memory:") + .create_if_missing(true) + .foreign_keys(true) + .busy_timeout(Duration::from_secs(5)); + let pool = SqlitePoolOptions::new() + .max_connections(1) + .connect_with(options) + .await + .expect("connect :memory: pool"); + crate::db::run_migrations_for_tests(&pool) + .await + .expect("run migrations"); + + sqlx::query( + r#" + INSERT INTO employees (id, email, full_name) VALUES + ('maya', 'maya@x.com', 'Maya Patel'), + ('noone', 'noone@x.com', 'No Reviews') + "#, + ) + .execute(&pool) + .await + .expect("insert employees"); + + // UNIQUE(employee_id, review_cycle_id) — two reviews need two cycles. + sqlx::query( + r#" + INSERT INTO review_cycles (id, name, cycle_type, start_date, end_date) VALUES + ('c1', '2025 Annual', 'annual', '2025-01-01', '2025-12-31'), + ('c2', '2026 H1', 'annual', '2026-01-01', '2026-06-30') + "#, + ) + .execute(&pool) + .await + .expect("insert cycles"); + + // Reviews only — deliberately NO performance_ratings rows for Maya. + sqlx::query( + r#" + INSERT INTO performance_reviews + (id, employee_id, review_cycle_id, manager_comments, review_date) VALUES + ('r1', 'maya', 'c1', 'Strong systems thinker.', '2025-12-01'), + ('r2', 'maya', 'c2', 'Led the redesign.', '2026-03-01') + "#, + ) + .execute(&pool) + .await + .expect("insert reviews"); + + // Single-employee path. + let ctx = get_employee_context(&pool, "maya") + .await + .expect("single-employee context"); + assert!( + ctx.all_ratings.is_empty(), + "fixture must have no ratings — that is the whole point of #154" + ); + assert_eq!(ctx.review_count, 2, "both narrative reviews must be counted"); + assert_eq!( + ctx.latest_review_date.as_deref(), + Some("2026-03-01"), + "latest_review_date must be MAX(review_date)" + ); + + // Batch path must agree with the single path. + let batch = get_employee_contexts(&pool, &["maya".to_string(), "noone".to_string()]) + .await + .expect("batch context"); + assert_eq!(batch[0].review_count, 2); + assert_eq!(batch[0].latest_review_date.as_deref(), Some("2026-03-01")); + + // An employee with no reviews must report zero, not a phantom count. + assert_eq!(batch[1].review_count, 0); + assert_eq!(batch[1].latest_review_date, None); + + let solo = get_employee_context(&pool, "noone") + .await + .expect("single-employee context for reviewless employee"); + assert_eq!(solo.review_count, 0); + assert_eq!(solo.latest_review_date, None); + } }