Skip to content

Commit 5951924

Browse files
committed
schema: address tenth Copilot review round (plan 156)
- validate.go: claimedScopeExceeded now consumes a parallel claimCounts map seeded by the matcher itself, eliminating the heading-rescan that double-counted yielded broad-matcher acceptances. - acronyms.go: walkRanges callback surfaces the matched doc heading text. acronymRanges adds the heading text to the scope-name allowlist check so disjunctive regexes like `Symptoms|Indicators` align with intuitive scope config such as `scope: ["Indicators"]`; plan 156 dropped the scope-level aliases field, so the heading match keeps existing users from having to mirror the regex body verbatim. - plan156_acceptance_test.go: regression for the heading-text scope match (TestPlan156_AcronymScopeMatchesByHeadingText). https://claude.ai/code/session_012GGH62fZUzLuzP8T4ocGkJ
1 parent 4ac6fee commit 5951924

3 files changed

Lines changed: 128 additions & 56 deletions

File tree

internal/schema/acronyms.go

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -97,13 +97,21 @@ func acronymRanges(f *lint.File, sch *Schema, scope []string, docFM map[string]a
9797

9898
var out []lineRange
9999
walkRanges(sch.Sections, body, rootLevel, 1, len(f.Lines)+1, docFM,
100-
func(sc Scope, start, end int) {
100+
func(sc Scope, headingText string, start, end int) {
101101
// walkRanges already skips preamble and slot scopes, so
102102
// any sc reaching here has a literal Matcher. Match the
103-
// scope label or the matcher's regex against the
104-
// configured scope-name allowlist; plan 156 removed the
105-
// scope-level `aliases:` field, so disjunctive scope
106-
// names live in the regex body (`A|B`).
103+
// scope-name allowlist against (a) the actual heading
104+
// text in the document, (b) the schema label, and (c)
105+
// the raw regex body. The document-heading match keeps
106+
// disjunctive regexes like `Symptoms|Indicators` aligned
107+
// with intuitive `acronyms.scope: ["Indicators"]` config;
108+
// plan 156 removed the scope-level `aliases:` field, so
109+
// without (a) users would have to mirror the regex body
110+
// verbatim.
111+
if matchSet[headingText] {
112+
out = append(out, lineRange{Start: start, End: end})
113+
return
114+
}
107115
if matchSet[sc.Heading] {
108116
out = append(out, lineRange{Start: start, End: end})
109117
return
@@ -127,7 +135,7 @@ func walkRanges(
127135
scopes []Scope, heads []DocHeading,
128136
expectedLevel, parentStart, parentEnd int,
129137
docFM map[string]any,
130-
visit func(sc Scope, start, end int),
138+
visit func(sc Scope, headingText string, start, end int),
131139
) {
132140
claimed := make(map[int]bool, len(heads))
133141
for i, sc := range scopes {
@@ -142,7 +150,7 @@ func walkRanges(
142150
claimed[idx] = true
143151
start := heads[idx].Line
144152
end := nextSectionLine(heads, idx, heads[idx].Level, parentEnd)
145-
visit(sc, start, end)
153+
visit(sc, heads[idx].Text, start, end)
146154
if len(sc.Sections) > 0 {
147155
walkRanges(sc.Sections, heads, expectedLevel+1, start, end, docFM, visit)
148156
}

internal/schema/plan156_acceptance_test.go

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

462+
// TestPlan156_OverlappingMatcherDoesNotInflateClaimCount
463+
// regresses a Copilot finding: an earlier `regex: 'A|B'` scope
464+
// that yields a `B` heading to a later named `B` scope used to
465+
// have that yielded heading counted toward its own claim total
466+
// because the trailing pass re-scanned by regex. Now the count
467+
// tracks actual claims, so the early scope's `max` isn't
468+
// over-counted.
469+
func TestPlan156_OverlappingMatcherDoesNotInflateClaimCount(t *testing.T) {
470+
raw := map[string]any{
471+
"sections": []any{
472+
map[string]any{
473+
"heading": map[string]any{
474+
"regex": "A|B",
475+
"repeat": map[string]any{"min": 1, "max": 2},
476+
},
477+
},
478+
map[string]any{"heading": "B"},
479+
},
480+
}
481+
sch, err := ParseInline(raw, "kind x")
482+
require.NoError(t, err)
483+
// Doc: A, B, A. The first scope claims A (counted = 1),
484+
// yields B to the named B scope, then sees the second A.
485+
// The trailing A should be its 2nd claim — within max=2.
486+
doc := newDocFile(t, "doc.md",
487+
"# T\n\n## A\n\nx\n\n## B\n\ny\n\n## A\n\nz\n")
488+
diags := Validate(doc, sch, nil, false, makeDiagForTest)
489+
for _, d := range diags {
490+
assert.NotContains(t, d.Message, "exceeds scope",
491+
"yielded headings must not inflate the first scope's claim count")
492+
}
493+
}
494+
462495
// TestPlan156_BoundedMaxExceededOutOfOrder regresses a Copilot
463496
// finding: the recovery path used to skip already-claimed
464497
// scopes with `max > 1`, so a `repeat: { max: 2 }` scope
@@ -1042,3 +1075,39 @@ func TestPlan156_RulesOnRepeatedScope(t *testing.T) {
10421075
"each repeated occurrence must contribute its own range "+
10431076
"so per-scope checks fire on every occurrence")
10441077
}
1078+
1079+
// TestPlan156_AcronymScopeMatchesByHeadingText regresses a
1080+
// Copilot review finding: a disjunctive matcher like
1081+
// `regex: 'Symptoms|Indicators'` paired with
1082+
// `acronyms.scope: ["Indicators"]` must scan the matched
1083+
// `## Indicators` body. Plan 156 dropped the scope-level
1084+
// `aliases:` field, so the walker must compare the configured
1085+
// scope name against the actual matched heading text (not just
1086+
// the schema label or the raw regex body).
1087+
func TestPlan156_AcronymScopeMatchesByHeadingText(t *testing.T) {
1088+
raw := map[string]any{
1089+
"sections": []any{
1090+
map[string]any{
1091+
"heading": map[string]any{"regex": "Symptoms|Indicators"},
1092+
},
1093+
},
1094+
"acronyms": map[string]any{
1095+
"scope": []any{"Indicators"},
1096+
"known-safe": []any{},
1097+
},
1098+
}
1099+
sch, err := ParseInline(raw, "kind x")
1100+
require.NoError(t, err)
1101+
doc := newDocFile(t, "doc.md",
1102+
"# T\n\n## Indicators\n\nOIDC first.\n")
1103+
diags := ValidateAcronyms(doc, sch, nil, makeDiagForTest)
1104+
var seenOIDC bool
1105+
for _, d := range diags {
1106+
if strings.Contains(d.Message, "OIDC") {
1107+
seenOIDC = true
1108+
}
1109+
}
1110+
assert.True(t, seenOIDC,
1111+
"acronyms.scope must match the actual matched heading text, "+
1112+
"not just the schema label or the raw regex body")
1113+
}

internal/schema/validate.go

Lines changed: 44 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,7 @@ func validateScopes(
146146
) (int, []lint.Diagnostic) {
147147
var diags []lint.Diagnostic
148148
claimed := make(map[int]bool)
149+
claimCounts := make(map[int]int)
149150
allowExtra := false
150151

151152
for i, sc := range scopes {
@@ -166,7 +167,7 @@ func validateScopes(
166167
}
167168
newIdx, scDiags, claimedThis := matchScope(
168169
f, scopes, i, expectedLevel, docHeads, docIdx,
169-
claimed, allowExtra, closed, docFM, mkDiag)
170+
claimed, claimCounts, allowExtra, closed, docFM, mkDiag)
170171
diags = append(diags, scDiags...)
171172
docIdx = newIdx
172173
if claimedThis {
@@ -179,7 +180,7 @@ func validateScopes(
179180
}
180181

181182
newIdx, leftoverDiags := handleLeftoverHeadings(
182-
f, scopes, claimed, docHeads, docIdx, expectedLevel,
183+
f, scopes, claimed, claimCounts, docHeads, docIdx, expectedLevel,
183184
closed, allowExtra, docFM, mkDiag)
184185
diags = append(diags, leftoverDiags...)
185186
return newIdx, diags
@@ -194,7 +195,7 @@ func validateScopes(
194195
// closed: flagged as unexpected in closed scopes, silently
195196
// consumed in open ones.
196197
func handleLeftoverHeadings(
197-
f *lint.File, scopes []Scope, claimed map[int]bool,
198+
f *lint.File, scopes []Scope, claimed map[int]bool, claimCounts map[int]int,
198199
docHeads []DocHeading, docIdx, expectedLevel int,
199200
closed, allowExtra bool, docFM map[string]any, mkDiag MakeDiag,
200201
) (int, []lint.Diagnostic) {
@@ -210,7 +211,8 @@ func handleLeftoverHeadings(
210211
}
211212
if idx := unclaimedListedScope(scopes, dh, claimed, docFM); idx >= 0 {
212213
newIdx, claimDiags := claimLateScope(
213-
f, scopes, idx, expectedLevel, docHeads, docIdx, claimed, docFM, mkDiag)
214+
f, scopes, idx, expectedLevel, docHeads, docIdx,
215+
claimed, claimCounts, docFM, mkDiag)
214216
diags = append(diags, claimDiags...)
215217
docIdx = newIdx
216218
continue
@@ -221,7 +223,7 @@ func handleLeftoverHeadings(
221223
// diagnostic so the user sees which scope was over-filled
222224
// rather than a generic "unexpected".
223225
if idx := claimedScopeExceeded(
224-
scopes, dh, claimed, docFM, docHeads, docIdx, expectedLevel); idx >= 0 {
226+
scopes, dh, claimed, claimCounts, docFM); idx >= 0 {
225227
sc := scopes[idx]
226228
diags = append(diags, mkDiag(f.Path, dh.Line,
227229
fmt.Sprintf(
@@ -241,21 +243,23 @@ func handleLeftoverHeadings(
241243
return docIdx, diags
242244
}
243245

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.
246+
// claimedScopeExceeded reports whether dh would push an
247+
// already-claimed non-slot, non-preamble scope past its `max`.
248+
// It also increments `claimCounts[i]` for the first claimed
249+
// scope whose matcher accepts dh — so consecutive same-scope
250+
// extras are counted accurately. Returns the matched scope's
251+
// index when the new count exceeds max, or -1 otherwise.
249252
//
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.
253+
// Claim counts are tracked via the parallel `claimCounts` map,
254+
// so the check stays accurate even when overlapping matchers
255+
// yield specific headings to later named scopes: only headings
256+
// actually claimed by THIS scope count toward its max.
257+
//
258+
// Unbounded matchers (`max == 0`) are excluded — they accept
259+
// any number of occurrences.
256260
func claimedScopeExceeded(
257261
scopes []Scope, dh DocHeading, claimed map[int]bool,
258-
docFM map[string]any, docHeads []DocHeading, dhIdx, expectedLevel int,
262+
claimCounts map[int]int, docFM map[string]any,
259263
) int {
260264
for i, sc := range scopes {
261265
if !claimed[i] {
@@ -267,41 +271,24 @@ func claimedScopeExceeded(
267271
if sc.Matcher == nil {
268272
continue
269273
}
270-
_, max := sc.Matcher.Repeat.Bounds()
271-
if max == 0 {
272-
continue
273-
}
274274
if !scopeMatchesHeading(sc, dh, docFM) {
275275
continue
276276
}
277-
count := countSameLevelMatches(sc, docHeads, expectedLevel, dhIdx, docFM)
278-
if count > max {
277+
_, max := sc.Matcher.Repeat.Bounds()
278+
if max == 0 {
279+
// Unbounded — count it but never flag.
280+
claimCounts[i]++
281+
return -1
282+
}
283+
claimCounts[i]++
284+
if claimCounts[i] > max {
279285
return i
280286
}
287+
return -1
281288
}
282289
return -1
283290
}
284291

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-
305292
// claimLateScope marks a late-arriving listed scope as claimed,
306293
// emits its out-of-order diagnostic, and recurses into the scope's
307294
// nested children so missing-required-section diagnostics still
@@ -320,7 +307,8 @@ func countSameLevelMatches(
320307
// surface.
321308
func claimLateScope(
322309
f *lint.File, scopes []Scope, idx, expectedLevel int,
323-
docHeads []DocHeading, docIdx int, claimed map[int]bool,
310+
docHeads []DocHeading, docIdx int,
311+
claimed map[int]bool, claimCounts map[int]int,
324312
docFM map[string]any, mkDiag MakeDiag,
325313
) (int, []lint.Diagnostic) {
326314
sc := scopes[idx]
@@ -330,6 +318,7 @@ func claimLateScope(
330318
"section %q out of order: expected before this position",
331319
formatHeading(dh.Level, dh.Text)))}
332320
claimed[idx] = true
321+
claimCounts[idx]++
333322
docIdx++
334323
if len(sc.Sections) > 0 {
335324
newIdx, childDiags := validateScopes(
@@ -368,12 +357,14 @@ func unclaimedListedScope(
368357
func matchScope(
369358
f *lint.File, scopes []Scope, idx, expectedLevel int,
370359
docHeads []DocHeading, docIdx int,
371-
claimed map[int]bool, allowExtra, closed bool,
360+
claimed map[int]bool, claimCounts map[int]int,
361+
allowExtra, closed bool,
372362
docFM map[string]any, mkDiag MakeDiag,
373363
) (int, []lint.Diagnostic, bool) {
374364
state := matchRun{
375365
f: f, scopes: scopes, idx: idx, expectedLevel: expectedLevel,
376-
claimed: claimed, allowExtra: allowExtra, closed: closed,
366+
claimed: claimed, claimCounts: claimCounts,
367+
allowExtra: allowExtra, closed: closed,
377368
docFM: docFM, mkDiag: mkDiag,
378369
}
379370
state.min, state.max = scopes[idx].Matcher.Repeat.Bounds()
@@ -471,6 +462,7 @@ type matchRun struct {
471462
idx int
472463
expectedLevel int
473464
claimed map[int]bool
465+
claimCounts map[int]int
474466
allowExtra bool
475467
closed bool
476468
docFM map[string]any
@@ -554,6 +546,7 @@ func (s *matchRun) claimMatch(docHeads []DocHeading, docIdx int, captured string
554546
dh := docHeads[docIdx]
555547
s.diags = append(s.diags, levelDiagIfNeeded(s.f, dh, s.expectedLevel, s.mkDiag)...)
556548
s.claimed[s.idx] = true
549+
s.claimCounts[s.idx]++
557550
if len(sc.Sections) > 0 {
558551
newIdx, childDiags := validateScopes(
559552
s.f, sc.Sections, sc.Closed, docHeads, docIdx+1,
@@ -579,7 +572,7 @@ func (s *matchRun) handleNonMatch(docHeads []DocHeading, docIdx int) (bool, int,
579572
if ooIdx := findOutOfOrderIdx(s.scopes, dh, s.claimed, s.idx+1, s.docFM); ooIdx >= 0 {
580573
newIdx, ooDiags := claimOutOfOrder(
581574
s.f, s.scopes, s.idx, ooIdx, s.expectedLevel, docHeads, docIdx,
582-
s.claimed, s.docFM, s.mkDiag)
575+
s.claimed, s.claimCounts, s.docFM, s.mkDiag)
583576
s.diags = append(s.diags, ooDiags...)
584577
return false, newIdx, false
585578
}
@@ -590,7 +583,7 @@ func (s *matchRun) handleNonMatch(docHeads []DocHeading, docIdx int) (bool, int,
590583
// doc [B, B, A] silently consumes the second B because
591584
// findOutOfOrderIdx ignores claimed scopes.
592585
if idx := claimedScopeExceeded(
593-
s.scopes, dh, s.claimed, s.docFM, docHeads, docIdx, s.expectedLevel); idx >= 0 {
586+
s.scopes, dh, s.claimed, s.claimCounts, s.docFM); idx >= 0 {
594587
sc := s.scopes[idx]
595588
s.diags = append(s.diags, s.mkDiag(s.f.Path, dh.Line,
596589
fmt.Sprintf(
@@ -723,7 +716,8 @@ func levelDiagIfNeeded(
723716
// recurses into the matched scope's child sections.
724717
func claimOutOfOrder(
725718
f *lint.File, scopes []Scope, idx, ooIdx, expectedLevel int,
726-
docHeads []DocHeading, docIdx int, claimed map[int]bool,
719+
docHeads []DocHeading, docIdx int,
720+
claimed map[int]bool, claimCounts map[int]int,
727721
docFM map[string]any, mkDiag MakeDiag,
728722
) (int, []lint.Diagnostic) {
729723
sc := scopes[idx]
@@ -763,6 +757,7 @@ func claimOutOfOrder(
763757
formatHeading(expectedLevel, displayHeading(ooSc)))))
764758
}
765759
claimed[ooIdx] = true
760+
claimCounts[ooIdx]++
766761
docIdx++
767762
if len(ooSc.Sections) > 0 {
768763
newIdx, childDiags := validateScopes(

0 commit comments

Comments
 (0)