Skip to content

test(scoring): cover pending-pr helpers' ghost-login exclusion and case-insensitive matching - #8493

Closed
jeffrey701 wants to merge 1 commit into
JSONbored:mainfrom
jeffrey701:test-8329-pending-pr-helpers-coverage-v2
Closed

test(scoring): cover pending-pr helpers' ghost-login exclusion and case-insensitive matching#8493
jeffrey701 wants to merge 1 commit into
JSONbored:mainfrom
jeffrey701:test-8329-pending-pr-helpers-coverage-v2

Conversation

@jeffrey701

Copy link
Copy Markdown
Contributor

What

src/scoring/pending-pr-scenarios.ts's two filter helpers had a real branch-coverage gap on
the logic that decides which of a contributor's open PRs count toward their pending-PR signals:

  • sameLogin's value && short-circuit — which exists to exclude ghost/deleted GitHub
    accounts whose authorLogin is null/undefined (PullRequestRecord.authorLogin is typed
    string | null | undefined) — had no test proving the falsy-value side (a null login is
    excluded, not matched or thrown).
  • The .toLowerCase() case-normalization in both sameLogin and sameRepoFullName — GitHub
    owner/repo and login are case-insensitive but stored values aren't guaranteed consistent —
    had no test proving a case-only difference still matches.

Every existing test set authorLogin to a concrete same-case string, so neither branch was
exercised.

Change

Pure test addition (no source change) in test/unit/pending-pr-scenarios.test.ts: one case
feeding loadContributorRepoOpenPrSignalRecords four PRs —

  • authorLogin: null and authorLogin: undefined for the searched login → both excluded
    (covers sameLogin's falsy-value short-circuit);
  • a repoFullName differing only in case → included (covers sameRepoFullName's
    case-normalization);
  • an authorLogin differing only in case → included (covers sameLogin's truthy side +
    case-normalization).

The per-PR review mock lets the test assert exactly which pull numbers survive the filter
([82, 83]), pinning the include/exclude outcome for each branch.

Validation

  • npx vitest run test/unit/pending-pr-scenarios.test.ts → all pass.
  • Full npm run test:changed net green (modulo the pre-existing Windows-only
    subprocess/symlink baseline failures that pass on Linux CI).

Closes #8329

@jeffrey701
jeffrey701 requested a review from JSONbored as a code owner July 24, 2026 15:24
@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 15:37:34 UTC

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

🛑 Suggested Action - Fix Blockers

Review summary
This is a pure test-only addition that adds one new test case to pending-pr-scenarios.test.ts, covering two previously-untested branches: sameLogin's short-circuit on null/undefined authorLogin (ghost/deleted accounts) and the case-insensitive matching in both sameLogin and sameRepoFullName. The test correctly seeds four PRs, mocks listPullRequestReviews per-pull-number so it can assert on distinct pull numbers, and asserts the exact survivor set ([82, 83]) which pins both the exclude and include outcomes precisely. No source code is touched, and the test construction (using the existing pr() and approvedReview() helpers) is consistent with the file's existing conventions.

Nits — 4 non-blocking
  • The two FAILED checks (validate, validate-tests-merge) have no detail provided, and this branch is 2 commits behind default — per BASE BRANCH STATUS this is more likely stale-branch drift than a defect in this diff; worth rebasing to confirm.
  • The inline comments on each `pr({...})` line partially duplicate the block comment above the test (test/unit/pending-pr-scenarios.test.ts:466-476); could trim one for conciseness.
  • Consider asserting on `records.pullRequestReviews.length` explicitly (in addition to the sorted pull-number array) to make the exclusion count immediately visible without needing to count array elements.
  • If the underlying `loadContributorRepoOpenPrSignalRecords` also filters checks by the same sameLogin/sameRepoFullName logic, consider seeding `listCheckSummaries` similarly (rather than only `[]`) to double-cover the filter via the checks path too, though this is optional given reviews already exercise it.

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 #8329
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: minor
Linked issue satisfaction

Addressed
The diff adds a single test that exercises loadContributorRepoOpenPrSignalRecords with null and undefined authorLogin (proving sameLogin's falsy short-circuit excludes them) and with case-differing repoFullName and authorLogin (proving both helpers' case-insensitive matching), asserting exactly the expected included pull numbers as the issue requested.

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 pending-pr-scenarios.ts's null-login and case-insensitive matching branches

1 participant