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 9a0ad0a645..3b4a3751d0 100644 --- a/docs/release-control/v6/internal/subsystems/performance-and-scalability.md +++ b/docs/release-control/v6/internal/subsystems/performance-and-scalability.md @@ -161,8 +161,10 @@ QGA/thaw, installed recovery, fleet-performance or release-availability claim. Choosing a node's temperature source in `resourceStateAdapters.ts` reads one more field of the facet already in the canonical payload -(`proxmox.temperatureDetails`) when earlier sources have no usable reading. It -adds no resource read, merge pass, timer or subscription. +(`proxmox.temperatureDetails`), only when the existing record selection yields +no usable reading, and keeps it only when it holds a CPU reading. That is one +extra record mapping per such node per adapter call. It adds no resource read, +merge pass, timer or subscription. ### Unavailable filesystem values share the existing disk model diff --git a/docs/release-control/v6/internal/subsystems/unified-resources.md b/docs/release-control/v6/internal/subsystems/unified-resources.md index 40bf3e9036..aa5a022d7e 100644 --- a/docs/release-control/v6/internal/subsystems/unified-resources.md +++ b/docs/release-control/v6/internal/subsystems/unified-resources.md @@ -3182,15 +3182,19 @@ footer after the visible rows. The footer may dim the final row edge and expose the remaining-row count, but expanding or collapsing it only changes the client-side visible slice; it must not refetch, reorder, or redefine the canonical resource collection. -The shared `nodeFromResource` adapter takes a node's temperature from the first -source with a usable reading: a platform or Proxmox temperature record, the -agent facet, then `proxmox.temperatureDetails`. That last field is the Proxmox -poller's full reading for a node without a host agent, including the CPU low -and record values it tracks; `proxmox.temperature` is only the scalar maximum. -A bare scalar reading carries no history, so the adapter leaves the CPU low and -record unset rather than repeating the current value, and the node drawer omits -those rows. `resourceStateAdapters.test.ts` pins both with the API's payload -shape. +The shared `nodeFromResource` adapter selects a node's temperature record as +before: the platform temperature record, else the Proxmox temperature record, +else the agent facet. When that selection yields no usable reading, it uses +`proxmox.temperatureDetails` if that holds a current CPU reading, ahead of the +bare scalar fallback. That field is +the Proxmox poller's full reading for a node without a host agent, including +the CPU low and record values the poller supplies; `proxmox.temperature` is +only the scalar maximum. The adapter forwards those values; keeping them across +polls is the poller's job (`monitor_previous_state.go` and the temperature carry +in monitoring). A bare scalar reading carries no history, so the adapter leaves +the CPU low and record unset rather than repeating the current value, and the +node drawer omits those rows. `resourceStateAdapters.test.ts` pins these cases +with the API's payload shape. The registry and presentation coalescer also own metric-source freshness. When two source facets contribute the same metric, source priority decides only if both sources have equivalent freshness. A stale source must not hold CPU, diff --git a/frontend-modern/browser-verification/ae27640ea2ed301f.json b/frontend-modern/browser-verification/ae27640ea2ed301f.json new file mode 100644 index 0000000000..8c18857784 --- /dev/null +++ b/frontend-modern/browser-verification/ae27640ea2ed301f.json @@ -0,0 +1,37 @@ +{ + "version": 1, + "base_sha": "60fb6e842b702c35c93ce7c5e8397487c15aab6a", + "verified_at": "2026-10-06T16:05:00Z", + "result": "passed", + "changed_paths": [ + "frontend-modern/src/utils/resourceStateAdapters.ts" + ], + "content_sha256": { + "frontend-modern/src/utils/resourceStateAdapters.ts": "744fb071acc493f973c6064e7c7eaaac831c9cef2a88491a8b51e49a08f139be" + }, + "routes": [ + "/proxmox" + ], + "viewports": [ + { + "width": 1440, + "height": 900 + }, + { + "width": 390, + "height": 844 + } + ], + "states": [ + "Private mock stack built from this tree (backend :7681, Vite :5199) after restoring the original record selection and gating temperatureDetails on a current CPU reading", + "Proxmox node Analytics A (pve31) made PVE-only (agent facet removed from REST and websocket payloads), compared on identical data: main's adapter showed CPU low 63\u00b0C and CPU record 63\u00b0C (the current reading repeated); this change showed CPU low 39\u00b0C and CPU record 91\u00b0C from an injected temperatureDetails record (cpuMin 38.5, cpuMaxRecord 91.2) at 1440x900 and 390x844", + "Same PVE-only node without a record in temperatureDetails: CPU low and CPU record rows absent; NVMe row and table TEMP unchanged", + "Operator-context Temperature row: hidden on main and on this change at 63\u00b0C (it appears only in the warning band), so no difference", + "Agent-linked mock node with and without an injected record, desktop and phone width: no CPU low/record rows and Temperature from the agent facet, as on main", + "Unit tests cover the two cases this commit fixes: details with only NVMe do not displace a valid scalar CPU reading, and an unusable legacy record still falls back to the scalar" + ], + "interactions": [ + "Opened the node drawer from the row's detail toggle (aria-controls proxmox-host-drawer-) on /proxmox at desktop and phone widths", + "Read the Thermals and Operator context rows and the row's TEMP cell, and took screenshots, for main's adapter and for this change via Playwright with REST and websocket payload rewriting" + ] +} diff --git a/frontend-modern/src/utils/__tests__/resourceStateAdapters.test.ts b/frontend-modern/src/utils/__tests__/resourceStateAdapters.test.ts index dbb6aef82c..fa946df7c7 100644 --- a/frontend-modern/src/utils/__tests__/resourceStateAdapters.test.ts +++ b/frontend-modern/src/utils/__tests__/resourceStateAdapters.test.ts @@ -299,6 +299,34 @@ describe('resourceStateAdapters nodeFromResource', () => { expect(bare?.temperature?.cpuMin).toBeUndefined(); expect(bare?.temperature?.cpuMaxRecord).toBeUndefined(); + // Details without a CPU reading do not displace a valid scalar CPU reading. + const sensorsOnly = nodeFromResource({ + ...createNodeResource({ + proxmox: { + nodeName: 'pve-node-2', + temperatureDetails: { + available: true, + hasCPU: false, + nvme: [{ device: 'nvme0', temp: 44 }], + }, + }, + }), + temperature: 58, + } as unknown as Resource); + expect(sensorsOnly?.temperature?.cpuPackage).toBe(58); + expect(sensorsOnly?.temperature?.available).toBe(true); + + // An unusable earlier record still falls back to the scalar reading. + const legacyUnavailable = nodeFromResource({ + ...createNodeResource({ + temperature: { available: false }, + proxmox: { nodeName: 'pve-node-2', temperature: { available: false, cpuPackage: 58 } }, + }), + temperature: 58, + } as unknown as Resource); + expect(legacyUnavailable?.temperature?.cpuPackage).toBe(58); + expect(legacyUnavailable?.temperature?.available).toBe(true); + // A node with a host agent keeps reading the agent facet first. const agentLinked = nodeFromResource({ ...createNodeResource({ proxmox, agent: { agentId: 'pve-agent', temperature: 61 } }), diff --git a/frontend-modern/src/utils/resourceStateAdapters.ts b/frontend-modern/src/utils/resourceStateAdapters.ts index f78e577757..72efec2fb5 100644 --- a/frontend-modern/src/utils/resourceStateAdapters.ts +++ b/frontend-modern/src/utils/resourceStateAdapters.ts @@ -1373,77 +1373,90 @@ const buildDisk = (metric: Resource['disk'], fallback?: Record) }; }; +const temperatureFromRecord = ( + raw: Record, + resource: Resource, +): Temperature | undefined => { + const available = asBoolean(raw.available); + const cpuPackage = asNumber(raw.cpuPackage) ?? asNumber(raw.temperature) ?? asNumber(raw.cpu); + const lastUpdate = toISOTime(raw.lastUpdate, resource.lastSeen); + if (available || typeof cpuPackage === 'number') { + return { + cpuPackage, + cpuMax: asNumber(raw.cpuMax), + cpuMin: asNumber(raw.cpuMin), + cpuMaxRecord: asNumber(raw.cpuMaxRecord), + minRecorded: asString(raw.minRecorded), + maxRecorded: asString(raw.maxRecorded), + cores: asArray(raw.cores) + .map((entry) => { + const rec = asRecord(entry); + if (!rec) return null; + const core = asNumber(rec.core); + const temp = asNumber(rec.temp); + if (typeof core !== 'number' || typeof temp !== 'number') return null; + return { core, temp }; + }) + .filter((entry): entry is NonNullable => Boolean(entry)), + gpu: asArray(raw.gpu) + .map((entry) => { + const rec = asRecord(entry); + if (!rec) return null; + const device = asString(rec.device); + if (!device) return null; + return { + device, + edge: asNumber(rec.edge), + junction: asNumber(rec.junction), + mem: asNumber(rec.mem), + }; + }) + .filter((entry): entry is NonNullable => Boolean(entry)), + nvme: asArray(raw.nvme) + .map((entry) => { + const rec = asRecord(entry); + if (!rec) return null; + const device = asString(rec.device); + const temp = asNumber(rec.temp); + if (!device || typeof temp !== 'number') return null; + return { device, temp }; + }) + .filter((entry): entry is NonNullable => Boolean(entry)), + available: available ?? true, + hasCPU: asBoolean(raw.hasCPU) ?? (typeof cpuPackage === 'number' ? true : undefined), + hasGPU: asBoolean(raw.hasGPU), + hasNVMe: asBoolean(raw.hasNVMe), + lastUpdate, + }; + } + return undefined; +}; + +const hasCPUTemperatureReading = (temperature: Temperature): boolean => + (temperature.cpuPackage ?? 0) > 0 || + (temperature.cpuMax ?? 0) > 0 || + (temperature.cores ?? []).some((core) => core.temp > 0); + const buildTemperature = ( resource: Resource, nodeMeta?: Record, ): Temperature | undefined => { const platform = resourcePlatformData(resource); - // Take the first source with a usable reading. A Proxmox node without a host - // agent publishes its full reading, including the CPU low and record values - // the poller tracks, only as proxmox.temperatureDetails (proxmox.temperature - // is the scalar maximum), so it comes before the bare-reading fallback. - const candidates = [ - platform?.temperature, - nodeMeta?.temperature, - platform?.agent, - nodeMeta?.temperatureDetails, - ]; - for (const candidate of candidates) { - const raw = asRecord(candidate); - if (!raw) continue; - const available = asBoolean(raw.available); - const cpuPackage = asNumber(raw.cpuPackage) ?? asNumber(raw.temperature) ?? asNumber(raw.cpu); - const lastUpdate = toISOTime(raw.lastUpdate, resource.lastSeen); - if (available || typeof cpuPackage === 'number') { - return { - cpuPackage, - cpuMax: asNumber(raw.cpuMax), - cpuMin: asNumber(raw.cpuMin), - cpuMaxRecord: asNumber(raw.cpuMaxRecord), - minRecorded: asString(raw.minRecorded), - maxRecorded: asString(raw.maxRecorded), - cores: asArray(raw.cores) - .map((entry) => { - const rec = asRecord(entry); - if (!rec) return null; - const core = asNumber(rec.core); - const temp = asNumber(rec.temp); - if (typeof core !== 'number' || typeof temp !== 'number') return null; - return { core, temp }; - }) - .filter((entry): entry is NonNullable => Boolean(entry)), - gpu: asArray(raw.gpu) - .map((entry) => { - const rec = asRecord(entry); - if (!rec) return null; - const device = asString(rec.device); - if (!device) return null; - return { - device, - edge: asNumber(rec.edge), - junction: asNumber(rec.junction), - mem: asNumber(rec.mem), - }; - }) - .filter((entry): entry is NonNullable => Boolean(entry)), - nvme: asArray(raw.nvme) - .map((entry) => { - const rec = asRecord(entry); - if (!rec) return null; - const device = asString(rec.device); - const temp = asNumber(rec.temp); - if (!device || typeof temp !== 'number') return null; - return { device, temp }; - }) - .filter((entry): entry is NonNullable => Boolean(entry)), - available: available ?? true, - hasCPU: asBoolean(raw.hasCPU) ?? (typeof cpuPackage === 'number' ? true : undefined), - hasGPU: asBoolean(raw.hasGPU), - hasNVMe: asBoolean(raw.hasNVMe), - lastUpdate, - }; - } - } + const raw = + asRecord(platform?.temperature) || + asRecord(nodeMeta?.temperature) || + asRecord(platform?.agent) || + undefined; + const fromRecord = raw ? temperatureFromRecord(raw, resource) : undefined; + if (fromRecord) return fromRecord; + + // A Proxmox node without a host agent publishes its full reading, including + // the CPU low and record values the poller tracks, only as + // proxmox.temperatureDetails (proxmox.temperature is the scalar maximum). Use + // it ahead of the bare-reading fallback when it holds a current CPU reading. + const details = asRecord(nodeMeta?.temperatureDetails); + const fromDetails = details ? temperatureFromRecord(details, resource) : undefined; + if (fromDetails?.available && hasCPUTemperatureReading(fromDetails)) return fromDetails; if (typeof resource.temperature === 'number' && Number.isFinite(resource.temperature)) { const temp = resource.temperature;