Skip to content

Commit d944520

Browse files
committed
schema: per-scope walkers respect structural run boundaries
Three review findings: the per-scope walkers (rules, content, acronyms) iterated every matching heading in the parent window, even after an intervening same-level non-match closed the run for structural validation. A schema like `Step+` followed by `Summary` would still apply Step's content / rule / acronym- scope checks to a `## Step` that appeared after `## Summary`, diverging from `matchRun`'s contiguous-run semantics. Extracted a shared `ScopeRunIndices` helper in internal/schema/validate.go that mirrors matchScope's run semantics: scan forward from the first match for additional same-level matches, stop at the first same-level non-match, skip deeper headings as body content. Falls back to the first wrong-level match in the window when no in-level match exists so a misindented section still triggers per-scope checks. Wired all three walkers through the helper: - internal/schema/acronyms.go::walkRanges - internal/schema/validate_content.go::walkContentScopes - internal/rules/requiredstructure/scope_rules.go::walkScopes Dropped the no-longer-used `findHead` / `findContentMatchingHead` / `scanContentHeads` / `findMatchingHead` / `scanHeads` helpers that the walkers replaced. Regression test: TestPlan156_ScopeRunStopsAtBoundary. Two `## Step` sections satisfy the content constraint, then `## Summary`, then a third `## Step` without the required code block; the third Step is outside the run and its content check must not fire. https://claude.ai/code/session_012GGH62fZUzLuzP8T4ocGkJ
1 parent 23be2a0 commit d944520

5 files changed

Lines changed: 175 additions & 131 deletions

File tree

internal/rules/requiredstructure/scope_rules.go

