Skip to content

Conversation

@mohammedahmed18
Copy link
Contributor

@mohammedahmed18 mohammedahmed18 commented Oct 14, 2025

User description


Description

  • Extract common init suggestion helpers

  • Add cached valid subdirs discovery

  • Split test framework detection paths

  • Improve interactive choices with icons


Diagram Walkthrough

flowchart LR
  A["collect_setup_info"] -- "uses" --> B["get_suggestions"]
  B -- "module_root/tests_root/test_framework" --> C["get_valid_subdirs (cached)"]
  B -- "test framework" --> D["detect_test_framework_from_config_files"]
  A -- "fallback detection" --> E["detect_test_framework_from_test_files"]
  A -- "choices" --> F["framework_choices with icons"]
Loading

File Walkthrough

Relevant files
Enhancement
cmd_init.py
Refactor init helpers and framework detection                       

codeflash/cli_cmds/cmd_init.py

  • Introduce CommonSections enum and suggestion helpers.
  • Cache valid subdirectories via lru_cache.
  • Split test framework detection into config vs files.
  • Enhance CLI choices and defaults with dynamic options.
+77/-40 


PR Type

Enhancement, Bug fix


Description

  • Add LSP-aware init and config flow

  • Split test framework detection paths

  • Provide cached directory suggestions

  • Improve pyproject read/write robustness


Diagram Walkthrough

flowchart LR
  A["LSP client"] -- "getConfigSuggestions" --> B["cmd_init.get_suggestions"]
  B -- "uses" --> C["get_valid_subdirs (lru_cache)"]
  A -- "writeConfig" --> D["configure_pyproject_toml"]
  D -- "reads/creates" --> E["pyproject.toml"]
  A -- "initProject" --> F["is_valid_pyproject_toml"]
  F -- "validates" --> E
  D -- "formatter/telemetry handling" --> G["config sections"]
  B -- "detects" --> H["detect_test_framework_*"]
Loading

File Walkthrough

Relevant files
Enhancement
cmd_init.py
LSP-friendly init helpers and config writer                           

codeflash/cli_cmds/cmd_init.py

  • Add LSP-aware helpers and telemetry flag
  • Cache subdir suggestions with lru_cache
  • Split test framework detection (config vs files)
  • Robust pyproject writing; return success flag
+139/-91
beta.py
LSP endpoints for config suggestions and writing                 

codeflash/lsp/beta.py

  • Add writeConfig and getConfigSuggestions handlers
  • Integrate init validation with detailed reasons
  • Reuse CLI init helpers in LSP server
+73/-7   
Bug fix
config_parser.py
LSP-safe config parsing and defaults                                         

codeflash/code_utils/config_parser.py

  • Make parsing resilient in LSP mode
  • Allow empty/missing codeflash block
  • Guard formatter and framework assertions
+17/-6   

@CLAassistant
Copy link

CLAassistant commented Oct 14, 2025

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Codeflash Bot seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@github-actions
Copy link

github-actions bot commented Oct 14, 2025

PR Reviewer Guide 🔍

(Review updated until commit 71f8076)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Type/Import Bugs

The new helper functions use Enum members and typing annotations in ways that appear inconsistent: CommonSections is an Enum but get_suggestions compares 'section == CommonSections.module_root' and type hints reference 'tuple(list[str], Optional[str])' which is invalid typing syntax. Also, is_valid_pyproject_toml returns Optional[str] paths but later detect_test_framework_from_test_files expects a Path and calls .iterdir()—ensure callers pass Path objects. Verify imports of InvalidGitRepositoryError and tomlkit are present after refactors.

def get_suggestions(section: str) -> tuple(list[str], Optional[str]):
    valid_subdirs = get_valid_subdirs()
    if section == CommonSections.module_root:
        return [d for d in valid_subdirs if d != "tests"], None
    if section == CommonSections.tests_root:
        default = "tests" if "tests" in valid_subdirs else None
        return valid_subdirs, default
    if section == CommonSections.test_framework:
        auto_detected = detect_test_framework_from_config_files(Path.cwd())
        return ["pytest", "unittest"], auto_detected
    msg = f"Unknown section: {section}"
    raise ValueError(msg)


