Skip to content

fix/issue 5934#5974

Closed
nghetienhiep wants to merge 1 commit into
JSONbored:mainfrom
nghetienhiep:fix/issue-5934
Closed

fix/issue 5934#5974
nghetienhiep wants to merge 1 commit into
JSONbored:mainfrom
nghetienhiep:fix/issue-5934

Conversation

@nghetienhiep

@nghetienhiep nghetienhiep commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

@nghetienhiep
fix(selfhost): default MINER_BIN to loopover-miner in prometheus text…
75fe1b0 
…file export

export-miner-prometheus-textfile.sh fell back to the nonexistent
gittensory-miner binary when LOOPOVER_MINER_BIN wasn't set, so a
zero-config run silently produced an empty .prom file (every
export_family call hit command-not-found and was swallowed by the
fail-open-per-family design). The package's real bin entry is
loopover-miner.

Closes JSONbored#5934
@nghetienhiep
nghetienhiep requested a review from JSONbored as a code owner July 15, 2026 05:16
@superagent-security

superagent-security Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.16%. Comparing base (4749ff7) to head (75fe1b0).

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #5974   +/-   ##
=======================================
  Coverage   95.16%   95.16%           
=======================================
  Files         589      589           
  Lines       46790    46790           
  Branches    14943    14943           
=======================================
  Hits        44528    44528           
  Misses       1511     1511           
  Partials      751      751
Flag Coverage Δ
shard-1 43.91% <ø> (ø)
shard-2 36.22% <ø> (+0.01%) ⬆️
shard-3 31.91% <ø> (-0.07%) ⬇️
shard-4 31.95% <ø> (ø)
shard-5 32.49% <ø> (-0.01%) ⬇️
shard-6 44.70% <ø> (ø)

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 15, 2026
@loopover-orb

loopover-orb Bot commented Jul 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Caution

🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥

🛑 LoopOver review result - reject/close recommended

Review updated: 2026-07-15 05:23:17 UTC

2 files · 1 AI reviewer · 1 blocker · readiness 98/100 · CI green · clean

🛑 Suggested Action - Reject/Close

Review summary
This is a one-line fix changing the default MINER_BIN fallback from a nonexistent `gittensory-miner` binary to the actual installed `loopover-miner` binary, addressing #5934 where the script silently produced an empty .prom file due to fail-open per-family export handling. The fix is correct and traced to the real cause (wrong default binary name), and it's backed by a solid regression test using a fake binary on PATH that verifies all four metric families are populated when no override is given, plus a test confirming the explicit override still works.

Nits — 4 non-blocking
  • scripts/export-miner-prometheus-textfile.sh:20 — the OUT_FILE default still uses the old `gittensory_miner.prom` filename, which is inconsistent with this rebrand fix and could be flagged in a follow-up full-cutover rename PR.
  • test/unit/export-miner-prometheus-textfile-script.test.ts — the test shells out via `execFileSync("sh", ...)`, which is fine but ties the test to POSIX sh being on PATH in CI; a comment noting this assumption would help future maintainers.
  • Consider renaming the OUT_FILE default's `gittensory_miner.prom` to `loopover_miner.prom` in the same rebrand pass referenced by fix(rebrand): full-cutover rename Prometheus/Alertmanager/Grafana observability identifiers #5768/fix(rebrand): full-cutover rename miner/AMS per-repo and operator config filenames #5765, to fully close out the identifier rename.
  • The regression test comment nicely documents the fail-open rationale — worth linking directly to script line numbers so it stays accurate if the script is refactored.

Why this is blocked

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
📋 Copy for AI agents — paste into your coding agent 
Fix the following blocker(s) from this PR review:

1. No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
Signal Result Evidence
Code review ❌ 1 blocker 1 reviewer
Linked issue ✅ No-issue rationale PR body explains why no issue is linked.
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 (no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 52 registered-repo PR(s), 23 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor nghetienhiep; Gittensor profile; 52 PR(s), 0 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Improvement ℹ️ Insufficient signal risk: clean · value: insufficient-signal · LLM: minor
Review context
  • Author: nghetienhiep
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: Vue, C#, JavaScript, Python, C, C++, Go, Objective-C
  • Official Gittensor activity: 52 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://gittensory.aethereal.dev/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 15, 2026

Copy link
Copy Markdown
Contributor

LoopOver is closing this pull request on the maintainer's behalf (No linked issue detected). 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 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JSONbored JSONbored Awaiting requested review from JSONbored JSONbored is a code owner

Assignees

@nghetienhiep nghetienhiep

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@nghetienhiep