Skip to content

fix(engine): dedup provider-track-record aggregation per provider+target #8955

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jeffrey701 wants to merge 1 commit into from
Closed

fix(engine): dedup provider-track-record aggregation per provider+target #8955

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion packages/loopover-engine/src/calibration/provider-track-record.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,17 @@ export function computeProviderTrackRecords(
stances.set(signal.provider, signal.vote === "fail");
}

// #8876: collapse repeated (provider, targetKey) signals to the LATEST one before aggregating. A provider
// re-reviewing the same PR (common after a new push) emits another audit row, and loadLiveProviderTrackRecords
// reads raw rows with no dedup — so iterating `signals` directly counted signals/decided/shared/consensus once
// per row rather than once per distinct (provider, targetKey) pair, inflating that provider's precision and
// agreement stats. Keep the last row per pair, matching stancesByTarget's own latest-vote `.set` dedup above.
const latestByProviderTarget = new Map<string, ProviderReviewSignal>();
for (const signal of signals) latestByProviderTarget.set(`${signal.provider} ${signal.targetKey}`, signal);

const perRepo = new Map<string, Map<string, MutableStats>>(); // provider → repo → stats
const overall = new Map<string, MutableStats>();
for (const signal of signals) {
for (const signal of latestByProviderTarget.values()) {
let repos = perRepo.get(signal.provider);
if (repos === undefined) {
repos = new Map();
Expand Down
16 changes: 16 additions & 0 deletions test/unit/provider-track-record-engine.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,22 @@ describe("computeProviderTrackRecords (#8228)", () => {
expect(aRepo).toMatchObject({ signals: aOverall.signals, decided: aOverall.decided, precision: aOverall.precision });
});

// #8876: a provider re-reviewing the same PR (a second audit row after a push) must count ONCE per
// (provider, targetKey), scored on the LATEST vote — not inflate its signals/precision/agreement per raw row.
it("collapses a provider's repeated same-target signals to the latest, not double-counting (#8876)", () => {
const records = computeProviderTrackRecords(
[
signal("solo", "acme/widgets#1", "pass"), // first review
signal("solo", "acme/widgets#1", "fail"), // re-review after a new push — the latest vote wins
],
[labeled("acme/widgets#1", "confirmed")],
);
const overall = records.find((r) => r.provider === "solo" && r.repoFullName === null)!;
// Before the fix this was signals: 2 / agreementRate: 0.5 (both raw rows counted); now one distinct pair,
// scored on the latest fail vote against the confirmed firing.
expect(overall).toMatchObject({ signals: 1, decided: 1, confirmed: 1, precision: 1, agreementRate: 1 });
});

it("keeps a one-provider corpus's consensus/split rates null — no shared targets, no consensus to measure", () => {
const records = computeProviderTrackRecords(
[signal("solo", "acme/widgets#1", "fail"), signal("solo", "acme/widgets#2", "pass")],
Expand Down