diff --git a/.github/scripts/check.mjs b/.github/scripts/check.mjs index d83c2e3..6b9d1f9 100644 --- a/.github/scripts/check.mjs +++ b/.github/scripts/check.mjs @@ -7,7 +7,7 @@ // Run by .github/workflows/qep-checks.yml. Exits non-zero on any failure. import { execSync } from 'node:child_process'; import { readFileSync } from 'node:fs'; -import { FRONTMATTER, parseQep, qepFiles, readIndex, versionCell } from './qeps.mjs'; +import { FRONTMATTER, parseQep, qepFiles, readIndex, renderIndex } from './qeps.mjs'; const base = process.env.BASE_REF || 'main'; const errors = []; @@ -67,7 +67,6 @@ const idx = readIndex(); if (idx.cols.type === -1) errors.push(`${'README.md'}: index table is missing a Type column`); if (idx.cols.status === -1) errors.push(`${'README.md'}: index table is missing a Status column`); if (idx.cols.version === -1) errors.push(`${'README.md'}: index table is missing a Version column`); -const rows = new Map(idx.rows.map((r) => [r.qep, r])); for (const path of qepFiles()) { const q = parseQep(path); if (q.qep === undefined) continue; @@ -78,29 +77,42 @@ for (const path of qepFiles()) { if (q.status !== undefined && !STATUSES.has(q.status)) { errors.push(`${path}: unknown status "${q.status}" (expected one of ${[...STATUSES].join(', ')})`); } +} - const row = rows.get(q.qep); - if (!row) { - errors.push(`README index has no row for QEP-${q.qep} (${path})`); - continue; +// Every QEP file declares a number, and no two declare the same one. The index is +// generated from these, so a missing number drops a QEP out of the table silently +// and a duplicate emits two rows under one heading — neither shows up anywhere +// else. QEP-1 expects colliding proposals to be "adjusted at merge"; this is what +// tells the author there is a collision to adjust. +{ + const seen = new Map(); + for (const path of qepFiles()) { + const q = parseQep(path); + if (q.qep === undefined) { + errors.push(`${path}: no "qep:" number in the frontmatter`); + continue; + } + if (seen.has(q.qep)) { + errors.push(`${path}: QEP number ${q.qep} is already used by ${seen.get(q.qep)}`); + continue; + } + seen.set(q.qep, path); } - const expect = (label, colIdx, want) => { - if (colIdx === -1) return; - const got = row.cells[colIdx]; - if (got !== want) errors.push(`QEP-${q.qep}: README ${label} "${got}" != frontmatter "${want}"`); - }; - expect('Type', idx.cols.type, q.type); - expect('Status', idx.cols.status, q.status); +} - // Version parity, tolerating a hand-typed ASCII "-" or empty cell for a v0 QEP: - // stamp.mjs normalises it to the en dash post-merge, so don't block the PR on it. - if (idx.cols.version !== -1) { - const got = row.cells[idx.cols.version]; - const want = versionCell(q.version); // en dash for v0 - const v0ok = q.version === undefined && (got === '-' || got === ''); - if (got !== want && !v0ok) { - errors.push(`QEP-${q.qep}: README Version "${got}" != frontmatter "${want}"`); - } +// The index is GENERATED post-merge from frontmatter (stamp.mjs), so a PR need +// not carry its own row and row content is never a PR failure: that is what +// stops two QEP PRs colliding on one line of one table. A stale index is worth +// saying out loud, though, so the author is not surprised by the bot commit. +{ + const want = renderIndex(idx, qepFiles().map((p) => parseQep(p))); + if (want.join('\n') !== idx.lines.join('\n')) { + // `::warning::` so this lands as a PR annotation: the parity check is a warning + // now, and a line in the raw log is a signal nobody reads on a green check. + console.warn( + '::warning file=README.md::the index differs from what frontmatter implies; ' + + 'stamp.mjs will regenerate it after merge (this is not a failure)', + ); } } @@ -140,6 +152,57 @@ for (const path of qepFiles()) { } } +// 5. Ordered-list numbering ascends in source. +// Markdown renumbers an ordered list on render, so a repeated or out-of-order +// marker looks correct on the page while every external "clause N" citation +// silently shifts. QEP-6's Adoption section shipped as 1., 2., 2., 3., 4. and +// survived a twelve-amendment review, a field report and four PR comments — +// which is why this is a check and not a convention. +{ + const FENCE = /^\s*(?:```|~~~)/; + for (const path of qepFiles()) { + const lines = readFileSync(path, 'utf8').split('\n'); + const runs = new Map(); // indent -> { last, line } + let fenced = false; + for (let i = 0; i < lines.length; i++) { + const line = lines[i]; + if (FENCE.test(line)) { + fenced = !fenced; + // A fence ends only the lists it is not nested inside: an INDENTED fence is + // a continuation of its list item, so the run around it must survive, or a + // marker repeated across it goes unreported. Same indent rule as below. + const fi = line.match(/^(\s*)/)[1].length; + for (const k of [...runs.keys()]) if (k >= fi) runs.delete(k); + continue; + } + if (fenced) continue; + + const item = line.match(/^(\s*)(\d+)\.\s/); + if (item) { + const indent = item[1].length; + const n = Number(item[2]); + for (const k of [...runs.keys()]) if (k > indent) runs.delete(k); // deeper lists end + const prev = runs.get(indent); + if (prev !== undefined && n <= prev.last) { + errors.push( + `${path}:${i + 1}: ordered-list marker "${n}." does not ascend ` + + `(previous was "${prev.last}." at line ${prev.line}); Markdown renumbers on ` + + `render, so a repeat shifts every external "clause N" citation`, + ); + } + runs.set(indent, { last: n, line: i + 1 }); + continue; + } + + if (line.trim() === '') continue; // a blank line does not end a list + // Any other non-blank line ends runs at or deeper than its own indent; + // a more-indented line is an item's continuation and leaves the run alone. + const indent = line.match(/^(\s*)/)[1].length; + for (const k of [...runs.keys()]) if (k >= indent) runs.delete(k); + } + } +} + if (errors.length) { console.error('QEP checks failed:\n' + errors.map((e) => ` - ${e}`).join('\n')); process.exit(1); diff --git a/.github/scripts/qeps.mjs b/.github/scripts/qeps.mjs index 7c4dfa0..06a57c1 100644 --- a/.github/scripts/qeps.mjs +++ b/.github/scripts/qeps.mjs @@ -103,17 +103,58 @@ export function readIndex() { if (start === -1) throw new Error(`${README}: no index table header (| QEP | ...) found`); const header = splitRow(lines[start]); const col = (name) => header.findIndex((c) => c.toLowerCase() === name); - const cols = { type: col('type'), status: col('status'), version: col('version') }; + const cols = { + qep: col('qep'), + title: col('title'), + type: col('type'), + status: col('status'), + version: col('version'), + }; + // `end` is the first line after the table body, so the body is exactly + // lines[start + 2 .. end). renderIndex() replaces that span wholesale, which + // is why the bound is tracked rather than just the rows that parsed. const rows = []; + let end = start + 2; for (let i = start + 2; i < lines.length; i++) { const line = lines[i]; if (!line.trimStart().startsWith('|')) break; // table ended + end = i + 1; const m = line.match(/qep-(\d+)-/); if (!m) continue; rows.push({ index: i, cells: splitRow(line), qep: Number(m[1]) }); } - return { lines, cols, rows }; + return { lines, cols, rows, start, end, header }; +} + +// The index row a QEP's frontmatter implies. Column ORDER comes from the table +// header, so a reordered or extended table needs no change here; a column this +// function does not know about is left empty rather than guessed at. `width` is +// the header's own column count, so an unknown column at the END of the table is +// emitted empty like any other rather than dropped off the row. +export function buildRow(q, cols, width = Math.max(...Object.values(cols)) + 1) { + const cells = new Array(Math.max(width, Math.max(...Object.values(cols)) + 1)).fill(''); + const put = (i, v) => { + if (i !== -1) cells[i] = v; + }; + put(cols.qep, `[QEP-${q.qep}](${q.path})`); + put(cols.title, q.title ?? ''); + put(cols.type, q.type ?? ''); + put(cols.status, q.status ?? ''); + put(cols.version, versionCell(q.version)); + return cells; +} + +// The whole index body, rebuilt from frontmatter and ordered by QEP number. +// Returns the new `lines` array; the caller decides whether to write it. +// This is the generated-index rule: the table is derived, never hand-edited, +// so a PR need not carry its own row and two PRs cannot collide on one line. +export function renderIndex(idx, qeps) { + const body = [...qeps] + .filter((q) => q.qep !== undefined) + .sort((a, b) => a.qep - b.qep) + .map((q) => formatRow(buildRow(q, idx.cols, idx.header.length))); + return [...idx.lines.slice(0, idx.start + 2), ...body, ...idx.lines.slice(idx.end)]; } // Rebuild a single-spaced Markdown row from its trimmed cells. diff --git a/.github/scripts/stamp.mjs b/.github/scripts/stamp.mjs index f43ac41..c002c6a 100644 --- a/.github/scripts/stamp.mjs +++ b/.github/scripts/stamp.mjs @@ -7,11 +7,10 @@ import { readFileSync, writeFileSync } from 'node:fs'; import { FRONTMATTER, README, - formatRow, parseQep, qepFiles, readIndex, - versionCell, + renderIndex, } from './qeps.mjs'; const sha = execSync('git rev-parse --short HEAD').toString().trim(); @@ -65,25 +64,19 @@ for (const path of qepFiles()) { } } -// 2. Sync the README Type/Version columns from frontmatter. -const meta = new Map( - qepFiles() - .map((p) => parseQep(p)) - .filter((q) => q.qep !== undefined) - .map((q) => [q.qep, q]), -); +// 2. Regenerate the README index from frontmatter. +// The index is DERIVED, not hand-maintained: every column comes from a QEP's +// own frontmatter and the rows are ordered by number. This is what lets a PR +// omit its own row entirely, so two QEP PRs can no longer collide on one line +// of one table — the add/add conflict class that made #18 unmergeable against +// QEP-3's row. check.mjs warns when a PR's index is stale; it never fails on it. const idx = readIndex(); -const out = [...idx.lines]; -for (const row of idx.rows) { - const q = meta.get(row.qep); - if (!q) continue; - const cells = [...row.cells]; - if (idx.cols.type !== -1 && q.type !== undefined) cells[idx.cols.type] = q.type; - if (idx.cols.version !== -1) cells[idx.cols.version] = versionCell(q.version); - if (cells.join('|') !== row.cells.join('|')) { - out[row.index] = formatRow(cells); - console.log(`README: synced QEP-${row.qep} row`); - } +const out = renderIndex( + idx, + qepFiles().map((p) => parseQep(p)), +); +if (out.join('\n') !== idx.lines.join('\n')) { + console.log('README: index regenerated from frontmatter'); } const readme = out.join('\n'); if (readme !== readFileSync(README, 'utf8')) { diff --git a/AGENTS.md b/AGENTS.md index a0d730c..8056bf0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -15,24 +15,39 @@ not `QEP-0002`); only the filename pads it. The site is built and published to GitHub Pages by [`.github/workflows/deploy.yml`](.github/workflows/deploy.yml) on every push to `main`. -## The README index is a complete registry +## The README index is generated — do not hand-edit it The [README](README.md) index lists **every** QEP with its `Type`, current `Status`, and -`Version` — not only accepted ones. A row is added when the PR opens (status `Draft`, -version `–`) and its status is updated in place as the QEP moves: Draft → Accepted / -Rejected / Withdrawn / Superseded. The `Type`, `Status`, and `Version` columns must match -the QEP's frontmatter — CI checks this parity on every PR (see *What CI does*). +`Version` — not only accepted ones. **Every cell is derived from the QEP's own +frontmatter, and the whole table is regenerated post-merge** by +[`stamp.mjs`](.github/scripts/stamp.mjs), ordered by QEP number. So: + +- **A PR does not add its own row.** Set `status`, `type` and `version` in the + frontmatter; the row appears when the PR merges. +- **Editing the table by hand achieves nothing durable** — the next merge overwrites it + from frontmatter. CI warns when a branch's table is stale; it never fails on it. +- **Gaps are normal.** The index shows only merged QEPs, so while drafts are open the + numbers skip (a number is reserved when its draft PR opens and released if that PR + closes unmerged). A QEP that merges out of order slots into its numeric position + automatically. + +This is why the table stopped being a merge-conflict magnet: four open QEP PRs used to +contend for rows in one table, and #18 was unmergeable on that single line. A branch +that still carries a row will conflict textually — strip the row — but a *mis-resolved* +index conflict is now self-healing, because the post-merge regeneration restores the +table from frontmatter whatever the resolution did to it. ## Accepting a QEP When a QEP reaches a decision (see QEP-1 for the lazy-consensus rule), apply the outcome -in a **single PR**. The status is **duplicated in three places** — keep them in sync: +in a **single PR**. The status lives in **two places in the document** — keep them in +sync: -1. the YAML frontmatter `status:` field, -2. the **Status** row in the in-document header table, and -3. the QEP's row in the [README](README.md) index table. +1. the YAML frontmatter `status:` field, and +2. the **Status** row in the in-document header table. -This applies to every terminal outcome — **Accepted**, **Rejected**, **Withdrawn**, or +The [README](README.md) index row is *generated* from the frontmatter post-merge, so do +not edit it. This applies to every terminal outcome — **Accepted**, **Rejected**, **Withdrawn**, or **Superseded** — not just acceptance. Then **merge** the PR; do not close it. Accepted, Rejected, and Withdrawn QEPs are all @@ -41,12 +56,16 @@ merged so the record stays durable — only abandoned or spam drafts are closed. Before merging, confirm the QEP number is final and not colliding with another open PR, and that the filename is zero-padded to four digits. +A merged QEP may not carry `status: Draft` — Draft means *under discussion on an open +PR*, and only abandoned drafts are closed unmerged, so nothing should reach `main` in +that state. QEP-3 did, for two months. + ## Drafting a new QEP Copy [`qeps/template.md`](qeps/template.md) to `qeps/qep-XXXX-slug.md`, fill it in with -**Status: Draft** and a discussion link, add the QEP's row to the README index with -status `Draft` and version `–`, and open a PR. A new QEP is unversioned (implicitly v0): -omit the `version` field. See QEP-1 for the full process. +**Status: Draft** and a discussion link, and open a PR. **Do not add a README index +row** — it is generated from the frontmatter when the PR merges. A new QEP is unversioned +(implicitly v0): omit the `version` field. See QEP-1 for the full process. ## Amending an accepted QEP @@ -81,13 +100,29 @@ GitHub UI choose **Squash and merge**. ## What CI does (don't do these by hand) - **Post-merge** — [`stamp-version.yml`](.github/workflows/stamp-version.yml) stamps the - merged short hash into the `version-hash` field and syncs the README `Type`/`Version` - columns from each QEP's frontmatter. + merged short hash into the `version-hash` field and **regenerates the whole README + index** from each QEP's frontmatter, ordered by number. - **On every PR** — [`qep-checks.yml`](.github/workflows/qep-checks.yml) checks that `version` moves legally (a new QEP starts unversioned; a versioned QEP stays versioned; the number stays the same or increases by exactly one), that `type` and `status` are - known values, and that the README `Type`/`Status`/`Version` columns match each QEP's - frontmatter. - -You still set `version`, `type`, and the README row in the PR; CI stamps the hash and -enforces parity. The checks live in [`.github/scripts/`](.github/scripts/). + known values, that `related:` and the header table's **Related** row agree, and that + **ordered-list markers ascend in source** (see below). A stale README index is a + warning, not a failure. + +You still set `version`, `type` and `status` in the frontmatter; CI stamps the hash and +generates the index. The checks live in [`.github/scripts/`](.github/scripts/). + +## Cite a section by its name, not its number + +Refer to a QEP section by its heading — *QEP-6 § Constraints are dependencies* — rather +than by number. **Section numbers move.** Inserting §2 into QEP-6 mid-draft renumbered +§2–§7 to §3–§8 and forced a correction onto a ruling that had already cited them, and +external consumers cite these: the `qe` skills, the projects dashboard's tracker +contract, and several tracking issues. + +The same hazard applies inside a document. Markdown **renumbers an ordered list on +render**, so a source list reading `1., 2., 2., 3., 4.` displays as 1–5 while every +external "clause N" citation silently shifts by one. QEP-6 shipped exactly that and it +survived a twelve-amendment review, a field report and four PR comments — so it is now a +CI check rather than a convention. Keep ordered-list markers strictly ascending in +source; fenced code blocks are exempt. diff --git a/README.md b/README.md index 011229b..ba641a0 100644 --- a/README.md +++ b/README.md @@ -20,9 +20,9 @@ need a QEP. QEPs that set an ongoing rule are **maintained in place**: a substantive amendment bumps the QEP's `version` (shown above) under the same review process, rather than superseding -the whole document — see **QEP-1**. The `Type`/`Version` columns are kept in sync by CI, -and each QEP's `version-hash` is stamped into its frontmatter at merge; `Version` reads -`–` until a QEP is first amended. +the whole document — see **QEP-1**. The index table is **generated** from each QEP's +frontmatter after merge — do not hand-edit it; `version-hash` is stamped at the same time. +`Version` reads `–` until a QEP is first amended. ## Proposing a QEP