Skip to content

Commit aad3f93

Browse files
authored
test(signals): cover buildReviewRiskExplanation's 5-way branch logic (#9287) (#9338)
Existing coverage only checked REST/MCP parity for the shared review-risk builder, never asserting what each of the five recommendation branches (likely_duplicate, maintainer_lane, needs_author, review, watch) actually resolves to. Add direct unit tests with minimal fixtures for each branch.
1 parent e7dbc23 commit aad3f93

1 file changed

Lines changed: 102 additions & 0 deletions

File tree

test/unit/review-risk.test.ts

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
import { describe, expect, it } from "vitest";
2+
import { buildReviewRiskExplanation } from "../../src/signals/review-risk";
3+
import type { IssueRecord, PullRequestRecord, RegistryRepoConfig, RepositoryRecord } from "../../src/types";
4+
5+
function repo(fullName: string, overrides: Partial<RegistryRepoConfig> = {}): RepositoryRecord {
6+
const [owner, name] = fullName.split("/") as [string, string];
7+
return {
8+
fullName,
9+
owner,
10+
name,
11+
isInstalled: true,
12+
isRegistered: true,
13+
isPrivate: false,
14+
registryConfig: {
15+
repo: fullName,
16+
emissionShare: 0.02,
17+
issueDiscoveryShare: 0,
18+
labelMultipliers: {},
19+
maintainerCut: 0,
20+
raw: {},
21+
...overrides,
22+
},
23+
};
24+
}
25+
26+
function issue(repoFullName: string, number: number, title: string, overrides: Partial<IssueRecord> = {}): IssueRecord {
27+
return { repoFullName, number, title, state: "open", labels: [], linkedPrs: [], ...overrides };
28+
}
29+
30+
function pr(repoFullName: string, number: number, title: string, overrides: Partial<PullRequestRecord> = {}): PullRequestRecord {
31+
return { repoFullName, number, title, state: "open", labels: [], linkedIssues: [], ...overrides };
32+
}
33+
34+
// #9287: buildReviewRiskExplanation previously only had parity coverage (does the REST route match the
35+
// MCP tool?) with no test asserting what each of its five recommendation branches actually resolves to.
36+
describe("buildReviewRiskExplanation", () => {
37+
it("recommends likely_duplicate when a high-risk collision cluster is present", () => {
38+
const activeRepo = repo("acme/widgets");
39+
const dupIssue = issue(activeRepo.fullName, 100, "Duplicate work");
40+
const firstCandidate = pr(activeRepo.fullName, 101, "First candidate fix", { linkedIssues: [100] });
41+
const secondCandidate = pr(activeRepo.fullName, 102, "Second candidate fix", { linkedIssues: [100] });
42+
const result = buildReviewRiskExplanation({
43+
input: { repoFullName: activeRepo.fullName, title: "Third candidate fix", linkedIssues: [100] },
44+
repo: activeRepo,
45+
issues: [dupIssue],
46+
pullRequests: [firstCandidate, secondCandidate],
47+
});
48+
expect(result.recommendation).toBe("likely_duplicate");
49+
expect(result.summary).toBe(`LoopOver review-risk explanation for ${activeRepo.fullName}.`);
50+
});
51+
52+
it("recommends maintainer_lane when the contributor login owns the repo", () => {
53+
const result = buildReviewRiskExplanation({
54+
input: { repoFullName: "acme/widgets", title: "Repo housekeeping", contributorLogin: "acme" },
55+
repo: null,
56+
issues: [],
57+
pullRequests: [],
58+
});
59+
expect(result.recommendation).toBe("maintainer_lane");
60+
expect(result.roleContext?.maintainerLane).toBe(true);
61+
expect(result.summary).toBe("LoopOver review-risk explanation for acme/widgets.");
62+
});
63+
64+
it("recommends needs_author when preflight status is needs_work", () => {
65+
const activeRepo = repo("acme/widgets");
66+
const result = buildReviewRiskExplanation({
67+
input: { repoFullName: activeRepo.fullName, title: "Fix pagination", body: "Just a fix, no context." },
68+
repo: activeRepo,
69+
issues: [],
70+
pullRequests: [],
71+
});
72+
expect(result.preflight.status).toBe("needs_work");
73+
expect(result.recommendation).toBe("needs_author");
74+
expect(result.summary).toBe(`LoopOver review-risk explanation for ${activeRepo.fullName}.`);
75+
});
76+
77+
it("recommends review when preflight status is ready", () => {
78+
const activeRepo = repo("acme/widgets");
79+
const result = buildReviewRiskExplanation({
80+
input: { repoFullName: activeRepo.fullName, title: "Fix pagination", body: "Fixes #1", linkedIssues: [1] },
81+
repo: activeRepo,
82+
issues: [],
83+
pullRequests: [],
84+
});
85+
expect(result.preflight.status).toBe("ready");
86+
expect(result.recommendation).toBe("review");
87+
expect(result.summary).toBe(`LoopOver review-risk explanation for ${activeRepo.fullName}.`);
88+
});
89+
90+
it("recommends watch when the repo lane is unavailable and the author is not a maintainer", () => {
91+
const result = buildReviewRiskExplanation({
92+
input: { repoFullName: "missing/repo", title: "Docs note" },
93+
repo: null,
94+
issues: [],
95+
pullRequests: [],
96+
});
97+
expect(result.preflight.status).toBe("hold");
98+
expect(result.recommendation).toBe("watch");
99+
expect(result.roleContext).toBeNull();
100+
expect(result.summary).toBe("LoopOver review-risk explanation for missing/repo.");
101+
});
102+
});

0 commit comments

Comments
 (0)