Skip to content

Implement plan 156: unify section schema entry shape - #295

Merged
jeduden merged 1 commit into
mainfrom
claude/schema-entry-unification-JYcXC
May 16, 2026
Merged

Implement plan 156: unify section schema entry shape#295
jeduden merged 1 commit into
mainfrom
claude/schema-entry-unification-JYcXC

Conversation

@jeduden

@jeduden jeduden commented May 15, 2026

Copy link
Copy Markdown
Owner

Implements plan 156, which collapses the section-entry vocabulary to a single heading: discriminator with a unified Matcher shape.

Summary

This change restructures how document schemas define section entries. Instead of separate keys for required, aliases, repeats, sequential, min, and max scattered across the entry and scope levels, all matching logic now flows through a single Matcher struct containing Regex, Repeat (with Min/Max bounds), and Sequential fields. The schema-level require.filename: nesting is flattened to a top-level filename: key.

Key Changes

Schema structure (internal/schema/schema.go)

  • Replaced Scope fields: removed Aliases, Required, Repeats, Sequential (top-level), Min/Max, Wildcard
  • Added Matcher sub-struct with Regex, Repeat, and Sequential fields
  • Flattened Schema.Require struct to a single Schema.Filename string field
  • Preamble remains as a derived flag (when Heading is null)

Inline parser (internal/schema/parse_inline.go)

  • Rewrote heading-mapping parser to accept { regex, repeat?, sequential? } shape
  • Rejects all removed keys (required, aliases, repeats, sequential, min, max) with "removed; see plan 156" diagnostics naming the replacement
  • Flattened require: wrapper; now parses top-level filename: directly
  • Added validation that closed: only appears on schemas with sections:

Matcher implementation (internal/schema/matcher.go, new file)

  • Implements regex pattern resolution with support for \#(digits) and \#(fmvar(name)) interpolations
  • digits captures numeric sequences for sequential validation
  • fmvar() resolves frontmatter field values into the pattern
  • Provides scanInterps, resolvePattern, and helper functions for pattern compilation

Validator (internal/schema/validate.go)

  • Refactored validateScopes to thread docFM (document frontmatter) through the call stack
  • Replaced buildRequiredByText map with direct matcher-based matching via matchHeading
  • Introduced matchRun state machine to handle cardinality constraints (min/max repetitions)
  • Added sequential validation: detects out-of-order numeric headings when sequential: true
  • Updated scopeMatchesHeading to accept frontmatter for field interpolation resolution

File parser (internal/schema/parse_file.go)

  • Maps proto.md heading-row tokens to new Matcher shape:
    • ## ?{regex: '.+'}
    • ## ...{regex: '.+', repeat: {min: 0}}
    • ## Step {n}{regex: 'Step \#(digits)'}
    • ## {id}{regex: '\#(fmvar(id))'}

Test updates

  • Removed tests for rejected keys; added tests verifying rejection with migration diagnostics
  • Updated all test fixtures to use new shape (removed required:, aliases:, etc.)
  • Added plan156_acceptance_test.go covering regex matching, digit capture, and sequential validation
  • Added test_helpers_test.go with literalScope, optionalScope, slotScope constructors

Documentation

  • Updated docs/guides/schemas.md with new entry shape examples
  • Updated docs/reference/section-schema.md to reflect current grammar
  • Updated plan 156 status to ✅ (complete)
  • Updated .mdsmith.yml and example schemas to use new shape

Notable Implementation Details

  • Regex anchoring: Patterns are anchored with ^(?:...)$ to match whole heading text
  • Frontmatter interpolation: The

https://claude.ai/code/session_012GGH62fZUzLuzP8T4ocGkJ

Copilot AI review requested due to automatic review settings May 15, 2026 18:50
@codecov

codecov Bot commented May 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.75816% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.24%. Comparing base (4704dc5) to head (4b2656a).

Files with missing lines Patch % Lines
internal/schema/parse_file.go 96.82% 1 Missing and 1 partial ⚠️
Additional details and impacted files
Components Coverage Δ
Go 96.20% <99.75%> (+0.15%) ⬆️
TypeScript 99.35% <ø> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Implements plan 156 by migrating section schemas to the unified heading: discriminator and matcher-based validation model.

