Skip to content

Conversation

@KRRT7
Copy link
Contributor

@KRRT7 KRRT7 commented Jul 2, 2025

PR Type

Enhancement


Description

  • Introduce suppress_errors flag in CF API requests

  • Suppress error logging when flag is enabled

  • Update CLI GitHub app installer to use suppression

  • Clean up LSP server formatting and docs


Changes walkthrough 📝

Relevant files
Enhancement
cfapi.py
Add suppress_errors flag for API requests                               

codeflash/api/cfapi.py

  • Add suppress_errors parameter to make_cfapi_request
  • Conditional error logging based on suppress_errors
  • Update is_github_app_installed_on_repo signature and calls
  • Document suppress_errors in function docstring
  • +17/-7   
    cmd_init.py
    Use suppress_errors in installer checks                                   

    codeflash/cli_cmds/cmd_init.py

  • Pass suppress_errors=True to install checks
  • Suppress warnings during GitHub app installation flow
  • Improve CLI UX by hiding unnecessary errors
  • +2/-2     
    Formatting
    server.py
    Format imports and docstrings                                                       

    codeflash/lsp/server.py

  • Reorder imports (MessageType, LogMessageParams)
  • Tidy show_message_log docstring formatting
  • Add missing commas for consistency
  • +7/-6     
    Documentation
    server_entry.py
    Refine docstring and type annotation                                         

    codeflash/lsp/server_entry.py

  • Rewrite module docstring for clarity
  • Add return type annotation to setup_logging
  • Remove extraneous blank lines
  • +4/-3     

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @github-actions
    Copy link

    github-actions bot commented Jul 2, 2025

    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

    Missing Return

    The setup_logging function signature indicates it returns a logging.Logger but lacks a return statement, causing callers to receive None instead of the logger.

    def setup_logging() -> logging.Logger:
    Uninitialized Variable

    The variable error_message may be used without being initialized if the JSON response contains neither error nor message keys and no exception is raised.

    if not suppress_errors:
        logger.error(
            f"CF_API_Error:: making request to Codeflash API (url: {url}, method: {method}, status {response.status_code}): {error_message}"
        )

    @github-actions
    Copy link

    github-actions bot commented Jul 2, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Return logger instance

    The setup_logging function signature declares a return value but doesn’t actually
    return the logger. Add return root_logger so callers receive the configured logger
    instance.

    codeflash/lsp/server_entry.py [17-20]

     def setup_logging() -> logging.Logger:
         root_logger = logging.getLogger()
         root_logger.handlers.clear()
    +    return root_logger
    Suggestion importance[1-10]: 8

    __

    Why: The function signature declares returning a logging.Logger but currently returns None, so adding return root_logger fixes this mismatch and ensures callers receive the configured logger.

    Medium

    logger.error(
    f"CF_API_Error:: making request to Codeflash API (url: {url}, method: {method}, status {response.status_code}): {error_message}"
    )
    if not suppress_errors:
    Copy link
    Contributor

    Choose a reason for hiding this comment

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

    Q: Why are we suppressing the errors?

    Saga4
    Saga4 previously approved these changes Jul 2, 2025
    @KRRT7
    Copy link
    Contributor Author

    KRRT7 commented Jul 3, 2025

    Original
    Screenshot 2025-07-02 at 5 34 29 PM
    New
    Screenshot 2025-07-02 at 5 36 01 PM

    @KRRT7 KRRT7 changed the title CF 680 don't emit unnecessary warning CF 680 don't emit unnecessary warning & remove click / inquirer reps Jul 3, 2025
    @KRRT7 KRRT7 requested a review from misrasaurabh1 July 3, 2025 00:40
    codeflash-ai bot added a commit that referenced this pull request Jul 3, 2025
    …(`better-UX`)
    
    Here are targeted and *safe* optimizations for your code, focusing primarily on the **parse_config_file** function, since it dominates the runtime (~98% of `should_modify_pyproject_toml`).  
    The main bottlenecks per the profile are both TOML parsing (external, little to be optimized from user code) and the __massive number of slow in-place config key conversions__ (`config[key.replace("-", "_")] = config[key]; del config[key]`).  
    Most of the `key in config` lookups and repeated work can be reduced by processing keys more efficiently in fewer iterations.
    
    **Key Optimizations:**
    
    1. **Single Pass Normalization:**  
       Instead of scanning the dictionary repeatedly converting hyphens to underscores, process the keys in-place in a single pass, creating a new dict with both normalized and original keys pointing to the same value, replacing `config`.  
       This is faster and safe.
    
    2. **Batch Default Handling:**  
       Instead of sequentially modifying for each key + default type, merge in default values for all missing keys at once using `.setdefault`.
    
    3. **Avoid Excessive Path Conversion/Resolving:**  
       Convert/resolve each path once, only if present, and do not build new `Path` objects multiple times.
    
    4. **Minimize Repeated `Path(...).parent` Calculations:**  
       Compute parent once.
    
    5. **Optimize `[str(cmd) for cmd in config[key]]`:**  
       Move path computations and casting to lists earlier, minimize unnecessary transformations.
    
    6. **Re-use objects and variables rather than repeated lookups.**
    
    7. **Pre-filter config keys for path work.**
    
    No changes to behavior or function signatures.  
    **All existing comments are kept where relevant.**
    
    Here is your optimized, drop-in replacement.
    
    
    
    **Summary of changes:**
    - Dramatically reduced config dict key normalization cost (single scan, not per key).
    - Minimized resolve/path operations, and batch-applied defaults.
    - The rest of the logic and all comments are unchanged.
    - No change to function names or signatures.
    
    This version will significantly reduce the overhead in `parse_config_file` due to a much more efficient key normalization and default merging logic.  
    If you want even more speed, consider switching from `tomlkit` to `tomllib` for TOML parsing if you do not require preservation of comments or formatting.
    @codeflash-ai
    Copy link
    Contributor

    codeflash-ai bot commented Jul 3, 2025

    ⚡️ Codeflash found optimizations for this PR

    📄 146% (1.46x) speedup for should_modify_pyproject_toml in codeflash/cli_cmds/cmd_init.py

    ⏱️ Runtime : 266 milliseconds 108 milliseconds (best of 11 runs)

    I created a new dependent PR with the suggested changes. Please review:

    If you approve, it will be merged into this PR (branch better-UX).

    @KRRT7 KRRT7 changed the title CF 680 don't emit unnecessary warning & remove click / inquirer reps CF 680 don't emit unnecessary warning & improve UX when using cmd init Jul 3, 2025
    @Saga4 Saga4 merged commit f8c7f2a into main Jul 3, 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