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: 8 additions & 2 deletions src/review/ops-wire.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,11 @@ export async function runOpsAlerts(env: Env): Promise<Record<string, string[]>>
try {
const [gatePrecision, calibration, reviewBurst, reviewFailureBurst] = await Promise.all([
loadGatePrecisionReport(env, repoFullName),
buildRepoOutcomeCalibration(env, repoFullName),
// #orb-anomaly-slop-false-positive: exclude maintainer-authored PRs from the slop half -- they merge
// by human judgment regardless of score, so pooling them with contributor PRs can invert the
// merge-rate-by-band comparison detectOutcomeAnomalies reads below, in any repo the maintainer is
// heavily active in, without the deterministic score itself being wrong.
buildRepoOutcomeCalibration(env, repoFullName, undefined, { excludeMaintainerAuthors: true }),
findHottestReviewTargetForRepo(env, repoFullName, reviewBurstSinceIso),
findHottestInconclusiveReviewTargetForRepo(env, repoFullName, reviewBurstSinceIso),
]);
Expand Down Expand Up @@ -303,7 +307,9 @@ export async function computeOpsStats(env: Env): Promise<OpsStatsPayload> {
try {
const [gatePrecision, calibration, reviewBurst, reviewFailureBurst, byokUsage] = await Promise.all([
loadGatePrecisionReport(env, repoFullName),
buildRepoOutcomeCalibration(env, repoFullName),
// #orb-anomaly-slop-false-positive: same maintainer exclusion as runOpsAlerts above, so this row's
// `slop`/`anomalies` fields stay consistent with each other and with the cron alert.
buildRepoOutcomeCalibration(env, repoFullName, undefined, { excludeMaintainerAuthors: true }),
findHottestReviewTargetForRepo(env, repoFullName, reviewBurstSinceIso),
findHottestInconclusiveReviewTargetForRepo(env, repoFullName, reviewBurstSinceIso),
sumByokAiUsageForRepoSince(env, repoFullName, byokUsageSinceIso),
Expand Down
24 changes: 21 additions & 3 deletions src/services/outcome-calibration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,15 @@ export type RecommendationOutcomeCalibration = { total: number; positive: number
export type RecommendationOutcomeCalibrationOptions = {
/** Only maintainer-lane outcomes are authoritative enough for live self-tune policy changes. */
maintainerOnly?: boolean | undefined;
/** Exclude maintainer-authored PRs (author_association OWNER/MEMBER/COLLABORATOR) from the SLOP half of the
* report. Maintainer PRs merge by human judgment regardless of score (the same population
* settings-preview.ts's `includeMaintainerAuthors` already excludes from the public surface by default), so
* pooling them with contributor PRs can invert a merge-rate-by-band comparison in any repo the maintainer is
* heavily active in without the deterministic score itself being wrong. Off by default so every existing
* caller (dashboards, the MCP tool, the public API route) stays byte-identical; mirrors the miner/human
* `cohorts` split gate-precision.ts already applies to the sibling #554 false-positive measurement, for the
* same reason. */
excludeMaintainerAuthors?: boolean | undefined;
};

export type OutcomeCalibration = {
Expand All @@ -57,12 +66,21 @@ function terminalOutcome(pr: PullRequestRecord): "merged" | "closed" | null {
return null;
}

// Same OWNER/MEMBER/COLLABORATOR classification settings-preview.ts's includeMaintainerAuthors check already
// uses to exclude this population from the public surface by default.
const MAINTAINER_AUTHOR_ASSOCIATIONS = new Set(["OWNER", "MEMBER", "COLLABORATOR"]);

function isMaintainerAuthoredPr(pr: PullRequestRecord): boolean {
return pr.authorAssociation != null && MAINTAINER_AUTHOR_ASSOCIATIONS.has(pr.authorAssociation);
}

/** Per-slop-band merge/close calibration over the resolved PRs that carry a slop assessment. Pure. */
export function buildSlopOutcomeCalibration(pullRequests: PullRequestRecord[]): SlopOutcomeCalibration {
export function buildSlopOutcomeCalibration(pullRequests: PullRequestRecord[], options: RecommendationOutcomeCalibrationOptions = {}): SlopOutcomeCalibration {
const scoped = options.excludeMaintainerAuthors ? pullRequests.filter((pr) => !isMaintainerAuthoredPr(pr)) : pullRequests;
const counts = new Map<SlopBand, { merged: number; closed: number }>();
let totalMerged = 0;
let totalResolved = 0;
for (const pr of pullRequests) {
for (const pr of scoped) {
if (typeof pr.slopRisk !== "number" || !pr.slopBand) continue; // never assessed
const band = pr.slopBand as SlopBand;
if (!SLOP_BAND_ORDER.includes(band)) continue;
Expand Down Expand Up @@ -158,7 +176,7 @@ export async function buildRepoOutcomeCalibration(
listPullRequests(env, repoFullName),
listAgentRecommendationOutcomes(env, windowDays !== undefined ? { repoFullName, windowDays } : { repoFullName }),
]);
const slop = buildSlopOutcomeCalibration(pullRequests);
const slop = buildSlopOutcomeCalibration(pullRequests, options);
const recommendations = buildRecommendationOutcomeCalibration(outcomes, repoFullName, options);
return { repoFullName, generatedAt: nowIso(), windowDays: windowDays ?? null, slop, recommendations, signals: buildOutcomeCalibrationSignals(slop, recommendations) };
}
69 changes: 68 additions & 1 deletion test/unit/ops-wire.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { afterEach, describe, expect, it, vi } from "vitest";
import { createApp } from "../../src/api/routes";
import { recordAiUsageEvent, recordAuditEvent, recordGateBlockOutcome, upsertPullRequestFromGitHub } from "../../src/db/repositories";
import { recordAiUsageEvent, recordAuditEvent, recordGateBlockOutcome, updatePullRequestSlopAssessment, upsertPullRequestFromGitHub } from "../../src/db/repositories";
import {
classifyAnomalySeverity,
computeOpsStats,
Expand Down Expand Up @@ -221,6 +221,27 @@ async function seedAgentConfiguredRepo(env: Env, fullName: string, installationI
.run();
}

// Seed a slop-band population where contributor PRs alone discriminate correctly (clean 0.667 > high 0.167),
// but `maintainerHighCount` maintainer-authored "high" PRs that ALL merge anyway (real maintainer behavior --
// merged by human judgment, not the score) are also present. With maintainerHighCount=10 the blended high-band
// rate (11/16=0.6875) edges just over the contributor-only clean rate (0.667), inverting the BLENDED signal
// without either population's own signal actually being inverted -- the #orb-anomaly-slop-false-positive confound.
async function seedSlopMaintainerConfound(env: Env, repoFullName: string, maintainerHighCount: number): Promise<void> {
let number = 1;
for (let i = 0; i < 6; i += 1, number += 1) {
await upsertPullRequestFromGitHub(env, repoFullName, { number, title: `contributor clean ${number}`, state: "closed", user: { login: "contributor" }, author_association: "CONTRIBUTOR", merged_at: i < 4 ? "2026-06-01T00:00:00.000Z" : null } as never);
await updatePullRequestSlopAssessment(env, repoFullName, number, { slopRisk: 0, slopBand: "clean" });
}
for (let i = 0; i < 6; i += 1, number += 1) {
await upsertPullRequestFromGitHub(env, repoFullName, { number, title: `contributor high ${number}`, state: "closed", user: { login: "contributor" }, author_association: "CONTRIBUTOR", merged_at: i < 1 ? "2026-06-01T00:00:00.000Z" : null } as never);
await updatePullRequestSlopAssessment(env, repoFullName, number, { slopRisk: 70, slopBand: "high" });
}
for (let i = 0; i < maintainerHighCount; i += 1, number += 1) {
await upsertPullRequestFromGitHub(env, repoFullName, { number, title: `maintainer high ${number}`, state: "closed", user: { login: "owner" }, author_association: "OWNER", merged_at: "2026-06-01T00:00:00.000Z" } as never);
await updatePullRequestSlopAssessment(env, repoFullName, number, { slopRisk: 70, slopBand: "high" });
}
}

// Seed a gate-block ledger anomaly: blocked PRs that later MERGED (false positives) over the min sample.
async function seedGateFalsePositiveAnomaly(env: Env, repoFullName: string): Promise<void> {
for (let i = 1; i <= 6; i += 1) {
Expand Down Expand Up @@ -276,6 +297,40 @@ describe("runOpsAlerts — cron path over gittensory's outcome data", () => {
expect(errors.mock.calls.map((c) => String(c[0])).some((line) => line.includes("ops_anomaly\""))).toBe(false);
});

it("REGRESSION (#orb-anomaly-slop-false-positive): does NOT flag 'slop score NOT discriminating' when the inversion is purely a maintainer-PR confound", async () => {
const env = createTestEnv();
await seedRegisteredRepo(env, "owner/repo");
await seedSlopMaintainerConfound(env, "owner/repo", 10); // blended would invert (verified in outcome-calibration.test.ts); contributor-only signal does not
const errors = vi.spyOn(console, "error").mockImplementation(() => {});

const found = await runOpsAlerts(env);

expect(found["owner/repo"]?.some((a) => /slop score NOT discriminating/.test(a))).not.toBe(true);
const logged = errors.mock.calls.map((c) => String(c[0])).find((line) => line.includes("ops_anomaly") && line.includes("owner/repo"));
expect(logged).toBeUndefined();
});

it("REGRESSION (#orb-anomaly-slop-false-positive): still flags a GENUINE non-discriminating score among contributor PRs alone (not a blanket suppression)", async () => {
const env = createTestEnv();
await seedRegisteredRepo(env, "owner/repo");
// Contributor-only population where a higher band genuinely merges more -- no maintainer PRs involved at all.
for (let i = 1; i <= 6; i += 1) {
await upsertPullRequestFromGitHub(env, "owner/repo", { number: i, title: `contributor clean ${i}`, state: "closed", user: { login: "contributor" }, author_association: "CONTRIBUTOR", merged_at: i <= 1 ? "2026-06-01T00:00:00.000Z" : null } as never);
await updatePullRequestSlopAssessment(env, "owner/repo", i, { slopRisk: 0, slopBand: "clean" });
}
for (let i = 7; i <= 12; i += 1) {
await upsertPullRequestFromGitHub(env, "owner/repo", { number: i, title: `contributor high ${i}`, state: "closed", user: { login: "contributor" }, author_association: "CONTRIBUTOR", merged_at: i <= 11 ? "2026-06-01T00:00:00.000Z" : null } as never);
await updatePullRequestSlopAssessment(env, "owner/repo", i, { slopRisk: 70, slopBand: "high" });
}
const errors = vi.spyOn(console, "error").mockImplementation(() => {});

const found = await runOpsAlerts(env);

expect(found["owner/repo"]?.some((a) => /slop score NOT discriminating/.test(a))).toBe(true);
const logged = errors.mock.calls.map((c) => String(c[0])).find((line) => line.includes("ops_anomaly") && line.includes("owner/repo"));
expect(logged).toBeDefined();
});

it("REGRESSION (#sweep-requires-installation): prefers the agent-configured repo and never scans an uninstalled registered repo when a configured one exists", async () => {
const env = createTestEnv();
await seedAgentConfiguredRepo(env, "owner/configured", 9501);
Expand Down Expand Up @@ -532,6 +587,18 @@ describe("computeOpsStats — cross-repo outcome aggregate", () => {
const row = payload.repos.find((r) => r.repoFullName === "owner/repo");
expect(row?.byokUsage).toEqual({ calls: 0, inputTokens: 0, outputTokens: 0, totalTokens: 0, costUsd: 0 });
});

it("REGRESSION (#orb-anomaly-slop-false-positive): the row's slop.discriminates and anomalies stay consistent -- both exclude the maintainer-PR confound", async () => {
const env = createTestEnv();
await seedRegisteredRepo(env, "owner/repo");
await seedSlopMaintainerConfound(env, "owner/repo", 10);

const payload = await computeOpsStats(env);
const row = payload.repos.find((r) => r.repoFullName === "owner/repo");
expect(row?.slop.discriminates).toBe(true); // contributor-only signal, not the inverted blended one
expect(row?.slop.totalResolved).toBe(12); // only the 12 contributor PRs, not the 10 maintainer PRs too
expect(row?.anomalies.some((a) => /slop score NOT discriminating/.test(a))).toBe(false);
});
});

describe("GET /v1/internal/ops/stats — bearer-gated, flag-gated endpoint", () => {
Expand Down
66 changes: 63 additions & 3 deletions test/unit/outcome-calibration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import type { AgentActionRecord, AgentRecommendationOutcomeRecord, AgentRecommen
import { createTestEnv } from "../helpers/d1";

// A resolved PR carrying a slop assessment. `merged` → has a merge timestamp; otherwise closed-unmerged.
function pr(band: SlopBand, merged: boolean, number: number): PullRequestRecord {
function pr(band: SlopBand, merged: boolean, number: number, authorAssociation?: string): PullRequestRecord {
return {
repoFullName: "owner/repo",
number,
Expand All @@ -24,12 +24,13 @@ function pr(band: SlopBand, merged: boolean, number: number): PullRequestRecord
linkedIssues: [],
slopRisk: band === "clean" ? 0 : band === "low" ? 10 : band === "elevated" ? 40 : 70,
slopBand: band,
...(authorAssociation ? { authorAssociation } : {}),
};
}

// n PRs in a band, `merged` of them merged (the rest closed-unmerged).
function band(b: SlopBand, n: number, merged: number, base: number): PullRequestRecord[] {
return Array.from({ length: n }, (_, i) => pr(b, i < merged, base + i));
function band(b: SlopBand, n: number, merged: number, base: number, authorAssociation?: string): PullRequestRecord[] {
return Array.from({ length: n }, (_, i) => pr(b, i < merged, base + i, authorAssociation));
}

describe("buildSlopOutcomeCalibration", () => {
Expand Down Expand Up @@ -60,6 +61,50 @@ describe("buildSlopOutcomeCalibration", () => {
const result = buildSlopOutcomeCalibration([open, unassessed, ...band("clean", 1, 1, 0)]);
expect(result.totalResolved).toBe(1); // only the one assessed+resolved PR
});

// REGRESSION (#orb-anomaly-slop-false-positive): the ops-anomaly detector flagged "slop score NOT
// discriminating" on real repos, but the score itself was correct -- a pool of maintainer-authored PRs
// (which merge by human judgment regardless of severity) inverted the blended merge-rate-by-band comparison.
describe("excludeMaintainerAuthors", () => {
it("is a no-op by default (existing callers stay byte-identical) -- a maintainer-PR pool still inverts the blended result", () => {
// Contributor PRs alone would discriminate correctly (clean 0.667 > high 0.167), but 10 maintainer
// "high" PRs that ALL merged anyway (real maintainer behavior: merged by human judgment, not the score)
// pull the blended high-band rate up to 11/16=0.6875, just over clean's 0.667 -- flipping the blended
// comparison to non-discriminating even though neither population's OWN signal is actually inverted.
const contributorClean = band("clean", 6, 4, 0, "CONTRIBUTOR");
const contributorHigh = band("high", 6, 1, 100, "CONTRIBUTOR");
const maintainerHigh = band("high", 10, 10, 200, "OWNER");
const result = buildSlopOutcomeCalibration([...contributorClean, ...contributorHigh, ...maintainerHigh]);
expect(result.discriminates).toBe(false); // the blended confound this fix addresses
});

it("excludeMaintainerAuthors:true removes the maintainer-PR confound and reveals the true (discriminating) signal", () => {
const contributorClean = band("clean", 6, 4, 0, "CONTRIBUTOR");
const contributorHigh = band("high", 6, 1, 100, "CONTRIBUTOR");
const maintainerHigh = band("high", 10, 10, 200, "OWNER");
const result = buildSlopOutcomeCalibration([...contributorClean, ...contributorHigh, ...maintainerHigh], { excludeMaintainerAuthors: true });
expect(result.totalResolved).toBe(12); // only the 12 contributor PRs counted
expect(result.discriminates).toBe(true);
});

it("excludeMaintainerAuthors:true still reports a genuine non-discriminating score among contributor PRs (not a blanket suppression)", () => {
const contributorClean = band("clean", 6, 1, 0, "CONTRIBUTOR");
const contributorHigh = band("high", 6, 5, 100, "CONTRIBUTOR");
const result = buildSlopOutcomeCalibration([...contributorClean, ...contributorHigh], { excludeMaintainerAuthors: true });
expect(result.discriminates).toBe(false);
});

it("treats MEMBER and COLLABORATOR the same as OWNER, and leaves FIRST_TIME_CONTRIBUTOR/NONE/unset untouched", () => {
const memberOnly = buildSlopOutcomeCalibration(band("high", 3, 3, 0, "MEMBER"), { excludeMaintainerAuthors: true });
const collaboratorOnly = buildSlopOutcomeCalibration(band("high", 3, 3, 0, "COLLABORATOR"), { excludeMaintainerAuthors: true });
const firstTimeOnly = buildSlopOutcomeCalibration(band("high", 3, 3, 0, "FIRST_TIME_CONTRIBUTOR"), { excludeMaintainerAuthors: true });
const unsetOnly = buildSlopOutcomeCalibration(band("high", 3, 3, 0), { excludeMaintainerAuthors: true });
expect(memberOnly.totalResolved).toBe(0);
expect(collaboratorOnly.totalResolved).toBe(0);
expect(firstTimeOnly.totalResolved).toBe(3); // not a maintainer association -- kept
expect(unsetOnly.totalResolved).toBe(3); // no association at all -- kept, never over-excluded
});
});
});

describe("buildRecommendationOutcomeCalibration", () => {
Expand Down Expand Up @@ -189,6 +234,21 @@ describe("buildRepoOutcomeCalibration (env loader)", () => {
const report = await buildRepoOutcomeCalibration(env, "owner/repo", 365);
expect(report.recommendations).toMatchObject({ total: 2, positive: 1, negative: 1, positiveRate: 0.5 });
});

it("REGRESSION (#orb-anomaly-slop-false-positive): excludeMaintainerAuthors threads through to the slop half of the loaded report", async () => {
const env = createTestEnv();
await upsertPullRequestFromGitHub(env, "owner/repo", { number: 1, title: "contributor clean", state: "closed", user: { login: "alice" }, author_association: "CONTRIBUTOR", merged_at: "2026-06-01T00:00:00.000Z" });
await updatePullRequestSlopAssessment(env, "owner/repo", 1, { slopRisk: 0, slopBand: "clean" });
await upsertPullRequestFromGitHub(env, "owner/repo", { number: 2, title: "maintainer high, merged anyway", state: "closed", user: { login: "owner" }, author_association: "OWNER", merged_at: "2026-06-01T00:05:00.000Z" });
await updatePullRequestSlopAssessment(env, "owner/repo", 2, { slopRisk: 70, slopBand: "high" });

const blended = await buildRepoOutcomeCalibration(env, "owner/repo");
expect(blended.slop.totalResolved).toBe(2);

const contributorOnly = await buildRepoOutcomeCalibration(env, "owner/repo", undefined, { excludeMaintainerAuthors: true });
expect(contributorOnly.slop.totalResolved).toBe(1);
expect(contributorOnly.slop.bands.find((b) => b.band === "high")).toMatchObject({ sampleSize: 0 });
});
});

function runRecord(id: string, actorLogin: string, createdAt: string): AgentRunRecord {
Expand Down