Skip to content

Rapidsmpf logger API changes - #22972

Merged
rapids-bot[bot] merged 5 commits into
NVIDIA:mainfrom
nirandaperera:rapidsmpf-logger-api-changes
Jul 7, 2026
Merged

Rapidsmpf logger API changes#22972
rapids-bot[bot] merged 5 commits into
NVIDIA:mainfrom
nirandaperera:rapidsmpf-logger-api-changes

Conversation

@nirandaperera

Copy link
Copy Markdown
Contributor

Description

Depends on rapidsai/rapidsmpf#1081

Adds new API changes to cudf polars and cudf streaming

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@nirandaperera
nirandaperera requested a review from a team as a code owner June 24, 2026 21:57
@github-actions github-actions Bot added the libcudf Affects libcudf (C++/CUDA) code. label Jun 24, 2026
@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

CI scripts across cudf's build and test pipeline now source two new helper scripts, ci/use_conda_packages_from_prs.sh and ci/use_wheels_from_prs.sh, which fetch PR-specific rapidsmpf conda channels or wheel artifacts and inject them into dependency generation and pip constraints. SPDX copyright headers are also updated to include "& AFFILIATES".

Changes

PR artifact CI helper sourcing

Layer / File(s) Summary
Conda helper script
ci/use_conda_packages_from_prs.sh
New script downloads PR conda artifact channels for rapidsmpf, adds them to conda system config, and exports RAPIDS_PREPENDED_CONDA_CHANNELS/RAPIDS_PREPENDED_CHANNEL_ARGS.
Wheel helper script
ci/use_wheels_from_prs.sh
New script computes CUDA suffix, downloads rapidsmpf PR wheel artifacts, and appends file:// glob constraints to PIP_CONSTRAINT.
CI jobs source conda helper
ci/build_cpp.sh, ci/build_docs.sh, ci/build_python.sh, ci/build_python_noarch.sh, ci/cudf_pandas_scripts/third-party-integration/test.sh, ci/test_cpp_common.sh, ci/test_java.sh, ci/test_notebooks.sh, ci/test_python_common.sh
Scripts source the conda helper and pass RAPIDS_PREPENDED_CHANNEL_ARGS into rapids-dependency-file-generator/rattler channel setup; SPDX headers updated.
CI jobs source wheel helper
ci/build_wheel_cudf_streaming.sh, ci/build_wheel_libcudf_streaming.sh, ci/test_cudf_polars_polars_tests.sh, ci/test_wheel_cudf_polars.sh, ci/test_wheel_cudf_streaming.sh
Scripts source the wheel helper after pip constraint generation to control wheels installed in subsequent steps; SPDX headers updated.

Estimated code review effort: 2 (Simple) | ~15 minutes

Possibly related PRs

  • rapidsai/cudf#22189: Both PRs modify ci/test_wheel_cudf_polars.sh in the same dependency/install setup path.
  • rapidsai/cudf#22480: Both PRs touch ci/test_wheel_cudf_polars.sh to change how the cudf_polars wheel/dependencies are prepared/installed.

Suggested labels: `Python`, `cudf-polars`

Suggested reviewers: `vyasr`, `jameslamb`, `bdice`, `pentschev`

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title matches the PR’s main intent around rapidsmpf logger API changes, even though the diff itself is mostly CI support.
Description check ✅ Passed The description is clearly related to the changeset and the linked rapidsmpf dependency.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@nirandaperera nirandaperera added breaking Breaking change improvement Improvement / enhancement to an existing function labels Jul 1, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
cpp/libcudf_streaming/benchmarks/streaming/bench_streaming_shuffle.cpp (1)

368-368: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Inconsistent logger source for Context construction.

ctx is still built with comm->logger() here, while the equivalent site in ndsh/utils.cpp (line 244) was updated to use the newly-created local log directly. Assuming comm->logger() now simply returns the same log passed into the communicator constructor, this is functionally fine, but it's inconsistent with the sibling file in this same cohort and relies on that equivalence holding for all communicator types (MPI/UCXX bootstrap paths).

♻️ Suggested fix for consistency
-  auto ctx = std::make_shared<rapidsmpf::streaming::Context>(options, comm->logger(), br);
+  auto ctx = std::make_shared<rapidsmpf::streaming::Context>(options, log, br);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpp/libcudf_streaming/benchmarks/streaming/bench_streaming_shuffle.cpp` at
line 368, The `Context` construction here is still pulling the logger from
`comm->logger()`, which is inconsistent with the updated pattern used in
`ndsh/utils.cpp`. Update the `ctx` initialization in
`bench_streaming_shuffle.cpp` to pass the local `log` directly into
`rapidsmpf::streaming::Context`, matching the sibling benchmark code and
avoiding reliance on `Communicator::logger()` equivalence across bootstrap
paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@cpp/libcudf_streaming/benchmarks/streaming/bench_streaming_shuffle.cpp`:
- Line 368: The `Context` construction here is still pulling the logger from
`comm->logger()`, which is inconsistent with the updated pattern used in
`ndsh/utils.cpp`. Update the `ctx` initialization in
`bench_streaming_shuffle.cpp` to pass the local `log` directly into
`rapidsmpf::streaming::Context`, matching the sibling benchmark code and
avoiding reliance on `Communicator::logger()` equivalence across bootstrap
paths.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 12ceb7c1-0951-447f-bb5e-e8067a153899

📥 Commits

Reviewing files that changed from the base of the PR and between 76318f8 and 60fbe7f.

📒 Files selected for processing (7)
  • cpp/libcudf_streaming/benchmarks/bench_shuffle.cpp
  • cpp/libcudf_streaming/benchmarks/streaming/bench_streaming_shuffle.cpp
  • cpp/libcudf_streaming/benchmarks/streaming/ndsh/utils.cpp
  • cpp/libcudf_streaming/examples/example_shuffle.cpp
  • cpp/libcudf_streaming/tests/main/mpi.cpp
  • cpp/libcudf_streaming/tests/main/single.cpp
  • cpp/libcudf_streaming/tests/main/ucxx.cpp

