From 1c3d66976804cb4271c3476f7a5d5d7d61a76df3 Mon Sep 17 00:00:00 2001 From: gouyt13clear Date: Fri, 23 Oct 2026 13:19:31 +0800 Subject: [PATCH] ci: classify pull requests for release notes --- .github/CONTRIBUTING.md | 4 + .github/pull_request_template.md | 4 + .github/release.yml | 19 ++++ .github/scripts/ci_changes.py | 3 + .github/scripts/test_pr_labels.py | 172 ++++++++++++++++++++++++++++++ .github/workflows/pr-labels.yml | 65 +++++++++++ docs/maintaining.md | 59 +++++++--- 7 files changed, 313 insertions(+), 13 deletions(-) create mode 100644 .github/scripts/test_pr_labels.py create mode 100644 .github/workflows/pr-labels.yml diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 3b6b6079..d8d9ca5e 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -142,6 +142,10 @@ In your PR, describe: - The commands you ran and their results, including failures or unavailable checks. - Any compatibility implications or performance measurements relevant to the change. +Use a title such as `fix(ivf): correct append metadata` or +`docs: clarify installation`. The prefix automatically selects the release-note +category; see the [title and label mapping](../docs/maintaining.md#pull-request-labels-and-release-notes). + **CI and merge requirements:** CI selects jobs from the changed files and may run broader checks than your focused local tests. Core library and binding changes require the full [static analysis check](../docs/development.md#static-analysis) before merging. diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 1b332b1a..3f60cdc2 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,5 +1,9 @@ ## Summary + + diff --git a/.github/release.yml b/.github/release.yml index e5c73cc0..98db5107 100644 --- a/.github/release.yml +++ b/.github/release.yml @@ -7,24 +7,43 @@ changelog: categories: - title: Added labels: + - release:added - enhancement - title: Fixed labels: + - release:fixed - bug + - title: Performance + labels: + - release:performance + - performance + - title: Documentation labels: + - release:documentation - documentation + - title: Build and CI + labels: + - release:build + - build + - ci + - title: Python labels: - python - title: Dependencies labels: + - release:dependencies - dependencies + - title: Maintenance + labels: + - release:maintenance + - title: Other changes labels: - "*" diff --git a/.github/scripts/ci_changes.py b/.github/scripts/ci_changes.py index a4f9ad4b..8bcea765 100644 --- a/.github/scripts/ci_changes.py +++ b/.github/scripts/ci_changes.py @@ -60,6 +60,9 @@ def classify(paths): if path == ".github/workflows/docs.yml": selected.update(("docs", "python_quality")) continue + if path == ".github/workflows/pr-labels.yml": + selected.add("python_quality") + continue if path.startswith(".github/ISSUE_TEMPLATE/") or path in ( ".github/release.yml", ".github/CODEOWNERS", diff --git a/.github/scripts/test_pr_labels.py b/.github/scripts/test_pr_labels.py new file mode 100644 index 00000000..08a3e266 --- /dev/null +++ b/.github/scripts/test_pr_labels.py @@ -0,0 +1,172 @@ +"""Execute the metadata-only PR classifier with mocked GitHub API calls.""" + +import json +import subprocess +import unittest +from pathlib import Path + +from ci_changes import classify + +ROOT = Path(__file__).resolve().parents[2] +WORKFLOW = ROOT / ".github/workflows/pr-labels.yml" +SCRIPT = "\n".join( + line[12:] + for line in WORKFLOW.read_text().split(" script: |\n", 1)[1].splitlines() +) + +HARNESS = r""" +const fs = require('fs'); +const input = JSON.parse(fs.readFileSync(0, 'utf8')); +const calls = []; +const warnings = []; +let lookups = 0; +const issues = { + listLabelsOnIssue: () => {}, + getLabel: async args => { + calls.push(['get', args.name]); + const status = (input.lookupErrors || [])[lookups++]; + if (status) throw { status }; + }, + createLabel: async args => { + calls.push(['create', args.name]); + if (input.createError) throw { status: input.createError }; + }, + addLabels: async args => { calls.push(['add', args.labels]); }, + removeLabel: async args => { calls.push(['remove', args.name]); }, +}; +const github = { + rest: { + issues, + pulls: { get: async () => ({ data: { title: input.title } }) }, + }, + paginate: async () => (input.labels || []).map(name => ({ name })), +}; +const context = { + repo: { owner: 'owner', repo: 'repo' }, + payload: { pull_request: { number: 42, title: 'stale event title' } }, +}; +const core = { warning: message => warnings.push(message) }; +const AsyncFunction = Object.getPrototypeOf(async function() {}).constructor; +new AsyncFunction('github', 'context', 'core', input.script)(github, context, core) + .then(() => process.stdout.write(JSON.stringify({ calls, warnings }))) + .catch(error => { + process.stdout.write(JSON.stringify({ calls, warnings, error: error.status })); + }); +""" + + +def run_classifier(title, **state): + result = subprocess.run( + ["node", "-e", HARNESS], + input=json.dumps(dict(script=SCRIPT, title=title, **state)), + text=True, + capture_output=True, + check=True, + ) + return json.loads(result.stdout) + + +class PrLabelsTest(unittest.TestCase): + def test_prefixes_scopes_and_breaking_markers(self): + categories = { + "feat": "added", + "fix": "fixed", + "perf": "performance", + "docs": "documentation", + "ci": "build", + "build": "build", + "deps": "dependencies", + "chore": "maintenance", + "refactor": "maintenance", + "test": "maintenance", + "style": "maintenance", + } + release_config = (ROOT / ".github/release.yml").read_text() + for prefix, category in categories.items(): + for suffix in ("", "(ivf)", "!", "(python)!"): + with self.subTest(prefix=prefix, suffix=suffix): + label = f"release:{category}" + result = run_classifier(f"{prefix}{suffix}: meaningful summary") + self.assertEqual(result["calls"][-1], ["add", [label]]) + self.assertFalse(result["warnings"]) + self.assertIn(f"- {label}\n", release_config) + + def test_edit_adds_new_category_before_removing_old_and_preserves_manual_labels( + self, + ): + result = run_classifier( + "fix(ivf): append metadata", + labels=["release:documentation", "bug", "python", "duplicate", "custom"], + ) + self.assertEqual( + result["calls"], + [ + ["get", "release:fixed"], + ["add", ["release:fixed"]], + ["remove", "release:documentation"], + ], + ) + + def test_repeated_event_is_idempotent(self): + result = run_classifier("docs: guide", labels=["release:documentation"]) + self.assertEqual(result["calls"], []) + + def test_unknown_titles_clear_only_managed_labels_and_warn(self): + for title in ( + "Improve IVF", + "fix:", + "fix: ", + "prefix fix: title", + "fix(): title", + ): + with self.subTest(title=title): + result = run_classifier( + title, labels=["release:fixed", "bug", "release:custom"] + ) + self.assertEqual(result["calls"], [["remove", "release:fixed"]]) + self.assertEqual(len(result["warnings"]), 1) + + def test_title_is_data_and_current_api_title_is_authoritative(self): + result = run_classifier("docs: `$(command)` ${process.exit(1)} '") + self.assertEqual(result["calls"][-1], ["add", ["release:documentation"]]) + + def test_missing_repository_label_is_created(self): + result = run_classifier("perf: reduce scratch", lookupErrors=[404]) + self.assertEqual(result["calls"][1], ["create", "release:performance"]) + self.assertEqual(result["calls"][-1][0], "add") + + def test_label_creation_race_requires_successful_recheck(self): + result = run_classifier("ci: lint", lookupErrors=[404], createError=422) + self.assertNotIn("error", result) + self.assertEqual( + [call[0] for call in result["calls"]], ["get", "create", "get", "add"] + ) + result = run_classifier("ci: lint", lookupErrors=[404, 404], createError=422) + self.assertEqual(result["error"], 404) + self.assertNotIn("add", [call[0] for call in result["calls"]]) + + def test_api_failures_are_not_silenced(self): + for state, status in ( + ({"lookupErrors": [403]}, 403), + ({"lookupErrors": [404], "createError": 500}, 500), + ): + with self.subTest(state=state): + result = run_classifier( + "fix: regression", labels=["release:added"], **state + ) + self.assertEqual(result["error"], status) + self.assertNotIn("remove", [call[0] for call in result["calls"]]) + + def test_workflow_uses_metadata_only_and_routes_to_regression_checks(self): + text = WORKFLOW.read_text() + self.assertIn("pull_request_target:", text) + self.assertIn("types: [opened, edited, reopened, ready_for_review]", text) + self.assertNotIn("actions/checkout", text) + self.assertNotIn("${{", SCRIPT) + self.assertEqual( + classify([".github/workflows/pr-labels.yml"]), {"python_quality"} + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/.github/workflows/pr-labels.yml b/.github/workflows/pr-labels.yml new file mode 100644 index 00000000..41415b30 --- /dev/null +++ b/.github/workflows/pr-labels.yml @@ -0,0 +1,65 @@ +name: Classify pull request + +on: + pull_request_target: + types: [opened, edited, reopened, ready_for_review] + +concurrency: + group: pr-labels-${{ github.event.pull_request.number }} + cancel-in-progress: false + +permissions: + pull-requests: read + issues: write + +jobs: + classify: + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + # Metadata only: never check out or execute the PR's code here. + - uses: actions/github-script@v9 + with: + script: | + const repo = context.repo; + const issue_number = context.payload.pull_request.number; + // Fetch current state so queued events cannot restore an old title's label. + const { data: pr } = await github.rest.pulls.get({ ...repo, pull_number: issue_number }); + const categories = { + feat: 'added', fix: 'fixed', perf: 'performance', docs: 'documentation', + ci: 'build', build: 'build', deps: 'dependencies', + chore: 'maintenance', refactor: 'maintenance', test: 'maintenance', + style: 'maintenance', + }; + const managed = new Set(Object.values(categories).map(value => `release:${value}`)); + const match = /^(feat|fix|perf|docs|ci|build|deps|chore|refactor|test|style)(?:\([^()\r\n]+\))?!?:\s+\S/.exec(pr.title); + const target = match ? `release:${categories[match[1]]}` : null; + const labels = await github.paginate(github.rest.issues.listLabelsOnIssue, { + ...repo, issue_number, per_page: 100, + }); + if (target && !labels.some(label => label.name === target)) { + try { + await github.rest.issues.getLabel({ ...repo, name: target }); + } catch (error) { + if (error.status !== 404) throw error; + try { + await github.rest.issues.createLabel({ + ...repo, name: target, color: 'ededed', + description: 'Release category managed from the PR title', + }); + } catch (creationError) { + // Another PR may have created the same repository label. + if (creationError.status !== 422) throw creationError; + await github.rest.issues.getLabel({ ...repo, name: target }); + } + } + await github.rest.issues.addLabels({ ...repo, issue_number, labels: [target] }); + } + for (const label of labels) { + if (managed.has(label.name) && label.name !== target) { + await github.rest.issues.removeLabel({ ...repo, issue_number, name: label.name }); + } + } + if (!target) { + core.warning('Unrecognized PR title prefix; add a release label manually or use the documented title convention.'); + } diff --git a/docs/maintaining.md b/docs/maintaining.md index f5e66441..0fd77d12 100644 --- a/docs/maintaining.md +++ b/docs/maintaining.md @@ -17,6 +17,7 @@ only the affected job groups: | Known root benchmark Python scripts | Python lint and dataset-independent benchmark protocol tests; no index or wheel builds | | `tests/python/test_benchmark_protocol.py` | Python lint and benchmark protocol tests; no index or wheel builds | | `.github/workflows/docs.yml` | Documentation build and Python quality checks for workflow regression tests | +| `.github/workflows/pr-labels.yml` | Python quality, including metadata-classification regression tests | | `sample/run_cpp.sh`, `sample/run_python.sh` | ShellCheck, including these runners | | `tests/core_consumer/`, `tests/fixtures/core_api_v1/` | C++ checks; the frozen v1 fixture is excluded from formatting | | C++ headers or library sources | C++ formatting, clang-tidy, platform tests, sanitizers, consumer build, and Python builds/wheels | @@ -146,16 +147,48 @@ python -m unittest discover -s .github/scripts -p 'test_*.py' ## Pull request labels and release notes -GitHub generates categorized release notes from merged pull requests. Before -merging, maintainers should apply the label that best describes the change: - -| Label | Release note category | -| --- | --- | -| `enhancement` | Added | -| `bug` | Fixed | -| `documentation` | Documentation | -| `python` | Python | -| `dependencies` | Dependencies | - -Pull requests without one of these labels appear under `Other changes`. Pull -requests labeled `duplicate` or `invalid` are omitted from release notes. +GitHub generates categorized release notes from merged pull requests using +[the release configuration](../.github/release.yml). The +[classification workflow](../.github/workflows/pr-labels.yml) reads the current +PR title when a PR is opened, edited, reopened, or marked ready for review: + +| Title prefix | Automatic label | Release note category | +| --- | --- | --- | +| `feat:` | `release:added` | Added | +| `fix:` | `release:fixed` | Fixed | +| `perf:` | `release:performance` | Performance | +| `docs:` | `release:documentation` | Documentation | +| `ci:`, `build:` | `release:build` | Build and CI | +| `deps:` | `release:dependencies` | Dependencies | +| `chore:`, `refactor:`, `test:`, `style:` | `release:maintenance` | Maintenance | + +Use lowercase prefixes and a nonempty summary, for example +`fix(ivf): update only affected clusters`. A scope and a `!` marker before the +colon are optional. The marker does not change the category or authorize a +breaking change; compatibility decisions still follow the project rules. +For dependency-only changes, use `deps:` rather than `chore(deps):`. + +The workflow creates its repository labels as needed and updates only the +listed `release:*` labels. Renaming a PR replaces its old automatic category; +other labels are preserved. Unrecognized titles produce a workflow warning and +clear any previous automatic category. They do not block a contribution. +Before merging, check the title and resulting label, especially for mixed changes. + +Manual labels remain supported: `enhancement`, `bug`, `performance`, +`documentation`, `build`/`ci`, `python`, and `dependencies`. They map to Added, +Fixed, Performance, Documentation, Build and CI, Python, and Dependencies, +respectively. If labels match several categories, the first matching category +in the release configuration wins; keep manual and automatic labels consistent. +PRs without a matching label appear under `Other changes`. Labels `duplicate` +or `invalid` exclude a PR from release notes. + +The classifier uses `pull_request_target` so fork PRs can be labeled. It reads +GitHub metadata only and never checks out or executes PR code. Keep PR titles +as data; never interpolate them into executable workflow expressions. +Local classifier tests execute the workflow's JavaScript with Node.js and mocked +GitHub APIs through the automation unittest command above. + +The workflow becomes active after merging into the default branch. It does not +backfill historical PRs or update published releases. Before a version-bump PR +is merged, review the upcoming PR labels and prepare any user-facing highlights: +automatic categories organize the PR list but do not write a release summary.