Skip to content

Commit 58ea1cb

Browse files
committed
feat: enforce global dependency budget
1 parent 69e2b1e commit 58ea1cb

2 files changed

Lines changed: 106 additions & 32 deletions

File tree

tools/03_code_analysis/agent_governance_check.py

Lines changed: 73 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -120,42 +120,52 @@ def changed_paths(root: Path, args: argparse.Namespace) -> Tuple[Dict[str, str],
120120
return parse_name_status(output)
121121

122122

123-
def parse_added_lines(diff_text: str) -> List[DiffLine]:
124-
lines: List[DiffLine] = []
125-
path = ""
123+
def parse_changed_lines(diff_text: str) -> Tuple[List[DiffLine], List[DiffLine]]:
124+
added: List[DiffLine] = []
125+
removed: List[DiffLine] = []
126+
old_path = ""
127+
new_path = ""
128+
old_line: Optional[int] = None
126129
new_line: Optional[int] = None
127-
hunk_re = re.compile(r"@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@")
130+
hunk_re = re.compile(r"@@ -(\d+)(?:,\d+)? \+(\d+)(?:,\d+)? @@")
128131
for raw in diff_text.splitlines():
132+
if raw.startswith("--- a/"):
133+
old_path = raw[6:]
134+
continue
129135
if raw.startswith("+++ b/"):
130-
path = raw[6:]
136+
new_path = raw[6:]
131137
continue
132-
if raw.startswith("+++ "):
133-
path = raw[4:]
138+
if raw.startswith("--- ") or raw.startswith("+++ "):
134139
continue
135140
match = hunk_re.match(raw)
136141
if match:
137-
new_line = int(match.group(1))
142+
old_line = int(match.group(1))
143+
new_line = int(match.group(2))
144+
continue
145+
if old_line is None or new_line is None:
138146
continue
139-
if new_line is None:
147+
if raw.startswith("\\"):
140148
continue
141149
if raw.startswith("+") and not raw.startswith("+++"):
142-
lines.append(DiffLine(path, new_line, raw[1:]))
150+
added.append(DiffLine(new_path, new_line, raw[1:]))
143151
new_line += 1
144152
elif raw.startswith("-") and not raw.startswith("---"):
145-
continue
153+
removed.append(DiffLine(old_path, old_line, raw[1:]))
154+
old_line += 1
146155
else:
156+
old_line += 1
147157
new_line += 1
148-
return lines
158+
return added, removed
149159

150160

151-
def added_lines(root: Path, args: argparse.Namespace) -> List[DiffLine]:
161+
def changed_lines(root: Path, args: argparse.Namespace) -> Tuple[List[DiffLine], List[DiffLine]]:
152162
if args.staged:
153163
output = git(["diff", "--cached", "--ignore-cr-at-eol", "-U0"], root).stdout
154164
elif args.base and args.head:
155165
output = git(["diff", "--ignore-cr-at-eol", "-U0", args.base, args.head], root).stdout
156166
else:
157167
output = ""
158-
return parse_added_lines(output)
168+
return parse_changed_lines(output)
159169

160170

161171
def read_changed_file_bytes(root: Path, path: str, args: argparse.Namespace) -> bytes:
@@ -232,22 +242,57 @@ def check_line_endings(
232242
)
233243

234244

235-
def check_global_dependencies(findings: List[Finding], lines: Iterable[DiffLine]) -> None:
236-
pattern = re.compile(r"\b(GlobalV::|GlobalC::|PARAM(?:\.|->|::|\b))")
245+
GLOBAL_DEPENDENCY_RE = re.compile(r"\b(GlobalV::|GlobalC::|PARAM(?:\.|->|::|\b))")
246+
247+
248+
def is_global_dependency_check_path(path: str) -> bool:
249+
if path.startswith("tools/03_code_analysis/"):
250+
return False
251+
return Path(path).suffix.lower() in CODE_EXTENSIONS
252+
253+
254+
def global_dependency_hits(lines: Iterable[DiffLine]) -> List[Tuple[DiffLine, int]]:
255+
hits: List[Tuple[DiffLine, int]] = []
237256
for line in lines:
238-
if line.path.startswith("tools/03_code_analysis/"):
257+
if not is_global_dependency_check_path(line.path):
239258
continue
240-
if Path(line.path).suffix.lower() not in CODE_EXTENSIONS:
241-
continue
242-
if pattern.search(line.content):
243-
add_finding(
259+
count = len(GLOBAL_DEPENDENCY_RE.findall(line.content))
260+
if count:
261+
hits.append((line, count))
262+
return hits
263+
264+
265+
def check_global_dependencies(
266+
findings: List[Finding],
267+
added_lines: Iterable[DiffLine],
268+
removed_lines: Iterable[DiffLine],
269+
) -> None:
270+
added_hits = global_dependency_hits(added_lines)
271+
removed_hits = global_dependency_hits(removed_lines)
272+
added_count = sum(count for _, count in added_hits)
273+
removed_count = sum(count for _, count in removed_hits)
274+
delta = added_count - removed_count
275+
if added_count == 0:
276+
return
277+
278+
severity = BLOCK if delta > 0 else WARN
279+
action = (
280+
"Reduce or explicitly pass dependencies so this PR does not increase global dependency usage."
281+
if delta > 0
282+
else "Confirm this is a migration-neutral move or partial cleanup, and explain the remaining global dependency rationale."
283+
)
284+
for line, count in added_hits:
285+
add_finding(
244286
findings,
245-
"No new cross-layer globals",
246-
WARN,
287+
"Global dependency budget",
288+
severity,
247289
line.path,
248290
line.line,
249-
"Added line introduces GlobalV, GlobalC, or PARAM as a dependency.",
250-
"Prefer explicit parameters or a narrow local interface. Document any required exception in the PR.",
291+
(
292+
f"Added line introduces {count} GlobalV/GlobalC/PARAM reference(s); "
293+
f"PR total added={added_count}, removed={removed_count}, net_delta={delta}."
294+
),
295+
action,
251296
)
252297

253298

@@ -518,7 +563,7 @@ def check_pr_metadata(findings: List[Finding], body: Optional[str]) -> None:
518563
add_finding(
519564
findings,
520565
"PR metadata completeness",
521-
WARN,
566+
BLOCK,
522567
"pull_request.body",
523568
None,
524569
"; ".join(reason_parts),
@@ -621,12 +666,12 @@ def check_documentation_warning(findings: List[Finding], changed: Sequence[str],
621666
def collect_findings(root: Path, args: argparse.Namespace) -> List[Finding]:
622667
findings: List[Finding] = []
623668
statuses, changed = changed_paths(root, args)
624-
lines = added_lines(root, args)
669+
lines, removed_lines = changed_lines(root, args)
625670
body = read_pr_body(args.event_path)
626671
body_text = body or ""
627672

628673
check_line_endings(findings, root, changed, statuses, args)
629-
check_global_dependencies(findings, lines)
674+
check_global_dependencies(findings, lines, removed_lines)
630675
check_default_parameters(findings, lines)
631676
check_hpp_warnings(findings, statuses, lines)
632677
check_header_include_warnings(findings, lines)

tools/03_code_analysis/test_agent_governance_check.py

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -94,14 +94,43 @@ def test_allows_crlf_in_windows_scripts(self):
9494

9595
self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
9696

97-
def test_blocks_new_global_dependencies_on_added_lines(self):
97+
def test_blocks_when_global_dependency_budget_increases(self):
9898
self.write("source/source_base/global.cpp", "int n = GlobalV::NPROC + PARAM.inp.nbands;\n")
9999
self.write("source/source_base/CMakeLists.txt", "add_library(global global.cpp)\n")
100100
head = self.commit_change()
101101

102102
result = self.run_checker("--base", self.base, "--head", head)
103103

104-
self.assert_blocked_by(result, "No new cross-layer globals")
104+
self.assert_blocked_by(result, "Global dependency budget")
105+
self.assertIn("net_delta=2", result.stdout)
106+
107+
def test_warns_when_global_dependency_usage_is_rebalanced(self):
108+
self.write("source/source_base/global.cpp", "int old_n = PARAM.inp.nbands;\n")
109+
self.write("source/source_base/CMakeLists.txt", "add_library(global global.cpp)\n")
110+
self.git("add", ".")
111+
self.git("commit", "-m", "add baseline global usage")
112+
base = self.git("rev-parse", "HEAD").stdout.strip()
113+
self.write("source/source_base/global.cpp", "int moved_n = GlobalV::NPROC;\n")
114+
head = self.commit_change()
115+
116+
result = self.run_checker("--base", base, "--head", head)
117+
118+
self.assert_warns_with_success(result, "Global dependency budget")
119+
self.assertIn("net_delta=0", result.stdout)
120+
121+
def test_allows_global_dependency_budget_reduction(self):
122+
self.write("source/source_base/global.cpp", "int old_n = PARAM.inp.nbands;\n")
123+
self.write("source/source_base/CMakeLists.txt", "add_library(global global.cpp)\n")
124+
self.git("add", ".")
125+
self.git("commit", "-m", "add baseline global usage")
126+
base = self.git("rev-parse", "HEAD").stdout.strip()
127+
self.write("source/source_base/global.cpp", "int old_n = 0;\n")
128+
head = self.commit_change()
129+
130+
result = self.run_checker("--base", base, "--head", head)
131+
132+
self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
133+
self.assertNotIn("Global dependency budget", result.stdout)
105134

106135
def test_allows_global_names_in_documentation(self):
107136
self.write("docs/governance-notes.md", "Mention GlobalV::NPROC and PARAM.inp in documentation.\n")
@@ -575,14 +604,14 @@ def test_warns_for_heterogeneous_file_without_test_evidence(self):
575604

576605
self.assert_warns_with_success(result, "Heterogeneous test evidence review")
577606

578-
def test_staged_mode_checks_index_content(self):
607+
def test_staged_mode_blocks_global_dependency_budget_increase(self):
579608
self.write("source/source_base/staged.cpp", "int n = GlobalC::ucell.nat;\n")
580609
self.write("source/source_base/CMakeLists.txt", "add_library(staged staged.cpp)\n")
581610
self.git("add", ".")
582611

583612
result = self.run_checker("--staged")
584613

585-
self.assert_blocked_by(result, "No new cross-layer globals")
614+
self.assert_blocked_by(result, "Global dependency budget")
586615

587616
def test_rejects_staged_with_base_head(self):
588617
result = self.run_checker("--staged", "--base", self.base, "--head", self.base)

0 commit comments

Comments
 (0)