From ab5655514c78071f6e9fc64b39371f9947e41836 Mon Sep 17 00:00:00 2001 From: Matthew Edmondson Date: Mon, 28 Sep 2026 17:28:34 +0000 Subject: [PATCH] Merge keys that normalize alike under one name instead of swapping them [patch] FindBasicCanonicalName mapped each key in a normalize-alike class to another key in the class, so related went to page_related and page_related to related. Aggressive and Maximum then wrote each value under the other key's name and merged nothing. Every key in the class now maps to one name, chosen independently of key order: a known property first, then the shortest key, then ordinally. The partial-match pass picks between a pair the same way, Maximum no longer moves the chosen key elsewhere, and a key left alone in its group keeps its own name rather than taking another key's. Fixes #131 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015BJVibRvmTotkqEGp3695R --- Frontmatter.Test/EquivalentKeyMergeTests.cs | 68 ++++++++++++++++++ Frontmatter/PropertyMerger.cs | 76 +++++++++++++++++---- 2 files changed, 131 insertions(+), 13 deletions(-) create mode 100644 Frontmatter.Test/EquivalentKeyMergeTests.cs diff --git a/Frontmatter.Test/EquivalentKeyMergeTests.cs b/Frontmatter.Test/EquivalentKeyMergeTests.cs new file mode 100644 index 0000000..83d2fc5 --- /dev/null +++ b/Frontmatter.Test/EquivalentKeyMergeTests.cs @@ -0,0 +1,68 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.Frontmatter.Test; + +using System.Collections.Generic; +using System.Linq; + +using Microsoft.VisualStudio.TestTools.UnitTesting; + +/// +/// Regression tests for keys that normalize to the same name, such as related and +/// page_related. Each key used to be mapped to the other key's name, so the two were +/// swapped rather than merged. +/// +[TestClass] +public class EquivalentKeyMergeTests +{ + private static readonly string[] ListA = ["a"]; + private static readonly string[] ListB = ["b"]; + + [TestMethod] + [DataRow(FrontmatterMergeStrategy.Aggressive, false)] + [DataRow(FrontmatterMergeStrategy.Aggressive, true)] + [DataRow(FrontmatterMergeStrategy.Maximum, false)] + [DataRow(FrontmatterMergeStrategy.Maximum, true)] + public void EquivalentListKeys_MergeUnderOneNameWithBothValues(FrontmatterMergeStrategy strategy, bool prefixedFirst) + { + Dictionary frontmatter = prefixedFirst + ? new() { ["page_related"] = ListB, ["related"] = ListA } + : new() { ["related"] = ListA, ["page_related"] = ListB }; + + Dictionary result = PropertyMerger.MergeSimilarProperties(frontmatter, strategy); + + Assert.HasCount(1, result); + Assert.IsTrue(result.TryGetValue("related", out object? related), "The unprefixed key should name the merged property"); + CollectionAssert.AreEquivalent(new object[] { "a", "b" }, ((IEnumerable)related).ToArray()); + } + + [TestMethod] + [DataRow(FrontmatterMergeStrategy.Aggressive, false)] + [DataRow(FrontmatterMergeStrategy.Aggressive, true)] + [DataRow(FrontmatterMergeStrategy.Maximum, false)] + [DataRow(FrontmatterMergeStrategy.Maximum, true)] + public void EquivalentScalarKeys_MergeIntoOneProperty(FrontmatterMergeStrategy strategy, bool prefixedFirst) + { + Dictionary frontmatter = prefixedFirst + ? new() { ["page_related"] = "y", ["related"] = "x" } + : new() { ["related"] = "x", ["page_related"] = "y" }; + + Dictionary result = PropertyMerger.MergeSimilarProperties(frontmatter, strategy); + + Assert.HasCount(1, result, "Two scalars that normalize alike should merge into one property"); + } + + [TestMethod] + public void CombineFrontmatter_Aggressive_MergesRelatedListsInsteadOfSwappingThem() + { + const string input = "---\nrelated:\n- a\npage_related:\n- b\n---\nBody\n"; + + string result = Frontmatter.CombineFrontmatter(input, FrontmatterNaming.AsIs, FrontmatterOrder.AsIs, FrontmatterMergeStrategy.Aggressive); + + Dictionary? frontmatter = Frontmatter.ExtractFrontmatter(result); + Assert.IsNotNull(frontmatter); + Assert.HasCount(1, frontmatter); + Assert.IsTrue(frontmatter.TryGetValue("related", out object? related), "The merged list should be written under related"); + CollectionAssert.AreEquivalent(new object[] { "a", "b" }, ((IEnumerable)related).ToArray()); + } +} diff --git a/Frontmatter/PropertyMerger.cs b/Frontmatter/PropertyMerger.cs index 323059a..baa50d2 100644 --- a/Frontmatter/PropertyMerger.cs +++ b/Frontmatter/PropertyMerger.cs @@ -3,6 +3,7 @@ namespace ktsu.Frontmatter; using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; using System.Linq; /// @@ -40,9 +41,14 @@ internal static Dictionary MergeSimilarProperties(Dictionary group in propertyMappings.GroupBy(x => x.Value, x => x.Key)) { - string canonicalKey = group.Key; List originalKeys = [.. group]; + // A key with nothing to merge with keeps its own name, so no key is ever renamed onto + // another key's name without the two actually being merged. + string canonicalKey = originalKeys.Count == 1 && frontmatter.ContainsKey(group.Key) && group.Key != originalKeys[0] + ? originalKeys[0] + : group.Key; + MergePropertyGroup(frontmatter, mergedFrontmatter, canonicalKey, originalKeys); } @@ -203,19 +209,9 @@ private static string FindBasicCanonicalName(string key, string[] existingKeys) } // Look for exact matches after normalization - foreach (string existingKey in existingKeys) + if (TryFindEquivalenceClassName(key, normalizedKey, existingKeys, out string? className)) { - if (existingKey == key) - { - continue; - } - - string normalizedExisting = NormalizePropertyName(existingKey); - if (string.Equals(normalizedKey, normalizedExisting, StringComparison.OrdinalIgnoreCase)) - { - // If the existing key is a known property, use its canonical name - return PropertyMappings.All.TryGetValue(existingKey, out string? knownName) ? knownName : existingKey; - } + return className; } // Look for partial matches @@ -235,6 +231,13 @@ private static string FindBasicCanonicalName(string key, string[] existingKeys) if (normalizedKey.Contains(normalizedExisting2, StringComparison.OrdinalIgnoreCase) || normalizedExisting2.Contains(normalizedKey, StringComparison.OrdinalIgnoreCase)) { + // Both keys of a pair must agree on one name, or each would be renamed to the other. + string preferred = PreferredName([key, existingKey]); + if (preferred == key) + { + continue; + } + // If the existing key is a known property, use its canonical name return PropertyMappings.All.TryGetValue(existingKey, out string? knownName2) ? knownName2 : existingKey; } @@ -243,6 +246,46 @@ private static string FindBasicCanonicalName(string key, string[] existingKeys) return key; } + /// + /// Finds the one name shared by every key that normalizes the same as . + /// + /// The key to analyze. + /// The normalized form of . + /// All existing keys in the frontmatter. + /// The name every key in the class maps to. + /// True if at least one other key normalizes the same as . + private static bool TryFindEquivalenceClassName(string key, string normalizedKey, string[] existingKeys, [NotNullWhen(true)] out string? className) + { + List equivalenceClass = [key]; + equivalenceClass.AddRange(existingKeys.Where(existingKey => existingKey != key && + string.Equals(normalizedKey, NormalizePropertyName(existingKey), StringComparison.OrdinalIgnoreCase))); + + if (equivalenceClass.Count == 1) + { + className = null; + return false; + } + + // Every key in the class must reach the same name, whichever of them is asking. Mapping each + // key to some other key instead sent related to page_related and page_related to related, + // which swapped their values rather than merging them. + string preferred = PreferredName(equivalenceClass); + className = PropertyMappings.All.TryGetValue(preferred, out string? knownName) ? knownName : preferred; + return true; + } + + /// + /// Picks one name from a set of keys, independently of their order: a known property first, + /// then the shortest key, with any tie broken ordinally. + /// + /// The keys to choose between. + /// The preferred key. + private static string PreferredName(IEnumerable keys) => keys + .OrderBy(k => PropertyMappings.All.ContainsKey(k) ? 0 : 1) + .ThenBy(k => k.Length) + .ThenBy(k => k, StringComparer.Ordinal) + .First(); + /// /// Attempts to find a canonical name for a property key using semantic analysis. /// @@ -258,6 +301,13 @@ private static string FindSemanticCanonicalName(string key, string[] existingKey return basicMatch; } + // The key is the name its equivalence class merges under, so it must not move elsewhere. + string normalizedKey = NormalizePropertyName(key); + if (normalizedKey.Length > 0 && TryFindEquivalenceClassName(key, normalizedKey, existingKeys, out _)) + { + return key; + } + // Then try more aggressive matching using word similarity string[] keyWords = PropertyNameNormalizer.NormalizeToWords(key);