From 0c6f6f533d09449fd2233c2ec9d6b89f0ba1cf73 Mon Sep 17 00:00:00 2001 From: Mohamed Ashraf Date: Tue, 3 Feb 2026 00:43:39 +0000 Subject: [PATCH] fix: add JSON-aware comparison to Python comparator fallback Fixed a bug where the Python fallback comparator used simple string comparison for JSON results, causing false negatives when JSON was semantically identical but formatted differently. Problem: The compare_invocations_directly() function compared result_json fields using direct string comparison (orig_result != cand_result). This failed for semantically identical JSON with: - Different whitespace: {"a":1,"b":2} vs { "a": 1, "b": 2 } - Different key ordering: {"a":1,"b":2} vs {"b":2,"a":1} The Java Comparator handles this correctly by parsing JSON, but the Python fallback did not. Solution: - Added _compare_json_values() helper function that: 1. Handles None values correctly 2. Fast-path for exact string matches 3. Parses JSON and compares deserialized objects 4. Falls back to string comparison if JSON parsing fails - Updated compare_invocations_directly() to use JSON-aware comparison Impact: - Prevents false negatives in behavior verification - Matches Java Comparator behavior for consistency - Handles whitespace, key ordering, and nested objects correctly - Gracefully handles invalid JSON by falling back to string comparison Tests added: - Updated test_whitespace_in_json to expect correct behavior (True) - Added TestJsonComparison class with 8 comprehensive tests: * test_json_key_ordering_difference * test_json_whitespace_and_ordering_combined * test_json_nested_object_comparison * test_json_array_comparison_order_matters * test_json_invalid_json_falls_back_to_string * test_json_null_vs_string_null * test_json_empty_object_vs_null * test_json_numeric_equivalence Test results: 344 Java tests pass (26 comparator tests) Co-Authored-By: Claude Sonnet 4.5 --- codeflash/languages/java/comparator.py | 37 +++++- .../test_java/test_comparator.py | 118 +++++++++++++++++- 2 files changed, 149 insertions(+), 6 deletions(-) diff --git a/codeflash/languages/java/comparator.py b/codeflash/languages/java/comparator.py index c30bd2446..2da70cc51 100644 --- a/codeflash/languages/java/comparator.py +++ b/codeflash/languages/java/comparator.py @@ -19,6 +19,39 @@ logger = logging.getLogger(__name__) +def _compare_json_values(json1: str | None, json2: str | None) -> bool: + """Compare two JSON strings for semantic equality. + + This function parses JSON strings and compares the deserialized objects, + handling differences in whitespace and key ordering. + + Args: + json1: First JSON string (or None). + json2: Second JSON string (or None). + + Returns: + True if the JSON values are semantically equal, False otherwise. + """ + # Handle None cases + if json1 is None and json2 is None: + return True + if json1 is None or json2 is None: + return False + + # Try exact string match first (fast path) + if json1 == json2: + return True + + # Parse and compare as JSON + try: + obj1 = json.loads(json1) + obj2 = json.loads(json2) + return obj1 == obj2 + except (json.JSONDecodeError, TypeError): + # If JSON parsing fails, fall back to string comparison + return json1 == json2 + + def _find_comparator_jar(project_root: Path | None = None) -> Path | None: """Find the codeflash-runtime JAR with the Comparator class. @@ -308,8 +341,8 @@ def compare_invocations_directly( original_pytest_error=orig_error, ) ) - elif orig_result != cand_result: - # Results differ + elif not _compare_json_values(orig_result, cand_result): + # Results differ (using JSON-aware comparison) test_diffs.append( TestDiff( scope=TestDiffScope.RETURN_VALUE, diff --git a/tests/test_languages/test_java/test_comparator.py b/tests/test_languages/test_java/test_comparator.py index bd067b5b2..df81b1462 100644 --- a/tests/test_languages/test_java/test_comparator.py +++ b/tests/test_languages/test_java/test_comparator.py @@ -269,11 +269,10 @@ def test_whitespace_in_json(self): "1": {"result_json": '{ "a": 1, "b": 2 }', "error_json": None}, # With spaces } - # Note: Direct string comparison will see these as different - # The Java comparator would handle this correctly by parsing JSON + # JSON-aware comparison should handle whitespace differences equivalent, diffs = compare_invocations_directly(original, candidate) - # This will fail with direct comparison - expected behavior - assert equivalent is False # String comparison doesn't normalize whitespace + assert equivalent is True # JSON comparison normalizes whitespace + assert len(diffs) == 0 def test_large_number_of_invocations(self): """Test handling large number of invocations.""" @@ -308,3 +307,114 @@ def test_deeply_nested_objects(self): equivalent, diffs = compare_invocations_directly(original, candidate) assert equivalent is True + + +class TestJsonComparison: + """Tests for JSON-aware comparison in compare_invocations_directly.""" + + def test_json_key_ordering_difference(self): + """Test that different JSON key ordering is handled correctly.""" + original = { + "1": {"result_json": '{"a":1,"b":2,"c":3}', "error_json": None}, + } + candidate = { + "1": {"result_json": '{"c":3,"a":1,"b":2}', "error_json": None}, # Different order + } + + equivalent, diffs = compare_invocations_directly(original, candidate) + assert equivalent is True + assert len(diffs) == 0 + + def test_json_whitespace_and_ordering_combined(self): + """Test combined whitespace and key ordering differences.""" + original = { + "1": {"result_json": '{"name":"test","value":42,"active":true}', "error_json": None}, + } + candidate = { + "1": {"result_json": '{ "active": true, "value": 42, "name": "test" }', "error_json": None}, + } + + equivalent, diffs = compare_invocations_directly(original, candidate) + assert equivalent is True + assert len(diffs) == 0 + + def test_json_nested_object_comparison(self): + """Test that nested JSON objects are compared correctly.""" + original = { + "1": {"result_json": '{"outer":{"inner":{"value":123}}}', "error_json": None}, + } + candidate = { + "1": {"result_json": '{ "outer": { "inner": { "value": 123 } } }', "error_json": None}, + } + + equivalent, diffs = compare_invocations_directly(original, candidate) + assert equivalent is True + assert len(diffs) == 0 + + def test_json_array_comparison_order_matters(self): + """Test that array element order matters in comparison.""" + original = { + "1": {"result_json": '[1,2,3]', "error_json": None}, + } + candidate = { + "1": {"result_json": '[3,2,1]', "error_json": None}, # Different order + } + + equivalent, diffs = compare_invocations_directly(original, candidate) + assert equivalent is False # Array order matters + assert len(diffs) == 1 + assert diffs[0].scope == TestDiffScope.RETURN_VALUE + + def test_json_invalid_json_falls_back_to_string(self): + """Test that invalid JSON falls back to string comparison.""" + original = { + "1": {"result_json": 'not valid json {', "error_json": None}, + } + candidate = { + "1": {"result_json": 'not valid json {', "error_json": None}, # Same invalid JSON + } + + # Should fall back to string comparison + equivalent, diffs = compare_invocations_directly(original, candidate) + assert equivalent is True + assert len(diffs) == 0 + + def test_json_null_vs_string_null(self): + """Test comparison of JSON null vs string 'null'.""" + original = { + "1": {"result_json": 'null', "error_json": None}, + } + candidate = { + "1": {"result_json": 'null', "error_json": None}, + } + + equivalent, diffs = compare_invocations_directly(original, candidate) + assert equivalent is True + assert len(diffs) == 0 + + def test_json_empty_object_vs_null(self): + """Test that empty object and null are different.""" + original = { + "1": {"result_json": '{}', "error_json": None}, + } + candidate = { + "1": {"result_json": 'null', "error_json": None}, + } + + equivalent, diffs = compare_invocations_directly(original, candidate) + assert equivalent is False + assert len(diffs) == 1 + + def test_json_numeric_equivalence(self): + """Test that numerically equivalent JSON values match.""" + original = { + "1": {"result_json": '{"value":42}', "error_json": None}, + } + candidate = { + "1": {"result_json": '{"value":42.0}', "error_json": None}, # Int vs float + } + + # Python JSON parsing treats 42 and 42.0 as equal + equivalent, diffs = compare_invocations_directly(original, candidate) + assert equivalent is True + assert len(diffs) == 0