feat(deployment): show placements and services on the redesigned Details tab - #3586
feat(deployment): show placements and services on the redesigned Details tab#3586ygrishajev wants to merge 1 commit into
Conversation
…ils tab Replace the flat lease-card list on the redesigned deployment detail page with a placements -> services overview. Each placement lists its service count, aggregate GPU/vCPU/memory/storage and provider + region, and holds collapsible services. Expanding a service reveals its resources, Docker image, environment variables, commands and live endpoints (URIs, forwarded ports, IPs) with their copy/open actions. Confidential-compute and reclamation states stay visible. Per-service SDL detail is parsed defensively so live status still renders when the local manifest is missing. The legacy page and LeaseRow are left untouched for a clean teardown at rollout.
📝 WalkthroughWalkthroughThe deployment details view now shows placement cards instead of lease rows. New components parse deployment manifests, display placement and service data, render endpoint links, and expose service details. Unit and UI tests cover the new flow. ChangesDeployment placements
Estimated code review effort: 4 (Complex) | ~45 minutes ✨ 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/components/deployments/DeploymentDetail/DeploymentDetail.spec.tsxESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. apps/deploy-web/src/components/deployments/DeploymentDetail/DeploymentDetail.tsxESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. apps/deploy-web/src/components/deployments/DeploymentDetail/DeploymentPlacements/DeploymentPlacements.spec.tsxESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox.
Comment |
❌ 2 Tests Failed:
View the top 2 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
ygrishajev
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/deploy-web/src/components/deployments/DeploymentDetail/DeploymentPlacements/PlacementServiceRow.tsx (1)
93-95: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winService sizes use raw SDL units while the placement card uses normalized units.
formatSizeprints the manifest value and unit verbatim, for example512 Miand10 Gi.buildPlacementStatsinPlacementCard.tsxformats the same class of value throughbytesToShrink, producing1.05 GBand11.27 GB. Both appear in the same card, so memory and storage read in two different unit systems. Consider converting the manifest size to bytes and reusingbytesToShrinkfor one consistent presentation.🤖 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 `@apps/deploy-web/src/components/deployments/DeploymentDetail/DeploymentPlacements/PlacementServiceRow.tsx` around lines 93 - 95, Update formatSize to convert the manifest resource value and unit into bytes, then format the result with the existing bytesToShrink helper used by buildPlacementStats in PlacementCard. Preserve the "--" fallback for missing sizes and keep the display consistent for both memory and storage values.
🤖 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/deployments/DeploymentDetail/DeploymentPlacements/placementModel.ts`:
- Around line 38-53: Update parseManifestServices to resolve each service’s
compute profile through the deployment[name] reference before reading
profiles.compute, rather than using the service name directly. Use the
referenced profile name for parseComputeResources, while preserving the existing
fallback behavior when deployment or the profile reference is absent.
---
Nitpick comments:
In
`@apps/deploy-web/src/components/deployments/DeploymentDetail/DeploymentPlacements/PlacementServiceRow.tsx`:
- Around line 93-95: Update formatSize to convert the manifest resource value
and unit into bytes, then format the result with the existing bytesToShrink
helper used by buildPlacementStats in PlacementCard. Preserve the "--" fallback
for missing sizes and keep the display consistent for both memory and storage
values.
🪄 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: 61b85573-8604-45fa-86ff-8d8b139806c3
📒 Files selected for processing (14)
apps/deploy-web/src/components/deployments/DeploymentDetail/DeploymentDetail.spec.tsxapps/deploy-web/src/components/deployments/DeploymentDetail/DeploymentDetail.tsxapps/deploy-web/src/components/deployments/DeploymentDetail/DeploymentPlacements/DeploymentPlacements.spec.tsxapps/deploy-web/src/components/deployments/DeploymentDetail/DeploymentPlacements/DeploymentPlacements.tsxapps/deploy-web/src/components/deployments/DeploymentDetail/DeploymentPlacements/PlacementCard.spec.tsxapps/deploy-web/src/components/deployments/DeploymentDetail/DeploymentPlacements/PlacementCard.tsxapps/deploy-web/src/components/deployments/DeploymentDetail/DeploymentPlacements/PlacementServiceRow.spec.tsxapps/deploy-web/src/components/deployments/DeploymentDetail/DeploymentPlacements/PlacementServiceRow.tsxapps/deploy-web/src/components/deployments/DeploymentDetail/DeploymentPlacements/PlacementStats.tsxapps/deploy-web/src/components/deployments/DeploymentDetail/DeploymentPlacements/ServiceEndpoints.spec.tsxapps/deploy-web/src/components/deployments/DeploymentDetail/DeploymentPlacements/ServiceEndpoints.tsxapps/deploy-web/src/components/deployments/DeploymentDetail/DeploymentPlacements/placementModel.spec.tsapps/deploy-web/src/components/deployments/DeploymentDetail/DeploymentPlacements/placementModel.tsapps/deploy-web/tests/ui/deployment-detail-preview.spec.ts
| export function parseManifestServices(manifest: string | null | undefined): Record<string, ManifestServiceDetail> { | ||
| const parsed = safeLoadYaml(manifest); | ||
| const services = parsed?.services; | ||
| if (!services || typeof services !== "object") return {}; | ||
|
|
||
| return Object.keys(services).reduce<Record<string, ManifestServiceDetail>>((all, name) => { | ||
| const service = services[name] ?? {}; | ||
| all[name] = { | ||
| image: typeof service.image === "string" ? service.image : undefined, | ||
| resources: parseComputeResources(get(parsed, ["profiles", "compute", name, "resources"])), | ||
| env: parseEnv(service.env), | ||
| command: joinCommand(service.command, service.args) | ||
| }; | ||
| return all; | ||
| }, {}); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate what deploymentManifest holds and how profiles are referenced elsewhere.
rg -nP --type=ts --type=tsx -C4 '\bdeploymentManifest\b' | head -80
rg -nP --type=ts -C6 'profiles\W+compute|\.profile\b' apps/deploy-web/src/utils/sdl | head -120
fd -t f 'sdlImport' apps/deploy-web/src | xargs -r rg -nP -C4 'profiles|profile\b' | head -80Repository: akash-network/console
Length of output: 16618
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- CLAUDE files ---'
fd -HI -t f 'CLAUDE.md' -a -x sh -c 'echo "--- $1"; sed -n "1,220p" "$1"' sh {}
printf '%s\n' '--- placement model and direct callers ---'
fd -t f 'placementModel.ts' -a -x sh -c 'echo "--- $1"; cat -n "$1"' sh {}
rg -n -C5 'parseManifestServices|deploymentManifest' --glob '*.ts' --glob '*.tsx' .
printf '%s\n' '--- SDL deployment/profile definitions ---'
rg -n -C8 'deployment:|profile:|profiles:|compute:' apps/deploy-web/src --glob '*.ts' --glob '*.tsx' | head -240Repository: akash-network/console
Length of output: 50379
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- manifest storage and schema ---'
rg -n -C8 'deploymentLocalStorage|get\(.*manifest|manifest:' apps/deploy-web/src --glob '*.ts' --glob '*.tsx' | head -220
printf '%s\n' '--- generator service/profile/deployment mapping ---'
sed -n '100,235p' apps/deploy-web/src/utils/sdl/sdlGenerator.ts
rg -n -C10 'sdl\.deployment|deployment\[|profile:' apps/deploy-web/src/utils/sdl/sdlGenerator.ts apps/deploy-web/src/utils/sdl/sdlImport.ts
printf '%s\n' '--- placement parser test helper ---'
sed -n '1,75p' apps/deploy-web/src/components/deployments/DeploymentDetail/DeploymentPlacements/placementModel.spec.ts
printf '%s\n' '--- representative SDL fixtures ---'
rg -n -C5 'deployment:|profile:' apps/deploy-web/src --glob '*.yaml' --glob '*.yml' --glob '*.ts' --glob '*.tsx' | head -180Repository: akash-network/console
Length of output: 50377
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- deploymentLocalStorage implementation ---'
rg -n -C10 'deploymentLocalStorage' . --glob '!node_modules/**' --glob '!*.map' | head -220
printf '%s\n' '--- local deployment data types and writers ---'
rg -n -C8 'setDeploymentData|saveDeployment|localStorage.*manifest|manifest.*sdl|interface DeploymentData|type DeploymentData' apps/deploy-web/src packages --glob '*.ts' --glob '*.tsx' | head -260
printf '%s\n' '--- direct behavioral probe of the current lookup ---'
node - <<'JS'
const yaml = require("js-yaml");
const get = require("lodash/get");
const manifest = yaml.dump({
version: "2.0",
services: { web: { image: "nginx" } },
profiles: {
compute: {
custom: { resources: { cpu: { units: 2 } } }
}
},
deployment: {
web: {
dcloud: { profile: "custom", count: 1 }
}
}
});
const parsed = yaml.load(manifest);
console.log(JSON.stringify({
serviceNameLookup: get(parsed, ["profiles", "compute", "web", "resources"]),
deploymentProfile: get(parsed, ["deployment", "web", "dcloud", "profile"]),
profileLookup: get(parsed, ["profiles", "compute", get(parsed, ["deployment", "web", "dcloud", "profile"]), "resources"])
}, null, 2));
JSRepository: akash-network/console
Length of output: 43192
Resolve the compute profile through deployment[name]. If the profile name differs from the service name, the current lookup returns no resources and the UI shows empty resource stats. The stored deploymentManifest is the SDL document, so read the referenced profile from deployment[name] before accessing profiles.compute.
🤖 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
`@apps/deploy-web/src/components/deployments/DeploymentDetail/DeploymentPlacements/placementModel.ts`
around lines 38 - 53, Update parseManifestServices to resolve each service’s
compute profile through the deployment[name] reference before reading
profiles.compute, rather than using the service name directly. Use the
referenced profile name for parseComputeResources, while preserving the existing
fallback behavior when deployment or the profile reference is absent.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| export function getServiceStatus(service: Pick<LeaseServiceStatus, "available" | "total" | "ready_replicas"> | undefined, leaseState: LeaseDto["state"]): ServiceStatusView { | ||
| if (leaseState === "closed") return { label: "Closed", tone: "closed" }; | ||
| if (service && service.available > 0) return { label: "Running", tone: "running" }; | ||
| return { label: "Starting", tone: "pending" }; | ||
| } |
There was a problem hiding this comment.
🟡 getServiceStatus (placementModel.ts:82-86) only treats leaseState === "closed" as terminal, but isProviderReclaimed(lease) (reclamationUtils.ts:101-104) can be true while lease.state is still "active" — it fires on reclamation.startedAt or group.state === "paused", independent of the lease's own state field. In that window every service row on PlacementCard renders amber "Starting" while the ReclamationCard banner above it says the deployment "was stopped by the provider, so it's no longer running." getServiceStatus should also treat a provider-reclaimed lease as closed.
Extended reasoning...
The bug: getServiceStatus (placementModel.ts:82-86) derives a service's displayed status purely from leaseState, treating only the literal string "closed" as terminal:
export function getServiceStatus(service, leaseState) {
if (leaseState === "closed") return { label: "Closed", tone: "closed" };
if (service && service.available > 0) return { label: "Running", tone: "running" };
return { label: "Starting", tone: "pending" };
}Meanwhile PlacementCard.tsx decides whether to render the terminal ReclamationCard banner using a different signal: isProviderReclaimed(lease) from reclamationUtils.ts:101-104, which is !isReclaiming(lease) && (hasReclamationStarted(lease) || lease.group?.state === "paused"). Neither hasReclamationStarted (reclamation.startedAt > 0) nor group.state === "paused" requires lease.state === "closed" — they are independent chain fields set as soon as the provider reclaims, while the lease record itself can still read "active" until the on-chain close is finalized.
Why this isn't just theoretical: ReclamationCard.tsx's own doc comment describes exactly this state: 'Reclamation is terminal — there is no restart — so recovery is Close (recover any escrow still locked in the active-but-dead deployment) + Redeploy.' The card's primary action is literally a 'Close & refund' button that submits a CloseDeploymentMsg — an action that only makes sense if the deployment/lease has not yet been closed on-chain. If lease.state were already "closed" whenever isProviderReclaimed fires, this button (and the whole card) would be closing something already closed.
The contradiction this produces: In PlacementCard.tsx, isLeaseActive = isLeaseLive(lease) is true for state === "active", so useLeaseStatus stays enabled and keeps polling. Since the provider has torn down the workload, the status query returns available: 0 for every service (or 404s, falling back to manifest service names with service: undefined). getServiceStatus(service, "active") then falls through both branches to { label: "Starting", tone: "pending" } for every row — rendered directly beneath the terminal ReclamationCard banner that says the deployment 'is no longer running'. A user sees amber 'Starting' dots suggesting their service is coming back up, right under a banner telling them the opposite.
Step-by-step proof:
- A lease is bid with an AEP-82 reclamation window; provider later reclaims it. The chain sets
lease.reclamation.started_at(and/or transitions the deployment group to"paused") before the lease record itself is marked"closed". leaseToDto(deploymentDetailUtils.ts:72-104) copieslease.lease.state(still"active") andlease.lease.reclamation.startedAtstraight from the chain response — these are independent fields, so the DTO ends up withstate: "active"andreclamation.startedAt > 0simultaneously.PlacementCardcomputesisReclaimed = isProviderReclaimed(lease)=true(not reclaiming, andhasReclamationStartedis true) and renders<ReclamationCard>.isLeaseActive = isLeaseLive(lease)=true(state is"active"), souseLeaseStatusruns; the provider has no workload left, soavailable: 0for every service in the response.- Each row calls
getServiceStatus(service, lease.state)=getServiceStatus({available: 0}, "active")→{label: "Starting", tone: "pending"}, shown right under the ReclamationCard.
Addressing the refutation: One reviewer argued this window can't occur because 'whenever isProviderReclaimed is true, lease.state must already be closed,' reasoning that the two OR-branches only fire alongside a close. That's not backed by the code: hasReclamationStarted/group.state === "paused" are read straight off independent chain fields with no code path that atomically forces lease.state to "closed" at the same time, and ReclamationCard's own doc comment ('active-but-dead deployment', a live 'Close & refund' action) explicitly describes a still-open lease. If the refuter's invariant held, the 'Close & refund' button would be dead code.
Fix: thread isProviderReclaimed(lease) (or the lease itself) into getServiceStatus and treat it the same as leaseState === "closed", so the service rows agree with the ReclamationCard banner above them.
| .filter((entry): entry is string => typeof entry === "string") | ||
| .map(entry => { | ||
| const separatorIndex = entry.indexOf("="); | ||
| return separatorIndex === -1 ? { key: entry } : { key: entry.slice(0, separatorIndex), value: entry.slice(separatorIndex + 1) }; | ||
| }); | ||
| } | ||
|
|
There was a problem hiding this comment.
🟡 parseComputeResources in placementModel.ts only reads source.storage[0], so services with multiple SDL storage entries (e.g. an ephemeral + a persistent volume) have every volume after the first silently dropped from the per-service Storage stat shown in the expanded row. This is display-only and pre-existing to this new tab, but worth summing all entries (reconciling units) so multi-volume services report their actual footprint.
Extended reasoning...
The bug: parseComputeResources (placementModel.ts, around the firstStorage line) handles the SDL storage field like this:
const firstStorage = Array.isArray(source.storage) ? source.storage[0] : source.storage;
...
storage: parseSize((firstStorage as { size?: unknown } | undefined)?.size)Akash SDL compute profiles allow more than one storage entry per service — most commonly a default ephemeral volume plus one or more persistent volumes with different class/mount configurations. When source.storage is an array with 2+ entries, this code takes only index 0 and discards the rest.
Confirmation this is a real multi-entry shape: the existing SDL importer already treats storage as a list in exactly this codebase — apps/deploy-web/src/utils/sdl/sdlImport.ts:69 does const storages = compute.resources.storage.map ? compute.resources.storage : [compute.resources.storage] and then maps over every entry (line 79) to build the SDL form's volume list. So the manifest shape parseComputeResources is parsing from can and does contain multiple storage entries for real deployments.
Where it surfaces: parseManifestServices calls parseComputeResources per service and stores the result on ManifestServiceDetail.resources. PlacementServiceRow.buildServiceStats renders that single storage value as the "Storage" stat in the expanded service row. For a service with 2+ storage entries, every volume after the first is silently missing from that stat — the displayed number understates the service's actual storage footprint.
Why nothing else catches this: the placement-level "Storage" stat in PlacementCard/buildPlacementStats comes from lease.storageAmount (on-chain lease data), which is correct and unaffected — so there's no crash or top-level inconsistency, just a quietly wrong number in the per-service drill-down that this PR newly introduces.
Step-by-step proof:
- SDL defines
service.compute.resources.storage: [{ size: "1Gi" }, { size: "10Gi", name: "data", attributes: { persistent: true, class: "beta2" } }](ephemeral + persistent, a common stateful pattern). parseManifestServicescallsparseComputeResources(get(parsed, ["profiles", "compute", name, "resources"]))with that array assource.storage.firstStorage = Array.isArray(source.storage) ? source.storage[0] : source.storage→{ size: "1Gi" }, dropping the10Gipersistent entry entirely.storage: parseSize(firstStorage?.size)→{ value: 1, unit: "Gi" }.PlacementServiceRowrenders "Storage: 1 Gi" for a service that actually requests 11Gi total — an ~11x understatement, and the persistent volume (the more operationally important one) is the part that vanishes.
Fix: sum the size of every entry in source.storage (converting to a common unit, e.g. bytes, before adding, since entries can use different units/classes) rather than taking only index 0.
This is display-only, affects only manifests with multiple storage entries per service, and doesn't corrupt the placement-level stat or crash anything — hence nit severity rather than blocking.
| function parseEnv(env: unknown): ManifestEnvVar[] { | ||
| if (!Array.isArray(env)) return []; | ||
|
|
||
| return env | ||
| .filter((entry): entry is string => typeof entry === "string") | ||
| .map(entry => { | ||
| const separatorIndex = entry.indexOf("="); | ||
| return separatorIndex === -1 ? { key: entry } : { key: entry.slice(0, separatorIndex), value: entry.slice(separatorIndex + 1) }; | ||
| }); | ||
| } |
There was a problem hiding this comment.
🟡 parseEnv in placementModel.ts (lines 102-111) reimplements the exact same first-= split already used by sdlImport.ts's svc.env?.map (lines 111-118: indexOf('=') then slice into key/value), with no shared helper. Consider extracting a small parseEnvEntry(entry) helper that both call sites reuse (with sdlImport layering its id/reserved-key logic on top), so a future change to env-string parsing semantics doesn't need to be made in two places.
Extended reasoning...
placementModel.ts introduces a parseEnv helper (lines 102-111) that parses an SDL env-string array ("KEY=VALUE" entries) into { key, value? } objects:
function parseEnv(env: unknown): ManifestEnvVar[] {
if (!Array.isArray(env)) return [];
return env
.filter((entry): entry is string => typeof entry === "string")
.map(entry => {
const separatorIndex = entry.indexOf("=");
return separatorIndex === -1 ? { key: entry } : { key: entry.slice(0, separatorIndex), value: entry.slice(separatorIndex + 1) };
});
}This is the exact same first-= split already implemented in apps/deploy-web/src/utils/sdl/sdlImport.ts (svc.env?.map, lines 111-118), which does:
const separatorIndex = e.indexOf("=");
const key = separatorIndex === -1 ? e : e.slice(0, separatorIndex);
const value = separatorIndex === -1 ? undefined : e.slice(separatorIndex + 1);Step-by-step proof of the duplication:
- Input
"FOO=bar"fed toplacementModel.ts#parseEnv→separatorIndex = 3→{ key: "FOO", value: "bar" }. - The same input fed to
sdlImport.ts's inline mapper →separatorIndex = 3→ same split logic →key: "FOO",value: "bar"(then wrapped with anidand reserved-key handling before being returned). - Input
"FLAG"(no=) → both implementations independently take theseparatorIndex === -1branch and produce a key-only entry with no value.
The two are not byte-identical: sdlImport.ts additionally assigns an id (via nanoid) and applies reserved-key filtering, and returns a different shape ({id, key, value} vs. {key, value}). But the actual env-string-splitting behavior — the part that would need to change if, say, values containing = needed different handling, or new reserved-key semantics were introduced — is expressed twice with no shared source of truth.
Why this matters: since there's no shared helper, a future change to the parsing semantics (e.g., correctly handling = characters inside values, or aligning reserved-key logic) has to be applied in both files, and it's easy to update one and forget the other, causing behavior to silently drift between the SDL builder's env parsing and the deployment detail page's env display.
Suggested fix: extract a small pure helper, e.g. parseEnvEntry(entry: string): { key: string; value?: string }, into a shared location (or export it from sdlImport.ts for reuse), and have both placementModel.ts#parseEnv and sdlImport.ts's svc.env?.map call it — with sdlImport.ts layering its id/reserved-key logic on top of the shared split result.
This is a pure reuse/DRY cleanup with no behavioral bug today — all verifiers agree the current code in both files behaves correctly and consistently; the risk is purely in future maintenance drift. It should not block merging this PR.
| const { data: providerDetail } = d.useProviderDetail(lease.provider, { enabled: !!lease.provider }); | ||
| const carveouts = d.useTeeResourceCarveouts(lease); | ||
|
|
||
| const isReclaimed = isProviderReclaimed(lease); | ||
| const name = getPlacementName(lease.group, index); | ||
| const region = getProviderRegion(providerDetail); |
There was a problem hiding this comment.
🟡 PlacementCard calls d.useProviderDetail(lease.provider, ...) (PlacementCard.tsx:46) purely to read the region via getProviderRegion(providerDetail) at line 51, firing one extra HTTP request per placement card even though DeploymentPlacements already fetched the full ApiProviderList (which carries the same attributes/locationRegion fields) and passes it down as the provider prop. Swapping to getProviderRegion(provider) removes the redundant fetch entirely (keeping in mind provider can be undefined when a lease's owner isn't in the list, which getProviderRegion already handles by returning undefined).
Extended reasoning...
PlacementCard receives a provider?: ApiProviderList prop (already fetched in bulk by DeploymentPlacements via useProviderList and matched to the lease by owner, see DeploymentPlacements.tsx:44), yet it separately calls d.useProviderDetail(lease.provider, { enabled: !!lease.provider }) at PlacementCard.tsx:46 and only uses the result for getProviderRegion(providerDetail) at line 51. Every other field the card needs (providerDisplayName, isProviderReclaimed, DownloadAttestationEvidence) is already read from the provider prop directly, so this one call exists solely to re-derive a region string.
ApiProviderDetail (types/provider.ts:242-248) extends ApiProviderList and adds nothing but an uptime array — it carries no additional field relevant to getProviderRegion. ApiProviderList itself already declares both attributes and locationRegion (types/provider.ts:167-213), which are precisely the two fields getProviderRegion reads (placementModel.ts's getProviderRegion(provider: { attributes?: {...}[]; locationRegion?: string | null })). This isn't just a type-level coincidence: the backend's LIST endpoint (mapProviderToList in apps/api/src/utils/map/provider.ts) populates both attributes (full raw attributes array) and locationRegion (derived from the location-region attribute) at runtime, and the DETAIL endpoint just spreads the same mapProviderToList output with uptime appended. So the list and detail responses carry identical region data — the detail fetch buys nothing new.
The practical effect: for a deployment with N leases, the Details tab now fires N extra GET /providers/:owner requests on mount (and again on lease-status polling if the query re-triggers), purely to recompute a value already sitting in memory from the single useProviderList call made once for the whole page. This scales with placement count and adds avoidable network load and loading-state flicker to a card that would otherwise render entirely from already-fetched data.
Step-by-step proof:
DeploymentPlacements.tsx:44callsproviders.find(provider => provider.owner === lease.provider)and passes the match asproviderto eachPlacementCard.PlacementCard.tsx:46ignores that data for region purposes and callsd.useProviderDetail(lease.provider, ...), which issues a fresh request per card.PlacementCard.tsx:51callsgetProviderRegion(providerDetail)on the detail response.getProviderRegion's signature only needs{ attributes, locationRegion }(placementModel.tslines ~64-66) — fields theproviderprop from step 1 already has, populated by the same backend mapper that produces the detail response.- Therefore
getProviderRegion(provider)in place ofgetProviderRegion(providerDetail)produces the identical region string without any network call.
Fix: drop the d.useProviderDetail call and its dependency entry, and call getProviderRegion(provider) instead, keeping the existing provider prop as the single source of truth. Since provider can be undefined (a lease whose owner isn't present in the fetched provider list), no extra guard is needed — getProviderRegion already returns undefined for an undefined input, matching current behavior when providerDetail is also absent.
| </div> | ||
| <div className="w-full lg:max-w-2xl"> | ||
| <PlacementStats stats={buildPlacementStats(lease, serviceNames.length, gpuModels)} /> | ||
| </div> | ||
| </div> | ||
|
|
||
| {(isReclaimed || carveouts.length > 0 || (isLeaseActive && !!provider)) && ( |
There was a problem hiding this comment.
🟡 The wrapper div around ReclamationCard/ConfidentialComputeResources/DownloadAttestationEvidence (PlacementCard.tsx:84) mounts whenever isLeaseActive && !!provider, which is true for essentially every ordinary active lease — but all three children render null unless the lease is reclaimed or TEE-carved-out, so an empty <div className="space-y-4 px-6 pt-6"> adds an unwanted ~24px gap for the vast majority of deployments. Fix: replace the third disjunct with getGroupTeeType(lease.group) (or otherwise mirror the children'''s actual render conditions) so the wrapper only mounts when something will actually render.
Extended reasoning...
The PlacementCard component wraps three optional sub-components — ReclamationCard, ConfidentialComputeResources, and DownloadAttestationEvidence — in a <div className="space-y-4 px-6 pt-6"> that is gated by isReclaimed || carveouts.length > 0 || (isLeaseActive && !!provider). The intent of that third disjunct was presumably to also cover the TEE-attestation case, but it does not actually check for TEE — it just checks that the lease is live and a provider was resolved, which is true for essentially every active lease regardless of whether it uses confidential computing.
The problem is that none of the three children render anything under that condition alone. ReclamationCard is internally gated by {isReclaimed && ...}, so it contributes nothing unless isReclaimed is already true (already covered by the first disjunct). ConfidentialComputeResources returns null when carveouts.length === 0 (already covered by the second disjunct). DownloadAttestationEvidence returns null unless isLeaseLive(lease) && provider && getGroupTeeType(lease.group) is truthy — i.e., it additionally requires the deployment to actually be a TEE/confidential-compute workload, which the outer condition never checks.
So for the common case — any active, non-reclaimed, non-confidential-compute lease with a resolved provider — the outer condition evaluates to true purely because of isLeaseActive && !!provider, yet all three children bail out to null. React still mounts the wrapper <div>, and because it carries pt-6 (24px top padding), it renders as a visible empty gap between the placement header's bottom border and the services list below, even though there is no content to justify it.
Step-by-step proof: Take an ordinary lease that is active, has a matched provider, is not reclaimed, and requests no GPU/TEE carve-outs (the majority case). isReclaimed = isProviderReclaimed(lease) = false. carveouts = useTeeResourceCarveouts(lease) = [], so carveouts.length > 0 is false. isLeaseActive = isLeaseLive(lease) = true, and provider is defined (matched by owner in DeploymentPlacements), so isLeaseActive && !!provider = true. The overall condition false || false || true = true, so the wrapper renders. Inside it: {isReclaimed && <ReclamationCard .../>} → false && ... → renders nothing. <ConfidentialComputeResources carveouts={[]} /> → returns null per its own guard. <DownloadAttestationEvidence lease={lease} provider={provider} /> → getGroupTeeType(lease.group) is falsy for a non-TEE group, so it returns null. The wrapper <div className="space-y-4 px-6 pt-6"> therefore renders with zero children content, contributing an empty ~24px-tall block.
This is purely a visual/layout defect — no data is lost, no interaction is broken, and it does not affect the newly-added services list or any other functionality in this PR. The fix is simple: align the outer condition with what actually determines whether the children render something, e.g. isReclaimed || carveouts.length > 0 || (isLeaseActive && !!provider && getGroupTeeType(lease.group)), or extract a small helper that any child component could evaluate up front.
| const name = getPlacementName(lease.group, index); | ||
| const region = getProviderRegion(providerDetail); | ||
| const gpuModels = getPlacementGpuModels(lease.group); | ||
| const serviceNames = leaseStatus ? Object.keys(leaseStatus.services) : Object.keys(manifestServices); |
There was a problem hiding this comment.
🟡 In multi-group SDL deployments, PlacementCard.tsx:53 falls back to Object.keys(manifestServices) when leaseStatus is unavailable (closed/reclaimed lease, or before the status query resolves) — but manifestServices is the deployment-wide service list (from parseManifestServices, which never reads the SDL's deployment: placement mapping), not the subset actually deployed to this card's group. This causes a card to display every service from every placement (wrong Docker image/env/command/ports) instead of just its own. Consider empty-listing on fallback (as the old LeaseRow did) or plumbing per-group service names through so each card only shows what it actually runs.
Extended reasoning...
The bug: PlacementCard.tsx line 53 computes serviceNames as:
const serviceNames = leaseStatus ? Object.keys(leaseStatus.services) : Object.keys(manifestServices);The leaseStatus branch is correctly scoped — useLeaseStatus queries the provider's /lease/{dseq}/{gseq}/{oseq}/status endpoint for this specific lease/group, so leaseStatus.services only contains services actually running in that group. The fallback branch, however, uses manifestServices, which is built once in DeploymentPlacements.tsx via parseManifestServices(deploymentManifest) and passed identically to every PlacementCard. parseManifestServices (in placementModel.ts) only reads the SDL's top-level services: block — it never consults the deployment: section that maps each service to a specific placement/group. So manifestServices is the full, deployment-wide service list, not scoped to any one group.
When it triggers: the fallback fires whenever leaseStatus is falsy. Looking at line 45, useLeaseStatus is only enabled when isLeaseActive && !!provider, so for a closed/reclaimed lease this query never even runs and the fallback is permanent for that card. It also fires transiently for a live lease before the status query resolves.
Why nothing else prevents it: there is no per-group filtering anywhere in the pipeline — DeploymentPlacements builds manifestServices from the raw manifest and hands the same object to every card by reference. Contrast this with the pre-existing LeaseRow component this PR replaces, which used leaseStatus ? Object.keys(leaseStatus.services) : [] — i.e., it showed nothing rather than everything when status was unavailable. So the over-listing here is new behavior introduced by this PR, not something inherited from before.
Concrete walkthrough: Suppose an SDL defines two placements — group A running services web and api, group B running service worker — and group A's lease has been reclaimed (closed). manifestServices (built once, deployment-wide) equals { web, api, worker }. Placement card A calls useLeaseStatus with enabled: false (lease not active), so leaseStatus is undefined, and serviceNames falls back to Object.keys(manifestServices) = [web, api, worker]. Card A now renders a worker row — a service it never ran — showing worker's Docker image/env/command pulled from manifestServices.worker, attributed to the wrong placement.
Impact: wrong image/env/command/expose-port data displayed under the wrong placement card. This is confined to multi-group deployments with distinct service subsets per group (a real but less common Akash pattern — most deployments are single-group, and multi-group redundancy setups run the same services in every group, where the fallback happens to be harmless) combined with a non-live lease. It's a genuine, actionable correctness gap, but narrow in trigger and doesn't crash or lose data, and the feature is behind a not-yet-rolled-out flag.
Suggested fix: either revert to empty-listing on fallback (matching LeaseRow's prior behavior — safer, since showing nothing is preferable to showing wrong data), or extend parseManifestServices/the SDL parsing to read the deployment: section and scope manifestServices per group before passing it to each card.
1 participant
Why
On the redesigned deployment detail page, the Details tab still rendered the deployment as a flat list of lease cards. A user couldn't see at a glance how their app is spread across placements and services, or drill into a single service's status, resources, and endpoints. This slice replaces that flat list with a placements → services overview — the first real content tab built on the redesign shell (CON-821).
It ships behind the same feature flags as the shell, so nothing changes for users until the redesign is deliberately rolled out. The legacy page and
LeaseRoware left untouched.Closes CON-822
What
The Details tab is rebuilt as a placements → services overview (new
DeploymentDetail/DeploymentPlacements/folder):Placements · N placements · M services) and one card per lease/placement.--), vCPU, memory, storage. Confidential-compute (TEE carve-outs, attestation evidence) and reclamation states stay visible where applicable.Data & reuse:
regionattribute via the provider-detail endpoint (useProviderDetail); provider name viaproviderDisplayName— the same values the configure marketplace shows.useLeaseStatus/useProviderDetail/useTeeResourceCarveouts,ReclamationCard/ConfidentialComputeResources/DownloadAttestationEvidence/CopyTextToClipboardButton, andbytesToShrink/roundDecimal/getGpusFromAttributes/providerDisplayName/parseSvcCommand/isLeaseLive/isProviderReclaimed.Testing:
/previewsmoke e2e: after deploying a container it asserts the Details tab renders the placements overview, findsservice-1, expands it, and shows the Docker image (nginx:latest).Summary by CodeRabbit