Drive schema-source translation via rule.SettingsTranslator

Addresses repo-owner review on merge.go: the config merge layer
no longer special-cases the "required-structure" rule name.

- New rule.SettingsTranslator interface (mirrors the existing
  rule.ListMerger pattern). requiredstructure.Rule implements
  TranslateLayerSettings; the schema:/inline-schema: ->
  schema-sources collapse logic (plus its deep-clone helper)
  moves out of internal/config into the rule that owns those
  setting semantics.
- merge.go applies translation generically via a
  rule.ByName-driven translateLayerSettings helper, exactly like
  deepmerge.go already resolves rule.ListMerger. provenance.go
  mirrors it with translateLayerRules so kinds resolve / --explain
  stay consistent with the engine.
- Removed the now-unused translateSchemaSource /
  extractSchemaSourceFromSettings / translateSchemaSourcesInRules
  / needsSchemaSourceTranslation from internal/config.

Tests reworked to target the new generic surface plus the rule's
translator directly; full suite green, lint clean, 100% line and
branch coverage on the moved/added code.

Author: claude

internal/config/merge.go

@@ -1,5 +1,7 @@
 package config
 
+import "github.com/jeduden/mdsmith/internal/rule"
+
 // Merge merges a loaded config on top of defaults. The loaded config's rules
 // override the defaults; any rule not mentioned in loaded keeps its default
 // value. Ignore and Overrides come from the loaded config only.
@@ -339,21 +341,20 @@ func effectiveRules(cfg *Config, filePath string, kinds []string) map[string]Rul
 	// the convention's `Enabled: true` and silently disable the rule
 	// the user just asked the convention to enable.
 	//
