fix(installer): restore GPU qualification and align PR E2E - #8748
Conversation
|
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. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR aligns trusted E2E selector validation with advisor recommendations. It adds ChangesPR E2E alignment
Installer preflight coverage
NVIDIA driver detection
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant PRReviewAdvisor
participant TrustedInventory
participant RiskPlan
participant E2EController
PRReviewAdvisor->>TrustedInventory: Load trusted E2E job IDs
TrustedInventory->>RiskPlan: Classify planning and manual-only jobs
RiskPlan-->>TrustedInventory: Return selector categories
TrustedInventory-->>PRReviewAdvisor: Provide validated recommendations
PRReviewAdvisor->>E2EController: Dispatch supported exact-revision selectors
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
4 terminology differences from the second opinionAdvisory only. These are normalized differences from the primary terminology receipt.
Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests. 2 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. A maintainer can dispatch the default E2E suite against this exact revision. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
c7aec2d to
948717d
Compare
Signed-off-by: Prekshi Vyas <34834085+prekshivyas@users.noreply.github.com>
cv
left a comment
There was a problem hiding this comment.
There's no "inexact-revision". Please review WRITING.md and follow it across all communications and docs.
Signed-off-by: Prekshi Vyas <34834085+prekshivyas@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@test/pr-review-advisor-normalization.test.ts`:
- Around line 216-239: Strengthen the test for buildComment so it verifies that
the manual-only network-policy test is absent from the recommended E2E section,
not merely present in the manual-only section. Add an assertion against the
recommended-section content or otherwise validate that network-policy does not
appear there while preserving the existing inference-routing and manual-only
checks.
🪄 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: 05f79eb3-684c-405e-848b-6a6ec259a5f7
📒 Files selected for processing (9)
.github/workflows/e2e.yamltest/e2e/README.mdtest/e2e/support/e2e-operations-workflow-boundary.test.tstest/pr-review-advisor-normalization.test.tstools/advisors/e2e-recommendations.mtstools/advisors/risk-plan.mtstools/e2e/operations-workflow-boundary.mtstools/e2e/workflow-boundary.mtstools/pr-review-advisor/comment.mts
Signed-off-by: Prekshi Vyas <34834085+prekshivyas@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@test/package-contract/installer-host-preflight.test.ts`:
- Around line 80-92: Extend the installer preflight rejection tests around the
onboard admission fixture to return an unknown finding ID and a nonempty
capability ID, then assert both identifiers appear in the installer output.
Ensure the assertions validate fallback diagnostics directly without relying on
finding-summary mappings, covering the finding-ID and required-capability output
paths in the admission rejection flow.
In `@tools/advisors/risk-plan.mts`:
- Line 32: Replace the duplicated PR planning selector lists used by
PR_E2E_PLANNING_JOB_IDS and the e2e workflow with one machine-readable canonical
contract consumed by both layers; if direct workflow consumption is unavailable,
add a deterministic parity test that compares them and fails on any difference.
Preserve the shared contract between the Advisor and trusted-main controller,
including the current selectors.
🪄 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: 8c8ee41b-321d-4278-b372-10cdf250d409
📒 Files selected for processing (11)
.github/workflows/pr-self-hosted.yamlscripts/install.shtest/e2e/README.mdtest/e2e/support/pr-self-hosted-llama-selector.test.tstest/package-contract/installer-host-preflight.test.tstest/pr-review-advisor-normalization.test.tstest/pr-risk-plan.test.tstools/advisors/e2e-recommendations.mtstools/advisors/risk-plan.mtstools/pr-review-advisor/analyze.mtstools/pr-review-advisor/comment.mts
🚧 Files skipped from review as they are similar to previous changes (3)
- test/pr-review-advisor-normalization.test.ts
- test/e2e/README.md
- tools/pr-review-advisor/comment.mts
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Summary
Non-interactive installation now continues to onboarding when NemoClaw can repair a supported Docker storage configuration. NVIDIA driver detection now accepts valid two-part versions such as
595.84, so newer GPU hosts satisfy the declarative driver floor. PR E2E guidance now preserves the broad trusted Advisor inventory while the manual PR controller remains limited to its two safe explicit selectors, and installer/readiness changes select the generic NVIDIA GPU qualification job.Together, these changes remove two infrastructure blockers that prevented the GPU lane from reaching model startup. Tomorrow's Lightning 3.5 integration can therefore use the installer and GPU qualification path without misclassifying a repairable storage setup or a valid NVIDIA driver as a host failure.
Related Issue
Fixes #8681
Changes
inference-routingandmanaged-image-protected-runtime.Type of Change
Quality Gates
Documentation Writer Review
docs-updatedtest/e2e/README.mdDGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailablenpm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run docsbuilds without warnings (doc changes only)Additional validation:
npm run typecheck:clinpm run checks:repositorynpm run test:e2e-phases:checknpm run test-size:checknpm run test:changedgit diff --checkLive NVIDIA GPU E2E passed for final head
d62bb82e8: generic NVIDIA GPU job.Signed-off-by: Prekshi Vyas 34834085+prekshivyas@users.noreply.github.com
Summary by CodeRabbit
New Features
inference-routingas a supported manual E2E validation target.Bug Fixes