@nirandaperera
nirandaperera requested a review from a team as a code owner July 2, 2026 00:25
@nirandaperera
nirandaperera requested a review from jameslamb July 2, 2026 00:25

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (1)
ci/use_wheels_from_prs.sh (1)

18-21: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Harden the wheel-glob expansion before writing constraints.

$(echo "${LIBRAPIDSMPF_WHEELHOUSE}"/librapidsmpf_*.whl) (and the rapidsmpf line) will silently produce a broken constraint line if the glob matches zero files (literal * left in the URL) or more than one file (space-joined paths concatenated into a single requirement line). Since this heredoc write isn't validated, the failure only surfaces later as a confusing pip install error rather than a clear message at download time.

Consider validating the match count before writing, e.g. with shopt -s nullglob failglob or an explicit array/count check, to fail fast with a clear message.

🛠️ Example hardening
+shopt -s failglob
+
 cat >> "${PIP_CONSTRAINT}" <<EOF
 librapidsmpf-${RAPIDS_PY_CUDA_SUFFIX} @ file://$(echo "${LIBRAPIDSMPF_WHEELHOUSE}"/librapidsmpf_*.whl)
 rapidsmpf-${RAPIDS_PY_CUDA_SUFFIX} @ file://$(echo "${RAPIDSMPF_WHEELHOUSE}"/rapidsmpf_*.whl)
 EOF
+
+shopt -u failglob
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ci/use_wheels_from_prs.sh` around lines 18 - 21, The wheel constraint entries
in use_wheels_from_prs.sh are built from unchecked glob expansion, so a missing
or multiple wheel match can write an invalid requirement into PIP_CONSTRAINT.
Update the logic around the librapidsmpf and rapidsmpf wheel lookups to validate
that exactly one wheel is found before the heredoc is appended, and fail fast
with a clear error if the match count is zero or greater than one. Use the
existing LIBRAPIDSMPF_WHEELHOUSE and RAPIDSMPF_WHEELHOUSE variables in the check
so the constraint file is only written with a single resolved wheel path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@ci/build_python_noarch.sh`:
- Around line 24-26: The PR-only helper is being sourced unconditionally in
build_python_noarch.sh, which causes branch builds to try fetching PR artifacts.
Guard the source of use_conda_packages_from_prs.sh behind the same pull-request
check used elsewhere, or relocate that call to PR-only paths so build_type:
branch runs do not hit the hardcoded rapidsmpf#1081 packages.

In `@ci/test_cpp_common.sh`:
- Around line 12-14: Guard the PR-only helper in test_cpp_common.sh so it is
only sourced for pull-request builds. The unconditional source of
use_conda_packages_from_prs.sh affects the shared setup used by test_cpp.sh,
test_cpp_memcheck.sh, and test_cpp_benchmarks.sh, so add a build_type-based
check around that source call and keep non-PR jobs on the normal conda packages.
Use the existing build_type flow in test_cpp_common.sh to distinguish
pull-request from branch/nightly runs.

In `@ci/test_java.sh`:
- Around line 12-14: The PR-only conda bootstrap in test_java.sh is being
sourced unconditionally, which can break branch and nightly runs because it
depends on PR artifacts. Add the same RAPIDS_BUILD_TYPE pull-request guard used
in ci/cudf_pandas_scripts/third-party-integration/test.sh around the source of
ci/use_conda_packages_from_prs.sh so it only runs for PR builds.

In `@ci/test_notebooks.sh`:
- Around line 12-14: The notebook test script unconditionally sources the
PR-only package helper, which breaks non-PR runs like nightly and branch builds.
Update ci/test_notebooks.sh to guard the source of
use_conda_packages_from_prs.sh behind a check that RAPIDS_BUILD_TYPE is
"pull-request", matching the pattern used by the third-party integration test,
so the helper only runs for PR builds.

---

Nitpick comments:
In `@ci/use_wheels_from_prs.sh`:
- Around line 18-21: The wheel constraint entries in use_wheels_from_prs.sh are
built from unchecked glob expansion, so a missing or multiple wheel match can
write an invalid requirement into PIP_CONSTRAINT. Update the logic around the
librapidsmpf and rapidsmpf wheel lookups to validate that exactly one wheel is
found before the heredoc is appended, and fail fast with a clear error if the
match count is zero or greater than one. Use the existing
LIBRAPIDSMPF_WHEELHOUSE and RAPIDSMPF_WHEELHOUSE variables in the check so the
constraint file is only written with a single resolved wheel path.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 482caa11-9b98-4bbc-80b6-eb209c44f6e8

📥 Commits

Reviewing files that changed from the base of the PR and between 60fbe7f and bebb8da.

📒 Files selected for processing (16)
  • ci/build_cpp.sh
  • ci/build_docs.sh
  • ci/build_python.sh
  • ci/build_python_noarch.sh
  • ci/build_wheel_cudf_streaming.sh
  • ci/build_wheel_libcudf_streaming.sh
  • ci/cudf_pandas_scripts/third-party-integration/test.sh
  • ci/test_cpp_common.sh
  • ci/test_cudf_polars_polars_tests.sh
  • ci/test_java.sh
  • ci/test_notebooks.sh
  • ci/test_python_common.sh
  • ci/test_wheel_cudf_polars.sh
  • ci/test_wheel_cudf_streaming.sh
  • ci/use_conda_packages_from_prs.sh
  • ci/use_wheels_from_prs.sh

Comment thread ci/build_python_noarch.sh Outdated
Comment thread ci/test_cpp_common.sh Outdated
Comment thread ci/test_java.sh Outdated
Comment on lines +12 to +14
# TODO: Remove before merging. Use rapidsmpf conda packages from rapidsai/rapidsmpf#1081.
source ./ci/use_conda_packages_from_prs.sh

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== ci/test_java.sh ==\n'
cat -n ci/test_java.sh | sed -n '1,120p'

printf '\n== ci/cudf_pandas_scripts/third-party-integration/test.sh ==\n'
cat -n ci/cudf_pandas_scripts/third-party-integration/test.sh | sed -n '1,120p'

printf '\n== references to use_conda_packages_from_prs.sh ==\n'
rg -n "use_conda_packages_from_prs\.sh|RAPIDS_BUILD_TYPE" ci -g '!**/build/**'

Repository: rapidsai/cudf

Length of output: 7947


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== ci/use_conda_packages_from_prs.sh ==\n'
cat -n ci/use_conda_packages_from_prs.sh | sed -n '1,220p'

printf '\n== java-related CI references ==\n'
rg -n "test_java\.sh|java test|RAPIDS_BUILD_TYPE|pull-request" ci .github -g '!**/build/**'

Repository: rapidsai/cudf

Length of output: 9454


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== .github/workflows/test.yaml around ci/test_java.sh ==\n'
sed -n '130,175p' .github/workflows/test.yaml | cat -n

printf '\n== .github/workflows/pr.yaml around ci/test_java.sh ==\n'
sed -n '470,510p' .github/workflows/pr.yaml | cat -n

Repository: rapidsai/cudf

Length of output: 4034


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== .github/workflows/test.yaml top ==\n'
sed -n '1,90p' .github/workflows/test.yaml | cat -n

printf '\n== invocations of test.yaml and build_type assignments ==\n'
rg -n "build_type: (branch|nightly|pull-request)|uses: .*\/\.github\/workflows\/test\.yaml@|uses: .*test\.yaml" .github/workflows -g '!**/build/**'

Repository: rapidsai/cudf

Length of output: 3749


Guard the PR-only conda bootstrap in ci/test_java.shtest.yaml runs this job for branch and nightly too, but ci/use_conda_packages_from_prs.sh always calls rapids-get-pr-artifact, so non-PR Java runs can fail here. Add the same RAPIDS_BUILD_TYPE == pull-request guard used in ci/cudf_pandas_scripts/third-party-integration/test.sh.

🧰 Tools
🪛 Shellcheck (0.11.0)

[info] 13-13: Not following: ./ci/use_conda_packages_from_prs.sh was not specified as input (see shellcheck -x).

(SC1091)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ci/test_java.sh` around lines 12 - 14, The PR-only conda bootstrap in
test_java.sh is being sourced unconditionally, which can break branch and
nightly runs because it depends on PR artifacts. Add the same RAPIDS_BUILD_TYPE
pull-request guard used in
ci/cudf_pandas_scripts/third-party-integration/test.sh around the source of
ci/use_conda_packages_from_prs.sh so it only runs for PR builds.

Comment thread ci/test_notebooks.sh Outdated
Comment on lines +12 to +14
# TODO: Remove before merging. Use rapidsmpf conda packages from rapidsai/rapidsmpf#1081.
source ./ci/use_conda_packages_from_prs.sh

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n## ci/test_notebooks.sh\n'
cat -n ci/test_notebooks.sh | sed -n '1,120p'

printf '\n## Search RAPIDS_BUILD_TYPE and use_conda_packages_from_prs.sh\n'
rg -n "RAPIDS_BUILD_TYPE|use_conda_packages_from_prs\.sh" ci -g '*.sh' -g '*.bash'

printf '\n## Related integration test script\n'
if [ -f ci/cudf_pandas_scripts/third-party-integration/test.sh ]; then
  cat -n ci/cudf_pandas_scripts/third-party-integration/test.sh | sed -n '1,220p'
fi

Repository: rapidsai/cudf

Length of output: 9383


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n## ci/use_conda_packages_from_prs.sh\n'
cat -n ci/use_conda_packages_from_prs.sh | sed -n '1,220p'

printf '\n## Search workflow/job references to ci/test_notebooks.sh\n'
rg -n "test_notebooks\.sh|test_notebooks" .github ci -g '*.yml' -g '*.yaml' -g '*.sh'

printf '\n## Search for RAPIDS_BUILD_TYPE setup in notebook-related CI\n'
rg -n "RAPIDS_BUILD_TYPE" .github ci -g '*.yml' -g '*.yaml' -g '*.sh'

Repository: rapidsai/cudf

Length of output: 3483


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n## .github/workflows/test.yaml relevant section\n'
sed -n '130,210p' .github/workflows/test.yaml

printf '\n## .github/workflows/pr.yaml relevant section around test_notebooks\n'
sed -n '200,240p' .github/workflows/pr.yaml
printf '\n## .github/workflows/pr.yaml relevant job section around test_notebooks script\n'
sed -n '490,525p' .github/workflows/pr.yaml

Repository: rapidsai/cudf

Length of output: 5731


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n## top of .github/workflows/test.yaml\n'
sed -n '1,120p' .github/workflows/test.yaml

printf '\n## references to conda-notebook-tests\n'
rg -n "conda-notebook-tests|build_type:" .github/workflows -g '*.yml' -g '*.yaml'

printf '\n## search for workflow_call / workflow_dispatch in test.yaml\n'
rg -n "workflow_call|workflow_dispatch|schedule|push:" .github/workflows/test.yaml

Repository: rapidsai/cudf

Length of output: 9082


Guard PR-only conda package sourcing in notebook tests. ci/test_notebooks.sh always sources ci/use_conda_packages_from_prs.sh, but .github/workflows/test.yaml runs this job with build_type values like nightly and branch too. That helper is PR-specific and will try to pull PR artifacts in non-PR runs, so gate it on RAPIDS_BUILD_TYPE == "pull-request" like the third-party integration test does.

🧰 Tools
🪛 Shellcheck (0.11.0)

[info] 13-13: Not following: ./ci/use_conda_packages_from_prs.sh was not specified as input (see shellcheck -x).

(SC1091)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ci/test_notebooks.sh` around lines 12 - 14, The notebook test script
unconditionally sources the PR-only package helper, which breaks non-PR runs
like nightly and branch builds. Update ci/test_notebooks.sh to guard the source
of use_conda_packages_from_prs.sh behind a check that RAPIDS_BUILD_TYPE is
"pull-request", matching the pattern used by the third-party integration test,
so the helper only runs for PR builds.

@nirandaperera
nirandaperera force-pushed the rapidsmpf-logger-api-changes branch from bebb8da to da5c894 Compare July 2, 2026 15:35

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 9

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@ci/build_cpp.sh`:
- Around line 25-27: The unconditional sourcing of
ci/use_conda_packages_from_prs.sh in build_cpp.sh is PR-only and can break
branch/nightly jobs. Update the script so this helper is only sourced when the
build type is pull-request, using the same guard pattern as
ci/cudf_pandas_scripts/third-party-integration/test.sh (or remove the TODO
helper entirely before merge). Keep the change localized around the existing
source call so non-PR paths skip the rapidsmpf#1081 dependency.

In `@ci/build_docs.sh`:
- Around line 18-20: The unconditional source call in the build_docs script will
try to fetch PR artifacts even on branch/nightly runs. Update the logic around
the ci/use_conda_packages_from_prs.sh sourcing to match the guarded pattern used
in ci/build_python_noarch.sh, using the same job/PR condition check so it only
runs for PR builds and skips artifact fetching otherwise.

In `@ci/build_python.sh`:
- Around line 24-26: The build_python.sh setup unconditionally sources
ci/use_conda_packages_from_prs.sh, which will break non-PR runs by trying to
fetch PR-only artifacts. Guard this logic the same way as the other build
scripts by checking the PR context before sourcing, and keep the existing
artifact helper invocation under that condition so branch/nightly jobs skip it.
Use the ci/use_conda_packages_from_prs.sh call site in the build_python.sh flow
as the target for the fix.

In `@ci/build_wheel_cudf_streaming.sh`:
- Around line 24-26: Gate the PR-only wheel sourcing in
build_wheel_cudf_streaming.sh behind the same RAPIDS_BUILD_TYPE ==
"pull-request" check used elsewhere, since source ./ci/use_wheels_from_prs.sh
only works with PR artifacts. Update the script around the
use_wheels_from_prs.sh import so branch and nightly builds skip it and only
pull-request builds execute it.

In `@ci/build_wheel_libcudf_streaming.sh`:
- Around line 20-22: The unconditional source of ci/use_wheels_from_prs.sh in
build_wheel_libcudf_streaming.sh will break non-PR wheel builds because it
assumes PR artifact 1081 exists. Update the script to only source
ci/use_wheels_from_prs.sh when running in PR builds, and leave branch/nightly
builds to use the normal wheel sources; use the build_wheel_libcudf_streaming.sh
entrypoint and its existing PR context checks to gate this behavior.

In `@ci/test_cudf_polars_polars_tests.sh`:
- Around line 21-23: The PR-only wheel sourcing in
ci/test_cudf_polars_polars_tests.sh is unconditional, so branch and nightly runs
can still try to fetch PR artifacts. Update the script around the source
./ci/use_wheels_from_prs.sh call to only run when the build type indicates a
pull request, using the existing build_type handling or equivalent check in this
script. Keep the PR-wheel setup isolated to the same conditional path that
already distinguishes build_type values like branch and nightly.

In `@ci/test_python_common.sh`:
- Around line 14-16: The shared Python test setup currently sources
ci/use_conda_packages_from_prs.sh unconditionally, which affects every job that
uses this script. Update the shared setup in test_python_common.sh to only
source that helper when running PR-based jobs, using the existing job/context
detection logic already available in the Python test flow. Keep the change
localized around the sourcing step so branch and nightly jobs skip the
PR-artifact fetch while PR jobs still use it.

In `@ci/test_wheel_cudf_polars.sh`:
- Around line 23-25: The wheel-fetch setup in the test script is unguarded and
always sources ci/use_wheels_from_prs.sh, which will try to pull PR-only
artifacts on non-PR runs. Update the logic around the ci/use_wheels_from_prs.sh
invocation to only run when the job is executing for a PR, using the same gating
approach as other PR-only wheel helpers in this stack, so branch and nightly
runs skip the rapidsmpf#1081 fetch path.

In `@ci/test_wheel_cudf_streaming.sh`:
- Around line 20-22: The unguarded source of use_wheels_from_prs.sh in the
streaming wheel test script can break non-PR branch/nightly runs by trying to
fetch rapidsmpf#1081 artifacts that do not exist there. Update the test setup so
the wheel fetch in test_wheel_cudf_streaming.sh is only invoked for PR builds,
following the same guard pattern used in the related test_wheel_cudf_polars.sh
flow, and keep the conditional centered around the use_wheels_from_prs.sh call.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 846b66f4-b876-4519-a8bc-263b06454663

📥 Commits

Reviewing files that changed from the base of the PR and between bebb8da and da5c894.

📒 Files selected for processing (16)
  • ci/build_cpp.sh
  • ci/build_docs.sh
  • ci/build_python.sh
  • ci/build_python_noarch.sh
  • ci/build_wheel_cudf_streaming.sh
  • ci/build_wheel_libcudf_streaming.sh
  • ci/cudf_pandas_scripts/third-party-integration/test.sh
  • ci/test_cpp_common.sh
  • ci/test_cudf_polars_polars_tests.sh
  • ci/test_java.sh
  • ci/test_notebooks.sh
  • ci/test_python_common.sh
  • ci/test_wheel_cudf_polars.sh
  • ci/test_wheel_cudf_streaming.sh
  • ci/use_conda_packages_from_prs.sh
  • ci/use_wheels_from_prs.sh
🚧 Files skipped from review as they are similar to previous changes (2)
  • ci/use_wheels_from_prs.sh
  • ci/use_conda_packages_from_prs.sh

Comment thread ci/build_cpp.sh Outdated
Comment thread ci/build_docs.sh Outdated
Comment on lines +18 to +20
# TODO: Remove before merging. Use rapidsmpf conda packages from rapidsai/rapidsmpf#1081.
source ./ci/use_conda_packages_from_prs.sh

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Same unguarded PR-artifact fetch as other build scripts.

Same concern as flagged in ci/build_python_noarch.sh: sourcing this unconditionally will attempt to fetch rapidsmpf#1081 artifacts even on non-PR (branch/nightly) runs of this job.

🧰 Tools
🪛 Shellcheck (0.11.0)

[info] 19-19: Not following: ./ci/use_conda_packages_from_prs.sh was not specified as input (see shellcheck -x).

(SC1091)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ci/build_docs.sh` around lines 18 - 20, The unconditional source call in the
build_docs script will try to fetch PR artifacts even on branch/nightly runs.
Update the logic around the ci/use_conda_packages_from_prs.sh sourcing to match
the guarded pattern used in ci/build_python_noarch.sh, using the same job/PR
condition check so it only runs for PR builds and skips artifact fetching
otherwise.

