refactor(queue): extract the public-comment merge-facts derivation from maybePublishPrPublicSurface

[glorydavid03023]

Closes #4607

Summary

maybePublishPrPublicSurface is 2,600+ lines, and #4607's stated cost is precise: "most cross-cutting review-pipeline concerns live as unnamed inline blocks in one function rather than named, independently-testable steps." This PR lifts one such block out as a named step, in the same incremental-slice style as the merged #4688 / #4695 / #4728 / #4752 / #5066.

New: derivePublicCommentMergeFacts (src/queue/processors.ts) — derives the five merge/disposition values the public PR comment renders (ciState, mergeStateLabel, mergeReadiness, heldForReview, neverClosed) from the live CI aggregate, the live merge-state refresh, the repo settings, and the changed files.

The seam is real rather than arbitrary: this function's return shape is buildUnifiedCommentBody's input contract for exactly those five fields. It is pure — the incr("loopover_gate_decisions_total", …) metrics emit that sat in the middle of the original block stays at the call site, because it reads the gate conclusion the derivation never sees.

Why these two flags matter

They exist so the comment agrees with the action the disposition planner will take:

• heldForReview — a clean, green PR whose diff touches a hard-guardrail path is held for owner review by planAgentMaintenanceActions, never auto-merged, so the comment must not headline "safe to merge" (#guarded-hold-comment). It uses the same shared isGuardrailHit the planner uses, not a second copy.
• neverClosed — the disposition never auto-closes a repo-owner or protected-automation PR, so a gate "close" verdict on one must headline "held", not "Closed" (#8/#9).

The testability win

This is the part that makes the block worth extracting. Today those five values are:

• unit-tested only as inputs to the renderer — test/unit/unified-comment-bridge.test.ts:506 hand-writes const mergeReadiness: MergeReadiness = { ciState: "passed", mergeStateLabel: "clean" } rather than deriving it; and
• exercised only transitively by the queue suites, which must stand up an entire webhook delivery to reach them.

That same test file notes at :1205 that testing maybePublishPrPublicSurface end-to-end "is net-new and entangled". After this change the derivation is directly table-testable.

One behavior had no test at all and is now pinned: an empty changed-file list holds for review rather than claiming safe-to-merge. isGuardrailHit treats "no known changed paths" as a hit — the file list may simply not have resolved, and a false "safe to merge" is the dangerous direction. I asserted the opposite first, and the code corrected me; the fail-safe is intentional and is now locked down.

Scope

• The PR title follows type(scope): short summary Conventional Commit format.
• This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
• This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
• I linked a currently open issue this PR resolves — see the closing reference at the top of this body.

Validation

• git diff --check
• npm run typecheck
• npm run test:coverage locally; codecov/patch requires >=99% coverage of the lines AND branches changed
• npm run engine-parity:drift-check
• npm run docs:drift-check
• npm run ui:openapi:check
• npm audit --audit-level=moderate
• New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

Patch coverage: 100% (13/13 executable changed lines, zero partial branches), measured by intersecting the lcov report against this diff's added lines.

No behavior change: the 882 tests in the existing queue suites (queue, queue-2…queue-5, queue-lifecycle-guards) pass unchanged. New suite test/unit/processors-public-comment-merge-facts.test.ts (13 tests) covers every branch of the extracted step: live-vs-stored merge-state fallback and the both-absent case; passed/failed/else ciState collapse; the failing-check projection with and without summary/detailsUrl (absent optionals must be omitted, never rendered as undefined); non-required red checks staying visible without turning the PR red; guardrail hit / miss / mixed / override-away; the empty-file-list fail-safe; and neverClosed for owner (case-insensitively), protected bot, ordinary contributor, missing login, and an owner-less repoFullName (the "" === "" trap that would otherwise make an author-less PR un-closable).

If any required check was skipped, explain why:

• None skipped.

Safety

• No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
• Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
• Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests.
• API/OpenAPI/MCP behavior is updated and tested where needed.
• UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks.
• Visible UI changes include a UI Evidence section. Not applicable here.
• Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs.

UI Evidence

Not applicable — no UI, frontend, docs, or extension surface is touched. The diff is one backend file plus one new unit-test file.

Notes

• Pure move: the derivation logic is unchanged line-for-line apart from being parameterised (pr.mergeableState/pr.authorLogin become explicit arguments), so this is a no-op at runtime.
• The settings parameter is narrowed to the two fields the derivation actually reads, so the step can be tested without constructing a whole RepositorySettings.

[superagent-security]

Superagent didn't find any vulnerabilities or security issues in this PR.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.07%. Comparing base (e615ec7) to head (9fbe7cb).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #5787   +/-   ##
=======================================
  Coverage   95.06%   95.07%           
=======================================
  Files         580      580           
  Lines       46156    46152    -4     
  Branches    14804    14800    -4     
=======================================
  Hits        43880    43880           
  Misses       1516     1516           
+ Partials      760      756    -4     

Flag	Coverage Δ
shard-1	43.39% <88.88%> (-0.54%)	⬇️
shard-2	35.96% <55.55%> (+0.19%)	⬆️
shard-3	32.25% <0.00%> (-0.02%)	⬇️
shard-4	32.94% <0.00%> (-0.01%)	⬇️
shard-5	31.36% <0.00%> (-0.39%)	⬇️
shard-6	45.00% <100.00%> (+0.37%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
src/queue/processors.ts	95.80% <100.00%> (+0.13%)	⬆️

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[loopover-orb]

Tip

🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩

✅ LoopOver review result - approve/merge recommended

_{Review updated: 2026-07-14 15:52:35 UTC}

2 files · 1 AI reviewer · no blockers · readiness 100/100 · CI green · clean

✅ Suggested Action - Approve/Merge

• safe to merge

Review summary
This is a pure extract-method refactor: the inline block computing mergeStateLabel/ciState/failingDetails/heldForReview/neverClosed inside maybePublishPrPublicSurface is lifted verbatim into a new exported function derivePublicCommentMergeFacts, with the call site destructuring the same five values. The logic is copied unchanged (same isGuardrailHit, same author/owner comparison, same optional-key spreading), and a new focused unit test file exercises each derived field including the empty-file fail-safe and empty-author edge cases. The PR closes #4607 as claimed and stays narrowly scoped to this one seam, matching the described incremental-slice pattern from prior merged PRs.

Nits — 4 non-blocking

• The 'magic numbers' 4607/8/9 flagged by the external brief are issue-tracker references embedded in comments, not real numeric literals affecting logic — not worth acting on.
• src/queue/processors.ts:1904-1966 — the doc comments on derivePublicCommentMergeFacts largely duplicate the ones removed from the call site; consider trimming the call-site comments further now that the source-of-truth lives in the function doc.
• The new test file doesn't cover the case where liveCi.failingDetails and nonRequiredFailingDetails are both non-empty simultaneously, though this is a minor gap given the two are independent projections tested separately.
• Consider a follow-up test asserting mergeReadiness shape when both failingDetails and nonRequiredFailingDetails are populated together, since buildUnifiedCommentBody consumes both simultaneously in production.

Signal	Result	Evidence
Code review	✅ No blockers	1 reviewer
Linked issue	✅ Linked	#4607
Related work	✅ No active overlap found	No same-issue or scoped active PR overlap found.
Change scope	✅ 20/20	Low review scope from cached public metadata (1 linked issue).
Validation posture	✅ 25/25	PR body includes validation/test evidence.
Contributor workload	✅ 10/10	Author activity: 273 registered-repo PR(s), 165 merged, 20 issue(s).
Contributor context	✅ Confirmed Gittensor contributor	glorydavid03023; Gittensor profile; 273 PR(s), 20 issue(s).
Gate result	✅ Passing	No configured blocker found.
Improvement	✅ Minor	risk: clean · value: minor — Code changes are accompanied by test evidence. LLM value judgment: moderate — A faithful, well-tested extraction of a previously untestable inline block into a named pure function, directly advancing the stated #4607 goal of decomposing the mega-function without touching runtime behavior.

Linked issue satisfaction

Partially addressed
This PR performs exactly the kind of pure, named extraction #4607 asks for (derivePublicCommentMergeFacts is well-tested and reduces one inline block in maybePublishPrPublicSurface), but it only addresses a small slice of the 2,600+ line function and does not touch processGitHubWebhook or runAgentMaintenanceActions, both explicitly named in the issue's acceptance criteria.

Review context

• Author: glorydavid03023
• Role context: outside_contributor
• Public audience mode: oss maintainer
• Lane context: Repository is configured for direct PR review.
• Public profile languages: not available
• Official Gittensor activity: 273 PR(s), 20 issue(s).
• PR-specific overlap: none found.

Contributor next steps

• Keep the PR focused and include validation evidence before maintainer review.

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.

[BETA] Chat with Gittensory

Ask Gittensory a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

• @gittensory ask <question> answers contribution-quality Q&A with source citations and freshness.
• @gittensory chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
• A plain-language @gittensory mention with a real question is routed to the closest matching read-only command automatically -- no exact syntax required.

Full command reference: https://gittensory.aethereal.dev/docs/gittensory-commands

_{🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed}

---

💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

• Re-run LoopOver review

[loopover-orb]

Gittensory approves — the gate is satisfied and CI is green.