def collect_setup_info() -> SetupInfo:
    curdir = Path.cwd()
    # Check if the cwd is writable
    if not os.access(curdir, os.W_OK):
        click.echo(f"❌ The current directory isn't writable, please check your folder permissions and try again.{LF}")
        click.echo("It's likely you don't have write permissions for this folder.")
        sys.exit(1)

    # Check for the existence of pyproject.toml or setup.py
    project_name = check_for_toml_or_setup_file()
    valid_module_subdirs, _ = get_suggestions(CommonSections.module_root)

    curdir_option = f"current directory ({curdir})"
    custom_dir_option = "enter a custom directory…"
    module_subdir_options = [*valid_module_subdirs, curdir_option, custom_dir_option]

    info_panel = Panel(
        Text(
            "📁 Let's identify your Python module directory.\n\n"
            "This is usually the top-level directory containing all your Python source code.\n",
            style="cyan",
        ),
        title="🔍 Module Discovery",
        border_style="bright_blue",
    )
    console.print(info_panel)
    console.print()
    questions = [
        inquirer.List(
            "module_root",
            message="Which Python module do you want me to optimize?",
            choices=module_subdir_options,
            default=(project_name if project_name in module_subdir_options else module_subdir_options[0]),
            carousel=True,
        )
    ]

    answers = inquirer.prompt(questions, theme=CodeflashTheme())
    if not answers:
        apologize_and_exit()
    module_root_answer = answers["module_root"]
    if module_root_answer == curdir_option:
        module_root = "."
    elif module_root_answer == custom_dir_option:
        custom_panel = Panel(
            Text(
                "📂 Enter a custom module directory path.\n\nPlease provide the path to your Python module directory.",
                style="yellow",
            ),
            title="📂 Custom Directory",
            border_style="bright_yellow",
        )
        console.print(custom_panel)
        console.print()

        custom_questions = [
            inquirer.Path(
                "custom_path",
                message="Enter the path to your module directory",
                path_type=inquirer.Path.DIRECTORY,
                exists=True,
            )
        ]

        custom_answers = inquirer.prompt(custom_questions, theme=CodeflashTheme())
        if custom_answers:
            module_root = Path(custom_answers["custom_path"])
        else:
            apologize_and_exit()
    else:
        module_root = module_root_answer
    ph("cli-project-root-provided")

    # Discover test directory
    create_for_me_option = f"🆕 Create a new tests{os.pathsep} directory for me!"
    tests_suggestions, default_tests_subdir = get_suggestions(CommonSections.tests_root)
    test_subdir_options = [sub_dir for sub_dir in tests_suggestions if sub_dir != module_root]
    if "tests" not in tests_suggestions:
        test_subdir_options.append(create_for_me_option)
    custom_dir_option = "📁 Enter a custom directory…"
    test_subdir_options.append(custom_dir_option)

    tests_panel = Panel(
        Text(
            "🧪 Now let's locate your test directory.\n\n"
            "This is where all your test files are stored. If you don't have tests yet, "
            "I can create a directory for you!",
            style="green",
        ),
        title="🧪 Test Discovery",
        border_style="bright_green",
    )
    console.print(tests_panel)
    console.print()

    tests_questions = [
        inquirer.List(
            "tests_root",
            message="Where are your tests located?",
            choices=test_subdir_options,
            default=(default_tests_subdir or test_subdir_options[0]),
            carousel=True,
        )
    ]

    tests_answers = inquirer.prompt(tests_questions, theme=CodeflashTheme())
    if not tests_answers:
        apologize_and_exit()
    tests_root_answer = tests_answers["tests_root"]

    if tests_root_answer == create_for_me_option:
        tests_root = Path(curdir) / default_tests_subdir
        tests_root.mkdir()
        click.echo(f"✅ Created directory {tests_root}{os.path.sep}{LF}")
    elif tests_root_answer == custom_dir_option:
        custom_tests_panel = Panel(
            Text(
                "🧪 Enter a custom test directory path.\n\nPlease provide the path to your test directory, relative to the current directory.",
                style="yellow",
            ),
            title="🧪 Custom Test Directory",
            border_style="bright_yellow",
        )
        console.print(custom_tests_panel)
        console.print()

        custom_tests_questions = [
            inquirer.Path(
                "custom_tests_path", message="Enter the path to your tests directory", path_type=inquirer.Path.DIRECTORY
            )
        ]

        custom_tests_answers = inquirer.prompt(custom_tests_questions, theme=CodeflashTheme())
        if custom_tests_answers:
            tests_root = Path(curdir) / Path(custom_tests_answers["custom_tests_path"])
        else:
            apologize_and_exit()
    else:
        tests_root = Path(curdir) / Path(cast("str", tests_root_answer))

    tests_root = tests_root.relative_to(curdir)

    resolved_module_root = (Path(curdir) / Path(module_root)).resolve()
    resolved_tests_root = (Path(curdir) / Path(tests_root)).resolve()
    if resolved_module_root == resolved_tests_root:
        logger.warning(
            "It looks like your tests root is the same as your module root. This is not recommended and can lead to unexpected behavior."
        )

    ph("cli-tests-root-provided")

    test_framework_choices, detected_framework = get_suggestions(CommonSections.test_framework)
    autodetected_test_framework = detected_framework or detect_test_framework_from_test_files(tests_root)

    framework_message = "⚗️ Let's configure your test framework.\n\n"
    if autodetected_test_framework:
        framework_message += f"I detected that you're using {autodetected_test_framework}. "
    framework_message += "Please confirm or select a different one."

    framework_panel = Panel(Text(framework_message, style="blue"), title="⚗️ Test Framework", border_style="bright_blue")
    console.print(framework_panel)
    console.print()

    framework_choices = []
    # add icons based on the detected framework
    for choice in test_framework_choices:
        if choice == "pytest":
            framework_choices.append(("🧪 pytest", "pytest"))
        elif choice == "unittest":
            framework_choices.append(("🐍 unittest", "unittest"))

    framework_questions = [
        inquirer.List(
            "test_framework",
            message="Which test framework do you use?",
            choices=framework_choices,
            default=autodetected_test_framework or "pytest",
            carousel=True,
        )
    ]

    framework_answers = inquirer.prompt(framework_questions, theme=CodeflashTheme())
    if not framework_answers:
        apologize_and_exit()
    test_framework = framework_answers["test_framework"]

    ph("cli-test-framework-provided", {"test_framework": test_framework})

    benchmarks_root = None

    # TODO: Implement other benchmark framework options
    # if benchmarks_root:
    #     benchmarks_root = benchmarks_root.relative_to(curdir)
    #
    #     # Ask about benchmark framework
    #     benchmark_framework_options = ["pytest-benchmark", "asv (Airspeed Velocity)", "custom/other"]
    #     benchmark_framework = inquirer_wrapper(
    #         inquirer.list_input,
    #         message="Which benchmark framework do you use?",
    #         choices=benchmark_framework_options,
    #         default=benchmark_framework_options[0],
    #         carousel=True,
    #     )

    formatter_panel = Panel(
        Text(
            "🎨 Let's configure your code formatter.\n\n"
            "Code formatters help maintain consistent code style. "
            "Codeflash will use this to format optimized code.",
            style="magenta",
        ),
        title="🎨 Code Formatter",
        border_style="bright_magenta",
    )
    console.print(formatter_panel)
    console.print()

    formatter_questions = [
        inquirer.List(
            "formatter",
            message="Which code formatter do you use?",
            choices=[
                ("⚫ black", "black"),
                ("⚡ ruff", "ruff"),
                ("🔧 other", "other"),
                ("❌ don't use a formatter", "don't use a formatter"),
            ],
            default="black",
            carousel=True,
        )
    ]

    formatter_answers = inquirer.prompt(formatter_questions, theme=CodeflashTheme())
    if not formatter_answers:
        apologize_and_exit()
    formatter = formatter_answers["formatter"]

    git_remote = ""
    try:
        repo = Repo(str(module_root), search_parent_directories=True)
        git_remotes = get_git_remotes(repo)
        if git_remotes:  # Only proceed if there are remotes
            if len(git_remotes) > 1:
                git_panel = Panel(
                    Text(
                        "🔗 Configure Git Remote for Pull Requests.\n\n"
                        "Codeflash will use this remote to create pull requests with optimized code.",
                        style="blue",
                    ),
                    title="🔗 Git Remote Setup",
                    border_style="bright_blue",
                )
                console.print(git_panel)
                console.print()

                git_questions = [
                    inquirer.List(
                        "git_remote",
                        message="Which git remote should Codeflash use for Pull Requests?",
                        choices=git_remotes,
                        default="origin",
                        carousel=True,
                    )
                ]

                git_answers = inquirer.prompt(git_questions, theme=CodeflashTheme())
                git_remote = git_answers["git_remote"] if git_answers else git_remotes[0]
            else:
                git_remote = git_remotes[0]
        else:
            click.echo(
                "No git remotes found. You can still use Codeflash locally, but you'll need to set up a remote "
                "repository to use GitHub features."
            )
    except InvalidGitRepositoryError:
        git_remote = ""

    enable_telemetry = ask_for_telemetry()

    ignore_paths: list[str] = []
    return SetupInfo(
        module_root=str(module_root),
        tests_root=str(tests_root),
        benchmarks_root=str(benchmarks_root) if benchmarks_root else None,
        test_framework=cast("str", test_framework),
        ignore_paths=ignore_paths,
        formatter=cast("str", formatter),
        git_remote=str(git_remote),
        enable_telemetry=enable_telemetry,
    )


