[mataylor-nvidia]

Description

Adds tools/package_impact_graph.py which walks every .py file under source/, parses runtime imports (excluding TYPE_CHECKING guards), and builds a reverse-dependency graph from that. Given a set of changed files it computes which CI test jobs are transitively affected and emits a manifest showing the import chain for each triggered job.

Integrates this into build.yaml via a new compute-affected-packages job that runs on ubuntu-latest in parallel with the Docker build. Each test job gains a contains(fromJSON(...)) guard so it is skipped when its package is not in the affected set. Non-Python file changes and any failure in the compute job both fail open (all jobs run).

6 seconds to scan 963 files across all packages - well within CI budget. The script stays as a tool, never committed as a JSON artifact. CI generates it fresh each run with python3 tools/package_impact_graph.py.

Fixes # (issue)

Type of change

• New feature (non-breaking change which adds functionality)

Screenshots

Please attach before and after screenshots of the change if applicable.

Checklist

• I have read and understood the contribution guidelines
• I have run the pre-commit checks with ./isaaclab.sh --format
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• I have updated the changelog and the corresponding version in the extension's config/extension.toml file
• I have added my name to the CONTRIBUTORS.md or my name already exists there

[greptile-apps]

Greptile Summary

This PR adds tools/package_impact_graph.py, which walks every .py file under source/, parses runtime imports (excluding TYPE_CHECKING guards via AST analysis), and builds a reverse-dependency graph to identify which CI test jobs are transitively affected by a set of changed files. The result is wired into build.yaml via a new compute-affected-packages job that gates each test job through a contains(fromJSON(...)) guard.

• The import-graph script is well-structured: it handles non-Python files and unrecognised paths by failing open, and the JSON manifest clearly explains each triggered job's dependency chain.
• The fail-open guarantee claimed in the PR description does not hold when compute-affected-packages actually errors: GitHub Actions auto-cancels dependent jobs before evaluating their if conditions unless always() is present, so a script crash silently skips all tests rather than running them all.
• Minor issues: _changed_files_since compares against the working tree instead of HEAD (safe in CI, misleading locally), an unused re import and _DEP_NAME_RE constant remain, and the TYPE_CHECKING exclusion incorrectly captures else-branch imports.

Confidence Score: 3/5

Merging as-is means a crash in the compute step silently cancels all GPU test jobs instead of running them all — the opposite of the stated design.

The core logic of the import graph and the CI guard expressions is sound, but the fail-open contract is inverted: when compute-affected-packages fails (script crash, git error, JSON parse failure), GitHub Actions cancels every dependent test job without ever evaluating the if conditions that were supposed to re-enable them.

.github/workflows/build.yaml — all test-job if blocks need always() && prepended to honour the fail-open contract.

Important Files Changed

Filename	Overview
.github/workflows/build.yaml	Adds compute-affected-packages job and gates all test jobs on its output, but the fail-open contract breaks when the compute job errors because test jobs lack always() in their if conditions.
tools/package_impact_graph.py	New script that walks source/ AST to build a reverse-dependency graph and emit a CI job manifest; mostly correct, but uses a working-tree diff instead of a commit-range diff and carries an unused re import and _DEP_NAME_RE constant.

_{Reviews (1): Last reviewed commit: "Add file-level import graph for per-PR C..." | Re-trigger Greptile}

[greptile-apps]

Fail-open broken when compute-affected-packages fails

In GitHub Actions, when a job listed in needs fails (not skipped — actually errors), GitHub automatically cancels dependent jobs before evaluating their if condition. The condition needs.compute-affected-packages.result != 'success' is never reached because the test job is pre-emptively skipped. The PR description promises fail-open behavior ("any failure in the compute job fails open"), but the current implementation delivers fail-closed: if the script crashes or git errors out, every test job is silently cancelled.

To honour the fail-open contract each test job's if needs to begin with always() && so GitHub evaluates the expression instead of auto-cancelling. The same fix applies to all other test jobs that share the same if pattern.

[greptile-apps]

git diff --name-only <ref> compares the working tree (including staged and unstaged changes) against ref, not two commits. In CI this produces the right result only because there are no local modifications; locally, any uncommitted edits are silently included, causing the impact graph to diverge from what was actually pushed. Using the two-argument form ref HEAD is explicit about comparing two commits.

Suggested change

	def _changed_files_since(ref: str) -> list[str]:
	result = subprocess.run(
	["git", "diff", "--name-only", ref],
	capture_output=True,
	text=True,
	check=True,
	)
	def _changed_files_since(ref: str) -> list[str]:
	result = subprocess.run(
	["git", "diff", "--name-only", ref, "HEAD"],
	capture_output=True,
	text=True,
	check=True,
	)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[greptile-apps]

_DEP_NAME_RE is defined but never referenced anywhere in the file, dragging import re along as an unused import.

Suggested change

	import re
	import subprocess
	import sys
	from collections import defaultdict
	from pathlib import Path
	# ---------------------------------------------------------------------------
	# Constants
	# ---------------------------------------------------------------------------
	_PREFIX = "isaaclab"
	_DEP_NAME_RE = re.compile(r"^([A-Za-z0-9_-]+)")
	import subprocess
	import sys
	from collections import defaultdict
	from pathlib import Path
	# ---------------------------------------------------------------------------
	# Constants
	# ---------------------------------------------------------------------------
	_PREFIX = "isaaclab"

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[greptile-apps]

else branch of if TYPE_CHECKING incorrectly excluded

ast.walk(node) starting from the ast.If node traverses the entire subtree — both the body and the orelse (the else branch). Any imports in an else clause of if TYPE_CHECKING: ... else: <runtime imports> are added to tc_ids and filtered out, so those runtime imports are silently missed. This pattern is uncommon but valid Python; the fix is to walk only node.body instead of the full If node.

[AntoineRichard]

🤖 AI-assisted review (batch triage of open CI/infra PRs)

Closing as stale: no activity since creation a month ago, and develop's workflows have restructured underneath it — the changes job output this gates on was renamed (run_docker_tests → should_run), so even a clean merge would leave the feature silently inert. Selective CI is still a worthwhile goal, but it would need re-authoring on top of the path-based detect-changes gate that landed in #6699. Also note the unaddressed fail-open inversion: without always() in the dependent jobs' if, a compute-job crash skips all GPU tests instead of running them.

Feel free to reopen if this was closed in error.