Skip to content

fix(signals): group contributor open PRs by repo case-insensitively - #1527

Merged
JSONbored merged 4 commits into
JSONbored:mainfrom
galuis116:fix/open-pr-monitor-repo-case
Jun 28, 2026
Merged

fix(signals): group contributor open PRs by repo case-insensitively#1527
JSONbored merged 4 commits into
JSONbored:mainfrom
galuis116:fix/open-pr-monitor-repo-case

Conversation

@galuis116

Copy link
Copy Markdown
Contributor

Summary

Fixes #1526.

groupByRepo in buildContributorOpenPrMonitor bucketed a contributor's open PRs by the raw pr.repoFullName, while every other repo-name operation in the same function is case-insensitive (the registered-repo set, the open-PR filter, the repo lookup). GitHub repo names are case-insensitive, so case-variant repoFullName values for one repo split into separate groups — under-counting per-repo open PRs, running duplicate detection only within each partial group, and querying listPullRequests once per casing.

What changed

  • groupByRepo now keys buckets on pr.repoFullName.toLowerCase(), matching the function's own repo-name handling.
  • The consumer loop derives the canonical repoFullName from a bucket's first PR (its original casing) for the case-sensitive per-repo DB queries — the established key-by-lower / keep-original pattern used in contributor-evidence-graph.ts.

Scope

Validation

  • git diff --check
  • npm run typecheck
  • npm run test:coverage locally (4619 passed); the new grouping lines are covered by a case-variant test, and the existing same-casing tests are unchanged.
  • npm run test:workers
  • npm audit --audit-level=moderate
  • New unit test: two open PRs for one repo under different casings group together and the case-sensitive query never runs against the case-variant.

If any required check was skipped, explain why:

  • OpenAPI/types/migrations unchanged — internal grouping only; the monitor's output shape is identical.

Safety

  • No secrets, wallets, hotkeys, PATs, trust scores, or private evidence exposed.
  • Public text sanitized and low-noise (monitor output sanitizer test still passes).
  • No auth/CORS/session change.
  • API/OpenAPI/MCP behavior unchanged.
  • No UI changes.
  • No changelog edit.

Notes

  • Confined to src/signals/contributor-open-pr-monitor.ts; mirrors the case-insensitive repo handling already present in the same function and in contributor-evidence-graph.ts.

@galuis116
galuis116 requested a review from JSONbored as a code owner June 26, 2026 13:22
@dosubot dosubot Bot added the size:S label Jun 26, 2026
@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jun 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.54%. Comparing base (758e524) to head (d20b607).
⚠️ Report is 42 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1527   +/-   ##
=======================================
  Coverage   95.53%   95.54%           
=======================================
  Files         204      204           
  Lines       22084    22086    +2     
  Branches     7972     7972           
=======================================
+ Hits        21097    21101    +4     
  Misses        412      412           