def detect_test_framework_from_config_files(curdir: Path) -> Optional[str]:
    test_framework = None
    pytest_files = ["pytest.ini", "pyproject.toml", "tox.ini", "setup.cfg"]
    pytest_config_patterns = {
        "pytest.ini": "[pytest]",
        "pyproject.toml": "[tool.pytest.ini_options]",
        "tox.ini": "[pytest]",
        "setup.cfg": "[tool:pytest]",
    }
    for pytest_file in pytest_files:
        file_path = curdir / pytest_file
        if file_path.exists():
            with file_path.open(encoding="utf8") as file:
                contents = file.read()
                if pytest_config_patterns[pytest_file] in contents:
                    test_framework = "pytest"
                    break
        test_framework = "pytest"
    return test_framework


def detect_test_framework_from_test_files(tests_root: Path) -> Optional[str]:
    test_framework = None
    # Check if any python files contain a class that inherits from unittest.TestCase
    for filename in tests_root.iterdir():
        if filename.suffix == ".py":
            with filename.open(encoding="utf8") as file:
                contents = file.read()
                try:
                    node = ast.parse(contents)
                except SyntaxError:
                    continue
                if any(
                    isinstance(item, ast.ClassDef)
                    and any(
                        (isinstance(base, ast.Attribute) and base.attr == "TestCase")
                        or (isinstance(base, ast.Name) and base.id == "TestCase")
                        for base in item.bases
                    )
                    for item in node.body
                ):
                    test_framework = "unittest"
                    break
    return test_framework
Config Mapping Risk

write_config maps between LSP config keys and pyproject fields but mixes snake-case and dash-case (e.g., 'ignore_paths' vs 'ignore-paths', 'formatter_cmds' vs 'formatter-cmds'). Ensure consistency with parse_config_file expectations and that enable_telemetry is inverted correctly when writing 'disable-telemetry'.

@server.feature("writeConfig")
def write_config(_server: CodeflashLanguageServer, params: WriteConfigParams) -> dict[str, any]:
    cfg = params.config
    cfg_file = Path(params.config_file) if params.config_file else None

    parsed_config = {}

    if cfg_file and not cfg_file.exists():
        # the client provided a config path but it doesn't exist
        create_empty_pyproject_toml(cfg_file)
    elif cfg_file and cfg_file.exists():
        try:
            parsed_config, _ = parse_config_file(cfg_file)
        except Exception as e:
            return {"status": "error", "message": f"Failed to parse configuration: {e}"}

    setup_info = SetupInfo(
        module_root=getattr(cfg, "module_root", ""),
        tests_root=getattr(cfg, "tests_root", ""),
        test_framework=getattr(cfg, "test_framework", "pytest"),
        # keep other stuff as it is
        benchmarks_root=None,  # we don't support benchmarks in the LSP
        ignore_paths=parsed_config.get("ignore_paths", []),
        formatter=parsed_config.get("formatter_cmds", ["disabled"]),
        git_remote=parsed_config.get("git_remote", ""),
        enable_telemetry=parsed_config.get("disable_telemetry", True),
    )
    devnull_writer = open(os.devnull, "w")  # noqa
    with contextlib.redirect_stdout(devnull_writer):
        configured = configure_pyproject_toml(setup_info, cfg_file)
        if configured:
            return {"status": "success"}
        return {"status": "error", "message": "Failed to configure pyproject.toml"}
LSP Mode Edge Cases

parse_config_file returns {} in LSP mode when the 'codeflash' block is missing, but later code assumes presence of keys like 'formatter-cmds' and 'test-framework'. Confirm all callers handle empty configs, and guard assertions only when keys exist.

def parse_config_file(
    config_file_path: Path | None = None,
    override_formatter_check: bool = False,  # noqa: FBT001, FBT002
) -> tuple[dict[str, Any], Path]:
    config_file_path = find_pyproject_toml(config_file_path)
    try:
        with config_file_path.open("rb") as f:
            data = tomlkit.parse(f.read())
    except tomlkit.exceptions.ParseError as e:
        msg = f"Error while parsing the config file {config_file_path}. Please recheck the file for syntax errors. Error: {e}"
        raise ValueError(msg) from e

    lsp_mode = is_LSP_enabled()

    try:
        tool = data["tool"]
        assert isinstance(tool, dict)
        config = tool.get("codeflash", {})
    except tomlkit.exceptions.NonExistentKey as e:
        if lsp_mode:
            # don't fail in lsp mode if codeflash config is not found.
            return {}, config_file_path
        msg = f"Could not find the 'codeflash' block in the config file {config_file_path}. Please run 'codeflash init' to create the config file."
        raise ValueError(msg) from e
    assert isinstance(config, dict)

    if config == {} and lsp_mode:
        return {}, config_file_path

    # default values:
    path_keys = ["module-root", "tests-root", "benchmarks-root"]
    path_list_keys = ["ignore-paths"]
    str_keys = {"pytest-cmd": "pytest", "git-remote": "origin"}
    bool_keys = {
        "override-fixtures": False,
        "disable-telemetry": False,
        "disable-imports-sorting": False,
        "benchmark": False,
    }
    list_str_keys = {"formatter-cmds": ["black $file"]}

    for key, default_value in str_keys.items():
        if key in config:
            config[key] = str(config[key])
        else:
            config[key] = default_value
    for key, default_value in bool_keys.items():
        if key in config:
            config[key] = bool(config[key])
        else:
            config[key] = default_value
    for key in path_keys:
        if key in config:
            config[key] = str((Path(config_file_path).parent / Path(config[key])).resolve())
    for key, default_value in list_str_keys.items():
        if key in config:
            config[key] = [str(cmd) for cmd in config[key]]
        else:
            config[key] = default_value

    for key in path_list_keys:
        if key in config:
            config[key] = [str((Path(config_file_path).parent / path).resolve()) for path in config[key]]
        else:
            config[key] = []

    if config.get("test-framework"):
        assert config["test-framework"] in {"pytest", "unittest"}, (
            "In pyproject.toml, Codeflash only supports the 'test-framework' as pytest and unittest."
        )
    # see if this is happening during GitHub actions setup
    if config.get("formatter-cmds") and len(config.get("formatter-cmds")) > 0 and not override_formatter_check:
        assert config.get("formatter-cmds")[0] != "your-formatter $file", (

@github-actions
Copy link

github-actions bot commented Oct 14, 2025

PR Code Suggestions ✨

Latest suggestions up to 71f8076
Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Align config key mapping

The config keys used here don't match those produced by parse_config_file (which
normalizes to kebab-case names). This yields wrong defaults (e.g., telemetry
inverted) and empty values. Map to the correct keys and fix telemetry polarity to
avoid silently misconfiguring users.

codeflash/lsp/beta.py [189-198]

 setup_info = SetupInfo(
-    module_root=getattr(cfg, "module_root", ""),
-    tests_root=getattr(cfg, "tests_root", ""),
-    test_framework=getattr(cfg, "test_framework", "pytest"),
-    # keep other stuff as it is
-    benchmarks_root=None,  # we don't support benchmarks in the LSP
-    ignore_paths=parsed_config.get("ignore_paths", []),
-    formatter=parsed_config.get("formatter_cmds", ["disabled"]),
-    git_remote=parsed_config.get("git_remote", ""),
-    enable_telemetry=parsed_config.get("disable_telemetry", True),
+    module_root=getattr(cfg, "module_root", "") or parsed_config.get("module-root", ""),
+    tests_root=getattr(cfg, "tests_root", "") or parsed_config.get("tests-root", ""),
+    test_framework=getattr(cfg, "test_framework", "") or parsed_config.get("test-framework", "pytest"),
+    benchmarks_root=None,  # not supported in LSP
+    ignore_paths=parsed_config.get("ignore-paths", []),
+    formatter=parsed_config.get("formatter-cmds", ["disabled"]),
+    git_remote=parsed_config.get("git-remote", ""),
+    enable_telemetry=not parsed_config.get("disable-telemetry", False),
 )
Suggestion importance[1-10]: 9

__

Why: The current code uses snake_case keys that don't match parse_config_file's kebab-case outputs and inverses telemetry; the proposed mapping is precise and prevents misconfiguration, directly improving correctness.

High
Fix enum comparison bug

The CommonSections enum values are strings, but get_suggestions compares section ==
CommonSections.module_root, causing all branches to miss and raise an error. Compare
against the enum members or use .value consistently. This currently breaks
configuration suggestions and setup flow.

codeflash/cli_cmds/cmd_init.py [224-228]

 class CommonSections(Enum):
     module_root = "module_root"
     tests_root = "tests_root"
     test_framework = "test_framework"
 
+def get_suggestions(section: CommonSections) -> tuple[list[str], Optional[str]]:
+    valid_subdirs = get_valid_subdirs()
+    if section is CommonSections.module_root:
+        return [d for d in valid_subdirs if d != "tests"], None
+    if section is CommonSections.tests_root:
+        default = "tests" if "tests" in valid_subdirs else None
+        return valid_subdirs, default
+    if section is CommonSections.test_framework:
+        auto_detected = detect_test_framework_from_config_files(Path.cwd())
+        return ["pytest", "unittest"], auto_detected
+    msg = f"Unknown section: {section}"
+    raise ValueError(msg)
+
Suggestion importance[1-10]: 7

__

Why: Correctly comparing CommonSections members is important to avoid the ValueError path; changing the function signature to accept CommonSections and using identity comparisons is accurate and prevents broken suggestion flow.

Medium
General
Close devnull writer properly

