feat: pie chart and concentration chart#302
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds four new market-detail charts (borrowers/suppliers pie charts, concentration chart, collateral-at-risk area chart), new hooks to fetch all market positions, a persistent tab preference store, and small defaults/parameter changes to existing position hooks and market view integration. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant MarketView
participant Hooks as useAllMarketPositions
participant API as Morpho/Subgraph
participant Charts as Pie/Concentration/Collateral
User->>MarketView: open market detail (selects tab)
MarketView->>Hooks: request borrowers & suppliers (marketId, network)
Hooks->>API: fetch positions (Morpho preferred, fallback to Subgraph)
API-->>Hooks: return items & totalCount
Hooks-->>MarketView: provide normalized data
MarketView->>Charts: pass data + oraclePrice
Charts->>Charts: compute segments/metrics and render
User->>Charts: click "Other" segment
Charts->>Charts: toggle expansion and show detailed list
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/features/market-detail/components/charts/collateral-at-risk-chart.tsx`:
- Around line 51-57: The collateral-to-loan value calculation in
collateralValueInLoan (used to compute currentLTV) ignores token decimal
differences; update the formula in collateral-at-risk-chart.tsx to multiply by
BigInt(10 ** market.collateralAsset.decimals) and divide by BigInt(10 **
market.loanAsset.decimals) along with the existing 10**36 scaling (i.e., adjust
the (collateral * oraclePrice) term to account for
market.collateralAsset.decimals and market.loanAsset.decimals). Apply the same
decimal-adjusted calculation to all LTV/debt conversions in borrowers-table.tsx,
borrowers-pie-chart.tsx and the borrow modal components where
collateralValueInLoan, currentLTV or similar computed values appear (search for
collateralValueInLoan, currentLTV, debtValue, and any direct (collateral *
oraclePrice) expressions) so all LTV math consistently uses
market.collateralAsset.decimals and market.loanAsset.decimals.
🧹 Nitpick comments (1)
src/features/market-detail/components/charts/concentration-chart.tsx (1)
33-34: Minor: Redundant filter condition.The condition
p.percentage > 0 && p.percentage >= MIN_PERCENT_THRESHOLDis redundant sinceMIN_PERCENT_THRESHOLDis 0.1. The> 0check is always satisfied when>= 0.1.Suggested simplification
- const meaningful = sorted.filter((p) => p.percentage > 0 && p.percentage >= MIN_PERCENT_THRESHOLD); + const meaningful = sorted.filter((p) => p.percentage >= MIN_PERCENT_THRESHOLD);
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/features/market-detail/components/charts/collateral-at-risk-chart.tsxsrc/features/market-detail/components/charts/concentration-chart.tsxsrc/features/market-detail/market-view.tsx
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-11-25T09:39:42.148Z
Learnt from: antoncoding
Repo: antoncoding/monarch PR: 87
File: app/home/HomePage.tsx:17-39
Timestamp: 2024-11-25T09:39:42.148Z
Learning: In `app/home/HomePage.tsx`, the `useEffect` hook depends on `[showCustomized]` because changing `showCustomized` triggers updates to the yield and risk terms.
Applied to files:
src/features/market-detail/components/charts/collateral-at-risk-chart.tsxsrc/features/market-detail/market-view.tsx
📚 Learning: 2024-10-12T09:23:16.495Z
Learnt from: antoncoding
Repo: antoncoding/monarch PR: 63
File: app/markets/components/MarketRowDetail.tsx:49-52
Timestamp: 2024-10-12T09:23:16.495Z
Learning: When rendering oracle feeds in `ExpandedMarketDetail` (`app/markets/components/MarketRowDetail.tsx`), prefer explicit rendering over iterating keys when dealing with a small number of feeds.
Applied to files:
src/features/market-detail/market-view.tsx
🧬 Code graph analysis (1)
src/features/market-detail/components/charts/concentration-chart.tsx (2)
src/components/ui/spinner.tsx (1)
Spinner(22-58)src/features/market-detail/components/charts/chart-utils.tsx (2)
ChartGradients(17-43)chartTooltipCursor(124-128)
🔇 Additional comments (4)
src/features/market-detail/components/charts/collateral-at-risk-chart.tsx (1)
1-241: Overall implementation looks good.The chart correctly visualizes collateral at risk with appropriate handling of edge cases, loading states, and a clear UI. The step function approach for cumulative debt and the risk metrics summary are well-implemented.
src/features/market-detail/market-view.tsx (2)
138-163: Inconsistent fallback values.When
marketis undefined,scaledMinSupplierSharesandscaledMinBorrowerSharesreturn'1'(lines 148, 161). But whenmarketexists and input is empty,convertAssetToSharesreturns'0'. This creates inconsistent filtering behavior during initial load vs after market loads.Was the
'1'fallback intentional to filter dust positions, or should it be'0'for consistency?
358-396: Charts integration looks good.The layout is well-structured with responsive grid columns. Data flows correctly from the hooks through the memoized concentration data to the chart components.
src/features/market-detail/components/charts/concentration-chart.tsx (1)
1-235: Clean implementation overall.The concentration chart handles multiple edge cases well: loading, empty data, and filtered-out positions. The Lorenz curve comparison with ideal distribution is a nice touch for visualizing concentration.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| // Calculate collateral value in loan asset terms | ||
| const collateralValueInLoan = (collateral * oraclePrice) / BigInt(10 ** 36); | ||
| if (collateralValueInLoan === 0n) return null; | ||
|
|
||
| // Calculate current LTV as decimal | ||
| const currentLTV = Number(borrowAssets) / Number(collateralValueInLoan); | ||
| const debtValue = Number(formatUnits(borrowAssets, market.loanAsset.decimals)); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for oracle price usage with decimal adjustments
rg -n "oraclePrice.*10\s*\*\*" --type=ts --type=tsx -B2 -A2Repository: antoncoding/monarch
Length of output: 90
🏁 Script executed:
#!/bin/bash
# Search for ORACLE_PRICE_SCALE or similar constants
rg -n "ORACLE_PRICE_SCALE|36.*decimal|oracle.*scale" --type=ts --type=tsx -iRepository: antoncoding/monarch
Length of output: 90
🏁 Script executed:
#!/bin/bash
# Look at market-view.tsx to understand the oracle price adjustment
fd "market-view.tsx" --type=fRepository: antoncoding/monarch
Length of output: 192
🏁 Script executed:
#!/bin/bash
# Search for how collateral value is calculated in the codebase
rg -n "collateralValue|collateral.*oraclePrice" --type=ts --type=tsx -B2 -A2Repository: antoncoding/monarch
Length of output: 90
🏁 Script executed:
#!/bin/bash
# Search for oracle price usage with decimal adjustments (broader search)
rg -n "oraclePrice.*10\s*\*\*" -B2 -A2Repository: antoncoding/monarch
Length of output: 4711
🏁 Script executed:
#!/bin/bash
# Look at the market-view.tsx in market-detail folder around line 123
cat -n src/features/market-detail/market-view.tsx | sed -n '110,140p'Repository: antoncoding/monarch
Length of output: 1469
🏁 Script executed:
#!/bin/bash
# Check collateral-at-risk-chart.tsx to see full context
cat -n src/features/market-detail/components/charts/collateral-at-risk-chart.tsx | sed -n '1,80p'Repository: antoncoding/monarch
Length of output: 4030
🏁 Script executed:
#!/bin/bash
# Search for how the oracle price scale is defined or used
rg -n "36|ORACLE" | grep -i "scale\|decimal\|price"Repository: antoncoding/monarch
Length of output: 50375
🏁 Script executed:
#!/bin/bash
# Look at borrowers-table.tsx around line 54 to see the full context
cat -n src/features/market-detail/components/borrowers-table.tsx | sed -n '45,75p'Repository: antoncoding/monarch
Length of output: 1303
🏁 Script executed:
#!/bin/bash
# Check if there are any comments or documentation about decimal adjustment
rg -n "collateralAsset.decimals|loanAsset.decimals" -B2 -A2Repository: antoncoding/monarch
Length of output: 50375
Collateral value calculation needs decimal adjustment for different asset decimals.
The formula (collateral * oraclePrice) / BigInt(10 ** 36) doesn't account for differences between collateral and loan asset decimals. This is inconsistent with how market-view.tsx handles the same oracle price:
const adjusted = (oraclePrice * BigInt(10 ** market.collateralAsset.decimals)) / BigInt(10 ** market.loanAsset.decimals);The comment in borrowers-table.tsx (line 53) acknowledges this: "oraclePrice is scaled by 10^36, need to adjust for token decimals"—but the adjustment is missing from that file and others. Update the calculation to:
const collateralValueInLoan = (collateral * oraclePrice * BigInt(10 ** market.collateralAsset.decimals)) / (BigInt(10 ** 36) * BigInt(10 ** market.loanAsset.decimals));Apply this fix to all LTV calculations in collateral-at-risk-chart.tsx, borrowers-table.tsx, borrowers-pie-chart.tsx, and the borrow modals.
🤖 Prompt for AI Agents
In `@src/features/market-detail/components/charts/collateral-at-risk-chart.tsx`
around lines 51 - 57, The collateral-to-loan value calculation in
collateralValueInLoan (used to compute currentLTV) ignores token decimal
differences; update the formula in collateral-at-risk-chart.tsx to multiply by
BigInt(10 ** market.collateralAsset.decimals) and divide by BigInt(10 **
market.loanAsset.decimals) along with the existing 10**36 scaling (i.e., adjust
the (collateral * oraclePrice) term to account for
market.collateralAsset.decimals and market.loanAsset.decimals). Apply the same
decimal-adjusted calculation to all LTV/debt conversions in borrowers-table.tsx,
borrowers-pie-chart.tsx and the borrow modal components where
collateralValueInLoan, currentLTV or similar computed values appear (search for
collateralValueInLoan, currentLTV, debtValue, and any direct (collateral *
oraclePrice) expressions) so all LTV math consistently uses
market.collateralAsset.decimals and market.loanAsset.decimals.
There was a problem hiding this comment.
this is incorrect. oracle.price already absorb the decimals.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for oracle price documentation and comments about decimal handling
rg -n "oracle.*price.*decimal|oracle.*absorb|10\s*\*\*\s*36" -i -B2 -A2
# Look for oracle price type definitions or interfaces
rg -n "oraclePrice.*type|interface.*oracle" -i -B2 -A2
# Find where oracle price is fetched or defined
rg -n "getOraclePrice|fetchOracle|oracle.*=.*price" -B3 -A3
Length of output: 9835
@antoncoding You're absolutely right, and I apologize for the incorrect review comment.
Looking at the codebase, the oracle price in Morpho is scaled by 10^36 and already incorporates the decimal differences between collateral and loan assets. The formula (collateral * oraclePrice) / BigInt(10 ** 36) correctly converts collateral value to loan asset terms without needing additional decimal adjustments.
The decimal adjustment in market-view.tsx line 123 is specifically for display purposes—to format the oracle price as a human-readable ratio—not for LTV calculations. The comment in borrowers-table.tsx line 53 ("need to adjust for token decimals") is misleading since the code correctly doesn't perform that adjustment.
The current implementation across all files is correct as-is.
✏️ Learnings added
Learnt from: antoncoding
Repo: antoncoding/monarch PR: 302
File: src/features/market-detail/components/charts/collateral-at-risk-chart.tsx:51-57
Timestamp: 2026-01-15T09:30:41.378Z
Learning: In Morpho Blue, the oracle price (oraclePrice) is scaled by 10^36 and already incorporates decimal differences between collateral and loan assets. When calculating collateral value in loan asset terms, use `(collateral * oraclePrice) / BigInt(10 ** 36)` without additional decimal adjustments. The decimal adjustment in market-view.tsx is only for display formatting, not for LTV/value calculations.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: antoncoding
Repo: antoncoding/monarch PR: 87
File: app/home/HomePage.tsx:17-39
Timestamp: 2024-11-25T09:39:42.148Z
Learning: In `app/home/HomePage.tsx`, the `useEffect` hook depends on `[showCustomized]` because changing `showCustomized` triggers updates to the yield and risk terms.
Additional
Summary by CodeRabbit
New Features
Chores
Behavior
✏️ Tip: You can customize this high-level summary in your review settings.