Skip to content

Commit 0f9c06b

Browse files
authored
Merge pull request #1457 from codeflash-ai/fix-coverage-qualified-name
Fix coverage function identification using qualified names
2 parents 9de2e59 + 0567a09 commit 0f9c06b

7 files changed

Lines changed: 266 additions & 19 deletions

File tree

codeflash/code_utils/concolic_utils.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,12 +105,12 @@ def clean_concolic_tests(test_suite_code: str) -> str:
105105
can_parse = False
106106
tree = None
107107

108-
if not can_parse:
108+
if not can_parse or tree is None:
109109
return AssertCleanup().transform_asserts(test_suite_code)
110110

111111
for node in ast.walk(tree):
112112
if isinstance(node, ast.FunctionDef) and node.name.startswith("test_"):
113-
new_body = []
113+
new_body: list[ast.stmt] = []
114114
for stmt in node.body:
115115
if isinstance(stmt, ast.Assert):
116116
if isinstance(stmt.test, ast.Compare) and isinstance(stmt.test.left, ast.Call):

codeflash/code_utils/coverage_utils.py

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,28 @@
1313
def extract_dependent_function(main_function: str, code_context: CodeOptimizationContext) -> str | Literal[False]:
1414
"""Extract the single dependent function from the code context excluding the main function."""
1515
dependent_functions = set()
16+
17+
# Compare using bare name since AST extracts bare function names
18+
bare_main = main_function.rsplit(".", 1)[-1] if "." in main_function else main_function
19+
1620
for code_string in code_context.testgen_context.code_strings:
17-
ast_tree = ast.parse(code_string.code)
18-
dependent_functions.update(
19-
{node.name for node in ast_tree.body if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef))}
20-
)
21+
# Quick heuristic: skip parsing entirely if there is no 'def' token,
22+
# since no function definitions can be present without it.
23+
if "def" not in code_string.code:
24+
continue
2125

22-
if main_function in dependent_functions:
23-
dependent_functions.discard(main_function)
26+
ast_tree = ast.parse(code_string.code)
27+
# Add function names directly, skipping the bare main name.
28+
for node in ast_tree.body:
29+
if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)):
30+
name = node.name
31+
if name == bare_main:
32+
continue
33+
dependent_functions.add(name)
34+
# If more than one dependent function (other than the main) is found,
35+
# we can return False early since the final result cannot be a single name.
36+
if len(dependent_functions) > 1:
37+
return False
2438

