fix: Validate the release-notes rewrite before publishing #80

ahmad · 2026-09-09T18:27:24Z

ahmad commented

2026-09-09 18:27:24 +00:00

Issue

Closes #79

Problem

ahmad/imamah v0.7.1 was published with notes reading "I don't see any commit subjects in your message. Could you please provide the list…". Two faults met: the release was a forced rebake over an empty commit range, so the summariser was asked to summarise nothing; and the step accepted any non-empty answer, so a question passed for release notes.

Solution

An empty range no longer calls the summariser — the notes say plainly that nothing changed and name the release being republished. A summary is accepted only when it carries the headings and bullets the prompt asks for; a question, an apology or a refusal is a failed rewrite and takes the commit-derived notes, with notes-fallback set and the rejecting rule named in the log. The prompt now also tells the model to output nothing when it cannot produce notes.

Review notes

Four behaviour cases cover it, two of them driving the real refusal text from the imamah release. The README gains the rule under The contract, in the general form: a deterministic fallback, a shape check on the generated version, and the fallback whenever the check fails.

The imamah release itself has already been corrected by hand; this stops it recurring.

Risks and trade-offs

The shape check is deliberately structural rather than semantic: it cannot tell good notes from bad ones, only notes from not-notes. A valid summary that omits every heading would now fall back — acceptable, since the prompt demands them and the fallback is always true.

### Issue Closes #79 ### Problem `ahmad/imamah` v0.7.1 was published with notes reading "I don't see any commit subjects in your message. Could you please provide the list…". Two faults met: the release was a forced rebake over an empty commit range, so the summariser was asked to summarise nothing; and the step accepted any non-empty answer, so a question passed for release notes. ### Solution An empty range no longer calls the summariser — the notes say plainly that nothing changed and name the release being republished. A summary is accepted only when it carries the headings and bullets the prompt asks for; a question, an apology or a refusal is a failed rewrite and takes the commit-derived notes, with `notes-fallback` set and the rejecting rule named in the log. The prompt now also tells the model to output nothing when it cannot produce notes. ### Review notes Four behaviour cases cover it, two of them driving the real refusal text from the imamah release. The README gains the rule under **The contract**, in the general form: a deterministic fallback, a shape check on the generated version, and the fallback whenever the check fails. The imamah release itself has already been corrected by hand; this stops it recurring. ### Risks and trade-offs The shape check is deliberately structural rather than semantic: it cannot tell good notes from bad ones, only notes from not-notes. A valid summary that omits every heading would now fall back — acceptable, since the prompt demands them and the fallback is always true.

ahmad added 1 commit

2026-09-09 18:27:24 +00:00

fix: Validate the release-notes rewrite before publishing

PR / validate (pull_request) Successful in 20s

Details

4dad26f395

A model asked to summarise an empty commit range answers with a
question about the missing input, and the step accepted any non-empty
answer — so that question was published as the notes of a release.

Do not call the summariser when there is nothing to summarise, and
accept its answer only when it has the shape the prompt asked for.
Anything else takes the commit-derived notes and is flagged, which is
what the existing fallback was for.

ahmad changed title from ~~WIP: fix: Validate the release-notes rewrite before publishing~~ to fix: Validate the release-notes rewrite before publishing

2026-09-09 18:28:06 +00:00

ahmad requested review from ahmad_bot

2026-09-09 18:28:12 +00:00

ahmad_bot reviewed

2026-09-09 18:51:25 +00:00

ahmad_bot left a comment

One finding, inline, non-blocking — an adjacent case the fix leaves in the state that produced #79. Everything the PR claims for itself is verified.

Verified by running the suite and mutating each guard, at 4dad26f3:

baseline                                    all cases pass   (check-actions also ok)
drop the heading check       -> exit 1, FAIL "a refusal is not release notes"
drop the bullet check        -> exit 1, FAIL "headings without bullets are a half-answer"
drop `[ -n "${SUBJECTS}" ]`  -> exit 1, FAIL "an empty range never asks a model to summarise nothing"
restored                                    all cases pass

Three guards, three cases, each killing exactly one — so none of them is resting on another, which is the failure mode the last two of these had.

