fix(signals): sample collision pairwise PRs linked-first with deterministic updatedAt rank - #2365
Conversation
…nistic updatedAt rank buildCollisionReport capped pairwise pull requests with a raw slice(0, 120) while issues already use boundedCollisionIssues. Mirror that pattern: keep linked PRs, then fill by newest updatedAt so title/path overlap is not dropped on large queues due to arbitrary DB order.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2365 +/- ##
==========================================
+ Coverage 95.79% 95.80% +0.01%
==========================================
Files 224 224
Lines 24947 24970 +23
Branches 9068 9075 +7
==========================================
+ Hits 23897 23922 +25
Misses 428 428
+ Partials 622 620 -2
🚀 New features to boost your workflow:
|
|
Warning 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏸️ Gittensory review result - manual review recommendedReview updated: 2026-07-01 20:22:29 UTC
⏸️ Suggested Action - Manual Review Review summary Nits — 3 non-blocking
Review context
Contributor next steps
Signal definitions
🟩 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.
|
Summary
buildCollisionReport's pairwise open-PR sampling used a plainopenPullRequests.slice(0, MAX_COLLISION_PAIRWISE_PULL_REQUESTS)(cap 120): whichever 120 PRs happened to be first in the caller's array, with no preference for linked-issue PRs and no recency ordering. Once a repo's open-PR queue passes 120, two genuinely overlapping/duplicate PRs positioned later in the array were silently excluded from pairwise comparison and never clustered as a collision.boundedCollisionPullRequests, mirroring the existingboundedCollisionIssueshelper: linked-issue PRs are kept first, then the remaining budget is filled by most-recently-updated, so overlapping PRs are no longer dropped by input-order luck..gittensory.ymlsetsissueDiscoveryPolicy: discouragedandlinkedIssuePolicy: preferred(not required). This is a small, self-contained, self-evident fix confined to one file plus its regression test.Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coveragelocally;codecov/patchrequires >=99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.npm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderateIf any required check was skipped, explain why:
npm run test:cichain on this Windows dev machine stops attest:coveragebecause of pre-existing, environment-only failures unrelated to this diff (CRLF vs LF mismatch from a Windowscore.autocrlfcheckout ingittensory-focus-manifest.test.ts, missingclaude/codexCLI binaries on PATH inselfhost-ai.test.ts, and a libuv/Windows subprocess assertion in thegittensory-mcpCLI tests). None of the 17 pre-existing failures touchsrc/signals/**or the changed test file; the targeted suites for this change (signals-v2.test.ts,signals.test.ts,signals-coverage.test.ts) pass cleanly andtsc --noEmitis clean. I did not reachtest:workers/build:mcp/ui:*locally because the&&-chainedtest:ciscript stops at the first failing step; this PR does not touch workers, MCP packaging, orapps/gittensory-ui/**, so those steps are unaffected by the diff.Safety
UI Evidencesection below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.If any required check was skipped, explain why: this PR has no auth/CORS/session surface and no UI change, so those boxes are not applicable.
Notes
test/unit/signals-v2.test.ts) that reproduces the bug: with more than 120 open PRs and two title-overlapping PRs positioned after the naive cap, the collision cluster was missing before this fix and is produced after it. I confirmed this by reverting the source change locally and observing the new test fail.