+ Partials      575      573    -2     
Files with missing lines Coverage Δ
src/signals/contributor-open-pr-monitor.ts 98.11% <100.00%> (+1.95%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JSONbored JSONbored added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jun 26, 2026
@loopover-orb

loopover-orb Bot commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Tip

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

✅ Gittensory review — safe to merge

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

✅ Approved — safe to merge

Review summary
The `groupByRepo` case-fold (`pr.repoFullName.toLowerCase()`) is exactly the right fix: it collapses case-variant entries for one repo into a single bucket, matching every other case-insensitive repo-name operation in `buildContributorOpenPrMonitor`. The consumer loop recovers original casing from `repoOpen[0]!.repoFullName`, mirroring the key-by-lower/keep-original pattern from `contributor-evidence-graph.ts`. The new integration test drives the real `buildContributorOpenPrMonitor` path through mocked repositories and correctly asserts both grouping and query-casing behaviour. No blockers.

Signal Result Evidence
Code review ✅ No blockers 1 reviewers, synthesized
Linked issue ✅ Linked #1526
Related work ⚠️ 2 scoped overlaps Top overlaps are listed below; lower-confidence bulk is hidden.
Review load ❌ 8/20 Readiness component derived from cached public PR metadata and labels; size label size:S.
Validation evidence ❌ 5/25 Cached preflight status is hold.
Open PR queue ❌ 3/10 28 open PR(s), 14 likely reviewable, 14 unlinked.
Contributor context ✅ Confirmed Gittensor contributor galuis116; Gittensor profile; 1266 PR(s), 156 issue(s).
Gate result ✅ Passing No configured blocker found.
Nits — 5 non-blocking
  • src/signals/contributor-open-pr-monitor.ts:61 — `repoOpen[0]!.repoFullName` as the canonical name for `listPullRequests` and `loadContributorRepoOpenPrSignals` is order-dependent: if the first PR in the bucket carries the atypical casing (`Entrius/Allways-UI`), case-sensitive DB queries use that casing and can return a partial result set for `detectPendingPrScenario`; prefer `repo?.fullName ?? repoOpen[0]!.repoFullName` (restructure so `repo` is derived first from the lowercase key, then `repoFullName` falls back to the PR only when no registered repo matches).
  • test/unit/contributor-open-pr-monitor.test.ts:203 — the assertion `expect(listPrSpy).toHaveBeenCalledWith(env, "entrius/allways-ui")` is coupled to mock insertion order: the lowercase PR is listed first so `repoOpen[0]` is always `entrius/allways-ui`; reversing the two mock entries would flip the casing and falsify the assertion without any bug in the fix — add a complementary reversed-order case, or address the root by deriving `repoFullName` from `repo.fullName` to make the assertion order-independent.
  • src/signals/contributor-open-pr-monitor.ts:63 and :220 — the two added comment blocks are significantly more verbose than the existing style in this file; both re-explain what the code already expresses — trim each to a single short clause or remove entirely.
  • src/signals/contributor-open-pr-monitor.ts:61 — derive `repoFullName` from the registered repo's authoritative `fullName` to make the casing deterministic regardless of which PR arrives first: `const repo = repositories.find(e => e.fullName.toLowerCase() === repoOpen[0]!.repoFullName.toLowerCase()) ?? null; const repoFullName = repo?.fullName ?? repoOpen[0]!.repoFullName;` — this also lets you drop the `!` assertion on the find.
  • test/unit/contributor-open-pr-monitor.test.ts — add a second `it` block (or a second `mockResolvedValue` variant) where PR 31 (`Entrius/Allways-UI`) is listed first; it should still assert `listPullRequests` is called with the registered canonical casing, proving the fix is robust to DB-delivery order rather than accidentally passing because of the current fixture ordering.
Review context
Contributor next steps
  • Review top overlaps.
  • Add scope summary.
  • Fix blocker.
  • Expect slower review.
  • Refresh registry data or choose a registered active repo.
  • Check active issues and PRs before submitting.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Review load = cached public PR metadata such as size labels, changed paths, and preflight status.
  • Open PR queue = repo-wide review pressure; it is not a PR quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
Review details

Generated from public PR metadata and the diff. Advisory only; deterministic signals remain authoritative.

The `groupByRepo` case-fold (`pr.repoFullName.toLowerCase()`) is exactly the right fix: it collapses case-variant entries for one repo into a single bucket, matching every other case-insensitive repo-name operation in `buildContributorOpenPrMonitor`. The consumer loop recovers original casing from `repoOpen[0]!.repoFullName`, mirroring the key-by-lower/keep-original pattern from `contributor-evidence-graph.ts`. The new integration test drives the real `buildContributorOpenPrMonitor` path through mocked repositories and correctly asserts both grouping and query-casing behaviour. No blockers.

Nits (5)

  • src/signals/contributor-open-pr-monitor.ts:61 — `repoOpen[0]!.repoFullName` as the canonical name for `listPullRequests` and `loadContributorRepoOpenPrSignals` is order-dependent: if the first PR in the bucket carries the atypical casing (`Entrius/Allways-UI`), case-sensitive DB queries use that casing and can return a partial result set for `detectPendingPrScenario`; prefer `repo?.fullName ?? repoOpen[0]!.repoFullName` (restructure so `repo` is derived first from the lowercase key, then `repoFullName` falls back to the PR only when no registered repo matches).
  • test/unit/contributor-open-pr-monitor.test.ts:203 — the assertion `expect(listPrSpy).toHaveBeenCalledWith(env, "entrius/allways-ui")` is coupled to mock insertion order: the lowercase PR is listed first so `repoOpen[0]` is always `entrius/allways-ui`; reversing the two mock entries would flip the casing and falsify the assertion without any bug in the fix — add a complementary reversed-order case, or address the root by deriving `repoFullName` from `repo.fullName` to make the assertion order-independent.
  • src/signals/contributor-open-pr-monitor.ts:63 and :220 — the two added comment blocks are significantly more verbose than the existing style in this file; both re-explain what the code already expresses — trim each to a single short clause or remove entirely.
  • src/signals/contributor-open-pr-monitor.ts:61 — derive `repoFullName` from the registered repo's authoritative `fullName` to make the casing deterministic regardless of which PR arrives first: `const repo = repositories.find(e => e.fullName.toLowerCase() === repoOpen[0]!.repoFullName.toLowerCase()) ?? null; const repoFullName = repo?.fullName ?? repoOpen[0]!.repoFullName;` — this also lets you drop the `!` assertion on the find.
  • test/unit/contributor-open-pr-monitor.test.ts — add a second `it` block (or a second `mockResolvedValue` variant) where PR 31 (`Entrius/Allways-UI`) is listed first; it should still assert `listPullRequests` is called with the registered canonical casing, proving the fix is robust to DB-delivery order rather than accidentally passing because of the current fixture ordering.

🟩 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 Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@dosubot dosubot Bot added the lgtm label Jun 28, 2026
@JSONbored
JSONbored merged commit 8e84e67 into JSONbored:main Jun 28, 2026
16 checks passed
@github-project-automation github-project-automation Bot moved this from Todo to Done in gittensory - v1 roadmap Jun 28, 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

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

[Bug]: contributor open-PR monitor groups PRs by case-sensitive repoFullName, splitting one repo into two

2 participants