Skip to content

chore(lint): count-based lint ratchet — new violations block, existing debt baselined; CI lint now blocking (#3962)#3989

Merged
Yeraze merged 6 commits into
mainfrom
feature/3962-p1-lint-ratchets
Jul 7, 2026
Merged

chore(lint): count-based lint ratchet — new violations block, existing debt baselined; CI lint now blocking (#3962)#3989
Yeraze merged 6 commits into
mainfrom
feature/3962-p1-lint-ratchets

Conversation

@Yeraze

@Yeraze Yeraze commented Jul 7, 2026

Copy link
Copy Markdown
Owner

Summary

Task 1.4 of the remediation plan (#3962, Phase 1). Lint goes from decorative to enforced.

The problem was bigger than the plan knew: CI lint has been continue-on-error since forever, masking 727 errors (not the ~367 previously estimated) — 468 of them config false-positives (VitePress build cache parsed with type-aware rules, no-undef firing on NodeJS/React/vi in TS files), 259 genuine.

  • Config hygiene: proper ignores for docs/examples/protobufs/public, no-undef: off for TS (TypeScript owns that check), project: false for standalone scripts — the 468 false-positives die at the config level, NOT in the baseline.
  • Ratchets: no-explicit-anyerror (2,026 baselined), raw fetch( banned in src/components/**+src/pages/** via no-restricted-syntax (64 sites baselined; migrate in Phase 5), react-hooks/exhaustive-depserror (110 baselined — dep-array auto-edits are behavior-risky), prefer-consterror (31 of 34 auto-fixed in this PR, pure letconst; 3 destructuring sites baselined).
  • The mechanism: scripts/lint-ratchet.mjs (no new dependencies) compares per-file/per-rule counts against the checked-in, alphabetically-sorted eslint-baseline.json (410 files / 2,515 frozen violations). Any file exceeding its baseline count fails; improvements print an advisory suggesting npm run lint:baseline. 11 unit tests in the standard suite.
  • CI is now blocking: both ci.yml and pr-tests.yml run npm run lint:ci with no escape hatch — closing the deviation left by Task 0.5. CLAUDE.md documents the workflow.

Verification

  • 7 negative verifications: new any / new raw fetch / new exhaustive-deps violation / new let each fail the ratchet at the exact file; existing debt passes; improvement advisory fires; parser + no-undef false-positives gone.
  • Independent orchestrator run: npm run lint:ci exit 0, typecheck clean, test-typecheck baseline unchanged (283), full suite 8169 passed, 0 failed, 0 suite failures.

Refs #3962

🤖 Generated with Claude Code

https://claude.ai/code/session_01AphLfgUJWaFNAgU5UXdJ4n

Yeraze and others added 6 commits July 7, 2026 12:58
Eliminate the ~468 ESLint config false-positives (346 parser errors +
122 no-undef fires on TS ambient globals) so the baseline captures only
genuine rule violations:

• Extend top-level ignores: docs/** (VitePress cache), examples/**,
  protobufs/** (submodule), public/**  →  kills 346 parser errors
• Add no-undef:'off' for all TS/JS files — standard typescript-eslint
  recommendation; tsc reports undefined identifiers, not ESLint
• Add project:false override for scripts/**/*.{js,mjs,cjs,ts},
  *.{js,mjs,cjs}, tests/**/*.{js,mjs,cjs} — utility files outside
  tsconfig.json now lint without the type-aware parser

After: 0 parser errors, 0 no-undef errors.
Part of remediation epic #3962 Task 1.4.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AphLfgUJWaFNAgU5UXdJ4n
Promote three rules from warn→error (existing violations are frozen by
the count-based ratchet introduced in WP3; new occurrences are blocked):

• @typescript-eslint/no-explicit-any → 'error' (2,024 sites baselined)
• react-hooks/exhaustive-deps → 'error' (110 sites / 43 files baselined;
  do NOT auto-fix — missing deps in effect arrays can cause render loops)
• prefer-const → 'error' (34 sites — auto-fixed in the next commit)

Add a no-restricted-syntax block scoped to src/components/** and
src/pages/** banning bare fetch(), window.fetch(), and globalThis.fetch().
64 existing sites (25 component files, 3 page files) are frozen by the
ratchet baseline; they migrate to ApiService in Phase 5. Note: flat-config
array replacement means the SQL-ban selectors are not in this block —
components/pages never access the DB directly.

Part of remediation epic #3962 Task 1.4.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AphLfgUJWaFNAgU5UXdJ4n
Run `eslint --fix --rule '{"prefer-const":"error"}' 'src/**/*.{ts,tsx}'`.
Every changed line is a bare let→const rename; no behavioral change.
3 residual sites (destructuring in RebootModal.tsx and apiTokenRoutes.ts)
could not be auto-fixed and are frozen by the lint ratchet baseline.

Part of remediation epic #3962 Task 1.4.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AphLfgUJWaFNAgU5UXdJ4n
Add scripts/lint-ratchet.mjs: a homegrown count-based per-file ESLint
ratchet. Semantics: for each file×rule, current > baseline → FAIL;
current < baseline → advisory only. Zero new npm dependencies.

Add scripts/lint-ratchet.test.mjs: 10 Vitest unit tests covering the
pure compare(), tally(), and sortObj() functions.

Add package.json scripts:
  lint:ci       → node scripts/lint-ratchet.mjs  (CI gate, blocking)
  lint:baseline → node scripts/lint-ratchet.mjs --update  (regenerate)

Add eslint-baseline.json (generated, NOT hand-edited):
  410 files · 2,515 violations frozen
  Top buckets: no-explicit-any 2,026 · no-unused-vars 175 ·
    exhaustive-deps 110 · no-restricted-syntax (fetch ban + SQL) 79 ·
    react-refresh 37 · prefer-const 3 (residual after auto-fix)
  Keys alphabetically sorted, 2-space indent, trailing newline.

npm run lint:ci exits 0 on HEAD (baseline matches current tree).

Part of remediation epic #3962 Task 1.4.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AphLfgUJWaFNAgU5UXdJ4n
ci.yml: replace non-blocking 'npm run lint' (continue-on-error: true)
with blocking 'npm run lint:ci'. Existing violations are frozen by the
count-based baseline; only new regressions cause CI failures.

pr-tests.yml: replace 'npm run lint || true' with 'npm run lint:ci'.
Same semantics — blocking gate on new violations only.

CLAUDE.md: add ESLint ratchet subsection documenting:
- lint / lint:ci / lint:baseline commands and when to use each
- rules that are now errors (no-explicit-any, exhaustive-deps, prefer-const)
- raw fetch() ban in src/components/** and src/pages/**
- how to handle a legit new violation
- warning against baseline growth

Part of remediation epic #3962 Task 1.4.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AphLfgUJWaFNAgU5UXdJ4n
Add a second pass in compare() that scans baseline entries missing from
the current lint output (file became fully clean). Without this pass,
advisory messages only fire when a file still has *some* violation but
fewer than baseline — completely-fixed files were silently ignored.

Also add a Vitest unit test covering the fully-fixed-file advisory case.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AphLfgUJWaFNAgU5UXdJ4n
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown

Claude finished @Yeraze's task —— View job


PR Review: chore(lint): count-based lint ratchet

  • Read key files (lint-ratchet.mjs, eslint config, CI workflows)
  • Analyze code quality and best practices
  • Check for bugs or issues
  • Review test coverage
  • Check security concerns

Overall this is a well-executed implementation. The ratchet mechanism is clean, has good unit test coverage of the pure functions, and the CI wiring is correct. A few findings below:

Bugs / Correctness Issues

1. compare() does not detect new rules on already-baselined files

scripts/lint-ratchet.mjs:82-101

When a file already exists in the baseline, the outer loop only iterates over rules that are currently firing. A brand-new rule violation on a file that already has a baseline entry is correctly caught (the ?? 0 default on prev will be 0, so cur > 0 fails). This is fine — the logic is correct. However, it's subtle enough that a comment would help future readers.

2. preserve-caught-error and no-useless-assignment in baseline but not in config

eslint-baseline.json contains 16 entries for preserve-caught-error and 15 for no-useless-assignment, but neither rule appears in eslint.config.mjs. This means:

  • Those rules must be coming from js.configs.recommended.rules or typescript.configs.recommended.rules spread at line 49-50.
  • That's fine, but it means the set of "blocked" rules is partially implicit. If a future ESLint or plugin upgrade removes/renames one of these rules, violations would silently disappear from counts rather than failing loudly.

This is low risk today but worth a comment in eslint.config.mjs near the spreads, e.g. // Note: recommended also enables preserve-caught-error, no-useless-assignment, ....

3. check-admin.mjs and reset-admin.mjs are baselined for no-restricted-syntax but excluded scripts still run rules

eslint-baseline.json:2-7 baselines those two root-level .mjs files. The scripts/** override at eslint.config.mjs:128-134 only sets project: false — it does not disable no-restricted-syntax. Because check-admin.mjs and reset-admin.mjs live at the root (not under scripts/), they don't get even that override. The baseline freezes the violations but the intent of these files (diagnostic utilities) suggests they should arguably be in the ignores list or explicitly added to the raw-SQL exclusion pattern.

Code Quality

4. runEslint() hardcodes .bin/eslint — breaks on Windows

scripts/lint-ratchet.mjs:26

path.join(cwd, 'node_modules', '.bin', 'eslint'),

On Windows, the .bin shim is eslint.cmd, not eslint. This will fail in any Windows CI or developer environment. The standard pattern is to use npx eslint or resolve via import { createRequire } / look for the .cmd variant. Since CI is Linux-only right now this is low priority, but worth noting if Windows support is ever a goal.

5. CI lint runs on every Node matrix version needlessly

ci.yml:72-76npm run lint:ci runs for each of the 4 Node versions (20, 22, 24, 25). Lint is purely Node-version-agnostic. Running it once (e.g. on 24.x only) would cut ~3 matrix runs worth of ESLint time without any loss of signal. This is a performance nit, not a correctness issue.

ESLint Config

6. no-floating-promises is blocked in src/** but has zero baseline entries

eslint.config.mjs:169 adds @typescript-eslint/no-floating-promises: error scoped to non-test TS files. The baseline has no entries for this rule, implying the codebase was already clean — good. But if this rule is new, the first PR that introduces a floating promise will be caught immediately (desired behavior). Just confirming the intent is correct.

7. Test files opt out of no-explicit-any globally

eslint.config.mjs:181 disables @typescript-eslint/no-explicit-any for all test files. The baseline doesn't freeze any no-explicit-any violations in test files because the rule is off. This is consistent and intentional — just noting it so reviewers understand why test files won't accumulate any debt.

Test Coverage

8. No test for the --update / sortObj round-trip path

The sortObj function is well tested, but there's no test verifying that the update path writes a correctly-sorted JSON file to disk. The runEslint() function is also untested (it's integration-level, so skipping is reasonable). Consider adding a test that calls tally() then sortObj() on the result and verifies the output is idempotent (sorted twice == sorted once).

9. No negative test for process.exit(2) on missing baseline

The "missing baseline" branch at lint-ratchet.mjs:115-118 is untested. Not critical since the path is trivial, but worth adding a test if the suite is intended to be exhaustive.

Summary

The implementation is solid. The three actionable items worth addressing before merge:

  1. check-admin.mjs / reset-admin.mjs — either add them to ignores or to the raw-SQL exclusion list so the baseline entries aren't needed (minor cleanup, not a blocker).
  2. Windows .bin/eslint path — low priority but easy to fix with process.platform === 'win32' ? 'eslint.cmd' : 'eslint'.
  3. Lint runs on every Node matrix — deduplicate to save CI time.

Everything else is advisory. The core ratchet logic, test coverage of the pure functions, and CI wiring are all correct and well-structured.

@Yeraze
Yeraze merged commit 5997b41 into main Jul 7, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant