fix: add loading/error states to Enhancer results panel#1221
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to improve the AI Enhancer results panel UX by adding explicit per-result loading and error states during cover/repaint generation.
Changes:
- Extend
ResultEntrywithstatus(generating | ready | error) and optionalerrormessage. - Wrap cover/repaint generation flows to update result entries based on success/failure.
- Update the results list UI to show a spinner while generating and display error details on failure.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1030,14 +1056,17 @@ export function EnhancePanel() { | |||
| <svg className="w-3 h-3" viewBox="0 0 24 24" fill="currentColor"><path d="M8 5v14l11-7z" /></svg> | |||
| )} | |||
| </button> | |||
| )} | |||
| <div className="flex-1 min-w-0"> | |||
| <p className="text-[11px] text-zinc-300 truncate"> | |||
| <p className={`text-[11px] truncate ${r.status === 'error' ? 'text-red-400' : 'text-zinc-300'}`}> | |||
| {r.title} | |||
| {canAB && isSelected && ( | |||
| <span className={`ml-1 ${abSide === 'B' ? 'text-violet-400 font-bold' : 'text-zinc-600'}`}>B</span> | |||
| )} | |||
| </p> | |||
| <p className="text-[10px] text-zinc-600">{r.duration}</p> | |||
| <p className="text-[10px] text-zinc-600"> | |||
| {r.status === 'generating' ? 'Generating...' : r.status === 'error' ? (r.error ?? 'Failed') : r.duration} | |||
| </p> | |||
There was a problem hiding this comment.
There are existing component tests for EnhancePanel, but none currently assert the new ResultEntry status transitions/UI (spinner while generating, error message on failure, ready state after finalize). Please add tests that mock a failed generation outcome and verify the panel updates the corresponding result row (and likewise for the generating → ready path).
| try { | ||
| await generateCoverClip({ | ||
| clipId: enhancerTarget.clipId, | ||
| caption, | ||
| lyrics, | ||
| coverStrength, | ||
| createNew, | ||
| }); | ||
| await finalizeResult(resultId, enhancerTarget.clipId); |
There was a problem hiding this comment.
The generation pipeline helpers (generateCoverClip) don’t throw on failure (they return early / return false inside withGenerationToast). This try/catch will therefore miss most failures and the ResultEntry can remain stuck in status='generating' (spinner forever). Consider changing generateCoverClip to return a success flag (and/or target clipId) or throw on failure, and update the result entry to status='error' when the generation outcome is unsuccessful.
| try { | ||
| await generateRepaintClip({ | ||
| clipId: enhancerTarget.clipId, | ||
| repaintStart: selStart, | ||
| repaintEnd: selEnd, | ||
| prompt, | ||
| globalCaption: globalCaption || undefined, | ||
| repaintMode, | ||
| repaintStrength, | ||
| }); | ||
| await finalizeResult(resultId, enhancerTarget.clipId); | ||
| } catch (err) { |
There was a problem hiding this comment.
Same issue as cover: generateRepaintClip typically resolves without throwing even when it fails, so this catch block won’t run and the optimistic result may never transition out of status='generating'. Please propagate success/failure from generateRepaintClip (return/throw) or explicitly check the updated clip/job state after awaiting it, then set status to 'ready' or 'error' accordingly.
| @@ -333,7 +350,7 @@ export function EnhancePanel() { | |||
| const dur = buffer.duration; | |||
| setResults((prev) => prev.map((r) => | |||
| r.id === resultId | |||
| ? { ...r, audioKey, peaks, duration: formatDuration(dur), durationSec: dur } | |||
| ? { ...r, audioKey, peaks, duration: formatDuration(dur), durationSec: dur, status: 'ready' as const } | |||
There was a problem hiding this comment.
finalizeResult is the only place that sets status='ready'. If audioKey is empty or loadBuffer returns null (both paths currently just return), the entry stays in status='generating' indefinitely. Update those early-return paths (and the decode-failure catch) to set status='error' with a helpful message (and consider clearing any previous error when setting status to 'ready').
| r.id === resultId ? { ...r, status: 'error', error: message } : r, | ||
| )); | ||
| } | ||
| }, [enhancerTarget, caption, lyrics, consistency, createNew, isGenerating]); |
There was a problem hiding this comment.
handleCoverGenerate calls finalizeResult, but finalizeResult isn’t included in this useCallback dependency list. Since finalizeResult depends on results.length, this can cause stale closures (e.g. miniPlayerIdx pointing at the wrong entry) and will violate exhaustive-deps if enabled. Include finalizeResult in the deps, or refactor finalizeResult to avoid capturing results.length (e.g. derive the new index from the functional setResults update).
| r.id === resultId ? { ...r, status: 'error', error: message } : r, | ||
| )); | ||
| } | ||
| }, [enhancerTarget, selStart, selEnd, prompt, globalCaption, repaintMode, repaintStrength, isGenerating]); |
There was a problem hiding this comment.
handleRepaintGenerate also uses finalizeResult but doesn’t list it as a dependency, risking stale behavior for the same reasons as cover generation. Add finalizeResult to the dependency array or remove the results.length capture from finalizeResult.
…ion callbacks - Wrap generateCoverClip and generateRepaintClip calls in try/catch - Add status field to ResultEntry: 'generating' | 'ready' | 'error' - Show animated spinner icon for actively generating results - Show error icon with red text and error message for failed results - Set status to 'ready' on successful finalization Previously, unhandled promise rejections left ghost entries with '--:--' duration permanently in the results panel. Closes #1197 https://claude.ai/code/session_01CzJuSwLHvtiJfKFfcXbXhC
- Set status:'error' on all early-return paths in finalizeResult (missing clip, empty audioKey, failed buffer load, audio decode error) - Add local isSubmitting guard to prevent rapid double-clicks - Add tests for generating spinner, error icon/message display https://claude.ai/code/session_01CzJuSwLHvtiJfKFfcXbXhC
Summary
status: 'generating' | 'ready' | 'error'anderror?: stringfields toResultEntryinterfaceTest plan
Closes #1197
https://claude.ai/code/session_01CzJuSwLHvtiJfKFfcXbXhC