Improve React code quality#34
Conversation
There was a problem hiding this comment.
16 issues found across 30 files
Confidence score: 2/5
- In
apps/web/src/components/songs/SongDetail.tsx, saving while mixes/microphones are still loading can treat empty fallback data as real and wipe saved assignments/mic overrides, which is direct data-loss risk for song configuration — preserve prior persisted mappings until dependent data has actually loaded. - In
apps/web/src/routes/shows/$showId/setlist/$songId.tsxandapps/web/src/components/songs/SongDetail.tsx, optimistic remounts and reset-on-error paths can allow overlapping saves and then roll back newer edits, so users can lose recent changes or see inconsistent form state — gate concurrent mutations and avoid whole-form resets for single-field failures. - In
apps/web/src/components/mixes/MixCard.tsxandapps/web/src/components/songs/MicrophoneName.tsx, concurrent/stale event flows can submit outdated payloads (number vs color races, Escape still triggering blur-save), causing one user edit to overwrite another — serialize or patch only changed fields and suppress blur-save after cancel/escape. - In
apps/web/src/components/microphones/MicrophoneCard.tsx,apps/web/src/components/shows/ShowDeleteDialog.tsx,apps/web/src/components/chats/ChatPresetDialog.tsx,apps/web/src/components/chats/ChatWorkspace.tsx, andapps/web/src/components/microphones/MicrophoneDeleteDialog.tsx, pending/delete dialog and selection state transitions have timing gaps that can leave stale IDs, incorrect navigation/close behavior, or unreported failures — tighten pending-state guards and route all close/selection updates through a single synchronized path.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/web/src/components/microphones/MicrophoneCard.tsx">
<violation number="1" location="apps/web/src/components/microphones/MicrophoneCard.tsx:25">
P3: MicrophoneCard now duplicates MixCard's editable-card behavior, so lifecycle and reliability fixes can diverge between microphones and mixes. A shared named-item card or shared edit-field logic would keep these workflows synchronized while retaining their entity-specific labels and mutations.</violation>
<violation number="2" location="apps/web/src/components/microphones/MicrophoneCard.tsx:89">
P2: An optimistic microphone can still be edited while its create mutation is pending, so these controls can submit edits against the temporary ID and show avoidable failures or conflicting optimistic state. The pending guard would be more reliable if it also disabled the number/name inputs, the popover trigger, and color buttons, as `MixCard` does.</violation>
</file>
<file name="apps/web/src/components/mixes/MixDeleteDialog.tsx">
<violation number="1" location="apps/web/src/components/mixes/MixDeleteDialog.tsx:23">
P3: The mix delete workflow now duplicates the existing microphone delete dialog, so fixes to shared deletion behavior can diverge between these screens. A shared resource-agnostic delete dialog or hook would keep the two flows consistent.</violation>
</file>
<file name="apps/web/src/components/connections/ConnectionDialog.tsx">
<violation number="1" location="apps/web/src/components/connections/ConnectionDialog.tsx:94">
P3: Pairing selection and reset behavior can regress without a focused test for the new reducer, especially around preserving a valid selection versus clearing the QR/copy state. Adding reducer-level tests for `loaded`, `select`, `reset`, and error transitions would make this refactor safer to maintain.</violation>
</file>
<file name="apps/web/src/components/shows/ShowDeleteDialog.tsx">
<violation number="1" location="apps/web/src/components/shows/ShowDeleteDialog.tsx:31">
P2: A fast deletion completion can observe a stale `dialogRef` during the gap before this passive effect runs, causing a closed or replaced dialog to be treated as the original delete and triggering incorrect close/navigation behavior. Synchronizing this latest-value ref in a layout effect (or another commit-synchronous path) preserves render purity without leaving that window.</violation>
</file>
<file name="apps/web/src/components/songs/MicrophoneName.tsx">
<violation number="1" location="apps/web/src/components/songs/MicrophoneName.tsx:56">
P2: Pressing Escape can still save the edited microphone name: this branch closes the editor without suppressing `onBlur`, whose stale `value` may be submitted when focus is lost. Tracking a `suppressNextBlurSave` ref (as `ProfileSwitcher` does) and skipping the next blur save would preserve Escape as a cancel action.</violation>
</file>
<file name="apps/web/src/components/songs/SongDetail.tsx">
<violation number="1" location="apps/web/src/components/songs/SongDetail.tsx:106">
P1: Editing a song while mixes or microphones are still loading can silently clear every saved assignment and microphone override. The empty fallback lists should not be treated as authoritative; preserving the existing payload or deferring saves until both lists are loaded avoids this data loss.</violation>
<violation number="2" location="apps/web/src/components/songs/SongDetail.tsx:180">
P2: A microphone-name save can fail after the user has edited another song field, and the failure handler then discards those newer local edits by resetting the entire form to the original song. Serializing this request with the other saves, or reconciling only the failed microphone-name change, prevents that data loss.</violation>
</file>
<file name="apps/web/src/components/songs/SongMixAssignments.tsx">
<violation number="1" location="apps/web/src/components/songs/SongMixAssignments.tsx:66">
P3: The assignment count can include microphones that have since been deleted, causing the UI to display e.g. “1 mic on” while every visible microphone is unselected. Filtering assignment IDs against the current `microphones` list before constructing `selected` keeps the count consistent with the controls rendered below.</violation>
</file>
<file name="apps/web/src/components/chats/ChatPresetDialog.tsx">
<violation number="1" location="apps/web/src/components/chats/ChatPresetDialog.tsx:145">
P2: After a successful preset send, reopening the dialog shows the previously used preset instead of the preset list because the close callback bypasses `changeOpen` and the reset effect is gone. Routing this close through `changeOpen(false)` (and preserving a reset for externally controlled closes) would keep dialog sessions predictable.</violation>
</file>
<file name="apps/web/src/components/chats/ChatMessageBody.tsx">
<violation number="1" location="apps/web/src/components/chats/ChatMessageBody.tsx:15">
P2: Empty adjacent parts can still produce duplicate React keys, so React may warn and reconcile these sibling spans unpredictably. Including the part index as a uniqueness fallback (or otherwise assigning an ID) would preserve unique keys for all contract-valid `ChatMessagePart` arrays.</violation>
</file>
<file name="apps/web/src/routes/shows/$showId/setlist/$songId.tsx">
<violation number="1" location="apps/web/src/routes/shows/$showId/setlist/$songId.tsx:75">
P1: Saving an edit can immediately remount `SongDetail` before the RPC resolves because the optimistic edit changes `updatedAt`; the new instance recreates `isSaving`/`isSavingRef` as false, allowing overlapping edits, and failure updates target the discarded instance so the save error can be lost. A stable component identity plus explicit synchronization of server changes would preserve the in-flight save state without reintroducing stale drafts.</violation>
</file>
<file name="apps/web/src/components/chats/ChatWorkspace.tsx">
<violation number="1" location="apps/web/src/components/chats/ChatWorkspace.tsx:134">
P2: Deleting the active channel now displays the first remaining channel without notifying `onSelectedChannelChange`, so `ChatDrawer` retains the deleted channel ID as its requested selection. The selection fallback needs to update the owning selection state (or the delete flow needs to explicitly select and publish the replacement channel) rather than only changing the rendered value.</violation>
</file>
<file name="apps/web/src/components/mixes/MixCard.tsx">
<violation number="1" location="apps/web/src/components/mixes/MixCard.tsx:71">
P3: The duplicate-label warning misses existing mix labels with surrounding whitespace because `trimmedNumber` is normalized but `other.number` is not. Comparing `other.number.trim()` would match the normalization performed by `commitNumber` and the existing microphone UI.</violation>
<violation number="2" location="apps/web/src/components/mixes/MixCard.tsx:141">
P2: Changing a mix number and immediately choosing a color can lose one of the two edits: both handlers issue full `mixes.edit` payloads, so whichever request runs last overwrites the other field with its stale snapshot. Serializing edits or changing the mutation to merge/patch fields would preserve both user changes.</violation>
</file>
<file name="apps/web/src/components/microphones/MicrophoneDeleteDialog.tsx">
<violation number="1" location="apps/web/src/components/microphones/MicrophoneDeleteDialog.tsx:57">
P2: The dialog can still be dismissed through the header X or Escape while `isDeleting` is true; only the footer Cancel button is disabled. That unmounts the component while the delete continues, so a failed request has no visible error and the user can reopen the item and start another deletion; keep the dialog open for all dismiss paths while deletion is in flight.</violation>
</file>
Architecture diagram
sequenceDiagram
participant App as App Bootstrap (main.tsx)
participant ChatNav as ChatNavigation module
participant Router as TanStack Router
participant Route as Route Component ($songId.tsx)
participant SongDetail as SongDetail component
participant MicrophoneCard as MicrophoneCard component
participant MixCard as MixCard component
participant DeleteDialog as DeleteDialog (Microphone/Mix/Song)
participant UIItem as UI Item component
participant Profiles as currentProfilesState
participant AtomLayer as Effect Atom Layer
participant Backend as Backend API
Note over App,ChatNav: NEW: Chat navigation configuration extracted to bootstrap
App->>ChatNav: configureChatNavigation({ getActiveShowId, navigateToShow })
ChatNav->>ChatNav: Create internal makeChatNavigation instance
App->>Router: router.state.matches (deferred)
Note over App,AtomLayer: Connection dialog state management consolidated
ConnectionDialog->>ConnectionDialog: useReducer(reducePairClientState, initial)
loop Poll pairing info
ConnectionDialog->>Backend: fetchPairingInfo(client)
Backend-->>ConnectionDialog: { discovery, candidates }
ConnectionDialog->>ConnectionDialog: dispatch({ type: "loaded", ... })
alt Candidates empty
ConnectionDialog->>ConnectionDialog: dispatch({ type: "error", message: "No local network found" })
end
end
Note over Route,SongDetail: NEW: SongDetail extracted with useReducer
Route->>AtomLayer: songAtoms(showId).songs
Route->>AtomLayer: microphoneAtoms(showId)
Route->>AtomLayer: mixAtoms(showId)
Route-->>SongDetail: Pass data via props
SongDetail->>SongDetail: useReducer(reduceSongDetailState, initialState)
SongDetail->>AtomLayer: useAtomSet(songAtoms.edit)
Note over SongDetail: CHANGED: derived state from useReducer, not multiple useStates
SongDetail->>SongDetail: save() with field normalization
SongDetail->>AtomLayer: edit(payload, { reactivityKeys })
alt Success
AtomLayer-->>SongDetail: Exit.success
else Failure
AtomLayer-->>SongDetail: Exit.failure
SongDetail->>SongDetail: Rollback to original values
end
Note over SongDetail,UIItem: CHANGED: list item semantics (li/ul) for correct accessibility
UIItem->>UIItem: Changed defaultTagName from "div" to "li"
ItemGroup->>ItemGroup: Changed from "div role=list" to "ul"
Note over Route,DeleteDialog: NEW: Delete dialogs extracted as separate components
DeleteDialog->>AtomLayer: useAtomSet(microphoneAtoms.delete) / mixAtoms.delete / songAtoms.delete
DeleteDialog->>DeleteDialog: confirmDelete()
DeleteDialog->>AtomLayer: delete(payload, { reactivityKeys })
alt Success
AtomLayer-->>DeleteDialog: Exit.success
DeleteDialog->>DeleteDialog: onClose()
else Failure
AtomLayer-->>DeleteDialog: Exit.failure
DeleteDialog->>DeleteDialog: Display error, keep dialog open
end
Note over MicrophoneCard,MixCard: NEW: Extracted from route files
MicrophoneCard->>AtomLayer: useAtomSet(microphoneAtoms.edit)
MicrophoneCard->>MicrophoneCard: Use Map lookup for duplicate check
Note over MicrophoneCard: CHANGED: MicrophoneById map avoids linear find()
Note over Profiles: CHANGED: currentProfilesState extracted to dedicated module
ProfilesSwitcher->>Profiles: import from /profiles/currentProfilesState
Profiles->>Profiles: Handle AsyncResult.Success & Failure with fallback
Note over App,Backend: Preset dialog reset moved from useEffect to callback
ChatPresetDialog->>ChatPresetDialog: changeOpen() resets mode synchronously
Note over ChatPresetDialog: CHANGED: No useEffect dependency on "open"
Note over Route,AtomLayer: SongDetail re-keyed on update to force fresh instance
Route->>Route: key={`${song.id}:${DateTime.toEpochMillis(song.updatedAt)}`}
Route-->>SongDetail: Force remount on update
Note over App,ChatNav: ChatWorkspace: derived state replaces useEffect sync
ChatWorkspace->>ChatWorkspace: selectedChannelId = requestedChannelId ?? internalSelectedChannelId
Note over ChatWorkspace: CHANGED: Removed two useEffects, replaced with derived computation
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
|
|
||
| return ( | ||
| <SongDetail | ||
| key={`${song.id}:${DateTime.toEpochMillis(song.updatedAt)}`} |
There was a problem hiding this comment.
P1: Saving an edit can immediately remount SongDetail before the RPC resolves because the optimistic edit changes updatedAt; the new instance recreates isSaving/isSavingRef as false, allowing overlapping edits, and failure updates target the discarded instance so the save error can be lost. A stable component identity plus explicit synchronization of server changes would preserve the in-flight save state without reintroducing stale drafts.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/routes/shows/$showId/setlist/$songId.tsx, line 75:
<comment>Saving an edit can immediately remount `SongDetail` before the RPC resolves because the optimistic edit changes `updatedAt`; the new instance recreates `isSaving`/`isSavingRef` as false, allowing overlapping edits, and failure updates target the discarded instance so the save error can be lost. A stable component identity plus explicit synchronization of server changes would preserve the in-flight save state without reintroducing stale drafts.</comment>
<file context>
@@ -110,6 +72,7 @@ function RouteComponent() {
return (
<SongDetail
+ key={`${song.id}:${DateTime.toEpochMillis(song.updatedAt)}`}
showId={typedShowId}
song={song}
</file context>
| const nextName = (next?.name ?? name).trim(); | ||
| const nextArtist = (next?.artist ?? artist).trim(); | ||
| if (blockUi && isSavingRef.current) return false; | ||
| const activeMixIds = new Set(mixes.map((mix) => mix.id)); |
There was a problem hiding this comment.
P1: Editing a song while mixes or microphones are still loading can silently clear every saved assignment and microphone override. The empty fallback lists should not be treated as authoritative; preserving the existing payload or deferring saves until both lists are loaded avoids this data loss.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/components/songs/SongDetail.tsx, line 106:
<comment>Editing a song while mixes or microphones are still loading can silently clear every saved assignment and microphone override. The empty fallback lists should not be treated as authoritative; preserving the existing payload or deferring saves until both lists are loaded avoids this data loss.</comment>
<file context>
@@ -0,0 +1,272 @@
+ const nextName = (next?.name ?? name).trim();
+ const nextArtist = (next?.artist ?? artist).trim();
+ if (blockUi && isSavingRef.current) return false;
+ const activeMixIds = new Set(mixes.map((mix) => mix.id));
+ const activeMicrophoneIds = new Set(microphones.map((microphone) => microphone.id));
+ const normalizedAssignments = (next?.assignments ?? assignments).flatMap((assignment) => {
</file context>
| variant="destructive" | ||
| size="icon-sm" | ||
| aria-label={`Delete microphone ${number}`} | ||
| disabled={microphone.pending} |
There was a problem hiding this comment.
P2: An optimistic microphone can still be edited while its create mutation is pending, so these controls can submit edits against the temporary ID and show avoidable failures or conflicting optimistic state. The pending guard would be more reliable if it also disabled the number/name inputs, the popover trigger, and color buttons, as MixCard does.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/components/microphones/MicrophoneCard.tsx, line 89:
<comment>An optimistic microphone can still be edited while its create mutation is pending, so these controls can submit edits against the temporary ID and show avoidable failures or conflicting optimistic state. The pending guard would be more reliable if it also disabled the number/name inputs, the popover trigger, and color buttons, as `MixCard` does.</comment>
<file context>
@@ -0,0 +1,182 @@
+ variant="destructive"
+ size="icon-sm"
+ aria-label={`Delete microphone ${number}`}
+ disabled={microphone.pending}
+ onClick={onDelete}
+ className="absolute top-2 right-2 md:opacity-0 md:transition-opacity md:group-hover/card:opacity-100 md:focus-visible:opacity-100"
</file context>
|
|
||
| return ( | ||
| <Dialog open={open} onOpenChange={onOpenChange}> | ||
| <Dialog open={open} onOpenChange={changeOpen}> |
There was a problem hiding this comment.
P2: After a successful preset send, reopening the dialog shows the previously used preset instead of the preset list because the close callback bypasses changeOpen and the reset effect is gone. Routing this close through changeOpen(false) (and preserving a reset for externally controlled closes) would keep dialog sessions predictable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/components/chats/ChatPresetDialog.tsx, line 145:
<comment>After a successful preset send, reopening the dialog shows the previously used preset instead of the preset list because the close callback bypasses `changeOpen` and the reset effect is gone. Routing this close through `changeOpen(false)` (and preserving a reset for externally controlled closes) would keep dialog sessions predictable.</comment>
<file context>
@@ -142,7 +142,7 @@ export function ChatPresetDialog({
return (
- <Dialog open={open} onOpenChange={onOpenChange}>
+ <Dialog open={open} onOpenChange={changeOpen}>
<DialogContent className="sm:max-w-xl">
<DialogHeader>
</file context>
| return parts.map((part) => { | ||
| const key = `${part.type}:${characterOffset}:${part.text}`; |
There was a problem hiding this comment.
P2: Empty adjacent parts can still produce duplicate React keys, so React may warn and reconcile these sibling spans unpredictably. Including the part index as a uniqueness fallback (or otherwise assigning an ID) would preserve unique keys for all contract-valid ChatMessagePart arrays.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/components/chats/ChatMessageBody.tsx, line 15:
<comment>Empty adjacent parts can still produce duplicate React keys, so React may warn and reconcile these sibling spans unpredictably. Including the part index as a uniqueness fallback (or otherwise assigning an ID) would preserve unique keys for all contract-valid `ChatMessagePart` arrays.</comment>
<file context>
@@ -11,14 +11,17 @@ export function ChatMessageBody({
- return parts.map((part, index) => {
- if (part.type === "text") return <span key={index}>{part.text}</span>;
+ let characterOffset = 0;
+ return parts.map((part) => {
+ const key = `${part.type}:${characterOffset}:${part.text}`;
+ characterOffset += part.text.length;
</file context>
| return parts.map((part) => { | |
| const key = `${part.type}:${characterOffset}:${part.text}`; | |
| return parts.map((part, index) => { | |
| const key = `${part.type}:${characterOffset}:${part.text}:${index}`; |
| DialogTitle, | ||
| } from "@/components/ui/dialog"; | ||
|
|
||
| export function MixDeleteDialog({ |
There was a problem hiding this comment.
P3: The mix delete workflow now duplicates the existing microphone delete dialog, so fixes to shared deletion behavior can diverge between these screens. A shared resource-agnostic delete dialog or hook would keep the two flows consistent.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/components/mixes/MixDeleteDialog.tsx, line 23:
<comment>The mix delete workflow now duplicates the existing microphone delete dialog, so fixes to shared deletion behavior can diverge between these screens. A shared resource-agnostic delete dialog or hook would keep the two flows consistent.</comment>
<file context>
@@ -0,0 +1,77 @@
+ DialogTitle,
+} from "@/components/ui/dialog";
+
+export function MixDeleteDialog({
+ mix,
+ showId,
</file context>
| | { readonly type: "copied" } | ||
| | { readonly type: "error"; readonly message: string }; | ||
|
|
||
| const reducePairClientState = ( |
There was a problem hiding this comment.
P3: Pairing selection and reset behavior can regress without a focused test for the new reducer, especially around preserving a valid selection versus clearing the QR/copy state. Adding reducer-level tests for loaded, select, reset, and error transitions would make this refactor safer to maintain.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/components/connections/ConnectionDialog.tsx, line 94:
<comment>Pairing selection and reset behavior can regress without a focused test for the new reducer, especially around preserving a valid selection versus clearing the QR/copy state. Adding reducer-level tests for `loaded`, `select`, `reset`, and error transitions would make this refactor safer to maintain.</comment>
<file context>
@@ -56,11 +56,70 @@ import {
+ | { readonly type: "copied" }
+ | { readonly type: "error"; readonly message: string };
+
+const reducePairClientState = (
+ state: PairClientState,
+ action: PairClientAction,
</file context>
| }; | ||
|
|
||
| const trimmedNumber = number.trim(); | ||
| const duplicate = mixes.some((other) => other.id !== mix.id && other.number === trimmedNumber); |
There was a problem hiding this comment.
P3: The duplicate-label warning misses existing mix labels with surrounding whitespace because trimmedNumber is normalized but other.number is not. Comparing other.number.trim() would match the normalization performed by commitNumber and the existing microphone UI.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/components/mixes/MixCard.tsx, line 71:
<comment>The duplicate-label warning misses existing mix labels with surrounding whitespace because `trimmedNumber` is normalized but `other.number` is not. Comparing `other.number.trim()` would match the normalization performed by `commitNumber` and the existing microphone UI.</comment>
<file context>
@@ -0,0 +1,189 @@
+ };
+
+ const trimmedNumber = number.trim();
+ const duplicate = mixes.some((other) => other.id !== mix.id && other.number === trimmedNumber);
+ const commitNumber = async () => {
+ const valid = trimmedNumber || mix.number;
</file context>
| const duplicate = mixes.some((other) => other.id !== mix.id && other.number === trimmedNumber); | |
| const duplicate = mixes.some((other) => other.id !== mix.id && other.number.trim() === trimmedNumber); |
| <div className="grid gap-3 md:grid-cols-2"> | ||
| {orderedMixes.map((mix, index) => { | ||
| const assignment = assignments.find((item) => item.mixId === mix.id); | ||
| const selected = new Set(assignment?.microphoneIds ?? []); |
There was a problem hiding this comment.
P3: The assignment count can include microphones that have since been deleted, causing the UI to display e.g. “1 mic on” while every visible microphone is unselected. Filtering assignment IDs against the current microphones list before constructing selected keeps the count consistent with the controls rendered below.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/components/songs/SongMixAssignments.tsx, line 66:
<comment>The assignment count can include microphones that have since been deleted, causing the UI to display e.g. “1 mic on” while every visible microphone is unselected. Filtering assignment IDs against the current `microphones` list before constructing `selected` keeps the count consistent with the controls rendered below.</comment>
<file context>
@@ -0,0 +1,154 @@
+ <div className="grid gap-3 md:grid-cols-2">
+ {orderedMixes.map((mix, index) => {
+ const assignment = assignments.find((item) => item.mixId === mix.id);
+ const selected = new Set(assignment?.microphoneIds ?? []);
+ return (
+ <Card
</file context>
| const selected = new Set(assignment?.microphoneIds ?? []); | |
| const selected = new Set( | |
| assignment?.microphoneIds.filter((id) => microphones.some((microphone) => microphone.id === id)) ?? [], | |
| ); |
There was a problem hiding this comment.
11 issues found across 34 files
Confidence score: 2/5
- The highest-risk pattern is optimistic UI unmounting components before mutations settle in
apps/web/src/components/songs/SongDeleteDialog.tsxandapps/web/src/routes/shows/$showId/mixes.tsx: failed delete/edit paths lose their error and rollback handlers becauseSongDetail/MixCardare gone, so users can see silent failures or stale state — keep failure UI/rollback state mounted until the request resolves (or decouple keys from optimistic timestamps). - Deletion dialogs remain dismissible while requests are in flight in
apps/web/src/components/microphones/MicrophoneDeleteDialog.tsx,apps/web/src/components/mixes/MixDeleteDialog.tsx, andapps/web/src/components/songs/SongDeleteDialog.tsx, which can route callbacks to a now-closed/new dialog and trigger unexpected close/navigation behavior — block X/Escape/backdrop dismissal until the mutation completes. apps/web/src/components/microphones/MicrophoneCard.tsxallows edits whilemicrophone.pending, so submits can use a temporary ID and fail with “Microphone not found,” creating a clear create-then-edit regression for new items — disable editable fields until creation finishes.- Concurrent/local-edit synchronization looks fragile in
apps/web/src/components/songs/SongDetail.tsxandapps/web/src/components/songs/MicrophoneName.tsx: out-of-order full-record saves and effect-driven value resets can overwrite newer microphone-name input, leading to lost user edits — serialize/patch saves and avoid syncing remote value while actively editing.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/web/src/components/mixes/MixDeleteDialog.tsx">
<violation number="1" location="apps/web/src/components/mixes/MixDeleteDialog.tsx:58">
P2: An in-flight removal can still be dismissed through the dialog's X, Escape, or backdrop. Reopening another mix before the first request resolves lets that first success call `onClose()` and close the newer confirmation; keep the controlled dialog open while `isDeleting`.</violation>
</file>
<file name="apps/web/src/components/microphones/MicrophoneDeleteDialog.tsx">
<violation number="1" location="apps/web/src/components/microphones/MicrophoneDeleteDialog.tsx:23">
P3: Microphone and mix removal flows now have near-identical dialog implementations, so behavior fixes (including pending-close handling) must be made twice. A shared confirmation dialog or shared delete-flow hook would keep loading/error handling consistent while callers supply entity labels and mutation callbacks.</violation>
<violation number="2" location="apps/web/src/components/microphones/MicrophoneDeleteDialog.tsx:60">
P2: Closing via the title-bar X while removal is pending hides the dialog but does not cancel `deleteMicrophone`; a failed request then sets an error in a newly closed dialog. Keep the dialog open until the in-flight deletion resolves, matching the disabled Cancel button's behavior.</violation>
</file>
<file name="apps/web/src/components/songs/SongDeleteDialog.tsx">
<violation number="1" location="apps/web/src/components/songs/SongDeleteDialog.tsx:45">
P1: A rejected delete cannot show this error: optimistic removal unmounts `SongDetail` and this dialog before `deleteSong` resolves. Keep failure UI mounted outside the detail route or retain the detail while the mutation settles, so users can retry instead of only seeing “Song not found.”</violation>
<violation number="2" location="apps/web/src/components/songs/SongDeleteDialog.tsx:55">
P2: While deletion is pending, the dialog can still be closed with its X button (and other dialog dismissal gestures), then successful completion forces navigation to the setlist. Prevent dismissal while `isDeleting`, or cancel/ignore the pending completion after dismissal, so closing the dialog does not unexpectedly override navigation.</violation>
</file>
<file name="apps/web/src/routes/shows/$showId/mixes.tsx">
<violation number="1" location="apps/web/src/routes/shows/$showId/mixes.tsx:64">
P2: A failed mix edit no longer reports its error: the optimistic edit changes `updatedAt` immediately, which changes this key and unmounts `MixCard` before `edit()` resolves, so `setSaveError` and the rollback setters run against the discarded instance. Keeping the card mounted through the mutation and synchronizing/resetting its draft state after the result would preserve the failure feedback.</violation>
</file>
<file name="apps/web/src/components/microphones/MicrophoneCard.tsx">
<violation number="1" location="apps/web/src/components/microphones/MicrophoneCard.tsx:76">
P2: On medium-and-larger screens, the delete control remains transparent on mouse hover because no `group/card` parent exists. Add the named group class to this card so the visible-on-hover affordance works.</violation>
<violation number="2" location="apps/web/src/components/microphones/MicrophoneCard.tsx:110">
P2: Editing a newly added microphone before its create request finishes submits its temporary ID and fails with “Microphone not found.” Disable number, color, and name editing while `microphone.pending`, as `MixCard` does.</violation>
</file>
<file name="apps/web/src/components/songs/SongDetail.tsx">
<violation number="1" location="apps/web/src/components/songs/SongDetail.tsx:180">
P2: Rapidly renaming two microphones can lose the later override: this non-blocking full-record save may complete after a newer save and overwrite its `microphoneNames`. Serialize these edits (or make the mutation patch a single override) so requests cannot apply stale snapshots out of order.</violation>
</file>
<file name="apps/web/src/components/songs/MicrophoneName.tsx">
<violation number="1" location="apps/web/src/components/songs/MicrophoneName.tsx:22">
P2: An incoming microphone-name update can erase text currently being typed because this effect resets `value` even while `editing` is true. Sync the displayed value only outside edit mode so the local draft remains stable until blur/Escape.</violation>
</file>
<file name="apps/web/src/components/connections/ConnectToShowtime.tsx">
<violation number="1" location="apps/web/src/components/connections/ConnectToShowtime.tsx:41">
P2: Dialog snaps shut instead of animating out. Changing `key` on every `connectOpen` transition causes React to immediately unmount the dialog when closing, which interrupts the CSS close animation (`data-closed:animate-out`, `data-closed:fade-out-0`, `data-closed:zoom-out-95`, 100ms duration) defined in the Base UI dialog overlay and content.</violation>
</file>
Architecture diagram
sequenceDiagram
participant App as main.tsx (App Root)
participant ChatNav as ChatNavigation module
participant ChatDrawer as ChatDrawer component
participant ChatPreset as ChatPresetDialog
participant PairClient as PairClientDialog (ConnectionDialog)
participant SongDet as SongDetail component
participant MicroList as Microphones route
participant MixList as Mixes route
participant Item as Item/ItemGroup UI
participant NotifCenter as NotificationCenter
Note over App,NotifCenter: NEW: Chat navigation configuration moved to app root
App->>ChatNav: NEW: configureChatNavigation({getActiveShowId, navigateToShow})
ChatNav->>ChatNav: creates/assigns singleton chatNavigation
Note over ChatNav,ChatDrawer: CHANGED: ChatDrawer uses useEffectEvent for stable callback
ChatDrawer->>ChatDrawer: CHANGED: wraps consumeChatOpenRequest in useEffectEvent
ChatDrawer->>ChatDrawer: CHANGED: empty dependency array on useEffect (no reactivity to showId)
Note over ChatPreset: CHANGED: Dialog reset on close via onOpenChange wrapper
ChatPreset->>ChatPreset: back() on close → reset mode to "list"
Note over PairClient: CHANGED: useReducer replaces multiple useState calls
PairClient->>PairClient: dispatch({type:"reset"}) on mount
loop polling
PairClient->>PairClient: CHANGED: dispatch({type:"loaded", ...}) with aggregated state
alt failure
PairClient->>PairClient: dispatch({type:"error", message})
end
end
PairClient->>PairClient: dispatch({type:"prepare-qr-code"}) before QR generation
Note over SongDet: CHANGED: useReducer for song detail state
SongDet->>SongDet: updatedAt key forces remount on data change
SongDet->>SongDet: try/finally around async edit operations
alt RPC failure
SongDet->>SongDet: restore to original song state
end
Note over MicroList,MixList: EXTRACTED: MicrophoneCard, MixCard to separate components
MicroList->>MicroList: key={`${id}:${updatedAt}`}
MixList->>MixList: key={`${id}:${updatedAt}`}
Note over Item: CHANGED: semantic HTML (ul/li replaces role=list)
Item->>Item: CHANGED: aria attributes (aria-label on Switch)
Note over NotifCenter: REMOVED: subscribeNotifications listener set
NotifCenter->>NotifCenter: CHANGED: no longer iterates listeners on publish
Note over PairClient,SongDet: CHANGED: all async mutations wrapped in try/finally
PairClient->>PairClient: try/finally ensures setSaving(false) on error or success
SongDet->>SongDet: try/finally prevents stuck saving state
Note over App,NotifCenter: KEY: no stale closure bugs due to useEffectEvent/reducer patterns
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
| reactivityKeys: songsRpcReactivityKey(showId), | ||
| }); | ||
| if (Exit.isFailure(result)) { | ||
| setError(rpcErrorMessageFromCause(result.cause)); |
There was a problem hiding this comment.
P1: A rejected delete cannot show this error: optimistic removal unmounts SongDetail and this dialog before deleteSong resolves. Keep failure UI mounted outside the detail route or retain the detail while the mutation settles, so users can retry instead of only seeing “Song not found.”
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/components/songs/SongDeleteDialog.tsx, line 45:
<comment>A rejected delete cannot show this error: optimistic removal unmounts `SongDetail` and this dialog before `deleteSong` resolves. Keep failure UI mounted outside the detail route or retain the detail while the mutation settles, so users can retry instead of only seeing “Song not found.”</comment>
<file context>
@@ -0,0 +1,79 @@
+ reactivityKeys: songsRpcReactivityKey(showId),
+ });
+ if (Exit.isFailure(result)) {
+ setError(rpcErrorMessageFromCause(result.cause));
+ return;
+ }
</file context>
| const label = mix?.name?.trim() ? `“${mix.name}” (${mix.number})` : `mix ${mix?.number ?? ""}`; | ||
|
|
||
| return ( | ||
| <Dialog open={mix !== undefined} onOpenChange={(open) => !open && onClose()}> |
There was a problem hiding this comment.
P2: An in-flight removal can still be dismissed through the dialog's X, Escape, or backdrop. Reopening another mix before the first request resolves lets that first success call onClose() and close the newer confirmation; keep the controlled dialog open while isDeleting.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/components/mixes/MixDeleteDialog.tsx, line 58:
<comment>An in-flight removal can still be dismissed through the dialog's X, Escape, or backdrop. Reopening another mix before the first request resolves lets that first success call `onClose()` and close the newer confirmation; keep the controlled dialog open while `isDeleting`.</comment>
<file context>
@@ -0,0 +1,80 @@
+ const label = mix?.name?.trim() ? `“${mix.name}” (${mix.number})` : `mix ${mix?.number ?? ""}`;
+
+ return (
+ <Dialog open={mix !== undefined} onOpenChange={(open) => !open && onClose()}>
+ <DialogContent>
+ <DialogHeader>
</file context>
| : `microphone ${microphone?.number ?? ""}`; | ||
|
|
||
| return ( | ||
| <Dialog open={microphone !== undefined} onOpenChange={(open) => !open && onClose()}> |
There was a problem hiding this comment.
P2: Closing via the title-bar X while removal is pending hides the dialog but does not cancel deleteMicrophone; a failed request then sets an error in a newly closed dialog. Keep the dialog open until the in-flight deletion resolves, matching the disabled Cancel button's behavior.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/components/microphones/MicrophoneDeleteDialog.tsx, line 60:
<comment>Closing via the title-bar X while removal is pending hides the dialog but does not cancel `deleteMicrophone`; a failed request then sets an error in a newly closed dialog. Keep the dialog open until the in-flight deletion resolves, matching the disabled Cancel button's behavior.</comment>
<file context>
@@ -0,0 +1,84 @@
+ : `microphone ${microphone?.number ?? ""}`;
+
+ return (
+ <Dialog open={microphone !== undefined} onOpenChange={(open) => !open && onClose()}>
+ <DialogContent>
+ <DialogHeader>
</file context>
| } | ||
| }; | ||
| return ( | ||
| <Dialog open={open} onOpenChange={onOpenChange}> |
There was a problem hiding this comment.
P2: While deletion is pending, the dialog can still be closed with its X button (and other dialog dismissal gestures), then successful completion forces navigation to the setlist. Prevent dismissal while isDeleting, or cancel/ignore the pending completion after dismissal, so closing the dialog does not unexpectedly override navigation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/components/songs/SongDeleteDialog.tsx, line 55:
<comment>While deletion is pending, the dialog can still be closed with its X button (and other dialog dismissal gestures), then successful completion forces navigation to the setlist. Prevent dismissal while `isDeleting`, or cancel/ignore the pending completion after dismissal, so closing the dialog does not unexpectedly override navigation.</comment>
<file context>
@@ -0,0 +1,79 @@
+ }
+ };
+ return (
+ <Dialog open={open} onOpenChange={onOpenChange}>
+ <DialogContent>
+ <DialogHeader>
</file context>
| {mixes.map((mix) => ( | ||
| <MixCard | ||
| key={mix.id} | ||
| key={`${mix.id}:${DateTime.toEpochMillis(mix.updatedAt)}`} |
There was a problem hiding this comment.
P2: A failed mix edit no longer reports its error: the optimistic edit changes updatedAt immediately, which changes this key and unmounts MixCard before edit() resolves, so setSaveError and the rollback setters run against the discarded instance. Keeping the card mounted through the mutation and synchronizing/resetting its draft state after the result would preserve the failure feedback.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/routes/shows/$showId/mixes.tsx, line 64:
<comment>A failed mix edit no longer reports its error: the optimistic edit changes `updatedAt` immediately, which changes this key and unmounts `MixCard` before `edit()` resolves, so `setSaveError` and the rollback setters run against the discarded instance. Keeping the card mounted through the mutation and synchronizing/resetting its draft state after the result would preserve the failure feedback.</comment>
<file context>
@@ -83,7 +61,7 @@ function RouteComponent() {
{mixes.map((mix) => (
<MixCard
- key={mix.id}
+ key={`${mix.id}:${DateTime.toEpochMillis(mix.updatedAt)}`}
mix={mix}
mixes={mixes}
</file context>
|
|
||
| const colors = microphoneColorClassNames[color]; | ||
| return ( | ||
| <Card className="relative"> |
There was a problem hiding this comment.
P2: On medium-and-larger screens, the delete control remains transparent on mouse hover because no group/card parent exists. Add the named group class to this card so the visible-on-hover affordance works.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/components/microphones/MicrophoneCard.tsx, line 76:
<comment>On medium-and-larger screens, the delete control remains transparent on mouse hover because no `group/card` parent exists. Add the named group class to this card so the visible-on-hover affordance works.</comment>
<file context>
@@ -0,0 +1,175 @@
+
+ const colors = microphoneColorClassNames[color];
+ return (
+ <Card className="relative">
+ <Button
+ type="button"
</file context>
| ...(override ? [{ microphoneId: microphone.id, name: override }] : []), | ||
| ]; | ||
| update({ microphoneNames: nextMicrophoneNames }); | ||
| void save({ microphoneNames: nextMicrophoneNames }, false); |
There was a problem hiding this comment.
P2: Rapidly renaming two microphones can lose the later override: this non-blocking full-record save may complete after a newer save and overwrite its microphoneNames. Serialize these edits (or make the mutation patch a single override) so requests cannot apply stale snapshots out of order.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/components/songs/SongDetail.tsx, line 180:
<comment>Rapidly renaming two microphones can lose the later override: this non-blocking full-record save may complete after a newer save and overwrite its `microphoneNames`. Serialize these edits (or make the mutation patch a single override) so requests cannot apply stale snapshots out of order.</comment>
<file context>
@@ -0,0 +1,272 @@
+ ...(override ? [{ microphoneId: microphone.id, name: override }] : []),
+ ];
+ update({ microphoneNames: nextMicrophoneNames });
+ void save({ microphoneNames: nextMicrophoneNames }, false);
+ };
+
</file context>
| const [editing, setEditing] = React.useState(false); | ||
| const [value, setValue] = React.useState(displayedName); | ||
|
|
||
| React.useEffect(() => setValue(displayedName), [displayedName]); |
There was a problem hiding this comment.
P2: An incoming microphone-name update can erase text currently being typed because this effect resets value even while editing is true. Sync the displayed value only outside edit mode so the local draft remains stable until blur/Escape.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/components/songs/MicrophoneName.tsx, line 22:
<comment>An incoming microphone-name update can erase text currently being typed because this effect resets `value` even while `editing` is true. Sync the displayed value only outside edit mode so the local draft remains stable until blur/Escape.</comment>
<file context>
@@ -0,0 +1,65 @@
+ const [editing, setEditing] = React.useState(false);
+ const [value, setValue] = React.useState(displayedName);
+
+ React.useEffect(() => setValue(displayedName), [displayedName]);
+
+ if (!editing) {
</file context>
| </Empty> | ||
| <ConnectionLinkDialog open={connectOpen} onOpenChange={setConnectOpen} /> | ||
| <ConnectionLinkDialog | ||
| key={connectOpen ? "open" : "closed"} |
There was a problem hiding this comment.
P2: Dialog snaps shut instead of animating out. Changing key on every connectOpen transition causes React to immediately unmount the dialog when closing, which interrupts the CSS close animation (data-closed:animate-out, data-closed:fade-out-0, data-closed:zoom-out-95, 100ms duration) defined in the Base UI dialog overlay and content.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/components/connections/ConnectToShowtime.tsx, line 41:
<comment>Dialog snaps shut instead of animating out. Changing `key` on every `connectOpen` transition causes React to immediately unmount the dialog when closing, which interrupts the CSS close animation (`data-closed:animate-out`, `data-closed:fade-out-0`, `data-closed:zoom-out-95`, 100ms duration) defined in the Base UI dialog overlay and content.</comment>
<file context>
@@ -37,7 +37,11 @@ export function ConnectToShowtime({ error }: { readonly error?: string }) {
</Empty>
- <ConnectionLinkDialog open={connectOpen} onOpenChange={setConnectOpen} />
+ <ConnectionLinkDialog
+ key={connectOpen ? "open" : "closed"}
+ open={connectOpen}
+ onOpenChange={setConnectOpen}
</file context>
| DialogTitle, | ||
| } from "@/components/ui/dialog"; | ||
|
|
||
| export function MicrophoneDeleteDialog({ |
There was a problem hiding this comment.
P3: Microphone and mix removal flows now have near-identical dialog implementations, so behavior fixes (including pending-close handling) must be made twice. A shared confirmation dialog or shared delete-flow hook would keep loading/error handling consistent while callers supply entity labels and mutation callbacks.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/components/microphones/MicrophoneDeleteDialog.tsx, line 23:
<comment>Microphone and mix removal flows now have near-identical dialog implementations, so behavior fixes (including pending-close handling) must be made twice. A shared confirmation dialog or shared delete-flow hook would keep loading/error handling consistent while callers supply entity labels and mutation callbacks.</comment>
<file context>
@@ -0,0 +1,84 @@
+ DialogTitle,
+} from "@/components/ui/dialog";
+
+export function MicrophoneDeleteDialog({
+ microphone,
+ showId,
</file context>
Summary
Why
React Doctor identified state synchronization and render-time patterns that could produce stale UI, unnecessary subscriptions, extra renders, or order-dependent initialization. The structural extractions keep those fixes maintainable and reduce the risk of future regressions in show-critical workflows.
Validation
npx react-doctor@latest . --verbose— 100/100, no issues foundvp check --fix— formatting, lint, and type checks passvp check --no-fmt— full repository lint and type checks passvp run -r test— all monorepo tests passpnpm --filter @showtime/web build— passespnpm --filter @showtime/desktop build— passes, including Windows installer packaging