Fix NaN handling in responseAdSlotCount with tests

[pavankumar-vh]

Overview

Fix NaN handling in the responseAdSlotCount function in common/src/util/response-ad-positions.ts.

Bug Description

The function didn't validate that nodeCount is a finite number. If nodeCount was NaN or Infinity, Math.floor((NaN - firstAdAfterNodes - 1) / step) would return NaN, causing incorrect results.

Fix

Added Number.isFinite() checks for nodeCount, step, and firstAdAfterNodes to default to safe values for invalid numbers.

Testing

Added comprehensive test coverage for:

• Valid nodeCount values (0, 1, 5, 10, 20, etc.)
• NaN, Infinity, and negative nodeCount handling
• Custom step and firstAdAfterNodes parameters
• Empty and valid adCount cases

All 11 tests pass.

Files Changed

• common/src/util/response-ad-positions.ts - Added NaN validation
• common/src/util/__tests__/response-ad-positions.test.ts - Added test coverage

Scope

This change only touches common/ which is an approved contribution area per the Contributing Guide.

[codebuff-team]

Good catch: Math.max(0, Math.floor((NaN - firstAdAfterNodes - 1) / step) + 1) does in fact evaluate to NaN, not 0, because Math.max with a NaN operand returns NaN rather than clamping it. Your fix correctly guards nodeCount, step, and firstAdAfterNodes with Number.isFinite before they enter the arithmetic, and the added tests in response-ad-positions.test.ts cover the NaN/Infinity/negative cases plus the existing happy path for both responseAdSlotCount and responseAdNodePositions. This is in scope (common/) and the change is small and well-isolated.

A few things worth tightening before this is ready to port:

1. nodeCount is typed as number and, in every real call site I'd expect, comes from something like nodes.length, which is always a finite integer. It would help the review if you can point to where an untrusted/derived nodeCount could actually go NaN or Infinity in practice — otherwise this reads as defensive coding against a scenario that can't occur, which is fine but worth being upfront about in the PR description.
2. Lines like the safeStep/safeFirstAdAfterNodes declarations are quite long (well past typical 80-100 col limits) — run this through the project's prettier config so it doesn't need reformatting on port.
3. Minor: you could simplify params.step !== undefined && Number.isFinite(params.step) to just Number.isFinite(params.step) since Number.isFinite(undefined) is already false.

Overall the logic is sound and the tests are genuinely useful regression coverage. Clean up the formatting and firm up the justification and this is portable.