Skip to content

Doctor cmd check for CF installation including LSP Server #481

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Saga4
Copy link
Contributor

@Saga4 Saga4 commented Jul 2, 2025

PR Type

Enhancement


Description

  • Add 'doctor' CLI subcommand for diagnostics

  • Implement setup checks: Python, Codeflash, VS Code

  • Integrate LSP, Git, and config file verifications

  • Hook '--doctor' flag into main entrypoint


Changes diagram

flowchart LR
  A["CLI parser"] -- "doctor cmd" --> B["run_doctor()"]
  B -- "execute checks" --> C["Env\nInstall\nVS Code\nLSP\nGit\nConfig"]
  C -- "print diagnosis" --> D["print_results()"]
Loading

Changes walkthrough 📝

Relevant files
Enhancement
cli.py
Add doctor sub-command to CLI                                                       

codeflash/cli_cmds/cli.py

  • Import run_doctor from cmd_doctor
  • Add doctor subparser with run_doctor default
  • Include --doctor argument for diagnostics
  • +9/-0     
    cmd_doctor.py
    Implement doctor diagnostic logic                                               

    codeflash/cli_cmds/cmd_doctor.py

  • Create run_doctor diagnostic entrypoint
  • Implement checks: Python, installation, VS Code extension
  • Add LSP server, Git repo, and project config verifications
  • Format and display results with paneled output
  • +187/-0 
    main.py
    Hook doctor flag into main                                                             

    codeflash/main.py

  • Handle args.doctor in main dispatch
  • Invoke run_doctor() when --doctor used
  • +3/-0     

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @Saga4 Saga4 marked this pull request as draft July 2, 2025 13:52
    @Saga4 Saga4 changed the title Doctor cmd check for CF installation Doctor cmd check for CF installation including LSP Server Jul 2, 2025
    Copy link

    github-actions bot commented Jul 2, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

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

    Subcommand vs flag conflict

    The PR adds both a doctor subcommand and a global --doctor flag, which may collide or confuse users. Consider consolidating to a single interface.

    doctor_parser = subparsers.add_parser("doctor", help="Verify Codeflash setup and diagnose issues")
    doctor_parser.set_defaults(func=run_doctor)
    parser.add_argument("--file", help="Try to optimize only this file")
    parser.add_argument("--function", help="Try to optimize only this function within the given file path")
    parser.add_argument(
        "--all",
        help="Try to optimize all functions. Can take a really long time. Can pass an optional starting directory to"
        " optimize code from. If no args specified (just --all), will optimize all code in the project.",
        nargs="?",
        const="",
        default=SUPPRESS,
    )
    parser.add_argument(
        "--module-root",
        type=str,
        help="Path to the project's Python module that you want to optimize."
        " This is the top-level root directory where all the Python source code is located.",
    )
    parser.add_argument(
        "--tests-root", type=str, help="Path to the test directory of the project, where all the tests are located."
    )
    parser.add_argument("--test-framework", choices=["pytest", "unittest"], default="pytest")
    parser.add_argument("--config-file", type=str, help="Path to the pyproject.toml with codeflash configs.")
    parser.add_argument("--replay-test", type=str, nargs="+", help="Paths to replay test to optimize functions from")
    parser.add_argument(
        "--no-pr", action="store_true", help="Do not create a PR for the optimization, only update the code locally."
    )
    parser.add_argument(
        "--verify-setup",
        action="store_true",
        help="Verify that codeflash is set up correctly by optimizing bubble sort as a test.",
    )
    parser.add_argument(
        "--doctor",
        action="store_true",
        help="Run setup diagnostics to verify Codeflash installation and configuration.",
    )
    Failed checks formatting

    In print_results, failed checks are joined with chr(10) under one bullet. Use a clearer per-item bullet list (e.g., "\n• ".join) to improve readability.

    f"⚠️  Setup Issues Detected\n\n"
    f"The following checks failed:\n"
    f"• {chr(10).join(failed_checks)}\n\n"
    f"Please address these issues and run 'codeflash doctor' again.\n\n"
    f"For help, visit: https://codeflash.ai/docs",

    Copy link

    github-actions bot commented Jul 2, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Remove duplicate doctor flag

    Remove the global --doctor flag to avoid collision with the doctor subcommand and
    rely solely on the subparser dispatch. This prevents ambiguous behavior when both
    args.command == "doctor" and args.doctor exist.

    codeflash/cli_cmds/cli.py [27-63]

     doctor_parser = subparsers.add_parser("doctor", help="Verify Codeflash setup and diagnose issues")
     doctor_parser.set_defaults(func=run_doctor)
    -parser.add_argument(
    -    "--doctor",
    -    action="store_true",
    -    help="Run setup diagnostics to verify Codeflash installation and configuration.",
    -)
    Suggestion importance[1-10]: 7

    __

    Why: The --doctor global flag duplicates the doctor subparser command and can lead to ambiguous CLI behavior, so removing it clarifies command dispatch without introducing new functionality.

    Medium
    Fix failed checks bullet list

    Improve the bullet list formatting by prefixing each failed check on its own line.
    Use "\n• ".join(...) to ensure each item is clearly separated and consistently
    marked.

    codeflash/cli_cmds/cmd_doctor.py [178-183]

     paneled_text(
    -    f"⚠️  Setup Issues Detected\n\n"
    -    f"The following checks failed:\n"
    -    f"• {chr(10).join(failed_checks)}\n\n"
    -    f"Please address these issues and run 'codeflash doctor' again.\n\n"
    -    f"For help, visit: https://codeflash.ai/docs",
    +    "⚠️  Setup Issues Detected\n\n"
    +    "The following checks failed:\n"
    +    f"• {'\n• '.join(failed_checks)}\n\n"
    +    "Please address these issues and run 'codeflash doctor' again.\n\n"
    +    "For help, visit: https://codeflash.ai/docs",
    Suggestion importance[1-10]: 4

    __

    Why: Adjusting the join to "\n• ".join(failed_checks) improves readability by ensuring each failed check appears on its own bullet line, but it's a minor formatting enhancement.

    Low
    Simplify LSP connectivity check

    Simplify the LSP server connectivity check by merging these two branches into a
    single success path. This avoids redundant code and clarifies that either case is
    acceptable.

    codeflash/cli_cmds/cmd_doctor.py [101-104]

    -if hasattr(server_class, 'initialize_optimizer'):
    -    return True, "LSP server available ✓"
    -else:
    -    return True, "LSP server module loaded successfully ✓"
    +return True, "LSP server module loaded successfully ✓"
    Suggestion importance[1-10]: 4

    __

    Why: Consolidating the two return True branches removes redundant hasattr logic and slightly reduces code complexity, though the original distinction was already harmless.

    Low

    codeflash-ai bot added a commit that referenced this pull request Jul 2, 2025
    …(`saga4/doctor_cmd_check`)
    
    Here’s how you can make your program faster.
    
    - Use a generator expression with `next()` to short-circuit and return immediately once a config file is found, instead of collecting all matches first.
    - Avoid unnecessary list allocations for `found_configs` if you only need to check if any file exists and display their names.
    - For speed, batch existence checks with a single loop, but short-circuit if only presence is needed. However, since the result prints all matches, collecting is still required.
    - Remove broad `try:/except:` unless truly necessary, as reading filenames is very unlikely to fail and exception handling is expensive.
    - Minimize repeated `str.join` calls.
    
    Below is an optimized version that is faster, memory-friendly, and functionally identical.
    
    
    
    **Summary of changes**.
    - Replaced the `for` loop and manual list appending with a fast list comprehension.
    - Removed unnecessary `try:/except:` block; catching `Exception` in this context is rarely helpful and slows the "happy path."
      
    This will improve both performance and readability while delivering the same results.
    Comment on lines +136 to +150
    try:
    config_files = ["pyproject.toml", "setup.py", "requirements.txt", "setup.cfg"]
    found_configs = []

    for config_file in config_files:
    if Path(config_file).exists():
    found_configs.append(config_file)

    if found_configs:
    return True, f"Project configuration found: {', '.join(found_configs)} ✓"
    else:
    return False, "No project configuration files found (pyproject.toml, setup.py, etc.)"

    except Exception as e:
    return False, f"Project configuration check failed: {e}"
    Copy link
    Contributor

    Choose a reason for hiding this comment

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

    ⚡️Codeflash found 105% (1.05x) speedup for check_project_configuration in codeflash/cli_cmds/cmd_doctor.py

    ⏱️ Runtime : 1.42 milliseconds 693 microseconds (best of 1 runs)

    📝 Explanation and details Here’s how you can make your program faster.
    • Use a generator expression with next() to short-circuit and return immediately once a config file is found, instead of collecting all matches first.
    • Avoid unnecessary list allocations for found_configs if you only need to check if any file exists and display their names.
    • For speed, batch existence checks with a single loop, but short-circuit if only presence is needed. However, since the result prints all matches, collecting is still required.
    • Remove broad try:/except: unless truly necessary, as reading filenames is very unlikely to fail and exception handling is expensive.
    • Minimize repeated str.join calls.

    Below is an optimized version that is faster, memory-friendly, and functionally identical.

    Summary of changes.

    • Replaced the for loop and manual list appending with a fast list comprehension.
    • Removed unnecessary try:/except: block; catching Exception in this context is rarely helpful and slows the "happy path."

    This will improve both performance and readability while delivering the same results.

    Correctness verification report:

    Test Status
    ⚙️ Existing Unit Tests 🔘 None Found
    🌀 Generated Regression Tests 14 Passed
    ⏪ Replay Tests 🔘 None Found
    🔎 Concolic Coverage Tests 🔘 None Found
    📊 Tests Coverage 100.0%
    🌀 Generated Regression Tests and Runtime
    import os
    from pathlib import Path
    from typing import Tuple
    
    # imports
    import pytest  # used for our unit tests
    from codeflash.cli_cmds.cmd_doctor import check_project_configuration
    
    # ---------------------- BASIC TEST CASES ----------------------
    
    def test_no_config_files_present():
        # No config files present in the directory
        result, msg = check_project_configuration()
    
    def test_pyproject_toml_present():
        # Only pyproject.toml present
        Path("pyproject.toml").write_text("[build-system]\nrequires = []\n")
        result, msg = check_project_configuration()
    
    def test_setup_py_present():
        # Only setup.py present
        Path("setup.py").write_text("from setuptools import setup\n")
        result, msg = check_project_configuration()
    
    def test_requirements_txt_present():
        # Only requirements.txt present
        Path("requirements.txt").write_text("pytest\n")
        result, msg = check_project_configuration()
    
    def test_setup_cfg_present():
        # Only setup.cfg present
        Path("setup.cfg").write_text("[metadata]\n")
        result, msg = check_project_configuration()
    
    def test_multiple_config_files_present():
        # Multiple config files present
        Path("pyproject.toml").write_text("")
        Path("setup.py").write_text("")
        Path("setup.cfg").write_text("")
        result, msg = check_project_configuration()
        # Should be in the order defined in config_files
        expected = "pyproject.toml, setup.py, setup.cfg"
    
    # ---------------------- EDGE TEST CASES ----------------------
    
    def test_config_files_are_directories():
        # Create directories with the same names as config files
        for fname in ["pyproject.toml", "setup.py", "requirements.txt", "setup.cfg"]:
            Path(fname).mkdir()
        # Path.exists() is True for directories, so should be detected as present
        result, msg = check_project_configuration()
        for fname in ["pyproject.toml", "setup.py", "requirements.txt", "setup.cfg"]:
            pass
    
    def test_config_files_are_symlinks(tmp_path):
        # Create a real file and a symlink to it for one config file
        real_file = tmp_path / "actual_config"
        real_file.write_text("foo")
        symlink = tmp_path / "setup.py"
        symlink.symlink_to(real_file)
        os.chdir(tmp_path)
        result, msg = check_project_configuration()
    
    def test_config_file_with_special_characters(tmp_path):
        # Create a file with a similar name but extra characters (should not be detected)
        (tmp_path / "pyproject.toml.bak").write_text("backup")
        os.chdir(tmp_path)
        result, msg = check_project_configuration()
    
    def test_permission_denied(monkeypatch):
        # Simulate an exception when checking for file existence
        orig_exists = Path.exists
        def raise_permission_error(self):
            raise PermissionError("Access denied")
        monkeypatch.setattr(Path, "exists", raise_permission_error)
        result, msg = check_project_configuration()
    
    def test_files_with_similar_names():
        # Files with similar names should not be detected
        Path("pyproject.toml.old").write_text("")
        Path("setup_py").write_text("")
        result, msg = check_project_configuration()
    
    def test_config_files_case_sensitivity():
        # Create config files with uppercase names (should not be detected on case-sensitive FS)
        Path("PYPROJECT.TOML").write_text("")
        Path("SETUP.PY").write_text("")
        result, msg = check_project_configuration()
    
    # ---------------------- LARGE SCALE TEST CASES ----------------------
    
    def test_large_number_of_irrelevant_files(tmp_path):
        # Create 999 irrelevant files, and one valid config file
        for i in range(999):
            (tmp_path / f"random_file_{i}.txt").write_text("irrelevant")
        (tmp_path / "requirements.txt").write_text("pytest")
        os.chdir(tmp_path)
        result, msg = check_project_configuration()
    
    def test_large_number_of_config_files_present(tmp_path):
        # Create all config files and 995 irrelevant files
        for i in range(995):
            (tmp_path / f"junk_{i}.txt").write_text("junk")
        for fname in ["pyproject.toml", "setup.py", "requirements.txt", "setup.cfg"]:
            (tmp_path / fname).write_text("")
        os.chdir(tmp_path)
        result, msg = check_project_configuration()
        # All config files should be present in the message, in the correct order
        expected = "pyproject.toml, setup.py, requirements.txt, setup.cfg"
    
    def test_performance_with_many_files(tmp_path):
        # Create 500 config files with random names and only one valid config file
        for i in range(500):
            (tmp_path / f"config_{i}.toml").write_text("foo")
        (tmp_path / "setup.cfg").write_text("")
        os.chdir(tmp_path)
        result, msg = check_project_configuration()
        # Should not mention any of the random config files
        for i in range(500):
            pass
    
    def test_all_config_files_absent_with_many_files(tmp_path):
        # 1000 files, none of them are config files
        for i in range(1000):
            (tmp_path / f"not_a_config_{i}.txt").write_text("data")
        os.chdir(tmp_path)
        result, msg = check_project_configuration()
    
    # ---------------------- ADDITIONAL EDGE CASES ----------------------
    
    def test_empty_directory():
        # Directory is empty
        result, msg = check_project_configuration()
    
    def test_config_file_is_empty():
        # Config file is empty, but should still be detected
        Path("requirements.txt").write_text("")
        result, msg = check_project_configuration()
    
    def test_config_file_is_large(tmp_path):
        # Config file is very large
        large_file = tmp_path / "setup.cfg"
        large_file.write_text("a" * 10**6)  # 1 MB file
        os.chdir(tmp_path)
        result, msg = check_project_configuration()
    # codeflash_output is used to check that the output of the original code is the same as that of the optimized code.
    
    import os
    import tempfile
    from pathlib import Path
    from typing import Tuple
    
    # imports
    import pytest
    from codeflash.cli_cmds.cmd_doctor import check_project_configuration
    
    # --------------------------
    # Unit tests for the function
    # --------------------------
    
    # Helper fixture to run tests in a temporary directory
    @pytest.fixture
    def temp_cwd(tmp_path):
        """Change working directory to a temporary path for test isolation."""
        old_cwd = os.getcwd()
        os.chdir(tmp_path)
        try:
            yield tmp_path
        finally:
            os.chdir(old_cwd)
    
    # --------------------------
    # 1. Basic Test Cases
    # --------------------------
    
    def test_no_config_files(temp_cwd):
        """Test when no configuration files are present."""
        result, message = check_project_configuration()
    
    def test_pyproject_toml_present(temp_cwd):
        """Test when only pyproject.toml is present."""
        (temp_cwd / "pyproject.toml").write_text("# sample")
        result, message = check_project_configuration()
    
    def test_setup_py_present(temp_cwd):
        """Test when only setup.py is present."""
        (temp_cwd / "setup.py").write_text("# sample")
        result, message = check_project_configuration()
    
    def test_requirements_txt_present(temp_cwd):
        """Test when only requirements.txt is present."""
        (temp_cwd / "requirements.txt").write_text("# sample")
        result, message = check_project_configuration()
    
    def test_setup_cfg_present(temp_cwd):
        """Test when only setup.cfg is present."""
        (temp_cwd / "setup.cfg").write_text("# sample")
        result, message = check_project_configuration()
    
    def test_multiple_config_files_present(temp_cwd):
        """Test when multiple config files are present."""
        (temp_cwd / "pyproject.toml").write_text("# sample")
        (temp_cwd / "setup.py").write_text("# sample")
        result, message = check_project_configuration()
    
    # --------------------------
    # 2. Edge Test Cases
    # --------------------------
    
    def test_config_file_as_directory(temp_cwd):
        """Test when a config file name exists as a directory, not a file."""
        (temp_cwd / "pyproject.toml").mkdir()
        result, message = check_project_configuration()
    
    def test_config_file_hidden(temp_cwd):
        """Test when a config file is hidden (should not be detected)."""
        (temp_cwd / ".pyproject.toml").write_text("# hidden config")
        result, message = check_project_configuration()
    
    def test_config_file_with_similar_name(temp_cwd):
        """Test when a file with a similar name exists (should not be detected)."""
        (temp_cwd / "pyproject.toml.bak").write_text("# backup")
        result, message = check_project_configuration()
    
    def test_config_file_case_sensitivity(temp_cwd):
        """Test case sensitivity: 'PyProject.toml' should not be detected on case-sensitive filesystems."""
        (temp_cwd / "PyProject.toml").write_text("# wrong case")
        result, message = check_project_configuration()
    
    def test_config_file_symlink(temp_cwd):
        """Test when a config file is a symlink to a real file."""
        real_file = temp_cwd / "real_requirements.txt"
        real_file.write_text("# real")
        symlink = temp_cwd / "requirements.txt"
        symlink.symlink_to(real_file)
        result, message = check_project_configuration()
    
    def test_config_file_permission_denied(temp_cwd):
        """Test when a config file exists but is not readable (should still be detected)."""
        config_file = temp_cwd / "setup.cfg"
        config_file.write_text("# sample")
        # Remove read permissions (if possible)
        try:
            config_file.chmod(0)
            result, message = check_project_configuration()
        finally:
            # Restore permissions so temp dir can be cleaned up
            config_file.chmod(0o644)
    
    def test_config_file_in_subdirectory(temp_cwd):
        """Test that config files in subdirectories are NOT detected."""
        subdir = temp_cwd / "subdir"
        subdir.mkdir()
        (subdir / "pyproject.toml").write_text("# sample")
        result, message = check_project_configuration()
    
    def test_config_file_with_spaces_in_name(temp_cwd):
        """Test that files with spaces in their name are not detected."""
        (temp_cwd / "pyproject toml").write_text("# sample")
        result, message = check_project_configuration()
    
    def test_exception_handling(monkeypatch):
        """Test that an exception in Path.exists() is handled gracefully."""
        class DummyPath:
            def __init__(self, *a, **kw): pass
            def exists(self): raise RuntimeError("fail!")
        monkeypatch.setattr("pathlib.Path", DummyPath)
        result, message = check_project_configuration()
    
    # --------------------------
    # 3. Large Scale Test Cases
    # --------------------------
    
    def test_large_number_of_unrelated_files(temp_cwd):
        """Test with a large number of unrelated files present."""
        for i in range(900):
            (temp_cwd / f"file_{i}.txt").write_text("data")
        result, message = check_project_configuration()
    
    def test_large_number_of_config_files_and_others(temp_cwd):
        """Test with many unrelated files and all config files present."""
        # Create unrelated files
        for i in range(900):
            (temp_cwd / f"file_{i}.txt").write_text("data")
        # Create all config files
        for fname in ["pyproject.toml", "setup.py", "requirements.txt", "setup.cfg"]:
            (temp_cwd / fname).write_text("# config")
        result, message = check_project_configuration()
        # All config files should be mentioned
        for fname in ["pyproject.toml", "setup.py", "requirements.txt", "setup.cfg"]:
            pass
    
    def test_large_number_of_config_files_present(temp_cwd):
        """Test with all config files present and many similarly-named files."""
        # Create all config files
        for fname in ["pyproject.toml", "setup.py", "requirements.txt", "setup.cfg"]:
            (temp_cwd / fname).write_text("# config")
        # Create many files with similar names
        for i in range(900):
            (temp_cwd / f"pyproject.toml_{i}").write_text("data")
            (temp_cwd / f"setup.py_{i}").write_text("data")
        result, message = check_project_configuration()
        # Only the exact config files should be mentioned
        for fname in ["pyproject.toml", "setup.py", "requirements.txt", "setup.cfg"]:
            pass
    
    def test_large_scale_edge_case_all_dirs(temp_cwd):
        """Test with a large number of directories named like config files (should still detect them)."""
        # Create directories with config file names
        for fname in ["pyproject.toml", "setup.py", "requirements.txt", "setup.cfg"]:
            (temp_cwd / fname).mkdir()
        # Create many unrelated directories
        for i in range(900):
            (temp_cwd / f"dir_{i}").mkdir()
        result, message = check_project_configuration()
        # All config directories should be mentioned
        for fname in ["pyproject.toml", "setup.py", "requirements.txt", "setup.cfg"]:
            pass
    # codeflash_output is used to check that the output of the original code is the same as that of the optimized code.
    
    from codeflash.cli_cmds.cmd_doctor import check_project_configuration
    
    def test_check_project_configuration():
        check_project_configuration()

    To test or edit this optimization locally git merge codeflash/optimize-pr481-2025-07-02T14.03.32

    Suggested change
    try:
    config_files = ["pyproject.toml", "setup.py", "requirements.txt", "setup.cfg"]
    found_configs = []
    for config_file in config_files:
    if Path(config_file).exists():
    found_configs.append(config_file)
    if found_configs:
    return True, f"Project configuration found: {', '.join(found_configs)} ✓"
    else:
    return False, "No project configuration files found (pyproject.toml, setup.py, etc.)"
    except Exception as e:
    return False, f"Project configuration check failed: {e}"
    config_files = ["pyproject.toml", "setup.py", "requirements.txt", "setup.cfg"]
    # Use list comprehension for efficiency and readability
    found_configs = [fname for fname in config_files if Path(fname).exists()]
    if found_configs:
    return True, f"Project configuration found: {', '.join(found_configs)} ✓"
    else:
    return False, "No project configuration files found (pyproject.toml, setup.py, etc.)"

    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