Skip to content

Conversation

@Saga4
Copy link
Contributor

@Saga4 Saga4 commented Jul 8, 2025

PR Type

Bug fix


Description

  • Update tuple unpacking for optimizer response

  • Include third return value in two calls

  • Prevent unpacking errors in LSP beta module


Changes diagram

flowchart LR
  A["Call get_optimizable_functions()"]
  B_old["Unpack (optimizable_funcs, _)"]
  B_new["Unpack (optimizable_funcs, _, _)"]
  A -- "before" --> B_old
  A -- "after" --> B_new
Loading

Changes walkthrough 📝

Relevant files
Bug fix
beta.py
Unpack correct return tuple size                                                 

codeflash/lsp/beta.py

  • Adjusted unpacking to match three return values
  • Updated in get_optimizable_functions function
  • Updated in initialize_function_optimization function
  • +2/-2     

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @Saga4 Saga4 requested a review from mohammedahmed18 July 8, 2025 06:48
    @Saga4 Saga4 merged commit e17d0f7 into main Jul 8, 2025
    15 checks passed
    @github-actions
    Copy link

    github-actions bot commented Jul 8, 2025

    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

    Unpacking Error

    The code now unpacks three values from get_optimizable_functions(). If that function in some contexts still returns only two values, this will raise a ValueError.

    optimizable_funcs, _, _ = server.optimizer.get_optimizable_functions()
    Ignored Return Values

    The second and third return values from get_optimizable_functions() are ignored. Verify whether they carry important information that should be handled or documented.

    optimizable_funcs, _, _ = server.optimizer.get_optimizable_functions()
    Performance Concern

    get_optimizable_functions() is called twice in quick succession (in both handlers). Consider caching its result to avoid redundant or expensive operations.

            optimizable_funcs, _, _ = server.optimizer.get_optimizable_functions()
    
            path_to_qualified_names = {}
            for path, functions in optimizable_funcs.items():
                path_to_qualified_names[path.as_posix()] = [func.qualified_name for func in functions]
    
            server.show_message_log(
                f"Found {len(path_to_qualified_names)} files with functions: {path_to_qualified_names}", "Info"
            )
            return path_to_qualified_names
        finally:
            # Restore original args to prevent state corruption
            if original_file is not None:
                server.optimizer.args.file = original_file
            if original_function is not None:
                server.optimizer.args.function = original_function
            else:
                server.optimizer.args.function = None
            if original_checkpoint is not None:
                server.optimizer.args.previous_checkpoint_functions = original_checkpoint
    
            server.show_message_log(
                f"Restored args - file: {server.optimizer.args.file}, function: {server.optimizer.args.function}", "Info"
            )
    
    
    @server.feature("initializeFunctionOptimization")
    def initialize_function_optimization(
        server: CodeflashLanguageServer, params: FunctionOptimizationParams
    ) -> dict[str, str]:
        file_path = Path(uris.to_fs_path(params.textDocument.uri))
        server.show_message_log(f"Initializing optimization for function: {params.functionName} in {file_path}", "Info")
    
        # IMPORTANT: Store the specific function for optimization, but don't corrupt global state
        server.optimizer.args.function = params.functionName
        server.optimizer.args.file = file_path
    
        server.show_message_log(
            f"Args set - function: {server.optimizer.args.function}, file: {server.optimizer.args.file}", "Info"
        )
    
        optimizable_funcs, _, _ = server.optimizer.get_optimizable_functions()

    @github-actions
    Copy link

    github-actions bot commented Jul 8, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Use flexible tuple unpacking

    Use star unpacking to ignore any extra return values, making the code resilient to
    changes in the function’s return signature. This prevents a ValueError if the number
    of returned elements changes in the future.

    codeflash/lsp/beta.py [55]

    -optimizable_funcs, _, _ = server.optimizer.get_optimizable_functions()
    +optimizable_funcs, *_ = server.optimizer.get_optimizable_functions()
    Suggestion importance[1-10]: 4

    __

    Why: Using *_ makes the unpacking resilient to extra return values and matches that only optimizable_funcs is needed, but it’s a minor stylistic improvement.

    Low

    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.

    1 participant