Skip to content

fix the tracer#884

Merged
KRRT7 merged 6 commits into
mainfrom
tracing-improvements
Nov 8, 2025
Merged

fix the tracer#884
KRRT7 merged 6 commits into
mainfrom
tracing-improvements

Conversation

@KRRT7
Copy link
Copy Markdown
Contributor

@KRRT7 KRRT7 commented Nov 6, 2025

PR Type

Bug fix, Enhancement


Description

  • Parse args preserving unknown for target

  • Set PYTHONPATH to project root

  • Fix pytest_split return shape

  • Skip license bump on dev/post builds


Diagram Walkthrough

flowchart LR
  ARGS["Args parsing"] -- "parse_known_args; keep unknown" --> TRACER["Tracer main()"]
  TRACER -- "spawn subprocess" --> SUBP["tracing_new_process.py"]
  ENV["Project root in PYTHONPATH"] -- "env=..." --> SUBP
  SPLIT["pytest_split()"] -- "ensure list of lists" --> PARALLEL["Parallel test splits"]
  VERSION["update_license_version"] -- "skip dev/post versions" --> LICENSE["License version unchanged"]
Loading

File Walkthrough

Relevant files
Enhancement
tracer.py
Robust arg handling and PYTHONPATH for tracer subprocesses

codeflash/tracer.py

  • Use parse_known_args; preserve unknowns in sys.argv.
  • Respect --module via parsed temp args.
  • Inject project root into PYTHONPATH for subprocesses.
  • Pass env to Popen/run for correct imports.
+23/-2   
update_license_version.py
Skip license version update for non-final builds                 

codeflash/update_license_version.py

  • Skip updates for dev, local (+), and post releases.
  • Guard to avoid unintended license version bumps.
+3/-0     
Bug fix
pytest_parallelization.py
Fix pytest_split return structure for small sets                 

codeflash/tracing/pytest_parallelization.py

  • Ensure return is list of lists when <4 files.
  • Prevent shape mismatch in downstream consumers.
+1/-1     

KRRT7 added 2 commits November 6, 2025 15:35
When there were fewer than 4 test files, the pytest_split() function returned a flat list of strings instead of a list of lists
@github-actions github-actions Bot changed the title fix the tracer fix the tracer Nov 6, 2025
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Nov 6, 2025

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

Arg Parsing Behavior

Rewriting sys.argv with only unknown args after parse_known_args may drop flags the subprocess expects (e.g., the parsed --module or other known options). Validate that downstream logic and help/usage still behave correctly and that required args are not lost.

temp_parsed, unknown_args = parser.parse_known_args()
parsed_args.module = temp_parsed.module
sys.argv[:] = unknown_args
Env Composition

PYTHONPATH prefixing is duplicated and string-based; confirm no duplicates or path length issues and that project_root is correct for both parallel and single-process paths. Consider normalizing to avoid repeated prefixes on nested invocations.

env = os.environ.copy()
pythonpath = env.get("PYTHONPATH", "")
project_root_str = str(project_root)
if pythonpath:
    env["PYTHONPATH"] = f"{project_root_str}{os.pathsep}{pythonpath}"
else:
    env["PYTHONPATH"] = project_root_str
Return Shape Consistency

Changing the zero-split case to return [test_files] aligns shapes, but verify all callers handle an empty test_files list and that num_splits reduction logic still yields expected distribution when len<4.

return [test_files], test_paths

@KRRT7 KRRT7 requested a review from misrasaurabh1 November 6, 2025 23:58
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Nov 6, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Preserve argv[0] when rewriting argv

Preserve the original sys.argv[0] (script path) when replacing argv, otherwise
downstream command construction may break. Insert the original first argument back
to sys.argv when reassigning.

codeflash/tracer.py [69-71]

 temp_parsed, unknown_args = parser.parse_known_args()
 parsed_args.module = temp_parsed.module
-sys.argv[:] = unknown_args
+# Preserve original argv[0] (executable/script) while passing unknown args through
+original_argv0 = sys.argv[0] if sys.argv else ""
+sys.argv[:] = [original_argv0, *unknown_args] if original_argv0 else unknown_args
Suggestion importance[1-10]: 7

__

Why: Correctly identifies that replacing sys.argv with only unknown args drops argv[0], which can break downstream command construction; the proposed fix is small, accurate, and low-risk.

Medium
Handle empty test list edge-case

Ensure the function consistently returns a tuple where the first element is a list
of lists. When test_files is empty, return an empty list of splits instead of [[]]
to avoid running an empty split.

codeflash/tracing/pytest_parallelization.py [63-65]

 max_possible_splits = len(test_files) // 4
 if max_possible_splits == 0:
-    return [test_files], test_paths
+    return ([], test_paths) if not test_files else ([test_files], test_paths)
Suggestion importance[1-10]: 5

__

Why: This is a reasonable edge-case improvement to return no splits when there are no tests, aligning return type consistency; impact is modest and context-dependent.

Low
General
Prevent PYTHONPATH duplication

Avoid duplicating project_root in PYTHONPATH when it’s already present, which can
lead to very long env vars over multiple runs. Check containment before prepending.

codeflash/tracer.py [133-139]

 env = os.environ.copy()
 pythonpath = env.get("PYTHONPATH", "")
 project_root_str = str(project_root)
-if pythonpath:
-    env["PYTHONPATH"] = f"{project_root_str}{os.pathsep}{pythonpath}"
+path_elems = pythonpath.split(os.pathsep) if pythonpath else []
+if project_root_str not in path_elems:
+    env["PYTHONPATH"] = os.pathsep.join([project_root_str] + path_elems) if path_elems else project_root_str
 else:
-    env["PYTHONPATH"] = project_root_str
+    env["PYTHONPATH"] = pythonpath
Suggestion importance[1-10]: 6

__

Why: Avoiding repeated project_root entries in PYTHONPATH is a practical enhancement that prevents env var bloat across runs; the check is straightforward and accurate.

Low

@KRRT7
Copy link
Copy Markdown
Contributor Author

KRRT7 commented Nov 7, 2025

previously pytest_split was just a string, when a list of list was expected, it ended up splitting that string into a list of individual strings causing the parser to fail

@KRRT7 KRRT7 enabled auto-merge (squash) November 7, 2025 20:32
@KRRT7 KRRT7 merged commit 6696f07 into main Nov 8, 2025
20 of 21 checks passed
@KRRT7 KRRT7 deleted the tracing-improvements branch November 8, 2025 05:14
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.

2 participants