Driving the real refusal text is the right call. REFUSAL is the string that shipped as ahmad/imamah v0.7.1's public notes, so the case proves the fix against the input that caused the issue rather than against a plausible stand-in. That is worth keeping as the shape for this kind of fix.
The root cause is genuinely addressed, not just its symptom. SUBJECTS is now computed before the summariser and an empty one skips the call entirely — so the model is never asked to summarise nothing, which is what produced a question instead of notes. The shape check is the second line of defence rather than the fix, and the PR describes it that way.
FALLBACK=false on the empty-range branch is right, and I checked what the flag means before saying so: it signals a failed rewrite, and on an empty range no rewrite was needed. The deterministic sentence naming the republished version is a better answer than the commit list would be.
The set -euo pipefail question on the new unguarded SUBJECTS="$(git log …)" resolves clean: RANGE is either "${PREVIOUS}..HEAD" with PREVIOUS from git describe — so the tag exists locally — or HEAD. Neither makes git log exit non-zero, so the assignment cannot trip set -e.
The README's contract line now states the general rule (deterministic fallback, shape check on the generated version, fallback whenever the check fails) rather than this instance, which is what makes it useful to the next action that calls a model.
CI green (validate).

One finding, inline, non-blocking — an adjacent case the fix leaves in the state that produced `#79`. Everything the PR claims for itself is verified. **Verified by running the suite and mutating each guard**, at `4dad26f3`: baseline all cases pass (check-actions also ok) drop the heading check -> exit 1, FAIL "a refusal is not release notes" drop the bullet check -> exit 1, FAIL "headings without bullets are a half-answer" drop `[ -n "${SUBJECTS}" ]` -> exit 1, FAIL "an empty range never asks a model to summarise nothing" restored all cases pass Three guards, three cases, each killing exactly one — so none of them is resting on another, which is the failure mode the last two of these had. - **Driving the real refusal text is the right call.** `REFUSAL` is the string that shipped as `ahmad/imamah` v0.7.1's public notes, so the case proves the fix against the input that caused the issue rather than against a plausible stand-in. That is worth keeping as the shape for this kind of fix. - **The root cause is genuinely addressed, not just its symptom.** `SUBJECTS` is now computed before the summariser and an empty one skips the call entirely — so the model is never asked to summarise nothing, which is what produced a question instead of notes. The shape check is the second line of defence rather than the fix, and the PR describes it that way. - **`FALLBACK=false` on the empty-range branch is right**, and I checked what the flag means before saying so: it signals a *failed rewrite*, and on an empty range no rewrite was needed. The deterministic sentence naming the republished version is a better answer than the commit list would be. - The `set -euo pipefail` question on the new unguarded `SUBJECTS="$(git log …)"` resolves clean: `RANGE` is either `"${PREVIOUS}..HEAD"` with `PREVIOUS` from `git describe` — so the tag exists locally — or `HEAD`. Neither makes `git log` exit non-zero, so the assignment cannot trip `set -e`. - The README's contract line now states the general rule (deterministic fallback, shape check on the generated version, fallback whenever the check fails) rather than this instance, which is what makes it useful to the next action that calls a model. - CI green (`validate`).

release/action.yml Outdated

					
				@ -178,0 +180,4 @@

				        # nothing to summarise. Asked to summarise nothing, a model answers

				        # about the absence of input, and that answer was once published as the

				        # notes of a release (#79). Say plainly that nothing changed instead.

				        SUBJECTS="$(git log --format='%s' ${RANGE})"

SUBJECTS is the unfiltered log while NOTES one line above is the filtered one, and that asymmetry leaves one case in exactly the state #79 was about.

A force-bump over a range whose commits are all excluded types has non-empty SUBJECTS, so it takes the summariser path — and the model is handed the very subjects the project has decided are not user-facing, to rewrite as user-facing notes. I ran it rather than reasoned it, with a git stub returning chore: Bump deps and docs: Fix a typo:

PROBE: a force-bump over an all-excluded range still calls the model   pass
  -> notes_fallback=false, the model's output published

It is reachable by the same path as #79 and no other: BUMP=none for a chore-only range, so nothing releases unless force-bump: patch — the rebake dispatch. So the two cases are siblings. #79 was force-bump over an empty range; this is force-bump over a range with nothing releasable in it, and the project's own EXCLUDE_TYPES rule is the thing saying so.

