review: remove the legacy comment renderer (#6103) - #6128
Merged
Conversation
…Comment) 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
…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.
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6128 +/- ##
=======================================
Coverage 95.31% 95.31%
=======================================
Files 595 595
Lines 47133 47046 -87
Branches 15038 14993 -45
=======================================
- Hits 44925 44843 -82
Misses 1477 1477
+ Partials 731 726 -5
Flags with carried forward coverage won't be shown. Click here to find out more.
|
loopover-orb
Bot
added
the
gittensor:bug
6 tasks
andriypolanski
pushed a commit
to andriypolanski/gittensory
that referenced
this pull request
Jul 15, 2026
…follow-up) The unified-comment bridge became the sole PR-comment renderer in JSONbored#6128, which left the unifiedComment ConvergedFeatureKey and its LOOPOVER_REVIEW_UNIFIED_COMMENT env flag declared but functionally inert. Since the drift-check gate requires every declared LOOPOVER_REVIEW_* flag to be documented, and there was no working precedent for a retired-but-still-required flag, this fully removes the key/flag from the resolver, config-as-code examples, self-hosting docs, and env declarations (regenerating worker-configuration.d.ts via cf-typegen) rather than leaving stale references. Also fixes the "signal table" description in docs.how-reviews-work.tsx, left over from the earlier decision-drivers redesign (JSONbored#6083), and a timing-flaky GraphQL single-flight test unrelated to this change but caught by the same full-suite run (crypto.subtle.digest is backed by a real OS thread pool, so two concurrent callers with identical inputs can complete in either order under load; stubbed with a deterministic pure-microtask digest for the one test that exercises the race).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
buildPublicPrIntelligenceCommentandbuildMinimalInviteCommentfrompackages/loopover-engine/src/signals/engine.ts— the converged renderer (renderUnifiedReviewComment/buildUnifiedCommentBody) has been the only comment-rendering path reachable from the queue processor since the earlier convergence work, and this was the last dead call site.buildSamplePreviewCommentinsrc/signals/settings-preview.ts) — onto the converged renderer, synthesizing a sample gate evaluation with the same enabled/hard-linked-issue-block heuristic the legacy renderer used internally.unifiedCommentAllowedbranch insrc/queue/processors.tsinto an unconditional call, falling back to a synthesized "skipped" gate evaluation (mirroring the existingbuildClosedUnifiedCommentBodypattern) when no real gate evaluation ran.LOOPOVER_REVIEW_UNIFIED_COMMENTas functionally inert insrc/env.d.ts(kept only so an operator's existing deployment config doesn't error).buildPublicPrPanelSignalRows,buildPublicSafeCollapsibles,publicSafePreflightFindings, gate-row helpers,contributorNextStepsBody, and the sanitizer boundary.buildSamplePreviewComment's new simulated-gate branches (enabled/disabled, linked-issue block pass/fail), which had no prior test.Test plan
npx tsc --noEmitcleannpm run test:coverage(unsharded): 882 files / 16993 tests passed, 0 failuresnpm audit --audit-level=moderate: 0 vulnerabilitiesnpm run test:ci(full local gate incl. drift checks, engine parity, MCP/miner packs, UI build): greenorigin/main, no conflicts