Skip to content

fix: missing datapoints#299

Merged
antoncoding merged 3 commits intomasterfrom
fix/chart-current-data-point
Jan 15, 2026
Merged

fix: missing datapoints#299
antoncoding merged 3 commits intomasterfrom
fix/chart-current-data-point

Conversation

@antoncoding
Copy link
Copy Markdown
Owner

@antoncoding antoncoding commented Jan 15, 2026

Summary by CodeRabbit

  • Bug Fixes

    • Improved null-value handling and filtering in chart series to prevent invalid points.
    • Averages and stats now exclude missing values and default safely when data is absent.
    • Charts handle missing historical data more robustly to avoid misleading visuals.
  • New Features

    • Added a real-time "now" data point to the volume chart in asset view reflecting current on-chain state.
  • Improvements

    • Simplified timeframe selection for more immediate updates and reliable recomputation.

✏️ Tip: You can customize this high-level summary in your review settings.

@vercel
Copy link
Copy Markdown
Contributor

vercel Bot commented Jan 15, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
monarch Ready Ready Preview, Comment Jan 15, 2026 3:50am

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jan 15, 2026

📝 Walkthrough

Walkthrough

Rate and Volume chart components now build chart series with stricter null-safety, derive current "now" values from live market.state (volume chart), and update timeframe selection by calling setTimeframe directly.

Changes

Cohort / File(s) Summary
Rate chart
src/features/market-detail/components/charts/rate-chart.tsx
Replaced local timeframe handler with direct setTimeframe; introduced chartData via useMemo; filter out null points and guard against null y values; updated average calculation to ignore nulls; AreaChart uses chartData.
Volume chart
src/features/market-detail/components/charts/volume-chart.tsx
Removed wrapper timeframe handler; added moment import and STATE_KEY_MAP; derive current values from market.state, append a live "now" point in Asset view; filter null y-values; refactored getVolumeStats to use asset-state current and asset-based averages; expanded useMemo deps to include market.state.
Shared rendering / deps
src/features/market-detail/components/charts/*.tsx
Charts switched to memoized chartData; timeframe Selects call setTimeframe directly; dependency lists updated so charts recompute on live state changes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • refactor: prop drills #267: Modifies the same chart components and similarly replaces local timeframe handlers with direct setTimeframe calls and adjusts chart data/null-safety.
🚥 Pre-merge checks | ✅ 2 | ❌ 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 (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: missing datapoints' directly matches the core change—adding missing data points to charts through improved null handling and current-state point injection.

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/chart-current-data-point

🧹 Recent nitpick comments
src/features/market-detail/components/charts/volume-chart.tsx (2)

2-2: Consider native Date instead of moment for timestamps.

moment().unix() can be replaced with Math.floor(Date.now() / 1000). Avoids pulling in moment if it's not used elsewhere.


129-133: STATE_KEY_MAP can be moved outside the component.

It's a constant. Defining it inside means it's recreated each render. Minor optimization.

Move outside component
+const STATE_KEY_MAP = {
+  supply: 'supplyAssets',
+  borrow: 'borrowAssets',
+  liquidity: 'liquidityAssets',
+} as const;
+
 function VolumeChart({ marketId, chainId, market }: VolumeChartProps) {
   // ...
-  const STATE_KEY_MAP = {
-    supply: 'supplyAssets',
-    borrow: 'borrowAssets',
-    liquidity: 'liquidityAssets',
-  } as const;

📜 Recent review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7b13a7f and 1d58e39.

📒 Files selected for processing (1)
  • src/features/market-detail/components/charts/volume-chart.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/features/market-detail/components/charts/volume-chart.tsx (2)
src/utils/types.ts (1)
  • TimeseriesDataPoint (336-339)
src/utils/balance.ts (1)
  • formatReadable (25-49)
🔇 Additional comments (5)
src/features/market-detail/components/charts/volume-chart.tsx (5)

64-77: Good fallback for missing historical data.

Clear handling: Asset mode shows current state, USD mode returns empty since market.state lacks USD values.


102-114: Correct: nowPoint restricted to Asset mode.

Addresses the unit mismatch issue - no USD values available from market.state, so we don't append a point with mismatched units in USD view.


135-161: Good refactor for unit consistency.

netChangePercentage now compares asset-to-asset, avoiding the unit mismatch between asset current and USD start. Comments clearly explain the design choices.


234-237: Direct setTimeframe call is cleaner.

Removes unnecessary wrapper.


83-101: Type mismatch: TimeseriesDataPoint.y is typed as number, but code checks for null.

The type definition has y: number (not nullable), yet the code defensively checks for null at runtime. This same pattern appears across multiple chart components (rate-chart.tsx also does point.y === null), suggesting either the type should be y: number | null or the null checks are unnecessary.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


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.

@coderabbitai coderabbitai Bot added the ui User interface label Jan 15, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@src/features/market-detail/components/charts/volume-chart.tsx`:
- Around line 99-106: getVolumeStats and the nowPoint construction mix
asset-unit current values with USD historical points when volumeView === 'USD';
fix by ensuring current and nowPoint use USD-denominated numbers: in
getVolumeStats, when volumeView === 'USD' compute current from USD equivalents
(e.g., market.state.supplyAssetsUsd / borrowAssetsUsd / liquidityAssetsUsd) or
convert asset units to USD via the same price feed used for historical points
before running netChangePercentage, and in the nowPoint block only append USD
values (use supplyAssetsUsd/borrowAssetsUsd/liquidityAssetsUsd or converted
values) or skip appending nowPoint when USD equivalents are unavailable; update
references to volumeView, getVolumeStats, nowPoint and market.state to apply the
USD conversion or conditional append so units match between current and
historical data.
🧹 Nitpick comments (1)
src/features/market-detail/components/charts/volume-chart.tsx (1)

121-125: Move constant outside component.

STATE_KEY_MAP is recreated each render. Hoist it to module scope.

+const STATE_KEY_MAP = {
+  supply: 'supplyAssets',
+  borrow: 'borrowAssets',
+  liquidity: 'liquidityAssets',
+} as const;
+
 function VolumeChart({ marketId, chainId, market }: VolumeChartProps) {
   ...
-  const STATE_KEY_MAP = {
-    supply: 'supplyAssets',
-    borrow: 'borrowAssets',
-    liquidity: 'liquidityAssets',
-  } as const;
📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 03393f7 and 5f43188.

📒 Files selected for processing (2)
  • src/features/market-detail/components/charts/rate-chart.tsx
  • src/features/market-detail/components/charts/volume-chart.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
src/features/market-detail/components/charts/rate-chart.tsx (2)
src/utils/types.ts (1)
  • TimeseriesDataPoint (336-339)
src/utils/rateMath.ts (1)
  • convertApyToApr (15-29)
src/features/market-detail/components/charts/volume-chart.tsx (2)
src/utils/types.ts (1)
  • TimeseriesDataPoint (336-339)
src/utils/balance.ts (1)
  • formatReadable (25-49)
🔇 Additional comments (7)
src/features/market-detail/components/charts/rate-chart.tsx (4)

47-70: Null-safety logic looks solid.

The filtering approach handles potentially malformed data well. One minor note: lines 59-60 use ?? 0 fallback, but the null check on line 54 already ensures these values aren't null. The fallback is harmless but redundant.


76-81: Good defensive filtering for averages.

Worth noting: TimeseriesDataPoint.y is typed as number (not nullable) in src/utils/types.ts. If nulls are expected at runtime, consider updating the type definition to y: number | null for accuracy.


132-132: Direct state setter is cleaner.

Removes indirection. Type assertion is safe given the constrained SelectItem values.


160-162: LGTM.

Using memoized chartData directly.

src/features/market-detail/components/charts/volume-chart.tsx (3)

64-73: Good fallback for missing data.

Returns single point with current state. Same USD/Asset concern applies here though - if volumeView === 'USD', these values won't be in USD units.


79-97: Null filtering is appropriate.

Type guard and outlier filter (100B USD threshold) look reasonable.


221-221: Consistent with rate-chart.

Direct setter, type assertion safe.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

Comment thread src/features/market-detail/components/charts/volume-chart.tsx Outdated
Comment thread src/features/market-detail/components/charts/volume-chart.tsx Outdated
@antoncoding antoncoding merged commit 2846c89 into master Jan 15, 2026
4 checks passed
@antoncoding antoncoding deleted the fix/chart-current-data-point branch January 15, 2026 03:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ui User interface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant