Refuse the Ruled-out: split that provably went wrong, and report the rest - #375
Merged
Conversation
CommitLore — record lintTrailers: clean — 1 commit in Active constraints for the paths this PR touchesLimits (133)
Ruled out (305)
Truncated: 191 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. |
…rest `Ruled-out:` separates on the first `|` and has no escape, so an alternative that quotes a shell pipeline is truncated mid-sentence. The record then cannot match the thing it rules out: against a proposal quoting the real alternative, `guard --threshold 0` returned no match at all, while the truncated fragment matched at 0.75. Nothing said so. `validate` exited 0 with `shape ok`, and `commitlore ruled-out` printed the value verbatim -- which reads exactly like a value whose separator landed where its author meant it to. Which end to split from is a question about real records, so it was counted rather than argued. This history holds 620 distinct `Ruled-out:` values. Three carry more than one pipe. Two of those three carry it in the *reason* -- `||` quoted from shell prose in aa68a9a, `.mjs|.js` quoted from a filename alternation in 616005d -- and both parse correctly today. The third, 7bf6ced, carries it in the alternative (`irm | iex`) and is a live instance of this bug in our own history. Splitting on the last pipe would destroy two correct records to rescue one broken one, so the first pipe stays the separator. That leaves the silent half, and it is closed in two pieces because the evidence does not support one. An alternative whose code span the separator closes outside of is not a judgement about intent. The span opened before the pipe and closed after it, so the pipe was inside quoted text and the alternative is a fragment ending mid-span. That is a `format` violation now, refused at commit time by the schema. It rejects none of the 620 -- backticks are balanced in every one -- and none of the fixtures. A value that merely carries a second pipe cannot be refused without taking the two correct records with it, so `validate` warns and quotes back the alternative the split produced. Only the author knows which split was meant, and the commit-msg hook is the last moment they can still change it. For records already written, where validate can no longer reach, `ruled-out` and `context` annotate the line, and `guard` carries a `caveat:` line into the stderr block a PreToolUse hook routes back to the agent. The split itself moved out of `core/guard.ts` into `core/trailers.ts`, because four callers now need the same answer about one grammar and three of them are not the matcher. Record-Id: r-pipesplit Limit: test/dogfood.test.ts validates every record in this history, so a new violation class is only available if it rejects none of the 620 Ruled-out: values already written Ruled-out: Splitting on the last pipe instead of the first | this repository's own records refute the premise that reasons rarely contain a pipe: two of the three multi-pipe values carry it in the reason, so the change would break two correct records to fix one Ruled-out: Defining a backslash escape for the separator | that is a new grammar rather than a clarification -- parser, renderer and validator would all have to agree on unescaping, and it changes what a conforming record looks like, which needs sign-off this fix does not have Ruled-out: Making any value with more than one pipe a format violation | it invalidates 616005d and aa68a9a, whose extra pipe sits in the reason and whose split is already correct, and dogfood validates all of history Ruled-out: Having guard score both the first-pipe and last-pipe splits and keep the better one | it invents an alternative no author wrote and then reports it as recorded, which puts the silent wrong answer in a new place instead of removing it Ruled-out: Rendering every Ruled-out: value pre-split in the query output | it rewrites 620 lines of output to mark 3, and the annotation is only information where the split is actually in doubt Ruled-out: Warning on the code-span case rather than refusing it | a span that crosses the separator is provable rather than suspected, and this repository refuses what it can prove instead of hoping the author reads stderr Certainty: firm Blast: module Undo: easy Verified: the issue reproduced against 0.5.1 -- validate exit 0 with shape ok, guard matched:false at --threshold 0 against the real alternative and 0.75 against the truncated fragment, and a backslash before the pipe was kept and split anyway; the 620-value census was taken with the real parser over git log --all; 14 tests were run red first across trailers, guard, validate and query; after the fix trailers, guard, validate, schema, dogfood, squash, query, stale, index-db and cli all pass, dogfood included, so no past record was invalidated; typecheck clean, dist rebuilt, spec/verify.sh reports 26 fixtures, check-readme-numbers clean Unverified: whether any record outside this repository puts a pipe in an alternative without a code span -- that class is warned about, not refused, and the warning is only read by whoever is committing; core/inject.ts still renders Ruled-out: values verbatim, because annotating there would spend an entry's budget and could evict another record, so an agent reading injected context alone does not see the caveat CommitLore-Version: 2.0.0
MongLong0214
force-pushed
the
fix-issue-372
branch
from
August 1, 2026 06:52
d6949ac to
b4fa571
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 #372
The defect, reproduced
Against 0.5.1 the issue's record is accepted, and then cannot match the thing it rules out.
\|does not escape: the alternative comes back asshelling out to grep \and the value still splits at that pipe.The split decision, from real records
The issue proposes splitting on the last pipe, on the premise that reasons rarely contain one. This repository's own history refutes that premise. Counted with the real parser over
git log --all:Ruled-out:valuesThe three:
616005d— reason quotes.mjs|.js. Parses correctly today.aa68a9a— reason quotes||. Parses correctly today.7bf6ced— alternative isPassing the version through $args so irm | iex could take one, truncated at... so irm. A live instance of Ruled-out: splits on the first pipe, so an alternative containing '|' is truncated and cannot match #372 in our own history.Splitting on the last pipe would destroy two correct records to rescue one broken one. Rejected on the evidence.
What ships
The first pipe stays the separator, so no existing record changes meaning. The silent-acceptance half is closed in two pieces, because the evidence does not support one.
Refused — a
formatviolation, exit 1, at commit time: an alternative whose code span the separator closes outside of, meaning an odd number of backticks before the first|. This is not an inference about intent. The span opened before the pipe and closed after it, so the pipe was inside quoted text and the alternative is a fragment ending mid-span. It rejects none of the 620 records and none of the fixtures.Reported — a warning, exit unchanged: any other value with more than one pipe. This cannot be a violation. It would invalidate
616005dandaa68a9a, which are correct, andtest/dogfood.test.tsvalidates all of history.validatequotes back the alternative the split produced, because only the author knows which split was meant.Made visible on read, for records already written where
validatecan no longer reach:commitlore ruled-outandcontextannotate the line with the alternative the split produced.commitlore guardadds amalformed:ambiguous-separatorsignal and acaveat:line to the stderr block a PreToolUse hook routes back to the agent.After
With the pipe out of the alternative, the record matches what it rules out again —
matched: trueat 0.39.Spec change
SPEC §3.1 gains a
The Ruled-out: separatorsubsection: the first|separates, a backslash in front of one is not an escape, an alternative MUST NOT contain a|and a reason MAY, plus the two rules above. The §3.1 grammar cell and the §6.2formatrow are extended to match, and the four READMEs' vocabulary tables now state the separator rule — the issue notes there was no escaping guidance anywhere.This is a narrowing, and worth a reviewer's eye. The test applied was: does any record that conforms today stop conforming? No — all 620 real records, every spec fixture and every contract case keep their status. A
\|escape, the option with the widest blast radius, was not taken: it is a new grammar rather than a clarification, and it changes what a conforming record looks like.Verification
test/trailers.test.ts, 2 intest/guard.test.ts, 3 intest/validate.test.ts, 1 intest/query.test.ts. No existing test was weakened.npx vitest runon trailers, guard, validate, schema, dogfood, squash, query, stale, index-db, cli — 638 passed, 0 failed. Dogfood included, so no past record was invalidated.npm run typecheckclean,npm run buildrebuiltdist/,bash spec/verify.shreportsOK: 26 fixtures + README example sync + vocab table,node scripts/check-readme-numbers.mjsclean.Known gap
core/inject.tsstill rendersRuled-out:values verbatim. Annotating there would spend an entry's character budget and could evict another record, so an agent reading injected context alone does not see the caveat — it sees it onguard, the route SPEC §5 actually assigns toRuled-out:. Recorded in the commit'sUnverified:.