2539
if not dependent_functions:
2640
return False
@@ -32,6 +46,9 @@ def extract_dependent_function(main_function: str, code_context: CodeOptimizatio
3246

3347

3448
def build_fully_qualified_name(function_name: str, code_context: CodeOptimizationContext) -> str:
49+
# If the name is already qualified (contains a dot), return as-is
50+
if "." in function_name:
51+
return function_name
3552
full_name = function_name
3653
for obj_name, parents in code_context.preexisting_objects:
3754
if obj_name == function_name:

codeflash/github/PrComment.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,10 @@ class PrComment:
2626

2727
def to_json(self) -> dict[str, Union[str, int, dict[str, dict[str, int]], list[BenchmarkDetail], None]]:
2828
report_table: dict[str, dict[str, int]] = {}
29-
for test_type, result in self.winning_behavior_test_results.get_test_pass_fail_report_by_type().items():
29+
for test_type, counts in self.winning_behavior_test_results.get_test_pass_fail_report_by_type().items():
3030
name = test_type.to_name()
3131
if name:
32-
report_table[name] = result
32+
report_table[name] = counts
3333

3434
result: dict[str, Union[str, int, dict[str, dict[str, int]], list[BenchmarkDetail], None]] = {
3535
"optimization_explanation": self.optimization_explanation,

codeflash/optimization/function_optimizer.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2788,7 +2788,7 @@ def run_and_parse_tests(
27882788
test_config=self.test_cfg,
27892789
optimization_iteration=optimization_iteration,
27902790
run_result=run_result,
2791-
function_name=self.function_to_optimize.function_name,
2791+
function_name=self.function_to_optimize.qualified_name,
27922792
source_file=self.function_to_optimize.file_path,
27932793
code_context=code_context,
27942794
coverage_database_file=coverage_database_file,

codeflash/tracing/pytest_parallelization.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ def pytest_split(
3333

3434
except ImportError:
3535
return None, None
36-
test_files = set()
36+
test_files_set: set[str] = set()
3737

3838
# Find all test_*.py files recursively in the directory
3939
for test_path in test_paths:
@@ -42,20 +42,20 @@ def pytest_split(
4242
return None, None
4343
if _test_path.is_dir():
4444
# Find all test files matching the pattern test_*.py
45-
test_files.update(map(str, _test_path.rglob("test_*.py")))
46-
test_files.update(map(str, _test_path.rglob("*_test.py")))
45+
test_files_set.update(map(str, _test_path.rglob("test_*.py")))
46+
test_files_set.update(map(str, _test_path.rglob("*_test.py")))
4747
elif _test_path.is_file():
48-
test_files.add(str(_test_path))
48+
test_files_set.add(str(_test_path))
4949

50-
if not test_files:
50+
if not test_files_set:
5151
return [[]], None
5252

5353
# Determine number of splits
5454
if num_splits is None:
5555
num_splits = os.cpu_count() or 4
5656

5757
# randomize to increase chances of all splits being balanced
58-
test_files = list(test_files)
58+
test_files = list(test_files_set)
5959
shuffle(test_files)
6060

6161
# Apply limit if specified
@@ -75,7 +75,7 @@ def pytest_split(
7575
chunk_size = ceil(total_files / num_splits)
7676

7777
# Initialize result groups
78-
result_groups = [[] for _ in range(num_splits)]
78+
result_groups: list[list[str]] = [[] for _ in range(num_splits)]
7979

8080
# Distribute files across groups
8181
for i, test_file in enumerate(test_files):

codeflash/verification/coverage_utils.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,9 @@ def grab_dependent_function_from_coverage_data(
351351
for file in files:
352352
functions = files[file]["functions"]
353353
for function in functions:
354-
if dependent_function_name in function:
354+
if function == dependent_function_name or (
355+
"." in dependent_function_name and function.endswith(f".{dependent_function_name}")
356+
):
355357
return FunctionCoverage(
356358
name=dependent_function_name,
357359
coverage=functions[function]["summary"]["percent_covered"],
Lines changed: 228 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,228 @@
1+
from __future__ import annotations
2+
3+
from typing import Any
4+
5+
from codeflash.code_utils.coverage_utils import build_fully_qualified_name, extract_dependent_function
6+
from codeflash.models.function_types import FunctionParent
7+
from codeflash.models.models import CodeOptimizationContext, CodeString, CodeStringsMarkdown
8+
from codeflash.verification.coverage_utils import CoverageUtils
9+
10+
11+
def _make_code_context(
12+
preexisting_objects: set[tuple[str, tuple[FunctionParent, ...]]],
13+
testgen_code_strings: list[CodeString] | None = None,
14+
) -> CodeOptimizationContext:
15+
"""Helper to create a minimal CodeOptimizationContext for testing."""
16+
return CodeOptimizationContext(
17+
testgen_context=CodeStringsMarkdown(code_strings=testgen_code_strings or []),
18+
read_writable_code=CodeStringsMarkdown(),
19+
helper_functions=[],
20+
preexisting_objects=preexisting_objects,
21+
)
22+
23+
24+
class TestBuildFullyQualifiedName:
25+
def test_bare_name_with_class_parent(self) -> None:
26+
ctx = _make_code_context({("__init__", (FunctionParent(name="HttpInterface", type="ClassDef"),))})
27+
assert build_fully_qualified_name("__init__", ctx) == "HttpInterface.__init__"
28+
29+
def test_bare_name_no_parent(self) -> None:
30+
ctx = _make_code_context({("helper_func", ())})
31+
assert build_fully_qualified_name("helper_func", ctx) == "helper_func"
32+
33+
def test_already_qualified_name_returned_as_is(self) -> None:
34+
"""If name already contains a dot, skip preexisting_objects lookup."""
35+
ctx = _make_code_context({("__init__", (FunctionParent(name="WrongClass", type="ClassDef"),))})
36+
result = build_fully_qualified_name("HttpInterface.__init__", ctx)
37+
assert result == "HttpInterface.__init__"
38+
39+
def test_bare_name_picks_first_match_from_set(self) -> None:
40+
"""With multiple __init__ entries, bare name picks an arbitrary one."""
41+
ctx = _make_code_context(
42+
{
43+
("__init__", (FunctionParent(name="ClassA", type="ClassDef"),)),
44+
("__init__", (FunctionParent(name="ClassB", type="ClassDef"),)),
45+
}
46+
)
47+
result = build_fully_qualified_name("__init__", ctx)
48+
assert result in {"ClassA.__init__", "ClassB.__init__"}
49+
50+
def test_qualified_name_avoids_ambiguity(self) -> None:
51+
"""Qualified name bypasses preexisting_objects entirely, avoiding ambiguity."""
52+
ctx = _make_code_context(
53+
{
54+
("__init__", (FunctionParent(name="ClassA", type="ClassDef"),)),
55+
("__init__", (FunctionParent(name="ClassB", type="ClassDef"),)),
56+
}
57+
)
58+
assert build_fully_qualified_name("ClassB.__init__", ctx) == "ClassB.__init__"
59+
60+
def test_bare_name_not_in_preexisting_objects(self) -> None:
61+
ctx = _make_code_context(set())
62+
assert build_fully_qualified_name("some_func", ctx) == "some_func"
63+
64+
def test_nested_class_parent(self) -> None:
65+
"""Bare name under nested class parents gets fully qualified."""
66+
ctx = _make_code_context(
67+
{("method", (FunctionParent(name="Outer", type="ClassDef"), FunctionParent(name="Inner", type="ClassDef")))}
68+
)
69+
assert build_fully_qualified_name("method", ctx) == "Inner.Outer.method"
70+
71+
def test_non_classdef_parent_ignored(self) -> None:
72+
"""Only ClassDef parents are prepended to the name."""
73+
ctx = _make_code_context({("helper", (FunctionParent(name="wrapper", type="FunctionDef"),))})
74+
assert build_fully_qualified_name("helper", ctx) == "helper"
75+
76+
77+
class TestExtractDependentFunction:
78+
def test_single_dependent_function(self) -> None:
79+
ctx = _make_code_context(
80+
preexisting_objects={("helper", ())},
81+
testgen_code_strings=[CodeString(code="def main_func(): pass\ndef helper(): pass")],
82+
)
83+
result = extract_dependent_function("main_func", ctx)
84+
assert result == "helper"
85+
86+
def test_qualified_main_function_discards_bare_match(self) -> None:
87+
"""Qualified main_function should still discard the matching bare name."""
88+
ctx = _make_code_context(
89+
preexisting_objects={("helper", ())},
90+
testgen_code_strings=[CodeString(code="def __init__(): pass\ndef helper(): pass")],
91+
)
92+
result = extract_dependent_function("HttpInterface.__init__", ctx)
93+
assert result == "helper"
94+
95+
def test_bare_main_function_discards_match(self) -> None:
96+
"""Bare main_function should still work for discarding."""
97+
ctx = _make_code_context(
98+
preexisting_objects={("helper", ())},
99+
testgen_code_strings=[CodeString(code="def main_func(): pass\ndef helper(): pass")],
100+
)
101+
result = extract_dependent_function("main_func", ctx)
102+
assert result == "helper"
103+
104+
def test_no_dependent_functions(self) -> None:
105+
ctx = _make_code_context(preexisting_objects=set(), testgen_code_strings=[CodeString(code="x = 1\n")])
106+
result = extract_dependent_function("main_func", ctx)
107+
assert result is False
108+
109+
def test_multiple_dependent_functions_returns_false(self) -> None:
110+
ctx = _make_code_context(
111+
preexisting_objects=set(),
112+
testgen_code_strings=[CodeString(code="def helper_a(): pass\ndef helper_b(): pass")],
113+
)
114+
result = extract_dependent_function("main_func", ctx)
115+
assert result is False
116+
117+
def test_dependent_function_gets_qualified(self) -> None:
118+
"""The dependent function returned should be qualified via build_fully_qualified_name."""
119+
ctx = _make_code_context(
120+
preexisting_objects={("helper", (FunctionParent(name="MyClass", type="ClassDef"),))},
121+
testgen_code_strings=[CodeString(code="def main_func(): pass\ndef helper(): pass")],
122+
)
123+
result = extract_dependent_function("main_func", ctx)
124+
assert result == "MyClass.helper"
125+
126+
def test_only_main_in_code_returns_false(self) -> None:
127+
"""When code only contains the main function, no dependent function exists."""
128+
ctx = _make_code_context(
129+
preexisting_objects=set(), testgen_code_strings=[CodeString(code="def __init__(): pass")]
130+
)
131+
result = extract_dependent_function("HttpInterface.__init__", ctx)
132+
assert result is False
133+
134+
def test_async_functions_extracted(self) -> None:
135+
"""Async function definitions are also extracted as dependent functions."""
136+
ctx = _make_code_context(
137+
preexisting_objects={("async_helper", ())},
138+
testgen_code_strings=[CodeString(code="def main(): pass\nasync def async_helper(): pass")],
139+
)
140+
result = extract_dependent_function("main", ctx)
141+
assert result == "async_helper"
142+
143+
144+
class TestGrabDependentFunctionFromCoverageData:
145+
def _make_func_data(self, coverage_pct: float = 80.0) -> dict[str, Any]:
146+
return {
147+
"summary": {"percent_covered": coverage_pct},
148+
"executed_lines": [1, 2, 3],
149+
"missing_lines": [4],
150+
"executed_branches": [[1, 0]],
151+
"missing_branches": [[2, 1]],
152+
}
153+
154+
def test_exact_match_in_coverage_data(self) -> None:
155+
coverage_data = {"HttpInterface.__init__": self._make_func_data(90.0)}
156+
result = CoverageUtils.grab_dependent_function_from_coverage_data("HttpInterface.__init__", coverage_data, {})
157+
assert result.name == "HttpInterface.__init__"
158+
assert result.coverage == 90.0
159+
160+
def test_fallback_exact_match_in_original_data(self) -> None:
161+
original_cov_data = {
162+
"files": {"http_api.py": {"functions": {"HttpInterface.__init__": self._make_func_data(75.0)}}}
163+
}
164+
result = CoverageUtils.grab_dependent_function_from_coverage_data(
165+
"HttpInterface.__init__", {}, original_cov_data
166+
)
167+
assert result.name == "HttpInterface.__init__"
168+
assert result.coverage == 75.0
169+
170+
def test_fallback_suffix_match_in_original_data(self) -> None:
171+
"""Qualified dependent name matches via suffix in original coverage data."""
172+
original_cov_data = {
173+
"files": {"http_api.py": {"functions": {"module.HttpInterface.__init__": self._make_func_data(60.0)}}}
174+
}
175+
result = CoverageUtils.grab_dependent_function_from_coverage_data(
176+
"HttpInterface.__init__", {}, original_cov_data
177+
)
178+
assert result.name == "HttpInterface.__init__"
179+
assert result.coverage == 60.0
180+
181+
def test_no_false_substring_match_bare_init(self) -> None:
182+
"""Bare __init__ should NOT match PathAwareCORSMiddleware.__init__ via substring."""
183+
original_cov_data = {
184+
"files": {"cors.py": {"functions": {"PathAwareCORSMiddleware.__init__": self._make_func_data(50.0)}}}
185+
}
186+
result = CoverageUtils.grab_dependent_function_from_coverage_data("__init__", {}, original_cov_data)
187+
assert result.coverage == 0
188+
189+
def test_no_false_substring_match_different_class(self) -> None:
190+
"""Qualified name for one class should not match another class's method."""
191+
original_cov_data = {
192+
"files": {
193+
"api.py": {
194+
"functions": {
195+
"PathAwareCORSMiddleware.__init__": self._make_func_data(50.0),
196+
"HttpInterface.__init__": self._make_func_data(85.0),
197+
}
198+
}
199+
}
200+
}
201+
result = CoverageUtils.grab_dependent_function_from_coverage_data(
202+
"HttpInterface.__init__", {}, original_cov_data
203+
)
204+
assert result.name == "HttpInterface.__init__"
205+
assert result.coverage == 85.0
206+
207+
def test_no_match_returns_zero_coverage(self) -> None:
208+
result = CoverageUtils.grab_dependent_function_from_coverage_data("nonexistent_func", {}, {"files": {}})
209+
assert result.coverage == 0
210+
assert result.executed_lines == []
211+
212+
def test_qualified_suffix_no_match_for_partial_name(self) -> None:
213+
"""Ensure suffix match requires a dot boundary, not just string suffix."""
214+
original_cov_data = {
215+
"files": {"api.py": {"functions": {"XHttpInterface.__init__": self._make_func_data(40.0)}}}
216+
}
217+
# "HttpInterface.__init__" should NOT match "XHttpInterface.__init__" via suffix
218+
result = CoverageUtils.grab_dependent_function_from_coverage_data(
219+
"HttpInterface.__init__", {}, original_cov_data
220+
)
221+
assert result.coverage == 0
222+
223+
def test_bare_name_exact_match_in_fallback(self) -> None:
224+
"""Bare function name should still work with exact match in fallback."""
225+
original_cov_data = {"files": {"utils.py": {"functions": {"helper_func": self._make_func_data(95.0)}}}}
226+
result = CoverageUtils.grab_dependent_function_from_coverage_data("helper_func", {}, original_cov_data)
227+
assert result.name == "helper_func"
228+
assert result.coverage == 95.0

0 commit comments

Comments
 (0)