From 061bec7bc11a43793f98ce56ba5d71f257cab02b Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 15 Jul 2026 03:43:27 -0700 Subject: [PATCH 1/2] review: remove the legacy comment renderer (buildPublicPrIntelligenceComment) The legacy renderer had two production callers: the webhook/queue comment-publish path (already unreachable -- no cloud/hosted deployment processes review webhooks) and, less obviously, the maintainer dashboard's Settings Preview feature (a live HTTP API endpoint, unconditional, no flag gating). Migrated the latter onto the converged renderer first (buildSamplePreviewComment now calls buildPublicPrPanelSignalRows + buildUnifiedCommentBody, with a synthesized sample gate verdict mirroring the same enabled/hard-linked-issue-block heuristic the legacy renderer used internally), which brought buildPublicPrIntelligenceComment to zero remaining callers. Deleted from packages/loopover-engine/src/signals/engine.ts: buildPublicPrIntelligenceComment, buildMinimalInviteComment, and their private-only helpers (footerEarnUrl, formatAlertBlock, aiReviewMainHasBlockers, escapeAiReviewMarkdown, escapeTableCell) -- ~300 lines of genuinely legacy-exclusive code. Every shared helper the two functions called (buildPublicPrPanelSignalRows, buildPublicSafeCollapsibles, publicSafePreflightFindings, etc.) stays, since the converged renderer depends on them too. Removed the now-dead webhook/queue branch in processors.ts (unifiedCommentAllowed && gateEvaluation conditional + its else fallback) -- the converged renderer now runs unconditionally. Handled the one real behavioral edge case this surfaced: a repo with no gate evaluation (reviewCheckMode disabled + no autonomy configured) used to silently fall through to the legacy renderer; it now gets a synthetic "skipped" gate for rendering purposes only (mirroring buildClosedUnifiedCommentBody's existing pattern for the same situation), scoped to the comment body -- the real, possibly-undefined gateEvaluation is untouched everywhere else (check-run/label/audit/disposition lanes). Fixed the 5 test files that called the legacy function directly: rewired tests exercising still-shared logic onto buildPublicPrPanelSignalRows/buildPublicSafeCollapsibles directly (same assertion intent, now via structured row/collapsible data instead of scraping rendered markdown); deleted tests that were purely exercising the deleted function's own unique formatting (panelTitle/ alert derivation, the 4-column table template, buildMinimalInviteComment's welcome copy), each left with an explanatory comment citing why there's no surviving equivalent to rewire onto. Verified: full unsharded npm run test:coverage (16,822 passed, 12 skipped, 0 failed), engine-parity + manifest/docs drift checks all clean. Closes #6103 --- .../loopover-engine/src/signals/engine.ts | 337 +------------ src/env.d.ts | 7 +- src/queue/processors.ts | 73 ++- src/review/unified-comment-bridge.ts | 31 +- src/signals/settings-preview.ts | 44 +- test/unit/queue-2.test.ts | 18 +- test/unit/queue-4.test.ts | 19 +- test/unit/queue-5.test.ts | 12 +- test/unit/settings-preview.test.ts | 3 +- test/unit/signals-coverage.test.ts | 471 +++++++----------- test/unit/signals-v2.test.ts | 14 +- test/unit/signals.test.ts | 172 +------ test/unit/unified-comment-bridge.test.ts | 7 +- test/unit/unified-comment-parity.test.ts | 26 +- 14 files changed, 345 insertions(+), 889 deletions(-) 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..d16628db2a 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,8 @@ 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: the converged renderer shows readiness as a `readiness N/100` status chip, not "Readiness score:" prose. + expect(finalComment).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 +1306,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 +1404,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 +1479,8 @@ 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: the converged renderer shows readiness as a `readiness N/100` status chip, not "Readiness score:" prose. + expect(finalComment).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..e918796102 100644 --- a/test/unit/queue-4.test.ts +++ b/test/unit/queue-4.test.ts @@ -1855,9 +1855,13 @@ 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: the converged renderer shows readiness as a `readiness N/100` status chip, not "Readiness score:" prose. + expect(patchedBody).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 +2712,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..0748203055 100644 --- a/test/unit/settings-preview.test.ts +++ b/test/unit/settings-preview.test.ts @@ -449,7 +449,8 @@ 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); }); 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. From cbca389df1e1db5a5eb091e4f4a3ca6ce6a20726 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 15 Jul 2026 04:13:01 -0700 Subject: [PATCH 2/2] test(review): fix readiness-chip assertions and cover simulated gate branches (#6103) Post-removal fixes surfaced by the full unsharded suite: three tests asserted a `readiness N/100` chip that the converged renderer correctly omits when CI is still pending (status downgrades to held) or when the overall verdict isn't "ready", per the #6066 readiness-chip rule. Also adds coverage for buildSamplePreviewComment's simulated gate-enabled and linked-issue-block branches, which had no prior test exercising them. --- test/unit/queue-2.test.ts | 14 ++++++++++---- test/unit/queue-4.test.ts | 10 ++++++++-- test/unit/settings-preview.test.ts | 20 ++++++++++++++++++++ 3 files changed, 38 insertions(+), 6 deletions(-) diff --git a/test/unit/queue-2.test.ts b/test/unit/queue-2.test.ts index d16628db2a..4be213131c 100644 --- a/test/unit/queue-2.test.ts +++ b/test/unit/queue-2.test.ts @@ -1229,8 +1229,11 @@ describe("queue processors", () => { expect(commentBodies[0]).toContain("🟪"); const finalComment = commentBodies.find((body) => !body.includes("is reviewing")); expect(finalComment).toBeDefined(); - // #6103: the converged renderer shows readiness as a `readiness N/100` status chip, not "Readiness score:" prose. - expect(finalComment).toMatch(/`readiness \d+\/100`/); + // #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."); @@ -1479,8 +1482,11 @@ describe("queue processors", () => { expect(aiRun).not.toHaveBeenCalled(); expect(commentBodies.length).toBeGreaterThanOrEqual(2); const finalComment = commentBodies.find((body) => !body.includes("is reviewing")); - // #6103: the converged renderer shows readiness as a `readiness N/100` status chip, not "Readiness score:" prose. - expect(finalComment).toMatch(/`readiness \d+\/100`/); + // #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 e918796102..a1a913a441 100644 --- a/test/unit/queue-4.test.ts +++ b/test/unit/queue-4.test.ts @@ -1860,8 +1860,14 @@ describe("queue processors", () => { // 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(""); - // #6103: the converged renderer shows readiness as a `readiness N/100` status chip, not "Readiness score:" prose. - expect(patchedBody).toMatch(/`readiness \d+\/100`/); + // #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 = ?") diff --git a/test/unit/settings-preview.test.ts b/test/unit/settings-preview.test.ts index 0748203055..9daeb8e496 100644 --- a/test/unit/settings-preview.test.ts +++ b/test/unit/settings-preview.test.ts @@ -454,6 +454,26 @@ describe("buildRepoSettingsPreview", () => { 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,