Skip to content

chore(mcp): remove dead referenceScorePreviewCommand export in local-branch.js - #6402

Merged
JSONbored merged 1 commit into
JSONbored:mainfrom
galuis116:chore/remove-dead-mcp-score-preview-command-export
Jul 16, 2026
Merged

chore(mcp): remove dead referenceScorePreviewCommand export in local-branch.js#6402
JSONbored merged 1 commit into
JSONbored:mainfrom
galuis116:chore/remove-dead-mcp-score-preview-command-export

Conversation

@galuis116

Copy link
Copy Markdown
Contributor

Closes #6259

Summary

  • Investigated both directions the issue offers before picking one: referenceScorePreviewCommand (computes the real, absolute path to the bundled scorer via packageRoot) vs. referenceScorePreviewExample (a hardcoded ./node_modules/@loopover/mcp/scripts/... string used in guidance text).
  • Wiring referenceScorePreviewCommand into guidance text would have been a regression, not a fix. test/unit/local-scorer-adapter.test.ts's existing "redacts local paths from scorer diagnostics and setup guidance" test explicitly asserts the guidance text (a) never contains the real local filesystem path and (b) matches /node_modules\/@loopover\/mcp\/scripts\// — i.e. the generic relative example is intentional, deliberately avoiding leaking an absolute local path (which could embed a username or other identifying info) into text a user might copy elsewhere. referenceScorePreviewExample is correct as-is; left untouched.
  • referenceScorePreviewCommand's real (and legitimate) use was as a test helper: local-scorer-adapter.test.ts's "runs the packaged reference scorer against metadata only" test uses it to invoke the actual bundled scorer script end to end — a genuinely valuable smoke test, not dead functionality. Removing it outright would have thrown that test coverage away.
  • Resolution: moved the packaged-scorer path resolution into the test file itself (a small local helper using the same join(process.cwd(), ...) pattern the test's neighboring fixtureCommand() helper already uses), since the test is genuinely its only real caller. Removed the now-orphaned referenceScorePreviewCommand export, packageRoot, and the resulting unused dirname import from lib/local-branch.js.

Scope

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck (root) — reliably OOMs on this shared sandbox regardless of what changed (reproduced repeatedly this session). packages/loopover-mcp is plain JS with its own npm run build (node --check across every lib/bin file) — ran it directly and it passes clean.
  • npm run test:coverage — not run repo-wide (same OOM risk); this change touches a packages/loopover-mcp/** JS file (not src/**) and a test file, so codecov/patch scope is minimal. Ran the full set of tests that import from local-branch.js: local-scorer-adapter.test.ts (7), local-branch.test.ts (59), mcp-release-candidate.test.ts (20) — 86/86 passing, including the "runs the packaged reference scorer against metadata only" smoke test still genuinely invoking the real bundled script, and the redaction test confirming guidance text is unaffected.
  • npm run test:workers — N/A, no Worker-facing code changed.
  • npm run build:mcp / npm run test:mcp-pack — N/A wiring, but packages/loopover-mcp's own npm run build (syntax-checks every lib/bin file including the edited local-branch.js) passes clean.
  • npm run ui:openapi:check / ui:lint / ui:typecheck / ui:build — N/A, no apps/loopover-ui changes.
  • npm audit --audit-level=moderate — 0 vulnerabilities.
  • New/changed behavior has tests — N/A new behavior; this preserves existing test coverage (the real-scorer smoke test) while removing the export it no longer needs to come from production code.

If any required check was skipped, explain why:

  • Root npm run typecheck / npm run test:coverage: reliably OOMs on this shared sandbox under memory pressure from concurrent sessions, independent of the diff. Substituted with packages/loopover-mcp's own lighter build check (clean) and the full set of directly-affected tests (86/86 passing).

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.
  • API/OpenAPI/MCP behavior is updated and tested where needed. — N/A, no MCP tool surface changed.
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. — N/A, no UI changes.
  • Visible UI changes include a UI Evidence section below with screenshots. — N/A, no visible UI change.
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs. — CHANGELOG.md untouched.

Notes

  • Confirmed via grep -rn "referenceScorePreviewCommand\b" across the repo (excluding node_modules) that zero references remain after the change.

…branch.js

Its only real caller was test/unit/local-scorer-adapter.test.ts, which
uses it to exercise the actual bundled scorer script end to end.
Production guidance text always uses the sibling
referenceScorePreviewExample instead -- and that's intentional, not a
bug: an existing test explicitly asserts the guidance text stays
generic/path-redacted (never leaks the real local filesystem path),
so wiring in the absolute-path version would have been a regression,
not a fix.

Moved the packaged-scorer path resolution into the test file itself
(the same join(process.cwd(), ...) pattern already used by its
neighboring fixtureCommand() helper) since the test is genuinely its
only caller. packageRoot and its now-unused dirname import are
removed from local-branch.js along with the export.

Closes JSONbored#6259
@galuis116
galuis116 requested a review from JSONbored as a code owner July 16, 2026 06:39
@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 (0d3f4cb) to head (fdd0a96).
⚠️ Report is 28 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6402   +/-   ##
=======================================
  Coverage   95.61%   95.61%           
=======================================
  Files         600      601    +1     
  Lines       47350    47352    +2     
  Branches    15063    15063           
=======================================
+ Hits        45272    45274    +2     
  Misses       1291     1291           
  Partials      787      787           
Flag Coverage Δ
shard-1 44.02% <ø> (-0.02%) ⬇️
shard-2 36.75% <ø> (+<0.01%) ⬆️
shard-3 32.56% <ø> (+<0.01%) ⬆️
shard-4 34.62% <ø> (+<0.01%) ⬆️
shard-5 31.59% <ø> (+<0.01%) ⬆️
shard-6 44.98% <ø> (+<0.01%) ⬆️

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

@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

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-16 07:03:09 UTC

2 files · 1 AI reviewer · no blockers · readiness 86/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This PR moves the packaged-scorer path builder from a production export (referenceScorePreviewCommand) into a local test-only helper, since the only real caller was the test file itself, and correctly leaves the intentionally path-redacted referenceScorePreviewExample untouched. The diff is coherent, small, closes #6259, and the now-unused dirname import is also removed; the new test helper uses join(process.cwd(), ...) mirroring the existing fixtureCommand() pattern in the same file. CI is fully green and the change is a clean, well-scoped dead-code removal with no functional regression.

Nits — 3 non-blocking
  • test/unit/local-scorer-adapter.test.ts: packagedScorerCommand assumes process.cwd() is the repo root when tests run, same assumption fixtureCommand() already makes, so it's consistent but worth confirming test runner cwd is stable across CI/local invocations.
  • packages/loopover-mcp/lib/local-branch.js: the PR also silently drops the packageRoot constant along with fileURLToPath usage for that constant — worth double-checking fileURLToPath import is still used elsewhere in the file (not shown in diff) since it's still imported.
  • Consider adding a short JSDoc-style note directly above referenceScorePreviewExample in local-branch.js cross-referencing the test file's packagedScorerCommand, so future readers know why a similar-looking function lives in two places.

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 #6259
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ❌ 8/20 High 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: 1926 registered-repo PR(s), 1268 merged, 54 issue(s).
Contributor context ✅ Confirmed Gittensor contributor galuis116; Gittensor profile; 1926 PR(s), 54 issue(s).
Improvement ✅ Minor risk: clean · value: minor
Review context
  • Author: galuis116
  • 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: 1926 PR(s), 54 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Add a concise scope and risk note.
  • Then work through the remaining 1 step in the Signals table above.
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 added the manual-review Gittensor contributor context label Jul 16, 2026
@JSONbored
JSONbored merged commit eaaae0f into JSONbored:main Jul 16, 2026
16 checks passed
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. manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

chore(mcp): remove dead referenceScorePreviewCommand export in lib/local-branch.js

2 participants