Skip to content

Fix NaN handling in min-heap index calculations - #1249

Open
pavankumar-vh wants to merge 1 commit into
CodebuffAI:mainfrom
pavankumar-vh:fix/min-heap-index-validation
Open

Fix NaN handling in min-heap index calculations#1249
pavankumar-vh wants to merge 1 commit into
CodebuffAI:mainfrom
pavankumar-vh:fix/min-heap-index-validation

Conversation

@pavankumar-vh

Copy link
Copy Markdown

Overview

Fix NaN handling in min-heap index calculations in common/src/util/min-heap.ts.

Bug Description

The functions didn't validate that index is a finite number. If index was NaN or Infinity, Math.floor((NaN - 1) / 2) would return NaN, causing the heap operations to fail.

Fix

Added Number.isFinite() checks to default to 0 for invalid numbers.

Testing

No existing tests for this function, but the fix prevents runtime errors with invalid inputs.

Files Changed

  • common/src/util/min-heap.ts - Added NaN validation

Scope

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

The functions didn't validate that index is a finite number. If index was
NaN or Infinity, Math.floor((NaN - 1) / 2) would return NaN, causing the heap
operations to fail.

Added Number.isFinite() checks to default to 0 for invalid numbers.
@codebuff-team

Copy link
Copy Markdown
Contributor

Thanks for the contribution, but this doesn't fix an actual bug. getParentIndex, getLeftChildIndex, and getRightChildIndex are private methods only ever called internally with indices derived from this.heap.length or loop counters incrementing/decrementing from valid starting points — there's no path in min-heap.ts where index becomes NaN or Infinity. Silently coercing an invalid index to 0 would also be the wrong fix even if this could happen: swallowing programmer errors in a heap's internal bookkeeping produces corrupted heap ordering rather than a clean crash, which is strictly worse for debugging.

If you found a real code path that produces a non-finite index (e.g. via a NaN score somewhere upstream), please include a failing test that reproduces it and a fix at the actual source of the NaN, not a defensive patch three call-sites downstream. As written, this adds dead branches with no test coverage and no reproducible bug behind it, so it isn't something a maintainer can act on.

@codebuff-team codebuff-team added bot:triaged Classified by the community triage bot pr:rejected Not a change this project wants; closed with an explanation labels Sep 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:triaged Classified by the community triage bot pr:rejected Not a change this project wants; closed with an explanation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants