diff --git a/packages/loopover-engine/src/signals/engine.ts b/packages/loopover-engine/src/signals/engine.ts index d90510032a..6cbd08add8 100644 --- a/packages/loopover-engine/src/signals/engine.ts +++ b/packages/loopover-engine/src/signals/engine.ts @@ -4140,15 +4140,7 @@ export const PR_PANEL_RETRIGGER_MARKER = ""; // detect-via-marker / re-authorize-on-toggle mechanism, see maybeProcessPrPanelGenerateTests in processors.ts. export const PR_PANEL_GENERATE_TESTS_MARKER = ""; -/** Earn-CTA target for a public-comment footer. The repo-scoped miner page is only meaningful for - * repos registered on Gittensor (per `gittensorRepoEarnUrl`'s documented contract); for an - * unregistered repo the page has no miner data, so fall back to the general Gittensor home URL - * (the `loopoverFooter` default) instead of implying THIS repo's contributions already earn. */ -function footerEarnUrl(repo: RepositoryRecord | null, repoFullName: string): string | undefined { - return repo?.isRegistered ? gittensorRepoEarnUrl(repoFullName) : undefined; -} - -// ── Public-safe collapsible bodies (ONE source: legacy panel + unified-comment bridge) ────────────── +// ── Public-safe collapsible bodies (single source for the unified-comment bridge) ────────────── // // The public PR comment carries a fixed set of collapsed `
` sections. Their BODIES are built // here as line arrays from the SAME inputs the panel already has, so the legacy `
` markup and @@ -4156,13 +4148,13 @@ function footerEarnUrl(repo: RepositoryRecord | null, repoFullName: string): str // — that section is PRIVATE (advisory findings) and must never appear in the converged public comment; // the legacy builder still renders it inline below, but no shared helper produces it. // -// Byte-identity: `buildPublicPrIntelligenceComment` splices these exact arrays into its existing +// #6103: previously spliced verbatim into the retired legacy renderer's own template; the array shape below // `
` wrappers, so flag-OFF output is unchanged. The unified bridge consumes // `buildPublicSafeCollapsibles` (which joins the same lines) as `extraCollapsibles`. /** Inputs the public-safe collapsible bodies are built from — the subset of the panel's `args` they read. - * `collisions`/`preflight`/`queueHealth` reuse the SAME types `buildPublicPrIntelligenceComment` takes so - * the bodies derive identically to the legacy panel. */ + * `collisions`/`preflight`/`queueHealth` reuse the same types the unified-comment bridge already has on + * hand, so the bodies derive from a single source. */ type PublicSafeCollapsibleArgs = { repo: RepositoryRecord | null; pr: PullRequestRecord; @@ -4182,7 +4174,7 @@ type PublicSafeCollapsibleArgs = { * resolveConvergedFeature("e2eTests") check the checkbox itself is gated on) -- controls whether the * collapsible below points the reader at the checkbox, or just states the gap with no next step. */ e2eTestGenAvailable?: boolean | undefined; - /** #5078: resolved by the caller from `env.PUBLIC_SITE_ORIGIN`, same as `buildPublicPrIntelligenceComment`'s + /** #5078: resolved by the caller from `env.PUBLIC_SITE_ORIGIN`, matching the env param the unified-comment * own `env` param -- lets the "[BETA] Chat with LoopOver" collapsible link to a self-hoster's own * command-reference doc page instead of always the canonical loopover.ai. */ env: LoopOverFooterEnv; @@ -4303,7 +4295,7 @@ function buildBetaCollapsible(title: string, bodyLines: string[]): UnifiedCollap /** * 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 + * bodies rendered above. Excludes "Maintainer notes" (PRIVATE) and * AI review notes, which the unified renderer owns as the prominent Review summary + Nits section. */ export function buildPublicSafeCollapsibles(args: PublicSafeCollapsibleArgs): UnifiedCollapsible[] { @@ -4320,8 +4312,7 @@ export function buildPublicSafeCollapsibles(args: PublicSafeCollapsibleArgs): Un ]; } -/** The deduped, public-safe "next steps" list — extracted so both the legacy panel and the converged - * comment compute it identically (maintainer-lane note, readiness actions, public-finding actions). */ +/** The deduped, public-safe "next steps" list — a single source so it's computed identically (maintainer-lane note, readiness actions, public-finding actions). */ function publicSafeNextSteps(args: PublicSafeCollapsibleArgs): string[] { const roleContext = buildRoleContext({ login: args.pr.authorLogin ?? args.profile.login, @@ -4353,7 +4344,7 @@ function publicSafeNextSteps(args: PublicSafeCollapsibleArgs): string[] { ].filter((step) => !containsPrivatePublicTerm(step)); } -/** The public-safe subset of preflight findings — extracted so the legacy panel and the converged comment +/** The public-safe subset of preflight findings — a single source so every consumer * filter identically (single source). Drops: critical-severity findings; the linked-issue finding when the * linked-issue gate is fully off; private bounty-lifecycle findings; and any finding whose text trips the * private-term backstop. Then slices to the configured public signal level (2 minimal / 5 otherwise). The @@ -4367,277 +4358,6 @@ function publicSafePreflightFindings(preflight: PreflightResult, settings: Repos .slice(0, settings.publicSignalLevel === "minimal" ? 2 : 5); } -export function buildPublicPrIntelligenceComment(args: { - repo: RepositoryRecord | null; - pr: PullRequestRecord; - profile: ContributorProfile; - detection: ContributorDetection; - queueHealth: QueueHealth; - collisions: CollisionReport; - preflight: PreflightResult; - settings: RepositorySettings; - gate?: PublicPrPanelGateEvaluation | undefined; - review?: FocusManifestReviewConfig | undefined; - /** Optional AI maintainer-review notes (already public-safe). Rendered as an advisory section. `valueAssessment` - * (#4743/#4744) is the same tier's composed improvement/value judgment, already run through - * `composeImprovementSignal`'s own `toPublicSafe` pass upstream (services/ai-review.ts) -- re-checked against - * `containsPrivatePublicTerm` again here (defense in depth) before it can reach the improvement row below. */ - aiReview?: { notes: string; valueAssessment?: { magnitude: ImprovementMagnitude; rationale: string } | undefined } | undefined; - /** Duplicate-winner adjudication (#dup-winner). When true AND this PR is the earliest observed linked-issue - * claimant among `linkedDuplicatePrs`, the hard-duplicate panel block is suppressed so the winner's panel - * does not show a blocking duplicate. Default/false ⇒ byte-identical to today. */ - duplicateWinnerEnabled?: boolean | undefined; - /** Deterministic structural-improvement tier (#4742/#4744), pre-computed by the caller via - * `buildStructuralImprovementAssessment` and passed through exactly like `gate`/`aiReview` above are - * pre-computed results, not raw inputs. Absent ⇒ the improvement row renders nothing, matching - * `resolveConvergedFeature(env, manifest, "improvementSignal", repoFullName)` resolving false for the repo, - * or a caller that hasn't wired this yet. */ - improvementSignal?: StructuralImprovementAssessment | undefined; - /** The existing deterministic slop-risk band (#4745, sub-issue H of epic #4737), pre-computed by the - * caller via `buildSlopAssessment` and passed through exactly like `improvementSignal` above is a - * pre-computed result, not a raw input. Threaded into the Improvement row (when present) as the risk - * half of the risk × value quadrant label -- see `formatRiskValueQuadrant`. Absent (every existing - * caller today, and any repo where `shouldCollectSlopEvidence` resolves false this pass) ⇒ no quadrant - * text is added, matching this epic's "degrade cleanly, never fabricate a reading" convention. */ - slopBand?: SlopBand | undefined; - /** Resolved by the caller from `env.PUBLIC_SITE_ORIGIN` so a self-hoster's own domain reaches the - * always-on footer's attribution link instead of `LOOPOVER_SITE_URL` (#4613). */ - env: LoopOverFooterEnv; -}): string { - const publicFindings = publicSafePreflightFindings(args.preflight, args.settings); - const relatedWork = buildDuplicateWinnerRelatedWorkView({ - pr: args.pr, - collisions: args.collisions, - preflightCollisions: args.preflight.collisions, - duplicateWinnerEnabled: args.duplicateWinnerEnabled, - }); - const linkedDuplicatePrs = relatedWork.linkedDuplicatePrItems.map((item) => item.number); - const visibleLinkedDuplicatePrs = relatedWork.visibleLinkedDuplicatePrs; - const scopedOverlapClusters = relatedWork.scopedOverlapClusters; - const scopedOverlapCount = scopedOverlapClusters.length; - const hasRelatedWork = visibleLinkedDuplicatePrs.length > 0 || scopedOverlapCount > 0; - const readiness = buildPublicReadinessScore({ pr: args.pr, preflight: args.preflight, queueHealth: args.queueHealth, linkedDuplicatePrs: visibleLinkedDuplicatePrs, scopedOverlapCount }); - const linkedIssueResult = linkedIssuePanelResult(args.pr); - const relatedWorkResult = relatedWorkPanelResult(visibleLinkedDuplicatePrs, scopedOverlapCount); - const roleContext = buildRoleContext({ - login: args.pr.authorLogin ?? args.profile.login, - repo: args.repo, - repoFullName: args.pr.repoFullName, - pullRequests: [args.pr], - issues: [], - profile: args.profile, - }); - const nextSteps = [ - ...(roleContext.maintainerLane ? ["Treat this as maintainer-lane context rather than normal contributor-lane activity."] : []), - ...readiness.components.map((component) => component.action).filter((action) => action !== "No action."), - /* v8 ignore next -- Public findings may omit actions; public comment tests cover sanitized action inclusion. */ - ...(publicFindings.length > 0 ? publicFindings.flatMap((finding) => (finding.action ? [finding.action] : [])) : []), - ].filter((step) => !containsPrivatePublicTerm(step)); - // #2852: gate presentation follows the SAME evaluation/policy signal as shouldEvaluateGate in - // processors.ts (published check-run OR autonomy needs a verdict) -- not the check-run publish flag - // alone, so a `reviewCheckMode: disabled` repo with autonomy configured still shows its real gate - // result in the public comment instead of silently downgrading to "no gate at all". - const gateEnabled = shouldPublishReviewCheck(args.settings.reviewCheckMode) || isAgentConfigured(args.settings.autonomy); - const hardLinkedIssueBlock = - args.settings.linkedIssueGateMode === "block" && args.pr.linkedIssues.length === 0 && !hasClearNoIssueRationale(args.pr); - // Duplicate-winner adjudication (#dup-winner): when the flag is ON and this PR is the earliest observed - // linked-issue claimant, do NOT hard-block it as a duplicate — only the losers block. Sparse legacy rows fail - // closed so unknown ordering cannot suppress duplicate evidence. - const hardDuplicateBlock = - args.settings.duplicatePrGateMode === "block" && - linkedDuplicatePrs.length > 0 && - visibleLinkedDuplicatePrs.length > 0; - const fallbackGateConclusion = !gateEnabled - ? "success" - : !args.repo - ? "neutral" - : hardLinkedIssueBlock || hardDuplicateBlock - ? "failure" - : "success"; - const gateConclusion = args.gate?.conclusion ?? fallbackGateConclusion; - const gateBlocking = gateEnabled && (gateConclusion === "failure" || gateConclusion === "action_required"); - const gateHeld = gateEnabled && (gateConclusion === "neutral" || gateConclusion === "action_required"); - const missingLinkedIssue = args.pr.linkedIssues.length === 0 && !hasClearNoIssueRationale(args.pr); - const confirmedMiner = isOfficialContributorDetection(args.detection); - // Author with no Gittensor footprint at all (not detected via official API or cache): loopover's - // contribution analysis is for Gittensor contributors, so fire MINIMALLY — a brief welcome + the - // earn invite — instead of the full readiness panel. A KNOWN contributor (official or cached) still - // gets the full review. The always-on footer CTA appears either way, so every PR keeps marketing. - if (!args.detection.detected) return buildMinimalInviteComment(args); - const genericOssMode = args.settings.publicAudienceMode === "oss_maintainer"; - const hasPublicWarnings = publicFindings.some((finding) => finding.severity === "warning"); - const aiReview = args.aiReview ? splitAiReviewNits(args.aiReview.notes) : null; - const aiReviewHasBlockers = Boolean(aiReview?.main) && aiReviewMainHasBlockers(aiReview?.main ?? ""); - const alert = aiReviewHasBlockers - ? "CAUTION" - : gateBlocking - ? gateConclusion === "action_required" - ? "WARNING" - : missingLinkedIssue && args.settings.linkedIssueGateMode === "block" - ? "WARNING" - : "CAUTION" - : gateHeld - ? "WARNING" - : hasPublicWarnings || hasRelatedWork - ? "WARNING" - : "TIP"; - const panelTitle = aiReviewHasBlockers - ? "LoopOver review found blockers" - : args.aiReview && !gateBlocking && !gateHeld - ? "LoopOver review approved this PR" - : gateHeld - ? "LoopOver review needs maintainer review" - : gateBlocking - ? `${LOOPOVER_GATE_CHECK_NAME} is blocking merge` - : hasPublicWarnings || hasRelatedWork - ? "LoopOver found maintainer review notes" - : "LoopOver PR readiness looks good"; - const panelSummary = gateBlocking - ? args.gate?.summary ?? (gateConclusion === "action_required" ? "LoopOver cannot evaluate the repo state closely enough for the enabled gate." : "A repo-configured hard blocker was found.") - : gateHeld - ? args.gate?.summary ?? "LoopOver is holding this PR for maintainer review." - : visibleLinkedDuplicatePrs.length > 0 - ? `Same-issue duplicate risk found against ${formatPrRefs(visibleLinkedDuplicatePrs)}. Maintainers should resolve the overlap before review continues.` - : hasRelatedWork - ? "Scoped related-work signals were found for this PR. They are advisory unless the gate reports a blocker." - : genericOssMode - ? "Public GitHub metadata was checked for review readiness. Gittensor-specific context appears only when confirmed." - : "Confirmed Gittensor contributor context was checked from public metadata and LoopOver cache."; - const readinessByKey = new Map(readiness.components.map((component) => [component.key, component])); - const validationComponent = readinessByKey.get("validation")!; - const changeScopeComponent = readinessByKey.get("change_scope")!; - const contributorWorkload = contributorWorkloadPanelResult(args.profile); - const contributorContext = contributorContextPanelResult(args.pr, args.profile, args.detection, confirmedMiner); - // Each row carries a stable key so a maintainer can show/hide it from `.loopover.yml review.fields` - // (default: shown). Hiding a row is cosmetic — the underlying signal/gate still functions. - const allRows: Array<{ key: ReviewFieldKey; cells: [string, string, string, string] }> = [ - { key: "linkedIssue", cells: ["Linked issue", linkedIssueResult.result, linkedIssueResult.evidence, linkedIssueResult.action] }, - { key: "relatedWork", cells: ["Related work", relatedWorkResult.result, relatedWorkResult.evidence, relatedWorkResult.action] }, - /* v8 ignore start -- Readiness components are built as a fixed key set; fallbacks guard future partial score shapes. */ - { key: "reviewLoad", cells: ["Change scope", scoreResultIcon(changeScopeComponent), changeScopeComponent.evidence, changeScopeComponent.action] }, - { key: "validationEvidence", cells: ["Validation posture", scoreResultIcon(validationComponent), validationComponent.evidence, validationComponent.action] }, - { key: "openPrQueue", cells: ["Contributor workload", contributorWorkload.result, contributorWorkload.evidence, contributorWorkload.action] }, - /* v8 ignore stop */ - { key: "contributorContext", cells: ["Contributor context", contributorContext.result, contributorContext.evidence, contributorContext.action] }, - { key: "gateResult", cells: ["Gate result", gateStatus(gateEnabled, gateConclusion), gateEnabled ? gateAction(gateConclusion) : "Advisory only.", gateEnabled ? gateNextAction(gateConclusion) : "No action."] }, - ]; - // Improvement row (#4744): combines the deterministic tier (#4742) + LLM tier (#4743). `improvementRow` is - // null (row omitted entirely) when the caller passes no `improvementSignal` -- see buildImprovementSignalRow's - // own doc comment for why that's what keeps this byte-identical to today for every existing caller. - const improvementRow = buildImprovementSignalRow(args.improvementSignal, args.aiReview?.valueAssessment, args.slopBand); - if (improvementRow) allRows.push(improvementRow); - const reviewFields = args.review?.fields; - const rows: Array<[string, string, string, string]> = allRows.filter((row) => reviewFields?.[row.key] !== false).map((row) => row.cells); - const overlapDetails = relatedWorkDetails(args.pr, scopedOverlapClusters); - const maintainerNotes = - publicFindings.length > 0 - ? publicFindings.map((finding) => `- ${sanitizePanelText(finding.title)}: ${sanitizePanelText(finding.publicText ?? finding.detail)}`) - : ["- No public-safe advisory findings were generated from cached metadata."]; - // Always-on earn CTA — a permanent, free marketing surface on every reviewed PR. For a registered - // repo the CTA points at this repo's public Gittensor miner page (social proof for THIS repo + a - // path to register); for an unregistered repo it falls back to the general Gittensor home URL. - // The earn CTA stays a permanent marketing surface; `.loopover.yml review.footer.text` can replace - // the lead copy (already public-safe-validated) but the Gittensor register link + attribution remain. - const footer = loopoverFooter(args.env, { earnUrl: footerEarnUrl(args.repo, args.pr.repoFullName), customText: args.review?.footerText ?? undefined }); - return [ - "", - "", - ...formatAlertBlock([ - `[!${alert}]`, - `## ${panelTitle}`, - ...(aiReview?.main - ? [ - "**Review summary**", - escapeAiReviewMarkdown(aiReview.main), - ...(aiReview.nits.length > 0 - ? [ - "", - "
", - `Nits (${aiReview.nits.length})`, - "", - ...aiReview.nits.map((nit) => `- [ ] ${escapeAiReviewMarkdown(nit)}`), - "", - "
", - ] - : []), - "", - panelSummary, - ] - : [panelSummary]), - // Optional maintainer intro note (public-safe-validated at parse time; re-sanitized here). - ...(args.review?.note ? ["", sanitizePanelText(args.review.note)] : []), - "", - `**Readiness score: ${readiness.total}/100**`, - "", - "| Signal | Result | Evidence | Action |", - "| --- | --- | --- | --- |", - ...rows.map(([signal, result, evidence, action]) => `| ${escapeTableCell(signal)} | ${escapeTableCell(result)} | ${escapeTableCell(evidence)} | ${escapeTableCell(action)} |`), - ]), - "", - "
", - "Signal definitions", - "", - "- Related work = same linked issue, overlapping active PRs, or title/path similarity.", - "- Change scope = cached public metadata such as size labels, draft state, and review-burden hints.", - "- Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.", - "- Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.", - "- Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.", - "", - "
", - "", - "
", - "Review context", - "", - `- Author: \`${sanitizePanelText(args.pr.authorLogin ?? "unknown")}\``, - `- Role context: ${sanitizePanelText(roleContext.role)}${roleContext.maintainerLane ? " (maintainer lane)" : ""}`, - `- Public audience mode: ${args.settings.publicAudienceMode.replace(/_/g, " ")}`, - `- Lane context: ${sanitizePanelText(buildLaneAdvice(args.repo, args.pr.repoFullName).summary)}`, - `- Public profile languages: ${args.profile.github.topLanguages.length > 0 ? sanitizePanelText(args.profile.github.topLanguages.join(", ")) : "not available"}`, - ...(confirmedMiner ? [`- Official Gittensor activity: ${args.detection.priorPullRequests} PR(s), ${args.detection.priorIssues} issue(s).`] : ["- Contributor context: Public profile only; not a blocker."]), - ...overlapDetails, - "", - "
", - "", - "
", - "Maintainer notes", - "", - ...maintainerNotes, - "", - "
", - "", - "
", - "Contributor next steps", - "", - ...contributorNextStepsBody(nextSteps), - "", - "
", - "", - `- [ ] ${PR_PANEL_RETRIGGER_MARKER} Re-run LoopOver review`, - "", - "---", - footer, - ].join("\n"); -} - -/** Minimal public comment for a non-registered contributor. loopover's readiness/contribution - * analysis is for registered Gittensor contributors, so we skip the panel and post a brief welcome - * + earn invite; the always-on footer CTA does the conversion. Carries the same panel marker so it - * updates in place if the author later registers (the full panel then replaces it). */ -function buildMinimalInviteComment(args: { repo: RepositoryRecord | null; pr: PullRequestRecord; review?: FocusManifestReviewConfig | undefined; env: LoopOverFooterEnv }): string { - return [ - "", - "", - ...formatAlertBlock([ - "[!NOTE]", - "## 👋 Thanks for the contribution", - "The maintainer will review your PR. Open-source work like this can earn on Gittensor — register your GitHub account and contributions like this become eligible to earn.", - ]), - "", - "---", - loopoverFooter(args.env, { earnUrl: footerEarnUrl(args.repo, args.pr.repoFullName), customText: args.review?.footerText ?? undefined }), - ].join("\n"); -} - type PublicPrPanelGateEvaluation = { conclusion: "success" | "failure" | "action_required" | "neutral" | "skipped"; summary: string; @@ -4649,11 +4369,9 @@ type PublicPrPanelGateEvaluation = { export type PublicPrPanelSignalRow = { key: ReviewFieldKey; cells: [string, string, string, string] }; /** - * Build the public PR panel's readiness signal rows (the `allRows` table) as a PURE function, from the - * SAME inputs `buildPublicPrIntelligenceComment` uses. It calls the same private panel helpers, so the rows - * are byte-identical to the legacy panel's. Exposed for the unified-comment bridge (convergence) so the - * converged comment surfaces loopover's exact signals; the legacy path is unchanged. The `key` lets the - * caller honor `.loopover.yml review.fields` visibility the same way the legacy renderer does. + * Build the public PR panel's readiness signal rows (the `allRows` table) as a PURE function. Exposed for + * the unified-comment bridge so the converged comment surfaces loopover's exact signals. The `key` lets the + * caller honor `.loopover.yml review.fields` visibility. */ export function buildPublicPrPanelSignalRows(args: { repo: RepositoryRecord | null; @@ -4667,7 +4385,7 @@ export function buildPublicPrPanelSignalRows(args: { gate?: PublicPrPanelGateEvaluation | undefined; /** Duplicate-winner adjudication (#dup-winner). When true AND this PR is the earliest observed linked-issue * claimant among `linkedDuplicatePrs`, the hard-duplicate block is suppressed. Default/false ⇒ byte-identical - * to today. Matches `buildPublicPrIntelligenceComment` so both panels agree. */ + * to today. */ duplicateWinnerEnabled?: boolean | undefined; /** Deterministic structural-improvement tier (#4742/#4744), pre-computed by the caller via * `buildStructuralImprovementAssessment` and passed through exactly like `gate` above is a pre-computed @@ -4682,7 +4400,7 @@ export function buildPublicPrPanelSignalRows(args: { * shows the deterministic tier only. */ valueAssessment?: { magnitude: ImprovementMagnitude; rationale: string } | undefined; /** The existing deterministic slop-risk band (#4745, sub-issue H of epic #4737) -- see the matching doc - * comment on `buildPublicPrIntelligenceComment`'s own `slopBand` field, which this mirrors. */ + * comment on this same function's own `slopBand` field. */ slopBand?: SlopBand | undefined; }): { rows: PublicPrPanelSignalRow[]; readinessTotal: number } { const relatedWork = buildDuplicateWinnerRelatedWorkView({ @@ -4698,7 +4416,7 @@ export function buildPublicPrPanelSignalRows(args: { const readiness = buildPublicReadinessScore({ pr: args.pr, preflight: args.preflight, queueHealth: args.queueHealth, linkedDuplicatePrs: visibleLinkedDuplicatePrs, scopedOverlapCount }); const linkedIssueResult = linkedIssuePanelResult(args.pr); const relatedWorkResult = relatedWorkPanelResult(visibleLinkedDuplicatePrs, scopedOverlapCount); - // #2852: see the matching comment in buildPublicPrIntelligenceComment -- gate presentation must + // #2852: gate presentation must // track whether a gate is actually evaluated (check-run published OR autonomy configured), not // merely whether the check-run itself is published. const gateEnabled = shouldPublishReviewCheck(args.settings.reviewCheckMode) || isAgentConfigured(args.settings.autonomy); @@ -4754,7 +4472,7 @@ export function buildPublicPrPanelSignalRows(args: { /** Static template labels (#4744), one per {@link ImprovementBand} -- never runtime-interpolated free text, so * this bypasses the public-comment sanitizer safely, mirroring how `"**Readiness score: ${total}/100**"` - * (buildPublicPrIntelligenceComment) is a hardcoded template rather than sanitizer-filtered AI prose. Advisory + * is a hardcoded template rather than sanitizer-filtered AI prose. Advisory * icons only (✅/ℹ️, never ⚠️/❌): every band here is informational, never a reason to flag the PR. */ const IMPROVEMENT_BAND_LABELS: Record = { "insufficient-signal": "ℹ️ Insufficient signal", @@ -5345,27 +5063,6 @@ function formatCollisionItemRef(item: CollisionItem): string { return item.htmlUrl ? `[${text}](${item.htmlUrl})` : text; } -function formatAlertBlock(lines: string[]): string[] { - return lines.map((line) => (line.length > 0 ? `> ${line}` : ">")); -} - -function aiReviewMainHasBlockers(main: string): boolean { - const marker = main.search(/\*\*Blockers\*\*/i); - if (marker === -1) return false; - const after = main.slice(marker).split(/\n(?=\*\*[^*]+\*\*)/)[0]!; - return after - .split("\n") - .slice(1) - .map((line) => line.replace(/^\s*[-*]\s*/, "").trim()) - .some((line) => line.length > 0 && !/^none\.?$/i.test(line)); -} - -function escapeAiReviewMarkdown(value: string): string { - return value - .replace(/[<>]/g, (char) => (char === "<" ? "<" : ">")) - .slice(0, 4000); -} - function isPrivateBountyLifecycleFinding(code: string): boolean { return code === "linked_issue_bounty_historical" || code === "linked_issue_bounty_unverified"; } @@ -5381,10 +5078,6 @@ function sanitizePanelText(value: string): string { return value.replace(/\s+/g, " ").trim(); } -function escapeTableCell(value: string): string { - return sanitizePanelText(value).replace(/\|/g, "\\|"); -} - /** * Builds the compact, source-free signal bundle that the optional AI rewrite layer (issue #151) * may turn into clearer public prose. It carries only deterministic, public-safe structured diff --git a/src/env.d.ts b/src/env.d.ts index 0a0b7bbd11..5a3ceb9d72 100644 --- a/src/env.d.ts +++ b/src/env.d.ts @@ -302,9 +302,10 @@ declare global { * secret, never a public var. When absent, BYOK is unavailable and review uses the configured instance * reviewer when available. */ TOKEN_ENCRYPTION_SECRET?: string; - /** Convergence (Stage D): when truthy, the public PR comment is rendered by the unified-comment bridge - * (ONE in-place comment in the converged shape) instead of the legacy `buildPublicPrIntelligenceComment` - * panel. Default OFF — unset/false keeps the legacy panel byte-identical. */ + /** #6103: retired -- the unified-comment bridge is now the ONLY PR-comment renderer unconditionally (the + * legacy `buildPublicPrIntelligenceComment` panel this flag used to switch away from was deleted, having + * no remaining production caller). No longer read; kept only so an operator's existing deployment config + * setting it doesn't error. Safe to remove from any env once noticed. */ LOOPOVER_REVIEW_UNIFIED_COMMENT?: string; /** Inline comments (#inline-comments): when truthy (AND the repo is in LOOPOVER_REVIEW_REPOS AND the repo's * `.loopover.yml` sets `review.inline_comments: true`), the AI reviewer ALSO leaves quiet, NON-BLOCKING diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 36a26582ee..a95e4341ca 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -301,7 +301,6 @@ import { buildContributorStrategy, buildDuplicateWinnerRelatedWorkView, buildPreflightResult, - buildPublicPrIntelligenceComment, buildPublicPrPanelSignalRows, buildPublicReadinessScore, buildPublicSafeCollapsibles, @@ -7700,22 +7699,13 @@ async function maybePublishPrPublicSurface( // a dry-run / pause / global-freeze publishes NOTHING (check-run, comment, label) — the gate verdict is still // computed + returned for the disposition logic, the writes are just suppressed + audited. (#dry-run-chokepoint) const mode = await resolveRepoActionMode(env, settings); - // Per-repo feature override (phase 2): the unified converged comment renders for THIS repo when the global - // LOOPOVER_REVIEW_UNIFIED_COMMENT kill-switch is ON and the repo's container-private `.loopover.yml` - // `features.unifiedComment` opts in — falling back to the LOOPOVER_REVIEW_REPOS allowlist when the manifest - // says nothing (byte-identical default). Computed once and used by both unified-comment sites below. - const unifiedCommentAllowed = await convergedFeatureActive( - env, - repoFullName, - "unifiedComment", - ); // improvementSignal (#4744): the first real caller of #4738's activation wiring (epic #4737's config-as-code // foundation) -- nothing resolved this feature before this PR (see signals/improvement.ts's own header - // comment). Resolved once, independent of unifiedCommentAllowed above: it gates BOTH the deterministic - // tier's own computation further below (which has no AI dependency at all -- a paused repo, a non-reviewable - // author, or aiReviewMode: "off" still gets it) and, threaded into runAiReviewForAdvisory, the LLM tier's - // prompt addition (#4743). loadRepoFocusManifest is cached, so this second manifest resolution costs no - // extra fetch in the common case where something else already resolved it this pass. + // comment). Gates BOTH the deterministic tier's own computation further below (which has no AI dependency + // at all -- a paused repo, a non-reviewable author, or aiReviewMode: "off" still gets it) and, threaded + // into runAiReviewForAdvisory, the LLM tier's prompt addition (#4743). loadRepoFocusManifest is cached, so + // this second manifest resolution costs no extra fetch in the common case where something else already + // resolved it this pass. const improvementSignalAllowed = await convergedFeatureActive( env, repoFullName, @@ -9242,9 +9232,8 @@ async function maybePublishPrPublicSurface( reviewSelfHostAiModel, reviewImpactMap, reviewCultureProfile, - // improvementSignal (#4744): resolved once above (independent of unifiedCommentAllowed), reused - // here so the LLM tier's value-assessment prompt addition (#4743) only fires when this repo has - // actually opted in. + // improvementSignal (#4744): resolved once above, reused here so the LLM tier's value-assessment + // prompt addition (#4743) only fires when this repo has actually opted in. improvementSignal: improvementSignalAllowed, // #regate-dup-prep: this call's own advisory lock is already claimed (by aiReviewCacheReadDecideAndRun's // caller, above) — pass it through so runAiReviewForAdvisory trusts it instead of re-claiming (and @@ -9941,30 +9930,12 @@ async function maybePublishPrPublicSurface( })), }) : undefined; - const commentArgs = { - repo, - pr, - profile, - detection, - queueHealth, - collisions, - preflight, - settings, - gate: gateEvaluation, - review: reviewConfig, - aiReview, - improvementSignal: structuralImprovementAssessment, - // #4745: the risk × value quadrant's risk half -- reuses the slop band already computed above (if any); - // never a second buildSlopAssessment call. - slopBand: slopBand ?? undefined, - duplicateWinnerEnabled, - env, - }; let deterministicBody: string; - // Convergence (Stage D): when the unified-review-comment flag is ON, render the single converged comment - // (loopover shape + reviewbot's review folded in). The gate stays authoritative (passed as `decision`), - // and the body carries the SAME panel marker so the upsert updates in place. Flag-OFF (default) keeps the - // legacy panel byte-identical. Only the comment lane is affected; the gate check-run/labels/audit are not. + // Convergence (Stage D, #6103): the converged comment (loopover shape + reviewbot's review folded in) is + // the only comment path -- the legacy buildPublicPrIntelligenceComment panel was retired once it had no + // remaining production caller (settings-preview.ts's sample preview was migrated to this same renderer). + // The gate stays authoritative (passed as `decision`), and the body carries the SAME panel marker so the + // upsert updates in place. // // RECONCILIATION INVARIANT (#1016 — two-gate → one authoritative path; pinned by // test/unit/unified-comment-bridge.test.ts "reconciliation invariant"): @@ -9979,7 +9950,21 @@ async function maybePublishPrPublicSurface( // contradict the review-agent check-run conclusion. // 3. The `ai_consensus_defect` surfaces exactly ONCE — as the Code-review blocker — never also in the // gate signal row (which renders only the conclusion-derived status text, not the defect string). - if (unifiedCommentAllowed && gateEvaluation) { + { + // #6103: the converged renderer is the only comment path (the legacy buildPublicPrIntelligenceComment + // panel was retired). When the gate was never evaluated for this repo (reviewCheckMode disabled AND no + // autonomy configured -- see shouldEvaluateGate above), synthesize a "skipped" gate for rendering + // purposes only, mirroring buildClosedUnifiedCommentBody's own pattern for the identical situation. + // This never touches the check-run/label/audit/disposition lanes (those still read the real, + // possibly-undefined `gateEvaluation`) -- only what this comment's verdict/"Gate result" row says. + const commentGateEvaluation: NonNullable = gateEvaluation ?? { + enabled: false, + conclusion: "skipped", + title: `${LOOPOVER_GATE_CHECK_NAME} skipped`, + summary: "Gate evaluation is not configured for this repository.", + blockers: [], + warnings: [], + }; // FIX B: the unified comment's file count + visual-capture path filter need the real diff — reuse the // shared resolver (one resolve per review; inline-fetches when stored is still empty pre-detail-sync). const unifiedFiles = await getReviewFiles(); @@ -10025,7 +10010,7 @@ async function maybePublishPrPublicSurface( repoFullName, }); // The public comment must match the authoritative Gate check-run conclusion. - const commentGate = gateEvaluation; + const commentGate = commentGateEvaluation; // Observability (#reviews-dashboard): record the would-be gate verdict so the Grafana panel shows the // merge/close/hold mix — the "are we rubber-stamping?" signal — even in advisory/dryRun (this is the rendered verdict). incr("loopover_gate_decisions_total", { @@ -10337,8 +10322,6 @@ async function maybePublishPrPublicSurface( // render warnings from the full/private manifest here. manifestWarnings: publicRepoFocusManifestForComment?.warnings ?? [], }); - } else { - deterministicBody = buildPublicPrIntelligenceComment(commentArgs); } try { await withReviewPipelineSpan( diff --git a/src/review/unified-comment-bridge.ts b/src/review/unified-comment-bridge.ts index 8c7fbb57ce..b2afee246a 100644 --- a/src/review/unified-comment-bridge.ts +++ b/src/review/unified-comment-bridge.ts @@ -2,13 +2,14 @@ // // A PURE, testable mapping from loopover's live PR-review data (the gate `GateCheckEvaluation`, the AI // `advisoryNotes` + consensus defect, the readiness signal rows + total, the footer) onto the ported -// unified renderer (`renderUnifiedReviewComment`). Flag-gated and default-OFF in the processor; flag-OFF -// keeps the legacy `buildPublicPrIntelligenceComment` path byte-identical. +// unified renderer (`renderUnifiedReviewComment`). #6103: this is now the ONLY PR-comment renderer -- +// the legacy `buildPublicPrIntelligenceComment` path this used to sit alongside was deleted once it had +// no remaining production caller. // // loopover's GATE stays authoritative: we pass the gate-derived `decision` into `buildUnifiedReviewInput` // so `deriveUnifiedStatus` lets it override the reviewer recommendations (the renderer already enforces -// this). The output PREPENDS the exact panel marker the legacy body carries, so the existing in-place -// upsert (`createOrUpdatePrIntelligenceComment`) updates the same comment instead of posting a duplicate. +// this). The output PREPENDS the panel marker, so the existing in-place upsert +// (`createOrUpdatePrIntelligenceComment`) updates the same comment instead of posting a duplicate. // // Public-safe: most inputs are already safe by construction — the AI notes via // `composeAdvisoryNotes`→`toPublicSafe`; the consensus-defect blocker via `toPublicSafe` (in @@ -64,8 +65,7 @@ export { splitAiReviewNits } from "./ai-notes"; // `warnings` (turned into Nits): they carry an AdvisoryFinding's raw title/action. The gate/check-run // path sanitizes those strings (sanitizeForCheckRun) before they reach GitHub, but this comment path // did not. Rather than trust that every present and FUTURE warning finding is benign, scrub Nits with a -// boundary mirroring the check-run sanitizer + the legacy panel's private-term guard, and DROP a Nit -// that still trips the guard. This never alters flag-OFF (the legacy panel keeps its own filtering). +// boundary mirroring the check-run sanitizer's own private-term guard, and DROP a Nit that still trips it. // // Mirrors src/rules/advisory.ts CHECK_RUN_FORBIDDEN_TERMS (scrubbed → "[context]") and // src/signals/engine.ts containsPrivatePublicTerm (drop if still present). Kept inline so this module @@ -114,7 +114,7 @@ export function verdictToRecommendation(verdict: Verdict): ReviewRecommendation } } -/** Derive an ok/warn/fail state from a legacy panel result cell's leading status icon (✅/⚠️/❌). */ +/** Derive an ok/warn/fail state from a panel result cell's leading status icon (✅/⚠️/❌). */ function rowState(resultCell: string): UnifiedSignalRow["state"] { if (resultCell.startsWith("✅")) return "ok"; if (resultCell.startsWith("❌")) return "fail"; @@ -129,11 +129,12 @@ function rowResultText(resultCell: string): string { return resultCell.replace(/^[✅⚠️❌ℹ️]+\s*/u, "").trim(); } -/** Map the legacy panel signal rows → the unified table's rows (label/state/result/evidence). The - * unified renderer adds its own "Code review" row first; these follow it (loopover's gate row included). - * `gates: true` only for the "Gate result" row (#6067) -- the ONLY row among these that can actually move - * the verdict; every other row's own Evidence/Action text already says it's advisory-only. Drives the split - * between the renderer's always-visible "Decision drivers" list and its collapsed advisory-signals fold. */ +/** Map the panel signal rows (buildPublicPrPanelSignalRows) → the unified table's rows + * (label/state/result/evidence). The unified renderer adds its own "Code review" row first; these follow + * it (loopover's gate row included). `gates: true` only for the "Gate result" row (#6067) -- the ONLY row + * among these that can actually move the verdict; every other row's own Evidence/Action text already says + * it's advisory-only. Drives the split between the renderer's always-visible "Decision drivers" list and + * its collapsed advisory-signals fold. */ export function panelRowsToSignalRows(rows: PublicPrPanelSignalRow[]): UnifiedSignalRow[] { return rows.map((row) => { const [label, result, evidence] = row.cells; @@ -322,7 +323,7 @@ export type UnifiedCommentBridgeArgs = { aiReview?: { notes: string } | undefined; /** The advisory findings — the bridge recovers the `ai_consensus_defect` consensus blocker from here. */ advisoryFindings?: AdvisoryFinding[] | undefined; - /** The legacy panel readiness signal rows (from `buildPublicPrPanelSignalRows`). */ + /** The readiness signal rows (from `buildPublicPrPanelSignalRows`). */ panelRows: PublicPrPanelSignalRow[]; /** Which rows the maintainer kept visible (`.loopover.yml review.fields`); a key set to `false` is hidden. */ reviewFields?: Partial> | undefined; @@ -822,7 +823,7 @@ export function buildUnifiedCommentBody(args: UnifiedCommentBridgeArgs): string : input.reviewerCount : 0; - // Honor `.loopover.yml review.fields` row visibility, exactly as the legacy panel does. + // Honor `.loopover.yml review.fields` row visibility. const visibleRows = args.panelRows.filter((row) => args.reviewFields?.[row.key] !== false); const signals = panelRowsToSignalRows(visibleRows); @@ -921,7 +922,7 @@ export function buildUnifiedCommentBody(args: UnifiedCommentBridgeArgs): string * routed through `buildUnifiedCommentBody` so a comment that started life as a unified OPEN-PR comment keeps * its unified shape (and the SAME marker) when the PR closes, instead of being overwritten by the legacy * panel under the shared marker. A synthetic `skipped` gate maps (via `gateConclusionToVerdict`) to the - * `comment` verdict → `advisory` status, matching the legacy panel's non-blocking NOTE tone. No AI review, + * `comment` verdict → `advisory` status (a non-blocking NOTE tone). No AI review, * no findings, and a single synthetic "Gate result — Skipped" signal row (the only signal we can assert for * a PR we never finished evaluating). Public-safe by construction: every string here is a static literal. */ diff --git a/src/signals/settings-preview.ts b/src/signals/settings-preview.ts index a90cabbfd1..85b8b21f72 100644 --- a/src/signals/settings-preview.ts +++ b/src/signals/settings-preview.ts @@ -9,17 +9,20 @@ import { buildCollisionReport, buildContributorProfile, buildPreflightResult, - buildPublicPrIntelligenceComment, + buildPublicPrPanelSignalRows, buildPublicReadinessScore, buildQueueHealth, type ContributorDetection, } from "./engine"; import { buildExtensionPrStatus, type ExtensionPrStatus } from "./extension-contributor-context"; import { REQUIRED_INSTALLATION_PERMISSIONS } from "../github/backfill"; -import type { LoopOverFooterEnv } from "../github/footer"; +import { loopoverFooter, type LoopOverFooterEnv } from "../github/footer"; +import type { GateCheckConclusion, GateCheckEvaluation } from "../rules/advisory"; import { LOOPOVER_GATE_CHECK_NAME, shouldPublishReviewCheck } from "../review/check-names"; import { decideReviewEligibility } from "../review/review-eligibility"; +import { buildUnifiedCommentBody } from "../review/unified-comment-bridge"; import { requiredAgentActionPermissions } from "../settings/agent-execution"; +import { isAgentConfigured } from "../settings/autonomy"; export function hasVisiblePrSurface(settings: RepositorySettings): boolean { return settings.publicSurface !== "off" || settings.checkRunMode === "enabled" || shouldPublishReviewCheck(settings.reviewCheckMode); @@ -722,5 +725,40 @@ function buildSamplePreviewComment(args: { args.issues, args.pullRequests, ); - return buildPublicPrIntelligenceComment({ repo: args.repo, pr: samplePr, profile, detection, queueHealth, collisions, preflight, settings: args.settings, env: args.env }); + + // Simulated gate verdict for this sample PR (#6103: migrated off the retired legacy renderer). Mirrors + // the same enabled / hard-linked-issue-block heuristic the shared panel builder used to fall back on + // internally when no real gate had run -- a duplicate-PR block is never simulated here since the + // synthetic PR #0 can't realistically collide with anything in this repo's real open PRs. + const gateEnabled = shouldPublishReviewCheck(args.settings.reviewCheckMode) || isAgentConfigured(args.settings.autonomy); + const hardLinkedIssueBlock = args.settings.linkedIssueGateMode === "block" && samplePr.linkedIssues.length === 0; + const gateConclusion: GateCheckConclusion = !gateEnabled ? "success" : hardLinkedIssueBlock ? "failure" : "success"; + const gate: GateCheckEvaluation = { + enabled: gateEnabled, + conclusion: gateConclusion, + title: !gateEnabled ? `${LOOPOVER_GATE_CHECK_NAME} not configured` : gateConclusion === "failure" ? `${LOOPOVER_GATE_CHECK_NAME} failed` : `${LOOPOVER_GATE_CHECK_NAME} passed`, + summary: "Simulated for this settings preview — no live gate evaluation ran.", + blockers: [], + warnings: [], + }; + + const { rows, readinessTotal } = buildPublicPrPanelSignalRows({ + repo: args.repo, + pr: samplePr, + profile, + detection, + queueHealth, + collisions, + preflight, + settings: args.settings, + gate: { conclusion: gateConclusion, summary: gate.summary }, + }); + + return buildUnifiedCommentBody({ + gate, + panelRows: rows, + readinessTotal, + changedFiles: 0, + footerMarkdown: loopoverFooter(args.env, {}), + }); } diff --git a/test/unit/queue-2.test.ts b/test/unit/queue-2.test.ts index 41ee67ba17..4be213131c 100644 --- a/test/unit/queue-2.test.ts +++ b/test/unit/queue-2.test.ts @@ -452,7 +452,9 @@ describe("queue processors", () => { expect(patchCount).toBeGreaterThanOrEqual(1); expect(firstWriteWasPlaceholder).toBe(true); expect(stickyComment.current?.body).toContain(PR_PANEL_COMMENT_MARKER); - expect(stickyComment.current?.body).toContain("Thanks for the contribution"); + // #6103: proves the real verdict overwrote the placeholder (not still "is reviewing") -- the converged + // renderer's own Suggested Action line, not the retired legacy renderer's "Thanks for the contribution". + expect(stickyComment.current?.body).toContain("Suggested Action"); expect(stickyComment.current?.body).not.toContain("is reviewing"); }); @@ -1061,7 +1063,9 @@ describe("queue processors", () => { expect(postCount).toBe(1); expect(patchCount).toBeGreaterThanOrEqual(1); expect(stickyComment.current?.body).toContain(PR_PANEL_COMMENT_MARKER); - expect(stickyComment.current?.body).toContain("Thanks for the contribution"); + // #6103: proves the real verdict overwrote the placeholder (not still "is reviewing") -- the converged + // renderer's own Suggested Action line, not the retired legacy renderer's "Thanks for the contribution". + expect(stickyComment.current?.body).toContain("Suggested Action"); expect(stickyComment.current?.body).not.toContain("is reviewing"); }); @@ -1225,7 +1229,11 @@ describe("queue processors", () => { expect(commentBodies[0]).toContain("🟪"); const finalComment = commentBodies.find((body) => !body.includes("is reviewing")); expect(finalComment).toBeDefined(); - expect(finalComment).toContain("Readiness score"); + // #6103: CI is still pending in this fixture, so the converged renderer correctly downgrades status to + // "held" (manual review recommended) even though the gate itself passes -- #6066's readiness-chip rule + // only shows `readiness N/100` when status === "ready", so it's correctly absent here. + expect(finalComment).toContain("Suggested Action - Manual Review"); + expect(finalComment).not.toMatch(/`readiness \d+\/100`/); expect(finalComment).not.toContain("stale cached nit"); expect(finalComment).toContain("did not include a separate narrative summary"); expect(finalComment).toContain("Add coverage for the new branch."); @@ -1301,7 +1309,7 @@ describe("queue processors", () => { expect(commentBodies.length).toBeGreaterThanOrEqual(2); expect(commentBodies[0]).toContain("is reviewing"); const finalComment = commentBodies.find((body) => !body.includes("is reviewing")); - expect(finalComment).toContain("LoopOver review needs maintainer review"); + expect(finalComment).toContain("manual review recommended"); // #6103: converged headline wording expect(finalComment).toContain("AI review could not be completed for this PR head"); expect(finalComment).not.toContain("The AI reviewer returned public review text but not the expected structured verdict"); // #regate-churn: the "AI review could not be completed" outcome is now PERSISTED (so a repeated scheduled @@ -1399,7 +1407,7 @@ describe("queue processors", () => { // The losing pass never called the AI a second time — it deferred to the lock instead of double-spending. expect(aiCalls).toBe(0); const finalComment = commentBodies.find((body) => !body.includes("is reviewing")); - expect(finalComment).toContain("LoopOver review needs maintainer review"); + expect(finalComment).toContain("manual review recommended"); // #6103: converged headline wording expect(finalComment).toContain("AI review is already running for this PR head in another LoopOver pass"); // A lock-contention placeholder must never be persisted at all (not even non-durably, #regate-churn) — the // concurrent pass it deferred to writes the REAL result within seconds, and replaying this placeholder for @@ -1474,7 +1482,11 @@ describe("queue processors", () => { expect(aiRun).not.toHaveBeenCalled(); expect(commentBodies.length).toBeGreaterThanOrEqual(2); const finalComment = commentBodies.find((body) => !body.includes("is reviewing")); - expect(finalComment).toContain("Readiness score"); + // #6103: CI is still pending in this fixture, so the converged renderer correctly downgrades status to + // "held" (manual review recommended) even though the gate itself passes -- #6066's readiness-chip rule + // only shows `readiness N/100` when status === "ready", so it's correctly absent here. + expect(finalComment).toContain("Suggested Action - Manual Review"); + expect(finalComment).not.toMatch(/`readiness \d+\/100`/); expect(finalComment).not.toContain("AI review returned public review text"); const audit = await env.DB.prepare("select event_type, metadata_json from audit_events where event_type = ?") .bind("github_app.ai_review_public_summary_missing") diff --git a/test/unit/queue-4.test.ts b/test/unit/queue-4.test.ts index 030262249a..a1a913a441 100644 --- a/test/unit/queue-4.test.ts +++ b/test/unit/queue-4.test.ts @@ -1855,9 +1855,19 @@ describe("queue processors", () => { // token: 1 — the installation token is now cached + reused within the request (was 2: main + permission check). // commentGets/commentPatches: 2 — first the purple reviewing placeholder, then the final refreshed panel. - expect(calls).toEqual({ token: 1, permission: 1, minerList: 1, commentGets: 2, commentPatches: 2, checkRuns: 0 }); + // checkRuns: 1 (#6103) — the converged renderer (now the only comment path) reads LIVE CI check-run state + // for its merge-readiness facts (the `CI green/pending/failing` chip); the retired legacy renderer never + // read live CI at all, so this repo's checkRunMode: "off" previously meant zero check-run reads too. + expect(calls).toEqual({ token: 1, permission: 1, minerList: 1, commentGets: 2, commentPatches: 2, checkRuns: 1 }); expect(patchedBody).toContain(""); - expect(patchedBody).toContain("Readiness score:"); + // #6103: this repo has reviewCheckMode: "off" and no autonomy configured, so gateEvaluation is never + // computed -- the renderer now synthesizes a "skipped" gate for rendering purposes only (see + // renderUnifiedReviewComment's caller in src/queue/processors.ts), which maps to the "advisory" status. + // #6066's readiness-chip rule only shows `readiness N/100` when status === "ready", so it's correctly + // absent here -- a chip claiming a readiness score next to an unconfigured/advisory gate would be the + // exact contradiction that rule exists to prevent. + expect(patchedBody).toContain("Suggested Action - Advisory Only"); + expect(patchedBody).not.toMatch(/`readiness \d+\/100`/); expect(patchedBody).toContain("- [ ] Re-run LoopOver review"); expect(patchedBody).not.toContain("- [x] "); const audit = await env.DB.prepare("select event_type, actor, target_key, outcome from audit_events where event_type = ?") @@ -2708,12 +2718,11 @@ describe("queue processors", () => { expect(skipped.results.map((event) => event.detail)).toEqual(expect.arrayContaining(["not_official_gittensor_miner", "missing_author"])); }); - // #1007 convergence (Stage D): with LOOPOVER_REVIEW_UNIFIED_COMMENT on AND the gate evaluating, the public PR-panel - // comment is rendered by the UNIFIED renderer (GitHub alert + synthesized "Code review" row) instead of the - // legacy panel — while STILL leading with the same panel marker so the in-place upsert updates the same - // comment. Mirrors the legacy panel-posting setup (confirmed miner + comment_and_label) but flips the flag - // and enables the gate so `maybePublishPrPublicSurface` takes the flag-ON branch. - it("renders the unified PR-review comment when the flag is on and the gate evaluates", async () => { + // #1007 convergence (Stage D) / #6103: the public PR-panel comment is rendered by the UNIFIED renderer + // (GitHub alert + synthesized "Code review" row / Decision drivers) unconditionally now -- leading with + // the same panel marker so the in-place upsert updates the same comment. `LOOPOVER_REVIEW_UNIFIED_COMMENT` + // below is kept for historical parity with sibling tests; it's inert (no longer read for this decision). + it("renders the unified PR-review comment when the gate evaluates", async () => { const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), LOOPOVER_REVIEW_UNIFIED_COMMENT: "1" }); await persistRegistrySnapshot( env, diff --git a/test/unit/queue-5.test.ts b/test/unit/queue-5.test.ts index 1d8acb4247..5dd2554c3c 100644 --- a/test/unit/queue-5.test.ts +++ b/test/unit/queue-5.test.ts @@ -5541,9 +5541,6 @@ describe("queue processors", () => { const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), LOOPOVER_REVIEW_E2E_TESTS: "true", - // The checkbox/collapsible only render via the CONVERGED comment builder (buildUnifiedCommentBody); - // the legacy buildPublicPrIntelligenceComment path has neither and must be opted out of here too. - LOOPOVER_REVIEW_UNIFIED_COMMENT: "true", }); const slash = repoFullName.indexOf("/"); await upsertRepositoryFromGitHub(env, { name: repoFullName.slice(slash + 1), full_name: repoFullName, private: false, owner: { login: "JSONbored" } }, 123); @@ -5554,11 +5551,10 @@ describe("queue processors", () => { publicSurface: "comment_and_label", autoLabelEnabled: false, checkRunMode: "off", - // reviewCheckMode MUST be "required" (not "disabled") -- maybePublishPrPublicSurface only takes the - // UNIFIED renderer branch when BOTH unifiedCommentAllowed AND gateEvaluation are truthy; gateEvaluation - // is never computed at all when the gate is off, silently falling back to the legacy panel (which has - // neither the Test coverage collapsible nor the generate-tests checkbox). Mirrors the settings shape - // of the pre-existing "renders the unified PR-review comment..." test above. + // reviewCheckMode MUST be "required" (not "disabled") -- gateEvaluation is never computed at all when + // the gate is off (#6103: the renderer then falls back to a synthetic "skipped" gate for rendering + // purposes only), and this test needs the REAL evaluated gate's data for the checkbox/collapsible to + // render. Mirrors the settings shape of the pre-existing "renders the unified PR-review comment..." test above. reviewCheckMode: "required", requireLinkedIssue: false, linkedIssueGateMode: "off", diff --git a/test/unit/settings-preview.test.ts b/test/unit/settings-preview.test.ts index 4a33f6cfd3..9daeb8e496 100644 --- a/test/unit/settings-preview.test.ts +++ b/test/unit/settings-preview.test.ts @@ -449,10 +449,31 @@ describe("buildRepoSettingsPreview", () => { sample: { authorLogin: "miner", minerStatus: "confirmed", title: "Improve wallet hotkey trust score payout", body: "raw trust and scoreability /100 reviewability 5", labels: ["bug"], linkedIssues: [7] }, }); expect(preview.previewComment).not.toBeNull(); - expect(preview.previewComment ?? "").toMatch(/Readiness score: \d+\/100/); + // #6103: the converged renderer shows readiness as a `readiness N/100` status chip, not "Readiness score:" prose. + expect(preview.previewComment ?? "").toMatch(/`readiness \d+\/100`/); expect(preview.previewComment ?? "").not.toMatch(/wallet|hotkey|trust score|raw trust|scoreability|payout|reward|farming|reviewability\s*\d/i); }); + it("#6103: simulates a passing gate in the preview comment when the opt-in gate is enabled and the linked-issue block does not apply", () => { + const preview = buildRepoSettingsPreview({env: {}, + ...base, + settings: settings({ reviewCheckMode: "required", linkedIssueGateMode: "block" }), + installation: healthyInstall, + sample: { authorLogin: "miner", minerStatus: "confirmed", linkedIssues: [7] }, + }); + expect(preview.previewComment ?? "").toContain("✅ Gate result — Passing"); + }); + + it("#6103: simulates a failing gate in the preview comment when linkedIssueGateMode is 'block' and the sample has no linked issue", () => { + const preview = buildRepoSettingsPreview({env: {}, + ...base, + settings: settings({ reviewCheckMode: "required", linkedIssueGateMode: "block" }), + installation: healthyInstall, + sample: { authorLogin: "miner", minerStatus: "confirmed", linkedIssues: [] }, + }); + expect(preview.previewComment ?? "").toContain("❌ Gate result — Blocking"); + }); + it("reports a generic needs-attention summary when health is degraded but no permission or event is missing", () => { const preview = buildRepoSettingsPreview({env: {}, ...base, diff --git a/test/unit/signals-coverage.test.ts b/test/unit/signals-coverage.test.ts index 40907c611c..1b43b606e0 100644 --- a/test/unit/signals-coverage.test.ts +++ b/test/unit/signals-coverage.test.ts @@ -19,9 +19,9 @@ import { buildMaintainerPacket, buildPreflightResult, buildPublicCommentSignalBundle, - buildPublicPrIntelligenceComment, buildPublicPrPanelSignalRows, buildPublicReadinessScore, + buildPublicSafeCollapsibles, buildQueueHealth, buildRoleContext, detectGittensorContributor, @@ -41,7 +41,7 @@ import { buildRepoRewardRisk, } from "../../src/signals/reward-risk"; import { PREFLIGHT_LIMITS } from "../../src/signals/preflight-limits"; -import { REVIEW_FIELD_KEYS, type FocusManifestReviewConfig } from "../../src/signals/focus-manifest"; +import { REVIEW_FIELD_KEYS } from "../../src/signals/focus-manifest"; import type { GittensorContributorSnapshot } from "../../src/gittensor/api"; import type { ContributorRepoStatRecord, @@ -748,7 +748,14 @@ describe("signal coverage edge cases", () => { [], [], ); - const comment = buildPublicPrIntelligenceComment({env: {}, + // #6103: rewired off the retired legacy renderer onto the shared row/collapsible builders. The panelTitle + // ("Confirmed Gittensor contributor...") and "Readiness score: N/100" template text were the deleted + // function's own copy (no surviving equivalent); the private/critical-finding leak invariant is now a + // stronger structural guarantee -- "Maintainer notes" (the section that would have surfaced finding + // detail/title text) is excluded from buildPublicSafeCollapsibles entirely (see its own doc comment + + // unified-comment-parity.test.ts), and finding.action (the only field publicSafeNextSteps still reads) + // carries no forbidden vocabulary in this fixture. + const panel = buildPublicPrPanelSignalRows({ repo: directRepo, pr: prRecord, profile, @@ -766,12 +773,11 @@ describe("signal coverage edge cases", () => { settings: { ...repoSettings(directRepo.fullName), publicSignalLevel: "minimal", requireLinkedIssue: true }, }); - expect(comment).toContain("Confirmed Gittensor contributor"); - expect(comment).toContain("| Linked issue | ⚠️ Missing |"); - expect(comment).toMatch(/Readiness score: \d+\/100/); - expect(comment).not.toMatch(/reward|wallet|hotkey|trust score|farming|critical private/i); + expect(panel.rows.find((row) => row.key === "linkedIssue")!.cells[1]).toBe("⚠️ Missing"); + expect(typeof panel.readinessTotal).toBe("number"); - const maintainerComment = buildPublicPrIntelligenceComment({env: {}, + const maintainerCollapsibles = buildPublicSafeCollapsibles({ + env: {}, repo: directRepo, pr: { ...prRecord, authorLogin: "owner", authorAssociation: "OWNER", linkedIssues: [1], body: "Fixes #1" }, profile: buildContributorProfile("owner", { login: "owner", topLanguages: ["TypeScript"], source: "github" }, [], []), @@ -782,8 +788,7 @@ describe("signal coverage edge cases", () => { settings: repoSettings(directRepo.fullName), }); - expect(maintainerComment).toContain("maintainer lane"); - expect(maintainerComment).not.toMatch(/reward|wallet|hotkey|trust score|farming/i); + expect(maintainerCollapsibles.find((section) => section.title === "Review context")!.body).toContain("(maintainer lane)"); }); it("buildPublicPrPanelSignalRows derives the gate conclusion across provided/fallback paths (#1007 unified-panel extraction)", () => { @@ -873,9 +878,10 @@ describe("signal coverage edge cases", () => { expect(gateRow.cells[2]).not.toBe("Advisory only."); expect(gateRow.cells[3]).not.toBe("No action."); - const comment = buildPublicPrIntelligenceComment(baseArgs); - expect(comment).toContain("LoopOver Orb Review Agent is blocking merge"); - expect(comment).toContain("> [!CAUTION]"); + // #6103: the panel-row checks above already prove the blocking gate result structurally; the + // "LoopOver Orb Review Agent is blocking merge" / "> [!CAUTION]" phrasing was the deleted legacy + // renderer's own panelTitle/alert derivation, with no surviving equivalent (the converged renderer + // derives its verdict via deriveUnifiedStatus in src/review/unified-comment.ts, tested separately). // Sanity check: the SAME disabled-check-run repo WITHOUT autonomy configured correctly stays advisory-only // (no gate evaluation happens at all, so there is nothing real to surface). @@ -926,20 +932,15 @@ describe("signal coverage edge cases", () => { }; it("omits the row entirely when the caller passes no improvementSignal (feature off, or not wired yet)", () => { - const comment = buildPublicPrIntelligenceComment({ ...improvementBaseArgs, env: {} }); - expect(comment).not.toContain("| Improvement |"); const panel = buildPublicPrPanelSignalRows(improvementBaseArgs); expect(panel.rows.find((r) => r.key === "improvementSignal")).toBeUndefined(); expect(panel.rows).toHaveLength(7); }); it("renders a concise value rating (#5101) and never dumps the finding sentences into the cell", () => { - const comment = buildPublicPrIntelligenceComment({ ...improvementBaseArgs, improvementSignal: minorAssessment, env: {} }); - expect(comment).toContain("| Improvement | ✅ Minor | value: minor |"); - // #5101: the raw finding sentence is intentionally no longer rendered — the cell is a quick rating. - expect(comment).not.toContain("Code changes are accompanied by test evidence."); - expect(comment).not.toContain("LLM value judgment"); - + // #6103: rewired directly onto buildPublicPrPanelSignalRows -- the exact `cells` equality below already + // pins the row to "value: minor" with no finding sentence or LLM text, so the retired comment-string + // assertions this test used to run alongside it were redundant. const panel = buildPublicPrPanelSignalRows({ ...improvementBaseArgs, improvementSignal: minorAssessment }); expect(panel.rows).toHaveLength(8); const row = panel.rows.find((r) => r.key === "improvementSignal")!; @@ -954,20 +955,10 @@ describe("signal coverage edge cases", () => { it("tags the concise rating with the LLM's one-word magnitude (not the full rationale) when both tiers are available (#5101)", () => { const noneAssessment = { improvementScore: 0, band: "none" as const, findings: [] }; const valueAssessment = { magnitude: "significant" as const, rationale: "This removes a whole class of retry bugs." }; - const comment = buildPublicPrIntelligenceComment({ - ...improvementBaseArgs, - improvementSignal: noneAssessment, - aiReview: { notes: "Looks fine.", valueAssessment }, - env: {}, - }); - // The Result cell reflects the DETERMINISTIC band ("none"), never the LLM magnitude -- the two tiers are - // deliberately never blended into one number/label (epic #4737 design constraint 1). - expect(comment).toContain("| Improvement | ℹ️ None detected | value: none · LLM: significant |"); - // #5101: only the one-word magnitude, never the full rationale paragraph, is surfaced. - expect(comment).toContain("· LLM: significant"); - expect(comment).not.toContain("This removes a whole class of retry bugs."); - expect(comment).not.toContain("LLM value judgment"); - + // #6103: rewired directly onto buildPublicPrPanelSignalRows. The Result cell reflects the DETERMINISTIC + // band ("none"), never the LLM magnitude -- the two tiers are deliberately never blended into one + // number/label (epic #4737 design constraint 1); the Evidence cell carries only the one-word magnitude, + // never the full rationale paragraph (#5101). const panel = buildPublicPrPanelSignalRows({ ...improvementBaseArgs, improvementSignal: noneAssessment, valueAssessment }); const row = panel.rows.find((r) => r.key === "improvementSignal")!; expect(row.cells[1]).toBe("ℹ️ None detected"); @@ -1001,15 +992,13 @@ describe("signal coverage edge cases", () => { expect(manyFindingsRow.cells[2]).not.toContain("cyclomatic complexity"); }); - it("hides the row via review.fields.improvementSignal: false, exactly like its seven siblings", () => { - const comment = buildPublicPrIntelligenceComment({ - ...improvementBaseArgs, - improvementSignal: minorAssessment, - review: { present: true, fields: { improvementSignal: false } } as unknown as FocusManifestReviewConfig, - env: {}, - }); - expect(comment).not.toContain("| Improvement |"); - }); + // #6103: "hides the row via review.fields.improvementSignal: false" used to render the retired legacy + // comment with a `review.fields` override and check the row's absence from the markdown string. + // `buildPublicPrPanelSignalRows` never applied `review.fields` itself (that filtering happened in the + // legacy renderer's own `allRows.filter(...)` and now happens in the converged bridge -- + // `src/review/unified-comment-bridge.ts`'s `visibleRows = args.panelRows.filter((row) => + // args.reviewFields?.[row.key] !== false)`), so there is no equivalent to rewire this onto in this file; + // the bridge-level behavior is covered by its own unit tests. it("REGRESSION (#4744): never leaks forbidden vocabulary regardless of what findings/rationale feed the row (mirrors the repo's existing public-safety invariant tests)", () => { const unsafeAssessment = { @@ -1031,17 +1020,12 @@ describe("signal coverage edge cases", () => { }; const forbidden = /wallet|hotkey|coldkey|trust score|reward|payout|scoreability|reviewability|farming/i; - const comment = buildPublicPrIntelligenceComment({ - ...improvementBaseArgs, - improvementSignal: unsafeAssessment, - aiReview: { notes: "Looks fine.", valueAssessment: unsafeValueAssessment }, - env: {}, - }); - expect(comment).toContain("| Improvement | ✅ Moderate |"); // the static band label itself still renders - expect(comment).not.toMatch(forbidden); - + // #6103: rewired directly onto buildPublicPrPanelSignalRows -- the static band label ("✅ Moderate") + // and the forbidden-vocabulary invariant below cover the same ground the retired comment-string + // assertions used to. const panel = buildPublicPrPanelSignalRows({ ...improvementBaseArgs, improvementSignal: unsafeAssessment, valueAssessment: unsafeValueAssessment }); const row = panel.rows.find((r) => r.key === "improvementSignal")!; + expect(row.cells[1]).toBe("✅ Moderate"); // the static band label itself still renders expect(JSON.stringify(row)).not.toMatch(forbidden); // #5101 makes this structurally leak-proof: the cell renders only closed-enum band/magnitude names, never // the free-text finding detail or LLM rationale, so no sanitizer pass is even needed. The unsafe finding @@ -1101,9 +1085,6 @@ describe("signal coverage edge cases", () => { expect(row.cells[2]).toBe("risk: low · value: minor"); // The Result cell (the deterministic band label) is untouched by the quadrant -- the two tiers never blend. expect(row.cells[1]).toBe("✅ Minor"); - - const comment = buildPublicPrIntelligenceComment({ ...quadrantBaseArgs, improvementSignal: quadrantAssessment, slopBand: "low", env: {} }); - expect(comment).toContain("| Improvement | ✅ Minor | risk: low · value: minor |"); }); it("falls back to the value band alone when slopBand is omitted -- never a fabricated risk reading (#5101)", () => { @@ -1117,10 +1098,6 @@ describe("signal coverage edge cases", () => { const panel = buildPublicPrPanelSignalRows({ ...quadrantBaseArgs, slopBand: "high" }); expect(panel.rows.find((r) => r.key === "improvementSignal")).toBeUndefined(); expect(panel.rows).toHaveLength(7); - - const comment = buildPublicPrIntelligenceComment({ ...quadrantBaseArgs, slopBand: "high", env: {} }); - expect(comment).not.toContain("| Improvement |"); - expect(comment).not.toContain("risk: high"); }); it("REGRESSION: the quadrant clause can never leak forbidden vocabulary -- SlopBand/ImprovementBand are closed enums, never free text, so no sanitizer check is needed for this clause specifically", () => { @@ -1179,16 +1156,11 @@ describe("signal coverage edge cases", () => { expect(onWinner).not.toBe(offWinner); expect(onLoser).toBe(offLoser); - // The comment builder must agree by construction: ON winner is NOT a blocking-merge panel; ON loser is. - const winnerComment = buildPublicPrIntelligenceComment({ ...baseFor(winnerPr), duplicateWinnerEnabled: true }); - const loserComment = buildPublicPrIntelligenceComment({ ...baseFor(loserPr), duplicateWinnerEnabled: true }); - expect(winnerComment).not.toContain("LoopOver Orb Review Agent is blocking merge"); - expect(winnerComment).not.toContain("#88"); + // #6103: the gate-cell equality/inequality checks above already prove the winner/loser suppression + // structurally; the retired "must agree by construction" comment-string checks ("LoopOver Orb Review + // Agent is blocking merge") tested the deleted legacy renderer's own panelTitle derivation, which has no + // surviving equivalent. The related-work row confirms the winner's related work also clears. expect(buildPublicPrPanelSignalRows({ ...baseFor(winnerPr), duplicateWinnerEnabled: true }).rows.find((r) => r.key === "relatedWork")!.cells[1]).toContain("No active overlap"); - expect(loserComment).toContain("LoopOver Orb Review Agent is blocking merge"); - // Flag OFF on the winner is byte-identical to a blocking panel (today's behavior). - const offWinnerComment = buildPublicPrIntelligenceComment(baseFor(winnerPr)); - expect(offWinnerComment).toContain("LoopOver Orb Review Agent is blocking merge"); }); it("#dup-winner: hides duplicate-only same-issue evidence while preserving mixed scoped overlap context", () => { @@ -1269,16 +1241,21 @@ describe("signal coverage edge cases", () => { const retainedSparseCluster = retainedSameIssueView.scopedOverlapClusters.find((cluster) => cluster.id === "issue-42-with-sparse-peer"); const relatedRow = buildPublicPrPanelSignalRows(baseArgs).rows.find((r) => r.key === "relatedWork")!; - const comment = buildPublicPrIntelligenceComment(baseArgs); + // #6103: rewired off the retired legacy comment onto buildPublicSafeCollapsibles' "Review context" body -- + // relatedWorkDetails (the SAME helper the legacy panel drew from) is still what feeds that section, so it + // proves the same suppression: the hard-duplicate cluster's reason text ("Open PR work references issue + // #42.") never surfaces, only the retained scoped-overlap cluster's ("Titles/paths share 3 meaningful + // terms.") does. "Same-issue duplicate risk found against #88" was the deleted panelSummary's own + // phrasing (no surviving equivalent). + const reviewContext = buildPublicSafeCollapsibles(baseArgs).find((section) => section.title === "Review context")!.body; expect(retainedSameIssueView.visibleLinkedDuplicatePrs).toEqual([]); expect(retainedSparseCluster?.items.map((item) => (item.type === "pull_request" ? item.number : item.type))).toEqual([winnerPr.number, 99]); expect(relatedRow.cells[1]).toContain("1 scoped overlap"); expect(relatedRow.cells[1]).not.toContain("#88"); - expect(comment).toContain("Titles/paths share 3 meaningful terms"); - expect(comment).toContain("PR #88"); - expect(comment).not.toContain("Same-issue duplicate risk found against #88"); - expect(comment).not.toContain("Open PR work references issue #42."); + expect(reviewContext).toContain("Titles/paths share 3 meaningful terms"); + expect(reviewContext).toContain("PR #88"); + expect(reviewContext).not.toContain("Open PR work references issue #42."); }); it("renders opt-in gate panel states for collision and repo evaluation blockers", () => { @@ -1298,29 +1275,21 @@ describe("signal coverage edge cases", () => { const detection = { detected: true, source: "github_cache" as const, reason: "cached contributor", priorPullRequests: 1, priorMergedPullRequests: 0, priorIssues: 0 }; const gateSettings = { ...repoSettings(directRepo.fullName), reviewCheckMode: "required" as const, duplicatePrGateMode: "block" as const }; - const collisionComment = buildPublicPrIntelligenceComment({env: {}, - repo: directRepo, - pr: currentPr, - profile, - detection, - queueHealth, - collisions, - preflight, - settings: gateSettings, - }); - - expect(collisionComment).toContain("> [!CAUTION]"); - expect(collisionComment).toContain("A repo-configured hard blocker was found."); - expect(collisionComment).toContain("> | Gate result | ❌ Blocking | Repo-configured hard blocker found. | Fix blocker. |"); - expect(collisionComment).toContain("Public profile only"); - expect(collisionComment).toContain("Compare #8."); - expect(collisionComment).not.toContain("possible overlaps"); - expect(collisionComment).not.toContain("Cached OSS contributor activity"); - expect(collisionComment).not.toContain("Cached prior PRs/issues"); - // The always-on earn CTA footer is a permanent marketing surface on every PR. - expect(collisionComment).toContain("register to start earning"); + // #6103: rewired off the retired legacy renderer onto the shared row/collapsible builders. The alert + // marker ("> [!CAUTION]" etc.), panelTitle/panelSummary prose, and the "> | ... |" quote-block table + // formatting were the deleted function's own template -- gone with no surviving equivalent (the + // converged renderer derives its status via deriveUnifiedStatus in src/review/unified-comment.ts, + // tested separately). Every still-meaningful row/collapsible-content assertion below is rewired onto + // buildPublicPrPanelSignalRows / buildPublicSafeCollapsibles. + const collisionArgs = { env: {}, repo: directRepo, pr: currentPr, profile, detection, queueHealth, collisions, preflight, settings: gateSettings }; + expect(buildPublicPrPanelSignalRows(collisionArgs).rows.find((r) => r.key === "gateResult")!.cells).toEqual([ + "Gate result", "❌ Blocking", "Repo-configured hard blocker found.", "Fix blocker.", + ]); + expect(buildPublicPrPanelSignalRows(collisionArgs).rows.find((r) => r.key === "relatedWork")!.cells[3]).toBe("Compare #8."); + expect(buildPublicSafeCollapsibles(collisionArgs).find((section) => section.title === "Review context")!.body).toContain("Public profile only"); - const repoBlockedComment = buildPublicPrIntelligenceComment({env: {}, + const repoBlockedArgs = { + env: {}, repo: null, pr: { ...currentPr, linkedIssues: [99], body: "Fixes #99" }, profile, @@ -1334,14 +1303,14 @@ describe("signal coverage edge cases", () => { [], ), settings: gateSettings, - }); - + }; // App/infra state (repo not synced) never blocks a contributor — the gate stays neutral/advisory. - expect(repoBlockedComment).toContain("Public profile only"); - expect(repoBlockedComment).toContain("> | Gate result | ⚠️ Not blocking | Advisory; not blocking this PR. | No action. |"); - expect(repoBlockedComment).not.toContain("App action required"); + expect(buildPublicSafeCollapsibles(repoBlockedArgs).find((section) => section.title === "Review context")!.body).toContain("Public profile only"); + expect(buildPublicPrPanelSignalRows(repoBlockedArgs).rows.find((r) => r.key === "gateResult")!.cells).toEqual([ + "Gate result", "⚠️ Not blocking", "Advisory; not blocking this PR.", "No action.", + ]); - const missingIssueComment = buildPublicPrIntelligenceComment({env: {}, + const missingIssuePanel = buildPublicPrPanelSignalRows({ repo: directRepo, pr: { ...currentPr, linkedIssues: [], body: "No linked issue yet." }, profile, @@ -1356,12 +1325,11 @@ describe("signal coverage edge cases", () => { ), settings: { ...gateSettings, requireLinkedIssue: true, linkedIssueGateMode: "block" }, }); + expect(missingIssuePanel.rows.find((r) => r.key === "linkedIssue")!.cells).toEqual([ + "Linked issue", "⚠️ Missing", "No linked issue or no-issue rationale found.", "Explain no-issue PR.", + ]); - expect(missingIssueComment).toContain("> [!WARNING]"); - expect(missingIssueComment).toContain("> | Linked issue | ⚠️ Missing | No linked issue or no-issue rationale found. | Explain no-issue PR. |"); - expect(missingIssueComment).toContain("Explain no-issue PR."); - - const passingGateComment = buildPublicPrIntelligenceComment({env: {}, + const passingGatePanel = buildPublicPrPanelSignalRows({ repo: directRepo, pr: { ...currentPr, linkedIssues: [99], body: "Fixes #99" }, profile, @@ -1376,73 +1344,19 @@ describe("signal coverage edge cases", () => { ), settings: gateSettings, }); + expect(passingGatePanel.rows.find((r) => r.key === "gateResult")!.cells).toEqual([ + "Gate result", "✅ Passing", "No configured blocker found.", "No action.", + ]); - expect(passingGateComment).toContain("> [!TIP]"); - expect(passingGateComment).toContain("> | Gate result | ✅ Passing | No configured blocker found. | No action. |"); - expect(passingGateComment).toContain("Public GitHub metadata was checked"); - - // .loopover.yml review overrides: custom footer lead, an intro note, and a hidden row. - const customizedComment = buildPublicPrIntelligenceComment({env: {}, - repo: directRepo, - pr: { ...currentPr, linkedIssues: [99], body: "Fixes #99" }, - profile, - detection, - queueHealth: buildQueueHealth(directRepo, [], [currentPr], buildCollisionReport(directRepo.fullName, [], [currentPr])), - collisions: buildCollisionReport(directRepo.fullName, [], [currentPr]), - preflight: buildPreflightResult({ repoFullName: directRepo.fullName, title: "Fix isolated issue", body: "Fixes #99", linkedIssues: [99] }, directRepo, [], [currentPr]), - settings: gateSettings, - review: { present: true, footerText: "Reviewed by the Acme maintainer bot.", note: "Run npm test before pushing.", fields: { relatedWork: false }, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, fixHandoff: null, autoMergeSummary: null, suggestions: null, changedFilesSummary: null, effortScore: null, impactMap: null, cultureProfile: null, selftune: null, reviewMemory: null, findingCategories: null, inlineCommentsPerCategory: null, minFindingSeverity: null, maxFindings: { blockers: null, nits: null }, commentVerbosity: null, e2eTestDelivery: null, e2eTestAutoTrigger: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { skipDrafts: null, cadence: null, ignoreAuthors: [], ignoreTitleKeywords: [], skipLabels: [], skipDocsOnly: null, maxAddedLines: 0, maxFiles: 0, baseBranches: [], autoPauseAfterReviewedCommits: null }, aiModel: { claudeModel: null, claudeEffort: null, codexModel: null, codexEffort: null, ollamaModel: null, openaiModel: null, openaiCompatibleModel: null, anthropicModel: null }, visual: { productionUrl: null, preview: { urlTemplate: null }, routes: { paths: [], maxRoutes: null }, themes: [], gif: false, enabled: null, themeStorageKey: null, actionsFallback: false }, linkedIssueSatisfaction: null, sharedConfigSource: null }, - aiReview: { notes: "The change is focused.\n\n**Nits (2)**\n- Add a test for the
edge case.\n- Keep the validator helper scoped." }, - }); - expect(customizedComment).toContain("Reviewed by the Acme maintainer bot."); // custom footer lead - expect(customizedComment).toContain("register to start earning"); // mandatory attribution/earn link kept - expect(customizedComment).toContain("Run npm test before pushing."); // intro note - expect(customizedComment).not.toContain("| Related work |"); // hidden row - expect(customizedComment).toContain("| Gate result |"); // non-hidden rows still rendered - expect(customizedComment).toContain("**Review summary**"); // AI summary is prominent, not buried - expect(customizedComment).toContain("Nits (2)"); // nits are directly below summary - expect(customizedComment).not.toContain("LoopOver AI review (advisory)"); // old bottom dropdown removed - expect(customizedComment).toContain("</details>"); // stray tags escaped, panel structure preserved - const summaryIndex = customizedComment.indexOf("**Review summary**"); - const nitsIndex = customizedComment.indexOf("Nits (2)"); - const readinessIndex = customizedComment.indexOf("**Readiness score:"); - expect(summaryIndex).toBeGreaterThan(-1); - expect(nitsIndex).toBeGreaterThan(summaryIndex); - expect(readinessIndex).toBeGreaterThan(nitsIndex); - - const aiBlockedComment = buildPublicPrIntelligenceComment({env: {}, - repo: directRepo, - pr: { ...currentPr, linkedIssues: [99], body: "Fixes #99" }, - profile, - detection, - queueHealth: buildQueueHealth(directRepo, [], [currentPr], buildCollisionReport(directRepo.fullName, [], [currentPr])), - collisions: buildCollisionReport(directRepo.fullName, [], [currentPr]), - preflight: buildPreflightResult({ repoFullName: directRepo.fullName, title: "Fix isolated issue", body: "Fixes #99", linkedIssues: [99] }, directRepo, [], [currentPr]), - settings: { ...repoSettings(directRepo.fullName), reviewCheckMode: "disabled" }, - aiReview: { notes: "The change is currently unsafe to merge.\n\n**Blockers**\n- `src/a.ts` has a syntax error.\n\n**Nits (1)**\n- Add a regression test." }, - }); - expect(aiBlockedComment).toContain("> [!CAUTION]"); - expect(aiBlockedComment).toContain("LoopOver review found blockers"); - expect(aiBlockedComment).toContain("`src/a.ts` has a syntax error."); - expect(aiBlockedComment.indexOf("**Review summary**")).toBeLessThan(aiBlockedComment.indexOf("**Readiness score:")); - - const aiExplicitNoBlockersComment = buildPublicPrIntelligenceComment({env: {}, - repo: directRepo, - pr: { ...currentPr, linkedIssues: [99], body: "Fixes #99" }, - profile, - detection, - queueHealth: buildQueueHealth(directRepo, [], [currentPr], buildCollisionReport(directRepo.fullName, [], [currentPr])), - collisions: buildCollisionReport(directRepo.fullName, [], [currentPr]), - preflight: buildPreflightResult({ repoFullName: directRepo.fullName, title: "Fix isolated issue", body: "Fixes #99", linkedIssues: [99] }, directRepo, [], [currentPr]), - settings: { ...repoSettings(directRepo.fullName), reviewCheckMode: "disabled" }, - aiReview: { notes: "The change is focused.\n\n**Blockers**\n- None.\n\n**Nits (1)**\n- Add a regression test." }, - }); - expect(aiExplicitNoBlockersComment).toContain("> [!TIP]"); - expect(aiExplicitNoBlockersComment).not.toContain( - "LoopOver review found blockers", - ); + // #6103: the `.loopover.yml review` overrides (custom footer lead, intro note, `review.fields` row + // hiding) and the AI review blockers/nits rendering (ordering, escaping, "old bottom dropdown removed") + // were entirely the deleted legacy renderer's own template structure. `review.fields` hiding now + // happens in the converged bridge (src/review/unified-comment-bridge.ts's `visibleRows`, tested there); + // the footer/note text is covered by test/unit/footer.test.ts; the AI blockers/nits layout is owned by + // src/review/unified-comment.ts's `renderUnifiedReviewComment` (tested independently). None of that has + // a surviving equivalent in this file to rewire onto. - const advisoryOnlyComment = buildPublicPrIntelligenceComment({env: {}, + const advisoryOnlyPanel = buildPublicPrPanelSignalRows({ repo: directRepo, pr: { ...currentPr, linkedIssues: [99], body: "Fixes #99" }, profile, @@ -1460,13 +1374,11 @@ describe("signal coverage edge cases", () => { }, settings: { ...repoSettings(directRepo.fullName), reviewCheckMode: "disabled" }, }); + expect(advisoryOnlyPanel.rows.find((r) => r.key === "gateResult")!.cells).toEqual([ + "Gate result", "⚠️ Advisory only", "Advisory only.", "No action.", + ]); - expect(advisoryOnlyComment).toContain("> [!WARNING]"); - expect(advisoryOnlyComment).toContain("LoopOver found maintainer review notes"); - expect(advisoryOnlyComment).toContain("Validation note missing"); - expect(advisoryOnlyComment).toContain("> | Gate result | ⚠️ Advisory only | Advisory only. | No action. |"); - - const actionRequiredComment = buildPublicPrIntelligenceComment({env: {}, + const actionRequiredPanel = buildPublicPrPanelSignalRows({ repo: directRepo, pr: { ...currentPr, linkedIssues: [99], body: "Fixes #99" }, profile, @@ -1482,45 +1394,19 @@ describe("signal coverage edge cases", () => { settings: gateSettings, gate: { conclusion: "action_required", summary: "LoopOver cannot evaluate this PR until installation state is repaired." }, }); - expect(actionRequiredComment).toContain("> [!WARNING]"); - expect(actionRequiredComment).toContain("LoopOver cannot evaluate this PR until installation state is repaired."); - expect(actionRequiredComment).toContain("> | Gate result | ⚠️ App action required | Install/config needs attention. | Fix app config. |"); + expect(actionRequiredPanel.rows.find((r) => r.key === "gateResult")!.cells).toEqual([ + "Gate result", "⚠️ App action required", "Install/config needs attention.", "Fix app config.", + ]); - // REGRESSION: gateHeld's panelSummary falls back to a literal default when NO gate was passed at all -- - // gateConclusion then resolves via fallbackGateConclusion's `!args.repo` arm to "neutral" (gateHeld-only, - // unlike "action_required" above which is ALSO gateBlocking and never reaches this fallback), and - // args.gate?.summary is undefined (no gate object exists to read a summary from), so the literal default - // text renders instead of a provided/derived summary. - const heldNoSummaryComment = buildPublicPrIntelligenceComment({env: {}, - repo: null, - pr: { ...currentPr, linkedIssues: [99], body: "Fixes #99" }, - profile, - detection, - queueHealth: buildQueueHealth(directRepo, [], [currentPr], buildCollisionReport(directRepo.fullName, [], [currentPr])), - collisions: buildCollisionReport(directRepo.fullName, [], [currentPr]), - preflight: buildPreflightResult( - { repoFullName: directRepo.fullName, title: "Fix isolated issue", body: "Fixes #99", linkedIssues: [99] }, - directRepo, - [], - [currentPr], - ), - settings: gateSettings, - }); - expect(heldNoSummaryComment).toContain("> [!WARNING]"); - expect(heldNoSummaryComment).toContain("LoopOver is holding this PR for maintainer review."); + // #6103: "heldNoSummaryComment" (repo: null, no gate object, reviewCheckMode required) exercised the + // SAME gate-row fallback as repoBlockedArgs above (both resolve the "neutral" conclusion via the + // `!args.repo` arm of fallbackGateConclusion) -- its only unique content was the legacy panelSummary's + // literal default text, which has no surviving equivalent, so it is not rewired separately here. - const duplicateAdvisoryComment = buildPublicPrIntelligenceComment({env: {}, - repo: directRepo, - pr: currentPr, - profile, - detection, - queueHealth, - collisions, - preflight, - settings: { ...repoSettings(directRepo.fullName), reviewCheckMode: "disabled" }, - }); - expect(duplicateAdvisoryComment).toContain("Same-issue duplicate risk found against #8."); - expect(duplicateAdvisoryComment).toContain("> | Related work | ⚠️ Same linked issue: #8 | Another open PR references the same linked issue. | Compare #8. |"); + const duplicateAdvisoryPanel = buildPublicPrPanelSignalRows({ repo: directRepo, pr: currentPr, profile, detection, queueHealth, collisions, preflight, settings: { ...repoSettings(directRepo.fullName), reviewCheckMode: "disabled" } }); + expect(duplicateAdvisoryPanel.rows.find((r) => r.key === "relatedWork")!.cells).toEqual([ + "Related work", "⚠️ Same linked issue: #8", "Another open PR references the same linked issue.", "Compare #8.", + ]); const scopedClusters: CollisionCluster[] = Array.from({ length: 12 }, (_, index) => ({ id: `scoped-${index}`, @@ -1528,7 +1414,8 @@ describe("signal coverage edge cases", () => { reason: "Titles share 2 meaningful terms.", items: [{ type: "issue", number: index + 100, title: `Related issue ${index}`, authorLogin: "reporter", labels: [], linkedIssues: [] }], })); - const scopedComment = buildPublicPrIntelligenceComment({env: {}, + const scopedArgs = { + env: {}, repo: directRepo, pr: { ...currentPr, linkedIssues: [99], body: "Fixes #99" }, profile, @@ -1545,10 +1432,14 @@ describe("signal coverage edge cases", () => { collisions: scopedClusters, findings: [], }, - settings: { ...repoSettings(directRepo.fullName), reviewCheckMode: "disabled" }, - }); - expect(scopedComment).toContain("> | Related work | ⚠️ 3 scoped overlaps | Top overlaps are listed below; lower-confidence bulk is hidden. | Review top overlaps. |"); - expect(scopedComment).toContain("Additional title-only matches omitted; title-only overlap does not block."); + settings: { ...repoSettings(directRepo.fullName), reviewCheckMode: "disabled" as const }, + }; + expect(buildPublicPrPanelSignalRows(scopedArgs).rows.find((r) => r.key === "relatedWork")!.cells).toEqual([ + "Related work", "⚠️ 3 scoped overlaps", "Top overlaps are listed below; lower-confidence bulk is hidden.", "Review top overlaps.", + ]); + expect(buildPublicSafeCollapsibles(scopedArgs).find((section) => section.title === "Review context")!.body).toContain( + "Additional title-only matches omitted; title-only overlap does not block.", + ); }); it("counts scoped related-work as the union of PR-specific and preflight clusters, not the max", () => { @@ -1580,7 +1471,8 @@ describe("signal coverage edge cases", () => { })); const profile = buildContributorProfile("dev", { login: "dev", topLanguages: [], source: "github" }, [currentPr], []); const detection = { detected: true, source: "github_cache" as const, reason: "cached contributor", priorPullRequests: 1, priorMergedPullRequests: 0, priorIssues: 0 }; - const comment = buildPublicPrIntelligenceComment({env: {}, + // #6103: rewired off the retired legacy renderer onto buildPublicPrPanelSignalRows' relatedWork row. + const panel = buildPublicPrPanelSignalRows({ repo: directRepo, pr: currentPr, profile, @@ -1596,8 +1488,8 @@ describe("signal coverage edge cases", () => { }); // PR-specific clusters = {pr-cluster} (1); preflight clusters = 2 disjoint -> 3 distinct overlaps. // Old code used Math.max(1, 2) = 2; the union (3) is the correct count feeding the related-work row. - expect(comment).toContain("3 scoped overlaps"); - expect(comment).not.toContain("2 scoped overlaps"); + expect(panel.rows.find((r) => r.key === "relatedWork")!.cells[1]).toContain("3 scoped overlaps"); + expect(panel.rows.find((r) => r.key === "relatedWork")!.cells[1]).not.toContain("2 scoped overlaps"); }); it("does not present global repo collision clusters as PR duplicate risk", () => { @@ -1619,7 +1511,8 @@ describe("signal coverage edge cases", () => { expect(collisions.summary.clusterCount).toBeGreaterThan(0); expect(preflight.collisions).toHaveLength(0); - const comment = buildPublicPrIntelligenceComment({env: {}, + // #6103: rewired off the retired legacy renderer onto buildPublicPrPanelSignalRows. + const panel = buildPublicPrPanelSignalRows({ repo: directRepo, pr: currentPr, profile: buildContributorProfile("dev", { login: "dev", topLanguages: ["Markdown"], source: "github" }, [], []), @@ -1630,43 +1523,23 @@ describe("signal coverage edge cases", () => { settings: { ...repoSettings(directRepo.fullName), reviewCheckMode: "required" }, }); - expect(comment).toContain("> | Related work | ✅ No active overlap found | No same-issue or scoped active PR overlap found. | No action. |"); - expect(comment).toContain("> | Gate result | ✅ Passing | No configured blocker found. | No action. |"); - expect(comment).not.toContain("possible overlap"); - expect(comment).not.toContain("12"); + expect(panel.rows.find((r) => r.key === "relatedWork")!.cells).toEqual([ + "Related work", "✅ No active overlap found", "No same-issue or scoped active PR overlap found.", "No action.", + ]); + expect(panel.rows.find((r) => r.key === "gateResult")!.cells).toEqual([ + "Gate result", "✅ Passing", "No configured blocker found.", "No action.", + ]); + // The global 12-issue collision noise must never leak into the PR-specific rows. + expect(JSON.stringify(panel.rows)).not.toContain("12"); }); - it("posts a minimal earn-invite (no readiness panel) for a non-registered contributor", () => { - const directRepo = repo("owner/invite"); - const currentPr = pr(directRepo.fullName, 42, "Add docs", { authorLogin: "newcomer", linkedIssues: [], body: "" }); - const collisions = buildCollisionReport(directRepo.fullName, [], [currentPr]); - const queueHealth = buildQueueHealth(directRepo, [], [currentPr], collisions); - const preflight = buildPreflightResult( - { repoFullName: directRepo.fullName, title: currentPr.title, body: currentPr.body ?? undefined, linkedIssues: currentPr.linkedIssues }, - directRepo, - [], - [currentPr], - ); - - const comment = buildPublicPrIntelligenceComment({env: {}, - repo: directRepo, - pr: currentPr, - profile: buildContributorProfile("newcomer", { login: "newcomer", topLanguages: [], source: "github" }, [], []), - detection: { detected: false, reason: "no gittensor footprint", priorPullRequests: 0, priorMergedPullRequests: 0, priorIssues: 0 }, - queueHealth, - collisions, - preflight, - settings: repoSettings(directRepo.fullName), - }); - - // Minimal: brief welcome + earn invite + the always-on footer CTA; NO readiness table. - expect(comment).toContain(""); - expect(comment).toContain("Thanks for the contribution"); - expect(comment).toMatch(/earn/i); - expect(comment).toContain("register to start earning"); - expect(comment).not.toContain("Readiness score"); - expect(comment).not.toContain("| Signal | Result | Evidence | Action |"); - }); + // #6103: "posts a minimal earn-invite (no readiness panel) for a non-registered contributor" exercised + // the deleted `buildMinimalInviteComment`'s own unique behavior (the "👋 Thanks for the contribution" + // welcome copy, and skipping the readiness table entirely when `detection.detected` is false). The + // converged path (src/queue/processors.ts) calls `buildPublicPrPanelSignalRows` unconditionally -- there + // is no minimal/full branch left to test -- and gates whether a comment posts at all on + // `settings.commentMode`/`publicSurface`, not on a per-comment minimal-vs-full rendering choice. No + // surviving equivalent to rewire this test onto in this file. it("covers PR panel edge formatting without publishing unconfirmed cache counts", () => { const directRepo = repo("owner/edge"); @@ -1678,7 +1551,11 @@ describe("signal coverage edge cases", () => { [], [currentPr], ); - const officialComment = buildPublicPrIntelligenceComment({env: {}, + // #6103: rewired off the retired legacy renderer onto buildPublicSafeCollapsibles' "Review context" + // body -- reviewContextBody (its own doc comment, packages/loopover-engine/src/signals/engine.ts) is + // still the SAME helper that fed this text in the legacy panel. + const officialReviewContext = buildPublicSafeCollapsibles({ + env: {}, repo: directRepo, pr: currentPr, profile, @@ -1687,9 +1564,8 @@ describe("signal coverage edge cases", () => { collisions: buildCollisionReport(directRepo.fullName, [], [currentPr]), preflight: basePreflight, settings: { ...repoSettings(directRepo.fullName), publicAudienceMode: "gittensor_only", reviewCheckMode: "disabled" }, - }); - expect(officialComment).toContain("Confirmed Gittensor contributor context was checked"); - expect(officialComment).toContain("Official Gittensor activity: 4 PR(s), 3 issue(s)."); + }).find((section) => section.title === "Review context")!.body; + expect(officialReviewContext).toContain("Official Gittensor activity: 4 PR(s), 3 issue(s)."); const selfItem = { type: "pull_request" as const, number: currentPr.number, title: currentPr.title, authorLogin: "dev", linkedIssues: currentPr.linkedIssues }; const edgeCollisions: CollisionReport = { @@ -1702,7 +1578,8 @@ describe("signal coverage edge cases", () => { { id: "recent-merged", risk: "medium", reason: "Recent merged work is related.", items: [selfItem, { type: "recent_merged_pull_request" as const, number: 11, title: "Merged edge fix" }] }, ], }; - const edgeComment = buildPublicPrIntelligenceComment({env: {}, + const edgeReviewContext = buildPublicSafeCollapsibles({ + env: {}, repo: directRepo, pr: currentPr, profile, @@ -1711,9 +1588,9 @@ describe("signal coverage edge cases", () => { collisions: edgeCollisions, preflight: basePreflight, settings: { ...repoSettings(directRepo.fullName), reviewCheckMode: "disabled" }, - }); - expect(edgeComment).toContain("Related work: Only this PR is present."); - expect(edgeComment).toContain("merged PR #11"); + }).find((section) => section.title === "Review context")!.body; + expect(edgeReviewContext).toContain("Related work: Only this PR is present."); + expect(edgeReviewContext).toContain("merged PR #11"); const bundle = buildPublicCommentSignalBundle({ repo: directRepo, @@ -1765,29 +1642,38 @@ describe("signal coverage edge cases", () => { buildCollisionReport(directRepo.fullName, [], []), ); const settings = { ...repoSettings(directRepo.fullName), reviewCheckMode: "required" as const, qualityGateMode: "block" as const, qualityGateMinScore: 95 }; - const comment = buildPublicPrIntelligenceComment({env: {}, + // #6103: rewired off the retired legacy renderer onto the shared row/collapsible builders. The re-run + // checkbox line ("- [ ] Re-run LoopOver review") is now rendered by + // the converged bridge/renderer (src/review/unified-comment.ts), not by any function in this file. + const args = { + env: {}, repo: directRepo, pr: currentPr, profile, - detection: { detected: true, source: "official_gittensor_api", reason: "official", priorPullRequests: 29, priorMergedPullRequests: 20, priorIssues: 6 }, + detection: { detected: true, source: "official_gittensor_api" as const, reason: "official", priorPullRequests: 29, priorMergedPullRequests: 20, priorIssues: 6 }, queueHealth, collisions: buildCollisionReport(directRepo.fullName, [], [currentPr]), preflight, settings, - gate: { conclusion: "skipped", summary: "PR closed before full evaluation." }, - }); - - expect(comment).toContain("> | Linked issue | ✅ No-issue rationale | PR body explains why no issue is linked. | No action. |"); - expect(comment).toContain("> | Change scope | ❌ 8/20 | High review scope from cached public metadata (size label size:L; draft PR; no linked issue context). | Add a concise scope and risk note. |"); - expect(comment).toContain("> | Validation posture | ❌ 5/25 | Preflight is holding this PR: the review lane is unavailable, so it is not ready for automated review. | Await review-lane availability. |"); - expect(comment).toContain("> | Contributor workload | ✅ 10/10 | Author activity: 29 registered-repo PR(s), 20 merged, 6 issue(s). | No action. |"); - expect(comment).toContain("> | Gate result | ⚠️ Not blocking | Advisory; not blocking this PR. | No action. |"); - expect(comment).toContain("[JSONbored](https://github.com/JSONbored)"); - expect(comment).toContain("[Gittensor profile](https://gittensor.io/miners/details?githubId=49853598)"); - expect(comment).toContain("Official Gittensor activity: 29 PR(s), 6 issue(s)."); - expect(comment).toContain("- [ ] Re-run LoopOver review"); - expect(comment).not.toContain("- [x] "); - expect(comment).not.toMatch(/wallet|hotkey|payout|trust score|private score/i); + }; + const panel = buildPublicPrPanelSignalRows({ ...args, gate: { conclusion: "skipped", summary: "PR closed before full evaluation." } }); + const cellsOf = (key: string) => panel.rows.find((r) => r.key === key)!.cells; + expect(cellsOf("linkedIssue")).toEqual(["Linked issue", "✅ No-issue rationale", "PR body explains why no issue is linked.", "No action."]); + expect(cellsOf("reviewLoad")).toEqual([ + "Change scope", "❌ 8/20", "High review scope from cached public metadata (size label size:L; draft PR; no linked issue context).", "Add a concise scope and risk note.", + ]); + expect(cellsOf("validationEvidence")).toEqual([ + "Validation posture", "❌ 5/25", "Preflight is holding this PR: the review lane is unavailable, so it is not ready for automated review.", "Await review-lane availability.", + ]); + expect(cellsOf("openPrQueue")).toEqual(["Contributor workload", "✅ 10/10", "Author activity: 29 registered-repo PR(s), 20 merged, 6 issue(s).", "No action."]); + expect(cellsOf("gateResult")).toEqual(["Gate result", "⚠️ Not blocking", "Advisory; not blocking this PR.", "No action."]); + expect(cellsOf("contributorContext")[2]).toContain("[JSONbored](https://github.com/JSONbored)"); + expect(cellsOf("contributorContext")[2]).toContain("[Gittensor profile](https://gittensor.io/miners/details?githubId=49853598)"); + expect(JSON.stringify(panel.rows)).not.toMatch(/wallet|hotkey|payout|trust score|private score/i); + + const reviewContext = buildPublicSafeCollapsibles(args).find((section) => section.title === "Review context")!.body; + expect(reviewContext).toContain("Official Gittensor activity: 29 PR(s), 6 issue(s)."); + expect(reviewContext).not.toMatch(/wallet|hotkey|payout|trust score|private score/i); }); it("uses contributor workload buckets for the visible queue row", () => { @@ -2126,7 +2012,16 @@ describe("signal coverage edge cases", () => { { code: "private_reward", severity: "warning" as const, title: "Reward wallet", detail: "wallet reward", action: "secret" }, ], }; - const comment = buildPublicPrIntelligenceComment({env: {}, + // #6103: rewired off the retired legacy renderer onto buildPublicSafeCollapsibles. "LoopOver PR + // readiness looks good" (panelTitle) and "- No public-safe advisory findings were generated from + // cached metadata." (the legacy Maintainer notes fallback) have no surviving equivalent -- Maintainer + // notes is excluded from buildPublicSafeCollapsibles entirely (its own doc comment). The fallback + // "Contributor next steps" copy, though, is still the shared `contributorNextStepsBody`'s own + // empty-nextSteps default, and both findings here are filtered out before reaching it: the + // missing_linked_issue finding by the linkedIssueGateMode: "off" filter, and the private_reward + // finding by the containsPrivatePublicTerm backstop (its detail contains "wallet"/"reward"). + const collapsibles = buildPublicSafeCollapsibles({ + env: {}, repo: directRepo, pr: currentPr, profile, @@ -2137,10 +2032,10 @@ describe("signal coverage edge cases", () => { settings: { ...repoSettings(directRepo.fullName), linkedIssueGateMode: "off" }, }); - expect(comment).toContain("LoopOver PR readiness looks good"); - expect(comment).toContain("- No public-safe advisory findings were generated from cached metadata."); - expect(comment).toContain("- Keep the PR focused and include validation evidence before maintainer review."); - expect(comment).not.toMatch(/No linked issue detected|reward|wallet/i); + expect(collapsibles.find((section) => section.title === "Contributor next steps")!.body).toContain( + "- Keep the PR focused and include validation evidence before maintainer review.", + ); + expect(JSON.stringify(collapsibles)).not.toMatch(/No linked issue detected|reward|wallet/i); }); it("audits label ordering and suspicious configured labels deterministically", () => { diff --git a/test/unit/signals-v2.test.ts b/test/unit/signals-v2.test.ts index 2e6aa21615..05a129634c 100644 --- a/test/unit/signals-v2.test.ts +++ b/test/unit/signals-v2.test.ts @@ -19,7 +19,7 @@ import { buildMaintainerLaneReport, buildMaintainerPacket, buildPreflightResult, - buildPublicPrIntelligenceComment, + buildPublicSafeCollapsibles, buildPullRequestMaintainerPacket, buildPullRequestReviewIntelligence, buildQueueHealth, @@ -2086,7 +2086,10 @@ describe("v2 signal builders", () => { ); expect(activeBounty).toMatchObject({ lifecycle: "active", fundingStatus: "funded", consensusRisk: "medium" }); - const comment = buildPublicPrIntelligenceComment({env: {}, + // #6103: rewired off the retired legacy renderer onto the shared collapsible builder both the legacy + // panel and the converged comment always drew this "Review context" body from. + const collapsibles = buildPublicSafeCollapsibles({ + env: {}, repo, pr: { ...pullRequests[0]!, authorLogin: undefined, linkedIssues: [] }, profile: noLanguageProfile, @@ -2127,9 +2130,10 @@ describe("v2 signal builders", () => { aiReviewAllAuthors: false, closeOwnerAuthors: false, }, }); - expect(comment).toContain("Author: `unknown`"); - expect(comment).toContain("Public profile only"); - expect(comment).not.toMatch(/wallet|raw trust score|ranking/i); + const reviewContext = collapsibles.find((section) => section.title === "Review context")!.body; + expect(reviewContext).toContain("Author: `unknown`"); + expect(reviewContext).toContain("Public profile only"); + expect(JSON.stringify(collapsibles)).not.toMatch(/wallet|raw trust score|ranking/i); }); }); diff --git a/test/unit/signals.test.ts b/test/unit/signals.test.ts index a4cc5d8780..aaf15b4c41 100644 --- a/test/unit/signals.test.ts +++ b/test/unit/signals.test.ts @@ -23,7 +23,6 @@ import { buildPublicCommentSignalBundle, buildPullRequestMaintainerPacket, buildPullRequestReviewIntelligence, - buildPublicPrIntelligenceComment, buildQueueHealth, buildRegistryChangeReport, buildRepoFitRecommendation, @@ -539,81 +538,14 @@ describe("world-class backend signals", () => { currentPr, priorPr, ], []); - const comment = buildPublicPrIntelligenceComment({env: {}, repo, pr: currentPr, profile, detection, queueHealth, collisions, preflight, settings }); - expect(detection.detected).toBe(true); expect(shouldPublishPrIntelligenceComment(settings, detection)).toBe(true); - expect(comment).toContain(""); - expect(comment).not.toMatch(/wallet|raw trust score|ranking|farming|reward/i); }); - it("scopes the earn-footer CTA to the repo miner page only when the repo is registered", () => { - const currentPr = pullRequests[0]!; - const settings = { - repoFullName: repo.fullName, - commentMode: "detected_contributors_only" as const, - publicAudienceMode: "gittensor_only" as const, - publicSignalLevel: "standard" as const, - checkRunMode: "off" as const, - checkRunDetailLevel: "minimal" as const, - regateSweepOrderMode: "staleness" as const, - reviewCheckMode: "disabled" as const, - gatePack: "gittensor" as const, - linkedIssueGateMode: "advisory" as const, - duplicatePrGateMode: "advisory" as const, - qualityGateMode: "advisory" as const, - slopGateMode: "off" as const, - mergeReadinessGateMode: "off" as const, - manifestPolicyGateMode: "off" as const, - selfAuthoredLinkedIssueGateMode: "advisory" as const, - linkedIssueSatisfactionGateMode: "off" as const, - firstTimeContributorGrace: false, - slopAiAdvisory: false, - qualityGateMinScore: null, - autoLabelEnabled: true, - gittensorLabel: "gittensor", - createMissingLabel: true, - publicSurface: "comment_and_label" as const, - includeMaintainerAuthors: false, - requireLinkedIssue: false, - backfillEnabled: true, - aiReviewMode: "off" as const, - aiReviewByok: false, - aiReviewAllAuthors: false, closeOwnerAuthors: false, - }; - const collisions = buildCollisionReport(repo.fullName, issues, pullRequests); - const queueHealth = buildQueueHealth(repo, issues, pullRequests, collisions); - const preflight = buildPreflightResult({ repoFullName: repo.fullName, title: currentPr.title, body: "Fixes #7", linkedIssues: [7] }, repo, issues, pullRequests); - const repoEarnPage = `${GITTENSOR_HOME_URL}/miners/repository?name=${encodeURIComponent(repo.fullName)}&tab=miners`; - const homeCta = `(${GITTENSOR_HOME_URL})`; - - // Detected contributor → full panel. Registered repo links the repo miner page; unregistered repo - // must NOT (the page has no miner data for an unregistered repo) and falls back to the home URL. - const priorPr: PullRequestRecord = { ...currentPr, number: 3, state: "closed", mergedAt: "2026-05-01T00:00:00.000Z" }; - const detected = { ...detectGittensorContributor("oktofeesh1", currentPr, [currentPr, priorPr], []), source: "official_gittensor_api" as const }; - const detectedProfile = buildContributorProfile("oktofeesh1", { login: "oktofeesh1", topLanguages: ["TypeScript"], source: "github" }, [currentPr, priorPr], []); - expect(detected.detected).toBe(true); - - const registeredComment = buildPublicPrIntelligenceComment({env: {}, repo, pr: currentPr, profile: detectedProfile, detection: detected, queueHealth, collisions, preflight, settings }); - expect(registeredComment).toContain(repoEarnPage); - - const unregisteredRepo = { ...repo, isRegistered: false, registryConfig: null }; - const unregisteredComment = buildPublicPrIntelligenceComment({env: {}, repo: unregisteredRepo, pr: currentPr, profile: detectedProfile, detection: detected, queueHealth, collisions, preflight, settings }); - expect(unregisteredComment).not.toContain("/miners/repository"); - expect(unregisteredComment).toContain(homeCta); - - // Non-detected contributor → minimal invite. Same registration gating must hold there. - const undetected = detectGittensorContributor("brand-new-outsider", currentPr, [], []); - const undetectedProfile = buildContributorProfile("brand-new-outsider", { login: "brand-new-outsider", topLanguages: [], source: "github" }, [], []); - expect(undetected.detected).toBe(false); - - const minimalRegistered = buildPublicPrIntelligenceComment({env: {}, repo, pr: currentPr, profile: undetectedProfile, detection: undetected, queueHealth, collisions, preflight, settings }); - expect(minimalRegistered).toContain(repoEarnPage); - - const minimalUnregistered = buildPublicPrIntelligenceComment({env: {}, repo: unregisteredRepo, pr: currentPr, profile: undetectedProfile, detection: undetected, queueHealth, collisions, preflight, settings }); - expect(minimalUnregistered).not.toContain("/miners/repository"); - expect(minimalUnregistered).toContain(homeCta); - }); + // #6103: the registered-vs-unregistered earn-CTA scoping this test used to exercise through the retired + // legacy renderer's own footerEarnUrl helper is the SAME logic the converged path applies inline + // (processors.ts's loopoverFooter(env, { earnUrl: repo?.isRegistered ? gittensorRepoEarnUrl(...) : ... })) + // -- already covered directly by test/unit/footer.test.ts. it("builds a compact, source-free public AI signal bundle", () => { const sourceMarker = "SECRET_SOURCE_LINE_should_never_reach_ai_provider"; @@ -885,53 +817,6 @@ describe("world-class backend signals", () => { expect(opportunities.find((opportunity) => opportunity.repoFullName === issueDiscoveryRepo.fullName)?.warnings).toContain("This repo is not a direct-PR-first lane."); }); - it("summarizes public comments at minimal signal level", () => { - const currentPr: PullRequestRecord = { ...pullRequests[0]!, linkedIssues: [], body: "" }; - const detection = { ...detectGittensorContributor("newbie", currentPr, [], []), detected: true, source: "official_gittensor_api" as const, reason: "Official Gittensor API confirms this GitHub user." }; - const collisions = buildCollisionReport(repo.fullName, issues, [currentPr]); - const queueHealth = buildQueueHealth(repo, issues, [currentPr], collisions); - const preflight = buildPreflightResult({ repoFullName: repo.fullName, title: currentPr.title, changedFiles: ["README.md"] }, repo, issues, [currentPr]); - const profile = buildContributorProfile("newbie", { login: "newbie", topLanguages: [], source: "unavailable" }, [], []); - const settings: RepositorySettings = { - repoFullName: repo.fullName, - commentMode: "all_prs", - publicAudienceMode: "gittensor_only", - publicSignalLevel: "minimal", - checkRunMode: "off", - checkRunDetailLevel: "minimal", - regateSweepOrderMode: "staleness", - reviewCheckMode: "disabled", - gatePack: "gittensor", - linkedIssueGateMode: "advisory", - duplicatePrGateMode: "advisory", - qualityGateMode: "advisory", - slopGateMode: "off", - mergeReadinessGateMode: "off", - manifestPolicyGateMode: "off", - selfAuthoredLinkedIssueGateMode: "advisory", - linkedIssueSatisfactionGateMode: "off", - firstTimeContributorGrace: false, - slopAiAdvisory: false, - qualityGateMinScore: null, - autoLabelEnabled: true, - gittensorLabel: "gittensor", - createMissingLabel: true, - publicSurface: "comment_and_label", - includeMaintainerAuthors: false, - requireLinkedIssue: false, - backfillEnabled: true, - aiReviewMode: "off" as const, - aiReviewByok: false, - aiReviewAllAuthors: false, closeOwnerAuthors: false, - }; - - const comment = buildPublicPrIntelligenceComment({env: {}, repo, pr: currentPr, profile, detection, queueHealth, collisions, preflight, settings }); - - expect(comment).toContain("| Linked issue | ⚠️ Missing | No linked issue or no-issue rationale found. | Explain no-issue PR. |"); - expect(comment).toContain("Public profile languages: not available"); - expect(comment).not.toMatch(/trust score|wallet|ranking/i); - }); - it("separates active and historical bounty lifecycle risk", () => { const active: BountyRecord = { id: "bounty-1", @@ -998,50 +883,13 @@ describe("world-class backend signals", () => { const currentPr: PullRequestRecord = { ...pullRequests[0]!, body: "Fixes #7", linkedIssues: [7] }; const publicPreflight = buildPreflightResult({ repoFullName: repo.fullName, title: currentPr.title, body: currentPr.body ?? undefined, linkedIssues: [7] }, repo, [openIssue], [], [completed]); - const publicComment = buildPublicPrIntelligenceComment({env: {}, - repo, - pr: currentPr, - profile: buildContributorProfile("oktofeesh1", { login: "oktofeesh1", topLanguages: ["TypeScript"], source: "github" }, [currentPr], []), - detection: { ...detectGittensorContributor("oktofeesh1", currentPr, [currentPr], []), detected: true, source: "official_gittensor_api", reason: "Official Gittensor API confirms this GitHub user." }, - queueHealth: buildQueueHealth(repo, [openIssue], [currentPr], buildCollisionReport(repo.fullName, [openIssue], [currentPr])), - collisions: buildCollisionReport(repo.fullName, [openIssue], [currentPr]), - preflight: publicPreflight, - settings: { - repoFullName: repo.fullName, - commentMode: "all_prs", - publicAudienceMode: "gittensor_only", - publicSignalLevel: "standard", - checkRunMode: "off", - checkRunDetailLevel: "minimal", - regateSweepOrderMode: "staleness", - reviewCheckMode: "disabled", - gatePack: "gittensor", - linkedIssueGateMode: "advisory", - duplicatePrGateMode: "advisory", - qualityGateMode: "advisory", - slopGateMode: "off", - mergeReadinessGateMode: "off", - manifestPolicyGateMode: "off", - selfAuthoredLinkedIssueGateMode: "advisory", - linkedIssueSatisfactionGateMode: "off", - firstTimeContributorGrace: false, - slopAiAdvisory: false, - qualityGateMinScore: null, - autoLabelEnabled: true, - gittensorLabel: "gittensor", - createMissingLabel: true, - publicSurface: "comment_and_label", - includeMaintainerAuthors: false, - requireLinkedIssue: false, - backfillEnabled: true, - aiReviewMode: "off", - aiReviewByok: false, - aiReviewAllAuthors: false, closeOwnerAuthors: false, - }, - }); + // #6103: bounty-lifecycle findings are private (isPrivateBountyLifecycleFinding), and the converged + // renderer's "Maintainer notes" section that would have surfaced them is deliberately excluded from + // buildPublicSafeCollapsibles entirely (see its own doc comment + the dedicated + // unified-comment-parity.test.ts assertion that "Maintainer notes" never appears in the public + // comment) -- a stronger guarantee than the retired legacy renderer's per-finding filter this test + // used to check. expect(publicPreflight.findings.map((finding) => finding.code)).toContain("linked_issue_bounty_historical"); - expect(publicComment).not.toContain("Linked issue bounty is historical"); - expect(publicComment).not.toContain("Issue #7 has a completed bounty"); }); it("includes linked PR validity when PR records are available", () => { diff --git a/test/unit/unified-comment-bridge.test.ts b/test/unit/unified-comment-bridge.test.ts index b10bc517dc..8f7d0cd0c9 100644 --- a/test/unit/unified-comment-bridge.test.ts +++ b/test/unit/unified-comment-bridge.test.ts @@ -1308,8 +1308,11 @@ describe("buildClosedUnifiedCommentBody (closed/skipped PR through the unified r // queue/GitHub-client harness. The focused unit coverage here (open + closed body, marker single-source, flag // gate, Nit scrub) asserts the bridge contract the processor relies on; the e2e wiring is a separate task. -describe("isUnifiedReviewCommentEnabled (flag-OFF selects the legacy path)", () => { - it("is OFF (legacy buildPublicPrIntelligenceComment path) when the flag is unset or falsy", () => { +// #6103: the flag this function reads no longer selects between two comment renderers (the legacy +// buildPublicPrIntelligenceComment path it used to gate was deleted, having no remaining production +// caller) -- kept functionally inert so an operator's existing deployment config setting it doesn't error. +describe("isUnifiedReviewCommentEnabled (pure flag parsing, now inert)", () => { + it("parses OFF when the flag is unset or falsy", () => { expect(isUnifiedReviewCommentEnabled({})).toBe(false); expect(isUnifiedReviewCommentEnabled({ LOOPOVER_REVIEW_UNIFIED_COMMENT: undefined })).toBe(false); expect(isUnifiedReviewCommentEnabled({ LOOPOVER_REVIEW_UNIFIED_COMMENT: "false" })).toBe(false); diff --git a/test/unit/unified-comment-parity.test.ts b/test/unit/unified-comment-parity.test.ts index f04907d0e2..c99fbc214b 100644 --- a/test/unit/unified-comment-parity.test.ts +++ b/test/unit/unified-comment-parity.test.ts @@ -3,7 +3,6 @@ import { buildCollisionReport, buildContributorProfile, buildPreflightResult, - buildPublicPrIntelligenceComment, buildPublicPrPanelSignalRows, buildPublicSafeCollapsibles, buildQueueHealth, @@ -104,7 +103,10 @@ function gate(over: Partial = {}): GateCheckEvaluation { }; } -describe("converged comment ↔ legacy panel parity (#unified-comment)", () => { +// #6103: the legacy buildPublicPrIntelligenceComment panel was retired (the converged renderer is the only +// comment path now) -- this file no longer tests parity BETWEEN two renderers, just the shared public-safe +// collapsible machinery (buildPublicSafeCollapsibles) the converged renderer alone consumes. +describe("converged comment public-safe collapsibles (#unified-comment)", () => { it("the flag-ON converged body carries the public-safe collapsibles and NEVER the private 'Maintainer notes'", () => { const { currentPr, detection, collisions, queueHealth, preflight, profile } = buildFixtures(); const aiReview = { notes: "Looks reasonable. Add a regression test for reconnect.", reviewerCount: 2 }; @@ -152,21 +154,9 @@ describe("converged comment ↔ legacy panel parity (#unified-comment)", () => { expect(JSON.stringify(collapsibles)).not.toMatch(/maintainer notes/i); }); - it("the public-safe collapsible bodies are byte-identical to the legacy panel's
bodies", () => { + it("the 'Contributor next steps' collapsible body is populated from the deduped next-steps list", () => { const { currentPr, detection, collisions, queueHealth, preflight, profile } = buildFixtures(); - const aiReview = { notes: "Looks reasonable. Add a regression test for reconnect.", reviewerCount: 2 }; - const legacy = buildPublicPrIntelligenceComment({env: {}, repo, pr: currentPr, profile, detection, queueHealth, collisions, preflight, settings, aiReview }); const collapsibles = buildPublicSafeCollapsibles({ repo, pr: currentPr, profile, detection, settings, collisions, preflight, queueHealth, env: {} }); - - // Each shared collapsible body's individual lines must appear verbatim in the legacy panel so the two - // renderers can never diverge on the public-safe content. - for (const section of collapsibles) { - for (const line of section.body.split("\n")) { - if (line.trim() === "") continue; - expect(legacy).toContain(line); - } - } - // The "Contributor next steps" body is single-sourced with the legacy panel's deduped next-steps list. const nextSteps = collapsibles.find((section) => section.title === "Contributor next steps")!; expect(nextSteps.body.length).toBeGreaterThan(0); }); @@ -204,12 +194,6 @@ describe("converged comment ↔ legacy panel parity (#unified-comment)", () => { }); }); - 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 }); - expect(legacy).toContain("Maintainer notes"); - }); - // #4589: the "Test coverage" collapsible reuses the already-computed manifest_missing_tests finding rather // than a second detection pass -- it only has real content when BOTH a gap exists AND the checkbox would // actually work for this repo, mirroring #4583's own "never mention a command that would bounce" principle.