Skip to content

fix(cli): report GPU inventory and disclose omitted devices - #13070

Open
Dongni-Yang wants to merge 5 commits into
mainfrom
fix/13019-host-probe-gpu-inventory
Open

Dongni-Yang wants to merge 5 commits into
mainfrom
fix/13019-host-probe-gpu-inventory

Conversation

@Dongni-Yang

@Dongni-Yang Dongni-Yang commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Outcome

Host probe reports each detected GPU's index, name, and memory in JSON and text.
When the public evidence limit omits device rows, both outputs report how many were collected, listed, and omitted.

Reason

The host projection retained aggregate GPU memory but discarded individual device details.
The first inventory fix exposed those details, but large inventories could silently exceed the public evidence limit.

Related issues

Closes #13019.

Changes

  • Copy per-device observations from the existing detector and render a GPU list.
  • Preserve referenced evidence and give other diagnostics space before optional device rows.
  • Add a validated report-level gpuInventory summary without replacing evidence details or increasing evidence limits.
  • Keep aggregate memory, admission decisions, status, and exit codes unchanged.

Verification

Current commit: 3fec450eeb0b0a53ff9270728d9e1fac95ca814c.

  • npx vitest run --project cli src/lib/readiness --maxWorkers=1 — 335 tests pass across 13 files. The final inventory and contract suites pass all 42 tests.
  • Four original overflow regressions fail on the earlier PR source; 17 controls pass.
  • Five report-summary regressions fail on 58e6887; 22 controls pass. The fixed focused suites pass. The cases check report ownership, no unrelated-evidence annotation, malformed totals, and repeated projection.
  • Tests cover 255, 256, and 257 device rows, required evidence, gateway/platform diagnostics, zero available device slots, repeated projection, and malformed counts.
  • The full-capacity test verifies that all 16 existing diagnostic fields remain intact.
  • CLI build and npm run typecheck:cli pass.
  • Formatting, lint, repository, source-shape, and growth checks pass.
  • Normal pre-push publication and compiler checks pass. All published commits are GitHub Verified.

Detection gap: the original tests covered small inventories and did not test evidence-limit behavior.
The added boundary cases require explicit omission counts in JSON and text.
Tests inject GPU observations. No physical multi-GPU run is claimed.
The diff contains no secrets, API keys, or credentials.

Review notes

Independent reviews checked evidence priority, count validation, redaction, retained diagnostic details, and unchanged readiness decisions.
The optional report summary fits the existing additive report schema. The contract test verifies the projected result with the published JSON schema.
The final repair addresses the Advisor architecture finding on 58e6887: aggregate GPU counts now belong to the report instead of an unrelated diagnostic entry.
The branch includes main's shared image dependency repair and qualification receipts.
At head 3fec450eeb0b0a53ff9270728d9e1fac95ca814c, source CI and self-hosted qualification passed, including the generic NVIDIA GPU lane. CodeRabbit reviewed this exact head and identified no concrete blocker. All nine Advisor specialist artifacts passed digest and head/base/workflow provenance checks. Eight reports were clear. One report requests a separate identity channel for omitted GPUs. That request is excluded from this repair as a deferred enhancement: the reported two-device host fits the current production report, while the synthetic zero-slot case intentionally preserves required evidence and prints exact omission counts. The required-first selector and 256-entry limit are inherited from the recorded base. The Advisor gate remains failed on that report; no check waiver or maintainer approval is claimed.

Maintainer approval remains required.


Signed-off-by: Dongni Yang dongniy@nvidia.com

Summary by CodeRabbit

  • New Features
    • Readiness results can include details for each detected NVIDIA GPU, including its name, index, and memory capacity.
    • Text reports display GPU details when available and omit the section when per-device information is unavailable.
    • Reports list up to 256 GPUs and indicate how many devices were omitted when the inventory is incomplete.
    • Other readiness evidence is prioritized when the report reaches its evidence limit.
    • GPU names are limited in length, and invalid inventory summaries are excluded from public reports.

Preserve each detected GPU name and memory size in readiness evidence.
Keep the minimum-memory observation used by serving-profile admission.

Refs #13019

Signed-off-by: Dongni Yang <dongniy@nvidia.com>
@Dongni-Yang Dongni-Yang self-assigned this Oct 13, 2026
@copy-pr-bot

copy-pr-bot Bot commented Oct 13, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration
  • Configuration used: Repository: NVIDIA/NemoClaw/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Enterprise
  • Run ID: ee44bced-7fbc-4aa1-b2ea-637faf36c148
📥 Commits

Reviewing files that changed from the base of the PR and between 3fec450 and 7a25ef9.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: NVIDIA/NemoClaw/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Enterprise
  • Run ID: 2740f041-5f57-4189-a27e-c03c4f19d0b0
📥 Commits

Reviewing files that changed from the base of the PR and between 58e6887 and 3fec450.

📒 Files selected for processing: 4
  • src/lib/readiness/contract.test.ts
  • src/lib/readiness/host-gpu-inventory.test.ts
  • src/lib/readiness/presentation.ts
  • src/lib/readiness/types.ts

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.

📝 Walkthrough

Host readiness snapshots now retain per-GPU names and memory. Readiness reports include indexed evidence for available GPUs, preserve non-GPU evidence within the public limit, and show valid inventory counts and device summaries in text reports.

Changes: GPU readiness inventory

