Skip to content

feat: support void method optimization in Java pipeline#1980

Open
HeshamHM28 wants to merge 5 commits intomainfrom
cf-java-void-optimization
Open

feat: support void method optimization in Java pipeline#1980
HeshamHM28 wants to merge 5 commits intomainfrom
cf-java-void-optimization

Conversation

@HeshamHM28
Copy link
Copy Markdown
Contributor

Summary

  • Fix Java void method instrumentation to generate compilable code for both static and instance void methods
  • For static void calls (e.g., InPlaceSorter.bubbleSortInPlace(arr)), serialize the arguments instead of the class name, which is not a value and can't be cast to Object
  • For instance void calls (e.g., sorter.bubbleSortInPlace(arr)), serialize the receiver object to capture side effects
  • Add void-specific discovery, comparison, and assert-removal support across the Java pipeline
  • Add e2e test and CI workflow for void function optimization

HeshamHM28 and others added 2 commits April 2, 2026 11:55
Discover void methods, instrument them by serializing the receiver instead
of a return value, and treat all-null comparisons as equivalent.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions github-actions bot added the workflow-modified This PR modifies GitHub Actions workflows label Apr 3, 2026
@claude
Copy link
Copy Markdown
Contributor

claude bot commented Apr 3, 2026

Claude finished @HeshamHM28's task in 10m 46s —— View job


PR Review Summary

Tasks

  • Triage PR size — LARGE (140+ lines of production code changed)
  • Lint and typecheck — prek passes; mypy errors in changed files are pre-existing
  • Resolve stale threads — none found
  • Code review
  • Duplicate detection
  • Test coverage
  • Merge optimization PRs — none open

Prek Checks

uv run prek run --from-ref origin/main passes cleanly. The mypy errors reported in codeflash/models/models.py and codeflash/discovery/functions_to_optimize.py are pre-existing (confirmed by diffing — only 1 line added to each file).


Code Review

Bug: Non-precise timing void path generates undeclared variable reference

codeflash/languages/java/instrumentation.py:438-440

elif is_void:
    replacement = f"{bare_call_stmt} {serialize_stmt}"

In this path (precise_call_timing=False), serialize_stmt is:

f"_cf_serializedResult{iter_id} = com.codeflash.Serializer.serialize({serialize_target});"

But _cf_serializedResult{iter_id} is never declared in non-precise mode — only _cf_serializedResult{iter_id}_{call_counter} is declared (lines 399/406). This would produce a compile error if wrap_target_calls_with_treesitter is ever called with precise_call_timing=False for a void method.

Currently unreachable in production (_add_behavior_instrumentation always passes precise_call_timing=True), but it's misleading dead code. Fix this →

Bug: Static-imported void methods serialized as instance calls

codeflash/languages/java/instrumentation.py:356

is_static_call = receiver != "this" and receiver[:1].isupper()

When a void method is called via static import without an explicit receiver — e.g., sort(data) — tree-sitter finds no object node, so receiver defaults to "this" (line 517). This makes is_static_call = False, so the test class instance (this) is included in the serialized array. Serializing the test class captures no meaningful state about the method under test.

This affects static imports (import static com.example.InPlaceSorter.bubbleSortInPlace) when called as bare bubbleSortInPlace(arr). Fix this →

Minor: var_with_cast computed but unused for void methods

codeflash/languages/java/instrumentation.py:348

var_with_cast = f"({cast_type}){var_name}" if cast_type else var_name

var_with_cast is computed unconditionally but never referenced in the is_void=True branch. Not a correctness issue, just dead computation.

Design note: target_return_type in transform_java_assertions has no production caller

codeflash/languages/java/remove_asserts.py:1257

The new target_return_type parameter in transform_java_assertions is not passed by any production call site (strip_java_assertions uses mode="strip" which already short-circuits, and remove_assertions_from_test doesn't pass it). The parameter is useful for tests and direct callers, but it might be worth noting in the docstring that production callers rely on instrument_existing_test which extracts the return type internally.

Positive findings

  • The is_static_call naming convention heuristic (receiver[:1].isupper()) is consistent with test_discovery.py:438 and import_resolver.py:219.
  • VOID_STATE = "void_state" is correctly added to VerificationType (resolves mashraf-222's finding from their Apr 6 review).
  • The discover_functions change correctly disables require_return=True for Java, allowing void method discovery.
  • Test coverage for instrumentation.py is 83%, which is solid.

Duplicate Detection

No duplicate logic detected across Python/JavaScript/Java modules.


Test Coverage

  • codeflash/languages/java/instrumentation.py: 83% — acceptable for a complex codegen module.
  • codeflash/languages/java/remove_asserts.py: 12% against instrumentation tests alone (broader test suite in test_java_assertion_removal.py covers it separately).
  • 443 lines of new instrumentation tests added, including TestVoidMethodInstrumentation — good coverage of the happy path.
  • The non-precise timing void path (dead code at line 438-440) is not tested.

@codeflash-ai
Copy link
Copy Markdown
Contributor

codeflash-ai bot commented Apr 3, 2026

⚡️ Codeflash found optimizations for this PR

📄 25% (0.25x) speedup for JavaAssertTransformer._generate_replacement in codeflash/languages/java/remove_asserts.py

⏱️ Runtime : 886 microseconds 708 microseconds (best of 250 runs)

A dependent PR with the suggested changes has been created. Please review:

If you approve, it will be merged into this PR (branch cf-java-void-optimization).

Static Badge

claude bot added a commit that referenced this pull request Apr 3, 2026
…2026-04-03T13.47.39

⚡️ Speed up method `JavaAssertTransformer._generate_replacement` by 25% in PR #1980 (`cf-java-void-optimization`)
@codeflash-ai
Copy link
Copy Markdown
Contributor

codeflash-ai bot commented Apr 3, 2026

@HeshamHM28 HeshamHM28 force-pushed the cf-java-void-optimization branch from 10cfedf to 3bc4941 Compare April 3, 2026 14:50
@HeshamHM28 HeshamHM28 marked this pull request as ready for review April 3, 2026 17:06
@mashraf-222
Copy link
Copy Markdown
Contributor

Review Finding: Missing VOID_STATE enum value

File: codeflash/models/models.py:737VerificationType enum

The instrumentation writes verification_type="void_state" to SQLite, but VerificationType only has FUNCTION_CALL, INIT_STATE_FTO, and INIT_STATE_HELPER. No VOID_STATE member exists.

What happens: Every behavioral test result row for void methods hits ValueError: 'void_state' is not a valid VerificationType in parse_test_output.py:565. This produces ~255 exception tracebacks per optimization run. The pipeline still works because:

  1. The try/except at line 568 swallows each error
  2. The Java-side Comparator.java reads SQLite directly (bypasses Python parsing entirely)
  3. Test pass/fail data comes from XML parsing, not SQLite

Verified with E2E: Ran InPlaceSorter.bubbleSortInPlace (static void) twice — once without the fix (255 ValueErrors, 0 SQLite rows parsed) and once after adding VOID_STATE = "void_state" to the enum (0 errors, 17 rows parsed per candidate). Both runs found an optimization successfully.

Fix: One line — add VOID_STATE = "void_state" to the VerificationType enum in models/models.py.

Not blocking correctness, but should be fixed for clean logs and proper Python-side test result availability.

Instrumentation writes verification_type="void_state" for void methods,
but the enum lacked this value, causing ValueError on every SQLite row parse.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

workflow-modified This PR modifies GitHub Actions workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants