Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
23 changes: 21 additions & 2 deletions src/upstream/ruleset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -603,7 +603,17 @@ function semanticPayload(payload: RulesetPayload): Record<string, JsonValue> {
}

type RulesetRegistryRepo = RulesetPayload["registry"]["repositories"][number];
type RegistryHyperparameterDriftPayload = RegistryHyperparameterDriftSummary & { events: RegistryHyperparameterDriftEvent[] };
// `affectedRepos` is the pre-cap list of distinct affected repositories, kept on the stored payload
// (not the public summary) so multi-report aggregation can union repos across reports instead of
// summing per-report counts. Mirrors how `affectedFields`/`affectedSurfaces` are unioned.
type RegistryHyperparameterDriftPayload = RegistryHyperparameterDriftSummary & {
events: RegistryHyperparameterDriftEvent[];
affectedRepos: string[];
};

function uniqueRepoNames(events: RegistryHyperparameterDriftEvent[]): string[] {
return [...new Set(events.map((event) => event.repoFullName))].sort((left, right) => left.localeCompare(right));
}

const REGISTRY_DRIFT_FIELD_ORDER: RegistryHyperparameterDriftField[] = [
"repo",
Expand Down Expand Up @@ -654,6 +664,7 @@ function buildRegistryHyperparameterDrift(previous: RulesetRegistryRepo[], curre
return {
...summarizeRegistryHyperparameterDriftEvents(events),
events: capped,
affectedRepos: uniqueRepoNames(events),
omittedEvents: Math.max(events.length - capped.length, 0),
};
}
Expand Down Expand Up @@ -710,7 +721,10 @@ function summarizeRegistryHyperparameterDriftReports(reports: UpstreamDriftRepor
totalEvents: sum(payloads.map((payload) => payload.totalEvents)) || fallbackSummary.totalEvents,
omittedEvents: sum(payloads.map((payload) => payload.omittedEvents)),
highImpactCount: sum(payloads.map((payload) => payload.highImpactCount)) || fallbackSummary.highImpactCount,
affectedRepoCount: sum(payloads.map((payload) => payload.affectedRepoCount)) || fallbackSummary.affectedRepoCount,
// Distinct repos across reports, not the sum of per-report unique counts -- a repo affected in
// several open reports must be counted once. Union the pre-cap repo lists (same approach as
// affectedFields/affectedSurfaces below).
affectedRepoCount: new Set(payloads.flatMap((payload) => payload.affectedRepos)).size || fallbackSummary.affectedRepoCount,
affectedFields: uniqueSorted(payloads.flatMap((payload) => payload.affectedFields), REGISTRY_DRIFT_FIELD_ORDER),
affectedSurfaces: uniqueSorted(
payloads.flatMap((payload) => payload.affectedSurfaces),
Expand All @@ -726,6 +740,7 @@ function readRegistryHyperparameterDriftPayload(value: JsonValue | undefined): R
const fallback = summarizeRegistryHyperparameterDriftEvents(events);
const affectedFields = arrayPayload(payload.affectedFields).flatMap(readRegistryHyperparameterDriftField);
const affectedSurfaces = arrayPayload(payload.affectedSurfaces).flatMap(readRegistryDriftSurface);
const affectedRepos = arrayPayload(payload.affectedRepos).filter((entry): entry is string => typeof entry === "string");
return {
events,
totalEvents: numberPayload(payload.totalEvents) ?? fallback.totalEvents,
Expand All @@ -734,6 +749,9 @@ function readRegistryHyperparameterDriftPayload(value: JsonValue | undefined): R
affectedRepoCount: numberPayload(payload.affectedRepoCount) ?? fallback.affectedRepoCount,
affectedFields: affectedFields.length > 0 ? affectedFields : fallback.affectedFields,
affectedSurfaces: affectedSurfaces.length > 0 ? affectedSurfaces : fallback.affectedSurfaces,
// Legacy payloads predate `affectedRepos`; derive it from the stored (capped) events so the
// reports aggregator can still union repos rather than fall back to summing.
affectedRepos: affectedRepos.length > 0 ? affectedRepos : uniqueRepoNames(events),
};
}

Expand Down Expand Up @@ -766,6 +784,7 @@ function emptyRegistryHyperparameterDriftPayload(): RegistryHyperparameterDriftP
affectedRepoCount: 0,
affectedFields: [],
affectedSurfaces: [],
affectedRepos: [],
};
}

Expand Down
31 changes: 30 additions & 1 deletion test/unit/upstream-ruleset.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -605,13 +605,42 @@ describe("upstream ruleset drift tracking", () => {
totalEvents: 4,
omittedEvents: 1,
highImpactCount: 3,
affectedRepoCount: 3,
// Distinct repos across reports, not the sum of per-report counts: only `owner/repo` and
// `owner/missing-values` are identifiable (the summary-only payload reports a count with no
// repo identity to union), so the deduped count is 2 -- previously this summed to 3.
affectedRepoCount: 2,
affectedFields: ["repo", "maintainerCut"],
affectedSurfaces: ["maintainer_economics"],
},
});
});

it("counts distinct affected repos across open drift reports instead of summing per-report counts", async () => {
const env = createTestEnv();
await persistUpstreamRulesetSnapshot(env, ruleset("current", "current-hash", "pending_saturation_model", 1, 0.01, new Date().toISOString()));
const driftEvent = (repoFullName: string) => ({
repoFullName,
field: "maintainerCut",
previous: 0.1,
current: 0.2,
severity: "high",
affectedSurfaces: ["maintainer_economics"],
summary: `${repoFullName} maintainerCut changed`,
});
await upsertUpstreamDriftReport(
env,
driftReport("registry-drift-a", { affectedAreas: ["registry"], payload: { registryHyperparameterDrift: { events: [driftEvent("owner/x"), driftEvent("owner/y"), driftEvent("owner/z")] } } }),
);
await upsertUpstreamDriftReport(
env,
driftReport("registry-drift-b", { affectedAreas: ["registry"], payload: { registryHyperparameterDrift: { events: [driftEvent("owner/z"), driftEvent("owner/w")] } } }),
);

const status = await loadUpstreamStatus(env);
// owner/z is affected in both reports: 4 distinct repos (x, y, z, w), not 3 + 2 = 5.
expect(status.registryHyperparameterDrift.affectedRepoCount).toBe(4);
});

it("builds low-severity source drift reports from legacy or partial ruleset payloads", async () => {
const previous = {
...ruleset("legacy-previous", "legacy-previous-hash", "pending_saturation_model", 1, 0.01, "2026-05-30T00:00:00.000Z"),
Expand Down