Skip to content

[ASM] [CI] Remove committed scratch files and stop two from coming back - #5544

Merged
Boss2002n merged 4 commits into
mainfrom
satya/cleanup_aiter_repo_unused_files
Oct 2, 2026
Merged

Boss2002n merged 4 commits into
mainfrom
satya/cleanup_aiter_repo_unused_files

Conversation

@Boss2002n

Copy link
Copy Markdown
Contributor

Six tracked files that nothing reads and nothing will read again.

CI scratch committed by the bot. update-split-tests.yaml writes split-tests-update-summary.md and then pr-body.md into the workspace and hands them to peter-evans/create-pull-request, which has no add-paths filter and so commits every dirty file it finds. Both landed in the repo root that way in #5131. The workflow regenerates them on every run, so the tracked copies are stale by definition; .gitignore now keeps the bot from re-adding them.

Merge and POC leftovers that ship in the wheel.
hsa/gfx950/pa/pa_bf16_noquant_gqa8_1tg_4w.co.orig and .co.poc_kl_merg arrived with #3211 next to the real .co. Nothing references either by name, and the ASM loaders build .co paths explicitly rather than globbing the directory, so neither is reachable. They are not merely inert: setup.py copies all of hsa/ into aiter_meta and MANIFEST.in grafts it, so 48 KB of scratch binaries go out in every wheel. *.orig is now ignored so the next conflicted merge does not leave another one behind.

One-off files their PRs forgot. probe_noqa_check.py is a single line, x = 1, added in #4403 to confirm the pinned ruff config flagged it. REPO_CLEANUP_PLAN.md is a pull request description from #2167 in March. Its Phase 1 shipped -- the .pt test data is gone from main and the size rules are in .gitignore -- and its Phase 2 history rewrite needs a coordinated force push that has not happened in the six months since. Neither file is referenced anywhere in the tree.

@Boss2002n
Boss2002n requested review from a team and a lite review from Copilot September 15, 2026 13:07
@github-actions github-actions Bot changed the title Remove committed scratch files and stop two from coming back [ASM] Remove committed scratch files and stop two from coming back Sep 15, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🏷️ CI Guide

