Skip to content

Conversation

@aseembits93
Copy link
Contributor

@aseembits93 aseembits93 commented Nov 15, 2025

PR Type

Bug fix, Enhancement


Description

  • Add defensive try/except around arg parsing

  • Prevent crashes on invalid VSC config

  • Default formatter to "disabled" on errors

  • Clear subdir cache before suggestion build


Diagram Walkthrough

flowchart LR
  Args["Server args"] -- "parse with try/except" --> Parsed["Configured values or None"]
  Parsed -- "fallback to defaults" --> Defaults["Default suggestions"]
  Formatter["formatter_cmds"] -- "join/strip or error" --> FormatterOut["Configured or 'disabled'"]
  Defaults -- "assemble response" --> Output["Config suggestions dict"]
Loading

File Walkthrough

Relevant files
Bug fix
beta.py
Robust arg parsing with safe fallbacks                                     

codeflash/lsp/beta.py

  • Wrap module/tests root parsing in try/except.
  • Guard test framework extraction with try/except.
  • Safely build formatter command, defaulting to "disabled" on errors.
  • Maintain cache clear; return defaults when config invalid.
+21/-8   

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Codeflash Bot seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@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

Broad Exception

Multiple bare except blocks mask real errors and make debugging difficult; consider catching specific exceptions (e.g., AttributeError, ValueError) and optionally logging.

try:
    configured_module_root = (
        Path(server.args.module_root).relative_to(Path.cwd()) if server.args.module_root else None
    )
except:  # noqa : E722
    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:  # noqa : E722
    configured_tests_root = None
try:
    configured_test_framework = server.args.test_framework if server.args.test_framework else None
except:  # noqa : E722
    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:  # noqa : E722
    configured_formatter = "disabled"
Default Behavior Change

Formatter default changes to "disabled" only on exception; for non-exception invalid/empty inputs, behavior may differ from expectations—clarify and ensure consistent fallback.

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:  # noqa : E722
    configured_formatter = "disabled"
Path Handling

Using Path(...).relative_to(Path.cwd()) will raise if paths are not within CWD; confirm this is desired and consider more tolerant resolution (e.g., resolve/relpath) or explicit validation.

    configured_module_root = (
        Path(server.args.module_root).relative_to(Path.cwd()) if server.args.module_root else None
    )
except:  # noqa : E722
    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:  # noqa : E722

@github-actions
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Restrict overly broad exception

Avoid bare except which hides unexpected errors and makes debugging difficult. Limit
the exception to path resolution errors to prevent swallowing unrelated failures.

codeflash/lsp/beta.py [210-215]

 try:
     configured_module_root = (
         Path(server.args.module_root).relative_to(Path.cwd()) if server.args.module_root else None
     )
-except:  # noqa : E722
+except Exception:  # limit to runtime errors without masking KeyboardInterrupt/SystemExit
     configured_module_root = None
Suggestion importance[1-10]: 7

__

Why: Replacing a bare except with except Exception avoids catching system-exiting exceptions and improves debuggability without changing intended fallback behavior. This is a reasonable robustness fix directly aligned with the new code block.

Medium
Narrow exception handling

Replace the bare except to avoid catching interrupts and system-exiting exceptions.
Constrain to a generic runtime exception to keep behavior safe while still robust.

codeflash/lsp/beta.py [216-219]

 try:
     configured_tests_root = Path(server.args.tests_root).relative_to(Path.cwd()) if server.args.tests_root else None
-except:  # noqa : E722
+except Exception:
     configured_tests_root = None
Suggestion importance[1-10]: 7

__

Why: Constraining the bare except to Exception prevents masking KeyboardInterrupt/SystemExit while keeping the same functional fallback, improving reliability with minimal risk.

Medium
Constrain exception to runtime errors

Avoid blanket exception handling which can mask logic errors and interrupts. Limit
the except clause so only runtime errors flip to the "disabled" fallback.

codeflash/lsp/beta.py [225-231]

 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:  # noqa : E722
+except Exception:
     configured_formatter = "disabled"
Suggestion importance[1-10]: 7

__

Why: Using except Exception instead of a bare except is safer and avoids swallowing critical exceptions; it maintains the same fallback to "disabled" and enhances error handling quality.

Medium

@KRRT7
Copy link
Contributor

KRRT7 commented Nov 15, 2025

the real issue is in the VSC ext, opening PR in a second once I validate

@aseembits93 aseembits93 requested a review from KRRT7 November 15, 2025 04:51
@aseembits93 aseembits93 merged commit 139144b into main Nov 15, 2025
21 of 24 checks passed
@aseembits93 aseembits93 deleted the vsc-init-fix branch November 15, 2025 04:53
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