Skip to content

Commit c5a6cb4

Browse files
authored
fix(orb): report null mergeRate for a zero-sample slop band (#9921)
buildSlopOutcomeCalibration fabricated a 0% merge rate for a slop band with no resolved PRs, which for a discrimination table reads as "every PR in this band was closed" -- the strongest possible claim the table can make. Mirror overallMergeRate's existing null-for-empty treatment: SlopBandCalibration.mergeRate is now number | null, computeDiscriminates narrows sampled bands with a type guard instead of comparing against a nullable field, and the MCP CLI's shared n/a-below-sample renderer now actually exercises its null arm for a per-band rate. Widened the OpenAPI response shape and the UI card's type to match. Co-authored-by: bitfathers94 <237535319+bitfathers94@users.noreply.github.com>
1 parent 4b57927 commit c5a6cb4

8 files changed

Lines changed: 122 additions & 8 deletions

File tree

apps/loopover-ui/public/openapi.json

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15272,7 +15272,63 @@
1527215272
"nullable": true
1527315273
},
1527415274
"slop": {
15275-
"nullable": true
15275+
"type": "object",
15276+
"properties": {
15277+
"totalResolved": {
15278+
"type": "number"
15279+
},
15280+
"bands": {
15281+
"type": "array",
15282+
"items": {
15283+
"type": "object",
15284+
"properties": {
15285+
"band": {
15286+
"type": "string",
15287+
"enum": [
15288+
"clean",
15289+
"low",
15290+
"elevated",
15291+
"high"
15292+
]
15293+
},
15294+
"sampleSize": {
15295+
"type": "number"
15296+
},
15297+
"merged": {
15298+
"type": "number"
15299+
},
15300+
"closed": {
15301+
"type": "number"
15302+
},
15303+
"mergeRate": {
15304+
"type": "number",
15305+
"nullable": true
15306+
}
15307+
},
15308+
"required": [
15309+
"band",
15310+
"sampleSize",
15311+
"merged",
15312+
"closed",
15313+
"mergeRate"
15314+
]
15315+
}
15316+
},
15317+
"overallMergeRate": {
15318+
"type": "number",
15319+
"nullable": true
15320+
},
15321+
"discriminates": {
15322+
"type": "boolean",
15323+
"nullable": true
15324+
}
15325+
},
15326+
"required": [
15327+
"totalResolved",
15328+
"bands",
15329+
"overallMergeRate",
15330+
"discriminates"
15331+
]
1527615332
},
1527715333
"recommendations": {
1527815334
"nullable": true

apps/loopover-ui/src/components/site/app-panels/slop-band-calibration-card.tsx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ export type SlopBandCalibration = {
1212
sampleSize: number;
1313
merged: number;
1414
closed: number;
15-
mergeRate: number;
15+
mergeRate: number | null;
1616
};
1717

1818
export type SlopOutcomeCalibration = {
@@ -38,7 +38,9 @@ function discriminationPill(discriminates: boolean | null): { status: Status; la
3838

3939
function BandRow({ row }: { row: SlopBandCalibration }) {
4040
const hasSamples = row.sampleSize > 0;
41-
const pct = hasSamples ? Math.round(row.mergeRate * 100) : null;
41+
// hasSamples (sampleSize > 0) already guarantees mergeRate is non-null server-side; the fallback here is
42+
// unreachable and exists only to satisfy the wider (nullable) type.
43+
const pct = hasSamples ? Math.round((row.mergeRate ?? 0) * 100) : null;
4244
return (
4345
<div className="space-y-1.5">
4446
<div className="flex items-center justify-between gap-3 text-token-sm">

packages/loopover-mcp/bin/loopover-mcp.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2897,6 +2897,9 @@ export async function maintainCli(args: readonly string[]) {
28972897
const payload = await apiGet(`${repoBase}/outcome-calibration${query}`);
28982898
const window = payload.windowDays ? `last ${payload.windowDays}d` : "all history";
28992899
const recommendations = payload.recommendations ?? {};
2900+
// #9641: a zero-sample slop band reports mergeRate null server-side (never a fabricated 0, which for this
2901+
// discrimination table would read as "every PR in this band was closed") -- this shared null/undefined
2902+
// guard already renders that as "n/a (below sample)" rather than coercing it into "0%".
29002903
const rate = (value: any) => (value === null || value === undefined ? "n/a (below sample)" : `${Math.round(value * 100)}%`);
29012904
const lines = [
29022905
`Outcome calibration for ${repoFullName} (${window}): recommendations ${recommendations.positive ?? 0} positive, ${recommendations.negative ?? 0} negative, ${recommendations.pending ?? 0} pending (positive rate ${rate(recommendations.positiveRate)}).`,

src/openapi/schemas.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3290,7 +3290,24 @@ export const OutcomeCalibrationResponseSchema = z
32903290
repoFullName: z.string().optional(),
32913291
generatedAt: z.string().optional(),
32923292
windowDays: z.number().nullable().optional(),
3293-
slop: z.unknown().optional(),
3293+
// #9641: a band with sampleSize 0 (no resolved PRs) reports mergeRate null -- never a fabricated 0, which
3294+
// for a discrimination table would read as "every PR in this band was closed".
3295+
slop: z
3296+
.object({
3297+
totalResolved: z.number(),
3298+
bands: z.array(
3299+
z.object({
3300+
band: z.enum(["clean", "low", "elevated", "high"]),
3301+
sampleSize: z.number(),
3302+
merged: z.number(),
3303+
closed: z.number(),
3304+
mergeRate: z.number().nullable(),
3305+
}),
3306+
),
3307+
overallMergeRate: z.number().nullable(),
3308+
discriminates: z.boolean().nullable(),
3309+
})
3310+
.optional(),
32943311
recommendations: z.unknown().optional(),
32953312
signals: z.array(z.string()).optional(),
32963313
status: z.string().optional(),

src/services/outcome-calibration.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ const SLOP_BAND_ORDER: readonly SlopBand[] = ["clean", "low", "elevated", "high"
2626
// Below this per-band sample the merge rate is too noisy to judge discrimination.
2727
const MIN_BAND_SAMPLE = 5;
2828

29-
export type SlopBandCalibration = { band: SlopBand; sampleSize: number; merged: number; closed: number; mergeRate: number };
29+
export type SlopBandCalibration = { band: SlopBand; sampleSize: number; merged: number; closed: number; mergeRate: number | null };
3030

3131
export type SlopOutcomeCalibration = {
3232
totalResolved: number;
@@ -107,7 +107,9 @@ export function buildSlopOutcomeCalibration(pullRequests: PullRequestRecord[], o
107107
const bands: SlopBandCalibration[] = SLOP_BAND_ORDER.map((band) => {
108108
const { merged, closed } = counts.get(band) ?? { merged: 0, closed: 0 };
109109
const sampleSize = merged + closed;
110-
return { band, sampleSize, merged, closed, mergeRate: sampleSize > 0 ? round(merged / sampleSize) : 0 };
110+
// Mirrors overallMergeRate below: a band nobody has data for reports null ("unknown"), never a fabricated
111+
// 0 ("every PR in this band was closed"), the strongest possible discrimination claim (#9641).
112+
return { band, sampleSize, merged, closed, mergeRate: sampleSize > 0 ? round(merged / sampleSize) : null };
111113
});
112114
return {
113115
totalResolved,
@@ -117,8 +119,14 @@ export function buildSlopOutcomeCalibration(pullRequests: PullRequestRecord[], o
117119
};
118120
}
119121

122+
// A sampled band's sampleSize (>= MIN_BAND_SAMPLE, so > 0) always yields a non-null mergeRate above; this
123+
// predicate narrows the type so the comparison below doesn't need a runtime null check on an unreachable case.
124+
function isSampledBand(band: SlopBandCalibration): band is SlopBandCalibration & { mergeRate: number } {
125+
return band.sampleSize >= MIN_BAND_SAMPLE;
126+
}
127+
120128
function computeDiscriminates(bands: SlopBandCalibration[]): boolean | null {
121-
const sampled = bands.filter((band) => band.sampleSize >= MIN_BAND_SAMPLE); // already in severity order
129+
const sampled = bands.filter(isSampledBand); // already in severity order
122130
if (sampled.length < 2) return null; // not enough signal to judge
123131
for (let index = 1; index < sampled.length; index += 1) {
124132
// A later (higher-severity) band merging MORE than an earlier one means the score is not discriminating.

test/unit/mcp-cli-maintain.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ beforeEach(() => {
8989
planIssuesBodies.length = 0;
9090
apiRequests.length = 0;
9191
fixtureOptions.repoDocRefresh = undefined;
92+
fixtureOptions.outcomeCalibrationBands = undefined;
9293
});
9394

9495
async function captureStdout(fn: () => Promise<void>): Promise<string> {
@@ -396,6 +397,18 @@ describe("loopover-mcp CLI — maintain (#784)", () => {
396397
expect(scoped).toMatch(/Outcome calibration for owner\/repo \(last 30d\)/);
397398
});
398399

400+
// REGRESSION (#9641): a zero-sample band's mergeRate is null (not a fabricated 0), and the plain-text
401+
// renderer must show that as "n/a", matching the sampled bands' own "n/a (below sample)" wording.
402+
it("outcome-calibration renders a zero-sample band's null mergeRate as n/a, not 0%", async () => {
403+
fixtureOptions.outcomeCalibrationBands = [
404+
{ band: "clean", sampleSize: 12, merged: 9, closed: 3, mergeRate: 0.75 },
405+
{ band: "high", sampleSize: 0, merged: 0, closed: 0, mergeRate: null },
406+
];
407+
const out = await cli(["maintain", "outcome-calibration", "--repo", "owner/repo"]);
408+
expect(out).toMatch(/high: n\/a \(below sample\) merge rate over 0 PR\(s\)/);
409+
expect(out).not.toMatch(/high: 0% merge rate/);
410+
});
411+
399412
it("onboarding-pack mirrors the session-gated API payload and forwards refresh", async () => {
400413
const json = JSON.parse(
401414
await cli([

test/unit/outcome-calibration.test.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,18 @@ describe("buildSlopOutcomeCalibration", () => {
5555
expect(result.totalResolved).toBe(4);
5656
});
5757

58+
// REGRESSION (#9641): a band with zero resolved PRs reports mergeRate null -- never a fabricated 0, which
59+
// for a discrimination table would read as "every PR in this band was closed" -- and computeDiscriminates'
60+
// verdict over the remaining sampled bands is unaffected by the null band.
61+
it("reports a zero-sample band's mergeRate as null, leaving the verdict over the sampled bands unchanged", () => {
62+
const result = buildSlopOutcomeCalibration([...band("clean", 6, 5, 0), ...band("low", 6, 4, 100), ...band("elevated", 6, 1, 200)]);
63+
// "high" has no resolved PRs at all -- one zero-sample band alongside three sampled ones.
64+
const high = result.bands.find((b) => b.band === "high")!;
65+
expect(high.sampleSize).toBe(0);
66+
expect(high.mergeRate).toBeNull();
67+
expect(result.discriminates).toBe(true); // clean 0.833 > low 0.667 > elevated 0.167 -- non-increasing
68+
});
69+
5870
it("excludes open PRs and PRs with no slop assessment", () => {
5971
const open: PullRequestRecord = { repoFullName: "owner/repo", number: 9, title: "open", state: "open", labels: [], linkedIssues: [], slopRisk: 70, slopBand: "high" };
6072
const unassessed: PullRequestRecord = { repoFullName: "owner/repo", number: 10, title: "no slop", state: "closed", mergedAt: "2026-06-01T00:00:00.000Z", labels: [], linkedIssues: [] };

test/unit/support/mcp-cli-harness.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,9 @@ export async function startFixtureServer(
205205
onApiRequest?: (request: IncomingMessage) => void;
206206
/** #9300: captures DELETE /v1/repos/:owner/:repo/selftune/overrides body ({ confirm }). */
207207
onClearSelftuneOverride?: (body: { confirm?: boolean }) => void;
208+
/** #9641: overrides the outcome-calibration route's `slop` band list -- lets a test drive a zero-sample
209+
* band (mergeRate: null) through the CLI's plain-text renderer without touching the other two bands. */
210+
outcomeCalibrationBands?: unknown[] | undefined;
208211
validateConfigWarnings?: string[];
209212
openPrMonitor?: Record<string, unknown>;
210213
prOutcomes?: Record<string, unknown>;
@@ -842,7 +845,7 @@ export async function startFixtureServer(
842845
repoFullName: "owner/repo",
843846
generatedAt: "2026-05-30T00:00:00.000Z",
844847
windowDays: windowDays ? Number(windowDays) : null,
845-
slop: [
848+
slop: options.outcomeCalibrationBands ?? [
846849
{ band: "clean", sampleSize: 12, merged: 9, closed: 3, mergeRate: 0.75 },
847850
{ band: "high", sampleSize: 4, merged: 1, closed: 3, mergeRate: 0.25 },
848851
],

0 commit comments

Comments
 (0)