diff --git a/src/signals/engine.ts b/src/signals/engine.ts index 967ff1ef31..e28cc261f8 100644 --- a/src/signals/engine.ts +++ b/src/signals/engine.ts @@ -4261,9 +4261,22 @@ function reviewContextBody(args: PublicSafeCollapsibleArgs): string[] { ]; } -/** "Contributor next steps" body — the deduped actionable steps (or a fallback when none). */ -function contributorNextStepsBody(nextSteps: string[]): string[] { - return nextSteps.length > 0 ? [...new Set(nextSteps)].map((step) => `- ${step}`) : ["- Keep the PR focused and include validation evidence before maintainer review."]; +/** "Contributor next steps" body (#5097). The Signals table's own Action column already lists every one of these + * actions verbatim, so a flat re-listing here added nothing a reader hadn't already seen — which is why it read + * as low-value. Instead this leads with the single highest-priority step as a "Start here" synthesis (the one + * thing the flat table does not say) and points back to the table for the remainder, so the underlying signal + * (`publicSafeNextSteps`: the maintainer-lane note, the readiness actions, the finding actions) is preserved in + * full above while this collapsible finally earns its place. Falls back to the generic line when there are no + * steps at all. */ +export function contributorNextStepsBody(nextSteps: string[]): string[] { + const deduped = [...new Set(nextSteps)]; + if (deduped.length === 0) return ["- Keep the PR focused and include validation evidence before maintainer review."]; + const [first, ...rest] = deduped; + if (rest.length === 0) return [`- **Start here:** ${first}`]; + return [ + `- **Start here:** ${first}`, + `- Then work through the remaining ${rest.length} step${rest.length === 1 ? "" : "s"} in the Signals table above.`, + ]; } /** #5096: one reusable convention for EXPERIMENTAL ("beta") collapsibles in the public PR comment, so a reader @@ -4590,7 +4603,7 @@ export function buildPublicPrIntelligenceComment(args: { "
", "Contributor next steps", "", - ...(nextSteps.length > 0 ? [...new Set(nextSteps)].map((step) => `- ${step}`) : ["- Keep the PR focused and include validation evidence before maintainer review."]), + ...contributorNextStepsBody(nextSteps), "", "
", "", diff --git a/test/unit/unified-comment-parity.test.ts b/test/unit/unified-comment-parity.test.ts index bcc5ed9d42..f04907d0e2 100644 --- a/test/unit/unified-comment-parity.test.ts +++ b/test/unit/unified-comment-parity.test.ts @@ -7,6 +7,7 @@ import { buildPublicPrPanelSignalRows, buildPublicSafeCollapsibles, buildQueueHealth, + contributorNextStepsBody, detectGittensorContributor, } from "../../src/signals/engine"; import { buildUnifiedCommentBody } from "../../src/review/unified-comment-bridge"; @@ -170,6 +171,39 @@ describe("converged comment ↔ legacy panel parity (#unified-comment)", () => { expect(nextSteps.body.length).toBeGreaterThan(0); }); + describe("contributorNextStepsBody redesign (#5097)", () => { + it("leads with a prioritized 'Start here' step and points to the table for the rest", () => { + expect(contributorNextStepsBody(["Add a linked issue.", "Fix the failing check.", "Add tests."])).toEqual([ + "- **Start here:** Add a linked issue.", + "- Then work through the remaining 2 steps in the Signals table above.", + ]); + }); + + it("uses the singular 'step' when exactly one remains", () => { + expect(contributorNextStepsBody(["Add a linked issue.", "Fix the failing check."])).toEqual([ + "- **Start here:** Add a linked issue.", + "- Then work through the remaining 1 step in the Signals table above.", + ]); + }); + + it("shows only the single step when there is exactly one", () => { + expect(contributorNextStepsBody(["Add a linked issue."])).toEqual(["- **Start here:** Add a linked issue."]); + }); + + it("dedupes repeated steps before prioritizing", () => { + expect(contributorNextStepsBody(["Add tests.", "Add tests.", "Fix the check."])).toEqual([ + "- **Start here:** Add tests.", + "- Then work through the remaining 1 step in the Signals table above.", + ]); + }); + + it("falls back to the generic line when there are no steps at all", () => { + expect(contributorNextStepsBody([])).toEqual([ + "- Keep the PR focused and include validation evidence before maintainer review.", + ]); + }); + }); + it("the legacy panel still renders 'Maintainer notes' inline (private section is unchanged, just not shared)", () => { const { currentPr, detection, collisions, queueHealth, preflight, profile } = buildFixtures(); const legacy = buildPublicPrIntelligenceComment({env: {}, repo, pr: currentPr, profile, detection, queueHealth, collisions, preflight, settings });