Comment thread ci/build_python.sh Outdated
Comment on lines +24 to +26
# TODO: Remove before merging. Use rapidsmpf conda packages from rapidsai/rapidsmpf#1081.
source ./ci/use_conda_packages_from_prs.sh

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Same unguarded PR-only artifact-fetch pattern.

Same concern as the sibling build scripts: this unconditionally sources ci/use_conda_packages_from_prs.sh, which will fail on non-PR (branch/nightly) runs of this job since it fetches hardcoded rapidsmpf#1081 artifacts.

🧰 Tools
🪛 Shellcheck (0.11.0)

[info] 25-25: Not following: ./ci/use_conda_packages_from_prs.sh was not specified as input (see shellcheck -x).

(SC1091)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ci/build_python.sh` around lines 24 - 26, The build_python.sh setup
unconditionally sources ci/use_conda_packages_from_prs.sh, which will break
non-PR runs by trying to fetch PR-only artifacts. Guard this logic the same way
as the other build scripts by checking the PR context before sourcing, and keep
the existing artifact helper invocation under that condition so branch/nightly
jobs skip it. Use the ci/use_conda_packages_from_prs.sh call site in the
build_python.sh flow as the target for the fix.

Comment thread ci/build_wheel_cudf_streaming.sh Outdated
Comment on lines +24 to +26
# TODO: Remove before merging. Use rapidsmpf wheels from rapidsai/rapidsmpf#1081.
source ./ci/use_wheels_from_prs.sh

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
rg -n 'build_wheel_cudf_streaming\.sh' .github/workflows -g '*.yml' -g '*.yaml'
rg -n 'build_type' .github/workflows -g '*.yml' -g '*.yaml' -B5 -A5 | rg -n 'cudf.streaming' -B10
cat -n ci/use_wheels_from_prs.sh 2>/dev/null | sed -n '1,80p'

Repository: rapidsai/cudf

Length of output: 10771


Gate PR-only wheel sourcing behind RAPIDS_BUILD_TYPE == "pull-request". ci/build_wheel_cudf_streaming.sh is also used by branch/nightly wheel builds, but source ./ci/use_wheels_from_prs.sh depends on PR artifacts. Wrap it in the same guard used elsewhere so non-PR builds don’t fail.

🧰 Tools
🪛 Shellcheck (0.11.0)

[info] 25-25: Not following: ./ci/use_wheels_from_prs.sh was not specified as input (see shellcheck -x).

(SC1091)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ci/build_wheel_cudf_streaming.sh` around lines 24 - 26, Gate the PR-only
wheel sourcing in build_wheel_cudf_streaming.sh behind the same
RAPIDS_BUILD_TYPE == "pull-request" check used elsewhere, since source
./ci/use_wheels_from_prs.sh only works with PR artifacts. Update the script
around the use_wheels_from_prs.sh import so branch and nightly builds skip it
and only pull-request builds execute it.

Comment thread ci/build_wheel_libcudf_streaming.sh Outdated
Comment on lines +20 to +22
# TODO: Remove before merging. Use rapidsmpf wheels from rapidsai/rapidsmpf#1081.
source ./ci/use_wheels_from_prs.sh

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
rg -n 'build_wheel_libcudf_streaming\.sh' .github/workflows -g '*.yml' -g '*.yaml'

Repository: rapidsai/cudf

Length of output: 316


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the wheel script and its workflow callers.
printf '\n== ci/build_wheel_libcudf_streaming.sh ==\n'
cat -n ci/build_wheel_libcudf_streaming.sh | sed -n '1,120p'

printf '\n== .github/workflows/build.yaml around caller ==\n'
sed -n '170,230p' .github/workflows/build.yaml

printf '\n== .github/workflows/pr.yaml around caller ==\n'
sed -n '530,610p' .github/workflows/pr.yaml

Repository: rapidsai/cudf

Length of output: 8271


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the related wheel script for comparison.
printf '\n== ci/build_wheel_cudf_streaming.sh ==\n'
cat -n ci/build_wheel_cudf_streaming.sh | sed -n '1,120p'

Repository: rapidsai/cudf

Length of output: 4072


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find where use_wheels_from_prs.sh is sourced and whether it's guarded.
rg -n 'use_wheels_from_prs\.sh|build_type|nightly|branch|pull_request|PR' ci .github/workflows -g '*.sh' -g '*.yml' -g '*.yaml'

Repository: rapidsai/cudf

Length of output: 21243


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== ci/build_wheel_libcudf_streaming.sh ==\n'
cat -n ci/build_wheel_libcudf_streaming.sh | sed -n '1,120p'

printf '\n== ci/use_wheels_from_prs.sh ==\n'
cat -n ci/use_wheels_from_prs.sh | sed -n '1,200p'

printf '\n== .github/workflows/build.yaml top section ==\n'
sed -n '1,120p' .github/workflows/build.yaml

printf '\n== .github/workflows/pr.yaml top section ==\n'
sed -n '1,120p' .github/workflows/pr.yaml

Repository: rapidsai/cudf

Length of output: 11617


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path
for path, start, end in [
    (Path('.github/workflows/build.yaml'), 180, 220),
    (Path('.github/workflows/pr.yaml'), 540, 590),
]:
    print(f"\n== {path} {start}-{end} ==")
    lines = path.read_text().splitlines()
    for i in range(start-1, min(end, len(lines))):
        print(f"{i+1}: {lines[i]}")
PY

Repository: rapidsai/cudf

Length of output: 4366


Guard ci/use_wheels_from_prs.sh behind PR builds. ci/build_wheel_libcudf_streaming.sh runs in both build.yaml and pr.yaml, but ci/use_wheels_from_prs.sh hardcodes PR artifact 1081. Unconditional sourcing will break branch/nightly wheel builds when those artifacts don’t exist.

🧰 Tools
🪛 Shellcheck (0.11.0)

