feat(eval): label findings by recorded decision and match gold globally (#753)

* feat(eval): key finding gold on the recorded decision and match gold globally

Labeling a review finding used to key on whether a later review round still
raised it, which conflates the two decisions that matter: a finding the human
chose to fix and a finding they chose to ship both disappear from later rounds.
Gold now follows the round's own recorded fix-vs-skip decision plus the source
run's merge state. Selecting a finding for fix on a merged run is true-positive
gold even if a later round re-raised or rewrote it; leaving one unselected on a
merged run is shipped-unfixed false-positive gold; a round whose gate decision
was never recorded stays unlabeled.

The false-positive half deliberately reverses the earlier "never auto-label FP
from a skip" stance: in this operator's corpus, a finding they approve and ship
without fixing is a false positive. It still needs both halves - a recorded
decision and the merge - and no-op findings are never labeled.

Scoring replaces per-strength-tier greedy assignment with one globally optimal
bipartite matching over all gold and candidate findings, weighted so an exact
match outweighs any number of fuzzy ones. The tiered matcher could hand a
candidate to a gold that had alternatives and strand a gold that had none,
understating recall for reasons unrelated to the review under test.

* no-mistakes: apply CI fixes

AGENTS.md

@@ -212,8 +212,8 @@ Safest local verification sequence after non-trivial changes:
 - Collection is automatic and default-on through `eval.capture_provenance` / `eval.auto_capture` / `eval.max_cases` / `eval.diversified_size` in `config.yaml`, never an environment variable: the daemon's launchd/systemd unit is re-rendered on install and update and preserves only proxy variables (`internal/daemon/service.go` `proxyEnvKeys`), so an env-gated corpus silently stops collecting after an update. The keys are global-only - `Merge` copies them straight from `GlobalConfig`, and an `eval` block in a repo's `.no-mistakes.yaml` is ignored.
 - Provenance is unrecoverable: `executor.go` writes it with the review round or never. A round recorded with `capture_provenance` off can never be captured, so the rejection names the setting rather than the round's age.
 - The trigger is `RunManager.autoCaptureEvalCase`, called last in the run goroutine after the outcome is already reported: it recovers its own panic (the enclosing recover would otherwise mark a finished run failed), bounds itself with `evalAutoCaptureTimeout` off the run context, serializes runs on `evalCaptureMu` (shared pool + registry), and logs rather than propagates. `ErrNoCapturableReview` separates "nothing to freeze" (DEBUG) from a real fault (WARN). Automatic and manual capture call the same `eval.Capture`. A merged PR also best-effort relabels already-captured cases via `RunManager.relabelEvalRun` (same mutex/timeout); `eval relabel` is the CLI path.
-- The unit of truth is finding-level gold, not park/pass: a user-selected Fix is true-positive gold (no merge required); an auto-fix that landed in a merged PR is true-positive gold; a raised `auto-fix`/`ask-user` finding shipped unfixed in a merged PR is false-positive gold; a human-added finding is false-negative gold; skip/approve stay unlabeled / pending; unmatched candidate findings stay queued - never inferred as false positives - and a confirmed post-PR miss ingested via `eval miss ingest` is also false-negative gold (`recorded-post-pr-miss`). Owner: `internal/eval` (`goldFromRound`, `IngestPostPRMiss`, `ScoreCandidate`); user-facing language is `docs/src/content/docs/reference/eval.md`.
-- `diversified` is gold-only and pinned (empty gold -> empty set + `eval sets` warning, never unlabeled fill). Those pins are the held-out official set; leftover labeled cases are `tune`. ListCases trims pins to the live `eval.diversified_size` cap (at most one per stratum when reconciling to 0 or a lower cap); `RefreshDiversified` is only for an explicit rebuild. Never fit matcher thresholds or review prompts on `diversified`. Report F1 as the headline metric only when false-positive gold exists; otherwise recall + precision bounds. Shipped-unfixed FP looks at the last later review round, and RelabelRun drops obsolete derived merge labels. Matcher assignment is maximum matching per strength tier (exact before fuzzy). Regressions: `TestListCasesDiversified_*`, `TestCaptureWritesAutoFixMergedAsTruePositive`, `TestCaptureWritesShippedUnfixedAsFalsePositive`, `TestCaptureDoesNotWriteShippedUnfixedWhenIntermediateRoundThenFinalRoundDropsIt`, `TestRelabelRemovesShippedUnfixedWhenLaterRoundShowsTheFindingLanded`, `TestMergeGoldClearsStoredShippedUnfixedWhenRecomputedUnlabeled`, `TestRelabelClearsStoredShippedUnfixedFPWhenRecomputedUnlabeled`, `TestScoreCandidateDoesNotLetFuzzyEarlierGoldStealExactLaterMatch`, `TestEvaluationSummaryWithholdsHeadlineF1WithoutFalsePositiveGold`, `TestCaptureDoesNotLabelSkipOrApproveAsPass`, `TestCaptureWritesFalseNegativeGoldForUserAddedFinding`, `TestCaptureSkipsIncompleteReviewRoundAndKeepsCompletedSibling`, `TestIngestPostPRMissWritesFalseNegativeGoldOnGreenReview`, `TestCaptureAndReport*`, CLI `TestEvalCaptureAndSetsSpeakInFindingGoldTerms`, `TestEvalMissIngestLabelsFalseNegativeGold`.
+- The unit of truth is finding-level gold, not park/pass, and it is keyed on the round's **recorded fix-vs-skip decision** plus merge state, never on whether a later round still raises the finding (a fix and a ship both make it disappear): a user-selected Fix is true-positive gold (no merge required); an auto-fix selection on a merged run is true-positive gold even if a later round re-raised or rewrote it; a raised `auto-fix`/`ask-user` finding the human did NOT select, on a merged run, is false-positive gold - deliberately reversing the older "never auto-FP from a skip" stance, because in this operator's corpus an approved-and-shipped finding IS a false positive; a human-added finding is false-negative gold; skip/approve without a merge and any round with no recorded decision (unknown/aborted) stay unlabeled / pending; `no-op` findings are never labeled; unmatched candidate findings stay queued - never inferred as false positives - and a confirmed post-PR miss ingested via `eval miss ingest` is also false-negative gold (`recorded-post-pr-miss`). Owner: `internal/eval` (`goldFromRound`, `hasRecordedDecision`, `IngestPostPRMiss`, `ScoreCandidate`); user-facing language is `docs/src/content/docs/reference/eval.md`.
+- `diversified` is gold-only and pinned (empty gold -> empty set + `eval sets` warning, never unlabeled fill). Those pins are the held-out official set; leftover labeled cases are `tune`. ListCases trims pins to the live `eval.diversified_size` cap (at most one per stratum when reconciling to 0 or a lower cap); `RefreshDiversified` is only for an explicit rebuild. Never fit matcher thresholds or review prompts on `diversified`. Report F1 as the headline metric only when false-positive gold exists; otherwise recall + precision bounds. RelabelRun recomputes derived merge labels and drops the obsolete ones. Matcher assignment is ONE globally optimal bipartite matching over all gold and candidate findings, weighted so an exact match outweighs any number of fuzzy ones; per-strength-tier greedy assignment understated recall and must not come back. Regressions: `TestListCasesDiversified_*`, `TestGoldFromRoundLabelsByRecordedDecision`, `TestCaptureWritesAutoFixMergedAsTruePositive`, `TestCaptureWritesShippedUnfixedAsFalsePositive`, `TestCaptureWritesShippedUnfixedEvenWhenTheFinalRoundNoLongerRaisesIt`, `TestCaptureLabelsSelectedAutoFixAsTruePositiveEvenWhenLaterRoundReRaisesIt`, `TestRelabelReplacesShippedUnfixedWhenTheRoundLaterRecordsAFixDecision`, `TestMergeGoldClearsStoredShippedUnfixedWhenRecomputedUnlabeled`, `TestRelabelClearsStoredShippedUnfixedFPWhenRecomputedUnlabeled`, `TestScoreCandidateDoesNotLetFuzzyEarlierGoldStealExactLaterMatch`, `TestScoreCandidateRecoversMatchTheTieredMatcherLost`, `TestMaxWeightAssignmentMatchesBruteForceOptimum`, `TestEvaluationSummaryWithholdsHeadlineF1WithoutFalsePositiveGold`, `TestCaptureDoesNotLabelSkipOrApproveAsPass`, `TestCaptureWritesFalseNegativeGoldForUserAddedFinding`, `TestCaptureSkipsIncompleteReviewRoundAndKeepsCompletedSibling`, `TestIngestPostPRMissWritesFalseNegativeGoldOnGreenReview`, `TestCaptureAndReport*`, CLI `TestEvalCaptureAndSetsSpeakInFindingGoldTerms`, `TestEvalMissIngestLabelsFalseNegativeGold`.
 - A case stores no Git bundle. Bundles were a full history copy per review pass (~8 MB each here) and cannot be trimmed, because a bundle built with negative refs records prerequisites an empty restore gate lacks. Cases of one repository instead share `<NM_HOME>/eval/pools/<fingerprint>.git`, pinned by `refs/no-mistakes/eval/<caseID>/{head,source-head,base,trusted-config}`; the marginal case costs ~8 KB. `Store.Prune` applies `max_cases` oldest-first but protects active replay reservations and cases with recorded evaluations, so the cap is a retention target rather than a hard bound.
 - Capture stays read-only against the gate, so objects reach the pool through a throwaway bare clone plus a refspec fetch - never a bare-object-id fetch, whose want policy is off by default and version-dependent.
 - Regressions: `TestCaptureDoesNotCopyRepositoryHistoryPerCase`, `TestPruneBoundsTheCorpusOldestFirstAndKeepsEvaluatedCases`, `TestDropCaseObjectsReleasesOnlyItsOwnPins`, `TestAutoCaptureEvalCase*` (`internal/daemon`), `TestEvalDefaultsCollectWithoutSetup`, `TestRepoConfigCannotChangeEvalCollection`, e2e `TestEvalAutoCaptureJourney`.

docs/src/content/docs/reference/eval.md

@@ -54,17 +54,17 @@ The manifest never stores a remote URL. Capture is read-only against the existin
 
 The unit of truth is whether a review **finding** was a real issue, scored with scientific terms, not whether the run parked or passed.
 
-Capture writes gold from recorded gate evidence: human Fix and add-finding decisions, plus the merge-derived auto-fix and shipped-unfixed rules below. A merged PR is not a case-level pass or fail:
+Capture writes gold from the **recorded gate decision** for a review round - what the human chose to fix or ship - combined with the source run's merge state. It never keys a label on whether a later review round still happens to raise the finding, because a fixed finding and a shipped-unfixed finding both disappear from later rounds. A merged PR is still not a case-level pass or fail:
 
 - A finding the human selected for Fix (`selected_finding_ids` with a user source) is **true-positive** gold: that finding is a true issue. Merge is not required.
 - A finding the human added (`user_findings_json`, source `user`) is **false-negative** gold: the original review missed a real issue.
-- A finding the pipeline auto-fixed that later **landed in a merged PR** is **true-positive** gold (`recorded-auto-fix-merged`). Closed-not-merged, still-open, reverted, and superseded auto-fixes stay unlabeled.
-- A finding that was raised (`auto-fix` or `ask-user`, including a missing action that defaults to `ask-user`) and then **shipped unfixed in a merged PR** is **false-positive** gold (`recorded-shipped-unfixed`). If a later review round exists and the last of those rounds no longer raises the same issue, earlier rounds stay unlabeled - an intermediate re-raise that was gone before merge is a fix, not a false positive. Informational `no-op` findings are not labeled this way.
+- A finding the pipeline selected for auto-fix on a run whose PR **merged** is **true-positive** gold (`recorded-auto-fix-merged`): the decision to fix it is the evidence, so a fix a later round re-raised or rewrote is still labeled. Closed-not-merged and still-open runs stay unlabeled until the merge is observed.
+- A finding that was raised (`auto-fix` or `ask-user`, including a missing action that defaults to `ask-user`), **not selected for fix**, and then **shipped in a merged PR** is **false-positive** gold (`recorded-shipped-unfixed`). This is a deliberate operator judgement: a finding you approve and ship without fixing is a false positive in your own corpus. It needs both halves - a recorded gate decision for the round and the merge - and informational `no-op` findings are never labeled this way.
 - A confirmed post-PR miss ingested with `eval miss ingest` is also **false-negative** gold (`recorded-post-pr-miss`): review passed green, and a later vetted finding showed a real defect.
-- Skip, and approve-with-findings on an unmerged PR, stay **unlabeled / pending** until later adjudication.
+- Skip and approve-with-findings **without a merge** stay **unlabeled / pending** until later adjudication, and so does any round whose gate decision was never recorded (an unknown or aborted resolution), merged or not. Absence of a decision is never read as a judgement.
 - A later replay that raises a new issue absent from the gold set is queued as an unmatched candidate finding. It is never auto-scored as a false positive.
 
-If a PR merges after the first capture, already-captured cases are relabeled. The daemon does this best-effort when it observes the merge; `eval relabel [run-id]` or recapture is the CLI path. Relabel adds merge-derived labels onto previously unlabeled findings and drops obsolete derived merge labels that the current rounds no longer support. Adjudicated, user-fix, and ingested post-PR-miss labels are never overwritten.
+If a PR merges after the first capture, already-captured cases are relabeled. The daemon does this best-effort when it observes the merge; `eval relabel [run-id]` or recapture is the CLI path. Relabel adds merge-derived labels onto previously unlabeled findings and drops obsolete derived merge labels that the current recorded decisions no longer support. Adjudicated, user-fix, and ingested post-PR-miss labels are never overwritten.
 
 A case with no finding-level gold is unlabeled / pending, never a pass. True-negative also stays unlabeled because the current capture evidence cannot establish that a finding is invalid without the shipped-unfixed or adjudication paths above.
 
@@ -115,7 +115,7 @@ Replay scores each candidate finding against that gold:
 - **false-positive**: only when a candidate finding matches explicit false-positive gold (adjudicated invalid, or shipped-unfixed). Unmatched candidate findings are never treated as false positives
 - **pending / unlabeled**: unmatched candidate findings, and cases with no finding-level gold yet
 
-Matching is a documented cascade of strengths: the same finding ID, the same file and description after whitespace and case normalization, the same file with lines within 3 and token-Jaccard ≥ 0.5, then gated containment (same file, one normalized description contains the other, shorter side ≥ 8 tokens). Assignment is maximum matching per strength tier, preferring exact over fuzzy, so gold-label order cannot undercount. Headline recall uses the full cascade. Reports also show recall-if-exact-only so a fuzzy-threshold change is visible. File-less or description-less findings do not match on the text, location, or containment strengths.
+Matching is a documented cascade of strengths: the same finding ID, the same file and description after whitespace and case normalization, the same file with lines within 3 and token-Jaccard ≥ 0.5, then gated containment (same file, one normalized description contains the other, shorter side ≥ 8 tokens). Assignment is one globally optimal matching over every gold and candidate finding at once, ranked so an exact match outweighs any number of fuzzy ones, so neither gold-label order nor a tier boundary can undercount recall. Headline recall uses the full cascade. Reports also show recall-if-exact-only so a fuzzy-threshold change is visible. File-less or description-less findings do not match on the text, location, or containment strengths.
 
 The report prints recall, precision bounds (adjudicated vs pending-as-FP), and F1 as the headline metric **only when false-positive gold exists** so precision is real. Otherwise F1 is withheld rather than reported as recall-in-disguise.
 

internal/daemon/helpers_test.go

@@ -420,3 +420,30 @@ func waitForRunTerminalState(t *testing.T, d *db.DB, runID string) *db.Run {
 	t.Fatalf("run %s did not reach terminal state", runID)
 	return nil
 }
+
+// waitForDaemonReady blocks until the daemon started by RunWithOptions answers a
+// health probe. The singleton lock orders stale-run recovery (which performs
+// startup PR reconciliation) strictly before the socket bind and the first
+// health response, so a healthy daemon has already finished reconciling. Tests
+// that launch RunWithOptions in a goroutine and then wait for a recovered run to
+// go terminal must gate on this first: otherwise their terminal-state deadline
+// has to absorb the entire cold daemon startup, which is far slower on the
+// process-spawn-bound Windows runner and made the wait flaky.
+func waitForDaemonReady(t *testing.T, p *paths.Paths) {
+	t.Helper()
+
+	deadline := time.Now().Add(60 * time.Second)
+	for time.Now().Before(deadline) {
+		client, err := ipc.Dial(p.Socket())
+		if err == nil {
+			var result ipc.HealthResult
+			callErr := client.Call(ipc.MethodHealth, &ipc.HealthParams{}, &result)
+			_ = client.Close()
+			if callErr == nil && result.Status == "ok" {
+				return
+			}
+		}
+		time.Sleep(20 * time.Millisecond)
+	}
+	t.Fatalf("daemon at %s never became ready", p.Socket())
+}

internal/daemon/subscribe_recover_test.go

@@ -652,6 +652,7 @@ func TestRecoverOnStartup_ReconcilesHistoricalCIGateFromCurrentPRState(t *testin
 				}
 			}()
 
+			waitForDaemonReady(t, p)
 			completed := waitForRunTerminalState(t, d, run.ID)
 			if completed.Status != types.RunCompleted || completed.AwaitingAgentSince != nil {
 				t.Fatalf("historical CI gate after %s reconciliation = status %s awaiting %v", state, completed.Status, completed.AwaitingAgentSince)

internal/eval/capture.go

@@ -169,7 +169,7 @@ func Capture(ctx context.Context, store *Store, p *paths.Paths, database *db.DB,
 		return nil, fmt.Errorf("read source invocation metrics: %w", err)
 	}
 	captured := make([]Case, 0, len(reviewRounds))
-	for i, round := range reviewRounds {
+	for _, round := range reviewRounds {
 		if round.FindingsJSON == nil || strings.TrimSpace(*round.FindingsJSON) == "" {
 			// An interrupted or cancelled later round is not a replayable
 			// pass. Skip it so a completed sibling of the same run can still
@@ -177,7 +177,7 @@ func Capture(ctx context.Context, store *Store, p *paths.Paths, database *db.DB,
 			continue
 		}
 		decision := decisionForRound(round, reviewStep)
-		labels := goldFromRound(round, decision, runPRState(run), reviewRounds[i+1:])
+		labels := goldFromRound(round, decision, runPRState(run))
 		if !labels.HasGold() && (reviewStep.Status == types.StepStatusAwaitingApproval || reviewStep.Status == types.StepStatusFixReview) {
 			return nil, fmt.Errorf("%w: review round %q has no recorded gate decision", ErrNoCapturableReview, round.ID)
 		}
@@ -222,7 +222,7 @@ func Capture(ctx context.Context, store *Store, p *paths.Paths, database *db.DB,
 		caseID := run.ID + "-" + round.ID
 		caseDir := store.caseDir(caseID)
 		if existing, err := os.Stat(caseDir); err == nil && existing.IsDir() {
-			c, err := relabelExistingCase(store, caseDir, round, decision, runPRState(run), reviewRounds[i+1:])
+			c, err := relabelExistingCase(store, caseDir, round, decision, runPRState(run))
 			if err != nil {
 				return nil, fmt.Errorf("relabel existing case %q: %w", caseID, err)
 			}
@@ -452,8 +452,21 @@ func baselineForRound(invocations []db.AgentInvocation, round int) BaselineMetri
 	return baseline
 }
 
+// Recorded gate actions. These are the persisted spellings in decision.json;
+// decisionUnknown and decisionAbort record no fix-vs-skip choice, so they never
+// support a label.
+const (
+	decisionUnknown = "unknown"
+	decisionFix     = "fix"
+	decisionSkip    = "skip"
+	decisionApprove = "approve"
+	decisionAbort   = "abort"
+
+	prStateMerged = "merged"
+)
+
 func decisionForRound(round *db.StepRound, step *db.StepResult) Decision {
-	decision := Decision{Action: "unknown"}
+	decision := Decision{Action: decisionUnknown}
 	if round.SelectionSource != nil {
 		decision.SelectionSource = *round.SelectionSource
 	}
@@ -464,39 +477,54 @@ func decisionForRound(round *db.StepRound, step *db.StepResult) Decision {
 		decision.HasUserFindings = true
 	}
 	if len(decision.SelectedFindingIDs) > 0 {
-		decision.Action = "fix"
+		decision.Action = decisionFix
 		return decision
 	}
 	if step == nil {
 		return decision
 	}
 	if step.Status == types.StepStatusSkipped {
-		decision.Action = "skip"
+		decision.Action = decisionSkip
 		return decision
 	}
 	if step.Status == types.StepStatusFailed && step.Error != nil && strings.Contains(*step.Error, "aborted by user") {
-		decision.Action = "abort"
+		decision.Action = decisionAbort
 		return decision
 	}
 	if round.FindingsJSON != nil {
 		findings, err := types.ParseFindingsJSON(*round.FindingsJSON)
 		if err == nil && types.HasAskUserFindings(findings) && step.Status == types.StepStatusCompleted {
-			decision.Action = "approve"
+			decision.Action = decisionApprove
 		}
 	}
 	return decision
 }
 
-// goldFromRound writes labels the recorded gate evidence supports.
-// A user-selected finding is true-positive gold without merge. An auto-fix
-// selection is true-positive gold only when the source PR merged and the fix
-// landed. A raised auto-fix/ask-user finding that shipped unfixed in a merged
-// PR is false-positive gold. An intermediate re-raise that is gone from the
-// last later review round is treated as fixed, not shipped-unfixed. Skip,
-// approve-with-findings on an unmerged PR, reverted auto-fixes, and no-op
-// notes stay unlabeled. Confirmed post-PR misses are written later by
-// IngestPostPRMiss, not here.
-func goldFromRound(round *db.StepRound, decision Decision, prState string, later []*db.StepRound) Labels {
+// goldFromRound writes the labels the recorded gate DECISION supports.
+//
+// The key is what the human decided about a finding, never whether some later
+// review round still happens to raise it. Round presence conflated the two
+// decisions that matter: a finding the human chose to fix and a finding they
+// chose to ship both disappear from a later round, and an intermediate re-raise
+// that the final round dropped stayed unlabeled even though the gate recorded
+// exactly what was decided about it.
+//
+//   - Selected for fix with a user source: true-positive gold. Merge is not
+//     required - the human called it a real issue.
+//   - Selected for fix with an auto-fix source on a merged source run:
+//     true-positive gold.
+//   - Not selected for fix on a merged source run: false-positive
+//     "shipped unfixed" gold. This deliberately reverses the earlier principle
+//     that a skip is too ambiguous to auto-label: in this operator's corpus a
+//     finding they approve and ship without fixing IS a false positive. It
+//     still needs the merge - skip or approve without one stays unlabeled - and
+//     informational no-op findings are never labeled this way.
+//   - No recorded decision for the round: unlabeled, whether or not the finding
+//     survives into a later round. Absence of evidence is never a label.
+//
+// A user-added finding is false-negative gold. Confirmed post-PR misses are
+// written later by IngestPostPRMiss, not here.
+func goldFromRound(round *db.StepRound, decision Decision, prState string) Labels {
 	labels := Labels{Version: labelsVersion}
 	byID := findingIndex(round)
 	seen := map[string]bool{}
@@ -524,7 +552,7 @@ func goldFromRound(round *db.StepRound, decision Decision, prState string, later
 			labels.Findings = append(labels.Findings, goldForRecordedFinding(finding, goldSourceUserFix, GoldTruePositive))
 		}
 	}
-	if decision.SelectionSource == db.RoundSelectionSourceAutoFix && prState == "merged" {
+	if decision.SelectionSource == db.RoundSelectionSourceAutoFix && prState == prStateMerged {
 		for _, id := range decision.SelectedFindingIDs {
 			id = strings.TrimSpace(id)
 			if id == "" || seen[id] {
@@ -534,9 +562,6 @@ func goldFromRound(round *db.StepRound, decision Decision, prState string, later
 			if !ok {
 				continue
 			}
-			if !fixLanded(finding, later) {
-				continue
-			}
 			seen[id] = true
 			labels.Findings = append(labels.Findings, goldForRecordedFinding(finding, goldSourceAutoFixMerged, GoldTruePositive))
 		}
@@ -556,7 +581,7 @@ func goldFromRound(round *db.StepRound, decision Decision, prState string, later
 			labels.Findings = append(labels.Findings, goldForRecordedFinding(finding, goldSourceUserAdded, GoldFalseNegative))
 		}
 	}
-	if prState == "merged" && round.FindingsJSON != nil {
+	if prState == prStateMerged && hasRecordedDecision(decision) && round.FindingsJSON != nil {
 		for _, finding := range parseFindingItems(*round.FindingsJSON) {
 			id := strings.TrimSpace(finding.ID)
 			if id == "" || seen[id] || selected[id] {
@@ -565,9 +590,6 @@ func goldFromRound(round *db.StepRound, decision Decision, prState string, later
 			if finding.ActionOrDefault() == types.ActionNoOp {
 				continue
 			}
-			if !stillRaisedAtMerge(finding, later) {
-				continue
-			}
 			seen[id] = true
 			labels.Findings = append(labels.Findings, goldForRecordedFinding(finding, goldSourceShippedUnfixed, GoldFalsePositive))
 		}
@@ -588,55 +610,22 @@ func goldForRecordedFinding(finding types.Finding, source, kind string) FindingG
 	}
 }
 
-func fixLanded(finding types.Finding, later []*db.StepRound) bool {
-	gold := FindingGold{ID: finding.ID, File: finding.File, Description: finding.Description}
-	for _, round := range later {
-		if round == nil || round.FindingsJSON == nil {
-			continue
-		}
-		for _, item := range parseFindingItems(*round.FindingsJSON) {
-			if sameUnderlyingIssue(gold, item) {
-				return false
-			}
-		}
-	}
-	return true
-}
-
-// stillRaisedAtMerge reports whether the finding is still present in the last
-// later review round, which is the merge-time evidence for shipped-unfixed FP.
-// An intermediate re-raise that disappears before that last round is a fix,
-// not a shipped-unfixed finding. No later round means this round is last.
-func stillRaisedAtMerge(finding types.Finding, later []*db.StepRound) bool {
-	last := lastLaterRound(later)
-	if last == nil {
+// hasRecordedDecision reports whether the gate resolution for this round was
+// actually persisted. Only then can an unselected finding be read as "the human
+// looked at this and chose not to fix it", which is what makes a shipped-unfixed
+// false-positive label evidence rather than a guess. An unknown or aborted round
+// records no such choice, so its findings stay unlabeled.
+func hasRecordedDecision(decision Decision) bool {
+	if strings.TrimSpace(decision.SelectionSource) != "" {
 		return true
 	}
-	if last.FindingsJSON == nil {
-		return false
-	}
-	gold := FindingGold{ID: finding.ID, File: finding.File, Description: finding.Description}
-	for _, item := range parseFindingItems(*last.FindingsJSON) {
-		if sameUnderlyingIssue(gold, item) {
-			return true
-		}
+	switch decision.Action {
+	case decisionFix, decisionSkip, decisionApprove:
+		return true
 	}
 	return false
 }
 
-func lastLaterRound(later []*db.StepRound) *db.StepRound {
-	var last *db.StepRound
-	for _, round := range later {
-		if round == nil {
-			continue
-		}
-		if last == nil || round.Round > last.Round {
-			last = round
-		}
-	}
-	return last
-}
-
 func runPRState(run *db.Run) string {
 	if run == nil || run.PRState == nil || strings.TrimSpace(*run.PRState) == "" {
 		return "none"
@@ -741,9 +730,8 @@ func relabelRunLocked(store *Store, database *db.DB, runID string) ([]Case, erro
 			out = append(out, c)
 			continue
 		}
-		later := laterReviewRounds(reviewRounds, round)
 		decision := decisionForRound(round, reviewStep)
-		updated, err := relabelExistingCase(store, c.Dir, round, decision, runPRState(run), later)
+		updated, err := relabelExistingCase(store, c.Dir, round, decision, runPRState(run))
 		if err != nil {
 			return nil, err
 		}
@@ -781,25 +769,12 @@ func loadReviewRounds(database *db.DB, runID string) (*db.Run, []*db.StepRound,
 	return run, rounds, reviewStep, nil
 }
 
-func laterReviewRounds(rounds []*db.StepRound, current *db.StepRound) []*db.StepRound {
-	if current == nil {
-		return nil
-	}
-	out := make([]*db.StepRound, 0)
-	for _, round := range rounds {
-		if round.Round > current.Round {
-			out = append(out, round)
-		}
-	}
-	return out
-}
-
-func relabelExistingCase(store *Store, dir string, round *db.StepRound, decision Decision, prState string, later []*db.StepRound) (Case, error) {
+func relabelExistingCase(store *Store, dir string, round *db.StepRound, decision Decision, prState string) (Case, error) {
 	c, err := loadCase(dir)
 	if err != nil {
 		return Case{}, err
 	}
-	computed := goldFromRound(round, decision, prState, later)
+	computed := goldFromRound(round, decision, prState)
 	c.Labels = mergeGold(c.Labels, computed)
 	if err := writeJSON(filepath.Join(c.Dir, "labels.json"), c.Labels); err != nil {
 		return Case{}, fmt.Errorf("write relabeled gold: %w", err)

internal/eval/gold_label_test.go

@@ -53,7 +53,10 @@ func TestCaptureLeavesAutoFixClosedUnlabeled(t *testing.T) {
 	assertCaptureUnlabeled(t, ctx, p, sourceDB, run.ID)
 }
 
-func TestCaptureLeavesRevertedAutoFixUnlabeled(t *testing.T) {
+// A fix that was reverted (the same issue is raised again by a later round)
+// does not unmake the human's recorded decision to fix it on a run that merged:
+// the finding is still gold-standard evidence of a real issue.
+func TestCaptureLabelsSelectedAutoFixAsTruePositiveEvenWhenLaterRoundReRaisesIt(t *testing.T) {
 	ctx := context.Background()
 	p, sourceDB, run, _, firstRound := setupCapturedRun(t, ctx)
 	defer sourceDB.Close()
@@ -77,13 +80,22 @@ func TestCaptureLeavesRevertedAutoFixUnlabeled(t *testing.T) {
 		t.Fatalf("captured cases = %d, want both review rounds", len(cases))
 	}
 	for _, c := range cases {
-		if c.SourceRoundID == firstRound.ID && c.Labels.HasGold() {
-			t.Fatalf("reverted auto-fix round labels = %#v, want unlabeled (did not land)", c.Labels)
+		if c.SourceRoundID != firstRound.ID {
+			continue
+		}
+		if len(c.Labels.Findings) != 1 {
+			t.Fatalf("re-raised auto-fix round labels = %#v, want one gold finding", c.Labels)
+		}
+		gold := c.Labels.Findings[0]
+		if gold.Kind != GoldTruePositive || gold.Source != goldSourceAutoFixMerged || gold.ID != "real-bug" {
+			t.Fatalf("re-raised auto-fix gold = %#v, want recorded-auto-fix-merged true-positive from the recorded fix decision", gold)
 		}
 	}
 }
 
-func TestCaptureLeavesSupersededAutoFixUnlabeled(t *testing.T) {
+// Each round carries its own recorded decision, so a later round that rewrites
+// the finding under a new id does not retroactively unlabel the earlier one.
+func TestCaptureLabelsBothRoundsOfASupersededAutoFix(t *testing.T) {
 	ctx := context.Background()
 	p, sourceDB, run, _, firstRound := setupCapturedRun(t, ctx)
 	defer sourceDB.Close()
@@ -111,8 +123,9 @@ func TestCaptureLeavesSupersededAutoFixUnlabeled(t *testing.T) {
 	for _, c := range cases {
 		byRound[c.SourceRoundID] = c.Labels
 	}
-	if byRound[firstRound.ID].HasGold() {
-		t.Fatalf("superseded first-round labels = %#v, want unlabeled", byRound[firstRound.ID])
+	first := byRound[firstRound.ID]
+	if len(first.Findings) != 1 || first.Findings[0].Kind != GoldTruePositive || first.Findings[0].Source != goldSourceAutoFixMerged || first.Findings[0].ID != "real-bug" {
+		t.Fatalf("superseded first-round gold = %#v, want auto-fix-merged TP from its own recorded fix decision", first)
 	}
 	got := byRound[second.ID]
 	if len(got.Findings) != 1 || got.Findings[0].Kind != GoldTruePositive || got.Findings[0].Source != goldSourceAutoFixMerged || got.Findings[0].ID != "real-bug-v2" {
@@ -144,7 +157,10 @@ func TestCaptureWritesShippedUnfixedAsFalsePositive(t *testing.T) {
 	}
 }
 
-func TestCaptureLeavesLaterRoundLandedUnselectedUnlabeled(t *testing.T) {
+// The human resolved this round without selecting the finding and the run
+// merged, so it is shipped-unfixed false-positive gold. A later round raising a
+// different issue does not change what was decided about this one.
+func TestCaptureLabelsUnselectedFindingAsFalsePositiveEvenWhenALaterRoundDropsIt(t *testing.T) {
 	ctx := context.Background()
 	p, sourceDB, run, _, firstRound := setupCapturedRun(t, ctx)
 	defer sourceDB.Close()
@@ -171,8 +187,9 @@ func TestCaptureLeavesLaterRoundLandedUnselectedUnlabeled(t *testing.T) {
 	for _, c := range cases {
 		byRound[c.SourceRoundID] = c.Labels
 	}
-	if byRound[firstRound.ID].HasGold() {
-		t.Fatalf("later-fixed first-round labels = %#v, want unlabeled (finding no longer raised)", byRound[firstRound.ID])
+	first := byRound[firstRound.ID]
+	if len(first.Findings) != 1 || first.Findings[0].Kind != GoldFalsePositive || first.Findings[0].Source != goldSourceShippedUnfixed || first.Findings[0].ID != "real-bug" {
+		t.Fatalf("first-round gold = %#v, want shipped-unfixed FP from its own skip decision", first)
 	}
 	var second Labels
 	for id, labels := range byRound {
@@ -350,7 +367,10 @@ func TestRelabelDoesNotClobberAdjudicatedLabels(t *testing.T) {
 	}
 }
 
-func TestCaptureDoesNotWriteShippedUnfixedWhenIntermediateRoundThenFinalRoundDropsIt(t *testing.T) {
+// Round presence used to veto this label: the finding survived into round 2 but
+// was gone by the final round, so the earlier rounds stayed unlabeled. Labeling
+// now keys off each round's own recorded decision, which said "ship it".
+func TestCaptureWritesShippedUnfixedEvenWhenTheFinalRoundNoLongerRaisesIt(t *testing.T) {
 	ctx := context.Background()
 	p, sourceDB, run, _, firstRound := setupCapturedRun(t, ctx)
 	defer sourceDB.Close()
@@ -381,12 +401,16 @@ func TestCaptureDoesNotWriteShippedUnfixedWhenIntermediateRoundThenFinalRoundDro
 	for _, c := range cases {
 		byRound[c.SourceRoundID] = c.Labels
 	}
-	if byRound[firstRound.ID].HasGold() {
-		t.Fatalf("first-round labels = %#v, want unlabeled: the finding was gone from the final review round before merge", byRound[firstRound.ID])
+	first := byRound[firstRound.ID]
+	if len(first.Findings) != 1 || first.Findings[0].Kind != GoldFalsePositive || first.Findings[0].Source != goldSourceShippedUnfixed || first.Findings[0].ID != "real-bug" {
+		t.Fatalf("first-round gold = %#v, want shipped-unfixed FP keyed on this round's skip decision, not on the final round's finding list", first)
 	}
 }
 
-func TestRelabelRemovesShippedUnfixedWhenLaterRoundShowsTheFindingLanded(t *testing.T) {
+// Derived merge gold is recomputed, not appended: once the round records that
+// the human selected the finding for Fix, the earlier shipped-unfixed FP is gone
+// rather than kept alongside the true-positive label.
+func TestRelabelReplacesShippedUnfixedWhenTheRoundLaterRecordsAFixDecision(t *testing.T) {
 	ctx := context.Background()
 	p, sourceDB, run, _, firstRound := setupCapturedRun(t, ctx)
 	defer sourceDB.Close()
@@ -413,21 +437,19 @@ func TestRelabelRemovesShippedUnfixedWhenLaterRoundShowsTheFindingLanded(t *test
 		t.Fatalf("initial capture = %#v, want shipped-unfixed FP before the later round exists", first)
 	}
 
-	later := `{"findings":[{"id":"other-bug","severity":"warning","file":"main.go","line":1,"description":"style","action":"ask-user","review_scope":"source"}],"risk_level":"low","risk_rationale":"different issue","risk_scope":"source-or-external"}`
-	if _, err := sourceDB.InsertReviewStepRoundWithProvenance(steps[0].ID, 2, "auto_fix", &later, nil, run.HeadSHA, stringValue(firstRound.ReviewedHeadSHA), stringValue(firstRound.TrustedConfigSHA), firstRound.GlobalConfigYAML, firstRound.RepoConfigYAML, 25); err != nil {
+	if err := sourceDB.SetStepRoundSelection(firstRound.ID, strPtr(`["real-bug"]`), db.RoundSelectionSourceUser); err != nil {
 		t.Fatal(err)
 	}
 	relabeled, err := RelabelRun(ctx, store, p, sourceDB, run.ID)
 	if err != nil {
 		t.Fatal(err)
 	}
-	if len(relabeled) != 1 {
-		t.Fatalf("relabeled cases = %d, want the original round", len(relabeled))
+	if len(relabeled) != 1 || len(relabeled[0].Labels.Findings) != 1 {
+		t.Fatalf("relabeled = %#v, want exactly the recomputed user-fix gold", relabeled)
 	}
-	for _, gold := range relabeled[0].Labels.Findings {
-		if gold.ID == "real-bug" && gold.Source == goldSourceShippedUnfixed {
-			t.Fatalf("relabeled labels = %#v, want obsolete shipped-unfixed FP removed once a later round no longer raises the finding", relabeled[0].Labels)
-		}
+	gold := relabeled[0].Labels.Findings[0]
+	if gold.Kind != GoldTruePositive || gold.Source != goldSourceUserFix {
+		t.Fatalf("relabeled gold = %#v, want the obsolete shipped-unfixed FP replaced by the recorded user Fix", gold)
 	}
 }
 
@@ -469,15 +491,8 @@ func TestRelabelClearsStoredShippedUnfixedFPWhenRecomputedUnlabeled(t *testing.T
 	if err := sourceDB.UpdateStepStatus(steps[0].ID, types.StepStatusCompleted); err != nil {
 		t.Fatal(err)
 	}
-	stillRaised := `{"findings":[{"id":"real-bug","severity":"error","file":"main.go","line":3,"description":"bug","action":"ask-user","review_scope":"source"}],"risk_level":"high","risk_rationale":"still present","risk_scope":"source-or-external"}`
-	if _, err := sourceDB.InsertReviewStepRoundWithProvenance(steps[0].ID, 2, "auto_fix", &stillRaised, nil, run.HeadSHA, stringValue(firstRound.ReviewedHeadSHA), stringValue(firstRound.TrustedConfigSHA), firstRound.GlobalConfigYAML, firstRound.RepoConfigYAML, 25); err != nil {
-		t.Fatal(err)
-	}
-	final := `{"findings":[{"id":"other-bug","severity":"warning","file":"main.go","line":1,"description":"style","action":"ask-user","review_scope":"source"}],"risk_level":"low","risk_rationale":"different issue","risk_scope":"source-or-external"}`
-	if _, err := sourceDB.InsertReviewStepRoundWithProvenance(steps[0].ID, 3, "auto_fix", &final, nil, run.HeadSHA, stringValue(firstRound.ReviewedHeadSHA), stringValue(firstRound.TrustedConfigSHA), firstRound.GlobalConfigYAML, firstRound.RepoConfigYAML, 25); err != nil {
-		t.Fatal(err)
-	}
-	if err := sourceDB.UpdateRunPRState(run.ID, "merged"); err != nil {
+	// The PR never merged, so approve-with-findings supports no label at all.
+	if err := sourceDB.UpdateRunPRState(run.ID, "open"); err != nil {
 		t.Fatal(err)
 	}
 
@@ -541,6 +556,82 @@ func TestRelabelClearsStoredShippedUnfixedFPWhenRecomputedUnlabeled(t *testing.T
 	}
 }
 
+// TestGoldFromRoundLabelsByRecordedDecision pins the labeling contract itself:
+// what a finding is labeled follows from the round's recorded fix-vs-skip
+// decision plus the source run's merge state, and from nothing else.
+func TestGoldFromRoundLabelsByRecordedDecision(t *testing.T) {
+	raised := findingsJSON(findingSpec{ID: "real-bug", Severity: "error", File: "main.go", Line: 3, Description: "bug", Action: "auto-fix"})
+	informational := findingsJSON(findingSpec{ID: "note", Severity: "info", File: "main.go", Line: 3, Description: "style note", Action: "no-op"})
+
+	for _, tc := range []struct {
+		name     string
+		findings string
+		decision Decision
+		prState  string
+		wantKind string
+		wantGold string
+	}{
+		{
+			name: "selected for fix and merged is true-positive gold", findings: raised,
+			decision: Decision{Action: decisionFix, SelectionSource: db.RoundSelectionSourceAutoFix, SelectedFindingIDs: []string{"real-bug"}},
+			prState:  "merged", wantKind: GoldTruePositive, wantGold: goldSourceAutoFixMerged,
+		},
+		{
+			name: "user selected for fix needs no merge", findings: raised,
+			decision: Decision{Action: decisionFix, SelectionSource: db.RoundSelectionSourceUser, SelectedFindingIDs: []string{"real-bug"}},
+			prState:  "open", wantKind: GoldTruePositive, wantGold: goldSourceUserFix,
+		},
+		{
+			name: "skipped and merged is shipped-unfixed false-positive gold", findings: raised,
+			decision: Decision{Action: decisionApprove},
+			prState:  "merged", wantKind: GoldFalsePositive, wantGold: goldSourceShippedUnfixed,
+		},
+		{
+			name: "auto-fix selection leaves an unselected sibling shipped unfixed", findings: raised,
+			decision: Decision{Action: decisionFix, SelectionSource: db.RoundSelectionSourceAutoFix, SelectedFindingIDs: []string{"other"}},
+			prState:  "merged", wantKind: GoldFalsePositive, wantGold: goldSourceShippedUnfixed,
+		},
+		{
+			name: "no recorded decision stays unlabeled even on a merged run", findings: raised,
+			decision: Decision{Action: decisionUnknown},
+			prState:  "merged",
+		},
+		{
+			name: "an aborted round records no decision", findings: raised,
+			decision: Decision{Action: decisionAbort},
+			prState:  "merged",
+		},
+		{
+			name: "skipped without a merge stays unlabeled", findings: raised,
+			decision: Decision{Action: decisionApprove},
+			prState:  "open",
+		},
+		{
+			name: "informational no-op findings are never labeled", findings: informational,
+			decision: Decision{Action: decisionApprove},
+			prState:  "merged",
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			round := &db.StepRound{Round: 1, FindingsJSON: strPtr(tc.findings)}
+			got := goldFromRound(round, tc.decision, tc.prState)
+			if tc.wantKind == "" {
+				if got.HasGold() {
+					t.Fatalf("labels = %#v, want unlabeled", got)
+				}
+				return
+			}
+			if len(got.Findings) != 1 {
+				t.Fatalf("labels = %#v, want exactly one gold finding", got)
+			}
+			gold := got.Findings[0]
+			if gold.Kind != tc.wantKind || gold.Source != tc.wantGold {
+				t.Fatalf("gold = %#v, want %s gold from %s", gold, tc.wantKind, tc.wantGold)
+			}
+		})
+	}
+}
+
 func TestCaptureKeepsUserFixWithoutMerge(t *testing.T) {
 	ctx := context.Background()
 	p, sourceDB, run, _, _ := setupCapturedRun(t, ctx)

internal/eval/model.go

@@ -29,13 +29,13 @@ const (
 	labelsVersion = 2
 )
 
-// Gold kinds are the scientific labels written from recorded evidence.
-// Capture writes true-positive gold from a user Fix or from an auto-fix
-// selection that later landed in a merged PR, false-negative gold from a
-// user-added finding, and false-positive gold from a finding that shipped
-// unfixed in a merged PR. IngestPostPRMiss writes additional false-negative
-// gold after a green review. Adjudicated labels are never inferred from
-// pending unmatched candidate findings.
+// Gold kinds are the scientific labels written from the recorded gate decision
+// (see goldFromRound). Capture writes true-positive gold from a user Fix or
+// from an auto-fix selection on a merged run, false-negative gold from a
+// user-added finding, and false-positive gold from a finding the human did not
+// select for fix that shipped in a merged PR. IngestPostPRMiss writes
+// additional false-negative gold after a green review. Adjudicated labels are
+// never inferred from pending unmatched candidate findings.
 const (
 	GoldTruePositive  = "true-positive"
 	GoldFalseNegative = "false-negative"
@@ -109,7 +109,8 @@ type Manifest struct {
 
 // Decision records the human gate evidence available for the exported review
 // pass. Approval actions were not persisted in historical rows, so Action can
-// be "unknown". The original selections themselves are never guessed.
+// be "unknown". The original selections themselves are never guessed, and an
+// unknown action supports no label at all (see hasRecordedDecision).
 type Decision struct {
 	Action             string   `json:"action"`
 	SelectionSource    string   `json:"selection_source,omitempty"`

internal/eval/phase_a_user_facing_test.go

@@ -191,7 +191,7 @@ func TestPhaseAUserFacingTranscripts(t *testing.T) {
 		write("eval-report-f1-headline.txt", withFPOut)
 	})
 
-	t.Run("capture labels shipped-unfixed as FP and leaves dropped reraise unlabeled", func(t *testing.T) {
+	t.Run("capture labels shipped-unfixed as FP and leaves an undecided round unlabeled", func(t *testing.T) {
 		ctx := context.Background()
 		p, sourceDB, run, _, reviewRound := setupCapturedRun(t, ctx)
 		defer sourceDB.Close()
@@ -226,49 +226,37 @@ func TestPhaseAUserFacingTranscripts(t *testing.T) {
 		}
 		write("labels-shipped-unfixed.json", string(labelsJSON)+"\n")
 
+		// The same merged run, but with no recorded gate decision for the round,
+		// stays unlabeled: shipping unfixed is only evidence of a false positive
+		// when the human actually resolved the gate without selecting the finding.
 		p2, sourceDB2, run2, _, firstRound := setupCapturedRun(t, ctx)
 		defer sourceDB2.Close()
 		if err := sourceDB2.SetStepRoundSelection(firstRound.ID, nil, ""); err != nil {
 			t.Fatal(err)
 		}
-		steps2, err := sourceDB2.GetStepsByRun(run2.ID)
-		if err != nil {
-			t.Fatal(err)
-		}
-		if err := sourceDB2.UpdateStepStatus(steps2[0].ID, types.StepStatusCompleted); err != nil {
-			t.Fatal(err)
-		}
-		stillRaised := `{"findings":[{"id":"real-bug","severity":"error","file":"main.go","line":3,"description":"bug","action":"ask-user","review_scope":"source"}],"risk_level":"high","risk_rationale":"still present","risk_scope":"source-or-external"}`
-		if _, err := sourceDB2.InsertReviewStepRoundWithProvenance(steps2[0].ID, 2, "auto_fix", &stillRaised, nil, run2.HeadSHA, stringValue(firstRound.ReviewedHeadSHA), stringValue(firstRound.TrustedConfigSHA), firstRound.GlobalConfigYAML, firstRound.RepoConfigYAML, 25); err != nil {
-			t.Fatal(err)
-		}
-		final := `{"findings":[{"id":"other-bug","severity":"warning","file":"main.go","line":1,"description":"style","action":"ask-user","review_scope":"source"}],"risk_level":"low","risk_rationale":"different issue","risk_scope":"source-or-external"}`
-		if _, err := sourceDB2.InsertReviewStepRoundWithProvenance(steps2[0].ID, 3, "auto_fix", &final, nil, run2.HeadSHA, stringValue(firstRound.ReviewedHeadSHA), stringValue(firstRound.TrustedConfigSHA), firstRound.GlobalConfigYAML, firstRound.RepoConfigYAML, 25); err != nil {
-			t.Fatal(err)
-		}
 		if err := sourceDB2.UpdateRunPRState(run2.ID, "merged"); err != nil {
 			t.Fatal(err)
 		}
-		dropped := captureAll(t, ctx, p2, sourceDB2, run2.ID)
+		undecided := captureAll(t, ctx, p2, sourceDB2, run2.ID)
 		byRound := map[string]Labels{}
-		for _, c := range dropped {
+		for _, c := range undecided {
 			byRound[c.SourceRoundID] = c.Labels
 		}
 		if byRound[firstRound.ID].HasGold() {
-			t.Fatalf("dropped-reraise first-round labels = %#v, want unlabeled", byRound[firstRound.ID])
+			t.Fatalf("undecided first-round labels = %#v, want unlabeled", byRound[firstRound.ID])
 		}
-		type droppedRow struct {
+		type undecidedRow struct {
 			RoundID string `json:"source_round_id"`
 			Labels  Labels `json:"labels"`
 		}
-		rows := make([]droppedRow, 0, len(dropped))
-		for _, c := range dropped {
-			rows = append(rows, droppedRow{RoundID: c.SourceRoundID, Labels: c.Labels})
+		rows := make([]undecidedRow, 0, len(undecided))
+		for _, c := range undecided {
+			rows = append(rows, undecidedRow{RoundID: c.SourceRoundID, Labels: c.Labels})
 		}
-		droppedJSON, err := json.MarshalIndent(rows, "", "  ")
+		undecidedJSON, err := json.MarshalIndent(rows, "", "  ")
 		if err != nil {
 			t.Fatal(err)
 		}
-		write("labels-dropped-after-reraise.json", string(droppedJSON)+"\n")
+		write("labels-undecided-round.json", string(undecidedJSON)+"\n")
 	})
 }

internal/eval/score.go

@@ -39,10 +39,11 @@ type Score struct {
 //   - Pending: unmatched candidate findings, never inferred as invalid
 //
 // Matching is a documented cascade of strengths: exact-id, exact-text,
-// nearby-line Jaccard, then gated containment. Assignment is maximum matching
-// per strength tier so an earlier gold cannot consume a candidate that a later
-// gold matches more strongly. Headline recall uses the full cascade; exact vs
-// fuzzy counts are reported separately so a threshold change is visible.
+// nearby-line Jaccard, then gated containment. Assignment is one globally
+// optimal assignment over the whole graph (see assignMatches), so neither
+// candidate ordering nor a tier boundary can consume a candidate another gold
+// needed. Headline recall uses the full cascade; exact vs fuzzy counts are
+// reported separately so a threshold change is visible.
 func ScoreCandidate(labels Labels, findingsJSON string) Score {
 	candidate := parseFindingItems(findingsJSON)
 	assigned := assignMatches(labels.Findings, candidate)
@@ -93,77 +94,189 @@ type assignedMatch struct {
 	strength string
 }
 
+// assignMatches pairs gold findings with candidate findings as one globally
+// optimal assignment rather than a per-strength-tier pass.
+//
+// Tiered assignment - maximum matching on exact edges, then on the residual for
+// each weaker strength - is not optimal for the whole graph, because WHICH
+// maximum exact matching it happens to pick decides what the weaker tiers can
+// still reach. With gold A matching candidate 1 exactly and candidate 2 by
+// location, and gold B matching only candidate 1 exactly, the exact tier can
+// hand candidate 1 to A and strand B forever, scoring one match where two exist.
+// That understates recall for reasons that have nothing to do with the review
+// under test, so the assignment is solved once over every edge at once.
+//
+// Optimality is lexicographic by strength: an exact pair is worth strictly more
+// than every possible combination of weaker pairs (weightFor scales each tier by
+// a base larger than any achievable count), so the optimum never trades one
+// exact match for two fuzzy ones, and among the assignments with the most exact
+// pairs it takes the one with the most location pairs, then the most containment
+// pairs. hungarianMinCost solves that max-weight assignment exactly.
 func assignMatches(golds []FindingGold, candidate []types.Finding) []assignedMatch {
 	out := make([]assignedMatch, len(golds))
 	for i := range out {
 		out[i].cand = -1
 	}
-	matchedGold := make([]bool, len(golds))
-	usedCand := make([]bool, len(candidate))
-	for _, strength := range []string{matchExactID, matchExactText, matchLocation, matchContainment} {
-		adj := make([][]int, len(golds))
-		for gi, gold := range golds {
-			if matchedGold[gi] {
-				continue
-			}
-			for ci, finding := range candidate {
-				if usedCand[ci] {
-					continue
-				}
-				if matchAt(gold, finding, strength) {
-					adj[gi] = append(adj[gi], ci)
+	if len(golds) == 0 || len(candidate) == 0 {
+		return out
+	}
+	base := int64(len(golds)+len(candidate)) + 1
+	weight := make([][]int64, len(golds))
+	strength := make([][]string, len(golds))
+	for gi, gold := range golds {
+		weight[gi] = make([]int64, len(candidate))
+		strength[gi] = make([]string, len(candidate))
+		for ci, finding := range candidate {
+			for _, s := range []string{matchExactID, matchExactText, matchLocation, matchContainment} {
+				if matchAt(gold, finding, s) {
+					weight[gi][ci] = weightFor(s, base)
+					strength[gi][ci] = s
+					break
 				}
 			}
 		}
-		goldToCand := maxBipartiteMatching(adj, len(candidate))
-		for gi, ci := range goldToCand {
-			if ci < 0 {
-				continue
-			}
-			out[gi] = assignedMatch{cand: ci, strength: strength}
-			matchedGold[gi] = true
-			usedCand[ci] = true
+	}
+	for gi, ci := range maxWeightAssignment(weight) {
+		// A zero-weight pair is the absence of an edge, not a match: the
+		// assignment is over a complete matrix so every row gets a column.
+		if ci < 0 || weight[gi][ci] == 0 {
+			continue
+		}
+		out[gi] = assignedMatch{cand: ci, strength: strength[gi][ci]}
+	}
+	return out
+}
+
+// weightFor ranks the strengths so that no number of weaker pairs can outweigh
+// a single stronger one. base exceeds the largest possible pair count, so the
+// total weight of an assignment reads as a positional number whose digits are
+// the per-strength counts.
+func weightFor(strength string, base int64) int64 {
+	switch strength {
+	case matchExactID, matchExactText:
+		return base * base
+	case matchLocation:
+		return base
+	case matchContainment:
+		return 1
+	default:
+		return 0
+	}
+}
+
+// maxWeightAssignment returns, for each gold row, the candidate column assigned
+// to it (-1 when there are no columns), maximizing total weight. It solves the
+// rectangular assignment problem exactly, transposing when there are more rows
+// than columns because the solver requires rows <= columns.
+func maxWeightAssignment(weight [][]int64) []int {
+	rows, cols := len(weight), len(weight[0])
+	if rows <= cols {
+		return hungarianMinCost(negate(weight))
+	}
+	transposed := make([][]int64, cols)
+	for j := range transposed {
+		transposed[j] = make([]int64, rows)
+		for i := range weight {
+			transposed[j][i] = -weight[i][j]
+		}
+	}
+	colToRow := hungarianMinCost(transposed)
+	rowToCol := make([]int, rows)
+	for i := range rowToCol {
+		rowToCol[i] = -1
+	}
+	for j, i := range colToRow {
+		if i >= 0 {
+			rowToCol[i] = j
+		}
+	}
+	return rowToCol
+}
+
+func negate(weight [][]int64) [][]int64 {
+	out := make([][]int64, len(weight))
+	for i, row := range weight {
+		out[i] = make([]int64, len(row))
+		for j, w := range row {
+			out[i][j] = -w
 		}
 	}
 	return out
 }
 
-func maxBipartiteMatching(adj [][]int, candCount int) []int {
-	candToGold := make([]int, candCount)
-	for i := range candToGold {
-		candToGold[i] = -1
+// hungarianMinCost is the O(n^2*m) Hungarian (Kuhn-Munkres) algorithm for the
+// rectangular assignment problem: it returns the minimum-cost assignment of
+// every row to a distinct column, which is the exact optimum rather than a
+// greedy approximation. It requires len(cost) <= len(cost[0]).
+func hungarianMinCost(cost [][]int64) []int {
+	n := len(cost)
+	if n == 0 {
+		return nil
 	}
-	var dfs func(gi int, seen []bool) bool
-	dfs = func(gi int, seen []bool) bool {
-		for _, ci := range adj[gi] {
-			if seen[ci] {
-				continue
+	m := len(cost[0])
+	const inf = int64(1) << 62
+	// Potentials u/v and the column-to-row matching p are 1-indexed; index 0 is
+	// the algorithm's virtual starting column.
+	u := make([]int64, n+1)
+	v := make([]int64, m+1)
+	p := make([]int, m+1)
+	way := make([]int, m+1)
+	for i := 1; i <= n; i++ {
+		p[0] = i
+		j0 := 0
+		minv := make([]int64, m+1)
+		used := make([]bool, m+1)
+		for j := range minv {
+			minv[j] = inf
+		}
+		for {
+			used[j0] = true
+			i0 := p[j0]
+			delta := inf
+			j1 := 0
+			for j := 1; j <= m; j++ {
+				if used[j] {
+					continue
+				}
+				cur := cost[i0-1][j-1] - u[i0] - v[j]
+				if cur < minv[j] {
+					minv[j] = cur
+					way[j] = j0
+				}
+				if minv[j] < delta {
+					delta = minv[j]
+					j1 = j
+				}
 			}
-			seen[ci] = true
-			if candToGold[ci] < 0 || dfs(candToGold[ci], seen) {
-				candToGold[ci] = gi
-				return true
+			for j := 0; j <= m; j++ {
+				if used[j] {
+					u[p[j]] += delta
+					v[j] -= delta
+				} else {
+					minv[j] -= delta
+				}
+			}
+			j0 = j1
+			if p[j0] == 0 {
+				break
 			}
 		}
-		return false
-	}
-	for gi := range adj {
-		if len(adj[gi]) == 0 {
-			continue
-		}
-		seen := make([]bool, candCount)
-		_ = dfs(gi, seen)
-	}
-	goldToCand := make([]int, len(adj))
-	for i := range goldToCand {
-		goldToCand[i] = -1
-	}
-	for ci, gi := range candToGold {
-		if gi >= 0 {
-			goldToCand[gi] = ci
+		for j0 != 0 {
+			j1 := way[j0]
+			p[j0] = p[j1]
+			j0 = j1
 		}
 	}
-	return goldToCand
+	rowToCol := make([]int, n)
+	for i := range rowToCol {
+		rowToCol[i] = -1
+	}
+	for j := 1; j <= m; j++ {
+		if p[j] > 0 {
+			rowToCol[p[j]-1] = j - 1
+		}
+	}
+	return rowToCol
 }
 
 func matchAt(gold FindingGold, finding types.Finding, strength string) bool {
@@ -181,10 +294,6 @@ func matchAt(gold FindingGold, finding types.Finding, strength string) bool {
 	}
 }
 
-func sameUnderlyingIssue(gold FindingGold, finding types.Finding) bool {
-	return matchAt(gold, finding, matchExactID) || matchAt(gold, finding, matchExactText)
-}
-
 func exactTextMatch(gold FindingGold, finding types.Finding) bool {
 	goldFile, goldDesc := normalizeIssue(gold.File, gold.Description)
 	candFile, candDesc := normalizeIssue(finding.File, finding.Description)

internal/eval/score_test.go

@@ -1,6 +1,7 @@
 package eval
 
 import (
+	"math/rand"
 	"strings"
 	"testing"
 )
@@ -80,6 +81,113 @@ func TestScoreCandidateDoesNotLetFuzzyEarlierGoldStealExactLaterMatch(t *testing
 	}
 }
 
+// Both gold items match candidate 1 exactly by id, and only the first also has
+// a fuzzy (nearby-line) match on candidate 2. The tiered matcher this replaced
+// resolved the exact tier on its own, handed candidate 1 to the first gold, and
+// then had nothing left for the second - one match where two exist. A globally
+// optimal assignment gives candidate 1 to the gold that has no alternative and
+// covers the other fuzzily, with the same number of exact matches.
+func TestScoreCandidateRecoversMatchTheTieredMatcherLost(t *testing.T) {
+	labels := Labels{Findings: []FindingGold{
+		{
+			ID:          "shared-id",
+			Kind:        GoldTruePositive,
+			File:        "main.go",
+			Line:        10,
+			Description: "nil pointer dereference in the request handler",
+		},
+		{
+			ID:          "shared-id",
+			Kind:        GoldTruePositive,
+			File:        "lock.go",
+			Line:        40,
+			Description: "mutex not released on the error path",
+		},
+	}}
+	candidate := `{"findings":[` +
+		`{"id":"shared-id","file":"main.go","line":10,"description":"nil pointer dereference in the request handler"},` +
+		`{"id":"other","file":"main.go","line":11,"description":"nil pointer dereference in the request handler during shutdown"}` +
+		`]}`
+
+	score := ScoreCandidate(labels, candidate)
+	if score.TruePositive != 2 || score.FalseNegative != 0 {
+		t.Fatalf("score = %#v, want both gold items matched rather than one consumed by a tier boundary", score)
+	}
+	if score.TruePositiveExact != 1 || score.TruePositiveFuzzy != 1 {
+		t.Fatalf("score = %#v, want the exact match kept and the second gold covered fuzzily", score)
+	}
+	if score.Pending != 0 {
+		t.Fatalf("score = %#v, want both candidates consumed by the assignment", score)
+	}
+}
+
+// The assignment must be the exact optimum, not a good heuristic, so it is
+// checked against exhaustive enumeration on random small weight matrices of
+// both orientations.
+func TestMaxWeightAssignmentMatchesBruteForceOptimum(t *testing.T) {
+	rng := rand.New(rand.NewSource(20260816))
+	for trial := 0; trial < 400; trial++ {
+		rows := 1 + rng.Intn(5)
+		cols := 1 + rng.Intn(5)
+		weight := make([][]int64, rows)
+		for i := range weight {
+			weight[i] = make([]int64, cols)
+			for j := range weight[i] {
+				// Zero is the common case on purpose: it is how a missing edge
+				// is spelled, and it is where a greedy solver goes wrong.
+				if rng.Intn(3) == 0 {
+					weight[i][j] = int64(rng.Intn(4)) * 9
+				}
+			}
+		}
+		got := totalWeight(weight, maxWeightAssignment(weight))
+		want := bruteForceMaxWeight(weight)
+		if got != want {
+			t.Fatalf("assignment weight = %d, want the optimum %d for %v", got, want, weight)
+		}
+	}
+}
+
+func totalWeight(weight [][]int64, rowToCol []int) int64 {
+	var total int64
+	seen := map[int]bool{}
+	for i, j := range rowToCol {
+		if j < 0 {
+			continue
+		}
+		if seen[j] {
+			panic("assignment reused a column")
+		}
+		seen[j] = true
+		total += weight[i][j]
+	}
+	return total
+}
+
+func bruteForceMaxWeight(weight [][]int64) int64 {
+	cols := len(weight[0])
+	used := make([]bool, cols)
+	var best func(row int) int64
+	best = func(row int) int64 {
+		if row == len(weight) {
+			return 0
+		}
+		top := best(row + 1)
+		for j := 0; j < cols; j++ {
+			if used[j] {
+				continue
+			}
+			used[j] = true
+			if got := weight[row][j] + best(row+1); got > top {
+				top = got
+			}
+			used[j] = false
+		}
+		return top
+	}
+	return best(0)
+}
+
 func TestScoreCandidateKeepsUnmatchedPendingUntilAdjudicated(t *testing.T) {
 	labels := Labels{Findings: []FindingGold{{
 		ID:          "gold",