Skip to content

Conversation

mohammedahmed18
Copy link
Contributor

@mohammedahmed18 mohammedahmed18 commented Jun 26, 2025

PR Type

Enhancement


Description

  • Allow formatter to skip failures in quiet mode

  • Implement discoverFunctionTests LSP feature

  • Suppress stdout during test discovery

  • Pass discovered tests into optimizer

  • Return serialized optimization in response


Changes walkthrough 📝

Relevant files
Enhancement
formatter.py
Enable quiet-mode formatting without failure                         

codeflash/code_utils/formatter.py

  • Added check for console.quiet flag
  • Disabled exit-on-failure when in quiet (LSP) mode
  • +3/-0     
    beta.py
    Improve LSP test discovery and response payload                   

    codeflash/lsp/beta.py

  • Imported contextlib, json, and os
  • Wrapped discover_tests call in redirect_stdout
  • Stored discovered tests on server.optimizer
  • Passed tests into create_function_optimizer
  • Added optimization JSON field in response
  • +14/-6   

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @mohammedahmed18 mohammedahmed18 requested a review from KRRT7 June 26, 2025 18:52
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Devnull Open

    Using Path.open to open os.devnull is incorrect; use the built-in open(os.devnull, "w") instead.

    devnull_writer = Path.open(os.devnull, "w")
    Output Suppression

    Only stdout is redirected during test discovery; stderr may still leak and clutter logs. Consider also suppressing stderr.

    with contextlib.redirect_stdout(devnull_writer):
        function_to_tests, num_discovered_tests = server.optimizer.discover_tests(optimizable_funcs)

    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Use built-in open for devnull

    The call to Path.open is incorrect because open is an instance method on a Path
    object. Replace it with the built-in open function to correctly open the OS null
    device for writing.

    codeflash/lsp/beta.py [69]

    -devnull_writer = Path.open(os.devnull, "w")
    +devnull_writer = open(os.devnull, "w")
    Suggestion importance[1-10]: 9

    __

    Why: The call to Path.open will fail since it’s bound to a Path instance, and using the built-in open correctly opens the OS null device.

    High
    General
    Return raw optimized code

    Wrapping the optimized source in json.dumps adds extra quoting and escaping. Return
    the raw source string instead so clients receive the actual code directly.

    codeflash/lsp/beta.py [231]

    -"optimization": json.dumps(optimized_source, indent=None),
    +"optimization": optimized_source,
    Suggestion importance[1-10]: 5

    __

    Why: Wrapping optimized_source in json.dumps double-encodes the string and adds quoting, whereas returning it directly avoids unnecessary escaping.

    Low

    @@ -109,6 +109,9 @@ def format_code(
    print_status: bool = True, # noqa
    exit_on_failure: bool = True, # noqa
    ) -> str:
    if console.quiet:
    Copy link
    Contributor

    Choose a reason for hiding this comment

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

    nit: There should be a flag which limits this only for lsp.

    @Saga4 Saga4 merged commit 601e03b into main Jun 26, 2025
    16 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.

    2 participants