Lines changed: 7 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -87,16 +87,13 @@ func walkScopes(
8787
if isSlotScope(sc) {
8888
continue
8989
}
90-
// Repeated scopes apply their rule overrides to each
91-
// matched occurrence, mirroring the structural validator's
92-
// run semantics.
93-
_, max := sc.Matcher.Repeat.Bounds()
94-
for occ := 0; max == 0 || occ < max; occ++ {
95-
matched := findMatchingHead(
96-
sc, heads, expectedLevel, parentStart, parentEnd, claimed, docFM)
97-
if matched < 0 {
98-
break
99-
}
90+
// schema.ScopeRunIndices returns the contiguous matches the
91+
// structural validator would claim, so per-scope rule
92+
// overrides only apply inside sections that are actually
93+
// part of the same run — not later non-contiguous
94+
// occurrences past an intervening heading.
95+
for _, matched := range schema.ScopeRunIndices(
96+
sc, heads, expectedLevel, parentStart, parentEnd, claimed, docFM) {
10097
dh := heads[matched]
10198
// Yield broad matchers to later named scopes so a
10299
// `regex: '.+'` repeat does not absorb a heading the
@@ -122,51 +119,6 @@ func walkScopes(
122119
}
123120
}
124121

125-
// findMatchingHead returns the earliest unclaimed heading index in
126-
// heads whose level matches expectedLevel and whose text matches
127-
// sc, restricted to the [parentStart, parentEnd) line window. When
128-
// no in-window heading at the expected level matches, it falls back
129-
// to an in-window heading at any level — the same level-mismatch
130-
// case the validator's matchScope claims. The fallback stays inside
131-
// the parent window so the walker never pairs a scope with a
132-
// heading the validator could not have claimed.
133-
func findMatchingHead(
134-
sc schema.Scope, heads []schema.DocHeading,
135-
expectedLevel, parentStart, parentEnd int,
136-
claimed map[int]bool, docFM map[string]any,
137-
) int {
138-
if idx := scanHeads(sc, heads, parentStart, parentEnd, claimed, expectedLevel, docFM); idx >= 0 {
139-
return idx
140-
}
141-
return scanHeads(sc, heads, parentStart, parentEnd, claimed, -1, docFM)
142-
}
143-
144-
// scanHeads returns the first unclaimed heading in heads whose line
145-
// is in [parentStart, parentEnd) and whose level equals
146-
// requireLevel (or any level when requireLevel < 0), and whose text
147-
// matches sc.
148-
func scanHeads(
149-
sc schema.Scope, heads []schema.DocHeading,
150-
parentStart, parentEnd int, claimed map[int]bool,
151-
requireLevel int, docFM map[string]any,
152-
) int {
153-
for j, dh := range heads {
154-
if claimed[j] {
155-
continue
156-
}
157-
if dh.Line < parentStart || dh.Line >= parentEnd {
158-
continue
159-
}
160-
if requireLevel >= 0 && dh.Level != requireLevel {
161-
continue
162-
}
163-
if schema.MatchesHeading(sc, dh, docFM) {
164-
return j
165-
}
166-
}
167-
return -1
168-
}
169-
170122
// scopeEndLine returns the exclusive end-line of the section
171123
// beginning at heads[matched]. The section ends at the first
172124
// subsequent heading whose level is <= boundaryLevel and whose line

internal/schema/acronyms.go

Lines changed: 6 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -134,12 +134,12 @@ func walkRanges(
134134
if sc.Preamble || isSlotMatcher(sc.Matcher) {
135135
continue
136136
}
137-
_, max := sc.Matcher.Repeat.Bounds()
138-
for occ := 0; max == 0 || occ < max; occ++ {
139-
idx := findHead(sc, heads, expectedLevel, parentStart, parentEnd, claimed, docFM)
140-
if idx < 0 {
141-
break
142-
}
137+
// ScopeRunIndices returns only the contiguous occurrences
138+
// the structural validator would claim — so non-contiguous
139+
// later headings (e.g. `Step` after a `Summary` boundary)
140+
// don't get their bodies scanned for first-use acronyms.
141+
for _, idx := range ScopeRunIndices(
142+
sc, heads, expectedLevel, parentStart, parentEnd, claimed, docFM) {
143143
// Yield broad matchers to later named scopes so a `.+`
144144
// repeat does not consume a heading the user named.
145145
if isBroadMatcher(sc.Matcher) &&
@@ -157,27 +157,6 @@ func walkRanges(
157157
}
158158
}
159159

160-
func findHead(
161-
sc Scope, heads []DocHeading, expectedLevel, parentStart, parentEnd int,
162-
claimed map[int]bool, docFM map[string]any,
163-
) int {
164-
for i, h := range heads {
165-
if claimed[i] {
166-
continue
167-
}
168-
if h.Line < parentStart || h.Line >= parentEnd {
169-
continue
170-
}
171-
if h.Level != expectedLevel {
172-
continue
173-
}
174-
if scopeMatchesHeading(sc, h, docFM) {
175-
return i
176-
}
177-
}
178-
return -1
179-
}
180-
181160
func nextSectionLine(heads []DocHeading, idx, level, parentEnd int) int {
182161
for j := idx + 1; j < len(heads); j++ {
183162
if heads[j].Level <= level {

internal/schema/plan156_acceptance_test.go

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

462+
// TestPlan156_ScopeRunStopsAtBoundary regresses a Copilot
463+
// finding: per-scope walkers (content, rules, acronyms) used
464+
// to scan the whole parent window for matches, so a repeated
465+
// `Step` scope would silently apply its content / rule
466+
// overrides to a `## Step` that appeared AFTER a `## Summary`
467+
// boundary, even though the structural validator stopped the
468+
// run at Summary. The walkers now use ScopeRunIndices which
469+
// mirrors matchScope's contiguous-run semantics.
470+
func TestPlan156_ScopeRunStopsAtBoundary(t *testing.T) {
471+
raw := map[string]any{
472+
"sections": []any{
473+
map[string]any{
474+
"heading": map[string]any{
475+
"regex": "Step",
476+
"repeat": map[string]any{"min": 1},
477+
},
478+
"content": []any{
479+
map[string]any{"kind": "code-block", "lang": "yaml"},
480+
},
481+
},
482+
map[string]any{"heading": "Summary"},
483+
},
484+
}
485+
sch, err := ParseInline(raw, "kind x")
486+
require.NoError(t, err)
487+
// Two Step sections (each with the required code block),
488+
// then Summary, then a third Step WITHOUT the code block.
489+
// The third Step is not part of the run — its missing
490+
// code-block should NOT be flagged.
491+
doc := newDocFile(t, "doc.md",
492+
"# T\n\n"+
493+
"## Step\n\n```yaml\nfoo: bar\n```\n\n"+
494+
"## Step\n\n```yaml\nbaz: qux\n```\n\n"+
495+
"## Summary\n\nsummary text\n\n"+
496+
"## Step\n\nno code block here\n")
497+
diags := Validate(doc, sch, nil, false, makeDiagForTest)
498+
for _, d := range diags {
499+
assert.NotContains(t, d.Message, "missing required content",
500+
"the third Step is outside the structural run; "+
501+
"its content constraint must not fire")
502+
}
503+
}
504+
462505
// TestPlan156_FlagsExtrasInIterationStream regresses a Copilot
463506
// finding: when a doc has [B, B, A] against schema [A, B], the
464507
// second B used to be silently consumed by handleNonMatch

internal/schema/validate.go

Lines changed: 113 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -737,7 +737,119 @@ func findOutOfOrderIdx(
737737
return -1
738738
}
739739

740-
// MatchesHeading reports whether sc matches the heading text in dh.
740+
// ScopeRunIndices returns the doc-heading indices that match sc's
741+
// matcher inside the [parentStart, parentEnd) window, following
742+
// the same contiguous-run semantics matchScope uses: scan
743+
// forward from the first match for additional same-level
744+
// matches, but stop at the first same-level heading that does
745+
// not match (deeper headings inside the matched section are
746+
// skipped silently). When sc is non-repeating (Repeat.Bounds()
747+
// max == 1), only the first match in the window is returned —
748+
// the structural validator allows a single-occurrence scope to
749+
// appear anywhere in the parent's headings.
750+
//
751+
// When no in-level match is found, the helper falls back to the
752+
// first wrong-level match in the window. This mirrors the
753+
// structural validator's level-mismatch fallback so per-scope
754+
// walkers still visit a section whose author got the heading
755+
// level wrong.
756+
//
757+
// Used by the per-scope walkers (acronyms, content, rules) so
758+
// they only visit occurrences the structural validator would
759+
// also have claimed as part of the same run.
760+
func ScopeRunIndices(
761+
sc Scope, heads []DocHeading,
762+
expectedLevel, parentStart, parentEnd int,
763+
claimed map[int]bool, docFM map[string]any,
764+
) []int {
765+
if sc.Matcher == nil {
766+
return nil
767+
}
768+
_, max := sc.Matcher.Repeat.Bounds()
769+
out := scanScopeRunAtLevel(
770+
sc, heads, expectedLevel, parentStart, parentEnd, max, claimed, docFM)
771+
if len(out) > 0 {
772+
return out
773+
}
774+
// No in-level match — fall back to the first wrong-level
775+
// match. Wrong-level matches never form a run (the heading
776+
// already deviates from the schema's level expectation).
777+
if idx := firstWrongLevelMatch(
778+
sc, heads, expectedLevel, parentStart, parentEnd, claimed, docFM); idx >= 0 {
779+
return []int{idx}
780+
}
781+
return nil
782+
}
783+
784+
// scanScopeRunAtLevel scans heads in source order for contiguous
785+
// matches of sc at expectedLevel. Returns at most max matches; a
786+
// zero or negative max means unbounded. The run starts at the
787+
// first match (any number of tolerated extras may precede it) and
788+
// ends at the first same-level non-match heading (deeper headings
789+
// are skipped silently as body content of an earlier match).
790+
func scanScopeRunAtLevel(
791+
sc Scope, heads []DocHeading,
792+
expectedLevel, parentStart, parentEnd, max int,
793+
claimed map[int]bool, docFM map[string]any,
794+
) []int {
795+
var out []int
796+
started := false
797+
for i, h := range heads {
798+
if claimed[i] {
799+
continue
800+
}
801+
if h.Line < parentStart || h.Line >= parentEnd {
802+
continue
803+
}
804+
if h.Level != expectedLevel {
805+
if h.Level < expectedLevel && started {
806+
break
807+
}
808+
continue
809+
}
810+
if scopeMatchesHeading(sc, h, docFM) {
811+
out = append(out, i)
812+
started = true
813+
if max == 1 || (max > 0 && len(out) >= max) {
814+
break
815+
}
816+
continue
817+
}
818+
if started {
819+
break
820+
}
821+
}
822+
return out
823+
}
824+
825+
// firstWrongLevelMatch returns the index of the first heading in
826+
// the window that matches sc at any level other than
827+
// expectedLevel, or -1 when none exists. The walker uses this
828+
// fallback so a section authored at the wrong heading depth still
829+
// gets its per-scope checks applied.
830+
func firstWrongLevelMatch(
831+
sc Scope, heads []DocHeading,
832+
expectedLevel, parentStart, parentEnd int,
833+
claimed map[int]bool, docFM map[string]any,
834+
) int {
835+
for i, h := range heads {
836+
if claimed[i] {
837+
continue
838+
}
839+
if h.Line < parentStart || h.Line >= parentEnd {
840+
continue
841+
}
842+
if h.Level == expectedLevel {
843+
continue
844+
}
845+
if scopeMatchesHeading(sc, h, docFM) {
846+
return i
847+
}
848+
}
849+
return -1
850+
}
851+
852+
// MatchesHeading reports whether sc matches dh's heading text.
741853
// Exported so callers outside the validator (notably the per-scope
742854
// rule walker in internal/rules/requiredstructure) reuse the same
743855
// matching semantics. fm is the document's parsed front matter and

internal/schema/validate_content.go

Lines changed: 6 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -238,16 +238,12 @@ func walkContentScopes(
238238
runContent(f, sc, parentStart, expectedLevel, parentStart, end, blocks, mkDiag, diags)
239239
continue
240240
}
241-
// Repeated scopes get one content pass per occurrence so a
242-
// `content:` constraint under `repeat: { min: 1, max: N }`
243-
// is checked for every matched section, not just the first.
244-
_, max := sc.Matcher.Repeat.Bounds()
245-
for occ := 0; max == 0 || occ < max; occ++ {
246-
matched := findContentMatchingHead(
247-
sc, heads, expectedLevel, parentStart, parentEnd, claimed, docFM)
248-
if matched < 0 {
249-
break
250-
}
241+
// ScopeRunIndices restricts iteration to the contiguous
242+
// run the structural validator would claim, so a
243+
// `## Step` heading after a `## Summary` boundary doesn't
244+
// silently inherit Step's content constraints.
245+
for _, matched := range ScopeRunIndices(
246+
sc, heads, expectedLevel, parentStart, parentEnd, claimed, docFM) {
251247
dh := heads[matched]
252248
// Yield broad matchers to later named scopes so a `.+`
253249
// repeat does not absorb a heading the user named.
@@ -302,44 +298,6 @@ func blocksInRange(blocks []contentBlock, startLine, endLine int) []contentBlock
302298
return out
303299
}
304300

305-
// findContentMatchingHead picks the earliest unclaimed heading at the
306-
// expected level whose text matches sc, restricted to the parent
307-
// window. Falls back to a heading at any in-window level so a level-
308-
// mismatched section still pairs (the heading walker emits the
309-
// level-mismatch diagnostic separately).
310-
func findContentMatchingHead(
311-
sc Scope, heads []DocHeading,
312-
expectedLevel, parentStart, parentEnd int,
313-
claimed map[int]bool, docFM map[string]any,
314-
) int {
315-
if idx := scanContentHeads(sc, heads, parentStart, parentEnd, claimed, expectedLevel, docFM); idx >= 0 {
316-
return idx
317-
}
318-
return scanContentHeads(sc, heads, parentStart, parentEnd, claimed, -1, docFM)
319-
}
320-
321-
func scanContentHeads(
322-
sc Scope, heads []DocHeading,
323-
parentStart, parentEnd int, claimed map[int]bool,
324-
requireLevel int, docFM map[string]any,
325-
) int {
326-
for j, dh := range heads {
327-
if claimed[j] {
328-
continue
329-
}
330-
if dh.Line < parentStart || dh.Line >= parentEnd {
331-
continue
332-
}
333-
if requireLevel >= 0 && dh.Level != requireLevel {
334-
continue
335-
}
336-
if MatchesHeading(sc, dh, docFM) {
337-
return j
338-
}
339-
}
340-
return -1
341-
}
342-
343301
// contentScopeEndLine returns the exclusive end-line of a section
344302
// matched at heads[matched]. The boundary level is the matched
345303
// heading's own level so siblings at the same level terminate the

0 commit comments

Comments
 (0)