diff --git a/src/signals/engine.ts b/src/signals/engine.ts index 3b45092a1a..c4c2e6d7f0 100644 --- a/src/signals/engine.ts +++ b/src/signals/engine.ts @@ -4226,7 +4226,7 @@ function chatBetaBody(args: PublicSafeCollapsibleArgs): string[] { "- `@loopover ask ` answers contribution-quality Q&A with source citations and freshness.", "- `@loopover chat ` 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)}`, @@ -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 @@ -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)), ]; } diff --git a/test/unit/unified-comment-parity.test.ts b/test/unit/unified-comment-parity.test.ts index f8e0f02681..bcc5ed9d42 100644 --- a/test/unit/unified-comment-parity.test.ts +++ b/test/unit/unified-comment-parity.test.ts @@ -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"); @@ -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, @@ -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", () => { @@ -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 `"); expect(beta?.body).toContain("`@loopover chat `"); expect(beta?.body).toContain("https://example-selfhost.test/docs/loopover-commands"); @@ -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); });