Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
Saga4 wants to merge1 commit intomain
base:main
Choose a base branch
Loading
fromsaga4/doctor_cmd_check

Conversation

@Saga4
Copy link
Contributor

@Saga4Saga4 commentedJul 2, 2025
edited by github-actionsbot
Loading

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

  • Importrun_doctor fromcmd_doctor
  • Adddoctor subparser withrun_doctor default
  • Include--doctor argument for diagnostics
  • +9/-0     
    cmd_doctor.py
    Implement doctor diagnostic logic                                               

    codeflash/cli_cmds/cmd_doctor.py

  • Createrun_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

  • Handleargs.doctor in main dispatch
  • Invokerun_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 thedocumentation for more information.
  • @Saga4Saga4 marked this pull request as draftJuly 2, 2025 13:52
    @Saga4Saga4 changed the titleDoctor cmd check for CF installationDoctor cmd check for CF installation including LSP ServerJul 2, 2025
    @github-actions
    Copy link

    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 adoctor 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

    Inprint_results, failed checks are joined withchr(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",

    @github-actions
    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                   Impact
    General
    Remove duplicate doctor flag

    Remove the global--doctor flag to avoid collision with thedoctor subcommand and
    rely solely on the subparser dispatch. This prevents ambiguous behavior when both
    args.command == "doctor" andargs.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 thedoctor 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 tworeturn True branches removes redundanthasattr logic and slightly reduces code complexity, though the original distinction was already harmless.

    Low

    codeflash-aibot added a commit that referenced this pull requestJul 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 forcheck_project_configuration incodeflash/cli_cmds/cmd_doctor.py

    ⏱️ Runtime :1.42 milliseconds693 microseconds (best of1 runs)

    📝 Explanation and detailsHere’s how you can make your program faster.
    • Use a generator expression withnext() to short-circuit and return immediately once a config file is found, instead of collecting all matches first.
    • Avoid unnecessary list allocations forfound_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 broadtry:/except: unless truly necessary, as reading filenames is very unlikely to fail and exception handling is expensive.
    • Minimize repeatedstr.join calls.

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

    Summary of changes.

    • Replaced thefor loop and manual list appending with a fast list comprehension.
    • Removed unnecessarytry:/except: block; catchingException 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:

    TestStatus
    ⚙️ Existing Unit Tests🔘None Found
    🌀 Generated Regression Tests14 Passed
    ⏪ Replay Tests🔘None Found
    🔎 Concolic Coverage Tests🔘None Found
    📊 Tests Coverage100.0%
    🌀 Generated Regression Tests and Runtime
    importosfrompathlibimportPathfromtypingimportTuple# importsimportpytest# used for our unit testsfromcodeflash.cli_cmds.cmd_doctorimportcheck_project_configuration# ---------------------- BASIC TEST CASES ----------------------deftest_no_config_files_present():# No config files present in the directoryresult,msg=check_project_configuration()deftest_pyproject_toml_present():# Only pyproject.toml presentPath("pyproject.toml").write_text("[build-system]\nrequires = []\n")result,msg=check_project_configuration()deftest_setup_py_present():# Only setup.py presentPath("setup.py").write_text("from setuptools import setup\n")result,msg=check_project_configuration()deftest_requirements_txt_present():# Only requirements.txt presentPath("requirements.txt").write_text("pytest\n")result,msg=check_project_configuration()deftest_setup_cfg_present():# Only setup.cfg presentPath("setup.cfg").write_text("[metadata]\n")result,msg=check_project_configuration()deftest_multiple_config_files_present():# Multiple config files presentPath("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_filesexpected="pyproject.toml, setup.py, setup.cfg"# ---------------------- EDGE TEST CASES ----------------------deftest_config_files_are_directories():# Create directories with the same names as config filesforfnamein ["pyproject.toml","setup.py","requirements.txt","setup.cfg"]:Path(fname).mkdir()# Path.exists() is True for directories, so should be detected as presentresult,msg=check_project_configuration()forfnamein ["pyproject.toml","setup.py","requirements.txt","setup.cfg"]:passdeftest_config_files_are_symlinks(tmp_path):# Create a real file and a symlink to it for one config filereal_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()deftest_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()deftest_permission_denied(monkeypatch):# Simulate an exception when checking for file existenceorig_exists=Path.existsdefraise_permission_error(self):raisePermissionError("Access denied")monkeypatch.setattr(Path,"exists",raise_permission_error)result,msg=check_project_configuration()deftest_files_with_similar_names():# Files with similar names should not be detectedPath("pyproject.toml.old").write_text("")Path("setup_py").write_text("")result,msg=check_project_configuration()deftest_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 ----------------------deftest_large_number_of_irrelevant_files(tmp_path):# Create 999 irrelevant files, and one valid config fileforiinrange(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()deftest_large_number_of_config_files_present(tmp_path):# Create all config files and 995 irrelevant filesforiinrange(995):        (tmp_path/f"junk_{i}.txt").write_text("junk")forfnamein ["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 orderexpected="pyproject.toml, setup.py, requirements.txt, setup.cfg"deftest_performance_with_many_files(tmp_path):# Create 500 config files with random names and only one valid config fileforiinrange(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 filesforiinrange(500):passdeftest_all_config_files_absent_with_many_files(tmp_path):# 1000 files, none of them are config filesforiinrange(1000):        (tmp_path/f"not_a_config_{i}.txt").write_text("data")os.chdir(tmp_path)result,msg=check_project_configuration()# ---------------------- ADDITIONAL EDGE CASES ----------------------deftest_empty_directory():# Directory is emptyresult,msg=check_project_configuration()deftest_config_file_is_empty():# Config file is empty, but should still be detectedPath("requirements.txt").write_text("")result,msg=check_project_configuration()deftest_config_file_is_large(tmp_path):# Config file is very largelarge_file=tmp_path/"setup.cfg"large_file.write_text("a"*10**6)# 1 MB fileos.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.importosimporttempfilefrompathlibimportPathfromtypingimportTuple# importsimportpytestfromcodeflash.cli_cmds.cmd_doctorimportcheck_project_configuration# --------------------------# Unit tests for the function# --------------------------# Helper fixture to run tests in a temporary directory@pytest.fixturedeftemp_cwd(tmp_path):"""Change working directory to a temporary path for test isolation."""old_cwd=os.getcwd()os.chdir(tmp_path)try:yieldtmp_pathfinally:os.chdir(old_cwd)# --------------------------# 1. Basic Test Cases# --------------------------deftest_no_config_files(temp_cwd):"""Test when no configuration files are present."""result,message=check_project_configuration()deftest_pyproject_toml_present(temp_cwd):"""Test when only pyproject.toml is present."""    (temp_cwd/"pyproject.toml").write_text("# sample")result,message=check_project_configuration()deftest_setup_py_present(temp_cwd):"""Test when only setup.py is present."""    (temp_cwd/"setup.py").write_text("# sample")result,message=check_project_configuration()deftest_requirements_txt_present(temp_cwd):"""Test when only requirements.txt is present."""    (temp_cwd/"requirements.txt").write_text("# sample")result,message=check_project_configuration()deftest_setup_cfg_present(temp_cwd):"""Test when only setup.cfg is present."""    (temp_cwd/"setup.cfg").write_text("# sample")result,message=check_project_configuration()deftest_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# --------------------------deftest_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()deftest_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()deftest_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()deftest_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()deftest_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()deftest_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 upconfig_file.chmod(0o644)deftest_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()deftest_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()deftest_exception_handling(monkeypatch):"""Test that an exception in Path.exists() is handled gracefully."""classDummyPath:def__init__(self,*a,**kw):passdefexists(self):raiseRuntimeError("fail!")monkeypatch.setattr("pathlib.Path",DummyPath)result,message=check_project_configuration()# --------------------------# 3. Large Scale Test Cases# --------------------------deftest_large_number_of_unrelated_files(temp_cwd):"""Test with a large number of unrelated files present."""foriinrange(900):        (temp_cwd/f"file_{i}.txt").write_text("data")result,message=check_project_configuration()deftest_large_number_of_config_files_and_others(temp_cwd):"""Test with many unrelated files and all config files present."""# Create unrelated filesforiinrange(900):        (temp_cwd/f"file_{i}.txt").write_text("data")# Create all config filesforfnamein ["pyproject.toml","setup.py","requirements.txt","setup.cfg"]:        (temp_cwd/fname).write_text("# config")result,message=check_project_configuration()# All config files should be mentionedforfnamein ["pyproject.toml","setup.py","requirements.txt","setup.cfg"]:passdeftest_large_number_of_config_files_present(temp_cwd):"""Test with all config files present and many similarly-named files."""# Create all config filesforfnamein ["pyproject.toml","setup.py","requirements.txt","setup.cfg"]:        (temp_cwd/fname).write_text("# config")# Create many files with similar namesforiinrange(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 mentionedforfnamein ["pyproject.toml","setup.py","requirements.txt","setup.cfg"]:passdeftest_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 namesforfnamein ["pyproject.toml","setup.py","requirements.txt","setup.cfg"]:        (temp_cwd/fname).mkdir()# Create many unrelated directoriesforiinrange(900):        (temp_cwd/f"dir_{i}").mkdir()result,message=check_project_configuration()# All config directories should be mentionedforfnamein ["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.fromcodeflash.cli_cmds.cmd_doctorimportcheck_project_configurationdeftest_check_project_configuration():check_project_configuration()

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

    Suggested change
    try:
    config_files= ["pyproject.toml","setup.py","requirements.txt","setup.cfg"]
    found_configs= []
    forconfig_fileinconfig_files:
    ifPath(config_file).exists():
    found_configs.append(config_file)
    iffound_configs:
    returnTrue,f"Project configuration found:{', '.join(found_configs)} ✓"
    else:
    returnFalse,"No project configuration files found (pyproject.toml, setup.py, etc.)"
    exceptExceptionase:
    returnFalse,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= [fnameforfnameinconfig_filesifPath(fname).exists()]
    iffound_configs:
    returnTrue,f"Project configuration found:{', '.join(found_configs)} ✓"
    else:
    returnFalse,"No project configuration files found (pyproject.toml, setup.py, etc.)"

    Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

    Reviewers

    @codeflash-aicodeflash-ai[bot]codeflash-ai[bot] left review comments

    At least 1 approving review is required to merge this pull request.

    Assignees

    No one assigned

    Projects

    None yet

    Milestone

    No milestone

    Development

    Successfully merging this pull request may close these issues.

    2 participants

    @Saga4

    [8]ページ先頭

    ©2009-2025 Movatter.jp