1249 lines
82 KiB
Markdown
1249 lines
82 KiB
Markdown
# Architecture Decision Records — Tapir
|
||
|
||
Lightweight ADRs. Each records a decision, its context, and its consequences at the time
|
||
it was made. Decisions are **append-only**: to reverse one, add a new ADR that supersedes
|
||
it (note the supersession in both). This is the reversibility record the VISION guardrails
|
||
refer to.
|
||
|
||
Status values: `Accepted` · `Superseded by ADR-NNN` · `Proposed`.
|
||
|
||
---
|
||
|
||
## ADR-001 — Go, not Python
|
||
|
||
**Status:** Accepted (2026-06-02)
|
||
|
||
**Context.** The first design draft assumed a Python/FastAPI/arq stack. The homelab is a
|
||
Go estate (`template-go-web`, `template-go-agent`, `mcp-chassis`, `hyperguild/ingestion`).
|
||
A Python service would be the only one of its kind, outside every shared convention, chassis,
|
||
and CI pattern.
|
||
|
||
**Decision.** Tapir is written in Go. Python is used only where it makes no sense not to
|
||
(e.g. if an ML component later genuinely requires it — none does today).
|
||
|
||
**Consequences.** Reuses homelab Go conventions and the `mcp-chassis` auth pattern if/when
|
||
Tapir exposes an MCP surface. Closes the door on the Python async ecosystem (acceptable —
|
||
Go's concurrency model fits the watcher/worker shape).
|
||
|
||
---
|
||
|
||
## ADR-002 — No Supabase; reuse existing Dex / ESO / Postgres conventions
|
||
|
||
**Status:** Accepted (2026-06-02). The "Tapir does not write to the shared identity provider"
|
||
implication is **partially superseded by ADR-017** (invite flow writes Dex Password CRs); the
|
||
no-Supabase / reuse-existing-primitives decision stands.
|
||
|
||
**Context.** The draft proposed self-hosted Supabase for auth + RLS + secrets (Vault). The
|
||
homelab already runs **Dex** (OIDC), **ESO + 1Password** (secrets), and a **postgres18**
|
||
instance that the May-2026 architecture review is actively *de*-coupling by blast radius.
|
||
Supabase would bundle a second auth system, a second secrets store, and its own Postgres —
|
||
three duplications of things just consolidated.
|
||
|
||
**Decision.** No Supabase. Auth, secrets, and persistence reuse the homelab's existing
|
||
primitives: Dex for identity (when external identity is needed), ESO + 1Password for secret
|
||
custody, Postgres with per-tenant role/grant and the `tenant=` namespace label scheme for
|
||
isolation when Future B/C arrives.
|
||
|
||
**Consequences.** No new infra to operate or back up. Multi-tenancy uses the architecture
|
||
review's SC7/P6 primitives rather than Supabase RLS. For Stage 0 (single user) most of this
|
||
is dormant; it activates at Stage 1.
|
||
|
||
---
|
||
|
||
## ADR-003 — Standalone-first; brain is one sink behind an interface
|
||
|
||
**Status:** Accepted (2026-06-02)
|
||
|
||
**Context.** Tapir was initially framed as both a standalone service and a brain-ingestion
|
||
pipeline. The maintainer clarified that **standalone is the more important** of the two.
|
||
|
||
**Decision.** Tapir is a standalone service. Delivery of summaries is via a **Sink
|
||
interface** with multiple implementations. The user's own store is the primary sink; the
|
||
brain is *one optional sink among others*. The engine does not know or care which sinks are
|
||
attached.
|
||
|
||
**Consequences.** Brain ingestion can never dictate the core architecture (a named drift
|
||
signal in VISION). Homelab mode is "the brain sink is enabled"; it is not a separate build.
|
||
|
||
---
|
||
|
||
## ADR-004 — Copy the `llm` package from `hyperguild/ingestion`; do not lift to a shared lib
|
||
|
||
**Status:** Accepted (2026-06-02)
|
||
|
||
**Context.** Spike S5 (`infra/docs/superpowers/handoffs/2026-06-02-video-adapter-placement.md`)
|
||
examined what Tapir could reuse from `hyperguild/ingestion`. The `internal/llm` package
|
||
(`Client` + `Router`) is stdlib-only and its `Router` implements exactly the
|
||
Primary→Fallback pattern Tapir needs for local-first → BYO-AI. It is ~3.5 KB and stable.
|
||
|
||
**Decision.** **Copy** the `llm` package into Tapir and own it. Do not lift it into a shared
|
||
module. Lifting would couple Tapir's release cycle to the monolith to save ~100 lines —
|
||
a bad trade against the "standalone owes nothing" bias.
|
||
|
||
**Consequences.** Minor duplication. Tapir owes nothing to `hyperguild/ingestion` at the
|
||
dependency level. Primary = local stack via LiteLLM/piguard alias; Fallback = user BYO key.
|
||
|
||
---
|
||
|
||
## ADR-005 — Brain sink is an HTTP brain-mcp client, not the filesystem `brain` package
|
||
|
||
**Status:** Accepted (2026-06-02)
|
||
|
||
**Context.** `hyperguild/ingestion`'s `internal/brain` package manipulates the brain git
|
||
checkout **on the local filesystem** (`os.WriteFile` into `brain/wiki/...`). That assumes
|
||
co-location with the brain repo — fine for the monolith, wrong for a standalone service
|
||
where brain is one network-reached sink.
|
||
|
||
**Decision.** Tapir's brain sink is a thin **HTTP adapter calling brain-mcp's `brain_ingest`
|
||
tool**. It does not import or replicate the filesystem `brain` package.
|
||
|
||
**Consequences.** The brain sink works regardless of where Tapir runs. Adds a dependency on
|
||
brain-mcp availability when that sink is enabled (acceptable; sinks fail independently).
|
||
|
||
---
|
||
|
||
## ADR-006 — Outbound OAuth (YouTube/Vimeo) written fresh
|
||
|
||
**Status:** Accepted (2026-06-02)
|
||
|
||
**Context.** `hyperguild/ingestion`'s `internal/oauth` package is the MCP **server's
|
||
inbound** auth (client_credentials, authenticating claude.ai). Tapir needs **outbound**
|
||
authorization-code OAuth to YouTube/Vimeo with per-user token custody — a different concern
|
||
that happens to share a name.
|
||
|
||
**Decision.** Write the YouTube/Vimeo OAuth client fresh using `golang.org/x/oauth2`. Store
|
||
per-user refresh tokens via the homelab's ESO/1Password convention (ADR-002), not a new
|
||
secrets system.
|
||
|
||
**Consequences.** No reuse from the monolith here. Token custody is the highest-value
|
||
secret surface (see VISION Stage 2); it gets the existing, vetted secret path.
|
||
|
||
---
|
||
|
||
## ADR-007 — Captions-first; audio-download + speech-to-text deferred
|
||
|
||
**Status:** Accepted (2026-06-02). Acquisition mechanism superseded by ADR-010 (Data API
|
||
`captions.download` → player/timedtext baseUrl); captions-first stance and STT deferral stand.
|
||
|
||
**Context.** YouTube's Data API does not expose transcripts. Options: official captions
|
||
(clean, limited coverage) vs. audio download + local Whisper (broad coverage, ToS-grey,
|
||
GPU-contending with the JEPA PoC on koala, breakage-prone via yt-dlp).
|
||
|
||
**Decision.** The core path is **captions-only**. When a video has no usable transcript,
|
||
Tapir records "no transcript" and moves on. Audio-download + speech-to-text is a deferred,
|
||
clearly-bounded *optional* component, not part of the core path or Stage 0.
|
||
|
||
**Consequences.** Some videos won't be summarized at Stage 0 — accepted. The "is this
|
||
useful to me?" hypothesis is testable on captioned videos alone. Avoids GPU contention and
|
||
ToS risk in the validated path.
|
||
|
||
---
|
||
|
||
## ADR-008 — Future C (public multi-tenant SaaS) deferred behind the Stage 0 gate
|
||
|
||
**Status:** Accepted (2026-06-02)
|
||
|
||
**Context.** "Real users soon" was initially asserted, which would load Google OAuth
|
||
verification, billing, and hardened multi-tenant custody early. Lowered to Future B (1–5
|
||
trusted users), with the maintainer as the real first customer.
|
||
|
||
**Decision.** Build for Stage 0 then Stage 1 (Future B). Public SaaS machinery (sign-up,
|
||
billing, Google OAuth app verification at scale) is **not built** until the Stage 0
|
||
self-use gate passes **and** real external demand appears.
|
||
|
||
**Consequences.** Smaller, faster first build. The staged Definition of Success in VISION
|
||
governs advancement. Reversible: if demand appears, a new ADR opens the Future C scope.
|
||
|
||
---
|
||
|
||
## ADR-009 — Trunk-Based Development
|
||
|
||
**Status:** Accepted (2026-06-02)
|
||
|
||
**Context.** Platform-wide convention (homelab architecture review invariant; gitea-mcp #27):
|
||
commit directly to `main`, one logical change per commit, every commit deployable.
|
||
|
||
**Decision.** Tapir follows TBD. Commit directly to `main`. No feature branches or PRs for
|
||
solo/agent work; short-lived `agent/<desc>` branches only when parallel agents are active on
|
||
the repo simultaneously. CI is the quality gate, not branch protection.
|
||
|
||
**Consequences.** Consistent with the rest of the estate. Depends on the direct-to-main write
|
||
path tracked in gitea-mcp #35 (item #1).
|
||
|
||
---
|
||
|
||
## ADR-010 — Third-party caption acquisition via the timedtext/player baseUrl
|
||
|
||
**Status:** Accepted (2026-06-02)
|
||
|
||
**Context.** ADR-007 settled *captions-first*. The first build used the YouTube Data API
|
||
`captions.list` + `captions.download` endpoints to acquire them. A live Stage-0 run proved that
|
||
`captions.download` is **owner-only**: it requires the OAuth identity to own the video, so every
|
||
subscription video the user does *not* own returns HTTP 403. Result: 0 summaries produced and a
|
||
~150-line error spew. The captions-first decision is sound; only the *acquisition mechanism* was
|
||
wrong.
|
||
|
||
**Decision.** Acquire captions from the **player response + timedtext baseUrl**, not the Data API
|
||
`captions` endpoints:
|
||
|
||
1. `POST https://www.youtube.com/youtubei/v1/player` with an **InnerTube `ANDROID` client
|
||
context** (no API key, no OAuth). Read
|
||
`captions.playerCaptionsTracklistRenderer.captionTracks[]`. Each track carries `baseUrl`,
|
||
`languageCode`, and `kind` (`"asr"` = auto-generated).
|
||
2. Select by `PreferredLanguages`, preferring non-`asr` when both exist.
|
||
3. **GET the track's `baseUrl` unauthenticated** (plain `http.Client`, no OAuth token attached —
|
||
the token can break the timedtext endpoint). The ANDROID `baseUrl` is pinned to `fmt=srv3`
|
||
(timedtext XML); the parser also accepts `json3` and the legacy `<transcript>` XML.
|
||
|
||
A **watch-page scrape** of `ytInitialPlayerResponse` is the documented fallback if InnerTube
|
||
returns no `captionTracks`.
|
||
|
||
**Live validation (from koala, 2026-06-02):** the `ANDROID` InnerTube client returned 6
|
||
`captionTracks` with working `baseUrl`s for a public video the run identity does not own, and the
|
||
unauthenticated `baseUrl` GET returned real transcript text. `ANDROID` is the client of record
|
||
(historically returns baseUrls without a PoToken). The `WEB` client and watch-page scrape are
|
||
fallbacks.
|
||
|
||
**Consequences.**
|
||
|
||
- Works for **any public captioned video**, not just owned ones — this is the fix for the 403 wall.
|
||
- **ToS-grey:** `youtubei`/`timedtext` are unofficial endpoints. They can break when Google shifts
|
||
InnerTube client requirements or introduces PoToken gating. Mitigation: degrade, never error — a
|
||
missing/empty/403/unparseable caption yields `domain.Transcript{Source: SourceNone}`, so a future
|
||
breakage produces "no transcript" rather than a crash or error spew. Only genuine transport
|
||
(network) faults error.
|
||
- **No OAuth** is needed for the transcript fetch. OAuth is still required for `ListSubscriptions`
|
||
and `NewVideos` (Data API) — only the transcript path goes unauthenticated.
|
||
- **Still no Whisper.** Speech-to-text stays deferred (ADR-007 unchanged).
|
||
|
||
**Supersedes:** the *acquisition mechanism* of ADR-007 (Data API `captions.download` →
|
||
player/timedtext baseUrl) only. ADR-007's captions-first stance and the STT deferral stand.
|
||
|
||
---
|
||
|
||
## ADR-011 — Web read-surface at Stage 0: Dex authn (single-user authz), action signal, public ingress + GitOps
|
||
|
||
**Status:** Accepted (2026-06-02)
|
||
|
||
**Context.** The Stage-0 CLI (`list`/`show`) reads summaries but doesn't capture the Stage-0
|
||
*headline test* — whether the maintainer **acts** on a summary (watches/skips/saves because of
|
||
it). A browser surface is wanted, and the maintainer chose to deploy it properly:
|
||
**`tapir.d-ma.be` via the homelab ingress, full k3s/Flux GitOps**, with **Dex login from the
|
||
start** rather than a Tailscale-only no-auth dev page. The data-model note says "auth is
|
||
dormant at Stage 0", so logging in early is a deliberate deviation worth recording.
|
||
|
||
**Decision.**
|
||
1. Add a Stage-0 web reader (`tapir serve`, HTMX+Templ) over the existing `store` — a new
|
||
transport, not a core change (ADR-003). Pages: summary list + full view + **watch/skip/save**
|
||
actions recorded in a new `summary_actions` table. The action signal instruments the
|
||
Stage-0 success metric directly.
|
||
2. **Authentication** via Dex OIDC (ADR-002 already names Dex for identity). **Authorization
|
||
stays trivial**: an allowlist of one subject (the maintainer). NO user CRUD, NO per-tenant
|
||
isolation — that authz/isolation work is the real Stage-1/2 line and stays deferred. The
|
||
distinction (authn now, authz/tenancy later) is what keeps this honest with VISION.
|
||
3. **Deploy** at `tapir.d-ma.be` via the existing homelab pattern: gitea CI (buildah) →
|
||
registry → **Flux** reconciling manifests in `mathias/infra` `k3s/apps/tapir/`; secrets via
|
||
**ESO + 1Password**; `postgres18` reached in-cluster. This is application of existing
|
||
convention, not a new infra decision.
|
||
|
||
**Consequences.** The maintainer gets a real, authenticated reading surface and the Stage-0
|
||
metric becomes queryable. A browser session-login path now exists (distinct from
|
||
`mcp-chassis`'s inbound Bearer-JWT validation — not the same code). Wiring login early adds a
|
||
Dex static-client registration + ingress/TLS as prerequisites. If multiple users ever arrive,
|
||
authorization/isolation is a new ADR (Stage 1) — this one deliberately does not build it.
|
||
Full spec: `docs/ui-spec.md`.
|
||
|
||
---
|
||
|
||
## ADR-012 — Open Stage 1: multi-user with enforced isolation (RLS) in the same slice
|
||
|
||
**Status:** Accepted (2026-06-03)
|
||
|
||
**Context.** ADR-011 shipped a single-user web reader with an allowlist of one, deferring
|
||
multi-user authz/isolation to "a new ADR (Stage 1)". The maintainer has chosen to **open
|
||
Stage 1 now** — multi-user registration, per-user YouTube connect, and account management —
|
||
ahead of the formal Stage-0 self-use gate. VISION's hard invariant ("data isolation is a
|
||
promise, not a feature flag … holds from the first user") and the drift signal ("building
|
||
Stage 1+ machinery before the gate") make one thing non-negotiable: **multi-user features
|
||
must not ship before isolation is enforced.**
|
||
|
||
**Decision.**
|
||
1. **Open Stage 1.** Build registration (explicit, not just-in-time: a Dex-authenticated
|
||
subject with no `users` row completes a registration step that creates it), per-user
|
||
web-initiated YouTube OAuth connect (distinct from the CLI `tapir auth`), and account
|
||
management (view / disconnect / delete).
|
||
2. **Isolation is DB-enforced via Postgres Row-Level Security**, not application-layer
|
||
filtering — realising ADR-002's per-tenant-role intent. RLS is `FORCE`d on every
|
||
user-owned table (the app connects as the non-superuser table-owner `tapir` role, which
|
||
would otherwise bypass RLS); every request scopes rows via `tapir.current_user_id`
|
||
(`SET LOCAL` inside a transaction), routed through a single structural helper so scoping
|
||
is not per-query opt-in.
|
||
3. **The Stage-2 isolation bar is pulled forward into THIS slice, not deferred.** A real
|
||
isolation test (two users, disposable/embedded Postgres) — user A reads/writes zero of
|
||
user B's rows across every table — ships green **with** the multi-user features. No
|
||
multi-user feature merges ahead of that test passing.
|
||
|
||
**Consequences.** Stage 1 + the Stage-2 isolation guarantee land together; isolation is
|
||
structural (DB), so it cannot be forgotten per-query. Adds `video_connections` and
|
||
`subscriptions` tables, a web OAuth callback, and a registration surface. The single-user
|
||
allowlist (ADR-011) is replaced by per-subject `users` rows. Reversible only by a superseding
|
||
ADR. This deliberately advances ahead of the Stage-0 gate — recorded as the maintainer's
|
||
explicit call, with isolation as the guardrail that keeps it safe.
|
||
|
||
---
|
||
|
||
## ADR-013 — Account deletion is Tapir-side only; the Dex identity is left intact
|
||
|
||
**Status:** Accepted (2026-06-03). The "Tapir never holds write access to the shared identity
|
||
provider" rationale is **partially superseded by ADR-017** (the invite flow now *creates* Dex
|
||
Password CRs). The deletion-behaviour decision itself — delete Tapir-side state, leave the Dex
|
||
identity intact — still stands; ADR-017 only changes the create side, not delete. See ADR-017
|
||
for the now-asymmetric posture (Tapir can create Dex accounts but still does not delete them).
|
||
|
||
**Context.** Stage 1 (ADR-012) added account deletion. A registered user is two things: a
|
||
`users` row (plus all their data, cascade-linked) in Tapir's Postgres, and a subject identity
|
||
in **Dex** (the homelab OIDC provider, shared across the estate — Tapir does not own it).
|
||
"Delete my account" could mean (a) erase all Tapir-side data and secrets, or (b) that plus
|
||
deprovision the Dex identity. The maintainer chose (a).
|
||
|
||
**Decision.** Deleting a Tapir account removes **only Tapir-side state**:
|
||
- The `users` row, cascading to all user-owned tables (`videos`, `transcripts`, `summaries`,
|
||
`sink_deliveries`, `video_connections`, and — via an **explicit delete**, because it has no
|
||
FK — `summary_actions`). The delete test asserts the cascade reaches every table and leaves
|
||
other users' rows untouched.
|
||
- All of that user's secrets in the SecretStore (the per-user YouTube refresh-token refs).
|
||
|
||
The **Dex identity is deliberately left intact.** Tapir does not deprovision, disable, or
|
||
modify the shared Dex directory **on deletion**. (Note per ADR-017: Tapir *does* now create Dex
|
||
Password CRs on invite — so the create and delete sides are deliberately asymmetric, and a
|
||
deleted user's Dex Password CR persists. See ADR-017 consequences.)
|
||
|
||
**Consequences.**
|
||
- **Clean re-registration:** a deleted user who logs in again arrives as a Dex-authenticated
|
||
subject with no `users` row, so they hit the registration gate as a "new" user — no special
|
||
resurrection path needed. This is a feature of the choice, not an accident.
|
||
- **Right-to-erasure is partial.** The user's *identity* still exists in Dex after deletion.
|
||
For Future B (trusted friends) this is acceptable: Dex is the maintainer's own directory and
|
||
the identity carries no Tapir content. **But if Tapir ever moves toward Future C (real
|
||
external/public users), this is a GDPR-shaped gap** — a true "delete my account" there must
|
||
also deprovision or anonymise the Dex identity, which is a new ADR and likely a Dex-admin
|
||
integration Tapir does not currently have. **ADR-017 widens this gap:** Tapir now *creates*
|
||
the Dex Password CR but does not delete it, so a deleted Tapir user leaves an orphaned Dex
|
||
local-password account. Tracked as a known gap in ADR-017.
|
||
- **Blast radius:** ADR-013 originally claimed "Tapir never holds write access to the shared
|
||
identity provider." **ADR-017 changes this** — Tapir now holds scoped `create`+`get` on
|
||
`passwords.dex.coreos.com` in the `auth` namespace. The blast radius is no longer zero; it is
|
||
bounded by that RBAC. See ADR-017 for the security analysis.
|
||
|
||
**Reversibility.** Adding Dex deprovisioning later is a superseding ADR; nothing about the
|
||
current choice blocks it. Recorded now because "deletion is partial by design" is a deliberate
|
||
semantic that future-Tapir (and any compliance review) must know was chosen, not overlooked.
|
||
|
||
---
|
||
|
||
## ADR-014 — Timedtext 429 handling: per-host backoff + honest in-flight UX, before any Whisper reconsideration
|
||
|
||
**Status:** Accepted (2026-06-03). **Partially implemented as of v0.6.0** — see the
|
||
implementation note at the end; the shared per-egress-IP rate gate (decision item 2) may not be
|
||
fully realised. Verify against `internal/runner` + the youtube adapter before treating as done.
|
||
**ADR-018 (in-process scheduled discovery + auto-summarize) makes decision item 2 load-bearing
|
||
and folds its confirm/finish into that build.**
|
||
|
||
**Context.** ADR-010 acquires captions from the unauthenticated `timedtext` baseUrl. Live runs
|
||
show that endpoint **rate-limits per source IP (HTTP 429) under volume** — many videos fetched
|
||
in one pass from one egress IP. Stage 1 (ADR-012) made this sharper in two ways: multiple users
|
||
now drive fetches from the *same cluster egress IP*, and the v0.4.0 "Summarize" button fires an
|
||
**immediate, synchronous-feeling** fetch on click (HTMX polls `/v/{videoId}/status`), so a 429
|
||
now surfaces as a *user-facing stall* rather than a background batch hiccup. A throttle
|
||
(`TAPIR_FETCH_DELAY`) exists but is a fixed inter-fetch delay, not 429-aware, and does not
|
||
coordinate across the concurrent click-path and the `tapir run` batch path.
|
||
|
||
This ADR is **not** a decision to build Whisper. ADR-007/010 keep STT deferred *pending
|
||
measurement of the sustainable caption rate* — and that rate cannot be measured while the
|
||
client reacts badly to the 429s it already provokes. Fix the backoff and the UX first; the
|
||
clean data then tells you whether Whisper is warranted.
|
||
|
||
**Decision.**
|
||
|
||
1. **429-aware backoff at the fetch layer.** On a 429 from the timedtext/InnerTube fetch,
|
||
respect `Retry-After` when present; otherwise exponential backoff with jitter. This replaces
|
||
reliance on a fixed `TAPIR_FETCH_DELAY` alone (which stays as a floor/politeness delay).
|
||
2. **A single per-egress-IP rate gate** shared by *both* the `tapir run` batch path and the
|
||
web click path, so they cannot collectively exceed the sustainable rate. Concurrency into
|
||
the timedtext endpoint is serialised/limited at this gate regardless of how many users or
|
||
goroutines are upstream. (The 429 is per *IP*, not per user — so the gate is process-/
|
||
cluster-egress-wide, not per-`withUser`.)
|
||
3. **Honest in-flight UX (the product-shaping part).** The status poll distinguishes states
|
||
the user can understand instead of a spinner that silently stalls:
|
||
- *summarizing* — actively processing (the existing tapir spinner).
|
||
- *queued / waiting for rate limit* — fetch deferred behind the rate gate; show a calm
|
||
"queued, this can take a few minutes when busy" state, not a stuck spinner.
|
||
- *no transcript* — terminal, per ADR-010's degrade-never-error (a 429 that exhausts retries
|
||
resolves to `SourceNone`, same as any unavailable caption — it must not present as a hard
|
||
error to the user).
|
||
The spinner promising imminence is the wrong signal under rate-limiting; the UX must be able
|
||
to say "waiting" truthfully.
|
||
4. **Measurement before Whisper.** Only once (1)-(3) are in and a real sustainable
|
||
per-IP rate is observed do we revisit whether caption coverage is good enough or whether the
|
||
deferred Whisper fallback (ADR-007) is finally warranted. That reconsideration is a future
|
||
ADR, gated on this data.
|
||
|
||
**Consequences.**
|
||
- Caption fetching becomes well-behaved under multi-user load instead of self-inflicting 429s;
|
||
the endpoint is treated as the shared, rate-limited resource it is.
|
||
- The click-path UX stays honest: "waiting" reads as waiting, failure degrades to "no
|
||
transcript", never a stuck spinner or error spew.
|
||
- A future per-IP cooldown / second egress IP / proxy becomes an option the rate gate can sit
|
||
in front of without UX changes.
|
||
- **Still no Whisper** — and now there's a clean path to the *data* that decides whether it's
|
||
ever needed (`docs/homelab-integration.md` and a future ADR own that measurement).
|
||
|
||
**Open (tracked, not in this ADR's scope):** the actual sustainable rate number; whether a
|
||
dedicated egress IP / outbound proxy is worth it; CronJob-driven `tapir run` interaction with
|
||
the rate gate (the batch path moves into k3s per the deferred CronJob item).
|
||
|
||
**Implementation note (2026-06-03, v0.6.0 — added during reconciliation).** A v0.6.0 release
|
||
shipped 429 handling: `domain.SourceRateLimited` (429 no longer collapsed to `SourceNone`),
|
||
youtube adapter maps 429 → `SourceRateLimited`, **migration 007 added `videos.transcript_status`
|
||
+ `videos.rate_limited_at`** (durable per-video retry state), `TAPIR_FETCH_BACKOFF` config,
|
||
runner records rate-limited + skips within the backoff window, and a "⏳ Retrying later" badge.
|
||
This realises decision items 1, 3, and 4 well. **Item 2 (a single shared per-egress-IP rate
|
||
gate) appears to be realised as per-*video* backoff state, NOT a process-wide IP gate** —
|
||
multiple videos can still each hit the endpoint and collectively trip the per-IP 429. Treat item
|
||
2 as **not yet confirmed done**; verify in `internal/runner`/youtube adapter and, if absent, it
|
||
remains open. (Resolved during reconciliation: the v0.6.0 report's reference to "migration 008
|
||
videos.rate_limited_at" was a **mislabel** — both columns shipped in migration **007**, not a
|
||
missing 008. No migration was lost; the sequence legitimately skips 008. `rate_limited_at`
|
||
exists and the runner's column reads are sound.) **ADR-018 makes item 2 a hard requirement:
|
||
in-process scheduled discovery + auto-summarize drives all users' fetches through one pod egress,
|
||
so the process-wide gate is confirmed/built as part of that slice.**
|
||
|
||
---
|
||
|
||
## ADR-015 — Per-user credentials: envelope-encrypted in PG18, not vault-stored
|
||
|
||
**Status:** Accepted (2026-06-03)
|
||
|
||
**Context.** The Stage-0/1 SecretStore (`internal/adapters/secrets/file.go`) holds per-user
|
||
YouTube OAuth refresh tokens as a flat key-value JSON map on a PVC — explicitly a stand-in for
|
||
"op/ESO later" (ADR-002, ADR-006). infra#86 proposed migrating it to an ESO-backed store. The
|
||
decision spike (infra#88) found that framing subtly wrong: **ESO syncs vault→cluster at
|
||
deploy/refresh time; it is not a runtime write API.** Per-user tokens are written *at runtime,
|
||
per end-user* (every YouTube connect; on token rotation) — they are application state, not
|
||
configuration. The homelab 1Password SA is also read-only, so a vault-write path would require
|
||
a new write-capable SA, widening Tapir's blast radius to shared estate infra to store what is
|
||
fundamentally Tapir's own row-data. Reading the actual SecretStore confirmed the shape: a
|
||
3-method port (`Get`/`Put`/`Delete`) over opaque refs, written interactively per user.
|
||
|
||
**Decision.** Per-user credentials are stored **envelope-encrypted in PG18**, not in the vault:
|
||
|
||
1. Tokens are encrypted with a **single app-level envelope key** and stored as ciphertext in
|
||
PG18, under the Row-Level Security already enforced and tested (ADR-012). Reads/writes go
|
||
through the existing `withUser` RLS-scoped seam.
|
||
2. The **envelope key** is the only secret in 1Password — fetched via the **existing read-only
|
||
SA** (confirmed working). No new write-capable SA; no per-user vault items.
|
||
3. The `ports.SecretStore` port is unchanged (`Get`/`Put`/`Delete`). The implementation swaps
|
||
`FileStore` (PVC JSON) for a `PGStore` (encrypted rows). Every consumer — connect,
|
||
disconnect, delete-account — is untouched (the port abstraction holds, ADR-003 spirit).
|
||
4. **Infra/operator credentials** (Dex client secret, MCP-auth tokens, service tokens) stay an
|
||
**ESO/1Password** concern. This ADR governs *per-user runtime* credentials only. The two
|
||
classes use two mechanisms deliberately — because they are two different things (runtime
|
||
app-state vs deploy-time config), not as a compromise. The "one mechanism" question
|
||
(maintainer's initial preference) was answered in #88 by correctly *classifying* the
|
||
secrets rather than unifying their storage.
|
||
|
||
**Consequences.**
|
||
- Runtime credential writes are normal RLS'd DB writes — no ESO sync latency, no indirection,
|
||
no write-SA blast radius. The interactive connect→store→use flow works without a vault
|
||
round-trip.
|
||
- Keeps PG18 and keeps ADR-002 intact (Supabase was considered and rejected again in #88 —
|
||
adding a datastore to hold a few encrypted strings PG18 already holds).
|
||
- Adds an encrypt/decrypt seam and an **envelope-key rotation** responsibility (re-encrypt the
|
||
per-user rows under a new key). infra#89 (build) must implement and test rotation, not assume
|
||
it — this is the real engineering cost of the choice.
|
||
- The vault's involvement shrinks to one static key via the SA already trusted for reads.
|
||
- **Supersedes** the "PVC stand-in for op/ESO" intent recorded in `secrets/file.go` and
|
||
`docs/homelab-integration.md` for the *per-user* secret path (the ESO/1Password reference in
|
||
ADR-006 stands for the *infra-cred* path).
|
||
|
||
**Reversibility / falsification (from infra#88).** Revisit if: per-user tokens need
|
||
high-frequency rotation writes (weak — PG18 handles it); an estate compliance policy requires
|
||
all credentials in 1P for a single audit surface (maintainer-knowable, not currently believed
|
||
to hold — would favour the vault-write path on policy grounds); or envelope-key rotation proves
|
||
operationally worse than per-secret vault rotation (the real cost #89 must prove). If none hold,
|
||
this stands. Full reasoning + rejected candidates (write-capable SA; Supabase): the infra#88
|
||
decision doc (`infra/docs/superpowers/handoffs/`). Build + reboot-validation: infra#89.
|
||
|
||
---
|
||
|
||
## ADR-016 — Stage 0 gate revised: "useful to me OR a friend", behavioural not feedback
|
||
|
||
**Status:** Accepted (2026-06-03). Revises the Stage 0 definition in VISION.md (supersedes the
|
||
original "useful to me, specifically" gate and folds in the old Stage 1 "a trusted user returns"
|
||
test). **The clock-start is further revised by ADR-018** (starts when scheduled discovery ships,
|
||
since unprompted use was not possible before then).
|
||
|
||
**Context.** The original Stage 0 gate was "the maintainer reads summaries weekly for four weeks
|
||
and acts on one." The maintainer chose to change it to include friendly users, reasoning that
|
||
early signal from friendly users is valuable. Two sub-decisions shaped the final form:
|
||
- *Me OR a friend* (not AND): either the maintainer or an onboarded friend showing use clears it.
|
||
- *Behavioural, not feedback*: the test is **return usage**, not stated approval.
|
||
|
||
**Decision.** Stage 0 passes when, over a 3–4 week window, **either the maintainer or at least
|
||
one onboarded friend returns to Tapir unprompted and reads/acts on summaries in ≥2 separate
|
||
weeks.** Friend feedback is gathered and valued but is **not** the gate.
|
||
|
||
**Why behavioural, not feedback (the load-bearing part).** Asked-for feedback from friendly
|
||
users is the least reliable signal in product development — politeness bias means a friend you
|
||
onboarded will tend to say encouraging things regardless of real value. The thing actually worth
|
||
knowing is whether they *come back on their own*. So the gate measures returns, not nice words.
|
||
This deliberately resists the most common way a principled gate dies: being declared "passed" on
|
||
the strength of a polite reaction.
|
||
|
||
**Honest note on what this change does.** This is a *guardrail edit made while the original gate
|
||
was unmet* (Stage 0 had barely started; build had run well ahead of use-evidence). That is
|
||
precisely the pattern that warrants scrutiny — redrawing a gate around work already done. It was
|
||
examined on that basis and proceeds because: (a) the new gate is **not softer in kind** — it
|
||
stays behavioural and sustained, merely broadening *who* can supply the signal; (b) friendly-user
|
||
signal is genuinely valuable; (c) the politeness-bias guard keeps it from collapsing into
|
||
"someone said it's nice." It is *not* a licence to treat the already-shipped Stage-1 machinery as
|
||
evidence the gate passed — use-evidence remains open.
|
||
|
||
**Consequences.**
|
||
- VISION.md Stage 0 rewritten; old Stage 1 ("a trusted user returns") folded in (it was
|
||
near-identical to the new test); hardening renumbered to Stage 1.
|
||
- New drift signal added: declaring the gate passed on polite feedback rather than return-usage.
|
||
- The 2026-07-01 check-in now asks "is anyone (me or a friend) coming back unprompted?", not
|
||
"am I using it weekly?". (Check-in date itself revised by ADR-018.)
|
||
|
||
**Reversibility.** A superseding ADR could tighten it back to maintainer-only or raise it to
|
||
require multiple returning users. Recorded with the full rationale (including the self-scrutiny
|
||
about editing a gate while it's unmet) so the reasoning survives, not just the new wording.
|
||
|
||
---
|
||
|
||
## ADR-017 — Invite flow: Tapir creates Dex local-password accounts (write access to the shared identity provider)
|
||
|
||
**Status:** ~~Accepted~~ **SUPERSEDED by [ADR-019](#adr-019--authentik-owns-invites-tapir-stops-provisioning-accounts) (2026-06-07).** The Dex local-password
|
||
invite provisioning was removed when the homelab IdP migrated Dex→Authentik
|
||
(infra ADR-0001); Authentik now owns invites. Original record below.
|
||
|
||
Accepted (2026-06-03), **recorded retroactively during reconciliation, then
|
||
deliberately ratified KEEP (2026-06-03).** This capability **shipped in v0.7.0 without an ADR** —
|
||
code, RBAC, and a deployed ServiceAccount landed before any decision record existed. This ADR
|
||
documents what shipped and honestly records that the decision-before-code discipline was not
|
||
followed here (see "Process note"). **Partially supersedes ADR-002 and ADR-013** (the "Tapir
|
||
holds no write access to the shared identity provider" posture).
|
||
|
||
**Keep-or-reverse, decided (2026-06-03).** After the retroactive recording, the maintainer
|
||
weighed keep vs. reverse (drop the RBAC + invite flow, rely on Google OIDC only). **Decision:
|
||
KEEP.** Deciding fact: not all intended Future-B users have / will use Google accounts, so Google
|
||
OIDC alone *cannot* onboard them — the invite flow is therefore **load-bearing, not a redundant
|
||
convenience**, and reversing it would leave some intended users with no onboarding path. The
|
||
trust-surface cost (scoped `create` on `passwords` in `auth`) is accepted deliberately in
|
||
exchange. The opposing argument (this contradicts ADR-015's "no write credential to shared infra"
|
||
logic, applied there to secrets) was considered and outweighed *only* because the capability is
|
||
genuinely necessary for real users here — it is **not** a precedent for widening the surface
|
||
further. **Explicitly NOT chosen:** adding `delete` to the RBAC to close the orphan gap (that
|
||
widens the surface in the wrong direction; accept the orphan at Future B instead — see open
|
||
items).
|
||
|
||
**Context.** Stage 1 onboarding needs a way for a friend to get a login. Two paths shipped:
|
||
Google OIDC via Dex (no Tapir code — Dex handles it), and an **invite flow** where Tapir itself
|
||
provisions a Dex **local-password** account. The invite flow is what this ADR is about.
|
||
|
||
**What shipped (reconstructed from `internal/adapters/dex/dex.go`, `internal/web/invite.go`,
|
||
migration `009_invitations`).**
|
||
1. `tapir invite <email>` (host CLI) writes an `invitations` row: a 32-byte crypto-random,
|
||
single-use, 7-day-expiry `token` (the token is the capability), `email`, `used_at`.
|
||
The `invitations` table is deliberately **not** RLS/`user_id`-scoped — the invitee has no
|
||
user yet and the token itself is the secret. (Sound; documented in the migration.)
|
||
2. Recipient visits `/invite/{token}` (public, no session), sets a password (validated *before*
|
||
the token is consumed, so a typo doesn't burn it), the token is claimed exactly once.
|
||
3. Tapir bcrypt-hashes (cost 12) and **POSTs a `passwords.dex.coreos.com` Custom Resource into
|
||
the `auth` namespace** via the in-cluster Kubernetes API, authenticating as the `tapir`
|
||
ServiceAccount. TLS validated against the mounted cluster CA. Off-cluster (dev) it returns
|
||
`ErrNotInCluster` and degrades without consuming the invite.
|
||
4. RBAC applied this deploy: the `tapir` ServiceAccount has **`create`+`get` on
|
||
`passwords.dex.coreos.com` in the `auth` namespace** (and only that).
|
||
5. Dex (kubernetes storage) then serves local-password login for that email; Tapir's
|
||
registration gate creates the profile on first login.
|
||
|
||
**Decision (as ratified now).** Accept the invite flow as built: Tapir *may* hold scoped write
|
||
access to Dex's `passwords` resource in the `auth` namespace, for the purpose of invite-based
|
||
local-account creation. This is a deliberate, bounded reversal of the prior "no write access to
|
||
the shared identity provider" posture (ADR-002/013).
|
||
|
||
**Why this is acceptable (the case for keeping it).**
|
||
- The RBAC is **minimally scoped**: `create`+`get` on one resource type in one namespace, not
|
||
broad Dex/cluster write. Blast radius is bounded and inspectable.
|
||
- It enables friend onboarding **without Google OAuth app verification** (ADR-008 deferred that),
|
||
which is genuinely useful for Future B — and necessary for users who won't use Google (the
|
||
deciding fact in the keep decision above).
|
||
- The code is careful: validate-before-consume, single-use tokens, graceful off-cluster degrade,
|
||
sentinel errors mapped to clear messages, the base64-bcrypt storage gotcha handled.
|
||
|
||
**Consequences / known gaps (the case to watch).**
|
||
- **Reverses a load-bearing principle.** ADR-002 and ADR-013 leaned on "Tapir never writes to the
|
||
shared identity provider" for blast-radius minimisation. That is no longer true. Anyone reading
|
||
those ADRs without this one would be misled — hence the cross-references added to both.
|
||
- **Create/delete asymmetry → orphaned Dex accounts.** Tapir now *creates* Dex Password CRs but
|
||
(per ADR-013) does not *delete* them on account deletion. A deleted Tapir user leaves an
|
||
orphaned Dex local-password account that can still authenticate (though it would hit the
|
||
registration gate with no profile). This widens the ADR-013 right-to-erasure gap. **Open —
|
||
accepted for Future B, revisit before Future C (do NOT add `delete` RBAC just to fix this).**
|
||
- **`create` on a shared-namespace identity resource** is a meaningfully larger trust surface than
|
||
the rest of Tapir. If the `tapir` pod is compromised, the attacker can mint Dex local accounts.
|
||
Bounded by the namespace/resource scope, but real — worth a deliberate look before Future C.
|
||
- Token custody for invites is in PG (`invitations.token`) in plaintext; single-use + 7-day TTL
|
||
bound the exposure, but a DB read yields live invite tokens until claimed/expired.
|
||
|
||
**Process note (why this ADR is retroactive).** This capability was built and deployed by
|
||
parallel agent sessions while the main planning thread was elsewhere, and shipped with no ADR —
|
||
the first time in this project a guardrail-reversing change skipped the decision-before-code
|
||
discipline. Recorded here not to rubber-stamp it but to restore the audit trail: the decision is
|
||
now visible, its principle-reversal is named, and its open gaps (orphaned accounts, the larger
|
||
trust surface) are tracked rather than buried in a release note. The maintainer ratified KEEP
|
||
after the fact (see status block) on the deciding fact that some intended users cannot use Google
|
||
OIDC.
|
||
|
||
**Open items this ADR creates (tracked in infra):**
|
||
- Confirm the RBAC really is `create`+`get` only (not broader) against the deployed manifest in
|
||
`infra` — a security claim currently resting on a sibling report, not a verified manifest read.
|
||
- Accept the orphaned-Dex-account gap for Future B; revisit (orphan cleanup + the whole Dex-write
|
||
surface) before any Future C move. Do **not** add `delete` RBAC solely to fix the orphan.
|
||
- Review the larger trust surface before any Future C move.
|
||
|
||
---
|
||
|
||
## ADR-018 — Make Tapir usable unprompted: in-process scheduled discovery, auto-summarize default, gate-clock reset
|
||
|
||
**Status:** Accepted (2026-06-03)
|
||
|
||
**Context.** The Stage-0 gate (ADR-016) measures whether the maintainer or a friend *returns and
|
||
reads/acts* over weeks. But the system could not actually be used that way: discovery (`tapir
|
||
run`) was **host-side manual**, so a newly onboarded user saw an empty list and had no reason to
|
||
return. The gate was structurally unmeetable — not because the product failed, but because the
|
||
*workflow* depended on the maintainer SSHing in to trigger each pass. The runner already has a
|
||
`Loop(ctx, interval)` (run-on-start, then every tick) and per-user mode/dedup/backoff; what was
|
||
missing was *invocation* — nothing called it for the deployed users on a schedule.
|
||
|
||
**Decision.**
|
||
1. **In-process scheduled discovery.** `tapir serve` launches a background goroutine that runs
|
||
discovery for **all users** on an interval (`TAPIR_DISCOVERY_INTERVAL`; unset/0 = off). It
|
||
enumerates users (un-RLS'd `user_identities`) and runs each user's pass inside `withUser`,
|
||
reusing the existing `runner` — not a new scheduler. Stateless timing; ctx-cancellable;
|
||
per-user failures isolated.
|
||
2. **Auto-summarize default ON** for Future-B users (new registrations default `true`; existing
|
||
rows updated), so discovery both populates *and* summarizes — the list fills itself. The
|
||
per-user manual toggle remains.
|
||
3. **Gate-clock reset.** The Stage-0 3–4 week window (ADR-016) **starts when this ships**, because
|
||
unprompted use was impossible before it. This is starting the clock when the experiment can
|
||
actually run, **not** a reset to dodge a failing gate (the prior window measured nothing —
|
||
there was no way to use the system unprompted). The 2026-07-01 check-in moves accordingly to
|
||
~3–4 weeks after this deploys.
|
||
|
||
**Why in-process and not a k8s CronJob.** Maintainer's call: simpler deploy (no second deployable),
|
||
acceptable at Future-B scale (1–3 users). The CronJob's advantage — failure isolation between
|
||
discovery and the web/read path — was weighed and traded away knowingly.
|
||
|
||
**Consequences / constraints.**
|
||
- **Discovery shares the web process's lifetime and egress.** A wedged discovery pass can degrade
|
||
the reading UI (the coupling a CronJob would have avoided). Accepted at this scale.
|
||
- **SINGLE-REPLICA ASSUMPTION (load-bearing).** If `tapir serve` ever runs >1 replica, *every*
|
||
replica runs the discovery loop → every user fetched in parallel (429s + duplicate work). Tapir
|
||
must stay single-replica while in-process scheduling is enabled, or this is revisited (move to
|
||
CronJob, or add leader-election). Recorded so a future scale-up doesn't silently double-run.
|
||
- **Makes ADR-014 item 2 (shared per-egress-IP rate gate) load-bearing.** In-process + auto +
|
||
multi-user drives all caption fetches through one pod egress, concurrent with click-path
|
||
Summarize. The build confirms/finishes the process-wide rate gate in the same slice; without it
|
||
the system self-inflicts 429s every cycle. **Auto-summarize ON is gated on the rate gate
|
||
existing** (fallback: ship discovery with auto OFF until it does).
|
||
|
||
**Reversibility.** Disable via `TAPIR_DISCOVERY_INTERVAL=0` (reverts to manual `tapir run`).
|
||
Moving to a CronJob later is a superseding ADR; the per-user `runner` is unchanged either way.
|
||
Spec: `docs/specs/scheduled-discovery.md`.
|
||
|
||
---
|
||
|
||
## ADR-019 — Authentik owns invites; Tapir stops provisioning accounts
|
||
|
||
**Status:** Accepted (2026-06-07). **Supersedes ADR-017** (Dex local-password invite
|
||
provisioning).
|
||
|
||
**Context:** infra ADR-0001 migrated the homelab IdP Dex→Authentik. Authentik provides
|
||
first-class invite flows; the Dex local-password connector never consulted the Password
|
||
CRs Tapir wrote (the defect that triggered the migration). Tapir-web's OIDC issuer now
|
||
points at Authentik (infra ADR-0001 step 3).
|
||
|
||
**Decision:** Tapir no longer provisions accounts. The Dex-password invite path is removed:
|
||
`internal/adapters/dex`, the public `/invite/{token}` set-password UI (`internal/web/invite.go`),
|
||
the `tapir invite` CLI (`cmd/tapir/invite.go`), the `InvitationStore`/`DexPasswordCreator`
|
||
ports + `App.Invitations`/`App.Dex` wiring, the invite Templ pages, and the
|
||
`tapir invite` Taskfile target. New users are invited via Authentik's flow, log into Tapir
|
||
via OIDC, and are captured by Tapir's existing provider-agnostic `/register` (display name).
|
||
Login + Google moved by config only (Authentik per-app issuer); the OIDC adapter is unchanged.
|
||
|
||
**Consequences:** smaller Tapir blast surface — no writes to the shared identity provider, no
|
||
configmap/CR access, the dedicated `passwords.dex.coreos.com` RBAC + ServiceAccount are
|
||
removed (infra side, coupled change). The `invitations` table (migration 009) is left in
|
||
place — migrations are append-only and the unused table is harmless; a future migration may
|
||
drop it. The `*_DEX_*` config/identity names (`TAPIR_DEX_CLIENT_*`, `dex_subject`, the
|
||
`DexAuth`/`oidc` package) are now misnomers; renaming is deferred (cosmetic, not behavioural).
|
||
|
||
**Rejected:** keeping Tapir's `/invite` UI but calling Authentik's API on claim — couples
|
||
Tapir to Authentik's admin API + a token for no real gain; Authentik's own invite flow is
|
||
the supported path.
|
||
|
||
---
|
||
|
||
## ADR-020 — Recency-bounded auto-summarize + honest sparse-state surface
|
||
|
||
**Status:** Accepted (2026-06-08). **Refines ADR-018** (auto-summarize) and **ADR-014**
|
||
(per-IP caption rate gate).
|
||
|
||
**Context.** Tapir is operational but sparse: at real subscription volume the maintainer's
|
||
account holds ~283 discovered videos, ~15 summarized, ~256 behind the respected per-IP caption
|
||
rate gate, ~12 no-captions. Two problems follow. (1) **Load:** ADR-018 auto-summarizes *every*
|
||
unseen video, so a large back-catalogue re-drives the whole queue through the gate every cycle —
|
||
self-inflicted 429s with no user value (nobody is waiting on a 6-month-old video). (2) **First
|
||
contact:** a new user sees a mostly-empty feed with no moving parts and a UI that implied
|
||
abundance/imminence ("fetching soon" ×256, "Run `tapir run`", "Summarize now"); the Stage-0 gate
|
||
is *return usage*, and the experience died at the first visit. Source: a UX heuristic review
|
||
(`docs/ux-review/UX-REVIEW-stage0-recency.md`).
|
||
|
||
**Decision.**
|
||
1. **Recency bound on auto-summarize.** In automatic mode the scheduler only summarizes videos
|
||
published within `TAPIR_AUTO_SUMMARIZE_WINDOW` (default ~7d). Older videos are still discovered
|
||
and listed but not auto-processed — they keep the manual "Summarize" affordance. An explicit
|
||
manual request bypasses the bound. `0` disables it (pre-recency behaviour). This bounds auto
|
||
*load*; it does not fetch harder — the gate (ADR-014) is untouched and the manual path still
|
||
serialises through it.
|
||
2. **Honest sparse-state surface.** Copy is reframed to surface scarcity truthfully, never to
|
||
look fuller: "N ready · M in queue · K no captions" (not "fetching soon"); a one-line "captions
|
||
are fetched slowly on purpose" note; "Summarize" (not "Summarize now"); the empty-connected
|
||
state stops printing an impossible CLI command.
|
||
3. **Feed IA = one list, noise-collapsed.** Summarized + recent un-summarized cards lead inline;
|
||
the older un-summarized back-catalogue collapses behind a single "Show N older videos"
|
||
disclosure; caption-less videos collapse to a one-line count instead of N dead cards. List is
|
||
ordered summarized-first, then `published_at DESC NULLS LAST`.
|
||
|
||
**Consequences.** The auto path's per-cycle fetch volume is bounded by recent uploads, not the
|
||
whole back-catalogue, so steady-state 429 pressure drops sharply. Older videos become explicitly
|
||
on-demand — a deliberate honesty trade (the user chooses to spend a scarce fetch on old content).
|
||
The single-replica assumption (ADR-018) is unchanged.
|
||
|
||
**Deliberately NOT done (premature until the Stage-0 loop is validated).** Return-nudges
|
||
(digest email / push) — a nudge contaminates the *unprompted*-return signal the gate measures
|
||
(ADR-016); building it now poisons the experiment. Also deferred: full-text search, channel
|
||
facets, read/unread, saved views — all need summary abundance to matter.
|
||
|
||
**Reversibility.** `TAPIR_AUTO_SUMMARIZE_WINDOW=0` restores summarize-every-unseen; the feed
|
||
collapse keys off the same window (`App.RecencyWindow=0` → everything inline).
|
||
|
||
---
|
||
|
||
## ADR-021 — Persist transcripts as shared, video-keyed public content (re-analysis never re-fetches)
|
||
|
||
**Status:** Accepted (2026-06-09). **Reopens the transcripts half of** the "Global cross-tenant
|
||
`videos`/`transcripts` table" rejection (data-model.md). **Builds on ADR-007** (captions-first),
|
||
**ADR-010/ADR-014** (the per-IP caption rate gate), and **ADR-012** (per-user RLS isolation).
|
||
|
||
**Context.** Every summarization fetches the transcript fresh through the caption path, even when
|
||
the exact same transcript was fetched moments ago — for the same user re-summarizing, or for a
|
||
second user who happens to watch the same video. The caption fetch is the one genuinely scarce,
|
||
genuinely risky operation in the system: YouTube's timedtext endpoint is unofficial and per-IP
|
||
rate-limited (ADR-010), and tripping it risks the maintainer's Google standing (ADR-014). So the
|
||
operation we most want to *avoid repeating* is the one we currently repeat unconditionally. A
|
||
transcript is **public content** — the same words YouTube serves to anyone — and carries nothing
|
||
user-identifying. The per-user isolation that protects summaries, feeds, and tokens (ADR-012) is
|
||
the wrong shape for it: it forces a re-fetch per user for data that is identical across users.
|
||
|
||
The original rejection ("Global cross-tenant `videos`/`transcripts` table") bundled videos and
|
||
transcripts together and rejected both on the grounds that "at 1–5 users, re-summarizing is
|
||
cheaper than the coupling." That reasoning holds for **videos** (per-user feed rows, genuinely
|
||
user-scoped) but not for **transcripts**: the cost being avoided is not LLM re-summarization, it
|
||
is a *rate-gated, reputation-risky network fetch*, and that cost is paid per re-fetch regardless
|
||
of user count. One re-fetch avoided is strictly worth more than the coupling it removes.
|
||
|
||
**Decision.**
|
||
1. **A single shared `transcripts` table, keyed by the cross-user dedup key
|
||
`(provider, provider_video_id)`** — the stable public identity of the video, not Tapir's
|
||
internal per-user `videos.id`. Columns: the key, `source` (`captions`/`none`), `language`,
|
||
`content`, `fetched_at`. It holds **only public caption content + the video's public id** —
|
||
nothing user-identifying — and is therefore **NOT RLS-scoped**: no `user_id`, no policy, no
|
||
`FORCE ROW LEVEL SECURITY`. This is the deliberate, single exception to the ADR-012 isolation
|
||
boundary, and the only one.
|
||
2. **Summarize path becomes read-stored-first.** Have a stored transcript for this video? →
|
||
summarize from the stored text, **no caption fetch**. No stored transcript? → fetch *through
|
||
the unchanged gate* (ADR-014) → store it → summarize. The gate is neither bypassed nor
|
||
weakened; persistence reduces how *often* we reach it, never how *fast*.
|
||
3. **De-facto cross-user dedup is the intended behaviour, not a feature with a switch.** Two
|
||
users who share a video share the one transcript row. A permanent `source = 'none'` (no
|
||
captions) is stored too, so a known-caption-less video is not re-fetched by anyone. A
|
||
transient 429 (`SourceRateLimited`) is **never** stored as terminal — it stays a per-user
|
||
retry via the existing `transcript_status` backoff (ADR-014), so persistence cannot mask a
|
||
rate-limit into a false "no transcript."
|
||
4. **Per-user `summaries` stay RLS-scoped (ADR-012 unchanged)** and reference the transcript by
|
||
video id. Videos stay per-user. Only transcripts go shared.
|
||
|
||
**Consequences.** Re-analysis (re-summarize, different model, paste of an already-seen video,
|
||
onboarding of a second user with overlapping subscriptions) never re-touches YouTube — the
|
||
primary win, and it *reduces* aggregate caption-gate pressure, reinforcing ADR-010/ADR-014 rather
|
||
than straining them. The isolation surface gains exactly one non-RLS table; an isolation test
|
||
asserts the boundary is *exactly* there and has not leaked to any user-owned table (this is the
|
||
proof the public-content classification was implemented as designed). It also unblocks
|
||
multi-model / customizable analysis (re-run analysis on stored text for free) — enabling that is
|
||
this ADR's point; building it is separate.
|
||
|
||
**Reversibility.** The read-stored-first check is the only behavioural coupling; removing it
|
||
restores fetch-every-time. The down-migration recreates the per-user RLS-scoped transcripts shape
|
||
(001/003). No user-facing surface depends on cross-user sharing — sharing is the *storage shape*,
|
||
never exposed in the UI.
|
||
|
||
---
|
||
|
||
## ADR-022 — Summarizer is a resilient endpoint chain, not a single model
|
||
|
||
**Status:** Accepted (2026-06-10). **Extends ADR-004** (the copied `llm` Primary→Fallback
|
||
routing). Triggered by the first friendly-pilot live run, where a connected user got **zero**
|
||
summaries after 12h.
|
||
|
||
**Context.** Stage-0 ran a single summarizer model (`koala/phi4-mini`) with no fallback wired
|
||
(`summarizer.New(primary, nil)`). The live run exposed three independent failure modes, each of
|
||
which silently produced no summary:
|
||
|
||
1. **Context overflow.** `phi4-mini` has an 8k context. Real transcripts (one was 11,602 tokens)
|
||
exceed it and the gateway returns HTTP 400 — and the request also sent `max_tokens=8192`, so
|
||
even a short transcript plus the completion budget could overflow the window.
|
||
2. **Malformed model output.** `phi4-mini` intermittently emits `highlights` as a bare string
|
||
instead of an array, producing `cannot unmarshal string into []string`. The old code returned
|
||
the parse error **without** trying any other model — a 200-with-bad-JSON short-circuited.
|
||
3. **No fallback existed at all** — any primary failure was terminal for that video.
|
||
|
||
`phi4-mini` is kept as primary deliberately: it is fast and, on transcripts that fit, correct.
|
||
The fix is resilience around it, not replacing it.
|
||
|
||
**Decision.**
|
||
1. **Ordered endpoint chain (`summarizer.NewChain`).** Endpoints are tried in order; the first to
|
||
return a *parseable* summary wins. Default chain:
|
||
`koala/phi4-mini` (primary, local) → `iguana/gemma4-26b` (fallback, local on a
|
||
*different host*) → `berget/mistral-small` (worst-case, external). All three are reached through
|
||
the **one** LiteLLM gateway by alias — the gateway already fronts both llama-swap and berget — so
|
||
a fallback is a different alias, not a second client config.
|
||
|
||
**Update 2026-06-11:** the local fallback moved from `koala/phi4-14b` to `iguana/gemma4-26b`.
|
||
koala now carries other GPU loads, so keeping the fallback on koala competed with them; iguana
|
||
(M2 Ultra) has the headroom, and a different host is also a different egress IP for the rare
|
||
fallback fetch. `gemma4-26b` is the brain-validated homelab general-purpose model (agentsquad
|
||
H2/H3 executor) and returned valid summary JSON on the real prompt in a smoke test
|
||
(~37s incl. cold-load — fine for a path hit only when the fast primary fails). Pure config:
|
||
`TAPIR_FALLBACK_MODEL`.
|
||
2. **A parse failure advances the chain, same as a transport error.** "Reliably summarized" means
|
||
*parseable summary returned*, not *HTTP 200*. This is the behaviour the old Primary→Fallback
|
||
shape missed.
|
||
3. **Tolerant parse.** `highlights`/`takeaways` coerce from a bare string (or a mixed scalar
|
||
array) to `[]string`, so the most common small-model quirk is absorbed **without** spending a
|
||
fallback round-trip — keeping the fast path fast.
|
||
4. **Transcript truncation (`TAPIR_MAX_TRANSCRIPT_CHARS`, default 18000).** Input is bounded
|
||
up-front to fit a small-context primary, so overflow is prevented rather than recovered-from.
|
||
5. **Bounded completion budget (`TAPIR_SUMMARY_MAX_TOKENS`, default 1500).** A summary needs few
|
||
hundred tokens; the old 8192 budget itself contributed to 8k-window overflow.
|
||
|
||
**Local-first guarantee preserved.** The chain ordering *is* the guarantee: locals are tried
|
||
first, so content reaches the external endpoint only after every local endpoint has failed.
|
||
`TAPIR_CLOUD_FALLBACK_MODEL=""` removes the external endpoint entirely — the lever a
|
||
**client/NDA deployment** pulls so content never leaves the local stack. With no external endpoint
|
||
configured the `ai_routing.feature` "content only local" scenarios hold unchanged.
|
||
|
||
**Reversibility.** Pure wiring + config. Setting `TAPIR_FALLBACK_MODEL` and
|
||
`TAPIR_CLOUD_FALLBACK_MODEL` empty collapses the chain back to single-primary behaviour; the
|
||
tolerant parse and truncation are strict supersets of the old behaviour (a previously-parseable
|
||
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.
|
||
|
||
---
|
||
|
||
## ADR-024 — Per-channel caption-availability memory
|
||
|
||
**Status:** Accepted (2026-06-10). **Builds on ADR-014** (per-IP caption budget), **ADR-021**
|
||
(shared transcript cache), **ADR-023** (Shorts filter).
|
||
|
||
**Context.** After ADR-021 caches transcripts and ADR-023 drops Shorts, the remaining caption
|
||
waste is the *first* fetch on every new video of a channel that never publishes English captions
|
||
(foreign-language news, music, etc.). Each costs one rate-limited fetch to resolve to "none" —
|
||
and on a throttled IP that fetch may 429 and churn the backoff machinery before it ever gets a
|
||
verdict. A pilot user's feed had several such channels.
|
||
|
||
**Decision.** Remember, per `(user, channel)`, a streak of consecutive no-caption outcomes
|
||
(`channel_caption_state`, migration 016, RLS-scoped like the rest of the user-owned schema).
|
||
Once the streak reaches `TAPIR_CHANNEL_CAPTIONLESS_THRESHOLD` (default 5) the channel is
|
||
suppressed — its videos are discovered/listed but not caption-fetched — for
|
||
`TAPIR_CHANNEL_CAPTIONLESS_WINDOW` (default 14d), after which one video is re-probed
|
||
(auto-recovery for a channel that starts adding captions). A successful fetch resets the streak;
|
||
a fresh 429 does NOT count (transient, not a caption verdict). An explicit manual request
|
||
bypasses suppression. `threshold = 0` disables the feature.
|
||
|
||
**Why per-user, not global.** Caption availability is really a channel property (public), so a
|
||
global table would let users share the learning. But subscriptions are per-user (ADR-012) and at
|
||
pilot scale users' channel sets barely overlap, so per-user + RLS keeps it consistent with the
|
||
existing isolation model with no new non-RLS exception to justify. Promoting to a shared table
|
||
(like transcripts, ADR-021) is a future optimisation if channel overlap grows.
|
||
|
||
**Reversibility.** Migration 016 is a clean drop; `threshold = 0` disables at runtime. The
|
||
memory only ever *suppresses fetches* — it never deletes content or affects already-stored
|
||
summaries.
|
||
|
||
---
|
||
|
||
## ADR-025 — Honest, state-aware foreground summarization status
|
||
|
||
**Status:** Accepted (2026-06-10). **Pillar B of the manual-mode UX work** (Pillar A, foreground
|
||
fetch priority, is a separate follow-up). Builds on ADR-014 (the rate limit the UX must make
|
||
legible).
|
||
|
||
**Context.** Clicking "Summarize" spawned a background goroutine and polled `/status`, which
|
||
returned only two states: the spinner (in-flight) or the normal card (done). But the web
|
||
`ProcessVideo` only recorded an outcome on *success* — a 429'd or caption-less click left
|
||
`transcript_status` unset, so the next poll silently reverted to the "Summarize" button. The
|
||
user saw either an endless spinner or a button that did nothing useful when clicked again. The
|
||
binding constraint (YouTube's caption rate limit) was completely invisible.
|
||
|
||
**Decision.**
|
||
1. **Record every outcome on the web path**, mirroring the runner: `ProcessVideo` stamps
|
||
`rate_limited` / `none` / `fetched`. A rate-limited video keeps its requested flag so the
|
||
background sweep retries it; `none` and `fetched` are terminal.
|
||
2. **`/status` is state-aware**: summarized → summary card; in-flight → working spinner;
|
||
`rate_limited` → a calm "waiting, will retry" card that keeps polling (every 30s) so the
|
||
summary appears on its own when the retry lands — the user never clicks again;
|
||
`none` → a terminal "no captions" card with no poll and no dead-end button.
|
||
3. **Charm status text** (Claude-Code / Crush inspired): the working spinner cycles playful,
|
||
tapir-themed gerunds ("Chewing the cud…", "Munching leaves…", "Distilling the gist…") via
|
||
CSS only — no JS, keeping the HTMX/no-JS ethos. Decorative (aria-hidden) with a stable
|
||
`role=status` line for assistive tech.
|
||
|
||
**Principle.** When the system cannot be fast (throttled IP), it is at least honest, and it
|
||
self-resolves without making the user retry. Honesty is the load-bearing half — Pillar A's
|
||
priority lane only improves the odds of a fast slot; it cannot beat an already-hot IP.
|
||
|
||
**Reversibility.** Pure transport-layer + view change over the unchanged engine/ports. No
|
||
schema change (reuses `transcript_status` from migration 007).
|
||
|
||
---
|
||
|
||
## ADR-026 — Foreground caption fetches take priority; the credentials probe is dead
|
||
|
||
**Status:** Accepted (2026-06-10). **Pillar A of the manual-mode UX work** (Pillar B was
|
||
ADR-025). Builds on ADR-014 (the shared per-IP gate).
|
||
|
||
**Context.** Every caption fetch — the background sweep and the web click-path — shared one
|
||
process-wide rate gate equally. So a user waiting on a "Summarize" click competed with the
|
||
firehose for both pacing and the scarce pre-429 window; on a busy IP the click was slow or
|
||
429'd while the background churned.
|
||
|
||
**Decision.** A context-marked priority lane. The web path
|
||
(`engineProcessor.ProcessVideo`) wraps its context with `ForegroundContext`; the gate gives
|
||
foreground fetches a token immediately, while **background fetches yield** — they wait until no
|
||
foreground fetch is pending before taking a token. Threaded via a context value (not new
|
||
signatures) and a process-wide `foregroundPending` counter. Clicks are rare and bursty, so the
|
||
background barely loses throughput; the waiting human gets the next (and cleanest) slot.
|
||
|
||
**Credentials probe — rejected, not built.** The idea was to fetch captions with the user's
|
||
auth in manual mode to dodge 429s. It is a dead end, already settled by ADR-010 and the code:
|
||
the caption path is *deliberately anonymous* because the InnerTube/timedtext endpoints **reject
|
||
or break on authenticated requests** (`captions.go`: "no OAuth token — it can break the
|
||
timedtext endpoint"). The user's OAuth (a Data API credential) does not authenticate InnerTube
|
||
at all, and the official `captions.download` is owner-only (403 on third-party). So auth cannot
|
||
help here and can actively hurt. No probe needed — building one would only re-confirm the ADR.
|
||
|
||
**Reversibility.** Context-marker + a yield loop in the gate; removing the marker collapses to
|
||
the prior equal-share behaviour. No schema or API change.
|
||
|
||
---
|
||
|
||
## ADR-027 — Chat with a video's stored transcript (deeper-dive, on an already-summarized video)
|
||
|
||
**Status:** Accepted (2026-06-11). **Consumes ADR-021** (the shared, video-keyed transcript
|
||
store) for the first time beyond summarization; **uses the ADR-022 chain models**; relates to
|
||
ADR-012 (isolation) and ADR-016 (the Stage-0 gate).
|
||
|
||
**Context — observed demand, not hypothetical.** The maintainer read 10+ real pilot summaries
|
||
and reported the reactions: *many good; some he wanted to dig deeper into; some less useful*
|
||
(the "less useful" split between weak-*model* output and uninteresting-*video* content). The
|
||
middle reaction is the signal: a good summary that makes the reader want *more* is the summary
|
||
succeeding at triage and then hitting a wall — there is nowhere to go deeper short of watching
|
||
the video. That want is the feature. It is also the cheapest possible feature to satisfy
|
||
honestly, because ADR-021 already persists the transcript: the deeper-dive runs entirely on
|
||
stored public-content text + local models, touching **no** caption fetch and **no** YouTube.
|
||
|
||
**Decision.** Add a per-video chat that lets the user ask questions against a video's stored
|
||
transcript.
|
||
1. **Entry from the summary view only.** A "dig deeper / ask" affordance on a summarized video —
|
||
the chat lives exactly where the "I want more" reaction happens. No standalone chat surface.
|
||
2. **Stored-transcript-only (load-bearing constraint).** Chat is available **only** for videos
|
||
that already have a stored transcript. It never triggers a caption fetch, so it cannot touch
|
||
the rate gate, the 429 surface, or YouTube at all — the entire account-safety constraint that
|
||
governs the rest of Tapir is satisfied *by construction* here, not by careful gating. (Entry
|
||
being "from a summary" guarantees the transcript exists.) On-demand fetch for un-stored videos
|
||
is explicitly deferred.
|
||
3. **Model = the summary's model by default; user-switchable among the ADR-022 chain models**
|
||
(`phi4-mini` / `gemma4-26b` / `mistral-small` to start). This is deliberate: it doubles as
|
||
live model-comparison instrumentation — ask the same question of the same transcript under two
|
||
models and the difference is directly felt. This is the mechanism by which the maintainer
|
||
learns *which* model is worth defaulting to, and it is the multi-model-analysis direction
|
||
ADR-021 anticipated, arriving as a user-facing capability.
|
||
- Chat is a **read-bounded retrieval/QA task** (the user supplies the focus), which is
|
||
*easier* than summarization (the model must decide what matters). So a model that summarizes
|
||
mediocrely may chat well — chat is plausibly a partial remedy for the weak-summary case, not
|
||
an inheritor of it.
|
||
4. **Ephemeral chat (v1).** No persisted history; chat is per-session. Persisting per-user,
|
||
RLS-scoped history is deferred until there is evidence anyone wants to revisit a conversation.
|
||
5. **Chat-only, trust-the-model (v1) — with a recorded limitation.** The chat does not expose the
|
||
raw transcript for verification in v1 (kept simple). **Known limitation:** because some
|
||
summaries were weak-model output, the user has reason not to fully trust a chat answer's
|
||
fidelity to the transcript, and v1 gives no in-UI way to check. The model-switcher partially
|
||
compensates (two models disagreeing on the same question is itself a signal). A
|
||
"show source / view transcript" verification path is the natural **v2** and is *not*
|
||
foreclosed — ADR-021's stored transcript already makes it cheap. Recorded so v2 is a known
|
||
next step, not a rediscovery.
|
||
|
||
**Why this is safe and in-scope.** It adds no caption-fetch surface (stored-only), no new
|
||
non-RLS table (transcripts already shared per ADR-021; ephemeral chat stores nothing), and no
|
||
auth change. It is additive to the read path. The one genuine product expansion — Tapir becomes
|
||
an interactive transcript-QA tool, not only a summarizer — is justified by *observed* demand from
|
||
real reading, which is exactly the kind of evidence the Stage-0 discipline asks for before
|
||
building.
|
||
|
||
**Relation to the Stage-0 gate.** This is **not** a return-nudge (those stay deferred, ADR-020) —
|
||
it adds nothing that prompts the user to return; it deepens the value *once they are already
|
||
reading*. It does not contaminate the unprompted-return signal. If anything it strengthens the
|
||
"useful to me" case the gate measures, by giving a good summary somewhere to lead.
|
||
|
||
**Reversibility.** Additive read-path feature over the unchanged engine + the ADR-021 store.
|
||
Removing the summary-view affordance removes the feature; nothing else depends on it. Ephemeral =
|
||
no migration, no stored state to unwind. Spec: `docs/specs/chat-with-transcript.md`.
|
||
|
||
---
|
||
|
||
## 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`.
|
||
|
||
---
|
||
|
||
## ADR-029 — Stateless session cookie (survives restarts, browser-close, idle)
|
||
|
||
**Status:** Accepted (2026-06-11). Triggered by pilot feedback: "lots of clicking to log in again
|
||
on iPhone." Supersedes the in-memory session store in the ADR-011 login.
|
||
|
||
**Context.** Three compounding causes made users re-login constantly:
|
||
1. **In-memory session store** (`sessionStore` map) — wiped on every pod restart, so each deploy
|
||
logged everyone out. During the active build period that was ~15 logouts.
|
||
2. **1-hour session TTL** — for a "check back tomorrow" reader, idle > 1h forced a re-login on
|
||
nearly every visit.
|
||
3. **No cookie Max-Age** — a session cookie (deleted on browser/app close); iPhone Safari closing
|
||
the tab dropped it.
|
||
Each re-login is the full Dex/Authentik redirect dance — many taps on mobile.
|
||
|
||
**Decision.** Make the session **stateless**: the identity (subject + email) and an absolute
|
||
expiry live INSIDE the existing HMAC-signed (HS256) cookie — no server-side table. Plus:
|
||
- **30-day sliding TTL** (was 1h), re-signed on each request so an active user never lapses.
|
||
- **Persistent cookie** (`Max-Age` set) so it survives browser/app close.
|
||
The cookie is HttpOnly + Secure + SameSite=Lax; the HMAC (keyed by the stable ESO
|
||
`tapir-session-secret`, which does NOT rotate per deploy) makes it tamper-proof. The payload is
|
||
identity, not secrets — the OIDC access/ID tokens are still discarded after callback.
|
||
|
||
**Consequences.** A deploy/restart no longer logs anyone out (proven by a test: a cookie issued by
|
||
one instance is accepted by a fresh instance with the same secret); works across replicas for
|
||
free. **Trade:** no server-side revocation — `logout` clears the cookie client-side, but a copied
|
||
cookie stays valid until expiry. Accepted for the Stage-0 reader pilot; revisit (server-side
|
||
revocation list, or shorter TTL + refresh) if it ever holds sensitive actions. Rotating
|
||
`tapir-session-secret` invalidates all sessions — the global logout lever.
|
||
|
||
**Not addressed here:** the tap-count of the IdP login page itself is Authentik's UX; with
|
||
re-login now rare (30-day idle or explicit logout), it matters far less.
|
||
|
||
---
|
||
|
||
## ADR-030 — Observability: slog timing + Prometheus metrics (AI-focused)
|
||
|
||
**Status:** Proposed (2026-06-11). Issue #15. **Draft for review — no code yet.**
|
||
|
||
**Context / requirements.** Nothing measures the activities that drive Tapir's performance and
|
||
UX, and the Stage-0 eval gate (ADR-016) needs a *performance* dimension to sit beside the
|
||
return-usage one. We need timing for: caption fetches (the scarce op), summarization (which model
|
||
won, how long, fallbacks), Q&A latency, LLM token spend, and basic session/usage (request rate,
|
||
latency by route, logins). Requirements:
|
||
- R1: structured `slog` timing at each AI call site (human-readable, already the logging stack).
|
||
- R2: Prometheus metrics for the same, scrapeable by the cluster's prometheus-operator.
|
||
- R3: **AI metrics are the priority** — summarize latency by `model`/`outcome`/`fallback`,
|
||
caption-fetch latency by `outcome`, chat latency by `model`, and LLM `tokens` by model+kind.
|
||
- R4: HTTP/session metrics via middleware — request count + latency by route, logins.
|
||
- R5: bounded label cardinality (no per-user, no raw-path labels).
|
||
- R6: `/metrics` must NOT be publicly exposed.
|
||
|
||
**Decision / architecture.**
|
||
1. **New package `internal/metrics`** owns all Prometheus collectors + a typed API
|
||
(`ObserveSummarize`, `ObserveCaptionFetch`, `ObserveChat`, `RecordTokens`, `IncLogin`,
|
||
`HTTPMiddleware`, `Handler`). Adapters call this API; they never import prometheus types.
|
||
2. **New dependency `github.com/prometheus/client_golang`.** Justification: it is *the* standard
|
||
Go Prometheus client and the cluster already runs prometheus-operator; hand-rolling exposition
|
||
is not worth it. (Needs the dep-justification note in the commit per repo rules.)
|
||
3. **The copied `llm` package stays stdlib-only (ADR-004).** It must not import `internal/metrics`.
|
||
Token usage is surfaced via an **optional callback** `llm.WithUsageHook(func(model string, prompt, completion int))`
|
||
set at wiring time (`buildSummarizer`/`buildChat`) to `metrics.RecordTokens`; `llm.Client` only
|
||
gains parsing of the response `usage` block. Our own adapters (`summarizer`, `youtube`, `chat`)
|
||
may import `internal/metrics` directly.
|
||
4. **HTTP middleware** reads `r.Pattern` AFTER routing (Go 1.22 sets it during ServeMux match), so
|
||
the `route` label is the bounded registered pattern (`GET /v/{videoId}`), satisfying R5;
|
||
unmatched → `other`.
|
||
5. **Dedicated metrics port** (`TAPIR_METRICS_ADDR`, default `:9090`) served by a second
|
||
`http.Server` in `cmdServe`; `/metrics` is never on the public app mux (R6). A **PodMonitor**
|
||
in `mathias/infra` scrapes it; the deployment exposes the port.
|
||
6. **slog** elapsed fields are emitted alongside each metric at the call sites (R1).
|
||
|
||
**Hook points (where the instrumentation lands).**
|
||
- `summarizer.Summarize` — per-endpoint timing + outcome (`success`/`parse_error`/`error`) + fallback flag.
|
||
- `youtube.FetchTranscript` — fetch timing + outcome from `domain.Transcript.Source`.
|
||
- `chat.Service` answer — timing by model.
|
||
- `llm.Client.Complete` — parse `usage`, fire the usage hook.
|
||
- `oidc.handleCallback` — `IncLogin`.
|
||
- `cmdServe` — wrap `Router()` in `metrics.HTTPMiddleware`; start the metrics server.
|
||
|
||
**Out of scope / later.** Persisting per-summary latency into Postgres for `tapir report`
|
||
(derive UX latency — publish/discovery → summary — from existing timestamps first; only persist
|
||
op-latency if the scrape proves insufficient). SPA view (#16) and visual refresh (#17).
|
||
|
||
**Reversibility.** Additive: a new package + middleware + a metrics port. Removing the PodMonitor
|
||
stops scraping; the app is unaffected. No schema change.
|
||
|
||
**Next steps (gated):** on approval of this ADR → BDD scenarios (`docs/use-cases/observability.feature`
|
||
+ scenario-coverage map) → TDD → implement → SemVer + docs + PodMonitor.
|
||
|
||
---
|
||
|
||
## Rejected alternatives
|
||
|
||
Approaches considered during the 2026-06-02 planning + grill session and **deliberately not
|
||
taken**. Recorded so a later session doesn't re-propose them as if they were fresh ideas. Each
|
||
maps to the ADR that settles it.
|
||
|
||
| Rejected | Why rejected | Settled by |
|
||
|----------|-------------|-----------|
|
||
| Python / FastAPI / arq stack | Would be the only non-Go service in the estate, outside every shared chassis/convention/CI pattern | ADR-001 |
|
||
| Self-hosted Supabase (auth + RLS + Vault) | Duplicates Dex, ESO+1Password, and the postgres instance being actively de-coupled; three new things to run and back up | ADR-002 |
|
||
| Living inside the `hyperguild/ingestion` monolith | The only worthwhile reuse is ~100 lines (`llm`) that get copied anyway; standalone-first wants the service to owe the monolith nothing | ADR-003, ADR-004 |
|
||
| Lifting shared packages into a `brain-common` module | Couples Tapir's release cycle to the monolith for negligible code savings | ADR-004 |
|
||
| Importing/replicating the filesystem `brain` package | Assumes co-location with the brain git checkout; wrong for a standalone networked service | ADR-005 |
|
||
| Reusing `ingestion`'s `oauth` package for YouTube/Vimeo | Same name, opposite direction — it's inbound MCP-server auth, not outbound provider OAuth | ADR-006 |
|
||
| Global cross-tenant `videos`/`transcripts` table (dedup) | Reintroduces the cross-domain DB coupling the homelab review is removing; at 1–5 users, re-summarizing is cheaper than the coupling. **Transcripts half reopened by ADR-021** — the avoided cost there is a rate-gated, reputation-risky *caption fetch*, not LLM re-summarization, so it outweighs the coupling; **videos stay per-user.** | data-model.md, **ADR-021** (transcripts only) |
|
||
| Audio-download + Whisper STT in the core path | ToS-grey, breakage-prone (yt-dlp), contends for koala GPU with the JEPA PoC; captions alone test the core hypothesis | ADR-007 |
|
||
| Building multi-tenant SaaS / Google OAuth verification now | "Real users soon" was lowered to Future B; SaaS machinery before the Stage 0 self-use gate is the primary documented anti-goal | ADR-008, VISION |
|
||
| Delegating the S5 reuse spike to an agent swarm | A 1-hour sequential read-and-judge with a single coupled conclusion; orchestration overhead exceeds the work, and it's Diamond-1 judgment the maintainer wanted to own | (process note) |
|
||
| Vault-write SA for per-user OAuth tokens (ESO as runtime write path) | ESO syncs vault→cluster at deploy time, not a runtime write API; a write-SA widens blast radius to shared infra to store app row-data | ADR-015, infra#88 |
|
||
| Supabase for per-user credential storage | Adds a second datastore for a few encrypted strings PG18 already holds; reopens ADR-002 | ADR-015, infra#88 |
|
||
| 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.
|