Match triage verdicts to findings by index, not by title#119
Open
thejesh23 wants to merge 1 commit into
Open
Conversation
triageBatch mapped each model verdict back to a finding with
`batch.find((b) => b.finding.title === verdict.title)`. Finding titles
are free-text and not unique — generic titles ("Missing authorization
check", "SQL injection") recur across the up-to-30 findings in a batch.
`find` returns the first title match, so same-titled findings are all
assigned the first one's verdict: earlier ones get overwritten, later
ones are never triaged, and the P0/skip counts are mis-attributed.
Emit the 1-based index already shown in the prompt as an `id` field and
correlate on it, falling back to title only when the model omits the id.
An applied-set guard prevents a duplicate id/title in the reply from
assigning one finding twice (and, unlike a `finding.triage`-presence
guard, it doesn't break `--force` re-triage).
Adds a regression test: two same-titled findings receive two distinct
verdicts, and both are triaged.
|
@thejesh23 is attempting to deploy a commit to the Vercel Labs Team on Vercel. A member of the Team first needs to authorize it. |
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
1 participant
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.
What changed
triageBatchnow correlates each model verdict to the finding it judged by the 1-based ID already shown in the prompt, instead of byfinding.title. The prompt emits an- **ID:**line and asks the model to echo it in each verdict; the parse loop matches on that id, falling back to title only when the id is missing, with an applied-set guard so a duplicate id/title in the reply can't assign one finding twice.Why
Finding titles are free-text and not unique — generic ones ("Missing authorization check", "SQL injection") recur across the up-to-30 findings in a batch. The old
batch.find((b) => b.finding.title === verdict.title)returned the first title match, so same-titled findings collapsed onto one: earlier verdicts were overwritten, later findings were never triaged, and the P0/P1/P2/skip counts were mis-attributed.The applied-set guard is used instead of a
finding.triage-presence check on purpose: with--force, findings retain a prior run'striage, so a presence check would skip re-triage entirely.Fixes #118
Verification
pnpm testpasses (added a regression test: two same-titled findings receive two distinct verdicts and both are triaged; it fails onmainand passes with this change)pnpm lintpassespnpm knippassesAlso ran
pnpm -r build(typecheck) andpnpm test:bundle— both green.Notes for reviewer
Behavioral change is limited to
packages/processor/src/triage.ts. Theidfield is additive and optional onTriageVerdict; if a model omits it the code falls back to the previous title-matching, so this degrades gracefully rather than hard-failing.