Claude commited on
Commit
1d5cfba
·
unverified ·
1 Parent(s): fc1a5f0

fix(frontend): Sprint Fix 3 — type safety, memory leaks, error handling

Browse files

11 issues fixed:

- #5: Fix memory leak in RegionOverlay: use AbortController to clean up
all event listeners on unmount/re-render
- #6: Add pages_skipped to IngestResponse type
- #7: Make ImageInfo.master/width/height required, add thumbnail field
- #8: Add blocks/lines to OCRResult type
- #14: Fix stale closure in Viewer.tsx: use ref for onViewerReady callback
to always access the latest version
- #17: Type fetchManifest return as Record, selectModel as CorpusModelConfig;
add extensions/processing to PageMaster type
- #25: Add useEffect cleanup for success timeout in Editor (prevents
setState on unmounted component)
- #27: Show error message in Home.tsx when fetchManuscripts fails
(was silently caught)
- #28: Distinguish 404 (not analyzed) from network errors in Reader.tsx
master.json loading
- #29: Guard handleRetryFailed against concurrent polling/launching state
- #30: Use item.label as key instead of array index in RetroMenuBar

TypeScript compiles clean. 563 backend tests pass, 0 regressions.

https://claude.ai/code/session_01UB4he7RdRPHLvNjky4X8Sw

