feat: add orchestrator leaderboard plugin with real-time ClickHouse-backed rankings#213
Open
feat: add orchestrator leaderboard plugin with real-time ClickHouse-backed rankings#213
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
github-actions
bot
added
scope/shell
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a complete Orchestrator Leaderboard feature: server APIs (/rank, /filters), ClickHouse gateway integration, in-memory caching and plan refresh, ranking/filtering/SLA libraries, frontend plugin with hooks/pages, tests (unit, integration, E2E), gateway connector, docs, and CLI/registry tooling. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server as POST /rank
participant Cache as In-Memory Cache
participant ClickHouse as ClickHouse Gateway
participant Reranker as Ranking/Filtering
Client->>Server: POST { capability, topN?, filters?, slaWeights? }
Server->>Cache: getCached(capability)
alt cache hit
Cache-->>Server: { rows, cachedAt }
else cache miss
Server->>ClickHouse: POST SQL (built from capability/topN)
ClickHouse-->>Server: JSON rows
Server->>Cache: setCached(capability, rows)
end
Server->>Reranker: applyFilters(rows, filters)
Reranker->>Reranker: rerank & compute slaScore? (slaWeights)
Reranker-->>Server: OrchestratorRow[] (sliced to topN)
Server-->>Client: 200 { data } + headers (Cache-Control, X-Cache, X-Cache-Age, X-Data-Freshness)
sequenceDiagram
participant UI as LeaderboardPage
participant Hook as useLeaderboard
participant API as fetchRank/fetchCapabilities
participant Server as /rank & /filters
participant BrowserCache as Client Cache
UI->>Hook: request={capability,...}, autoRefresh?
Hook->>API: fetchRank(request)
API->>Server: POST /rank
Server-->>API: 200 + headers (X-Cache / Cache-Control)
API->>BrowserCache: store per Cache-Control
API-->>Hook: data + cache metadata
Hook-->>UI: render table, stats, SLA editor
Note over Hook,UI: if autoRefresh enabled -> repeat fetch interval
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 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 |
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
| const TOP_N_OPTIONS = [5, 10, 20, 50]; | ||
|
|
||
| export const LeaderboardPage: React.FC = () => { | ||
| const { capabilities, loading: capsLoading } = useCapabilities(); |
Contributor
There was a problem hiding this comment.
Capabilities fetch error is ignored, causing "No capabilities available" to display misleadingly when the actual issue is a network or server failure.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (9)
plugins/orchestrator-leaderboard/frontend/package.json (1)
8-9: Duplicate build scripts.
buildandbuild:umdare identical commands. If UMD output requires different configuration, consider updatingbuild:umdto use a separate Vite config or mode. Otherwise, one script could be removed to avoid confusion.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/orchestrator-leaderboard/frontend/package.json` around lines 8 - 9, The package.json contains duplicate npm scripts "build" and "build:umd" running the exact same command; either remove the redundant "build:umd" script or change it to run a distinct UMD-specific build (for example by referencing a separate Vite config or mode) so that "build" remains the default production build and "build:umd" produces UMD output via its own config (update the scripts entry for "build:umd" accordingly).apps/web-next/src/lib/gateway/__tests__/transform.test.ts (1)
199-206: Consider adding a test with consumer Authorization header on the request.The current test verifies that
headerMapping.Authorizationis not forwarded, but doesn't test the scenario where the consumer's incomingRequestobject has anAuthorizationheader (e.g., from gateway API key authentication). Consider adding a test case where the request includes anAuthorizationheader directly:const request = new Request('https://example.com', { headers: { Authorization: 'Bearer gw_consumer_key' }, });This would ensure that consumer gateway credentials are also stripped when
authTypeisbasic.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web-next/src/lib/gateway/__tests__/transform.test.ts` around lines 199 - 206, Add a new test case that constructs a Request with an incoming Authorization header (e.g., headers: { Authorization: 'Bearer gw_consumer_key' }) and calls buildUpstreamRequest with authType set to basic; verify the resulting Request.headers.get('Authorization') equals the expected upstream basic credential (Basic ${Buffer.from('admin:secret').toString('base64')}) and assert it does not contain the consumer token or any gateway-forwarded markers (e.g., not.toContain('gw_consumer_key'), not.toContain('consumer-style-token'), not.toContain('gw_not_forwarded')), so buildUpstreamRequest properly strips incoming consumer Authorization while applying the configured basic auth.apps/web-next/src/lib/orchestrator-leaderboard/__tests__/cache.test.ts (1)
39-48: Use deterministic fake timers to avoid test flakiness.The test uses
await new Promise((r) => setTimeout(r, 150))which relies on wall-clock timing and can fail intermittently in CI environments. Replace with Vitest's fake timer API for reliable, deterministic expiry assertions.♻️ Proposed change
describe('Leaderboard Cache', () => { beforeEach(() => { clearCache(); }); + afterEach(() => { + vi.useRealTimers(); + }); it('expires entries after TTL', async () => { + vi.useFakeTimers(); const rows = [makeRow()]; setCached('test-cap', rows, 100); expect(getCached('test-cap')).not.toBeNull(); - await new Promise((r) => setTimeout(r, 150)); + await vi.advanceTimersByTimeAsync(150); expect(getCached('test-cap')).toBeNull(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web-next/src/lib/orchestrator-leaderboard/__tests__/cache.test.ts` around lines 39 - 48, Replace the real-time delay in the test for TTL expiry with Vitest's fake timers: wrap the test so it uses vi.useFakeTimers() before scheduling, call setCached('test-cap', rows, 100), verify getCached('test-cap') is not null, then advance timers deterministically with vi.advanceTimersByTime(150) to trigger expiry and assert getCached('test-cap') is null, and finally restore timers with vi.useRealTimers(); update the test around the setCached/getCached calls to use these vi fake-timer calls.apps/web-next/src/lib/orchestrator-leaderboard/ranking.ts (1)
64-72: Normalize only non-negative SLA weights.Mixed-sign inputs currently normalize into negative or
>1coefficients, which can invert one metric and push scores outside the intended model. Clamping negatives to0or rejecting them up front would make the API harder to misuse.♻️ Suggested hardening
function normalizeWeights(weights?: SLAWeights): Required<SLAWeights> { - const w = { ...DEFAULT_WEIGHTS, ...weights }; - const sum = (w.latency || 0) + (w.swapRate || 0) + (w.price || 0); + const w = { + latency: Math.max(0, weights?.latency ?? DEFAULT_WEIGHTS.latency), + swapRate: Math.max(0, weights?.swapRate ?? DEFAULT_WEIGHTS.swapRate), + price: Math.max(0, weights?.price ?? DEFAULT_WEIGHTS.price), + }; + const sum = w.latency + w.swapRate + w.price; if (sum === 0) return DEFAULT_WEIGHTS; return { - latency: (w.latency || 0) / sum, - swapRate: (w.swapRate || 0) / sum, - price: (w.price || 0) / sum, + latency: w.latency / sum, + swapRate: w.swapRate / sum, + price: w.price / sum, }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web-next/src/lib/orchestrator-leaderboard/ranking.ts` around lines 64 - 72, normalizeWeights currently allows negative SLAWeights to produce invalid normalized coefficients; update normalizeWeights to clamp each input weight to a non-negative value before summing (e.g., treat undefined as 0 and use Math.max(0, w.latency) etc.), compute the sum of these clamped values, return DEFAULT_WEIGHTS if the sum is 0, otherwise divide each clamped value by the sum to produce the normalized latency, swapRate, and price; reference function normalizeWeights, type SLAWeights and DEFAULT_WEIGHTS when making this change.plugins/orchestrator-leaderboard/frontend/e2e/leaderboard.spec.ts (2)
33-33: SimplifypostDataJSONparsing.The
postDataJSON?.()syntax is unusual. Playwright'srequest.postDataJSON()returnsobject | nulldirectly. The fallback pattern is cleaner:-const postData = request.postDataJSON?.() ?? JSON.parse(request.postData() || '{}'); +const postData = request.postDataJSON() ?? {};If
postDataJSON()returnsnull(non-JSON body), fallback to empty object. The manualJSON.parsefallback is redundant for this use case.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/orchestrator-leaderboard/frontend/e2e/leaderboard.spec.ts` at line 33, Replace the unusual call pattern that parses post data with Playwright's direct API: use request.postDataJSON() with a nullish fallback to an empty object instead of calling postDataJSON?.() and manually JSON.parse; update the assignment where postData is set so it reads the result of request.postDataJSON() ?? {} (referencing the request.postDataJSON API and the postData variable in the spec) to remove the redundant JSON.parse fallback.
101-103: PreferwaitForResponseoverwaitForTimeoutfor request assertions.Using
waitForTimeout(500)introduces flakiness. Consider waiting for the actual network request:await Promise.all([ page.waitForResponse('**/api/v1/orchestrator-leaderboard/rank'), page.locator('select').first().selectOption('streamdiffusion-sdxl'), ]); expect(capturedBody?.capability).toBe('streamdiffusion-sdxl');This applies to similar patterns on lines 123, 150.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/orchestrator-leaderboard/frontend/e2e/leaderboard.spec.ts` around lines 101 - 103, Replace flaky page.waitForTimeout(500) usage by awaiting the actual network response: use page.waitForResponse to wait for the leaderboard rank request (the same route used in the test) and wrap it in Promise.all with the UI action (e.g., page.locator('select').first().selectOption('streamdiffusion-sdxl')) so the test waits for the request to complete before asserting on capturedBody; update the occurrences that reference page.waitForTimeout (including the one preceding assertions against capturedBody.capability and capturedBody.topN and the similar patterns later in the file) to follow this waitForResponse + Promise.all pattern using the same request URL/route the test intercepts.plugins/orchestrator-leaderboard/examples/client-test.ts (1)
64-67: Consider checkingres.okbefore parsing JSON.If the server returns a non-JSON error response (e.g., 502 HTML page),
res.json()will throw. For robustness:♻️ Suggested improvement
+if (!res.ok) { + throw new Error(`Request failed: ${res.status} ${res.statusText}`); +} + const json = await res.json(); if (!json.success) { throw new Error(`API error: ${json.error?.message || res.status}`); }This is a minor improvement for example code.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/orchestrator-leaderboard/examples/client-test.ts` around lines 64 - 67, The code currently calls res.json() before checking HTTP status which can throw on non-JSON error pages; update the logic around the res variable so you first check res.ok and handle non-OK responses (e.g., await res.text() and include status and body in the thrown Error) before attempting to parse JSON, or wrap res.json() in try/catch and fall back to res.text() on parse failure; locate and modify the block using res and json in the example to implement this check and produce a clearer error message.plugins/orchestrator-leaderboard/examples/client-test.sh (1)
51-56: Consider noting curl version requirement.The
%header{...}format specifier in-wrequires curl 7.83.0+. For broader compatibility in the example, you could alternatively usegrepto extract headers:curl -sI ... | grep -i 'X-Cache'This is minor since it's an example script, not production code.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/orchestrator-leaderboard/examples/client-test.sh` around lines 51 - 56, The example curl invocation in client-test.sh uses the -w format specifier %header{...} (seen in the -w argument) which requires curl >= 7.83.0; update the example to either (A) add a brief comment above the curl line noting the minimum curl version requirement (curl 7.83.0+) or (B) replace the header-extraction approach by requesting headers and filtering them (e.g., use curl -I or -sI and pipe to grep for X-Cache/X-Cache-Age/X-Data-Freshness/Cache-Control) so the script works on older curl versions—modify the -w "%header{...}" usage in client-test.sh accordingly.plugins/orchestrator-leaderboard/frontend/src/hooks/useLeaderboard.ts (1)
60-66: Consider usinguseMemofor stable dependency comparison.Using
JSON.stringifydirectly in the dependency array can cause unnecessary re-renders if the parent recreates thefiltersorslaWeightsobjects on each render (even with identical content). A more robust approach:♻️ Suggested improvement
+const filtersKey = useMemo(() => JSON.stringify(request?.filters), [request?.filters]); +const slaWeightsKey = useMemo(() => JSON.stringify(request?.slaWeights), [request?.slaWeights]); useEffect(() => { if (!request?.capability) return; doFetch(); }, [ request?.capability, request?.topN, - JSON.stringify(request?.filters), - JSON.stringify(request?.slaWeights), + filtersKey, + slaWeightsKey, doFetch, ]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/orchestrator-leaderboard/frontend/src/hooks/useLeaderboard.ts` around lines 60 - 66, The dependency array in the useEffect inside useLeaderboard is unstable because it uses JSON.stringify directly on request.filters and request.slaWeights; change this to compute stable memoized values using useMemo (e.g., const serializedFilters = useMemo(() => JSON.stringify(request?.filters), [request?.filters]) and similarly for slaWeights) and then use serializedFilters, serializedSlaWeights, request?.capability, request?.topN and doFetch in the effect dependency array; ensure useMemo is imported and reference the existing useLeaderboard hook, the effect that depends on request?.filters/request?.slaWeights, and the doFetch function when applying this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web-next/src/app/api/v1/orchestrator-leaderboard/filters/route.ts`:
- Around line 68-70: The Cache-Control header is incorrectly set to public for
an authenticated route gated by authorize(); change the response header in the
route handler (the code that creates response via success(...) and sets
response.headers.set) to use a private caching policy (e.g., "private,
max-age=60") instead of "public, max-age=60" so shared caches won't reuse one
caller's capabilities for another.
- Around line 39-41: The code builds the ClickHouse proxy URL using a hardcoded
fallback ('http://localhost:3000'), which breaks in deployed environments;
update the logic so that if process.env.NEXT_PUBLIC_APP_URL is unset you derive
the origin from the incoming request (use new URL(request.url).origin) and use
that origin to construct the url variable used for the ClickHouse query; modify
the assignment that currently sets baseUrl and url (references: getAuthToken,
baseUrl, url) to prefer NEXT_PUBLIC_APP_URL and otherwise use the
request-derived origin so the route won't incorrectly fall back to
FALLBACK_CAPABILITIES.
In `@apps/web-next/src/app/api/v1/orchestrator-leaderboard/rank/route.ts`:
- Around line 60-89: The leaderboard cache is currently keyed only by capability
so cached results can leak across tenants; include the caller's auth/tenant
context in the cache key used by fetchLeaderboard (pass the authToken or derived
tenantId into fetchLeaderboard's caching logic and ensure the cache key
incorporates that value) and change the HTTP response caching to private by
setting Cache-Control to 'private, max-age=10' (update the code that sets
response.headers — e.g., where success(data) is assigned and
response.headers.set('Cache-Control', ...) — and ensure any cache-metadata like
X-Cache still reflects tenant-scoped cache hits).
- Around line 28-43: After parsing the request body with request.json() into the
local variable body (typed LeaderboardRequest), add an explicit null check
before accessing body.capability: if body is null (or not an object), return the
same NextResponse.json validation error used for missing/invalid capability
(code: 'VALIDATION_ERROR', message: 'capability is required and must be a
string') with status 400; ensure this check occurs immediately after await
request.json() and before the existing typeof body.capability validation so a
null JSON payload yields the intended 400 response.
In `@apps/web-next/src/lib/orchestrator-leaderboard/cache.ts`:
- Around line 40-45: The eviction logic currently deletes the oldest entry even
when setCached() is called to refresh an existing key; change it to check
whether the incoming key already exists and only perform FIFO eviction when
inserting a new key. Specifically, in setCached (and around LEADERBOARD_CACHE/
MAX_ENTRIES usage), call evictExpired() first, then if LEADERBOARD_CACHE.size >=
MAX_ENTRIES and !LEADERBOARD_CACHE.has(key) remove the oldest key; if the key
already exists, skip deleting oldest and simply overwrite/update the existing
entry.
In `@apps/web-next/src/lib/orchestrator-leaderboard/query.ts`:
- Line 12: The constant MAX_QUERY_ROWS is set to 100 but validateTopN allows up
to 1000, so fetchLeaderboard and related query logic (functions validateTopN,
fetchLeaderboard and the query/path that uses MAX_QUERY_ROWS around the other
blocks) silently truncate requests >100; update the logic so the ClickHouse
LIMIT uses the actual required candidate window (e.g., set MAX_QUERY_ROWS to the
same upper bound as validateTopN or compute limit = Math.min(requestedTopN,
VALID_MAX) / ensure the query uses that computed limit), and propagate that
change to the other query/filter/rerank blocks referenced (around the blocks
currently using MAX_QUERY_ROWS at the other spots) so the returned candidate set
is large enough for subsequent filtering and reranking.
- Around line 144-147: The current logic treats (json.data ?? json) as a
ClickHouseJSONResponse and then accesses .data, which drops rows when res.json()
already returned the raw row array; change the return path to detect an array
response first (e.g., if Array.isArray(json) return json), otherwise cast to
ClickHouseJSONResponse and return json.data ?? []; update the variables
referenced (json and chResponse) or remove chResponse to use this conditional
check so raw array responses are returned intact.
In `@apps/web-next/src/lib/orchestrator-leaderboard/ranking.ts`:
- Around line 54-55: The max-filter logic currently lets records with null
metrics pass; update the predicates so when filters.maxAvgLatencyMs is set you
return false if r.avg_lat_ms is null OR r.avg_lat_ms > filters.maxAvgLatencyMs,
and similarly for filters.maxSwapRatio return false if r.swap_ratio is null OR
r.swap_ratio > filters.maxSwapRatio (update the lines referencing
filters.maxAvgLatencyMs/r.avg_lat_ms and filters.maxSwapRatio/r.swap_ratio).
In `@plugins/orchestrator-leaderboard/frontend/e2e/leaderboard.spec.ts`:
- Around line 1-158: The test file imports from `@playwright/test` (see the
top-level import and use of test/expect in leaderboard.spec.ts) but the project
is missing that dependency and Playwright config; add `@playwright/test` to
devDependencies, create a playwright.config.ts (use the existing pattern from
plugins/lightning-client/frontend/playwright.config.ts) and ensure it points to
the tests directory, and update the package.json test script to run Playwright
(e.g., "playwright test") instead of "vitest run" so leaderboard.spec.ts
executes under Playwright.
In `@plugins/orchestrator-leaderboard/frontend/src/pages/LeaderboardPage.tsx`:
- Around line 446-482: FilterInput and WeightSlider render visible labels that
are not programmatically associated with their inputs; update both components to
add accessible names: in FilterInput generate a stable id (e.g. via
React.useId()) and set that id on the <input> and htmlFor on the <label>; in
WeightSlider either wrap the visible label in a <label htmlFor={id}> or give the
label <span> an id and set aria-labelledby={labelId} on the range input (again
using React.useId() to create unique ids). Ensure types/signatures for
FilterInput and WeightSlider remain unchanged but add the id generation and the
corresponding htmlFor/aria-labelledby attributes so screen readers correctly
announce the controls.
- Around line 26-35: hasActiveFilters currently uses truthiness which drops
valid zero values; update the check to detect presence explicitly (!==
undefined/null) for each filter field so zero is preserved. Change the logic
around hasActiveFilters (used by request in the useMemo that builds
LeaderboardRequest) to test each filters.gpuRamGbMin, filters.gpuRamGbMax,
filters.priceMax, filters.maxAvgLatencyMs, and filters.maxSwapRatio for !==
undefined (or !== null) instead of truthiness, then keep the rest of the useMemo
(capability, topN, slaWeights) unchanged so the filters object is included in
the payload when any filter is set to 0.
In `@plugins/service-gateway/docs/clickhouse-query-connector.md`:
- Around line 13-19: The docs currently show an inline password in the curl
example ("curl --user 'default:YOUR_PASSWORD' ..."), which risks leaking
secrets; update the example to remove the inline password and instruct users to
omit the password so curl will prompt (e.g., use "curl --user 'default' ...") or
to use a safe alternative such as a .netrc/credentials file or environment-based
secret injection, and update the surrounding text to recommend these safer
methods instead of embedding the password directly in the command.
In `@plugins/service-gateway/docs/connector-catalog.md`:
- Line 171: The detailed-section numbering is inconsistent: the current heading
"### 19. ClickHouse Query API (`clickhouse-query`)" follows "### 17" leaving 18
missing; either insert the missing section for "ssh-bridge" as "### 18" with its
content, or renumber the "ClickHouse Query API (`clickhouse-query`)" heading to
"### 18" (and update any subsequent headings) so numbering is sequential; locate
the "ClickHouse Query API (`clickhouse-query`)" heading and adjust the heading
number or add the "ssh-bridge" section accordingly.
---
Nitpick comments:
In `@apps/web-next/src/lib/gateway/__tests__/transform.test.ts`:
- Around line 199-206: Add a new test case that constructs a Request with an
incoming Authorization header (e.g., headers: { Authorization: 'Bearer
gw_consumer_key' }) and calls buildUpstreamRequest with authType set to basic;
verify the resulting Request.headers.get('Authorization') equals the expected
upstream basic credential (Basic
${Buffer.from('admin:secret').toString('base64')}) and assert it does not
contain the consumer token or any gateway-forwarded markers (e.g.,
not.toContain('gw_consumer_key'), not.toContain('consumer-style-token'),
not.toContain('gw_not_forwarded')), so buildUpstreamRequest properly strips
incoming consumer Authorization while applying the configured basic auth.
In `@apps/web-next/src/lib/orchestrator-leaderboard/__tests__/cache.test.ts`:
- Around line 39-48: Replace the real-time delay in the test for TTL expiry with
Vitest's fake timers: wrap the test so it uses vi.useFakeTimers() before
scheduling, call setCached('test-cap', rows, 100), verify getCached('test-cap')
is not null, then advance timers deterministically with
vi.advanceTimersByTime(150) to trigger expiry and assert getCached('test-cap')
is null, and finally restore timers with vi.useRealTimers(); update the test
around the setCached/getCached calls to use these vi fake-timer calls.
In `@apps/web-next/src/lib/orchestrator-leaderboard/ranking.ts`:
- Around line 64-72: normalizeWeights currently allows negative SLAWeights to
produce invalid normalized coefficients; update normalizeWeights to clamp each
input weight to a non-negative value before summing (e.g., treat undefined as 0
and use Math.max(0, w.latency) etc.), compute the sum of these clamped values,
return DEFAULT_WEIGHTS if the sum is 0, otherwise divide each clamped value by
the sum to produce the normalized latency, swapRate, and price; reference
function normalizeWeights, type SLAWeights and DEFAULT_WEIGHTS when making this
change.
In `@plugins/orchestrator-leaderboard/examples/client-test.sh`:
- Around line 51-56: The example curl invocation in client-test.sh uses the -w
format specifier %header{...} (seen in the -w argument) which requires curl >=
7.83.0; update the example to either (A) add a brief comment above the curl line
noting the minimum curl version requirement (curl 7.83.0+) or (B) replace the
header-extraction approach by requesting headers and filtering them (e.g., use
curl -I or -sI and pipe to grep for
X-Cache/X-Cache-Age/X-Data-Freshness/Cache-Control) so the script works on older
curl versions—modify the -w "%header{...}" usage in client-test.sh accordingly.
In `@plugins/orchestrator-leaderboard/examples/client-test.ts`:
- Around line 64-67: The code currently calls res.json() before checking HTTP
status which can throw on non-JSON error pages; update the logic around the res
variable so you first check res.ok and handle non-OK responses (e.g., await
res.text() and include status and body in the thrown Error) before attempting to
parse JSON, or wrap res.json() in try/catch and fall back to res.text() on parse
failure; locate and modify the block using res and json in the example to
implement this check and produce a clearer error message.
In `@plugins/orchestrator-leaderboard/frontend/e2e/leaderboard.spec.ts`:
- Line 33: Replace the unusual call pattern that parses post data with
Playwright's direct API: use request.postDataJSON() with a nullish fallback to
an empty object instead of calling postDataJSON?.() and manually JSON.parse;
update the assignment where postData is set so it reads the result of
request.postDataJSON() ?? {} (referencing the request.postDataJSON API and the
postData variable in the spec) to remove the redundant JSON.parse fallback.
- Around line 101-103: Replace flaky page.waitForTimeout(500) usage by awaiting
the actual network response: use page.waitForResponse to wait for the
leaderboard rank request (the same route used in the test) and wrap it in
Promise.all with the UI action (e.g.,
page.locator('select').first().selectOption('streamdiffusion-sdxl')) so the test
waits for the request to complete before asserting on capturedBody; update the
occurrences that reference page.waitForTimeout (including the one preceding
assertions against capturedBody.capability and capturedBody.topN and the similar
patterns later in the file) to follow this waitForResponse + Promise.all pattern
using the same request URL/route the test intercepts.
In `@plugins/orchestrator-leaderboard/frontend/package.json`:
- Around line 8-9: The package.json contains duplicate npm scripts "build" and
"build:umd" running the exact same command; either remove the redundant
"build:umd" script or change it to run a distinct UMD-specific build (for
example by referencing a separate Vite config or mode) so that "build" remains
the default production build and "build:umd" produces UMD output via its own
config (update the scripts entry for "build:umd" accordingly).
In `@plugins/orchestrator-leaderboard/frontend/src/hooks/useLeaderboard.ts`:
- Around line 60-66: The dependency array in the useEffect inside useLeaderboard
is unstable because it uses JSON.stringify directly on request.filters and
request.slaWeights; change this to compute stable memoized values using useMemo
(e.g., const serializedFilters = useMemo(() => JSON.stringify(request?.filters),
[request?.filters]) and similarly for slaWeights) and then use
serializedFilters, serializedSlaWeights, request?.capability, request?.topN and
doFetch in the effect dependency array; ensure useMemo is imported and reference
the existing useLeaderboard hook, the effect that depends on
request?.filters/request?.slaWeights, and the doFetch function when applying
this change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 09b5a613-4f3c-47bc-9282-4ccf17d4853e
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json,!package-lock.json
📒 Files selected for processing (36)
apps/web-next/src/__tests__/api/orchestrator-leaderboard.test.tsapps/web-next/src/app/api/v1/gw/admin/templates/route.tsapps/web-next/src/app/api/v1/orchestrator-leaderboard/filters/route.tsapps/web-next/src/app/api/v1/orchestrator-leaderboard/rank/route.tsapps/web-next/src/lib/gateway/__tests__/transform.test.tsapps/web-next/src/lib/gateway/transform.tsapps/web-next/src/lib/orchestrator-leaderboard/__tests__/cache.test.tsapps/web-next/src/lib/orchestrator-leaderboard/__tests__/query.test.tsapps/web-next/src/lib/orchestrator-leaderboard/__tests__/ranking.test.tsapps/web-next/src/lib/orchestrator-leaderboard/cache.tsapps/web-next/src/lib/orchestrator-leaderboard/query.tsapps/web-next/src/lib/orchestrator-leaderboard/ranking.tsapps/web-next/src/lib/orchestrator-leaderboard/types.tsapps/web-next/src/middleware.tsbin/seed-public-connectors.tsplugins/orchestrator-leaderboard/docs/api-reference.mdplugins/orchestrator-leaderboard/examples/client-test.shplugins/orchestrator-leaderboard/examples/client-test.tsplugins/orchestrator-leaderboard/frontend/e2e/leaderboard.spec.tsplugins/orchestrator-leaderboard/frontend/package.jsonplugins/orchestrator-leaderboard/frontend/src/App.tsxplugins/orchestrator-leaderboard/frontend/src/globals.cssplugins/orchestrator-leaderboard/frontend/src/hooks/useCapabilities.tsplugins/orchestrator-leaderboard/frontend/src/hooks/useLeaderboard.tsplugins/orchestrator-leaderboard/frontend/src/lib/api.tsplugins/orchestrator-leaderboard/frontend/src/mount.tsxplugins/orchestrator-leaderboard/frontend/src/pages/LeaderboardPage.tsxplugins/orchestrator-leaderboard/frontend/tailwind.config.jsplugins/orchestrator-leaderboard/frontend/tsconfig.jsonplugins/orchestrator-leaderboard/frontend/vite.config.tsplugins/orchestrator-leaderboard/plugin.jsonplugins/service-gateway/connectors/clickhouse-query.jsonplugins/service-gateway/connectors/connector-template.schema.jsonplugins/service-gateway/connectors/loader.tsplugins/service-gateway/docs/clickhouse-query-connector.mdplugins/service-gateway/docs/connector-catalog.md
Comment on lines
+1
to
+158
| import { test, expect, type Page } from '@playwright/test'; | ||
|
|
||
| const FIXTURE_CAPABILITIES = { | ||
| success: true, | ||
| data: { capabilities: ['noop', 'streamdiffusion-sdxl', 'streamdiffusion-sdxl-v2v'] }, | ||
| }; | ||
|
|
||
| const FIXTURE_RANK = { | ||
| success: true, | ||
| data: [ | ||
| { orchUri: 'https://orch-1.test', gpuName: 'RTX 4090', gpuGb: 24, avail: 3, totalCap: 4, pricePerUnit: 100, bestLatMs: 50, avgLatMs: 80, swapRatio: 0.05, avgAvail: 3.2 }, | ||
| { orchUri: 'https://orch-2.test', gpuName: 'A100', gpuGb: 80, avail: 1, totalCap: 2, pricePerUnit: 500, bestLatMs: 200, avgLatMs: 350, swapRatio: 0.3, avgAvail: 1.5 }, | ||
| { orchUri: 'https://orch-3.test', gpuName: 'RTX 3090', gpuGb: 24, avail: 2, totalCap: 2, pricePerUnit: 80, bestLatMs: null, avgLatMs: null, swapRatio: null, avgAvail: 2.0 }, | ||
| ], | ||
| }; | ||
|
|
||
| const FIXTURE_RANK_WITH_SLA = { | ||
| success: true, | ||
| data: [ | ||
| { orchUri: 'https://orch-1.test', gpuName: 'RTX 4090', gpuGb: 24, avail: 3, totalCap: 4, pricePerUnit: 100, bestLatMs: 50, avgLatMs: 80, swapRatio: 0.05, avgAvail: 3.2, slaScore: 0.95 }, | ||
| { orchUri: 'https://orch-3.test', gpuName: 'RTX 3090', gpuGb: 24, avail: 2, totalCap: 2, pricePerUnit: 80, bestLatMs: null, avgLatMs: null, swapRatio: null, avgAvail: 2.0, slaScore: 0.65 }, | ||
| { orchUri: 'https://orch-2.test', gpuName: 'A100', gpuGb: 80, avail: 1, totalCap: 2, pricePerUnit: 500, bestLatMs: 200, avgLatMs: 350, swapRatio: 0.3, avgAvail: 1.5, slaScore: 0.3 }, | ||
| ], | ||
| }; | ||
|
|
||
| async function stubAPIs(page: Page) { | ||
| await page.route('**/api/v1/orchestrator-leaderboard/filters', (route) => { | ||
| route.fulfill({ status: 200, contentType: 'application/json', body: JSON.stringify(FIXTURE_CAPABILITIES) }); | ||
| }); | ||
|
|
||
| await page.route('**/api/v1/orchestrator-leaderboard/rank', async (route) => { | ||
| const request = route.request(); | ||
| const postData = request.postDataJSON?.() ?? JSON.parse(request.postData() || '{}'); | ||
|
|
||
| const response = postData.slaWeights ? FIXTURE_RANK_WITH_SLA : FIXTURE_RANK; | ||
| route.fulfill({ | ||
| status: 200, | ||
| contentType: 'application/json', | ||
| body: JSON.stringify(response), | ||
| headers: { | ||
| 'X-Cache': 'MISS', | ||
| 'X-Cache-Age': '0', | ||
| 'X-Data-Freshness': new Date().toISOString(), | ||
| 'Cache-Control': 'public, max-age=10', | ||
| }, | ||
| }); | ||
| }); | ||
| } | ||
|
|
||
| test.describe('Orchestrator Leaderboard Page', () => { | ||
| test.beforeEach(async ({ page }) => { | ||
| await stubAPIs(page); | ||
| }); | ||
|
|
||
| test('loads capabilities dropdown and renders table on selection', async ({ page }) => { | ||
| await page.goto('/orchestrator-leaderboard'); | ||
|
|
||
| const select = page.locator('select').first(); | ||
| await expect(select).toBeVisible(); | ||
|
|
||
| const options = await select.locator('option').allTextContents(); | ||
| expect(options).toContain('streamdiffusion-sdxl'); | ||
| expect(options).toContain('noop'); | ||
|
|
||
| await select.selectOption('streamdiffusion-sdxl'); | ||
|
|
||
| await expect(page.locator('table')).toBeVisible(); | ||
|
|
||
| const rows = page.locator('tbody tr'); | ||
| await expect(rows).toHaveCount(3); | ||
| }); | ||
|
|
||
| test('renders all expected table columns', async ({ page }) => { | ||
| await page.goto('/orchestrator-leaderboard'); | ||
| await page.locator('select').first().selectOption('noop'); | ||
|
|
||
| await expect(page.locator('table')).toBeVisible(); | ||
|
|
||
| const headers = await page.locator('thead th').allTextContents(); | ||
| expect(headers).toContain('Orchestrator URL'); | ||
| expect(headers).toContain('GPU'); | ||
| expect(headers).toContain('GPU RAM'); | ||
| expect(headers).toContain('Capacity'); | ||
| expect(headers).toContain('Price/Unit'); | ||
| expect(headers).toContain('Best Lat (ms)'); | ||
| expect(headers).toContain('Avg Lat (ms)'); | ||
| expect(headers).toContain('Swap Ratio'); | ||
| expect(headers).toContain('Avg Avail'); | ||
| }); | ||
|
|
||
| test('sends correct capability and topN in POST body', async ({ page }) => { | ||
| let capturedBody: any = null; | ||
| await page.route('**/api/v1/orchestrator-leaderboard/rank', async (route) => { | ||
| capturedBody = JSON.parse(route.request().postData() || '{}'); | ||
| route.fulfill({ status: 200, contentType: 'application/json', body: JSON.stringify(FIXTURE_RANK) }); | ||
| }); | ||
|
|
||
| await page.goto('/orchestrator-leaderboard'); | ||
| await page.locator('select').first().selectOption('streamdiffusion-sdxl'); | ||
|
|
||
| await page.waitForTimeout(500); | ||
| expect(capturedBody?.capability).toBe('streamdiffusion-sdxl'); | ||
| expect(capturedBody?.topN).toBe(10); | ||
| }); | ||
|
|
||
| test('SLA toggle adds slaWeights and shows SLA Score column', async ({ page }) => { | ||
| let capturedBody: any = null; | ||
| await page.route('**/api/v1/orchestrator-leaderboard/rank', async (route) => { | ||
| capturedBody = JSON.parse(route.request().postData() || '{}'); | ||
| const response = capturedBody.slaWeights ? FIXTURE_RANK_WITH_SLA : FIXTURE_RANK; | ||
| route.fulfill({ status: 200, contentType: 'application/json', body: JSON.stringify(response) }); | ||
| }); | ||
|
|
||
| await page.goto('/orchestrator-leaderboard'); | ||
| await page.locator('select').first().selectOption('noop'); | ||
| await expect(page.locator('table')).toBeVisible(); | ||
|
|
||
| const headers1 = await page.locator('thead th').allTextContents(); | ||
| expect(headers1).not.toContain('SLA Score'); | ||
|
|
||
| await page.getByText('SLA Ranking OFF').click(); | ||
|
|
||
| await page.waitForTimeout(500); | ||
| expect(capturedBody?.slaWeights).toBeDefined(); | ||
|
|
||
| const headers2 = await page.locator('thead th').allTextContents(); | ||
| expect(headers2).toContain('SLA Score'); | ||
| }); | ||
|
|
||
| test('shows empty state message when no results', async ({ page }) => { | ||
| await page.route('**/api/v1/orchestrator-leaderboard/rank', (route) => { | ||
| route.fulfill({ status: 200, contentType: 'application/json', body: JSON.stringify({ success: true, data: [] }) }); | ||
| }); | ||
|
|
||
| await page.goto('/orchestrator-leaderboard'); | ||
| await page.locator('select').first().selectOption('noop'); | ||
|
|
||
| await expect(page.getByText('No orchestrators found')).toBeVisible(); | ||
| }); | ||
|
|
||
| test('auto-refresh triggers multiple API calls', async ({ page }) => { | ||
| let callCount = 0; | ||
| await page.route('**/api/v1/orchestrator-leaderboard/rank', (route) => { | ||
| callCount++; | ||
| route.fulfill({ status: 200, contentType: 'application/json', body: JSON.stringify(FIXTURE_RANK) }); | ||
| }); | ||
|
|
||
| await page.goto('/orchestrator-leaderboard'); | ||
| await page.locator('select').first().selectOption('noop'); | ||
| await page.waitForTimeout(500); | ||
|
|
||
| const initialCount = callCount; | ||
| await page.getByText('Auto-refresh (5s)').click(); | ||
|
|
||
| await page.waitForTimeout(6000); | ||
| expect(callCount).toBeGreaterThan(initialCount); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if playwright.config.ts exists for orchestrator-leaderboard frontend
fd -t f 'playwright.config' plugins/orchestrator-leaderboard/frontend/
# Also check package.json for playwright dev dependencies
cat plugins/orchestrator-leaderboard/frontend/package.json 2>/dev/null | jq '.devDependencies | keys | map(select(contains("playwright")))'Repository: livepeer/naap
Length of output: 59
🏁 Script executed:
# Check if lightning-client has playwright.config.ts
fd -t f 'playwright.config' plugins/lightning-client/frontend/
# Check for playwright.config at repo root
fd -t f 'playwright.config' . --max-depth 2
# Check root package.json for playwright
cat package.json 2>/dev/null | jq '.devDependencies | keys | map(select(contains("playwright")))' || cat package.json 2>/dev/null | jq '.dependencies | keys | map(select(contains("playwright")))'Repository: livepeer/naap
Length of output: 114
🏁 Script executed:
# Check if orchestrator-leaderboard frontend has package.json and its structure
test -f plugins/orchestrator-leaderboard/frontend/package.json && echo "EXISTS" || echo "NOT_FOUND"
# List all files in orchestrator-leaderboard frontend to understand structure
fd -t f . plugins/orchestrator-leaderboard/frontend/ --max-depth 1Repository: livepeer/naap
Length of output: 292
🏁 Script executed:
# Check actual content of orchestrator-leaderboard package.json (first 80 lines)
head -80 plugins/orchestrator-leaderboard/frontend/package.json
# Check for root-level playwright.config
cat playwright.config.ts 2>/dev/null || cat playwright.config.js 2>/dev/null || echo "No root config found"Repository: livepeer/naap
Length of output: 941
🏁 Script executed:
# List all orchestrator-leaderboard files to see what's there
find plugins/orchestrator-leaderboard -type f -name "*.ts" -o -name "*.json" | head -30Repository: livepeer/naap
Length of output: 590
🏁 Script executed:
# Check lightning-client/frontend/package.json for playwright
cat plugins/lightning-client/frontend/package.json | jq '.devDependencies | keys | map(select(contains("playwright")))'
# Check root package.json for playwright
cat package.json | jq '.devDependencies | keys | map(select(contains("playwright")))' 2>/dev/null || echo "Check failed"
# Also check if there's a monorepo setup with shared playwright
ls -la | grep -i playwright || echo "No playwright files at root"Repository: livepeer/naap
Length of output: 183
🏁 Script executed:
# Check if lightning-client structure is different
ls -la plugins/lightning-client/ 2>/dev/null | head -20
# Check what orchestrator-leaderboard package.json test script actually uses
cat plugins/orchestrator-leaderboard/frontend/package.json | jq '.scripts.test'
# Look for any playwright.config in the entire plugins directory
fd -t f 'playwright.config' plugins/Repository: livepeer/naap
Length of output: 406
🏁 Script executed:
# Check lightning-client structure
ls -la plugins/lightning-client/frontend/
# Verify what the test file is actually trying to do vs what can run
head -20 plugins/orchestrator-leaderboard/frontend/e2e/leaderboard.spec.ts
# Check if there's a shared package.json or scripts at root
cat package.json | jq '.scripts | keys | map(select(contains("test")))' 2>/dev/null || echo "No match"Repository: livepeer/naap
Length of output: 1386
🏁 Script executed:
# Check lightning-client/frontend content more carefully
find plugins/lightning-client/frontend -maxdepth 2 -type f | head -20
# Check if lightning-client has a different structure or parent package.json
cat plugins/lightning-client/frontend/playwright.config.ts | head -30
# Verify: does orchestrator-leaderboard/frontend have a way to run playwright tests?
grep -r "playwright" plugins/orchestrator-leaderboard/frontend/ 2>/dev/null || echo "No playwright mentions found"Repository: livepeer/naap
Length of output: 1035
🏁 Script executed:
# Check if there's a monorepo with shared playwright at root
cat package.json | grep -A 20 "devDependencies" | grep -i playwright
# Check orchestrator-leaderboard/frontend for any playwright-related tooling
ls -la plugins/orchestrator-leaderboard/frontend/e2e/
# Verify: is the test command in orchestrator-leaderboard configured to run playwright?
cat plugins/orchestrator-leaderboard/frontend/package.json | jq '.scripts'Repository: livepeer/naap
Length of output: 455
Playwright E2E tests cannot run: missing dependency and test configuration.
The test file imports from @playwright/test but playwright is not listed in devDependencies. Additionally, the test script is configured to run vitest run, not playwright test. To enable these E2E tests, add @playwright/test to devDependencies, create a playwright.config.ts file (following the pattern in plugins/lightning-client/frontend/playwright.config.ts), and update the test script to run Playwright instead of Vitest.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/orchestrator-leaderboard/frontend/e2e/leaderboard.spec.ts` around
lines 1 - 158, The test file imports from `@playwright/test` (see the top-level
import and use of test/expect in leaderboard.spec.ts) but the project is missing
that dependency and Playwright config; add `@playwright/test` to devDependencies,
create a playwright.config.ts (use the existing pattern from
plugins/lightning-client/frontend/playwright.config.ts) and ensure it points to
the tests directory, and update the package.json test script to run Playwright
(e.g., "playwright test") instead of "vitest run" so leaderboard.spec.ts
executes under Playwright.
seanhanca
force-pushed
the
feat/orchestrator-leaderboard
branch
from
e3325cc to
a5d24ca
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (6)
apps/web-next/src/app/api/v1/orchestrator-leaderboard/rank/route.ts (2)
28-43:⚠️ Potential issue | 🟠 MajorNull JSON body can cause 500 instead of 400.
If
request.json()returnsnull(which is valid JSON), accessingbody.capabilityon line 38 will throw a TypeError before reaching the validation response, resulting in a 500 error instead of the intended 400.🐛 Suggested fix
- let body: LeaderboardRequest; + let body: unknown; try { body = await request.json(); } catch { return NextResponse.json( { success: false, error: { code: 'VALIDATION_ERROR', message: 'Invalid JSON body' } }, { status: 400 } ); } - if (!body.capability || typeof body.capability !== 'string') { + if ( + !body || + typeof body !== 'object' || + Array.isArray(body) || + typeof (body as LeaderboardRequest).capability !== 'string' || + !(body as LeaderboardRequest).capability + ) { return NextResponse.json( { success: false, error: { code: 'VALIDATION_ERROR', message: 'capability is required and must be a string' } }, { status: 400 } ); } + + const validatedBody = body as LeaderboardRequest;🤖 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/orchestrator-leaderboard/rank/route.ts` around lines 28 - 43, The handler assumes request.json() returns an object and accesses body.capability directly, causing a TypeError if JSON is null; update the parsing/validation in route.ts (the body variable and LeaderboardRequest check) to treat null as invalid by verifying body is non-null and typeof body === 'object' before accessing body.capability, and return the existing 400 NextResponse with VALIDATION_ERROR when body is null or not an object.
83-90:⚠️ Potential issue | 🟠 MajorCache-Control
publicmay leak data across tenants.The route fetches data using the caller's
authTokenbut caches responses withCache-Control: public. If the ClickHouse data is tenant-scoped, one tenant's cached response could be served to another tenant via intermediate caches. Consider usingprivateunless the leaderboard data is intentionally public.🔐 Suggested fix if data should be private
- response.headers.set('Cache-Control', 'public, max-age=10'); + response.headers.set('Cache-Control', 'private, max-age=10');🤖 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/orchestrator-leaderboard/rank/route.ts` around lines 83 - 90, The Cache-Control header is incorrectly set to "public" which can leak tenant-scoped responses; in the handler that builds the Response (see the response variable and the block that sets response.headers.set('Cache-Control', ...)), change it to "private, max-age=10" (or make it conditional based on the request/tenant/public-flag if leaderboard data can be globally public) so cached responses are not served to other tenants; update the response.headers.set('Cache-Control', ...) call accordingly and ensure any tests/assertions expecting the previous header are updated.apps/web-next/src/lib/orchestrator-leaderboard/ranking.ts (1)
54-55:⚠️ Potential issue | 🟠 MajorNull latency/swap metrics still pass explicit max filters.
The current filter predicates allow rows with
nullvalues foravg_lat_msorswap_ratioto pass through whenmaxAvgLatencyMsormaxSwapRatiofilters are set. If a user explicitly sets a maximum threshold, rows without metric data should likely be excluded.🐛 Suggested fix
- if (filters.maxAvgLatencyMs != null && r.avg_lat_ms != null && r.avg_lat_ms > filters.maxAvgLatencyMs) return false; - if (filters.maxSwapRatio != null && r.swap_ratio != null && r.swap_ratio > filters.maxSwapRatio) return false; + if (filters.maxAvgLatencyMs != null && (r.avg_lat_ms == null || r.avg_lat_ms > filters.maxAvgLatencyMs)) return false; + if (filters.maxSwapRatio != null && (r.swap_ratio == null || r.swap_ratio > filters.maxSwapRatio)) return false;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web-next/src/lib/orchestrator-leaderboard/ranking.ts` around lines 54 - 55, The current filter logic in ranking.ts treats null metrics as passing when a max threshold is provided; update the predicates so that when filters.maxAvgLatencyMs is set, rows with r.avg_lat_ms === null or undefined are excluded (return false) in addition to those exceeding the threshold, and likewise when filters.maxSwapRatio is set exclude rows with r.swap_ratio === null/undefined or > filters.maxSwapRatio; modify the checks around filters.maxAvgLatencyMs / r.avg_lat_ms and filters.maxSwapRatio / r.swap_ratio accordingly in the filtering function.plugins/service-gateway/docs/clickhouse-query-connector.md (1)
13-19:⚠️ Potential issue | 🟡 MinorAvoid inline passwords in curl examples.
The example
curl --user 'default:YOUR_PASSWORD'can leak credentials via shell history and process listings. While this is documentation, it teaches users an insecure practice.🔐 Safer example that prompts for password
Verify your instance is reachable: ```bash -curl --user 'default:YOUR_PASSWORD' \ +# curl will prompt for password (avoids shell history leak) +curl --user 'default' \ --data-binary 'SELECT 1' \ https://your-instance.clickhouse.cloud:8443</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@plugins/service-gateway/docs/clickhouse-query-connector.mdaround lines 13 -
19, Replace the vulnerable example "curl --user 'default:YOUR_PASSWORD'
--data-binary 'SELECT 1' https://your-instance.clickhouse.cloud:8443" so it does
not include an inline password; change the user flag to just "--user 'default'"
(so curl will prompt for the password) and update the surrounding text to
recommend prompting or using a secure alternative (e.g., .netrc or
environment-based credential injection) instead of embedding credentials in
shell commands.</details> </blockquote></details> <details> <summary>plugins/orchestrator-leaderboard/frontend/src/pages/LeaderboardPage.tsx (2)</summary><blockquote> `446-482`: _⚠️ Potential issue_ | _🟠 Major_ **Give the filter controls accessible names.** Both `FilterInput` and `WeightSlider` render visible label text without associating it to the underlying input, so assistive tech will announce unlabeled controls here. <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@plugins/orchestrator-leaderboard/frontend/src/pages/LeaderboardPage.tsx
around lines 446 - 482, Both FilterInput and WeightSlider render visual labels
but do not associate them with their inputs; update FilterInput and WeightSlider
to provide explicit accessible labeling by giving each input an id and linking
the label via htmlFor (or use React's useId to generate stable ids) or
alternatively set aria-labelledby/aria-label on the input. Modify the
FilterInput component (props for label, value, onChange) to accept or generate
an id and ensure the targets the ; do the
same in WeightSlider (label text and the range ), keeping existing props
like value and onChange intact so no behavior changes occur. Ensure IDs are
unique (useId or a passed id prop) and update any JSX where these components are
used if you add an id prop.</details> --- `26-35`: _⚠️ Potential issue_ | _🟠 Major_ **Preserve zero-valued filters.** `hasActiveFilters` still relies on truthiness, so valid bounds like `0` are treated as inactive and dropped from the request payload. <details> <summary>Suggested fix</summary> ```diff - const hasActiveFilters = !!(filters.gpuRamGbMin || filters.gpuRamGbMax || filters.priceMax || filters.maxAvgLatencyMs || filters.maxSwapRatio); + const hasActiveFilters = + filters.gpuRamGbMin != null || + filters.gpuRamGbMax != null || + filters.priceMax != null || + filters.maxAvgLatencyMs != null || + filters.maxSwapRatio != null;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/orchestrator-leaderboard/frontend/src/pages/LeaderboardPage.tsx` around lines 26 - 35, The current hasActiveFilters calculation uses truthiness, dropping valid zero values (e.g., gpuRamGbMin = 0); change hasActiveFilters to test for null/undefined instead of falsiness so zero is preserved (e.g., (filters.gpuRamGbMin != null) || (filters.gpuRamGbMax != null) || (filters.priceMax != null) || (filters.maxAvgLatencyMs != null) || (filters.maxSwapRatio != null)), then keep the useMemo that builds request (LeaderboardRequest in the request variable) to use filters when hasActiveFilters is true; update only the hasActiveFilters expression so zero-valued bounds are included in the request payload.
🧹 Nitpick comments (1)
plugins/orchestrator-leaderboard/docs/api-reference.md (1)
103-115: Consider documenting thefromFallbackfield.The response example shows
"fromFallback"but its purpose isn't explained. Consider adding a brief description of what this field indicates (e.g., whether the capabilities came from a fallback source when ClickHouse is unavailable).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/orchestrator-leaderboard/docs/api-reference.md` around lines 103 - 115, The response example is missing documentation for the fromFallback field; update the API response section (the "Response" example that currently shows "capabilities") to include a fromFallback boolean and a one-line description explaining it indicates whether the listed capabilities were sourced from a fallback mechanism (e.g., used when ClickHouse or the primary store was unavailable), and ensure the example JSON and the surrounding text mention caching (Cache-Control) remains the same; modify the example JSON and add a short sentence describing fromFallback's semantics and possible values.
🤖 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/orchestrator-leaderboard/frontend/src/globals.css`:
- Around line 10-16: The CSS removed the native focus outline for
input[type="range"] making keyboard focus invisible; restore an accessible focus
state by adding a focus-visible rule for input[type="range"]:focus-visible (or
input[type="range"]:focus) that reintroduces a clear visual indicator (e.g., a
visible outline or box-shadow and/or border color change) while keeping the
existing custom appearance, ensuring the selector targets the same
input[type="range"] used in this file.
In `@plugins/orchestrator-leaderboard/frontend/src/pages/LeaderboardPage.tsx`:
- Around line 41-49: handleWeightChange currently allows all weights to become
zero which breaks the "normalize to 100%" behavior; update it so when computed
sum === 0 you set the changed key to a non-zero default (e.g., set updated[key]
= 1 and the other keys = 0) before normalization, then proceed with the existing
normalization and call setSlaWeights; reference the handler function
handleWeightChange, the SLAWeights object shape (latency, swapRate, price), and
the state updater setSlaWeights so the change is applied in the same place the
current code mutates updated.
- Around line 214-228: The FilterInput onChange handlers in LeaderboardPage are
coercing input to Number on every keystroke (see uses of setFilters and
properties like maxSwapRatio, priceMax, gpuRamGbMin, gpuRamGbMax,
maxAvgLatencyMs), which breaks decimal entry and yields NaN for intermediate
strings; change the filter state to store the raw string for these inputs (e.g.,
keep "0.", ".25", "") and only parse/validate to Number when you assemble or
submit the filters (submit handler / validation function) so typing is not
interrupted and numeric conversion happens at form validation/submission time.
---
Duplicate comments:
In `@apps/web-next/src/app/api/v1/orchestrator-leaderboard/rank/route.ts`:
- Around line 28-43: The handler assumes request.json() returns an object and
accesses body.capability directly, causing a TypeError if JSON is null; update
the parsing/validation in route.ts (the body variable and LeaderboardRequest
check) to treat null as invalid by verifying body is non-null and typeof body
=== 'object' before accessing body.capability, and return the existing 400
NextResponse with VALIDATION_ERROR when body is null or not an object.
- Around line 83-90: The Cache-Control header is incorrectly set to "public"
which can leak tenant-scoped responses; in the handler that builds the Response
(see the response variable and the block that sets
response.headers.set('Cache-Control', ...)), change it to "private, max-age=10"
(or make it conditional based on the request/tenant/public-flag if leaderboard
data can be globally public) so cached responses are not served to other
tenants; update the response.headers.set('Cache-Control', ...) call accordingly
and ensure any tests/assertions expecting the previous header are updated.
In `@apps/web-next/src/lib/orchestrator-leaderboard/ranking.ts`:
- Around line 54-55: The current filter logic in ranking.ts treats null metrics
as passing when a max threshold is provided; update the predicates so that when
filters.maxAvgLatencyMs is set, rows with r.avg_lat_ms === null or undefined are
excluded (return false) in addition to those exceeding the threshold, and
likewise when filters.maxSwapRatio is set exclude rows with r.swap_ratio ===
null/undefined or > filters.maxSwapRatio; modify the checks around
filters.maxAvgLatencyMs / r.avg_lat_ms and filters.maxSwapRatio / r.swap_ratio
accordingly in the filtering function.
In `@plugins/orchestrator-leaderboard/frontend/src/pages/LeaderboardPage.tsx`:
- Around line 446-482: Both FilterInput and WeightSlider render visual labels
but do not associate them with their inputs; update FilterInput and WeightSlider
to provide explicit accessible labeling by giving each input an id and linking
the label via htmlFor (or use React's useId to generate stable ids) or
alternatively set aria-labelledby/aria-label on the input. Modify the
FilterInput component (props for label, value, onChange) to accept or generate
an id and ensure the <label htmlFor={id}> targets the <input id={id}>; do the
same in WeightSlider (label text and the range <input>), keeping existing props
like value and onChange intact so no behavior changes occur. Ensure IDs are
unique (useId or a passed id prop) and update any JSX where these components are
used if you add an id prop.
- Around line 26-35: The current hasActiveFilters calculation uses truthiness,
dropping valid zero values (e.g., gpuRamGbMin = 0); change hasActiveFilters to
test for null/undefined instead of falsiness so zero is preserved (e.g.,
(filters.gpuRamGbMin != null) || (filters.gpuRamGbMax != null) ||
(filters.priceMax != null) || (filters.maxAvgLatencyMs != null) ||
(filters.maxSwapRatio != null)), then keep the useMemo that builds request
(LeaderboardRequest in the request variable) to use filters when
hasActiveFilters is true; update only the hasActiveFilters expression so
zero-valued bounds are included in the request payload.
In `@plugins/service-gateway/docs/clickhouse-query-connector.md`:
- Around line 13-19: Replace the vulnerable example "curl --user
'default:YOUR_PASSWORD' --data-binary 'SELECT 1'
https://your-instance.clickhouse.cloud:8443" so it does not include an inline
password; change the user flag to just "--user 'default'" (so curl will prompt
for the password) and update the surrounding text to recommend prompting or
using a secure alternative (e.g., .netrc or environment-based credential
injection) instead of embedding credentials in shell commands.
---
Nitpick comments:
In `@plugins/orchestrator-leaderboard/docs/api-reference.md`:
- Around line 103-115: The response example is missing documentation for the
fromFallback field; update the API response section (the "Response" example that
currently shows "capabilities") to include a fromFallback boolean and a one-line
description explaining it indicates whether the listed capabilities were sourced
from a fallback mechanism (e.g., used when ClickHouse or the primary store was
unavailable), and ensure the example JSON and the surrounding text mention
caching (Cache-Control) remains the same; modify the example JSON and add a
short sentence describing fromFallback's semantics and possible values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3fed1f01-47b6-458f-b60e-3eaced134a42
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json,!package-lock.json
📒 Files selected for processing (36)
apps/web-next/src/__tests__/api/orchestrator-leaderboard.test.tsapps/web-next/src/app/api/v1/gw/admin/templates/route.tsapps/web-next/src/app/api/v1/orchestrator-leaderboard/filters/route.tsapps/web-next/src/app/api/v1/orchestrator-leaderboard/rank/route.tsapps/web-next/src/lib/gateway/__tests__/transform.test.tsapps/web-next/src/lib/gateway/transform.tsapps/web-next/src/lib/orchestrator-leaderboard/__tests__/cache.test.tsapps/web-next/src/lib/orchestrator-leaderboard/__tests__/query.test.tsapps/web-next/src/lib/orchestrator-leaderboard/__tests__/ranking.test.tsapps/web-next/src/lib/orchestrator-leaderboard/cache.tsapps/web-next/src/lib/orchestrator-leaderboard/query.tsapps/web-next/src/lib/orchestrator-leaderboard/ranking.tsapps/web-next/src/lib/orchestrator-leaderboard/types.tsapps/web-next/src/middleware.tsbin/seed-public-connectors.tsplugins/orchestrator-leaderboard/docs/api-reference.mdplugins/orchestrator-leaderboard/examples/client-test.shplugins/orchestrator-leaderboard/examples/client-test.tsplugins/orchestrator-leaderboard/frontend/e2e/leaderboard.spec.tsplugins/orchestrator-leaderboard/frontend/package.jsonplugins/orchestrator-leaderboard/frontend/src/App.tsxplugins/orchestrator-leaderboard/frontend/src/globals.cssplugins/orchestrator-leaderboard/frontend/src/hooks/useCapabilities.tsplugins/orchestrator-leaderboard/frontend/src/hooks/useLeaderboard.tsplugins/orchestrator-leaderboard/frontend/src/lib/api.tsplugins/orchestrator-leaderboard/frontend/src/mount.tsxplugins/orchestrator-leaderboard/frontend/src/pages/LeaderboardPage.tsxplugins/orchestrator-leaderboard/frontend/tailwind.config.jsplugins/orchestrator-leaderboard/frontend/tsconfig.jsonplugins/orchestrator-leaderboard/frontend/vite.config.tsplugins/orchestrator-leaderboard/plugin.jsonplugins/service-gateway/connectors/clickhouse-query.jsonplugins/service-gateway/connectors/connector-template.schema.jsonplugins/service-gateway/connectors/loader.tsplugins/service-gateway/docs/clickhouse-query-connector.mdplugins/service-gateway/docs/connector-catalog.md
✅ Files skipped from review due to trivial changes (14)
- plugins/orchestrator-leaderboard/frontend/tailwind.config.js
- plugins/orchestrator-leaderboard/frontend/vite.config.ts
- apps/web-next/src/lib/gateway/tests/transform.test.ts
- plugins/orchestrator-leaderboard/frontend/tsconfig.json
- plugins/orchestrator-leaderboard/plugin.json
- plugins/orchestrator-leaderboard/frontend/src/hooks/useCapabilities.ts
- plugins/orchestrator-leaderboard/frontend/package.json
- plugins/orchestrator-leaderboard/examples/client-test.sh
- apps/web-next/src/lib/orchestrator-leaderboard/tests/query.test.ts
- apps/web-next/src/lib/orchestrator-leaderboard/tests/ranking.test.ts
- plugins/orchestrator-leaderboard/frontend/src/lib/api.ts
- apps/web-next/src/lib/orchestrator-leaderboard/cache.ts
- apps/web-next/src/lib/orchestrator-leaderboard/tests/cache.test.ts
- apps/web-next/src/lib/orchestrator-leaderboard/types.ts
🚧 Files skipped from review as they are similar to previous changes (12)
- apps/web-next/src/app/api/v1/gw/admin/templates/route.ts
- plugins/service-gateway/connectors/loader.ts
- plugins/service-gateway/connectors/connector-template.schema.json
- plugins/service-gateway/docs/connector-catalog.md
- apps/web-next/src/tests/api/orchestrator-leaderboard.test.ts
- plugins/orchestrator-leaderboard/frontend/src/hooks/useLeaderboard.ts
- plugins/orchestrator-leaderboard/frontend/e2e/leaderboard.spec.ts
- plugins/orchestrator-leaderboard/frontend/src/App.tsx
- plugins/orchestrator-leaderboard/examples/client-test.ts
- apps/web-next/src/app/api/v1/orchestrator-leaderboard/filters/route.ts
- apps/web-next/src/lib/orchestrator-leaderboard/query.ts
- apps/web-next/src/lib/gateway/transform.ts
Comment on lines
+214
to
+228
| <FilterInput label="Min GPU RAM (GB)" value={filters.gpuRamGbMin ?? ''} | ||
| onChange={(v) => setFilters({ ...filters, gpuRamGbMin: v !== '' ? Number(v) : undefined })} | ||
| type="number" min={0} step={1} /> | ||
| <FilterInput label="Max GPU RAM (GB)" value={filters.gpuRamGbMax ?? ''} | ||
| onChange={(v) => setFilters({ ...filters, gpuRamGbMax: v !== '' ? Number(v) : undefined })} | ||
| type="number" min={0} step={1} /> | ||
| <FilterInput label="Max Price" value={filters.priceMax ?? ''} | ||
| onChange={(v) => setFilters({ ...filters, priceMax: v !== '' ? Number(v) : undefined })} | ||
| type="number" min={0} step={0.001} /> | ||
| <FilterInput label="Max Avg Latency (ms)" value={filters.maxAvgLatencyMs ?? ''} | ||
| onChange={(v) => setFilters({ ...filters, maxAvgLatencyMs: v !== '' ? Number(v) : undefined })} | ||
| type="number" min={0} /> | ||
| <FilterInput label="Max Swap Ratio" value={filters.maxSwapRatio ?? ''} | ||
| onChange={(v) => setFilters({ ...filters, maxSwapRatio: v !== '' ? Number(v) : undefined })} | ||
| type="number" min={0} max={1} step={0.01} /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find the FilterInput component
fd -t f -e tsx -e ts FilterInputRepository: livepeer/naap
Length of output: 39
🏁 Script executed:
# Also check the LeaderboardPage.tsx file around the mentioned lines
cat -n plugins/orchestrator-leaderboard/frontend/src/pages/LeaderboardPage.tsx | sed -n '200,240p'Repository: livepeer/naap
Length of output: 2305
🏁 Script executed:
# Find the FilterInput component definition
rg -t tsx "FilterInput" -A 10 -B 2 | head -100Repository: livepeer/naap
Length of output: 84
🏁 Script executed:
# Search for FilterInput in all files
rg "FilterInput" -A 15 --type-list | head -5Repository: livepeer/naap
Length of output: 446
🏁 Script executed:
# Try with ts type instead
fd -t f -name "*FilterInput*"Repository: livepeer/naap
Length of output: 287
🏁 Script executed:
# Search in the frontend directory
fd -t f -name "*.ts" -o -name "*.tsx" | head -20Repository: livepeer/naap
Length of output: 287
🏁 Script executed:
# Find FilterInput definition in components
rg "export.*FilterInput|function FilterInput" -A 20Repository: livepeer/naap
Length of output: 39
🏁 Script executed:
# Find all source files
find . -type f \( -name "*.ts" -o -name "*.tsx" \) | grep -i filterinputRepository: livepeer/naap
Length of output: 39
🏁 Script executed:
# Search for FilterInput component definition
rg "FilterInput" --max-count 50 | head -30Repository: livepeer/naap
Length of output: 935
🏁 Script executed:
# Look for FilterInput in the entire directory
rg "export.*FilterInput" -A 20Repository: livepeer/naap
Length of output: 39
🏁 Script executed:
# Find the FilterInput component definition in LeaderboardPage.tsx
rg "const FilterInput:" -A 25 plugins/orchestrator-leaderboard/frontend/src/pages/LeaderboardPage.tsxRepository: livepeer/naap
Length of output: 1179
🏁 Script executed:
# Also check what imports are at the top of LeaderboardPage.tsx
head -50 plugins/orchestrator-leaderboard/frontend/src/pages/LeaderboardPage.tsxRepository: livepeer/naap
Length of output: 2219
🏁 Script executed:
# Check the request handling and API types
rg "LeaderboardRequest|LeaderboardFilters" -A 10 plugins/orchestrator-leaderboard/frontend/src/lib/api.ts | head -60Repository: livepeer/naap
Length of output: 1019
🏁 Script executed:
# Check if there's any request validation or conversion
rg "priceMax|maxSwapRatio" plugins/orchestrator-leaderboard/frontend/src/Repository: livepeer/naap
Length of output: 1089
Don't coerce filter input on every keystroke.
The onChange handlers immediately convert raw input text to Number(...), breaking decimal entry. When typing 0.25 into Max Swap Ratio, the intermediate state 0. coerces to 0, reverting the input field and making it impossible to continue. Similarly, typing . alone produces NaN, which can leak into requests. Decimal inputs with step={0.001} and step={0.01} are effectively unusable in their current form.
Store the raw string value in state and convert to numbers only when validating or submitting filters.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/orchestrator-leaderboard/frontend/src/pages/LeaderboardPage.tsx`
around lines 214 - 228, The FilterInput onChange handlers in LeaderboardPage are
coercing input to Number on every keystroke (see uses of setFilters and
properties like maxSwapRatio, priceMax, gpuRamGbMin, gpuRamGbMax,
maxAvgLatencyMs), which breaks decimal entry and yields NaN for intermediate
strings; change the filter state to store the raw string for these inputs (e.g.,
keep "0.", ".25", "") and only parse/validate to Number when you assemble or
submit the filters (submit handler / validation function) so typing is not
interrupted and numeric conversion happens at form validation/submission time.
|
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 |
github-actions
bot
added
needs-rebase
github-actions
bot
added
needs-rebase
Introduce an `"experimental": true` flag in plugin.json that causes new plugins to auto-register as hidden (visibleToUsers: false) during sync. Admins can then add specific preview testers via the admin API, keeping the plugin invisible to all other users. Works identically on local dev and Vercel preview deployments (separate Neon DB branches). - plugin-discovery.ts: read experimental flag, pass through to package data - sync-plugin-registry.ts: set visibleToUsers=false only on first create - bin/preview-plugin.sh: helper script to configure any experimental plugin - docs/experimental-plugin-preview.md: full workflow documentation Made-with: Cursor
…E tests - Remove NEXT_PUBLIC_APP_URL/NEXTAUTH_URL from committed .env (use .env.local) - Patch start.sh to sync NEXT_PUBLIC_APP_URL with SHELL_PORT on every start - Remove hardcoded localhost:3000 from CORS allowlist in next.config.js - Consolidate 7 inline localhost fallbacks to use shared appUrl from lib/env - Fix layout.tsx stale localhost:3001 fallback - Use request.url origin for ClickHouse gateway URL resolution - Add auth headers + credentials to plugin frontend API calls - Make Playwright webServer.url respect PLAYWRIGHT_BASE_URL - Add orchestrator-leaderboard E2E spec (stub + live modes) - Add resolveClickhouseGatewayQueryUrl unit tests Made-with: Cursor
Add persistent DiscoveryPlan model to store parameterized discovery queries for external consumers (signers, SDKs). Uses a dedicated plugin_orchestrator_leaderboard Postgres schema with indexes on teamId and ownerUserId for scoped access. Made-with: Cursor
Extend types.ts with DiscoveryPlan, PlanResults, PlanSortBy types and Zod schemas for create/update validation. Capabilities are validated against the existing alphanumeric pattern. Made-with: Cursor
Create plans.ts with create, list, get, update, delete functions for DiscoveryPlan records. Plans are scoped by teamId or ownerUserId. Includes listEnabledPlans() for the cron refresh path. Made-with: Cursor
Add evaluatePlan() to ranking.ts that applies the full pipeline: filter -> SLA score -> min-gate -> custom sort -> topN. Supports all PlanSortBy options (slaScore, latency, price, swapRate, avail). Made-with: Cursor
…upport Create refresh.ts with: - evaluateAndCache(): lazy evaluation with stale-while-revalidate - refreshAllPlans(): bulk refresh for Vercel Cron warming - startLocalRefreshLoop(): optional setInterval for local dev - In-memory plan result cache with configurable TTL Made-with: Cursor
Create API routes for the Discovery Plan system: - GET/POST /plans — list and create plans - GET/PUT/DELETE /plans/:id — get, update, delete a plan - GET /plans/:id/results — lazy-evaluated cached results - POST /plans/refresh — cron-triggered bulk refresh Made-with: Cursor
Schedule plan refresh every 1 minute so most reads are cache hits. Uses CRON_SECRET auth (same pattern as bff-warm). Made-with: Cursor
Add 3 new test files (29 tests total): - evaluate-plan.test.ts: evaluatePlan() pipeline (filter, SLA, gate, sort, topN) - types-validation.test.ts: Zod schema validation (CreatePlan, UpdatePlan) - refresh.test.ts: evaluateAndCache(), plan cache behavior Made-with: Cursor
The function is synchronous so await import() is invalid. Switch to a top-level static import to fix the Vercel build. Made-with: Cursor
…d webhook guide - Add TabNav component for Leaderboard / Plans tab switching - Add PlansOverviewPage with summary stats, plan cards, endpoint info - Add PlanDetailPage with live-editing config (SLA weights, min score, topN, sort, filters) and "Apply Changes" to see results update - Add EndpointGuide component showing API endpoint URL, copy button, and 3-step webhook setup guide for signer ORCHESTRATOR_DISCOVERY_URL - Add usePlans/usePlanDetail hooks for data fetching and state management - Add frontend API functions: fetchPlans, fetchPlanResults, updatePlan, seedDemoPlans - Add backend POST /plans/seed route for dev-only demo data (4 plans) Made-with: Cursor
The results endpoint returns success({ data: results }) which wraps the
response as { success, data: { data: PlanResults } }. The frontend was
accessing json.data directly, causing "Cannot read properties of
undefined (reading 'totalOrchestrators')" at runtime.
Made-with: Cursor
Migrate orchestrator-leaderboard plugin from hardcoded Tailwind gray/blue classes to NaaP's CSS-variable-based design tokens. Rewrites tailwind.config.js with semantic color mappings, globals.css with :root/.dark CSS variables and .glass-card, and all 7 TSX component files with token-backed utility classes. Also fixes a bug where updating a plan's configuration (e.g. SLA min score) did not refresh the orchestrator results — the PUT handler now invalidates the in-memory planCache entry so the next GET /results forces a fresh evaluation. Made-with: Cursor
… preview Create bin/seed-gateway-connector.ts that seeds the clickhouse-query ServiceConnector, its endpoints, and encrypted upstream credentials (username/password) into SecretVault using Prisma directly — no running app or base-svc required. Add step 3.5 to vercel-build.sh that runs this seed after prisma db push when CLICKHOUSE_QUERY_USERNAME and CLICKHOUSE_QUERY_PASSWORD are set. This enables the orchestrator-leaderboard plugin to query ClickHouse on Vercel preview deployments, matching the local dev experience. Made-with: Cursor
Made-with: Cursor
…ookup The User model has no direct `role` column — roles are in a separate UserRole relation table. Use findFirst with orderBy createdAt instead. Made-with: Cursor
Server-to-server calls from leaderboard routes to the gateway endpoint go through the external URL, which gets intercepted by Vercel's deployment protection SSO on preview deployments. Forward the incoming request's cookie header (contains _vercel_jwt) and also support VERCEL_AUTOMATION_BYPASS_SECRET for cron-initiated calls. Made-with: Cursor
Adds bin/seed-discovery-plans.ts (idempotent) that creates 4 demo plans during Vercel preview/production builds so the leaderboard UI has data to display without manual API calls. Runs as step 3.6 in vercel-build.sh after schema push and gateway connector seed. Made-with: Cursor
seanhanca
force-pushed
the
feat/orchestrator-leaderboard
branch
from
04498d5 to
43696c6
Compare
… scoping
- Fix auth redirect loop: always clear httpOnly cookie via logout endpoint
before redirecting to /login, preventing middleware loop when DB is
unreachable or session is invalid
- Fix plan visibility: use OR logic in scopeWhere so plans match by either
teamId or ownerUserId (build-time seed sets ownerUserId only)
- Fix seed button hidden on Vercel: remove isLocalhost gate so all
authenticated users can seed demo data
- Fix seed API blocked on Vercel: remove NODE_ENV=production check that
blocked preview deployments
- Fix per-user seeding: use user-scoped billingPlanIds (demo-{userId}-slug)
so each user gets their own isolated set of demo plans without unique
constraint collisions
- Set teamId in build-time seed for proper scope matching
Made-with: Cursor
…dmin-configurable refresh - Make orchestrator-leaderboard a core plugin (isCore: true, remove experimental) - Add LeaderboardConfig Prisma model (singleton, refreshIntervalHours, lastRefreshedAt) - Add global dataset in-memory cache with full-replace strategy and configurable TTL - Add time-gated cron endpoint (hourly Vercel cron, skips if interval not elapsed) - Wire plan evaluation to read from global dataset cache (zero ClickHouse calls when populated) - Add GET /dataset, GET/PUT /dataset/config, POST /dataset/refresh API routes - Add AdminSettings panel in plugin frontend (1h/4h/8h/12h selector, refresh now, admin-only) - Add description field to DiscoveryPlan and plan metadata in results response - Add unit tests for config, global dataset cache, plan evaluation with global dataset - Add e2e tests for admin settings panel and plan results metadata Made-with: Cursor
… error handling - Fix TOCTOU race in updatePlan: use updateMany with scoped where clause - Fix TOCTOU race in deletePlan: use deleteMany with scoped where clause - Consistent nullable field handling (remove ?? undefined for slaWeights/filters) - Add .catch() to fire-and-forget refreshSingle in stale-while-revalidate path - Add .catch() to local dev refresh loop to surface errors - Log per-capability evaluation failures instead of silently swallowing Made-with: Cursor
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
2 participants
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
clickhouse-querygateway connector template withquery,orchestrator-leaderboard, andpingendpoints, along with documentation and the connector loader.Cache-Control+X-Cacheheaders.Authorizationheader from being forwarded with basic auth to upstream services.Included Commits
e3325cc863fdff6f31679df46e940b122accd67021cfb0dcKey Files
plugins/orchestrator-leaderboard/— plugin config, frontend UI, docs, examplesapps/web-next/src/app/api/v1/orchestrator-leaderboard/—/rankand/filtersAPI routesapps/web-next/src/lib/orchestrator-leaderboard/— query builder, ranking engine, cache, typesplugins/service-gateway/connectors/clickhouse-query.json— connector templateplugins/service-gateway/docs/clickhouse-query-connector.md— connector documentationTest plan
/rankand/filtersendpointsMade with Cursor
Summary by CodeRabbit
New Features
API / Behavior
Documentation
Tests