From cb6917ca596a7930f425cd34063051463bc11b67 Mon Sep 17 00:00:00 2001 From: Mathias Date: Tue, 9 Jun 2026 23:32:12 +0200 Subject: [PATCH] feat(store): shared, video-keyed transcript persistence (ADR-021) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reshape the dead per-user transcripts table (PK videos.id, user_id, RLS-FORCEd — never read or written by app code) into the shared public caption store ADR-021 specifies: keyed by (provider, provider_video_id), no user_id, NOT RLS-scoped. Migration 015 (reversible). Add ports.TranscriptStore + Store.GetTranscript/SaveTranscript via the raw pool (no withUser): public content, shared across users by construction. SaveTranscript persists only terminal outcomes (captions/none) and refuses SourceRateLimited so a transient 429 can never be stored as a false permanent absence (ADR-014). Flip the isolation proof: transcripts leaves the RLS-scoped set; TestTranscriptsTableIsSharedNotRLS asserts it is the SINGLE non-RLS surface (writable/readable with no user scope, no user_id column, RLS off on it alone, still on every user-owned table) — the proof the public-content classification was applied exactly here and leaked nowhere. appPool made idempotent so two tests can build it. Adjust the 010/011/014 up-down migration tests for the new HEAD. account.go: user deletion no longer strips shared transcripts. Reconcile data-model.md + CLAUDE.md. Wiring the engine to read-stored-first is the next commit. Co-Authored-By: Claude Opus 4.8 (1M context) --- CLAUDE.md | 5 +- docs/data-model.md | 34 +++++--- internal/adapters/store/account.go | 11 ++- internal/adapters/store/migrate_test.go | 11 ++- .../015_shared_transcripts.down.sql | 19 ++++ .../migrations/015_shared_transcripts.up.sql | 30 +++++++ internal/adapters/store/rls_test.go | 84 +++++++++++++++--- internal/adapters/store/transcript.go | 66 ++++++++++++++ internal/adapters/store/transcript_test.go | 87 +++++++++++++++++++ internal/ports/ports.go | 20 +++++ 10 files changed, 332 insertions(+), 35 deletions(-) create mode 100644 internal/adapters/store/migrations/015_shared_transcripts.down.sql create mode 100644 internal/adapters/store/migrations/015_shared_transcripts.up.sql create mode 100644 internal/adapters/store/transcript.go create mode 100644 internal/adapters/store/transcript_test.go diff --git a/CLAUDE.md b/CLAUDE.md index f5f045f..6890043 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -46,8 +46,9 @@ These caused real mistakes that were caught and corrected; the corrections are l (See `DECISIONS.md` for full rationale. Listed here so you don't propose them.) - **No Supabase** — reuse Dex / ESO+1Password / Postgres (ADR-002). -- **No global cross-tenant video/transcript table** — per-user isolation (data-model). Dedup - across users is a Future C concern, not a Stage 0/1 default. +- **No global cross-tenant *video* table** — videos stay per-user (data-model). Transcripts ARE + shared since ADR-021 (public caption content, keyed by `(provider, provider_video_id)`, non-RLS) + so re-analysis never re-fetches; the *videos* half of cross-tenant dedup stays a Future C concern. - **No audio-download + speech-to-text in the core path** — captions-first (ADR-007). STT is a deferred, bounded optional component. - **No public SaaS / sign-up / billing / Google OAuth verification at scale** — Future C, diff --git a/docs/data-model.md b/docs/data-model.md index a0c21f8..f290124 100644 --- a/docs/data-model.md +++ b/docs/data-model.md @@ -10,12 +10,15 @@ only opaque references to them; the secret material lives in ESO/1Password (ADR- ## Design decisions baked into this model -- **Per-user isolation, not a shared global video table.** The earlier draft proposed a - global `videos`/`transcripts` table deduped across tenants. Rejected for Future B: it - reintroduces exactly the cross-domain coupling the homelab architecture review is - removing, and at 1–5 users the cost of occasionally re-summarizing the same video is - trivial compared to the isolation it would cost. Each user's data is self-contained. - (Revisit only if Future C makes GPU/transcription cost dominate — a new ADR, not a default.) +- **Per-user isolation for everything except transcripts.** The earlier draft proposed a + global `videos`/`transcripts` table deduped across tenants. **Videos** stay per-user and + RLS-scoped — a shared video table reintroduces exactly the cross-domain coupling the homelab + architecture review is removing. **Transcripts**, however, are now shared (ADR-021): keyed by + `(provider, provider_video_id)`, no `user_id`, **not** RLS-scoped. The cost avoided there is + not LLM re-summarization but a rate-gated, reputation-risky caption fetch (ADR-010/014), which + is paid per re-fetch regardless of user count — so persisting public caption content once and + sharing it strictly beats the coupling it removes. Everything else each user owns is + self-contained; `rls_test.go` proves transcripts is the single exception. - **Secrets by reference only.** Tables hold a `secret_ref` (opaque string/UUID resolved via the `SecretStore` port), never tokens or keys. - **The brain sink is just a delivery target.** No brain-specific tables. Whether a summary @@ -34,7 +37,7 @@ erDiagram USER ||--o{ AI_CREDENTIAL : "has (planned)" VIDEO_CONNECTION ||--o{ SUBSCRIPTION : "exposes (planned)" SUBSCRIPTION ||--o{ VIDEO : "produces (per user)" - VIDEO ||--o| TRANSCRIPT : "has at most one" + VIDEO }o--o| TRANSCRIPT : "shares one by (provider, provider_video_id) — not FK (ADR-021)" VIDEO ||--o| SUMMARY : "has at most one" SUMMARY ||--o{ SINK_DELIVERY : "delivered via" USER ||--o{ CHANNEL_ERROR : "reports unavailable channels" @@ -92,12 +95,12 @@ erDiagram timestamptz rate_limited_at "backoff clock for 429 retries (migration 007)" } TRANSCRIPT { - uuid video_id PK_FK - uuid user_id FK + text provider PK "part of shared key (ADR-021)" + text provider_video_id PK "part of shared key — the cross-user dedup key" text source "captions | none" text language text content "null when source = none" - timestamptz resolved_at + timestamptz fetched_at } SUMMARY { uuid id PK @@ -173,8 +176,12 @@ mechanism. `transcript_status` and `rate_limited_at` (migration 007) track caption-fetch outcomes for rate-limit backoff: `NULL` = not attempted; `rate_limited` = 429 seen, skip until `NOW() - rate_limited_at > TAPIR_FETCH_BACKOFF`; `fetched` = resolved; `none` = no transcript. -- **TRANSCRIPT** — at most one per video. `source = none` records "checked, no usable - transcript" so the watcher doesn't reprocess (ADR-007). `content` null in that case. +- **TRANSCRIPT** — shared public caption content, one row per `(provider, provider_video_id)`, + **not** RLS-scoped and carrying no `user_id` (ADR-021). Two users who watch the same video + share the one row; the summarize path reads it before any caption fetch, so re-analysis never + re-touches YouTube (ADR-010/014). `source = none` records "checked, no usable transcript" so + no one reprocesses (ADR-007); `content` null in that case. A transient 429 is never stored + here — it stays a per-user retry via `VIDEO.transcript_status`. - **SUMMARY** — at most one per video. `fallback_used` + `ai_provider`/`ai_model` make the "is local good enough?" question queryable (the Stage 0 quality signal). `highlights`/ `takeaways` as jsonb to stay schema-flexible while the output format settles. @@ -231,7 +238,8 @@ queue, doesn't replace it). Deferred until there's a reason. ## Explicitly out of scope (Future C) -- Global cross-tenant video/transcript dedup (rejected above). +- Global cross-tenant *video* dedup (rejected above). Note: cross-tenant *transcript* sharing + is now in scope and shipped (ADR-021); only the videos half stays per-user. - Sharding / per-tenant physical databases. - Soft-delete + full audit trail on connections/credentials (a Stage 2 hardening item; add via ADR when Stage 2 work starts). diff --git a/internal/adapters/store/account.go b/internal/adapters/store/account.go index 780c3f1..e13e877 100644 --- a/internal/adapters/store/account.go +++ b/internal/adapters/store/account.go @@ -10,10 +10,13 @@ import ( // DeleteUser permanently removes a user and all of their data. It runs through // withUser so RLS confines every statement to the calling user's own rows. // -// Deleting the users row cascades (ON DELETE CASCADE) to videos, transcripts, -// summaries (→ sink_deliveries), video_connections, and the user_identities map -// — referential-integrity cascades bypass RLS, so a user's child rows are removed -// even though the deleting connection is scoped. summary_actions and login_events +// Deleting the users row cascades (ON DELETE CASCADE) to videos, summaries +// (→ sink_deliveries), video_connections, and the user_identities map — +// referential-integrity cascades bypass RLS, so a user's child rows are removed +// even though the deleting connection is scoped. Transcripts are NOT removed: +// since ADR-021 they are shared public content keyed by (provider, +// provider_video_id) with no user_id, so another user may still reference the +// same row — a user deletion must not strip shared caption content. summary_actions and login_events // are the exceptions: each carries a user_id but has NO foreign key to users // (migrations 002 and 010), so the cascade does not reach them; they are deleted // explicitly in the same scoped transaction. Deleting an absent user is a no-op diff --git a/internal/adapters/store/migrate_test.go b/internal/adapters/store/migrate_test.go index 822b90d..e75d023 100644 --- a/internal/adapters/store/migrate_test.go +++ b/internal/adapters/store/migrate_test.go @@ -53,7 +53,9 @@ func TestMigration010LoginEventsUpDown(t *testing.T) { require.True(t, loginEventsExists(t), "login_events must exist at latest migration") m := fileMigrator(t) - // 011, 012, 013, 014 sit above 010; step them down first so 010 is exercised in isolation. + // 011..015 sit above 010; step them down first so 010 is exercised in isolation. + require.NoError(t, m.Steps(-1), "down 015 reshapes transcripts, login_events intact") + require.True(t, loginEventsExists(t), "015 down leaves login_events intact") require.NoError(t, m.Steps(-1), "down 014 drops channel_title, login_events intact") require.True(t, loginEventsExists(t), "014 down leaves login_events intact") require.NoError(t, m.Steps(-1), "down 013 drops channel_errors, login_events intact") @@ -66,7 +68,7 @@ func TestMigration010LoginEventsUpDown(t *testing.T) { require.NoError(t, m.Steps(-1), "down 010 must drop login_events") require.False(t, loginEventsExists(t), "login_events must be gone after the down migration") - require.NoError(t, m.Steps(5), "up must recreate 010 then re-apply 011, 012, 013, 014") + require.NoError(t, m.Steps(6), "up must recreate 010 then re-apply 011..015") require.True(t, loginEventsExists(t), "login_events must be restored after the up migration") } @@ -89,6 +91,7 @@ func TestMigration011AutoSummarizeDefaultUpDown(t *testing.T) { require.Equal(t, "true", autoSummarizeDefault(t), "011 sets the default to TRUE") m := fileMigrator(t) + require.NoError(t, m.Steps(-1), "down 015 reshapes transcripts") require.NoError(t, m.Steps(-1), "down 014 drops channel_title") require.NoError(t, m.Steps(-1), "down 013 drops channel_errors") require.NoError(t, m.Steps(-1), "down 012 is a no-op") @@ -100,6 +103,7 @@ func TestMigration011AutoSummarizeDefaultUpDown(t *testing.T) { require.NoError(t, m.Steps(1), "up 012 runs clean (no FORCE RLS on fresh schema)") require.NoError(t, m.Steps(1), "up 013 creates channel_errors") require.NoError(t, m.Steps(1), "up 014 recreates channel_title") + require.NoError(t, m.Steps(1), "up 015 reshapes transcripts to shared") } // channelTitleExists reports whether videos.channel_title is present. @@ -119,11 +123,14 @@ func TestMigration014VideoChannelTitleUpDown(t *testing.T) { require.True(t, channelTitleExists(t), "channel_title exists at latest migration") m := fileMigrator(t) + require.NoError(t, m.Steps(-1), "down 015 reshapes transcripts, channel_title intact") + require.True(t, channelTitleExists(t), "015 down leaves channel_title intact") require.NoError(t, m.Steps(-1), "down 014 must drop channel_title") require.False(t, channelTitleExists(t), "channel_title must be gone after the down migration") require.NoError(t, m.Steps(1), "up 014 must recreate channel_title") require.True(t, channelTitleExists(t), "channel_title must be restored after the up migration") + require.NoError(t, m.Steps(1), "up 015 restores the shared transcripts shape (HEAD)") } // TestMigration012FixAutoSummarizeRLS proves 012 runs cleanly and flips any diff --git a/internal/adapters/store/migrations/015_shared_transcripts.down.sql b/internal/adapters/store/migrations/015_shared_transcripts.down.sql new file mode 100644 index 0000000..3239094 --- /dev/null +++ b/internal/adapters/store/migrations/015_shared_transcripts.down.sql @@ -0,0 +1,19 @@ +-- Down 015: restore the per-user RLS-scoped transcripts shape (001 + 003). +DROP TABLE transcripts; + +CREATE TABLE transcripts ( + video_id UUID PRIMARY KEY REFERENCES videos(id) ON DELETE CASCADE, + user_id UUID NOT NULL REFERENCES users(id) ON DELETE CASCADE, + source TEXT NOT NULL, + language TEXT, + content TEXT, + resolved_at TIMESTAMPTZ NOT NULL DEFAULT NOW() +); + +CREATE INDEX idx_transcripts_user_id ON transcripts(user_id); + +ALTER TABLE transcripts ENABLE ROW LEVEL SECURITY; +ALTER TABLE transcripts FORCE ROW LEVEL SECURITY; +CREATE POLICY transcripts_isolation ON transcripts + FOR ALL + USING (user_id = current_setting('tapir.current_user_id', true)::uuid); diff --git a/internal/adapters/store/migrations/015_shared_transcripts.up.sql b/internal/adapters/store/migrations/015_shared_transcripts.up.sql new file mode 100644 index 0000000..7ef3a28 --- /dev/null +++ b/internal/adapters/store/migrations/015_shared_transcripts.up.sql @@ -0,0 +1,30 @@ +-- Migration 015: transcripts become SHARED public-content storage (ADR-021). +-- +-- The per-user transcripts table from 001 (PK videos.id, user_id NOT NULL, RLS +-- FORCEd in 003) was dead: no application code ever read or wrote it — only the +-- transcript_status columns on `videos` (007) carried fetch outcomes. ADR-021 +-- repurposes it as the single shared store of public caption content, keyed by +-- the cross-user dedup key (provider, provider_video_id) — the video's public +-- identity, not Tapir's per-user videos.id — so re-analysis never re-fetches +-- from YouTube (ADR-010/014). +-- +-- It holds ONLY public caption content + the video's public id (nothing +-- user-identifying), so it is deliberately NOT RLS-scoped: no user_id, no +-- policy, no FORCE. This is the single, intentional exception to the ADR-012 +-- isolation boundary; rls_test.go asserts the boundary is exactly here and +-- nowhere else. Dropping the old table drops its RLS policy with it; it held no +-- real data, so drop+recreate loses nothing. +DROP TABLE transcripts; + +CREATE TABLE transcripts ( + provider TEXT NOT NULL, + provider_video_id TEXT NOT NULL, + source TEXT NOT NULL, -- 'captions' (content set) | 'none' (no captions; content NULL) + language TEXT, + content TEXT, + fetched_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + PRIMARY KEY (provider, provider_video_id) +); + +COMMENT ON TABLE transcripts IS + 'Shared public caption content keyed by (provider, provider_video_id). NOT RLS-scoped — public content only, de-facto cross-user dedup (ADR-021).'; diff --git a/internal/adapters/store/rls_test.go b/internal/adapters/store/rls_test.go index 4f07018..a3b80c8 100644 --- a/internal/adapters/store/rls_test.go +++ b/internal/adapters/store/rls_test.go @@ -22,9 +22,11 @@ import ( // (no GUC set → zero rows) proves the enforcement path is live, not bypassed. // userIsolatedTables are the tables that carry a user_id and whose policy keys -// directly off the tapir.current_user_id GUC. +// directly off the tapir.current_user_id GUC. transcripts is deliberately ABSENT +// — ADR-021 made it shared public content (non-RLS); TestTranscriptsTableIsSharedNotRLS +// proves that is the only place the isolation boundary moved. var userIsolatedTables = []string{ - "users", "videos", "transcripts", "summaries", "summary_actions", "login_events", "video_connections", + "users", "videos", "summaries", "summary_actions", "login_events", "video_connections", } // allIsolatedTables adds sink_deliveries, whose ownership is derived from its @@ -38,9 +40,10 @@ type seeded struct { summaryID string } -// seedUser inserts one full chain (user → video → transcript → summary → -// action → delivery) as the superuser pool, which bypasses RLS so both users' -// data lands regardless of the GUC. +// seedUser inserts one full chain (user → video → summary → action → delivery) +// as the superuser pool, which bypasses RLS so both users' data lands regardless +// of the GUC. Transcripts are NOT seeded here: they are shared, non-RLS public +// content (ADR-021), so they have no place in a per-user isolation chain. func seedUser(t *testing.T, p *pgxpool.Pool, userID string) seeded { t.Helper() ctx := context.Background() @@ -54,11 +57,6 @@ func seedUser(t *testing.T, p *pgxpool.Pool, userID string) seeded { VALUES ($1, 'youtube', $2, 'title') RETURNING id`, userID, "vid-"+userID).Scan(&videoID)) - _, err = p.Exec(ctx, - `INSERT INTO transcripts (video_id, user_id, source, content) - VALUES ($1, $2, 'captions', 'words')`, videoID, userID) - require.NoError(t, err) - var summaryID string require.NoError(t, p.QueryRow(ctx, `INSERT INTO summaries (user_id, video_id, summary) VALUES ($1, $2, 'sum') @@ -92,9 +90,16 @@ func appPool(t *testing.T, super *pgxpool.Pool) *pgxpool.Pool { t.Helper() ctx := context.Background() - // Idempotent across test runs (schema/role persist for the TestMain PG). - _, _ = super.Exec(ctx, `DROP ROLE IF EXISTS app`) - _, err := super.Exec(ctx, `CREATE ROLE app LOGIN PASSWORD 'app'`) + // Idempotent across tests AND runs: the role persists for the TestMain PG and + // owns granted privileges, so a plain DROP ROLE fails once any GRANT exists + // (and more than one test now builds an app pool). Create only if absent; the + // GRANTs below are themselves idempotent. + _, err := super.Exec(ctx, + `DO $$ BEGIN + IF NOT EXISTS (SELECT FROM pg_roles WHERE rolname = 'app') THEN + CREATE ROLE app LOGIN PASSWORD 'app'; + END IF; + END $$`) require.NoError(t, err) _, err = super.Exec(ctx, `GRANT USAGE ON SCHEMA public TO app`) require.NoError(t, err) @@ -186,7 +191,6 @@ func TestRLSEnforcesPerUserIsolation(t *testing.T) { {"update users", `UPDATE users SET display_name = 'hacked' WHERE id = $1`, b.userID}, {"update videos", `UPDATE videos SET title = 'hacked' WHERE user_id = $1`, b.userID}, {"queue videos summarize", `UPDATE videos SET summarize_requested = TRUE WHERE id = $1`, b.videoID}, - {"update transcripts", `UPDATE transcripts SET content = 'hacked' WHERE user_id = $1`, b.userID}, {"update summaries", `UPDATE summaries SET summary = 'hacked' WHERE user_id = $1`, b.userID}, {"update summary_actions", `UPDATE summary_actions SET action = 'skipped' WHERE user_id = $1`, b.userID}, {"update login_events", `UPDATE login_events SET seen_at = NOW() WHERE user_id = $1`, b.userID}, @@ -235,3 +239,55 @@ func TestRLSEnforcesPerUserIsolation(t *testing.T) { _ = a // a's ids are seeded for the symmetric read assertions above } + +// TestTranscriptsTableIsSharedNotRLS is the ADR-021 isolation proof: transcripts +// is the ONE shared, non-RLS surface, and the public-content classification +// leaked to nothing else. It is the inverse of TestRLSEnforcesPerUserIsolation — +// where that asserts deny-all on every user-owned table, this asserts transcripts +// is readable and writable with no user scope at all, holds no user_id, and is +// the single table with row-level security switched off. +func TestTranscriptsTableIsSharedNotRLS(t *testing.T) { + newStore(t) + super := rawPool(t) + resetDB(t, super) + app := appPool(t, super) + ctx := context.Background() + + // 1. Shared + non-RLS: with NO GUC set, the app role both writes and reads a + // transcript. On an RLS table this would be deny-all (zero rows), exactly as + // the main isolation test asserts for every user-owned table. + _, err := app.Exec(ctx, + `INSERT INTO transcripts (provider, provider_video_id, source, content) + VALUES ('youtube', 'shared-vid', 'captions', 'public words')`) + require.NoError(t, err, "app role must write shared transcript content with no user scope") + require.Equal(t, 1, scopedCount(t, app, "", "transcripts"), + "transcripts must be readable with NO user scope — it is shared, non-RLS (ADR-021)") + + // 2. No user_id column: the table holds only public caption content + the + // video's public id, nothing user-identifying. + var hasUserID bool + require.NoError(t, super.QueryRow(ctx, + `SELECT EXISTS (SELECT 1 FROM information_schema.columns + WHERE table_name = 'transcripts' AND column_name = 'user_id')`).Scan(&hasUserID)) + require.False(t, hasUserID, "transcripts must carry no user_id (ADR-021 public content)") + + // 3. The boundary is EXACTLY here: every user-owned table still has row-level + // security enabled; transcripts alone has it off. This is the proof the + // non-RLS classification was applied to transcripts and leaked nowhere else. + for _, table := range allIsolatedTables { + require.True(t, rlsEnabled(t, super, table), + "%s must still enforce row-level security — isolation must not have regressed", table) + } + require.False(t, rlsEnabled(t, super, "transcripts"), + "transcripts must be the single table with row-level security OFF (the one shared surface)") +} + +// rlsEnabled reports whether a public table has ROW LEVEL SECURITY enabled. +func rlsEnabled(t *testing.T, p *pgxpool.Pool, table string) bool { + t.Helper() + var enabled bool + require.NoError(t, p.QueryRow(context.Background(), + `SELECT relrowsecurity FROM pg_class + WHERE relname = $1 AND relnamespace = 'public'::regnamespace`, table).Scan(&enabled)) + return enabled +} diff --git a/internal/adapters/store/transcript.go b/internal/adapters/store/transcript.go new file mode 100644 index 0000000..4011aa7 --- /dev/null +++ b/internal/adapters/store/transcript.go @@ -0,0 +1,66 @@ +package store + +import ( + "context" + "errors" + "fmt" + + "github.com/jackc/pgx/v5" + + "gitea.d-ma.be/mathias/tapir/internal/domain" +) + +// GetTranscript returns the shared, stored transcript for a video keyed by the +// cross-user dedup key (provider, providerVideoID), and whether one exists +// (ADR-021). It reads via the raw pool, NOT withUser: the table holds public +// content with no user_id and no RLS policy, so it is shared across users by +// construction. A stored SourceNone is a real hit (ok == true, HasText() == +// false) — a known caption-less video, so the caller skips without re-fetching. +func (s *Store) GetTranscript(ctx context.Context, provider, providerVideoID string) (domain.Transcript, bool, error) { + var source, lang, content string + err := s.pool.QueryRow(ctx, + `SELECT source, COALESCE(language, ''), COALESCE(content, '') + FROM transcripts WHERE provider = $1 AND provider_video_id = $2`, + provider, providerVideoID).Scan(&source, &lang, &content) + if errors.Is(err, pgx.ErrNoRows) { + return domain.Transcript{}, false, nil + } + if err != nil { + return domain.Transcript{}, false, fmt.Errorf("store: get transcript: %w", err) + } + return domain.Transcript{ + Source: domain.TranscriptSource(source), + Language: lang, + Content: content, + }, true, nil +} + +// SaveTranscript upserts the shared transcript for (provider, providerVideoID). +// Only terminal outcomes belong here: SourceCaptions (with text) or SourceNone +// (no captions). A transient SourceRateLimited is rejected so persistence never +// masks a 429 as a permanent absence — that stays a per-user retry (ADR-014). +// Last write wins on conflict (a later re-fetch may correct an entry). It writes +// via the raw pool, NOT withUser — public content, shared, non-RLS (ADR-021). +func (s *Store) SaveTranscript(ctx context.Context, provider, providerVideoID string, t domain.Transcript) error { + switch t.Source { + case domain.SourceCaptions, domain.SourceNone: + // terminal — persist + case domain.SourceRateLimited: + return fmt.Errorf("store: refusing to persist transient rate-limited transcript for %s/%s", provider, providerVideoID) + default: + return fmt.Errorf("store: invalid transcript source %q", t.Source) + } + _, err := s.pool.Exec(ctx, + `INSERT INTO transcripts (provider, provider_video_id, source, language, content) + VALUES ($1, $2, $3, NULLIF($4, ''), NULLIF($5, '')) + ON CONFLICT (provider, provider_video_id) + DO UPDATE SET source = EXCLUDED.source, + language = EXCLUDED.language, + content = EXCLUDED.content, + fetched_at = NOW()`, + provider, providerVideoID, string(t.Source), t.Language, t.Content) + if err != nil { + return fmt.Errorf("store: save transcript: %w", err) + } + return nil +} diff --git a/internal/adapters/store/transcript_test.go b/internal/adapters/store/transcript_test.go new file mode 100644 index 0000000..c83be85 --- /dev/null +++ b/internal/adapters/store/transcript_test.go @@ -0,0 +1,87 @@ +package store_test + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + + "gitea.d-ma.be/mathias/tapir/internal/adapters/store" + "gitea.d-ma.be/mathias/tapir/internal/domain" + "gitea.d-ma.be/mathias/tapir/internal/ports" +) + +// Static check: Store satisfies the shared TranscriptStore port (ADR-021). +var _ ports.TranscriptStore = (*store.Store)(nil) + +func TestSaveAndGetTranscript_RoundTrip(t *testing.T) { + s := newStore(t) + resetDB(t, rawPool(t)) + ctx := context.Background() + + want := domain.Transcript{Source: domain.SourceCaptions, Language: "en", Content: "the words"} + require.NoError(t, s.SaveTranscript(ctx, "youtube", "vid-1", want)) + + got, ok, err := s.GetTranscript(ctx, "youtube", "vid-1") + require.NoError(t, err) + require.True(t, ok, "a saved transcript must be found") + require.Equal(t, domain.SourceCaptions, got.Source) + require.Equal(t, "en", got.Language) + require.Equal(t, "the words", got.Content) + require.True(t, got.HasText()) +} + +func TestGetTranscript_Miss(t *testing.T) { + s := newStore(t) + resetDB(t, rawPool(t)) + + _, ok, err := s.GetTranscript(context.Background(), "youtube", "absent") + require.NoError(t, err, "a miss is not an error") + require.False(t, ok) +} + +// A stored "no captions" outcome is a real hit: callers must skip without +// re-fetching, so ok is true even though there is no text (ADR-021 / ADR-007). +func TestSaveAndGetTranscript_NoneIsAStoredHit(t *testing.T) { + s := newStore(t) + resetDB(t, rawPool(t)) + ctx := context.Background() + + require.NoError(t, s.SaveTranscript(ctx, "youtube", "vid-none", domain.Transcript{Source: domain.SourceNone})) + + got, ok, err := s.GetTranscript(ctx, "youtube", "vid-none") + require.NoError(t, err) + require.True(t, ok, "a stored SourceNone is a hit, not a miss") + require.Equal(t, domain.SourceNone, got.Source) + require.False(t, got.HasText()) +} + +// A transient 429 must never be persisted as a terminal transcript, or a later +// read would mask the rate-limit as a permanent "no transcript" (ADR-014). +func TestSaveTranscript_RejectsRateLimited(t *testing.T) { + s := newStore(t) + resetDB(t, rawPool(t)) + + err := s.SaveTranscript(context.Background(), "youtube", "vid-429", + domain.Transcript{Source: domain.SourceRateLimited}) + require.Error(t, err) + + _, ok, _ := s.GetTranscript(context.Background(), "youtube", "vid-429") + require.False(t, ok, "a rejected rate-limited save must leave nothing stored") +} + +func TestSaveTranscript_UpsertLastWriteWins(t *testing.T) { + s := newStore(t) + resetDB(t, rawPool(t)) + ctx := context.Background() + + require.NoError(t, s.SaveTranscript(ctx, "youtube", "vid-up", domain.Transcript{Source: domain.SourceNone})) + require.NoError(t, s.SaveTranscript(ctx, "youtube", "vid-up", + domain.Transcript{Source: domain.SourceCaptions, Language: "en", Content: "now resolved"})) + + got, ok, err := s.GetTranscript(ctx, "youtube", "vid-up") + require.NoError(t, err) + require.True(t, ok) + require.Equal(t, domain.SourceCaptions, got.Source) + require.Equal(t, "now resolved", got.Content) +} diff --git a/internal/ports/ports.go b/internal/ports/ports.go index 77f148c..2d2224a 100644 --- a/internal/ports/ports.go +++ b/internal/ports/ports.go @@ -27,6 +27,26 @@ type Summarizer interface { Summarize(ctx context.Context, v domain.Video, t domain.Transcript) (domain.Summary, error) } +// TranscriptStore persists transcripts as shared, video-keyed public content +// (ADR-021). It is keyed by the cross-user dedup key (provider, providerVideoID) +// — the video's public identity, NOT Tapir's per-user videos.id — and holds only +// public caption content, so it is deliberately NOT user-scoped: two users who +// share a video share the one row. The engine reads it before any caption fetch +// so re-analysis never re-touches YouTube (ADR-010/014). +type TranscriptStore interface { + // GetTranscript returns the stored transcript for a video and whether one + // exists. A stored Source == SourceNone (captions permanently absent) is a + // real hit: ok is true and HasText() is false, so callers skip without + // re-fetching. A transient rate-limit is never stored, so it never appears + // here as a false absence. + GetTranscript(ctx context.Context, provider, providerVideoID string) (t domain.Transcript, ok bool, err error) + // SaveTranscript upserts the transcript for (provider, providerVideoID). Only + // terminal outcomes are persisted: SourceCaptions (with text) or SourceNone. + // SourceRateLimited must NOT be passed — it is a per-user retry (ADR-014), not + // a shared terminal state. + SaveTranscript(ctx context.Context, provider, providerVideoID string, t domain.Transcript) error +} + // Sink delivers a summary to a destination (user store, brain, ...). // Implementations fail independently of one another. type Sink interface {