The devnull_writer file handle is never closed, causing a resource leak and
potential FD exhaustion under repeated calls. Use a context manager to ensure it is
closed after use. This is critical for long-running LSP servers.

codeflash/lsp/beta.py [199-205]

-devnull_writer = open(os.devnull, "w")  # noqa
-with contextlib.redirect_stdout(devnull_writer):
-    configured = configure_pyproject_toml(setup_info, cfg_file)
-    if configured:
-        return {"status": "success"}
-    return {"status": "error", "message": "Failed to configure pyproject.toml"}
+with open(os.devnull, "w") as devnull_writer:
+    with contextlib.redirect_stdout(devnull_writer):
+        configured = configure_pyproject_toml(setup_info, cfg_file)
+        if configured:
+            return {"status": "success"}
+        return {"status": "error", "message": "Failed to configure pyproject.toml"}
Suggestion importance[1-10]: 8

__

Why: Using a context manager to close devnull_writer prevents file descriptor leaks in a long-running LSP; the change is minimal, correct, and has meaningful stability impact.

Medium

Previous suggestions

Suggestions up to commit 7aee1c1
CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix enum type mismatch

The CommonSections enum stores string values, but elsewhere it's compared directly
to enum members, which is fine; however, functions like get_suggestions type their
section as str and compare against enum members, causing comparisons to always fail.
Accept and compare CommonSections consistently to avoid unreachable branches and the
"Unknown section" error.

codeflash/cli_cmds/cmd_init.py [219-223]

 class CommonSections(Enum):
     module_root = "module_root"
     tests_root = "tests_root"
     test_framework = "test_framework"
 
+def get_suggestions(section: CommonSections) -> tuple[list[str], Optional[str]]:
+    valid_subdirs = get_valid_subdirs()
+    if section is CommonSections.module_root:
+        return [d for d in valid_subdirs if d != "tests"], None
+    if section is CommonSections.tests_root:
+        default = "tests" if "tests" in valid_subdirs else None
+        return valid_subdirs, default
+    if section is CommonSections.test_framework:
+        auto_detected = detect_test_framework_from_config_files(Path.cwd())
+        return ["pytest", "unittest"], auto_detected
+    msg = f"Unknown section: {section}"
+    raise ValueError(msg)
+
Suggestion importance[1-10]: 8

__

Why: The code types get_suggestions parameter as str but compares it to CommonSections members, leading to unreachable branches and a potential ValueError. Changing the parameter to CommonSections and using identity comparisons fixes a correctness bug with meaningful impact.

Medium
Guard against missing tests directory

tests_root can be a string or unresolved path earlier, and it may not exist;
iterating with iterdir() will raise. Resolve and verify existence before scanning to
prevent crashes in new projects without a tests directory.

codeflash/cli_cmds/cmd_init.py [568-590]

 def detect_test_framework_from_test_files(tests_root: Path) -> Optional[str]:
-    test_framework = None
+    tests_root = Path(tests_root).resolve()
+    if not tests_root.exists() or not tests_root.is_dir():
+        return None
     # Check if any python files contain a class that inherits from unittest.TestCase
     for filename in tests_root.iterdir():
         if filename.suffix == ".py":
             with filename.open(encoding="utf8") as file:
                 contents = file.read()
                 try:
                     node = ast.parse(contents)
                 except SyntaxError:
                     continue
                 if any(
                     isinstance(item, ast.ClassDef)
                     and any(
                         (isinstance(base, ast.Attribute) and base.attr == "TestCase")
                         or (isinstance(base, ast.Name) and base.id == "TestCase")
                         for base in item.bases
                     )
                     for item in node.body
                 ):
-                    test_framework = "unittest"
-                    break
-    return test_framework
+                    return "unittest"
+    return None
Suggestion importance[1-10]: 7

__

Why: The function iterates tests_root.iterdir() without verifying existence, which can raise if the directory is absent. Adding existence checks prevents crashes and improves robustness with accurate code changes.

