schema: address fourth Copilot review round (plan 156)

Five new review findings (plus the optional-matcher case from
the third round):

1. matchScope::step — an optional matcher (`repeat.min == 0`)
   used to terminate at the first non-matching same-level
   heading even when consumed == 0. That made an in-order
   optional section after a tolerated extra get flagged as
   out-of-order by the leftover pass. The step path now
   distinguishes "active run terminated" (consumed > 0) from
   "optional not started" (consumed == 0) and, for the latter,
   yields to a matching later scope or advances past the
   tolerated heading. Test:
   TestPlan156_OptionalMatcherSkipsTolerated.

2. matcher.go::resolvePatternForCheck — `fmvar(name)` no longer
   passes parse-time validation with a non-CUE-path argument.
   `fmvar(my-key)` (unquoted hyphen) now errors at parse with a
   pointer at the canonical `fmvar("my-key")` quoting. Test:
   TestPlan156_RejectsInvalidFmvarPath.

3. schema_test.go — delete the permanently-skipped
   TestParseInline_ContentRejectedOnQuestionMarkHeading. The
   `heading: "?"` shape is gone; the test was dead code.

4. acronyms.go::acronymRanges — refresh the function comment so
   it matches the repeated-scope behavior the prior round added
   (one range per occurrence, not first-match-only).

5. docs/reference/section-schema.md — the "regex: is a CUE
   expression" wording oversold what mdsmith actually does. The
   resolver handles only `\#(digits)` and `\#(fmvar(...))`;
   rewrite the page to describe the helper-only surface.

Known-limitation comment on claimLateScope: out-of-order
recovery still claims one heading and skips repeat/sequential
checks. The in-order matchScope path remains the primary
enforcement surface; non-contiguous run enforcement is tracked
as a separate plan.

https://claude.ai/code/session_012GGH62fZUzLuzP8T4ocGkJ

Author: claude

docs/reference/section-schema.md

@@ -108,20 +108,19 @@ value is a mapping.
 
 ## The regex matcher
 
-`regex:` is a CUE expression evaluating to a
-string. The string is compiled as Go RE2.
-
-The YAML value is the body of a CUE
-raw-interpolation string. mdsmith wraps it in
-`#"..."#` before evaluating. Two consequences:
+`regex:` is a Go RE2 pattern body. mdsmith
+recognizes two interpolation references in the
+pattern surface — borrowed from CUE's
+raw-interpolation syntax (`\#(expr)`) — but it
+does not actually evaluate the body as CUE. Two
+consequences:
 
 - **Backslash is literal.** Write `\d`, `\w`,
   `\.`, `\(` directly — no doubling. Plain
   RE2 patterns work as-is.
-- **Interpolation is `\#(expr)`.** Inside the
-  string, `\#(x)` evaluates `x` in the CUE
-  scope (frontmatter fields plus mdsmith
-  helpers) and substitutes the result.
+- **Interpolation is `\#(expr)`.** Only the two
+  helpers below are accepted; any other `expr`
+  parse-errors with "unknown helper".
 
 **Anchoring.** Whole-string. `regex: 'Overview'`
 matches a heading whose text is exactly

internal/schema/acronyms.go