What happens then is not obviously wrong — a model will turn "chore: Bump deps" into "### Improvements — Updated some dependencies", which passes the shape check and is arguably better than the fallback, which here is the bare "See the commit history.". So I am raising the inconsistency rather than asserting a defect: the fix establishes that "nothing user-facing changed" deserves a plain sentence instead of a summarisation, and then applies it only to one of the two ways that can be true.

If you want the consistent version, it is the filtered set that decides:

NOTES="$(git log --format='- %s' ${RANGE} | grep -vE "^- (${EXCLUDE_RE}): " || true)"
# The same filter decides whether there is anything to summarise: a range whose
# every commit is an excluded type has nothing user-facing in it, which is the
# same statement as an empty range and deserves the same plain answer.
SUBJECTS="$(git log --format='%s' ${RANGE} | grep -vE "^(${EXCLUDE_RE}): " || true)"
if [ -z "${SUBJECTS}" ]; then
  NOTES="No changes to the application.${PREVIOUS:+ This release republishes ${PREVIOUS}.}"
fi

That also removes the second git log call over the same range, and it makes the existing "an empty range never asks a model to summarise nothing" case cover both ways of being empty — worth a second case naming the excluded-only range, since after the change one deletion would no longer kill both.

If you would rather keep it as is — chores described in plain language beat "See the commit history." — then say so in the comment above SUBJECTS, because the next reader will meet a filtered NOTES and an unfiltered SUBJECTS two lines apart and will assume one of them is a bug.

`SUBJECTS` is the **unfiltered** log while `NOTES` one line above is the **filtered** one, and that asymmetry leaves one case in exactly the state `#79` was about. A force-bump over a range whose commits are all excluded types has non-empty `SUBJECTS`, so it takes the summariser path — and the model is handed the very subjects the project has decided are not user-facing, to rewrite as user-facing notes. I ran it rather than reasoned it, with a `git` stub returning `chore: Bump deps` and `docs: Fix a typo`: PROBE: a force-bump over an all-excluded range still calls the model pass -> notes_fallback=false, the model's output published It is reachable by the same path as `#79` and no other: `BUMP=none` for a chore-only range, so nothing releases **unless** `force-bump: patch` — the rebake dispatch. So the two cases are siblings. `#79` was force-bump over an empty range; this is force-bump over a range with nothing releasable in it, and the project's own `EXCLUDE_TYPES` rule is the thing saying so. What happens then is not obviously wrong — a model will turn "chore: Bump deps" into "### Improvements — Updated some dependencies", which passes the shape check and is arguably better than the fallback, which here is the bare `"See the commit history."`. So I am raising the inconsistency rather than asserting a defect: **the fix establishes that "nothing user-facing changed" deserves a plain sentence instead of a summarisation, and then applies it only to one of the two ways that can be true.** If you want the consistent version, it is the filtered set that decides: ```bash NOTES="$(git log --format='- %s' ${RANGE} | grep -vE "^- (${EXCLUDE_RE}): " || true)" # The same filter decides whether there is anything to summarise: a range whose # every commit is an excluded type has nothing user-facing in it, which is the # same statement as an empty range and deserves the same plain answer. SUBJECTS="$(git log --format='%s' ${RANGE} | grep -vE "^(${EXCLUDE_RE}): " || true)" if [ -z "${SUBJECTS}" ]; then NOTES="No changes to the application.${PREVIOUS:+ This release republishes ${PREVIOUS}.}" fi ``` That also removes the second `git log` call over the same range, and it makes the existing "an empty range never asks a model to summarise nothing" case cover both ways of being empty — worth a second case naming the excluded-only range, since after the change one deletion would no longer kill both. If you would rather keep it as is — chores described in plain language beat `"See the commit history."` — then say so in the comment above `SUBJECTS`, because the next reader will meet a filtered `NOTES` and an unfiltered `SUBJECTS` two lines apart and will assume one of them is a bug.

ahmad force-pushed feature/79-validate-release-notes from 4dad26f395

PR / validate (pull_request) Successful in 20s

Details

to 8cc1c15bea

PR / validate (pull_request) Successful in 20s

Details

2026-09-09 20:18:08 +00:00

Compare

ahmad added 1 commit

2026-09-09 20:22:36 +00:00

fix: Summarise only the commits a reader would care about

PR / validate (pull_request) Successful in 19s

Details

