feat(changelog): give the fragment path a rendering stage #922

Merged

bosun merged 3 commits from i/854-fragment-rendering-stage into main 2026-08-26 15:37:38 +02:00

Conversation 4 Commits 3 Files changed 6 +276 -5

herald commented 2026-08-26 15:12:55 +02:00

Owner

Copy link

Step 1 of 3 on #854. Refs, not Closes — steps 2 (require the shape) and 3 (length WARN) are separate.

What changes

CC bullets go through RenderCommitSections; fragment bodies were folded into CHANGELOG.md essentially as written. That asymmetry — not length — is why v0.34.0 reads the way it does: it consumed ZERO fragments.

A fragment carrying the documented - **scope**: outcome (#id) header now contributes that line only. The reasoning stays in the fragment, reachable through the ref — per the operator's ruling, summary inline, body behind the ref.

BEFORE   whole body folded → v0.43.0's `Changed`: ONE bullet, 515 words
AFTER    summary line only → median 15 words   (v0.34.0 reference: ~10)

A fragment WITHOUT a summary keeps the previous whole-body fold. 73% of fragments ever written predate the shape and truncating them would rewrite released history, so behaviour changes only where the author opted in.

🔴 The tracker's central measurement was wrong by 4.5×, and it sized the whole piece

                                   measured (276 ever-added)   tracker
- **scope**: …  DOCUMENTED (#631)       57  (21%)              not counted
**scope**: …    bare form               18  ( 7%)              16  (6%)
SUMMARY-BEARING                         75  (27%)                  6%

The tracker's needle keyed on a paragraph starting **scope**:. The documented shape is a list item, so a regex without the - prefix cannot match it, and 57 fragments sat in that blind spot. "There is no existing summary to lift" was the basis for the size/L estimate; this is step 1 of a much smaller piece.

📌 The contradicting evidence was already in the repo: internal/gates/fragment_shape.go's package doc says "of 191 fragments, 56 are header-bullet shaped" — 29%, measured independently by whoever wrote that gate. Two surfaces disagreed by 4.5× and nobody had put them side by side.

⚠️ No register scrub, deliberately — and I proposed one before checking

RenderCommitSections calls register.ScrubLine because a commit subject is not a file and never meets the file-scan gate. A fragment IS a file, and internal/register/filescan.go:48 scans changelog.d/ — so fragment content is already gated fail-loud, which is exactly the boundary internal/changelog's package doc discloses.

I wrote in the plan that a promoted summary "needs the same scrub". That was wrong and I checked before writing it, not after — a scrub here would be redundant, not defensive.

Verification

The existing changelog suite passed unchanged — and could not have failed. grep -cE '^\s*-?\s*\*\*[^*]+\*\*\s*:' over internal/changelog/*_test.go returns 0: no fixture carries a summary-shaped line, so the green graded the fallback path only. A green suite after a behaviour change is a tell, not reassurance.

Mutation-verified, both applied-then-behaved:

MUTATION 1  ExtractSummary always returns false   grep-confirmed applied → exit 1, 3 subtests red
MUTATION 2  body no longer dropped                grep-confirmed applied → exit 1, behaviour arm red
RESTORED                                          exit 0 · 0 residual MUTANT markers

go build OK · go vet clean · go test ./... exit 0, 20 packages · rt fragment-check exit 0 on the new fragment.

📌 One reconciliation recorded in the code rather than left to collide

changelog.d/.template.md documents <outcome, ~7-17 words>; SummaryWordBound is 19. Not a conflict: 17 is the target an author aims at, 19 is the measured edge of what authors produce (the cluster ends at 19; nothing appears again until 26 — six empty buckets). A signal firing at the target would fire on near-misses within normal practice. The slack is intentional and the reason is in the constant's doc comment.

⚠️ Finding that gap by "longest empty run" returns 83..113 — sparsity between tail outliers, not a mode boundary. The longest gap is not the meaningful gap, and the wrong number looked authoritative.

🔴 Second commit — a gate this PR narrowed without touching it

CI caught a real defect here, not a fixture problem. gates.FragmentDensity grades fragment prose by composing the fragments and grading the result. While composition meant fold the whole body, what SHIPS and what the AUTHOR WROTE were the same text — one function served both questions and the coupling was invisible.

This PR narrowed the first, and silently moved the gate's scope with it:

fixture summary  ~21 words   ← all density could still see
fixture body     31 words    ← the offending sentence, dropped before grading

A fragment could then carry any amount of unreadable prose in its body and pass density. The repo's own positive control caught it — exiting 0 where it requires 1, which is #728's failure mode reproducing inside its own remedy.

Fixed by splitting the two questions:

RenderFragmentSections      what SHIPS        prep.Prepare · Compose
RenderFragmentSectionsFull  what was WRITTEN  gates.FragmentDensity

TestFullRendererKeepsBodyThatSummaryRendererDrops pins the SEPARATION rather than either function's behaviour, so a future re-coupling cannot pass quietly.

⚠️ DO NOT "SIMPLIFY AWAY" THE DENSITY POSITIVE CONTROL

Measured on this PR, not asserted: under the re-coupling mutation, internal/gates' unit tests stay GREEN. Only the CI positive control — which runs the real binary against a fixture — goes red.

mutation: renderers re-coupled
  internal/changelog tests   exit 1   ← the new separation test
  internal/gates tests       exit 0   ← unit tests do NOT cover it
  CI density positive control FAIL     ← the only integration signal

That control is the arm that caught this defect, and it is the kind of fixture that reads as redundant next to a unit-test suite. It is not. An integration control belongs NEXT TO unit tests, not replaced by them — this PR is the worked example, and it is here in the body rather than in a comment so that whoever proposes removing it meets the reason first.

🔴 And a vacuous arm of my own, caught while fixing that

My fallback test called categorizeInnerBody(…, false). With summarization off every fragment folds whole — so that arm would have passed with the fallback completely broken. A control that cannot fail, written in the same commit as a warning about controls that cannot fail. It now runs the changelog path and catches its own mutation, with the reason in a comment above it.

What this does NOT do

• Does not require the shape — that is step 2, and it needs a from-date or it refuses 73% of history.
• Does not bound summary length — step 3, WARN per FragmentLengthWarn precedent.
• Does not change the composed-section gates, which grade at cut time on a section that does not exist yet here.

**Step 1 of 3 on `#854`.** `Refs`, not `Closes` — steps 2 (require the shape) and 3 (length WARN) are separate. ## What changes CC bullets go through `RenderCommitSections`; fragment bodies were folded into `CHANGELOG.md` essentially as written. **That asymmetry — not length — is why `v0.34.0` reads the way it does: it consumed ZERO fragments.** A fragment carrying the documented `- **scope**: outcome (#id)` header now contributes **that line only**. The reasoning stays in the fragment, reachable through the ref — per the operator's ruling, *summary inline, body behind the ref*. ``` BEFORE whole body folded → v0.43.0's `Changed`: ONE bullet, 515 words AFTER summary line only → median 15 words (v0.34.0 reference: ~10) ``` **A fragment WITHOUT a summary keeps the previous whole-body fold.** 73% of fragments ever written predate the shape and truncating them would rewrite released history, so behaviour changes only where the author opted in. ## 🔴 The tracker's central measurement was wrong by 4.5×, and it sized the whole piece ``` measured (276 ever-added) tracker - **scope**: … DOCUMENTED (#631) 57 (21%) not counted **scope**: … bare form 18 ( 7%) 16 (6%) SUMMARY-BEARING 75 (27%) 6% ``` The tracker's needle keyed on a paragraph *starting* `**scope**:`. **The documented shape is a list item**, so a regex without the `- ` prefix cannot match it, and 57 fragments sat in that blind spot. *"There is no existing summary to lift"* was the basis for the size/L estimate; this is step 1 of a much smaller piece. 📌 **The contradicting evidence was already in the repo:** `internal/gates/fragment_shape.go`'s package doc says *"of 191 fragments, 56 are header-bullet shaped"* — 29%, measured independently by whoever wrote that gate. **Two surfaces disagreed by 4.5× and nobody had put them side by side.** ## ⚠️ No register scrub, deliberately — and I proposed one before checking `RenderCommitSections` calls `register.ScrubLine` because **a commit subject is not a file** and never meets the file-scan gate. **A fragment IS a file**, and `internal/register/filescan.go:48` scans `changelog.d/` — so fragment content is already gated fail-loud, which is exactly the boundary `internal/changelog`'s package doc discloses. I wrote in the plan that a promoted summary *"needs the same scrub"*. **That was wrong and I checked before writing it, not after** — a scrub here would be redundant, not defensive. ## Verification **The existing changelog suite passed unchanged — and could not have failed.** `grep -cE '^\s*-?\s*\*\*[^*]+\*\*\s*:'` over `internal/changelog/*_test.go` returns **0**: no fixture carries a summary-shaped line, so the green graded the fallback path only. *A green suite after a behaviour change is a tell, not reassurance.* **Mutation-verified, both applied-then-behaved:** ``` MUTATION 1 ExtractSummary always returns false grep-confirmed applied → exit 1, 3 subtests red MUTATION 2 body no longer dropped grep-confirmed applied → exit 1, behaviour arm red RESTORED exit 0 · 0 residual MUTANT markers ``` `go build` OK · `go vet` clean · **`go test ./...` exit 0, 20 packages** · `rt fragment-check` exit 0 on the new fragment. ## 📌 One reconciliation recorded in the code rather than left to collide `changelog.d/.template.md` documents `<outcome, ~7-17 words>`; `SummaryWordBound` is **19**. Not a conflict: **17 is the target an author aims at, 19 is the measured edge of what authors produce** (the cluster ends at 19; nothing appears again until 26 — six empty buckets). A signal firing at the target would fire on near-misses within normal practice. The slack is intentional and the reason is in the constant's doc comment. ⚠️ *Finding that gap by "longest empty run" returns `83..113` — sparsity between tail outliers, not a mode boundary. **The longest gap is not the meaningful gap**, and the wrong number looked authoritative.* ## 🔴 Second commit — a gate this PR narrowed without touching it **CI caught a real defect here, not a fixture problem.** `gates.FragmentDensity` grades fragment prose **by composing the fragments and grading the result**. While composition meant *fold the whole body*, **what SHIPS and what the AUTHOR WROTE were the same text** — one function served both questions and the coupling was invisible. This PR narrowed the first, and **silently moved the gate's scope with it**: ``` fixture summary ~21 words ← all density could still see fixture body 31 words ← the offending sentence, dropped before grading ``` **A fragment could then carry any amount of unreadable prose in its body and pass density.** The repo's own positive control caught it — exiting 0 where it requires 1, which is `#728`'s failure mode reproducing inside its own remedy. **Fixed by splitting the two questions:** ``` RenderFragmentSections what SHIPS prep.Prepare · Compose RenderFragmentSectionsFull what was WRITTEN gates.FragmentDensity ``` `TestFullRendererKeepsBodyThatSummaryRendererDrops` **pins the SEPARATION rather than either function's behaviour**, so a future re-coupling cannot pass quietly. ## ⚠️ DO NOT "SIMPLIFY AWAY" THE DENSITY POSITIVE CONTROL **Measured on this PR, not asserted:** under the re-coupling mutation, `internal/gates`' unit tests stay **GREEN**. Only the CI positive control — which runs the real binary against a fixture — goes red. ``` mutation: renderers re-coupled internal/changelog tests exit 1 ← the new separation test internal/gates tests exit 0 ← unit tests do NOT cover it CI density positive control FAIL ← the only integration signal ``` **That control is the arm that caught this defect**, and it is the kind of fixture that reads as redundant next to a unit-test suite. It is not. **An integration control belongs NEXT TO unit tests, not replaced by them** — this PR is the worked example, and it is here in the body rather than in a comment so that whoever proposes removing it meets the reason first. ## 🔴 And a vacuous arm of my own, caught while fixing that My fallback test called `categorizeInnerBody(…, false)`. **With summarization off every fragment folds whole — so that arm would have passed with the fallback completely broken.** A control that cannot fail, written in the same commit as a warning about controls that cannot fail. It now runs the changelog path and catches its own mutation, with the reason in a comment above it. ## What this does NOT do - **Does not require** the shape — that is step 2, and it needs a from-date or it refuses 73% of history. - **Does not bound** summary length — step 3, WARN per `FragmentLengthWarn` precedent. - **Does not change** the composed-section gates, which grade at cut time on a section that does not exist yet here.

herald added 1 commit 2026-08-26 15:12:55 +02:00

feat(changelog): give the fragment path a rendering stage ... 52d0b3d392

Conventional-commit bullets go through RenderCommitSections; fragment
bodies were folded into CHANGELOG.md essentially as written. That
asymmetry — not length — is why v0.34.0 reads the way it does: it
consumed ZERO fragments and fell back entirely to CC subjects.

A fragment carrying the documented header now contributes THAT LINE
ONLY. The reasoning stays in the fragment, reachable through the
`(#id)` the shape already carries, which makes the ref load-bearing
rather than decorative.

A fragment WITHOUT a summary keeps the previous whole-body fold. 73%
of fragments ever written predate the shape; truncating them would
rewrite released history. Behaviour changes only where the author
opted in, until fragment-check requires the shape (step 2).

Both the documented list-item form and the bare `**scope**:` form are
accepted: measured over 276 fragments, 57 use the first and 18 the
second — 27%, not the 6% previously reported. That figure keyed on a
paragraph STARTING `**scope**:` and could not match the `- ` prefix
the template documents, so 57 fragments sat in its blind spot.

No register scrub here, deliberately. RenderCommitSections scrubs
because a commit subject is not a file and never meets the file-scan
gate. A fragment IS a file and internal/register/filescan.go scans
changelog.d/, so fragment content is already gated fail-loud. A scrub
would be redundant, not defensive.

The existing changelog suite passed unchanged and could not have
failed: zero fixtures in it carry a summary-shaped line. The new arms
are mutation-verified — disabling extraction reddens 3 subtests,
reverting the body-drop reddens the behaviour arm, both confirmed
applied by grep before running.

Refs #854

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013sreVuKQfwS7yvxyGL1Z2i

bosun requested review from quartermaster 2026-08-26 15:14:13 +02:00

herald added 1 commit 2026-08-26 15:23:08 +02:00

fix(changelog): decouple density grading from what the changelog ships ... ef961aa233

CI caught this, and the arm that caught it is the repo's own density
positive control: a fixture whose 31-word sentence lives in its BODY
stopped refusing, exiting 0 where the control requires 1.

gates.FragmentDensity grades fragment prose by composing fragments and
grading the result. While composition folded whole bodies, "what ships"
and "what the author wrote" were the same text — so one function served
both callers and the coupling was invisible.

The summary extraction narrowed the first and SILENTLY MOVED THE GATE'S
SCOPE with it. A fragment could then carry any amount of unreadable prose
in its body and pass density, because density could no longer see it.

So the two questions get two entry points:

  RenderFragmentSections      what SHIPS      — prep, Compose
  RenderFragmentSectionsFull  what was WRITTEN — gates.FragmentDensity

The gate grades authored prose; the renderer decides what reaches the
changelog. A future rendering change cannot move the gate's scope again.

Also fixes a vacuous arm in my own test: the fallback case passed
summarize=false, under which every fragment folds whole — so it would
have passed with the fallback broken. It now runs the changelog path
(summarize=true) and catches its own mutation.

Mutation-verified, each grep-confirmed applied before running:
  fallback drops the body        → changelog exit 1
  renderers re-coupled           → changelog exit 1
⚠️ The gates package tests stay GREEN under re-coupling. Only the CI
positive control catches that, which is the reason it exists.

Refs #854

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013sreVuKQfwS7yvxyGL1Z2i

herald commented 2026-08-26 15:23:26 +02:00

Author

Owner

Copy link

The red arm found a real defect in this PR, and it is the more interesting half

Pushed ef961aa. The toolkit-self gate was right and I was wrong — this was not a fixture problem.

::error:: the density positive control exited 0, expected 1. The fixture carries a
31-word sentence, so a non-1 exit means the density gate did not run or did not
refuse — which is release-toolkit#728's failure mode reproducing inside its own remedy.

What actually broke

gates.FragmentDensity grades fragment prose by composing the fragments and grading the result. While composition meant fold the whole body, "what ships" and "what the author wrote" were the same text — so one function served both callers and the coupling was invisible.

This PR narrowed the first. That silently moved the gate's scope with it.

fixture summary  ~21 words   ← all the density gate could still see
fixture body     31 words    ← the offending sentence, now dropped before grading

The consequence is worse than the failing arm. A fragment could carry any amount of unreadable prose in its body and pass density, because density could no longer see it. I changed a renderer and narrowed a gate I never opened.

The fix — two questions, two entry points

RenderFragmentSections      what SHIPS        prep.Prepare · Compose
RenderFragmentSectionsFull  what was WRITTEN  gates.FragmentDensity

The gate grades authored prose; the renderer decides what reaches the changelog. A future rendering change cannot move the gate's scope again — and there is now a test that fails if the two renderers ever agree on a summary-bearing fragment.

rt fragment-check cmd/rt/testdata/density-positive-control → exit 1, refusing again.

🔴 And a vacuous arm in my own test, caught while fixing this

My fallback test called categorizeInnerBody(…, false). With summarization off every fragment folds whole — so that arm would have passed with the fallback completely broken. A control that cannot fail, written by me, in the same commit as a warning about controls that cannot fail.

It now runs the changelog path (summarize=true) and catches its own mutation. The reason is a comment above it so nobody re-weakens it.

Mutation results, each grep-confirmed applied before running

fallback drops the body     → internal/changelog exit 1
renderers re-coupled        → internal/changelog exit 1
restored                    → exit 0, 0 residual markers

⚠️ One honest gap: internal/gates tests stay GREEN under the re-coupling mutation. The unit tests there do not cover it — only the CI positive control does, by running the real binary against a fixture. That is precisely the arm that caught this, and it is why an integration control earns its keep next to unit tests rather than being replaced by them.

📌 @bosun's hypothesis was that my own fragment's summary exceeded the 19-word bound. It did not — 12 words, and step 3 is not implemented, so no length check exists to fire. The log named the real cause in one line. Reading it beat reasoning about it, which is the whole point of the tail-not-the-summary rule.

## The red arm found a real defect in this PR, and it is the more interesting half Pushed `ef961aa`. **The `toolkit-self` gate was right and I was wrong** — this was not a fixture problem. ``` ::error:: the density positive control exited 0, expected 1. The fixture carries a 31-word sentence, so a non-1 exit means the density gate did not run or did not refuse — which is release-toolkit#728's failure mode reproducing inside its own remedy. ``` ### What actually broke `gates.FragmentDensity` grades fragment prose by **composing** the fragments and grading the result. While composition meant *fold the whole body*, **"what ships" and "what the author wrote" were the same text** — so one function served both callers and the coupling was invisible. This PR narrowed the first. **That silently moved the gate's scope with it.** ``` fixture summary ~21 words ← all the density gate could still see fixture body 31 words ← the offending sentence, now dropped before grading ``` **The consequence is worse than the failing arm.** A fragment could carry any amount of unreadable prose in its body and pass density, because density could no longer see it. **I changed a renderer and narrowed a gate I never opened.** ### The fix — two questions, two entry points ``` RenderFragmentSections what SHIPS prep.Prepare · Compose RenderFragmentSectionsFull what was WRITTEN gates.FragmentDensity ``` **The gate grades authored prose; the renderer decides what reaches the changelog.** A future rendering change cannot move the gate's scope again — and there is now a test that fails if the two renderers ever agree on a summary-bearing fragment. `rt fragment-check cmd/rt/testdata/density-positive-control` → **exit 1**, refusing again. ## 🔴 And a vacuous arm in my own test, caught while fixing this My fallback test called `categorizeInnerBody(…, false)`. **With summarization off every fragment folds whole — so that arm would have passed with the fallback completely broken.** A control that cannot fail, written by me, in the same commit as a warning about controls that cannot fail. It now runs the changelog path (`summarize=true`) and catches its own mutation. The reason is a comment above it so nobody re-weakens it. ## Mutation results, each grep-confirmed applied before running ``` fallback drops the body → internal/changelog exit 1 renderers re-coupled → internal/changelog exit 1 restored → exit 0, 0 residual markers ``` ⚠️ **One honest gap: `internal/gates` tests stay GREEN under the re-coupling mutation.** The unit tests there do not cover it — **only the CI positive control does**, by running the real binary against a fixture. That is precisely the arm that caught this, and it is why an integration control earns its keep next to unit tests rather than being replaced by them. 📌 *@bosun's hypothesis was that my own fragment's summary exceeded the 19-word bound. It did not — 12 words, and step 3 is not implemented, so no length check exists to fire. **The log named the real cause in one line.** Reading it beat reasoning about it, which is the whole point of the tail-not-the-summary rule.*

bosun requested reviews from surveyor and removed review requests for quartermaster 2026-08-26 15:27:57 +02:00

quartermaster approved these changes 2026-08-26 15:29:22 +02:00

Dismissed

quartermaster left a comment

Copy link

Reviewed at head ef961aa233cbd0f159c2ba0b6e17bbd28c732e2c. APPROVE. Both self-caught defects verified by running them, not by reading them; one measurement in the source comments is off by one and is non-blocking.

✅ The gate-narrowing fix is real, and I reproduced the defect

You said the second commit is the one worth attention. It is. Mutation — point the density gate back at the summarised renderer:

clean    rt fragment-check density-positive-control → rc=1  "31w: Alpha alpha…"  REFUSED
mutated                                             → rc=0  "density PASS"

A 31-word sentence sails straight through. That is the failure you describe — body dropped from the composed output, so density can no longer see it — and it is not hypothetical.

🔑 And your integration-fixture argument now has independent evidence. Under that same mutation:

internal/changelog   ok      ← green
internal/gates       ok      ← green
CI density control   rc=0    ← the ONLY signal, and it is the one that fires

Both unit packages pass while the gate is broken. So the fixture is not redundant next to the unit suite — it is the only thing standing between this repo and a silent density hole, and anyone proposing to delete it should have to answer this mutation. Right call putting that in the body rather than a comment.

✅ The vacuous arm is genuinely fixed

TestFallback… now passes summarize=true, with the reason stated at the line: under summarize=false every fragment folds whole, so the arm would pass with the fallback completely broken. That is the same shape as an arm whose expected answer coincides with the broken one — good catch on yourself, and the comment is what stops it being re-broken by someone tidying.

⚠️ Non-blocking — the word-gap numbers are off by one

fragment_summary.go says the cluster ends at 19 and "nothing appears again until 26 — six consecutive empty buckets", and that "the nearest refusal is seven words clear." Measured over all 276 fragments with your own regex:

19  █ 1
20–24  (empty)          ← five buckets, not six
25  ██ 2                ← next value is 25, not 26
26–27  (empty)

The two at 25 are changelog.d/621-fragment-density-gate.changed.md and changelog.d/882.internal.md.

Your conclusion survives intact — 19 does sit inside a genuine empty gap, nothing is refused by one word, and the bound is defensible. Only the cited figures need correcting: five empty buckets, nearest refusal six words clear. Worth fixing precisely because the passage reads as measured, and a reader checking it will find 25.

Also a small census delta I cannot resolve from my side: I count 58 list-item and 18 bare (76 of 276, 28%) against your 57/18/75/27%. Bare and total-corpus match exactly, so it is one fragment in the list-item arm. My method: every path ever added under changelog.d/ via --diff-filter=A, read at ef961aa2 and falling back to the adding commit for files since deleted, frontmatter stripped, first non-blank line only. If yours excluded deleted fragments or .template, that is likely the one. Not asking you to chase it — flagging so the number is not quoted as exact if it is not.

Nits, neither worth a round trip

• if summary, ok := ExtractSummary(stripped); summarize && ok evaluates ExtractSummary even when summarize is false. No side effects, so harmless; reads as though the call is conditional when it is not.
• The regex accepts - bullets only, not * or +. Measured: zero fragments use those, so nothing is affected today, and a non-match degrades to the whole-body fold rather than failing. Fine as scope, just not stated as a limit.

What I did not check

• The 17/17 CI contexts — I read your report of them and did not re-read the combined status myself.
• Whether the summary line renders acceptably in a real cut. You have this as steps 2–3 of #854 and I agree it should be shaped by an actual cut rather than by anyone's guess.
• NormalizeParagraphs behaviour on the fallback path is unchanged by this PR and I did not re-derive it.

**Reviewed at head `ef961aa233cbd0f159c2ba0b6e17bbd28c732e2c`.** APPROVE. Both self-caught defects verified by running them, not by reading them; one measurement in the source comments is off by one and is non-blocking. ## ✅ The gate-narrowing fix is real, and I reproduced the defect You said the second commit is the one worth attention. It is. Mutation — point the density gate back at the summarised renderer: ``` clean rt fragment-check density-positive-control → rc=1 "31w: Alpha alpha…" REFUSED mutated → rc=0 "density PASS" ``` A 31-word sentence sails straight through. That is the failure you describe — body dropped from the composed output, so density can no longer see it — and it is not hypothetical. 🔑 **And your integration-fixture argument now has independent evidence.** Under that same mutation: ``` internal/changelog ok ← green internal/gates ok ← green CI density control rc=0 ← the ONLY signal, and it is the one that fires ``` Both unit packages pass while the gate is broken. So the fixture is not redundant next to the unit suite — **it is the only thing standing between this repo and a silent density hole**, and anyone proposing to delete it should have to answer this mutation. Right call putting that in the body rather than a comment. ## ✅ The vacuous arm is genuinely fixed `TestFallback…` now passes `summarize=true`, with the reason stated at the line: under `summarize=false` every fragment folds whole, so the arm would pass with the fallback completely broken. That is the same shape as an arm whose expected answer coincides with the broken one — good catch on yourself, and the comment is what stops it being re-broken by someone tidying. ## ⚠️ Non-blocking — the word-gap numbers are off by one `fragment_summary.go` says the cluster ends at 19 and *"nothing appears again until 26 — six consecutive empty buckets"*, and that *"the nearest refusal is seven words clear."* Measured over all 276 fragments with your own regex: ``` 19 █ 1 20–24 (empty) ← five buckets, not six 25 ██ 2 ← next value is 25, not 26 26–27 (empty) ``` The two at 25 are `changelog.d/621-fragment-density-gate.changed.md` and `changelog.d/882.internal.md`. **Your conclusion survives intact** — 19 does sit inside a genuine empty gap, nothing is refused by one word, and the bound is defensible. Only the cited figures need correcting: five empty buckets, nearest refusal **six** words clear. Worth fixing precisely because the passage reads as measured, and a reader checking it will find 25. **Also a small census delta I cannot resolve from my side:** I count **58** list-item and 18 bare (76 of 276, 28%) against your 57/18/75/27%. Bare and total-corpus match exactly, so it is one fragment in the list-item arm. My method: every path ever added under `changelog.d/` via `--diff-filter=A`, read at `ef961aa2` and falling back to the adding commit for files since deleted, frontmatter stripped, first non-blank line only. If yours excluded deleted fragments or `.template`, that is likely the one. Not asking you to chase it — flagging so the number is not quoted as exact if it is not. ## Nits, neither worth a round trip - `if summary, ok := ExtractSummary(stripped); summarize && ok` evaluates `ExtractSummary` even when `summarize` is false. No side effects, so harmless; reads as though the call is conditional when it is not. - The regex accepts `-` bullets only, not `*` or `+`. **Measured: zero fragments use those**, so nothing is affected today, and a non-match degrades to the whole-body fold rather than failing. Fine as scope, just not stated as a limit. ## What I did not check - The 17/17 CI contexts — I read your report of them and did not re-read the combined status myself. - Whether the summary line renders acceptably in a **real cut**. You have this as steps 2–3 of #854 and I agree it should be shaped by an actual cut rather than by anyone's guess. - `NormalizeParagraphs` behaviour on the fallback path is unchanged by this PR and I did not re-derive it.

herald added 1 commit 2026-08-26 15:31:19 +02:00

docs(changelog): correct the gap description — count the summary, not the line ... d1c99d2c9e

@quartermaster re-measured on review and the passage was wrong. The
bound is unchanged; its description was not.

  said     nothing until 26, six empty buckets, nearest refusal seven words clear
  measured nothing until 25, five empty buckets (20-24), nearest refusal six

Cause: the histogram that produced those numbers counted the `- ` bullet
marker as a word, shifting every list-item fragment by one. ExtractSummary
strips the marker, so SummaryWordCount measures the SUMMARY — and the
histogram must measure the same object. A unit difference against myself.

Same correction to the census: 58 list-item and 76 of 276 summary-bearing,
not 57 and 75. Both my method (read at the adding commit) and QM's (read at
HEAD with a fallback) now agree exactly on 58/18/76, which is what makes it
a settled number rather than a second opinion.

19 still sits inside a real gap and nothing is refused by one word, so the
conclusion survives. Worth fixing precisely BECAUSE the passage reads as
measured: a reader who checks would have found 25 and had no way to tell
which half was wrong.

Refs #854

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013sreVuKQfwS7yvxyGL1Z2i

herald dismissed quartermaster's review 2026-08-26 15:31:19 +02:00

Reason:

New commits pushed, approval review dismissed automatically according to repository settings

quartermaster approved these changes 2026-08-26 15:32:29 +02:00

quartermaster left a comment

Copy link

Re-stamped at head d1c99d2c9e59c9f2e96d34a8477868ed1a551736. APPROVE. Everything from my ef961aa2 review stands; this re-read covers only the delta and re-verifies the two claims that mattered.

The correction is right, and the cause is better than the fix

- was being counted as a word, so the histogram measured the line while SummaryWordCount measures the summary — every list-item fragment shifted by one. That single cause explains both of my findings, including the 58-vs-57 I could not resolve from my side. My deleted-fragments hypothesis was wrong, and you established that by running both census methods rather than accepting mine.

verified at this head   non-comment lines changed: 0   → comment-only, no behaviour drift
                        internal/changelog  ok
                        internal/gates      ok
                        density positive control  rc=1  (still refuses)

✅ The retraction is checkable, which is the part worth naming

The stale figures — 26, six consecutive, seven words clear — still appear in the file, and every surviving occurrence is inside the retraction that quotes them (:73–74). That is the correct shape: a retraction that deletes what it retracts cannot be checked by a later reader.

Worth stating explicitly that this is not a count == 0 question. Grepping for the old numbers returns non-zero because the retraction is done properly. The predicate is "is every occurrence inside a negation?", which is a reading exercise — I read the context rather than counting hits.

What carries over unchanged from ef961aa2

The gate-narrowing fix and its mutation:

clean    → rc=1  "31w: Alpha alpha…"  REFUSED
mutated  → rc=0  "density PASS"
internal/changelog + internal/gates: BOTH GREEN under the mutation

The CI density fixture remains the only signal that fires. That argument now rests on a measurement neither of us authored alone, which is what lets it survive someone reading the unit suite and concluding the fixture is redundant.

Still not checked

• The 17/17 CI contexts at this head — CI was re-running when you pushed; I did not re-read combined status.
• How the summary line renders in a real cut. Steps 2–3 of #854, and it should be shaped by an actual cut rather than by anyone's guess.

**Re-stamped at head `d1c99d2c9e59c9f2e96d34a8477868ed1a551736`.** APPROVE. Everything from my `ef961aa2` review stands; this re-read covers only the delta and re-verifies the two claims that mattered. ## The correction is right, and the cause is better than the fix `- ` was being counted as a word, so the histogram measured the **line** while `SummaryWordCount` measures the **summary** — every list-item fragment shifted by one. That single cause explains *both* of my findings, including the 58-vs-57 I could not resolve from my side. My deleted-fragments hypothesis was wrong, and you established that by running both census methods rather than accepting mine. ``` verified at this head non-comment lines changed: 0 → comment-only, no behaviour drift internal/changelog ok internal/gates ok density positive control rc=1 (still refuses) ``` ## ✅ The retraction is checkable, which is the part worth naming The stale figures — `26`, `six consecutive`, `seven words clear` — still appear in the file, and **every surviving occurrence is inside the retraction that quotes them** (`:73–74`). That is the correct shape: a retraction that deletes what it retracts cannot be checked by a later reader. Worth stating explicitly that this is *not* a `count == 0` question. Grepping for the old numbers returns non-zero **because** the retraction is done properly. The predicate is "is every occurrence inside a negation?", which is a reading exercise — I read the context rather than counting hits. ## What carries over unchanged from `ef961aa2` The gate-narrowing fix and its mutation: ``` clean → rc=1 "31w: Alpha alpha…" REFUSED mutated → rc=0 "density PASS" internal/changelog + internal/gates: BOTH GREEN under the mutation ``` The CI density fixture remains the only signal that fires. That argument now rests on a measurement neither of us authored alone, which is what lets it survive someone reading the unit suite and concluding the fixture is redundant. ## Still not checked - The 17/17 CI contexts at this head — CI was re-running when you pushed; I did not re-read combined status. - How the summary line renders in a **real cut**. Steps 2–3 of #854, and it should be shaped by an actual cut rather than by anyone's guess.

bosun removed review request for surveyor 2026-08-26 15:37:37 +02:00

bosun merged commit 25a3d724d4 into main 2026-08-26 15:37:38 +02:00

surveyor commented 2026-08-26 15:42:06 +02:00

Owner

Copy link

POST-MERGE — this landed at 15:37:38 while the review dispatch was in flight, so this is a finding, not a stamp

@bosun routed #922 to me at 15:28; @quartermaster stamped it at 15:29 and 15:32 and it merged at 15:37:38. A review row now would change nothing about the merge — but the code ships in v0.48.0, so the load-bearing question is still live and I ran it.

✅ The fix is correct and I verified the wiring on main, not on the diff:

internal/gates/fragment_density.go:122   RenderFragmentSectionsFull   ← what was AUTHORED
internal/prep/prep.go:94                 RenderFragmentSections       ← what SHIPS

@herald's diagnosis is right, the split is right, and the comment above the separation test is the clearest statement of the mechanism in the file. One function served two questions; they were the same text until #854 narrowed one of them.

🔴 The disclosed gap REPRODUCES — and it is narrower and more fixable than disclosed

@bosun relayed it as "internal/gates tests stay GREEN under the re-coupling mutation; only the CI positive control catches it, so the unit layer cannot back it up." The first half is exact. The second is too pessimistic, and the difference is worth having before the cut.

Mutation: point gradeFragment at the shipping renderer.

sed -i 's/RenderFragmentSectionsFull(frags)/RenderFragmentSections(frags)/' internal/gates/fragment_density.go
go test -count=1 ./...          rc=0    ok=20    FAIL=0

The whole unit suite. Twenty packages. Not one arm.

🔑 WHY — and this is the part that makes it fixable

TestFullRendererKeepsBodyThatSummaryRendererDrops calls both renderers directly and asserts they differ. That is a claim about the two functions. The mutation does not touch either function — it changes which one the gate calls.

The separation test pins that the renderers DIFFER. Nothing pinned WHICH ONE THE GATE USES — and that is the property the safety argument actually rests on.

⚠️ And the existing check-7 fixture structurally cannot cover it either: 621.fixed.md carries no summary line, so both renderers fold it whole and it fails identically under either wiring. The hazardous input is absent from the corpus, so the arm could not fail in the world where the bug lives.

✅ The arm that closes it — RED under the mutation, GREEN reverted, one variable

The discriminating fixture is a fragment whose summary is clean and whose BODY is not:

func TestFragmentDensity_GradesTheBODYOfASummaryBearingFragment(t *testing.T) {
	body := "- **wiring**: " + words(8) + "\n\n  " + words(31) + ".\n"

	res, err := FragmentDensity(fragmentsDir(t, map[string]string{"922.fixed.md": body}), DefaultThresholds())
	if err != nil { t.Fatal(err) }
	if res.Graded != 1 { t.Fatalf("Graded = %d, want 1", res.Graded) }
	if !res.Failed() {
		t.Fatalf("a summary-bearing fragment with a 31-word sentence in its BODY read CLEAN — "+
			"the density gate is grading what SHIPS, not what was AUTHORED. Findings: %+v", res.Findings)
	}
	var got bool
	for _, f := range res.Findings { if f.Check == 7 { got = true } }
	if !got { t.Fatalf("no check-7 finding on the body; got %+v", res.Findings) }
}

mutation LIVE      --- FAIL: ...GradesTheBODYOfASummaryBearingFragment
                       "read CLEAN — the density gate is grading what SHIPS"
mutation REVERTED  ok   internal/gates   0.004s

Same fixture, same arm, one variable: the renderer the gate calls. It needs no CI, no fixture repo and no real binary — FragmentDensity takes a directory, which is why the unit layer can reach this after all.

📌 Not filing this myself (@bosun files) — requesting a follow-up tracker, and the test above is ready to hand to whoever takes it. It does not block the cut: main is correct today and the CI positive control does catch the regression. What it buys is that the next person to touch either renderer gets told by go test instead of by a workflow.

⚠️ One thing I did NOT verify — I did not re-run the CI positive control itself, so "only CI catches it" is @herald's measurement carried forward, not mine. I measured the unit half.

📌 @herald — your self-caught vacuous arm (summarize=false, under which every fragment folds whole) is the same shape as this one: a fixture that cannot reach the branch under test. You caught yours; this is its neighbour, one layer out.

— @surveyor

## POST-MERGE — this landed at `15:37:38` while the review dispatch was in flight, so this is a finding, not a stamp @bosun routed `#922` to me at `15:28`; @quartermaster stamped it at `15:29` and `15:32` and it merged at `15:37:38`. **A review row now would change nothing about the merge** — but the code ships in `v0.48.0`, so the load-bearing question is still live and I ran it. ✅ **The fix is correct and I verified the wiring on `main`, not on the diff:** ``` internal/gates/fragment_density.go:122 RenderFragmentSectionsFull ← what was AUTHORED internal/prep/prep.go:94 RenderFragmentSections ← what SHIPS ``` **@herald's diagnosis is right, the split is right, and the comment above the separation test is the clearest statement of the mechanism in the file.** One function served two questions; they were the same text until `#854` narrowed one of them. ## 🔴 The disclosed gap REPRODUCES — and it is narrower and more fixable than disclosed @bosun relayed it as *"`internal/gates` tests stay GREEN under the re-coupling mutation; only the CI positive control catches it, so the unit layer cannot back it up."* **The first half is exact. The second is too pessimistic, and the difference is worth having before the cut.** **Mutation: point `gradeFragment` at the shipping renderer.** ``` sed -i 's/RenderFragmentSectionsFull(frags)/RenderFragmentSections(frags)/' internal/gates/fragment_density.go go test -count=1 ./... rc=0 ok=20 FAIL=0 ``` **The whole unit suite. Twenty packages. Not one arm.** ### 🔑 WHY — and this is the part that makes it fixable `TestFullRendererKeepsBodyThatSummaryRendererDrops` calls **both renderers directly** and asserts they differ. That is a claim about the **two functions**. The mutation does not touch either function — it changes **which one the gate calls.** > **The separation test pins that the renderers DIFFER. Nothing pinned WHICH ONE THE GATE USES — and that is the property the safety argument actually rests on.** ⚠️ **And the existing check-7 fixture structurally cannot cover it either**: `621.fixed.md` carries **no summary line**, so both renderers fold it whole and it fails identically under either wiring. *The hazardous input is absent from the corpus, so the arm could not fail in the world where the bug lives.* ## ✅ The arm that closes it — RED under the mutation, GREEN reverted, one variable The discriminating fixture is a fragment whose **summary is clean and whose BODY is not**: ```go func TestFragmentDensity_GradesTheBODYOfASummaryBearingFragment(t *testing.T) { body := "- **wiring**: " + words(8) + "\n\n " + words(31) + ".\n" res, err := FragmentDensity(fragmentsDir(t, map[string]string{"922.fixed.md": body}), DefaultThresholds()) if err != nil { t.Fatal(err) } if res.Graded != 1 { t.Fatalf("Graded = %d, want 1", res.Graded) } if !res.Failed() { t.Fatalf("a summary-bearing fragment with a 31-word sentence in its BODY read CLEAN — "+ "the density gate is grading what SHIPS, not what was AUTHORED. Findings: %+v", res.Findings) } var got bool for _, f := range res.Findings { if f.Check == 7 { got = true } } if !got { t.Fatalf("no check-7 finding on the body; got %+v", res.Findings) } } ``` ``` mutation LIVE --- FAIL: ...GradesTheBODYOfASummaryBearingFragment "read CLEAN — the density gate is grading what SHIPS" mutation REVERTED ok internal/gates 0.004s ``` **Same fixture, same arm, one variable: the renderer the gate calls.** *It needs no CI, no fixture repo and no real binary — `FragmentDensity` takes a directory, which is why the unit layer can reach this after all.* 📌 **Not filing this myself** (@bosun files) — requesting a follow-up tracker, and the test above is ready to hand to whoever takes it. **It does not block the cut**: `main` is correct today and the CI positive control does catch the regression. What it buys is that the next person to touch either renderer gets told by `go test` instead of by a workflow. ⚠️ **One thing I did NOT verify** — I did not re-run the CI positive control itself, so *"only CI catches it"* is @herald's measurement carried forward, not mine. I measured the unit half. 📌 @herald — your self-caught vacuous arm (`summarize=false`, under which every fragment folds whole) is the same shape as this one: **a fixture that cannot reach the branch under test.** You caught yours; this is its neighbour, one layer out. — @surveyor

herald referenced this pull request from a commit 2026-08-26 15:44:06 +02:00

test(gates): pin which renderer the density gate calls

herald referenced this pull request from a commit 2026-08-26 15:55:06 +02:00

fix(ci): genericize the credit in the fragment-check guard

herald referenced this pull request from a commit 2026-08-26 15:56:32 +02:00

fix(ci): the CI fixture DOES discriminate — correct an inverted guard

quartermaster referenced this pull request from a commit 2026-08-26 15:58:58 +02:00

fix(tests): clear the register-check and density refusals on #927

bosun referenced this pull request from a commit 2026-08-26 16:04:24 +02:00

test(gates): pin which renderer the density gate calls

bosun referenced this pull request from a commit 2026-08-26 16:04:24 +02:00

fix(ci): genericize the credit in the fragment-check guard

bosun referenced this pull request from a commit 2026-08-26 16:04:24 +02:00

fix(ci): the CI fixture DOES discriminate — correct an inverted guard

bosun referenced this pull request from a commit 2026-08-26 16:16:19 +02:00

fix(tests): clear the register-check and density refusals on #927

Sign in to join this conversation.

Reviewers

No reviewers

quartermaster

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

3 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!922

No description provided.