@@ -77,13 +77,11 @@ type lineRange struct {
 
 // acronymRanges returns the line windows the acronym check should
 // scan. An empty scope list applies to the whole document.
-// Otherwise the schema scope tree is walked and each schema scope
-// claims the first matching document heading at its level; if the
-// document repeats a heading text under the same schema scope,
-// only the first occurrence is included today. This matches the
-// validator's claim semantics; widening to multi-match would
-// require schema repeats support (currently rejected by the
-// parser) and is tracked as a follow-up.
+// Otherwise the schema scope tree is walked and one line range is
+// emitted per occurrence: a repeated scope (`repeat.max > 1` or
+// unbounded) contributes a range for each matched heading, so a
+// scope name like "Diagnosis" applied to two `## Diagnosis`
+// sections scans both bodies for first-use acronyms.
 func acronymRanges(f *lint.File, sch *Schema, scope []string, docFM map[string]any) []lineRange {
 	if len(scope) == 0 {
 		return []lineRange{{Start: 1, End: len(f.Lines) + 1}}

internal/schema/matcher.go

@@ -97,16 +97,24 @@ func resolvePattern(pattern string, fm map[string]any) (string, error) {
 // frontmatter lookups so the result is syntactically valid even
 // when no document is in hand. Used at parse time to catch invalid
 // RE2 in `regex:` early. Returns an error when the pattern uses an
-// unsupported helper or an unterminated `\#(` reference; the
-// validator never has a chance to compile a pattern that didn't
-// pass this check.
+// unsupported helper, an unterminated `\#(` reference, or an
+// `fmvar(name)` whose argument is not a valid CUE path — turning
+// a schema typo into a parse-time diagnostic instead of a
+// confusing missing-section diagnostic at validate-time.
 func resolvePatternForCheck(pattern string) (string, error) {
 	return rewriteInterps(pattern, func(expr string) (string, error) {
 		expr = strings.TrimSpace(expr)
 		switch {
 		case expr == "digits":
 			return digitsCaptureExpr, nil
 		case fmvarCallRe.MatchString(expr):
+			name := strings.TrimSpace(fmvarCallRe.FindStringSubmatch(expr)[1])
+			if fieldinterp.ParseCUEPath(name) == nil {
+				return "", fmt.Errorf(
+					"`fmvar(%s)`: invalid frontmatter path "+
+						"(non-identifier keys must be quoted, "+
+						"e.g. `fmvar(\"my-key\")`)", name)
+			}
 			// Use a literal placeholder so the compiled regex is
 			// syntactically valid. The validator will re-resolve
 			// against the real frontmatter at validate-time.

internal/schema/plan156_acceptance_test.go

@@ -389,6 +389,57 @@ func TestPlan156_OptionalSpecificClaimsOwnSlot(t *testing.T) {
 			"a broad `.+` matcher must not absorb it")
 }
 
+// TestPlan156_RejectsInvalidFmvarPath regresses a Copilot
+// finding: `fmvar(name)` accepted any argument string at parse
+// time, so a typo like `fmvar(my-key)` (unquoted hyphenated key)
+// only surfaced as a confusing missing-section diagnostic at
+// validate-time. Parse-time validation now applies the same CUE
+// path rules as the runtime lookup.
+func TestPlan156_RejectsInvalidFmvarPath(t *testing.T) {
+	raw := map[string]any{
+		"sections": []any{
+			map[string]any{"heading": map[string]any{
+				"regex": `\#(fmvar(my-key))`,
+			}},
+		},
+	}
+	_, err := ParseInline(raw, "kind x")
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "fmvar(my-key)")
+	assert.Contains(t, err.Error(), "non-identifier keys must be quoted")
+}
+
+// TestPlan156_OptionalMatcherSkipsTolerated regresses a Copilot
+// low-confidence finding: an optional matcher (`repeat.min == 0`)
+// stopped at the first non-matching heading and left later
+// in-order occurrences to be flagged as out-of-order by the
+// leftover pass. The matcher should scan past tolerated extras
+// in open schemas the same way a required matcher does, only
+// terminating once it has actually started a run.
+func TestPlan156_OptionalMatcherSkipsTolerated(t *testing.T) {
+	raw := map[string]any{
+		"sections": []any{
+			map[string]any{
+				"heading": map[string]any{
+					"regex":  "X",
+					"repeat": map[string]any{"min": 0, "max": 1},
+				},
+			},
+		},
+	}
+	sch, err := ParseInline(raw, "kind x")
+	require.NoError(t, err)
+	// Doc has a tolerated extra before X. The open-schema walk
+	// should silently consume Other and then claim X normally.
+	doc := newDocFile(t, "doc.md",
+		"# T\n\n## Other\n\nx\n\n## X\n\ny\n")
+	diags := Validate(doc, sch, nil, false, makeDiagForTest)
+	for _, d := range diags {
+		assert.NotContains(t, d.Message, "out of order",
+			"optional X must claim itself even after a tolerated extra")
+	}
+}
+
 // TestPlan156_BroadRepeatYieldsAcronymScope covers the acronym
 // walker's broad-matcher yield: an acronym-scoped named section
 // after a broad repeated matcher must still get its body

internal/schema/schema_test.go

@@ -427,19 +427,6 @@ func TestParseInline_ContentRejectedOnWildcard(t *testing.T) {
 		"not allowed on a slot")
 }
 
-func TestParseInline_ContentRejectedOnQuestionMarkHeading(t *testing.T) {
-	t.Skip("plan 156 dropped the inline `heading: \"?\"` shape — use `heading: { regex: '.+' }`")
-	_, err := ParseInline(map[string]any{
-		"sections": []any{map[string]any{
-			"heading": "?",
-			"content": []any{map[string]any{"kind": "paragraph"}},
-		}},
-	}, "kind x")
-	require.Error(t, err)
-	assert.Contains(t, err.Error(),
-		"not allowed on a `?` wildcard heading")
-}
-
 // ---- Validate (content:) ----
 
 func TestValidate_Content_MissingCodeBlock(t *testing.T) {

internal/schema/validate.go

@@ -229,6 +229,13 @@ func handleLeftoverHeadings(
 // emits its out-of-order diagnostic, and recurses into the scope's
 // nested children so missing-required-section diagnostics still
 // surface beneath a late parent.
+// Known limitation: claimLateScope (and claimOutOfOrder below)
+// claim a single heading and mark the scope fully claimed. Repeat
+// cardinality (`repeat.min/max`) and `sequential:` checks are
+// skipped for the late/out-of-order recovery path. Enforcing run
+// semantics for non-contiguous occurrences is a separate plan;
+// the in-order matchScope path is the primary enforcement
+// surface.
 func claimLateScope(
 	f *lint.File, scopes []Scope, idx, expectedLevel int,
 	docHeads []DocHeading, docIdx int, claimed map[int]bool,
@@ -414,17 +421,32 @@ func (s *matchRun) step(docHeads []DocHeading, docIdx int) (bool, int, bool) {
 		}
 		return false, s.claimMatch(docHeads, docIdx, captured), false
 	}
+	// A deeper-than-expected heading is part of an earlier claimed
+	// scope's body, not a sibling boundary — skip it so the run
+	// keeps scanning for the next same-level match.
+	if dh.Level > s.expectedLevel {
+		return false, docIdx + 1, false
+	}
+	if s.consumed > 0 {
+		// An active run is contiguous; the first same-level
+		// non-match closes it.
+		return true, docIdx, true
+	}
 	if s.consumed >= s.min {
-		// A deeper-than-expected heading is part of an earlier
-		// claimed scope's body, not a sibling boundary — skip it
-		// so the run keeps scanning for the next same-level match.
-		// Without this, a repeated `## Step` whose first occurrence
-		// contains a `### Detail` would terminate after Step 1 and
-		// leave later steps unclaimed.
-		if dh.Level > s.expectedLevel {
-			return false, docIdx + 1, false
+		// Optional matcher (min=0) hasn't started yet. Yield to a
+		// later listed scope when the heading would claim it;
+		// otherwise the heading is a tolerated extra: skip it in
+		// open schemas, flag it in closed.
+		if claimsLaterLiteral(s.scopes, s.idx+1, dh, s.claimed, s.docFM) {
+			return true, docIdx, false
 		}
-		return true, docIdx, s.consumed > 0
+		if !s.allowExtra && s.closed {
+			s.diags = append(s.diags, s.mkDiag(s.f.Path, dh.Line,
+				fmt.Sprintf("unexpected section %q (expected %q)",
+					formatHeading(dh.Level, dh.Text),
+					formatHeading(s.expectedLevel, displayHeading(sc)))))
+		}
+		return false, docIdx + 1, false
 	}
 	return s.handleNonMatch(docHeads, docIdx)
 }