Skip to content

Commit 1181f6a

Browse files
committed
fix: use qualified_name for coverage function identification
The coverage system was using bare function_name (e.g., "__init__") instead of qualified_name (e.g., "HttpInterface.__init__"), causing it to match the wrong class's method when multiple classes define the same method name (like __init__). Changes: - function_optimizer.py: pass qualified_name to parse_test_results - build_fully_qualified_name: skip re-qualifying already-qualified names - extract_dependent_function: compare using bare name from qualified input - grab_dependent_function_from_coverage_data: replace substring match with exact or dot-bounded suffix match
1 parent 9de2e59 commit 1181f6a

4 files changed

Lines changed: 237 additions & 4 deletions

File tree

codeflash/code_utils/coverage_utils.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,10 @@ def extract_dependent_function(main_function: str, code_context: CodeOptimizatio
1919
{node.name for node in ast_tree.body if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef))}
2020
)
2121

22-
if main_function in dependent_functions:
23-
dependent_functions.discard(main_function)
22+
# Compare using bare name since AST extracts bare function names
23+
bare_main = main_function.rsplit(".", 1)[-1] if "." in main_function else main_function
24+
if bare_main in dependent_functions:
25+
dependent_functions.discard(bare_main)
2426

2527
if not dependent_functions:
2628
return False
@@ -32,6 +34,9 @@ def extract_dependent_function(main_function: str, code_context: CodeOptimizatio
3234

3335

3436
def build_fully_qualified_name(function_name: str, code_context: CodeOptimizationContext) -> str:
37+
# If the name is already qualified (contains a dot), return as-is
38+
if "." in function_name:
39+
return function_name
3540
full_name = function_name
3641
for obj_name, parents in code_context.preexisting_objects:
3742
if obj_name == function_name:

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

0 commit comments

Comments
 (0)