feat(discovery): persist video duration_s instead of discarding it (ADR-028)

ADR-023's filterLowValue already fetches each candidate's duration via the
cheap videos.list quota call to drop Shorts/live, then threw it away — the
videos.duration_s column (migration 001) was never written. Carry it onto the
kept domain.Video and have UpsertVideo persist it, COALESCE-preserving a known
value so an unknown (0) re-upsert never clobbers it (the channel_title backfill
stance, migration 014). This is the enabling change for length-aware burst
selection. No new migration — the column already exists.
This commit is contained in:
2026-06-11 19:54:29 +02:00
parent d21077303d
commit 9f0d8cf198
5 changed files with 60 additions and 4 deletions
+16 -4
View File
@@ -46,15 +46,16 @@ func (s *Store) UpsertVideo(ctx context.Context, v domain.Video) (string, error)
}
if err := tx.QueryRow(ctx,
`INSERT INTO videos (user_id, provider, provider_video_id, title, url, published_at, channel_title)
VALUES ($1, $2, $3, $4, $5, $6, $7)
`INSERT INTO videos (user_id, provider, provider_video_id, title, url, published_at, channel_title, duration_s)
VALUES ($1, $2, $3, $4, $5, $6, $7, $8)
ON CONFLICT (user_id, provider, provider_video_id) DO UPDATE SET
title = EXCLUDED.title,
url = EXCLUDED.url,
published_at = EXCLUDED.published_at,
channel_title = COALESCE(NULLIF(EXCLUDED.channel_title, ''), videos.channel_title)
channel_title = COALESCE(NULLIF(EXCLUDED.channel_title, ''), videos.channel_title),
duration_s = COALESCE(EXCLUDED.duration_s, videos.duration_s)
RETURNING id`,
v.UserID, provider, v.ProviderVideoID, v.Title, v.URL, nullTime(v.PublishedAt), v.ChannelTitle,
v.UserID, provider, v.ProviderVideoID, v.Title, v.URL, nullTime(v.PublishedAt), v.ChannelTitle, nullDuration(v.DurationSeconds),
).Scan(&id); err != nil {
return fmt.Errorf("store: upsert video: %w", err)
}
@@ -74,6 +75,17 @@ func nullTime(t time.Time) *time.Time {
return &t
}
// nullDuration maps an unknown duration (0) to SQL NULL so the upsert's
// COALESCE(EXCLUDED.duration_s, videos.duration_s) preserves a previously-known
// value instead of clobbering it with 0 (ADR-028; the channel_title backfill
// stance, migration 014).
func nullDuration(seconds int) *int {
if seconds <= 0 {
return nil
}
return &seconds
}
// NewestUnsummarizedVideoIDs returns up to limit of the user's videos that have
// no summary yet, newest first (published_at DESC, NULLS LAST). It caps the
// connect-time onboarding burst (Feature 1) at a fixed count: the caller marks
+30
View File
@@ -52,6 +52,36 @@ func TestUpsertVideo_ReturnsStableID(t *testing.T) {
require.Equal(t, 1, count, "must not duplicate the row")
}
func TestUpsertVideo_PersistsAndPreservesDuration(t *testing.T) {
ctx := context.Background()
s := newStore(t)
resetDB(t, rawPool(t))
// First upsert carries a known duration (ADR-028: discovery enriches it).
v := ytVideo(userA, "dur0000001x", "with duration")
v.DurationSeconds = 750
id, err := s.UpsertVideo(ctx, v)
require.NoError(t, err)
p := rawPool(t)
readDuration := func() *int {
var d *int
require.NoError(t, p.QueryRow(ctx, `SELECT duration_s FROM videos WHERE id = $1`, id).Scan(&d))
return d
}
require.NotNil(t, readDuration())
require.Equal(t, 750, *readDuration(), "duration must persist")
// A later upsert that does NOT know the duration (0) must not clobber it —
// the channel_title backfill stance (migration 014): COALESCE-preserve.
v2 := ytVideo(userA, "dur0000001x", "title updated, duration unknown")
v2.DurationSeconds = 0
_, err = s.UpsertVideo(ctx, v2)
require.NoError(t, err)
require.NotNil(t, readDuration(), "a 0/unknown re-upsert must not erase a known duration")
require.Equal(t, 750, *readDuration())
}
func TestUpsertVideo_IDMatchesSummaryDedup(t *testing.T) {
ctx := context.Background()
s := newStore(t)
+4
View File
@@ -302,6 +302,10 @@ func (a *Adapter) filterLowValue(ctx context.Context, client *http.Client, video
if m.seconds > 0 && m.seconds < a.cfg.MinVideoSeconds {
continue // Short / sub-threshold clip
}
// Carry the duration we already fetched onto the kept video so the store
// can persist it (ADR-028) — the burst's length-aware selection depends on
// it. Discarding it here was the gap the onboarding investigation found.
v.DurationSeconds = m.seconds
kept = append(kept, v)
}
return kept
@@ -224,6 +224,11 @@ func TestNewVideosFiltersShortsAndLive(t *testing.T) {
if len(vids) != 1 || vids[0].ProviderVideoID != "long1" {
t.Fatalf("expected only long1 to survive the filter, got %+v", vids)
}
// The duration fetched for the filter is carried onto the kept video so the
// store can persist it (ADR-028) instead of discarding it.
if vids[0].DurationSeconds != 750 {
t.Fatalf("kept video DurationSeconds = %d, want 750 (PT12M30S)", vids[0].DurationSeconds)
}
}
// TestNewVideosNoFilterWhenDisabled: MinVideoSeconds=0 keeps the pre-ADR-023