Skip to content

Skip context check for our tests#361

Merged
misrasaurabh1 merged 9 commits into
mainfrom
skip_context-check-for-our-tests
Jul 3, 2025
Merged

Skip context check for our tests#361
misrasaurabh1 merged 9 commits into
mainfrom
skip_context-check-for-our-tests

Conversation

@KRRT7
Copy link
Copy Markdown
Contributor

@KRRT7 KRRT7 commented Jun 21, 2025 •
edited by github-actions Bot
Loading

PR Type

Enhancement

Description

  • Add args.no_pr bypass to skip optimization

  • Include args: Namespace parameter in check

  • Pass self.args to optimization function

  • Pin Python version to 3.12

Changes walkthrough 📝

Relevant files
Enhancement
functions_to_optimize.py
Add args.no_pr bypass to skip check                                           

codeflash/discovery/functions_to_optimize.py

  • Import Namespace from argparse
  • Add args: Namespace parameter to was_function_previously_optimized
  • Skip optimization when args.no_pr flag is set
    		• +4/-2     
    function_optimizer.py
    Pass args to previous optimization check                                 

    codeflash/optimization/function_optimizer.py

    • Update call to was_function_previously_optimized with self.args
    		 +1/-1     
    Configuration changes
    .python-version
    Pin Python version to 3.12                                                             

    .python-version

    • Pin Python version to 3.12
    		 +1/-0     
    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
    Copy Markdown
    Contributor

    github-actions Bot commented Jun 21, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

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

    Missing import

    The Namespace annotation is only imported under TYPE_CHECKING, which may cause NameError at runtime when evaluating the function signature. Consider importing Namespace unconditionally or using string annotations.

     
    from argparse import Namespace
    Attribute access

    Accessing args.no_pr without checking if args has that attribute may raise an AttributeError. Ensure a default or use getattr(args, "no_pr", False).

     
    if not owner or not repo or pr_number is None or args.no_pr:
    Args availability

    self.args is passed to was_function_previously_optimized; ensure that args is initialized on this object in all code paths to avoid attribute errors.

     
        self.function_to_optimize, code_context, self.args
):

    @github-actions
    Copy link
    Copy Markdown
    Contributor

    github-actions Bot commented Jun 21, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Guard args.no_pr access

    Use getattr to safely check args.no_pr and avoid an AttributeError if no_pr isn’t
    defined on the passed args object.

    codeflash/discovery/functions_to_optimize.py [450-451]

    -if not owner or not repo or pr_number is None or args.no_pr:
+if not owner or not repo or pr_number is None or getattr(args, "no_pr", False):
     return False
    Suggestion importance[1-10]: 5

    __

    Why: Replacing direct args.no_pr access with getattr prevents potential AttributeError and modestly increases robustness.

    		Low

    @github-actions github-actions Bot added the workflow-modified This PR modifies GitHub Actions workflows label Jun 21, 2025
    @KRRT7 KRRT7 changed the title Skip context check for our tests Skip context check for our tests & add 3.13 to the test suite Jun 21, 2025
    codeflash-ai Bot added a commit that referenced this pull request Jun 21, 2025
    @codeflash-ai
    ⚡️ Speed up function was_function_previously_optimized by 737% in PR …
    8931306 
    …#361 (`skip_context-check-for-our-tests`)

Here is the optimized version of your program, with specific changes and the *reasoning* behind each to maximize speed and efficiency.

**Key Optimizations:**

1. **Remove duplicate imports** and unused imports:  
   - Remove duplicate imports of `Repo`, `git`, and unnecessary `Any`, only import the used ones.
2. **Use local variables to minimize repeated attribute lookups/`getattr`:**  
   - Evaluate `getattr(args, "no_pr", False)` once, store in a variable.
3. **Avoid creating `code_contexts` list if not needed:**  
   - Since only one code context is ever added and checked, don’t append in a separate step, construct the list in one go.
4. **Replace unnecessary checks and shorten conditionals:**  
   - Since you unconditionally append to `code_contexts` and the only check is for a non-empty list immediately after, this is always true, so the check is extraneous and can be removed.
5. **Move try-except as close as possible to just what can fail:**  
   - Narrow down exception blocks only to calls that can actually fail, so that local variables remain in scope and bugs are easier to find.
6. **Explicitly cache/utilize import of `get_repo_owner_and_name` from the proper module**:  
   - Your implementation mistakenly re-defines and caches `get_repo_owner_and_name`, and uses an internal call to `get_remote_url` that isn’t allowed per your module rules. The right way is to use the imported one from `codeflash.code_utils.git_utils` (already imported); do not reimplement or cache it.  
   - Remove your own definition and just use the import.

The core logic, return values, signatures, and all user-facing functionality remain unchanged.

Here is your optimized code.



### Summary of what’s changed.
- Removed duplicate and unnecessary imports.
- Use imported/cached `get_repo_owner_and_name`, don’t redeclare.
- Short-circuit return for missing repo/pr info.
- Do not check or append `code_contexts` redundantly.
- Reduced the scope of try-except.
- Retained all user-facing logic & comments except where logic was changed.

This version should run with lower call overhead and improved clarity, while maintaining correctness and fast execution.
    @codeflash-ai codeflash-ai Bot mentioned this pull request Jun 21, 2025
    Closed
    @codeflash-ai
    Copy link
    Copy Markdown
    Contributor

    codeflash-ai Bot commented Jun 21, 2025

    ⚡️ Codeflash found optimizations for this PR

    📄 737% (7.37x) speedup for was_function_previously_optimized in codeflash/discovery/functions_to_optimize.py

    ⏱️ Runtime : 2.13 seconds → 254 milliseconds (best of 19 runs)

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

    If you approve, it will be merged into this PR (branch skip_context-check-for-our-tests).

    @KRRT7 KRRT7 changed the title Skip context check for our tests & add 3.13 to the test suite Skip context check for our tests Jun 21, 2025
    @KRRT7 KRRT7 requested a review from a team June 21, 2025 04:27
    @misrasaurabh1 misrasaurabh1 enabled auto-merge June 28, 2025 02:47
    @misrasaurabh1 misrasaurabh1 merged commit a54be51 into main Jul 3, 2025
    16 checks passed
    @KRRT7 KRRT7 deleted the skip_context-check-for-our-tests branch July 3, 2025 23:05
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Reviewers

    @misrasaurabh1 misrasaurabh1 misrasaurabh1 approved these changes

    Assignees

    No one assigned

    Labels

    Review effort 3/5 workflow-modified This PR modifies GitHub Actions workflows

    Projects

    None yet

    Milestone

    No milestone

    Development

    Successfully merging this pull request may close these issues.

    2 participants

    @KRRT7 @misrasaurabh1