Skip to content

[CI] Fix parity commit run selection#3210

Closed
ethanwee1 wants to merge 6 commits into
developfrom
ethanwee/fix-parity-run-selection-shards
Closed

[CI] Fix parity commit run selection#3210
ethanwee1 wants to merge 6 commits into
developfrom
ethanwee/fix-parity-run-selection-shards

Conversation

@ethanwee1

@ethanwee1 ethanwee1 commented May 11, 2026 •
edited
Loading

Copy link
Copy Markdown

Summary

A set of parity-tooling fixes and parity-summary report improvements:

  • Fix parity commit run selection — pick the correct upstream run when a SHA has multiple/partial runs, so the report isn't built from the wrong run.
  • Exclude inductor.test_cpu_repro from the comparison (not a meaningful ROCm-vs-CUDA parity signal).
  • Surface XML failure messages in the report so devs (and the dashboard flow that parses them) can see why a test failed without digging into logs.
  • Per-row collapsible "Error Message" column — each failed-test row carries its message in a <details> toggle, so the table stays skimmable; the full untruncated text is always kept in the CSV artifact's Error Message column.
  • Budget failure messages against the 1 MiB step-summary cap — the whole .md is piped into $GITHUB_STEP_SUMMARY, which GitHub restricts to 1 MiB per step (the upload fails / content is dropped past that, see docs). Instead of hard-truncating every message to 2000 chars, messages are now sized against a global ~950 KB budget: the rest of the summary is measured first, then the largest uniform per-message cap that fits the remaining budget is applied. Typical runs render full tracebacks; only when a run would exceed the cap do the longest (outlier) messages get clipped. Full text remains in the CSV.

Files

  • generate_summary.py — run selection, failure-message column, the 1 MiB budgeting.
  • summarize_xml_testreports.py, detect_log_failures.py, download_testlogs — supporting changes for the above.

Test plan

  • Local unit + end-to-end checks of the summary budgeting: a run with 400 × ~20 KB tracebacks renders to ~949 KB (just under the 950 KB budget) with only the longest messages clipped, while a small run shows every message in full and untruncated.
  • Validate a parity run end-to-end and confirm the FAILED TESTS table renders the collapsible Error Message column and the step summary is accepted by GitHub.

@rocm-repo-management-api

rocm-repo-management-api Bot commented May 11, 2026 •
edited
Loading

Copy link
Copy Markdown

Jenkins build for 17c298d1b90b1a01b325b996e281915e9b9898e1 commit finished as FAILURE
Links: Pipeline Overview / Build artifacts / Test Results

@rocm-repo-management-api

rocm-repo-management-api Bot commented Jun 18, 2026 •
edited
Loading

Copy link
Copy Markdown

Jenkins build for 4fdcd4f120c32ac2ac23c3384474c0175eb28a2f commit finished as FAILURE
Links: Pipeline Overview / Build artifacts / Test Results

@rocm-repo-management-api

rocm-repo-management-api Bot commented Jun 19, 2026 •
edited
Loading

Copy link
Copy Markdown

Jenkins build for 0e43854731b9712013e265c8aae27d652159bd2c commit finished as FAILURE
Links: Pipeline Overview / Build artifacts / Test Results

@rocm-repo-management-api

rocm-repo-management-api Bot commented Jun 22, 2026 •
edited
Loading

Copy link
Copy Markdown

Jenkins build for c9e1a4ddfa5930a2bfd83bef39561ef9f127ecd9 commit finished as NOT_BUILT
Links: Pipeline Overview / Build artifacts / Test Results

@rocm-repo-management-api

rocm-repo-management-api Bot commented Jun 22, 2026 •
edited
Loading

Copy link
Copy Markdown

Jenkins build for f3a7377ac1d877a3efbf6836d916559cdd6bf1e6 commit finished as NOT_BUILT
Links: Pipeline Overview / Build artifacts / Test Results

@rocm-repo-management-api

rocm-repo-management-api Bot commented Jun 22, 2026 •
edited
Loading

Copy link
Copy Markdown

Jenkins build for f3a7377ac1d877a3efbf6836d916559cdd6bf1e6 commit finished as FAILURE
Links: Pipeline Overview / Build artifacts / Test Results

ethanwee1 added 6 commits June 22, 2026 21:17
@ethanwee1
[CI] Fix parity commit run selection
0e3d7a8 
Filter explicit-SHA parity lookups to main and carry commit labels through log parsing and running-time CSVs.
@ethanwee1
parity summary: exclude inductor.test_cpu_repro from comparison
099dad5 
test_cpu_repro is blocklisted on ROCm upstream, so it never runs there.
The parity summary was therefore counting it as MISSED against the CUDA
baseline, which is misleading. Add it to EXCLUDED_TEST_SUITES so it is
dropped from the ROCm-vs-CUDA comparison.
@ethanwee1
parity summary: surface XML failure messages
4412345 
Add the per-test error message (parsed from the test report XML, already
stored in the message_<set> CSV column) to the parity summary:

- CSV FAILED TESTS table gets a new 'Error Message' column so downstream
  dashboard parsing has it in structured form.
- Markdown summary gets a top-level collapsible 'Failure messages'
  <details> with one nested <details> per failed test (full traceback in
  an HTML-escaped <pre>), so the message is available without bloating
  the table.
@ethanwee1
parity summary: cap failure-message size in markdown
aef8a84 
The full .md is piped into $GITHUB_STEP_SUMMARY (1 MB cap); many full
tracebacks can push it over the limit and GitHub drops the whole summary.
Truncate each message to its tail (where the exception is) in the md;
the CSV 'Error Message' column keeps the full text for the dashboard.
@ethanwee1
parity summary: move failure message into a per-row collapsible column
6ed3a9d 
Replace the separate bottom 'Failure messages' section with an
'Error Message' column in the FAILED TESTS markdown table. Each cell is
a <details><summary>view</summary> toggle wrapping a <pre> (newlines as
&#10;, pipes escaped) so the table stays scannable and expands per row.
The CSV 'Error Message' column (full text) is unchanged.
@ethanwee1
parity summary: budget failure messages against the 1 MiB step-summar…
9255fdf 
…y cap

The per-row 'Error Message' column hard-truncated each message to 2000 chars so
the markdown couldn't exceed GitHub's per-step $GITHUB_STEP_SUMMARY limit
(1 MiB), which clipped useful tracebacks even on small runs with room to spare.

Raise the per-message cap to 50k and size messages against a global 950 KB
budget (1 MiB minus headroom for the surrounding tables): the rest of the
summary is measured first, then the largest uniform per-message cap that fits
the remaining budget is applied. Typical runs render full tracebacks; only when
a run would exceed the cap do the longest (outlier) messages get clipped. Full
text always remains in the CSV 'Error Message' column.

https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-commands#step-isolation-and-limits
@ethanwee1 ethanwee1 force-pushed the ethanwee/fix-parity-run-selection-shards branch from f3a7377 to 9255fdf Compare June 23, 2026 15:15
@ethanwee1 ethanwee1 changed the base branch from docker-full-clone-build-env to develop June 23, 2026 15:15
@ethanwee1 ethanwee1 marked this pull request as ready for review June 23, 2026 15:15
@rocm-repo-management-api

rocm-repo-management-api Bot commented Jun 23, 2026

Copy link
Copy Markdown

Jenkins build for 9255fdf99f4026aa9ffd5bc0b8588c33c70d9491 commit is in progress
Links: Pipeline Overview / Build artifacts

This was referenced Jun 23, 2026
Open
Merged
Merged
@ethanwee1

ethanwee1 commented Jun 23, 2026

Copy link
Copy Markdown
Author

Closing in favor of splitting this into three smaller, independently-reviewable PRs (the download_testlogs run-selection portion here was already superseded by #3278 on develop):

All three are based on the current develop.

@ethanwee1 ethanwee1 closed this Jun 23, 2026
jithunnair-amd pushed a commit that referenced this pull request Jun 24, 2026
@ethanwee1
[CI] Parity: surface XML failure messages in the summary (#3364)
001abbc 
Split out of #3210 (3 of 3) for easier review.

Surfaces test failure messages inline in the parity summary
(`generate_summary.py`):

- Extract the XML failure message per failed test and add it as a
per-row collapsible **Error Message** column in the FAILED TESTS table.
- Cap individual messages and, crucially, **budget total message size
against GitHub's 1 MiB step-summary limit** so a run with many large
tracebacks doesn't drop the entire summary. Only the longest messages
are clipped, and only when a run would otherwise exceed the budget; the
full text always stays in the CSV artifact.

Independent of the other two split PRs (touches only
`generate_summary.py`).

Made with [Cursor](https://cursor.com)
jithunnair-amd added a commit that referenced this pull request Jun 24, 2026
@ethanwee1 @jithunnair-amd
[CI] Parity: exclude inductor.test_cpu_repro from comparison (#3363)
93a1a68 
Split out of #3210 (2 of 3) for easier review.

`inductor.test_cpu_repro` is blocklisted on ROCm upstream, so it never
runs there and would otherwise show up as falsely **MISSED** against the
CUDA baseline (~791 phantom MISSED rows on a recent mi350 run). Add it
to `EXCLUDED_TEST_SUITES` so it is dropped from the ROCm-vs-CUDA
comparison.

Verified against a real mi350 run: removes all `inductor.test_cpu_repro`
rows; MISSED gap drops ~1,294 -> ~503 and DEFAULT disagreement 3.03% ->
2.77% (SKIPPED unchanged).

Made with [Cursor](https://cursor.com)

---------

Co-authored-by: Jithun Nair <37884920+jithunnair-amd@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@ethanwee1