ci(lint): ratchet as-any / @ts-ignore / eslint-disable counts#2658
ci(lint): ratchet as-any / @ts-ignore / eslint-disable counts#2658jordan-simonovski wants to merge 2 commits into
Conversation
Adds scripts/ci/ratchet.mjs, which counts tracked escape hatches (as any, @ts-ignore, eslint-disable) per package and compares them against a committed baseline. Counts above baseline fail CI (a new occurrence was added); counts below baseline also fail, prompting the contributor to run yarn ratchet:update so the improvement is locked in. This two-way-strict behaviour keeps the baseline honest and stops the escape-hatch count from silently creeping up. The check is wired into the ci-lint Makefile target so the existing lint CI job enforces it, and yarn ratchet / yarn ratchet:update scripts are exposed for local use.
|
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
github-actions
Bot
added
the
review/tier-2
🔵 Tier 2 — Low RiskSmall, isolated change with no API route or data model modifications. Why this tier:
Review process: AI review + quick human skim (target: 5–15 min). Reviewer validates AI assessment and checks for domain-specific concerns. Stats
|
Greptile SummaryThis PR introduces a CI ratchet that counts type-safety escape hatches (
Confidence Score: 5/5No production code is touched; the ratchet is a standalone CI script that only reads source files and exits non-zero on regressions. All changes are confined to a new CI utility script, its baseline snapshot, and two wiring lines. The script correctly handles missing directories, zero-match grep exits, and the below-baseline non-fatal path. The Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[yarn ratchet / make ci-lint] --> B{--update flag?}
B -- Yes --> C[Count patterns in each package src/]
C --> D[Write ratchet-baseline.json]
D --> E[Exit 0]
B -- No --> F[Count patterns in each package src/]
F --> G[Read ratchet-baseline.json]
G --> H{For each pkg + pattern}
H --> I{now > baseline?}
I -- Yes --> J[console.error + set failed=true]
I -- No --> K{now < baseline?}
K -- Yes --> L[console.warn non-fatal]
K -- No --> M[OK, continue]
J --> H
L --> H
M --> H
H -- Done --> N{failed?}
N -- Yes --> O[Exit 1 CI fails]
N -- No --> P[Exit 0 ratchet ok]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[yarn ratchet / make ci-lint] --> B{--update flag?}
B -- Yes --> C[Count patterns in each package src/]
C --> D[Write ratchet-baseline.json]
D --> E[Exit 0]
B -- No --> F[Count patterns in each package src/]
F --> G[Read ratchet-baseline.json]
G --> H{For each pkg + pattern}
H --> I{now > baseline?}
I -- Yes --> J[console.error + set failed=true]
I -- No --> K{now < baseline?}
K -- Yes --> L[console.warn non-fatal]
K -- No --> M[OK, continue]
J --> H
L --> H
M --> H
H -- Done --> N{failed?}
N -- Yes --> O[Exit 1 CI fails]
N -- No --> P[Exit 0 ratchet ok]
Reviews (2): Last reviewed commit: "ci(lint): soften below-baseline ratchet ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| '../..', | ||
| ); | ||
| const BASELINE = path.join(ROOT, 'scripts/ci/ratchet-baseline.json'); | ||
| const PACKAGES = ['app', 'api', 'common-utils', 'cli', 'hdx-eval']; |
There was a problem hiding this comment.
PACKAGES lists every tracked package by name. When a new package is added to the monorepo it will be silently skipped until someone remembers to update both this array and ratchet-baseline.json. Auto-discovering packages from the packages/ directory (e.g. reading its subdirectories or package.json workspaces) would make the ratchet self-updating, though it would also require that new packages always start with a baseline entry.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Intentional for now: the explicit PACKAGES list is mirrored 1:1 by the keys in ratchet-baseline.json, so the two stay in lockstep and a new package is a deliberate, reviewed addition rather than silently folded in. Auto-discovery via packages/*/src is a reasonable follow-up but would need the baseline generation to track it too — out of scope for this PR.
| 'ts-ignore': '@ts-ignore', | ||
| 'eslint-disable': 'eslint-disable', | ||
| }; |
There was a problem hiding this comment.
eslint-disable pattern matches eslint-disable-next-line and eslint-disable-line
Because grep -Eo does prefix matching, the literal eslint-disable pattern matches all three directive forms (block-disable, eslint-disable-next-line, eslint-disable-line) and counts each separately. This is probably the intended behaviour — all three are escape hatches — but it means the reported eslint-disable count is not the number of block-level suppressions, which may surprise maintainers trying to interpret it. A brief comment clarifying this is intentional would help.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Intentional — block eslint-disable, eslint-disable-next-line and eslint-disable-line are all escape hatches we want the ratchet to count, so prefix matching catching all three is the desired behaviour, not a bug.
Deep ReviewThis PR adds a CI ratchet ( ✅ No critical issues found. 🟡 P2 — recommended
🔵 P3 nitpicks (3)
Reviewers (3): correctness, maintainability, testing. Testing gaps: No test asserts prose substrings ("has anything") are excluded from counts; no test covers the |
E2E Test Results✅ All tests passed • 241 passed • 3 skipped • 1602s
Tests ran across 4 shards in parallel. |
|
Thanks — actioned in 7b7ae02:
Deliberately not changed (dev-only gate, keeping it simple):
The Makefile-vs-nx point (ratchet stops running if the lint job goes nx-direct) is real and tracked against the sister lint PR (#2661) — whichever merges second adds an explicit ratchet step. |
|
Folded into #2661 (combined lint + ratchet PR) via cherry-pick, with the ratchet now wired as an explicit lint-job step so it survives the nx-direct lint change. Closing to reduce merge complexity between the two. |
1 participant
Type-safety escape hatches (
as any,@ts-ignore,eslint-disable) accumulate silently — nothing stops the count creeping up over time. This adds a ratchet that fails CI when a package's count of any of those rises above a committed baseline, so they can only trend down.What changed
scripts/ci/ratchet.mjscounts the three patterns per package and compares againstscripts/ci/ratchet-baseline.json.yarn ratchet(check) andyarn ratchet:update(re-baseline).make ci-lintso the existinglintCI job enforces it.Key decisions
yarn ratchet:update. This forces improvements to be locked into the baseline rather than silently drifting, so the baseline always reflects reality.Impact
as any/@ts-ignore/eslint-disablefails thelintjob until it's removed or the baseline is deliberately updated.lintCI job to callnxdirectly instead ofmake ci-lint. Whichever of these two PRs merges second must add an explicit- run: node scripts/ci/ratchet.mjsstep to thelintjob — otherwise the ratchet stops running in CI (it's a whole-repo check, so it must run unconditionally, not gated bynx affected).Implementation detail
grepexit code 1 (no matches) is treated as count 0, not an error.const x = 1 as any;to apackages/apisource file makesyarn ratchetexit 1 namingapi/as-any; reverting restores "ratchet ok".