Recognize multi-record messages so squash inheritance stops losing identity and provenance (#60) - #86
Merged
Merged
Conversation
…entity and provenance (#60) Three findings, in the order the issue settled on after two self-corrections: 1. squash-preserve existed and worked, but nothing invoked it, and even when it was invoked it folded every inherited record in the range into one merged record — losing an identity outright once a range declared more than one Record-Id, and naming Provenance after only the newest source commit regardless of how many records actually contributed. 2. GitHub's squash button pastes full commit messages into the merge body, so the trailer text survives, but git interpret-trailers (SPEC B1) reads only the message's last paragraph. Every earlier commit's trailer block silently became prose; validate agreed the message was fine. 3. A folded record with an ambiguous identity could never be Follows:'d or Supersedes:'d again — the graph lost it for good. Fix, in the order the issue asked for: - SPEC S2.4: a message MAY carry several record blocks. The message's own last paragraph keeps its existing, unconditional recognition; every other paragraph is an additional block only when it is entirely trailer-shaped AND declares a Record-Id (parseRecordBlocks, core/trailers.ts). The identity gate is what keeps this from reopening B2's own worked example (Context:/Source:, trailer-shaped, no identity, correctly still prose). A message with at most one Record-Id has exactly one block, so a single-record message parses byte-identically to before. - squash-preserve (core/squash.ts) now emits one block per inherited record (SquashPlan.blocks), each keeping its own Record-Id and its own accurate Provenance: inherited <sha>. X-Inherited-From is no longer written; an old note that carries it still resolves, because X-<Name>: is an ordinary preserved extension. - core/index-db.ts (schema v2, a `block` column) and core/query.ts recover every block a commit or note carries, not only the last, so `commitlore context` shows each record with its own resolvable id. `commitlore validate` checks every block, not only the message's last paragraph. - `commitlore doctor` gained a `squash-conservation` check: it warns when a local branch that looks like an un-preserved squash source declared a Record-Id that HEAD's history cannot find. Detection, not prevention — nothing local can hook GitHub's server-side squash button. Full reasoning, including the CI-step alternative that was ruled out, is in ADR-0014. Measured (spec/SPEC.md S2.4, docs/adr/ADR-0014-multi-record-grammar.md): - local `git merge --squash` + `squash-preserve --target HEAD`: both branch records now reach the merge commit, each with its own id and its own true source sha (previously: one merged record, Provenance correct for one of them, Record-Id dropped once the range held two). - GitHub-style squash message (subject, blank line, then concatenated commit messages): `commitlore context` recovers all record blocks; previously git's own B1 rule silently limited every consumer to the last one. - a single-record message parses byte-identically (123/123 fixture-driven trailers.test.ts cases, all boundary fixtures B1-B8 included). - `commitlore context` prints each recovered record next to a resolvable id. Full suite: 1289 passed, 1 skipped, 35 files (up from the 1253/1/35 dev baseline). Limit: parseRecordBlocks only recognizes a non-final block by its declared Record-Id, so an unidentified inherited record beyond the first stays recoverable in the plan that computed it but not in a later re-parse of stored text; squash-preserve orders unidentified blocks last so the common case (at most one) is unaffected. Limit: multi-block reference checking (Follows:/Supersedes:) does not resolve one block's reference against a sibling block declared by the same commit; each block is still checked against every earlier commit in history. Warn: index-db.ts SCHEMA_VERSION is now 2 for the added `block` column — bump it again, not the shape in place, the next time the trailers table changes. Ruled-out: minting a fresh Record-Id for an inherited record that never declared one | no other code path in this project invents identity; Record-Id is always author-declared, and doing it here would be new scope beyond what the issue asked for Ruled-out: a CI step comparing a PR's commits against its post-merge squash commit, as the primary mechanism for finding 1 | needs a GitHub API dependency this tool takes nowhere else, and can only run after the squash is already pushed; doctor catches the same mistake locally, before push, when the source branch is still in refs/heads Verified: full suite (npm test) 1289 passed, 1 skipped, 35 files; tsc --noEmit clean; manual verification of all four cases in the issue's VERIFY list against dist/commitlore.mjs Blast: system Undo: costly Certainty: tentative Provenance: authored Record-Id: r-multirec01 CommitLore-Version: 2.0.0
CommitLore — record lintTrailers: clean — 1 commit in Active constraints for the paths this PR touchesLimits (30)
Ruled out (79)
Warnings (48)
Trailer violations fail this check. Active constraints are informational — they are what the repository already decided, not a verdict on this PR. |
5 tasks
This was referenced Jul 27, 2026
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.
Summary
Closes #60. The issue's own author self-corrected twice before landing on the real finding; this PR fixes all three, in the order requested:
Record-Id. The identity gate is what keeps this from reopening B2's own worked example (Context:/Source:— trailer-shaped, no identity, correctly still prose). A message with at most oneRecord-Idanywhere has exactly one block, so a single-record message parses byte-identically to before — backward compatibility is a property of the grammar, not a shim.X-Inherited-Frombecomes unnecessary and is no longer written.squash-preserveemits one block per inherited record (SquashPlan.blocks, replacing the old singlemergedrecord), each with its ownRecord-Idand its own accurateProvenance: inherited <sha>naming that record's true source, never a different record's. Old notes carryingX-Inherited-Fromstill resolve — it's an ordinary preserved extension (SPEC §3.2).commitlore doctorgained asquash-conservationcheck. It warns when a local branch that looks like an un-preserved squash source declared aRecord-IdHEAD's history can't find. Detection, not prevention — nothing local can hook GitHub's server-side squash button.Ruled-out: a CI step— see ADR-0014 for the full reasoning (needs a GitHub API dependency this tool takes nowhere else, and can only run after the squash is already pushed).No trailer key was added.
Supersedes:still means deliberate replacement.What changed
src/core/trailers.ts—parseRecordBlocks, the multi-record grammar primitive.src/core/squash.ts—planSquashgroups by identity and folds within an identity only (the old code folded across unrelated records too, which was part of the underlying bug);SquashPlan.blocksreplacesmerged.src/core/notes.ts—writeRecordBlocks/readRecordBlocks, the multi-block note read/write path;writeRecord/readRecordunchanged for single-block callers.src/core/index-db.ts— schema v2 (trailers.blockcolumn, derived/disposable per ADR-0003) and a narrow secondgit logpass (mirroring the existingreadPathspattern) that recovers extra blocks only for the sliver of commits that already carry ≥1 trailer.src/core/query.ts—groupByCommit/identityOf/foldMirroredNotesare block-aware, socommitlore contextand every consumer route show each recovered record separately.src/commands/validate.ts— validates every block a message carries, not only the last paragraph; the existing merge-title special case (A PR title of the form 'word: description' becomes an invalid trailer in GitHub's merge commit #76) stays scoped to the last block only.src/commands/doctor.ts— newsquash-conservationcheck.spec/SPEC.md§2.4,docs/adr/ADR-0014-multi-record-grammar.md,CHANGELOG.md.Measured — before / after, all four verification cases
Baseline confirmed first: 1253 passed / 1 skipped / 35 files on
dev.1. Local
git merge --squash+squash-preserve --target HEADBefore: one merged record; with two distinct
Record-Ids in range, the old code declared neither (the merge record declares none), andProvenance:named only the newest source. After: both ids survive, each block'sProvenance:names its own true source commit —a05a7dea…forr-verify01,6ee3634…forr-verify02, not both pointing at the same sha.2. GitHub-style squash message (subject, blank line, then concatenated commit messages)
Before (this is the literal reproduction from the issue's first correction): 2 records in, 1 out,
validateexit 0, nothing said so. After: bothr-ghtest1andr-ghtest2recovered, each with a resolvable id.3. Single-record message parses byte-identically
git interpret-trailers --parseandcommitlore parseagree exactly, including a record whoseRecord-IdandProvenancesit in canonical §3 vocabulary order (Record-IdbeforeProvenance) — the ordering that made a naive "split right afterRecord-Id" design wrong during development; the shipped grammar only draws a boundary when a secondRecord-Idjustifies one. All 123 fixture-driven cases intest/trailers.test.ts(spec/fixtures/valid,spec/fixtures/boundaryB1–B8,spec/fixtures/invalid) pass unchanged.4.
commitlore contextshows each recovered record with a resolvable idDemonstrated in cases 1 and 2 above —
r-verify01/r-verify02andr-ghtest1/r-ghtest2each print next to their own id.Test plan
npm run typecheck— cleannpm test— 1289 passed, 1 skipped, 35 files (up from the 1253/1/35 baseline; net new/updated coverage intrailers.test.ts,squash.test.ts,action-preserve.test.ts,index-db.test.ts,doctor.test.ts,validate.test.ts)dist/commitlore.mjsbuild (transcript above)node dist/commitlore.mjs validate --commit HEADpasses on this branch's commitdist/rebuilt and committed alongsidesrc/, matching this repo's existing convention (.gitignore's own comment; confirmed against a priordevcommit)Known, documented limitations (not silent)
Record-Idis recoverable from the plansquash-preservecomputes, but if there is more than one such unidentified record, only the last one survives a later re-parse of already-written text (the grammar can only use identity to tell a non-final block apart from body prose).squash-preserveorders unidentified blocks last, so the common case — at most one — is unaffected, andwarningsForsays so when it isn't.Follows:/Supersedes:across two blocks declared by the same commit is not resolved specially; each block is still checked against every earlier commit in history.Both are named explicitly in ADR-0014 and in code comments, not discovered later.