Fail the Semantic Diff check for commits without a description

.github/pull_request_template.md

@@ -2,4 +2,5 @@
 
 - [ ] Added a changelog fragment (see `changes/README.md`), or no release notes are needed and a maintainer should apply `changelog:skip`.
 - [ ] Added tests for the fix or feature.
+- [ ] Gave every commit a description below its subject, saying why it is made (see "Commit messages" in `CONTRIBUTING.md`).
 - [ ] Signed off every commit for the DCO (`git commit -s`).

.github/workflows/semantic_diff.yml

@@ -5,7 +5,9 @@ name: Semantic Diff
 # Markdown (scripts/recordMarkdown.js) with the page's link. A pull request
 # has one such comment, which each push updates in place to hold every
 # commit, so the pull request shows its current records in one place; the
-# comment marks the commits the last push added or edited.
+# comment marks the commits the last push added or edited. After commenting,
+# the job fails if a commit's message has no description below its subject,
+# naming those commits.
 #
 # A pull request from a branch of this repository runs on pull_request, with
 # the workflow and renderer of its merge into the base branch: GitHub takes
@@ -107,3 +109,7 @@ jobs:
           comment-tag: semantic-diff
           mode: upsert
           file-path: semantic-diff/comment.md
+
+      # Last, so that the page and the comment show the records either way.
+      - name: Check commit descriptions
+        run: node scripts/commitRecords.js --check "$(git merge-base "$BASE_SHA" "pr-${PR_NUMBER}")..pr-${PR_NUMBER}"

AGENTS.md

@@ -78,6 +78,10 @@ relevant guidance from each.
 - Write each commit message as a record in the format of
   [`.agents/skills/records/SKILL.md`](.agents/skills/records/SKILL.md), and
   render review pages from records with `make semantic-diff`
+- Every commit needs a description below its subject: a line or two of
+  prose or a `Why:` section. This includes small follow-ups like adding a
+  changelog fragment or fixing CI; a subject alone fails the Semantic Diff
+  check (`node scripts/commitRecords.js --check origin/master..HEAD`)
 
 ### Stacked pull requests
 

CONTRIBUTING.md

@@ -570,6 +570,21 @@ Javascript Code
 
 Note that there is one design goal to keep in mind, never introduce any meaningless symbol unless necessary, we do optimizations, however, it should also compile readable output code.
 
+## Commit messages
+
+Every commit message has a subject, a description and a sign-off. The description says why the commit is made, as a line or two of prose or a `Why:` section; this applies to small follow-up commits too, such as adding a changelog fragment. Pull requests are squash-merged with their commit messages, so these descriptions end up in `master`'s history.
+
+    Warn about unused open statements
+
+    An open whose module contributes no name silently widens the scope;
+    the new warning points at it.
+
+    Signed-off-by: Random J. Developer <[email protected]>
+
+The Semantic Diff workflow draws each pull request's commit messages as a review page and comments them on the pull request. It fails if a commit has no description and names that commit; add one with `git rebase -i`, marking the commit `reword`, and force-push. Run the same check locally with `node scripts/commitRecords.js --check origin/master..HEAD`.
+
+A message can also show what the commit changes, such as the parse tree, generated JavaScript or an error message, Before and After; [`.agents/skills/records/SKILL.md`](.agents/skills/records/SKILL.md) describes the format.
+
 ## PR target branch
 
 Target branch `master` for development of new (breaking) features (v12).

changes/commit-description-check.internal.md

@@ -0,0 +1 @@
+- Fail the Semantic Diff check when a commit message has no description below its subject.

scripts/commitRecords.js

@@ -16,13 +16,22 @@
  *   the commits' authors, drawn in place of their names. With
  *   --previous <rev>, the pull request's head before the last push, the
  *   comment marks the commits that push added or whose message it edited.
+ *   --check fails when a record has no description, a paragraph of prose
+ *   or a `Why:` section, naming those records; it needs no --out.
  */
 
 import { execFileSync } from "node:child_process";
 import { readFileSync, writeFileSync } from "node:fs";
 import { categories, parseFragment } from "./changelogFragments.js";
 import { commentMarkdown } from "./recordMarkdown.js";
-import { CSS, esc, pageBody, pageTitle, parseRecord } from "./recordRender.js";
+import {
+  CSS,
+  described,
+  esc,
+  pageBody,
+  pageTitle,
+  parseRecord,
+} from "./recordRender.js";
 
 /** @import { CommitRecord, Group } from "./recordRender.js" */
 /** @import { ChangelogEntry } from "./recordMarkdown.js" */
@@ -183,7 +192,8 @@ const options = {};
 /** @type {string[]} */
 const targets = [];
 for (let i = 0; i < args.length; i++) {
-  if (
+  if (args[i] === "--check") options.check = "true";
+  else if (
     [
       "--out",
       "--pr",
@@ -199,9 +209,9 @@ for (let i = 0; i < args.length; i++) {
   else targets.push(args[i]);
 }
 const outFile = options.out;
-if (!outFile) {
+if (!outFile && !options.check) {
   console.error(
-    "usage: commitRecords.js --out <page.html> [--pr <number>] [--title <text>] [--markdown <comment.md> [--url <page url>] [--previous <rev>]] [--logins <file>] <rev | base..tip>...",
+    "usage: commitRecords.js (--out <page.html> [--pr <number>] [--title <text>] [--markdown <comment.md> [--url <page url>] [--previous <rev>]] [--logins <file>] | --check) <rev | base..tip>...",
   );
   process.exit(1);
 }
@@ -218,27 +228,56 @@ const heading = {
   title: options.title ?? "",
 };
 const groups = targets.map(group);
-const body = pageBody(groups, heading);
-if (options.markdown) {
-  const [base, tip] = targets[targets.length - 1].split("..");
-  const records = groups.flatMap(g => g.records);
+
+/** Draws the page and, with --markdown, the comment. @param {string} outFile */
+function draw(outFile) {
+  const body = pageBody(groups, heading);
+  if (options.markdown) {
+    const [base, tip] = targets[targets.length - 1].split("..");
+    const records = groups.flatMap(g => g.records);
+    writeFileSync(
+      options.markdown,
+      commentMarkdown(groups, {
+        url: options.url,
+        ...(tip ? { changelog: changelog(base, tip) } : {}),
+        ...(options.previous && tip
+          ? marks(records, base, options.previous)
+          : {}),
+      }),
+    );
+    console.log(options.markdown);
+  }
+  const head = `<title>${esc(pageTitle(heading))}</title>
+<link rel="stylesheet" href="https://fonts.googleapis.com/css2?family=Fraunces:opsz,[email protected],600&family=Inter:wght@400;500;600&family=JetBrains+Mono:wght@400;500&family=STIX+Two+Text:ital,wght@0,400;0,500;1,400&display=swap">
+<style>${CSS}</style>`;
   writeFileSync(
-    options.markdown,
-    commentMarkdown(groups, {
-      url: options.url,
-      ...(tip ? { changelog: changelog(base, tip) } : {}),
-      ...(options.previous && tip
-        ? marks(records, base, options.previous)
-        : {}),
-    }),
+    outFile,
+    `<!doctype html>\n<html lang="en"><head><meta charset="utf-8"><meta name="viewport" content="width=device-width, initial-scale=1">\n${head}</head><body>${body}</body></html>\n`,
   );
-  console.log(options.markdown);
+  console.log(outFile);
 }
-const head = `<title>${esc(pageTitle(heading))}</title>
-<link rel="stylesheet" href="https://fonts.googleapis.com/css2?family=Fraunces:opsz,[email protected],600&family=Inter:wght@400;500;600&family=JetBrains+Mono:wght@400;500&family=STIX+Two+Text:ital,wght@0,400;0,500;1,400&display=swap">
-<style>${CSS}</style>`;
-writeFileSync(
-  outFile,
-  `<!doctype html>\n<html lang="en"><head><meta charset="utf-8"><meta name="viewport" content="width=device-width, initial-scale=1">\n${head}</head><body>${body}</body></html>\n`,
-);
-console.log(outFile);
+
+/**
+ * Names the records without a description and fails, telling how to add
+ * one. A pull request's check runs this after its page and comment are
+ * drawn, so the comment shows the records as they are.
+ */
+function check() {
+  const missing = groups.flatMap(g => g.records).filter(r => !described(r));
+  if (!missing.length) return;
+  console.error(
+    missing.length === 1
+      ? "A commit has no description:\n"
+      : `${missing.length} commits have no description:\n`,
+  );
+  for (const r of missing) console.error(`  ${r.sha} ${r.subject}`);
+  console.error(`
+Below its subject, a commit message says why the commit is made, as a line
+or two of prose or a \`Why:\` section (see "Commit messages" in
+CONTRIBUTING.md). Add one with \`git rebase -i\`, marking these commits
+\`reword\`, or squash them into a commit that has one, and force-push.`);
+  process.exit(1);
+}
+
+if (outFile) draw(outFile);
+if (options.check) check();

scripts/commitRecords.test.js

@@ -1026,3 +1026,50 @@ test("--previous marks the commits the last push added or edited", async () => {
   has(marked("Added"), "<summary>✨ <b>Added</b></summary>");
   has(marked("Added"), " · new</sub>");
 });
+
+test("--check names the commits without a description and fails", async () => {
+  const dir = await fs.mkdtemp(path.join(os.tmpdir(), "commit-records-"));
+  const git = (/** @type {string[]} */ ...a) =>
+    execFileSync("git", a, { cwd: dir, encoding: "utf8" }).trim();
+  git("init", "-q", "-b", "main");
+  git("config", "user.name", "A Person");
+  git("config", "user.email", "[email protected]");
+  await commit(git, dir, { "a.txt": "a\n" }, "base");
+  const base = git("rev-parse", "HEAD");
+  const check = () =>
+    spawnSync("node", [script, "--check", `${base}..HEAD`], {
+      cwd: dir,
+      encoding: "utf8",
+    });
+  await commit(git, dir, {}, `Prose\n\nA line of why.\n\n${SIGNED}`);
+  await commit(git, dir, {}, `Why block\n\nWhy:\n    The reason.\n\n${SIGNED}`);
+  await commit(
+    git,
+    dir,
+    {},
+    `Why on its line\n\nWhy: the reason.\nReview: a question?\n\n${SIGNED}`,
+  );
+  let r = check();
+  assert.equal(r.status, 0, r.stderr);
+  await commit(git, dir, {}, `Title only\n\n${SIGNED}`);
+  await commit(
+    git,
+    dir,
+    {},
+    `Trailers only\n\nCo-Authored-By: B <[email protected]>\n${SIGNED}`,
+  );
+  await commit(
+    git,
+    dir,
+    {},
+    `Blocks only\n\nJS modified:\nBefore (js):\n    a;\nAfter (js):\n    b;\n\nReview: a question?\n\n${SIGNED}`,
+  );
+  await commit(git, dir, {}, `Empty why\n\nWhy:\n\n${SIGNED}`);
+  r = check();
+  assert.equal(r.status, 1);
+  assert.match(r.stderr, /^4 commits have no description/);
+  for (const s of ["Title only", "Trailers only", "Blocks only", "Empty why"])
+    assert.ok(r.stderr.includes(` ${s}\n`), `stderr lacks ${s}`);
+  for (const s of ["Prose", "Why block", "Why on its line"])
+    assert.ok(!r.stderr.includes(` ${s}\n`), `stderr has ${s}`);
+});

scripts/recordRender.js

@@ -193,6 +193,19 @@ export function parseRecord(commit) {
   return { ...commit, subject, sections, coauthors: [...new Set(coauthors)] };
 }
 
+/**
+ * Whether a record says why its commit is made, as the format requires: a
+ * paragraph of prose or a `Why:` section with content. A subject with only
+ * trailers, object blocks or code has no description.
+ * @param {CommitRecord} r
+ */
+export const described = r =>
+  r.sections.some(
+    s =>
+      (s.name === "" && s.text.length > 0) ||
+      (s.name === "Why" && (s.text.length > 0 || s.block.some(l => l.trim()))),
+  );
+
 /**
  * Who made a commit, as `A Person + Claude`: its author, by the GitHub login
  * when known, and its co-authors, each name drawn by `name` and the login