Skip to content

fix(ci): pass required framework input to triton-test (unblock PR workflow) - #14768

Closed
nnshah1 wants to merge 1 commit into
mainfrom
neelays/fix-ci-triton-test-framework
Closed

nnshah1 wants to merge 1 commit into
mainfrom
neelays/fix-ci-triton-test-framework

Conversation

@nnshah1

@nnshah1 nnshah1 commented Sep 12, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes a repo-wide PR-workflow startup_failure that has blocked heavy CI on every PR based on main since ~2026-09-11 23:00 UTC.

shared-test.yml declares framework as a required input, but the triton-test job (added in #13774) does not pass it. A reusable-workflow call missing a required input fails workflow startup validation, which aborts the entire PR workflow before any job runs — so this is not triton-specific, it breaks CI for all PRs.

Root cause — merge skew: #14688 made framework required in shared-test.yml; #13774 added triton-test in parallel without it. Neither PR re-ran against the other (triton jobs are path-filtered to skipped on unrelated diffs), so the break only surfaced once both landed on main.

Every other shared-test caller (vllm / sglang / trtllm / planner) already passes framework. This adds the missing framework: triton to match — a one-line change.

Validation

  • pr.yaml parses as valid YAML.
  • All shared-test.yml callers now pass the required framework input (verified per-job; triton-test was the only one missing it).

Regression from #13774 (× #14688).

Summary by CodeRabbit

  • Tests
    • Updated the Triton test workflow configuration to ensure tests run with the correct framework setting.

…kflow)

shared-test.yml requires the `framework` input, but the triton-test job added
in #13774 omits it. Because a reusable-workflow call missing a required input
fails workflow *startup* validation, this breaks the entire `PR` workflow at
startup for every PR based on main — repo-wide, not triton-specific.

Root cause is a merge skew: #14688 made `framework` required in shared-test.yml
and #13774 added triton-test in parallel without it; neither PR re-ran against
the other (triton jobs are path-filtered to "skipped" on unrelated diffs), so
the break only appeared once both were on main.

Every other shared-test caller (vllm/sglang/trtllm/planner) already passes
`framework`; this adds the missing `framework: triton` to match.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: nnshah1 <neelays@nvidia.com>
@nnshah1
nnshah1 requested a review from a team as a code owner September 12, 2026 20:27
@nnshah1

nnshah1 commented Sep 12, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 4533984

@coderabbitai

coderabbitai Bot commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 169e91a2-c891-4c15-8a6d-7cd491701d54

📥 Commits

Reviewing files that changed from the base of the PR and between 4a3943c and 4533984.

📒 Files selected for processing (1)
  • .github/workflows/pr.yaml

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.


Walkthrough

The Triton test job now passes framework: triton to the shared test workflow.

Changes

Triton test workflow

Layer / File(s) Summary
Pass Triton framework input
.github/workflows/pr.yaml
The Triton test job passes the triton framework identifier to the shared test workflow.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~3 minutes

Merge Risk: ⚪ Minimal · up to 45339

The workflow startup failure is addressed, with no remaining merge-blocking risk identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description explains the root cause, impact, change, and validation. However, it does not follow the required template headings and omits the required Related Issues section, including confirmatio… Add the required Overview, Details, Where should the reviewer start?, and Related Issues sections. In Related Issues, include a valid issue link or check the no-related-issue confirmation.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the CI fix and the missing framework input for the Triton test workflow.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description explains the root cause, impact, change, and validation. However, it does not follow the required template headings and omits the required Related Issues section, including confirmation that no related issue exists or a valid issue reference.

  • Fix all pre-merge checks with AI

Comment @coderabbitai help to get the list of available commands.

@nnshah1

nnshah1 commented Sep 12, 2026

Copy link
Copy Markdown
Contributor Author

Closing as a duplicate of #14760, which lands the same framework: triton fix and also covers post-merge-ci.yml (which #14768 missed). #14760 is already approved — deferring to it. Thanks @bzsuni.

@nnshah1 nnshah1 closed this Sep 12, 2026
Comment thread .github/workflows/pr.yaml
uses: $/.github/workflows/shared-test.yml
with:
test_suite_name: triton
framework: triton # matches the framework passed to triton-build

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The inline comment only restates the matching framework value and nearby build-job relationship; it does not record a lasting constraint beyond the configuration itself.

🤖 AI Fix

Remove the inline comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant