diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index eacc57a3..3f392c0e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -30,7 +30,11 @@ jobs: # the six-hour default. timeout-minutes: 10 steps: + # Full history, not the default single commit: the fix-needs-a-test gate compares the branch + # against origin/main, and with one commit there is no merge base to compare from. - uses: actions/checkout@v7 + with: + fetch-depth: 0 - uses: actions/setup-node@v7 with: @@ -41,19 +45,13 @@ jobs: # added without committing the lockfile fails here rather than on someone else's machine. - run: npm ci - - name: Type check - run: npm run typecheck - - - name: Unit tests - run: npm test - - # Fails only when hook dependency warnings grow, i.e. when a change adds new stale-closure - # risk. See scripts/hook-baseline.mjs for why zero is not the target. - - name: Hook baseline - run: node scripts/hook-baseline.mjs - - - name: Build - run: npm run build + # The whole gate, as one command - the same one run locally. This used to list its steps by + # hand (type check, tests, hook baseline, build), and as gates were added to `npm run check` + # the copy here was never updated: by 2026-10-14 CI ran four of the ten, and the helper index, + # unreachable names, unused imports, stroke writes, native dialogs and i18n checks bound only + # on a machine where someone remembered to run them. One list cannot drift from itself. + - name: npm run check + run: npm run check # Separate from `check` on purpose: that job is the required gate and it finishes in half a # minute, so it must not wait behind a browser. This one is allowed to be the slow one. diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index dd76091b..f0021093 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -9,7 +9,7 @@ Frame-by-frame MV/animation app. Vite + React 19. Capacitor wraps it for Android ## Files -App.tsx is around 2,100 lines; it is mostly wiring now — which hook owns what, and +App.tsx is around 2,200 lines; it is mostly wiring now — which hook owns what, and which component gets which props. **Put new pure logic in a module, not in App.tsx** — anything that is a function of its arguments belongs next to its tests. See [#234] for where the rest of App is going and why. @@ -52,6 +52,10 @@ know storage exists at all, because reading it is `useStored`'s decoder. write UI strings in Korean and add the English to the dictionary. Called `tr`, not `t` — `t` is a local variable in dozens of places. - `src/db.ts` — IndexedDB autosave (`saveAutosave`, `loadAutosave`, plus project CRUD). +- `src/i18n.ja.ts` — the Japanese dictionary, keyed by the same Korean text; a missing entry falls + back to English, not to the Korean key. +- `src/globals.d.ts` — browser and vendor globals the type checker does not know (the EyeDropper + among them), declared as possibly absent because the code checks before using them. **core/** — pure logic, all of it tested, all of it TypeScript (the first folder converted; the shapes the modules share are in `types.ts`, and the document's own are declared in @@ -117,6 +121,44 @@ index signature, so a misspelt one is an error. Ids there are `DocId` - a number to come: presets that use the room instead of zooming to make some, and a camera that can span more than one cut. +- `ids.ts` — `nextId`, `randomId`. These were `Date.now()` in nineteen places, and a millisecond + is long enough to make two things with one id. +- `camera.ts` — where the camera is looking at a moment in a cut: a window onto the artwork (a + centre and a zoom), its presets and paths. Applied at playback only. +- `clipping.ts` — `clipGroups`, `canClip`: a layer that only shows where the layer beneath it has + paint, which is how shading goes over flat colour. +- `shapeStroke.ts` — `rectPoints`, `ellipsePoints`: ruler shapes as ordinary point lists, which is + the whole idea - a shape is then just a stroke. +- `pathMotion.ts` — turning a drawn line into a path something can travel along evenly: smoothing + and resampling by length, since a pen piles points up wherever it slowed down. +- `brushSize.ts` / `viewZoom.ts` — the brush width range and the canvas zoom range, each in one + place after being written in two and three places that disagreed. +- `timelineZoom.ts` — timeline pixels to time and back (`timeAtX`, `xAtTime`), and zooming without + the content sliding out from under the cursor. +- `windowDrag.ts` — `dragOnWindow`: a drag run on window listeners, because the pointer leaves the + element it started on constantly. Ends on `pointercancel` too, which every hand-rolled one missed. +- `persist.ts` — `readStored`/`writeStored` and the codecs under `useStored`: the small preferences + kept in localStorage. +- `playbackRate.ts` / `playbackStart.ts` / `timeScale.ts` — the preview speed and that it is + remembered, where play begins when it is pressed, and baking a preview speed into the film's + real timings. +- `mediaReducer.ts` / `mediaEl.ts` — the audio and video tracks as one reducer, and the one thing + done to the elements playing them (`detachMedia`). +- `api.ts` / `probeBackoff.ts` — talking to the local project server (`apiFetch` adds the status + check fetch does not do; `retryDelayMs` honours a 429), and how often to ask whether it is there. +- `projectAssets.ts` — how each piece of a project is stored - as an asset, a blob or a data URL - + and how a stored one comes back. +- `splitProject.ts` — cutting one long project into sequential pieces that can be worked on apart + (#123). +- `textEdit.ts` — a text object's twenty-odd fields in one place (`textFromEdit`, `editFromText`) + instead of three hand-copied ones. +- `videoCuts.ts` — the arithmetic in the middle of a video import: extracted frames into cuts, + their durations and parts. +- `recordBitrate.ts` — the bitrate to ask the recorder for, by codec and canvas size; left unset, + Chrome settles near 2.5 Mbps whatever the size. +- `decodeBudget.ts` — how many decoded frames to hold, which to let go of, and which cached layers + a decode invalidates. + **engine/** — what a frame *is*, with no canvas anywhere near it. TypeScript; `Scene`, `EvaluatedCut` and `EvaluatedGroup` in `evaluateFrame.ts` are the renderer's whole input, named. @@ -155,6 +197,21 @@ frame is made of, and `SceneDeps` what the renderer needs from the app. - `warpRender.ts` / `shearSlices.ts` / `swayRender.ts` — a selection's rotate/skew/bend outline, the slice stack that draws a sheared bitmap, and the sway deformation. +- `canvasFactory.ts` — `makeCanvas`/`setCanvasFactory`: where the canvas modules get a canvas, which + is what lets the pixel tests run them in Node on `@napi-rs/canvas`. + +**export/** — turning the timeline into a file. No React; what only the app knows (the range, the +audio graph, the playback loop) stays in `hooks/useExport.ts`. + +- `recorder.ts` — `pickRecordingType` (MP4 first, asked for by a codec name browsers accept), + `frameSource`, `startRecorder`: recording the canvas, minus the parts that need the app. +- `gif.ts` — a GIF89a encoder, enough for an animation with a transparent background, which no + browser API produces. +- `zip.ts` / `byteWriter.ts` — a store-only ZIP writer (PNGs are already deflated, so compressing + them again buys nothing) and the growable byte buffer under it. +- `download.ts` — `downloadBlob`: handing a finished file to the browser, once rather than three + times. + **styles/** — the stylesheets, one per area (top bar, toolbar, canvas, the cut/layer panel, the timeline, the colour panel, touch, …), imported in their original cascade order by `styles/index.css`. The order is load-bearing: some rules override others by position alone. @@ -178,6 +235,13 @@ prompt, tool keys, project picker, progress overlay), all over `Modal.tsx`. And them stops the work underneath; that is the rule they share. - `NumField.tsx` — **use NumField for any new numeric field.** +- `Logo.tsx` — the app mark: three receding frames - the onion skin - with a play mark on the front. +- `dialogs/` — one file per dialog. `AskModal.tsx` is the app's own confirm and prompt (a browser + told to block dialogs makes `confirm` return silently, so the button looks dead). + `SettingsModal.tsx` / `ToolKeysModal.tsx` are the settings and the two keymap editors; + `VideoImportModal.tsx`, `SceneDetectModal.tsx`, `ExportRangeModal.tsx`, `ProjectPicker.tsx`, + `LinkPromptModal.tsx`, `ProgressOverlay.tsx` and `HelpModal.tsx` do what they are called. + **hooks/** — TypeScript. State that belongs together, lifted out of App so its wiring is somewhere with a name; each hook's dependency bag is an interface (`PlaybackDeps`, `ExportDeps`, `LayerCacheDeps`, …). Each takes what it cannot own as arguments, and the rule for what it cannot own is the same every time: `buildData` reads most of App's state and `restore` writes most of it, so a hook that @@ -236,6 +300,11 @@ needs a document takes those two functions rather than the document. call sites keep the shape `window.confirm` gave them. `AskModal` draws it. - `useDialogs.ts` — which dialog is open, and the rebinding two of them share. +- `useCutListUi.ts` — what is picked in the cut list and how much of it is unfolded, kept in one + hook so opening a document clears all of it at once (#241). +- `useTimelineView.ts` — how far the timeline is zoomed, which part is on screen, and the two + overlays drawn while a drag is happening. + - `server/` — the Express file-backed project DB on :8787, files under `server/data/`. `index.js` is the wiring (body limit, rate-limit tiers, mount, listen); `projects.js`, `backups.js` and `youtube.js` are one route family each; `paths.js` builds every path, so `safeId` cannot be diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f54eb518..5bd08812 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -50,7 +50,19 @@ git commit -m "fix: ..." git push -u origin HEAD gh pr create --fill # put "Closes #12" in the body gh pr checks # CI -gh pr merge --squash --delete-branch +gh pr merge --rebase --delete-branch # rebase, so each commit lands on main as written +``` + +**A `fix:` brings its test.** If a branch has a `fix:` commit, it has to change something under +`test/` - the test that fails without the fix. That is the moment a defect is best understood and +a test is cheapest to write, and it is what stops the same defect coming back: before this rule, +over half of all fixes had landed without one. When nothing can be observed by a test (a colour, +a config value), say so in the commit message and the gate accepts it: + +``` +fix: the panel border is visible in the light theme + +No-Test: colour only; nothing here a test can observe ``` ## `main` is protected @@ -76,7 +88,7 @@ Turning it back on is the part that gets forgotten, so do both in one sitting. ## Verifying ```bash -npm run check # typecheck → tests → hook-warning baseline → build +npm run check # typecheck → tests → nine static guards → build; CI runs exactly this ``` **Passing this proves less than it looks.** A component that returns `undefined` is legal @@ -107,7 +119,8 @@ evidence you will have. ## Where code goes The folders under `src/` say what a file is allowed to touch. The details are in -[ARCHITECTURE.md](ARCHITECTURE.md) — **read it before editing App.jsx.** +[ARCHITECTURE.md](ARCHITECTURE.md) — **read it before editing App.tsx.** Every file under `src/` +is named in it, and `npm run check` fails when a new one is not. ``` core/ pure logic — no React, no DOM, no canvas. Everything here has tests. diff --git a/README.ko.md b/README.ko.md index 18af6e7d..f4fd2cfc 100644 --- a/README.ko.md +++ b/README.ko.md @@ -96,7 +96,7 @@ npm run lint # eslint-plugin-react-hooks npm run smoke # 빌드된 앱을 헤드리스 브라우저에서 띄워 선 하나 긋기 ``` -`npm run check`가 관문입니다: 타입 검사, 단위 테스트, 정적 가드 여섯, 빌드. 같은 단계가 모든 푸시와 PR에서 CI로 돕니다. +`npm run check`가 관문입니다: 타입 검사, 단위 테스트, 정적 가드 아홉, 빌드. CI는 모든 푸시와 PR에서 이 명령 하나를 그대로 돌립니다 - 예전에는 단계를 손으로 나열했고, 10월에는 그 사본이 열 개 중 넷만 돌리고 있었습니다([docs/process/](docs/process/2026-10-14.md)). 코드는 TypeScript입니다 — JavaScript에서 폴더 하나씩 옮겼고(#268) 모든 파일이 `strict`로 검사됩니다. 목적은 주석을 타입으로 바꾸는 것이 아니라(JS도 이미 JSDoc으로 타입이 있었음) JSDoc이 줄 수 없던 @@ -115,9 +115,11 @@ npm run smoke # 빌드된 앱을 헤드리스 브라우저에서 띄워 선 | `scripts/unused-imports.mjs` | 파일 안 아무것도 쓰지 않는 import | | `scripts/stroke-writes.mjs` | `commitStroke`를 거치지 않고 레이어에 선을 추가하는 쓰기 | | `scripts/native-dialogs.mjs` | `src/` 안의 `alert`·`confirm`·`prompt` — 브라우저가 대화상자를 막으면 셋 다 조용히 실패함 | +| `scripts/architecture-index.mjs` | `ARCHITECTURE.md`가 이름을 대지 않는 `src/` 아래 파일 | +| `scripts/fix-needs-test.mjs` | `fix:` 커밋이 있는데 `test/` 아래를 하나도 바꾸지 않은 브랜치 (`No-Test:` 트레일러로 이유를 적으면 예외) | | `scripts/i18n-check.mjs` | 영어 항목 없는 `tr()` 리터럴, 또는 소스에 키가 더는 없는 사전 항목 | -단위 테스트 약 1,150개가 `src/core`, `src/canvas`, `src/engine`, `src/export`의 순수 모듈을 +단위 테스트 약 1,300개가 `src/core`, `src/canvas`, `src/engine`, `src/export`의 순수 모듈을 덮습니다 — 기하, 이징, 키프레임 샘플링, 컷 리듀서, 레이어 트리 이동, 올가미 오려내기, 타임라인 스냅, 시간 배율 굳히기, GIF·zip 라이터. DOM도 프레임워크도 필요 없어 Node 내장 러너를 씁니다. 실제로 그리는 함수들 — 선을 픽셀로, 노이즈, 모자이크, 채우기 — 는 `@napi-rs/canvas`(브라우저 diff --git a/README.md b/README.md index 2ee73a49..86710fbf 100644 --- a/README.md +++ b/README.md @@ -98,8 +98,9 @@ npm run lint # eslint-plugin-react-hooks npm run smoke # boots the built app in a headless browser and draws a stroke ``` -`npm run check` is the gate: typecheck, the unit tests, then six static guards, then the build. -The same steps run in CI on every push and pull request. +`npm run check` is the gate: typecheck, the unit tests, then nine static guards, then the build. +CI runs that one command on every push and pull request - it used to list its steps by hand, and +by October that copy ran four of the ten (see [docs/process/](docs/process/2026-10-14.md)). The code is TypeScript, moved from JavaScript one folder at a time (#268), and every file is held to `strict`. The point was never the annotations - the JavaScript was already typed through @@ -119,15 +120,21 @@ dependency), and the bundler and `tsc` accept it. | `scripts/unused-imports.mjs` | an import nothing in the file uses | | `scripts/stroke-writes.mjs` | a write that adds a stroke to a layer without going through `commitStroke` | | `scripts/native-dialogs.mjs` | an `alert`, `confirm` or `prompt` in `src/` — a browser that blocks dialogs makes all three fail silently | +| `scripts/architecture-index.mjs` | a file under `src/` that `ARCHITECTURE.md` does not name | +| `scripts/fix-needs-test.mjs` | a branch with a `fix:` commit that changes nothing under `test/`, unless a `No-Test:` trailer says why | +| `scripts/i18n-check.mjs` | a `tr()` literal with no English entry, or a dictionary row whose key no longer appears in the source | -Two measuring tools, neither in the gate — they print numbers rather than pass or fail: +Measuring tools, none in the gate — they print numbers rather than pass or fail: `npm run bench` times the pure hot paths, and `node scripts/profile-frame.mjs` drives a real browser against a running dev server and reports frame time, 2d calls per frame attributed to the canvas they landed on, and image readbacks. The first cannot see the canvas; the second cannot see -raster and compositing. Both say so in their own headers. -| `scripts/i18n-check.mjs` | a `tr()` literal with no English entry, or a dictionary row whose key no longer appears in the source | +raster and compositing. Both say so in their own headers. `node scripts/video-dedupe-check.mjs` +imports a generated clip whose every frame differs and fails if any are merged (#124), and +`node scripts/process-metrics.mjs` reads the repository's own records into the process measures +- deployment frequency, lead time, test pass ratio, defect escape rate - that each +[process cycle](docs/process/) is judged against. -Around 1,150 unit tests cover the pure modules under `src/core`, `src/canvas`, `src/engine` and +Around 1,300 unit tests cover the pure modules under `src/core`, `src/canvas`, `src/engine` and `src/export` — geometry, easing, keyframe sampling, the cuts reducer, layer-tree moves, lasso cut-out, timeline snapping, the time-scale bake, GIF and zip writers. They use Node's built-in runner because none of it needs a DOM or a framework. The functions that draw - strokes into diff --git a/docs/process/2026-10-14.baseline.json b/docs/process/2026-10-14.baseline.json new file mode 100644 index 00000000..5d0ac977 --- /dev/null +++ b/docs/process/2026-10-14.baseline.json @@ -0,0 +1,163 @@ +{ + "all": { + "window": "all history", + "measuredAt": "2026-10-14", + "merges": 291, + "mergesByKind": { + "refactor": 108, + "fix": 73, + "feat": 54, + "docs": 17, + "chore": 10, + "perf": 7, + "test": 5, + "ci": 2, + "security": 2, + "deps": 2 + }, + "deploymentsPerWeek": 32.8, + "prLeadTimeHours": { + "median": 0, + "p90": 1.9 + }, + "issueLeadTimeDays": { + "median": 0, + "p90": 15.5, + "closed": 49 + }, + "ciPassRatio": 96.9, + "ciRunsJudged": 769, + "mainBranchFailures": 1, + "defectEscapeRate": 43.2, + "fixMerges": 73, + "introducingMerges": 169, + "mttrDays": { + "median": 0, + "bugsClosed": 11 + }, + "fixCommitsWithATest": 43.6, + "fixCommits": 101, + "risk": { + "untickedChecklistItems": 3, + "issuesAwaitingVerification": 2, + "reverts": 0, + "fixesOfARecentFix": 43, + "longestGapBetweenMergesDays": 9, + "openIssues": 3, + "oldestOpenIssueDays": 62.1 + }, + "defectHotspots": [ + [ + "src/App", + 63 + ], + [ + "src/ui/AnimPanels", + 7 + ], + [ + "src/canvasUtils", + 6 + ], + [ + "src/hooks/useExport", + 5 + ], + [ + "src/canvas/canvasUtils", + 5 + ], + [ + "src/core/layerOps", + 4 + ], + [ + "src/ui/TopBar", + 4 + ], + [ + "src/canvas/pixelEffects", + 4 + ] + ] + }, + "last30": { + "window": "last 30 days", + "measuredAt": "2026-10-14", + "merges": 170, + "mergesByKind": { + "refactor": 67, + "fix": 46, + "feat": 26, + "docs": 11, + "chore": 7, + "perf": 6, + "test": 3 + }, + "deploymentsPerWeek": 39.7, + "prLeadTimeHours": { + "median": 0, + "p90": 0.1 + }, + "issueLeadTimeDays": { + "median": 0, + "p90": 9.7, + "closed": 29 + }, + "ciPassRatio": 99.7, + "ciRunsJudged": 371, + "mainBranchFailures": 0, + "defectEscapeRate": 46.5, + "fixMerges": 46, + "introducingMerges": 99, + "mttrDays": { + "median": 4.9, + "bugsClosed": 5 + }, + "fixCommitsWithATest": 60.4, + "fixCommits": 48, + "risk": { + "untickedChecklistItems": 3, + "issuesAwaitingVerification": 2, + "reverts": 0, + "fixesOfARecentFix": 25, + "longestGapBetweenMergesDays": 5.2, + "openIssues": 3, + "oldestOpenIssueDays": 62.1 + }, + "defectHotspots": [ + [ + "src/App", + 23 + ], + [ + "src/ui/AnimPanels", + 7 + ], + [ + "src/hooks/useExport", + 5 + ], + [ + "src/canvas/canvasUtils", + 4 + ], + [ + "src/canvas/pixelEffects", + 4 + ], + [ + "src/canvas/strokes", + 3 + ], + [ + "src/core/layerOps", + 3 + ], + [ + "src/export/recorder", + 2 + ] + ] + } +} diff --git a/docs/process/2026-10-14.md b/docs/process/2026-10-14.md new file mode 100644 index 00000000..f5b1c2d7 --- /dev/null +++ b/docs/process/2026-10-14.md @@ -0,0 +1,147 @@ +# Process cycle 1 — 2026-10-14 + +The first pass through Measure → Analyze → Change. It starts from a written review of how this +project is developed, done for a software engineering course: it identified the process as +incremental development run on XP practices, checked it against the ten XP practices, and closed +with a risk plan and five steps to take. The review's own ordering was that **the measurement +baseline comes first** - it is the cheapest step and the one every other step is judged by - so +that is where this cycle began, and the changes were chosen from what the numbers showed rather +than from the list. + +Raw numbers: [`2026-10-14.baseline.json`](2026-10-14.baseline.json), produced by +`node scripts/process-metrics.mjs --json` (all history and `--since 30`). + +## 1. Measure + +The history runs from 2026-06-17 to 2026-10-14: 291 merged PRs, 52 issues, 430 commits. + +| measure | all history | last 30 days | +|---|---|---| +| deployment frequency | 32.8 merges/week (291) | 39.7 merges/week (170) | +| lead time, PR → deploy | median 0h, p90 1.9h | median 0h, p90 0.1h | +| lead time, issue → close | median 0d, p90 15.5d (49 issues) | median 0d, p90 9.7d (29) | +| automated test pass ratio | 96.9% of 769 CI runs; 1 failure on main | 99.7% of 371; 0 on main | +| defect escape rate | **43.2%** (73 fixes / 169 feat+perf+refactor) | **46.5%** (46 / 99) | +| MTTR, bug-labelled issues | median 0d over 11 bugs | median 4.9d over 5 | +| fixes carrying a test | **43.6%** of 101 fix commits | **60.4%** of 48 | + +| risk indicator (the review's monitoring list) | all history | last 30 days | +|---|---|---| +| estimation — deferred verification | 3 unticked checklist items; 0 issues labelled as waiting | same | +| tools — reverts | 0 | 0 | +| tools — a fix landing on a file another fix touched that week (App aside) | 43 | 25 | +| people — longest gap between merges | 9d | 5.2d | +| open issues | 3, oldest 62 days | | + +Defect hotspots, all history - the source files fix commits touched most: + +| fixes | file | +|---|---| +| 63 | `src/App` (`.jsx` then `.tsx`, counted as one) | +| 11 | `canvasUtils` (`src/` then `src/canvas/`; split up in #290) | +| 7 | `src/ui/AnimPanels` | +| 5 | `src/hooks/useExport` | +| 4 | `src/core/layerOps`, `src/ui/TopBar`, `src/canvas/pixelEffects` | + +### Corrections made before the numbers were trusted + +The first run gave a defect escape rate of 48.5%. Three things were wrong with how it was being +read, and each moved a number: + +- **55 PR titles were prose** ("hooks/ in TypeScript"), so they had no `feat:`/`fix:` prefix and + counted as neither. Most were the TypeScript migration - refactors - and leaving them out + shrank the denominator. Classifying a prefix-less PR by its commits, which rebase-merge puts on + main as written, took the rate to 43.2%. +- **Renames split one file's history.** `App.jsx` and `App.tsx` are the same component; counted + apart, the hotspot list understated it. +- **The translation tables ranked as hotspots** because every fix that changes a UI string adds + its translations. They are left out, as is CSS. + +And one indicator was redefined: "a fix touching a file another fix touched that week" read 77 +because nearly every change wires through App. An indicator that is always high cannot be +watched, so App is excluded from the overlap; it reads 43. + +## 2. Analyze + +**CI says yes, and defects ship anyway.** The pass ratio is 97-99.7% while nearly one fix lands +for every two changes. The review put this down to CI checking syntax rather than meaning; the +numbers agree, with one refinement - CI does run 1,300 unit tests, so the issue is not that it +only checks syntax but that the tests do not cover where the defects actually land. + +**Where they land is one file.** App took 63 fix-touches, against 11 for the next. This is the +"super component" the review named as the cost of building without design, now with a count. The +second hotspot, `canvasUtils`, no longer exists - it was split up in #290 - which is some evidence +the refactoring the review credited is the right remedy. + +**Test-after, measured.** 56% of all fixes landed without touching a test. That has improved - +40% over the last month - but a fix without a test is the clearest case of the test-after +development the review blamed, because nothing stops the same defect returning. + +**CI enforced four of its ten gates.** Found while wiring in the first change below. `ci.yml` +said it existed to run `npm run check` on every push, and then listed the steps by hand - type +check, tests, hook baseline, build. Every gate added locally since (helper index, unreachable +names, unused imports, stroke writes, native dialogs, i18n) bound only on a machine where someone +remembered to run it. The review's statement that the XP rule "every build must pass the tests" +was already implemented in CI was true for four tenths of it. CONTRIBUTING.md said "`npm run +check` is what CI runs", and was wrong. + +**The documents drift, and by how much.** `ARCHITECTURE.md` - the file whose own title says to +read it before editing - did not name 44 of the 152 files under `src/`, 29%. The `export/` folder +had no section at all; App's stated size was 2,100 lines against 2,198. A loose search (the bare +word, not the filename) found only 34 missing, because "zip" and "camera" appear in prose without +the files ever being named. The review's diagnosis was that keeping documents current is never +the cheapest next step in incremental development, which is why the cure has to be a check rather +than a resolution. + +**Some of the measurements cannot be trusted yet.** +- Only 11 of the 73 fixes had a bug-labelled issue, so MTTR describes about 15% of defects. +- PR lead time is near zero because PRs are opened after the work is done; the issue → close + figure is the meaningful one, and it is bimodal - most issues close the day they open, and the + rest wait weeks. +- "Deferred verification" read 0 while #338 was waiting on a device, because nothing recorded the + wait. + +## 3. Change + +Each change names the part of the review it answers and the number it should move. + +| # | change | answers | expected to move | +|---|---|---|---| +| C1 | **A fix brings its test.** `scripts/fix-needs-test.mjs`: a branch with a `fix:` commit must change something under `test/`, or carry a `No-Test:` trailer saying why. In the gate. | XP practice "test-first development" (not applied), adopted where the review said it should be: where defects happen. Risk plan: *semantic bugs reaching deploy - avoidance*. | fixes carrying a test: 60% → ~100%, waivers aside | +| C2 | **The map is checked.** `scripts/architecture-index.mjs`: every file under `src/` must be named in `ARCHITECTURE.md`. The 44 missing were added, with an `export/` section. In the gate. | The review's documentation step - make document freshness something verified, not hoped for. Risk plan: *black-box code accumulating - minimization*. | 152/152 named, and staying there | +| C3 | **CI runs the gate, all of it.** `ci.yml` now runs `npm run check` as one step instead of a hand-kept copy of it, so it cannot drift again. | XP practice "continuous integration", which the review counted as applied. | test pass ratio may *dip* at first - six more checks now count. That would be the gates working | +| C4 | **Deferred verification is recorded.** A `needs-verification` label, applied to #338 and #4, and counted by the metrics. | Risk monitoring, *estimation*: the backlog of untested work. | the indicator now reads 2; it should reach 0 as checks are done | +| C5 | **The measures themselves.** `scripts/process-metrics.mjs`, and this record. | The review's fifth step - a baseline before anything else. | - | + +Also corrected in passing, because they were the same kind of drift: README's guard table had its +last row stranded outside the table and said "six static guards" when there were seven; +CONTRIBUTING told contributors to squash-merge where the repository rebase-merges, and to read +ARCHITECTURE.md "before editing App.jsx". + +## 4. Not changed this cycle, and why + +- **A requirements specification** - the review's first step, derived backwards from what is + built and then checked by the customer. Its last step is the customer's own, so it is a draft + to write next rather than something to finish here. +- **Outside feedback** and **DevOps readiness** need a decision and a server respectively; there + is nothing to measure yet. +- **A testing cadence.** How often deferred checks must be closed is the owner's call. C4 makes + the backlog countable, so whatever period is chosen can be checked rather than remembered. +- **Labelling every fix with an issue,** which would make MTTR mean something. It is a habit, not + something a gate can enforce cheaply - flagged here so the next cycle can see whether it took. + +## 5. Next measurement + +Rerun `node scripts/process-metrics.mjs --since 30` at the end of the next cycle and compare it +with the `last30` block of the baseline file. What should have happened, written down now so it +cannot be fitted to the result afterwards: + +- **fixes carrying a test** up from 60.4%, towards 100%. If it does not move, C1 is being waived + rather than followed, and the waivers in history will say so. +- **fixes of a recent fix** down from 25, a little later - a test stops a defect *returning*, not + appearing. +- **defect escape rate** is the slowest to respond for the same reason. A rise would not on its + own mean C1 failed. +- **CI pass ratio** may fall below 99.7% for a while, since it now runs ten checks rather than + four. A fall here is the gate catching what it used to let through. +- **awaiting verification** down from 2. diff --git a/docs/process/README.md b/docs/process/README.md new file mode 100644 index 00000000..7b88ad6d --- /dev/null +++ b/docs/process/README.md @@ -0,0 +1,41 @@ +# Process cycles + +How the way this project is built gets improved, and the record of each time it was. + +Each cycle is **Measure → Analyze → Change**, then measure again. The measuring comes first and +is not optional: a process change judged by feel cannot be told apart from one that did nothing, +and without a baseline the next cycle cannot know what to change either. It is also the cheapest +step - every figure comes out of records the repository already keeps. + +## Running a cycle + +```bash +node scripts/process-metrics.mjs # all of history +node scripts/process-metrics.mjs --since 30 # the last 30 days - compare this one +node scripts/process-metrics.mjs --json # for diffing against a saved baseline +``` + +Then write a new file here, named for the date - **a new file each cycle, never an edit to an old +one**, so the trend stays readable (the same rule as `docs/health/`). Save the `--json` output +beside it, so the next cycle compares numbers rather than prose. + +A record says what was measured, what the numbers meant, what was changed because of them and +which number each change is expected to move - so the next cycle can check. + +## What the measures are + +| measure | from | note | +|---|---|---| +| deployment frequency | merged PRs | every merge to main deploys to Vercel | +| lead time | PRs, issues | PR lead time is near zero because PRs open after the work; issue → close is the real one | +| automated test pass ratio | Actions runs of `ci.yml` | cancelled runs are superseded, not verdicts, and are left out | +| defect escape rate | `fix:` merges ÷ feat + perf + refactor merges | nothing lands without passing CI, so each fix is a defect that passed it | +| MTTR | bug-labelled issues, open → close | only as good as the labelling | +| fixes carrying a test | fix commits touching `test/` | the test-after rate, directly | +| risk indicators | issues, git history | deferred verification, reverts, fixes of a recent fix, stalls | +| defect hotspots | files touched by fix commits | where a test-first rule pays most | + +## Records + +- [2026-10-14](2026-10-14.md) — cycle 1. The baseline; CI found running four of its ten gates; + ARCHITECTURE.md missing 29% of the files; fixes required to carry a test. diff --git a/package.json b/package.json index f39ef468..2b1e4912 100644 --- a/package.json +++ b/package.json @@ -23,7 +23,7 @@ "bench": "node --experimental-strip-types --no-warnings=ExperimentalWarning scripts/bench.mjs", "typecheck": "tsc --noEmit", "lint": "eslint src", - "check": "npm run typecheck && npm test && node scripts/hook-baseline.mjs && node scripts/helper-index.mjs && node scripts/unreachable.mjs && node scripts/unused-imports.mjs && node scripts/stroke-writes.mjs && node scripts/native-dialogs.mjs && node scripts/i18n-check.mjs && npm run build", + "check": "npm run typecheck && npm test && node scripts/hook-baseline.mjs && node scripts/helper-index.mjs && node scripts/unreachable.mjs && node scripts/unused-imports.mjs && node scripts/stroke-writes.mjs && node scripts/native-dialogs.mjs && node scripts/architecture-index.mjs && node scripts/fix-needs-test.mjs && node scripts/i18n-check.mjs && npm run build", "smoke": "node test/smoke/boot.mjs && node test/smoke/draw.mjs" }, "dependencies": { diff --git a/scripts/architecture-index.mjs b/scripts/architecture-index.mjs new file mode 100644 index 00000000..14648ffc --- /dev/null +++ b/scripts/architecture-index.mjs @@ -0,0 +1,56 @@ +// Keeps ARCHITECTURE.md's file map honest. +// +// The map is the first thing to read before editing - its own title says so - and on 2026-10-14 +// it was missing 34 of the 152 files under src/, 22% of the codebase. Nothing had gone wrong in +// any one change. Each file was added by a PR that had something else to do, and the map was +// never on anyone's list, so it fell further behind with every module: the same drift the +// process review (docs/process/) put down to incremental development itself, where producing a +// document for every version is never the cheapest next step. +// +// HELPERS.md had the same problem and solved it by being checked, and this is that rule applied +// to the map. Add a file under src/ and forget to say what it is, and the gate fails. It checks +// that the file is named, not that the sentence beside it is good - the same line HELPERS.md +// draws, and for the same reason: whether a description is right is a review question, whether +// it exists at all is not. +// +// A file counts as named when its filename appears in backticks - `layerOps.ts`, or a path that +// ends in it. Basenames under src/ are unique, which is what makes the filename alone enough. + +import { readFileSync } from 'node:fs'; +import { execFileSync } from 'node:child_process'; +import { pathToFileURL } from 'node:url'; + +const MAP = 'ARCHITECTURE.md'; + +/** + * The files the map does not name. + * + * @param {string} doc the map's text + * @param {string[]} files repository paths + * @returns {string[]} + */ +export function unnamed(doc, files) { + const named = new Set(); + for (const m of doc.matchAll(/`([^`\n]+)`/g)) { + // A backtick span can hold several names ("`a.ts` / `b.ts`" is two spans, but + // "`frameExport.ts`/`exportQueue.ts`" and a path like `src/core/x.ts` are common too). + for (const part of m[1].split(/[\s,/]+/)) if (/\.(tsx?|d\.ts)$/.test(part)) named.add(part); + } + return files.filter(f => !named.has(f.split('/').pop())); +} + +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + const files = execFileSync('git', ['ls-files', 'src'], { encoding: 'utf8' }) + .split('\n').filter(f => /\.(ts|tsx)$/.test(f)); + const missing = unnamed(readFileSync(MAP, 'utf8'), files); + if (missing.length) { + console.error(`${missing.length} file(s) under src/ that ${MAP} does not name:`); + for (const f of missing) console.error(` ${f}`); + console.error(` +${MAP} is read before editing, and a map that is missing a fifth of the territory teaches +people to stop reading it. Add a line under the file's folder saying what it is for - one +sentence is enough; the name alone is what this checks.`); + process.exit(1); + } + console.log(`Architecture map passed - all ${files.length} files under src/ are named in ${MAP}`); +} diff --git a/scripts/commitKind.mjs b/scripts/commitKind.mjs new file mode 100644 index 00000000..7ad94daf --- /dev/null +++ b/scripts/commitKind.mjs @@ -0,0 +1,13 @@ +// The conventional-commit type a subject line opens with: feat, fix, perf, refactor, ... +// +// Shared by the process metrics and the fix-needs-a-test gate, which must agree on what counts as +// a fix - if they did not, the gate could pass a change the metrics then count as an untested fix. + +/** + * @param {string} subject a commit subject or PR title + * @returns {string} the type in lower case, or 'other' when there is no prefix + */ +export function kindOf(subject) { + const m = /^(\w+)(\(.+?\))?!?:/.exec(String(subject || '').trim()); + return m ? m[1].toLowerCase() : 'other'; +} diff --git a/scripts/fix-needs-test.mjs b/scripts/fix-needs-test.mjs new file mode 100644 index 00000000..9c89213f --- /dev/null +++ b/scripts/fix-needs-test.mjs @@ -0,0 +1,68 @@ +// A fix has to bring the test that would have caught it. +// +// Measured on 2026-10-14 (docs/process/): 56% of all fix commits in this repository had landed +// without touching a test, and 40% of the last month's still did. A fix with no test is the +// cleanest case of test-after development - nothing stops the same defect coming back - and the +// process review put most of the semantic bugs that pass CI down to exactly that. +// +// Full test-first would be the textbook answer and is not realistic here. This is the targeted +// version the review asked for: the place where a test is cheapest to write and worth most is +// the moment a defect has just been understood, so that is where it is required. +// +// The rule: if a branch carries a `fix:` commit, the branch must change something under test/. +// A fix that genuinely cannot be tested - a stylesheet, a config value - says so with a trailer +// in its commit message, and the reason stays in history next to the change: +// +// No-Test: colour only; nothing here a test can observe +// +// Compared against origin/main, so it checks a branch and not the whole history. On main itself, +// or with no origin/main to compare against, there is nothing to check and it passes. + +import { execFileSync } from 'node:child_process'; +import { pathToFileURL } from 'node:url'; +import { kindOf } from './commitKind.mjs'; + +/** + * Whether a branch's fixes carry a test. + * + * @param {Array<{subject: string, body?: string}>} commits the branch's commits + * @param {string[]} files every path the branch changes + * @returns {{ok: boolean, why: string, fixes?: string[]}} + */ +export function verdict(commits, files) { + const fixes = commits.filter(c => kindOf(c.subject) === 'fix'); + if (!fixes.length) return { ok: true, why: 'no fix commits on this branch' }; + if (files.some(f => f.startsWith('test/'))) return { ok: true, why: `${fixes.length} fix commit(s), with a test` }; + for (const c of commits) { + const m = /^No-Test:[ \t]*(\S.*)$/mi.exec(c.body || ''); + if (m) return { ok: true, why: `waived - ${m[1].trim()}` }; + } + return { ok: false, why: 'fix without a test', fixes: fixes.map(c => c.subject) }; +} + +const git = (...a) => execFileSync('git', a, { encoding: 'utf8', stdio: ['ignore', 'pipe', 'ignore'] }); + +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + let base; + try { base = git('merge-base', 'origin/main', 'HEAD').trim(); } catch { + console.log('Fix-needs-a-test skipped - no origin/main to compare against.'); + process.exit(0); + } + const SEP = '\u001e'; + const commits = git('log', '--no-merges', `--format=%s%x1f%b${SEP}`, `${base}..HEAD`) + .split(SEP).map(x => x.trim()).filter(Boolean) + .map(x => { const [subject, body = ''] = x.split('\u001f'); return { subject, body }; }); + const files = git('diff', '--name-only', `${base}...HEAD`).split('\n').filter(Boolean); + const v = verdict(commits, files); + if (!v.ok) { + console.error(`This branch fixes something without changing a test:`); + for (const s of v.fixes) console.error(` ${s}`); + console.error(` +Add the test that fails without the fix - that is what keeps the defect from coming back. If +nothing here can be observed by a test, say why with a trailer in the commit message: + + No-Test: `); + process.exit(1); + } + console.log(`Fix-needs-a-test passed - ${v.why}.`); +} diff --git a/scripts/process-metrics.mjs b/scripts/process-metrics.mjs new file mode 100644 index 00000000..daeca130 --- /dev/null +++ b/scripts/process-metrics.mjs @@ -0,0 +1,214 @@ +// Process metrics, read straight out of the repository's own records. +// +// Until this script, "it is fast" and "it got more stable" were judged by feel. A process change +// can only be called an improvement against a number taken before it, so this produces that +// number - the baseline in a Measure -> Analyze -> Change cycle (docs/process/). +// +// Every figure comes from what GitHub and git already record: merged PRs, issues, Actions runs +// and commit history. Nothing to install and nothing to keep in sync, so it can be rerun at the +// end of each cycle and compared line by line. +// +// node scripts/process-metrics.mjs all of history +// node scripts/process-metrics.mjs --since 30 the last 30 days only +// node scripts/process-metrics.mjs --json machine-readable, for diffing two runs +// +// Needs the gh CLI, authenticated against this repository. + +import { execFileSync } from 'node:child_process'; +import { kindOf } from './commitKind.mjs'; + +const args = process.argv.slice(2); +const sinceDays = args.includes('--since') ? Number(args[args.indexOf('--since') + 1]) : null; +const asJson = args.includes('--json'); +const now = Date.now(); +const DAY = 86_400_000; +const since = sinceDays ? now - sinceDays * DAY : 0; + +const gh = (...a) => JSON.parse(execFileSync('gh', a, { encoding: 'utf8', maxBuffer: 64 << 20 })); +const git = (...a) => execFileSync('git', a, { encoding: 'utf8', maxBuffer: 64 << 20 }); + +const t = (iso) => new Date(iso).getTime(); +const inWindow = (iso) => iso && t(iso) >= since; +const median = (xs) => { if (!xs.length) return null; const s = [...xs].sort((a, b) => a - b); const m = s.length >> 1; return s.length % 2 ? s[m] : (s[m - 1] + s[m]) / 2; }; +const pct = (xs, p) => { if (!xs.length) return null; const s = [...xs].sort((a, b) => a - b); return s[Math.min(s.length - 1, Math.floor(p * s.length))]; }; +const hours = (ms) => ms == null ? null : Math.round(ms / 36e5 * 10) / 10; +const days = (ms) => ms == null ? null : Math.round(ms / DAY * 10) / 10; + + + +// --- the records ------------------------------------------------------------------------------- + +// Paged by hand through GraphQL rather than `gh pr list --json commits`. That command asks for +// every commit's authors as well, gh pages at 100 PRs, and 100 x 100 x 100 is twice GitHub's +// 500,000-node ceiling - it fails at any --limit. Asking for the commit headline alone is 2,000. +function mergedPrs() { + const { owner, name } = gh('repo', 'view', '--json', 'owner,name'); + const q = `query($after: String) { repository(owner: "${owner.login}", name: "${name}") { + pullRequests(states: MERGED, first: 100, after: $after) { + pageInfo { hasNextPage endCursor } + nodes { number title createdAt mergedAt author { login } + commits(first: 20) { nodes { commit { messageHeadline } } } } } } }`; + const out = []; + for (let after = null; ;) { + const page = gh('api', 'graphql', '-f', `query=${q}`, ...(after ? ['-f', `after=${after}`] : [])) + .data.repository.pullRequests; + for (const n of page.nodes) out.push({ ...n, commits: n.commits.nodes.map(c => c.commit) }); + if (!page.pageInfo.hasNextPage) return out; + after = page.pageInfo.endCursor; + } +} +const prs = mergedPrs().filter(p => inWindow(p.mergedAt)); + +// A PR's kind is its title's prefix when it has one. 55 titles were written as prose instead - +// the TypeScript migration series ("hooks/ in TypeScript") is most of them - and counting those +// as "other" shrank the denominator of the escape rate. Their commits did carry a prefix, and +// rebase-merge puts exactly those commits on main, so they are the better witness. +const kindOfPr = (p) => { + const k = kindOf(p.title); + if (k !== 'other') return k; + for (const c of p.commits || []) { const ck = kindOf(c.messageHeadline); if (ck !== 'other') return ck; } + return 'other'; +}; +const issues = gh('issue', 'list', '--state', 'all', '--limit', '2000', '--json', 'number,title,state,createdAt,closedAt,labels,body'); +const runs = gh('run', 'list', '--workflow', 'ci.yml', '--limit', '2000', '--json', 'conclusion,event,createdAt,headBranch') + .filter(r => inWindow(r.createdAt)); + +const human = prs.filter(p => !/dependabot/i.test(p.author?.login || '')); +const byKind = {}; +for (const p of human) (byKind[kindOfPr(p)] ??= []).push(p); +const count = (k) => (byKind[k] || []).length; + +// --- DevOps measures, as the course defines them ------------------------------------------------ + +// Deployment frequency. Every merge to main deploys to Vercel, so a merge is a deployment. +const spanDays = sinceDays ?? Math.max(1, (now - Math.min(...prs.map(p => t(p.mergedAt)))) / DAY); +const deployPerWeek = Math.round(prs.length / spanDays * 7 * 10) / 10; + +// Lead time, PR opened -> merged (deployed). The issue -> close figure is separate because many +// changes never had an issue: they were asked for and done in the same sitting. +const prLead = human.map(p => t(p.mergedAt) - t(p.createdAt)); +const closedIssues = issues.filter(i => i.closedAt && inWindow(i.closedAt)); +const issueLead = closedIssues.map(i => t(i.closedAt) - t(i.createdAt)); + +// Automated test pass ratio. Cancelled runs are left out: the workflow cancels a branch's older +// run when a newer push arrives, so a cancellation is a superseded run, not a verdict. +const judged = runs.filter(r => r.conclusion === 'success' || r.conclusion === 'failure'); +const passRatio = judged.length ? judged.filter(r => r.conclusion === 'success').length / judged.length : null; +const mainRuns = judged.filter(r => r.event === 'push' && r.headBranch === 'main'); +const mainFailures = mainRuns.filter(r => r.conclusion === 'failure').length; + +// Defect escape rate. Nothing reaches main without passing CI, so every `fix:` that lands is a +// defect that passed CI and shipped - an escape, by the definition in the course. Measured +// against the changes that can introduce one. A proxy, and stated as one: a fix for something +// that was never shipped would count too, and that is rare here because work lands as it goes. +const introducing = count('feat') + count('perf') + count('refactor'); +const escapeRate = introducing ? count('fix') / introducing : null; + +// MTTR. Time a bug-labelled issue stayed open. MTTD - from the defect being introduced to it +// being noticed - would need blame on every fix and is not attempted; see the cycle notes. +const bugs = closedIssues.filter(i => i.labels.some(l => l.name === 'bug')); +const mttr = bugs.map(i => t(i.closedAt) - t(i.createdAt)); + +// --- risk indicators, as named in the risk-monitoring section ----------------------------------- + +// Estimation: verification that has been put off. Unticked checklist items in open issues, plus +// issues explicitly waiting on a check on a device. +const open = issues.filter(i => i.state === 'OPEN'); +const unticked = open.reduce((n, i) => n + (String(i.body || '').match(/^\s*[-*] \[ \]/gm) || []).length, 0); +const awaitingCheck = open.filter(i => i.labels.some(l => l.name === 'needs-verification')).length; + +// Tools: the agent undoing its own work. Reverts, and fixes landing on a file that another fix +// touched within the previous week - a fix that needed a fix. +const sinceArg = sinceDays ? [`--since=${sinceDays}.days`] : []; +const log = git('log', '--no-merges', '--format=@@%H%x09%ct%x09%s', '--name-only', ...sinceArg); +const commits = []; +for (const chunk of log.split('@@').slice(1)) { + const [head, ...files] = chunk.trim().split('\n'); + const [hash, ct, subject] = head.split('\t'); + commits.push({ hash, at: Number(ct) * 1000, subject, files: files.filter(Boolean) }); +} +const reverts = commits.filter(c => /^revert/i.test(c.subject)).length; +const fixes = commits.filter(c => kindOf(c.subject) === 'fix'); +// App is left out of the overlap. Nearly every change wires through it, so two fixes sharing it +// says nothing about one fix needing another - counted in, this read 77 and would never move. +const overlapFile = (x) => x.startsWith('src/') && !/^src\/App\.(jsx|tsx)$/.test(x); +let refix = 0; +for (const f of fixes) { + if (fixes.some(g => g !== f && g.at < f.at && f.at - g.at < 7 * DAY && g.files.some(x => overlapFile(x) && f.files.includes(x)))) refix++; +} + +// Test-after, measured. The plan blames semantic defects on tests written after the code; a fix +// that lands without touching a test is the clearest case of it, because nothing then stops the +// same defect coming back. +const fixesWithTest = fixes.filter(f => f.files.some(x => x.startsWith('test/'))).length; + +// People: work stalling. The longest gap between two merges, and how long open issues have sat. +const mergeTimes = prs.map(p => t(p.mergedAt)).sort((a, b) => a - b); +let longestGap = 0; +for (let i = 1; i < mergeTimes.length; i++) longestGap = Math.max(longestGap, mergeTimes[i] - mergeTimes[i - 1]); +const openAge = open.map(i => now - t(i.createdAt)); + +// Where the defects are. Source files touched most often by fixes - the place a test-first rule +// buys the most, which is the targeted adoption the plan calls for. +// Two corrections, or the list says the wrong thing. The TypeScript migration renamed App.jsx to +// App.tsx, so one file's history was split across two names; the extension is dropped so they +// count as one. And the translation tables are left out: every fix that changes a UI string adds +// its translations, so they rank high for touching, not for breaking. +const NOISE = /^src\/i18n(\.\w+)?\.(js|ts)$|\.css$/; +const stem = (x) => x.replace(/\.(jsx|tsx|js|ts)$/, ''); +const hot = {}; +for (const f of fixes) for (const x of f.files) if (x.startsWith('src/') && !NOISE.test(x)) hot[stem(x)] = (hot[stem(x)] || 0) + 1; +const hotspots = Object.entries(hot).sort((a, b) => b[1] - a[1]).slice(0, 8); + +// --- report ------------------------------------------------------------------------------------ + +const result = { + window: sinceDays ? `last ${sinceDays} days` : 'all history', + measuredAt: new Date(now).toISOString().slice(0, 10), + merges: prs.length, + mergesByKind: Object.fromEntries(Object.entries(byKind).map(([k, v]) => [k, v.length]).sort((a, b) => b[1] - a[1])), + deploymentsPerWeek: deployPerWeek, + prLeadTimeHours: { median: hours(median(prLead)), p90: hours(pct(prLead, 0.9)) }, + issueLeadTimeDays: { median: days(median(issueLead)), p90: days(pct(issueLead, 0.9)), closed: closedIssues.length }, + ciPassRatio: passRatio == null ? null : Math.round(passRatio * 1000) / 10, + ciRunsJudged: judged.length, + mainBranchFailures: mainFailures, + defectEscapeRate: escapeRate == null ? null : Math.round(escapeRate * 1000) / 10, + fixMerges: count('fix'), + introducingMerges: introducing, + mttrDays: { median: days(median(mttr)), bugsClosed: bugs.length }, + fixCommitsWithATest: fixes.length ? Math.round(fixesWithTest / fixes.length * 1000) / 10 : null, + fixCommits: fixes.length, + risk: { + untickedChecklistItems: unticked, + issuesAwaitingVerification: awaitingCheck, + reverts, + fixesOfARecentFix: refix, + longestGapBetweenMergesDays: days(longestGap), + openIssues: open.length, + oldestOpenIssueDays: days(Math.max(0, ...openAge)), + }, + defectHotspots: hotspots, +}; + +if (asJson) { + console.log(JSON.stringify(result, null, 2)); +} else { + const r = result; + console.log(`Process metrics - ${r.window}, measured ${r.measuredAt}\n`); + console.log('DevOps measures'); + console.log(` deployment frequency ${r.deploymentsPerWeek} merges/week (${r.merges} merges; each deploys)`); + console.log(` lead time, PR -> deploy median ${r.prLeadTimeHours.median}h, p90 ${r.prLeadTimeHours.p90}h`); + console.log(` lead time, issue -> close median ${r.issueLeadTimeDays.median}d, p90 ${r.issueLeadTimeDays.p90}d (${r.issueLeadTimeDays.closed} issues)`); + console.log(` automated test pass ratio ${r.ciPassRatio}% of ${r.ciRunsJudged} CI runs; ${r.mainBranchFailures} failed on main`); + console.log(` defect escape rate ${r.defectEscapeRate}% (${r.fixMerges} fixes / ${r.introducingMerges} feat+perf+refactor)`); + console.log(` MTTR (bug issues) median ${r.mttrDays.median}d over ${r.mttrDays.bugsClosed} bugs`); + console.log(` fixes carrying a test ${r.fixCommitsWithATest}% of ${r.fixCommits} fix commits`); + console.log(` merges by kind ${Object.entries(r.mergesByKind).map(([k, v]) => `${k} ${v}`).join(', ')}`); + console.log('\nRisk indicators'); + console.log(` estimation unticked checklist items in open issues: ${r.risk.untickedChecklistItems}; awaiting verification: ${r.risk.issuesAwaitingVerification}`); + console.log(` tools reverts: ${r.risk.reverts}; fixes landing on a file another fix touched that week (App aside): ${r.risk.fixesOfARecentFix}`); + console.log(` people longest gap between merges: ${r.risk.longestGapBetweenMergesDays}d; open issues: ${r.risk.openIssues}, oldest ${r.risk.oldestOpenIssueDays}d`); + console.log('\nDefect hotspots (src files most touched by fixes)'); + for (const [f, n] of r.defectHotspots) console.log(` ${String(n).padStart(3)} ${f}`); +} diff --git a/test/scripts/architectureIndex.test.js b/test/scripts/architectureIndex.test.js new file mode 100644 index 00000000..4e535019 --- /dev/null +++ b/test/scripts/architectureIndex.test.js @@ -0,0 +1,33 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { unnamed } from '../../scripts/architecture-index.mjs'; + +// On 2026-10-14 the map was missing 44 of the 152 files under src/. These pin down what "named" +// means, because a loose reading is how the drift was undercounted in the first place: a grep for +// the bare word found 34 missing, since "zip" and "camera" appear in prose without the files +// ever being named. + +test('a file named in backticks is named', () => { + assert.deepEqual(unnamed('- `layerOps.ts` — layer tree', ['src/core/layerOps.ts']), []); +}); + +test('a file mentioned only as a word in prose is not named', () => { + // The export folder was described as "zip, gif, the recorder" and none of its files appeared. + assert.deepEqual(unnamed('export/ turning the timeline into a file: zip, gif, the recorder.', ['src/export/zip.ts']), ['src/export/zip.ts']); +}); + +test('several names in one span, and paths, all count', () => { + const doc = '- `frameExport.ts` / `exportQueue.ts` — sizes\n- `src/App.tsx` — the component\n`a.ts/b.ts`'; + assert.deepEqual(unnamed(doc, ['src/core/frameExport.ts', 'src/core/exportQueue.ts', 'src/App.tsx', 'src/x/a.ts', 'src/x/b.ts']), []); +}); + +test('a declaration file is named by its full name', () => { + assert.deepEqual(unnamed('- `src/globals.d.ts` — vendor globals', ['src/globals.d.ts']), []); + assert.deepEqual(unnamed('- `globals` — vendor globals', ['src/globals.d.ts']), ['src/globals.d.ts']); +}); + +test('the extension is part of the name', () => { + // `App.jsx` in an old line does not cover App.tsx - that is exactly the stale reference the + // map is supposed to stop carrying. + assert.deepEqual(unnamed('- `App.jsx`', ['src/App.tsx']), ['src/App.tsx']); +}); diff --git a/test/scripts/fixNeedsTest.test.js b/test/scripts/fixNeedsTest.test.js new file mode 100644 index 00000000..d654ac8b --- /dev/null +++ b/test/scripts/fixNeedsTest.test.js @@ -0,0 +1,55 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { verdict } from '../../scripts/fix-needs-test.mjs'; +import { kindOf } from '../../scripts/commitKind.mjs'; + +// Measured before this gate existed: 56% of all fix commits, and 40% of the last month's, landed +// without touching a test. The cases below are the ones that decide whether it actually closes +// that gap or only looks like it does. + +const fix = (subject, body = '') => ({ subject, body }); + +test('a fix with no test is refused', () => { + const v = verdict([fix('fix: the pencil keeps answering pressure')], ['src/canvas/strokes.ts']); + assert.equal(v.ok, false); + assert.deepEqual(v.fixes, ['fix: the pencil keeps answering pressure']); +}); + +test('a fix that changes a test passes', () => { + const v = verdict([fix('fix: seek off the frame boundary')], ['src/canvas/videoFrames.ts', 'test/canvas/canvasHelpers.test.js']); + assert.equal(v.ok, true); +}); + +test('a branch with no fix on it is not this gate\'s business', () => { + assert.equal(verdict([fix('feat: drag the frame guide'), fix('docs: the map')], ['src/App.tsx']).ok, true); + assert.equal(verdict([], []).ok, true, 'main itself, or a branch with nothing on it'); +}); + +test('a scoped or breaking fix still counts as a fix', () => { + // Otherwise `fix(export):` would walk straight past the rule. + assert.equal(verdict([fix('fix(export): ask for mp4 by its real name')], ['src/export/recorder.ts']).ok, false); + assert.equal(verdict([fix('fix!: change the save format')], ['src/core/projectFormat.ts']).ok, false); +}); + +test('a waiver passes, and only with a reason', () => { + const waived = verdict([fix('fix: the panel colour', 'Slightly darker.\n\nNo-Test: colour only; nothing a test can observe')], ['src/styles/panel.css']); + assert.equal(waived.ok, true); + assert.match(waived.why, /colour only/); + // An empty trailer is not a reason. It would let the waiver become a habit with nothing in + // history saying why each one was allowed. + assert.equal(verdict([fix('fix: x', 'No-Test:')], ['src/a.ts']).ok, false); + assert.equal(verdict([fix('fix: x', 'No-Test: ')], ['src/a.ts']).ok, false); +}); + +test('a file merely named like a test does not count', () => { + // Only a change under test/ is a test. A source file with "test" in its name is not one. + assert.equal(verdict([fix('fix: x')], ['src/core/testHelpers.ts', 'scripts/smoke-test.mjs']).ok, false); +}); + +test('the gate and the metrics agree on what a fix is', () => { + // They share kindOf. If they drifted, the gate could pass a change the metrics then count + // as an untested fix, and the number this gate exists to move would stop meaning anything. + for (const [s, k] of [['fix: a', 'fix'], ['fix(scope): a', 'fix'], ['Fix: a', 'fix'], ['feat: a', 'feat'], ['hooks/ in TypeScript', 'other']]) { + assert.equal(kindOf(s), k, s); + } +});