Skip to content

Conversation

@KRRT7
Copy link
Contributor

@KRRT7 KRRT7 commented Jul 25, 2025

PR Type

Bug fix


Description

  • Increase pytest verbosity with "-vv" flag

  • Print subprocess stdout and stderr

  • Add debug start message in trace benchmarks

  • Enable traverse_up in module resolution


Diagram Walkthrough

flowchart LR
  A["pytest_new_process_trace_benchmarks.py"] -- "added -vv flag" --> B["trace_benchmarks.py"]
  B -- "print start, stdout, stderr" --> C["models/models.py"]
  C -- "pass traverse_up flag" --> D["module_name resolution"]
Loading

File Walkthrough

Relevant files
Tests
pytest_new_process_trace_benchmarks.py
Increase pytest verbosity                                                               

codeflash/benchmarking/pytest_new_process_trace_benchmarks.py

  • Added "-vv" pytest verbosity flag
+1/-0     
Miscellaneous
trace_benchmarks.py
Add debug prints for subprocess                                                   

codeflash/benchmarking/trace_benchmarks.py

  • Inserted debug start print
  • Print subprocess stdout and stderr
+3/-0     
Bug fix
models.py
Fix module path traversal flag                                                     

codeflash/models/models.py

  • Pass traverse_up=True to module resolution
+1/-0     

@github-actions
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

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

Debug prints

The use of raw print statements for start, stdout, and stderr will flood CI logs and isn't configurable. Consider using the logging module with appropriate log levels and only printing on failures.

print("running trace_benchmarks_pytest")
result = subprocess.run(
    [
        SAFE_SYS_EXECUTABLE,
        Path(__file__).parent / "pytest_new_process_trace_benchmarks.py",
        benchmarks_root,
        tests_root,
        trace_file,
    ],
    cwd=project_root,
    check=False,
    capture_output=True,
    text=True,
    env=benchmark_env,
    timeout=timeout,
)
print(result.stdout)
print(result.stderr)
Module resolution change

Enabling traverse_up in module_name_from_file_path may alter import resolution paths globally. Verify this change works in all repository layouts and add tests to cover edge cases.

    project_root,
    traverse_up=True
)

@github-actions
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Handle subprocess timeouts

Wrap the subprocess call in a try/except block to catch TimeoutExpired and log a
clear error message, preventing unhandled exceptions on long runs.

codeflash/benchmarking/trace_benchmarks.py [21-35]

-result = subprocess.run(
-    [
-        SAFE_SYS_EXECUTABLE,
-        Path(__file__).parent / "pytest_new_process_trace_benchmarks.py",
-    ],
-    cwd=project_root,
-    check=False,
-    capture_output=True,
-    text=True,
-    env=benchmark_env,
-    timeout=timeout,
-)
+try:
+    result = subprocess.run(
+        [
+            SAFE_SYS_EXECUTABLE,
+            Path(__file__).parent / "pytest_new_process_trace_benchmarks.py",
+        ],
+        cwd=project_root,
+        check=False,
+        capture_output=True,
+        text=True,
+        env=benchmark_env,
+        timeout=timeout,
+    )
+except subprocess.TimeoutExpired:
+    logger.error(f"Benchmark timed out after {timeout}s")
+    raise
Suggestion importance[1-10]: 7

__

Why: Catching TimeoutExpired and logging an error prevents unhandled exceptions on long runs and improves robustness.

Medium
General
Use structured logging

Replace raw print statements with structured logging so that output can be
controlled by log levels and integrated into the existing logging framework.

codeflash/benchmarking/trace_benchmarks.py [20-37]

-print("running trace_benchmarks_pytest")
-print(result.stdout)
-print(result.stderr)
+logger.info("running trace_benchmarks_pytest")
+logger.debug(result.stdout)
+logger.error(result.stderr)
Suggestion importance[1-10]: 6

__

Why: Replacing print calls with logger calls integrates with the existing logging framework and allows controlling output via log levels.

Low

@github-actions github-actions bot added the workflow-modified This PR modifies GitHub Actions workflows label Jul 25, 2025
@KRRT7 KRRT7 closed this Jul 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Review effort 2/5 workflow-modified This PR modifies GitHub Actions workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant