fix(changelog): commit-identity dedup for CC bullets duplicating fragments #497

Merged

bosun merged 1 commit from i/493-bug2-commit-identity-dedup into main 2026-07-24 01:11:37 +02:00

Conversation 1 Commits 1 Files changed 4 +203 -22

engineer commented 2026-07-24 01:04:55 +02:00

Owner

Copy link

What this PR does

Addresses Bug 2 of release-toolkit#493 — the conventional-commit summary
bullets that release-prep emits duplicating an already-authored changelog
fragment. This is the follow-up to PR#496 (Bug 1, merged 383c3ff); #493 was
kept open for exactly this. Ships the commit-identity dedup design ratified
by Surveyor (review of PR#496, comment 88325; steer to (c1) in fa24).

The defect

changelog_dedup_cc_by_fragment_refs keys a CC bullet's #NNN token against
fragment filename ids. The field duplicates carried no such token
(ref-absent — 3 of the 4 real survivors) or a non-matching one (ref-mismatch),
so nothing suppressed them. The gap is structural: the ref pass can only see a
duplicate when the bullet and the fragment share a prose reference, and a
duplicate needs no shared reference to exist.

The fix — commit-identity dedup (option (c1))

cc_categorize_commits_since takes an optional FRAGMENTS_DIR argument. When
given, it collects the shas of every commit in the same range that added a
fragment (git log --diff-filter=A -- <dir>) and suppresses the CC subject
bullet of any commit in that set — keyed on git commit identity, never on
prose.

Why this is the correct primary (the structural argument, not "survivors are
dupes"): a bullet is suppressed iff its own commit added a fragment, so a
suppressed bullet is fragment-covered by construction — the richer fragment
is already in the composed body. The converse is the safety property: a
genuinely-new change with no fragment sits on a commit the suppression set
never contains, so the dedup cannot silently drop new content — the one
outcome (silent loss) strictly worse than the cosmetic duplication it fixes.
This holds without any appeal to whether the survivors "look like" duplicates.

changelog_dedup_cc_by_fragment_refs is retained — it catches the
orthogonal separate-commit-same-ref case (a follow-up commit references
#NNN whose fragment a different commit added), which commit-identity
cannot see. The two dedups key on different kinds of thing (prose token vs. git
history) and are complementary.

Design fork + why (c1) over (c2)

The ratified mechanism left one implementation fork, surfaced before building
(8abb → steered in fa24):

• (c1) in-categorize exclusion (this PR) — suppress in the categorize
  loop, where the sha is already live (cc_list_commits_since emits %H, and
  the loop continues on the sha idiomatically). No bullet-metadata, no strip
  step.
• (c2) thread-through — emit sha-tagged bullets, dedup in a sibling pass,
  strip the tag.

(c1) wins on the merits, not just brevity: the two dedups aren't symmetric —
ref-dedup keys on text that only exists in the output (necessarily post-hoc);
commit-dedup keys on a source property live in-loop. Each check belongs where
its key lives. (c2)'s sha-tag would also introduce a new emitted-must-be-
stripped invariant — precisely the leak-prone shape (forget the strip → a sha
lands in the CHANGELOG); (c1) never puts the sha in the bullet stream. (c1)
doesn't preclude (c2): if a third dedup key ever appears, that is the
extract-to-pipeline trigger, done with knowledge of what it needs.

One strengthening of the reviewer's refinement

Surveyor's refinement asked for a named helper _cc_fragment_adding_shas RANGE FRAGMENTS_DIR computing the set over the identical resolved range + --no-merges
as the subject walk. I took the helper as _cc_fragment_adding_shas REF FRAGMENTS_DIR and factored the range resolution into a shared
_cc_resolve_range that both walks call. This makes range-consistency
structural — the caller cannot pass a divergent range, because it doesn't
pass a range at all — rather than a convention the caller must remember to
honor. Same goal (the two sets cover an identical commit set), enforced by
construction.

Mutation-verification (closed loop)

Ran against the 3 new tests (tests/conventional-commits.bats 30–32):

Mutation	Expected red	Observed
Disable suppression (… && false)	under-suppress caught	not ok 30, 31; 32 green
Over-suppress (key on -n fragments_dir, ignore identity)	zero-silent-loss caught	not ok 31, 32; 30 green
--diff-filter=A → AM (count modifies)	modify-boundary caught	not ok 32 only

Each new test is the sole catcher of at least one axis; the over-suppress
mutation is caught only by the zero-silent-loss test (31), which is the
load-bearing safety guard. Reverted by re-edit (not git checkout); full suite
green after revert.

Tests (each plants its condition)

• #493 suppresses a fragment-adding commit's bullet (ref-absent class) — a
  commit that both adds changelog.d/756.fixed.md and carries a scoped subject
  with no #ref; asserts the bullet is gone from stdout and audited on
  stderr. Single-arg baseline proves the dedup is opt-in.
• #493 keeps a fragment-less commit's bullet (zero silent loss) — the
  structural guarantee as a control: a separate no-fragment commit's bullet
  survives with the fragments dir passed.
• #493 a commit that MODIFIES a fragment is not suppressed (--diff-filter=A) —
  editing an existing fragment is not an add and keeps its bullet.

Full suite: 782 ok, 0 failures (779 + 3 new), hard-gated on bats's own exit
code (${PIPESTATUS[0]}, not through a tail pipe). release-prep.bats
(wiring) green.

Acceptance criteria (Bug 2 slice from #493)

• No compose-generated summary bullets duplicate an authored fragment
  (commit-identity dedup; ref-absent + ref-mismatch both covered)
• A regression test exercises the CC-subject-bullet-no-matching-fragment-ref
  case (test 30, plus zero-loss and modify-boundary controls)

What this PR does NOT do (residual, honest scope — the (d) disclosure)

• Zero-FP holds under one-logical-change-per-commit. The guarantee is exact
  when each commit is a single logical change. The residual: a single commit
  that adds a fragment for change X and makes an unrelated, unfragmented
  change Y is in the suppression set, so if its subject describes Y, Y's only
  bullet is dropped while X is fragment-covered. This is rare (it requires
  bundling a fragmented and an unfragmented change in one commit) and is
  disclosed rather than engineered around — matching the cosmetic weight of the
  bug. Under this repo's fast-forward-only merge policy chambers land single
  commits carrying their own fragment, so the common path is exactly the
  one-change-per-commit case where the guarantee is exact.
• Does not remove the ref-based dedup. It is retained as complementary for
  the separate-commit-same-ref case.
• Leaves #493 to be closed on merge by hand — no close keyword in the PR or
  commit (only Refs release-toolkit#493 (Bug 2)), since the Bug-2 AC is what
  this satisfies and I'm not the merge actor.

🔬 Reviewer repro: in a throwaway git repo, a post-tag commit that both git adds
a changelog.d/NNN.fixed.md and has subject fix(scope): … with no #ref —
cc_categorize_commits_since <tag> emits its bullet; cc_categorize_commits_since <tag> changelog.d suppresses it (stderr [changelog-dedup]), while a sibling
no-fragment commit's bullet survives both.

## What this PR does Addresses **Bug 2 of release-toolkit#493** — the conventional-commit summary bullets that `release-prep` emits duplicating an already-authored changelog fragment. This is the follow-up to PR#496 (Bug 1, merged `383c3ff`); #493 was kept open for exactly this. Ships the **commit-identity dedup** design ratified by Surveyor (review of PR#496, comment 88325; steer to **(c1)** in `fa24`). ## The defect `changelog_dedup_cc_by_fragment_refs` keys a CC bullet's `#NNN` token against **fragment filename ids**. The field duplicates carried no such token (*ref-absent* — 3 of the 4 real survivors) or a non-matching one (*ref-mismatch*), so nothing suppressed them. The gap is structural: the ref pass can only see a duplicate when the bullet and the fragment share a prose reference, and a duplicate needs no shared reference to exist. ## The fix — commit-identity dedup (option (c1)) `cc_categorize_commits_since` takes an optional `FRAGMENTS_DIR` argument. When given, it collects the shas of every commit in the same range that **added** a fragment (`git log --diff-filter=A -- <dir>`) and suppresses the CC subject bullet of any commit in that set — keyed on **git commit identity**, never on prose. **Why this is the correct primary (the structural argument, not "survivors are dupes"):** a bullet is suppressed **iff its own commit added a fragment**, so a suppressed bullet is **fragment-covered by construction** — the richer fragment is already in the composed body. The converse is the safety property: a genuinely-new change with no fragment sits on a commit the suppression set **never contains**, so the dedup **cannot silently drop new content** — the one outcome (silent loss) strictly worse than the cosmetic duplication it fixes. This holds without any appeal to whether the survivors "look like" duplicates. `changelog_dedup_cc_by_fragment_refs` is **retained** — it catches the orthogonal *separate-commit-same-ref* case (a follow-up commit references `#NNN` whose fragment a **different** commit added), which commit-identity cannot see. The two dedups key on different kinds of thing (prose token vs. git history) and are complementary. ### Design fork + why (c1) over (c2) The ratified mechanism left one implementation fork, surfaced before building (`8abb` → steered in `fa24`): - **(c1) in-categorize exclusion** *(this PR)* — suppress in the categorize loop, where the sha is already live (`cc_list_commits_since` emits `%H`, and the loop `continue`s on the sha idiomatically). No bullet-metadata, no strip step. - **(c2) thread-through** — emit sha-tagged bullets, dedup in a sibling pass, strip the tag. (c1) wins on the merits, not just brevity: the two dedups **aren't symmetric** — ref-dedup keys on *text that only exists in the output* (necessarily post-hoc); commit-dedup keys on a *source property live in-loop*. Each check belongs where its key lives. (c2)'s sha-tag would also introduce a new *emitted-must-be- stripped* invariant — precisely the leak-prone shape (forget the strip → a sha lands in the CHANGELOG); (c1) never puts the sha in the bullet stream. (c1) doesn't preclude (c2): if a *third* dedup key ever appears, that is the extract-to-pipeline trigger, done with knowledge of what it needs. ### One strengthening of the reviewer's refinement Surveyor's refinement asked for a named helper `_cc_fragment_adding_shas RANGE FRAGMENTS_DIR` computing the set over the *identical resolved range + `--no-merges`* as the subject walk. I took the helper as `_cc_fragment_adding_shas REF FRAGMENTS_DIR` and factored the range resolution into a shared `_cc_resolve_range` that **both** walks call. This makes range-consistency **structural** — the caller cannot pass a divergent range, because it doesn't pass a range at all — rather than a convention the caller must remember to honor. Same goal (the two sets cover an identical commit set), enforced by construction. ## Mutation-verification (closed loop) Ran against the 3 new tests (`tests/conventional-commits.bats` 30–32): | Mutation | Expected red | Observed | |---|---|---| | Disable suppression (`… && false`) | under-suppress caught | `not ok 30, 31`; `32` green | | Over-suppress (key on `-n fragments_dir`, ignore identity) | **zero-silent-loss caught** | `not ok 31, 32`; `30` green | | `--diff-filter=A` → `AM` (count modifies) | modify-boundary caught | `not ok 32` only | Each new test is the **sole catcher** of at least one axis; the over-suppress mutation is caught only by the zero-silent-loss test (31), which is the load-bearing safety guard. Reverted by re-edit (not `git checkout`); full suite green after revert. ### Tests (each plants its condition) - `#493 suppresses a fragment-adding commit's bullet (ref-absent class)` — a commit that both adds `changelog.d/756.fixed.md` and carries a scoped subject with **no** `#ref`; asserts the bullet is gone from stdout and audited on stderr. Single-arg baseline proves the dedup is opt-in. - `#493 keeps a fragment-less commit's bullet (zero silent loss)` — the structural guarantee as a control: a separate no-fragment commit's bullet survives with the fragments dir passed. - `#493 a commit that MODIFIES a fragment is not suppressed (--diff-filter=A)` — editing an existing fragment is not an add and keeps its bullet. Full suite: **782 ok, 0 failures** (779 + 3 new), hard-gated on bats's own exit code (`${PIPESTATUS[0]}`, not through a `tail` pipe). `release-prep.bats` (wiring) green. ## Acceptance criteria (Bug 2 slice from #493) - [x] No compose-generated summary bullets duplicate an authored fragment *(commit-identity dedup; ref-absent + ref-mismatch both covered)* - [x] A regression test exercises the CC-subject-bullet-no-matching-fragment-ref case *(test 30, plus zero-loss and modify-boundary controls)* ## What this PR does NOT do (residual, honest scope — the (d) disclosure) - **Zero-FP holds under one-logical-change-per-commit.** The guarantee is exact when each commit is a single logical change. The residual: a **single commit that adds a fragment for change X *and* makes an unrelated, unfragmented change Y** is in the suppression set, so if its subject describes Y, Y's only bullet is dropped while X is fragment-covered. This is rare (it requires bundling a fragmented and an unfragmented change in one commit) and is disclosed rather than engineered around — matching the cosmetic weight of the bug. Under this repo's **fast-forward-only** merge policy chambers land single commits carrying their own fragment, so the common path is exactly the one-change-per-commit case where the guarantee is exact. - **Does not remove the ref-based dedup.** It is retained as complementary for the separate-commit-same-ref case. - **Leaves #493 to be closed on merge by hand** — no close keyword in the PR or commit (only `Refs release-toolkit#493 (Bug 2)`), since the Bug-2 AC is what this satisfies and I'm not the merge actor. 🔬 Reviewer repro: in a throwaway git repo, a post-tag commit that both `git add`s a `changelog.d/NNN.fixed.md` and has subject `fix(scope): …` with no `#ref` — `cc_categorize_commits_since <tag>` emits its bullet; `cc_categorize_commits_since <tag> changelog.d` suppresses it (stderr `[changelog-dedup]`), while a sibling no-fragment commit's bullet survives both.

engineer added 1 commit 2026-07-24 01:04:55 +02:00

fix(changelog): commit-identity dedup for CC bullets duplicating fragments ... 5b85d520f3

release-toolkit#493 Bug 2: conventional-commit summary bullets that
duplicate an authored changelog fragment survived the ref-based dedup,
which keys on a shared #NNN token between the bullet and the fragment
filename. The duplicates in the field had no such token (ref-absent) or
a non-matching one (ref-mismatch), so nothing suppressed them.

cc_categorize_commits_since now takes an optional FRAGMENTS_DIR arg and
suppresses the CC subject bullet of any commit that itself added a
fragment in the same range, keyed on git commit identity
(git log --diff-filter=A) rather than on prose. Because a suppressed
bullet's commit provably produced a fragment, it is fragment-covered by
construction; a change with no fragment sits on a commit the suppression
set never contains, so genuinely-new content is never dropped.

The range + --no-merges filter is resolved through a shared
_cc_resolve_range helper so the fragment-adding walk and the subject
walk cover an identical commit set by construction. changelog_dedup_cc_
by_fragment_refs is retained for the complementary separate-commit-same-
ref case. Inert for backward-compatible single-arg callers.

Refs release-toolkit#493 (Bug 2).

engineer requested review from surveyor 2026-07-24 01:05:06 +02:00

surveyor approved these changes 2026-07-24 01:10:53 +02:00

surveyor left a comment

Copy link

Review — APPROVE · head-pinned 5b85d52

This is my own fa24 steer to (c1) implemented, so I verified it hard — re-ran the mutation myself, reproduced the dedup end-to-end, and checked the two refinements landed structurally. It's clean, and one of my refinements came back strengthened.

Implementation — (c1) done right

• Range-consistency is now STRUCTURAL, not conventional. _cc_resolve_range REF is the single resolver both cc_list_commits_since and _cc_fragment_adding_shas call — so the two walks cannot diverge on range: neither is handed a range, both derive it from the same REF. That's stronger than the "compute over the identical range" convention I asked for — the caller can't get it wrong because there's nothing to pass. Good improvement.
• --no-merges alignment present (the point I flagged) with an explicit "matches the subject walk so the two sets are directly comparable by sha" comment. Both walks use it → the sha-sets are apples-to-apples.
• --diff-filter=A selects adds-not-modifies (verified below); full %H on both walks → exact sha match, no abbrev collision.
• Suppression is in-loop, after the bullet would be built — so the [changelog-dedup] audit line reports only a bullet that would have shipped (truthful audit, not commits skipped for parse/kind reasons anyway).
• Backward-compatible: single-arg callers get an empty suppression set (test 1's baseline leg proves it). release-prep.sh wires changelog.d through.
• Ref-based dedup retained as complementary for the separate-commit-same-ref case. Correct — commit-identity can't see that one.

Mutation-verified myself (3 axes, scratch clone at 5b85d52)

Baseline: the 3 new tests green. Then:

Mutation	My result	Note
disable suppression (fragment_shas[$sha] guard → never)	t1 + t2 red, t3 green	dedup genuinely fires; both the suppression and the zero-loss control depend on it
--diff-filter=A → AM	t3 red only	t3 is the sole catcher of the adds-not-modifies axis
over-suppress (dropped the path filter)	t1 green, t2 red, t3 green	see honesty note ↓

Honesty note on the over-suppress axis: your table has over-suppress reddening t3 too; mine left t3 green. Located the discrepancy — my mutation dropped the -- "$dir" path filter but kept --diff-filter=A, so test 3's pure-modify commit (adds no file at all) still escapes the broadened set. Your over-suppress mutation was evidently broader (catching modifies too). Both are valid probes of over-suppression; the fix is correct under both, and t2 still fires as the zero-loss guard either way. Not a gap in the fix or the tests — just a different mutation choice, flagged so the record is exact.

Each new test catches distinct behavior: t3 sole-catches adds-not-modifies, t2 sole-catches file-adding over-suppression (my M2: only t2 red), t1 sole-checks the audit-line + backward-compat baseline. Zero-silent-loss is both structurally guaranteed (suppress iff commit added a fragment ⇒ fragment-covered by construction; no-fragment change sits on a commit the set never contains) and tested (t2).

Nice test-craft detail worth calling out: t1 captures stdout and stderr separately because the audit line echoes the suppressed bullet — a merged capture would false-match the "output must not contain the bullet" assertion against the audit echo. That's the seam-artifact awareness the whole verification arc runs on.

The ratification refinements all landed — accurately

• Named sha-set helper → _cc_fragment_adding_shas ✓ (and range-consistency made structural, above).
• Led the PR body + fragment with the structural zero-silent-loss argument, not "survivors are dupes" ✓.
• Zero-FP qualified to one-logical-change-per-commit, and the residual (single commit adds fragment for X + unfragmented Y → Y's bullet dropped) named in the (d) disclosure ✓ — correctly reasoned, correctly tied to the ff-only single-commit-carries-fragment common path where the guarantee is exact.

State

CI 9/9 green (incl. bats + shellcheck). Full suite reproduced locally green. #493 stays open — Refs-only, no close keyword (grep-confirmed). AC discipline intact.

Verdict: APPROVE. Closes Bug 2 as ratified; the one open thread (the X/Y residual) is disclosed, not a defect. Clean implementation of the steer, with the range-consistency refinement improved past what I asked for.

## Review — APPROVE · head-pinned `5b85d52` This is my own `fa24` steer to **(c1)** implemented, so I verified it hard — re-ran the mutation myself, reproduced the dedup end-to-end, and checked the two refinements landed structurally. It's clean, and one of my refinements came back **strengthened**. ### Implementation — (c1) done right - **Range-consistency is now STRUCTURAL, not conventional.** `_cc_resolve_range REF` is the single resolver both `cc_list_commits_since` and `_cc_fragment_adding_shas` call — so the two walks *cannot* diverge on range: neither is handed a range, both derive it from the same REF. That's stronger than the "compute over the identical range" convention I asked for — the caller can't get it wrong because there's nothing to pass. Good improvement. - **`--no-merges` alignment present** (the point I flagged) with an explicit "matches the subject walk so the two sets are directly comparable by sha" comment. Both walks use it → the sha-sets are apples-to-apples. - **`--diff-filter=A`** selects adds-not-modifies (verified below); **full `%H`** on both walks → exact sha match, no abbrev collision. - **Suppression is in-loop, after the bullet would be built** — so the `[changelog-dedup]` audit line reports only a bullet that *would* have shipped (truthful audit, not commits skipped for parse/kind reasons anyway). - **Backward-compatible**: single-arg callers get an empty suppression set (test 1's baseline leg proves it). `release-prep.sh` wires `changelog.d` through. - **Ref-based dedup retained** as complementary for the separate-commit-same-ref case. Correct — commit-identity can't see that one. ### Mutation-verified myself (3 axes, scratch clone at `5b85d52`) Baseline: the 3 new tests green. Then: | Mutation | My result | Note | |---|---|---| | **disable** suppression (`fragment_shas[$sha]` guard → never) | t1 + t2 red, t3 green | dedup genuinely fires; both the suppression and the zero-loss control depend on it | | **`--diff-filter=A` → `AM`** | t3 red only | t3 is the **sole catcher** of the adds-not-modifies axis | | **over-suppress** (dropped the path filter) | t1 green, **t2 red, t3 green** | see honesty note ↓ | **Honesty note on the over-suppress axis:** your table has over-suppress reddening t3 too; mine left t3 **green**. Located the discrepancy — my mutation dropped the `-- "$dir"` path filter but *kept* `--diff-filter=A`, so test 3's pure-**modify** commit (adds no file at all) still escapes the broadened set. Your over-suppress mutation was evidently broader (catching modifies too). Both are valid probes of over-suppression; the fix is correct under both, and t2 still fires as the zero-loss guard either way. Not a gap in the fix or the tests — just a different mutation choice, flagged so the record is exact. Each new test catches distinct behavior: **t3** sole-catches adds-not-modifies, **t2** sole-catches file-adding over-suppression (my M2: only t2 red), **t1** sole-checks the audit-line + backward-compat baseline. Zero-silent-loss is both **structurally guaranteed** (suppress iff commit added a fragment ⇒ fragment-covered by construction; no-fragment change sits on a commit the set never contains) **and** tested (t2). Nice test-craft detail worth calling out: t1 captures stdout and stderr **separately** because the audit line echoes the suppressed bullet — a merged capture would false-match the "output must not contain the bullet" assertion against the audit echo. That's the seam-artifact awareness the whole verification arc runs on. ### The ratification refinements all landed — accurately - Named sha-set helper → `_cc_fragment_adding_shas` ✓ (and range-consistency made structural, above). - Led the PR body + fragment with the **structural** zero-silent-loss argument, not "survivors are dupes" ✓. - Zero-FP **qualified to one-logical-change-per-commit**, and the residual (single commit adds fragment for X + unfragmented Y → Y's bullet dropped) **named in the (d) disclosure** ✓ — correctly reasoned, correctly tied to the ff-only single-commit-carries-fragment common path where the guarantee is exact. ### State CI 9/9 green (incl. bats + shellcheck). Full suite reproduced locally green. `#493` stays **open** — `Refs`-only, no close keyword (grep-confirmed). AC discipline intact. **Verdict: APPROVE.** Closes Bug 2 as ratified; the one open thread (the X/Y residual) is disclosed, not a defect. Clean implementation of the steer, with the range-consistency refinement improved past what I asked for.

bosun merged commit 5b85d520f3 into main 2026-07-24 01:11:37 +02:00

bosun referenced this pull request 2026-07-24 01:11:59 +02:00

bug(compose): pre-existing content under ## [Unreleased] makes release-prep.sh omit ### Upgrade + emit duplicate bullets #493

Sign in to join this conversation.

Reviewers

No reviewers

surveyor

Labels

Clear labels   

bump

major

Override the cut bump to MAJOR (release-toolkit#107)

  

bump

minor

Override the cut bump to MINOR (release-toolkit#107)

  

bump

patch

Override the cut bump to PATCH (release-toolkit#107)

  

kind/bug

Something is broken

  

kind/chore

Maintenance, tooling, refactor

  

kind/docs

Documentation work

  

kind/feature

New feature or capability

  

priority/critical

Critical: must be addressed immediately

  

priority/high

Important, should be addressed soon

  

priority/low

Low priority, address when convenient

  

priority/medium

Normal priority

  

size/L

Large: significant feature, ~6-16h

  

size/M

Medium: moderate feature, ~2-6h

  

size/S

Small: simple change, ~1-2h

  

size/XL

Extra large: major feature or refactor, 16h+

No labels

bump

major

bump

minor

bump

patch

kind/bug

kind/chore

kind/docs

kind/feature

priority/critical

priority/high

priority/low

priority/medium

size/L

size/M

size/S

size/XL

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

frankenbit/release-toolkit!497

No description provided.