Skip to content

Commit 6fac85b

Browse files
committed
chore(checks): add local quality checks and CI ratchets
Adds scripts/checks: complexity, duplication and convention ratchets that the pre-commit hook runs on staged content and that CI runs against the base of the pull request, so a change can only lower the debt it touches. Extends lint-staged and the lint script to the scripts directory. The eslint config gains the segmentation feature boundary ahead of the module landing, a node-globals block for .mjs tooling, and warn-level limits that mirror the ratchet thresholds so editors surface them. Each pure layer is guarded by its one list of pure files: a pure file may import from its own feature only what is on that list, spelled from the alias, so the processing engine's sibling imports move to the alias.
1 parent 3697368 commit 6fac85b

19 files changed

Lines changed: 745 additions & 56 deletions

‎.github/workflows/checks.yml‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ jobs:
99
name: Code check and tests
1010
steps:
1111
- uses: actions/checkout@v4
12+
with:
13+
fetch-depth: 0
1214
- name: Setup node
1315
uses: actions/setup-node@v4
1416
with:
@@ -19,6 +21,15 @@ jobs:
1921
run: npx prettier --list-different "src/**/*.[jt]s" "tests/**/*.[jt]s" "src/**/*.vue"
2022
- name: Lint
2123
run: npm run lint
24+
- name: Quality ratchets
25+
# A file may not get worse than it is on the pull request's target branch.
26+
if: github.event_name == 'pull_request'
27+
env:
28+
BASE_REF: ${{ github.base_ref }}
29+
run: |
30+
git fetch --no-tags origin \
31+
"+refs/heads/$BASE_REF:refs/remotes/origin/$BASE_REF"
32+
node scripts/checks/check.mjs --base "origin/$BASE_REF"
2233
- name: Tests
2334
run: npm test
2435

‎CONTRIBUTING.md‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,18 @@ npm run lint
7575

7676
---
7777

78+
## Quality checks
79+
80+
The pre-commit hook and CI run `node scripts/checks/check.mjs`. It compares the files you changed with the base (`HEAD` locally, the target branch in CI), so existing code is left alone and only a change that makes a file worse fails.
81+
82+
- Complexity: in `src`, a function stays within cyclomatic complexity 10, nesting depth 3 and 4 parameters, and a file within 600 lines. A file already over a limit may not go further over, and a new file must be within all of them.
83+
- Duplication: a change to the tests may not add a block of 60 or more tokens that repeats another test.
84+
- Conventions: no new binary files outside the directories that already hold them, such as `tests/baseline`.
85+
86+
The limits are in `scripts/checks/config.mjs`. Stage your changes before running the script by hand, since it reads the index. `CHECKS_SKIP=1 git commit` skips it for one commit.
87+
88+
---
89+
7890
## Testing
7991

