Skip to content

chore(miner): migrate batch 2.2 utility modules to TypeScript - #7338

Closed
RealDiligent wants to merge 1 commit into
JSONbored:mainfrom
RealDiligent:chore/miner-ts-batch-2.2-7301
Closed

chore(miner): migrate batch 2.2 utility modules to TypeScript#7338
RealDiligent wants to merge 1 commit into
JSONbored:mainfrom
RealDiligent:chore/miner-ts-batch-2.2-7301

Conversation

@RealDiligent

Copy link
Copy Markdown
Contributor

Closes #7301

Summary

  • Converts 8 leaf-most, lowest-fan-in packages/loopover-miner/lib modules from plain .js to real, compiler-verified TypeScript: deployment-docs-audit, chat-discover-attempt-actions, update-check, init-wizard, deny-hook-synthesis, calibration-run, manage-poll, orb-export.
  • Follows the in-place-emit pattern the Phase 1 build pipeline (chore(miner): add a real tsc build pipeline for loopover-miner (#7290) #7299) already wired up (lib/foo.ts compiles to lib/foo.js next to it; no import-path changes for any consumer).
  • Each file's previously hand-maintained .d.ts sibling is now tsc-generated instead of hand-written.
  • No behavior change.

Scope checklist

Validation checklist

  • npx tsc -p packages/loopover-miner/tsconfig.json passes clean; node --check on all 8 compiled outputs
  • Existing module tests: deployment-docs-audit, chat-discover-attempt-actions, calibration-run, manage-poll, orb-export, miner-cli (update-check), wire-cli-modules
  • init-wizard / deny-hook-synthesis path-separator and Windows mode assertions fail locally on win32 only (path.join / 0o600); Linux CI is the gate

Safety checklist

  • No secrets, tokens, wallets, hotkeys/coldkeys, trust scores, or reward/payout values touched
  • No changes to site/, CNAME, or **/lovable/**

Made with Cursor

@RealDiligent
RealDiligent requested a review from JSONbored as a code owner July 19, 2026 18:28
@superagent-security

Copy link
Copy Markdown
Contributor

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

Closes JSONbored#7301. Convert eight leaf-most lib modules to real .ts with
in-place emit; hand-maintained .d.ts siblings are now tsc-generated.

Co-authored-by: Cursor <cursoragent@cursor.com>
@codecov

codecov Bot commented Jul 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.60807% with 86 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.89%. Comparing base (f158305) to head (25bb3db).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
packages/loopover-miner/lib/manage-poll.ts 68.70% 2 Missing and 39 partials ⚠️
packages/loopover-miner/lib/update-check.ts 77.77% 4 Missing and 14 partials ⚠️
packages/loopover-miner/lib/orb-export.ts 93.83% 0 Missing and 9 partials ⚠️
packages/loopover-miner/lib/deny-hook-synthesis.ts 88.05% 2 Missing and 6 partials ⚠️
...ckages/loopover-miner/lib/deployment-docs-audit.ts 88.88% 0 Missing and 5 partials ⚠️
packages/loopover-miner/lib/init-wizard.ts 94.79% 0 Missing and 5 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7338      +/-   ##
==========================================
+ Coverage   91.03%   91.89%   +0.86%     
==========================================
  Files         714      631      -83     
  Lines       72118    51553   -20565     
  Branches    19232    15866    -3366     
==========================================
- Hits        65651    47374   -18277     
+ Misses       5427     3239    -2188     
+ Partials     1040      940     -100     
Flag Coverage Δ
rees ?
shard-1 41.29% <0.00%> (-0.44%) ⬇️
shard-2 35.70% <29.39%> (-0.02%) ⬇️
shard-3 31.55% <9.94%> (-0.07%) ⬇️
shard-4 32.65% <24.63%> (+0.07%) ⬆️
shard-5 30.02% <6.48%> (-0.55%) ⬇️
shard-6 43.72% <33.28%> (+0.17%) ⬆️

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

Files with missing lines Coverage Δ
packages/loopover-miner/lib/calibration-run.ts 100.00% <100.00%> (ø)
...oopover-miner/lib/chat-discover-attempt-actions.ts 100.00% <100.00%> (ø)
...ckages/loopover-miner/lib/deployment-docs-audit.ts 88.88% <88.88%> (ø)
packages/loopover-miner/lib/init-wizard.ts 94.79% <94.79%> (ø)
packages/loopover-miner/lib/deny-hook-synthesis.ts 88.05% <88.05%> (ø)
packages/loopover-miner/lib/orb-export.ts 93.83% <93.83%> (ø)
packages/loopover-miner/lib/update-check.ts 77.77% <77.77%> (ø)
packages/loopover-miner/lib/manage-poll.ts 68.70% <68.70%> (ø)

... and 90 files with indirect coverage changes

@loopover-orb loopover-orb Bot added the gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. label Jul 19, 2026
@loopover-orb

loopover-orb Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Caution

🛑 LoopOver review result - reject/close recommended

Review updated: 2026-07-19 18:36:40 UTC

24 files · 1 AI reviewer · 1 blocker · CI failing · blocked

🛑 Suggested Action - Reject/Close

Review summary
This PR ports 8 leaf-most `packages/loopover-miner/lib` modules from `.js` to `.ts`, generating `.d.ts` output in place, matching the Phase 1 build pipeline established in #7299. The diff appears to be a faithful mechanical conversion (full post-change file content for the fully-visible files, e.g. calibration-run, matches the described logic with only type annotations added), and it is scoped to the 8 files named in #7301, closing that issue. However, the PR adds ~3,709 source lines with zero new test lines, and CI's `codecov/patch` check already failed at 87.60% against the repo's ~97-99% patch-coverage bar, plus `validate` and `validate-tests-merge` are also failing.

Blockers

  • CI already reports `codecov/patch` failing at 87.60% (target 99%) with zero test lines added across 3,709 new/changed source lines in 24 files — per this repo's coverage bar, that needs to be closed (either by demonstrating the existing suite already exercises the compiled `.js` output paths for every branch, or by adding coverage) before this is merge-ready.
  • `validate` and `validate-tests-merge` are failing on this commit and the PR description's validation checklist doesn't explain what's failing there — these need to be resolved or explained before merge.
Nits — 6 non-blocking
  • `packages/loopover-miner/lib/orb-export.ts:209` and `update-check.ts:2` hardcode `api.loopover.ai` / `registry.npmjs.org` hosts directly in source rather than via config — verify this matches the pre-existing `.js` behavior (likely does, given `resolveAmsCollectorUrl` allows an env override) rather than being a new hardcode introduced by the migration.
  • Several new `0o700`/`0o600` file-mode and timeout literals (e.g. `orb-export.ts:127`, `init-wizard.ts:203`) are unnamed magic numbers, though these mirror the pre-existing `.js` conventions used elsewhere in this package, so likely not worth changing here.
  • Deep nesting (depth 5) flagged in a few spots (e.g. `manage-poll.ts:144`, `orb-export.ts:261`) — again likely inherited from the original `.js`, so only worth a follow-up refactor if the maintainer wants to use this migration as an opportunity to flatten them.
  • Confirm (e.g. in the PR description) that the existing test suite (like `test/integration/miner-calibration-loop.test.ts`) runs against the compiled `.js` outputs of these modules and thus already covers the ported logic, to justify the 0% new-test-line ratio despite the large diff.
  • Since `.d.ts` files are now `tsc`-generated, double check `.gitignore`/`files` glob in `package.json` doesn't end up double-publishing both hand-authored and generated `.d.ts` artifacts for any transitional period.
  • Code changes lack test evidence — Add focused regression tests or explain why existing coverage is sufficient.

Why this is blocked

  • CI already reports `codecov/patch` failing at 87.60% (target 99%) with zero test lines added across 3,709 new/changed source lines in 24 files — per this repo's coverage bar, that needs to be closed (either by demonstrating the existing suite already exercises the compiled `.js` output paths for every branch, or by adding coverage) before this is merge-ready.
📋 Copy for AI agents — paste into your coding agent
Fix the following blocker(s) from this PR review:

1. CI already reports \`codecov/patch\` failing at 87.60% \(target 99%\) with zero test lines added across 3,709 new/changed source lines in 24 files — per this repo's coverage bar, that needs to be closed \(either by demonstrating the existing suite already exercises the compiled \`.js\` output paths for every branch, or by adding coverage\) before this is merge-ready.

CI checks failing

  • validate
  • codecov/patch — 87.60% of diff hit (target 99.00%)
  • validate-tests-merge

Decision drivers

  • ❌ Code review — 1 blocker (1 reviewer)
  • ❌ Gate result — Blocking (Repo-configured hard blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #7301
Related work ⚠️ 3 scoped overlaps Top overlaps are listed below; lower-confidence bulk is hidden.
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: 399 registered-repo PR(s), 204 merged, 36 issue(s).
Contributor context ✅ Confirmed Gittensor contributor RealDiligent; Gittensor profile; 399 PR(s), 36 issue(s).
Improvement ℹ️ None detected risk: low · value: none · LLM: moderate
Linked issue satisfaction

Partially addressed
The diff shows 4 of the 8 required files (deny-hook-synthesis, calibration-run, manage-poll, orb-export) converted to real .ts with hand-maintained .d.ts presumably removed, but the truncated diff does not show deployment-docs-audit.ts, chat-discover-attempt-actions.ts, update-check.ts, or init-wizard.ts being converted, though the PR description claims all 8 are included.

Review context
  • Author: RealDiligent
  • 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: 399 PR(s), 36 issue(s).
  • Related work: Titles/paths share 7 meaningful terms. (issue #7300)
  • Related work: Titles/paths share 7 meaningful terms. (issue #7302)
  • Related work: Titles/paths share 6 meaningful terms. (PR #7339)
  • Additional title-only matches omitted; title-only overlap does not block.
Contributor next steps
  • Start here: Review top overlaps.
  • Then work through the remaining 2 steps 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 commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

LoopOver is closing this pull request on the maintainer's behalf (CI is failing (validate, codecov/patch, validate-tests-merge); AI reviewers agree on a likely critical defect: CI already reports `codecov/patch` failing at 87.60% (target 99%) with zero test lines added across 3,709 new/changed source lines in 24 files — per this repo's coverage bar, that needs to be closed (either by demonstrating the existing suite already exercises the compiled `.js` output paths for every branch, or by adding coverage) before this is merge-ready.). 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

chore(miner): migrate leaf-most, lowest-fan-in utility packages/loopover-miner/lib modules to TypeScript (batch 2.2 of 5 (Phase 2))

1 participant