Skip to content

ci: activate dependency-aware package builds and tests - #2467

Open
kkraus14 wants to merge 4 commits into
NVIDIA:mainfrom
kkraus14:agent/selective-ci-orchestration
Open

ci: activate dependency-aware package builds and tests#2467
kkraus14 wants to merge 4 commits into
NVIDIA:mainfrom
kkraus14:agent/selective-ci-orchestration

Conversation

@kkraus14

@kkraus14 kkraus14 commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

Important

Stack completion: PR 4 of 4. #2464, #2465, and #2466 have merged. This is the final PR in the stack.

Merge order: #2464 (merged) -> #2465 (merged) -> #2466 (merged) -> #2467 (this PR). The branch lives in the kkraus14 fork and targets upstream main.

What

  • Compute one structured CI workplan with a small standard-library Python planner instead of maintaining duplicated positive/negative path filters and dependency expressions in workflow YAML.
  • Keep the planner focused on CI execution: it classifies merge-base...HEAD, applies the four-package build/test impact policy, and emits final module and job decisions as JSON.
  • Pass that single workplan through the wheel-build, sdist, and wheel-test reusable workflows. An omitted workplan preserves their existing full-build/full-test behavior for nightly callers.
  • Resolve a successful CI run for the PR's exact merge-base commit and reuse its complete wheel set. Missing, expired, or incomplete baseline artifacts conservatively force a full build and test.
  • Treat unknown paths and shared build/orchestration infrastructure as full-pipeline changes; treat package tests and shared wheel-test infrastructure as test-only.
  • Leave known Pixi manifests to the dedicated source-build workflow. New or unknown Pixi manifests still fail open to full CI.
  • Preserve deletion and cross-package rename handling with NUL-delimited git diff --no-renames.
  • Keep the universal Linux artifact path for docs and downstream jobs, while gating unnecessary platform, sdist, wheel-test, and cuda-core API jobs.
  • Keep the final status gate strict: intentional selective skips pass, while skips caused by upstream failures fail CI.

Expected package behavior

  • cuda_pathfinder source: build and test all four packages.
  • cuda_bindings source: build and test bindings, core, and cuda-python.
  • cuda_core source: build core; test core and cuda-python; run cuda-core API checks.
  • cuda_python source: build bindings and cuda-python because development cuda-python wheels exactly pin bindings; test cuda-python.
  • Package tests/examples: run that package's tests without rebuilding wheels.
  • Test-helper implementation: run bindings and core tests.
  • Package docs, known Pixi manifests, and independently validated tooling: no conditioned package work.
  • Unknown paths, shared build infrastructure, a non-PR event, or an unavailable baseline: run the full pipeline.

Validation

  • Added table-driven planner coverage for package impact, ignored paths, mixed changes, unknown-path fail-open behavior, and unavailable/incomplete baselines.
  • Planner unit tests pass under the repository Pixi environment.
  • Ruff check and format pass.
  • Full targeted pre-commit hooks pass, including SPDX, YAML, and GitHub Actions lint.
  • Independent planner, workflow, regression-scope, and maintainability reviews found no blocking issues.
  • The exact prior head completed all 102 CI jobs successfully; this commit triggers a fresh exact-head run.

Implements the remaining dependency-aware build/test selection requested in #299.

@kkraus14 kkraus14 added this to the cuda.bindings next milestone Jul 31, 2026
@kkraus14 kkraus14 added enhancement Any code-related improvements CI/CD CI/CD infrastructure labels Jul 31, 2026
@copy-pr-bot

copy-pr-bot Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@kkraus14
kkraus14 force-pushed the agent/selective-ci-orchestration branch 2 times, most recently from edfbf97 to 66445a4 Compare July 31, 2026 16:22
@github-actions github-actions Bot added the cuda.core Everything related to the cuda.core module label Jul 31, 2026
@kkraus14
kkraus14 force-pushed the agent/selective-ci-orchestration branch 5 times, most recently from a06ad8b to adf15bd Compare August 3, 2026 17:24
@kkraus14
kkraus14 force-pushed the agent/selective-ci-orchestration branch from adf15bd to ba702ca Compare August 13, 2026 19:36
@kkraus14 kkraus14 self-assigned this Aug 13, 2026
@kkraus14
kkraus14 force-pushed the agent/selective-ci-orchestration branch from ba702ca to 85f173e Compare August 13, 2026 20:30
@kkraus14
kkraus14 force-pushed the agent/selective-ci-orchestration branch from 85f173e to 9cf09a9 Compare August 14, 2026 01:38
@kkraus14
kkraus14 marked this pull request as ready for review August 14, 2026 02:54
@github-actions

Copy link
Copy Markdown

@mdboom mdboom 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.

Sorry this review will be long, and will negate some of the earlier decisions in the stack. I should have evaluated the whole stack at once to see where this is going.

I think this is fundamentally a maintenance nightmare and way more verbose than it needs to be. Rather than passing 9 new parameters between jobs, there should be a single, structured JSON object that is passed around. I am thinking something like (using YAML because it's easier to type, but it would be JSON).

modules:
  test_helpers:
    needs_build: false
    needs_test: false
  pathfinder:
    needs_build: false
    needs_test: false
    build_depends: []
    test_depends: ["test_helpers"]
  bindings:
    needs_build: false
    needs_test: false
    build_depends: ["pathfinder"]
    test_depends: ["test_helpers"]
  core:
    needs_build: false
    needs_test: false
    build_depends: ["bindings"]
    test_depends: ["test_helpers"]
baseline_run_id: 123456789
baseline_sha: c0ffee

This makes verbose things that are sensitive to modules coming and going like:

${{ needs.detect-changes.outputs.build_pathfinder == 'true' ||
                               needs.detect-changes.outputs.build_bindings == 'true' ||
                               needs.detect-changes.outputs.build_core == 'true' ||
                               needs.detect-changes.outputs.build_python == 'true' }}

into:

Object.values(data.modules).some(m => m.needs_test)

Rather than this:

      test_bindings: >-
        ${{ steps.baseline.outputs.available != 'true' ||
            steps.filter.outputs.changes == '' ||
            steps.filter.outputs.force_all == 'true' ||
            steps.filter.outputs.all_tests == 'true' ||
            steps.filter.outputs.pathfinder_source == 'true' ||
            steps.filter.outputs.bindings_source == 'true' ||
            steps.filter.outputs.bindings_tests == 'true' ||
            steps.filter.outputs.test_helpers == 'true' }}

You can do:

(name, modules, seen = new Set()) => {
  if (seen.has(name)) return false;
  seen.add(name);
  const m = modules[name];
  return m.needs_test || (m.depends || []).some(dep => moduleNeedsTest(dep, modules, seen));
};

rather than hardcoding the dependencies into a bunch of large boolean expressions.

We can pre-compute values for downstream consumers so this kind of logic is only written in one place.

data["needs_build"] = Object.values(data.modules).some(m => m.needs_build)

# or Python
data["needs_build"] = any(mod["needs_build"] for mod in data["modules"])

This object would get passed between jobs as "workplan" (or some better name).

Given my other comment that dorny/paths-filter seems like the wrong tool for the job, I would propose:

A standalone Python script that does the analysis of changes and builds this workplan object, including as many pre-computed dependendent information as we can to make downstream consumers of this information concise and prevent long complicated expressions being repeated everywhere.

This script has the side benefit of forming the basis of a future local helper to test only what is needed (which all of this GHA work doesn't move us toward).

(I confirmed that outputs can have a size maximum of 1MB which should be more than enough for this.)

Comment thread .github/workflows/ci.yml Outdated

has_match() {
grep -qE "$1" <<< "$changed" && echo true || echo false
- name: Classify changed paths

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.

The hardcoded path lists here is going to be a maintenance nightmare.

Each subsection follows a pretty similar pattern, though. If there were a better way of specifying things to reduce duplication, I think that would be fine. (For example, if we could globally exclude all AGENTS.md).

As it stands, if this is the best we can do with dorny/paths-filter, I think we should reach for a different tool or write our own in Python. Upside of that, too, is we could build a "test everything we need to" local script on top of it, whereas this is stuck in the GHA silo.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Originally my agent had implemented a Python script to encapsulate the logic of what paths / files impact which builds / tests and I found that script even more difficult to understand and maintain than maintaining a list of paths like this that an agent could easily update (and we should include in the AGENTS.md in hindsight). Let me see what I can do to simplify things here, because I agree this is going to be an ongoing annoyance.

For what it's worth, I think this overall work is going to be a stopgap as opposed to the end solution, where moving to something like moon (https://moonrepo.dev/moon) or bazel is probably a better longer term solution that makes things more "target" oriented as opposed to path / file oriented.

Comment thread .github/workflows/ci.yml Outdated
Comment on lines +266 to +267
# cuda_python/README.md is a symlink to this packaging input.
- 'README.md'

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.

Not sure I understand the comment.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

cuda_python/README.md symlinks the root README.md and it gets included in the Python package that we produce, so changes to the top level README need to trigger a rebuild of the metapackage.

@mdboom mdboom 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.

Some suggestions to simplify the logic.

I think the agent here is trying to use a scalpel, and thereby creating real pitfalls if assumptions that hold true today don't hold true in the future.

I think instead, we take an approach to be as simple as possible, err on the side of doing too much work, rather than missing things that legitimately should be tested.

Maybe some description similar to the above would cause the agent to come up with something similar/safer? If not, I think my concrete suggestions should also help.

Comment on lines +45 to +46
"pixi.lock",
"pixi.toml",

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.

We can probably safely put these in IGNORED_BASENAMES

".gitignore",
".pre-commit-config.yaml",
".spdx-ignore",
"CONTRIBUTING.md",

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.

We can probably safely ignore all .md files, especially if we handle files downstream of a directory called test specially, which it looks like we already do below.

"benchmarks/cuda_bindings/pixi.lock",
"benchmarks/cuda_bindings/pixi.toml",
"ci/.ci-pipeline-regen.md",
"ci/ci-pipeline.svg",

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.

We can probably safely ignore all .svg files.

Comment on lines +51 to +59
"ci/.ci-pipeline-regen.md",
"ci/ci-pipeline.svg",
"ci/cleanup-pr-previews",
"ci/tools/check_mempool_hygiene.py",
"ci/tools/check_pixi_cuda_version.py",
"ci/tools/check_release_notes.py",
"ci/tools/download-wheels",
"ci/tools/run_pytest_with_stack.py",
"ci/tools/validate-release-wheels",

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.

I think never ignoring anything in ci would be a safer (and simpler) choice.

"greptile.json",
"pixi.lock",
"pixi.toml",
"pytest.ini",

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.

This should not be on the ignore list -- this could have an appreciable impact on tests in the whole repo.

if not path_parts or path in IGNORED_PATHS or path_parts[-1] in IGNORED_BASENAMES:
continue

if path == "README.md":

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.

Can we just track symlinks generically (using Path.resolve or similar)?

continue

if path.startswith("cuda_python_test_helpers/cuda_python_test_helpers/"):
test_changes.update(("bindings", "core"))

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.

It doesn't today, but it wouldn't surprise me if cuda_pathfinder depended on test helpers someday. Probably safer to do this:

Suggested change
test_changes.update(("bindings", "core"))
test_changes.update(MODULES)

Comment on lines +75 to +85
ALL_TEST_PATHS = {
".github/workflows/test-wheel-linux.yml",
".github/workflows/test-wheel-windows.yml",
"ci/test-matrix.yml",
"ci/tools/configure_driver_mode.ps1",
"ci/tools/guess_latest.sh",
"ci/tools/install_gpu_driver.ps1",
"ci/tools/install_gpu_driver.sh",
"ci/tools/run-tests",
"ci/tools/setup-sanitizer",
}

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.

I think it's safer to trigger full tests if anything in ci or .github/{workflow|actions} changes.

"ci/tools/setup-sanitizer",
}

INDEPENDENT_GITHUB_PATHS = {

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.

As I said elsewhere, I think we can invert the logic and consider anything in workflows or actions to trigger a full test and everything else ignored.

if path.startswith("cuda_python_test_helpers/cuda_python_test_helpers/"):
test_changes.update(("bindings", "core"))
elif path.startswith("benchmarks/cuda_bindings/"):
test_changes.add("bindings")

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.

Ditto to above.

Suggested change
test_changes.add("bindings")
test_changes.add(MODULES)

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

Labels

CI/CD CI/CD infrastructure cuda.core Everything related to the cuda.core module enhancement Any code-related improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants