A block with no CommitLore key is not a record - #338
Merged
Conversation
CommitLore — record lintTrailers: clean — 1 commit in Active constraints for the paths this PR touchesLimits (68)
Ruled out (154)
Warnings (51)
Trailer violations fail this check. Active constraints are informational — they are what the repository already decided, not a verdict on this PR. |
On a repository with zero records, `index` populated 106 rows by accepting any RFC-822-shaped `key: value` line -- conventional-commit prefixes (`ax:`, `fix:`, `docs:`), a Homebrew digest (`sha256:`), arbitrary body fields. `doctor` called that healthy. `context` is wired into the PreToolUse hook, so what an agent was handed before editing was a commit subject presented as a recorded decision. `stale` reads git and said 0, so two commands in one tool disagreed about whether the repository had records at all. There was already a filter here, and it was not wrong -- it was answering a different question. `CONVENTIONAL_TRAILER_KEYS` decides which trailers to keep *inside* a record, and its comment argues for a denylist so a project's own `Ticket:` survives beside a `Limit:`. That reasoning holds, and SPEC agrees: an implementation must preserve keys it cannot interpret. What nothing asked was the prior question -- **is this a record?** -- and a denylist cannot answer it, because the set of keys nobody has claimed is unbounded. So the prior question is asked separately. `isCommitLoreKey` is SPEC §3's fifteen keys plus `X-<Name>:`, which §3 gives a slot of its own; a block with none of them is dropped rather than indexed. The denylist keeps its job unchanged, and a `Ticket:` inside a real record is still preserved. One case cannot be decided and is written down instead of guessed at. `Verified:` is vocabulary, so a release note using it as a field is indistinguishable from a record using it for what it means. Reading context to tell them apart is how a tool starts discarding real records, so the block is a record and both keys are kept. The test says so, rather than leaving it to be rediscovered as a bug. Record-Id: r-recordgate335 Limit: a denylist cannot decide whether something is a record, because the keys nobody has claimed are unbounded; that question needs the vocabulary, and the two must not be answered by one filter Limit: `Verified:` in a release note is indistinguishable from `Verified:` in a record, and no context signal separates them without risking real records Ruled-out: Adding the observed noise keys to the denylist | `ax`, `sha256` and the rest are one repository's accident, and the next repository invents different ones Ruled-out: Requiring `Record-Id:` to make a block a record | `stale` needs it for identity, but a commit carrying only `Limit:` is a record SPEC recognises and dropping it would lose real context Ruled-out: Inferring intent from position or neighbouring lines | that is guessing, and the failure mode is discarding a record somebody wrote on purpose Certainty: firm Blast: local Undo: easy Verified: the issue's own repro executed against the build -- a repository of conventional commits plus a `sha256:` body now reports 0 trailers from doctor and 0 records from stale, and `context` returns nothing where it previously served a commit subject; index-db suite at 49 including four assertions that failed before the change; full suite 78 files, 1922 passed, 1 skipped Unverified: the reporter's own repository, where the 106 rows were observed
MongLong0214
force-pushed
the
fix-issue-335
branch
from
August 1, 2026 00:14
4d51e47 to
a83ebe3
Compare
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 #335.
The reported state
A repository with zero CommitLore records, where
indexhad 106 rows:contextis wired into the PreToolUse hook, so what the agent was handed before editing was a commit subject presented as a recorded decision:There was already a filter, and it was not wrong
CONVENTIONAL_TRAILER_KEYSexists, and its comment argues its case well:That reasoning holds, and SPEC agrees: an implementation must preserve keys it cannot interpret.
But it was answering a different question than the one that failed. It decides which trailers to keep inside a record. Nothing asked the prior question — is this a record? — and a denylist cannot answer it, because the set of keys nobody has claimed is unbounded.
The fix asks the prior question separately
isCommitLoreKey: SPEC §3's fifteen keys, or anX-<Name>:extension, which §3 gives a slot of its own. A block with none of them is not a record.The denylist keeps its job unchanged. A
Ticket:beside aLimit:is still preserved — asserted.One case cannot be decided, so it is written down instead
Verified:is vocabulary. A release note using it as a field is indistinguishable from a record using it for what it means, and reading surrounding context to tell them apart is how a tool starts discarding records somebody wrote on purpose. So that block is a record and both keys are kept, and there is a test saying so rather than leaving it to be rediscovered.This means one of the issue's listed noise keys —
Verified:— is not filtered, deliberately.The issue's repro, executed against the build
doctorandstaleagree, and the agent is handed nothing.Verification
test/index-db.test.ts— 49 passed; four of the new assertions failed before the changenpm run typecheckclean;dist/rebuiltNot fixed here:
doctor's wording still says "trailers", which the issue also asked about. With the count now correct that reads accurately, and changing the noun is a separate, smaller change.