diff --git a/.env.example b/.env.example index f29d81c..8974f05 100644 --- a/.env.example +++ b/.env.example @@ -111,9 +111,16 @@ PROVIDER_TIMEOUT_MS=45000 # ============================================================================= # Worker tuning +# +# WORKER_CLAIM_BATCH_SIZE is reserved: the worker loop currently claims one +# job at a time. This key is accepted for forward compatibility with a future +# batch-claiming path and is otherwise inert. +# WORKER_RECLAIM_BATCH_SIZE caps how many expired leases are reclaimed per +# reclaim tick. # ============================================================================= WORKER_ID=worker-1 WORKER_CLAIM_BATCH_SIZE=5 +WORKER_RECLAIM_BATCH_SIZE=25 WORKER_LEASE_SECONDS=60 WORKER_POLL_MS=2000 WORKER_HEARTBEAT_MS=15000 diff --git a/CLAUDE.md b/CLAUDE.md index 089ef84..f05c0d3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -27,5 +27,5 @@ Auth is implemented — see `docs/auth-plan.md` for the current auth status and - Mutations require `Idempotency-Key` header - Processing phases are monotonic (rank only moves forward) - Inbound and outbound webhooks use timestamped HMAC headers -- The worker loop claims one job at a time despite `WORKER_CLAIM_BATCH_SIZE` existing in config +- The worker loop claims one job at a time; `WORKER_CLAIM_BATCH_SIZE` is reserved/dormant and accepted only for forward compatibility. The reclaim tick uses `WORKER_RECLAIM_BATCH_SIZE`. - Soft delete with delayed `cleanup_artifacts` job (5 min) diff --git a/README.md b/README.md index c7601d5..d63d377 100644 --- a/README.md +++ b/README.md @@ -97,9 +97,6 @@ That starts: 5. worker queues `transcribe_video`, then `generate_ai` when eligible 6. frontend polls status and shows playback, transcript, edits, and enrichments -## Tooling - - ## Where to look next - [docs/system.md](docs/system.md) — runtime topology, architecture decisions, and capacity guidance @@ -107,4 +104,5 @@ That starts: - [docs/contracts.md](docs/contracts.md) — API/webhook contracts, versioning stance, and contract changelog - [docs/status.md](docs/status.md) — current gaps and next improvement areas - [docs/auth-plan.md](docs/auth-plan.md) — current auth status and constraints -- [docs/review-auth-system.md](docs/review-auth-system.md) — auth review notes and follow-up suggestions +- [docs/review-auth-system.md](docs/review-auth-system.md) — dated auth-system code-review snapshot +- [docs/review-2026-04-10.md](docs/review-2026-04-10.md) — dated full-repo review + changelog (most recent) diff --git a/apps/web-api/src/lib/s3.ts b/apps/web-api/src/lib/s3.ts index 2cbb6f8..c838582 100644 --- a/apps/web-api/src/lib/s3.ts +++ b/apps/web-api/src/lib/s3.ts @@ -25,7 +25,8 @@ export { // --------------------------------------------------------------------------- export function getS3ClientAndBucket() { - const publicEndpoint = process.env.S3_PUBLIC_ENDPOINT ?? "http://localhost:9000"; + // Default aligned with packages/config (host-mapped MinIO API port in docker-compose). + const publicEndpoint = process.env.S3_PUBLIC_ENDPOINT ?? "http://localhost:8922"; const signingEndpoint = publicEndpoint; const region = process.env.S3_REGION ?? "us-east-1"; const accessKeyId = process.env.S3_ACCESS_KEY; diff --git a/apps/web-api/src/routes/auth.test.ts b/apps/web-api/src/routes/auth.test.ts index abeedf3..32266e5 100644 --- a/apps/web-api/src/routes/auth.test.ts +++ b/apps/web-api/src/routes/auth.test.ts @@ -1,7 +1,23 @@ import Fastify from "fastify"; import cookie from "@fastify/cookie"; +import type { Logger } from "@cap/logger"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +// Minimal shape the auth routes reach for; cast through `unknown` to avoid +// pulling in every Logger method in this test mock. +const mockServiceLogger = (): Logger => + ({ + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + trace: vi.fn(), + withContext: vi.fn(), + logger: {}, + context: {}, + logRequest: vi.fn() + }) as unknown as Logger; + const queryMock = vi.fn(); const verifyPasswordMock = vi.fn(); const signTokenMock = vi.fn(() => "signed-token"); @@ -41,17 +57,7 @@ describe("authRoutes login hardening", () => { verifyPasswordMock.mockResolvedValue(false); const app = Fastify(); - app.decorate("serviceLogger", { - info: vi.fn(), - warn: vi.fn(), - error: vi.fn(), - debug: vi.fn(), - trace: vi.fn(), - withContext: vi.fn(), - logger: {}, - context: {}, - logRequest: vi.fn() - } as any); + app.decorate("serviceLogger", mockServiceLogger()); await app.register(cookie); const { authRoutes } = await import("./auth.js"); await app.register(authRoutes); @@ -88,17 +94,7 @@ describe("authRoutes login hardening", () => { .mockResolvedValueOnce(true); const app = Fastify(); - app.decorate("serviceLogger", { - info: vi.fn(), - warn: vi.fn(), - error: vi.fn(), - debug: vi.fn(), - trace: vi.fn(), - withContext: vi.fn(), - logger: {}, - context: {}, - logRequest: vi.fn() - } as any); + app.decorate("serviceLogger", mockServiceLogger()); await app.register(cookie); const { authRoutes } = await import("./auth.js"); await app.register(authRoutes); diff --git a/apps/web/e2e/layout.spec.ts b/apps/web/e2e/layout.spec.ts index 530593c..fad264c 100644 --- a/apps/web/e2e/layout.spec.ts +++ b/apps/web/e2e/layout.spec.ts @@ -16,9 +16,10 @@ test.describe("Responsive layout", () => { await expect( page.getByRole("heading", { name: "Building APIs: Architecture and Best Practices", exact: true }) ).toBeVisible(); - await expect(page.getByRole("button", { name: "Notes", exact: true })).toBeVisible(); - await expect(page.getByRole("button", { name: "Summary", exact: true })).toBeVisible(); - await expect(page.getByRole("button", { name: "Transcript", exact: true })).toBeVisible(); + // VideoRail tabs are now proper ARIA tabs (role="tab"), not plain buttons. + await expect(page.getByRole("tab", { name: "Notes", exact: true })).toBeVisible(); + await expect(page.getByRole("tab", { name: "Summary", exact: true })).toBeVisible(); + await expect(page.getByRole("tab", { name: "Transcript", exact: true })).toBeVisible(); await expect(page.getByText("Generated by Cap5 AI")).toBeVisible(); await expect(page.getByRole("heading", { name: "Chapters", exact: true })).toBeVisible(); }); @@ -32,7 +33,7 @@ test.describe("Responsive layout", () => { await expect( page.getByRole("heading", { name: "Building APIs: Architecture and Best Practices", exact: true }) ).toBeVisible(); - await expect(page.getByRole("button", { name: "Transcript", exact: true })).toBeVisible(); + await expect(page.getByRole("tab", { name: "Transcript", exact: true })).toBeVisible(); await page.getByRole("button").nth(1).click(); await expect(page.getByRole("link", { name: "Home" })).toBeVisible(); diff --git a/apps/web/e2e/mock-api.ts b/apps/web/e2e/mock-api.ts index d084daf..8c380db 100644 --- a/apps/web/e2e/mock-api.ts +++ b/apps/web/e2e/mock-api.ts @@ -20,16 +20,20 @@ export const MOCK_VIDEO_STATUS = { language: "en", vttKey: "cap5/00000000-0000-0000-0000-000000000001/transcript.vtt", text: "Welcome to this demonstration. Today we cover API architecture. Let us begin with the basics.", + speakerLabels: { + "0": "Host", + "1": "Guest", + }, segments: [ - { startSeconds: 0, endSeconds: 5, text: "Welcome to this demonstration." }, - { startSeconds: 5, endSeconds: 12, text: "Today we cover API architecture." }, - { startSeconds: 12, endSeconds: 20, text: "Let us begin with the basics." }, - { startSeconds: 20, endSeconds: 28, text: "First, we define our endpoints clearly." }, - { startSeconds: 28, endSeconds: 36, text: "Authentication is the next consideration." }, - { startSeconds: 36, endSeconds: 44, text: "We use JSON web tokens for auth." }, - { startSeconds: 44, endSeconds: 52, text: "Rate limiting protects against abuse." }, - { startSeconds: 52, endSeconds: 60, text: "Finally, logging ties everything together." }, - { startSeconds: 60, endSeconds: 68, text: "In summary, good APIs require planning." } + { startSeconds: 0, endSeconds: 5, text: "Welcome to this demonstration.", speaker: 0 }, + { startSeconds: 5, endSeconds: 12, text: "Today we cover API architecture.", speaker: 1 }, + { startSeconds: 12, endSeconds: 20, text: "Let us begin with the basics.", speaker: null }, + { startSeconds: 20, endSeconds: 28, text: "First, we define our endpoints clearly.", speaker: 0 }, + { startSeconds: 28, endSeconds: 36, text: "Authentication is the next consideration.", speaker: 1 }, + { startSeconds: 36, endSeconds: 44, text: "We use JSON web tokens for auth.", speaker: 0 }, + { startSeconds: 44, endSeconds: 52, text: "Rate limiting protects against abuse.", speaker: 1 }, + { startSeconds: 52, endSeconds: 60, text: "Finally, logging ties everything together.", speaker: 0 }, + { startSeconds: 60, endSeconds: 68, text: "In summary, good APIs require planning.", speaker: 1 } ] }, aiOutput: { diff --git a/apps/web/e2e/player.spec.ts b/apps/web/e2e/player.spec.ts index 05b3344..81367f3 100644 --- a/apps/web/e2e/player.spec.ts +++ b/apps/web/e2e/player.spec.ts @@ -13,7 +13,7 @@ test.describe("Video watch page", () => { await page.goto(VIDEO_URL); await page.waitForLoadState("networkidle"); - await expect(page.getByRole("button", { name: "Transcript", exact: true })).toHaveClass(/rail-tab-active/); + await expect(page.getByRole("tab", { name: "Transcript", exact: true })).toHaveAttribute("aria-selected", "true"); await expect(page.getByPlaceholder("Search transcript…")).toBeVisible(); await expect(page.getByRole("button", { name: "Current", exact: true })).toBeVisible(); await expect(page.getByRole("button", { name: "Original", exact: true })).toBeVisible(); @@ -24,9 +24,9 @@ test.describe("Video watch page", () => { await page.goto(VIDEO_URL); await page.waitForLoadState("networkidle"); - await page.getByRole("button", { name: "Summary", exact: true }).click(); + await page.getByRole("tab", { name: "Summary", exact: true }).click(); - const summaryPanel = page.locator(".rail-tab-panel-enter").filter({ + const summaryPanel = page.getByRole("tabpanel", { name: "Summary", exact: true }).filter({ has: page.getByText("Generated by Cap5 AI"), }); @@ -52,4 +52,67 @@ test.describe("Video watch page", () => { await expect(chapterSection.getByRole("button", { name: /00:20 define endpoints clearly/i })).toBeVisible(); await expect(chapterSection.getByRole("button", { name: /rate limiting/i })).toBeVisible(); }); + + test("selected-speaker playback skips deselected speakers and persists per video", async ({ page }) => { + await page.goto(VIDEO_URL); + await page.waitForLoadState("networkidle"); + + await page.locator("video").evaluate((video) => { + let currentTime = 0; + let paused = false; + + Object.defineProperty(video, "duration", { + configurable: true, + get: () => 68, + }); + Object.defineProperty(video, "currentTime", { + configurable: true, + get: () => currentTime, + set: (value: number) => { + currentTime = value; + }, + }); + Object.defineProperty(video, "paused", { + configurable: true, + get: () => paused, + }); + Object.defineProperty(video, "pause", { + configurable: true, + value: () => { + paused = true; + }, + }); + Object.defineProperty(video, "play", { + configurable: true, + value: async () => { + paused = false; + }, + }); + + video.dispatchEvent(new Event("loadedmetadata")); + }); + + const speakerChips = page.locator("button.speaker-filter-chip"); + await speakerChips.filter({ hasText: "Guest" }).click(); + + await expect(page.getByText("Today we cover API architecture.")).toHaveCount(0); + await expect(page.getByText("Let us begin with the basics.")).toBeVisible(); + await expect(page.getByText("1 of 2 selected")).toBeVisible(); + + await speakerChips.filter({ hasText: "Host" }).click(); + + await expect(page.getByText("No speakers selected.", { exact: true })).toBeVisible(); + await expect(page.getByText("No speakers selected. Re-enable at least one speaker to resume filtered playback.")).toBeVisible(); + + const isPaused = await page.locator("video").evaluate((video) => video.paused); + expect(isPaused).toBe(true); + + await page.reload(); + await page.waitForLoadState("networkidle"); + + await expect(page.getByText("No speakers selected.", { exact: true })).toBeVisible(); + await expect(page.getByText("Welcome to this demonstration.")).toHaveCount(0); + await expect(page.getByText("Today we cover API architecture.")).toHaveCount(0); + await expect(page.getByText("Let us begin with the basics.")).toBeVisible(); + }); }); diff --git a/apps/web/src/__tests__/setup.ts b/apps/web/src/__tests__/setup.ts index 4aa1ee5..c635a55 100644 --- a/apps/web/src/__tests__/setup.ts +++ b/apps/web/src/__tests__/setup.ts @@ -1,2 +1,26 @@ // Global test setup // Extend expect with jest-dom matchers if needed in future +if (typeof window !== "undefined" && typeof window.localStorage?.clear !== "function") { + const storage = new Map(); + const mockLocalStorage = { + getItem: (key: string) => storage.get(key) ?? null, + setItem: (key: string, value: string) => { + storage.set(key, String(value)); + }, + removeItem: (key: string) => { + storage.delete(key); + }, + clear: () => { + storage.clear(); + }, + key: (index: number) => Array.from(storage.keys())[index] ?? null, + get length() { + return storage.size; + }, + }; + + Object.defineProperty(window, "localStorage", { + configurable: true, + value: mockLocalStorage, + }); +} diff --git a/apps/web/src/components/PlayerCard.test.tsx b/apps/web/src/components/PlayerCard.test.tsx new file mode 100644 index 0000000..0bbb064 --- /dev/null +++ b/apps/web/src/components/PlayerCard.test.tsx @@ -0,0 +1,64 @@ +import { describe, expect, it, vi } from "vitest"; +import { render, screen } from "@testing-library/react"; +import { PlayerCard } from "./PlayerCard"; + +describe("PlayerCard speaker filtering", () => { + it("shows an empty-state message and pauses playback when all speakers are deselected", () => { + const { container, rerender } = render( + , + ); + + const video = container.querySelector("video") as HTMLVideoElement; + let paused = false; + Object.defineProperty(video, "paused", { + configurable: true, + get: () => paused, + }); + const pauseSpy = vi.fn(() => { + paused = true; + }); + Object.defineProperty(video, "pause", { + configurable: true, + value: pauseSpy, + }); + + rerender( + ()} + speakerFilteringActive={true} + allSpeakersDeselected={true} + />, + ); + + expect(pauseSpy).toHaveBeenCalledTimes(1); + expect( + screen.getByText("No speakers selected. Re-enable at least one speaker to resume filtered playback."), + ).toBeTruthy(); + }); +}); diff --git a/apps/web/src/components/PlayerCard.tsx b/apps/web/src/components/PlayerCard.tsx index 367750f..7918dc1 100644 --- a/apps/web/src/components/PlayerCard.tsx +++ b/apps/web/src/components/PlayerCard.tsx @@ -1,19 +1,17 @@ import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { formatTimestamp } from "../lib/format"; import { CustomVideoControls } from "./CustomVideoControls"; +import { + buildPlayableSpeakerRanges, + getPlaybackCorrection, + DEFAULT_SEGMENT_START_PADDING_SECONDS, + DEFAULT_SEGMENT_END_PADDING_SECONDS, + type SpeakerPlaybackSegment, +} from "./player-card/playbackFilter"; type SeekRequest = { seconds: number; requestId: number }; type ChapterItem = { title: string; seconds: number }; -type TranscriptSegment = { - startSeconds?: number; - endSeconds?: number; - speaker?: number | null; -}; - -const SEGMENT_START_PADDING_SECONDS = 0.08; -const SEGMENT_END_PADDING_SECONDS = 0.12; -const SEGMENT_MERGE_GAP_SECONDS = 0.08; -const PLAYBACK_GUARD_INTERVAL_MS = 40; +const PLAYBACK_GUARD_INTERVAL_MS = 120; const SPEAKER_PALETTE = [ "#0ea5e9", @@ -35,7 +33,9 @@ export function PlayerCard({ chapters, onSeekToSeconds, transcriptSegments, - hiddenSpeakers, + selectedSpeakerIds, + speakerFilteringActive, + allSpeakersDeselected, }: { videoUrl: string | null; thumbnailUrl: string | null; @@ -44,8 +44,10 @@ export function PlayerCard({ onDurationChange?: (seconds: number) => void; chapters: ChapterItem[]; onSeekToSeconds: (seconds: number) => void; - transcriptSegments: TranscriptSegment[]; - hiddenSpeakers: Set; + transcriptSegments: SpeakerPlaybackSegment[]; + selectedSpeakerIds: Set; + speakerFilteringActive: boolean; + allSpeakersDeselected: boolean; }) { const [playbackTimeSeconds, setPlaybackTimeSeconds] = useState(0); const [durationSeconds, setDurationSeconds] = useState(0); @@ -63,6 +65,11 @@ export function PlayerCard({ const trackRef = useRef(null); const lastAutoSkipRef = useRef<{ from: number; to: number; atMs: number } | null>(null); + const syncPlaybackTime = useCallback((seconds: number) => { + setPlaybackTimeSeconds(seconds); + onPlaybackTimeChange?.(seconds); + }, [onPlaybackTimeChange]); + /* ── Derived values ───────────────────────────────────────────────────── */ const hasResult = Boolean(videoUrl); const timelineChapters = useMemo( @@ -81,11 +88,13 @@ export function PlayerCard({ const fallbackEnd = startSeconds + 0.25; const rawEnd = Number(segment.endSeconds); const endSeconds = Number.isFinite(rawEnd) ? rawEnd : fallbackEnd; - const speaker = Number(segment.speaker); + const speaker = typeof segment.speaker === "number" && Number.isInteger(segment.speaker) && segment.speaker >= 0 + ? segment.speaker + : null; if (!Number.isFinite(startSeconds) || !Number.isFinite(endSeconds)) return null; - if (!Number.isInteger(speaker) || speaker < 0) return null; - const safeStart = Math.max(0, Math.min(durationSeconds, startSeconds - SEGMENT_START_PADDING_SECONDS)); - const safeEnd = Math.max(safeStart, Math.min(durationSeconds, endSeconds + SEGMENT_END_PADDING_SECONDS)); + if (speaker === null) return null; + const safeStart = Math.max(0, Math.min(durationSeconds, startSeconds - DEFAULT_SEGMENT_START_PADDING_SECONDS)); + const safeEnd = Math.max(safeStart, Math.min(durationSeconds, endSeconds + DEFAULT_SEGMENT_END_PADDING_SECONDS)); if (safeEnd <= safeStart) return null; return { startSeconds: safeStart, endSeconds: safeEnd, speaker }; }) @@ -101,123 +110,79 @@ export function PlayerCard({ })); }, [durationSeconds, transcriptSegments]); - const allowedSpeakerRanges = useMemo(() => { - if (durationSeconds <= 0) return [] as Array<{ startSeconds: number; endSeconds: number }>; - - const rawSegments = (Array.isArray(transcriptSegments) ? transcriptSegments : []) - .map((segment) => { - const speaker = Number(segment.speaker); - if (!Number.isInteger(speaker) || speaker < 0) return null; - if (hiddenSpeakers.has(speaker)) return null; - - const startSeconds = Number(segment.startSeconds); - const rawEnd = Number(segment.endSeconds); - if (!Number.isFinite(startSeconds)) return null; - - const fallbackEnd = startSeconds + 0.25; - const endSeconds = Number.isFinite(rawEnd) ? rawEnd : fallbackEnd; - const safeStart = Math.max(0, Math.min(durationSeconds, startSeconds - SEGMENT_START_PADDING_SECONDS)); - const safeEnd = Math.max(safeStart, Math.min(durationSeconds, endSeconds + SEGMENT_END_PADDING_SECONDS)); - if (safeEnd <= safeStart) return null; - - return { startSeconds: safeStart, endSeconds: safeEnd }; - }) - .filter((segment): segment is { startSeconds: number; endSeconds: number } => Boolean(segment)) - .sort((a, b) => a.startSeconds - b.startSeconds); - - if (rawSegments.length <= 1) return rawSegments; - - const merged: Array<{ startSeconds: number; endSeconds: number }> = []; - for (const range of rawSegments) { - const previous = merged[merged.length - 1]; - if (!previous || range.startSeconds > previous.endSeconds + SEGMENT_MERGE_GAP_SECONDS) { - merged.push({ ...range }); - } else { - previous.endSeconds = Math.max(previous.endSeconds, range.endSeconds); - } - } - - return merged; - }, [durationSeconds, transcriptSegments, hiddenSpeakers]); + const playableSpeakerRanges = useMemo(() => buildPlayableSpeakerRanges({ + durationSeconds, + transcriptSegments, + selectedSpeakerIds, + speakerFilteringActive, + }), [durationSeconds, transcriptSegments, selectedSpeakerIds, speakerFilteringActive]); - const getNextAllowedPlaybackPosition = useCallback((candidateSeconds: number): number | null => { - if (allowedSpeakerRanges.length === 0) return null; - const current = Math.max(0, candidateSeconds); + const enforcePlaybackFilter = useCallback((source: "filter-change" | "timeupdate" | "guard" | "seek" | "play" | "seeked") => { + const player = videoRef.current; + if (!player) return null; - for (const range of allowedSpeakerRanges) { - if (current < range.startSeconds) { - return range.startSeconds; - } - if (current >= range.startSeconds && current <= range.endSeconds) { - return null; - } + if (allSpeakersDeselected) { + if (!player.paused) player.pause(); + return null; } - return null; - }, [allowedSpeakerRanges]); - - const getCurrentAllowedRangeEnd = useCallback((candidateSeconds: number): number | null => { - if (allowedSpeakerRanges.length === 0) return null; - const current = Math.max(0, candidateSeconds); - - for (const range of allowedSpeakerRanges) { - if (current < range.startSeconds) return null; - if (current >= range.startSeconds && current <= range.endSeconds) { - return range.endSeconds; - } + if (!speakerFilteringActive) return null; + + const nextAllowed = getPlaybackCorrection(player.currentTime, playableSpeakerRanges); + if (nextAllowed === null) return null; + + const lastSkip = lastAutoSkipRef.current; + const now = Date.now(); + if ( + source !== "seek" + && source !== "filter-change" + && lastSkip + && Math.abs(lastSkip.from - player.currentTime) <= 0.03 + && Math.abs(lastSkip.to - nextAllowed) <= 0.03 + && now - lastSkip.atMs <= 200 + ) { + return null; } - return null; - }, [allowedSpeakerRanges]); + lastAutoSkipRef.current = { from: player.currentTime, to: nextAllowed, atMs: now }; + player.currentTime = nextAllowed; + syncPlaybackTime(nextAllowed); + return nextAllowed; + }, [allSpeakersDeselected, playableSpeakerRanges, speakerFilteringActive, syncPlaybackTime]); useEffect(() => { const player = videoRef.current; if (!player) return; - const nextAllowed = getNextAllowedPlaybackPosition(player.currentTime); - if (nextAllowed === null) return; - - player.currentTime = nextAllowed; - setPlaybackTimeSeconds(nextAllowed); - onPlaybackTimeChange?.(nextAllowed); - }, [hiddenSpeakers, getCurrentAllowedRangeEnd, getNextAllowedPlaybackPosition, onPlaybackTimeChange]); + if (allSpeakersDeselected) { + if (!player.paused) player.pause(); + return; + } + enforcePlaybackFilter("filter-change"); + }, [allSpeakersDeselected, enforcePlaybackFilter]); useEffect(() => { const player = videoRef.current; - if (!player || hiddenSpeakers.size === 0) return; + if (!player || !speakerFilteringActive || allSpeakersDeselected) return; const guardPlayback = () => { if (player.paused || player.seeking || player.ended) return; - - const currentTime = player.currentTime; - const currentAllowedEnd = getCurrentAllowedRangeEnd(currentTime); - const nextAllowed = getNextAllowedPlaybackPosition(currentTime); - const shouldAdvance = nextAllowed !== null && (currentAllowedEnd === null || currentTime > currentAllowedEnd - 0.015); - if (!shouldAdvance) return; - - const lastSkip = lastAutoSkipRef.current; - const now = Date.now(); - if (!lastSkip || Math.abs(lastSkip.from - currentTime) > 0.03 || Math.abs(lastSkip.to - nextAllowed) > 0.03 || now - lastSkip.atMs > 200) { - lastAutoSkipRef.current = { from: currentTime, to: nextAllowed, atMs: now }; - player.currentTime = nextAllowed; - setPlaybackTimeSeconds(nextAllowed); - onPlaybackTimeChange?.(nextAllowed); - } + enforcePlaybackFilter("guard"); }; + const handlePlay = () => { void enforcePlaybackFilter("play"); }; + const handleSeeked = () => { void enforcePlaybackFilter("seeked"); }; const interval = window.setInterval(guardPlayback, PLAYBACK_GUARD_INTERVAL_MS); - player.addEventListener("seeking", guardPlayback); - player.addEventListener("play", guardPlayback); - player.addEventListener("seeked", guardPlayback); + player.addEventListener("play", handlePlay); + player.addEventListener("seeked", handleSeeked); return () => { window.clearInterval(interval); - player.removeEventListener("seeking", guardPlayback); - player.removeEventListener("play", guardPlayback); - player.removeEventListener("seeked", guardPlayback); + player.removeEventListener("play", handlePlay); + player.removeEventListener("seeked", handleSeeked); }; - }, [hiddenSpeakers, getNextAllowedPlaybackPosition, onPlaybackTimeChange]); + }, [allSpeakersDeselected, enforcePlaybackFilter, speakerFilteringActive]); /* ── Seek on external request ─────────────────────────────────────────── */ useEffect(() => { @@ -225,11 +190,15 @@ export function PlayerCard({ const player = videoRef.current; if (!player) return; const clamped = Math.max(0, seekRequest.seconds); - const nextAllowed = getNextAllowedPlaybackPosition(clamped) ?? clamped; + const nextAllowed = !allSpeakersDeselected && speakerFilteringActive + ? getPlaybackCorrection(clamped, playableSpeakerRanges) ?? clamped + : clamped; player.currentTime = nextAllowed; - setPlaybackTimeSeconds(nextAllowed); - onPlaybackTimeChange?.(nextAllowed); - }, [seekRequest, onPlaybackTimeChange, getNextAllowedPlaybackPosition]); + syncPlaybackTime(nextAllowed); + if (!allSpeakersDeselected) { + enforcePlaybackFilter("seek"); + } + }, [allSpeakersDeselected, enforcePlaybackFilter, playableSpeakerRanges, seekRequest, speakerFilteringActive, syncPlaybackTime]); @@ -253,13 +222,14 @@ export function PlayerCard({ /* ── Chapter seek helpers ─────────────────────────────────────────────── */ const handleChapterSeek = useCallback((seconds: number) => { const clamped = Math.max(0, seconds); - const nextAllowed = getNextAllowedPlaybackPosition(clamped) ?? clamped; + const nextAllowed = !allSpeakersDeselected && speakerFilteringActive + ? getPlaybackCorrection(clamped, playableSpeakerRanges) ?? clamped + : clamped; const player = videoRef.current; if (player) player.currentTime = nextAllowed; - setPlaybackTimeSeconds(nextAllowed); - onPlaybackTimeChange?.(nextAllowed); + syncPlaybackTime(nextAllowed); onSeekToSeconds(nextAllowed); - }, [onPlaybackTimeChange, onSeekToSeconds, getNextAllowedPlaybackPosition]); + }, [allSpeakersDeselected, onSeekToSeconds, playableSpeakerRanges, speakerFilteringActive, syncPlaybackTime]); const goToPrevChapter = () => { if (activeChapterIndex <= 0) return; @@ -353,24 +323,14 @@ export function PlayerCard({ onTimeUpdate={(e) => { const player = e.currentTarget; const time = player.currentTime || 0; - const currentAllowedEnd = getCurrentAllowedRangeEnd(time); - const nextAllowed = getNextAllowedPlaybackPosition(time); - const shouldAdvance = nextAllowed !== null && (currentAllowedEnd === null || time > currentAllowedEnd - 0.015); - - if (shouldAdvance) { - const lastSkip = lastAutoSkipRef.current; - const now = Date.now(); - if (!lastSkip || Math.abs(lastSkip.from - time) > 0.03 || Math.abs(lastSkip.to - nextAllowed) > 0.03 || now - lastSkip.atMs > 200) { - lastAutoSkipRef.current = { from: time, to: nextAllowed, atMs: now }; - player.currentTime = nextAllowed; - setPlaybackTimeSeconds(nextAllowed); - onPlaybackTimeChange?.(nextAllowed); + if (speakerFilteringActive && !allSpeakersDeselected) { + const corrected = enforcePlaybackFilter("timeupdate"); + if (corrected !== null) { return; } } - setPlaybackTimeSeconds(time); - onPlaybackTimeChange?.(time); + syncPlaybackTime(time); }} onWaiting={() => setIsBuffering(true)} onPlaying={() => setIsBuffering(false)} @@ -385,6 +345,11 @@ export function PlayerCard({ isBuffering={isBuffering} onSeek={handleChapterSeek} /> + {allSpeakersDeselected && ( +
+ No speakers selected. Re-enable at least one speaker to resume filtered playback. +
+ )} @@ -518,6 +483,11 @@ export function PlayerCard({ ))} )} + {allSpeakersDeselected && ( +

+ Filtered playback is paused because no speakers are selected. +

+ )} {/* Time display + Prev / Next */}
diff --git a/apps/web/src/components/TranscriptCard.speaker-selection.test.tsx b/apps/web/src/components/TranscriptCard.speaker-selection.test.tsx new file mode 100644 index 0000000..9489b19 --- /dev/null +++ b/apps/web/src/components/TranscriptCard.speaker-selection.test.tsx @@ -0,0 +1,83 @@ +import { describe, expect, it, vi, beforeEach } from "vitest"; +import { cleanup, fireEvent, render, screen } from "@testing-library/react"; +import { TranscriptCard } from "./TranscriptCard"; +import type { VideoStatusResponse } from "../lib/api"; + +const baseTranscript: NonNullable = { + provider: "deepgram", + language: "en", + vttKey: "cap5/test/transcript.vtt", + text: "Host intro.\nGuest reply.\nNarration bridge.", + speakerLabels: { "0": "Host", "1": "Guest" }, + segments: [ + { startSeconds: 0, endSeconds: 3, text: "Host intro.", speaker: 0 }, + { startSeconds: 3, endSeconds: 6, text: "Guest reply.", speaker: 1 }, + { startSeconds: 6, endSeconds: 8, text: "Narration bridge.", speaker: null }, + ], +}; + +function renderTranscriptCard( + transcript: NonNullable = baseTranscript, + videoId = "video-1", +) { + return render( + true)} + onSaveSpeakerLabels={vi.fn(async () => true)} + compact + />, + ); +} + +describe("TranscriptCard speaker selection", () => { + beforeEach(() => { + cleanup(); + window.localStorage.clear(); + }); + + it("hides a deselected speaker line", () => { + const view = renderTranscriptCard(); + + fireEvent.click(view.container.querySelectorAll("button.speaker-filter-chip")[0] as HTMLButtonElement); + + expect(screen.queryByText("Host intro.")).toBeNull(); + expect(screen.getByText("Guest reply.")).toBeTruthy(); + expect(screen.getByText("Narration bridge.")).toBeTruthy(); + }); + + it("persists speaker selection per video across remounts", () => { + const firstRender = renderTranscriptCard(); + fireEvent.click(firstRender.container.querySelectorAll("button.speaker-filter-chip")[1] as HTMLButtonElement); + expect(screen.queryByText("Guest reply.")).toBeNull(); + + firstRender.unmount(); + + renderTranscriptCard(); + + expect(screen.queryByText("Guest reply.")).toBeNull(); + expect(screen.getByText("Host intro.")).toBeTruthy(); + }); + + it("prunes restored selection when the transcript speaker IDs change", () => { + window.localStorage.setItem("cap5:selected-speakers:video-1", JSON.stringify([0])); + + const nextTranscript: NonNullable = { + ...baseTranscript, + speakerLabels: { "1": "Guest" }, + segments: [ + { startSeconds: 3, endSeconds: 6, text: "Guest reply.", speaker: 1 }, + ], + }; + + renderTranscriptCard(nextTranscript); + + expect(screen.getByText("No speakers selected.")).toBeTruthy(); + expect(screen.queryByText("Guest reply.")).toBeNull(); + }); +}); diff --git a/apps/web/src/components/TranscriptCard.tsx b/apps/web/src/components/TranscriptCard.tsx index f7440a3..6144f84 100644 --- a/apps/web/src/components/TranscriptCard.tsx +++ b/apps/web/src/components/TranscriptCard.tsx @@ -14,7 +14,13 @@ type TranscriptCardProps = { onSeekToSeconds: (seconds: number) => void; onSaveTranscript: (text: string) => Promise; onSaveSpeakerLabels: (labels: Record) => Promise; - onHiddenSpeakersChange?: (hiddenSpeakers: Set) => void; + onSpeakerSelectionChange?: (selection: { + selectedSpeakerIds: Set; + hiddenSpeakers: Set; + speakerIds: number[]; + allSpeakersDeselected: boolean; + speakerFilteringActive: boolean; + }) => void; /** When true, omits the outer card wrapper — for embedding in the right rail */ compact?: boolean; }; @@ -28,7 +34,7 @@ export function TranscriptCard({ onSeekToSeconds, onSaveTranscript, onSaveSpeakerLabels, - onHiddenSpeakersChange, + onSpeakerSelectionChange, compact = false, }: TranscriptCardProps) { const { @@ -69,6 +75,9 @@ export function TranscriptCard({ activeLineIndex, getSpeakerLabel, toggleSpeakerVisibility, + allSpeakersDeselected, + speakerFilteringActive, + speakerSelectionSummary, startSpeakerEdit, cancelSpeakerEdit, saveSpeakerLabel, @@ -87,7 +96,7 @@ export function TranscriptCard({ onSaveSpeakerLabels, onSeekToSeconds, playbackTimeSeconds, - onHiddenSpeakersChange, + onSpeakerSelectionChange, }); const Inner = ( @@ -135,6 +144,9 @@ export function TranscriptCard({ onToggleReviewMode={toggleReviewMode} onNavigateReview={navigateReview} speakerIds={speakerIds} + allSpeakersDeselected={allSpeakersDeselected} + speakerFilteringActive={speakerFilteringActive} + speakerSelectionSummary={speakerSelectionSummary} hiddenSpeakers={hiddenSpeakers} getSpeakerLabel={(speaker) => getSpeakerLabel(speaker) ?? ''} onToggleSpeakerVisibility={toggleSpeakerVisibility} diff --git a/apps/web/src/components/player-card/playbackFilter.test.ts b/apps/web/src/components/player-card/playbackFilter.test.ts new file mode 100644 index 0000000..92e44f2 --- /dev/null +++ b/apps/web/src/components/player-card/playbackFilter.test.ts @@ -0,0 +1,98 @@ +import { describe, expect, it } from "vitest"; +import { + buildPlayableSpeakerRanges, + getPlaybackCorrection, +} from "./playbackFilter"; + +describe("playbackFilter", () => { + it("merges adjacent playable speaker ranges", () => { + const ranges = buildPlayableSpeakerRanges({ + durationSeconds: 30, + transcriptSegments: [ + { startSeconds: 1, endSeconds: 2, speaker: 0 }, + { startSeconds: 2.05, endSeconds: 3, speaker: 0 }, + ], + selectedSpeakerIds: new Set([0]), + speakerFilteringActive: true, + segmentStartPaddingSeconds: 0, + segmentEndPaddingSeconds: 0, + segmentMergeGapSeconds: 0.1, + }); + + expect(ranges).toEqual([{ startSeconds: 1, endSeconds: 3 }]); + }); + + it("skips deselected speaker gaps by returning the next playable start", () => { + const ranges = buildPlayableSpeakerRanges({ + durationSeconds: 30, + transcriptSegments: [ + { startSeconds: 0, endSeconds: 2, speaker: 0 }, + { startSeconds: 2, endSeconds: 4, speaker: 1 }, + { startSeconds: 4, endSeconds: 6, speaker: 0 }, + ], + selectedSpeakerIds: new Set([0]), + speakerFilteringActive: true, + segmentStartPaddingSeconds: 0, + segmentEndPaddingSeconds: 0, + segmentMergeGapSeconds: 0, + }); + + expect(getPlaybackCorrection(2.5, ranges)).toBe(4); + }); + + it("keeps unlabeled segments playable while filtering is active", () => { + const ranges = buildPlayableSpeakerRanges({ + durationSeconds: 30, + transcriptSegments: [ + { startSeconds: 0, endSeconds: 2, speaker: 0 }, + { startSeconds: 2, endSeconds: 4, speaker: null }, + { startSeconds: 4, endSeconds: 6, speaker: 1 }, + ], + selectedSpeakerIds: new Set([0]), + speakerFilteringActive: true, + segmentStartPaddingSeconds: 0, + segmentEndPaddingSeconds: 0, + segmentMergeGapSeconds: 0, + }); + + expect(ranges).toEqual([ + { startSeconds: 0, endSeconds: 4 }, + ]); + expect(getPlaybackCorrection(2.5, ranges)).toBeNull(); + }); + + it("produces no playable ranges when every detected speaker is deselected", () => { + const ranges = buildPlayableSpeakerRanges({ + durationSeconds: 30, + transcriptSegments: [ + { startSeconds: 0, endSeconds: 2, speaker: 0 }, + { startSeconds: 2, endSeconds: 4, speaker: 1 }, + ], + selectedSpeakerIds: new Set(), + speakerFilteringActive: true, + segmentStartPaddingSeconds: 0, + segmentEndPaddingSeconds: 0, + segmentMergeGapSeconds: 0, + }); + + expect(ranges).toEqual([]); + expect(getPlaybackCorrection(1, ranges)).toBeNull(); + }); + + it("corrects seeks before the first playable segment", () => { + const ranges = buildPlayableSpeakerRanges({ + durationSeconds: 30, + transcriptSegments: [ + { startSeconds: 5, endSeconds: 6, speaker: 0 }, + { startSeconds: 10, endSeconds: 12, speaker: 1 }, + ], + selectedSpeakerIds: new Set([0]), + speakerFilteringActive: true, + segmentStartPaddingSeconds: 0, + segmentEndPaddingSeconds: 0, + segmentMergeGapSeconds: 0, + }); + + expect(getPlaybackCorrection(0, ranges)).toBe(5); + }); +}); diff --git a/apps/web/src/components/player-card/playbackFilter.ts b/apps/web/src/components/player-card/playbackFilter.ts new file mode 100644 index 0000000..d20f8fd --- /dev/null +++ b/apps/web/src/components/player-card/playbackFilter.ts @@ -0,0 +1,114 @@ +export const DEFAULT_SEGMENT_START_PADDING_SECONDS = 0.08; +export const DEFAULT_SEGMENT_END_PADDING_SECONDS = 0.12; +export const DEFAULT_SEGMENT_MERGE_GAP_SECONDS = 0.08; + +export type SpeakerPlaybackSegment = { + startSeconds?: number; + endSeconds?: number; + speaker?: number | null; +}; + +export type SpeakerPlaybackRange = { + startSeconds: number; + endSeconds: number; +}; + +export type BuildPlayableSpeakerRangesOptions = { + durationSeconds: number; + transcriptSegments: SpeakerPlaybackSegment[]; + selectedSpeakerIds: Set; + speakerFilteringActive: boolean; + segmentStartPaddingSeconds?: number; + segmentEndPaddingSeconds?: number; + segmentMergeGapSeconds?: number; +}; + +export type PlaybackPositionAnalysis = { + currentRange: SpeakerPlaybackRange | null; + nextRange: SpeakerPlaybackRange | null; +}; + +export function buildPlayableSpeakerRanges({ + durationSeconds, + transcriptSegments, + selectedSpeakerIds, + speakerFilteringActive, + segmentStartPaddingSeconds = DEFAULT_SEGMENT_START_PADDING_SECONDS, + segmentEndPaddingSeconds = DEFAULT_SEGMENT_END_PADDING_SECONDS, + segmentMergeGapSeconds = DEFAULT_SEGMENT_MERGE_GAP_SECONDS, +}: BuildPlayableSpeakerRangesOptions): SpeakerPlaybackRange[] { + if (durationSeconds <= 0) return []; + + const rawRanges = (Array.isArray(transcriptSegments) ? transcriptSegments : []) + .map((segment) => { + const startSeconds = Number(segment.startSeconds); + if (!Number.isFinite(startSeconds)) return null; + + const fallbackEnd = startSeconds + 0.25; + const rawEnd = Number(segment.endSeconds); + const endSeconds = Number.isFinite(rawEnd) ? rawEnd : fallbackEnd; + const safeStart = Math.max(0, Math.min(durationSeconds, startSeconds - segmentStartPaddingSeconds)); + const safeEnd = Math.max(safeStart, Math.min(durationSeconds, endSeconds + segmentEndPaddingSeconds)); + if (safeEnd <= safeStart) return null; + + const speaker = typeof segment.speaker === "number" && Number.isInteger(segment.speaker) && segment.speaker >= 0 + ? segment.speaker + : null; + const hasKnownSpeaker = speaker !== null; + const isPlayable = !speakerFilteringActive || !hasKnownSpeaker || selectedSpeakerIds.has(speaker); + if (!isPlayable) return null; + + return { startSeconds: safeStart, endSeconds: safeEnd }; + }) + .filter((range): range is SpeakerPlaybackRange => Boolean(range)) + .sort((a, b) => a.startSeconds - b.startSeconds); + + if (rawRanges.length <= 1) return rawRanges; + + const merged: SpeakerPlaybackRange[] = []; + for (const range of rawRanges) { + const previous = merged[merged.length - 1]; + if (!previous || range.startSeconds > previous.endSeconds + segmentMergeGapSeconds) { + merged.push({ ...range }); + continue; + } + previous.endSeconds = Math.max(previous.endSeconds, range.endSeconds); + } + + return merged; +} + +export function analyzePlaybackPosition( + candidateSeconds: number, + playableRanges: SpeakerPlaybackRange[], +): PlaybackPositionAnalysis { + if (playableRanges.length === 0) { + return { currentRange: null, nextRange: null }; + } + + const current = Math.max(0, candidateSeconds); + + for (let index = 0; index < playableRanges.length; index += 1) { + const range = playableRanges[index]!; + if (current < range.startSeconds) { + return { currentRange: null, nextRange: range }; + } + if (current >= range.startSeconds && current <= range.endSeconds) { + return { + currentRange: range, + nextRange: playableRanges[index + 1] ?? null, + }; + } + } + + return { currentRange: null, nextRange: null }; +} + +export function getPlaybackCorrection( + candidateSeconds: number, + playableRanges: SpeakerPlaybackRange[], +): number | null { + const { currentRange, nextRange } = analyzePlaybackPosition(candidateSeconds, playableRanges); + if (currentRange) return null; + return nextRange?.startSeconds ?? null; +} diff --git a/apps/web/src/components/transcript-card/TranscriptControls.tsx b/apps/web/src/components/transcript-card/TranscriptControls.tsx index 9525732..1967a56 100644 --- a/apps/web/src/components/transcript-card/TranscriptControls.tsx +++ b/apps/web/src/components/transcript-card/TranscriptControls.tsx @@ -24,6 +24,9 @@ export function TranscriptControls({ onToggleReviewMode, onNavigateReview, speakerIds, + allSpeakersDeselected, + speakerFilteringActive, + speakerSelectionSummary, hiddenSpeakers, getSpeakerLabel, onToggleSpeakerVisibility, @@ -49,6 +52,9 @@ export function TranscriptControls({ onToggleReviewMode: () => void; onNavigateReview: (direction: "prev" | "next") => void; speakerIds: number[]; + allSpeakersDeselected: boolean; + speakerFilteringActive: boolean; + speakerSelectionSummary: string | null; hiddenSpeakers: Set; getSpeakerLabel: (speaker: number) => string | null; onToggleSpeakerVisibility: (speaker: number) => void; @@ -186,6 +192,11 @@ export function TranscriptControls({ {speakerIds.length > 0 && (
Speakers: + {speakerSelectionSummary && ( + + {speakerSelectionSummary} + + )} {speakerIds.map((speaker) => { const isHidden = hiddenSpeakers.has(speaker); return ( @@ -204,6 +215,16 @@ export function TranscriptControls({ ); })} + {speakerFilteringActive && !allSpeakersDeselected && ( + + Playing selected speakers only. + + )} + {allSpeakersDeselected && ( + + No speakers selected. + + )}
)} diff --git a/apps/web/src/hooks/useTranscriptState.tsx b/apps/web/src/hooks/useTranscriptState.tsx index 6f194af..3e015b0 100644 --- a/apps/web/src/hooks/useTranscriptState.tsx +++ b/apps/web/src/hooks/useTranscriptState.tsx @@ -15,9 +15,17 @@ export type TranscriptStateOptions = { onSaveSpeakerLabels: (labels: Record) => Promise; onSeekToSeconds: (seconds: number) => void; playbackTimeSeconds: number; - onHiddenSpeakersChange?: (hiddenSpeakers: Set) => void; + onSpeakerSelectionChange?: (selection: { + selectedSpeakerIds: Set; + hiddenSpeakers: Set; + speakerIds: number[]; + allSpeakersDeselected: boolean; + speakerFilteringActive: boolean; + }) => void; }; +const SPEAKER_SELECTION_STORAGE_PREFIX = 'cap5:selected-speakers:'; + export function useTranscriptState({ videoId, transcript, @@ -25,7 +33,7 @@ export function useTranscriptState({ onSaveSpeakerLabels, onSeekToSeconds, playbackTimeSeconds, - onHiddenSpeakersChange, + onSpeakerSelectionChange, }: TranscriptStateOptions) { // ── Edit & copy feedback ────────────────────────────────────────────── const [copyFeedback, setCopyFeedback] = useState(null); @@ -51,7 +59,7 @@ export function useTranscriptState({ const [speakerLabels, setSpeakerLabels] = useState>(() => normalizeSpeakerLabels(transcript?.speakerLabels ?? {}) ); - const [hiddenSpeakers, setHiddenSpeakers] = useState>(new Set()); + const [selectedSpeakerIds, setSelectedSpeakerIds] = useState>(new Set()); const [editingSpeaker, setEditingSpeaker] = useState(null); const [editingSpeakerLineIndex, setEditingSpeakerLineIndex] = useState(null); const [speakerDraft, setSpeakerDraft] = useState(''); @@ -81,8 +89,8 @@ export function useTranscriptState({ : null, confidence, speaker: - Number.isInteger(Number(segment.speaker)) && Number(segment.speaker) >= 0 - ? Number(segment.speaker) + typeof segment.speaker === 'number' && Number.isInteger(segment.speaker) && segment.speaker >= 0 + ? segment.speaker : null, }; }) @@ -115,6 +123,26 @@ export function useTranscriptState({ return Array.from(unique.values()).sort((a, b) => a - b); }, [transcriptLines]); + const hiddenSpeakers = useMemo(() => { + if (speakerIds.length === 0) return new Set(); + const hidden = new Set(); + const selected = selectedSpeakerIds; + for (const speaker of speakerIds) { + if (!selected.has(speaker)) hidden.add(speaker); + } + return hidden; + }, [selectedSpeakerIds, speakerIds]); + + const selectedSpeakerCount = selectedSpeakerIds.size; + const allSpeakersDeselected = speakerIds.length > 0 && selectedSpeakerCount === 0; + const speakerFilteringActive = speakerIds.length > 0 && selectedSpeakerCount < speakerIds.length; + const speakerSelectionSummary = useMemo(() => { + if (speakerIds.length === 0) return null; + if (allSpeakersDeselected) return 'None selected'; + if (!speakerFilteringActive) return 'All selected'; + return `${selectedSpeakerCount} of ${speakerIds.length} selected`; + }, [allSpeakersDeselected, selectedSpeakerCount, speakerFilteringActive, speakerIds.length]); + const confidenceStats = useMemo(() => { const withConfidence = transcriptLines.filter(line => line.confidence !== null); if (withConfidence.length === 0) return null; @@ -168,18 +196,63 @@ export function useTranscriptState({ }, [transcript?.speakerLabels]); useEffect(() => { - setHiddenSpeakers(prev => { - if (speakerIds.length === 0) return new Set(); + if (!videoId) { + setSelectedSpeakerIds(new Set(speakerIds)); + return; + } + + if (speakerIds.length === 0) { + setSelectedSpeakerIds(new Set()); + return; + } + + const storageKey = `${SPEAKER_SELECTION_STORAGE_PREFIX}${videoId}`; + const raw = window.localStorage.getItem(storageKey); + if (!raw) { + setSelectedSpeakerIds(new Set(speakerIds)); + return; + } + + try { + const parsed = JSON.parse(raw); + const validStoredIds = Array.isArray(parsed) + ? parsed + .map((value) => Number(value)) + .filter((value) => Number.isInteger(value)) + : []; const allowed = new Set(speakerIds); const next = new Set(); - for (const speaker of prev) { if (allowed.has(speaker)) next.add(speaker); } - return next; - }); - }, [speakerIds]); + for (const speaker of validStoredIds) { + if (allowed.has(speaker)) next.add(speaker); + } + setSelectedSpeakerIds(next); + } catch { + setSelectedSpeakerIds(new Set(speakerIds)); + } + }, [videoId, speakerIds]); useEffect(() => { - onHiddenSpeakersChange?.(new Set(hiddenSpeakers)); - }, [hiddenSpeakers, onHiddenSpeakersChange]); + if (!videoId || speakerIds.length === 0) return; + const storageKey = `${SPEAKER_SELECTION_STORAGE_PREFIX}${videoId}`; + window.localStorage.setItem(storageKey, JSON.stringify(Array.from(selectedSpeakerIds.values()).sort((a, b) => a - b))); + }, [videoId, selectedSpeakerIds, speakerIds.length]); + + useEffect(() => { + onSpeakerSelectionChange?.({ + selectedSpeakerIds: new Set(selectedSpeakerIds), + hiddenSpeakers: new Set(hiddenSpeakers), + speakerIds: [...speakerIds], + allSpeakersDeselected, + speakerFilteringActive, + }); + }, [ + allSpeakersDeselected, + hiddenSpeakers, + onSpeakerSelectionChange, + selectedSpeakerIds, + speakerFilteringActive, + speakerIds, + ]); useEffect(() => { if (!isEditing) { setDraftText(transcriptText); setSaveError(null); } @@ -261,7 +334,7 @@ export function useTranscriptState({ }, [speakerLabels]); const toggleSpeakerVisibility = useCallback((speaker: number) => { - setHiddenSpeakers(prev => { + setSelectedSpeakerIds(prev => { const next = new Set(prev); if (next.has(speaker)) next.delete(speaker); else next.add(speaker); @@ -415,7 +488,12 @@ export function useTranscriptState({ isReviewMode, reviewIndex, speakerLabels, + selectedSpeakerIds, hiddenSpeakers, + selectedSpeakerCount, + allSpeakersDeselected, + speakerFilteringActive, + speakerSelectionSummary, editingSpeaker, editingSpeakerLineIndex, speakerDraft, diff --git a/apps/web/src/lib/format.ts b/apps/web/src/lib/format.ts index 7242c35..d5300b1 100644 --- a/apps/web/src/lib/format.ts +++ b/apps/web/src/lib/format.ts @@ -1,3 +1,10 @@ +/** + * Format a duration in seconds as "mm:ss" (or "hh:mm:ss" for durations >= 1h). + * + * `formatTimestamp` is kept as an alias for historical call sites that treat + * the same formatter as a "media timecode" rather than a "duration". Both names + * share one implementation so output is guaranteed to stay consistent. + */ export function formatDuration(totalSeconds: number): string { const seconds = Math.max(0, Math.floor(totalSeconds)); const hrs = Math.floor(seconds / 3600); @@ -8,6 +15,8 @@ export function formatDuration(totalSeconds: number): string { return `${String(mins).padStart(2, "0")}:${String(secs).padStart(2, "0")}`; } +export const formatTimestamp = formatDuration; + export function formatBytes(bytes: number): string { if (!Number.isFinite(bytes) || bytes <= 0) return "0 B"; const units = ["B", "KB", "MB", "GB"]; @@ -41,13 +50,3 @@ export function buildPublicObjectUrl(key: string): string { return `${base}/${encodedKey}`; } -export function formatTimestamp(secondsInput: number): string { - const totalSeconds = Math.max(0, Math.floor(secondsInput)); - const hours = Math.floor(totalSeconds / 3600); - const minutes = Math.floor((totalSeconds % 3600) / 60); - const seconds = totalSeconds % 60; - if (hours > 0) { - return `${String(hours).padStart(2, "0")}:${String(minutes).padStart(2, "0")}:${String(seconds).padStart(2, "0")}`; - } - return `${String(minutes).padStart(2, "0")}:${String(seconds).padStart(2, "0")}`; -} diff --git a/apps/web/src/pages/VideoPage.tsx b/apps/web/src/pages/VideoPage.tsx index 8c6861c..10a328a 100644 --- a/apps/web/src/pages/VideoPage.tsx +++ b/apps/web/src/pages/VideoPage.tsx @@ -65,7 +65,7 @@ export function VideoPage() { /* ── Store ───────────────────────────────────────────────────────────── */ const { status, jobStatus, loading, errorMessage, consecutivePollFailures, lastUpdatedAt, - playbackTimeSeconds, videoDurationSeconds: _videoDurationSeconds, seekRequest, + playbackTimeSeconds, videoDurationSeconds, seekRequest, copyFeedback, isTitleEditing, titleDraft, isSavingTitle, titleSaveMessage, isRetrying, retryMessage, isDeleteDialogOpen, isDeleting, isDeleted, deleteError, isSummaryExpanded, railTab, @@ -89,7 +89,19 @@ export function VideoPage() { () => deriveVideoChapters(status?.aiOutput, transcriptSegments), [status?.aiOutput, transcriptSegments], ); - const [hiddenSpeakers, setHiddenSpeakers] = useState>(new Set()); + const [speakerSelection, setSpeakerSelection] = useState<{ + selectedSpeakerIds: Set; + hiddenSpeakers: Set; + speakerIds: number[]; + allSpeakersDeselected: boolean; + speakerFilteringActive: boolean; + }>({ + selectedSpeakerIds: new Set(), + hiddenSpeakers: new Set(), + speakerIds: [], + allSpeakersDeselected: false, + speakerFilteringActive: false, + }); const summaryText = status?.aiOutput?.summary?.trim() ?? ""; const hasSummaryStrip = summaryText.length > 0; @@ -284,7 +296,7 @@ export function VideoPage() { onSeekToSeconds={requestSeek} onSaveTranscript={saveTranscript} onSaveSpeakerLabels={saveSpeakerLabels} - onHiddenSpeakersChange={setHiddenSpeakers} + onSpeakerSelectionChange={setSpeakerSelection} compact /> ); @@ -389,7 +401,9 @@ export function VideoPage() { chapters={chapters} onSeekToSeconds={requestSeek} transcriptSegments={status?.transcript?.segments ?? []} - hiddenSpeakers={hiddenSpeakers} + selectedSpeakerIds={speakerSelection.selectedSpeakerIds} + speakerFilteringActive={speakerSelection.speakerFilteringActive} + allSpeakersDeselected={speakerSelection.allSpeakersDeselected} /> )}
@@ -416,7 +430,7 @@ export function VideoPage() { diff --git a/apps/worker/src/queue/claim.test.ts b/apps/worker/src/queue/claim.test.ts new file mode 100644 index 0000000..e8dc172 --- /dev/null +++ b/apps/worker/src/queue/claim.test.ts @@ -0,0 +1,134 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +const withTransactionMock = vi.fn(); + +vi.mock("@cap/db", () => ({ + withTransaction: withTransactionMock +})); + +beforeEach(() => { + process.env.DATABASE_URL = "postgres://cap5:cap5_test@localhost:5432/cap5_test"; + process.env.MEDIA_SERVER_WEBHOOK_SECRET = "test-webhook-secret-with-32-plus-chars"; + process.env.DEEPGRAM_API_KEY = "test-deepgram"; + process.env.GROQ_API_KEY = "test-groq"; + process.env.S3_ACCESS_KEY = "test-access-key"; + process.env.S3_SECRET_KEY = "test-secret-key"; + process.env.WORKER_ID = "worker-test"; + process.env.WORKER_LEASE_SECONDS = "60"; + process.env.WORKER_RECLAIM_BATCH_SIZE = "25"; +}); + +afterEach(() => { + vi.resetModules(); + vi.clearAllMocks(); +}); + +describe("claimOne", () => { + it("returns the first row from the claim SQL and forwards the lease params", async () => { + const queryMock = vi.fn().mockResolvedValue({ + rows: [ + { + id: 42, + video_id: "22222222-2222-2222-2222-222222222222", + job_type: "process_video", + lease_token: "tok", + payload: {}, + attempts: 1, + max_attempts: 6 + } + ] + }); + withTransactionMock.mockImplementation(async (_dbUrl, fn) => fn({ query: queryMock })); + + const { claimOne } = await import("./claim.js"); + const result = await claimOne(); + + expect(result).not.toBeNull(); + expect(result?.id).toBe(42); + expect(queryMock).toHaveBeenCalledTimes(1); + const params = queryMock.mock.calls[0]?.[1] as unknown[]; + // [limit, worker_id, lease_interval] + expect(params[0]).toBe(1); + expect(params[1]).toBe("worker-test"); + expect(params[2]).toBe("60 seconds"); + }); + + it("returns null when no job is available", async () => { + const queryMock = vi.fn().mockResolvedValue({ rows: [] }); + withTransactionMock.mockImplementation(async (_dbUrl, fn) => fn({ query: queryMock })); + + const { claimOne } = await import("./claim.js"); + const result = await claimOne(); + + expect(result).toBeNull(); + }); + + it("appends exclude types as trailing params when excludeTypes is non-empty", async () => { + const queryMock = vi.fn().mockResolvedValue({ rows: [] }); + withTransactionMock.mockImplementation(async (_dbUrl, fn) => fn({ query: queryMock })); + + const { claimOne } = await import("./claim.js"); + await claimOne(["deliver_webhook", "cleanup_artifacts"]); + + expect(queryMock).toHaveBeenCalledTimes(1); + const sql = queryMock.mock.calls[0]?.[0] as string; + const params = queryMock.mock.calls[0]?.[1] as unknown[]; + // The excluded-types SQL variant references job_type NOT IN (...). + expect(sql).toContain("job_type NOT IN"); + expect(params).toEqual([1, "worker-test", "60 seconds", "deliver_webhook", "cleanup_artifacts"]); + }); +}); + +describe("reclaimExpiredLeases", () => { + it("uses WORKER_RECLAIM_BATCH_SIZE as the LIMIT parameter", async () => { + const queryMock = vi.fn().mockResolvedValue({ rows: [] }); + withTransactionMock.mockImplementation(async (_dbUrl, fn) => fn({ query: queryMock })); + + const { reclaimExpiredLeases } = await import("./claim.js"); + await reclaimExpiredLeases(); + + expect(queryMock).toHaveBeenCalledTimes(1); + const params = queryMock.mock.calls[0]?.[1] as unknown[]; + expect(params).toEqual([25]); + }); + + it("returns the rows the reclaim SQL surfaced (retryable + dead)", async () => { + const stale = [ + { + id: 101, + video_id: "33333333-3333-3333-3333-333333333333", + job_type: "process_video", + status: "queued" + }, + { + id: 102, + video_id: "44444444-4444-4444-4444-444444444444", + job_type: "transcribe_audio", + status: "dead" + } + ]; + const queryMock = vi.fn().mockResolvedValue({ rows: stale }); + withTransactionMock.mockImplementation(async (_dbUrl, fn) => fn({ query: queryMock })); + + const { reclaimExpiredLeases } = await import("./claim.js"); + const result = await reclaimExpiredLeases(); + + expect(result).toEqual(stale); + // Verify we're actually executing the reclaim statement, not the claim one. + const sql = queryMock.mock.calls[0]?.[0] as string; + expect(sql).toContain("WITH stale AS"); + expect(sql).toContain("locked_until < now()"); + }); + + it("honors a custom WORKER_RECLAIM_BATCH_SIZE from env", async () => { + process.env.WORKER_RECLAIM_BATCH_SIZE = "7"; + const queryMock = vi.fn().mockResolvedValue({ rows: [] }); + withTransactionMock.mockImplementation(async (_dbUrl, fn) => fn({ query: queryMock })); + + const { reclaimExpiredLeases } = await import("./claim.js"); + await reclaimExpiredLeases(); + + const params = queryMock.mock.calls[0]?.[1] as unknown[]; + expect(params).toEqual([7]); + }); +}); diff --git a/apps/worker/src/queue/claim.ts b/apps/worker/src/queue/claim.ts index f4a240f..cb723cb 100644 --- a/apps/worker/src/queue/claim.ts +++ b/apps/worker/src/queue/claim.ts @@ -16,7 +16,7 @@ export async function claimOne(excludeTypes: JobType[] = []): Promise> { return withTransaction(env.DATABASE_URL, async (client) => { - const result = await client.query(RECLAIM_SQL, [env.WORKER_CLAIM_BATCH_SIZE]); + const result = await client.query(RECLAIM_SQL, [env.WORKER_RECLAIM_BATCH_SIZE]); return result.rows; }); } diff --git a/docker-compose.yml b/docker-compose.yml index 533893b..468f87d 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -18,27 +18,26 @@ services: timeout: 3s retries: 20 - # Runs all pending SQL migrations once on every stack startup. - # web-api and worker wait for this to complete before starting. + # Runs pending SQL migrations once on every stack startup using the + # transactional Node runner in packages/db/scripts/migrate.mjs. The runner + # tracks applied versions in the `schema_migrations` table, wraps each + # migration in a transaction, and is safe to re-run idempotently. migrate: - image: postgres:16-alpine + build: + context: . + dockerfile: Dockerfile + image: cap5-migrate:prod # container_name: cap5-migrate restart: "no" env_file: - .env environment: + NODE_ENV: production DATABASE_URL: ${DATABASE_URL} + MIGRATIONS_DIR: /workspace/db/migrations volumes: - - ./db/migrations:/migrations:ro - entrypoint: - - sh - - -c - - | - for f in /migrations/*.sql; do - echo "Applying $$f ..." - psql "$$DATABASE_URL" -v ON_ERROR_STOP=1 -f "$$f" || exit 1 - done - echo "All migrations applied." + - ./db/migrations:/workspace/db/migrations:ro + command: pnpm --filter @cap/db migrate depends_on: postgres: condition: service_healthy diff --git a/docs/auth-plan.md b/docs/auth-plan.md index 59602c6..98a09e5 100644 --- a/docs/auth-plan.md +++ b/docs/auth-plan.md @@ -55,6 +55,10 @@ Database/config: - `packages/config/src/index.ts` - `.env.example` +## Related review + +`docs/review-auth-system.md` is a dated code-review snapshot of the same system. This file is the living status doc; the review file is a point-in-time sign-off. + ## Remaining auth follow-up ideas These are not blockers for the current single-user app, but they are reasonable future improvements: diff --git a/docs/cap5_implementation_plan.md b/docs/cap5_implementation_plan.md index f2f6327..35c1fe2 100644 --- a/docs/cap5_implementation_plan.md +++ b/docs/cap5_implementation_plan.md @@ -83,11 +83,12 @@ In progress. - tests for queue failure transitions via `fail()` - tests for `markRunning()` lease-loss behavior - tests for cleanup-artifact key collection and no-object path +- tests for `claimOne()` + `reclaimExpiredLeases()` (parameters, SQL path, batch-size env) +- `WORKER_CLAIM_BATCH_SIZE` is now explicitly reserved/dormant; reclaim was split out to a dedicated `WORKER_RECLAIM_BATCH_SIZE` ### Remaining work -1. add direct tests for reclaim / expired leases / terminal failure transitions -2. expand delete + retry lifecycle coverage beyond focused unit tests -3. decide whether `WORKER_CLAIM_BATCH_SIZE` should become real worker concurrency or be removed from config +1. expand delete + retry lifecycle coverage beyond focused unit tests +2. revisit the claim loop itself (one-at-a-time vs batch) if/when worker throughput scaling is needed — at that point `WORKER_CLAIM_BATCH_SIZE` graduates from dormant to real ## Phase 3 — Frontend resilience and operator clarity diff --git a/docs/development.md b/docs/development.md index 1e36c04..7ec8d84 100644 --- a/docs/development.md +++ b/docs/development.md @@ -72,8 +72,11 @@ Local env overrides (set in `.env` or export): ```bash DATABASE_URL=postgres://app:app@localhost:5432/cap5 -S3_ENDPOINT=http://localhost:9000 -S3_PUBLIC_ENDPOINT=http://localhost:9000 +# When running host-side against docker-compose MinIO, both the internal and +# public endpoints are the host-mapped port (8922). Inside the compose network, +# S3_ENDPOINT is the docker hostname (http://minio:9000) instead. +S3_ENDPOINT=http://localhost:8922 +S3_PUBLIC_ENDPOINT=http://localhost:8922 MEDIA_SERVER_BASE_URL=http://localhost:3100 ``` @@ -124,7 +127,8 @@ Canonical sources: | `WORKER_POLL_MS` | 2000 | Poll interval between claim attempts | | `WORKER_HEARTBEAT_MS` | 15000 | Heartbeat interval to extend lease | | `WORKER_RECLAIM_MS` | 10000 | Interval to reclaim expired leases | -| `WORKER_CLAIM_BATCH_SIZE` | 5 | Exists in config but loop claims one at a time | +| `WORKER_CLAIM_BATCH_SIZE` | 5 | Reserved/dormant — loop claims one job per tick; kept for forward compatibility | +| `WORKER_RECLAIM_BATCH_SIZE` | 25 | LIMIT on the reclaim-expired-leases tick | ### Provider config diff --git a/docs/review-2026-04-10.md b/docs/review-2026-04-10.md new file mode 100644 index 0000000..0eb33db --- /dev/null +++ b/docs/review-2026-04-10.md @@ -0,0 +1,139 @@ +# cap5 — full-repo review and changelog + +**Date:** 2026-04-10 +**Scope:** Full-repo code + docs audit, targeted fixes, new test coverage, and doc reconciliation +**Verification:** `pnpm typecheck` (7/7 packages) + `pnpm test` (56/56 unit tests) all green after changes + +This file is a point-in-time record of the review that produced the edits below. It is intentionally dated and should not be treated as living documentation. For ongoing status, see `docs/status.md`. + +## Verdict + +The repo is in materially good shape. No critical bugs, no security-incident-class issues, no silently-broken code paths. The issues found are of two kinds: (1) small correctness and semantic-drift cleanups that were worth fixing immediately, and (2) cosmetic / on-disk cruft that is safe to leave but worth the user's attention on a future pass. + +## Changes applied in this review + +### 1. Docker migrate service switched to the transactional Node runner + +`docker-compose.yml` — the `migrate` service previously ran an ad-hoc shell `psql` for-loop over `db/migrations/*.sql`. That path did not track applied versions in `schema_migrations`, did not wrap each migration in a transaction, and was not the same code path used by `pnpm db:migrate` locally. + +Now the service builds the repo Dockerfile and runs `pnpm --filter @cap/db migrate`, which invokes `packages/db/scripts/migrate.mjs` — the same transactional Node runner already used for local development. Migrations are idempotent, transactional, and tracked. + +**Why this matters:** production-ish and local now share one migration code path, so "migrated successfully on my machine" means the same thing as "migrated successfully in the compose stack." + +### 2. WORKER_CLAIM_BATCH_SIZE semantic fix + +Before: the config key `WORKER_CLAIM_BATCH_SIZE` was read in two places. The worker's claim path never used it (the loop always claims exactly one job per tick), but the *reclaim* path was using it as the `LIMIT` on the expired-lease reclaim SQL — a different operation with different tuning needs. + +After: +- `packages/config/src/index.ts` adds a `WORKER_RECLAIM_BATCH_SIZE` key (default 25) and documents `WORKER_CLAIM_BATCH_SIZE` as reserved/dormant — accepted for forward compatibility with a future batch-claiming worker loop, but currently inert. +- `apps/worker/src/queue/claim.ts::reclaimExpiredLeases` now reads `WORKER_RECLAIM_BATCH_SIZE` instead of `WORKER_CLAIM_BATCH_SIZE`. +- `.env.example` documents both keys with an explanatory block. +- `CLAUDE.md` and `docs/status.md` are corrected — the previous "loop claims one at a time despite `WORKER_CLAIM_BATCH_SIZE` existing in config" wording is replaced with an accurate description, and the status-doc TODO "decide whether `WORKER_CLAIM_BATCH_SIZE` should be used or removed" is marked resolved. + +**Why this matters:** claim tuning and reclaim tuning are independent concerns. Conflating them under one key would have made a future attempt to raise reclaim throughput accidentally change claim semantics. + +### 3. `formatDuration` / `formatTimestamp` deduped + +`apps/web/src/lib/format.ts` had two byte-identical functions — `formatDuration` (2 call sites) and `formatTimestamp` (25+ call sites) — both doing the same hh:mm:ss/mm:ss formatting. Easy to let one drift from the other. + +Now `formatDuration` is the single implementation and `formatTimestamp` is exported as an alias to it (`export const formatTimestamp = formatDuration;`). All existing call sites continue to compile and run unchanged; output is guaranteed to stay consistent. Tests pass (19/19 in `apps/web`). + +### 4. S3 public-endpoint default aligned + +`apps/web-api/src/lib/s3.ts::getS3ClientAndBucket` had a fallback default of `"http://localhost:9000"` for `S3_PUBLIC_ENDPOINT`. The `packages/config` zod default is `"http://localhost:8922"` (the docker-compose host-mapped MinIO API port). Drift between the two meant that a misconfigured env file could produce presigned URLs pointing at the wrong host. + +Now both default to `http://localhost:8922` with an inline comment explaining the alignment. + +### 5. Auth docs cross-referenced + +`docs/auth-plan.md` and `docs/review-auth-system.md` are related but serve different roles (living status doc vs. dated code-review sign-off). Rather than merging them and losing the review's date context, `auth-plan.md` now explicitly cross-references the review file under a "Related review" section so readers landing on either doc know which one to trust for current state. + +### 6. Closed the reclaim-test gap — new `apps/worker/src/queue/claim.test.ts` + +`docs/status.md` previously listed "reclaim / expired-lease worker behavior still needs direct coverage" as a highest-risk gap. This review adds a new `claim.test.ts` file under `apps/worker/src/queue/` covering: + +- `claimOne()` — parameters passed to `CLAIM_SQL` (limit, worker id, lease interval), returned-row mapping, null case when no job is available, and the `CLAIM_SQL_WITH_EXCLUDE` variant when `excludeTypes` is non-empty +- `reclaimExpiredLeases()` — that it uses `WORKER_RECLAIM_BATCH_SIZE` (not `WORKER_CLAIM_BATCH_SIZE`) as the LIMIT parameter, that it returns the rows reclaim surfaced (mix of retryable + dead), and that it respects a custom env value + +Mocking strategy mirrors the existing `lease.test.ts` — mock `withTransaction` from `@cap/db`, assert on SQL text and bind parameters rather than spinning up a real postgres. Six new tests; worker suite goes from 18 to 24 passing. + +### 7. Pre-existing Web E2E breakage on `main` fixed + +While this PR was open, CI surfaced a red `Web E2E` job. Investigation showed the breakage is not caused by this PR — it has been red on `main` since commit `68eaaf1 fix: harden video rail tab rendering` (2026-04-06, runs #21 through #25 all failing the same way, #20 was the last green). + +`68eaaf1` refactored `apps/web/src/pages/video-page/VideoRail.tsx` to use proper ARIA semantics (`
` containing `