feat(dashboard): a few tweaks to the AI models page#3315
Conversation
|
WalkthroughUI and feature-flag related changes across model pages, navigation, server config, and a test. Tabs were relabeled and swapped (the former "Your Usage" → "Metrics"; "Global Metrics" → "Global metrics") with corresponding keyboard-shortcut and tab-state mapping updates. Several UI widgets were removed (the "Total Calls" big-number and the "Calls over time" chart), grid/height layouts adjusted, and the models table lost its "Calls (7d)" column. Server-side changes: removed Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Additional notes
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
...tes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.models._index/route.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/webapp/app/components/navigation/SideMenu.tsx (1)
480-490: Remove redundant nested gate for “Models”.Line 480 repeats the same guard already applied at Line 462, so this branch can be simplified.
♻️ Proposed simplification
- {(user.admin || user.isImpersonating || featureFlags.hasAiAccess) && ( - <SideMenuItem - name="Models" - icon={CubeIcon} - activeIconColor="text-purple-500" - inactiveIconColor="text-purple-500" - to={v3ModelsPath(organization, project, environment)} - data-action="models" - isCollapsed={isCollapsed} - /> - )} + <SideMenuItem + name="Models" + icon={CubeIcon} + activeIconColor="text-purple-500" + inactiveIconColor="text-purple-500" + to={v3ModelsPath(organization, project, environment)} + data-action="models" + isCollapsed={isCollapsed} + />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/components/navigation/SideMenu.tsx` around lines 480 - 490, The "Models" SideMenuItem is wrapped in a duplicate conditional (user.admin || user.isImpersonating || featureFlags.hasAiAccess) that is already applied by the parent guard; remove the redundant outer conditional and render the SideMenuItem directly so the component (SideMenuItem name="Models", icon=CubeIcon, to={v3ModelsPath(organization, project, environment)}, data-action="models", isCollapsed={isCollapsed}) is not double-guarded.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/webapp/app/components/navigation/SideMenu.tsx`:
- Around line 480-490: The "Models" SideMenuItem is wrapped in a duplicate
conditional (user.admin || user.isImpersonating || featureFlags.hasAiAccess)
that is already applied by the parent guard; remove the redundant outer
conditional and render the SideMenuItem directly so the component (SideMenuItem
name="Models", icon=CubeIcon, to={v3ModelsPath(organization, project,
environment)}, data-action="models", isCollapsed={isCollapsed}) is not
double-guarded.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 30011a20-ad16-48b7-b89d-91bf9f5e7c05
📒 Files selected for processing (5)
apps/webapp/app/components/navigation/SideMenu.tsxapps/webapp/app/env.server.tsapps/webapp/app/presenters/OrganizationsPresenter.server.tsapps/webapp/app/v3/canAccessAiModels.server.tsapps/webapp/app/v3/featureFlags.ts
💤 Files with no reviewable changes (3)
- apps/webapp/app/v3/featureFlags.ts
- apps/webapp/app/v3/canAccessAiModels.server.ts
- apps/webapp/app/env.server.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (28)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: sdk-compat / Node.js 22.12 (ubuntu-latest)
- GitHub Check: sdk-compat / Node.js 20.20 (ubuntu-latest)
- GitHub Check: sdk-compat / Cloudflare Workers
- GitHub Check: sdk-compat / Deno Runtime
- GitHub Check: sdk-compat / Bun Runtime
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
Files:
apps/webapp/app/presenters/OrganizationsPresenter.server.tsapps/webapp/app/components/navigation/SideMenu.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/presenters/OrganizationsPresenter.server.tsapps/webapp/app/components/navigation/SideMenu.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/presenters/OrganizationsPresenter.server.tsapps/webapp/app/components/navigation/SideMenu.tsx
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/presenters/OrganizationsPresenter.server.tsapps/webapp/app/components/navigation/SideMenu.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Files:
apps/webapp/app/presenters/OrganizationsPresenter.server.tsapps/webapp/app/components/navigation/SideMenu.tsx
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries
**/*.ts: Usetypecheckto verify changes in apps and internal packages (apps/*,internal-packages/*), notbuild- building proves almost nothing about correctness
When writing Trigger.dev tasks, always import from@trigger.dev/sdk. Never use@trigger.dev/sdk/v3or deprecatedclient.defineJob
Add crumbs as you write code - mark lines with//@Crumbsor wrap blocks in `// `#region` `@crumbsfor agentcrumbs debug tracing, then strip before merge
Files:
apps/webapp/app/presenters/OrganizationsPresenter.server.ts
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier before committing
Files:
apps/webapp/app/presenters/OrganizationsPresenter.server.tsapps/webapp/app/components/navigation/SideMenu.tsx
apps/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
When modifying only server components (
apps/webapp/,apps/supervisor/, etc.) with no package changes, add a.server-changes/file instead of a changeset
Files:
apps/webapp/app/presenters/OrganizationsPresenter.server.tsapps/webapp/app/components/navigation/SideMenu.tsx
apps/webapp/**/*.server.{ts,tsx}
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
apps/webapp/**/*.server.{ts,tsx}: Environment variables must be accessed via theenvexport fromapp/env.server.tsand never useprocess.envdirectly
Always usefindFirstinstead offindUniquein Prisma queries due to implicit DataLoader batching issues and performance concerns
Files:
apps/webapp/app/presenters/OrganizationsPresenter.server.ts
apps/webapp/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Use named constants for sentinel/placeholder values instead of raw string literals scattered across comparisons
Files:
apps/webapp/app/presenters/OrganizationsPresenter.server.tsapps/webapp/app/components/navigation/SideMenu.tsx
apps/webapp/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Only use
useCallback/useMemofor context provider values, expensive derived data with external dependencies, or stable refs required by dependency arrays; don't wrap ordinary event handlers or trivial computations
Files:
apps/webapp/app/components/navigation/SideMenu.tsx
🧠 Learnings (7)
📚 Learning: 2026-03-26T17:27:09.938Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 3264
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.private-connections._index/route.tsx:176-176
Timestamp: 2026-03-26T17:27:09.938Z
Learning: In `apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.private-connections._index/route.tsx`, the variable `hasPrivateNetworking` is intentionally hardcoded to `true` as a placeholder. Plan-level gating will be wired to actual billing data (e.g., `plan?.v3Subscription?.plan?.limits?.hasPrivateNetworking`) once the billing integration is complete. The route is already guarded at the feature-flag level via `hasPrivateConnections` in the loader. Do not flag this hardcoded value as dead code or a bug until the billing integration is in place.
Applied to files:
apps/webapp/app/presenters/OrganizationsPresenter.server.ts
📚 Learning: 2025-08-06T14:25:20.438Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 2354
File: apps/webapp/app/presenters/v3/RegionsPresenter.server.ts:117-131
Timestamp: 2025-08-06T14:25:20.438Z
Learning: In RegionsPresenter.server.ts, the final sort by name that removes the "default first" ordering is intentional behavior as a way to prefer the default, as clarified by matt-aitken.
Applied to files:
apps/webapp/app/presenters/OrganizationsPresenter.server.ts
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/app/v3/presenters/**/*.server.{ts,tsx} : Organize presenters in the webapp following the pattern `app/v3/presenters/*/*.server.ts` to move complex loader code into classes
Applied to files:
apps/webapp/app/presenters/OrganizationsPresenter.server.ts
📚 Learning: 2026-03-22T13:26:12.060Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3244
File: apps/webapp/app/components/code/TextEditor.tsx:81-86
Timestamp: 2026-03-22T13:26:12.060Z
Learning: In the triggerdotdev/trigger.dev codebase, do not flag `navigator.clipboard.writeText(...)` calls for `missing-await`/`unhandled-promise` issues. These clipboard writes are intentionally invoked without `await` and without `catch` handlers across the project; keep that behavior consistent when reviewing TypeScript/TSX files (e.g., usages like in `apps/webapp/app/components/code/TextEditor.tsx`).
Applied to files:
apps/webapp/app/presenters/OrganizationsPresenter.server.tsapps/webapp/app/components/navigation/SideMenu.tsx
📚 Learning: 2026-03-22T19:24:14.403Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3187
File: apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.ts:200-204
Timestamp: 2026-03-22T19:24:14.403Z
Learning: In the triggerdotdev/trigger.dev codebase, webhook URLs are not expected to contain embedded credentials/secrets (e.g., fields like `ProjectAlertWebhookProperties` should only hold credential-free webhook endpoints). During code review, if you see logging or inclusion of raw webhook URLs in error messages, do not automatically treat it as a credential-leak/secrets-in-logs issue by default—first verify the URL does not contain embedded credentials (for example, no username/password in the URL, no obvious secret/token query params or fragments). If the URL is credential-free per this project’s conventions, allow the logging.
Applied to files:
apps/webapp/app/presenters/OrganizationsPresenter.server.tsapps/webapp/app/components/navigation/SideMenu.tsx
📚 Learning: 2026-03-22T13:45:36.346Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3244
File: apps/webapp/app/components/navigation/SideMenu.tsx:460-489
Timestamp: 2026-03-22T13:45:36.346Z
Learning: In triggerdotdev/trigger.dev, sidebar navigation items (SideMenu.tsx) are intentionally NOT gated behind feature-flag or permission checks at the nav level. Authorization is enforced at the route/loader level instead. Hiding nav items based on access checks is considered confusing UX. This applies to items like "AI Metrics" (v3BuiltInDashboardPath) and other dashboard links — they are always rendered in the sidebar regardless of hasQueryAccess or similar flags.
Applied to files:
apps/webapp/app/components/navigation/SideMenu.tsx
📚 Learning: 2026-02-11T16:37:32.429Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3019
File: apps/webapp/app/components/primitives/charts/Card.tsx:26-30
Timestamp: 2026-02-11T16:37:32.429Z
Learning: In projects using react-grid-layout, avoid relying on drag-handle class to imply draggability. Ensure drag-handle elements only affect dragging when the parent grid item is configured draggable in the layout; conditionally apply cursor styles based on the draggable prop. This improves correctness and accessibility.
Applied to files:
apps/webapp/app/components/navigation/SideMenu.tsx
🔇 Additional comments (1)
apps/webapp/app/presenters/OrganizationsPresenter.server.ts (1)
160-164: Good default-flag wiring for AI access.Line 162 correctly sources the flag from
envand keeps the existing global+org override flow intact.
No description provided.