decorator filtering working: Strip ANSI escape codes from Griffe paths

Signed-off-by: Pablo Garay <[email protected]>

Author: pablo-garay

scripts/check_api_backwards_compatibility.py

@@ -11,6 +11,7 @@
 
 import argparse
 import os
+import re
 import sys
 from collections import Counter
 
@@ -40,19 +41,36 @@ def has_exempt_decorator(obj: Object) -> bool:
     """Check if object has any exempt decorator."""
     if not hasattr(obj, 'decorators'):
         return False
+    if not obj.decorators:
+        return False
     for decorator in obj.decorators:
-        dec_str = str(decorator)
-        if any(exempt in dec_str for exempt in EXEMPT_DECORATORS):
+        # Get the actual decorator name from the value attribute
+        dec_value = str(getattr(decorator, 'value', ''))
+        if any(exempt in dec_value for exempt in EXEMPT_DECORATORS):
             return True
     return False
 
 
-def get_filtered_paths(package: Object) -> set:
+def get_filtered_paths(package: Object, package_name: str) -> set:
     """Get all paths that should be filtered (have exempt decorators)."""
     filtered = set()
+    visited = set()
 
-    def visit(obj, path):
-        current_path = f"{path}.{obj.name}" if path else obj.name
+    def visit(obj, path, depth=0, is_root=False):
+        # Prevent infinite recursion
+        if depth > 20 or id(obj) in visited:
+            return
+        visited.add(id(obj))
+        
+        # For root object, use the provided path; for children, append obj.name
+        if is_root:
+            current_path = path
+        else:
+            current_path = f"{path}.{obj.name}" if path else obj.name
+        
+        # Skip aliases (imported objects)
+        if hasattr(obj, 'is_alias') and obj.is_alias:
+            return
 
         # Skip private members
         if obj.name.startswith('_') and not obj.name.startswith('__'):
@@ -66,12 +84,22 @@ def visit(obj, path):
         # Visit children
         if hasattr(obj, 'members'):
             for member in obj.members.values():
-                visit(member, current_path)
+                visit(member, current_path, depth + 1, is_root=False)
 
-    visit(package, "")
+    # Start with the full package name (e.g., "megatron.core")
+    visit(package, package_name, is_root=True)
     return filtered
 
 
+def strip_ansi_codes(text):
+    """Remove ANSI escape codes from text."""
+    if not text:
+        return text
+    # Pattern to match ANSI escape codes
+    ansi_escape = re.compile(r'\x1b\[[0-9;]*m')
+    return ansi_escape.sub('', text)
+
+
 def get_object_path(change) -> str:
     """Extract the object path from a breaking change."""
     # Try different attributes
@@ -80,33 +108,90 @@ def get_object_path(change) -> str:
             getattr(change, 'path', None))
 
     if path:
-        return path
+        return strip_ansi_codes(path)
 
     # Try from values
     if hasattr(change, 'new_value') and change.new_value:
-        return getattr(change.new_value, 'path', None)
+        path = getattr(change.new_value, 'path', None)
+        if path:
+            return strip_ansi_codes(path)
+    
     if hasattr(change, 'old_value') and change.old_value:
-        return getattr(change.old_value, 'path', None)
+        path = getattr(change.old_value, 'path', None)
+        if path:
+            return strip_ansi_codes(path)
+    
+    # Last resort: parse from explanation
+    # Format: "filepath:line: object_path: description"
+    # Example: "megatron/core/model_parallel_config.py:338: ModelParallelConfig.cpu_offloading_weights: Attribute value was changed"
+    try:
+        explanation = change.explain()
+        # Split by ": " and get the second part (object path)
+        parts = explanation.split(': ')
+        if len(parts) >= 2:
+            # Get the part after "filepath:line" but before the description
+            # It's usually the second part
+            object_path = parts[1]
+            
+            # Extract the module path from file path (first part)
+            file_part = parts[0].split(':')[0]  # Get just the file path, remove line number
+            
+            # Convert file path to module path
+            # e.g., "megatron/core/model_parallel_config.py" -> "megatron.core.model_parallel_config"
+            module_path = file_part.replace('/', '.').replace('\\', '.').replace('.py', '')
+            
+            # If object_path doesn't start with module, prepend it
+            if not object_path.startswith(module_path):
+                full_path = f"{module_path}.{object_path}"
+            else:
+                full_path = object_path
+            
+            return strip_ansi_codes(full_path)
+    except Exception:
+        pass
 
     return None
 
 
-def should_skip_change(change, filtered_paths: set) -> bool:
+def should_skip_change(change, filtered_paths: set, debug=False) -> bool:
     """Check if a breaking change should be skipped based on filters."""
     path = get_object_path(change)
     if not path:
         return False
 
