Skip to content

Skip context check for our tests #361

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
Jul 3, 2025
Merged

Skip context check for our tests #361

merged 9 commits into from
Jul 3, 2025

Conversation

@KRRT7
Copy link
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

    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

    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
    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