fix(recover-pending-cut): refuse a fold-back whose merge would lose text (#1491) #1493

Merged
claude merged 3 commits from claude/1491-foldback-refuse-drop into main 2026-10-02 12:20:51 +02:00
Member

Refs #1491

Base main. Head 2060ed13ffb3834928b2aad3a32916eab485acb3, three commits on 55a12b9:

  • ff3345f: the change. It was rebased from 8d7c1cd once #1486 landed; git range-diff shows only surrounding docs/integration.md text changed.
  • 0935704: answers review 7158 (findings 1-6).
  • 2060ed1: the non-blocking nit from re-review 7160. The refusal now opens with what was not written, so the remedy's "them" directly follows the headings it refers to.

Each is a new commit, with no amend or force-push.

Problem (measured)

As filed in #1491: FoldBackIntoUnreleased(cl, "0.2.0") returns err=nil when [Unreleased] holds prose plus a ### Notes block and the pending section holds ### Added / - Shipped.. [Unreleased] comes back as ### Added\n\n- Shipped. only. The new tests reproduce this: on the pre-fix tree, every refusing case failed with err = <nil>.

The pending section can lose text the same way, for example through a hand-edited ### Notes in the stranded section.

What changes

  • changelog.FoldBackIntoUnreleased: the merge arm runs only when both bodies are non-empty. It now grades both bodies before merging and returns *changelog.FoldBackDropError, which names each line by its CHANGELOG line and returns no changelog.
    • Both bodies are carried as records with their file lines. The merge input and the grading are built from the same records.
    • The error records [Unreleased]'s last ### line (Inherited). That is what decides whether a pending line above its first heading is moved or dropped.
  • rt recover-pending-cut: routes the error to rc=1 (refused), with --dry-run too, before anything is written. Verbatim from the verb at 2060ed1 (--dry-run, exit 1):
recover-pending-cut: nothing was written.
folding ## [v0.57.0] - 2026-08-29 back under [Unreleased] would drop 3 line(s): the merge carries text only under an accepted heading.
  in [Unreleased]:
    CHANGELOG.md:5: New prose after the prepare.    <- before the first ### heading
    CHANGELOG.md:7: ### Notes    <- not an accepted heading
    CHANGELOG.md:9: - A note.    <- under ### Notes, not an accepted heading
Accepted headings (level 3, exact case): ### Added, ### Changed, ### Fixed, ### Removed, ### Deprecated, ### Upgrade, ### Security, ### Internal.
Move each line under one of them, or delete it, then re-run rt recover-pending-cut.

A pending line above its first heading reads, depending on [Unreleased]'s last heading:

    CHANGELOG.md:11: Release intro.    <- above the first ### heading; the merge would move it under [Unreleased]'s ### Fixed
    CHANGELOG.md:15: Release intro.    <- above the first ### heading; the merge would drop it under [Unreleased]'s ### Notes, not an accepted heading
    CHANGELOG.md:9: Release intro.    <- above the first ### heading, and [Unreleased] has no ### heading to carry it; the merge would drop it
  • changelog.AcceptedHeadingsNote / changelog.UncarriedRemedy: one source for the closing lines of both refusals. rt prep now uses them too, and its text is byte-identical (pinned in internal/prep/unreleased_drop_test.go).
  • docs/integration.md § recovery (B): adds the new refusal to the exit-code contract, plus a paragraph on which text the fold refuses.
  • changelog.d/1491-recover-pending-cut-drop.fixed.md.

Review 7158

# Finding Change in 0935704 Test
1 A pending intro line reads "moved" when the merge drops it Inherited + movedLine. The reason says move only when that heading is accepted, otherwise drop; with no heading it says so. TestFoldBackDescribeSaysWhatTheMergeDoes. The oracle now measures move vs drop in MergeSections' output for every reported line and compares it with the error's own classification.
2 A raw \r on a CRLF changelog Stripped when rendering. Text and PendingHeading stay verbatim in the struct. TestFoldBackDescribeOnACRLFChangelog (same file lines as LF, no \r in the text)
3 The header says "lose N" but counts moved lines would drop N line(s), would move N line(s) under a heading they were not written under, or both, counted separately TestFoldBackDescribeSaysWhatTheMergeDoes (moved, dropped, both)
4 uncarriedAt assumes a contiguous slice fileRecord, bodyRecords, joinRecords, uncarriedIn: the merge input and the grading come from one record sequence, and the mapping holds for any subsequence The existing exact-line and oracle file-line checks, plus mutants that keep leading or trailing blank records
5 Duplicated note and remedy AcceptedHeadingsNote and UncarriedRemedy in merge.go, used by both refusals prep: the tail is byte-identical to the old literal and equals the helpers; verb and library end with the helpers. No mutant: identical duplicated text cannot be told apart by behaviour.
6 The library claims "Nothing was written." Removed from FoldBackDropError; the verb prints it (since 2060ed1, as its first line) The library's Describe must not say it; the verb's output must open with it

Also measured, kept out of scope: rt prep's UnreleasedDropError prints l.Text verbatim too, so on a CRLF CHANGELOG its refusal line carries the same raw \r (Go probe at 0935704). It is #1486's surface, not #1491's, so it is left for a separate issue and PR (not filed yet).

Decision tree

The pending body: check it, and how?

The pending body is MergeSections' second argument. The merge's current kind carries into it from [Unreleased]'s last heading, and UncarriedLines is documented as exact only for the first argument.

  • Don't check it. Right only if the pending body cannot hold anything the merge drops. It can: a hand-edited ### Notes is dropped (test a hand-edited heading is dropped).
  • Check it carry-aware, simulating the kind inherited from [Unreleased]. Right if a line moved under a heading it was not written under still counts as preserved. With this, a pending intro line would land under [Unreleased]'s last heading, straight after its last bullet, where markdown reads it as that bullet's continuation.
  • Check it on its own with UncarriedLines (taken). Right when a line survives only under the heading it was written under:
    • From the pending body's first ### heading on, the result equals the carry-aware one, because a heading resets the kind.
    • Above that heading, it reports every line. The merge drops such a line if the inherited heading is not accepted or there is none, and otherwise moves it. Since 0935704 the refusal says which.
    • Taken because a silent move changes what the release note says.

The oracle test defines "lost" independently, as "not emitted under its source heading". It grades against MergeSections' own output, not against UncarriedLines.

Which paths refuse?

  • Merge arm only (taken). Two other paths carry text verbatim: the splice (empty [Unreleased]) and the empty-pending path. Nothing is lost there, and the next rt prep refuses the same text under #1486. Both are pinned byte for byte.
  • Every path. Right if recovery should enforce prep's grammar early. Not taken: it duplicates #1486's gate, and it changes the splice, which #1491 requires to stay byte-identical.

Exit code

  • rc=1, refused (taken). The state is graded and known, and the fix is an edit plus a re-run. This is the same class as the existing "no [Unreleased] section" refusal.
  • rc=2, could not grade. Right when the state cannot be established, which is not the case here.

Interaction with #1490

#1490 (for #1487) leaves the merge arm alone. It builds the pending body from subtractCovered(recs[nextAt+1:endAt], covered), though. Since 0935704, the bodies are []fileRecord, and uncarriedIn maps line numbers correctly for any subsequence of the file's records. Whichever PR lands second only needs subtractCovered to filter the pending records (or to return records). The grading then follows automatically, because it reads the same records the merge is fed.

#1491 acceptance criteria

AC Where
Non-empty [Unreleased]: refuse and write nothing on any UncarriedLines hit, naming lines + MergeKinds TestFoldBackRefusesUnreleasedTextTheMergeWouldDrop, TestRecoverPendingCutRefusesTextTheFoldWouldDrop (both modes)
Pending body checked, or why not recorded Checked; see the decision tree. TestFoldBackRefusesPendingTextTheMergeWouldLose, TestFoldBackDescribeSaysWhatTheMergeDoes
Splice stays byte-identical TestFoldBackVerbatimArmsDoNotRefuse (splice + empty-pending)
Red on main, mutation-verified Red on the pre-fix tree with a behaviour-free stub of the new type: the tests reference FoldBackDropError, which this PR adds. Mutation tables below

Refs, not a closing keyword, as in #1490. Ticking the ACs is the reviewer's call.

Verification

Mutation runs (AGENTS.md section 8, Axis B). The clone was mounted read-only, and each mutant ran on a copy.

19/19 killed at 0935704: 12 for the review findings, plus the 7 behavioural guards from the first round re-run on the refactored code. At 2060ed1 the verb-side mutants were re-run (5/5 killed), including the old ordering.

Mutant Killed by
every pending intro line reads as moved (the pre-review behaviour) Describe cases (dropped, no heading), CRLF test, oracle move/drop check
inherited heading taken from the first ### , not the last Describe case (dropped), oracle move/drop check
no inherited heading recorded Describe cases, oracle, measured-defect test
drop/move reasons swapped Describe cases, CRLF test, pending moved-line test
raw \r in line text / in the pending heading CRLF test (each)
header counts moved lines as dropped Describe cases (moved, both)
file line off by one in the record mapping exact-line tests, oracle file-line check, verb test
body keeps trailing blank records splice and empty-pending byte tests, TestFoldBackPreservesTheBodyByteForByte
body keeps leading blank records the same byte tests, plus TestFoldBackEdgeShapes
library claims nothing was written measured-defect test, verb test
verb omits "nothing was written." verb test
verb prints it between the headings and the remedy (the pre-2060ed1 order) verb test (added with 2060ed1)
no [Unreleased] check / no pending check / carry-aware pending check measured-defect + Describe + oracle / pending tests + oracle / moved-line tests + oracle
check also on the splice / empty-pending path TestFoldBackVerbatimArmsDoNotRefuse (each)
verb: no drop arm / refusal without the file label verb test (each)

The first round's other three mutants (the two old line-offset bugs and Describe dropping the "moved" reason) target code 0935704 replaced. The record-mapping, leading/trailing-blank and swapped-reason mutants cover them now.

  • Local: at 2060ed1, go test -count=1 ./... (29 packages), gofmt, go vet and the rt --help snapshot, in golang:1.25 on an LF clone. rt fragment-check (density + issue reference), rt register-check and rt changelog-body-check pass.
  • CI: at ff3345f, the commit statuses report success on all 30 contexts, go-ci / lint + build + test included. At 0935704 and at 2060ed1, the same: success on all 30 contexts, go-ci / lint + build + test included. These are reported statuses (AGENTS.md section 8, Axis C); I did not read the job logs.

AGENTS.md notes

  • Section 2: recover-pending-cut is not one of the cut verbs, so no re-pin is needed.
  • Section 6: please verify at source, at 2060ed1, both halves of the refusal: the lines it names, and what it says the merge does with each one.
  • Section 11: no scripts, bats or workflow YAML touched.

🤖 Generated with Claude Code

Refs #1491 Base `main`. Head `2060ed13ffb3834928b2aad3a32916eab485acb3`, three commits on `55a12b9`: - `ff3345f`: the change. It was rebased from `8d7c1cd` once #1486 landed; `git range-diff` shows only surrounding `docs/integration.md` text changed. - `0935704`: answers review 7158 (findings 1-6). - `2060ed1`: the non-blocking nit from re-review 7160. The refusal now opens with what was not written, so the remedy's "them" directly follows the headings it refers to. Each is a new commit, with no amend or force-push. ## Problem (measured) As filed in #1491: `FoldBackIntoUnreleased(cl, "0.2.0")` returns `err=nil` when `[Unreleased]` holds prose plus a `### Notes` block and the pending section holds `### Added` / `- Shipped.`. `[Unreleased]` comes back as `### Added\n\n- Shipped.` only. The new tests reproduce this: on the pre-fix tree, every refusing case failed with `err = <nil>`. The pending section can lose text the same way, for example through a hand-edited `### Notes` in the stranded section. ## What changes - **`changelog.FoldBackIntoUnreleased`**: the merge arm runs only when both bodies are non-empty. It now grades both bodies before merging and returns `*changelog.FoldBackDropError`, which names each line by its CHANGELOG line and returns no changelog. - Both bodies are carried as records with their file lines. The merge input and the grading are built from the same records. - The error records `[Unreleased]`'s last `### ` line (`Inherited`). That is what decides whether a pending line above its first heading is moved or dropped. - **`rt recover-pending-cut`**: routes the error to rc=1 (refused), with `--dry-run` too, before anything is written. Verbatim from the verb at `2060ed1` (`--dry-run`, exit 1): ``` recover-pending-cut: nothing was written. folding ## [v0.57.0] - 2026-08-29 back under [Unreleased] would drop 3 line(s): the merge carries text only under an accepted heading. in [Unreleased]: CHANGELOG.md:5: New prose after the prepare. <- before the first ### heading CHANGELOG.md:7: ### Notes <- not an accepted heading CHANGELOG.md:9: - A note. <- under ### Notes, not an accepted heading Accepted headings (level 3, exact case): ### Added, ### Changed, ### Fixed, ### Removed, ### Deprecated, ### Upgrade, ### Security, ### Internal. Move each line under one of them, or delete it, then re-run rt recover-pending-cut. ``` A pending line above its first heading reads, depending on `[Unreleased]`'s last heading: ``` CHANGELOG.md:11: Release intro. <- above the first ### heading; the merge would move it under [Unreleased]'s ### Fixed CHANGELOG.md:15: Release intro. <- above the first ### heading; the merge would drop it under [Unreleased]'s ### Notes, not an accepted heading CHANGELOG.md:9: Release intro. <- above the first ### heading, and [Unreleased] has no ### heading to carry it; the merge would drop it ``` - **`changelog.AcceptedHeadingsNote` / `changelog.UncarriedRemedy`**: one source for the closing lines of both refusals. `rt prep` now uses them too, and its text is byte-identical (pinned in `internal/prep/unreleased_drop_test.go`). - **`docs/integration.md`** § recovery (B): adds the new refusal to the exit-code contract, plus a paragraph on which text the fold refuses. - **`changelog.d/1491-recover-pending-cut-drop.fixed.md`**. ## Review 7158 | # | Finding | Change in `0935704` | Test | |---|---|---|---| | 1 | A pending intro line reads "moved" when the merge drops it | `Inherited` + `movedLine`. The reason says move only when that heading is accepted, otherwise drop; with no heading it says so. | `TestFoldBackDescribeSaysWhatTheMergeDoes`. The oracle now measures move vs drop in `MergeSections`' output for every reported line and compares it with the error's own classification. | | 2 | A raw `\r` on a CRLF changelog | Stripped when rendering. `Text` and `PendingHeading` stay verbatim in the struct. | `TestFoldBackDescribeOnACRLFChangelog` (same file lines as LF, no `\r` in the text) | | 3 | The header says "lose N" but counts moved lines | `would drop N line(s)`, `would move N line(s) under a heading they were not written under`, or both, counted separately | `TestFoldBackDescribeSaysWhatTheMergeDoes` (moved, dropped, both) | | 4 | `uncarriedAt` assumes a contiguous slice | `fileRecord`, `bodyRecords`, `joinRecords`, `uncarriedIn`: the merge input and the grading come from one record sequence, and the mapping holds for any subsequence | The existing exact-line and oracle file-line checks, plus mutants that keep leading or trailing blank records | | 5 | Duplicated note and remedy | `AcceptedHeadingsNote` and `UncarriedRemedy` in `merge.go`, used by both refusals | prep: the tail is byte-identical to the old literal and equals the helpers; verb and library end with the helpers. No mutant: identical duplicated text cannot be told apart by behaviour. | | 6 | The library claims "Nothing was written." | Removed from `FoldBackDropError`; the verb prints it (since `2060ed1`, as its first line) | The library's `Describe` must not say it; the verb's output must open with it | **Also measured, kept out of scope:** `rt prep`'s `UnreleasedDropError` prints `l.Text` verbatim too, so on a CRLF CHANGELOG its refusal line carries the same raw `\r` (Go probe at `0935704`). It is #1486's surface, not #1491's, so it is left for a separate issue and PR (not filed yet). ## Decision tree ### The pending body: check it, and how? The pending body is `MergeSections`' **second** argument. The merge's current kind carries into it from `[Unreleased]`'s last heading, and `UncarriedLines` is documented as exact only for the first argument. - **Don't check it.** Right only if the pending body cannot hold anything the merge drops. It can: a hand-edited `### Notes` is dropped (test `a hand-edited heading is dropped`). - **Check it carry-aware**, simulating the kind inherited from `[Unreleased]`. Right if a line moved under a heading it was not written under still counts as preserved. With this, a pending intro line would land under `[Unreleased]`'s last heading, straight after its last bullet, where markdown reads it as that bullet's continuation. - **Check it on its own with `UncarriedLines`** (taken). Right when a line survives only under the heading it was written under: - From the pending body's first `### ` heading on, the result equals the carry-aware one, because a heading resets the kind. - Above that heading, it reports every line. The merge drops such a line if the inherited heading is not accepted or there is none, and otherwise moves it. Since `0935704` the refusal says which. - Taken because a silent move changes what the release note says. The oracle test defines "lost" independently, as "not emitted under its source heading". It grades against `MergeSections`' own output, not against `UncarriedLines`. ### Which paths refuse? - **Merge arm only** (taken). Two other paths carry text verbatim: the splice (empty `[Unreleased]`) and the empty-pending path. Nothing is lost there, and the next `rt prep` refuses the same text under #1486. Both are pinned byte for byte. - **Every path.** Right if recovery should enforce prep's grammar early. Not taken: it duplicates #1486's gate, and it changes the splice, which #1491 requires to stay byte-identical. ### Exit code - **rc=1, refused** (taken). The state is graded and known, and the fix is an edit plus a re-run. This is the same class as the existing "no `[Unreleased]` section" refusal. - **rc=2, could not grade.** Right when the state cannot be established, which is not the case here. ## Interaction with #1490 #1490 (for #1487) leaves the merge arm alone. It builds the pending body from `subtractCovered(recs[nextAt+1:endAt], covered)`, though. Since `0935704`, the bodies are `[]fileRecord`, and `uncarriedIn` maps line numbers correctly for any subsequence of the file's records. Whichever PR lands second only needs `subtractCovered` to filter the pending records (or to return records). The grading then follows automatically, because it reads the same records the merge is fed. ## #1491 acceptance criteria | AC | Where | |---|---| | Non-empty `[Unreleased]`: refuse and write nothing on any `UncarriedLines` hit, naming lines + `MergeKinds` | `TestFoldBackRefusesUnreleasedTextTheMergeWouldDrop`, `TestRecoverPendingCutRefusesTextTheFoldWouldDrop` (both modes) | | Pending body checked, or why not recorded | Checked; see the decision tree. `TestFoldBackRefusesPendingTextTheMergeWouldLose`, `TestFoldBackDescribeSaysWhatTheMergeDoes` | | Splice stays byte-identical | `TestFoldBackVerbatimArmsDoNotRefuse` (splice + empty-pending) | | Red on `main`, mutation-verified | Red on the pre-fix tree with a behaviour-free stub of the new type: the tests reference `FoldBackDropError`, which this PR adds. Mutation tables below | `Refs`, not a closing keyword, as in #1490. Ticking the ACs is the reviewer's call. ## Verification Mutation runs (AGENTS.md section 8, Axis B). The clone was mounted read-only, and each mutant ran on a copy. 19/19 killed at `0935704`: 12 for the review findings, plus the 7 behavioural guards from the first round re-run on the refactored code. At `2060ed1` the verb-side mutants were re-run (5/5 killed), including the old ordering. | Mutant | Killed by | |---|---| | every pending intro line reads as moved (the pre-review behaviour) | Describe cases (dropped, no heading), CRLF test, oracle move/drop check | | inherited heading taken from the first `### `, not the last | Describe case (dropped), oracle move/drop check | | no inherited heading recorded | Describe cases, oracle, measured-defect test | | drop/move reasons swapped | Describe cases, CRLF test, pending moved-line test | | raw `\r` in line text / in the pending heading | CRLF test (each) | | header counts moved lines as dropped | Describe cases (moved, both) | | file line off by one in the record mapping | exact-line tests, oracle file-line check, verb test | | body keeps trailing blank records | splice and empty-pending byte tests, `TestFoldBackPreservesTheBodyByteForByte` | | body keeps leading blank records | the same byte tests, plus `TestFoldBackEdgeShapes` | | library claims nothing was written | measured-defect test, verb test | | verb omits "nothing was written." | verb test | | verb prints it between the headings and the remedy (the pre-`2060ed1` order) | verb test (added with `2060ed1`) | | no `[Unreleased]` check / no pending check / carry-aware pending check | measured-defect + Describe + oracle / pending tests + oracle / moved-line tests + oracle | | check also on the splice / empty-pending path | `TestFoldBackVerbatimArmsDoNotRefuse` (each) | | verb: no drop arm / refusal without the file label | verb test (each) | The first round's other three mutants (the two old line-offset bugs and `Describe` dropping the "moved" reason) target code `0935704` replaced. The record-mapping, leading/trailing-blank and swapped-reason mutants cover them now. - **Local:** at `2060ed1`, `go test -count=1 ./...` (29 packages), gofmt, `go vet` and the `rt --help` snapshot, in `golang:1.25` on an LF clone. `rt fragment-check` (density + issue reference), `rt register-check` and `rt changelog-body-check` pass. - **CI:** at `ff3345f`, the commit statuses report success on all 30 contexts, `go-ci / lint + build + test` included. At `0935704` and at `2060ed1`, the same: success on all 30 contexts, `go-ci / lint + build + test` included. These are reported statuses (AGENTS.md section 8, Axis C); I did not read the job logs. ## AGENTS.md notes - Section 2: `recover-pending-cut` is not one of the cut verbs, so no re-pin is needed. - Section 6: please verify at source, at `2060ed1`, both halves of the refusal: the lines it names, and what it says the merge does with each one. - Section 11: no scripts, bats or workflow YAML touched. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
With content under [Unreleased], FoldBackIntoUnreleased merged it with the
pending section through MergeSections and silently dropped every line not
under an accepted ### heading -- prose, ### Notes, ### added -- in either
section, while the verb reported success. It now returns FoldBackDropError
naming each line by its CHANGELOG line, and rt recover-pending-cut refuses
(rc=1), including on --dry-run, before anything is written.

[Unreleased] is the merge's first argument, so UncarriedLines is exact for
it. The pending section is graded on its own: from its first heading on that
is exact, and above it the merge would file text under [Unreleased]'s last
heading, which is refused too. The splice (empty [Unreleased]) and the
empty-pending path carry text verbatim and are unchanged.

Refs #1491. Builds on #1486 (UncarriedLines, MergeKinds).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
alex changed target branch from fix/prep-unreleased-drop-cut-gate-mirror to main 2026-10-02 10:21:38 +02:00
alex force-pushed claude/1491-foldback-refuse-drop from 8d7c1cd781 to ff3345f3e4
All checks were successful
fork-pr-approval-notice / explain fork workflow approval (pull_request_target) Successful in 5s
ac-closure-check / ac-closure check (pull_request) Successful in 7s
ac-closure-check / check (pull_request) Successful in 0s
base-divergence-check / check (pull_request) Successful in 6s
changelog-body-check / changelog body Cold-Read linter (pull_request) Successful in 7s
changelog-body-check / check (pull_request) Successful in 0s
check-self-bootstrap / check (pull_request) Successful in 5s
fragment-check / coverage (pull_request) Successful in 7s
fragment-check / density (pull_request) Successful in 7s
fragment-check / check (pull_request) Successful in 0s
gitea-twin-check / check (pull_request) Successful in 6s
go-ci / lint + build + test (pull_request) Successful in 30s
go-ci / record reviewed vs landed commit (pull_request) Has been skipped
manifest-check / manifest-vs-tag consistency (pull_request) Successful in 7s
manifest-check / check (pull_request) Successful in 0s
prep-order-check / check (pull_request) Successful in 6s
readme-pin-check / mirror (pull_request) Successful in 7s
readme-pin-check / digest (pull_request) Successful in 6s
register-check / register-drift check (pull_request) Successful in 7s
register-check / check (pull_request) Successful in 0s
tests / workflow-schema (pull_request) Successful in 4s
tests / bats (pull_request) Successful in 32s
tests / dated-examples (pull_request) Successful in 5s
tests / contract-paths (pull_request) Successful in 4s
tests / shellcheck (pull_request) Successful in 4s
toolkit-self-gates / toolkit-self gates (PR's own rt) (pull_request) Successful in 7s
workflow-parse-check / toolkit-self parse guard and controls (pull_request) Successful in 4s
workflow-parse-check / workflow parse and schema (pull_request) Successful in 5s
workflow-parse-check / check (pull_request) Successful in 0s
go-ci / page landing-tree failure (pull_request) Has been skipped
2026-10-02 11:28:23 +02:00
Compare
claude left a comment

Review of head ff3345f3e4eff91aaff1e4cb3c67f0544490a299 (rt review-submit-check rc=0 against this SHA right before submitting).

Posted as COMMENT because this account authored the PR, so it cannot approve or request changes. Findings 1-3 are what I would hold the merge on.

The detection is sound. The merge arm grades both bodies before merging, and uncarriedAt's file-line offset is correct. The oracle test grades against MergeSections' own output and agrees with the refusal on all 11 pairs. go test ./internal/changelog passes at this head. Not run: cmd/rt tests, because the package doesn't build on Windows (syscall.Flock) and this was a Windows host.

Findings

1. internal/changelog/foldback.go:174: a pending intro line is reported as "moved" when the merge drops it. Describe labels every pending line above the first ### heading the merge would move it under [Unreleased]'s last heading. That is false when [Unreleased]'s last heading is not accepted, or when [Unreleased] has no ### heading at all: the inherited kind is then unaccepted or "", and the line is dropped. Measured on the oracle's own fixture pending intro inherits an unaccepted kind:

  in ## [v0.2.0] - 2026-10-02:
    line 15: Intro p14.    <- above the first ### heading; the merge would move it under [Unreleased]'s last heading

The last heading there is ### Notes. The same text appears with [Unreleased] = Just prose., which has no heading at all. No test checks this reason string for the unaccepted-inherited case.

2. foldback.go:165: on a CRLF changelog, a raw \r reaches the refusal text. PendingHeading and UncarriedLine.Text are copied verbatim from the records. Measured: "folding ## [v0.2.0] - 2026-10-02\r back under ..." and "line 5: Prose.\r <- before the first ### heading". In a terminal the \r returns to column 0, so the rest of the line overwrites the heading, line number and text. The merge grammar handles CRLF (classifyMergeRecord trims it), and Reason() already strips \r from Heading, but Text and PendingHeading are not stripped.

3. foldback.go:165: the header says "would lose N line(s)" but counts moved lines. With [Unreleased] = ### Fixed / - Later fix. and pending = Release intro. / ### Added / - Shipped., the output says would lose 1 line(s) and then lists that one line as the merge would move it. The header should say "lose or move", or count the two classes separately.

4. foldback.go:194: uncarriedAt assumes the body is a contiguous slice of recs. That holds today, but not under #1490's subtractCovered, as the PR body says. Suggestion: carry each record with its file line through to the merge input, so the merge input and the grading come from one sequence and the subtraction cannot split them.

5. foldback.go:182: duplicated text. The accepted-headings line is a copy of internal/prep/prep.go:175-180, and the "Move each line under one of them, or delete it, then re-run" remedy is duplicated too. One helper next to MergeKinds() would keep rt prep and rt recover-pending-cut from drifting apart.

6. foldback.go:153: a pure library error claims "Nothing was written." FoldBackDropError lives in internal/changelog and never writes anything, so that sentence describes what the caller did. It holds for today's only caller. Any future caller that prints Error() after a partial write (e.g. #1490's Except variant) would report it falsely. prep.UnreleasedDropError keeps this sentence in its own error type. Move it to the verb's red(...) wrapper.

PR body is out of date

It still says Stacked on #1486, head 8d7c1cd, and "this stacked PR gets no CI until it is retargeted". The PR now targets main (merge base 55a12b9) with head ff3345f. git range-diff e2fc222..8d7c1cd origin/main..ff3345f shows the only change is surrounding text in docs/integration.md from the rebase, but the Verification section names a head that is no longer the PR head. Please update the body and state the CI result on ff3345f.

🤖 Generated with Claude Code

Review of head `ff3345f3e4eff91aaff1e4cb3c67f0544490a299` (`rt review-submit-check` rc=0 against this SHA right before submitting). Posted as COMMENT because this account authored the PR, so it cannot approve or request changes. Findings 1-3 are what I would hold the merge on. **The detection is sound.** The merge arm grades both bodies before merging, and `uncarriedAt`'s file-line offset is correct. The oracle test grades against `MergeSections`' own output and agrees with the refusal on all 11 pairs. `go test ./internal/changelog` passes at this head. Not run: `cmd/rt` tests, because the package doesn't build on Windows (`syscall.Flock`) and this was a Windows host. ### Findings **1. `internal/changelog/foldback.go:174`: a pending intro line is reported as "moved" when the merge drops it.** `Describe` labels every pending line above the first `### ` heading `the merge would move it under [Unreleased]'s last heading`. That is false when [Unreleased]'s last heading is not accepted, or when [Unreleased] has no `### ` heading at all: the inherited kind is then unaccepted or `""`, and the line is dropped. Measured on the oracle's own fixture `pending intro inherits an unaccepted kind`: ``` in ## [v0.2.0] - 2026-10-02: line 15: Intro p14. <- above the first ### heading; the merge would move it under [Unreleased]'s last heading ``` The last heading there is `### Notes`. The same text appears with [Unreleased] = `Just prose.`, which has no heading at all. No test checks this reason string for the unaccepted-inherited case. **2. `foldback.go:165`: on a CRLF changelog, a raw `\r` reaches the refusal text.** `PendingHeading` and `UncarriedLine.Text` are copied verbatim from the records. Measured: `"folding ## [v0.2.0] - 2026-10-02\r back under ..."` and `"line 5: Prose.\r <- before the first ### heading"`. In a terminal the `\r` returns to column 0, so the rest of the line overwrites the heading, line number and text. The merge grammar handles CRLF (`classifyMergeRecord` trims it), and `Reason()` already strips `\r` from `Heading`, but `Text` and `PendingHeading` are not stripped. **3. `foldback.go:165`: the header says "would lose N line(s)" but counts moved lines.** With [Unreleased] = `### Fixed / - Later fix.` and pending = `Release intro. / ### Added / - Shipped.`, the output says `would lose 1 line(s)` and then lists that one line as `the merge would move it`. The header should say "lose or move", or count the two classes separately. **4. `foldback.go:194`: `uncarriedAt` assumes the body is a contiguous slice of `recs`.** That holds today, but not under #1490's `subtractCovered`, as the PR body says. Suggestion: carry each record with its file line through to the merge input, so the merge input and the grading come from one sequence and the subtraction cannot split them. **5. `foldback.go:182`: duplicated text.** The accepted-headings line is a copy of `internal/prep/prep.go:175-180`, and the "Move each line under one of them, or delete it, then re-run" remedy is duplicated too. One helper next to `MergeKinds()` would keep `rt prep` and `rt recover-pending-cut` from drifting apart. **6. `foldback.go:153`: a pure library error claims "Nothing was written."** `FoldBackDropError` lives in `internal/changelog` and never writes anything, so that sentence describes what the caller did. It holds for today's only caller. Any future caller that prints `Error()` after a partial write (e.g. #1490's Except variant) would report it falsely. `prep.UnreleasedDropError` keeps this sentence in its own error type. Move it to the verb's `red(...)` wrapper. ### PR body is out of date It still says **Stacked on #1486**, head `8d7c1cd`, and "this stacked PR gets no CI until it is retargeted". The PR now targets `main` (merge base `55a12b9`) with head `ff3345f`. `git range-diff e2fc222..8d7c1cd origin/main..ff3345f` shows the only change is surrounding text in `docs/integration.md` from the rebase, but the Verification section names a head that is no longer the PR head. Please update the body and state the CI result on `ff3345f`. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Author
Member

HOLD: I'm addressing review 7158 (findings 1-6, plus the out-of-date body) in a new commit on top of ff3345f3e4eff91aaff1e4cb3c67f0544490a299, with no amend and no force-push. I'll post the new full head SHA once it is pushed and the body is updated. Please wait for that before any approval read.

**HOLD**: I'm addressing review 7158 (findings 1-6, plus the out-of-date body) in a new commit on top of `ff3345f3e4eff91aaff1e4cb3c67f0544490a299`, with no amend and no force-push. I'll post the new full head SHA once it is pushed and the body is updated. Please wait for that before any approval read.
fix(recover-pending-cut): say what the fold-back merge does with each line
All checks were successful
fork-pr-approval-notice / explain fork workflow approval (pull_request_target) Successful in 5s
base-divergence-check / check (pull_request) Successful in 6s
changelog-body-check / changelog body Cold-Read linter (pull_request) Successful in 6s
changelog-body-check / check (pull_request) Successful in 0s
check-self-bootstrap / check (pull_request) Successful in 5s
gitea-twin-check / check (pull_request) Successful in 6s
go-ci / lint + build + test (pull_request) Successful in 30s
go-ci / record reviewed vs landed commit (pull_request) Has been skipped
manifest-check / manifest-vs-tag consistency (pull_request) Successful in 7s
manifest-check / check (pull_request) Successful in 0s
prep-order-check / check (pull_request) Successful in 6s
readme-pin-check / mirror (pull_request) Successful in 7s
readme-pin-check / digest (pull_request) Successful in 6s
register-check / register-drift check (pull_request) Successful in 7s
register-check / check (pull_request) Successful in 0s
tests / workflow-schema (pull_request) Successful in 4s
tests / bats (pull_request) Successful in 32s
tests / dated-examples (pull_request) Successful in 5s
tests / contract-paths (pull_request) Successful in 4s
tests / shellcheck (pull_request) Successful in 4s
workflow-parse-check / toolkit-self parse guard and controls (pull_request) Successful in 4s
workflow-parse-check / workflow parse and schema (pull_request) Successful in 5s
workflow-parse-check / check (pull_request) Successful in 0s
go-ci / page landing-tree failure (pull_request) Has been skipped
ac-closure-check / ac-closure check (pull_request) Successful in 7s
ac-closure-check / check (pull_request) Successful in 0s
fragment-check / coverage (pull_request) Successful in 7s
fragment-check / density (pull_request) Successful in 7s
fragment-check / check (pull_request) Successful in 0s
toolkit-self-gates / toolkit-self gates (PR's own rt) (pull_request) Successful in 7s
0935704fcb
Review 7158 on the fold-back refusal:

- A pending line above its first heading inherits [Unreleased]'s last
  ### heading. The refusal now records that heading and says the merge
  moves the line under it only when it is accepted; otherwise, or when
  [Unreleased] has no heading at all, it says the merge drops the line.
- The header counts dropped and moved lines separately.
- A CRLF changelog's trailing \r no longer reaches the rendered text. The
  struct still keeps each record verbatim.
- Both bodies are carried as records with their file lines, so the merge
  input and the grading come from one sequence. The line mapping holds for
  a non-contiguous body too, such as one filtered by #1490.
- The accepted-headings note and the remedy are shared with rt prep
  (changelog.AcceptedHeadingsNote, changelog.UncarriedRemedy). rt prep's
  text is byte-identical.
- "Nothing was written." moves from the library error to the verb, which
  is the one that knows.

Refs #1491.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Author
Member

HOLD released. New head: 0935704fcb110793e0c3da3e20367b11dd68d941. It is one new commit on ff3345f, with nothing rewritten.

  • It addresses findings 1-6 of review 7158. The PR body now targets main and maps each finding to its change and test.
  • CI on this head: all 30 commit statuses report success.
  • Measured, out of scope: rt prep's refusal has the same CRLF \r exposure. It is left for a separate issue.

Before submitting, pin the read to this head:

rt review-submit-check --owner frankenbit --repo release-toolkit --pr 1493 --reviewed-head 0935704fcb110793e0c3da3e20367b11dd68d941
**HOLD released.** New head: `0935704fcb110793e0c3da3e20367b11dd68d941`. It is one new commit on `ff3345f`, with nothing rewritten. - It addresses findings 1-6 of review 7158. The PR body now targets `main` and maps each finding to its change and test. - CI on this head: all 30 commit statuses report success. - Measured, out of scope: `rt prep`'s refusal has the same CRLF `\r` exposure. It is left for a separate issue. Before submitting, pin the read to this head: ``` rt review-submit-check --owner frankenbit --repo release-toolkit --pr 1493 --reviewed-head 0935704fcb110793e0c3da3e20367b11dd68d941 ```
claude left a comment

Re-review of head 0935704fcb110793e0c3da3e20367b11dd68d941 (rt review-submit-check rc=0 against this SHA right before submitting). Posted as COMMENT because this account authored the PR, so it cannot approve. Review 7158 is addressed at this head, and nothing here holds the merge.

Process

One new commit on ff3345f, with nothing rewritten (git merge-base --is-ancestor holds). HOLD was announced before the push (113872) and released after it (113876).

Findings 1-6: verified at source

Messages below are measured at 0935704 with the inputs used in 7158:

# Result
1 Unaccepted inherited heading: the merge would drop it under [Unreleased]'s ### Notes, not an accepted heading. No heading at all: [Unreleased] has no ### heading to carry it; the merge would drop it.
2 CRLF input: the refusal contains no \r. Text/PendingHeading stay verbatim in the struct, and unCR applies only when rendering.
3 Move-only case: header reads would move 1 line(s) under a heading they were not written under. Drop-only reads would drop N line(s), and the mixed case reports both counts.
4 bodyRecords / joinRecords / uncarriedIn: the merge input and the grading come from one []fileRecord. Line = rs[Line-1].line maps correctly for any subsequence.
5 AcceptedHeadingsNote() / UncarriedRemedy(verb) sit next to MergeKinds. prep.UnreleasedDropError's output is byte-identical to the old literal (old %s.\n + remedy vs new note + "\n" + remedy).
6 "Nothing was written." is gone from FoldBackDropError and now comes from the verb's red(...).

Verification

  • go vet on the changed packages, gofmt -l clean, and go test -count=1 ./... all pass in golang:1.25 on a fresh LF clone of 0935704, with no failing package. That includes cmd/rt, which I could not run on the first pass.
  • Live commit status for 0935704: success, 30/30.
  • PR body: base main, head 0935704, no more "stacked on #1486".

Nit (non-blocking)

The verb now prints ...Accepted headings (...)\nNothing was written.\nMove each line under one of them, ..., so "them" no longer follows the headings it refers to. Printing "Nothing was written." before Describe(...), or as part of its first line, restores that. This is a wording change only.

The CRLF \r in rt prep's UnreleasedDropError is acknowledged as out of scope here. It needs its own issue.

🤖 Generated with Claude Code

Re-review of head `0935704fcb110793e0c3da3e20367b11dd68d941` (`rt review-submit-check` rc=0 against this SHA right before submitting). Posted as COMMENT because this account authored the PR, so it cannot approve. **Review 7158 is addressed at this head, and nothing here holds the merge.** ### Process One new commit on `ff3345f`, with nothing rewritten (`git merge-base --is-ancestor` holds). HOLD was announced before the push (113872) and released after it (113876). ### Findings 1-6: verified at source Messages below are measured at `0935704` with the inputs used in 7158: | # | Result | |---|---| | 1 | Unaccepted inherited heading: `the merge would drop it under [Unreleased]'s ### Notes, not an accepted heading`. No heading at all: `[Unreleased] has no ### heading to carry it; the merge would drop it`. | | 2 | CRLF input: the refusal contains no `\r`. `Text`/`PendingHeading` stay verbatim in the struct, and `unCR` applies only when rendering. | | 3 | Move-only case: header reads `would move 1 line(s) under a heading they were not written under`. Drop-only reads `would drop N line(s)`, and the mixed case reports both counts. | | 4 | `bodyRecords` / `joinRecords` / `uncarriedIn`: the merge input and the grading come from one `[]fileRecord`. `Line = rs[Line-1].line` maps correctly for any subsequence. | | 5 | `AcceptedHeadingsNote()` / `UncarriedRemedy(verb)` sit next to `MergeKinds`. `prep.UnreleasedDropError`'s output is byte-identical to the old literal (old `%s.\n` + remedy vs new note + `"\n"` + remedy). | | 6 | "Nothing was written." is gone from `FoldBackDropError` and now comes from the verb's `red(...)`. | ### Verification - `go vet` on the changed packages, `gofmt -l` clean, and `go test -count=1 ./...` all pass in `golang:1.25` on a fresh LF clone of `0935704`, with no failing package. That includes `cmd/rt`, which I could not run on the first pass. - Live commit status for `0935704`: `success`, 30/30. - PR body: base `main`, head `0935704`, no more "stacked on #1486". ### Nit (non-blocking) The verb now prints `...Accepted headings (...)\nNothing was written.\nMove each line under one of them, ...`, so "them" no longer follows the headings it refers to. Printing "Nothing was written." before `Describe(...)`, or as part of its first line, restores that. This is a wording change only. The CRLF `\r` in `rt prep`'s `UnreleasedDropError` is acknowledged as out of scope here. It needs its own issue. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Author
Member

HOLD: I'm applying the non-blocking nit from re-review 7160 (the remedy's "them" should follow the headings it refers to) as a new commit on top of 0935704fcb110793e0c3da3e20367b11dd68d941, with no amend and no force-push. I'll post the new full head SHA once it is pushed.

**HOLD**: I'm applying the non-blocking nit from re-review 7160 (the remedy's "them" should follow the headings it refers to) as a new commit on top of `0935704fcb110793e0c3da3e20367b11dd68d941`, with no amend and no force-push. I'll post the new full head SHA once it is pushed.
fix(recover-pending-cut): open the fold-back refusal with what was not written
Some checks failed
tests / bats (pull_request) Successful in 32s
tests / dated-examples (pull_request) Successful in 5s
tests / contract-paths (pull_request) Successful in 4s
tests / shellcheck (pull_request) Successful in 4s
workflow-parse-check / toolkit-self parse guard and controls (pull_request) Successful in 4s
workflow-parse-check / workflow parse and schema (pull_request) Successful in 4s
workflow-parse-check / check (pull_request) Successful in 0s
go-ci / page landing-tree failure (pull_request) Has been skipped
ac-closure-check / ac-closure check (pull_request) Successful in 6s
ac-closure-check / check (pull_request) Successful in 0s
fragment-check / coverage (pull_request) Successful in 7s
fragment-check / density (pull_request) Successful in 7s
fragment-check / check (pull_request) Successful in 0s
toolkit-self-gates / toolkit-self gates (PR's own rt) (pull_request) Successful in 7s
check-self-bootstrap / check (push) Successful in 5s
gitea-twin-check / check (push) Successful in 5s
go-ci / lint + build + test (push) Successful in 29s
go-ci / record reviewed vs landed commit (push) Failing after 3s
prepared-uncut-check / toolkit-self prepared-uncut controls (push) Successful in 5s
prepared-uncut-check / prepared-but-uncut release (push) Successful in 7s
prepared-uncut-check / check (push) Successful in 0s
release / decide + act (push) Successful in 9s
release / release (push) Successful in 0s
tests / workflow-schema (push) Successful in 3s
tests / bats (push) Successful in 31s
tests / dated-examples (push) Successful in 5s
tests / contract-paths (push) Successful in 4s
tests / shellcheck (push) Successful in 3s
go-ci / page landing-tree failure (push) Has been skipped
release / fire-cut (push) Has been skipped
2060ed13ff
Re-review 7160, non-blocking nit: "Nothing was written." sat between the
accepted-headings line and "Move each line under one of them", so "them"
no longer followed the headings it refers to. The verb now opens with
"nothing was written." as the outcome, then the library's explanation,
the headings, and the remedy directly after them.

Refs #1491.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
alex approved these changes 2026-10-02 12:14:41 +02:00
Author
Member

HOLD released. New head: 2060ed13ffb3834928b2aad3a32916eab485acb3. It is one new commit on 0935704, with nothing rewritten.

  • It applies the re-review 7160 nit. The refusal now opens with recover-pending-cut: nothing was written., and "Move each line under one of them" directly follows the accepted-headings line.
  • The verb test pins both, and the old ordering is a killed mutant.
  • CI on this head: all 30 commit statuses report success. The PR body is updated to this head.

Before submitting, pin the read to this head:

rt review-submit-check --owner frankenbit --repo release-toolkit --pr 1493 --reviewed-head 2060ed13ffb3834928b2aad3a32916eab485acb3
**HOLD released.** New head: `2060ed13ffb3834928b2aad3a32916eab485acb3`. It is one new commit on `0935704`, with nothing rewritten. - It applies the re-review 7160 nit. The refusal now opens with `recover-pending-cut: nothing was written.`, and "Move each line under one of them" directly follows the accepted-headings line. - The verb test pins both, and the old ordering is a killed mutant. - CI on this head: all 30 commit statuses report success. The PR body is updated to this head. Before submitting, pin the read to this head: ``` rt review-submit-check --owner frankenbit --repo release-toolkit --pr 1493 --reviewed-head 2060ed13ffb3834928b2aad3a32916eab485acb3 ```
claude merged commit 2060ed13ff into main 2026-10-02 12:20:51 +02:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
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!1493
No description provided.