chore: resolve issue #118 — skip LLM auto-extraction for sample data#120
Merged
Merged
Conversation
import_reviews_bulk auto-extracts review highlights for every inserted review. During fresh-install onboarding this fires ~261 sequential LLM calls over the bundled 100-employee sample dataset. On a key-less Mac each fails fast and is invisible; but when a provider key exists in the keychain (returning user, reinstall, dev machine), onboarding silently spends real API credits and the progress bar freezes at 75% for 15-20 min. Fix: exclude reviews belonging to sample-flagged employees (is_sample = 1, migration 014) from auto-extraction and summary regeneration. Sample insights aren't worth real tokens and the demo works without them. - Track (review_id, employee_id) per inserted review so sample data can be filtered before the token-spending extraction step. - New reviews_eligible_for_extraction / fetch_sample_employee_ids helpers; fetch_sample_employee_ids fails open (empty set on query error → prior behavior) so a query failure never drops real users' reviews. - New test sample_employee_reviews_are_excluded_from_auto_extraction proves only the non-sample employee's review is queued (the no-key environment yields zero highlights either way, so the test asserts the extraction *decision*, not the side-effect). Full suite: 788 passed, 0 failed. No new dependencies. One file changed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR prevents token-spending LLM highlight extraction (and related summary regeneration) from running on bundled sample onboarding data by filtering out reviews belonging to sample-flagged employees (employees.is_sample = 1) during import_reviews_bulk.
Changes:
- Track inserted reviews as
(review_id, employee_id)pairs to enable per-review sample filtering. - Add helpers to fetch sample employee IDs and compute which inserted review IDs are eligible for extraction.
- Add a regression test asserting that only non-sample reviews are queued for extraction.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+336
to
+342
| if !inserted_reviews.is_empty() { | ||
| let review_ids = reviews_eligible_for_extraction(pool, &inserted_reviews).await; | ||
| let sample_ids = fetch_sample_employee_ids(pool).await; | ||
| let employee_ids: Vec<String> = affected_employee_ids | ||
| .into_iter() | ||
| .filter(|emp_id| !sample_ids.contains(emp_id)) | ||
| .collect(); |
Comment on lines
+383
to
+393
| async fn reviews_eligible_for_extraction( | ||
| pool: &DbPool, | ||
| inserted_reviews: &[(String, String)], | ||
| ) -> Vec<String> { | ||
| let sample_ids = fetch_sample_employee_ids(pool).await; | ||
| inserted_reviews | ||
| .iter() | ||
| .filter(|(_, emp_id)| !sample_ids.contains(emp_id)) | ||
| .map(|(review_id, _)| review_id.clone()) | ||
| .collect() | ||
| } |
Comment on lines
+757
to
+758
| let eligible = reviews_eligible_for_extraction(&pool, &inserted).await; | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Auto-generated by portfolio-orchestrator nightly run on 2026-06-13 (live mode). Resolves #118.
Problem
import_reviews_bulkauto-extracts review highlights for every inserted review. Fresh-install onboarding loads the bundled 100-employee sample dataset through this path → ~261 sequential LLM calls. Key-less Macs fail fast (invisible), but when a provider key exists in the keychain, onboarding silently spends real API credits and the progress bar freezes at 75% for 15–20 min (caught live in 2026-06-11 fresh-install QA: ~106 real Claude calls before the app was killed).Fix
Exclude reviews belonging to sample-flagged employees (
is_sample = 1, migration 014) from both highlight auto-extraction and summary regeneration.(review_id, employee_id)per inserted review so sample data can be filtered before the token-spending step.reviews_eligible_for_extraction/fetch_sample_employee_idshelpers.fetch_sample_employee_idsfails open (empty set on query error → prior behavior) so a query failure never drops real users' reviews.sample_employee_reviews_are_excluded_from_auto_extraction. Note: the no-API-key test environment yields zero highlights regardless of the fix, so the test asserts the extraction decision (which reviews get queued), not the downstream side-effect — a true RED→GREEN.Verification
cargo testfull suite: 788 passed, 0 failed (+1 new test).origin/main(the pre-existingtrial.rs:332tautological-assert error is on origin/main, untouched).do-not-touch(recruiting/,proxy/) untouched.🤖 Generated with Claude Code