+    # Strip parameter names from path for matching
+    # e.g., "Class.__init__(param)" -> "Class.__init__"
+    clean_path = path.split('(')[0] if '(' in path else path
+    
+    # Debug specific paths
+    if debug and ('ModelParallelConfig' in path or 'DistributedDataParallel' in path):
+        print(f"\n  🔍 DEBUG matching:", file=sys.stderr)
+        print(f"     Original path: {path}", file=sys.stderr)
+        print(f"     Clean path: {clean_path}", file=sys.stderr)
+        print(f"     Filtered paths: {filtered_paths}", file=sys.stderr)
+    
     # Check exact match
-    if path in filtered_paths:
+    if clean_path in filtered_paths or path in filtered_paths:
+        if debug and ('ModelParallelConfig' in path or 'DistributedDataParallel' in path):
+            print(f"     ✓ EXACT MATCH!", file=sys.stderr)
         return True
 
     # Check if it's a child of a filtered object
-    # e.g., MyClass.__init__ is child of MyClass
+    # e.g., MyClass.__init__ is child of MyClass, MyClass.attr is child of MyClass
     for filtered_path in filtered_paths:
-        if path.startswith(filtered_path + '.') or path.startswith(filtered_path + '('):
+        if clean_path.startswith(filtered_path + '.'):
+            if debug and ('ModelParallelConfig' in path or 'DistributedDataParallel' in path):
+                print(f"     ✓ CHILD MATCH with: {filtered_path}", file=sys.stderr)
+            return True
+        # Also check the original path in case parameter names matter
+        if path.startswith(filtered_path + '.'):
+            if debug and ('ModelParallelConfig' in path or 'DistributedDataParallel' in path):
+                print(f"     ✓ CHILD MATCH (orig) with: {filtered_path}", file=sys.stderr)
             return True
 
+    if debug and ('ModelParallelConfig' in path or 'DistributedDataParallel' in path):
+        print(f"     ✗ NO MATCH", file=sys.stderr)
+    
     return False
 
 
@@ -118,9 +203,6 @@ def main():
     parser.add_argument('--verbose', '-v', action='store_true', help='Verbose output')
     args = parser.parse_args()
 
-    # Clean up stale worktrees before starting
-    cleanup_git_worktrees()
-    
     try:
         package_name = args.package
 
@@ -147,24 +229,46 @@ def main():
 
         # Get filtered paths from CURRENT version only
         print(f"\n🔍 Finding exempt objects in current version...", file=sys.stderr)
-        filtered_paths = get_filtered_paths(current)
+        filtered_paths = get_filtered_paths(current, package_name)
         print(f"   Found {len(filtered_paths)} exempt objects", file=sys.stderr)
 
         # Find breaking changes
         print(f"\n🔍 Comparing versions...", file=sys.stderr)
         all_changes = list(griffe.find_breaking_changes(baseline, current))
         print(f"   Found {len(all_changes)} potential breaking changes", file=sys.stderr)
 
-        # Filter out exempt changes
+        # Filter out exempt changes  
         breaking_changes = []
         skipped_count = 0
+        
+        # DEBUG: Print first 5 breaking changes to STDOUT for debugging
+        print("\n===TEST DEBUG (first 5 changes)===", flush=True)
+        print(f"Filtered paths: {filtered_paths}", flush=True)
+        for i, change in enumerate(all_changes[:5]):
+            path = get_object_path(change)
+            clean_path = path.split('(')[0] if path and '(' in path else path
+            print(f"\nChange {i+1}: {path}", flush=True)
+            print(f"  Clean: {clean_path}", flush=True)
+            print(f"  Clean repr: {repr(clean_path)}", flush=True)
+            
+            # Test matching
+            matched = False
+            for fpath in filtered_paths:
+                if clean_path and (clean_path == fpath or clean_path.startswith(fpath + '.')):
+                    print(f"  ✓ MATCH with: {fpath}", flush=True)
+                    matched = True
+                    break
+            if not matched:
+                print(f"  ✗ NO MATCH", flush=True)
+        print("\n===END TEST DEBUG===\n", flush=True)
+        
         for change in all_changes:
             if should_skip_change(change, filtered_paths):
                 skipped_count += 1
             else:
                 breaking_changes.append(change)
 
-        print(f"   Skipped {skipped_count} exempt | Reporting {len(breaking_changes)} breaking changes", file=sys.stderr)
+        print(f"\n   Skipped {skipped_count} exempt | Reporting {len(breaking_changes)} breaking changes", file=sys.stderr)
 
         # Print results
         if not breaking_changes: