Skip to content

fix: prevent "NaN" market cap in watchlist when Finnhub omits the value#82

Open
Mubashirrrr wants to merge 1 commit into
Open-Dev-Society:mainfrom
Mubashirrrr:fix/format-number-nan-marketcap
Open

fix: prevent "NaN" market cap in watchlist when Finnhub omits the value#82
Mubashirrrr wants to merge 1 commit into
Open-Dev-Society:mainfrom
Mubashirrrr:fix/format-number-nan-marketcap

Conversation

@Mubashirrrr

@Mubashirrrr Mubashirrrr commented Jun 3, 2026

Copy link
Copy Markdown

Bug

The watchlist table renders the literal string NaN in the Market Cap column for any stock whose market cap is unavailable.

formatNumber (lib/utils.ts) computes num * 1e6 and formats the result. But getWatchlistData (lib/actions/finnhub.actions.ts) passes the value straight through:

marketCap: profile?.marketCapitalization,   // number | undefined

Finnhub's /stock/profile2 endpoint omits marketCapitalization for many symbols (ETFs and a lot of non-US tickers). When it's missing, formatNumber(undefined) evaluates undefined * 1e6 → NaN, falls through every value >= … branch, and returns NaN.toString()"NaN", which WatchlistTable.tsx renders directly:

<td className="px-6 py-4 text-gray-400 font-medium">
    {formatNumber(stock.marketCap)}
</td>

Reproduction

formatNumber(undefined) // => "NaN"  (rendered in the Market Cap cell)
formatNumber(NaN)       // => "NaN"

The included regression test asserts this and fails before the fix:

AssertionError: expected 'NaN' to be 'N/A'

Fix

Guard against undefined / null / non-finite input and return 'N/A' — the same convention already used by the sibling helper formatMarketCapValue. The parameter type is widened to number | null | undefined to reflect what actually reaches the function on the watchlist call path. No other behavior changes; the existing millions-scaling logic is untouched.

Regression test

Added a formatNumber block to __tests__/utils.test.ts covering:

  • the normal millions-scaled path (3_000_000 → "3.00T", 150_000 → "150.00B", 150 → "150.00M")
  • the missing / non-finite path (undefined, null, NaN, Infinity"N/A")

npm test is green (81 passed, 4 pre-existing skips). Confirmed the new test fails before the fix and passes after.

Summary by CodeRabbit

  • Bug Fixes
    • Improved number formatting to gracefully handle missing, null, or invalid market data by displaying "N/A" instead of potential errors, ensuring more stable and reliable data presentation throughout the application.

formatNumber computes `num * 1e6` and renders the result. When a stock's
market cap is unavailable, getWatchlistData passes
`profile?.marketCapitalization` straight through as undefined, so
WatchlistTable renders the literal string "NaN" in the Market Cap column.
This is common for ETFs and many non-US symbols where Finnhub's profile2
endpoint does not return marketCapitalization.

Guard against undefined/null/non-finite input and return 'N/A', matching
the convention already used by the sibling formatMarketCapValue helper.

Adds a regression test covering both the normal (millions-scaled) path
and the missing/non-finite path.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@vercel

vercel Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

@Mubashirrrr is attempting to deploy a commit to the ravixalgorithm's projects Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c03993eb-4389-4f29-bf90-1a4f45e027d4

📥 Commits

Reviewing files that changed from the base of the PR and between 8326133 and dd6afc8.

📒 Files selected for processing (2)
  • __tests__/utils.test.ts
  • lib/utils.ts

📝 Walkthrough

Walkthrough

The formatNumber utility function is enhanced to handle edge cases by accepting optional or nullable inputs and returning 'N/A' for undefined, null, or non-finite values. A comprehensive test suite validates the updated behavior, including formatting market-cap values in millions to suffixes (T, B, M) and error cases.

Changes

formatNumber Input Validation

Layer / File(s) Summary
formatNumber robustness update
lib/utils.ts, __tests__/utils.test.ts
formatNumber signature widens to accept num?: number | null and returns 'N/A' for invalid inputs. Tests verify suffix formatting for millions (→M), billions (→B), trillions (→T) and error handling for undefined, null, NaN, and Infinity.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A number walks in, could be null or divine,
formatNumber asks: "Are you finite and fine?"
If not, it returns a sweet 'N/A' reply,
While tests keep watch—no edge case to defy!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main fix: preventing 'NaN' display in market cap when Finnhub omits the value. It is specific, concise, and directly reflects the core change.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant