diff --git a/.claude/skills/open-pr/SKILL.md b/.claude/skills/open-pr/SKILL.md index 79ee5ea95d..44bb82e000 100644 --- a/.claude/skills/open-pr/SKILL.md +++ b/.claude/skills/open-pr/SKILL.md @@ -77,10 +77,13 @@ A `/verify-*` gate only counts if the **diff's own flow** was driven — rendering a proxy or asserting a 200 is not verification. **Changelog gate:** every non-exempt PR adds `changelog.d/.md` -before the PR opens and touches `CHANGELOG.md` not at all — that shared +before the PR opens and adds nothing to `CHANGELOG.md` — that shared spot is where sibling PRs conflict (rule + exemptions in CLAUDE.md, format in `changelog.d/README.md`; the `/pull_request` command enforces -the same gate and appends the PR number after creation). Locally: +the same gate and appends the PR number after creation). Correcting the +wording of a bullet `[Unreleased]` already carries is allowed — identity +is the bold title, so only a new title is an added bullet — and a +fragment never ships the bare `(#NNNNN)` placeholder. Locally: `uv run python -m tools.changelog check --base origin/main`. Then the local CI equivalents — the same commands the pipeline runs. diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 62626894a1..3c92f111ef 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -8,14 +8,14 @@ A companion guide `CLAUDE.md` at the repo root carries the shared rules for Clau - **Always write in English** - All output text (code comments, commit messages, PR descriptions, issue comments, documentation) must be in English, even if the user writes in another language. - **Repository prose follows the Google developer documentation style guide** - `docs/`, `README.md`, `agentic/docs/`, changelog entries, and PR/issue text use [Google style](https://developers.google.com/style) (sentence-case headings, second person, numbered procedures); the concrete rules and the house-style exception (`docs/reference/style-guide.md` governs website/brand surfaces) live in the `write-docs` skill (`.claude/skills/write-docs/SKILL.md`). Existing docs migrate on touch, not via bulk rewrites. -- **Changelog and releases** - see [Changelog + releases](#changelog--releases) below: every PR adds a fragment under `changelog.d/` and never edits `CHANGELOG.md`, a release folds the fragments and bumps the version files via a PR, and the GitHub release is that section condensed, never copied. When reviewing a non-exempt PR that lacks a fragment — or that writes a bullet into `[Unreleased]` — flag it. +- **Changelog and releases** - see [Changelog + releases](#changelog--releases) below: every PR adds a fragment under `changelog.d/` and adds nothing to `CHANGELOG.md`, a release folds the fragments and bumps the version files via a PR, and the GitHub release is that section condensed, never copied. When reviewing a non-exempt PR that lacks a fragment — or that ADDS a bullet to `[Unreleased]` — flag it; correcting the wording of a bullet already there is allowed and must not be flagged. - **Never echo secret values into transcripts or logs** - Verify secrets by exit code or metadata, never by printing them. - **Structural fix over symptomatic fix** - When a cheap symptomatic fix and a correct structural fix compete, take the structural one: fix the cause, never mute the alarm. Never modify working code to make a broken test pass — fix the test or flag it. - **Never manually merge a pipeline PR, and never bypass the pipeline** - Specifications and implementations go through the GitHub Actions workflows: `spec-create.yml` writes `plots/{spec-id}/specification.md` and `.yaml`, `impl-merge.yml` merges implementation PRs and creates their `metadata/*.yaml`, promotes the preview images to GCS and sets the `impl:{library}:done` label. Merging one by hand skips all of that and leaves `quality_score: null`, no review data, missing GCS images and the issue open. So: never write those files by hand, never run `gh pr merge` on a spec or implementation PR, and add the `approved` label to the **issue**, never to the PR. The full DON'T/DO table is in `CLAUDE.md`. Ordinary (non-pipeline) PRs are merged by the repository owner, not by an agent. ## Changelog + releases -- **Every PR adds `changelog.d/.md`, and NEVER a bullet in `CHANGELOG.md`** — the fragment is a slice of the changelog in the changelog's own format (`### Category` over bold-titled English bullets with PR refs; the rules and an example are in `changelog.d/README.md`). That shared `[Unreleased]` spot is where sibling PRs used to conflict each other — three times in one night on 2026-09-02/03 — and the CI job "Changelog (fragment)" refuses both a missing fragment and a bullet written into `[Unreleased]` directly. Same check locally: `uv run python -m tools.changelog check --base origin/main`. **Exempt:** catalogue-only PRs (everything under `plots/`), the automated plot pipeline (`github-actions[bot]`) and Dependabot — summarized in aggregate at release time (see `agentic/commands/release.md`) — plus any PR labelled `skip-changelog`. This rule is duplicated in `CLAUDE.md` and `agentic/commands/pull_request.md`; keep all three in sync when changing it. +- **Every PR adds `changelog.d/.md`, and NEVER a new bullet in `CHANGELOG.md`** — the fragment is a slice of the changelog in the changelog's own format (`### Category` over bold-titled English bullets with PR refs; the rules and an example are in `changelog.d/README.md`). That shared `[Unreleased]` spot is where sibling PRs used to conflict each other — three times in one night on 2026-09-02/03 — and the CI job "Changelog (fragment)" refuses both a missing fragment and a bullet ADDED to `[Unreleased]` directly — added, not merely different: a bullet is identified by its bold title, so correcting the wording of one already there passes. It also refuses a bare `(#NNNNN)` placeholder: leave the reference out and let `/pull_request` append the real number. Same check locally: `uv run python -m tools.changelog check --base origin/main`. **Exempt:** catalogue-only PRs (everything under `plots/`), the automated plot pipeline (`github-actions[bot]`) and Dependabot — summarized in aggregate at release time (see `agentic/commands/release.md`) — plus any PR labelled `skip-changelog`. This rule is duplicated in `CLAUDE.md` and `agentic/commands/pull_request.md`; keep all three in sync when changing it. - **A release** runs `uv run python -m tools.changelog release X.Y.Z --title ""`, which folds every fragment under a new version heading (`## [X.Y.Z] — YYYY-MM-DD — `), bumps the version files (`pyproject.toml` `project.version`, `uv.lock`, `app/package.json`), repoints the compare links and deletes the fragments. The two aggregate lines for the exempt classes stay by hand (the italic *Catalog* line, the single **Dependencies:** bullet) because they summarize a window rather than a PR. All of it on a `release/vX.Y.Z` PR. Every bullet already carries its PR reference — `/pull_request` appends it when the PR opens. After the merge the tag goes on the merge commit and the GitHub release is created from the section. - **A GitHub release is that section condensed, never copied:** an intro line (merge count, PR range, link to `CHANGELOG.md`); the section's own `### Added / Changed / Removed / Fixed` headings in the section's order (an empty one is omitted); one bullet per NOTABLE entry — chores, dependency bumps and small fixes are left out, no fixed count — each at most two lines: its bold title, one clause with the essence or the headline number, its PR reference; a compare link (`compare/vPREV...vNEW`) as the last line. Numbers are copied exactly; only PR numbers from the section are cited; the full text lives only in the CHANGELOG — the release page is the index into it. The cut procedure itself is `agentic/commands/release.md`. diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 15141d022b..40853a020b 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -18,7 +18,8 @@ Dependabot bumps ignore this template — they are summarized at release time. ## Checklist - [ ] `changelog.d/.md` added (Keep-a-Changelog categories, bold-titled - bullets, PR ref — `changelog.d/README.md`), and `CHANGELOG.md` left alone + bullets, no `(#NNNNN)` placeholder — `changelog.d/README.md`), and no new + bullet in `CHANGELOG.md` (correcting one already there is fine) — required for every non-pipeline PR - [ ] Related documentation updated (e.g. `docs/reference/`, `docs/workflows/`, `docs/contributing.md`) if behavior changed diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c093da458..e1e430bf8c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,11 +5,13 @@ All notable changes to this project are documented here. The format is based on rather than library SemVer: major for milestone releases, minor for feature batches, patch for fix-only (see `agentic/commands/release.md`). -**Do not edit this file in a feature PR.** Every non-exempt PR adds one fragment, +**Do not add to this file in a feature PR.** Every non-exempt PR adds one fragment, `changelog.d/.md`, in this file's own format, each bullet with its PR reference. The shared `[Unreleased]` spot is where sibling PRs used to conflict each other, so the CI job -"Changelog (fragment)" refuses both a missing fragment and a bullet written in here directly -(format and exemptions: `changelog.d/README.md`). A release runs +"Changelog (fragment)" refuses both a missing fragment and a bullet added in here directly +(format and exemptions: `changelog.d/README.md`). Correcting an entry `[Unreleased]` already +carries is allowed — a bullet is identified by its bold title, so a re-worded body is a change, +and only a NEW title (or one more copy of a title already there) is an added bullet. A release runs `uv run python -m tools.changelog release X.Y.Z --title ""`, which folds every fragment under a new version heading (`## [X.Y.Z] — YYYY-MM-DD — `), bumps `pyproject.toml` (`project.version`), `uv.lock` and `app/package.json`, repoints the compare links at the bottom of diff --git a/CLAUDE.md b/CLAUDE.md index d635d991af..a2fd5d4bf0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -17,7 +17,7 @@ A companion guide `.github/copilot-instructions.md` carries the shared rules for - **Always write in English** - All output text (code comments, commit messages, PR descriptions, issue comments, documentation) must be in English, even if the user writes in another language. - **Repository prose follows the Google developer documentation style guide** - `docs/`, `README.md`, `agentic/docs/`, changelog entries, and PR/issue text use [Google style](https://developers.google.com/style) (sentence-case headings, second person, numbered procedures); the concrete rules and the house-style exception (`docs/reference/style-guide.md` governs website/brand surfaces) live in the `write-docs` skill. Existing docs migrate on touch, not via bulk rewrites. - **Update documentation when making changes** - When adding new features, events, or modifying behavior, always check if related documentation needs updating (e.g., `docs/reference/plausible.md` for analytics events, `docs/workflows/` for workflow changes, `docs/contributing.md` for user-facing changes). -- **Changelog and releases** - see [Changelog + releases](#changelog--releases) below: every PR adds a fragment under `changelog.d/` and never edits `CHANGELOG.md`, a release folds the fragments and bumps the version files via a PR, and the GitHub release is that section condensed, never copied. +- **Changelog and releases** - see [Changelog + releases](#changelog--releases) below: every PR adds a fragment under `changelog.d/` and adds nothing to `CHANGELOG.md` (correcting the wording of a bullet already under `[Unreleased]` is allowed), a release folds the fragments and bumps the version files via a PR, and the GitHub release is that section condensed, never copied. - **External-system writes need explicit, named authorization** - Merging or closing PRs/issues this session did not create, bulk merges, and label changes on others' PRs are blocked by the permission classifier unless the user named that action; a generic "ok, sounds good" authorizes nothing. The same discipline covers every prod-touching action in interactive sessions — Cloud SQL writes/DDL, GCS production-folder changes, Secret Manager access, Cloud Build config: name the exact action, resource, and id, and ask before acting (the automated `spec-*`/`impl-*` workflows write these by design and are exempt). For any change to `.claude/settings*.json`, use the built-in `/update-config` skill (a Claude Code harness skill, not a repo command) immediately — direct writes are blocked as self-modification and retrying variants just burns round trips. - **Never echo secret values into the transcript** - Verify secrets by exit code or metadata, never by printing them; never create a Secret Manager version via `echo` (the trailing newline corrupts the value). - **Snapshot before destructive prod operations** - Before anything that can overwrite or delete shared prod DB data or GCS production objects (bulk UPDATE/DELETE, a data-rewriting migration, bulk GCS overwrite/delete): take a timestamped backup first (`pg_dump` to a new directory outside the working tree, `gsutil cp` to a backup prefix), sanity-check it (row/object counts — a silent empty snapshot is worse than none because it looks like safety), and never write into, delete, or rename an existing snapshot. @@ -27,7 +27,7 @@ A companion guide `.github/copilot-instructions.md` carries the shared rules for ## Changelog + releases -- **Every PR adds `changelog.d/.md`, and NEVER a bullet in `CHANGELOG.md`** — the fragment is a slice of the changelog in the changelog's own format (`### Category` over bold-titled English bullets with PR refs; the rules and an example are in `changelog.d/README.md`). That shared `[Unreleased]` spot is where sibling PRs used to conflict each other — three times in one night on 2026-09-02/03 — and the CI job "Changelog (fragment)" refuses both a missing fragment and a bullet written into `[Unreleased]` directly. Same check locally: `uv run python -m tools.changelog check --base origin/main`; `… preview` prints the pending section. **Exempt:** catalogue-only PRs (everything under `plots/`), the automated plot pipeline (`github-actions[bot]`: spec-create, impl-generate/review/repair/merge, spec auto-polish, daily-regen) and Dependabot — those are summarized in aggregate at release time (see `agentic/commands/release.md`) — plus any PR labelled `skip-changelog`. This rule is duplicated in `.github/copilot-instructions.md` and `agentic/commands/pull_request.md`; keep all three in sync when changing it. +- **Every PR adds `changelog.d/.md`, and NEVER a new bullet in `CHANGELOG.md`** — the fragment is a slice of the changelog in the changelog's own format (`### Category` over bold-titled English bullets with PR refs; the rules and an example are in `changelog.d/README.md`). That shared `[Unreleased]` spot is where sibling PRs used to conflict each other — three times in one night on 2026-09-02/03 — and the CI job "Changelog (fragment)" refuses both a missing fragment and a bullet ADDED to `[Unreleased]` directly — added, not merely different: a bullet is identified by its bold title, so correcting the wording of one already there passes. It also refuses a bare `(#NNNNN)` placeholder: leave the reference out and let `/pull_request` append the real number. Same check locally: `uv run python -m tools.changelog check --base origin/main`; `… preview` prints the pending section. **Exempt:** catalogue-only PRs (everything under `plots/`), the automated plot pipeline (`github-actions[bot]`: spec-create, impl-generate/review/repair/merge, spec auto-polish, daily-regen) and Dependabot — those are summarized in aggregate at release time (see `agentic/commands/release.md`) — plus any PR labelled `skip-changelog`. This rule is duplicated in `.github/copilot-instructions.md` and `agentic/commands/pull_request.md`; keep all three in sync when changing it. - **A release** runs `uv run python -m tools.changelog release X.Y.Z --title ""`, which folds every fragment under a new version heading (`## [X.Y.Z] — YYYY-MM-DD — `), bumps the version files (`pyproject.toml` `project.version`, `uv.lock`, `app/package.json`), repoints the compare links and deletes the fragments. The two aggregate lines for the exempt classes stay by hand, because they summarize a window rather than a PR: the italic *Catalog* line and the single **Dependencies:** bullet. All of it on a `release/vX.Y.Z` PR. Every bullet already carries its PR reference — `/pull_request` appends it when the PR opens. After the merge the tag goes on the merge commit and the GitHub release is created from the section. - **A GitHub release is that section condensed, never copied:** an intro line (merge count, PR range, link to `CHANGELOG.md`); the section's own `### Added / Changed / Removed / Fixed` headings in the section's order (an empty one is omitted); one bullet per NOTABLE entry — chores, dependency bumps and small fixes are left out, no fixed count — each at most two lines: its bold title, one clause with the essence or the headline number, its PR reference; a compare link (`compare/vPREV...vNEW`) as the last line. Numbers are copied exactly; only PR numbers from the section are cited; the full text lives only in the CHANGELOG — the release page is the index into it. The cut procedure itself is `agentic/commands/release.md`. diff --git a/agentic/commands/pull_request.md b/agentic/commands/pull_request.md index 073bf15119..63b04eb940 100644 --- a/agentic/commands/pull_request.md +++ b/agentic/commands/pull_request.md @@ -27,9 +27,12 @@ branch state matches what the PR description claims. 1. Run `git diff origin/main...HEAD --stat` to see changed files summary 2. Run `git log origin/main..HEAD --oneline` to see commits in this branch 3. Run `git branch --show-current` to get the current branch name -4. **Changelog gate** — the branch carries `changelog.d/.md` and does NOT touch +4. **Changelog gate** — the branch carries `changelog.d/.md` and does NOT add bullets to `CHANGELOG.md` (Keep-a-Changelog categories, English, bold-titled bullets — the format and an - example are in `changelog.d/README.md`); add and commit the fragment if missing, and run + example are in `changelog.d/README.md`); write the bullets WITHOUT a PR reference (step 7 adds + the real number — a bare `(#NNNNN)` placeholder is refused, and correcting the wording of a + bullet `[Unreleased]` already carries is allowed: identity is the bold title, not the full + text); add and commit the fragment if missing, and run `uv run python -m tools.changelog check --base origin/main`, which is the same check the CI job "Changelog (fragment)" makes. Exempt: catalogue-only PRs (everything under `plots/`), automated pipeline PRs (`github-actions[bot]`: spec-create, impl-*, auto-polish, daily-regen) and @@ -56,8 +59,8 @@ EOF )" ``` -7. If step 4 added changelog entries without a PR reference, append the new PR number to those - bullets (e.g. `(#1234)`) and push the follow-up commit. +7. Append the new PR number to the fragment's bullets (e.g. `(#1234)` — the real number, never + the `(#NNNNN)` placeholder, which the gate refuses) and push the follow-up commit. 8. **The Copilot review runs ONCE**, when the PR opens (or leaves draft) — review-on-push is off in the "Automated Copilot Code Review" ruleset (owner, 2026-09-03), so step 7's follow-up commit and every later push trigger nothing. Do not re-request a review per push; the rule and its one diff --git a/agentic/docs/project-guide.md b/agentic/docs/project-guide.md index c73a889d9f..ca884a89af 100644 --- a/agentic/docs/project-guide.md +++ b/agentic/docs/project-guide.md @@ -1091,16 +1091,20 @@ pytest --pdb # Debug on failure The release notes ARE the changelog — they accumulate PR-by-PR, so cutting a release is pure mechanics (see `agentic/commands/release.md` for the executable flow). -- **`changelog.d/.md`**, one fragment per PR, is what a PR writes — never a bullet in +- **`changelog.d/.md`**, one fragment per PR, is what a PR writes — never a NEW bullet in `CHANGELOG.md` itself, which is the line two sibling PRs then conflict on. A fragment is a slice of the changelog in the changelog's own format ([Keep a Changelog](https://keepachangelog.com/en/1.1.0/) - categories over bold-titled bullets with PR refs; rules and an example in + categories over bold-titled bullets, the PR ref appended by `/pull_request` once the number + exists — a bare `(#NNNNN)` placeholder is refused; rules and an example in `changelog.d/README.md`), and the CI job "Changelog (fragment)" refuses both a missing fragment - and a bullet written into `[Unreleased]`. **Exempt** (would drown the file): catalogue-only PRs - under `plots/`, the automated plot pipeline's output (spec-create, impl-generate/review/repair/ - merge, spec auto-polish, daily-regen PRs) and individual Dependabot bumps — these are summarized - in aggregate at release time (an italic *Catalog* line at the end of the version section and a - single **Dependencies:** bullet under `### Changed`) — plus any PR labelled `skip-changelog`. + and a bullet ADDED to `[Unreleased]`. Added, not merely different: a bullet is identified by its + bold title, so correcting the wording of one already there passes, and only a new title (or one + more copy of a title already there) belongs in a fragment. **Exempt** (would drown the file): + catalogue-only PRs under `plots/`, the automated plot pipeline's output (spec-create, + impl-generate/review/repair/merge, spec auto-polish, daily-regen PRs) and individual Dependabot + bumps — these are summarized in aggregate at release time (an italic *Catalog* line at the end + of the version section and a single **Dependencies:** bullet under `### Changed`) — plus any PR + labelled `skip-changelog`. - **Versioning** is product communication: major for milestone releases (new language waves, rebrands, breaking URL/schema changes — v2.0.0/v3.0.0 precedent), minor for feature batches, patch for fix-only. Version lives in `pyproject.toml`, mirrored in `uv.lock` and diff --git a/changelog.d/README.md b/changelog.d/README.md index 3e312946dd..f2b9c2431e 100644 --- a/changelog.d/README.md +++ b/changelog.d/README.md @@ -3,8 +3,8 @@ Every PR that changes code adds ONE file here instead of editing `CHANGELOG.md`: `changelog.d/.md`, the slug naming the change (the branch name minus its prefix does fine — `origin-gate-rest.md`, `csp-hashes.md`). -Nothing else touches `CHANGELOG.md` between releases, so two PRs never meet at -the same line again — the reason this directory exists (three conflicts in one +Nothing else is ADDED to `CHANGELOG.md` between releases, so two PRs never meet +at the same line again — the reason this directory exists (three conflicts in one night on 2026-09-02/03, each a hand-resolved rebase for text neither branch disagreed about; a union merge driver heals only the local rebase, and GitHub's own mergeability check ignores merge drivers). @@ -16,13 +16,21 @@ A fragment is a slice of the changelog in the changelog's own format: - **The thing, named as the reader will meet it.** One clause on what it does and where, then why it is the right shape — the rationale a diff - cannot carry. Ends with the PR reference once known (#NNNNN). + cannot carry. ### Fixed -- **What was wrong, as a title.** What it did, what it does now (#NNNNN). +- **What was wrong, as a title.** What it did, what it does now. ``` +**Leave the PR reference out; `/pull_request` appends it.** The number does +not exist until the PR does, so write the bullet without one and let step 7 +of `agentic/commands/pull_request.md` add the real `(#11215)` afterwards. What +`check` refuses is the letter N left standing where a number was meant to go: +`(#NNNNN)` shipped as written reads as a reference in the released section and +points nowhere. So: a number, or nothing. (Quoted in backticks it is prose +ABOUT the placeholder — this paragraph included — and passes.) + Rules, all enforced by `uv run python -m tools.changelog check`: - Headings are `### Added` · `### Changed` · `### Deprecated` · `### Removed` @@ -31,7 +39,9 @@ Rules, all enforced by `uv run python -m tools.changelog check`: - A bullet opens with its bold title and wraps with two-space indentation, exactly like the entries already in `CHANGELOG.md`; English (`CLAUDE.md` § "Always write in English"), written like the existing entries: what, - where, why. + where, why. The closing `**` may sit on the continuation line — a long + title is allowed to wrap — but it has to be there. +- No bare `(#NNNNN)`: fill the number in or leave the reference out. - Nothing else in the file — no prose above the first heading, no `##`. The CI job "Changelog (fragment)" requires a fragment in every PR — except @@ -39,8 +49,13 @@ catalogue-only PRs (everything under `plots/`), PRs labelled `skip-changelog`, and the two bot authors: the automated plot pipeline (`github-actions[bot]`: spec-create, impl-generate/review/repair/merge, spec auto-polish, daily-regen) and Dependabot. Those are exactly the classes `CLAUDE.md` already exempts and -the release summarizes in aggregate. The job also refuses bullets written into +the release summarizes in aggregate. The job also refuses bullets ADDED to `[Unreleased]` directly. + +Added, not merely different: a bullet is identified by its bold title, so +correcting the wording of an entry `[Unreleased]` already carries — a typo in a +shipped line, a sharper clause — passes, and only a title the base does not +have counts as a new entry that belongs in a fragment. `uv run python -m tools.changelog check --base origin/main` is the same check locally; `uv run python -m tools.changelog preview` prints the pending section. diff --git a/changelog.d/changelog-changed-bullets.md b/changelog.d/changelog-changed-bullets.md new file mode 100644 index 0000000000..e6c4e5912b --- /dev/null +++ b/changelog.d/changelog-changed-bullets.md @@ -0,0 +1,22 @@ +### Fixed + +- **Correcting a bullet in `[Unreleased]` no longer reads as adding one.** + The fragment gate compared bullet SETS, so re-wording an entry the base + already carried was indistinguishable from writing a new one and got the + same refusal — "gained a bullet, it belongs in a fragment" — for a change + that added nothing. A bullet is now identified by its bold title + (`tools/changelog`, `bullet_title`), collapsed over the line breaks so a + correction may reflow the very line the title runs over, and counted rather + than set-differenced so a second copy of a title cannot slip in behind the + first: a title the base lacks is an ADDED bullet and is still refused, a + title it has is a CHANGED one and passes. + +### Added + +- **`check` refuses a fragment that still says `(#NNNNN)`.** The reference is + appended by `/pull_request` when the PR opens, so a fragment is written + without one — but the placeholder shipped as written reads as a reference in + the released section and points nowhere, and nothing caught it. The + complaint names the file and the line; a placeholder quoted in backticks is + prose about the rule, not a reference, and passes. The README example no + longer hands the writer a placeholder to leave behind. diff --git a/tests/unit/test_agent_instructions.py b/tests/unit/test_agent_instructions.py index 8ff95ce220..ff26816786 100644 --- a/tests/unit/test_agent_instructions.py +++ b/tests/unit/test_agent_instructions.py @@ -267,11 +267,14 @@ def test_every_named_skill_exists() -> None: "docs/reference/style-guide.md", ], # The keywords carry the PROHIBITION as well as the duty: a fragment is - # added AND `CHANGELOG.md` is left alone. A pin on "changelog.d" alone would - # stay green if a guide were rewritten to allow both. - "every PR adds a changelog fragment, never a CHANGELOG.md bullet": [ + # added AND no bullet is added to `CHANGELOG.md`. A pin on "changelog.d" + # alone would stay green if a guide were rewritten to allow both. "new" is + # part of the pin since 2026-09-04, when the gate learned to tell a + # CORRECTED bullet from an added one — the prohibition is on adding, and + # the phrase has to say so. + "every PR adds a changelog fragment, never a new CHANGELOG.md bullet": [ "changelog.d/.md", - "never a bullet in `changelog.md`", + "never a new bullet in `changelog.md`", 'changelog (fragment)"', ], "a release is condensed, never copied": ["condensed, never copied", "agentic/commands/release.md"], diff --git a/tests/unit/tools/test_changelog_tool.py b/tests/unit/tools/test_changelog_tool.py index 2a736e106a..08103ae895 100644 --- a/tests/unit/tools/test_changelog_tool.py +++ b/tests/unit/tools/test_changelog_tool.py @@ -86,6 +86,45 @@ def test_a_malformed_fragment_names_its_line(text: str, complaint: str) -> None: cl.parse_entries(text, where="f.md") +def test_a_bullet_is_identified_by_its_title_whatever_the_line_breaks() -> None: + """The identity is the title's words, so a reflow of the very line it runs over keeps it.""" + assert cl.bullet_title("- **A title that runs onto the\n next line.** Its body.") == ( + "- **A title that runs onto the next line.**" + ) + assert cl.bullet_title("- **A title that runs\n onto the next line.** Its body, corrected.") == ( + "- **A title that runs onto the next line.**" + ) + + +@pytest.mark.parametrize( + "bullet", ["- **A thing.** Done (#NNN).", "- **A thing.** Done (#NNNNN).", "- **A thing.** Done\n (#NNN)."] +) +def test_the_unfilled_placeholder_is_refused(bullet: str) -> None: + with pytest.raises(cl.ChangelogError, match="placeholder"): + cl.check_placeholder(bullet + "\n", where="f.md") + + +@pytest.mark.parametrize( + "bullet", + [ + "- **A thing.** Done (#11215).", + "- **A thing.** Done.", # the reference is appended when the PR opens + "- **A thing.** The `(#NNN)` placeholder, quoted — prose about it, not a reference.", + ], +) +def test_a_number_no_number_and_a_quoted_placeholder_all_pass(bullet: str) -> None: + cl.check_placeholder(bullet + "\n", where="f.md") + + +def test_the_placeholder_complaint_names_the_fragment_file(tmp_path: Path) -> None: + (tmp_path / cl.FRAGMENT_DIR_NAME).mkdir() + (tmp_path / cl.FRAGMENT_DIR_NAME / "topic.md").write_text( + "### Added\n\n- **A thing.** Done (#NNNNN).\n", encoding="utf-8" + ) + with pytest.raises(cl.ChangelogError, match=r"changelog\.d/topic\.md:3: .*placeholder"): + cl.load_fragments(tmp_path) + + def test_merge_puts_fragments_above_the_old_section_in_category_order() -> None: older = cl.Fragment(Path("a.md"), {"Fixed": ["- **old fix.**"], "Added": ["- **old add.**"]}) newer = cl.Fragment(Path("b.md"), {"Added": ["- **new add.**"]}) @@ -278,6 +317,48 @@ def test_a_bullet_written_into_unreleased_directly_is_refused(repo: Path) -> Non assert "Sneaked in" in problem +def test_a_second_bullet_under_a_title_unreleased_already_holds_is_refused(repo: Path) -> None: + """Identity by title has to COUNT: a copy of an entry already there is added too.""" + old = "- **An old-style entry.** Written before the fragments existed (#0).\n" + text = (repo / "CHANGELOG.md").read_text(encoding="utf-8") + text = text.replace(old, old + "- **An old-style entry.** Sneaked in under a title already there.\n") + (repo / "CHANGELOG.md").write_text(text, encoding="utf-8") + (repo / "changelog.d" / "topic.md").write_text(FRAGMENT, encoding="utf-8") + _commit_all(repo, "a second bullet under the same title") + (problem,) = cl.check_pr("main", root=repo) + assert "belongs in a fragment" in problem + assert "Sneaked in" in problem + + +def test_correcting_a_bullet_unreleased_already_carries_passes(repo: Path) -> None: + """A re-worded entry is a CHANGED bullet, not an added one — the gate refuses only new titles.""" + text = (repo / "CHANGELOG.md").read_text(encoding="utf-8") + text = text.replace("Written before the fragments existed (#0).", "Written before they existed, said better (#0).") + (repo / "CHANGELOG.md").write_text(text, encoding="utf-8") + (repo / "changelog.d" / "topic.md").write_text(FRAGMENT, encoding="utf-8") + _commit_all(repo, "correct the wording") + assert cl.check_pr("main", root=repo) == [] + + +def test_a_correction_that_reflows_a_wrapped_title_is_not_a_new_bullet(repo: Path) -> None: + """The one case a line-by-line identity gets wrong: the title itself runs over the wrap.""" + _git(repo, "checkout", "-q", "main") + old = "- **An old-style entry.** Written before the fragments existed (#0)." + wrapped = "- **An old-style entry with a title long enough to run onto the\n next line.** Its body (#0)." + base = (repo / "CHANGELOG.md").read_text(encoding="utf-8").replace(old, wrapped) + (repo / "CHANGELOG.md").write_text(base, encoding="utf-8") + _commit_all(repo, "a wrapped title") + _git(repo, "checkout", "-q", "-b", "reflow") + + reflowed = ( + "- **An old-style entry with a title long enough to run\n onto the next line.** Its body, corrected (#0)." + ) + (repo / "CHANGELOG.md").write_text(base.replace(wrapped, reflowed), encoding="utf-8") + (repo / "changelog.d" / "topic.md").write_text(FRAGMENT, encoding="utf-8") + _commit_all(repo, "reflow the correction") + assert cl.check_pr("main", root=repo) == [] + + def test_the_cut_orders_fragments_by_their_commit_and_passes_the_gate(repo: Path) -> None: """Newest first, an uncommitted fragment newest of all; the cut PR itself deletes fragments and passes.""" _git(repo, "checkout", "-q", "main") # two merged PRs left their fragments on main, in this order diff --git a/tools/changelog/__init__.py b/tools/changelog/__init__.py index 645a4505af..f78bd965ec 100644 --- a/tools/changelog/__init__.py +++ b/tools/changelog/__init__.py @@ -28,9 +28,11 @@ Three verbs (`python -m tools.changelog …`, see `__main__`): -* `check [--base REF]` — every fragment parses; with `--base`, the diff against - REF must carry a fragment (or be a release cut, or catalogue-only) and must - not add bullets to `[Unreleased]` directly. +* `check [--base REF]` — every fragment parses and carries no unfilled `(#NNN)` + placeholder; with `--base`, the diff against REF must carry a fragment (or be + a release cut, or catalogue-only) and must not ADD bullets to `[Unreleased]` + directly (correcting one that is already there is fine — a bullet is + identified by its bold title, not by its full text). * `preview` — the merged `[Unreleased]` as the next cut would write it. * `release VERSION --title …` — the cut itself. """ @@ -39,6 +41,7 @@ import re import subprocess +from collections import Counter from dataclasses import dataclass from pathlib import Path @@ -69,6 +72,10 @@ # The bold title, over the WHOLE bullet: it regularly runs onto the continuation # line before its closing `**`, so DOTALL and the non-greedy body are both load-bearing. _BOLD_TITLE = re.compile(r"- \*\*.+?\*\*", re.DOTALL) +# The unfilled reference placeholder, in any width of N. Quoted in backticks it +# is a fragment TALKING about the placeholder (this rule's own entry does), which +# is prose like any other — only a bare one is a reference nobody filled in. +_PLACEHOLDER = re.compile(r"(? None: return entries +def bullet_title(bullet: str) -> str: + """The bold title, whitespace-collapsed — a bullet's identity across edits. + + Two bullets are the same entry when they open with the same title, whatever + the body behind it now says. That is what lets the gate below tell a + CORRECTED bullet from an ADDED one, and the collapse is what makes it hold + when the correction rewraps the lines the title itself runs over. + """ + match = _BOLD_TITLE.match(bullet) + return " ".join((match.group(0) if match else bullet).split()) + + @dataclass(frozen=True) class Changelog: """`CHANGELOG.md` split at its `[Unreleased]` section.""" @@ -220,6 +239,23 @@ def _added_at(root: Path, path: Path) -> float: return float(stamp) if stamp else float("inf") +def check_placeholder(text: str, *, where: str) -> None: + """Refuse `(#NNN)` left standing where a number was meant to go. + + The reference itself is optional at write time — the number does not exist + until the PR does, and `/pull_request` appends it once it opens. What is + never right is the placeholder shipped as written: it reads as a reference + in the released section and points nowhere. So: a number, or nothing, but + not the letter N. + """ + for n, line in enumerate(text.splitlines(), 1): + if match := _PLACEHOLDER.search(line): + raise ChangelogError( + f"{where}:{n}: '{match.group(0)}' is the unfilled placeholder — put the PR number in, " + "or take the reference out (it is appended when the PR opens)" + ) + + def load_fragments(root: Path = REPO_ROOT) -> list[Fragment]: """Every `changelog.d/*.md` but the README, newest first. @@ -234,9 +270,11 @@ def load_fragments(root: Path = REPO_ROOT) -> list[Fragment]: if path.name == "README.md": continue where = str(path.relative_to(root)) - entries = parse_entries(path.read_text(encoding="utf-8"), where=where) + text = path.read_text(encoding="utf-8") + entries = parse_entries(text, where=where) if not any(entries.values()): raise ChangelogError(f"{where}: no bullets") + check_placeholder(text, where=where) fragments.append(Fragment(path, entries)) return sorted(fragments, key=lambda f: (-_added_at(root, f.path), f.path.name)) @@ -366,8 +404,27 @@ def _changed_files(root: Path, base: str) -> dict[str, str]: return changed -def _bullets(section: str) -> set[str]: - return {b for bullets in parse_entries(section, where=f"{CHANGELOG_NAME} [Unreleased]").values() for b in bullets} +def _bullets(section: str) -> list[str]: + return [b for bullets in parse_entries(section, where=f"{CHANGELOG_NAME} [Unreleased]").values() for b in bullets] + + +def _added_bullets(before: str, after: str) -> list[str]: + """The bullets `after` holds beyond `before`, by the identity `bullet_title` gives them. + + Counted, not set-differenced. Two bullets may legitimately carry the same + title, and a set of titles would let a second copy of one the section + already holds pass unseen — the gate has to notice the COPY as much as the + newcomer (Copilot review). + """ + held = Counter(bullet_title(b) for b in _bullets(before)) + seen: Counter[str] = Counter() + added: list[str] = [] + for bullet in _bullets(after): + title = bullet_title(bullet) + seen[title] += 1 + if seen[title] > held[title]: + added.append(bullet) + return added def check_pr(base: str, *, root: Path = REPO_ROOT) -> list[str]: @@ -377,9 +434,15 @@ def check_pr(base: str, *, root: Path = REPO_ROOT) -> list[str]: one corrected), when it is the release cut (a version heading the base lacks — the cut moves bullets OUT and needs no fragment of its own), or when everything it touches is exempt (catalogue-only, `plots/`). Fails when - it carries no fragment, and — independently — when it writes bullets into - `[Unreleased]` directly: that is the shared spot the fragments exist to - retire. + it carries no fragment, and — independently — when it ADDS a bullet to + `[Unreleased]`: that is the shared spot the fragments exist to retire. + + Added, not merely different. A bullet is identified by its bold title + (`bullet_title`), so re-wording the body of an entry the base already + carries — a typo in a shipped line, a sharper clause — is a change and + passes; only a title the base does not have — or one more copy of a title + it has — is a new entry and is refused into a fragment. Set-of-bullets + identity could not tell the two apart and refused both. """ changed = _changed_files(root, base) problems: list[str] = [] @@ -389,8 +452,7 @@ def check_pr(base: str, *, root: Path = REPO_ROOT) -> list[str]: before = split_changelog(_git(root, "show", f"{merge_base}:{CHANGELOG_NAME}", required=True)) after = split_changelog((root / CHANGELOG_NAME).read_text(encoding="utf-8")) release_cut = after.newest_version != before.newest_version - gained = _bullets(after.unreleased) - _bullets(before.unreleased) - for bullet in sorted(gained): + for bullet in sorted(_added_bullets(before.unreleased, after.unreleased)): title = bullet.split("\n", 1)[0][:72] problems.append(f"{CHANGELOG_NAME} [Unreleased] gained a bullet — it belongs in a fragment: {title}…") touches_fragments = any(p.startswith(f"{FRAGMENT_DIR_NAME}/") for p in changed)