Skip to content

chore(miner): migrate batch 2.3 utility modules to TypeScript - #7323

Closed
joaovictor91123 wants to merge 2 commits into
JSONbored:mainfrom
joaovictor91123:chore/miner-ts-migration-batch-2.3-7302
Closed

chore(miner): migrate batch 2.3 utility modules to TypeScript#7323
joaovictor91123 wants to merge 2 commits into
JSONbored:mainfrom
joaovictor91123:chore/miner-ts-migration-batch-2.3-7302

Conversation

@joaovictor91123

Copy link
Copy Markdown
Contributor

Closes #7302

Summary

  • Converts 8 leaf-most, lowest-fan-in packages/loopover-miner/lib modules from plain .js to real, compiler-verified TypeScript: cross-repo-evaluation, version, slop-assessment, discovery-throttle, claim-ledger-expiry, prompt-injection-defense, execute-local-write, portfolio-queue-expiry.
  • 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.
  • Fixes two stale hand-written .d.ts signatures that real type-checking surfaced: claim-ledger-expiry's and portfolio-queue-expiry's store interfaces (expireClaim/reclaimStuckItem) were both missing the optional apiBaseUrl third parameter the code was already always calling them with.
  • No behavior change. Two latent runtime quirks in cross-repo-evaluation were preserved exactly (not "fixed away") during conversion: non-string identity fields (repo/commitT/lastActivityAt) fall back to null, and a malformed commit's sha stays undefined in the knownCommitShas array rather than being filtered out — both verified against the existing test suite.

Scope checklist

Validation checklist

  • npm run build --workspace @loopover/miner (tsc + node --check on every compiled output) passes clean
  • npm run typecheck (repo-wide) passes clean
  • All 8 modules' existing test files pass: miner-version, miner-slop-assessment, miner-discovery-throttle, miner-claim-ledger-expiry, miner-prompt-injection-defense, miner-execute-local-write, miner-portfolio-queue-expiry, miner-cross-repo-evaluation (53 tests total)
  • Two pre-existing failures verified unrelated via git stash against a clean checkout (reproduce identically without this PR's changes): miner-cross-repo-evaluation.test.ts's known Vitest-collection SyntaxError, and miner-execute-local-write.test.ts's Windows sh -c path-format mismatch (/tmp/... vs C:\...)

Safety checklist

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

Converts env-file-indirection, deny-check, cli, chat-action-dispatch,
ams-health-server, replay-task-bridge, chat-governor-actions, and
chat-portfolio-actions from plain .js to real, compiler-verified
TypeScript, following the in-place-emit pattern the Phase 1 build
pipeline (JSONbored#7299) already wired up. Each file's hand-maintained .d.ts
sibling is now tsc-generated instead. No behavior change; existing
tests pass unmodified.

Closes JSONbored#7300
Converts cross-repo-evaluation, version, slop-assessment,
discovery-throttle, claim-ledger-expiry, prompt-injection-defense,
execute-local-write, and portfolio-queue-expiry from plain .js to
real, compiler-verified TypeScript, following the in-place-emit
pattern the Phase 1 build pipeline (JSONbored#7299) already wired up. Each
file's hand-maintained .d.ts sibling is now tsc-generated instead.

Fixes two stale hand-written .d.ts signatures surfaced by real type
checking: claim-ledger-expiry's and portfolio-queue-expiry's store
interfaces were both missing the optional apiBaseUrl third parameter
that expireClaim/reclaimStuckItem are always actually called with.

No behavior change; existing tests pass unmodified (two pre-existing,
unrelated Windows/path failures verified via git stash against a
clean checkout: miner-cross-repo-evaluation.test.ts's known collection
SyntaxError, and miner-execute-local-write.test.ts's sh -c path-format
mismatch).

Closes JSONbored#7302
@superagent-security

Copy link
Copy Markdown
Contributor

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

@superagent-security

Copy link
Copy Markdown
Contributor

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

@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 16:41:32 UTC

48 files · 1 AI reviewer · 1 blocker · CI green · clean

🛑 Suggested Action - Reject/Close

Review summary
The stated intent is a narrow TS migration of 8 named leaf modules from #7302, and that part (cross-repo-evaluation, claim-ledger-expiry, portfolio-queue-expiry, etc.) looks like a faithful behavior-preserving conversion with the two documented .d.ts signature fixes. However the diff actually touches 48 files and includes several brand-new, non-migration modules — chat-action-dispatch.ts, chat-governor-actions.ts, chat-portfolio-actions.ts, ams-health-server.ts, and replay-task-bridge.ts — that implement new chat-action dispatch chokepoints, governor pause/resume wiring, portfolio queue chat actions, an HTTP health surface, and a replay-snapshot-to-task bridge. None of these are in the 8-file list the PR description claims to be migrating, and they are new logic, not TS ports of existing .js files. This is a scope-creep bundling problem on top of an otherwise reasonable migration.

Blockers

  • The diff bundles entirely new feature modules (chat-action-dispatch.ts, chat-governor-actions.ts, chat-portfolio-actions.ts, ams-health-server.ts, replay-task-bridge.ts) into a PR described as a pure `chore: migrate 8 named modules to TypeScript`; these are not among the 8 files chore(miner): migrate leaf-most, lowest-fan-in utility packages/loopover-miner/lib modules to TypeScript (batch 2.3 of 5 (Phase 2)) #7302 lists and are net-new functionality (chat dispatch chokepoint, governor pause/resume, portfolio release/requeue actions, an HTTP health server, and a snapshot→replay-task bridge), not straight ports of existing .js code — this is description≠diff scope-creep that needs to be split out and tied to its own issue(s).
  • None of the new logic introduced by the bundled feature modules (dispatchChatAction's flag/registry/validator branches, the portfolio/governor chat-action registration and gating, ams-health-server's readiness aggregation, replay-task-bridge's leakage-context/candidate builders) ships with any tests — the external evidence confirms 0 test-path changes across all 48 changed files, so these new branches have no coverage at all.
Nits — 6 non-blocking
  • `packages/loopover-miner/lib/prompt-injection-defense.ts:17,20` uses `whitelist` — prefer `allowlist` for consistent terminology.
  • Several files flagged for deep nesting (depth 5) such as `cross-repo-evaluation.ts:138` and `execute-local-write.ts:46` — consider extracting guard clauses to flatten.
  • `cross-repo-evaluation.ts` is ~458 lines, over the repo's informal 400-line file-size guideline; consider splitting the manifest-parsing helpers from the evaluation/report functions.
  • HTTP status codes in `ams-health-server.ts` (200/503/404) are magic numbers; naming them would aid readability though this is minor.
  • `deny-check.js:19`'s `parseDenyCheckArgs` is flagged at ~13 branches (threshold 10); worth a look for simplification but not blocking here.
  • Code changes lack test evidence — Add focused regression tests or explain why existing coverage is sufficient.

Why this is blocked

  • The diff bundles entirely new feature modules (chat-action-dispatch.ts, chat-governor-actions.ts, chat-portfolio-actions.ts, ams-health-server.ts, replay-task-bridge.ts) into a PR described as a pure `chore: migrate 8 named modules to TypeScript`; these are not among the 8 files chore(miner): migrate leaf-most, lowest-fan-in utility packages/loopover-miner/lib modules to TypeScript (batch 2.3 of 5 (Phase 2)) #7302 lists and are net-new functionality (chat dispatch chokepoint, governor pause/resume, portfolio release/requeue actions, an HTTP health server, and a snapshot→replay-task bridge), not straight ports of existing .js code — this is description≠diff scope-creep that needs to be split out and tied to its own issue(s).
📋 Copy for AI agents — paste into your coding agent
Fix the following blocker(s) from this PR review:

1. The diff bundles entirely new feature modules \(chat-action-dispatch.ts, chat-governor-actions.ts, chat-portfolio-actions.ts, ams-health-server.ts, replay-task-bridge.ts\) into a PR described as a pure \`chore: migrate 8 named modules to TypeScript\`; these are not among the 8 files \#7302 lists and are net-new functionality \(chat dispatch chokepoint, governor pause/resume, portfolio release/requeue actions, an HTTP health server, and a snapshot→replay-task bridge\), not straight ports of existing .js code — this is description≠diff scope-creep that needs to be split out and tied to its own issue\(s\).

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 #7302
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: 111 registered-repo PR(s), 53 merged, 4 issue(s).
Contributor context ✅ Confirmed Gittensor contributor joaovictor91123; Gittensor profile; 111 PR(s), 4 issue(s).
Improvement ℹ️ None detected risk: low · value: none · LLM: unclear
Linked issue satisfaction

Addressed
The PR converts all 8 listed lib modules from .js to .ts using the in-place-emit pattern, removes hand-written .d.ts siblings in favor of tsc-generated ones, and preserves existing behavior as verified against the shown cross-repo-evaluation example (import paths and test suite unchanged).

Review context
  • Author: joaovictor91123
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: Python, JavaScript, MDX, C++, CSS, Rust, TypeScript
  • Official Gittensor activity: 111 PR(s), 4 issue(s).
  • Related work: Titles/paths share 7 meaningful terms. (issue #7300)
  • Related work: Titles/paths share 7 meaningful terms. (issue #7301)
  • Related work: Titles/paths share 6 meaningful terms. (PR #7322)
  • 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 <question> answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat <question> 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 (Linked issue #7302 is assigned to the maintainer (@JSONbored) — that work is reserved for the maintainer, so this PR cannot be auto-accepted.). 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 19, 2026
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.3 of 5 (Phase 2))

1 participant