Skip to content

Commit cd8cc45

Browse files
authored
feat(ci): surface pre-existing failures in PR run-slow comment (#50122)
* feat(ci): surface pre-existing failures in PR CI comment When a PR fixes a pre-existing test failure, `run-slow` previously silently dropped it (failing on both PR and base = "not new"). This made it impossible to verify the fix via the CI comment. The report now buckets failures into three categories: - New (✅ ⟹ ❌): bad_commit is not None, no "DIFFERENT error message" - Error changed (❌ ⟹ ❌): bad_commit is not None, "DIFFERENT error message" - Same error (❌ ⟹ ❌): bad_commit is None, failure_at_base_commit non-empty Flaky / bisect-failed entries (bad_commit=None, failure_at_base_commit='') are still silently discarded as before. The PR commit status (green/red) is based on new failures only, so a PR with only pre-existing failures still gets a green check. Pre-existing failures are shown below the status line in an always-visible section. Same-error lists with >10 entries are collapsed behind a "show all N" toggle using the dl/dd trick to avoid ugly blockquote styling. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(ci): keep error_changed failures red, fix unused vars - error_changed (bad_commit set, different error message) was accidentally moved to green by the previous commit. The gate now fires (STATUS=failure) when new_count + error_changed_count > 0, matching the old behaviour where bad_commit is not None counted as a failure. - When only error_changed failures are present (no new), the header says "N tests with changed errors" instead of "N new failing tests". - Unused preexisting_count return values replaced with _. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(ci): extract report formatting into utils/format_ci_report.py Move the inline Python logic from self-comment-ci.yml and self-comment-ci-amd.yml into a shared script so both workflows stay in sync. The script accepts --report-path, --url-path, --header-out, --details-out, and --kind (model|quantization). This also brings self-comment-ci-amd.yml up to date with the new bucketing logic (pre-existing failures now visible in the AMD section of the PR comment). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(ci): show same-error details in AMD success path Mirror the detail-showing logic from the Nvidia workflow: when STATUS=success but model_ci_details.txt exists (same-error-only failures), cat it into the comment body so the AMD section is consistent with the Nvidia section. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * style: ruff format and fix PERF102 in format_ci_report.py Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(ci): add checkout to report job so format_ci_report.py is available The report job had no checkout step, so python3 utils/format_ci_report.py would fail with "can't open file" — the workspace only contains downloaded artifacts, not the repo. Add actions/checkout and contents: read permission to both self-comment-ci.yml and self-comment-ci-amd.yml. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(ci): use fetch-depth=1 in report job checkout We only need utils/format_ci_report.py, not the full git history. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(ci): group by (model, job_link) not model alone in format_entries_by_model A model failing on both single-gpu and multi-gpu has different job_links per device. Grouping only by model meant all tests were listed under the first device's link. Group by (model, job_link) to emit a separate header per device, matching the old behaviour. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(ci): improve header label and remove fragile status string check - Header now shows both parts when new and error_changed coexist: e.g. "❌ [2 new failing tests · 1 tests with changed errors](url) 😭" so the one-liner accurately reflects all red-gate failures. - Remove redundant "SAME error message" in status check: bad_commit is None is already implied by the elif branch, and failure_at_base non-empty uniquely identifies same-error entries per check_bad_commit.py logic. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(ci): add TODO for potential 65536-char GitHub comment limit Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: ydshieh <ydshieh@users.noreply.github.com>
1 parent aaf32f8 commit cd8cc45

3 files changed

Lines changed: 269 additions & 194 deletions

File tree

‎.github/workflows/self-comment-ci-amd.yml‎

Lines changed: 16 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -277,11 +277,17 @@ jobs:
277277
name: Check & Report
278278
needs: [get-pr-number, get-pr-info, check-timestamps, create_run, model-ci]
279279
permissions:
280+
contents: read
280281
pull-requests: write
281282
statuses: write
282283
if: ${{ always() && needs.create_run.result == 'success' }}
283284
runs-on: ubuntu-22.04
284285
steps:
286+
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
287+
with:
288+
fetch-depth: "1"
289+
persist-credentials: false
290+
285291
- name: Download model CI new failures artifact
286292
uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1
287293
with:
@@ -309,90 +315,12 @@ jobs:
309315
310316
- name: Process and filter reports
311317
run: |
312-
# Preprocess with Python
313-
python3 << 'PYTHON_SCRIPT'
314-
import json
315-
import os
316-
from pathlib import Path
317-
318-
def count_failures(data):
319-
"""
320-
Count total number of failures (excluding None commits)
321-
"""
322-
total = 0
323-
for model, model_result in data.items():
324-
for device, failures in model_result.items():
325-
# Count failures where commit is not None
326-
total += sum(
327-
1 for failure in failures
328-
if isinstance(failure, dict) and failure.get('bad_commit') is not None
329-
)
330-
return total
331-
332-
def filter_and_format_report(data):
333-
"""
334-
Filter out entries where commit is `None` (failing tests who status is not certain) and format as text
335-
"""
336-
lines = []
337-
338-
for model, model_result in data.items():
339-
model_lines = []
340-
for device, failures in model_result.items():
341-
342-
filtered_failures = [
343-
failure for failure in failures
344-
if isinstance(failure, dict) and failure.get('bad_commit') is not None
345-
]
346-
347-
# Add tests to model lines
348-
for idx, failure in enumerate(filtered_failures):
349-
if idx == 0:
350-
job_link = failure['job_link']
351-
model_lines.append(f"- [{model}]({job_link}):")
352-
353-
test_name = failure['test']
354-
status = failure.get('status', '')
355-
if "DIFFERENT error message" in status:
356-
indicator = "(❌ ⟹ ❌)"
357-
else:
358-
indicator = "(✅ ⟹ ❌)"
359-
360-
model_lines.append(f" {test_name} {indicator}")
361-
362-
if len(model_lines) > 0:
363-
lines.extend(model_lines)
364-
lines.append("") # Empty line between models
365-
366-
return "\n".join(lines).strip()
367-
368-
# Read report from downloaded artifact file
369-
model_report_path = Path('./new_failures/new_failures_with_bad_commit_run_models_gpu/new_failures_with_bad_commit.json')
370-
model_url_path = Path('./new_failures/new_failures_with_bad_commit_run_models_gpu/new_failures_with_bad_commit_url.txt')
371-
372-
model_url = None
373-
if model_url_path.exists():
374-
with open(model_url_path, 'r') as f:
375-
model_url = f.read().strip()
376-
377-
model_report = {}
378-
if model_report_path.exists():
379-
with open(model_report_path, 'r') as f:
380-
model_report = json.load(f)
381-
382-
# Count failures and format
383-
model_count = count_failures(model_report)
384-
formatted_model = filter_and_format_report(model_report)
385-
386-
# Write to files
387-
if formatted_model:
388-
header = f"❌ [{model_count} new failing tests]({model_url}) 😭" if model_url else f"❌ {model_count} new failing tests 😭"
389-
with open('model_ci_header.txt', 'w') as f:
390-
f.write(header)
391-
with open('model_ci_details.txt', 'w') as f:
392-
f.write("<details>\n<summary>Failure details</summary>\n\n")
393-
f.write(formatted_model)
394-
f.write('\n\n</details>\n')
395-
PYTHON_SCRIPT
318+
python3 utils/format_ci_report.py \
319+
--report-path ./new_failures/new_failures_with_bad_commit_run_models_gpu/new_failures_with_bad_commit.json \
320+
--url-path ./new_failures/new_failures_with_bad_commit_run_models_gpu/new_failures_with_bad_commit_url.txt \
321+
--header-out model_ci_header.txt \
322+
--details-out model_ci_details.txt \
323+
--kind model
396324
397325
- name: Post results as PR comment
398326
env:
@@ -433,6 +361,10 @@ jobs:
433361
else
434362
echo "STATUS=success" >> $GITHUB_ENV
435363
echo "**AMD** · ✅ No new failures 🎉👏 · ⚙️ [Workflow]($GITHUB_RUN_URL) · 📊 [Dashboard]($DASHBOARD_URL)"
364+
if [ -s model_ci_details.txt ]; then
365+
echo ''
366+
cat model_ci_details.txt
367+
fi
436368
fi
437369
} > comment_body.txt
438370

