Skip to content

Commit 1943d88

Browse files
authored
build(typescript): enable noUnusedLocals/noUnusedParameters repo-wide (#9553, #9570, #9571, #9572) (#9573)
* build(typescript): enable noUnusedLocals/noUnusedParameters repo-wide (#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. * chore(engine): bump to 3.15.4 for the dead type-import removal in gate-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. * fix(scripts,mcp): restore actionlint's `.ts` specifier and drop a dead 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. * chore(release): sync .release-please-manifest.json to the 3.16.1 engine 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. * chore: prune the dead symbols the new flags caught in newly-merged code 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.
1 parent 423f410 commit 1943d88

79 files changed

Lines changed: 147 additions & 1001 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.release-please-manifest.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"packages/loopover-mcp": "3.15.2",
3-
"packages/loopover-engine": "3.16.0",
3+
"packages/loopover-engine": "3.16.1",
44
"packages/loopover-miner": "3.15.2",
55
"packages/loopover-ui-kit": "1.2.0"
66
}

packages/loopover-engine/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@loopover/engine",
3-
"version": "3.16.0",
3+
"version": "3.16.1",
44
"license": "AGPL-3.0-only",
55
"type": "module",
66
"description": "Shared deterministic engine logic for the LoopOver review stack and loopover-miner.",

packages/loopover-engine/src/advisory/gate-advisory.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,10 @@ import type {
1313
AdvisoryFinding,
1414
AdvisorySeverity,
1515
GateRuleMode,
16-
IssueRecord,
1716
PullRequestRecord,
1817
RepositoryRecord,
1918
} from "../types/predicted-gate-types.js";
20-
import type { CollisionReport } from "../types/predicted-gate-types.js";
19+
import type { } from "../types/predicted-gate-types.js";
2120
import { isDuplicateClusterWinnerByClaim } from "../signals/duplicate-winner.js";
2221
import type { GuardrailPathMatch } from "../signals/change-guardrail.js";
2322
import { nowIso } from "../utils/json.js";

packages/loopover-engine/src/config-lint.ts

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,11 +51,6 @@ function recognizedFieldsFor(text: string | null | undefined): string[] {
5151
);
5252
}
5353

54-
// Fields retired from TOP_LEVEL_FIELDS that still warrant a migration-specific warning (rather than the
55-
// generic "unknown field" message) pointing operators at their replacement mechanism.
56-
const RETIRED_FIELD_MIGRATION_WARNINGS: Record<string, string> = {
57-
blockedPaths: "blockedPaths is retired; use settings.hardGuardrailGlobs for path holds.",
58-
};
5954

6055
// #9167: gate.mergeReadiness is a composite that only FILLS IN a sub-gate mode the operator left UNSET
6156
// (src/rules/advisory.ts's applyMergeReadinessGate, and its engine twin) -- it never overrides an

packages/loopover-engine/src/focus-manifest.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,25 +28,21 @@ import { normalizeAutonomyPolicy, normalizeAutoMaintainPolicy } from "./settings
2828
import { normalizeCommandAuthorizationPolicy } from "./settings/command-authorization.js";
2929
import { normalizeContributorBlacklist } from "./settings/contributor-blacklist.js";
3030
import { normalizeAutoCloseExemptLogins } from "./settings/auto-close-exempt.js";
31-
import { DEFAULT_TYPE_LABELS, MAX_TYPE_LABEL_NAME_LENGTH, normalizeTypeLabelSet } from "./settings/pr-type-label.js";
31+
import { MAX_TYPE_LABEL_NAME_LENGTH, normalizeTypeLabelSet } from "./settings/pr-type-label.js";
3232
import {
33-
DEFAULT_LINKED_ISSUE_LABEL_PROPAGATION,
3433
normalizeLinkedIssueLabelPropagationConfig,
3534
VALID_LINKED_ISSUE_LABEL_PROPAGATION_MODES,
3635
} from "./review/linked-issue-label-propagation.js";
3736
import {
38-
DEFAULT_LINKED_ISSUE_HARD_RULES,
3937
isLinkedIssueHardRuleMode,
4038
normalizeLinkedIssueHardRulesConfig,
4139
} from "./review/linked-issue-hard-rules-config.js";
4240
import {
43-
DEFAULT_UNLINKED_ISSUE_GUARDRAIL,
4441
isUnlinkedIssueGuardrailMode,
4542
normalizeUnlinkedIssueGuardrailConfig,
4643
} from "./review/unlinked-issue-guardrail-config.js";
4744
import { normalizeAdvisoryAiRoutingConfig } from "./review/advisory-ai-routing-config.js";
4845
import {
49-
DEFAULT_SCREENSHOT_TABLE_GATE,
5046
isScreenshotTableGateAction,
5147
normalizeScreenshotTableGateConfig,
5248
} from "./review/screenshot-table-gate.js";

packages/loopover-engine/src/scoring/preview.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import type { ContributorEvidenceRecord, JsonValue, RepositoryRecord, RepoTimeDecayOverrides, ScoringModelSnapshotRecord, ScorePreviewRecord } from "./types.js";
22
import { DEFAULT_SCORING_CONSTANTS } from "./model.js";
3-
import { hasUnsafeWildcardCount } from "../signals/change-guardrail.js";
43
import {
54
clearLabelPatternRegExpCacheForTest,
65
LABEL_PATTERN_REGEXP_CACHE_MAX_ENTRIES,
@@ -11,10 +10,12 @@ import {
1110
// Deterministic score-preview builder extracted verbatim from the backend's `src/scoring/preview.ts`
1211
// (#2282) — this file has no D1/network/env dependency in the original, so it ports unchanged aside from
1312
// its imports and one tiny pure helper (`nowIso`) inlined below, which the backend sources from
14-
// `src/utils/json.ts`. `hasUnsafeWildcardCount` is imported from this package's own
15-
// `signals/change-guardrail.ts` (#4611) rather than re-derived here — that file is a verbatim port of the
16-
// backend's `src/signals/change-guardrail.ts`, kept in sync by the engine-parity contract test, so importing
17-
// it carries the same ReDoS-safety guarantee without a third hand-maintained copy.
13+
// `src/utils/json.ts`.
14+
//
15+
// The ReDoS-safety guarantee this file used to claim via a direct `hasUnsafeWildcardCount` import (#4611) now
16+
// arrives through `labelMatchesPattern` (./label-match.ts), which applies the same cap internally — the import
17+
// here had been dead since 625e236b4 deduped this file's label matching onto that module. The guarantee is
18+
// unchanged; only the route to it is, and stating the live route is the point of saying so at all.
1819

1920
// The package's tsconfig sets `types: []` (no ambient DOM/Node globals, keeping the engine's type surface
2021
// independent of any consumer's lib config), so the Web Crypto global needs a minimal local declaration.

packages/loopover-engine/src/signals/engine.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,22 +25,21 @@ import type {
2525
ScoringModelSnapshotRecord,
2626
} from "../../../../src/types.js";
2727
import type { PublicContributorProfile } from "../../../../src/github/public.js";
28-
import { commandReferenceUrl, loopoverFooter, gittensorRepoEarnUrl, type LoopOverFooterEnv } from "../../../../src/github/footer.js";
28+
import { commandReferenceUrl, type LoopOverFooterEnv } from "../../../../src/github/footer.js";
2929
import type { FocusManifestReviewConfig, ReviewFieldKey } from "../../../../src/signals/focus-manifest.js";
3030
import type { GittensorContributorSnapshot } from "../../../../src/gittensor/api.js";
3131
import { nowIso } from "../utils/json.js";
3232
import { extractLinkedIssueNumbers } from "../../../../src/db/repositories.js";
3333
import { sanitizePublicComment } from "../../../../src/queue-intelligence.js";
3434
import { labelMatchesPattern, projectLinkedIssueMultiplierForPlannedSolve, type LinkedIssueMultiplierStatus } from "../scoring/preview.js";
3535
import { isSuspiciousConfiguredLabel } from "../scoring/label-match.js";
36-
import { hasLocalTestEvidence, hasValidationNote, isTestPath } from "./test-evidence.js";
36+
import { hasLocalTestEvidence, hasValidationNote, } from "./test-evidence.js";
3737
import { isCodeFile, isTestFile } from "./path-matchers.js";
3838
import { isFailingCheckSummary } from "./check-summary.js";
3939
import { isDuplicateClusterWinnerByClaim } from "./duplicate-winner.js";
4040
import { PREFLIGHT_LIMITS } from "./preflight-limits.js";
4141
import type { UnifiedCollapsible } from "../../../../src/review/unified-comment.js";
42-
import { splitAiReviewNits } from "../../../../src/review/ai-notes.js";
43-
import { LOOPOVER_GATE_CHECK_NAME, shouldPublishReviewCheck } from "../../../../src/review/check-names.js";
42+
import { shouldPublishReviewCheck } from "../../../../src/review/check-names.js";
4443
import { isAgentConfigured } from "../settings/autonomy.js";
4544
import { diffFilePriority } from "../review/diff-file-priority.js";
4645
import type { ImprovementBand, StructuralImprovementAssessment } from "../../../../src/signals/improvement.js";

packages/loopover-engine/src/signals/predicted-gate-engine.ts

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import type {
2-
AdvisoryFinding,
32
BountyLifecycle,
43
BountyRecord,
54
CollisionCluster,
@@ -46,13 +45,6 @@ const STOPWORDS = new Set([
4645
const MAX_COLLISION_PAIRWISE_ISSUES = 80;
4746
const MAX_COLLISION_PAIRWISE_PULL_REQUESTS = 120;
4847
const MAX_COLLISION_PAIRWISE_RECENT_MERGES = 40;
49-
const ISSUE_DISCOVERY_LIFECYCLE_REPORT_CAP = 300;
50-
const ISSUE_QUALITY_REPORT_CAP = 100;
51-
const REPO_OUTCOME_STALE_OPEN_DAYS = 30;
52-
const REPO_OUTCOME_MIN_DECIDED_SAMPLE = 3;
53-
const REPO_OUTCOME_MERGE_WELL_RATE = 0.7;
54-
const REPO_OUTCOME_CLOSURE_RISK_RATE = 0.34;
55-
const REPO_OUTCOME_MAX_PATTERNS = 12;
5648

5749
export function buildLaneAdvice(repo: RepositoryRecord | null, fullName: string): LaneAdvice {
5850
const config = repo?.registryConfig;
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
3.16.0
1+
3.16.1

packages/loopover-miner/lib/cross-repo-evaluation.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ function resolveEvaluationRepoPath(
282282
return resolveRepoCloneDir(entry.repoFullName, options.env ?? process.env);
283283
}
284284

285-
function defaultClaimLedger(repoFullName: string): { listClaims: () => never[] } {
285+
function defaultClaimLedger(_repoFullName: string): { listClaims: () => never[] } {
286286
return { listClaims: () => [] };
287287
}
288288

0 commit comments

Comments
 (0)