Skip to main content

max / makenotwork

tagtree: fix merge_tags orphaning descendants merge_tags moved only the exact source tag, so descendants like genre.electronic.house dangled after merging genre.electronic into genre.dance. Delegate to rename_prefix_bulk so the whole subtree's prefix is rewritten and colliding tags dedupe on rebuild. Correct the doc (it claimed "all occurrences" while doing the opposite), rewrite the doctest to exercise a descendant, and add subtree + cross-subtree-dedup tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-07-15 18:10 UTC
Commit: 7f9475adbdef3710e7949760f2320d1b725d9667
Parent: 7845654
1 file changed, +48 insertions, -13 deletions
M shared/tagtree/src/lib.rs +48 -13
@@ -625,29 +625,26 @@ pub fn remove_subtree(prefix: &str, index: &mut TagIndex) -> usize {
625 625 removed
626 626 }
627 627
628 - /// Merge one tag into another: all occurrences of `source` become `target`.
629 - /// If `target` already exists, `source` is removed (deduplication).
630 - /// Returns the number of tags affected (0 or 1).
628 + /// Merge one tag subtree into another: `source` and every descendant have their
629 + /// `source` prefix rewritten to `target`, so `genre.electronic` and
630 + /// `genre.electronic.house` become `genre.dance` and `genre.dance.house`. Any
631 + /// resulting tag that collides with one already under `target` is deduplicated.
632 + /// Returns the number of tags moved.
631 633 ///
632 634 /// ```
633 635 /// # use tagtree::{TagIndex, merge_tags};
634 636 /// let mut idx = TagIndex::new(vec![
635 637 /// "genre.electronic".into(),
638 + /// "genre.electronic.house".into(),
636 639 /// "genre.dance".into(),
637 640 /// ]);
638 - /// assert_eq!(merge_tags("genre.electronic", "genre.dance", &mut idx), 1);
639 - /// assert_eq!(idx.len(), 1);
641 + /// assert_eq!(merge_tags("genre.electronic", "genre.dance", &mut idx), 2);
640 642 /// assert!(idx.contains("genre.dance"));
643 + /// assert!(idx.contains("genre.dance.house"));
644 + /// assert!(!idx.contains("genre.electronic.house"));
641 645 /// ```
642 646 pub fn merge_tags(source: &str, target: &str, index: &mut TagIndex) -> usize {
643 - if !index.contains(source) {
644 - return 0;
645 - }
646 - index.remove(source);
647 - if !index.contains(target) {
648 - index.insert(target.to_string());
649 - }
650 - 1
647 + rename_prefix_bulk(source, target, index)
651 648 }
652 649
653 650 /// Apply a batch of prefix-rename operations in order. Returns total tags modified.
@@ -1876,6 +1873,44 @@ mod tests {
1876 1873 assert_eq!(idx.len(), 1);
1877 1874 }
1878 1875
1876 + #[test]
1877 + fn merge_tags_carries_descendants() {
1878 + // The subtree, not just the exact tag, must move — descendants of
1879 + // `source` were orphaned before the fuzz-1 fix (merge_tags-orphans).
1880 + let mut idx = TagIndex::new(vec![
1881 + "genre.electronic".into(),
1882 + "genre.electronic.house".into(),
1883 + "genre.electronic.techno".into(),
1884 + "genre.rock".into(),
1885 + ]);
1886 + assert_eq!(merge_tags("genre.electronic", "genre.dance", &mut idx), 3);
1887 + assert!(idx.contains("genre.dance"));
1888 + assert!(idx.contains("genre.dance.house"));
1889 + assert!(idx.contains("genre.dance.techno"));
1890 + assert!(idx.contains("genre.rock"));
1891 + assert!(!idx.contains("genre.electronic"));
1892 + assert!(!idx.contains("genre.electronic.house"));
1893 + assert!(!idx.contains("genre.electronic.techno"));
1894 + }
1895 +
1896 + #[test]
1897 + fn merge_tags_deduplicates_across_subtrees() {
1898 + // A descendant that collides with an existing tag under `target`
1899 + // collapses rather than duplicating.
1900 + let mut idx = TagIndex::new(vec![
1901 + "genre.electronic".into(),
1902 + "genre.electronic.house".into(),
1903 + "genre.dance".into(),
1904 + "genre.dance.house".into(),
1905 + ]);
1906 + // Both source-subtree tags are rewritten; the resulting
1907 + // `genre.dance` / `genre.dance.house` dedupe against the existing pair.
1908 + assert_eq!(merge_tags("genre.electronic", "genre.dance", &mut idx), 2);
1909 + assert_eq!(idx.len(), 2);
1910 + assert!(idx.contains("genre.dance"));
1911 + assert!(idx.contains("genre.dance.house"));
1912 + }
1913 +
1879 1914 // --- batch_rename ---
1880 1915
1881 1916 #[test]