🏗️🚀：read a message without segmenting every line

build/shared/commit-message.mts

@@ -122,6 +122,16 @@ const ASSISTED_BY_VALUE = /^[^\s:]+:\S+( \S+)*$/;
 /** git folds a trailer whose value runs onto an indented line beneath it. */
 const CONTINUATION_LINE = /^\s/;
 
+/**
+ * Each known token with the pattern that also matches it misspelt with a
+ * space where a hyphen belongs. Built here because the check that uses them
+ * runs for every line of the last paragraph.
+ */
+const SPACED_TRAILER_TOKENS = TRAILER_ORDER.map((token) => ({
+  token,
+  spaced: new RegExp(`^${token.replaceAll('-', '[ -]')}:`, 'i'),
+}));
+
 /**
  * Splits a commit message into its lines, without the blank ones git leaves
  * at the end. Written as a scan rather than as `/[\r\n]+$/`, which takes time
@@ -146,29 +156,42 @@ export function linesOf(message: string) {
 }
 
 /**
- * Splits a message body into paragraphs of non-empty lines.
+ * Splits a message body into paragraphs of non-empty lines. Walks the lines
+ * it is given, so nothing copies the message.
  * @param {string[]} lines Every line after the subject.
  * @returns {string[][]} The paragraphs, in order.
  */
-const paragraphsOf = (lines: string[]) =>
-  lines
-    .join('\n')
-    .split(/\n{2,}/)
-    .map((paragraph) => paragraph.split('\n').filter(Boolean))
-    .filter((paragraph) => paragraph.length > 0);
+const paragraphsOf = (lines: string[]) => {
+  const paragraphs: string[][] = [];
+  let paragraph: string[] = [];
+
+  for (const line of lines) {
+    if (line === '') {
+      if (paragraph.length > 0) {
+        paragraphs.push(paragraph);
+        paragraph = [];
+      }
+    } else {
+      paragraph.push(line);
+    }
+  }
+
+  if (paragraph.length > 0) paragraphs.push(paragraph);
+
+  return paragraphs;
+};
 
 /**
- * Reads the trailer block out of a message, agreeing with git about whether
- * there is one: the last paragraph, every line of it either a trailer or a
- * continuation of the one above, and the first of them a trailer. A paragraph
- * that merely contains a colon somewhere is prose, and git reads no trailers
- * in it -- so neither does this.
- * @param {string} message The whole commit message.
+ * Reads the trailer block out of a message's paragraphs, agreeing with git
+ * about whether there is one: the last paragraph, every line of it either a
+ * trailer or a continuation of the one above, and the first of them a
+ * trailer. A paragraph that merely contains a colon somewhere is prose, and
+ * git reads no trailers in it -- so neither does this.
+ * @param {string[][]} paragraphs The paragraphs after the subject.
  * @returns {string[]} The trailer lines, one per trailer, empty if there is no block.
  */
-export function readTrailers(message: string) {
-  const [, ...rest] = linesOf(message);
-  const last = paragraphsOf(rest).at(-1) ?? [];
+const trailerBlockOf = (paragraphs: string[][]) => {
+  const last = paragraphs.at(-1) ?? [];
   const isBlock =
     last.length > 0 &&
     TRAILER_LINE.test(last[0] ?? '') &&
@@ -177,6 +200,17 @@ export function readTrailers(message: string) {
     );
 
   return isBlock ? last.filter((line) => !CONTINUATION_LINE.test(line)) : [];
+};
+
+/**
+ * Reads the trailer block out of a whole message.
+ * @param {string} message The whole commit message.
+ * @returns {string[]} The trailer lines, one per trailer, empty if there is no block.
+ */
+export function readTrailers(message: string) {
+  const [, ...rest] = linesOf(message);
+
+  return trailerBlockOf(paragraphsOf(rest));
 }
 
 /**
@@ -276,9 +310,7 @@ const checkTrailers = (lines: string[]) => {
   // known tokens with their hyphens loosened, since a looser test than that
   // flags any body sentence containing a colon.
   for (const line of last) {
-    for (const token of TRAILER_ORDER) {
-      const spaced = new RegExp(`^${token.replaceAll('-', '[ -]')}:`, 'i');
-
+    for (const { token, spaced } of SPACED_TRAILER_TOKENS) {
       if (
         spaced.test(line) &&
         !line.toLowerCase().startsWith(`${token.toLowerCase()}:`)
@@ -306,7 +338,7 @@ const checkTrailers = (lines: string[]) => {
   }
 
   const tokens: string[] = [];
-  const block = readTrailers(['', ...lines].join('\n'));
+  const block = trailerBlockOf(paragraphs);
 
   // A last paragraph that is not a clean block is prose, and git reads no
   // trailers in it. Saying so is only worth doing for a line that was plainly
@@ -403,10 +435,18 @@ export function validateCommitMessage(message: string) {
 
     // An unbreakable line -- a URL, near enough always -- cannot be wrapped,
     // and reflowing one to fit would break it.
-    if (countGraphemes(line) > BODY_MAX && /\s/.test(line.trim())) {
-      problems.push(
-        `line is ${countGraphemes(line)} characters; the limit is ${BODY_MAX}: “${line.slice(0, 40)}…”`
-      );
+    //
+    // No grapheme is fewer than one code unit, so a line of BODY_MAX code
+    // units or fewer is within the limit whatever it is made of, and the
+    // segmenter need not see it.
+    if (line.length > BODY_MAX && /\s/.test(line.trim())) {
+      const width = countGraphemes(line);
+
+      if (width > BODY_MAX) {
+        problems.push(
+          `line is ${width} characters; the limit is ${BODY_MAX}: “${line.slice(0, 40)}…”`
+        );
+      }
     }
   }
 

build/shared/commit-message.test.mts

@@ -368,35 +368,179 @@ describe('the vocabulary', () => {
 
 describe('a message written to be slow', () => {
   // A commit message comes from whoever opened the pull request, and the
-  // commit queue reads it holding credentials that can write here. Taking
-  // time proportional to the square of its length is a way to stop the queue
-  // working, so these hold the rules to reading it in linear time.
-  // Generous, because the work itself is linear and CI is slow. What it
-  // separates is linear from quadratic: at this size the regexes these
-  // replaced took twenty seconds and ninety seconds respectively.
-  const budget = 3000;
+  // commit queue reads it holding credentials that can write here. How long
+  // it can be made to take is therefore a security property, and these hold
+  // the rules to it.
+  //
+  // Each is a ratio between two measurements taken on the same machine
+  // moments apart, one of them work known to be linear. A machine that is
+  // slow, or busy, raises both and leaves the ratio alone, where a figure in
+  // milliseconds would measure the machine as much as the code.
+
+  /** Timings to take for each measurement, of which the fastest is kept. */
+  const runs = 5;
+
+  /** Times to measure both sides of a ratio, alternating between them. */
+  const rounds = 3;
+
+  /** The size of the messages measured, in lines or in repetitions. */
+  const size = 100_000;
+
+  /**
+   * How long something takes, at its fastest out of a few goes.
+   *
+   * The fastest, because everything that happens to a timing on a shared
+   * machine -- losing the processor, waiting on a collection, sharing a core
+   * -- adds to it and nothing takes away. The smallest of several is the
+   * closest to what the work itself costs.
+   * @param {() => unknown} work What to measure.
+   * @returns {number} The smallest of `runs` timings, in milliseconds.
+   */
+  const spentOn = (work: () => unknown) => {
+    let fastest = Infinity;
+
+    for (let run = 0; run < runs; run += 1) {
+      const started = performance.now();
+
+      work();
+
+      fastest = Math.min(fastest, performance.now() - started);
+    }
 
-  test('reads a long run of newlines quickly', () => {
-    const message = `🏗️🔧：fix it\n\nA body.${'\n'.repeat(200_000)}x`;
-    const started = performance.now();
+    return fastest;
+  };
+
+  /**
+   * What one piece of work costs in multiples of another.
+   * @param {() => unknown} measured The work being judged.
+   * @param {() => unknown} baseline The work it is judged against.
+   * @returns {number} The multiple.
+   */
+  const ratioOf = (measured: () => unknown, baseline: () => unknown) => {
+    // Leaves both compiled before either is measured.
+    measured();
+    baseline();
+
+    let cost = Infinity;
+    let floor = Infinity;
+
+    // Alternated, so that a machine getting busier partway through raises
+    // both sides.
+    for (let round = 0; round < rounds; round += 1) {
+      floor = Math.min(floor, spentOn(baseline));
+      cost = Math.min(cost, spentOn(measured));
+    }
 
-    validateCommitMessage(message);
+    return cost / floor;
+  };
+
+  /**
+   * The floor: split a message into lines and look at each one once. Reading
+   * a message cannot cost less than this, so it stands in for how fast this
+   * machine is at the work in question.
+   *
+   * Written out here rather than taken from the module, so that a change to
+   * the module's own splitting moves the measurement without moving the
+   * floor.
+   * @param {string} message The message to read.
+   * @returns {number} How many lines were a run of dashes, which is beside the point.
+   */
+  const readEveryLine = (message: string) => {
+    let seen = 0;
+
+    for (const line of message.split(/\r?\n/)) {
+      if (/^-{3,}$/.test(line)) seen += 1;
+    }
 
-    const spent = performance.now() - started;
+    return seen;
+  };
+
+  /**
+   * What reading a message costs against looking at every line of it once.
+   * @param {string} message The message to read.
+   * @returns {number} The multiple.
+   */
+  const costOfReading = (message: string) =>
+    ratioOf(
+      () => validateCommitMessage(message),
+      () => readEveryLine(message)
+    );
 
-    ok(spent < budget, `took ${spent.toFixed(0)}ms, budget ${budget}ms`);
+  /**
+   * A message of blank lines gives the reading almost nothing to do beyond
+   * splitting, so its cost sits just above the floor and anything spent per
+   * line shows at once. Costs about 1, and 9 with a stray second pass over
+   * the lines.
+   */
+  const blankLineCeiling = 5;
+
+  /**
+   * A message that is all one paragraph is trailer candidates from top to
+   * bottom, so the reading has real work to do and the ratio is both higher
+   * and looser under load. A bound on the whole rather than a fine measure.
+   */
+  const paragraphCeiling = 30;
+
+  test('reads a body of many lines without looking at each one twice', () => {
+    const message = [
+      '🏗️🔧：fix it',
+      '',
+      ...Array.from({ length: size }, () => 'x'),
+    ].join('\n');
+    const factor = costOfReading(message);
+
+    ok(
+      factor < paragraphCeiling,
+      `reading cost ${factor.toFixed(1)}x looking at every line, ceiling ${paragraphCeiling}x`
+    );
   });
 
-  test('reads a long Assisted-by value quickly', () => {
-    // `\S` matches a colon, so the obvious spelling of agent:model lets the
-    // engine try every colon as the split point.
-    const message = `🏗️🔧：fix it\n\nAssisted-by: ${'a:'.repeat(100_000)} `;
-    const started = performance.now();
+  test('reads a long run of blank lines without segmenting each one', () => {
+    const message = `🏗️🔧：fix it\n\nA body.${'\n'.repeat(size)}x`;
+    const factor = costOfReading(message);
 
-    validateCommitMessage(message);
+    ok(
+      factor < blankLineCeiling,
+      `reading cost ${factor.toFixed(1)}x looking at every line, ceiling ${blankLineCeiling}x`
+    );
+  });
 
-    const spent = performance.now() - started;
+  test('strips a trailing run of newlines without backtracking', () => {
+    // A run of newlines ending on something else is what a regex written to
+    // strip it backtracks over; `linesOf` scans. The floor splits the same
+    // message without that cost, so such a change shows here.
+    const message = `🏗️🔧：fix it\n\nA body.${'\n'.repeat(size)}\nend`;
+    const factor = costOfReading(message);
 
-    ok(spent < budget, `took ${spent.toFixed(0)}ms, budget ${budget}ms`);
+    ok(
+      factor < blankLineCeiling,
+      `reading cost ${factor.toFixed(1)}x looking at every line, ceiling ${blankLineCeiling}x`
+    );
+  });
+
+  test('reads an Assisted-by value that does not match, quickly', () => {
+    // `\S` matches a colon, so the obvious spelling of agent:model lets the
+    // engine try every colon as the split point. This value is built to fail,
+    // which is when a pattern that can backtrack does so.
+    //
+    // Judged against a value of the same length that matches at once: the
+    // message is one line, so splitting it measures nothing. Both sides do
+    // the same work but for the pattern.
+    const failing = `🏗️🔧：fix it\n\nAssisted-by: ${'a:'.repeat(size)} `;
+    const matching = `🏗️🔧：fix it\n\nAssisted-by: Claude-Code:${'a'.repeat(
+      2 * size - 12
+    )}`;
+    const factor = ratioOf(
+      () => validateCommitMessage(failing),
+      () => validateCommitMessage(matching)
+    );
+
+    // The two sit within a few percent of each other while the pattern holds.
+    const patternCeiling = 10;
+
+    ok(
+      factor < patternCeiling,
+      `the failing value cost ${factor.toFixed(1)}x the matching one, ceiling ${patternCeiling}x`
+    );
   });
 });