Skip to content

Conversation

@misrasaurabh1
Copy link
Contributor

@misrasaurabh1 misrasaurabh1 commented Oct 21, 2025

User description

the function to optimize is not an unused helper


PR Type

Bug fix, Tests


Description

  • Prevent entrypoint misclassification as unused

  • Initialize called set with entrypoint name

  • Add test for recursive helper detection


Diagram Walkthrough

flowchart LR
  detector["Unused helper detector"] -- "init called set with entrypoint" --> analysis["AST call analysis"]
  tests["New recursive test"] -- "validates no false positives" --> detector
Loading

File Walkthrough

Relevant files
Bug fix
unused_definition_remover.py
Seed called set with entrypoint to avoid false positives 

codeflash/context/unused_definition_remover.py

  • Initialize called_function_names with entrypoint name
  • Avoid falsely marking entrypoint as unused
+1/-1     
Tests
test_unused_helper_revert.py
Add regression test for recursive helper detection             

tests/test_unused_helper_revert.py

  • Add test for recursive function gcd_recursive
  • Ensure recursion not flagged as unused
  • Create minimal project context and assert behavior
+67/-0   

Signed-off-by: Saurabh Misra <[email protected]>
@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

Behavior Change

Initializing called set with the entrypoint name alters semantics: the entrypoint will never be considered unused even if analysis previously allowed that. Confirm this aligns with intended logic across all call-graph analyses and does not hide genuine unused-entrypoint cases in batch modes.

called_function_names = {function_to_optimize.function_name}
for node in ast.walk(entrypoint_function_ast):
    if isinstance(node, ast.Call):
        if isinstance(node.func, ast.Name):
            # Regular function call: function_name()
Test Fixture Isolation

The test writes a triple-fenced markdown code block (```python:main.py ...) into optimized_code; ensure the downstream parser is expected to handle the language-and-filename fence variant and that no leakage from temp paths affects other tests; consider using tmp_path fixture for better isolation.

        optimized_code = """
```python:main.py
def gcd_recursive(a: int, b: int) -> int:
    \"\"\"Calculate greatest common divisor using Euclidean algorithm with recursion.\"\"\"
    if b == 0:
        return a
    return gcd_recursive(b, a % b)

"""


</details>

</td></tr>
</table>

@github-actions
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Avoid seeding with entrypoint name

Initializing called_function_names with the entrypoint name will falsely mark the
entrypoint as "called" even if it's unused, skewing helper detection. Revert to
starting with an empty set and only add names discovered during AST walk.

codeflash/context/unused_definition_remover.py [682-687]

-called_function_names = {function_to_optimize.function_name}
+called_function_names = set()
 for node in ast.walk(entrypoint_function_ast):
     if isinstance(node, ast.Call):
         if isinstance(node.func, ast.Name):
             # Regular function call: function_name()
Suggestion importance[1-10]: 8

__

Why: The PR seeds called_function_names with function_to_optimize.function_name, which can incorrectly treat the entrypoint as "called" and mask unused detection; reverting to an empty set aligns with accurate call discovery via AST walk and improves correctness. The existing_code matches the new hunk, and the improved_code correctly reflects the proposed change.

Medium

@codeflash-ai
Copy link
Contributor

codeflash-ai bot commented Oct 21, 2025

⚡️ Codeflash found optimizations for this PR

📄 36% (0.36x) speedup for detect_unused_helper_functions in codeflash/context/unused_definition_remover.py

⏱️ Runtime : 37.2 milliseconds 27.5 milliseconds (best of 5 runs)

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

If you approve, it will be merged into this PR (branch fix-detect-unused-helper-functions).

@misrasaurabh1
Copy link
Contributor Author

@mohammedahmed18 im merging this but the tests are not good, i.e. they dont catch errors

@misrasaurabh1 misrasaurabh1 merged commit 76a923f into main Oct 21, 2025
23 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.

1 participant