feat: add tab bar display with category icons and kubb regeneration#253
feat: add tab bar display with category icons and kubb regeneration#253
Conversation
📝 WalkthroughWalkthroughThis PR refactors the frontend API client typing from a custom Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes The scope spans 100+ files with heterogeneous changes across API hooks, schemas, types, and UI components. While many changes follow repetitive patterns (systematic Client→typeof fetch refactoring across 75+ files), the presence of new complex modules (activity feeds, category management, link parsing) with multiple variants (suspense, pagination), combined with type system changes affecting multiple layers and UI integration logic, creates moderate overall complexity. The clear patterns and documented migrations mitigate risk. Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
|
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
frontend/api/memberships/useJoinTripByInvite.ts (1)
54-65:⚠️ Potential issue | 🟠 MajorRestore the mutation context generic instead of using
typeof mutationKey.Line 64 passes
typeof mutationKeyas the 4th generic tomutationOptions. In@tanstack/react-queryv5.x, this parameter represents the mutation context type (the return value ofonMutate), not the mutation key. This incorrectly constrains context typing for consumers and breaks typed mutation lifecycle handlers.Proposed fix
-export function joinTripByInviteMutationOptions( +export function joinTripByInviteMutationOptions<TContext = unknown>( config: Partial<RequestConfig> & { client?: typeof fetch } = {}, ) { const mutationKey = joinTripByInviteMutationKey(); return mutationOptions< JoinTripByInviteMutationResponse, ResponseErrorConfig< JoinTripByInvite400 | JoinTripByInvite401 | JoinTripByInvite500 >, { code: JoinTripByInvitePathParams["code"] }, - typeof mutationKey + TContext >({ mutationKey, mutationFn: async ({ code }) => { return joinTripByInvite(code, config); }, }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/api/memberships/useJoinTripByInvite.ts` around lines 54 - 65, The fourth generic passed to mutationOptions is wrong: replace the incorrect typeof mutationKey used in joinTripByInviteMutationOptions with the proper mutation context type (e.g., unknown or a specific context interface) so the mutation context typing matches `@tanstack/react-query` v5 expectations; update the generics on mutationOptions in joinTripByInviteMutationOptions (keeping mutationKey, JoinTripByInviteMutationResponse, ResponseErrorConfig<...>, and the params type) so the 4th generic is the context type (unknown or your defined Context type) instead of typeof mutationKey.frontend/api/polls/useSubmitRanking.ts (1)
70-92:⚠️ Potential issue | 🟠 MajorFix the fourth TanStack Query generic in the mutation helper.
The fourth generic parameter of
mutationOptions/UseMutationOptionsin TanStack Query v5 is for the context type returned fromonMutate, not for the mutation key. Currently,submitRankingMutationOptionsincorrectly usestypeof mutationKeyas the fourth generic, which forces the consuming code inuseSubmitRankingto cast the result back to the correct type withTContext. The helper should acceptTContextas a generic parameter and pass it through instead.Since this file is generated by Kubb, the generator template needs the same fix:
Suggested fix
-export function submitRankingMutationOptions( +export function submitRankingMutationOptions<TContext = unknown>( config: Partial<RequestConfig<SubmitRankingMutationRequest>> & { client?: typeof fetch; } = {}, ) { const mutationKey = submitRankingMutationKey(); return mutationOptions< SubmitRankingMutationResponse, ResponseErrorConfig< | SubmitRanking400 | SubmitRanking401 | SubmitRanking403 | SubmitRanking404 | SubmitRanking422 | SubmitRanking500 >, { tripID: SubmitRankingPathParams["tripID"]; pollId: SubmitRankingPathParams["pollId"]; data: SubmitRankingMutationRequest; }, - typeof mutationKey + TContext >({ mutationKey, mutationFn: async ({ tripID, pollId, data }) => { return submitRanking(tripID, pollId, data, config); }, }); }- const baseOptions = submitRankingMutationOptions( - config, - ) as UseMutationOptions< - SubmitRankingMutationResponse, - ResponseErrorConfig< - | SubmitRanking400 - | SubmitRanking401 - | SubmitRanking403 - | SubmitRanking404 - | SubmitRanking422 - | SubmitRanking500 - >, - { - tripID: SubmitRankingPathParams["tripID"]; - pollId: SubmitRankingPathParams["pollId"]; - data: SubmitRankingMutationRequest; - }, - TContext - >; + const baseOptions = submitRankingMutationOptions<TContext>(config);Also applies to: 133-151
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/api/polls/useSubmitRanking.ts` around lines 70 - 92, The fourth generic for TanStack Query's mutationOptions in submitRankingMutationOptions is wrong: it's using typeof mutationKey (the mutation key) but should be the onMutate context type (TContext). Update submitRankingMutationOptions to accept a generic TContext (default unknown) and pass that TContext as the fourth generic to mutationOptions/UseMutationOptions instead of typeof mutationKey; also update the generator/template used by Kubb so other helpers (including the similar block at 133-151) follow the same pattern so callers like useSubmitRanking no longer need to cast the result to TContext.frontend/api/comments/useCreateComment.ts (1)
71-82:⚠️ Potential issue | 🟠 MajorThe fourth
mutationOptionsgeneric should not be the mutation key type.TanStack Query v5 documents the fourth generic as
TOnMutateResult—the return type of theonMutatecallback, which is passed as context toonSuccess,onError, andonSettled. Passingtypeof mutationKeyin this position exports an incorrect callback contract fromcreateCommentMutationOptions. Consumers that implement optimistic-update callbacks will encounter type mismatches between theirTContextand the mutation key tuple type.The same issue exists in
frontend/api/polls/useCreatePoll.ts.Proposed fix
return mutationOptions< CreateCommentMutationResponse, ResponseErrorConfig< | CreateComment400 | CreateComment401 | CreateComment404 | CreateComment422 | CreateComment500 >, { data: CreateCommentMutationRequest }, - typeof mutationKey + unknown >({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/api/comments/useCreateComment.ts` around lines 71 - 82, The fourth generic parameter passed to mutationOptions in createCommentMutationOptions is incorrectly set to typeof mutationKey (it should represent the onMutate return/context type), so change that fourth generic from typeof mutationKey to an appropriate context type such as unknown (or void) so optimistic-update callbacks get the correct TOnMutateResult type; update the same pattern in the analogous createPoll mutation options (look for mutationOptions<...> usages in createCommentMutationOptions and createPollMutationOptions and replace the fourth generic accordingly).frontend/api/users/useGetCurrentUser.ts (1)
47-63:⚠️ Potential issue | 🟠 MajorPreserve caller-provided abort signals.
Line 61 now overwrites any
config.signalpassed through the exportedclientoption and mutates the caller’s config object in place. That makes manual cancellation ineffective for consumers that rely on their own signal. Build a fresh config instead and only fall back to the React Query signal when the caller did not provide one. This same template change is repeated in the other regenerated query helpers in this PR.Proposed fix
return queryOptions< GetCurrentUserQueryResponse, ResponseErrorConfig< GetCurrentUser401 | GetCurrentUser404 | GetCurrentUser500 >, GetCurrentUserQueryResponse, typeof queryKey >({ queryKey, queryFn: async ({ signal }) => { - config.signal = signal; - return getCurrentUser(config); + return getCurrentUser({ + ...config, + signal: config.signal ?? signal, + }); }, }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/api/users/useGetCurrentUser.ts` around lines 47 - 63, The getCurrentUserQueryOptions function currently mutates the caller's config by assigning config.signal = signal; instead create a fresh local config (e.g., newConfig) that spreads the caller-provided config and sets signal only when config.signal is undefined (newConfig = { ...config, signal: config.signal ?? signal }), and pass newConfig to getCurrentUser; do not mutate the original config object. Apply the same pattern to the other regenerated query helper functions that set config.signal in their queryFn (look for queryFn: async ({ signal }) => { config.signal = signal; ... }).frontend/api/files/useConfirmUpload.ts (1)
60-73:⚠️ Potential issue | 🟠 MajorAdd
TContextgeneric toconfirmUploadMutationOptions.The 4th generic passed to
mutationOptionsmust be the mutation context type (TContext), not the mutation key. Currently,typeof mutationKeyis passed, which is a tuple type—this violates TanStack Query v5'sUseMutationOptionscontract. Downstream code works around this with type casts, hiding the mismatch. Since this function is exported viafrontend/index.ts, consumers using it directly will face type issues.Proposed fix
-export function confirmUploadMutationOptions( +export function confirmUploadMutationOptions<TContext = unknown>( config: Partial<RequestConfig<ConfirmUploadMutationRequest>> & { client?: typeof fetch; } = {}, ) { const mutationKey = confirmUploadMutationKey(); return mutationOptions< ConfirmUploadMutationResponse, ResponseErrorConfig< ConfirmUpload400 | ConfirmUpload404 | ConfirmUpload422 | ConfirmUpload500 >, { data: ConfirmUploadMutationRequest }, - typeof mutationKey + TContext >({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/api/files/useConfirmUpload.ts` around lines 60 - 73, The fourth generic on the call to mutationOptions in confirmUploadMutationOptions is wrong: it currently passes typeof mutationKey (the mutation key tuple) but should be the mutation context type TContext. Update the confirmUploadMutationOptions signature to introduce a generic TContext (e.g., <TContext = unknown>) and pass that TContext as the fourth generic to mutationOptions instead of typeof mutationKey; keep mutationKey as the mutationKey variable passed into mutationOptions. Ensure callers and the exported symbol confirmUploadMutationOptions keep the new generic so downstream code no longer needs casts.frontend/api/comments/useUpdateComment.ts (1)
67-87:⚠️ Potential issue | 🟠 MajorFix the fourth
mutationOptionsgeneric parameter.In TanStack Query v5, the fourth generic parameter is
TOnMutateResult—the type returned by theonMutatecallback. The current code passestypeof mutationKey, which is incorrect. This changes the exported helper's public type signature and breaks type safety for any consumer that callsupdateCommentMutationOptionsdirectly.Add
TContextas a generic parameter toupdateCommentMutationOptionsand pass it as the fourth generic argument tomutationOptions. This aligns with the pattern used inuseUpdateComment<TContext>.Suggested fix
-export function updateCommentMutationOptions( +export function updateCommentMutationOptions<TContext = unknown>( config: Partial<RequestConfig<UpdateCommentMutationRequest>> & { client?: typeof fetch; } = {}, ) { const mutationKey = updateCommentMutationKey(); return mutationOptions< UpdateCommentMutationResponse, ResponseErrorConfig< | UpdateComment400 | UpdateComment401 | UpdateComment404 | UpdateComment422 | UpdateComment500 >, { commentID: UpdateCommentPathParams["commentID"]; data: UpdateCommentMutationRequest; }, - typeof mutationKey + TContext >({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/api/comments/useUpdateComment.ts` around lines 67 - 87, The exported updateCommentMutationOptions function currently uses typeof mutationKey as the fourth generic to mutationOptions; change its signature to be generic over TContext (e.g. updateCommentMutationOptions<TContext = unknown>) and pass that TContext as the fourth generic parameter to mutationOptions (instead of typeof mutationKey), matching the pattern used by useUpdateComment<TContext> and restoring correct TOnMutateResult typing for the onMutate callback; update any related type annotations in the function signature to include the new TContext generic.frontend/api/places/useGetPlaceDetails.ts (1)
33-54:⚠️ Potential issue | 🔴 CriticalDo not make the entire request body optional.
The
dataparameter is now optional, allowinggetPlaceDetails()to be called without arguments. However, the API contract states "provide either place_id or input," meaning at least one of these fields is required. An empty request body will trigger a 400 error at runtime. Either keepdatarequired or add validation to enforce that at least one ofplace_idorinputis provided.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/api/places/useGetPlaceDetails.ts` around lines 33 - 54, The getPlaceDetails function currently allows an undefined request body which violates the API contract requiring either place_id or input; fix by making the data parameter required or adding validation: in getPlaceDetails (and its parameter signature) ensure data is non-optional (remove the ? on data: GetPlaceDetailsMutationRequest) or, if you want to keep the optional signature, add a runtime check at the top of getPlaceDetails that verifies data exists and that either data.place_id or data.input is present and otherwise throw or reject with a clear error before calling request; update callers/tests accordingly.
♻️ Duplicate comments (1)
frontend/api/files/useGetFileSuspense.ts (1)
76-79:⚠️ Potential issue | 🟡 MinorSame signal mutation concern applies here.
This has the same issue as
useGetFile.ts: the sharedconfigobject is mutated directly. The same fix (spreading config with the signal) should be applied in the Kubb template.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/api/files/useGetFileSuspense.ts` around lines 76 - 79, In useGetFileSuspense's queryFn avoid mutating the shared config by not assigning config.signal = signal; instead call getFileSuspense with a new config object that spreads the existing config and adds the signal (e.g., pass { ...config, signal }) so the original config is unchanged; update the Kubb template the same way to ensure consistency between useGetFileSuspense and getFileSuspense.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/api/activities/useGetActivity.ts`:
- Around line 88-90: The query hook mutates the shared config by assigning
signal directly (in queryFn), overwriting caller-provided signals; to fix,
create a new config object (e.g., const merged = { ...config, signal:
config.signal ?? signal }) and pass that to getActivity(tripID, activityID,
merged) so you never mutate the original config and you preserve any
caller-supplied signal; update the same pattern in the generated query/suspense
templates that produce queryFn.
In `@frontend/api/activities/useGetActivityCategoriesSuspense.ts`:
- Around line 100-102: The queryFn is mutating the caller-owned config by
assigning config.signal = signal; instead create a per-request config object and
pass that into getActivityCategoriesSuspense (e.g., build requestConfig = {
...config, signal } and call getActivityCategoriesSuspense(tripID, activityID,
params, requestConfig)); update queryFn in useGetActivityCategoriesSuspense and
replicate the same non-mutating fix for the other regenerated suspense hooks
that use the same unconditional assignment pattern.
In `@frontend/api/categories/useCreateCategory.ts`:
- Around line 67-92: The mutation option factory createCategoryMutationOptions
incorrectly binds the 4th generic parameter of mutationOptions to typeof
mutationKey; change that 4th generic (the TContext parameter) to unknown (or
remove it) so the signature reads
mutationOptions<CreateCategoryMutationResponse, ResponseErrorConfig<...>,
{tripID: CreateCategoryPathParams["tripID"]; data:
CreateCategoryMutationRequest}, unknown> and regenerate/update the other
generated mutation option functions in frontend/api/ to use unknown for the 4th
generic so consuming onMutate callbacks can infer their own context types;
ensure you update the mutationOptions invocations (and any casts) accordingly
rather than using typeof mutationKey for TContext.
In `@frontend/api/comments/useGetPaginatedComments.ts`:
- Around line 104-106: The queryFn in useGetPaginatedComments currently mutates
the captured config by assigning config.signal = signal which can leak signals
across invocations; change the call to create a new config object that preserves
caller-provided values and injects the per-invocation signal instead (e.g., pass
{ ...config, signal } to getPaginatedComments) so you do not mutate the outer
config or discard an existing signal/timeout; update the queryFn where config is
referenced (in useGetPaginatedComments and the other generated hooks like
useGetMembership) to follow this pattern.
In `@frontend/api/files/useGetFileAllSizes.ts`:
- Around line 67-69: The queryFn mutates the external config by setting
config.signal = signal which can leak cancellation signals; instead create a
derived config object (e.g., const derived = { ...config, signal } or
Object.assign({}, config, { signal })) and pass that to getFileAllSizes(imageId,
derived) so the original config is not modified; update the queryFn in
useGetFileAllSizes to use the derived config and remove any direct assignments
to config.signal.
In `@frontend/api/files/useGetFileAllSizesSuspense.ts`:
- Around line 67-69: The queryFn currently mutates the caller-provided config by
assigning config.signal = signal; update the Kubb generator template so it
produces a non-mutating merge like using a spread with nullish coalescing (e.g.,
pass { ...config, signal: config.signal ?? signal } into
getFileAllSizesSuspense) instead of direct assignment; change the template that
generates queryFn (and equivalent hooks across the generator) so all hooks use
this pattern, ensuring no in-place mutation of the incoming config while
preserving any existing signal.
In `@frontend/api/polls/useUpdatePoll.ts`:
- Around line 39-42: The PATCH request body was made optional causing callers to
be able to call mutate({ tripID, pollId }) with no UpdatePollMutationRequest and
produce invalid/no-op PATCHes; restore the body as required by changing the
function signature and all related mutation variable shapes so data:
UpdatePollMutationRequest (not optional) in the request params and in the
UseMutationOptions/UseMutationResult generics/variable types referenced in this
file (e.g. the update function signature that currently reads data?:
UpdatePollMutationRequest, the RequestConfig generic, and the
UseMutationOptions/UseMutationResult shapes further down around the referenced
blocks) so callers must supply a valid body for the PATCH.
In `@frontend/api/search/useSearchTripMembers.ts`:
- Around line 90-92: The queryFn currently mutates the passed-in config by
assigning config.signal = signal; instead, avoid side effects by creating a new
config object and passing that to searchTripMembers — e.g., construct const
reqConfig = { ...config, signal } (or Object.assign({}, config, { signal }))
inside queryFn and call searchTripMembers(tripID, params, reqConfig); this
preserves the original config for use elsewhere and works with frozen objects;
update the queryFn in useSearchTripMembers accordingly.
In `@frontend/api/search/useSearchTrips.ts`:
- Around line 65-67: In useSearchTrips's queryFn, don't unconditionally
overwrite the caller's RequestConfig.signal; instead create a new config object
that preserves any existing config.signal and only uses TanStack's signal when
none was provided (e.g., newConfig = { ...config, signal: config.signal ??
signal }) and pass that newConfig into searchTrips; avoid mutating the incoming
config parameter.
In `@frontend/api/users/useDeleteUser.ts`:
- Around line 46-55: The mutationOptions call in deleteUserMutationOptions
incorrectly uses typeof mutationKey as the fourth generic (which in TanStack
Query v5 is the onMutate return type); update the generic list for
mutationOptions in deleteUserMutationOptions to omit the fourth generic or
replace it with unknown so the exported helper is correctly typed for consumers
that pass onMutate (refer to mutationOptions, deleteUserMutationOptions,
deleteUserMutationKey, and onMutate).
In `@frontend/app/`(app)/trips/[id]/components/tabs/tab-bar.tsx:
- Around line 70-74: Replace the inline numeric spacing in the TabBar
component's style props with the design-system spacing tokens: swap gap: 6,
paddingHorizontal: 12, paddingVertical: 8 (and the other occurrence at the later
style block) for the semantic Box/spacing props or token values (e.g., gap="sm"
or gap="xs", paddingHorizontal="md", paddingVertical="sm") using the centralized
spacing tokens exposed by your Box/design system; update the JSX to use those
named props (or token constants) instead of hard-coded numbers so the Box
component's spacing API (e.g., gap, px, py or paddingHorizontal/paddingVertical
with tokens) is used consistently across tab-bar.tsx.
In `@frontend/app/`(app)/trips/[id]/index.tsx:
- Around line 23-27: The code currently treats a failed
useGetCategoriesByTripID(tripID) fetch as an empty categories array
(categoryData?.categories ?? []) and never surfaces errors; update the component
to read the query's isError and error (from useGetCategoriesByTripID) and render
an ErrorState UI (user-friendly message, not raw API error) when isError is
true, render LoadingState when categoriesLoading is true, and only derive
categories and initialize selectedTab/activeTab after a successful fetch (use
categoryData.categories when available) so a failed request is distinguishable
from a true empty-state; ensure you reference useGetCategoriesByTripID,
categoryData, categoriesLoading, isError/error, selectedTab and activeTab when
making these changes.
In `@frontend/app/`(app)/trips/[id]/polls/components/step-settings.tsx:
- Around line 102-112: The code is using cat.name defaulting to "" which can
cause key collisions and wrong selection logic; update the rendering logic in
step-settings.tsx to derive a stable identifier (e.g., prefer cat.id if present,
otherwise cat.name, otherwise cat.label) and use that identifier for key, for
selection checks (categories.includes(identifier)), and when calling
toggleCategory(identifier); if no stable identifier exists, skip rendering that
category (do not render a Chip). Keep getCategoryIcon(name) using the real name
when available (fall back to label only for display), but ensure
key/selection/toggle consistently use the same stable identifier (refer to
cat.name, cat.id, label, getCategoryIcon, Chip, categories.includes,
toggleCategory).
In `@frontend/schemas/modelsParseLinkRequest.json`:
- Around line 3-4: The request schema for modelsParseLinkRequest.json currently
declares "url" as required but allows empty or non-URL values; update the "url"
property to include validation constraints such as "minLength": 1 and "format":
"uri" (or "uri-reference" if you accept relative URLs) so the "url" property
(key "url") enforces non-empty valid URIs and rejects malformed payloads before
reaching the API layer.
In `@frontend/utilities/category-icons.ts`:
- Around line 46-49: The fallback hashing uses the raw name while the lookup
uses name.toLowerCase(), causing inconsistent icons for equivalent names; in
getCategoryIcon normalize the input once (e.g., const key =
name.trim().toLowerCase()) and use that key for both CATEGORY_ICONS lookup and
for computing hashStr(key) when indexing RANDOM_ICON_POOL (ensuring the modulo
uses RANDOM_ICON_POOL.length).
---
Outside diff comments:
In `@frontend/api/comments/useCreateComment.ts`:
- Around line 71-82: The fourth generic parameter passed to mutationOptions in
createCommentMutationOptions is incorrectly set to typeof mutationKey (it should
represent the onMutate return/context type), so change that fourth generic from
typeof mutationKey to an appropriate context type such as unknown (or void) so
optimistic-update callbacks get the correct TOnMutateResult type; update the
same pattern in the analogous createPoll mutation options (look for
mutationOptions<...> usages in createCommentMutationOptions and
createPollMutationOptions and replace the fourth generic accordingly).
In `@frontend/api/comments/useUpdateComment.ts`:
- Around line 67-87: The exported updateCommentMutationOptions function
currently uses typeof mutationKey as the fourth generic to mutationOptions;
change its signature to be generic over TContext (e.g.
updateCommentMutationOptions<TContext = unknown>) and pass that TContext as the
fourth generic parameter to mutationOptions (instead of typeof mutationKey),
matching the pattern used by useUpdateComment<TContext> and restoring correct
TOnMutateResult typing for the onMutate callback; update any related type
annotations in the function signature to include the new TContext generic.
In `@frontend/api/files/useConfirmUpload.ts`:
- Around line 60-73: The fourth generic on the call to mutationOptions in
confirmUploadMutationOptions is wrong: it currently passes typeof mutationKey
(the mutation key tuple) but should be the mutation context type TContext.
Update the confirmUploadMutationOptions signature to introduce a generic
TContext (e.g., <TContext = unknown>) and pass that TContext as the fourth
generic to mutationOptions instead of typeof mutationKey; keep mutationKey as
the mutationKey variable passed into mutationOptions. Ensure callers and the
exported symbol confirmUploadMutationOptions keep the new generic so downstream
code no longer needs casts.
In `@frontend/api/memberships/useJoinTripByInvite.ts`:
- Around line 54-65: The fourth generic passed to mutationOptions is wrong:
replace the incorrect typeof mutationKey used in joinTripByInviteMutationOptions
with the proper mutation context type (e.g., unknown or a specific context
interface) so the mutation context typing matches `@tanstack/react-query` v5
expectations; update the generics on mutationOptions in
joinTripByInviteMutationOptions (keeping mutationKey,
JoinTripByInviteMutationResponse, ResponseErrorConfig<...>, and the params type)
so the 4th generic is the context type (unknown or your defined Context type)
instead of typeof mutationKey.
In `@frontend/api/places/useGetPlaceDetails.ts`:
- Around line 33-54: The getPlaceDetails function currently allows an undefined
request body which violates the API contract requiring either place_id or input;
fix by making the data parameter required or adding validation: in
getPlaceDetails (and its parameter signature) ensure data is non-optional
(remove the ? on data: GetPlaceDetailsMutationRequest) or, if you want to keep
the optional signature, add a runtime check at the top of getPlaceDetails that
verifies data exists and that either data.place_id or data.input is present and
otherwise throw or reject with a clear error before calling request; update
callers/tests accordingly.
In `@frontend/api/polls/useSubmitRanking.ts`:
- Around line 70-92: The fourth generic for TanStack Query's mutationOptions in
submitRankingMutationOptions is wrong: it's using typeof mutationKey (the
mutation key) but should be the onMutate context type (TContext). Update
submitRankingMutationOptions to accept a generic TContext (default unknown) and
pass that TContext as the fourth generic to mutationOptions/UseMutationOptions
instead of typeof mutationKey; also update the generator/template used by Kubb
so other helpers (including the similar block at 133-151) follow the same
pattern so callers like useSubmitRanking no longer need to cast the result to
TContext.
In `@frontend/api/users/useGetCurrentUser.ts`:
- Around line 47-63: The getCurrentUserQueryOptions function currently mutates
the caller's config by assigning config.signal = signal; instead create a fresh
local config (e.g., newConfig) that spreads the caller-provided config and sets
signal only when config.signal is undefined (newConfig = { ...config, signal:
config.signal ?? signal }), and pass newConfig to getCurrentUser; do not mutate
the original config object. Apply the same pattern to the other regenerated
query helper functions that set config.signal in their queryFn (look for
queryFn: async ({ signal }) => { config.signal = signal; ... }).
---
Duplicate comments:
In `@frontend/api/files/useGetFileSuspense.ts`:
- Around line 76-79: In useGetFileSuspense's queryFn avoid mutating the shared
config by not assigning config.signal = signal; instead call getFileSuspense
with a new config object that spreads the existing config and adds the signal
(e.g., pass { ...config, signal }) so the original config is unchanged; update
the Kubb template the same way to ensure consistency between useGetFileSuspense
and getFileSuspense.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 51876a7c-262e-4abd-ae7c-ac465cb14433
📒 Files selected for processing (130)
frontend/api/activities/useAddCategoryToActivity.tsfrontend/api/activities/useCreateActivity.tsfrontend/api/activities/useDeleteActivity.tsfrontend/api/activities/useGetActivitiesByTripID.tsfrontend/api/activities/useGetActivitiesByTripIDSuspense.tsfrontend/api/activities/useGetActivity.tsfrontend/api/activities/useGetActivityCategories.tsfrontend/api/activities/useGetActivityCategoriesSuspense.tsfrontend/api/activities/useGetActivitySuspense.tsfrontend/api/activities/useGetApiV1TripsTripidActivitiesActivityidRsvps.tsfrontend/api/activities/useGetApiV1TripsTripidActivitiesActivityidRsvpsSuspense.tsfrontend/api/activities/useParseActivityLink.tsfrontend/api/activities/usePostApiV1TripsTripidActivitiesActivityidRsvp.tsfrontend/api/activities/useRemoveCategoryFromActivity.tsfrontend/api/activities/useUpdateActivity.tsfrontend/api/activity-feed/useGetTripActivityFeed.tsfrontend/api/activity-feed/useGetTripActivityFeedSuspense.tsfrontend/api/activity-feed/useGetUnreadActivityCount.tsfrontend/api/activity-feed/useGetUnreadActivityCountSuspense.tsfrontend/api/activity-feed/useMarkActivityEventRead.tsfrontend/api/categories/useCreateCategory.tsfrontend/api/categories/useDeleteCategory.tsfrontend/api/categories/useGetCategoriesByTripID.tsfrontend/api/categories/useGetCategoriesByTripIDSuspense.tsfrontend/api/categories/useHideCategory.tsfrontend/api/categories/useShowCategory.tsfrontend/api/comments/useCreateComment.tsfrontend/api/comments/useDeleteComment.tsfrontend/api/comments/useGetPaginatedComments.tsfrontend/api/comments/useGetPaginatedCommentsSuspense.tsfrontend/api/comments/useUpdateComment.tsfrontend/api/example/useHealthcheck.tsfrontend/api/example/useHealthcheckSuspense.tsfrontend/api/files/useCheckS3Health.tsfrontend/api/files/useCheckS3HealthSuspense.tsfrontend/api/files/useConfirmUpload.tsfrontend/api/files/useCreateUploadURLs.tsfrontend/api/files/useDeleteImage.tsfrontend/api/files/useGetFile.tsfrontend/api/files/useGetFileAllSizes.tsfrontend/api/files/useGetFileAllSizesSuspense.tsfrontend/api/files/useGetFileSuspense.tsfrontend/api/index.tsfrontend/api/memberships/useAddMember.tsfrontend/api/memberships/useDemoteFromAdmin.tsfrontend/api/memberships/useGetMembership.tsfrontend/api/memberships/useGetMembershipSuspense.tsfrontend/api/memberships/useGetTripMembers.tsfrontend/api/memberships/useGetTripMembersSuspense.tsfrontend/api/memberships/useJoinTripByInvite.tsfrontend/api/memberships/usePromoteToAdmin.tsfrontend/api/memberships/useRemoveMember.tsfrontend/api/memberships/useUpdateMembership.tsfrontend/api/notifications/useSendBulkNotification.tsfrontend/api/notifications/useSendNotification.tsfrontend/api/pitches/useCreatePitch.tsfrontend/api/pitches/useDeletePitch.tsfrontend/api/pitches/useGetPitch.tsfrontend/api/pitches/useGetPitchSuspense.tsfrontend/api/pitches/useListPitches.tsfrontend/api/pitches/useListPitchesSuspense.tsfrontend/api/pitches/useUpdatePitch.tsfrontend/api/places/useGetPlaceDetails.tsfrontend/api/places/useGoogleMapsHealth.tsfrontend/api/places/useGoogleMapsHealthSuspense.tsfrontend/api/places/useTypeaheadPlaces.tsfrontend/api/places/useTypeaheadPlacesSuspense.tsfrontend/api/polls/useAddPollOption.tsfrontend/api/polls/useAddRankPollOption.tsfrontend/api/polls/useCastVote.tsfrontend/api/polls/useCreatePoll.tsfrontend/api/polls/useCreateRankPoll.tsfrontend/api/polls/useDeletePoll.tsfrontend/api/polls/useDeletePollOption.tsfrontend/api/polls/useDeleteRankPoll.tsfrontend/api/polls/useDeleteRankPollOption.tsfrontend/api/polls/useGetPoll.tsfrontend/api/polls/useGetPollSuspense.tsfrontend/api/polls/useGetPollsByTripID.tsfrontend/api/polls/useGetPollsByTripIDSuspense.tsfrontend/api/polls/useGetRankPollResults.tsfrontend/api/polls/useGetRankPollResultsSuspense.tsfrontend/api/polls/useGetRankPollVoters.tsfrontend/api/polls/useGetRankPollVotersSuspense.tsfrontend/api/polls/useSubmitRanking.tsfrontend/api/polls/useUpdatePoll.tsfrontend/api/polls/useUpdateRankPoll.tsfrontend/api/search/useSearchActivities.tsfrontend/api/search/useSearchActivitiesSuspense.tsfrontend/api/search/useSearchTripMembers.tsfrontend/api/search/useSearchTripMembersSuspense.tsfrontend/api/search/useSearchTrips.tsfrontend/api/search/useSearchTripsSuspense.tsfrontend/api/trips/useCreateTrip.tsfrontend/api/trips/useCreateTripInvite.tsfrontend/api/trips/useDeleteTrip.tsfrontend/api/trips/useGetAllTrips.tsfrontend/api/trips/useGetAllTripsSuspense.tsfrontend/api/trips/useGetTrip.tsfrontend/api/trips/useGetTripSuspense.tsfrontend/api/trips/useUpdateTrip.tsfrontend/api/users/useCreateUser.tsfrontend/api/users/useDeleteUser.tsfrontend/api/users/useGetCurrentUser.tsfrontend/api/users/useGetCurrentUserSuspense.tsfrontend/api/users/useGetUser.tsfrontend/api/users/useGetUserSuspense.tsfrontend/api/users/useUpdateUser.tsfrontend/app/(app)/trips/[id]/components/tabs/tab-bar.tsxfrontend/app/(app)/trips/[id]/index.tsxfrontend/app/(app)/trips/[id]/polls/components/step-review.tsxfrontend/app/(app)/trips/[id]/polls/components/step-settings.tsxfrontend/schemas/modelsCategoryAPIResponse.jsonfrontend/schemas/modelsCategoryListResponse.jsonfrontend/schemas/modelsCreateCategoryRequest.jsonfrontend/schemas/modelsCreatePitchRequest.jsonfrontend/schemas/modelsCreatePitchResponse.jsonfrontend/schemas/modelsLinkType.jsonfrontend/schemas/modelsParseLinkRequest.jsonfrontend/schemas/modelsParsedActivityData.jsonfrontend/schemas/modelsPitchAPIResponse.jsonfrontend/schemas/modelsPitchCursorPageResult.jsonfrontend/schemas/modelsPitchImageInfo.jsonfrontend/schemas/modelsUpdatePitchRequest.jsonfrontend/schemas/realtimeEvent.jsonfrontend/schemas/realtimeUnreadCountResponse.jsonfrontend/types/schema.gen.tsfrontend/types/types.gen.tsfrontend/utilities/category-icons.tsfrontend/utilities/string.ts
| queryFn: async ({ signal }) => { | ||
| if (!config.signal) { | ||
| config.signal = signal; | ||
| } | ||
| config.signal = signal; | ||
| return getActivity(tripID, activityID, config); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f "useGetActivity.ts" --type fileRepository: GenerateNU/toggo
Length of output: 101
🏁 Script executed:
cat -n frontend/api/activities/useGetActivity.ts | head -120Repository: GenerateNU/toggo
Length of output: 4002
🏁 Script executed:
rg "queryFn.*signal" frontend/api/activities/ -A 3 -B 1Repository: GenerateNU/toggo
Length of output: 3694
🏁 Script executed:
rg "export.*RequestConfig" frontend/api/ -t tsRepository: GenerateNU/toggo
Length of output: 129
🏁 Script executed:
cat -n frontend/api/client.ts | grep -A 20 "export type RequestConfig"Repository: GenerateNU/toggo
Length of output: 808
🏁 Script executed:
rg "signal.*AbortSignal|AbortSignal.*signal" frontend/api/client.tsRepository: GenerateNU/toggo
Length of output: 83
🏁 Script executed:
git log --oneline -20 -- frontend/api/activities/useGetActivity.ts 2>/dev/null | head -10 || echo "Git history not available"Repository: GenerateNU/toggo
Length of output: 135
Fix signal mutation in generated query hooks.
Lines 89-90 unconditionally overwrite any caller-supplied abort signal and mutate the shared config object in place. This is a breaking change for the public API, as getActivity() still accepts Partial<RequestConfig> and callers may provide their own signal.
Create a fresh config object with spread syntax and preserve both signals using nullish coalescing:
Suggested fix
queryFn: async ({ signal }) => {
- config.signal = signal;
- return getActivity(tripID, activityID, config);
+ return getActivity(tripID, activityID, {
+ ...config,
+ signal: config.signal ?? signal,
+ });
},This pattern appears in all generated query and suspense query files. Fix the Kubb template and regenerate.
📝 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.
| queryFn: async ({ signal }) => { | |
| if (!config.signal) { | |
| config.signal = signal; | |
| } | |
| config.signal = signal; | |
| return getActivity(tripID, activityID, config); | |
| queryFn: async ({ signal }) => { | |
| return getActivity(tripID, activityID, { | |
| ...config, | |
| signal: config.signal ?? signal, | |
| }); | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/api/activities/useGetActivity.ts` around lines 88 - 90, The query
hook mutates the shared config by assigning signal directly (in queryFn),
overwriting caller-provided signals; to fix, create a new config object (e.g.,
const merged = { ...config, signal: config.signal ?? signal }) and pass that to
getActivity(tripID, activityID, merged) so you never mutate the original config
and you preserve any caller-supplied signal; update the same pattern in the
generated query/suspense templates that produce queryFn.
| queryFn: async ({ signal }) => { | ||
| if (!config.signal) { | ||
| config.signal = signal; | ||
| } | ||
| config.signal = signal; | ||
| return getActivityCategoriesSuspense(tripID, activityID, params, config); |
There was a problem hiding this comment.
Do not mutate the caller's request config in queryFn.
Line 101 overwrites caller-owned config and silently replaces any signal they supplied. If that object is reused, later requests share mutable state. Build a per-request config instead. This same unconditional assignment pattern shows up in the other regenerated suspense hooks in this PR.
Proposed fix
queryFn: async ({ signal }) => {
- config.signal = signal;
- return getActivityCategoriesSuspense(tripID, activityID, params, config);
+ return getActivityCategoriesSuspense(tripID, activityID, params, {
+ ...config,
+ signal: config.signal ?? signal,
+ });
},📝 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.
| queryFn: async ({ signal }) => { | |
| if (!config.signal) { | |
| config.signal = signal; | |
| } | |
| config.signal = signal; | |
| return getActivityCategoriesSuspense(tripID, activityID, params, config); | |
| queryFn: async ({ signal }) => { | |
| return getActivityCategoriesSuspense(tripID, activityID, params, { | |
| ...config, | |
| signal: config.signal ?? signal, | |
| }); | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/api/activities/useGetActivityCategoriesSuspense.ts` around lines 100
- 102, The queryFn is mutating the caller-owned config by assigning
config.signal = signal; instead create a per-request config object and pass that
into getActivityCategoriesSuspense (e.g., build requestConfig = { ...config,
signal } and call getActivityCategoriesSuspense(tripID, activityID, params,
requestConfig)); update queryFn in useGetActivityCategoriesSuspense and
replicate the same non-mutating fix for the other regenerated suspense hooks
that use the same unconditional assignment pattern.
| export function createCategoryMutationOptions( | ||
| config: Partial<RequestConfig<CreateCategoryMutationRequest>> & { | ||
| client?: typeof fetch; | ||
| } = {}, | ||
| ) { | ||
| const mutationKey = createCategoryMutationKey(); | ||
| return mutationOptions< | ||
| CreateCategoryMutationResponse, | ||
| ResponseErrorConfig< | ||
| | CreateCategory400 | ||
| | CreateCategory401 | ||
| | CreateCategory403 | ||
| | CreateCategory404 | ||
| | CreateCategory500 | ||
| >, | ||
| { | ||
| tripID: CreateCategoryPathParams["tripID"]; | ||
| data: CreateCategoryMutationRequest; | ||
| }, | ||
| typeof mutationKey | ||
| >({ | ||
| mutationKey, | ||
| mutationFn: async ({ tripID, data }) => { | ||
| return createCategory(tripID, data, config); | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In @tanstack/react-queryv5.90.20, what do the generic parameters ofmutationOptionsandUseMutationOptionsrepresent? Is the 4th generic parameter the mutation key type, or the value returned fromonMutate / optimistic-update context?
💡 Result:
In @tanstack/react-query v5.90.20, the generic parameters of mutationOptions and UseMutationOptions are: 1. TData = unknown - The type of data returned by the mutation function 2. TError = DefaultError - The type of error that can be thrown by the mutation function 3. TVariables = void - The type of variables passed to the mutation function 4. TContext = unknown (also called TOnMutateResult) - The type of context returned by the onMutate callback (used for optimistic updates and passed to other callbacks like onSuccess, onError) The 4th generic parameter (TContext) is NOT the mutation key type. The mutation key is mutationKey?: MutationKey (where MutationKey is unknown[]) and is not a generic parameter of UseMutationOptions. It is the type of the value returned from onMutate / optimistic-update context.
Citations:
- 1: https://tanstack.com/query/v5/docs/framework/react/reference/useMutation
- 2: `UseMutationOptions` generic type variables TanStack/query#3581
- 3: https://www.mintlify.com/TanStack/query/api/react/mutation-options
- 4: https://tessl.io/registry/tessl/npm-tanstack--react-query/5.87.0/files/docs/mutations.md
- 5: https://github.com/TanStack/query/blob/main/packages/query-core/CHANGELOG.md
🏁 Script executed:
# Check if the file exists and review the actual code
fd -t f "useCreateCategory.ts"Repository: GenerateNU/toggo
Length of output: 104
🏁 Script executed:
cat -n frontend/api/categories/useCreateCategory.ts | sed -n '67,92p'Repository: GenerateNU/toggo
Length of output: 934
🏁 Script executed:
# Check if this pattern is widespread in other mutation hooks
rg -t ts "mutationOptions<" frontend/api/ -A 5 | grep -E "(mutationOptions|typeof.*Key)" | head -20Repository: GenerateNU/toggo
Length of output: 1356
🏁 Script executed:
# Check if any of these mutation options files actually use onMutate callbacks
rg -t ts "onMutate" frontend/api/ --context 3 | head -30Repository: GenerateNU/toggo
Length of output: 42
🏁 Script executed:
# Check the full structure of a couple of these mutation options to understand the pattern better
cat -n frontend/api/users/useCreateUser.ts | head -95 | tail -35Repository: GenerateNU/toggo
Length of output: 1404
🏁 Script executed:
# See the full useCreateUser hook implementation
cat -n frontend/api/users/useCreateUser.ts | sed -n '73,115p'Repository: GenerateNU/toggo
Length of output: 1680
Fix the 4th generic parameter in mutation option functions across all generated hooks.
The 4th generic parameter in mutationOptions<TData, TError, TVariables, TContext> represents the context type returned by the onMutate callback for optimistic updates. Binding it to typeof mutationKey (lines 73–86) is incorrect and breaks type safety when callers use onMutate callbacks.
This pattern appears across all generated mutation hooks in frontend/api/. The hooks then cast the base options to match caller-provided context types, which masks the type error. Regenerate these mutation option functions with the 4th generic parameter as unknown or remove it entirely to allow proper type inference from UseMutationOptions in consuming code.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/api/categories/useCreateCategory.ts` around lines 67 - 92, The
mutation option factory createCategoryMutationOptions incorrectly binds the 4th
generic parameter of mutationOptions to typeof mutationKey; change that 4th
generic (the TContext parameter) to unknown (or remove it) so the signature
reads mutationOptions<CreateCategoryMutationResponse, ResponseErrorConfig<...>,
{tripID: CreateCategoryPathParams["tripID"]; data:
CreateCategoryMutationRequest}, unknown> and regenerate/update the other
generated mutation option functions in frontend/api/ to use unknown for the 4th
generic so consuming onMutate callbacks can infer their own context types;
ensure you update the mutationOptions invocations (and any casts) accordingly
rather than using typeof mutationKey for TContext.
| queryFn: async ({ signal }) => { | ||
| if (!config.signal) { | ||
| config.signal = signal; | ||
| } | ||
| config.signal = signal; | ||
| return getPaginatedComments(tripID, entityType, entityID, params, config); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the target file to verify the code at lines 104-106
cat -n frontend/api/comments/useGetPaginatedComments.ts | sed -n '100,110p'Repository: GenerateNU/toggo
Length of output: 391
🏁 Script executed:
# Check if the file exists and get general structure
head -20 frontend/api/comments/useGetPaginatedComments.tsRepository: GenerateNU/toggo
Length of output: 551
🏁 Script executed:
# Also check the useGetMembership.ts file mentioned as having the same pattern
find . -name "useGetMembership.ts" -type fRepository: GenerateNU/toggo
Length of output: 106
🏁 Script executed:
# Check the useGetMembership.ts file for the same pattern
cat -n ./frontend/api/memberships/useGetMembership.ts | grep -A 5 "queryFn:"Repository: GenerateNU/toggo
Length of output: 235
🏁 Script executed:
# Get more context around the queryFn in useGetMembership.ts to see lines
cat -n ./frontend/api/memberships/useGetMembership.ts | sed -n '70,120p'Repository: GenerateNU/toggo
Length of output: 1869
🏁 Script executed:
# Let's also verify if this is a generated file
head -5 ./frontend/api/memberships/useGetMembership.tsRepository: GenerateNU/toggo
Length of output: 135
🏁 Script executed:
# Check the config object usage - where does it come from in useGetPaginatedComments.ts?
cat -n frontend/api/comments/useGetPaginatedComments.ts | sed -n '1,110p'Repository: GenerateNU/toggo
Length of output: 4016
🏁 Script executed:
# Let's check how the config object flows through the functions
# Looking at getPaginatedCommentsQueryOptions to see the full parameter context
cat -n frontend/api/comments/useGetPaginatedComments.ts | sed -n '77,109p'Repository: GenerateNU/toggo
Length of output: 1278
🏁 Script executed:
# Check RequestConfig type to understand what signal could be
rg "type RequestConfig" -A 10 frontend/api/clientRepository: GenerateNU/toggo
Length of output: 119
🏁 Script executed:
# Let's verify this is indeed a Kubb-generated template issue by checking if other hooks exist with similar patterns
find frontend/api -name "*.ts" -type f -exec grep -l "config.signal = signal" {} \;Repository: GenerateNU/toggo
Length of output: 2915
🏁 Script executed:
# Let's find the RequestConfig type definition
find frontend -name "*.ts" -o -name "*.tsx" | xargs grep -l "type RequestConfig"Repository: GenerateNU/toggo
Length of output: 82
🏁 Script executed:
# Count how many files have this pattern
find frontend/api -name "*.ts" -type f -exec grep -l "config.signal = signal" {} \; | wc -lRepository: GenerateNU/toggo
Length of output: 62
🏁 Script executed:
# Let's check how the config object is used - is it mutated for each call?
# Look at how the hook is consumed to understand if mutation is problematic
cat -n frontend/api/comments/useGetPaginatedComments.ts | sed -n '110,130p'Repository: GenerateNU/toggo
Length of output: 949
🏁 Script executed:
# Check if there's a useQuery hook that wraps this function
rg "useGetPaginatedComments|useGetMembership" --type ts -B 3 -A 3 frontend | head -50Repository: GenerateNU/toggo
Length of output: 4876
🏁 Script executed:
# Check the RequestConfig type definition
cat -n frontend/api/client.ts | head -50Repository: GenerateNU/toggo
Length of output: 1844
🏁 Script executed:
# Look for where useGetPaginatedComments is actually called/used in the codebase
rg "useGetPaginatedComments\(" frontend --type ts -B 2 -A 2 | head -40Repository: GenerateNU/toggo
Length of output: 42
🏁 Script executed:
# Check if config might come with a pre-existing signal
cat -n frontend/api/client.ts | sed -n '1,100p'Repository: GenerateNU/toggo
Length of output: 3640
Fix signal handling in generated query hooks to avoid mutation and signal leakage.
TanStack Query provides a unique AbortSignal to each queryFn invocation for cancellation. Directly assigning config.signal = signal overwrites any caller-supplied signal and mutates the captured config object across multiple query invocations. This can cause a signal from one request to leak into a later refetch, and it discards any timeout or custom cancellation logic the caller may have provided. Use a spread operator to derive a new config object instead.
Suggested fix
queryFn: async ({ signal }) => {
- config.signal = signal;
- return getPaginatedComments(tripID, entityType, entityID, params, config);
+ return getPaginatedComments(
+ tripID,
+ entityType,
+ entityID,
+ params,
+ { ...config, signal: config.signal ?? signal },
+ );
},This pattern appears in 60+ generated files (e.g., useGetMembership.ts). Applying this fix to the Kubb template will resolve the issue across all generated query hooks.
📝 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.
| queryFn: async ({ signal }) => { | |
| if (!config.signal) { | |
| config.signal = signal; | |
| } | |
| config.signal = signal; | |
| return getPaginatedComments(tripID, entityType, entityID, params, config); | |
| queryFn: async ({ signal }) => { | |
| return getPaginatedComments( | |
| tripID, | |
| entityType, | |
| entityID, | |
| params, | |
| { ...config, signal: config.signal ?? signal }, | |
| ); | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/api/comments/useGetPaginatedComments.ts` around lines 104 - 106, The
queryFn in useGetPaginatedComments currently mutates the captured config by
assigning config.signal = signal which can leak signals across invocations;
change the call to create a new config object that preserves caller-provided
values and injects the per-invocation signal instead (e.g., pass { ...config,
signal } to getPaginatedComments) so you do not mutate the outer config or
discard an existing signal/timeout; update the queryFn where config is
referenced (in useGetPaginatedComments and the other generated hooks like
useGetMembership) to follow this pattern.
| queryFn: async ({ signal }) => { | ||
| if (!config.signal) { | ||
| config.signal = signal; | ||
| } | ||
| config.signal = signal; | ||
| return getFileAllSizes(imageId, config); |
There was a problem hiding this comment.
Avoid mutating request config inside queryFn
Line 68 mutates the external config object (config.signal = signal). If the same config object is reused, signals can bleed across calls and cause incorrect cancellation behavior. Pass a derived object instead.
Proposed fix
queryFn: async ({ signal }) => {
- config.signal = signal;
- return getFileAllSizes(imageId, config);
+ return getFileAllSizes(imageId, {
+ ...config,
+ signal,
+ });
},📝 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.
| queryFn: async ({ signal }) => { | |
| if (!config.signal) { | |
| config.signal = signal; | |
| } | |
| config.signal = signal; | |
| return getFileAllSizes(imageId, config); | |
| queryFn: async ({ signal }) => { | |
| return getFileAllSizes(imageId, { | |
| ...config, | |
| signal, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/api/files/useGetFileAllSizes.ts` around lines 67 - 69, The queryFn
mutates the external config by setting config.signal = signal which can leak
cancellation signals; instead create a derived config object (e.g., const
derived = { ...config, signal } or Object.assign({}, config, { signal })) and
pass that to getFileAllSizes(imageId, derived) so the original config is not
modified; update the queryFn in useGetFileAllSizes to use the derived config and
remove any direct assignments to config.signal.
| style={{ | ||
| gap: 6, | ||
| paddingHorizontal: 12, | ||
| paddingVertical: 8, | ||
| }} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using design system spacing tokens instead of magic numbers.
Lines 70-74 and 98-101 use hard-coded padding and gap values (6, 8, 12). The design system's Box component already supports semantic spacing props like marginRight="md". If spacing tokens are available (e.g., xs, sm, md), prefer those over inline numeric values to maintain consistency with the centralized design system.
As per coding guidelines: "React/Expo UI must use a centralized design system; avoid hard-coded color values, magic spacing numbers, and duplicated style definitions."
Also applies to: 98-101
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/`(app)/trips/[id]/components/tabs/tab-bar.tsx around lines 70 -
74, Replace the inline numeric spacing in the TabBar component's style props
with the design-system spacing tokens: swap gap: 6, paddingHorizontal: 12,
paddingVertical: 8 (and the other occurrence at the later style block) for the
semantic Box/spacing props or token values (e.g., gap="sm" or gap="xs",
paddingHorizontal="md", paddingVertical="sm") using the centralized spacing
tokens exposed by your Box/design system; update the JSX to use those named
props (or token constants) instead of hard-coded numbers so the Box component's
spacing API (e.g., gap, px, py or paddingHorizontal/paddingVertical with tokens)
is used consistently across tab-bar.tsx.
| const { data: categoryData, isLoading: categoriesLoading } = | ||
| useGetCategoriesByTripID(tripID!); | ||
| const categories = categoryData?.categories ?? []; | ||
| const [selectedTab, setSelectedTab] = useState<string | null>(null); | ||
| const activeTab = selectedTab ?? categories.find((c) => !c.is_hidden)?.name ?? ""; |
There was a problem hiding this comment.
Handle category fetch failures with an explicit error UI state.
Line 23 reads query state, but errors are ignored and Line 25 silently falls back to []. That makes a failed request indistinguishable from a real empty state.
Proposed fix
- const { data: categoryData, isLoading: categoriesLoading } =
- useGetCategoriesByTripID(tripID!);
+ const {
+ data: categoryData,
+ isLoading: categoriesLoading,
+ isError: categoriesError,
+ } = useGetCategoriesByTripID(tripID!);
const categories = categoryData?.categories ?? [];- <Box paddingVertical="md">
- <TabBar
- categories={categories}
- activeTab={activeTab}
- onTabPress={setSelectedTab}
- onEditPress={() => {
- // TODO: open edit tabs modal
- }}
- loading={categoriesLoading}
- />
- </Box>
+ <Box paddingVertical="md">
+ {categoriesError ? (
+ <Text variant="xsParagraph" color="textQuaternary">
+ Unable to load categories. Please try again.
+ </Text>
+ ) : (
+ <TabBar
+ categories={categories}
+ activeTab={activeTab}
+ onTabPress={setSelectedTab}
+ onEditPress={() => {
+ // TODO: open edit tabs modal
+ }}
+ loading={categoriesLoading}
+ />
+ )}
+ </Box>As per coding guidelines, "React/Expo UI error handling must not expose raw API errors; use UI states (LoadingState, ErrorState, EmptyState); errors shown to users must be clear and user-friendly."
Also applies to: 79-89
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/`(app)/trips/[id]/index.tsx around lines 23 - 27, The code
currently treats a failed useGetCategoriesByTripID(tripID) fetch as an empty
categories array (categoryData?.categories ?? []) and never surfaces errors;
update the component to read the query's isError and error (from
useGetCategoriesByTripID) and render an ErrorState UI (user-friendly message,
not raw API error) when isError is true, render LoadingState when
categoriesLoading is true, and only derive categories and initialize
selectedTab/activeTab after a successful fetch (use categoryData.categories when
available) so a failed request is distinguishable from a true empty-state;
ensure you reference useGetCategoriesByTripID, categoryData, categoriesLoading,
isError/error, selectedTab and activeTab when making these changes.
| const name = cat.name ?? ""; | ||
| const label = cat.label ?? name; | ||
| const icon = getCategoryIcon(name); | ||
| return ( | ||
| <Chip | ||
| key={rawName} | ||
| key={name} | ||
| label={label} | ||
| icon={icon} | ||
| selected={categories.includes(rawName)} | ||
| onPress={() => toggleCategory(rawName)} | ||
| selected={categories.includes(name)} | ||
| onPress={() => toggleCategory(name)} | ||
| /> |
There was a problem hiding this comment.
Do not use empty-string fallback for category identity.
Line 102 defaults missing cat.name to "", then Line 107/110/111 reuse it as key and selection value. This can create key collisions and incorrect selection state updates.
Proposed fix
- {visibleCategories.map((cat) => {
- const name = cat.name ?? "";
+ {visibleCategories.map((cat) => {
+ if (!cat.name) return null;
+ const name = cat.name;
const label = cat.label ?? name;
const icon = getCategoryIcon(name);
return (
<Chip
key={name}
label={label}
icon={icon}
selected={categories.includes(name)}
onPress={() => toggleCategory(name)}
/>
);
})}📝 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.
| const name = cat.name ?? ""; | |
| const label = cat.label ?? name; | |
| const icon = getCategoryIcon(name); | |
| return ( | |
| <Chip | |
| key={rawName} | |
| key={name} | |
| label={label} | |
| icon={icon} | |
| selected={categories.includes(rawName)} | |
| onPress={() => toggleCategory(rawName)} | |
| selected={categories.includes(name)} | |
| onPress={() => toggleCategory(name)} | |
| /> | |
| if (!cat.name) return null; | |
| const name = cat.name; | |
| const label = cat.label ?? name; | |
| const icon = getCategoryIcon(name); | |
| return ( | |
| <Chip | |
| key={name} | |
| label={label} | |
| icon={icon} | |
| selected={categories.includes(name)} | |
| onPress={() => toggleCategory(name)} | |
| /> | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/`(app)/trips/[id]/polls/components/step-settings.tsx around
lines 102 - 112, The code is using cat.name defaulting to "" which can cause key
collisions and wrong selection logic; update the rendering logic in
step-settings.tsx to derive a stable identifier (e.g., prefer cat.id if present,
otherwise cat.name, otherwise cat.label) and use that identifier for key, for
selection checks (categories.includes(identifier)), and when calling
toggleCategory(identifier); if no stable identifier exists, skip rendering that
category (do not render a Chip). Keep getCategoryIcon(name) using the real name
when available (fall back to label only for display), but ensure
key/selection/toggle consistently use the same stable identifier (refer to
cat.name, cat.id, label, getCategoryIcon, Chip, categories.includes,
toggleCategory).
| "required": ["url"], | ||
| "properties": { "url": { "type": "string" } }, |
There was a problem hiding this comment.
Tighten url validation in request schema.
url is required but currently allows empty and non-URL values. Add minimum length and URI format constraints to prevent malformed payloads from reaching the API layer.
Proposed schema update
{
"type": "object",
"required": ["url"],
- "properties": { "url": { "type": "string" } },
+ "properties": {
+ "url": { "type": "string", "minLength": 1, "format": "uri" }
+ },
"x-readme-ref-name": "models.ParseLinkRequest"
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/schemas/modelsParseLinkRequest.json` around lines 3 - 4, The request
schema for modelsParseLinkRequest.json currently declares "url" as required but
allows empty or non-URL values; update the "url" property to include validation
constraints such as "minLength": 1 and "format": "uri" (or "uri-reference" if
you accept relative URLs) so the "url" property (key "url") enforces non-empty
valid URIs and rejects malformed payloads before reaching the API layer.
| export function getCategoryIcon(name: string): LucideIcon { | ||
| return ( | ||
| CATEGORY_ICONS[name.toLowerCase()] ?? | ||
| RANDOM_ICON_POOL[hashStr(name) % RANDOM_ICON_POOL.length]! |
There was a problem hiding this comment.
Normalize the fallback key before hashing.
Mapped icons use name.toLowerCase(), but fallback icons hash the raw string. Any category that misses CATEGORY_ICONS can therefore get different icons for "Food", "food", or " food ", which breaks deterministic icon selection across screens.
Proposed fix
export function getCategoryIcon(name: string): LucideIcon {
+ const normalizedName = name.trim().toLowerCase();
return (
- CATEGORY_ICONS[name.toLowerCase()] ??
- RANDOM_ICON_POOL[hashStr(name) % RANDOM_ICON_POOL.length]!
+ CATEGORY_ICONS[normalizedName] ??
+ RANDOM_ICON_POOL[hashStr(normalizedName) % RANDOM_ICON_POOL.length]!
);
}📝 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.
| export function getCategoryIcon(name: string): LucideIcon { | |
| return ( | |
| CATEGORY_ICONS[name.toLowerCase()] ?? | |
| RANDOM_ICON_POOL[hashStr(name) % RANDOM_ICON_POOL.length]! | |
| export function getCategoryIcon(name: string): LucideIcon { | |
| const normalizedName = name.trim().toLowerCase(); | |
| return ( | |
| CATEGORY_ICONS[normalizedName] ?? | |
| RANDOM_ICON_POOL[hashStr(normalizedName) % RANDOM_ICON_POOL.length]! | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/utilities/category-icons.ts` around lines 46 - 49, The fallback
hashing uses the raw name while the lookup uses name.toLowerCase(), causing
inconsistent icons for equivalent names; in getCategoryIcon normalize the input
once (e.g., const key = name.trim().toLowerCase()) and use that key for both
CATEGORY_ICONS lookup and for computing hashStr(key) when indexing
RANDOM_ICON_POOL (ensuring the modulo uses RANDOM_ICON_POOL.length).
in-mai-space
left a comment
There was a problem hiding this comment.
can you add a video recording of the frontend? sad you got merge conflicts again 😞 maybe just accept incoming for all the docs and regenerate kubb
Description
This PR implements the tab bar display for the trip detail screen, rendering a horizontally scrollable row of category pills sourced from the backend categories API. Each pill shows a Lucide icon and the category's
labelfield. The selected tab is highlighted with a black filled pill; unselected tabs use a light grey background. An edit button styled as an unselected pill sits at the far right of the tab bar to stub the future edit/reorder flow.All visual decisions were verified against Figma:
borderRadiusis 8px (sm), padding is 8px vertical and 12px horizontal, icon size is 14px for tab pills and 11px for the edit button (Settings2), and the unselected background color matches#F4F4F4(surfaceBackground).A skeleton loading state is shown while categories are fetching, using
SkeletonRectwith widths derived dynamically from the default category label lengths rather than hardcoded magic numbers.Refactoring:
getCategoryIconand its supporting hash/icon pool logic were duplicated acrosstab-bar.tsx,step-review.tsx, andstep-settings.tsx. These have been extracted toutilities/category-icons.ts. Similarly,toPascalCasewas duplicated across the same three files and has been extracted toutilities/string.ts. Bothstep-review.tsxandstep-settings.tsxhave been updated to import from the shared utilities.step-settings.tsxnow also usescategory.labeldirectly instead of transformingcategory.namewithtoPascalCase, which is the correct approach now that the backend provides a dedicated display label.Kubb was regenerated against the updated Swagger spec to pick up
label,is_default, andpositionfields onModelsCategoryAPIResponsefrom the backend category PR.Future Changes:
useReorderCategorieshook is generatedis_defaultshould be surfaced in the tab bar displayHow has this been tested?
Manual device testing was blocked by a Doppler authentication issue in the local environment. Will verify on device once resolved.
Checklist
User-Visible Changes
Changes by Category
Features
TabBarcomponent (frontend/app/(app)/trips/[id]/components/tabs/tab-bar.tsx) with category pill rendering and loading skeleton stateuseGetCategoriesByTripIDhookgetTripActivityFeed,getUnreadActivityCount,markActivityEventReadwith suspense variantscreateCategory,deleteCategoryfrontend/utilities/category-icons.tswith deterministic fallback selection via string hashingtoPascalCasestring utility tofrontend/utilities/string.tsImprovements
Clienttype withtypeof fetchfor narrower client typing across all activities, categories, comments, files, memberships, notifications, pitches, places, polls, search, trips, and users APIsconfig.signal = signalassignment (removed conditional guards)TContextgeneric parameters from mutation option builders; context type now derives fromtypeof mutationKeyInfra
label,is_default, andpositionfields toModelsCategoryAPIResponseModelsParseLinkRequest,ModelsParsedActivityData), pitch media (ModelsPitchImageInfo), realtime events (RealtimeEvent,RealtimeUnreadCountResponse), and category creation (ModelsCreateCategoryRequest)ModelsCreatePitchRequestandModelsUpdatePitchRequestwith optionalimage_idsfield (max 5 items)GetCategoriesByTripIDQueryParamswith optionalinclude_hiddenfilterUpdates to Existing Screens
frontend/app/(app)/trips/[id]/polls/components/step-review.tsx: removed local category icon logic, now importsgetCategoryIconfrom shared utilitiesfrontend/app/(app)/trips/[id]/polls/components/step-settings.tsx: removed local icon mapping andtoPascalCaseimplementation, updated to usecat.label ?? cat.namefor display and sharedgetCategoryIconutilityContribution Summary
Primary focus: 100+ API hook files refactored for type safety; new tab bar UI component and category icon/string utilities extracted for reuse across screens.