8092
```bash

‎eslint.config.js‎

Lines changed: 144 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import path from 'node:path';
12
import js from '@eslint/js';
23
import eslintPluginVue from 'eslint-plugin-vue';
34
import tseslint from 'typescript-eslint';
@@ -21,15 +22,58 @@ import globals from 'globals';
2122
// the full pattern set its files need, and no block silently erases another
2223
// feature's boundary.
2324
// ---------------------------------------------------------------------------
24-
// `pure.upperModules` is a hand-maintained list of the feature's non-pure
25-
// modules: a new one has to be added here or the pure layer may import it.
25+
// `pure` lists the feature's pure files, and that one list is also everything
26+
// a pure file may import from its own feature: a module that is not on it is
27+
// denied without having to be named. A directory glob admits the directory.
28+
const pureModules = (feature) =>
29+
feature.pure.map((file) =>
30+
file
31+
.slice(`src/${feature.dir}/`.length)
32+
.replace(/\/\*\*\/.*$/, '/**')
33+
.replace(/\.\w+$/, '')
34+
);
35+
36+
// Patterns follow gitignore, which cannot re-admit a path under a denied
37+
// directory. So each directory on the way to a pure module denies its children
38+
// and re-admits the ones that are pure or lead to one.
39+
const ownFeatureExceptPure = (feature) => {
40+
const root = `@/src/${feature.dir}`;
41+
const admitted = new Set(
42+
pureModules(feature).flatMap((module) => {
43+
const parts = module.replace(/\/\*\*$/, '').split('/');
44+
return parts.map((_, depth) => parts.slice(0, depth + 1).join('/'));
45+
})
46+
);
47+
const wholeDirs = pureModules(feature)
48+
.filter((module) => module.endsWith('/**'))
49+
.map((module) => module.slice(0, -3));
50+
const parentOf = (entry) => path.posix.dirname(entry).replace(/^\.$/, '');
51+
const denyingDirs = new Set(
52+
[...admitted].map(parentOf).filter((dir) => !wholeDirs.includes(dir))
53+
);
54+
return [...denyingDirs]
55+
.sort()
56+
.flatMap((dir) => [
57+
`${root}/${dir ? `${dir}/` : ''}*`,
58+
...[...admitted]
59+
.filter((entry) => parentOf(entry) === dir)
60+
.map((entry) => `!${root}/${entry}`),
61+
]);
62+
};
63+
64+
const upperLayerMessage = (feature) =>
65+
`The ${feature.dir} pure layer must not import stores, components, or upper feature modules. Dependencies point downward only.`;
66+
2667
const featureBoundaries = (features) => {
2768
const publicSurface = ({ dir }) => ({
2869
group: [`@/src/${dir}/*`, `@/src/${dir}/*/**`, `!@/src/${dir}/index`],
2970
message: `Import the ${dir} feature only from its public surface \`@/src/${dir}\` (src/${dir}/index.ts). A deep import bypasses the feature boundary.`,
3071
});
72+
const publicFeatures = features.filter(
73+
(feature) => feature.publicSurface !== false
74+
);
3175
const otherSurfaces = (feature) =>
32-
features.filter((other) => other !== feature).map(publicSurface);
76+
publicFeatures.filter((other) => other !== feature).map(publicSurface);
3377

