diff --git a/packages/loopover-engine/src/finding-severity-calibration.ts b/packages/loopover-engine/src/finding-severity-calibration.ts index 5b292eb771..d5fa55487f 100644 --- a/packages/loopover-engine/src/finding-severity-calibration.ts +++ b/packages/loopover-engine/src/finding-severity-calibration.ts @@ -148,6 +148,10 @@ function finiteNonNegativeInt(value: number | undefined): number { return Math.floor(value); } +function isInvalidWeight(value: number | undefined): boolean { + return value !== undefined && (!Number.isFinite(value) || value < 0); +} + function roundScore(value: number): number { return Math.round(Math.min(1, Math.max(0, value)) * 1_000_000) / 1_000_000; } @@ -366,7 +370,19 @@ function normalizeCompositeWeights(weights: FindingSeverityCalibrationWeights | // Preserve explicitly-zeroed weights rather than substituting the defaults: a caller that zeroes every component // must reach the objective-only fallback in the composite scorer, not silently get the default 45/35/20 blend // (converges with reviewer-consensus-calibration.ts's already-correct behavior; #6170). - if (total <= 0) return { objectiveAnchor: 0, pairwiseJudge: 0, structuredFindingSeverity: 0 }; + // NaN/negative inputs still recover to DEFAULT_COMPOSITE_WEIGHTS when the clamped total is empty rather than + // collapsing to a 100%-objective-anchor composite, mirroring pairwise-calibration.ts's invalid-weight + // distinction (#7443; #8643). + if (total <= 0) { + if ( + isInvalidWeight(weights?.objectiveAnchor) || + isInvalidWeight(weights?.pairwiseJudge) || + isInvalidWeight(weights?.structuredFindingSeverity) + ) { + return DEFAULT_COMPOSITE_WEIGHTS; + } + return { objectiveAnchor: 0, pairwiseJudge: 0, structuredFindingSeverity: 0 }; + } return { objectiveAnchor: raw.objectiveAnchor / total, pairwiseJudge: raw.pairwiseJudge / total, diff --git a/packages/loopover-engine/src/gate-verdict-calibration.ts b/packages/loopover-engine/src/gate-verdict-calibration.ts index c0569ebe68..3be844481c 100644 --- a/packages/loopover-engine/src/gate-verdict-calibration.ts +++ b/packages/loopover-engine/src/gate-verdict-calibration.ts @@ -147,6 +147,10 @@ function finiteNonNegative(value: number | undefined, fallback: number): number return value; } +function isInvalidWeight(value: number | undefined): boolean { + return value !== undefined && (!Number.isFinite(value) || value < 0); +} + function roundScore(value: number): number { return Math.round(Math.min(1, Math.max(0, value)) * 1_000_000) / 1_000_000; } @@ -319,7 +323,19 @@ function normalizeCompositeWeights(weights: GateVerdictCalibrationWeights | unde // Preserve explicitly-zeroed weights rather than substituting the defaults: a caller that zeroes every component // must reach the objective-only fallback in the composite scorer, not silently get the default 45/35/20 blend // (converges with reviewer-consensus-calibration.ts's already-correct behavior; #6170). - if (total <= 0) return { objectiveAnchor: 0, pairwiseJudge: 0, structuredGateVerdict: 0 }; + // NaN/negative inputs still recover to DEFAULT_COMPOSITE_WEIGHTS when the clamped total is empty rather than + // collapsing to a 100%-objective-anchor composite, mirroring pairwise-calibration.ts's invalid-weight + // distinction (#7443; #8643). + if (total <= 0) { + if ( + isInvalidWeight(weights?.objectiveAnchor) || + isInvalidWeight(weights?.pairwiseJudge) || + isInvalidWeight(weights?.structuredGateVerdict) + ) { + return DEFAULT_COMPOSITE_WEIGHTS; + } + return { objectiveAnchor: 0, pairwiseJudge: 0, structuredGateVerdict: 0 }; + } return { objectiveAnchor: raw.objectiveAnchor / total, pairwiseJudge: raw.pairwiseJudge / total, diff --git a/packages/loopover-engine/src/reviewer-consensus-calibration.ts b/packages/loopover-engine/src/reviewer-consensus-calibration.ts index 105f58e9f3..7b70162b1b 100644 --- a/packages/loopover-engine/src/reviewer-consensus-calibration.ts +++ b/packages/loopover-engine/src/reviewer-consensus-calibration.ts @@ -154,6 +154,10 @@ function finiteNonNegative(value: number | undefined, fallback: number): number return value; } +function isInvalidWeight(value: number | undefined): boolean { + return value !== undefined && (!Number.isFinite(value) || value < 0); +} + function roundScore(value: number): number { return Math.round(Math.min(1, Math.max(0, value)) * 1_000_000) / 1_000_000; } @@ -402,7 +406,19 @@ function normalizeCompositeWeights(weights: ReviewerConsensusCalibrationWeights const total = raw.objectiveAnchor + raw.pairwiseJudge + raw.structuredReviewerConsensus; // Preserve explicitly-zeroed weights rather than substituting the defaults: a caller that zeroes every component // must reach the objective-only fallback in the composite scorer, not silently get the default 45/35/20 blend. - if (total <= 0) return { objectiveAnchor: 0, pairwiseJudge: 0, structuredReviewerConsensus: 0 }; + // NaN/negative inputs still recover to DEFAULT_COMPOSITE_WEIGHTS when the clamped total is empty rather than + // collapsing to a 100%-objective-anchor composite, mirroring pairwise-calibration.ts's invalid-weight + // distinction (#7443; #8643). + if (total <= 0) { + if ( + isInvalidWeight(weights?.objectiveAnchor) || + isInvalidWeight(weights?.pairwiseJudge) || + isInvalidWeight(weights?.structuredReviewerConsensus) + ) { + return DEFAULT_COMPOSITE_WEIGHTS; + } + return { objectiveAnchor: 0, pairwiseJudge: 0, structuredReviewerConsensus: 0 }; + } return { objectiveAnchor: raw.objectiveAnchor / total, pairwiseJudge: raw.pairwiseJudge / total, diff --git a/packages/loopover-engine/test/finding-severity-calibration.test.ts b/packages/loopover-engine/test/finding-severity-calibration.test.ts index ee5d1f07c5..653eb238c2 100644 --- a/packages/loopover-engine/test/finding-severity-calibration.test.ts +++ b/packages/loopover-engine/test/finding-severity-calibration.test.ts @@ -483,3 +483,15 @@ test("computeFindingSeverityCompositeCalibrationScore falls back to objective-on assert.deepEqual(result.weights, { objectiveAnchor: 1, pairwiseJudge: 0, structuredFindingSeverity: 0 }); assert.equal(result.compositeScore, 0.4); }); + +test("computeFindingSeverityCompositeCalibrationScore normalizes invalid weights to the default blend, not an objective-only collapse (#8643)", () => { + const result = computeFindingSeverityCompositeCalibrationScore({ + objectiveAnchor: 0.4, + pairwise: 0.4, + findingSeverity: [signal()], + weights: { objectiveAnchor: Number.NaN, pairwiseJudge: -1, structuredFindingSeverity: -1 }, + }); + // NaN/negative weights (with real pairwise + structured scores present) recover to the documented default + // 45/35/20 blend -- NOT a 100%-objective-anchor collapse, matching pairwise-calibration.ts's distinction. + assert.deepEqual(result.weights, { objectiveAnchor: 0.45, pairwiseJudge: 0.35, structuredFindingSeverity: 0.2 }); +}); diff --git a/packages/loopover-engine/test/gate-verdict-calibration.test.ts b/packages/loopover-engine/test/gate-verdict-calibration.test.ts index 60c995e75b..403bee2f36 100644 --- a/packages/loopover-engine/test/gate-verdict-calibration.test.ts +++ b/packages/loopover-engine/test/gate-verdict-calibration.test.ts @@ -602,6 +602,40 @@ test("computeGateVerdictCompositeCalibrationScore falls back to objective-only w assert.equal(result.compositeScore, 0.4); }); +test("computeGateVerdictCompositeCalibrationScore normalizes invalid weights to the default blend, not an objective-only collapse (#8643)", () => { + const objectiveAnchor = scoreObjectiveAnchor({ + replayed: { paths: ["src/review/a.ts"], labels: ["feature"] }, + revealed: { paths: ["src/review/b.ts"], labels: ["feature"] }, + }); + const pairwise = computePairwiseCalibrationScore({ + objectiveAnchor, + samples: [{ attempts: [{ replayFirst: "replay_better", revealedFirst: "revealed_better" }] }], + }); + const result = computeGateVerdictCompositeCalibrationScore({ + objectiveAnchor, + pairwise, + gateVerdicts: [ + { + repoFullName: "JSONbored/Loopover", + replayRunId: "replay-7", + gateRunId: "gate-7", + optedIn: true, + dimensions: [ + { dimension: "correctness", outcome: "pass" }, + { dimension: "tests", outcome: "warn" }, + ], + }, + ], + weights: { objectiveAnchor: Number.NaN, pairwiseJudge: -1, structuredGateVerdict: -1 }, + }); + + // NaN/negative weights (with real pairwise + structured scores present) recover to the documented default + // 45/35/20 blend -- NOT a 100%-objective-anchor collapse, matching pairwise-calibration.ts's distinction. + assert.equal(pairwise.pairwiseJudgeScore, 1); + assert.equal(result.structuredGateVerdictScore, 0.75); + assert.deepEqual(result.weights, { objectiveAnchor: 0.45, pairwiseJudge: 0.35, structuredGateVerdict: 0.2 }); +}); + test("computeGateVerdictCompositeCalibrationScore preserves a malformed-repo (invalid_repo) rejected row instead of dropping it (#6170)", () => { const result = computeGateVerdictCompositeCalibrationScore({ objectiveAnchor: 0.5, diff --git a/packages/loopover-engine/test/reviewer-consensus-calibration.test.ts b/packages/loopover-engine/test/reviewer-consensus-calibration.test.ts index 0e86892882..7733af1640 100644 --- a/packages/loopover-engine/test/reviewer-consensus-calibration.test.ts +++ b/packages/loopover-engine/test/reviewer-consensus-calibration.test.ts @@ -309,6 +309,23 @@ test("composite honors custom weights and falls back to objective-only when all assert.equal(allZero.compositeScore, 0.4); }); +test("composite normalizes invalid weights to the default blend, not an objective-only collapse (#8643)", () => { + const ingestion = ingestReviewerConsensusCalibrationSignals([signal()]); + const result = computeReviewerConsensusCompositeCalibrationScore({ + objectiveAnchor: 0.4, + pairwise: 0.4, + reviewerConsensus: ingestion, + weights: { objectiveAnchor: Number.NaN, pairwiseJudge: -1, structuredReviewerConsensus: -1 }, + }); + // NaN/negative weights (with real pairwise + structured scores present) recover to the documented default + // 45/35/20 blend -- NOT a 100%-objective-anchor collapse, matching pairwise-calibration.ts's distinction. + assert.deepEqual(result.weights, { + objectiveAnchor: 0.45, + pairwiseJudge: 0.35, + structuredReviewerConsensus: 0.2, + }); +}); + test("composite sanitizes pre-ingested reviewer-consensus rows before auditing", () => { const poisoned = { accepted: [ diff --git a/test/unit/engine-calibration-convergence.test.ts b/test/unit/engine-calibration-convergence.test.ts index e2dd190520..d45dda45d4 100644 --- a/test/unit/engine-calibration-convergence.test.ts +++ b/test/unit/engine-calibration-convergence.test.ts @@ -3,6 +3,8 @@ import { computeGateVerdictCompositeCalibrationScore, computeFindingSeverityCompositeCalibrationScore, computePairwiseCalibrationScore, + computeReviewerConsensusCompositeCalibrationScore, + ingestReviewerConsensusCalibrationSignals, } from "../../packages/loopover-engine/src/index"; // Converges gate-verdict + finding-severity calibration with reviewer-consensus-calibration.ts's already-correct @@ -54,6 +56,64 @@ describe("gate-verdict/finding-severity calibration convergence (#6170)", () => expect(result.weights.structuredFindingSeverity).toBeGreaterThan(0); }); + it("gate-verdict: NaN/negative weights recover to the default 45/35/20 blend, NOT a 100%-objective-anchor collapse (#8643)", () => { + const result = computeGateVerdictCompositeCalibrationScore({ + objectiveAnchor: 0.4, + pairwise: 0.4, + gateVerdicts: [ + { + repoFullName: "acme/widgets", + replayRunId: "replay-1", + gateRunId: "gate-1", + optedIn: true, + dimensions: [{ dimension: "correctness", outcome: "pass" }], + }, + ], + weights: { objectiveAnchor: Number.NaN, pairwiseJudge: -1, structuredGateVerdict: -1 }, + }); + // Real pairwise + structured scores are present, so the recovered default blend survives unrenormalized. + expect(result.weights).toEqual({ objectiveAnchor: 0.45, pairwiseJudge: 0.35, structuredGateVerdict: 0.2 }); + }); + + it("finding-severity: NaN/negative weights recover to the default blend, NOT the objective-only collapse (#8643)", () => { + const result = computeFindingSeverityCompositeCalibrationScore({ + objectiveAnchor: 0.4, + pairwise: 0.4, + findingSeverity: [ + { repoFullName: "acme/widgets", replayRunId: "replay-1", reviewRunId: "review-1", optedIn: true, tiers: [{ tier: "blocker", total: 2, confirmed: 2 }] }, + ], + weights: { objectiveAnchor: Number.NaN, pairwiseJudge: -1, structuredFindingSeverity: -1 }, + }); + expect(result.weights).toEqual({ objectiveAnchor: 0.45, pairwiseJudge: 0.35, structuredFindingSeverity: 0.2 }); + }); + + it("reviewer-consensus: explicit all-zero weights fall back to objective-only (covers the total<=0 real-zeros arm)", () => { + const ingestion = ingestReviewerConsensusCalibrationSignals([ + { repoFullName: "acme/widgets", replayRunId: "replay-1", reviewRunId: "review-1", optedIn: true, dimensions: [{ dimension: "correctness", votes: ["pass", "pass"] }] }, + ]); + const result = computeReviewerConsensusCompositeCalibrationScore({ + objectiveAnchor: 0.4, + pairwise: 0.4, + reviewerConsensus: ingestion, + weights: { objectiveAnchor: 0, pairwiseJudge: 0, structuredReviewerConsensus: 0 }, + }); + expect(result.weights).toEqual({ objectiveAnchor: 1, pairwiseJudge: 0, structuredReviewerConsensus: 0 }); + expect(result.compositeScore).toBe(0.4); + }); + + it("reviewer-consensus: NaN/negative weights recover to the default blend, NOT the objective-only collapse (#8643)", () => { + const ingestion = ingestReviewerConsensusCalibrationSignals([ + { repoFullName: "acme/widgets", replayRunId: "replay-1", reviewRunId: "review-1", optedIn: true, dimensions: [{ dimension: "correctness", votes: ["pass", "pass"] }] }, + ]); + const result = computeReviewerConsensusCompositeCalibrationScore({ + objectiveAnchor: 0.4, + pairwise: 0.4, + reviewerConsensus: ingestion, + weights: { objectiveAnchor: Number.NaN, pairwiseJudge: -1, structuredReviewerConsensus: -1 }, + }); + expect(result.weights).toEqual({ objectiveAnchor: 0.45, pairwiseJudge: 0.35, structuredReviewerConsensus: 0.2 }); + }); + it("gate-verdict: preserves a malformed-repo (invalid_repo) rejected row instead of dropping it; keeps a valid repo and drops a non-string one", () => { const result = computeGateVerdictCompositeCalibrationScore({ objectiveAnchor: 0.5,