feat: optimize comparison handling for KryoPlaceholder and enhance void method state tests

Author: HeshamHM28

codeflash-java-runtime/src/main/java/com/codeflash/Comparator.java

@@ -284,18 +284,10 @@ private static boolean compareInternal(Object orig, Object newObj,
             return false;
         }
 
-        // Detect and reject KryoPlaceholder
-        if (orig instanceof KryoPlaceholder) {
-            KryoPlaceholder p = (KryoPlaceholder) orig;
-            throw new KryoPlaceholderAccessException(
-                "Cannot compare: original contains placeholder for unserializable object",
-                p.getObjType(), p.getPath());
-        }
-        if (newObj instanceof KryoPlaceholder) {
-            KryoPlaceholder p = (KryoPlaceholder) newObj;
-            throw new KryoPlaceholderAccessException(
-                "Cannot compare: new object contains placeholder for unserializable object",
-                p.getObjType(), p.getPath());
+        // KryoPlaceholder means the Serializer couldn't serialize this part.
+        // Skip comparison for placeholder fields — we can't compare what we couldn't serialize.
+        if (orig instanceof KryoPlaceholder || newObj instanceof KryoPlaceholder) {
+            return true;
         }
 
         // Handle exceptions specially

codeflash-java-runtime/src/test/java/com/codeflash/ComparatorCorrectnessTest.java

@@ -60,9 +60,9 @@ void testAllPlaceholderSkips() throws Exception {
         String json = Comparator.compareDatabases(originalDb.toString(), candidateDb.toString());
         Map<String, Object> result = parseJson(json);
 
-        assertFalse((Boolean) result.get("equivalent"));
-        assertEquals(0, ((Number) result.get("actualComparisons")).intValue());
-        assertTrue(((Number) result.get("skippedPlaceholders")).intValue() > 0);
+        // Placeholders are skipped during comparison (treated as matching) — counts as actual comparison
+        assertTrue((Boolean) result.get("equivalent"));
+        assertEquals(1, ((Number) result.get("actualComparisons")).intValue());
     }
 
     @Test
@@ -108,8 +108,8 @@ void testMixedRealAndPlaceholder() throws Exception {
         Map<String, Object> result = parseJson(json);
 
         assertTrue((Boolean) result.get("equivalent"));
-        assertEquals(2, ((Number) result.get("actualComparisons")).intValue());
-        assertEquals(1, ((Number) result.get("skippedPlaceholders")).intValue());
+        // All 3 compared: 2 real values + 1 placeholder (placeholder skipped during deep compare)
+        assertEquals(3, ((Number) result.get("actualComparisons")).intValue());
     }
 
     @Test
@@ -209,6 +209,87 @@ void testIsDeserializationError() {
         assertFalse(Comparator.isDeserializationError(42));
     }
 
+    // ============================================================
+    // VOID METHOD STATE COMPARISON — proves we actually compare
+    // post-call state for void methods, not just skip them
+    // ============================================================
+
+    @Test
+    @DisplayName("void state: both sides sorted identically → equivalent")
+    void testVoidState_identicalMutation_equivalent() throws Exception {
+        createTestDb(originalDb);
+        createTestDb(candidateDb);
+
+        // Simulate: bubbleSortInPlace(arr) — both original and candidate sort correctly
+        // Post-call state: Object[]{sorted_array}
+        int[] sortedArr = {1, 2, 3, 4, 5};
+        byte[] origState = Serializer.serialize(new Object[]{sortedArr});
+        byte[] candState = Serializer.serialize(new Object[]{new int[]{1, 2, 3, 4, 5}});
+
+        insertRow(originalDb, "L1_1", 1, origState);
+        insertRow(candidateDb, "L1_1", 1, candState);
+
+        String json = Comparator.compareDatabases(originalDb.toString(), candidateDb.toString());
+        Map<String, Object> result = parseJson(json);
+
+        assertTrue((Boolean) result.get("equivalent"),
+            "Both sides produce same sorted array — should be equivalent");
+        assertEquals(1, ((Number) result.get("actualComparisons")).intValue());
+    }
+
+    @Test
+    @DisplayName("void state: candidate mutates array differently → NOT equivalent")
+    void testVoidState_differentMutation_rejected() throws Exception {
+        createTestDb(originalDb);
+        createTestDb(candidateDb);
+
+        // Simulate: original sorts [3,1,2] → [1,2,3]
+        // Bad optimization doesn't sort correctly → [3,1,2] unchanged
+        byte[] origState = Serializer.serialize(new Object[]{new int[]{1, 2, 3}});
+        byte[] candState = Serializer.serialize(new Object[]{new int[]{3, 1, 2}});
+
+        insertRow(originalDb, "L1_1", 1, origState);
+        insertRow(candidateDb, "L1_1", 1, candState);
+
+        String json = Comparator.compareDatabases(originalDb.toString(), candidateDb.toString());
+        Map<String, Object> result = parseJson(json);
+
+        assertFalse((Boolean) result.get("equivalent"),
+            "Candidate produced wrong array — must be rejected");
+        assertEquals(1, ((Number) result.get("actualComparisons")).intValue());
+    }
+
+    @Test
+    @DisplayName("void state: receiver + args both compared — wrong receiver state rejected")
+    void testVoidState_receiverAndArgs_wrongReceiverRejected() throws Exception {
+        createTestDb(originalDb);
+        createTestDb(candidateDb);
+
+        // Simulate: instance method sorter.sort(data)
+        // Post-call state is Object[]{receiver_fields_map, mutated_data}
+        // Original: receiver has size=3, data is [1,2,3]
+        // Candidate: receiver has size=0 (wrong), data is [1,2,3]
+        Map<String, Object> origReceiver = new HashMap<>();
+        origReceiver.put("size", 3);
+        origReceiver.put("sorted", true);
+        Map<String, Object> candReceiver = new HashMap<>();
+        candReceiver.put("size", 0);
+        candReceiver.put("sorted", true);
+
+        byte[] origState = Serializer.serialize(new Object[]{origReceiver, new int[]{1, 2, 3}});
+        byte[] candState = Serializer.serialize(new Object[]{candReceiver, new int[]{1, 2, 3}});
+
+        insertRow(originalDb, "L1_1", 1, origState);
+        insertRow(candidateDb, "L1_1", 1, candState);
+
+        String json = Comparator.compareDatabases(originalDb.toString(), candidateDb.toString());
+        Map<String, Object> result = parseJson(json);
+
+        assertFalse((Boolean) result.get("equivalent"),
+            "Receiver state differs (size 3 vs 0) — must be rejected even though args match");
+        assertEquals(1, ((Number) result.get("actualComparisons")).intValue());
+    }
+
     // --- Helpers ---
 
     private void createTestDb(Path dbPath) throws Exception {

codeflash-java-runtime/src/test/java/com/codeflash/ComparatorTest.java

@@ -311,49 +311,47 @@ void testBothNullMessages() {
     class PlaceholderTests {
 
         @Test
-        @DisplayName("original contains placeholder: throws exception")
+        @DisplayName("original contains placeholder: skipped, treated as matching")
         void testOriginalPlaceholder() {
             KryoPlaceholder placeholder = new KryoPlaceholder(
                 "java.net.Socket", "<socket>", "error", "path"
             );
 
-            assertThrows(KryoPlaceholderAccessException.class, () -> {
-                Comparator.compare(placeholder, "anything");
-            });
+            // Placeholders are skipped — comparison returns true (not comparable, so skip)
+            assertTrue(Comparator.compare(placeholder, "anything"));
         }
 
         @Test
-        @DisplayName("new contains placeholder: throws exception")
+        @DisplayName("new contains placeholder: skipped, treated as matching")
         void testNewPlaceholder() {
             KryoPlaceholder placeholder = new KryoPlaceholder(
                 "java.net.Socket", "<socket>", "error", "path"
             );
 
-            assertThrows(KryoPlaceholderAccessException.class, () -> {
-                Comparator.compare("anything", placeholder);
-            });
+            assertTrue(Comparator.compare("anything", placeholder));
         }
 
         @Test
-        @DisplayName("placeholder in nested structure: throws exception")
+        @DisplayName("placeholder in nested structure: skipped, rest compared normally")
         void testNestedPlaceholder() {
             KryoPlaceholder placeholder = new KryoPlaceholder(
                 "java.net.Socket", "<socket>", "error", "data.socket"
             );
 
             Map<String, Object> map1 = new HashMap<>();
             map1.put("socket", placeholder);
+            map1.put("name", "same");
 
             Map<String, Object> map2 = new HashMap<>();
             map2.put("socket", "different");
+            map2.put("name", "same");
 
-            assertThrows(KryoPlaceholderAccessException.class, () -> {
-                Comparator.compare(map1, map2);
-            });
+            // Placeholder field skipped, "name" field compared normally → equivalent
+            assertTrue(Comparator.compare(map1, map2));
         }
 
         @Test
-        @DisplayName("compareWithDetails captures error message")
+        @DisplayName("compareWithDetails with placeholder returns equal (skipped)")
         void testCompareWithDetails() {
             KryoPlaceholder placeholder = new KryoPlaceholder(
                 "java.net.Socket", "<socket>", "error", "path"
@@ -362,9 +360,8 @@ void testCompareWithDetails() {
             Comparator.ComparisonResult result =
                 Comparator.compareWithDetails(placeholder, "anything");
 
-            assertFalse(result.isEqual());
-            assertTrue(result.hasError());
-            assertNotNull(result.getErrorMessage());
+            assertTrue(result.isEqual());
+            assertFalse(result.hasError());
         }
     }
 

codeflash-java-runtime/src/test/java/com/codeflash/SerializerTest.java

@@ -268,8 +268,8 @@ void testClassWithUnserializableAttribute() throws Exception {
     class PlaceholderAccessTests {
 
         @Test
-        @DisplayName("comparing objects with placeholder throws KryoPlaceholderAccessException")
-        void testPlaceholderComparisonThrowsException() throws Exception {
+        @DisplayName("comparing objects with placeholder skips the placeholder field")
+        void testPlaceholderComparisonSkips() throws Exception {
             try (Socket socket = new Socket()) {
                 Map<String, Object> data = new LinkedHashMap<>();
                 data.put("socket", socket);
@@ -279,9 +279,8 @@ void testPlaceholderComparisonThrowsException() throws Exception {
 
                 KryoPlaceholder placeholder = (KryoPlaceholder) reloaded.get("socket");
 
-                assertThrows(KryoPlaceholderAccessException.class, () -> {
-                    Comparator.compare(placeholder, "anything");
-                });
+                // Placeholders are skipped during comparison — treated as matching
+                assertTrue(Comparator.compare(placeholder, "anything"));
             }
         }
     }

codeflash/languages/java/comparator.py

@@ -299,16 +299,6 @@ def compare_test_results(
         skipped_deser_errors = comparison.get("skippedDeserializationErrors", 0)
 
         if actual_comparisons == 0:
-            if skipped_placeholders > 0 and skipped_deser_errors == 0 and not comparison.get("diffs"):
-                # For void methods, all return values are null → all are "placeholder" skips.
-                # If no diffs and no deser errors, treat as equivalent (pass/fail verification).
-                logger.info(
-                    "Java comparison: void method — all return values null, treating as equivalent "
-                    "(total=%s, skipped_placeholders=%s)",
-                    comparison.get("totalInvocations", 0),
-                    skipped_placeholders,
-                )
-                return True, []
             logger.warning(
                 "Java comparison: no actual comparisons performed "
                 "(total=%s, skipped_placeholders=%s, skipped_deser_errors=%s). "

codeflash/languages/java/instrumentation.py

@@ -203,6 +203,7 @@ def _generate_sqlite_write_code(
     func_name: str,
     test_method_name: str,
     invocation_id: str = "",
+    verification_type: str = "function_call",
 ) -> list[str]:
     """Generate SQLite write code for a single function call.
 
@@ -249,7 +250,7 @@ def _generate_sqlite_write_code(
         f'{inner_indent}                _cf_pstmt{id_pair}.setString(6, "{inv_id_str}");',
         f"{inner_indent}                _cf_pstmt{id_pair}.setLong(7, _cf_dur{id_pair});",
         f"{inner_indent}                _cf_pstmt{id_pair}.setBytes(8, _cf_serializedResult{id_pair});",
-        f'{inner_indent}                _cf_pstmt{id_pair}.setString(9, "function_call");',
+        f'{inner_indent}                _cf_pstmt{id_pair}.setString(9, "{verification_type}");',
         f"{inner_indent}                _cf_pstmt{id_pair}.executeUpdate();",
         f"{inner_indent}            }}",
         f"{inner_indent}        }}",
@@ -349,14 +350,18 @@ def wrap_target_calls_with_treesitter(
         if is_void:
             bare_call_stmt = f"{call['full_call']};"
             # For void methods, serialize the post-call state to capture side effects.
-            # For instance methods (receiver is a variable), serialize the receiver.
-            # For static methods (receiver is a class name), serialize the arguments
-            # since the class name itself is not a value and can't be cast to Object.
+            # We always serialize the arguments (which are mutated in place).
+            # For instance methods, we also include the receiver to capture object state changes.
+            # For static methods, the receiver is a class name (not a value), so args only.
             is_static_call = receiver != "this" and receiver[:1].isupper()
-            if is_static_call and arg_texts:
-                serialize_target = f"new Object[]{{{', '.join(arg_texts)}}}"
+            parts: list[str] = []
+            if not is_static_call:
+                parts.append(receiver)
+            parts.extend(arg_texts)
+            if parts:
+                serialize_target = f"new Object[]{{{', '.join(parts)}}}"
             else:
-                serialize_target = f"(Object) {receiver}"
+                serialize_target = "new Object[]{}"
             if precise_call_timing:
                 serialize_stmt = f"_cf_serializedResult{iter_id}_{call_counter} = com.codeflash.Serializer.serialize({serialize_target});"
                 start_stmt = f"_cf_start{iter_id}_{call_counter} = System.nanoTime();"
@@ -418,7 +423,14 @@ def wrap_target_calls_with_treesitter(
                         f"    {serialize_stmt}",
                     ]
                 finally_block = _generate_sqlite_write_code(
-                    iter_id, call_counter, "", class_name, func_name, test_method_name, invocation_id=inv_id
+                    iter_id,
+                    call_counter,
+                    "",
+                    class_name,
+                    func_name,
+                    test_method_name,
+                    invocation_id=inv_id,
+                    verification_type="void_state" if is_void else "function_call",
                 )
                 all_lines = [*var_decls, start_marker, *try_block, *finally_block]
                 replacement = (