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
21 changes: 17 additions & 4 deletions src/signals/engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -4590,7 +4603,7 @@ export function buildPublicPrIntelligenceComment(args: {
"<details>",
"<summary>Contributor next steps</summary>",
"",
...(nextSteps.length > 0 ? [...new Set(nextSteps)].map((step) => `- ${step}`) : ["- Keep the PR focused and include validation evidence before maintainer review."]),
...contributorNextStepsBody(nextSteps),
"",
"</details>",
"",
Expand Down
34 changes: 34 additions & 0 deletions test/unit/unified-comment-parity.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
buildPublicPrPanelSignalRows,
buildPublicSafeCollapsibles,
buildQueueHealth,
contributorNextStepsBody,
detectGittensorContributor,
} from "../../src/signals/engine";
import { buildUnifiedCommentBody } from "../../src/review/unified-comment-bridge";
Expand Down Expand Up @@ -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 });
Expand Down