[chyunsu3]

When no GPU is available, raft_handle_ is null. The nvForest handle should not access raft_handle_ if it's null.

[coderabbitai]

📝 Walkthrough

Summary by CodeRabbit

• Bug Fixes
  • Improved GPU operation safety when no RAFT handle is provided.
  • Stream-related queries now return safe empty values, and synchronization is skipped when unavailable.
  • Existing behavior remains unchanged when a valid handle is present.

Walkthrough

GPU-enabled handle_t methods now safely handle null RAFT handles. Null handles produce empty streams and zero stream-pool size, and skip synchronization. Non-null handles retain existing behavior.

Changes

GPU handle safety

Layer / File(s)	Summary
Guard GPU handle operations cpp/include/nvforest/handle.hpp	GPU stream retrieval, stream-pool sizing, and synchronization now check for a null RAFT handle before calling RAFT APIs.

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

Suggested reviewers: csadorf, dantegd, ramitchell

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the fix for null GPU handles in nvforest::handle_t.
Description check	✅ Passed	The description explains that nvforest::handle_t must avoid accessing a null raft_handle_ when no GPU is available.
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.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

cpp/include/nvforest/handle.hpp (1)

21-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Document the new null-handle contract.

This public header changes the behavior of existing methods. Add Doxygen documentation that states:

• When a null handle_t is valid.
• Whether GPU inference requires a non-null RAFT handle.
• That get_stream_pool_size() returns zero while get_usable_stream_count() remains one.
• Whether synchronize() is a no-op for null handles.
• Thread-safety and RAFT-handle lifetime expectations.

Update migration documentation if null-handle usage changes the supported API.

As per path instructions: public C++ API changes must document thread-safety, GPU requirements, numerical behavior, and changed API semantics.

🤖 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/include/nvforest/handle.hpp` around lines 21 - 35, Document the
null-handle contract in the public handle_t API around get_stream_pool_size(),
get_usable_stream_count(), and synchronize(): specify when a null handle is
valid, that GPU inference requires a non-null RAFT handle, that pool size is
zero while usable stream count is one, and that synchronization is a no-op for
null handles. Also document thread-safety, RAFT-handle lifetime requirements,
and any relevant numerical behavior, and update migration documentation if this
changes supported null-handle usage.

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.

Inline comments:
In `@cpp/include/nvforest/handle.hpp`:
- Around line 30-35: Update handle_t::synchronize() so the null raft_handle_
path synchronizes the default CUDA stream via cuda_stream{}; preserve the
existing raft stream-pool and stream synchronization when raft_handle_ is
present.

---

Nitpick comments:
In `@cpp/include/nvforest/handle.hpp`:
- Around line 21-35: Document the null-handle contract in the public handle_t
API around get_stream_pool_size(), get_usable_stream_count(), and synchronize():
specify when a null handle is valid, that GPU inference requires a non-null RAFT
handle, that pool size is zero while usable stream count is one, and that
synchronization is a no-op for null handles. Also document thread-safety,
RAFT-handle lifetime requirements, and any relevant numerical behavior, and
update migration documentation if this changes supported null-handle usage.

🪄 Autofix

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: a43da01e-7805-477b-bf9c-73ff47ecd591

📥 Commits

Reviewing files that changed from the base of the PR and between 87097c9 and 7aceae9.

📒 Files selected for processing (1)

• cpp/include/nvforest/handle.hpp

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

rg -n -C 6 'using[[:space:]]+cuda_stream|cudaStreamSynchronize|handle_t[[:space:]]*\{\}|get_next_usable_stream|\.synchronize[[:space:]]*\(' cpp

Repository: rapidsai/nvforest

Length of output: 9917

---

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

printf '%s\n' '--- handle.hpp ---'
cat -n cpp/include/nvforest/handle.hpp

printf '%s\n' '--- cuda_stream.hpp ---'
cat -n cpp/include/nvforest/cuda_stream.hpp

printf '%s\n' '--- forest_model.hpp focused API and prediction ---'
sed -n '1,280p' cpp/include/nvforest/forest_model.hpp | cat -n

printf '%s\n' '--- README GPU example ---'
sed -n '70,130p' cpp/include/nvforest/README.md | cat -n

printf '%s\n' '--- synchronization and prediction call sites ---'
rg -n -C 5 'synchronize[[:space:]]*\(|get_next_usable_stream|predict\(' cpp/tests cpp/include/nvforest -g '*.{h,hpp,cpp,cu,md}'

Repository: rapidsai/nvforest

Length of output: 35447

---

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

printf '%s\n' '--- copy implementation ---'
cat -n cpp/include/nvforest/detail/copy.hpp

printf '%s\n' '--- prediction implementations and CUDA launches ---'
rg -n -C 8 'cudaMemcpy|cudaMemcpyAsync|cudaMemcpy2D|cudaMemcpy2DAsync|<<<|cudaStream|stream[), ]' cpp/include/nvforest -g '*.{h,hpp,cu,cpp}'

printf '%s\n' '--- focused source invariant verifier ---'
python3 - <<'PY'
from pathlib import Path

handle = Path("cpp/include/nvforest/handle.hpp").read_text()
streams = Path("cpp/include/nvforest/cuda_stream.hpp").read_text()
model = Path("cpp/include/nvforest/forest_model.hpp").read_text()
readme = Path("cpp/include/nvforest/README.md").read_text()
copy = Path("cpp/include/nvforest/detail/copy.hpp").read_text()

checks = {
    "null handle returns default cuda stream": "if (!raft_handle_) { return cuda_stream{}; }" in handle,
    "null handle synchronize path has no sync call": (
        "if (raft_handle_) {" in handle
        and "raft_handle_->sync_stream_pool();" in handle
        and "raft_handle_->sync_stream();" in handle
    ),
    "cuda stream is CUDA stream in GPU builds": "using cuda_stream = cudaStream_t;" in streams,
    "stream synchronization uses cudaStreamSynchronize": "cudaStreamSynchronize(stream);" in streams,
    "handle stream reaches direct inference": "handle.get_next_usable_stream(), predict_type" in model,
    "handle stream reaches partition inference": "partition_out, partition_in, stream, predict_type" in model,
    "handle stream reaches partition copy": "partition_out.size(),\n                                            stream);" in model,
    "GPU README uses null handle": "auto handle = nvforest::handle_t{};" in readme,
    "copy API accepts explicit stream": "cuda_stream stream" in copy,
}

for name, result in checks.items():
    print(f"{'PASS' if result else 'FAIL'}: {name}")

if not all(checks.values()):
    raise SystemExit("source invariant check failed")
PY

Repository: rapidsai/nvforest

Length of output: 50373

---

Synchronize the default CUDA stream for null handles.

handle_t{} is used for GPU inference, and forest_model::predict submits inference and copy operations to its default CUDA stream. handle_t::synchronize() does nothing when raft_handle_ is null, so callers can read incomplete output after synchronization. Synchronize cuda_stream{} on this path, or reject null handles for GPU execution and document that contract.

🤖 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/include/nvforest/handle.hpp` around lines 30 - 35, Update
handle_t::synchronize() so the null raft_handle_ path synchronizes the default
CUDA stream via cuda_stream{}; preserve the existing raft stream-pool and stream
synchronization when raft_handle_ is present.

[chyunsu3]

Closing in favor of #193 (which would remove nvforest::handle_t entirely)