Skip to content

Conversation

@Saga4
Copy link
Contributor

@Saga4 Saga4 commented Nov 15, 2025

PR Type

Enhancement


Description

  • Improve telemetry prompt clarity

  • Add robust config suggestion fallbacks

  • Suggest only existing workspace directories

  • Default to current directory when unknown


Diagram Walkthrough

flowchart LR
  telemetry["CLI telemetry prompt copy"] -- "clarified message" --> user_prompt["User confirmation"]
  lsp["LSP config suggestions"] -- "no subdirs found" --> fallbacks["Compute sensible fallbacks"]
  fallbacks -- "include existing dirs" --> suggestions["Module/Tests roots suggestions"]
  fallbacks -- "use '.' as default" --> defaults["Default roots"]
Loading

File Walkthrough

Relevant files
Enhancement
cmd_init.py
Clarify telemetry enablement prompt                                           

codeflash/cli_cmds/cmd_init.py

  • Reword telemetry confirmation message.
  • Clarify data scope and purpose.
+1/-1     
beta.py
Robust LSP config suggestion fallbacks                                     

codeflash/lsp/beta.py

  • Add fallbacks when no subdirs detected.
  • Suggest only existing module/test directories.
  • Always include '.' as safe default.
  • Set default roots based on computed list.
+28/-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

Path handling

Ensure that returning "." in suggestions aligns with downstream consumers' expectations (e.g., JSON schema, UI rendering, and any path normalization that might require strings vs Path objects).

if not module_root_suggestions:
    cwd = Path.cwd()
    common_module_dirs = ["src", "lib", "app"]
    module_root_suggestions = ["."]  # Always include current directory

    # Add common patterns only if they exist
    for dir_name in common_module_dirs:
        if (cwd / dir_name).is_dir():
            module_root_suggestions.append(dir_name)

    default_module_root = "."

if not tests_root_suggestions:
    cwd = Path.cwd()
    common_test_dirs = ["tests", "test", "__tests__"]
    tests_root_suggestions = []

    # Add common test directories only if they exist
    for dir_name in common_test_dirs:
        if (cwd / dir_name).is_dir():
            tests_root_suggestions.append(dir_name)

    # Always include current directory as fallback
    tests_root_suggestions.append(".")
    default_tests_root = tests_root_suggestions[0] if tests_root_suggestions else "."
Ordering Default

The default tests root picks the first suggestion; when "." is appended last, the default may become a non-existent directory if none of the common ones exist. Confirm this behavior is intended.

if not tests_root_suggestions:
    cwd = Path.cwd()
    common_test_dirs = ["tests", "test", "__tests__"]
    tests_root_suggestions = []

    # Add common test directories only if they exist
    for dir_name in common_test_dirs:
        if (cwd / dir_name).is_dir():
            tests_root_suggestions.append(dir_name)

    # Always include current directory as fallback
    tests_root_suggestions.append(".")
    default_tests_root = tests_root_suggestions[0] if tests_root_suggestions else "."

@github-actions
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Avoid overriding a better default

Preserve the original default when valid suggestions exist; unconditionally setting
default_module_root to "." can override a previously better default. Set the default
to the first valid suggestion derived or keep the precomputed one if present.

codeflash/lsp/beta.py [211-222]

 if not module_root_suggestions:
     cwd = Path.cwd()
     common_module_dirs = ["src", "lib", "app"]
     module_root_suggestions = ["."]  # Always include current directory
 
-    # Add common patterns only if they exist
     for dir_name in common_module_dirs:
         if (cwd / dir_name).is_dir():
             module_root_suggestions.append(dir_name)
 
-    default_module_root = "."
+    # Prefer the first non-dot suggestion if available, otherwise keep "."; don't override an existing non-empty default
+    if default_module_root in (None, "") or default_module_root not in module_root_suggestions:
+        default_module_root = next((d for d in module_root_suggestions if d != "."), ".")
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly targets the block where default_module_root is unconditionally set to "." and proposes preserving a better precomputed default or preferring an actual directory. It's a reasonable maintainability/usability improvement, not critical.

Low
General
Prefer real test dirs as default

Ensure the chosen default prioritizes real test directories over the current
directory. Select the first discovered test dir if any; otherwise fall back to "."
without overriding a valid precomputed default.

codeflash/lsp/beta.py [223-236]

 if not tests_root_suggestions:
     cwd = Path.cwd()
     common_test_dirs = ["tests", "test", "__tests__"]
     tests_root_suggestions = []
 
-    # Add common test directories only if they exist
     for dir_name in common_test_dirs:
         if (cwd / dir_name).is_dir():
             tests_root_suggestions.append(dir_name)
 
     # Always include current directory as fallback
     tests_root_suggestions.append(".")
-    default_tests_root = tests_root_suggestions[0] if tests_root_suggestions else "."
+    # Prefer first actual test directory; only set if default is empty or not in suggestions
+    preferred = next((d for d in tests_root_suggestions if d != "."), ".")
+    if default_tests_root in (None, "") or default_tests_root not in tests_root_suggestions:
+        default_tests_root = preferred
Suggestion importance[1-10]: 6

__

Why: It improves default selection by prioritizing real test directories and avoids overwriting a valid precomputed default. This is accurate and context-aware, offering a modest but meaningful UX improvement.

Low
Deduplicate working directory resolution

Avoid redundant Path.cwd() calls; store it once to prevent inconsistencies if the
working directory changes between checks and to reduce filesystem queries. Compute
cwd once before both blocks.

codeflash/lsp/beta.py [209-236]

 # Provide sensible fallbacks when no subdirectories are found
 # Only suggest directories that actually exist in the workspace
+cwd = Path.cwd()
 if not module_root_suggestions:
-    cwd = Path.cwd()
-    ...
+    common_module_dirs = ["src", "lib", "app"]
+    module_root_suggestions = ["."]
+    for dir_name in common_module_dirs:
+        if (cwd / dir_name).is_dir():
+            module_root_suggestions.append(dir_name)
+    if default_module_root in (None, "") or default_module_root not in module_root_suggestions:
+        default_module_root = next((d for d in module_root_suggestions if d != "."), ".")
 if not tests_root_suggestions:
-    cwd = Path.cwd()
-    ...
+    common_test_dirs = ["tests", "test", "__tests__"]
+    tests_root_suggestions = []
+    for dir_name in common_test_dirs:
+        if (cwd / dir_name).is_dir():
+            tests_root_suggestions.append(dir_name)
+    tests_root_suggestions.append(".")
+    preferred = next((d for d in tests_root_suggestions if d != "."), ".")
+    if default_tests_root in (None, "") or default_tests_root not in tests_root_suggestions:
+        default_tests_root = preferred
Suggestion importance[1-10]: 5

__

Why: Consolidating Path.cwd() is logically sound and slightly reduces overhead and potential inconsistencies. Impact is minor but valid; the improved_code aligns with the new hunk context.

Low


return Confirm.ask(
"⚡️ Would you like to enable telemetry to help us improve the Codeflash experience?",
"⚡️ Help us improve Codeflash by sharing anonymous usage data (e.g., commands run, errors encountered)?",
Copy link
Contributor

Choose a reason for hiding this comment

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

we don't even get this data right now. only get posthog events. what do we get from sentry?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are not having cmd runs but errors for sure.

aseembits93
aseembits93 previously approved these changes Nov 15, 2025
@aseembits93 aseembits93 merged commit b13fac7 into main Nov 15, 2025
21 of 22 checks passed
@aseembits93 aseembits93 deleted the saga_misc_fixes_14nov branch November 15, 2025 07:14
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.

5 participants