feat(metrics): Integrate Leaderboard API metrics into NaaP Overview + developer-api#163
Closed
eliteprox wants to merge 16 commits into
Closed
feat(metrics): Integrate Leaderboard API metrics into NaaP Overview + developer-api#163eliteprox wants to merge 16 commits into
eliteprox wants to merge 16 commits into
Conversation
…terminology - Removed all references to "gateway" in the API key creation and management flow, replacing them with "provider" terminology for consistency. - Updated related components and hooks to reflect the removal of gateway-related logic, simplifying the API key creation process. - Adjusted the data model to eliminate gateway-specific fields, ensuring a cleaner and more focused structure. - Enhanced the UI to display provider information instead of gateway details, improving clarity for users. This change streamlines the API key management process and aligns terminology across the application.
- Eliminated all references to gateways in the API key management flow, replacing them with provider terminology for consistency. - Removed model ID handling from API key creation, simplifying the process and focusing on project and provider details. - Updated related components and data structures to reflect these changes, enhancing clarity and maintainability. - Adjusted UI elements to display provider information instead of gateway details, improving user experience. This refactor streamlines the API key management process and aligns terminology across the application.
…ctural improvements - Removed `IMPROVEMENT.md`, `leaderboard-api-data-contract.md`, and `leaderboard-api-gaps.md` as they contained outdated information and were no longer relevant to the current project structure. - This cleanup helps streamline the documentation and ensures that only current and useful resources are available for developers.
- Updated the API key generation method to use a secure random hex string format, improving security and uniqueness. - Enhanced clipboard copy functionality in the CreateKeyModal and APIKeysTab components to handle potential clipboard access errors gracefully. - Removed outdated model ID references from API key handling, focusing on project and provider details for clarity. - Introduced a notification banner for successful key creation, improving user experience by providing immediate feedback. These changes streamline the API key management process and enhance the overall user interface.
…onents - Introduced a new leaderboard API for fetching network capabilities, including pipelines, GPU metrics, SLA compliance, and network demand. - Updated model components (ModelCard, ModelDetailPanel, CompareDrawer) to utilize the new NetworkModel type and display relevant metrics. - Enhanced filtering and sorting functionality in ModelsTab for better user experience. - Added new data structures and hooks to manage network capabilities and GPU hardware summaries. - Improved UI elements to reflect real-time data availability and GPU fleet information. This commit lays the groundwork for a more robust model management system, integrating real-time data and enhancing user interaction with the models.
…l data handling - Introduced a new leaderboard API for fetching network capabilities, including GPU metrics and SLA compliance. - Added a new route for handling leaderboard requests in the API. - Enhanced model components (ModelCard, ModelDetailPanel, CompareDrawer) to utilize the new NetworkModel type and display relevant metrics. - Updated filtering and sorting functionality in ModelsTab to improve user experience. - Introduced advanced filtering options for GPU metrics and network demand. - Refactored utility functions for GPU name formatting and query string generation. This commit enhances the model management system by integrating real-time data and improving user interaction with the models.
…etail modal - Added state management for selected model in DeveloperView, allowing users to click on model cards to view detailed information. - Implemented a modal to display detailed metrics for the selected model, including average FPS, latency, SLA score, and GPU types. - Improved user interaction by enabling model selection and providing a comprehensive view of model attributes. This update enhances the user experience by allowing for better exploration of model details directly within the DeveloperView.
- Updated CreateKeyModal to use API-based key generation with proper error handling - Kept NetworkModel-based display for CompareDrawer, ModelCard, and ModelDetailPanel - Enhanced ModelsTab with improved filtering and state management - Added getServiceOrigin import and requestJson helper in useApi hook - Preserved both NetworkModel and AIModel type definitions - Maintained CatalogModel usage in DeveloperView frontend Made-with: Cursor
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR transitions the dashboard provider from mock data to live data sourced from the Livepeer Leaderboard API, subgraph, and L1 RPC. It introduces new API routes for data proxying, updates data structures to support network metrics, refactors UI components to display real-time orchestrator and fees data, and implements data aggregation hooks. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Dashboard Client
participant Dashboard as Dashboard API<br/>(Next.js)
participant Leaderboard as Leaderboard API
participant Subgraph as The Graph<br/>Subgraph
participant RPC as L1 RPC
Client->>Dashboard: GraphQL Query<br/>(KPI, Protocol, Fees)
activate Dashboard
par Parallel Requests
Dashboard->>Leaderboard: Fetch Network Demand<br/>GPU Metrics, SLA Compliance
Dashboard->>Subgraph: Query Fees & Protocol<br/>Daily/Weekly Data
Dashboard->>RPC: Get Latest Block
end
Leaderboard-->>Dashboard: Metrics Data
Subgraph-->>Dashboard: Fees, Protocol Overview
RPC-->>Dashboard: Block Number
Dashboard->>Dashboard: Aggregate & Transform<br/>- Compute KPI deltas<br/>- Weight SLA scores<br/>- Build Gateway Offers
Dashboard-->>Client: Resolved Query<br/>with Aggregated Data
deactivate Dashboard
sequenceDiagram
participant Models as Models Tab Component
participant Hook as useNetworkCapabilities<br/>(React Hook)
participant Leaderboard as Leaderboard API
participant SDK as Data APIs<br/>(Pipelines, Metrics, SLA)
Models->>Hook: Call Hook<br/>with Interval
activate Hook
par Fetch All Data
Hook->>Leaderboard: fetchPipelines()
Hook->>Leaderboard: fetchRegions()
Hook->>Leaderboard: fetchGPUMetrics(timeRange)
Hook->>Leaderboard: fetchSLACompliance(period)
Hook->>Leaderboard: fetchNetworkDemand(interval)
end
Leaderboard-->>Hook: Aggregated Data
Hook->>Hook: Match & Aggregate<br/>- Build GPUHardware Summary<br/>- Weight SLA Scores<br/>- Construct GatewayOffers<br/>- Derive NetworkModels
Hook-->>Models: Return NetworkCapabilities<br/>(models[], gpuTypes[], regions[])
deactivate Hook
Models->>Models: Filter & Sort<br/>by FPS, Latency, SLA
Models->>Models: Render Model Grid<br/>with Live Metrics
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 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 |
github-actions
Bot
added
scope/shell
|
|
eliteprox
changed the title
eliteprox
changed the title
There was a problem hiding this comment.
Pull request overview
This PR integrates live Livepeer Leaderboard API metrics into two areas: the NaaP dashboard overview (via the dashboard-provider-mock plugin, now backed by real data) and the developer-api plugin's model catalog. It also introduces a richer model browsing experience in the developer-web app with live GPU, SLA, region, and gateway data, replaces all mock model data with live leaderboard-sourced models, and adds a Next.js proxy route for the leaderboard API.
Changes:
- Replaces mock dashboard KPI, pipeline, and GPU capacity data with live leaderboard API calls; adds "Coming Soon" overlays for fees, pricing, and job feed
- Introduces
useModelCataloghook (developer-api plugin) anduseNetworkCapabilitieshook (developer-web app) to fetch and aggregate live leaderboard model data - Adds a Next.js catch-all proxy route at
/api/v1/leaderboard/[...path]and updates the developer-web models tab with rich filtering, sorting, and detail panel powered by live data
Reviewed changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
plugins/developer-api/frontend/src/pages/DeveloperView.tsx |
Replaces static model state with useModelCatalog hook; adds model detail modal with live metrics |
plugins/developer-api/frontend/src/hooks/useModelCatalog.ts |
New hook aggregating leaderboard pipeline, GPU, and SLA data into CatalogModel objects |
plugins/developer-api/frontend/src/data/model-catalog.ts |
New static config mapping model IDs to display names and exclusion list |
plugins/developer-api/frontend/src/api/leaderboard.ts |
New typed fetch client for the leaderboard API (routes through proxy by default) |
plugins/dashboard-provider-mock/frontend/src/provider.ts |
Replaces mock KPI/pipeline/GPU resolvers with real leaderboard API calls |
plugins/dashboard-provider-mock/frontend/src/data/pipeline-config.ts |
New pipeline display name and color configuration |
plugins/dashboard-provider-mock/frontend/src/api/leaderboard.ts |
New typed fetch client making direct calls to leaderboard API (hardcoded URL) |
plugins/dashboard-provider-mock/frontend/src/__tests__/provider.test.ts |
Updates tests to mock the leaderboard API fetch and validate live data shapes |
packages/types/src/index.ts |
Adds NetworkModel, GPUHardwareSummary, GatewayOffer, and related types |
apps/workflows/developer-web/src/api/leaderboard.ts |
New typed fetch client making direct calls to leaderboard API (hardcoded URL) |
apps/workflows/developer-web/src/hooks/useNetworkCapabilities.ts |
New hook with polling, aggregating full model catalog with GPU fleet, regions, and gateway offers |
apps/workflows/developer-web/src/data/network-config.ts |
New static config mirroring model-catalog.ts for developer-web |
apps/workflows/developer-web/src/utils/gpu.ts |
New utility for shortening GPU names |
apps/workflows/developer-web/src/components/tabs/ModelsTab.tsx |
Full rewrite with live data, advanced filters, skeleton loading |
apps/workflows/developer-web/src/components/models/ModelDetailPanel.tsx |
Updated to NetworkModel, shows GPU fleet table and regions |
apps/workflows/developer-web/src/components/models/ModelCard.tsx |
Updated to NetworkModel, shows live FPS, latency, SLA, GPU, and region data |
apps/workflows/developer-web/src/components/models/CompareDrawer.tsx |
Updated to NetworkModel, shows live metrics in comparison table |
apps/web-next/src/app/api/v1/leaderboard/[...path]/route.ts |
New Next.js proxy route forwarding leaderboard API requests server-side |
apps/web-next/src/app/(dashboard)/dashboard/page.tsx |
Adds "Coming Soon" overlays on Fees, Job Feed, and Pricing cards |
apps/workflows/developer-web/src/hooks/useApi.ts |
Minor whitespace cleanup |
package-lock.json |
Marks fsevents as a dev dependency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+4
to
+8
| * Calls https://leaderboard-api.livepeer.cloud directly from the browser, | ||
| * following the same pattern as plugins/dashboard-provider-mock/frontend/src/api/leaderboard.ts | ||
| */ | ||
|
|
||
| const BASE_URL = 'https://leaderboard-api.livepeer.cloud'; |
There was a problem hiding this comment.
The apps/workflows/developer-web/src/api/leaderboard.ts file hardcodes BASE_URL = 'https://leaderboard-api.livepeer.cloud' and makes direct browser-to-external-API calls, while the plugins/developer-api plugin correctly routes through the Next.js proxy at /api/v1/leaderboard (with a VITE_LEADERBOARD_BASE_URL override). Similarly, plugins/dashboard-provider-mock/frontend/src/api/leaderboard.ts also hardcodes the external URL. Using the proxy has advantages: it avoids CORS issues, allows per-environment configuration, enables server-side caching, and avoids exposing the third-party endpoint directly. The developer-web and dashboard-provider-mock files should use an environment variable with the proxy URL as the default, consistent with the developer-api plugin approach.
Suggested change
| * Calls https://leaderboard-api.livepeer.cloud directly from the browser, | |
| * following the same pattern as plugins/dashboard-provider-mock/frontend/src/api/leaderboard.ts | |
| */ | |
| const BASE_URL = 'https://leaderboard-api.livepeer.cloud'; | |
| * Calls the leaderboard API through a proxy endpoint by default, | |
| * configurable via the VITE_LEADERBOARD_BASE_URL environment variable. | |
| */ | |
| const BASE_URL = | |
| (import.meta as any).env?.VITE_LEADERBOARD_BASE_URL ?? '/api/v1/leaderboard'; |
| import { CompareDrawer } from '../models/CompareDrawer'; | ||
| import { CreateKeyModal } from '../api-keys/CreateKeyModal'; | ||
| import { shortGPUName } from '../../utils/gpu'; | ||
| /** Shorten "NVIDIA GeForce RTX 5090" → "RTX 5090" for filter chip labels */ |
There was a problem hiding this comment.
The JSDoc comment at line 22 is misplaced: it appears after the import statement for shortGPUName, and is rendered as a floating comment between imports and the first function declaration rather than as documentation on any construct. JSDoc comments should appear directly before the function, class, or variable they describe. Move the comment to apps/workflows/developer-web/src/utils/gpu.ts where the function is defined, or convert it to a regular inline comment if it needs to stay here.
Suggested change
| /** Shorten "NVIDIA GeForce RTX 5090" → "RTX 5090" for filter chip labels */ | |
| // Shorten "NVIDIA GeForce RTX 5090" → "RTX 5090" for filter chip labels |
| } | ||
|
|
||
| function shortGPUName(name: string): string { | ||
| return name.replace(/^NVIDIA\s+/i, '').replace(/^GeForce\s+/i, ''); |
There was a problem hiding this comment.
The shortGPUName function is defined twice: once in apps/workflows/developer-web/src/utils/gpu.ts (exported) and again as a private function in plugins/developer-api/frontend/src/hooks/useModelCatalog.ts. If this logic ever changes (e.g., more prefix patterns to strip), both copies would need to be updated. Consider extracting it to a shared package (e.g., @naap/utils) or at minimum to a shared utility file within the developer-api plugin.
Suggested change
| return name.replace(/^NVIDIA\s+/i, '').replace(/^GeForce\s+/i, ''); | |
| const prefixes = [/^NVIDIA\s+/i, /^GeForce\s+/i]; | |
| let result = name; | |
| for (const prefix of prefixes) { | |
| result = result.replace(prefix, ''); | |
| } | |
| return result; |
| </p> | ||
| </div> | ||
| <div className="p-4 rounded-xl bg-bg-tertiary border border-white/10"> | ||
| <p className="text-xs text-text-secondary uppercase tracking-wider mb-1">Latency (p50)</p> |
There was a problem hiding this comment.
The UI label "Latency (p50)" at line 766 is inaccurate: selectedModel.latencyP50 is computed in useModelCatalog.ts as the arithmetic mean (average) of all available e2e_latency_ms values, not the 50th percentile (median). This misinforms users about the nature of the metric. The label should be updated to "Avg E2E Latency" or similar, and the field should be renamed to avgLatencyMs or e2eLatencyMs in CatalogModel.
Suggested change
| <p className="text-xs text-text-secondary uppercase tracking-wider mb-1">Latency (p50)</p> | |
| <p className="text-xs text-text-secondary uppercase tracking-wider mb-1">Avg E2E Latency</p> |
| <div className="p-4 rounded-xl bg-bg-tertiary border border-white/10"> | ||
| <p className="text-xs text-text-secondary uppercase tracking-wider mb-1">SLA Score</p> | ||
| <p className="text-2xl font-bold text-accent-emerald font-mono"> | ||
| {selectedModel.slaScore != null ? `${Math.round(selectedModel.slaScore * 100)}%` : '—'} |
There was a problem hiding this comment.
The slaScore field is displayed as Math.round(selectedModel.slaScore * 100)%, which assumes slaScore is in the 0-1 range. However, the test stub data in provider.test.ts (line 70, 73, 76) uses sla_score: 100, implying the real leaderboard API may return sla_score as a 0-100 integer rather than a 0-1 float. If the API actually returns 0-100, weightedSLAScore would return ~100, and this line would render "10000%". Verify the actual API range: if it is 0-100, remove the * 100 multiplication here; if it is 0-1, update the test stub in provider.test.ts to use sla_score: 1.0 to avoid future confusion.
Suggested change
| {selectedModel.slaScore != null ? `${Math.round(selectedModel.slaScore * 100)}%` : '—'} | |
| {selectedModel.slaScore != null ? `${Math.round(selectedModel.slaScore)}%` : '—'} |
| model: NetworkModel; | ||
| onClose: () => void; | ||
| onCreateKey: () => void; | ||
| onCreateKey: (model: NetworkModel, gateway: GatewayOffer) => void; |
There was a problem hiding this comment.
The onCreateKey prop is declared as (model: NetworkModel, gateway: GatewayOffer) => void, but the button calls it directly via onClick={onCreateKey}, which means the click MouseEvent is passed as the first argument instead of a NetworkModel. Additionally, there is no gateway selection UI in this panel, so the GatewayOffer argument would never be provided. The caller in ModelsTab.tsx supplies a () => void handler (which ignores its arguments), so this works at runtime, but the prop type is incorrect and misleading. The prop type should match the actual callback signature (no parameters, or at least just model: NetworkModel) and the button should use an inline arrow function if specific arguments need to be passed.
| orchestratorCount: number; | ||
| gpuTypes: string[]; | ||
| avgFPS: number; | ||
| latencyP50: number | null; |
There was a problem hiding this comment.
The variable latencyP50 and the associated CatalogModel.latencyP50 field are computed as the arithmetic mean (average) of all e2e_latency_ms values across rows, not as the 50th percentile (median). The name "P50" implies a percentile calculation. The UI label in DeveloperView.tsx also shows "Latency (p50)", which is misleading to users. Consider renaming this field to avgLatencyMs (consistent with GPUHardwareSummary.avgLatencyMs in the same codebase) and updating the UI label accordingly.
Suggested change
| latencyP50: number | null; | |
| avgLatencyMs: number | null; |
| * @param eventBus - The shell event bus instance | ||
| * @returns Cleanup function to call on plugin unmount | ||
| */ | ||
| export function registerMockDashboardProvider(eventBus: IEventBus): () => void { |
There was a problem hiding this comment.
The function registerMockDashboardProvider still carries the "Mock" prefix in its name, but it now serves live data from the leaderboard API for KPI, pipelines, and GPU capacity. The "Mock" name is misleading since the primary data sources are real. This affects the exported function name in provider.ts, its call sites in App.tsx, and the test file. Consider renaming it to registerDashboardProvider or registerLeaderboardDashboardProvider to accurately reflect its behavior.
qianghan
suggested changes
Feb 27, 2026
Contributor
qianghan
left a comment
qianghan
left a comment
There was a problem hiding this comment.
Code Review — PR #163
Reviewed the full diff (21 files, +2224/−363). Below are the findings organized by severity.
🔴 Security Issues
1. SSRF via open proxy — apps/web-next/src/app/api/v1/leaderboard/[...path]/route.ts
The catch-all proxy route forwards any path to the leaderboard API with zero validation. An attacker can use path traversal or URL manipulation to probe internal services:
GET /api/v1/leaderboard/../../internal-service/secret
GET /api/v1/leaderboard/foo?callback=http://evil.com
The LEADERBOARD_API_URL env var can be overridden to point anywhere, and no path allowlist exists.
Action: Add an allowlist of permitted path prefixes (pipelines, gpu/metrics, sla/compliance, network/demand, regions). Reject any path not in the allowlist. Strip .. segments. Example:
const ALLOWED_PATHS = new Set(['pipelines', 'gpu/metrics', 'sla/compliance', 'network/demand', 'regions']);
const normalized = pathString.replace(/\.\./g, '');
const prefix = normalized.split('/').slice(0, 2).join('/');
if (!ALLOWED_PATHS.has(prefix) && !ALLOWED_PATHS.has(normalized.split('/')[0])) {
return NextResponse.json({ error: 'Forbidden' }, { status: 403 });
}2. No authentication on the proxy route
The leaderboard proxy has no auth check — any unauthenticated request to /api/v1/leaderboard/* is forwarded. Every other NaaP API route validates JWT or session.
Action: Add validateSession() or at minimum a rate limiter. If this is intentionally public, document why and add rate limiting in vercel.json functions config.
3. Hardcoded external API URL in client-side code (3 places)
Direct browser-to-external-API calls bypass the proxy entirely, making the proxy redundant and leaking the upstream URL to clients:
apps/workflows/developer-web/src/api/leaderboard.ts— hardcodedhttps://leaderboard-api.livepeer.cloudplugins/dashboard-provider-mock/frontend/src/api/leaderboard.ts— same hardcoded URLplugins/developer-api/frontend/src/api/leaderboard.ts— usesimport.meta.envwith fallback to proxy ✅ (this one is correct)
Action: All three should use the proxy route (/api/v1/leaderboard/...) or an env variable. Direct external calls create CORS dependencies, leak infrastructure URLs, and bypass any future auth/rate-limiting on the proxy.
🟠 Architecture Issues
4. Triple-duplicated leaderboard API client
The same fetch logic + interfaces are copy-pasted across three locations:
apps/workflows/developer-web/src/api/leaderboard.ts(190 lines)plugins/dashboard-provider-mock/frontend/src/api/leaderboard.ts(96 lines)plugins/developer-api/frontend/src/api/leaderboard.ts(95 lines)
The interfaces (GPUMetricRow, SLAComplianceRow, NetworkDemandRow, etc.) are subtly different across copies — some include fields like jitter_coeff_fps, cuda_version, runner_version that others omit. This will drift further over time.
Action: Create a single shared client in packages/types/ or a new packages/leaderboard-api/ package. Export the interfaces and fetch functions once. All three consumers import from the shared package. The base URL can be parameterized.
5. Triple-duplicated config data
Pipeline display names and model catalogs are duplicated:
apps/workflows/developer-web/src/data/network-config.tsplugins/dashboard-provider-mock/frontend/src/data/pipeline-config.tsplugins/developer-api/frontend/src/data/model-catalog.ts
All contain PIPELINE_DISPLAY, MODEL_DISPLAY, EXCLUDED_MODELS with the same values. They even have comments saying "keep in sync with" each other.
Action: Consolidate into one file in packages/types/ or a shared config package. "Keep in sync" comments are a code smell — DRY it.
6. Duplicated helper functions
These functions are copy-pasted across useNetworkCapabilities.ts and useModelCatalog.ts:
matchesModel()— identical logicweightedSLAScore()— identical logicshortGPUName()— exists inutils/gpu.tsAND inlined inuseModelCatalog.ts
Action: Move shared aggregation helpers to the shared leaderboard package or packages/types/.
7. useModelCatalog is a subset of useNetworkCapabilities
useModelCatalog (developer-api plugin) does exactly the same thing as useNetworkCapabilities (developer-web app) but returns fewer fields (CatalogModel vs NetworkModel). The data fetching, aggregation logic, and even the matchesModel() function are identical.
Action: Use useNetworkCapabilities as the single implementation. If CatalogModel needs fewer fields, just map NetworkModel → CatalogModel at the consumer. Don't duplicate 150 lines of aggregation logic.
🟡 Code Quality Issues
8. latencyP50 is not actually p50
In useModelCatalog.ts, the field is named latencyP50 but computed as an arithmetic mean:
const latencyP50 = latencies.length
? Math.round(latencies.reduce((sum, value) => sum + value, 0) / latencies.length)
: null;Action: Rename to avgLatencyMs for accuracy (matching the naming in useNetworkCapabilities), or compute the actual p50 (median).
9. No error boundaries for API failures
Both useNetworkCapabilities and useModelCatalog catch errors and set an error string, but the polling in useNetworkCapabilities silently swallows errors on refresh cycles (doesn't set error state on non-initial loads). If the API goes down after initial load, the UI shows stale data with no indication.
Action: Always update error state on failure, even during refresh polls. Show a stale-data indicator in the UI.
10. PR description is empty
The summary, changes list, type checkboxes, and plugin(s) affected are all blank. This makes it impossible for reviewers to understand intent without reading 2200+ lines of diff.
Action: Fill in the PR template — summary, changes, type (Feature), plugins affected (dashboard-provider-mock, developer-api), and checklist.
🟢 Minor / Nits
11. Dashboard "Coming Soon" overlays
The fees card, job feed, and pricing card have blur overlays saying "Coming Soon" but the underlying data is still fetched and rendered (just hidden). This wastes API calls and compute.
Action: Skip the data fetch entirely when the overlay is shown, or gate the API call behind a feature flag.
12. toQueryString doesn't handle array values
The query string builder in the leaderboard API only handles string | undefined. If the API ever needs array params (e.g., multiple regions), this will silently break.
Action (nit): Fine for now, but worth noting for future extensibility.
Summary
| Category | Count | Key actions |
|---|---|---|
| 🔴 Security | 3 | Add path allowlist + auth to proxy; stop hardcoding external URLs in client code |
| 🟠 Architecture | 4 | Consolidate 3x duplicated API client, config, and hooks into shared packages |
| 🟡 Code Quality | 3 | Fix misleading latencyP50 name; handle refresh errors; fill PR description |
| 🟢 Minor | 2 | Skip hidden data fetches; note query string limitation |
The leaderboard integration itself is valuable, but the triple-duplication pattern will create significant maintenance burden. Consolidating the API client + config into a shared package before merge will save considerable effort later.
6 tasks
6 tasks
This was
linked to
issues
Mar 3, 2026
|
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 |
…nd extend dashboard queries - Added Recharts library for improved charting capabilities in the dashboard. - Updated the dashboard GraphQL query to include orchestrator data and extended fee metrics. - Modified the FeesCard component to visualize fee data over time with daily and weekly breakdowns. - Enhanced the KPIRow component to display orchestrator metrics with toggle functionality for raw data. - Updated mock data and tests to reflect new fee structures and orchestrator information.
…ntegration - Added a toggle to the FeesCard component for displaying raw fees data, allowing users to switch between aggregated and detailed views. - Implemented logic to fetch and display raw fees data based on daily or weekly grouping. - Updated the .env.example file to include new subgraph API key and ID configurations for better integration with the dashboard-provider plugin. - Adjusted package.json scripts to include subgraph API key for testing purposes. - Refactored subgraph fetching logic to streamline error handling and improve data retrieval processes.
…e documentation - Changed environment variable names from VITE_SUBGRAPH_API_KEY and VITE_SUBGRAPH_ID to SUBGRAPH_API_KEY and SUBGRAPH_ID for consistency and clarity. - Updated .env.example and .env.local.example files to reflect new variable names and provide clearer instructions for local development. - Added a new API route for subgraph proxying, ensuring server-side handling of API keys. - Refactored subgraph fetching logic in the dashboard to improve error handling and streamline data retrieval processes.
- Renamed variables for clarity, changing `currentDay` to `latestDay` and added `dayBeforePrevious` for improved data handling. - Enhanced logic to determine if the latest day and week data are incomplete, adjusting the display logic accordingly. - Updated return values to reflect the new logic, ensuring accurate volume and change calculations based on the latest available data.
…configuration - Introduced a new API route to fetch the current L1 (Ethereum mainnet) block number, essential for accurate Livepeer round progress. - Updated .env.example to include a new L1_RPC_URL variable for specifying the Ethereum RPC URL. - Added the 'viem' package to manage Ethereum interactions and updated package.json accordingly. - Enhanced dashboard provider to integrate the new protocol block fetching logic, ensuring real-time data availability. - Updated tests to validate the new protocol block functionality and error handling.
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
| : `${uniqueGPUs.slice(0, 2).join(' · ')} +${uniqueGPUs.length - 2}`; | ||
|
|
||
| const slaPercent = | ||
| model.slaScore != null ? `${Math.round(model.slaScore * 100)}%` : null; |
Contributor
There was a problem hiding this comment.
SLA score values incorrectly multiplied by 100 when they are already in 0-100 range, causing display of "10000%" instead of "100%"
| <div className="p-4 rounded-xl bg-bg-tertiary border border-white/10"> | ||
| <p className="text-xs text-text-secondary uppercase tracking-wider mb-1">Latency (p50)</p> | ||
| <p className="text-2xl font-bold text-text-primary font-mono"> | ||
| {selectedModel.latencyP50 != null ? `${selectedModel.latencyP50}ms` : '—'} |
Contributor
There was a problem hiding this comment.
The "Latency (p50)" metric is computed as an arithmetic mean (average) instead of an actual P50 (median), causing misleading data display to users.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (8)
plugins/dashboard-provider-mock/frontend/src/__tests__/provider.test.ts (2)
450-453: Redundant module-levelafterEachfor timer cleanup.The module-level
afterEach(lines 451-453) duplicates the cleanup already performed in theregisterMockJobFeedEmitterdescribe block (lines 387-389). Whilevi.useRealTimers()is idempotent, the duplication is unnecessary and could cause confusion.♻️ Suggested removal
-// Need afterEach at module level for fake timer cleanup -afterEach(() => { - vi.useRealTimers(); -});The existing
afterEachin theregisterMockJobFeedEmitterdescribe block already handles timer cleanup for that suite, and theregisterMockDashboardProvidersuite doesn't use fake timers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/dashboard-provider-mock/frontend/src/__tests__/provider.test.ts` around lines 450 - 453, Remove the redundant module-level afterEach that calls vi.useRealTimers(); it's duplicated by the afterEach inside the registerMockJobFeedEmitter describe block; locate the top-level afterEach invocation (the one outside any describe) and delete it so only the registerMockJobFeedEmitter's timer cleanup remains—do not touch the afterEach inside registerMockJobFeedEmitter or any other cleanup in registerMockDashboardProvider.
280-295: Re-stubbingfetchmid-suite may affect test isolation.This test re-stubs
fetchto simulate failure, but the stub persists for subsequent tests if they run in the same suite context. WhileafterEachon line 200-202 callsvi.unstubAllGlobals(), test execution order isn't guaranteed in parallel mode.Consider using
vi.spyOnwithmockImplementationOncefor single-test overrides, or ensure the stub is explicitly restored.🧪 Alternative using mockImplementationOnce
it('returns protocol null and errors when subgraph or protocol-block fails', async () => { - vi.stubGlobal( - 'fetch', - vi.fn(() => Promise.resolve({ ok: false, status: 503 } as Response)) - ); + const originalFetch = globalThis.fetch; + vi.stubGlobal('fetch', vi.fn(() => Promise.resolve({ ok: false, status: 503 } as Response))); registerMockDashboardProvider(mockEventBus as any); const response = (await mockEventBus._invoke(DASHBOARD_QUERY_EVENT, { query: '{ protocol { currentRound blockProgress totalBlocks totalStakedLPT } }', })) as DashboardQueryResponse; expect(response.data?.protocol).toBeNull(); expect(response.errors).toBeDefined(); expect(response.errors!.length).toBeGreaterThan(0); expect(response.errors!.some((e) => e.message?.includes('protocol-block') || e.message?.includes('subgraph'))).toBe(true); + vi.stubGlobal('fetch', originalFetch); // Restore for subsequent tests });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/dashboard-provider-mock/frontend/src/__tests__/provider.test.ts` around lines 280 - 295, The test "returns protocol null and errors when subgraph or protocol-block fails" currently stubs global fetch via vi.stubGlobal which can leak across tests; change it to a single-test override using vi.spyOn(globalThis, 'fetch').mockImplementationOnce(...) (or mockImplementation once on the existing spy) so the failure response is only applied for this invocation, or explicitly restore the stub immediately after the test using vi.restoreAllMocks()/vi.unstubAllGlobals(); update the test that calls registerMockDashboardProvider and mockEventBus._invoke (DASHBOARD_QUERY_EVENT) to use the one-off spy to ensure test isolation.apps/workflows/developer-web/src/data/network-config.ts (1)
1-46: Duplication withplugins/developer-api/frontend/src/data/model-catalog.ts.As noted in the sister file review, this configuration is duplicated. The sync comment on line 6 references
pipeline-config.tsbut doesn't mentionmodel-catalog.ts, which also contains identical mappings. Update the comment to list all locations that must stay in sync, or consolidate into a shared module.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/workflows/developer-web/src/data/network-config.ts` around lines 1 - 46, The comment about keeping mappings in sync only mentions pipeline-config.ts but this config (PIPELINE_DISPLAY, MODEL_DISPLAY, EXCLUDED_MODELS, etc.) is duplicated in plugins/developer-api/frontend/src/data/model-catalog.ts; update the header comment to either list that file as another location that must stay in sync or refactor by consolidating these exports into a shared module and import them here and in model-catalog.ts so PIPELINE_DISPLAY, MODEL_DISPLAY and EXCLUDED_MODELS are single-source-of-truth.plugins/dashboard-provider-mock/frontend/src/api/leaderboard.ts (2)
77-95: URL-encode query parameter values to handle special characters safely.The
interval,timeRange, andperiodparameters are interpolated directly into the URL without encoding. If these ever contain special characters, the request could fail or behave unexpectedly.🛡️ Proposed fix
export async function fetchNetworkDemand(interval: string): Promise<NetworkDemandRow[]> { const data = await apiFetch<{ demand: NetworkDemandRow[] }>( - `/api/network/demand?interval=${interval}` + `/api/network/demand?interval=${encodeURIComponent(interval)}` ); return data.demand ?? []; } export async function fetchGPUMetrics(timeRange: string): Promise<GPUMetricRow[]> { const data = await apiFetch<{ metrics: GPUMetricRow[] }>( - `/api/gpu/metrics?time_range=${timeRange}` + `/api/gpu/metrics?time_range=${encodeURIComponent(timeRange)}` ); return data.metrics ?? []; } export async function fetchSLACompliance(period: string): Promise<SLAComplianceRow[]> { const data = await apiFetch<{ compliance: SLAComplianceRow[] }>( - `/api/sla/compliance?period=${period}` + `/api/sla/compliance?period=${encodeURIComponent(period)}` ); return data.compliance ?? []; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/dashboard-provider-mock/frontend/src/api/leaderboard.ts` around lines 77 - 95, The URL query values are interpolated raw in fetchNetworkDemand, fetchGPUMetrics, and fetchSLACompliance which can break for special characters; update those functions to URL-encode their parameters (use encodeURIComponent or equivalent) when building the request string passed to apiFetch so `/api/network/demand?interval=...`, `/api/gpu/metrics?time_range=...`, and `/api/sla/compliance?period=...` use encoded values to ensure safe, correct requests.
14-14: Consider using the leaderboard proxy route instead of the hardcoded external URL.The
BASE_URLis hardcoded to the external leaderboard API, butapps/web-next/src/app/api/v1/leaderboard/[...path]/route.tsprovides a proxy. Using the proxy (e.g.,/api/v1/leaderboard) would:
- Keep API calls within the same origin (avoiding CORS in browser contexts)
- Allow centralized configuration via
LEADERBOARD_API_URLenv var- Provide consistent error handling
♻️ Suggested change
-const BASE_URL = 'https://leaderboard-api.livepeer.cloud'; +// Use relative URL to leverage the Next.js proxy route +const BASE_URL = '/api/v1/leaderboard';Note: This requires updating the
apiFetchpath construction since the proxy expects paths like/api/v1/leaderboard/network/demandrather than/api/network/demand.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/dashboard-provider-mock/frontend/src/api/leaderboard.ts` at line 14, The code hardcodes BASE_URL to the external leaderboard API; switch to using the internal proxy by replacing BASE_URL with the proxy root '/api/v1/leaderboard' and update any call sites that build paths (e.g., in this file's apiFetch or functions referencing BASE_URL) so they construct requests like '/api/v1/leaderboard/network/demand' (not '/api/network/demand') to avoid CORS and use centralized LEADERBOARD_API_URL via the proxy; ensure functions that concatenate paths handle leading/trailing slashes to prevent duplicate slashes and that error handling remains consistent when calling the proxy.apps/web-next/src/app/api/v1/leaderboard/[...path]/route.ts (1)
9-11: Validate or sanitize the path segments to prevent unintended API access.The
patharray is directly concatenated into the target URL. While Next.js URL-encodes segments, consider validating that path segments don't contain unexpected patterns (e.g.,..or protocol prefixes) that could be exploited if the upstream API has routing vulnerabilities.🛡️ Example path validation
const { path } = await params; + // Basic validation: reject suspicious patterns + if (path.some(segment => segment.includes('..') || segment.includes(':'))) { + return NextResponse.json( + { error: { code: 'BAD_REQUEST', message: 'Invalid path' } }, + { status: 400 } + ); + } const pathString = path.join('/');🤖 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/leaderboard/`[...path]/route.ts around lines 9 - 11, The path segments from params are used directly to build targetUrl which can allow traversal or protocol-injection; in the code that computes pathString and targetUrl (look for params, pathString, and targetUrl in route.ts) validate and sanitize each segment before joining: reject or return 400 for empty segments, "." or "..", segments starting with protocols (e.g., "http:", "https:"), or containing suspicious characters, and apply a safe encoding (encode each segment or enforce an allowlist like /^[A-Za-z0-9-_]+$/) before joining with '/' so targetUrl is built only from validated, encoded segments against LEADERBOARD_API_URL.plugins/developer-api/frontend/src/data/model-catalog.ts (1)
1-45: Consider consolidating withapps/workflows/developer-web/src/data/network-config.ts.This file duplicates nearly the same constants as
network-config.tsin the developer-web app. Maintaining two copies increases the risk of drift when models are added or renamed. Consider extracting these shared mappings into a common package (e.g.,@naap/typesor a shared config module) that both locations can import.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/developer-api/frontend/src/data/model-catalog.ts` around lines 1 - 45, This module duplicates constants (PIPELINE_DISPLAY, MODEL_DISPLAY, EXCLUDED_MODELS, REALTIME_FPS_THRESHOLD, UNKNOWN_GPU) that are also defined in apps/workflows/developer-web/src/data/network-config.ts; extract these shared mappings into a single shared package or module (e.g., `@naap/types` or a new shared config module), export the constants there, then replace the local definitions in model-catalog.ts with imports from the new shared module and update the other consumer (network-config.ts) to import the same source so both locations reference a single canonical definition to prevent drift.apps/web-next/.env.local.example (1)
60-64: Clarify where the SUBGRAPH_ID default is applied.The comment states "defaults to FE63Y..." but it's unclear whether this default is hardcoded in the application code or if developers must uncomment and set this value. Consider either:
- Documenting which file contains the default, or
- Providing the default value in the variable itself (commented) for easy uncommenting
📝 Suggested clarification
# ─── Subgraph Fees Proxy (server-only) ───────────────────────────────── # Used by /api/v1/subgraph. Keep keys out of VITE_* vars. # SUBGRAPH_API_KEY= -# Optional: defaults to FE63YgkzcpVocxdCEyEYbvjYqEf2kb1A6daMYRxmejYC -# SUBGRAPH_ID= +# SUBGRAPH_ID defaults to the Livepeer mainnet subgraph if not set (see apps/web-next/src/app/api/v1/subgraph/route.ts) +# SUBGRAPH_ID=FE63YgkzcpVocxdCEyEYbvjYqEf2kb1A6daMYRxmejYC🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web-next/.env.local.example` around lines 60 - 64, The SUBGRAPH_ID default is ambiguous; update the env example to make where the default is applied explicit by either adding a commented default value for SUBGRAPH_ID (e.g., "# SUBGRAPH_ID=FE63Ygkz...") so developers can easily uncomment, or add a short note pointing to the application code that injects the default (mentioning the /api/v1/subgraph handler or the module that reads SUBGRAPH_ID) so it's clear whether the default is hardcoded or must be provided; modify the block containing SUBGRAPH_API_KEY and SUBGRAPH_ID accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.env.example:
- Around line 33-42: The .env.example contains duplicated environment variables;
remove the redundant declarations of SUBGRAPH_API_KEY and SUBGRAPH_ID so each
variable (SUBGRAPH_API_KEY, SUBGRAPH_ID) is declared only once in the file,
consolidating the entries (keep the descriptive comment block and the primary
definitions near the SUBGRAPH section and delete the later duplicates around
lines mentioned in the review) to avoid confusion when copying into runtime .env
files.
In `@apps/web-next/src/app/`(dashboard)/dashboard/page.tsx:
- Around line 287-297: The icon-only button that toggles orchestrator raw data
(the JSX button using onToggleOrchestrators and the orchestratorsOpen state with
the List icon) lacks an accessible name; add an explicit aria-label (or
aria-labelledby) to that button—e.g., aria-label="Toggle raw orchestrator
data"—so screen readers can announce its purpose, and apply the same fix to the
other icon-only button referenced (the one at the other location using a similar
pattern on lines 812-817).
- Around line 780-788: The sortable header currently uses onClick on the <th>,
which is not keyboard-accessible; move the interactive behavior into a real
<button type="button"> inside the <th>: keep the existing classes and layout
(including the span, label, SortIcon, and conditional right-alignment), attach
onClick={() => toggleSort(col)} to the button, ensure the button has
type="button" and appropriate styling/classes so it looks like the current
header, and add an aria-sort attribute on the <th> or button (based on
SortIcon/col state) to expose sort direction for accessibility.
- Around line 950-954: The effect currently scrolls whenever data refreshes
while orchestratorsOpen is true; change it to only scroll on the open transition
by tracking the previous open state (e.g., a ref like prevOrchestratorsOpenRef)
and only calling orchestratorsPanelRef.current?.scrollIntoView(...) when
orchestratorsOpen is true AND prevOrchestratorsOpenRef.current is false and
data?.orchestrators?.length > 0; update the ref after the check so subsequent
data updates don't re-trigger scrolling.
In `@apps/web-next/src/app/api/v1/leaderboard/`[...path]/route.ts:
- Around line 14-20: The fetch to targetUrl has no timeout and can hang; update
the call in route.ts to use an AbortSignal timeout and combine it with the
incoming request signal (e.g., use AbortSignal.timeout(ms) and merge with the
request's signal) so the fetch is aborted after a fixed duration and also when
the client disconnects; locate the fetch call that passes method/headers/cache
and replace its signal handling to pass a combined AbortSignal (timeout +
request.signal) and handle the aborted error path appropriately.
In `@apps/web-next/src/app/api/v1/subgraph/route.ts`:
- Around line 34-44: The outbound fetch to targetUrl is missing a timeout; wrap
the fetch with an AbortController, pass controller.signal into fetch(targetUrl,
{..., signal}), start a setTimeout to call controller.abort() after an
appropriate timeout (e.g., 10s) and clear the timer on success, and handle the
abort case (detect AbortError from fetch) to return a proper timeout response;
update the block around request.text() / fetch(...) to use this controller and
timer.
- Around line 14-33: In the POST handler (exported async function POST) add
authentication and rate-limiting: call getAuthToken() at the top, pass the token
into validateSession() and return a 401 JSON NextResponse if validation fails;
then enforce per-IP rate limits using rateLimiters.api (consume a token for the
client IP derived from request.headers.get('x-forwarded-for') fallback to
request.ip or request.socket.remoteAddress) configured for ~100 requests/min and
return a 429 JSON NextResponse when the limit is exceeded; keep the existing
SUBGRAPH_API_KEY/subgraph proxying logic after these checks.
In `@apps/web-next/src/hooks/useDashboardQuery.ts`:
- Line 114: The current console.warn in useDashboardQuery uses
JSON.stringify(response.errors, null, 2) which can throw on non-serializable
values; wrap the serialization in a safe fallback (e.g., try JSON.stringify and
on failure fall back to String(response.errors) or util.inspect) so logging
never throws, and replace the direct call inside the console.warn in
useDashboardQuery with the safe-serialized value to avoid routing
partial-success responses into the catch block.
In `@apps/workflows/developer-web/src/api/leaderboard.ts`:
- Line 8: The BASE_URL constant is hardcoded which prevents staging/local
routing; change it to read from a configurable environment variable (e.g.,
LEADERBOARD_API_URL or VITE_LEADERBOARD_API_URL depending on your build system)
with the current 'https://leaderboard-api.livepeer.cloud' as a sensible
fallback, and update any references that import BASE_URL in this module
(leaderboard.ts) so the app can route to local/staging proxies without code
changes.
In `@apps/workflows/developer-web/src/components/models/ModelDetailPanel.tsx`:
- Around line 11-12: The onCreateKey prop on ModelDetailPanel has signature
(model: NetworkModel, gateway: GatewayOffer) => void but is being passed
directly to a button's onClick which expects (event: React.MouseEvent) => void;
fix by wrapping the prop call in a click handler (e.g. onClick={() =>
onCreateKey(model, gateway)}) inside the ModelDetailPanel render so the button
supplies no event to onCreateKey, or alternatively adapt the handler to accept
an event and forward to onCreateKey, referencing the onCreateKey prop and the
button's onClick usage in ModelDetailPanel to locate where to apply the wrapper.
In `@apps/workflows/developer-web/src/hooks/useNetworkCapabilities.ts`:
- Around line 219-229: The load function can be run concurrently by the poller
causing out-of-order state updates; introduce a serializing request id (or
"currentLoadId") stored in a ref used by the hook (e.g., requestIdRef) that you
increment at the start of load(), capture into a local myId, then after awaiting
Promise.all([...fetchPipelines(), fetchRegions(), fetchGPUMetrics(...),
fetchSLACompliance(...), fetchNetworkDemand(...)]) check if myId ===
requestIdRef.current and only then apply setState/setLoading; do the same fix
for the analogous load call at lines ~332-336 so any earlier/overlapped
responses are ignored (alternative: abort previous requests with AbortController
if the fetch helpers accept a signal).
In `@docs/development-setup.md`:
- Around line 136-138: Update the local environment example to include the new
L1_RPC_URL variable so the protocol-block proxy works locally; add a line like
L1_RPC_URL=<your-l1-rpc-url> alongside SUBGRAPH_API_KEY and SUBGRAPH_ID in the
docs snippet (refer to the env snippet showing SUBGRAPH_API_KEY and SUBGRAPH_ID)
and ensure the placeholder makes it clear it must be a full RPC HTTP endpoint.
In `@packages/plugin-sdk/src/contracts/__tests__/dashboard.test.ts`:
- Line 10: Tests are being conditionally skipped using the hasSubgraphApiKey
boolean (derived from SUBGRAPH_API_KEY); remove this gating and make the tests
deterministic by mocking/stubbing the subgraph/network layer used in the
dashboard contract tests instead of relying on an env var. Locate uses of
hasSubgraphApiKey in the dashboard.test.ts file and replace the skip logic with
mocks for the GraphQL/subgraph client used by the tests (or inject a fake
response provider) so the schema/query contract tests run in CI without network
access.
In `@plugins/dashboard-provider-mock/frontend/src/api/subgraph.ts`:
- Around line 180-181: The current expressions for totalEth and totalUsd use ||
which treats 0 as falsy and wrongly falls back; update the assignments in the
block that computes totalEth and totalUsd (the calls using
round2(toNumber(data?.protocol?.totalVolumeETH) || fallbackTotalEth) and
round2(toNumber(data?.protocol?.totalVolumeUSD) || fallbackTotalUsd)) to use the
nullish coalescing operator (??) so you use toNumber(...) ?? fallbackTotalEth
and toNumber(...) ?? fallbackTotalUsd, preserving valid zero totals.
In `@plugins/dashboard-provider-mock/frontend/src/provider.ts`:
- Around line 224-235: The weighted accumulator multiplies ratios by
row.known_sessions without guarding for null/undefined which can produce NaN;
update the block handling row.success_ratio, row.sla_score, and
row.no_swap_ratio to first validate/coerce row.known_sessions (e.g., const ks =
Number.isFinite(row.known_sessions) ? row.known_sessions : 0) and only perform
d.srSum += row.success_ratio * ks and d.srSessions += ks (and likewise for
d.slaSum/d.slaSessions and d.noSwapSum/d.noSwapSessions), or skip the
accumulation when ks is not a valid number so NaN cannot propagate.
- Around line 75-79: The function weightedSuccessRatio currently returns 1 when
there are no known_sessions, producing a misleading 100% KPI; update
weightedSuccessRatio to return a neutral value (e.g., 0.5) or null/undefined
instead of 1 when totalSessions === 0, and ensure the caller can handle that
neutral value; locate the function by name weightedSuccessRatio and replace the
early return of 1 with the chosen neutral fallback and update any
consumers/tests that assume a numeric 1.
In `@plugins/developer-api/frontend/src/hooks/useModelCatalog.ts`:
- Around line 103-108: latencyP50 is currently computed as the arithmetic mean
of latencies (variable latencies from modelGPURows), but p50 should be the
median; replace the mean calculation for latencyP50 with a median calculation:
sort latencies numerically, if odd length pick the middle element, if even take
the average of the two middle elements, and keep the result as a rounded number
(or null when no latencies). Update the logic around latencyP50 in
useModelCatalog (referencing modelGPURows, latencies, and latencyP50) to
implement this median computation.
---
Nitpick comments:
In `@apps/web-next/.env.local.example`:
- Around line 60-64: The SUBGRAPH_ID default is ambiguous; update the env
example to make where the default is applied explicit by either adding a
commented default value for SUBGRAPH_ID (e.g., "# SUBGRAPH_ID=FE63Ygkz...") so
developers can easily uncomment, or add a short note pointing to the application
code that injects the default (mentioning the /api/v1/subgraph handler or the
module that reads SUBGRAPH_ID) so it's clear whether the default is hardcoded or
must be provided; modify the block containing SUBGRAPH_API_KEY and SUBGRAPH_ID
accordingly.
In `@apps/web-next/src/app/api/v1/leaderboard/`[...path]/route.ts:
- Around line 9-11: The path segments from params are used directly to build
targetUrl which can allow traversal or protocol-injection; in the code that
computes pathString and targetUrl (look for params, pathString, and targetUrl in
route.ts) validate and sanitize each segment before joining: reject or return
400 for empty segments, "." or "..", segments starting with protocols (e.g.,
"http:", "https:"), or containing suspicious characters, and apply a safe
encoding (encode each segment or enforce an allowlist like /^[A-Za-z0-9-_]+$/)
before joining with '/' so targetUrl is built only from validated, encoded
segments against LEADERBOARD_API_URL.
In `@apps/workflows/developer-web/src/data/network-config.ts`:
- Around line 1-46: The comment about keeping mappings in sync only mentions
pipeline-config.ts but this config (PIPELINE_DISPLAY, MODEL_DISPLAY,
EXCLUDED_MODELS, etc.) is duplicated in
plugins/developer-api/frontend/src/data/model-catalog.ts; update the header
comment to either list that file as another location that must stay in sync or
refactor by consolidating these exports into a shared module and import them
here and in model-catalog.ts so PIPELINE_DISPLAY, MODEL_DISPLAY and
EXCLUDED_MODELS are single-source-of-truth.
In `@plugins/dashboard-provider-mock/frontend/src/__tests__/provider.test.ts`:
- Around line 450-453: Remove the redundant module-level afterEach that calls
vi.useRealTimers(); it's duplicated by the afterEach inside the
registerMockJobFeedEmitter describe block; locate the top-level afterEach
invocation (the one outside any describe) and delete it so only the
registerMockJobFeedEmitter's timer cleanup remains—do not touch the afterEach
inside registerMockJobFeedEmitter or any other cleanup in
registerMockDashboardProvider.
- Around line 280-295: The test "returns protocol null and errors when subgraph
or protocol-block fails" currently stubs global fetch via vi.stubGlobal which
can leak across tests; change it to a single-test override using
vi.spyOn(globalThis, 'fetch').mockImplementationOnce(...) (or mockImplementation
once on the existing spy) so the failure response is only applied for this
invocation, or explicitly restore the stub immediately after the test using
vi.restoreAllMocks()/vi.unstubAllGlobals(); update the test that calls
registerMockDashboardProvider and mockEventBus._invoke (DASHBOARD_QUERY_EVENT)
to use the one-off spy to ensure test isolation.
In `@plugins/dashboard-provider-mock/frontend/src/api/leaderboard.ts`:
- Around line 77-95: The URL query values are interpolated raw in
fetchNetworkDemand, fetchGPUMetrics, and fetchSLACompliance which can break for
special characters; update those functions to URL-encode their parameters (use
encodeURIComponent or equivalent) when building the request string passed to
apiFetch so `/api/network/demand?interval=...`,
`/api/gpu/metrics?time_range=...`, and `/api/sla/compliance?period=...` use
encoded values to ensure safe, correct requests.
- Line 14: The code hardcodes BASE_URL to the external leaderboard API; switch
to using the internal proxy by replacing BASE_URL with the proxy root
'/api/v1/leaderboard' and update any call sites that build paths (e.g., in this
file's apiFetch or functions referencing BASE_URL) so they construct requests
like '/api/v1/leaderboard/network/demand' (not '/api/network/demand') to avoid
CORS and use centralized LEADERBOARD_API_URL via the proxy; ensure functions
that concatenate paths handle leading/trailing slashes to prevent duplicate
slashes and that error handling remains consistent when calling the proxy.
In `@plugins/developer-api/frontend/src/data/model-catalog.ts`:
- Around line 1-45: This module duplicates constants (PIPELINE_DISPLAY,
MODEL_DISPLAY, EXCLUDED_MODELS, REALTIME_FPS_THRESHOLD, UNKNOWN_GPU) that are
also defined in apps/workflows/developer-web/src/data/network-config.ts; extract
these shared mappings into a single shared package or module (e.g., `@naap/types`
or a new shared config module), export the constants there, then replace the
local definitions in model-catalog.ts with imports from the new shared module
and update the other consumer (network-config.ts) to import the same source so
both locations reference a single canonical definition to prevent drift.
ℹ️ Review info
Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d7111060-409c-40cc-9b7b-8e65de5d6c11
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json,!package-lock.json
📒 Files selected for processing (34)
.env.exampleapps/web-next/.env.local.exampleapps/web-next/package.jsonapps/web-next/src/app/(dashboard)/dashboard/page.tsxapps/web-next/src/app/api/v1/leaderboard/[...path]/route.tsapps/web-next/src/app/api/v1/protocol-block/route.tsapps/web-next/src/app/api/v1/subgraph/route.tsapps/web-next/src/hooks/useDashboardQuery.tsapps/workflows/developer-web/src/api/leaderboard.tsapps/workflows/developer-web/src/components/models/CompareDrawer.tsxapps/workflows/developer-web/src/components/models/ModelCard.tsxapps/workflows/developer-web/src/components/models/ModelDetailPanel.tsxapps/workflows/developer-web/src/components/tabs/ModelsTab.tsxapps/workflows/developer-web/src/data/network-config.tsapps/workflows/developer-web/src/hooks/useApi.tsapps/workflows/developer-web/src/hooks/useNetworkCapabilities.tsapps/workflows/developer-web/src/utils/gpu.tsdocs/development-setup.mdpackages/plugin-sdk/src/contracts/__tests__/dashboard.test.tspackages/plugin-sdk/src/contracts/createDashboardProvider.tspackages/plugin-sdk/src/contracts/dashboard.tspackages/plugin-sdk/src/contracts/index.tspackages/plugin-sdk/src/index.tspackages/types/src/index.tsplugins/dashboard-provider-mock/frontend/src/__tests__/provider.test.tsplugins/dashboard-provider-mock/frontend/src/api/leaderboard.tsplugins/dashboard-provider-mock/frontend/src/api/subgraph.tsplugins/dashboard-provider-mock/frontend/src/data/mock-fees.tsplugins/dashboard-provider-mock/frontend/src/data/pipeline-config.tsplugins/dashboard-provider-mock/frontend/src/provider.tsplugins/developer-api/frontend/src/api/leaderboard.tsplugins/developer-api/frontend/src/data/model-catalog.tsplugins/developer-api/frontend/src/hooks/useModelCatalog.tsplugins/developer-api/frontend/src/pages/DeveloperView.tsx
💤 Files with no reviewable changes (1)
- apps/workflows/developer-web/src/hooks/useApi.ts
Comment on lines
+33
to
+42
| # SUBGRAPH | ||
| # ═══════════════════════════════════════════════════════════════════════════════ | ||
| # | ||
| # Used by dashboard-provider plugin for fees data (server-only; never exposed to browser). | ||
| # For local dev, set in apps/web-next/.env.local (this is the runtime env file). | ||
| # | ||
| # Option A: Build endpoint from key + id | ||
| SUBGRAPH_API_KEY= | ||
| SUBGRAPH_ID= | ||
|
|
There was a problem hiding this comment.
Consolidate duplicated SUBGRAPH_API_KEY / SUBGRAPH_ID entries.
Line 131 and Line 133 redefine keys already declared at Line 40 and Line 41. Duplicate keys in env templates are error-prone and can cause misconfiguration when copied into runtime files.
Suggested fix
-# ═══════════════════════════════════════════════════════════════════════════════
-# SUBGRAPH
-# ═══════════════════════════════════════════════════════════════════════════════
-#
-# Used by dashboard-provider-mock plugin for fees data (server-only; never exposed to browser).
-# For local dev, set in apps/web-next/.env.local (this is the runtime env file).
-#
-# Option A: Direct endpoint (recommended if available)
-# VITE_SUBGRAPH_ENDPOINT=https://gateway.thegraph.com/api/<key>/subgraphs/id/<id>
-#
-# Option B: Build endpoint from key + id
-SUBGRAPH_API_KEY=
-# Optional: falls back to project default subgraph ID when unset
-SUBGRAPH_ID=
+## NOTE:
+## Reuse the SUBGRAPH_API_KEY/SUBGRAPH_ID block above for both dashboard-provider
+## and dashboard-provider-mock flows; do not redeclare the keys.Also applies to: 120-134
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.env.example around lines 33 - 42, The .env.example contains duplicated
environment variables; remove the redundant declarations of SUBGRAPH_API_KEY and
SUBGRAPH_ID so each variable (SUBGRAPH_API_KEY, SUBGRAPH_ID) is declared only
once in the file, consolidating the entries (keep the descriptive comment block
and the primary definitions near the SUBGRAPH section and delete the later
duplicates around lines mentioned in the review) to avoid confusion when copying
into runtime .env files.
Comment on lines
+287
to
+297
| <button | ||
| onClick={onToggleOrchestrators} | ||
| title={orchestratorsOpen ? 'Hide raw data' : 'View raw orchestrator data'} | ||
| className={`p-0.5 rounded transition-colors ${ | ||
| orchestratorsOpen | ||
| ? 'text-foreground bg-muted' | ||
| : 'text-muted-foreground hover:text-foreground' | ||
| }`} | ||
| > | ||
| <List className="w-3 h-3" /> | ||
| </button> |
There was a problem hiding this comment.
Add explicit accessible names to icon-only buttons.
Both icon-only buttons should expose aria-label for screen readers.
Suggested labels
<button
onClick={onToggleOrchestrators}
title={orchestratorsOpen ? 'Hide raw data' : 'View raw orchestrator data'}
+ aria-label={orchestratorsOpen ? 'Hide raw orchestrator data' : 'View raw orchestrator data'}
className={`p-0.5 rounded transition-colors ${
orchestratorsOpen
? 'text-foreground bg-muted'
: 'text-muted-foreground hover:text-foreground'
}`}
> <button
onClick={onClose}
+ aria-label="Close orchestrator raw data panel"
className="p-0.5 text-muted-foreground hover:text-foreground transition-colors rounded"
>Also applies to: 812-817
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web-next/src/app/`(dashboard)/dashboard/page.tsx around lines 287 - 297,
The icon-only button that toggles orchestrator raw data (the JSX button using
onToggleOrchestrators and the orchestratorsOpen state with the List icon) lacks
an accessible name; add an explicit aria-label (or aria-labelledby) to that
button—e.g., aria-label="Toggle raw orchestrator data"—so screen readers can
announce its purpose, and apply the same fix to the other icon-only button
referenced (the one at the other location using a similar pattern on lines
812-817).
Comment on lines
+780
to
+788
| <th | ||
| className={`pb-2 font-medium cursor-pointer select-none hover:text-foreground transition-colors ${right ? 'text-right' : 'text-left'}`} | ||
| onClick={() => toggleSort(col)} | ||
| > | ||
| <span className={`inline-flex items-center gap-1 ${right ? 'flex-row-reverse' : ''}`}> | ||
| {label} | ||
| <SortIcon col={col} /> | ||
| </span> | ||
| </th> |
There was a problem hiding this comment.
Use a real interactive control for sortable headers.
onClick on <th> is mouse-centric and not keyboard-accessible. Put a <button type="button"> inside the header cell for proper accessibility semantics.
Suggested accessible header interaction
- <th
- className={`pb-2 font-medium cursor-pointer select-none hover:text-foreground transition-colors ${right ? 'text-right' : 'text-left'}`}
- onClick={() => toggleSort(col)}
- >
- <span className={`inline-flex items-center gap-1 ${right ? 'flex-row-reverse' : ''}`}>
- {label}
- <SortIcon col={col} />
- </span>
- </th>
+ <th className={`pb-2 font-medium ${right ? 'text-right' : 'text-left'}`}>
+ <button
+ type="button"
+ onClick={() => toggleSort(col)}
+ className={`inline-flex items-center gap-1 select-none hover:text-foreground transition-colors ${right ? 'flex-row-reverse' : ''}`}
+ aria-label={`Sort by ${label}`}
+ >
+ {label}
+ <SortIcon col={col} />
+ </button>
+ </th>📝 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
| <th | |
| className={`pb-2 font-medium cursor-pointer select-none hover:text-foreground transition-colors ${right ? 'text-right' : 'text-left'}`} | |
| onClick={() => toggleSort(col)} | |
| > | |
| <span className={`inline-flex items-center gap-1 ${right ? 'flex-row-reverse' : ''}`}> | |
| {label} | |
| <SortIcon col={col} /> | |
| </span> | |
| </th> | |
| <th className={`pb-2 font-medium ${right ? 'text-right' : 'text-left'}`}> | |
| <button | |
| type="button" | |
| onClick={() => toggleSort(col)} | |
| className={`inline-flex items-center gap-1 select-none hover:text-foreground transition-colors ${right ? 'flex-row-reverse' : ''}`} | |
| aria-label={`Sort by ${label}`} | |
| > | |
| {label} | |
| <SortIcon col={col} /> | |
| </button> | |
| </th> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web-next/src/app/`(dashboard)/dashboard/page.tsx around lines 780 - 788,
The sortable header currently uses onClick on the <th>, which is not
keyboard-accessible; move the interactive behavior into a real <button
type="button"> inside the <th>: keep the existing classes and layout (including
the span, label, SortIcon, and conditional right-alignment), attach onClick={()
=> toggleSort(col)} to the button, ensure the button has type="button" and
appropriate styling/classes so it looks like the current header, and add an
aria-sort attribute on the <th> or button (based on SortIcon/col state) to
expose sort direction for accessibility.
Comment on lines
+950
to
+954
| useEffect(() => { | ||
| if (orchestratorsOpen && data?.orchestrators && data.orchestrators.length > 0) { | ||
| orchestratorsPanelRef.current?.scrollIntoView({ behavior: 'smooth', block: 'start' }); | ||
| } | ||
| }, [orchestratorsOpen, data?.orchestrators]); |
There was a problem hiding this comment.
Prevent repeated auto-scroll while the orchestrator panel is already open.
This effect will fire on every data refresh while orchestratorsOpen is true, causing repeated scroll jumps during polling.
Suggested guard to only scroll on open transition
+ const didAutoScrollRef = useRef(false);
useEffect(() => {
- if (orchestratorsOpen && data?.orchestrators && data.orchestrators.length > 0) {
+ if (!orchestratorsOpen) {
+ didAutoScrollRef.current = false;
+ return;
+ }
+ if (!didAutoScrollRef.current && data?.orchestrators && data.orchestrators.length > 0) {
orchestratorsPanelRef.current?.scrollIntoView({ behavior: 'smooth', block: 'start' });
+ didAutoScrollRef.current = true;
}
- }, [orchestratorsOpen, data?.orchestrators]);
+ }, [orchestratorsOpen, data?.orchestrators?.length]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web-next/src/app/`(dashboard)/dashboard/page.tsx around lines 950 - 954,
The effect currently scrolls whenever data refreshes while orchestratorsOpen is
true; change it to only scroll on the open transition by tracking the previous
open state (e.g., a ref like prevOrchestratorsOpenRef) and only calling
orchestratorsPanelRef.current?.scrollIntoView(...) when orchestratorsOpen is
true AND prevOrchestratorsOpenRef.current is false and
data?.orchestrators?.length > 0; update the ref after the check so subsequent
data updates don't re-trigger scrolling.
Comment on lines
+14
to
+20
| const response = await fetch(targetUrl, { | ||
| method: 'GET', | ||
| headers: { | ||
| Accept: 'application/json', | ||
| }, | ||
| cache: 'no-store', | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, examine the actual route file
cat -n apps/web-next/src/app/api/v1/leaderboard/[...path]/route.tsRepository: livepeer/naap
Length of output: 1748
🌐 Web query:
AbortSignal.timeout Node.js version support compatibility
💡 Result:
AbortSignal.timeout(delay) is supported in Node.js starting with:
- Node.js v16.14.0+ (v16 LTS line)
- Node.js v17.3.0+ (v17 current line at the time)
So it’s available in all newer majors (v18, v20, v22, …) as long as they’re not older patch/minor releases than the above introduction points. [1]
Docs/reference: Node.js “Globals” (AbortSignal.timeout(delay) “Added in: v17.3.0, v16.14.0”). [1]
(Also standardized on the web platform as a static AbortSignal.timeout() method.) [2]
Practical check:
const hasTimeout = typeof AbortSignal?.timeout === 'function';If you must support older Node (e.g., Node 14 / early 16): you’ll need a small fallback (e.g., AbortController + setTimeout) or a polyfill.
Sources:
[1] Node.js globals documentation (v18.16.0): AbortSignal.timeout(delay) history and “Added in” versions. (nodejs.org)
[2] MDN: AbortSignal.timeout() static method overview/spec context. (developer.mozilla.org)
Citations:
- 1: https://nodejs.org/download/release/v18.16.0/docs/api/globals.html?utm_source=openai
- 2: https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/timeout_static?utm_source=openai
🌐 Web query:
Next.js fetch timeout best practices AbortSignal
💡 Result:
Best practices for fetch timeouts in Next.js (using AbortSignal)
-
Prefer standards-based cancellation (
signal) over library-specifictimeoutoptions
In Next.js (Node runtime or Edge), use the Fetch standardsignalto cancel/timeout requests. -
Use
AbortSignal.timeout(ms)when it’s supported in your runtime
It returns a signal that aborts automatically after active time and throws aTimeoutError. You can pass it directly tofetch. (developer.mozilla.org)const res = await fetch(url, { signal: AbortSignal.timeout(5_000) });
-
For maximum compatibility (especially on Node), use
AbortController+setTimeout
This avoids edge cases around runtime support/behavior ofAbortSignal.timeout()(there have been undici/Node fetch issues historically). (github.com)const controller = new AbortController(); const t = setTimeout(() => controller.abort(), 5_000); try { return await fetch(url, { signal: controller.signal }); } finally { clearTimeout(t); // prevent timer leaks }
-
If you also want to respect client disconnect/cancellation, combine signals
Next.js APIs often provide a request-linked signal (e.g., Route Handlers receiveRequestwithrequest.signal). Combine it with your timeout so either cancels the upstream call. MDN documentsAbortSignal.any()for this pattern. (developer.mozilla.org)const timeout = AbortSignal.timeout(5_000); const signal = AbortSignal.any([request.signal, timeout]); const res = await fetch(url, { signal });
-
Don’t confuse “fetch timeout” with “function/runtime timeout”
A fetch timeout only cancels the outbound request; your deployment platform may still hard-stop the whole request after its function max duration. Next.js exposesmaxDurationso platforms can apply limits. (nextjs.org)
On Vercel, function duration has defaults/max limits and can returnFUNCTION_INVOCATION_TIMEOUTif exceeded. (vercel.com)
If the work can exceed platform limits, move it to a background job/queue and return a job id.
If you tell me whether you’re running Route Handlers, Server Actions, or Server Components, and whether you deploy to Vercel/Node server/Edge, I can suggest the most idiomatic snippet for that specific context.
Citations:
- 1: https://developer.mozilla.org/docs/Web/API/AbortSignal/timeout_static?utm_source=openai
- 2: fetch() AbortSignal.timeout() does not work as expected nodejs/undici#1926
- 3: https://developer.mozilla.org/docs/Web/API/AbortSignal/timeout_static?utm_source=openai
- 4: https://nextjs.org/docs/app/api-reference/file-conventions/route-segment-config?utm_source=openai
- 5: https://vercel.com/docs/functions/limitations?utm_source=openai
Add a timeout to the fetch request to prevent indefinite hangs.
The proxy fetch has no timeout configured. If the leaderboard API becomes unresponsive, requests will hang until the client times out. Use AbortSignal.timeout() to abort the request after a specified duration.
🛡️ Proposed fix with timeout
const response = await fetch(targetUrl, {
method: 'GET',
headers: {
Accept: 'application/json',
},
cache: 'no-store',
+ signal: AbortSignal.timeout(10000), // 10 second timeout
});For more robust handling that also respects client disconnection, combine the timeout with the request signal:
- signal: AbortSignal.timeout(10000),
+ signal: AbortSignal.any([request.signal, AbortSignal.timeout(10000)]),📝 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 response = await fetch(targetUrl, { | |
| method: 'GET', | |
| headers: { | |
| Accept: 'application/json', | |
| }, | |
| cache: 'no-store', | |
| }); | |
| const response = await fetch(targetUrl, { | |
| method: 'GET', | |
| headers: { | |
| Accept: 'application/json', | |
| }, | |
| cache: 'no-store', | |
| signal: AbortSignal.timeout(10000), // 10 second timeout | |
| }); |
🤖 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/leaderboard/`[...path]/route.ts around lines 14
- 20, The fetch to targetUrl has no timeout and can hang; update the call in
route.ts to use an AbortSignal timeout and combine it with the incoming request
signal (e.g., use AbortSignal.timeout(ms) and merge with the request's signal)
so the fetch is aborted after a fixed duration and also when the client
disconnects; locate the fetch call that passes method/headers/cache and replace
its signal handling to pass a combined AbortSignal (timeout + request.signal)
and handle the aborted error path appropriately.
|
|
||
| import { describe, it, expect, vi, beforeEach } from 'vitest'; | ||
|
|
||
| const hasSubgraphApiKey = !!process.env.SUBGRAPH_API_KEY; |
There was a problem hiding this comment.
Don’t gate pure unit tests behind SUBGRAPH_API_KEY.
Line 99 and Line 269 now skip tests that do not require network access. This makes schema/query contract coverage environment-dependent and can mask regressions in CI.
Suggested fix
-const hasSubgraphApiKey = !!process.env.SUBGRAPH_API_KEY;
@@
- it.skipIf(!hasSubgraphApiKey)('includes all expected types', () => {
+ it('includes all expected types', () => {
@@
- it.skipIf(!hasSubgraphApiKey)('supports variables in queries', async () => {
+ it('supports variables in queries', async () => {Also applies to: 99-99, 269-269
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/plugin-sdk/src/contracts/__tests__/dashboard.test.ts` at line 10,
Tests are being conditionally skipped using the hasSubgraphApiKey boolean
(derived from SUBGRAPH_API_KEY); remove this gating and make the tests
deterministic by mocking/stubbing the subgraph/network layer used in the
dashboard contract tests instead of relying on an env var. Locate uses of
hasSubgraphApiKey in the dashboard.test.ts file and replace the skip logic with
mocks for the GraphQL/subgraph client used by the tests (or inject a fake
response provider) so the schema/query contract tests run in CI without network
access.
Comment on lines
+180
to
+181
| totalEth: round2(toNumber(data?.protocol?.totalVolumeETH) || fallbackTotalEth), | ||
| totalUsd: round2(toNumber(data?.protocol?.totalVolumeUSD) || fallbackTotalUsd), |
There was a problem hiding this comment.
Use nullish checks instead of || for protocol totals.
A valid 0 total is treated as falsy and replaced by fallback aggregates.
Suggested fix
+ const protocolTotalEth = data?.protocol?.totalVolumeETH;
+ const protocolTotalUsd = data?.protocol?.totalVolumeUSD;
+
return {
- totalEth: round2(toNumber(data?.protocol?.totalVolumeETH) || fallbackTotalEth),
- totalUsd: round2(toNumber(data?.protocol?.totalVolumeUSD) || fallbackTotalUsd),
+ totalEth: protocolTotalEth != null ? round2(toNumber(protocolTotalEth)) : fallbackTotalEth,
+ totalUsd: protocolTotalUsd != null ? round2(toNumber(protocolTotalUsd)) : fallbackTotalUsd,
oneDayVolumeUsd: round2(dayForDisplay?.volumeUsd ?? 0),📝 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
| totalEth: round2(toNumber(data?.protocol?.totalVolumeETH) || fallbackTotalEth), | |
| totalUsd: round2(toNumber(data?.protocol?.totalVolumeUSD) || fallbackTotalUsd), | |
| const protocolTotalEth = data?.protocol?.totalVolumeETH; | |
| const protocolTotalUsd = data?.protocol?.totalVolumeUSD; | |
| return { | |
| totalEth: protocolTotalEth != null ? round2(toNumber(protocolTotalEth)) : fallbackTotalEth, | |
| totalUsd: protocolTotalUsd != null ? round2(toNumber(protocolTotalUsd)) : fallbackTotalUsd, | |
| oneDayVolumeUsd: round2(dayForDisplay?.volumeUsd ?? 0), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/dashboard-provider-mock/frontend/src/api/subgraph.ts` around lines
180 - 181, The current expressions for totalEth and totalUsd use || which treats
0 as falsy and wrongly falls back; update the assignments in the block that
computes totalEth and totalUsd (the calls using
round2(toNumber(data?.protocol?.totalVolumeETH) || fallbackTotalEth) and
round2(toNumber(data?.protocol?.totalVolumeUSD) || fallbackTotalUsd)) to use the
nullish coalescing operator (??) so you use toNumber(...) ?? fallbackTotalEth
and toNumber(...) ?? fallbackTotalUsd, preserving valid zero totals.
Comment on lines
+75
to
+79
| function weightedSuccessRatio(rows: Array<{ success_ratio: number; known_sessions: number }>): number { | ||
| const totalSessions = rows.reduce((s, r) => s + r.known_sessions, 0); | ||
| if (totalSessions === 0) return 1; | ||
| return rows.reduce((s, r) => s + r.success_ratio * r.known_sessions, 0) / totalSessions; | ||
| } |
There was a problem hiding this comment.
Avoid reporting 100% success when no sessions exist.
Returning 1 for empty data makes KPI show a false-perfect 100%. This should fall back to a neutral value.
Suggested fix
function weightedSuccessRatio(rows: Array<{ success_ratio: number; known_sessions: number }>): number {
const totalSessions = rows.reduce((s, r) => s + r.known_sessions, 0);
- if (totalSessions === 0) return 1;
+ if (totalSessions === 0) return 0;
return rows.reduce((s, r) => s + r.success_ratio * r.known_sessions, 0) / totalSessions;
}📝 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
| function weightedSuccessRatio(rows: Array<{ success_ratio: number; known_sessions: number }>): number { | |
| const totalSessions = rows.reduce((s, r) => s + r.known_sessions, 0); | |
| if (totalSessions === 0) return 1; | |
| return rows.reduce((s, r) => s + r.success_ratio * r.known_sessions, 0) / totalSessions; | |
| } | |
| function weightedSuccessRatio(rows: Array<{ success_ratio: number; known_sessions: number }>): number { | |
| const totalSessions = rows.reduce((s, r) => s + r.known_sessions, 0); | |
| if (totalSessions === 0) return 0; | |
| return rows.reduce((s, r) => s + r.success_ratio * r.known_sessions, 0) / totalSessions; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/dashboard-provider-mock/frontend/src/provider.ts` around lines 75 -
79, The function weightedSuccessRatio currently returns 1 when there are no
known_sessions, producing a misleading 100% KPI; update weightedSuccessRatio to
return a neutral value (e.g., 0.5) or null/undefined instead of 1 when
totalSessions === 0, and ensure the caller can handle that neutral value; locate
the function by name weightedSuccessRatio and replace the early return of 1 with
the chosen neutral fallback and update any consumers/tests that assume a numeric
1.
Comment on lines
+224
to
+235
| if (row.success_ratio != null) { | ||
| d.srSum += row.success_ratio * row.known_sessions; | ||
| d.srSessions += row.known_sessions; | ||
| } | ||
| if (row.sla_score != null) { | ||
| d.slaSum += row.sla_score * row.known_sessions; | ||
| d.slaSessions += row.known_sessions; | ||
| } | ||
| if (row.no_swap_ratio != null) { | ||
| d.noSwapSum += row.no_swap_ratio * row.known_sessions; | ||
| d.noSwapSessions += row.known_sessions; | ||
| } |
There was a problem hiding this comment.
Guard weighted math against missing known_sessions to prevent NaN propagation.
The weighted accumulators multiply by row.known_sessions directly. If it is null/undefined, ratios can become NaN and leak into UI/API.
Suggested fix
- if (row.success_ratio != null) {
- d.srSum += row.success_ratio * row.known_sessions;
- d.srSessions += row.known_sessions;
+ const knownSessions = row.known_sessions ?? 0;
+
+ if (row.success_ratio != null && knownSessions > 0) {
+ d.srSum += row.success_ratio * knownSessions;
+ d.srSessions += knownSessions;
}
- if (row.sla_score != null) {
- d.slaSum += row.sla_score * row.known_sessions;
- d.slaSessions += row.known_sessions;
+ if (row.sla_score != null && knownSessions > 0) {
+ d.slaSum += row.sla_score * knownSessions;
+ d.slaSessions += knownSessions;
}
- if (row.no_swap_ratio != null) {
- d.noSwapSum += row.no_swap_ratio * row.known_sessions;
- d.noSwapSessions += row.known_sessions;
+ if (row.no_swap_ratio != null && knownSessions > 0) {
+ d.noSwapSum += row.no_swap_ratio * knownSessions;
+ d.noSwapSessions += knownSessions;
}📝 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
| if (row.success_ratio != null) { | |
| d.srSum += row.success_ratio * row.known_sessions; | |
| d.srSessions += row.known_sessions; | |
| } | |
| if (row.sla_score != null) { | |
| d.slaSum += row.sla_score * row.known_sessions; | |
| d.slaSessions += row.known_sessions; | |
| } | |
| if (row.no_swap_ratio != null) { | |
| d.noSwapSum += row.no_swap_ratio * row.known_sessions; | |
| d.noSwapSessions += row.known_sessions; | |
| } | |
| const knownSessions = row.known_sessions ?? 0; | |
| if (row.success_ratio != null && knownSessions > 0) { | |
| d.srSum += row.success_ratio * knownSessions; | |
| d.srSessions += knownSessions; | |
| } | |
| if (row.sla_score != null && knownSessions > 0) { | |
| d.slaSum += row.sla_score * knownSessions; | |
| d.slaSessions += knownSessions; | |
| } | |
| if (row.no_swap_ratio != null && knownSessions > 0) { | |
| d.noSwapSum += row.no_swap_ratio * knownSessions; | |
| d.noSwapSessions += knownSessions; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/dashboard-provider-mock/frontend/src/provider.ts` around lines 224 -
235, The weighted accumulator multiplies ratios by row.known_sessions without
guarding for null/undefined which can produce NaN; update the block handling
row.success_ratio, row.sla_score, and row.no_swap_ratio to first validate/coerce
row.known_sessions (e.g., const ks = Number.isFinite(row.known_sessions) ?
row.known_sessions : 0) and only perform d.srSum += row.success_ratio * ks and
d.srSessions += ks (and likewise for d.slaSum/d.slaSessions and
d.noSwapSum/d.noSwapSessions), or skip the accumulation when ks is not a valid
number so NaN cannot propagate.
Comment on lines
+103
to
+108
| const latencies = modelGPURows | ||
| .map((row) => row.e2e_latency_ms) | ||
| .filter((value): value is number => value != null); | ||
| const latencyP50 = latencies.length | ||
| ? Math.round(latencies.reduce((sum, value) => sum + value, 0) / latencies.length) | ||
| : null; |
There was a problem hiding this comment.
latencyP50 is calculated as mean, not p50.
Line 107 currently computes an arithmetic average, which is incorrect for a p50 metric and can mislead model selection.
Suggested fix
- const latencyP50 = latencies.length
- ? Math.round(latencies.reduce((sum, value) => sum + value, 0) / latencies.length)
- : null;
+ const latencyP50 = (() => {
+ if (!latencies.length) return null;
+ const sorted = [...latencies].sort((a, b) => a - b);
+ const mid = Math.floor(sorted.length / 2);
+ if (sorted.length % 2 === 0) {
+ return Math.round((sorted[mid - 1] + sorted[mid]) / 2);
+ }
+ return Math.round(sorted[mid]);
+ })();📝 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 latencies = modelGPURows | |
| .map((row) => row.e2e_latency_ms) | |
| .filter((value): value is number => value != null); | |
| const latencyP50 = latencies.length | |
| ? Math.round(latencies.reduce((sum, value) => sum + value, 0) / latencies.length) | |
| : null; | |
| const latencies = modelGPURows | |
| .map((row) => row.e2e_latency_ms) | |
| .filter((value): value is number => value != null); | |
| const latencyP50 = (() => { | |
| if (!latencies.length) return null; | |
| const sorted = [...latencies].sort((a, b) => a - b); | |
| const mid = Math.floor(sorted.length / 2); | |
| if (sorted.length % 2 === 0) { | |
| return Math.round((sorted[mid - 1] + sorted[mid]) / 2); | |
| } | |
| return Math.round(sorted[mid]); | |
| })(); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/developer-api/frontend/src/hooks/useModelCatalog.ts` around lines 103
- 108, latencyP50 is currently computed as the arithmetic mean of latencies
(variable latencies from modelGPURows), but p50 should be the median; replace
the mean calculation for latencyP50 with a median calculation: sort latencies
numerically, if odd length pick the middle element, if even take the average of
the two middle elements, and keep the result as a rounded number (or null when
no latencies). Update the logic around latencyP50 in useModelCatalog
(referencing modelGPURows, latencies, and latencyP50) to implement this median
computation.
- Renamed the plugin from "Dashboard Provider (Mock)" to "Dashboard Provider" to reflect its new functionality. - Updated the description to indicate that it now serves live data from the Livepeer Leaderboard API, subgraph, and L1 RPC. - Removed mock data references and replaced them with real API calls, including job feed and KPI data. - Refactored code to remove mock data files and integrate new data generation logic for job events. - Enhanced README and configuration files to align with the new live data functionality and environment variable requirements.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
plugins/dashboard-provider-mock/frontend/src/provider.ts (2)
74-77:⚠️ Potential issue | 🟠 MajorAvoid reporting 100% success when there are no known sessions.
Returning
1here inflates KPI to a false-perfect baseline and distorts delta math.Suggested fix
function weightedSuccessRatio(rows: Array<{ success_ratio: number; known_sessions: number }>): number { const totalSessions = rows.reduce((s, r) => s + r.known_sessions, 0); - if (totalSessions === 0) return 1; + if (totalSessions === 0) return 0; return rows.reduce((s, r) => s + r.success_ratio * r.known_sessions, 0) / totalSessions; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/dashboard-provider-mock/frontend/src/provider.ts` around lines 74 - 77, The function weightedSuccessRatio currently returns 1 when there are zero known sessions, falsely reporting 100% success; in weightedSuccessRatio(rows) replace the guard "if (totalSessions === 0) return 1;" with a neutral/explicit value (e.g., return 0 or NaN) that does not inflate KPIs (choose return 0 for a conservative baseline) and ensure callers of weightedSuccessRatio handle that value appropriately; update the single-line guard in weightedSuccessRatio to return the chosen neutral value instead of 1.
223-234:⚠️ Potential issue | 🟠 MajorGuard weighted accumulators against invalid
known_sessionsvalues.These multiplications can leak
NaNinto orchestrator metrics when payloads are malformed/nullish.Suggested fix
+ const knownSessions = + Number.isFinite(row.known_sessions) && row.known_sessions > 0 ? row.known_sessions : 0; + - if (row.success_ratio != null) { - d.srSum += row.success_ratio * row.known_sessions; - d.srSessions += row.known_sessions; + if (row.success_ratio != null && knownSessions > 0) { + d.srSum += row.success_ratio * knownSessions; + d.srSessions += knownSessions; } - if (row.sla_score != null) { - d.slaSum += row.sla_score * row.known_sessions; - d.slaSessions += row.known_sessions; + if (row.sla_score != null && knownSessions > 0) { + d.slaSum += row.sla_score * knownSessions; + d.slaSessions += knownSessions; } - if (row.no_swap_ratio != null) { - d.noSwapSum += row.no_swap_ratio * row.known_sessions; - d.noSwapSessions += row.known_sessions; + if (row.no_swap_ratio != null && knownSessions > 0) { + d.noSwapSum += row.no_swap_ratio * knownSessions; + d.noSwapSessions += knownSessions; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/dashboard-provider-mock/frontend/src/provider.ts` around lines 223 - 234, The weighted accumulator code can produce NaN if row.known_sessions is null/undefined/invalid; update each block that multiplies by row.known_sessions (the branches updating d.srSum/d.srSessions, d.slaSum/d.slaSessions, d.noSwapSum/d.noSwapSessions) to first validate that row.known_sessions is a finite non-negative number (e.g., Number.isFinite(row.known_sessions) && row.known_sessions >= 0) before doing the multiplication and increment, and skip or treat as 0 when the check fails so no NaN is introduced.
🧹 Nitpick comments (2)
plugins/dashboard-provider-mock/frontend/src/provider.ts (1)
5-7: Top-level resolver-source comment is stale.The comment says protocol/fees fall back to static values, but this file now resolves both from live subgraph/RPC sources.
Suggested doc fix
- * resolvers without one (protocol, fees, pricing) fall back to static values + * resolvers without one (pricing) fall back to static values🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/dashboard-provider-mock/frontend/src/provider.ts` around lines 5 - 7, Update the stale top-level comment in provider.ts that claims "resolvers without one (protocol, fees, pricing) fall back to static values" to accurately describe current behavior: state that protocol and fees resolvers (and any others) now fetch live data from the subgraph/RPC sources instead of using static defaults, and briefly note which resolver functions or exports perform live resolution (refer to the module-level comment above the resolver implementations and the resolver functions/exports named for protocol and fees in this file).plugins/dashboard-provider-mock/frontend/src/data/index.ts (1)
4-5: Clarify the header comment to avoid mixed messaging.This module re-exports local seed/simulated job data, so “All data comes from live APIs” reads contradictory.
Suggested wording tweak
- * Re-exports seed data used by the dashboard provider. - * All data comes from live APIs. Job feed uses seed data. + * Re-exports job-feed seed/simulation data used by the dashboard provider. + * Other dashboard cards are backed by live APIs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/dashboard-provider-mock/frontend/src/data/index.ts` around lines 4 - 5, Update the top-of-file comment in this module to remove the contradictory claim that "All data comes from live APIs" and clearly state that this file re-exports local seed/simulated job data used by the dashboard provider (including the job feed); replace the ambiguous sentence with a concise line such as "Re-exports local seed/simulated job data used by the dashboard provider (job feed is seeded), not live API responses" so callers understand the data is mocked.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/dashboard-provider-mock/frontend/src/__tests__/provider.test.ts`:
- Around line 4-9: Update the test file header comment in provider.test.ts to
reflect current behavior: replace the bullet claiming "Returns static fallbacks
for protocol / fees / pricing" with wording that indicates these tests validate
live-derived protocol and fees paths (e.g., "Validates live-derived
protocol/fees/pricing paths" or similar), so the top-of-file docstring matches
the actual test assertions and behavior in the suite.
---
Duplicate comments:
In `@plugins/dashboard-provider-mock/frontend/src/provider.ts`:
- Around line 74-77: The function weightedSuccessRatio currently returns 1 when
there are zero known sessions, falsely reporting 100% success; in
weightedSuccessRatio(rows) replace the guard "if (totalSessions === 0) return
1;" with a neutral/explicit value (e.g., return 0 or NaN) that does not inflate
KPIs (choose return 0 for a conservative baseline) and ensure callers of
weightedSuccessRatio handle that value appropriately; update the single-line
guard in weightedSuccessRatio to return the chosen neutral value instead of 1.
- Around line 223-234: The weighted accumulator code can produce NaN if
row.known_sessions is null/undefined/invalid; update each block that multiplies
by row.known_sessions (the branches updating d.srSum/d.srSessions,
d.slaSum/d.slaSessions, d.noSwapSum/d.noSwapSessions) to first validate that
row.known_sessions is a finite non-negative number (e.g.,
Number.isFinite(row.known_sessions) && row.known_sessions >= 0) before doing the
multiplication and increment, and skip or treat as 0 when the check fails so no
NaN is introduced.
---
Nitpick comments:
In `@plugins/dashboard-provider-mock/frontend/src/data/index.ts`:
- Around line 4-5: Update the top-of-file comment in this module to remove the
contradictory claim that "All data comes from live APIs" and clearly state that
this file re-exports local seed/simulated job data used by the dashboard
provider (including the job feed); replace the ambiguous sentence with a concise
line such as "Re-exports local seed/simulated job data used by the dashboard
provider (job feed is seeded), not live API responses" so callers understand the
data is mocked.
In `@plugins/dashboard-provider-mock/frontend/src/provider.ts`:
- Around line 5-7: Update the stale top-level comment in provider.ts that claims
"resolvers without one (protocol, fees, pricing) fall back to static values" to
accurately describe current behavior: state that protocol and fees resolvers
(and any others) now fetch live data from the subgraph/RPC sources instead of
using static defaults, and briefly note which resolver functions or exports
perform live resolution (refer to the module-level comment above the resolver
implementations and the resolver functions/exports named for protocol and fees
in this file).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cb8eeac9-d8a1-4ce4-a334-c3ab134aa2d3
📒 Files selected for processing (18)
plugins/dashboard-provider-mock/README.mdplugins/dashboard-provider-mock/frontend/index.htmlplugins/dashboard-provider-mock/frontend/src/App.tsxplugins/dashboard-provider-mock/frontend/src/__tests__/provider.test.tsplugins/dashboard-provider-mock/frontend/src/data/index.tsplugins/dashboard-provider-mock/frontend/src/data/jobs.tsplugins/dashboard-provider-mock/frontend/src/data/mock-fees.tsplugins/dashboard-provider-mock/frontend/src/data/mock-gpu.tsplugins/dashboard-provider-mock/frontend/src/data/mock-kpi.tsplugins/dashboard-provider-mock/frontend/src/data/mock-pipelines.tsplugins/dashboard-provider-mock/frontend/src/data/mock-pricing.tsplugins/dashboard-provider-mock/frontend/src/data/mock-protocol.tsplugins/dashboard-provider-mock/frontend/src/job-feed-emitter.tsplugins/dashboard-provider-mock/frontend/src/main.tsxplugins/dashboard-provider-mock/frontend/src/mount.tsxplugins/dashboard-provider-mock/frontend/src/provider.tsplugins/dashboard-provider-mock/frontend/vite.config.tsplugins/dashboard-provider-mock/plugin.json
💤 Files with no reviewable changes (6)
- plugins/dashboard-provider-mock/frontend/src/data/mock-pipelines.ts
- plugins/dashboard-provider-mock/frontend/src/data/mock-gpu.ts
- plugins/dashboard-provider-mock/frontend/src/data/mock-kpi.ts
- plugins/dashboard-provider-mock/frontend/src/data/mock-protocol.ts
- plugins/dashboard-provider-mock/frontend/src/data/mock-pricing.ts
- plugins/dashboard-provider-mock/frontend/src/data/mock-fees.ts
✅ Files skipped from review due to trivial changes (1)
- plugins/dashboard-provider-mock/frontend/src/mount.tsx
Comment on lines
+4
to
9
| * Tests that the provider correctly: | ||
| * 1. Registers as a dashboard:query handler | ||
| * 2. Responds with correct mock data shapes | ||
| * 2. Transforms leaderboard API responses into the dashboard contract shape | ||
| * 3. Handles partial queries | ||
| * 4. Registers as a job-feed:subscribe handler | ||
| * 4. Returns static fallbacks for protocol / fees / pricing | ||
| * 5. Cleans up handlers on unmount |
There was a problem hiding this comment.
Update the test docstring to match current behavior.
The header still says protocol/fees are static fallbacks, but these tests now validate live-derived protocol/fees paths. Keep the comment aligned to avoid confusion.
Suggested doc fix
- * 4. Returns static fallbacks for protocol / fees / pricing
+ * 4. Resolves protocol and fees from subgraph/L1 paths, with pricing fallback🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/dashboard-provider-mock/frontend/src/__tests__/provider.test.ts`
around lines 4 - 9, Update the test file header comment in provider.test.ts to
reflect current behavior: replace the bullet claiming "Returns static fallbacks
for protocol / fees / pricing" with wording that indicates these tests validate
live-derived protocol and fees paths (e.g., "Validates live-derived
protocol/fees/pricing paths" or similar), so the top-of-file docstring matches
the actual test assertions and behavior in the suite.
…shboard-data-provider - Renamed the plugin from "dashboard-provider-mock" to "dashboard-data-provider" to reflect its new functionality. - Updated references throughout the codebase to align with the new plugin name. - Removed mock data files and integrated live data fetching from the Livepeer Leaderboard API and subgraph. - Enhanced documentation and environment configuration to support the new data provider setup. - Updated tests and example files to ensure compatibility with the new plugin structure.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web-next/src/content/docs/guides/dashboard-data-provider.mdx (1)
17-21:⚠️ Potential issue | 🟡 MinorQuick Start wording is stale after provider rename
Line 17 still says “mock provider plugin,” but Line 21 points to
plugins/dashboard-data-provider. Please align the text so users don’t think this guide is still mock-data-first.Suggested doc tweak
-The fastest way to get started is to clone the mock provider plugin: +The fastest way to get started is to clone the reference dashboard data provider plugin:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web-next/src/content/docs/guides/dashboard-data-provider.mdx` around lines 17 - 21, Update the wording so it no longer calls the example a “mock provider plugin”: change the phrase "mock provider plugin" to "dashboard data provider plugin" (or just "provider plugin") and ensure the example path remains consistent (e.g., cp -r plugins/dashboard-data-provider plugins/my-network-provider) so the text and the referenced plugin directory name "dashboard-data-provider" align; edit the sentence containing "mock provider plugin" and the copy command text to use the updated term.
♻️ Duplicate comments (6)
.env.example (1)
120-134:⚠️ Potential issue | 🟠 MajorRemove duplicated
SUBGRAPH_API_KEY/SUBGRAPH_IDdeclarations.Line 131 and Line 133 redeclare keys already defined at Line 40 and Line 41. Keeping both blocks invites misconfiguration when users copy values.
Proposed consolidation
-# ═══════════════════════════════════════════════════════════════════════════════ -# SUBGRAPH -# ═══════════════════════════════════════════════════════════════════════════════ -# -# Used by dashboard-network-data plugin for fees data (server-only; never exposed to browser). -# For local dev, set in apps/web-next/.env.local (this is the runtime env file). -# -# Option A: Direct endpoint (recommended if available) -# VITE_SUBGRAPH_ENDPOINT=https://gateway.thegraph.com/api/<key>/subgraphs/id/<id> -# -# Option B: Build endpoint from key + id -SUBGRAPH_API_KEY= -# Optional: falls back to project default subgraph ID when unset -SUBGRAPH_ID= +# NOTE: +# Reuse the SUBGRAPH_API_KEY/SUBGRAPH_ID block above; do not redeclare keys.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.env.example around lines 120 - 134, The .env.example contains duplicate declarations of SUBGRAPH_API_KEY and SUBGRAPH_ID; remove the repeated entries in this SUBGRAPH block and keep only the single canonical declarations (SUBGRAPH_API_KEY and SUBGRAPH_ID) defined earlier to avoid confusion when users copy values, ensuring the file contains one clear place to set these variables and any explanatory comments remain with that single declaration.apps/workflows/developer-web/src/api/leaderboard.ts (1)
8-8:⚠️ Potential issue | 🟠 MajorUse a configurable/proxied base URL instead of a hardcoded production host.
Line 8 still hardcodes production and bypasses environment/proxy routing, which makes local/staging deployments brittle and ignores the new proxy path introduced in this PR.
Suggested fix
-const BASE_URL = 'https://leaderboard-api.livepeer.cloud'; +const BASE_URL = + import.meta.env.VITE_LEADERBOARD_API_URL?.replace(/\/$/, '') ?? + '/api/v1/leaderboard';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/workflows/developer-web/src/api/leaderboard.ts` at line 8, Replace the hardcoded BASE_URL constant with a configurable/proxied value: read from a public env var (e.g. NEXT_PUBLIC_LEADERBOARD_BASE_URL) and fall back to the local/proxy path (e.g. '/api/leaderboard') so dev/staging use the proxy while production can override; update the BASE_URL declaration in apps/workflows/developer-web/src/api/leaderboard.ts accordingly and ensure any consumers of BASE_URL continue to work with the new value.plugins/dashboard-data-provider/frontend/src/__tests__/provider.test.ts (1)
4-9:⚠️ Potential issue | 🟡 MinorUpdate the stale test header behavior summary.
This header still says protocol/fees/pricing are static fallbacks, but the suite now validates live-derived protocol/fees paths and pricing fallback only.
📝 Proposed doc fix
- * 4. Returns static fallbacks for protocol / fees / pricing + * 4. Resolves protocol and fees from subgraph/L1 paths; pricing uses static fallback🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/dashboard-data-provider/frontend/src/__tests__/provider.test.ts` around lines 4 - 9, Update the stale test header comment in provider.test.ts to reflect current behavior: change the bullet about "Returns static fallbacks for protocol / fees / pricing" to state that the suite validates live-derived protocol and fees paths and only uses a pricing fallback, so the header accurately lists the five checks as (1) registers as a dashboard:query handler, (2) transforms leaderboard API responses into the dashboard contract shape, (3) handles partial queries, (4) validates live-derived protocol and fees paths (and pricing fallback only), and (5) cleans up handlers on unmount.plugins/dashboard-data-provider/frontend/src/api/subgraph.ts (1)
179-181:⚠️ Potential issue | 🟡 MinorPreserve valid zero protocol totals instead of falling back.
Line 180 and Line 181 use
||, so a legitimate0total gets replaced by fallback totals.Suggested fix
+ const protocolTotalEth = data?.protocol?.totalVolumeETH; + const protocolTotalUsd = data?.protocol?.totalVolumeUSD; + return { - totalEth: round2(toNumber(data?.protocol?.totalVolumeETH) || fallbackTotalEth), - totalUsd: round2(toNumber(data?.protocol?.totalVolumeUSD) || fallbackTotalUsd), + totalEth: protocolTotalEth != null ? round2(toNumber(protocolTotalEth)) : fallbackTotalEth, + totalUsd: protocolTotalUsd != null ? round2(toNumber(protocolTotalUsd)) : fallbackTotalUsd, oneDayVolumeUsd: round2(dayForDisplay?.volumeUsd ?? 0),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/dashboard-data-provider/frontend/src/api/subgraph.ts` around lines 179 - 181, The code computing totalEth and totalUsd uses the || operator which treats legitimate zero values as falsy and replaces them with fallbacks; update the expressions that build totalEth and totalUsd (the lines using round2(toNumber(data?.protocol?.totalVolumeETH) || fallbackTotalEth) and round2(toNumber(data?.protocol?.totalVolumeUSD) || fallbackTotalUsd)) to use a nullish-coalescing or explicit null/undefined check (e.g., use ?? with the toNumber result or check for value == null) so that 0 is preserved while only undefined/null causes fallback.plugins/dashboard-data-provider/frontend/src/provider.ts (2)
223-234:⚠️ Potential issue | 🟠 MajorGuard weighted math with a validated
known_sessionsvalue.Line 224–Line 234 use
row.known_sessionsdirectly; if it is absent, weighted stats can collapse incorrectly.Suggested fix
+ const knownSessions = row.known_sessions ?? 0; + - if (row.success_ratio != null) { - d.srSum += row.success_ratio * row.known_sessions; - d.srSessions += row.known_sessions; + if (row.success_ratio != null && knownSessions > 0) { + d.srSum += row.success_ratio * knownSessions; + d.srSessions += knownSessions; } - if (row.sla_score != null) { - d.slaSum += row.sla_score * row.known_sessions; - d.slaSessions += row.known_sessions; + if (row.sla_score != null && knownSessions > 0) { + d.slaSum += row.sla_score * knownSessions; + d.slaSessions += knownSessions; } - if (row.no_swap_ratio != null) { - d.noSwapSum += row.no_swap_ratio * row.known_sessions; - d.noSwapSessions += row.known_sessions; + if (row.no_swap_ratio != null && knownSessions > 0) { + d.noSwapSum += row.no_swap_ratio * knownSessions; + d.noSwapSessions += knownSessions; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/dashboard-data-provider/frontend/src/provider.ts` around lines 223 - 234, The weighted aggregations use row.known_sessions without validation, so validate and guard on a numeric positive known_sessions before doing the weighted math: compute a local let sessions = Number(row.known_sessions) and check Number.isFinite(sessions) && sessions > 0 (or treat NaN/undefined as 0), then only update d.srSum/d.srSessions, d.slaSum/d.slaSessions, and d.noSwapSum/d.noSwapSessions when sessions is valid; reference the existing variables row.known_sessions and the accumulator object d to locate where to add the guard and replace direct uses of row.known_sessions with the validated sessions variable.
74-77:⚠️ Potential issue | 🟠 MajorDo not return perfect success for empty windows.
Line 76 returns
1when there are no sessions, which surfaces as a false 100% KPI.Suggested fix
function weightedSuccessRatio(rows: Array<{ success_ratio: number; known_sessions: number }>): number { const totalSessions = rows.reduce((s, r) => s + r.known_sessions, 0); - if (totalSessions === 0) return 1; + if (totalSessions === 0) return 0; return rows.reduce((s, r) => s + r.success_ratio * r.known_sessions, 0) / totalSessions; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/dashboard-data-provider/frontend/src/provider.ts` around lines 74 - 77, The function weightedSuccessRatio currently returns 1 when totalSessions === 0, which incorrectly reports 100% success for empty windows; update weightedSuccessRatio to return 0 (or another neutral/empty indicator such as null if preferred by callers) instead of 1, and ensure callers handle that value; modify the guard in weightedSuccessRatio (the totalSessions === 0 branch) to return the chosen neutral value so the final weighted average is not artificially 100%.
🧹 Nitpick comments (4)
apps/workflows/developer-web/src/data/network-config.ts (1)
9-46: Deduplicate catalog constants to avoid drift across apps.These exports are duplicated with
plugins/developer-api/frontend/src/data/model-catalog.ts(same keys/values). Please move them to a shared module/package and import from both locations to keep one source of truth.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/workflows/developer-web/src/data/network-config.ts` around lines 9 - 46, Move the duplicated catalog constants into a single shared module and import them where needed: create a new shared export (e.g., export const PIPELINE_DISPLAY, MODEL_DISPLAY, EXCLUDED_MODELS, REALTIME_FPS_THRESHOLD, UNKNOWN_GPU) preserving the types (Record<string, string | null> for MODEL_DISPLAY and Set for EXCLUDED_MODELS) and identical values, then in apps/workflows/developer-web/src/data/network-config.ts replace the local definitions of PIPELINE_DISPLAY, MODEL_DISPLAY, EXCLUDED_MODELS, REALTIME_FPS_THRESHOLD and UNKNOWN_GPU with imports from that shared module and update the other duplicate file (plugins/developer-api/frontend/src/data/model-catalog.ts) to import the same shared symbols so both consumers use the single source of truth.plugins/dashboard-data-provider/frontend/src/api/leaderboard.ts (1)
20-65: Avoid local contract drift by reusing shared leaderboard row types.These local interfaces already diverge from
apps/workflows/developer-web/src/api/leaderboard.ts(seeGPUMetricRowandSLAComplianceRowat Lines 26-74 there). Reuse a shared type source to prevent silent schema skew between plugins.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/dashboard-data-provider/frontend/src/api/leaderboard.ts` around lines 20 - 65, The local interfaces (NetworkDemandRow, GPUMetricRow, SLAComplianceRow) duplicate types maintained elsewhere and risk schema drift; remove these local type declarations and import the canonical types from the shared leaderboard type module, then update any references to use the imported GPUMetricRow, SLAComplianceRow and NetworkDemandRow symbols (or re-export them) so this plugin consumes the single source of truth rather than redefining the interfaces.plugins/dashboard-data-provider/frontend/src/__tests__/provider.test.ts (1)
162-163: Implementoffandonceto matchIEventBuscontract.The test double's
offandoncemethods are currently non-functional, which violates theIEventBusinterface contract even though they're not called in provider.test.ts. While this doesn't mask bugs in the current tests, implementing proper semantics improves test robustness and prevents future regressions if these methods are needed.The proposed implementation correctly reuses the existing
listenersMap infrastructure and returns an unsubscribe function fromonce, matching the contract defined in packages/plugin-sdk.♻️ Proposed refactor
- off: vi.fn(), - once: vi.fn(() => vi.fn()), + off: vi.fn((event: string, callback: (data: unknown) => void) => { + listeners.get(event)?.delete(callback); + }), + once: vi.fn((event: string, callback: (data: unknown) => void) => { + if (!listeners.has(event)) { + listeners.set(event, new Set()); + } + const wrapped = (data: unknown) => { + listeners.get(event)?.delete(wrapped); + callback(data); + }; + listeners.get(event)!.add(wrapped); + return () => { + listeners.get(event)?.delete(wrapped); + }; + }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/dashboard-data-provider/frontend/src/__tests__/provider.test.ts` around lines 162 - 163, The test double for the event bus violates the IEventBus contract because off and once are no-ops; implement off to remove a listener from the existing listeners Map (use the same Map used by on/emit) and implement once to register a wrapper that removes itself after first invocation and returns an unsubscribe function; update the test double's off and once functions (referencing the listeners Map, and method names off and once) so once reuses on/emit semantics and returns the unsubscribe closure.plugins/dashboard-data-provider/README.md (1)
31-33: Consider specifying a language for the fenced code block.The static analysis tool flagged this code block for missing language specification. Since this is an ASCII diagram, you can use
textorplaintextto satisfy the linter.📝 Suggested fix
-``` +```text Dashboard (core) ←— eventBus.request('dashboard:query', {query}) —→ This plugin -``` +```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/dashboard-data-provider/README.md` around lines 31 - 33, Update the fenced code block in README.md that currently shows the ASCII diagram "Dashboard (core) ←— eventBus.request('dashboard:query', {query}) —→ This plugin" to include a language specifier (e.g., "text" or "plaintext") after the opening triple backticks so the linter stops flagging it; locate the block in plugins/dashboard-data-provider/README.md and change the opening fence from ``` to ```text (or ```plaintext) while leaving the diagram content and closing fence unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.env.example:
- Around line 39-42: The first SUBGRAPH block currently lists SUBGRAPH_API_KEY
and SUBGRAPH_ID without noting that SUBGRAPH_ID is optional; add a short
clarifying comment alongside SUBGRAPH_ID (same text as the later duplicated
block) stating that the runtime will fall back to the default subgraph ID when
SUBGRAPH_ID is unset so readers know it’s optional; update the comment near
SUBGRAPH_ID (and keep it consistent with the duplicate block) to avoid
confusion.
In `@apps/workflows/developer-web/src/data/network-config.ts`:
- Around line 4-7: Update the header comment that currently tells maintainers to
"Keep in sync with pipeline-config.ts in dashboard-data-provider": clarify or
remove that guidance because the identifiers in this file (parent pipeline IDs
used by MODEL_DISPLAY) are different from the model/workload IDs in
dashboard-data-provider; specifically edit the comment near the MODEL_DISPLAY
mapping to either point to the correct canonical mapping file or explicitly note
the identifier differences (parent pipeline IDs vs model/workload IDs) so future
edits won’t attempt to mirror the wrong file.
In `@plugins/dashboard-data-provider/frontend/src/api/leaderboard.ts`:
- Line 14: The constant BASE_URL in leaderboard.ts is hardcoded to the
production host; change it to use a configurable/proxied base URL (e.g., read
from an environment variable like VITE_LEADERBOARD_API_BASE or fall back to a
relative/proxy path such as '/api/leaderboard') so staging/local builds and the
new server proxy route work; update the BASE_URL definition and any callers that
import it (BASE_URL) to use the env/relative value and ensure the default
preserves current production behavior if no override is provided.
- Around line 77-94: The three functions fetchNetworkDemand, fetchGPUMetrics,
and fetchSLACompliance build query strings by interpolating raw parameters into
the URL which can break for special characters; change each to build encoded
query parameters (e.g., via URLSearchParams or encodeURIComponent) and pass the
encoded query to apiFetch (e.g., construct params = new URLSearchParams({
interval }), then call `/api/network/demand?${params}`), repeating the same
pattern for time_range and period so requests are safe for special characters.
In `@plugins/dashboard-data-provider/frontend/src/provider.ts`:
- Around line 136-154: resolvePipelines currently passes the raw limit into
.slice(...), which allows negative values to produce unintended results; clamp
and normalize limit before slicing by computing a non-negative integer (e.g.,
let n = Math.max(0, Math.floor(limit || 0))) and use n in the final .slice call;
update the resolvePipelines function to validate/normalize the limit parameter
before the return pipeline array is sliced to ensure negative or non-integer
limits do not produce unexpected output.
---
Outside diff comments:
In `@apps/web-next/src/content/docs/guides/dashboard-data-provider.mdx`:
- Around line 17-21: Update the wording so it no longer calls the example a
“mock provider plugin”: change the phrase "mock provider plugin" to "dashboard
data provider plugin" (or just "provider plugin") and ensure the example path
remains consistent (e.g., cp -r plugins/dashboard-data-provider
plugins/my-network-provider) so the text and the referenced plugin directory
name "dashboard-data-provider" align; edit the sentence containing "mock
provider plugin" and the copy command text to use the updated term.
---
Duplicate comments:
In @.env.example:
- Around line 120-134: The .env.example contains duplicate declarations of
SUBGRAPH_API_KEY and SUBGRAPH_ID; remove the repeated entries in this SUBGRAPH
block and keep only the single canonical declarations (SUBGRAPH_API_KEY and
SUBGRAPH_ID) defined earlier to avoid confusion when users copy values, ensuring
the file contains one clear place to set these variables and any explanatory
comments remain with that single declaration.
In `@apps/workflows/developer-web/src/api/leaderboard.ts`:
- Line 8: Replace the hardcoded BASE_URL constant with a configurable/proxied
value: read from a public env var (e.g. NEXT_PUBLIC_LEADERBOARD_BASE_URL) and
fall back to the local/proxy path (e.g. '/api/leaderboard') so dev/staging use
the proxy while production can override; update the BASE_URL declaration in
apps/workflows/developer-web/src/api/leaderboard.ts accordingly and ensure any
consumers of BASE_URL continue to work with the new value.
In `@plugins/dashboard-data-provider/frontend/src/__tests__/provider.test.ts`:
- Around line 4-9: Update the stale test header comment in provider.test.ts to
reflect current behavior: change the bullet about "Returns static fallbacks for
protocol / fees / pricing" to state that the suite validates live-derived
protocol and fees paths and only uses a pricing fallback, so the header
accurately lists the five checks as (1) registers as a dashboard:query handler,
(2) transforms leaderboard API responses into the dashboard contract shape, (3)
handles partial queries, (4) validates live-derived protocol and fees paths (and
pricing fallback only), and (5) cleans up handlers on unmount.
In `@plugins/dashboard-data-provider/frontend/src/api/subgraph.ts`:
- Around line 179-181: The code computing totalEth and totalUsd uses the ||
operator which treats legitimate zero values as falsy and replaces them with
fallbacks; update the expressions that build totalEth and totalUsd (the lines
using round2(toNumber(data?.protocol?.totalVolumeETH) || fallbackTotalEth) and
round2(toNumber(data?.protocol?.totalVolumeUSD) || fallbackTotalUsd)) to use a
nullish-coalescing or explicit null/undefined check (e.g., use ?? with the
toNumber result or check for value == null) so that 0 is preserved while only
undefined/null causes fallback.
In `@plugins/dashboard-data-provider/frontend/src/provider.ts`:
- Around line 223-234: The weighted aggregations use row.known_sessions without
validation, so validate and guard on a numeric positive known_sessions before
doing the weighted math: compute a local let sessions =
Number(row.known_sessions) and check Number.isFinite(sessions) && sessions > 0
(or treat NaN/undefined as 0), then only update d.srSum/d.srSessions,
d.slaSum/d.slaSessions, and d.noSwapSum/d.noSwapSessions when sessions is valid;
reference the existing variables row.known_sessions and the accumulator object d
to locate where to add the guard and replace direct uses of row.known_sessions
with the validated sessions variable.
- Around line 74-77: The function weightedSuccessRatio currently returns 1 when
totalSessions === 0, which incorrectly reports 100% success for empty windows;
update weightedSuccessRatio to return 0 (or another neutral/empty indicator such
as null if preferred by callers) instead of 1, and ensure callers handle that
value; modify the guard in weightedSuccessRatio (the totalSessions === 0 branch)
to return the chosen neutral value so the final weighted average is not
artificially 100%.
---
Nitpick comments:
In `@apps/workflows/developer-web/src/data/network-config.ts`:
- Around line 9-46: Move the duplicated catalog constants into a single shared
module and import them where needed: create a new shared export (e.g., export
const PIPELINE_DISPLAY, MODEL_DISPLAY, EXCLUDED_MODELS, REALTIME_FPS_THRESHOLD,
UNKNOWN_GPU) preserving the types (Record<string, string | null> for
MODEL_DISPLAY and Set for EXCLUDED_MODELS) and identical values, then in
apps/workflows/developer-web/src/data/network-config.ts replace the local
definitions of PIPELINE_DISPLAY, MODEL_DISPLAY, EXCLUDED_MODELS,
REALTIME_FPS_THRESHOLD and UNKNOWN_GPU with imports from that shared module and
update the other duplicate file
(plugins/developer-api/frontend/src/data/model-catalog.ts) to import the same
shared symbols so both consumers use the single source of truth.
In `@plugins/dashboard-data-provider/frontend/src/__tests__/provider.test.ts`:
- Around line 162-163: The test double for the event bus violates the IEventBus
contract because off and once are no-ops; implement off to remove a listener
from the existing listeners Map (use the same Map used by on/emit) and implement
once to register a wrapper that removes itself after first invocation and
returns an unsubscribe function; update the test double's off and once functions
(referencing the listeners Map, and method names off and once) so once reuses
on/emit semantics and returns the unsubscribe closure.
In `@plugins/dashboard-data-provider/frontend/src/api/leaderboard.ts`:
- Around line 20-65: The local interfaces (NetworkDemandRow, GPUMetricRow,
SLAComplianceRow) duplicate types maintained elsewhere and risk schema drift;
remove these local type declarations and import the canonical types from the
shared leaderboard type module, then update any references to use the imported
GPUMetricRow, SLAComplianceRow and NetworkDemandRow symbols (or re-export them)
so this plugin consumes the single source of truth rather than redefining the
interfaces.
In `@plugins/dashboard-data-provider/README.md`:
- Around line 31-33: Update the fenced code block in README.md that currently
shows the ASCII diagram "Dashboard (core) ←—
eventBus.request('dashboard:query', {query}) —→ This plugin" to include a
language specifier (e.g., "text" or "plaintext") after the opening triple
backticks so the linter stops flagging it; locate the block in
plugins/dashboard-data-provider/README.md and change the opening fence from ```
to ```text (or ```plaintext) while leaving the diagram content and closing fence
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f3d6f5fb-9fd7-4bde-882b-3b14ffe8f852
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json,!package-lock.json
📒 Files selected for processing (28)
.env.exampleapps/web-next/prisma/seed.tsapps/web-next/src/app/cdn/plugins/__tests__/cdn-serve.test.tsapps/web-next/src/components/plugin/BackgroundPluginLoader.tsxapps/web-next/src/content/docs/guides/dashboard-data-provider.mdxapps/workflows/developer-web/src/api/leaderboard.tsapps/workflows/developer-web/src/data/network-config.tsbin/cleanup-moved-plugins.tspackages/database/src/plugin-discovery.tsplugins/dashboard-data-provider/README.mdplugins/dashboard-data-provider/frontend/index.htmlplugins/dashboard-data-provider/frontend/package.jsonplugins/dashboard-data-provider/frontend/src/App.tsxplugins/dashboard-data-provider/frontend/src/__tests__/provider.test.tsplugins/dashboard-data-provider/frontend/src/api/leaderboard.tsplugins/dashboard-data-provider/frontend/src/api/subgraph.tsplugins/dashboard-data-provider/frontend/src/data/index.tsplugins/dashboard-data-provider/frontend/src/data/jobs.tsplugins/dashboard-data-provider/frontend/src/data/pipeline-config.tsplugins/dashboard-data-provider/frontend/src/globals.cssplugins/dashboard-data-provider/frontend/src/job-feed-emitter.tsplugins/dashboard-data-provider/frontend/src/main.tsxplugins/dashboard-data-provider/frontend/src/mount.tsxplugins/dashboard-data-provider/frontend/src/provider.tsplugins/dashboard-data-provider/frontend/tailwind.config.jsplugins/dashboard-data-provider/frontend/tsconfig.jsonplugins/dashboard-data-provider/frontend/vite.config.tsplugins/dashboard-data-provider/plugin.json
✅ Files skipped from review due to trivial changes (2)
- packages/database/src/plugin-discovery.ts
- bin/cleanup-moved-plugins.ts
Comment on lines
+39
to
+42
| # Option A: Build endpoint from key + id | ||
| SUBGRAPH_API_KEY= | ||
| SUBGRAPH_ID= | ||
|
|
There was a problem hiding this comment.
Clarify SUBGRAPH_ID optionality consistently in the first SUBGRAPH block.
The runtime route falls back to a default subgraph ID when SUBGRAPH_ID is unset, but this is only documented in the later (duplicated) block. Add the same note near Line 41 to avoid confusion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.env.example around lines 39 - 42, The first SUBGRAPH block currently lists
SUBGRAPH_API_KEY and SUBGRAPH_ID without noting that SUBGRAPH_ID is optional;
add a short clarifying comment alongside SUBGRAPH_ID (same text as the later
duplicated block) stating that the runtime will fall back to the default
subgraph ID when SUBGRAPH_ID is unset so readers know it’s optional; update the
comment near SUBGRAPH_ID (and keep it consistent with the duplicate block) to
avoid confusion.
Comment on lines
+4
to
+7
| * Maps internal leaderboard API identifiers to dashboard-friendly labels. | ||
| * A null MODEL_DISPLAY entry means "exclude from the model catalog". | ||
| * Keep in sync with pipeline-config.ts in dashboard-data-provider when adding new entries. | ||
| */ |
There was a problem hiding this comment.
Fix misleading sync guidance in the file header.
Line 6 points to dashboard-data-provider pipeline config as the sync target, but that file maps different identifiers (model/workload IDs vs parent pipeline IDs here). This can cause incorrect future edits.
Suggested doc fix
- * Keep in sync with pipeline-config.ts in dashboard-data-provider when adding new entries.
+ * Keep in sync with shared model/pipeline catalog mappings used by developer-web and developer-api.
+ * Note: dashboard-data-provider pipeline-config.ts maps a different identifier space.📝 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
| * Maps internal leaderboard API identifiers to dashboard-friendly labels. | |
| * A null MODEL_DISPLAY entry means "exclude from the model catalog". | |
| * Keep in sync with pipeline-config.ts in dashboard-data-provider when adding new entries. | |
| */ | |
| * Maps internal leaderboard API identifiers to dashboard-friendly labels. | |
| * A null MODEL_DISPLAY entry means "exclude from the model catalog". | |
| * Keep in sync with shared model/pipeline catalog mappings used by developer-web and developer-api. | |
| * Note: dashboard-data-provider pipeline-config.ts maps a different identifier space. | |
| */ |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/workflows/developer-web/src/data/network-config.ts` around lines 4 - 7,
Update the header comment that currently tells maintainers to "Keep in sync with
pipeline-config.ts in dashboard-data-provider": clarify or remove that guidance
because the identifiers in this file (parent pipeline IDs used by MODEL_DISPLAY)
are different from the model/workload IDs in dashboard-data-provider;
specifically edit the comment near the MODEL_DISPLAY mapping to either point to
the correct canonical mapping file or explicitly note the identifier differences
(parent pipeline IDs vs model/workload IDs) so future edits won’t attempt to
mirror the wrong file.
| * interval=14h → 7 d lookback at 14 h resolution (weekly fees) | ||
| */ | ||
|
|
||
| const BASE_URL = 'https://leaderboard-api.livepeer.cloud'; |
There was a problem hiding this comment.
Use a configurable/proxied base URL instead of hardcoding production.
Line 14 pins this module to the production host, which makes staging/local overrides harder and bypasses the new server proxy route introduced in this PR.
Suggested change
-const BASE_URL = 'https://leaderboard-api.livepeer.cloud';
+// Route through app proxy so host/env can be managed server-side
+const BASE_URL = '/api/v1/leaderboard';📝 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 BASE_URL = 'https://leaderboard-api.livepeer.cloud'; | |
| // Route through app proxy so host/env can be managed server-side | |
| const BASE_URL = '/api/v1/leaderboard'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/dashboard-data-provider/frontend/src/api/leaderboard.ts` at line 14,
The constant BASE_URL in leaderboard.ts is hardcoded to the production host;
change it to use a configurable/proxied base URL (e.g., read from an environment
variable like VITE_LEADERBOARD_API_BASE or fall back to a relative/proxy path
such as '/api/leaderboard') so staging/local builds and the new server proxy
route work; update the BASE_URL definition and any callers that import it
(BASE_URL) to use the env/relative value and ensure the default preserves
current production behavior if no override is provided.
Comment on lines
+77
to
+94
| export async function fetchNetworkDemand(interval: string): Promise<NetworkDemandRow[]> { | ||
| const data = await apiFetch<{ demand: NetworkDemandRow[] }>( | ||
| `/api/network/demand?interval=${interval}` | ||
| ); | ||
| return data.demand ?? []; | ||
| } | ||
|
|
||
| export async function fetchGPUMetrics(timeRange: string): Promise<GPUMetricRow[]> { | ||
| const data = await apiFetch<{ metrics: GPUMetricRow[] }>( | ||
| `/api/gpu/metrics?time_range=${timeRange}` | ||
| ); | ||
| return data.metrics ?? []; | ||
| } | ||
|
|
||
| export async function fetchSLACompliance(period: string): Promise<SLAComplianceRow[]> { | ||
| const data = await apiFetch<{ compliance: SLAComplianceRow[] }>( | ||
| `/api/sla/compliance?period=${period}` | ||
| ); |
There was a problem hiding this comment.
Encode query parameters before composing endpoint URLs.
Line 79, Line 86, and Line 93 interpolate raw strings into URLs. Use URLSearchParams so special characters don’t produce malformed requests.
Suggested change
export async function fetchNetworkDemand(interval: string): Promise<NetworkDemandRow[]> {
+ const query = new URLSearchParams({ interval });
const data = await apiFetch<{ demand: NetworkDemandRow[] }>(
- `/api/network/demand?interval=${interval}`
+ `/api/network/demand?${query.toString()}`
);
return data.demand ?? [];
}
export async function fetchGPUMetrics(timeRange: string): Promise<GPUMetricRow[]> {
+ const query = new URLSearchParams({ time_range: timeRange });
const data = await apiFetch<{ metrics: GPUMetricRow[] }>(
- `/api/gpu/metrics?time_range=${timeRange}`
+ `/api/gpu/metrics?${query.toString()}`
);
return data.metrics ?? [];
}
export async function fetchSLACompliance(period: string): Promise<SLAComplianceRow[]> {
+ const query = new URLSearchParams({ period });
const data = await apiFetch<{ compliance: SLAComplianceRow[] }>(
- `/api/sla/compliance?period=${period}`
+ `/api/sla/compliance?${query.toString()}`
);
return data.compliance ?? [];
}📝 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
| export async function fetchNetworkDemand(interval: string): Promise<NetworkDemandRow[]> { | |
| const data = await apiFetch<{ demand: NetworkDemandRow[] }>( | |
| `/api/network/demand?interval=${interval}` | |
| ); | |
| return data.demand ?? []; | |
| } | |
| export async function fetchGPUMetrics(timeRange: string): Promise<GPUMetricRow[]> { | |
| const data = await apiFetch<{ metrics: GPUMetricRow[] }>( | |
| `/api/gpu/metrics?time_range=${timeRange}` | |
| ); | |
| return data.metrics ?? []; | |
| } | |
| export async function fetchSLACompliance(period: string): Promise<SLAComplianceRow[]> { | |
| const data = await apiFetch<{ compliance: SLAComplianceRow[] }>( | |
| `/api/sla/compliance?period=${period}` | |
| ); | |
| export async function fetchNetworkDemand(interval: string): Promise<NetworkDemandRow[]> { | |
| const query = new URLSearchParams({ interval }); | |
| const data = await apiFetch<{ demand: NetworkDemandRow[] }>( | |
| `/api/network/demand?${query.toString()}` | |
| ); | |
| return data.demand ?? []; | |
| } | |
| export async function fetchGPUMetrics(timeRange: string): Promise<GPUMetricRow[]> { | |
| const query = new URLSearchParams({ time_range: timeRange }); | |
| const data = await apiFetch<{ metrics: GPUMetricRow[] }>( | |
| `/api/gpu/metrics?${query.toString()}` | |
| ); | |
| return data.metrics ?? []; | |
| } | |
| export async function fetchSLACompliance(period: string): Promise<SLAComplianceRow[]> { | |
| const query = new URLSearchParams({ period }); | |
| const data = await apiFetch<{ compliance: SLAComplianceRow[] }>( | |
| `/api/sla/compliance?${query.toString()}` | |
| ); | |
| return data.compliance ?? []; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/dashboard-data-provider/frontend/src/api/leaderboard.ts` around lines
77 - 94, The three functions fetchNetworkDemand, fetchGPUMetrics, and
fetchSLACompliance build query strings by interpolating raw parameters into the
URL which can break for special characters; change each to build encoded query
parameters (e.g., via URLSearchParams or encodeURIComponent) and pass the
encoded query to apiFetch (e.g., construct params = new URLSearchParams({
interval }), then call `/api/network/demand?${params}`), repeating the same
pattern for time_range and period so requests are safe for special characters.
Comment on lines
+136
to
+154
| async function resolvePipelines({ limit = 5 }: { limit?: number }): Promise<DashboardPipelineUsage[]> { | ||
| // interval=2h → last 24 h; aggregate inference minutes per pipeline | ||
| const demand = await fetchNetworkDemand('2h'); | ||
|
|
||
| const totals = new Map<string, number>(); | ||
| for (const row of demand) { | ||
| totals.set(row.pipeline, (totals.get(row.pipeline) ?? 0) + row.total_inference_minutes); | ||
| } | ||
|
|
||
| return [...totals.entries()] | ||
| .filter(([name]) => PIPELINE_DISPLAY[name] !== null) // drop excluded pipelines | ||
| .map(([name, mins]) => ({ | ||
| name: PIPELINE_DISPLAY[name] ?? name, | ||
| mins: Math.round(mins), | ||
| color: PIPELINE_COLOR[name] ?? DEFAULT_PIPELINE_COLOR, | ||
| })) | ||
| .sort((a, b) => b.mins - a.mins) | ||
| .slice(0, limit); | ||
| } |
There was a problem hiding this comment.
Clamp limit before slicing pipeline results.
Line 153 uses raw limit; negative values yield unintended slices.
Suggested fix
async function resolvePipelines({ limit = 5 }: { limit?: number }): Promise<DashboardPipelineUsage[]> {
+ const safeLimit = Number.isFinite(limit) ? Math.max(0, Math.floor(limit)) : 5;
// interval=2h → last 24 h; aggregate inference minutes per pipeline
const demand = await fetchNetworkDemand('2h');
@@
- .slice(0, limit);
+ .slice(0, safeLimit);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/dashboard-data-provider/frontend/src/provider.ts` around lines 136 -
154, resolvePipelines currently passes the raw limit into .slice(...), which
allows negative values to produce unintended results; clamp and normalize limit
before slicing by computing a non-negative integer (e.g., let n = Math.max(0,
Math.floor(limit || 0))) and use n in the final .slice call; update the
resolvePipelines function to validate/normalize the limit parameter before the
return pipeline array is sliced to ensure negative or non-integer limits do not
produce unexpected output.
6 tasks
Contributor
Author
|
closing in favor of new pr #187 |
5 tasks
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
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
Integrates live Livepeer network data into two areas of NaaP:
Dashboard Overview (
dashboard-provider-mockplugin): Replaces all mock/static KPI, pipeline, GPU, and orchestrator data with real-time calls to the Livepeer Leaderboard API and the Livepeer subgraph. Adds a new Fees card backed by The Graph subgraph data, and a Protocol card showing live round progress using the Ethereum L1 block number viaviem. The Job Feed and Pricing cards are scaffolded with "Coming Soon" overlays pending upstream endpoints.Developer API plugin (
developer-api): Replaces the static model list with a live model catalog sourced from the Leaderboard API. Each model card now shows real GPU types, average FPS, average E2E latency, SLA score, orchestrator count, and region availability. Clicking a model opens a detail modal with the full metrics breakdown.Three new server-side proxy routes are added to
apps/web-nextto keep secrets and third-party URLs off the client:/api/v1/leaderboard/[...path]— proxies the Livepeer Leaderboard API/api/v1/subgraph— proxies The Graph subgraph queries (requiresSUBGRAPH_API_KEY)/api/v1/protocol-block— returns current L1 block number viaviem(requiresL1_RPC_URL)Changes
plugins/dashboard-provider-mock— full rewrite ofprovider.ts; replaces mock resolvers with live Leaderboard, subgraph, and L1 RPC data for KPI, pipelines, GPU capacity, orchestrators, protocol, and feesplugins/developer-api— newuseModelCataloghook andleaderboard.tsAPI client;DeveloperViewnow renders live model cards with a detail modalapps/web-next— three new API proxy routes (leaderboard,subgraph,protocol-block); dashboard page extended withFeesCard(Recharts bar chart with day/week toggle + raw data drawer),OrchestratorTableCard(sortable/filterable table), andProtocolCardwith L1 block progresspackages/types— new shared types:NetworkModel,GPUHardwareSummary,GatewayOffer,DashboardOrchestrator, and extendedDashboardProtocol/DashboardFeesInfoshapesapps/workflows/developer-web—ModelsTab,ModelCard,ModelDetailPanel,CompareDrawerall updated to useNetworkModelwith live GPU fleet, region, and SLA data; newuseNetworkCapabilitieshook with polling; newutils/gpu.tshelper.env.example/.env.local.example— documents new required env vars:SUBGRAPH_API_KEY,SUBGRAPH_ID,L1_RPC_URL,LEADERBOARD_API_URLrecharts,viem— added as dependenciesType
Plugin(s) Affected
dashboard-provider-mockdeveloper-apiChecklist
npm run lint)npm run build)Breaking Changes
Environment variables: Three new env vars are required for full functionality. Without them the affected cards degrade gracefully to a
503unavailable state rather than crashing:SUBGRAPH_API_KEYSUBGRAPH_IDL1_RPC_URLLEADERBOARD_API_URLhttps://leaderboard-api.livepeer.cloud)DashboardProtocoltype — extended withblockProgressandtotalBlocks; provider plugins that implement this resolver must now return these fields.DashboardFeesInfotype — extended withdayData,weeklyData, and weekly volume change fields.DashboardOrchestratortype — new type added toplugin-sdk; providers must resolveorchestratorsif the dashboard query includes it.Summary by CodeRabbit
Release Notes
New Features
Chores