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
24 changes: 21 additions & 3 deletions src/signals/engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4226,7 +4226,7 @@ function chatBetaBody(args: PublicSafeCollapsibleArgs): string[] {
"- `@loopover ask <question>` answers contribution-quality Q&A with source citations and freshness.",
"- `@loopover chat <question>` answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).",
...(intentRoutingEnabled
? ["- A plain-language `@loopover` mention with a real question is routed to the closest matching read-only command automatically -- no exact syntax required."]
? ["- A plain-language `@loopover` mention with a real question is routed to the closest matching read-only command automatically no exact syntax required."]
: []),
"",
`Full command reference: ${commandReferenceUrl(args.env)}`,
Expand Down Expand Up @@ -4266,6 +4266,23 @@ 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."];
}

/** #5096: one reusable convention for EXPERIMENTAL ("beta") collapsibles in the public PR comment, so a reader
* can tell experimental features from long-shipped ones at a glance — stronger than a bare "[BETA]" text prefix
* that's easy to miss. Any beta feature routes through this so the next one gets the same treatment for free:
* a consistent 🧪 badge on the title, plus a one-line "may change" disclaimer auto-appended to the body. Static
* text only (no author/finding input), so it's public-safe by construction. Degrades cleanly: an empty body
* yields an empty-body collapsible the renderer skips, so a repo with no beta features shows nothing extra —
* never a bare 🧪 header over nothing. */
const BETA_COLLAPSIBLE_DISCLAIMER = "_🧪 Experimental — new and may change._";

function buildBetaCollapsible(title: string, bodyLines: string[]): UnifiedCollapsible {
const badgedTitle = `🧪 ${title}`;
// Empty body ⇒ empty-body collapsible (the renderer skips it), so nothing beta-marked is shown for a repo with
// no beta features enabled; the badge only ever surfaces alongside real content.
if (bodyLines.length === 0) return { title: badgedTitle, body: "" };
return { title: badgedTitle, body: [...bodyLines, "", BETA_COLLAPSIBLE_DISCLAIMER].join("\n") };
}

