Skip to main content

max / makenotwork

Tighten empty-input contracts on the tag-path helpers prefix_at_depth("", n) returned Some("") while its doc promised None for tags with fewer than n segments, and depth("") is already 0. semantic_prefix inherited the bug by delegation. Return None instead. subtree("") returned [] while children_at_prefix documents "" as the root and returns root children. Read "" as the root in both. batch_rename's count is the sum of each operation's count, so a tag renamed twice counts twice; the doc said "total tags modified", which reads as distinct tags. Reword and pin the chained case. Tests cover each edge; the two behavior changes fail against the prior implementations. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-07-17 02:45 UTC
Commit: de678b51b48f862bce781926b354503c8b93610e
Parent: 2b7defc
1 file changed, +74 insertions, -3 deletions
M shared/tagtree/src/lib.rs +74 -3
@@ -251,9 +251,12 @@ pub fn segment(tag: &str, index: usize) -> Option<&str> {
251 251 /// assert_eq!(prefix_at_depth("genre.electronic.house", 2), Some("genre.electronic"));
252 252 /// assert_eq!(prefix_at_depth("genre.electronic.house", 3), Some("genre.electronic.house"));
253 253 /// assert_eq!(prefix_at_depth("genre.electronic.house", 4), None);
254 + ///
255 + /// // The empty tag has no segments (`depth("") == 0`), so no prefix exists at any depth.
256 + /// assert_eq!(prefix_at_depth("", 1), None);
254 257 /// ```
255 258 pub fn prefix_at_depth(tag: &str, n: usize) -> Option<&str> {
256 - if n == 0 {
259 + if n == 0 || tag.is_empty() {
257 260 return None;
258 261 }
259 262 let mut dots_seen = 0;
@@ -363,6 +366,9 @@ pub fn common_ancestor<'a>(a: &'a str, b: &str) -> Option<&'a str> {
363 366 ///
364 367 /// // No semantic prefix
365 368 /// assert_eq!(semantic_prefix("rock", 0), None);
369 + ///
370 + /// // The empty tag has no segments to take a prefix from.
371 + /// assert_eq!(semantic_prefix("", 1), None);
366 372 /// ```
367 373 pub fn semantic_prefix(tag: &str, semantic_depth: usize) -> Option<&str> {
368 374 if semantic_depth == 0 {
@@ -451,17 +457,24 @@ pub fn children_at_prefix(prefix: &str, tags: &[impl AsRef<str>]) -> Vec<String>
451 457
452 458 /// Get all tags that are descendants of a prefix (including the prefix itself if present).
453 459 ///
460 + /// An empty prefix means the root, so every tag is a descendant. This matches
461 + /// [`children_at_prefix`], which also reads `""` as the root.
462 + ///
454 463 /// ```
455 464 /// # use tagtree::subtree;
456 465 /// let tags = ["genre.electronic.house", "genre.electronic.techno", "genre.rock", "mood.dark"];
457 466 /// let sub = subtree("genre.electronic", &tags);
458 467 /// assert_eq!(sub, vec!["genre.electronic.house", "genre.electronic.techno"]);
468 + ///
469 + /// // Root: every tag.
470 + /// assert_eq!(subtree("", &tags), tags.to_vec());
459 471 /// ```
460 472 pub fn subtree<'a>(prefix: &str, tags: &'a [impl AsRef<str>]) -> Vec<&'a str> {
461 473 tags.iter()
462 474 .map(|t| t.as_ref())
463 475 .filter(|tag| {
464 - *tag == prefix
476 + prefix.is_empty()
477 + || *tag == prefix
465 478 || (tag.starts_with(prefix)
466 479 && tag.len() > prefix.len()
467 480 && tag.as_bytes()[prefix.len()] == SEPARATOR_BYTE)
@@ -656,7 +669,10 @@ pub fn merge_tags(source: &str, target: &str, index: &mut TagIndex) -> usize {
656 669 rename_prefix_bulk(source, target, index)
657 670 }
658 671
659 - /// Apply a batch of prefix-rename operations in order. Returns total tags modified.
672 + /// Apply a batch of prefix-rename operations in order.
673 + ///
674 + /// Returns the sum of each operation's modified-tag count, so a tag that a later
675 + /// operation renames again counts once per operation rather than once overall.
660 676 ///
661 677 /// ```
662 678 /// # use tagtree::{TagIndex, batch_rename};
@@ -670,6 +686,16 @@ pub fn merge_tags(source: &str, target: &str, index: &mut TagIndex) -> usize {
670 686 /// assert!(idx.contains("genre.dance.house"));
671 687 /// assert!(idx.contains("vibe.dark"));
672 688 /// ```
689 + ///
690 + /// Chained operations count per operation:
691 + ///
692 + /// ```
693 + /// # use tagtree::{TagIndex, batch_rename};
694 + /// let mut idx = TagIndex::new(vec!["a.x".into()]);
695 + /// // `a.x` becomes `b.x`, then `c.x`: one tag, but two operations touched it.
696 + /// assert_eq!(batch_rename(&[("a", "b"), ("b", "c")], &mut idx), 2);
697 + /// assert!(idx.contains("c.x"));
698 + /// ```
673 699 pub fn batch_rename(operations: &[(&str, &str)], index: &mut TagIndex) -> usize {
674 700 let mut total = 0;
675 701 for &(old, new) in operations {
@@ -1306,6 +1332,51 @@ mod tests {
1306 1332 }
1307 1333
1308 1334 #[test]
1335 + fn empty_prefix_is_root_for_both_subtree_and_children() {
1336 + let tags = [
1337 + "genre.electronic.house",
1338 + "genre.rock",
1339 + "mood.dark",
1340 + "instrument.kick",
1341 + ];
1342 + // The root prefix yields every tag, and agrees with children_at_prefix's
1343 + // reading of "" as the root.
1344 + assert_eq!(subtree("", &tags), tags.to_vec());
1345 + assert_eq!(
1346 + children_at_prefix("", &tags),
1347 + vec!["genre", "instrument", "mood"]
1348 + );
1349 + assert_eq!(subtree("", &[] as &[&str]), Vec::<&str>::new());
1350 + }
1351 +
1352 + #[test]
1353 + fn empty_tag_has_no_prefix_at_any_depth() {
1354 + // depth("") == 0, so no depth n >= 1 can yield a prefix.
1355 + assert_eq!(depth(""), 0);
1356 + for n in 1..4 {
1357 + assert_eq!(prefix_at_depth("", n), None, "prefix_at_depth(\"\", {n})");
1358 + assert_eq!(semantic_prefix("", n), None, "semantic_prefix(\"\", {n})");
1359 + }
1360 + assert_eq!(prefix_at_depth("", 0), None);
1361 + assert_eq!(semantic_prefix("", 0), None);
1362 + }
1363 +
1364 + #[test]
1365 + fn batch_rename_counts_each_operation_separately() {
1366 + // One tag, two operations touching it: the count is per-operation, not distinct tags.
1367 + let mut idx = TagIndex::new(vec!["a.x".into()]);
1368 + assert_eq!(batch_rename(&[("a", "b"), ("b", "c")], &mut idx), 2);
1369 + assert!(idx.contains("c.x"));
1370 + assert!(!idx.contains("a.x"));
1371 + assert!(!idx.contains("b.x"));
1372 +
1373 + // A no-op operation contributes nothing.
1374 + let mut idx = TagIndex::new(vec!["a.x".into()]);
1375 + assert_eq!(batch_rename(&[("nope", "other")], &mut idx), 0);
1376 + assert!(idx.contains("a.x"));
1377 + }
1378 +
1379 + #[test]
1309 1380 fn children_one_level() {
1310 1381 let tags = [
1311 1382 "genre.electronic.house",