From 77c79d7ce192df9a27cf234928a9c7193ba89990 Mon Sep 17 00:00:00 2001 From: Mohamed Ashraf Date: Wed, 1 Apr 2026 15:54:18 +0000 Subject: [PATCH] fix: reject Java optimizations with unused additions and unchanged target method Adds a wiring check in replace_function() that detects when the AI generates "optimizations" adding fields/helpers that the target method never references. Previously these passed through because benchmark noise produced fake speedups. Co-Authored-By: Claude Opus 4.6 --- codeflash/languages/java/replacement.py | 71 ++++++ .../test_java/test_replacement.py | 237 ++++++++++++++++++ 2 files changed, 308 insertions(+) diff --git a/codeflash/languages/java/replacement.py b/codeflash/languages/java/replacement.py index 5ed9bf8f1..dd015b746 100644 --- a/codeflash/languages/java/replacement.py +++ b/codeflash/languages/java/replacement.py @@ -158,6 +158,60 @@ def _parse_optimization_source(new_source: str, target_method_name: str, analyze ) +def _has_unused_additions(parsed: ParsedOptimization, original_method_source: str, analyzer: JavaAnalyzer) -> bool: + """Check if an optimization adds fields/helpers that the target method never references. + + Returns True if the optimization has additions (fields/helpers) that are completely + unreferenced by the target method body. This catches fake optimizations where the AI + adds new code but leaves the target method unchanged. + + Returns False (additions are used / no problem) when: + - There are no additions + - At least one addition's identifier appears in the target method body + - The target method itself was changed + """ + has_additions = bool(parsed.new_fields or parsed.helpers_before_target or parsed.helpers_after_target) + if not has_additions: + return False + + # Normalize whitespace for comparison: strip leading/trailing whitespace from each line + def normalize_ws(s: str) -> str: + return "\n".join(line.strip() for line in s.strip().splitlines() if line.strip()) + + if normalize_ws(parsed.target_method_source) != normalize_ws(original_method_source): + # Target method was actually changed — additions may or may not be used, + # but the optimization is not a no-op on the target function. + return False + + # Target method is unchanged. Check if any addition identifiers are referenced. + addition_names: set[str] = set() + + for field_src in parsed.new_fields: + dummy_class = f"class __Dummy__ {{\n{field_src}\n}}" + for field_info in analyzer.find_fields(dummy_class): + addition_names.add(field_info.name) + + for helper_src in parsed.helpers_before_target + parsed.helpers_after_target: + for method_info in analyzer.find_methods(helper_src): + addition_names.add(method_info.name) + + if not addition_names: + return False + + # Check if any addition name appears in the target method body as a word boundary match + target_body = parsed.target_method_source + for name in addition_names: + if re.search(rf"\b{re.escape(name)}\b", target_body): + return False # At least one addition is referenced + + logger.info( + "Rejecting optimization: target method '%s' is unchanged and new additions %s are unreferenced.", + "target", + addition_names, + ) + return True + + def _dedent_member(source: str) -> str: """Strip the common leading whitespace from a class member source.""" return textwrap.dedent(source).strip() @@ -459,6 +513,23 @@ def replace_function( logger.error("Could not find method %s in source", func_name) return source + # Extract original method source for comparison + orig_start = (target_method.javadoc_start_line or target_method.start_line) - 1 + orig_end = target_method.end_line + orig_lines = source.splitlines(keepends=True) + original_method_source = "".join(orig_lines[orig_start:orig_end]) + + # Reject optimizations that add unused code while leaving the target method unchanged. + # This catches fake optimizations (e.g., adding a static field that the method never references) + # that produce spurious speedups from benchmark noise. + if _has_unused_additions(parsed, original_method_source, analyzer): + logger.warning( + "Optimization for '%s' adds unreferenced fields/helpers without changing the target method. " + "Returning original source.", + func_name, + ) + return source + # Get the class name for inserting new members class_name = target_method.class_name or function.class_name diff --git a/tests/test_languages/test_java/test_replacement.py b/tests/test_languages/test_java/test_replacement.py index 1bd4f7abb..9bf5ddc3d 100644 --- a/tests/test_languages/test_java/test_replacement.py +++ b/tests/test_languages/test_java/test_replacement.py @@ -1929,3 +1929,240 @@ def test_anonymous_iterator_methods_not_hoisted_to_class(self, tmp_path, java_su } """ assert new_code == expected_code + + +class TestUnusedAdditionsRejection: + """Tests that optimizations adding unused fields/helpers with unchanged target method are rejected.""" + + def test_unchanged_method_with_unused_field_rejected(self, tmp_path: Path, java_support: JavaSupport): + """An optimization that adds a field but doesn't change the method should be rejected.""" + java_file = (tmp_path / "SystemUtils.java").resolve() + original_code = """public class SystemUtils { + public static String getJavaIoTmpdir() { + return System.getProperty("java.io.tmpdir"); + } +} +""" + java_file.write_text(original_code, encoding="utf-8") + + # AI adds NULL_SUPPLIER but doesn't change getJavaIoTmpdir + optimized_markdown = f"""```java:{java_file.relative_to(tmp_path)} +public class SystemUtils {{ + private static final String CACHED_TMPDIR = System.getProperty("java.io.tmpdir"); + + public static String getJavaIoTmpdir() {{ + return System.getProperty("java.io.tmpdir"); + }} +}} +```""" + + optimized_code = CodeStringsMarkdown.parse_markdown_code(optimized_markdown, expected_language="java") + + from codeflash.discovery.functions_to_optimize import FunctionParent, FunctionToOptimize + + function_to_optimize = FunctionToOptimize( + function_name="getJavaIoTmpdir", + file_path=java_file, + starting_line=2, + ending_line=4, + parents=[FunctionParent(name="SystemUtils", type="ClassDef")], + qualified_name="SystemUtils.getJavaIoTmpdir", + is_method=True, + ) + + result = replace_function_definitions_for_language( + function_names=["getJavaIoTmpdir"], + optimized_code=optimized_code, + module_abspath=java_file, + project_root_path=tmp_path, + lang_support=java_support, + function_to_optimize=function_to_optimize, + ) + + # Should reject: method unchanged, field unreferenced + assert result is False + assert java_file.read_text(encoding="utf-8") == original_code + + def test_unchanged_method_with_unused_helper_rejected(self, tmp_path: Path, java_support: JavaSupport): + """An optimization that adds a helper method but doesn't change the target should be rejected.""" + java_file = (tmp_path / "Calculator.java").resolve() + original_code = """public class Calculator { + public int add(int a, int b) { + return a + b; + } +} +""" + java_file.write_text(original_code, encoding="utf-8") + + # AI adds a helper method but doesn't change add() + optimized_markdown = f"""```java:{java_file.relative_to(tmp_path)} +public class Calculator {{ + private static int fastAdd(int a, int b) {{ + return Math.addExact(a, b); + }} + + public int add(int a, int b) {{ + return a + b; + }} +}} +```""" + + optimized_code = CodeStringsMarkdown.parse_markdown_code(optimized_markdown, expected_language="java") + + from codeflash.discovery.functions_to_optimize import FunctionParent, FunctionToOptimize + + function_to_optimize = FunctionToOptimize( + function_name="add", + file_path=java_file, + starting_line=2, + ending_line=4, + parents=[FunctionParent(name="Calculator", type="ClassDef")], + qualified_name="Calculator.add", + is_method=True, + ) + + result = replace_function_definitions_for_language( + function_names=["add"], + optimized_code=optimized_code, + module_abspath=java_file, + project_root_path=tmp_path, + lang_support=java_support, + function_to_optimize=function_to_optimize, + ) + + # Should reject: method unchanged, helper unreferenced + assert result is False + assert java_file.read_text(encoding="utf-8") == original_code + + def test_changed_method_with_used_field_accepted(self, tmp_path: Path, java_support: JavaSupport): + """An optimization that adds a field AND uses it in the changed method should be accepted.""" + java_file = (tmp_path / "SystemUtils.java").resolve() + original_code = """public class SystemUtils { + public static String getJavaIoTmpdir() { + return System.getProperty("java.io.tmpdir"); + } +} +""" + java_file.write_text(original_code, encoding="utf-8") + + # AI adds CACHED_TMPDIR and actually uses it in the method + optimized_markdown = f"""```java:{java_file.relative_to(tmp_path)} +public class SystemUtils {{ + private static final String CACHED_TMPDIR = System.getProperty("java.io.tmpdir"); + + public static String getJavaIoTmpdir() {{ + return CACHED_TMPDIR; + }} +}} +```""" + + optimized_code = CodeStringsMarkdown.parse_markdown_code(optimized_markdown, expected_language="java") + + from codeflash.discovery.functions_to_optimize import FunctionParent, FunctionToOptimize + + function_to_optimize = FunctionToOptimize( + function_name="getJavaIoTmpdir", + file_path=java_file, + starting_line=2, + ending_line=4, + parents=[FunctionParent(name="SystemUtils", type="ClassDef")], + qualified_name="SystemUtils.getJavaIoTmpdir", + is_method=True, + ) + + result = replace_function_definitions_for_language( + function_names=["getJavaIoTmpdir"], + optimized_code=optimized_code, + module_abspath=java_file, + project_root_path=tmp_path, + lang_support=java_support, + function_to_optimize=function_to_optimize, + ) + + # Should accept: method changed to use CACHED_TMPDIR + assert result is True + new_code = java_file.read_text(encoding="utf-8") + assert "CACHED_TMPDIR" in new_code + assert "private static final String CACHED_TMPDIR" in new_code + + def test_changed_method_without_additions_accepted(self, tmp_path: Path, java_support: JavaSupport): + """A normal optimization that just changes the method body should be accepted.""" + java_file = (tmp_path / "Calculator.java").resolve() + original_code = """public class Calculator { + public int add(int a, int b) { + return a + b; + } +} +""" + java_file.write_text(original_code, encoding="utf-8") + + optimized_markdown = f"""```java:{java_file.relative_to(tmp_path)} +public class Calculator {{ + public int add(int a, int b) {{ + return Math.addExact(a, b); + }} +}} +```""" + + optimized_code = CodeStringsMarkdown.parse_markdown_code(optimized_markdown, expected_language="java") + + result = replace_function_definitions_for_language( + function_names=["add"], + optimized_code=optimized_code, + module_abspath=java_file, + project_root_path=tmp_path, + lang_support=java_support, + ) + + # Should accept: method was changed + assert result is True + + def test_unchanged_method_with_used_helper_accepted(self, tmp_path: Path, java_support: JavaSupport): + """Method unchanged but references the new helper — should be accepted (helper IS used).""" + java_file = (tmp_path / "Calculator.java").resolve() + original_code = """public class Calculator { + public int add(int a, int b) { + return a + b; + } +} +""" + java_file.write_text(original_code, encoding="utf-8") + + # AI adds helper AND rewrites method to use it + optimized_markdown = f"""```java:{java_file.relative_to(tmp_path)} +public class Calculator {{ + private static int fastAdd(int a, int b) {{ + return Math.addExact(a, b); + }} + + public int add(int a, int b) {{ + return fastAdd(a, b); + }} +}} +```""" + + optimized_code = CodeStringsMarkdown.parse_markdown_code(optimized_markdown, expected_language="java") + + from codeflash.discovery.functions_to_optimize import FunctionParent, FunctionToOptimize + + function_to_optimize = FunctionToOptimize( + function_name="add", + file_path=java_file, + starting_line=2, + ending_line=4, + parents=[FunctionParent(name="Calculator", type="ClassDef")], + qualified_name="Calculator.add", + is_method=True, + ) + + result = replace_function_definitions_for_language( + function_names=["add"], + optimized_code=optimized_code, + module_abspath=java_file, + project_root_path=tmp_path, + lang_support=java_support, + function_to_optimize=function_to_optimize, + ) + + # Should accept: method changed to call fastAdd + assert result is True