/**
* The public-safe collapsibles for the CONVERGED comment, as `UnifiedCollapsible[]`. Built from the SAME
* bodies the legacy panel renders (above) so the two never diverge. Excludes "Maintainer notes" (PRIVATE) and
Expand All @@ -4279,8 +4296,9 @@ export function buildPublicSafeCollapsibles(args: PublicSafeCollapsibleArgs): Un
// #4589: after Signal definitions -- empty (thus invisible, per the caller's empty-body skip) unless
// there's an actual coverage gap AND the generate-tests checkbox is available for this repo.
{ title: "Test coverage", body: testCoverageBody(args).join("\n") },
// #5078: last -- empty (thus invisible) unless chatQa or intentRouting is enabled for this repo.
{ title: "[BETA] Chat with LoopOver", body: chatBetaBody(args).join("\n") },
// #5078/#5096: last -- routes through the shared beta wrapper (🧪 badge + disclaimer). Empty (thus invisible)
// unless chatQa or intentRouting is enabled for this repo.
buildBetaCollapsible("Chat with LoopOver", chatBetaBody(args)),
];
}

Expand Down
34 changes: 25 additions & 9 deletions test/unit/unified-comment-parity.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,18 +133,18 @@ describe("converged comment ↔ legacy panel parity (#unified-comment)", () => {
// #4589: no coverage gap was supplied here, so "Test coverage" stays an empty (thus invisible) collapsible.
expect(body).not.toContain("Test coverage");
// #5078: advisoryAiRouting isn't set in the base `settings` fixture, so the beta chat collapsible stays empty too.
expect(body).not.toContain("[BETA] Chat with LoopOver");
expect(body).not.toContain("🧪 Chat with LoopOver");
});

it("never includes a duplicate AI 'Review details' collapsible", () => {
const { currentPr, detection, collisions, queueHealth, preflight, profile } = buildFixtures();
const collapsibles = buildPublicSafeCollapsibles({ repo, pr: currentPr, profile, detection, settings, collisions, preflight, queueHealth, env: {} });
// #4589/#5078: "Test coverage" and "[BETA] Chat with LoopOver" are always present (title-wise) after
// #4589/#5078: "Test coverage" and "🧪 Chat with LoopOver" are always present (title-wise) after
// Signal definitions, but their bodies are empty (thus invisible when rendered) whenever their respective
// gating inputs aren't supplied, as here.
expect(collapsibles.map((section) => section.title)).toEqual(["Review context", "Contributor next steps", "Signal definitions", "Test coverage", "[BETA] Chat with LoopOver"]);
expect(collapsibles.map((section) => section.title)).toEqual(["Review context", "Contributor next steps", "Signal definitions", "Test coverage", "🧪 Chat with LoopOver"]);
expect(collapsibles.find((section) => section.title === "Test coverage")?.body).toBe("");
expect(collapsibles.find((section) => section.title === "[BETA] Chat with LoopOver")?.body).toBe("");
expect(collapsibles.find((section) => section.title === "🧪 Chat with LoopOver")?.body).toBe("");
expect(collapsibles.map((section) => section.title)).not.toContain("Review details");
// No section may carry the private maintainer-notes content.
expect(collapsibles.map((section) => section.title)).not.toContain("Maintainer notes");
Expand Down Expand Up @@ -213,10 +213,10 @@ describe("converged comment ↔ legacy panel parity (#unified-comment)", () => {
});
});

// #5078: the "[BETA] Chat with LoopOver" collapsible points readers at the ask/chat commands -- empty
// #5078: the "🧪 Chat with LoopOver" collapsible points readers at the ask/chat commands -- empty
// (thus invisible) unless the repo has opted into chatQa or intentRouting, mirroring #4589's own
// "never mention a command that would bounce" principle.
describe("[BETA] Chat with LoopOver collapsible (#5078)", () => {
describe("🧪 Chat with LoopOver collapsible (#5078)", () => {
const advisoryAiRoutingAllOff = {
slop: false, e2eTestGen: false, planner: false, summaries: false,
chatQa: false, chatQaFrontierFallback: false, intentRouting: false,
Expand All @@ -228,7 +228,23 @@ describe("converged comment ↔ legacy panel parity (#unified-comment)", () => {
repo, pr: currentPr, profile, detection, collisions, preflight, queueHealth, env: {},
settings: { ...settings, advisoryAiRouting: advisoryAiRoutingAllOff },
});
expect(collapsibles.find((section) => section.title === "[BETA] Chat with LoopOver")?.body).toBe("");
expect(collapsibles.find((section) => section.title === "🧪 Chat with LoopOver")?.body).toBe("");
});

it("marks the beta collapsible with the shared 🧪 badge + 'may change' disclaimer, and uses em-dashes consistently (#5096)", () => {
const { currentPr, detection, collisions, queueHealth, preflight, profile } = buildFixtures();
const collapsibles = buildPublicSafeCollapsibles({
repo, pr: currentPr, profile, detection, collisions, preflight, queueHealth, env: {},
settings: { ...settings, advisoryAiRouting: { ...advisoryAiRoutingAllOff, chatQa: true, intentRouting: true } },
});
const beta = collapsibles.find((section) => section.title === "🧪 Chat with LoopOver")!;
// The 🧪 badge (stronger than a "[BETA]" text prefix) is on the title, and the disclaimer is auto-appended.
expect(beta.title).toBe("🧪 Chat with LoopOver");
expect(beta.title).not.toContain("[BETA]");
expect(beta.body).toContain("_🧪 Experimental — new and may change._");
// #5096: the body's copy is em-dash-consistent — the old literal double-hyphen is gone.
expect(beta.body).toContain("automatically — no exact syntax required");
expect(beta.body).not.toContain("automatically -- no exact syntax");
});

it("renders ask/chat usage + the docs link when chatQa is enabled", () => {
Expand All @@ -238,7 +254,7 @@ describe("converged comment ↔ legacy panel parity (#unified-comment)", () => {
env: { PUBLIC_SITE_ORIGIN: "https://example-selfhost.test" },
settings: { ...settings, advisoryAiRouting: { ...advisoryAiRoutingAllOff, chatQa: true } },
});
const beta = collapsibles.find((section) => section.title === "[BETA] Chat with LoopOver");
const beta = collapsibles.find((section) => section.title === "🧪 Chat with LoopOver");
expect(beta?.body).toContain("`@loopover ask <question>`");
expect(beta?.body).toContain("`@loopover chat <question>`");
expect(beta?.body).toContain("https://example-selfhost.test/docs/loopover-commands");
Expand All @@ -252,7 +268,7 @@ describe("converged comment ↔ legacy panel parity (#unified-comment)", () => {
repo, pr: currentPr, profile, detection, collisions, preflight, queueHealth, env: {},
settings: { ...settings, advisoryAiRouting: { ...advisoryAiRoutingAllOff, intentRouting: true } },
});
const beta = collapsibles.find((section) => section.title === "[BETA] Chat with LoopOver");
const beta = collapsibles.find((section) => section.title === "🧪 Chat with LoopOver");
expect(beta?.body).toContain("routed to the closest matching read-only command");
expect(beta?.body).toContain(LOOPOVER_SITE_URL);
});
Expand Down