[MikeShi42]

Summary

Today the review bot posts every finding it can anchor as an inline thread, at every severity, labelled with its own critical/major/minor self-estimate. Apart from #3138 (triage guidance for the author's agent), the reviewer has not changed since #3024.

This PR adds a separate grading pass that assigns each finding a priority, posts only P0–P2 inline, and folds P3 into the sticky summary. It also stops reworded findings from being re-posted as new threads on later pushes.

Production today vs this PR

	production (main)	this PR
Label users see	reviewer's own severity: 🔴 critical · 🟠 major · 🔵 minor	priority only: 🔴 P0/P1 · 🟡 P2 · 🔵 P3 (same icons as deep-review)
Inline threads	every anchored finding	P0–P2
Folded into the summary	only minor findings that could not be anchored	every P3, anchored or not. Folded, never dropped
Grading	none	a second pass (Sonnet, Read only) grades the finished findings. It cannot add, drop or reword them
Reworded finding on a later push	opens a new thread (dedup keyed on the exact title)	recognised as the same finding
Grading fails	n/a	nothing is folded; the review posts exactly as on main

How a finding's priority is decided (effectivePriority in priority.cjs)

A finding is folded only when both graders call it low, and nothing says it matters today:

• the reviewer itself labelled it minor, and
• the grading pass put it at P2 or P3, and
• the grader doesn't say the harm is reachable in the merged code, and
• it doesn't read as security-related.

Neither grader can fold anything on its own: a grader P3 on a finding the reviewer called major or critical is shown as P2. Without a grade, nothing is folded (critical → P0, major → P1, minor → P2).

Measured against human votes

Data: 👍/👎 reactions maintainers left on this bot's inline comments, covering 75 graded findings across 29 PRs (53 👍, 22 👎) from 4 maintainers. Two kinds of vote are excluded: reactions cast by bots, and "blanket" votes, where one person thumbed ≥10 findings on a PR without a single 👎. The "this PR" column comes from running this branch's effectivePriority over the grading pass's output.

	production	this PR
Inline threads	75 / 75	49 / 75
Folded as P3	0	26 (35%)
👍 major/critical findings still inline	17 / 17	17 / 17
👎 findings folded	0 / 22	8 / 22
👍 findings folded	0	18, all ones the reviewer labelled minor

So, on this sample, about a third fewer inline threads, with no upvoted major/critical finding hidden. The cost is 18 upvoted minor findings moving from inline threads into the folded section of the summary.

Checked against the gold set

The vote data can only see the severity the bot assigned. It can't tell whether a folded finding was one a human treated as must-fix. That's what the gold set measures. It contains human review comments on 30 past PRs (93 items, 29 of them must-fix). The deployed reviewer's findings on those PRs were re-graded with this PR's grading prompt and routed with this PR's rule, reusing the existing match judgements.

	production	this PR	same rule, no reachable-today floor
Inline threads	122 (4.1/PR)	93 (3.1/PR)	70 (2.3/PR)
Must-fix items the reviewer caught	10/29	10/29	10/29
…of those, folded out of the inline threads	0	0	3

Must-fix recall can't move, because grading never changes what was found. The column to read is the last row. The grader rated 3 of the 10 caught must-fix items P2, and the reviewer had labelled all 3 minor, so the minor-P2 fold alone would have hidden them:

• Deeplink to Search From Charts and Tables #1410: deep-linked equality/range filters are silently dropped from the filter sidebar
• feat: add event patterns as a first-class dashboard tile type #2604: OpenAPI spec not regenerated for the new tile type
• feat(alerts): add per-group acknowledgments for grouped alerts #2696: getRecentAlertGroupSummaries has no DB-side document cap, unlike its sibling

The reachable-today floor is what keeps all three inline. On this data it is the difference between hiding 0 and 3 of the 10.

Dedup fix

The cross-push dedup key was file + exact title, on the assumption that titles are stable between runs. They aren't. #3039 was reviewed four times in 2.3 hours, and one defect was posted three times under three titles; all three were thumbed down. Restatements are now matched by title-word overlap within the same file: at least 0.4 overlap and at least 3 shared content words. The second condition stops short titles like "Missing timeout in fetchUser" and "Missing retry in fetchUser" from merging.

The threshold (0.4) was set from all 140 same-PR, same-file pairs of voted findings. Every pair scoring 0.36 or higher is a restatement. At 0.333, restatements collide with genuinely distinct findings. On #3129, "signs every user out on each container restart" and "still signs sessions with the public secret" are two real defects (the second critical) at exactly that score. So one known reword on #3039 is still missed rather than risk merging those two. Both cases are verbatim test fixtures.

