refactor(api): knowledge → src/api modules (Effort 1)

[chiptus]

Mechanical move of the knowledge feature from src/hooks/queries/knowledge to src/api/knowledge, adding queryOptions factories for the query hooks. No behavior change. Part of #52.

Manual verification

• No external consumers of this feature — zero repoint required.
• No route loaders use this feature.
• pnpm run lint passes (0 warnings, 0 errors).
• pnpm test passes (20 test files, 290 tests).
• pnpm run build passes.
• No references to hooks/queries/knowledge remain in src.

---

Generated by Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
upline	Ready	Preview, Comment	Jun 25, 2026 3:44am

[qodo-code-review]

PR Summary by Qodo

Refactor knowledge feature into src/api modules with queryOptions factories
✨ Enhancement 🕐 10-20 Minutes

Description

• Move the knowledge query/mutation hooks into src/api/knowledge for API-module consistency.
• Introduce queryOptions factories to reuse query configuration outside hooks.
• Split composite, query, and mutation concerns into focused modules (no behavior change).

Diagram

graph TD
  UI["UI component"] --> useK["useKnowledge()"] --> userQ["useUserKnowledgeQuery()"] --> RQ[("React Query cache")] --> SB{{"Supabase"}}
  useK --> toggleM["useKnowledgeToggleMutation()"] --> RQ
  useK --> AUTH["AuthContext"]

  subgraph Legend
    direction LR
    _hook["Hook/module"] ~~~ _cache[("Cache/state")] ~~~ _ext{{"External service"}}
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Export fetchers instead of queryOptions factories

• ➕ Simpler API surface if loaders/SSR aren’t using TanStack Query directly
• ➕ Keeps React Query-specific configuration inside hooks only
• ➖ Harder to reuse consistent queryKey/queryFn wiring across non-hook consumers
• ➖ Encourages duplicated query configuration when adding loaders later

2. Keep knowledge in hooks/queries with a thin api re-export

• ➕ Minimizes churn for existing import paths while introducing the new structure
• ➕ Allows gradual migration by consumers
• ➖ Creates two ‘homes’ for the same feature, increasing discoverability/ownership ambiguity
• ➖ Prolongs deprecation/removal work

Recommendation: The chosen approach (moving to src/api/knowledge and introducing queryOptions factories) is the best fit if the codebase is standardizing on API modules and anticipates reuse of query configuration (e.g., loaders). The split into types (keys), query, mutation, and composite hook keeps responsibilities clear and reduces future duplication.

Files changed (4) +78 / -77

Enhancement (2) +72 / -0

useKnowledge.tsExtract composite knowledge hook into its own module +34/-0

Extract composite knowledge hook into its own module

• Adds 'useKnowledge()' that composes auth state, 'useUserKnowledgeQuery', and 'useKnowledgeToggleMutation'. It exposes 'userKnowledge' and a guarded 'handleKnowledgeToggle' that returns 'requiresAuth' when no user is present.

src/api/knowledge/useKnowledge.ts

useUserKnowledgeQuery.tsAdd user knowledge query + queryOptions factory +38/-0

Add user knowledge query + queryOptions factory

• Adds 'userKnowledgeQuery(userId)' returning TanStack 'queryOptions' for consistent queryKey/queryFn reuse. Implements 'useUserKnowledgeQuery' by spreading the factory options and gating execution with 'enabled: !!userId'.

src/api/knowledge/useUserKnowledgeQuery.ts

Refactor (2) +6 / -77

types.tsAdd centralized React Query key factory for knowledge +4/-0

Add centralized React Query key factory for knowledge

• Introduces 'knowledgeKeys' to standardize query keys for the knowledge feature, including a per-user key builder. This enables consistent cache access across query/mutation modules.

src/api/knowledge/types.ts

useKnowledgeToggleMutation.tsRefocus module on toggle mutation and reuse shared keys +2/-77

Refocus module on toggle mutation and reuse shared keys

• Removes the embedded query key factory, query hook, and composite hook from this file, leaving only the toggle mutation logic. Updates imports to use 'knowledgeKeys' from 'types' while preserving optimistic updates, rollback, and invalidation behavior.

src/api/knowledge/useKnowledgeToggleMutation.ts

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📎 Requirement gaps (0) 📜 Skill insights (0)

Context used

✅ Tickets: 🎫 Effort 1 — Restructure src/hooks/queries → src/api (feature-sliced modules)

✅ Compliance rules (platform): 109 rules

✅ Skills: 4 invoked

  supabase

  supabase-postgres-best-practices

  tdd

  improve-codebase-architecture

1. handleKnowledgeToggle uses mutateAsync try/catch 📘 Rule violation ⚙ Maintainability

Description

handleKnowledgeToggle wraps knowledgeToggleMutation.mutateAsync(...) in a try/catch to handle
success/error paths. This violates the requirement to use callback-based `mutate(variables, {
onSuccess, onError })` for mutation handling.

Code

src/api/knowledge/useKnowledge.ts[R10-27]

+  async function handleKnowledgeToggle(artistId: string) {
+    if (!user) {
+      return { requiresAuth: true };
+    }
+
+    const isKnown = userKnowledge[artistId];
+
+    try {
+      await knowledgeToggleMutation.mutateAsync({
+        artistId,
+        userId: user.id,
+        isKnown,
+      });
+      return { requiresAuth: false };
+    } catch (error) {
+      console.error("failed toggling knowledge", error);
+      return { requiresAuth: false };
+    }

Evidence

PR Compliance ID 721323 forbids wrapping mutation.mutateAsync(...) in try/catch for
success/error handling, requiring callback-based mutate instead. The added handleKnowledgeToggle
implementation uses mutateAsync inside try/catch to implement the handling logic.

Rule 721323: Use callback-based mutate instead of try/catch mutateAsync for mutations
src/api/knowledge/useKnowledge.ts[10-27]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`handleKnowledgeToggle` currently uses `mutateAsync` with a surrounding `try/catch`. The mutation should instead be invoked via `mutate` with `onSuccess`/`onError` callbacks.

## Issue Context
This hook currently returns `{ requiresAuth: boolean }` for both success and failure; preserve that behavior while moving handling into callbacks.

## Fix Focus Areas
- src/api/knowledge/useKnowledge.ts[10-27]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Unsafe userId non-null ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

useUserKnowledgeQuery calls userKnowledgeQuery(userId!) before the enabled guard is applied, so
undefined userIds still build a queryKey/queryFn using an invalid value. This breaks the function’s
declared type contract (string) and can register a disabled query under a key containing undefined,
making cache behavior harder to reason about.

Code

src/api/knowledge/useUserKnowledgeQuery.ts[R33-37]

+export function useUserKnowledgeQuery(userId: string | undefined) {
+  return useQuery({
+    ...userKnowledgeQuery(userId!),
+    enabled: !!userId,
+  });

Evidence

The hook currently invokes userKnowledgeQuery(userId!) regardless of whether userId is defined,
because the spread expression is evaluated before React Query applies enabled. Other hooks in the
codebase avoid undefined key segments by passing a fallback value when enabled is false.

src/api/knowledge/useUserKnowledgeQuery.ts[33-37]
src/api/sets/useSetsByEdition.ts[56-60]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`useUserKnowledgeQuery` uses `userId!` when spreading `userKnowledgeQuery(...)`, meaning the query options are constructed even when `userId` is `undefined`.

## Issue Context
Even though `enabled: !!userId` prevents the query from running, the hook still builds a `queryKey` (and captures a `queryFn`) with an invalid `userId` value, which undermines type-safety and can leave a disabled query registered under a key containing `undefined`.

## Fix Focus Areas
- src/api/knowledge/useUserKnowledgeQuery.ts[33-37]

### Suggested fix
Update the hook to avoid passing `undefined` into the query options factory, e.g.:
- Use a fallback string (consistent with other hooks): `...userKnowledgeQuery(userId || "")`
- Or conditionally select options when `userId` is missing (preferred for strict correctness): build a disabled query config without calling `userKnowledgeQuery` until `userId` is present.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[qodo-code-review]

1. handleknowledgetoggle uses mutateasync try/catch 📘 Rule violation ⚙ Maintainability

handleKnowledgeToggle wraps knowledgeToggleMutation.mutateAsync(...) in a try/catch to handle
success/error paths. This violates the requirement to use callback-based `mutate(variables, {
onSuccess, onError })` for mutation handling.

Agent Prompt

## Issue description
`handleKnowledgeToggle` currently uses `mutateAsync` with a surrounding `try/catch`. The mutation should instead be invoked via `mutate` with `onSuccess`/`onError` callbacks.

## Issue Context
This hook currently returns `{ requiresAuth: boolean }` for both success and failure; preserve that behavior while moving handling into callbacks.

## Fix Focus Areas
- src/api/knowledge/useKnowledge.ts[10-27]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

[chiptus]

Triage of Qodo's two findings:

1. handleKnowledgeToggle uses mutateAsync in try/catch (rule violation) — pre-existing, not introduced here. The identical try { await knowledgeToggleMutation.mutateAsync(...) } catch is on main in the original src/hooks/queries/knowledge/useKnowledge.ts (~lines 146–153); this PR relocated it verbatim. It does violate the project convention (prefer mutate(vars, { onSuccess, onError })), so it's worth fixing — but as a behavioral change it's out of scope for this mechanical move. I can convert it (and audit other mutateAsync-try/catch usages) in a small dedicated follow-up.

2. userKnowledgeQuery(userId!) before the enabled guard — behavior-neutral, matches the established template. This mirrors the already-merged useFestivalBySlugQuery pattern (...factory(x!), enabled: !!x). The query is disabled when userId is undefined, so it never runs; the only nit is the key momentarily contains undefined. sets uses a || "" fallback instead — a minor inconsistency in the new layer worth unifying in a later consistency pass, not a bug. Leaving as-is to keep this slice a faithful move.

No code changes here.

---

Generated by Claude Code

[chiptus]

Correction to my earlier triage of finding 2 (the userKnowledgeQuery(userId!) one): I initially declined it as "matches the festivals template / behavior-neutral." That was wrong — I checked main, and the original useUserKnowledgeQuery normalized the key with knowledgeKeys.user(userId || ""). So the move's switch to userId! is the same disabled-state-key regression as #61/#62, not a faithful carry-over.

Fixed in ec52445: restored the original normalization with ...userKnowledgeQuery(userId ?? ""). No runtime impact (query is disabled when userId is missing), but it keeps the move behavior-neutral. Lint/build/290 tests green.

(Finding 1 — the pre-existing mutateAsync try/catch carried verbatim from main — still stands as out-of-scope for this mechanical move; offered as a separate convention follow-up.)

---

Generated by Claude Code