feat: add cpu power query & subscription #1745
Conversation
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a new CpuPackages GraphQL type and exposes per-package topology and telemetry via InfoCpu.topology, InfoCpu.packages, and a systemMetricsCpuTelemetry subscription; introduces CpuTopologyService to read /sys for topology/power/temps, wires services into a new CpuModule, updates MetricsResolver subscriptions and tests, and adds a deploy script for unraid-shared. Changes
Sequence Diagram(s)sequenceDiagram
participant Sub as Subscriber
participant MR as MetricsResolver
participant CTS as CpuTopologyService
participant Sysfs as /sys
participant Pub as PubSub
Note over MR,CTS: onModuleInit registers CPU_TELEMETRY handler
Sub->>MR: subscribe systemMetricsCpuTelemetry
MR->>Pub: subscribe to CPU_TELEMETRY
loop periodic telemetry
MR->>CTS: generateTelemetry()
CTS->>Sysfs: read cpu topology, hwmon temps, powercap energy_uj
Sysfs-->>CTS: topology, temps, energy samples
CTS->>CTS: compute per-package power, temps, totalPower
CTS-->>MR: return CpuPackages payload
MR->>Pub: publish CPU_TELEMETRY payload
Pub-->>Sub: deliver systemMetricsCpuTelemetry payload
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1745 +/- ##
==========================================
- Coverage 52.69% 52.25% -0.44%
==========================================
Files 865 874 +9
Lines 49343 50536 +1193
Branches 4952 5053 +101
==========================================
+ Hits 26000 26407 +407
- Misses 23270 24054 +784
- Partials 73 75 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts (1)
23-33: Guard against NaN for stepping; consider latency impact of power fetch.
- Number(stepping) can yield NaN (GraphQL Int cannot represent NaN). Guard it.
- generateCpuPower() performs a ~200ms sampling; this adds latency to the InfoCpu query. Consider caching, lazy resolution via a separate field resolver, or background sampling.
- power: await this.cpuPowerService.generateCpuPower(), - stepping: Number(stepping), + power: await this.cpuPowerService.generateCpuPower(), + stepping: stepping != null && `${stepping}` !== '' ? Number(stepping) : undefined,
♻️ Duplicate comments (1)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts (1)
2-4: Remove unused imports (CodeQL findings).fsConstants, access, readdir, readFile, basename, join are unused in this file.
-import { constants as fsConstants } from 'node:fs'; -import { access, readdir, readFile } from 'node:fs/promises'; -import { basename, join } from 'node:path';
🧹 Nitpick comments (7)
api/src/unraid-api/graph/resolvers/info/info.module.ts (1)
5-5: Remove unused CpuService import.CpuService is no longer used directly in InfoModule since it's now provided by CpuModule (line 18).
Apply this diff to remove the unused import:
-import { CpuService } from '@app/unraid-api/graph/resolvers/info/cpu/cpu.service.js';api/src/unraid-api/graph/resolvers/metrics/metrics.module.ts (1)
3-5: Remove unused imports.CpuPowerService and CpuService are imported but not directly used in MetricsModule. Both services are now provided by CpuModule (line 11), making these direct imports unnecessary.
Apply this diff to remove the unused imports:
-import { CpuPowerService } from '@app/unraid-api/graph/resolvers/info/cpu/cpu-power.service.js'; import { CpuModule } from '@app/unraid-api/graph/resolvers/info/cpu/cpu.module.js'; -import { CpuService } from '@app/unraid-api/graph/resolvers/info/cpu/cpu.service.js';api/src/unraid-api/graph/resolvers/info/cpu/cpu.model.ts (1)
51-61: Clarify naming vs. semantics (packages/sockets vs. cores).The field name coresPower suggests per-core values, but RAPL reads package/socket power. Either rename the field or update the description to avoid confusion.
Apply this minimal-description fix:
- description: 'CPU power in watts for each physical processor', + description: 'CPU power in watts for each CPU package/socket (physical processor)',api/src/unraid-api/graph/resolvers/info/cpu/cpu-power.service.ts (4)
4-4: Usenode:pathfor ESM consistency.Prefer Node built-in ESM specifier.
-import { basename, join } from 'path'; +import { basename, join } from 'node:path';
46-48: Make sampling window a constant/configurable.Hardcoded 200ms can be a knob for accuracy vs. latency; extract to a const.
- // ~200ms delay to measure energy delta - await new Promise((resolve) => setTimeout(resolve, 200)); + // Sampling window to measure energy delta + const measureWindowMs = 200; + await new Promise((resolve) => setTimeout(resolve, measureWindowMs));
71-78: Handle RAPL energy counter wrap-around.energy_uj can wrap; adjust diff using max_energy_range_uj when negative.
- const diffEnergy = now - prevE; // microjoules + let diffEnergy = now - prevE; // microjoules + if (diffEnergy < 0) { + try { + const maxFile = join(domainPath, 'max_energy_range_uj'); + await access(maxFile, fsConstants.R_OK); + const maxStr = await readFile(maxFile, 'utf8'); + const maxRange = Number.parseInt(maxStr.trim(), 10); + if (Number.isFinite(maxRange) && maxRange > 0) { + diffEnergy = (now + maxRange) - prevE; + } + } catch { + // if wrap-around can't be determined, skip this sample + continue; + } + } const diffTimeNs = Number(tNow - prevT); // nanoseconds if (!(diffTimeNs > 0)) continue;
132-134: Log unexpected errors for observability.Capture and log the error; return safe default.
- } catch { - return { id }; - } + } catch (e) { + this.logger.debug(`generateCpuPower failed: ${String(e)}`); + return { id }; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
api/generated-schema.graphql(3 hunks)api/src/unraid-api/graph/resolvers/info/cpu/cpu-power.service.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/cpu/cpu.model.ts(2 hunks)api/src/unraid-api/graph/resolvers/info/cpu/cpu.module.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts(3 hunks)api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts(2 hunks)api/src/unraid-api/graph/resolvers/info/info.module.ts(2 hunks)api/src/unraid-api/graph/resolvers/info/info.resolver.integration.spec.ts(2 hunks)api/src/unraid-api/graph/resolvers/metrics/metrics.module.ts(1 hunks)api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.ts(2 hunks)api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.ts(3 hunks)api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts(4 hunks)packages/unraid-shared/src/pubsub/graphql.pubsub.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
api/src/unraid-api/**
📄 CodeRabbit inference engine (.cursor/rules/api-rules.mdc)
Prefer adding new files to the Nest repo at api/src/unraid-api/ instead of legacy code
Files:
api/src/unraid-api/graph/resolvers/info/info.resolver.integration.spec.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.module.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.model.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.module.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu-power.service.tsapi/src/unraid-api/graph/resolvers/info/info.module.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.service.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts
api/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/api-rules.mdc)
api/**/*.{test,spec}.{js,jsx,ts,tsx}: Use Vitest for tests in the api; do not use Jest
Prefer not to mock simple dependencies in tests
For error testing, use.rejects.toThrow()without arguments; avoid asserting exact error messages unless the message format is the subject under test
Files:
api/src/unraid-api/graph/resolvers/info/info.resolver.integration.spec.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript import specifiers with .js extensions for ESM compatibility
Never use the any type; prefer precise typing
Avoid type casting; model proper types from the start
Files:
api/src/unraid-api/graph/resolvers/info/info.resolver.integration.spec.tspackages/unraid-shared/src/pubsub/graphql.pubsub.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.module.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.model.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.module.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu-power.service.tsapi/src/unraid-api/graph/resolvers/info/info.module.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.service.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts
api/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
api/**/*.{test,spec}.{ts,tsx}: API test suite is Vitest; do not use Jest
Prefer not to mock simple dependencies in API tests
Files:
api/src/unraid-api/graph/resolvers/info/info.resolver.integration.spec.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts
{api,web}/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
{api,web}/**/*.{test,spec}.{ts,tsx}: For error tests, use .rejects.toThrow() without arguments; avoid asserting exact error messages unless that format is the subject
Focus tests on behavior, not implementation details
Avoid brittle tests tied to exact error or log wording
Use mocks as nouns, not verbs
Always await async operations before making assertions
Place all mock declarations at the top level; use factory functions for module mocks; clear mocks between tests
Files:
api/src/unraid-api/graph/resolvers/info/info.resolver.integration.spec.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts
api/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
cache-manager v7 TTL values must be in milliseconds (e.g., 600000 for 10 minutes)
Files:
api/src/unraid-api/graph/resolvers/info/info.resolver.integration.spec.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.module.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.model.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.module.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu-power.service.tsapi/src/unraid-api/graph/resolvers/info/info.module.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.service.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts
🧠 Learnings (2)
📚 Learning: 2025-08-09T01:03:29.676Z
Learnt from: elibosley
PR: unraid/api#1575
File: packages/unraid-shared/src/services/socket-config.service.spec.ts:10-13
Timestamp: 2025-08-09T01:03:29.676Z
Learning: Vitest is used for all testing across all repositories in the unraid organization, not Jest. Always use `vi` for mocking utilities, not `jest`.
Applied to files:
api/src/unraid-api/graph/resolvers/info/info.resolver.integration.spec.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts
📚 Learning: 2025-09-12T01:36:59.838Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T01:36:59.838Z
Learning: Applies to api/**/*.{test,spec}.{ts,tsx} : API test suite is Vitest; do not use Jest
Applied to files:
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.ts
🧬 Code graph analysis (6)
api/src/unraid-api/graph/resolvers/metrics/metrics.module.ts (3)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.module.ts (1)
Module(6-10)api/src/unraid-api/graph/resolvers/resolvers.module.ts (1)
Module(37-76)api/src/unraid-api/graph/services/services.module.ts (1)
Module(8-13)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.model.ts (2)
api/src/unraid-api/graph/resolvers/info/info.model.ts (1)
ObjectType(13-44)api/src/unraid-api/cli/generated/graphql.ts (1)
Node(1371-1373)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.module.ts (2)
api/src/unraid-api/graph/resolvers/info/info.module.ts (1)
Module(17-37)api/src/unraid-api/graph/resolvers/metrics/metrics.module.ts (1)
Module(10-15)
api/src/unraid-api/graph/resolvers/info/cpu/cpu-power.service.ts (1)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts (1)
Injectable(11-54)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts (1)
api/src/unraid-api/graph/resolvers/info/cpu/cpu-power.service.ts (1)
Injectable(8-136)
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts (2)
api/src/unraid-api/cli/generated/graphql.ts (1)
Subscription(2046-2058)web/composables/gql/graphql.ts (1)
Subscription(2032-2044)
🪛 GitHub Check: CodeQL
api/src/unraid-api/graph/resolvers/metrics/metrics.module.ts
[notice] 3-3: Unused variable, import, function or class
Unused import CpuPowerService.
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts
[notice] 2-2: Unused variable, import, function or class
Unused import fsConstants.
[notice] 3-3: Unused variable, import, function or class
Unused imports access, readFile, readdir.
[notice] 4-4: Unused variable, import, function or class
Unused imports basename, join.
🔇 Additional comments (7)
packages/unraid-shared/src/pubsub/graphql.pubsub.ts (1)
8-8: LGTM!The CPU_POWER channel addition is consistent with existing patterns and properly supports the new CPU power metrics feature.
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts (1)
3-3: LGTM!The test properly mocks the CpuPowerService dependency and updates expectations to include the new power field. The mock implementation is appropriately simple for unit testing.
Also applies to: 92-99, 130-130
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.ts (1)
8-8: LGTM!The integration test correctly includes CpuPowerService in the DI container, enabling proper integration testing of CPU power metrics.
Also applies to: 26-26
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.ts (1)
6-6: LGTM!The test correctly accommodates the new CpuPowerService dependency and updates the topic registration expectation from 2 to 3 to account for the new CPU_POWER topic.
Also applies to: 22-22, 168-168, 176-176
api/src/unraid-api/graph/resolvers/info/cpu/cpu.module.ts (1)
1-10: LGTM!The CpuModule is well-structured and follows NestJS conventions. The module properly encapsulates CPU-related services and exports them for use by other modules.
api/src/unraid-api/graph/resolvers/info/info.resolver.integration.spec.ts (1)
9-9: LGTM!The integration test correctly includes CpuPowerService in the DI container, ensuring the new CPU power functionality is properly tested.
Also applies to: 32-32
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts (1)
91-101: Approve CPU power subscription wiringPUBSUB_CHANNEL.CPU_POWER is defined in the shared GRAPHQL_PUBSUB_CHANNEL enum.
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (4)
api/generated-schema.graphql (1)
2669-2671: Regenerate GraphQL client typings for new subscriptions.Clients likely miss
systemMetricsCpuPowerandsystemMetricsCpuTelemetry. Regenerate the codegen outputs across CLI/plugin/web.#!/bin/bash # Locate client graphql typings fd 'graphql.ts' | sed -n '1,200p' # Check presence of new fields rg -n 'systemMetricsCpuPower|systemMetricsCpuTelemetry' -t ts -C2 || true # Run project codegen pnpm run generate || npm run generate || yarn generateapi/src/unraid-api/graph/resolvers/metrics/metrics.module.ts (1)
3-5: Remove unused imports (CodeQL warning).
CpuTopologyServiceandCpuServiceare not referenced in this module.import { Module } from '@nestjs/common'; -import { CpuTopologyService } from '@app/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.js'; -import { CpuModule } from '@app/unraid-api/graph/resolvers/info/cpu/cpu.module.js'; +import { CpuModule } from '@app/unraid-api/graph/resolvers/info/cpu/cpu.module.js'; -import { CpuService } from '@app/unraid-api/graph/resolvers/info/cpu/cpu.service.js'; import { MemoryService } from '@app/unraid-api/graph/resolvers/info/memory/memory.service.js'; import { MetricsResolver } from '@app/unraid-api/graph/resolvers/metrics/metrics.resolver.js'; import { ServicesModule } from '@app/unraid-api/graph/services/services.module.js';api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts (1)
2-4: Drop unused Node fs/path imports.These are not used and trigger CodeQL “unused import” notices.
-import { constants as fsConstants } from 'node:fs'; -import { access, readdir, readFile } from 'node:fs/promises'; -import { basename, join } from 'node:path';api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts (1)
66-74: Comment interval mismatch (should be 5s).The code uses 5000ms; comment says 1 second. Please update.
- // Register CPU power polling with 1 second interval + // Register CPU power polling with 5 second interval
🧹 Nitpick comments (6)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts (1)
20-23: Consider removing unused injection until needed.
CpuPowerServiceis injected but unused here; keep only if a field resolver consumes it elsewhere.If unused across the service, remove it from the constructor to avoid dead deps. Otherwise, add a TODO indicating which resolver uses it.
api/generated-schema.graphql (1)
1404-1412: Field naming clarity:coresPowerseems per-package.If values are per physical package, consider renaming to
packagesPowerorperPackagePowerfor clarity before GA.api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts (2)
175-177: Harden package-id inference for non-package-Xlabels.Fallback to parsing the domain path (e.g.,
intel-rapl:1) likeCpuPowerServicedoes.- const pkgMatch = label.match(/package-(\d+)/i); - const pkgId = pkgMatch ? Number(pkgMatch[1]) : 0; + const pkgMatch = label.match(/package-(\d+)/i); + let pkgId = pkgMatch ? Number(pkgMatch[1]) : 0; + if (!pkgMatch) { + const base = p.split('/').pop() ?? ''; + const m2 = base.match(/:(\d+)/); + if (m2) pkgId = Number(m2[1]); + }
12-13: Remove unused cache or implement usage.
topologyCacheis defined but never used; either wire it up or drop it.api/src/unraid-api/graph/resolvers/info/cpu/cpu.model.ts (2)
63-74: Name doesn’t match description; clarify per-core vs per-package.
coresPowersuggests per-core, but description says “each physical processor.” Either:
- rename to
packagePower(orprocessorsPower) if per-package; or- fix the description if it’s truly per-core.
export class CpuPower extends Node { @Field(() => Float, { nullable: true, description: 'CPU power in watts' }) totalPower?: number; - @Field(() => [Float], { - nullable: true, - description: 'CPU power in watts for each physical processor', - }) - coresPower?: number[]; + @Field(() => [Float], { + nullable: true, + description: 'CPU power in watts for each physical processor (package)', + }) + packagePower?: number[]; }
131-135: Type alias for readability of triple-nested arrays.Define a named alias (e.g.,
type CpuTopology = number[][][];) and use it here to improve readability and future reuse.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
api/generated-schema.graphql(3 hunks)api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/cpu/cpu.model.ts(4 hunks)api/src/unraid-api/graph/resolvers/info/cpu/cpu.module.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts(1 hunks)api/src/unraid-api/graph/resolvers/metrics/metrics.module.ts(1 hunks)api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.ts(2 hunks)api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.ts
🧰 Additional context used
📓 Path-based instructions (3)
api/src/unraid-api/**
📄 CodeRabbit inference engine (.cursor/rules/api-rules.mdc)
Prefer adding new files to the Nest repo at api/src/unraid-api/ instead of legacy code
Files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.module.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.service.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.module.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.model.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript import specifiers with .js extensions for ESM compatibility
Never use the any type; prefer precise typing
Avoid type casting; model proper types from the start
Files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.module.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.service.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.module.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.model.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts
api/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
cache-manager v7 TTL values must be in milliseconds (e.g., 600000 for 10 minutes)
Files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.module.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.service.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.module.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.model.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts
🧬 Code graph analysis (6)
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts (1)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts (1)
Injectable(16-78)
api/src/unraid-api/graph/resolvers/metrics/metrics.module.ts (3)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.module.ts (1)
Module(7-11)api/src/unraid-api/graph/resolvers/resolvers.module.ts (1)
Module(37-76)api/src/unraid-api/graph/services/services.module.ts (1)
Module(8-13)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts (2)
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts (1)
Injectable(8-208)api/src/unraid-api/graph/resolvers/info/cpu/cpu-power.service.ts (1)
Injectable(8-136)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.module.ts (2)
api/src/unraid-api/graph/resolvers/metrics/metrics.module.ts (1)
Module(10-15)api/src/unraid-api/graph/resolvers/info/info.module.ts (1)
Module(17-37)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.model.ts (2)
api/src/unraid-api/graph/resolvers/info/info.model.ts (1)
ObjectType(13-44)api/src/unraid-api/cli/generated/graphql.ts (1)
Node(1371-1373)
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts (3)
api/src/core/pubsub.ts (1)
pubsub(12-12)api/src/unraid-api/cli/generated/graphql.ts (1)
Subscription(2046-2058)web/composables/gql/graphql.ts (1)
Subscription(2032-2044)
🪛 GitHub Actions: CI - Main (API)
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts
[error] 199-199: TS2345: Argument of type '{ id: number; power: number; temp: number; }' is not assignable to parameter of type 'never'.
api/generated-schema.graphql
[error] 1-1: Build failed during code generation. GraphQL Code Generator could not load schema from './generated-schema.graphql': Unknown type: 'number'. Command failed: 'tsx ./scripts/build.ts'.
🪛 GitHub Check: CodeQL
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts
[notice] 182-182: Unused variable, import, function or class
Unused variable pkgId.
api/src/unraid-api/graph/resolvers/metrics/metrics.module.ts
[notice] 3-3: Unused variable, import, function or class
Unused import CpuTopologyService.
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts
[notice] 2-2: Unused variable, import, function or class
Unused import fsConstants.
[notice] 3-3: Unused variable, import, function or class
Unused imports access, readFile, readdir.
[notice] 4-4: Unused variable, import, function or class
Unused imports basename, join.
🔇 Additional comments (4)
api/src/unraid-api/graph/resolvers/metrics/metrics.module.ts (1)
11-13: Module wiring looks good.Importing
CpuModuleand trimming providers is correct.api/src/unraid-api/graph/resolvers/info/cpu/cpu.module.ts (1)
7-11: LGTM — module composition is correct.Providers and exports align with consumers; ESM
.jspaths follow our guideline.api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts (2)
24-31: Module wiring is correctly configured; no action required.The verification confirms that
MetricsModuleimportsCpuModule(line 11 of metrics.module.ts), which provides bothCpuPowerServiceandCpuTopologyService. Dependency injection will resolve these services correctly in the constructor.
43-64: The review suggestion to changetotalpowertototalPoweris incorrect.The original code is already type-safe. The
CpuPackagesmodel inapi/src/unraid-api/graph/resolvers/info/cpu/cpu.model.tsdefines the property astotalpower(lowercase 'p') at line 45, so the code in metrics.resolver.ts is correct. Changing it tototalPowerwould break the type contract and cause TypeScript errors.The only valid suggestion is a minor comment clarity: line 43 could say "Register CPU telemetry polling" instead of "Register CPU power polling" to align with the channel name
CPU_TELEMETRY, but this is a cosmetic improvement, not a code fix.Likely an incorrect or invalid review comment.
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts
Outdated
Show resolved
Hide resolved
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (4)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.model.ts (2)
42-52: Align GraphQL nullability and casing; make fields required.The TypeScript properties are optional (
?) while GraphQL fields are non-null (nonullable: true), which can cause "Cannot return null for non-nullable field" errors. Additionally,totalpoweruses inconsistent casing compared toCpuPower.totalPower.Apply this diff:
@ObjectType() export class CpuPackages { - @Field(() => Float, { description: 'Total CPU package power draw (W)' }) - totalpower?: number; + @Field(() => Float, { description: 'Total CPU package power draw (W)' }) + totalPower!: number; - @Field(() => [Float], { description: 'Power draw per package (W)' }) - power?: number[]; + @Field(() => [Float], { description: 'Power draw per package (W)' }) + power!: number[]; - @Field(() => [Float], { description: 'Temperature per package (°C)' }) - temp?: number[]; + @Field(() => [Float], { description: 'Temperature per package (°C)' }) + temp!: number[]; }Based on past review comments.
136-137: Non-null GraphQL field should have definite assignment.The
packagesfield is non-null in GraphQL but lacks definite assignment in TypeScript. Use!to satisfy strict property initialization.Apply this diff:
- @Field(() => CpuPackages) - packages: CpuPackages; + @Field(() => CpuPackages) + packages!: CpuPackages;Based on past review comments.
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts (2)
36-44: Fix total power calculation; filter sentinel values and align field naming.The calculation
pkg.power ?? 0includes-1sentinel values in the sum, producing incorrect totals. Additionally, the field nametotalpowerdoesn't match the GraphQL model's camelCase conventiontotalPower.Apply this diff:
// Compute total power (2 decimals) const totalpower = - Math.round(packageList.reduce((sum, pkg) => sum + (pkg.power ?? 0), 0) * 100) / 100; + Math.round( + packageList.reduce( + (sum, pkg) => sum + (typeof pkg.power === 'number' && pkg.power >= 0 ? pkg.power : 0), + 0 + ) * 100 + ) / 100; // Build CpuPackages object const packages: CpuPackages = { - totalpower, + totalPower: totalpower, power: packageList.map((pkg) => pkg.power ?? -1), temp: packageList.map((pkg) => pkg.temp ?? -1), };Based on past review comments.
2-4: Remove unused imports.The imports
fsConstants,access,readFile,readdir,basename, andjoinare not used in this file. They appear to be leftover from code that was moved toCpuPowerServiceandCpuTopologyService.Apply this diff:
import { Injectable, Logger } from '@nestjs/common'; -import { constants as fsConstants } from 'node:fs'; -import { access, readdir, readFile } from 'node:fs/promises'; -import { basename, join } from 'node:path'; import { cpu, cpuFlags, currentLoad } from 'systeminformation';
🧹 Nitpick comments (1)
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts (1)
45-68: Consolidate duplicate CPU telemetry logic.The telemetry calculation logic (lines 51-59) duplicates similar code in
cpu.service.ts(lines 36-44). Both computetotalpowerfrompackageListand build aCpuPackagesobject. Consider extracting this into a shared method inCpuTopologyService.Additionally, this uses
Number().toFixed(2)whilecpu.service.tsusesMath.round() * 100 / 100for the same purpose—standardize the rounding approach.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.model.ts(4 hunks)api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts(1 hunks)api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.ts(3 hunks)api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts(4 hunks)packages/unraid-shared/src/pubsub/graphql.pubsub.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript import specifiers with .js extensions for ESM compatibility
Never use the any type; prefer precise typing
Avoid type casting; model proper types from the start
Files:
packages/unraid-shared/src/pubsub/graphql.pubsub.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.model.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.service.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.ts
api/src/unraid-api/**
📄 CodeRabbit inference engine (.cursor/rules/api-rules.mdc)
Prefer adding new files to the Nest repo at api/src/unraid-api/ instead of legacy code
Files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu.model.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.service.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.ts
api/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
cache-manager v7 TTL values must be in milliseconds (e.g., 600000 for 10 minutes)
Files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu.model.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.service.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.ts
api/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/api-rules.mdc)
api/**/*.{test,spec}.{js,jsx,ts,tsx}: Use Vitest for tests in the api; do not use Jest
Prefer not to mock simple dependencies in tests
For error testing, use.rejects.toThrow()without arguments; avoid asserting exact error messages unless the message format is the subject under test
Files:
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.ts
api/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
api/**/*.{test,spec}.{ts,tsx}: API test suite is Vitest; do not use Jest
Prefer not to mock simple dependencies in API tests
Files:
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.ts
{api,web}/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
{api,web}/**/*.{test,spec}.{ts,tsx}: For error tests, use .rejects.toThrow() without arguments; avoid asserting exact error messages unless that format is the subject
Focus tests on behavior, not implementation details
Avoid brittle tests tied to exact error or log wording
Use mocks as nouns, not verbs
Always await async operations before making assertions
Place all mock declarations at the top level; use factory functions for module mocks; clear mocks between tests
Files:
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.ts
🧠 Learnings (1)
📚 Learning: 2025-09-12T01:36:59.838Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T01:36:59.838Z
Learning: Applies to api/**/*.{test,spec}.{ts,tsx} : API test suite is Vitest; do not use Jest
Applied to files:
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.ts
🧬 Code graph analysis (3)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.model.ts (2)
api/src/unraid-api/graph/resolvers/info/info.model.ts (1)
ObjectType(13-44)api/src/unraid-api/cli/generated/graphql.ts (1)
Node(1371-1373)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts (2)
api/src/unraid-api/graph/resolvers/info/cpu/cpu-power.service.ts (1)
Injectable(8-136)api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts (1)
Injectable(8-208)
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts (3)
api/src/core/pubsub.ts (1)
pubsub(12-12)api/src/unraid-api/cli/generated/graphql.ts (1)
Subscription(2046-2058)web/composables/gql/graphql.ts (1)
Subscription(2032-2044)
🪛 GitHub Check: CodeQL
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts
[notice] 2-2: Unused variable, import, function or class
Unused import fsConstants.
[notice] 3-3: Unused variable, import, function or class
Unused imports access, readFile, readdir.
[notice] 4-4: Unused variable, import, function or class
Unused imports basename, join.
🔇 Additional comments (1)
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts (1)
135-157: LGTM: New subscriptions correctly implemented.The
systemMetricsCpuPowerandsystemMetricsCpuTelemetrysubscriptions follow the established pattern with proper permissions, resolve functions, and channel bindings.
| CpuPowerService, | ||
| CpuTopologyService, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Mock CpuPowerService and CpuTopologyService for unit tests.
These services read from the filesystem (/sys/class/powercap, /sys/devices/system/cpu) and are not simple dependencies. In unit tests, they should be mocked to avoid filesystem dependencies and ensure tests run in isolation.
Apply this diff:
- CpuPowerService,
- CpuTopologyService,
+ {
+ provide: CpuPowerService,
+ useValue: {
+ generateCpuPower: vi.fn().mockResolvedValue({
+ id: 'info/cpu-power',
+ totalPower: 65.5,
+ coresPower: [32.5, 33.0],
+ }),
+ },
+ },
+ {
+ provide: CpuTopologyService,
+ useValue: {
+ generateTelemetry: vi.fn().mockResolvedValue([
+ { id: 0, power: 32.5, temp: 45.0 },
+ { id: 1, power: 33.0, temp: 46.0 },
+ ]),
+ generateTopology: vi.fn().mockResolvedValue([
+ [[0, 1], [2, 3]],
+ [[4, 5], [6, 7]],
+ ]),
+ },
+ },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| CpuPowerService, | |
| CpuTopologyService, | |
| { | |
| provide: CpuPowerService, | |
| useValue: { | |
| generateCpuPower: vi.fn().mockResolvedValue({ | |
| id: 'info/cpu-power', | |
| totalPower: 65.5, | |
| coresPower: [32.5, 33.0], | |
| }), | |
| }, | |
| }, | |
| { | |
| provide: CpuTopologyService, | |
| useValue: { | |
| generateTelemetry: vi.fn().mockResolvedValue([ | |
| { id: 0, power: 32.5, temp: 45.0 }, | |
| { id: 1, power: 33.0, temp: 46.0 }, | |
| ]), | |
| generateTopology: vi.fn().mockResolvedValue([ | |
| [[0, 1], [2, 3]], | |
| [[4, 5], [6, 7]], | |
| ]), | |
| }, | |
| }, |
🤖 Prompt for AI Agents
In api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.ts around
lines 23 to 24, CpuPowerService and CpuTopologyService are real
filesystem-reading services and must be replaced with test doubles; update the
test module setup to provide mock implementations (e.g., use providers with
useValue or jest mocks) for CpuPowerService and CpuTopologyService that return
deterministic, minimal data used by the resolver tests, and remove any reliance
on the real filesystem so tests run in isolation.
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.ts
Outdated
Show resolved
Hide resolved
| this.subscriptionTracker.registerTopic( | ||
| PUBSUB_CHANNEL.CPU_POWER, | ||
| async () => { | ||
| const payload = new CpuPower(); |
There was a problem hiding this comment.
Initialize CpuPower with required id field.
CpuPower extends Node, which requires an id field. Creating an instance without setting id will cause GraphQL validation errors.
Apply this diff:
- const payload = new CpuPower();
+ const payload = { id: 'info/cpu-power' } as CpuPower;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const payload = new CpuPower(); | |
| const payload = new CpuPower(); | |
| payload.id = 'info/cpu-power'; |
| const payload = new CpuPower(); | |
| const payload = new CpuPower({ id: 'info/cpu-power' }); |
🤖 Prompt for AI Agents
In api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts around line
74, the code creates a new CpuPower without setting the required Node.id field;
update the instantiation to provide a valid id (for example a generated/derived
string or UUID) when constructing CpuPower so the object includes id before
being returned or used, ensuring GraphQL validation passes.
| ARRAY = "ARRAY", | ||
| CPU_UTILIZATION = "CPU_UTILIZATION", | ||
| CPU_POWER = "CPU_POWER", | ||
| CPY_TELEMETRY = "CPU_TELEMETRY", |
There was a problem hiding this comment.
Fix typo in enum key name.
The enum key is misspelled as CPY_TELEMETRY (missing 'U') while the value correctly uses "CPU_TELEMETRY". This inconsistency can cause confusion and errors when referencing the enum.
Apply this diff:
- CPY_TELEMETRY = "CPU_TELEMETRY",
+ CPU_TELEMETRY = "CPU_TELEMETRY",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| CPY_TELEMETRY = "CPU_TELEMETRY", | |
| CPU_TELEMETRY = "CPU_TELEMETRY", |
🤖 Prompt for AI Agents
packages/unraid-shared/src/pubsub/graphql.pubsub.ts around line 9: the enum key
is misspelled as CPY_TELEMETRY while the value is "CPU_TELEMETRY"; rename the
enum key to CPU_TELEMETRY to match the value, and update all references/usages
in the codebase to use the corrected enum key to avoid runtime/compile errors.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/unraid-shared/deploy.sh (2)
40-40: Replace eval with array-based command execution.Using
evalwith constructed commands is error-prone and a security anti-pattern, even with quoted variables. Construct the rsync command as a bash array and invoke directly to avoid shell injection risks and improve reliability.Apply this diff to remove eval:
-# Replace the value inside the rsync command with the user's input -rsync_command="rsync -avz --delete --progress --stats -e ssh \"$source_directory/\" \"root@${server_name}:$destination_directory/\"" - -echo "Executing the following command:" -echo "$rsync_command" - -# Execute the rsync command and capture the exit code -eval "$rsync_command" -exit_code=$? +# Execute rsync using array to avoid eval +rsync_args=( + -avz + --delete + --progress + --stats + -e ssh + "$source_directory/" + "root@${server_name}:$destination_directory/" +) + +echo "Executing: rsync ${rsync_args[*]}" +rsync "${rsync_args[@]}" +exit_code=$?Also applies to: 46-46
56-65: Consider making sound notification optional for automated/CI environments.The sound notifications (lines 56–65) are a nice developer ergonomic feature but may not be appropriate in CI/automated environments. Consider adding an environment variable to suppress them:
if [[ -z "$QUIET_DEPLOY" ]]; then if [[ "$OSTYPE" == "darwin"* ]]; then afplay /System/Library/Sounds/Glass.aiff elif [[ "$OSTYPE" == "linux-gnu" ]]; then paplay /usr/share/sounds/freedesktop/stereo/complete.oga elif [[ "$OSTYPE" == "msys" || "$OSTYPE" == "win32" ]]; then powershell.exe -c "(New-Object Media.SoundPlayer 'C:\Windows\Media\Windows Default.wav').PlaySync()" fi fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/unraid-shared/deploy.sh(1 hunks)packages/unraid-shared/package.json(1 hunks)
🔇 Additional comments (1)
packages/unraid-shared/package.json (1)
24-25: No issues found—deploy.sh has proper executable permissions.The verification confirms that
packages/unraid-shared/deploy.shis tracked in git with mode100755(executable). The script reference in the npm script is correctly configured and ready for use.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (8)
api/generated-schema.graphql (3)
2659-2659: Regenerate GraphQL client typings for new subscription.
systemMetricsCpuTelemetrywas added. Ensure CLI/web/generated clients are updated so Subscription types include it; otherwise type/runtime mismatches will occur.
1404-1413: Schema blocker: invalid scalar and missingidin CpuPackages.
totalPower: numberis not valid GraphQL; useFloat.CpuPackages implements Nodebut lacksid: PrefixedID!.- Docstring malformed for
temp.Apply in the code-first model (
api/src/unraid-api/graph/resolvers/info/cpu/cpu.model.ts), then regenerate this file:type CpuPackages implements Node { - """Total CPU package power draw (W)""" - totalPower: number + id: PrefixedID! + """Total CPU package power draw (W)""" + totalPower: Float """Power draw per package (W)""" power: [Float!] - """description: 'Temperature per package (°C)""" + """Temperature per package (°C)""" temp: [Float!] }Also ensure the TS class either implements Node or declares
idwith@Field(() => PrefixedID). Regenerate schema and client types afterward.
1460-1462: Expose topology on InfoCpu to match service output.
CpuServicereturnstopology: number[][][]but the schema lacks this field. Add it to the model and regenerate.type InfoCpu implements Node { ... """CPU packages information""" packages: CpuPackages + """Per-package core/thread groupings (package -> cores -> thread IDs)""" + topology: [[[Int!]!]!] ... }api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts (3)
2-4: Remove unused imports.
fsConstants,access/readdir/readFile, andbasename/joinare unused here. Clean them up.-import { constants as fsConstants } from 'node:fs'; -import { access, readdir, readFile } from 'node:fs/promises'; -import { basename, join } from 'node:path';
32-33: Exclude sentinel values from total power.Summing
pkg.power ?? 0includes-1(unknown), underreporting totals. Ignore negatives.- const totalPower = - Math.round(packageList.reduce((sum, pkg) => sum + (pkg.power ?? 0), 0) * 100) / 100; + const totalPower = + Math.round( + packageList.reduce( + (sum, pkg) => sum + (typeof pkg.power === 'number' && pkg.power >= 0 ? pkg.power : 0), + 0 + ) * 100 + ) / 100;
36-40: If CpuPackages implements Node, includeid.Once the model is corrected, return an
id(e.g.,'info/cpu-packages') to satisfy the Node contract.- const packages: CpuPackages = { - totalPower, - power: packageList.map((pkg) => pkg.power ?? -1), - temp: packageList.map((pkg) => pkg.temp ?? -1), - }; + const packages: CpuPackages = { + // id required if CpuPackages implements Node + // id: 'info/cpu-packages', + totalPower, + power: packageList.map((pkg) => pkg.power ?? -1), + temp: packageList.map((pkg) => pkg.temp ?? -1), + };api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.ts (1)
166-172: Replace{}casts with typed stubs.Construct
MetricsResolverwith a real stub forCpuTopologyServiceinstead of{} as any.- const testModule = new MetricsResolver( - cpuService, - {} as any, - memoryService, - subscriptionTracker as any, - {} as any - ); + const cpuTopologyStub = { + generateTelemetry: vi.fn().mockResolvedValue([{ id: 0, power: 10, temp: 40 }]), + generateTopology: vi.fn().mockResolvedValue([[[0, 1]]]), + } as unknown as CpuTopologyService; + const helperStub = { createTrackedSubscription: vi.fn() } as SubscriptionHelperService; + const testModule = new MetricsResolver( + cpuService, + cpuTopologyStub, + memoryService, + subscriptionTracker as any, + helperStub as any + );api/src/unraid-api/graph/resolvers/info/cpu/cpu.model.ts (1)
42-52: Make CpuPackages fields required to match non-nullable GraphQL schema.The TypeScript properties are optional (
?) but the@Fielddecorators don't specifynullable: true, meaning GraphQL expects these fields to always be present. This mismatch can cause runtime errors if any field is omitted.Apply this diff:
@ObjectType() export class CpuPackages { @Field(() => Float, { description: 'Total CPU package power draw (W)' }) - totalPower?: number; + totalPower!: number; @Field(() => [Float], { description: 'Power draw per package (W)' }) - power?: number[]; + power!: number[]; @Field(() => [Float], { description: 'Temperature per package (°C)' }) - temp?: number[]; + temp!: number[]; }
🧹 Nitpick comments (4)
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.ts (1)
8-8: Mock CpuTopologyService in integration tests to avoid FS flakiness.Provide a lightweight stub via
useValue(return fixed topology/telemetry) instead of the real service. This keeps tests hermetic and faster, especially on CI.- CpuTopologyService, + { + provide: CpuTopologyService, + useValue: { + generateTelemetry: vi.fn().mockResolvedValue([ + { id: 0, power: 32.5, temp: 45.0 }, + { id: 1, power: 33.0, temp: 46.0 }, + ]), + generateTopology: vi.fn().mockResolvedValue([ + [[0, 1], [2, 3]], + [[4, 5], [6, 7]], + ]), + }, + },Also applies to: 26-26
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.ts (2)
6-6: Provide a stubbed CpuTopologyService in unit tests.Avoid the real FS-backed service; inject a simple stub via
useValue.- CpuTopologyService, + { + provide: CpuTopologyService, + useValue: { + generateTelemetry: vi.fn().mockResolvedValue([ + { id: 0, power: 32.5, temp: 45.0 }, + { id: 1, power: 33.0, temp: 46.0 }, + ]), + generateTopology: vi.fn().mockResolvedValue([ + [[0, 1], [2, 3]], + [[4, 5], [6, 7]], + ]), + }, + },Also applies to: 22-22
176-186: Also assert the CPU telemetry topic name.You expect three registrations; verify the third is the new CPU telemetry channel to prevent regressions.
testModule.onModuleInit(); expect(subscriptionTracker.registerTopic).toHaveBeenCalledTimes(3); expect(subscriptionTracker.registerTopic).toHaveBeenCalledWith( 'CPU_UTILIZATION', expect.any(Function), 1000 ); + expect(subscriptionTracker.registerTopic).toHaveBeenCalledWith( + 'CPU_TELEMETRY', + expect.any(Function), + expect.any(Number) + ); expect(subscriptionTracker.registerTopic).toHaveBeenCalledWith( 'MEMORY_UTILIZATION', expect.any(Function), 2000 );api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts (1)
61-61: Remove duplicate debug log.This debug statement is redundant—the same payload was already logged on line 54.
Apply this diff:
pubsub.publish(PUBSUB_CHANNEL.CPU_TELEMETRY, { systemMetricsCpuTelemetry: packages, }); - - this.logger.debug(`CPU_TELEMETRY payload2: ${JSON.stringify(packages)}`); },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
api/generated-schema.graphql(3 hunks)api/src/unraid-api/graph/resolvers/info/cpu/cpu.model.ts(2 hunks)api/src/unraid-api/graph/resolvers/info/cpu/cpu.module.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts(3 hunks)api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/info.resolver.integration.spec.ts(2 hunks)api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.ts(2 hunks)api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.ts(3 hunks)api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts(4 hunks)packages/unraid-shared/src/pubsub/graphql.pubsub.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- api/src/unraid-api/graph/resolvers/info/info.resolver.integration.spec.ts
- api/src/unraid-api/graph/resolvers/info/cpu/cpu.module.ts
- packages/unraid-shared/src/pubsub/graphql.pubsub.ts
🧰 Additional context used
📓 Path-based instructions (6)
api/src/unraid-api/**
📄 CodeRabbit inference engine (.cursor/rules/api-rules.mdc)
Prefer adding new files to the Nest repo at api/src/unraid-api/ instead of legacy code
Files:
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.model.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.service.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.ts
api/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/api-rules.mdc)
api/**/*.{test,spec}.{js,jsx,ts,tsx}: Use Vitest for tests in the api; do not use Jest
Prefer not to mock simple dependencies in tests
For error testing, use.rejects.toThrow()without arguments; avoid asserting exact error messages unless the message format is the subject under test
Files:
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript import specifiers with .js extensions for ESM compatibility
Never use the any type; prefer precise typing
Avoid type casting; model proper types from the start
Files:
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.model.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.service.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.ts
api/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
api/**/*.{test,spec}.{ts,tsx}: API test suite is Vitest; do not use Jest
Prefer not to mock simple dependencies in API tests
Files:
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.ts
{api,web}/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
{api,web}/**/*.{test,spec}.{ts,tsx}: For error tests, use .rejects.toThrow() without arguments; avoid asserting exact error messages unless that format is the subject
Focus tests on behavior, not implementation details
Avoid brittle tests tied to exact error or log wording
Use mocks as nouns, not verbs
Always await async operations before making assertions
Place all mock declarations at the top level; use factory functions for module mocks; clear mocks between tests
Files:
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.ts
api/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
cache-manager v7 TTL values must be in milliseconds (e.g., 600000 for 10 minutes)
Files:
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.model.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.service.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.ts
🧠 Learnings (2)
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to **/__test__/components/**/*.ts : Mock external dependencies and services in component tests (e.g., vi.mock for helper modules)
Applied to files:
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.ts
📚 Learning: 2025-08-09T01:03:29.676Z
Learnt from: elibosley
PR: unraid/api#1575
File: packages/unraid-shared/src/services/socket-config.service.spec.ts:10-13
Timestamp: 2025-08-09T01:03:29.676Z
Learning: Vitest is used for all testing across all repositories in the unraid organization, not Jest. Always use `vi` for mocking utilities, not `jest`.
Applied to files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts
🧬 Code graph analysis (3)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.model.ts (2)
api/src/unraid-api/graph/resolvers/info/info.model.ts (1)
ObjectType(13-44)api/src/unraid-api/graph/resolvers/metrics/metrics.model.ts (1)
ObjectType(8-21)
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts (3)
api/src/core/pubsub.ts (1)
pubsub(12-12)api/src/unraid-api/cli/generated/graphql.ts (1)
Subscription(2046-2058)web/composables/gql/graphql.ts (1)
Subscription(2032-2044)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts (2)
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts (1)
Injectable(8-208)api/src/unraid-api/cli/generated/graphql.ts (1)
InfoCpu(855-892)
🔇 Additional comments (3)
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts (2)
39-64: LGTM!The CPU telemetry subscription registration is well-structured: it aggregates package power with appropriate precision, handles missing data with -1 fallbacks, and publishes the payload correctly.
110-120: LGTM!The GraphQL subscription is properly configured with the correct return type, name, resolve function, and permissions.
api/src/unraid-api/graph/resolvers/info/cpu/cpu.model.ts (1)
116-122: LGTM!The new
topologyandpackagesfields are correctly declared with definite assignment (!) to match their non-nullable GraphQL schema declarations.
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts (1)
95-99: Fix mock: still has typo and missing required methods.Despite being marked as addressed in a previous commit, the mock still defines
generateCpuTotopology(typo) and lacks the methodsCpuServiceactually calls:generateTelemetry()andgenerateTopology(). This will cause runtime failures.Apply this diff to fix the mock:
- cpuTopologyService = { - generateCpuTotopology: vi.fn().mockResolvedValue({ id: 'info/cpu-topology' }), - } as any; + cpuTopologyService = { + generateTelemetry: vi.fn().mockResolvedValue([ + { id: 0, power: 32.5, temp: 45.0 }, + { id: 1, power: 33.0, temp: 46.0 }, + ]), + generateTopology: vi.fn().mockResolvedValue([ + [[0, 1], [2, 3]], + [[4, 5], [6, 7]], + ]), + } as unknown as CpuTopologyService;Based on learnings
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts (1)
195-195: Fix type inference: explicitly typeresultarray.TypeScript infers
resultasnever[], causing the push on line 199 to fail type checking. This is blocking the pipeline.- const result = []; + const result: { id: number; power: number; temp: number }[] = [];api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts (1)
27-28: Fix totalPower: exclude sentinel-1values from sum.
pkg.power ?? 0only handles null/undefined. Whenpkg.poweris-1(the sentinel for unavailable data), it gets added to the sum, producing incorrect totals.const totalPower = - Math.round(packageList.reduce((sum, pkg) => sum + (pkg.power ?? 0), 0) * 100) / 100; + Math.round( + packageList.reduce( + (sum, pkg) => sum + (typeof pkg.power === 'number' && pkg.power >= 0 ? pkg.power : 0), + 0 + ) * 100 + ) / 100;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts(3 hunks)api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
api/src/unraid-api/**
📄 CodeRabbit inference engine (.cursor/rules/api-rules.mdc)
Prefer adding new files to the Nest repo at api/src/unraid-api/ instead of legacy code
Files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.service.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts
api/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/api-rules.mdc)
api/**/*.{test,spec}.{js,jsx,ts,tsx}: Use Vitest for tests in the api; do not use Jest
Prefer not to mock simple dependencies in tests
For error testing, use.rejects.toThrow()without arguments; avoid asserting exact error messages unless the message format is the subject under test
Files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript import specifiers with .js extensions for ESM compatibility
Never use the any type; prefer precise typing
Avoid type casting; model proper types from the start
Files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.service.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts
api/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
api/**/*.{test,spec}.{ts,tsx}: API test suite is Vitest; do not use Jest
Prefer not to mock simple dependencies in API tests
Files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts
{api,web}/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
{api,web}/**/*.{test,spec}.{ts,tsx}: For error tests, use .rejects.toThrow() without arguments; avoid asserting exact error messages unless that format is the subject
Focus tests on behavior, not implementation details
Avoid brittle tests tied to exact error or log wording
Use mocks as nouns, not verbs
Always await async operations before making assertions
Place all mock declarations at the top level; use factory functions for module mocks; clear mocks between tests
Files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts
api/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
cache-manager v7 TTL values must be in milliseconds (e.g., 600000 for 10 minutes)
Files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.service.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts
🧠 Learnings (1)
📚 Learning: 2025-08-09T01:03:29.676Z
Learnt from: elibosley
PR: unraid/api#1575
File: packages/unraid-shared/src/services/socket-config.service.spec.ts:10-13
Timestamp: 2025-08-09T01:03:29.676Z
Learning: Vitest is used for all testing across all repositories in the unraid organization, not Jest. Always use `vi` for mocking utilities, not `jest`.
Applied to files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts
🧬 Code graph analysis (2)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts (3)
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts (1)
Injectable(8-208)api/src/unraid-api/cli/generated/graphql.ts (1)
InfoCpu(855-892)web/composables/gql/graphql.ts (1)
InfoCpu(842-879)
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts (1)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts (1)
Injectable(12-70)
🪛 GitHub Actions: CI - Main (API)
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts
[error] 199-199: TypeScript error TS2345: Argument of type '{ id: number; power: number; temp: number; }' is not assignable to parameter of type 'never'.
[error] 199-199: Command failed with exit code 1 while running 'pnpm run type-check' (tsc --noEmit).
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts (1)
53-53: Use consistent logging: preferthis.logger.warn.This line still uses
console.warninstead of the injected logger. The rest of the file consistently usesthis.logger.warn.- console.warn('Topology read error for', dir, err); + this.logger.warn('Topology read error for', dir, err);Based on coding guidelines.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
api/generated-schema.graphql(3 hunks)api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- api/generated-schema.graphql
🧰 Additional context used
📓 Path-based instructions (3)
api/src/unraid-api/**
📄 CodeRabbit inference engine (.cursor/rules/api-rules.mdc)
Prefer adding new files to the Nest repo at api/src/unraid-api/ instead of legacy code
Files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript import specifiers with .js extensions for ESM compatibility
Never use the any type; prefer precise typing
Avoid type casting; model proper types from the start
Files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts
api/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
cache-manager v7 TTL values must be in milliseconds (e.g., 600000 for 10 minutes)
Files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts
🧬 Code graph analysis (1)
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts (1)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts (1)
Injectable(12-70)
🪛 GitHub Check: Test API
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts
[failure] 5-5: src/unraid-api/app/test/app.module.integration.spec.ts
Error: [vitest] No "promises" export is defined on the "node:fs" mock. Did you forget to return it from "vi.mock"?
If you need to partially mock a module, you can use "importOriginal" helper inside:
vi.mock(import("node:fs"), async (importOriginal) => {
const actual = await importOriginal()
return {
...actual,
// your mocked methods
}
})
❯ src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts:5:39
❯ src/unraid-api/graph/resolvers/info/cpu/cpu.module.ts:3:1
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts
Outdated
Show resolved
Hide resolved
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts (1)
94-113: Consider adding explicit mock clearing for consistency.While the service mock is recreated in
beforeEach, consider adding explicit mock clearing for the module mocks to follow the guideline about clearing mocks between tests.As per coding guidelines.
beforeEach(() => { + vi.clearAllMocks(); + cpuTopologyService = {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
api/src/unraid-api/**
📄 CodeRabbit inference engine (.cursor/rules/api-rules.mdc)
Prefer adding new files to the Nest repo at api/src/unraid-api/ instead of legacy code
Files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts
api/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/api-rules.mdc)
api/**/*.{test,spec}.{js,jsx,ts,tsx}: Use Vitest for tests in the api; do not use Jest
Prefer not to mock simple dependencies in tests
For error testing, use.rejects.toThrow()without arguments; avoid asserting exact error messages unless the message format is the subject under test
Files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript import specifiers with .js extensions for ESM compatibility
Never use the any type; prefer precise typing
Avoid type casting; model proper types from the start
Files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts
api/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
api/**/*.{test,spec}.{ts,tsx}: API test suite is Vitest; do not use Jest
Prefer not to mock simple dependencies in API tests
Files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts
{api,web}/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
{api,web}/**/*.{test,spec}.{ts,tsx}: For error tests, use .rejects.toThrow() without arguments; avoid asserting exact error messages unless that format is the subject
Focus tests on behavior, not implementation details
Avoid brittle tests tied to exact error or log wording
Use mocks as nouns, not verbs
Always await async operations before making assertions
Place all mock declarations at the top level; use factory functions for module mocks; clear mocks between tests
Files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts
api/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
cache-manager v7 TTL values must be in milliseconds (e.g., 600000 for 10 minutes)
Files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts
🧠 Learnings (1)
📚 Learning: 2025-08-09T01:03:29.676Z
Learnt from: elibosley
PR: unraid/api#1575
File: packages/unraid-shared/src/services/socket-config.service.spec.ts:10-13
Timestamp: 2025-08-09T01:03:29.676Z
Learning: Vitest is used for all testing across all repositories in the unraid organization, not Jest. Always use `vi` for mocking utilities, not `jest`.
Applied to files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts
🔇 Additional comments (1)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts (1)
143-157: LGTM!The test expectations correctly verify the new
packagesandtopologystructure, withtotalPowerproperly computed as the sum of individual package powers and arrays matching the mock telemetry data.
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts
Outdated
Show resolved
Hide resolved
Signed-off-by: Simon Fairweather <simon.n.fairweather@gmail.com>
Signed-off-by: Simon Fairweather <simon.n.fairweather@gmail.com>
Signed-off-by: Simon Fairweather <simon.n.fairweather@gmail.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Signed-off-by: Simon Fairweather <simon.n.fairweather@gmail.com>
Signed-off-by: Simon Fairweather <simon.n.fairweather@gmail.com>
Signed-off-by: Simon Fairweather <simon.n.fairweather@gmail.com>
Signed-off-by: Simon Fairweather <simon.n.fairweather@gmail.com>
Signed-off-by: Simon Fairweather <simon.n.fairweather@gmail.com>
Signed-off-by: Simon Fairweather <simon.n.fairweather@gmail.com>
pujitm
force-pushed
the
feat/cpu-power
branch
from
7dd49d1 to
1aa6525
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
api/generated-schema.graphql (2)
1415-1471: Regenerate schema to add missingtopologyfield to InfoCpu.The model declares
topology!: number[][][];but this field is missing from the InfoCpu type in the generated schema. Regenerate to sync.
2650-2662: Regenerate clients to includesystemMetricsCpuTelemetrysubscription.Verification confirms that
systemMetricsCpuTelemetryis defined inapi/generated-schema.graphqlbut is absent from all generated client type files:
web/src/composables/gql/graphql.tsapi/src/unraid-api/cli/generated/graphql.tspackages/unraid-api-plugin-connect/src/graphql/generated/client/graphql.tsOther subscriptions are present in these files, confirming that codegen has run but is now out of sync with the schema. Run
pnpm codegento regenerate all clients and avoid type/runtime mismatches.
♻️ Duplicate comments (5)
packages/unraid-shared/deploy.sh (3)
37-37: Add error handling for SSH commands.Lines 37 and 50 execute SSH operations without checking for failure. If these commands fail (connection issue, permission denied, or command execution failure), the script silently continues and may deploy a broken state. The issue was flagged in a previous review and remains unresolved.
Apply this diff to add error checking:
# Create destination directory on remote server ssh root@"${server_name}" "mkdir -p $destination_directory" + if [ $? -ne 0 ]; then + echo "Error: Failed to create remote directory on $server_name!" + exit 1 + fi # (... rsync execution ...) # Chown the directory ssh root@"${server_name}" "chown -R root:root /usr/local/unraid-api/node_modules/@unraid/" + if [ $? -ne 0 ]; then + echo "Error: Failed to change ownership on remote $server_name!" + exit 1 + fiAlso applies to: 53-53
Also applies to: 50-50
50-50: Document SSH key setup and narrow ownership scope.The script uses
root@${server_name}for SSH and recursively changes permissions on the entire@unraiddirectory. This was also flagged previously and remains unaddressed. Ensure the following are documented:
- SSH key-based authentication must be configured (passwordless root access).
- Whether the full
chown -Ron the entire@unraid/directory is necessary—consider narrowing to just thesharedpackage to reduce blast radius.- Root SSH access requirement and security implications.
Consider narrowing the chown scope:
- ssh root@"${server_name}" "chown -R root:root /usr/local/unraid-api/node_modules/@unraid/" + ssh root@"${server_name}" "chown -R root:root /usr/local/unraid-api/node_modules/@unraid/shared"
37-53: Fix exit code logic: SSH commands execute after rsync exit code is captured.The script captures the rsync exit code at line 47, but then executes SSH commands on lines 50 and 53. If these SSH commands fail, the script exits with the rsync code (line 68), not the actual failure reason. This masks deployment failures and should be restructured so all critical operations check for success before proceeding.
Consider restructuring to check and exit on each critical operation:
# Create destination directory on remote server ssh root@"${server_name}" "mkdir -p $destination_directory" || { + echo "Error: Failed to create remote directory!" + exit 1 + } # ... rsync and other operations ... # Chown the directory - ssh root@"${server_name}" "chown -R root:root /usr/local/unraid-api/node_modules/@unraid/" + ssh root@"${server_name}" "chown -R root:root /usr/local/unraid-api/node_modules/@unraid/" || { + echo "Error: Failed to change ownership!" + exit 1 + } # Run unraid-api restart on remote host - ssh root@"${server_name}" 'INTROSPECTION=true LOG_LEVEL=trace unraid-api restart' + ssh root@"${server_name}" 'INTROSPECTION=true LOG_LEVEL=trace unraid-api restart' || { + echo "Error: Failed to restart unraid-api!" + exit 1 + } - # Exit with the rsync command's exit code - exit $exit_code + exit 0api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts (1)
26-35: Exclude sentinel values when summing totalPower.
pkg.power ?? 0will add -1, skewing totals. Only include non-negative numbers.- const totalPower = - Math.round(packageList.reduce((sum, pkg) => sum + (pkg.power ?? 0), 0) * 100) / 100; + const totalPower = + Math.round( + packageList.reduce( + (sum, pkg) => sum + (typeof pkg.power === 'number' && pkg.power >= 0 ? pkg.power : 0), + 0, + ) * 100, + ) / 100;api/generated-schema.graphql (1)
1404-1413: Schema invalid:CpuPackages implements Nodebut lacksid.Either add
id: PrefixedID!here or (preferred) update the code-first model to extend/implement Node and regenerate this file.Suggested model fix (then regenerate):
-@ObjectType() -export class CpuPackages { +@ObjectType({ implements: () => Node }) +export class CpuPackages extends Node { @Field(() => Float, { description: 'Total CPU package power draw (W)' }) - totalPower?: number; + totalPower!: number; @Field(() => [Float], { description: 'Power draw per package (W)' }) - power?: number[]; + power!: number[]; @Field(() => [Float], { description: 'Temperature per package (°C)' }) - temp?: number[]; + temp!: number[]; }
🧹 Nitpick comments (8)
packages/unraid-shared/deploy.sh (2)
40-40: Quote the destination path in rsync command.Line 40 expands
$destination_directorywithin a double-quoted eval string. Although the current path is safe, adding quotes prevents potential issues if the path ever contains spaces or special characters.- rsync_command="rsync -avz --delete --progress --stats -e ssh \"$source_directory/\" \"root@${server_name}:$destination_directory/\"" + rsync_command="rsync -avz --delete --progress --stats -e ssh \"$source_directory/\" \"root@${server_name}:${destination_directory}/\""
55-65: Handle missing sound files gracefully.The sound notification uses hard-coded paths that may not exist on all systems (e.g.,
/usr/share/sounds/freedesktop/stereo/complete.ogamay not be present on minimal Linux installations). Consider adding existence checks or usingstderrredirection to suppress errors.if [[ "$OSTYPE" == "darwin"* ]]; then # macOS - afplay /System/Library/Sounds/Glass.aiff + afplay /System/Library/Sounds/Glass.aiff 2>/dev/null || true elif [[ "$OSTYPE" == "linux-gnu" ]]; then # Linux - paplay /usr/share/sounds/freedesktop/stereo/complete.oga + paplay /usr/share/sounds/freedesktop/stereo/complete.oga 2>/dev/null || true elif [[ "$OSTYPE" == "msys" || "$OSTYPE" == "win32" ]]; then # Windows - powershell.exe -c "(New-Object Media.SoundPlayer 'C:\Windows\Media\Windows Default.wav').PlaySync()" + powershell.exe -c "(New-Object Media.SoundPlayer 'C:\Windows\Media\Windows Default.wav').PlaySync()" 2>/dev/null || true fiapi/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts (2)
95-110: Avoidas anyon the service mock; use a typed Vitest mock.Replace the unsafe cast with a precise mocked type to satisfy our “no any” guideline. Based on learnings.
- cpuTopologyService = { + cpuTopologyService = { generateTopology: vi.fn().mockResolvedValue([ @@ generateTelemetry: vi.fn().mockResolvedValue([ { power: 32.5, temp: 45.0 }, { power: 33.0, temp: 46.0 }, ]), - } as any; + } as unknown as vi.Mocked<Pick<CpuTopologyService, 'generateTopology' | 'generateTelemetry'>>;
161-179: Prefer ts-expect-error overas anyfor partial systeminformation mock.Document the intentional type bypass and drop
as any. Based on learnings.- vi.mocked(cpu).mockResolvedValueOnce({ + // @ts-expect-error test-only partial shape of systeminformation.cpu() + vi.mocked(cpu).mockResolvedValueOnce({ manufacturer: 'Intel', @@ - } as any); + });api/src/unraid-api/graph/resolvers/info/info.module.ts (1)
4-6: Remove unused CpuService import.CpuService is provided via CpuModule; this import is unused.
import { CpuModule } from '@app/unraid-api/graph/resolvers/info/cpu/cpu.module.js'; -import { CpuService } from '@app/unraid-api/graph/resolvers/info/cpu/cpu.service.js';api/generated-schema.graphql (1)
1411-1412: Docstring typo.Drop the accidental “description:” prefix.
- """description: 'Temperature per package (°C)""" + """Temperature per package (°C)"""api/src/unraid-api/graph/resolvers/metrics/metrics.module.ts (1)
3-6: Remove unused imports (CpuTopologyService, CpuService).They aren’t referenced in this module file.
-import { CpuTopologyService } from '@app/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.js'; import { CpuModule } from '@app/unraid-api/graph/resolvers/info/cpu/cpu.module.js'; -import { CpuService } from '@app/unraid-api/graph/resolvers/info/cpu/cpu.service.js';api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts (1)
54-62: Drop duplicate debug log.Two identical logs add noise.
- this.logger.debug(`CPU_TELEMETRY payload: ${JSON.stringify(packages)}`); @@ - this.logger.debug(`CPU_TELEMETRY payload2: ${JSON.stringify(packages)}`); + this.logger.debug(`CPU_TELEMETRY payload: ${JSON.stringify(packages)}`);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
api/generated-schema.graphql(3 hunks)api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/cpu/cpu.model.ts(2 hunks)api/src/unraid-api/graph/resolvers/info/cpu/cpu.module.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts(3 hunks)api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/info.module.ts(2 hunks)api/src/unraid-api/graph/resolvers/info/info.resolver.integration.spec.ts(2 hunks)api/src/unraid-api/graph/resolvers/metrics/metrics.module.ts(1 hunks)api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.ts(2 hunks)api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.ts(3 hunks)api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts(4 hunks)packages/unraid-shared/deploy.sh(1 hunks)packages/unraid-shared/package.json(1 hunks)packages/unraid-shared/src/pubsub/graphql.pubsub.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- api/src/unraid-api/graph/resolvers/info/cpu/cpu.module.ts
- api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.ts
- api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.ts
- api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts
- packages/unraid-shared/src/pubsub/graphql.pubsub.ts
- api/src/unraid-api/graph/resolvers/info/info.resolver.integration.spec.ts
🧰 Additional context used
📓 Path-based instructions (6)
api/src/unraid-api/**
📄 CodeRabbit inference engine (.cursor/rules/api-rules.mdc)
Prefer adding new files to the Nest repo at api/src/unraid-api/ instead of legacy code
Files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.tsapi/src/unraid-api/graph/resolvers/info/info.module.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.model.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.service.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.module.ts
api/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/api-rules.mdc)
api/**/*.{test,spec}.{js,jsx,ts,tsx}: Use Vitest for tests in the api; do not use Jest
Prefer not to mock simple dependencies in tests
For error testing, use.rejects.toThrow()without arguments; avoid asserting exact error messages unless the message format is the subject under test
Files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript import specifiers with .js extensions for ESM compatibility
Never use the any type; prefer precise typing
Avoid type casting; model proper types from the start
Files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.tsapi/src/unraid-api/graph/resolvers/info/info.module.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.model.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.service.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.module.ts
api/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
api/**/*.{test,spec}.{ts,tsx}: API test suite is Vitest; do not use Jest
Prefer not to mock simple dependencies in API tests
Files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts
{api,web}/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
{api,web}/**/*.{test,spec}.{ts,tsx}: For error tests, use .rejects.toThrow() without arguments; avoid asserting exact error messages unless that format is the subject
Focus tests on behavior, not implementation details
Avoid brittle tests tied to exact error or log wording
Use mocks as nouns, not verbs
Always await async operations before making assertions
Place all mock declarations at the top level; use factory functions for module mocks; clear mocks between tests
Files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts
api/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
cache-manager v7 TTL values must be in milliseconds (e.g., 600000 for 10 minutes)
Files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.tsapi/src/unraid-api/graph/resolvers/info/info.module.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.model.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.service.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.module.ts
🧠 Learnings (2)
📚 Learning: 2025-03-07T19:29:12.540Z
Learnt from: pujitm
PR: unraid/api#1219
File: api/src/unraid-api/main.ts:18-25
Timestamp: 2025-03-07T19:29:12.540Z
Learning: When handling type mismatches between dependencies in TypeScript (such as between NestJS and Fastify), prefer using `ts-expect-error` with a detailed explanation comment instead of `as any` type assertions. This better documents the reason for the type bypass and makes the issue more visible for future maintenance.
Applied to files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts
📚 Learning: 2025-08-09T01:03:29.676Z
Learnt from: elibosley
PR: unraid/api#1575
File: packages/unraid-shared/src/services/socket-config.service.spec.ts:10-13
Timestamp: 2025-08-09T01:03:29.676Z
Learning: Vitest is used for all testing across all repositories in the unraid organization, not Jest. Always use `vi` for mocking utilities, not `jest`.
Applied to files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts
🧬 Code graph analysis (4)
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts (3)
api/src/core/pubsub.ts (1)
pubsub(12-12)api/src/unraid-api/cli/generated/graphql.ts (1)
Subscription(2046-2058)web/composables/gql/graphql.ts (1)
Subscription(2032-2044)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.model.ts (2)
api/src/unraid-api/graph/resolvers/info/info.model.ts (1)
ObjectType(13-44)api/src/unraid-api/graph/resolvers/metrics/metrics.model.ts (1)
ObjectType(8-21)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts (3)
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts (1)
Injectable(6-211)api/src/unraid-api/cli/generated/graphql.ts (1)
InfoCpu(855-892)web/composables/gql/graphql.ts (1)
InfoCpu(842-879)
api/src/unraid-api/graph/resolvers/metrics/metrics.module.ts (2)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.module.ts (1)
Module(6-10)api/src/unraid-api/graph/services/services.module.ts (1)
Module(8-13)
🪛 GitHub Check: CodeQL
api/src/unraid-api/graph/resolvers/metrics/metrics.module.ts
[notice] 3-3: Unused variable, import, function or class
Unused import CpuTopologyService.
🔇 Additional comments (5)
packages/unraid-shared/package.json (1)
20-26: Script configuration looks good.The JSON is well-formed, and the new
unraid:deployscript is properly registered. No concerns here.api/src/unraid-api/graph/resolvers/info/info.module.ts (1)
18-18: LGTM on CpuModule wiring.Importing CpuModule here is the right DI boundary.
api/src/unraid-api/graph/resolvers/metrics/metrics.module.ts (1)
11-13: LGTM on DI: consume CpuService/CpuTopologyService via CpuModule.Providers list is correct.
api/src/unraid-api/graph/resolvers/info/cpu/cpu.model.ts (1)
116-123: Definite assignment fortopologyandpackagesis correct.This matches strict initialization and schema.
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts (1)
110-120: Subscription wiring looks correct.Field name and payload key match; permissions applied.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts (1)
59-67: Remove duplicate debug log statement.Line 66 logs the same payload that was already logged on line 59. This is redundant.
Apply this diff:
this.logger.debug(`CPU_TELEMETRY payload: ${JSON.stringify(packages)}`); // Publish the payload pubsub.publish(PUBSUB_CHANNEL.CPU_TELEMETRY, { systemMetricsCpuTelemetry: packages, }); - - this.logger.debug(`CPU_TELEMETRY payload2: ${JSON.stringify(packages)}`); },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
api/generated-schema.graphql(3 hunks)api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/cpu/cpu.model.ts(2 hunks)api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts(3 hunks)api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts(1 hunks)api/src/unraid-api/graph/resolvers/metrics/metrics.module.ts(1 hunks)api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.ts(3 hunks)api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- api/generated-schema.graphql
- api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.ts
🧰 Additional context used
🧠 Learnings (11)
📚 Learning: 2025-03-07T19:29:12.540Z
Learnt from: pujitm
Repo: unraid/api PR: 1219
File: api/src/unraid-api/main.ts:18-25
Timestamp: 2025-03-07T19:29:12.540Z
Learning: When handling type mismatches between dependencies in TypeScript (such as between NestJS and Fastify), prefer using `ts-expect-error` with a detailed explanation comment instead of `as any` type assertions. This better documents the reason for the type bypass and makes the issue more visible for future maintenance.
Applied to files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts
📚 Learning: 2025-09-09T14:34:07.189Z
Learnt from: pujitm
Repo: unraid/api PR: 1582
File: packages/unraid-shared/src/util/processing.ts:69-71
Timestamp: 2025-09-09T14:34:07.189Z
Learning: In TypeScript code, pujitm considers it acceptable to use `any` type for private/internal fields when the type is not exposed through the public API and is masked by properly typed method declarations. The focus is on ensuring the public interface maintains type safety rather than enforcing strict typing on all internal implementation details.
Applied to files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts
📚 Learning: 2025-08-09T01:03:29.676Z
Learnt from: elibosley
Repo: unraid/api PR: 1575
File: packages/unraid-shared/src/services/socket-config.service.spec.ts:10-13
Timestamp: 2025-08-09T01:03:29.676Z
Learning: Vitest is used for all testing across all repositories in the unraid organization, not Jest. Always use `vi` for mocking utilities, not `jest`.
Applied to files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts
📚 Learning: 2025-03-12T13:48:14.850Z
Learnt from: pujitm
Repo: unraid/api PR: 1211
File: web/composables/gql/gql.ts:17-18
Timestamp: 2025-03-12T13:48:14.850Z
Learning: In the Unraid API project, the duplicate GraphQL query and mutation strings in gql.ts files are intentionally generated by GraphQL CodeGen tool and are necessary for the type system to function properly.
Applied to files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.model.ts
📚 Learning: 2025-04-21T18:44:39.643Z
Learnt from: elibosley
Repo: unraid/api PR: 1352
File: packages/unraid-api-plugin-connect/src/config.persistence.ts:0-0
Timestamp: 2025-04-21T18:44:39.643Z
Learning: When working with file operations in Node.js, prefer using the native Promise-based APIs from the fs/promises module instead of callback-based APIs from fs or manually wrapping callbacks in Promises.
Applied to files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts
📚 Learning: 2025-05-23T21:59:29.632Z
Learnt from: elibosley
Repo: unraid/api PR: 1408
File: web/components/ApiKey/PermissionCounter.vue:6-6
Timestamp: 2025-05-23T21:59:29.632Z
Learning: This codebase uses ESM (ECMAScript Modules) and requires .js extensions in import statements, even when importing from TypeScript files, as the imports refer to the compiled JavaScript output.
Applied to files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts
📚 Learning: 2025-03-13T16:17:55.820Z
Learnt from: elibosley
Repo: unraid/api PR: 1211
File: api/src/unraid-api/graph/connect/connect-settings.service.ts:48-53
Timestamp: 2025-03-13T16:17:55.820Z
Learning: In NestJS services, dynamic imports should be wrapped in try/catch blocks with proper error logging and fallback handling to prevent unhandled rejections from crashing the application.
Applied to files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts
📚 Learning: 2025-03-04T14:55:00.903Z
Learnt from: elibosley
Repo: unraid/api PR: 1151
File: plugin/builder/utils/consts.ts:6-6
Timestamp: 2025-03-04T14:55:00.903Z
Learning: The startingDir constant in plugin/builder/utils/consts.ts is defined using process.cwd(), which can cause issues if directory changes occur after importing this constant. Using __dirname.split('/builder')[0] would make it more reliable by making it relative to the file location rather than the current working directory.
Applied to files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts
📚 Learning: 2025-07-22T13:29:32.175Z
Learnt from: elibosley
Repo: unraid/api PR: 1526
File: api/src/unraid-api/graph/resolvers/ups/ups.resolver.ts:82-85
Timestamp: 2025-07-22T13:29:32.175Z
Learning: In GraphQL subscriptions using the graphql-subscriptions PubSub, the correct method is `asyncIterableIterator()`, not `asyncIterator()`. The `asyncIterableIterator` method returns an `AsyncIterableIterator` that conforms to both `AsyncIterable` and `AsyncIterator` interfaces, which is required for GraphQL subscription resolvers. Using `asyncIterator()` would cause TypeScript type errors as it only returns an `AsyncIterator` that doesn't satisfy the `AsyncIterable` interface requirement.
Applied to files:
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts
📚 Learning: 2024-11-06T20:59:25.809Z
Learnt from: mdatelle
Repo: unraid/api PR: 942
File: api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts:111-113
Timestamp: 2024-11-06T20:59:25.809Z
Learning: In the Unraid API project, error handling for mutations is handled at the service level rather than in the GraphQL resolvers. Specifically, in `api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts`, methods like `removeRoleFromApiKey` rely on service-level error handling.
Applied to files:
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts
📚 Learning: 2025-03-10T18:43:13.016Z
Learnt from: mdatelle
Repo: unraid/api PR: 1219
File: api/src/unraid-api/types/fastify.ts:25-30
Timestamp: 2025-03-10T18:43:13.016Z
Learning: In the Unraid API, cookies should be marked as optional (with `?`) in the FastifyRequest type definition since the cookies may not always be present, which is consistent with the authentication flow throughout the codebase.
Applied to files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu.model.ts
🧬 Code graph analysis (5)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts (2)
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts (1)
Injectable(6-216)api/src/unraid-api/cli/generated/graphql.ts (1)
InfoCpu(855-892)
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts (1)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts (1)
Injectable(12-76)
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts (3)
api/src/core/pubsub.ts (1)
pubsub(12-12)api/src/unraid-api/cli/generated/graphql.ts (1)
Subscription(2046-2058)web/composables/gql/graphql.ts (1)
Subscription(2032-2044)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.model.ts (3)
api/src/unraid-api/graph/resolvers/info/info.model.ts (1)
ObjectType(13-44)api/src/unraid-api/graph/resolvers/metrics/metrics.model.ts (1)
ObjectType(8-21)api/src/unraid-api/cli/generated/graphql.ts (1)
Node(1371-1373)
api/src/unraid-api/graph/resolvers/metrics/metrics.module.ts (1)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.module.ts (1)
Module(6-10)
🪛 GitHub Actions: CI - Main (API)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts
[error] 119-119: AssertionError: expected { id: 'info/cpu', …(19) } to deeply equal { id: 'info/cpu', …(19) }
🔇 Additional comments (15)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.model.ts (2)
42-52: LGTM! CpuPackages implementation is correct.The type correctly implements Node, uses definite assignment assertions, and has consistent camelCase naming. All concerns from previous reviews have been addressed.
116-122: LGTM! New fields properly integrated.The topology and packages fields are correctly typed with definite assignments and appropriate descriptions.
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts (1)
95-112: LGTM! Mock implementation is correct.The cpuTopologyService mock properly implements both required methods (generateTopology and generateTelemetry) with appropriate return values. The
as unknown as CpuTopologyServicecast is acceptable for test mocks.api/src/unraid-api/graph/resolvers/metrics/metrics.module.ts (1)
3-11: LGTM! Proper module-based dependency injection.The refactoring correctly imports CpuModule instead of directly providing CpuService. This follows NestJS best practices for module organization.
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts (3)
14-14: LGTM! Dependency injection properly configured.CpuTopologyService is correctly injected via constructor.
22-41: LGTM! Telemetry gathering and totalPower calculation are correct.The implementation properly:
- Gathers telemetry and topology data
- Filters out negative sentinel values (≥ 0) before summing power
- Rounds to 2 decimal places
- Includes required
idfield for the Node type- Maps arrays with -1 sentinel fallback
43-55: LGTM! Return structure correctly includes new fields.The InfoCpu object now properly includes processors, packages, and topology fields as expected by the GraphQL schema.
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts (5)
1-8: LGTM! Proper service setup with logging.Injectable decorator and Logger initialization follow NestJS best practices.
13-67: LGTM! Topology generation with proper error handling.The implementation correctly:
- Reads CPU topology from /sys/devices/system/cpu
- Parses package IDs and thread siblings
- Expands ranges (e.g., "0-3" → [0,1,2,3])
- Deduplicates and sorts results
- Returns empty array on failure with appropriate warning
72-109: LGTM! Temperature reading with defensive I/O.The method properly reads hwmon sensors, filters for CPU temperature labels (coretemp, k10temp, zenpower), and converts millidegrees to Celsius. Error handling is defensive and uses the logger consistently.
111-191: LGTM! RAPL power measurement implementation is sound.The implementation:
- Correctly samples energy counters over a 100ms interval
- Computes power in watts from energy delta and time delta
- Aggregates domains per package with proper rounding
- Handles file access failures gracefully
193-215: LGTM! Telemetry aggregation correctly combines power and temperature.The method properly combines temperature and power data, uses -1 as sentinel for missing values, and returns a well-typed array.
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts (3)
1-26: LGTM! Dependencies properly injected.Logger and CpuTopologyService are correctly added to the resolver with proper typing.
39-58: LGTM! CPU telemetry publisher correctly implemented.The topic registration properly:
- Calls generateTelemetry to fetch per-package data
- Filters negative values before computing totalPower
- Rounds to 2 decimals
- Builds CpuPackages with required id field
- Uses 5-second polling interval
115-125: LGTM! Subscription handler properly configured.The systemMetricsCpuTelemetry subscription is correctly set up with:
- Proper return type (CpuPackages)
- Correct resolve function
- Required permissions decorator
- Tracked subscription via helper service
There was a problem hiding this comment.
🧩 Analysis chain
Investigate pipeline failure on line 119.
The test expectations look correct (32.5 + 33.0 = 65.5 for totalPower), but the pipeline reports an assertion failure. This could indicate a mismatch between the actual implementation output and the expected structure.
Run the test locally to see the full diff:
🏁 Script executed:
#!/bin/bash
# Run the specific test to see detailed output
cd api
npm test -- cpu.service.spec.ts --reporter=verboseLength of output: 118
I'll examine the test file directly to understand the assertion failure at line 119.
---
🏁 Script executed:
```shell
#!/bin/bash
# Read the test file to see the actual test and what's at line 119
fd -e "cpu.service.spec.ts" -x cat {} | head -200 | tail -100
Length of output: 0
Let me search for the test file more broadly:
---
🏁 Script executed:
```shell
#!/bin/bash
# Find the test file
find . -name "cpu.service.spec.ts" -type f 2>/dev/null
Length of output: 65
Now let me read the test file to examine line 119 and the surrounding context:
---
🏁 Script executed:
```shell
#!/bin/bash
# Read the test file with line numbers, focusing on the relevant area
cat -n ./api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts | sed -n '100,170p'
Length of output: 2747
Let me examine the CpuService implementation to see what it actually returns:
---
🏁 Script executed:
```shell
#!/bin/bash
# Find the CpuService implementation file
find . -name "cpu.service.ts" -type f ! -name "*.spec.ts" 2>/dev/null
Length of output: 60
---
🏁 Script executed:
```shell
#!/bin/bash
# Read the CpuService implementation
cat -n ./api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts
Length of output: 2975
Let me check the CpuPackages type definition:
---
🏁 Script executed:
```shell
#!/bin/bash
# Find and read the cpu model file
find . -name "cpu.model.ts" -type f 2>/dev/null
Length of output: 58
---
🏁 Script executed:
```shell
#!/bin/bash
# Read the cpu model file to see CpuPackages definition
cat -n ./api/src/unraid-api/graph/resolvers/info/cpu/cpu.model.ts
Length of output: 4916
Update test expectation to include id field in packages object.
The assertion fails because CpuPackages extends the Node base class (line 43 of cpu.model.ts), which provides an id field. The service correctly sets id: 'info/cpu/packages' on line 37 of cpu.service.ts, but the test expectation at lines 143-157 doesn't include this field. Add id: 'info/cpu/packages' to the packages object in the expected result to match the actual implementation output.
🤖 Prompt for AI Agents
In api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts around lines
143 to 157, the test expectation for the packages object is missing the id
field; update the expected packages object to include id: 'info/cpu/packages' so
it matches CpuPackages (which extends Node) and the service output that sets id
on line 37 of cpu.service.ts.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts (1)
39-69: CPU telemetry topic logic is sound; consider trimming duplicate debug logThe CPU_TELEMETRY registration correctly:
- uses
generateTelemetry()as source of per-package data,- filters out negative values when summing
totalPower,- builds a CpuPackages object with a stable Node id, and
- publishes on the new CPU_TELEMETRY pubsub channel with
systemMetricsCpuTelemetrypayload key matching the subscription.You can optionally drop one of the two
this.logger.debugcalls logging the same payload to reduce log noise.api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts (1)
193-215: Optional: parallelize temperature and power reads for telemetry
generateTelemetrycurrently does:const temps = await this.getPackageTemps(); const powerData = await this.getPackagePower();These are independent IO-bound operations; running them in parallel will slightly reduce the interval work:
- const temps = await this.getPackageTemps(); - const powerData = await this.getPackagePower(); + const [temps, powerData] = await Promise.all([ + this.getPackageTemps(), + this.getPackagePower(), + ]);Not required for correctness, but a small win given this runs on a timer.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
api/generated-schema.graphql(3 hunks)api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/cpu/cpu.model.ts(2 hunks)api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts(3 hunks)api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts(1 hunks)api/src/unraid-api/graph/resolvers/metrics/metrics.module.ts(1 hunks)api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.ts(3 hunks)api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.ts
- api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts
- api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts
🧰 Additional context used
🧠 Learnings (11)
📚 Learning: 2025-03-12T13:48:14.850Z
Learnt from: pujitm
Repo: unraid/api PR: 1211
File: web/composables/gql/gql.ts:17-18
Timestamp: 2025-03-12T13:48:14.850Z
Learning: In the Unraid API project, the duplicate GraphQL query and mutation strings in gql.ts files are intentionally generated by GraphQL CodeGen tool and are necessary for the type system to function properly.
Applied to files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu.model.tsapi/generated-schema.graphql
📚 Learning: 2025-03-10T18:43:13.016Z
Learnt from: mdatelle
Repo: unraid/api PR: 1219
File: api/src/unraid-api/types/fastify.ts:25-30
Timestamp: 2025-03-10T18:43:13.016Z
Learning: In the Unraid API, cookies should be marked as optional (with `?`) in the FastifyRequest type definition since the cookies may not always be present, which is consistent with the authentication flow throughout the codebase.
Applied to files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu.model.ts
📚 Learning: 2025-01-30T19:38:02.478Z
Learnt from: pujitm
Repo: unraid/api PR: 1075
File: unraid-ui/src/register.ts:15-34
Timestamp: 2025-01-30T19:38:02.478Z
Learning: In the web components registration process for unraid-ui, use a soft-fail approach (logging warnings/errors) instead of throwing errors, to ensure other components can still register successfully even if one component fails.
Applied to files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts
📚 Learning: 2025-04-21T18:44:39.643Z
Learnt from: elibosley
Repo: unraid/api PR: 1352
File: packages/unraid-api-plugin-connect/src/config.persistence.ts:0-0
Timestamp: 2025-04-21T18:44:39.643Z
Learning: When working with file operations in Node.js, prefer using the native Promise-based APIs from the fs/promises module instead of callback-based APIs from fs or manually wrapping callbacks in Promises.
Applied to files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts
📚 Learning: 2025-03-07T19:29:12.540Z
Learnt from: pujitm
Repo: unraid/api PR: 1219
File: api/src/unraid-api/main.ts:18-25
Timestamp: 2025-03-07T19:29:12.540Z
Learning: When handling type mismatches between dependencies in TypeScript (such as between NestJS and Fastify), prefer using `ts-expect-error` with a detailed explanation comment instead of `as any` type assertions. This better documents the reason for the type bypass and makes the issue more visible for future maintenance.
Applied to files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts
📚 Learning: 2025-05-23T21:59:29.632Z
Learnt from: elibosley
Repo: unraid/api PR: 1408
File: web/components/ApiKey/PermissionCounter.vue:6-6
Timestamp: 2025-05-23T21:59:29.632Z
Learning: This codebase uses ESM (ECMAScript Modules) and requires .js extensions in import statements, even when importing from TypeScript files, as the imports refer to the compiled JavaScript output.
Applied to files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.module.ts
📚 Learning: 2025-03-13T16:17:55.820Z
Learnt from: elibosley
Repo: unraid/api PR: 1211
File: api/src/unraid-api/graph/connect/connect-settings.service.ts:48-53
Timestamp: 2025-03-13T16:17:55.820Z
Learning: In NestJS services, dynamic imports should be wrapped in try/catch blocks with proper error logging and fallback handling to prevent unhandled rejections from crashing the application.
Applied to files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts
📚 Learning: 2025-03-04T14:55:00.903Z
Learnt from: elibosley
Repo: unraid/api PR: 1151
File: plugin/builder/utils/consts.ts:6-6
Timestamp: 2025-03-04T14:55:00.903Z
Learning: The startingDir constant in plugin/builder/utils/consts.ts is defined using process.cwd(), which can cause issues if directory changes occur after importing this constant. Using __dirname.split('/builder')[0] would make it more reliable by making it relative to the file location rather than the current working directory.
Applied to files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts
📚 Learning: 2025-09-09T14:34:07.189Z
Learnt from: pujitm
Repo: unraid/api PR: 1582
File: packages/unraid-shared/src/util/processing.ts:69-71
Timestamp: 2025-09-09T14:34:07.189Z
Learning: In TypeScript code, pujitm considers it acceptable to use `any` type for private/internal fields when the type is not exposed through the public API and is masked by properly typed method declarations. The focus is on ensuring the public interface maintains type safety rather than enforcing strict typing on all internal implementation details.
Applied to files:
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts
📚 Learning: 2025-07-22T13:29:32.175Z
Learnt from: elibosley
Repo: unraid/api PR: 1526
File: api/src/unraid-api/graph/resolvers/ups/ups.resolver.ts:82-85
Timestamp: 2025-07-22T13:29:32.175Z
Learning: In GraphQL subscriptions using the graphql-subscriptions PubSub, the correct method is `asyncIterableIterator()`, not `asyncIterator()`. The `asyncIterableIterator` method returns an `AsyncIterableIterator` that conforms to both `AsyncIterable` and `AsyncIterator` interfaces, which is required for GraphQL subscription resolvers. Using `asyncIterator()` would cause TypeScript type errors as it only returns an `AsyncIterator` that doesn't satisfy the `AsyncIterable` interface requirement.
Applied to files:
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts
📚 Learning: 2024-11-06T20:59:25.809Z
Learnt from: mdatelle
Repo: unraid/api PR: 942
File: api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts:111-113
Timestamp: 2024-11-06T20:59:25.809Z
Learning: In the Unraid API project, error handling for mutations is handled at the service level rather than in the GraphQL resolvers. Specifically, in `api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts`, methods like `removeRoleFromApiKey` rely on service-level error handling.
Applied to files:
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts
🧬 Code graph analysis (4)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.model.ts (1)
api/src/unraid-api/cli/generated/graphql.ts (1)
Node(1371-1373)
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts (1)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts (1)
Injectable(12-76)
api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts (3)
api/src/core/pubsub.ts (1)
pubsub(12-12)api/src/unraid-api/cli/generated/graphql.ts (1)
Subscription(2046-2058)web/composables/gql/graphql.ts (1)
Subscription(2032-2044)
api/src/unraid-api/graph/resolvers/metrics/metrics.module.ts (3)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.module.ts (1)
Module(6-10)api/src/unraid-api/graph/resolvers/info/info.module.ts (1)
Module(17-37)api/src/unraid-api/graph/services/services.module.ts (1)
Module(8-13)
🔇 Additional comments (7)
api/src/unraid-api/graph/resolvers/metrics/metrics.module.ts (1)
3-10: MetricsModule wiring with CpuModule looks correctImporting
CpuModuleand droppingCpuServicefromproviderscleanly delegates CPU-related providers (includingCpuTopologyService) toCpuModule, matching howInfoModuleis wired and avoiding duplicate provider declarations. No issues here.api/src/unraid-api/graph/resolvers/info/cpu/cpu.model.ts (2)
42-52: CpuPackages Node type aligns with resolvers and schema
CpuPackagescorrectly implementsNode, uses non-nullFloat/[Float!]!fields, and matches how CpuService/MetricsResolver construct the payloads (including the requiredid). This keeps the schema and TS model in sync.
116-122: Non-nulltopologyandpackageson InfoCpu are consistent with CpuServiceMarking
topologyandpackagesas non-null matches the CpuService implementation, which always returns an array (possibly empty) and a fully-populatedCpuPackages(with sentinel-1for missing readings). This avoids non-null GraphQL field failures while giving clients predictable shapes.api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts (2)
1-26: DI updates (logger + CpuTopologyService) are wired correctlyImplementing
OnModuleInit, injectingCpuTopologyService, and adding a scopedLoggerfollow existing patterns in this project. Combined withCpuModuleimport inMetricsModule, CpuService and CpuTopologyService will resolve correctly.
115-125: NewsystemMetricsCpuTelemetrysubscription matches pubsub topic and payloadThe subscription decorator, payload key (
systemMetricsCpuTelemetry), permissions, and tracked subscription onPUBSUB_CHANNEL.CPU_TELEMETRYare consistent with the registration inonModuleInitand with the CpuPackages GraphQL type.api/generated-schema.graphql (2)
1394-1405: CpuPackages schema matches code-first model and Node contract
CpuPackages implements Nodewithid: PrefixedID!and non-nullFloat/[Float!]!fields, which aligns with theCpuPackagesclass incpu.model.tsand how CpuService/MetricsResolver populate these values.
1471-1475: Client typings must be regenerated before mergeThe new
systemMetricsCpuTelemetrysubscription field is missing from all generated GraphQL client typings (checked in api/src/unraid-api/cli/generated/graphql.ts, packages/unraid-api-plugin-connect/src/graphql/generated/client/graphql.ts, web/src/composables/gql/graphql.ts, and related files). Regenerate client typings to include the newCpuPackagestype andsystemMetricsCpuTelemetrysubscription field before merging.
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
🤖 I have created a release *beep* *boop* --- ## [4.26.0](v4.25.3...v4.26.0) (2025-11-17) ### Features * add cpu power query & subscription ([#1745](#1745)) ([d7aca81](d7aca81)) * add schema publishing to apollo studio ([#1772](#1772)) ([7e13202](7e13202)) * add workflow_dispatch trigger to schema publishing workflow ([818e7ce](818e7ce)) * apollo studio readme link ([c4cd0c6](c4cd0c6)) * **cli:** make `unraid-api plugins remove` scriptable ([#1774](#1774)) ([64eb9ce](64eb9ce)) * use persisted theme css to fix flashes on header ([#1784](#1784)) ([854b403](854b403)) ### Bug Fixes * **api:** decode html entities before parsing notifications ([#1768](#1768)) ([42406e7](42406e7)) * **connect:** disable api plugin if unraid plugin is absent ([#1773](#1773)) ([c264a18](c264a18)) * detection of flash backup activation state ([#1769](#1769)) ([d18eaf2](d18eaf2)) * re-add missing header gradient styles ([#1787](#1787)) ([f8a6785](f8a6785)) * respect OS safe mode in plugin loader ([#1775](#1775)) ([92af3b6](92af3b6)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary by CodeRabbit
New Features
Chores