Changes:

  • Replaces legacy section fields with Matcher/Repeat structures and updates parsing/validation logic.
  • Adds matcher interpolation support for digits and fmvar(...), plus acceptance/unit test updates.
  • Updates docs, plan status, fixtures, and repository schema config to the new shape.

Reviewed changes

Copilot reviewed 28 out of 28 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
plan/156_schema-entry-unification.md Marks plan tasks and acceptance criteria complete.
plan/146_inline-schema-in-kinds.md Notes old entry grammar is superseded.
PLAN.md Updates plan 156 status in the catalog.
internal/schema/validate.go Refactors section matching to matcher/repeat semantics.
internal/schema/validate_content.go Updates content walker to skip slot matchers.
internal/schema/test_helpers_test.go Adds test helpers for matcher-shaped scopes.
internal/schema/schema.go Replaces legacy schema fields with matcher/repeat types.
internal/schema/schema_test.go Updates schema tests for the new grammar.
internal/schema/plan156_acceptance_test.go Adds acceptance tests for regex, fmvar, digits, and repeat behavior.
internal/schema/parse_inline.go Parses the new inline schema grammar and rejects removed keys.
internal/schema/parse_file.go Desugars proto heading tokens into matcher forms.
internal/schema/matcher.go Adds matcher interpolation, compilation, caching, and matching.
internal/schema/extras_test.go Updates acronym-related tests to matcher-shaped scopes.
internal/schema/coverage_test.go Updates parser/validator coverage for new schema fields.
internal/schema/acronyms.go Adapts acronym scope matching to matcher-based scopes.
internal/rules/requiredstructure/scope_rules.go Updates per-scope rule walker slot detection.
internal/rules/requiredstructure/inline_schema_test.go Updates required-structure inline schema tests.
internal/rules/MDS020-required-structure/README.md Documents the new inline schema entry shape.
internal/rules/MDS020-required-structure/good/inline-wildcard.md Migrates wildcard fixture to matcher slot syntax.
internal/rules/MDS020-required-structure/good/inline-runbook.md Migrates optional/disjunctive fixture entries.
internal/rules/MDS020-required-structure/good/inline-flat.md Removes now-default required fields.
internal/rules/MDS020-required-structure/good/inline-acronyms-scoped.md Migrates optional scope syntax.
internal/rules/MDS020-required-structure/bad/inline-missing.md Removes now-default required fields.
internal/rules/MDS020-required-structure/bad/inline-level-mismatch.md Removes now-default required fields.
internal/rules/MDS020-required-structure/bad/inline-closed-unlisted.md Removes now-default required fields.
docs/reference/section-schema.md Removes the “upcoming” notice.
docs/guides/schemas.md Rewrites schema guide examples and grammar for matcher syntax.
.mdsmith.yml Migrates built-in kind schemas to the new shape.

Comment thread internal/schema/validate.go
Comment thread internal/schema/matcher.go
Comment thread internal/schema/validate.go Outdated
jeduden pushed a commit that referenced this pull request May 15, 2026
Three fixes from the PR #295 review:

1. parse_inline.go: surface unsupported helpers and unterminated
   `\#(` references at parse time. `resolvePatternForCheck` used
   to swallow `rewriteInterps` errors and treat any non-`digits`
   interpolation as a probe literal, so `\#(unknown)` and `\#(`
   would slip through and degrade into runtime missing-section
   diagnostics. Both now parse-error with the regex field path.

2. validate.go: thread `docFM` through the per-scope walkers so
   `\#(fmvar(...))` resolves against the document's frontmatter.
   `MatchesHeading` now takes the FM map explicitly; the
   requiredstructure scope-rules walker, the schema acronym
   walker, and `ValidateContent` all forward `docFM` from
   `checkInlineSchema`. Without this, scoped rules / `content:`
   under an fmvar heading were silently skipped because the
   walker's match call used `nil` FM.

3. validate.go: enforce `repeat.max` in open schemas. The
   matchScope loop exited as soon as `consumed == max`, leaving
   extras to the trailing-leftover pass — which only flagged
   them under `closed: true`. The new `flagExtrasBeyondMax`
   helper walks remaining same-level matches and emits a
   "matched N times, allowed at most M" diagnostic, while still
   yielding any heading that would be claimed by a later listed
   scope.

