From 82d369e244d68fa09a95272b11dfe0081469e573 Mon Sep 17 00:00:00 2001 From: Michael O'Boyle Date: Thu, 1 Oct 2026 09:49:57 -0400 Subject: [PATCH] fix(ci): the mutation gate failed its own baseline, because the report rounded The first real run of the job added in #66 went red, correctly, against the suite its floor was measured on. 131/257 is **50.97%**. The report printed it with `toFixed(1)` as "51.0%". I read that printed value back and set the floor to 51. Then `score < threshold` is `50.97 < 51`, which is true, so the gate rejected the exact baseline it was calibrated from. The instrument rounded, and its own output was treated as ground truth. ## Fix `fmtScore` truncates toward zero at two decimals instead of rounding, so the printed number is always a **lower bound** on the real score. A floor copied from the job output can then never exceed the score that produced it. Two decimals because one does not separate 50.97 from 51. Floor corrected to `50.97`, with the reason recorded next to it in the workflow so the next person to move it knows why it is not a round number. ## Tests Four new, suite 189/189. Red-first: restoring `toFixed(1)` turns all four red. The fourth is the property rather than an example: for a range of killed/total pairs, a floor copied from `fmtScore` must never fail the score that produced it. That is the invariant the gate actually depends on, and stating it as a property is what stops the next off-by-one at a different denominator. ## Why this is worth its own commit The gate did its job on its first outing, and what it caught was a defect in the gate. Two things follow that are worth writing down. The failure was **real, not a false alarm**. A floor that rejects its own baseline would have failed every nightly run until someone edited the number, and the obvious edit under time pressure is to lower the floor, which quietly removes the ratchet the floor exists to provide. And it is the same class of error as the one the job was built to find. #66's own commit message says a high mutation score is not evidence of correctness. Here a *rounded* score was not evidence of the floor being safe. In both cases the measurement was fine and the reading of it was wrong. --- .github/workflows/mutation.yml | 8 +++++++- scripts/mutate.mjs | 23 ++++++++++++++++++--- test/mutate.test.js | 37 +++++++++++++++++++++++++++++++++- 3 files changed, 63 insertions(+), 5 deletions(-) diff --git a/.github/workflows/mutation.yml b/.github/workflows/mutation.yml index e8ba0bc..7cc7344 100644 --- a/.github/workflows/mutation.yml +++ b/.github/workflows/mutation.yml @@ -51,7 +51,13 @@ jobs: # The floor is the score measured when this job landed, so the suite # can only ratchet upward. Moving it DOWN is a deliberate act that # belongs in a PR with a reason, not a quiet edit. - MUTATION_FLOOR: '51' + # + # Take this number from the job output verbatim. That output truncates + # rather than rounds, precisely so a value copied from it is always a + # safe floor. 131/257 is 50.97%, not the 51.0% an earlier rounded + # report printed, and that one-hundredth of a point failed the gate on + # the very suite it was measured against. + MUTATION_FLOOR: '50.97' - name: Publish the report if: always() diff --git a/scripts/mutate.mjs b/scripts/mutate.mjs index 7152d92..8881867 100644 --- a/scripts/mutate.mjs +++ b/scripts/mutate.mjs @@ -71,6 +71,23 @@ const OPERATORS = [ // prose. A mutated comment is always a survivor and always meaningless, and a // few hundred of those would bury the real findings. // --------------------------------------------------------------------------- +/** + * Format a score so the printed number is safe to paste into the floor. + * + * `toFixed` ROUNDS, and that is a trap for this specific use. The first CI run + * of this job failed its own baseline: 131/257 is 50.97%, which printed as + * "51.0%", the floor was set from that printed value, and `score < threshold` + * was then true against the very suite the floor was measured on. The + * instrument rounded and its own output was read back as ground truth. + * + * Truncating toward zero makes the printed value a lower bound on the real one, + * so a floor copied from this output can never exceed the score that produced + * it. Two decimals, because one does not separate 50.97 from 51. + */ +export function fmtScore(score){ + return (Math.floor(score * 100) / 100).toFixed(2); +} + export function maskNonCode(src) { const out = src.split(''); let i = 0; @@ -189,7 +206,7 @@ async function main() { const survivors = results.filter(r => !r.killed); const score = results.length ? (killed / results.length) * 100 : 100; - console.log(`\n\n${killed}/${results.length} mutants killed. Mutation score ${score.toFixed(1)}%.`); + console.log(`\n\n${killed}/${results.length} mutants killed. Mutation score ${fmtScore(score)}%.`); if (survivors.length) { console.log(`\n${survivors.length} SURVIVORS (behaviour no test asserts on):`); for (const s of survivors) console.log(` ${s.file}:${s.line} ${s.label}\n ${s.context}`); @@ -201,7 +218,7 @@ async function main() { } if (threshold !== null && score < threshold) { - console.error(`\nMutation score ${score.toFixed(1)}% is below the floor of ${threshold}%.`); + console.error(`\nMutation score ${fmtScore(score)}% is below the floor of ${threshold}%.`); console.error('Either add an assertion that kills a survivor above, or move the floor deliberately.'); process.exit(1); } @@ -212,7 +229,7 @@ function renderReport({ results, killed, survivors, score, targets }) { for (const s of survivors) (byFile[s.file] ||= []).push(s); return [ '# Mutation report', '', - `**${killed}/${results.length} mutants killed. Score ${score.toFixed(1)}%.**`, '', + `**${killed}/${results.length} mutants killed. Score ${fmtScore(score)}%.**`, '', 'A *survivor* is a deliberate break in the source that the test suite did not', 'notice. It marks behaviour nothing asserts on. The score is a summary; the', 'survivor list is the finding.', '', diff --git a/test/mutate.test.js b/test/mutate.test.js index b07e845..b7bd457 100644 --- a/test/mutate.test.js +++ b/test/mutate.test.js @@ -7,7 +7,7 @@ import { test } from 'node:test'; import assert from 'node:assert/strict'; import { readFileSync } from 'node:fs'; -import { maskNonCode, planMutations } from '../scripts/mutate.mjs'; +import { maskNonCode, planMutations, fmtScore } from '../scripts/mutate.mjs'; // --------------------------------------------------------------------------- // Masking. A mutation landing in a comment or a string is always a survivor and @@ -122,3 +122,38 @@ test('importing the harness does not mutate anything', async () => { assert.equal(readFileSync(target, 'utf8'), before, 'importing scripts/mutate.mjs must not touch source files'); }); + +// --------------------------------------------------------------------------- +// Score formatting. The printed score is copied into the workflow's floor, so +// it has to be a LOWER bound on the real value. `toFixed` rounds, and that +// failed the gate on its own baseline: 131/257 is 50.97%, printed as "51.0%", +// floor set to 51, and 50.97 < 51. +// --------------------------------------------------------------------------- +test('the printed score truncates rather than rounds, so it is safe as a floor', () => { + const real = (131 / 257) * 100; // 50.97276... + assert.equal(fmtScore(real), '50.97'); + assert.ok(Number(fmtScore(real)) <= real, + 'the printed value must never exceed the real score, or a copied floor fails its own baseline'); +}); + +test('a score that would round UP is still printed down', () => { + assert.equal(fmtScore(50.999), '50.99', 'must not print 51.00'); + assert.equal(fmtScore(99.999), '99.99', 'must not print 100.00 for an imperfect suite'); + assert.ok(Number(fmtScore(50.999)) <= 50.999); +}); + +test('exact values are preserved', () => { + assert.equal(fmtScore(100), '100.00'); + assert.equal(fmtScore(0), '0.00'); + assert.equal(fmtScore(51), '51.00'); +}); + +test('a floor copied from the output always passes the suite it was measured on', () => { + // The property the gate actually needs, stated as a property. + for (const [killed, total] of [[131, 257], [1, 3], [2, 3], [99, 100], [7, 9], [0, 5], [5, 5]]) { + const score = (killed / total) * 100; + const floor = Number(fmtScore(score)); + assert.ok(!(score < floor), + `floor ${floor} copied from ${killed}/${total} must not fail score ${score}`); + } +});