fix: chart for inactive markets#491
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 7 minutes and 37 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR adds an environment variable to conditionally disable RPC fallback when Morpho API fails, enhances Changes
Sequence Diagram(s)sequenceDiagram
participant Chart as Chart Component
participant Hook as useMarketHistoricalData
participant Monarch as Monarch API
participant Morpho as Morpho API
participant RPC as Custom RPC
participant BlockFetch as Block Timestamp Service
Chart->>Hook: Call with market, timeframe, timeRange
Hook->>Monarch: Fetch historical rates (if network supported)
alt Monarch returns data
Monarch-->>Hook: historicalData
else Morpho fallback enabled
Hook->>Morpho: Fetch rates
Morpho-->>Hook: historicalData
end
alt market & timeframe provided
Hook->>Hook: Generate target timestamps from config
Hook->>BlockFetch: Resolve block numbers for targets
BlockFetch-->>Hook: Block mapping
Hook->>RPC: Batch fetch boundary market states
RPC-->>Hook: Snapshots at blocks
Hook->>Hook: Compute APYs, utilization, rateAtTarget
Hook-->>Hook: stateReadPoints array
end
Hook-->>Chart: { historicalData, stateReadPoints }
Chart->>Chart: Build timestamp lookup from stateReadPoints
Chart->>Chart: Merge blockNumber & isStateRead into chart data
Chart->>Chart: Render with "State Read" badges
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/hooks/useMarketHistoricalData.ts`:
- Around line 256-260: The code currently drops unresolved timestamps by
filtering out nulls from the mapped array (filledPoints), which hides gaps;
instead preserve the full target grid and fail the rebuild when any slots remain
unresolved or boundaries are stale: replace the .map/.filter sequence with const
grid = targetTimestamps.map(t => pointsByTarget.get(t) ?? null) and then if
(grid.some(p => p === null) || !bracketsWindow(grid[0], grid[grid.length-1])) {
throw new Error("Historical samples do not bracket requested window"); } else
set historicalData = buildHistoricalDataFromSamplePoints(grid) (or adapt
buildHistoricalDataFromSamplePoints to accept nullable points) so the chart
keeps nulls or the function fails closed; reference: targetTimestamps,
pointsByTarget, filledPoints, buildHistoricalDataFromSamplePoints,
historicalData.
In `@src/utils/market-rate-enrichment.ts`:
- Around line 550-599: The loop over boundaryBlocks currently runs sequentially
and any thrown error bubbles to the outer try/catch causing an empty [] return;
change this to process boundaryBlocks with bounded parallelism (e.g., use a
concurrency limiter like p-limit or Promise.allSettled with slicing) and wrap
the per-block work (the body that calls fetchMarketsSnapshots,
fetchBoundaryBorrowRates, accrueSnapshotToTimestamp and pushes into
boundaryStates) in its own try/catch so failures for a single boundaryBlock are
logged and skipped rather than aborting the whole function; keep returning only
the successfully constructed HistoricalMarketBoundaryState items (i.e., the
collected boundaryStates) from the surrounding function.
🪄 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: 43c9429b-efeb-44cf-8f0f-13cc9f5566c0
📒 Files selected for processing (7)
.env.local.examplesrc/features/market-detail/components/charts/chart-utils.tsxsrc/features/market-detail/components/charts/rate-chart.tsxsrc/features/market-detail/components/charts/supplier-positions-chart.tsxsrc/features/market-detail/components/charts/volume-chart.tsxsrc/hooks/useMarketHistoricalData.tssrc/utils/market-rate-enrichment.ts
| const filledPoints = targetTimestamps | ||
| .map((targetTimestamp) => pointsByTarget.get(targetTimestamp) ?? null) | ||
| .filter((point): point is HistoricalSamplePoint => point !== null); | ||
|
|
||
| historicalData = filledPoints.length > 0 ? buildHistoricalDataFromSamplePoints(filledPoints) : null; |
There was a problem hiding this comment.
Don't collapse unresolved timestamps out of the rebuilt series.
If some target slots still miss after the RPC fill, filtering them out here makes the chart look continuous anyway—the X values disappear, so the line just interpolates across the hole. Keep the full target grid with null values, or fail the rebuild unless every slot is covered.
As per coding guidelines, "Fail closed when historical rate snapshots do not actually bracket requested window, including stale lastUpdate on either boundary".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/useMarketHistoricalData.ts` around lines 256 - 260, The code
currently drops unresolved timestamps by filtering out nulls from the mapped
array (filledPoints), which hides gaps; instead preserve the full target grid
and fail the rebuild when any slots remain unresolved or boundaries are stale:
replace the .map/.filter sequence with const grid = targetTimestamps.map(t =>
pointsByTarget.get(t) ?? null) and then if (grid.some(p => p === null) ||
!bracketsWindow(grid[0], grid[grid.length-1])) { throw new Error("Historical
samples do not bracket requested window"); } else set historicalData =
buildHistoricalDataFromSamplePoints(grid) (or adapt
buildHistoricalDataFromSamplePoints to accept nullable points) so the chart
keeps nulls or the function fails closed; reference: targetTimestamps,
pointsByTarget, filledPoints, buildHistoricalDataFromSamplePoints,
historicalData.
starksama
left a comment
starksama
left a comment
There was a problem hiding this comment.
LGTM. Main 7d cadence concern is addressed by reverting to the shared 4h interval. Remaining CodeRabbit note looks like optional robustness hardening, not something I’d block this PR on.
2 participants
Summary by CodeRabbit
New Features
Improvements