Skip to content

Conversation

@misrasaurabh1
Copy link
Contributor

@misrasaurabh1 misrasaurabh1 commented Jun 20, 2025

User description

  • trace also if function seen only once
  • trace also if method of a class

PR Type

Bug fix


Description

  • Use dot separator for class methods

  • Initialize function count to one


Changes walkthrough 📝

Relevant files
Bug fix
tracer.py`
Fix class method separator and initial count                         

codeflash/tracer.py

  • Replace class_name separator from ':' to '.'
  • Initialize function_count to 1 on first encounter
  • Additional files
    tracer.py +2/-2     

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

    Inconsistent Key Separator

    The separator for class methods in the function_count key (dot) does not match the separator used when populating function_count (colon), causing filter mismatches and potentially filtering out valid class methods.

    # filter any functions where we did not capture the return
    self.function_modules = [
        function
        for function in self.function_modules
        if self.function_count[
            str(function.file_name)
            + ":"
            + (function.class_name + "." if function.class_name else "")
    Count Initialization Off-by-One

    Initializing the function count to 1 may lead to off-by-one errors if the count is incremented later, resulting in inaccurate trace filtering.

    self.function_count[function_qualified_name] = 1
    file_valid = filter_files_optimized(

    @github-actions
    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Unify qualified name separator

    Use the same dot separator for class and function names here to match the exit
    filter’s key format. This ensures consistency in how qualified names are constructed
    and looked up.

    codeflash/tracer.py [350]

    -function_qualified_name = f"{file_name}:{(class_name + ':' if class_name else '')}{code.co_name}"
    +function_qualified_name = f"{file_name}:{(class_name + '.' if class_name else '')}{code.co_name}"
    Suggestion importance[1-10]: 8

    __

    Why: The exit filter now uses a . between class and function names, so changing the entry construction to use . ensures consistent keys and correct function counting.

    Medium

    @KRRT7
    Copy link
    Contributor

    KRRT7 commented Jun 20, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    Category **Suggestion                                                                                                                                    ** Impact
    Possible issue
    Unify qualified name separator
    Use the same dot separator for class and function names here to match the exit filter’s key format. This ensures consistency in how qualified names are constructed and looked up.

    codeflash/tracer.py [350]

    -function_qualified_name = f"{file_name}:{(class_name + ':' if class_name else '')}{code.co_name}"
    +function_qualified_name = f"{file_name}:{(class_name + '.' if class_name else '')}{code.co_name}"

    Suggestion importance[1-10]: 8
    Medium

    this looks correct @misrasaurabh1

    @misrasaurabh1
    Copy link
    Contributor Author

    that was the bug. We were using Class.method convention for qualified name but with that colon, the key was Class:method which led to the bug where NO methods could be traced! This change fixes it.
    I am updating the tests to check for method tracing as well

    @misrasaurabh1 misrasaurabh1 merged commit 5298a68 into main Jun 20, 2025
    16 checks passed
    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