Skip to content

Commit 88684b4

Browse files
committed
Fix annotation/discovery save reliability and rubric delete
- Add failed save queue with auto-retry for annotation phase - Add failed save queue with auto-retry for discovery phase - Add navigation debouncing to prevent rapid clicks overwhelming backend - Add visual indicator for pending saves with manual retry option - Add beforeunload warning when there are unsaved annotations/findings - Fix rubric question delete to update UI immediately - Restore error handling in annotation submission endpoint
1 parent 24e2d97 commit 88684b4

4 files changed

Lines changed: 480 additions & 17 deletions

File tree

client/src/pages/AnnotationDemo.tsx

Lines changed: 250 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,21 @@ export function AnnotationDemo() {
9292
const savedStateRef = useRef<Map<string, SavedAnnotationState>>(new Map());
9393
const savingTracesRef = useRef<Set<string>>(new Set()); // Track which traces are currently saving
9494
const isSavingRef = useRef(false); // Track if any user-initiated save is in progress
95+
const lastNavigationTimeRef = useRef<number>(0); // Track last navigation to prevent rapid clicking
96+
const NAVIGATION_DEBOUNCE_MS = 300; // Minimum time between navigations
97+
98+
// Failed save queue for retry mechanism
99+
interface FailedSaveData {
100+
traceId: string;
101+
ratings: Record<string, number>;
102+
freeformResponses: Record<string, string>;
103+
comment: string;
104+
attempts: number;
105+
lastAttempt: number;
106+
}
107+
const failedSaveQueueRef = useRef<Map<string, FailedSaveData>>(new Map());
108+
const [failedSaveCount, setFailedSaveCount] = useState(0);
109+
const retryIntervalRef = useRef<NodeJS.Timeout | null>(null);
95110

96111
// Retry utility with exponential backoff
97112
const retryWithBackoff = useCallback(async <T,>(
@@ -578,8 +593,43 @@ export function AnnotationDemo() {
578593
traceId: targetTraceId,
579594
isBackground
580595
});
581-
// Only show toast for user-initiated saves
582-
if (!isBackground) {
596+
597+
// Queue for retry if this was a background save
598+
if (isBackground) {
599+
const existingEntry = failedSaveQueueRef.current.get(targetTraceId);
600+
const attempts = existingEntry ? existingEntry.attempts + 1 : 1;
601+
602+
// Only add to queue if not already there (avoid duplicates from rapid clicking)
603+
if (!existingEntry) {
604+
failedSaveQueueRef.current.set(targetTraceId, {
605+
traceId: targetTraceId,
606+
ratings: { ...ratingsToSave },
607+
freeformResponses: { ...freeformToSave },
608+
comment: commentToSave,
609+
attempts,
610+
lastAttempt: Date.now()
611+
});
612+
setFailedSaveCount(failedSaveQueueRef.current.size);
613+
614+
// Notify user once when save fails (only for new failures)
615+
toast.warning('Save in progress... will retry automatically', {
616+
duration: 3000,
617+
id: `save-retry-${targetTraceId}` // Prevent duplicate toasts
618+
});
619+
} else {
620+
// Update existing entry with latest data
621+
failedSaveQueueRef.current.set(targetTraceId, {
622+
...existingEntry,
623+
ratings: { ...ratingsToSave },
624+
freeformResponses: { ...freeformToSave },
625+
comment: commentToSave,
626+
attempts,
627+
lastAttempt: Date.now()
628+
});
629+
}
630+
631+
console.warn(`Queued trace ${targetTraceId} for retry (attempt ${attempts})`);
632+
} else {
583633
toast.error('Failed to save annotation. Please try again.');
584634
}
585635
return false;
@@ -592,6 +642,95 @@ export function AnnotationDemo() {
592642
}
593643
}
594644
};
645+
646+
// Process failed save queue - retry one at a time
647+
const processFailedSaveQueue = useCallback(async () => {
648+
if (failedSaveQueueRef.current.size === 0) return;
649+
650+
const now = Date.now();
651+
const entries = Array.from(failedSaveQueueRef.current.entries());
652+
653+
for (const [traceId, data] of entries) {
654+
// Skip if attempted too recently (wait at least 5 seconds between retries)
655+
if (now - data.lastAttempt < 5000) continue;
656+
657+
// Skip if max attempts reached (10 attempts max)
658+
if (data.attempts >= 10) {
659+
console.error(`Max retry attempts reached for trace ${traceId}, removing from queue`);
660+
failedSaveQueueRef.current.delete(traceId);
661+
setFailedSaveCount(failedSaveQueueRef.current.size);
662+
continue;
663+
}
664+
665+
console.log(`Retrying failed save for trace ${traceId} (attempt ${data.attempts + 1})`);
666+
667+
// Update last attempt time
668+
data.lastAttempt = now;
669+
data.attempts += 1;
670+
671+
try {
672+
const numericRatings = Object.fromEntries(
673+
Object.entries(data.ratings).filter(([_, v]) => typeof v === 'number')
674+
);
675+
676+
// Calculate legacy rating
677+
let legacyRating = 3;
678+
for (const question of rubricQuestions) {
679+
if (question.judgeType === 'likert') {
680+
const rating = data.ratings[question.id];
681+
if (typeof rating === 'number' && rating >= 1 && rating <= 5) {
682+
legacyRating = rating;
683+
break;
684+
}
685+
}
686+
}
687+
688+
const annotationData = {
689+
trace_id: traceId,
690+
user_id: currentUserId,
691+
rating: legacyRating,
692+
ratings: numericRatings,
693+
comment: buildCombinedComment(data.comment, data.freeformResponses)
694+
};
695+
696+
await submitAnnotation.mutateAsync(annotationData);
697+
698+
// Success! Remove from queue
699+
failedSaveQueueRef.current.delete(traceId);
700+
setFailedSaveCount(failedSaveQueueRef.current.size);
701+
setSubmittedAnnotations(prev => new Set([...prev, traceId]));
702+
703+
// Update saved state
704+
savedStateRef.current.set(traceId, {
705+
ratings: { ...data.ratings },
706+
freeformResponses: { ...data.freeformResponses },
707+
comment: data.comment
708+
});
709+
710+
console.log(`Successfully saved queued annotation for trace ${traceId}`);
711+
712+
// Only process one at a time to avoid overwhelming the backend
713+
break;
714+
} catch (error) {
715+
console.error(`Retry failed for trace ${traceId}:`, error);
716+
// Will be retried on next interval
717+
}
718+
}
719+
}, [rubricQuestions, currentUserId, submitAnnotation, buildCombinedComment]);
720+
721+
// Set up periodic retry for failed saves
722+
useEffect(() => {
723+
// Run retry every 5 seconds
724+
retryIntervalRef.current = setInterval(() => {
725+
processFailedSaveQueue();
726+
}, 5000);
727+
728+
return () => {
729+
if (retryIntervalRef.current) {
730+
clearInterval(retryIntervalRef.current);
731+
}
732+
};
733+
}, [processFailedSaveQueue]);
595734

596735
const handleSubmitAnnotation = async () => {
597736
await saveAnnotation();
@@ -613,6 +752,17 @@ export function AnnotationDemo() {
613752
return; // Prevent concurrent navigation
614753
}
615754

755+
// Debounce rapid clicks to prevent overwhelming the backend
756+
const now = Date.now();
757+
if (now - lastNavigationTimeRef.current < NAVIGATION_DEBOUNCE_MS) {
758+
console.warn('nextTrace: Navigation debounced', {
759+
timeSinceLastNav: now - lastNavigationTimeRef.current,
760+
debounceMs: NAVIGATION_DEBOUNCE_MS
761+
});
762+
return;
763+
}
764+
lastNavigationTimeRef.current = now;
765+
616766
// Store current trace data for save
617767
const currentTraceId = currentTrace.id;
618768
const ratingsToSave = { ...currentRatings };
@@ -699,6 +849,17 @@ export function AnnotationDemo() {
699849
return; // Prevent concurrent navigation
700850
}
701851

852+
// Debounce rapid clicks to prevent overwhelming the backend
853+
const now = Date.now();
854+
if (now - lastNavigationTimeRef.current < NAVIGATION_DEBOUNCE_MS) {
855+
console.warn('prevTrace: Navigation debounced', {
856+
timeSinceLastNav: now - lastNavigationTimeRef.current,
857+
debounceMs: NAVIGATION_DEBOUNCE_MS
858+
});
859+
return;
860+
}
861+
lastNavigationTimeRef.current = now;
862+
702863
// Check if we can navigate
703864
if (currentTraceIndex <= 0) {
704865
console.log('prevTrace: Already at first trace');
@@ -755,6 +916,80 @@ export function AnnotationDemo() {
755916
// Navigation is now optimistic, so we don't block on isSaving
756917
const isNextDisabled = !canAnnotate || Object.keys(currentRatings).length === 0 || isNavigating;
757918

919+
// Warn user before leaving if there are pending saves
920+
useEffect(() => {
921+
const handleBeforeUnload = (e: BeforeUnloadEvent) => {
922+
if (failedSaveQueueRef.current.size > 0) {
923+
e.preventDefault();
924+
e.returnValue = 'You have unsaved annotations. Are you sure you want to leave?';
925+
return e.returnValue;
926+
}
927+
};
928+
929+
window.addEventListener('beforeunload', handleBeforeUnload);
930+
return () => window.removeEventListener('beforeunload', handleBeforeUnload);
931+
}, []);
932+
933+
// Manual retry all failed saves
934+
const retryAllFailedSaves = async () => {
935+
if (failedSaveQueueRef.current.size === 0) return;
936+
937+
toast.info(`Retrying ${failedSaveQueueRef.current.size} unsaved annotations...`);
938+
939+
// Process all entries (not just one)
940+
const entries = Array.from(failedSaveQueueRef.current.entries());
941+
let successCount = 0;
942+
943+
for (const [traceId, data] of entries) {
944+
try {
945+
const numericRatings = Object.fromEntries(
946+
Object.entries(data.ratings).filter(([_, v]) => typeof v === 'number')
947+
);
948+
949+
let legacyRating = 3;
950+
for (const question of rubricQuestions) {
951+
if (question.judgeType === 'likert') {
952+
const rating = data.ratings[question.id];
953+
if (typeof rating === 'number' && rating >= 1 && rating <= 5) {
954+
legacyRating = rating;
955+
break;
956+
}
957+
}
958+
}
959+
960+
const annotationData = {
961+
trace_id: traceId,
962+
user_id: currentUserId,
963+
rating: legacyRating,
964+
ratings: numericRatings,
965+
comment: buildCombinedComment(data.comment, data.freeformResponses)
966+
};
967+
968+
await submitAnnotation.mutateAsync(annotationData);
969+
970+
failedSaveQueueRef.current.delete(traceId);
971+
setSubmittedAnnotations(prev => new Set([...prev, traceId]));
972+
savedStateRef.current.set(traceId, {
973+
ratings: { ...data.ratings },
974+
freeformResponses: { ...data.freeformResponses },
975+
comment: data.comment
976+
});
977+
successCount++;
978+
} catch (error) {
979+
console.error(`Failed to save annotation for trace ${traceId}:`, error);
980+
}
981+
}
982+
983+
setFailedSaveCount(failedSaveQueueRef.current.size);
984+
985+
if (successCount > 0) {
986+
toast.success(`Successfully saved ${successCount} annotations`);
987+
}
988+
if (failedSaveQueueRef.current.size > 0) {
989+
toast.error(`${failedSaveQueueRef.current.size} annotations still pending`);
990+
}
991+
};
992+
758993
if (tracesLoading || rubricLoading) {
759994
return (
760995
<div className="min-h-screen bg-gray-50 p-6 flex items-center justify-center">
@@ -806,7 +1041,19 @@ export function AnnotationDemo() {
8061041
<div className="mb-6">
8071042
<div className="flex justify-between items-center mb-2">
8081043
<span className="text-sm text-gray-600">Progress</span>
809-
<span className="text-sm text-gray-600">{completedCount} of {traceData.length} complete</span>
1044+
<div className="flex items-center gap-3">
1045+
{failedSaveCount > 0 && (
1046+
<button
1047+
onClick={retryAllFailedSaves}
1048+
className="flex items-center gap-1 text-sm text-amber-600 bg-amber-50 hover:bg-amber-100 px-2 py-0.5 rounded cursor-pointer transition-colors"
1049+
title="Click to retry saving"
1050+
>
1051+
<RefreshCw className="h-3 w-3 animate-spin" />
1052+
{failedSaveCount} pending save{failedSaveCount > 1 ? 's' : ''} - click to retry
1053+
</button>
1054+
)}
1055+
<span className="text-sm text-gray-600">{completedCount} of {traceData.length} complete</span>
1056+
</div>
8101057
</div>
8111058
<div className="w-full bg-gray-200 rounded-full h-2">
8121059
<div

client/src/pages/RubricCreationDemo.tsx

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -330,19 +330,26 @@ export function RubricCreationDemo() {
330330
const deleteQuestion = async (id: string) => {
331331
try {
332332
// Call the new delete endpoint for individual questions
333-
await fetch(`/workshops/${workshopId}/rubric/questions/${id}`, {
333+
const response = await fetch(`/workshops/${workshopId}/rubric/questions/${id}`, {
334334
method: 'DELETE',
335335
headers: {
336336
'Content-Type': 'application/json',
337337
},
338338
});
339339

340-
// Invalidate queries to refresh the UI
341-
queryClient.invalidateQueries({ queryKey: ['rubric', workshopId] });
340+
if (!response.ok) {
341+
throw new Error('Failed to delete question');
342+
}
342343

344+
// Immediately update local state to remove the deleted question
345+
setQuestions(prevQuestions => prevQuestions.filter(q => q.id !== id));
343346

344-
} catch (error) {
347+
// Also invalidate queries to ensure consistency
348+
queryClient.invalidateQueries({ queryKey: ['rubric', workshopId] });
345349

350+
toast.success('Question deleted successfully');
351+
} catch (error) {
352+
console.error('Error deleting question:', error);
346353
toast.error('Failed to delete question. Please try again.');
347354
}
348355
};

0 commit comments

Comments
 (0)