Skip to content

Conversation

@misrasaurabh1
Copy link
Contributor

@misrasaurabh1 misrasaurabh1 commented Jun 28, 2025

PR Type

Enhancement


Description

  • Wrap optimizer logic in try/finally

  • Ensure cleanup_generated_files always called

  • Invoke cleanup_temporary_paths after each iteration


Changes diagram

flowchart LR
  A["Start function optimization"] --> B["create_function_optimizer"]
  B --> C["optimize_function"]
  C --> D{"Optimization result"}
  D -->|any| E["cleanup_generated_files"]
  E --> F["cleanup_temporary_paths"]
Loading

Changes walkthrough 📝

Relevant files
Error handling
optimizer.py
Add cleanup and error-safe block                                                 

codeflash/optimization/optimizer.py

  • Wrapped optimizer creation and execution
  • Moved existing optimize logic into try block
  • Added cleanup_generated_files in finally
  • Added cleanup_temporary_paths after cleanup
  • +29/-23 

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @github-actions
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Exception Handling

    Any exceptions raised in create_function_optimizer or optimize_function will propagate and break the loop, stopping further optimizations. Consider catching and logging errors to allow the loop to continue processing remaining functions.

    function_optimizer = None
    try:
        function_optimizer = self.create_function_optimizer(
            function_to_optimize,
            function_to_tests=function_to_tests,
            function_to_optimize_source_code=validated_original_code[original_module_path].source_code,
            function_benchmark_timings=function_benchmark_timings,
            total_benchmark_timings=total_benchmark_timings,
            original_module_ast=original_module_ast,
            original_module_path=original_module_path,
        )
    
        self.current_function_optimizer = (
            function_optimizer  # needed to clean up from the outside of this function
        )
        best_optimization = function_optimizer.optimize_function()
        if self.functions_checkpoint:
            self.functions_checkpoint.add_function_to_checkpoint(
                function_to_optimize.qualified_name_with_modules_from_root(self.args.project_root)
            )
        if is_successful(best_optimization):
            optimizations_found += 1
        else:
            logger.warning(best_optimization.failure())
            console.rule()
            continue
    finally:
        if function_optimizer is not None:
            function_optimizer.cleanup_generated_files()
        self.cleanup_temporary_paths()
    State Management

    The attribute current_function_optimizer is set inside the try block but never reset on failure. This could leave a stale optimizer reference; consider clearing it in the finally block.

    self.current_function_optimizer = (
        function_optimizer  # needed to clean up from the outside of this function
    )

    @github-actions
    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Catch and log optimization errors

    Add an except Exception block before finally to catch and log errors during
    optimizer creation or execution. This prevents a single failure from aborting the
    entire optimization run.

    codeflash/optimization/optimizer.py [292-320]

     try:
         function_optimizer = self.create_function_optimizer(...)
    -    self.current_function_optimizer = (
    -        function_optimizer  # needed to clean up from the outside of this function
    -    )
    +    self.current_function_optimizer = function_optimizer
         best_optimization = function_optimizer.optimize_function()
         ...
         if is_successful(best_optimization):
             optimizations_found += 1
         else:
             logger.warning(best_optimization.failure())
             console.rule()
             continue
    +except Exception as err:
    +    logger.error(f"Error optimizing {function_to_optimize.qualified_name}: {err}", exc_info=True)
    +    continue
     finally:
         if function_optimizer is not None:
             function_optimizer.cleanup_generated_files()
         self.cleanup_temporary_paths()
    Suggestion importance[1-10]: 8

    __

    Why: Adding an except Exception block prevents a single failure during optimizer creation or execution from aborting the entire run, enhancing robustness.

    Medium
    Protect cleanup from errors

    Wrap each cleanup call in its own try/except block so that failures during cleanup
    don’t mask the original exception or stop the loop. Log any cleanup errors for
    troubleshooting.

    codeflash/optimization/optimizer.py [317-320]

     finally:
         if function_optimizer is not None:
    -        function_optimizer.cleanup_generated_files()
    -    self.cleanup_temporary_paths()
    +        try:
    +            function_optimizer.cleanup_generated_files()
    +        except Exception as cleanup_err:
    +            logger.error("Failed to cleanup generated files", exc_info=cleanup_err)
    +    try:
    +        self.cleanup_temporary_paths()
    +    except Exception as cleanup_err:
    +        logger.error("Failed to cleanup temporary paths", exc_info=cleanup_err)
    Suggestion importance[1-10]: 7

    __

    Why: Wrapping cleanup calls in separate try/except blocks ensures cleanup failures don’t mask original exceptions or interrupt the loop, improving reliability.

    Medium
    General
    Reset optimizer reference

    Reset the self.current_function_optimizer reference after each iteration to avoid
    retaining stale optimizer instances. This ensures that subsequent cleanup operations
    won’t operate on an outdated optimizer.

    codeflash/optimization/optimizer.py [317-320]

     finally:
         if function_optimizer is not None:
             function_optimizer.cleanup_generated_files()
         self.cleanup_temporary_paths()
    +    self.current_function_optimizer = None
    Suggestion importance[1-10]: 5

    __

    Why: Resetting current_function_optimizer prevents holding onto stale optimizer instances across iterations, reducing potential side effects.

    Low

    @misrasaurabh1 misrasaurabh1 requested a review from KRRT7 June 30, 2025 21:30
    @misrasaurabh1 misrasaurabh1 merged commit 6b1a24c into main Jun 30, 2025
    16 checks passed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    1 participant