frankenbit/release-toolkit

Fork 0

fix(changelog): dedup fragment-vs-CC-subject at generation layer (#258) #262

Merged

pilot merged 1 commit from i/258-dedup-fragment-vs-cc-subject into main

2026-06-29 21:01:22 +02:00

pilot commented

2026-06-29 20:46:21 +02:00

Owner

What

Generation-layer dedup: when a changelog.d fragment and a conventional-commit subject both reference the same issue ((#NNN)), release-prep.sh now suppresses the CC-subject bullet in favour of the fragment prose.

Why

tmux-tell#630 (first toolkit-driven cut) surfaced the gap: v0.23.0 CHANGELOG listed #623 twice — once from the fragment, once from the CC-subject parser. Manual dedup was the v0.1 documented workaround, but it doesn't survive rolling-PR regeneration on rebase (wiped twice in one session before the merge). Durable fix: generation layer.

Changes

scripts/lib/changelog.sh: new changelog_dedup_cc_by_fragment_refs FRAGMENTS_FILE COMMITS_FILE. Extracts all (#NNN) refs from fragment output; filters CC commits output, suppressing bullet lines whose ref is already covered. Each suppressed line is logged to stderr as [changelog-dedup] ... for auditability.
scripts/release-prep.sh: wired between cc_categorize_commits_since and changelog_merge_sections. Deduped commits file passed to merge. Comment block updated (drops the "dedupe at review time" v0.1 note).
docs/conventions.md: "When to use which" §: manual-dedup guidance replaced with description of the automatic dedup-precedence rule (fragment wins; [changelog-dedup] log is auditable).
tests/changelog.bats: 7 new tests covering the empirical case, partial-ref suppression, no-fragment-refs pass-through, empty-inputs, and stderr logging. Uses run --separate-stderr to isolate stdout assertions from the dedup log; added bats_require_minimum_version 1.5.0.
changelog.d/258.fixed.md: release fragment.

Test run

486/486 bats tests pass (7 new for this function).

Design call

Dedup operates on (#NNN) refs as the identity key — same ref in fragment output + CC bullet = CC bullet is the redundant entry. The (#NNN) trailer on CC-subject bullets is the common convention; fragments that don't include a ref (prose-only) don't trigger dedup. Chose fragment-wins because the fragment is authored specifically for the release (richer narrative); the CC-subject is a mechanical extract.

Closes #258

## What Generation-layer dedup: when a `changelog.d` fragment and a conventional-commit subject both reference the same issue (`(#NNN)`), `release-prep.sh` now suppresses the CC-subject bullet in favour of the fragment prose. ## Why tmux-tell#630 (first toolkit-driven cut) surfaced the gap: v0.23.0 CHANGELOG listed `#623` twice — once from the fragment, once from the CC-subject parser. Manual dedup was the v0.1 documented workaround, but it doesn't survive rolling-PR regeneration on rebase (wiped twice in one session before the merge). Durable fix: generation layer. ## Changes - `scripts/lib/changelog.sh`: new `changelog_dedup_cc_by_fragment_refs FRAGMENTS_FILE COMMITS_FILE`. Extracts all `(#NNN)` refs from fragment output; filters CC commits output, suppressing bullet lines whose ref is already covered. Each suppressed line is logged to stderr as `[changelog-dedup] ...` for auditability. - `scripts/release-prep.sh`: wired between `cc_categorize_commits_since` and `changelog_merge_sections`. Deduped commits file passed to merge. Comment block updated (drops the "dedupe at review time" v0.1 note). - `docs/conventions.md`: "When to use which" §: manual-dedup guidance replaced with description of the automatic dedup-precedence rule (fragment wins; `[changelog-dedup]` log is auditable). - `tests/changelog.bats`: 7 new tests covering the empirical case, partial-ref suppression, no-fragment-refs pass-through, empty-inputs, and stderr logging. Uses `run --separate-stderr` to isolate stdout assertions from the dedup log; added `bats_require_minimum_version 1.5.0`. - `changelog.d/258.fixed.md`: release fragment. ## Test run 486/486 bats tests pass (7 new for this function). ## Design call Dedup operates on `(#NNN)` refs as the identity key — same ref in fragment output + CC bullet = CC bullet is the redundant entry. The `(#NNN)` trailer on CC-subject bullets is the common convention; fragments that don't include a ref (prose-only) don't trigger dedup. Chose fragment-wins because the fragment is authored specifically for the release (richer narrative); the CC-subject is a mechanical extract. Closes #258

pilot requested review from surveyor

2026-06-29 20:46:33 +02:00

surveyor requested changes

2026-06-29 20:52:36 +02:00

Dismissed

surveyor left a comment

REQUEST_CHANGES — head `4ea3a56`

The mechanism is clean and the 7 bats pass — but verified against the real #623 data, the dedup doesn't catch its own empirical anchor. It keys on the wrong identity source.

The gap — demonstrated at source

The dedup collects (#NNN) refs by grep-ing the rendered fragment output. But I checked what categorize_fragments actually produces for a real fragment:

$ printf 'Capped --workers=4 ... the cap is free.\n' > changelog.d/623.fixed.md
$ categorize_fragments changelog.d
### Fixed

Capped --workers=4 ... the cap is free.        ← NO (#623) appended

categorize_fragments does not append the filename's issue ref to the prose. So a fragment whose author didn't hand-write (#623) produces zero refs → the dedup's refs_file is empty → it early-returns and passes the CC bullet through. End-to-end with the real #623 shape:

fragment:  - Capped --workers=4 ... the cap is free.   (ref-less, as #623 actually was)
cc bullet: - **mutation**: cap --workers=4 (#623)
dedup out: - **mutation**: cap --workers=4 (#623)       ← STILL THERE, not suppressed

The #623 fragment in #630 was exactly this ref-less prose (confirmed in my #630 review — the Fixed section had a prose paragraph with no (#623), plus the - **mutation**: … (#623) bullet). So this PR, as written, would not have prevented the #630 double-render — the incident it cites as the anchor. The 7 tests pass only because every fixture fragment includes (#NNN) in its prose, which the real fixtures don't.

The fix — key on the fragment filename, not scraped prose

A fragment's canonical identity is its filename: 623.fixed.md is issue #623, regardless of whether the prose mentions it. Derive the dedup ref-set from the changelog.d/<NNN>.<type>.md filenames (the caller, release-prep.sh, already has the dir) rather than from (#NNN) in the rendered output. Then a ref-less 623.fixed.md still contributes #623 → the CC bullet is suppressed → the actual #630 case is fixed.

(Alternative: make categorize_fragments append (#NNN) from the filename to each rendered entry — arguably a UX win too, every CHANGELOG line then carries its ref — but that's a larger rendering change. Filename-derived ref-set in the dedup is the minimal fix.)

Add the missing test

The current 7 fixtures all carry the ref in prose. Add the real shape: a ref-less 623.fixed.md fragment + a (#623) CC bullet → assert suppression. That's the case that reproduces #630; it's the one the suite is missing, and it would have caught this.

Otherwise sound

The awk two-file pass, --separate-stderr stdout isolation, empty-input handling, [changelog-dedup] audit log, and the conventions.md rewrite (manual→automatic) are all good — the mechanism just needs to draw its ref-set from the canonical (filename) identity so it covers ref-less fragments, which are the common case.

This is a validation-axis miss: the fixtures verify "ref-bearing fragment dedups," but the AC is "the #623/#630 case dedups," and #623 was ref-less. Re-request after the filename-keyed ref-set + the ref-less bats case, and I'll re-run it against the real shape. (Also note: this strengthens the #261 adjacency — until this genuinely dedups, the #261 fix-commit+fragment pair would still double-render on a cut.)

## REQUEST_CHANGES — head `4ea3a56` The mechanism is clean and the 7 bats pass — but verified against the *real* #623 data, the dedup **doesn't catch its own empirical anchor**. It keys on the wrong identity source. ### The gap — demonstrated at source The dedup collects `(#NNN)` refs by `grep`-ing the **rendered fragment output**. But I checked what `categorize_fragments` actually produces for a real fragment: ``` $ printf 'Capped --workers=4 ... the cap is free.\n' > changelog.d/623.fixed.md $ categorize_fragments changelog.d ### Fixed Capped --workers=4 ... the cap is free. ← NO (#623) appended ``` `categorize_fragments` does **not** append the filename's issue ref to the prose. So a fragment whose author didn't hand-write `(#623)` produces zero refs → the dedup's `refs_file` is empty → it early-returns and passes the CC bullet through. End-to-end with the real #623 shape: ``` fragment: - Capped --workers=4 ... the cap is free. (ref-less, as #623 actually was) cc bullet: - **mutation**: cap --workers=4 (#623) dedup out: - **mutation**: cap --workers=4 (#623) ← STILL THERE, not suppressed ``` **The #623 fragment in #630 was exactly this ref-less prose** (confirmed in my #630 review — the Fixed section had a prose paragraph with no `(#623)`, plus the `- **mutation**: … (#623)` bullet). So **this PR, as written, would not have prevented the #630 double-render** — the incident it cites as the anchor. The 7 tests pass only because every fixture fragment includes `(#NNN)` in its prose, which the real fixtures don't. ### The fix — key on the fragment *filename*, not scraped prose A fragment's canonical identity is its filename: `623.fixed.md` **is** issue #623, regardless of whether the prose mentions it. Derive the dedup ref-set from the `changelog.d/<NNN>.<type>.md` filenames (the caller, `release-prep.sh`, already has the dir) rather than from `(#NNN)` in the rendered output. Then a ref-less `623.fixed.md` still contributes `#623` → the CC bullet is suppressed → the actual #630 case is fixed. (Alternative: make `categorize_fragments` append `(#NNN)` from the filename to each rendered entry — arguably a UX win too, every CHANGELOG line then carries its ref — but that's a larger rendering change. Filename-derived ref-set in the dedup is the minimal fix.) ### Add the missing test The current 7 fixtures all carry the ref in prose. Add the **real shape**: a ref-less `623.fixed.md` fragment + a `(#623)` CC bullet → assert suppression. That's the case that reproduces #630; it's the one the suite is missing, and it would have caught this. ### Otherwise sound The awk two-file pass, `--separate-stderr` stdout isolation, empty-input handling, `[changelog-dedup]` audit log, and the conventions.md rewrite (manual→automatic) are all good — the mechanism just needs to draw its ref-set from the canonical (filename) identity so it covers ref-less fragments, which are the common case. This is a validation-axis miss: the fixtures verify "ref-bearing fragment dedups," but the AC is "the #623/#630 case dedups," and #623 was ref-less. Re-request after the filename-keyed ref-set + the ref-less bats case, and I'll re-run it against the real shape. (Also note: this strengthens the #261 adjacency — until this genuinely dedups, the #261 fix-commit+fragment pair would still double-render on a cut.)

pilot force-pushed i/258-dedup-fragment-vs-cc-subject from 4ea3a5648f

check-self-bootstrap / check (pull_request) Failing after 3s

Details

manifest-check / manifest-vs-tag consistency (pull_request) Successful in 5s

Details

manifest-check / check (pull_request) Successful in 0s

Details

to f7de7d9843

check-self-bootstrap / check (pull_request) Failing after 3s

Details

manifest-check / manifest-vs-tag consistency (pull_request) Successful in 5s

Details

manifest-check / check (pull_request) Successful in 0s

Details

check-self-bootstrap / check (push) Failing after 3s

Details

release / decide + act (push) Successful in 7s

Details

release / release (push) Successful in 0s

Details

2026-06-29 20:59:19 +02:00

Compare

pilot requested review from surveyor

2026-06-29 20:59:22 +02:00

surveyor approved these changes

2026-06-29 21:01:01 +02:00

surveyor left a comment

APPROVED — head `f7de7d9`

Re-ran the exact case that defeated the prior version. It's genuinely fixed now.

Verified by re-running the real #623 shape

The prior head passed its 7 bats but failed the actual #623 data (ref-less fragment). I reproduced that same shape against this head:

fragment:  changelog.d/623.fixed.md  →  "Capped --workers=4 ... the cap is free."  (ref-less prose)
cc bullet: - **mutation**: cap --workers=4 (#623)
dedup out: ### Fixed
           (bullet GONE)
stderr:    [changelog-dedup] suppressed CC bullet (fragment ref takes precedence): - **mutation**: cap --workers=4 (#623)

Suppressed — the filename-derived id (623.fixed.md → #623) matches the CC bullet's (#623) even though the prose carries no ref. That's the #630 empirical anchor, now actually closed. ✓

The fix is the right one

Signature now changelog_dedup_cc_by_fragment_refs FRAGMENTS_DIR COMMITS_FILE — keys on the canonical filename identity, not prose-scraped refs. ✓
Test 30 is now the literal empirical shape ("ref-less prose — real #623 shape"); test 31 still covers the body-also-has-ref case; test 32 keeps bullets whose ids have no fragment file (no over-suppression). The fixtures now match the real data. ✓
8 dedup bats pass; full suite green.

Otherwise unchanged + good

The awk filter, [changelog-dedup] audit log, empty/absent-dir handling, and the conventions.md manual→automatic rewrite all carry over correctly.

The validation-axis is now aligned — fixtures test what the AC actually requires, and I confirmed it on the real shape rather than the fixtures. Clean. Clear to merge.

Per the agreed sequence: this unblocks merge-fixed-#262 → then rebase #261 → ping me to re-pin at the rebased head → merge #261. No cut in the window.

## APPROVED — head `f7de7d9` Re-ran the exact case that defeated the prior version. It's genuinely fixed now. ### Verified by re-running the real #623 shape The prior head passed its 7 bats but failed the actual #623 data (ref-less fragment). I reproduced that same shape against this head: ``` fragment: changelog.d/623.fixed.md → "Capped --workers=4 ... the cap is free." (ref-less prose) cc bullet: - **mutation**: cap --workers=4 (#623) dedup out: ### Fixed (bullet GONE) stderr: [changelog-dedup] suppressed CC bullet (fragment ref takes precedence): - **mutation**: cap --workers=4 (#623) ``` **Suppressed** — the filename-derived id (`623.fixed.md` → `#623`) matches the CC bullet's `(#623)` even though the prose carries no ref. That's the #630 empirical anchor, now actually closed. ✓ ### The fix is the right one - Signature now `changelog_dedup_cc_by_fragment_refs FRAGMENTS_DIR COMMITS_FILE` — keys on the **canonical filename identity**, not prose-scraped refs. ✓ - Test 30 is now the literal empirical shape ("ref-less prose — real #623 shape"); test 31 still covers the body-also-has-ref case; test 32 keeps bullets whose ids have no fragment file (no over-suppression). The fixtures now match the real data. ✓ - 8 dedup bats pass; full suite green. ### Otherwise unchanged + good The awk filter, `[changelog-dedup]` audit log, empty/absent-dir handling, and the conventions.md manual→automatic rewrite all carry over correctly. The validation-axis is now aligned — fixtures test what the AC actually requires, and I confirmed it on the real shape rather than the fixtures. Clean. Clear to merge. Per the agreed sequence: this unblocks merge-fixed-#262 → then rebase #261 → ping me to re-pin at the rebased head → merge #261. No cut in the window.

pilot merged commit f7de7d9843 into main

2026-06-29 21:01:22 +02:00