feat(pricing): add estimated pricing support#283
Open
eliteprox wants to merge 11 commits into
Open
Conversation
This commit introduces a new tasks.json file in the .vscode directory, defining a shell task for building the web-next application using npm. The task is configured to run in the specified working directory and includes presentation options for visibility during execution.
This commit introduces several improvements to the dashboard's KPI data management. Key changes include: - Added `orchestratorsWindowHours` to the KPI data model, reflecting the effective time window based on the oldest `LastSeen` timestamp from the orchestrator registry. - Updated the dashboard page to fetch and display orchestrator metrics, including online counts and window hours, ensuring accurate representation of orchestrator availability. - Enhanced the pricing model to include `ethUsd` for better financial insights in the dashboard. - Refactored the OverviewContent component to utilize the new KPI structure, improving data presentation and user experience. These changes aim to provide more accurate and actionable insights into orchestrator performance and pricing dynamics.
…hance dashboard metrics This commit introduces a new ETH/USD pricing mechanism for the dashboard, utilizing data from public exchanges to improve accuracy. Key changes include: - Added a new route for fetching ETH/USD prices, enhancing the dashboard's financial insights. - Updated existing pricing logic to utilize the new ETH/USD oracle, ensuring consistent pricing across the application. - Enhanced the dashboard to display ETH/USD alongside other pricing metrics, improving user experience and data visibility. - Refactored related components to accommodate the new pricing structure and ensure seamless integration. These updates aim to provide users with more reliable and actionable financial data in the dashboard.
This commit introduces several improvements to the dashboard's tooltip and KPI data handling. Key changes include: - Added a new portaled tooltip for KPI definitions, improving user experience by providing contextual information on hover. - Refactored the `KpiTileTitleWithDefinition` component to utilize the new tooltip functionality, enhancing the presentation of KPI labels and definitions. - Updated the `pipelinePricingEstimate` utility to normalize pipeline IDs for consistent pricing across the application. - Enhanced the `DeveloperView` component with a new tooltip system for displaying detailed pricing information, improving data visibility and user interaction. These updates aim to provide users with clearer insights and a more interactive experience when navigating the dashboard.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
scope/shell
|
|
📝 WalkthroughWalkthroughAdds a server-side ETH/USD oracle and route, pricing utilities, resolver changes to merge net-orchestrator snapshot fields into KPI, dashboard page merging of BFF orchestrator KPI fields, portaled definition tooltips and enriched pricing UI in OverviewContent and DeveloperView, and related contract/stub/timeframe updates. ChangesDashboard & pricing feature
Sequence DiagramsequenceDiagram
participant Client as Dashboard UI
participant Page as Dashboard Page
participant BFF as BFF Endpoint
participant API as eth-usd Route
participant Oracle as getEthUsdOracle
participant Exchange as Public Exchange (Binance/Kraken)
participant Cache as In-Process Cache
Client->>Page: Load dashboard
Page->>BFF: fetch KPI (and orchestrator snapshot)
Page->>API: fetch ETH/USD
API->>Oracle: getEthUsdOracle()
Oracle->>Cache: check ethUsdOracle:v1
alt cache miss
Oracle->>Exchange: fetch Binance -> fallback Kraken
Exchange-->>Oracle: price|null
Oracle->>Cache: store price (5min)
end
Oracle-->>API: ethUsd
API-->>Page: { ethUsd }
BFF-->>Page: { kpi, orchestrator snapshot }
Page->>Page: merge KPI orchestrator fields
Page-->>Client: merged KPI + ethUsd
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 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)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/web-next/src/app/api/v1/wallet/prices/route.ts (1)
44-55:⚠️ Potential issue | 🟠 MajorDo not persist
0prices when upstream spot fetch returns null/invalid values.Right now null/invalid exchange responses are normalized to
0and then written to cache, which can overwrite good data and serve false zero prices until cache refresh.Proposed fix
- const lptUsd = lptUsdRaw != null && Number.isFinite(lptUsdRaw) && lptUsdRaw > 0 ? lptUsdRaw : 0; - const ethUsd = ethUsdRaw != null && Number.isFinite(ethUsdRaw) && ethUsdRaw > 0 ? ethUsdRaw : 0; + const lptUsd = lptUsdRaw != null && Number.isFinite(lptUsdRaw) && lptUsdRaw > 0 ? lptUsdRaw : null; + const ethUsd = ethUsdRaw != null && Number.isFinite(ethUsdRaw) && ethUsdRaw > 0 ? ethUsdRaw : null; const now = new Date(); + if (lptUsd == null || ethUsd == null) { + throw new Error('Invalid spot price(s) from upstream exchanges'); + } + await Promise.all([ prisma.walletPriceCache.create({ data: { symbol: 'LPT', priceUsd: lptUsd, fetchedAt: now } }), prisma.walletPriceCache.create({ data: { symbol: 'ETH', priceUsd: ethUsd, fetchedAt: now } }), ]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web-next/src/app/api/v1/wallet/prices/route.ts` around lines 44 - 55, The code currently normalizes null/invalid fetch results to 0 (lptUsd/ethUsd) and then writes those zeros into the cache; change this so you only persist when the raw values are valid positive finite numbers: after calling fetchLptUsdFromPublicExchanges() and fetchEthUsdFromPublicExchanges(), validate lptUsdRaw and ethUsdRaw (same checks currently used) and only call prisma.walletPriceCache.create for each symbol when its raw value passes validation (do not create entries with priceUsd = 0); you can conditionally build an array of create promises (or push per-symbol creates) and await Promise.all on that array so good cached prices aren’t overwritten by invalid upstream responses.apps/web-next/src/lib/facade/resolvers/pricing.ts (1)
208-224:⚠️ Potential issue | 🟠 MajorDon't cache
outputPerDollarbehind the full pricing TTL.
cachedFetch('facade:pricing', TTL.PRICING, ...)now memoizes the ETH/USD-converted strings together with the pricing rows, so the new 5-minute oracle is only consulted on a pricing-cache miss. In practiceoutputPerDollarcan stay stale for the entire pricing TTL and drift from the dedicated ETH/USD route. Cache the raw pricing rows separately, then apply the current ETH/USD conversion outside this cache, or give the decorated result a much shorter TTL.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web-next/src/lib/facade/resolvers/pricing.ts` around lines 208 - 224, The cachedFetch in resolvePricing currently stores ETH-converted fields (outputPerDollar) so the 5-minute getEthUsdOracle updates are ignored until the pricing TTL expires; change resolvePricing to cache only the raw pricing rows (the naapGet('dashboard/pricing') result and merged netCapacity/netModels) under 'facade:pricing' and then, after retrieving that cached raw data, call getEthUsdOracle() and compute/decorate outputPerDollar on-the-fly (or cache that decorated result with a much shorter TTL) before returning; update references to resolvePricing, cachedFetch('facade:pricing'), getEthUsdOracle, and the code path that computes outputPerDollar so the ETH/USD conversion is applied outside the long-lived cache.apps/web-next/src/lib/facade/resolvers/net-orchestrators.ts (1)
242-255:⚠️ Potential issue | 🟠 MajorIgnore blank-URI rows when deriving the snapshot window.
listedCountonly counts non-blank(address, URI)pairs, but this code returnslastSeenMseven for rows whoseuriis blank. That means an unlisted stale row can stretcholdestLastSeenMs, soorchestratorsWindowHoursno longer describes the same snapshot asorchestratorsObserved.Suggested fix
const ls = parseLastSeenMs(r.last_seen); if (ls !== undefined) { const prev = lastSeenMsByAddress.get(addr); if (prev === undefined || ls > prev) { lastSeenMsByAddress.set(addr, ls); } if (uri.length > 0) { const pairKey = orchestratorPairKey(addr, uri); const prevPair = lastSeenMsByPair.get(pairKey); if (prevPair === undefined || ls > prevPair) { lastSeenMsByPair.set(pairKey, ls); } + return { lastSeenMs: ls }; } - return { lastSeenMs: ls }; } return {};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web-next/src/lib/facade/resolvers/net-orchestrators.ts` around lines 242 - 255, The code currently uses parseLastSeenMs(r.last_seen) for every row and can return lastSeenMs even when uri is blank, letting unlisted rows extend the snapshot; change the logic so that rows with an empty uri are ignored when deriving the snapshot window: only parse and consider lastSeenMs and update lastSeenMsByAddress and lastSeenMsByPair (using orchestratorPairKey) when uri.length > 0, so listedCount, orchestratorsObserved and oldestLastSeenMs/orchestratorsWindowHours are computed from the same non-blank (address, URI) pairs.
🧹 Nitpick comments (2)
.vscode/tasks.json (1)
5-20: Optional: usecommand+args(ortype: "npm") for better portability.Right now it’s a single shell string (
"npm run build"). You could optionally switch to a structured form to reduce shell-quoting differences across environments, e.g.command: "npm", args: ["run","build"], or use VS Code’s built-intype: "npm".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.vscode/tasks.json around lines 5 - 20, The task entry labeled "NAAP: web-next build" currently uses a single shell string in "command": "npm run build" which can cause portability/quoting issues; update this task to use either a structured form with "command": "npm" and "args": ["run","build"] or change "type" to "npm" and move the script into the appropriate npm fields, keeping the existing "options.cwd" for "${workspaceFolder}/apps/web-next" so the task name/label and working directory remain unchanged.apps/web-next/src/lib/dashboard/overview-timeframe.ts (1)
29-31: Avoid rounding non-24h windows down/up to an imprecise whole-day label.
Math.round(hours / 24)makes25hshow as1d(and36has2d), which can misrepresent the actual timeframe. Consider preserving hour precision or using a 1-decimal day label.Proposed tweak
export function formatOverviewTimeframeLabel(hours: number): string { if (hours === 1) return '1h'; if (hours < 24) return `${hours}h`; if (hours % 24 === 0) return `${hours / 24}d`; - return `${Math.round(hours / 24)}d`; + const days = (hours / 24).toFixed(1).replace(/\.0$/, ''); + return `${days}d`; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web-next/src/lib/dashboard/overview-timeframe.ts` around lines 29 - 31, The current logic uses Math.round(hours / 24) (when hours >= 24 and not an exact multiple of 24) which collapses 25h -> 1d and 36h -> 2d; change the fallback so multi-day windows keep precision instead of rounding to the nearest whole day — e.g., format days as a single-decimal label or preserve hours for non-whole-day spans: replace the Math.round(hours / 24) branch with a days calculation using (hours / 24).toFixed(1) (and trim trailing “.0” if present) or return the hour-based label for >24h, referencing the existing hours variable and the Math.round(hours / 24) expression to locate the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web-next/src/app/api/v1/wallet/jobs/prices/route.ts`:
- Around line 20-30: The code currently normalizes failed fetches to 0 and
always writes them to the DB; change it so after calling
fetchLptUsdFromPublicExchanges and fetchEthUsdFromPublicExchanges and computing
lptUsd and ethUsd, you only call prisma.walletPriceCache.create for each symbol
when the corresponding value is a valid positive finite number (> 0). Keep the
existing normalization to 0 for responses but prevent persisting zeros by
guarding the prisma.walletPriceCache.create calls (symbols: LPT, ETH; variables:
lptUsd, ethUsd; timestamp: now).
In `@apps/web-next/src/components/dashboard/overview-content.tsx`:
- Around line 549-553: The span elements that spread triggerProps from
useOverviewPortaledDefinitionTooltip (e.g., the element with {...triggerProps}
and aria-label={definition}) are not keyboard-focusable so onFocus/onBlur never
fire; update those elements (both occurrences, including lines ~1862-1865) to be
keyboard-accessible by either converting the span to a semantic button or adding
tabIndex={0} plus visible focus styles and appropriate role/aria attributes so
triggerProps' onFocus/onBlur work as intended.
- Around line 184-199: The tooltip rendering inside the table cell is currently
an inline absolutely-positioned element (the div under the "group/cell" wrapper)
which gets clipped by the table's overflow; replace that inline block with the
same portaled tooltip used for the definition tooltips so the popup is rendered
into a portal/outside the overflow container. Concretely: in the component that
returns the td (the element using tdClass, tip, main and richLines), remove the
absolute bottom-full div and instead render the portaled Tooltip/Portal
component used elsewhere in the codebase (the one used for the "definition"
tooltips), move the richLines!.map(...) content into that portaled tooltip, keep
the hover trigger tied to the "group/cell" span so the title fallback and hover
behavior remain, and preserve styling/spacing inside the portaled content so the
tooltip appearance matches the original.
In `@apps/web-next/src/lib/facade/resolvers/kpi.ts`:
- Around line 85-90: When overriding kpi.orchestratorsObserved.value with the
snapshot count (inside the hasNetRegistrySnapshot branch), also update its delta
so it matches the same snapshot source; either compute the delta from the
snapshot data if available (e.g. use a snapshot-derived delta field on netData)
or explicitly clear/reset the prior upstream delta so it cannot display a
mismatched change. Locate the assignment to kpi.orchestratorsObserved and
replace the spread with something like {...kpi.orchestratorsObserved, value:
netData.listedCount, delta: <snapshot-derived-delta> || undefined} and keep the
existing call to orchestratorSnapshotWindowHours(netData) for
orchestratorsWindowHours. Ensure the chosen fix keeps the KPI tile showing a
coherent value+delta pair.
In `@apps/web-next/src/lib/prices/public-exchange-spot.ts`:
- Around line 12-14: All four exchange fetches should use an explicit 3000ms
AbortSignal timeout to avoid hanging: add signal: AbortSignal.timeout(3000) to
the fetch options for the Binance ticker call, the Kraken ticker call, the
Binance 24h stats call, and the Binance klines call in public-exchange-spot.ts;
ensure you pass the signal in the same options object where Accept header is set
(and for any other fetch options) so fetch will abort after 3000ms.
In `@packages/utils/src/pipelinePricingEstimate.ts`:
- Around line 94-100: The tooltip text currently hardcodes "cached oracle"
inside the pipeline pricing helper (see the return that builds richLines using
formatUsdPipelineEstimate and variables like weiLabel, fps, ethUsd), which is
incorrect because callers may have fallen back before invoking this helper;
either remove the provenance phrase from the strings here or add an explicit
provenance parameter (e.g., ethUsdSource or ethUsdProvenance) to the helper
signature and use that to render the third richLine (instead of the hardcoded
"cached oracle"), ensuring callers pass the real source when available.
In `@plugins/developer-api/frontend/src/pages/DeveloperView.tsx`:
- Around line 428-433: The tooltip trigger uses triggerProps (which supplies
onFocus/onBlur) but is rendered as a non-focusable div, preventing keyboard
users from opening it; update the element containing {...triggerProps} and the
displayed avg.main to be focusable—either replace the div with a semantic
<button> (or a <span> with tabIndex={0} and appropriate role/aria attributes) so
the onFocus/onBlur handlers work, preserve the className "inline-flex
max-w-full" and the inner span with the border styles, and ensure keyboard
activation/appearance remains identical to the original visual behavior.
---
Outside diff comments:
In `@apps/web-next/src/app/api/v1/wallet/prices/route.ts`:
- Around line 44-55: The code currently normalizes null/invalid fetch results to
0 (lptUsd/ethUsd) and then writes those zeros into the cache; change this so you
only persist when the raw values are valid positive finite numbers: after
calling fetchLptUsdFromPublicExchanges() and fetchEthUsdFromPublicExchanges(),
validate lptUsdRaw and ethUsdRaw (same checks currently used) and only call
prisma.walletPriceCache.create for each symbol when its raw value passes
validation (do not create entries with priceUsd = 0); you can conditionally
build an array of create promises (or push per-symbol creates) and await
Promise.all on that array so good cached prices aren’t overwritten by invalid
upstream responses.
In `@apps/web-next/src/lib/facade/resolvers/net-orchestrators.ts`:
- Around line 242-255: The code currently uses parseLastSeenMs(r.last_seen) for
every row and can return lastSeenMs even when uri is blank, letting unlisted
rows extend the snapshot; change the logic so that rows with an empty uri are
ignored when deriving the snapshot window: only parse and consider lastSeenMs
and update lastSeenMsByAddress and lastSeenMsByPair (using orchestratorPairKey)
when uri.length > 0, so listedCount, orchestratorsObserved and
oldestLastSeenMs/orchestratorsWindowHours are computed from the same non-blank
(address, URI) pairs.
In `@apps/web-next/src/lib/facade/resolvers/pricing.ts`:
- Around line 208-224: The cachedFetch in resolvePricing currently stores
ETH-converted fields (outputPerDollar) so the 5-minute getEthUsdOracle updates
are ignored until the pricing TTL expires; change resolvePricing to cache only
the raw pricing rows (the naapGet('dashboard/pricing') result and merged
netCapacity/netModels) under 'facade:pricing' and then, after retrieving that
cached raw data, call getEthUsdOracle() and compute/decorate outputPerDollar
on-the-fly (or cache that decorated result with a much shorter TTL) before
returning; update references to resolvePricing, cachedFetch('facade:pricing'),
getEthUsdOracle, and the code path that computes outputPerDollar so the ETH/USD
conversion is applied outside the long-lived cache.
---
Nitpick comments:
In @.vscode/tasks.json:
- Around line 5-20: The task entry labeled "NAAP: web-next build" currently uses
a single shell string in "command": "npm run build" which can cause
portability/quoting issues; update this task to use either a structured form
with "command": "npm" and "args": ["run","build"] or change "type" to "npm" and
move the script into the appropriate npm fields, keeping the existing
"options.cwd" for "${workspaceFolder}/apps/web-next" so the task name/label and
working directory remain unchanged.
In `@apps/web-next/src/lib/dashboard/overview-timeframe.ts`:
- Around line 29-31: The current logic uses Math.round(hours / 24) (when hours
>= 24 and not an exact multiple of 24) which collapses 25h -> 1d and 36h -> 2d;
change the fallback so multi-day windows keep precision instead of rounding to
the nearest whole day — e.g., format days as a single-decimal label or preserve
hours for non-whole-day spans: replace the Math.round(hours / 24) branch with a
days calculation using (hours / 24).toFixed(1) (and trim trailing “.0” if
present) or return the hour-based label for >24h, referencing the existing hours
variable and the Math.round(hours / 24) expression to locate the change.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dc79481e-4bb2-4eb3-8cb5-ea5f466ef376
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json,!package-lock.json
📒 Files selected for processing (22)
.vscode/tasks.jsonapps/web-next/src/app/(dashboard)/dashboard/page.tsxapps/web-next/src/app/api/internal/bff-warm/route.tsapps/web-next/src/app/api/v1/dashboard/eth-usd/route.tsapps/web-next/src/app/api/v1/wallet/jobs/prices/route.tsapps/web-next/src/app/api/v1/wallet/network/overview/route.tsapps/web-next/src/app/api/v1/wallet/prices/route.tsapps/web-next/src/components/dashboard/overview-content.tsxapps/web-next/src/lib/dashboard/overview-timeframe.tsapps/web-next/src/lib/facade/resolvers/kpi.tsapps/web-next/src/lib/facade/resolvers/net-orchestrators.tsapps/web-next/src/lib/facade/resolvers/pricing.tsapps/web-next/src/lib/facade/stubs.tsapps/web-next/src/lib/prices/eth-usd-oracle.tsapps/web-next/src/lib/prices/public-exchange-spot.tsapps/web-next/src/lib/wallet/subgraph.tspackages/plugin-sdk/src/contracts/dashboard.tspackages/utils/src/index.tspackages/utils/src/pipelinePricingEstimate.tsplugins/dashboard-data-provider/frontend/src/__tests__/provider.test.tsplugins/developer-api/backend/src/server.tsplugins/developer-api/frontend/src/pages/DeveloperView.tsx
This commit updates the wallet price caching logic across multiple routes and services to use the `upsert` method instead of `create`. Key changes include: - Modified the `GET` methods in the wallet price API routes to upsert prices for 'LPT' and 'ETH', ensuring that existing entries are updated rather than duplicated. - Updated the `persistEthUsd` function to utilize upsert for caching ETH prices. - Adjusted the database schema to enforce uniqueness on the combination of `symbol` and `fetchedAt`, replacing the previous index with a unique constraint. These changes enhance data integrity and prevent duplicate entries in the wallet price cache.
|
🗃️ Database Migration Detected This PR includes changes to Prisma schema files. Please ensure:
Requesting review from the core team: @livepeer/core |
…ractions This commit refines the wallet price caching logic by implementing conditional upserts for 'LPT' and 'ETH' prices, ensuring that only valid prices are persisted. Additionally, it enhances the dashboard's tooltip functionality by integrating new portaled tooltips for better user experience and clarity in KPI data presentation. Key changes include: - Conditional upsert logic in the wallet price API routes to prevent unnecessary database operations. - Improved tooltip interactions in the dashboard components, providing users with contextual information on hover. - Updated utility functions to support the new tooltip features and ensure consistent data handling. These enhancements aim to improve data integrity and user engagement within the application.
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
plugins/developer-api/frontend/src/pages/DeveloperView.tsx (1)
256-333: Extract the portaled-tooltip primitive into shared UI code.This hook/panel pair is now effectively duplicated with
apps/web-next/src/components/dashboard/overview-content.tsx:482-548, including the anchor math and window listeners. Pulling it into a shared helper/component will keep tooltip behavior and future fixes from drifting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/developer-api/frontend/src/pages/DeveloperView.tsx` around lines 256 - 333, Extract the portaled tooltip primitive into a shared UI module by moving usePortaledTooltip and NetworkModelTooltipPanel out of DeveloperView.tsx and into a new reusable file (e.g., a shared hooks/components module), then import and use them from DeveloperView and the duplicate in apps/web-next; ensure you preserve the API: usePortaledTooltip<T>() returns { tipAnchor, tipOpen, triggerProps } and NetworkModelTooltipPanel accepts props { anchor: TooltipAnchor, children }; keep the anchor math and event listeners (syncTipAnchor, useLayoutEffect/useEffect) unchanged and update both call sites to use the shared symbols so behavior and positioning remain identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web-next/src/app/api/v1/wallet/jobs/prices/route.ts`:
- Around line 31-35: The upsert calls to prisma.walletPriceCache (e.g.,
prisma.walletPriceCache.upsert for LPT) are missing the source field so rows
default to "coingecko"; add source to both create and update objects and set it
to the actual pricing source used when fetching (e.g., the variable that holds
"binance" or "kraken" in your fetch logic—replace with the correct name from
scope), and make the identical change for the other upsert block referenced (the
one around lines 40-44) so provenance is persisted for both create and update
paths.
- Around line 20-25: The code currently masks a total outage by turning
null/invalid returns from fetchLptUsdFromPublicExchanges and
fetchEthUsdFromPublicExchanges into zeros; change this so the job fails when
both upstream lookups miss: after awaiting the two helpers and computing
lptUsdRaw/ethUsdRaw (and the sanitized lptUsd/ethUsd), add a guard that detects
when both raw values are null or both sanitized values are <= 0 and throw an
Error (including the raw values and a short message) instead of returning {
lptUsd: 0, ethUsd: 0 } so the cron will surface the failure. Ensure the check
references fetchLptUsdFromPublicExchanges, fetchEthUsdFromPublicExchanges,
lptUsdRaw, ethUsdRaw, lptUsd and ethUsd to locate and implement the change.
In `@apps/web-next/src/app/api/v1/wallet/prices/route.ts`:
- Around line 55-59: The upsert calls to prisma.walletPriceCache.upsert for
symbols like 'LPT' (and the similar upserts around lines 64-68) rely on the
schema default source="coingecko" and thus record the wrong source; update each
upsert (both create and update objects) to explicitly set a non-CoinGecko source
(e.g. source: "public_exchange" or "exchange") so new rows and updates correctly
reflect the public exchange source; locate the prisma.walletPriceCache.upsert
calls and add source: "public_exchange" to the create and update payloads for
each symbol.
- Around line 44-49: The current logic converts null failures from
fetchLptUsdFromPublicExchanges and fetchEthUsdFromPublicExchanges into 0
immediately (via lptUsdRaw -> lptUsd and ethUsdRaw -> ethUsd) which prevents the
historical-price fallback from ever running; change the flow so that when
lptUsdRaw or ethUsdRaw is null/invalid you call the existing historical-price
fallback (the fallback query used elsewhere for cached/last-known prices) and
only set lptUsd or ethUsd to 0 if both the public-exchange result and the
historical fallback fail; update the handling around lptUsdRaw/ethUsdRaw and the
assignment to lptUsd/ethUsd (preserving the positive-finite check for live
results) so outages fall through to the fallback instead of becoming 0
immediately.
In `@apps/web-next/src/lib/facade/resolvers/kpi.ts`:
- Around line 81-90: The current guard (hasNetRegistrySnapshot) can be true due
to non-empty activeCount or urisByAddress.size even when no real (address,URI)
pairs exist, causing kpi.orchestratorsObserved to be overwritten with a zero
listedCount; update the condition so you only override kpi.orchestratorsObserved
when there are actual pairs by checking netData.listedCount > 0 (or
netData.lastSeenMsByPair.size > 0) before assigning to
kpi.orchestratorsObserved.value/delta, leaving the upstream KPI intact
otherwise.
In `@apps/web-next/src/lib/facade/resolvers/net-orchestrators.ts`:
- Around line 242-255: Move the parseLastSeenMs call so you parse r.last_seen
before checking uri and, when parseLastSeenMs(r.last_seen) returns a value,
always update lastSeenMsByAddress with orchestrator address (use
lastSeenMsByAddress.set(addr, ls) if ls is newer) and return { lastSeenMs: ls };
only update lastSeenMsByPair and use orchestratorPairKey(addr, uri) when uri is
non-blank (and then apply the same newer-than check before setting). This
ensures oldestLastSeenMs/orchestratorsWindowHours sees valid last_seen from rows
with blank URI while preserving pair-level updates when a URI exists.
In `@apps/web-next/src/lib/prices/public-exchange-spot.ts`:
- Around line 94-97: The function fetchLptUsdDailyCloseChart doesn't guard
against days being NaN, causing limit to become NaN and returning an empty
chart; before computing limit (the Math.min/Math.max/Math.ceil expression)
validate and sanitize days (e.g., if !Number.isFinite(days) or days <= 0) set a
sensible default like 1, then apply the existing clamping logic to produce
limit; update the code around the days/limit calculation in
fetchLptUsdDailyCloseChart to use the sanitizedDays variable.
---
Nitpick comments:
In `@plugins/developer-api/frontend/src/pages/DeveloperView.tsx`:
- Around line 256-333: Extract the portaled tooltip primitive into a shared UI
module by moving usePortaledTooltip and NetworkModelTooltipPanel out of
DeveloperView.tsx and into a new reusable file (e.g., a shared hooks/components
module), then import and use them from DeveloperView and the duplicate in
apps/web-next; ensure you preserve the API: usePortaledTooltip<T>() returns {
tipAnchor, tipOpen, triggerProps } and NetworkModelTooltipPanel accepts props {
anchor: TooltipAnchor, children }; keep the anchor math and event listeners
(syncTipAnchor, useLayoutEffect/useEffect) unchanged and update both call sites
to use the shared symbols so behavior and positioning remain identical.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0540e20e-2905-40ad-8987-97863444acbf
📒 Files selected for processing (14)
apps/web-next/src/app/api/v1/wallet/jobs/prices/route.tsapps/web-next/src/app/api/v1/wallet/prices/route.tsapps/web-next/src/components/dashboard/overview-content.tsxapps/web-next/src/lib/dashboard/overview-timeframe.tsapps/web-next/src/lib/facade/resolvers/kpi.tsapps/web-next/src/lib/facade/resolvers/net-orchestrators.tsapps/web-next/src/lib/facade/resolvers/pricing.tsapps/web-next/src/lib/prices/eth-usd-oracle.tsapps/web-next/src/lib/prices/public-exchange-spot.tsexamples/my-wallet/backend/src/lib/priceService.tspackages/database/prisma/migrations/20260429120000_wallet_price_cache_symbol_fetched_at_unique/migration.sqlpackages/database/prisma/schema.prismapackages/utils/src/pipelinePricingEstimate.tsplugins/developer-api/frontend/src/pages/DeveloperView.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/web-next/src/lib/dashboard/overview-timeframe.ts
- apps/web-next/src/lib/prices/eth-usd-oracle.ts
- apps/web-next/src/lib/facade/resolvers/pricing.ts
This commit improves the wallet price fetching and caching mechanisms by introducing source tracking for prices from public exchanges. Key changes include: - Updated the price fetching functions to return both price and source information for 'LPT' and 'ETH'. - Refined the caching logic to ensure that only valid prices are persisted, utilizing conditional upserts. - Enhanced error handling for price fetching failures, providing clearer logging and fallback mechanisms. These updates aim to improve data accuracy and reliability in the wallet price API, enhancing the overall user experience.
… public exchange dependencies This commit refactors the wallet price fetching logic to utilize CoinGecko as the primary source for ETH and LPT prices, enhancing data accuracy and reliability. Key changes include: - Removed the public exchange price fetching functions and related logic. - Updated API routes to fetch prices directly from CoinGecko, simplifying the codebase. - Adjusted caching mechanisms to store prices fetched from CoinGecko. - Enhanced error handling and fallback mechanisms for price fetching. These updates aim to streamline the pricing process and improve the overall user experience in the wallet application.
…rendering in dashboard components This commit introduces a new utility function, `splitPriceDisplay`, which separates formatted price strings into amount and unit parts for improved styling. Key changes include: - Added `splitPriceDisplay` function to the `pipelinePricingEstimate` utility for better price formatting. - Updated the `PipelineTablePriceCell` and `DeveloperModelCombinedPriceCell` components to utilize the new utility, enhancing the display of pricing information. - Refactored button and tooltip interactions in the dashboard to improve user experience and clarity in price presentation. These updates aim to provide a more consistent and visually appealing representation of pricing data across the application.
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 `@apps/web-next/src/lib/facade/resolvers/kpi.ts`:
- Around line 81-89: Compute const window =
orchestratorSnapshotWindowHours(netData) and only override
kpi.orchestratorsObserved.value/delta when window !== null; if window is null,
do not replace the upstream KPI values and instead set an explicit snapshot
signal (e.g., set kpi.orchestratorsWindowHours = 'snapshot' or add
kpi.orchestratorsObservedSource = 'snapshot') so the UI can detect
snapshot-without-timeframe rather than falling back to the selected timeframe;
update the branch that currently sets kpi.orchestratorsObserved and
kpi.orchestratorsWindowHours to use this conditional logic around
orchestratorSnapshotWindowHours(netData).
In `@plugins/developer-api/frontend/src/pages/DeveloperView.tsx`:
- Around line 648-666: The code clears networkModelsEthUsdFromOracle up front
but only re-sets it if ethRes yields a valid number, causing the UI badge to
disagree with the retained numeric rate; fix by either (A) moving the call to
setNetworkModelsEthUsdFromOracle(false) into the failure branch so you only
clear the oracle flag when you also replace the rate, or (B) if you want to
clear immediately, also set the numeric fallback via
setNetworkModelsEthUsd(fallbackValue) inside the catch/failure path so the
numeric value and the oracle flag remain consistent; update the logic around
ethRes handling (the Promise.allSettled result and the setNetworkModelsEthUsd /
setNetworkModelsEthUsdFromOracle calls) to implement one of these two
approaches.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e2e9820e-f0e8-476b-a856-1f09f2dfb1c0
📒 Files selected for processing (7)
apps/web-next/src/components/dashboard/overview-content.tsxapps/web-next/src/lib/facade/resolvers/kpi.tsapps/web-next/src/lib/facade/resolvers/net-orchestrators.tsapps/web-next/src/lib/prices/eth-usd-oracle.tsapps/web-next/src/lib/prices/public-exchange-spot.tspackages/utils/src/pipelinePricingEstimate.tsplugins/developer-api/frontend/src/pages/DeveloperView.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web-next/src/lib/prices/public-exchange-spot.ts
…dSource This commit introduces the `orchestratorsObservedSource` field to the KPI data model, indicating when the data is sourced from the registry snapshot. Key changes include: - Updated the `DashboardKPI` interface to include `orchestratorsObservedSource`. - Modified the dashboard page to fetch and set the new field based on the data source. - Enhanced the `KPIGroupCard` component to display appropriate labels and tooltips based on the `orchestratorsObservedSource`. These updates aim to improve clarity and accuracy in the presentation of orchestrator metrics on the dashboard.
|
This PR has been inactive for 7 days. It will be closed in 5 days if no activity occurs. Feel free to reopen anytime. |
|
Closed due to inactivity. Feel free to reopen if you'd like to continue this work. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@apps/web-next/src/lib/facade/resolvers/kpi.ts`:
- Around line 85-94: The code sets kpi.orchestratorsObservedSource = 'snapshot'
unconditionally which mislabels data when
orchestratorSnapshotWindowHours(netData) returns null; change the logic so the
source is only set to 'snapshot' when window !== null (i.e., move the
kpi.orchestratorsObservedSource assignment inside the if block that updates
kpi.orchestratorsObserved and kpi.orchestratorsWindowHours), or explicitly set
kpi.orchestratorsObservedSource to the correct alternative (e.g., 'upstream' or
null) when window is null; update references to orchestratorSnapshotWindowHours,
kpi.orchestratorsObserved, kpi.orchestratorsObservedSource,
kpi.orchestratorsWindowHours and netData.listedCount accordingly.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9fbe9cd4-d3ba-4888-97df-bc3cf4ab6f59
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json,!package-lock.json
📒 Files selected for processing (6)
apps/web-next/src/app/(dashboard)/dashboard/page.tsxapps/web-next/src/app/api/internal/bff-warm/route.tsapps/web-next/src/components/dashboard/overview-content.tsxapps/web-next/src/lib/facade/resolvers/kpi.tspackages/plugin-sdk/src/contracts/dashboard.tsplugins/developer-api/frontend/src/pages/DeveloperView.tsx
Comment on lines
+85
to
+94
| const window = orchestratorSnapshotWindowHours(netData); | ||
| kpi.orchestratorsObservedSource = 'snapshot'; | ||
| if (window !== null) { | ||
| kpi.orchestratorsObserved = { | ||
| ...kpi.orchestratorsObserved, | ||
| value: netData.listedCount, | ||
| delta: 0, | ||
| }; | ||
| kpi.orchestratorsWindowHours = window; | ||
| } |
There was a problem hiding this comment.
Align orchestratorsObservedSource with the actual value source.
On Line 86, orchestratorsObservedSource is set to 'snapshot' even when Line 87 skips overriding orchestratorsObserved (window is null). That can label upstream data as snapshot-derived.
Suggested fix
if (hasNetRegistrySnapshot) {
const window = orchestratorSnapshotWindowHours(netData);
+ kpi.orchestratorsObserved = {
+ ...kpi.orchestratorsObserved,
+ value: netData.listedCount,
+ delta: 0,
+ };
kpi.orchestratorsObservedSource = 'snapshot';
if (window !== null) {
- kpi.orchestratorsObserved = {
- ...kpi.orchestratorsObserved,
- value: netData.listedCount,
- delta: 0,
- };
kpi.orchestratorsWindowHours = window;
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const window = orchestratorSnapshotWindowHours(netData); | |
| kpi.orchestratorsObservedSource = 'snapshot'; | |
| if (window !== null) { | |
| kpi.orchestratorsObserved = { | |
| ...kpi.orchestratorsObserved, | |
| value: netData.listedCount, | |
| delta: 0, | |
| }; | |
| kpi.orchestratorsWindowHours = window; | |
| } | |
| const window = orchestratorSnapshotWindowHours(netData); | |
| if (window !== null) { | |
| kpi.orchestratorsObserved = { | |
| ...kpi.orchestratorsObserved, | |
| value: netData.listedCount, | |
| delta: 0, | |
| }; | |
| kpi.orchestratorsObservedSource = 'snapshot'; | |
| kpi.orchestratorsWindowHours = window; | |
| } |
🤖 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 `@apps/web-next/src/lib/facade/resolvers/kpi.ts` around lines 85 - 94, The code
sets kpi.orchestratorsObservedSource = 'snapshot' unconditionally which
mislabels data when orchestratorSnapshotWindowHours(netData) returns null;
change the logic so the source is only set to 'snapshot' when window !== null
(i.e., move the kpi.orchestratorsObservedSource assignment inside the if block
that updates kpi.orchestratorsObserved and kpi.orchestratorsWindowHours), or
explicitly set kpi.orchestratorsObservedSource to the correct alternative (e.g.,
'upstream' or null) when window is null; update references to
orchestratorSnapshotWindowHours, kpi.orchestratorsObserved,
kpi.orchestratorsObservedSource, kpi.orchestratorsWindowHours and
netData.listedCount accordingly.
|
This PR has conflicts with the base branch. Please rebase to resolve them: git fetch origin
git rebase origin/main
# resolve conflicts, then:
git push --force-with-leaseThe |
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
Introduce estimated pricing functionality to provide approximate cost calculations for NAAP workflows. This enables users to preview pricing before execution and improves transparency around resource usage.
Changes
Motivation
Users currently lack visibility into expected costs prior to execution. This feature improves UX by surfacing estimated pricing early in the workflow.
Notes
Testing
Future Work
Summary by CodeRabbit
Release Notes
New Features
Chores