Tests cover each fix:
- TestPlan156_RejectsUnknownInterpHelperAtParseTime
- TestPlan156_RepeatMaxEnforcedInOpenSchema
- TestCheck_InlineSchema_ScopeRulesUnderFmvarHeading

https://claude.ai/code/session_012GGH62fZUzLuzP8T4ocGkJ
@jeduden
jeduden requested a review from Copilot May 15, 2026 19:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 29 out of 29 changed files in this pull request and generated 6 comments.

Comment thread internal/schema/validate.go
Comment thread internal/schema/validate.go Outdated
Comment thread internal/schema/validate_content.go Outdated
Comment thread internal/rules/requiredstructure/scope_rules.go Outdated
Comment thread internal/schema/acronyms.go Outdated
Comment thread internal/schema/matcher.go Outdated
jeduden pushed a commit that referenced this pull request May 15, 2026
Codecov flagged the patch coverage on PR #295. Add focused tests
for the new helpers introduced by plan 156:

- claimsLaterLiteral: cover all four branches (slot, preamble,
  optional, already-claimed) plus the no-match case.
- displayHeading: cover the three label-source paths (bare
  Heading, Matcher fallback, preamble empty).
- claimRun (wrong-level): exercise the level-mismatch path via
  a nested schema whose inner heading appears at the outer
  level.
- sequentialDiagMessage: cover the non-integer guard and the
  happy path.
- cachedMatcher / compileMatcher: nil and invalid-pattern
  early-error branches.
- matchHeading: nil-matcher branch and `digits` capture path.
- fmvarLookup: missing-field, nil-fm, empty-name returns.
- resolvePattern: passthrough literals and unknown-helper error.
- setMatcherRegex / setMatcherRepeat: type and bound guards.
- scopeMatchesHeading: nil-matcher branch.
- protoTokenRegex: literal, {n}, {field}, and mixed forms.
- Scope.Required and Repeat.Bounds: every branch.

Schema-package coverage moves from 95.4% to 96.8%.

https://claude.ai/code/session_012GGH62fZUzLuzP8T4ocGkJ
@jeduden
jeduden requested a review from Copilot May 15, 2026 19:12

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 30 out of 30 changed files in this pull request and generated 8 comments.

Comment thread internal/schema/validate.go
Comment thread internal/schema/parse_inline.go
Comment thread internal/schema/parse_file.go
Comment thread internal/schema/plan156_acceptance_test.go Outdated
Comment thread internal/schema/validate_content.go Outdated
Comment thread internal/rules/requiredstructure/scope_rules.go Outdated
Comment thread internal/schema/acronyms.go Outdated
Comment thread internal/schema/validate.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 30 out of 30 changed files in this pull request and generated 5 comments.

Comment thread internal/schema/validate_content.go Outdated
Comment thread internal/rules/requiredstructure/scope_rules.go Outdated
Comment thread internal/schema/acronyms.go Outdated
Comment thread internal/schema/validate.go Outdated
Comment thread internal/schema/matcher.go

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 30 out of 30 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (3)

internal/schema/validate.go:427

  • Optional matchers (repeat.min == 0) stop at the first same-level non-matching heading because consumed >= min is true before any match. That prevents them from scanning past tolerated/unlisted headings, so an in-order optional section that appears after an extra heading is later reported as out-of-order by the leftover pass instead of being claimed normally. Required matchers still skip open-scope extras via handleNonMatch, so optional matchers should not terminate solely because min is already satisfied when the current heading is not a boundary for a later listed scope.
	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
		}
		return true, docIdx, s.consumed > 0

