From beeb5bc31b63026e7b9446624144b806eafbd11e Mon Sep 17 00:00:00 2001 From: Mathias Date: Wed, 10 Jun 2026 22:10:15 +0200 Subject: [PATCH] feat(gate): foreground caption fetches take priority over the background sweep (ADR-026) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A user waiting on a Summarize click shared the per-IP caption gate equally with the background firehose, so on a busy IP the click was slow or 429'd. Add a context-marked priority lane: the web path (engineProcessor.ProcessVideo) marks its context foreground; the gate serves foreground immediately while background fetches yield until no foreground is pending. Threaded via a context value (no new signatures) + a process-wide foregroundPending counter. Clicks are rare, so the background barely loses throughput; the waiting human gets the cleaner slot. Drops the credentials probe: ADR-010 and captions.go already settle it — the timedtext/InnerTube path rejects authenticated requests and the OAuth token does not authenticate it anyway, so auth cannot help and can hurt. Documented in ADR-026 rather than built. Co-Authored-By: Claude Opus 4.8 (1M context) --- DECISIONS.md | 30 ++++++++++++++++ cmd/tapir/processor.go | 5 +++ internal/adapters/youtube/gate.go | 47 ++++++++++++++++++++++++++ internal/adapters/youtube/gate_test.go | 34 +++++++++++++++++++ 4 files changed, 116 insertions(+) diff --git a/DECISIONS.md b/DECISIONS.md index 9b19462..9e03cf1 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -958,6 +958,36 @@ 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. + +--- + ## Rejected alternatives Approaches considered during the 2026-06-02 planning + grill session and **deliberately not diff --git a/cmd/tapir/processor.go b/cmd/tapir/processor.go index a61359f..40f0377 100644 --- a/cmd/tapir/processor.go +++ b/cmd/tapir/processor.go @@ -113,6 +113,11 @@ type engineProcessor struct { } func (p *engineProcessor) ProcessVideo(ctx context.Context, userID, videoID string) error { + // This is the user-initiated (foreground) path — a click on "Summarize", + // "Try now", or a pasted URL. Mark the context so the caption gate gives it + // priority over the background sweep (ADR-026, Pillar A). + ctx = youtube.ForegroundContext(ctx) + row, err := p.store.GetVideoRow(ctx, userID, videoID) if err != nil { return fmt.Errorf("load video %q: %w", videoID, err) diff --git a/internal/adapters/youtube/gate.go b/internal/adapters/youtube/gate.go index 08f52c4..1b3c639 100644 --- a/internal/adapters/youtube/gate.go +++ b/internal/adapters/youtube/gate.go @@ -2,6 +2,7 @@ package youtube import ( "context" + "sync/atomic" "time" "golang.org/x/time/rate" @@ -29,9 +30,55 @@ func SetFetchRate(interval time.Duration) { globalFetchGate = rate.NewLimiter(rate.Every(interval), 1) } +// foregroundPending counts in-flight foreground (user-initiated) caption fetches. +// The background sweep yields the gate while this is non-zero so a human waiting +// on a click gets the next slot — and, on a near-throttled IP, the pre-429 window +// — instead of competing equally with the firehose (ADR-026, Pillar A). Clicks are +// rare and bursty, so background barely notices; the win to the click is large. +var foregroundPending atomic.Int64 + +// fgCtxKey marks a context as foreground (user-initiated). Unexported; set via +// ForegroundContext and read via isForeground so only this package owns the key. +type fgCtxKey struct{} + +// ForegroundContext marks ctx as a user-initiated (foreground) fetch so the gate +// gives it priority. The web "Summarize"/paste/retry path wraps its context with +// this; the background scheduler leaves it unset. +func ForegroundContext(ctx context.Context) context.Context { + return context.WithValue(ctx, fgCtxKey{}, true) +} + +func isForeground(ctx context.Context) bool { + v, _ := ctx.Value(fgCtxKey{}).(bool) + return v +} + +// fgYieldPoll is how often a background waiter re-checks whether a foreground +// fetch is still pending. Short enough to feel immediate, long enough not to spin. +const fgYieldPoll = 200 * time.Millisecond + // WaitFetchGate blocks until the process-wide gate allows one timedtext fetch, // respecting ctx cancellation. Called from httpDo before every live outbound // caption fetch so the scheduler and the click-path share the same egress budget. +// +// Foreground (user-initiated) fetches take priority: they register as pending and +// acquire a token immediately. Background fetches first yield — they wait until no +// foreground fetch is pending — so a live click is never stuck behind the +// background sweep and gets the cleaner slot against the per-IP limit (ADR-026). func WaitFetchGate(ctx context.Context) error { + if isForeground(ctx) { + foregroundPending.Add(1) + defer foregroundPending.Add(-1) + return globalFetchGate.Wait(ctx) + } + + // Background: defer to any pending foreground fetch before taking a token. + for foregroundPending.Load() > 0 { + select { + case <-ctx.Done(): + return ctx.Err() + case <-time.After(fgYieldPoll): + } + } return globalFetchGate.Wait(ctx) } diff --git a/internal/adapters/youtube/gate_test.go b/internal/adapters/youtube/gate_test.go index eeaefe8..a8916cd 100644 --- a/internal/adapters/youtube/gate_test.go +++ b/internal/adapters/youtube/gate_test.go @@ -82,3 +82,37 @@ func TestSetFetchRateZeroIsUnlimited(t *testing.T) { require.NoError(t, WaitFetchGate(context.Background())) } } + +func TestForegroundContextMarker(t *testing.T) { + require.False(t, isForeground(context.Background()), "plain context is background") + require.True(t, isForeground(ForegroundContext(context.Background())), "marked context is foreground") +} + +// TestWaitFetchGateForegroundProceedsImmediately: a foreground fetch acquires a +// token without yielding, even when background callers exist. +func TestWaitFetchGateForegroundProceedsImmediately(t *testing.T) { + SetFetchRate(0) // unlimited limiter — isolate the yield logic from pacing + foregroundPending.Store(0) + t.Cleanup(func() { foregroundPending.Store(0) }) + + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + require.NoError(t, WaitFetchGate(ForegroundContext(ctx)), "foreground proceeds immediately") +} + +// TestWaitFetchGateBackgroundYieldsToForeground: while a foreground fetch is +// pending, a background fetch yields (does not take a token) until the foreground +// clears — proven by a background wait timing out against its own deadline, then +// succeeding once the foreground is done. +func TestWaitFetchGateBackgroundYieldsToForeground(t *testing.T) { + SetFetchRate(0) + foregroundPending.Store(1) // simulate a foreground fetch in flight + t.Cleanup(func() { foregroundPending.Store(0) }) + + ctx, cancel := context.WithTimeout(context.Background(), 250*time.Millisecond) + defer cancel() + require.Error(t, WaitFetchGate(ctx), "background yields (blocks) while foreground is pending") + + foregroundPending.Store(0) // foreground done + require.NoError(t, WaitFetchGate(context.Background()), "background proceeds once foreground clears") +}