frontend/src/components/RegionOverlay.tsx CHANGED
@@ -21,6 +21,11 @@ const RegionOverlay: FC<Props> = ({ viewer, regions, onRegionClick }) => {
21
  useEffect(() => {
22
  if (!viewer) return
23
 
 
 
 
 
 
24
  const addOverlays = () => {
25
  viewer.clearOverlays()
26
  const item = viewer.world.getItemAt(0)
@@ -38,14 +43,14 @@ const RegionOverlay: FC<Props> = ({ viewer, regions, onRegionClick }) => {
38
 
39
  el.addEventListener('mouseenter', () => {
40
  el.style.backgroundColor = `${color}33`
41
- })
42
  el.addEventListener('mouseleave', () => {
43
  el.style.backgroundColor = ''
44
- })
45
  el.addEventListener('click', (e: MouseEvent) => {
46
  e.stopPropagation()
47
  onRegionClick(region)
48
- })
49
 
50
  const rect = item.imageToViewportRectangle(x, y, w, h)
51
  viewer.addOverlay(el, rect)
@@ -59,7 +64,8 @@ const RegionOverlay: FC<Props> = ({ viewer, regions, onRegionClick }) => {
59
  }
60
 
61
  return () => {
62
- // Nettoyage : retire les overlays au prochain rendu
 
63
  try {
64
  viewer.clearOverlays()
65
  } catch {
 
21
  useEffect(() => {
22
  if (!viewer) return
23
 
24
+ // AbortController pour nettoyer proprement tous les event listeners
25
+ // lors du démontage ou du re-rendu (évite les fuites mémoire).
26
+ const controller = new AbortController()
27
+ const { signal } = controller
28
+
29
  const addOverlays = () => {
30
  viewer.clearOverlays()
31
  const item = viewer.world.getItemAt(0)
 
43
 
44
  el.addEventListener('mouseenter', () => {
45
  el.style.backgroundColor = `${color}33`
46
+ }, { signal })
47
  el.addEventListener('mouseleave', () => {
48
  el.style.backgroundColor = ''
49
+ }, { signal })
50
  el.addEventListener('click', (e: MouseEvent) => {
51
  e.stopPropagation()
52
  onRegionClick(region)
53
+ }, { signal })
54
 
55
  const rect = item.imageToViewportRectangle(x, y, w, h)
56
  viewer.addOverlay(el, rect)
 
64
  }
65
 
66
  return () => {
67
+ // Supprime tous les event listeners d'un coup, puis les overlays
68
+ controller.abort()
69
  try {
70
  viewer.clearOverlays()
71
  } catch {
frontend/src/components/Viewer.tsx CHANGED
@@ -10,6 +10,9 @@ interface Props {
10
  const Viewer: FC<Props> = ({ imageUrl, onViewerReady }) => {
11
  const containerRef = useRef<HTMLDivElement>(null)
12
  const viewerRef = useRef<OpenSeadragon.Viewer | null>(null)
 
 
 
13
 
14
  useEffect(() => {
15
  if (!containerRef.current) return
@@ -38,9 +41,9 @@ const Viewer: FC<Props> = ({ imageUrl, onViewerReady }) => {
38
 
39
  viewer.open({ type: 'image', url: imageUrl })
40
  viewer.addOnceHandler('open', () => {
41
- onViewerReady?.(viewer)
42
  })
43
- }, [imageUrl]) // eslint-disable-line react-hooks/exhaustive-deps
44
 
45
  return (
46
  <div className="relative w-full h-full bg-retro-black">
 
10
  const Viewer: FC<Props> = ({ imageUrl, onViewerReady }) => {
11
  const containerRef = useRef<HTMLDivElement>(null)
12
  const viewerRef = useRef<OpenSeadragon.Viewer | null>(null)
13
+ // Ref pour toujours accéder au callback le plus récent (évite stale closure)
14
+ const onViewerReadyRef = useRef(onViewerReady)
15
+ onViewerReadyRef.current = onViewerReady
16
 
17
  useEffect(() => {
18
  if (!containerRef.current) return
 
41
 
42
  viewer.open({ type: 'image', url: imageUrl })
43
  viewer.addOnceHandler('open', () => {
44
+ onViewerReadyRef.current?.(viewer)
45
  })
46
+ }, [imageUrl])
47
 
48
  return (
49
  <div className="relative w-full h-full bg-retro-black">
frontend/src/components/retro/RetroMenuBar.tsx CHANGED
@@ -28,10 +28,10 @@ export default function RetroMenuBar({ items = [], right, className = '' }: Prop
28
  ${className}
29
  `}
30
  >
31
- {items.map((item, i) => (
32
  <button
33
  type="button"
34
- key={i}
35
  onClick={item.onClick}
36
  disabled={item.disabled}
37
  className={`
 
28
  ${className}
29
  `}
30
  >
31
+ {items.map((item) => (
32
  <button
33
  type="button"
34
+ key={item.label}
35
  onClick={item.onClick}
36
  disabled={item.disabled}
37
  className={`
frontend/src/lib/api.ts CHANGED
@@ -30,6 +30,7 @@ export interface IngestResponse {
30
  corpus_id: string
31
  manuscript_id: string
32
  pages_created: number
 
33
  page_ids: string[]
34
  }
35
 
@@ -105,6 +106,8 @@ export interface Region {
105
 
106
  export interface OCRResult {
107
  diplomatic_text: string
 
 
108
  language: string
109
  confidence: number
110
  uncertain_segments: string[]
@@ -143,11 +146,12 @@ export interface EditorialInfo {
143
  }
144
 
145
  export interface ImageInfo {
146
- master?: string
147
- derivative_web?: string
148
- iiif_base?: string
149
- width?: number
150
- height?: number
 
151
  }
152
 
153
  export interface PageMaster {
@@ -163,6 +167,8 @@ export interface PageMaster {
163
  translation: Translation | null
164
  summary: { short: string; detailed: string } | null
165
  commentary: Commentary | null
 
 
166
  editorial: EditorialInfo
167
  }
168
 
@@ -245,7 +251,7 @@ export const fetchPages = (manuscriptId: string): Promise<Page[]> =>
245
  export const fetchMasterJson = (pageId: string): Promise<PageMaster> =>
246
  get(`/api/v1/pages/${pageId}/master-json`)
247
 
248
- export const fetchManifest = (manuscriptId: string): Promise<unknown> =>
249
  get(`/api/v1/manuscripts/${manuscriptId}/iiif-manifest`)
250
 
251
  export const fetchProfile = (profileId: string): Promise<CorpusProfile> =>
@@ -268,7 +274,7 @@ export const selectModel = (
268
  modelId: string,
269
  displayName: string,
270
  providerType: string,
271
- ): Promise<unknown> =>
272
  put(`/api/v1/corpora/${corpusId}/model`, {
273
  model_id: modelId,
274
  display_name: displayName,
 
30
  corpus_id: string
31
  manuscript_id: string
32
  pages_created: number
33
+ pages_skipped: number
34
  page_ids: string[]
35
  }
36
 
 
106
 
107
  export interface OCRResult {
108
  diplomatic_text: string
109
+ blocks: Record<string, unknown>[]
110
+ lines: Record<string, unknown>[]
111
  language: string
112
  confidence: number
113
  uncertain_segments: string[]
 
146
  }
147
 
148
  export interface ImageInfo {
149
+ master: string
150
+ derivative_web?: string | null
151
+ thumbnail?: string | null
152
+ iiif_base?: string | null
153
+ width: number
154
+ height: number
155
  }
156
 
157
  export interface PageMaster {
 
167
  translation: Translation | null
168
  summary: { short: string; detailed: string } | null
169
  commentary: Commentary | null
170
+ extensions: Record<string, unknown>
171
+ processing: Record<string, unknown> | null
172
  editorial: EditorialInfo
173
  }
174
 
 
251
  export const fetchMasterJson = (pageId: string): Promise<PageMaster> =>
252
  get(`/api/v1/pages/${pageId}/master-json`)
253
 
254
+ export const fetchManifest = (manuscriptId: string): Promise<Record<string, unknown>> =>
255
  get(`/api/v1/manuscripts/${manuscriptId}/iiif-manifest`)
256
 
257
  export const fetchProfile = (profileId: string): Promise<CorpusProfile> =>
 
274
  modelId: string,
275
  displayName: string,
276
  providerType: string,
277
+ ): Promise<CorpusModelConfig> =>
278
  put(`/api/v1/corpora/${corpusId}/model`, {
279
  model_id: modelId,
280
  display_name: displayName,
frontend/src/pages/Admin.tsx CHANGED
@@ -414,6 +414,7 @@ function RunPanel({ corpusId, hasModel }: { corpusId: string; hasModel: boolean
414
  }
415
 
416
  const handleRetryFailed = async () => {
 
417
  const failedIds = Object.values(jobs).filter((j) => j.status === 'failed').map((j) => j.id)
418
  if (failedIds.length === 0) return
419
  await Promise.allSettled(failedIds.map((id) => retryJob(id)))
 
414
  }
415
 
416
  const handleRetryFailed = async () => {
417
+ if (polling || launching) return
418
  const failedIds = Object.values(jobs).filter((j) => j.status === 'failed').map((j) => j.id)
419
  if (failedIds.length === 0) return
420
  await Promise.allSettled(failedIds.map((id) => retryJob(id)))
frontend/src/pages/Editor.tsx CHANGED
@@ -48,6 +48,13 @@ export default function Editor({ pageId, onBack }: Props) {
48
 
49
  const successTimeout = useRef<ReturnType<typeof setTimeout> | null>(null)
50
 
 
 
 
 
 
 
 
51
  const loadData = useCallback(async () => {
52
  setLoading(true)
53
  setError(null)
@@ -59,7 +66,7 @@ export default function Editor({ pageId, onBack }: Props) {
59
  setCommentaryPublic(m.commentary?.public ?? '')
60
  setCommentaryScholarly(m.commentary?.scholarly ?? '')
61
  setEditorialStatus(m.editorial.status)
62
- const ext = (m as unknown as { extensions?: { region_validations?: Record<string, string> } }).extensions
63
  setRegionValidations(ext?.region_validations ?? {})
64
  } catch (e: unknown) {
65
  setError((e as Error).message)
 
48
 
49
  const successTimeout = useRef<ReturnType<typeof setTimeout> | null>(null)
50
 
51
+ // Nettoyage du timeout de succès lors du démontage du composant
52
+ useEffect(() => {
53
+ return () => {
54
+ if (successTimeout.current) clearTimeout(successTimeout.current)
55
+ }
56
+ }, [])
57
+
58
  const loadData = useCallback(async () => {
59
  setLoading(true)
60
  setError(null)
 
66
  setCommentaryPublic(m.commentary?.public ?? '')
67
  setCommentaryScholarly(m.commentary?.scholarly ?? '')
68
  setEditorialStatus(m.editorial.status)
69
+ const ext = m.extensions as { region_validations?: Record<string, string> } | undefined
70
  setRegionValidations(ext?.region_validations ?? {})
71
  } catch (e: unknown) {
72
  setError((e as Error).message)
frontend/src/pages/Home.tsx CHANGED
@@ -29,6 +29,7 @@ export default function Home({ onOpenManuscript, onOpenPage, onAdmin }: Props) {
29
  const [manuscripts, setManuscripts] = useState<Record<string, Manuscript[]>>({})
30
  const [expanding, setExpanding] = useState<string | null>(null)
31
  const [selectedCorpus, setSelectedCorpus] = useState<Corpus | null>(null)
 
32
 
33
  useEffect(() => {
34
  fetchCorpora()
@@ -47,12 +48,13 @@ export default function Home({ onOpenManuscript, onOpenPage, onAdmin }: Props) {
47
  }
48
 
49
  setExpanding(corpus.id)
 
50
  try {
51
  const ms = await fetchManuscripts(corpus.id)
52
  setManuscripts((prev) => ({ ...prev, [corpus.id]: ms }))
53
  if (ms.length === 1) onOpenManuscript(ms[0].id, corpus.profile_id)
54
- } catch {
55
- // silent
56
  } finally {
57
  setExpanding(null)
58
  }
@@ -149,6 +151,11 @@ export default function Home({ onOpenManuscript, onOpenPage, onAdmin }: Props) {
149
  Chargement...
150
  </div>
151
  )}
 
 
 
 
 
152
  </div>
153
  ))}
154
  </div>
 
29
  const [manuscripts, setManuscripts] = useState<Record<string, Manuscript[]>>({})
30
  const [expanding, setExpanding] = useState<string | null>(null)
31
  const [selectedCorpus, setSelectedCorpus] = useState<Corpus | null>(null)
32
+ const [expandError, setExpandError] = useState<string | null>(null)
33
 
34
  useEffect(() => {
35
  fetchCorpora()
 
48
  }
49
 
50
  setExpanding(corpus.id)
51
+ setExpandError(null)
52
  try {
53
  const ms = await fetchManuscripts(corpus.id)
54
  setManuscripts((prev) => ({ ...prev, [corpus.id]: ms }))
55
  if (ms.length === 1) onOpenManuscript(ms[0].id, corpus.profile_id)
56
+ } catch (e: unknown) {
57
+ setExpandError(e instanceof Error ? e.message : 'Erreur de chargement')
58
  } finally {
59
  setExpanding(null)
60
  }
 
151
  Chargement...
152
  </div>
153
  )}
154
+ {expandError && selectedCorpus?.id === corpus.id && !expanding && (
155
+ <div className="px-3 py-1 text-retro-xs text-retro-black bg-retro-light">
156
+ Erreur: {expandError}
157
+ </div>
158
+ )}
159
  </div>
160
  ))}
161
  </div>
frontend/src/pages/Reader.tsx CHANGED
@@ -47,11 +47,24 @@ export default function Reader({ manuscriptId, profileId, onBack, onEdit }: Prop
47
  .finally(() => setLoading(false))
48
  }, [manuscriptId, profileId])
49
 
 
 
50
  useEffect(() => {
51
  if (pages.length === 0) return
52
  setMaster(null)
 
53
  setSelectedRegion(null)
54
- fetchMasterJson(pages[currentIndex].id).then(setMaster).catch(() => setMaster(null))
 
 
 
 
 
 
 
 
 
 
55
  }, [pages, currentIndex])
56
 
57
  const handleViewerReady = useCallback((v: OpenSeadragon.Viewer) => {
@@ -194,10 +207,13 @@ export default function Reader({ manuscriptId, profileId, onBack, onEdit }: Prop
194
  </div>
195
  )}
196
 
197
- {/* Not analyzed badge */}
198
  {!master && !loading && imageUrl && (
199
  <div className="absolute top-2 left-2">
200
- <RetroBadge variant="warning">Non analysee</RetroBadge>
 
 
 
201
  </div>
202
  )}
203
  </div>
 
47
  .finally(() => setLoading(false))
48
  }, [manuscriptId, profileId])
49
 
50
+ const [masterError, setMasterError] = useState<string | null>(null)
51
+
52
  useEffect(() => {
53
  if (pages.length === 0) return
54
  setMaster(null)
55
+ setMasterError(null)
56
  setSelectedRegion(null)
57
+ fetchMasterJson(pages[currentIndex].id)
58
+ .then(setMaster)
59
+ .catch((e: unknown) => {
60
+ // 404 = page non analysée (normal), autres erreurs = problème réseau
61
+ const msg = e instanceof Error ? e.message : ''
62
+ if (msg.includes('404')) {
63
+ setMaster(null)
64
+ } else {
65
+ setMasterError(msg || 'Erreur de chargement')
66
+ }
67
+ })
68
  }, [pages, currentIndex])
69
 
70
  const handleViewerReady = useCallback((v: OpenSeadragon.Viewer) => {
 
207
  </div>
208
  )}
209
 
210
+ {/* Not analyzed / error badge */}
211
  {!master && !loading && imageUrl && (
212
  <div className="absolute top-2 left-2">
213
+ {masterError
214
+ ? <RetroBadge variant="error">Erreur: {masterError}</RetroBadge>
215
+ : <RetroBadge variant="warning">Non analysee</RetroBadge>
216
+ }
217
  </div>
218
  )}
219
  </div>