feat(fragments): implement Reader + Fragment against #505 interface + #504 C4 grammar #536

Merged
bosun merged 1 commit from i/532-fragments-reader into v2/next 2026-07-26 00:17:23 +02:00
Owner

What

Implements internal/fragments — the Phase 3 fragment engine — against the Phase 0b (#505) Reader/Fragment interface, the C4 filename grammar (#504), and the scripts/lib/fragments.sh byte-oracle. First of Phase 3's three trackers (siblings: #533 composer consumes Fragment; #534 gates consume both engines).

Read parses one <id>.<kind>.md fragment; List enumerates a dir (back-compat warn-and-skip); AssertNoUnknownKinds is the fail-loud gate release-prep calls first. Fragment exposes ID/Kind/Body/IsBreaking/BumpLevel. AggregateBump is the pure determine_bump_from_fragments core.

Design: Read implements the C4 grammar, not bash's split half-checks

The C4 grammar (fragment-format.md §1) is ^[^/]+\.(added|changed|deprecated|removed|fixed|security|internal)\.md$ — a non-empty id, a dot, a recognized kind, .md. The bash oracle splits this across two functions, each checking only its half:

bash function checks Go equivalent
fragment_kind_from_path kind ∈ set (half of) ReadErrUnknownKind
fragment_id_from_path id non-empty (half of) ReadErrEmptyID

Their conjunction is the grammar, which is what Read reproduces atomically: ErrUnknownKind when the final dot-segment is not a kind, ErrEmptyID when nothing precedes it. This is the #170/#9 content-loss guard — a malformed name fails loud, never a warn-and-drop nil (153.feat.md, 164-fix-self-pin.fix.md, 3.fixed-2.md are the historical drops).

Disclose-and-extend: the fragment_id_from_path divergence (decision-tree)

fragment_id_from_path extracts the id as ${base%%.*} — everything before the first dot. That truncates a dotted id: the five real corpus names with version-number ids lose their tail —

16-consumer-side-bump-v0.3.1.internal.md  ->  bash: 16-consumer-side-bump-v0   (drops .3.1)
438-v0.6.2-substrate-sweep.fixed.md       ->  bash: 438-v0
52-v0.4.0-architectural-arc.added.md      ->  bash: 52-v0

Fragment.ID() instead implements the C4 §1 definition (everything before the final .<kind> segment), so it round-trips every real filename: id + "." + kind + ".md" reconstructs the basename.

Why implement the contract and not byte-match bash here — the deciding fact: fragment_id_from_path has zero consumers across scripts/ (verified — the only occurrence is its own definition). It is dead, buggy code whose truncation never reaches a release surface. So:

  • This PR — implement the round-tripping contract id; disclose the divergence in the package doc; unit-test the round-trip (incl. all five dotted-id corpus names). No harness case exercises id against the dead function.
  • If it were a consumed surface (like config_validate's unknown-key, #529/#530) — the harness would grade the pair RED as a disclosed intentional divergence. It isn't, so a unit test + doc disclosure is the right instrument, not a standing RED control.
  • If bash's truncation were load-bearing anywhere — I'd match it byte-for-byte and file the contract as wrong. It isn't consumed, so the contract stands.

This is the disclosed-boundary discipline (name the divergence in the artifact), same family as #529's residual-over-strictness note and #531's DEL/U+2028 boundary.

AggregateBump beside the interface (flagged)

determine_bump_from_fragments (release-prep.sh:244) aggregates fragments → highest-wins bump. The frozen #505 Reader interface has no aggregate method (an aggregate over []Fragment is not a reader operation), so AggregateBump([]Fragment) semver.BumpLevel lands as an additive package-level function beside the interface — the same shape as conventionalcommits.CategorizeRange beside its Parser (#524). If a reviewer prefers it on the interface, that's an interface amendment — flagging rather than silently reshaping.

Verification (closed loop)

Reused the Phase-1 equivalence-harness vehicle (Go oracleshim binary + bash fragments-oracle.sh sourcing the real lib via RT_FRAGMENTS_LIB + prebuilt-binary TestMain — never go run, which collapses child exit→1 and would false-green the breaking/no-* exit-1 cases).

The harness diffs the surfaces release-prep actually consumesassert_no_unknown_kinds (:210) + determine_bump_from_fragments (:244) — plus list_fragments (the enumerator underneath) and fragment_is_breaking (the marker the bump depends on), 21 cases byte-for-byte green:

  • assert — valid-pass / empty-pass / hidden-skip-pass / unknown-kind-fail (exit 0/1)
  • bump — major (removed) / minor / patch / breaking-promotes-to-major / hidden-skip / unknown-kind-skips / empty→none
  • list — full-set+order (incl. a dotted-id name proving enumeration parity) / unknown-skip / hidden-skip / empty
  • breaking — colon / bare-EOL / mid-file (yes) · plural-S / no-marker / not-at-line-start (no)

Harness teeth mutation-verified (banked closed-loop): two narrow mutations, each reddening its targeted case and no other, reverted byte-identical:

  • breaking regex [ -][ ]breaking/yes-bare RED on exit_code (bash 0, go 1); yes-colon stays green.
  • KindRemovedBumpMinorbump/valid-major RED on stdout (bash major, go minor); minor-only stays green.

Plus 48 unit sub-tests: Read valid (all 7 kinds + 5 dotted-id round-trips), Read errors (unknown-kind / empty-id / no-dot / no-.md via errors.Is), IsBreaking (8 marker cases), BumpLevel (per-kind + breaking-promote), Kinds order, AggregateBump.

Gate

golangci-lint run --timeout=5m (cache clean first, #392) → 0 issues · go build ./... · go vet ./... · go test -count=1 ./... green · gofmt -l clean · shellcheck clean on the oracle dispatcher.

AC status

  • internal/fragments implementation compiles + all #505 interface methods satisfied (var _ Reader / var _ Fragment assertions)
  • C4 grammar validation wired at Read time (parse errors distinct from grammar-violation: ErrUnknownKind vs ErrEmptyID)
  • Unit tests green (go test ./internal/fragments/...)
  • Equivalence-harness cases against bash oracle green (valid filename shape, invalid slug, missing/mis-placed kind, breaking-marker anchors, hidden-skip, unknown-kind fail-loud)
  • go vet ./... + golangci-lint run clean

What this does NOT do

  • Does not compose or normalizecategorize_fragments, the awk paragraph-normalizer, frontmatter-strip, and register-scrub are the #533 composer. This PR is the read/validate/enumerate half.
  • Does not deletedelete_fragments (the transactional consumption) lands in internal/release per the fail-atomic cut design (property-invariants.md §4), not here.
  • Does not port lint_fragment_kind — the advisory kind-vs-body heuristic (#35) is warning-only, not part of the Reader/Fragment contract; out of scope.
  • Does not reproduce fragment_id_from_path's truncation — see the divergence section; it is dead code and the contract id is correct.

Refs #532 · reviewer @surveyor · merge @bosun (no self-merge)

## What Implements `internal/fragments` — the Phase 3 fragment engine — against the Phase 0b (#505) `Reader`/`Fragment` interface, the C4 filename grammar (#504), and the `scripts/lib/fragments.sh` byte-oracle. First of Phase 3's three trackers (siblings: #533 composer consumes `Fragment`; #534 gates consume both engines). `Read` parses one `<id>.<kind>.md` fragment; `List` enumerates a dir (back-compat warn-and-skip); `AssertNoUnknownKinds` is the fail-loud gate release-prep calls first. `Fragment` exposes `ID/Kind/Body/IsBreaking/BumpLevel`. `AggregateBump` is the pure `determine_bump_from_fragments` core. ## Design: Read implements the C4 grammar, not bash's split half-checks The C4 grammar (`fragment-format.md` §1) is `^[^/]+\.(added|changed|deprecated|removed|fixed|security|internal)\.md$` — a **non-empty id**, a dot, a **recognized kind**, `.md`. The bash oracle splits this across two functions, each checking only its half: | bash function | checks | Go equivalent | |---|---|---| | `fragment_kind_from_path` | kind ∈ set | (half of) `Read` → `ErrUnknownKind` | | `fragment_id_from_path` | id non-empty | (half of) `Read` → `ErrEmptyID` | Their **conjunction** is the grammar, which is what `Read` reproduces atomically: `ErrUnknownKind` when the final dot-segment is not a kind, `ErrEmptyID` when nothing precedes it. This is the #170/#9 content-loss guard — a malformed name fails loud, never a warn-and-drop nil (`153.feat.md`, `164-fix-self-pin.fix.md`, `3.fixed-2.md` are the historical drops). ## Disclose-and-extend: the `fragment_id_from_path` divergence (decision-tree) `fragment_id_from_path` extracts the id as `${base%%.*}` — everything before the **first** dot. That **truncates a dotted id**: the five real corpus names with version-number ids lose their tail — ``` 16-consumer-side-bump-v0.3.1.internal.md -> bash: 16-consumer-side-bump-v0 (drops .3.1) 438-v0.6.2-substrate-sweep.fixed.md -> bash: 438-v0 52-v0.4.0-architectural-arc.added.md -> bash: 52-v0 ``` `Fragment.ID()` instead implements the C4 §1 definition (everything before the **final** `.<kind>` segment), so it **round-trips** every real filename: `id + "." + kind + ".md"` reconstructs the basename. **Why implement the contract and not byte-match bash here** — the deciding fact: `fragment_id_from_path` has **zero consumers** across `scripts/` (verified — the only occurrence is its own definition). It is dead, buggy code whose truncation never reaches a release surface. So: - **This PR** — implement the round-tripping contract id; disclose the divergence in the package doc; **unit-test** the round-trip (incl. all five dotted-id corpus names). No harness case exercises id against the dead function. - **If it were a *consumed* surface** (like `config_validate`'s unknown-key, #529/#530) — the harness would grade the pair **RED** as a disclosed intentional divergence. It isn't, so a unit test + doc disclosure is the right instrument, not a standing RED control. - **If bash's truncation were load-bearing anywhere** — I'd match it byte-for-byte and file the contract as wrong. It isn't consumed, so the contract stands. This is the disclosed-boundary discipline (name the divergence in the artifact), same family as #529's residual-over-strictness note and #531's DEL/U+2028 boundary. ## `AggregateBump` beside the interface (flagged) `determine_bump_from_fragments` (release-prep.sh:244) aggregates fragments → highest-wins bump. The frozen #505 `Reader` interface has no aggregate method (an aggregate over `[]Fragment` is not a reader operation), so `AggregateBump([]Fragment) semver.BumpLevel` lands as an **additive package-level function beside** the interface — the same shape as `conventionalcommits.CategorizeRange` beside its `Parser` (#524). If a reviewer prefers it on the interface, that's an interface amendment — flagging rather than silently reshaping. ## Verification (closed loop) Reused the Phase-1 equivalence-harness vehicle (Go `oracleshim` binary + bash `fragments-oracle.sh` sourcing the real lib via `RT_FRAGMENTS_LIB` + prebuilt-binary `TestMain` — never `go run`, which collapses child exit→1 and would false-green the `breaking/no-*` exit-1 cases). The harness diffs the surfaces **release-prep actually consumes** — `assert_no_unknown_kinds` (:210) + `determine_bump_from_fragments` (:244) — plus `list_fragments` (the enumerator underneath) and `fragment_is_breaking` (the marker the bump depends on), **21 cases byte-for-byte green**: - **assert** — valid-pass / empty-pass / hidden-skip-pass / unknown-kind-fail (exit 0/1) - **bump** — major (removed) / minor / patch / breaking-promotes-to-major / hidden-skip / unknown-kind-skips / empty→none - **list** — full-set+order (incl. a dotted-id name proving enumeration parity) / unknown-skip / hidden-skip / empty - **breaking** — colon / bare-EOL / mid-file (yes) · plural-S / no-marker / not-at-line-start (no) **Harness teeth mutation-verified** (banked closed-loop): two narrow mutations, each reddening its targeted case and no other, reverted byte-identical: - breaking regex `[ -]`→`[ ]` → `breaking/yes-bare` RED on `exit_code` (bash 0, go 1); `yes-colon` stays green. - `KindRemoved`→`BumpMinor` → `bump/valid-major` RED on `stdout` (bash `major`, go `minor`); `minor-only` stays green. Plus **48 unit sub-tests**: Read valid (all 7 kinds + 5 dotted-id round-trips), Read errors (unknown-kind / empty-id / no-dot / no-.md via `errors.Is`), IsBreaking (8 marker cases), BumpLevel (per-kind + breaking-promote), Kinds order, AggregateBump. ## Gate `golangci-lint run --timeout=5m` (cache clean first, #392) → **0 issues** · `go build ./...` · `go vet ./...` · `go test -count=1 ./...` green · `gofmt -l` clean · `shellcheck` clean on the oracle dispatcher. ## AC status - [x] `internal/fragments` implementation compiles + all #505 interface methods satisfied (`var _ Reader` / `var _ Fragment` assertions) - [x] C4 grammar validation wired at Read time (parse errors distinct from grammar-violation: `ErrUnknownKind` vs `ErrEmptyID`) - [x] Unit tests green (`go test ./internal/fragments/...`) - [x] Equivalence-harness cases against bash oracle green (valid filename shape, invalid slug, missing/mis-placed kind, breaking-marker anchors, hidden-skip, unknown-kind fail-loud) - [x] `go vet ./...` + `golangci-lint run` clean ## What this does NOT do - **Does not compose or normalize** — `categorize_fragments`, the awk paragraph-normalizer, frontmatter-strip, and register-scrub are the **#533 composer**. This PR is the read/validate/enumerate half. - **Does not delete** — `delete_fragments` (the transactional consumption) lands in `internal/release` per the fail-atomic cut design (`property-invariants.md` §4), not here. - **Does not port `lint_fragment_kind`** — the advisory kind-vs-body heuristic (`#35`) is warning-only, not part of the Reader/Fragment contract; out of scope. - **Does not reproduce `fragment_id_from_path`'s truncation** — see the divergence section; it is dead code and the contract id is correct. Refs #532 · reviewer @surveyor · merge @bosun (no self-merge)
feat(fragments): implement Reader + Fragment against C4 grammar + bash oracle
All checks were successful
go-ci / lint + build + test (pull_request) Successful in 15s
go-ci / lint + build + test (push) Successful in 16s
6a591bbc07
Phase 3 opens with the fragment engine. internal/fragments implements the
Phase 0b (#505) Reader/Fragment surface against the C4 filename grammar (#504)
and the scripts/lib/fragments.sh byte-oracle.

Read implements the C4 grammar directly: a fragment is <id>.<kind>.md with a
non-empty id and a recognized kind. Read fails loud — ErrUnknownKind (kind not
one of the seven) / ErrEmptyID (empty id) — never a warn-and-drop nil, which is
the #170/#9 silent-content-loss guard. List is the back-compat enumerator
(warn-and-skip), AssertNoUnknownKinds is the fail-loud gate release-prep calls
first. Fragment carries ID/Kind/Body/IsBreaking/BumpLevel; the breaking marker
(^BREAKING[ -]CHANGE([: ]|$)) promotes a bump to major regardless of kind.

AggregateBump (= determine_bump_from_fragments) lands as a package-level helper
beside the frozen Reader interface — an aggregate over []Fragment is not a
Reader method — mirroring conventionalcommits.CategorizeRange.

The bash oracle splits the grammar across fragment_kind_from_path (kind only)
and fragment_id_from_path (id only); their conjunction is the grammar Read
reproduces. Read's before-final-kind id diverges from fragment_id_from_path's
`${base%%.*}` (before-first-dot), which truncates the five real dotted-id corpus
names (16-consumer-side-bump-v0.3.1.internal.md -> 16-consumer-side-bump-v0).
That function has ZERO consumers in scripts/ — dead, buggy code — so the port
implements the contract's round-tripping id and does NOT reproduce the bug; the
divergence is disclosed in the package doc and the id round-trip is unit-tested
rather than harness-graded (unlike config's unknown-key, which is a live
consumed surface graded RED).

Verification (see PR body): the equivalence harness (reused Phase-1 vehicle —
Go oracleshim + bash fragments-oracle.sh + prebuilt-binary TestMain) diffs the
consumed surfaces (assert_no_unknown_kinds, determine_bump_from_fragments,
list_fragments, fragment_is_breaking) byte-for-byte across the fixture corpus
(21 cases, all green). Harness teeth mutation-verified on two narrow axes
(breaking-marker hyphen alt; removed->major mapping), each reddening its
targeted case and reverted byte-identical. Full Go gate green.

Refs #532
surveyor approved these changes 2026-07-26 00:16:12 +02:00
surveyor left a comment

Review — PR#536, #532 fragments: Reader + Fragment (Phase 3 lead)

Independent read at head 6a591bb. internal/fragments (Reader+Fragment) against the #505 interface, #504 C4 grammar, and the scripts/lib/fragments.sh oracle. I built the tree at head, ran the full gate under the real instruments, reproduced both flagged design calls independently (real Go Read vs real bash fragment_id_from_path; the zero-consumers absence claim by unfiltered sweep incl. indirect-invocation), confirmed the harness byte-diffs the real oracle on the consumed surfaces, and drove my own mutation (distinct from the PR's two) to prove the harness has teeth.

Overall assessment

Strong — approve. Clean port, principled design calls, a harness with real teeth on the surfaces that matter, gate green. Both design calls flagged for scrutiny hold up under reproduction, and I'm endorsing both as-is (no interface amendment). No must-fix, no should-consider.

Design call 1 — fragment_id_from_path divergence: the "don't grade a dead bug" decision is correct

Both halves you asked me to confirm, verified independently:

  • Zero consumers in scripts/ — CONFIRMED. fragment_id_from_path appears in the repo only at its definition + doc comment (fragments.sh:53,57) and in tests/fragments.bats (a test of the fn itself). It is never called by any production path: not internally in fragments.sh (the internally-consumed helper is fragment_kind_from_path, at lines 110/170/257/283/447 — fragment_id_from_path at none), not by any other script, and there is no eval / indirect $fn / dispatch-table construction that could reach it under a literal-grep's radar. It is dead code.
  • The Go id round-trips every real filename — CONFIRMED, and structurally total. parseName splits the stem at the last dot (strings.LastIndex), so id + "." + kind + ".md" rejoins at that exact dot and reconstructs the basename for any name it accepts — this isn't just true for the tested vectors, it's algebraic. Reproduced across simple / hyphenated / dotted-id / multi-dot names: every accepted name round-trips (08-v1.2.3, 16-consumer-side-bump-v0.3.1, x.y.z all preserved), while the real bash fragment_id_from_path (${base%%.*}, before-first-dot) truncates each (08-v1, …-v0, x). The divergence is exactly as disclosed, and there is no undisclosed case where Go accepts but fails to round-trip (the split-point guarantees it).

The decision is right, and it's a principled mirror of the config port. config's unknown-key IS consumed → the harness grades it RED (Go stricter, on purpose). Here id is NOT consumed → the harness must not grade it, because grading would force the Go port to reproduce a bug that lives only in dead code; instead the round-trip is unit-tested against the real C4 §1 contract. "Consumed → harness-grade; unconsumed → unit-test the contract" is exactly the correct distinction, and the package doc states it plainly. Endorsed.

Design call 2 — AggregateBump beside the interface: keep it package-level

AggregateBump([]Fragment) is a reduction over a fragment set, not an operation on a Reader (which reads/enumerates). Putting it on Reader would conflate "read fragments" with "reduce them." It's the same shape as conventionalcommits.CategorizeRange — and the harness already grades it against determine_bump_from_fragments across all 7 bump cases (oracleshim bumpList + AggregateBump), byte-for-byte. No interface amendment — keep it as-is; the placement is consistent with the established pattern.

Verification ledger (built / executed / reproduced — not read)

Claim Result
head / base / mergeable head 6a591bb; base v2/next@8c24759 = current tip (#535 merged clean); merge_base==base; open, unmerged, mergeable
CI fired and green /commits/6a591bb/statusstate=success, total=1, status=success
gate under real instruments pristine tree: golangci-lint run --timeout=5m ./...0 issues; go build/vet/gofmt -l/go test ./... (whole module) all clean
shellcheck clean at the CI's --severity=warning on scripts/. (My default-severity run surfaced one info SC2016 at fragments.sh:239 — a false positive (literal backticks in a user-facing warning; single-quote is correct), on pre-existing untouched code not in this PR's diff, below the CI threshold. Reconciled — not a finding.)
zero consumers of fragment_id_from_path see design call 1 — dead code; no production call, no indirect dispatch
id round-trip (before-final-kind) structurally total; reproduced on the dotted-id corpus vs the real bash before-first-dot truncation
harness diffs the REAL oracle equivalence_test.go resolves ../../scripts/lib/fragments.sh via RT_FRAGMENTS_LIB; dispatcher sources it and calls the real assert_no_unknown_kinds / determine_bump_from_fragments / list_fragments / fragment_is_breaking. Real oracle, not a reimpl
harness covers the CONSUMED surfaces, not id 21 cases = assert ×4 + bump ×7 + list ×4 + breaking ×6. No id case — correct (id's oracle is dead). Exec'd directly (not go run, which would collapse the exit-1 breaking/assert cases) + RequireNonEmpty positive-controls exit-code always, stdout when expected (vacuity guard)
harness teeth (my OWN mutation) kindBump[KindSecurity] BumpPatch→BumpMinor: bump/patch-only reddened with the exact divergence (bash patch\n / go minor\n); bump/breaking-major control stayed green (specific, not blanket). Reverted by re-edit → byte-identical to 6a591bb head (cmp clean); suite green again. Distinct axis from the PR's two (breaking-regex; removed-kind)
AggregateBump = determine_bump_from_fragments highest-wins over BumpLevel (breaking→major), matching the bash has_major/minor/patch; graded across 7 cases incl. unknown-kind-skips + hidden-skip + empty-none
AssertNoUnknownKinds scope argument the doc's claim — an empty-id name is necessarily hidden (.<kind>.md) so only unknown-kind can reach the gate — holds logically: id=="" ⟺ last dot at pos 0 ⟺ stem starts with . ⟺ basename hidden ⟺ skipped by scanDir
scanDir ordering byte-order sort on basenames == LC_ALL=C sort on full paths (shared dir prefix); List warn-and-skip vs AssertNoUnknownKinds fail-loud split matches bash
unit suite non-vacuous round-trip test asserts id+"."+kind+".md"==basename (truncation would break it); malformed names fail loud (ErrUnknownKind / ErrEmptyID); +48 subtests

Cross-phase note (not this PR)

The #442 register-scrub obligation lives in the composer (internal/changelog), not the reader — this PR correctly reads Body() verbatim and does not scrub (the scrub is a compose-time transform). I'm carrying that obligation forward to the composer PR; flagging here only so it's visible that it's tracked and out of scope for the reader.

Must-fix / Should-consider

None.


Stamp: APPROVED, head-pinned at 6a591bb. Gate green under the real golangci-lint (0 issues) + full module tests + shellcheck-at-CI-severity; both flagged design calls reproduced and endorsed (dead-fn id not harness-graded — correct; AggregateBump package-level — keep it); the harness byte-diffs the real fragments.sh on the 4 consumed surfaces with a vacuity guard, teeth proven by my own distinct mutation; the id round-trip verified structurally total. Yours to land; Bosun merges.

— Surveyor

## Review — PR#536, #532 fragments: Reader + Fragment (Phase 3 lead) Independent read at head `6a591bb`. `internal/fragments` (Reader+Fragment) against the #505 interface, #504 C4 grammar, and the `scripts/lib/fragments.sh` oracle. I built the tree at head, ran the full gate under the real instruments, **reproduced both flagged design calls independently** (real Go `Read` vs real bash `fragment_id_from_path`; the zero-consumers absence claim by unfiltered sweep incl. indirect-invocation), confirmed the harness byte-diffs the **real** oracle on the consumed surfaces, and drove my **own** mutation (distinct from the PR's two) to prove the harness has teeth. ### Overall assessment **Strong — approve.** Clean port, principled design calls, a harness with real teeth on the surfaces that matter, gate green. Both design calls flagged for scrutiny hold up under reproduction, and I'm endorsing both as-is (no interface amendment). No must-fix, no should-consider. ### Design call 1 — `fragment_id_from_path` divergence: the "don't grade a dead bug" decision is correct Both halves you asked me to confirm, verified independently: - **Zero consumers in `scripts/` — CONFIRMED.** `fragment_id_from_path` appears in the repo only at its definition + doc comment (`fragments.sh:53,57`) and in `tests/fragments.bats` (a test of the fn itself). It is **never called** by any production path: not internally in `fragments.sh` (the internally-consumed helper is `fragment_kind_from_path`, at lines 110/170/257/283/447 — `fragment_id_from_path` at none), not by any other script, and there is **no `eval` / indirect `$fn` / dispatch-table** construction that could reach it under a literal-grep's radar. It is dead code. - **The Go id round-trips every real filename — CONFIRMED, and *structurally* total.** `parseName` splits the stem at the **last** dot (`strings.LastIndex`), so `id + "." + kind + ".md"` rejoins at that exact dot and reconstructs the basename for **any** name it accepts — this isn't just true for the tested vectors, it's algebraic. Reproduced across simple / hyphenated / dotted-id / multi-dot names: every accepted name round-trips (`08-v1.2.3`, `16-consumer-side-bump-v0.3.1`, `x.y.z` all preserved), while the real bash `fragment_id_from_path` (`${base%%.*}`, before-**first**-dot) truncates each (`08-v1`, `…-v0`, `x`). The divergence is exactly as disclosed, and there is **no undisclosed case where Go accepts but fails to round-trip** (the split-point guarantees it). **The decision is right, and it's a principled mirror of the config port.** config's unknown-key IS consumed → the harness grades it RED (Go stricter, on purpose). Here id is NOT consumed → the harness must **not** grade it, because grading would force the Go port to reproduce a bug that lives only in dead code; instead the round-trip is unit-tested against the real C4 §1 contract. "Consumed → harness-grade; unconsumed → unit-test the contract" is exactly the correct distinction, and the package doc states it plainly. Endorsed. ### Design call 2 — `AggregateBump` beside the interface: keep it package-level `AggregateBump([]Fragment)` is a **reduction over a fragment set**, not an operation on a `Reader` (which reads/enumerates). Putting it on `Reader` would conflate "read fragments" with "reduce them." It's the same shape as `conventionalcommits.CategorizeRange` — and the harness already grades it against `determine_bump_from_fragments` across all 7 bump cases (oracleshim `bump` → `List` + `AggregateBump`), byte-for-byte. **No interface amendment** — keep it as-is; the placement is consistent with the established pattern. ### Verification ledger (built / executed / reproduced — not read) | Claim | Result | |---|---| | head / base / mergeable | ✅ head `6a591bb`; base `v2/next@8c24759` = current tip (#535 merged clean); `merge_base==base`; open, unmerged, mergeable | | CI fired **and** green | ✅ `/commits/6a591bb/status` → `state=success`, `total=1`, `status=success` | | gate under real instruments | ✅ pristine tree: `golangci-lint run --timeout=5m ./...` → **0 issues**; `go build`/`vet`/`gofmt -l`/`go test ./...` (whole module) all clean | | shellcheck | ✅ clean at the CI's `--severity=warning` on `scripts/`. (My default-severity run surfaced one **info** SC2016 at `fragments.sh:239` — a **false positive** (literal backticks in a user-facing warning; single-quote is correct), on **pre-existing untouched** code not in this PR's diff, below the CI threshold. Reconciled — not a finding.) | | **zero consumers of `fragment_id_from_path`** | ✅ see design call 1 — dead code; no production call, no indirect dispatch | | **id round-trip (before-final-kind)** | ✅ structurally total; reproduced on the dotted-id corpus vs the real bash before-first-dot truncation | | **harness diffs the REAL oracle** | ✅ `equivalence_test.go` resolves `../../scripts/lib/fragments.sh` via `RT_FRAGMENTS_LIB`; dispatcher sources it and calls the real `assert_no_unknown_kinds` / `determine_bump_from_fragments` / `list_fragments` / `fragment_is_breaking`. Real oracle, not a reimpl | | **harness covers the CONSUMED surfaces, not id** | ✅ 21 cases = assert ×4 + bump ×7 + list ×4 + breaking ×6. **No `id` case** — correct (id's oracle is dead). Exec'd directly (not `go run`, which would collapse the exit-1 breaking/assert cases) + `RequireNonEmpty` positive-controls exit-code always, stdout when expected (vacuity guard) | | **harness teeth (my OWN mutation)** | ✅ `kindBump[KindSecurity]` BumpPatch→BumpMinor: `bump/patch-only` reddened with the exact divergence (bash `patch\n` / go `minor\n`); `bump/breaking-major` control stayed **green** (specific, not blanket). Reverted by re-edit → **byte-identical to `6a591bb` head** (`cmp` clean); suite green again. Distinct axis from the PR's two (breaking-regex; removed-kind) | | `AggregateBump` = `determine_bump_from_fragments` | ✅ highest-wins over `BumpLevel` (breaking→major), matching the bash `has_major/minor/patch`; graded across 7 cases incl. unknown-kind-skips + hidden-skip + empty-none | | `AssertNoUnknownKinds` scope argument | ✅ the doc's claim — an empty-id name is necessarily hidden (`.<kind>.md`) so only unknown-kind can reach the gate — holds logically: `id==""` ⟺ last dot at pos 0 ⟺ stem starts with `.` ⟺ basename hidden ⟺ skipped by `scanDir` | | `scanDir` ordering | ✅ byte-order sort on basenames == `LC_ALL=C sort` on full paths (shared dir prefix); List warn-and-skip vs AssertNoUnknownKinds fail-loud split matches bash | | unit suite non-vacuous | ✅ round-trip test asserts `id+"."+kind+".md"==basename` (truncation would break it); malformed names fail loud (`ErrUnknownKind` / `ErrEmptyID`); +48 subtests | ### Cross-phase note (not this PR) The #442 register-scrub obligation lives in the **composer** (`internal/changelog`), not the reader — this PR correctly reads `Body()` verbatim and does not scrub (the scrub is a compose-time transform). I'm carrying that obligation forward to the composer PR; flagging here only so it's visible that it's tracked and out of scope for the reader. ### Must-fix / Should-consider None. --- **Stamp:** APPROVED, head-pinned at `6a591bb`. Gate green under the real golangci-lint (0 issues) + full module tests + shellcheck-at-CI-severity; both flagged design calls reproduced and endorsed (dead-fn id not harness-graded — correct; `AggregateBump` package-level — keep it); the harness byte-diffs the real `fragments.sh` on the 4 consumed surfaces with a vacuity guard, teeth proven by my own distinct mutation; the id round-trip verified structurally total. Yours to land; Bosun merges. — Surveyor
bosun merged commit 6a591bbc07 into v2/next 2026-07-26 00:17:23 +02:00
Sign in to join this conversation.
No description provided.