Phase 4: Architecture & tooling investment

Fixes a real N+1 API-call bug (per-cell project-member fetches in shot/asset
task-status cells, now a shared cached store), replaces a polling-based
in-flight request de-dup with promise memoization, adds markRaw() around
icon components in reactive state, and fixes a deep watcher that re-scanned
the whole projects array just to trigger thumbnail loading.

Introduces useAsyncAction and usePermission composables to cut duplicated
store boilerplate and duplicated role/admin checks, applied only where the
existing code was a clean match rather than forced onto everything. Extracts
assets.ts's shared per-asset optimistic-update/rollback helper without
merging the single and bulk API paths, which hit genuinely different
endpoints. Adds toast-on-error for previously-silent secondary loads and
fixes a dead try/catch in the router's auth-init guard along the way.

Also fixes a thumbnail-loading regression introduced earlier in this same
pass: the new ID-keyed watcher never fired on a repeat visit to the Projects
page when the store already held the same project list, so thumbnails
(local component state, reset per mount) silently stopped loading after the
first visit. Replaced the watcher with a direct call after each fetch.

Table virtualization, splitting the two largest view files, broad store
caching, and moving domain types into types/ were scoped out as separate,
higher-risk follow-ups (documented in frontend_tasks.md).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

frontend_tasks.md

@@ -46,43 +46,56 @@ Derived from `frontend_report.md`. Checklist form for tracking progress — chec
 - `GET /assets/{id}` never returned `task_details` (schema didn't even declare the field) — unlike `GET /shots/{id}`, which already did. Broke the asset detail panel's task list and, transitively, every new create-task/note/reference/version feature. Fixed in `backend/schemas/asset.py` + `backend/routers/assets.py`.
 - `TaskBrowser.vue`'s `handleRowClick` is a no-op by design (single click reserved for selection) — the new row-actions menu's "View Details"/"Reassign" needed to emit `row-double-click` instead, which is what actually opens the panel.
 
-## Phase 3 — Unify controls
+## Phase 3 — Unify controls ✅ (done)
 
-- [ ] Extract `ColumnToggleList` component (built on `DropdownMenuCheckboxItem`, not hand-rolled div+Check) and point at it from:
-  - [ ] `components/ui/sidebar/SidebarColumnSwitch.vue`
-  - [ ] `components/shot/ShotTableToolbar.vue`
-  - [ ] `components/asset/AssetTableToolbar.vue`
-  - [ ] `components/task/TaskTableToolbar.vue`
-- [ ] Extract shared `EntityTableToolbar` (search, column-visibility trigger, detail-panel toggle, clear-filters) to de-duplicate the three toolbar files
-- [ ] Extract `SegmentedToggle` component for the three hand-built view-mode switches (shot grid/list/table, asset grid/list, task all/shots/assets)
-- [ ] Standardize delete confirmation on the shot pattern (`Dialog` + `Alert` + impact summary + type-to-confirm) for any cascading delete; fix asset's dialog to match or consciously keep it lighter
-- [ ] Swap hand-rolled checkbox-divs for real `Checkbox`/`DropdownMenuCheckboxItem` in:
-  - [ ] `SidebarColumnSwitch.vue`
-  - [ ] `ShotTableToolbar.vue`
-  - [ ] `AssetTableToolbar.vue`
-  - [ ] `TaskTableToolbar.vue`
-  - [ ] `ShotTaskStatusFilter.vue`
-  - [ ] `asset/TaskStatusFilter.vue`
-- [ ] Replace ad hoc `size="sm" class="h-8 w-8 p-0"` with `size="icon-sm"` across toolbar/browser icon buttons
-- [ ] Decide one convention (icon+label vs. icon-only+tooltip) for primary "create" CTAs and apply consistently (shot/asset create buttons currently lack tooltips project/episode create buttons have)
-- [ ] Standardize row-actions/bulk-actions on `DropdownMenu`/`DropdownMenuItem`; replace ad hoc `<button>` list in `components/task/TaskBulkActionsMenu.vue`
-- [ ] Extract `DetailPanelShell` (header/close button, loading state, error state, Tabs scaffold, slide-in transition) for `ShotDetailPanel.vue`, `AssetDetailPanel.vue`, `TaskDetailPanel.vue` to share; add missing error state to Task's panel
+- [x] Extract `ColumnToggleList` component and point at it from:
+  - [x] `components/ui/sidebar/SidebarColumnSwitch.vue`
+  - [x] `components/shot/ShotTableToolbar.vue`
+  - [x] `components/asset/AssetTableToolbar.vue`
+  - [x] `components/task/TaskTableToolbar.vue`
 
-## Phase 4 — Architecture & tooling investment
+  Built on a new `CheckableCommandItem` primitive (`Command`/`CommandItem` wrapping a decorative `Checkbox`) instead of `DropdownMenuCheckboxItem` — keeps `CommandInput` search (needed for task-type columns) and avoids the ARIA double-toggle bug already found and fixed in the dead `ShotColumnVisibilityControl.vue`.
+- [x] Shared toolbar pieces extracted as separate composable pieces rather than one monolithic `EntityTableToolbar` (scope decision, approved before implementation: shot/asset/task toolbars diverge enough — shot's 3-way view mode + bulk-create, asset's thumbnail toggle, task's different filter set — that a config-driven mega-component would be a leaky abstraction): `useDebouncedSearch` composable, `DetailPanelToggleButton.vue`, `ClearFiltersButton.vue`, `SegmentedToggle.vue`. All three toolbars now compose these instead of hand-rolled equivalents.
+- [x] `SegmentedToggle` component — done (shot grid/list/table, asset grid/list, task all/shots/assets).
+- [x] Delete confirmation — verified already consistent (Shot/Asset both use `Dialog`+`Alert`+impact-summary+type-to-confirm since Phase 1). Episode/Project deletes are simple client-side guards with no impact-summary fetch at all, so giving them one would be new feature work, not unification — left as-is (verification-only, no code change).
+- [x] Swap hand-rolled checkbox-divs for `CheckableCommandItem` in:
+  - [x] `SidebarColumnSwitch.vue`
+  - [x] `ShotTableToolbar.vue`
+  - [x] `AssetTableToolbar.vue`
+  - [x] `TaskTableToolbar.vue`
+  - [x] `ShotTaskStatusFilter.vue` / `asset/TaskStatusFilter.vue` — merged into one `components/shared/TaskStatusFilter.vue`
+- [x] Replace ad hoc `size="sm" class="h-8 w-8 p-0"` with `size="icon-sm"` — 24 occurrences across toolbars, detail panels, row-action triggers, and columns.ts render functions
+- [x] CTA convention: dense table toolbars (Shot/Asset create buttons) stay icon-only but now have the same `title` tooltip every sibling icon button already had; full-page views (Project/Episode "New X") standardized wording between header and empty-state instances
+- [x] Standardize row-actions/bulk-actions on `DropdownMenu`/`DropdownMenuItem` — `TaskBulkActionsMenu.vue`'s "Assign To" section now mirrors "Set Status"'s existing submenu instead of a hand-rolled `<button>` list
+- [x] Extract shared detail-panel shell for `ShotDetailPanel.vue`, `AssetDetailPanel.vue`, `TaskDetailPanel.vue`: `DetailPanelOverlay.vue` (slide-in transition + mobile `Sheet`), `DetailPanelHeader.vue`, `DetailPanelLoading.vue`, `DetailPanelError.vue`. The `Tabs` scaffold was deliberately *not* unified — tab sets genuinely differ per domain (5 vs. 3 vs. 4 tabs) and forcing a shared config would be the kind of premature abstraction this phase was meant to avoid. Added the previously-missing error state to `TaskDetailPanel.vue` (it had no `error` ref or error branch at all — a failed load rendered nothing).
 
-- [ ] Add a shared `useAsyncAction`-style composable (`isLoading`/`error`/`run(fn)`) and adopt it across `stores/*.ts` to cut repeated try/catch/finally boilerplate
-- [ ] Replace `stores/taskStatuses.ts`'s polling-based in-flight request de-dup (100ms `setInterval` loop, lines 96-117) with promise memoization
-- [ ] Extend a similar TTL/cache strategy to `stores/assets.ts`, `stores/tasks.ts`, `stores/projects.ts`, `stores/episodes.ts` (currently no caching — every view re-fetches on mount)
-- [ ] De-duplicate `assets.ts`'s optimistic-update/rollback logic between single (`lines 156-234`) and bulk (`lines 236-356`) task-status updates into one parameterized function
-- [ ] Add `markRaw()` around icon components stored in reactive state — `stores/projects.ts` `assignProjectIcon`
-- [ ] Audit `deep: true` watchers, especially `views/ProjectsView.vue:770` (watches the full, non-`markRaw`'d projects array)
-- [ ] Fix N+1 project-member fetch: hoist `getProjectMembers()` call out of `EditableTaskStatus.vue` (mounted per row × task-type column) into a shared store/cache or parent-passed prop
-- [ ] Add table virtualization (e.g. `@tanstack/vue-virtual`) to `ShotsDataTable.vue`, `AssetsDataTable.vue`, `TasksDataTable.vue`
-- [ ] Introduce a shared `EmptyState` component and standardize on `Skeleton` for loading states (currently: 5 files use `Skeleton`, 62 use ad hoc spinners; 31 files hand-roll empty-state text)
-- [ ] Add toast-on-error for currently-silent secondary loads: episodes/task-types/task-statuses/project-context in `ShotBrowser.vue` (lines 474-508), `stores/notifications.ts:fetchStats`, router auth-init failure (`router/index.ts:190-196`)
-- [ ] Centralize a `usePermission()`/`can()` composable to consolidate role/admin checks currently duplicated across ~20 components outside the router guard
-- [ ] Consider splitting the largest files into smaller pieces: `views/project/ShotDetailView.vue` (1658 lines), `views/admin/DeletedItemsManagementView.vue` (1279 lines), and evaluate a shared "entity browser" composable for `ShotBrowser.vue`/`AssetBrowser.vue`'s ~70% structural overlap
-- [ ] Move ad hoc domain types out of `services/*.ts` into `types/` for discoverability (currently only `auth.ts`, `notification.ts`, `activity.ts` live in `types/`)
+**Bugs found and fixed during verification (approved mid-implementation):**
+- Shot's episode filter and Asset's category filter had silently been left on the old hand-rolled checkbox pattern (visually identical to `CheckableCommandItem`, so it wasn't caught by screenshots alone) — converted along with everything else.
+- `ShotDetailPanel.vue`'s Tasks list built `assigned_user_name: undefined // Will be resolved if needed` and never actually resolved it, so every task always showed "Unassigned" regardless of real assignment; and its task-status `Badge` used a hardcoded 5-value switch instead of the same `TaskStatusBadge`/task-statuses-store the shot table uses, so colors/labels (including custom per-project statuses) could drift from the table. Fixed both, and added an avatar next to the assignee name to match the table's assignment control.
+
+## Phase 4 — Architecture & tooling investment ✅ (done, scoped)
+
+Scope was narrowed with the user before implementation: this pass covers the mechanical/low-risk items with a demonstrated, concrete problem behind each. Several checklist items below turned out to be either genuine new feature work, carry real regression risk on large already-working files, or would touch a very wide surface for a purely organizational payoff — those are called out as **deferred**, not silently dropped.
+
+- [x] Fix N+1 project-member fetch: new `stores/projectMembers.ts` (same Map+TTL+in-flight-promise-dedup pattern as `taskStatuses.ts`), both `EditableTaskStatus.vue` variants now use it instead of independently calling `projectService.getProjectMembers` per row × task-type column. Verified via network interception: one call per project load, not one per cell.
+- [x] Replace `stores/taskStatuses.ts`'s polling-based in-flight de-dup (100ms `setInterval` loop) with a stored in-flight `Promise` per project ID — same pattern applied to the new `projectMembers` store.
+- [x] Add `markRaw()` around icon components stored in reactive state — `stores/projects.ts` `assignProjectIcon` and the static `allProjectsView.icon`.
+- [x] Fix `views/ProjectsView.vue`'s deep watcher on the full projects array (only existed to trigger thumbnail loading) — replaced with a shallow watch keyed on project IDs. The other ~10 `deep: true` watchers found elsewhere were audited but not changed — each needs its own read to judge triviality, out of scope for this pass.
+- [x] Add a shared `useAsyncAction` composable (`{isLoading, error}` in, `run(fn, {errorMessage, rethrow})` out) — applied only where the shape was already a clean match with no divergence: `auth.ts` (`login`/`register`), `episodes.ts` (all 5 actions), `settings.ts` (all 6), and the plain fetch actions in `projects.ts` (5) and `tasks.ts` (`fetchTasks`/`fetchTask`). Left untouched: `user.ts`'s mutations (no `isLoading` flag — adding one would be new behavior), `notifications.ts` (chains a second async call), `assets.ts`'s optimistic updaters (different shape, see below), `auth.ts:logout` (intentionally swallows and always clears state).
+- [x] Extract `assets.ts`'s shared per-asset optimistic-update helper (`snapshotAssetTaskStatus`/`applyAssetTaskStatus`/`rollbackAssetTaskStatus`) used by both `updateTaskStatus` and `bulkUpdateTaskStatus` — the two outer functions stay separate since they call genuinely different API endpoints (single-task vs. a distinct bulk endpoint with its own response shape); only the local-state snapshot/apply/rollback was actually duplicated.
+- [x] Add `usePermission()` composable (`isAdmin`, `isCoordinatorOrAdmin`) and apply to the ~15 confirmed sites duplicating the `is_admin || role === 'coordinator'` check (`ShotDetailPanel.vue`, `TaskDetailPanel.vue`, `TaskList.vue`, `EpisodesView.vue`, `EpisodeDropdown.vue`, `ProjectsView.vue`, `ProjectSettingsView.vue`, `TechnicalSpecsPanel.vue`, `TechnicalSpecsManager.vue`, `AppSidebar.vue`, `ProjectSwitcher.vue`). Left untouched: `UserMenu.vue`'s three-way variant (adds a `developer` role check) and `NoteItem.vue`'s ownership check — genuinely different logic.
+- [x] Add toast-on-error for the silent secondary loads in `ShotBrowser.vue` (episodes/task-types/task-statuses/project-context) and the router's auth-init failure path. In fixing the latter, found and fixed a real bug: `router/index.ts`'s `try/catch` around `authStore.initializeAuth()` was dead code — that function never throws (it swallows its own errors and calls `logout()` internally) — so the catch could never fire. Replaced with a check of `authStore.isAuthenticated` after the call, which correctly detects the failure. **`stores/notifications.ts:fetchStats` was deliberately left as a silent `console.error`**: it's polled every 30s by `startPolling()`, and a destructive toast firing on every poll during a network blip would be worse UX than the silent failure it replaces — this deviates from the original checklist wording, flagging rather than silently skipping.
+
+**Bugs found and fixed during verification (approved mid-implementation):**
+- `ShotDetailPanel.vue` had a dead `authStore.isAdmin` check sitting right next to the ones consolidated into `usePermission()` — swapped to the composable's `isAdmin` for consistency while already in that file.
+- Found (not fixed — backend, out of scope): `GET /settings/upload-limit` 404s because `backend/routers/settings.py` declares its own `/settings` prefix and `main.py` adds another, so the real path is `/settings/settings/upload-limit`. Confirmed via direct backend curl; unrelated to any frontend change in this phase.
+
+**Deferred** (documented, not implemented — see the approved plan for full rationale):
+- [ ] Extend TTL caching to `stores/assets.ts`, `stores/tasks.ts`, `stores/projects.ts`, `stores/episodes.ts` — no demonstrated redundant-fetch problem for these stores (unlike task statuses/project members), and caching list data in a multi-user live-editing tool risks staleness bugs.
+- [ ] Add table virtualization (`@tanstack/vue-virtual`) to `ShotsDataTable.vue`, `AssetsDataTable.vue`, `TasksDataTable.vue` — real feature work with design decisions that interact with the existing frozen-column two-pane scroll-sync layout, not a mechanical cleanup.
+- [ ] Introduce a shared `EmptyState` component and standardize on `Skeleton` for loading states — both already exist (`ui/empty/EmptyState.vue`, `ui/skeleton/Skeleton.vue`) and are cheap to reach for going forward, but a full sweep of ~58 ad hoc spinner sites is out of scope for this pass.
+- [ ] Split `views/project/ShotDetailView.vue` (1658 lines) / `views/admin/DeletedItemsManagementView.vue` (1279 lines) and extract a shared "entity browser" composable for `ShotBrowser.vue`/`AssetBrowser.vue` — both files are live and wired, and the overlap between Shot/Asset browsers is only partial (episode vs. category filters, store vs. local state). Bigger regression risk than the payoff justifies bundling here.
+- [ ] Move domain types (`Shot`/`Asset`/`Task`/`Project`/`Episode`) out of `services/*.ts` into `types/` — mechanical but touches every import site for a purely organizational win; fix opportunistically when a file is substantially touched anyway, same approach as the `ApiError` helper below.
 
 ## Phase 5 — Testing & quality gates