From 10be5650954dab24283458dac8fe3f54430a5cb1 Mon Sep 17 00:00:00 2001 From: joel-joseph-george Date: Wed, 14 Oct 2026 11:36:20 +0530 Subject: [PATCH 1/5] test: add tests and state override logic for display mode in AudioReviewPage --- .../audio/components/AudioReviewPage.test.tsx | 257 ++++++++++++++++++ .../audio/components/AudioReviewPage.tsx | 13 +- 2 files changed, 268 insertions(+), 2 deletions(-) create mode 100644 src/features/audio/components/AudioReviewPage.test.tsx diff --git a/src/features/audio/components/AudioReviewPage.test.tsx b/src/features/audio/components/AudioReviewPage.test.tsx new file mode 100644 index 00000000..9d67b5ac --- /dev/null +++ b/src/features/audio/components/AudioReviewPage.test.tsx @@ -0,0 +1,257 @@ +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { act, fireEvent, render, screen } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { PlaybackRegistryProvider } from '@/features/tts'; +import { type PericopeGroup, type ProjectItem, type VerseAudioRecording } from '@/lib/types'; +import { useAppStore } from '@/store/store'; + +import AudioReviewPage from './AudioReviewPage'; + +vi.mock('react-i18next', () => import('@/test/i18n')); + +const mockRouter = { + history: { + back: vi.fn(), + }, +}; + +const mockProjectItem = { + chapterAssignmentId: 101, + projectId: 1, + projectName: 'Test Project', + bookId: 1, + bookCode: 'GEN', + book: 'Genesis', + chapterNumber: 6, + projectUnitId: 1, +} as unknown as ProjectItem; + +const mockRecordings = [ + { + id: 1, + projectUnitId: 1, + bibleTextId: 1, + uploadedBy: 1, + contentType: 'audio/webm', + sizeBytes: 1000, + durationSeconds: 5, + versionToken: 1, + downloadUrl: 'https://example.com/1.webm', + verseNumber: 1, + conflictStatus: 'clean', + activeTakeId: 11, + createdAt: '2026-03-01T00:00:00Z', + updatedAt: '2026-03-01T00:00:00Z', + takes: [ + { + id: 11, + uploadedBy: 1, + contentType: 'audio/webm', + sizeBytes: 1000, + durationSeconds: 5, + contentHash: 'hash-11', + downloadUrl: 'https://example.com/11.webm', + createdAt: '2026-03-01T00:00:00Z', + updatedAt: '2026-03-01T00:00:00Z', + }, + ], + }, + { + id: 2, + projectUnitId: 1, + bibleTextId: 2, + uploadedBy: 1, + contentType: 'audio/webm', + sizeBytes: 1000, + durationSeconds: 5, + versionToken: 1, + downloadUrl: 'https://example.com/2.webm', + verseNumber: 2, + conflictStatus: 'clean', + activeTakeId: 12, + createdAt: '2026-03-01T00:00:00Z', + updatedAt: '2026-03-01T00:00:00Z', + takes: [ + { + id: 12, + uploadedBy: 1, + contentType: 'audio/webm', + sizeBytes: 1000, + durationSeconds: 5, + contentHash: 'hash-12', + downloadUrl: 'https://example.com/12.webm', + createdAt: '2026-03-01T00:00:00Z', + updatedAt: '2026-03-01T00:00:00Z', + }, + ], + }, + { + id: 3, + projectUnitId: 1, + bibleTextId: 3, + uploadedBy: 1, + contentType: 'audio/webm', + sizeBytes: 1000, + durationSeconds: 5, + versionToken: 1, + downloadUrl: 'https://example.com/3.webm', + verseNumber: 3, + conflictStatus: 'clean', + activeTakeId: 13, + createdAt: '2026-03-01T00:00:00Z', + updatedAt: '2026-03-01T00:00:00Z', + takes: [ + { + id: 13, + uploadedBy: 1, + contentType: 'audio/webm', + sizeBytes: 1000, + durationSeconds: 5, + contentHash: 'hash-13', + downloadUrl: 'https://example.com/13.webm', + createdAt: '2026-03-01T00:00:00Z', + updatedAt: '2026-03-01T00:00:00Z', + }, + ], + }, +] as unknown as VerseAudioRecording[]; + +const mockPericopes: PericopeGroup[] = [ + { + pericopeNumber: '1', + pericopeTitle: null, + verses: [ + { verseNumber: 1, chapterNumber: 6 }, + { verseNumber: 2, chapterNumber: 6 }, + ], + }, + { + pericopeNumber: '2', + pericopeTitle: null, + verses: [{ verseNumber: 3, chapterNumber: 6 }], + }, +]; + +vi.mock('@tanstack/react-router', () => ({ + getRouteApi: () => ({ + useLoaderData: () => ({ projectItem: mockProjectItem }), + }), + useRouter: () => mockRouter, +})); + +vi.mock('@/features/audio/hooks/useChapterAudio', () => ({ + useChapterAudio: () => ({ + data: { items: mockRecordings, total: mockRecordings.length }, + isLoading: false, + }), +})); + +vi.mock('@/features/projects/hooks/useProjectUsers', () => ({ + useProjectUsers: () => ({ + data: [{ userId: 1, displayName: 'Alice' }], + }), +})); + +vi.mock('@/features/pericopes/hooks/useChapterPericopes', () => ({ + useChapterPericopes: () => ({ + data: mockPericopes, + }), +})); + +vi.mock('@/features/audio/hooks/useResolveConflict', () => ({ + useResolveConflict: () => ({ + mutate: vi.fn(), + }), +})); + +vi.mock('@/features/bible/hooks/useBibleTarget', () => ({ + targetTextQueryOptions: () => ({ + queryKey: ['target-text'], + queryFn: () => [], + }), +})); + +function renderPage() { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + return render( + + + + + + ); +} + +describe('AudioReviewPage display mode', () => { + beforeEach(() => { + vi.spyOn(HTMLMediaElement.prototype, 'play').mockImplementation(() => Promise.resolve()); + vi.spyOn(HTMLMediaElement.prototype, 'pause').mockImplementation(() => {}); + useAppStore.setState({ + displayMode: 'verse', + userdetail: { + id: 1, + username: 'alice', + displayName: 'Alice', + email: 'alice@example.com', + role: 'user', + grants: [ + { + orgId: 1, + projectId: 1, + roleId: 1, + roleName: 'project_manager', + permissions: [], + }, + ], + lastActiveOrgId: 1, + }, + }); + }); + + it('renders verses when global displayMode is verse', () => { + renderPage(); + + expect(screen.getByText('Verse 1')).toBeInTheDocument(); + expect(screen.getByText('Verse 2')).toBeInTheDocument(); + expect(screen.getByText('Verse 3')).toBeInTheDocument(); + }); + + it('reacts immediately when displayMode is changed in the global settings to pericope', () => { + renderPage(); + + expect(screen.getByText('Verse 1')).toBeInTheDocument(); + + act(() => { + useAppStore.getState().setDisplayMode('pericope'); + }); + + expect(screen.getByText('Verses 1–2')).toBeInTheDocument(); + expect(screen.getByText('Verse 3')).toBeInTheDocument(); + }); + + it('switches to verse view locally when clicking page-specified switch button, keeping global setting unchanged', () => { + useAppStore.setState({ displayMode: 'pericope' }); + const { unmount } = renderPage(); + + expect(screen.getByText('Verses 1–2')).toBeInTheDocument(); + + // Click page-specified "Switch to Verse view" + const switchButton = screen.getByRole('button', { name: /switch to verse view/i }); + fireEvent.click(switchButton); + + // Page view switches to verse locally + expect(screen.getByText('Verse 1')).toBeInTheDocument(); + expect(screen.getByText('Verse 2')).toBeInTheDocument(); + + // Global store displayMode remains 'pericope' untouched + expect(useAppStore.getState().displayMode).toBe('pericope'); + + // When the user moves out and back in (remounts the page), it still respects the global setting ('pericope') + unmount(); + renderPage(); + expect(screen.getByText('Verses 1–2')).toBeInTheDocument(); + }); +}); diff --git a/src/features/audio/components/AudioReviewPage.tsx b/src/features/audio/components/AudioReviewPage.tsx index 9022dfd8..e2e5a719 100644 --- a/src/features/audio/components/AudioReviewPage.tsx +++ b/src/features/audio/components/AudioReviewPage.tsx @@ -105,8 +105,17 @@ const AudioReviewPage: React.FC = ({ projectItem: propItem const loaderData = routeApi.useLoaderData(); const projectItem = propItem ?? loaderData.projectItem; const router = useRouter(); - const initialDisplayMode = useAppStore(state => state.displayMode); - const [displayMode, setDisplayMode] = useState(initialDisplayMode); + const globalDisplayMode = useAppStore(state => state.displayMode); + const [pageDisplayModeOverride, setPageDisplayModeOverride] = useState(null); + const [prevGlobalDisplayMode, setPrevGlobalDisplayMode] = useState(globalDisplayMode); + + if (prevGlobalDisplayMode !== globalDisplayMode) { + setPrevGlobalDisplayMode(globalDisplayMode); + setPageDisplayModeOverride(null); + } + + const displayMode = pageDisplayModeOverride ?? globalDisplayMode; + const setDisplayMode = setPageDisplayModeOverride; const userdetail = useAppStore(state => state.userdetail); // Only PMs may resolve conflicts (spec #383). From 9ebb041e0d2d097c2c0056d1de1b69a043e3d18d Mon Sep 17 00:00:00 2001 From: joel-joseph-george Date: Mon, 19 Oct 2026 11:37:51 +0530 Subject: [PATCH 2/5] feat: implement stitched pericope audio playback and cross-view conflict indicators for audio review --- .../audio/components/AudioReviewPage.test.tsx | 213 ++++++++- .../audio/components/AudioReviewPage.tsx | 404 +++++++++++++---- .../components/StitchedAudioPlayer.test.tsx | 124 ++++++ .../audio/components/StitchedAudioPlayer.tsx | 414 ++++++++++++++++++ src/features/audio/components/TakeCard.tsx | 9 +- .../audio/hooks/useStitchedAudio.test.ts | 145 ++++++ src/features/audio/hooks/useStitchedAudio.ts | 131 ++++++ src/lib/types.ts | 9 + 8 files changed, 1358 insertions(+), 91 deletions(-) create mode 100644 src/features/audio/components/StitchedAudioPlayer.test.tsx create mode 100644 src/features/audio/components/StitchedAudioPlayer.tsx create mode 100644 src/features/audio/hooks/useStitchedAudio.test.ts create mode 100644 src/features/audio/hooks/useStitchedAudio.ts diff --git a/src/features/audio/components/AudioReviewPage.test.tsx b/src/features/audio/components/AudioReviewPage.test.tsx index 9d67b5ac..fc4e29ef 100644 --- a/src/features/audio/components/AudioReviewPage.test.tsx +++ b/src/features/audio/components/AudioReviewPage.test.tsx @@ -140,9 +140,11 @@ vi.mock('@tanstack/react-router', () => ({ useRouter: () => mockRouter, })); +let currentRecordings = [...mockRecordings]; + vi.mock('@/features/audio/hooks/useChapterAudio', () => ({ useChapterAudio: () => ({ - data: { items: mockRecordings, total: mockRecordings.length }, + data: { items: currentRecordings, total: currentRecordings.length }, isLoading: false, }), })); @@ -187,6 +189,7 @@ function renderPage() { describe('AudioReviewPage display mode', () => { beforeEach(() => { + currentRecordings = [...mockRecordings]; vi.spyOn(HTMLMediaElement.prototype, 'play').mockImplementation(() => Promise.resolve()); vi.spyOn(HTMLMediaElement.prototype, 'pause').mockImplementation(() => {}); useAppStore.setState({ @@ -232,13 +235,21 @@ describe('AudioReviewPage display mode', () => { expect(screen.getByText('Verse 3')).toBeInTheDocument(); }); - it('switches to verse view locally when clicking page-specified switch button, keeping global setting unchanged', () => { + it('switches to verse view locally when clicking page-specified switch button in conflict banner, keeping global setting unchanged', () => { + currentRecordings = [ + { + ...mockRecordings[0], + conflictStatus: 'conflict', + }, + mockRecordings[1], + mockRecordings[2], + ]; useAppStore.setState({ displayMode: 'pericope' }); const { unmount } = renderPage(); expect(screen.getByText('Verses 1–2')).toBeInTheDocument(); - // Click page-specified "Switch to Verse view" + // Click page-specified "Switch to Verse view" in the conflict banner const switchButton = screen.getByRole('button', { name: /switch to verse view/i }); fireEvent.click(switchButton); @@ -255,3 +266,199 @@ describe('AudioReviewPage display mode', () => { expect(screen.getByText('Verses 1–2')).toBeInTheDocument(); }); }); + +describe('AudioReviewPage pericope stitched audio & cross-view conflicts (Issue #554)', () => { + beforeEach(() => { + currentRecordings = [...mockRecordings]; + vi.spyOn(HTMLMediaElement.prototype, 'play').mockImplementation(() => Promise.resolve()); + vi.spyOn(HTMLMediaElement.prototype, 'pause').mockImplementation(() => {}); + useAppStore.setState({ + displayMode: 'pericope', + userdetail: { + id: 1, + username: 'alice', + displayName: 'Alice', + email: 'alice@example.com', + role: 'user', + grants: [ + { + orgId: 1, + projectId: 1, + roleId: 1, + roleName: 'project_manager', + permissions: [], + }, + ], + lastActiveOrgId: 1, + }, + }); + }); + + it('renders stitched audio player and correct subtitle for verse-recorded pericopes without conflicts', () => { + renderPage(); + + // Subtitle under Recorded Takes + expect(screen.getByText('Recorded by verse. Played as one recording.')).toBeInTheDocument(); + + // Stitched card with heading + expect(screen.getByTestId('stitched-audio-card')).toBeInTheDocument(); + expect(screen.getByText('Stitched from verse recordings, vv. 1-2')).toBeInTheDocument(); + + // No individual TakeCard elements rendered + expect(screen.queryByText(/Take 1/)).not.toBeInTheDocument(); + }); + + it('shows skipped verse notes when a verse has conflict in pericope view', () => { + currentRecordings = [ + mockRecordings[0], // verse 1 clean + { + ...mockRecordings[1], + conflictStatus: 'conflict', // verse 2 conflict + }, + mockRecordings[2], + ]; + + renderPage(); + + expect(screen.getByTestId('stitched-audio-card')).toBeInTheDocument(); + expect(screen.getByText('Verse 2 not included: conflict')).toBeInTheDocument(); + }); + + it('shows skipped verse notes when a verse is unrecorded in pericope view', () => { + // Only verse 1 is recorded in pericope 1-2, verse 2 is missing + currentRecordings = [ + mockRecordings[0], + mockRecordings[2], // verse 3 + ]; + + renderPage(); + + expect(screen.getByTestId('stitched-audio-card')).toBeInTheDocument(); + expect(screen.getByText('Verse 2 not included: no recording')).toBeInTheDocument(); + }); + + it('displays cross-view warning indicator and banner in pericope view when verse has conflict', () => { + currentRecordings = [ + { + ...mockRecordings[0], + conflictStatus: 'conflict', + }, + mockRecordings[1], + mockRecordings[2], + ]; + + renderPage(); + + // Left column warning indicator + expect(screen.getByText('Conflict in verse 1')).toBeInTheDocument(); + + // Center column conflict banner + expect(screen.getByText(/This conflict was recorded by verse\./)).toBeInTheDocument(); + expect(screen.getByRole('button', { name: /switch to verse view/i })).toBeInTheDocument(); + + // In opposing mode, no "Select as Draft" button is rendered + expect(screen.queryByText(/Select as Draft/i)).not.toBeInTheDocument(); + }); + + it('displays cross-view warning indicator and banner in verse view when pericope has conflict', () => { + useAppStore.setState({ displayMode: 'verse' }); + + // Pericope covering verses 1-2 is in conflict + currentRecordings = [ + { + id: 99, + projectUnitId: 1, + bibleTextId: 1, + uploadedBy: 1, + contentType: 'audio/webm', + sizeBytes: 1000, + durationSeconds: 10, + versionToken: 1, + downloadUrl: 'https://example.com/p.webm', + verseNumber: 1, + isPericope: true, + pericopeRange: [1, 2], + conflictStatus: 'conflict', + activeTakeId: 991, + createdAt: '2026-03-01T00:00:00Z', + updatedAt: '2026-03-01T00:00:00Z', + takes: [ + { + id: 991, + uploadedBy: 1, + contentType: 'audio/webm', + sizeBytes: 1000, + durationSeconds: 10, + contentHash: 'hash-991', + downloadUrl: 'https://example.com/991.webm', + createdAt: '2026-03-01T00:00:00Z', + updatedAt: '2026-03-01T00:00:00Z', + }, + ], + } as unknown as VerseAudioRecording, + mockRecordings[2], // verse 3 + ]; + + renderPage(); + + // Left column warning indicator on verse 1 and verse 2 + expect(screen.getAllByText('Conflict in pericope vv. 1-2')).toHaveLength(2); + + // Center column banner + expect(screen.getByText(/This conflict was recorded as a pericope\./)).toBeInTheDocument(); + const switchBtn = screen.getByRole('button', { name: /switch to pericope view/i }); + expect(switchBtn).toBeInTheDocument(); + + // Conflicting pericope takes are suppressed in verse view + expect(screen.queryByText(/Take.*Pericope/)).not.toBeInTheDocument(); + expect(screen.queryByText(/Select as Draft/i)).not.toBeInTheDocument(); + + // Clicking switch to pericope switches local view + fireEvent.click(switchBtn); + expect(screen.getByText('Verses 1–2')).toBeInTheDocument(); + }); + + it('renders pericope takes under covered verses in verse view when pericope is clean', () => { + useAppStore.setState({ displayMode: 'verse' }); + + currentRecordings = [ + mockRecordings[0], // verse 1 has its own verse recording + { + id: 99, + projectUnitId: 1, + bibleTextId: 1, + uploadedBy: 1, + contentType: 'audio/webm', + sizeBytes: 1000, + durationSeconds: 10, + versionToken: 1, + downloadUrl: 'https://example.com/p.webm', + verseNumber: 1, + isPericope: true, + pericopeRange: [1, 2], + conflictStatus: 'clean', + activeTakeId: 991, + createdAt: '2026-03-01T00:00:00Z', + updatedAt: '2026-03-01T00:00:00Z', + takes: [ + { + id: 991, + uploadedBy: 1, + contentType: 'audio/webm', + sizeBytes: 1000, + durationSeconds: 10, + contentHash: 'hash-991', + downloadUrl: 'https://example.com/991.webm', + createdAt: '2026-03-01T00:00:00Z', + updatedAt: '2026-03-01T00:00:00Z', + }, + ], + } as unknown as VerseAudioRecording, + ]; + + renderPage(); + + // Should display verse 1 take and pericope take labeled "Take 1 - Pericope - vv. 1-2" + expect(screen.getByText('Take 1 - Pericope - vv. 1-2')).toBeInTheDocument(); + }); +}); diff --git a/src/features/audio/components/AudioReviewPage.tsx b/src/features/audio/components/AudioReviewPage.tsx index e2e5a719..ff7c0802 100644 --- a/src/features/audio/components/AudioReviewPage.tsx +++ b/src/features/audio/components/AudioReviewPage.tsx @@ -7,13 +7,15 @@ import { useTranslation } from 'react-i18next'; import { useChapterAudio } from '@/features/audio/hooks/useChapterAudio'; import { useResolveConflict } from '@/features/audio/hooks/useResolveConflict'; +import { isPericopeRecording, useStitchedAudio } from '@/features/audio/hooks/useStitchedAudio'; import { targetTextQueryOptions } from '@/features/bible/hooks/useBibleTarget'; import { useChapterPericopes } from '@/features/pericopes/hooks/useChapterPericopes'; import { useProjectUsers } from '@/features/projects/hooks/useProjectUsers'; import { getActiveGrants, isProjectManager } from '@/lib/grant-utils'; -import { type ProjectItem, type VerseAudioRecording } from '@/lib/types'; +import { type ProjectItem, type VerseAudioRecording, type VerseAudioTake } from '@/lib/types'; import { type DisplayMode, useAppStore } from '@/store/store'; +import StitchedAudioPlayer from './StitchedAudioPlayer'; import TakeCard, { SelectAsDraftButton } from './TakeCard'; // --------------------------------------------------------------------------- @@ -39,6 +41,8 @@ interface PassageListItemProps { isSelected: boolean; /** True if this passage was previously conflicted but is now resolved */ isResolved: boolean; + /** Cross-view conflict warning label, e.g. "Conflict in verse 6" or "Conflict in pericope vv. 1-5" */ + crossViewConflict?: string | null; onClick: () => void; } @@ -47,34 +51,47 @@ const PassageListItem: React.FC = ({ dotState, isSelected, isResolved, + crossViewConflict, onClick, }) => { const resolvedGreen = 'var(--success-subtle-foreground)'; + const warningColor = 'var(--warning-subtle-foreground)'; return ( ); }; @@ -92,6 +109,9 @@ interface Passage { recordings: VerseAudioRecording[]; hasConflict: boolean; wasConflicted: boolean; + crossViewConflict?: string | null; + isOppositeModeConflict?: boolean; + conflictRecordedMode?: 'verse' | 'pericope'; } const isPassage = (passage: Passage | null): passage is Passage => passage !== null; @@ -153,59 +173,158 @@ const AudioReviewPage: React.FC = ({ projectItem: propItem [projectUsers] ); + interface ReviewTake extends VerseAudioTake { + displayLabel?: string; + bibleTextId?: number; + } + // ---------- Build passage list ---------- /** * A "passage" is either a single verse (verse/chapter mode) or a pericope group. - * Each entry carries the set of VerseAudioRecording rows that belong to it. + * Each entry carries the set of VerseAudioRecording rows that belong to it, + * plus cross-view conflict metadata. */ const passages: Passage[] = useMemo(() => { const recordings = audioData?.items ?? []; + const pericopeRecordings = recordings.filter(isPericopeRecording); + const verseRecordings = recordings.filter(r => !isPericopeRecording(r)); if (displayMode === 'pericope' && pericopes && pericopes.length > 0) { return pericopes .map((group): Passage | null => { - const groupVerseNums = new Set(group.verses.map(v => v.verseNumber)); - const groupRecordings = recordings.filter(r => groupVerseNums.has(r.verseNumber)); - if (groupRecordings.length === 0) return null; + // Only include verses belonging to the current chapter + const chapterVerses = group.verses.filter( + v => v.chapterNumber === projectItem.chapterNumber + ); + if (chapterVerses.length === 0) return null; + + const groupVerseNums = new Set(chapterVerses.map(v => v.verseNumber)); + const startVerse = chapterVerses[0]?.verseNumber; + const endVerse = chapterVerses[chapterVerses.length - 1]?.verseNumber; + + // Direct pericope recordings + const directPericopeRecs = pericopeRecordings.filter(r => { + if (r.pericopeRange) { + return r.pericopeRange[0] === startVerse && r.pericopeRange[1] === endVerse; + } + return r.verseNumber === startVerse; + }); + + // Verse recordings for verses in this pericope + const groupVerseRecs = verseRecordings.filter(r => groupVerseNums.has(r.verseNumber)); + + if (directPericopeRecs.length === 0 && groupVerseRecs.length === 0) return null; - const startVerse = group.verses[0]?.verseNumber; - const endVerse = group.verses[group.verses.length - 1]?.verseNumber; const label = startVerse === endVerse ? t('verseLabel', 'Verse {{verse}}', { verse: startVerse }) : t('versesLabel', 'Verses {{start}}–{{end}}', { start: startVerse, end: endVerse }); - const hasConflictNow = groupRecordings.some(isConflict); - // 'resolved' is set by the API when the conflict was explicitly resolved - // via PUT /resolve — unambiguous, no heuristic needed. - const wasConflicted = groupRecordings.some(isResolved); + const hasDirectConflict = directPericopeRecs.some(isConflict); + const conflictedVerses = groupVerseRecs + .filter(isConflict) + .map(r => r.verseNumber) + .sort((a, b) => a - b); + const hasVerseConflict = conflictedVerses.length > 0; + + const hasConflictNow = hasDirectConflict || hasVerseConflict; + const wasConflicted = + directPericopeRecs.some(isResolved) || groupVerseRecs.some(isResolved); + + let crossViewConflict: string | null = null; + let isOppositeModeConflict = false; + let conflictRecordedMode: 'verse' | 'pericope' | undefined; + + if (hasVerseConflict && !hasDirectConflict) { + isOppositeModeConflict = true; + conflictRecordedMode = 'verse'; + crossViewConflict = + conflictedVerses.length === 1 + ? t('conflictInVerse', 'Conflict in verse {{verse}}', { + verse: conflictedVerses[0], + }) + : t('conflictInVerses', 'Conflict in verses {{verses}}', { + verses: conflictedVerses.join(', '), + }); + } + + const passageRecs = directPericopeRecs.length > 0 ? directPericopeRecs : groupVerseRecs; return { id: `pericope-${group.pericopeNumber}`, label, - verseNumbers: group.verses.map(v => v.verseNumber), - recordings: groupRecordings, + verseNumbers: chapterVerses.map(v => v.verseNumber), + recordings: passageRecs, hasConflict: hasConflictNow, wasConflicted, + crossViewConflict, + isOppositeModeConflict, + conflictRecordedMode, }; }) .filter(isPassage); } - // Verse / chapter mode: one row per verse - return recordings.map( - (r): Passage => ({ - id: `verse-${r.verseNumber}`, - label: t('verseLabel', 'Verse {{verse}}', { verse: r.verseNumber }), - verseNumbers: [r.verseNumber], - recordings: [r], - hasConflict: isConflict(r), - // 'resolved' status is set by the API on successful conflict resolution. - wasConflicted: isResolved(r), - }) - ); - }, [audioData, displayMode, pericopes, t]); + // Verse / chapter mode: + // Collect all verse numbers that have verse recordings or are covered by pericope recordings + const verseNumSet = new Set(); + for (const r of verseRecordings) { + verseNumSet.add(r.verseNumber); + } + for (const pr of pericopeRecordings) { + const [start, end] = pr.pericopeRange ?? [pr.verseNumber, pr.verseNumber]; + for (let v = start; v <= end; v++) { + verseNumSet.add(v); + } + } + + const sortedVerses = Array.from(verseNumSet).sort((a, b) => a - b); + + return sortedVerses.map((v): Passage => { + const r = verseRecordings.find(rec => rec.verseNumber === v); + + // Check if this verse is covered by a pericope recording + const coveringPericope = pericopeRecordings.find(pr => { + const [start, end] = pr.pericopeRange ?? [pr.verseNumber, pr.verseNumber]; + return v >= start && v <= end; + }); + + const hasDirectConflict = Boolean(r && isConflict(r)); + const hasPericopeConflict = Boolean(coveringPericope && isConflict(coveringPericope)); + const hasConflictNow = hasDirectConflict || hasPericopeConflict; + const wasConflicted = + Boolean(r && isResolved(r)) || Boolean(coveringPericope && isResolved(coveringPericope)); + + let crossViewConflict: string | null = null; + let isOppositeModeConflict = false; + let conflictRecordedMode: 'verse' | 'pericope' | undefined; + + if (hasPericopeConflict && coveringPericope && !hasDirectConflict) { + isOppositeModeConflict = true; + conflictRecordedMode = 'pericope'; + const [start, end] = coveringPericope.pericopeRange ?? [ + coveringPericope.verseNumber, + coveringPericope.verseNumber, + ]; + crossViewConflict = t('conflictInPericope', 'Conflict in pericope vv. {{range}}', { + range: `${start}-${end}`, + }); + } + + return { + id: `verse-${v}`, + label: t('verseLabel', 'Verse {{verse}}', { verse: v }), + verseNumbers: [v], + recordings: r ? [r] : [], + hasConflict: hasConflictNow, + wasConflicted, + crossViewConflict, + isOppositeModeConflict, + conflictRecordedMode, + }; + }); + }, [audioData, displayMode, pericopes, projectItem.chapterNumber, t]); // ---------- All-resolved chip ---------- @@ -224,23 +343,72 @@ const AudioReviewPage: React.FC = ({ projectItem: propItem return passages.find(p => p.id === selectedPassageId) ?? passages[0]; }, [passages, selectedPassageId]); - // All takes for the selected passage (union of all recordings in the passage). - // Pericope mode may have multiple recordings — we flatten all their takes. - const selectedTakes = useMemo( - () => (selectedPassage?.recordings ?? []).flatMap(r => r.takes), - [selectedPassage] - ); + const effectivePassage = selectedPassage ?? passages[0]; + + // ---------- Stitched audio calculation for pericope mode ---------- + + const stitchedAudio = useStitchedAudio({ + verseNumbers: effectivePassage.verseNumbers, + recordings: audioData?.items ?? [], + }); // Map each take.id to its parent recording's bibleTextId const takeBibleTextMap = useMemo(() => { const map = new Map(); - for (const recording of selectedPassage?.recordings ?? []) { + const recordings = audioData?.items ?? []; + for (const recording of recordings) { for (const take of recording.takes) { map.set(take.id, recording.bibleTextId); } } return map; - }, [selectedPassage]); + }, [audioData]); + + // All takes for the selected passage + const selectedTakes = useMemo((): ReviewTake[] => { + if (!selectedPassage) return []; + const recordings = audioData?.items ?? []; + const pericopeRecordings = recordings.filter(isPericopeRecording); + + if (displayMode === 'verse') { + const v = selectedPassage.verseNumbers[0]; + const verseTakes: ReviewTake[] = (selectedPassage.recordings[0]?.takes ?? []).map(t => ({ + ...t, + bibleTextId: selectedPassage.recordings[0]?.bibleTextId, + })); + + // Find pericope recordings covering this verse + const coveringPericopes = pericopeRecordings.filter(pr => { + const [start, end] = pr.pericopeRange ?? [pr.verseNumber, pr.verseNumber]; + return v >= start && v <= end; + }); + + const pericopeTakes: ReviewTake[] = []; + for (const pr of coveringPericopes) { + // "When that pericope is in conflict, do not list its takes under the verses; + // the cross-view conflict indicator and message apply instead." + if (isConflict(pr)) continue; + + const [start, end] = pr.pericopeRange ?? [pr.verseNumber, pr.verseNumber]; + const range = `${start}-${end}`; + pr.takes.forEach((take, idx) => { + pericopeTakes.push({ + ...take, + displayLabel: t('takePericopeLabel', 'Take {{takeNumber}} - Pericope - vv. {{range}}', { + takeNumber: idx + 1, + range, + }), + bibleTextId: pr.bibleTextId, + }); + }); + } + + return [...verseTakes, ...pericopeTakes]; + } + + // Pericope mode + return selectedPassage.recordings.flatMap(r => r.takes); + }, [selectedPassage, displayMode, audioData, t]); // Show contributor names only when the passage has takes from more than one contributor (spec #383) const showContributor = useMemo(() => { @@ -250,16 +418,36 @@ const AudioReviewPage: React.FC = ({ projectItem: propItem /** * Active take id — look across all recordings in the passage. - * In verse mode there's always exactly one recording; in pericope mode - * there can be many (one per verse). We find the first take id that is - * marked active in any of those recordings. */ const activeTakeId = useMemo(() => { - for (const r of selectedPassage?.recordings ?? []) { - if (r.activeTakeId !== null) return r.activeTakeId; + if (displayMode === 'pericope') { + if (stitchedAudio.hasActivePericopeTake && stitchedAudio.activePericopeTake) { + return stitchedAudio.activePericopeTake.id; + } + for (const r of selectedPassage?.recordings ?? []) { + if (r.activeTakeId !== null) return r.activeTakeId; + } + return null; + } + + // Verse mode + const vRec = selectedPassage?.recordings[0]; + if (vRec && vRec.activeTakeId !== null) return vRec.activeTakeId; + + // Check if covering pericope has active take + const pericopeRecordings = (audioData?.items ?? []).filter(isPericopeRecording); + const v = selectedPassage?.verseNumbers[0]; + if (v !== undefined) { + const coveringPericope = pericopeRecordings.find(pr => { + const [start, end] = pr.pericopeRange ?? [pr.verseNumber, pr.verseNumber]; + return v >= start && v <= end && !isConflict(pr); + }); + if (coveringPericope && coveringPericope.activeTakeId !== null) { + return coveringPericope.activeTakeId; + } } return null; - }, [selectedPassage]); + }, [selectedPassage, displayMode, stitchedAudio, audioData]); const selectedHasConflict = selectedPassage?.hasConflict ?? false; @@ -317,8 +505,6 @@ const AudioReviewPage: React.FC = ({ projectItem: propItem ); } - const effectivePassage = selectedPassage ?? passages[0]; - return (
{/* ── Page header ─────────────────────────────────────── */} @@ -377,11 +563,17 @@ const AudioReviewPage: React.FC = ({ projectItem: propItem if (passage.hasConflict) dotState = 'conflict'; else if (passage.wasConflicted) dotState = 'resolved'; + // If crossViewConflict is present, suppress the simple dot in favour of the warning triangle + if (passage.crossViewConflict) { + dotState = null; + } + const isPassageResolved = dotState === 'resolved'; return ( = ({ projectItem: propItem : t('recordedTakes', 'Recorded Takes')}

- {selectedHasConflict - ? t( - 'conflictingTakesDesc', - 'Select one take as the active draft. All recordings are retained.' - ) - : effectivePassage.wasConflicted + {displayMode === 'pericope' && stitchedAudio.hasOnlyPerVerseRecordings + ? t('recordedByVersePlayedAsOne', 'Recorded by verse. Played as one recording.') + : selectedHasConflict ? t( - 'resolvedTakesDesc', - 'This conflict has been resolved. All recordings are retained.' + 'conflictingTakesDesc', + 'Select one take as the active draft. All recordings are retained.' ) - : t('recordedTakesDesc', 'Listen to every recorded take for this passage.')} + : effectivePassage.wasConflicted + ? t( + 'resolvedTakesDesc', + 'This conflict has been resolved. All recordings are retained.' + ) + : t('recordedTakesDesc', 'Listen to every recorded take for this passage.')}

@@ -434,9 +628,40 @@ const AudioReviewPage: React.FC = ({ projectItem: propItem style={{ color: 'var(--warning-border)' }} /> - {t( - 'conflictBanner', - 'This passage has conflicting takes — select one to resolve.' + {effectivePassage.isOppositeModeConflict ? ( + effectivePassage.conflictRecordedMode === 'verse' ? ( + <> + {t('conflictRecordedByVerseLead', 'This conflict was recorded by verse.')}{' '} + {' '} + {t('toResolveIt', 'to resolve it.')} + + ) : ( + <> + {t( + 'conflictRecordedAsPericopeLead', + 'This conflict was recorded as a pericope.' + )}{' '} + {' '} + {t('toResolveIt', 'to resolve it.')} + + ) + ) : ( + t( + 'conflictBanner', + 'This passage has conflicting takes — select one to resolve.' + ) )} @@ -448,23 +673,23 @@ const AudioReviewPage: React.FC = ({ projectItem: propItem className='min-h-0 flex-1 overflow-y-auto px-6 pb-6' style={{ scrollbarGutter: 'stable' }} > - {/* TODO(fluent-api#377): Spec: "List takes as normal for a pericope recorded as a pericope". - Currently, recordings are stored at verse grain, so pericope view prompts the user - to switch to Verse view to review and resolve individual verse takes. Once the upstream - granularity field lands (fluent-api#377), check recording granularity here to list - pericope-grain takes directly. */} - {displayMode === 'pericope' ? ( -

- {t('recordedByVerse', 'Recorded by verse.')}{' '} - {' '} - {t('toReviewTheseTakes', 'to review these takes.')} -

+ {displayMode === 'pericope' && stitchedAudio.hasOnlyPerVerseRecordings ? ( + ) : selectedTakes.length === 0 ? (

{t('noAudioDraftsAvailable', 'No audio drafts available.')} @@ -472,8 +697,12 @@ const AudioReviewPage: React.FC = ({ projectItem: propItem ) : (

{selectedTakes.map((take, idx) => { - const bibleTextId = takeBibleTextMap.get(take.id); - const canResolveAsDraft = isPM && selectedHasConflict && take.id !== activeTakeId; + const bibleTextId = take.bibleTextId ?? takeBibleTextMap.get(take.id); + const canResolveAsDraft = + isPM && + selectedHasConflict && + !effectivePassage.isOppositeModeConflict && + take.id !== activeTakeId; return ( = ({ projectItem: propItem : undefined } isActiveDraft={take.id === activeTakeId} + label={take.displayLabel} take={take} takeNumber={idx + 1} /> diff --git a/src/features/audio/components/StitchedAudioPlayer.test.tsx b/src/features/audio/components/StitchedAudioPlayer.test.tsx new file mode 100644 index 00000000..f8219e86 --- /dev/null +++ b/src/features/audio/components/StitchedAudioPlayer.test.tsx @@ -0,0 +1,124 @@ +import { fireEvent, render, screen } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { PlaybackRegistryProvider } from '@/features/tts'; + +import StitchedAudioPlayer from './StitchedAudioPlayer'; + +vi.mock('react-i18next', () => import('@/test/i18n')); + +describe('StitchedAudioPlayer', () => { + const mockTracks = [ + { + verseNumber: 8, + takeId: 81, + downloadUrl: 'https://example.com/take-81.webm', + durationSeconds: 10, + }, + { + verseNumber: 9, + takeId: 91, + downloadUrl: 'https://example.com/take-91.webm', + durationSeconds: 13, + }, + ]; + + const mockSkipped = [ + { verseNumber: 6, reason: 'conflict' as const }, + { verseNumber: 7, reason: 'no recording' as const }, + ]; + + beforeEach(() => { + vi.clearAllMocks(); + vi.spyOn(HTMLMediaElement.prototype, 'play').mockImplementation(() => Promise.resolve()); + vi.spyOn(HTMLMediaElement.prototype, 'pause').mockImplementation(() => {}); + }); + + const renderComponent = (props = {}) => + render( + + + + ); + + it('renders label, duration, and skipped verses notes', () => { + renderComponent(); + + expect(screen.getByText('Stitched from verse recordings, vv. 6-9')).toBeInTheDocument(); + expect(screen.getByText('0:23')).toBeInTheDocument(); + expect(screen.getByText('Verse 6 not included: conflict')).toBeInTheDocument(); + expect(screen.getByText('Verse 7 not included: no recording')).toBeInTheDocument(); + }); + + it('toggles play and pause on button click', async () => { + const onPlay = vi.fn(); + const onPause = vi.fn(); + renderComponent({ onPlay, onPause }); + + const playBtn = screen.getByRole('button', { name: /play/i }); + fireEvent.click(playBtn); + + expect(HTMLMediaElement.prototype.play).toHaveBeenCalled(); + expect(onPlay).toHaveBeenCalled(); + + const pauseBtn = await screen.findByRole('button', { name: /pause/i }); + fireEvent.click(pauseBtn); + + expect(HTMLMediaElement.prototype.pause).toHaveBeenCalled(); + expect(onPause).toHaveBeenCalled(); + }); + + it('supports seeking via keyboard ArrowRight on the slider', () => { + renderComponent(); + + const slider = screen.getByRole('slider'); + fireEvent.keyDown(slider, { key: 'ArrowRight' }); + + // Advances time + expect(slider).toBeInTheDocument(); + }); + + it('automatically advances to the next track when current track ends while playing', async () => { + const { container } = renderComponent(); + + const playBtn = screen.getByRole('button', { name: /play/i }); + fireEvent.click(playBtn); + + const audioElement = container.querySelector('audio'); + expect(audioElement).toHaveAttribute('src', 'https://example.com/take-81.webm'); + + // Simulate first track ending + fireEvent.ended(audioElement!); + + // Should seamlessly update src to next track and continue playing + expect(audioElement).toHaveAttribute('src', 'https://example.com/take-91.webm'); + expect(HTMLMediaElement.prototype.play).toHaveBeenCalledTimes(2); + }); + + it('stops and resets when the last track ends', async () => { + const onPause = vi.fn(); + const { container } = renderComponent({ onPause }); + + const playBtn = screen.getByRole('button', { name: /play/i }); + fireEvent.click(playBtn); + + const audioElement = container.querySelector('audio'); + + // End track 1 -> advances to track 2 + fireEvent.ended(audioElement!); + expect(audioElement).toHaveAttribute('src', 'https://example.com/take-91.webm'); + + // End track 2 (last track) -> completes playback + fireEvent.ended(audioElement!); + + // Now playback is paused and reset to end + expect(onPause).toHaveBeenCalled(); + expect(screen.getByRole('button', { name: /play/i })).toBeInTheDocument(); + }); +}); diff --git a/src/features/audio/components/StitchedAudioPlayer.tsx b/src/features/audio/components/StitchedAudioPlayer.tsx new file mode 100644 index 00000000..29c52611 --- /dev/null +++ b/src/features/audio/components/StitchedAudioPlayer.tsx @@ -0,0 +1,414 @@ +import React, { useCallback, useEffect, useRef, useState } from 'react'; + +import { AlertTriangle, Pause, Play } from 'lucide-react'; +import { useTranslation } from 'react-i18next'; + +import { formatClock, usePlaybackRegistry } from '@/features/tts'; + +import { type SkippedVerse, type StitchedTrack } from '../hooks/useStitchedAudio'; + +export interface StitchedAudioPlayerProps { + /** Title label, e.g. "Stitched from verse recordings, vv. 6-9" */ + label: string; + /** Ordered active verse tracks */ + tracks: StitchedTrack[]; + /** Verses skipped from the sequence, with reasons */ + skippedVerses?: SkippedVerse[]; + /** Total calculated duration of the stitched tracks */ + totalDuration: number; + /** Called when playback starts */ + onPlay?: () => void; + /** Called when playback pauses or ends */ + onPause?: () => void; + /** Accent color for play button and waveform fill */ + accentColor?: string; +} + +const BARS = 40; +const WAVEFORM_HEIGHTS = [ + 0.3, 0.5, 0.7, 0.9, 0.6, 0.4, 0.8, 0.95, 0.5, 0.3, 0.7, 0.85, 0.6, 0.4, 1.0, 0.7, 0.45, 0.6, 0.8, + 0.5, 0.35, 0.65, 0.9, 0.55, 0.4, 0.75, 0.6, 0.45, 0.8, 0.95, 0.5, 0.3, 0.7, 0.85, 0.6, 0.4, 0.55, + 0.75, 0.4, 0.25, +]; + +/** + * Continuous stitched audio player for verse-recorded pericopes. + * Sequences active verse takes sequentially and provides a single unified scrub bar. + * Integrates with PlaybackRegistry for app-wide playback exclusivity. + */ +const StitchedAudioPlayer: React.FC = ({ + label, + tracks, + skippedVerses = [], + totalDuration, + onPlay, + onPause, + accentColor = 'var(--primary)', +}) => { + const { t } = useTranslation(); + const registry = usePlaybackRegistry(); + + const audioRef = useRef(null); + const releaseRef = useRef<(() => void) | null>(null); + + const [isPlaying, setIsPlaying] = useState(false); + const [currentTrackIndex, setCurrentTrackIndex] = useState(0); + const [currentTime, setCurrentTime] = useState(0); + const [isLoading, setIsLoading] = useState(false); + + const currentTrackIndexRef = useRef(0); + useEffect(() => { + currentTrackIndexRef.current = currentTrackIndex; + }, [currentTrackIndex]); + + const isPlayingRef = useRef(isPlaying); + useEffect(() => { + isPlayingRef.current = isPlaying; + }, [isPlaying]); + + // Keep audio.src initialized for the current track + useEffect(() => { + const audio = audioRef.current; + if (!audio || !tracks.length) return; + const track = tracks[currentTrackIndex]; + if (!audio.src.includes(track.downloadUrl)) { + audio.src = track.downloadUrl; + } + }, [tracks, currentTrackIndex]); + + // Pre-calculate track start offsets + const trackOffsets = useRef([]); + useEffect(() => { + const offsets: number[] = []; + let acc = 0; + for (const track of tracks) { + offsets.push(acc); + acc += track.durationSeconds; + } + trackOffsets.current = offsets; + }, [tracks]); + + const pausePlayback = useCallback(() => { + const audio = audioRef.current; + if (audio) { + audio.pause(); + } + setIsPlaying(false); + setIsLoading(false); + if (releaseRef.current) { + releaseRef.current(); + releaseRef.current = null; + } + onPause?.(); + }, [onPause]); + + // Clean up on unmount or tracks change + useEffect(() => { + const audio = audioRef.current; + return () => { + if (audio) { + audio.pause(); + } + if (releaseRef.current) { + releaseRef.current(); + releaseRef.current = null; + } + }; + }, [tracks]); + + // Helper to locate which track and internal offset correspond to a given total time + const locateTrack = useCallback( + (targetTotalTime: number) => { + let accumulated = 0; + for (let i = 0; i < tracks.length; i++) { + const d = tracks[i].durationSeconds; + if (targetTotalTime <= accumulated + d || i === tracks.length - 1) { + const localOffset = Math.max(0, Math.min(d, targetTotalTime - accumulated)); + return { trackIndex: i, localOffset }; + } + accumulated += d; + } + return { trackIndex: 0, localOffset: 0 }; + }, + [tracks] + ); + + const seekTo = useCallback( + (targetTotalTime: number) => { + if (!tracks.length || totalDuration <= 0) return; + const clamped = Math.max(0, Math.min(totalDuration, targetTotalTime)); + const { trackIndex, localOffset } = locateTrack(clamped); + + setCurrentTime(clamped); + + const audio = audioRef.current; + if (!audio) return; + + if (trackIndex === currentTrackIndexRef.current) { + audio.currentTime = localOffset; + } else { + currentTrackIndexRef.current = trackIndex; + setCurrentTrackIndex(trackIndex); + audio.src = tracks[trackIndex].downloadUrl; + audio.currentTime = localOffset; + if (isPlayingRef.current) { + void audio.play().catch((err: unknown) => { + if (err instanceof DOMException && err.name === 'AbortError') return; + setIsPlaying(false); + }); + } + } + }, + [tracks, totalDuration, locateTrack] + ); + + const handleToggle = useCallback(() => { + if (!tracks.length) return; + const audio = audioRef.current; + if (!audio) return; + + if (isPlaying) { + pausePlayback(); + } else { + onPlay?.(); + setIsLoading(true); + + // Claim exclusivity in the shared registry + releaseRef.current?.(); + releaseRef.current = registry.claim(() => { + const el = audioRef.current; + if (el) el.pause(); + setIsPlaying(false); + setIsLoading(false); + releaseRef.current = null; + onPause?.(); + }); + + // If at end, start from beginning + if (currentTime >= totalDuration && totalDuration > 0) { + currentTrackIndexRef.current = 0; + setCurrentTrackIndex(0); + setCurrentTime(0); + audio.src = tracks[0].downloadUrl; + audio.currentTime = 0; + } else { + const track = tracks[currentTrackIndexRef.current]; + if (!audio.src.includes(track.downloadUrl)) { + audio.src = track.downloadUrl; + } + } + + setIsPlaying(true); + void audio + .play() + .then(() => { + if (!releaseRef.current) { + audio.pause(); + setIsPlaying(false); + setIsLoading(false); + return; + } + setIsLoading(false); + }) + .catch((err: unknown) => { + if (err instanceof DOMException && err.name === 'AbortError') { + return; + } + setIsPlaying(false); + setIsLoading(false); + if (releaseRef.current) { + releaseRef.current(); + releaseRef.current = null; + } + }); + } + }, [tracks, isPlaying, onPlay, onPause, pausePlayback, registry, currentTime, totalDuration]); + + const handleTimeUpdate = useCallback(() => { + const audio = audioRef.current; + if (!audio) return; + const currentOffset = trackOffsets.current[currentTrackIndexRef.current] ?? 0; + const computedTotal = currentOffset + audio.currentTime; + setCurrentTime(Math.min(totalDuration, computedTotal)); + }, [totalDuration]); + + const handleEnded = useCallback(() => { + const audio = audioRef.current; + if (!audio) return; + + if (currentTrackIndexRef.current < tracks.length - 1) { + // Advance to next track seamlessly + const nextIndex = currentTrackIndexRef.current + 1; + currentTrackIndexRef.current = nextIndex; + setCurrentTrackIndex(nextIndex); + + const nextTrack = tracks[nextIndex]; + audio.src = nextTrack.downloadUrl; + audio.currentTime = 0; + void audio.play().catch((err: unknown) => { + if (err instanceof DOMException && err.name === 'AbortError') return; + setIsPlaying(false); + }); + } else { + // Reached end of all tracks + setIsPlaying(false); + setCurrentTime(totalDuration); + if (releaseRef.current) { + releaseRef.current(); + releaseRef.current = null; + } + onPause?.(); + } + }, [tracks, totalDuration, onPause]); + + const handleSeek = useCallback( + (e: React.MouseEvent) => { + const rect = e.currentTarget.getBoundingClientRect(); + const ratio = Math.max(0, Math.min(1, (e.clientX - rect.left) / rect.width)); + seekTo(ratio * totalDuration); + }, + [totalDuration, seekTo] + ); + + const handleKeyDown = useCallback( + (e: React.KeyboardEvent) => { + if (!totalDuration) return; + const step = Math.min(5, Math.max(1, totalDuration * 0.05)); + const pageStep = step * 2; + + switch (e.key) { + case 'ArrowRight': + case 'ArrowUp': + e.preventDefault(); + seekTo(currentTime + step); + break; + case 'ArrowLeft': + case 'ArrowDown': + e.preventDefault(); + seekTo(currentTime - step); + break; + case 'PageUp': + e.preventDefault(); + seekTo(currentTime + pageStep); + break; + case 'PageDown': + e.preventDefault(); + seekTo(currentTime - pageStep); + break; + case 'Home': + e.preventDefault(); + seekTo(0); + break; + case 'End': + e.preventDefault(); + seekTo(totalDuration); + break; + } + }, + [currentTime, totalDuration, seekTo] + ); + + const progress = totalDuration > 0 ? currentTime / totalDuration : 0; + const hasTracks = tracks.length > 0; + + return ( +
+ {/* Hidden audio element */} +
)} - {contributorName ? null : ( + {label ? ( + <> + {label}{' '} + + ) : contributorName ? null : ( <> {t('takeNumber', 'Take {{number}}', { number: takeNumber })} diff --git a/src/features/audio/hooks/useStitchedAudio.test.ts b/src/features/audio/hooks/useStitchedAudio.test.ts new file mode 100644 index 00000000..40b99104 --- /dev/null +++ b/src/features/audio/hooks/useStitchedAudio.test.ts @@ -0,0 +1,145 @@ +import { renderHook } from '@testing-library/react'; +import { describe, expect, it } from 'vitest'; + +import { type VerseAudioRecording } from '@/lib/types'; + +import { useStitchedAudio } from './useStitchedAudio'; + +describe('useStitchedAudio', () => { + const makeRecording = ( + verseNumber: number, + takes: Array<{ id: number; durationSeconds: number }>, + activeTakeId: number | null, + conflictStatus: 'clean' | 'conflict' | 'resolved' = 'clean' + ): VerseAudioRecording => ({ + id: verseNumber, + projectUnitId: 1, + bibleTextId: verseNumber, + uploadedBy: 1, + contentType: 'audio/webm', + sizeBytes: 1000, + durationSeconds: takes.find(t => t.id === activeTakeId)?.durationSeconds ?? null, + versionToken: 1, + conflictStatus, + activeTakeId, + verseNumber, + downloadUrl: `https://example.com/${verseNumber}.webm`, + createdAt: '2026-03-01T00:00:00Z', + updatedAt: '2026-03-01T00:00:00Z', + takes: takes.map(t => ({ + id: t.id, + uploadedBy: 1, + contentType: 'audio/webm', + sizeBytes: 1000, + durationSeconds: t.durationSeconds, + contentHash: `hash-${t.id}`, + downloadUrl: `https://example.com/take-${t.id}.webm`, + createdAt: '2026-03-01T00:00:00Z', + updatedAt: '2026-03-01T00:00:00Z', + })), + }); + + it('stitches active takes in sequential verse order when all verses are clean', () => { + const recordings = [ + makeRecording(6, [{ id: 61, durationSeconds: 5 }], 61), + makeRecording(7, [{ id: 71, durationSeconds: 6 }], 71), + makeRecording(8, [{ id: 81, durationSeconds: 7 }], 81), + makeRecording(9, [{ id: 91, durationSeconds: 8 }], 91), + ]; + + const { result } = renderHook(() => + useStitchedAudio({ + verseNumbers: [6, 7, 8, 9], + recordings, + }) + ); + + expect(result.current.hasOnlyPerVerseRecordings).toBe(true); + expect(result.current.hasActivePericopeTake).toBe(false); + expect(result.current.skippedVerses).toEqual([]); + expect(result.current.tracks).toHaveLength(4); + expect(result.current.tracks.map(t => t.verseNumber)).toEqual([6, 7, 8, 9]); + expect(result.current.tracks.map(t => t.takeId)).toEqual([61, 71, 81, 91]); + expect(result.current.totalDuration).toBe(26); + }); + + it('skips verses in conflict and verses with no recording, matching the issue mockup', () => { + // Verses 6-9: + // Verse 6: conflict + // Verse 7: no recording (missing from list) + // Verse 8: active take 81 (10s) + // Verse 9: active take 91 (13s) + const recordings = [ + makeRecording( + 6, + [ + { id: 61, durationSeconds: 4 }, + { id: 62, durationSeconds: 5 }, + ], + null, + 'conflict' + ), + makeRecording(8, [{ id: 81, durationSeconds: 10 }], 81), + makeRecording(9, [{ id: 91, durationSeconds: 13 }], 91), + ]; + + const { result } = renderHook(() => + useStitchedAudio({ + verseNumbers: [6, 7, 8, 9], + recordings, + }) + ); + + expect(result.current.hasOnlyPerVerseRecordings).toBe(true); + expect(result.current.hasActivePericopeTake).toBe(false); + expect(result.current.tracks.map(t => t.verseNumber)).toEqual([8, 9]); + expect(result.current.totalDuration).toBe(23); // 10 + 13 = 23s (matches mockup 0:23) + + expect(result.current.skippedVerses).toEqual([ + { verseNumber: 6, reason: 'conflict' }, + { verseNumber: 7, reason: 'no recording' }, + ]); + }); + + it('skips a verse that has a recording row but activeTakeId is null (no active take)', () => { + const recordings = [ + makeRecording(1, [{ id: 11, durationSeconds: 5 }], null, 'clean'), + makeRecording(2, [{ id: 21, durationSeconds: 7 }], 21, 'clean'), + ]; + + const { result } = renderHook(() => + useStitchedAudio({ + verseNumbers: [1, 2], + recordings, + }) + ); + + expect(result.current.tracks.map(t => t.verseNumber)).toEqual([2]); + expect(result.current.skippedVerses).toEqual([{ verseNumber: 1, reason: 'no recording' }]); + }); + + it('detects active pericope-level take when pericope recording is present and clean', () => { + const pericopeRecording: VerseAudioRecording = { + ...makeRecording(6, [{ id: 600, durationSeconds: 25 }], 600), + isPericope: true, + pericopeRange: [6, 9], + }; + + const recordings = [ + pericopeRecording, + makeRecording(6, [{ id: 61, durationSeconds: 5 }], 61), + makeRecording(7, [{ id: 71, durationSeconds: 6 }], 71), + ]; + + const { result } = renderHook(() => + useStitchedAudio({ + verseNumbers: [6, 7, 8, 9], + recordings, + }) + ); + + expect(result.current.hasOnlyPerVerseRecordings).toBe(false); + expect(result.current.hasActivePericopeTake).toBe(true); + expect(result.current.activePericopeTake?.id).toBe(600); + }); +}); diff --git a/src/features/audio/hooks/useStitchedAudio.ts b/src/features/audio/hooks/useStitchedAudio.ts new file mode 100644 index 00000000..5df6d2d6 --- /dev/null +++ b/src/features/audio/hooks/useStitchedAudio.ts @@ -0,0 +1,131 @@ +import { useMemo } from 'react'; + +import { type VerseAudioRecording, type VerseAudioTake } from '@/lib/types'; + +export interface StitchedTrack { + verseNumber: number; + takeId: number; + downloadUrl: string; + durationSeconds: number; +} + +export type SkippedReason = 'conflict' | 'no recording'; + +export interface SkippedVerse { + verseNumber: number; + reason: SkippedReason; +} + +export interface UseStitchedAudioOptions { + /** Ordered list of verse numbers in the pericope */ + verseNumbers: number[]; + /** Chapter audio recordings */ + recordings: VerseAudioRecording[]; +} + +export interface StitchedAudioResult { + /** Whether the pericope has an active pericope-level take (takes precedence over stitching) */ + hasActivePericopeTake: boolean; + /** Active pericope take if present */ + activePericopeTake?: VerseAudioTake; + /** All pericope-level takes covering this pericope */ + pericopeTakes: VerseAudioTake[]; + /** Whether the pericope has only per-verse recordings */ + hasOnlyPerVerseRecordings: boolean; + /** Ordered active verse tracks to stitch together */ + tracks: StitchedTrack[]; + /** Verses skipped from stitching, with reason */ + skippedVerses: SkippedVerse[]; + /** Total duration in seconds of all stitched tracks */ + totalDuration: number; +} + +/** Check if a recording is at pericope granularity */ +export function isPericopeRecording(r: VerseAudioRecording): boolean { + return ( + r.isPericope === true || + r.granularity === 'pericope' || + Boolean(r.pericopeRange && r.pericopeRange[0] !== r.pericopeRange[1]) || + r.takes.some(t => t.isPericope) + ); +} + +export function useStitchedAudio({ + verseNumbers, + recordings, +}: UseStitchedAudioOptions): StitchedAudioResult { + return useMemo(() => { + const sortedVerseNums = [...verseNumbers].sort((a, b) => a - b); + const startVerse = sortedVerseNums[0]; + const endVerse = sortedVerseNums[sortedVerseNums.length - 1]; + + // Check for pericope-level recordings covering this pericope + const pericopeRecordings = recordings.filter(r => { + if (!isPericopeRecording(r)) return false; + if (r.pericopeRange) { + return r.pericopeRange[0] === startVerse && r.pericopeRange[1] === endVerse; + } + return r.verseNumber === startVerse; + }); + + const pericopeTakes = pericopeRecordings.flatMap(r => r.takes); + const activePericopeRecording = pericopeRecordings.find( + r => r.conflictStatus !== 'conflict' && r.activeTakeId !== null + ); + const activePericopeTake = activePericopeRecording?.takes.find( + t => t.id === activePericopeRecording.activeTakeId + ); + + const hasActivePericopeTake = Boolean(activePericopeTake); + const hasOnlyPerVerseRecordings = pericopeTakes.length === 0; + + // Build verse-by-verse recording map for quick lookup + const verseRecordingMap = new Map(); + for (const r of recordings) { + if (!isPericopeRecording(r)) { + verseRecordingMap.set(r.verseNumber, r); + } + } + + const tracks: StitchedTrack[] = []; + const skippedVerses: SkippedVerse[] = []; + + for (const v of sortedVerseNums) { + const rec = verseRecordingMap.get(v); + if (!rec) { + skippedVerses.push({ verseNumber: v, reason: 'no recording' }); + continue; + } + + if (rec.conflictStatus === 'conflict') { + skippedVerses.push({ verseNumber: v, reason: 'conflict' }); + continue; + } + + const activeTake = rec.takes.find(t => t.id === rec.activeTakeId); + if (!activeTake?.downloadUrl) { + skippedVerses.push({ verseNumber: v, reason: 'no recording' }); + continue; + } + + tracks.push({ + verseNumber: v, + takeId: activeTake.id, + downloadUrl: activeTake.downloadUrl, + durationSeconds: activeTake.durationSeconds ?? 0, + }); + } + + const totalDuration = tracks.reduce((sum, t) => sum + t.durationSeconds, 0); + + return { + hasActivePericopeTake, + activePericopeTake, + pericopeTakes, + hasOnlyPerVerseRecordings, + tracks, + skippedVerses, + totalDuration, + }; + }, [verseNumbers, recordings]); +} diff --git a/src/lib/types.ts b/src/lib/types.ts index d677556f..785c6e1f 100644 --- a/src/lib/types.ts +++ b/src/lib/types.ts @@ -465,6 +465,9 @@ export interface VerseAudioTake { downloadUrl: string; createdAt: string; updatedAt: string; + isPericope?: boolean; + pericopeRange?: [number, number]; + pericopeNumber?: string; } export interface VerseAudioRecording { @@ -484,6 +487,12 @@ export interface VerseAudioRecording { takes: VerseAudioTake[]; createdAt: string; updatedAt: string; + granularity?: 'verse' | 'pericope'; + isPericope?: boolean; + pericopeRange?: [number, number]; + pericopeNumber?: string; + startVerse?: number; + endVerse?: number; } export interface ChapterAudioResponse { From 38fffbd3553df9258cf3aad9d7a2d76d2ce2259b Mon Sep 17 00:00:00 2001 From: joel-joseph-george Date: Mon, 19 Oct 2026 12:46:06 +0530 Subject: [PATCH 3/5] fix: handle pericope take selection rules and reset player state on track changes --- .../audio/components/AudioReviewPage.test.tsx | 132 ++++++++++++++++++ .../audio/components/AudioReviewPage.tsx | 10 +- .../components/StitchedAudioPlayer.test.tsx | 25 +++- .../audio/components/StitchedAudioPlayer.tsx | 57 ++++++-- 4 files changed, 206 insertions(+), 18 deletions(-) diff --git a/src/features/audio/components/AudioReviewPage.test.tsx b/src/features/audio/components/AudioReviewPage.test.tsx index fc4e29ef..5a4a4b1f 100644 --- a/src/features/audio/components/AudioReviewPage.test.tsx +++ b/src/features/audio/components/AudioReviewPage.test.tsx @@ -461,4 +461,136 @@ describe('AudioReviewPage pericope stitched audio & cross-view conflicts (Issue // Should display verse 1 take and pericope take labeled "Take 1 - Pericope - vv. 1-2" expect(screen.getByText('Take 1 - Pericope - vv. 1-2')).toBeInTheDocument(); }); + + it('renders pericope takes under covered verses in chapter view when pericope is clean', () => { + useAppStore.setState({ displayMode: 'chapter' }); + + currentRecordings = [ + mockRecordings[0], + { + id: 99, + projectUnitId: 1, + bibleTextId: 1, + uploadedBy: 1, + contentType: 'audio/webm', + sizeBytes: 1000, + durationSeconds: 10, + versionToken: 1, + downloadUrl: 'https://example.com/p.webm', + verseNumber: 1, + isPericope: true, + pericopeRange: [1, 2], + conflictStatus: 'clean', + activeTakeId: 991, + createdAt: '2026-03-01T00:00:00Z', + updatedAt: '2026-03-01T00:00:00Z', + takes: [ + { + id: 991, + uploadedBy: 1, + contentType: 'audio/webm', + sizeBytes: 1000, + durationSeconds: 10, + contentHash: 'hash-991', + downloadUrl: 'https://example.com/991.webm', + createdAt: '2026-03-01T00:00:00Z', + updatedAt: '2026-03-01T00:00:00Z', + }, + ], + } as unknown as VerseAudioRecording, + ]; + + renderPage(); + + expect(screen.getByText('Take 1 - Pericope - vv. 1-2')).toBeInTheDocument(); + }); + + it('excludes covering pericope takes from Select as Draft when verse is in conflict', () => { + useAppStore.setState({ + displayMode: 'verse', + userdetail: { + id: 1, + username: 'alice', + displayName: 'Alice', + email: 'alice@example.com', + role: 'user', + grants: [ + { + orgId: 1, + projectId: 1, + roleId: 1, + roleName: 'Project Manager', + permissions: [], + }, + ], + lastActiveOrgId: 1, + }, + }); + + currentRecordings = [ + { + ...mockRecordings[0], + conflictStatus: 'conflict', + activeTakeId: 11, + takes: [ + mockRecordings[0].takes[0], + { + id: 112, + uploadedBy: 1, + contentType: 'audio/webm', + sizeBytes: 1000, + durationSeconds: 5, + contentHash: 'hash-112', + downloadUrl: 'https://example.com/112.webm', + createdAt: '2026-03-01T00:00:00Z', + updatedAt: '2026-03-01T00:00:00Z', + }, + ], + }, + { + id: 99, + projectUnitId: 1, + bibleTextId: 1, + uploadedBy: 1, + contentType: 'audio/webm', + sizeBytes: 1000, + durationSeconds: 10, + versionToken: 1, + downloadUrl: 'https://example.com/p.webm', + verseNumber: 1, + isPericope: true, + pericopeRange: [1, 2], + conflictStatus: 'clean', + activeTakeId: 991, + createdAt: '2026-03-01T00:00:00Z', + updatedAt: '2026-03-01T00:00:00Z', + takes: [ + { + id: 991, + uploadedBy: 1, + contentType: 'audio/webm', + sizeBytes: 1000, + durationSeconds: 10, + contentHash: 'hash-991', + downloadUrl: 'https://example.com/991.webm', + createdAt: '2026-03-01T00:00:00Z', + updatedAt: '2026-03-01T00:00:00Z', + }, + ], + } as unknown as VerseAudioRecording, + ]; + + renderPage(); + + // Select as Draft button should appear exactly once (for take 112 belonging to verse recording), + // and NOT for covering pericope take 991. + const selectButtons = screen.getAllByRole('button', { name: /select as draft/i }); + expect(selectButtons).toHaveLength(1); + }); + + it('renders without crashing when there are no recordings or passages', () => { + currentRecordings = []; + renderPage(); + expect(screen.getByText('No recorded takes found for this chapter.')).toBeInTheDocument(); + }); }); diff --git a/src/features/audio/components/AudioReviewPage.tsx b/src/features/audio/components/AudioReviewPage.tsx index ff7c0802..ae2c6b4c 100644 --- a/src/features/audio/components/AudioReviewPage.tsx +++ b/src/features/audio/components/AudioReviewPage.tsx @@ -116,6 +116,8 @@ interface Passage { const isPassage = (passage: Passage | null): passage is Passage => passage !== null; +const EMPTY_VERSE_NUMBERS: number[] = []; + interface AudioReviewPageProps { projectItem?: ProjectItem; } @@ -176,6 +178,7 @@ const AudioReviewPage: React.FC = ({ projectItem: propItem interface ReviewTake extends VerseAudioTake { displayLabel?: string; bibleTextId?: number; + isCoveringPericope?: boolean; } // ---------- Build passage list ---------- @@ -348,7 +351,7 @@ const AudioReviewPage: React.FC = ({ projectItem: propItem // ---------- Stitched audio calculation for pericope mode ---------- const stitchedAudio = useStitchedAudio({ - verseNumbers: effectivePassage.verseNumbers, + verseNumbers: selectedPassage?.verseNumbers ?? EMPTY_VERSE_NUMBERS, recordings: audioData?.items ?? [], }); @@ -370,11 +373,12 @@ const AudioReviewPage: React.FC = ({ projectItem: propItem const recordings = audioData?.items ?? []; const pericopeRecordings = recordings.filter(isPericopeRecording); - if (displayMode === 'verse') { + if (displayMode !== 'pericope') { const v = selectedPassage.verseNumbers[0]; const verseTakes: ReviewTake[] = (selectedPassage.recordings[0]?.takes ?? []).map(t => ({ ...t, bibleTextId: selectedPassage.recordings[0]?.bibleTextId, + isCoveringPericope: false, })); // Find pericope recordings covering this verse @@ -399,6 +403,7 @@ const AudioReviewPage: React.FC = ({ projectItem: propItem range, }), bibleTextId: pr.bibleTextId, + isCoveringPericope: true, }); }); } @@ -702,6 +707,7 @@ const AudioReviewPage: React.FC = ({ projectItem: propItem isPM && selectedHasConflict && !effectivePassage.isOppositeModeConflict && + !take.isCoveringPericope && take.id !== activeTakeId; return ( diff --git a/src/features/audio/components/StitchedAudioPlayer.test.tsx b/src/features/audio/components/StitchedAudioPlayer.test.tsx index f8219e86..768a7d01 100644 --- a/src/features/audio/components/StitchedAudioPlayer.test.tsx +++ b/src/features/audio/components/StitchedAudioPlayer.test.tsx @@ -81,7 +81,8 @@ describe('StitchedAudioPlayer', () => { fireEvent.keyDown(slider, { key: 'ArrowRight' }); // Advances time - expect(slider).toBeInTheDocument(); + expect(slider).toHaveAttribute('aria-valuetext', '0:01'); + expect(slider).toHaveAttribute('aria-valuenow', '5'); }); it('automatically advances to the next track when current track ends while playing', async () => { @@ -121,4 +122,26 @@ describe('StitchedAudioPlayer', () => { expect(onPause).toHaveBeenCalled(); expect(screen.getByRole('button', { name: /play/i })).toBeInTheDocument(); }); + + it('resets player state when tracks change without crashing on empty or reduced list', async () => { + const { rerender } = renderComponent(); + + const playBtn = screen.getByRole('button', { name: /play/i }); + fireEvent.click(playBtn); + + // Now update tracks to empty array + rerender( + + + + ); + + // Should reset without throwing error + expect(screen.getByRole('button', { name: /play/i })).toBeDisabled(); + }); }); diff --git a/src/features/audio/components/StitchedAudioPlayer.tsx b/src/features/audio/components/StitchedAudioPlayer.tsx index 29c52611..c7ce8919 100644 --- a/src/features/audio/components/StitchedAudioPlayer.tsx +++ b/src/features/audio/components/StitchedAudioPlayer.tsx @@ -66,11 +66,29 @@ const StitchedAudioPlayer: React.FC = ({ isPlayingRef.current = isPlaying; }, [isPlaying]); + // Reset player state when tracks change + useEffect(() => { + const audio = audioRef.current; + if (audio) { + audio.pause(); + } + setCurrentTrackIndex(0); + currentTrackIndexRef.current = 0; + setCurrentTime(0); + setIsPlaying(false); + setIsLoading(false); + if (releaseRef.current) { + releaseRef.current(); + releaseRef.current = null; + } + }, [tracks]); + // Keep audio.src initialized for the current track useEffect(() => { const audio = audioRef.current; if (!audio || !tracks.length) return; const track = tracks[currentTrackIndex]; + if (!track) return; if (!audio.src.includes(track.downloadUrl)) { audio.src = track.downloadUrl; } @@ -390,21 +408,30 @@ const StitchedAudioPlayer: React.FC = ({ className='mt-3 flex flex-col gap-1 border-t pt-2.5' style={{ borderColor: 'var(--border)' }} > - {skippedVerses.map(skipped => ( -
- - - {t('verseNotIncluded', 'Verse {{verse}} not included: {{reason}}', { - verse: skipped.verseNumber, - reason: skipped.reason, - })} - -
- ))} + {skippedVerses.map(skipped => { + const reasonText = + skipped.reason === 'conflict' + ? t('conflict', 'conflict') + : skipped.reason === 'no recording' + ? t('noRecording', 'no recording') + : skipped.reason; + + return ( +
+ + + {t('verseNotIncluded', 'Verse {{verse}} not included: {{reason}}', { + verse: skipped.verseNumber, + reason: reasonText, + })} + +
+ ); + })}
)} From a4553c256897ec3b36c27ac10f2e91e0afa63919 Mon Sep 17 00:00:00 2001 From: joel-joseph-george Date: Tue, 20 Oct 2026 12:21:08 +0530 Subject: [PATCH 4/5] feat: support cross-chapter pericope indicators and robust stitched audio playback handling --- docs/code-map.md | 4 +- docs/features/audio-playback/design.md | 2 +- .../audio/components/AudioReviewPage.test.tsx | 174 +++++++++++++- .../audio/components/AudioReviewPage.tsx | 216 ++++++++++++------ .../components/StitchedAudioPlayer.test.tsx | 105 ++++++++- .../audio/components/StitchedAudioPlayer.tsx | 168 ++++++++------ .../audio/hooks/useStitchedAudio.test.ts | 86 ++++++- src/features/audio/hooks/useStitchedAudio.ts | 27 ++- src/lib/types.ts | 3 - 9 files changed, 624 insertions(+), 161 deletions(-) diff --git a/docs/code-map.md b/docs/code-map.md index 3ceceae2..a790ea26 100644 --- a/docs/code-map.md +++ b/docs/code-map.md @@ -86,7 +86,9 @@ Where things live in `src/` and what each part calls. Setup and scripts are in t recorded in fluent-mobile, guarded to Project Managers — the route redirects anyone else and the drafting header only links it for PMs. `components/AudioReviewPage.tsx` lists a chapter's takes by verse or pericope (`components/TakeCard.tsx`, `components/AudioPlayer.tsx`), flags - conflicting ones and lets the PM pick the winner. It calls `GET /verse-audio` + conflicting ones and lets the PM pick the winner. For pericopes recorded only by verse, + `components/StitchedAudioPlayer.tsx` and `hooks/useStitchedAudio.ts` sequence active verse takes + into continuous playback. It calls `GET /verse-audio` (`hooks/useChapterAudio.ts`, which `DraftingUI` also uses for the conflict indicator and review link) and `POST /verse-audio/:projectUnitId/:bibleTextId/resolve` (`hooks/useResolveConflict.ts`). diff --git a/docs/features/audio-playback/design.md b/docs/features/audio-playback/design.md index 71e1a03c..e01e9439 100644 --- a/docs/features/audio-playback/design.md +++ b/docs/features/audio-playback/design.md @@ -185,7 +185,7 @@ The drafting delivery includes source and reference Bible audio in verse, perico Translation Notes, Questions, Words, and Open Study Notes remain later delivery. Their concrete controls and article chunking will be decided with that work. The native FIA guide/key-term player now honors the aggregate audio gate and Hide Audio, but it remains outside the shared playback registry. Later resource/FIA work can add app-wide exclusivity and decide whether FIA receives redesigned controls. Audio-only source Bibles are not ruled out by the segment model. -Audio review (take evaluation and conflict resolution) connects to the shared `PlaybackRegistry` for app-wide playback exclusivity: claiming playback immediately pauses any active drafting or resource audio, and subsequent sounding claimants pause the review player. Because take review requires side-by-side comparison of multiple candidate takes for the same verse or pericope with per-take visual waveform progress and fast A/B scrubbing, it retains a lightweight take-comparison player rather than the drafting queue, ScrubBar, and segment-resolution stack. +Audio review (take evaluation and conflict resolution) connects to the shared `PlaybackRegistry` for app-wide playback exclusivity: claiming playback immediately pauses any active drafting or resource audio, and subsequent sounding claimants pause the review player. Because take review requires side-by-side comparison of multiple candidate takes for the same verse or pericope with per-take visual waveform progress and fast A/B scrubbing, it retains a lightweight take-comparison player rather than the drafting queue, ScrubBar, and segment-resolution stack. Similarly, continuous playback of pericopes recorded only by verse uses a dedicated `StitchedAudioPlayer` that sequences active verse recordings within audio review and registers with `PlaybackRegistry`, rather than invoking the full drafting playback engine. ## 16. Current delivery decisions and later follow-ups diff --git a/src/features/audio/components/AudioReviewPage.test.tsx b/src/features/audio/components/AudioReviewPage.test.tsx index 5a4a4b1f..831897a2 100644 --- a/src/features/audio/components/AudioReviewPage.test.tsx +++ b/src/features/audio/components/AudioReviewPage.test.tsx @@ -141,6 +141,7 @@ vi.mock('@tanstack/react-router', () => ({ })); let currentRecordings = [...mockRecordings]; +let currentPericopes: PericopeGroup[] = mockPericopes; vi.mock('@/features/audio/hooks/useChapterAudio', () => ({ useChapterAudio: () => ({ @@ -157,7 +158,7 @@ vi.mock('@/features/projects/hooks/useProjectUsers', () => ({ vi.mock('@/features/pericopes/hooks/useChapterPericopes', () => ({ useChapterPericopes: () => ({ - data: mockPericopes, + data: currentPericopes, }), })); @@ -270,6 +271,7 @@ describe('AudioReviewPage display mode', () => { describe('AudioReviewPage pericope stitched audio & cross-view conflicts (Issue #554)', () => { beforeEach(() => { currentRecordings = [...mockRecordings]; + currentPericopes = mockPericopes; vi.spyOn(HTMLMediaElement.prototype, 'play').mockImplementation(() => Promise.resolve()); vi.spyOn(HTMLMediaElement.prototype, 'pause').mockImplementation(() => {}); useAppStore.setState({ @@ -285,7 +287,7 @@ describe('AudioReviewPage pericope stitched audio & cross-view conflicts (Issue orgId: 1, projectId: 1, roleId: 1, - roleName: 'project_manager', + roleName: 'Project Manager', permissions: [], }, ], @@ -593,4 +595,172 @@ describe('AudioReviewPage pericope stitched audio & cross-view conflicts (Issue renderPage(); expect(screen.getByText('No recorded takes found for this chapter.')).toBeInTheDocument(); }); + + it('displays both conflict dot and cross-view warning indicator in pericope view when both direct and verse conflicts exist', () => { + // Both direct pericope conflict and verse 1 conflict + currentRecordings = [ + { + ...mockRecordings[0], + conflictStatus: 'conflict', + }, + mockRecordings[1], + mockRecordings[2], + { + id: 99, + projectUnitId: 1, + bibleTextId: 1, + uploadedBy: 1, + contentType: 'audio/webm', + sizeBytes: 1000, + durationSeconds: 10, + versionToken: 1, + downloadUrl: 'https://example.com/p.webm', + verseNumber: 1, + isPericope: true, + pericopeRange: [1, 2], + conflictStatus: 'conflict', + activeTakeId: 991, + createdAt: '2026-03-01T00:00:00Z', + updatedAt: '2026-03-01T00:00:00Z', + takes: [ + { + id: 991, + uploadedBy: 1, + contentType: 'audio/webm', + sizeBytes: 1000, + durationSeconds: 10, + contentHash: 'hash-991', + downloadUrl: 'https://example.com/991.webm', + createdAt: '2026-03-01T00:00:00Z', + updatedAt: '2026-03-01T00:00:00Z', + }, + { + id: 992, + uploadedBy: 2, + contentType: 'audio/webm', + sizeBytes: 1000, + durationSeconds: 10, + contentHash: 'hash-992', + downloadUrl: 'https://example.com/992.webm', + createdAt: '2026-03-01T00:00:00Z', + updatedAt: '2026-03-01T00:00:00Z', + }, + ], + } as unknown as VerseAudioRecording, + ]; + + renderPage(); + + // Cross-view indicator is present on the row + expect(screen.getByText('Conflict in verse 1')).toBeInTheDocument(); + + // Direct conflict banner and cross-view notice are present + expect( + screen.getByText('This passage has conflicting takes — select one to resolve.') + ).toBeInTheDocument(); + expect(screen.getByText(/There is also a conflict recorded by verse\./)).toBeInTheDocument(); + expect(screen.getByRole('button', { name: /switch to verse view/i })).toBeInTheDocument(); + + // Direct pericope takes can still be resolved as draft + expect(screen.getAllByRole('button', { name: /select as draft/i })[0]).toBeInTheDocument(); + }); + + it('displays both conflict dot and cross-view warning indicator in verse view when both verse direct conflict and covering pericope conflict exist', () => { + useAppStore.setState({ displayMode: 'verse' }); + + currentRecordings = [ + { + ...mockRecordings[0], + conflictStatus: 'conflict', + takes: [ + mockRecordings[0].takes[0], + { + id: 113, + uploadedBy: 2, + contentType: 'audio/webm', + sizeBytes: 1000, + durationSeconds: 10, + contentHash: 'hash-113', + downloadUrl: 'https://example.com/113.webm', + createdAt: '2026-03-01T00:00:00Z', + updatedAt: '2026-03-01T00:00:00Z', + }, + ], + }, + mockRecordings[1], + mockRecordings[2], + { + id: 99, + projectUnitId: 1, + bibleTextId: 1, + uploadedBy: 1, + contentType: 'audio/webm', + sizeBytes: 1000, + durationSeconds: 10, + versionToken: 1, + downloadUrl: 'https://example.com/p.webm', + verseNumber: 1, + isPericope: true, + pericopeRange: [1, 2], + conflictStatus: 'conflict', + activeTakeId: 991, + createdAt: '2026-03-01T00:00:00Z', + updatedAt: '2026-03-01T00:00:00Z', + takes: [ + { + id: 991, + uploadedBy: 1, + contentType: 'audio/webm', + sizeBytes: 1000, + durationSeconds: 10, + contentHash: 'hash-991', + downloadUrl: 'https://example.com/991.webm', + createdAt: '2026-03-01T00:00:00Z', + updatedAt: '2026-03-01T00:00:00Z', + }, + ], + } as unknown as VerseAudioRecording, + ]; + + renderPage(); + + // Cross-view indicator is present on verse 1 row (and verse 2 row covered by pericope 1-2) + expect(screen.getAllByText('Conflict in pericope vv. 1-2')[0]).toBeInTheDocument(); + + // Direct conflict banner and cross-view notice are present + expect( + screen.getByText('This passage has conflicting takes — select one to resolve.') + ).toBeInTheDocument(); + expect( + screen.getByText(/There is also a conflict recorded as a pericope\./) + ).toBeInTheDocument(); + expect(screen.getByRole('button', { name: /switch to pericope view/i })).toBeInTheDocument(); + + // Direct verse takes can still be resolved as draft + expect(screen.getAllByRole('button', { name: /select as draft/i })[0]).toBeInTheDocument(); + }); + + it('honestly labels cross-chapter pericopes showing continuation in the next chapter', () => { + currentPericopes = [ + { + pericopeNumber: '1', + pericopeTitle: null, + verses: [ + { verseNumber: 1, chapterNumber: 6 }, + { verseNumber: 2, chapterNumber: 6 }, + { verseNumber: 1, chapterNumber: 7 }, + ], + }, + ]; + + renderPage(); + + // The passage in the list should note continuation in ch. 7 + expect(screen.getByText('Verses 1–2 (cont. in ch. 7)')).toBeInTheDocument(); + + // The StitchedAudioPlayer title should also note continuation + expect( + screen.getByText('Stitched from verse recordings, vv. 1-2 (cont. in ch. 7)') + ).toBeInTheDocument(); + }); }); diff --git a/src/features/audio/components/AudioReviewPage.tsx b/src/features/audio/components/AudioReviewPage.tsx index ae2c6b4c..fb4d4e6b 100644 --- a/src/features/audio/components/AudioReviewPage.tsx +++ b/src/features/audio/components/AudioReviewPage.tsx @@ -7,7 +7,11 @@ import { useTranslation } from 'react-i18next'; import { useChapterAudio } from '@/features/audio/hooks/useChapterAudio'; import { useResolveConflict } from '@/features/audio/hooks/useResolveConflict'; -import { isPericopeRecording, useStitchedAudio } from '@/features/audio/hooks/useStitchedAudio'; +import { + isPericopeRecording, + pericopeSpan, + useStitchedAudio, +} from '@/features/audio/hooks/useStitchedAudio'; import { targetTextQueryOptions } from '@/features/bible/hooks/useBibleTarget'; import { useChapterPericopes } from '@/features/pericopes/hooks/useChapterPericopes'; import { useProjectUsers } from '@/features/projects/hooks/useProjectUsers'; @@ -108,10 +112,12 @@ interface Passage { verseNumbers: number[]; recordings: VerseAudioRecording[]; hasConflict: boolean; + hasDirectConflict: boolean; wasConflicted: boolean; crossViewConflict?: string | null; isOppositeModeConflict?: boolean; conflictRecordedMode?: 'verse' | 'pericope'; + continuationNote?: string; } const isPassage = (passage: Passage | null): passage is Passage => passage !== null; @@ -208,10 +214,9 @@ const AudioReviewPage: React.FC = ({ projectItem: propItem // Direct pericope recordings const directPericopeRecs = pericopeRecordings.filter(r => { - if (r.pericopeRange) { - return r.pericopeRange[0] === startVerse && r.pericopeRange[1] === endVerse; - } - return r.verseNumber === startVerse; + const span = pericopeSpan(r); + if (!span) return false; + return span[0] === startVerse && span[1] === endVerse; }); // Verse recordings for verses in this pericope @@ -219,11 +224,41 @@ const AudioReviewPage: React.FC = ({ projectItem: propItem if (directPericopeRecs.length === 0 && groupVerseRecs.length === 0) return null; - const label = + const baseLabel = startVerse === endVerse ? t('verseLabel', 'Verse {{verse}}', { verse: startVerse }) : t('versesLabel', 'Verses {{start}}–{{end}}', { start: startVerse, end: endVerse }); + const prevChapterVerses = group.verses.filter( + v => v.chapterNumber < projectItem.chapterNumber + ); + const nextChapterVerses = group.verses.filter( + v => v.chapterNumber > projectItem.chapterNumber + ); + + let continuationNote: string | undefined; + if (prevChapterVerses.length > 0 && nextChapterVerses.length > 0) { + const prevCh = prevChapterVerses[prevChapterVerses.length - 1]?.chapterNumber; + const nextCh = nextChapterVerses[0]?.chapterNumber; + continuationNote = t( + 'pericopeContFromAndInNote', + '(cont. from ch. {{prevCh}}, in ch. {{nextCh}})', + { prevCh, nextCh } + ); + } else if (nextChapterVerses.length > 0) { + const nextCh = nextChapterVerses[0]?.chapterNumber; + continuationNote = t('pericopeContInNextNote', '(cont. in ch. {{nextCh}})', { + nextCh, + }); + } else if (prevChapterVerses.length > 0) { + const prevCh = prevChapterVerses[prevChapterVerses.length - 1]?.chapterNumber; + continuationNote = t('pericopeContFromPrevNote', '(cont. from ch. {{prevCh}})', { + prevCh, + }); + } + + const label = continuationNote ? `${baseLabel} ${continuationNote}` : baseLabel; + const hasDirectConflict = directPericopeRecs.some(isConflict); const conflictedVerses = groupVerseRecs .filter(isConflict) @@ -239,8 +274,7 @@ const AudioReviewPage: React.FC = ({ projectItem: propItem let isOppositeModeConflict = false; let conflictRecordedMode: 'verse' | 'pericope' | undefined; - if (hasVerseConflict && !hasDirectConflict) { - isOppositeModeConflict = true; + if (hasVerseConflict) { conflictRecordedMode = 'verse'; crossViewConflict = conflictedVerses.length === 1 @@ -250,6 +284,9 @@ const AudioReviewPage: React.FC = ({ projectItem: propItem : t('conflictInVerses', 'Conflict in verses {{verses}}', { verses: conflictedVerses.join(', '), }); + if (!hasDirectConflict) { + isOppositeModeConflict = true; + } } const passageRecs = directPericopeRecs.length > 0 ? directPericopeRecs : groupVerseRecs; @@ -260,10 +297,12 @@ const AudioReviewPage: React.FC = ({ projectItem: propItem verseNumbers: chapterVerses.map(v => v.verseNumber), recordings: passageRecs, hasConflict: hasConflictNow, + hasDirectConflict, wasConflicted, crossViewConflict, isOppositeModeConflict, conflictRecordedMode, + continuationNote, }; }) .filter(isPassage); @@ -276,7 +315,9 @@ const AudioReviewPage: React.FC = ({ projectItem: propItem verseNumSet.add(r.verseNumber); } for (const pr of pericopeRecordings) { - const [start, end] = pr.pericopeRange ?? [pr.verseNumber, pr.verseNumber]; + const span = pericopeSpan(pr); + if (!span) continue; + const [start, end] = span; for (let v = start; v <= end; v++) { verseNumSet.add(v); } @@ -289,7 +330,9 @@ const AudioReviewPage: React.FC = ({ projectItem: propItem // Check if this verse is covered by a pericope recording const coveringPericope = pericopeRecordings.find(pr => { - const [start, end] = pr.pericopeRange ?? [pr.verseNumber, pr.verseNumber]; + const span = pericopeSpan(pr); + if (!span) return false; + const [start, end] = span; return v >= start && v <= end; }); @@ -303,16 +346,18 @@ const AudioReviewPage: React.FC = ({ projectItem: propItem let isOppositeModeConflict = false; let conflictRecordedMode: 'verse' | 'pericope' | undefined; - if (hasPericopeConflict && coveringPericope && !hasDirectConflict) { - isOppositeModeConflict = true; - conflictRecordedMode = 'pericope'; - const [start, end] = coveringPericope.pericopeRange ?? [ - coveringPericope.verseNumber, - coveringPericope.verseNumber, - ]; - crossViewConflict = t('conflictInPericope', 'Conflict in pericope vv. {{range}}', { - range: `${start}-${end}`, - }); + if (hasPericopeConflict && coveringPericope) { + const span = pericopeSpan(coveringPericope); + if (span) { + conflictRecordedMode = 'pericope'; + const [start, end] = span; + crossViewConflict = t('conflictInPericope', 'Conflict in pericope vv. {{range}}', { + range: `${start}-${end}`, + }); + if (!hasDirectConflict) { + isOppositeModeConflict = true; + } + } } return { @@ -321,6 +366,7 @@ const AudioReviewPage: React.FC = ({ projectItem: propItem verseNumbers: [v], recordings: r ? [r] : [], hasConflict: hasConflictNow, + hasDirectConflict, wasConflicted, crossViewConflict, isOppositeModeConflict, @@ -383,7 +429,9 @@ const AudioReviewPage: React.FC = ({ projectItem: propItem // Find pericope recordings covering this verse const coveringPericopes = pericopeRecordings.filter(pr => { - const [start, end] = pr.pericopeRange ?? [pr.verseNumber, pr.verseNumber]; + const span = pericopeSpan(pr); + if (!span) return false; + const [start, end] = span; return v >= start && v <= end; }); @@ -393,7 +441,9 @@ const AudioReviewPage: React.FC = ({ projectItem: propItem // the cross-view conflict indicator and message apply instead." if (isConflict(pr)) continue; - const [start, end] = pr.pericopeRange ?? [pr.verseNumber, pr.verseNumber]; + const span = pericopeSpan(pr); + if (!span) continue; + const [start, end] = span; const range = `${start}-${end}`; pr.takes.forEach((take, idx) => { pericopeTakes.push({ @@ -444,7 +494,9 @@ const AudioReviewPage: React.FC = ({ projectItem: propItem const v = selectedPassage?.verseNumbers[0]; if (v !== undefined) { const coveringPericope = pericopeRecordings.find(pr => { - const [start, end] = pr.pericopeRange ?? [pr.verseNumber, pr.verseNumber]; + const span = pericopeSpan(pr); + if (!span) return false; + const [start, end] = span; return v >= start && v <= end && !isConflict(pr); }); if (coveringPericope && coveringPericope.activeTakeId !== null) { @@ -563,14 +615,12 @@ const AudioReviewPage: React.FC = ({ projectItem: propItem style={{ scrollbarGutter: 'stable' }} > {passages.map(passage => { - // Dot: orange = currently in conflict; green = API confirmed 'resolved'; none otherwise + // Dot: orange = currently in direct conflict in this view; green = API confirmed 'resolved'; none otherwise let dotState: 'conflict' | 'resolved' | null = null; - if (passage.hasConflict) dotState = 'conflict'; - else if (passage.wasConflicted) dotState = 'resolved'; - - // If crossViewConflict is present, suppress the simple dot in favour of the warning triangle - if (passage.crossViewConflict) { - dotState = null; + if (passage.hasDirectConflict) { + dotState = 'conflict'; + } else if (passage.wasConflicted && !passage.hasConflict) { + dotState = 'resolved'; } const isPassageResolved = dotState === 'resolved'; @@ -633,40 +683,75 @@ const AudioReviewPage: React.FC = ({ projectItem: propItem style={{ color: 'var(--warning-border)' }} /> - {effectivePassage.isOppositeModeConflict ? ( - effectivePassage.conflictRecordedMode === 'verse' ? ( - <> - {t('conflictRecordedByVerseLead', 'This conflict was recorded by verse.')}{' '} - {' '} - {t('toResolveIt', 'to resolve it.')} - - ) : ( - <> - {t( - 'conflictRecordedAsPericopeLead', - 'This conflict was recorded as a pericope.' - )}{' '} - {' '} - {t('toResolveIt', 'to resolve it.')} - - ) + {effectivePassage.hasDirectConflict ? ( + <> + {t( + 'conflictBanner', + 'This passage has conflicting takes — select one to resolve.' + )} + {effectivePassage.crossViewConflict && ( + + {effectivePassage.conflictRecordedMode === 'verse' ? ( + <> + {t( + 'conflictAlsoRecordedByVerse', + 'There is also a conflict recorded by verse.' + )}{' '} + {' '} + {t('toResolveIt', 'to resolve it.')} + + ) : ( + <> + {t( + 'conflictAlsoRecordedAsPericope', + 'There is also a conflict recorded as a pericope.' + )}{' '} + {' '} + {t('toResolveIt', 'to resolve it.')} + + )} + + )} + + ) : effectivePassage.conflictRecordedMode === 'verse' ? ( + <> + {t('conflictRecordedByVerseLead', 'This conflict was recorded by verse.')}{' '} + {' '} + {t('toResolveIt', 'to resolve it.')} + ) : ( - t( - 'conflictBanner', - 'This passage has conflicting takes — select one to resolve.' - ) + <> + {t( + 'conflictRecordedAsPericopeLead', + 'This conflict was recorded as a pericope.' + )}{' '} + {' '} + {t('toResolveIt', 'to resolve it.')} + )} @@ -682,13 +767,16 @@ const AudioReviewPage: React.FC = ({ projectItem: propItem { const mockSkipped = [ { verseNumber: 6, reason: 'conflict' as const }, { verseNumber: 7, reason: 'no recording' as const }, + { verseNumber: 8, reason: 'no active take' as const }, ]; beforeEach(() => { @@ -54,24 +55,21 @@ describe('StitchedAudioPlayer', () => { expect(screen.getByText('0:23')).toBeInTheDocument(); expect(screen.getByText('Verse 6 not included: conflict')).toBeInTheDocument(); expect(screen.getByText('Verse 7 not included: no recording')).toBeInTheDocument(); + expect(screen.getByText('Verse 8 not included: no active take')).toBeInTheDocument(); }); it('toggles play and pause on button click', async () => { - const onPlay = vi.fn(); - const onPause = vi.fn(); - renderComponent({ onPlay, onPause }); + renderComponent(); const playBtn = screen.getByRole('button', { name: /play/i }); fireEvent.click(playBtn); expect(HTMLMediaElement.prototype.play).toHaveBeenCalled(); - expect(onPlay).toHaveBeenCalled(); const pauseBtn = await screen.findByRole('button', { name: /pause/i }); fireEvent.click(pauseBtn); expect(HTMLMediaElement.prototype.pause).toHaveBeenCalled(); - expect(onPause).toHaveBeenCalled(); }); it('supports seeking via keyboard ArrowRight on the slider', () => { @@ -103,8 +101,7 @@ describe('StitchedAudioPlayer', () => { }); it('stops and resets when the last track ends', async () => { - const onPause = vi.fn(); - const { container } = renderComponent({ onPause }); + const { container } = renderComponent(); const playBtn = screen.getByRole('button', { name: /play/i }); fireEvent.click(playBtn); @@ -119,7 +116,6 @@ describe('StitchedAudioPlayer', () => { fireEvent.ended(audioElement!); // Now playback is paused and reset to end - expect(onPause).toHaveBeenCalled(); expect(screen.getByRole('button', { name: /play/i })).toBeInTheDocument(); }); @@ -144,4 +140,97 @@ describe('StitchedAudioPlayer', () => { // Should reset without throwing error expect(screen.getByRole('button', { name: /play/i })).toBeDisabled(); }); + + it('preserves playback position and continues playing when only presigned URLs change', async () => { + const { container, rerender } = renderComponent(); + + const playBtn = screen.getByRole('button', { name: /play/i }); + fireEvent.click(playBtn); + + const audioElement = container.querySelector('audio')!; + expect(audioElement).toHaveAttribute('src', 'https://example.com/take-81.webm'); + + // Simulate playback advancing 5 seconds into track 0 + audioElement.currentTime = 5; + fireEvent.timeUpdate(audioElement); + + const pauseSpy = vi.spyOn(HTMLMediaElement.prototype, 'pause'); + + // Refetch produces new presigned URLs but same track identities (verseNumber:takeId) + const refreshedTracks = [ + { + verseNumber: 8, + takeId: 81, + downloadUrl: 'https://example.com/take-81.webm?expires=123456', + durationSeconds: 10, + }, + { + verseNumber: 9, + takeId: 91, + downloadUrl: 'https://example.com/take-91.webm?expires=123456', + durationSeconds: 13, + }, + ]; + + rerender( + + + + ); + + // Audio src is swapped to the new URL + expect(audioElement).toHaveAttribute('src', 'https://example.com/take-81.webm?expires=123456'); + // Position is preserved and playback continues (pause was not called) + expect(audioElement.currentTime).toBe(5); + expect(pauseSpy).not.toHaveBeenCalled(); + expect(screen.getByRole('button', { name: /pause/i })).toBeInTheDocument(); + }); + + it('resets player state when track identity (takeId) changes', async () => { + const { container, rerender } = renderComponent(); + + const playBtn = screen.getByRole('button', { name: /play/i }); + fireEvent.click(playBtn); + + const audioElement = container.querySelector('audio')!; + audioElement.currentTime = 5; + fireEvent.timeUpdate(audioElement); + + // Track identity changed: verse 8 has a different takeId (take 82 instead of 81) + const newTakesTracks = [ + { + verseNumber: 8, + takeId: 82, + downloadUrl: 'https://example.com/take-82.webm', + durationSeconds: 10, + }, + { + verseNumber: 9, + takeId: 91, + downloadUrl: 'https://example.com/take-91.webm', + durationSeconds: 13, + }, + ]; + + rerender( + + + + ); + + // Player state should reset to paused and start of track 0 + expect(screen.getByRole('button', { name: /play/i })).toBeInTheDocument(); + expect(audioElement.currentTime).toBe(0); + expect(audioElement).toHaveAttribute('src', 'https://example.com/take-82.webm'); + }); }); diff --git a/src/features/audio/components/StitchedAudioPlayer.tsx b/src/features/audio/components/StitchedAudioPlayer.tsx index c7ce8919..7221006d 100644 --- a/src/features/audio/components/StitchedAudioPlayer.tsx +++ b/src/features/audio/components/StitchedAudioPlayer.tsx @@ -5,7 +5,11 @@ import { useTranslation } from 'react-i18next'; import { formatClock, usePlaybackRegistry } from '@/features/tts'; -import { type SkippedVerse, type StitchedTrack } from '../hooks/useStitchedAudio'; +import { + type SkippedReason, + type SkippedVerse, + type StitchedTrack, +} from '../hooks/useStitchedAudio'; export interface StitchedAudioPlayerProps { /** Title label, e.g. "Stitched from verse recordings, vv. 6-9" */ @@ -16,12 +20,6 @@ export interface StitchedAudioPlayerProps { skippedVerses?: SkippedVerse[]; /** Total calculated duration of the stitched tracks */ totalDuration: number; - /** Called when playback starts */ - onPlay?: () => void; - /** Called when playback pauses or ends */ - onPause?: () => void; - /** Accent color for play button and waveform fill */ - accentColor?: string; } const BARS = 40; @@ -31,6 +29,15 @@ const WAVEFORM_HEIGHTS = [ 0.75, 0.4, 0.25, ]; +function isSameSrc(currentSrc: string, targetUrl: string): boolean { + if (!currentSrc || !targetUrl) return false; + try { + return new URL(targetUrl, window.location.href).href === currentSrc; + } catch { + return currentSrc.includes(targetUrl); + } +} + /** * Continuous stitched audio player for verse-recorded pericopes. * Sequences active verse takes sequentially and provides a single unified scrub bar. @@ -41,9 +48,6 @@ const StitchedAudioPlayer: React.FC = ({ tracks, skippedVerses = [], totalDuration, - onPlay, - onPause, - accentColor = 'var(--primary)', }) => { const { t } = useTranslation(); const registry = usePlaybackRegistry(); @@ -66,31 +70,64 @@ const StitchedAudioPlayer: React.FC = ({ isPlayingRef.current = isPlaying; }, [isPlaying]); - // Reset player state when tracks change + const tracksKey = tracks.map(t => `${t.verseNumber}:${t.takeId}`).join(','); + const prevTracksKeyRef = useRef(tracksKey); + + // Reset player when track identity changes, and clean up on unmount useEffect(() => { const audio = audioRef.current; - if (audio) { - audio.pause(); - } - setCurrentTrackIndex(0); - currentTrackIndexRef.current = 0; - setCurrentTime(0); - setIsPlaying(false); - setIsLoading(false); - if (releaseRef.current) { - releaseRef.current(); - releaseRef.current = null; + if (prevTracksKeyRef.current !== tracksKey) { + prevTracksKeyRef.current = tracksKey; + if (audio) { + audio.pause(); + audio.currentTime = 0; + } + setCurrentTrackIndex(0); + currentTrackIndexRef.current = 0; + setCurrentTime(0); + setIsPlaying(false); + isPlayingRef.current = false; + setIsLoading(false); + if (releaseRef.current) { + releaseRef.current(); + releaseRef.current = null; + } } - }, [tracks]); - // Keep audio.src initialized for the current track + return () => { + if (audio) { + audio.pause(); + } + if (releaseRef.current) { + releaseRef.current(); + releaseRef.current = null; + } + }; + }, [tracksKey]); + + // Keep audio.src initialized for the current track; when only URLs change, keep currentTime and playback state useEffect(() => { const audio = audioRef.current; if (!audio || !tracks.length) return; - const track = tracks[currentTrackIndex]; + const track = tracks[currentTrackIndex] as StitchedTrack | undefined; if (!track) return; - if (!audio.src.includes(track.downloadUrl)) { + if (!isSameSrc(audio.src, track.downloadUrl)) { + const prevTime = audio.currentTime; + const wasPlaying = isPlayingRef.current; audio.src = track.downloadUrl; + if (prevTime > 0) { + audio.currentTime = prevTime; + const restoreTime = () => { + audio.currentTime = prevTime; + }; + audio.addEventListener('loadedmetadata', restoreTime, { once: true }); + } + if (wasPlaying) { + void audio.play().catch((err: unknown) => { + if (err instanceof DOMException && err.name === 'AbortError') return; + setIsPlaying(false); + }); + } } }, [tracks, currentTrackIndex]); @@ -117,22 +154,7 @@ const StitchedAudioPlayer: React.FC = ({ releaseRef.current(); releaseRef.current = null; } - onPause?.(); - }, [onPause]); - - // Clean up on unmount or tracks change - useEffect(() => { - const audio = audioRef.current; - return () => { - if (audio) { - audio.pause(); - } - if (releaseRef.current) { - releaseRef.current(); - releaseRef.current = null; - } - }; - }, [tracks]); + }, []); // Helper to locate which track and internal offset correspond to a given total time const locateTrack = useCallback( @@ -188,7 +210,6 @@ const StitchedAudioPlayer: React.FC = ({ if (isPlaying) { pausePlayback(); } else { - onPlay?.(); setIsLoading(true); // Claim exclusivity in the shared registry @@ -199,7 +220,6 @@ const StitchedAudioPlayer: React.FC = ({ setIsPlaying(false); setIsLoading(false); releaseRef.current = null; - onPause?.(); }); // If at end, start from beginning @@ -211,8 +231,12 @@ const StitchedAudioPlayer: React.FC = ({ audio.currentTime = 0; } else { const track = tracks[currentTrackIndexRef.current]; - if (!audio.src.includes(track.downloadUrl)) { + if (!isSameSrc(audio.src, track.downloadUrl)) { + const savedTime = audio.currentTime; audio.src = track.downloadUrl; + if (savedTime > 0) { + audio.currentTime = savedTime; + } } } @@ -240,7 +264,7 @@ const StitchedAudioPlayer: React.FC = ({ } }); } - }, [tracks, isPlaying, onPlay, onPause, pausePlayback, registry, currentTime, totalDuration]); + }, [tracks, isPlaying, pausePlayback, registry, currentTime, totalDuration]); const handleTimeUpdate = useCallback(() => { const audio = audioRef.current; @@ -275,9 +299,8 @@ const StitchedAudioPlayer: React.FC = ({ releaseRef.current(); releaseRef.current = null; } - onPause?.(); } - }, [tracks, totalDuration, onPause]); + }, [tracks, totalDuration]); const handleSeek = useCallback( (e: React.MouseEvent) => { @@ -329,6 +352,12 @@ const StitchedAudioPlayer: React.FC = ({ const progress = totalDuration > 0 ? currentTime / totalDuration : 0; const hasTracks = tracks.length > 0; + const skippedReasonLabels: Record = { + conflict: t('conflict', 'conflict'), + 'no recording': t('noRecording', 'no recording'), + 'no active take': t('noActiveTake', 'no active take'), + }; + return (
= ({ aria-label={isPlaying ? t('pause', 'Pause') : t('play', 'Play')} className='flex h-9 w-9 shrink-0 cursor-pointer items-center justify-center rounded-full text-white transition-transform hover:scale-105 active:scale-95 disabled:opacity-60' disabled={!hasTracks || isLoading} - style={{ backgroundColor: accentColor }} + style={{ backgroundColor: 'var(--primary)' }} type='button' onClick={handleToggle} > @@ -388,7 +417,7 @@ const StitchedAudioPlayer: React.FC = ({ className='flex-1 rounded-sm transition-all duration-75' style={{ height: `${h * 100}%`, - backgroundColor: isPast ? accentColor : 'var(--border)', + backgroundColor: isPast ? 'var(--primary)' : 'var(--border)', minWidth: 0, }} /> @@ -408,30 +437,21 @@ const StitchedAudioPlayer: React.FC = ({ className='mt-3 flex flex-col gap-1 border-t pt-2.5' style={{ borderColor: 'var(--border)' }} > - {skippedVerses.map(skipped => { - const reasonText = - skipped.reason === 'conflict' - ? t('conflict', 'conflict') - : skipped.reason === 'no recording' - ? t('noRecording', 'no recording') - : skipped.reason; - - return ( -
- - - {t('verseNotIncluded', 'Verse {{verse}} not included: {{reason}}', { - verse: skipped.verseNumber, - reason: reasonText, - })} - -
- ); - })} + {skippedVerses.map(skipped => ( +
+ + + {t('verseNotIncluded', 'Verse {{verse}} not included: {{reason}}', { + verse: skipped.verseNumber, + reason: skippedReasonLabels[skipped.reason], + })} + +
+ ))}
)} diff --git a/src/features/audio/hooks/useStitchedAudio.test.ts b/src/features/audio/hooks/useStitchedAudio.test.ts index 40b99104..b236480c 100644 --- a/src/features/audio/hooks/useStitchedAudio.test.ts +++ b/src/features/audio/hooks/useStitchedAudio.test.ts @@ -3,7 +3,7 @@ import { describe, expect, it } from 'vitest'; import { type VerseAudioRecording } from '@/lib/types'; -import { useStitchedAudio } from './useStitchedAudio'; +import { pericopeSpan, useStitchedAudio } from './useStitchedAudio'; describe('useStitchedAudio', () => { const makeRecording = ( @@ -114,10 +114,43 @@ describe('useStitchedAudio', () => { }) ); + expect(result.current.tracks.map(t => t.verseNumber)).toEqual([2]); + expect(result.current.skippedVerses).toEqual([{ verseNumber: 1, reason: 'no active take' }]); + }); + + it('marks verse as "no recording" when recording has empty takes', () => { + const recordings = [ + makeRecording(1, [], null, 'clean'), + makeRecording(2, [{ id: 21, durationSeconds: 7 }], 21, 'clean'), + ]; + + const { result } = renderHook(() => + useStitchedAudio({ + verseNumbers: [1, 2], + recordings, + }) + ); + expect(result.current.tracks.map(t => t.verseNumber)).toEqual([2]); expect(result.current.skippedVerses).toEqual([{ verseNumber: 1, reason: 'no recording' }]); }); + it('marks verse as "no active take" when active take lacks a downloadUrl', () => { + const rec1 = makeRecording(1, [{ id: 11, durationSeconds: 5 }], 11, 'clean'); + rec1.takes[0].downloadUrl = null as unknown as string; + const recordings = [rec1, makeRecording(2, [{ id: 21, durationSeconds: 7 }], 21, 'clean')]; + + const { result } = renderHook(() => + useStitchedAudio({ + verseNumbers: [1, 2], + recordings, + }) + ); + + expect(result.current.tracks.map(t => t.verseNumber)).toEqual([2]); + expect(result.current.skippedVerses).toEqual([{ verseNumber: 1, reason: 'no active take' }]); + }); + it('detects active pericope-level take when pericope recording is present and clean', () => { const pericopeRecording: VerseAudioRecording = { ...makeRecording(6, [{ id: 600, durationSeconds: 25 }], 600), @@ -142,4 +175,55 @@ describe('useStitchedAudio', () => { expect(result.current.hasActivePericopeTake).toBe(true); expect(result.current.activePericopeTake?.id).toBe(600); }); + + it('skips pericope recordings that have no range instead of falling back to single verse', () => { + // Pericope recording marked isPericope: true but with missing range + const rangelessPericope: VerseAudioRecording = { + ...makeRecording(6, [{ id: 600, durationSeconds: 25 }], 600), + isPericope: true, + }; + + const recordings = [ + rangelessPericope, + makeRecording(6, [{ id: 61, durationSeconds: 5 }], 61), + makeRecording(7, [{ id: 71, durationSeconds: 6 }], 71), + ]; + + const { result } = renderHook(() => + useStitchedAudio({ + verseNumbers: [6, 7], + recordings, + }) + ); + + // Should NOT treat rangelessPericope as covering verse 6 + expect(result.current.hasOnlyPerVerseRecordings).toBe(true); + expect(result.current.hasActivePericopeTake).toBe(false); + }); +}); + +describe('pericopeSpan', () => { + it('returns [start, end] when pericopeRange is present on recording', () => { + const r = { + pericopeRange: [3, 7] as [number, number], + takes: [], + } as unknown as VerseAudioRecording; + expect(pericopeSpan(r)).toEqual([3, 7]); + }); + + it('returns [start, end] when pericopeRange is on take', () => { + const r = { + takes: [{ pericopeRange: [4, 8] }], + } as unknown as VerseAudioRecording; + expect(pericopeSpan(r)).toEqual([4, 8]); + }); + + it('returns null when range is missing or incomplete', () => { + const r = { + verseNumber: 3, + isPericope: true, + takes: [], + } as unknown as VerseAudioRecording; + expect(pericopeSpan(r)).toBeNull(); + }); }); diff --git a/src/features/audio/hooks/useStitchedAudio.ts b/src/features/audio/hooks/useStitchedAudio.ts index 5df6d2d6..34bce41e 100644 --- a/src/features/audio/hooks/useStitchedAudio.ts +++ b/src/features/audio/hooks/useStitchedAudio.ts @@ -9,7 +9,7 @@ export interface StitchedTrack { durationSeconds: number; } -export type SkippedReason = 'conflict' | 'no recording'; +export type SkippedReason = 'conflict' | 'no recording' | 'no active take'; export interface SkippedVerse { verseNumber: number; @@ -50,6 +50,17 @@ export function isPericopeRecording(r: VerseAudioRecording): boolean { ); } +export function pericopeSpan(r: VerseAudioRecording): [number, number] | null { + const range = r.pericopeRange ?? r.takes[0]?.pericopeRange; + if (range) { + const [start, end] = range; + if (typeof start === 'number' && typeof end === 'number') { + return [start, end]; + } + } + return null; +} + export function useStitchedAudio({ verseNumbers, recordings, @@ -62,10 +73,9 @@ export function useStitchedAudio({ // Check for pericope-level recordings covering this pericope const pericopeRecordings = recordings.filter(r => { if (!isPericopeRecording(r)) return false; - if (r.pericopeRange) { - return r.pericopeRange[0] === startVerse && r.pericopeRange[1] === endVerse; - } - return r.verseNumber === startVerse; + const span = pericopeSpan(r); + if (!span) return false; + return span[0] === startVerse && span[1] === endVerse; }); const pericopeTakes = pericopeRecordings.flatMap(r => r.takes); @@ -77,6 +87,9 @@ export function useStitchedAudio({ ); const hasActivePericopeTake = Boolean(activePericopeTake); + // When any pericope-level takes exist (including while in conflict or pending resolution), + // the takes list is shown so the PM can review and resolve the pericope-level takes. + // Verse stitching is only used when the pericope has exclusively per-verse recordings. const hasOnlyPerVerseRecordings = pericopeTakes.length === 0; // Build verse-by-verse recording map for quick lookup @@ -92,7 +105,7 @@ export function useStitchedAudio({ for (const v of sortedVerseNums) { const rec = verseRecordingMap.get(v); - if (!rec) { + if (!rec || rec.takes.length === 0) { skippedVerses.push({ verseNumber: v, reason: 'no recording' }); continue; } @@ -104,7 +117,7 @@ export function useStitchedAudio({ const activeTake = rec.takes.find(t => t.id === rec.activeTakeId); if (!activeTake?.downloadUrl) { - skippedVerses.push({ verseNumber: v, reason: 'no recording' }); + skippedVerses.push({ verseNumber: v, reason: 'no active take' }); continue; } diff --git a/src/lib/types.ts b/src/lib/types.ts index 785c6e1f..1ae18aaa 100644 --- a/src/lib/types.ts +++ b/src/lib/types.ts @@ -490,9 +490,6 @@ export interface VerseAudioRecording { granularity?: 'verse' | 'pericope'; isPericope?: boolean; pericopeRange?: [number, number]; - pericopeNumber?: string; - startVerse?: number; - endVerse?: number; } export interface ChapterAudioResponse { From a2eeb3824a41e042936d59e22d90f532a08ff20c Mon Sep 17 00:00:00 2001 From: joel-joseph-george Date: Tue, 20 Oct 2026 12:59:31 +0530 Subject: [PATCH 5/5] fix: select conflicted verse when switching from pericope to verse view in AudioReviewPage --- .../audio/components/AudioReviewPage.test.tsx | 24 ++++++++++++++ .../audio/components/AudioReviewPage.tsx | 33 ++++++++++++++++--- 2 files changed, 53 insertions(+), 4 deletions(-) diff --git a/src/features/audio/components/AudioReviewPage.test.tsx b/src/features/audio/components/AudioReviewPage.test.tsx index 831897a2..8ffe38fa 100644 --- a/src/features/audio/components/AudioReviewPage.test.tsx +++ b/src/features/audio/components/AudioReviewPage.test.tsx @@ -266,6 +266,30 @@ describe('AudioReviewPage display mode', () => { renderPage(); expect(screen.getByText('Verses 1–2')).toBeInTheDocument(); }); + + it('selects the conflicted verse when switching to verse view from a pericope with conflict on a later verse', () => { + currentRecordings = [ + mockRecordings[0], + { + ...mockRecordings[1], // verse 2 has the conflict + conflictStatus: 'conflict', + }, + mockRecordings[2], + ]; + useAppStore.setState({ displayMode: 'pericope' }); + renderPage(); + + const switchButton = screen.getByRole('button', { name: /switch to verse view/i }); + fireEvent.click(switchButton); + + // Selected passage in verse view should be Verse 2, showing its conflicting takes header + expect( + screen.getByRole('heading', { level: 2, name: /conflicting takes/i }) + ).toBeInTheDocument(); + // Verse 2 list item should be selected (has bg-accent class) + const verse2Button = screen.getByRole('button', { name: /verse 2/i }); + expect(verse2Button).toHaveClass('bg-accent'); + }); }); describe('AudioReviewPage pericope stitched audio & cross-view conflicts (Issue #554)', () => { diff --git a/src/features/audio/components/AudioReviewPage.tsx b/src/features/audio/components/AudioReviewPage.tsx index fb4d4e6b..7a13eebd 100644 --- a/src/features/audio/components/AudioReviewPage.tsx +++ b/src/features/audio/components/AudioReviewPage.tsx @@ -118,6 +118,7 @@ interface Passage { isOppositeModeConflict?: boolean; conflictRecordedMode?: 'verse' | 'pericope'; continuationNote?: string; + conflictedVerses?: number[]; } const isPassage = (passage: Passage | null): passage is Passage => passage !== null; @@ -303,6 +304,7 @@ const AudioReviewPage: React.FC = ({ projectItem: propItem isOppositeModeConflict, conflictRecordedMode, continuationNote, + conflictedVerses, }; }) .filter(isPassage); @@ -394,6 +396,29 @@ const AudioReviewPage: React.FC = ({ projectItem: propItem const effectivePassage = selectedPassage ?? passages[0]; + const handleSwitchToVerseView = () => { + const targetVerse = effectivePassage.conflictedVerses?.[0] ?? effectivePassage.verseNumbers[0]; + if (typeof targetVerse === 'number') { + setSelectedPassageId(`verse-${targetVerse}`); + } + setDisplayMode('verse'); + }; + + const handleSwitchToPericopeView = () => { + const v = effectivePassage.verseNumbers[0]; + if (typeof v === 'number' && pericopes) { + const coveringGroup = pericopes.find(group => + group.verses.some( + gv => gv.chapterNumber === projectItem.chapterNumber && gv.verseNumber === v + ) + ); + if (coveringGroup) { + setSelectedPassageId(`pericope-${coveringGroup.pericopeNumber}`); + } + } + setDisplayMode('pericope'); + }; + // ---------- Stitched audio calculation for pericope mode ---------- const stitchedAudio = useStitchedAudio({ @@ -700,7 +725,7 @@ const AudioReviewPage: React.FC = ({ projectItem: propItem {' '} @@ -715,7 +740,7 @@ const AudioReviewPage: React.FC = ({ projectItem: propItem {' '} @@ -731,7 +756,7 @@ const AudioReviewPage: React.FC = ({ projectItem: propItem {' '} @@ -746,7 +771,7 @@ const AudioReviewPage: React.FC = ({ projectItem: propItem {' '}