fix(inference): recognize Spark dual-controller QSFP

[deepujain]

Summary

Recognize the verified DGX Spark dual-controller PCI pair as one physical QSFP connector. Before this change, managed-cluster discovery rejected the shipped two-rail stacking topology as multiple physical ports; after it, the pair qualifies while other distinct-port combinations remain rejected.

Related Issue

Fixes #8520

Changes

• Map the verified 0000:01:00.0 and 0002:01:00.0 ConnectX-7 functions to one DGX Spark physical-port identity during host qualification.
• Preserve the existing fail-closed behavior for every other PCI/physical-port combination.
• Include observed physical-port IDs in topology rejection diagnostics.
• Add discovery and topology regression coverage for both accepted and rejected layouts.

Type of Change

• Code change (feature, bug fix, or refactor)
• Code change with doc updates
• Doc only (prose changes, no code sample modifications)
• Doc only (includes code sample changes)

Quality Gates

• Tests added or updated for changed behavior
• Existing tests cover changed behavior — justification:
• Tests not applicable — justification:
• Docs updated for user-facing behavior changes
• Docs not applicable — justification: This corrects internal hardware qualification without changing commands, configuration, defaults, or the support matrix.
• Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging)
• Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification:
• Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Documentation Writer Review

• Documentation writer subagent reviewed the completed changes
• Result: no-docs-needed
• Evidence: The final diff only changes internal managed-cluster qualification and its tests; no user-facing contract or support claim changes.
• Agent: Codex Desktop

DGX Station Hardware Evidence

• Tested on DGX Station
• Tested commit: Not applicable; scripts/prepare-dgx-station-host.sh is unchanged.
• Station profile/scenario: Not applicable.
• Result: Not applicable.
• Supporting evidence: Not applicable.

Verification

• PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub
• Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run validate:pr passed after refreshing origin/main when hooks were skipped or unavailable
• Targeted behavior tests pass for the current change set, or tests are marked not applicable above — build:cli, the two-file run (62 passed), and pre-push typecheck:cli passed at contributor head 06caa6982; exact head 10822e338 adds only the verified upstream branch update.
• Applicable broad gate passed — repository CI is the pending broad gate.
• Quality Gates section completed with required justifications or waivers
• No secrets, API keys, or credentials committed
• npm run docs builds without warnings (doc changes only)
• Doc pages follow the style guide (doc changes only)
• New doc pages include SPDX header and frontmatter (new pages only)

---

Signed-off-by: Deepak Jain deepujain@gmail.com

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[coderabbitai]

📝 Walkthrough

Walkthrough

The detector treats the DGX Spark dual-controller PCI rail pair as one QSFP port. Topology validation reports observed physical-port identifiers when candidate rails do not match.

Changes

DGX Spark QSFP detection

Layer / File(s)	Summary
Normalize DGX Spark QSFP rails src/lib/inference/serving/managed-cluster-discovery.ts, src/lib/inference/serving/managed-cluster-discovery.test.ts	qualifyHost maps PCI addresses 0000:01:00.0 and 0002:01:00.0 to dgx-spark-qsfp-near-rj45. Tests cover matching and differing physical-port layouts.
Report physical-port mismatches src/lib/inference/serving/managed-cluster-topology.ts, src/lib/inference/serving/managed-cluster-topology.test.ts	The fabric-multiple diagnostic lists sorted observed physical-port identifiers. Tests cover local and peer nodes with differing ports.

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

Suggested labels: bug-fix

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes satisfy issue #8520 by mapping the two Spark PCI functions, preserving rejection behavior, and improving per-node diagnostics.
Out of Scope Changes check	✅ Passed	All changes support issue #8520 through the implementation, regression tests, and topology diagnostics.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely summarizes the main change: recognizing the DGX Spark dual-controller QSFP topology.

✨ 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

🤖 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 `@src/lib/inference/serving/managed-cluster-topology.test.ts`:
- Around line 479-487: Update the test case around qualificationInput so the
physical-port IDs are assigned in reverse order relative to the expected
message. Keep the expected fabric-multiple message sorted lexicographically,
ensuring qualifyManagedClusterTopology must sort observed port identities for
the assertion to pass.

🪄 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: d371fc4d-cd68-4cf2-9a1b-5402a0a7d654

📥 Commits

Reviewing files that changed from the base of the PR and between 5429cec and fd2ec56.

📒 Files selected for processing (4)

• src/lib/inference/serving/managed-cluster-discovery.test.ts
• src/lib/inference/serving/managed-cluster-discovery.ts
• src/lib/inference/serving/managed-cluster-topology.test.ts
• src/lib/inference/serving/managed-cluster-topology.ts

[github-actions]

PR Review Advisor — No blocking findings reported

Advisor assessment: No blocking advisor findings reported
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions

Model lanes

• GPT-5.6 Terra (primary): Completed · medium confidence · 0 blockers · 0 warnings · 0 suggestions
• Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
• Model comparison: normalized findings match; normalized terminology decisions differ; normalized E2E selections differ; severity counts match.

2 terminology differences from the second opinion

Advisory only. These are normalized differences from the primary terminology receipt.

• physicalPortId at src/lib/inference/serving/managed-cluster-discovery.test.ts:1004: selected only by the second-opinion lane as established.
• dgx-spark-qsfp-near-rj45 at src/lib/inference/serving/managed-cluster-discovery.ts:653: selected only by the second-opinion lane as justified.

2 additional E2E selections from the second opinion

Advisory only. The primary lane did not select these E2E jobs or targets.

• llama-cpp-dgx-spark-qualification: The completed second-opinion lane identified E2E coverage that the primary lane omitted.
• spark-install: The completed second-opinion lane identified E2E coverage that the primary lane omitted.

Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests.

3 semantic terminology decisions

Terminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.

• established — physical port at src/lib/inference/serving/managed-cluster-topology.ts:361: Retain "physical port" for the connector identity in the diagnostic.
• established — logical rail at src/lib/inference/serving/managed-cluster-topology.ts:361: Retain "logical rail" to distinguish interface observations from the physical port.
• justified — dual-controller at src/lib/inference/serving/managed-cluster-discovery.test.ts:997: Retain "dual-controller" because the tests define its contrast with other two-rail pairs.

E2E guidance

Advisory only. A maintainer can dispatch the default E2E suite against this exact revision.

Recommended E2E: inference-routing, network-policy

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

[cv]

src/lib/inference/serving/managed-cluster-topology.ts:361 reports the observed physical-port IDs but does not identify which node failed qualification. Because validateNodeRails returns the same message for the controller and every peer, users cannot determine which DGX Spark has the incorrect cable topology. This does not satisfy issue #8520's accepted requirement to name the per-node observation. Include an unambiguous node identifier in this failure and add regression tests that trigger the mismatch on the local node and a peer, asserting the correct node and sorted physical-port IDs.

[deepujain]

The diagnostic now identifies the failing node, with regression coverage for both local and peer nodes and deterministic port ordering.

[deepujain]

Rebased on current main. The 62 focused tests and CLI typecheck still pass. @coderabbitai review

[coderabbitai]

@deepujain I will review the rebased changes and the reported test coverage.

⚠️ Action not completed

No files to review.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[deepujain]

The local and peer mismatch cases now identify the failing node and report sorted physical-port IDs. The branch is current with main, and the focused 62-test run plus CLI typecheck pass. Could you take another look at 10822e3?