Skip to content

Commit 76a790e

Browse files
committed
schema: enforce repeat.max > 1 on out-of-order recovery
The earlier round restricted the "exceeds scope's allowed occurrences" diagnostic to scopes with `max == 1` because the boolean claim state can't distinguish "second of three allowed" from a true excess. The reviewer flagged that this leaves `max > 1` out-of-order extras unenforced: a schema [A, B max=2] with doc [B, A, B, B] silently absorbs the third B in open schemas. Replace `claimedScopeMatches` with `claimedScopeExceeded`, which counts same-level matches in the parent window up to and including the current heading. The diagnostic fires only when `count > max`, so two Bs are accepted, the third B is flagged. The check still skips unbounded matchers and the in-order path keeps using flagExtrasBeyondMax for its own enforcement. Test: TestPlan156_BoundedMaxExceededOutOfOrder. https://claude.ai/code/session_012GGH62fZUzLuzP8T4ocGkJ
1 parent 702f6e7 commit 76a790e

2 files changed

Lines changed: 81 additions & 24 deletions

File tree

internal/schema/plan156_acceptance_test.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -459,6 +459,41 @@ func TestPlan156_OutOfOrderSequentialDiagFires(t *testing.T) {
459459
"the unenforced-sequential diagnostic")
460460
}
461461

462+
// TestPlan156_BoundedMaxExceededOutOfOrder regresses a Copilot
463+
// finding: the recovery path used to skip already-claimed
464+
// scopes with `max > 1`, so a `repeat: { max: 2 }` scope
465+
// claimed out of order could silently absorb three or more
466+
// occurrences in open schemas. The trailing pass now counts
467+
// same-level matches in the parent window and flags the
468+
// excess (only) when count > max.
469+
func TestPlan156_BoundedMaxExceededOutOfOrder(t *testing.T) {
470+
raw := map[string]any{
471+
"sections": []any{
472+
map[string]any{"heading": "A"},
473+
map[string]any{"heading": map[string]any{
474+
"regex": "B",
475+
"repeat": map[string]any{"max": 2},
476+
}},
477+
},
478+
}
479+
sch, err := ParseInline(raw, "kind x")
480+
require.NoError(t, err)
481+
// Doc: B (out of order), A, B, B. Three Bs total; max=2.
482+
// The third B must be flagged.
483+
doc := newDocFile(t, "doc.md",
484+
"# T\n\n## B\n\nx\n\n## A\n\ny\n\n## B\n\nz\n\n## B\n\nw\n")
485+
diags := Validate(doc, sch, nil, false, makeDiagForTest)
486+
var exceededCount int
487+
for _, d := range diags {
488+
if strings.Contains(d.Message, "exceeds scope") {
489+
exceededCount++
490+
}
491+
}
492+
assert.Equal(t, 1, exceededCount,
493+
"only the third B should be flagged as exceeding max=2 "+
494+
"(the first two are within bounds)")
495+
}
496+
462497
// TestPlan156_RejectsUserNamedCaptureN regresses a Copilot
463498
// finding: the matcher runtime reads the named capture `n` for
464499
// sequential ordering, but `regex:` is raw RE2 and a user

internal/schema/validate.go

Lines changed: 46 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -220,12 +220,12 @@ func handleLeftoverHeadings(
220220
// path absorbed. Surface it specifically as a max-exceeded
221221
// diagnostic so the user sees which scope was over-filled
222222
// rather than a generic "unexpected".
223-
if idx := claimedScopeMatches(scopes, dh, claimed, docFM); idx >= 0 {
223+
if idx := claimedScopeExceeded(
224+
scopes, dh, claimed, docFM, docHeads, docIdx, expectedLevel); idx >= 0 {
224225
sc := scopes[idx]
225226
diags = append(diags, mkDiag(f.Path, dh.Line,
226227
fmt.Sprintf(
227-
"section %q exceeds scope %q's allowed occurrences "+
228-
"(out-of-order recovery only counts one match)",
228+
"section %q exceeds scope %q's allowed occurrences",
229229
formatHeading(dh.Level, dh.Text),
230230
formatHeading(expectedLevel, displayHeading(sc)))))
231231
docIdx++
@@ -241,23 +241,21 @@ func handleLeftoverHeadings(
241241
return docIdx, diags
242242
}
243243

244-
// claimedScopeMatches returns the index of the first already-claimed
245-
// non-slot, non-preamble scope with `max == 1` that accepts dh, or
246-
// -1 when no such scope is a candidate. Used by
247-
// handleLeftoverHeadings to distinguish "max exceeded" from
248-
// generic "unexpected".
244+
// claimedScopeExceeded returns the index of the first already-
245+
// claimed non-slot, non-preamble scope whose matcher accepts dh
246+
// AND whose bounded `max` has been exceeded by the count of
247+
// same-level matching headings up to and including dhIdx — or
248+
// -1 when no such scope is over-filled.
249249
//
250-
// Only `max == 1` scopes are flagged because the recovery state
251-
// is a boolean claim and doesn't track how many occurrences were
252-
// already counted. For `max > 1` we can't tell whether the new
253-
// heading is the second of three allowed or a true excess
254-
// without re-running the run-matcher — false positives there
255-
// would be worse than the loss of specificity, so those cases
256-
// fall through to the generic unexpected handling. Unbounded
257-
// matchers (`max == 0`) are likewise excluded — they accept any
258-
// number of occurrences.
259-
func claimedScopeMatches(
260-
scopes []Scope, dh DocHeading, claimed map[int]bool, docFM map[string]any,
250+
// Counting matches in the parent window (rather than tracking a
251+
// per-scope counter through the validator) lets the late/out-of-
252+
// order recovery path enforce `repeat.max > 1` without
253+
// false-positives at count <= max. Unbounded matchers
254+
// (`max == 0`) are excluded — they accept any number of
255+
// occurrences.
256+
func claimedScopeExceeded(
257+
scopes []Scope, dh DocHeading, claimed map[int]bool,
258+
docFM map[string]any, docHeads []DocHeading, dhIdx, expectedLevel int,
261259
) int {
262260
for i, sc := range scopes {
263261
if !claimed[i] {
@@ -270,16 +268,40 @@ func claimedScopeMatches(
270268
continue
271269
}
272270
_, max := sc.Matcher.Repeat.Bounds()
273-
if max != 1 {
271+
if max == 0 {
274272
continue
275273
}
276-
if scopeMatchesHeading(sc, dh, docFM) {
274+
if !scopeMatchesHeading(sc, dh, docFM) {
275+
continue
276+
}
277+
count := countSameLevelMatches(sc, docHeads, expectedLevel, dhIdx, docFM)
278+
if count > max {
277279
return i
278280
}
279281
}
280282
return -1
281283
}
282284

285+
// countSameLevelMatches returns the number of headings in
286+
// docHeads[0..upToIdx] (inclusive) at expectedLevel that sc's
287+
// matcher accepts. Used to compute the occurrence count for the
288+
// max-exceeded check in the trailing-leftover pass.
289+
func countSameLevelMatches(
290+
sc Scope, docHeads []DocHeading, expectedLevel, upToIdx int,
291+
docFM map[string]any,
292+
) int {
293+
count := 0
294+
for j := 0; j <= upToIdx && j < len(docHeads); j++ {
295+
if docHeads[j].Level != expectedLevel {
296+
continue
297+
}
298+
if scopeMatchesHeading(sc, docHeads[j], docFM) {
299+
count++
300+
}
301+
}
302+
return count
303+
}
304+
283305
// claimLateScope marks a late-arriving listed scope as claimed,
284306
// emits its out-of-order diagnostic, and recurses into the scope's
285307
// nested children so missing-required-section diagnostics still
@@ -567,12 +589,12 @@ func (s *matchRun) handleNonMatch(docHeads []DocHeading, docIdx int) (bool, int,
567589
// was over-filled. Without this, a sequence like [A, B] with
568590
// doc [B, B, A] silently consumes the second B because
569591
// findOutOfOrderIdx ignores claimed scopes.
570-
if idx := claimedScopeMatches(s.scopes, dh, s.claimed, s.docFM); idx >= 0 {
592+
if idx := claimedScopeExceeded(
593+
s.scopes, dh, s.claimed, s.docFM, docHeads, docIdx, s.expectedLevel); idx >= 0 {
571594
sc := s.scopes[idx]
572595
s.diags = append(s.diags, s.mkDiag(s.f.Path, dh.Line,
573596
fmt.Sprintf(
574-
"section %q exceeds scope %q's allowed occurrences "+
575-
"(out-of-order recovery only counts one match)",
597+
"section %q exceeds scope %q's allowed occurrences",
576598
formatHeading(dh.Level, dh.Text),
577599
formatHeading(s.expectedLevel, displayHeading(sc)))))
578600
return false, docIdx + 1, false

0 commit comments

Comments
 (0)