feat: add vault share price chart and adapter caps support#532
Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughPR adds historical vault share-price charts, makes adapterType optional and derives aliases from active adapters, tracks positive-cap metrics, introduces a 3-month period, centralizes period controls, and refactors vault/autovault pages to render multi-adapter details and analytics controls. ChangesShare Price Charts, Adapter Caps Awareness, and Period Control Refactor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
coderabbitai
Bot
added
feature request
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/data-sources/morpho-api/vault-share-price-history.ts`:
- Around line 65-79: The mapping that builds normalizedPoints currently trusts
point.x for timestamp; before returning each MorphoVaultSharePricePoint ensure
the timestamp is a valid finite number (e.g., coerce with Number(point.x) and
test with Number.isFinite) and drop points with non-finite timestamps; update
the map/filter pipeline around normalizeSharePrice(point.y) to also validate the
timestamp and only return objects with a finite timestamp so sortByTimestamp and
downstream chart math cannot receive invalid values.
In `@src/features/autovault/components/vault-detail/vault-header.tsx`:
- Around line 102-104: The current logic initializes capsAdapterRows with a
fallback to the single `adapter` value which can incorrectly indicate configured
caps and render “Active” badges; change the caps state to rely only on
`capsAdapters` (remove the fallback to `adapter`) so `capsAdapterRows =
capsAdapters`, then recompute `capsAdapterKeys = new Set(capsAdapterRows.map(row
=> row.adapter.toLowerCase()))` and keep `showCapsAdapter = adapterRows.length >
1 && capsAdapterRows.length > 0` so the UI only reflects actual `capsAdapters`
data; update any downstream usages that assumed the fallback to `adapter`
accordingly (symbols: capsAdapterRows, capsAdapters, adapter, capsAdapterKeys,
showCapsAdapter, adapterRows).
In `@src/features/history/components/transaction-history-preview.tsx`:
- Around line 50-53: In sourceAccounts.flatMap in
transaction-history-preview.tsx the normalization only lowercases addresses but
doesn't trim whitespace, so use a trimmed lowercase value (e.g., compute
normalizedAccount from sourceAccount.trim().toLowerCase()) and then check the
falsy/seen Set using that trimmed value; update the variable used in subsequent
logic (normalizedAccount and any seen.add calls) to prevent padded strings from
slipping through.
In `@src/features/market-detail/components/charts/chart-utils.tsx`:
- Around line 159-171: The onClick and formatter handlers for the chart legend
currently coerce entry.dataKey even when it's undefined, which creates a literal
"undefined" key in the visibleLines state; update the onClick in the legend and
the formatter to guard that entry.dataKey is non-null/undefined before calling
String or using it as a key (return early from onClick if missing), and use the
guarded key when reading from visibleLines in formatter (e.g., compute const key
= entry.dataKey != null ? String(entry.dataKey) as keyof T : null and only
toggle or read state when key is not null) so you never create or reference the
"undefined" state key; apply changes to the functions referenced as onClick,
formatter, setVisibleLines, and visibleLines.
In `@src/features/positions/components/supplied-morpho-blue-grouped-table.tsx`:
- Line 388: Inconsistent label for the 'threemonth' period exists across
components; standardize it to a single format (use '3M') by updating the
'threemonth' mapping currently set in supplied-morpho-blue-grouped-table (and
user-vaults-table) and changing the occurrences in overview-tab and
position-view from '3mo' to '3M'; ideally extract this into a shared constant
(e.g., PERIOD_LABELS or a PERIODS enum with a 'threemonth' key) exported and
imported by the components to avoid future drift.
In `@src/hooks/useVaultSharePriceHistory.ts`:
- Around line 70-106: selectNearestMorphoPoints currently prevents reusing a
source point by tracking usedIndexes, which causes sparse early points to be
consumed and leaves later targets unmapped or stale; change the algorithm to
match each targetTimestamp to its nearest Morpho point independently (remove
usedIndexes, related checks, and usedIndexes.add) so a single source point may
be reused for multiple targets, still guarding for empty points and sorting the
resulting VaultSharePricePoint[] by targetTimestamp before returning; keep
function signature and field mapping (sharePrice, source:'morpho-api',
timestamp, targetTimestamp).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 65f695a1-f417-42e7-bc58-1f0017701c98
📒 Files selected for processing (28)
docs/VALIDATIONS.mdsrc/contexts/VaultRegistryContext.tsxsrc/data-sources/monarch-api/vaults.tssrc/data-sources/morpho-api/vault-share-price-history.tssrc/features/autovault/components/vault-detail/vault-header.tsxsrc/features/autovault/vault-view.tsxsrc/features/history/components/transaction-history-preview.tsxsrc/features/market-detail/components/charts/chart-utils.tsxsrc/features/position-detail/components/overview-tab.tsxsrc/features/position-detail/components/position-period-selector.tsxsrc/features/position-detail/position-view.tsxsrc/features/positions/components/supplied-morpho-blue-grouped-table.tsxsrc/features/positions/components/user-positions-chart.tsxsrc/features/positions/components/user-vaults-table.tsxsrc/features/vault/components/vault-adapter-position-overview.tsxsrc/features/vault/components/vault-analytics-period-control.tsxsrc/features/vault/components/vault-market-allocations-table.tsxsrc/features/vault/components/vault-share-price-chart.tsxsrc/features/vault/vault-view.tsxsrc/graphql/vault-queries.tssrc/hooks/useAddressLabel.tssrc/hooks/useMorphoMarketAdapters.tssrc/hooks/usePositionsWithEarnings.tssrc/hooks/useVaultSharePriceHistory.tssrc/hooks/useVaultV2Data.tssrc/stores/usePositionsFilters.tssrc/utils/vaultAllocation.tssrc/utils/vaults.ts
| day: '1D', | ||
| week: '7D', | ||
| month: '30D', | ||
| threemonth: '3M', |
There was a problem hiding this comment.
Inconsistent period label format across the UI.
The threemonth period uses different label formats in different parts of the UI:
'3M'here and inuser-vaults-table.tsx(Line 30)'3mo'inoverview-tab.tsx(Line 25) andposition-view.tsx(Line 32)
Consider standardizing to one format for consistency.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/features/positions/components/supplied-morpho-blue-grouped-table.tsx` at
line 388, Inconsistent label for the 'threemonth' period exists across
components; standardize it to a single format (use '3M') by updating the
'threemonth' mapping currently set in supplied-morpho-blue-grouped-table (and
user-vaults-table) and changing the occurrences in overview-tab and
position-view from '3mo' to '3M'; ideally extract this into a shared constant
(e.g., PERIOD_LABELS or a PERIODS enum with a 'threemonth' key) exported and
imported by the components to avoid future drift.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a vault share price history chart and refactors vault adapter management to support multiple active adapters. Key changes include the addition of a shared analytics period control, a new share price history data source (Morpho API with RPC fallback), and updated logic for identifying and displaying configured adapters based on vault caps. Feedback focuses on handling edge cases in share price calculations (e.g., zero baseline), potential performance optimizations for data point selection, and ensuring RPC batch sizes are tuned for rate limits.
| const baseline = chartData[0].sharePrice; | ||
| const dataMin = Math.min(...values); | ||
| const dataMax = Math.max(...values); | ||
| const minimumGrowth = Math.max(Math.abs(baseline) * SHARE_PRICE_DOMAIN_GROWTH_BY_TIMEFRAME[timeframe], Number.EPSILON); |
There was a problem hiding this comment.
The minimumGrowth calculation could potentially be zero if baseline is 0. While Number.EPSILON is used as a fallback, consider if this is the intended behavior for vaults with a share price of 0 or if a more robust minimum should be defined.
Comment on lines
+140
to
+143
| const changePercent = | ||
| firstPoint && lastPoint && firstPoint.sharePrice > 0 | ||
| ? ((lastPoint.sharePrice - firstPoint.sharePrice) / firstPoint.sharePrice) * 100 | ||
| : null; |
There was a problem hiding this comment.
The changePercent calculation assumes firstPoint.sharePrice > 0. If firstPoint.sharePrice is 0, this will be null. Ensure that the UI handles null correctly, which it seems to do, but consider if a different calculation is needed for 0-value share prices.
| }); | ||
| } | ||
|
|
||
| return selected.sort((left, right) => left.targetTimestamp - right.targetTimestamp); |
There was a problem hiding this comment.
The selectNearestMorphoPoints function uses an O(N*M) approach. While acceptable for small datasets, if the number of points grows significantly, consider optimizing this to O(N log M) using binary search.
Comment on lines
+233
to
+239
| for (let i = 0; i < blocksWithTimestamps.length; i += PARALLEL_BATCH_SIZE) { | ||
| const batch = blocksWithTimestamps.slice(i, i + PARALLEL_BATCH_SIZE); | ||
| const batchResults = await Promise.all( | ||
| batch.map((block) => fetchSharePricePoint(client, vaultAddress, oneShareUnit, assetDecimals, block)), | ||
| ); | ||
| points.push(...batchResults.filter((point): point is VaultSharePricePoint => point !== null)); | ||
| } |
There was a problem hiding this comment.
The parallel batch processing for RPC calls is a good optimization. This aligns with the repository's preference for addressing performance bottlenecks via batched data fetching at the data layer rather than adding UI state complexity. Ensure that PARALLEL_BATCH_SIZE is tuned appropriately for the RPC provider's rate limits to avoid 429 errors.
References
- Address performance bottlenecks via batched data fetching at the data layer rather than adding UI state complexity.
antoncoding
changed the title
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/features/market-detail/components/charts/chart-utils.tsx (1)
159-168:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAvoid adding unknown legend keys into state.
onClickstill toggles any stringifieddataKey, so non-member keys can be inserted intovisibleLines. Guard membership before writing state.Suggested fix
onClick: (entry: { dataKey?: unknown }) => { if (entry.dataKey == null) { return; } const dataKey = String(entry.dataKey) as keyof T; - setVisibleLines((prev) => ({ - ...prev, - [dataKey]: !prev[dataKey], - })); + setVisibleLines((prev) => { + if (!(dataKey in prev)) return prev; + return { + ...prev, + [dataKey]: !prev[dataKey], + }; + }); }, formatter: (value: string, entry: { dataKey?: unknown }) => { const dataKey = entry.dataKey == null ? null : (String(entry.dataKey) as keyof T); return ( <span className="text-xs" style={{ - color: dataKey === null || visibleLines[dataKey] ? 'var(--color-text-secondary)' : '`#666`', + color: + dataKey === null || !(dataKey in visibleLines) || visibleLines[dataKey] + ? 'var(--color-text-secondary)' + : '`#666`', }} >Also applies to: 170-178
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/market-detail/components/charts/chart-utils.tsx` around lines 159 - 168, The onClick handler toggles any stringified dataKey into visibleLines, allowing unknown legend keys to be inserted; update the onClick(s) to first verify membership before updating state (e.g., check that dataKey exists in the current visibleLines object or use Object.prototype.hasOwnProperty.call(prev, dataKey) / dataKey in prev) and return early if it isn’t present, then call setVisibleLines to toggle only known keys (apply the same guard to the other occurrence around the setVisibleLines usage).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@src/features/market-detail/components/charts/chart-utils.tsx`:
- Around line 159-168: The onClick handler toggles any stringified dataKey into
visibleLines, allowing unknown legend keys to be inserted; update the onClick(s)
to first verify membership before updating state (e.g., check that dataKey
exists in the current visibleLines object or use
Object.prototype.hasOwnProperty.call(prev, dataKey) / dataKey in prev) and
return early if it isn’t present, then call setVisibleLines to toggle only known
keys (apply the same guard to the other occurrence around the setVisibleLines
usage).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 4264dcc3-4765-4b34-8552-d570352c8b79
📒 Files selected for processing (11)
src/components/common/period-selector.tsxsrc/data-sources/morpho-api/vault-share-price-history.tssrc/features/autovault/components/vault-detail/vault-header.tsxsrc/features/history/components/transaction-history-preview.tsxsrc/features/market-detail/components/charts/chart-utils.tsxsrc/features/position-detail/components/overview-tab.tsxsrc/features/position-detail/position-view.tsxsrc/features/vault/components/vault-adapter-position-overview.tsxsrc/features/vault/components/vault-analytics-period-control.tsxsrc/features/vault/vault-view.tsxsrc/hooks/useVaultSharePriceHistory.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- src/features/position-detail/components/overview-tab.tsx
- src/features/vault/components/vault-adapter-position-overview.tsx
- src/features/history/components/transaction-history-preview.tsx
- src/features/autovault/components/vault-detail/vault-header.tsx
- src/data-sources/morpho-api/vault-share-price-history.ts
- src/hooks/useVaultSharePriceHistory.ts
- src/features/vault/vault-view.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
UX Changes
Documentation