Skip to content

fix: guard Java replacement against wrong-method-name candidates and anonymous-class method hoisting#1704

Open
misrasaurabh1 wants to merge 2 commits intoomni-javafrom
fix/java/wrong-method-name-replacement
Open

fix: guard Java replacement against wrong-method-name candidates and anonymous-class method hoisting#1704
misrasaurabh1 wants to merge 2 commits intoomni-javafrom
fix/java/wrong-method-name-replacement

Conversation

@misrasaurabh1
Copy link
Contributor

Summary

This PR fixes two bugs in _parse_optimization_source (Java replacement logic) that caused Maven compilation failures when running codeflash on aerospike-client-java.

Bug 1 — Standalone method with wrong name replaces target

Root cause: When the LLM generates a standalone method whose name does not match the optimisation target (e.g. generates unpackMap for target unpackObjectMap, or sizeTxn for target estimateKeySize), the code fell back to using the entire generated snippet as target_method_source. This silently replaced the target with the wrong method body, producing:

  • A duplicate definition of the wrong method
  • Removal of the target method, breaking all callers

Observed errors in aerospike-client-java log:

[ERROR] method unpackMap() is already defined in class Unpacker
[ERROR] cannot find symbol: method unpackObjectMap(byte[],int,int) 
[ERROR] method sizeTxn(...) is already defined in class Command

Fix: After parsing standalone (class-free) code, verify at least one discovered method matches target_method_name. If no match is found, set target_method_source = "" and log a warning. A guard in replace_function returns the original source unchanged when target_method_source is empty. The same guard applies to the full-class path (class found, but target method absent).

Bug 2 — Anonymous inner-class methods hoisted as top-level helpers

Root cause: When an optimised method returned an anonymous class (e.g. keySetIterator returning new Iterator<LuaValue>() { ... }), tree-sitter's recursive walk found the anonymous class's hasNext, next, and remove method_declaration nodes and classified them as helpers to insert at the outer-class level. The hoisted methods:

  • Carried @Override annotations matching nothing in the outer class
  • Referenced local variables (it) only in scope inside the optimised method body

Observed errors in aerospike-client-java log:

[ERROR] LuaMap.java:[148,5] method does not override or implement a method from a supertype
[ERROR] LuaMap.java:[150,16] cannot find symbol: variable it

Fix: When extracting helpers, skip any method whose line range is entirely contained within the target method's line range — such methods belong to anonymous/nested classes inside the method body.

Test plan

  • TestWrongMethodNameGeneration::test_standalone_wrong_method_name_leaves_source_unchanged — verifies the Unpacker bug is fixed
  • TestWrongMethodNameGeneration::test_class_wrapper_with_wrong_target_method_leaves_source_unchanged — verifies the Command bug is fixed
  • TestAnonymousInnerClassMethods::test_anonymous_iterator_methods_not_hoisted_to_class — verifies the LuaMap bug is fixed
  • All 624 existing Java tests continue to pass

🤖 Generated with Claude Code

misrasaurabh1 and others added 2 commits March 2, 2026 03:35
…anonymous-class method hoisting

Two bugs in _parse_optimization_source (replacement.py) caused Maven compilation
failures when codeflash optimised aerospike-client-java:

Bug 1 – standalone method with wrong name replaces target
When the LLM generated a standalone method whose name did not match the
optimisation target (e.g. generated `unpackMap` for target `unpackObjectMap`,
or generated `sizeTxn` for target `estimateKeySize`), the function fell back to
using the entire generated snippet as `target_method_source`.  This silently
replaced the target with the wrong method, producing:
  • a duplicate definition of the wrong method
  • removal of the target method (breaking all callers)

Fix: after parsing standalone (class-free) code, verify that at least one
discovered method matches the target name.  If no match is found, set
`target_method_source` to the empty string and log a warning.  A corresponding
guard in `replace_function` returns the original source unchanged when
`target_method_source` is empty.

The same guard is applied to the full-class path: if the generated class does
not contain the target method, the candidate is also rejected.

Bug 2 – anonymous inner-class methods hoisted as top-level helpers
When an optimised method returned an anonymous class (e.g. `keySetIterator`
returning `new Iterator<LuaValue>() { … }`), tree-sitter's recursive walk
found the anonymous class's `hasNext`, `next`, and `remove` method_declaration
nodes and classified them as helpers to be inserted at the outer-class level.
The inserted methods carried `@Override` annotations that matched nothing in the
outer class and referenced local variables (`it`) that were only in scope inside
the optimised method body, producing compilation errors.

