Skip to content

Conversation

@mohammedahmed18
Copy link
Contributor

@mohammedahmed18 mohammedahmed18 commented Oct 9, 2025

PR Type

Enhancement, Bug fix


Description

  • Always process args during project init

  • Remove git repository validation checks

  • Simplify init flow and dependencies

  • Return consistent success payload


Diagram Walkthrough

flowchart LR
  init["initProject handler"] -- "set args_processed_before=False" --> args["process_args(server)"]
  init -- "skip git checks" --> result["Return success with paths"]
Loading

File Walkthrough

Relevant files
Enhancement
beta.py
Always process args and drop git validations                         

codeflash/lsp/beta.py

  • Force re-processing of server args at init by resetting
    args_processed_before.
  • Remove git repository validation logic and dependency on git.
  • Keep config resolution and validation flow unchanged.
  • Streamline return payload after successful initialization.
+4/-11   

@github-actions
Copy link

github-actions bot commented Oct 9, 2025

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

Type Consistency

The success and error payloads mix Path objects with strings; ensure values like pyprojectPath are serialized to strings to avoid JSON encoding issues in the LSP response.

server.show_message_log("Validating project...", "Info")
config = is_valid_pyproject_toml(pyproject_toml_path)
if config is None:
    server.show_message_log("pyproject.toml is not valid", "Error")
    return {"status": "error", "message": "not valid", "pyprojectPath": pyproject_toml_path}

args = process_args(server)

return {"status": "success", "moduleRoot": args.module_root, "pyprojectPath": pyproject_toml_path, "root": root}
Arg Processing Order

Forcing server.args_processed_before = False ensures re-processing, but confirm no side effects for concurrent or repeated init calls; consider guarding against race conditions or unintended resets.

# Always process args in the init project, the extension can call
server.args_processed_before = False

pyproject_toml_path: Path | None = getattr(params, "config_file", None) or getattr(server.args, "config_file", None)
Return Payload Consistency

The returned keys now guarantee "moduleRoot", "pyprojectPath", and "root"; validate these align with client expectations and that root is defined in all branches before return.

args = process_args(server)

return {"status": "success", "moduleRoot": args.module_root, "pyprojectPath": pyproject_toml_path, "root": root}

@github-actions
Copy link

github-actions bot commented Oct 9, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Ensure JSON-serializable response

The response returns pyprojectPath as a Path object, which may not be
JSON-serializable for the LSP. Convert it to str (or URI) before returning to avoid
runtime serialization errors.

codeflash/lsp/beta.py [201]

 pyproject_toml_path: Path | None = getattr(params, "config_file", None) or getattr(server.args, "config_file", None)
 if pyproject_toml_path is not None:
     # if there is a config file provided use it
     server.prepare_optimizer_arguments(pyproject_toml_path)
 else:
     ...
 ...
-return {"status": "success", "moduleRoot": args.module_root, "pyprojectPath": pyproject_toml_path, "root": root}
+return {
+    "status": "success",
+    "moduleRoot": args.module_root,
+    "pyprojectPath": str(pyproject_toml_path) if pyproject_toml_path is not None else None,
+    "root": root,
+}
Suggestion importance[1-10]: 7

__

Why: Returning a Path in pyprojectPath can cause serialization issues in LSP responses; converting to str is a concrete, accurate fix with practical impact, though not critical.

Medium
Avoid unconditional state reset

Resetting server.args_processed_before unconditionally can break concurrent sessions
or override a previously processed, valid state. Guard this assignment so it only
resets when necessary, or make it idempotent per request to avoid race conditions.
This prevents inadvertent reprocessing and state thrashing.

codeflash/lsp/beta.py [160-162]

-# Always process args in the init project, the extension can call
-server.args_processed_before = False
+# Ensure idempotent, per-request processing intent without clobbering global state
+if getattr(server, "args_processed_before", None) is True:
+    server.show_message_log("Args already processed; skipping forced reset.", "Info")
+else:
+    server.args_processed_before = False
Suggestion importance[1-10]: 3

__

Why: The concern about unconditionally resetting server.args_processed_before is plausible but speculative; no concurrency context is shown. The proposed guard changes behavior and may skip intended resets, offering limited, potentially incorrect benefit.

Low

@mohammedahmed18 mohammedahmed18 requested review from KRRT7 and Saga4 October 9, 2025 18:13
return {"status": "error", "message": "not valid", "pyprojectPath": pyproject_toml_path}

args = process_args(server)
repo = git.Repo(args.module_root, search_parent_directories=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

Moved it to the VSC extension.

@Saga4 Saga4 merged commit 8599750 into main Oct 9, 2025
19 of 21 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