Don't run routing tests if PR only touches LP/MIP files#935
Open
chris-maes wants to merge 2 commits intoNVIDIA:mainfrom
Open
Don't run routing tests if PR only touches LP/MIP files#935chris-maes wants to merge 2 commits intoNVIDIA:mainfrom
chris-maes wants to merge 2 commits intoNVIDIA:mainfrom
Conversation
📝 WalkthroughWalkthroughAdds a workflow grouping Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/pr.yaml:
- Around line 282-283: The YAML contains tab characters in the lines with the
patterns '!cpp/src/math_optimization/**' and '!cpp/src/pdlp/**' which breaks
parsing; open the workflow file and replace any tab characters surrounding those
entries with spaces so indentation uses only spaces, ensure list items remain
properly aligned (consistent 2 or 4 spaces as used elsewhere), and re-run a YAML
linter (yamllint) to confirm no tabs remain.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dbff7485-1478-4bd9-a371-30f94a012aa2
📒 Files selected for processing (7)
.github/workflows/pr.yamlci/run_ctests.shci/run_cuopt_pytests.shci/run_cuopt_server_pytests.shci/test_cpp.shci/test_python.shci/test_wheel_cuopt.sh
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/pr.yaml:
- Around line 280-291: The has_non_lp_mip_changes exclusion list is incomplete
and misses multiple LP/MIP-owned paths, causing irrelevant CI routing; update
the has_non_lp_mip_changes pattern set to also exclude the public headers and
other LP/MIP directories (examples: cpp/include/cuopt/linear_programming/**,
python/cuopt/cuopt/linear_programming/**, benchmarks/linear_programming/**,
datasets/linear_programming/** and datasets/mip/**,
python/cuopt_server/**/linear_programming/**, regression/lp_*.sh and
regression/mip_*.sh, plus docs/ and skills/examples paths that contain LP/MIP
examples) so that only truly non-LP/MIP changes flip has_non_lp_mip_changes;
locate the has_non_lp_mip_changes block in the workflow and add the
corresponding glob patterns (using the same negation style like '!cpp/src/...'
already present) to the list.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 30715bd2-7654-413c-a6c6-4ebfbc44ce55
📒 Files selected for processing (1)
.github/workflows/pr.yaml
Comment on lines
+280
to
+291
| has_non_lp_mip_changes: | ||
| - '**' | ||
| - '!cpp/src/math_optimization/**' | ||
| - '!cpp/src/pdlp/**' | ||
| - '!cpp/src/dual_simplex/**' | ||
| - '!cpp/src/cuts/**' | ||
| - '!cpp/src/barrier/**' | ||
| - '!cpp/src/mip_heuristics/**' | ||
| - '!cpp/src/branch_and_bound/**' | ||
| - '!cpp/tests/dual_simplex/**' | ||
| - '!cpp/tests/linear_programming/**' | ||
| - '!cpp/tests/mip/**' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python - <<'PY'
import pathlib
import re
workflow = pathlib.Path(".github/workflows/pr.yaml").read_text()
# Extract has_non_lp_mip_changes block
m = re.search(r'^\s*has_non_lp_mip_changes:\n((?:\s+- .+\n)+)', workflow, re.M)
if not m:
raise SystemExit("Could not find has_non_lp_mip_changes block in .github/workflows/pr.yaml")
block = m.group(1).splitlines()
excluded_prefixes = []
for line in block:
s = line.strip()
if s.startswith("- '!") and s.endswith("/**'"):
excluded_prefixes.append(s[len("- '!"):-len("/**'")])
# Candidate LP/MIP-related files in repo (heuristic by path tokens)
tokens = (
"mip", "lp", "pdlp", "dual_simplex", "linear_programming",
"math_optimization", "branch_and_bound", "barrier", "cuts"
)
candidates = []
for p in pathlib.Path(".").rglob("*"):
if not p.is_file():
continue
ps = p.as_posix()
if ".git/" in ps:
continue
low = ps.lower()
if any(t in low for t in tokens):
covered = any(ps.startswith(prefix + "/") for prefix in excluded_prefixes)
if not covered:
candidates.append(ps)
print("Excluded prefixes from has_non_lp_mip_changes:")
for ep in excluded_prefixes:
print(" -", ep)
print("\nPotential LP/MIP-related files NOT covered by exclusions:")
for c in sorted(candidates):
print(" -", c)
print(f"\nTotal uncovered candidates: {len(candidates)}")
PYRepository: NVIDIA/cuopt
Length of output: 16454
Verify has_non_lp_mip_changes covers all LP/MIP-owned paths.
The exclusion list is incomplete. Currently excluded are only cpp/src/ and cpp/tests/ directories, but significant LP/MIP-related files exist elsewhere:
cpp/include/cuopt/linear_programming/**(38 files) — public API headers for LP/MIP solverspython/cuopt/cuopt/linear_programming/**(20+ files) — Python LP modulebenchmarks/linear_programming/**— benchmark infrastructuredatasets/linear_programming/**anddatasets/mip/**— test datapython/cuopt_server/...linear_programming/**— server-side LP utilities and testsregression/lp_*.shandregression/mip_*.sh— regression test suites- Documentation and skill examples containing LP/MIP examples (50+ files)
PRs touching only these files will incorrectly be treated as has_non_lp_mip_changes=true, causing routing tests to run unnecessarily. Add these paths to the exclusion list or document the intended scope.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/pr.yaml around lines 280 - 291, The has_non_lp_mip_changes
exclusion list is incomplete and misses multiple LP/MIP-owned paths, causing
irrelevant CI routing; update the has_non_lp_mip_changes pattern set to also
exclude the public headers and other LP/MIP directories (examples:
cpp/include/cuopt/linear_programming/**,
python/cuopt/cuopt/linear_programming/**, benchmarks/linear_programming/**,
datasets/linear_programming/** and datasets/mip/**,
python/cuopt_server/**/linear_programming/**, regression/lp_*.sh and
regression/mip_*.sh, plus docs/ and skills/examples paths that contain LP/MIP
examples) so that only truly non-LP/MIP changes flip has_non_lp_mip_changes;
locate the has_non_lp_mip_changes block in the workflow and add the
corresponding glob patterns (using the same negation style like '!cpp/src/...'
already present) to the list.
|
🔔 Hi @anandhkb, this pull request has had no activity for 7 days. Please update or let us know if it can be closed. Thank you! If this is an "epic" issue, then please add the "epic" label to this issue. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Issue
Checklist