Skip to content

Conversation

@mohammedahmed18
Copy link
Contributor

@mohammedahmed18 mohammedahmed18 commented Jul 16, 2025

PR Type

Bug fix


Description

  • Skip context hash on non-codeflash repos

  • Update optimizer context hash comment


Changes walkthrough 📝

Relevant files
Bug fix
cfapi.py
Add skip guard for code context hashing                                   

codeflash/api/cfapi.py

  • Added condition to skip context hash for non-codeflash repo
  • Placed guard after retrieving repo owner/name
  • +3/-0     
    Refactoring
    function_optimizer.py
    Refine context hash comment in optimizer                                 

    codeflash/optimization/function_optimizer.py

  • Replaced GH actions comment with repo-skipping note
  • Removed outdated context hash comment
  • +1/-3     

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @mohammedahmed18 mohammedahmed18 marked this pull request as draft July 16, 2025 15:16
    @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

    Redundant PR number fetch

    get_pr_number() is called twice before and after the guard. Consider removing the redundant second invocation.

    pr_number = get_pr_number()
    if pr_number is None:
        return
    try:
        owner, repo = get_repo_owner_and_name()
        # don't add the context hash for codeflash repo
        if f"{owner}/{repo}" != "codeflash-ai/codeflash":
            return
        pr_number = get_pr_number()
    Missing guard for context hash

    The added comment indicates skipping context hash for non-codeflash repos, but the code still calls add_code_context_hash. Introduce the same conditional guard as in cfapi.py.

    # don't add the context hash for codeflash repo
    add_code_context_hash(code_context.hashing_code_context_hash)

    @github-actions
    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Invert repository check logic

    The condition currently returns for non-codeflash-ai/codeflash repos, which is
    inverted. To skip only for the codeflash-ai/codeflash repository, invert the
    comparison operator.

    codeflash/api/cfapi.py [243-245]

    -# don't add the context hash for codeflash repo
    -if f"{owner}/{repo}" != "codeflash-ai/codeflash":
    +# skip adding the context hash for codeflash repo
    +if f"{owner}/{repo}" == "codeflash-ai/codeflash":
         return
    Suggestion importance[1-10]: 9

    __

    Why: The current guard logic is inverted, causing context hashes to be added only for the codeflash-ai/codeflash repo instead of excluded, so flipping the comparison fixes a critical bug.

    High
    General
    Add repo guard before hash call

    Add a repository guard here to skip adding the context hash for the
    codeflash-ai/codeflash repo, mirroring the logic in cfapi.py, to avoid duplicating
    unwanted context entries.

    codeflash/optimization/function_optimizer.py [338-339]

    -# don't add the context hash for codeflash repo
    +# skip adding the context hash for codeflash repo
    +owner, repo = get_repo_owner_and_name()
    +if f"{owner}/{repo}" == "codeflash-ai/codeflash":
    +    return
     add_code_context_hash(code_context.hashing_code_context_hash)
    Suggestion importance[1-10]: 6

    __

    Why: Introducing the same repository check as in cfapi.py prevents unintended context hashing for the codeflash repo and maintains consistency.

    Low
    Remove redundant PR number fetch

    The second call to get_pr_number() is redundant since pr_number was retrieved at the
    start; remove this extra call to avoid unnecessary overhead.

    codeflash/api/cfapi.py [246]

    -pr_number = get_pr_number()
    +# (this line should be removed)
    Suggestion importance[1-10]: 4

    __

    Why: Removing the second get_pr_number() call eliminates unnecessary overhead with minimal impact on functionality.

    Low

    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