fix(insights): history dedup uses title fallback, limits after dedup#461
Conversation
- Empty subjectKey no longer collapses unrelated insights (P1) - Fetch 50 rows then dedup to 12 unique subjects (was LIMIT 12 before dedup) - Remove unused runId from select
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR fixes three issues from review #460: it prevents empty
Confidence Score: 3/5The core dedup fix is correct, but the early-exit guard in the dedup loop still misbehaves when insights carry description and rootCause fields. An org with fully-populated insights would see only 4-6 subjects in the history context instead of the intended 12, quietly degrading AI prompt quality on every run. apps/insights/src/prompts.ts — the dedup loop and its early-exit condition Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[DB query LIMIT 50 ordered by createdAt desc] --> B{rows empty?}
B -- Yes --> C[return empty string]
B -- No --> D[Build subjectCounts map using subjectKey OR title fallback]
D --> E[Loop over rows]
E --> F{lines.length >= RECENT_INSIGHTS_PROMPT_LIMIT?}
F -- Yes --> G[break]
F -- No --> H{key already in seen set?}
H -- Yes --> E
H -- No --> I[Add key to seen and push 1 to 3 lines per row]
I --> E
G --> J[Return formatted context block]
I --> J
Reviews (1): Last reviewed commit: "fix(insights): history dedup uses title ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const seen = new Set<string>(); | ||
| const lines: string[] = []; | ||
| for (const row of rows) { | ||
| if (seen.has(row.subjectKey)) { | ||
| if (lines.length >= RECENT_INSIGHTS_PROMPT_LIMIT) { | ||
| break; | ||
| } | ||
| const key = row.subjectKey || row.title; | ||
| if (seen.has(key)) { | ||
| continue; | ||
| } | ||
| seen.add(row.subjectKey); | ||
| seen.add(key); |
There was a problem hiding this comment.
lines.length, so insights with description and rootCause don't erroneously reduce the number of distinct subjects shown. The PR description explicitly calls for "12 unique subjects" but the current guard measures output lines.
| const seen = new Set<string>(); | |
| const lines: string[] = []; | |
| for (const row of rows) { | |
| if (seen.has(row.subjectKey)) { | |
| if (lines.length >= RECENT_INSIGHTS_PROMPT_LIMIT) { | |
| break; | |
| } | |
| const key = row.subjectKey || row.title; | |
| if (seen.has(key)) { | |
| continue; | |
| } | |
| seen.add(row.subjectKey); | |
| seen.add(key); | |
| const seen = new Set<string>(); | |
| const lines: string[] = []; | |
| let subjectCount = 0; | |
| for (const row of rows) { | |
| if (subjectCount >= RECENT_INSIGHTS_PROMPT_LIMIT) { | |
| break; | |
| } | |
| const key = row.subjectKey || row.title; | |
| if (seen.has(key)) { | |
| continue; | |
| } | |
| seen.add(key); | |
| subjectCount++; |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
Summary
Fixes three issues from PR #460 review:
Test plan
bun testin apps/insightsSummary by cubic
Fix insight history dedup to prevent empty subjectKey from merging unrelated insights, and apply the 12-item cap after dedup by fetching 50 rows first. Remove the unused runId from the query and clean up HTML spacing in
packages/evals/ui.Written for commit 50b1ddc. Summary will update on new commits.
Review in cubic