- 
                Notifications
    
You must be signed in to change notification settings  - Fork 22
 
[FIX] Respect parent classes in revert helpers #763
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
[FIX] Respect parent classes in revert helpers #763
Conversation
          PR Reviewer Guide 🔍Here are some key observations to aid the review process: 
  | 
    
          PR Code Suggestions ✨Explore these optional code suggestions: 
  | 
    
The optimized version eliminates recursive function calls by replacing the recursive `_find` helper with an iterative approach. This provides significant performance benefits: **Key Optimizations:** 1. **Removed Recursion Overhead**: The original code used a recursive helper function `_find` that created new stack frames for each parent traversal. The optimized version uses a simple iterative loop that traverses parents sequentially without function call overhead. 2. **Eliminated Function Creation**: The original code defined the `_find` function on every call to `find_target_node`. The optimized version removes this repeated function definition entirely. 3. **Early Exit with for-else**: The optimized code uses Python's `for-else` construct to immediately return `None` when a parent class isn't found, avoiding unnecessary continued searching. 4. **Reduced Attribute Access**: By caching `function_to_optimize.function_name` in a local variable `target_name` and reusing `body` variables, the code reduces repeated attribute lookups. **Performance Impact by Test Case:** - **Simple cases** (top-level functions, basic class methods): 23-62% faster due to eliminated recursion overhead - **Nested class scenarios**: 45-84% faster, with deeper nesting showing greater improvements as recursion elimination has more impact - **Large-scale tests**: 12-22% faster, showing consistent benefits even with many nodes to traverse - **Edge cases** (empty modules, non-existent classes): 52-76% faster due to more efficient early termination The optimization is particularly effective for deeply nested class hierarchies where the original recursive approach created multiple stack frames, while the iterative version maintains constant memory usage regardless of nesting depth.
          ⚡️ Codeflash found optimizations for this PR📄 18% (0.18x) speedup for 
 | 
    
…25-09-25T14.28.58 ⚡️ Speed up function `find_target_node` by 18% in PR #763 (`fix/correctly-find-funtion-node-when-reverting-helpers`)
| 
           This PR is now faster! 🚀 @mohammedahmed18 accepted my optimizations from:  | 
    
| 
           @mohammedahmed18 can you share an example (screenshot) of an instance when the old way of reverting was causing the issue? it's slightly hard to directly follow from the code, thanks!  | 
    
| 
           @aseembits93  yeah I kept the logs, this is the optimization of  INFO     2025-09-24 15:58:09,036 [/home/mohammed/Work/codeflash/codeflash/lsp/lsp_logger.py:100 in function enhanced_log] Best candidate 🚀                                                                                                 
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
   1 # file: codeflash/lsp/helpers.py                                                                                                                                                                                                       
   2 def simplify_worktree_paths(msg: str, highlight: bool = True) -> str:  # noqa: FBT001, FBT002                                                                                                                                          
   3     m = worktree_path_regex.search(msg)                                                                                                                                                                                                
   4     if m:                                                                                                                                                                                                                              
   5         # More efficient way to get last path part                                                                                                                                                                                     
   6         last_part_of_path = m.group(0).rpartition('/')[-1]                                                                                                                                                                             
   7         if highlight:                                                                                                                                                                                                                  
   8             last_part_of_path = f"`{last_part_of_path}`"                                                                                                                                                                               
   9         return msg.replace(m.group(0), last_part_of_path)                                                                                                                                                                              
  10     return msg                                                                                                                                                                                                                         
  11                                                                                                                                                                                                                                        
  12 def replace_quotes_with_backticks(text: str) -> str:                                                                                                                                                                                   
  13     # Efficient string substitution, reduces intermediate string allocations                                                                                                                                                           
  14     return _single_quote_pat.sub(                                                                                                                                                                                                      
  15         r"`\1`",                                                                                                                                                                                                                       
  16         _double_quote_pat.sub(r"`\1`", text),                                                                                                                                                                                          
  17     )                                                                                                                                                                                                                                  
  18 # file: codeflash/lsp/lsp_message.py                                                                                                                                                                                                   
  19 from __future__ import annotations                                                                                                                                                                                                     
  20                                                                                                                                                                                                                                        
  21 import json                                                                                                                                                                                                                            
  22 from dataclasses import asdict, dataclass                                                                                                                                                                                              
  23                                                                                                                                                                                                                                        
  24 from codeflash.lsp.helpers import (replace_quotes_with_backticks,                                                                                                                                                                      
  25                                    simplify_worktree_paths)                                                                                                                                                                            
  26                                                                                                                                                                                                                                        
  27                                                                                                                                                                                                                                        
  28 @dataclass                                                                                                                                                                                                                             
  29 class LspMessage:                                                                                                                                                                                                                      
  30                                                                                                                                                                                                                                        
  31     def serialize(self) -> str:                                                                                                                                                                                                        
  32         # Use local variable to minimize lookup costs and avoid unnecessary dictionary unpacking                                                                                                                                       
  33         data = self._loop_through(asdict(self))                                                                                                                                                                                        
  34         msg_type = self.type()                                                                                                                                                                                                         
  35         ordered = {'type': msg_type}                                                                                                                                                                                                   
  36         ordered.update(data)                                                                                                                                                                                                           
  37         return (                                                                                                                                                                                                                       
  38             message_delimiter                                                                                                                                                                                                          
  39             + json.dumps(ordered)                                                                                                                                                                                                      
  40             + message_delimiter  # \u241F is the message delimiter becuase it can be more than one message sent over the same message, so we need something to separate each message                                                   
  41         )                                                                                                                                                                                                                              
  42                                                                                                                                                                                                                                        
  43 @dataclass                                                                                                                                                                                                                             
  44 class LspMarkdownMessage(LspMessage):                                                                                                                                                                                                  
  45                                                                                                                                                                                                                                        
  46     def serialize(self) -> str:                                                                                                                                                                                                        
  47         # Side effect required, must preserve for behavioral correctness                                                                                                                                                               
  48         self.markdown = simplify_worktree_paths(self.markdown)                                                                                                                                                                         
  49         self.markdown = replace_quotes_with_backticks(self.markdown)                                                                                                                                                                   
  50         return super().serialize()                                                                                                                                                                                                     
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
DEBUG    2025-09-24 15:58:09,421 [/home/mohammed/Work/codeflash/.venv/lib/python3.12/site-packages/urllib3/connectionpool.py:544 in function _make_request] https://us.i.posthog.com:443 "POST /batch/ HTTP/1.1" 200 15                     
DEBUG    2025-09-24 15:58:10,018 [/home/mohammed/Work/codeflash/codeflash/lsp/lsp_logger.py:100 in function enhanced_log] Could not find entrypoint function serialize in optimized code                                                    
DEBUG    2025-09-24 15:58:10,020 [/home/mohammed/Work/codeflash/codeflash/lsp/lsp_logger.py:100 in function enhanced_log] Functions called in optimized entrypoint: {'LspMarkdownMessage._loop_through', 'LspMarkdownMessage.type', 'type', 
         'dumps', 'ordered.update', '_loop_through', 'asdict', 'update', 'json.dumps'}                                                                                                                                                      
