Skip to content

[gpu-info, mcp] fix: surface listing enumeration failures - #235

Merged
Wangmerlyn merged 1 commit into
mainfrom
codex/list-gpus-enumeration-unavailable
Jul 1, 2026
Merged

[gpu-info, mcp] fix: surface listing enumeration failures#235
Wangmerlyn merged 1 commit into
mainfrom
codex/list-gpus-enumeration-unavailable

Conversation

@Wangmerlyn

@Wangmerlyn Wangmerlyn commented Jul 1, 2026

Copy link
Copy Markdown
Owner

Summary

  • raise DeviceEnumerationUnavailableError when CUDA/ROCm visible device counts fail during GPU listing
  • keep list_gpus//api/gpus aligned with startup-unavailable classification instead of returning successful empty lists
  • add CUDA, ROCm, and service-level regression coverage plus docs/AGENTS guidance

Test Plan

  • PYTHONPATH=src pytest tests/utilities/test_gpu_info.py -q -k 'count_fails or enumeration_unavailable' (RED before fix, green after)
  • PYTHONPATH=src pytest tests/mcp/test_server.py -q -k 'list_gpus and enumeration' (RED before fix, green after)
  • PYTHONPATH=src pytest tests/utilities/test_gpu_info.py -q
  • PYTHONPATH=src pytest tests/mcp/test_server.py -q -k 'list_gpus or DeviceEnumerationUnavailableError or startup_unavailable'
  • PYTHONPATH=src pytest tests/mcp/test_http_api.py -q -k 'api_gpus or list_gpus or startup_unavailable or enumeration_unavailable'
  • PYTHONPATH=src pytest tests -q
  • PYTHONPATH=src mkdocs build --strict
  • pre-commit run --all-files --show-diff-on-failure
  • git diff --check

Summary by CodeRabbit

  • New Features

    • GPU enumeration failures are now surfaced as unavailable errors instead of being treated like empty GPU lists.
    • CLI docs now clearly describe the updated GPU listing error behavior and telemetry handling.
  • Bug Fixes

    • Improved GPU detection so failed CUDA/ROCm device checks propagate correctly through the app.
    • Added coverage to ensure GPU shutdown cleanup still runs when enumeration fails.
  • Documentation

    • Updated guidance and reference docs to match the new GPU listing behavior.

@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@Wangmerlyn, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 35 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 13e29800-06b1-42c1-b244-a44a58e8962f

📥 Commits

Reviewing files that changed from the base of the PR and between 9c5708a and d253f18.

📒 Files selected for processing (6)
  • AGENTS.md
  • docs/plans/list-gpus-enumeration-unavailable.md
  • docs/reference/cli.md
  • src/keep_gpu/utilities/gpu_info.py
  • tests/mcp/test_server.py
  • tests/utilities/test_gpu_info.py
📝 Walkthrough

Walkthrough

CUDA/ROCm device enumeration failures via torch.cuda.device_count() are now classified as DeviceEnumerationUnavailableError and propagated through gpu_info.py's fallback chain instead of returning successful empty GPU lists. A new helper wraps torch.cuda.device_count(), error propagation was added to fallback branches, tests were updated/added, and documentation and a plan file were updated accordingly.

Changes

Enumeration Unavailable Error Propagation

