From 9c5e144ff7d0d45a9a99f643f795fc61b39233ba Mon Sep 17 00:00:00 2001 From: chaofengw Date: Sat, 12 Sep 2026 01:40:54 +0000 Subject: [PATCH] fix(ci): retain changed GPU families Preserve directly changed existing families when shared paths broaden impact to all. Add them to the shared smoke selection, and keep release performance policy changes from unnecessarily broadening GPU scope. This ensures mixed diffs validate the model family they actually modify. Signed-off-by: chaofengw --- .github/workflows/community-ci.yml | 31 +++++++++++++++++++++------ tools/community_gpu_ci.py | 15 ++++++++++--- tools/test_impact.py | 13 ++++++++--- tools/tests/test_community_ci.py | 25 ++++++++++++++++++---- tools/tests/test_community_gpu_ci.py | 32 +++++++++++++++++----------- tools/tests/test_family_impact.py | 29 +++++++++++++++++++++++++ 6 files changed, 117 insertions(+), 28 deletions(-) diff --git a/.github/workflows/community-ci.yml b/.github/workflows/community-ci.yml index a76710a642..92d8f54a62 100644 --- a/.github/workflows/community-ci.yml +++ b/.github/workflows/community-ci.yml @@ -300,6 +300,7 @@ jobs: base_sha: ${{ needs.authorize.outputs.base_sha }} merge_sha: ${{ needs.authorize.outputs.merge_sha }} families: ${{ steps.impact.outputs.families }} + direct_families: ${{ steps.impact.outputs.direct_families }} added_families: ${{ steps.impact.outputs.added_families }} scope: ${{ steps.impact.outputs.scope }} gpu_enabled: ${{ steps.impact.outputs.gpu_enabled }} @@ -381,6 +382,11 @@ jobs: + json.dumps(summary["families"], separators=(",", ":")), file=output, ) + print( + "direct_families=" + + json.dumps(summary["direct_families"], separators=(",", ":")), + file=output, + ) print( "added_families=" + json.dumps(sorted(added_families), separators=(",", ":")), @@ -403,6 +409,7 @@ jobs: - name: Validate the GPU selection env: FAMILIES: ${{ steps.impact.outputs.families }} + DIRECT_FAMILIES: ${{ steps.impact.outputs.direct_families }} ADDED_FAMILIES: ${{ steps.impact.outputs.added_families }} SCOPE: ${{ steps.impact.outputs.scope }} GPU_ENABLED: ${{ steps.impact.outputs.gpu_enabled }} @@ -416,6 +423,7 @@ jobs: pattern = re.compile(r"^[a-z][a-z0-9_]*$") families = json.loads(os.environ["FAMILIES"]) + direct_families = json.loads(os.environ["DIRECT_FAMILIES"]) added_families = json.loads(os.environ["ADDED_FAMILIES"]) scope = os.environ["SCOPE"] gpu_enabled = os.environ["GPU_ENABLED"] @@ -435,6 +443,14 @@ jobs: raise SystemExit("invalid family selection") if families != sorted(set(families)): raise SystemExit("family selection must be sorted and unique") + if not isinstance(direct_families, list) or not all( + isinstance(name, str) and pattern.fullmatch(name) for name in direct_families + ): + raise SystemExit("invalid direct-family selection") + if direct_families != sorted(set(direct_families)): + raise SystemExit("direct-family selection must be sorted and unique") + if not set(direct_families) <= set(families): + raise SystemExit("direct families must belong to the trusted family inventory") if not isinstance(added_families, list) or not all( isinstance(name, str) and pattern.fullmatch(name) for name in added_families ): @@ -445,9 +461,9 @@ jobs: raise SystemExit("added families must not be in the trusted family inventory") if scope != "all" and added_families: raise SystemExit("only all scope may select added families") - if scope == "families" and not families: - raise SystemExit("family scope requires at least one family") - if scope in {"docs", "none"} and families: + if scope == "families" and (not families or direct_families != families): + raise SystemExit("family scope requires its directly changed families") + if scope in {"docs", "none"} and (families or direct_families): raise SystemExit(f"{scope} scope must not select families") PY @@ -457,13 +473,14 @@ jobs: RUN_GPU: ${{ steps.impact.outputs.run_gpu }} SCOPE: ${{ steps.impact.outputs.scope }} FAMILIES: ${{ steps.impact.outputs.families }} + DIRECT_FAMILIES: ${{ steps.impact.outputs.direct_families }} run: | set -euo pipefail if [ "$GPU_ENABLED" = "false" ]; then message="Automatic Community GPU execution is disabled and is not a merge gate." echo "::notice::$message" - printf '## Community GPU\n\n%s\n\nImpact scope: `%s`; selected families: `%s`.\n' \ - "$message" "$SCOPE" "$FAMILIES" >> "$GITHUB_STEP_SUMMARY" + printf '## Community GPU\n\n%s\n\nImpact scope: `%s`; directly changed families: `%s`.\n' \ + "$message" "$SCOPE" "$DIRECT_FAMILIES" >> "$GITHUB_STEP_SUMMARY" exit 0 fi message="Experimental Community GPU smoke was manually enabled; it is not a merge gate." @@ -561,6 +578,7 @@ jobs: MERGE_SHA: ${{ needs.gpu-authorize.outputs.merge_sha }} PR_NUMBER: ${{ needs.gpu-authorize.outputs.pr_number }} FAMILIES: ${{ needs.gpu-authorize.outputs.families }} + DIRECT_FAMILIES: ${{ needs.gpu-authorize.outputs.direct_families }} ADDED_FAMILIES: ${{ needs.gpu-authorize.outputs.added_families }} SCOPE: ${{ needs.gpu-authorize.outputs.scope }} HF_TOKEN: ${{ secrets.HF_TOKEN }} @@ -582,7 +600,7 @@ jobs: # against huggingface_hub.get_token()); no code change is needed # in tools.community_gpu_ci to consume it. GitHub Actions masks # the secret value everywhere it appears, including here. - echo "Changed families: $FAMILIES; added families: $ADDED_FAMILIES (scope: $SCOPE)" + echo "Directly changed families: $DIRECT_FAMILIES; added families: $ADDED_FAMILIES (scope: $SCOPE)" python3 -m tools.brev_exec \ --instance "$INSTANCE_NAME" \ --log /tmp/gpu-ci-output.log \ @@ -591,6 +609,7 @@ jobs: -v /tmp/model_connect:/src -w /src \ -e "TRTMC_GPU_SCOPE=$SCOPE" \ -e "TRTMC_GPU_FAMILIES=$FAMILIES" \ + -e "TRTMC_GPU_DIRECT_FAMILIES=$DIRECT_FAMILIES" \ -e "TRTMC_GPU_ADDED_FAMILIES=$ADDED_FAMILIES" \ -e "HF_TOKEN=$HF_TOKEN" \ -e CMAKE_CUDA_ARCHITECTURES=89 \ diff --git a/tools/community_gpu_ci.py b/tools/community_gpu_ci.py index 591acb9d7f..bb5af3d26d 100644 --- a/tools/community_gpu_ci.py +++ b/tools/community_gpu_ci.py @@ -49,16 +49,24 @@ def _family_list(raw: str, label: str) -> tuple[str, ...]: return tuple(values) -def selected_families(scope: str, families: str, added_families: str) -> tuple[str, ...]: +def selected_families( + scope: str, + families: str, + direct_families: str, + added_families: str, +) -> tuple[str, ...]: """Resolve the family jobs without reading contributor-controlled shell text.""" selected = _family_list(families, "TRTMC_GPU_FAMILIES") + direct = _family_list(direct_families, "TRTMC_GPU_DIRECT_FAMILIES") added = _family_list(added_families, "TRTMC_GPU_ADDED_FAMILIES") if set(selected) & set(added): raise CiError("added families overlap the trusted family inventory") + if not set(direct) <= set(selected): + raise CiError("direct families must belong to the trusted family inventory") if scope == "all": - return tuple(sorted(set(SHARED_SMOKE_FAMILIES) | set(added))) + return tuple(sorted(set(SHARED_SMOKE_FAMILIES) | set(direct) | set(added))) if scope == "families": - if not selected or added: + if not selected or direct != selected or added: raise CiError("family scope requires existing families only") return selected raise CiError(f"GPU execution received non-GPU scope: {scope!r}") @@ -199,6 +207,7 @@ def run(repository: Path, env: dict[str, str]) -> None: selected = selected_families( env.get("TRTMC_GPU_SCOPE", ""), env.get("TRTMC_GPU_FAMILIES", ""), + env.get("TRTMC_GPU_DIRECT_FAMILIES", ""), env.get("TRTMC_GPU_ADDED_FAMILIES", ""), ) failures: list[tuple[str, str]] = [] diff --git a/tools/test_impact.py b/tools/test_impact.py index aebd0bdb22..456b9180a2 100644 --- a/tools/test_impact.py +++ b/tools/test_impact.py @@ -23,6 +23,9 @@ "CONTRIBUTING.md", "README.md", } +MODEL_PROOF_NEUTRAL_FILES = { + "apps/benchmark/performance/release.yaml", +} SHARED_PREFIXES = ( ".github/", "apps/", @@ -58,6 +61,7 @@ class Impact: scope: str families: tuple[str, ...] + direct_families: tuple[str, ...] changed_files: tuple[str, ...] run_core_tests: bool run_docs: bool @@ -107,6 +111,8 @@ def classify(repo: Path, files: Sequence[str]) -> Impact: if path in DOC_FILES or path.startswith(DOC_PREFIXES): docs = True continue + if path in MODEL_PROOF_NEUTRAL_FILES: + continue if len(parts) == 1 and path.endswith(".py"): shared = True continue @@ -120,11 +126,12 @@ def classify(repo: Path, files: Sequence[str]) -> Impact: if unknown: raise ValueError("unclassified changed paths: " + ", ".join(unknown)) + direct_families = tuple(sorted(selected)) if shared: - return Impact("all", tuple(sorted(known)), changed, True, docs) + return Impact("all", tuple(sorted(known)), direct_families, changed, True, docs) if selected: - return Impact("families", tuple(sorted(selected)), changed, True, docs) - return Impact("docs" if docs else "none", (), changed, False, docs) + return Impact("families", direct_families, direct_families, changed, True, docs) + return Impact("docs" if docs else "none", (), (), changed, False, docs) def changed_files(repo: Path, base: str, head: str) -> list[str]: diff --git a/tools/tests/test_community_ci.py b/tools/tests/test_community_ci.py index efaf2c0ae4..336a3eb5f9 100644 --- a/tools/tests/test_community_ci.py +++ b/tools/tests/test_community_ci.py @@ -120,6 +120,7 @@ def test_impact_publishes_only_the_public_cpu_scope( lambda *_args: community_ci.test_impact.Impact( scope="families", families=("qwen",), + direct_families=("qwen",), changed_files=("families/qwen/model.py",), run_core_tests=True, run_docs=False, @@ -283,6 +284,9 @@ def test_public_workflow_is_one_exact_merge_cpu_then_gpu_authorization() -> None assert gpu_authorize["outputs"]["added_families"] == ( "${{ steps.impact.outputs.added_families }}" ) + assert gpu_authorize["outputs"]["direct_families"] == ( + "${{ steps.impact.outputs.direct_families }}" + ) assert gpu_authorize["outputs"]["gpu_enabled"] == ("${{ steps.impact.outputs.gpu_enabled }}") assert gpu_authorize["outputs"]["run_gpu"] == "${{ steps.impact.outputs.run_gpu }}" gpu_authorize_steps = {step["name"]: step for step in gpu_authorize["steps"]} @@ -337,11 +341,15 @@ def test_public_workflow_is_one_exact_merge_cpu_then_gpu_authorization() -> None "persist-credentials": False, } assert gpu_test["env"]["MERGE_SHA"] == "${{ needs.gpu-authorize.outputs.merge_sha }}" + assert gpu_test["env"]["DIRECT_FAMILIES"] == ( + "${{ needs.gpu-authorize.outputs.direct_families }}" + ) assert "refs/pull/$PR_NUMBER/merge" in gpu_test["run"] assert r"\$(git rev-parse FETCH_HEAD)" in gpu_test["run"] assert '= $MERGE_SHA && git checkout --detach $MERGE_SHA"' in gpu_test["run"] assert "python3.12 -m tools.community_gpu_ci" in gpu_test["run"] assert "python3 -m tools.brev_exec" in gpu_test["run"] + assert "TRTMC_GPU_DIRECT_FAMILIES=$DIRECT_FAMILIES" in gpu_test["run"] assert "tests/e2e/models" not in gpu_test["run"] assert "py-only" not in gpu_test["run"] assert "python3.12 -m pytest" not in gpu_test["run"] @@ -731,11 +739,17 @@ def test_gpu_status_and_cleanup_fail_closed() -> None: @pytest.mark.parametrize( - ("changed_path", "expected_scope", "expected_families", "expected_added_families"), + ( + "changed_path", + "expected_scope", + "expected_families", + "expected_direct_families", + "expected_added_families", + ), [ - ("families/bert/model.py", "families", ["bert"], []), - ("families/new_family/model.py", "all", ["bert", "gpt2"], ["new_family"]), - ("README.md", "docs", [], []), + ("families/bert/model.py", "families", ["bert"], ["bert"], []), + ("families/new_family/model.py", "all", ["bert", "gpt2"], [], ["new_family"]), + ("README.md", "docs", [], [], []), ], ) def test_gpu_impact_executes_only_trusted_base_code( @@ -743,6 +757,7 @@ def test_gpu_impact_executes_only_trusted_base_code( changed_path: str, expected_scope: str, expected_families: list[str], + expected_direct_families: list[str], expected_added_families: list[str], ) -> None: repository = tmp_path / "repository" @@ -834,6 +849,8 @@ def git(*arguments: str) -> str: assert summary["scope"] == expected_scope assert summary["families"] == expected_families assert json.loads(values["families"]) == summary["families"] + assert summary["direct_families"] == expected_direct_families + assert json.loads(values["direct_families"]) == expected_direct_families assert json.loads(values["added_families"]) == expected_added_families assert values["scope"] == summary["scope"] assert values["gpu_enabled"] == "false" diff --git a/tools/tests/test_community_gpu_ci.py b/tools/tests/test_community_gpu_ci.py index 2dd5082984..17226c2fc6 100644 --- a/tools/tests/test_community_gpu_ci.py +++ b/tools/tests/test_community_gpu_ci.py @@ -28,36 +28,42 @@ def _family(repository: Path, name: str, manifests: list[dict[str, object]]) -> return root -def test_shared_gpu_plan_adds_new_families_without_replacing_smoke_coverage() -> None: - """Shared changes retain the fixed smoke set and include new family owners.""" +def test_shared_gpu_plan_includes_direct_and_new_families() -> None: + """Shared changes retain smoke coverage and every directly affected owner.""" selected = community_gpu_ci.selected_families( "all", - '["bert","gpt2"]', + '["bert","gpt2","llama"]', + '["llama"]', '["new_family"]', ) - assert selected == tuple(sorted((*community_gpu_ci.SHARED_SMOKE_FAMILIES, "new_family"))) + assert selected == tuple( + sorted((*community_gpu_ci.SHARED_SMOKE_FAMILIES, "llama", "new_family")) + ) @pytest.mark.parametrize( - ("scope", "families", "added"), + ("scope", "families", "direct", "added"), [ - ("docs", "[]", "[]"), - ("families", "[]", "[]"), - ("families", '["bert"]', '["new_family"]'), - ("all", '["bert"]', '["bert"]'), - ("all", '["not-valid"]', "[]"), - ("all", '["gpt2","bert"]', "[]"), + ("docs", "[]", "[]", "[]"), + ("families", "[]", "[]", "[]"), + ("families", '["bert"]', '["bert"]', '["new_family"]'), + ("families", '["bert"]', "[]", "[]"), + ("all", '["bert"]', '["bert"]', '["bert"]'), + ("all", '["bert"]', '["gpt2"]', "[]"), + ("all", '["not-valid"]', "[]", "[]"), + ("all", '["gpt2","bert"]', "[]", "[]"), ], ) def test_gpu_plan_rejects_malformed_or_inconsistent_selection( scope: str, families: str, + direct: str, added: str, ) -> None: """Selection crossing into the isolated runner stays fail closed.""" with pytest.raises(CiError): - community_gpu_ci.selected_families(scope, families, added) + community_gpu_ci.selected_families(scope, families, direct, added) def test_family_plan_selects_only_explicit_premerge_cases(tmp_path: Path) -> None: @@ -225,6 +231,7 @@ def _run(self, families: tuple[str, ...], testcases: tuple[str, ...]) -> None: { "TRTMC_GPU_SCOPE": "families", "TRTMC_GPU_FAMILIES": '["alpha"]', + "TRTMC_GPU_DIRECT_FAMILIES": '["alpha"]', "TRTMC_GPU_ADDED_FAMILIES": "[]", "TRTMC_NATIVE_BUILD_DIR": str(build), }, @@ -300,6 +307,7 @@ def _run(self, families: tuple[str, ...], _testcases: tuple[str, ...]) -> None: { "TRTMC_GPU_SCOPE": "families", "TRTMC_GPU_FAMILIES": '["alpha","beta"]', + "TRTMC_GPU_DIRECT_FAMILIES": '["alpha","beta"]', "TRTMC_GPU_ADDED_FAMILIES": "[]", "TRTMC_NATIVE_BUILD_DIR": str(build), }, diff --git a/tools/tests/test_family_impact.py b/tools/tests/test_family_impact.py index 8183df0da9..2f6b395527 100644 --- a/tools/tests/test_family_impact.py +++ b/tools/tests/test_family_impact.py @@ -44,6 +44,20 @@ def test_family_requirements_select_only_the_owner(tmp_path: Path) -> None: assert impact.families == ("alpha",) +def test_release_performance_policy_does_not_expand_family_scope(tmp_path: Path) -> None: + repo = _repo(tmp_path) + impact = test_impact.classify( + repo, + [ + "apps/benchmark/performance/release.yaml", + "families/alpha/model.py", + ], + ) + + assert impact.scope == "families" + assert impact.families == ("alpha",) + + def test_base_requirements_select_all_families(tmp_path: Path) -> None: repo = _repo(tmp_path) impact = test_impact.classify(repo, ["requirements/base.txt"]) @@ -58,6 +72,21 @@ def test_shared_contract_selects_all_directly(tmp_path: Path) -> None: assert impact.families == ("alpha", "beta") +def test_shared_change_preserves_directly_changed_family(tmp_path: Path) -> None: + repo = _repo(tmp_path) + impact = test_impact.classify( + repo, + [ + "core/runtime/include/trtmc/task.h", + "families/alpha/model.py", + ], + ) + + assert impact.scope == "all" + assert impact.families == ("alpha", "beta") + assert impact.direct_families == ("alpha",) + + def test_families_package_selects_all_directly(tmp_path: Path) -> None: repo = _repo(tmp_path) impact = test_impact.classify(repo, ["families/__init__.py"])