From 94b7870855120b2c62fce19102903601b9fc4bfc Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Tue, 6 Oct 2026 22:47:48 +0100 Subject: [PATCH 1/2] Call a disk that is only hot Running Hot, not Replace Now The Physical Disks Health verdict mapped any critical disk risk to "Replace Now". Disk risk also grades temperature (temperature_high at >=70C critical, >=60C warning), so a SMART-passed disk with good life running at 72C told the user to buy a new disk when the fix is airflow. Alerts already keep heat apart: the SMART health and wearout alerts skip temperature_high and raise disk heat as a Temperature metric. The verdict now splits risk reasons into heat and everything else. Heat alone reads "Running Hot" (phone "Hot"), red at the critical tier and amber at warning, with the temperature reason still as its title and drawer summary. FAILED health, any other critical reason, and a critical level no reason explains keep "Replace Now". The more severe class names the verdict and failure evidence wins a tie, so merged reasons that list heat first no longer hide pending sectors. Sorting and the health filter follow the same verdict, so hot disks stay in "Needs attention" at their temperature tier. A reason's code and severity count even when it has no display text. A critical level that no reason explains shows the generic summary rather than a weaker reason's text. Thresholds are unchanged. The tier still comes from the server's flat 60/70C rule, not the per-type alert thresholds that colour the Temp cell. Reconciling those is separate work. --- .../subsystems/frontend-primitives.md | 5 + .../internal/subsystems/storage-recovery.md | 20 ++ .../ad11a59541280777.json | 44 +++ .../Storage/__tests__/DiskList.test.tsx | 105 +++++++ .../__tests__/diskPresentation.test.ts | 290 ++++++++++++++++++ .../storageBackups/diskPresentation.ts | 178 +++++++++-- 6 files changed, 610 insertions(+), 32 deletions(-) create mode 100644 frontend-modern/browser-verification/ad11a59541280777.json diff --git a/docs/release-control/v6/internal/subsystems/frontend-primitives.md b/docs/release-control/v6/internal/subsystems/frontend-primitives.md index b741322660..6fb37dbe06 100644 --- a/docs/release-control/v6/internal/subsystems/frontend-primitives.md +++ b/docs/release-control/v6/internal/subsystems/frontend-primitives.md @@ -412,6 +412,11 @@ disk header and tooltips. Structured ZFS scan activity supplies a compact badge only during reported rebuild activity; its complete provider summary remains available. Stable disk History catalog/organisation/access ownership is unchanged. +The physical-disk verdict words and their phone forms are `Needs Attention` +(`Attention`), `Running Hot` (`Hot`) and `Replace Now` (`Replace`), all from +`getPhysicalDiskHealthCompactLabel` in `diskPresentation.ts`. A hot disk keeps +the same label at either tier; its tone carries the severity. + Controller expansions carry the supplied kind-specific targets, absolute times, duration and cluster/namespace when the phone row omits those columns. Proxmox coverage distinguishes independent backups from guest-local snapshots; By date diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index cd2ec5bf9e..85d3b643e8 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -6727,6 +6727,26 @@ disk/pool fixture `frontend-modern/browser-tests/history-touch.cjs` and mounted `HistoryChart.test.tsx` regressions verify presentation/input behaviour only, not live appliance collection or recovery success. +### Physical-disk heat is not a replacement verdict + +The Physical Disks Health verdict names the action it asks for. Risk whose only +reason is `temperature_high` reads `Running Hot` (compact `Hot`), red at the +critical tier and amber at warning, with the temperature reason as its summary. +`Replace Now` stays reserved for FAILED health, any other critical reason, and +a critical level that no listed reason explains. `Needs Attention` keeps the +remaining warning evidence. When heat and other evidence both apply, the more +severe class names the verdict and failure evidence wins a tie, whatever order +merged reasons arrive in. Sorting follows the verdict (replace, critical heat, +attention, warning heat), and the health filter places a hot disk at its +temperature tier, so `Needs attention` still lists it. This matches alerting, +which raises disk heat as a Temperature metric and never as disk health. +Thresholds are unchanged: the tier still comes from the server's disk risk +(flat 60/70C), not the per-type alert thresholds that colour the Temp column. +Reconciling those is separate work. `frontend-modern/src/features/storageBackups/__tests__/diskPresentation.test.ts` +and `frontend-modern/src/components/Storage/__tests__/DiskList.test.tsx` cover +the split, merged reason order, the unexplained critical level, sorting and +filtering. + ### Pool-to-physical-disk ownership in Storage details `storagePoolDetailPresentation.ts` must scope inferred ZFS device and UnRAID diff --git a/frontend-modern/browser-verification/ad11a59541280777.json b/frontend-modern/browser-verification/ad11a59541280777.json new file mode 100644 index 0000000000..f0678d07dd --- /dev/null +++ b/frontend-modern/browser-verification/ad11a59541280777.json @@ -0,0 +1,44 @@ +{ + "version": 1, + "base_sha": "8259a74c9d0503df630b09c12a2c1be906fa3d78", + "verified_at": "2026-10-06T22:20:00Z", + "result": "passed", + "changed_paths": [ + "frontend-modern/src/features/storageBackups/diskPresentation.ts" + ], + "content_sha256": { + "frontend-modern/src/features/storageBackups/diskPresentation.ts": "87469b0d62a31a1657a94a5f645041ad34987cc5af55ca238c6cbf49b0c068ee" + }, + "routes": [ + "/proxmox/storage?tab=disks", + "/proxmox/storage?tab=disks&status=attention", + "/proxmox/storage?tab=disks&status=critical", + "/proxmox/storage?tab=disks&status=warning" + ], + "viewports": [ + { + "width": 1440, + "height": 900 + }, + { + "width": 390, + "height": 844 + } + ], + "states": [ + "Private mock stack (8 nodes) whose generator was overlaid locally, never committed, to pin readings from a control file on pve3: Crucial MX500 2TB /dev/nvme2 (SMART PASSED) at 72C, WD Blue SN570 /dev/sd1 at 63C, and Crucial MX500 2TB /dev/nvme0 at 72C with 2 pending sectors. The API returned risk {critical: [temperature_high]}, {warning: [temperature_high]} and {critical: [pending_sectors, temperature_high]} respectively; the backend binary was rebuilt from base 8259a74c9d (includes #2611)", + "Same-session control with the pre-change diskPresentation.ts (3ac5664d95, whose verdict code is unchanged on base 8259a74c9d) swapped in under the running Vite: the 72C temperature-only disk read 'Replace Now' (phone 'Replace') titled 'Disk temperature is 72C' in the row and in its drawer, and the 63C disk read 'Needs Attention' and sorted above the 7%-life disk", + "After the change at 1440 (full layout): rows sort Replace Now (pending sectors, title 'Pending sectors detected (2)', rgb(185,28,28)), Running Hot (title 'Disk temperature is 72C', rgb(185,28,28)), Needs Attention (970 EVO, 'SSD life remaining is 7%', rgb(180,83,9)), Running Hot (title 'Disk temperature is 63C', rgb(180,83,9)), then Healthy/Unknown disks unchanged", + "After the change at 390 (narrow layout): the same rows show the compact words Replace, Hot, Attention, Hot with the same tones and titles", + "Disk drawer header for the 72C temperature-only disk reads 'Running Hot Disk temperature is 72C' at both widths", + "Health filter from the URL at both widths: status=attention lists the two Replace/Hot critical rows plus Attention and the amber Hot row; status=critical lists only Replace Now and the red Running Hot; status=warning lists Needs Attention and the amber Running Hot", + "Known and unchanged: the 63C NVMe reads Running Hot in amber beside a green Temp cell, because the risk tier is the server's flat 60C while the Temp cell uses the NVMe alert threshold; the same disk read Needs Attention before this change. Threshold reconciliation is separate work", + "Amended content (empty-summary reasons keep their code and severity, an unexplained critical level no longer borrows a weaker reason's text): full pass repeated at 1440 and 390 with identical rows, compact words, titles, colours, drawer header and status=attention/critical/warning results" + ], + "interactions": [ + "Loaded the Physical Disks tab unfiltered at 1440x900 and 390x844 and read every row's visible health word, its title, computed colour and the Temp cell", + "Opened the inline drawer for the 72C temperature-only disk by clicking its row at both widths and read the health header", + "Loaded status=attention, status=critical and status=warning at both widths and read the listed rows", + "Swapped the pre-change diskPresentation.ts in under the running Vite, repeated the reads, restored the change and confirmed it byte-identical with cmp; after rebasing onto 8259a74c9d repeated the full after-change pass with a rebuilt backend" + ] +} diff --git a/frontend-modern/src/components/Storage/__tests__/DiskList.test.tsx b/frontend-modern/src/components/Storage/__tests__/DiskList.test.tsx index 73b9cd330d..887fe416aa 100644 --- a/frontend-modern/src/components/Storage/__tests__/DiskList.test.tsx +++ b/frontend-modern/src/components/Storage/__tests__/DiskList.test.tsx @@ -343,6 +343,111 @@ describe('DiskList', () => { ); }); + it('says a disk that is only hot is running hot, not due for replacement', () => { + const view = renderDiskList({ + disks: [ + buildDisk('sda', 'pve3', { + model: 'Crucial MX500 2TB', + diskType: 'ssd', + wearout: 95, + temperature: 72, + risk: { + level: 'critical', + reasons: [ + { + code: 'temperature_high', + severity: 'critical', + summary: 'Disk temperature is 72C', + }, + ], + }, + }), + buildDisk('sdb', 'pve3', { + temperature: 72, + smart: { pendingSectors: 2 }, + risk: { + level: 'critical', + reasons: [ + { + code: 'temperature_high', + severity: 'critical', + summary: 'Disk temperature is 72C', + }, + { + code: 'pending_sectors', + severity: 'critical', + summary: 'Pending sectors detected (2)', + }, + ], + }, + }), + ], + nodes: [buildNode('node-pve3', 'pve3')], + selectedNode: null, + searchTerm: '', + }); + + const rows = Array.from(view.container.querySelectorAll('[data-row-id]')); + // The disk that needs replacing sorts above the one that needs cooling. + expect(rows.map((row) => row.getAttribute('data-row-id'))).toEqual(['sdb', 'sda']); + const hotRow = within(rows[1] as HTMLElement); + expect(hotRow.queryByText('Replace Now')).not.toBeInTheDocument(); + expect(hotRow.getByText('Running Hot')).toHaveClass( + 'platform-table-label-full', + 'text-red-700', + ); + expect(hotRow.getByText('Running Hot')).toHaveAttribute('title', 'Disk temperature is 72C'); + expect(hotRow.getByText('Hot')).toHaveClass('platform-table-label-compact', 'text-red-700'); + const failingRow = within(rows[0] as HTMLElement); + expect(failingRow.getByText('Replace Now')).toHaveAttribute( + 'title', + 'Pending sectors detected (2)', + ); + }); + + it('follows a disk from warm to critically hot and back to healthy', async () => { + const heat = (severity: 'warning' | 'critical', celsius: number) => ({ + level: severity, + reasons: [{ code: 'temperature_high', severity, summary: `Disk temperature is ${celsius}C` }], + }); + const [disks, setDisks] = createStore({ + items: [buildDisk('sda', 'pve3', { temperature: 63, risk: heat('warning', 63) })], + }); + const [healthFilter, setHealthFilter] = createSignal<'all' | 'critical'>('all'); + const view = render(() => ( + {}} + /> + )); + const row = () => view.container.querySelector('[data-row-id="sda"]') as HTMLElement | null; + expect(within(row()!).getByText('Running Hot')).toHaveClass('text-amber-700'); + expect(within(row()!).getByText('Hot')).toHaveClass('text-amber-700'); + setHealthFilter('critical'); + await waitFor(() => expect(row()).toBeNull()); + + setDisks( + 'items', + reconcile([buildDisk('sda', 'pve3', { temperature: 72, risk: heat('critical', 72) })]), + ); + await waitFor(() => expect(row()).not.toBeNull()); + expect(within(row()!).getByText('Running Hot')).toHaveClass('text-red-700'); + expect(within(row()!).getByText('Running Hot')).toHaveAttribute( + 'title', + 'Disk temperature is 72C', + ); + + setHealthFilter('all'); + setDisks('items', reconcile([buildDisk('sda', 'pve3', { temperature: 41 })])); + await waitFor(() => expect(within(row()!).getByText('Healthy')).toBeInTheDocument()); + expect(within(row()!).queryByText('Running Hot')).not.toBeInTheDocument(); + }); + it('renders SSD life and falls back to the Proxmox usage string for Belongs', () => { renderDiskList({ disks: [ diff --git a/frontend-modern/src/features/storageBackups/__tests__/diskPresentation.test.ts b/frontend-modern/src/features/storageBackups/__tests__/diskPresentation.test.ts index a18d7ccdfe..04ac75dc46 100644 --- a/frontend-modern/src/features/storageBackups/__tests__/diskPresentation.test.ts +++ b/frontend-modern/src/features/storageBackups/__tests__/diskPresentation.test.ts @@ -268,10 +268,300 @@ describe('diskPresentation', () => { it('shortens only the health words that cannot fit a phone health column', () => { expect(getPhysicalDiskHealthCompactLabel('Needs Attention')).toBe('Attention'); expect(getPhysicalDiskHealthCompactLabel('Replace Now')).toBe('Replace'); + expect(getPhysicalDiskHealthCompactLabel('Running Hot')).toBe('Hot'); expect(getPhysicalDiskHealthCompactLabel('Healthy')).toBe('Healthy'); expect(getPhysicalDiskHealthCompactLabel('Unknown')).toBe('Unknown'); }); + describe('heat versus replacement evidence', () => { + const buildRiskDisk = ( + id: string, + risk: { level: string; reasons: { code: string; severity: string; summary: string }[] }, + overrides: Record = {}, + ): Resource => + ({ + id, + name: id, + type: 'physical_disk', + status: 'warning', + physicalDisk: { + devPath: `/dev/${id}`, + model: 'Crucial MX500 2TB', + diskType: 'sata', + health: 'PASSED', + wearout: 95, + temperature: 72, + risk, + ...overrides, + }, + identity: { hostname: 'pve3' }, + canonicalIdentity: { hostname: 'pve3' }, + platformType: 'proxmox-pve', + }) as unknown as Resource; + const hot = (severity: string, celsius = 72) => ({ + code: 'temperature_high', + severity, + summary: `Disk temperature is ${celsius}C`, + }); + + it('calls a disk that is only hot Running Hot at the tier of its reading', () => { + const criticalDisk = buildRiskDisk('sda', { level: 'critical', reasons: [hot('critical')] }); + const criticalData = extractPhysicalDiskPresentationData(criticalDisk); + expect(criticalData.riskReasonDetails).toEqual([hot('critical')]); + expect(getPhysicalDiskHealthStatus(criticalData)).toEqual({ + label: 'Running Hot', + summary: 'Disk temperature is 72C', + tone: 'text-red-700 dark:text-red-300', + }); + expect(getPhysicalDiskNormalizedHealth(criticalDisk, criticalData)).toBe('critical'); + + const warmDisk = buildRiskDisk( + 'sdb', + { level: 'warning', reasons: [hot('warning', 63)] }, + { temperature: 63 }, + ); + const warmData = extractPhysicalDiskPresentationData(warmDisk); + expect(getPhysicalDiskHealthStatus(warmData)).toEqual({ + label: 'Running Hot', + summary: 'Disk temperature is 63C', + tone: 'text-amber-700 dark:text-amber-300', + }); + expect(getPhysicalDiskNormalizedHealth(warmDisk, warmData)).toBe('warning'); + }); + + it('keeps Replace Now for failure evidence even when heat is listed first', () => { + // Merged risks keep insertion order, so heat can precede the reason + // that actually calls for a new disk. + const disk = buildRiskDisk('sda', { + level: 'critical', + reasons: [ + hot('critical'), + { + code: 'pending_sectors', + severity: 'critical', + summary: 'Pending sectors detected (2)', + }, + ], + }); + const data = extractPhysicalDiskPresentationData(disk); + expect(getPhysicalDiskHealthStatus(data)).toEqual({ + label: 'Replace Now', + summary: 'Pending sectors detected (2)', + tone: 'text-red-700 dark:text-red-300', + }); + + const failed = extractPhysicalDiskPresentationData( + buildRiskDisk( + 'sdb', + { + level: 'critical', + reasons: [ + hot('critical'), + { + code: 'health_status', + severity: 'critical', + summary: 'Disk reports health status FAILED', + }, + ], + }, + { health: 'FAILED' }, + ), + ); + expect(getPhysicalDiskHealthStatus(failed).label).toBe('Replace Now'); + expect(getPhysicalDiskHealthStatus(failed).summary).toBe('Disk reports health status FAILED'); + }); + + it('lets the more severe of heat and wear evidence name the verdict', () => { + const hotAndWorn = extractPhysicalDiskPresentationData( + buildRiskDisk('sda', { + level: 'critical', + reasons: [ + hot('critical'), + { code: 'wearout_low', severity: 'warning', summary: 'SSD life remaining is 8%' }, + ], + }), + ); + expect(getPhysicalDiskHealthStatus(hotAndWorn).label).toBe('Running Hot'); + expect(getPhysicalDiskHealthStatus(hotAndWorn).summary).toBe('Disk temperature is 72C'); + + const warmAndWorn = extractPhysicalDiskPresentationData( + buildRiskDisk( + 'sdb', + { + level: 'warning', + reasons: [ + hot('warning', 63), + { code: 'wearout_low', severity: 'warning', summary: 'SSD life remaining is 8%' }, + ], + }, + { temperature: 63 }, + ), + ); + expect(getPhysicalDiskHealthStatus(warmAndWorn)).toEqual({ + label: 'Needs Attention', + summary: 'SSD life remaining is 8%', + tone: 'text-amber-700 dark:text-amber-300', + }); + }); + + it('keeps a critical level that no listed reason explains as Replace Now', () => { + const data = extractPhysicalDiskPresentationData( + buildRiskDisk('sda', { level: 'critical', reasons: [hot('warning', 63)] }), + ); + expect(getPhysicalDiskHealthStatus(data)).toEqual({ + label: 'Replace Now', + summary: 'Disk health has degraded to a critical state.', + tone: 'text-red-700 dark:text-red-300', + }); + }); + + it('decides the verdict from codes and severities even without display text', () => { + const pendingWithoutText = extractPhysicalDiskPresentationData( + buildRiskDisk('sda', { + level: 'critical', + reasons: [ + hot('critical'), + { code: 'pending_sectors', severity: 'critical', summary: '' }, + ], + }), + ); + expect(pendingWithoutText.riskReasons).toEqual(['Disk temperature is 72C']); + expect(getPhysicalDiskHealthStatus(pendingWithoutText)).toEqual({ + label: 'Replace Now', + summary: 'Disk health has degraded to a critical state.', + tone: 'text-red-700 dark:text-red-300', + }); + + const heatWithoutText = extractPhysicalDiskPresentationData( + buildRiskDisk('sdb', { + level: 'critical', + reasons: [{ code: 'temperature_high', severity: 'critical', summary: '' }], + }), + ); + expect(getPhysicalDiskHealthStatus(heatWithoutText).label).toBe('Running Hot'); + }); + + it('does not let a weaker reason stand in for an unexplained critical level', () => { + const data = extractPhysicalDiskPresentationData( + buildRiskDisk('sda', { + level: 'critical', + reasons: [ + { code: 'crc_errors', severity: 'monitor', summary: 'UDMA CRC errors detected (2)' }, + hot('warning', 63), + ], + }), + ); + expect(getPhysicalDiskHealthStatus(data)).toEqual({ + label: 'Replace Now', + summary: 'Disk health has degraded to a critical state.', + tone: 'text-red-700 dark:text-red-300', + }); + }); + + it('reads severities case-insensitively and in any order within a class', () => { + const shouting = extractPhysicalDiskPresentationData( + buildRiskDisk('sda', { + level: 'CRITICAL', + reasons: [{ ...hot('critical'), severity: ' Critical ' }], + }), + ); + expect(getPhysicalDiskHealthStatus(shouting).label).toBe('Running Hot'); + expect(getPhysicalDiskHealthStatus(shouting).tone).toBe('text-red-700 dark:text-red-300'); + + const unsorted = extractPhysicalDiskPresentationData( + buildRiskDisk('sdb', { + level: 'critical', + reasons: [ + { code: 'wearout_low', severity: 'warning', summary: 'SSD life remaining is 8%' }, + { + code: 'pending_sectors', + severity: 'critical', + summary: 'Pending sectors detected (2)', + }, + ], + }), + ); + expect(getPhysicalDiskHealthStatus(unsorted).summary).toBe('Pending sectors detected (2)'); + }); + + it('lets SMART counters and low life outrank warning heat', () => { + const warmAndLow = extractPhysicalDiskPresentationData( + buildRiskDisk( + 'sda', + { level: 'warning', reasons: [hot('warning', 63)] }, + { diskType: 'ssd', wearout: 8, temperature: 63 }, + ), + ); + expect(getPhysicalDiskHealthStatus(warmAndLow)).toEqual({ + label: 'Needs Attention', + summary: 'SSD life is running low.', + tone: 'text-amber-700 dark:text-amber-300', + }); + + const warmAndReallocating = extractPhysicalDiskPresentationData( + buildRiskDisk( + 'sdb', + { level: 'warning', reasons: [hot('warning', 63)] }, + { temperature: 63, smart: { reallocatedSectors: 3 } }, + ), + ); + expect(getPhysicalDiskHealthStatus(warmAndReallocating).label).toBe('Needs Attention'); + }); + + it('sorts a FAILED disk with no risk payload ahead of a critically hot one', () => { + const failedDisk = buildRiskDisk( + 'sdb', + { level: 'healthy', reasons: [] }, + { risk: undefined, health: 'FAILED', temperature: 40 }, + ); + const hotDisk = buildRiskDisk('sda', { level: 'critical', reasons: [hot('critical')] }); + const failedData = extractPhysicalDiskPresentationData(failedDisk); + const hotData = extractPhysicalDiskPresentationData(hotDisk); + expect(getPhysicalDiskHealthStatus(failedData).label).toBe('Replace Now'); + expect( + comparePhysicalDiskPresentation(failedDisk, failedData, hotDisk, hotData), + ).toBeLessThan(0); + }); + + it('sorts and filters hot disks by the action their verdict asks for', () => { + const warm = buildRiskDisk('sdc', { level: 'warning', reasons: [hot('warning', 63)] }); + const worn = buildRiskDisk('sdd', { + level: 'warning', + reasons: [ + { code: 'wearout_low', severity: 'warning', summary: 'SSD life remaining is 8%' }, + ], + }); + const hotDisk = buildRiskDisk('sda', { level: 'critical', reasons: [hot('critical')] }); + const failing = buildRiskDisk('sdb', { + level: 'critical', + reasons: [ + { code: 'media_errors', severity: 'critical', summary: 'Media errors detected (4)' }, + ], + }); + const healthy = buildRiskDisk( + 'sde', + { level: 'healthy', reasons: [] }, + { risk: undefined, temperature: 38 }, + ); + const disks = [healthy, warm, worn, hotDisk, failing]; + const dataMap = buildPhysicalDiskPresentationDataMap(disks); + const sorted = (healthFilter: 'all' | 'attention' | 'critical' | 'warning') => + filterAndSortPhysicalDisks(disks, { + selectedNode: null, + healthFilter, + searchTerm: '', + getDiskData: (disk) => dataMap.get(disk.id)!, + matchesNode: () => true, + }).map((disk) => disk.id); + + // Device order alone would put each hot disk first. + expect(sorted('all')).toEqual(['sdb', 'sda', 'sdd', 'sdc', 'sde']); + expect(sorted('attention')).toEqual(['sdb', 'sda', 'sdd', 'sdc']); + expect(sorted('critical')).toEqual(['sdb', 'sda']); + expect(sorted('warning')).toEqual(['sdd', 'sdc']); + }); + }); + it('detects SMART warnings from counters', () => { expect( hasPhysicalDiskSmartWarning({ diff --git a/frontend-modern/src/features/storageBackups/diskPresentation.ts b/frontend-modern/src/features/storageBackups/diskPresentation.ts index 2499be5969..3248b29bf6 100644 --- a/frontend-modern/src/features/storageBackups/diskPresentation.ts +++ b/frontend-modern/src/features/storageBackups/diskPresentation.ts @@ -3,6 +3,7 @@ import type { PhysicalDiskCollectionStatus, PhysicalDiskFieldStatus, Resource, + ResourceStorageRiskReason, } from '@/types/resource'; import { getPlatformTableWeightedColumnWidthStyle } from '@/features/platformPage/sharedPlatformPage'; import { @@ -53,6 +54,8 @@ export interface PhysicalDiskPresentationData { health: string; riskLevel?: string; riskReasons: string[]; + /** The same reasons with their codes and severities, as the API sent them. */ + riskReasonDetails?: ResourceStorageRiskReason[]; wearout: number; storageRole?: string; storageGroup?: string; @@ -198,6 +201,7 @@ export const getPhysicalDiskCellPaddingClass = (layout: PhysicalDiskTableLayoutM export const getPhysicalDiskHealthCompactLabel = (label: string): string => { if (label === 'Needs Attention') return 'Attention'; if (label === 'Replace Now') return 'Replace'; + if (label === 'Running Hot') return 'Hot'; return label; }; @@ -402,14 +406,21 @@ export function extractPhysicalDiskPresentationData( ): PhysicalDiskPresentationData { const pd = resource.physicalDisk || ((resource.platformData as any)?.physicalDisk ?? {}); const diskNode = getPhysicalDiskNodeIdentity(resource); - const riskReasons = Array.isArray(pd.risk?.reasons) + // A reason's code and severity decide the verdict even when it carries no + // display text, so only the summary list drops empty summaries. + const riskReasonDetails: ResourceStorageRiskReason[] = Array.isArray(pd.risk?.reasons) ? pd.risk.reasons - .map((reason: { summary?: unknown }) => reason?.summary) - .filter( - (summary: unknown): summary is string => - typeof summary === 'string' && summary.length > 0, - ) + .filter((reason: unknown) => reason !== null && typeof reason === 'object') + .map((reason: { code?: unknown; severity?: unknown; summary?: unknown }) => ({ + code: typeof reason.code === 'string' ? reason.code : '', + severity: typeof reason.severity === 'string' ? reason.severity : '', + summary: typeof reason.summary === 'string' ? reason.summary : '', + })) + .filter((reason: ResourceStorageRiskReason) => reason.code || reason.summary) : []; + const riskReasons = riskReasonDetails + .map((reason) => reason.summary) + .filter((summary) => summary.length > 0); return { node: diskNode.node, @@ -438,6 +449,7 @@ export function extractPhysicalDiskPresentationData( collection: pd.collection, riskLevel: pd.risk?.level, riskReasons, + riskReasonDetails, smartAttributes: pd.smart ? { powerOnHours: pd.smart.powerOnHours, @@ -534,9 +546,24 @@ export function buildPhysicalDiskPresentationDataMap( return map; } +// Disks sort in the order their verdicts ask for action: replacement first, +// then a disk running critically hot, then other warnings, then a warm disk. +const getPhysicalDiskVerdictPriority = (disk: PhysicalDiskPresentationData): number => { + const verdict = getPhysicalDiskHealthVerdict(disk); + switch (verdict.kind) { + case 'replace': + return 400; + case 'hot': + return verdict.critical ? 300 : 100; + case 'attention': + return 200; + default: + return 0; + } +}; + const getPhysicalDiskPriority = (disk: PhysicalDiskPresentationData): number => - (disk.riskLevel === 'critical' ? 300 : disk.riskLevel === 'warning' ? 200 : 0) + - (hasPhysicalDiskSmartWarning(disk) ? 50 : 0); + getPhysicalDiskVerdictPriority(disk) + (hasPhysicalDiskSmartWarning(disk) ? 50 : 0); export function matchesPhysicalDiskSearch( resource: Resource, @@ -685,31 +712,116 @@ export function hasPhysicalDiskSmartWarning(disk: PhysicalDiskPresentationData): ); } +// Heat is the one disk risk whose fix is cooling, not a replacement disk. +// Alerts raise it as a temperature metric rather than as disk health, so the +// verdict names it apart from the evidence that does mean replacing the disk. +const PHYSICAL_DISK_TEMPERATURE_RISK_CODE = 'temperature_high'; + +const PHYSICAL_DISK_RISK_SEVERITY_RANK: Record = { + monitor: 1, + warning: 2, + critical: 3, +}; + +const getPhysicalDiskRiskSeverityRank = (severity: string | undefined): number => + PHYSICAL_DISK_RISK_SEVERITY_RANK[(severity || '').trim().toLowerCase()] ?? 0; + +interface PhysicalDiskRiskEvidence { + rank: number; + summary?: string; +} + +// Splits the disk's risk into heat and everything else, keeping the most +// severe reason of each. A risk level that none of its reasons explains, or +// one sent without reason codes, stays with the disk-health evidence. +function splitPhysicalDiskRisk(disk: PhysicalDiskPresentationData): { + health: PhysicalDiskRiskEvidence; + heat: PhysicalDiskRiskEvidence; +} { + const health: PhysicalDiskRiskEvidence = { rank: 0 }; + const heat: PhysicalDiskRiskEvidence = { rank: 0 }; + const details = + disk.riskReasonDetails ?? + disk.riskReasons.map((summary) => ({ code: '', severity: disk.riskLevel || '', summary })); + for (const reason of details) { + const evidence = reason.code === PHYSICAL_DISK_TEMPERATURE_RISK_CODE ? heat : health; + const rank = getPhysicalDiskRiskSeverityRank(reason.severity); + if (rank > evidence.rank) { + evidence.rank = rank; + evidence.summary = reason.summary || undefined; + } else if (rank === evidence.rank && !evidence.summary) { + evidence.summary = reason.summary || undefined; + } + } + const levelRank = getPhysicalDiskRiskSeverityRank(disk.riskLevel); + if (levelRank > Math.max(health.rank, heat.rank)) { + // A reason with a known, lower severity does not explain this level, so + // its text must not stand in as the verdict's reason. + if (health.rank > 0) health.summary = undefined; + health.rank = levelRank; + } + return { health, heat }; +} + +type PhysicalDiskHealthVerdict = + | { kind: 'replace'; summary: string } + | { kind: 'hot'; critical: boolean; summary: string } + | { kind: 'attention'; summary: string } + | { kind: 'clear' }; + +function getPhysicalDiskHealthVerdict( + disk: PhysicalDiskPresentationData, +): PhysicalDiskHealthVerdict { + const { health, heat } = splitPhysicalDiskRisk(disk); + const critical = PHYSICAL_DISK_RISK_SEVERITY_RANK.critical; + const warning = PHYSICAL_DISK_RISK_SEVERITY_RANK.warning; + const lowLife = isPhysicalDiskWearoutReported(disk) && disk.wearout < 10; + + if (normalizePhysicalDiskHealth(disk.health) === 'FAILED' || health.rank >= critical) { + return { + kind: 'replace', + summary: health.summary || 'Disk health has degraded to a critical state.', + }; + } + if (heat.rank >= critical) { + return { kind: 'hot', critical: true, summary: heat.summary || 'Disk temperature is high.' }; + } + if (health.rank >= warning || hasPhysicalDiskSmartWarning(disk) || lowLife) { + return { + kind: 'attention', + summary: + health.summary || + (lowLife ? 'SSD life is running low.' : 'SMART counters indicate elevated risk.'), + }; + } + if (heat.rank >= warning) { + return { kind: 'hot', critical: false, summary: heat.summary || 'Disk temperature is high.' }; + } + return { kind: 'clear' }; +} + +const PHYSICAL_DISK_CRITICAL_TONE = 'text-red-700 dark:text-red-300'; +const PHYSICAL_DISK_WARNING_TONE = 'text-amber-700 dark:text-amber-300'; + export function getPhysicalDiskHealthStatus( disk: PhysicalDiskPresentationData, ): DiskHealthStatusPresentation { - const normalizedHealth = normalizePhysicalDiskHealth(disk.health); - const criticalRisk = (disk.riskLevel || '').trim().toLowerCase() === 'critical'; - const warningRisk = (disk.riskLevel || '').trim().toLowerCase() === 'warning'; - const smartWarning = hasPhysicalDiskSmartWarning(disk); - const lowLife = isPhysicalDiskWearoutReported(disk) && disk.wearout < 10; + const verdict = getPhysicalDiskHealthVerdict(disk); - if (normalizedHealth === 'FAILED' || criticalRisk) { + if (verdict.kind === 'replace') { + return { label: 'Replace Now', summary: verdict.summary, tone: PHYSICAL_DISK_CRITICAL_TONE }; + } + + if (verdict.kind === 'hot') { return { - label: 'Replace Now', - summary: disk.riskReasons[0] || 'Disk health has degraded to a critical state.', - tone: 'text-red-700 dark:text-red-300', + label: 'Running Hot', + summary: verdict.summary, + tone: verdict.critical ? PHYSICAL_DISK_CRITICAL_TONE : PHYSICAL_DISK_WARNING_TONE, }; } - if (warningRisk || smartWarning || lowLife) { - return { - label: 'Needs Attention', - summary: - disk.riskReasons[0] || - (lowLife ? 'SSD life is running low.' : 'SMART counters indicate elevated risk.'), - tone: 'text-amber-700 dark:text-amber-300', - }; + if (verdict.kind === 'attention') { + return { label: 'Needs Attention', summary: verdict.summary, tone: PHYSICAL_DISK_WARNING_TONE }; } if (isUnraidPhysicalDisk(disk) && !hasUnraidPhysicalDiskFaultSignal(disk)) { @@ -720,7 +832,7 @@ export function getPhysicalDiskHealthStatus( }; } - const isHealthy = PHYSICAL_DISK_HEALTHY_STATES.has(normalizedHealth); + const isHealthy = PHYSICAL_DISK_HEALTHY_STATES.has(normalizePhysicalDiskHealth(disk.health)); return { label: isHealthy ? 'Healthy' : 'Unknown', summary: isHealthy ? 'No active disk-health issues.' : 'Health state is not reported.', @@ -732,13 +844,15 @@ export function getPhysicalDiskNormalizedHealth( resource: Resource, disk: PhysicalDiskPresentationData, ): NormalizedHealth { - const status = getPhysicalDiskHealthStatus(disk).label; - if (status === 'Online') return 'healthy'; + const verdict = getPhysicalDiskHealthVerdict(disk); + const clearLabel = verdict.kind === 'clear' ? getPhysicalDiskHealthStatus(disk).label : null; + if (clearLabel === 'Online') return 'healthy'; if (resource.status === 'offline') return 'offline'; - if (status === 'Replace Now') return 'critical'; - if (status === 'Needs Attention') return 'warning'; - if (status === 'Healthy') return 'healthy'; - return 'unknown'; + if (verdict.kind === 'replace') return 'critical'; + // A disk running hot sits in the health filter at its temperature tier. + if (verdict.kind === 'hot') return verdict.critical ? 'critical' : 'warning'; + if (verdict.kind === 'attention') return 'warning'; + return clearLabel === 'Healthy' ? 'healthy' : 'unknown'; } export function matchesPhysicalDiskHealthFilter( From 7b362b353239a51b61b9d4f4c733ac8633a127e5 Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Tue, 6 Oct 2026 23:16:04 +0100 Subject: [PATCH 2/2] Load guest physical disks with the drawer to keep Workloads in budget GuestDrawerOverview statically imported GuestPhysicalDisks, which pulled all of diskPresentation.ts into the WorkloadsSurface table chunk. With the Running Hot verdict that chunk measured 70.64 kB gzip against its 70.23 kB cap, so the Frontend bundle check would fail. The card renders only inside an open drawer for a guest with a Pulse agent, so the overview now lazy-loads it under a local Suspense, the same pattern as the bulk web-link editor. WorkloadsSurface drops to 64.31 kB and the disk presentation builds as its own chunk. The contract test pins the dynamic import. --- .../subsystems/performance-and-scalability.md | 5 +++ .../fb12a674d61da775.json | 35 +++++++++++++++++++ .../Workloads/GuestDrawerOverview.tsx | 16 +++++++-- ...loadsSurface.performance.contract.test.tsx | 6 ++++ 4 files changed, 59 insertions(+), 3 deletions(-) create mode 100644 frontend-modern/browser-verification/fb12a674d61da775.json diff --git a/docs/release-control/v6/internal/subsystems/performance-and-scalability.md b/docs/release-control/v6/internal/subsystems/performance-and-scalability.md index f78c71e278..5071809ca9 100644 --- a/docs/release-control/v6/internal/subsystems/performance-and-scalability.md +++ b/docs/release-control/v6/internal/subsystems/performance-and-scalability.md @@ -1182,6 +1182,11 @@ change may globally weaken the Task 03 lifecycle-state idempotency invariant. Saves run one metadata write per changed link, then dispatch the metadata-changed events together so whole-snapshot listeners coalesce onto one in-flight refetch instead of refetching per link. + The drawer overview lazy-loads `GuestPhysicalDisks.tsx` under a local + `Suspense`, so the physical-disk presentation module + (`diskPresentation.ts`) loads with an open agent guest's drawer instead + of joining the WorkloadsSurface chunk. + `WorkloadsSurface.performance.contract.test.tsx` pins the dynamic import. Drawer history charts belong to `frontend-modern/src/components/Workloads/GuestDrawerHistory.tsx`. A current metric may remain visible in a chart legend only when labelled `current`; it must never be expanded into synthetic timestamps or a flat diff --git a/frontend-modern/browser-verification/fb12a674d61da775.json b/frontend-modern/browser-verification/fb12a674d61da775.json new file mode 100644 index 0000000000..78a9f2ccca --- /dev/null +++ b/frontend-modern/browser-verification/fb12a674d61da775.json @@ -0,0 +1,35 @@ +{ + "version": 1, + "base_sha": "94b7870855120b2c62fce19102903601b9fc4bfc", + "verified_at": "2026-10-06T22:30:00Z", + "result": "passed", + "changed_paths": [ + "frontend-modern/src/components/Workloads/GuestDrawerOverview.tsx" + ], + "content_sha256": { + "frontend-modern/src/components/Workloads/GuestDrawerOverview.tsx": "6e4003a1508856b944680e35234d9f93ef59a02f870fea6f9f1173f7e641c905" + }, + "routes": [ + "/proxmox/overview" + ], + "viewports": [ + { + "width": 1440, + "height": 900 + }, + { + "width": 390, + "height": 844 + } + ], + "states": [ + "Private mock stack (8 nodes, local uncommitted generator overlay pinning pve3 /dev/nvme2 at 72C). Mock guests carry no agent facet and no linked physical disks, so Playwright route interception added agent.agentVersion '6.5.0' to redis-cache-01 (vm-3ae77f2c237ff912) in /api/resources responses and answered the guest's type=physical_disk&parent= query with the stack's real 72C Crucial MX500 resource reparented to that guest", + "Production builds: with this commit (base 8259a74c9d plus the verdict commit 94b7870855) check-bundle-size reports WorkloadsSurface 64.31 kB gzip against a 70.23 kB max, every tracked chunk ok and none missing; origin/main d9ffb39ce8 plus 94b7870855 without this commit measured 70.64 kB (over by 0.40) and d9ffb39ce8 alone 70.20 kB. diskPresentation and GuestPhysicalDisks now build as their own chunks and the WorkloadsSurface chunk no longer contains the disk verdict strings", + "Guest drawer at 1440 and 390: the GuestPhysicalDisks module is requested only after the guest row is clicked open (none before), the parent query is sent once for the guest id, and the card reads 'Physical Disks & SMART (1)', 'Crucial MX500 2TB', '/dev/nvme2', 'Running Hot' (title 'Disk temperature is 72C', rgb(185,28,28)), '72\u00b0C', '1.82 TB'" + ], + "interactions": [ + "Loaded /proxmox/overview at 1440x900 and 390x844, recorded module requests, then clicked the redis-cache-01 guest row to open its drawer and waited for the guest physical-disk card", + "Read the card text, the verdict label, its title and computed colour, and clipped a screenshot of the card at both widths", + "Ran vite build and check-bundle-size on d9ffb39ce8, on d9ffb39ce8 plus 94b7870855, and on this branch" + ] +} diff --git a/frontend-modern/src/components/Workloads/GuestDrawerOverview.tsx b/frontend-modern/src/components/Workloads/GuestDrawerOverview.tsx index de41cfe2e4..9b30b81b1f 100644 --- a/frontend-modern/src/components/Workloads/GuestDrawerOverview.tsx +++ b/frontend-modern/src/components/Workloads/GuestDrawerOverview.tsx @@ -1,4 +1,4 @@ -import { Show } from 'solid-js'; +import { Show, Suspense, lazy } from 'solid-js'; import { formatDiscoveryAge } from '@/api/discovery'; import { @@ -29,11 +29,19 @@ import { } from './guestDrawerModel'; import type { NestedWorkloadContext } from './nestedWorkloadContext'; import { WORKLOAD_ACTION_AGENT_LABEL } from './workloadAgentReadiness'; -import { GuestPhysicalDisks } from './GuestPhysicalDisks'; import { RaidCard } from '@/components/shared/cards/RaidCard'; import type { GuestDrawerProps } from './guestDrawerModel'; +// Linked physical disks render only inside an open agent guest's drawer. Load +// them on demand so the disk presentation module stays out of the +// WorkloadsSurface table chunk. +const GuestPhysicalDisks = lazy(() => + import('./GuestPhysicalDisks').then(({ GuestPhysicalDisks }) => ({ + default: GuestPhysicalDisks, + })), +); + interface GuestDrawerOverviewProps { guest: GuestDrawerProps['guest']; guestOsSummary: string; @@ -317,7 +325,9 @@ export function GuestDrawerOverview(props: GuestDrawerOverviewProps) { props.guest.agentRaid?.length } > - + + + diff --git a/frontend-modern/src/components/Workloads/__tests__/WorkloadsSurface.performance.contract.test.tsx b/frontend-modern/src/components/Workloads/__tests__/WorkloadsSurface.performance.contract.test.tsx index d2aa75fff9..4fa12f172d 100644 --- a/frontend-modern/src/components/Workloads/__tests__/WorkloadsSurface.performance.contract.test.tsx +++ b/frontend-modern/src/components/Workloads/__tests__/WorkloadsSurface.performance.contract.test.tsx @@ -1667,6 +1667,12 @@ describe('Workloads performance contract', () => { expect(guestDrawerManageSource).toContain('ResourceOperatorStateSection'); expect(guestDrawerOverviewSource).toContain('buildWorkloadsDiskPresentation'); expect(guestDrawerOverviewSource).toContain('Filesystems'); + // Linked physical disks load with an open agent guest's drawer, so the + // disk presentation module stays out of the WorkloadsSurface chunk. + expect(guestDrawerOverviewSource).toContain("import('./GuestPhysicalDisks')"); + expect(guestDrawerOverviewSource).not.toMatch( + /^import (?!type )[^;]*from '\.\/GuestPhysicalDisks';/m, + ); // The shared DetailSectionTable gives the earlier of the last two sections // the wider span, so the longer filesystem list must be declared before the // short tag list for mount points to get the readable panel width.