Layer / File(s) Summary
Capture and project GPU inventory
src/lib/readiness/host.ts, src/lib/readiness/host-gpu-inventory.test.ts
Host observations copy GPU names and memory, with names limited to 512 characters. Readiness projection adds indexed per-device evidence with memory in MiB and bytes. Tests cover device collection and projection.
Render and limit GPU evidence
src/lib/readiness/types.ts, src/lib/readiness/presentation.ts, src/lib/readiness/contract.test.ts, src/lib/readiness/host-gpu-inventory.test.ts
The public report type adds GPU inventory counts. Public evidence selection prioritizes non-GPU entries and reports valid collected, listed, and omitted GPU counts. Text reports show GPU summaries and incomplete inventory counts. Tests cover evidence limits, count validation, rendering, and schema validation.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: cv

Merge Risk: ⚪ Minimal · up to 3fec4

The change adds per-GPU inventory and omission counts to the host probe output. No concrete merge-blocking risk was identified in the supplied changes.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 5 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Linked Issues check Passed Issue #13019 requires nemoclaw host probe to identify each provider-visible GPU and report each device's memory in JSON and text. host.ts copies per-device names and memory from GPU detection. `pr…
Out of Scope Changes check Passed The reviewed changes remain within issue #13019. Per-device snapshot copying, indexed evidence, text rendering, evidence-budget handling, count validation, schema support, and regression tests directl…
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main changes: reporting GPU inventory and disclosing devices omitted by the public evidence limit.
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@github-code-quality

github-code-quality Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Code coverage overview

Languages: TypeScript

TypeScript / code-coverage/plugin

main 63002cd fix/13019-host-probe... 7a25ef9 +/-
96% 97% +1%
View coverage of the 10 most impacted files.
File main 63002cd fix/13019-host-probe... 7a25ef9 +/-
nemoclaw/src/onboard/config.ts 98% 96% -2%
nemoclaw/src/index.ts 94% 93% -1%
nemoclaw/src/bl...print/runner.ts 95% 95% 0%
nemoclaw/src/bl...ime-identity.ts 97% 97% 0%
nemoclaw/src/bl...t-management.ts 100% 100% 0%
nemoclaw/src/co.../config-show.ts 100% 100% 0%
nemoclaw/src/commands/slash.ts 100% 100% 0%
nemoclaw/src/ru...time-context.ts 100% 100% 0%
nemoclaw/src/sh...er-boundary.cts 100% 100% 0%
nemoclaw/src/on...native-route.ts 0% 100% +100%

TypeScript / code-coverage/cli

main 63002cd fix/13019-host-probe... 7a25ef9 +/-
84% 86% +2%
View coverage of the 10 most impacted files.
File main 63002cd fix/13019-host-probe... 7a25ef9 +/-
src/lib/onboard.ts 62% 50% -12%
src/lib/actions...ess-recovery.ts 65% 81% +16%
src/lib/state/l...diness-lease.ts 66% 83% +17%
src/lib/inferen...ollama/proxy.ts 41% 61% +20%
src/lib/onboard.../application.ts 55% 84% +29%
src/lib/domain/...y/dimensions.ts 0% 71% +71%
src/lib/inferen...er/lifecycle.ts 0% 72% +72%
src/lib/actions...ry/operation.ts 0% 76% +76%
src/lib/actions...try/snapshot.ts 0% 78% +78%
src/lib/inferen...ovider/index.ts 0% 88% +88%

ℹ️ Calculations represent line coverage. Results may change as calculations are updated.


Updated October 21, 2026 05:45 UTC

Use the current E2E assertion budget and its matching runtime tests.
The GPU reporting change stays unchanged.

Signed-off-by: Dongni Yang <dongniy@nvidia.com>
@Dongni-Yang
Dongni-Yang marked this pull request as ready for review October 13, 2026 03:39
@Dongni-Yang Dongni-Yang removed their assignment Oct 13, 2026
Preserve referenced and non-GPU evidence within the existing report limit.

Expose bounded omission counts without replacing existing diagnostic details.

Closes #13019

Signed-off-by: Dongni Yang <dongniy@nvidia.com>
@Dongni-Yang Dongni-Yang changed the title fix(cli): report individual GPUs in host probe fix(cli): report GPU inventory and disclose omitted devices Oct 15, 2026
Signed-off-by: Dongni Yang <dongniy@nvidia.com>
@github-actions

Copy link
Copy Markdown
Contributor

PR Review Advisor finished for commit 7a25ef9. Include the Advisor findings in the complete PR feedback collection. Verify and group valid findings before repair.

Request review only when Require no Advisor blockers is green.

All previous runs

@rsliter rsliter left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed latest PR commit 7a25ef923fe433a6a342efed3721952a7965c87a against base 46888639a2bf2727b6703c9beabe0ec8670db1ef.

The change preserves the existing readiness decision and evidence limits while exposing bounded per-GPU name and memory details. Required evidence remains ahead of optional GPU rows, omitted-device counts stay report-owned and validated, repeated sanitization is stable, and the text path receives escaped, length-bounded names. The regression suite covers asymmetric inventories, 255/256/257-device boundaries, required-evidence saturation, malformed summaries, stale or failed collection, and terminal controls. I found no blocking correctness, compatibility, or security issue. All nine security categories pass; the change adds no credential, authorization, dependency, cryptography, policy, or mutation surface.

Required checks pass on this commit. PR Review Advisor evaluated this exact commit and base, reported no findings, and recommended no additional E2E selectors because the changed boundary has deterministic coverage. The exact-commit self-hosted qualification, including the generic NVIDIA GPU lane, also passed.

@github-actions github-actions Bot added v0.0.138 Release target and removed v0.0.137 labels Oct 21, 2026

This branch has not been deployed

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

Labels

v0.0.138 Release target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[DGX Station][CLI&UX] host probe collapses two GPUs into one memory figure and omits the second device

2 participants