‎.github/workflows/self-comment-ci.yml‎

Lines changed: 26 additions & 110 deletions
Original file line numberDiff line numberDiff line change
@@ -265,11 +265,17 @@ jobs:
265265
name: Check & Report
266266
needs: [get-pr-number, get-pr-info, check-timestamps, create_run, model-ci, quantization-ci]
267267
permissions:
268+
contents: read
268269
pull-requests: write
269270
statuses: write
270271
if: ${{ always() && needs.create_run.result == 'success' }}
271272
runs-on: ubuntu-22.04
272273
steps:
274+
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
275+
with:
276+
fetch-depth: "1"
277+
persist-credentials: false
278+
273279
- name: Download model CI new failures artifact
274280
uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1
275281
with:
@@ -312,116 +318,18 @@ jobs:
312318
313319
- name: Process and filter reports
314320
run: |
315-
# Preprocess with Python
316-
python3 << 'PYTHON_SCRIPT'
317-
import json
318-
import os
319-
from pathlib import Path
320-
321-
def count_failures(data):
322-
"""
323-
Count total number of failures (excluding None commits)
324-
"""
325-
total = 0
326-
for model, model_result in data.items():
327-
for device, failures in model_result.items():
328-
# Count failures where commit is not None
329-
total += sum(
330-
1 for failure in failures
331-
if isinstance(failure, dict) and failure.get('bad_commit') is not None
332-
)
333-
return total
334-
335-
def filter_and_format_report(data):
336-
"""
337-
Filter out entries where commit is `None` (failing tests who status is not certain) and format as text
338-
"""
339-
lines = []
340-
341-
for model, model_result in data.items():
342-
model_lines = []
343-
for device, failures in model_result.items():
344-
345-
filtered_failures = [
346-
failure for failure in failures
347-
if isinstance(failure, dict) and failure.get('bad_commit') is not None
348-
]
349-
350-
# Add tests to model lines
351-
for idx, failure in enumerate(filtered_failures):
352-
if idx == 0:
353-
job_link = failure['job_link']
354-
model_lines.append(f"- [{model}]({job_link}):")
355-
356-
test_name = failure['test']
357-
status = failure.get('status', '')
358-
if "DIFFERENT error message" in status:
359-
indicator = "(❌ ⟹ ❌)"
360-
else:
361-
indicator = "(✅ ⟹ ❌)"
362-
363-
model_lines.append(f" {test_name} {indicator}")
364-
365-
if len(model_lines) > 0:
366-
lines.extend(model_lines)
367-
lines.append("") # Empty line between models
368-
369-
return "\n".join(lines).strip()
370-
371-
# Read reports from downloaded artifact files
372-
model_report_path = Path('./new_failures/new_failures_with_bad_commit_run_models_gpu/new_failures_with_bad_commit.json')
373-
quant_report_path = Path('./new_failures/new_failures_with_bad_commit_run_quantization_torch_gpu/new_failures_with_bad_commit.json')
374-
375-
# Read URL files if they exist
376-
model_url_path = Path('./new_failures/new_failures_with_bad_commit_run_models_gpu/new_failures_with_bad_commit_url.txt')
377-
quant_url_path = Path('./new_failures/new_failures_with_bad_commit_run_quantization_torch_gpu/new_failures_with_bad_commit_url.txt')
378-
379-
model_url = None
380-
if model_url_path.exists():
381-
with open(model_url_path, 'r') as f:
382-
model_url = f.read().strip()
383-
384-
quant_url = None
385-
if quant_url_path.exists():
386-
with open(quant_url_path, 'r') as f:
387-
quant_url = f.read().strip()
388-
389-
model_report = {}
390-
if model_report_path.exists():
391-
with open(model_report_path, 'r') as f:
392-
model_report = json.load(f)
393-
394-
quant_report = {}
395-
if quant_report_path.exists():
396-
with open(quant_report_path, 'r') as f:
397-
quant_report = json.load(f)
398-
399-
# Count failures
400-
model_count = count_failures(model_report)
401-
quant_count = count_failures(quant_report)
402-
403-
formatted_model = filter_and_format_report(model_report)
404-
formatted_quant = filter_and_format_report(quant_report)
405-
406-
# Write to files
407-
if formatted_model:
408-
header = f"❌ [{model_count} new failing tests]({model_url}) 😭" if model_url else f"❌ {model_count} new failing tests 😭"
409-
with open('model_ci_header.txt', 'w') as f:
410-
f.write(header)
411-
with open('model_ci_details.txt', 'w') as f:
412-
f.write("<details>\n<summary>Failure details</summary>\n\n")
413-
f.write(formatted_model)
414-
f.write('\n\n</details>\n')
415-
416-
if formatted_quant:
417-
header = f"❌ [{quant_count} new failing quant tests]({quant_url}) 😭" if quant_url else f"❌ {quant_count} new failing quant tests 😭"
418-
with open('quantization_ci_header.txt', 'w') as f:
419-
f.write(header)
420-
with open('quantization_ci_details.txt', 'w') as f:
421-
f.write("<details>\n<summary>Failure details (quantization)</summary>\n\n")
422-
f.write(formatted_quant)
423-
f.write('\n\n</details>\n')
424-
PYTHON_SCRIPT
321+
python3 utils/format_ci_report.py \
322+
--report-path ./new_failures/new_failures_with_bad_commit_run_models_gpu/new_failures_with_bad_commit.json \
323+
--url-path ./new_failures/new_failures_with_bad_commit_run_models_gpu/new_failures_with_bad_commit_url.txt \
324+
--header-out model_ci_header.txt \
325+
--details-out model_ci_details.txt \
326+
--kind model
327+
python3 utils/format_ci_report.py \
328+
--report-path ./new_failures/new_failures_with_bad_commit_run_quantization_torch_gpu/new_failures_with_bad_commit.json \
329+
--url-path ./new_failures/new_failures_with_bad_commit_run_quantization_torch_gpu/new_failures_with_bad_commit_url.txt \
330+
--header-out quantization_ci_header.txt \
331+
--details-out quantization_ci_details.txt \
332+
--kind quantization
425333
426334
- name: Post results as PR comment
427335
env:
@@ -481,6 +389,14 @@ jobs:
481389
else
482390
echo "STATUS=success" >> $GITHUB_ENV
483391
echo "**Nvidia** · ✅ No new failures 🎉👏 · ⚙️ [Workflow]($GITHUB_RUN_URL) · 📊 [Dashboard]($DASHBOARD_URL)"
392+
if [ -s model_ci_details.txt ]; then
393+
echo ''
394+
cat model_ci_details.txt
395+
fi
396+
if [ -s quantization_ci_details.txt ]; then
397+
echo ''
398+
cat quantization_ci_details.txt
399+
fi
484400
fi
485401
} > comment_body.txt
486402

0 commit comments

Comments
 (0)