don't import future as standalone

[KRRT7]

PR Type

Bug fix, Tests

---

Description

• Skip invalid and future imports

• Handle None aliases safely

• Reduce noisy warnings to debug

• Add tests for alias edge cases

---

Diagram Walkthrough

flowchart LR
  Extractor["code_extractor.add_needed_imports_from_module"] -- "skip __future__ / None aliases" --> SaferImports["safer import resolution"]
  SaferImports -- "adds/removes imports" --> Transformed["transformed module"]
  Parser["parse_test_output"] -- "less noisy logs + None checks" --> StableParse["more robust XML parsing"]
  Tests["new tests"] -- "alias and usage scenarios" --> Extractor

Loading

File Walkthrough

	Relevant files
Bug fix	code_extractor.py Safer import insertion with alias and future handling        codeflash/code_utils/code_extractor.py Skip importing __future__ as module. Track and skip already-aliased objects. Guard against empty aliases and alias pairs. Use visitor instance before transform. +20/-1    parse_test_output.py Robust XML parsing with quieter logging                                    codeflash/verification/parse_test_output.py Downgrade warnings to debug logs. Guard when testcase.name is None. Safer loop index parsing for pytest. +7/-4
Tests	test_code_extractor_none_aliases_exact.py Tests for import alias edge cases                                                tests/test_code_extractor_none_aliases_exact.py Add tests for None aliases and alias pairs. Validate no imports added when unused. Cover alias usage and partial imports. Include litellm-style import scenarios. +331/-0

---

[github-actions]

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 Logic Coverage Skipping 'future' imports is correct, but consider whether to also skip adding removal operations for them to avoid unintended cleanup if they exist in destination; verify RemoveImportsVisitor behavior when 'future' is present. # Skip __future__ imports as they cannot be imported directly # __future__ imports should only be imported with specific objects i.e from __future__ import annotations if mod == "__future__": continue if mod not in dotted_import_collector.imports: AddImportsVisitor.add_needed_import(dst_context, mod) RemoveImportsVisitor.remove_unused_import(dst_context, mod) aliased_objects = set() Aliases Handling The 'aliased_objects' set prevents duplicate imports, but it relies on f"{mod}.{name}" matching. Confirm that it covers both 'from X import Y as Z' and potential nested objects (e.g., submodules) and that it doesn’t suppress needed imports when alias names overlap with real objects. aliased_objects = set() for mod, alias_pairs in gatherer.alias_mapping.items(): for alias_pair in alias_pairs: if alias_pair[0] and alias_pair[1]: # Both name and alias exist aliased_objects.add(f"{mod}.{alias_pair[0]}") for mod, obj_seq in gatherer.object_mapping.items(): for obj in obj_seq: if ( f"{mod}.{obj}" in helper_functions_fqn or dst_context.full_module_name == mod # avoid circular deps ): continue # Skip adding imports for helper functions already in the context if f"{mod}.{obj}" in aliased_objects: continue Robustness Using testcase.name in loop_index parsing now checks for None, which is good; ensure parsing "[ " with space matches actual pytest param formatting and won’t raise on unexpected patterns. if test_config.test_framework == "pytest": loop_index = int(testcase.name.split("[ ")[-1][:-2]) if testcase.name and "[" in testcase.name else 1 if len(testcase.result) > 1: logger.debug(f"!!!!!Multiple results for {testcase.name or '<None>'} in {test_xml_file_path}!!!") if len(testcase.result) == 1: message = testcase.result[0].message.lower() if "failed: timeout >" in message: timed_out = True else:

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Guard None before lower() testcase.result[0].message may be None; calling .lower() will raise an exception. Guard against None by defaulting to an empty string before lowering. This avoids crashes when parsing flaky junit outputs. codeflash/verification/parse_test_output.py [283-286] if len(testcase.result) == 1: - message = testcase.result[0].message.lower() + raw_msg = testcase.result[0].message + message = (raw_msg or "").lower() if "failed: timeout >" in message: timed_out = True Suggestion importance[1-10]: 7 __ Why: This adds a safe guard for a potential None message before .lower(), preventing crashes on flaky XML; it's accurate and modestly improves robustness.	Medium
	Safely lower possibly None message Similar to the pytest branch, .message can be None. Ensure safe handling by coalescing to an empty string before .lower(). This prevents exceptions on junit results lacking a message. codeflash/verification/parse_test_output.py [290-293] if len(testcase.result) == 1: - message = testcase.result[0].message.lower() + raw_msg = testcase.result[0].message + message = (raw_msg or "").lower() if "timed out" in message: timed_out = True Suggestion importance[1-10]: 7 __ Why: Mirrors the safety improvement in the unittest branch; accurate and prevents errors if .message is None, with moderate impact.	Medium
General	Avoid fragile duplicate-check logic The dotted_import_collector.imports likely stores fully-qualified import entries, so checking mod directly may never match and cause duplicate imports. Compare against a normalized set of module-only imports or rely on the visitor's internal de-duplication logic consistently. This prevents redundant import statements being added. codeflash/code_utils/code_extractor.py [530-538] for mod in gatherer.module_imports: # Skip __future__ imports as they cannot be imported directly # __future__ imports should only be imported with specific objects i.e from __future__ import annotations if mod == "__future__": continue - if mod not in dotted_import_collector.imports: - AddImportsVisitor.add_needed_import(dst_context, mod) + # Always request add; the visitor will avoid duplicates if already present + AddImportsVisitor.add_needed_import(dst_context, mod) RemoveImportsVisitor.remove_unused_import(dst_context, mod) Suggestion importance[1-10]: 4 __ Why: The suggestion is plausible but speculative; it assumes dotted_import_collector.imports structure without evidence. Removing the check may cause unnecessary operations; impact is minor and correctness uncertain.	Low

[CLAassistant]

All committers have signed the CLA.

[KRRT7]

CF-748