From d37820de9915803c099873fba3c0ef87a48034e6 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 14:39:05 +0000 Subject: [PATCH 1/2] Text tab: show extracted text started from another tab, and why a run failed Tabs keeps every panel mounted, so DocumentTextPanel fetched once when the asset opened (empty, nothing running) and never again. "Extract text" lives on the Info tab; the job ran and succeeded, but the Text tab kept saying no text had been read. The panel now reloads whenever the activity store's extract_text job for its asset changes. The job's error (a scanned PDF has no text layer) was likewise visible only beside the button on the Info tab, while the Text tab invited a re-run that cannot work. It is shown there too. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_016qCc34GmfeFJr83WN71291 --- .../src/components/DocumentTextPanel.test.tsx | 45 ++++++++++++++++++- frontend/src/components/DocumentTextPanel.tsx | 34 +++++++++++++- 2 files changed, 77 insertions(+), 2 deletions(-) diff --git a/frontend/src/components/DocumentTextPanel.test.tsx b/frontend/src/components/DocumentTextPanel.test.tsx index d921ed9..4f5e7e9 100644 --- a/frontend/src/components/DocumentTextPanel.test.tsx +++ b/frontend/src/components/DocumentTextPanel.test.tsx @@ -1,8 +1,9 @@ -import { render, screen } from '@testing-library/react' +import { act, cleanup, render, screen } from '@testing-library/react' import { afterEach, describe, expect, it, vi } from 'vitest' import DocumentTextPanel from '@/components/DocumentTextPanel' import { enrichmentApi, type DocumentPage } from '@/api/enrichment' import { activityApi, type ActivityJob } from '@/api/transcripts' +import { useActivityStore } from '@/stores/activity' function page(overrides: Partial = {}): DocumentPage { return { @@ -33,6 +34,11 @@ function job(overrides: Partial = {}): ActivityJob { } afterEach(() => { + // Unmount before touching the store or the spies: the panel reloads whenever its job + // changes, so emptying the store under a mounted panel — with the spies already + // restored — sends a real request to nowhere. + cleanup() + useActivityStore.setState({ jobs: [] }) vi.restoreAllMocks() }) @@ -75,4 +81,41 @@ describe('DocumentTextPanel', () => { expect(await screen.findByText('Extracting text')).toBeInTheDocument() expect(screen.queryByText('Summarising')).not.toBeInTheDocument() }) + + it('shows the text when the extraction was started from another tab', async () => { + // Tabs keeps every panel mounted, so this one has already loaded — empty, with + // nothing running — by the time "Extract text" is pressed on the Info tab. The + // button knows nothing of this panel; the activity store is the only thing both can + // hear, and without listening to it the panel says "no text yet" for good. + const text = vi.spyOn(enrichmentApi, 'text').mockResolvedValue([]) + vi.spyOn(activityApi, 'list').mockResolvedValue([]) + + render() + expect(await screen.findByText(/run extract text/i)).toBeInTheDocument() + + text.mockResolvedValue([page()]) + act(() => useActivityStore.setState({ jobs: [job({ status: 'done' })] })) + + expect(await screen.findByText('Gecko Asset Manager')).toBeInTheDocument() + }) + + it('says why when the extraction failed, instead of asking for another run', async () => { + // A scan has no text to read, and the reason lives on the failed job. Shown only on + // the Info tab it is invisible from here, where the empty state invites a re-run + // that cannot work. + vi.spyOn(enrichmentApi, 'text').mockResolvedValue([]) + vi.spyOn(activityApi, 'list').mockResolvedValue([]) + useActivityStore.setState({ + jobs: [ + job({ + status: 'error', + error_message: 'No text could be read from this document.', + }), + ], + }) + + render() + + expect(await screen.findByText(/no text could be read/i)).toBeInTheDocument() + }) }) diff --git a/frontend/src/components/DocumentTextPanel.tsx b/frontend/src/components/DocumentTextPanel.tsx index e80ebed..0335927 100644 --- a/frontend/src/components/DocumentTextPanel.tsx +++ b/frontend/src/components/DocumentTextPanel.tsx @@ -1,8 +1,9 @@ -import { useCallback, useEffect, useState } from 'react' +import { useCallback, useEffect, useRef, useState } from 'react' import { FileText, Loader2 } from 'lucide-react' import { enrichmentApi, type DocumentPage } from '@/api/enrichment' import { activityApi, type ActivityJob } from '@/api/transcripts' import { apiErrorMessage } from '@/api/client' +import { useActivityStore } from '@/stores/activity' interface Props { assetId: string @@ -53,6 +54,28 @@ export default function DocumentTextPanel({ assetId }: Props) { void load() }, [load]) + // The job as the rest of the app sees it. `Tabs` keeps this panel mounted while the + // "Extract text" button on the Info tab starts a job, so the panel's own fetch above + // ran long before there was anything to find and nothing here would ever run it again + // — the document would read "no text yet" beside a job that had succeeded. Reloading on + // each change of the store's job is what lets two components that cannot see each + // other agree. The id is part of the key because a re-run goes done -> done. + const storeJob = useActivityStore((s) => + s.jobs.find((j) => j.asset_id === assetId && j.action === 'extract_text') + ) + const storeJobKey = storeJob ? `${storeJob.id}:${storeJob.status}` : '' + const seenJobKey = useRef(null) + useEffect(() => { + // The first run is the mount, which the effect above already loads for. + if (seenJobKey.current === null) { + seenJobKey.current = storeJobKey + return + } + if (seenJobKey.current === storeJobKey) return + seenJobKey.current = storeJobKey + void load() + }, [storeJobKey, load]) + // Poll only while something is running. Extracted text is static once written. useEffect(() => { if (!job || (job.status !== 'queued' && job.status !== 'processing')) return @@ -103,6 +126,15 @@ export default function DocumentTextPanel({ assetId }: Props) {

)} + {/* Why it failed, here as well as beside the button. A scanned PDF has no text to + read, and that reason would otherwise live only on the Info tab while this one + invited another run that cannot work. */} + {!error && !running && storeJob?.status === 'error' && storeJob.error_message && ( +

+ {storeJob.error_message} +

+ )} + {!error && pages.map((page) => (
From a9d6cdbf33f7d8bcf1cafd01683381bfbef0165e Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 14:59:56 +0000 Subject: [PATCH 2/2] Move Extract text to the Text tab; keep each tab's suggestions to its own Extract text moves from Info to the top of the Text panel, beside the only place its output is shown. The button reports a failed run's reason itself, so the separate error block the panel gained earlier goes. SuggestionPanel took no notion of where it was. Details and Source both mount one (Tabs keeps every panel mounted) and both read every suggestion, so attribution proposals appeared on Details and title/tag proposals under Source. It now takes a scope - details: title and tag, source: attribution - applied as suggestions load, so "Accept all" cannot accept the other tab's. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_016qCc34GmfeFJr83WN71291 --- frontend/src/components/AssetDetail.tsx | 15 +--- frontend/src/components/AttributionPanel.tsx | 2 +- .../src/components/DocumentTextPanel.test.tsx | 14 +++- frontend/src/components/DocumentTextPanel.tsx | 29 ++++--- .../src/components/SuggestionPanel.test.tsx | 78 ++++++++++++++++--- frontend/src/components/SuggestionPanel.tsx | 24 +++++- 6 files changed, 122 insertions(+), 40 deletions(-) diff --git a/frontend/src/components/AssetDetail.tsx b/frontend/src/components/AssetDetail.tsx index aef1d67..8e1c070 100644 --- a/frontend/src/components/AssetDetail.tsx +++ b/frontend/src/components/AssetDetail.tsx @@ -7,7 +7,6 @@ import { Mic, Pencil, Quote, - ScanText, Scissors, Tags as TagsIcon, Trash2, @@ -488,7 +487,7 @@ export default function AssetDetail({ )}
- + ) @@ -505,18 +504,6 @@ export default function AssetDetail({ failureMessage="Could not start extracting" /> )} - {asset.asset_type === TEXT_TYPE && ( - - )}
diff --git a/frontend/src/components/AttributionPanel.tsx b/frontend/src/components/AttributionPanel.tsx index 389df10..2108fe1 100644 --- a/frontend/src/components/AttributionPanel.tsx +++ b/frontend/src/components/AttributionPanel.tsx @@ -216,7 +216,7 @@ export default function AttributionPanel({ asset }: Props) { /> )} - + {FIELDS.map((field) => (
diff --git a/frontend/src/components/DocumentTextPanel.test.tsx b/frontend/src/components/DocumentTextPanel.test.tsx index 4f5e7e9..c572f88 100644 --- a/frontend/src/components/DocumentTextPanel.test.tsx +++ b/frontend/src/components/DocumentTextPanel.test.tsx @@ -43,6 +43,15 @@ afterEach(() => { }) describe('DocumentTextPanel', () => { + it('offers the Extract text button', async () => { + vi.spyOn(enrichmentApi, 'text').mockResolvedValue([]) + vi.spyOn(activityApi, 'list').mockResolvedValue([]) + + render() + + expect(await screen.findByRole('button', { name: /extract text/i })).toBeEnabled() + }) + it('shows the extracted text with its labels', async () => { vi.spyOn(enrichmentApi, 'text').mockResolvedValue([ page(), @@ -100,9 +109,8 @@ describe('DocumentTextPanel', () => { }) it('says why when the extraction failed, instead of asking for another run', async () => { - // A scan has no text to read, and the reason lives on the failed job. Shown only on - // the Info tab it is invisible from here, where the empty state invites a re-run - // that cannot work. + // A scan has no text to read, and the reason lives on the failed job. The empty + // state invites a re-run, so the reason has to be on screen beside it. vi.spyOn(enrichmentApi, 'text').mockResolvedValue([]) vi.spyOn(activityApi, 'list').mockResolvedValue([]) useActivityStore.setState({ diff --git a/frontend/src/components/DocumentTextPanel.tsx b/frontend/src/components/DocumentTextPanel.tsx index 0335927..1248538 100644 --- a/frontend/src/components/DocumentTextPanel.tsx +++ b/frontend/src/components/DocumentTextPanel.tsx @@ -1,8 +1,9 @@ import { useCallback, useEffect, useRef, useState } from 'react' -import { FileText, Loader2 } from 'lucide-react' +import { FileText, Loader2, ScanText } from 'lucide-react' import { enrichmentApi, type DocumentPage } from '@/api/enrichment' import { activityApi, type ActivityJob } from '@/api/transcripts' import { apiErrorMessage } from '@/api/client' +import EnrichmentButton from '@/components/EnrichmentButton' import { useActivityStore } from '@/stores/activity' interface Props { @@ -106,6 +107,23 @@ export default function DocumentTextPanel({ assetId }: Props) {
+ {/* Here rather than on the Info tab, beside the only place its output is shown. + The button also reports a failed run's reason — a scan has no text to read — + so the explanation sits under the empty state that would otherwise invite a + re-run that cannot work. */} +
+ +
+ {error && (

{error} @@ -126,15 +144,6 @@ export default function DocumentTextPanel({ assetId }: Props) {

)} - {/* Why it failed, here as well as beside the button. A scanned PDF has no text to - read, and that reason would otherwise live only on the Info tab while this one - invited another run that cannot work. */} - {!error && !running && storeJob?.status === 'error' && storeJob.error_message && ( -

- {storeJob.error_message} -

- )} - {!error && pages.map((page) => (
diff --git a/frontend/src/components/SuggestionPanel.test.tsx b/frontend/src/components/SuggestionPanel.test.tsx index 7e90408..723bf5f 100644 --- a/frontend/src/components/SuggestionPanel.test.tsx +++ b/frontend/src/components/SuggestionPanel.test.tsx @@ -3,7 +3,7 @@ import userEvent from '@testing-library/user-event' import { MemoryRouter } from 'react-router-dom' import { AxiosError } from 'axios' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import SuggestionPanel from '@/components/SuggestionPanel' +import SuggestionPanel, { type SuggestionScope } from '@/components/SuggestionPanel' import { enrichmentApi, type Suggestion } from '@/api/enrichment' import { assetsApi } from '@/api/assets' import { useActivityStore } from '@/stores/activity' @@ -53,10 +53,10 @@ function codedError(code: string) { return error } -function renderPanel() { +function renderPanel(scope: SuggestionScope = 'details') { return render( - + ) } @@ -153,7 +153,7 @@ describe('SuggestionPanel', () => { useActivityStore.setState({ jobs: [job({ status: 'done' })] }) rerender( - + ) @@ -172,7 +172,7 @@ describe('SuggestionPanel', () => { useActivityStore.setState({ jobs: [job({ action: 'generate_all', status: 'done' })] }) rerender( - + ) @@ -228,7 +228,7 @@ describe('attribution suggestions (M10)', () => { it('shows the field, the value and the evidence', async () => { vi.spyOn(enrichmentApi, 'suggestions').mockResolvedValue([attribution()]) - renderPanel() + renderPanel('source') expect(await screen.findByText('BBC Two')).toBeInTheDocument() expect(screen.getByText('Publisher')).toBeInTheDocument() @@ -242,7 +242,7 @@ describe('attribution suggestions (M10)', () => { const accept = vi .spyOn(enrichmentApi, 'accept') .mockResolvedValue(attribution({ status: 'accepted' })) - renderPanel() + renderPanel('source') await userEvent.click(await screen.findByRole('button', { name: /accept bbc two/i })) @@ -254,11 +254,71 @@ describe('attribution suggestions (M10)', () => { // others beside it. vi.spyOn(enrichmentApi, 'suggestions').mockResolvedValue([ attribution({ id: 'bad', field: null, proposed_value: null }), - suggestion({ id: 'tag-1', kind: 'tag', value: 'DARPA' }), + attribution({ id: 'good', field: 'creator', proposed_value: 'Jane Doe' }), ]) - renderPanel() + renderPanel('source') + + expect(await screen.findByText('Jane Doe')).toBeInTheDocument() + expect(screen.queryByText('BBC Two')).toBeNull() + }) +}) + +describe('which tab a suggestion appears on', () => { + // Details and Source are both mounted at once and ask the same endpoint, so each has + // to keep to its own kinds or the other tab's proposals turn up under it. + const mixed = () => [ + suggestion({ id: 't', kind: 'title', value: 'A Better Title' }), + suggestion({ id: 'g', kind: 'tag', value: 'DARPA' }), + suggestion({ + id: 'a', + kind: 'attribution', + value: '{}', + field: 'publisher', + proposed_value: 'BBC Two', + evidence: 'the lower third', + }), + ] + + it('shows only the title and tags on Details', async () => { + vi.spyOn(enrichmentApi, 'suggestions').mockResolvedValue(mixed()) + renderPanel('details') expect(await screen.findByText('DARPA')).toBeInTheDocument() + expect(screen.getByText('A Better Title')).toBeInTheDocument() expect(screen.queryByText('BBC Two')).toBeNull() }) + + it('shows only attribution on Source', async () => { + vi.spyOn(enrichmentApi, 'suggestions').mockResolvedValue(mixed()) + renderPanel('source') + + expect(await screen.findByText('BBC Two')).toBeInTheDocument() + expect(screen.queryByText('DARPA')).toBeNull() + expect(screen.queryByText('A Better Title')).toBeNull() + }) + + it('does not accept the other tab’s suggestions with Accept all', async () => { + vi.spyOn(enrichmentApi, 'suggestions').mockResolvedValue(mixed()) + const accept = vi + .spyOn(enrichmentApi, 'accept') + .mockImplementation((_assetId, id) => + Promise.resolve(suggestion({ id, status: 'accepted' })) + ) + renderPanel('details') + + await userEvent.click(await screen.findByRole('button', { name: /accept all/i })) + + await waitFor(() => expect(accept).toHaveBeenCalledTimes(2)) + expect(accept).not.toHaveBeenCalledWith('a1', 'a') + }) + + it('renders nothing on a tab with no suggestions of its own', async () => { + vi.spyOn(enrichmentApi, 'suggestions').mockResolvedValue([ + suggestion({ kind: 'tag', value: 'DARPA' }), + ]) + renderPanel('source') + + await waitFor(() => expect(enrichmentApi.suggestions).toHaveBeenCalled()) + expect(screen.queryByText(/nothing is applied until you say so/i)).toBeNull() + }) }) diff --git a/frontend/src/components/SuggestionPanel.tsx b/frontend/src/components/SuggestionPanel.tsx index 3a13b86..7507bfa 100644 --- a/frontend/src/components/SuggestionPanel.tsx +++ b/frontend/src/components/SuggestionPanel.tsx @@ -17,8 +17,22 @@ import { useLibraryStore } from '@/stores/library' * store for the same job to know when to reload rather than owning its own trigger. */ +/** + * Which tab this instance lives on. `Tabs` keeps every panel mounted, and Details and + * Source each render one of these against the same asset, so without a scope each showed + * the other's suggestions: attribution proposals on Details, and title and tag proposals + * under Source. A suggestion belongs where the field it would change is. + */ +export type SuggestionScope = 'details' | 'source' + +const KINDS_BY_SCOPE: Record> = { + details: ['title', 'tag'], + source: ['attribution'], +} + interface Props { assetId: string + scope: SuggestionScope } /** Field names as the Source tab labels them, so the two do not disagree. */ @@ -31,7 +45,7 @@ const FIELD_LABELS: Record = { license: 'Licence / rights', } -export default function SuggestionPanel({ assetId }: Props) { +export default function SuggestionPanel({ assetId, scope }: Props) { // Autotag can also be run through "Generate all", which is a different action on // the same asset — this still has to notice that run ending, or accepted-looking // suggestions from it would sit unread until something else reloaded the panel. @@ -54,11 +68,15 @@ export default function SuggestionPanel({ assetId }: Props) { const running = job ? isActive(job) : false const load = useCallback(() => { + // Filtered on the way in rather than at render, so "Accept all", the count that + // decides whether to offer it, and the empty state all speak of this tab's + // suggestions alone — accepting all on Details must not also accept attribution. + const kinds = KINDS_BY_SCOPE[scope] enrichmentApi .suggestions(assetId) - .then(setSuggestions) + .then((all) => setSuggestions(all.filter((s) => kinds.includes(s.kind)))) .catch(() => setSuggestions([])) - }, [assetId]) + }, [assetId, scope]) useEffect(() => { load()