A draft that only git misreads was the one thing nothing said out loud - #834
Merged
Conversation
`squash-preserve --message-file` writes one paragraph per inherited record, and git decides a commit's trailers by reading only the last paragraph. On a four-commit branch that leaves 9 of 13 trailer lines as ordinary prose to `git interpret-trailers` and `git log --format=%(trailers)`, in a merge commit nobody may rewrite (#833). The report's suggested repair -- fold every record into the final paragraph -- was implemented and measured before being rejected. It moves the loss rather than closing it: shape git-native parse SPEC 2.4 grammar one paragraph per record last block only every record, with its id all records in one every trailer line collapses to one record Two existing tests fail on the folded shape and they are right to. One is named "every branch record reaches the merge commit with its own id and true source sha", and under the fold every `Record-Id` but the first is unreachable. Splitting a single block back into records needs a boundary rule, and the vocabulary order emits `Record-Id` before `Provenance`, so neither "split before Record-Id" nor "split after Provenance" is clean. That is a grammar decision, not a bug fix. Refusing the write was implemented next and also rejected. It closes the reported harm by disabling the repair for the common case: most branches carry more than one record, and CommitLore itself loses nothing -- `parseRecordBlocks` walks every paragraph and recovers all of them, which is what the D3 repair has always relied on. So the command says what the draft costs. The notice names the block count, says git will see only the last paragraph, says CommitLore reads all of them, and points at `--target`, which SPEC 1 already calls the destination for records inherited across squash merges and which git does not parse as a trailer block. The report's real complaint was that nothing said any of this. The draft reads correctly, `commitlore parse` accepts it, and `commitlore validate` does not object. Now the command that writes it does. Record-Id: r-833multiblocknotice Provenance: authored Certainty: firm Blast: local Undo: easy Ruled-out: folding every record into the final paragraph | it satisfies git by collapsing the records for CommitLore, and the boundary rule that would split them back has no clean form while `Record-Id` is emitted before `Provenance` Ruled-out: refusing to write a multi-block draft | it disables the documented repair for most branches to prevent a loss that is real for other tooling and not for this one, and the notes path was already available for callers who need git-visible fidelity Ruled-out: changing SPEC 2.4's record boundary here | the docstring already says a record is "terminated by Record-Id" while the code splits on paragraphs, and reconciling those is a format decision that belongs with the owner rather than inside a bug fix Limit: this makes the cost audible, it does not remove it. A merge commit written from a multi-block draft still carries records that git-native tooling will not report as trailers, and `--target` remains the only way to have every record visible on both channels Verified: reproduced against `git interpret-trailers --parse` on a real two-record branch -- draft carries 2, git reports 1. The notice fires on multi-block and is absent on single-block, and the new test proves the gap by arrival: it commits the draft, asks git what it sees, and asserts the grammar recovers strictly more. 15 squash tests pass, typecheck clean CommitLore-Version: 2.0.0
CommitLore — record lintTrailers: clean — 1 commit in Active constraints for the paths this PR touchesLimits (254)
Ruled out (391)
Truncated: 388 lines omitted — the comment hit GitHub's 65000 character limit. Trailer violations fail this check. Active constraints are informational — they are what the repository already decided, not a verdict on this PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #833.
squash-preserve --message-filewrites one paragraph per inherited record, and git decides a commit's trailers by reading only the last paragraph. On a four-commit branch that leaves 9 of 13 trailer lines as ordinary prose togit interpret-trailersandgit log --format=%(trailers), in a merge commit nobody may rewrite.I implemented the suggested fix first, then measured what it costs
Folding every record into the final paragraph moves the loss rather than closing it:
Two existing tests fail on the folded shape and they are right to. One is named "every branch record reaches the merge commit with its own id and true source sha" — under the fold, every
Record-Idbut the first is unreachable.Splitting a single block back into records needs a boundary rule, and the vocabulary order emits
Record-IdbeforeProvenance, so neither "split beforeRecord-Id" nor "split afterProvenance" is clean. That is a grammar decision, not a bug fix.I implemented refusal second, and rejected that too
Refusing the write closes the reported harm by disabling the repair for the common case. Most branches carry more than one record, and CommitLore itself loses nothing —
parseRecordBlockswalks every paragraph and recovers all of them, which is what the D3 repair has always relied on.So the command says what the draft costs
The report's real complaint was that nothing said any of this: the draft reads correctly,
commitlore parseaccepts it, andcommitlore validatedoes not object. Now the command that writes it does.--targetis not a workaround — SPEC §1 already calls notes "the destination for records inherited across squash merges".Verification
Reproduced against
git interpret-trailers --parseon a real two-record branch: draft carries 2, git reports 1. The notice fires on multi-block and is absent on single-block.The new test proves the gap by arrival: it commits the draft, asks git what it actually sees, and asserts the grammar recovers strictly more. 20 tests pass, typecheck clean, canonical artifact rebuilt and verified in the same commit.
Left for the owner
The docstring on
parseRecordBlockssays a record is "terminated byRecord-Id:" while the code splits on paragraphs. Reconciling those is a format decision, and it is the only route to records surviving on both channels.