From 2668f25ba06af5d847568886ae46b028263842bf Mon Sep 17 00:00:00 2001 From: Matt McKay Date: Wed, 24 Jun 2026 15:19:21 +1000 Subject: [PATCH 1/6] QEP-1 v1: introduce in-place versioning of QEPs Amend QEP-1 so a QEP gains a `version` the first time it is substantively changed after acceptance, rather than being superseded wholesale for every small change. Living standards (the label policy, a style guide, this process itself) are now amended in place under the same lazy-consensus review. - QEP-1: add the `version` field (implicit v0, git-hash anchor stamped at merge), the substantive-vs-editorial granularity rule, amend-in-place vs supersede, squash-merge-only, the `QEP-N vM:` / `QEP-N:` commit-subject convention, the `standard`/`process`/`informational` content taxonomy for `type`, and git (surfaced on the site) as the change record. QEP-1's own first substantive amendment, so it becomes v1. - template.md: lowercase `type` enum; note that `version` appears on first amendment. - README: add `Type` and `Version` columns; QEP-1 -> process / v1. - AGENTS.md: author-side guidance (substantive vs editorial, version bump, commit subjects, squash-merge) and what CI does. - CI: post-merge `stamp-version.yml` stamps the merged hash into `version: N # ` and syncs the README columns; `qep-checks.yml` enforces version-increment and README<->frontmatter parity. Logic in `.github/scripts/`. Implements the design agreed in #4. Co-Authored-By: Claude Opus 4.8 --- .github/scripts/check.mjs | 69 +++++++++++ .github/scripts/qeps.mjs | 99 +++++++++++++++ .github/scripts/stamp.mjs | 70 +++++++++++ .github/workflows/qep-checks.yml | 29 +++++ .github/workflows/stamp-version.yml | 50 ++++++++ AGENTS.md | 54 +++++++- README.md | 12 +- qeps/qep-0001-purpose-and-process.md | 176 ++++++++++++++++++++++++--- qeps/template.md | 8 +- 9 files changed, 538 insertions(+), 29 deletions(-) create mode 100644 .github/scripts/check.mjs create mode 100644 .github/scripts/qeps.mjs create mode 100644 .github/scripts/stamp.mjs create mode 100644 .github/workflows/qep-checks.yml create mode 100644 .github/workflows/stamp-version.yml diff --git a/.github/scripts/check.mjs b/.github/scripts/check.mjs new file mode 100644 index 0000000..ae1150c --- /dev/null +++ b/.github/scripts/check.mjs @@ -0,0 +1,69 @@ +// Pull-request checks: +// 1. `version`, when present, only ever increments by one (and a first +// version is 1) relative to the base branch. +// 2. The README Type/Status/Version columns match each QEP's frontmatter. +// Run by .github/workflows/qep-checks.yml. Exits non-zero on any failure. +import { execSync } from 'node:child_process'; +import { parseQep, qepFiles, readIndex, versionCell } from './qeps.mjs'; + +const base = process.env.BASE_REF || 'main'; +const errors = []; + +// `version` of a QEP file on the base branch (undefined if absent / new / v0). +function baseVersion(path) { + let text; + try { + text = execSync(`git show "origin/${base}:${path}"`, { + stdio: ['pipe', 'pipe', 'ignore'], + }).toString(); + } catch { + return undefined; // file did not exist on the base branch + } + const m = text.match(/^version:[ \t]*(\d+)/m); + return m ? Number(m[1]) : undefined; +} + +// 1. version increments by at most one; a newly introduced version must be 1. +for (const path of qepFiles()) { + const { version } = parseQep(path); + if (version === undefined) continue; // v0 — always fine + const prev = baseVersion(path); + if (prev === undefined) { + if (version !== 1) { + errors.push(`${path}: introduces version ${version}; a first version must be 1`); + } + } else if (version !== prev && version !== prev + 1) { + errors.push( + `${path}: version ${prev} -> ${version} must stay equal (editorial) or increment by one (substantive)`, + ); + } +} + +// 2. README Type/Status/Version columns match frontmatter. +const idx = readIndex(); +if (idx.cols.type === -1) errors.push(`${'README.md'}: index table is missing a Type 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; + const row = rows.get(q.qep); + if (!row) { + errors.push(`README index has no row for QEP-${q.qep} (${path})`); + continue; + } + 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); + expect('Version', idx.cols.version, versionCell(q.version)); +} + +if (errors.length) { + console.error('QEP checks failed:\n' + errors.map((e) => ` - ${e}`).join('\n')); + process.exit(1); +} +console.log('QEP checks passed.'); diff --git a/.github/scripts/qeps.mjs b/.github/scripts/qeps.mjs new file mode 100644 index 0000000..bfb4a7d --- /dev/null +++ b/.github/scripts/qeps.mjs @@ -0,0 +1,99 @@ +// Shared helpers for QEP CI: parse QEP frontmatter and the README index table. +// Used by stamp.mjs (post-merge stamp + README sync) and check.mjs (PR checks). +import { readFileSync, readdirSync } from 'node:fs'; + +export const QEP_DIR = 'qeps'; +export const README = 'README.md'; + +// Every qeps/qep-XXXX-slug.md file, sorted by name (so by QEP number). +export function qepFiles() { + return readdirSync(QEP_DIR) + .filter((f) => /^qep-\d{4}-.*\.md$/.test(f)) + .sort() + .map((f) => `${QEP_DIR}/${f}`); +} + +const FRONTMATTER = /^---\r?\n([\s\S]*?)\r?\n---/; + +function field(block, name) { + const m = block.match(new RegExp(`^${name}:[ \\t]*(.*?)[ \\t]*$`, 'm')); + return m ? m[1] : undefined; +} + +function stripQuotes(s) { + return s === undefined ? undefined : s.replace(/^["']|["']$/g, ''); +} + +// Parse the fields we care about from a QEP file's YAML frontmatter. +// `version` is a number (undefined = implicitly v0); `hash` is the stamped short +// SHA from the `version: N # ` comment (undefined when not yet stamped). +export function parseQep(path) { + const text = readFileSync(path, 'utf8'); + const fm = text.match(FRONTMATTER); + if (!fm) throw new Error(`${path}: missing YAML frontmatter`); + const block = fm[1]; + + const raw = field(block, 'version'); + let version, hash; + if (raw !== undefined && raw !== '') { + const v = raw.match(/^(\d+)(?:[ \t]*#[ \t]*([0-9a-fA-F]+))?$/); + if (!v) throw new Error(`${path}: malformed "version: ${raw}"`); + version = Number(v[1]); + hash = v[2]; + } + + const qep = field(block, 'qep'); + return { + path, + qep: qep === undefined ? undefined : Number(qep), + title: stripQuotes(field(block, 'title')), + status: stripQuotes(field(block, 'status')), + type: stripQuotes(field(block, 'type')), + version, + hash, + }; +} + +// How `version` is displayed in the README Version column. +export function versionCell(version) { + return version === undefined ? '–' : `v${version}`; +} + +// --- README index table --------------------------------------------------- +// The Markdown table under the "## Index" heading: its first column links each +// QEP file. Column positions are read from the header row, so the column order +// can change without touching this code. + +function splitRow(line) { + // "| a | b |" -> ["a", "b"] + return line + .replace(/^\s*\|/, '') + .replace(/\|\s*$/, '') + .split('|') + .map((c) => c.trim()); +} + +export function readIndex() { + const text = readFileSync(README, 'utf8'); + const lines = text.split('\n'); + const start = lines.findIndex((l) => /^\|\s*QEP\s*\|/i.test(l)); + 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 rows = []; + for (let i = start + 2; i < lines.length; i++) { + const line = lines[i]; + if (!line.trimStart().startsWith('|')) break; // table ended + const m = line.match(/qep-(\d+)-/); + if (!m) continue; + rows.push({ index: i, cells: splitRow(line), qep: Number(m[1]) }); + } + return { lines, cols, rows }; +} + +// Rebuild a single-spaced Markdown row from its trimmed cells. +export function formatRow(cells) { + return `| ${cells.join(' | ')} |`; +} diff --git a/.github/scripts/stamp.mjs b/.github/scripts/stamp.mjs new file mode 100644 index 0000000..ec63d63 --- /dev/null +++ b/.github/scripts/stamp.mjs @@ -0,0 +1,70 @@ +// Post-merge: stamp the merged short hash into each changed QEP's +// `version: N # ` comment, and sync the README Type/Version columns from +// each QEP's frontmatter. Run by .github/workflows/stamp-version.yml on push to +// main. Idempotent: writes nothing when everything is already current. +import { execSync } from 'node:child_process'; +import { readFileSync, writeFileSync } from 'node:fs'; +import { + README, + formatRow, + parseQep, + qepFiles, + readIndex, + versionCell, +} from './qeps.mjs'; + +const sha = execSync('git rev-parse --short HEAD').toString().trim(); + +// Files touched by the tip commit (QEP PRs are squash-merged => one commit). +const changed = new Set( + execSync('git diff-tree --no-commit-id --name-only -r HEAD') + .toString() + .split('\n') + .map((s) => s.trim()) + .filter(Boolean), +); + +let dirty = false; + +// 1. Stamp the hash into any changed v1+ QEP whose comment isn't already this SHA. +for (const path of qepFiles()) { + if (!changed.has(path)) continue; + const { version, hash } = parseQep(path); + if (version === undefined) continue; // v0 — no version line to stamp + if (hash === sha) continue; // already current + const text = readFileSync(path, 'utf8'); + const stamped = text.replace(/^(version:[ \t]*\d+)[ \t]*(#.*)?$/m, `$1 # ${sha}`); + if (stamped !== text) { + writeFileSync(path, stamped); + dirty = true; + console.log(`stamped ${path} -> ${sha}`); + } +} + +// 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]), +); +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 readme = out.join('\n'); +if (readme !== readFileSync(README, 'utf8')) { + writeFileSync(README, readme); + dirty = true; +} + +console.log(dirty ? 'changes written' : 'nothing to stamp or sync'); diff --git a/.github/workflows/qep-checks.yml b/.github/workflows/qep-checks.yml new file mode 100644 index 0000000..451850f --- /dev/null +++ b/.github/workflows/qep-checks.yml @@ -0,0 +1,29 @@ +name: QEP checks +on: + pull_request: + paths: + - "qeps/**" + - "README.md" + - ".github/scripts/**" + - ".github/workflows/qep-checks.yml" + +# Read-only: this job only validates the PR. +permissions: + contents: read + +jobs: + checks: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + - uses: actions/setup-node@v4 + with: + node-version: "24" + - name: Fetch base branch + run: git fetch --no-tags --depth=1 origin "${{ github.base_ref }}" + - name: Run QEP checks + run: node .github/scripts/check.mjs + env: + BASE_REF: ${{ github.base_ref }} diff --git a/.github/workflows/stamp-version.yml b/.github/workflows/stamp-version.yml new file mode 100644 index 0000000..1173895 --- /dev/null +++ b/.github/workflows/stamp-version.yml @@ -0,0 +1,50 @@ +name: Stamp QEP version +on: + push: + branches: [main] + paths: + - "qeps/**" + +# Needs write access to push the stamp/sync commit back to main. +permissions: + contents: write + +# Serialise stamp runs so two QEP merges in quick succession don't race on the +# push to main (the later push would otherwise be rejected non-fast-forward). +concurrency: + group: stamp-main + cancel-in-progress: false + +jobs: + stamp: + # The bot's own commit carries [skip-stamp]; pushes made with GITHUB_TOKEN + # don't retrigger workflows anyway, so this guard is belt-and-suspenders. + # That same no-retrigger behaviour means this push does not redeploy the + # site — which is fine: everything visible (the type/version pills and the + # README columns) is already correct from the merge commit, and only the + # invisible `# ` comment waits for the next push to main to republish. + if: ${{ !contains(github.event.head_commit.message, '[skip-stamp]') }} + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + - uses: actions/setup-node@v4 + with: + node-version: "24" + - name: Stamp hash and sync README + run: node .github/scripts/stamp.mjs + - name: Commit and push if changed + run: | + if git diff --quiet; then + echo "Nothing to commit." + exit 0 + fi + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + git add -A + git commit -m "Stamp QEP version hash and sync README [skip-stamp]" + # Rebase onto anything that landed on main while we were stamping, + # then push, so a near-simultaneous merge can't drop this stamp. + git pull --rebase origin main + git push diff --git a/AGENTS.md b/AGENTS.md index bf270a7..66d249c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -17,9 +17,11 @@ on every push to `main`. ## The README index is a complete registry -The [README](README.md) index lists **every** QEP with its current status — not only -accepted ones. A row is added when the PR opens (status `Draft`) and its status is -updated in place as the QEP moves: Draft → Accepted / Rejected / Withdrawn / Superseded. +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*). ## Accepting a QEP @@ -43,4 +45,48 @@ and that the filename is zero-padded to four digits. 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 open a PR. See QEP-1 for the full process. +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. + +## Amending an accepted QEP + +An accepted QEP is a living document (QEP-1). A substantive evolution of the *same* +standard is a normal PR against the QEP under lazy-consensus that **bumps its +`version`**; a *different* decision that replaces it wholesale is a **new** QEP that +marks the old one `Superseded`. Don't supersede for routine maintenance. + +**Does `version` move?** — the author/reviewer call, not CI's: + +- **Substantive** (any change to normative content — a rule, a value, a table row, a + machine-readable appendix): increment `version` by one in **both** the frontmatter and + the header table. The first amendment introduces `version: 1` and adds a **Version** + row to the header table; the README `Version` column moves from `–` to `v{N}`. +- **Editorial** (no change to normative content — typo, wording, formatting, link): + leave `version` unchanged. + +Leave the `# ` comment **off** when you set or bump `version` — CI stamps it with +the merge hash; never hand-write it (a commit cannot contain its own hash). + +**Commit subjects** carry the same distinction: + +- Substantive → `QEP-N vM: ` (e.g. `QEP-2 v1: add release-blocker label`). +- Editorial → `QEP-N: `. + +## Squash-merge only + +Every QEP PR is **squash-merged** so one amendment is one commit and a QEP's history +reads as one line per change. This is a repository setting; when merging through the +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 `version: N # ` and syncs the README `Type`/`Version` + columns from each QEP's frontmatter. +- **On every PR** — [`qep-checks.yml`](.github/workflows/qep-checks.yml) checks that + `version`, if present, either stays the same (editorial) or increases by exactly one + (substantive), 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/). diff --git a/README.md b/README.md index 130d749..f514d6f 100644 --- a/README.md +++ b/README.md @@ -12,9 +12,15 @@ need a QEP. ## Index -| QEP | Title | Status | -|-----|-------|--------| -| [QEP-1](qeps/qep-0001-purpose-and-process.md) | QEP Purpose and Process | Accepted | +| QEP | Title | Type | Status | Version | +|-----|-------|------|--------|---------| +| [QEP-1](qeps/qep-0001-purpose-and-process.md) | QEP Purpose and Process | process | Accepted | v1 | + +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. ## Proposing a QEP diff --git a/qeps/qep-0001-purpose-and-process.md b/qeps/qep-0001-purpose-and-process.md index bb7104c..ba95dfd 100644 --- a/qeps/qep-0001-purpose-and-process.md +++ b/qeps/qep-0001-purpose-and-process.md @@ -3,7 +3,8 @@ qep: 1 title: QEP Purpose and Process author: "@mmcky" status: Accepted -type: Process +type: process +version: 1 created: 2026-06-16 discussion: https://github.com/QuantEcon/meta/issues/325 --- @@ -16,7 +17,8 @@ discussion: https://github.com/QuantEcon/meta/issues/325 | **Title** | QEP Purpose and Process | | **Author** | @mmcky | | **Status** | Accepted | -| **Type** | Process | +| **Type** | process | +| **Version** | 1 | | **Created** | 2026-06-16 | | **Discussion** | [QuantEcon/meta#325](https://github.com/QuantEcon/meta/issues/325) | @@ -24,11 +26,12 @@ discussion: https://github.com/QuantEcon/meta/issues/325 A **QuantEcon Enhancement Proposal (QEP)** is a short, durable document that records a decision affecting **more than one QuantEcon repository**, or that **changes how the -team works**. This QEP defines what a QEP is, when one is needed, where QEPs live, and -how a proposal moves from draft to decision. It is deliberately lightweight: the aim is -a ten-minute read, a clear deadline, and a clean close — not governance for its own -sake. As the first proposal, this document is also a worked example of the template it -describes. +team works**. This QEP defines what a QEP is, when one is needed, where QEPs live, how a +proposal moves from draft to decision, and how an accepted QEP is **maintained over +time**. It is deliberately lightweight: the aim is a ten-minute read, a clear deadline, +and a clean close — not governance for its own sake. As the first proposal, this +document is also a worked example of the template it describes and — now at v1 — of the +in-place versioning it introduces. ## Motivation @@ -45,6 +48,12 @@ A lightweight enhancement-proposal process gives us: deserves before anything rolls out. - **A clear decision rule**, so proposals actually close instead of drifting. +Many of these decisions are not one-shot: a label set, a style guide, or this process +itself is a **standard that keeps evolving** in small steps. The *current state* is the +point, so a QEP must also be maintainable in place — adding a label should not mean +retiring the whole document and chasing the live standard across a chain of superseding +proposals. + This mirrors the role [PEPs](https://peps.python.org/pep-0001/) play for Python and [MEPs](https://mep.mystmd.org) for MyST, but is scoped more broadly: QEPs cover **governance and editorial decisions as well as software**, and the process is kept far @@ -89,6 +98,12 @@ Accepted, Rejected, and Withdrawn QEPs are all **merged**, so the record is dura only abandoned or spam drafts are closed without merging. There is no separate `Final` state — `Accepted` *is* final. +`status` answers only *was this agreed?* — it is universal and unaffected by the +versioning below. In particular, `Superseded` is reserved for a **wholesale +replacement** (a different decision that retires this one), not for the everyday +evolution of a living standard, which is handled by amending the QEP in place and +bumping its `version`. + ### How a QEP is decided 1. **(Optional) Float the idea.** Open a *QEP discussion* issue to socialise it and @@ -102,7 +117,102 @@ only abandoned or spam drafts are closed without merging. There is no separate objections are raised as PR comments, and no sustained objection means the QEP is Accepted. If there is no consensus, the lead (@jstac) decides or defers. 5. **Record.** On acceptance, set **Status: Accepted** — in the frontmatter, the header - table, and the README index row — confirm the number, and merge. + table, and the README index row — confirm the number, and merge. A newly accepted + QEP carries no `version`: it is implicitly **v0** until first amended. + +### Amending an accepted QEP + +An accepted QEP is not frozen. A QEP that sets an ongoing rule — a label schema, a style +guide, editorial conventions — is a **living standard**: its *current state* is the +point, and it evolves in small, frequent steps. Two paths keep that change orderly: + +- **Amend in place.** A substantive evolution of the *same* standard — tweak a value, + add a label, clarify a rule — is a normal PR against the QEP, reviewed under the same + lazy-consensus rule, that bumps the QEP's `version` (below). The document stays + `Accepted`, and a reader always sees one current standard instead of chasing a chain + of superseding documents. +- **Supersede.** A *different* decision that replaces the QEP wholesale is a **new** QEP + that marks the old one `Superseded` (link it). Superseding is reserved for a genuine + rethink, not routine maintenance. + +**Squash-merge only.** Each amendment lands as a single commit, so a QEP's history +reads as one line per change. This is a repository setting, not a convention to +remember. + +**Commit subjects** carry the substantive/editorial distinction (below): a substantive +amendment uses `QEP-N vM: ` (e.g. `QEP-2 v1: add release-blocker label`); an +editorial fix uses `QEP-N: `. The `vM:` token in the log is itself the +substantive-milestone marker. + +### Versioning: `version` and its git anchor + +A QEP gains a `version` the first time it is **substantively** changed after acceptance: + +| `version` | Meaning | +| ----------- | --------------------------------------------------------------------- | +| *absent* | Implicitly **v0** — as originally accepted, never substantively changed. Many QEPs (a one-off decision) stay here forever. | +| `1`, `2`, … | The current substantive revision. The first substantive amendment introduces `version: 1`; each later substantive change climbs to `2`, `3`, … | + +From `v1` onward the field carries a short commit hash as a YAML comment, anchoring the +revision to git history: + +```yaml +version: 1 # a1b2c3d +``` + +The hash is stamped **automatically at merge** — a commit cannot contain its own hash, +so a post-merge step (see *Automation*) writes it; never hand-write it. Tooling that +pins a standard (for example a labels-sync command) reads `version` and verifies against +the hash. A per-QEP `version` is the right anchor because a git *tag* tags the whole +repository, not one QEP's revision. + +**Substantive vs editorial** decides whether the number moves: + +- **Substantive** — any change to normative content (a rule, a value, a table row, a + machine-readable appendix) → **bump `version`** by one; the hash moves too. +- **Editorial** — no change to normative content (a typo, wording, formatting, a link) + → **`version` unchanged**; only the hash moves (at v0, the change is simply a git + commit). + +One-line rule: *editorial = no change to normative content; substantive = any change to +normative content.* This keeps version numbers meaningful — not inflated by typos — +while every change stays precisely pinned by the hash and visible in git. The call sits +with author and reviewer; CI does not classify it. + +### History and publication + +The change record is **git itself**, surfaced rather than duplicated into a +hand-maintained changelog (which would drift and clutter the document): + +- **On the rendered site** the QuantEcon theme's git-history control shows + *Last changed: ⟨date⟩* and expands a dropdown of recent commits for the page, each + linked to GitHub ([quantecon-theme.mystmd#83](https://github.com/QuantEcon/quantecon-theme.mystmd/pull/83)). + The squash-commit subjects are the changelog entries. +- **On GitHub** the file's full commit history and blame cover anything beyond the + recent window. + +Type and version are made visible on publication: + +- a coloured **`type` pill** always, and a **`version` pill** once a QEP reaches `v1` — + e.g. `standard` · `v2`; a v0 QEP shows only the type pill; +- the **README index** carries `Type` and `Version` columns, with `Version` showing `–` + at v0 and `v{N}` thereafter. + +### Automation + +Two mechanical steps are enforced by CI rather than left to memory: + +- a **post-merge action** (`.github/workflows/stamp-version.yml`) reads the merged short + hash, stamps it into the `version: N # ` comment, and keeps the README + `Type`/`Version` columns in sync with each QEP's frontmatter; +- a **pull-request check** (`.github/workflows/qep-checks.yml`) confirms that `version`, + when present, either stays the same (editorial) or increases by exactly one + (substantive), and that the README `Type`/`Status`/`Version` columns match each QEP's + frontmatter. + +The author-side judgement — substantive vs editorial, bumping `version`, the commit +subject — is documented in `AGENTS.md`; CI enforces the mechanical steps that a +maintainer merging through the GitHub UI would otherwise have to remember. ### Numbering @@ -124,13 +234,22 @@ light as the decisions it records. ### Format Each QEP is a Markdown file with YAML frontmatter (`qep`, `title`, `author`, `status`, -`type`, `created`, `discussion`) followed by the sections in +`type`, `created`, `discussion` — plus `version`, which sits just after `type` once the +QEP is first amended) followed by the sections in [`qeps/template.md`](../qeps/template.md): **Summary, Motivation, Proposal, Alternatives -considered, Rollout**. The `type` field is one of: +considered, Rollout**. The `type` field describes the **kind of content** the QEP +carries: + +- **`standard`** — a normative spec or rule you conform to (a label schema, a style + guide, editorial or metadata conventions, a licensing choice). +- **`process`** — how the team or the QEP system itself operates (this QEP; a review or + release procedure; governance). +- **`informational`** — non-binding guidance, rationale, or reference (design notes, + recommendations, recorded rationale). -- **Process** — how QuantEcon works (this QEP). -- **Standard** — a concrete shared standard (e.g. a label set, a style rule). -- **Informational** — guidance or a recommendation with no binding action. +A *policy* is a normative rule — a `standard` you conform to — so it is not a separate +type; a one-off *decision* is a `standard` if it sets an ongoing rule, or +`informational` if it only records rationale. ## Alternatives considered @@ -147,13 +266,30 @@ considered, Rollout**. The `type` field is one of: `Draft → Accepted → Final` split) was rejected as disproportionate for a team of a handful of maintainers. The Django DEP and MEP processes — small-team enhancement processes on a comparable scale — informed the lightweight shape adopted here. +- **A `Living` or `Active` status vs versioning** (added at v1). PEP marks + continuously-maintained documents `Active`; we instead carry living-ness on a + `version` and leave `status` to mean only *was this agreed?* This keeps the status set + small and sidesteps the collision with MyST's MEPs, where `Active` means *frozen for + voting* — the opposite sense. +- **A hand-maintained changelog vs git** (added at v1). A changelog section inside each + QEP would duplicate git, drift from it, and clutter the document; we point at git + instead, surfaced on the site by the theme's history feature and on GitHub by + history/blame. ## Rollout -1. Merge this QEP (QEP-1) to establish the process. -2. Re-record the label-set decision - ([QuantEcon/meta#324](https://github.com/QuantEcon/meta/issues/324)) as a worked - `Standard`-type QEP. -3. Add a short "How we make cross-repo decisions" entry to the team manual pointing at - `QuantEcon/qeps`, and have Core Maintainers watch the repository so proposal - deadlines are seen. +1. **(v0) Establish the process.** Merge this QEP to set the process; re-record the + label-set decision + ([QuantEcon/meta#324](https://github.com/QuantEcon/meta/issues/324)) as a + `standard`-type QEP; add a short "How we make cross-repo decisions" entry to the team + manual pointing at `QuantEcon/qeps`, and have Core Maintainers watch the repository so + proposal deadlines are seen. +2. **(v1) Adopt in-place versioning.** Add the `version` field and its git-hash anchor, + the substantive/editorial rule, the amend-in-place + squash-merge + commit-subject + conventions, the `standard`/`process`/`informational` `type` taxonomy, and git + (surfaced on the site) as the change record. Supporting changes: update + `qeps/template.md`; add `Type`/`Version` columns to the README index; add the + post-merge stamp + README-sync action and the pull-request check; document the + author-side steps in `AGENTS.md`; and set the repository to squash-merge only. + Adopting this mechanism is QEP-1's own first substantive amendment, so QEP-1 + becomes **v1**. diff --git a/qeps/template.md b/qeps/template.md index 1dfc495..ac240a0 100644 --- a/qeps/template.md +++ b/qeps/template.md @@ -3,12 +3,16 @@ qep: N title: author: "@your-handle" status: Draft -type: Process | Standard | Informational +type: standard | process | informational created: YYYY-MM-DD discussion: <link to the QEP discussion issue or thread> --- <!-- Number unpadded, e.g. 7 — the filename zero-pads it to four digits: qeps/qep-0007-short-slug.md --> +<!-- A new QEP is unversioned (implicitly v0): omit `version`. It gains + `version: 1` — with a `# <hash>` comment that CI stamps at merge — and a + Version row in the table below the first time it is substantively amended + after acceptance. See QEP-1 (Versioning). --> # QEP-N: <Title> @@ -18,7 +22,7 @@ discussion: <link to the QEP discussion issue or thread> | **Title** | \<Title> | | **Author** | @your-handle | | **Status** | Draft | -| **Type** | Process / Standard / Informational | +| **Type** | standard / process / informational | | **Created** | YYYY-MM-DD | | **Discussion** | \<issue link> | From 96ad24af291528c31fe0c9e919b9e31979fb2540 Mon Sep 17 00:00:00 2001 From: Matt McKay <mmcky@users.noreply.github.com> Date: Thu, 25 Jun 2026 10:31:09 +1000 Subject: [PATCH 2/6] QEP-1 v1: anchor revision via a version-hash key, not a YAML comment Move the git anchor from the `version: N # <hash>` comment to a sibling `version-hash` frontmatter key. A YAML comment is non-semantic and silently dropped by any parser; a real key is preserved as data and parseQep now throws on a malformed value. `version` stays a plain scalar, so the version pill and the increment check read it untouched. Scope all version parsing/stamping to the frontmatter block: a QEP body may now contain a literal version:/version-hash: YAML example (QEP-1 does), which the old whole-file regex in stamp.mjs/check.mjs would have matched by mistake. Update QEP-1 text, the template, AGENTS.md, and the README to match. Stays at v1 (no bump) since v1 is not yet merged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --- .github/scripts/check.mjs | 7 +++++-- .github/scripts/qeps.mjs | 29 +++++++++++++++++++--------- .github/scripts/stamp.mjs | 23 +++++++++++++++------- .github/workflows/stamp-version.yml | 2 +- AGENTS.md | 6 +++--- README.md | 2 +- qeps/qep-0001-purpose-and-process.md | 28 +++++++++++++++------------ qeps/template.md | 6 +++--- 8 files changed, 65 insertions(+), 38 deletions(-) diff --git a/.github/scripts/check.mjs b/.github/scripts/check.mjs index ae1150c..fbd5ef3 100644 --- a/.github/scripts/check.mjs +++ b/.github/scripts/check.mjs @@ -4,7 +4,7 @@ // 2. The README Type/Status/Version columns match each QEP's frontmatter. // Run by .github/workflows/qep-checks.yml. Exits non-zero on any failure. import { execSync } from 'node:child_process'; -import { parseQep, qepFiles, readIndex, versionCell } from './qeps.mjs'; +import { FRONTMATTER, parseQep, qepFiles, readIndex, versionCell } from './qeps.mjs'; const base = process.env.BASE_REF || 'main'; const errors = []; @@ -19,7 +19,10 @@ function baseVersion(path) { } catch { return undefined; // file did not exist on the base branch } - const m = text.match(/^version:[ \t]*(\d+)/m); + // Read `version` from the frontmatter only — ignore any body YAML example. + const fm = text.match(FRONTMATTER); + const block = fm ? fm[1] : text; + const m = block.match(/^version:[ \t]*(\d+)/m); return m ? Number(m[1]) : undefined; } diff --git a/.github/scripts/qeps.mjs b/.github/scripts/qeps.mjs index bfb4a7d..7cf12e4 100644 --- a/.github/scripts/qeps.mjs +++ b/.github/scripts/qeps.mjs @@ -13,7 +13,10 @@ export function qepFiles() { .map((f) => `${QEP_DIR}/${f}`); } -const FRONTMATTER = /^---\r?\n([\s\S]*?)\r?\n---/; +// The leading YAML frontmatter block. Exported so the stamp/check scripts can +// scope their version parsing to it — a QEP *body* may contain literal +// `version:`/`version-hash:` lines inside a YAML example, which must be ignored. +export const FRONTMATTER = /^---\r?\n([\s\S]*?)\r?\n---/; function field(block, name) { const m = block.match(new RegExp(`^${name}:[ \\t]*(.*?)[ \\t]*$`, 'm')); @@ -25,21 +28,29 @@ function stripQuotes(s) { } // Parse the fields we care about from a QEP file's YAML frontmatter. -// `version` is a number (undefined = implicitly v0); `hash` is the stamped short -// SHA from the `version: N # <hash>` comment (undefined when not yet stamped). +// `version` is a plain number (undefined = implicitly v0); `hash` is the stamped +// short SHA from the sibling `version-hash` field (undefined when not yet stamped). export function parseQep(path) { const text = readFileSync(path, 'utf8'); const fm = text.match(FRONTMATTER); if (!fm) throw new Error(`${path}: missing YAML frontmatter`); const block = fm[1]; - const raw = field(block, 'version'); - let version, hash; - if (raw !== undefined && raw !== '') { - const v = raw.match(/^(\d+)(?:[ \t]*#[ \t]*([0-9a-fA-F]+))?$/); - if (!v) throw new Error(`${path}: malformed "version: ${raw}"`); + const rawVersion = field(block, 'version'); + let version; + if (rawVersion !== undefined && rawVersion !== '') { + const v = rawVersion.match(/^(\d+)$/); + if (!v) throw new Error(`${path}: malformed "version: ${rawVersion}"`); version = Number(v[1]); - hash = v[2]; + } + + // `version-hash` may carry a trailing "# stamped by CI" signpost comment. + const rawHash = field(block, 'version-hash'); + let hash; + if (rawHash !== undefined && rawHash !== '') { + const h = rawHash.match(/^([0-9a-fA-F]+)(?:[ \t]*#.*)?$/); + if (!h) throw new Error(`${path}: malformed "version-hash: ${rawHash}"`); + hash = h[1]; } const qep = field(block, 'qep'); diff --git a/.github/scripts/stamp.mjs b/.github/scripts/stamp.mjs index ec63d63..a1c0d1e 100644 --- a/.github/scripts/stamp.mjs +++ b/.github/scripts/stamp.mjs @@ -1,10 +1,11 @@ -// Post-merge: stamp the merged short hash into each changed QEP's -// `version: N # <hash>` comment, and sync the README Type/Version columns from -// each QEP's frontmatter. Run by .github/workflows/stamp-version.yml on push to -// main. Idempotent: writes nothing when everything is already current. +// Post-merge: stamp the merged short hash into each changed QEP's `version-hash` +// field, and sync the README Type/Version columns from each QEP's frontmatter. +// Run by .github/workflows/stamp-version.yml on push to main. Idempotent: writes +// nothing when everything is already current. import { execSync } from 'node:child_process'; import { readFileSync, writeFileSync } from 'node:fs'; import { + FRONTMATTER, README, formatRow, parseQep, @@ -26,14 +27,22 @@ const changed = new Set( let dirty = false; -// 1. Stamp the hash into any changed v1+ QEP whose comment isn't already this SHA. +// 1. Stamp the hash into any changed v1+ QEP whose `version-hash` isn't this SHA. +const SIGNPOST = '# stamped by CI; do not edit'; for (const path of qepFiles()) { if (!changed.has(path)) continue; const { version, hash } = parseQep(path); - if (version === undefined) continue; // v0 — no version line to stamp + if (version === undefined) continue; // v0 — no version to stamp if (hash === sha) continue; // already current const text = readFileSync(path, 'utf8'); - const stamped = text.replace(/^(version:[ \t]*\d+)[ \t]*(#.*)?$/m, `$1 # ${sha}`); + const line = `version-hash: ${sha} ${SIGNPOST}`; + // Edit only the frontmatter block (anchored at the file start) — the body may + // carry a literal version:/version-hash: YAML example that must not be touched. + const head = text.match(FRONTMATTER)[0]; + const newHead = /^version-hash:.*$/m.test(head) + ? head.replace(/^version-hash:.*$/m, line) // re-stamp: replace existing + : head.replace(/^(version:[ \t]*\d+)[ \t]*$/m, `$1\n${line}`); // first stamp: insert + const stamped = newHead + text.slice(head.length); if (stamped !== text) { writeFileSync(path, stamped); dirty = true; diff --git a/.github/workflows/stamp-version.yml b/.github/workflows/stamp-version.yml index 1173895..23afe8c 100644 --- a/.github/workflows/stamp-version.yml +++ b/.github/workflows/stamp-version.yml @@ -22,7 +22,7 @@ jobs: # That same no-retrigger behaviour means this push does not redeploy the # site — which is fine: everything visible (the type/version pills and the # README columns) is already correct from the merge commit, and only the - # invisible `# <hash>` comment waits for the next push to main to republish. + # invisible `version-hash` field waits for the next push to main to republish. if: ${{ !contains(github.event.head_commit.message, '[skip-stamp]') }} runs-on: ubuntu-latest steps: diff --git a/AGENTS.md b/AGENTS.md index 66d249c..50c644a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -64,8 +64,8 @@ marks the old one `Superseded`. Don't supersede for routine maintenance. - **Editorial** (no change to normative content — typo, wording, formatting, link): leave `version` unchanged. -Leave the `# <hash>` comment **off** when you set or bump `version` — CI stamps it with -the merge hash; never hand-write it (a commit cannot contain its own hash). +Leave `version-hash` **off** when you set or bump `version` — CI adds it with the merge +hash; never hand-write it (a commit cannot contain its own hash). **Commit subjects** carry the same distinction: @@ -81,7 +81,7 @@ 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 `version: N # <hash>` and syncs the README `Type`/`Version` + merged short hash into the `version-hash` field and syncs the README `Type`/`Version` columns from each QEP's frontmatter. - **On every PR** — [`qep-checks.yml`](.github/workflows/qep-checks.yml) checks that `version`, if present, either stays the same (editorial) or increases by exactly one diff --git a/README.md b/README.md index f514d6f..eac2854 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ 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 +and each QEP's `version-hash` is stamped into its frontmatter at merge; `Version` reads `–` until a QEP is first amended. ## Proposing a QEP diff --git a/qeps/qep-0001-purpose-and-process.md b/qeps/qep-0001-purpose-and-process.md index ba95dfd..2f472fd 100644 --- a/qeps/qep-0001-purpose-and-process.md +++ b/qeps/qep-0001-purpose-and-process.md @@ -153,18 +153,22 @@ A QEP gains a `version` the first time it is **substantively** changed after acc | *absent* | Implicitly **v0** — as originally accepted, never substantively changed. Many QEPs (a one-off decision) stay here forever. | | `1`, `2`, … | The current substantive revision. The first substantive amendment introduces `version: 1`; each later substantive change climbs to `2`, `3`, … | -From `v1` onward the field carries a short commit hash as a YAML comment, anchoring the -revision to git history: +From `v1` onward a sibling `version-hash` field carries the short commit hash that +anchors the revision to git history: ```yaml -version: 1 # a1b2c3d +version: 1 +version-hash: a1b2c3d # stamped by CI; do not edit ``` -The hash is stamped **automatically at merge** — a commit cannot contain its own hash, -so a post-merge step (see *Automation*) writes it; never hand-write it. Tooling that -pins a standard (for example a labels-sync command) reads `version` and verifies against -the hash. A per-QEP `version` is the right anchor because a git *tag* tags the whole -repository, not one QEP's revision. +`version` stays a plain number — so the rendered version pill and any tooling that reads +it are untouched — and the hash lives in its own **field rather than a YAML comment**, so +a standard YAML parser keeps it (a comment would be discarded). The hash is stamped +**automatically at merge** — a commit cannot contain its own hash, so a post-merge step +(see *Automation*) writes it; never hand-write it. Tooling that pins a standard (for +example a labels-sync command) reads `version` and verifies against `version-hash`. A +per-QEP `version` is the right anchor because a git *tag* tags the whole repository, not +one QEP's revision. **Substantive vs editorial** decides whether the number moves: @@ -203,8 +207,8 @@ Type and version are made visible on publication: Two mechanical steps are enforced by CI rather than left to memory: - a **post-merge action** (`.github/workflows/stamp-version.yml`) reads the merged short - hash, stamps it into the `version: N # <hash>` comment, and keeps the README - `Type`/`Version` columns in sync with each QEP's frontmatter; + hash, writes it into the `version-hash` field, and keeps the README `Type`/`Version` + columns in sync with each QEP's frontmatter; - a **pull-request check** (`.github/workflows/qep-checks.yml`) confirms that `version`, when present, either stays the same (editorial) or increases by exactly one (substantive), and that the README `Type`/`Status`/`Version` columns match each QEP's @@ -234,8 +238,8 @@ light as the decisions it records. ### Format Each QEP is a Markdown file with YAML frontmatter (`qep`, `title`, `author`, `status`, -`type`, `created`, `discussion` — plus `version`, which sits just after `type` once the -QEP is first amended) followed by the sections in +`type`, `created`, `discussion` — plus `version` and its CI-stamped `version-hash`, which +sit just after `type` once the QEP is first amended) followed by the sections in [`qeps/template.md`](../qeps/template.md): **Summary, Motivation, Proposal, Alternatives considered, Rollout**. The `type` field describes the **kind of content** the QEP carries: diff --git a/qeps/template.md b/qeps/template.md index ac240a0..ecdccb2 100644 --- a/qeps/template.md +++ b/qeps/template.md @@ -10,9 +10,9 @@ discussion: <link to the QEP discussion issue or thread> <!-- Number unpadded, e.g. 7 — the filename zero-pads it to four digits: qeps/qep-0007-short-slug.md --> <!-- A new QEP is unversioned (implicitly v0): omit `version`. It gains - `version: 1` — with a `# <hash>` comment that CI stamps at merge — and a - Version row in the table below the first time it is substantively amended - after acceptance. See QEP-1 (Versioning). --> + `version: 1` — with a sibling `version-hash` field that CI stamps at merge — + and a Version row in the table below the first time it is substantively + amended after acceptance. See QEP-1 (Versioning). --> # QEP-N: <Title> From 4486c4b9ad0b5a5c0032bbc7cfdf5190f7f795ed Mon Sep 17 00:00:00 2001 From: Matt McKay <mmcky@users.noreply.github.com> Date: Thu, 25 Jun 2026 11:01:31 +1000 Subject: [PATCH 3/6] QEP-1 v1: edit prose to describe the process, not its own history QEP-1's own principle is that the change record is git, not in-document. Make the prose practise that: it should read as the current process, with the v0->v1 story left to git history. No normative content changes. - Fold the status/version orthogonality and the Superseded clarification into the Lifecycle paragraph, dropping the redundant standalone paragraph (the Amending section already owns amend-in-place vs supersede). - De-tense the Summary (drop "now at v1 ... introduces"). - Compress the version-hash rationale, dropping the framing relative to the rejected YAML-comment design. - Remove the "(added at v1)" annotations from Alternatives considered. - Fix a stale "git-hash anchor" -> "version-hash" in Rollout. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --- qeps/qep-0001-purpose-and-process.md | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/qeps/qep-0001-purpose-and-process.md b/qeps/qep-0001-purpose-and-process.md index 2f472fd..5766ac7 100644 --- a/qeps/qep-0001-purpose-and-process.md +++ b/qeps/qep-0001-purpose-and-process.md @@ -30,8 +30,8 @@ team works**. This QEP defines what a QEP is, when one is needed, where QEPs liv proposal moves from draft to decision, and how an accepted QEP is **maintained over time**. It is deliberately lightweight: the aim is a ten-minute read, a clear deadline, and a clean close — not governance for its own sake. As the first proposal, this -document is also a worked example of the template it describes and — now at v1 — of the -in-place versioning it introduces. +document is also a worked example of the template and the in-place versioning it +describes. ## Motivation @@ -96,13 +96,10 @@ A QEP carries one status: Accepted, Rejected, and Withdrawn QEPs are all **merged**, so the record is durable; only abandoned or spam drafts are closed without merging. There is no separate -`Final` state — `Accepted` *is* final. - -`status` answers only *was this agreed?* — it is universal and unaffected by the -versioning below. In particular, `Superseded` is reserved for a **wholesale -replacement** (a different decision that retires this one), not for the everyday -evolution of a living standard, which is handled by amending the QEP in place and -bumping its `version`. +`Final` state — `Accepted` *is* final. `status` records only *whether a QEP was +agreed* and is independent of a QEP's `version`; `Superseded` marks a wholesale +replacement by a later QEP (below), not the routine in-place evolution of a living +standard. ### How a QEP is decided @@ -161,9 +158,8 @@ version: 1 version-hash: a1b2c3d # stamped by CI; do not edit ``` -`version` stays a plain number — so the rendered version pill and any tooling that reads -it are untouched — and the hash lives in its own **field rather than a YAML comment**, so -a standard YAML parser keeps it (a comment would be discarded). The hash is stamped +`version` is a plain number; the commit hash lives in the separate `version-hash` field — +a real key, so any YAML parser keeps it. The hash is stamped **automatically at merge** — a commit cannot contain its own hash, so a post-merge step (see *Automation*) writes it; never hand-write it. Tooling that pins a standard (for example a labels-sync command) reads `version` and verifies against `version-hash`. A @@ -270,12 +266,12 @@ type; a one-off *decision* is a `standard` if it sets an ongoing rule, or `Draft → Accepted → Final` split) was rejected as disproportionate for a team of a handful of maintainers. The Django DEP and MEP processes — small-team enhancement processes on a comparable scale — informed the lightweight shape adopted here. -- **A `Living` or `Active` status vs versioning** (added at v1). PEP marks +- **A `Living` or `Active` status vs versioning.** PEP marks continuously-maintained documents `Active`; we instead carry living-ness on a `version` and leave `status` to mean only *was this agreed?* This keeps the status set small and sidesteps the collision with MyST's MEPs, where `Active` means *frozen for voting* — the opposite sense. -- **A hand-maintained changelog vs git** (added at v1). A changelog section inside each +- **A hand-maintained changelog vs git.** A changelog section inside each QEP would duplicate git, drift from it, and clutter the document; we point at git instead, surfaced on the site by the theme's history feature and on GitHub by history/blame. @@ -288,8 +284,8 @@ type; a one-off *decision* is a `standard` if it sets an ongoing rule, or `standard`-type QEP; add a short "How we make cross-repo decisions" entry to the team manual pointing at `QuantEcon/qeps`, and have Core Maintainers watch the repository so proposal deadlines are seen. -2. **(v1) Adopt in-place versioning.** Add the `version` field and its git-hash anchor, - the substantive/editorial rule, the amend-in-place + squash-merge + commit-subject +2. **(v1) Adopt in-place versioning.** Add the `version` field and its `version-hash` + anchor, the substantive/editorial rule, the amend-in-place + squash-merge + commit-subject conventions, the `standard`/`process`/`informational` `type` taxonomy, and git (surfaced on the site) as the change record. Supporting changes: update `qeps/template.md`; add `Type`/`Version` columns to the README index; add the From 791af8c5bc484584a0829efc80400d6d35c8ef7c Mon Sep 17 00:00:00 2001 From: Matt McKay <mmcky@users.noreply.github.com> Date: Thu, 25 Jun 2026 11:18:57 +1000 Subject: [PATCH 4/6] QEP-1 v1: harden CI checks and stamping; make rendering claims contingent Address the PR review's must-fixes and cheap hardening. - stamp.mjs (#1): detect a merge commit and diff against the first parent (`git diff --name-only HEAD^1 HEAD`) instead of `git diff-tree -r HEAD`, which prints nothing for a merge and would make stamping silently no-op if a QEP PR is merged as a merge commit rather than squashed. Verified on synthetic merge and squash commits. - check.mjs (#2, #5): a new QEP must start unversioned (v0); a versioned QEP may not drop its version field (once versioned, stays versioned). Distinguishes a brand-new file from a v0 file via a base-existence check. - check.mjs (#3): validate that type/status are known enum values. - check.mjs (#8): tolerate a hand-typed ASCII "-" or empty README Version cell for a v0 QEP (stamp.mjs normalises it to the en dash post-merge), so the cell no longer blocks the PR with a visually identical-looking parity error. - stamp-version.yml (#6b): push explicitly with `git push origin HEAD:main`. - QEP-1 + AGENTS.md (#7): the site uses book-theme today, so make the type/version pills and git-history-dropdown claims contingent on the QuantEcon theme; lead with the always-true README columns. Describe the strengthened PR checks. All eight check.mjs scenarios pass in an isolated synthetic repo; check.mjs passes on the real tree. Stays at v1 (no normative rule changed). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --- .github/scripts/check.mjs | 63 ++++++++++--- .github/scripts/stamp.mjs | 19 +++- .github/workflows/stamp-version.yml | 2 +- AGENTS.md | 5 +- pr-5-review.md | 135 +++++++++++++++++++++++++++ qeps/qep-0001-purpose-and-process.md | 22 +++-- 6 files changed, 218 insertions(+), 28 deletions(-) create mode 100644 pr-5-review.md diff --git a/.github/scripts/check.mjs b/.github/scripts/check.mjs index fbd5ef3..cb9dd91 100644 --- a/.github/scripts/check.mjs +++ b/.github/scripts/check.mjs @@ -1,7 +1,9 @@ // Pull-request checks: -// 1. `version`, when present, only ever increments by one (and a first -// version is 1) relative to the base branch. -// 2. The README Type/Status/Version columns match each QEP's frontmatter. +// 1. `version` moves legally: a new QEP starts unversioned (v0); once a QEP is +// versioned it stays versioned; a first version is 1; otherwise it stays equal +// (editorial) or increments by one (substantive) relative to the base branch. +// 2. `type` and `status` are known values. +// 3. The README Type/Status/Version columns match each QEP's frontmatter. // Run by .github/workflows/qep-checks.yml. Exits non-zero on any failure. import { execSync } from 'node:child_process'; import { FRONTMATTER, parseQep, qepFiles, readIndex, versionCell } from './qeps.mjs'; @@ -9,28 +11,45 @@ import { FRONTMATTER, parseQep, qepFiles, readIndex, versionCell } from './qeps. const base = process.env.BASE_REF || 'main'; const errors = []; -// `version` of a QEP file on the base branch (undefined if absent / new / v0). -function baseVersion(path) { +const TYPES = new Set(['standard', 'process', 'informational']); +const STATUSES = new Set(['Draft', 'Accepted', 'Rejected', 'Withdrawn', 'Superseded']); + +// State of a QEP file on the base branch: whether it existed, and its `version` +// (read from the frontmatter only, so a body YAML example is ignored). +function baseState(path) { let text; try { text = execSync(`git show "origin/${base}:${path}"`, { stdio: ['pipe', 'pipe', 'ignore'], }).toString(); } catch { - return undefined; // file did not exist on the base branch + return { existed: false, version: undefined }; } - // Read `version` from the frontmatter only — ignore any body YAML example. const fm = text.match(FRONTMATTER); const block = fm ? fm[1] : text; const m = block.match(/^version:[ \t]*(\d+)/m); - return m ? Number(m[1]) : undefined; + return { existed: true, version: m ? Number(m[1]) : undefined }; } -// 1. version increments by at most one; a newly introduced version must be 1. +// 1. version moves legally relative to the base branch. for (const path of qepFiles()) { const { version } = parseQep(path); - if (version === undefined) continue; // v0 — always fine - const prev = baseVersion(path); + const { existed, version: prev } = baseState(path); + + if (!existed) { + // A brand-new QEP must start unversioned (implicitly v0). + if (version !== undefined) { + errors.push(`${path}: a new QEP must start unversioned (v0) — remove the version field`); + } + continue; + } + if (version === undefined) { + // Dropping `version` is only legal if the QEP was never versioned. + if (prev !== undefined) { + errors.push(`${path}: version ${prev} was removed; once versioned, a QEP stays versioned`); + } + continue; + } if (prev === undefined) { if (version !== 1) { errors.push(`${path}: introduces version ${version}; a first version must be 1`); @@ -42,7 +61,7 @@ for (const path of qepFiles()) { } } -// 2. README Type/Status/Version columns match frontmatter. +// 2/3. type/status enums, and README Type/Status/Version parity with frontmatter. const idx = readIndex(); if (idx.cols.type === -1) errors.push(`${'README.md'}: index table is missing a Type column`); if (idx.cols.version === -1) errors.push(`${'README.md'}: index table is missing a Version column`); @@ -50,6 +69,14 @@ 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; + + if (q.type !== undefined && !TYPES.has(q.type)) { + errors.push(`${path}: unknown type "${q.type}" (expected one of ${[...TYPES].join(', ')})`); + } + 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})`); @@ -62,7 +89,17 @@ for (const path of qepFiles()) { }; expect('Type', idx.cols.type, q.type); expect('Status', idx.cols.status, q.status); - expect('Version', idx.cols.version, versionCell(q.version)); + + // 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}"`); + } + } } if (errors.length) { diff --git a/.github/scripts/stamp.mjs b/.github/scripts/stamp.mjs index a1c0d1e..f43ac41 100644 --- a/.github/scripts/stamp.mjs +++ b/.github/scripts/stamp.mjs @@ -16,9 +16,24 @@ import { const sha = execSync('git rev-parse --short HEAD').toString().trim(); -// Files touched by the tip commit (QEP PRs are squash-merged => one commit). +// Files changed by the push tip. QEP PRs should be squash-merged (one parent), +// but `git diff-tree -r HEAD` prints nothing for a merge commit — so if a PR is +// merged as a merge commit, stamping would silently no-op. Diffing against the +// first parent works for both squash and merge commits. +const parents = execSync('git rev-list --parents -n 1 HEAD') + .toString() + .trim() + .split(/\s+/) + .slice(1); +if (parents.length > 1) { + console.log(`note: HEAD has ${parents.length} parents (merge commit); diffing against the first parent`); +} const changed = new Set( - execSync('git diff-tree --no-commit-id --name-only -r HEAD') + execSync( + parents.length + ? 'git diff --name-only HEAD^1 HEAD' + : 'git diff-tree --no-commit-id --name-only -r HEAD', // root commit has no parent + ) .toString() .split('\n') .map((s) => s.trim()) diff --git a/.github/workflows/stamp-version.yml b/.github/workflows/stamp-version.yml index 23afe8c..4acaad8 100644 --- a/.github/workflows/stamp-version.yml +++ b/.github/workflows/stamp-version.yml @@ -47,4 +47,4 @@ jobs: # Rebase onto anything that landed on main while we were stamping, # then push, so a near-simultaneous merge can't drop this stamp. git pull --rebase origin main - git push + git push origin HEAD:main diff --git a/AGENTS.md b/AGENTS.md index 50c644a..a0d730c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -84,8 +84,9 @@ GitHub UI choose **Squash and merge**. merged short hash into the `version-hash` field and syncs the README `Type`/`Version` columns from each QEP's frontmatter. - **On every PR** — [`qep-checks.yml`](.github/workflows/qep-checks.yml) checks that - `version`, if present, either stays the same (editorial) or increases by exactly one - (substantive), and that the README `Type`/`Status`/`Version` columns match each QEP's + `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 diff --git a/pr-5-review.md b/pr-5-review.md new file mode 100644 index 0000000..447a618 --- /dev/null +++ b/pr-5-review.md @@ -0,0 +1,135 @@ +# Review — PR #5: QEP-1 v1, version-defined living standards + +**PR:** https://github.com/QuantEcon/qeps/pull/5 +**Branch:** `qep-1-versioned-living-standards` → `main` +**Design issue:** [#4](https://github.com/QuantEcon/qeps/issues/4) +**Reviewed:** 2026-06-25 +**Status of this doc:** working notes for a follow-up session (untracked; delete or `.gitignore` when done) + +--- + +## Verdict + +Strong, well-conceived PR. The design is **faithful to the converged design in #4** — no new status word, living-ness carried on `version`, `type` repurposed as a content taxonomy, git-as-changelog, squash-merge model. The prose is high quality and internally consistent, and the frontmatter-scoping in the second commit (anchoring all `version`/`version-hash` parsing to the frontmatter block so a YAML example in a QEP *body* is never mistaken for the real field) is correct and tested. `check.mjs` passes locally and on CI. + +There are a few real issues — one a **silent failure this very PR can trigger on merge**. Address **#1, #2, #7** before merging; the rest is low-priority polish. + +### Priority order + +1. **#1** — make stamping robust to non-squash merges *or* enable squash-only before merge (silent failure otherwise). +2. **#2** — close the version-deletion gap in `check.mjs` (Copilot's comment; valid). +3. **#7** — reconcile QEP-1's rendering claims with the configured theme. +4. Optional polish: **#3, #4, #5, #6, #8**. +5. Prose/process: **#9, #10**. + +--- + +## Correctness / CI + +### 1. [High] Stamping silently no-ops on a non-squash merge — and squash-only isn't enabled yet + +`stamp.mjs` derives its changed-file set from `git diff-tree --no-commit-id --name-only -r HEAD` ([.github/scripts/stamp.mjs:20-21](.github/scripts/stamp.mjs#L20-L21)). Confirmed by reproduction: for a **two-parent merge commit this prints nothing** (git suppresses merge diffs without `-m`/`--cc`); for a squash commit it lists files correctly. So the stamp step depends entirely on squash-merge — but enabling squash-only is the PR's own "one manual step," which *cannot* be applied by this PR. If QEP-1 (or any QEP PR before the setting lands) is merged as a merge commit, `changed` is empty, **no `version-hash` is stamped, and there is no error** — the bootstrap example silently fails to produce the artifact it demonstrates. + +**Fix (do both):** +- Apply the squash-only repo setting *before* this merges (Settings → General → Pull Requests). +- Harden `stamp.mjs` to detect a multi-parent `HEAD` and either diff against the first parent (`git diff --name-only HEAD^1 HEAD`) or fail loudly rather than no-op. + +- [ ] Addressed + +### 2. [Medium] A versioned QEP can silently drop back to v0 (Copilot comment — valid) + +`check.mjs` skips the base comparison entirely when `version` is absent: `if (version === undefined) continue;` ([.github/scripts/check.mjs:32](.github/scripts/check.mjs#L32)). A v1+ QEP that deletes its `version:` field passes CI, breaking the "once versioned, stays versioned" guarantee and desyncing the README. + +**Fix:** when the current `version` is undefined, still read `baseVersion(path)` and error if the base had a version. Sketch: + +```js +for (const path of qepFiles()) { + const { version } = parseQep(path); + const prev = baseVersion(path); + if (version === undefined) { + if (prev !== undefined) + errors.push(`${path}: version ${prev} was removed; once versioned a QEP stays versioned`); + continue; + } + if (prev === undefined) { + if (version !== 1) errors.push(`${path}: introduces version ${version}; a first version must be 1`); + } else if (version !== prev && version !== prev + 1) { + errors.push(`${path}: version ${prev} -> ${version} must stay equal (editorial) or increment by one (substantive)`); + } +} +``` + +(There's a `copilot-review` skill available to formally reply to and resolve Copilot's inline comment once fixed.) + +- [ ] Addressed + +### 3. [Low] No enum validation of `type` / `status` + +CI checks README↔frontmatter *parity* but never that `type ∈ {standard, process, informational}` or that `status` is a known value. `type: standardd` passes as long as the README matches the typo. Cheap hardening if wanted. + +- [ ] Addressed / [ ] Won't fix + +### 4. [Low] In-document header table is not parity-checked + +CI enforces README↔frontmatter parity, but the in-document header table (`| **Version** | 1 |`, `| **Type** | … |`) is hand-maintained and unchecked. For a document now meant to be amended frequently, that duplicated Type/Status/Version is exactly where drift will accumulate. [AGENTS.md:60-63](AGENTS.md#L60-L63) tells authors to bump it "in both the frontmatter and the header table," but nothing verifies it. Consider checking it, or at least flag it as the known manual-sync point. + +- [ ] Addressed / [ ] Won't fix + +### 5. [Low] A brand-new QEP can be born at v1 + +The "first version must be 1" branch ([.github/scripts/check.mjs:34-36](.github/scripts/check.mjs#L34-L36)) accepts a *new* file introduced at `version: 1`, contradicting "a new QEP is unversioned (v0)." Easy to tighten so new files must be v0. + +- [ ] Addressed / [ ] Won't fix + +### 6. [Nit] Minor robustness + +- `stamp.mjs`'s README-sync loop is effectively redundant given `check.mjs` already enforces parity pre-merge — fine as defense-in-depth, just not load-bearing. +- The final `git push` in [.github/workflows/stamp-version.yml](.github/workflows/stamp-version.yml) relies on checkout leaving you on `main`; `git push origin HEAD:main` is more explicit. + +- [ ] Addressed / [ ] Won't fix + +--- + +## Consistency + +### 7. [Medium] QEP-1 describes site rendering the repo doesn't currently produce + +[myst.yml:14](myst.yml#L14) sets `template: book-theme`, not the QuantEcon theme. But [qeps/qep-0001-purpose-and-process.md:186-203](qeps/qep-0001-purpose-and-process.md#L186-L203) asserts, in present tense, "the QuantEcon theme's git-history control shows *Last changed*…" and "a coloured `type` pill always, and a `version` pill once a QEP reaches v1." The history dropdown is a QuantEcon-theme feature (cited as the still-open `quantecon-theme.mystmd#83`), and the pills are theme-specific — under `book-theme` none of this renders. The README *columns* claim is accurate (repo-controlled); the *pills/dropdown* claims are not yet true. + +**Fix:** either switch the theme, or soften to contingent language ("once the QuantEcon theme is adopted…"). + +- [ ] Addressed + +### 8. [Low] en-dash footgun + +`versionCell` emits U+2013 EN DASH `–` for a v0 row ([.github/scripts/qeps.mjs:70](.github/scripts/qeps.mjs#L70)). The committed docs use that exact codepoint consistently (good), but a future author who hand-types an ASCII hyphen `-` in a new QEP's Version cell hits a confusing parity failure *pre*-merge — and `stamp.mjs` only fixes it *post*-merge, so it blocks the PR. Either make the character copy-pasteable in the template/AGENTS, or relax the v0 check to accept `-`/`–`/empty. + +- [ ] Addressed / [ ] Won't fix + +--- + +## Process & prose + +### 9. Draft status & @jstac's feedback + +The PR is OPEN but flagged as "supposed to be in DRAFT." @jstac approved the principle while calling the "What this does" section "word salad" — he wants a crisp sequence of rules, "like an algorithm / proof." That applies to the PR body, but the QEP's *Versioning*/*Amending* sections are also dense and could be tightened into the same rule-first shape. Convert to draft until the language pass + automation fixes land. + +- [ ] Addressed + +### 10. Two commits, squash model + +The PR has two commits; the model is "one amendment = one commit," true only if squash-merged — which loops back to **#1**. Make sure this one is squashed. + +- [ ] Addressed + +--- + +## What was verified during review + +- Read all 9 changed files, the three CI scripts, both new workflows, and the existing `deploy.yml`. +- Cross-checked the implementation against the agreed design in issue #4 (faithful). +- Ran `check.mjs` locally against `main` — passes. +- Reproduced `git diff-tree` behaviour on squash vs. true merge commits (basis for #1). +- Confirmed dash codepoints: `versionCell` and the committed docs both use U+2013 (basis for #8). +- Confirmed `myst.yml` uses `book-theme`, not the QuantEcon theme (basis for #7). +- Reviewed the Copilot bot's one inline comment — valid (#2). diff --git a/qeps/qep-0001-purpose-and-process.md b/qeps/qep-0001-purpose-and-process.md index 5766ac7..cb32ac1 100644 --- a/qeps/qep-0001-purpose-and-process.md +++ b/qeps/qep-0001-purpose-and-process.md @@ -184,19 +184,20 @@ with author and reviewer; CI does not classify it. The change record is **git itself**, surfaced rather than duplicated into a hand-maintained changelog (which would drift and clutter the document): -- **On the rendered site** the QuantEcon theme's git-history control shows - *Last changed: ⟨date⟩* and expands a dropdown of recent commits for the page, each +- **On the rendered site**, once the QuantEcon theme is adopted, its git-history control + shows *Last changed: ⟨date⟩* and expands a dropdown of recent commits for the page, each linked to GitHub ([quantecon-theme.mystmd#83](https://github.com/QuantEcon/quantecon-theme.mystmd/pull/83)). The squash-commit subjects are the changelog entries. - **On GitHub** the file's full commit history and blame cover anything beyond the recent window. -Type and version are made visible on publication: +Type and version are surfaced two ways: -- a coloured **`type` pill** always, and a **`version` pill** once a QEP reaches `v1` — - e.g. `standard` · `v2`; a v0 QEP shows only the type pill; - the **README index** carries `Type` and `Version` columns, with `Version` showing `–` - at v0 and `v{N}` thereafter. + at v0 and `v{N}` thereafter — repo-controlled, so it renders on any theme; +- under the **QuantEcon theme** (once adopted), a coloured **`type` pill** always and a + **`version` pill** once a QEP reaches `v1` — e.g. `standard` · `v2`; a v0 QEP shows only + the type pill. ### Automation @@ -205,10 +206,11 @@ Two mechanical steps are enforced by CI rather than left to memory: - a **post-merge action** (`.github/workflows/stamp-version.yml`) reads the merged short hash, writes it into the `version-hash` field, and keeps the README `Type`/`Version` columns in sync with each QEP's frontmatter; -- a **pull-request check** (`.github/workflows/qep-checks.yml`) confirms that `version`, - when present, either stays the same (editorial) or increases by exactly one - (substantive), and that the README `Type`/`Status`/`Version` columns match each QEP's - frontmatter. +- a **pull-request check** (`.github/workflows/qep-checks.yml`) confirms that `version` + moves legally — a new QEP starts unversioned, a versioned QEP stays versioned, and the + number stays the same (editorial) or increases by exactly one (substantive) — that + `type` and `status` are known values, and that the README `Type`/`Status`/`Version` + columns match each QEP's frontmatter. The author-side judgement — substantive vs editorial, bumping `version`, the commit subject — is documented in `AGENTS.md`; CI enforces the mechanical steps that a From a9a3bdc0692396a26b70b474f7bb4a2c6285109c Mon Sep 17 00:00:00 2001 From: Matt McKay <mmcky@users.noreply.github.com> Date: Thu, 25 Jun 2026 11:19:25 +1000 Subject: [PATCH 5/6] Untrack pr-5-review.md working notes (committed by mistake) These are local review notes, not part of the QEP record; gitignore the pr-*-review.md pattern so they can't be re-added. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --- .gitignore | 3 ++ pr-5-review.md | 135 ------------------------------------------------- 2 files changed, 3 insertions(+), 135 deletions(-) delete mode 100644 pr-5-review.md diff --git a/.gitignore b/.gitignore index f1e8b65..8e78ee3 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,6 @@ _build/ node_modules/ .DS_Store + +# Local review working notes (not part of the QEP record) +pr-*-review.md diff --git a/pr-5-review.md b/pr-5-review.md deleted file mode 100644 index 447a618..0000000 --- a/pr-5-review.md +++ /dev/null @@ -1,135 +0,0 @@ -# Review — PR #5: QEP-1 v1, version-defined living standards - -**PR:** https://github.com/QuantEcon/qeps/pull/5 -**Branch:** `qep-1-versioned-living-standards` → `main` -**Design issue:** [#4](https://github.com/QuantEcon/qeps/issues/4) -**Reviewed:** 2026-06-25 -**Status of this doc:** working notes for a follow-up session (untracked; delete or `.gitignore` when done) - ---- - -## Verdict - -Strong, well-conceived PR. The design is **faithful to the converged design in #4** — no new status word, living-ness carried on `version`, `type` repurposed as a content taxonomy, git-as-changelog, squash-merge model. The prose is high quality and internally consistent, and the frontmatter-scoping in the second commit (anchoring all `version`/`version-hash` parsing to the frontmatter block so a YAML example in a QEP *body* is never mistaken for the real field) is correct and tested. `check.mjs` passes locally and on CI. - -There are a few real issues — one a **silent failure this very PR can trigger on merge**. Address **#1, #2, #7** before merging; the rest is low-priority polish. - -### Priority order - -1. **#1** — make stamping robust to non-squash merges *or* enable squash-only before merge (silent failure otherwise). -2. **#2** — close the version-deletion gap in `check.mjs` (Copilot's comment; valid). -3. **#7** — reconcile QEP-1's rendering claims with the configured theme. -4. Optional polish: **#3, #4, #5, #6, #8**. -5. Prose/process: **#9, #10**. - ---- - -## Correctness / CI - -### 1. [High] Stamping silently no-ops on a non-squash merge — and squash-only isn't enabled yet - -`stamp.mjs` derives its changed-file set from `git diff-tree --no-commit-id --name-only -r HEAD` ([.github/scripts/stamp.mjs:20-21](.github/scripts/stamp.mjs#L20-L21)). Confirmed by reproduction: for a **two-parent merge commit this prints nothing** (git suppresses merge diffs without `-m`/`--cc`); for a squash commit it lists files correctly. So the stamp step depends entirely on squash-merge — but enabling squash-only is the PR's own "one manual step," which *cannot* be applied by this PR. If QEP-1 (or any QEP PR before the setting lands) is merged as a merge commit, `changed` is empty, **no `version-hash` is stamped, and there is no error** — the bootstrap example silently fails to produce the artifact it demonstrates. - -**Fix (do both):** -- Apply the squash-only repo setting *before* this merges (Settings → General → Pull Requests). -- Harden `stamp.mjs` to detect a multi-parent `HEAD` and either diff against the first parent (`git diff --name-only HEAD^1 HEAD`) or fail loudly rather than no-op. - -- [ ] Addressed - -### 2. [Medium] A versioned QEP can silently drop back to v0 (Copilot comment — valid) - -`check.mjs` skips the base comparison entirely when `version` is absent: `if (version === undefined) continue;` ([.github/scripts/check.mjs:32](.github/scripts/check.mjs#L32)). A v1+ QEP that deletes its `version:` field passes CI, breaking the "once versioned, stays versioned" guarantee and desyncing the README. - -**Fix:** when the current `version` is undefined, still read `baseVersion(path)` and error if the base had a version. Sketch: - -```js -for (const path of qepFiles()) { - const { version } = parseQep(path); - const prev = baseVersion(path); - if (version === undefined) { - if (prev !== undefined) - errors.push(`${path}: version ${prev} was removed; once versioned a QEP stays versioned`); - continue; - } - if (prev === undefined) { - if (version !== 1) errors.push(`${path}: introduces version ${version}; a first version must be 1`); - } else if (version !== prev && version !== prev + 1) { - errors.push(`${path}: version ${prev} -> ${version} must stay equal (editorial) or increment by one (substantive)`); - } -} -``` - -(There's a `copilot-review` skill available to formally reply to and resolve Copilot's inline comment once fixed.) - -- [ ] Addressed - -### 3. [Low] No enum validation of `type` / `status` - -CI checks README↔frontmatter *parity* but never that `type ∈ {standard, process, informational}` or that `status` is a known value. `type: standardd` passes as long as the README matches the typo. Cheap hardening if wanted. - -- [ ] Addressed / [ ] Won't fix - -### 4. [Low] In-document header table is not parity-checked - -CI enforces README↔frontmatter parity, but the in-document header table (`| **Version** | 1 |`, `| **Type** | … |`) is hand-maintained and unchecked. For a document now meant to be amended frequently, that duplicated Type/Status/Version is exactly where drift will accumulate. [AGENTS.md:60-63](AGENTS.md#L60-L63) tells authors to bump it "in both the frontmatter and the header table," but nothing verifies it. Consider checking it, or at least flag it as the known manual-sync point. - -- [ ] Addressed / [ ] Won't fix - -### 5. [Low] A brand-new QEP can be born at v1 - -The "first version must be 1" branch ([.github/scripts/check.mjs:34-36](.github/scripts/check.mjs#L34-L36)) accepts a *new* file introduced at `version: 1`, contradicting "a new QEP is unversioned (v0)." Easy to tighten so new files must be v0. - -- [ ] Addressed / [ ] Won't fix - -### 6. [Nit] Minor robustness - -- `stamp.mjs`'s README-sync loop is effectively redundant given `check.mjs` already enforces parity pre-merge — fine as defense-in-depth, just not load-bearing. -- The final `git push` in [.github/workflows/stamp-version.yml](.github/workflows/stamp-version.yml) relies on checkout leaving you on `main`; `git push origin HEAD:main` is more explicit. - -- [ ] Addressed / [ ] Won't fix - ---- - -## Consistency - -### 7. [Medium] QEP-1 describes site rendering the repo doesn't currently produce - -[myst.yml:14](myst.yml#L14) sets `template: book-theme`, not the QuantEcon theme. But [qeps/qep-0001-purpose-and-process.md:186-203](qeps/qep-0001-purpose-and-process.md#L186-L203) asserts, in present tense, "the QuantEcon theme's git-history control shows *Last changed*…" and "a coloured `type` pill always, and a `version` pill once a QEP reaches v1." The history dropdown is a QuantEcon-theme feature (cited as the still-open `quantecon-theme.mystmd#83`), and the pills are theme-specific — under `book-theme` none of this renders. The README *columns* claim is accurate (repo-controlled); the *pills/dropdown* claims are not yet true. - -**Fix:** either switch the theme, or soften to contingent language ("once the QuantEcon theme is adopted…"). - -- [ ] Addressed - -### 8. [Low] en-dash footgun - -`versionCell` emits U+2013 EN DASH `–` for a v0 row ([.github/scripts/qeps.mjs:70](.github/scripts/qeps.mjs#L70)). The committed docs use that exact codepoint consistently (good), but a future author who hand-types an ASCII hyphen `-` in a new QEP's Version cell hits a confusing parity failure *pre*-merge — and `stamp.mjs` only fixes it *post*-merge, so it blocks the PR. Either make the character copy-pasteable in the template/AGENTS, or relax the v0 check to accept `-`/`–`/empty. - -- [ ] Addressed / [ ] Won't fix - ---- - -## Process & prose - -### 9. Draft status & @jstac's feedback - -The PR is OPEN but flagged as "supposed to be in DRAFT." @jstac approved the principle while calling the "What this does" section "word salad" — he wants a crisp sequence of rules, "like an algorithm / proof." That applies to the PR body, but the QEP's *Versioning*/*Amending* sections are also dense and could be tightened into the same rule-first shape. Convert to draft until the language pass + automation fixes land. - -- [ ] Addressed - -### 10. Two commits, squash model - -The PR has two commits; the model is "one amendment = one commit," true only if squash-merged — which loops back to **#1**. Make sure this one is squashed. - -- [ ] Addressed - ---- - -## What was verified during review - -- Read all 9 changed files, the three CI scripts, both new workflows, and the existing `deploy.yml`. -- Cross-checked the implementation against the agreed design in issue #4 (faithful). -- Ran `check.mjs` locally against `main` — passes. -- Reproduced `git diff-tree` behaviour on squash vs. true merge commits (basis for #1). -- Confirmed dash codepoints: `versionCell` and the committed docs both use U+2013 (basis for #8). -- Confirmed `myst.yml` uses `book-theme`, not the QuantEcon theme (basis for #7). -- Reviewed the Copilot bot's one inline comment — valid (#2). From 516a29cde2403bbb1d1bcb55336213231c3677f3 Mon Sep 17 00:00:00 2001 From: Matt McKay <mmcky@users.noreply.github.com> Date: Thu, 25 Jun 2026 11:45:44 +1000 Subject: [PATCH 6/6] QEP-1 v1: enforce presence of the README Status column in check.mjs check.mjs guarded the presence of the Type and Version columns but not Status, so a removed/renamed Status header would make the status-parity check silently no-op (expect() early-returns when the column index is -1). Add the matching presence check (Copilot review comment). Verified with a scenario test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --- .github/scripts/check.mjs | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/scripts/check.mjs b/.github/scripts/check.mjs index cb9dd91..e94f3e4 100644 --- a/.github/scripts/check.mjs +++ b/.github/scripts/check.mjs @@ -64,6 +64,7 @@ for (const path of qepFiles()) { // 2/3. type/status enums, and README Type/Status/Version parity with frontmatter. 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()) {