-
Notifications
You must be signed in to change notification settings - Fork 26
improve usage screen loadtime #1834
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughOptimization of the usage settings component to conditionally fetch integrations and thread audit events based on the active usage type tab, reducing unnecessary API calls when specific tabs aren't displayed. Changes
Sequence DiagramsequenceDiagram
actor User
participant Component
participant useIntegrations
participant useAuditEvents
participant API
User->>Component: Switch to "contract" tab
Component->>useIntegrations: shouldFetch: true
useIntegrations->>API: Fetch integrations
API-->>useIntegrations: Return data
useIntegrations-->>Component: integrations loaded
User->>Component: Switch to "thread" tab
Component->>useAuditEvents: enabled: true
useAuditEvents->>API: Fetch thread audit events
API-->>useAuditEvents: Return data
useAuditEvents-->>Component: threadsData loaded
User->>Component: Switch to other tab
Component->>useIntegrations: shouldFetch: false
useIntegrations-->>Component: skip fetch
Component->>useAuditEvents: enabled: false
useAuditEvents-->>Component: skip fetch
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10–12 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ 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 |
Deploying decocms-admin-frontend with
|
| Latest commit: |
5170544
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://149b3680.admin-decocms-frontend.pages.dev |
| Branch Preview URL: | https://tavano-fix-usage-loadtime.admin-decocms-frontend.pages.dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/src/components/settings/usage/filters.tsx(1 hunks)apps/web/src/components/settings/usage/usage.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
apps/web/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (apps/web/.cursor/rules/posthog-integration.mdc)
apps/web/**/*.{js,jsx,ts,tsx}: Use each feature flag in as few places as possible; if a flag must appear at multiple callsites, explicitly flag this for careful developer review
Gate any flag-dependent code behind checks that verify the flag’s values are valid and expected
If a custom person or event property is referenced in two or more files or at two or more callsites in the same file, centralize the keys in an enum (TS) or const object (JS)
Files:
apps/web/src/components/settings/usage/usage.tsxapps/web/src/components/settings/usage/filters.tsx
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/web/.cursor/rules/posthog-integration.mdc)
In TypeScript, store feature flag names in an enum with members written UPPERCASE_WITH_UNDERSCORE and use a consistent naming convention
Files:
apps/web/src/components/settings/usage/usage.tsxapps/web/src/components/settings/usage/filters.tsx
apps/web/**
📄 CodeRabbit inference engine (AGENTS.md)
Place the Vite/React web client in apps/web
Files:
apps/web/src/components/settings/usage/usage.tsxapps/web/src/components/settings/usage/filters.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Biome formatting: use two-space indentation and double quotes
Keep imports sorted
Name hooks and utility functions using camelCase
Files:
apps/web/src/components/settings/usage/usage.tsxapps/web/src/components/settings/usage/filters.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Name React components and classes using PascalCase
Files:
apps/web/src/components/settings/usage/usage.tsxapps/web/src/components/settings/usage/filters.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Prefer specific TypeScript types over any
Files:
apps/web/src/components/settings/usage/usage.tsxapps/web/src/components/settings/usage/filters.tsx
{apps/web,packages}/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Keep Tailwind design tokens consistent with the design system
Files:
apps/web/src/components/settings/usage/usage.tsxapps/web/src/components/settings/usage/filters.tsx
🧬 Code graph analysis (2)
apps/web/src/components/settings/usage/usage.tsx (5)
packages/sdk/src/mcp/schema.ts (1)
integrations(351-363)packages/sdk/src/hooks/mcp.ts (1)
useIntegrations(141-175)packages/sdk/src/hooks/wallet.ts (3)
useUsagePerAgent(29-42)useUsagePerThread(47-59)useContractsCommits(82-94)apps/web/src/components/settings/usage/members.ts (1)
useMembersWithUnknownUsers(59-85)packages/sdk/src/hooks/audit.ts (1)
useAuditEvents(6-15)
apps/web/src/components/settings/usage/filters.tsx (1)
packages/sdk/src/hooks/mcp.ts (1)
useIntegrations(141-175)
⏰ 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). (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (3)
apps/web/src/components/settings/usage/filters.tsx (1)
34-37: LGTM! Optimization aligns fetch with usage.The conditional fetch correctly ensures integrations are only loaded when displaying the contract tab. Since the contract dropdown (lines 63-81) only renders when
usageType === "contract", this optimization eliminates unnecessary API calls without affecting functionality.apps/web/src/components/settings/usage/usage.tsx (2)
73-78: LGTM! Well-documented optimization.The conditional fetch is correctly aligned with usage (line 146 in chartData), and the detailed comment explains the performance rationale. React Query will deduplicate this request with the identical call in
filters.tsx.
299-299: LGTM! Safe handling of conditionally-fetched data.The variable rename improves clarity, and the optional chaining with
|| []fallback safely handles the case whenthreadsDatais undefined (whenusageType !== "thread"). SinceThreadsTableonly renders whenusageType === "thread"(line 293), the data will be available when needed.
| // ONLY fetch audit events when actually viewing thread tab | ||
| // This is the most expensive query (100 records) - defer it! | ||
| const { data: threadsData } = useAuditEvents({ | ||
| orderBy: "updatedAt_desc", | ||
| limit: 100, | ||
| enabled: usageType === "thread", | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify that enabled option is properly forwarded to React Query.
The useAuditEvents hook receives enabled: usageType === "thread" but the implementation shown in packages/sdk/src/hooks/audit.ts (lines 5-14) doesn't destructure or forward enabled to the useQuery config. If enabled is being passed as part of the generic options object, it will:
- Pollute the query key (causing unnecessary cache variations)
- Potentially be passed to
listThreads(which shouldn't receive React Query options) - Not control query execution (defeating this optimization)
For conditional queries, enabled must be passed directly to useQuery's config, not through the options object.
🏁 Script executed:
#!/bin/bash
# Verify that useAuditEvents properly forwards the enabled option to useQuery
echo "=== Current useAuditEvents implementation ==="
rg -A 15 "export const useAuditEvents" packages/sdk/src/hooks/audit.ts
echo -e "\n=== Check if ThreadFilterOptions type includes 'enabled' ==="
rg -B 5 -A 10 "ThreadFilterOptions" packages/sdk/src/
echo -e "\n=== Search for any recent changes to useAuditEvents that handle 'enabled' ==="
git log --all --oneline --grep="enabled" --grep="useAuditEvents" -- packages/sdk/src/hooks/audit.ts | head -20
echo -e "\n=== Check if there are other patterns for conditional queries in the SDK ==="
rg -A 10 "enabled.*usageType|shouldFetch" packages/sdk/src/hooks/Length of output: 12625
Fix enabled handling in useAuditEvents hook to properly implement conditional fetching.
The code passes enabled: usageType === "thread" to useAuditEvents, but:
ThreadFilterOptions(packages/sdk/src/crud/thread.ts) does not include anenabledproperty- The hook doesn't extract or forward
enabledto React Query's config, so the query executes regardless of the condition - Other hooks in the codebase (e.g.,
useThreadMessages,useMCPIntegrations) use a separateshouldFetchparameter extracted and checked withinqueryFn
Required fix: Either extract enabled and pass it to useQuery's config object, or adopt the shouldFetch pattern used consistently elsewhere in packages/sdk/src/hooks/.
🤖 Prompt for AI Agents
In apps/web/src/components/settings/usage/usage.tsx around lines 98 to 104, the
call to useAuditEvents uses enabled: usageType === "thread" but the hook does
not accept or forward an enabled flag so the query always runs; update the hook
call and/or the hook implementation to support conditional fetching: either (A)
change the hook signature to accept an enabled boolean and forward it to the
underlying useQuery config (useQuery(..., { enabled })) or (B) follow the
project's shouldFetch pattern — accept a shouldFetch boolean, check it at the
top of the hook/queryFn (return early/skip) and only execute the query when true
— then update this call to pass usageType === "thread" as the new parameter.
WIP