Skip to content

Conversation

@aseembits93
Copy link
Contributor

@aseembits93 aseembits93 commented Aug 14, 2025

User description

In rare situations, line profiler and refinement APIs would not have returned and all original candidates would be processed and the loop would exit. This PR would solve this situation.


PR Type

Enhancement


Description

  • Ensure all candidates processed before exit

  • Append line profiler results when depleted

  • Append refinement candidates when done profiling

  • Simplify loop with processing flags


Diagram Walkthrough

flowchart LR
  Start["Initial Candidates"] --> Process["Process Candidates"]
  Process --> Check1{"Candidates Exhausted?"}
  Check1 -- "No" --> Process
  Check1 -- "Yes" --> LP["Add Line Profiler Results"]
  LP --> Process
  Check1 -- "Yes" after LP --> Refine["Add Refinement Results"]
  Refine --> Process
  Check1 -- "Yes" after Refine --> End["Exit Loop"]
Loading

File Walkthrough

Relevant files
Enhancement
aiservice.py
Update line profiler logging                                                         

codeflash/api/aiservice.py

  • Enhanced logging with profiler context
  • Expanded candidate count info message
+1/-1     
function_optimizer.py
Refactor candidate processing loop                                             

codeflash/optimization/function_optimizer.py

  • Replaced loop with state-driven while True
  • Introduced line_profiler_done and refinement_done
  • Dynamically enqueue profiler and refinement candidates
  • Removed redundant post-loop logic
+43/-46 

@github-actions
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

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

Missing try block

There's an except KeyboardInterrupt added without a corresponding try block, which will cause a syntax error or unexpected behavior. Ensure the try is properly placed to wrap the loop or relevant code.

    except KeyboardInterrupt as e:
        self.write_code_and_helpers(
            self.function_to_optimize_source_code, original_helper_code, self.function_to_optimize.file_path
        )
        logger.exception(f"Optimization interrupted: {e}")
        raise
if not valid_optimizations:
Undefined future variable

The code references future_line_profile_results during the empty-candidates condition, but it's not shown being initialized or checked for None. Validate that this future is always created before use to avoid NameError.

if not line_profiler_done:
    logger.debug("all candidates processed, await candidates from line profiler")
    concurrent.futures.wait([future_line_profile_results])
    line_profile_results = future_line_profile_results.result()
    candidates.extend(line_profile_results)
Potential infinite loop

The while True loop may hang if no new candidates are produced by the line profiler or refinement steps. Verify that break conditions cover all edge cases to prevent deadlocks.

    continue
if line_profiler_done and not refinement_done:
    concurrent.futures.wait(future_all_refinements)
    refinement_response = []
    for future_refinement in future_all_refinements:
        possible_refinement = future_refinement.result()
        if len(possible_refinement) > 0:  # if the api returns a valid response
            refinement_response.append(possible_refinement[0])
    candidates.extend(refinement_response)
    original_len += len(refinement_response)
    logger.info(
        f"Added {len(refinement_response)} candidates from refinement, total candidates now: {original_len}"
    )
    refinement_done = True
    continue
if line_profiler_done and refinement_done:
    logger.debug("everything done, exiting")
    break

@github-actions
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Handle future exceptions safely

Wrap calls to .result() on futures in try/except blocks to catch exceptions from
asynchronous tasks and log any errors without crashing the optimization loop.

codeflash/optimization/function_optimizer.py [412-426]

 concurrent.futures.wait([future_line_profile_results])
-line_profile_results = future_line_profile_results.result()
+try:
+    line_profile_results = future_line_profile_results.result()
+except Exception as e:
+    logger.exception(f"Failed to get line profiler results: {e}")
+    line_profile_results = []
 ...
+refinement_response = []
 for future_refinement in future_all_refinements:
-    possible_refinement = future_refinement.result()
+    try:
+        possible_refinement = future_refinement.result()
+        if possible_refinement:
+            refinement_response.append(possible_refinement[0])
+    except Exception as e:
+        logger.exception(f"Refinement future failed: {e}")
Suggestion importance[1-10]: 7

__

Why: Wrapping .result() calls in try/except prevents unhandled exceptions from diverging tasks, improving reliability without altering functionality.

Medium
General
Add timeout to future waits

Add a timeout to the concurrent.futures.wait calls and log a warning if futures do
not complete within the timeout to avoid blocking indefinitely.

codeflash/optimization/function_optimizer.py [412-423]

-concurrent.futures.wait([future_line_profile_results])
+done, not_done = concurrent.futures.wait([future_line_profile_results], timeout=300)
+if not_done:
+    logger.warning("Timeout waiting for line profiler results")
 ...
-concurrent.futures.wait(future_all_refinements)
+done_all, not_all = concurrent.futures.wait(future_all_refinements, timeout=300)
+if not_all:
+    logger.warning("Timeout waiting for refinement futures")
Suggestion importance[1-10]: 6

__

Why: Specifying a timeout on concurrent.futures.wait avoids potential infinite blocking, enhancing robustness in asynchronous flows.

Low
Broaden interrupt cleanup scope

Move the try/except KeyboardInterrupt to wrap the entire loop and any following
logic so that interruptions after breaking also perform cleanup.

codeflash/optimization/function_optimizer.py [406-602]

-while True:
-    try:
+try:
+    while True:
         ...
-    except KeyboardInterrupt as e:
-        self.write_code_and_helpers(
-            self.function_to_optimize_source_code, original_helper_code, self.function_to_optimize.file_path
-        )
-        logger.exception(f"Optimization interrupted: {e}")
-        raise
+except KeyboardInterrupt as e:
+    self.write_code_and_helpers(
+        self.function_to_optimize_source_code, original_helper_code, self.function_to_optimize.file_path
+    )
+    logger.exception(f"Optimization interrupted: {e}")
+    raise
Suggestion importance[1-10]: 5

__

Why: Extending the KeyboardInterrupt handler to encompass the full loop and subsequent logic ensures cleanup occurs in all interruption scenarios.

Low

@aseembits93 aseembits93 enabled auto-merge August 15, 2025 00:07
@aseembits93 aseembits93 merged commit 51c5e5a into main Aug 15, 2025
35 of 36 checks passed
@aseembits93 aseembits93 deleted the candidate-loop-prevent-exit branch August 17, 2025 19:26
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.

3 participants