Repository navigation
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the
Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 Summary
Priority: ⬇️ Low Change: Bug fix · Severity of issue fixed: Low Merge Risk: 🔵 Low · up to
|
Jacobfaib
left a comment
There was a problem hiding this comment.
I'm not sure this is the right approach.
We already have separate knobs to enable building each optional project, steps.export-flags.outputs.matx_enabled, steps.export-flags.outputs.rapids_enabled, etc. We should hook into that instead of inventing a new bespoke path.
|
Updated in 1ddb3bd to feed change detection into the existing |
Preserve upstream cuda_coop_python and license routing while regenerating the fixture expectations for third-party consumers. Assisted-by: OpenAI Codex
|
Merged current Local validation passes all 27 change-detection fixtures, 297 actual flag/skip-tag scenarios, 27 controlled main-matrix comparisons, and scoped pre-commit checks including actionlint. The PR description has the current results and limits. The NVIDIA and actual third-party builds still need maintainer authorization for this new SHA through copy-pr-bot. Could a maintainer re-review the updated head and authorize CI when appropriate? AI assistance: this merge update and local validation were prepared with OpenAI Codex. |
|
/ok to test dbe9e3e |
This comment has been minimized.
This comment has been minimized.
|
The native NVIDIA CI run has completed successfully for @Jacobfaib, could you take another look now that native CI is green? The requested change-detection approach remains addressed: the existing FULL/LITE outputs drive the existing enable flags, with AI assistance: this status check and follow-up were prepared with OpenAI Codex. |
Jacobfaib
left a comment
There was a problem hiding this comment.
Small docs fixups, otherwise LGTM
Cover each public CMake entry path independently and keep internal-only coverage on an actual test file. Remove the unnecessary documentation and configuration commentary requested in review. Assisted-by: OpenAI Codex
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · important: End-anchor the third-party workflow… · project_files_and_dependencies.yaml:266
ci/project_files_and_dependencies.yaml:266
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick winimportant: End-anchor the third-party workflow selectors.
The three workflow regexes stop at
.ymlwithout$. A changed file such as.github/workflows/build-matx.yml.bakwill match the MatX selector and enable an unnecessary third-party build. Add$to the MatX, PyTorch, and RAPIDS workflow patterns to prevent false-positive CI jobs.Also applies to: 275-275, 284-284
Source: Path instructions
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository: NVIDIA/cccl/.coderabbit.yaml
- Review profile: CHILL
- Plan: Enterprise
- Run ID:
89cab3d0-1493-4b73-959b-624904a7a260
📒 Files selected for processing: 16
ci/project_files_and_dependencies.yamlci/test/inspect_changes/cmake_near_misses.dirty_filesci/test/inspect_changes/cmake_near_misses.outputci/test/inspect_changes/cub_cmake_add_subdir.dirty_filesci/test/inspect_changes/cub_cmake_add_subdir.outputci/test/inspect_changes/cub_cmake_lists.dirty_filesci/test/inspect_changes/cub_cmake_lists.outputci/test/inspect_changes/libcudacxx_cmake_add_subdir.dirty_filesci/test/inspect_changes/libcudacxx_cmake_add_subdir.outputci/test/inspect_changes/libcudacxx_cmake_lists.dirty_filesci/test/inspect_changes/libcudacxx_cmake_lists.outputci/test/inspect_changes/libcudacxx_internal_only.dirty_filesci/test/inspect_changes/thrust_cmake_add_subdir.dirty_filesci/test/inspect_changes/thrust_cmake_add_subdir.outputci/test/inspect_changes/thrust_cmake_lists.dirty_filesci/test/inspect_changes/thrust_cmake_lists.output
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.
|
/ok to test 72ca424 |
😬 CI Workflow Results🟥 Finished in 8h 30m: Pass: 99%/567 | Total: 6d 17h | Max: 6h 05m | Hits: 99%/897205See results here. AI failure analysis1. Windows cuda.compute py3.14t job cancelled after six hours · 1 jobExplanation: The workflow verifier failed because the CTK 12.9/MSVC/Python 3.14t cuda.compute job was cancelled after approximately six hours and never uploaded its success artifact. GitHub provided no dedicated log for the cancelled job, so the evidence does not identify which pytest phase or test hung; the PR changes CI selection logic rather than cuda.compute implementation or tests. Evidence: Copy this prompt into a coding agentJobs: |
Documentation and library-test-only changes currently launch the optional MatX, PyTorch, and RAPIDS builds. Add these consumers to the existing project graph and derive their existing enable flags from
FULL_BUILD/LITE_BUILD.Public header and downstream CMake entry-point changes select all three consumers. A consumer's own workflow or build scripts select that consumer; docs and other library-internal test changes skip these builds. Existing skip tags still apply.
inspect_changes.pyand the matrix builder are unchanged.Fixes #7290.
Local validation:
git diff --checkpass.The earlier successful native NVIDIA run covered
dbe9e3ec, before these review changes. The new head needs maintainer authorization for native CI; actual third-party builds were not run locally.AI assistance: this update and its local validation were prepared with OpenAI Codex.