[info] 21-21: Not following: ./ci/use_wheels_from_prs.sh was not specified as input (see shellcheck -x).

(SC1091)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ci/build_wheel_libcudf_streaming.sh` around lines 20 - 22, The unconditional
source of ci/use_wheels_from_prs.sh in build_wheel_libcudf_streaming.sh will
break non-PR wheel builds because it assumes PR artifact 1081 exists. Update the
script to only source ci/use_wheels_from_prs.sh when running in PR builds, and
leave branch/nightly builds to use the normal wheel sources; use the
build_wheel_libcudf_streaming.sh entrypoint and its existing PR context checks
to gate this behavior.

Comment thread ci/test_cudf_polars_polars_tests.sh Outdated
Comment on lines +21 to +23
# TODO: Remove before merging. Use rapidsmpf wheels from rapidsai/rapidsmpf#1081.
source ./ci/use_wheels_from_prs.sh

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
rg -n 'test_cudf_polars_polars_tests\.sh' .github/workflows -g '*.yml' -g '*.yaml'

Repository: rapidsai/cudf

Length of output: 317


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the polars test script and the PR/test workflow jobs that call it.
sed -n '1,120p' ci/test_cudf_polars_polars_tests.sh
printf '\n--- pr.yaml excerpt ---\n'
sed -n '660,730p' .github/workflows/pr.yaml
printf '\n--- test.yaml excerpt ---\n'
sed -n '260,320p' .github/workflows/test.yaml
printf '\n--- helper script ---\n'
sed -n '1,200p' ci/use_wheels_from_prs.sh

Repository: rapidsai/cudf

Length of output: 9684


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- test.yaml top ---'
sed -n '1,120p' .github/workflows/test.yaml
printf '\n%s\n' '--- pr.yaml top ---'
sed -n '1,120p' .github/workflows/pr.yaml
printf '\n%s\n' '--- references to test.yaml ---'
rg -n 'test\.yaml@main|build_type: .*branch|build_type: .*nightly|build_type: .*pull-request|cudf-polars-polars-tests' .github/workflows ci -g '*.yml' -g '*.yaml' -g '*.sh'

Repository: rapidsai/cudf

Length of output: 12440


Guard the PR-only wheel sourcing behind a build-type check ci/test_cudf_polars_polars_tests.sh:21-23 sources ./ci/use_wheels_from_prs.sh unconditionally, but .github/workflows/test.yaml can run this job with build_type: branch or nightly. Those runs will still try to fetch PR 1081 artifacts and can fail outside pull requests.

🧰 Tools
🪛 Shellcheck (0.11.0)

[info] 22-22: Not following: ./ci/use_wheels_from_prs.sh was not specified as input (see shellcheck -x).

(SC1091)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ci/test_cudf_polars_polars_tests.sh` around lines 21 - 23, The PR-only wheel
sourcing in ci/test_cudf_polars_polars_tests.sh is unconditional, so branch and
nightly runs can still try to fetch PR artifacts. Update the script around the
source ./ci/use_wheels_from_prs.sh call to only run when the build type
indicates a pull request, using the existing build_type handling or equivalent
check in this script. Keep the PR-wheel setup isolated to the same conditional
path that already distinguishes build_type values like branch and nightly.

Comment thread ci/test_python_common.sh Outdated
Comment on lines +14 to +16
# TODO: Remove before merging. Use rapidsmpf conda packages from rapidsai/rapidsmpf#1081.
source ./ci/use_conda_packages_from_prs.sh

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Same unguarded PR-artifact fetch, shared across all Python test jobs.

Since this is common setup sourced by all Python test jobs (per the file's own comment), unconditionally sourcing ci/use_conda_packages_from_prs.sh here risks breaking every non-PR (branch/nightly) Python test job that calls this script, not just one.

🧰 Tools
🪛 Shellcheck (0.11.0)

[info] 15-15: Not following: ./ci/use_conda_packages_from_prs.sh was not specified as input (see shellcheck -x).

(SC1091)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ci/test_python_common.sh` around lines 14 - 16, The shared Python test setup
currently sources ci/use_conda_packages_from_prs.sh unconditionally, which
affects every job that uses this script. Update the shared setup in
test_python_common.sh to only source that helper when running PR-based jobs,
using the existing job/context detection logic already available in the Python
test flow. Keep the change localized around the sourcing step so branch and
nightly jobs skip the PR-artifact fetch while PR jobs still use it.

Comment thread ci/test_wheel_cudf_polars.sh Outdated
Comment on lines +23 to +25
# TODO: Remove before merging. Use rapidsmpf wheels from rapidsai/rapidsmpf#1081.
source ./ci/use_wheels_from_prs.sh

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Same unguarded PR-only wheel fetch pattern.

Sourcing ci/use_wheels_from_prs.sh unconditionally will attempt to fetch rapidsmpf#1081 wheel artifacts even on non-PR (branch/nightly) runs of this test job, mirroring the conda-helper concern raised elsewhere in this stack.

🧰 Tools
🪛 Shellcheck (0.11.0)

[info] 24-24: Not following: ./ci/use_wheels_from_prs.sh was not specified as input (see shellcheck -x).

(SC1091)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ci/test_wheel_cudf_polars.sh` around lines 23 - 25, The wheel-fetch setup in
the test script is unguarded and always sources ci/use_wheels_from_prs.sh, which
will try to pull PR-only artifacts on non-PR runs. Update the logic around the
ci/use_wheels_from_prs.sh invocation to only run when the job is executing for a
PR, using the same gating approach as other PR-only wheel helpers in this stack,
so branch and nightly runs skip the rapidsmpf#1081 fetch path.

Comment thread ci/test_wheel_cudf_streaming.sh Outdated
Comment on lines +20 to +22
# TODO: Remove before merging. Use rapidsmpf wheels from rapidsai/rapidsmpf#1081.
source ./ci/use_wheels_from_prs.sh

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Same unguarded PR-only wheel fetch pattern.

Same concern as ci/test_wheel_cudf_polars.sh: unconditionally sourcing ci/use_wheels_from_prs.sh will fail to find rapidsmpf#1081 wheel artifacts on non-PR (branch/nightly) test runs.

🧰 Tools
🪛 Shellcheck (0.11.0)

[info] 21-21: Not following: ./ci/use_wheels_from_prs.sh was not specified as input (see shellcheck -x).

(SC1091)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ci/test_wheel_cudf_streaming.sh` around lines 20 - 22, The unguarded source
of use_wheels_from_prs.sh in the streaming wheel test script can break non-PR
branch/nightly runs by trying to fetch rapidsmpf#1081 artifacts that do not
exist there. Update the test setup so the wheel fetch in
test_wheel_cudf_streaming.sh is only invoked for PR builds, following the same
guard pattern used in the related test_wheel_cudf_polars.sh flow, and keep the
conditional centered around the use_wheels_from_prs.sh call.

Comment thread ci/build_cpp.sh
@@ -1,5 +1,5 @@
#!/bin/bash

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Please disregard all changes in .sh files. They will be reverted.

Comment on lines 50 to 51

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 don't think we need the option anymore?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hmm. Yes, I am wondering why clang-tidy didnt flag it. 🤔

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah. -Wunused-variable doesnt flag unused complex structs.

RAPIDSMPF_MPI(MPI_Comm_split(mpi_comm_, rank, 0, &split_comm));
return std::shared_ptr<rapidsmpf::MPI>(
new rapidsmpf::MPI(split_comm, options, comm_->progress_thread()),
new rapidsmpf::MPI(split_comm, comm_->progress_thread(), comm_->logger()),

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 is reusing the parent logger for a rank-0 split communicator, so the constructor renames that shared logger to "0", causing subsequent parent-communicator logs from every process to be mislabeled as rank 0, no? I think the split communicator should receive a fresh logger.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's a good point.

@nirandaperera
nirandaperera requested a review from madsbk July 2, 2026 23:27

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

Thanks @nirandaperera, the C++ code looks good to me, but I haven't looked at the CI scripts.

@nirandaperera

Copy link
Copy Markdown
Contributor Author

Thanks @nirandaperera, the C++ code looks good to me, but I haven't looked at the CI scripts.

Thanks @madsbk. CI scripts are only for testing against the rapidsmpf PR. I am reverting them once its sufficiently green. 😉

@nirandaperera
nirandaperera force-pushed the rapidsmpf-logger-api-changes branch 2 times, most recently from 164cb0c to 4028efb Compare July 6, 2026 18:18

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

♻️ Duplicate comments (6)
ci/test_java.sh (1)

12-14: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Unconditional PR-only conda bootstrap affects branch/nightly Java test runs.

Same issue as raised previously: use_conda_packages_from_prs.sh is sourced without a RAPIDS_BUILD_TYPE == "pull-request" guard, unlike the pattern used in ci/cudf_pandas_scripts/third-party-integration/test.sh.

Also applies to: 26-26

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ci/test_java.sh` around lines 12 - 14, The PR-only conda bootstrap is being
sourced unconditionally in the Java test script. Update the test setup around
use_conda_packages_from_prs.sh in ci/test_java.sh to only source it when
RAPIDS_BUILD_TYPE equals pull-request, matching the conditional pattern used in
ci/cudf_pandas_scripts/third-party-integration/test.sh. Keep the existing Java
test flow intact while guarding the PR-specific package override behind that
build-type check.

Source: Path instructions

ci/test_cudf_polars_polars_tests.sh (1)

21-23: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Unconditional PR-wheel sourcing can break non-PR polars test runs.

.github/workflows/test.yaml can invoke this job with branch/nightly build types; ci/use_wheels_from_prs.sh is sourced without a guard, matching a previously raised comment.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ci/test_cudf_polars_polars_tests.sh` around lines 21 - 23, The PR-wheel
sourcing in ci/test_cudf_polars_polars_tests.sh is unconditional and can break
non-PR test runs. Guard the source of ci/use_wheels_from_prs.sh so it only runs
for PR-based builds, using the existing build-type context from the test job
invocation (for example branch/nightly versus PR) and keep the behavior scoped
to the relevant test setup path.

Source: Path instructions

ci/build_wheel_cudf_streaming.sh (1)

24-26: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Unconditionally sourcing PR-only wheel helper affects branch/nightly builds.

This script is also used for non-PR wheel builds, but ci/use_wheels_from_prs.sh fetches hard-coded PR #1081 artifacts unconditionally. This was already flagged in a previous review round.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ci/build_wheel_cudf_streaming.sh` around lines 24 - 26, The
build_wheel_cudf_streaming.sh script is unconditionally sourcing the PR-only
wheel helper, which breaks non-PR branch and nightly builds. Update the build
setup so ci/use_wheels_from_prs.sh is only invoked for PR-specific builds, and
keep the wheel selection logic in the script guarded by the appropriate
build-type check so regular builds do not fetch the hard-coded
rapidsai/rapidsmpf#1081 artifacts.

Source: Path instructions

ci/build_wheel_libcudf_streaming.sh (1)

20-22: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Same unconditional PR-wheel sourcing issue as build_wheel_cudf_streaming.sh.

ci/use_wheels_from_prs.sh is sourced without a build-type guard here too, which can break non-PR wheel builds.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ci/build_wheel_libcudf_streaming.sh` around lines 20 - 22, The wheel build
script currently sources ci/use_wheels_from_prs.sh unconditionally, which can
break non-PR wheel builds. Update build_wheel_libcudf_streaming.sh to gate that
sourcing behind the same build-type check used in build_wheel_cudf_streaming.sh,
so the PR-wheel override only runs for PR builds and not for release or
scheduled builds.

Source: Path instructions

ci/test_cpp_common.sh (1)

12-14: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Unconditional PR-artifact sourcing in a script used by branch/nightly C++ test jobs.

ci/test_cpp_common.sh is shared by test_cpp.sh, test_cpp_memcheck.sh, and test_cpp_benchmarks.sh, which also run outside pull-request builds; sourcing use_conda_packages_from_prs.sh unconditionally (and consuming RAPIDS_PREPENDED_CHANNEL_ARGS at Line 26) ties those runs to PR #1081 artifacts. Flagged previously on the same lines.

Also applies to: 26-26

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ci/test_cpp_common.sh` around lines 12 - 14, Remove the unconditional
sourcing of use_conda_packages_from_prs.sh from test_cpp_common.sh so branch and
nightly jobs don’t depend on PR-only artifacts. Update the shared setup in
test_cpp_common.sh to only prepend RAPIDS_PREPENDED_CHANNEL_ARGS when running in
a PR context, and ensure test_cpp.sh, test_cpp_memcheck.sh, and
test_cpp_benchmarks.sh can run without rapidsmpf PR package overrides.

Source: Path instructions

ci/test_notebooks.sh (1)

12-14: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Unconditional PR-only conda bootstrap affects branch/nightly notebook test runs.

Same guard-missing issue as raised previously in this file; non-PR runs will still attempt to fetch PR #1081 artifacts.

Also applies to: 30-30

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ci/test_notebooks.sh` around lines 12 - 14, The notebook test bootstrap
currently sources the PR-only conda package setup unconditionally, so branch and
nightly runs still try to fetch rapidsai/rapidsmpf#1081 artifacts. Add the same
PR-only guard used elsewhere in this script around source
./ci/use_conda_packages_from_prs.sh so it only runs for PR builds, and leave
non-PR notebook runs on the default conda environment.

Source: Path instructions

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@ci/build_wheel_cudf_streaming.sh`:
- Around line 24-26: The build_wheel_cudf_streaming.sh script is unconditionally
sourcing the PR-only wheel helper, which breaks non-PR branch and nightly
builds. Update the build setup so ci/use_wheels_from_prs.sh is only invoked for
PR-specific builds, and keep the wheel selection logic in the script guarded by
the appropriate build-type check so regular builds do not fetch the hard-coded
rapidsai/rapidsmpf#1081 artifacts.

In `@ci/build_wheel_libcudf_streaming.sh`:
- Around line 20-22: The wheel build script currently sources
ci/use_wheels_from_prs.sh unconditionally, which can break non-PR wheel builds.
Update build_wheel_libcudf_streaming.sh to gate that sourcing behind the same
build-type check used in build_wheel_cudf_streaming.sh, so the PR-wheel override
only runs for PR builds and not for release or scheduled builds.

In `@ci/test_cpp_common.sh`:
- Around line 12-14: Remove the unconditional sourcing of
use_conda_packages_from_prs.sh from test_cpp_common.sh so branch and nightly
jobs don’t depend on PR-only artifacts. Update the shared setup in
test_cpp_common.sh to only prepend RAPIDS_PREPENDED_CHANNEL_ARGS when running in
a PR context, and ensure test_cpp.sh, test_cpp_memcheck.sh, and
test_cpp_benchmarks.sh can run without rapidsmpf PR package overrides.

In `@ci/test_cudf_polars_polars_tests.sh`:
- Around line 21-23: The PR-wheel sourcing in
ci/test_cudf_polars_polars_tests.sh is unconditional and can break non-PR test
runs. Guard the source of ci/use_wheels_from_prs.sh so it only runs for PR-based
builds, using the existing build-type context from the test job invocation (for
example branch/nightly versus PR) and keep the behavior scoped to the relevant
test setup path.

In `@ci/test_java.sh`:
- Around line 12-14: The PR-only conda bootstrap is being sourced
unconditionally in the Java test script. Update the test setup around
use_conda_packages_from_prs.sh in ci/test_java.sh to only source it when
RAPIDS_BUILD_TYPE equals pull-request, matching the conditional pattern used in
ci/cudf_pandas_scripts/third-party-integration/test.sh. Keep the existing Java
test flow intact while guarding the PR-specific package override behind that
build-type check.

In `@ci/test_notebooks.sh`:
- Around line 12-14: The notebook test bootstrap currently sources the PR-only
conda package setup unconditionally, so branch and nightly runs still try to
fetch rapidsai/rapidsmpf#1081 artifacts. Add the same PR-only guard used
elsewhere in this script around source ./ci/use_conda_packages_from_prs.sh so it
only runs for PR builds, and leave non-PR notebook runs on the default conda
environment.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: dcf62e41-fef5-453d-a916-a650e47b2592

📥 Commits

Reviewing files that changed from the base of the PR and between 40a26de and 4028efb.

📒 Files selected for processing (16)
  • ci/build_cpp.sh
  • ci/build_docs.sh
  • ci/build_python.sh
  • ci/build_python_noarch.sh
  • ci/build_wheel_cudf_streaming.sh
  • ci/build_wheel_libcudf_streaming.sh
  • ci/cudf_pandas_scripts/third-party-integration/test.sh
  • ci/test_cpp_common.sh
  • ci/test_cudf_polars_polars_tests.sh
  • ci/test_java.sh
  • ci/test_notebooks.sh
  • ci/test_python_common.sh
  • ci/test_wheel_cudf_polars.sh
  • ci/test_wheel_cudf_streaming.sh
  • ci/use_conda_packages_from_prs.sh
  • ci/use_wheels_from_prs.sh
🚧 Files skipped from review as they are similar to previous changes (2)
  • ci/use_conda_packages_from_prs.sh
  • ci/use_wheels_from_prs.sh

Signed-off-by: niranda perera <niranda.perera@gmail.com>
@nirandaperera
nirandaperera force-pushed the rapidsmpf-logger-api-changes branch from 4028efb to 8eda57a Compare July 6, 2026 20:34
Signed-off-by: niranda perera <niranda.perera@gmail.com>
Signed-off-by: niranda perera <niranda.perera@gmail.com>
@nirandaperera
nirandaperera force-pushed the rapidsmpf-logger-api-changes branch from df21f85 to 3da4723 Compare July 6, 2026 21:05
@Matt711

Matt711 commented Jul 7, 2026

Copy link
Copy Markdown
Member

/merge

@rapids-bot
rapids-bot Bot merged commit 52d322c into NVIDIA:main Jul 7, 2026
256 of 261 checks passed
@nirandaperera

Copy link
Copy Markdown
Contributor Author

Thanks @Matt711

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

Labels

breaking Breaking change improvement Improvement / enhancement to an existing function libcudf Affects libcudf (C++/CUDA) code.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants