fix: flash on page transition#516
Conversation
Greptile SummaryThis PR fixes page transition flash by adding Major changes:
The fix addresses the flash issue by instructing the router to render nothing during the pending state rather than showing a default loading indicator, combined with improved data prefetching in loaders. Confidence Score: 5/5
|
| Filename | Overview |
|---|---|
| services/platform/app/routes/dashboard/$id/automations/index.tsx | Added pendingComponent: () => null and pendingMs: 0 to prevent flash during navigation, switched to paginated queries, streamlined loading states |
| services/platform/app/routes/dashboard/$id/custom-agents/index.tsx | Added pendingComponent: () => null and pendingMs: 0 to prevent flash, switched to paginated queries, simplified loading logic |
| services/platform/app/features/automations/components/automations-table.tsx | Refactored from automations-client.tsx, now accepts paginated results, improved data handling with activeVersionMap |
| services/platform/convex/workflows/definitions/list_automations_paginated.ts | New paginated query handler that enriches automation data with active version IDs and resolved status |
| services/platform/convex/custom_agents/list_custom_agents_paginated.ts | New paginated handler for custom agents with team access filtering and version resolution |
| services/platform/app/routes/dashboard/$id/_knowledge/products.tsx | Added pendingComponent: () => null and pendingMs: 0, renamed ProductTable to ProductsTable, added count check |
| services/platform/app/routes/dashboard/$id/_knowledge.tsx | Added pendingComponent: () => null and pendingMs: 0 to layout route |
Last reviewed commit: f61000c
📝 WalkthroughWalkthroughThis pull request introduces pagination support across multiple table components and their associated routes. Changes include renaming components for naming consistency (ProductTable → ProductsTable, CustomAgentTable → CustomAgentsTable), migrating table components to consume Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 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 unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
services/platform/app/routes/dashboard/$id/_knowledge/documents.tsx (1)
22-25:⚠️ Potential issue | 🟡 MinorFire-and-forget prefetch still allows a cold-cache flash on first load.
void context.queryClient.prefetchQuery(...)resolves the loader synchronously, so TanStack Router renders the component immediately without waiting for the prefetch. On a warm cache (subsequent page transitions),useHasMicrosoftAccount()returns the cached value instantly — the stated flash is fixed. On a cold cache (first navigation to this route),hasMicrosoftAccountstarts asfalseand re-renders totrueonce the query resolves, causing the same brief flash for users with a Microsoft account.If first-load flash is also in scope, the loader should
awaitthe prefetch:💡 Proposed change to block on first load
loader: ({ context }) => { - void context.queryClient.prefetchQuery( + return context.queryClient.prefetchQuery( convexQuery(api.accounts.queries.hasMicrosoftAccount, {}), ); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/app/routes/dashboard/`$id/_knowledge/documents.tsx around lines 22 - 25, The loader currently calls void context.queryClient.prefetchQuery(convexQuery(api.accounts.queries.hasMicrosoftAccount, {})) which fire-and-forgets and lets TanStack Router resolve the loader before the query completes, causing a cold-cache flash in useHasMicrosoftAccount; change the loader to await the prefetchQuery call (i.e., await context.queryClient.prefetchQuery(...)) so the loader blocks until convexQuery(api.accounts.queries.hasMicrosoftAccount) resolves and the component renders with the cached value on first load.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@services/platform/app/features/automations/components/automations-table.tsx`:
- Around line 41-52: The code is using unnecessary optional chaining and nullish
fallback on paginatedResult.results even though
UsePaginatedQueryResult<T>.results is always an array; remove the `?.` and `??
[]` around paginatedResult.results in both the map that builds the id->version
Map and the useMemo that defines tableResults (the
paginatedResult.results.map(...) and the fallback to []), so both expressions
directly use paginatedResult.results to simplify types and avoid redundant
guards while keeping the same behavior in the functions that reference
paginatedResult.results and the tableResults memo.
In `@services/platform/app/routes/dashboard/`$id/_knowledge/tone-of-voice.tsx:
- Around line 27-28: The route's pending configuration uses pendingComponent: ()
=> null with pendingMs: 0 but lacks pendingMinMs: 0, causing a 500ms enforced
blank; update the route config in
services/platform/app/routes/dashboard/$id/_knowledge/tone-of-voice.tsx (the
object containing pendingComponent and pendingMs) to add pendingMinMs: 0
alongside pendingMs so the null pendingComponent does not get the router's
default 500ms minimum display time; apply the same change to the other routes
using pendingComponent: () => null + pendingMs: 0 or alternatively set
defaultPendingMinMs: 0 in the router if you intend this behavior globally.
In `@services/platform/app/routes/dashboard/`$id/automations/index.tsx:
- Around line 24-29: Remove the ineffective React Query prefetch: delete the
call to
context.queryClient.prefetchQuery(convexQuery(api.wf_definitions.queries.listAutomationsPaginated,
{...})) since the component uses useListAutomationsPaginated →
useCachedPaginatedQuery → useConvexPaginatedQuery (native Convex
usePaginatedQuery) which reads from its own Map-based cache and never from React
Query; leave the pendingMs: 0 + pendingComponent noop flash-fix as-is and ensure
no other code relies on the removed prefetch call.
In `@services/platform/app/routes/dashboard/`$id/settings/teams.tsx:
- Line 52: The teams route is swallowing errors from the approxCountMyTeams
fetch by wrapping ensureQueryData in a try/catch, causing count to remain
undefined and the component to return null; remove the try/catch in the teams
loader so ensureQueryData/approxCountMyTeams errors propagate to TanStack
Router's error boundary (consistent with vendors/customers/products) and restore
proper AccessDenied/error handling for non-admin users; alternatively, if you
prefer an in-component recovery, replace the final "if (isAuthLoading ||
isMemberLoading || count === undefined) return null;" with a small recoverable
error UI when isAuthLoading and isMemberLoading are false and count is
undefined, referencing ensureQueryData, approxCountMyTeams, isAuthLoading,
isMemberLoading, and count to locate the change.
In `@services/platform/convex/custom_agents/list_custom_agents_paginated.ts`:
- Around line 34-57: The paginated result is being filtered after fetching
(hasTeamAccess on each root), causing undersized pages; fix by either (A)
pre-filtering at query-time with a team-aware index or (B) over-fetching and
trimming: when building enrichedItems in list_custom_agents_paginated, call
paginate() with a larger pageSize (e.g., numItems * 2), keep fetching additional
pages (using continueCursor) and processing roots/querying drafts/active until
enrichedItems.length >= numItems or no more pages, then slice enrichedItems to
numItems and set continueCursor appropriately; alternatively implement a new DB
index (e.g., withIndex('by_team_root_status') or similar) and include teamId in
the query to avoid post-pagination filtering.
- Around line 37-56: Summary: current code inside list_custom_agents_paginated
runs an N+1 (1–2) extra indexed lookup per root by querying for draft then
active versions (variables: draft, active, root, root._id, enrichedItems), which
will scale poorly if page sizes grow. Fix: either denormalize a "bestVersionId"
onto the root record and return root or fetch that single version in one step,
or batch the lookups by collecting all root._id values for the page and issuing
a single query to customAgents for those root IDs (filtering status = 'draft'
and 'active' in one request) and then map results back to each root to populate
enrichedItems; update the code that currently does the two sequential queries
(the draft/active lookups) to use the chosen batched or denormalized approach so
you avoid per-root queries when populating enrichedItems.
In
`@services/platform/convex/workflows/definitions/list_automations_paginated.ts`:
- Around line 10-18: Change the return type of resolveEffectiveStatus to the
literal union 'active'|'archived'|'draft' instead of string so TypeScript
narrows status correctly; update the function signature for
resolveEffectiveStatus(name: string, activeByName: Map<string,
Doc<'wfDefinitions'>>, archivedNames: Set<string>): 'active' | 'archived' |
'draft' and ensure any variable or type annotations that capture its result
(e.g., the status field on AutomationItem or callers expecting a string) accept
the literal union rather than plain string.
- Around line 6-8: The AutomationItem type currently declares activeVersionId as
string | null which loses Convex's Id<'wfDefinitions'> typing; change the type
of activeVersionId on the AutomationItem alias to Id<'wfDefinitions'> | null so
callers (e.g., AutomationRowActions and any navigation logic using
activeVersion?._id) preserve the Convex-branded id type and avoid casts.
In `@services/platform/messages/en.json`:
- Around line 2868-2871: The metadata object contains two identical keys named
"customAgents" so the earlier one is ignored; locate both "customAgents" entries
and either (A) rename the newly added entry to a unique key (e.g.,
"customAgentsSettings") wherever it's referenced (update any lookup/usage
accordingly) if you need a separate settings-nav entry, or (B) remove the new
duplicate and unify the description into the existing "customAgents" entry by
updating its "description" to the preferred text; ensure all consumers/reference
keys are updated to match the chosen key.
---
Outside diff comments:
In `@services/platform/app/routes/dashboard/`$id/_knowledge/documents.tsx:
- Around line 22-25: The loader currently calls void
context.queryClient.prefetchQuery(convexQuery(api.accounts.queries.hasMicrosoftAccount,
{})) which fire-and-forgets and lets TanStack Router resolve the loader before
the query completes, causing a cold-cache flash in useHasMicrosoftAccount;
change the loader to await the prefetchQuery call (i.e., await
context.queryClient.prefetchQuery(...)) so the loader blocks until
convexQuery(api.accounts.queries.hasMicrosoftAccount) resolves and the component
renders with the cached value on first load.
| for (const a of paginatedResult.results ?? []) { | ||
| if (a.activeVersionId) map.set(a._id, a.activeVersionId); | ||
| } | ||
| return map; | ||
| }, [automations]); | ||
| }, [paginatedResult.results]); | ||
|
|
||
| const tableData = useMemo( | ||
| () => automations.map(({ activeVersionId: _, ...rest }) => rest), | ||
| [automations], | ||
| const tableResults = useMemo( | ||
| (): Doc<'wfDefinitions'>[] => | ||
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
| paginatedResult.results?.map(({ activeVersionId: _, ...rest }) => rest) ?? | ||
| [], | ||
| [paginatedResult.results], |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Redundant ?. and ?? [] on paginatedResult.results.
UsePaginatedQueryResult<T>.results is typed as T[] — it is always an array, never null or undefined. Both the optional chain (?.) on line 50 and the ?? [] fallback on lines 41 and 50–51 can be removed.
♻️ Proposed cleanup
- for (const a of paginatedResult.results ?? []) {
+ for (const a of paginatedResult.results) {- paginatedResult.results?.map(({ activeVersionId: _, ...rest }) => rest) ??
- [],
+ paginatedResult.results.map(({ activeVersionId: _, ...rest }) => rest),📝 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.
| for (const a of paginatedResult.results ?? []) { | |
| if (a.activeVersionId) map.set(a._id, a.activeVersionId); | |
| } | |
| return map; | |
| }, [automations]); | |
| }, [paginatedResult.results]); | |
| const tableData = useMemo( | |
| () => automations.map(({ activeVersionId: _, ...rest }) => rest), | |
| [automations], | |
| const tableResults = useMemo( | |
| (): Doc<'wfDefinitions'>[] => | |
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | |
| paginatedResult.results?.map(({ activeVersionId: _, ...rest }) => rest) ?? | |
| [], | |
| [paginatedResult.results], | |
| for (const a of paginatedResult.results) { | |
| if (a.activeVersionId) map.set(a._id, a.activeVersionId); | |
| } | |
| return map; | |
| }, [paginatedResult.results]); | |
| const tableResults = useMemo( | |
| (): Doc<'wfDefinitions'>[] => | |
| // eslint-disable-next-line `@typescript-eslint/no-unused-vars` | |
| paginatedResult.results.map(({ activeVersionId: _, ...rest }) => rest), | |
| [paginatedResult.results], |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/features/automations/components/automations-table.tsx`
around lines 41 - 52, The code is using unnecessary optional chaining and
nullish fallback on paginatedResult.results even though
UsePaginatedQueryResult<T>.results is always an array; remove the `?.` and `??
[]` around paginatedResult.results in both the map that builds the id->version
Map and the useMemo that defines tableResults (the
paginatedResult.results.map(...) and the fallback to []), so both expressions
directly use paginatedResult.results to simplify types and avoid redundant
guards while keeping the same behavior in the functions that reference
paginatedResult.results and the tableResults memo.
| pendingComponent: () => null, | ||
| pendingMs: 0, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 1. Search for router instantiation and defaultPendingMinMs configuration
echo "=== Router Configuration Search ==="
rg -n "defaultPendingMinMs|createRouter|RootRoute|Router" --type=ts --type=tsx -A5 -B2 | head -100
echo -e "\n=== Router File Locations ==="
fd -e ts -e tsx -path "*/router*" | head -20Repository: tale-project/tale
Length of output: 341
🏁 Script executed:
#!/bin/bash
# 2. Check all routes in the dashboard for pendingMinMs/pendingMs patterns
echo "=== Checking pendingMinMs and pendingMs across dashboard routes ==="
fd -e tsx . services/platform/app/routes/dashboard --exec grep -l "pendingMs\|pendingComponent" {} \; | head -20
echo -e "\n=== Detailed view of routes with pending settings ==="
fd -e tsx . services/platform/app/routes/dashboard --exec grep -H -n "pendingComponent\|pendingMinMs\|pendingMs" {} \; | head -50Repository: tale-project/tale
Length of output: 2221
🏁 Script executed:
#!/bin/bash
# 3. Look at the actual file mentioned in the review
echo "=== Target file: tone-of-voice.tsx (lines 20-100) ==="
if [ -f "services/platform/app/routes/dashboard/\$id/_knowledge/tone-of-voice.tsx" ]; then
sed -n '20,100p' "services/platform/app/routes/dashboard/\$id/_knowledge/tone-of-voice.tsx"
else
# Try alternate path patterns
find . -name "tone-of-voice.tsx" -type f 2>/dev/null | head -5
fiRepository: tale-project/tale
Length of output: 2301
🏁 Script executed:
#!/bin/bash
# Look for router setup/instantiation files
echo "=== Searching for router configuration files ==="
find . -name "*router*" -type f \( -name "*.ts" -o -name "*.tsx" \) | grep -v node_modules | head -20
echo -e "\n=== Looking for root route or createRootRoute ==="
rg "createRootRoute|new Router|createRouter" --type ts --max-count 10 -A10
echo -e "\n=== Check main entry point ==="
ls -la services/platform/app/ | head -20Repository: tale-project/tale
Length of output: 4910
🏁 Script executed:
#!/bin/bash
# Search for defaultPendingMinMs without type restriction
echo "=== Searching for defaultPendingMinMs ==="
rg "defaultPendingMinMs" -A2 -B2
echo -e "\n=== Search routes directory structure ==="
ls -la services/platform/app/routes/
echo -e "\n=== Look for __root.tsx or similar ==="
find services/platform/app/routes -name "__root*" -o -name "root*" | head -10Repository: tale-project/tale
Length of output: 820
🏁 Script executed:
#!/bin/bash
# Read the full router.tsx file
echo "=== Full router.tsx content ==="
cat services/platform/app/router.tsxRepository: tale-project/tale
Length of output: 1780
Add pendingMinMs: 0 to prevent a 500 ms blank screen on navigation
pendingMs: 0 means the pendingComponent (() => null) activates immediately. However, TanStack Router enforces a minimum display time of 500 ms for pending components by default to avoid jarring flashes. With a null component and no pendingMinMs: 0 override, this manifests as a half-second blank screen on every route navigation—even when the loader's ensureQueryData resolves from cache in milliseconds.
The router is not configured with a global defaultPendingMinMs: 0, so this needs a route-level fix:
🛠️ Proposed fix
pendingComponent: () => null,
pendingMs: 0,
+ pendingMinMs: 0,Note: This pattern (pendingComponent: () => null + pendingMs: 0 without pendingMinMs: 0) is used across 8 routes in this PR. Consider either applying this fix to all affected routes or adding defaultPendingMinMs: 0 at the router level in services/platform/app/router.tsx if this is intentional across all dashboard routes.
📝 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.
| pendingComponent: () => null, | |
| pendingMs: 0, | |
| pendingComponent: () => null, | |
| pendingMs: 0, | |
| pendingMinMs: 0, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/routes/dashboard/`$id/_knowledge/tone-of-voice.tsx
around lines 27 - 28, The route's pending configuration uses pendingComponent:
() => null with pendingMs: 0 but lacks pendingMinMs: 0, causing a 500ms enforced
blank; update the route config in
services/platform/app/routes/dashboard/$id/_knowledge/tone-of-voice.tsx (the
object containing pendingComponent and pendingMs) to add pendingMinMs: 0
alongside pendingMs so the null pendingComponent does not get the router's
default 500ms minimum display time; apply the same change to the other routes
using pendingComponent: () => null + pendingMs: 0 or alternatively set
defaultPendingMinMs: 0 in the router if you intend this behavior globally.
| void context.queryClient.prefetchQuery( | ||
| convexQuery(api.wf_definitions.queries.listAutomations, { | ||
| convexQuery(api.wf_definitions.queries.listAutomationsPaginated, { | ||
| organizationId: params.id, | ||
| paginationOpts: { numItems: 10, cursor: null }, | ||
| }), | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find useCachedPaginatedQuery implementation
rg "useCachedPaginatedQuery" -B 2 -A 15 --type=ts --type=tsxRepository: tale-project/tale
Length of output: 88
🏁 Script executed:
# Find useListAutomationsPaginated implementation
rg "useListAutomationsPaginated" -B 2 -A 15 --type=ts --type=tsxRepository: tale-project/tale
Length of output: 88
🏁 Script executed:
# Check the file directory structure to locate hook files
fd "hooks\|queries" --type=f services/platform/app --max-depth 4 | head -20Repository: tale-project/tale
Length of output: 43
🏁 Script executed:
# Find useCachedPaginatedQuery implementation
rg "useCachedPaginatedQuery" -B 2 -A 15Repository: tale-project/tale
Length of output: 38655
🏁 Script executed:
# Find useListAutomationsPaginated implementation
rg "useListAutomationsPaginated" -B 2 -A 15Repository: tale-project/tale
Length of output: 5193
🏁 Script executed:
# Check for hooks directory in automations features
fd "hooks\|queries" services/platform/app/features --type f | head -30Repository: tale-project/tale
Length of output: 43
🏁 Script executed:
# Read the full useCachedPaginatedQuery implementation
cat services/platform/app/hooks/use-cached-paginated-query.tsRepository: tale-project/tale
Length of output: 2247
🏁 Script executed:
# Find useConvexPaginatedQuery - likely from convex/react library
rg "useConvexPaginatedQuery" -B 2 -A 10Repository: tale-project/tale
Length of output: 4082
🏁 Script executed:
# Check the actual file in the automations route where the prefetch happens
cat services/platform/app/routes/dashboard/$id/automations/index.tsxRepository: tale-project/tale
Length of output: 154
🏁 Script executed:
# Find the automations route file
fd "automations.*index" services/platform/app/routesRepository: tale-project/tale
Length of output: 43
🏁 Script executed:
# Also check for any convexQuery imports and uses
rg "convexQuery" -B 3 -A 8 services/platform/app/routes/dashboardRepository: tale-project/tale
Length of output: 50373
🏁 Script executed:
# Check if convexQuery is intended for paginated queries
rg "convexQuery" -B 2 -A 2 services/platform/app/hooks/use-react-query.tsRepository: tale-project/tale
Length of output: 43
🏁 Script executed:
# Look at the implementation of convexQuery from `@convex-dev/react-query`
rg "@convex-dev/react-query" services/platform --max-count=5Repository: tale-project/tale
Length of output: 4337
🏁 Script executed:
# Check if there are any comments or patterns around convexQuery with paginated queries
rg "convexQuery.*Paginated" -B 2 -A 2 services/platform/app/routesRepository: tale-project/tale
Length of output: 1225
🏁 Script executed:
# Search for documentation or comments about convexQuery design
rg "convexQuery" -B 5 -A 5 services/platform/app/hooks/use-convex-query.tsRepository: tale-project/tale
Length of output: 562
🏁 Script executed:
# Check if useCachedPaginatedQuery reads from React Query anywhere
rg "queryClient\|useQuery\|useInfiniteQuery" services/platform/app/hooks/use-cached-paginated-query.tsRepository: tale-project/tale
Length of output: 43
🏁 Script executed:
# Look for any bridge between convexQuery prefetch and useCachedPaginatedQuery
rg "paginatedQueryCache\|cacheKey" services/platform/app/hooks/use-cached-paginated-query.ts -A 2 -B 2Repository: tale-project/tale
Length of output: 43
🏁 Script executed:
# Read the full automations index route to see component usage
sed -n '1,100p' services/platform/app/routes/dashboard/$id/automations/index.tsxRepository: tale-project/tale
Length of output: 165
🏁 Script executed:
# Check ListAutomationsPaginatedArgs type definition
rg "ListAutomationsPaginatedArgs" -B 2 -A 5 services/platform/app/features/automationsRepository: tale-project/tale
Length of output: 1243
🏁 Script executed:
# Find the automations index route file with correct path
find services/platform/app/routes -name "*automations*" -type fRepository: tale-project/tale
Length of output: 119
🏁 Script executed:
# Check how useConvexQuery is actually used vs useCachedPaginatedQuery
rg "useConvexQuery|useCachedPaginatedQuery" services/platform/app/features/automations/hooks/queries.ts -B 1 -A 3Repository: tale-project/tale
Length of output: 2426
🏁 Script executed:
# Check the exact file path for automations route
cat "services/platform/app/routes/dashboard/\$id/automations.tsx" | head -50Repository: tale-project/tale
Length of output: 1523
🏁 Script executed:
# Verify that convexQuery is truly incompatible with paginated queries by checking `@convex-dev/react-query` documentation in package
rg "paginat" services/platform/package.jsonRepository: tale-project/tale
Length of output: 43
🏁 Script executed:
# Final check: see how the automations loader calls prefetchQuery with the paginated query
rg "listAutomationsPaginated" "services/platform/app/routes/dashboard/\$id/automations.tsx" -B 5 -A 5Repository: tale-project/tale
Length of output: 43
🏁 Script executed:
# Find the actual index file in the automations directory
find services/platform/app/routes/dashboard -path "*automations/index*"Repository: tale-project/tale
Length of output: 125
🏁 Script executed:
# Search for the prefetch of listAutomationsPaginated across all routes
rg "listAutomationsPaginated" services/platform/app/routes -B 3 -A 3Repository: tale-project/tale
Length of output: 792
🏁 Script executed:
# Read the full automations index.tsx file to see the complete context
cat "services/platform/app/routes/dashboard/\$id/automations/index.tsx"Repository: tale-project/tale
Length of output: 2535
Remove ineffective convexQuery prefetch of paginated query.
The loader prefetches listAutomationsPaginated using convexQuery(), which populates React Query's cache. However, the component calls useListAutomationsPaginated() → useCachedPaginatedQuery() → useConvexPaginatedQuery() (the native Convex usePaginatedQuery). The useCachedPaginatedQuery hook maintains its own internal Map-based cache and reads only from that, never from React Query. The two cache systems are completely separate, making the convexQuery prefetch a wasted network call with no effect on the component's loading state.
The flash fix is still achieved via pendingMs: 0 + pendingComponent: () => null, so remove the ineffective prefetch on line 24–29.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/routes/dashboard/`$id/automations/index.tsx around
lines 24 - 29, Remove the ineffective React Query prefetch: delete the call to
context.queryClient.prefetchQuery(convexQuery(api.wf_definitions.queries.listAutomationsPaginated,
{...})) since the component uses useListAutomationsPaginated →
useCachedPaginatedQuery → useConvexPaginatedQuery (native Convex
usePaginatedQuery) which reads from its own Map-based cache and never from React
Query; leave the pendingMs: 0 + pendingComponent noop flash-fix as-is and ensure
no other code relies on the removed prefetch call.
| /> | ||
| ); | ||
| } | ||
| if (isAuthLoading || isMemberLoading || count === undefined) return null; |
There was a problem hiding this comment.
Silent blank page when count query fails.
Unlike the other routes in this PR (vendors, customers, products), the teams loader wraps ensureQueryData in a try/catch and swallows the error. If approxCountMyTeams fails, count stays undefined indefinitely and the component returns null permanently — showing a blank page with no error feedback and no recovery path (short of a full reload). Non-admin users who hit this path also bypass the AccessDenied guard.
Consider either propagating the error (remove try/catch, let TanStack Router's error boundary handle it, consistent with the other routes) or replacing return null with a recoverable error UI when both loading states are false and count is still undefined:
💡 Option: surface count failure rather than silently returning null
- if (isAuthLoading || isMemberLoading || count === undefined) return null;
+ if (isAuthLoading || isMemberLoading) return null;
+ if (count === undefined) {
+ // count query failed (loader swallowed error); surface something actionable
+ return <ErrorState />; // or propagate via throw
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/routes/dashboard/`$id/settings/teams.tsx at line 52,
The teams route is swallowing errors from the approxCountMyTeams fetch by
wrapping ensureQueryData in a try/catch, causing count to remain undefined and
the component to return null; remove the try/catch in the teams loader so
ensureQueryData/approxCountMyTeams errors propagate to TanStack Router's error
boundary (consistent with vendors/customers/products) and restore proper
AccessDenied/error handling for non-admin users; alternatively, if you prefer an
in-component recovery, replace the final "if (isAuthLoading || isMemberLoading
|| count === undefined) return null;" with a small recoverable error UI when
isAuthLoading and isMemberLoading are false and count is undefined, referencing
ensureQueryData, approxCountMyTeams, isAuthLoading, isMemberLoading, and count
to locate the change.
| for (const root of page.page) { | ||
| if (!hasTeamAccess(root, userTeamIds)) continue; | ||
|
|
||
| const draft = await ctx.db | ||
| .query('customAgents') | ||
| .withIndex('by_root_status', (q) => | ||
| q.eq('rootVersionId', root._id).eq('status', 'draft'), | ||
| ) | ||
| .first(); | ||
|
|
||
| if (draft) { | ||
| enrichedItems.push(draft); | ||
| continue; | ||
| } | ||
|
|
||
| const active = await ctx.db | ||
| .query('customAgents') | ||
| .withIndex('by_root_status', (q) => | ||
| q.eq('rootVersionId', root._id).eq('status', 'active'), | ||
| ) | ||
| .first(); | ||
|
|
||
| enrichedItems.push(active ?? root); | ||
| } |
There was a problem hiding this comment.
Post-pagination team-access filtering can return undersized or empty pages.
paginate() fetches a fixed-size page, then hasTeamAccess filters items out in-memory. If most roots on a page belong to teams the user can't access, the returned page will be significantly smaller than numItems — potentially empty — while continueCursor still advances. This forces the client to issue extra loadMore calls to fill the view.
This is a known trade-off with post-pagination filtering. It works correctly with cursors, but if team-scoped agents are common, consider adding a pre-filter (e.g., an index that includes teamId) or over-fetching and trimming to numItems to ensure consistently-sized pages.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/convex/custom_agents/list_custom_agents_paginated.ts`
around lines 34 - 57, The paginated result is being filtered after fetching
(hasTeamAccess on each root), causing undersized pages; fix by either (A)
pre-filtering at query-time with a team-aware index or (B) over-fetching and
trimming: when building enrichedItems in list_custom_agents_paginated, call
paginate() with a larger pageSize (e.g., numItems * 2), keep fetching additional
pages (using continueCursor) and processing roots/querying drafts/active until
enrichedItems.length >= numItems or no more pages, then slice enrichedItems to
numItems and set continueCursor appropriately; alternatively implement a new DB
index (e.g., withIndex('by_team_root_status') or similar) and include teamId in
the query to avoid post-pagination filtering.
| const draft = await ctx.db | ||
| .query('customAgents') | ||
| .withIndex('by_root_status', (q) => | ||
| q.eq('rootVersionId', root._id).eq('status', 'draft'), | ||
| ) | ||
| .first(); | ||
|
|
||
| if (draft) { | ||
| enrichedItems.push(draft); | ||
| continue; | ||
| } | ||
|
|
||
| const active = await ctx.db | ||
| .query('customAgents') | ||
| .withIndex('by_root_status', (q) => | ||
| q.eq('rootVersionId', root._id).eq('status', 'active'), | ||
| ) | ||
| .first(); | ||
|
|
||
| enrichedItems.push(active ?? root); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
N+1 sub-queries per page item are acceptable at current page sizes but worth noting.
Each root triggers 1–2 additional indexed lookups (draft, then active). With numItems: 10, this yields up to 20 extra reads per page — within Convex's query budget. If page sizes grow, consider batching or denormalizing the "best version" pointer onto the root record.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/convex/custom_agents/list_custom_agents_paginated.ts`
around lines 37 - 56, Summary: current code inside list_custom_agents_paginated
runs an N+1 (1–2) extra indexed lookup per root by querying for draft then
active versions (variables: draft, active, root, root._id, enrichedItems), which
will scale poorly if page sizes grow. Fix: either denormalize a "bestVersionId"
onto the root record and return root or fetch that single version in one step,
or batch the lookups by collecting all root._id values for the page and issuing
a single query to customAgents for those root IDs (filtering status = 'draft'
and 'active' in one request) and then map results back to each root to populate
enrichedItems; update the code that currently does the two sequential queries
(the draft/active lookups) to use the chosen batched or denormalized approach so
you avoid per-root queries when populating enrichedItems.
| export type AutomationItem = Doc<'wfDefinitions'> & { | ||
| activeVersionId: string | null; | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
activeVersionId should be typed as Id<'wfDefinitions'> | null.
activeVersion?._id is Id<'wfDefinitions'>, a Convex-branded type. Widening to string | null discards that information and forces callers (e.g., AutomationRowActions navigating to a specific version) to cast it back.
♻️ Proposed fix
+import type { Id } from '../../_generated/dataModel';
+
export type AutomationItem = Doc<'wfDefinitions'> & {
- activeVersionId: string | null;
+ activeVersionId: Id<'wfDefinitions'> | null;
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/convex/workflows/definitions/list_automations_paginated.ts`
around lines 6 - 8, The AutomationItem type currently declares activeVersionId
as string | null which loses Convex's Id<'wfDefinitions'> typing; change the
type of activeVersionId on the AutomationItem alias to Id<'wfDefinitions'> |
null so callers (e.g., AutomationRowActions and any navigation logic using
activeVersion?._id) preserve the Convex-branded id type and avoid casts.
| function resolveEffectiveStatus( | ||
| name: string, | ||
| activeByName: Map<string, Doc<'wfDefinitions'>>, | ||
| archivedNames: Set<string>, | ||
| ): string { | ||
| if (activeByName.has(name)) return 'active'; | ||
| if (archivedNames.has(name)) return 'archived'; | ||
| return 'draft'; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Narrow resolveEffectiveStatus return type to the literal union.
The function can only return 'active', 'archived', or 'draft'. Returning string forces TypeScript to treat the status field in AutomationItem (which extends Doc<'wfDefinitions'>) as a plain string.
♻️ Proposed fix
function resolveEffectiveStatus(
name: string,
activeByName: Map<string, Doc<'wfDefinitions'>>,
archivedNames: Set<string>,
-): string {
+): 'active' | 'archived' | 'draft' {📝 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.
| function resolveEffectiveStatus( | |
| name: string, | |
| activeByName: Map<string, Doc<'wfDefinitions'>>, | |
| archivedNames: Set<string>, | |
| ): string { | |
| if (activeByName.has(name)) return 'active'; | |
| if (archivedNames.has(name)) return 'archived'; | |
| return 'draft'; | |
| } | |
| function resolveEffectiveStatus( | |
| name: string, | |
| activeByName: Map<string, Doc<'wfDefinitions'>>, | |
| archivedNames: Set<string>, | |
| ): 'active' | 'archived' | 'draft' { | |
| if (activeByName.has(name)) return 'active'; | |
| if (archivedNames.has(name)) return 'archived'; | |
| return 'draft'; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/convex/workflows/definitions/list_automations_paginated.ts`
around lines 10 - 18, Change the return type of resolveEffectiveStatus to the
literal union 'active'|'archived'|'draft' instead of string so TypeScript
narrows status correctly; update the function signature for
resolveEffectiveStatus(name: string, activeByName: Map<string,
Doc<'wfDefinitions'>>, archivedNames: Set<string>): 'active' | 'archived' |
'draft' and ensure any variable or type annotations that capture its result
(e.g., the status field on AutomationItem or callers expecting a string) accept
the literal union rather than plain string.
| "customAgents": { | ||
| "title": "Custom agents", | ||
| "description": "Manage custom AI agents for your organization." | ||
| }, |
There was a problem hiding this comment.
Duplicate customAgents key in metadata — the newly added entry is silently discarded.
A "customAgents" block already exists further down in the same metadata object at lines 2908–2911:
// lines 2908-2911 (pre-existing, NOT marked ~)
"customAgents": {
"title": "Custom agents",
"description": "Create and manage AI agents for customer service."
},JSON objects must have unique keys within the same scope. JSON.parse (and virtually every parser) will keep the last occurrence, so the entry added here (lines 2868–2871) is a dead no-op. The two descriptions also differ:
| Location | description |
|---|---|
| New (2868–2871) | "Manage custom AI agents for your organization." |
| Existing (2908–2911) | "Create and manage AI agents for customer service." |
Recommended fix: If these two entries are intentionally for different routing contexts (e.g., settings nav vs. top-level agents page), rename one of them to a distinct key (e.g., customAgentsSettings). If only one is needed, remove the duplicate.
🐛 Option A — rename the new entry to a unique key (if a separate settings-nav metadata entry is needed)
- "customAgents": {
- "title": "Custom agents",
- "description": "Manage custom AI agents for your organization."
- },
+ "customAgentsSettings": {
+ "title": "Custom agents",
+ "description": "Manage custom AI agents for your organization."
+ },🐛 Option B — remove the new entry and update the existing one if only one is needed
- "customAgents": {
- "title": "Custom agents",
- "description": "Manage custom AI agents for your organization."
- },
"integrations": {Then update the pre-existing entry at line 2908 to the preferred description, if needed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/messages/en.json` around lines 2868 - 2871, The metadata
object contains two identical keys named "customAgents" so the earlier one is
ignored; locate both "customAgents" entries and either (A) rename the newly
added entry to a unique key (e.g., "customAgentsSettings") wherever it's
referenced (update any lookup/usage accordingly) if you need a separate
settings-nav entry, or (B) remove the new duplicate and unify the description
into the existing "customAgents" entry by updating its "description" to the
preferred text; ensure all consumers/reference keys are updated to match the
chosen key.
1 participant
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Refactor