[testing, don't merge] feat: add cpu power query & subscription #1763
Conversation
WalkthroughAdds CPU package topology discovery and per-package telemetry. Introduces CpuTopologyService that reads sysfs (hwmon, RAPL) for temps and power, integrates it into CpuService and MetricsResolver, adds CpuPackages GraphQL type and subscription, registers new CpuModule, and adds a deployment script for the shared package. Changes
Sequence Diagram(s)sequenceDiagram
participant MetricsResolver
participant PubSub
participant CpuTopologyService
participant Sysfs as Sysfs (hwmon / powercap)
participant Subscriber
rect rgba(200,230,201,0.6)
MetricsResolver->>MetricsResolver: Init CPU_TELEMETRY polling (5s)
end
loop Every 5s
MetricsResolver->>CpuTopologyService: generateTelemetry()
CpuTopologyService->>Sysfs: Read RAPL energy samples (/sys/class/powercap)
Sysfs-->>CpuTopologyService: energy_uj samples
CpuTopologyService->>Sysfs: Read hwmon temps (/sys/class/hwmon)
Sysfs-->>CpuTopologyService: temp labels/values
CpuTopologyService-->>MetricsResolver: per-package {id,power,temp}[]
MetricsResolver->>MetricsResolver: build CpuPackages (totalPower,power[],temp[])
MetricsResolver->>PubSub: publish CPU_TELEMETRY payload
end
Subscriber->>PubSub: Subscribe systemMetricsCpuTelemetry
PubSub-->>Subscriber: Deliver CpuPackages
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| @@ -1,13 +1,15 @@ | |||
| import { Module } from '@nestjs/common'; | |||
|
|
|||
| import { CpuTopologyService } from '@app/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.js'; | |||
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 days ago
To fix this issue, simply remove the unused import statement for CpuTopologyService in api/src/unraid-api/graph/resolvers/metrics/metrics.module.ts. No reference to this import exists in the code, so its removal will not affect the functionality or structure of the module. Remove only the import on line 3, leaving other lines unchanged.
| @@ -1,6 +1,5 @@ | ||
| 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 { 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'; |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1763 +/- ##
==========================================
- Coverage 52.69% 52.68% -0.01%
==========================================
Files 865 867 +2
Lines 49343 49582 +239
Branches 4952 4982 +30
==========================================
+ Hits 26000 26121 +121
- Misses 23270 23388 +118
Partials 73 73 ☔ 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: 11
🧹 Nitpick comments (8)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.model.ts (1)
121-122: Reconsider required packages field.The
packagesfield is marked as required (!), but telemetry collection might fail in some environments. Consider making this field nullable to handle cases where package power/temperature data is unavailable.Apply this diff if telemetry can be unavailable:
- @Field(() => CpuPackages) + @Field(() => CpuPackages, { nullable: true }) packages!: CpuPackages;api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.ts (1)
22-22: Consider providing a proper mock for CpuTopologyService.While adding
CpuTopologyServiceto the providers is necessary, consider providing a proper mock implementation rather than relying on the empty object at line 168. This will make the test more robust and easier to maintain.For example:
{ provide: CpuTopologyService, useValue: { generateTelemetry: vi.fn().mockResolvedValue({ totalPower: 100, power: [50, 50], temp: [45, 45], }), }, },api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.ts (3)
8-8: WiringCpuTopologyServicein the test module is appropriate.Needed to satisfy
CpuServiceconstructor DI in this integration setup. Optionally, importCpuModuleinstead of listing CPU providers to mirror production wiring.Also applies to: 26-26
124-141: Avoidas anyin test spy return; type the payload.Model the return value to match
MemoryService.generateMemoryLoad()and keep types intact.- vi.spyOn(memoryService, 'generateMemoryLoad').mockImplementation(async () => { + vi.spyOn(memoryService, 'generateMemoryLoad').mockImplementation(async () => { executionCount++; await new Promise((resolve) => setTimeout(resolve, 50)); // Simulate slow operation - return { + const payload: Awaited<ReturnType<MemoryService['generateMemoryLoad']>> = { id: 'memory-utilization', total: 16000000000, used: 8000000000, free: 8000000000, available: 8000000000, active: 4000000000, buffcache: 2000000000, percentTotal: 50, swapTotal: 0, swapUsed: 0, swapFree: 0, percentSwapTotal: 0, - } as any; + }; + return payload; });
112-117: Deflake time-based polling tests with fake timers or injectable intervals.Real
setTimeout(50–2100ms) slows CI and can flake. Usevi.useFakeTimers()and advance timers, or make poll intervals injectable and set to 10–20ms in tests.Example:
vi.useFakeTimers(); trackerService.subscribe(PUBSUB_CHANNEL.CPU_UTILIZATION); await vi.advanceTimersByTimeAsync(1000); // or injected short intervalAlso applies to: 160-162, 185-187
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts (2)
22-25: Run topology and telemetry in parallel to cut latency.- const packageList = await this.cpuTopologyService.generateTelemetry(); - const topology = await this.cpuTopologyService.generateTopology(); + const [packageList, topology] = await Promise.all([ + this.cpuTopologyService.generateTelemetry(), + this.cpuTopologyService.generateTopology(), + ]);
19-20: Filter empty flag tokens.
split(' ')on empty string yields['']. Filter falsy tokens.- const flags = await cpuFlags() - .then((f) => f.split(' ')) + const flags = await cpuFlags() + .then((f) => f.split(' ').filter(Boolean)) .catch(() => []);api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts (1)
35-44: Consider extracting range expansion logic to a helper function.The range expansion logic is complex and could benefit from extraction for clarity and testability.
Example refactor:
private expandCpuRange(rangeStr: string): number[] { return rangeStr .trim() .replace(/(\d+)-(\d+)/g, (_, start, end) => Array.from( { length: parseInt(end) - parseInt(start) + 1 }, (_, i) => parseInt(start) + i ).join(',') ) .split(',') .map((n) => parseInt(n, 10)); }Then use it in
generateTopology:const siblings = this.expandCpuRange(siblingsStrRaw);
📜 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)
🧰 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.resolver.integration.spec.tsapi/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/info/cpu/cpu.module.tsapi/src/unraid-api/graph/resolvers/info/info.module.tsapi/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/metrics/metrics.resolver.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.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/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.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.model.tspackages/unraid-shared/src/pubsub/graphql.pubsub.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.module.tsapi/src/unraid-api/graph/resolvers/info/info.module.tsapi/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/metrics/metrics.resolver.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.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/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.resolver.integration.spec.tsapi/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/info/cpu/cpu.module.tsapi/src/unraid-api/graph/resolvers/info/info.module.tsapi/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/metrics/metrics.resolver.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.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/info.resolver.integration.spec.tsapi/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts
🧬 Code graph analysis (6)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.model.ts (1)
api/src/unraid-api/graph/resolvers/info/info.model.ts (1)
ObjectType(13-44)
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-topology.service.ts (1)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts (1)
Injectable(12-70)
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)
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 (1)
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts (1)
Injectable(6-211)
🪛 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 (17)
packages/unraid-shared/package.json (1)
24-25: LGTM!The trailing comma addition and new deployment script are clean additions. The script follows npm conventions and properly references the deployment script.
packages/unraid-shared/deploy.sh (1)
53-53: Verify debug settings are appropriate.The restart command uses
INTROSPECTION=trueandLOG_LEVEL=trace, which may produce excessive logging in production environments. Consider whether these debug settings should be configurable or removed for production deployments.api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.ts (1)
176-176: Update expectation correctly reflects new topic.The change from 2 to 3 registered topics correctly reflects the addition of the CPU_TELEMETRY channel. This is consistent with the changes in the codebase.
api/src/unraid-api/graph/resolvers/info/info.module.ts (1)
18-18: LGTM: Clean module composition.Adding
CpuModuleto the imports properly encapsulates CPU-related services and follows NestJS best practices for module organization.packages/unraid-shared/src/pubsub/graphql.pubsub.ts (1)
8-8: LGTM!The addition of
CPU_TELEMETRYchannel is clean and follows the existing naming convention. The placement maintains alphabetical ordering within the enum.api/src/unraid-api/graph/resolvers/info/info.resolver.integration.spec.ts (1)
9-9: LGTM!The import and registration of
CpuTopologyServiceproperly reflects the new dependency structure. This ensures the integration tests accurately exercise the full CPU service stack including topology functionality.api/src/unraid-api/graph/resolvers/metrics/metrics.module.ts (1)
11-13: Module wiring LGTM.Importing
CpuModuleand dropping directCpuServiceprovider is correct DI usage.api/src/unraid-api/graph/resolvers/info/cpu/cpu.module.ts (1)
6-10: LGTM.Clean module encapsulation; exports enable downstream injection. ESM
.jsspecifiers correct.api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts (1)
14-15: DI addition LGTM.Constructor injection of
CpuTopologyServiceis clean and testable.api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts (1)
143-157: Expectation updates LGTM.Validates
packagesandtopologystructure precisely.api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts (3)
1-26: LGTM!Logger instance and dependency injection are properly configured. The imports follow ESM guidelines with
.jsextensions as required.
39-53: Good defensive coding and consistent patterns.The telemetry polling implementation follows the established pattern from other subscriptions, with appropriate defensive coding (
?? [],-1for missing values) and proper rounding for power values.
110-120: LGTM!The subscription follows the same pattern as existing subscriptions with proper permissions and resolve configuration.
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts (4)
146-146: Verify power measurement timing is appropriate for subscription interval.The 100ms delay for power measurement means each telemetry call blocks for at least 100ms. With a 5-second subscription interval (from metrics.resolver.ts line 63), this is acceptable. However, if the interval were reduced or this method called more frequently, it could become a bottleneck.
1-9: LGTM!Imports follow ESM guidelines with
.jsextensions, and the service is properly decorated with@Injectable().
67-104: Good defensive temperature parsing.The method handles multiple hwmon drivers (coretemp, k10temp, zenpower) and gracefully handles missing files with appropriate error logging.
188-210: LGTM!The telemetry aggregation logic correctly combines temperature and power data, with consistent use of
-1for missing values.
| type CpuPackages implements Node { | ||
| """Total CPU package power draw (W)""" | ||
| totalPower: Float! | ||
|
|
||
| """Power draw per package (W)""" | ||
| power: [Float!] | ||
|
|
||
| """description: 'Temperature per package (°C)""" | ||
| temp: [Float!] | ||
| } |
There was a problem hiding this comment.
Fix malformed description and verify Node implementation.
Line 1411 has a malformed description: """description: 'Temperature per package (°C)""" should be """Temperature per package (°C)""".
Additionally, CpuPackages implements Node (line 1404), which requires an id: PrefixedID! field, but this field is not present in the TypeScript model (cpu.model.ts lines 42-52). This will cause runtime errors.
To fix the TypeScript model, add the id field:
@ObjectType({ description: 'CPU package telemetry data', implements: () => Node })
export class CpuPackages extends Node {
@Field(() => Float, { description: 'Total CPU package power draw (W)', nullable: true })
totalPower?: number;
@Field(() => [Float], { description: 'Power draw per package (W)', nullable: true })
power?: number[];
@Field(() => [Float], { description: 'Temperature per package (°C)', nullable: true })
temp?: number[];
}Or if CpuPackages shouldn't implement Node, remove it from the GraphQL ObjectType decorator in the TypeScript file and regenerate the schema.
🤖 Prompt for AI Agents
In api/generated-schema.graphql around lines 1404 to 1413, the GraphQL
description for temp is malformed and the type declares CpuPackages implements
Node but the corresponding TypeScript model (cpu.model.ts lines 42-52) lacks the
required id: PrefixedID! field; fix by correcting the triple-quoted description
to """Temperature per package (°C)""" and then either add the missing id field
to the CpuPackages TypeScript class (extend/implement Node and declare id:
PrefixedID with appropriate @Field() decorator) or remove the Node
implementation from the TypeScript @ObjectType decorator and regenerate the
schema so the GraphQL and TypeScript models stay consistent.
| export class CpuTopologyService { | ||
| private readonly logger = new Logger(CpuTopologyService.name); | ||
|
|
||
| private topologyCache: { id: number; cores: number[][] }[] | null = null; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove unused topologyCache field.
The topologyCache field is declared but never read or written. Either implement caching or remove this dead code.
Apply this diff if caching is not needed:
private readonly logger = new Logger(CpuTopologyService.name);
-
- private topologyCache: { id: number; cores: number[][] }[] | null = null;📝 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.
| private topologyCache: { id: number; cores: number[][] }[] | null = null; | |
| private readonly logger = new Logger(CpuTopologyService.name); |
🤖 Prompt for AI Agents
In api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts around
line 10, remove the unused topologyCache field declaration (private
topologyCache: { id: number; cores: number[][] }[] | null = null;) since it is
never read or written; simply delete that line and run the TypeScript
compiler/linter to ensure no references remain and commit the change.
| packages[pkgId].push(siblings); | ||
| } | ||
| } catch (err) { | ||
| console.warn('Topology read error for', dir, err); |
There was a problem hiding this comment.
Use logger instance instead of console.warn.
Line 51 uses console.warn while lines 90, 97, 101, and 170 correctly use this.logger.warn. This inconsistency should be resolved.
Apply this diff:
} catch (err) {
- console.warn('Topology read error for', dir, err);
+ this.logger.warn('Topology read error for', dir, err);
}📝 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.
| console.warn('Topology read error for', dir, err); | |
| } catch (err) { | |
| this.logger.warn('Topology read error for', dir, err); | |
| } |
🤖 Prompt for AI Agents
In api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts around
line 51, replace the console.warn call with this.logger.warn to match the other
warning usages; ensure the method uses the class logger (this.logger.warn) and
pass the same message and variables (dir and err) to the logger so it stays
consistent with lines 90, 97, 101, and 170.
|
|
||
| for (const domains of Object.values(results)) { | ||
| const total = Object.values(domains).reduce((a, b) => a + b, 0); | ||
| (domains as any)['total'] = Math.round(total * 100) / 100; |
There was a problem hiding this comment.
Avoid type casting; use proper typing instead.
The (domains as any)['total'] cast violates the coding guideline to never use the any type and to avoid type casting.
As per coding guidelines.
Define a proper type for the domains object:
type DomainPower = Record<string, number>;Then update the function signature and usage:
- private async getPackagePower(): Promise<Record<number, Record<string, number>>> {
+ private async getPackagePower(): Promise<Record<number, DomainPower>> {
// ... existing code ...
for (const domains of Object.values(results)) {
const total = Object.values(domains).reduce((a, b) => a + b, 0);
- (domains as any)['total'] = Math.round(total * 100) / 100;
+ domains['total'] = Math.round(total * 100) / 100;
}🤖 Prompt for AI Agents
In api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts around
line 182, avoid the (domains as any)['total'] cast: define a proper type (e.g.
type DomainPower = Record<string, number>) and use it for the domains
variable/function signature so domains is typed as DomainPower; then assign
domains['total'] = Math.round(total * 100) / 100 without any casting and update
any callers to pass/expect DomainPower.
| @ObjectType() | ||
| export class CpuPackages { | ||
| @Field(() => Float, { description: 'Total CPU package power draw (W)' }) | ||
| totalPower?: number; | ||
|
|
||
| @Field(() => [Float], { description: 'Power draw per package (W)' }) | ||
| power?: number[]; | ||
|
|
||
| @Field(() => [Float], { description: 'Temperature per package (°C)' }) | ||
| temp?: number[]; | ||
| } |
There was a problem hiding this comment.
Add description to ObjectType and verify nullability.
The CpuPackages type is missing a description in the @ObjectType decorator. Additionally, verify the nullability semantics: the fields are optional (?) in TypeScript but appear as required in parts of the generated GraphQL schema (line 1404-1413 in generated-schema.graphql), creating an inconsistency.
Apply this diff to add a description:
-@ObjectType()
+@ObjectType({ description: 'CPU package telemetry data' })
export class CpuPackages {Additionally, confirm whether these fields should be optional or required. If telemetry collection can fail, consider keeping them optional and ensuring the GraphQL schema reflects this.
📝 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.
| @ObjectType() | |
| export class CpuPackages { | |
| @Field(() => Float, { description: 'Total CPU package power draw (W)' }) | |
| totalPower?: number; | |
| @Field(() => [Float], { description: 'Power draw per package (W)' }) | |
| power?: number[]; | |
| @Field(() => [Float], { description: 'Temperature per package (°C)' }) | |
| temp?: number[]; | |
| } | |
| @ObjectType({ description: 'CPU package telemetry data' }) | |
| export class CpuPackages { | |
| @Field(() => Float, { description: 'Total CPU package power draw (W)' }) | |
| totalPower?: number; | |
| @Field(() => [Float], { description: 'Power draw per package (W)' }) | |
| power?: number[]; | |
| @Field(() => [Float], { description: 'Temperature per package (°C)' }) | |
| temp?: number[]; | |
| } |
🤖 Prompt for AI Agents
In api/src/unraid-api/graph/resolvers/info/cpu/cpu.model.ts around lines 42 to
52, add a description to the @ObjectType decorator (e.g., "Per-package CPU
telemetry: power and temperature") and make the GraphQL nullability match the
TypeScript optional fields by updating each @Field to include nullable: true
(keeping the TypeScript ? on totalPower, power, temp). This ensures the
generated schema shows these fields as optional when telemetry can fail.
| // Compute total power (2 decimals) | ||
| const totalPower = | ||
| Math.round(packageList.reduce((sum, pkg) => sum + (pkg.power ?? 0), 0) * 100) / 100; | ||
|
|
There was a problem hiding this comment.
Fix totalPower when a package has unknown power (-1).
pkg.power ?? 0 includes -1 in the sum, producing incorrect totals. Treat negatives/unknown as 0.
- const totalPower =
- Math.round(packageList.reduce((sum, pkg) => sum + (pkg.power ?? 0), 0) * 100) / 100;
+ const totalPower =
+ Math.round(
+ packageList.reduce(
+ (sum, pkg) => sum + ((pkg.power != null && pkg.power >= 0) ? pkg.power : 0),
+ 0
+ ) * 100
+ ) / 100;📝 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.
| // Compute total power (2 decimals) | |
| const totalPower = | |
| Math.round(packageList.reduce((sum, pkg) => sum + (pkg.power ?? 0), 0) * 100) / 100; | |
| // Compute total power (2 decimals) | |
| const totalPower = | |
| Math.round( | |
| packageList.reduce( | |
| (sum, pkg) => sum + ((pkg.power != null && pkg.power >= 0) ? pkg.power : 0), | |
| 0 | |
| ) * 100 | |
| ) / 100; |
🤖 Prompt for AI Agents
In api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts around lines 26 to
29, the totalPower calculation currently treats pkg.power of -1 as a valid
value; change the reducer to treat any negative/unknown power as 0 by using
Math.max(0, Number(pkg.power ?? 0)) (or equivalent) when summing, then apply the
same rounding to two decimals as before so negative sentinel values are excluded
from the total.
| 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'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove unused CpuService import.
With the introduction of CpuModule (line 4), the direct import of CpuService (line 5) is no longer needed since CpuModule now provides CpuService. This import should be removed to avoid confusion.
Apply this diff:
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 { DevicesResolver } from '@app/unraid-api/graph/resolvers/info/devices/devices.resolver.js';📝 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.
| 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 { CpuModule } from '@app/unraid-api/graph/resolvers/info/cpu/cpu.module.js'; | |
| import { DevicesResolver } from '@app/unraid-api/graph/resolvers/info/devices/devices.resolver.js'; |
🤖 Prompt for AI Agents
In api/src/unraid-api/graph/resolvers/info/info.module.ts around lines 4 to 5,
remove the unused import of CpuService (the second import line) because
CpuModule now provides CpuService; delete that import statement and run the
linter/build to ensure no remaining references to CpuService exist in this 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'; | ||
| import { MemoryService } from '@app/unraid-api/graph/resolvers/info/memory/memory.service.js'; |
There was a problem hiding this comment.
Remove unused imports to satisfy CodeQL and keep module clean.
CpuTopologyService and CpuService are no longer referenced here; drop both imports. This also aligns with moving CPU providers behind CpuModule.
Based on static analysis hints.
-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';
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';📝 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.
| 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'; | |
| import { MemoryService } from '@app/unraid-api/graph/resolvers/info/memory/memory.service.js'; | |
| import { CpuModule } from '@app/unraid-api/graph/resolvers/info/cpu/cpu.module.js'; | |
| import { MemoryService } from '@app/unraid-api/graph/resolvers/info/memory/memory.service.js'; |
🧰 Tools
🪛 GitHub Check: CodeQL
[notice] 3-3: Unused variable, import, function or class
Unused import CpuTopologyService.
🤖 Prompt for AI Agents
In api/src/unraid-api/graph/resolvers/metrics/metrics.module.ts around lines 3
to 6, remove the two unused imports CpuTopologyService and CpuService which are
no longer referenced; keep the remaining imports (CpuModule and MemoryService)
intact to satisfy CodeQL and align CPU providers behind CpuModule.
| 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)}`); |
There was a problem hiding this comment.
Remove duplicate debug log.
Line 61 logs the identical payload already logged at line 54. This creates unnecessary log noise.
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)}`);
},📝 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.
| 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)}`); | |
| this.logger.debug(`CPU_TELEMETRY payload: ${JSON.stringify(packages)}`); | |
| // Publish the payload | |
| pubsub.publish(PUBSUB_CHANNEL.CPU_TELEMETRY, { | |
| systemMetricsCpuTelemetry: packages, | |
| }); | |
| }, |
🤖 Prompt for AI Agents
In api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts around lines
54 to 61, there is a duplicate debug log of the same CPU_TELEMETRY payload
(logged at line 54 and again at line 61); remove the second
this.logger.debug(...) call (the one at line 61) so the payload is only logged
once when publishing, keeping the first debug before pubsub.publish and leaving
the rest of the method unchanged.
| 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" |
There was a problem hiding this comment.
Avoid eval with user input.
Building the rsync command as a string and executing it with eval introduces a command injection risk. Even though server_name is intended to be a trusted SSH hostname, this pattern is fragile.
Apply this diff to execute rsync directly:
-# 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 and capture the exit code
+echo "Syncing $source_directory/ to root@${server_name}:$destination_directory/"
+rsync -avz --delete --progress --stats -e ssh "$source_directory/" "root@${server_name}:$destination_directory/"
+exit_code=$?Committable suggestion skipped: line range outside the PR's diff.
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>
elibosley
force-pushed
the
feat/cpu-power2
branch
from
1f15167 to
2db3bf5
Compare
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (6)
api/src/unraid-api/graph/resolvers/metrics/metrics.module.ts (1)
3-6: Drop unused CPU service/topology imports; rely on CpuModule.These imports are unused here. Keep CpuModule in imports; remove direct imports to satisfy CodeQL and keep module clean.
Apply this diff:
-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/info/cpu/cpu.service.spec.ts (1)
94-113: Replaceas anymock; type it and includeidin telemetry.Avoid
anyand match the realCpuTopologyServicesignature to prevent type drift.Apply:
- let cpuTopologyService: CpuTopologyService; + let cpuTopologyService: CpuTopologyService; beforeEach(() => { - cpuTopologyService = { - generateTopology: vi.fn().mockResolvedValue([ + const cpuTopologyMock: Pick<CpuTopologyService, 'generateTopology' | 'generateTelemetry'> = { + generateTopology: vi.fn().mockResolvedValue([ [ [0, 1], [2, 3], ], [ [4, 5], [6, 7], ], ]), - generateTelemetry: vi.fn().mockResolvedValue([ - { power: 32.5, temp: 45.0 }, - { power: 33.0, temp: 46.0 }, - ]), - } as any; - - service = new CpuService(cpuTopologyService); + generateTelemetry: vi.fn().mockResolvedValue([ + { id: 0, power: 32.5, temp: 45.0 }, + { id: 1, power: 33.0, temp: 46.0 }, + ]), + }; + + cpuTopologyService = cpuTopologyMock as unknown as CpuTopologyService; + service = new CpuService(cpuTopologyService);api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts (1)
26-29: Correct totalPower: exclude unknown (-1) readings.Current reducer includes -1, producing under-reported totals.
Apply:
- const totalPower = - Math.round(packageList.reduce((sum, pkg) => sum + (pkg.power ?? 0), 0) * 100) / 100; + const totalPower = + Math.round( + packageList.reduce( + (sum, pkg) => sum + ((pkg.power != null && pkg.power >= 0) ? pkg.power : 0), + 0 + ) * 100 + ) / 100;api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts (3)
10-10: Remove unused topologyCache field.The
topologyCachefield is declared but never read or written throughout the service.Apply this diff:
private readonly logger = new Logger(CpuTopologyService.name); - - private topologyCache: { id: number; cores: number[][] }[] | null = null;
51-51: Use logger instance instead of console.warn.Line 51 uses
console.warnwhile lines 90, 97, 101, and 170 correctly usethis.logger.warn.Apply this diff:
} catch (err) { - console.warn('Topology read error for', dir, err); + this.logger.warn('Topology read error for', dir, err); }
182-182: Avoid type casting; use proper typing instead.The
(domains as any)['total']cast violates the coding guideline to avoid type casting and never use theanytype.As per coding guidelines, define a proper type for the domains object:
type DomainPower = Record<string, number>;Then update the function signature:
- private async getPackagePower(): Promise<Record<number, Record<string, number>>> { + private async getPackagePower(): Promise<Record<number, DomainPower>> { // ... existing code ... for (const domains of Object.values(results)) { const total = Object.values(domains).reduce((a, b) => a + b, 0); - (domains as any)['total'] = Math.round(total * 100) / 100; + domains['total'] = Math.round(total * 100) / 100; }
🧹 Nitpick comments (6)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts (1)
162-178: Avoidas anyin systeminformation mock; use Partial typing.Keep tests type-safe without
any.Apply:
- vi.mocked(cpu).mockResolvedValueOnce({ + vi.mocked(cpu).mockResolvedValueOnce({ manufacturer: 'Intel', brand: 'Core i7-9700K', vendor: 'Intel', family: '6', model: '158', stepping: '12', revision: '', voltage: '1.2V', speed: 3.6, cores: 16, physicalCores: 8, processors: 1, socket: 'LGA1151', cache: { l1d: 32768, l1i: 32768, l2: 262144, l3: 12582912 }, - } as any); + } as Partial<import('systeminformation').Systeminformation.CpuData> as unknown as import('systeminformation').Systeminformation.CpuData);api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts (3)
22-25: Harden topology/telemetry collection with graceful fallbacks.A top‑level fs failure in CpuTopologyService (e.g., initial readdir) will reject and break the resolver. Catch and default.
Apply:
- // Gather telemetry - const packageList = await this.cpuTopologyService.generateTelemetry(); - const topology = await this.cpuTopologyService.generateTopology(); + // Gather telemetry with graceful fallbacks + const [packageList, topology] = await Promise.all([ + this.cpuTopologyService.generateTelemetry().catch(() => [] as { power: number; temp: number }[]), + this.cpuTopologyService.generateTopology().catch(() => [] as number[][][]), + ]);Optionally set
packages.totalPowerto-1whenpackageList.length === 0.
19-20: Make flags parsing robust to extra whitespace.Trim and split by whitespace; drop empties.
Apply:
- const flags = await cpuFlags() - .then((f) => f.split(' ')) + const flags = await cpuFlags() + .then((f) => f.trim().split(/\s+/).filter(Boolean)) .catch(() => []);
44-45: Optional: safer stepping parse.Avoid NaN on empty strings; default to -1.
Apply:
- stepping: Number(stepping), + stepping: Number.isFinite(Number(stepping)) && String(stepping).trim() !== '' ? Number(stepping) : -1,api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts (2)
67-104: Consider more specific error messages for observability.Lines 90, 97, and 101 all log "Failed to read file" without indicating which operation or path failed, making debugging more difficult.
Consider including context in the error messages:
// Example for line 90 this.logger.warn(`Failed to read temp input file ${inputFile}`, err); // Example for line 97 this.logger.warn(`Failed to read hwmon path ${path}`, err); // Example for line 101 this.logger.warn('Failed to read /sys/class/hwmon directory', err);
146-146: Consider extracting the sampling interval as a named constant.The 100ms delay is a magic number. Extracting it would improve readability and make it easier to adjust if needed.
private static readonly POWER_SAMPLE_INTERVAL_MS = 100; // Then in the method: await new Promise((res) => setTimeout(res, CpuTopologyService.POWER_SAMPLE_INTERVAL_MS));
📜 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 (9)
- api/src/unraid-api/graph/resolvers/info/info.module.ts
- api/src/unraid-api/graph/resolvers/info/info.resolver.integration.spec.ts
- api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.ts
- api/generated-schema.graphql
- api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.ts
- packages/unraid-shared/deploy.sh
- packages/unraid-shared/package.json
- api/src/unraid-api/graph/resolvers/info/cpu/cpu.model.ts
- api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.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.module.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/info/cpu/cpu-topology.service.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.module.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.module.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/info/cpu/cpu-topology.service.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.module.tspackages/unraid-shared/src/pubsub/graphql.pubsub.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.module.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/info/cpu/cpu-topology.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
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
🧠 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.ts,**/__test__/{components,store}/**/*.ts} : Use factory functions in vi.mock calls to define mocks and avoid hoisting pitfalls
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/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.service.ts (2)
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)
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)
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 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 (3)
packages/unraid-shared/src/pubsub/graphql.pubsub.ts (1)
8-8: New CPU_TELEMETRY channel — looks good. Please verify wiring.Enum addition is fine. Ensure all publishers/subscribers use this constant and any filters/auth align with other channels.
api/src/unraid-api/graph/resolvers/info/cpu/cpu.module.ts (1)
6-10: CpuModule wiring — LGTM.Providers/exports are correct, ESM import specifiers use .js as required.
api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts (1)
188-210: LGTM!The telemetry aggregation logic correctly combines temperature and power data, handles missing values appropriately with -1 sentinels, and ensures all packages are represented.
Summary by CodeRabbit