From 5d029a28236f5710ffce0156554765b6bf44226f Mon Sep 17 00:00:00 2001 From: Mathias Date: Fri, 5 Jun 2026 23:42:23 +0200 Subject: [PATCH] feat(store): default auto_summarize ON for new users (ADR-018) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Migration 011 flips the auto_summarize column default to TRUE and brings existing rows (maintainer + current registrations) along. Onboarded friends now get zero-friction discovery: scheduled discovery (ADR-018) both discovers AND summarizes new videos, so a user's list fills and summarizes itself instead of presenting an empty list of manual Summarize buttons. Safe only because the process-wide caption-fetch rate gate (ADR-014 item 2, prior commit) now exists — auto + scheduled + multi-user would otherwise self-inflict 429s every cycle. The down migration reverts the default but intentionally leaves existing rows as-is (no surprise manual regression on rollback). RegisterUser already lets the column default drive the value, so no app change is needed; the account-page manual toggle still works. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/adapters/store/migrate_test.go | 37 +++++++++++++++++-- .../011_auto_summarize_default.down.sql | 5 +++ .../011_auto_summarize_default.up.sql | 17 +++++++++ .../adapters/store/summarize_mode_test.go | 21 +++++++++++ internal/web/handlers_test.go | 17 +++++---- 5 files changed, 86 insertions(+), 11 deletions(-) create mode 100644 internal/adapters/store/migrations/011_auto_summarize_default.down.sql create mode 100644 internal/adapters/store/migrations/011_auto_summarize_default.up.sql diff --git a/internal/adapters/store/migrate_test.go b/internal/adapters/store/migrate_test.go index 2bca650..dc5fc8a 100644 --- a/internal/adapters/store/migrate_test.go +++ b/internal/adapters/store/migrate_test.go @@ -49,13 +49,44 @@ func loginEventsExists(t *testing.T) bool { // production during a rollback. The test restores the schema to latest before // returning so the shared embedded-postgres stays at HEAD for sibling tests. func TestMigration010LoginEventsUpDown(t *testing.T) { - newStore(t) // ensure the schema is migrated to latest (010 applied) + newStore(t) // ensure the schema is migrated to latest (011 applied) require.True(t, loginEventsExists(t), "login_events must exist at latest migration") m := fileMigrator(t) - require.NoError(t, m.Steps(-1), "down one migration must drop login_events") + // 011 (auto_summarize default) sits above 010; step it down first so the + // 010 down/up is exercised in isolation. + require.NoError(t, m.Steps(-1), "down 011 must not touch login_events") + require.True(t, loginEventsExists(t), "011 down leaves login_events intact") + + 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(1), "up one migration must recreate login_events") + require.NoError(t, m.Steps(2), "up must recreate 010 then re-apply 011") require.True(t, loginEventsExists(t), "login_events must be restored after the up migration") } + +// autoSummarizeDefault reads the users.auto_summarize column default as text +// ("true"/"false"), so the migration's default flip is verifiable directly. +func autoSummarizeDefault(t *testing.T) string { + t.Helper() + var def string + require.NoError(t, rawPool(t).QueryRow(context.Background(), + `SELECT column_default FROM information_schema.columns + WHERE table_name = 'users' AND column_name = 'auto_summarize'`).Scan(&def)) + return def +} + +// TestMigration011AutoSummarizeDefaultUpDown proves migration 011 is reversible: +// up sets the auto_summarize column default to TRUE (ADR-018), down restores +// FALSE. The down intentionally does not revert existing rows — only the default. +func TestMigration011AutoSummarizeDefaultUpDown(t *testing.T) { + newStore(t) // latest (011 applied) + require.Equal(t, "true", autoSummarizeDefault(t), "011 sets the default to TRUE") + + m := fileMigrator(t) + require.NoError(t, m.Steps(-1), "down 011 reverts the column default") + require.Equal(t, "false", autoSummarizeDefault(t), "default is FALSE after the down migration") + + require.NoError(t, m.Steps(1), "up 011 re-applies the TRUE default") + require.Equal(t, "true", autoSummarizeDefault(t)) +} diff --git a/internal/adapters/store/migrations/011_auto_summarize_default.down.sql b/internal/adapters/store/migrations/011_auto_summarize_default.down.sql new file mode 100644 index 0000000..ecdfd0a --- /dev/null +++ b/internal/adapters/store/migrations/011_auto_summarize_default.down.sql @@ -0,0 +1,5 @@ +-- Revert the column default to FALSE. Existing rows are intentionally NOT +-- reverted: flipping live users back to manual on a rollback would be a +-- surprising regression (they may have come to rely on auto). The default change +-- is the reversible part; data stays as the user left it. +ALTER TABLE users ALTER COLUMN auto_summarize SET DEFAULT FALSE; diff --git a/internal/adapters/store/migrations/011_auto_summarize_default.up.sql b/internal/adapters/store/migrations/011_auto_summarize_default.up.sql new file mode 100644 index 0000000..fca9d60 --- /dev/null +++ b/internal/adapters/store/migrations/011_auto_summarize_default.up.sql @@ -0,0 +1,17 @@ +-- Migration 011: flip auto_summarize default to TRUE (ADR-018, Future-B). +-- +-- Scheduled discovery (ADR-018) makes Tapir watch unprompted. For onboarded +-- friends that only delivers zero-friction value if the list also SUMMARIZES +-- itself — a manual default would mean the scheduler discovers videos a user +-- still has to click through one by one, which is the empty-list problem again. +-- So new users default to AUTO. The account-page toggle still lets a user switch +-- to manual (SetAutoSummarize), so this only changes the out-of-the-box state. +-- +-- Safe only because the process-wide caption-fetch rate gate (ADR-014 item 2) +-- now exists: auto + scheduled + multi-user would otherwise self-inflict 429s +-- every cycle. The gate is the precondition for shipping this default. +ALTER TABLE users ALTER COLUMN auto_summarize SET DEFAULT TRUE; + +-- Bring existing rows (maintainer + any current registrations) onto the new +-- default so they benefit immediately, not just users created after this point. +UPDATE users SET auto_summarize = TRUE WHERE auto_summarize = FALSE; diff --git a/internal/adapters/store/summarize_mode_test.go b/internal/adapters/store/summarize_mode_test.go index 30d60dc..4f2284d 100644 --- a/internal/adapters/store/summarize_mode_test.go +++ b/internal/adapters/store/summarize_mode_test.go @@ -45,6 +45,27 @@ func TestAutoSummarizeRoundTripDefaultsFalse(t *testing.T) { require.False(t, got, "set back to manual round-trips") } +func TestRegisteredUserDefaultsAutoSummarizeOn(t *testing.T) { + ctx := context.Background() + s := newStore(t) + resetDB(t, rawPool(t)) + + // A real registration creates the users row, so the column default (migration + // 011: TRUE) drives the mode — onboarded friends get auto out of the box. + id, err := s.RegisterUser(ctx, subjectA, "Alice") + require.NoError(t, err) + + got, err := s.GetAutoSummarize(ctx, id) + require.NoError(t, err) + require.True(t, got, "new registrations default to auto-summarize (ADR-018)") + + // The account-page toggle still works: a user can switch to manual. + require.NoError(t, s.SetAutoSummarize(ctx, id, false)) + got, err = s.GetAutoSummarize(ctx, id) + require.NoError(t, err) + require.False(t, got, "the manual toggle still flips it off") +} + func TestRequestSummarizeSetsFlag(t *testing.T) { ctx := context.Background() s := newStore(t) diff --git a/internal/web/handlers_test.go b/internal/web/handlers_test.go index 94dc049..99dbc1d 100644 --- a/internal/web/handlers_test.go +++ b/internal/web/handlers_test.go @@ -357,27 +357,28 @@ func TestSummarizeModeToggle(t *testing.T) { app := newApp(t) resetDB(t, rawPool(t)) - // Account page defaults to manual. + // Account page defaults to automatic (ADR-018: onboarded users get + // zero-friction discovery — the list fills and summarizes itself). rec := do(t, app, httptest.NewRequest(http.MethodGet, "/account", nil)) require.Equal(t, http.StatusOK, rec.Code) html := body(t, rec) - require.Contains(t, html, "Manual", "default mode shown") - require.Contains(t, html, "Switch to automatic") + require.Contains(t, html, "Automatic", "default mode shown") + require.Contains(t, html, "Switch to manual") - // Toggle to automatic via HTMX returns the refreshed control. + // Toggle to manual via HTMX returns the refreshed control. req := httptest.NewRequest(http.MethodPost, "/account/summarize-mode", - strings.NewReader("enabled=true")) + strings.NewReader("enabled=false")) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") req.Header.Set("HX-Request", "true") rec = do(t, app, req) require.Equal(t, http.StatusOK, rec.Code) html = body(t, rec) - require.Contains(t, html, "Automatic") - require.Contains(t, html, "Switch to manual") + require.Contains(t, html, "Manual") + require.Contains(t, html, "Switch to automatic") got, err := app.Store.GetAutoSummarize(ctx, userID) require.NoError(t, err) - require.True(t, got, "mode persisted") + require.False(t, got, "mode persisted") } func postSummarize(t *testing.T, app *web.App, videoID string, htmx bool) *httptest.ResponseRecorder {