Use completed UTC buckets for position analytics#610
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughCompleted UTC periods now use daily position and market analytics with boundary snapshots, while rolling and custom periods retain bounded event handling. The change adds range utilities, paginated analytics retrieval, daily earnings calculation, historical APY boundaries, and updated chart/table wiring. ChangesCompleted analytics flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PositionView
participant useUserPositionsSummaryData
participant usePositionDailyAnalyticsQuery
participant fetchCompletedPositionDailyAnalytics
participant usePositionsWithEarnings
participant calculateEarningsFromDailyAnalytics
PositionView->>useUserPositionsSummaryData: select completed UTC period
useUserPositionsSummaryData->>usePositionDailyAnalyticsQuery: query positions and range
usePositionDailyAnalyticsQuery->>fetchCompletedPositionDailyAnalytics: fetch chain analytics
fetchCompletedPositionDailyAnalytics-->>usePositionDailyAnalyticsQuery: flows and market snapshots
usePositionDailyAnalyticsQuery-->>useUserPositionsSummaryData: daily analytics by chain
useUserPositionsSummaryData->>usePositionsWithEarnings: provide analytics and boundary snapshots
usePositionsWithEarnings->>calculateEarningsFromDailyAnalytics: calculate weighted earnings
calculateEarningsFromDailyAnalytics-->>usePositionsWithEarnings: earnings calculation
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
4bbedee to
32855b5
Compare
![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 daily analytics calculation mechanism for user positions, fetching daily flows and market snapshots via a new Envio GraphQL query to calculate earnings and APY without relying on heavy transaction history. The UI and hooks have been updated to support completed UTC days analytics periods. The review feedback highlights a potential bug in the daily analytics calculation where unaligned timestamps could break bucket lookups, and identifies type-safety issues where numeric chain IDs are implicitly converted to string keys via Object.fromEntries.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| for (let bucketStart = startTimestamp; bucketStart < endTimestamp; bucketStart += SECONDS_PER_DAY) { | ||
| const bucketEnd = bucketStart + SECONDS_PER_DAY; | ||
| const flow = flowByBucket.get(bucketStart); | ||
| const exposure = getCompletedFlowExposure(flow, currentShares, bucketEnd); | ||
| const endMarketSnapshot = marketSnapshotByBucket.get(bucketStart) ?? latestMarketSnapshot; | ||
| const startMarketSnapshot = latestMarketSnapshot ?? endMarketSnapshot; | ||
|
|
||
| if (exposure.weightedSharesSeconds > 0n) { | ||
| if (startMarketSnapshot && endMarketSnapshot) { | ||
| const startWeightedAssets = toWeightedAssetsSeconds(exposure.weightedSharesSeconds, startMarketSnapshot); | ||
| const endWeightedAssets = toWeightedAssetsSeconds(exposure.weightedSharesSeconds, endMarketSnapshot); | ||
| weightedSuppliedAssets += (startWeightedAssets + endWeightedAssets) / 2n; | ||
| } else { | ||
| weightedSuppliedAssets += getFallbackWeightedAssetsSeconds({ | ||
| weightedSharesSeconds: exposure.weightedSharesSeconds, | ||
| startingBalance, | ||
| startingShares, | ||
| endingBalance, | ||
| endingShares, | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| effectiveTime += exposure.activeSeconds; | ||
| currentShares = exposure.closingShares; | ||
| latestMarketSnapshot = endMarketSnapshot; | ||
| } |
There was a problem hiding this comment.
The daily analytics calculation loops through days using bucketStart += SECONDS_PER_DAY starting from startTimestamp. If startTimestamp or endTimestamp are not perfectly aligned to UTC midnight (e.g., due to block timestamp estimation or clock drift), bucketStart will not match the keys in flowByBucket and marketSnapshotByBucket (which are strictly aligned to UTC midnight multiples of 86400). This will cause the lookups to return undefined and break the calculations.
Aligning startTimestamp and endTimestamp to UTC midnight boundaries inside the function makes the daily bucket lookups robust against unaligned inputs.
const alignedStart = Math.floor(startTimestamp / SECONDS_PER_DAY) * SECONDS_PER_DAY;
const alignedEnd = Math.floor(endTimestamp / SECONDS_PER_DAY) * SECONDS_PER_DAY;
for (let bucketStart = alignedStart; bucketStart < alignedEnd; bucketStart += SECONDS_PER_DAY) {
const bucketEnd = bucketStart + SECONDS_PER_DAY;
const flow = flowByBucket.get(bucketStart);
const exposure = getCompletedFlowExposure(flow, currentShares, bucketEnd);
const endMarketSnapshot = marketSnapshotByBucket.get(bucketStart) ?? latestMarketSnapshot;
const startMarketSnapshot = latestMarketSnapshot ?? endMarketSnapshot;
if (exposure.weightedSharesSeconds > 0n) {
if (startMarketSnapshot && endMarketSnapshot) {
const startWeightedAssets = toWeightedAssetsSeconds(exposure.weightedSharesSeconds, startMarketSnapshot);
const endWeightedAssets = toWeightedAssetsSeconds(exposure.weightedSharesSeconds, endMarketSnapshot);
weightedSuppliedAssets += (startWeightedAssets + endWeightedAssets) / 2n;
} else {
weightedSuppliedAssets += getFallbackWeightedAssetsSeconds({
weightedSharesSeconds: exposure.weightedSharesSeconds,
startingBalance,
startingShares,
endingBalance,
endingShares,
});
}
}
effectiveTime += exposure.activeSeconds;
currentShares = exposure.closingShares;
latestMarketSnapshot = endMarketSnapshot;
}| const endTimestampsByChain = useMemo( | ||
| () => Object.fromEntries(Object.entries(earningsRangesByChain).map(([chainId, range]) => [chainId, range.endTimestamp])), | ||
| [earningsRangesByChain], | ||
| ); |
There was a problem hiding this comment.
Using Object.fromEntries on Object.entries converts the numeric chain IDs into string keys in the resulting object. This can lead to implicit type conversions or TypeScript type-checking issues when passing endTimestampsByChain to functions expecting Record<number, number>. Constructing the record with explicit numeric keys is safer and cleaner.
const endTimestampsByChain = useMemo(() => {
const result: Record<number, number> = {};
for (const [chainId, range] of Object.entries(earningsRangesByChain)) {
result[Number(chainId)] = range.endTimestamp;
}
return result;
}, [earningsRangesByChain]);
| const endTimestampsByChain = useMemo( | ||
| () => Object.fromEntries(Object.entries(earningsRangesByChain).map(([rangeChainId, range]) => [rangeChainId, range.endTimestamp])), | ||
| [earningsRangesByChain], | ||
| ); |
There was a problem hiding this comment.
Using Object.fromEntries on Object.entries converts the numeric chain IDs into string keys in the resulting object. This can lead to implicit type conversions or TypeScript type-checking issues when passing endTimestampsByChain to functions expecting Record<number, number>. Constructing the record with explicit numeric keys is safer and cleaner.
const endTimestampsByChain = useMemo(() => {
const result: Record<number, number> = {};
for (const [chainId, range] of Object.entries(earningsRangesByChain)) {
result[Number(chainId)] = range.endTimestamp;
}
return result;
}, [earningsRangesByChain]);
32855b5 to
f9bfef0
Compare
f9bfef0 to
1241b0d
Compare
1241b0d to
cc3d755
Compare
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: 2
🧹 Nitpick comments (2)
src/hooks/queries/usePositionDailyAnalyticsQuery.ts (1)
35-48: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winOne chain's analytics failure fails the whole query, discarding data for healthy chains.
Promise.allhere means a single chain'sfetchCompletedPositionDailyAnalyticsrejection fails the entire query. Downstream,dailyAnalyticsQuery.isErrorthen causesuseUserPositionsSummaryDatato fall back to raw-transaction earnings for every chain (and dropdailyAnalyticsByChainentirely), even if only one chain's fetch failed.Promise.allSettledwould let healthy chains keep using completed-day analytics while only the failing chain falls back.♻️ Proposed fix
- const entries = await Promise.all( - marketsByChain.map(async ([chainId, marketIds]) => { - const analytics = await fetchCompletedPositionDailyAnalytics({ - userAddress, - chainId, - marketIds, - startTimestamp: range.startTimestamp, - endTimestamp: range.endTimestamp, - }); - return [chainId, analytics] as const; - }), - ); - - return Object.fromEntries(entries); + const results = await Promise.allSettled( + marketsByChain.map(async ([chainId, marketIds]) => { + const analytics = await fetchCompletedPositionDailyAnalytics({ + userAddress, + chainId, + marketIds, + startTimestamp: range.startTimestamp, + endTimestamp: range.endTimestamp, + }); + return [chainId, analytics] as const; + }), + ); + + return Object.fromEntries( + results.filter((r): r is PromiseFulfilledResult<readonly [number, PositionDailyAnalytics]> => r.status === 'fulfilled').map((r) => r.value), + );🤖 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/hooks/queries/usePositionDailyAnalyticsQuery.ts` around lines 35 - 48, Update the analytics aggregation in the query to use Promise.allSettled instead of Promise.all, preserving successful chain results while handling rejected fetchCompletedPositionDailyAnalytics calls as missing analytics for only those chains. Ensure the returned Object.fromEntries input includes successful entries without causing the entire query to error when one chain fails, so downstream fallback remains chain-specific.src/utils/earnings-period.ts (1)
41-52: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse
hourCycle: 'h23'instead ofhour12: false+ string patch.
hour12: falsedoesn't guarantee midnight renders as00:00; depending on locale/runtime it can use theh24cycle and print24:00(confirmed via search: "Using hour12: false should give you 24-hour format... it might default to the h24 cycle (midnight = 24 00)"). The.replace('24:00', '00:00')patch is a fragile workaround for this —hourCycle: 'h23'forces 0–23 formatting directly and removes the need for the string hack.♻️ Proposed fix
const formatUtcTimestamp = (timestamp: number): string => new Intl.DateTimeFormat('en-US', { timeZone: 'UTC', month: 'short', day: 'numeric', year: 'numeric', hour: '2-digit', minute: '2-digit', - hour12: false, + hourCycle: 'h23', }) - .format(new Date(timestamp * 1000)) - .replace('24:00', '00:00'); + .format(new Date(timestamp * 1000));🤖 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/utils/earnings-period.ts` around lines 41 - 52, Update formatUtcTimestamp to use Intl.DateTimeFormat option hourCycle: 'h23' instead of hour12: false, and remove the chained .replace('24:00', '00:00') workaround so midnight is formatted directly as 00:00.
🤖 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/hooks/queries/useBlockTimestamps.ts`:
- Around line 19-32: Remove latestBlocks from the queryKey in the
useBlockTimestamps query while retaining its use inside queryFn for latest-block
validation and findBlockAtTimestamp. Keep snapshotBlocks and targetTimestamp as
the cache key inputs.
In `@src/hooks/useVaultHistoricalApy.ts`:
- Around line 112-121: Update the historical APY processing near the
block/timestamp assignments to return early when requiresHistoricalEnd is true
and historicalEndBlockData is missing. Preserve the existing blockData guard and
fallback end timestamp behavior for cases that do not require a historical end
block.
---
Nitpick comments:
In `@src/hooks/queries/usePositionDailyAnalyticsQuery.ts`:
- Around line 35-48: Update the analytics aggregation in the query to use
Promise.allSettled instead of Promise.all, preserving successful chain results
while handling rejected fetchCompletedPositionDailyAnalytics calls as missing
analytics for only those chains. Ensure the returned Object.fromEntries input
includes successful entries without causing the entire query to error when one
chain fails, so downstream fallback remains chain-specific.
In `@src/utils/earnings-period.ts`:
- Around line 41-52: Update formatUtcTimestamp to use Intl.DateTimeFormat option
hourCycle: 'h23' instead of hour12: false, and remove the chained
.replace('24:00', '00:00') workaround so midnight is formatted directly as
00:00.
🪄 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: 441b5223-a7d5-40a9-9805-bbca366a3d3e
📒 Files selected for processing (23)
docs/TECHNICAL_OVERVIEW.mddocs/VALIDATIONS.mdsrc/data-sources/monarch-api/index.tssrc/data-sources/monarch-api/position-daily-flows.tssrc/features/position-detail/components/history-tab.tsxsrc/features/position-detail/position-view.tsxsrc/features/positions/components/adapter-managed-exposure.tsxsrc/features/positions/components/positions-period-settings.tsxsrc/features/positions/components/supplied-markets-detail.tsxsrc/features/positions/components/supplied-morpho-blue-grouped-table.tsxsrc/features/positions/components/user-positions-chart.tsxsrc/features/positions/positions-view.tsxsrc/features/vault/components/vault-adapter-position-overview.tsxsrc/features/vault/vault-view.tsxsrc/graphql/envio-queries.tssrc/hooks/queries/useBlockTimestamps.tssrc/hooks/queries/usePositionDailyAnalyticsQuery.tssrc/hooks/usePositionsWithEarnings.tssrc/hooks/useUserPositionsSummaryData.tssrc/hooks/useVaultHistoricalApy.tssrc/utils/blockEstimation.tssrc/utils/earnings-period.tssrc/utils/interest.ts
| queryKey: ['block-timestamps', snapshotBlocks, targetTimestamp, latestBlocks], | ||
| queryFn: async () => { | ||
| const blockData: Record<number, { block: number; timestamp: number }> = {}; | ||
|
|
||
| await Promise.all( | ||
| Object.entries(snapshotBlocks).map(async ([chainId, blockNum]) => { | ||
| try { | ||
| const client = getClient(Number(chainId) as SupportedNetworks, customRpcUrls[Number(chainId) as SupportedNetworks]); | ||
| const block = await client.getBlock({ blockNumber: BigInt(blockNum) }); | ||
| const chainIdNumber = Number(chainId) as SupportedNetworks; | ||
| const latestBlock = latestBlocks?.[chainIdNumber]; | ||
| if (latestBlock === undefined) { | ||
| throw new Error(`Missing latest block for chain ${chainId}`); | ||
| } | ||
| const blockDataAtTimestamp = await findBlockAtTimestamp(client, chainIdNumber, blockNum, targetTimestamp, latestBlock); |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how frequently useCurrentBlocks refreshes latestBlocks
rg -n -A 30 'useCurrentBlocks' src/hooks --type=tsRepository: antoncoding/monarch
Length of output: 11267
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== useBlockTimestamps ==\n'
cat -n src/hooks/queries/useBlockTimestamps.ts
printf '\n== findBlockAtTimestamp / estimate helpers ==\n'
rg -n -A 80 -B 20 'function findBlockAtTimestamp|const findBlockAtTimestamp|estimateBlockAtTimestamp|currentBlocks|latestBlocks' src --type=tsRepository: antoncoding/monarch
Length of output: 50375
Drop latestBlocks from the query key.
latestBlocks changes whenever useCurrentBlocks refreshes, so this query gets a new cache entry on each head update even when snapshotBlocks and targetTimestamp stay the same. Keep it in the queryFn, but don’t key the cache on it.
🤖 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/hooks/queries/useBlockTimestamps.ts` around lines 19 - 32, Remove
latestBlocks from the queryKey in the useBlockTimestamps query while retaining
its use inside queryFn for latest-block validation and findBlockAtTimestamp.
Keep snapshotBlocks and targetTimestamp as the cache key inputs.
| const historicalEndBlockData = endBlockData?.[networkId]; | ||
|
|
||
| if (!pastBlock || !blockData) { | ||
| if (!blockData) { | ||
| return; | ||
| } | ||
|
|
||
| const pastBlock = blockData.block; | ||
| const endBlock = historicalEndBlockData?.block; | ||
| const startTimestamp = blockData.timestamp; | ||
| const endTimestamp = historicalEndBlockData?.timestamp ?? fallbackEndTimestamp; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== file list ==\n'
git ls-files 'src/hooks/useVaultHistoricalApy.ts' 'src/**/useBlockTimestamps*' 'src/**/block*timestamp*' 'src/**/historical*' | sed 's#^`#-` #'
printf '\n== outline: src/hooks/useVaultHistoricalApy.ts ==\n'
ast-grep outline src/hooks/useVaultHistoricalApy.ts --view expanded || true
printf '\n== relevant excerpt: src/hooks/useVaultHistoricalApy.ts ==\n'
sed -n '1,220p' src/hooks/useVaultHistoricalApy.ts | cat -n
printf '\n== search for useBlockTimestamps and end-block gating ==\n'
rg -n "useBlockTimestamps|requiresHistoricalEnd|endBlockData|fallbackEndTimestamp|historicalEndBlockData|blockNumber" src -SRepository: antoncoding/monarch
Length of output: 23168
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== outline: src/hooks/queries/useBlockTimestamps.ts ==\n'
ast-grep outline src/hooks/queries/useBlockTimestamps.ts --view expanded || true
printf '\n== excerpt: src/hooks/queries/useBlockTimestamps.ts ==\n'
sed -n '1,220p' src/hooks/queries/useBlockTimestamps.ts | cat -n
printf '\n== excerpt: src/hooks/useUserPositionsSummaryData.ts (end block handling) ==\n'
sed -n '120,290p' src/hooks/useUserPositionsSummaryData.ts | cat -n
printf '\n== search for fallbackEndTimestamp usage ==\n'
rg -n "fallbackEndTimestamp|endBlockData\\?\\.\\[|!!endBlockData|endSnapshotBlocks" src/hooks src/utils -SRepository: antoncoding/monarch
Length of output: 11498
Skip the network when the end block is missing.
endBlockData can exist while this chain is absent, so endBlock becomes undefined and the query falls back to the latest block plus Date.now(). For completed periods, that makes the chain end at the live balance instead of the day boundary. Return early when requiresHistoricalEnd && !historicalEndBlockData.
🤖 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/hooks/useVaultHistoricalApy.ts` around lines 112 - 121, Update the
historical APY processing near the block/timestamp assignments to return early
when requiresHistoricalEnd is true and historicalEndBlockData is missing.
Preserve the existing blockData guard and fallback end timestamp behavior for
cases that do not require a historical end block.
1 participant
Summary
PositionDailyFlowpaginator to fetch sparse user flows and market-day states together, instead of keeping a second analytics pipeline.Cleanup
Verification
npx ultracite fixnpx ultracite checkpnpm checkpnpm build0xfe6509875528e7ea210127fad61800d1fd0d77bd, Optimism): 303 flow rows + 863 market-day rows, one analytics request, ~381 KBSummary by CodeRabbit
New Features
Bug Fixes
Documentation