web/sdk/react/hooks/usePreferences.ts (3)

37-45: Consider validating that scopeType and scopeId are provided together.

If a caller passes scopeType without scopeId (or vice versa), the request will include an incomplete scope context. Depending on the API contract, this could silently return unscoped results or fail unexpectedly. A runtime guard or a type-level constraint would make this more robust.

Option A: Runtime guard

 export function usePreferences({
   autoFetch = true,
   scopeType,
   scopeId
 }: {
   autoFetch?: boolean;
   scopeType?: string;
   scopeId?: string;
 } = {}): UsePreferences {
+  if ((scopeType && !scopeId) || (!scopeType && scopeId)) {
+    console.warn('frontier:sdk:: usePreferences: scopeType and scopeId should be provided together');
+  }

Option B: Type-level constraint (discriminated union)

type UsePreferencesOptions =
  | { autoFetch?: boolean; scopeType?: undefined; scopeId?: undefined }
  | { autoFetch?: boolean; scopeType: string; scopeId: string };

---

89-101: Duplicate error logging — onError in useMutation and catch in updatePreferences both log.

When updatePreferencesMutation rejects, the onError callback on line 81–86 logs the error, and then the catch block on lines 96–100 logs the same error again before rethrowing. This results in double console output for every mutation failure.

♻️ Remove one of the duplicate error handlers

Either remove the onError callback from useMutation (since updatePreferences already handles errors), or remove the try/catch logging and let onError be the single logging site:

   const updatePreferences = useCallback(async (preferences: Preference[]) => {
-    try {
-      const req = create(CreateCurrentUserPreferencesRequestSchema, {
-        bodies: preferences
-      });
-      await updatePreferencesMutation(req);
-    } catch (err) {
-      console.error(
-        'frontier:sdk:: There is problem with updating user preferences'
-      );
-      console.error(err);
-      throw err;
-    }
+    const req = create(CreateCurrentUserPreferencesRequestSchema, {
+      bodies: preferences
+    });
+    await updatePreferencesMutation(req);
   }, [updatePreferencesMutation]);

The onError on the mutation already handles logging. If you need the caller to catch, mutateAsync rejects naturally.

---

104-118: The composite status field doesn't account for error states.

The status computation maps to 'loading' | 'fetching' | 'idle', but both the query and mutation can be in an error state. Consumers relying on status alone won't know about failures — they'd need to check fetchPreferencesStatus or updatePreferencesStatus separately. This is pre-existing behavior, but now that you're exposing the granular statuses, consider documenting or aligning the composite status type.

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro