build(typescript): enable noUnusedLocals/noUnusedParameters repo-wide (#9553, #9570, #9571, #9572) by JSONbored · Pull Request #9573 · JSONbored/loopover

JSONbored · 2026-07-28T10:24:39Z

Closes #9553
Closes #9570
Closes #9571
Closes #9572

Summary

Dead code is the substrate every drift bug in the 2026-07-27 audit grew on: a stale import or an orphaned constant reads exactly like a live wire, so the next person greps, finds it, and reasons about a code path that no longer runs.

Enabling noUnusedLocals / noUnusedParameters made the compiler enumerate all 515 instances. Every one in src/** and packages/** was traced to its replacement before deletion — all 82 were genuine supersession leftovers, no behaviour bug hiding among them. But the triage surfaced three real problems that were invisible while the noise was tolerated.

The three findings (each its own issue)

#9570 — `requestedBy` dropped from backlog-convergence audit events

sweepRepoBacklogConvergence accepted requestedBy: "schedule" | "api" | "test" and never read it, while every sibling sweep stamps it into recordAuditEvent's metadata. Both agent.sweep.backlog_convergence events omitted it, so those records could not be attributed to a schedule vs a manual API trigger.

Worth calling out: the mechanical response to the compiler warning is to rename it _requestedBy — which cements the gap permanently instead of closing it. Wired into both events instead.

#9571 — the #9302 parity guard watches objects nothing registers

The REST↔MCP parity test asserted against src/mcp/server.ts's gatePrecisionOutputSchema / maintainerMeasurementReportOutputSchema. #9518 moved those tools' outputs to @loopover/contract, and the tools now register GetGatePrecisionOutput.shape / GetOutcomeCalibrationOutput.shape.

The shapes are still identical, so nothing had drifted yet — the defect is that the guard was watching objects no runtime reads, so a future contract change would have sailed past it. Re-anchored onto what the tools actually register, which the test file's own header comment already claimed it did.

#9572 — `normalizeResolveFindingRef` implemented twice

src/github/resolve-command.ts had no production importer, because src/review/review-memory-wire.ts carried a byte-identical second copy (regex included) and that copy was the one production used. Two independent implementations of the same public-safety validation, free to drift, with the canonical module kept alive only by its own test — exactly the issue-rag-wire.ts shape #9492's dead-source check exists to catch. Deduped via re-export.

Also fixed (pre-existing, since the tree has to be green for the flags to mean anything)

scripts/actionlint.ts imported a .ts specifier (TS5097)
test/unit/contract-registry.test.ts had three .js specifiers in a Bundler-resolved zone
check-dead-source-files-script.test.ts tripped check-import-specifiers on its own string fixtures — the same self-referential false positive that checker's ALLOWED_FILENAMES already documents for its own test

Plus packages/loopover-engine/src/scoring/preview.ts's header, which claimed a ReDoS guarantee via a hasUnsafeWildcardCount import that had been dead since 625e236b4. The guarantee is real and unchanged — it arrives through labelMatchesPattern — and the comment now says so.

Mechanics

Unused parameters are renamed with a leading underscore, never deleted. They are positional: removing one silently re-binds every later argument. tsc honours the underscore by design.
Everything else was removed by its real TypeScript AST node span. A regex pass was tried first and mis-bounded declarations badly enough to produce unparseable files; the AST version is what landed.

Verification

tsc clean with both flags on
Full suite: 23,894 passed, 0 failed
dead-source-files:check, import-specifiers:check, regate-sort-key:check, coverage-boltons:check, branding-drift:check — all green
engine parity: 15/15

loopover-orb · 2026-07-28T10:24:50Z

Warning

⏸️ LoopOver review result - manual review recommended

_{Review updated: 2026-07-28 18:37:50 UTC}

79 files · 1 AI reviewer · no blockers · CI green · clean

⏸️ Suggested Action - Manual Review

Review summary
This PR enables noUnusedLocals/noUnusedParameters repo-wide and cleans up 82 genuine dead-code instances across src/**, packages/**, and scripts/**, closing three linked issues (#9553, #9570, #9571, #9572) with well-scoped, individually-explained fixes. The three named findings are each traceable and correct on inspection: #9570 wires requestedBy into two recordAuditEvent metadata calls in src/queue/processors.ts, #9571 re-anchors the OpenAPI parity test onto the tools actually registered (GetGatePrecisionOutput.shape/GetOutcomeCalibrationOutput.shape) instead of dead server-local schemas, and #9572 deduplicates normalizeResolveFindingRef by re-exporting from the single reachable implementation in src/github/resolve-command.ts. The check-import-specifiers.ts change is a legitimate, well-tested carve-out for scripts/** files run under --experimental-strip-types, deriving its exempt set from package.json rather than hardcoding it, with dedicated regression/invariant tests.

Nits — 6 non-blocking

packages/loopover-engine/src/advisory/gate-advisory.ts:16 replaces `import type { CollisionReport }` with an empty `import type { } from "../types/predicted-gate-types.js"`, which is dead weight the same audit should have caught — worth removing entirely rather than leaving a no-op import.
The CI failures (validate-code, validate, validate-tests) are listed with no detail — given the scale of this change (77 files, large deletions across test files), it's worth confirming these failures aren't from the enabled noUnusedLocals/noUnusedParameters flags catching something the manual triage missed, though this can't be verified from what's given.
scripts/check-import-specifiers.ts:42's `9553` issue-number reference in a comment (not a magic number in logic) is fine as documentation, but if it's meant to be load-bearing it should be a named constant instead of embedded in prose.
Remove the now-empty `import type { }` in packages/loopover-engine/src/advisory/gate-advisory.ts:16 rather than leaving a vestigial import statement.
Since CI shows FAILED checks with no detail, confirm locally that `npm run typecheck` (or equivalent) passes clean with the new tsconfig flags before merging, since this PR's entire premise depends on that compiler pass being green.
PR author also opened the linked issue — Link an issue that was opened by a different contributor, or provide a rationale for why this self-authored issue represents genuine discovery work.

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	#9553, #9570, #9571, #9572
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 (4 linked issues).
Validation posture	✅ 25/25	PR body includes validation/test evidence.
Contributor workload	✅ 10/10	Author activity: 14 registered-repo PR(s), 13 merged, 300 issue(s).
Contributor context	✅ Confirmed Gittensor contributor	JSONbored; Gittensor profile; 14 PR(s), 300 issue(s).
Improvement	✅ Minor	risk: clean · value: minor

Linked issue satisfaction

Addressed
The diff enables noUnusedLocals/noUnusedParameters in the root tsconfig.json and shows extensive dead-code removal and underscore-prefixing of unused parameters across src/**, packages/**, and test files, matching the issue's core requirements; the PR description also documents triage findings (#9570-#9572) as required by deliverable of recording what was dead vs. renamed.

Review context

Author: JSONbored
Role context: owner (maintainer lane)
Public audience mode: oss maintainer
Lane context: Repository is registered but has no active allocation in the current snapshot.
Public profile languages: not available
Official Gittensor activity: 14 PR(s), 300 issue(s).
PR-specific overlap: none found.

Contributor next steps

Start here: Treat this as maintainer-lane context rather than normal contributor-lane activity.
Then work through the remaining 3 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.

Decision record

action: hold · clause: success
config: 065aec5ffc5f86972dd04ade4a58f5e21f11b9db7572e9d2d20f64a72c730869 · pack: oss-anti-slop · ci: passed
record: 96e4dcb0602f6a7059865e6d4de482e786da6b3327e1259b616bd8cea10551ce (schema v5, head fdd5069)

_{🟩 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

superagent-security · 2026-07-28T10:25:28Z

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

cloudflare-workers-and-pages · 2026-07-28T10:26:52Z

Deploying with Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	loopover-ui	`ab116ca`	Commit Preview URL Branch Preview URL	Jul 28 2026, 06:18 PM

codecov · 2026-07-28T10:43:56Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.83%. Comparing base (0e990a3) to head (ab116ca).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9573      +/-   ##
==========================================
- Coverage   89.84%   89.83%   -0.01%     
==========================================
  Files         875      875              
  Lines      110980   110951      -29     
  Branches    26402    26402              
==========================================
- Hits        99706    99678      -28     
+ Misses       9992     9991       -1     
  Partials     1282     1282

Flag	Coverage Δ
backend	`95.62% <100.00%> (+<0.01%)`	⬆️
engine	`65.92% <100.00%> (-0.02%)`	⬇️

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

Files with missing lines	Coverage Δ
...ages/loopover-engine/src/advisory/gate-advisory.ts	`79.22% <100.00%> (-0.03%)`	⬇️
packages/loopover-engine/src/config-lint.ts	`67.17% <ø> (-1.21%)`	⬇️
packages/loopover-engine/src/focus-manifest.ts	`79.84% <100.00%> (-0.02%)`	⬇️
packages/loopover-engine/src/scoring/preview.ts	`51.22% <100.00%> (+0.03%)`	⬆️
packages/loopover-engine/src/signals/engine.ts	`97.67% <ø> (ø)`
...opover-engine/src/signals/predicted-gate-engine.ts	`82.13% <ø> (-0.14%)`	⬇️
...ckages/loopover-miner/lib/cross-repo-evaluation.ts	`99.25% <ø> (ø)`
packages/loopover-miner/lib/portfolio-queue-cli.ts	`100.00% <ø> (ø)`
src/api/routes.ts	`95.63% <ø> (+0.03%)`	⬆️
src/db/repositories.ts	`96.85% <ø> (ø)`
... and 14 more

…#9553) Dead code is the substrate every drift bug in the 2026-07-27 audit grew on: a stale import or an orphaned constant reads exactly like a live wire, so the next person greps, finds it, and reasons about a code path that no longer runs. Enabling the flags made the compiler enumerate all 515 instances. Every one in src/** and packages/** was traced to its replacement before deletion -- all 82 were genuine supersession leftovers, no behaviour bug hiding among them -- but the triage turned up three real problems that were invisible under the noise: 1. src/queue/processors.ts -- sweepRepoBacklogConvergence accepted `requestedBy` ("schedule" | "api" | "test") and dropped it. Every sibling sweep stamps it into recordAuditEvent's metadata; both agent.sweep.backlog_convergence events here omitted it, so those records could not be attributed to a schedule vs a manual API trigger. Now wired into both. (Note the mechanical fix would have been to rename it `_requestedBy`, which cements the gap instead of closing it.) 2. test/unit/openapi.test.ts -- the #9302 REST<->MCP parity guard asserted against src/mcp/server.ts's gatePrecisionOutputSchema and maintainerMeasurementReportOutputSchema, which the tools stopped registering when #9518 moved their outputs to @loopover/contract. The shapes are still identical, so nothing had drifted YET -- but the guard was watching objects no runtime reads and would not have caught a future contract change. Re-anchored onto GetGatePrecisionOutput.shape / GetOutcomeCalibrationOutput.shape, which is what the tools actually register, and what that file's own header comment already claimed it did. 3. src/github/resolve-command.ts was reachable only from its own test, because src/review/review-memory-wire.ts carried a SECOND byte-identical copy of normalizeResolveFindingRef (regex included) and that copy was the one production used. Two independent implementations of the same public-safety validation, free to drift. Deduped onto the original via re-export; dead-source-files:check now passes on a file it was about to start failing on. Also corrects packages/loopover-engine/src/scoring/preview.ts's header, which claimed a ReDoS guarantee via a hasUnsafeWildcardCount import that had been dead since 625e236 deduped its label matching onto label-match.ts. The guarantee is real and unchanged; it now arrives through labelMatchesPattern, and the comment says so. Pre-existing import-specifier violations fixed in the same pass, since the tree has to be green for the flags to mean anything: - scripts/actionlint.ts imported a `.ts` specifier (TS5097) - test/unit/contract-registry.test.ts had three `.js` specifiers in a Bundler zone - check-dead-source-files-script.test.ts tripped check-import-specifiers on its own string FIXTURES, the same self-referential false positive that checker's ALLOWED_FILENAMES already documents for its own test Mechanics: unused parameters are renamed with a leading underscore, never deleted -- they are positional, so removing one silently re-binds every later argument. Everything else was removed by its real TypeScript AST node span (a regex pass was tried first and mis-bounded declarations badly enough to produce unparseable files). Full suite green: 23,894 passed, 0 failed. tsc clean with the flags on.

…e-advisory.ts check-engine-parity holds the two hand-duplicated gate-decision twins (packages/loopover-engine/src/advisory/gate-advisory.ts and src/rules/advisory.ts) in lockstep: touching one without the other requires an engine version bump. That is the mechanism, and it is doing its job here. the engine twin. They are genuinely dead there and NOT in the host: the engine copy is a deliberately slimmed re-implementation (#4881) that omits buildIssueAdvisory / addIssueFindings / collisionClustersForPull, which are what use those types on the host side. So there is no matching host edit to make -- the asymmetry is correct, and the version bump is the sanctioned way to record it. No behaviour change: type-only imports are erased at compile time. The bump exists so the parity contract stays enforceable, not because the gate decides anything differently. packages/loopover-miner/expected-engine.version moves in lockstep, as its own check requires.

…d shape #9565 added Two rebase follow-ups after #9565 and #9574 landed. 1. scripts/actionlint.ts gets its `.ts` extension back. This PR had removed it to satisfy check-import-specifiers, which broke the script outright -- it runs under `node --experimental-strip-types`, whose ESM resolver does no extension resolution, so the process dies at startup with ERR_MODULE_NOT_FOUND. #9565 independently reached the same conclusion and added TYPE_STRIPPED_ENTRYPOINTS to the checker for exactly this file, so the extension is now permitted where it is required. Verified by running `npm run actionlint`, which fails before this change and passes after. #9565's version of the checker is taken wholesale over this PR's: it solves the same two problems (that entrypoint set, plus allowlisting check-dead-source-files-script.test.ts for its string fixtures), and re-litigating a file main just rewrote buys nothing. 2. src/mcp/server.ts's `loginRepoPullShape` is removed -- dead on arrival in #9565, and the first thing the newly-enabled noUnusedLocals caught on main. Which is the point of this PR: dead code now surfaces at the commit that introduces it rather than at the next audit. The engine bump lands at 3.16.1 (main released 3.16.0 while this was open). It is required by check-engine-parity: this PR removes two dead TYPE-only imports from packages/loopover-engine/src/advisory/gate-advisory.ts, and the parity contract holds that file in lockstep with its host twin src/rules/advisory.ts. There is no matching host edit to make -- the engine copy is a deliberately slimmed re-implementation (#4881) omitting the functions that use those types -- so the version bump is the sanctioned way to record a one-sided change. No behaviour change: type-only imports are erased at compile time. packages/loopover-miner/expected-engine.version moves with it, as its own check requires.

…ne bump The manifest is a generated artifact that must move with any package.json version, and release-manifest:sync:check fails CI when it drifts. Regenerated with the repo's own `npm run release-manifest:sync` rather than hand-edited.

Rebase onto main after #9579 and #9580 landed. The newly-enabled noUnusedLocals immediately flagged twelve dead symbols in code merged since this PR opened -- including two I left in #9580 myself: - src/queue/processors.ts: PrCommandPrologueOutcome imported but unused (the adapter's annotated return type was dropped in favour of inference), plus eight unused bindings in the prologue destructures -- handlers that do not need `pr`, `settings`, `authorization` or `command` were still pulling them out. - src/queue/pr-command-prologue.ts: LoopOverMentionCommandName, superseded by LoopOverActionCommandName once the spec narrowed to action verbs. - src/mcp/dispatch-telemetry-sink.ts: an unused McpToolCallTelemetry import from #9579. Which is the point of the PR: the flags catch dead code at the commit that introduces it rather than at the next audit. Zero behaviour change -- every removal is a binding or an import TypeScript proved unreferenced, and the suite passes 24,014 tests. The rebase conflict itself was in processors.ts's import block: main added the pr-command-prologue import on the same lines this PR removed the unused runRetentionPrune one. Both intents kept.

JSONbored force-pushed the chore/unused-locals-9553 branch from 8c9fca6 to a63a16e Compare July 28, 2026 10:43

loopover-orb Bot assigned JSONbored Jul 28, 2026

loopover-orb Bot added the manual-review Gittensor contributor context label Jul 28, 2026

JSONbored mentioned this pull request Jul 28, 2026

fix(orb): retire the orphaned review_targets ledger from every live reader (#9136, #9576, #9577) #9578

Merged

JSONbored force-pushed the chore/unused-locals-9553 branch from 60ccaf8 to ddc6bdf Compare July 28, 2026 11:59

JSONbored added 5 commits July 28, 2026 11:09

JSONbored force-pushed the chore/unused-locals-9553 branch from fdd5069 to ab116ca Compare July 28, 2026 18:15

JSONbored merged commit 1943d88 into main Jul 28, 2026
8 checks passed

JSONbored deleted the chore/unused-locals-9553 branch July 28, 2026 18:38

This was referenced Jul 29, 2026

chore(release): cut orb-v3.6.0 #9871

Merged

MCP release due: 3.15.2 #8638

Open

Miner release due: 3.15.2 #8639

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

build(typescript): enable noUnusedLocals/noUnusedParameters repo-wide (#9553, #9570, #9571, #9572) - #9573

build(typescript): enable noUnusedLocals/noUnusedParameters repo-wide (#9553, #9570, #9571, #9572)#9573
JSONbored merged 5 commits into
mainfrom
chore/unused-locals-9553

JSONbored commented Jul 28, 2026

Uh oh!

loopover-orb Bot commented Jul 28, 2026 •

edited

Loading

⏸️ LoopOver review result - manual review recommended

Uh oh!

superagent-security Bot commented Jul 28, 2026

Uh oh!

cloudflare-workers-and-pages Bot commented Jul 28, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Jul 28, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

JSONbored commented Jul 28, 2026

Summary

The three findings (each its own issue)

#9570 — requestedBy dropped from backlog-convergence audit events

#9571 — the #9302 parity guard watches objects nothing registers

#9572 — normalizeResolveFindingRef implemented twice

Also fixed (pre-existing, since the tree has to be green for the flags to mean anything)

Mechanics

Verification

Uh oh!

loopover-orb Bot commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⏸️ LoopOver review result - manual review recommended

Uh oh!

superagent-security Bot commented Jul 28, 2026

Uh oh!

cloudflare-workers-and-pages Bot commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deploying with Cloudflare Workers

Uh oh!

codecov Bot commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

#9570 — `requestedBy` dropped from backlog-convergence audit events

#9572 — `normalizeResolveFindingRef` implemented twice

loopover-orb Bot commented Jul 28, 2026 •

edited

Loading

cloudflare-workers-and-pages Bot commented Jul 28, 2026 •

edited

Loading

codecov Bot commented Jul 28, 2026 •

edited

Loading