diff --git a/build/shared/commit-message.mts b/build/shared/commit-message.mts index b35f5d00..1ee7b90e 100644 --- a/build/shared/commit-message.mts +++ b/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)}…”` + ); + } } } diff --git a/build/shared/commit-message.test.mts b/build/shared/commit-message.test.mts index 552ceb74..374332d0 100644 --- a/build/shared/commit-message.test.mts +++ b/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` + ); }); });