-	// required-structure's `schema:` and `inline-schema:` settings get
-	// a separate accumulation pass: each layer that declares either
-	// source contributes one entry to a `schema-sources` list rather
-	// than scalar-replacing the previous layer. The rule receives the
-	// list and composes the sources at Check time (plan 156).
+	// Some rules rewrite a layer's settings before deep-merge by
+	// implementing rule.SettingsTranslator (required-structure
+	// collapses `schema:` / `inline-schema:` into an append-mode
+	// `schema-sources` list so multiple kinds compose, plan 156).
+	// translateLayerSettings consults the rule registry, so this
+	// merge code carries no rule-name special cases.
 	result := make(map[string]RuleCfg, len(cfg.Rules))
 	for k, v := range cfg.Rules {
 		if !cfg.ExplicitRules[k] {
-			result[k] = copyRuleCfg(translateSchemaSource(v))
+			result[k] = copyRuleCfg(translateLayerSettings(k, v))
 		}
 	}
 	apply := func(name string, layer RuleCfg) {
-		if name == "required-structure" {
-			layer = translateSchemaSource(layer)
-		}
+		layer = translateLayerSettings(name, layer)
 		if existing, ok := result[name]; ok {
 			result[name] = mergeRuleCfg(name, existing, layer)
 			return
@@ -393,61 +394,34 @@ func effectiveRules(cfg *Config, filePath string, kinds []string) map[string]Rul
 	return result
 }
 
-// translateSchemaSource converts a layer's required-structure
-// `schema:` (file path) or `inline-schema:` (map) settings into a
-// single-entry `schema-sources` list and strips the legacy keys.
-// Layers that pass through deep-merge thus accumulate sources via the
-// rule's MergeAppend declaration on `schema-sources` instead of
-// scalar-replacing.
+// translateLayerSettings applies a rule's rule.SettingsTranslator
+// (when it implements one) to a single config layer's settings
+// before deep-merge. Rules without the interface — or rules not
+// registered in this binary — pass through unchanged, matching the
+// existing rule.ByName-driven ListMerger lookup in deepmerge.go.
 //
-// Empty values (`schema: ""`, `inline-schema: {}`) are stripped
-// without contributing a source. This keeps defaults' empty
-// placeholder for `schema:` from polluting the composed list.
-func translateSchemaSource(rc RuleCfg) RuleCfg {
+// The merge code calls this for every layer keyed only by rule
+// name, so it carries no rule-specific behaviour itself: a rule
+// that needs its settings rewritten (e.g. required-structure
+// collapsing `schema:`/`inline-schema:` into `schema-sources`)
+// declares that by implementing the interface.
+func translateLayerSettings(ruleName string, rc RuleCfg) RuleCfg {
 	if rc.Settings == nil {
 		return rc
 	}
-	source, hadKey := extractSchemaSourceFromSettings(rc.Settings)
-	if !hadKey {
+	r := rule.ByName(ruleName)
+	if r == nil {
 		return rc
 	}
-	newSettings := cloneSettings(rc.Settings)
-	delete(newSettings, "schema")
-	delete(newSettings, "inline-schema")
-	if source != nil {
-		existing, _ := newSettings["schema-sources"].([]any)
-		newSettings["schema-sources"] = append(existing, source)
+	t, ok := r.(rule.SettingsTranslator)
+	if !ok {
+		return rc
 	}
 	out := rc
-	out.Settings = newSettings
+	out.Settings = t.TranslateLayerSettings(rc.Settings)
 	return out
 }
 
-// extractSchemaSourceFromSettings inspects a settings map for a
-// schema source declaration. Returns (source, true) when either key
-// is present (even if the value is empty / no-op) so the caller can
-// strip the legacy keys; returns (nil, false) when no schema key
-// appears at all.
-func extractSchemaSourceFromSettings(s map[string]any) (any, bool) {
-	hadKey := false
-	if v, ok := s["schema"]; ok {
-		hadKey = true
-		if path, ok := v.(string); ok && path != "" {
-			return map[string]any{"file": path}, true
-		}
-	}
-	if v, ok := s["inline-schema"]; ok {
-		hadKey = true
-		if m, ok := v.(map[string]any); ok && len(m) > 0 {
-			return map[string]any{"inline": cloneSettings(m)}, true
-		}
-	}
-	if !hadKey {
-		return nil, false
-	}
-	return nil, true
-}
-
 // applyPathPattern appends a {kind, pattern} entry to the
 // `path-patterns` list setting on required-structure, creating the
 // rule entry if missing. Each kind in the file's effective kind list

internal/config/provenance.go

@@ -157,7 +157,7 @@ func buildLayers(cfg *Config, filePath string, kinds []ResolvedKind) []layerInfo
 	if len(defaults) > 0 {
 		layers = append(layers, layerInfo{
 			Source: layerSourceDefault,
-			Rules:  translateSchemaSourcesInRules(defaults),
+			Rules:  translateLayerRules(defaults),
 		})
 	}
 	if cfg.Convention != "" && len(cfg.ConventionPreset) > 0 {
@@ -167,13 +167,13 @@ func buildLayers(cfg *Config, filePath string, kinds []ResolvedKind) []layerInfo
 		}
 		layers = append(layers, layerInfo{
 			Source: source,
-			Rules:  translateSchemaSourcesInRules(cfg.ConventionPreset),
+			Rules:  translateLayerRules(cfg.ConventionPreset),
 		})
 	}
 	if len(user) > 0 {
 		layers = append(layers, layerInfo{
 			Source: layerSourceUser,
-			Rules:  translateSchemaSourcesInRules(user),
+			Rules:  translateLayerRules(user),
 		})
 	}
 	for _, k := range kinds {
@@ -190,7 +190,7 @@ func buildLayers(cfg *Config, filePath string, kinds []ResolvedKind) []layerInfo
 		if matchesAny(o.Patterns(), filePath) {
 			layers = append(layers, layerInfo{
 				Source: fmt.Sprintf("overrides[%d]", i),
-				Rules:  translateSchemaSourcesInRules(o.Rules),
+				Rules:  translateLayerRules(o.Rules),
 			})
 		}
 	}
@@ -211,14 +211,11 @@ func buildLayers(cfg *Config, filePath string, kinds []ResolvedKind) []layerInfo
 // diverges from the rule config the engine actually applied.
 func kindLayerRules(kindName string, body KindBody) map[string]RuleCfg {
 	if len(body.Schema) == 0 && body.PathPattern == "" {
-		return translateSchemaSourcesInRules(body.Rules)
+		return translateLayerRules(body.Rules)
 	}
 	out := make(map[string]RuleCfg, len(body.Rules)+1)
 	for k, v := range body.Rules {
-		if k == "required-structure" {
-			v = translateSchemaSource(v)
-		}
-		out[k] = v
+		out[k] = translateLayerSettings(k, v)
 	}
 	rs := out["required-structure"]
 	rs.Enabled = true
@@ -241,45 +238,20 @@ func kindLayerRules(kindName string, body KindBody) map[string]RuleCfg {
 	return out
 }
 
-// translateSchemaSourcesInRules walks a rules map looking for a
-// required-structure entry whose Settings still carry the legacy
-// `schema:` / `inline-schema:` keys; when found, the entry is
-// rewritten to use `schema-sources:` so the provenance layer chain
-// shows the same setting key the engine merges on. Other rules are
-// returned untouched (the caller's map is shared when no
-// translation is needed).
-func translateSchemaSourcesInRules(rules map[string]RuleCfg) map[string]RuleCfg {
-	rs, ok := rules["required-structure"]
-	if !ok {
-		return rules
-	}
-	if !needsSchemaSourceTranslation(rs) {
-		return rules
-	}
+// translateLayerRules applies each rule's rule.SettingsTranslator
+// (via translateLayerSettings) across a whole rules map so the
+// provenance layer chain shows the same setting keys the engine
+// merges on. Rules without a translator pass through unchanged.
+// Provenance is not a hot path, so this always returns a fresh
+// map rather than threading an allocation-free fast path.
+func translateLayerRules(rules map[string]RuleCfg) map[string]RuleCfg {
 	out := make(map[string]RuleCfg, len(rules))
 	for k, v := range rules {
-		out[k] = v
+		out[k] = translateLayerSettings(k, v)
 	}
-	out["required-structure"] = translateSchemaSource(rs)
 	return out
 }
 
-// needsSchemaSourceTranslation reports whether rc.Settings still
-// carries the legacy single-source keys that translateSchemaSource
-// would rewrite.
-func needsSchemaSourceTranslation(rc RuleCfg) bool {
-	if rc.Settings == nil {
-		return false
-	}
-	if _, ok := rc.Settings["schema"]; ok {
-		return true
-	}
-	if _, ok := rc.Settings["inline-schema"]; ok {
-		return true
-	}
-	return false
-}
-
 // splitRulesByExplicit divides cfg.Rules into two maps using
 // cfg.ExplicitRules as the discriminator: defaults (rules the user
 // did not explicitly set) and user (rules with an entry in

internal/config/schema_kinds_test.go

@@ -274,27 +274,47 @@ func TestEffectiveKinds_CfgDelegatesToResolver(t *testing.T) {
 	assert.Equal(t, []string{"plan"}, got)
 }
 
-// TestExtractSchemaSourceFromSettings_NonStringSchemaKey covers
-// the `path, ok := v.(string)` false branch: when `schema:` is
-// set to a non-string value, the extractor still flags hadKey
-// but produces no source.
-func TestExtractSchemaSourceFromSettings_NonStringSchemaKey(t *testing.T) {
-	src, hadKey := extractSchemaSourceFromSettings(map[string]any{
-		"schema": 42,
+// TestTranslateLayerSettings_NonStringSchemaKey covers a layer
+// whose `schema:` is a non-string value: the rule's translator
+// still strips the legacy key but adds no schema-sources entry.
+func TestTranslateLayerSettings_NonStringSchemaKey(t *testing.T) {
+	out := translateLayerSettings("required-structure", RuleCfg{
+		Enabled:  true,
+		Settings: map[string]any{"schema": 42},
 	})
-	assert.Nil(t, src)
-	assert.True(t, hadKey)
+	assert.NotContains(t, out.Settings, "schema")
+	assert.NotContains(t, out.Settings, "schema-sources")
 }
 
-// TestExtractSchemaSourceFromSettings_NonMapInlineKey covers the
-// `m, ok := v.(map[string]any)` false branch: a non-map
-// `inline-schema` value is treated as empty.
-func TestExtractSchemaSourceFromSettings_NonMapInlineKey(t *testing.T) {
-	src, hadKey := extractSchemaSourceFromSettings(map[string]any{
-		"inline-schema": "not-a-map",
+// TestTranslateLayerSettings_NonMapInlineKey covers a non-map
+// `inline-schema` value: treated as empty, key stripped, no
+// source added.
+func TestTranslateLayerSettings_NonMapInlineKey(t *testing.T) {
+	out := translateLayerSettings("required-structure", RuleCfg{
+		Enabled:  true,
+		Settings: map[string]any{"inline-schema": "not-a-map"},
 	})
-	assert.Nil(t, src)
-	assert.True(t, hadKey)
+	assert.NotContains(t, out.Settings, "inline-schema")
+	assert.NotContains(t, out.Settings, "schema-sources")
+}
+
+// TestTranslateLayerSettings_UnknownRulePassthrough covers the
+// rule-not-registered / no-translator branches: a rule with no
+// SettingsTranslator (or an unknown name) returns its settings
+// untouched.
+func TestTranslateLayerSettings_UnknownRulePassthrough(t *testing.T) {
+	in := RuleCfg{Enabled: true, Settings: map[string]any{"schema": "x.md"}}
+	out := translateLayerSettings("definitely-not-a-rule", in)
+	assert.Equal(t, "x.md", out.Settings["schema"],
+		"unknown rule must not have its settings rewritten")
+}
+
+// TestTranslateLayerSettings_NilSettingsPassthrough covers the
+// rc.Settings == nil early return.
+func TestTranslateLayerSettings_NilSettingsPassthrough(t *testing.T) {
+	out := translateLayerSettings("required-structure",
+		RuleCfg{Enabled: true})
+	assert.Nil(t, out.Settings)
 }
 
 // TestKindLayerRules_InlineSchemaWithoutPathPattern covers the
@@ -320,12 +340,13 @@ func TestKindLayerRules_InlineSchemaWithoutPathPattern(t *testing.T) {
 	assert.Contains(t, sources[0].(map[string]any), "inline")
 }
 
-// TestTranslateSchemaSource_StripsLegacyKeyWithoutAddingSource
-// covers the `source != nil` false branch in translateSchemaSource:
-// a layer with `schema: ""` (default placeholder) is stripped of
-// the legacy key without contributing a schema-sources entry.
-func TestTranslateSchemaSource_StripsLegacyKeyWithoutAddingSource(t *testing.T) {
-	out := translateSchemaSource(RuleCfg{
+// TestTranslateLayerSettings_StripsLegacyKeyWithoutAddingSource
+// covers the empty-value path: a layer with `schema: ""` (the
+// rule's DefaultSettings placeholder) is stripped of the legacy
+// key without contributing a schema-sources entry, and unrelated
+// settings survive.
+func TestTranslateLayerSettings_StripsLegacyKeyWithoutAddingSource(t *testing.T) {
+	out := translateLayerSettings("required-structure", RuleCfg{
 		Enabled: true,
 		Settings: map[string]any{
 			"schema":       "",
@@ -340,26 +361,20 @@ func TestTranslateSchemaSource_StripsLegacyKeyWithoutAddingSource(t *testing.T)
 		"unrelated settings must survive translation")
 }
 
-// TestExtractSchemaSourceFromSettings_EmptyValuesStripped covers the
-// `hadKey` branch in extractSchemaSourceFromSettings: a settings map
-// that contains a schema key but with an empty value (empty string
-// or empty inline map) returns (nil, true) so callers strip the
-// key without adding a source entry.
-func TestExtractSchemaSourceFromSettings_EmptyValuesStripped(t *testing.T) {
-	src, hadKey := extractSchemaSourceFromSettings(map[string]any{
-		"schema":        "",
-		"inline-schema": map[string]any{},
+// TestTranslateLayerSettings_BothEmptyValuesStripped covers a
+// layer carrying both legacy keys with empty values: both keys
+// are stripped, no source is added.
+func TestTranslateLayerSettings_BothEmptyValuesStripped(t *testing.T) {
+	out := translateLayerSettings("required-structure", RuleCfg{
+		Enabled: true,
+		Settings: map[string]any{
+			"schema":        "",
+			"inline-schema": map[string]any{},
+		},
 	})
-	assert.Nil(t, src)
-	assert.True(t, hadKey)
-}
-
-// TestNeedsSchemaSourceTranslation_NilSettings covers the
-// rc.Settings == nil early-return branch in
-// needsSchemaSourceTranslation.
-func TestNeedsSchemaSourceTranslation_NilSettings(t *testing.T) {
-	rc := RuleCfg{Enabled: true, Settings: nil}
-	assert.False(t, needsSchemaSourceTranslation(rc))
+	assert.NotContains(t, out.Settings, "schema")
+	assert.NotContains(t, out.Settings, "inline-schema")
+	assert.NotContains(t, out.Settings, "schema-sources")
 }
 
 // TestEmptyInlineSchemaIsNoOp ensures the merge layer doesn't add a

internal/rule/rule.go

@@ -49,6 +49,25 @@ type ListMerger interface {
 	SettingMergeMode(key string) MergeMode
 }
 
+// SettingsTranslator is implemented by Configurable rules that
+// rewrite one config layer's settings map before the deep-merge
+// runs. The config merge layer calls TranslateLayerSettings on
+// every layer that configures the rule, so merge logic stays free
+// of rule-name special cases.
+//
+// required-structure implements this to collapse the user-facing
+// `schema:` / `inline-schema:` keys into an append-mode
+// `schema-sources` list, letting multiple kinds compose their
+// schemas instead of overwriting (plan 156).
+type SettingsTranslator interface {
+	// TranslateLayerSettings returns the settings the merge layer
+	// should use for one layer. Implementations must treat the
+	// input as read-only and return a new map when they change
+	// anything; returning the input unchanged signals "no
+	// translation applies".
+	TranslateLayerSettings(settings map[string]any) map[string]any
+}
+
 // ConfigTarget is implemented by rules that validate the project
 // config file (.mdsmith.yml) rather than individual Markdown files.
 // The engine runner runs these rules once against a synthetic lint.File