Skip to content

chore(miner): migrate batch 4.1 foundational lib modules to TypeScript - #7384

Merged
JSONbored merged 1 commit into
JSONbored:mainfrom
tryeverything24:migrate-miner-lib-batch-4-1-typescript
Jul 20, 2026
Merged

chore(miner): migrate batch 4.1 foundational lib modules to TypeScript#7384
JSONbored merged 1 commit into
JSONbored:mainfrom
tryeverything24:migrate-miner-lib-batch-4-1-typescript

Conversation

@tryeverything24

Copy link
Copy Markdown
Contributor

Closes #7309

Summary

Convert batch 4.1 of the packages/loopover-miner/lib/** TypeScript migration (parent #7290, Phase 4): claim-adjudication, governor-chokepoint-persisted, sentry, idea-feasibility, rejection-state-machine, attempt-worktree, portfolio-discovery, and process-lifecycle from plain .js + hand-maintained .d.ts to real .ts, compiled in place via the package's existing tsc build pipeline (matching the merged Phase 1 #7299 and Phase 4 batches 4.3/#7377, 4.4/#7383 precedent). No tsconfig.json changes needed -- the package's include glob already picks up new .ts files under lib/**. Regenerated and committed the compiled .js/.d.ts output alongside every .ts source (npm run build --workspace @loopover/miner).

No behavior change: sources are faithful line-for-line transcriptions of the prior .js + .d.ts pairs with real type annotations, keeping the same defaults, error-message strings, and injectable-dependency test seams each file already used. Two genuinely-unreachable defensive fallbacks in attempt-worktree.ts (cloneResult.error ?? "ensure_repo_cloned_failed" and added.error ?? "git_worktree_add_failed") were simplified to non-null assertions rather than faked with a test -- ensureRepoCloned (repo-clone.ts) and the engine's addWorktree (worktree-plan.ts) both always set a real, non-empty error string on every ok: false return path, so the string-literal fallback was dead code.

Testing

  • Extended the existing per-module unit tests (miner-claim-adjudication, miner-governor-chokepoint-persisted, miner-sentry, miner-idea-feasibility, miner-rejection-state-machine, miner-attempt-worktree, miner-portfolio-discovery, miner-process-lifecycle, plus dependents miner-claim-conflict-resolver, miner-attempt-cli, miner-worktree-allocator, miner-local-store, miner-repo-clone-lock) -- 213 tests, all green.
  • Drove every one of the 8 converted files to 100% statement/branch/function/line coverage (per-file vitest run --coverage --coverage.reporter=json-summary, exact numbers parsed from coverage-summary.json).
  • npm run typecheck (root) and packages/loopover-miner's own tsc --noEmit clean.
  • npm run build --workspace @loopover/miner clean, compiled output committed.
  • Full CI parity run locally, all clean: actionlint, db:schema-drift:check, selfhost:env-reference:check, miner:env-reference:check, docs:drift-check, branding-drift:check, manifest:drift-check, engine-parity:drift-check, release-manifest:sync:check, command-reference:check, test:workers, build:mcp, test:mcp-pack, test:miner-pack, test:miner-deployment-docs-audit, ui:openapi:check, ui:typecheck, ui:lint, npm audit.
  • git diff --check against upstream/main clean (no trailing-whitespace regressions in the compiled .js output).
  • Rebased onto current upstream/main immediately before push (already up to date, no new commits to pick up).

Converts claim-adjudication, governor-chokepoint-persisted, sentry,
idea-feasibility, rejection-state-machine, attempt-worktree,
portfolio-discovery, and process-lifecycle from plain .js + hand-maintained
.d.ts to real .ts under the existing in-place tsc emit pipeline from JSONbored#7299.
Extends targeted unit tests to drive every converted file to 100%
statement/branch/function/line coverage.
@JSONbored
JSONbored force-pushed the migrate-miner-lib-batch-4-1-typescript branch from f2cf7bc to dd86655 Compare July 20, 2026 04:49
@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.19%. Comparing base (32e0998) to head (dd86655).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7384   +/-   ##
=======================================
  Coverage   91.19%   91.19%           
=======================================
  Files         716      716           
  Lines       72733    72759   +26     
  Branches    20578    20643   +65     
=======================================
+ Hits        66326    66353   +27     
+ Misses       5365     5364    -1     
  Partials     1042     1042           
Flag Coverage Δ
shard-1 35.92% <84.26%> (+0.02%) ⬆️
shard-2 41.34% <8.12%> (-0.11%) ⬇️
shard-3 38.88% <15.22%> (-0.06%) ⬇️
shard-4 39.86% <5.58%> (-0.02%) ⬇️
shard-5 27.18% <8.62%> (-0.02%) ⬇️
shard-6 31.29% <26.39%> (-0.02%) ⬇️

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

Files with missing lines Coverage Δ
packages/loopover-miner/lib/attempt-worktree.ts 100.00% <100.00%> (ø)
packages/loopover-miner/lib/claim-adjudication.ts 100.00% <100.00%> (ø)
...oopover-miner/lib/governor-chokepoint-persisted.ts 100.00% <100.00%> (ø)
packages/loopover-miner/lib/idea-feasibility.ts 100.00% <100.00%> (ø)
packages/loopover-miner/lib/portfolio-discovery.ts 100.00% <100.00%> (ø)
packages/loopover-miner/lib/process-lifecycle.ts 100.00% <100.00%> (ø)
...ages/loopover-miner/lib/rejection-state-machine.ts 100.00% <100.00%> (ø)
packages/loopover-miner/lib/sentry.ts 100.00% <100.00%> (ø)

... and 4 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 20, 2026
@loopover-orb

loopover-orb Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-20 05:40:26 UTC

28 files · 1 AI reviewer · no blockers · readiness 82/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This is a mechanical .js+.d.ts → .ts conversion for 8 lib modules plus their compiled output, matching the stated pattern from prior merged batches (#7299, #7377, #7383). The two `?? "..."` fallbacks in attempt-worktree.js were correctly identified as dead code and replaced with non-null assertions (`error!`), consistent with the trace showing repo-clone.ts and worktree-plan.ts always set a real error string on `ok:false`. Everything else is line-for-line faithful transcription with real types added; no behavior change is visible in the diff.

Nits — 5 non-blocking
  • The external brief's 'floating promise' flag on attempt-worktree.ts:23 is a false positive — `createRealWorktreeExec` returns a function that itself returns/constructs a `new Promise`, which is correctly returned to the caller to await.
  • packages/loopover-miner/lib/process-lifecycle.ts uses `console.error` as the default `log` — pre-existing behavior carried over from the .js file, not new, but worth confirming it's still the desired default now that Sentry capture exists alongside it.
  • Test additions (+71 lines across 4 new/updated test files) look thin relative to the size of the new .ts surface, though the PR description claims 100% coverage was achieved via the existing per-module suites which aren't fully shown in this diff — worth double-checking coverage numbers in CI rather than raw added-line count.
  • The magic numbers flagged (128 in process-lifecycle, 5671 in idea-feasibility comment, 2000 in sentry.ts) are either pre-existing conventions from the original .js/.d.ts (128 = standard signal exit code, already documented inline) or issue-reference numbers in comments, not real magic-number smells.
  • Confirm the 8 converted modules' test files (miner-attempt-worktree, miner-portfolio-discovery, miner-governor-chokepoint-persisted, miner-rejection-state-machine, etc.) were only extended, not reduced, in assertions — the diff's test files are truncated in what's shown here.

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 #7309
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: 54 registered-repo PR(s), 19 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor tryeverything24; Gittensor profile; 54 PR(s), 0 issue(s).
Improvement ✅ Minor risk: clean · value: minor
Review context
  • Author: tryeverything24
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: JavaScript, TypeScript, Python, HTML, C++, Java, PHP, C#
  • Official Gittensor activity: 54 PR(s), 0 issue(s).
  • Related work: Titles/paths share 9 meaningful terms. (issue #7300)
  • Related work: Titles/paths share 8 meaningful terms. (issue #7300, issue #7329)
  • Related work: Titles/paths share 8 meaningful terms. (issue #7300, issue #7330)
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

@JSONbored
JSONbored merged commit 72577ab into JSONbored:main Jul 20, 2026
15 checks passed
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. manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

chore(miner): migrate foundational, most-depended-on packages/loopover-miner/lib modules to TypeScript (batch 4.1 of 8 (Phase 4))

2 participants