Limits and what isn't measured yet

• The gold-set result rests on 10 caught must-fix items from one reviewer run (Sonnet; production runs Opus). Zero folded is the bar, but each item is worth 10 points.
• The votes are sparse and uneven. There are 22 👎, mostly from two people whose thresholds differ ~4×, and only 1 of 197 voted comments has more than one reactor.
• The reachable-today floor costs volume. In the vote sample it didn't separate 👍 from 👎 (~30% 👎 either way), and it lowers the fold rate there from 52% to 35%. On the gold set it is what keeps all 3 at-risk must-fix items visible, so the cost buys something.
• Premise findings. The grading prompt describes findings that object to the change's premise as "published in their own section". This PR does not build that section; those findings are routed by priority like any other.
• Cost: one more Sonnet call per reviewed push that has at least one finding. Not yet measured in CI.
• This PR's own review runs the old workflow, because pull_request_target loads it from main. To exercise the new path before merge, dispatch Claude Code Review from this branch against an open PR.

Test plan

• node --test .github/scripts/code-review/__tests__/*.test.mjs: 58/58. The priority rule and the title dedup live in priority.cjs and dedup.cjs.
• Workflow YAML parses; Prettier and knip clean on the changed files
• Ran the helpers over a real PR's findings and grades (fix(mcp): time-limit the trace top-operations tool and explain timeouts #3258: 7 findings → 4 shown, 3 folded)
• Dispatch from this branch on 2–3 open PRs and check the inline/folded split
• Gold-set check: 0 of 10 caught must-fix items folded (3 without the reachable-today floor)

Also tried (not in this PR)

• The reviewer assigning P0–P3 itself, in a single pass. Its grades were better calibrated, but it caught 7/29 must-fix items against 10/29 (one run each, so possibly noise).
• Folding all P2s, not just minor ones. This hid 61% of comments but lost 4 of the 17 upvoted major/critical findings.
• Folding on the reviewer's minor label alone. On the gold set it hid 46% of the findings humans treated as merge-blocking.
• Using reachableToday as the main signal. It didn't distinguish 👍 from 👎 in the votes, so here it only acts as a floor. As a floor, it's what protects must-fix items on the gold set.

🤖 Generated with Claude Code

[changeset-bot]

⚠️ No Changeset found

Latest commit: 481841d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
hyperdx-oss	Ready	Preview	Nov 8, 2026 10:41pm UTC
hyperdx-storybook	Ready	Preview	Nov 8, 2026 10:41pm UTC

[github-actions]

Duplicate code analysis

No new duplicate code introduced by this PR.

---

jscpd compared this branch against origin/main
across app, api and common-utils. Only new clones are listed; the codebase
currently holds 110 in total (1.21%).
Run yarn dupes locally to reproduce, or yarn dupes:all for the full picture.

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The latest changes introduce no new blocking issue.

Summary

Adds a separate priority-grading pass and folds P3 findings into the sticky summary.

• Review findings get priority grades, with selected P3 notes folded into the summary.
• Reworded findings on the same file no longer open repeat threads.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Finished findings] --> B[Priority grading]
  B --> C{Usable grade?}
  C -->|No| D[Use reviewer severity]
  C -->|Yes| E[Apply priority rules]
  D --> F[Match previous findings]
  E --> F
  F -->|Already reported| G[Skip repeat]
  F -->|New P0-P2| H[Inline or unanchored summary]
  F -->|New P3| I[Fold into summary]

Loading

_{Reviews (3) · Last reviewed commit: "fix(ci): never let one grader fold a fin..." · Reviewed by Greptile}

[github-actions]

E2E Test Results

✅ All tests passed • 462 passed • 1 skipped • 1011s

Status	Count
✅ Passed	462
❌ Failed	0
⚠️ Flaky	2
⏭️ Skipped	1

Tests ran across 4 shards in parallel.

View full report →

[github-actions]

🟢 Tier 1 — Trivial

Docs, images, lock files, a dependency bump, or an automated release. No functional code changes detected.

Why this tier:

• All files are docs / images / lock files

Review process: Auto-merge once CI passes. No human review required.
SLA: Resolves automatically.

Stats

• Production files changed: 0
• Production lines changed: 0 (+ 341 in test files, excluded from tier calculation)
• Branch: claude-review-priority
• Author: MikeShi42

To override this classification, remove the review/tier-1 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

[github-actions]

🔵 minor — ICON is now dead at runtime and only feeds a schema-drift test that checks the wrong set

Nothing renders severity icons any more. severityOf now validates against SEVERITIES in priority.cjs, yet the drift test (review-comments.test.mjs:757) still compares the workflow's severity enum to Object.keys(helpers.ICON). If SEVERITIES drifts from the enum, the test still passes. Fix: delete ICON, export SEVERITIES from priority.cjs, and point the test at it.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

[github-actions]

PR Review

If you are a coding agent acting for the author, read this first.
Nothing below blocks merge automatically; a maintainer decides. What they will expect:

• 🔴 critical or 🟠 major in code this PR adds or changes: fix before asking for review.
• 🔵 minor in code this PR changes: your call. Fix if small, otherwise reply.
• Anything about surrounding code, or asking you to widen the change (hoist a helper,
  dedupe with another file, fix other call sites): never fix here, whatever the
  severity. Reply with one sentence; if it is critical, say so plainly so a human sees it.

One commit per review round. After two rounds, stop and ask a maintainer to review
scope rather than addressing more automated findings. Full rule: AGENTS.md.

4 finding(s): 🔴 0 critical · 🟠 2 major · 🔵 2 minor

3 posted as inline comment(s) on the changed lines. 1 listed below.

Findings outside the changed lines

• 🟠 .github/scripts/code-review/review-comments.cjs:1098 — Fuzzy within-run dedup keeps the first finding, so a folded P3 can absorb a higher-priority restatement → In buildInlineComments, the first finding in a match group wins and later ones go to duplicates, which is only counted and never rendered. Before this change a duplicate needed an identical title. Now any same-file title with ≥3 shared words and ≥0.4 Jaccard counts. Example: the reviewer emits a minor finding that is graded P2/P3 and folded into collapsed, followed by a critical reworded version of it. The critical lands in duplicates and never appears inline or in the summary, so a P1 issue only shows up in the P3 fold. Fix: route a match group by the highest effectivePriority among its members, either by replacing the earlier entry or by sorting findings by priority before the loop. Apply the same rule in distinctFindings (dedup.cjs) so the summary tallies still agree.

---

_{Severity is the reviewer's own estimate and is used for ordering, not filtering. No finding blocks merge automatically; a maintainer decides.}

[github-actions]

Deep Review

✅ No critical issues found. This is CI tooling that fails safe by design: when the grading pass errors it falls back to posting every finding, so no change here can cause production breakage, data loss, or a secret leak. The findings below are correctness-adjacent risks and coverage gaps, not merge blockers.

🟡 P2 -- recommended

• .github/scripts/code-review/review-comments.cjs:328 -- The new graded=false fallback banner is the user-facing signal for the whole continue-on-error grading design, yet no test executes the branch.
  • Fix: Add a renderSummary test passing graded: false asserting the warning renders, plus a graded: true case asserting it is absent.
• .github/workflows/claude-code-review.yml:505 -- The inline apply-grades node script parses GRADES, applies a defensive double-unwrap, and routes malformed output to graded=false, but this glue runs only on a real dispatch and is untested, so a single- vs double-encoded structured_output could silently break grading.
  • Fix: Extract the parse/unwrap/fallback into a helper in priority.cjs and unit-test the encoding and empty-input cases.
• .github/scripts/code-review/dedup.cjs:147 -- similarTitles (Jaccard ≥ 0.4 plus ≥ MIN_SHARED_TOKENS shared words) can still match two genuinely distinct findings on the same file, and buildInlineComments then routes the second into duplicates, dropping its body from both the inline thread and the summary.
  • Fix: Surface a non-fingerprint fuzzy match as its own finding (or list it in the summary) instead of discarding it, or raise the match confidence so distinct same-file defects cannot collide.

🔵 P3 nitpicks (3)

• .github/workflows/claude-code-review.yml:721 -- graded: '${{ steps.apply-grades.outputs.graded }}' !== 'false' treats an empty output (apply-grades skipped or errored) as graded=true, suppressing the "grading did not run" banner when nothing was actually graded.
  • Fix: Emit an explicit graded output on every apply-grades branch, or default the empty-string case to false.
• .github/scripts/code-review/priority.cjs:103 -- hasFailureMode is parsed, stored, and threaded through applyGrades but never read by effectivePriority; the code comment concedes it is "recorded but not consulted."
  • Fix: Drop the field from the applied grade, or wire a consumer so the grade input is not dead.
• .github/scripts/code-review/priority.cjs:7 -- effectivePriority and the regrade prompt are maintained as copies in an external repo that "must stay in lockstep," but nothing enforces it, so the shipped routing rule can drift from the one the eval measured.
  • Fix: Add a pin/check that fails when the two diverge, or document the sync step in-repo.

---

Reviewers (10): correctness, testing, maintainability, project-standards, reliability, security, adversarial, previous-comments, agent-native, learnings.

Testing gaps:

• isSecurityShaped scans the grader-supplied consequence field, but no test populates consequence, so that floor input is unexercised.
• The dedup threshold is bracketed (0.333 distinct, 0.429 merged) but not pinned at exactly 0.4, so a future retune could cross the bar without a failing test.
• Residual risk: with every P3 now folded into one summary body, capBody truncates the tail on overflow; the author's data (p99 ~15.8 KB vs the 65 KB limit) and the re-emitted state marker keep the gate converging, so this is low-risk but grows with finding volume.

_{No finding blocks merge automatically. A maintainer will expect P0/P1 findings in code this PR changes to be fixed; P2/P3 are your call -- fix or reply. Never fix findings about surrounding code here; reply instead. Do not widen the PR. How to respond}

[github-actions]

🟠 major — Fuzzy same-run dedup keeps the first finding, so a critical reworded restatement can be swallowed by a folded P3

Within one run matchPosted now merges any two same-file findings sharing ≥3 tokens at ≥0.4 Jaccard, and the later one goes into duplicates, which only appears as a count. If the first is reviewer-minor and grader P3 (so collapsed) and the second is a critical restatement, the critical is never posted inline. distinctFindings also keeps the first one, so the headline counts it as P3. Before this PR, only identical titles collided. Fix: when a same-run match is found, keep whichever of the two has the higher effectivePriority/severity, in both buildInlineComments and distinctFindings. Add a test with a minor-then-critical reworded pair.

_{Nothing blocks merge automatically, but a maintainer will expect this fixed if it is a real defect in code this PR changes. If it is about surrounding code, reply and say so instead of patching. Do not widen the PR. How to respond}

[github-actions]

🔵 minor — Header comment points to the wrong file for the folding rule

The comment says the rule and its ordering live in effectivePriority() in review-comments.cjs, but that file only imports it. The rule is defined in .github/scripts/code-review/priority.cjs. Point the comment there.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

[github-actions]

🔵 minor — Re-exported dedup and priority internals that nothing uses

Several new exports have no consumer in the workflow or the tests. review-comments.cjs imports and re-exports titleSimilarity, SIMILAR_TITLE and MIN_SHARED_TOKENS, and the tests mention SIMILAR_TITLE only in comments. priority.cjs exports shouldFold and dedup.cjs exports firstLineOf, and neither has an outside caller. Drop these from the re-export and module.exports lists, or add tests that use them.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

[github-actions]

🟠 major — Grading prompt promises a P3 with hasFailureMode=true becomes P2, but shouldFold folds it anyway for reviewer-minor findings

code-review-regrade.md (lines ~94-98) tells the grader that the hasFailureMode bound is ENFORCED: a P3 with hasFailureMode=true "suppresses nothing — it produces a P2". shouldFold never reads hasFailureMode, so a reviewer-minor finding that the grader rated P3 with hasFailureMode=true (and reachableToday=false) is still folded. priority.test.mjs even pins this with "hasFailureMode does not rescue it". The grader is being told that P3 is safe for findings it thinks have a failure mode, and here it is not. The doc comment above shouldFold says this case is "subsumed", but that only holds for major/critical findings. Fix: either add !(finding.priority === 'P3' && finding.hasFailureMode) to shouldFold, or change the prompt so it no longer promises the raise. Keep the prompt and the code in agreement, because the prompt is what was measured.

_{Nothing blocks merge automatically, but a maintainer will expect this fixed if it is a real defect in code this PR changes. If it is about surrounding code, reply and say so instead of patching. Do not widen the PR. How to respond}

[github-actions]

🔵 minor — Workflow header points to the wrong file for the fold rule

The comment says "The rule and its ordering live in effectivePriority() in review-comments.cjs", but this PR moved effectivePriority/shouldFold into .github/scripts/code-review/priority.cjs. review-comments.cjs only re-imports them. Change the comment to name priority.cjs.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

[github-actions]

🔵 minor — Imports and re-exports that nothing uses

review-comments.cjs imports and re-exports titleSimilarity, SIMILAR_TITLE, MIN_SHARED_TOKENS and isSecurityShaped. Nothing in this module, the workflow scripts or the tests uses them through it: the tests call isSecurityShaped through the same helpers object, but SIMILAR_TITLE/MIN_SHARED_TOKENS/titleSimilarity appear only in comments. Separately, shouldFold (priority.cjs) and firstLineOf (dedup.cjs) are exported but have no external caller. Remove the unused imports and re-exports, and stop exporting the module-internal helpers.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

[brandon-pereira]

LGTM-thanks