internal/schema/validate.go:603

  • Out-of-order matching also claims only one heading from a repeated scope and then marks that scope as fully claimed. That skips repeat bounds and sequential validation for out-of-order runs, so a repeated section that appears before the expected predecessor can avoid min/max diagnostics after the first occurrence is claimed. Use the run matcher for the later scope here so repeat cardinality is enforced consistently regardless of order.
	claimed[ooIdx] = true
	docIdx++
	if len(ooSc.Sections) > 0 {
		newIdx, childDiags := validateScopes(
			f, ooSc.Sections, ooSc.Closed, docHeads, docIdx,
			expectedLevel+1, docFM, mkDiag)

internal/schema/validate.go:405

  • The wrong-level match path delegates to claimRun, but that helper claims a single heading without updating the current run's consumed count or captured digits. For repeated matchers, a wrong-level occurrence can therefore satisfy claimedThis while bypassing repeat.min/max and sequential diagnostics. This branch should either go through claimMatch/run accounting or otherwise preserve the same cardinality checks as normal matches.
		if matched, captured := matchHeading(sc.Matcher, dh, s.docFM); matched {
			newIdx, runDiags := claimRun(
				s.f, sc, s.idx, s.expectedLevel, docHeads, docIdx,
				s.claimed, s.docFM, s.mkDiag, captured)
			s.diags = append(s.diags, runDiags...)
			return true, newIdx, true

Comment thread internal/schema/schema_test.go Outdated
Comment thread internal/schema/matcher.go Outdated
Comment thread internal/schema/validate.go
Comment thread docs/reference/section-schema.md
Comment thread internal/schema/acronyms.go

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 30 out of 30 changed files in this pull request and generated 4 comments.

Comment thread docs/guides/schemas.md Outdated
Comment thread internal/schema/test_helpers_test.go Outdated
Comment thread internal/schema/validate.go Outdated
Comment thread internal/schema/validate.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 30 out of 30 changed files in this pull request and generated 2 comments.

Comment thread internal/schema/validate.go Outdated
Comment thread docs/reference/section-schema.md

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 33 out of 33 changed files in this pull request and generated 3 comments.

Comment thread internal/schema/validate.go Outdated
Comment thread internal/rules/requiredstructure/inline_schema_test.go
Comment thread internal/schema/plan156_coverage_test.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 33 out of 33 changed files in this pull request and generated 4 comments.

Comment thread internal/schema/parse_file.go
Comment thread internal/rules/requiredstructure/scope_rules.go Outdated
Comment thread internal/schema/parse_inline.go
Comment thread internal/schema/validate.go
@jeduden
jeduden force-pushed the claude/schema-entry-unification-JYcXC branch from 0587779 to 6665121 Compare May 16, 2026 08:05
@jeduden
jeduden requested a review from Copilot May 16, 2026 08:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 36 out of 36 changed files in this pull request and generated no new comments.

@jeduden
jeduden requested a review from Copilot May 16, 2026 08:10
@jeduden
jeduden force-pushed the claude/schema-entry-unification-JYcXC branch from 6665121 to cc9fe08 Compare May 16, 2026 08:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 36 out of 36 changed files in this pull request and generated 3 comments.

Comment thread internal/schema/validate.go
Comment thread internal/schema/matcher.go Outdated
Comment thread .mdsmith.pinned.yml
@jeduden
jeduden requested a review from Copilot May 16, 2026 08:32
@jeduden
jeduden force-pushed the claude/schema-entry-unification-JYcXC branch from cc9fe08 to a258470 Compare May 16, 2026 08:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 36 out of 36 changed files in this pull request and generated 2 comments.

Comment thread internal/schema/matcher.go Outdated
Comment thread internal/schema/parse_inline.go Outdated
@jeduden
jeduden requested a review from Copilot May 16, 2026 08:38
@jeduden
jeduden force-pushed the claude/schema-entry-unification-JYcXC branch from a258470 to 6b61a64 Compare May 16, 2026 08:38
@jeduden
jeduden force-pushed the claude/schema-entry-unification-JYcXC branch from 6b61a64 to fc217ec Compare May 16, 2026 08:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 36 out of 36 changed files in this pull request and generated 3 comments.

Comment thread internal/schema/validate.go
Comment thread internal/schema/validate.go
Comment thread internal/schema/matcher.go Outdated
@jeduden
jeduden requested a review from Copilot May 16, 2026 08:49
@jeduden
jeduden force-pushed the claude/schema-entry-unification-JYcXC branch from fc217ec to 6dbab57 Compare May 16, 2026 08:49
@jeduden
jeduden force-pushed the claude/schema-entry-unification-JYcXC branch 3 times, most recently from cafda94 to 923bf5b Compare May 16, 2026 08:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 36 out of 36 changed files in this pull request and generated 2 comments.

Comment thread internal/schema/matcher.go Outdated
Comment thread internal/schema/parse_inline.go
@jeduden
jeduden requested a review from Copilot May 16, 2026 09:01
@jeduden
jeduden force-pushed the claude/schema-entry-unification-JYcXC branch 4 times, most recently from cbb5fa6 to 0ad8825 Compare May 16, 2026 09:11
…156)

Plan 156 collapses the section-entry vocabulary to one
discriminator (`heading:` — null, string, or mapping) with a
single matcher (`regex:` as a Go RE2 body with `\#(digits)` and
`\#(fmvar(name))` helpers) and one cardinality field
(`repeat: { min, max }`). Hard cutover: `aliases:`, `required:`,
`{unlisted: true}`, scope-level `repeats:`/`sequential:`/
`min:`/`max:`, and `require.filename:` all parse-error with a
"removed; see plan 156" diagnostic naming the replacement.

Core changes:
- internal/schema: new Matcher / Repeat structs, parse_inline.go
  / parse_file.go rewritten for the new shape, matcher.go
  centralises RE2 compilation with a bounded cache keyed on the
  pattern plus a frontmatter fingerprint (collision-safe via
  strconv.Quote).
- validate.go: matchScope state machine (step / claimMatch /
  handleNonMatch / flagExtrasBeyondMax / finishRun) drives
  contiguous-run claiming with min/max enforcement, broad-and-
  after-min yield, out-of-order recovery, and non-contiguous
  flagging via claimedScopeMatches (claimCounts tracks per-scope
  occurrences so overlapping matchers don't inflate counts).
- ScopeRunIndices helper shared with validate_content.go,
  acronyms.go's walkRanges, and requiredstructure/scope_rules.go
  so per-scope walkers honour the same run / yield semantics.
- acronyms.go walkRanges surfaces the matched heading text so
  scope-name allowlists work with disjunctive regexes
  (`Symptoms|Indicators` + `scope: ["Indicators"]`).

Config and fixtures:
- .mdsmith.yml inline schemas migrated to the new shape.
- .mdsmith.pinned.yml restates the same inline schemas in the
  pre-156 form so the mdsmith-fixed-version CI gate (pinned at
  v0.15.0) can lint the tree via `-c`; merge-queue.yml swaps it
  in over .mdsmith.yml with `git update-index --skip-worktree`
  to keep the worktree clean for merge-queue-action's git ops.
- MDS020 good/bad fixtures updated to the new shape.
- docs/guides/schemas.md and docs/reference/section-schema.md
  rewritten; the `aliases: [A, B]` → `regex: 'A|B'` row sits in
  prose because GFM's pipe-escape inside code spans renders
  inconsistently.
- Reference acknowledges that MDS020's file-schema path still
  uses the legacy parseSchema pipeline; the new parser is the
  in-memory shape behind inline schemas.

Regression coverage (TestPlan156_*):
  RegexMatchesRenderedPlainText, DigitsCaptureSequential,
  FmvarInterpolates, RepeatBoundsEnforced,
  RepeatMaxEnforcedInOpenSchema, RejectsUnknownInterpHelper,
  RepeatYieldsToOptionalLaterScope, SequentialDiagOnPartialRun,
  FmFingerprintCollisionGuard, RepeatSpansDeeperHeadings,
  FlagExtrasBeyondMaxSkipsDeeper, RejectsMultipleDigitsInline,
  OptionalSpecificClaimsOwnSlot, FlagsExtrasMatchingClaimedScope,
  OutOfOrderSequentialDiagFires, OptionalYieldsToBroadFollower,
  ScopeRunStopsAtBoundary, FlagsExtrasInIterationStream,
  UnboundedRepeatDoesNotFlagAsExceeded,
  BroadMatcherYieldsBeforeMin, LateClaimFlagsRepeatMin,
  WrongLevelMatchCountsTowardRepeat, RejectsInvalidFmvarPath,
  OptionalMatcherSkipsTolerated, BroadRepeatYieldsAcronymScope,
  BroadRepeatYieldsToLaterScopeInPerScopeWalkers,
  RejectsMultipleNTokensProto, AcronymScopeOnRepeatedScope,
  ContentOnRepeatedScope, RulesOnRepeatedScope,
  AcronymScopeMatchesByHeadingText,
  OutOfOrderRunCountsAvailableMatches,
  NonContiguousClaimedScopeFlagged,
  LateClaimChildRecursionStopsAtParentLevel,
  OverlappingMatcherDoesNotInflateClaimCount.

https://claude.ai/code/session_012GGH62fZUzLuzP8T4ocGkJ
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants