Skip to content

Refinement #555

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 14 commits into
base: main
Choose a base branch
from
Draft

Refinement #555

wants to merge 14 commits into from

Conversation

misrasaurabh1
Copy link
Contributor

@misrasaurabh1 misrasaurabh1 commented Jul 17, 2025

PR Type

Enhancement


Description

  • Introduce AIServiceRefinerRequest and client refinement API

  • Integrate AI-powered refinement in optimize flow

  • Add helper methods for diff and ranking

  • Refactor test env and line profiling steps


Changes diagram

flowchart LR
  A["Initial optimize candidates"] --> B["Collect valid optimizations"]
  B --> C["Call optimize_python_code_refinement"]
  C --> D["Rank by diff length + runtime"]
  D --> E["Return best optimization"]
Loading

Changes walkthrough 📝

Relevant files
Enhancement
aiservice.py
Add AIService refinement request method                                   

codeflash/api/aiservice.py

  • Import ExperimentMetadata and pydantic dataclass
  • Add AIServiceRefinerRequest dataclass
  • Implement optimize_python_code_refinement method
  • Handle API success and error logging
  • +76/-1   
    models.py
    Update BestOptimization fields                                                     

    codeflash/models/models.py

  • Replace helper_functions with code_context in BestOptimization
  • Add line_profiler_test_results field
  • +2/-1     
    function_optimizer.py
    Integrate optimization refinement and helpers                       

    codeflash/optimization/function_optimizer.py

  • Track valid_optimizations and integrate refinement flow
  • Add refine_optimizations, diff_length, ranking helpers
  • Extend determine_best_candidate for refinement and ranking
  • Refactor test env and line profiling into methods
  • +193/-48
    critic.py
    Allow speedup critic no-best-runtime handling                       

    codeflash/result/critic.py

  • Allow best_runtime_until_now to be None in speedup_critic
  • Adjust logic to handle no prior best runtime scenario
  • +4/-1     

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • 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

    Payload Field Mismatch

    The payload uses the key "original_read_only_dependency_code" whereas the dataclass defines "read_only_dependency_code". This naming mismatch may result in missing or incorrect data being sent to the refinement endpoint.

    def optimize_python_code_refinement(self, request: list[AIServiceRefinerRequest]) -> list[str]:
        payload = [
            {
                "original_source_code": opt.original_source_code,
                "original_read_only_dependency_code": opt.original_read_only_dependency_code,
                "original_line_profiler_results": opt.original_line_profiler_results,
                "original_code_runtime": opt.original_code_runtime,
                "optimized_source_code": opt.optimized_source_code,
                "optimized_explanation": opt.optimized_explanation,
                "optimized_line_profiler_results": opt.optimized_line_profiler_results,
                "optimized_code_runtime": opt.optimized_code_runtime,
                "speedup": opt.speedup,
                "trace_id": opt.trace_id,
                "python_version": platform.python_version(),
                "experiment_metadata": opt.experiment_metadata,
                "codeflash_version": codeflash_version,
                "lsp_mode": is_LSP_enabled(),
            }
            for opt in request
    ExperimentMetadata Serialization

    The code passes the ExperimentMetadata dataclass directly in the payload. Ensure it is converted to a JSON-serializable form (e.g., via .dict() or pydantic_encoder) before sending, otherwise serialization may fail.

        "speedup": opt.speedup,
        "trace_id": opt.trace_id,
        "python_version": platform.python_version(),
        "experiment_metadata": opt.experiment_metadata,
        "codeflash_version": codeflash_version,
        "lsp_mode": is_LSP_enabled(),
    }
    Diff Performance

    The diff_length implementation computes full unified diffs on possibly large code strings. This may be slow or memory-intensive. Consider optimizing or limiting diff size for large inputs.

    def diff_length(a: str, b: str) -> int:
        """Compute the length (in characters) of the unified diff between two strings.
    
        Parameters
        ----------
            a (str): Original string.
            b (str): Modified string.
    
        Returns
        -------
            int: Total number of characters in the diff.
    
        """
        # Split input strings into lines for line-by-line diff
        a_lines = a.splitlines(keepends=True)
        b_lines = b.splitlines(keepends=True)
    
        # Compute unified diff
        diff_lines = list(difflib.unified_diff(a_lines, b_lines, lineterm=""))
    
        # Join all lines with newline to calculate total diff length
        diff_text = "\n".join(diff_lines)
    
        return len(diff_text)

    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Fix payload field name

    The payload uses a non-existent field original_read_only_dependency_code; it should
    match the dataclass field read_only_dependency_code. Rename both the key and
    attribute reference to keep them consistent.

    codeflash/api/aiservice.py [241-242]

     {
         "original_source_code": opt.original_source_code,
    -    "original_read_only_dependency_code": opt.original_read_only_dependency_code,
    +    "read_only_dependency_code": opt.read_only_dependency_code,
         ...
     }
    Suggestion importance[1-10]: 9

    __

    Why: The payload key "original_read_only_dependency_code" and attribute opt.original_read_only_dependency_code do not exist on AIServiceRefinerRequest, so renaming to read_only_dependency_code is essential to avoid runtime errors.

    High
    General
    Move refinement block

    This block is inside the while candidates: loop and will never run because the loop
    exits when len(candidates)==0. Move it outside the loop so refinement runs once
    after processing all candidates.

    codeflash/optimization/function_optimizer.py [543-571]

    -if len(candidates) == 0 and len(self.valid_optimizations) > 0 and not refinement_done:
    +# after the while-candidates loop ends
    +if len(self.valid_optimizations) > 0 and not refinement_done:
         # ... refine_optimizations ...
         candidates.extend(more_opt_candidates)
         refinement_done = True
    Suggestion importance[1-10]: 9

    __

    Why: The if len(candidates) == 0 check is inside the while candidates: loop and thus never executes, so relocating it after the loop is critical for the refinement step to run.

    High
    Serialize metadata field

    Sending a Pydantic dataclass instance directly may fail JSON serialization. Convert
    the experiment_metadata to a dict (or JSON) before adding it to the payload.

    codeflash/api/aiservice.py [252]

    -"experiment_metadata": opt.experiment_metadata,
    +"experiment_metadata": opt.experiment_metadata.dict() if opt.experiment_metadata else None,
    Suggestion importance[1-10]: 6

    __

    Why: Passing a Pydantic dataclass directly may not serialize with requests.json, so converting opt.experiment_metadata to a dict ensures the payload is JSON-serializable.

    Low

    codeflash-ai bot added a commit that referenced this pull request Jul 17, 2025
    Here’s an optimized version that preserves all existing function signatures, logic, and return values but reduces unnecessary overhead, short-circuits early, and eliminates redundant object lookups and function calls.
    
    **Key Optimizations:**
    - Use local variable binding early in `get_pr_number` to avoid repeated imports/GL lookups for `get_cached_gh_event_data`.
    - Inline the import of `get_cached_gh_event_data` once at the top—doing so locally in the function is much slower.
    - Use early returns in `speedup_critic` after fast checks to avoid unnecessary branches and function calls.
    - Remove unneeded bool() wrappers where the result is already bool.
    - Use direct access to already-imported functions instead of accessing via module (inlining `env_utils.get_pr_number`).
    
    
    
    **Summary**:  
    All function return values and signatures are preserved. Redundant lookups are eliminated, external calls are reduced, and fast-path branches short-circuit unnecessary logic to reduce overall runtime and memory allocations. Comments are preserved unless the associated code was optimized.
    Copy link
    Contributor

    codeflash-ai bot commented Jul 17, 2025

    ⚡️ Codeflash found optimizations for this PR

    📄 15% (0.15x) speedup for speedup_critic in codeflash/result/critic.py

    ⏱️ Runtime : 1.84 milliseconds 1.60 milliseconds (best of 56 runs)

    I created a new dependent PR with the suggested changes. Please review:

    If you approve, it will be merged into this PR (branch refinement).

    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