Medium
General
Avoid stale cached directory list

Caching get_valid_subdirs across the process can serve stale results if the user
creates or selects directories during the same run. Remove the cache or scope it per
call to ensure directory suggestions reflect current filesystem state.

codeflash/cli_cmds/cmd_init.py [225-241]

-@lru_cache(maxsize=1)
 def get_valid_subdirs() -> list[str]:
-    ignore_subdirs = [
+    ignore_subdirs = {
         "venv",
         "node_modules",
         "dist",
         "build",
         "build_temp",
         "build_scripts",
         "env",
         "logs",
         "tmp",
         "__pycache__",
-    ]
+    }
     return [
-        d for d in next(os.walk("."))[1] if not d.startswith(".") and not d.startswith("__") and d not in ignore_subdirs
+        d for d in next(os.walk("."))[1]
+        if not d.startswith(".") and not d.startswith("__") and d not in ignore_subdirs
     ]
Suggestion importance[1-10]: 6

__

Why: Using lru_cache on get_valid_subdirs can return stale results during an interactive session as the filesystem changes. Removing caching is a reasonable improvement to correctness, though the impact is moderate.

Low

@mohammedahmed18 mohammedahmed18 marked this pull request as ready for review October 15, 2025 16:38
@github-actions
Copy link

Persistent review updated to latest commit 71f8076

@mohammedahmed18 mohammedahmed18 requested a review from KRRT7 October 17, 2025 19:22
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.

we should add unit tests, cmd_init is an important workflow

@mohammedahmed18 mohammedahmed18 requested a review from KRRT7 October 20, 2025 21:14
KRRT7
KRRT7 previously approved these changes Oct 20, 2025
Codeflash Bot and others added 6 commits October 21, 2025 00:40
The optimized version improves performance by **replacing the `while isinstance()` condition with a tighter `while True` loop** that explicitly checks types inside the loop body. This eliminates redundant `isinstance()` calls on each iteration.

**Key optimizations:**

1. **Eliminated redundant type checking**: The original code calls `isinstance(current, ast.Attribute)` twice per iteration - once in the while condition and again when accessing `current.value`. The optimized version uses a single `isinstance()` check per iteration.

2. **More efficient loop structure**: Changed from `while isinstance(current, ast.Attribute):` to `while True:` with explicit type checks and early exits using `continue` and `break`. This reduces function call overhead on each loop iteration.

3. **Direct variable assignment**: Uses `val = current.value` once and reuses it, avoiding repeated property access.

**Performance impact by test case type:**
- **Simple names** (`foo()`): ~133% faster due to reduced overhead in the fast path
- **Attribute chains** (`obj.bar()`, `pkg.mod.func()`): ~114-225% faster, with deeper chains seeing more benefit
- **Long chains** (100+ attributes): ~70% faster, where the loop optimization compounds significantly
- **Edge cases** (non-callable nodes): ~92-191% faster due to faster bailout paths

The optimization is particularly effective for **attribute chain resolution**, which is common in method calls and module imports - the primary use case for this AST analysis code.
…CallFinder._get_call_name-mgzrorsj

⚡️ Speed up method `FunctionCallFinder._get_call_name` by 113%
@mohammedahmed18 mohammedahmed18 requested a review from KRRT7 October 21, 2025 00:06
@codeflash-ai
Copy link
Contributor

codeflash-ai bot commented Oct 21, 2025

⚡️ Codeflash found optimizations for this PR

📄 6,107,867% (61,078.67x) speedup for install_github_app in codeflash/cli_cmds/cmd_init.py

⏱️ Runtime : 11.5 seconds 188 microseconds (best of 24 runs)

A dependent PR with the suggested changes has been created. Please review:

If you approve, it will be merged into this PR (branch lsp/init-flow).

@mohammedahmed18 mohammedahmed18 merged commit 690e7b7 into main Oct 21, 2025
21 of 22 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.

4 participants