Skip to content

Conversation

@aseembits93
Copy link
Contributor

@aseembits93 aseembits93 commented Oct 31, 2025

User description

just match class name when method is not imported
for example
from code_to_optimize.topological_sort import Graph # topologicalSort not imported


PR Type

Bug fix, Tests


Description

  • Match class-only imports to methods

  • Avoid missing class.method targets

  • Update E2E to assert test count


Diagram Walkthrough

flowchart LR
  A["ImportFrom analyzer"] -- "fallback: match class to class.method" --> B["Target function detection"]
  B -- "found_qualified_name set" --> C["Unit test discovery"]
  C -- "validated by" --> D["End-to-end test updated (expected_unit_tests=1)"]
Loading

File Walkthrough

Relevant files
Bug fix
discover_unit_tests.py
Fallback: match class import to class.method                         

codeflash/discovery/discover_unit_tests.py

  • Add class-to-method fallback matching.
  • Set found flags when class name matches.
  • Handle dotted target functions via split.
  • Add TODO about cross-file class name collisions.
+11/-0   
Tests
end_to_end_test_topological_sort_worktree.py
E2E: assert single discovered unit test                                   

tests/scripts/end_to_end_test_topological_sort_worktree.py

  • Assert expected_unit_tests equals 1.
  • Align E2E with new discovery behavior.
+1/-0     

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Codeflash Bot seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@github-actions
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

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

Overmatching Risk

Class-name matching may incorrectly associate methods from different modules/classes sharing the same class name, potentially causing false positives in test discovery.

# Check if any target function is a method of the imported class/module
# e.g., importing Graph and looking for Graph.topologicalSort
# TODO will pick up all tests which have the same class name (could be coming from a different file)
for target_func in fnames:
    if "." in target_func:
        class_name, method_name = target_func.split(".", 1)
        if aname == class_name:
            self.found_any_target_function = True
            self.found_qualified_name = target_func
            return
Ambiguous Split

Splitting on the first dot assumes a single-level class.method; deeper qualifications (e.g., pkg.Class.method or Outer.Inner.method) may not be handled correctly.

if "." in target_func:
    class_name, method_name = target_func.split(".", 1)
    if aname == class_name:
Performance Consideration

Iterating over all function names for each ImportFrom without early filtering could be inefficient on large fnames; consider pre-indexing by class name.

for target_func in fnames:
    if "." in target_func:
        class_name, method_name = target_func.split(".", 1)
        if aname == class_name:
            self.found_any_target_function = True
            self.found_qualified_name = target_func
            return

@github-actions
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Validate dotted name structure

Validate the split to avoid unexpected behavior for malformed names (e.g.,
leading/trailing dots) and protect against empty class or method segments. This
prevents incorrect matches and potential crashes if later logic assumes non-empty
parts.

codeflash/discovery/discover_unit_tests.py [303-305]

 if "." in target_func:
-    class_name, method_name = target_func.split(".", 1)
+    parts = target_func.split(".", 1)
+    if len(parts) == 2 and parts[0] and parts[1]:
+        class_name, method_name = parts
+    else:
+        continue
Suggestion importance[1-10]: 6

__

Why: Adding defensive checks against malformed dotted names is reasonable and low-risk, improving robustness without changing behavior for valid inputs. Impact is moderate as current code likely handles normal cases but could be safer with validation.

Low
General
Reduce class-name collision risk

Guard against false positives by ensuring aname refers to a class import (not a
module alias) and matching only when the import kind aligns. Also restrict matches
to ast.ImportFrom with node.module resolving to the expected module if available to
avoid cross-file class name collisions.

codeflash/discovery/discover_unit_tests.py [302-308]

 for target_func in fnames:
     if "." in target_func:
         class_name, method_name = target_func.split(".", 1)
-        if aname == class_name:
+        # Only match when the imported alias equals the class name and this is a 'from X import Y' form
+        if aname == class_name and isinstance(node, ast.ImportFrom) and node.module:
             self.found_any_target_function = True
             self.found_qualified_name = target_func
             return
Suggestion importance[1-10]: 4

__

Why: The concern about class-name collisions is valid, but the added check isinstance(node, ast.ImportFrom) is redundant inside visit_ImportFrom, and requiring node.module may incorrectly skip valid cases (e.g., relative imports). The improvement is minor and partially misapplies context.

Low

Codeflash Bot and others added 3 commits October 31, 2025 14:01
The optimized code achieves a 38% speedup by eliminating expensive repeated string operations and set iterations within the hot path of `visit_Attribute()`.

**Key optimizations:**

1. **Precomputed lookup structures**: During initialization, the code now builds three efficient lookup structures:
   - `_dot_methods`: Maps method names to sets of possible class names (e.g., "my_method" → {"MyClass", "OtherClass"})
   - `_class_method_to_target`: Maps (class, method) tuples to full target names for O(1) reconstruction
   - These replace the expensive loop that called `target_func.rsplit(".", 1)` on every function name for every attribute node

2. **Eliminated expensive loops**: The original code had nested loops iterating through all `function_names_to_find` for each attribute access. The optimized version uses fast hash table lookups (`self._dot_methods.get(node_attr)`) followed by set membership tests.

3. **Reduced attribute access overhead**: Local variables `node_value` and `node_attr` cache the attribute lookups to avoid repeated property access.

**Performance impact by test case type:**
- **Large alias mappings**: Up to 985% faster (23.4μs → 2.15μs) - most dramatic improvement when many aliases need checking
- **Large instance mappings**: 342% faster (9.35μs → 2.11μs) - significant gains with many instance variables  
- **Class method access**: 24-27% faster - consistent improvement for dotted name resolution
- **Basic cases**: 7-15% faster - modest but consistent gains even for simple scenarios

The optimization is most effective for codebases with many qualified names (e.g., "Class.method" patterns) and particularly shines when the analyzer needs to check large sets of potential matches, which is common in real-world code discovery scenarios.
@codeflash-ai
Copy link
Contributor

codeflash-ai bot commented Oct 31, 2025

⚡️ Codeflash found optimizations for this PR

📄 38% (0.38x) speedup for ImportAnalyzer.visit_Attribute in codeflash/discovery/discover_unit_tests.py

⏱️ Runtime : 114 microseconds 82.6 microseconds (best of 250 runs)

A dependent PR with the suggested changes has been created. Please review:

If you approve, it will be merged into this PR (branch import-analyser-fix).

Static Badge

aseembits93 and others added 2 commits October 31, 2025 15:08
…25-10-31T21.36.53

⚡️ Speed up method `ImportAnalyzer.visit_Attribute` by 38% in PR #868 (`import-analyser-fix`)
@codeflash-ai
Copy link
Contributor

codeflash-ai bot commented Oct 31, 2025

This PR is now faster! 🚀 @misrasaurabh1 accepted my optimizations from:

misrasaurabh1
misrasaurabh1 previously approved these changes Oct 31, 2025
@aseembits93 aseembits93 enabled auto-merge October 31, 2025 23:55
changing from individual tests to test files
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.

4 participants