fix(signals): make shouldPublishPrIntelligenceComment respect commentMode for oss_maintainer (#6776)#6853
Conversation
…Mode for oss_maintainer (JSONbored#6776) The oss_maintainer branch returned `commentMode === "all_prs" || detection.detected || detection.source !== "official_gittensor_api"`. The third disjunct is a tautology: `detection.source` is only ever "official_gittensor_api" (paired with detected: true) or undefined, so it is true whenever `detected` is false — making the whole branch unconditionally true. For every oss_maintainer-audience repo, `commentMode: "detected_contributors_only"` silently behaved like `"all_prs"`, defeating the gate. Drops the tautological disjunct so the branch gates on commentMode + detection, matching the documented single source of truth (`shouldPublishPrComment`). Adds the previously-absent oss_maintainer branch tests, including the regression: detected_contributors_only + an undetected contributor now returns false. Closes JSONbored#6776
|
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 #6853 +/- ##
=======================================
Coverage 93.63% 93.64%
=======================================
Files 683 683
Lines 68163 68163
Branches 18706 18706
=======================================
+ Hits 63827 63828 +1
Misses 3350 3350
+ Partials 986 985 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-17 08:45:52 UTC
Review summary Nits — 4 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 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.
|
Summary
shouldPublishPrIntelligenceComment(packages/loopover-engine/src/signals/engine.ts) gated theoss_maintaineraudience on:The third disjunct is a tautology.
detection.sourceis only ever"official_gittensor_api"(alwayspaired with
detected: true) orundefined— no construction path producesdetected: falsewithsource: "official_gittensor_api". Sodetection.source !== "official_gittensor_api"istrueexactly whendetectedisfalse, making the whole expression unconditionally true. For everyoss_maintainer-audiencerepo,
commentMode: "detected_contributors_only"silently behaved identically to"all_prs", defeating the gate.This drops the tautological disjunct:
so the branch now respects
commentMode(matching the documented single source of truth,shouldPublishPrCommentinsrc/signals/settings-preview.ts, which gates oncommentModewith no audiencecarve-out). The function has zero production callers today — it's a superseded duplicate — so this only removes a
latent trap for a future caller; live behavior is unchanged.
Adds the previously-absent
oss_maintainer-branch test coverage, including the two regression cases the issuecalls for.
Closes #6776
Scope
type(scope): short summaryConventional Commit format.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #6776).Validation
git diff --checknpm run typechecknpm run test:coveragelocally — the one changed executable line (theoss_maintainerreturn) is exercised on both branches (commentMode: all_prsshort-circuit true;detected_contributors_onlywith an undetected contributor → false; with a detected contributor → true), verified viacoverage-final.json.npx vitest run test/unit/signals.test.ts test/unit/signals-coverage.test.ts(92 passing)build+node --test(588 passing) +engine-parity:drift-checkIf any required check was skipped, explain why:
ui:*/ui:openapi:check/cf-typegen/ migration checks are N/A to this diff.Safety
UI Evidencesection is required.Notes
shouldPublishPrIntelligenceCommenthas zero production callers insrc/(verified), so this is behavior-preserving for the live webhook/preview paths — it removes a tautology that would otherwise mislead any future caller relying oncommentModefor anoss_maintainerrepo.