3478
return [
3579
{
@@ -38,7 +82,7 @@ const featureBoundaries = (features) => {
3882
rules: {
3983
'no-restricted-imports': [
4084
'error',
41-
{ patterns: features.map(publicSurface) },
85+
{ patterns: publicFeatures.map(publicSurface) },
4286
],
4387
},
4488
},
@@ -52,28 +96,37 @@ const featureBoundaries = (features) => {
5296
},
5397
})),
5498
...features.map((feature) => ({
55-
files: feature.pure.files,
99+
files: feature.pure,
56100
ignores: ['**/__tests__/**'],
57101
rules: {
58102
'no-restricted-imports': [
59103
'error',
60104
{
61-
paths: ['pinia', 'vue'].map((name) => ({
62-
name,
63-
message: `The ${feature.dir} pure layer must stay framework-free: no ${name}.`,
64-
})),
105+
paths: [
106+
...['pinia', 'vue'].map((name) => ({
107+
name,
108+
message: `The ${feature.dir} pure layer must stay framework-free: no ${name}.`,
109+
})),
110+
// The bare directory is the index, an upper module.
111+
{
112+
name: `@/src/${feature.dir}`,
113+
message: upperLayerMessage(feature),
114+
},
115+
],
65116
patterns: [
66117
{
67118
group: [
68-
...feature.pure.upperModules.flatMap((mod) => [
69-
`@/src/${feature.dir}/${mod}`,
70-
`./${mod}`,
71-
`../${mod}`,
72-
]),
119+
...ownFeatureExceptPure(feature),
73120
'@/src/store/**',
74121
'@/src/components/**',
122+
'@/src/composables/**',
75123
],
76-
message: `The ${feature.dir} pure layer must not import stores, components, or upper feature modules — dependencies point downward only.`,
124+
message: upperLayerMessage(feature),
125+
},
126+
{
127+
// The list above can only judge the alias spelling.
128+
regex: '^\\.\\.?(/|$)',
129+
message: `The ${feature.dir} pure layer spells its imports from \`@/src/\`, so the pure-layer boundary can see them.`,
77130
},
78131
...otherSurfaces(feature),
79132
],
@@ -168,6 +221,17 @@ export default tseslint.config(
168221
'@typescript-eslint/no-unsafe-function-type': 'off',
169222
},
170223
},
224+
// Node tooling (scripts/checks/, fixture servers). The block above matches
225+
// only .js/.ts/.vue, so without this eslint reports `process`, `console` and
226+
// `URL` as undefined in every .mjs file.
227+
{
228+
files: ['**/*.mjs'],
229+
languageOptions: {
230+
ecmaVersion: 'latest',
231+
sourceType: 'module',
232+
globals: globals.node,
233+
},
234+
},
171235
{
172236
files: ['**/tests/pageobjects/**/*.ts'],
173237
rules: {
@@ -246,33 +310,77 @@ export default tseslint.config(
246310
},
247311
},
248312
...featureBoundaries([
313+
{
314+
dir: 'segmentation',
315+
pure: [
316+
'src/segmentation/geometry.ts',
317+
'src/segmentation/color.ts',
318+
'src/segmentation/model.ts',
319+
'src/segmentation/segment.ts',
320+
'src/segmentation/masks/storage.ts',
321+
'src/segmentation/masks/overlap.ts',
322+
'src/segmentation/masks/labelValue.ts',
323+
'src/segmentation/editing/algorithms/fillHoles.ts',
324+
'src/segmentation/editing/algorithms/fillHoles.worker.ts',
325+
'src/segmentation/editing/algorithms/gaussianSmooth.worker.ts',
326+
],
327+
// Consumers import explicit modules; the pure-layer rule still applies.
328+
publicSurface: false,
329+
},
249330
{
250331
dir: 'processing',
251-
pure: {
252-
files: [
253-
'src/processing/engine/**/*.{js,ts}',
254-
'src/processing/types.ts',
255-
'src/processing/config.ts',
256-
],
257-
upperModules: [
258-
'store',
259-
'applyResults',
260-
'jobResultReview',
261-
'index',
262-
'components/**',
263-
],
264-
},
332+
pure: [
333+
'src/processing/engine/**/*.{js,ts}',
334+
'src/processing/types.ts',
335+
'src/processing/config.ts',
336+
],
265337
},
266338
{
267339
dir: 'referenceLines',
268-
pure: {
269-
files: [
270-
'src/referenceLines/geometry.ts',
271-
'src/referenceLines/crossings.ts',
272-
],
273-
upperModules: ['store', 'index', 'useReferenceLines', 'components/**'],
274-
},
340+
pure: [
341+
'src/referenceLines/geometry.ts',
342+
'src/referenceLines/crossings.ts',
343+
],
275344
},
276345
]),
346+
// Tests are excluded so this block never matches a file the `vi.mock` block
347+
// above matches: flat config replaces a rule's options wholesale, so overlap
348+
// would erase that rule rather than add to it.
349+
{
350+
files: ['src/**/*.{ts,vue}'],
351+
ignores: ['src/**/__tests__/**', 'src/**/*.{spec,test}.{js,ts}'],
352+
rules: {
353+
'no-restricted-syntax': [
354+
'warn',
355+
{
356+
selector:
357+
"CallExpression[callee.name='computed'][typeArguments.params.length>0]",
358+
message:
359+
'Let computed() infer its type. An explicit generic goes stale and hides the inference errors it was meant to document.',
360+
},
361+
],
362+
},
363+
},
364+
// Mirrors the limits scripts/checks/complexity.mjs enforces per commit. Warn
365+
// level because the ratchet only fails a file whose debt grows, so existing
366+
// files stay over these numbers without failing `npm run lint`.
367+
{
368+
files: ['src/**/*.{js,ts,vue}'],
369+
ignores: [
370+
'src/**/__tests__/**',
371+
'src/**/*.{spec,test}.{js,ts}',
372+
'src/**/*.d.ts',
373+
'**/emscripten-build/**',
374+
],
375+
rules: {
376+
complexity: ['warn', 10],
377+
'max-depth': ['warn', 3],
378+
'max-params': ['warn', 4],
379+
'max-lines': [
380+
'warn',
381+
{ max: 600, skipBlankLines: true, skipComments: true },
382+
],
383+
},
384+
},
277385
eslintConfigPrettier
278386
);

0 commit comments

Comments
 (0)