Layer / File(s) Summary
Docs and plan for enumeration-unavailable behavior
AGENTS.md, docs/reference/cli.md, docs/plans/list-gpus-enumeration-unavailable.md
Documents that CUDA/ROCm device enumeration failures should be reported as startup-unavailable errors instead of empty GPU lists, and adds an implementation plan describing the fix and verification checklist.
Core enumeration helper and error propagation
src/keep_gpu/utilities/gpu_info.py
Introduces _visible_torch_device_count() to wrap torch.cuda.device_count(), raising DeviceEnumerationUnavailableError on failure; updates ROCm/torch query paths to use it and re-raises the error through _query_torch() and get_gpu_info() fallback branches instead of swallowing it.
Test coverage for enumeration failures
tests/utilities/test_gpu_info.py, tests/mcp/test_server.py
Adds device_count_error injection to ROCm mocks, replaces the prior empty-list test with tests asserting DeviceEnumerationUnavailableError is raised for CUDA/ROCm paths, and adds an MCP server test confirming list_gpus propagates the real enumeration failure while still shutting down NVML once.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

  • Wangmerlyn/KeepGPU#151: Introduces the DeviceEnumerationUnavailableError type and threads visible-device-count unavailability through related components as part of the same overall effort.
  • Wangmerlyn/KeepGPU#224: Updates how DeviceEnumerationUnavailableError is surfaced during list_gpus, mapping it to a JSON-RPC startup-unavailable response.
  • Wangmerlyn/KeepGPU#135: Also modifies gpu_info.py CUDA listing behavior based on Torch-visible device enumeration/count.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 4.55% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and accurately reflects the main change: surfacing GPU listing enumeration failures in gpu-info and mcp.
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
  • Commit unit tests in branch codex/list-gpus-enumeration-unavailable

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request implements a plan to surface CUDA/ROCm visible-device enumeration failures as startup-unavailable errors (DeviceEnumerationUnavailableError) instead of successful empty GPU lists. It modifies gpu_info.py to raise this error when torch.cuda.device_count() fails, propagates it through the query functions, and updates the tests and documentation accordingly. The review feedback highlights potential gaps where unhandled exceptions in torch.cuda.is_available() or torch.cuda.current_device() could still be swallowed by outer try-except blocks in get_gpu_info(), bypassing the intended error propagation. Additionally, a cleaner, more idiomatic exception handling structure is suggested for _torch_cuda_visible_count().

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/keep_gpu/utilities/gpu_info.py Outdated
Comment thread src/keep_gpu/utilities/gpu_info.py
Comment thread src/keep_gpu/utilities/gpu_info.py Outdated

@coderabbitai coderabbitai Bot 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.

🧹 Nitpick comments (3)
src/keep_gpu/utilities/gpu_info.py (2)

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

torch.cuda.is_available() is called twice in a row.

Minor duplication; caching the result would avoid a redundant call.

🤖 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 `@src/keep_gpu/utilities/gpu_info.py` around lines 240 - 243, The GPU
availability check in gpu_info.py is duplicated by calling
torch.cuda.is_available() twice in the same flow. Cache that result once in the
function that sets current_device and computes count, then reuse the cached
boolean for both the current_device assignment and the
_visible_torch_device_count() branch to avoid the redundant call.

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

Redundant double-wrapping of the enumeration error.

_visible_torch_device_count() already converts every failure into DeviceEnumerationUnavailableError. The surrounding try/except Exception in _torch_cuda_visible_count() then re-checks isinstance(exc, DeviceEnumerationUnavailableError) to re-raise it — this only matters for a failure in cuda.is_available() itself. Catching DeviceEnumerationUnavailableError explicitly first would be clearer:

♻️ Simplify exception handling
     try:
         if not cuda.is_available():
             return 0
         count = _visible_torch_device_count()
-    except Exception as exc:
-        if isinstance(exc, DeviceEnumerationUnavailableError):
-            raise
+    except DeviceEnumerationUnavailableError:
+        raise
+    except Exception as exc:
         logger.debug("Torch CUDA visible count failed: %s", exc)
         raise DeviceEnumerationUnavailableError(
             f"Unable to enumerate visible GPUs: {exc}"
         ) from exc
🤖 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 `@src/keep_gpu/utilities/gpu_info.py` around lines 35 - 49, The exception
handling in `_torch_cuda_visible_count()` is redundantly re-wrapping
`DeviceEnumerationUnavailableError` after calling
`_visible_torch_device_count()`. Update this function to catch
`DeviceEnumerationUnavailableError` explicitly before the generic `Exception`
path, and keep the generic wrapping only for failures from `cuda.is_available()`
or other unexpected errors. Use the existing `_torch_cuda_visible_count` and
`_visible_torch_device_count` symbols to keep the flow clear and avoid
double-handling the same error type.
docs/plans/list-gpus-enumeration-unavailable.md (1)

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

Heading level skips from h1 to h3.

Static analysis flagged that ### Task 1 jumps directly from the document's # Title (h1) without an intervening h2. Purely cosmetic for a plan doc.

🤖 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 `@docs/plans/list-gpus-enumeration-unavailable.md` at line 13, The document
heading hierarchy skips from the top-level title to a level-3 heading, so update
the Task 1 heading in the plan doc to use the appropriate intermediate level.
Keep the section label consistent with the surrounding structure by adjusting
the heading markup for “Task 1: Propagate Enumeration Failures from GPU Listing”
so it follows the document’s h1 title with an h2.

Source: Linters/SAST tools

🤖 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 `@docs/plans/list-gpus-enumeration-unavailable.md`:
- Line 13: The document heading hierarchy skips from the top-level title to a
level-3 heading, so update the Task 1 heading in the plan doc to use the
appropriate intermediate level. Keep the section label consistent with the
surrounding structure by adjusting the heading markup for “Task 1: Propagate
Enumeration Failures from GPU Listing” so it follows the document’s h1 title
with an h2.

In `@src/keep_gpu/utilities/gpu_info.py`:
- Around line 240-243: The GPU availability check in gpu_info.py is duplicated
by calling torch.cuda.is_available() twice in the same flow. Cache that result
once in the function that sets current_device and computes count, then reuse the
cached boolean for both the current_device assignment and the
_visible_torch_device_count() branch to avoid the redundant call.
- Around line 35-49: The exception handling in `_torch_cuda_visible_count()` is
redundantly re-wrapping `DeviceEnumerationUnavailableError` after calling
`_visible_torch_device_count()`. Update this function to catch
`DeviceEnumerationUnavailableError` explicitly before the generic `Exception`
path, and keep the generic wrapping only for failures from `cuda.is_available()`
or other unexpected errors. Use the existing `_torch_cuda_visible_count` and
`_visible_torch_device_count` symbols to keep the flow clear and avoid
double-handling the same error type.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2f6a3af6-a885-4e73-8553-06ff31e5daa4

📥 Commits

Reviewing files that changed from the base of the PR and between c3955bc and 9c5708a.

📒 Files selected for processing (6)
  • AGENTS.md
  • docs/plans/list-gpus-enumeration-unavailable.md
  • docs/reference/cli.md
  • src/keep_gpu/utilities/gpu_info.py
  • tests/mcp/test_server.py
  • tests/utilities/test_gpu_info.py

@Wangmerlyn
Wangmerlyn force-pushed the codex/list-gpus-enumeration-unavailable branch 2 times, most recently from 0282f86 to 4949be1 Compare July 1, 2026 08:36
@Wangmerlyn
Wangmerlyn force-pushed the codex/list-gpus-enumeration-unavailable branch from 4949be1 to d253f18 Compare July 1, 2026 08:42
@Wangmerlyn

Copy link
Copy Markdown
Owner Author

Addressed the CodeRabbit nitpick bundle on the current head: _query_rocm() now caches the availability result, _torch_cuda_visible_count() no longer uses the generic catch plus isinstance() pattern, and the plan heading is now an h2. Local verification after the final amend: PYTHONPATH=src pytest tests -q -> 884 passed, 11 skipped; PYTHONPATH=src mkdocs build --strict passed with the existing Material warning; pre-commit run --all-files --show-diff-on-failure passed; git diff --check passed.

@Wangmerlyn
Wangmerlyn merged commit ffdf3f3 into main Jul 1, 2026
6 checks passed
@Wangmerlyn
Wangmerlyn deleted the codex/list-gpus-enumeration-unavailable branch July 1, 2026 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant