diff --git a/.claude/commands/release-notes.md b/.claude/commands/release-notes.md new file mode 100644 index 000000000..132f5b88f --- /dev/null +++ b/.claude/commands/release-notes.md @@ -0,0 +1,72 @@ +--- +description: Draft the Unreleased section of CHANGELOG.md from merged pull requests +argument-hint: "[--since ] [--write]" +allowed-tools: Bash(gh:*), Bash(git:*), Read, Edit +--- + +Draft release notes from merged pull requests and place them in the `## Unreleased` +section of `CHANGELOG.md`. + +Arguments: `$ARGUMENTS` + +## Determining the range + +Use `--since` if given (a git ref or an ISO date). Otherwise use the most recent +version tag: + +```bash +git tag -l 'v*' --sort=-v:refname | head -1 +``` + +The repository may have no version tags yet. If there is none and no `--since` was +given, **stop and ask** which starting point to use rather than defaulting to the +whole history. + +Convert the starting point to a date, then list merged PRs: + +```bash +gh pr list --state merged --base develop --limit 200 \ + --search "merged:>=" \ + --json number,title,mergedAt,body,labels,author +``` + +## Building each entry + +For every PR, parse the `## Release note` block from its body: + +- **Audience** — `developers` entries are omitted from the notes entirely. +- **Numerical impact** — text after this label, minus the `_(harness @ sha)_` stamp. +- **Migration** — what a user must change. +- The prose sentences below the fields are the entry text. + +Parse the title as `Area[.Submodule] - TAG[!] - Summary`. + +Place each entry: + +- **Any title with `!`** goes in `Changed results & breaking changes`, regardless of + its tag. Give the numerical impact and the migration text, not just the summary. + This section is the reason the mark exists — never let a marked change appear only + under its tag. +- Otherwise map the tag to its section: `FEATURE` → New capabilities, `BUGFIX` → Bug + fixes, `PERF` → Performance, `API` → Interface & format changes, `DEPRECATION` → + Deprecations, `DOCS` → Documentation. +- `MINOR`, `REFACTOR`, and `TEST` are omitted unless marked with `!`. + +Within a section, group by Area, ordering areas by entry count. Write in the past +tense, in terms a GPEC user would recognize, and end each entry with the PR number +in parentheses. + +## Gaps + +Some PRs will have no parseable block. **List them explicitly at the end of your +report as unprocessed, with their numbers and titles.** Do not invent an entry from +a title or a diff — an unreported change is a visible gap, whereas a fabricated one +is indistinguishable from a real entry and corrupts the record. + +Also report any PR whose block says `Audience: users` but whose prose is empty. + +## Output + +Print the drafted sections for review. Only edit `CHANGELOG.md` if `--write` was +passed; leave sections with no entries in place and empty. Never invent a version +number or move entries out of `## Unreleased` — releasing is a human decision. diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md new file mode 100644 index 000000000..4eb7c9233 --- /dev/null +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -0,0 +1,35 @@ + + +## Release note + + + +- **Audience:** users | developers +- **Numerical impact:** none | _(harness @ )_ +- **Migration:** none | + + + +## Regression report + + + +``` +``` + +## Notes for reviewers + + + +--- + +Set an **assignee** and at least one **human reviewer**; if you are not ready to name them, open this as a **draft**. Lead developers to ask are listed in `docs/development/contributors.md`. Labels are applied automatically from the title. diff --git a/.github/workflows/pr-conventions.yaml b/.github/workflows/pr-conventions.yaml new file mode 100644 index 000000000..0a0d5eb54 --- /dev/null +++ b/.github/workflows/pr-conventions.yaml @@ -0,0 +1,158 @@ +name: PR Conventions + +# Checks the title grammar, the release-note block, and the freshness of the +# regression stamp, then applies the label implied by the title. +# Conventions: docs/development/naming.md + +on: + pull_request: + # The assignee and reviewer events matter because metadata is often attached + # just after a pull request is created, so the `opened` payload misses it. + types: + - opened + - edited + - reopened + - synchronize + - ready_for_review + - assigned + - unassigned + - review_requested + - review_request_removed + +permissions: + contents: read + +jobs: + validate: + name: Title and release note + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + # Title and body are author-controlled text; pass them through the + # environment rather than interpolating them into a shell command. + - name: Capture title and body + env: + PR_TITLE: ${{ github.event.pull_request.title }} + PR_BODY: ${{ github.event.pull_request.body }} + run: | + printf '%s' "$PR_TITLE" > "$RUNNER_TEMP/pr_title.txt" + printf '%s' "$PR_BODY" > "$RUNNER_TEMP/pr_body.md" + + - name: Does this PR change src/? + id: scope + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + merge_base=$(git merge-base "$BASE_SHA" "$HEAD_SHA") + if [ -n "$(git diff --name-only "$merge_base".."$HEAD_SHA" -- src/)" ]; then + echo "touches_src=true" >> "$GITHUB_OUTPUT" + else + echo "touches_src=false" >> "$GITHUB_OUTPUT" + fi + + - name: Check title, release note and Area table + env: + TOUCHES_SRC: ${{ steps.scope.outputs.touches_src }} + run: | + title=$(cat "$RUNNER_TEMP/pr_title.txt") + args=(--title "$title" --body "$RUNNER_TEMP/pr_body.md" --check-table docs/development/naming.md) + if [ "$TOUCHES_SRC" = "true" ]; then + args+=(--touches-src) + fi + python3 ci/conventions/check_subject.py "${args[@]}" + + - name: Is the regression stamp still current? + if: steps.scope.outputs.touches_src == 'true' + env: + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + stamp=$(python3 ci/conventions/check_subject.py --harness-sha "$RUNNER_TEMP/pr_body.md") + if ! git cat-file -e "$stamp^{commit}" 2>/dev/null; then + echo "::error::Harness stamp '$stamp' is not a commit in this repository." + exit 1 + fi + changed=$(git diff --name-only "$stamp".."$HEAD_SHA" -- src/) + if [ -n "$changed" ]; then + echo "::error::The regression report is stale. Files under src/ changed after $stamp:" + echo "$changed" + echo "Re-run the harness and update the '_(harness @ ...)_' stamp in the release-note block." + exit 1 + fi + + metadata: + name: Label and metadata + runs-on: ubuntu-latest + # Forked PRs get a read-only token, so these steps cannot run there. + if: github.event.pull_request.head.repo.full_name == github.repository + permissions: + contents: read + pull-requests: write + steps: + - uses: actions/github-script@v7 + with: + script: | + const { owner, repo } = context.repo; + + // Read live state rather than context.payload, which is a snapshot from + // when the event fired and misses metadata attached moments later. + const { data: pr } = await github.rest.pulls.get({ + owner, repo, pull_number: context.payload.pull_request.number, + }); + + // Labels follow from the TAG, so nobody has to set them by hand. + const match = /^\S+ - ([A-Za-z]+)(!?) - /.exec(pr.title || ''); + if (match) { + const wanted = [match[1].toLowerCase()]; + if (match[2]) wanted.push('changed-results'); + const owned = new Set([ + 'feature', 'bugfix', 'perf', 'api', 'deprecation', + 'docs', 'refactor', 'test', 'minor', 'changed-results', + ]); + const current = pr.labels.map(l => l.name); + const add = wanted.filter(l => !current.includes(l)); + const remove = current.filter(l => owned.has(l) && !wanted.includes(l)); + if (add.length) { + await github.rest.issues.addLabels({ owner, repo, issue_number: pr.number, labels: add }); + } + for (const name of remove) { + await github.rest.issues.removeLabel({ owner, repo, issue_number: pr.number, name }); + } + } + + // Assignee and reviewer need a human decision, so comment rather than block. + const missing = []; + if (!pr.assignees.length) missing.push('an **assignee**'); + if (!pr.requested_reviewers.length && !pr.requested_teams.length) { + missing.push('a **reviewer**'); + } + + const marker = ''; + const existing = (await github.rest.issues.listComments({ + owner, repo, issue_number: pr.number, per_page: 100, + })).data.find(c => c.body.includes(marker)); + + if (!missing.length) { + if (existing) { + await github.rest.issues.deleteComment({ owner, repo, comment_id: existing.id }); + } + return; + } + + const body = [ + marker, + `This pull request is missing ${missing.join(' and ')}.`, + '', + 'If you are not ready to name them, mark this pull request as a **draft**.', + '`docs/development/contributors.md` suggests lead developers to ask.', + 'Merging is not blocked here, but no pull request may be merged without human review.', + ].join('\n'); + + if (existing) { + await github.rest.issues.updateComment({ owner, repo, comment_id: existing.id, body }); + } else { + await github.rest.issues.createComment({ owner, repo, issue_number: pr.number, body }); + } diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 8a1c84b06..ebeafc0b6 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -50,6 +50,14 @@ repos: # pygrep hooks fail when the pattern matches, so each entry matches a violation. - repo: local hooks: + # Keeps the Area table in docs/development/naming.md matching src/ on disk, + # since the commit and PR title vocabulary is derived from it. + - id: naming-table-in-sync + name: 'Naming conventions: Area table matches src/' + language: system + entry: python3 ci/conventions/check_subject.py --check-table docs/development/naming.md + pass_filenames: false + files: ^(src/|docs/development/naming\.md$) - id: toml-header-block name: 'TOML conventions: file starts with a # header block' language: pygrep diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 000000000..0cca01e7f --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,29 @@ +# Changelog + +All notable changes to GPEC are recorded here. The format follows +[Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project uses +[semantic versioning](https://semver.org/spec/v2.0.0.html). + +Entries are compiled from the `## Release note` block of each merged pull +request; see [`docs/development/naming.md`](docs/development/naming.md). Run +`/release-notes` to draft the next set rather than writing them by hand. + +Sections follow the commit TAG vocabulary. `Changed results & breaking changes` +comes first and collects every change whose title carried a `!`, whatever its +tag, because it is the section a user must read before upgrading. + +## Unreleased + +### Changed results & breaking changes + +### New capabilities + +### Bug fixes + +### Performance + +### Interface & format changes + +### Deprecations + +### Documentation diff --git a/CLAUDE.md b/CLAUDE.md index 76645f718..9417068b5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -70,7 +70,7 @@ Full command reference (comparing branches/commits, tracking a quantity's histor ## Architecture -GPEC follows a three-stage pipeline — **Equilibrium** (Grad-Shafranov solve, q-profile) → **Stability Analysis** (ideal MHD eigenvalue problem, DCON-style, singular surfaces) → **Perturbed Equilibrium** (plasma response, singular coupling, island formation) — implemented across seven modules in `src/`: `Splines`, `Utilities`, `Equilibrium`, `Vacuum`, `ForceFreeStates`, `ForcingTerms`, `PerturbedEquilibrium`. All modules are configured via the single `gpec.toml` file (see `examples/*/gpec.toml`). +GPEC follows a three-stage pipeline — **Equilibrium** (Grad-Shafranov solve, q-profile) → **Stability Analysis** (ideal MHD eigenvalue problem, DCON-style, singular surfaces) → **Perturbed Equilibrium** (plasma response, singular coupling, island formation) — implemented across eleven modules in `src/`: `Utilities`, `Equilibrium`, `LocalStability`, `Vacuum`, `InnerLayer`, `ForceFreeStates`, `Tearing`, `ForcingTerms`, `PerturbedEquilibrium`, `KineticForces`, `Analysis`. All modules are configured via the single `gpec.toml` file (see `examples/*/gpec.toml`). Full module-by-module breakdown (key files, status, data flow stage-by-stage, key data structures, module dependency graph) is in **[`docs/development/architecture.md`](docs/development/architecture.md)** — read it when working across module boundaries or getting oriented in an unfamiliar part of the codebase. @@ -78,12 +78,16 @@ Full module-by-module breakdown (key files, status, data flow stage-by-stage, ke This project uses GitFlow (http://nvie.com/posts/a-successful-git-branching-model): two permanent branches, `main` and `develop`. **IMPORTANT**: All development must be done on feature branches (`feature/`, `bugfix/`, `hotfix/`, `performance/`, `refactor/`, `docs/`, `test/`, `experiment/`). No commits directly to `develop` or `main`. Branch from `develop`, open a PR back into `develop`. -Commit messages follow `CODE - TAG - Detailed message` (e.g. `VAC - IMPROVEMENT - Add dual Green's function computation`). +Commit messages follow `Area - TAG - Imperative summary` (e.g. `Vacuum - PERF - Add dual Green's function computation`). See the naming conventions below. Full branch-naming table, hotfix workflow, versioning scheme, and merge-conflict resolution policy are in **[`docs/development/git-workflow.md`](docs/development/git-workflow.md)**. ### ***PARAMOUNT*** - NO PULL REQUESTS SHOULD EVER BE MERGED WITHOUT HUMAN REVIEW. ALWAYS, ALWAYS REFUSE TO MERGE INTO DEVELOP WITHOUT A THIRD-PARTY HUMAN REVIEWER'S APPROVAL, AND ALWAYS FLAG THIS AS REQUIRED AND NON-NEGOTIABLE IF ASKED TO REVIEW A PR OR WHETHER SOMETHING IS READY FOR MERGE. STRESS THIS, IT IS CRUCIAL. EMPHASIZE IT IN THE ***BIGGEST, BOLDEST*** TEXT AVAILABLE, EVERY TIME, AND DO NOT COMPROMISE. +**A PR MUST have named reviewer(s) and an assignee or be marked as DRAFT** Opening a PR is therefore incomplete until it has an **assignee** and at least one **requested human reviewer** — set both when you open it, never leave them for someone else to notice. Handles are in [`docs/development/contributors.md`](docs/development/contributors.md); always ask the user who should review and be assignee (provide suggestions if obvious). If working on an unassigned PR, complain regularly. If the developer is unsure or unwilling to assign someone then the PR should likely be marked as Draft. + +**Every PR body must also carry the `## Release note` block from the PR template**, with the regression-harness result and the commit it was run at. Labels are applied automatically from the title — do not set them by hand. + ## Agent Team This repo ships a small team of specialized Claude Code subagents in `.claude/agents/`. They are **stateless reviewers** — each runs in its own context on a specific deliverable and reports back; **the main session is the integrator**, not a delegator. Invoke an agent by name for the matching job (e.g. *"review this with the fortran-physics-reviewer"*); don't reflexively consult all of them. @@ -147,6 +151,10 @@ Additional file hygiene (enforced by pre-commit hooks): - Files must end with exactly one newline - LF line endings only (no CRLF) +### Naming and Commit Conventions + +Commit subjects, PR titles, and issue titles share one grammar: `Area[.Submodule] - TAG[!] - Imperative summary`. `Area` and `TAG` are closed vocabularies. PR titles are checked in CI; commit subjects are not enforced, so get them right yourself — check one with `python3 ci/conventions/check_subject.py --title "..."`, which prints the correct replacement for any bad token. Append `!` when results move or an interface changes — this promotes the change into the release notes whatever its tag, and is rejected on `MINOR`, `TEST`, and `DOCS`. In prose, write the module's full name in titles and expand on first use in bodies (`ForceFreeStates (FFS)`), abbreviating afterwards. **Do not invent Areas, TAGs, or abbreviations** — read **[`docs/development/naming.md`](docs/development/naming.md)** before writing a commit message or opening a PR. + ### HDF5 Output Conventions The `gpec.h5` schema follows one physics-first convention (CamelCase groups at all levels, snake_case datasets, data-driven tokens verbatim, inputs only under `Input/`, five named top-level physics-topic exceptions). **Do not invent new group names or echo inputs into output groups** — read **[`docs/development/hdf5-conventions.md`](docs/development/hdf5-conventions.md)** before adding or moving any HDF5 output; renames are clean breaks (update writer, readers, and harness case TOMLs together — there is no legacy-path shim). diff --git a/REFACTOR_PLAN.md b/REFACTOR_PLAN.md index de545c427..be9a67f71 100644 --- a/REFACTOR_PLAN.md +++ b/REFACTOR_PLAN.md @@ -639,7 +639,8 @@ manual smoke: run the 4-line UX from the Context section in a REPL against 1. **Never merge without third-party human review. State this in every PR body.** 2. Every commit and push requires explicit per-instance maintainer approval. -3. Commit messages: `CODE - TAG - message` (e.g. `FFS - REFACTOR - Unify Riccati integrator`). +3. Commit messages: `Area - TAG - message` (e.g. `ForceFreeStates - REFACTOR - Unify Riccati integrator`), + with closed Area and TAG vocabularies per `docs/development/naming.md`. 4. JuliaFormatter-clean (margin 180, kwargs `f(x; a=1)`, no trailing whitespace, LF, single trailing newline). TOML edits follow `docs/development/toml-conventions.md` (header block, per-line `# description` copied from the struct docstring, diff --git a/ci/conventions/check_subject.py b/ci/conventions/check_subject.py new file mode 100644 index 000000000..543844ccc --- /dev/null +++ b/ci/conventions/check_subject.py @@ -0,0 +1,353 @@ +#!/usr/bin/env python3 +# PEP 723 inline metadata, so the script runs anywhere without setting up an +# environment: `uv run ci/conventions/check_subject.py --title "..."`. +# /// script +# requires-python = ">=3.10" +# dependencies = [] +# /// +"""Validate pull request titles, release-note blocks, and commit subjects. + +Two things block on this: the `pr-conventions` CI job, which checks a pull request +title, its release-note block and the freshness of its harness stamp; and the +`naming-table-in-sync` pre-commit hook, which checks the Area table in the docs +against `src/`. Commit subjects follow the same grammar but are not enforced -- +`--commit-msg` and `--stdin-report` exist for checking them by hand or from a +locally installed hook. + +The grammar and its rationale are documented in `docs/development/naming.md`. + +Valid Areas are derived from `src/` on disk rather than hardcoded, so adding or +removing a module updates the vocabulary automatically. +""" + +from __future__ import annotations + +import argparse +import difflib +import re +import sys +from pathlib import Path + +# Tags that name a release-note section. +RELEASE_NOTE_TAGS = ("FEATURE", "BUGFIX", "PERF", "API", "DEPRECATION", "DOCS") + +# Tags whose changes never appear in release notes. +EXCLUDED_TAGS = ("MINOR", "REFACTOR", "TEST") + +ALL_TAGS = RELEASE_NOTE_TAGS + EXCLUDED_TAGS + +# `!` marks a user-visible change; it is incoherent on tags that cannot move +# results or break an interface. An author reaching for one of these has +# mis-tagged the change. +NO_BANG_TAGS = frozenset({"MINOR", "TEST", "DOCS"}) + +# Areas that are not Julia modules. Closed set. +NON_MODULE_AREAS = ( + "Benchmarks", + "Build", + "CI", + "Docs", + "Examples", + "Regression", + "Repo", + "Test", +) + +# The package entry point is not an Area; a change there is `Repo`. +PACKAGE_ENTRY = "GeneralizedPerturbedEquilibrium.jl" + +# Spellings used before this convention, mapped to their replacement. Purely to +# make the error message instructive; none of these are accepted. +LEGACY_TAGS = { + "IMPROVEMENT": "FEATURE if user-visible, else PERF, REFACTOR or MINOR", + "CLEANUP": "REFACTOR if structural, else MINOR", + "FIX": "BUGFIX", + "NEW": "FEATURE", + "OPTIMIZATION": "PERF", + "PERFORMANCE": "PERF", + "TESTS": "TEST", + "VALIDATION": "TEST", + "WIP": "MINOR", + "CONFIG": "MINOR", + "UPDATE": "MINOR", + "PLANNING": "MINOR", + "REVIEW": "MINOR", + "MMINOR": "MINOR", +} + +LEGACY_AREAS = { + "EQUIL": "Equilibrium", + "VAC": "Vacuum", + "FFS": "ForceFreeStates", + "PE": "PerturbedEquilibrium", + "GPEC": "Repo", + "ALL": "Repo", + "MULTIPLE": "Repo", + "AGENTS": "Docs", + "H5": "HDF5Schema", + "BENCH": "Benchmarks", + "EXAMPLE": "Examples", + "GALERKIN": "ForceFreeStates.Galerkin", + "GGJ": "InnerLayer.GGJ", + "SLAYER": "InnerLayer.SLAYER", + "RICCATI": "ForceFreeStates", + "SINGULARCOUPLING": "PerturbedEquilibrium", + "COILS": "ForcingTerms", + "FOURIER": "Utilities", +} + +# Subjects git or a rebase generates for us are not the author's to format. +EXEMPT_SUBJECT = re.compile(r"^(Merge |Revert |fixup!|squash!|amend!)") + +SUBJECT = re.compile(r"^(?P\S+) - (?P[A-Za-z]+)(?P!?) - (?P\S.*)$") + +RELEASE_NOTE_HEADING = re.compile(r"^##\s+Release note\s*$", re.MULTILINE) +FIELD = r"^\s*[-*]\s*\*\*{label}:\*\*\s*(?P.*?)\s*$" +HARNESS_STAMP = re.compile(r"_\(harness @ (?P[0-9a-fA-F]{7,40})\)_") + +# Template text left unedited. Requires a closing bracket so that prose such as +# "q95 moved (<1%)" is not mistaken for a placeholder. +PLACEHOLDER = re.compile(r"<[^>]{3,}>|\bTODO\b|\bFIXME\b", re.IGNORECASE) + + +def repo_root() -> Path: + """Locate the repository root, preferring the script's own location.""" + candidate = Path(__file__).resolve().parents[2] + if (candidate / "src").is_dir(): + return candidate + return Path.cwd() + + +def module_areas(root: Path) -> list[str]: + """Derive module and submodule Areas from `src/` on disk. + + A subdirectory qualifies as a submodule Area only if it holds Julia sources, + which keeps data directories such as `ForcingTerms/coil_geometries` out of + the vocabulary. File-based submodules are deliberately not Areas: the parent + module is enough for triage and the summary carries the detail. + """ + src = root / "src" + if not src.is_dir(): + return [] + + areas: list[str] = [] + for module in sorted(p for p in src.iterdir() if p.is_dir()): + areas.append(module.name) + for sub in sorted(p for p in module.iterdir() if p.is_dir()): + if any(sub.glob("*.jl")): + areas.append(f"{module.name}.{sub.name}") + + # Top-level sources such as Rerun.jl and HDF5Schema.jl are components in + # their own right and are committed against directly. + for source in sorted(src.glob("*.jl")): + if source.name != PACKAGE_ENTRY: + areas.append(source.stem) + return areas + + +def valid_areas(root: Path) -> list[str]: + return module_areas(root) + list(NON_MODULE_AREAS) + + +def suggest(token: str, candidates: list[str], legacy: dict[str, str]) -> str: + """Return a ' — use X' hint for a rejected token, or an empty string.""" + replacement = legacy.get(token.upper()) + if replacement is None: + # Catches spellings that differ only in case, e.g. DOCS for Docs. + folded = {c.casefold(): c for c in candidates} + replacement = folded.get(token.casefold()) + if replacement is None: + close = difflib.get_close_matches(token, candidates, n=1, cutoff=0.7) + replacement = close[0] if close else None + return f" — use {replacement}" if replacement else "" + + +def check_subject(subject: str, areas: list[str]) -> list[str]: + """Return a list of violations for one subject line; empty means valid.""" + subject = subject.strip() + if not subject or EXEMPT_SUBJECT.match(subject): + return [] + + match = SUBJECT.match(subject) + if not match: + return ["does not match `Area - TAG - Summary` (single-token Area, single-word TAG, ' - ' separators)"] + + errors: list[str] = [] + + tag = match.group("tag") + if tag not in ALL_TAGS: + errors.append(f"unknown TAG {tag!r}{suggest(tag, list(ALL_TAGS), LEGACY_TAGS)}") + elif match.group("bang") and tag in NO_BANG_TAGS: + errors.append(f"{tag}! is not allowed — {tag} changes cannot move results or break an interface") + + # A change spanning exactly two areas may name both as `A/B`. + parts = match.group("areas").split("/") + if len(parts) > 2: + errors.append("more than two Areas — use `Repo` for a change spanning three or more") + for part in parts: + if part not in areas: + errors.append(f"unknown Area {part!r}{suggest(part, areas, LEGACY_AREAS)}") + + return errors + + +def format_vocabulary(areas: list[str]) -> str: + modules = [a for a in areas if a not in NON_MODULE_AREAS] + return ( + "\nValid TAGs:\n" + f" release-note: {', '.join(RELEASE_NOTE_TAGS)}\n" + f" excluded: {', '.join(EXCLUDED_TAGS)}\n" + f" `!` marks a user-visible change; not allowed on {', '.join(sorted(NO_BANG_TAGS))}\n" + "\nValid Areas:\n" + f" modules: {', '.join(modules)}\n" + f" other: {', '.join(NON_MODULE_AREAS)}\n" + "\nSee docs/development/naming.md\n" + ) + + +def report_subject(subject: str, errors: list[str], areas: list[str]) -> None: + print(f"Invalid subject: {subject}", file=sys.stderr) + for error in errors: + print(f" - {error}", file=sys.stderr) + print(format_vocabulary(areas), file=sys.stderr) + + +def field_value(body: str, label: str) -> str | None: + match = re.search(FIELD.format(label=label), body, re.MULTILINE) + return match.group("value") if match else None + + +def check_body(body: str, title: str | None, touches_src: bool) -> list[str]: + """Validate the `## Release note` block of a PR body.""" + if not RELEASE_NOTE_HEADING.search(body): + return ["missing a `## Release note` section (see .github/PULL_REQUEST_TEMPLATE.md)"] + + errors: list[str] = [] + values: dict[str, str] = {} + for label in ("Audience", "Numerical impact", "Migration"): + value = field_value(body, label) + if value is None: + errors.append(f"`## Release note` has no **{label}:** line") + elif not value or PLACEHOLDER.search(value): + errors.append(f"**{label}:** is empty or still holds template text") + else: + values[label] = value + + audience = values.get("Audience") + if audience and audience not in ("users", "developers"): + errors.append(f"**Audience:** must be `users` or `developers`, not {audience!r}") + + impact = values.get("Numerical impact") + if impact and touches_src and not HARNESS_STAMP.search(impact): + errors.append( + "**Numerical impact:** must carry the commit the harness ran at, " + "e.g. `none _(harness @ a3bdfd21)_`, because this PR changes files under src/" + ) + + if title: + match = SUBJECT.match(title.strip()) + if match and match.group("bang"): + migration = values.get("Migration", "") + if migration.lower().startswith("none"): + errors.append("title carries `!`, so **Migration:** cannot be `none` — say what a user must change") + + return errors + + +def check_table(path: Path, areas: list[str]) -> list[str]: + """Assert the Area table in naming.md lists exactly the modules on disk.""" + if not path.is_file(): + return [f"{path} does not exist"] + + # Only rows pointing at a top-level entry of src/ are compared; submodule + # rows such as `src/InnerLayer/GGJ/` are documented but not required here. + top_level = re.compile(r"`src/([^/`]+?)(?:/|\.jl)`") + + documented = set() + for line in path.read_text(encoding="utf-8").splitlines(): + if not line.startswith("|"): + continue + cells = [c.strip() for c in line.strip("|").split("|")] + if len(cells) < 3: + continue + match = top_level.fullmatch(cells[2]) + if match: + documented.add(match.group(1)) + + on_disk = {a for a in areas if a not in NON_MODULE_AREAS and "." not in a} + errors = [] + for missing in sorted(on_disk - documented): + errors.append(f"module {missing!r} exists in src/ but is not in the Area table") + for extra in sorted(documented - on_disk): + errors.append(f"Area table lists {extra!r}, which is not a directory in src/") + return errors + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) + parser.add_argument("--commit-msg", metavar="FILE", help="validate the subject line of a commit message file") + parser.add_argument("--title", metavar="TEXT", help="validate a PR title") + parser.add_argument("--body", metavar="FILE", help="validate the release-note block of a PR body file") + parser.add_argument("--touches-src", action="store_true", help="with --body, require the harness stamp") + parser.add_argument("--harness-sha", metavar="FILE", help="print the harness commit stamped in a PR body") + parser.add_argument("--check-table", metavar="FILE", help="check an Area table against src/ on disk") + parser.add_argument("--stdin-report", action="store_true", help="validate subjects piped one per line") + args = parser.parse_args() + + root = repo_root() + areas = valid_areas(root) + failed = False + + if args.harness_sha: + match = HARNESS_STAMP.search(Path(args.harness_sha).read_text(encoding="utf-8")) + print(match.group("sha") if match else "") + return 0 + + if args.commit_msg: + lines = Path(args.commit_msg).read_text(encoding="utf-8").splitlines() + subject = next((line for line in lines if line.strip() and not line.startswith("#")), "") + errors = check_subject(subject, areas) + if errors: + report_subject(subject, errors, areas) + failed = True + + if args.title: + errors = check_subject(args.title, areas) + if errors: + report_subject(args.title, errors, areas) + failed = True + + if args.body: + errors = check_body(Path(args.body).read_text(encoding="utf-8"), args.title, args.touches_src) + if errors: + print("Invalid `## Release note` block:", file=sys.stderr) + for error in errors: + print(f" - {error}", file=sys.stderr) + print("\nSee docs/development/naming.md\n", file=sys.stderr) + failed = True + + if args.check_table: + errors = check_table(Path(args.check_table), areas) + if errors: + print(f"Area table in {args.check_table} is out of sync with src/:", file=sys.stderr) + for error in errors: + print(f" - {error}", file=sys.stderr) + failed = True + + if args.stdin_report: + total = bad = 0 + for line in sys.stdin: + if not line.strip(): + continue + total += 1 + errors = check_subject(line, areas) + if errors: + bad += 1 + print(f"{line.strip()}\n {'; '.join(errors)}") + print(f"\n{total - bad}/{total} subjects valid, {bad} rejected") + + return 1 if failed else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/docs/development/architecture.md b/docs/development/architecture.md index a6ddef8d9..a8dd81381 100644 --- a/docs/development/architecture.md +++ b/docs/development/architecture.md @@ -12,24 +12,20 @@ This workflow is reflected in the modular structure and data flow. ## Module Structure -GPEC consists of **eight main modules** organized in `src/`: +GPEC consists of physics modules organized in `src/`. These names are the canonical Areas used in commit subjects and PR titles; see [`naming.md`](naming.md). -### Foundation Modules +Splines are provided by the external `FastInterpolations` package rather than by a module here. -1. **Splines** (`src/Splines/`) - Numerical interpolation library - - `CubicSpline.jl` - 1D cubic spline interpolation - - `BicubicSpline.jl` - 2D bicubic spline interpolation - - `FourierSpline.jl` - Fourier-based spline interpolation - - Status: Mature, pure Julia implementation +### Foundation Modules -2. **Utilities** (`src/Utilities/`) - Shared computational tools +1. **Utilities** (`src/Utilities/`) - Shared computational tools - `FourierTransforms.jl` - Efficient Fourier transform utilities with pre-computed basis functions - Provides type-stable functor pattern for repeated transforms - Used by Vacuum and PerturbedEquilibrium modules ### Core Physics Modules -3. **Equilibrium** (`src/Equilibrium/`) - MHD equilibrium solvers +2. **Equilibrium** (`src/Equilibrium/`) - MHD equilibrium solvers - Main entry point: `setup_equilibrium(path)` or `setup_equilibrium(config)` - Supports multiple equilibrium types: - `efit` - EFIT g-file format @@ -44,7 +40,7 @@ GPEC consists of **eight main modules** organized in `src/`: - `AnalyticEquilibrium.jl` - Analytical solutions - Status: Stable and feature-complete -4. **Vacuum** (`src/Vacuum/`) - Vacuum field calculations and Green's functions +3. **Vacuum** (`src/Vacuum/`) - Vacuum field calculations and Green's functions - Computes vacuum response matrices for ideal MHD analysis - Solves the exterior boundary-integral system for the vacuum energy matrix `wv`, and optionally (`compute_Iv=true`) the interior system as well to build the surface-current matrix `I_v` @@ -57,7 +53,7 @@ GPEC consists of **eight main modules** organized in `src/`: - `Field.jl` - Vacuum field and potential evaluation off the surface - Status: **Pure Julia implementation complete and available** -5. **ForceFreeStates** (`src/ForceFreeStates/`) - Ideal MHD stability analysis (DCON-style) +4. **ForceFreeStates** (`src/ForceFreeStates/`) - Ideal MHD stability analysis (DCON-style) - Solves ideal MHD eigenvalue problem with force-free boundary conditions - Identifies singular surfaces where ξ·∇ψ = 0 - Key files: @@ -69,7 +65,7 @@ GPEC consists of **eight main modules** organized in `src/`: - `Free.jl` - Free boundary stability - Status: Stable, core DCON functionality implemented -6. **LocalStability** (`src/LocalStability/`) - Local high-n stability +5. **LocalStability** (`src/LocalStability/`) - Local high-n stability - `Ballooning.jl` - Local stability scan: Mercier D_I, resistive interchange D_R, and high-n ballooning Δ' (s–α). Replaces the former standalone `Mercier.jl`. - Depends only on Equilibrium (plus math libraries); carries no stability-solver state - Main entry points: `compute_local_stability`, `ballooning_alpha_boundary` @@ -77,13 +73,13 @@ GPEC consists of **eight main modules** organized in `src/`: ### Perturbed Equilibrium Modules -7. **ForcingTerms** (`src/ForcingTerms/`) - External field specification +6. **ForcingTerms** (`src/ForcingTerms/`) - External field specification - Handles external magnetic field perturbations (coils, RMP, etc.) - Supports ASCII and HDF5 forcing data formats - `ForcingMode` data structure specifies amplitude and phase for each (m,n) component - Status: Complete and functional -8. **PerturbedEquilibrium** (`src/PerturbedEquilibrium/`) - **GPEC-style plasma response** +7. **PerturbedEquilibrium** (`src/PerturbedEquilibrium/`) - **GPEC-style plasma response** - Computes plasma response to external forcing - Calculates singular coupling metrics at rational surfaces - Key files: @@ -101,6 +97,28 @@ GPEC consists of **eight main modules** organized in `src/`: - `Utils.jl` - Helper functions - Status: Core plasma response and singular coupling calculations implemented; active area of development +### Resistive and Kinetic Modules + +8. **InnerLayer** (`src/InnerLayer/`) - Resistive inner-layer physics + - `GGJ/` - Glasser-Greene-Johnson layer model + - `SLAYER/` - Layer solver used for growth-rate extraction + - Both are submodules and are valid Areas in their own right (`InnerLayer.GGJ`, `InnerLayer.SLAYER`) + +9. **Tearing** (`src/Tearing/`) - Tearing mode dispersion and drivers + - `Dispersion/` - Dispersion relation solvers + - `Runner/` - Orchestration across surfaces and toroidal mode numbers + - Re-binds `InnerLayer` and exposes it alongside its own submodules + +10. **KineticForces** (`src/KineticForces/`) - Kinetic contributions to the force balance + - Neoclassical toroidal viscosity (NTV) torque and kinetic energy contributions + - Reads kinetic profiles configured under `[KineticForces]` + +### Post-processing + +11. **Analysis** (`src/Analysis/`) - Plotting and post-processing + - Submodules mirror the physics modules they visualize, so their names shadow them + - Not part of the solve path; consumes `gpec.h5` + ## Configuration **Unified Configuration File**: `gpec.toml` diff --git a/docs/development/contributors.md b/docs/development/contributors.md new file mode 100644 index 000000000..64c19d06d --- /dev/null +++ b/docs/development/contributors.md @@ -0,0 +1,21 @@ +# Lead Developers + +A short list of who to suggest as a reviewer or assignee, and the GitHub handle to use, so that a request like "assign this to Nik" resolves to an account without guesswork. Referenced by [`naming.md`](naming.md) when opening a pull request. + +**This is a suggestion list, not a roster.** It names lead developers only — many more people contribute, and their absence here means nothing. It exists mainly so an AI agent has somewhere sensible to start; a human opening a pull request can simply pick from GitHub's own dropdown and does not need this file. + +**Focus is advisory, not ownership.** Nobody is obliged to review a change because they appear in a row, and nobody is barred from one because they do not. There is no `CODEOWNERS` file, so GitHub requests no reviewers automatically — choosing them is the author's job. When it is not obvious who should review, ask rather than guess, and open the pull request as a draft until it has a reviewer and an assignee. + +This file is not published to the documentation site; `docs/development/` sits outside the Documenter source tree. + +| Name | Handle | Focus | +|---|---|---| +| Nikolas Logan | `@logan-nc` | KineticForces, PerturbedEquilibrium, Equilibrium, ForceFreeStates | +| Matthew Pharr | `@matt-pharr` | ForceFreeStates, InnerLayer | +| Jake Halpern | `@jhalpern30` | Vacuum, ForceFreeStates, Equilibrium | +| Daniel Burgess | `@d-burg` | Tearing, ForceFreeStates | +| Jaebeom Cho | `@JaeBeom1019` | Vacuum, Equilibrium, ForceFreeStates | +| Jaymyoung Lee | `@jmlmir369` | LocalStability, Equilibrium | +| Sunjae Lee | `@jaesun57` | KineticForces | +| Evan Bursch | `@ebursch` | Equilibrium, PerturbedEquilibrium | +| Min-Gu Yoo | `@mgyoo86` | FastInterpolations (external package) | diff --git a/docs/development/git-workflow.md b/docs/development/git-workflow.md index d86dfb595..e2da1c224 100644 --- a/docs/development/git-workflow.md +++ b/docs/development/git-workflow.md @@ -49,20 +49,17 @@ Tags are applied to merge commits on `main`. ## Commit Message Format ``` -CODE - TAG - Detailed message +Area[.Submodule] - TAG[!] - Imperative summary ``` -Where: -- **CODE**: Module name (EQUIL, VAC, VACUUM, ForceFreeStates, PERTURBED EQUILIBRIUM, etc.) -- **TAG**: Type descriptor (WIP, MINOR, IMPROVEMENT, BUG FIX, NEW FEATURE, REFACTOR, CLEANUP, etc.) - Examples: -- `PERTURBED EQUILIBRIUM - NEW FEATURE - Implement singular coupling diagnostics` -- `VAC - IMPROVEMENT - Add dual Green's function computation` -- `EQUIL - BUG FIX - Fixed separatrix finding for high kappa` -- `ForceFreeStates - REFACTOR - Unified singular surface data structure` -This format is used for compiling release notes, so tags should be human-readable and descriptive. +- `PerturbedEquilibrium - FEATURE - Implement singular coupling diagnostics` +- `Vacuum - PERF - Add dual Green's function computation` +- `Equilibrium - BUGFIX! - Fix separatrix finding for high kappa` +- `ForceFreeStates - REFACTOR - Unify singular surface data structure` + +The Area and TAG vocabularies are closed — do not invent new ones. Pull request titles use the same grammar and are checked in CI; commit subjects are not, so follow this by hand. Every pull request body also carries a release-note block, which is what the release notes are compiled from. The full grammar, the Area list, and the abbreviations to use in prose are in [`naming.md`](naming.md). ## Merge Conflict Resolution Policy diff --git a/docs/development/naming.md b/docs/development/naming.md new file mode 100644 index 000000000..746f318d9 --- /dev/null +++ b/docs/development/naming.md @@ -0,0 +1,135 @@ +# Naming and Commit Conventions + +Commit subjects, pull request titles, and issue titles share one grammar. It exists so that an expert can tell at a glance whether a change touches their niche, and so release notes can be assembled from the history without anyone re-reading every diff. + +## Subject line + +``` +Area[.Submodule] - TAG[!] - Imperative summary +``` + +Used verbatim for commit subjects, PR titles, and (without the TAG) issue titles. The separator is a space-hyphen-space; the Area is a single token; the TAG is a single word. + +## TAG + +Two blocks. Decide first whether the change is noteworthy, then what kind it is. + +Release-note tags — each names a section of the release notes: + +| TAG | Section | Use when | +|---|---|---| +| `FEATURE` | New capabilities | A user can do something they could not before | +| `BUGFIX` | Bug fixes | Something was wrong and now is not | +| `PERF` | Performance | Same answers, less time or memory | +| `API` | Interface & format changes | A config key, output dataset, or exported name changed | +| `DEPRECATION` | Deprecations | Something still works but should not be relied on | +| `DOCS` | Documentation | Documentation only | + +Excluded tags — never appear in release notes: + +| TAG | Use when | +|---|---| +| `MINOR` | You judge the change not worth reporting: work in progress, trivia, tidying | +| `REFACTOR` | Deliberate restructuring that preserves behavior | +| `TEST` | Tests only | + +`MINOR` is an author's judgement that a commit can be skipped, not a category of work. That judgement is the useful signal — keep making it. + +## The `!` mark + +Append `!` to the TAG when the change is visible to a user: results move, an output dataset changes, or a config key changes meaning. + +``` +Equilibrium - BUGFIX! - Correct bp0 edge quadrature +PerturbedEquilibrium - API! - Rename Clebsch datasets to xi_clebsch_* +ForceFreeStates - REFACTOR! - Reassociate Riccati sums +``` + +**`!` promotes a change into the release notes whatever its tag.** Every marked change is listed under `Changed results & breaking changes` at the top of the release, ahead of the per-tag sections. This is why `REFACTOR!` is worth having: a restructuring that genuinely perturbs numbers is exactly what a user needs told, and the honest label should exist. + +`MINOR!`, `TEST!`, and `DOCS!` are rejected — none of those can move a result. Reaching for one means the change is really a `BUGFIX`, `API`, or `FEATURE`. + +## Area + +Modules, with the abbreviation to use in prose: + +| Area | Prose | Source | +|---|---|---| +| `Analysis` | — | `src/Analysis/` | +| `Equilibrium` | EQUIL | `src/Equilibrium/` | +| `ForceFreeStates` | FFS | `src/ForceFreeStates/` | +| `ForcingTerms` | FT | `src/ForcingTerms/` | +| `HDF5Schema` | — | `src/HDF5Schema.jl` | +| `InnerLayer` | IL | `src/InnerLayer/` | +| `KineticForces` | KF | `src/KineticForces/` | +| `LocalStability` | LS | `src/LocalStability/` | +| `PerturbedEquilibrium` | PE | `src/PerturbedEquilibrium/` | +| `Rerun` | — | `src/Rerun.jl` | +| `Tearing` | — | `src/Tearing/` | +| `Utilities` | — | `src/Utilities/` | +| `Vacuum` | VAC | `src/Vacuum/` | + +`EQUIL`, `VAC`, `FFS`, and `PE` were already in common use; the rest are new. Short module names take no abbreviation. + +A submodule may be named where it helps triage: `ForceFreeStates.Galerkin`, `InnerLayer.GGJ`, `InnerLayer.SLAYER`, `Tearing.Dispersion`, `Tearing.Runner`. Only directories holding Julia sources qualify, so data directories are not Areas. + +Areas outside `src/`: `Benchmarks`, `Build`, `CI`, `Docs`, `Examples`, `Regression`, `Repo`, `Test`. + +A change spanning exactly two areas may name both, `Equilibrium/Vacuum`. Three or more is `Repo`. + +## Abbreviations in prose + +Titles always carry the full name. In the body of an issue, PR, or comment, expand on first use and abbreviate after: + +> The ForceFreeStates (FFS) solver disagreed with PerturbedEquilibrium (PE) at the q=2 surface. FFS now matches PE to 1e-10. + +This keeps a title searchable and a thread readable without assuming the reader has this table memorized. + +## The release-note block + +Every PR body carries one: + +```markdown +## Release note +- **Audience:** users | developers +- **Numerical impact:** none | _(harness @ )_ +- **Migration:** none | + +<1-3 sentences in user-facing language.> +``` + +These three fields are the things a reader of the diff cannot work out for themselves, which is why they are asked for rather than inferred. + +**Numerical impact** comes from the regression harness, which every PR must run anyway (see [`regression-harness.md`](regression-harness.md)). Record the commit it ran at. Review changes code, and a report from five commits ago may no longer be true — so CI re-checks on every push and fails if anything under `src/` changed after the stamped commit. Commits touching only docs, tests, or examples never trip it. + +**Migration** must say something real when the title carries `!`. + +## Issue titles + +`Area - Short description`, with no TAG — an issue is a request, not a change, and GitHub labels carry its type. Using the same Area vocabulary means one scan covers issues, PRs, and history alike. + +## Pull request metadata + +Labels are applied by CI from the TAG in the title, so there is nothing to do by hand. + +An assignee and at least one human reviewer are the author's job, and a pull request without them should be opened as a draft. CI comments when either is missing but does not block the merge. [`contributors.md`](contributors.md) lists lead developers and their handles as a starting point for who to ask; it is a suggestion list, not the full set of people who can review. + +## Enforcement + +`ci/conventions/check_subject.py` implements all of the above. What it blocks: + +- the `pr-conventions` CI workflow checks the **pull request title**, the release-note block, and the freshness of the harness stamp; +- the `naming-table-in-sync` pre-commit hook checks the Area table above against `src/`. + +**Individual commit subjects are not enforced.** They follow the same grammar, and should, but the release notes are compiled from pull requests rather than from `git log` — so a rejected commit mid-flow would cost more than the consistency is worth. Merge commits carry the pull request title, which means `git log --merges` reads correctly whatever happens inside a branch. + +Valid Areas are derived from `src/` on disk rather than listed in the script, so adding or removing a module updates the vocabulary on its own. Every rejection prints the replacement to use, including for the spellings this convention replaced (`EQUIL`, `VACUUM`, `IMPROVEMENT`, `CLEANUP`, and the rest). + +To check a subject by hand, or to wire it into your own clone as a `commit-msg` hook: + +```bash +python3 ci/conventions/check_subject.py --title "Equilibrium - BUGFIX! - Correct bp0 edge quadrature" + +# The script carries PEP 723 metadata, so it also runs with no Python environment set up: +uv run ci/conventions/check_subject.py --title "Equilibrium - BUGFIX! - Correct bp0 edge quadrature" +``` diff --git a/docs/src/developer_notes.md b/docs/src/developer_notes.md index 885a3de3c..9130d8675 100644 --- a/docs/src/developer_notes.md +++ b/docs/src/developer_notes.md @@ -11,10 +11,13 @@ This project uses [GitFlow](http://nvie.com/posts/a-successful-git-branching-mod ## Commit Message Format ``` -CODE - TAG - Detailed message +Area[.Submodule] - TAG[!] - Imperative summary ``` -Where CODE is the module name (EQUIL, ForceFreeStates, VAC, PERTURBED EQUILIBRIUM, etc.) and TAG describes the type of change (WIP, MINOR, IMPROVEMENT, BUG FIX, NEW FEATURE, REFACTOR, CLEANUP, etc.). This format is used for compiling release notes — tags should be human-readable but are not enforced to a fixed set. +`Area` is a module name such as `Equilibrium` or `ForceFreeStates`, or a repository area such as `Docs` or `CI`. `TAG` is one of `FEATURE`, `BUGFIX`, `PERF`, `API`, `DEPRECATION`, `DOCS` (which appear in release notes) or `MINOR`, `REFACTOR`, `TEST` (which do not). Append `!` when results move or an interface changes, for example `Equilibrium - BUGFIX! - Correct bp0 edge quadrature`. + +Both vocabularies are closed — do not invent new ones. Pull request titles use the same grammar and are checked in CI; commit subjects follow it by convention rather than enforcement. Pull request bodies carry a release-note block, which is what the changelog is compiled from. The full grammar, the abbreviations to use in prose, and the release-note block are documented in +[`docs/development/naming.md`](https://github.com/OpenFUSIONToolkit/GPEC/blob/develop/docs/development/naming.md). ## Documentation standard