From 15926fac0e83d86c418fdb6c1a21abf4472cb75f Mon Sep 17 00:00:00 2001 From: Mathias Date: Thu, 11 Jun 2026 18:26:42 +0200 Subject: [PATCH] docs: spec onboarding-wow-burst + ADR-028 (better picks, stronger model) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase-1 live-DB investigation found the connect-time burst (ADR-018) fires but delivers a weak first impression: pure newest-first selection picks junk (a livestream + regional news for pilot user Jonte), and all burst summaries run on the weak koala/phi4-mini instead of the validated iguana/gemma4-26b. Cached-first was investigated and rejected — ~3% cross-user overlap, 0 cached-and-unsummarized, newest-20 all uncached (newest-first and cached-first are structurally incompatible). ADR-028 records: persist duration_s at discovery (ADR-023 already fetches it, just stops discarding), junk-avoiding burst selection (drop known too-short/ too-long), and a stronger burst-only summarizer chain. Not a throughput change. --- DECISIONS.md | 69 +++++++++++++++++ docs/specs/onboarding-wow-burst.md | 116 +++++++++++++++++++++++++++++ 2 files changed, 185 insertions(+) create mode 100644 docs/specs/onboarding-wow-burst.md diff --git a/DECISIONS.md b/DECISIONS.md index 2cb66e3..c65a027 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1060,6 +1060,74 @@ no migration, no stored state to unwind. Spec: `docs/specs/chat-with-transcript. --- +## ADR-028 — Onboarding burst: pick likely-good videos, summarize them with a stronger model + +**Status:** Accepted (2026-06-11). **Refines ADR-018** (the connect-time burst) and **ADR-020** +(recency-bounded auto-summarize). **Builds on ADR-022** (the endpoint chain), **ADR-023** +(the discovery-time `videos.list` enrichment), and **ADR-021** (the shared transcript cache). +Triggered by a Phase-1 investigation of the live pilot DB. + +**Context.** A new user's first session decides whether they return (the Stage-0 gate, ADR-016). +The connect-time burst (ADR-018: summarize ≤`TAPIR_ONBOARD_SUMMARIZE_COUNT` newest videos so the +feed isn't empty) *fires* in production, but a live-DB investigation of the second pilot user +("Jonte") found it delivers a weak first impression for two reasons, and ruled out a third idea: + +1. **Junk picks.** Selection was pure newest-first (`NewestUnsummarizedVideoIDs`, + `ORDER BY published_at DESC`) with **no quality signal**. Jonte's live burst-3 were a + stock-ticker **livestream** + two regional news clips — newest, not best. The cheap signals + that *could* gate this (duration, live status) are fetched by ADR-023's `videos.list` + enrichment at discovery and then **thrown away**: the `videos.duration_s` column (migration + 001) was never written. +2. **Weakest model on the first impression.** All burst summaries ran on `koala/phi4-mini` — the + documented weak link (ADR-022 was born from its failures). The stronger, brain-validated + `iguana/gemma4-26b` was never used, even though the burst is only ~3 summaries. +3. **Cached-first instant summaries — REJECTED.** The idea: skip the fetch, summarize + already-cached transcripts (ADR-021) instantly. The pilot numbers kill it — only **11 videos** + overlap between the two users (~3% of each ~350–400-video library), **0** cached-and- + unsummarized, and a new user's newest-20 unsummarized are **20/20 NOT cached**. Newest-first + and cached-first are structurally incompatible: fresh uploads are exactly what nobody has + fetched. An empty lever at pilot scale. + +**Decision.** +1. **Persist `duration_s` at discovery.** `filterLowValue` (ADR-023) already has each candidate's + duration in hand; carry it onto the kept `domain.Video` and have `UpsertVideo` write it, + COALESCE-preserving a known value (the channel-title backfill stance, migration 014). No new + migration — the column exists. The connect-triggered discovery pass runs *before* the burst, + so a fresh user's candidates are enriched in time. +2. **Junk-avoiding selection.** A new `OnboardBurstVideoIDs(userID, limit, minSeconds, maxSeconds)` + keeps the newest-first order but drops a video when its duration is *known* and outside + `[minSeconds, maxSeconds]` — `minSeconds` = `TAPIR_MIN_VIDEO_SECONDS` (60, the Shorts floor), + `maxSeconds` = new `TAPIR_ONBOARD_MAX_VIDEO_SECONDS` (default 14400 = 4h, to drop multi-hour + livestream VODs that pass the live filter once ended). A NULL duration is **unknown** — kept + (degrade-open) but ranked after known-good rows. **has-captions stays un-gateable pre-fetch** + (only knowable after a gate fetch or a ~0-probability cache hit); selection only *avoids + known-junk*, it does not *promise* captions. +3. **Stronger model for the burst only.** `TAPIR_ONBOARD_SUMMARIZER_MODEL` (default + `iguana/gemma4-26b`) leads a burst-specific summarizer chain (onboard model first, then the + standard ADR-022 chain as resilience, deduped), wrapped in a burst-specific processor over the + *same* store/cache/sink — a pure wiring choice; the engine and ports are unchanged (ADR-003). + Empty or equal-to-primary collapses the burst back onto the shared processor. + +**Not a throughput change.** The caption rate gate (ADR-014) and the foreground priority lane +(ADR-026) are untouched — same pacing, same cap. This changes *which* ≤3 videos the burst spends +its fetches on and *which model* summarizes them, never how fast or how many. The engine's +existing read-stored-first (ADR-021) is unchanged and still yields a free instant summary on the +rare cache hit — we simply do not *select* for cache hits. + +**Consequences.** Better odds of a strong first session: the burst avoids the obvious junk and +runs the better model on the one impression that decides return. The selection improvement is +forward-looking — existing rows have NULL `duration_s` until their next discovery pass backfills +it (lazy, like channel_title); a brand-new user benefits immediately because connect-discovery +runs first. `duration_s` becoming live also unblocks future length-aware features (feed sorting, +"long read" badges) for free. + +**Reversibility.** Pure config + wiring + one column write + one query, no migration. +`TAPIR_ONBOARD_MAX_VIDEO_SECONDS=0` (and `TAPIR_MIN_VIDEO_SECONDS=0`) restores pure newest-first; +`TAPIR_ONBOARD_SUMMARIZER_MODEL=""` collapses the burst back to the shared processor. +Spec: `docs/specs/onboarding-wow-burst.md`. + +--- + ## Rejected alternatives Approaches considered during the 2026-06-02 planning + grill session and **deliberately not @@ -1083,6 +1151,7 @@ maps to the ADR that settles it. | Feedback-based Stage 0 gate (friends saying it's useful) | Politeness bias makes asked-for feedback the least reliable signal; return-usage is the real test | ADR-016 | | Reverse the Dex-write invite flow (Google OIDC only) | Some intended Future-B users won't use Google; OIDC-only leaves them with no onboarding path — invite flow is load-bearing | ADR-017 | | k8s CronJob for scheduled discovery (vs in-process) | At Future-B scale the in-process scheduler is simpler to deploy; CronJob's failure-isolation benefit was weighed and traded away knowingly (revisit if >1 replica or load grows) | ADR-018 | +| Cached-transcript-first onboarding burst (instant, zero-fetch picks) | Live pilot DB: ~3% cross-user video overlap, 0 cached-and-unsummarized, a new user's newest-20 are 20/20 uncached — newest-first and cached-first are structurally incompatible. Empty lever at pilot scale | ADR-028 | If a future case genuinely reopens one of these, that's a new ADR superseding the relevant one — not a silent reversal. diff --git a/docs/specs/onboarding-wow-burst.md b/docs/specs/onboarding-wow-burst.md new file mode 100644 index 0000000..d162526 --- /dev/null +++ b/docs/specs/onboarding-wow-burst.md @@ -0,0 +1,116 @@ +# Spec — Onboarding "wow" burst: better picks, stronger model + +**Repo:** tapir · **Size:** medium · **Solo session** (not a swarm). + +**Why this exists.** A new user's first session decides whether they return (the Stage-0 gate, +VISION.md). On connect, Tapir fires a capped burst (≤`TAPIR_ONBOARD_SUMMARIZE_COUNT`, default 3) +that summarizes the user's newest unsummarized videos so the feed isn't empty (the burst itself +works — wired in `cmd/tapir/discovery.go` → `cmd/tapir/main.go` `onboard`). A Phase-1 +investigation of the live pilot DB found the burst *fires* but delivers a **weak first +impression** for two concrete reasons, and ruled out a third idea: + +1. **Picks are junk.** Selection is pure newest-first (`videos.NewestUnsummarizedVideoIDs`, + `ORDER BY published_at DESC`) with **zero quality signal**. Pilot user "Jonte"'s live burst-3 + were a stock-ticker **livestream** + two regional news clips — the newest, not the best. +2. **Weakest model on the first impression.** All of Jonte's summaries ran on + `koala/phi4-mini` (the documented weak link — ADR-022 was born from its failures). The + stronger, brain-validated `iguana/gemma4-26b` was never used for the burst. +3. **Cached-first is empty at pilot scale — REJECTED.** The idea (summarize already-cached + transcripts instantly, zero fetch) dies on the numbers: only **11 videos** overlap between the + two pilot users (~3% of each library), **0** cached-and-unsummarized, and a new user's + newest-20 unsummarized are **20/20 NOT cached** — newest-first and cached-first are + structurally incompatible (fresh uploads are exactly what nobody has fetched yet). Not built. + +Read `CLAUDE.md`, `DECISIONS.md` (esp. ADR-014, ADR-018, ADR-020, ADR-021, ADR-022, ADR-023, +and the new **ADR-028**), and `VISION.md` (the Stage-0 gate) first. TBD — commit directly to +`main`, one logical change per commit, conventional commits, `task check` green before each +commit, `templ generate` if any view changes (none expected). + +## Decisions already made (do not reopen) +- **Not a throughput change.** The caption rate gate (ADR-014) is untouched — same pacing, same + priority lane (ADR-026). This slice changes *which* ≤3 videos the burst spends its fetches on + and *which model* summarizes them, never how fast or how many. +- **Cached-first is dropped** (ADR-028, the 3% overlap). The engine's existing read-stored-first + (ADR-021, `resolveTranscript`) stays — it already gives a free instant summary on the rare + cache hit, transparently. We do not *select* for cache hits. +- **has-captions is not a pre-fetch signal.** It is only knowable after a gate fetch (or a cache + hit, ~0 for new videos). Selection can only *avoid known-junk* (Shorts/live/over-long) — it + cannot *guarantee* captions. The spec is honest about this: better odds, not a promise. +- **No credentialed caption fetch** (ADR-010/ADR-026 dead end). **No client extension.** + +## 1. Persist `duration_s` at discovery (the enabling change) +The `videos.duration_s` column exists (migration 001) but is **never written** — ADR-023's +`filterLowValue` (`internal/adapters/youtube/youtube.go`) already fetches each candidate's +duration via the cheap quota `videos.list` call, uses it to drop Shorts/live, then **discards +it**. Stop discarding: +- Add `DurationSeconds int` to `domain.Video`. +- In `filterLowValue`, set `DurationSeconds` on each kept video from the `videos.list` `meta`. +- `UpsertVideo` writes `duration_s`, **COALESCE-preserving** a known value (never overwrite a + real duration with 0/unknown), mirroring the `channel_title` backfill stance (migration 014). +- No new migration — the column is already there. + +Consequence: a fresh user's connect-triggered discovery pass runs **before** the onboard burst +(`Enqueue`: `run()` then `onboard()`), so duration is populated for the burst's candidates at +connect. Existing rows backfill on their next discovery pass; until then their `duration_s` is +NULL and treated as "unknown" (§2). + +## 2. Junk-avoiding burst selection +New store method, RLS-scoped via `withUser`: + +``` +OnboardBurstVideoIDs(ctx, userID string, limit, minSeconds, maxSeconds int) ([]string, error) +``` + +- Same base as `NewestUnsummarizedVideoIDs`: the user's videos with no summary yet, + `ORDER BY published_at DESC NULLS LAST, seen_at DESC`, `LIMIT limit`. +- **Exclude known-junk**: a row is dropped only when `duration_s IS NOT NULL` **and** + (`duration_s < minSeconds` OR `duration_s > maxSeconds`). A NULL duration is **unknown** — kept + (degrade-open: never starve the burst because metadata is missing), but ordered *after* rows + with a known-good duration so a freshly-enriched good pick wins when both exist. +- `minSeconds` reuses `TAPIR_MIN_VIDEO_SECONDS` (default 60 — the Shorts floor, ADR-023). + `maxSeconds` is new: `TAPIR_ONBOARD_MAX_VIDEO_SECONDS` (default 14400 = 4h) — drops the + multi-hour livestream VODs that pass the live filter once ended. +- `minSeconds<=0` and `maxSeconds<=0` each disable that bound (so `0/0` == the old + newest-first behaviour, the reversibility lever). +- `NewestUnsummarizedVideoIDs` stays (still correct for any caller that wants pure-newest); the + burst switches to the new method. + +## 3. Stronger model for the burst +The burst summarizes only ≤3 videos, so a slower, stronger model is affordable exactly here. +- New config `TAPIR_ONBOARD_SUMMARIZER_MODEL` (default `iguana/gemma4-26b` — the brain-validated + homelab general-purpose model, already the ADR-022 fallback). +- Build a **burst-specific summarizer chain** that puts the onboard model **first**, then the + standard chain (primary → local fallback → cloud) as resilience, deduped. Wrap it in a + burst-specific `engineProcessor` reusing the same store/transcript-cache/sink — a pure wiring + choice, engine and ports unchanged (Clean Architecture, ADR-003). +- The `onboard` closure uses the burst processor instead of `app.Processor`. +- **Collapse cleanly**: when `OnboardSummarizerModel` is empty or equals `SummarizerModel`, the + onboard path reuses `app.Processor` (no separate chain) — the reversibility lever. +- Local-first preserved: the onboard model is a local alias; the cloud endpoint stays last in the + chain, so a client/NDA deployment with `TAPIR_CLOUD_FALLBACK_MODEL=""` keeps burst content + local too. + +## 4. Behaviour spec + docs +- Add a scenario to `docs/use-cases/connect_account.feature` (the connect → burst flow) covering: + burst skips a too-long/live video in favour of a reasonable-length one; burst summaries record + the onboard model. Map it in `scenarioCoverage` (or tag `@pending` with a reason) so + `TestScenarioCoverage` stays green. +- Update `docs/architecture/architecture.md` (the onboarding-burst / newest-first ordering + section) to describe the junk-avoiding selection + the burst model override. +- ADR-028 in `DECISIONS.md` records the rationale (incl. the rejected cached-first lever). + +## Success criteria +- `task check` green (fmt, vet, lint, `go test -p 1 ./...`). +- A unit test proves `OnboardBurstVideoIDs` drops a known too-long / sub-min video and keeps a + good one, newest-first, RLS-scoped. +- A test proves discovery persists `duration_s` and does not clobber it on re-upsert. +- A test proves the burst processor records the onboard model on its summaries (chain ordering). +- Config defaults + bounds tested (`OnboardMaxVideoSeconds`, `OnboardSummarizerModel`). +- No change to the rate gate, fetch pacing, or burst cap. `0/0` + empty model == prior behaviour. + +## Explicitly NOT in this slice +- Cached-first selection (rejected, ADR-028). +- Any caption-availability *guarantee* (impossible pre-fetch). +- Backfilling `duration_s` for existing rows via a migration (it backfills lazily on discovery). +- Return-nudges / digests (ADR-020: poisons the unprompted-return signal). +- Raising fetch throughput, multi-IP, or Whisper (out of scope; the gate is deliberate).