diff --git a/.changeset/hungry-bobcats-occur.md b/.changeset/hungry-bobcats-occur.md new file mode 100644 index 000000000..77014ee2a --- /dev/null +++ b/.changeset/hungry-bobcats-occur.md @@ -0,0 +1,5 @@ +--- +"codeowners-review-analysis": minor +--- + +add reviewer recommendations diff --git a/actions/codeowners-review-analysis/dist/index.js b/actions/codeowners-review-analysis/dist/index.js index 9b4b310ac..a6b2d2778 100644 --- a/actions/codeowners-review-analysis/dist/index.js +++ b/actions/codeowners-review-analysis/dist/index.js @@ -25759,7 +25759,7 @@ function getReviewForStatusFor(codeowner, currentReviewStatus) { // actions/codeowners-review-analysis/src/strings.ts var LEGEND = `Legend: ${iconFor(PullRequestReviewStateExt.Approved)} Approved | ${iconFor(PullRequestReviewStateExt.ChangesRequested)} Changes Requested | ${iconFor(PullRequestReviewStateExt.Commented)} Commented | ${iconFor(PullRequestReviewStateExt.Dismissed)} Dismissed | ${iconFor(PullRequestReviewStateExt.Pending)} Pending | ${iconFor("UNKNOWN" /* Unknown */)} Unknown`; -function formatPendingReviewsMarkdown(entryMap, summaryUrl) { +function formatPendingReviewsMarkdown(entryMap, summaryUrl, minimumHittingSets) { const lines = []; lines.push("### Codeowners Review Summary"); lines.push(""); @@ -25782,13 +25782,18 @@ function formatPendingReviewsMarkdown(entryMap, summaryUrl) { `| ${patternCell} | ${overallIcon} | ${processed.files.length} |${owners.join(", ")} |` ); } - if (summaryUrl) { + const recommendations = getReviewRecos(entryMap, minimumHittingSets, 2); + if (recommendations.length > 0) { lines.push(""); - lines.push( - `For more details, see the [full review summary](${summaryUrl}).` - ); + lines.push("### Reviewer Recommendations"); + recommendations.forEach((rec) => { + lines.push(`- ${rec}`); + }); lines.push(""); } + lines.push(""); + lines.push("---"); + lines.push(""); const { runId, repo: { owner, repo } @@ -25798,10 +25803,26 @@ function formatPendingReviewsMarkdown(entryMap, summaryUrl) { `Refresh analysis with: \`gh run rerun ${runId} -R ${owner}/${repo}\`` ); } + if (summaryUrl) { + lines.push(""); + lines.push( + `For more details, see the [full review summary](${summaryUrl}).` + ); + lines.push(""); + } return lines.join("\n"); } -async function formatAllReviewsSummaryByEntry(entryMap) { +async function formatAllReviewsSummaryByEntry(entryMap, minimumHittingSets) { core6.summary.addHeading("Codeowners Review Details", 2).addRaw(LEGEND).addBreak(); + const recommendations = getReviewRecos(entryMap, minimumHittingSets, 10); + if (recommendations.length > 0) { + core6.summary.addHeading( + `Reviewer Recommendations (${recommendations.length} of ${minimumHittingSets.size})`, + 3 + ); + core6.summary.addList(recommendations); + core6.summary.addBreak(); + } const sortedEntries = [...entryMap.entries()].sort(([a, _], [b, __]) => { return a.lineNumber - b.lineNumber; }); @@ -25880,6 +25901,118 @@ async function formatAllReviewsSummaryByEntry(entryMap) { } await core6.summary.addSeparator().write(); } +function getReviewRecos(entryMap, minimumHittingSets, limit = 3) { + if (minimumHittingSets.size === 0) { + return []; + } + const numEntries = entryMap.size; + if (numEntries <= 1) { + return []; + } + const setsArray = Array.from(minimumHittingSets); + const minimumSize = setsArray[0].length; + const numberOfSetsToSuggest = Math.min(minimumSize, limit, setsArray.length); + const trimmedSets = setsArray.slice(0, numberOfSetsToSuggest); + return trimmedSets.map((set2) => `${set2.join(", ")}`); +} + +// actions/codeowners-review-analysis/src/hitting-sets.ts +function calculateAllMinimumHittingSets(reviewSummary) { + const { superset, subsets } = getSupersetAndSubsets(reviewSummary); + if (superset.size === 0 || superset.size > 12 || subsets.length === 0) { + return /* @__PURE__ */ new Set(); + } + for (let k = 1; k <= superset.size; k++) { + const validHittingSets = /* @__PURE__ */ new Set(); + for (const combo of combinations(superset, k)) { + const candidateSet = new Set(combo); + const hitsAll = subsets.every((subset) => { + for (const elem of subset) { + if (candidateSet.has(elem)) { + return true; + } + } + return false; + }); + if (hitsAll) { + validHittingSets.add(combo); + } + } + if (validHittingSets.size > 0) { + return validHittingSets; + } + } + return /* @__PURE__ */ new Set(); +} +function combinations(superset, k) { + const supersetArr = Array.from(superset).sort(); + const results = []; + function backtrack(start, path) { + if (path.length === k) { + results.push([...path]); + return; + } + for (let i = start; i < supersetArr.length; i++) { + path.push(supersetArr[i]); + backtrack(i + 1, path); + path.pop(); + } + } + backtrack(0, []); + return results; +} +function getSupersetAndSubsets(reviewSummary) { + const allPendingOwners = /* @__PURE__ */ new Set(); + const allPendingEntries = []; + const subsetsSeen = /* @__PURE__ */ new Set(); + for (const [entry, processed] of reviewSummary.entries()) { + if (processed.overallStatus !== PullRequestReviewStateExt.Approved) { + entry.owners.forEach((owner) => { + allPendingOwners.add(owner); + }); + if (entry.owners.length > 0) { + addIfUnique(subsetsSeen, allPendingEntries, entry.owners); + } + } + } + const minimizedSubsets = removeSupersets(allPendingEntries); + return { superset: allPendingOwners, subsets: minimizedSubsets }; +} +function addIfUnique(seen, set2, item) { + const normalized = JSON.stringify([...item].sort()); + if (!seen.has(normalized)) { + seen.add(normalized); + set2.push(new Set(item)); + } +} +function removeSupersets(sets) { + const arrs = sets.map((s) => [...s].sort()); + arrs.sort((a, b) => a.length - b.length); + const keep = []; + outer: for (const S of arrs) { + for (const T of keep) { + if (isSubset(T, S)) { + continue outer; + } + } + keep.push(S); + } + return keep.map((a) => new Set(a)); +} +function isSubset(A, B) { + let i = 0, j = 0; + while (i < A.length && j < B.length) { + if (A[i] === B[j]) { + i++; + j++; + } else if (A[i] > B[j]) { + j++; + } else { + return false; + } + } + return i === A.length; +} // actions/codeowners-review-analysis/src/run.ts async function run() { @@ -25932,17 +26065,20 @@ async function run() { ); core7.endGroup(); core7.startGroup("Create CODEOWNERS Summary"); - const codeownersSummary = createReviewSummaryObjectV2( + const codeownersSummary = createReviewSummaryObject( currentPRReviewState, codeOwnersEntryToFiles ); - await formatAllReviewsSummaryByEntry(codeownersSummary); + const minimumHittingSets = calculateAllMinimumHittingSets(codeownersSummary); + await formatAllReviewsSummaryByEntry(codeownersSummary, minimumHittingSets); const summaryUrl = await getSummaryUrl(octokit, owner, repo); const pendingReviewMarkdown = formatPendingReviewsMarkdown( codeownersSummary, - summaryUrl + summaryUrl, + minimumHittingSets ); - console.log(pendingReviewMarkdown); + core7.debug(`Pending review markdown: +${pendingReviewMarkdown}`); if (inputs.postComment) { await upsertPRComment( octokit, @@ -25958,7 +26094,7 @@ async function run() { core7.setFailed(`Action failed: ${error}`); } } -function createReviewSummaryObjectV2(currentReviewStatus, codeOwnersEntryToFiles) { +function createReviewSummaryObject(currentReviewStatus, codeOwnersEntryToFiles) { const reviewSummary = /* @__PURE__ */ new Map(); for (const [entry, files] of codeOwnersEntryToFiles.entries()) { const ownerReviewStatuses = []; diff --git a/actions/codeowners-review-analysis/scripts/payload.json b/actions/codeowners-review-analysis/scripts/payload.json index 085db71b5..e2b2a8e07 100644 --- a/actions/codeowners-review-analysis/scripts/payload.json +++ b/actions/codeowners-review-analysis/scripts/payload.json @@ -2,6 +2,6 @@ "pull_request": { "base": { "sha": "46005c80f947ea200bc4ffcbd850dd9693ba279a" }, "head": { "sha": "4908cc0f05a73a6ff14cb2e34b21e89cf0aa0133" }, - "number": 19213 + "number": 19377 } } diff --git a/actions/codeowners-review-analysis/src/__tests__/hitting-sets.test.ts b/actions/codeowners-review-analysis/src/__tests__/hitting-sets.test.ts new file mode 100644 index 000000000..ef1c0b44a --- /dev/null +++ b/actions/codeowners-review-analysis/src/__tests__/hitting-sets.test.ts @@ -0,0 +1,253 @@ +import { describe, it, expect, vi } from "vitest"; + +import { calculateAllMinimumHittingSets } from "../hitting-sets"; +import { PullRequestReviewStateExt } from "../review-status"; + +import type { CodeownersEntry } from "../codeowners"; +import type { ProcessedCodeOwnersEntry } from "../run"; + +// Helpers +function mkMap( + pairs: Array< + [{ owners: string[] }, { overallStatus: PullRequestReviewStateExt }] + >, +): Map { + return new Map(pairs) as Map; +} +function s(arr: string[]): string { + return arr.join(","); +} +function toSortedStrings(result: Set) { + // serialize arrays for stable comparison (order of set iteration is not guaranteed) + return [...result].map((a) => s(a)).sort(); +} + +describe("calculateAllMinimumHittingSets", () => { + it("returns empty when there are no pending entries (empty map)", () => { + const m = mkMap([]); + const res = calculateAllMinimumHittingSets(m); + expect(res.size).toBe(0); + }); + + it("returns empty when superset is empty (all entries approved)", () => { + const e1 = { owners: ["a", "b"] }; + const e2 = { owners: ["b", "c"] }; + const m = mkMap([ + [e1, { overallStatus: PullRequestReviewStateExt.Approved }], + [e2, { overallStatus: PullRequestReviewStateExt.Approved }], + ]); + const res = calculateAllMinimumHittingSets(m); + expect(res.size).toBe(0); + }); + + it("finds the trivial single-owner solution", () => { + const e1 = { owners: ["alice"] }; + const m = mkMap([ + [e1, { overallStatus: PullRequestReviewStateExt.Pending }], + ]); + const res = calculateAllMinimumHittingSets(m); + expect(toSortedStrings(res)).toEqual(["alice"]); + }); + + it("finds all minimum-cardinality hitting sets (size 2 example)", () => { + // Subsets: {a,b}, {b,c}, {c,d} + const e1 = { owners: ["a", "b"] }; + const e2 = { owners: ["b", "c"] }; + const e3 = { owners: ["c", "d"] }; + const m = mkMap([ + [e1, { overallStatus: PullRequestReviewStateExt.Pending }], + [e2, { overallStatus: PullRequestReviewStateExt.Pending }], + [e3, { overallStatus: PullRequestReviewStateExt.Pending }], + ]); + + const res = calculateAllMinimumHittingSets(m); + // Minimum hitting sets of size 2 here include: [a,c], [b,c], [b,d] + const got = toSortedStrings(res); + expect(got).toEqual(["a,c", "b,c", "b,d"]); + }); + + it("ignores Approved entries when building subsets", () => { + // One pending, one approved + const e1 = { owners: ["a", "b"] }; // pending + const e2 = { owners: ["c", "d"] }; // approved → must not affect result + const m = mkMap([ + [e1, { overallStatus: PullRequestReviewStateExt.Pending }], + [e2, { overallStatus: PullRequestReviewStateExt.Approved }], + ]); + + const res = calculateAllMinimumHittingSets(m); + // For just {a,b}, minimum solutions are ["a"] and ["b"] + const got = toSortedStrings(res); + expect(got).toEqual(["a", "b"]); + }); + + it("drops inclusion-supersets: {a} makes {a,b,c} redundant", () => { + // After removeSupersets, only {a} remains, so min hitting sets are ["a"]. + const e1 = { owners: ["a"] }; + const e2 = { owners: ["a", "b", "c"] }; + const m = mkMap([ + [e1, { overallStatus: PullRequestReviewStateExt.Pending }], + [e2, { overallStatus: PullRequestReviewStateExt.Pending }], + ]); + const res = calculateAllMinimumHittingSets(m); + expect(toSortedStrings(res)).toEqual(["a"]); + }); + + it("dedupes identical subsets via addIfUnique", () => { + const e1 = { owners: ["x", "y"] }; + const e2 = { owners: ["y", "x"] }; // identical, different order + const m = mkMap([ + [e1, { overallStatus: PullRequestReviewStateExt.Pending }], + [e2, { overallStatus: PullRequestReviewStateExt.Pending }], + ]); + + // Only one subset should be considered; min solutions are ["x"] and ["y"]. + const res = calculateAllMinimumHittingSets(m); + const got = toSortedStrings(res); + expect(got).toEqual(["x", "y"]); + }); + + it("returns empty if all pending entries have empty owners (filtered out → no subsets)", () => { + const e1 = { owners: [] }; + const e2 = { owners: [] }; + const m = mkMap([ + [e1, { overallStatus: PullRequestReviewStateExt.Pending }], + [e2, { overallStatus: PullRequestReviewStateExt.Pending }], + ]); + + // getSupersetAndSubsets() will skip empty owner lists; subsets.length === 0 → empty result + const res = calculateAllMinimumHittingSets(m); + expect(res.size).toBe(0); + }); + + it("returns empty when superset size exceeds 12 (guard)", () => { + const manyOwners = Array.from({ length: 13 }, (_, i) => `o${i}`); + const e1 = { owners: manyOwners }; + const m = mkMap([ + [e1, { overallStatus: PullRequestReviewStateExt.Pending }], + ]); + const res = calculateAllMinimumHittingSets(m); + expect(res.size).toBe(0); + }); + + it("works with mixed statuses and multiple minimum solutions", () => { + // Pending: {a,b}, {b,c}, Approved: {c,d} + const e1 = { owners: ["a", "b"] }; + const e2 = { owners: ["b", "c"] }; + const e3 = { owners: ["c", "d"] }; + const m = mkMap([ + [e1, { overallStatus: PullRequestReviewStateExt.Pending }], + [e2, { overallStatus: PullRequestReviewStateExt.Pending }], + [e3, { overallStatus: PullRequestReviewStateExt.Approved }], + ]); + const res = calculateAllMinimumHittingSets(m); + // Only {a,b} and {b,c} matter → min solutions are ["a","c"] and ["b"] + // Wait: check carefully — to hit {a,b} and {b,c}: + // - ["b"] alone hits both → size 1 is minimum. + const got = toSortedStrings(res); + expect(got).toEqual(["b"]); + }); +}); + +describe("calculateAllMinimumHittingSets (complex scenario)", () => { + it("handles supersets, duplicates, approved entries, singletons, and chains", () => { + // ---- Structure ---- + // Block A (needs 3 owners min): + // S1 = {A,B} + // S2 = {B,C,D} + // S3 = {A,E} + // S4 = {C,E} + // S5 = {D,F} + // + // Minimal size-3 hitting set options for Block A: + // {B,D,E}, {B,E,F}, {A,C,D}, {A,C,F}, {A,D,E} + // + // Block B (singleton forces inclusion): + // S6 = {G} + // + // Block C (4-edge chain → unique size-2 cover): + // {H,I}, {I,J}, {J,K}, {K,L} → minimal {I,K} + // + // Extra noise: + // - Superset: {A,B,C,D,E,F,G,H} (should be removed by removeSupersets) + // - Duplicate subset: another {A,B} in different order + // - Approved entry: {C,D} (ignored) + // - Empty-owner entries: [], [] (ignored) + // + // Expected minimum size = 3 (Block A) + 1 (G) + 2 (I,K) = 6 + // Number of minimum solutions = 5 (from Block A’s 5 choices) + + // Block A + const eA1 = { owners: ["A", "B"] }; + const eA2 = { owners: ["B", "C", "D"] }; + const eA3 = { owners: ["A", "E"] }; + const eA4 = { owners: ["C", "E"] }; + const eA5 = { owners: ["D", "F"] }; + + // Duplicate of {A,B} (different order) → should be deduped + const eA1dup = { owners: ["B", "A"] }; + + // Superset that should be dropped by removeSupersets + const eSuperset = { + owners: ["A", "B", "C", "D", "E", "F", "G", "H"], + }; + + // Block B: singleton + const eB = { owners: ["G"] }; + + // Block C: chain + const eC1 = { owners: ["H", "I"] }; + const eC2 = { owners: ["I", "J"] }; + const eC3 = { owners: ["J", "K"] }; + const eC4 = { owners: ["K", "L"] }; + + // Approved (ignored) + const eApproved = { owners: ["C", "D"] }; + + // Empty owners (ignored by getter) + const eEmpty1 = { owners: [] }; + const eEmpty2 = { owners: [] }; + + const m = mkMap([ + [eA1, { overallStatus: PullRequestReviewStateExt.Pending }], + [eA2, { overallStatus: PullRequestReviewStateExt.Pending }], + [eA3, { overallStatus: PullRequestReviewStateExt.Pending }], + [eA4, { overallStatus: PullRequestReviewStateExt.Pending }], + [eA5, { overallStatus: PullRequestReviewStateExt.Pending }], + [eA1dup, { overallStatus: PullRequestReviewStateExt.Pending }], + [eSuperset, { overallStatus: PullRequestReviewStateExt.Pending }], + [eB, { overallStatus: PullRequestReviewStateExt.Pending }], + [eC1, { overallStatus: PullRequestReviewStateExt.Pending }], + [eC2, { overallStatus: PullRequestReviewStateExt.Pending }], + [eC3, { overallStatus: PullRequestReviewStateExt.Pending }], + [eC4, { overallStatus: PullRequestReviewStateExt.Pending }], + [eApproved, { overallStatus: PullRequestReviewStateExt.Approved }], + [eEmpty1, { overallStatus: PullRequestReviewStateExt.Pending }], + [eEmpty2, { overallStatus: PullRequestReviewStateExt.Pending }], + ]); + + const res = calculateAllMinimumHittingSets(m); + const key = (arr: string[]) => arr.join(","); + + // Expect size 6 solutions; enumerate all expected minimal solutions (5 of them): + // Block A options × {G} × {I,K} + const expected = [ + ["A", "C", "D", "G", "I", "K"], + ["A", "C", "F", "G", "I", "K"], + ["A", "D", "E", "G", "I", "K"], + ["B", "D", "E", "G", "I", "K"], + ["B", "E", "F", "G", "I", "K"], + ] + .map((a) => key(a)) + .sort(); + + const got = toSortedStrings(res); + + // Basic shape checks + expect(res.size).toBe(expected.length); + expect(got).toEqual(expected); + + // Ensure every returned set is size 6 (minimum for this construction) + for (const arr of res) expect(arr.length).toBe(6); + }); +}); diff --git a/actions/codeowners-review-analysis/src/hitting-sets.ts b/actions/codeowners-review-analysis/src/hitting-sets.ts new file mode 100644 index 000000000..7a6d66ed0 --- /dev/null +++ b/actions/codeowners-review-analysis/src/hitting-sets.ts @@ -0,0 +1,147 @@ +import type { CodeownersEntry } from "./codeowners"; +import type { ProcessedCodeOwnersEntry } from "./run"; + +import { PullRequestReviewStateExt } from "./review-status"; + +// We calculate the smallest sets of pending reviewers that satisfy all pending codeowners entries. +// This is a brute-force combinatorial approach, which is feasible for small sets. +// We've limited the superset size to 12 owners to avoid combinatorial explosion. +// See: https://en.wikipedia.org/wiki/Set_cover_problem + +/** + * Calculates all minimum hitting sets of owners that can satisfy all pending codeowners review entries. + */ +export function calculateAllMinimumHittingSets( + reviewSummary: Map, +) { + const { superset, subsets } = getSupersetAndSubsets(reviewSummary); + + // Quick sanity check to avoid combinatorial explosion + if (superset.size === 0 || superset.size > 12 || subsets.length === 0) { + return new Set(); + } + + for (let k = 1; k <= superset.size; k++) { + const validHittingSets: Set = new Set(); + + for (const combo of combinations(superset, k)) { + const candidateSet = new Set(combo); + + const hitsAll = subsets.every((subset) => { + for (const elem of subset) { + if (candidateSet.has(elem)) { + return true; + } + } + return false; + }); + + if (hitsAll) { + validHittingSets.add(combo); + } + } + if (validHittingSets.size > 0) { + return validHittingSets; + } + } + + return new Set(); +} + +/** + * Generates all combinations of k elements from the input array. + */ +function combinations(superset: Set, k: number) { + const supersetArr = Array.from(superset).sort(); + const results: string[][] = []; + + function backtrack(start: number, path: string[]) { + if (path.length === k) { + results.push([...path]); + return; + } + + for (let i = start; i < supersetArr.length; i++) { + path.push(supersetArr[i]); + backtrack(i + 1, path); + path.pop(); + } + } + + backtrack(0, []); + return results; +} + +/** + * Gets the superset of all pending owners and the subsets of owners for each pending entry. + */ +function getSupersetAndSubsets( + reviewSummary: Map, +) { + const allPendingOwners = new Set(); + const allPendingEntries: Set[] = []; + + const subsetsSeen = new Set(); + for (const [entry, processed] of reviewSummary.entries()) { + if (processed.overallStatus !== PullRequestReviewStateExt.Approved) { + entry.owners.forEach((owner) => { + allPendingOwners.add(owner); + }); + + if (entry.owners.length > 0) { + addIfUnique(subsetsSeen, allPendingEntries, entry.owners); + } + } + } + + const minimizedSubsets = removeSupersets(allPendingEntries); + return { superset: allPendingOwners, subsets: minimizedSubsets }; +} + +function addIfUnique(seen: Set, set: Set[], item: T[]) { + const normalized = JSON.stringify([...item].sort()); + if (!seen.has(normalized)) { + seen.add(normalized); + set.push(new Set(item)); + } +} + +// Keep only inclusion-minimal subsets: drop any set that is a superset of another. +// For example: +// - {A,B}, {A,B,C}, {B,C} → {A,B}, {B,C} +// - {A},{A,B,C}, {A,B,D} → {A} +function removeSupersets(sets: Set[]): Set[] { + // Normalize to sorted arrays for easy subset checks + const arrs = sets.map((s) => [...s].sort()); + // Sort by length so smaller sets are considered first + arrs.sort((a, b) => a.length - b.length); + + const keep: string[][] = []; + outer: for (const S of arrs) { + for (const T of keep) { + if (isSubset(T, S)) { + // S is a superset of T → redundant, skip S + continue outer; + } + } + keep.push(S); + } + return keep.map((a) => new Set(a)); +} + +function isSubset(A: string[], B: string[]): boolean { + // A, B sorted; check A ⊆ B with two pointers + let i = 0, + j = 0; + while (i < A.length && j < B.length) { + if (A[i] === B[j]) { + i++; + j++; + } else if (A[i] > B[j]) { + j++; + } else { + return false; + } + } + return i === A.length; +} diff --git a/actions/codeowners-review-analysis/src/run.ts b/actions/codeowners-review-analysis/src/run.ts index 51119dbae..699d7f600 100644 --- a/actions/codeowners-review-analysis/src/run.ts +++ b/actions/codeowners-review-analysis/src/run.ts @@ -25,6 +25,7 @@ import { getOverallState, getReviewForStatusFor, } from "./review-status"; +import { calculateAllMinimumHittingSets } from "./hitting-sets"; import type { OwnerReviewStatus } from "./review-status"; import type { CodeownersEntry, CodeOwnersToFilesMap } from "./codeowners"; @@ -92,19 +93,21 @@ export async function run(): Promise { core.endGroup(); core.startGroup("Create CODEOWNERS Summary"); - - const codeownersSummary = createReviewSummaryObjectV2( + const codeownersSummary = createReviewSummaryObject( currentPRReviewState, codeOwnersEntryToFiles, ); - await formatAllReviewsSummaryByEntry(codeownersSummary); + const minimumHittingSets = + calculateAllMinimumHittingSets(codeownersSummary); + await formatAllReviewsSummaryByEntry(codeownersSummary, minimumHittingSets); const summaryUrl = await getSummaryUrl(octokit, owner, repo); const pendingReviewMarkdown = formatPendingReviewsMarkdown( codeownersSummary, summaryUrl, + minimumHittingSets, ); - console.log(pendingReviewMarkdown); + core.debug(`Pending review markdown:\n${pendingReviewMarkdown}`); if (inputs.postComment) { await upsertPRComment( octokit, @@ -121,7 +124,7 @@ export async function run(): Promise { } } -function createReviewSummaryObjectV2( +function createReviewSummaryObject( currentReviewStatus: CurrentReviewStatus, codeOwnersEntryToFiles: CodeOwnersToFilesMap, ): Map { diff --git a/actions/codeowners-review-analysis/src/strings.ts b/actions/codeowners-review-analysis/src/strings.ts index a91075e61..980c2567b 100644 --- a/actions/codeowners-review-analysis/src/strings.ts +++ b/actions/codeowners-review-analysis/src/strings.ts @@ -17,6 +17,7 @@ const LEGEND = export function formatPendingReviewsMarkdown( entryMap: Map, summaryUrl: string, + minimumHittingSets: Set, ): string { const lines: string[] = []; @@ -55,14 +56,20 @@ export function formatPendingReviewsMarkdown( ); } - if (summaryUrl) { + const recommendations = getReviewRecos(entryMap, minimumHittingSets, 2); + if (recommendations.length > 0) { lines.push(""); - lines.push( - `For more details, see the [full review summary](${summaryUrl}).`, - ); + lines.push("### Reviewer Recommendations"); + recommendations.forEach((rec) => { + lines.push(`- ${rec}`); + }); lines.push(""); } + lines.push(""); + lines.push("---"); + lines.push(""); + const { runId, repo: { owner, repo }, @@ -73,6 +80,14 @@ export function formatPendingReviewsMarkdown( ); } + if (summaryUrl) { + lines.push(""); + lines.push( + `For more details, see the [full review summary](${summaryUrl}).`, + ); + lines.push(""); + } + return lines.join("\n"); } @@ -86,6 +101,7 @@ type TRow = TCell[]; export async function formatAllReviewsSummaryByEntry( entryMap: Map, + minimumHittingSets: Set, ): Promise { // Top-level heading & legend once core.summary @@ -93,6 +109,16 @@ export async function formatAllReviewsSummaryByEntry( .addRaw(LEGEND) .addBreak(); + const recommendations = getReviewRecos(entryMap, minimumHittingSets, 10); + if (recommendations.length > 0) { + core.summary.addHeading( + `Reviewer Recommendations (${recommendations.length} of ${minimumHittingSets.size})`, + 3, + ); + core.summary.addList(recommendations); + core.summary.addBreak(); + } + const sortedEntries = [...entryMap.entries()].sort(([a, _], [b, __]) => { return a.lineNumber - b.lineNumber; }); @@ -196,3 +222,28 @@ export async function formatAllReviewsSummaryByEntry( await core.summary.addSeparator().write(); } + +function getReviewRecos( + entryMap: Map, + minimumHittingSets: Set, + limit: number = 3, +): string[] { + if (minimumHittingSets.size === 0) { + // Return early if no hitting sets + return []; + } + + const numEntries = entryMap.size; + if (numEntries <= 1) { + // Return early for trivial cases + return []; + } + + const setsArray = Array.from(minimumHittingSets); + const minimumSize = setsArray[0].length; + + // Suggest up to `limit` sets of the minimum size + const numberOfSetsToSuggest = Math.min(minimumSize, limit, setsArray.length); + const trimmedSets = setsArray.slice(0, numberOfSetsToSuggest); + return trimmedSets.map((set) => `${set.join(", ")}`); +}