DEBUG    2025-09-24 15:58:10,023 [/home/mohammed/Work/codeflash/codeflash/lsp/lsp_logger.py:100 in function enhanced_log] Imported names mapping: {}                                                                                        
DEBUG    2025-09-24 15:58:10,026 [/home/mohammed/Work/codeflash/codeflash/lsp/lsp_logger.py:100 in function enhanced_log] Helper function simplify_worktree_paths is not called in optimized code                                           
DEBUG    2025-09-24 15:58:10,029 [/home/mohammed/Work/codeflash/codeflash/lsp/lsp_logger.py:100 in function enhanced_log]   Checked names: {'simplify_worktree_paths', 'codeflash.lsp.helpers.simplify_worktree_paths',                     
         'helpers.simplify_worktree_paths'}                                                                                                                                                                                                 
DEBUG    2025-09-24 15:58:10,031 [/home/mohammed/Work/codeflash/codeflash/lsp/lsp_logger.py:100 in function enhanced_log] Helper function replace_quotes_with_backticks is not called in optimized code                                     
DEBUG    2025-09-24 15:58:10,034 [/home/mohammed/Work/codeflash/codeflash/lsp/lsp_logger.py:100 in function enhanced_log]   Checked names: {'codeflash.lsp.helpers.replace_quotes_with_backticks', 'helpers.replace_quotes_with_backticks', 
         'replace_quotes_with_backticks'}                                                                                                                                                                                                   
DEBUG    2025-09-24 15:58:10,036 [/home/mohammed/Work/codeflash/codeflash/lsp/lsp_logger.py:100 in function enhanced_log] Helper function LspMessage.serialize is not called in optimized code                                              
DEBUG    2025-09-24 15:58:10,040 [/home/mohammed/Work/codeflash/codeflash/lsp/lsp_logger.py:100 in function enhanced_log]   Checked names: {'LspMessage.serialize', 'serialize', 'codeflash.lsp.lsp_message.LspMessage.serialize'}          
INFO     2025-09-24 15:58:10,044 [/home/mohammed/Work/codeflash/codeflash/lsp/lsp_logger.py:100 in function enhanced_log] Reverting 3 unused helper function(s) to original definitions                                                     
DEBUG    2025-09-24 15:58:10,555 [/home/mohammed/Work/codeflash/codeflash/lsp/lsp_logger.py:100 in function enhanced_log] Reverted unused helpers in /home/mohammed/Work/codeflash/codeflash/lsp/helpers.py: simplify_worktree_paths,       
         replace_quotes_with_backticks                                                                                                                                                                                                      
DEBUG    2025-09-24 15:58:11,211 [/home/mohammed/Work/codeflash/codeflash/lsp/lsp_logger.py:100 in function enhanced_log] Reverted unused helpers in /home/mohammed/Work/codeflash/codeflash/lsp/lsp_message.py: LspMessage.serialize       
 | 
    
The issue was that it could accidentally select a different function with the same name (but in another class). In that case, all helpers most-likely would appear unused—since it's another function, causing the optimization to end up empty if it's meant for helpers.
this happened when optimizing
LSPMarkdownMessage.serialize, it did find optimization for helpers but reverted it since it pickedLSPMessage.serializeas the target function instead