Remove /core/apps directory and move perpetua to features/perpetua

[zeeghazi]

No description provided.

[github-actions]

Code Review: Remove /core/apps directory and move perpetua to features/perpetua

Overview

This is a large structural cleanup PR removing the entire legacy src/alex_frontend/core/apps/ directory (~25,000 lines). The new features/perpetua implementation already on the base branch replaces it with a modern React Query + minimal Redux architecture.

The direction is correct. The old core/apps structure mixed server-state into Redux with large thunks and slices, while the new code separates concerns: TanStack Query for server state, Redux only for lightweight UI state (feed type, filters, edit mode).

What Is Good

• No dangling imports: Zero remaining references to core/apps or old module paths, confirmed by grep.
• No remaining valora references: The rename in commit 2 was thorough.
• Clean query key factory: perpetuaKeys in types.ts makes cache invalidation explicit and structured.
• Optimistic updates with rollback: useMutations.ts correctly implements cancel/optimistic-set/rollback-on-error across all major mutations.
• rootReducer.ts is clean: Already pointing to @/features/perpetua/store/slice.

Issues

Bug risk in useRemoveItem optimistic update (hooks/useMutations.ts):

items: prev.items.filter(([, item]) => item.id !== itemId)

prev.items is Array<[number, Item]> where the first tuple element is the item ID. This filter checks item.id on the second element. If the Item candid type does not carry an id field, the comparison is always undefined !== itemId and the optimistic remove silently no-ops. The onSettled query invalidation masks this at runtime. Fix:

items: prev.items.filter(([id]) => id !== itemId)

Type safety: as any cast in ShelfCard.tsx line 28:

navigate({ to: basePath + '/shelf/$shelfId', params: { shelfId: shelf.shelfId } } as any)

Dynamic URL construction bypasses TanStack Router compile-time route checking. If basePath is not a registered route prefix this silently navigates nowhere. Consider accepting a typed to route as a prop, or restricting basePath to a union of known prefixes.

useSetItemOrder has no optimistic update: Unlike the other mutations, this one only invalidates on success. After every drag-and-drop the list briefly flashes back to the original order while the refetch completes. Worth applying the same optimistic pattern used in useRemoveItem.

Process Concerns

• Empty PR description: With 25,000 lines deleted and a full architectural shift, even a brief note on what was removed and any manual testing would help future contributors.
• No tests: utils.ts has pure functions (normalizeShelf, unwrapResult, getItemContentType) that are good unit test candidates and would cover the most critical correctness risk in this refactor.

Summary

The cleanup is thorough and the architecture upgrade is the right call. Two items worth addressing before merge: the useRemoveItem filter (potential silent optimistic-update bug depending on the Item DID shape) and the as any cast in ShelfCard. Everything else is an improvement, not a blocker.