feat(deployment): show a gpu interconnect indicator on deployment views - #3571
feat(deployment): show a gpu interconnect indicator on deployment views#3571baktun14 wants to merge 2 commits into
Conversation
Surface GPU interconnect on the deployment detail and list views, derived from the on-chain group placement requirement (capabilities/gpu-interconnect), mirroring how confidential compute (tee/type) is surfaced. Reflects the pinned fabric (InfiniBand/RoCE) when present. No query or DTO changes. Closes CON-691
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR adds GPU interconnect parsing, aggregation, and display support. Deployment details and list rows now show badges for enabled interconnects and selected fabrics. ChangesGPU interconnect display
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
apps/deploy-web/src/utils/gpuInterconnect.spec.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. apps/deploy-web/src/utils/gpuInterconnect.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3571 +/- ##
==========================================
- Coverage 75.90% 75.56% -0.35%
==========================================
Files 1159 1047 -112
Lines 30270 27295 -2975
Branches 7529 6920 -609
==========================================
- Hits 22976 20625 -2351
+ Misses 6432 5868 -564
+ Partials 862 802 -60
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@apps/deploy-web/src/components/shared/GpuInterconnectBadge.tsx`:
- Around line 53-57: Update the compact branch of GpuInterconnectBadge so it
renders only the Waypoints icon visibly, while retaining “Interconnect” as an
sr-only accessible label. Adjust the compact-mode test to assert that the label
is not visible and preserve the non-compact rendering behavior.
In `@apps/deploy-web/src/utils/gpuInterconnect.ts`:
- Around line 54-55: Update the fabricAttribute selection in the GPU
interconnect utility to accept only attributes whose key is a string, starts
with GPU_INTERCONNECT_FABRIC_PREFIX, and has a non-empty suffix, while retaining
the ENABLED_VALUE check. Add regression tests covering a non-string key and an
empty suffix preceding a valid fabric attribute.
🪄 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: Pro
Run ID: 454f579e-cdf7-420b-88cc-798b3d40e091
📒 Files selected for processing (9)
apps/deploy-web/src/components/deployments/DeploymentDetail.tsxapps/deploy-web/src/components/deployments/DeploymentListRow.tsxapps/deploy-web/src/components/deployments/DeploymentSubHeader.tsxapps/deploy-web/src/components/shared/GpuInterconnectBadge.spec.tsxapps/deploy-web/src/components/shared/GpuInterconnectBadge.tsxapps/deploy-web/src/hooks/useDeclaredGpuInterconnect.spec.tsapps/deploy-web/src/hooks/useDeclaredGpuInterconnect.tsapps/deploy-web/src/utils/gpuInterconnect.spec.tsapps/deploy-web/src/utils/gpuInterconnect.ts
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — a well-scoped, additive UI indicator that closely mirrors the existing ConfidentialCompute read pattern.
What was reviewed:
- The on-chain attribute reader (
getGroupGpuInterconnect/getDeclaredGpuInterconnect) — capability/fabric parsing, orphan-fabric-pin handling, and malformed-input safety, all backed by matching unit tests. - The hook and badge wiring into
DeploymentDetail,DeploymentSubHeader, andDeploymentListRow— confirmedgroupsis already present on the DTOs used by both views, so no query/DTO changes were needed. - The compact vs. full badge rendering paths, verified against the spec's label/fabric/compact/disabled variants.
Extended reasoning...
Overview
This PR adds a GPU interconnect indicator to the deployment detail and list views, following the same pure-reader → memoized-hook → DI-injected-badge pattern already used for the Confidential Compute (TEE) badge. New files: utils/gpuInterconnect.ts (reader), hooks/useDeclaredGpuInterconnect.ts (hook), components/shared/GpuInterconnectBadge.tsx (badge), plus specs for each. Existing files (DeploymentDetail.tsx, DeploymentSubHeader.tsx, DeploymentListRow.tsx) are updated only to wire the new hook/badge in alongside the existing TEE badge.
Security risks
None identified. This is a pure read-only UI feature over already-fetched on-chain deployment data (group_spec.requirements.attributes), with no new network calls, no user input, no auth/permission logic, and no DTO/query changes.
Level of scrutiny
Low-to-moderate is appropriate: this is presentation-layer code that mirrors an established, already-shipped pattern (ConfidentialComputeBadge) almost exactly. The reader functions are pure, defensively coded against malformed/missing data (verified by tests), and ungated by design (matching the TEE precedent and the PR's stated on-chain-authoritative rationale).
Other factors
Test coverage is thorough: the reader, hook, and badge each have dedicated specs covering enabled/disabled, single/multiple fabrics, compact mode, and malformed-input cases. The diff is small and self-contained (9 files, mostly new), doesn't touch CODEOWNER-sensitive or critical backend paths, and the wiring into the three consuming components is minimal and consistent with how teeTypes/ConfidentialComputeBadge are already threaded through. No outstanding review comments exist on this PR.
Guard the fabric-pin lookup against a non-string attribute key (which would throw on startsWith) and against an empty fabric suffix matching before a valid pin. Keeps the reader's never-throws contract and picks the real fabric. Ref CON-691
There was a problem hiding this comment.
LGTM. Straightforward, well-tested feature that mirrors the existing ConfidentialComputeBadge read pattern.
What was reviewed: the on-chain attribute parsing/aggregation in utils/gpuInterconnect.ts, the memoized hook, the badge's compact/full/fabric rendering variants, and the wiring into DeploymentDetail, DeploymentListRow, and DeploymentSubHeader. Checked CodeRabbit's flagged malformed-fabric-key concern against the current code — the typeof and non-empty-suffix guards are already in place, so it's not live. Also checked CodeRabbit's compact-badge-should-be-icon-only suggestion — the compact chip intentionally shows a short visible "Interconnect" label (not icon-only), which is asserted by its own test, so this is a design choice rather than a bug.
Extended reasoning...
Overview
The PR adds a GPU interconnect indicator to the deployment detail and list views, mirroring the existing Confidential Compute (TEE) badge pattern: a pure reader (utils/gpuInterconnect.ts) that parses on-chain group_spec.requirements.attributes for the capabilities/gpu-interconnect flag and any pinned fabric, a thin memoized hook (useDeclaredGpuInterconnect), and a presentational badge component with compact/full variants. Three existing view components (DeploymentDetail, DeploymentListRow, DeploymentSubHeader) are updated to thread the new data through and render the badge. No API/DTO changes — the views already carry groups via the existing deploymentToDto mapper.
Security risks
None identified. This is a pure read/display feature over data the client already has (on-chain deployment groups); there's no new data fetching, no auth/permission logic, and no user input parsing beyond defensively reading attribute arrays that already exist in the app's type surface.
Level of scrutiny
Low-to-moderate. It's an additive, ungated UI feature (no feature flag, matching the TEE badge precedent) touching non-critical display code, not a core transaction or auth path. The reader functions are defensive (never throw on malformed/missing input) and this is verified with explicit malformed-input tests.
Other factors
Patch coverage is 100% on all three new files (reader, hook, badge) per Codecov, with tests covering label/fabric/compact/disabled variants and malformed attribute edge cases (non-string key, empty fabric suffix, orphaned fabric pin). CodeRabbit raised two automated suggestions: one (malformed fabric key guard) is already satisfied by the shipped code, and the other (compact badge should be icon-only) reflects a debatable a11y preference rather than a functional bug — the current behavior is deliberate and test-covered. No outstanding unaddressed human or bot review comments block this PR.
1 participant
Why
Once a deployment is running, users should be able to see at a glance that GPU interconnect is active — the same way we already surface confidential compute (TEE). This reads from authoritative on-chain deployment data, so no stored SDL is required.
Closes CON-691
What
Surfaces a GPU interconnect indicator on the deployment detail and list views when the deployment's on-chain group requirements include the interconnect capability, reflecting the pinned fabric when present.
Mirrors the existing Confidential Compute read pattern (pure reader → thin hook → DI-injected badge rendered next to
ConfidentialComputeBadge):utils/gpuInterconnect.ts— reads the on-chain placement requirementcapabilities/gpu-interconnect: "true"fromgroup_spec.requirements.attributes(the same array the TEE badge reads fortee/type) and derives the pinned fabric from anycapabilities/gpu-interconnect/fabric/<fabric>pin. Reuses the constants already defined inutils/sdl/gpuInterconnect.ts; ignores orphan fabric pins; never throws on missing/malformed data.hooks/useDeclaredGpuInterconnect.ts— memoized wrapper over the deployment's groups.components/shared/GpuInterconnectBadge.tsx— one badge, two forms: full-text (GPU Interconnect (InfiniBand)) on the detail view, and a compact icon + tooltip chip on the narrow list-row name cell. Fabric shown when pinned (InfiniBand/RoCE), otherwise provider-chosen.Behavior
Notes
ConfidentialComputeBadgeprecedent and the authoritative-on-chain rationale — it also catches raw-SDL interconnect deployments.groupsthrough the shareddeploymentToDtomapper.mainyet, but the reader already handles pinned fabrics produced by a hand-written/raw interconnect SDL.Tests: unit specs for the reader, hook, and badge (all label/fabric/compact/disabled variants, malformed-input safety), driven off realistically seeded on-chain groups.
Summary by CodeRabbit
New Features
Tests