diff --git a/.changeset/silly-lynx-click.md b/.changeset/silly-lynx-click.md new file mode 100644 index 000000000..6f8c96036 --- /dev/null +++ b/.changeset/silly-lynx-click.md @@ -0,0 +1,5 @@ +--- +type: Changed +pr: 4418 +--- +**Size-cap checks expose pressure before the hard limit** — workflow and agent suites report every capped file's remaining headroom and flag files past the 95% reserved margin. (#4261) diff --git a/docs/TESTING-SUITES.md b/docs/TESTING-SUITES.md index fd37ff4bf..e07c584ad 100644 --- a/docs/TESTING-SUITES.md +++ b/docs/TESTING-SUITES.md @@ -121,6 +121,17 @@ layers: |---|---|---| | **Differential attribution size ratchet** (primary, #2724 / ADR-2719 §4) | The same computed-attribution check that replaced the golden-install-parity fixtures also reports growth in any `gsd-core/workflows/*.md` or `agents/gsd-*.md` file, with the exact byte delta, comparing PR HEAD against `next`. Unacknowledged growth is a hard failure; shrinkage needs no acknowledgment. No committed snapshot — nothing to regenerate by hand. | `tests/emitted-attribution.test.cjs` (real-tree test) via `tests/helpers/emitted-diff.cjs` | | **Loose tier hard caps** (backstop) | Absolute outer red lines per tier — workflows: `XL ≤ 98304`, `LARGE ≤ 61440`, `DEFAULT ≤ 40960` bytes; agents: `XL ≤ 57344`, `LARGE ≤ 49152`, `DEFAULT ≤ 24576` bytes. A cap is **never raised** when a file approaches it: crossing it means *extract*, not bump. Independent of the ratchet above — unaffected by #2724. | `XL/LARGE/DEFAULT_CAP` in each guard file | +| **Headroom census + reserved margin** (visibility, [#4261](https://github.com/open-gsd/gsd-core/issues/4261)) | Every run prints each capped file's remaining bytes and percentage used — green runs included — sorted least-headroom-first, and appends a table of the files past a **95% reserved margin** to the GitHub job summary. The margin **reports, it does not fail**: a file at 96% is not broken, it is a file whose next contributor should extract before adding. Nothing here raises or relaxes a cap. | `buildHeadroomRows` / `marginFor` in `scripts/workflow-size.cjs` | + +Why the census exists: each PR's CI measures only its own base plus its own +diff, so two PRs that are individually under a cap can be jointly over it, and +no run either of them produces can show that. The census does not solve that +directly — measuring on the merge result would, and was deliberately left out +of #4261's approved scope — but it makes the density that causes it legible +before the collision, which a passing run previously did not. It also replaces +the hand-written per-tier high-water comments in both guard files, which had +gone stale by several kilobytes and were themselves the reason the shrinking +margin went unnoticed. `discuss-phase.md` additionally has a thin-dispatcher target of `< 32000` bytes (the discuss-phase progressive-disclosure split, #717). A net-new agent is diff --git a/scripts/workflow-size.cjs b/scripts/workflow-size.cjs index 174fa0b20..f6d198f28 100644 --- a/scripts/workflow-size.cjs +++ b/scripts/workflow-size.cjs @@ -89,10 +89,149 @@ function measureWorkflows(dir = WORKFLOWS_DIR) { return measureMdFiles(dir); } +/** + * #4261: the reserved margin, as a fraction of a tier's hard cap. + * + * The hard caps are red lines and stay exactly where they are. This is the + * second, softer level the caps never had: `execute-phase.md` already carries + * a hand-rolled version of this shape (a hard `< 93600` plus a `<= 93400` + * margin whose own message says it exists "so minor future edits don't + * re-trip the gate"), and it is the only capped file that does. Everywhere + * else a file is either fine or already over, with nothing in between — so + * the first signal a contributor gets is a failure, and by then the cheap + * moment to extract has passed. + * + * 95% is deliberately not derived from anything. It is the round number the + * issue proposed, and the census below is what makes it reviewable: if it + * turns out to name too many or too few files, that is visible in one table + * rather than argued from first principles. + */ +const MARGIN_RATIO = 0.95; + +/** + * The reserved-margin threshold for a hard cap, in bytes. + * + * Floor, not round: the margin must never land ON or above the cap it is + * meant to sit under, however small the cap. + * + * @param {number} cap - Tier hard cap in bytes. + * @returns {number} Margin threshold in bytes. + */ +function marginFor(cap) { + return Math.floor(cap * MARGIN_RATIO); +} + +/** + * Build the headroom census for a set of measured files. + * + * Sorted by pressure (least headroom first) because that is the reading + * order that matters: the top row is the file that will break next. + * + * @param {Object} sizes - Map of filename -> LF byte size. + * @param {function(string): {tier: string, cap: number}} capFor - Tier lookup, keyed by stem. + * @returns {Array<{name: string, tier: string, bytes: number, cap: number, margin: number, headroom: number, usedPct: number, overMargin: boolean}>} + */ +function buildHeadroomRows(sizes, capFor) { + return Object.entries(sizes) + .map(([file, bytes]) => { + const name = file.replace(/\.md$/, ''); + const { tier, cap } = capFor(name); + const margin = marginFor(cap); + return { + name, + tier, + bytes, + cap, + margin, + headroom: cap - bytes, + usedPct: (bytes / cap) * 100, + overMargin: bytes > margin, + }; + }) + .sort((a, b) => a.headroom - b.headroom || a.name.localeCompare(b.name)); +} + +/** + * Render the census as a fixed-width text table for the test diagnostics. + * + * @param {ReturnType} rows + * @param {{limit?: number}} [opts] - How many rows to render (default: all). + * @returns {string[]} Lines, ready to hand to `t.diagnostic` one at a time. + */ +function formatHeadroomTable(rows, { limit = Infinity } = {}) { + const shown = rows.slice(0, limit); + const width = Math.max(4, ...shown.map((r) => r.name.length)); + const head = `${'file'.padEnd(width)} ${'tier'.padEnd(7)} ${'bytes'.padStart(7)} ${'cap'.padStart(7)} ${'margin'.padStart(7)} ${'headroom'.padStart(8)} ${'used'.padStart(6)}`; + const body = shown.map( + (r) => + `${r.name.padEnd(width)} ${r.tier.padEnd(7)} ${String(r.bytes).padStart(7)} ${String(r.cap).padStart(7)} ${String(r.margin).padStart(7)} ${String(r.headroom).padStart(8)} ${r.usedPct.toFixed(1).padStart(5)}%`, + ); + return [head, ...body]; +} + +/** + * Render the census as a GitHub job-summary markdown table. + * + * Only the rows over the reserved margin: a job summary that lists all 124 + * capped files is a log dump nobody reads, and every row below the margin is + * by definition not the problem. The count of the rest is still reported so + * an empty table cannot be mistaken for an unmeasured one. + * + * @param {string} title - Section heading (e.g. `'Agent size headroom'`). + * @param {ReturnType} rows + * @returns {string} Markdown block. + */ +function buildHeadroomSummaryMarkdown(title, rows) { + const pressured = rows.filter((r) => r.overMargin); + const lines = [`### ${title}`, '']; + if (pressured.length === 0) { + lines.push(`All ${rows.length} files are under the ${Math.round(MARGIN_RATIO * 100)}% reserved margin.`); + return `${lines.join('\n')}\n`; + } + lines.push( + `**${pressured.length} of ${rows.length}** files are over the ${Math.round(MARGIN_RATIO * 100)}% reserved margin.`, + '', + '| file | tier | bytes | cap | headroom | used |', + '| --- | --- | ---: | ---: | ---: | ---: |', + ); + for (const r of pressured) { + lines.push(`| \`${r.name}\` | ${r.tier} | ${r.bytes} | ${r.cap} | ${r.headroom} | ${r.usedPct.toFixed(1)}% |`); + } + return `${lines.join('\n')}\n`; +} + +/** + * Append a census block to `$GITHUB_STEP_SUMMARY` when running in CI. + * + * No-op off CI, and never throws: this is reporting, and a test suite must + * not go red because a summary file was not writable. + * + * @param {string} title - Section heading. + * @param {ReturnType} rows + * @param {NodeJS.ProcessEnv} [env] + * @returns {boolean} Whether anything was written. + */ +function appendHeadroomStepSummary(title, rows, env = process.env) { + if (!env.GITHUB_STEP_SUMMARY) return false; + try { + fs.appendFileSync(env.GITHUB_STEP_SUMMARY, `${buildHeadroomSummaryMarkdown(title, rows)}\n`); + return true; + } catch (err) { + process.stderr.write(`workflow-size: could not write GITHUB_STEP_SUMMARY: ${err.message}\n`); + return false; + } +} + module.exports = { WORKFLOWS_DIR, + MARGIN_RATIO, lfByteCount, listWorkflowStems, measureMdFiles, measureWorkflows, + marginFor, + buildHeadroomRows, + formatHeadroomTable, + buildHeadroomSummaryMarkdown, + appendHeadroomStepSummary, }; diff --git a/tests/agent-size-budget.test.cjs b/tests/agent-size-budget.test.cjs index 18f200306..7af7dd21a 100644 --- a/tests/agent-size-budget.test.cjs +++ b/tests/agent-size-budget.test.cjs @@ -42,7 +42,16 @@ const assert = require('node:assert/strict'); const fs = require('fs'); const os = require('node:os'); const path = require('path'); -const { lfByteCount } = require('../scripts/workflow-size.cjs'); +const { + lfByteCount, + measureMdFiles, + MARGIN_RATIO, + marginFor, + buildHeadroomRows, + formatHeadroomTable, + buildHeadroomSummaryMarkdown, + appendHeadroomStepSummary, +} = require('../scripts/workflow-size.cjs'); const { cleanup } = require('./helpers.cjs'); const AGENTS_DIR = path.join(__dirname, '..', 'agents'); @@ -51,9 +60,16 @@ const isGsdAgent = (f) => f.startsWith('gsd-'); // Tier HARD CAPS (#1074, bytes) — absolute red lines, not high-water-hugging // ceilings. Day-to-day creep is caught per-agent by the baseline guard below; // these sit above each tier's current high-water with real headroom: -// XL 56 KiB — high-water gsd-debugger 51,043 → ~6.3 KB headroom -// LARGE 48 KiB — high-water gsd-executor 42,342 → ~6.8 KB headroom -// DEFAULT 24 KiB — high-water gsd-ui-researcher 19,095 → ~5.5 KB headroom +// XL 56 KiB +// LARGE 48 KiB +// DEFAULT 24 KiB +// +// #4261: the per-tier high-water marks that used to be written out here went +// stale — the LARGE line claimed "gsd-executor 42,342 → ~6.8 KB headroom" +// while the real high-water had reached 99.6% of the cap, so the comment +// documenting the margin was itself the reason nobody noticed the margin was +// gone. Hand-maintained measurements of a moving tree do not survive; the +// headroom census below emits the live numbers on every run instead. const XL_CAP = 57344; // 56 KiB const LARGE_CAP = 49152; // 48 KiB const DEFAULT_CAP = 24576; // 24 KiB @@ -140,6 +156,135 @@ describe('SIZE: agent hard-cap boundary fixtures (#1074 — negative proof)', () // must-have 6). The tier hard caps above are unaffected — they are independent of the // deleted baseline and remain the outer bound. +// ─── #4261: headroom visibility + reserved margin ────────────────────────── +// +// The hard caps above are red lines and this changes none of them. What was +// missing is everything BELOW the red line: a passing run said nothing, so a +// contributor sitting at 99.6% of a cap and one at 60% got identical +// feedback — green — and the density that produces merge-time collisions was +// invisible to the people creating it. +// +// Two levels, matching the shape `execute-phase.md` already has by hand (a +// hard ceiling plus a lower margin "so minor future edits don't re-trip the +// gate"), which until now was the only capped file with one: +// +// 1. the census, printed every run, green or not +// 2. the reserved margin, which REPORTS rather than fails +// +// The margin deliberately does not fail. A cap breach is a red line; a file +// at 96% is not broken, it is a file whose next contributor should know to +// extract before adding. Failing there would turn a warning into a second +// red line and force exactly the +N bumps this policy forbids. +const AGENT_HEADROOM_ROWS = buildHeadroomRows( + measureMdFiles(AGENTS_DIR, isGsdAgent), + capFor, +); + +describe('SIZE: agent headroom census (issue #4261)', () => { + test('reports every agent\'s remaining bytes, and never fails for it', (t) => { + for (const line of formatHeadroomTable(AGENT_HEADROOM_ROWS)) t.diagnostic(line); + const pressured = AGENT_HEADROOM_ROWS.filter((r) => r.overMargin); + t.diagnostic( + `agents: ${AGENT_HEADROOM_ROWS.length} | over the ${Math.round(MARGIN_RATIO * 100)}% margin: ${pressured.length}`, + ); + appendHeadroomStepSummary('Agent size headroom', AGENT_HEADROOM_ROWS); + + // The census is reporting, not a gate — the only thing asserted is that it + // measured the corpus at all. A census that silently went empty (a moved + // directory, a broken predicate) would otherwise read as good news. + assert.equal(AGENT_HEADROOM_ROWS.length, ALL_AGENTS.length); + }); + + test('names the agents inside the reserved margin', (t) => { + for (const r of AGENT_HEADROOM_ROWS.filter((row) => row.overMargin)) { + t.diagnostic( + `RESERVED MARGIN: ${r.name}.md is ${r.bytes} bytes — ${r.headroom} under the ${r.tier} cap ` + + `(${r.usedPct.toFixed(1)}%), past the ${r.margin}-byte margin. The cap is not moving: ` + + `extract shared boilerplate to gsd-core/references/ and load it lazily before adding more.`, + ); + } + // Intentionally no assertion on the COUNT. Pinning "3 agents are over the + // margin" would make this a baseline that every extraction has to update, + // which is the maintenance burden #2724 removed when it deleted the + // per-file size snapshot. The hard caps stay the only failing gate. + }); +}); + +describe('SIZE: reserved-margin boundary fixtures (#4261 — negative proof)', () => { + // The margin loop above reports whatever the real corpus happens to be, so + // its comparison branch is not exercised by construction — the same gap the + // hard-cap fixtures above exist to close. Pin marginFor and the > operator + // at the boundary so a future ratio or operator edit cannot quietly widen + // the margin to nothing (RULESET.TESTS.boundary-coverage.fixtures). + test('marginFor sits strictly below its cap and fires at the boundary', () => { + for (const cap of [DEFAULT_CAP, LARGE_CAP, XL_CAP]) { + const margin = marginFor(cap); + assert.ok(margin < cap, `margin ${margin} must sit below cap ${cap}`); + assert.equal(margin > cap * MARGIN_RATIO - 1, true, `margin ${margin} must track the ratio`); + const rows = buildHeadroomRows({ + 'below.md': margin - 1, + 'exact.md': margin, + 'above.md': margin + 1, + }, () => ({ tier: 'FIXTURE', cap })); + const byName = new Map(rows.map((row) => [row.name, row])); + assert.equal(byName.get('below').overMargin, false, 'margin - 1 is NOT over it'); + assert.equal(byName.get('exact').overMargin, false, 'exactly at the margin is NOT over it'); + assert.equal(byName.get('above').overMargin, true, 'margin + 1 IS over it'); + } + }); + + test('marginFor floors, so a tiny cap can never produce a margin at the cap', () => { + // Rounding here would put the margin ON the cap for small caps, making the + // warning fire only when the hard gate already had. + assert.equal(marginFor(1), 0); + assert.equal(marginFor(20), 19); + assert.ok(marginFor(20) < 20); + }); +}); + +describe('SIZE: headroom job summary (#4261)', () => { + // The census is only useful if it reaches a human. The diagnostics above go + // to the test log; this is the copy that lands on the PR's checks page, + // which is where a reviewer actually looks. + const row = (over) => ({ + name: 'gsd-example', tier: 'LARGE', bytes: over ? 49000 : 10000, + cap: 49152, margin: 46694, headroom: over ? 152 : 39152, + usedPct: over ? 99.7 : 20.3, overMargin: over, + }); + + test('a clean corpus still reports how many files it measured', () => { + const md = buildHeadroomSummaryMarkdown('Agent size headroom', [row(false), row(false)]); + // "no table" must be distinguishable from "nothing ran". + assert.match(md, /All 2 files are under the 95% reserved margin\./); + assert.doesNotMatch(md, /\| file \|/); + }); + + test('a pressured corpus renders one table row per file over the margin', () => { + const md = buildHeadroomSummaryMarkdown('Agent size headroom', [row(true), row(false)]); + assert.match(md, /\*\*1 of 2\*\* files are over the 95% reserved margin\./); + assert.match(md, /\| `gsd-example` \| LARGE \| 49000 \| 49152 \| 152 \| 99\.7% \|/); + }); + + test('writes to GITHUB_STEP_SUMMARY when set, and is a no-op when it is not', () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-size-summary-')); + try { + const file = path.join(tmp, 'summary.md'); + assert.equal(appendHeadroomStepSummary('T', [row(true)], { GITHUB_STEP_SUMMARY: file }), true); + assert.match(fs.readFileSync(file, 'utf8'), /### T/); + assert.equal(appendHeadroomStepSummary('T', [row(true)], {}), false); + } finally { + cleanup(tmp); + } + }); + + test('an unwritable summary path reports but does not fail the run', () => { + // Reporting must never be able to red a suite that is otherwise green — + // the whole point of this block is that it is additive. + const unwritable = path.join(os.tmpdir(), 'agent-size-nope', 'nested', 'summary.md'); + assert.equal(appendHeadroomStepSummary('T', [row(true)], { GITHUB_STEP_SUMMARY: unwritable }), false); + }); +}); + describe('SIZE: every agent is classified', () => { test('every agent falls in exactly one tier', () => { for (const agent of ALL_AGENTS) { diff --git a/tests/workflow-size-budget.test.cjs b/tests/workflow-size-budget.test.cjs index 2f5465acf..8d8db7ba8 100644 --- a/tests/workflow-size-budget.test.cjs +++ b/tests/workflow-size-budget.test.cjs @@ -79,7 +79,17 @@ const assert = require('node:assert/strict'); const fs = require('fs'); const os = require('node:os'); const path = require('path'); -const { lfByteCount: byteCount, listWorkflowStems, measureWorkflows } = require('../scripts/workflow-size.cjs'); +const fc = require('fast-check'); +const { + lfByteCount: byteCount, + listWorkflowStems, + measureWorkflows, + MARGIN_RATIO, + marginFor, + buildHeadroomRows, + formatHeadroomTable, + appendHeadroomStepSummary, +} = require('../scripts/workflow-size.cjs'); const { cleanup } = require('./helpers.cjs'); const WORKFLOWS_DIR = path.join(__dirname, '..', 'gsd-core', 'workflows'); @@ -89,9 +99,17 @@ const WORKFLOWS_DIR = path.join(__dirname, '..', 'gsd-core', 'workflows'); // only as the outer bound where the correct response is lazy extraction, never // a raise. Each sits above its tier's current high-water mark with real // headroom (vs the old GRACE=3000 hug): -// XL 96 KiB — high-water execute-phase.md 93,400 → ~4.8 KB headroom -// LARGE 60 KiB — high-water docs-update.md 55,468 → ~5.8 KB headroom -// DEFAULT 40 KiB — high-water settings.md 40,352 → ~608 B headroom +// XL 96 KiB +// LARGE 60 KiB +// DEFAULT 40 KiB +// +// #4261: the per-tier high-water marks that used to be written out here are +// gone rather than refreshed. They were measured once and then quietly +// diverged from the tree — the XL line named execute-phase.md as the +// high-water when plan-phase.md had passed it — so the comment meant to +// document the remaining headroom became a reason to believe there was more +// of it than there was. The headroom census below emits the live numbers on +// every run instead of asking a comment to stay true. // (DEFAULT is deliberately the tightest: a single-purpose workflow approaching // 40 KiB is the strongest extraction signal of the three. The previous DEFAULT // high-water, verify-phase.md at 40,931 (29 bytes of headroom), was deleted as @@ -150,6 +168,87 @@ function capFor(workflow) { // baseline generator so the guard and the snapshot can never measure // differently. See the #683 regression test at the bottom of this file. +// ─── #4261: headroom visibility + reserved margin ────────────────────────── +// +// See the twin block in tests/agent-size-budget.test.cjs for the rationale. +// The short version: a green run used to say nothing, so the difference +// between a file at 60% of its cap and one at 99.9% was invisible until the +// day someone crossed the line — and because each PR's CI measures only its +// own base plus its own diff, two individually-green PRs can be jointly over +// with no run either of them produces able to show it. +const WORKFLOW_HEADROOM_ROWS = buildHeadroomRows(SIZES, capFor); + +describe('SIZE: workflow headroom census (issue #4261)', () => { + test('reports every workflow\'s remaining bytes, and never fails for it', (t) => { + for (const line of formatHeadroomTable(WORKFLOW_HEADROOM_ROWS)) t.diagnostic(line); + const pressured = WORKFLOW_HEADROOM_ROWS.filter((r) => r.overMargin); + t.diagnostic( + `workflows: ${WORKFLOW_HEADROOM_ROWS.length} | over the ${Math.round(MARGIN_RATIO * 100)}% margin: ${pressured.length}`, + ); + appendHeadroomStepSummary('Workflow size headroom', WORKFLOW_HEADROOM_ROWS); + + // Reporting, not a gate — assert only that the corpus was measured, so an + // empty census cannot read as good news. + assert.equal(WORKFLOW_HEADROOM_ROWS.length, ALL_WORKFLOWS.length); + }); + + test('names the workflows inside the reserved margin', (t) => { + for (const r of WORKFLOW_HEADROOM_ROWS.filter((row) => row.overMargin)) { + t.diagnostic( + `RESERVED MARGIN: ${r.name}.md is ${r.bytes} bytes — ${r.headroom} under the ${r.tier} cap ` + + `(${r.usedPct.toFixed(1)}%), past the ${r.margin}-byte margin. The cap is not moving: ` + + `extract per-mode bodies to workflows/${r.name}/modes/, templates to ` + + `workflows/${r.name}/templates/, or shared references to gsd-core/references/ — lazily.`, + ); + } + // No assertion on the count, deliberately: pinning it would recreate the + // per-file size baseline #2724 deleted for conflicting on 7 of 7 PRs. + }); + + test('the reserved margin sits strictly below every tier cap', () => { + // Negative proof for the margin arithmetic, mirroring the hard-cap + // boundary fixtures: a ratio or operator edit that widened the margin to + // the cap would silently disable the warning, and no real-corpus test + // would notice. + for (const cap of [DEFAULT_CAP, LARGE_CAP, XL_CAP]) { + const margin = marginFor(cap); + assert.ok(margin < cap, `margin ${margin} must sit below cap ${cap}`); + const rows = buildHeadroomRows({ + 'below.md': margin - 1, + 'exact.md': margin, + 'above.md': margin + 1, + }, () => ({ tier: 'FIXTURE', cap })); + const byName = new Map(rows.map((row) => [row.name, row])); + assert.equal(byName.get('below').overMargin, false, 'margin - 1 is NOT over it'); + assert.equal(byName.get('exact').overMargin, false, 'exactly at the margin is NOT over it'); + assert.equal(byName.get('above').overMargin, true, 'margin + 1 IS over it'); + } + }); + + test('reserved-margin classification holds for every positive cap', () => { + fc.assert(fc.property( + fc.integer({ min: 2, max: 10_000_000 }), + (cap) => { + const margin = marginFor(cap); + const rows = buildHeadroomRows({ + 'below.md': margin - 1, + 'exact.md': margin, + 'above.md': margin + 1, + }, () => ({ tier: 'FIXTURE', cap })); + const byName = new Map(rows.map((row) => [row.name, row])); + + assert.ok(margin >= 0 && margin < cap); + assert.equal(byName.get('below').overMargin, false); + assert.equal(byName.get('exact').overMargin, false); + assert.equal(byName.get('above').overMargin, true); + assert.equal(byName.get('below').headroom, cap - (margin - 1)); + assert.equal(byName.get('exact').headroom, cap - margin); + assert.equal(byName.get('above').headroom, cap - (margin + 1)); + }, + )); + }); +}); + describe('SIZE: workflow tier hard caps (issue #1074)', () => { // Absolute outer bound per tier. Unlike the old tighten-only ceiling, a cap // is NOT raised when a file approaches it — crossing it means extract, not