refactor: extract PythonFunctionOptimizer subclass#1699
Conversation
…/optimizer.py Move AST parsing, code normalization, and imported module validation out of the language-agnostic Optimizer class into dedicated functions.
Consolidate Python AST resolution through the shared helper instead of importing get_first_top_level_function_or_method_ast directly.
…izer Move 6 Python-specific methods into PythonFunctionOptimizer in languages/python/function_optimizer.py. Base class gets no-op defaults; Optimizer.create_function_optimizer dispatches to the subclass when is_python().
…e ast to TYPE_CHECKING - Base class keeps the language-routing replacement logic (used by both Python and JS); Python subclass adds unused-helper revert on top via super() - Tests that exercise Python-specific replace+revert use PythonFunctionOptimizer - Move `ast` to TYPE_CHECKING in optimizer.py (fixes prek)
⚡️ Codeflash found optimizations for this PR📄 3,869% (38.69x) speedup for
|
Move the Python-specific test instrumentation branch (~80 lines using inject_profiling_into_existing_test) to the subclass. Base class keeps the JS/TS branch using language_support.instrument_existing_test as the default.
Move async decorator instrumentation (4 sites) and codeflash capture instrumentation (2 sites) into overridable hooks. Base class no-ops; PythonFunctionOptimizer provides the implementations.
PythonSupport.instrument_existing_test() already delegates to inject_profiling_into_existing_test(), so the base class handles Python correctly through the language_support abstraction.
PR Review SummaryPrek Checks
Mypy
Code Review1 bug found (inline comment posted):
No other critical issues, security vulnerabilities, breaking API changes, or documentation staleness found. Architecture docs were properly updated. Test Coverage
Overall project coverage: 79% (2483 passed, 57 skipped, 8 failed in unrelated Notes:
Optimization PRs
Last updated: 2026-03-02T08:00:00Z |
…izer Replace 2 is_python() guard sites around coverage_critic() calls with a should_check_coverage() hook, defaulting to False in the base class and True in PythonFunctionOptimizer.
…timized_candidate
…lts into PythonFunctionOptimizer
…timizer The factory method was resolving the function AST from original_module_ast before constructing PythonFunctionOptimizer, duplicating the _resolve_function_ast hook. Now the hook handles resolution and the factory checks the result post-construction.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
- Add clarifying comment on shared replace_function_definitions_in_module import - Remove misleading alias in test_unused_helper_revert.py, use PythonFunctionOptimizer directly - Align base line_profiler_step return type to dict[str, Any] - Fix latent bug: handle non-empty TestResults in line_profiler_step
|
@claude ensure codeflash/blob/main/.claude/rules/architecture.md Is up to date |
Add languages/python/function_optimizer.py and languages/python/optimizer.py to the directory tree and Key Entry Points table. Co-authored-by: Kevin Turcios <KRRT7@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@claude why is topological-sort-worktree-optimization failing? |
The refactoring changed the log message from 'Discovered' to 'Instrumented', breaking the E2E test regex in end_to_end_test_utilities.py:225. Co-authored-by: Kevin Turcios <KRRT7@users.noreply.github.com>
Log "Discovered N existing unit test files" after counting tests, and "Instrumented N existing unit test files" after injecting profiling. Python E2E harness matches "Discovered", JS harness matches "Instrumented".
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@claude double check that the style: fix unused loop variable in function_optimizer.py isn't a regression between our intention in this branch and what's on main |
…y dict for all TestResults The return statement was outside the inner if-not block, causing all TestResults (even populated ones) to return an empty dict. Co-authored-by: Kevin Turcios <KRRT7@users.noreply.github.com>
|
@claude work on the failures in js-esm-async-optimization |
|
Claude finished @KRRT7's task in 13m 15s —— View job Investigating
|
| Candidate | Behavior Tests | Speedup |
|---|---|---|
| #1 | Matched ✅ | 0.999X (no improvement) |
| #2 | Matched ✅ | 1.001X (no improvement) |
| #3 | Matched ✅ | 1.000X (no improvement) |
| #4 | Did NOT match ❌ | N/A (used Promise.all → changed error semantics) |
The AI generated 4 optimization candidates for processItemsSequential. Candidates 1-3 passed behavioral tests but showed zero speedup (~1.0X). Candidate 4 used Promise.all parallelization which is faster but changes error-handling behavior (sequential error propagation vs parallel), so it was correctly rejected. Result: "No optimizations found."
This is NOT caused by this PR. I verified that zero JS-related files were changed:
code_to_optimize/js/code_to_optimize_js_esm/— no difftests/scripts/end_to_end_test_js_esm_async.py— no difftests/scripts/end_to_end_test_utilities_js.py— no diffpackages/codeflash/runtime/capture.js— no diffcodeflash/languages/javascript/— no diff
I also audited the base class refactoring to confirm all JS code paths are preserved:
compare_candidate_results→ same SQLite comparison logic for JSshould_skip_sqlite_cleanup→ sameBEHAVIOR or iteration==0logic for non-Pythoninstrument_async_for_mode→ correctly no-op for JS (wasis_python()guarded before)collect_async_metrics→ correctly returns(None, None)for JS (wasis_python()guarded before)parse_line_profile_test_results→ returnsTestResults([])for JS (same as before)
Root cause: processItemsSequential is inherently hard to optimize — the only meaningful optimization (parallelization via Promise.all) changes error semantics. The AI-generated candidates that preserve behavior show no speedup. This is a non-deterministic E2E test that depends on the AI finding a correct-and-faster optimization.
Recommendation: Re-run the workflow. This test is non-deterministic by nature (depends on AI-generated optimizations). The last run was cancelled before all 3 retries completed.
--- |
1 participant
Summary
FunctionOptimizerinto aPythonFunctionOptimizersubclass inlanguages/python/function_optimizer.pyOptimizer.create_function_optimizerdispatches to the subclass whenis_python()I want to implement some new optimization strategies for Python, and this separation makes it easier to add Python-specific behavior without further bloating the base class.
Test plan
uv run ruff check+uv run ruff format --checkon all 3 filesfrom codeflash.languages.python.function_optimizer import PythonFunctionOptimizer