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
10 changes: 9 additions & 1 deletion src/orb/analytics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,13 +174,21 @@ export interface FleetAnalytics {
cycleTimeObservable: boolean;
}

function median(xs: number[]): number | null {
/** The fleet-aggregation median estimator (#9645) — the single one both computeFleetAnalytics and the federated
* peer benchmark (federated-benchmark.ts) share. Sorts a COPY internally (callers need not pre-sort) and, at
* even n, returns the MEAN of the two middle values — unlike {@link percentile}(…, 50)'s nearest-rank, which
* returns the upper-middle value. Null for an empty input. */
export function median(xs: number[]): number | null {
if (xs.length === 0) return null;
const s = [...xs].sort((a, b) => a - b);
const mid = Math.floor(s.length / 2);
return s.length % 2 === 0 ? (s[mid - 1]! + s[mid]!) / 2 : s[mid]!;
}

/** Nearest-rank percentile. PRECONDITION: `sorted` MUST already be ascending-sorted — this function does NOT
* sort. Satisfied by its two call sites, cycleP50Ms and cycleP95Ms below, which pass an already-sorted array.
* For a true median use {@link median} (which sorts and averages the two middle values at even n) — a p=50
* call here is nearest-rank, not a median. */
export function percentile(sorted: number[], p: number): number | null {
if (sorted.length === 0) return null;
// Nearest-rank: the p-th percentile is the value at 1-based rank ceil(p/100 * N), i.e. index
Expand Down
15 changes: 8 additions & 7 deletions src/orb/federated-benchmark.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import { buildFederatedBundle, isFederatedIntelligenceEnabled, resolveFederatedWindowDays } from "./federated-bundle";
import { applyFederatedPeerWatermarks, importPeerBundles } from "./federated-import";
import { pullPeerBundles, pushFederatedBundle, type CollectorOpts } from "./federated-collector";
import { percentile } from "./analytics";
import { median } from "./analytics";
import { loadRepoFocusManifest } from "../signals/focus-manifest-loader";
import { resolveLoopOverSelfRepoFullName } from "../config/loopover-repo-focus-manifest";
import type { FocusManifest } from "../signals/focus-manifest";
Expand Down Expand Up @@ -140,18 +140,19 @@ export async function refreshFederatedBenchmarkCache(
const stateless = importPeerBundles(manifest, peerBundles, { now, localWindowDays });
const { accepted } = config ? await applyFederatedPeerWatermarks(db, stateless, config.peerKeys, { now }) : stateless;

// MEDIAN, NOT MEAN (mirrors analytics.ts's own fleet aggregation, see federated-import.ts's header comment):
// a bounded number of outliers cannot drag a median arbitrarily, so re-deriving a mean here would quietly
// weaken the same poisoning-resistance property the import side already relies on holding by construction.
// MEDIAN, NOT MEAN, and via analytics.ts's OWN `median` — the identical estimator computeFleetAnalytics uses
// for the fleet median (#9645). A nearest-rank percentile(…, 50) differed on an even-sized peer set (it
// returns the upper-middle value, not the mean of the two middle ones), so the peer benchmark and the fleet
// aggregation could disagree; sharing one function makes them agree by construction. A bounded number of
// outliers cannot drag a median arbitrarily, preserving the poisoning-resistance the import side relies on.
// `accepted` is deduped one-bundle-per-instanceId by importPeerBundles (#9148), so this count is already a
// count of distinct CONTRIBUTING INSTANCES, not raw bundles — the bug the peerCount doc used to warn about.
const peerMergePrecisions = accepted
.map((bundle) => bundle.mergePrecision)
.filter((value): value is number => value !== null)
.sort((a, b) => a - b);
.filter((value): value is number => value !== null);

await writeFederatedBenchmarkCache(db, {
peerMedianMergePrecision: percentile(peerMergePrecisions, 50),
peerMedianMergePrecision: median(peerMergePrecisions),
peerCount: peerMergePrecisions.length,
refreshedAt: new Date(now).toISOString(),
});
Expand Down
26 changes: 22 additions & 4 deletions test/unit/federated-benchmark.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,24 @@ describe("buildFederatedBenchmark() — reads whatever refreshFederatedBenchmark
expect(result?.peerCount).toBe(1);
});

it("uses analytics.ts's median (mean of the two middle values), so 0.4 + 0.8 → 0.6, not the nearest-rank 0.4 (#9645)", async () => {
const db = makeDb();
for (let pr = 1; pr <= 5; pr++) await resolved(db, pr);
// Two DISTINCT peer instances (importPeerBundles dedupes by instanceId) with precisions 0.4 and 0.8.
const fetchFn = fetchReturning([
signedWith(PEER_KEY_A, { instanceId: "peer-a-inst", mergePrecision: 0.4 }),
signedWith(PEER_KEY_B, { instanceId: "peer-b-inst", mergePrecision: 0.8 }),
]);

await refreshFederatedBenchmarkCache(manifest({ peerKeys: [PEER_KEY_A, PEER_KEY_B] }), db, { now: NOW, fetchFn });
const result = await buildFederatedBenchmark(manifest({ peerKeys: [PEER_KEY_A, PEER_KEY_B] }), db, { now: NOW });

// A true median: (0.4 + 0.8) / 2 = 0.6 (float-close). The old nearest-rank percentile(…, 50) returned a
// single middle element (0.4 for this even-sized set), not the mean of the two — that was the bug.
expect(result?.peerMedianMergePrecision).toBeCloseTo(0.6, 10);
expect(result?.peerCount).toBe(2);
});

it("reports a null local precision (below MIN_DECIDED) while still surfacing a cached peer median", async () => {
const db = makeDb();
await resolved(db, 1); // only 1 decided PR, below MIN_DECIDED (5)
Expand Down Expand Up @@ -245,12 +263,12 @@ describe("refreshFederatedBenchmarkCache() — the background-tick write path (#
]);

await refreshFederatedBenchmarkCache(manifest({ peerKeys: [PEER_KEY_A, PEER_KEY_B] }), db, { now: NOW, fetchFn });
// Median of [0.5, 0.9] (the untrusted 0.01 is rejected, not merely a low outlier) is 0.5 under this
// module's nearest-rank percentile(50) (analytics.ts: idx = ceil(0.5*2)-1 = 0) — pinning the real
// cross-module contract, not a re-derivation.
// Median of [0.5, 0.9] (the untrusted 0.01 is rejected, not merely a low outlier) is (0.5 + 0.9) / 2 = 0.7
// via analytics.ts's shared `median` — the SAME estimator computeFleetAnalytics uses (#9645). This used to
// read 0.5 under the module's own nearest-rank percentile(50); the two halves now agree.
const result = await buildFederatedBenchmark(manifest({ peerKeys: [PEER_KEY_A, PEER_KEY_B] }), db, { now: NOW });
expect(result?.peerCount).toBe(2);
expect(result?.peerMedianMergePrecision).toBe(0.5);
expect(result?.peerMedianMergePrecision).toBe(0.7);
});

it("a second tick's median REPLACES the first's, rather than merging the two", async () => {
Expand Down