feat(admin): add history graphs, cost charts, logs - #1800
Merged
Merged
Conversation
- Add provider/model/mapping history API endpoints with 1m-24h time windows querying from history tables - Add global and org-level cost-by-model API endpoints - Add totalProcessed metric (Stripe gross revenue) - Expand project logs API to return full request details - Add expandable history charts on provider/model pages - Add cost-by-model bar charts on dashboard and org page - Replace simple log rows with rich expandable log cards with routing info, cost breakdown, and copyable IDs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (3)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughAdds enterprise admin UI under ee/admin, new history and cost-by-model server routes and schemas, client components (charts, tables, log cards), centralized admin types and a server API client, and updates infra/build targets to reference the EE admin bundle. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
apps/admin/src/components/providers-table.tsx (1)
55-57: Consider adding keyboard accessibility for row expansion.The row is clickable via
onClickbut may not be keyboard-accessible. Consider addingtabIndex={0}andonKeyDownhandler for Enter/Space keys to improve accessibility.🔧 Optional accessibility enhancement
<TableRow className="cursor-pointer hover:bg-muted/50" onClick={() => setExpanded(!expanded)} + tabIndex={0} + onKeyDown={(e) => { + if (e.key === "Enter" || e.key === " ") { + e.preventDefault(); + setExpanded(!expanded); + } + }} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin/src/components/providers-table.tsx` around lines 55 - 57, The TableRow currently toggles expansion only via onClick; make it keyboard-accessible by adding tabIndex={0} and an onKeyDown handler that listens for Enter (key === "Enter") and Space (key === " ") to call setExpanded(!expanded) (or the same toggle function used by onClick); also add appropriate ARIA attributes like role="button" and aria-expanded={expanded} on the TableRow to expose state to assistive tech.apps/admin/src/components/cost-by-model-chart.tsx (1)
89-91: Consider replacingconsole.errorwith a structured logging approach.ESLint flags this
console.errorstatement. While error logging here is reasonable for debugging, consider using a dedicated logger or removing it in production builds if the admin app has a logging utility.🔧 Optional: suppress or use logger
} catch (error) { - console.error("Failed to load cost by model:", error); + // eslint-disable-next-line no-console + console.error("Failed to load cost by model:", error); setData(null);Or, if a logger is available:
- console.error("Failed to load cost by model:", error); + logger.error("Failed to load cost by model:", error);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin/src/components/cost-by-model-chart.tsx` around lines 89 - 91, The catch block in the cost-by-model-chart component currently uses console.error in the error handler (inside the async data load in CostByModelChart / the fetch/useEffect block); replace that with your app's logging utility or remove it for production: import and call the shared logger (e.g., logger.error("Failed to load cost by model", error)) or guard with an environment check before logging, and keep the existing setData(null) behavior so the UI still clears on failure.
🤖 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/admin/src/components/history-chart.tsx`:
- Around line 105-116: The loadData callback currently treats a null result from
fetchData (which indicates API/auth failures) the same as an empty result, and
logs errors with console.error; update loadData (and the analogous block at the
other occurrence) to: detect when fetchData returns null and treat that as a
fetch failure (do not convert to []), set a dedicated error state (e.g.,
setError) with a descriptive message, avoid using console.error (use an existing
logger or set the error state instead), and only setData(result) when result is
a real array; ensure setLoading is still cleared in finally. Also check/follow
the same pattern where fetchServerData is used so failures surface as errors
rather than "No data".
- Around line 128-139: The current summaryStats.avgTtft re-averages per-bucket
avgTtft equally; change it to a weighted average using each bucket's non-cached
request volume (logsCount - cachedCount) as the weight: iterate data filtering
buckets with avgTtft !== null and weight > 0, accumulate sum += (d.avgTtft *
weight) and totalWeight += weight, then set avgTtft = totalWeight > 0 ?
Math.round(sum / totalWeight) : null; update the calculation referenced as
summaryStats and the avgTtft computation to use these names (data, logsCount,
cachedCount, avgTtft) and guard against negative weights or division by zero.
In `@apps/admin/src/components/log-card.tsx`:
- Around line 126-141: The expander and icon-only copy buttons in the LogCard
component lack accessible names and state; update the Button that toggles
isExpanded (the one rendering ChevronUp/ChevronDown) to include
aria-expanded={isExpanded} and a descriptive aria-label (e.g., "Toggle log
details"), and add meaningful aria-labels to the icon-only copy buttons (e.g.,
"Copy request body" / "Copy response body" or include dynamic context). Locate
the Button instances that render ChevronUp/ChevronDown and the copy icon Buttons
in log-card.tsx and add the appropriate aria attributes so screen readers get
both the control purpose and the expanded state.
In `@apps/admin/src/components/models-table.tsx`:
- Around line 129-143: The headers in models-table.tsx were made static but the
parent page still maintains sortBy/sortOrder, so restore clickable sortable
headers: update TableHead cells (e.g., the Model, Requests, Errors, Error Rate,
Last Updated columns) to call a passed-in onSort prop (or dispatch an event)
that toggles sortOrder and sets sortBy to the corresponding key; use the
existing sortBy/sortOrder props from apps/admin/src/app/models/page.tsx to
render an active sort indicator in the relevant TableHead and ensure the handler
signature matches the page’s state updater so admins can pivot by errors,
requests, updatedAt, etc.
---
Nitpick comments:
In `@apps/admin/src/components/cost-by-model-chart.tsx`:
- Around line 89-91: The catch block in the cost-by-model-chart component
currently uses console.error in the error handler (inside the async data load in
CostByModelChart / the fetch/useEffect block); replace that with your app's
logging utility or remove it for production: import and call the shared logger
(e.g., logger.error("Failed to load cost by model", error)) or guard with an
environment check before logging, and keep the existing setData(null) behavior
so the UI still clears on failure.
In `@apps/admin/src/components/providers-table.tsx`:
- Around line 55-57: The TableRow currently toggles expansion only via onClick;
make it keyboard-accessible by adding tabIndex={0} and an onKeyDown handler that
listens for Enter (key === "Enter") and Space (key === " ") to call
setExpanded(!expanded) (or the same toggle function used by onClick); also add
appropriate ARIA attributes like role="button" and aria-expanded={expanded} on
the TableRow to expose state to assistive tech.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7bb5d4dd-db5e-4479-8c44-56fe22b6bf5e
⛔ Files ignored due to path filters (4)
apps/admin/src/lib/api/v1.d.tsis excluded by!**/v1.d.tsapps/code/src/lib/api/v1.d.tsis excluded by!**/v1.d.tsapps/playground/src/lib/api/v1.d.tsis excluded by!**/v1.d.tsapps/ui/src/lib/api/v1.d.tsis excluded by!**/v1.d.ts
📒 Files selected for processing (16)
apps/admin/src/app/models/page.tsxapps/admin/src/app/organizations/[orgId]/org-cost-by-model.tsxapps/admin/src/app/organizations/[orgId]/page.tsxapps/admin/src/app/organizations/[orgId]/projects/[projectId]/project-logs.tsxapps/admin/src/app/page.tsxapps/admin/src/app/providers/page.tsxapps/admin/src/components/cost-by-model-chart.tsxapps/admin/src/components/dashboard-cost-by-model.tsxapps/admin/src/components/history-chart.tsxapps/admin/src/components/log-card.tsxapps/admin/src/components/models-table.tsxapps/admin/src/components/providers-table.tsxapps/admin/src/lib/admin-history.tsapps/admin/src/lib/admin-metrics.tsapps/admin/src/lib/admin-organizations.tsapps/api/src/routes/admin.ts
Comment on lines
+105
to
+116
| const loadData = useCallback( | ||
| async (w: HistoryWindow) => { | ||
| setLoading(true); | ||
| try { | ||
| const result = await fetchData(w); | ||
| setData(result ?? []); | ||
| } catch (error) { | ||
| console.error("Failed to load history:", error); | ||
| setData([]); | ||
| } finally { | ||
| setLoading(false); | ||
| } |
Contributor
There was a problem hiding this comment.
Don’t render fetch failures as “No data”.
fetchServerData() resolves null on API/auth failures, and this path converts that into [], so admins get the same “No data for this time window” message for both a real empty window and a failed request. That hides outages/auth problems as zero traffic, and Line 112 is also already tripping the current no-console rule.
🛠️ Suggested fix
const [data, setData] = useState<HistoryDataPoint[]>([]);
const [loading, setLoading] = useState(true);
+ const [loadError, setLoadError] = useState(false);
const [window, setWindow] = useState<HistoryWindow>("4h");
const [activeMetric, setActiveMetric] = useState<ActiveMetric>("requests");
const loadData = useCallback(
async (w: HistoryWindow) => {
setLoading(true);
+ setLoadError(false);
try {
const result = await fetchData(w);
- setData(result ?? []);
- } catch (error) {
- console.error("Failed to load history:", error);
+ if (result === null) {
+ setLoadError(true);
+ setData([]);
+ return;
+ }
+ setData(result);
+ } catch {
+ setLoadError(true);
setData([]);
} finally {
setLoading(false);
}
@@
{loading ? (
<div className="flex h-[200px] items-center justify-center text-sm text-muted-foreground">
Loading...
</div>
+ ) : loadError ? (
+ <div className="flex h-[200px] items-center justify-center text-sm text-muted-foreground">
+ Failed to load history for this time window
+ </div>
) : data.length === 0 ? (
<div className="flex h-[200px] items-center justify-center text-sm text-muted-foreground">
No data for this time windowAlso applies to: 213-220
🧰 Tools
🪛 ESLint
[error] 112-112: Unexpected console statement.
(no-console)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/admin/src/components/history-chart.tsx` around lines 105 - 116, The
loadData callback currently treats a null result from fetchData (which indicates
API/auth failures) the same as an empty result, and logs errors with
console.error; update loadData (and the analogous block at the other occurrence)
to: detect when fetchData returns null and treat that as a fetch failure (do not
convert to []), set a dedicated error state (e.g., setError) with a descriptive
message, avoid using console.error (use an existing logger or set the error
state instead), and only setData(result) when result is a real array; ensure
setLoading is still cleared in finally. Also check/follow the same pattern where
fetchServerData is used so failures surface as errors rather than "No data".
Comment on lines
+128
to
+139
| const summaryStats = { | ||
| totalRequests: data.reduce((sum, d) => sum + d.logsCount, 0), | ||
| totalErrors: data.reduce((sum, d) => sum + d.errorsCount, 0), | ||
| avgTtft: | ||
| data.filter((d) => d.avgTtft !== null).length > 0 | ||
| ? Math.round( | ||
| data | ||
| .filter((d) => d.avgTtft !== null) | ||
| .reduce((sum, d) => sum + (d.avgTtft ?? 0), 0) / | ||
| data.filter((d) => d.avgTtft !== null).length, | ||
| ) | ||
| : null, |
Contributor
There was a problem hiding this comment.
Weight the TTFT summary by non-cached request volume.
avgTtft is already a per-bucket average from the history API. Re-averaging those bucket means equally here makes a 1-request minute count the same as a 1,000-request minute, so the header can be badly skewed. Use each bucket’s logsCount - cachedCount as the weight.
📊 Suggested fix
const summaryStats = {
totalRequests: data.reduce((sum, d) => sum + d.logsCount, 0),
totalErrors: data.reduce((sum, d) => sum + d.errorsCount, 0),
- avgTtft:
- data.filter((d) => d.avgTtft !== null).length > 0
- ? Math.round(
- data
- .filter((d) => d.avgTtft !== null)
- .reduce((sum, d) => sum + (d.avgTtft ?? 0), 0) /
- data.filter((d) => d.avgTtft !== null).length,
- )
- : null,
+ avgTtft: (() => {
+ const ttftBuckets = data.filter((d) => d.avgTtft !== null);
+ const nonCachedRequests = ttftBuckets.reduce(
+ (sum, d) => sum + Math.max(d.logsCount - d.cachedCount, 0),
+ 0,
+ );
+ if (nonCachedRequests === 0) {
+ return null;
+ }
+ const weightedTtft = ttftBuckets.reduce(
+ (sum, d) =>
+ sum + (d.avgTtft ?? 0) * Math.max(d.logsCount - d.cachedCount, 0),
+ 0,
+ );
+ return Math.round(weightedTtft / nonCachedRequests);
+ })(),
errorRate:
data.reduce((sum, d) => sum + d.logsCount, 0) > 0
? (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/admin/src/components/history-chart.tsx` around lines 128 - 139, The
current summaryStats.avgTtft re-averages per-bucket avgTtft equally; change it
to a weighted average using each bucket's non-cached request volume (logsCount -
cachedCount) as the weight: iterate data filtering buckets with avgTtft !== null
and weight > 0, accumulate sum += (d.avgTtft * weight) and totalWeight +=
weight, then set avgTtft = totalWeight > 0 ? Math.round(sum / totalWeight) :
null; update the calculation referenced as summaryStats and the avgTtft
computation to use these names (data, logsCount, cachedCount, avgTtft) and guard
against negative weights or division by zero.
Comment on lines
+126
to
+141
| <Button | ||
| variant="ghost" | ||
| size="sm" | ||
| className="h-8 w-8 shrink-0 p-0" | ||
| onClick={() => setIsExpanded(!isExpanded)} | ||
| > | ||
| {isExpanded ? ( | ||
| <ChevronUp className="h-4 w-4" /> | ||
| ) : ( | ||
| <ChevronDown className="h-4 w-4" /> | ||
| )} | ||
| </Button> | ||
| </div> | ||
|
|
||
| {isExpanded && ( | ||
| <div className="space-y-4 p-4"> |
Contributor
There was a problem hiding this comment.
Give the icon-only controls accessible names and state.
The expander and copy buttons have no accessible label, and the toggle does not expose aria-expanded. Screen-reader users will only hear a generic “button”, which makes the new log details hard to operate.
♿ Suggested fix
export function LogCard({ log }: { log: ProjectLogEntry }) {
const [isExpanded, setIsExpanded] = useState(false);
+ const detailsId = `log-details-${log.id}`;
let StatusIcon = CheckCircle2;
let color = "text-green-500";
let bgColor = "bg-green-100";
@@
<Button
variant="ghost"
size="sm"
+ type="button"
className="h-8 w-8 shrink-0 p-0"
+ aria-expanded={isExpanded}
+ aria-controls={detailsId}
+ aria-label={isExpanded ? "Collapse log details" : "Expand log details"}
onClick={() => setIsExpanded(!isExpanded)}
>
@@
- {isExpanded && (
- <div className="space-y-4 p-4">
+ {isExpanded && (
+ <div id={detailsId} className="space-y-4 p-4">
@@
{log.requestId && (
<button
+ type="button"
+ aria-label="Copy request ID"
className="text-muted-foreground hover:text-foreground"
onClick={() => copyToClipboard(log.requestId!)}
>
@@
<button
+ type="button"
+ aria-label="Copy log ID"
className="text-muted-foreground hover:text-foreground"
onClick={() => copyToClipboard(log.id)}
>Also applies to: 149-166
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/admin/src/components/log-card.tsx` around lines 126 - 141, The expander
and icon-only copy buttons in the LogCard component lack accessible names and
state; update the Button that toggles isExpanded (the one rendering
ChevronUp/ChevronDown) to include aria-expanded={isExpanded} and a descriptive
aria-label (e.g., "Toggle log details"), and add meaningful aria-labels to the
icon-only copy buttons (e.g., "Copy request body" / "Copy response body" or
include dynamic context). Locate the Button instances that render
ChevronUp/ChevronDown and the copy icon Buttons in log-card.tsx and add the
appropriate aria attributes so screen readers get both the control purpose and
the expanded state.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (10)
ee/admin/src/components/history-chart.tsx (1)
82-89: Remove dead code: both branches return the same format.The
formatTimestampfunction has identical return values for both branches ("HH:mm"), making the conditional logic pointless. Either differentiate the formats (e.g., include date for 24h window) or simplify to a single return.♻️ Option 1: Simplify if formats are intentionally the same
function formatTimestamp(ts: string, window: HistoryWindow): string { const date = new Date(ts); - const minuteWindows = new Set(["1m", "2m", "5m", "30m", "1h", "2h"]); - if (minuteWindows.has(window)) { - return format(date, "HH:mm"); - } return format(date, "HH:mm"); }♻️ Option 2: Differentiate formats for longer windows
function formatTimestamp(ts: string, window: HistoryWindow): string { const date = new Date(ts); const minuteWindows = new Set(["1m", "2m", "5m", "30m", "1h", "2h"]); if (minuteWindows.has(window)) { return format(date, "HH:mm"); } - return format(date, "HH:mm"); + return format(date, "MMM d HH:mm"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ee/admin/src/components/history-chart.tsx` around lines 82 - 89, The conditional in formatTimestamp is dead: both branches return format(date, "HH:mm"); remove the minuteWindows check and return a single format, or if longer windows should show more context, change the non-minute branch to a different format (e.g., include date) instead of "HH:mm". Update the function formatTimestamp (and the minuteWindows Set/HistoryWindow usage) accordingly so there is no redundant branch.ee/admin/src/components/providers-table.tsx (1)
23-35: Consider extracting shared formatting utilities.
formatNumberandformatDateappear to be duplicated across components (similar patterns exist in the codebase). Consider extracting these to a shared utility file (e.g.,@/lib/format) to reduce duplication.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ee/admin/src/components/providers-table.tsx` around lines 23 - 35, formatNumber and formatDate are duplicated utilities; extract them into a shared module (e.g., a new "@/lib/format" file) and replace the local functions in providers-table.tsx with imports. Move the Intl.NumberFormat and Date formatting logic into exported functions (formatNumber, formatDate) in the new module, update providers-table.tsx to import and use those exports, and update any other components that duplicate the same logic to import from the shared module instead.ee/admin/src/app/providers/page.tsx (1)
9-9: Consider importingSortOrderfrom the source of truth.
SortOrderis redeclared locally here but is already exported fromadmin-providers.ts. Importing it ensures consistency if the type definition changes.-import type { ProviderSortBy } from "@/lib/admin-providers"; - -type SortOrder = "asc" | "desc"; +import type { ProviderSortBy, SortOrder } from "@/lib/admin-providers";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ee/admin/src/app/providers/page.tsx` at line 9, The local type alias SortOrder in page.tsx duplicates a type already exported from admin-providers.ts; remove the local declaration and import SortOrder from admin-providers.ts instead to keep a single source of truth (update the import list at the top of ee/admin/src/app/providers/page.tsx to include SortOrder and delete the local `type SortOrder = "asc" | "desc";` declaration).ee/admin/src/app/models/page.tsx (2)
11-31: Consider extractingSignInPromptto a shared component.
SignInPromptis duplicated across multiple pages (providers, models, and likely others). Extracting it to a shared component under@/componentswould reduce duplication.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ee/admin/src/app/models/page.tsx` around lines 11 - 31, The SignInPrompt component is duplicated; extract it into a shared component (e.g., create a new component named SignInPrompt in the shared components folder) and replace the inline SignInPrompt definitions with a single import and usage; ensure the new SignInPrompt exports the same component signature and retains use of Button and Link (preserve className and props such as size or asChild if needed), update all pages that defined their own SignInPrompt (e.g., models and providers pages) to import SignInPrompt and remove the local duplicates so there’s one centralized component to maintain.
11-11: ImportSortOrderfrom the shared module.-import type { ModelSortBy } from "@/lib/admin-models"; - -type SortOrder = "asc" | "desc"; +import type { ModelSortBy, SortOrder } from "@/lib/admin-models";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ee/admin/src/app/models/page.tsx` at line 11, The file declares a local type alias `SortOrder = "asc" | "desc"` but the review asks to import the shared definition instead; remove the local `type SortOrder` declaration and import `SortOrder` from the shared module where the canonical type lives, then update any uses in this file (e.g., props, interfaces, or function signatures in page.tsx) to reference the imported `SortOrder` symbol so the file uses the centralized type.ee/admin/src/components/log-card.tsx (1)
31-33: Consider providing user feedback after copying.
copyToClipboardsilently writes to the clipboard with no visual confirmation. A brief toast or icon change would improve UX.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ee/admin/src/components/log-card.tsx` around lines 31 - 33, The copyToClipboard function currently writes to the clipboard silently; update it to surface success/failure feedback by awaiting navigator.clipboard.writeText(text) and then triggering a UI notification (e.g., a toast) or toggling a copied state used by LogCard to show a transient icon change; specifically modify copyToClipboard(text: string) to return a Promise or accept a callback, handle errors and call the app's existing toast/snackbar utility or set a local copied boolean in the surrounding component (LogCard) so the user sees a success or error message and the copied state clears after a short timeout.ee/admin/src/lib/admin-history.ts (2)
26-45: Inconsistent URL encoding:providerIdis not encoded.
getModelHistory(line 57) andgetMappingHistory(line 79) useencodeURIComponent()formodelId, butproviderIdis never encoded. If provider IDs can contain special characters, this could cause request failures.For consistency and safety, consider encoding
providerIdas well:♻️ Proposed fix
export async function getProviderHistory( providerId: string, window: HistoryWindow, ): Promise<HistoryDataPoint[] | null> { if (!(await hasSession())) { return null; } const data = await fetchServerData<HistoryResponse>( "GET", - `/admin/providers/${providerId}/history` as "/admin/providers", + `/admin/providers/${encodeURIComponent(providerId)}/history` as "/admin/providers", { params: { query: { window }, }, }, ); return data?.data ?? null; }Similarly for
getMappingHistoryat line 79:- `/admin/providers/${providerId}/models/${encodeURIComponent(modelId)}/history` as "/admin/providers", + `/admin/providers/${encodeURIComponent(providerId)}/models/${encodeURIComponent(modelId)}/history` as "/admin/providers",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ee/admin/src/lib/admin-history.ts` around lines 26 - 45, The getProviderHistory function constructs the request URL without encoding providerId; update getProviderHistory to use encodeURIComponent(providerId) when building the path passed to fetchServerData (same pattern as getModelHistory and getMappingHistory) so special characters are percent-encoded and the GET to `/admin/providers/${...}/history` is safe and consistent.
14-20: Consider extractinghasSession()to a shared utility.This function duplicates the
hasSession()implementation inadmin-organizations.ts(lines 131-141). Consider extracting it to a shared module (e.g.,server-api.tsor a newauth-utils.ts) to adhere to DRY principles.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ee/admin/src/lib/admin-history.ts` around lines 14 - 20, The hasSession() implementation is duplicated; extract it into a shared utility (e.g., create and export async function hasSession() from a new auth-utils module or add it to server-api) and replace the duplicate implementations in admin-history.ts and admin-organizations.ts with an import of that shared hasSession; keep the same async signature and behavior (using cookies(), key "better-auth.session_token" and "__Secure-better-auth.session_token") and update both files to call the imported hasSession to eliminate duplication.ee/admin/src/components/cost-by-model-chart.tsx (2)
83-97: Replaceconsole.errorwith structured error handling or silent fallback.The static analysis tool flagged the
console.errorstatement. Consider either:
- Removing it since the UI already shows a "No data" state
- Using a structured logging utility if one exists in the project
♻️ Proposed fix
const loadData = useCallback( async (w: TokenWindow) => { setLoading(true); try { const result = await fetchData(w); setData(result); - } catch (error) { - console.error("Failed to load cost by model:", error); + } catch { setData(null); } finally { setLoading(false); } }, [fetchData], );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ee/admin/src/components/cost-by-model-chart.tsx` around lines 83 - 97, The console.error in the loadData async callback should be removed or replaced with the project's structured logger; update the loadData function to avoid using console.error by either omitting the log (since setData(null) drives the "No data" UI) or calling the centralized logger (e.g., useLogger or processLogger) with a clear message and the caught error; ensure you still setData(null) and setLoading(false) in the catch/finally blocks and keep references to fetchData, setData, and setLoading unchanged.
150-165: Addtype="button"to prevent accidental form submission.The tab buttons lack explicit
type="button". While unlikely to cause issues in this context, it's a good practice to prevent accidental form submission if this component is ever used inside a form.♻️ Proposed fix
{viewTabs.map((tab) => ( <button key={tab.key} + type="button" className={cn( "rounded-md px-3 py-1 text-xs font-medium transition-colors", activeView === tab.key ? "bg-primary text-primary-foreground" : "text-muted-foreground hover:text-foreground", )} onClick={() => setActiveView(tab.key)} > {tab.label} </button> ))}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ee/admin/src/components/cost-by-model-chart.tsx` around lines 150 - 165, The tab buttons in the JSX mapping (rendering viewTabs) are missing an explicit type and can accidentally submit a surrounding form; update the button elements created in the viewTabs.map (the buttons that call setActiveView and use activeView and cn) to include type="button" so they do not act as submit buttons if this component is used inside a form.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ee/admin/src/app/models/page.tsx`:
- Around line 43-47: The code currently casts untrusted URL params into
ModelSortBy/SortOrder directly (see params, sortBy, sortOrder) — validate the
incoming params against the allowed values before casting and fallback to
defaults; implement a small whitelist check (or helper like isValidModelSortBy
and isValidSortOrder) that verifies params?.sortBy is one of ModelSortBy values
and params?.sortOrder is one of SortOrder values, then assign sortBy =
validValue ?? "logsCount" and sortOrder = validValue ?? "desc" to avoid unsafe
casts.
In
`@ee/admin/src/app/organizations/`[orgId]/projects/[projectId]/project-logs.tsx:
- Around line 47-48: The catch block in project-logs.tsx currently calls
console.error("Failed to load project logs:", error); instead add proper error
handling by storing the error in component state (e.g., useState like
[loadError, setLoadError]) and call setLoadError(error) inside the catch;
optionally invoke a user-visible toast (e.g., toast.error) with a friendly
message. Then update the component render to show a user-facing error state or
component when loadError is set (or rethrow to an ErrorBoundary) so users see
the failure instead of only logging to the console.
In `@ee/admin/src/app/providers/page.tsx`:
- Around line 39-41: params?.sortBy and params?.sortOrder are being cast
directly to ProviderSortBy and SortOrder which bypasses validation; update the
logic around params, sortBy, and sortOrder in providers/page.tsx to explicitly
validate the string values against the allowed ProviderSortBy and SortOrder
enums (or arrays of allowed values) before casting — if the value is not in the
allowed set, fall back to the safe defaults ("logsCount" for sortBy and "desc"
for sortOrder) and only pass validated values into getProviders; implement a
small validator/helper that checks membership and use it where sortBy and
sortOrder are derived.
---
Nitpick comments:
In `@ee/admin/src/app/models/page.tsx`:
- Around line 11-31: The SignInPrompt component is duplicated; extract it into a
shared component (e.g., create a new component named SignInPrompt in the shared
components folder) and replace the inline SignInPrompt definitions with a single
import and usage; ensure the new SignInPrompt exports the same component
signature and retains use of Button and Link (preserve className and props such
as size or asChild if needed), update all pages that defined their own
SignInPrompt (e.g., models and providers pages) to import SignInPrompt and
remove the local duplicates so there’s one centralized component to maintain.
- Line 11: The file declares a local type alias `SortOrder = "asc" | "desc"` but
the review asks to import the shared definition instead; remove the local `type
SortOrder` declaration and import `SortOrder` from the shared module where the
canonical type lives, then update any uses in this file (e.g., props,
interfaces, or function signatures in page.tsx) to reference the imported
`SortOrder` symbol so the file uses the centralized type.
In `@ee/admin/src/app/providers/page.tsx`:
- Line 9: The local type alias SortOrder in page.tsx duplicates a type already
exported from admin-providers.ts; remove the local declaration and import
SortOrder from admin-providers.ts instead to keep a single source of truth
(update the import list at the top of ee/admin/src/app/providers/page.tsx to
include SortOrder and delete the local `type SortOrder = "asc" | "desc";`
declaration).
In `@ee/admin/src/components/cost-by-model-chart.tsx`:
- Around line 83-97: The console.error in the loadData async callback should be
removed or replaced with the project's structured logger; update the loadData
function to avoid using console.error by either omitting the log (since
setData(null) drives the "No data" UI) or calling the centralized logger (e.g.,
useLogger or processLogger) with a clear message and the caught error; ensure
you still setData(null) and setLoading(false) in the catch/finally blocks and
keep references to fetchData, setData, and setLoading unchanged.
- Around line 150-165: The tab buttons in the JSX mapping (rendering viewTabs)
are missing an explicit type and can accidentally submit a surrounding form;
update the button elements created in the viewTabs.map (the buttons that call
setActiveView and use activeView and cn) to include type="button" so they do not
act as submit buttons if this component is used inside a form.
In `@ee/admin/src/components/history-chart.tsx`:
- Around line 82-89: The conditional in formatTimestamp is dead: both branches
return format(date, "HH:mm"); remove the minuteWindows check and return a single
format, or if longer windows should show more context, change the non-minute
branch to a different format (e.g., include date) instead of "HH:mm". Update the
function formatTimestamp (and the minuteWindows Set/HistoryWindow usage)
accordingly so there is no redundant branch.
In `@ee/admin/src/components/log-card.tsx`:
- Around line 31-33: The copyToClipboard function currently writes to the
clipboard silently; update it to surface success/failure feedback by awaiting
navigator.clipboard.writeText(text) and then triggering a UI notification (e.g.,
a toast) or toggling a copied state used by LogCard to show a transient icon
change; specifically modify copyToClipboard(text: string) to return a Promise or
accept a callback, handle errors and call the app's existing toast/snackbar
utility or set a local copied boolean in the surrounding component (LogCard) so
the user sees a success or error message and the copied state clears after a
short timeout.
In `@ee/admin/src/components/providers-table.tsx`:
- Around line 23-35: formatNumber and formatDate are duplicated utilities;
extract them into a shared module (e.g., a new "@/lib/format" file) and replace
the local functions in providers-table.tsx with imports. Move the
Intl.NumberFormat and Date formatting logic into exported functions
(formatNumber, formatDate) in the new module, update providers-table.tsx to
import and use those exports, and update any other components that duplicate the
same logic to import from the shared module instead.
In `@ee/admin/src/lib/admin-history.ts`:
- Around line 26-45: The getProviderHistory function constructs the request URL
without encoding providerId; update getProviderHistory to use
encodeURIComponent(providerId) when building the path passed to fetchServerData
(same pattern as getModelHistory and getMappingHistory) so special characters
are percent-encoded and the GET to `/admin/providers/${...}/history` is safe and
consistent.
- Around line 14-20: The hasSession() implementation is duplicated; extract it
into a shared utility (e.g., create and export async function hasSession() from
a new auth-utils module or add it to server-api) and replace the duplicate
implementations in admin-history.ts and admin-organizations.ts with an import of
that shared hasSession; keep the same async signature and behavior (using
cookies(), key "better-auth.session_token" and
"__Secure-better-auth.session_token") and update both files to call the imported
hasSession to eliminate duplication.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: ba8d58bc-daeb-4679-98d6-737efd6abafa
⛔ Files ignored due to path filters (9)
ee/admin/public/favicon/android-chrome-192x192.pngis excluded by!**/*.pngee/admin/public/favicon/android-chrome-512x512.pngis excluded by!**/*.pngee/admin/public/favicon/apple-touch-icon.pngis excluded by!**/*.pngee/admin/public/favicon/favicon-16x16.pngis excluded by!**/*.pngee/admin/public/favicon/favicon-32x32.pngis excluded by!**/*.pngee/admin/public/favicon/favicon.icois excluded by!**/*.icoee/admin/public/opengraph.pngis excluded by!**/*.pngee/admin/src/lib/api/v1.d.tsis excluded by!**/v1.d.tspnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (98)
AGENTS.mdee/admin/.gitignoreee/admin/.lintstagedrc.jsonee/admin/.prettierignoreee/admin/README.mdee/admin/components.jsonee/admin/eslint.config.mjsee/admin/next.config.tsee/admin/package.jsonee/admin/postcss.config.mjsee/admin/proxy.tsee/admin/public/favicon/site.webmanifestee/admin/src/app/api/health/route.tsee/admin/src/app/discounts/page.tsxee/admin/src/app/globals.cssee/admin/src/app/layout.tsxee/admin/src/app/login/page.tsxee/admin/src/app/models/page.tsxee/admin/src/app/organizations/[orgId]/discounts/page.tsxee/admin/src/app/organizations/[orgId]/gift-credits-dialog.tsxee/admin/src/app/organizations/[orgId]/org-cost-by-model.tsxee/admin/src/app/organizations/[orgId]/org-metrics.tsxee/admin/src/app/organizations/[orgId]/page.tsxee/admin/src/app/organizations/[orgId]/projects/[projectId]/page.tsxee/admin/src/app/organizations/[orgId]/projects/[projectId]/project-logs.tsxee/admin/src/app/organizations/[orgId]/projects/[projectId]/project-metrics.tsxee/admin/src/app/organizations/page.tsxee/admin/src/app/page.tsxee/admin/src/app/providers/page.tsxee/admin/src/components/admin-shell.tsxee/admin/src/components/auth/user-provider.tsxee/admin/src/components/cost-by-model-chart.tsxee/admin/src/components/dashboard-cost-by-model.tsxee/admin/src/components/delete-user-button.tsxee/admin/src/components/discount-form.tsxee/admin/src/components/history-chart.tsxee/admin/src/components/landing/theme-toggle.tsxee/admin/src/components/log-card.tsxee/admin/src/components/models-table.tsxee/admin/src/components/providers-table.tsxee/admin/src/components/revenue-chart.tsxee/admin/src/components/server-data-wrapper.tsxee/admin/src/components/signups-chart.tsxee/admin/src/components/time-range-picker.tsxee/admin/src/components/token-time-range-toggle.tsxee/admin/src/components/ui/alert.tsxee/admin/src/components/ui/avatar.tsxee/admin/src/components/ui/badge.tsxee/admin/src/components/ui/button.tsxee/admin/src/components/ui/card.tsxee/admin/src/components/ui/carousel.tsxee/admin/src/components/ui/chart.tsxee/admin/src/components/ui/checkbox.tsxee/admin/src/components/ui/collapsible.tsxee/admin/src/components/ui/command.tsxee/admin/src/components/ui/dialog.tsxee/admin/src/components/ui/dropdown-menu.tsxee/admin/src/components/ui/form.tsxee/admin/src/components/ui/hover-card.tsxee/admin/src/components/ui/input-group.tsxee/admin/src/components/ui/input.tsxee/admin/src/components/ui/label.tsxee/admin/src/components/ui/logo.tsxee/admin/src/components/ui/popover.tsxee/admin/src/components/ui/progress.tsxee/admin/src/components/ui/scroll-area.tsxee/admin/src/components/ui/select.tsxee/admin/src/components/ui/separator.tsxee/admin/src/components/ui/sheet.tsxee/admin/src/components/ui/sidebar.tsxee/admin/src/components/ui/skeleton.tsxee/admin/src/components/ui/sonner.tsxee/admin/src/components/ui/table.tsxee/admin/src/components/ui/tabs.tsxee/admin/src/components/ui/textarea.tsxee/admin/src/components/ui/tooltip.tsxee/admin/src/hooks/use-mobile.tsee/admin/src/hooks/useUser.tsee/admin/src/lib/admin-discounts.tsee/admin/src/lib/admin-history.tsee/admin/src/lib/admin-metrics.tsee/admin/src/lib/admin-models.tsee/admin/src/lib/admin-organizations.tsee/admin/src/lib/admin-providers.tsee/admin/src/lib/auth-client.tsee/admin/src/lib/config-server.tsee/admin/src/lib/config.tsxee/admin/src/lib/fetch-client.tsee/admin/src/lib/getUser.tsee/admin/src/lib/providers.tsxee/admin/src/lib/server-api.tsee/admin/src/lib/stripe.tsee/admin/src/lib/types.tsee/admin/src/lib/utils.tsee/admin/src/types/next-themes.d.tsee/admin/tsconfig.jsoninfra/split.dockerfileinfra/supervisord.conf
Comment on lines
+43
to
+47
| const params = await searchParams; | ||
| const page = Math.max(1, parseInt(params?.page ?? "1", 10)); | ||
| const search = params?.search ?? ""; | ||
| const sortBy = (params?.sortBy as ModelSortBy) ?? "logsCount"; | ||
| const sortOrder = (params?.sortOrder as SortOrder) || "desc"; |
Contributor
There was a problem hiding this comment.
Validate URL parameters before casting to ModelSortBy.
Same issue as the providers page—casting untrusted sortBy and sortOrder from URL parameters without validation.
🛡️ Proposed fix
+const validSortByValues = new Set<string>([
+ "name",
+ "logsCount",
+ "errorsCount",
+ "cachedCount",
+ "avgTimeToFirstToken",
+ "providerCount",
+]);
+const validSortOrderValues = new Set<string>(["asc", "desc"]);
+
export default async function ModelsPage({
searchParams,
}: {
searchParams?: Promise<{
page?: string;
search?: string;
sortBy?: string;
sortOrder?: string;
}>;
}) {
const params = await searchParams;
const page = Math.max(1, parseInt(params?.page ?? "1", 10));
const search = params?.search ?? "";
- const sortBy = (params?.sortBy as ModelSortBy) ?? "logsCount";
- const sortOrder = (params?.sortOrder as SortOrder) || "desc";
+ const sortBy: ModelSortBy = validSortByValues.has(params?.sortBy ?? "")
+ ? (params!.sortBy as ModelSortBy)
+ : "logsCount";
+ const sortOrder: SortOrder = validSortOrderValues.has(params?.sortOrder ?? "")
+ ? (params!.sortOrder as SortOrder)
+ : "desc";📝 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 params = await searchParams; | |
| const page = Math.max(1, parseInt(params?.page ?? "1", 10)); | |
| const search = params?.search ?? ""; | |
| const sortBy = (params?.sortBy as ModelSortBy) ?? "logsCount"; | |
| const sortOrder = (params?.sortOrder as SortOrder) || "desc"; | |
| const validSortByValues = new Set<string>([ | |
| "name", | |
| "logsCount", | |
| "errorsCount", | |
| "cachedCount", | |
| "avgTimeToFirstToken", | |
| "providerCount", | |
| ]); | |
| const validSortOrderValues = new Set<string>(["asc", "desc"]); | |
| export default async function ModelsPage({ | |
| searchParams, | |
| }: { | |
| searchParams?: Promise<{ | |
| page?: string; | |
| search?: string; | |
| sortBy?: string; | |
| sortOrder?: string; | |
| }>; | |
| }) { | |
| const params = await searchParams; | |
| const page = Math.max(1, parseInt(params?.page ?? "1", 10)); | |
| const search = params?.search ?? ""; | |
| const sortBy: ModelSortBy = validSortByValues.has(params?.sortBy ?? "") | |
| ? (params!.sortBy as ModelSortBy) | |
| : "logsCount"; | |
| const sortOrder: SortOrder = validSortOrderValues.has(params?.sortOrder ?? "") | |
| ? (params!.sortOrder as SortOrder) | |
| : "desc"; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ee/admin/src/app/models/page.tsx` around lines 43 - 47, The code currently
casts untrusted URL params into ModelSortBy/SortOrder directly (see params,
sortBy, sortOrder) — validate the incoming params against the allowed values
before casting and fallback to defaults; implement a small whitelist check (or
helper like isValidModelSortBy and isValidSortOrder) that verifies
params?.sortBy is one of ModelSortBy values and params?.sortOrder is one of
SortOrder values, then assign sortBy = validValue ?? "logsCount" and sortOrder =
validValue ?? "desc" to avoid unsafe casts.
Comment on lines
+47
to
+48
| } catch (error) { | ||
| console.error("Failed to load project logs:", error); |
Contributor
There was a problem hiding this comment.
Replace console.error with proper error handling.
Static analysis flags this console.error statement. Consider showing a user-facing error state or using a toast notification so users know the load failed.
💡 Suggested approach
+ const [error, setError] = useState<string | null>(null);
+
const loadLogs = useCallback(
async (cursor?: string) => {
+ setError(null);
if (cursor) {
setLoadingMore(true);
} else {
setLoading(true);
}
try {
const data = await loadProjectLogsAction(orgId, projectId, cursor);
// ...
} catch (error) {
- console.error("Failed to load project logs:", error);
+ setError("Failed to load project logs. Please try again.");
} finally {
// ...
}
},
[orgId, projectId],
);Then render an error message in the UI when error is set.
🧰 Tools
🪛 ESLint
[error] 48-48: Unexpected console statement.
(no-console)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ee/admin/src/app/organizations/`[orgId]/projects/[projectId]/project-logs.tsx
around lines 47 - 48, The catch block in project-logs.tsx currently calls
console.error("Failed to load project logs:", error); instead add proper error
handling by storing the error in component state (e.g., useState like
[loadError, setLoadError]) and call setLoadError(error) inside the catch;
optionally invoke a user-visible toast (e.g., toast.error) with a friendly
message. Then update the component render to show a user-facing error state or
component when loadError is set (or rethrow to an ErrorBoundary) so users see
the failure instead of only logging to the console.
Comment on lines
+39
to
+41
| const params = await searchParams; | ||
| const sortBy = (params?.sortBy as ProviderSortBy) ?? "logsCount"; | ||
| const sortOrder = (params?.sortOrder as SortOrder) || "desc"; |
Contributor
There was a problem hiding this comment.
Validate URL parameters before casting to strict union types.
Casting params?.sortBy directly to ProviderSortBy bypasses TypeScript's type safety. If a user manipulates the URL with an invalid value (e.g., ?sortBy=malicious), the invalid string is passed to getProviders and sent to the server. While the server may validate, the client loses compile-time guarantees.
🛡️ Proposed fix to validate before casting
+const validSortByValues = new Set<string>([
+ "name",
+ "logsCount",
+ "errorsCount",
+ "cachedCount",
+ "avgTimeToFirstToken",
+ "modelCount",
+]);
+const validSortOrderValues = new Set<string>(["asc", "desc"]);
+
export default async function ProvidersPage({
searchParams,
}: {
searchParams?: Promise<{
sortBy?: string;
sortOrder?: string;
}>;
}) {
const params = await searchParams;
- const sortBy = (params?.sortBy as ProviderSortBy) ?? "logsCount";
- const sortOrder = (params?.sortOrder as SortOrder) || "desc";
+ const sortBy: ProviderSortBy = validSortByValues.has(params?.sortBy ?? "")
+ ? (params!.sortBy as ProviderSortBy)
+ : "logsCount";
+ const sortOrder: SortOrder = validSortOrderValues.has(params?.sortOrder ?? "")
+ ? (params!.sortOrder as SortOrder)
+ : "desc";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ee/admin/src/app/providers/page.tsx` around lines 39 - 41, params?.sortBy and
params?.sortOrder are being cast directly to ProviderSortBy and SortOrder which
bypasses validation; update the logic around params, sortBy, and sortOrder in
providers/page.tsx to explicitly validate the string values against the allowed
ProviderSortBy and SortOrder enums (or arrays of allowed values) before casting
— if the value is not in the allowed set, fall back to the safe defaults
("logsCount" for sortBy and "desc" for sortOrder) and only pass validated values
into getProviders; implement a small validator/helper that checks membership and
use it where sortBy and sortOrder are derived.
Replace fetchServerData wrapper with direct createServerApiClient() calls. Derive all types from OpenAPI spec using GetJsonResponse helper. Remove admin-metrics.ts, admin-models.ts, admin-providers.ts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ee/admin/src/app/organizations/[orgId]/projects/[projectId]/page.tsx (1)
47-55:⚠️ Potential issue | 🟠 MajorHandle non-2xx responses before showing
SignInPrompt.
openapi-fetchreturns{ data, error, response }for all calls—non-2xx responses do not throw. The current code only destructuresdataand ignoreserror, so 401 auth failures, 404 missing organizations, and 5xx backend errors all trigger the sign-in prompt.Check the
errorproperty and handle 404/5xx cases appropriately:
- 404: Show
notFound()- 5xx: Re-throw or show error page
- 401: Then show sign-in prompt
Current code
const { data: projectsData } = await $api.GET( "/admin/organizations/{orgId}/projects", { params: { path: { orgId } } }, ); if (!projectsData) { return <SignInPrompt />; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ee/admin/src/app/organizations/`[orgId]/projects/[projectId]/page.tsx around lines 47 - 55, The current call using createServerApiClient and $api.GET only reads data (projectsData) and treats falsy data as an auth failure; instead destructure and inspect error and response from $api.GET (e.g., const { data: projectsData, error, response } = await $api.GET(...)), then handle by: if response?.status === 401 return <SignInPrompt />; if response?.status === 404 call notFound(); if response?.status && response.status >= 500 re-throw or surface an error page (or throw error) ; otherwise proceed when projectsData is present; ensure you reference createServerApiClient, $api.GET, projectsData, error, response, SignInPrompt and notFound() when making the changes.
♻️ Duplicate comments (3)
ee/admin/src/components/models-table.tsx (1)
129-143:⚠️ Potential issue | 🟠 MajorStatic headers remove sorting capability.
The parent page (
models/page.tsx) still maintainssortBy/sortOrderstate, but these headers are now static. Admins can no longer change the sort order from the UI.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ee/admin/src/components/models-table.tsx` around lines 129 - 143, The table headers in models-table.tsx (TableHeader/TableHead) are static which prevents changing the page-level sort state (sortBy/sortOrder in models/page.tsx); update ModelsTable to render sortable headers by wiring each TableHead to the existing sort change handler (prop or callback from models/page.tsx), toggling sortOrder for the clicked sort key (e.g., "Model", "Family", "Status", "Requests", "Errors", "Error Rate", "Avg TTFT", "Last Updated"), and render a visual/ARIA sort indicator for the active sort column; ensure the component uses the sortBy and sortOrder values passed from models/page.tsx and calls the onSortChange (or similarly named) function with the new sort key and order when a header is clicked.ee/admin/src/app/organizations/[orgId]/projects/[projectId]/project-logs.tsx (1)
45-46:⚠️ Potential issue | 🟡 MinorReplace
console.errorwith user-facing error state.The static analysis correctly flags this console statement. Users won't see any feedback when log loading fails. Consider adding error state and displaying it in the UI.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ee/admin/src/app/organizations/`[orgId]/projects/[projectId]/project-logs.tsx around lines 45 - 46, Replace the console.error in the catch block with a user-facing error state: add a React state like error/setError in the component that owns the fetch (in project-logs.tsx), set setError(error) inside the catch (error) branch, and clear it when retries/start loading; then render a visible error UI (banner/message) in the component's JSX to inform the user that logs failed to load and optionally include the error message and a retry button. Ensure you reference the same catch (error) block and the component (project-logs.tsx) when making the change so the UI surfaces failures instead of logging to console.ee/admin/src/app/models/page.tsx (1)
49-50:⚠️ Potential issue | 🟠 MajorValidate URL parameters before casting.
Untrusted
sortByandsortOrderfrom URL query parameters are cast directly toModelSortByandSortOrderwithout validation. This could allow invalid values to reach the API.🛡️ Suggested validation
+const validSortByValues = new Set([ + "name", + "family", + "logsCount", + "errorsCount", + "cachedCount", + "avgTimeToFirstToken", + "providerCount", +]); + const params = await searchParams; const page = Math.max(1, parseInt(params?.page ?? "1", 10)); const search = params?.search ?? ""; -const sortBy = (params?.sortBy as ModelSortBy) ?? "logsCount"; -const sortOrder = (params?.sortOrder as SortOrder) || "desc"; +const sortBy: ModelSortBy = validSortByValues.has(params?.sortBy ?? "") + ? (params!.sortBy as ModelSortBy) + : "logsCount"; +const sortOrder: SortOrder = + params?.sortOrder === "asc" || params?.sortOrder === "desc" + ? params.sortOrder + : "desc";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ee/admin/src/app/models/page.tsx` around lines 49 - 50, Untrusted query values params?.sortBy and params?.sortOrder are being cast directly to ModelSortBy and SortOrder; validate them against allowed enum/whitelist values before casting to prevent invalid values reaching the API. Update the code that sets sortBy and sortOrder to check params.sortBy against the set of valid ModelSortBy members (and params.sortOrder against valid SortOrder values like "asc"/"desc"), and only assign the cast if the value is valid, otherwise fall back to the defaults ("logsCount" and "desc").
🧹 Nitpick comments (4)
ee/admin/src/lib/types.ts (1)
19-83: Drop the section-header comments or split the barrel.The export names already provide the grouping, so the
// Metrics,// Organizations, etc. headers mostly add noise. If the grouping matters, consider re-exporting from feature-specific type modules instead.As per coding guidelines "No unnecessary code comments".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ee/admin/src/lib/types.ts` around lines 19 - 83, Remove the section-header comments (e.g., "// Metrics", "// Organizations", "// Projects", etc.) from this barrel types file and either delete them or move the related type groups into feature-specific modules and re-export them; for example extract AdminDashboardMetrics/TimeseriesRange, Organization/OrganizationsListResponse/TokenWindow, Project/ProjectsListResponse/ProjectMetrics/ProjectLogsResponse, ApiKey/ApiKeysListResponse, Member/MembersListResponse, Discount/DiscountsListResponse/DiscountOptions, ProviderStats/ProvidersListResponse, ModelStats/ModelsListResponse, HistoryResponse/HistoryDataPoint, CostByModelResponse, etc., into per-feature files and then export those modules from this file so the grouping is preserved without inline comments. Ensure exported type names and imports/exports remain unchanged so consumers (e.g., AdminDashboardMetrics, Organization, Project, ApiKey, Member, DiscountOptions, ProviderModelMapping, ProviderStats, ModelStats, HistoryDataPoint) keep working.ee/admin/src/components/models-table.tsx (1)
21-33: Consider extracting shared formatting helpers.
formatNumberandformatDateare duplicated verbatim inproviders-table.tsx. Consider extracting these to a shared utility module (e.g.,@/lib/format-utils.ts).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ee/admin/src/components/models-table.tsx` around lines 21 - 33, formatNumber and formatDate are duplicated in models-table.tsx and providers-table.tsx; extract them into a shared utility (e.g., create functions formatNumber and formatDate in a new module like "@/lib/format-utils.ts") and replace the local implementations by importing those helpers in both ModelsTable (models-table.tsx) and ProvidersTable (providers-table.tsx); ensure you export the functions from the new module and update the files to import { formatNumber, formatDate } from the shared module so both components use the same implementation.ee/admin/src/components/cost-by-model-chart.tsx (1)
24-36: Consider importingCostByModelDatafrom shared types.The
ee/admin/src/lib/types.tsfile definesCostByModelResponsederived from the OpenAPI schema. Importing that type instead of redefiningCostByModelDatalocally would ensure consistency with the API contract.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ee/admin/src/components/cost-by-model-chart.tsx` around lines 24 - 36, Replace the locally defined CostByModelData/CostByModelEntry types with the canonical type from shared types: import CostByModelResponse (or the exact exported name) from the shared types module and use it in place of CostByModelData; remove the local interfaces (CostByModelData and CostByModelEntry) and update any references in this component (e.g., props, state, or variables used by the cost-by-model-chart component) to the imported CostByModelResponse (use a type alias if names differ, e.g., type CostByModelData = CostByModelResponse) to ensure the component uses the OpenAPI-derived schema.ee/admin/src/app/page.tsx (1)
116-128: Consider handling API errors explicitly.The code destructures
datafrom API responses but doesn't check forerror. If the API returns an error,datawill be undefined and the page will showSignInPrompt, which may be misleading when the actual issue is an API error rather than authentication.This is a minor concern since the current fallback behavior is reasonable, but for better UX you might want to distinguish between auth failures and API errors.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ee/admin/src/app/page.tsx` around lines 116 - 128, Check the responses from createServerApiClient and the two $api.GET calls (metricsRes and timeseriesRes) for error fields before using metrics/timeseries; instead of only testing if metrics is falsy and returning SignInPrompt, detect an authentication error (e.g., metricsRes.error || timeseriesRes.error with an auth flag/status) and return <SignInPrompt /> only for auth failures, otherwise surface or throw a clearer API error (render an error message or throw a new Error including metricsRes.error/timeseriesRes.error) so callers of metrics and timeseries see the actual API failure rather than a misleading SignInPrompt.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ee/admin/src/components/cost-by-model-chart.tsx`:
- Around line 89-91: The catch block in the cost-by-model-chart component
currently logs errors to the console and calls setData(null); replace that with
a user-facing error state: add a state variable (e.g., error / setError) in the
component, update the catch in the load/fetch function to call
setError(error?.message || String(error)) and setData(null) instead of
console.error, and ensure the render path for the component (where you currently
handle "No data") also renders a clear error UI when error is present; also
reset/clear error (setError(null)) at the start of the load/fetch function (or
on retries) so transient retries don't show stale messages.
---
Outside diff comments:
In `@ee/admin/src/app/organizations/`[orgId]/projects/[projectId]/page.tsx:
- Around line 47-55: The current call using createServerApiClient and $api.GET
only reads data (projectsData) and treats falsy data as an auth failure; instead
destructure and inspect error and response from $api.GET (e.g., const { data:
projectsData, error, response } = await $api.GET(...)), then handle by: if
response?.status === 401 return <SignInPrompt />; if response?.status === 404
call notFound(); if response?.status && response.status >= 500 re-throw or
surface an error page (or throw error) ; otherwise proceed when projectsData is
present; ensure you reference createServerApiClient, $api.GET, projectsData,
error, response, SignInPrompt and notFound() when making the changes.
---
Duplicate comments:
In `@ee/admin/src/app/models/page.tsx`:
- Around line 49-50: Untrusted query values params?.sortBy and params?.sortOrder
are being cast directly to ModelSortBy and SortOrder; validate them against
allowed enum/whitelist values before casting to prevent invalid values reaching
the API. Update the code that sets sortBy and sortOrder to check params.sortBy
against the set of valid ModelSortBy members (and params.sortOrder against valid
SortOrder values like "asc"/"desc"), and only assign the cast if the value is
valid, otherwise fall back to the defaults ("logsCount" and "desc").
In
`@ee/admin/src/app/organizations/`[orgId]/projects/[projectId]/project-logs.tsx:
- Around line 45-46: Replace the console.error in the catch block with a
user-facing error state: add a React state like error/setError in the component
that owns the fetch (in project-logs.tsx), set setError(error) inside the catch
(error) branch, and clear it when retries/start loading; then render a visible
error UI (banner/message) in the component's JSX to inform the user that logs
failed to load and optionally include the error message and a retry button.
Ensure you reference the same catch (error) block and the component
(project-logs.tsx) when making the change so the UI surfaces failures instead of
logging to console.
In `@ee/admin/src/components/models-table.tsx`:
- Around line 129-143: The table headers in models-table.tsx
(TableHeader/TableHead) are static which prevents changing the page-level sort
state (sortBy/sortOrder in models/page.tsx); update ModelsTable to render
sortable headers by wiring each TableHead to the existing sort change handler
(prop or callback from models/page.tsx), toggling sortOrder for the clicked sort
key (e.g., "Model", "Family", "Status", "Requests", "Errors", "Error Rate", "Avg
TTFT", "Last Updated"), and render a visual/ARIA sort indicator for the active
sort column; ensure the component uses the sortBy and sortOrder values passed
from models/page.tsx and calls the onSortChange (or similarly named) function
with the new sort key and order when a header is clicked.
---
Nitpick comments:
In `@ee/admin/src/app/page.tsx`:
- Around line 116-128: Check the responses from createServerApiClient and the
two $api.GET calls (metricsRes and timeseriesRes) for error fields before using
metrics/timeseries; instead of only testing if metrics is falsy and returning
SignInPrompt, detect an authentication error (e.g., metricsRes.error ||
timeseriesRes.error with an auth flag/status) and return <SignInPrompt /> only
for auth failures, otherwise surface or throw a clearer API error (render an
error message or throw a new Error including
metricsRes.error/timeseriesRes.error) so callers of metrics and timeseries see
the actual API failure rather than a misleading SignInPrompt.
In `@ee/admin/src/components/cost-by-model-chart.tsx`:
- Around line 24-36: Replace the locally defined
CostByModelData/CostByModelEntry types with the canonical type from shared
types: import CostByModelResponse (or the exact exported name) from the shared
types module and use it in place of CostByModelData; remove the local interfaces
(CostByModelData and CostByModelEntry) and update any references in this
component (e.g., props, state, or variables used by the cost-by-model-chart
component) to the imported CostByModelResponse (use a type alias if names
differ, e.g., type CostByModelData = CostByModelResponse) to ensure the
component uses the OpenAPI-derived schema.
In `@ee/admin/src/components/models-table.tsx`:
- Around line 21-33: formatNumber and formatDate are duplicated in
models-table.tsx and providers-table.tsx; extract them into a shared utility
(e.g., create functions formatNumber and formatDate in a new module like
"@/lib/format-utils.ts") and replace the local implementations by importing
those helpers in both ModelsTable (models-table.tsx) and ProvidersTable
(providers-table.tsx); ensure you export the functions from the new module and
update the files to import { formatNumber, formatDate } from the shared module
so both components use the same implementation.
In `@ee/admin/src/lib/types.ts`:
- Around line 19-83: Remove the section-header comments (e.g., "// Metrics", "//
Organizations", "// Projects", etc.) from this barrel types file and either
delete them or move the related type groups into feature-specific modules and
re-export them; for example extract AdminDashboardMetrics/TimeseriesRange,
Organization/OrganizationsListResponse/TokenWindow,
Project/ProjectsListResponse/ProjectMetrics/ProjectLogsResponse,
ApiKey/ApiKeysListResponse, Member/MembersListResponse,
Discount/DiscountsListResponse/DiscountOptions,
ProviderStats/ProvidersListResponse, ModelStats/ModelsListResponse,
HistoryResponse/HistoryDataPoint, CostByModelResponse, etc., into per-feature
files and then export those modules from this file so the grouping is preserved
without inline comments. Ensure exported type names and imports/exports remain
unchanged so consumers (e.g., AdminDashboardMetrics, Organization, Project,
ApiKey, Member, DiscountOptions, ProviderModelMapping, ProviderStats,
ModelStats, HistoryDataPoint) keep working.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d0e869af-7eb4-470e-b6fc-6cd3ac54e7d7
📒 Files selected for processing (26)
ee/admin/src/app/models/page.tsxee/admin/src/app/organizations/[orgId]/discounts/page.tsxee/admin/src/app/organizations/[orgId]/org-cost-by-model.tsxee/admin/src/app/organizations/[orgId]/org-metrics.tsxee/admin/src/app/organizations/[orgId]/page.tsxee/admin/src/app/organizations/[orgId]/projects/[projectId]/page.tsxee/admin/src/app/organizations/[orgId]/projects/[projectId]/project-logs.tsxee/admin/src/app/organizations/[orgId]/projects/[projectId]/project-metrics.tsxee/admin/src/app/organizations/page.tsxee/admin/src/app/page.tsxee/admin/src/app/providers/page.tsxee/admin/src/components/cost-by-model-chart.tsxee/admin/src/components/dashboard-cost-by-model.tsxee/admin/src/components/discount-form.tsxee/admin/src/components/log-card.tsxee/admin/src/components/models-table.tsxee/admin/src/components/providers-table.tsxee/admin/src/components/revenue-chart.tsxee/admin/src/components/signups-chart.tsxee/admin/src/components/time-range-picker.tsxee/admin/src/components/token-time-range-toggle.tsxee/admin/src/lib/admin-discounts.tsee/admin/src/lib/admin-history.tsee/admin/src/lib/admin-organizations.tsee/admin/src/lib/server-api.tsee/admin/src/lib/types.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- ee/admin/src/app/providers/page.tsx
- ee/admin/src/app/organizations/[orgId]/org-cost-by-model.tsx
- ee/admin/src/components/log-card.tsx
Comment on lines
+89
to
+91
| } catch (error) { | ||
| console.error("Failed to load cost by model:", error); | ||
| setData(null); |
Contributor
There was a problem hiding this comment.
Replace console.error with user-facing feedback.
ESLint flags this console statement. While setData(null) triggers the "No data" UI state, users won't know if it's genuinely empty or if an error occurred. Consider distinguishing error states.
💡 Suggested approach
+const [error, setError] = useState<string | null>(null);
const loadData = useCallback(
async (w: TokenWindow) => {
setLoading(true);
+ setError(null);
try {
const result = await fetchData(w);
setData(result);
} catch (error) {
- console.error("Failed to load cost by model:", error);
+ setError("Failed to load cost data. Please try again.");
setData(null);
} finally {
setLoading(false);
}
},
[fetchData],
);Then render the error message when error is set.
🧰 Tools
🪛 ESLint
[error] 90-90: Unexpected console statement.
(no-console)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ee/admin/src/components/cost-by-model-chart.tsx` around lines 89 - 91, The
catch block in the cost-by-model-chart component currently logs errors to the
console and calls setData(null); replace that with a user-facing error state:
add a state variable (e.g., error / setError) in the component, update the catch
in the load/fetch function to call setError(error?.message || String(error)) and
setData(null) instead of console.error, and ensure the render path for the
component (where you currently handle "No data") also renders a clear error UI
when error is present; also reset/clear error (setError(null)) at the start of
the load/fetch function (or on retries) so transient retries don't show stale
messages.
- Fix history chart showing string concatenation instead of numbers by adding Number() coercion in mapHistoryRows for PostgreSQL bigint - Add sortable column headers to models and providers tables using Link-based SortableHeader pattern matching organizations page - Add status, free, updatedAt sort options to models API - Add status, updatedAt sort options to providers API - Make tables responsive with horizontal scroll via min-w-0 - Expand seed to generate history for all providers and 50 models Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
smakosh
enabled auto-merge
March 7, 2026 14:39
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api/src/routes/admin.ts (1)
1645-1666:⚠️ Potential issue | 🟠 MajorReject cursors that do not belong to this project.
The cursor lookup only filters by
log.id. If a cursor from another project is passed in, pagination will be anchored to the wrong timestamp and can skip or duplicate rows for this project.🐛 Proposed fix
if (cursor) { const cursorLog = await db .select({ createdAt: tables.log.createdAt }) .from(tables.log) - .where(eq(tables.log.id, cursor)) + .where( + and( + eq(tables.log.id, cursor), + eq(tables.log.projectId, projectId), + ), + ) .limit(1);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/routes/admin.ts` around lines 1645 - 1666, The cursor lookup currently queries by log.id only (see db.select -> tables.log and cursorLog) which allows cursors from other projects; update the lookup to also filter by the current project (tables.log.projectId) so the fetched cursor row belongs to this project, then use that cursorCreatedAt to build whereConditions; ensure the same project constraint (tables.log.projectId === projectId) is applied when validating the cursor and when composing the pagination where clause alongside tables.log.createdAt and tables.log.id.
🧹 Nitpick comments (2)
packages/db/src/seed.ts (1)
1101-1101: Reuse the existing collection type instead of redeclaringRecord<string, any>.The new
topMappingsdeclaration unnecessarily reiterates theRecord<string, any>type. Useconst topMappings: typeof mappings = [];to keep both variables in sync and avoid duplicate type declarations, which also aligns with the guideline to minimizeanyusage in TypeScript.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/db/src/seed.ts` at line 1101, Replace the explicit Record<string, any> annotation on topMappings with the existing mappings' type so they stay in sync; change the declaration of topMappings to use typeof mappings (e.g., const topMappings: typeof mappings = []) rather than redeclaring Record<string, any>, ensuring both variables share the same inferred shape and reducing use of any.ee/admin/src/components/providers-table.tsx (1)
26-36: Share the sort unions instead of redefining them locally.
ee/admin/src/app/providers/page.tsxalready depends on the sameProviderSortBy/SortOrdershape when reading search params. Keeping a second copy here makes the page and table easy to drift the next time a sortable column is added. Export these unions from a shared admin types module and import them in both places.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ee/admin/src/components/providers-table.tsx` around lines 26 - 36, The local duplicate type unions ProviderSortBy and SortOrder should be removed and replaced with imports from a single shared admin types module: create or use a central exported type (exporting ProviderSortBy and SortOrder) and update the component to import those types instead of redefining them; specifically, delete the local definitions for ProviderSortBy and SortOrder in providers-table.tsx and update the import statements in this file and in the page that reads search params (which also uses ProviderSortBy/SortOrder) to reference the shared exported types so both files use the same canonical definitions.
🤖 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/api/src/routes/admin.ts`:
- Around line 3046-3081: The totals are being computed from the already-limited
top-20 result set (rows), so totalCost and totalRequests undercount when >20
models exist; fix by running a separate aggregate query against
projectHourlyModelStats (with the same where
gte(projectHourlyModelStats.hourTimestamp, startDate)) that computes SUM(cost)
and SUM(requestCount) for the full window and use those results for
totalCost/totalRequests, then keep the existing select/groupBy/orderBy/limit
query to populate rows/models. Update references to totalCost and totalRequests
to use the aggregate query results instead of reducing the limited rows.
- Around line 1717-1723: The current mapping uses truthy checks which turn valid
0 token counts into null; update each token field expression in the
paginatedLogs.map callback (promptTokens, completionTokens, totalTokens,
reasoningTokens, cachedTokens) to check for null/undefined instead of
falsiness—e.g. replace "l.promptTokens ? String(l.promptTokens) : null" with
"l.promptTokens != null ? String(l.promptTokens) : null" (do the same for
completionTokens, totalTokens, reasoningTokens, cachedTokens) so zero values are
preserved as "0".
- Around line 1571-1582: Replace the loose z.any() uses for errorDetails and
routingMetadata with the concrete Zod schemas: import errorDetails from
`@llmgateway/db` and import the routingMetadata schema from logs.ts (the same
symbol used there), then change the fields in the response schema from
errorDetails: z.any().nullable() and routingMetadata: z.any().nullable() to use
the imported errorDetails and routingMetadata schemas (preserving .nullable() if
needed); also add the corresponding imports at the top of the file so the types
are reused rather than any-typed.
In `@ee/admin/src/components/providers-table.tsx`:
- Line 18: The import of getProviderHistory from
ee/admin/src/lib/admin-history.ts breaks Next.js server/client boundaries
because admin-history.ts uses "use server" and createServerApiClient (which
calls next/headers.cookies()), so update ProvidersTable to avoid importing that
server-only module: either convert the ProvidersTable component into a server
component (move/rename it out of "use client" and remove any client-only
directives so it can safely import getProviderHistory), or keep ProvidersTable
as a client component and implement a Server Action (e.g., a new server-exported
async function in admin-history.ts or a separate server-actions file) that wraps
the history fetch and call that action from the client component; reference
ProvidersTable, getProviderHistory, admin-history.ts, and createServerApiClient
when making the change.
- Around line 80-88: The formatDate function uses toLocaleDateString without an
explicit timeZone which can cause SSR/hydration mismatches; update formatDate
(and any other direct toLocaleDateString calls) to pass a fixed timeZone (e.g.,
timeZone: "UTC") in the options so the server and client render the same value,
e.g., add timeZone: "UTC" to the options object in the formatDate function and
mirror the same change for other toLocaleDateString usages in this file.
In `@packages/db/src/seed.ts`:
- Around line 1099-1110: The current per-provider selection loop (using
seenProviders and topMappings over mappings) ensures one mapping per provider
but can yield fewer than 50 entries; to fix, after that loop add a second pass
over mappings and append additional mappings to topMappings until
topMappings.length === 50 (skipping ones already in seenProviders or already
present) so you preserve provider coverage first and then fill remaining slots;
update the logic that builds topMappings to perform the initial per-provider
pass then a follow-up pass that fills to 50 while honoring uniqueness and the
existing seenProviders set.
---
Outside diff comments:
In `@apps/api/src/routes/admin.ts`:
- Around line 1645-1666: The cursor lookup currently queries by log.id only (see
db.select -> tables.log and cursorLog) which allows cursors from other projects;
update the lookup to also filter by the current project (tables.log.projectId)
so the fetched cursor row belongs to this project, then use that cursorCreatedAt
to build whereConditions; ensure the same project constraint
(tables.log.projectId === projectId) is applied when validating the cursor and
when composing the pagination where clause alongside tables.log.createdAt and
tables.log.id.
---
Nitpick comments:
In `@ee/admin/src/components/providers-table.tsx`:
- Around line 26-36: The local duplicate type unions ProviderSortBy and
SortOrder should be removed and replaced with imports from a single shared admin
types module: create or use a central exported type (exporting ProviderSortBy
and SortOrder) and update the component to import those types instead of
redefining them; specifically, delete the local definitions for ProviderSortBy
and SortOrder in providers-table.tsx and update the import statements in this
file and in the page that reads search params (which also uses
ProviderSortBy/SortOrder) to reference the shared exported types so both files
use the same canonical definitions.
In `@packages/db/src/seed.ts`:
- Line 1101: Replace the explicit Record<string, any> annotation on topMappings
with the existing mappings' type so they stay in sync; change the declaration of
topMappings to use typeof mappings (e.g., const topMappings: typeof mappings =
[]) rather than redeclaring Record<string, any>, ensuring both variables share
the same inferred shape and reducing use of any.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2997dd90-84a6-47c3-8d35-6c6d753c7505
⛔ Files ignored due to path filters (1)
ee/admin/src/lib/api/v1.d.tsis excluded by!**/v1.d.ts
📒 Files selected for processing (7)
apps/api/src/routes/admin.tsee/admin/src/app/models/page.tsxee/admin/src/app/providers/page.tsxee/admin/src/components/models-table.tsxee/admin/src/components/providers-table.tsxee/admin/src/components/ui/sidebar.tsxpackages/db/src/seed.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- ee/admin/src/components/models-table.tsx
- ee/admin/src/app/providers/page.tsx
- ee/admin/src/app/models/page.tsx
Comment on lines
+1571
to
+1582
| errorDetails: z.any().nullable(), | ||
| finishReason: z.string().nullable(), | ||
| unifiedFinishReason: z.string().nullable(), | ||
| cached: z.boolean().nullable(), | ||
| cachedTokens: z.string().nullable(), | ||
| streamed: z.boolean().nullable(), | ||
| source: z.string().nullable(), | ||
| content: z.string().nullable(), | ||
| usedMode: z.string(), | ||
| discount: z.number().nullable(), | ||
| timeToFirstToken: z.number().nullable(), | ||
| responseSize: z.number().nullable(), | ||
| routingMetadata: z.any().nullable(), |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Schema and route usage:"
rg -n -C2 '\b(errorDetails|routingMetadata)\b' apps packages ee
echo
echo "Log table field definitions:"
rg -n -C3 '\b(errorDetails|routingMetadata)\b' packages/db/src/schema.tsRepository: theopenco/llmgateway
Length of output: 50376
🏁 Script executed:
#!/bin/bash
# Get the errorDetails and routingMetadata definitions from logs.ts
head -n 100 apps/api/src/routes/logs.ts | tail -n 50Repository: theopenco/llmgateway
Length of output: 1753
🏁 Script executed:
#!/bin/bash
# Get the complete routingMetadata and errorDetails schema definitions
rg -n -A 30 'routingMetadata: z' apps/api/src/routes/logs.ts | head -n 60Repository: theopenco/llmgateway
Length of output: 890
🏁 Script executed:
#!/bin/bash
# Check the imports at the top of logs.ts to see where errorDetails comes from
head -n 30 apps/api/src/routes/logs.tsRepository: theopenco/llmgateway
Length of output: 544
🏁 Script executed:
#!/bin/bash
# Check what's exported from packages/db and verify the errorDetails definition
cat packages/db/src/types.ts | head -n 20Repository: theopenco/llmgateway
Length of output: 567
🏁 Script executed:
#!/bin/bash
# Check imports in admin.ts to see what's currently imported from db
head -n 50 apps/api/src/routes/admin.ts | grep -A 30 "from.*db"Repository: theopenco/llmgateway
Length of output: 722
🏁 Script executed:
#!/bin/bash
# Get the complete import from `@llmgateway/db` in admin.ts
head -n 50 apps/api/src/routes/admin.ts | grep -B 50 "from.*@llmgateway/db"Repository: theopenco/llmgateway
Length of output: 523
🏁 Script executed:
#!/bin/bash
# Check if routingMetadata is exported from db or defined elsewhere
rg -n "export.*routingMetadata" packages/db/src/Repository: theopenco/llmgateway
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Check what's exported from the db package
rg -n "^export" packages/db/src/index.ts | head -n 30Repository: theopenco/llmgateway
Length of output: 426
🏁 Script executed:
#!/bin/bash
# Verify what the routingMetadata schema looks like in logs.ts is complete
rg -n -A 50 'routingMetadata: z' apps/api/src/routes/logs.ts | head -n 70Repository: theopenco/llmgateway
Length of output: 1384
🏁 Script executed:
#!/bin/bash
# Get the context around lines 1571-1582 in admin.ts to see the full schema
sed -n '1560,1590p' apps/api/src/routes/admin.tsRepository: theopenco/llmgateway
Length of output: 1082
Replace z.any() with properly typed schemas for errorDetails and routingMetadata.
Both fields have stable, well-defined schemas defined in the codebase:
errorDetailsis exported from@llmgateway/dband used in logs.tsroutingMetadatais explicitly modeled in logs.ts with all properties
Import and reuse these schemas instead of using z.any():
♻️ Proposed fix
- errorDetails: z.any().nullable(),
+ errorDetails: errorDetails.nullable(),
finishReason: z.string().nullable(),
unifiedFinishReason: z.string().nullable(),
cached: z.boolean().nullable(),
@@ -1579,7 +1579,35 @@
timeToFirstToken: z.number().nullable(),
responseSize: z.number().nullable(),
- routingMetadata: z.any().nullable(),
+ routingMetadata: z
+ .object({
+ availableProviders: z.array(z.string()).optional(),
+ selectedProvider: z.string().optional(),
+ selectionReason: z.string().optional(),
+ providerScores: z
+ .array(
+ z.object({
+ providerId: z.string(),
+ score: z.number(),
+ uptime: z.number().optional(),
+ latency: z.number().optional(),
+ price: z.number().optional(),
+ }),
+ )
+ .optional(),
+ routing: z
+ .array(
+ z.object({
+ provider: z.string(),
+ model: z.string(),
+ status_code: z.number(),
+ error_type: z.string(),
+ succeeded: z.boolean(),
+ }),
+ )
+ .optional(),
+ noFallback: z.boolean().optional(),
+ })
+ .nullable()
+ .optional(),Add to imports: errorDetails from @llmgateway/db
Per coding guidelines: "**/*.{ts,tsx}: Never use any or as any in TypeScript unless absolutely necessary`".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/api/src/routes/admin.ts` around lines 1571 - 1582, Replace the loose
z.any() uses for errorDetails and routingMetadata with the concrete Zod schemas:
import errorDetails from `@llmgateway/db` and import the routingMetadata schema
from logs.ts (the same symbol used there), then change the fields in the
response schema from errorDetails: z.any().nullable() and routingMetadata:
z.any().nullable() to use the imported errorDetails and routingMetadata schemas
(preserving .nullable() if needed); also add the corresponding imports at the
top of the file so the types are reused rather than any-typed.
Comment on lines
1717
to
1723
| logs: paginatedLogs.map((l) => ({ | ||
| ...l, | ||
| promptTokens: l.promptTokens ? String(l.promptTokens) : null, | ||
| completionTokens: l.completionTokens ? String(l.completionTokens) : null, | ||
| totalTokens: l.totalTokens ? String(l.totalTokens) : null, | ||
| reasoningTokens: l.reasoningTokens ? String(l.reasoningTokens) : null, | ||
| cachedTokens: l.cachedTokens ? String(l.cachedTokens) : null, |
Contributor
There was a problem hiding this comment.
Preserve zero token counts when serializing logs.
These truthy checks convert valid 0 values into null, so the new log cards will drop token counts for empty/error/cached responses.
🐛 Proposed fix
- promptTokens: l.promptTokens ? String(l.promptTokens) : null,
- completionTokens: l.completionTokens ? String(l.completionTokens) : null,
- totalTokens: l.totalTokens ? String(l.totalTokens) : null,
- reasoningTokens: l.reasoningTokens ? String(l.reasoningTokens) : null,
- cachedTokens: l.cachedTokens ? String(l.cachedTokens) : null,
+ promptTokens: l.promptTokens != null ? String(l.promptTokens) : null,
+ completionTokens:
+ l.completionTokens != null ? String(l.completionTokens) : null,
+ totalTokens: l.totalTokens != null ? String(l.totalTokens) : null,
+ reasoningTokens:
+ l.reasoningTokens != null ? String(l.reasoningTokens) : null,
+ cachedTokens: l.cachedTokens != null ? String(l.cachedTokens) : null,📝 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
| logs: paginatedLogs.map((l) => ({ | |
| ...l, | |
| promptTokens: l.promptTokens ? String(l.promptTokens) : null, | |
| completionTokens: l.completionTokens ? String(l.completionTokens) : null, | |
| totalTokens: l.totalTokens ? String(l.totalTokens) : null, | |
| reasoningTokens: l.reasoningTokens ? String(l.reasoningTokens) : null, | |
| cachedTokens: l.cachedTokens ? String(l.cachedTokens) : null, | |
| logs: paginatedLogs.map((l) => ({ | |
| ...l, | |
| promptTokens: l.promptTokens != null ? String(l.promptTokens) : null, | |
| completionTokens: | |
| l.completionTokens != null ? String(l.completionTokens) : null, | |
| totalTokens: l.totalTokens != null ? String(l.totalTokens) : null, | |
| reasoningTokens: | |
| l.reasoningTokens != null ? String(l.reasoningTokens) : null, | |
| cachedTokens: l.cachedTokens != null ? String(l.cachedTokens) : null, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/api/src/routes/admin.ts` around lines 1717 - 1723, The current mapping
uses truthy checks which turn valid 0 token counts into null; update each token
field expression in the paginatedLogs.map callback (promptTokens,
completionTokens, totalTokens, reasoningTokens, cachedTokens) to check for
null/undefined instead of falsiness—e.g. replace "l.promptTokens ?
String(l.promptTokens) : null" with "l.promptTokens != null ?
String(l.promptTokens) : null" (do the same for completionTokens, totalTokens,
reasoningTokens, cachedTokens) so zero values are preserved as "0".
Comment on lines
+3046
to
+3081
| const rows = await db | ||
| .select({ | ||
| usedModel: projectHourlyModelStats.usedModel, | ||
| cost: sql<number>`SUM(${projectHourlyModelStats.cost})`.as("cost"), | ||
| requestCount: | ||
| sql<number>`SUM(${projectHourlyModelStats.requestCount})`.as( | ||
| "request_count", | ||
| ), | ||
| totalTokens: | ||
| sql<number>`SUM(CAST(${projectHourlyModelStats.totalTokens} AS NUMERIC))`.as( | ||
| "total_tokens", | ||
| ), | ||
| }) | ||
| .from(projectHourlyModelStats) | ||
| .where(gte(projectHourlyModelStats.hourTimestamp, startDate)) | ||
| .groupBy(projectHourlyModelStats.usedModel) | ||
| .orderBy(desc(sql`SUM(${projectHourlyModelStats.cost})`)) | ||
| .limit(20); | ||
|
|
||
| const totalCost = rows.reduce((sum, r) => sum + Number(r.cost), 0); | ||
| const totalRequests = rows.reduce( | ||
| (sum, r) => sum + Number(r.requestCount), | ||
| 0, | ||
| ); | ||
|
|
||
| return c.json({ | ||
| window, | ||
| models: rows.map((r) => ({ | ||
| model: r.usedModel, | ||
| cost: Number(r.cost), | ||
| requestCount: Number(r.requestCount), | ||
| totalTokens: Number(r.totalTokens), | ||
| })), | ||
| totalCost, | ||
| totalRequests, | ||
| }); |
Contributor
There was a problem hiding this comment.
Compute totals before applying the top-20 limit.
Both cost-by-model endpoints derive totalCost and totalRequests from the already-limited top 20 rows. Once an org or the global dataset has more than 20 active models, these totals will underreport the real window totals.
🐛 Proposed fix pattern
+ const [totals] = await db
+ .select({
+ totalCost: sql<number>`COALESCE(SUM(${projectHourlyModelStats.cost}), 0)`.as(
+ "total_cost",
+ ),
+ totalRequests:
+ sql<number>`COALESCE(SUM(${projectHourlyModelStats.requestCount}), 0)`.as(
+ "total_requests",
+ ),
+ })
+ .from(projectHourlyModelStats)
+ .where(/* same filtered predicate, but no GROUP BY / LIMIT */);
+
const rows = await db
.select({
usedModel: projectHourlyModelStats.usedModel,
cost: sql<number>`SUM(${projectHourlyModelStats.cost})`.as("cost"),
requestCount:
sql<number>`SUM(${projectHourlyModelStats.requestCount})`.as(
"request_count",
),
totalTokens:
sql<number>`SUM(CAST(${projectHourlyModelStats.totalTokens} AS NUMERIC))`.as(
"total_tokens",
),
})
.from(projectHourlyModelStats)
.where(/* same filtered predicate */)
.groupBy(projectHourlyModelStats.usedModel)
.orderBy(desc(sql`SUM(${projectHourlyModelStats.cost})`))
.limit(20);
-
- const totalCost = rows.reduce((sum, r) => sum + Number(r.cost), 0);
- const totalRequests = rows.reduce(
- (sum, r) => sum + Number(r.requestCount),
- 0,
- );
return c.json({
window,
models: rows.map((r) => ({
model: r.usedModel,
cost: Number(r.cost),
requestCount: Number(r.requestCount),
totalTokens: Number(r.totalTokens),
})),
- totalCost,
- totalRequests,
+ totalCost: Number(totals?.totalCost ?? 0),
+ totalRequests: Number(totals?.totalRequests ?? 0),
});Also applies to: 3139-3179
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/api/src/routes/admin.ts` around lines 3046 - 3081, The totals are being
computed from the already-limited top-20 result set (rows), so totalCost and
totalRequests undercount when >20 models exist; fix by running a separate
aggregate query against projectHourlyModelStats (with the same where
gte(projectHourlyModelStats.hourTimestamp, startDate)) that computes SUM(cost)
and SUM(requestCount) for the full window and use those results for
totalCost/totalRequests, then keep the existing select/groupBy/orderBy/limit
query to populate rows/models. Update references to totalCost and totalRequests
to use the aggregate query results instead of reducing the limited rows.
| TableHeader, | ||
| TableRow, | ||
| } from "@/components/ui/table"; | ||
| import { getProviderHistory } from "@/lib/admin-history"; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ee/admin/src/lib/admin-history.ts =="
sed -n '1,120p' ee/admin/src/lib/admin-history.ts
echo
echo "== ee/admin/src/lib/server-api.ts =="
sed -n '1,240p' ee/admin/src/lib/server-api.ts
echo
echo "== server-only indicators =="
rg -n -C2 'server-only|next/headers|cookies\(|headers\(|createServerApiClient' ee/admin/src/libRepository: theopenco/llmgateway
Length of output: 11234
Remove or restructure this import—it violates Next.js server/client boundaries.
ee/admin/src/lib/admin-history.ts is marked "use server" and calls createServerApiClient(), which directly imports and uses next/headers.cookies() (a server-only API). A "use client" component cannot import or call server-only functions. This will fail at runtime. Either move ProvidersTable to a server component, or wrap the history fetch in a Server Action that you call from the client.
Also applies to: 99-103
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ee/admin/src/components/providers-table.tsx` at line 18, The import of
getProviderHistory from ee/admin/src/lib/admin-history.ts breaks Next.js
server/client boundaries because admin-history.ts uses "use server" and
createServerApiClient (which calls next/headers.cookies()), so update
ProvidersTable to avoid importing that server-only module: either convert the
ProvidersTable component into a server component (move/rename it out of "use
client" and remove any client-only directives so it can safely import
getProviderHistory), or keep ProvidersTable as a client component and implement
a Server Action (e.g., a new server-exported async function in admin-history.ts
or a separate server-actions file) that wraps the history fetch and call that
action from the client component; reference ProvidersTable, getProviderHistory,
admin-history.ts, and createServerApiClient when making the change.
Comment on lines
+80
to
+88
| function formatDate(dateString: string) { | ||
| return new Date(dateString).toLocaleDateString("en-US", { | ||
| year: "numeric", | ||
| month: "short", | ||
| day: "numeric", | ||
| hour: "2-digit", | ||
| minute: "2-digit", | ||
| }); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n ee/admin/src/components/providers-table.tsx | head -100Repository: theopenco/llmgateway
Length of output: 3116
🏁 Script executed:
cat -n ee/admin/src/components/providers-table.tsx | sed -n '140,150p'Repository: theopenco/llmgateway
Length of output: 447
🏁 Script executed:
cat -n ee/admin/src/components/providers-table.tsx | sed -n '75,95p' | cat -ARepository: theopenco/llmgateway
Length of output: 808
🏁 Script executed:
cat -n ee/admin/src/components/providers-table.tsx | head -30Repository: theopenco/llmgateway
Length of output: 1035
Stabilize the updatedAt timezone to prevent hydration mismatches.
Since this is a Client Component that renders server-side during SSR, toLocaleDateString without an explicit timeZone will use the server's timezone during initial render and the browser's timezone during hydration. If they differ, this causes hydration mismatches and potential off-by-hours displays.
Suggested fix
+const updatedAtFormatter = new Intl.DateTimeFormat("en-US", {
+ timeZone: "UTC",
+ year: "numeric",
+ month: "short",
+ day: "numeric",
+ hour: "2-digit",
+ minute: "2-digit",
+});
+
function formatDate(dateString: string) {
- return new Date(dateString).toLocaleDateString("en-US", {
- year: "numeric",
- month: "short",
- day: "numeric",
- hour: "2-digit",
- minute: "2-digit",
- });
+ return updatedAtFormatter.format(new Date(dateString));
}Also applies to: 144-145
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ee/admin/src/components/providers-table.tsx` around lines 80 - 88, The
formatDate function uses toLocaleDateString without an explicit timeZone which
can cause SSR/hydration mismatches; update formatDate (and any other direct
toLocaleDateString calls) to pass a fixed timeZone (e.g., timeZone: "UTC") in
the options so the server and client render the same value, e.g., add timeZone:
"UTC" to the options object in the formatDate function and mirror the same
change for other toLocaleDateString usages in this file.
Comment on lines
+1099
to
+1110
| // Pick one mapping per provider to ensure all providers have history data | ||
| const seenProviders = new Set<string>(); | ||
| const topMappings: Array<Record<string, any>> = []; | ||
| for (const m of mappings) { | ||
| if (!seenProviders.has(m.providerId)) { | ||
| seenProviders.add(m.providerId); | ||
| topMappings.push(m); | ||
| } | ||
| if (topMappings.length >= 50) { | ||
| break; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Fill the remaining mapping slots after the per-provider pass.
This guarantees provider coverage, but it no longer guarantees 50 seeded mapping histories. If there are fewer than 50 distinct providers, topMappings stops at provider count, so most model/provider rows still won't have history and the admin charts stay much sparser than this PR intends.
💡 Proposed fix
- // Pick one mapping per provider to ensure all providers have history data
- const seenProviders = new Set<string>();
- const topMappings: Array<Record<string, any>> = [];
- for (const m of mappings) {
- if (!seenProviders.has(m.providerId)) {
- seenProviders.add(m.providerId);
- topMappings.push(m);
- }
- if (topMappings.length >= 50) {
- break;
- }
- }
+ // First ensure every provider is represented, then fill the remaining slots up to 50.
+ const seenProviders = new Set<string>();
+ const selectedMappingIds = new Set<string>();
+ const topMappings: typeof mappings = [];
+ for (const m of mappings) {
+ if (seenProviders.has(m.providerId)) {
+ continue;
+ }
+ seenProviders.add(m.providerId);
+ selectedMappingIds.add(m.id);
+ topMappings.push(m);
+ }
+ for (const m of mappings) {
+ if (topMappings.length >= 50) {
+ break;
+ }
+ if (selectedMappingIds.has(m.id)) {
+ continue;
+ }
+ selectedMappingIds.add(m.id);
+ topMappings.push(m);
+ }📝 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
| // Pick one mapping per provider to ensure all providers have history data | |
| const seenProviders = new Set<string>(); | |
| const topMappings: Array<Record<string, any>> = []; | |
| for (const m of mappings) { | |
| if (!seenProviders.has(m.providerId)) { | |
| seenProviders.add(m.providerId); | |
| topMappings.push(m); | |
| } | |
| if (topMappings.length >= 50) { | |
| break; | |
| } | |
| } | |
| // First ensure every provider is represented, then fill the remaining slots up to 50. | |
| const seenProviders = new Set<string>(); | |
| const selectedMappingIds = new Set<string>(); | |
| const topMappings: typeof mappings = []; | |
| for (const m of mappings) { | |
| if (seenProviders.has(m.providerId)) { | |
| continue; | |
| } | |
| seenProviders.add(m.providerId); | |
| selectedMappingIds.add(m.id); | |
| topMappings.push(m); | |
| } | |
| for (const m of mappings) { | |
| if (topMappings.length >= 50) { | |
| break; | |
| } | |
| if (selectedMappingIds.has(m.id)) { | |
| continue; | |
| } | |
| selectedMappingIds.add(m.id); | |
| topMappings.push(m); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/db/src/seed.ts` around lines 1099 - 1110, The current per-provider
selection loop (using seenProviders and topMappings over mappings) ensures one
mapping per provider but can yield fewer than 50 entries; to fix, after that
loop add a second pass over mappings and append additional mappings to
topMappings until topMappings.length === 50 (skipping ones already in
seenProviders or already present) so you preserve provider coverage first and
then fill remaining slots; update the logic that builds topMappings to perform
the initial per-provider pass then a follow-up pass that fills to 50 while
honoring uniqueness and the existing seenProviders set.
smakosh
added this pull request to the merge queue
Mar 7, 2026
5 tasks
This was referenced Mar 25, 2026
4 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
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
Test plan
pnpm buildfor api and admin apps🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
UI/UX Improvements