schema: address third Copilot review round (plan 156)

Five new review findings:

1. claimsLaterLiteral was too permissive after the previous fix:
   an optional specific scope followed by a broad `.+` matcher
   yielded its heading to the broad scope, skipping its own
   content / rules. Add `isBroadMatcher` and skip broad-regex
   later scopes in the yield check. Test:
   TestPlan156_OptionalSpecificClaimsOwnSlot.

2-4. The per-scope walkers (rules, content, acronyms) were
   greedy: a broad repeated matcher claimed every matching
   heading before later named scopes got a chance, so the named
   scopes' content / rules / acronyms-scope checks silently
   skipped those headings. Add a walker-side yield helper
   (`laterScopeMatches` / `laterScopeClaimsHead`) and wire it
   into walkContentScopes, walkRanges, and the per-scope rule
   walker. Test:
   TestPlan156_BroadRepeatYieldsToLaterScopeInPerScopeWalkers.

5. The matcher cache was a process-global sync.Map that could
   grow unboundedly in a long-running LSP session — each unique
   `fmvar(...)` value adds an entry. Replace with a bounded
   map (cap 1024); reset the map when it fills. Mutex-guarded so
   parallel validator passes stay race-free. Test:
   TestMatcherCache_BoundsGrowth.

https://claude.ai/code/session_012GGH62fZUzLuzP8T4ocGkJ

Author: claude

internal/rules/requiredstructure/scope_rules.go

@@ -73,7 +73,7 @@ func walkScopes(
 	claimed map[int]bool, docFM map[string]any,
 	visit func(sc schema.Scope, startLine, endLine int),
 ) {
-	for _, sc := range scopes {
+	for i, sc := range scopes {
 		if sc.Preamble {
 			// Preamble range: [parentStart, first heading at this
 			// level in the window). Empty if the very first doc
@@ -97,8 +97,15 @@ func walkScopes(
 			if matched < 0 {
 				break
 			}
-			claimed[matched] = true
 			dh := heads[matched]
+			// Yield broad matchers to later named scopes so a
+			// `regex: '.+'` repeat does not absorb a heading the
+			// user named separately.
+			if isBroadScopeMatcher(sc) &&
+				laterScopeClaimsHead(scopes, i+1, dh, docFM) {
+				break
+			}
+			claimed[matched] = true
 			// The section's end boundary follows the doc heading's
 			// real level, not the schema's expectedLevel. When the
 			// two differ (level-mismatch fallback), basing the end
@@ -217,6 +224,35 @@ func isSlotScope(sc schema.Scope) bool {
 	return true
 }
 
+// isBroadScopeMatcher reports whether sc's matcher is the `.+`
+// catch-all, regardless of repeat bounds. Slot matchers are a
+// subset. The yield helper uses this to keep a broad repeated
+// matcher from absorbing a heading that a later named scope
+// would otherwise claim.
+func isBroadScopeMatcher(sc schema.Scope) bool {
+	return sc.Matcher != nil && sc.Matcher.Regex == ".+"
+}
+
+// laterScopeClaimsHead reports whether any scope at index >=
+// startIdx (in the same parent window) matches dh AND is more
+// specific than a broad matcher. Mirrors schema.laterScopeMatches
+// but stays local to this package so the import surface is small.
+func laterScopeClaimsHead(
+	scopes []schema.Scope, startIdx int, dh schema.DocHeading,
+	docFM map[string]any,
+) bool {
+	for i := startIdx; i < len(scopes); i++ {
+		sc := scopes[i]
+		if sc.Preamble || isSlotScope(sc) || isBroadScopeMatcher(sc) {
+			continue
+		}
+		if schema.MatchesHeading(sc, dh, docFM) {
+			return true
+		}
+	}
+	return false
+}
+
 // runScopeRules executes each rule named in sc.Rules and returns
 // diagnostics that fall within the scope's line range. Each rule is
 // cloned with its DefaultSettings and then has the scope's override

internal/schema/acronyms.go

@@ -132,7 +132,7 @@ func walkRanges(
 	visit func(sc Scope, start, end int),
 ) {
 	claimed := make(map[int]bool, len(heads))
-	for _, sc := range scopes {
+	for i, sc := range scopes {
 		if sc.Preamble || isSlotMatcher(sc.Matcher) {
 			continue
 		}
@@ -142,6 +142,12 @@ func walkRanges(
 			if idx < 0 {
 				break
 			}
+			// Yield broad matchers to later named scopes so a `.+`
+			// repeat does not consume a heading the user named.
+			if isBroadMatcher(sc.Matcher) &&
+				laterScopeMatches(scopes, i+1, heads[idx], docFM) {
+				break
+			}
 			claimed[idx] = true
 			start := heads[idx].Line
 			end := nextSectionLine(heads, idx, heads[idx].Level, parentEnd)

internal/schema/matcher.go

@@ -210,11 +210,25 @@ func compileMatcher(m *Matcher, fm map[string]any) (*compiledMatcher, error) {
 	return &compiledMatcher{re: re, digitsIdx: digitsIdx}, nil
 }
 
+// matcherCacheCap bounds the per-process compiled-matcher cache.
+// A long-running LSP session edits front matter often, and each
+// distinct `fmvar(...)` value produces a fresh cache key — without
+// a bound the cache grows unboundedly. 1024 entries covers a busy
+// workspace while keeping the working-set predictable; once full,
+// the cache resets so old entries do not pin compiled regexps for
+// the lifetime of the process.
+const matcherCacheCap = 1024
+
 // matcherCache memoises compiled matchers keyed on the raw pattern
 // plus a serialised frontmatter fingerprint. Hot loops (one
 // validator pass walks every heading × every scope) re-use the
-// compiled regexp instead of recompiling per heading.
-var matcherCache sync.Map
+// compiled regexp instead of recompiling per heading. The cache
+// is sized-bounded by matcherCacheCap; see matcherCacheReset.
+var (
+	matcherCacheMu  sync.Mutex
+	matcherCache    map[matcherCacheKey]*compiledMatcher = make(map[matcherCacheKey]*compiledMatcher, matcherCacheCap)
+	matcherCacheLen int
+)
 
 type matcherCacheKey struct {
 	regex string
@@ -224,19 +238,36 @@ type matcherCacheKey struct {
 // cachedMatcher returns the compiled matcher for m under fm,
 // compiling on cache miss. nil + err on a malformed pattern; the
 // caller decides whether to surface that as a diagnostic.
+//
+// When the cache reaches matcherCacheCap, the entire map is
+// dropped before inserting the new entry. A simple reset beats
+// per-entry LRU bookkeeping for workloads where hot patterns are
+// re-encountered on the next validator pass anyway.
 func cachedMatcher(m *Matcher, fm map[string]any) (*compiledMatcher, error) {
 	if m == nil {
 		return nil, fmt.Errorf("nil matcher")
 	}
 	key := matcherCacheKey{regex: m.Regex, fmKey: fmFingerprint(m.Regex, fm)}
-	if v, ok := matcherCache.Load(key); ok {
-		return v.(*compiledMatcher), nil
+	matcherCacheMu.Lock()
+	if v, ok := matcherCache[key]; ok {
+		matcherCacheMu.Unlock()
+		return v, nil
 	}
+	matcherCacheMu.Unlock()
 	cm, err := compileMatcher(m, fm)
 	if err != nil {
 		return nil, err
 	}
-	matcherCache.Store(key, cm)
+	matcherCacheMu.Lock()
+	if matcherCacheLen >= matcherCacheCap {
+		matcherCache = make(map[matcherCacheKey]*compiledMatcher, matcherCacheCap)
+		matcherCacheLen = 0
+	}
+	if _, exists := matcherCache[key]; !exists {
+		matcherCacheLen++
+	}
+	matcherCache[key] = cm
+	matcherCacheMu.Unlock()
 	return cm, nil
 }
 

internal/schema/parse_inline.go

@@ -326,6 +326,15 @@ func isSlotMatcher(m *Matcher) bool {
 	return true
 }
 
+// isBroadMatcher reports whether m's regex matches "anything" —
+// the `.+` body, regardless of repeat bounds. Used by the
+// yield-to-later helpers so a broad scope never claims a heading
+// that a more-specific later scope would have matched. Slot
+// matchers are a subset of broad matchers.
+func isBroadMatcher(m *Matcher) bool {
+	return m != nil && m.Regex == ".+"
+}
+
 // rejectKeys errors if any forbidden key is present in m. The
 // shape label and key list go into the error so the user sees
 // which field is incompatible and why. Forbidden keys are checked

internal/schema/plan156_acceptance_test.go

@@ -340,6 +340,96 @@ func TestPlan156_RejectsMultipleDigitsInline(t *testing.T) {
 	assert.Contains(t, err.Error(), "once")
 }
 
+// TestPlan156_OptionalSpecificClaimsOwnSlot regresses a Copilot
+// finding: a heading that matches an earlier optional specific
+// matcher must claim that scope, not yield to a later broad
+// (`.+`) matcher. The previous fix made claimsLaterLiteral
+// include optional scopes — necessary for the wildcard-slot
+// case — but the yield should only fire when the later
+// matcher is *narrower* than the current one (i.e. not a
+// broad `.+` regex).
+func TestPlan156_OptionalSpecificClaimsOwnSlot(t *testing.T) {
+	raw := map[string]any{
+		"sections": []any{
+			map[string]any{
+				"heading": map[string]any{
+					"regex":  "Overview",
+					"repeat": map[string]any{"min": 0, "max": 1},
+				},
+				"content": []any{
+					map[string]any{"kind": "code-block", "lang": "yaml"},
+				},
+			},
+			map[string]any{
+				"heading": map[string]any{
+					"regex":  ".+",
+					"repeat": map[string]any{"min": 1},
+				},
+			},
+		},
+	}
+	sch, err := ParseInline(raw, "kind x")
+	require.NoError(t, err)
+	// Overview is present but missing its required code block.
+	// If the broad `.+` matcher claims the heading first, the
+	// content check is silenced. We want the missing-content
+	// diagnostic to fire.
+	doc := newDocFile(t, "doc.md",
+		"# T\n\n## Overview\n\nNo code block here.\n\n## Body\n\nx\n")
+	diags := Validate(doc, sch, nil, false, makeDiagForTest)
+	var contentMissing bool
+	for _, d := range diags {
+		if strings.Contains(d.Message, "missing required content") &&
+			strings.Contains(d.Message, "Overview") {
+			contentMissing = true
+		}
+	}
+	assert.True(t, contentMissing,
+		"the optional specific scope must claim its own heading; "+
+			"a broad `.+` matcher must not absorb it")
+}
+
+// TestPlan156_BroadRepeatYieldsToLaterScopeInPerScopeWalkers
+// regresses a Copilot finding: the per-scope walkers (rules,
+// content, acronyms) must yield to later named scopes the same
+// way the structural validator does, otherwise a broad repeated
+// matcher silently consumes headings the user named separately.
+func TestPlan156_BroadRepeatYieldsToLaterScopeInPerScopeWalkers(t *testing.T) {
+	raw := map[string]any{
+		"sections": []any{
+			map[string]any{
+				"heading": map[string]any{
+					"regex":  ".+",
+					"repeat": map[string]any{"min": 1},
+				},
+			},
+			map[string]any{
+				"heading": "Diagnosis",
+				"content": []any{
+					map[string]any{"kind": "code-block", "lang": "yaml"},
+				},
+			},
+		},
+	}
+	sch, err := ParseInline(raw, "kind x")
+	require.NoError(t, err)
+	// Doc has Body + Diagnosis. The broad matcher must yield
+	// "Diagnosis" so the content check fires there.
+	doc := newDocFile(t, "doc.md",
+		"# T\n\n## Body\n\nx\n\n## Diagnosis\n\nNo code block.\n")
+	diags := Validate(doc, sch, nil, false, makeDiagForTest)
+	var missing bool
+	for _, d := range diags {
+		if strings.Contains(d.Message, "missing required content") &&
+			strings.Contains(d.Message, "Diagnosis") {
+			missing = true
+		}
+	}
+	assert.True(t, missing,
+		"named Diagnosis scope must claim its heading; "+
+			"a broad repeated matcher must not absorb it")
+}
+
 // TestPlan156_RejectsMultipleNTokensProto regresses the same
 // constraint for proto.md heading rows. `## Step {n} of {n}`
 // would expand to two `\#(digits)` helpers; the file parser

internal/schema/plan156_coverage_test.go

@@ -1,6 +1,7 @@
 package schema
 
 import (
+	"strconv"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -283,6 +284,46 @@ func TestScopeMatchesHeading_NilScope(t *testing.T) {
 	assert.False(t, scopeMatchesHeading(Scope{}, DocHeading{Text: "x"}, nil))
 }
 
+// TestMatcherCache_BoundsGrowth regresses a Copilot review
+// concern: the global matcher cache must not grow without bound
+// when an LSP session emits a stream of unique `fmvar(...)`
+// values. After matcherCacheCap inserts the cache resets so
+// memory stays predictable.
+func TestMatcherCache_BoundsGrowth(t *testing.T) {
+	matcherCacheMu.Lock()
+	matcherCache = make(map[matcherCacheKey]*compiledMatcher, matcherCacheCap)
+	matcherCacheLen = 0
+	matcherCacheMu.Unlock()
+
+	m := &Matcher{Regex: `\#(fmvar(id))`}
+	// Fill past the cap with distinct fmvar values so each entry
+	// is a fresh cache key.
+	for i := 0; i < matcherCacheCap+5; i++ {
+		_, err := cachedMatcher(m, map[string]any{
+			"id": "id-" + strconv.Itoa(i),
+		})
+		require.NoError(t, err)
+	}
+	matcherCacheMu.Lock()
+	size := matcherCacheLen
+	matcherCacheMu.Unlock()
+	assert.LessOrEqual(t, size, matcherCacheCap,
+		"matcher cache must stay within the configured cap")
+}
+
+// TestIsBroadMatcher covers the helper's three branches.
+func TestIsBroadMatcher(t *testing.T) {
+	assert.False(t, isBroadMatcher(nil))
+	assert.False(t, isBroadMatcher(&Matcher{Regex: "Specific"}))
+	assert.True(t, isBroadMatcher(&Matcher{Regex: ".+"}))
+	// A bounded-max `.+` is still broad; the helper is
+	// intentionally repeat-agnostic.
+	assert.True(t, isBroadMatcher(&Matcher{
+		Regex:  ".+",
+		Repeat: Repeat{Set: true, Min: 1, Max: 2},
+	}))
+}
+
 // TestScope_Required covers the three branches of Scope.Required.
 func TestScope_Required(t *testing.T) {
 	// Preamble is never required.

internal/schema/validate.go

@@ -494,20 +494,46 @@ func claimRun(
 	return docIdx, diags
 }
 
+// laterScopeMatches reports whether dh matches any later scope in
+// the same parent window that is more specific than a broad
+// matcher. Used by the per-scope walkers (rules, content,
+// acronyms) so a broad repeated matcher does not consume a
+// heading that a later named scope would claim. The walker
+// version does not need a scope-index `claimed` map because each
+// walker iterates scopes in declared order and acts immediately.
+func laterScopeMatches(
+	scopes []Scope, startIdx int, dh DocHeading,
+	docFM map[string]any,
+) bool {
+	for i := startIdx; i < len(scopes); i++ {
+		sc := scopes[i]
+		if sc.Preamble ||
+			isSlotMatcher(sc.Matcher) || isBroadMatcher(sc.Matcher) {
+			continue
+		}
+		if scopeMatchesHeading(sc, dh, docFM) {
+			return true
+		}
+	}
+	return false
+}
+
 // claimsLaterLiteral reports whether dh matches any unclaimed
-// non-slot, non-preamble scope at index >= startIdx — meaning the
-// heading should not be absorbed by a slot or a repeat-run at the
-// current position. Optional scopes participate: a `regex: 'A'`
-// scope with `repeat: { min: 0, max: 1 }` still wants the heading
-// to claim it (not the greedy predecessor) so its own content,
-// rules, and placement apply when present.
+// later scope that is more specific than a broad/slot matcher.
+// Used so a slot or repeat-run does not absorb a heading the
+// user named separately further down the list. Optional scopes
+// participate (a `regex: 'A'` with `repeat: { min: 0, max: 1 }`
+// still wants the heading), but later scopes with a broad `.+`
+// regex are skipped — yielding to them would let a generic
+// catch-all steal a heading from a specific predecessor.
 func claimsLaterLiteral(
 	scopes []Scope, startIdx int, dh DocHeading,
 	claimed map[int]bool, docFM map[string]any,
 ) bool {
 	for i := startIdx; i < len(scopes); i++ {
 		sc := scopes[i]
-		if claimed[i] || sc.Preamble || isSlotMatcher(sc.Matcher) {
+		if claimed[i] || sc.Preamble ||
+			isSlotMatcher(sc.Matcher) || isBroadMatcher(sc.Matcher) {
 			continue
 		}
 		if scopeMatchesHeading(sc, dh, docFM) {

internal/schema/validate_content.go

@@ -223,7 +223,7 @@ func walkContentScopes(
 	claimed map[int]bool, blocks []contentBlock,
 	docFM map[string]any, mkDiag MakeDiag, diags *[]lint.Diagnostic,
 ) {
-	for _, sc := range scopes {
+	for i, sc := range scopes {
 		if isSlotMatcher(sc.Matcher) {
 			continue
 		}
@@ -248,8 +248,14 @@ func walkContentScopes(
 			if matched < 0 {
 				break
 			}
-			claimed[matched] = true
 			dh := heads[matched]
+			// Yield broad matchers to later named scopes so a `.+`
+			// repeat does not absorb a heading the user named.
+			if isBroadMatcher(sc.Matcher) &&
+				laterScopeMatches(scopes, i+1, dh, docFM) {
+				break
+			}
+			claimed[matched] = true
 			end := contentScopeEndLine(heads, matched, dh.Level, parentEnd)
 			runContent(f, sc, dh.Line, dh.Level, dh.Line+1, end, blocks, mkDiag, diags)
 			if len(sc.Sections) > 0 {