Skip to content

chore(engine): remove dead isDuplicateClusterWinner (#6172) - #6434

Merged
JSONbored merged 1 commit into
JSONbored:mainfrom
shin-core:fix/remove-dead-duplicate-cluster-winner-6172
Jul 16, 2026
Merged

chore(engine): remove dead isDuplicateClusterWinner (#6172)#6434
JSONbored merged 1 commit into
JSONbored:mainfrom
shin-core:fix/remove-dead-duplicate-cluster-winner-6172

Conversation

@shin-core

Copy link
Copy Markdown
Contributor

Summary

  • packages/loopover-engine/src/duplicate-winner.ts's isDuplicateClusterWinner (PR-number election) was JSDoc-@deprecated in favor of isDuplicateClusterWinnerByClaim, "retained only for legacy compatibility callers that do not have claim timestamps." A fresh repo-wide search confirms those legacy callers do not exist: every real caller (e.g. advisory/gate-advisory.ts) uses isDuplicateClusterWinnerByClaim; the bare function had only re-exports, tests, and {@link} mentions.
  • Removed isDuplicateClusterWinner and its three re-exports (packages/loopover-engine/src/index.ts, packages/loopover-engine/src/signals/duplicate-winner.ts, src/signals/duplicate-winner.ts). isDuplicateClusterWinnerByClaim and resolveDuplicateClusterWinnerNumber are untouched.
  • Removed the tests that existed only to exercise the deleted function (dedicated cases in packages/loopover-engine/test/duplicate-winner.test.ts, the shim-identity assertion in duplicate-winner-signals-shim.test.ts, the root cases in test/unit/duplicate-winner.test.ts, and the two lines in test/unit/predicted-gate-engine-coverage.test.ts); every test for the remaining exports stays.
  • Updated the now-stale {@link isDuplicateClusterWinner} / prose mentions in src/queue/duplicate-detection.ts, src/services/maintainer-activation.ts, and packages/loopover-engine/src/predicted-gate.ts to reference the surviving election function (or describe it generically) so no comment points at a function that no longer exists.

Closes #6172

Scope

  • The PR title follows type(scope): short summary Conventional Commit format.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked a currently open issue this PR resolves (Closes #6172).

Validation

  • git diff --check
  • npm run actionlint (via npm run test:ci)
  • npm run typecheck
  • npm run test:coverage — this PR only DELETES source lines (no added source lines for codecov/patch to fall short on); the surviving isDuplicateClusterWinnerByClaim/resolveDuplicateClusterWinnerNumber keep their full existing test coverage.
  • npm run test:workers (via npm run test:ci)
  • npm run build:mcp (via npm run test:ci)
  • npm run test:mcp-pack (via npm run test:ci)
  • npm run ui:openapi:check (via npm run test:ci)
  • npm run ui:lint (via npm run test:ci)
  • npm run ui:typecheck (via npm run test:ci)
  • npm run ui:build (via npm run test:ci)
  • npm audit --audit-level=moderate
  • @loopover/engine package tests (567 pass) and the root duplicate-winner / predicted-gate-coverage suites pass with the removed cases gone.

If any required check was skipped, explain why:

  • No OpenAPI/cf-typegen/migration/env-reference regeneration needed — this removes an internal engine helper with no API, binding, schema, or env surface.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests. — N/A: none touched.
  • API/OpenAPI/MCP behavior is updated and tested where needed. — N/A: no API/OpenAPI/MCP surface changed.
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. — N/A: no UI change.
  • Visible UI changes include a UI Evidence section. — N/A: no visible UI change.
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs. — N/A: no docs/changelog change.

Notes

  • Verified the real duplicate-cluster election path (isDuplicateClusterWinnerByClaim, used by advisory/gate-advisory.ts through the signals/duplicate-winner.js shim) is unaffected — only the unused PR-number-election variant and its own tests/comments are removed.

isDuplicateClusterWinner (PR-number election) was @deprecated in favor of
isDuplicateClusterWinnerByClaim, retained "only for legacy compatibility
callers that do not have claim timestamps." A fresh repo-wide search confirms
those callers don't exist: every real caller uses isDuplicateClusterWinnerByClaim;
the bare function had only re-exports, wrapper-only tests, and {@link} mentions.

Remove the function and its three re-exports (engine index.ts, both
signals/duplicate-winner.ts shims), the tests that existed only to exercise it,
and refresh the now-stale {@link}/prose references in duplicate-detection.ts,
maintainer-activation.ts, and predicted-gate.ts. isDuplicateClusterWinnerByClaim
and resolveDuplicateClusterWinnerNumber are untouched.

Closes JSONbored#6172
@shin-core
shin-core requested a review from JSONbored as a code owner July 16, 2026 07:23
@superagent-security

Copy link
Copy Markdown
Contributor

🚨 Contributor flagged. Click here for more info: Superagent Dashboard

@superagent-security superagent-security Bot added the contributor:flagged Contributor flagged for review by trust analysis. label Jul 16, 2026
@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.61%. Comparing base (a2d8306) to head (f4dee3e).
⚠️ Report is 17 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6434   +/-   ##
=======================================
  Coverage   95.61%   95.61%           
=======================================
  Files         600      600           
  Lines       47346    47351    +5     
  Branches    15067    15067           
=======================================
+ Hits        45268    45273    +5     
  Misses       1291     1291           
  Partials      787      787           
Flag Coverage Δ
shard-1 44.03% <ø> (-0.01%) ⬇️
shard-2 36.74% <ø> (-0.03%) ⬇️
shard-3 32.38% <ø> (+0.03%) ⬆️
shard-4 34.60% <ø> (+0.07%) ⬆️
shard-5 30.80% <ø> (-0.77%) ⬇️
shard-6 45.30% <ø> (+0.31%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
packages/loopover-engine/src/duplicate-winner.ts 100.00% <ø> (ø)
packages/loopover-engine/src/predicted-gate.ts 100.00% <ø> (ø)
src/queue/duplicate-detection.ts 100.00% <ø> (ø)
src/services/maintainer-activation.ts 100.00% <ø> (ø)

... and 1 file with indirect coverage changes

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

loopover-orb Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Warning

⏸️ LoopOver review result - manual review recommended

Review updated: 2026-07-16 08:21:07 UTC

11 files · 1 AI reviewer · no blockers · CI green · unstable

⏸️ Suggested Action - Manual Review

Review summary
This is a clean dead-code removal: the deprecated PR-number-only `isDuplicateClusterWinner` is deleted along with its three barrel re-exports, and every stale `{@​link isDuplicateClusterWinner}`/prose mention is updated to reference the surviving `isDuplicateClusterWinnerByClaim`. Tests that existed solely to exercise the deleted function are removed, while tests for `isDuplicateClusterWinnerByClaim` and `resolveDuplicateClusterWinnerNumber` are left intact. The PR description's claim that no legacy (non-claim-timestamp) callers exist is consistent with what's shown here — only re-exports, tests, and doc links reference the removed symbol, no production call site does.

Nits — 5 non-blocking
  • The `index.ts` file is already ~708 lines per the size-smell note; not this PR's fault, but worth a future split since this diff touches it again.
  • test/unit/predicted-gate-engine-coverage.test.ts:249 renames the test to 'exercises the inactive lane advice branch' — worth double-checking no other coverage line in that describe block silently drops below the patch-coverage bar now that two assertions were removed.
  • Confirm CI codecov/patch (already shown PASSED) reflects no coverage regression from the removed branch-covering assertions in predicted-gate-engine-coverage.test.ts.
  • Grep the wider repo (docs/, README) for any remaining prose references to `isDuplicateClusterWinner` outside the files touched here, since the diff only fixes `{@​link}` mentions in three specific files.
  • Code changes lack test evidence — Add focused regression tests or explain why existing coverage is sufficient.
Flagged checks (non-blocking)
  • Contributor trust — Contributor flagged for review

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 #6172
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: 45 registered-repo PR(s), 21 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor shin-core; Gittensor profile; 45 PR(s), 0 issue(s).
Improvement ℹ️ None detected risk: low · value: none
Review context
  • Author: shin-core
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 45 PR(s), 0 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Keep the PR focused and include validation evidence before maintainer review.
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 &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; 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 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LoopOver approves — the gate is satisfied and CI is green.

@JSONbored
JSONbored merged commit fada218 into JSONbored:main Jul 16, 2026
15 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor:flagged Contributor flagged for review by trust analysis. gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

chore(engine): remove dead isDuplicateClusterWinner (its own @deprecated comment describes callers that don't exist)

2 participants