ref(grouping): Tweak enhancement naming and documentation (#87363)

This is a bunch of renaming and comment-adding that I've done as part of my travels through the enhancements code, both recently and last fall. No behavior changes.

Author: lobsterkatie

src/sentry/grouping/enhancer/__init__.py

@@ -190,14 +190,14 @@ def assemble_stacktrace_component(
         """
         match_frames = [create_match_frame(frame, platform) for frame in frames]
 
-        rust_components = [RustComponent(contributes=c.contributes) for c in frame_components]
+        rust_frame_components = [RustComponent(contributes=c.contributes) for c in frame_components]
 
         # Modify the rust components by applying +group/-group rules and getting hints for both
         # those changes and the `in_app` changes applied by earlier in the ingestion process by
         # `apply_category_and_updated_in_app_to_frames`. Also, get `hint` and `contributes` values
         # for the overall stacktrace (returned in `rust_results`).
         rust_results = self.rust_enhancements.assemble_stacktrace_component(
-            match_frames, make_rust_exception_data(exception_data), rust_components
+            match_frames, make_rust_exception_data(exception_data), rust_frame_components
         )
 
         # Tally the number of each type of frame in the stacktrace. Later on, this will allow us to
@@ -206,7 +206,7 @@ def assemble_stacktrace_component(
         frame_counts: Counter[str] = Counter()
 
         # Update frame components with results from rust
-        for py_component, rust_component in zip(frame_components, rust_components):
+        for py_component, rust_component in zip(frame_components, rust_frame_components):
             # TODO: Remove the first condition once we get rid of the legacy config
             if (
                 not (self.bases and self.bases[0].startswith("legacy"))
@@ -293,7 +293,7 @@ def _to_config_structure(self):
         return [
             self.version,
             self.bases,
-            [x._to_config_structure(self.version) for x in self.rules],
+            [rule._to_config_structure(self.version) for rule in self.rules],
         ]
 
     def dumps(self) -> str:
@@ -307,7 +307,7 @@ def _from_config_structure(cls, data, rust_enhancements: RustEnhancements) -> En
         if version not in VERSIONS:
             raise ValueError("Unknown version")
         return cls(
-            rules=[EnhancementRule._from_config_structure(x, version=version) for x in rules],
+            rules=[EnhancementRule._from_config_structure(rule, version=version) for rule in rules],
             rust_enhancements=rust_enhancements,
             version=version,
             bases=bases,
@@ -348,17 +348,17 @@ def from_config_string(cls, s, bases=None, id=None) -> Enhancements:
 
 
 def _load_configs() -> dict[str, Enhancements]:
-    rv = {}
-    base = os.path.join(os.path.abspath(os.path.dirname(__file__)), "enhancement-configs")
-    for fn in os.listdir(base):
-        if fn.endswith(".txt"):
-            with open(os.path.join(base, fn), encoding="utf-8") as f:
+    enhancement_bases = {}
+    configs_dir = os.path.join(os.path.abspath(os.path.dirname(__file__)), "enhancement-configs")
+    for filename in os.listdir(configs_dir):
+        if filename.endswith(".txt"):
+            with open(os.path.join(configs_dir, filename), encoding="utf-8") as f:
                 # We cannot use `:` in filenames on Windows but we already have ids with
                 # `:` in their names hence this trickery.
-                fn = fn.replace("@", ":")
-                enhancements = Enhancements.from_config_string(f.read(), id=fn[:-4])
-                rv[fn[:-4]] = enhancements
-    return rv
+                filename = filename.replace("@", ":")
+                enhancements = Enhancements.from_config_string(f.read(), id=filename[:-4])
+                enhancement_bases[filename[:-4]] = enhancements
+    return enhancement_bases
 
 
 ENHANCEMENT_BASES = _load_configs()

src/sentry/grouping/enhancer/actions.py

@@ -9,7 +9,9 @@
 
 ACTIONS = ["group", "app"]
 ACTION_BITSIZE = 8
-assert len(ACTIONS) < 1 << ACTION_BITSIZE
+# Ensure that the number of possible actions is smaller than the number of numbers which can be
+# represented with `ACTION_BITSIZE` bits
+assert len(ACTIONS) < 1 << ACTION_BITSIZE  # This is 2^ACTION_BITSIZE
 ACTION_FLAGS = {
     (True, None): 0,
     (True, "up"): 1,
@@ -54,19 +56,26 @@ def is_updater(self) -> bool:
 
     @classmethod
     def _from_config_structure(cls, val, version: int):
-        if isinstance(val, list):
+        if isinstance(val, list):  # This is a `VarAction`
             return VarAction(val[0], val[1])
+        # Otherwise, assume it's a `FlagAction`, since those are the only two types we currently have
         flag, range_direction = REVERSE_ACTION_FLAGS[val >> ACTION_BITSIZE]
         return FlagAction(ACTIONS[val & 0xF], flag, range_direction)
 
 
 class FlagAction(EnhancementAction):
+    """
+    An action which sets either a frame's `contributes` value or its `in_app` value.
+
+    May optionally set the value for all frames above or below it in the stacktrace as well.
+    """
+
     def __init__(self, key: str, flag: bool, range: str | None) -> None:
-        self.key = key
+        self.key = key  # The type of change (`app` or `group`)
         self._is_updater = key in {"group", "app"}
         self._is_modifier = key == "app"
-        self.flag = flag
-        self.range = range  # e.g. None, "up", "down"
+        self.flag = flag  # True for `+app/+group` rules, False for `-app/-group` rules
+        self.range = range  # None (apply the action to this frame), "up", or "down"
 
     def __str__(self) -> str:
         return "{}{}{}".format(
@@ -76,6 +85,15 @@ def __str__(self) -> str:
         )
 
     def _to_config_structure(self, version: int):
+        """
+        Convert the action into an integer by
+            - converting the combination of its boolean value (if it's a `+app/+group` rule or a
+              `-app/-group` rule) and its range (if it applies to this frame, frames above, or
+              frames below) into a number (see `ACTION_FLAGS`) and then multiplying that number by
+              2^ACTION_BITSIZE
+            - converting its type (app or group) into a number (using the index in `ACTIONS`)
+            - bitwise or-ing those two numbers
+        """
         return ACTIONS.index(self.key) | (ACTION_FLAGS[self.flag, self.range] << ACTION_BITSIZE)
 
     def _slice_to_range(self, seq, idx):

src/sentry/grouping/enhancer/matchers.py

@@ -64,7 +64,7 @@ def _cached(cache, function, *args, **kwargs):
     "value": "value",
     "mechanism": "mechanism",
     "category": "category",
-    # fingerprinting specific fields
+    # fingerprinting-specific fields
     "family": "family",
     "app": "app",
 }
@@ -112,8 +112,8 @@ def _to_config_structure(self, version):
         raise NotImplementedError()
 
     @staticmethod
-    def _from_config_structure(obj, version):
-        val = obj
+    def _from_config_structure(config_structure, version):
+        val = config_structure
         if val.startswith("|[") and val.endswith("]"):
             return CalleeMatch(EnhancementMatch._from_config_structure(val[2:-1], version))
         if val.startswith("[") and val.endswith("]|"):
@@ -195,18 +195,36 @@ def matches_frame(self, frames, idx, exception_data, cache):
         return rv
 
     def _positive_frame_match(self, match_frame, exception_data, cache):
-        # Implement is subclasses
+        # Implement in subclasses
         raise NotImplementedError
 
     def _to_config_structure(self, version):
+        """
+        Convert the matcher into a string of the form
+            <match_type><match_pattern>
+        where
+            match_type is a single letter code for the match type (see MATCH_KEYS)
+            match_pattern is the value to match against
+
+        This will be preceded by a `!` if the match is negated. Families against which to match are
+        also converted to single-letter abbreviations, and in-app booleans are converted to 0 or 1.
+        """
+        # Convert the families to match into a string of single letter abbreviations (so
+        # `javascript,native` becomes `JN`, for example)
         if self.key == "family":
-            arg = "".join(_f for _f in [FAMILIES.get(x) for x in self.pattern.split(",")] if _f)
+            value_to_match = "".join(
+                abbreviation
+                for abbreviation in [FAMILIES.get(family) for family in self.pattern.split(",")]
+                if abbreviation
+            )
         elif self.key == "app":
             boolified_pattern = bool_from_string(self.pattern)
-            arg = "1" if boolified_pattern is True else "0" if boolified_pattern is False else ""
+            value_to_match = (
+                "1" if boolified_pattern is True else "0" if boolified_pattern is False else ""
+            )
         else:
-            arg = self.pattern
-        return ("!" if self.negated else "") + MATCH_KEYS[self.key] + arg
+            value_to_match = self.pattern
+        return ("!" if self.negated else "") + MATCH_KEYS[self.key] + value_to_match
 
 
 def path_like_match(pattern, value):
@@ -268,13 +286,13 @@ def _positive_frame_match(self, match_frame, exception_data, cache):
 
 class FrameFieldMatch(FrameMatch):
     def _positive_frame_match(self, match_frame, exception_data, cache):
-        field = match_frame[self.field]
-        if field is None:
+        value = match_frame[self.field]
+        if value is None:
             return False
-        if field == self._encoded_pattern:
+        if value == self._encoded_pattern:
             return True
 
-        return _cached(cache, glob_match, field, self._encoded_pattern)
+        return _cached(cache, glob_match, value, self._encoded_pattern)
 
 
 class FunctionMatch(FrameFieldMatch):

src/sentry/grouping/enhancer/rules.py

@@ -24,7 +24,7 @@ def __init__(self, matchers, actions):
 
     @property
     def matcher_description(self):
-        rv = " ".join(x.description for x in self.matchers)
+        rv = " ".join(matcher.description for matcher in self.matchers)
         for action in self.actions:
             rv = f"{rv} {action}"
         return rv
@@ -47,7 +47,7 @@ def as_dict(self):
         matchers = {}
         for matcher in self.matchers:
             matchers[matcher.key] = matcher.pattern
-        return {"match": matchers, "actions": [str(x) for x in self.actions]}
+        return {"match": matchers, "actions": [str(action) for action in self.actions]}
 
     def get_matching_frame_actions(
         self,
@@ -81,13 +81,20 @@ def get_matching_frame_actions(
 
     def _to_config_structure(self, version):
         return [
-            [x._to_config_structure(version) for x in self.matchers],
-            [x._to_config_structure(version) for x in self.actions],
+            [matcher._to_config_structure(version) for matcher in self.matchers],
+            [action._to_config_structure(version) for action in self.actions],
         ]
 
     @classmethod
-    def _from_config_structure(cls, tuple, version):
+    def _from_config_structure(cls, config_structure, version):
+        matcher_abbreviations, encoded_actions = config_structure
         return EnhancementRule(
-            [EnhancementMatch._from_config_structure(x, version) for x in tuple[0]],
-            [EnhancementAction._from_config_structure(x, version) for x in tuple[1]],
+            [
+                EnhancementMatch._from_config_structure(matcher, version)
+                for matcher in matcher_abbreviations
+            ],
+            [
+                EnhancementAction._from_config_structure(action, version)
+                for action in encoded_actions
+            ],
         )