Skip to content

Commit af74a6c

Browse files
committed
fix(capacity): treat NaN as unknown in capacity comparator (refs #82)
Number.isFinite() replaces the typeof === 'number' guard in extractCapacity so that NaN values (which pass typeof but cause NaN subtraction in compareNullable) are folded into the unknown/null bucket and sort last alongside absent/zero/null fields. The previous code would produce NaN from the subtraction path, violating the Array.prototype.sort contract and causing unstable ordering whenever a details blob contained a NaN vcpu or memory_gb. Adds three discriminating tests via a recRaw() helper that bypasses the existing rec() guard to inject NaN directly into the details blob: NaN vcpu sorts after known vcpu, NaN memory_gb sorts after known memory_gb when vcpu ties, and two all-NaN recs fall back to the resource_type tiebreaker without any NaN-comparison instability. compareByCapacity is left as a utility without wiring to the recommendations table sort: adding a vcpu/memory column requires a UX decision (new RecommendationsColumnId, rendered column header, default direction). See the extractCapacity docstring and issue #82.
1 parent b65f4c9 commit af74a6c

2 files changed

Lines changed: 52 additions & 6 deletions

File tree

‎frontend/src/__tests__/capacity.test.ts‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,15 @@ function recNoDetails(resourceType: string): { details: undefined; resource_type
3232
return { details: undefined, resource_type: resourceType };
3333
}
3434

35+
/** Builds a rec with raw numeric values including NaN/Infinity (bypasses rec() guards). */
36+
function recRaw(
37+
resourceType: string,
38+
vcpu: unknown,
39+
memoryGb: unknown
40+
): { details: unknown; resource_type: string } {
41+
return { details: { vcpu, memory_gb: memoryGb }, resource_type: resourceType };
42+
}
43+
3544
// ---------------------------------------------------------------------------
3645
// Basic ordering
3746
// ---------------------------------------------------------------------------
@@ -114,6 +123,35 @@ describe('compareByCapacity — null/absent/zero sort last', () => {
114123
const nullMem = rec('something', 4, null);
115124
expect(compareByCapacity(known, nullMem)).toBeLessThan(0);
116125
});
126+
127+
// NaN guard: typeof NaN === 'number' but NaN is not a finite capacity value.
128+
// extractCapacity must fold NaN into the unknown bucket (null) so that
129+
// compareNullable never sees it; NaN subtraction would return NaN and violate
130+
// the sort contract (NaN is neither < 0 nor > 0 nor === 0).
131+
it('NaN vcpu is treated as unknown and sorts after positive vcpu', () => {
132+
const known = rec('m5.large', 2, 8);
133+
const nanVcpu = recRaw('bad-type', NaN, 8);
134+
expect(compareByCapacity(known, nanVcpu)).toBeLessThan(0);
135+
expect(compareByCapacity(nanVcpu, known)).toBeGreaterThan(0);
136+
});
137+
138+
it('NaN memoryGB is treated as unknown and sorts after known memoryGB when vCPU ties', () => {
139+
const known = rec('c5.xlarge', 4, 8);
140+
const nanMem = recRaw('r5.xlarge', 4, NaN);
141+
expect(compareByCapacity(known, nanMem)).toBeLessThan(0);
142+
expect(compareByCapacity(nanMem, known)).toBeGreaterThan(0);
143+
});
144+
145+
it('two NaN-vcpu recs: tie resolved by resource_type (no NaN-comparison instability)', () => {
146+
const a = recRaw('alpha', NaN, NaN);
147+
const b = recRaw('beta', NaN, NaN);
148+
const ab = compareByCapacity(a, b);
149+
const ba = compareByCapacity(b, a);
150+
expect(ab).toBeLessThan(0);
151+
expect(ba).toBeGreaterThan(0);
152+
// Symmetry: sign must flip, not collapse to 0 or NaN.
153+
expect(Math.sign(ab)).toBe(-Math.sign(ba));
154+
});
117155
});
118156

119157
// ---------------------------------------------------------------------------

‎frontend/src/lib/capacity.ts‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -66,9 +66,18 @@ export function compareByCapacity(
6666

6767
/**
6868
* extractCapacity pulls vcpu / memory_gb from an opaque details blob.
69-
* Returns zeros (treated as unknown by the comparator) for non-compute
70-
* payloads or absent fields, matching the ComputeDetails omitempty
71-
* behaviour on the wire.
69+
* Returns null for non-compute payloads or absent/non-finite fields,
70+
* matching the ComputeDetails omitempty behaviour on the wire.
71+
*
72+
* Note: Number.isFinite() is used (not typeof === 'number') to reject NaN,
73+
* which passes the typeof check but would produce NaN subtraction results in
74+
* compareNullable and cause an unstable sort. NaN falls into the unknown bucket
75+
* and sorts last alongside null/absent/0 values.
76+
*
77+
* Intended consumer: the recommendations table capacity sort (clicking the
78+
* vCPU/Memory column header). Wiring that column requires a UX decision
79+
* (adding a new RecommendationsColumnId entry and a rendered column header);
80+
* see issue #82 for context.
7281
*/
7382
function extractCapacity(details: unknown): ComputeCapacity {
7483
if (
@@ -78,9 +87,8 @@ function extractCapacity(details: unknown): ComputeCapacity {
7887
) {
7988
const d = details as Record<string, unknown>;
8089
return {
81-
vcpu: typeof d['vcpu'] === 'number' ? d['vcpu'] : null,
82-
memory_gb:
83-
typeof d['memory_gb'] === 'number' ? d['memory_gb'] : null,
90+
vcpu: Number.isFinite(d['vcpu']) ? (d['vcpu'] as number) : null,
91+
memory_gb: Number.isFinite(d['memory_gb']) ? (d['memory_gb'] as number) : null,
8492
};
8593
}
8694
return { vcpu: null, memory_gb: null };

0 commit comments

Comments
 (0)