Skip to content

Conversation

@KRRT7
Copy link
Contributor

@KRRT7 KRRT7 commented May 8, 2025

User description

accessing name attributes causes another error whcih propagates upwards


PR Type

Bug fix


Description

  • Wrap long function calls into multi-line statements

  • Simplify exception handling in get_function_sources_from_jedi

  • Change error logging level to debug

  • Clean up nested exception blocks


Changes walkthrough 📝

Relevant files
Error handling
code_context_extractor.py
Reformat calls and simplify error handling                             

codeflash/context/code_context_extractor.py

  • Wrap remove_unused_definitions_by_function_names calls across lines
  • Simplify exception block for definition lookup errors
  • Replace nested logger.exception with logger.debug
  • Reindent conditional length check for readability
  • +14/-9   

    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

    github-actions bot commented May 8, 2025

    PR Reviewer Guide 🔍

    (Review updated until commit e110f1a)

    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

    Missing Import

    Ensure remove_unused_definitions_by_function_names is imported in this module to avoid runtime NameError when calling it.

    code_without_unused_defs = remove_unused_definitions_by_function_names(
        original_code, qualified_helper_function_names
    )
    Exception Logging

    The logger.debug in the exception handler for name.goto omits the exception details, which may hinder troubleshooting; consider including the exception message or stack trace.

    except Exception:  # noqa: BLE001
        logger.debug(f"Error while getting definitions for {qualified_function_name}")
        definitions = []
    Assignment Expression

    The walrus operator inside the and chain for qualified_name reduces readability and relies on Python 3.8+; consider splitting the assignment and length check into separate statements.

    and len(
        (qualified_name := get_qualified_name(definition.module_name, definition.full_name)).split(
            "."
        )
    )
    <= 2

    @github-actions
    Copy link

    github-actions bot commented May 8, 2025

    PR Code Suggestions ✨

    Latest suggestions up to e110f1a
    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Log full exception traceback

    Include exception information in the log so you retain the stack trace for
    debugging. Consider using logger.exception or passing exc_info=True to logger.debug.
    This will help diagnose issues when name.goto fails.

    codeflash/context/code_context_extractor.py [410-412]

     except Exception:  # noqa: BLE001
    -    logger.debug(f"Error while getting definitions for {qualified_function_name}")
    +    logger.debug(f"Error while getting definitions for {qualified_function_name}", exc_info=True)
         definitions = []
    Suggestion importance[1-10]: 6

    __

    Why: Using exc_info=True with logger.debug improves debugging by retaining the full stack trace in the logs without changing functionality.

    Low

    Previous suggestions

    Suggestions up to commit f44f620
    CategorySuggestion                                                                                                                                    Impact
    General
    Log exception with context

    Include the caught exception details and the specific name causing the error for
    better debugging context. Use exc_info=True to capture the traceback.

    codeflash/context/code_context_extractor.py [410-412]

    -except Exception:  # noqa: BLE001
    -    logger.debug(f"Error while getting definitions for {qualified_function_name}")
    +except Exception as e:
    +    logger.debug(f"Error while getting definitions for {name.full_name}: {e}", exc_info=True)
         definitions = []
    Suggestion importance[1-10]: 7

    __

    Why: Including exc_info=True and the caught exception e in the log adds valuable debugging context for failures in name.goto(), improving traceability without altering logic.

    Medium
    Extract and simplify name check

    Precompute qualified_name before the if to improve readability and remove the inline
    assignment expression. Then use a simple length check in the condition.

    codeflash/context/code_context_extractor.py [427-432]

    -and len(
    -    (qualified_name := get_qualified_name(definition.module_name, definition.full_name)).split(
    -        "."
    -    )
    -)
    -<= 2
    +qualified_name = get_qualified_name(definition.module_name, definition.full_name)
    +...
    +and len(qualified_name.split(".")) <= 2
    Suggestion importance[1-10]: 4

    __

    Why: Precomputing qualified_name before the if improves readability, but the provided improved_code is incomplete and uses placeholders, limiting its direct applicability.

    Low

    @KRRT7 KRRT7 marked this pull request as ready for review May 12, 2025 23:54
    @KRRT7 KRRT7 requested a review from misrasaurabh1 May 12, 2025 23:55
    @github-actions
    Copy link

    Persistent review updated to latest commit e110f1a

    @KRRT7 KRRT7 merged commit 88babdd into main May 13, 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