Skip to content

[Bug]: summarizeRegistryHyperparameterDriftReports sums each report's unique-repo count instead of deduplicating, inflating affectedRepoCount across multiple open drift reports #396

Description

@galuis116

Summary

When the upstream-status surface aggregates registry hyperparameter drift across all open drift reports, it sums each report's affectedRepoCount rather than counting unique repositories across reports:

// src/upstream/ruleset.ts:705-719
function summarizeRegistryHyperparameterDriftReports(reports: UpstreamDriftReportRecord[]): RegistryHyperparameterDriftSummary {
  const payloads = reports.map((report) => readRegistryHyperparameterDriftPayload(report.payload.registryHyperparameterDrift));
  const events = payloads.flatMap((payload) => payload.events);
  const fallbackSummary = summarizeRegistryHyperparameterDriftEvents(events);
  return {
    totalEvents: sum(payloads.map((payload) => payload.totalEvents)) || fallbackSummary.totalEvents,       // additive: OK (distinct events)
    omittedEvents: sum(payloads.map((payload) => payload.omittedEvents)),                                  // additive: OK
    highImpactCount: sum(payloads.map((payload) => payload.highImpactCount)) || fallbackSummary.highImpactCount, // additive: OK (distinct events)
    affectedRepoCount: sum(payloads.map((payload) => payload.affectedRepoCount)) || fallbackSummary.affectedRepoCount, // <- BUG: sums unique-repo counts
    affectedFields: uniqueSorted(payloads.flatMap((payload) => payload.affectedFields), REGISTRY_DRIFT_FIELD_ORDER),     // deduped across reports
    affectedSurfaces: uniqueSorted(payloads.flatMap((payload) => payload.affectedSurfaces), [...]),                      // deduped across reports
  };
}

affectedRepoCount is defined everywhere else as a count of distinct repositories. The canonical single-report builder computes it as a set size:

// src/upstream/ruleset.ts:691-702 (summarizeRegistryHyperparameterDriftEvents)
affectedRepoCount: new Set(events.map((event) => event.repoFullName)).size,

So each payload.affectedRepoCount is the number of unique repos within that one report. Summing those across reports counts a repository once for every report it appears in. A repo whose hyperparameters drift across three open drift snapshots contributes 3 to the total instead of 1.

Why this is wrong

The aggregation is internally inconsistent, which makes the intent unambiguous:

  • affectedFields and affectedSurfaces are deduplicated across reports with uniqueSorted(payloads.flatMap(...)).
  • The sibling summarizeRegistryHyperparameterDriftEvents (single-report path) defines affectedRepoCount as new Set(repos).size.
  • The same struct's fallbackSummary.affectedRepoCount (computed from the flattened events) is already the cross-report deduplicated count — it's right there, but only used as the || 0 fallback.

The author clearly intended the union semantics (fields/surfaces prove it) but used sum for the repo count. The additive fields next to it (totalEvents, omittedEvents, highImpactCount) are genuinely additive because each report's events are distinct occurrences — but repositories are not: the same repo recurs across reports, so its count must be unioned, not summed.

Reachability

  • loadUpstreamStatus (ruleset.ts:210-214) calls this with openReports = reports.filter((r) => r.status === "open"), where reports = listUpstreamDriftReports(env, 20)up to 20 open reports. Whenever two or more open drift reports share an affected repo, the count inflates. (Reports stay "open" until their filed GitHub drift issue is resolved, so multiple concurrent open reports is the normal steady state once any drift exists.)
  • The result is returned as UpstreamStatus.registryHyperparameterDrift.affectedRepoCount (ruleset.ts:227) and served by GET /v1/upstream/status (src/api/routes.ts:1221) and embedded in other status payloads (routes.ts:1232, the admin/diagnostics surfaces) — i.e. it is observable to operators/dashboards.

Failure mode (concrete example)

Two open drift reports:

  • Report A affected repos {X, Y, Z}payload.affectedRepoCount = 3.

  • Report B affected repos {Z, W}payload.affectedRepoCount = 2.

  • Current: affectedRepoCount = 3 + 2 = 5.

  • Correct: |{X, Y, Z, W}| = 4 (repo Z is one repository, not two).

A dashboard reading the upstream status reports "5 repositories affected by registry drift" when only 4 distinct repos are affected — and the overstatement grows with the number of open reports and the amount of repo overlap between them.

Steps to reproduce

  1. Have two or more open UpstreamDriftReportRecords whose registryHyperparameterDrift payloads share at least one repoFullName.
  2. Call loadUpstreamStatus(env) (or GET /v1/upstream/status).
  3. Observe registryHyperparameterDrift.affectedRepoCount exceeds the number of distinct affected repositories (it equals the sum of per-report unique counts).

Expected behavior

affectedRepoCount is the number of distinct repositories affected across all open reports, consistent with affectedFields/affectedSurfaces (deduped across reports) and with the single-report new Set(repos).size definition.

Actual behavior

affectedRepoCount is the sum of each report's distinct-repo count, double-counting any repository that appears in more than one open report.

Suggested fix

Deduplicate the repository set across reports rather than summing the per-report counts. Mirror the affectedFields/affectedSurfaces approach by unioning per-report repo lists.

  • Preferred (exact, pre-cap): add an affectedRepos: string[] list to the drift summary/payload (populated in summarizeRegistryHyperparameterDriftEvents as uniqueSorted(events.map((e) => e.repoFullName))), then aggregate with affectedRepoCount: new Set(payloads.flatMap((payload) => payload.affectedRepos)).size. This matches how affectedFields/affectedSurfaces already union pre-cap payload lists and is robust to event capping. (readRegistryHyperparameterDriftPayload should default affectedRepos to [] for older stored payloads.)
  • Minimal (events-based): affectedRepoCount: fallbackSummary.affectedRepoCount, i.e. dedupe from the flattened events that are already available. This removes the double-count; its only caveat is a possible slight under-count when a repo's events were all omitted by per-report capping (omittedEvents > 0).

Add fail-on-revert coverage: two reports sharing a repo must yield a unioned affectedRepoCount (e.g. the example above returns 4, not 5).

Metadata

Metadata

Assignees

No one assigned

    Labels

    slopAI slop and/or attempts to game additional points via manipulation or alt profiles.

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions