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
58 changes: 57 additions & 1 deletion apps/loopover-ui/public/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -15272,7 +15272,63 @@
"nullable": true
},
"slop": {
"nullable": true
"type": "object",
"properties": {
"totalResolved": {
"type": "number"
},
"bands": {
"type": "array",
"items": {
"type": "object",
"properties": {
"band": {
"type": "string",
"enum": [
"clean",
"low",
"elevated",
"high"
]
},
"sampleSize": {
"type": "number"
},
"merged": {
"type": "number"
},
"closed": {
"type": "number"
},
"mergeRate": {
"type": "number",
"nullable": true
}
},
"required": [
"band",
"sampleSize",
"merged",
"closed",
"mergeRate"
]
}
},
"overallMergeRate": {
"type": "number",
"nullable": true
},
"discriminates": {
"type": "boolean",
"nullable": true
}
},
"required": [
"totalResolved",
"bands",
"overallMergeRate",
"discriminates"
]
},
"recommendations": {
"nullable": true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export type SlopBandCalibration = {
sampleSize: number;
merged: number;
closed: number;
mergeRate: number;
mergeRate: number | null;
};

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

function BandRow({ row }: { row: SlopBandCalibration }) {
const hasSamples = row.sampleSize > 0;
const pct = hasSamples ? Math.round(row.mergeRate * 100) : null;
// hasSamples (sampleSize > 0) already guarantees mergeRate is non-null server-side; the fallback here is
// unreachable and exists only to satisfy the wider (nullable) type.
const pct = hasSamples ? Math.round((row.mergeRate ?? 0) * 100) : null;
return (
<div className="space-y-1.5">
<div className="flex items-center justify-between gap-3 text-token-sm">
Expand Down
3 changes: 3 additions & 0 deletions packages/loopover-mcp/bin/loopover-mcp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2897,6 +2897,9 @@ export async function maintainCli(args: readonly string[]) {
const payload = await apiGet(`${repoBase}/outcome-calibration${query}`);
const window = payload.windowDays ? `last ${payload.windowDays}d` : "all history";
const recommendations = payload.recommendations ?? {};
// #9641: a zero-sample slop band reports mergeRate null server-side (never a fabricated 0, which for this
// discrimination table would read as "every PR in this band was closed") -- this shared null/undefined
// guard already renders that as "n/a (below sample)" rather than coercing it into "0%".
const rate = (value: any) => (value === null || value === undefined ? "n/a (below sample)" : `${Math.round(value * 100)}%`);
const lines = [
`Outcome calibration for ${repoFullName} (${window}): recommendations ${recommendations.positive ?? 0} positive, ${recommendations.negative ?? 0} negative, ${recommendations.pending ?? 0} pending (positive rate ${rate(recommendations.positiveRate)}).`,
Expand Down
19 changes: 18 additions & 1 deletion src/openapi/schemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3290,7 +3290,24 @@ export const OutcomeCalibrationResponseSchema = z
repoFullName: z.string().optional(),
generatedAt: z.string().optional(),
windowDays: z.number().nullable().optional(),
slop: z.unknown().optional(),
// #9641: a band with sampleSize 0 (no resolved PRs) reports mergeRate null -- never a fabricated 0, which
// for a discrimination table would read as "every PR in this band was closed".
slop: z
.object({
totalResolved: z.number(),
bands: z.array(
z.object({
band: z.enum(["clean", "low", "elevated", "high"]),
sampleSize: z.number(),
merged: z.number(),
closed: z.number(),
mergeRate: z.number().nullable(),
}),
),
overallMergeRate: z.number().nullable(),
discriminates: z.boolean().nullable(),
})
.optional(),
recommendations: z.unknown().optional(),
signals: z.array(z.string()).optional(),
status: z.string().optional(),
Expand Down
14 changes: 11 additions & 3 deletions src/services/outcome-calibration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const SLOP_BAND_ORDER: readonly SlopBand[] = ["clean", "low", "elevated", "high"
// Below this per-band sample the merge rate is too noisy to judge discrimination.
const MIN_BAND_SAMPLE = 5;

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

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

// A sampled band's sampleSize (>= MIN_BAND_SAMPLE, so > 0) always yields a non-null mergeRate above; this
// predicate narrows the type so the comparison below doesn't need a runtime null check on an unreachable case.
function isSampledBand(band: SlopBandCalibration): band is SlopBandCalibration & { mergeRate: number } {
return band.sampleSize >= MIN_BAND_SAMPLE;
}

function computeDiscriminates(bands: SlopBandCalibration[]): boolean | null {
const sampled = bands.filter((band) => band.sampleSize >= MIN_BAND_SAMPLE); // already in severity order
const sampled = bands.filter(isSampledBand); // already in severity order
if (sampled.length < 2) return null; // not enough signal to judge
for (let index = 1; index < sampled.length; index += 1) {
// A later (higher-severity) band merging MORE than an earlier one means the score is not discriminating.
Expand Down
13 changes: 13 additions & 0 deletions test/unit/mcp-cli-maintain.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ beforeEach(() => {
planIssuesBodies.length = 0;
apiRequests.length = 0;
fixtureOptions.repoDocRefresh = undefined;
fixtureOptions.outcomeCalibrationBands = undefined;
});

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

// REGRESSION (#9641): a zero-sample band's mergeRate is null (not a fabricated 0), and the plain-text
// renderer must show that as "n/a", matching the sampled bands' own "n/a (below sample)" wording.
it("outcome-calibration renders a zero-sample band's null mergeRate as n/a, not 0%", async () => {
fixtureOptions.outcomeCalibrationBands = [
{ band: "clean", sampleSize: 12, merged: 9, closed: 3, mergeRate: 0.75 },
{ band: "high", sampleSize: 0, merged: 0, closed: 0, mergeRate: null },
];
const out = await cli(["maintain", "outcome-calibration", "--repo", "owner/repo"]);
expect(out).toMatch(/high: n\/a \(below sample\) merge rate over 0 PR\(s\)/);
expect(out).not.toMatch(/high: 0% merge rate/);
});

it("onboarding-pack mirrors the session-gated API payload and forwards refresh", async () => {
const json = JSON.parse(
await cli([
Expand Down
12 changes: 12 additions & 0 deletions test/unit/outcome-calibration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,18 @@ describe("buildSlopOutcomeCalibration", () => {
expect(result.totalResolved).toBe(4);
});

// REGRESSION (#9641): a band with zero resolved PRs reports mergeRate null -- never a fabricated 0, which
// for a discrimination table would read as "every PR in this band was closed" -- and computeDiscriminates'
// verdict over the remaining sampled bands is unaffected by the null band.
it("reports a zero-sample band's mergeRate as null, leaving the verdict over the sampled bands unchanged", () => {
const result = buildSlopOutcomeCalibration([...band("clean", 6, 5, 0), ...band("low", 6, 4, 100), ...band("elevated", 6, 1, 200)]);
// "high" has no resolved PRs at all -- one zero-sample band alongside three sampled ones.
const high = result.bands.find((b) => b.band === "high")!;
expect(high.sampleSize).toBe(0);
expect(high.mergeRate).toBeNull();
expect(result.discriminates).toBe(true); // clean 0.833 > low 0.667 > elevated 0.167 -- non-increasing
});

it("excludes open PRs and PRs with no slop assessment", () => {
const open: PullRequestRecord = { repoFullName: "owner/repo", number: 9, title: "open", state: "open", labels: [], linkedIssues: [], slopRisk: 70, slopBand: "high" };
const unassessed: PullRequestRecord = { repoFullName: "owner/repo", number: 10, title: "no slop", state: "closed", mergedAt: "2026-06-01T00:00:00.000Z", labels: [], linkedIssues: [] };
Expand Down
5 changes: 4 additions & 1 deletion test/unit/support/mcp-cli-harness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,9 @@ export async function startFixtureServer(
onApiRequest?: (request: IncomingMessage) => void;
/** #9300: captures DELETE /v1/repos/:owner/:repo/selftune/overrides body ({ confirm }). */
onClearSelftuneOverride?: (body: { confirm?: boolean }) => void;
/** #9641: overrides the outcome-calibration route's `slop` band list -- lets a test drive a zero-sample
* band (mergeRate: null) through the CLI's plain-text renderer without touching the other two bands. */
outcomeCalibrationBands?: unknown[] | undefined;
validateConfigWarnings?: string[];
openPrMonitor?: Record<string, unknown>;
prOutcomes?: Record<string, unknown>;
Expand Down Expand Up @@ -842,7 +845,7 @@ export async function startFixtureServer(
repoFullName: "owner/repo",
generatedAt: "2026-05-30T00:00:00.000Z",
windowDays: windowDays ? Number(windowDays) : null,
slop: [
slop: options.outcomeCalibrationBands ?? [
{ band: "clean", sampleSize: 12, merged: 9, closed: 3, mergeRate: 0.75 },
{ band: "high", sampleSize: 4, merged: 1, closed: 3, mergeRate: 0.25 },
],
Expand Down
Loading