diff --git a/DECISIONS.md b/DECISIONS.md index 731ad58..58cff81 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -855,6 +855,42 @@ reply still parses; a transcript within budget is unchanged). --- +## ADR-023 — Drop Shorts/livestreams at discovery to protect the caption budget + +**Status:** Accepted (2026-06-10). **Builds on ADR-014** (per-IP caption rate limit is the +binding constraint) and the ADR-022 live-run findings. + +**Context.** The scarce resource is the unofficial timedtext caption fetch (per-egress-IP +429, ~3 successful/pass). The first multi-user run showed the candidate set was mostly noise — +Shorts, sub-minute clips, and live broadcasts — each of which still consumes a caption-fetch +attempt (and a "none" result is a *completed* fetch, so it costs budget even when it yields +nothing). Spending the rate-limited budget on content the user will not read is the waste to +cut first; it is cheaper and lower-risk than raising the ceiling (multi-IP, Whisper). + +Duration and live status are NOT in the playlistItems discovery response, but they ARE in the +Data API `videos.list` (contentDetails.duration + snippet.liveBroadcastContent) — the official +**quota-based** API (1 unit/call, 50 ids/call), which is a *different* limit from the timedtext +429. So one cheap quota call buys a filter that saves many expensive throttled fetches. + +**Decision.** +1. `NewVideos` enriches its candidates with a single `videos.list` call and drops, before + returning: videos shorter than `TAPIR_MIN_VIDEO_SECONDS` (default 60) and any `live`/ + `upcoming` broadcast. Dropped videos are never persisted, so they also declutter the list. +2. The filter is **degrade-open**: `MinVideoSeconds=0` disables it (no quota call), and a + `videos.list` error returns the candidates unfiltered — discovery must never break because a + metadata call hiccuped (worst case = pre-ADR-023 behaviour). +3. The paste-a-URL path (`VideoByID`) is **not** filtered — an explicit user request for a + specific video (even a Short) is honoured. + +**Reversibility.** Pure discovery-time filter + config. `TAPIR_MIN_VIDEO_SECONDS=0` restores +the old behaviour. No schema change, no effect on already-stored videos. + +**Quota note.** Per-channel enrichment adds ~1 unit/channel/pass. At pilot scale (≤3 users) +this is well under the 10k/day cap; at larger scale, batch `videos.list` across channels +(50 ids/call) by collecting all discovered ids per pass before enriching. + +--- + ## Rejected alternatives Approaches considered during the 2026-06-02 planning + grill session and **deliberately not diff --git a/cmd/tapir/processor.go b/cmd/tapir/processor.go index 84a41f7..4e222fe 100644 --- a/cmd/tapir/processor.go +++ b/cmd/tapir/processor.go @@ -88,6 +88,7 @@ func buildProcessor(cfg config.Config, st *store.Store) (*usecase.Engine, error) ClientSecret: cfg.YTClientSecret, TokenSecretRef: cfg.YTTokenRef, PreferredLanguages: []string{"en"}, + MinVideoSeconds: cfg.MinVideoSeconds, }, secretStore) sum := buildSummarizer(cfg) diff --git a/cmd/tapir/scheduler.go b/cmd/tapir/scheduler.go index 1402208..d8e6de3 100644 --- a/cmd/tapir/scheduler.go +++ b/cmd/tapir/scheduler.go @@ -33,6 +33,7 @@ func buildUserRunner(cfg config.Config, st *store.Store, secretStore ports.Secre ClientSecret: cfg.YTClientSecret, TokenSecretRef: web.YouTubeTokenRef(userID), PreferredLanguages: []string{"en"}, + MinVideoSeconds: cfg.MinVideoSeconds, }, secretStore) engine := usecase.NewEngine(src, buildSummarizer(cfg), st) diff --git a/docs/homelab-integration.md b/docs/homelab-integration.md index e2f8b0d..2fcbf3f 100644 --- a/docs/homelab-integration.md +++ b/docs/homelab-integration.md @@ -39,6 +39,11 @@ it** — endpoints and aliases drift, and this file is a snapshot (2026-06-06), - `TAPIR_MAX_TRANSCRIPT_CHARS` — transcript truncation budget sent to the model. **Default `18000`** (~fits an 8k-context model). `0` disables truncation. Prevents the context-overflow HTTP 400 a long transcript caused on `phi4-mini`. +- **Discovery low-value filter (ADR-023).** `TAPIR_MIN_VIDEO_SECONDS` — **default `60`**. At + discovery, `NewVideos` enriches candidates with one cheap `videos.list` call (quota API, NOT + the timedtext 429 path) and drops videos shorter than this plus any live/upcoming broadcast, + so the scarce caption-fetch budget isn't spent on Shorts. `0` disables the filter. The + paste-a-URL path is never filtered. - **Thinking models need an explicit `max_tokens`.** qwen3 / deepseek-r1 spend the budget on reasoning and return **empty content** if `max_tokens` is too low (or unset). The summarizer's parser treats an empty summary as an error for exactly this reason. **Done (2026-06-02, Worker F):** diff --git a/internal/adapters/youtube/youtube.go b/internal/adapters/youtube/youtube.go index 99fd61b..3e9fb22 100644 --- a/internal/adapters/youtube/youtube.go +++ b/internal/adapters/youtube/youtube.go @@ -66,6 +66,13 @@ type Config struct { // poll. Zero means defaultMaxVideos. MaxVideosPerSubscription int + // MinVideoSeconds drops videos shorter than this from discovery (Shorts/clips, + // ADR-023). NewVideos enriches candidates with a single cheap videos.list call + // (contentDetails.duration + snippet.liveBroadcastContent) and filters before + // returning, so the scarce caption-fetch budget is never spent on them. Live + // and upcoming broadcasts are dropped too. Zero disables the filter. + MinVideoSeconds int + // BaseURL overrides the Data API root. Empty means defaultBaseURL. BaseURL string @@ -242,7 +249,97 @@ func (a *Adapter) NewVideos(ctx context.Context, sub domain.Subscription) ([]dom break } } - return videos, nil + + // Drop Shorts/sub-minute clips and live/upcoming broadcasts before they ever + // reach the rate-limited caption path (ADR-023). One cheap videos.list call + // (quota API, not the timedtext throttle) supplies duration + live status. + return a.filterLowValue(ctx, client, videos), nil +} + +// filterLowValue removes videos shorter than cfg.MinVideoSeconds and any live or +// upcoming broadcast, using a single videos.list lookup for duration + +// liveBroadcastContent. The filter is best-effort: if MinVideoSeconds is 0 (off) +// or the lookup fails, the input is returned unfiltered — discovery must not break +// because a metadata call hiccuped; the worst case is the pre-ADR-023 behaviour. +func (a *Adapter) filterLowValue(ctx context.Context, client *http.Client, videos []domain.Video) []domain.Video { + if a.cfg.MinVideoSeconds <= 0 || len(videos) == 0 { + return videos + } + + ids := make([]string, 0, len(videos)) + for _, v := range videos { + ids = append(ids, v.ProviderVideoID) + } + q := url.Values{ + "part": {"contentDetails,snippet"}, + "id": {strings.Join(ids, ",")}, + } + var resp videoListResponse + if err := a.getJSON(ctx, client, "/videos", q, &resp); err != nil { + // Degrade open: keep the candidates rather than lose discovery. + return videos + } + + type meta struct { + seconds int + live string + } + byID := make(map[string]meta, len(resp.Items)) + for _, it := range resp.Items { + byID[it.ID] = meta{seconds: parseISO8601Seconds(it.ContentDetails.Duration), live: it.Snippet.LiveBroadcastContent} + } + + kept := videos[:0] + for _, v := range videos { + m, ok := byID[v.ProviderVideoID] + if !ok { + kept = append(kept, v) // unknown metadata: keep, let the fetch decide + continue + } + if m.live != "" && m.live != "none" { + continue // live or upcoming broadcast + } + if m.seconds > 0 && m.seconds < a.cfg.MinVideoSeconds { + continue // Short / sub-threshold clip + } + kept = append(kept, v) + } + return kept +} + +// parseISO8601Seconds parses an ISO 8601 duration as returned by the YouTube Data +// API (e.g. "PT1H2M3S", "PT45S", "PT3M") into seconds. Only the hour/minute/second +// components YouTube emits are handled; an unparseable or zero value returns 0, +// which the caller treats as "unknown" (not filtered on duration). +func parseISO8601Seconds(d string) int { + if !strings.HasPrefix(d, "PT") { + return 0 + } + d = d[2:] + total, num := 0, 0 + seen := false + for _, r := range d { + switch { + case r >= '0' && r <= '9': + num = num*10 + int(r-'0') + seen = true + case r == 'H': + total += num * 3600 + num, seen = 0, false + case r == 'M': + total += num * 60 + num, seen = 0, false + case r == 'S': + total += num + num, seen = 0, false + default: + return 0 // unexpected component (days/weeks) — treat as unknown + } + } + if seen { + return 0 // trailing digits without a unit: malformed + } + return total } // VideoByID fetches a single video's metadata (videos.list, snippet) for an @@ -377,11 +474,16 @@ type playlistItemListResponse struct { type videoListResponse struct { Items []struct { + ID string `json:"id"` Snippet struct { - Title string `json:"title"` - ChannelTitle string `json:"channelTitle"` - PublishedAt time.Time `json:"publishedAt"` + Title string `json:"title"` + ChannelTitle string `json:"channelTitle"` + PublishedAt time.Time `json:"publishedAt"` + LiveBroadcastContent string `json:"liveBroadcastContent"` } `json:"snippet"` + ContentDetails struct { + Duration string `json:"duration"` // ISO 8601, e.g. "PT1M30S" + } `json:"contentDetails"` } `json:"items"` } diff --git a/internal/adapters/youtube/youtube_test.go b/internal/adapters/youtube/youtube_test.go index 43e412e..1797796 100644 --- a/internal/adapters/youtube/youtube_test.go +++ b/internal/adapters/youtube/youtube_test.go @@ -186,6 +186,91 @@ func TestNewVideosCapsAtMax(t *testing.T) { } } +// TestNewVideosFiltersShortsAndLive: with MinVideoSeconds set, discovery enriches +// candidates via videos.list and drops sub-threshold clips (Shorts) and +// live/upcoming broadcasts before they reach the rate-limited caption path. +func TestNewVideosFiltersShortsAndLive(t *testing.T) { + a, _ := newTestAdapter(t, func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/playlistItems": + _, _ = w.Write([]byte(`{ + "items": [ + {"snippet": {"title": "Real Talk", "publishedAt": "2026-06-03T10:00:00Z", "resourceId": {"videoId": "long1"}}}, + {"snippet": {"title": "A Short", "publishedAt": "2026-06-03T09:00:00Z", "resourceId": {"videoId": "short1"}}}, + {"snippet": {"title": "Live Now", "publishedAt": "2026-06-03T08:00:00Z", "resourceId": {"videoId": "live1"}}} + ] + }`)) + case "/videos": + if got := r.URL.Query().Get("part"); got != "contentDetails,snippet" { + t.Errorf("videos.list part=%q, want contentDetails,snippet", got) + } + _, _ = w.Write([]byte(`{ + "items": [ + {"id": "long1", "contentDetails": {"duration": "PT12M30S"}, "snippet": {"liveBroadcastContent": "none"}}, + {"id": "short1", "contentDetails": {"duration": "PT45S"}, "snippet": {"liveBroadcastContent": "none"}}, + {"id": "live1", "contentDetails": {"duration": "PT0S"}, "snippet": {"liveBroadcastContent": "live"}} + ] + }`)) + default: + t.Errorf("unexpected path %q", r.URL.Path) + } + }) + a.cfg.MinVideoSeconds = 60 + + vids, err := a.NewVideos(context.Background(), domain.Subscription{ID: "s1", UserID: "u1", ChannelID: "UC_acme"}) + if err != nil { + t.Fatalf("NewVideos: %v", err) + } + if len(vids) != 1 || vids[0].ProviderVideoID != "long1" { + t.Fatalf("expected only long1 to survive the filter, got %+v", vids) + } +} + +// TestNewVideosNoFilterWhenDisabled: MinVideoSeconds=0 keeps the pre-ADR-023 +// behaviour — no videos.list call, no filtering. +func TestNewVideosNoFilterWhenDisabled(t *testing.T) { + a, _ := newTestAdapter(t, func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/videos" { + t.Errorf("videos.list must not be called when MinVideoSeconds is 0") + } + _, _ = w.Write([]byte(`{"items": [ + {"snippet": {"title": "A Short", "publishedAt": "2026-06-03T09:00:00Z", "resourceId": {"videoId": "short1"}}} + ]}`)) + }) + a.cfg.MinVideoSeconds = 0 + + vids, err := a.NewVideos(context.Background(), domain.Subscription{ID: "s1", UserID: "u1", ChannelID: "UC_acme"}) + if err != nil { + t.Fatalf("NewVideos: %v", err) + } + if len(vids) != 1 { + t.Fatalf("filter disabled must keep all videos, got %d", len(vids)) + } +} + +func TestParseISO8601Seconds(t *testing.T) { + cases := []struct { + in string + want int + }{ + {"PT45S", 45}, + {"PT1M30S", 90}, + {"PT3M", 180}, + {"PT1H2M3S", 3723}, + {"PT2H", 7200}, + {"PT0S", 0}, + {"", 0}, + {"garbage", 0}, + {"P1D", 0}, // days component not handled → unknown + {"PT10", 0}, // trailing digits without a unit → malformed + } + for _, c := range cases { + if got := parseISO8601Seconds(c.in); got != c.want { + t.Errorf("parseISO8601Seconds(%q) = %d, want %d", c.in, got, c.want) + } + } +} + // TestUploadsPlaylistID covers the zero-cost UC->UU derivation, including // non-standard ids that must fall through unchanged (handled via fallback). func TestUploadsPlaylistID(t *testing.T) { diff --git a/internal/config/config.go b/internal/config/config.go index b8cc86d..4e3334b 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -46,6 +46,12 @@ type Config struct { // MaxTranscriptChars bounds the transcript text sent to the model so a long // transcript does not overflow a small-context primary. 0 disables truncation. MaxTranscriptChars int + + // MinVideoSeconds drops videos shorter than this from discovery (Shorts and + // other sub-minute clips that are noise and waste the scarce caption-fetch + // budget, ADR-014/ADR-023). Enforced via a cheap Data API videos.list lookup at + // discovery, never the rate-limited caption path. 0 disables the filter. + MinVideoSeconds int // SummarizerTimeout bounds a single completion call. Thinking models are // slow, so the default is generous. SummarizerTimeout time.Duration @@ -138,6 +144,7 @@ const ( defaultCloudFallbackModel = "berget/mistral-small" defaultSummaryMaxTokens = 1500 defaultMaxTranscriptChars = 18000 + defaultMinVideoSeconds = 60 defaultSummarizerTimeout = 5 * time.Minute defaultYTTokenRef = "youtube/refresh_token" defaultYTConnectRedirectURL = "https://tapir.d-ma.be/oauth/youtube/callback" @@ -230,6 +237,15 @@ func Load() (Config, error) { } c.MaxTranscriptChars = maxChars + minVideo, err := intOr("TAPIR_MIN_VIDEO_SECONDS", defaultMinVideoSeconds) + if err != nil { + return Config{}, err + } + if minVideo < 0 { + minVideo = 0 + } + c.MinVideoSeconds = minVideo + onboard, err := intOr("TAPIR_ONBOARD_SUMMARIZE_COUNT", defaultOnboardSummarizeCount) if err != nil { return Config{}, err