9077f50d7e

A force-bump over a range whose commits are all excluded types reached
the summariser with the very subjects the project decided are not
user-facing, and asked for them back as release notes — the same shape
as the empty range this fixes.

Filter the subjects the model sees the way the fallback list is already
filtered, and say which of the two silences happened.

ahmad_bot approved these changes

2026-09-09 21:47:37 +00:00

ahmad_bot left a comment

Approved. The finding is closed, and the fix answers the part I left open rather than the part I asked about.

I raised the asymmetry — SUBJECTS unfiltered, NOTES filtered — and offered the one-line version. What landed is better: it keeps the two "nothing to say" cases distinguishable instead of collapsing them, so a rebake over an empty range still says "No changes to the application. This release republishes vX." while a rebake over a chore-only range says "No user-facing changes in this release." Those are different facts, and the version I proposed would have merged them.

Verified by running and mutating, at 9077f50d:

baseline                                      all cases pass  (check-actions ok)
revert SUBJECTS to the raw log             -> FAIL "a range of only excluded types never reaches the model either"
collapse the two messages into one         -> FAIL, same case
give the excluded-only branch the          -> FAIL, same case
  republishes wording
restored                                      all cases pass

The summariser is now fed the filtered list: printf '%s' "${SUBJECTS}" | node -e at line 208, where SUBJECTS is RAW_SUBJECTS minus ^(${EXCLUDE_RE}): . The two greps use the right anchors for their two formats — ^- (…) for the - %s notes list, ^(…) for the bare %s subjects — and the chore/docs stub proves it rather than my reading it: the filtered result comes out empty, so both lines matched.
|| true on the new grep is necessary and present: under set -euo pipefail a grep -v that filters everything exits 1, which is exactly the excluded-only case.
The RAW_SUBJECTS distinction reaches the log line too, so an operator sees which of the two happened without reading the notes.

This was my finding, and I want to be accurate about its weight: nothing was broken. A model handed chore: Bump deps would have produced plausible notes about dependency bumps, which is not wrong so much as beside the point the project's own EXCLUDE_TYPES makes. What the fix buys is that the rule is now stated once and applied in both places.

Approved. The finding is closed, and the fix answers the part I left open rather than the part I asked about. I raised the asymmetry — `SUBJECTS` unfiltered, `NOTES` filtered — and offered the one-line version. What landed is better: it keeps the two "nothing to say" cases **distinguishable** instead of collapsing them, so a rebake over an empty range still says "No changes to the application. This release republishes vX." while a rebake over a chore-only range says "No user-facing changes in this release." Those are different facts, and the version I proposed would have merged them. **Verified by running and mutating, at `9077f50d`:** baseline all cases pass (check-actions ok) revert SUBJECTS to the raw log -> FAIL "a range of only excluded types never reaches the model either" collapse the two messages into one -> FAIL, same case give the excluded-only branch the -> FAIL, same case republishes wording restored all cases pass - **The summariser is now fed the filtered list**: `printf '%s' "${SUBJECTS}" | node -e` at line 208, where `SUBJECTS` is `RAW_SUBJECTS` minus `^(${EXCLUDE_RE}): `. The two greps use the right anchors for their two formats — `^- (…)` for the `- %s` notes list, `^(…)` for the bare `%s` subjects — and the chore/docs stub proves it rather than my reading it: the filtered result comes out empty, so both lines matched. - `|| true` on the new grep is necessary and present: under `set -euo pipefail` a `grep -v` that filters everything exits 1, which is exactly the excluded-only case. - The `RAW_SUBJECTS` distinction reaches the log line too, so an operator sees which of the two happened without reading the notes. This was my finding, and I want to be accurate about its weight: nothing was broken. A model handed `chore: Bump deps` would have produced plausible notes about dependency bumps, which is not wrong so much as beside the point the project's own `EXCLUDE_TYPES` makes. What the fix buys is that the rule is now stated once and applied in both places.

ahmad merged commit e02a1a682c into main

2026-09-09 21:49:31 +00:00

ahmad deleted branch feature/79-validate-release-notes

2026-09-09 21:49:31 +00:00

ahmad referenced this pull request from a commit

2026-09-09 21:49:32 +00:00

fix: Validate the release-notes rewrite before publishing (#80)

Sign in to join this conversation.

No reviewers