Skip to content

Conversation

@mohammedahmed18
Copy link
Contributor

@mohammedahmed18 mohammedahmed18 commented Nov 14, 2025

PR Type

Bug fix, Enhancement


Description

  • Add config_found for pre-parse checks

  • Validate path, file type, and suffix

  • Use early config existence validation

  • Improve LSP init error messaging


Diagram Walkthrough

flowchart LR
  A["Caller (CLI/LSP)"] --> B["config_found(path)"]
  B -- "not found/invalid" --> E["Return error early"]
  B -- "found" --> C["is_valid_pyproject_toml(path)"]
  C -- "valid" --> D["Proceed (init/confirm)"]
  C -- "invalid" --> E
Loading

File Walkthrough

Relevant files
Enhancement
cmd_init.py
Introduce pre-parse config checks and refactor validation

codeflash/cli_cmds/cmd_init.py

  • Add config_found with path/file/suffix checks
  • Refactor is_valid_pyproject_toml signature to accept Union[str, Path]
  • Early-return in should_modify_pyproject_toml when config missing
  • Improve parse error message propagation
+19/-2   
Bug fix
beta.py
LSP init uses early config existence validation                   

codeflash/lsp/beta.py

  • Import and use config_found before parsing
  • Early error return with informative message in init_project
+5/-0     

@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

API Consistency

The return type of is_valid_pyproject_toml was broadened to accept Union[str, Path]; ensure all callers pass both types correctly and that Path conversion is consistently applied to avoid path-related bugs on different platforms.

def is_valid_pyproject_toml(pyproject_toml_path: Union[str, Path]) -> tuple[bool, dict[str, Any] | None, str]:
    pyproject_toml_path = Path(pyproject_toml_path)
    try:
        config, _ = parse_config_file(pyproject_toml_path)
    except Exception as e:
        return False, None, f"Failed to parse configuration: {e}"
UX Message Clarity

Error messages in config_found are generic; consider distinguishing between missing file, wrong type, and wrong suffix with actionable guidance (e.g., expected location) to improve troubleshooting.

def config_found(pyproject_toml_path: Union[str, Path]) -> tuple[bool, str]:
    pyproject_toml_path = Path(pyproject_toml_path)

    if not pyproject_toml_path.exists():
        return False, f"Configuration file not found: {pyproject_toml_path}"

    if not pyproject_toml_path.is_file():
        return False, f"Configuration file is not a file: {pyproject_toml_path}"

    if pyproject_toml_path.suffix != ".toml":
        return False, f"Configuration file is not a .toml file: {pyproject_toml_path}"

    return True, ""
Early Return Behavior

init_project now returns early on config_found failure; verify this aligns with desired LSP behavior (e.g., should it attempt creation or prompt configuration instead of erroring).

found, message = config_found(pyproject_toml_path)
if not found:
    return {"status": "error", "message": message}

valid, config, reason = is_valid_pyproject_toml(pyproject_toml_path)

@github-actions
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Default error message fallback

Guard against empty or missing messages from config_found to avoid returning an
unhelpful empty string to clients. Provide a clear default error message when
message is falsy.

codeflash/lsp/beta.py [267-270]

 found, message = config_found(pyproject_toml_path)
 if not found:
-    return {"status": "error", "message": message}
+    return {"status": "error", "message": message or f"Configuration file problem: {pyproject_toml_path}"}
Suggestion importance[1-10]: 6

__

Why: Given config_found may return an empty message on success in the PR, adding a fallback when found is False ensures clients receive a clear error. This is contextually accurate and modestly improves robustness.

Low
Return consistent success message

Normalize the returned message type by always returning a meaningful string, even
when valid. Returning an empty string may be mistaken as falsy and treated as an
error message downstream. Provide a success message or explicitly document and
enforce empty-as-success handling.

codeflash/cli_cmds/cmd_init.py [174-186]

 def config_found(pyproject_toml_path: Union[str, Path]) -> tuple[bool, str]:
     pyproject_toml_path = Path(pyproject_toml_path)
 
     if not pyproject_toml_path.exists():
         return False, f"Configuration file not found: {pyproject_toml_path}"
 
     if not pyproject_toml_path.is_file():
         return False, f"Configuration file is not a file: {pyproject_toml_path}"
 
     if pyproject_toml_path.suffix != ".toml":
         return False, f"Configuration file is not a .toml file: {pyproject_toml_path}"
 
-    return True, ""
+    return True, "Configuration file found"
Suggestion importance[1-10]: 4

__

Why: The code currently returns an empty string on success, which can be acceptable as a convention; adding a success message is a minor UX/readability improvement but not critical. The suggestion is accurate and applies cleanly to the shown function.

Low

@Saga4 Saga4 merged commit 86eb0a7 into main Nov 14, 2025
22 of 23 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