Skip to content

test(signals): cover decidePublicSurface's oss_maintainer + not_checked label fallback#8495

Closed
jeffrey701 wants to merge 1 commit into
JSONbored:mainfrom
jeffrey701:test-8324-decidepublicsurface-oss-maintainer-fallback
Closed

test(signals): cover decidePublicSurface's oss_maintainer + not_checked label fallback#8495
jeffrey701 wants to merge 1 commit into
JSONbored:mainfrom
jeffrey701:test-8324-decidepublicsurface-oss-maintainer-fallback

Conversation

@jeffrey701

Copy link
Copy Markdown
Contributor

What

decidePublicSurface (src/signals/settings-preview.ts) documents itself as the "single source of
truth … the preview can never drift from real behavior." Its willLabel computation (line ~149) has
an inline second disjunct handling publicAudienceMode: "oss_maintainer" + minerStatus: "not_checked"
— a real webhook combination — that duplicates shouldApplyPrLabel's own autoLabelEnabled && (label surface) logic while bypassing that function's oss_maintainer && !confirmed → false guard (line 39).

That branch had zero test coverage (grep "not_checked" test/unit/settings-preview.test.ts returned
nothing), so a future change to shouldApplyPrLabel could silently diverge from this inline copy with
no test catching it.

Change

Pure test addition (no production change) in test/unit/settings-preview.test.ts. Since for
oss_maintainer + not_checked the first disjunct (shouldApplyPrLabel) always returns false, the
inline fallback alone decides willLabel — so the new test exercises every operand of its && chain:

  • autoLabelEnabled: true + publicSurface: "comment_and_label"willLabel (label action) included.
  • autoLabelEnabled: true + publicSurface: "label_only" → included.
  • autoLabelEnabled: true + publicSurface: "comment_only"not included (the disjunct's own
    publicSurface check excludes it).
  • autoLabelEnabled: falsenot included (falls back to shouldApplyPrLabel's own false).

No divergence from shouldApplyPrLabel's intent was found while writing these (the inline condition
mirrors line 40 exactly), so nothing is "fixed" here per the issue's guidance.

Validation

  • npx vitest run test/unit/settings-preview.test.ts → all pass.
  • Every branch of the willLabel ||/&& chain now covered (verified via lcov — all BRDA arms on the
    disjunct non-zero).
  • Full npm run test:changed net green (modulo the pre-existing Windows-only baseline failures that
    pass on Linux CI).

Closes #8324

@jeffrey701
jeffrey701 requested a review from JSONbored as a code owner July 24, 2026 15:31
@superagent-security

Copy link
Copy Markdown
Contributor

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

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 24, 2026
@loopover-orb

loopover-orb Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Caution

🛑 LoopOver review result - fixes required

Review updated: 2026-07-24 16:07:17 UTC

1 file · 1 AI reviewer · no blockers · CI failing · blocked

🛑 Suggested Action - Fix Blockers

Review summary
This is a pure test-addition PR closing #8324, adding coverage for the oss_maintainer + not_checked fallback branch in decidePublicSurface's willLabel computation. The test correctly isolates the inline disjunct (since shouldApplyPrLabel always returns false for this combo) and exercises all four operand combinations of the && chain (autoLabelEnabled × publicSurface). The assertions are well-reasoned and match the described branch logic; no production code is touched, so risk is minimal.

Nits — 4 non-blocking
  • The test relies on comment-based reasoning ('shouldApplyPrLabel always returns false for this combo') rather than asserting that fact directly, e.g. via a companion check on shouldApplyPrLabel's own output for the same inputs, which would make the isolation claim self-verifying rather than doc-only.
  • The FAILED checks (validate-tests-merge, validate) have no detail provided and this branch is 17 commits behind the default branch, so the failures are most plausibly explained by that drift rather than this diff — worth rebasing to confirm before treating them as caused by this change.
  • Consider adding one assertion that directly calls shouldApplyPrLabel with the same oss_maintainer/not_checked inputs and confirms it returns false, making the test's core premise self-documenting instead of relying on a comment citing settings-preview.ts:39/149.
  • Rebase onto the current default branch (17 commits behind) to see whether validate/validate-tests-merge failures clear, since an undetailed failure on a stale branch is a common false signal.

CI checks failing

  • validate
  • validate-tests-merge

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #8324
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: 66 registered-repo PR(s), 31 merged, 17 issue(s).
Contributor context ✅ Confirmed Gittensor contributor jeffrey701; Gittensor profile; 66 PR(s), 17 issue(s).
Improvement ℹ️ Insufficient signal risk: clean · value: insufficient-signal · LLM: moderate
Linked issue satisfaction

Addressed
The PR adds exactly the four test cases the issue requested, exercising each operand of the oss_maintainer + not_checked fallback's && chain (autoLabelEnabled true/false, comment_and_label/label_only/comment_only surfaces) and asserting willLabel/actions accordingly, matching the issue's stated scope as pure test coverage with no production change.

Review context
  • Author: jeffrey701
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: Rust, TypeScript, Clojure, JavaScript
  • Official Gittensor activity: 66 PR(s), 17 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Triage stale or unlinked PRs.
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.
🧪 Chat with LoopOver

Ask 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.

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

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.

  • Re-run LoopOver review

@loopover-orb

loopover-orb Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

LoopOver is closing this pull request on the maintainer's behalf (CI is failing (validate, validate-tests-merge)). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed.

@loopover-orb loopover-orb Bot closed this Jul 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add test coverage for decidePublicSurface's oss_maintainer + not_checked auto-label fallback

1 participant