Skip to content

Conversation

@mohammedahmed18
Copy link
Contributor

@mohammedahmed18 mohammedahmed18 commented Nov 17, 2025

PR Type

Enhancement


Description

  • Move config defaults to client

  • Remove server-side default overrides

  • Return existing config in init response

  • Simplify suggestion defaults handling


Diagram Walkthrough

flowchart LR
  Server["LSP server get_config_suggestions"] -- "no server overrides" --> Client["Client sets defaults"]
  Init["init_project response"] -- "includes existingConfig" --> Client
Loading

File Walkthrough

Relevant files
Enhancement
beta.py
Shift defaulting to client; expose existingConfig               

codeflash/lsp/beta.py

  • Stop reading server args to prefill defaults.
  • Defaults now strictly from detected suggestions.
  • init_project now returns existing config to client.
  • Response structure expanded with existingConfig field.
+11/-31 

@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 Contract Change

The init response now includes existingConfig on success and changes the payload shape. Verify all clients are updated to consume existingConfig and the unchanged keys (moduleRoot, pyprojectPath, root) and that no callers rely on previous absence of existingConfig.

return {
    "status": "success",
    "moduleRoot": args.module_root,
    "pyprojectPath": pyproject_toml_path,
    "root": root,
    "existingConfig": config,
}
Defaults Behavior

Server-side overrides were removed; defaults are always computed from discovery. Ensure clients reliably set defaults and that removing configured_* fallbacks does not regress behavior when client omits fields.

return {
    "module_root": {"choices": module_root_suggestions, "default": default_module_root},
    "tests_root": {"choices": tests_root_suggestions, "default": default_tests_root},
    "test_framework": {"choices": test_framework_suggestions, "default": default_test_framework},
    "formatter_cmds": {"choices": formatter_suggestions, "default": default_formatter},
}

@github-actions
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Restore user-configured defaults

Preserve previously configured values from server.args as defaults when available;
otherwise fall back to computed defaults. Dropping them regresses user-pre-filled
configuration in clients.

codeflash/lsp/beta.py [239-243]

+try:
+    configured_module_root = Path(server.args.module_root).relative_to(Path.cwd()) if server.args.module_root else None
+except Exception:
+    configured_module_root = None
+try:
+    configured_tests_root = Path(server.args.tests_root).relative_to(Path.cwd()) if server.args.tests_root else None
+except Exception:
+    configured_tests_root = None
+try:
+    configured_test_framework = server.args.test_framework if server.args.test_framework else None
+except Exception:
+    configured_test_framework = None
+configured_formatter = ""
+try:
+    if isinstance(server.args.formatter_cmds, list):
+        configured_formatter = " && ".join([cmd.strip() for cmd in server.args.formatter_cmds])
+    elif isinstance(server.args.formatter_cmds, str):
+        configured_formatter = server.args.formatter_cmds.strip()
+except Exception:
+    configured_formatter = "disabled"
 return {
-    "module_root": {"choices": module_root_suggestions, "default": default_module_root},
-    "tests_root": {"choices": tests_root_suggestions, "default": default_tests_root},
-    "test_framework": {"choices": test_framework_suggestions, "default": default_test_framework},
-    "formatter_cmds": {"choices": formatter_suggestions, "default": default_formatter},
+    "module_root": {"choices": module_root_suggestions, "default": configured_module_root or default_module_root},
+    "tests_root": {"choices": tests_root_suggestions, "default": configured_tests_root or default_tests_root},
+    "test_framework": {"choices": test_framework_suggestions, "default": configured_test_framework or default_test_framework},
+    "formatter_cmds": {"choices": formatter_suggestions, "default": configured_formatter or default_formatter},
 }
Suggestion importance[1-10]: 7

__

Why: Correctly identifies that the PR removed previously respected user-configured defaults and proposes reinstating them; this improves UX but is not a critical bug fix.

Medium
General
Normalize success payload schema

Ensure existingConfig is present in both success and error outcomes with a
consistent schema. Some clients may expect identical shapes; mismatches can cause
runtime errors.

codeflash/lsp/beta.py [296-302]

 return {
     "status": "success",
     "moduleRoot": args.module_root,
     "pyprojectPath": pyproject_toml_path,
     "root": root,
-    "existingConfig": config,
+    "existingConfig": config or {},
 }
Suggestion importance[1-10]: 5

__

Why: Ensures existingConfig is always present on success mirroring error shape; a modest but valid compatibility improvement with minimal risk.

Low

@aseembits93 aseembits93 merged commit ad09525 into main Nov 18, 2025
22 checks passed
@aseembits93 aseembits93 deleted the lsp/move-config-suggestion-default-values-to-the-client branch November 18, 2025 01:13
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