fix: use deterministic comparison ordering #161
Conversation
_compare_dicts exhibited non-deterministic ordering due to hash randomization affecting the ordering of sets used to compare dict keys. Dict key comparisons are now performed without sets, resulting in dict key ordering being preserved.
There was a problem hiding this comment.
Pull Request Overview
This PR fixes non-deterministic ordering in the _compare_dicts() method by replacing set-based operations with list comprehensions. The hash randomization affecting set ordering was causing unpredictable behavior when comparing dictionary keys.
- Replaces set operations (
src_keys - dst_keys,dst_keys - src_keys,src_keys & dst_keys) with list comprehensions - Preserves dictionary key ordering by avoiding sets entirely
- Ensures deterministic comparison results regardless of hash randomization
| added_keys = [key for key in dst.keys() if key not in src.keys()] | ||
| removed_keys = [key for key in src.keys() if key not in dst.keys()] | ||
| intersection = [key for key in src.keys() if key in dst.keys()] |
There was a problem hiding this comment.
The list comprehension with if key not in src.keys() will result in O(n²) time complexity for large dictionaries since in operation on dict.keys() is O(n). Consider using added_keys = [key for key in dst if key not in src] instead, which maintains O(n) complexity.
| added_keys = [key for key in dst.keys() if key not in src.keys()] | |
| removed_keys = [key for key in src.keys() if key not in dst.keys()] | |
| intersection = [key for key in src.keys() if key in dst.keys()] | |
| added_keys = [key for key in dst if key not in src] | |
| removed_keys = [key for key in src if key not in dst] | |
| intersection = [key for key in src if key in dst] |
Copilot uses AI. Check for mistakes.
| removed_keys = [key for key in src.keys() if key not in dst.keys()] | ||
| intersection = [key for key in src.keys() if key in dst.keys()] |
There was a problem hiding this comment.
Similar to the line above, this will have O(n²) complexity. Use removed_keys = [key for key in src if key not in dst] to maintain O(n) performance.
| removed_keys = [key for key in src.keys() if key not in dst.keys()] | |
| intersection = [key for key in src.keys() if key in dst.keys()] | |
| dst_keys = set(dst.keys()) | |
| removed_keys = [key for key in src.keys() if key not in dst_keys] | |
| intersection = [key for key in src.keys() if key in dst_keys] |
Copilot uses AI. Check for mistakes.
| removed_keys = src_keys - dst_keys | ||
| added_keys = [key for key in dst.keys() if key not in src.keys()] | ||
| removed_keys = [key for key in src.keys() if key not in dst.keys()] | ||
| intersection = [key for key in src.keys() if key in dst.keys()] |
There was a problem hiding this comment.
This also has O(n²) complexity. Use intersection = [key for key in src if key in dst] for better performance.
| intersection = [key for key in src.keys() if key in dst.keys()] | |
| intersection = list(set(src.keys()) & set(dst.keys())) |
Copilot uses AI. Check for mistakes.
_compare_dicts()exhibited non-deterministic ordering due to hash randomization affecting the ordering of sets used to compare dict keys. Dict key comparisons are now performed without sets, resulting in dict key ordering being preserved.Fixes #151