Skip to content

Commit 0e249b5

Browse files
authored
Prevent oversized Forge PR descriptions from blocking publication (#9014)
* Bound Forge pull request descriptions * Fix bounded native test diff formatting
1 parent 3a45b5c commit 0e249b5

10 files changed

Lines changed: 156 additions & 32 deletions

forge/docs/git-scripts.md

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,12 @@ parameters its workflow's metrics actually produce. The subsections below state
7575
that per-publisher subset; publishers whose workflows share a body shape are
7676
grouped together.
7777

78+
PR bodies must remain publishable through GitHub. The shared publication helper
79+
therefore bounds optional generated detail below GitHub's body limit while
80+
preserving the issue link, summary, metrics, intervention record, and local CI
81+
evidence. Version-to-version test comparisons include a diff stat and a bounded
82+
excerpt; reviewers use the PR's **Files changed** tab for the complete diff.
83+
7884
### New library support and coverage improvement
7985

8086
Publishers: `make_pr_new_library_support.py` (`library-new-request`) and
@@ -96,8 +102,8 @@ single-PR run and `Refs:` for non-final chunked dynamic-access chunks
96102
Publishers: `make_pr_javac_fix.py` (`fixes-javac-fail`) and
97103
`make_pr_java_run_fix.py` (`fixes-java-run-fail`). They share one body shape:
98104
the agent generation metrics above, a stats comparison for the bumped version,
99-
and a unified diff between the previous version's and the new version's test
100-
sources so reviewers can see exactly what the fix changed. The two differ only
105+
and a bounded test-source comparison so reviewers can see what the fix changed
106+
without preventing PR creation. The two differ only
101107
in workflow identity (compilation vs. runtime wording), the metrics file, and
102108
the PR label (§WF-java-fail-fix-workflow).
103109

forge/git_scripts/make_pr_improve_coverage.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
BASE_BRANCH,
2929
REPO,
3030
REVIEWERS,
31+
bound_pr_body,
3132
parse_pr_number,
3233
publish_branch,
3334
)
@@ -331,7 +332,7 @@ def create_pull_request(
331332
"gh", "pr", "create",
332333
"--repo", REPO,
333334
"--title", title,
334-
"--body", body,
335+
"--body", bound_pr_body(body),
335336
"--base", BASE_BRANCH,
336337
"--head", f"{origin_owner}:{branch}",
337338
"--label", "GenAI",

forge/git_scripts/make_pr_java_run_fix.py

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@
2323
BASE_BRANCH,
2424
REPO,
2525
REVIEWERS,
26-
generate_diff_text,
26+
bound_pr_body,
27+
format_bounded_test_diff_section,
2728
publish_branch,
2829
stage_library_version_paths,
2930
)
@@ -88,7 +89,7 @@ def create_pull_request(
8889
"--title",
8990
title,
9091
"--body",
91-
body,
92+
bound_pr_body(body),
9293
"--base",
9394
BASE_BRANCH,
9495
"--head",
@@ -158,11 +159,7 @@ def build_pull_request_preview(
158159
159160
{format_forge_revision_section()}
160161
{format_stats_diff(repo_path, old_coordinates, new_coordinates)}
161-
**Comparison between existing test version and AI-Generated update**
162-
163-
```diff
164-
{generate_diff_text(group, artifact, old_version, new_version, repo_path)}
165-
```
162+
{format_bounded_test_diff_section(group, artifact, old_version, new_version, repo_path)}
166163
"""
167164
post_generation_intervention = metrics_entry.get("post_generation_intervention")
168165
if post_generation_intervention:

forge/git_scripts/make_pr_javac_fix.py

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@
2323
BASE_BRANCH,
2424
REPO,
2525
REVIEWERS,
26-
generate_diff_text,
26+
bound_pr_body,
27+
format_bounded_test_diff_section,
2728
publish_branch,
2829
stage_library_version_paths,
2930
)
@@ -93,7 +94,7 @@ def create_pull_request(
9394
"--title",
9495
title,
9596
"--body",
96-
body,
97+
bound_pr_body(body),
9798
"--base",
9899
BASE_BRANCH,
99100
"--head",
@@ -163,11 +164,7 @@ def build_pull_request_preview(
163164
164165
{format_forge_revision_section()}
165166
{format_stats_diff(repo_path, old_coordinates, new_coordinates)}
166-
**Comparison between existing test version and AI-Generated update**
167-
168-
```diff
169-
{generate_diff_text(group, artifact, old_version, new_version, repo_path)}
170-
```
167+
{format_bounded_test_diff_section(group, artifact, old_version, new_version, repo_path)}
171168
"""
172169
post_generation_intervention = metrics_entry.get("post_generation_intervention")
173170
if post_generation_intervention:

forge/git_scripts/make_pr_new_library_support.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
BASE_BRANCH,
2929
REPO,
3030
REVIEWERS,
31+
bound_pr_body,
3132
parse_pr_number,
3233
publish_branch,
3334
stage_library_version_paths,
@@ -405,7 +406,7 @@ def create_pull_request(
405406
"gh", "pr", "create",
406407
"--repo", REPO,
407408
"--title", title,
408-
"--body", body,
409+
"--body", bound_pr_body(body),
409410
"--base", BASE_BRANCH,
410411
"--head", f"{origin_owner}:{branch}",
411412
"--label", "GenAI",

forge/git_scripts/make_pr_ni_run_fix.py

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@
2525
BASE_BRANCH,
2626
REPO,
2727
REVIEWERS,
28-
generate_diff_text,
28+
bound_pr_body,
29+
format_bounded_test_diff_section,
2930
publish_branch,
3031
)
3132
from utility_scripts.metadata_index import resolve_test_version
@@ -170,7 +171,7 @@ def create_pull_request(
170171
"gh", "pr", "create",
171172
"--repo", REPO,
172173
"--title", title,
173-
"--body", body,
174+
"--body", bound_pr_body(body),
174175
"--base", BASE_BRANCH,
175176
"--head", f"{origin_owner}:{branch}",
176177
"--label", pr_label,
@@ -220,12 +221,7 @@ def build_test_comparison_section(group: str, artifact: str, old_version: str, n
220221
new_test_dir = os.path.join(repo_path, "tests", "src", group, artifact, new_version)
221222
if not os.path.isdir(new_test_dir):
222223
return ""
223-
return (
224-
"\n\n**Comparison between existing test version and AI-Generated update**\n\n"
225-
"```diff\n"
226-
f"{generate_diff_text(group, artifact, old_version, new_version, repo_path)}\n"
227-
"```"
228-
)
224+
return format_bounded_test_diff_section(group, artifact, old_version, new_version, repo_path)
229225

230226

231227
def build_pull_request_preview(

forge/git_scripts/make_pr_not_for_native_image.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
BASE_BRANCH,
2323
REPO,
2424
REVIEWERS,
25+
bound_pr_body,
2526
publish_branch,
2627
)
2728
from utility_scripts.metadata_index import get_not_for_native_image_marker
@@ -100,7 +101,7 @@ def create_pull_request(
100101
"gh", "pr", "create",
101102
"--repo", REPO,
102103
"--title", title,
103-
"--body", body,
104+
"--body", bound_pr_body(body),
104105
"--base", BASE_BRANCH,
105106
"--head", f"{origin_owner}:{branch}",
106107
"--label", "GenAI",

forge/git_scripts/pr_publication.py

Lines changed: 74 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,9 @@
5151
"human-intervention-logs",
5252
"forge/human-intervention-logs",
5353
)
54+
MAX_PR_BODY_CHARS: int = 60_000
55+
MAX_INLINE_TEST_DIFF_CHARS: int = 12_000
56+
PR_BODY_REQUIRED_TAIL_CHARS: int = 12_000
5457

5558

5659
def _publication_resume_marker(repo_path: str) -> ContinuationMarker | None:
@@ -260,8 +263,14 @@ def _copy_git_files_under(repo_path: str, source_dir: str, destination_dir: str)
260263
shutil.copy2(source_file, destination_file)
261264

262265

263-
def generate_diff_text(group: str, artifact: str, current_version: str, new_version: str, repo_path: str) -> str:
264-
"""Build a diff text between the current and new test directories for a library version."""
266+
def _generate_test_diff_outputs(
267+
group: str,
268+
artifact: str,
269+
current_version: str,
270+
new_version: str,
271+
repo_path: str,
272+
) -> tuple[str, str]:
273+
"""Build full and stat test diffs for bounded PR descriptions (§GIT-pr-body)."""
265274
old_dir = os.path.join(repo_path, "tests", "src", group, artifact, current_version)
266275
new_dir = os.path.join(repo_path, "tests", "src", group, artifact, new_version)
267276
old_display_dir = os.path.relpath(old_dir, repo_path).replace(os.sep, "/")
@@ -277,23 +286,83 @@ def generate_diff_text(group: str, artifact: str, current_version: str, new_vers
277286
_copy_git_files_under(repo_path, old_dir, tmp_old)
278287
_copy_git_files_under(repo_path, new_dir, tmp_new)
279288

280-
# Compute diff text using: git diff --no-index --find-renames tmp_old tmp_new
289+
diff_args = ["git", "diff", "--no-index", "--find-renames"]
281290
res = subprocess.run(
282-
["git", "diff", "--no-index", "--find-renames", tmp_old, tmp_new],
291+
[*diff_args, tmp_old, tmp_new],
283292
capture_output=True,
284293
text=True,
285294
)
286295
if res.returncode in (0, 1):
287296
diff_text = res.stdout
297+
stat_res = subprocess.run(
298+
[*diff_args, "--stat", tmp_old, tmp_new],
299+
capture_output=True,
300+
text=True,
301+
)
302+
if stat_res.returncode not in (0, 1):
303+
raise RuntimeError(
304+
f"Subprocess failed with return code: {stat_res.returncode}. "
305+
f"Error output:\n{stat_res.stderr.strip()}"
306+
)
288307

289308
diff_text = diff_text.replace(tmp_old.replace(os.sep, "/"), old_diff_display_dir)
290309
diff_text = diff_text.replace(tmp_new.replace(os.sep, "/"), new_diff_display_dir)
291310
diff_text = diff_text.replace(tmp_old, old_diff_display_dir)
292311
diff_text = diff_text.replace(tmp_new, new_diff_display_dir)
293-
return diff_text
312+
return diff_text, stat_res.stdout
294313

295314
error_message = (
296315
f"Subprocess failed with return code: {res.returncode}. "
297316
f"Error output:\n{res.stderr.strip()}"
298317
)
299318
raise RuntimeError(error_message)
319+
320+
321+
def generate_diff_text(group: str, artifact: str, current_version: str, new_version: str, repo_path: str) -> str:
322+
"""Build the complete test diff for callers that need it."""
323+
diff_text, _diff_stat = _generate_test_diff_outputs(
324+
group, artifact, current_version, new_version, repo_path,
325+
)
326+
return diff_text
327+
328+
329+
def format_bounded_test_diff_section(
330+
group: str,
331+
artifact: str,
332+
current_version: str,
333+
new_version: str,
334+
repo_path: str,
335+
) -> str:
336+
"""Format a reviewable test diff excerpt that cannot exhaust a PR body (§GIT-pr-body)."""
337+
diff_text, diff_stat = _generate_test_diff_outputs(
338+
group, artifact, current_version, new_version, repo_path,
339+
)
340+
excerpt = diff_text[:MAX_INLINE_TEST_DIFF_CHARS]
341+
truncation_note = ""
342+
if len(diff_text) > MAX_INLINE_TEST_DIFF_CHARS:
343+
truncation_note = (
344+
"\n\nThe complete test diff is available in this pull request's "
345+
"**Files changed** tab."
346+
)
347+
return (
348+
"**Test-source comparison**\n\n"
349+
"```text\n"
350+
f"{diff_stat.strip()}\n"
351+
"```\n\n"
352+
"```diff\n"
353+
f"{excerpt}\n"
354+
"```"
355+
f"{truncation_note}"
356+
)
357+
358+
359+
def bound_pr_body(body: str) -> str:
360+
"""Keep all publisher PR bodies below GitHub's limit (§GIT-pr-body)."""
361+
if len(body) <= MAX_PR_BODY_CHARS:
362+
return body
363+
truncation_notice = (
364+
"\n\n---\n\nAdditional generated detail was omitted to keep this pull request "
365+
"description within GitHub's size limit."
366+
)
367+
head_chars = MAX_PR_BODY_CHARS - len(truncation_notice) - PR_BODY_REQUIRED_TAIL_CHARS
368+
return body[:head_chars].rstrip() + truncation_notice + body[-PR_BODY_REQUIRED_TAIL_CHARS:]

forge/tests/test_make_pr_ni_run_fix.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,33 @@ def fake_gh(*args: str, check: bool = True):
6262

6363

6464
class NativeImageRunFinalizationTests(unittest.TestCase):
65+
def test_build_test_comparison_section_returns_bounded_markdown_section(self) -> None:
66+
with tempfile.TemporaryDirectory() as repo_path:
67+
new_test_dir = os.path.join(
68+
repo_path,
69+
"tests",
70+
"src",
71+
"org.example",
72+
"demo",
73+
"2.0.0",
74+
)
75+
os.makedirs(new_test_dir)
76+
bounded_section = (
77+
"**Test-source comparison**\n\n```text\n1 file changed\n```\n\n"
78+
"```diff\n+change\n```"
79+
)
80+
81+
with patch.object(
82+
make_pr_ni_run_fix,
83+
"format_bounded_test_diff_section",
84+
return_value=bounded_section,
85+
):
86+
comparison_section = make_pr_ni_run_fix.build_test_comparison_section(
87+
"org.example", "demo", "1.0.0", "2.0.0", repo_path,
88+
)
89+
90+
self.assertEqual(comparison_section, bounded_section)
91+
6592
def test_stage_and_commit_includes_jvm_test_source_directories(self) -> None:
6693
with tempfile.TemporaryDirectory() as repo_path:
6794
test_version_dir = os.path.join(

forge/tests/test_pr_publication.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import subprocess
88
import tempfile
99
import unittest
10+
from unittest.mock import patch
1011

1112
from git_scripts import pr_publication
1213
from utility_scripts.continuation_marker import CONTINUATION_MARKER_FILENAME
@@ -26,6 +27,34 @@ def _git(repo_path: str, *args: str) -> str:
2627

2728

2829
class PrPublicationTests(unittest.TestCase):
30+
def test_format_bounded_test_diff_section_truncates_large_diff(self) -> None:
31+
diff_text = "Q" * (pr_publication.MAX_INLINE_TEST_DIFF_CHARS + 1)
32+
with patch.object(
33+
pr_publication,
34+
"_generate_test_diff_outputs",
35+
return_value=(diff_text, " 1 file changed, 1 insertion(+)")
36+
):
37+
section = pr_publication.format_bounded_test_diff_section(
38+
"org.example", "demo", "1.0.0", "2.0.0", "/repo",
39+
)
40+
41+
self.assertIn("1 file changed", section)
42+
self.assertIn("Files changed", section)
43+
self.assertNotIn(diff_text, section)
44+
self.assertEqual(section.count("Q"), pr_publication.MAX_INLINE_TEST_DIFF_CHARS)
45+
46+
def test_bound_pr_body_keeps_body_below_github_limit(self) -> None:
47+
oversized_body = (
48+
"x" * (pr_publication.MAX_PR_BODY_CHARS + 100)
49+
+ "\n\n## Local CI Verification\n\nPassed"
50+
)
51+
52+
bounded_body = pr_publication.bound_pr_body(oversized_body)
53+
54+
self.assertLessEqual(len(bounded_body), pr_publication.MAX_PR_BODY_CHARS)
55+
self.assertIn("GitHub's size limit", bounded_body)
56+
self.assertIn("Local CI Verification", bounded_body)
57+
2958
def test_preservation_cleanup_keeps_pending_metrics_as_local_input(self) -> None:
3059
with tempfile.TemporaryDirectory() as repo_path:
3160
_git(repo_path, "init")

0 commit comments

Comments
 (0)