Runs automatically on every PR:

  • ✅ Pre-checks (submodule verification, code formatting)
  • ✅ Aiter op tests (gfx942 + gfx950)
  • ✅ Triton tests on MI35X (only when aiter/ops/triton/** or related paths are changed)

Extended tests (opt-in via labels):

Label Tests
ci:gfx1250-ffm-triton Run the five-shard gfx1250 FFM Triton test suite
ci:triton-300x Run an additional Triton test job on MI300X in PRs; main branch always runs both MI35X and MI300X
multigpu Aiter multi-GPU tests on the 8-GPU runner
ci:sglang SGLang integration tests: DeepSeek-R1-MXFP4 accuracy, Qwen 3.5 accuracy
ci:atom ATOM benchmark: DeepSeek-R1-0528, GPT-OSS-120B
ci:atom_full ATOM accuracy suite for PR and main models from ATOM models_accuracy.json
ci:vllm vLLM benchmark: GPT-OSS-120B, DeepSeek-R1-0528, Kimi-K2.5
ci:all All standard extended tests (excludes ci:atom_full)

Only add ci:atom_full for FlyDSL or Triton upgrades.
Add labels via the sidebar or gh pr edit 5544 --add-label <label>

PR title tags & labels:
Component tags ([Triton/Gluon], [HIP], [CK], [ASM], ...) are added to the PR title and as PR labels automatically from the changed files and re-synced on every push — change-type tags like [fix]/[Perf], op tags like [MLA], and human labels (ci:*) are left untouched. Add the no-auto-title label to opt this PR out.

Six tracked files that nothing reads and nothing will read again, plus the
workflow fix that stops two of them returning.

CI scratch the bot committed. update-split-tests.yaml wrote
split-tests-update-summary.md and then pr-body.md into the workspace and
handed them to peter-evans/create-pull-request, which commits every new and
modified file when add-paths is unset. Both reached the repo root that way in
#5131, and the tracked copies were stale the moment they landed because the
workflow regenerates them on every run. The job only ever means to change one
file, so add-paths now names .github/scripts/split_tests.sh, and both scratch
files move to the runner temp directory so they are never dirty workspace
files in the first place. The .gitignore entries remain as a backstop for
local runs, where --summary-path still defaults to the working directory.

Merge and POC leftovers that ship in the wheel.
hsa/gfx950/pa/pa_bf16_noquant_gqa8_1tg_4w.co.orig and .co.poc_kl_merg
arrived with #3211 next to the real .co. Nothing references either by name,
and the ASM loaders build .co paths explicitly rather than globbing the
directory, so neither is reachable. They are not merely inert: setup.py
copies all of hsa/ into aiter_meta and MANIFEST.in grafts it, so 48 KB of
scratch binaries go out in every wheel. *.orig is now ignored so the next
conflicted merge does not leave another one behind.

One-off files their PRs forgot. probe_noqa_check.py is a single line, x = 1,
added in #4403 to confirm the pinned ruff config flagged it.
REPO_CLEANUP_PLAN.md is a pull request description from #2167 in March. Its
Phase 1 shipped -- the .pt test data is gone from main and the size rules are
in .gitignore -- and its Phase 2 history rewrite needs a coordinated force
push that has not happened in the six months since. Neither file is
referenced anywhere in the tree.

Left alone: scripts/download_test_data.sh, a no-op stub from that same plan,
because docs/tutorials/index.rst still points readers at it. Retiring the
script and the doc together is a separate change.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The only noted issue is a non-blocking documentation nit.

Pull request overview

Removes stale CI artifacts and unreferenced scratch files while preventing them from being recommitted.

Changes:

  • Deletes obsolete generated files, documentation, probe code, and binaries.
  • Adds .gitignore rules for CI outputs and merge backups.
File summaries
File Summary
split-tests-update-summary.md Removes stale CI output.
REPO_CLEANUP_PLAN.md Removes obsolete cleanup documentation.
probe_noqa_check.py Removes one-off lint probe.
pr-body.md Removes generated PR body.
.gitignore Adds ignore rules; nit: update the inaccurate add-paths comment or configure the workflow input.
Review details
  • Files reviewed: 4/7 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@Boss2002n Boss2002n self-assigned this Sep 15, 2026
Copilot AI review requested due to automatic review settings September 15, 2026 13:21
@Boss2002n
Boss2002n force-pushed the satya/cleanup_aiter_repo_unused_files branch from 6916bc8 to df00de2 Compare September 15, 2026 13:21
@github-actions github-actions Bot changed the title [ASM] Remove committed scratch files and stop two from coming back [ASM] [CI] Remove committed scratch files and stop two from coming back Sep 15, 2026
@github-actions github-actions Bot added the CI label Sep 15, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

No unresolved review issues were identified.

Review details
  • Files reviewed: 5/8 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI review requested due to automatic review settings September 15, 2026 17:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

No unresolved blocking issues were identified.

Review details
  • Files reviewed: 5/8 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@zufayu

zufayu commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

ROCm/aiter PR #5544 — [ASM] [CI] Remove committed scratch files and stop two from coming back

Deletes six unreferenced scratch files (two CI-generated markdown at the repo root, two leftover gfx950 code objects, a one-line ruff probe, a stale cleanup plan) and stops the CI bot from recommitting its scratch by redirecting workflow output to the runner temp dir and constraining what the bot may commit to split_tests.sh.

Review (advisory): ✅ NO FINDINGS
Validation (deterministic): NOT RUN — triage found no test target ("the PR changes runtime code but ships no test target"); the changed paths are deletions of files nothing loads at run time (neither hsa binary is in hsa/gfx950/pa/pa_asm.csv, the sole co-name source for asm_pa.cu, and nothing references probe_noqa_check.py), so this is the not-executed-at-run-time case — no test target could have covered it and the absence is not a defect
Perf (advisory): NOT RUN — no benchmark entry point (triage: no perf target for the same reason there is no test target); the deleted files sit outside the serving path, so there is no latency a harness could measure

Copilot AI review requested due to automatic review settings September 20, 2026 15:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

Reviewed changes are limited to stale artifacts and safeguards preventing their recreation.

Review effort: Lite
Findings: None

Two modify/delete conflicts, pr-body.md and split-tests-update-summary.md:
#5713 regenerated both in the repo root, which is the behaviour this branch
removes. Resolved as deletions. The same branch fixes the cause -- the
update-split-tests job now writes them under RUNNER_TEMP and passes add-paths
so create-pull-request commits only split_tests.sh -- so #5713 is the last run
that could have put them back.

#5713's refreshed FILE_TIMES table in split_tests.sh merged cleanly and is
kept; only the two scratch files it also committed are removed.
Copilot AI review requested due to automatic review settings September 21, 2026 18:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

No unresolved blocking issues were identified.

Review effort: Lite
Findings: None

@gyohuangxin gyohuangxin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the improvement. But could you explain the reason to delete REPO_CLEANUP_PLAN.md ?

@zufayu can you also check the deletion of probe_noqa_check.py and has/gfx950/pa/*** ?

@Boss2002n

Copy link
Copy Markdown
Contributor Author

Thanks for the improvement. But could you explain the reason to delete REPO_CLEANUP_PLAN.md ?

@zufayu can you also check the deletion of probe_noqa_check.py and has/gfx950/pa/*** ?

Its a PR description file - that PR has been merged - rendering this md file moot

@zufayu

zufayu commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

let me check We generally need to be very careful when deleting files.

@zufayu

zufayu commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

Thanks for the clarification. I checked the remaining deletions:

  • probe_noqa_check.py is an unreferenced one-line Ruff test artifact.
  • The .orig and .poc_kl_merg files under hsa/gfx950/pa/ are unreferenced leftovers; the actual .co used by pa_asm.csv remains.

Those deletions look safe.

REPO_CLEANUP_PLAN.md, however, describes a separate Phase 2 history rewrite that #2167 did not execute. I’m fine with removing it from the repository root, but please first move any still-relevant plan to a tracking issue and link it here—or confirm that the work has been completed or abandoned. After that, this looks good to merge.

@Boss2002n

Copy link
Copy Markdown
Contributor Author

Thanks for the clarification. I checked the remaining deletions:

  • probe_noqa_check.py is an unreferenced one-line Ruff test artifact.
  • The .orig and .poc_kl_merg files under hsa/gfx950/pa/ are unreferenced leftovers; the actual .co used by pa_asm.csv remains.

Those deletions look safe.

REPO_CLEANUP_PLAN.md, however, describes a separate Phase 2 history rewrite that #2167 did not execute. I’m fine with removing it from the repository root, but please first move any still-relevant plan to a tracking issue and link it here—or confirm that the work has been completed or abandoned. After that, this looks good to merge.

Repo size: Phase 2 of REPO_CLEANUP_PLAN.md, retargeted

Phase 1 (from #2167) shipped and is live. Phase 2 never ran — but measuring it first shows the plan aims at the wrong target.

Phase 1: done, in place on main

  |   -- | -- .gitignore lines 65–78 | *.pt, *.pth, *.ckpt, op_tests/test_jenga_vsa/, op_tests/dump_data/, OUT_FOLDER/, *.att .githooks/pre-commit, .githooks/install | present scripts/download_test_data.sh | present

 

A default git clone fetches every refs/heads/* but no refs/pull/*, so a blob reachable only from a deleted branch stops being downloaded the moment that branch is gone — no server-side GC needed for the clone-size win, and no force-push, no coordinated re-clone.

#<stale-branch-archive-PR> ("Archive stale branches instead of deleting them") is exactly this: archive at 30 days, notice at +30, delete the archive ref at +14, skipping protected branches, open-PR refs and a checked-in exemption list.

 

How to reproduce

 

bash

 

Shell
git clone --mirror https://github.com/ROCm/aiter.git m && du -sh mgit -C m rev-list --objects --all \  | git -C m cat-file --batch-check='%(objecttype) %(objectname) %(objectsize) %(rest)' \  | awk '$1=="blob" && $3>5000000 {printf "%6.1f MB  %s\n", $3/1048576, $4}' | sort -rn | head -15git -C m log --oneline main -- op_tests/dump_data/q.pt      # empty: not reachable from maingit clone --bare --single-branch --branch main https://github.com/ROCm/aiter.git s && du -sh s

 

Note: a --filter=blob:none clone reports these paths as absent from history. It is wrong; use a full mirror for this measurement

@Boss2002n

Copy link
Copy Markdown
Contributor Author

So if we ever want to address phase 2 of this plan then we can use the changes I've made in #5540 and #5617

@zufayu

zufayu commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

Thanks, this answers my question. Phase 1 is complete, and the original Phase 2 history rewrite was never run and is no longer the plan.

Please link the branch-archive tracking issue or PR once it is created, so that follow-up is not lost when REPO_CLEANUP_PLAN.md is removed. Other than that, the deletions look good to me.

@Boss2002n

Copy link
Copy Markdown
Contributor Author

Thanks, this answers my question. Phase 1 is complete, and the original Phase 2 history rewrite was never run and is no longer the plan.

Please link the branch-archive tracking issue or PR once it is created, so that follow-up is not lost when REPO_CLEANUP_PLAN.md is removed. Other than that, the deletions look good to me.

Done

@rahulbatra85

Copy link
Copy Markdown
Contributor

@Boss2002n Is this just a cleanup PR?

@Boss2002n

Copy link
Copy Markdown
Contributor Author

@Boss2002n Is this just a cleanup PR?

yes

@brunomazzottiamd brunomazzottiamd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@Boss2002n, I think we have the green light to merge from CI team, given all the PR discussions. Am I right?

@gyohuangxin / @zufayu can you please get back to this PR and double check if all discussions are addressed according to your requests? Thanks!

@leo-automation leo-automation left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@Boss2002n
Boss2002n merged commit 3276d64 into main Oct 2, 2026
57 checks passed
@Boss2002n
Boss2002n deleted the satya/cleanup_aiter_repo_unused_files branch October 2, 2026 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants