Skip to content

Commit d2cc561

Browse files
committed
fix 'first-match-wins' bug and test it
Signed-off-by: Venky Ganesh <[email protected]>
1 parent d3d110f commit d2cc561

File tree

2 files changed

+51
-7
lines changed

2 files changed

+51
-7
lines changed

.github/scripts/assign_reviewers.py

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -68,19 +68,31 @@ def load_json(path: str):
6868

6969
def map_modules(changed_files: list[str],
7070
module_paths: dict[str, str]) -> tuple[set[str], list[str]]:
71-
"""Map changed files to modules and return both modules and unmapped files"""
71+
"""Map changed files to modules using MOST SPECIFIC (longest) prefix match"""
7272
modules: set[str] = set()
7373
unmapped_files: list[str] = []
7474

7575
for file in changed_files:
76-
mapped = False
76+
# Find ALL matching prefixes
77+
matches = []
7778
for prefix, module in module_paths.items():
7879
if file.startswith(prefix):
79-
modules.add(module)
80-
mapped = True
81-
break
82-
83-
if not mapped:
80+
matches.append((len(prefix), prefix, module))
81+
82+
if matches:
83+
# Sort by prefix length (descending) to get most specific first
84+
matches.sort(reverse=True)
85+
most_specific_module = matches[0][2]
86+
modules.add(most_specific_module)
87+
88+
# Log if there were multiple matches (for debugging)
89+
if len(matches) > 1:
90+
matches[0][1]
91+
print(f" File '{file}' has overlapping mappings:")
92+
for _, prefix, module in matches:
93+
marker = "→" if module == most_specific_module else " "
94+
print(f" {marker} {prefix} -> {module}")
95+
else:
8496
unmapped_files.append(file)
8597

8698
return modules, unmapped_files

.github/scripts/tests/test_assign_reviewers.py

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,38 @@ def test_all_files_unmapped(self):
386386
self.assertEqual(set(unmapped_files),
387387
{"unmapped/file1.txt", "another/file2.py"})
388388

389+
def test_most_specific_module_mapping(self):
390+
"""Test that files map to the most specific module match"""
391+
# Create overlapping module paths similar to the real config
392+
overlapping_paths = {
393+
"tensorrt_llm/": "LLM API/Workflow",
394+
"tensorrt_llm/_torch/": "Torch Framework",
395+
"tensorrt_llm/_torch/models/": "Torch Models",
396+
"tensorrt_llm/_torch/models/modeling_llama.py":
397+
"Torch Models Llama",
398+
"tests/": "General Tests",
399+
"tests/integration/": "Integration Tests",
400+
"tests/integration/test_lists/": "Test Configuration",
401+
}
402+
403+
# Test individual files mapping to most specific modules
404+
test_cases = [
405+
("tensorrt_llm/api.py", "LLM API/Workflow"),
406+
("tensorrt_llm/_torch/utils.py", "Torch Framework"),
407+
("tensorrt_llm/_torch/models/bert.py", "Torch Models"),
408+
("tensorrt_llm/_torch/models/modeling_llama.py",
409+
"Torch Models Llama"),
410+
("tests/unit_test.py", "General Tests"),
411+
("tests/integration/test_x.py", "Integration Tests"),
412+
("tests/integration/test_lists/config.json", "Test Configuration"),
413+
]
414+
415+
for file, expected_module in test_cases:
416+
modules, _ = assign_reviewers.map_modules([file], overlapping_paths)
417+
self.assertEqual(
418+
modules, {expected_module},
419+
f"File '{file}' should map to '{expected_module}'")
420+
389421
@patch('assign_reviewers.load_json')
390422
@patch('subprocess.run')
391423
def test_module_with_no_owners(self, mock_run, mock_load_json):

0 commit comments

Comments
 (0)