build: give CHANGELOG.md a union merge, with the guard that makes it safe (#996) - #1108
Conversation
jdatcmd
left a comment
jdatcmd
left a comment
There was a problem hiding this comment.
Reviewed at d741381, in a worktree on your head with the release tags present. The driver argument is right and the guard is the correct thing to ship beside it. Four findings, two of them blocking, and one of them is your check catching a real defect on main.
1. Blocking: two pytest jobs fail, one cause
test/pytest/test_compare_to_bash.py:1052 pins the suite-local check_* helpers to an exact set of four. Your check_skip is a fifth.
FAILED test_compare_to_bash.py::test_the_suite_local_helpers_are_known_and_excluded
got '... check_reconstruct (parallel_copy.sh), check_skip (docs_style.sh), check_split_happened ...'
want '... check_reconstruct (parallel_copy.sh), check_split_happened ...'
test_harness_deps.py::test_the_guard_half_of_the_corpus_runs_without_a_database_driver reports the same assertion because it re-runs that file. One fix clears both: add check_skip (docs_style.sh) to the pin with the reason you already wrote in the suite.
That pin is well built, by the way. It pins the SET, so it noticed a new helper instead of a changed count.
2. Blocking: docs_style.sh is red on your own head
$ bash test/docs_style.sh
FAIL the 1.0-alpha3 section carries the entries v1.0-alpha3 shipped, and no more: got [97] want [96]
checks run: 35
docs_style.sh: FAILED rc=1
CI cannot see this. At depth 1 with no tags, _cl_why is set and all four arms skip, so the job is green. The guard is inert exactly where CI runs it and red where it runs for real. That is worth saying out loud in the PR, because it changes what a green check on this PR means.
3. The alpha3 skip narrative does not reproduce
The PR body, the .gitattributes comment and the in-suite comment all say v1.0-alpha3 shipped with its section still named ## [Unreleased], and report a printed SKIP. Against the tags on the server, that is not so:
v1.0-alpha3 first heading: ## [1.0-alpha3] - 2026-09-02
v1.0-alpha2 first heading: ## [Unreleased] <- this is the one you are describing
and alpha2 also carries a dated ## [1.0-alpha2] - 2026-08-18 below it, so it does not skip either. In a tagged tree no section skips at all: grep -c '^SKIP' on the run above is 0. So the measured "false-positive budget: 1 of 4 released sections" is not measured, and in a tagged tree check_skip is dead code.
I checked my own refs before saying this, because the obvious way to get your result is a stale local tag and git fetch never moves one:
v1.0-alpha server=2c3f195c260e local=2c3f195c260e MATCH
v1.0-alpha2 server=7c317d3b7970 local=7c317d3b7970 MATCH
v1.0-alpha3 server=cec9e9b59d55 local=cec9e9b59d55 MATCH
v1.0-alpha4 server=228267c9ff29 local=228267c9ff29 MATCH
Worth running git ls-remote --tags in your tree. If your v1.0-alpha3 disagrees, that explains the skip and the budget in one go.
4. The red is your check working, and it found a live one
- A pytest harness beside the bash suites, with a layer that refuses tests which
is in the ## [1.0-alpha3] section of main and is not in what v1.0-alpha3 shipped. It came from d978e7f, docs: record the pytest harness in the changelog (#432), dated 2026-09-09, into a section tagged 2026-09-02. Exactly the class you built the guard for, already on main, seven days after the tag. That is the strongest argument for this PR and it should be in the body.
The question that has no answer yet
Fixing that entry is where the design needs a decision, and I do not think it has one.
The guard pins each released section to its own tag, so a released section can never be corrected. The stray entry belongs to alpha4's cycle, but the alpha4 arm passes at 112 == 112, so moving it there turns alpha4 red. Deleting it loses a true record. There is no exception mechanism, so the tree's only green state is the one that keeps every past mistake exactly as tagged.
That may be the right call — a changelog is a record of what shipped, and what shipped had the entry in the wrong place. If so, say it in the suite, because the next person to hit a red arm will try to fix the changelog and cannot. If it is not the right call, the guard needs a recorded-exception file with a reason per entry, and this entry is its first row.
Either way it has to be settled before merge, because this PR cannot go green without settling it.
Everything else checks out
CHANGELOG.md merge=unionmatches the grep's anchor, and removing the line reddens its own arm.- The
while ... done <<CLEOFheredoc keeps the loop in the current shell, sochecks,failand_cl_seenall survive. A pipe there would have lost every increment silently. - The
_cl_seen >= 3premise is the right shape: an empty sweep would otherwise report clean having compared nothing. docs_styleholds 0 ledger rows, so your "no rows move" claim is right.- The
check_skip-was-command not founddefect is a good catch and the honest way to report it.
Not approving yet. Fix 1, then settle the question, and I will re-run this in a tagged worktree.
d741381 to
d608f4a
Compare
|
Pushed What CI saidBoth reproduced locally before changing anything. Selftest 400 is right and I was wrong. What replaced itThe Mutation proof that it is not just a passing sentence — pretend that tag did ship one: So if a future release is ever dated before its tag, this arm reddens and the real comparison takes over. That is strictly better than the skip: a skip says nothing and this says something falsifiable. The no-tags case (CI, depth 1) is a
Verification after the changeAll four mutations still redden: entry added to closed alpha4 ( I should have run the pytest guards and the selftest before opening this, not after CI told me. The suite I was editing has a guard about exactly the vocabulary I reached for, and it is in the same tree. |
d608f4a to
dedb146
Compare
|
Correction: My mutation script restored a mutated
Mutations, each restored byte-exact: The commit is gated on the selftest now — the amend script refuses to run if |
dedb146 to
f5f5e44
Compare
|
Everything downstream of that ref was wrong: "alpha3 shipped with its section still named Your finding 4, confirmed independentlyFinding 1 is moot, for a different reason than you expectedThere is no The question, settled — and it is your option twoThe guard now pins each section to the entries its tag shipped, not a count, and I went with the recorded exception rather than "a released section can never change" because the latter is not quite the rule anyone wants — it would mean a mistake made before a tag can never be described afterwards. Entry-keyed, not counted, precisely because Two arms guard the file itself: every row carries a reason, and no row is stale. Seven mutations, each restored byte-exact: A third guard caught me while fixing the secondSelftest 070: introducing Your finding 2 is now in the body and the commitAt depth 1 with no tags every section arm is skipped, so CI cannot run any of this and a green check on this PR says nothing about it. It runs locally and in the five-major release gate — which is where a release is cut, and therefore where an entry can be filed into a closed section. The suite now tells a reader to check a tag against |
…safe (commandprompt#996) Entries insert as the first child of one heading, so two PRs sharing no file but CHANGELOG.md still conflict. Nine of 27 merges in one day touched it; three merge commits that day exist only to resolve it. Measured on the real pair, commandprompt#1098 and commandprompt#1106, both opening a new [Unreleased]: default 3-way rc=1, 2 conflict markers merge=union rc=0, 0 markers, ONE [Unreleased], both entries intact Union emits identical lines once, which is why the headings are not doubled: 7640 + 57 + 52 = 7749 against an actual 7744. THE DRIVER DOES NOT SHIP ALONE. Union keeps both sides of a divergent hunk silently, and a release cut EDITS the line pending PRs append beneath. Reproduced: a PR merged into a release cut files its entry inside the section that just shipped, rc=0, no marker. Today that conflicts and a human sees it. So docs_style.sh gains the check: each dated section holds the entries its own tag shipped, and nothing else. THE KEY IS THE ENTRY, not a count -- a count would let one post-tag entry be swapped for another with the arm still green. IT FOUND ONE ALREADY ON MAIN. ## [1.0-alpha3] carries an entry v1.0-alpha3 never shipped, added by d978e7f (commandprompt#432) on 2026-09-09, seven days after the 2026-09-02 tag. Found by jdatcmd in review. It cannot be corrected without making a second section wrong -- the work shipped in the alpha4 cycle and the v1.0-alpha4 tag does not carry it either -- so a released section MAY diverge, but only by being recorded in test/changelog_post_tag.txt with a reason visible in the diff. Two arms guard that file: every row needs a reason, and no row may be stale. Seven mutations, each restored byte-exact: an entry added to closed alpha4, one removed from alpha2, the allowance row deleted, the allowance naming a different entry, the reason blanked, a stale allowance row, and the driver line removed. Each reddens its own arm. Clean tree: 42 checks, rc=0. THE FIRST VERSION OF THIS CHECK WAS MEASURED AGAINST A STALE TAG. This tree's local v1.0-alpha3 was d9df031 against the server's cec9e9b, and git fetch never moves a tag that already exists. That produced a false narrative (alpha3 "shipped with its section still named [Unreleased]"), a skip for it, and a "false-positive budget of 1 of 4" -- all three wrong, with the arm green where the tree was in violation. jdatcmd caught it by checking their own refs against git ls-remote first. The suite now tells a reader to verify a tag before believing a red arm. Reaching for a skip was also wrong, and two guards said so. This suite keeps its own tally and emits none of the machine RESULT vocabulary, so lib.sh's check_skip is `command not found` inside it. A local one then tripped selftest 400, which refuses `echo "SKIP` in any file that calls check. A property this suite cannot compare is now a note(): printed, counted as nothing, claiming no outcome. Selftest 070 caught a third: introducing `comm` makes every `| sort` in the file collation-sensitive, and comm compares byte-wise without checking its inputs agree. All nine sorts are pinned LC_ALL=C, including the five that predate this block, and so is comm. KNOWN LIMIT: at depth 1 with no tags every section arm is skipped, so CI cannot run any of this. It runs locally and in the five-major release gate. Verified: docs_style 42 checks rc=0; harness_selftest 976 checks 0 failed, including selftest 070 and 400's own arms; pytest guards 346 passed, 897 checks, 0 failed. docs_style.sh holds no ledger rows, so no rows move. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012RSw4qMHS7ByE7PY8Ns4cs
f5f5e44 to
3049fb3
Compare
jdatcmd
left a comment
jdatcmd
left a comment
There was a problem hiding this comment.
Approving 3049fb3. Both blockers are gone and the design question is answered better than I framed it. Re-run in a tagged worktree, and I mutated the guard rather than reading it.
Blocker 1 is gone, and the fix is better than the one I asked for
I asked you to add check_skip to the suite-local helper pin. You removed the helper instead:
test/docs_style.sh:84 note() { # note TEXT
A note is not a check, so it does not belong in a check_* pin and the pin is untouched — test_compare_to_bash.py is not in your diff at all. That is the right shape: the earlier version was counting a skip as a check, which is what made it need the allow-list entry in the first place.
Blocker 2 is gone
$ bash test/docs_style.sh # tagged worktree, all four v1.0-alpha* tags present
checks run: 42
docs_style.sh: PASSED rc=0 stderr empty
No skips, no red arms, and the alpha3 narrative is corrected — your local d9df031d against the server's cec9e9b5 explains the original reading completely.
The design question, answered
I said the guard pinned released sections to their tags with no way to correct a misfiling, so the tree's only green state was one that preserved every past mistake. test/changelog_post_tag.txt is the answer, and the entry reasons it out rather than just permitting it:
It STAYS where it is. The work shipped in the 1.0-alpha4 cycle, but the v1.0-alpha4 tag does not carry the entry either, so moving it would make a second section disagree with its own tag and deleting it would lose a true record. A changelog records what happened, and what happened is this.
That is the right call and I had not seen the second half of it — moving the entry to alpha4 would have reddened alpha4, because that tag does not carry it either. There was no correct destination, only a correct record.
Removal proof, both directions, run here
An entry added to the closed alpha4 section:
FAIL the 1.0-alpha4 section holds what v1.0-alpha4 shipped, plus only what is recorded:
got [- a planted entry that v1.0-alpha4 never shipped (#0).] want []
An allowance recorded for an entry that is not there:
FAIL the 1.0-alpha2 section holds what v1.0-alpha2 shipped, plus only what is recorded:
got [] want [- an allowance for an entry that is not there]
So the escape hatch cannot rot open and cannot be widened without a diff a reviewer sees, which is the property that makes an allowance file acceptable rather than a permission slip. Both mutations restored byte-exact.
It names the ENTRY rather than a count, which is a real improvement over the version I reviewed — got [113] want [112] told a reader something was wrong; this tells them which line.
Still true, and still fine
The whole family is inert in CI: at depth 1 with no tags, _cl_why is set and every arm prints a note. It runs locally and in the five-major gate, which is where a release is cut and therefore where an entry can be filed into one wrongly. Worth a line in the body so a green check on this PR is not read as the guard having run.
Good change. The union driver was the easy half and you shipped the hard half with it.
) #996 gave CHANGELOG.md a union merge driver and it does what it was argued to do: four branches have rebased onto it with zero conflicts, one [Unreleased], every entry present, docs_style green. GITHUB DOES NOT HONOUR IT. Its mergeability calculation and its merge button do not read .gitattributes, so a PR shows the red "This branch has conflicts" badge the moment another changelog entry lands -- while the same merge is clean locally. #1108's body asked this explicitly and said it was untested; jdatcmd tested it, and I saw the same thing independently: four of my branches rebased clean while GitHub showed CONFLICTING throughout. Documented rather than worked around, which is what the measurement supports. The badge is wrong rather than harmful, and the cost #996 measured -- nine local merges and rebases a day -- is gone. CONTEXT.md now says what the badge means and gives the three commands; .gitattributes records it beside the driver. The section also carries the one case union resolves silently and WRONGLY: a release cut edits `## [Unreleased]` into a dated heading, so a pending entry lands inside the section that just shipped. docs_style.sh already compares each dated section against its own tag, so that is caught rather than trusted -- but a person cutting a release should know it before the arm tells them. Four arms hold the driver and the instruction TOGETHER, because the driver without the instruction is worse than either alone: it makes the badge lie and gives nobody the reason. Removing the section reddens three of them. docs_style 46 checks rc=0; it holds no ledger rows, so no rows move. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012RSw4qMHS7ByE7PY8Ns4cs
CONTEXT.md told a contributor to rebase rather than click "Update branch" and gave the commands, but not the reason -- and the reason is what makes the advice transferable: git reads .gitattributes from the tree it is merging INTO, so a branch opened before the driver landed cannot use it. Measured on #1107, whose head predates #1108: merge main INTO the branch conflicts: CHANGELOG.md <derived files> rebase the branch ONTO main conflicts: <derived files> grep -c 'CHANGELOG.md.*merge=union' gives 0 on that head and 1 on main. Merging brings main's commits into a tree whose attributes have no driver; rebasing replays the branch onto main, where the driver is already in force. For any branch older than the driver, "Update branch" cannot work even in principle. THE ARM WAS RED ON THE UNMUTATED TREE FIRST. Its grep spanned the prose's line break -- "the tree it is / merging **into**" -- so a line-based pattern found 0 on a correct document, and the mutation reddened it too. That looks like a working removal proof and is two failures agreeing. Re-anchored on a phrase that fits one line: control green, mutation red. docs_style 47 checks rc=0. It holds no ledger rows, so no rows move. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012RSw4qMHS7ByE7PY8Ns4cs
2 participants
Closes #996.
The union driver the issue asked for, plus the guard that makes it safe — because union's hazard is not hypothetical for this file, it arrives at every release.
The driver works, measured on the real pair
#1098 and #1106 both open a new
## [Unreleased]section, which is the case I expected union to mangle:It does not double the headings, because union emits identical lines once. The line count says so exactly:
The 5 are the shared
## [Unreleased]/ blank /### Fixed/ blank prefix, matched by the diff and emitted once.Union's hazard IS live for this file, at every release
The issue notes union silently keeps both sides where two branches edit the same existing line, and argues it cannot arise because entries are pure appends. Entries are — but a release cut is not. Cutting a release edits the exact line pending PRs append beneath:
Reproduced. A PR appending an entry, merged into a release cut, with the driver in place:
rc=0, no marker, nobody told. Today that same case conflicts and a human sees it — which is precisely the defect #1099 was about, and what @linuxhikerpm caught on my #1098. Shipping the driver alone would trade a loud daily cost for a silent one at every release.
So the guard ships with it
test/docs_style.shnow checks each dated section against what its own tag shipped. Both directions redden:Clean tree:
checks run: 35,docs_style.sh: PASSED, stderr empty. Every mutation restored byte-exact.v1.0-alpha3is skipped, and the reason is printedThat tag really did ship with
## [Unreleased]at the top — alpha3 was dated after it was tagged — so there is no baseline to compare against. False-positive budget measured before shipping: 1 of 4 released sections, and that one is this, not a flaw in the check. It closes for every release dated before its tag.Writing the skip found a second defect
docs_style.shkeeps its own tally rather thanlib.sh's, and has nocheck_skip. My first version calledlib.sh's:The check printed nothing, counted nothing and failed nothing, while the suite reported
PASSED. I only caught it because the alpha3 arm was missing from output I expected it in. A localcheck_skipnow exists and counts, with the reason written down so the next person does not reach for the wrong one.Portability and scope
awkavoids the gawk-only three-argumentmatch(); identical output undermawk,gawkand this box's defaultawk, checked rather than assumed.bash -nandshellcheck -S errorclean.docs_style.shholds no ledger rows, so no rows move andsuites_not_coveredis unchanged.Note: GitHub's server-side merge is not guaranteed to honour a
.gitattributesmerge driver, so this removes the conflict for local merges and rebases — which is where the nine-a-day cost is paid — and may or may not change what the merge button does. I have not tested the button, and say so rather than claim it.🤖 Generated with Claude Code
https://claude.ai/code/session_012RSw4qMHS7ByE7PY8Ns4cs