add tiebreaker logic to pattern specificity

Author: CarsonDavis

sde_collections/models/delta_patterns.py

@@ -50,11 +50,19 @@ def get_url_match_count(self):
 
     def is_most_distinctive_pattern(self, url) -> bool:
         """
-        Determine if this pattern should apply to a URL by checking if it matches
-        the smallest number of URLs among all patterns that match this URL.
+        Determine if this pattern should apply to a URL by checking:
+        1. First checks if this pattern matches this URL
+        2. If it matches the smallest number of URLs among all patterns that match this URL
+        3. If tied for smallest number of matches, uses the longest pattern string
         Returns True if this pattern should be applied.
         """
+        # First check if this pattern matches the URL
+        regex_pattern = self.get_regex_pattern()
+        if not re.search(regex_pattern, url.url):
+            return False
+
         my_match_count = self.get_url_match_count()
+        my_pattern_length = len(self.match_pattern)
 
         # Get patterns from same type that affect this URL
         pattern_class = self.__class__
@@ -63,12 +71,19 @@ def is_most_distinctive_pattern(self, url) -> bool:
             .filter(models.Q(delta_urls__url=url.url) | models.Q(curated_urls__url=url.url))
             .exclude(id=self.id)
             .distinct()
-        )  # TODO: does this have a distinct urls, or distinct model objects.
+        )
 
-        # If any matching pattern has a smaller URL set, don't apply
+        # Use M2M relationships for checking other patterns since those are already established
         for pattern in matching_patterns:
-            if pattern.get_url_match_count() < my_match_count:
+            other_match_count = pattern.get_url_match_count()
+            if other_match_count < my_match_count:
+                # Other pattern matches fewer URLs - definitely not most distinctive
                 return False
+            if other_match_count == my_match_count:
+                # Same match count - check pattern length
+                if len(pattern.match_pattern) > my_pattern_length:
+                    # Other pattern is longer - not most distinctive
+                    return False
 
         return True
 

sde_collections/tests/test_delta_patterns.py

@@ -283,7 +283,6 @@ def test_title_pattern_error_updates(self):
         general_pattern = DeltaTitlePattern.objects.create(
             collection=collection,
             match_pattern="*docs*",
-            # Use a different error-causing pattern
             title_pattern="{invalid}",  # Invalid variable name will cause error
             match_pattern_type=2,
         )
@@ -297,14 +296,17 @@ def test_title_pattern_error_updates(self):
         specific_pattern = DeltaTitlePattern.objects.create(
             collection=collection,
             match_pattern="*docs/specific*",
-            # Different invalid variable
             title_pattern="{another_invalid}",
             match_pattern_type=2,
         )
 
+        # Re-fetch error to see latest state
+        error.refresh_from_db()
+
         # Error should now be from specific pattern
-        error = url.deltaresolvedtitleerror  # Should still only be one
-        assert error.title_pattern == specific_pattern
+        assert (
+            error.title_pattern == specific_pattern
+        ), f"Error still associated with {error.title_pattern} instead of {specific_pattern}"
         assert "Variable 'another_invalid' not allowed in f-string pattern" in error.error_string
 
         # Verify we still only have one error record

sde_collections/tests/test_pattern_specificity.py

@@ -156,3 +156,65 @@ def test_field_modifying_pattern_layered_specificity():
     assert mid_tool.pk in broad_pattern.delta_urls.values_list("pk", flat=True)
 
     assert top_tool.pk in broad_pattern.delta_urls.values_list("pk", flat=True)
+
+
+@pytest.mark.django_db
+def test_pattern_specificity_tiebreaker():
+    """Test that when patterns match the same number of URLs, longer patterns are considered more specific."""
+    collection = CollectionFactory()
+
+    # Create URLs that would result in same match count for different patterns
+    url1 = DeltaUrlFactory(
+        collection=collection, url="https://example.com/docs/specific/item1.html", scraped_title="Title 1"
+    )
+    url2 = DeltaUrlFactory(
+        collection=collection, url="https://example.com/docs/specific/item2.html", scraped_title="Title 2"
+    )
+
+    # Create patterns with same match count but different lengths
+    general_pattern = DeltaTitlePattern.objects.create(
+        collection=collection,
+        match_pattern="*docs*",  # Shorter pattern
+        title_pattern="{title}",
+        match_pattern_type=2,
+    )
+
+    specific_pattern = DeltaTitlePattern.objects.create(
+        collection=collection,
+        match_pattern="*docs/specific*",  # Longer pattern
+        title_pattern="{title} - Specific",
+        match_pattern_type=2,
+    )
+
+    # Both patterns will match both URLs (same match count)
+    assert general_pattern.get_url_match_count() == 2
+    assert specific_pattern.get_url_match_count() == 2
+
+    # But the longer pattern should be considered more specific
+    assert general_pattern.is_most_distinctive_pattern(url1) is False
+    assert specific_pattern.is_most_distinctive_pattern(url1) is True
+
+    # Check that this applies to both URLs
+    assert general_pattern.is_most_distinctive_pattern(url2) is False
+    assert specific_pattern.is_most_distinctive_pattern(url2) is True
+
+    # Create an even more specific pattern
+    very_specific_pattern = DeltaTitlePattern.objects.create(
+        collection=collection,
+        match_pattern="*docs/specific/item1*",  # Even longer pattern
+        title_pattern="{title} - Very Specific",
+        match_pattern_type=2,
+    )
+
+    # It matches fewer URLs
+    assert very_specific_pattern.get_url_match_count() == 1
+
+    # For URL1, the very specific pattern should win due to fewer matches
+    assert general_pattern.is_most_distinctive_pattern(url1) is False
+    assert specific_pattern.is_most_distinctive_pattern(url1) is False
+    assert very_specific_pattern.is_most_distinctive_pattern(url1) is True
+
+    # For URL2, the middle pattern should still win since very_specific doesn't match
+    assert general_pattern.is_most_distinctive_pattern(url2) is False
+    assert specific_pattern.is_most_distinctive_pattern(url2) is True
+    assert very_specific_pattern.is_most_distinctive_pattern(url2) is False