Skip to content

Conversation

aseembits93
Copy link
Contributor

@aseembits93 aseembits93 commented Oct 15, 2025

User description

WIP


PR Type

Enhancement, Tests


Description

  • Add optimization impact plumbing to API

  • Introduce call analysis AST visitor

  • Add ripgrep-based code search helper

  • Extend models with impact metrics


Diagram Walkthrough

flowchart LR Optimizer["function_optimizer: process_review"] -- "compute + pass" --> AISvc["aiservice.get_optimization_impact"] Optimizer -- "send to CFAPI" --> CFAPI["cfapi.suggest_changes(payload)"] CFAPI -- "payload field" --> Backend["/suggest-pr-changes"] Models["models.ImpactMetrics"] -- "intended usage" --> Optimizer UtilsRG["code_extractor.search_with_ripgrep"] -- "search repo" --> Optimizer Visitor["function_call_visitor: FunctionCallVisitor"] -- "analyze calls" --> Metrics["Impact metrics (future)"] CreatePR["result.create_pr.check_create_pr"] -- "forward optimizationImpact" --> CFAPI 
Loading

File Walkthrough

Relevant files
Documentation
2 files
aiservice.py
Add TODO metrics notes in optimization impact flow             
+7/-0     
example_usage.py
Add example for ripgrep search usage                                         
+28/-0   
Enhancement
7 files
cfapi.py
Plumb optimizationImpact through suggest_changes payload 
+2/-0     
code_extractor.py
Add ripgrep search helper and impact metrics stub               
+86/-3   
compat.py
Expose SAFE_GREP_EXECUTABLE via Compat                                     
+3/-0     
models.py
Define ImpactMetrics dataclass for impact data                     
+8/-0     
function_optimizer.py
Compute optimization impact and attach metrics stub           
+10/-8   
create_pr.py
Forward optimization impact to suggestion API                       
+2/-0     
function_call_visitor.py
Implement AST visitor for call and loop detection               
+317/-0 
Tests
1 files
test_function_call_visitor.py
Add tests and demos for call visitor                                         
+263/-0 

@github-actions github-actions bot changed the title More metrics for determining Optimization Impact More metrics for determining Optimization Impact Oct 15, 2025
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 Security concerns

Command execution:
search_with_ripgrep shells out to 'rg' with a pattern that appears to be passed directly. Although subprocess.run uses a list (shell=False), consider guarding against unbounded patterns and very large outputs. Also, printing command lines and errors may leak paths; use logger with appropriate levels instead of print.

⚡ Recommended focus areas for review

Hardcoded Path

The ripgrep search excludes a user-specific absolute path which will not exist in other environments; this should be configurable or relative to the project root.

cmd = [ "rg", "-n", "--json", pattern, path, "-g", "!/Users/aseemsaxena/Downloads/codeflash_dev/codeflash/code_to_optimize/tests/**", ] print(" ".join(cmd))
Incomplete Return

get_opt_impact_metrics returns 0 but is annotated to return ImpactMetrics, which will cause type/usage errors when consumed.

def get_opt_impact_metrics(file_path: Path, qualified_name: str, project_root: Path, tests_root: Path) -> ImpactMetrics: # grep for function / use rg (respects gitignore) # SAFE_GREP_EXECUTABLE command # ast visitor for occurances and loop occurances # radon lib for complexity metrics print(file_path, qualified_name, project_root, tests_root) # grep windows alternative return 0
Argument Mismatch

get_opt_impact_metrics is called with two arguments but its signature expects four; this will raise at runtime.

data["impact_metrics"] = get_opt_impact_metrics( self.project_root, self.test_cfg.tests_root ) # need module root, tests root only
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix mismatched function call signature

The call to get_opt_impact_metrics does not match its signature, which expects
(file_path, qualified_name, project_root, tests_root). Pass the required arguments
to avoid runtime TypeError. If some values are not available yet, pass reasonable
placeholders or defer the call.

codeflash/optimization/function_optimizer.py [1471-1473]

+# Example placeholders; replace with real target file and qualified name when available +target_file: Path = self.project_root +target_qualified_name: str = "" data["impact_metrics"] = get_opt_impact_metrics( - self.project_root, self.test_cfg.tests_root -) # need module root, tests root only + target_file, target_qualified_name, self.project_root, self.test_cfg.tests_root +)
Suggestion importance[1-10]: 8

__

Why: The call site clearly passes two args while the function signature requires four; this would raise a runtime TypeError. Providing proper arguments prevents a crash and is a high-impact correctness fix.

Medium
Return correct ImpactMetrics type

The function advertises returning ImpactMetrics but returns an int, which will crash
callers expecting the model. Return a properly constructed ImpactMetrics (even with
placeholder values) and remove debug prints to avoid stdout noise.

codeflash/code_utils/code_extractor.py [825-834]

 def get_opt_impact_metrics(file_path: Path, qualified_name: str, project_root: Path, tests_root: Path) -> ImpactMetrics: - # grep for function / use rg (respects gitignore) - # SAFE_GREP_EXECUTABLE command - # ast visitor for occurances and loop occurances - # radon lib for complexity metrics - print(file_path, qualified_name, project_root, tests_root) + # TODO: Implement actual metrics collection + # Placeholder safe defaults to maintain contract + return ImpactMetrics( + complexity_score=0, + occurances=0, + loop_occurances=0, + presence_of_decorators=False, + ) - # grep windows alternative - return 0 -
Suggestion importance[1-10]: 8

__

Why: The function is annotated to return ImpactMetrics but returns an int, breaking the contract; returning a valid ImpactMetrics prevents downstream errors and aligns with the new dataclass added in the PR.

Medium
Remove hardcoded paths and debug prints

Avoid hardcoding an absolute exclude path and remove debug prints to prevent leaking
environment details. Also honor the provided path parameter instead of overriding it
with cwd, and use the project logger for errors. This prevents noisy stdout and
makes the function deterministic across environments.

codeflash/code_utils/code_extractor.py [754-823]

 def search_with_ripgrep(pattern: str, path: str = ".") -> dict[str, list[tuple[int, str]]]: - ... - path = str(Path.cwd()) + """Use ripgrep to search for a pattern in the repository.""" + search_root = str(Path(path).resolve()) cmd = [ "rg", "-n", "--json", pattern, - path, + search_root, "-g", - "!/Users/aseemsaxena/Downloads/codeflash_dev/codeflash/code_to_optimize/tests/**", + "!**/tests/**", ] - print(" ".join(cmd)) - ... + matches_dict: dict[str, list[tuple[int, str]]] = {} try: result = subprocess.run( cmd, capture_output=True, text=True, - check=False, # Don't raise exception on non-zero return + check=False, ) - ... + + if result.returncode not in (0, 1): + logger.debug(f"ripgrep returned {result.returncode}: {result.stderr}") + return {} + + for line in result.stdout.splitlines(): + if not line: + continue + try: + json_obj = json.loads(line) + if json_obj.get("type") != "match": + continue + data = json_obj.get("data", {}) + file_path = data.get("path", {}).get("text", "") + line_number = data.get("line_number") + line_content = data.get("lines", {}).get("text", "").rstrip("\n") + if file_path and isinstance(line_number, int): + matches_dict.setdefault(file_path, []).append((line_number, line_content)) + except json.JSONDecodeError: + continue except FileNotFoundError: - print("Error: ripgrep (rg) is not installed or not in PATH") + logger.warning("ripgrep (rg) is not installed or not in PATH") return {} except Exception as e: - print(f"Unexpected error: {e}") + logger.debug(f"ripgrep invocation failed: {e}") return {} return matches_dict
Suggestion importance[1-10]: 7

__

Why: Correctly identifies multiple issues: overriding the path arg, hardcoded absolute exclude, and noisy prints; proposed code aligns with context and improves portability and logging without changing functionality.

Medium
Copy link
Contributor

codeflash-ai bot commented Oct 17, 2025

⚡️ Codeflash found optimizations for this PR

📄 51% (0.51x) speedup for FunctionCallFinder._extract_source_code in codeflash/code_utils/code_extractor.py

⏱️ Runtime : 1.12 milliseconds 742 microseconds (best of 59 runs)

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

If you approve, it will be merged into this PR (branch opt-impact-aseem-improvement).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

1 participant