Apply suggestions from code review

Co-authored-by: William Durand <[email protected]>

src/olympia/blocklist/cron.py

@@ -126,6 +126,9 @@ def _upload_mlbf_to_remote_settings(*, force_base=False):
     )
 
     if create_stash:
+        # We generate unified stashes, which means they can contain data
+        # for both soft and hard blocks. We need the base filters of each
+        # block type to determine what goes in a stash.
         mlbf.generate_and_write_stash(
             previous_mlbf=previous_filter,
             blocked_base_filter=base_filters[BlockType.BLOCKED],

src/olympia/blocklist/mlbf.py

@@ -231,7 +231,8 @@ def generate_and_write_filter(self, block_type: BlockType):
         Generate and write the bloom filter for a given block type.
         Included items will be items in the specified block type list.
         Excluded items will be items in all other data types.
-        We use the language of inlcude and exclude to distinguish this concept
+
+        We use the language of include and exclude to distinguish this concept
         from blocked and unblocked which are more specific to the block type.
         """
         stats = {}
@@ -294,11 +295,13 @@ def generate_and_write_stash(
         Generate and write the stash file representing changes between the
         previous and current bloom filters. See:
         https://bugzilla.mozilla.org/show_bug.cgi?id=soft-blocking
+        
+        Since we might be generating both a filter and a stash at the exact same time,
+        we need to compute a stash that doesn't include the data already in the newly
+        created filter.
 
         Items that are removed from one block type and added to another are
-        excluded from the unblocked list to prevent double counting. This
-        can lead to invalid states in old FX clients that do not support soft
-        blocking.
+        excluded from the unblocked list to prevent double counting.
 
         If a block type needs a new filter, we do not include any items for that
         block type in the stash to prevent double counting items.

src/olympia/blocklist/tests/test_cron.py

@@ -534,8 +534,8 @@ def test_upload_stash_unless_enough_changes(self):
         assert new_mlbf.storage.exists(new_mlbf.stash_path)
         with new_mlbf.storage.open(new_mlbf.stash_path, 'r') as f:
             data = json.load(f)
-            # We expect an empty stash for hard blocks because we are
-            # uploading a new filter.
+            # We expect an empty list for hard blocks because we are
+            # uploading a new hard filter.
             assert len(data['blocked']) == 0
             # We can still see there are 2 hard blocked versions in the new filter.
             assert len(new_mlbf.data.blocked_items) == 2
@@ -551,7 +551,7 @@ def _test_upload_stash_and_filter(
         with override_switch('enable-soft-blocking', active=enable_soft_blocking):
             upload_mlbf_to_remote_settings()
 
-        # generation time is set to current time so we can load the mlbf
+        # Generation time is set to current time so we can load the MLBF.
         mlbf = MLBF.load_from_storage(self.current_time)
 
         if expected_stash is None:
@@ -584,7 +584,7 @@ def test_upload_blocked_stash_and_softblock_filter(self):
         # Expected stash does not change
         expected_stash = {
             'blocked': 1,
-            # Expect no soft blocks becuase there are enough for a filter
+            # Expect no soft blocks because there are enough for a filter.
             'softblocked': 0,
             # There are no unblocked versions
             'unblocked': 0,