Fix: when extracting helpers from the optimised class, skip any method whose
line range is entirely contained within the target method's line range.  Such
methods belong to anonymous/nested classes inside the method body and must not
be hoisted out as standalone class members.

Tests added for both bugs in TestWrongMethodNameGeneration and
TestAnonymousInnerClassMethods.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@claude
Copy link
Contributor

claude bot commented Mar 2, 2026

Duplicate Code Detection Results

Found 1 duplicate requiring attention:


1. JAR Finder Functions (HIGH Confidence)

What's duplicated: Three functions search for the codeflash-runtime-1.0.0.jar file in nearly identical locations using nearly identical logic.

Locations:

  • codeflash/languages/java/test_runner.py:64-95_find_runtime_jar()
  • codeflash/languages/java/line_profiler.py:566-586find_agent_jar()
  • codeflash/languages/java/comparator.py:28-74_find_comparator_jar() (includes additional project-root search logic)

Shared logic:
All three search for the same JAR file in:

  1. Local Maven repository: ~/.m2/repository/com/codeflash/codeflash-runtime/1.0.0/codeflash-runtime-1.0.0.jar
  2. Package resources: Path(__file__).parent / "resources" / "codeflash-runtime-1.0.0.jar"
  3. Development build directory: codeflash-java-runtime/target/codeflash-runtime-1.0.0.jar

Suggestion: Extract to a shared utility function in codeflash/languages/java/build_tools.py (which already has Maven-related utilities like find_maven_executable). Then update all three call sites to use this shared function.


Not Duplicates (Intentional Parallel Implementation)

The following similar patterns are NOT duplicates — they're intentionally parallel implementations for different languages:

  • Compare test results (comparator.py for Java/JavaScript/Python) — Each implements language-specific comparison logic (Java uses a JAR subprocess, JavaScript uses Node.js, Python uses Python comparator). This is expected architecture for multi-language support.
  • Replace function (replacement.py for Java vs code_replacer.py for Python) — Each handles language-specific AST manipulation and code replacement. Consolidating these would break the language abstraction.

@claude
Copy link
Contributor

claude bot commented Mar 2, 2026

PR Review Summary

Prek Checks

  • ruff check: Passed
  • ruff format: Fixed — 2 files reformatted (replacement.py, test_runner.py), committed and pushed in 7dd25a08

Mypy

  • No new type errors introduced by this PR. Pre-existing issues in replacement.py:194 (get_target_class_and_body missing return type) and standard test-method annotation omissions (project convention) remain unchanged.

Code Review

No critical issues found. The changes are well-structured:

  1. Bug 1 fix (wrong method name): Correctly validates standalone and class-wrapped generated code against the target method name. Empty target_method_source signals an invalid candidate, and replace_function returns the original source unchanged.

  2. Bug 2 fix (anonymous inner-class hoisting): Correctly skips methods whose line range (start_line..end_line) is entirely contained within the target method — preventing anonymous class methods from being extracted as top-level helpers.

  3. Guard in replace_function: Early return when target_method_source is empty prevents downstream processing of invalid candidates.

  4. Tests: 3 new focused test classes cover all three bug scenarios. All 32 tests pass; the 3 new tests properly fail on the base branch, confirming the bugs were real.

Test Coverage

File Base (omni-java) PR Delta
codeflash/languages/java/replacement.py 63% (207/330) 61% (209/342) -2pp
tests/test_languages/test_java/test_replacement.py N/A (new tests added) N/A (test file)

Coverage analysis for changed code:

  • _parse_optimization_source (main fix): 100% coverage — all new branches covered
  • replace_function (guard): 88% coverage — new guard at line 313 covered
  • The -2pp overall drop is due to the file gaining 12 new statements (342 vs 330) while the uncovered pre-existing functions (replace_method_body, insert_method, remove_method, remove_test_functions, add_runtime_comments) remain at 0%. All new code added by this PR is fully exercised by tests.

Optimization PRs

Checked 6 codeflash-ai[bot] PRs targeting omni-java (#1703, #1637, #1634, #1633, #1628, #1611). All have CI failures (unit-tests, JS e2e, or Snyk). None eligible for merge.


Last updated: 2026-03-02T03:47Z

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant