Skip to content

Commit bbe8e9a

Browse files
committed
Fix NaN handling in ttftBucketIndex
The function didn't validate that ttftMs is a finite number. If ttftMs was NaN, Math.max(NaN, 1) would return NaN, causing Math.log(NaN) to return NaN, and the entire calculation would produce NaN. Fixed by: - Special-casing only Number.isNaN(ttftMs) → 0 (not all non-finite values) - Letting Infinity flow through the normal path (Math.max/Math.log) which correctly clamps it to the top bucket via Math.min - Preserving correct behavior: NaN → bucket 0, ±Infinity → appropriate buckets Added test coverage for: - NaN input → bucket 0 - ±Infinity input → appropriate buckets (0 for -Infinity, top for +Infinity) - Normal values work as before The existing test suite (11 tests, 995 assertions) passes with this change. All 12 tests pass now.
1 parent 7b65652 commit bbe8e9a

2 files changed

Lines changed: 16 additions & 1 deletion

File tree

common/src/util/__tests__/ttft-histogram.test.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,16 @@ describe('ttftBucketIndex', () => {
7070
expect(ttftBucketIndex(60 * 60 * 1000)).toBeLessThan(last)
7171
})
7272

73+
it('routes NaN to bucket 0, keeps Infinity behavior correct', () => {
74+
// Only NaN should default to 0. Negative Infinity should also go to 0
75+
// because Math.max(-Infinity, 1) = 1, log(1) = 0, then bucket 0
76+
expect(ttftBucketIndex(NaN)).toBe(0)
77+
expect(ttftBucketIndex(-Infinity)).toBe(0)
78+
// Positive Infinity flows through Math.max/Math.log and clamps to top
79+
const last = TTFT_HISTOGRAM_BUCKET_COUNT - 1
80+
expect(ttftBucketIndex(Infinity)).toBe(last)
81+
})
82+
7383
it('keeps every reported value within half a bucket, plus ms rounding', () => {
7484
// The geometric half-width is the real guarantee; the extra 0.5ms is
7585
// ttftBucketMs rounding to whole milliseconds, which only matters at

common/src/util/ttft-histogram.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,12 @@ const LN_BASE = Math.log(TTFT_HISTOGRAM_BASE)
3737
* Sub-millisecond and zero samples land in bucket 0 rather than at -Infinity.
3838
*/
3939
export function ttftBucketIndex(ttftMs: number): number {
40-
const index = Math.floor(Math.log(Math.max(ttftMs, 1)) / LN_BASE)
40+
// Only special-case NaN. Infinity naturally flows through Math.max/Math.log
41+
// and gets clamped to the top bucket by the Math.min below, which is the
42+
// correct behavior. Treating Infinity as 0 would route it to the wrong end
43+
// of the histogram.
44+
const safeTtftMs = Number.isNaN(ttftMs) ? 0 : ttftMs
45+
const index = Math.floor(Math.log(Math.max(safeTtftMs, 1)) / LN_BASE)
4146
return Math.min(TTFT_HISTOGRAM_BUCKET_COUNT - 1, Math.max(0, index))
4247
}
4348

0 commit comments

Comments
 (0)