Skip to content

Conversation

@misrasaurabh1
Copy link
Contributor

@misrasaurabh1 misrasaurabh1 commented Mar 31, 2025

User description

for the optimize-me repo, we were going to ask users to run codeflash init, but the repo already exists and users would waste time overwriting the config which could break it.


PR Type

  • Enhancement

Description

  • Add conditional check for pyproject.toml configuration.

  • Introduce should_modify_pyproject_toml to prompt user confirmation.

  • Update CLI messaging to conditionally display module information.

  • Minor comment update for proper GitHub naming.


Changes walkthrough 📝

Relevant files
Enhancement
cmd_init.py
Conditional configuration and messaging update.                   

codeflash/cli_cmds/cmd_init.py

  • Added check to avoid unnecessary config reconfiguration.
  • Created should_modify_pyproject_toml function for user prompt.
  • Updated echo message to show module info conditionally.
  • +34/-3   
    Documentation
    config_parser.py
    Comment clarification in configuration parser.                     

    codeflash/code_utils/config_parser.py

  • Revised comment to correct GitHub naming convention.
  • Minor adjustments to inline documentation.
  • +1/-1     

    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: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Error Handling

    In the new function should_modify_pyproject_toml, exceptions during the config parsing are caught and result in a silent True return. Consider logging the exception to aid in debugging any unexpected issues with the pyproject.toml.

    def should_modify_pyproject_toml() -> bool:
        """
        Check if the current directory contains a valid pyproject.toml file with codeflash config
        If it does, ask the user if they want to re-configure it.
        """
        from rich.prompt import Confirm
        pyproject_toml_path = Path.cwd() / "pyproject.toml"
        if not pyproject_toml_path.exists():
            return True
        try:
            config, config_file_path = parse_config_file(pyproject_toml_path)
        except Exception as e:
            return True
    
        if "module_root" not in config or config["module_root"] is None or not Path(config["module_root"]).is_dir():
            return True
        if "tests_root" not in config or config["tests_root"] is None or not Path(config["tests_root"]).is_dir():
            return True
    
        create_toml = Confirm.ask(
            f"✅ A valid Codeflash config already exists in this project. Do you want to re-configure it?", default=False, show_default=True
        )
        return create_toml
    Messaging Clarity

    The addition of the module_string variable for CLI messaging relies on a local check for setup_info. Ensure that the message remains clear and consistent for all execution paths, especially when setup_info might not be set.

    module_string = ""
    if "setup_info" in locals():
        module_string = f" you selected ({setup_info.module_root})"
    
    
    click.echo(
        f"{LF}"
        f"⚡️ Codeflash is now set up! You can now run:{LF}"
        f"    codeflash --file <path-to-file> --function <function-name> to optimize a function within a file{LF}"
        f"    codeflash --file <path-to-file> to optimize all functions in a file{LF}"
        f"    codeflash --all to optimize all functions in all files in the module{module_string}{LF}"

    @github-actions
    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Log configuration parse errors

    Incorporate error logging in the exception block to help diagnose issues during
    configuration parsing.

    codeflash/cli_cmds/cmd_init.py [135-138]

     def should_modify_pyproject_toml() -> bool:
         ...
         try:
             config, config_file_path = parse_config_file(pyproject_toml_path)
         except Exception as e:
    +        click.echo(f"Warning: failed to parse configuration: {e}")
             return True
         ...
    Suggestion importance[1-10]: 5

    __

    Why: Adding error logging in the exception block can help diagnose issues during configuration parsing; however, it is a relatively minor improvement and introduces reliance on click.echo, which may require ensuring that click is properly imported in this context.

    Low

    Copy link
    Contributor

    @KRRT7 KRRT7 left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    LGTM

    @misrasaurabh1 misrasaurabh1 merged commit 778a213 into main Mar 31, 2025
    15 of 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