Skip to content

[FIX] Correct scope for new functions in the optimization context (CF-687) #490

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

mohammedahmed18
Copy link
Contributor

@mohammedahmed18 mohammedahmed18 commented Jul 3, 2025

PR Type

Bug fix, Tests


Description

  • Remove unused optimized helper functions

  • Add function usage and import tracking

  • Prevent duplicate helper file writes

  • Test new helper functions scope


Changes diagram

flowchart LR
  O["Optimized code inserted"] --> R["replace_functions_in_file"]
  R --> U["Collect function usage"]
  R --> I["Track imports"]
  U & I --> F["Determine unused functions"]
  F --> D["Remove unused functions"]
  D --> C["Cleaned module code"]
Loading

Changes walkthrough 📝

Relevant files
Formatting
1 files
constants.py
Remove stray commented metadata                                                   
+0/-6     
Enhancement
5 files
base.py
Add element serialization/deserialization functions           
+115/-0 
coordinates.py
Implement coordinate conversion system                                     
+113/-0 
elements.py
Add element classes and metadata handling                               
+989/-0 
optimized.py
Provide optimized element deserialization code                     
+165/-0 
utils.py
Define geometry type aliases                                                         
+8/-0     
Bug fix
2 files
code_replacer.py
Remove unused new helper functions                                             
+94/-2   
function_optimizer.py
Prevent duplicate helper writes                                                   
+2/-1     
Tests
1 files
test_code_replacement.py
Add test for helper scope correctness                                       
+63/-0   
Configuration changes
1 files
pyproject.toml
Add codeflash tool configuration                                                 
+7/-0     

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

    github-actions bot commented Jul 3, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

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

    Undefined variable

    module is not defined when tracking imports; the visitor should be applied to the parsed CST (e.g., modified_tree or original_module) instead of module.

    module.visit(import_tracker)
    Missing imports

    References to CoordinatesMetadata, DataSourceMetadata, and _kvform_rehydrate_internal_elements in ElementMetadata.from_dict are not imported, leading to potential NameError.

    coordinates = CoordinatesMetadata.from_dict(coords_val) if coords_val is not None else None
    
    data_source_val = key_value("data_source")
    data_source = DataSourceMetadata.from_dict(data_source_val) if data_source_val is not None else None
    
    orig_elements_val = key_value("orig_elements")
    orig_elements = (
            elements_from_base64_gzipped_json(orig_elements_val) if orig_elements_val is not None else None
    )
    
    key_value_pairs_val = key_value("key_value_pairs")
    key_value_pairs = (
            _kvform_rehydrate_internal_elements(key_value_pairs_val) if key_value_pairs_val is not None else None

    Copy link

    github-actions bot commented Jul 3, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Fix undefined AST traversal call

    The variable module is not defined in this scope and should use the parsed or
    modified CST. Replace the call to use modified_tree (or original_module) instead.
    This will correctly traverse the AST for import tracking.

    codeflash/code_utils/code_replacer.py [453-454]

     import_tracker = ImportTracker()
    -module.visit(import_tracker)
    +modified_tree.visit(import_tracker)
    Suggestion importance[1-10]: 9

    __

    Why: The call to module.visit uses an undefined module variable and should be modified_tree.visit to correctly track imports.

    High
    Simplify unused-check logic

    The removal condition is too broad and removes functions that are used and
    importable. Only functions that are neither used nor importable should be removed.
    Simplify the condition accordingly.

    codeflash/code_utils/code_replacer.py [474-476]

    -if used_and_can_be_imported or not_used_and_no_import_for_it:
    -    # we then going to remove it
    +if fn_name not in used_functions and not can_be_imported:
         unused_new_function_names.append(fn_name)
    Suggestion importance[1-10]: 7

    __

    Why: The original condition also removes functions that are used but importable, whereas only functions neither used nor importable should be pruned.

    Medium

    @mohammedahmed18 mohammedahmed18 changed the title [FIX] Correct scope for new functions in the optimization context [FIX] Correct scope for new functions in the optimization context (CF-687) Jul 3, 2025
    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