Skip to content

Commit 56fa627

Browse files
committed
schema: count out-of-order run before reporting min shortfall
claimOutOfOrder previously emitted "matched 1 times, required at least N" the moment it recovered, even when the document contained a contiguous run of N+ same-scope headings before the expected position. A doc like `## B, ## B, ## A` against an `A, B (repeat: min 2)` schema satisfies B's minimum count — only the ordering is wrong, which the out-of-order message already calls out. The spurious cardinality diagnostic added confusion. The recovery path still claims only the leading occurrence so child recursion has a single anchor; the new countMatchingRun helper walks ahead at the expected level (skipping deeper headings as body content) and reports the actual run length. Regression: TestPlan156_OutOfOrderRunCountsAvailableMatches. https://claude.ai/code/session_012GGH62fZUzLuzP8T4ocGkJ
1 parent 5951924 commit 56fa627

2 files changed

Lines changed: 82 additions & 16 deletions

File tree

internal/schema/plan156_acceptance_test.go

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1111,3 +1111,46 @@ func TestPlan156_AcronymScopeMatchesByHeadingText(t *testing.T) {
11111111
"acronyms.scope must match the actual matched heading text, "+
11121112
"not just the schema label or the raw regex body")
11131113
}
1114+
1115+
// TestPlan156_OutOfOrderRunCountsAvailableMatches regresses a
1116+
// Copilot review finding: when an out-of-order recovery occurs on
1117+
// a scope with `repeat.min > 1`, the diagnostic must count the
1118+
// available run of consecutive matches rather than always
1119+
// emitting "matched 1 times". Otherwise a document that contains
1120+
// enough occurrences of B (just before A) gets a spurious
1121+
// "matched 1 times, required at least 2" diagnostic on top of
1122+
// the legitimate out-of-order message.
1123+
func TestPlan156_OutOfOrderRunCountsAvailableMatches(t *testing.T) {
1124+
raw := map[string]any{
1125+
"sections": []any{
1126+
map[string]any{"heading": "A"},
1127+
map[string]any{
1128+
"heading": map[string]any{
1129+
"regex": "B",
1130+
"repeat": map[string]any{"min": 2, "max": 5},
1131+
},
1132+
},
1133+
},
1134+
}
1135+
sch, err := ParseInline(raw, "kind x")
1136+
require.NoError(t, err)
1137+
// Two B's before the expected A — B's min cardinality is
1138+
// satisfied by the run, only the ordering is wrong.
1139+
doc := newDocFile(t, "doc.md",
1140+
"# T\n\n## B\n\n## B\n\n## A\n")
1141+
diags := Validate(doc, sch, nil, false, makeDiagForTest)
1142+
var outOfOrder, falseMin int
1143+
for _, d := range diags {
1144+
if strings.Contains(d.Message, "out of order") {
1145+
outOfOrder++
1146+
}
1147+
if strings.Contains(d.Message, "matched 1 times") {
1148+
falseMin++
1149+
}
1150+
}
1151+
assert.GreaterOrEqual(t, outOfOrder, 1,
1152+
"the out-of-order condition must still be reported")
1153+
assert.Zero(t, falseMin,
1154+
"a contiguous B-run that satisfies min must not get a "+
1155+
"`matched 1 times, required at least 2` diagnostic")
1156+
}

internal/schema/validate.go

Lines changed: 39 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -728,25 +728,20 @@ func claimOutOfOrder(
728728
formatHeading(dh.Level, dh.Text),
729729
formatHeading(expectedLevel, displayHeading(sc))))}
730730
diags = append(diags, levelDiagIfNeeded(f, dh, expectedLevel, mkDiag)...)
731-
// The out-of-order claim only consumes one heading. A repeated
732-
// late scope (`repeat.min > 1`) gets a cardinality diagnostic
733-
// so the min contract is enforced even on the recovery path.
734-
// `repeat.max > 1` and `sequential: true` constraints are not
735-
// fully enforced here — the trailing-leftover loop flags
736-
// additional matching headings via claimedScopeMatches as
737-
// "exceeds scope's allowed occurrences", but the run-style
738-
// sequential check is not retraced for out-of-order claims.
739-
// Authors who need contiguous-run guarantees should put the
740-
// repeated scope at its natural position in the schema; the
741-
// in-order matchScope path is the canonical enforcement
742-
// surface for max and sequential.
743-
if ooSc.Matcher != nil && ooSc.Matcher.Repeat.Min > 1 {
731+
// Count the contiguous run of same-level headings starting at
732+
// docIdx that match ooSc. The recovery path still claims only
733+
// the leading occurrence (so child recursion has a single
734+
// anchor), but counting the full run avoids reporting a false
735+
// `repeat.min` shortfall when the document does contain enough
736+
// occurrences — they are simply in the wrong position, which
737+
// the out-of-order diagnostic already calls out.
738+
runLen := countMatchingRun(ooSc, docHeads, docIdx, expectedLevel, docFM)
739+
if ooSc.Matcher != nil && runLen < ooSc.Matcher.Repeat.Min {
744740
diags = append(diags, mkDiag(f.Path, dh.Line,
745741
fmt.Sprintf(
746-
"section %q matched 1 times, required at least %d "+
747-
"(out-of-order recovery only counts the first occurrence)",
742+
"section %q matched %d times, required at least %d",
748743
formatHeading(expectedLevel, displayHeading(ooSc)),
749-
ooSc.Matcher.Repeat.Min)))
744+
runLen, ooSc.Matcher.Repeat.Min)))
750745
}
751746
if ooSc.Matcher != nil && ooSc.Matcher.Sequential {
752747
diags = append(diags, mkDiag(f.Path, dh.Line,
@@ -769,6 +764,34 @@ func claimOutOfOrder(
769764
return docIdx, diags
770765
}
771766

767+
// countMatchingRun returns the number of contiguous same-level
768+
// headings starting at docIdx that match sc's matcher. Deeper
769+
// headings (level > expectedLevel) are skipped as body content of
770+
// an already-matched section, mirroring matchScope's run
771+
// semantics. The walk stops at the first same-level non-match or
772+
// the first shallower-than-expected heading.
773+
func countMatchingRun(
774+
sc Scope, docHeads []DocHeading, docIdx, expectedLevel int,
775+
docFM map[string]any,
776+
) int {
777+
run := 0
778+
for i := docIdx; i < len(docHeads); i++ {
779+
dh := docHeads[i]
780+
if dh.Level < expectedLevel {
781+
return run
782+
}
783+
if dh.Level > expectedLevel {
784+
continue
785+
}
786+
if matched, _ := matchHeading(sc.Matcher, dh, docFM); matched {
787+
run++
788+
continue
789+
}
790+
return run
791+
}
792+
return run
793+
}
794+
772795
// findOutOfOrderIdx returns the first unclaimed scope at index >=
773796
// minIdx that matches dh.
774797
func findOutOfOrderIdx(

0 commit comments

Comments
 (0)