)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"1882482b6b680127635b1ce903c47ae9780d4c21","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     Wael Halbawi \u003cwhalbawi@nvidia.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2026-04-10 13:52:04 -0700"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"relinker: Test cleaning up consecutive hashes in suffix directory"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The `hashes_filter` hook in `Relinker` suffers from shadowing. If it"},{"line_number":10,"context_line":"weren\u0027t for the additional path equality check in `process_policy`, the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"791c3d0e_9bbbf814","line":7,"range":{"start_line":7,"start_character":10,"end_line":7,"end_character":65},"updated":"2026-04-15 16:07:09.000000000","message":"Can we squeeze ``same suffix directory`` into the subject line?\n\nAlso, the message could perhaps be more up-front is stating the patches contribution:\n\n\"Add missing test coverage for iterating over multiple hashes in the same suffix\".","commit_id":"aa186378d238e384e7898a4536b2b9db89690334"},{"author":{"_account_id":38767,"name":"Wael Halbawi","display_name":"Wael Halbawi","email":"whalbawi@nvidia.com","username":"whalbawi"},"change_message_id":"c57c0caa1bb74578f4254713a556091efa6fda58","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Wael Halbawi \u003cwhalbawi@nvidia.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2026-04-10 13:52:04 -0700"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"relinker: Test cleaning up consecutive hashes in suffix directory"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The `hashes_filter` hook in `Relinker` suffers from shadowing. If it"},{"line_number":10,"context_line":"weren\u0027t for the additional path equality check in `process_policy`, the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"ca95abb5_3b7f1314","line":7,"range":{"start_line":7,"start_character":10,"end_line":7,"end_character":65},"in_reply_to":"791c3d0e_9bbbf814","updated":"2026-04-15 21:58:49.000000000","message":"Done.","commit_id":"aa186378d238e384e7898a4536b2b9db89690334"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"1882482b6b680127635b1ce903c47ae9780d4c21","unresolved":true,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"relinker: Test cleaning up consecutive hashes in suffix directory"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The `hashes_filter` hook in `Relinker` suffers from shadowing. If it"},{"line_number":10,"context_line":"weren\u0027t for the additional path equality check in `process_policy`, the"},{"line_number":11,"context_line":"relinker might unlinked hashes that are not candidates for cleanup."},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"0fbcbb3d_f7cad499","line":9,"range":{"start_line":9,"start_character":52,"end_line":9,"end_character":61},"updated":"2026-04-15 16:07:09.000000000","message":"Is it \u0027shadowing\u0027 (as in \u0027variable name shadowing\u0027) or \u0027list mutation while iterating\u0027?","commit_id":"aa186378d238e384e7898a4536b2b9db89690334"},{"author":{"_account_id":38767,"name":"Wael Halbawi","display_name":"Wael Halbawi","email":"whalbawi@nvidia.com","username":"whalbawi"},"change_message_id":"c57c0caa1bb74578f4254713a556091efa6fda58","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"relinker: Test cleaning up consecutive hashes in suffix directory"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The `hashes_filter` hook in `Relinker` suffers from shadowing. If it"},{"line_number":10,"context_line":"weren\u0027t for the additional path equality check in `process_policy`, the"},{"line_number":11,"context_line":"relinker might unlinked hashes that are not candidates for cleanup."},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"91f908e3_24ae29b8","line":9,"range":{"start_line":9,"start_character":52,"end_line":9,"end_character":61},"in_reply_to":"0fbcbb3d_f7cad499","updated":"2026-04-15 21:58:49.000000000","message":"Interesting perspective. I see shadowing as the root cause for mutation while iterating.","commit_id":"aa186378d238e384e7898a4536b2b9db89690334"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"1882482b6b680127635b1ce903c47ae9780d4c21","unresolved":true,"context_lines":[{"line_number":11,"context_line":"relinker might unlinked hashes that are not candidates for cleanup."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"This patch adds a regression test for one scenario where this could\u0027ve"},{"line_number":14,"context_line":"happened."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Change-Id: I0b3c9a5b4645ed1bbfa3beb25ae610d166533afb"},{"line_number":17,"context_line":"Signed-off-by: Wael Halbawi \u003cwhalbawi@nvidia.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"5091854c_707d2015","line":14,"updated":"2026-04-15 16:07:09.000000000","message":"Is there a reason why this patch doesn\u0027t fix the list mutation? I can see a follow-on patch - could/should it be squashed into this. It\u0027s a little odd to call out the bug but not address it.","commit_id":"aa186378d238e384e7898a4536b2b9db89690334"},{"author":{"_account_id":38767,"name":"Wael Halbawi","display_name":"Wael Halbawi","email":"whalbawi@nvidia.com","username":"whalbawi"},"change_message_id":"c57c0caa1bb74578f4254713a556091efa6fda58","unresolved":false,"context_lines":[{"line_number":11,"context_line":"relinker might unlinked hashes that are not candidates for cleanup."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"This patch adds a regression test for one scenario where this could\u0027ve"},{"line_number":14,"context_line":"happened."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Change-Id: I0b3c9a5b4645ed1bbfa3beb25ae610d166533afb"},{"line_number":17,"context_line":"Signed-off-by: Wael Halbawi \u003cwhalbawi@nvidia.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"5f1465ef_60c7aff4","line":14,"in_reply_to":"5091854c_707d2015","updated":"2026-04-15 21:58:49.000000000","message":"Fair point. I punted because there isn\u0027t a an actual bug in the relinker itself - the inline filter in `process_policy` ends up correcting the state.","commit_id":"aa186378d238e384e7898a4536b2b9db89690334"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":38767,"name":"Wael Halbawi","display_name":"Wael Halbawi","email":"whalbawi@nvidia.com","username":"whalbawi"},"change_message_id":"c9643ac9351a62f4af9ae1169bf3f798789cb708","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"77f470b3_2fa62aa2","updated":"2026-04-10 19:02:13.000000000","message":"The test fails when the following lines are commented out:\nhttps://opendev.org/openstack/swift/src/branch/master/swift/cli/relinker.py#L661-L662","commit_id":"f18343f98d5f410de300179714fa421e4f41c089"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"1882482b6b680127635b1ce903c47ae9780d4c21","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"34d75030_0ea07af1","updated":"2026-04-15 16:07:09.000000000","message":"AFAICT the new test plug a coverage whole, which is great.\n\nI can see the bug in ``hashes_filter``.\n\nI had a few questions/suggestions, but my main hesitation in merging is wondering if we couldn\u0027t just fix the bug on this patch too?","commit_id":"aa186378d238e384e7898a4536b2b9db89690334"},{"author":{"_account_id":38767,"name":"Wael Halbawi","display_name":"Wael Halbawi","email":"whalbawi@nvidia.com","username":"whalbawi"},"change_message_id":"c57c0caa1bb74578f4254713a556091efa6fda58","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"7305cfd7_5639eaa6","updated":"2026-04-15 21:58:49.000000000","message":"Thanks for the feedback @alistairncoles@gmail.com!","commit_id":"aa186378d238e384e7898a4536b2b9db89690334"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"8e78eb9cca60c272bee6773340d9a047c765b092","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"daca2f03_375bf241","updated":"2026-04-17 12:13:06.000000000","message":"LGTM. Adds test coverage that was previously missing.\n\nThe test helpers might be better arranged but that\u0027s not a blocker, I may push a follow-up.\n\nI was initially confused by mention of a bug in the commit message but that bug not being fixed. We had a verbal conversation about that topic. For the record, my conclusions:\n\n- there\u0027s a bug in hashes_filter\n- the bug is masked by code in process_policy, so the bug never manifests\n- there was no test coverage of the case where it might manifest if there were a regression in process_policy\n- this patch adds the test coverage, thus defending against regression in process_policy\n- the bug in hashes_filter remains, but these new tests don\u0027t cover that code because it is masked by process_policy\n- it is reasonable to limit the scope of this patch to purely increasing test coverage","commit_id":"ee4237795d7eb3414e32f4209b0376af9749788e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"429fe3882aec9c97a4da42180d32a443b337fcbc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"8f619434_c8d6ff62","updated":"2026-04-17 12:30:52.000000000","message":"floow-up 985096: test_relinker: simplify helper methods | https://review.opendev.org/c/openstack/swift/+/985096","commit_id":"ee4237795d7eb3414e32f4209b0376af9749788e"}],"test/unit/cli/test_relinker.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"1882482b6b680127635b1ce903c47ae9780d4c21","unresolved":true,"context_lines":[{"line_number":138,"context_line":"        if remove_objects_dir:"},{"line_number":139,"context_line":"            shutil.rmtree(objects_dir, ignore_errors\u003dTrue)"},{"line_number":140,"context_line":"        if not os.path.exists(objects_dir):"},{"line_number":141,"context_line":"            os.mkdir(objects_dir)"},{"line_number":142,"context_line":"        objdir \u003d os.path.join(objects_dir, str(part), _hash[-3:], _hash)"},{"line_number":143,"context_line":"        os.makedirs(objdir)"},{"line_number":144,"context_line":"        timestamp \u003d utils.Timestamp.now()"}],"source_content_type":"text/x-python","patch_set":4,"id":"b90bdef5_66e195e7","line":141,"updated":"2026-04-15 16:07:09.000000000","message":"this is unfortunate: cleanup first, but maybe not 🤗\n\nOn master there is only one test case that calls _create_object directly. Otherwise it is called from _setup_object. So maybe the rmtreee could be done using a helper called from _setup_object, test_relink_cleanup and the new tests in this patch. That way the tests get a little more explicit control over the state of the testdir.\n\nSomething like:\n```\ndiff --git a/test/unit/cli/test_relinker.py b/test/unit/cli/test_relinker.py\nindex e3b7a13e6..403f81ad4 100644\n--- a/test/unit/cli/test_relinker.py\n+++ b/test/unit/cli/test_relinker.py\n@@ -130,15 +130,10 @@ class TestRelinker(unittest.TestCase):\n                       % attempts)\n         return _hash, part, next_part, obj_path\n \n-    def _create_object(self, policy, part, _hash, ext\u003d\u0027.data\u0027,\n-                       remove_objects_dir\u003dTrue):\n+    def _create_object(self, policy, part, _hash, ext\u003d\u0027.data\u0027):\n         objects_dir \u003d os.path.join(self.devices, self.existing_device,\n                                    get_policy_string(\u0027objects\u0027, policy))\n \n-        if remove_objects_dir:\n-            shutil.rmtree(objects_dir, ignore_errors\u003dTrue)\n-        if not os.path.exists(objects_dir):\n-            os.mkdir(objects_dir)\n         objdir \u003d os.path.join(objects_dir, str(part), _hash[-3:], _hash)\n         os.makedirs(objdir)\n         timestamp \u003d utils.Timestamp.now()\n@@ -150,6 +145,13 @@ class TestRelinker(unittest.TestCase):\n                            {\u0027name\u0027: self.obj_path, \u0027Content-Length\u0027: \u002712\u0027})\n         return objdir, filename, timestamp\n \n+    def _recreate_objects_dir(self, policy):\n+        objects_dir \u003d os.path.join(self.devices, self.existing_device,\n+                                   get_policy_string(\u0027objects\u0027, policy))\n+        shutil.rmtree(objects_dir, ignore_errors\u003dTrue)\n+        os.mkdir(objects_dir)\n+        return objects_dir\n+\n     def _setup_object(self, condition\u003dNone, policy\u003dNone, ext\u003d\u0027.data\u0027):\n         policy \u003d policy or self.policy\n         _hash, part, next_part, obj_path \u003d self._get_object_name(condition)\n@@ -157,9 +159,7 @@ class TestRelinker(unittest.TestCase):\n         self.part \u003d part\n         self.next_part \u003d next_part\n         self.obj_path \u003d obj_path\n-        objects_dir \u003d os.path.join(self.devices, self.existing_device,\n-                                   get_policy_string(\u0027objects\u0027, policy))\n-\n+        objects_dir \u003d self._recreate_objects_dir(policy)\n         self.objdir, self.object_fname, self.obj_ts \u003d self._create_object(\n             policy, part, _hash, ext)\n \n@@ -2598,9 +2598,12 @@ class TestRelinker(unittest.TestCase):\n         part2 \u003d utils.get_partition_for_hash(hash2, self.rb.part_power)\n         self.assertEqual(part1, part2)\n \n-        objdir1, fname1, _ \u003d self._create_object(0, part1, hash1)\n-        objdir2, fname2, _ \u003d self._create_object(0, part2, hash2,\n-                                                 remove_objects_dir\u003dFalse)\n+        policy \u003d 0\n+        self._recreate_objects_dir(policy)\n+        objdir1, fname1, _ \u003d self._create_object(policy, part1, hash1)\n+        objdir2, fname2, _ \u003d self._create_object(policy, part2, hash2)\n+        self.assertEqual(os.path.dirname(objdir1), os.path.dirname(objdir2))\n+        self.assertEqual(2, len(os.listdir(os.path.dirname(objdir1))))\n \n         self.rb.prepare_increase_partition_power()\n         self._save_ring()\n@@ -2643,10 +2646,12 @@ class TestRelinker(unittest.TestCase):\n         part2 \u003d utils.get_partition_for_hash(hash2, self.rb.part_power)\n         self.assertEqual(part1, part2)\n         self.assertLess(part1, 2 ** (self.rb.part_power - 1))\n-\n-        objdir1, fname1, _ \u003d self._create_object(0, part1, hash1)\n-        objdir2, fname2, _ \u003d self._create_object(0, part2, hash2,\n-                                                 remove_objects_dir\u003dFalse)\n+        policy \u003d 0\n+        self._recreate_objects_dir(policy)\n+        objdir1, fname1, _ \u003d self._create_object(policy, part1, hash1)\n+        objdir2, fname2, _ \u003d self._create_object(policy, part2, hash2)\n+        self.assertEqual(os.path.dirname(objdir1), os.path.dirname(objdir2))\n+        self.assertEqual(2, len(os.listdir(os.path.dirname(objdir1))))\n \n         with self._mock_relinker():\n             self.assertEqual(0, relinker.main([\n@@ -2697,6 +2702,7 @@ class TestRelinker(unittest.TestCase):\n         # next part *will* be handled during cleanup\n         _hash, pol_1_part, pol_1_next_part, objpath \u003d self._get_object_name(\n             lambda part: part \u003c 2 ** (PART_POWER - 1))\n+        self._recreate_objects_dir(POLICIES[1])\n         self._create_object(POLICIES[1], pol_1_part, _hash)\n \n         state_files \u003d {\n\n```","commit_id":"aa186378d238e384e7898a4536b2b9db89690334"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"8e78eb9cca60c272bee6773340d9a047c765b092","unresolved":false,"context_lines":[{"line_number":138,"context_line":"        if remove_objects_dir:"},{"line_number":139,"context_line":"            shutil.rmtree(objects_dir, ignore_errors\u003dTrue)"},{"line_number":140,"context_line":"        if not os.path.exists(objects_dir):"},{"line_number":141,"context_line":"            os.mkdir(objects_dir)"},{"line_number":142,"context_line":"        objdir \u003d os.path.join(objects_dir, str(part), _hash[-3:], _hash)"},{"line_number":143,"context_line":"        os.makedirs(objdir)"},{"line_number":144,"context_line":"        timestamp \u003d utils.Timestamp.now()"}],"source_content_type":"text/x-python","patch_set":4,"id":"82613094_98d91445","line":141,"in_reply_to":"0d3ead2c_f265e3f7","updated":"2026-04-17 12:13:06.000000000","message":"My gripe was that there\u0027s a helper method to create an object, but as a side-effect it removes and recreates a dir, but now we need to optionally disable the side-effect. The pre-existing side-effect is what I felt was unfortunate, rather than a more explicit and flexible pattern of e.g.:\n\n```\n_recreate_objects_dir\n_create_object\n_create_object\n```\n\nwhich would not have required you to add the ``remove_objects_dir\u003dTrue`` arg.\n\nI should have prefixed my comment with \u0027nit\u0027. I think there is an opportunity to clean up / improve but it\u0027s not blocker for this patch IMHO.","commit_id":"aa186378d238e384e7898a4536b2b9db89690334"},{"author":{"_account_id":38767,"name":"Wael Halbawi","display_name":"Wael Halbawi","email":"whalbawi@nvidia.com","username":"whalbawi"},"change_message_id":"c57c0caa1bb74578f4254713a556091efa6fda58","unresolved":true,"context_lines":[{"line_number":138,"context_line":"        if remove_objects_dir:"},{"line_number":139,"context_line":"            shutil.rmtree(objects_dir, ignore_errors\u003dTrue)"},{"line_number":140,"context_line":"        if not os.path.exists(objects_dir):"},{"line_number":141,"context_line":"            os.mkdir(objects_dir)"},{"line_number":142,"context_line":"        objdir \u003d os.path.join(objects_dir, str(part), _hash[-3:], _hash)"},{"line_number":143,"context_line":"        os.makedirs(objdir)"},{"line_number":144,"context_line":"        timestamp \u003d utils.Timestamp.now()"}],"source_content_type":"text/x-python","patch_set":4,"id":"0d3ead2c_f265e3f7","line":141,"in_reply_to":"b90bdef5_66e195e7","updated":"2026-04-15 21:58:49.000000000","message":"Is it the fact that there are two if statements? I find that a little confusing after reading the code again. Is the following better?\n```\ndiff --git a/test/unit/cli/test_relinker.py b/test/unit/cli/test_relinker.py\nindex 3a78d66c3..d82801e3a 100644\n--- a/test/unit/cli/test_relinker.py\n+++ b/test/unit/cli/test_relinker.py\n@@ -16,6 +16,7 @@ import fcntl\n import json\n from contextlib import contextmanager\n import logging\n+from pathlib import Path\n from textwrap import dedent\n\n from unittest import mock\n@@ -137,8 +138,8 @@ class TestRelinker(unittest.TestCase):\n\n         if remove_objects_dir:\n             shutil.rmtree(objects_dir, ignore_errors\u003dTrue)\n-        if not os.path.exists(objects_dir):\n-            os.mkdir(objects_dir)\n+        Path(objects_dir).mkdir(exist_ok\u003dTrue)\n+\n         objdir \u003d os.path.join(objects_dir, str(part), _hash[-3:], _hash)\n         os.makedirs(objdir)\n         timestamp \u003d utils.Timestamp.now()\n```","commit_id":"aa186378d238e384e7898a4536b2b9db89690334"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"1882482b6b680127635b1ce903c47ae9780d4c21","unresolved":true,"context_lines":[{"line_number":2584,"context_line":"            os.path.join(self.objdir, self.object_fname)))"},{"line_number":2585,"context_line":"        self.assertEqual([], self.logger.get_lines_for_level(\u0027error\u0027))"},{"line_number":2586,"context_line":""},{"line_number":2587,"context_line":"    def test_relink_consecutive_in_suffix_dir(self):"},{"line_number":2588,"context_line":"        # Both hashes are:"},{"line_number":2589,"context_line":"        # In the same partition for part_power \u003d PART_POWER"},{"line_number":2590,"context_line":"        # In the lower half of the partition space: they\u0027re relink candidates"}],"source_content_type":"text/x-python","patch_set":4,"id":"e78d1ed6_3c5e1e7e","line":2587,"updated":"2026-04-15 16:07:09.000000000","message":"I got this, and only this, test to fail with this diff:\n```\ndiff --git a/swift/cli/relinker.py b/swift/cli/relinker.py\nindex 48e2e1169..ff16c846d 100644\n--- a/swift/cli/relinker.py\n+++ b/swift/cli/relinker.py\n@@ -387,7 +387,7 @@ class Relinker(object):\n         self._update_recon(device)\n \n     def hashes_filter(self, suff_path, hashes):\n-        hashes \u003d list(hashes)\n+        hashes \u003d list(hashes[:1])\n         for hsh in hashes:\n             fname \u003d os.path.join(suff_path, hsh)\n             if fname \u003d\u003d replace_partition_in_path(\n\n```","commit_id":"aa186378d238e384e7898a4536b2b9db89690334"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"8e78eb9cca60c272bee6773340d9a047c765b092","unresolved":false,"context_lines":[{"line_number":2584,"context_line":"            os.path.join(self.objdir, self.object_fname)))"},{"line_number":2585,"context_line":"        self.assertEqual([], self.logger.get_lines_for_level(\u0027error\u0027))"},{"line_number":2586,"context_line":""},{"line_number":2587,"context_line":"    def test_relink_consecutive_in_suffix_dir(self):"},{"line_number":2588,"context_line":"        # Both hashes are:"},{"line_number":2589,"context_line":"        # In the same partition for part_power \u003d PART_POWER"},{"line_number":2590,"context_line":"        # In the lower half of the partition space: they\u0027re relink candidates"}],"source_content_type":"text/x-python","patch_set":4,"id":"cbd275c0_4ca323e1","line":2587,"in_reply_to":"0e129a03_92d630a1","updated":"2026-04-17 12:13:06.000000000","message":"No, sorry, this was intended to be an affirming comment: the new test covers a possible regression that *no existing test covers* - that means it\u0027s a good thing to add this test 🙂","commit_id":"aa186378d238e384e7898a4536b2b9db89690334"},{"author":{"_account_id":38767,"name":"Wael Halbawi","display_name":"Wael Halbawi","email":"whalbawi@nvidia.com","username":"whalbawi"},"change_message_id":"c57c0caa1bb74578f4254713a556091efa6fda58","unresolved":true,"context_lines":[{"line_number":2584,"context_line":"            os.path.join(self.objdir, self.object_fname)))"},{"line_number":2585,"context_line":"        self.assertEqual([], self.logger.get_lines_for_level(\u0027error\u0027))"},{"line_number":2586,"context_line":""},{"line_number":2587,"context_line":"    def test_relink_consecutive_in_suffix_dir(self):"},{"line_number":2588,"context_line":"        # Both hashes are:"},{"line_number":2589,"context_line":"        # In the same partition for part_power \u003d PART_POWER"},{"line_number":2590,"context_line":"        # In the lower half of the partition space: they\u0027re relink candidates"}],"source_content_type":"text/x-python","patch_set":4,"id":"0e129a03_92d630a1","line":2587,"in_reply_to":"e78d1ed6_3c5e1e7e","updated":"2026-04-15 21:58:49.000000000","message":"Apologies - I\u0027m not following. Are you suggesting we improve `hashes_filter`\u0027s test coverage?","commit_id":"aa186378d238e384e7898a4536b2b9db89690334"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"1882482b6b680127635b1ce903c47ae9780d4c21","unresolved":true,"context_lines":[{"line_number":2623,"context_line":"                                                self.rb.next_part_power)"},{"line_number":2624,"context_line":""},{"line_number":2625,"context_line":"        self.assertTrue(os.path.isfile(path1))"},{"line_number":2626,"context_line":"        self.assertTrue(os.path.isfile(path2))"},{"line_number":2627,"context_line":""},{"line_number":2628,"context_line":"    def test_cleanup_consecutive_in_suffix_dir(self):"},{"line_number":2629,"context_line":"        # Both hashes are:"}],"source_content_type":"text/x-python","patch_set":4,"id":"110022b0_cfddc34f","line":2626,"updated":"2026-04-15 16:07:09.000000000","message":"worth asserting the original files are intact\n```\n        self.assertTrue(os.path.isfile(os.path.join(objdir1, fname1)))\n        self.assertTrue(os.path.isfile(os.path.join(objdir2, fname2)))\n\n```","commit_id":"aa186378d238e384e7898a4536b2b9db89690334"},{"author":{"_account_id":38767,"name":"Wael Halbawi","display_name":"Wael Halbawi","email":"whalbawi@nvidia.com","username":"whalbawi"},"change_message_id":"c57c0caa1bb74578f4254713a556091efa6fda58","unresolved":false,"context_lines":[{"line_number":2623,"context_line":"                                                self.rb.next_part_power)"},{"line_number":2624,"context_line":""},{"line_number":2625,"context_line":"        self.assertTrue(os.path.isfile(path1))"},{"line_number":2626,"context_line":"        self.assertTrue(os.path.isfile(path2))"},{"line_number":2627,"context_line":""},{"line_number":2628,"context_line":"    def test_cleanup_consecutive_in_suffix_dir(self):"},{"line_number":2629,"context_line":"        # Both hashes are:"}],"source_content_type":"text/x-python","patch_set":4,"id":"536028c3_583328e0","line":2626,"in_reply_to":"110022b0_cfddc34f","updated":"2026-04-15 21:58:49.000000000","message":"Done. Thanks!","commit_id":"aa186378d238e384e7898a4536b2b9db89690334"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"1882482b6b680127635b1ce903c47ae9780d4c21","unresolved":true,"context_lines":[{"line_number":2628,"context_line":"    def test_cleanup_consecutive_in_suffix_dir(self):"},{"line_number":2629,"context_line":"        # Both hashes are:"},{"line_number":2630,"context_line":"        # In the same partition for part_power \u003d PART_POWER + 1"},{"line_number":2631,"context_line":"        # In the lower half of the partition space: they\u0027re cleanup candidates"},{"line_number":2632,"context_line":"        # In the same suffix directory"},{"line_number":2633,"context_line":"        hash1 \u003d \"027ecbf9027e96e1fe83e6154e1a8380\""},{"line_number":2634,"context_line":"        hash2 \u003d \"024f19a8347495adc3cfa845f725e380\""}],"source_content_type":"text/x-python","patch_set":4,"id":"2878e49f_6d3d9443","line":2631,"range":{"start_line":2631,"start_character":60,"end_line":2631,"end_character":78},"updated":"2026-04-15 16:07:09.000000000","message":"Can I check my understanding: they are \"candidates\" ... but not actually selected for cleanup because they are in the correct partition?\n\nMight be helpful to clarify because on first read I found myself wondering why they had not been cleaned up!","commit_id":"aa186378d238e384e7898a4536b2b9db89690334"},{"author":{"_account_id":38767,"name":"Wael Halbawi","display_name":"Wael Halbawi","email":"whalbawi@nvidia.com","username":"whalbawi"},"change_message_id":"c57c0caa1bb74578f4254713a556091efa6fda58","unresolved":true,"context_lines":[{"line_number":2628,"context_line":"    def test_cleanup_consecutive_in_suffix_dir(self):"},{"line_number":2629,"context_line":"        # Both hashes are:"},{"line_number":2630,"context_line":"        # In the same partition for part_power \u003d PART_POWER + 1"},{"line_number":2631,"context_line":"        # In the lower half of the partition space: they\u0027re cleanup candidates"},{"line_number":2632,"context_line":"        # In the same suffix directory"},{"line_number":2633,"context_line":"        hash1 \u003d \"027ecbf9027e96e1fe83e6154e1a8380\""},{"line_number":2634,"context_line":"        hash2 \u003d \"024f19a8347495adc3cfa845f725e380\""}],"source_content_type":"text/x-python","patch_set":4,"id":"8c6e7e64_3c1854de","line":2631,"range":{"start_line":2631,"start_character":60,"end_line":2631,"end_character":78},"in_reply_to":"2878e49f_6d3d9443","updated":"2026-04-15 21:58:49.000000000","message":"They\u0027re candidates in the sense that they are not omitted due to a partition-level filter where partitions in the upper-half are skipped - before we even inspect the contexts on a partition.\n\nI\u0027ve added a clarifying comment closer to the object creation code. Does that help?","commit_id":"aa186378d238e384e7898a4536b2b9db89690334"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"8e78eb9cca60c272bee6773340d9a047c765b092","unresolved":false,"context_lines":[{"line_number":2628,"context_line":"    def test_cleanup_consecutive_in_suffix_dir(self):"},{"line_number":2629,"context_line":"        # Both hashes are:"},{"line_number":2630,"context_line":"        # In the same partition for part_power \u003d PART_POWER + 1"},{"line_number":2631,"context_line":"        # In the lower half of the partition space: they\u0027re cleanup candidates"},{"line_number":2632,"context_line":"        # In the same suffix directory"},{"line_number":2633,"context_line":"        hash1 \u003d \"027ecbf9027e96e1fe83e6154e1a8380\""},{"line_number":2634,"context_line":"        hash2 \u003d \"024f19a8347495adc3cfa845f725e380\""}],"source_content_type":"text/x-python","patch_set":4,"id":"158422e1_88230c19","line":2631,"range":{"start_line":2631,"start_character":60,"end_line":2631,"end_character":78},"in_reply_to":"8c6e7e64_3c1854de","updated":"2026-04-17 12:13:06.000000000","message":"Great, thanks","commit_id":"aa186378d238e384e7898a4536b2b9db89690334"}]}