fix(context): surface review narratives that carry no numeric rating (#154) - #157
fix(context): surface review narratives that carry no numeric rating (#154)#157matthewod11-stack wants to merge 1 commit into
Conversation
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) <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
There was a problem hiding this comment.
Pull request overview
Fixes a gap in the chat context builder where employees with prose performance reviews but no numeric ratings were treated as having “no performance data,” by adding explicit review presence metadata and rendering it independently of the ratings block.
Changes:
- Extend
EmployeeContextwithreview_countandlatest_review_date, retrieved fromperformance_reviewsin both single and batched context fetches. - Update prompt formatting to include a “Performance reviews” line even when
all_ratingsis empty. - Add/extend unit tests to cover “reviews-only” employees and singular/plural/date rendering.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src-tauri/src/context/retrieval.rs |
Adds review presence fields to EmployeeContext and fetches them via an additional batched query against performance_reviews. |
src-tauri/src/context/prompt.rs |
Renders review presence independent of ratings and adds tests for reviews-only employees and formatting rules. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// `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<String, (usize, Option<String>)> { | ||
| if employee_ids.is_empty() { | ||
| return std::collections::HashMap::new(); | ||
| } | ||
|
|
||
| let placeholders = employee_ids.iter().map(|_| "?").collect::<Vec<_>>().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 | ||
| ); |
| 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 | ||
| )); |
|
Closing as a duplicate — my mistake. PR #156 (opened 2026-07-16) already resolves #154 and is green across all 7 CI checks and mergeable; this PR is a second, redundant implementation of the same fix. Cause: the orchestrator's stale-resolved check only searches merged PRs for a resolving match, so #154 still looked like an unclaimed candidate tonight despite having an open PR from last night. Filing that gap in tonight's digest. Review #156, not this. No action needed here. |
Auto-generated by portfolio-orchestrator nightly run on 2026-07-17. Resolves #154.
What was wrong
src-tauri/src/context/readperformance_ratingsonly — confirmed: zero references toperformance_reviewsanywhere in the module. The whole performance block informat_single_employee_with_budgetis gated onif !emp.all_ratings.is_empty(), and thecareer_summary/key_strengths/recent_highlightsfields are populated from extracted highlights, not the raw narratives.So an employee reviewed in prose but never scored hit every empty path at once and their context rendered nothing about performance. The model then said "no performance data" — accurately reporting what it was handed — while Prep Brief (
people_map/context.rs, which callsget_reviews_for_employeedirectly) rendered ~18 facts from those same rows.What changed
retrieval.rs:review_count+latest_review_dateonEmployeeContext, sourced fromperformance_reviewsdirectly. Batched as a 5th IN-clause query inget_employee_contexts(no N+1); a query failure degrades to "no reviews known" rather than blanking the rest of the context.prompt.rs: renders the review line independently of the ratings block — that gating is the actual bug.2 files, +170/-4. No new dependencies.
people_map/andrecruiting/untouched per the issue's do-not-touch.Verification
cargo test --manifest-path src-tauri/Cargo.toml→ 858 passed, 0 failed, 2 ignoredThe issue's second verification step — seed a reviews-only employee and ask chat — needs a live model call and was not run here. Worth doing before merge.
Decision left open
The issue asks to decide the inclusion shape: full narratives vs. summary + acknowledgment vs. FTS excerpts. This PR implements the stated minimum (say the reviews exist, with count + latest date) and stops there. The richer shapes change what the assistant tells someone about their own performance data and how much of the token budget that claims — a product call, not a mechanical one, so it's left for you.