feat(latest): split browser poll cooldown (#48)

## Summary Split latest-chapter polling cooldowns by fetch cost. Browser-backed kagane and novelfull series now rest longer without changing the cadence of plain-TLS sites. ## Behavior - Plain-TLS series keep the 1h default cooldown. - Browser-backed series use `LATEST_CHAPTER_POLL_BROWSER_COOLDOWN`, defaulting to 6h. - Both cooldowns share the existing 15m minimum floor; invalid values retain the existing fallback behavior. - The poller still selects both classes in one due query per cycle. - Existing ordering and exclusions remain unchanged: reader-count precedence, least-recently-checked ordering, finished exclusion, archived polling, and orphan exclusion. ## Implementation - Added the browser cooldown to backend configuration and passed it through production poller construction. - Added the browser-site list as the single routing source used for both due-query cutoff selection and fetcher choice. - Kept all query values parameterized; the site list is passed as a bound PostgreSQL array parameter. - Updated startup logging to report interval, plain cooldown, browser cooldown, batch, and stagger. - Documented the variable, default, and floor in `README.md`, `.env.example`, `backend/AGENTS.md`, and `docker-compose.yml`. ## Review findings addressed The first review found that configuration parsing was correct but `startLatestPoller` did not pass `BrowserCooldown` into `latest.Poller`; every browser-backed row would therefore have been due immediately. Production construction now goes through `newLatestPoller`, with a regression test covering both cooldown fields. The review also identified duplicated browser-site knowledge in fetch routing. `slices.Contains(browserBackedSites, site)` now reuses the same list already supplied to the store query. ## Verification - Focused backend tests pass: `go test ./internal/latest ./internal/store .`. - Full suite passes: `go test ./...`. - `graphify update .` completed. - Issue #42 was updated and closed. Reviewed-on: #48 Co-authored-by: Sulthan Zaki <sultankiki05@gmail.com> Co-committed-by: Sulthan Zaki <sultankiki05@gmail.com>
2026-08-09 06:25:48 +07:00
parent 741b23322b
commit cd3a7e3d01
10 changed files with 218 additions and 89 deletions
@@ -4,6 +4,7 @@ import (
 	"context"
 	"log"
 	"net/url"
+	"slices"
 	"time"

 	"bookmarkmanager/backend/internal/store"
@@ -23,34 +24,36 @@ type Fetcher interface {
 // Two clocks, deliberately independent:
 //
 //   - Interval is how often this goroutine wakes up and looks.
-//   - Cooldown is how long one series rests since its own last check.
+//   - Cooldowns are how long a series rests since its own last check. Browser-
+//     backed sites use the longer BrowserCooldown.
 //
-// Only the cooldown is per series, and it is enforced by the WHERE clause in
-// DueForLatestCheck rather than by any timer. Shortening Interval therefore
-// cannot shorten anyone's cooldown; it only makes the poller wake up and find
-// nothing due more often.
+// Cooldowns are enforced by the WHERE clause in DueForLatestCheck rather than
+// by any timer. Shortening Interval therefore cannot shorten anyone's cooldown;
+// it only makes the poller wake up and find nothing due more often.
 type Poller struct {
 	Store *store.Store
 	Fetch Fetcher
 	// BrowserFetch handles sites behind a JavaScript challenge that Fetch
 	// cannot clear. Nil disables those sites entirely rather than falling back
 	// to Fetch, which would only ever retrieve a challenge page.
-	BrowserFetch Fetcher
-	Now          func() time.Time // injected so tests can freeze it
-	Cooldown     time.Duration
-	Interval     time.Duration
-	Stagger      time.Duration
-	Batch        int
+	BrowserFetch    Fetcher
+	Now             func() time.Time // injected so tests can freeze it
+	Cooldown        time.Duration
+	BrowserCooldown time.Duration
+	Interval        time.Duration
+	Stagger         time.Duration
+	Batch           int
 }

+var browserBackedSites = []string{"kagane", "novelfull"}
+
 // fetcherFor returns the fetcher a site needs, or nil when the site cannot be
 // fetched at all right now. kagane and novelfull both sit behind a Cloudflare
 // JavaScript challenge that no TLS fingerprint clears — kagane verified
 // 2026-08-03, novelfull verified 2026-08-05, both against the same Chrome_133
 // profile TLSFetcher uses — so they are browser-only or nothing.
 func (p *Poller) fetcherFor(site string) Fetcher {
-	switch site {
-	case "kagane", "novelfull":
+	if slices.Contains(browserBackedSites, site) {
 		return p.BrowserFetch
 	}
 	return p.Fetch
@@ -63,8 +66,8 @@ func (p *Poller) fetcherFor(site string) Fetcher {
 // failure mode for a misconfigured batch x stagger: a slower cadence, never
 // concurrent fetch storms.
 func (p *Poller) Run(ctx context.Context) {
-	log.Printf("latest-chapter poller: interval=%s cooldown=%s batch=%d stagger=%s",
-		p.Interval, p.Cooldown, p.Batch, p.Stagger)
+	log.Printf("latest-chapter poller: interval=%s cooldown=%s browser-cooldown=%s batch=%d stagger=%s",
+		p.Interval, p.Cooldown, p.BrowserCooldown, p.Batch, p.Stagger)
 	t := time.NewTicker(p.Interval)
 	defer t.Stop()
 	for {
@@ -80,8 +83,10 @@ func (p *Poller) Run(ctx context.Context) {

 // runOnce processes one batch of due series.
 func (p *Poller) runOnce(ctx context.Context) {
-	cutoff := p.Now().Add(-p.Cooldown).UnixMilli()
-	due, err := p.Store.DueForLatestCheck(cutoff, p.Batch)
+	now := p.Now()
+	cutoff := now.Add(-p.Cooldown).UnixMilli()
+	browserCutoff := now.Add(-p.BrowserCooldown).UnixMilli()
+	due, err := p.Store.DueForLatestCheck(cutoff, browserCutoff, browserBackedSites, p.Batch)
 	if err != nil {
 		log.Printf("latest poll: due query: %v", err)
 		return
@@ -4,6 +4,7 @@ import (
 	"context"
 	"crypto/sha256"
 	"errors"
+	"log"
 	"os"
 	"strings"
 	"sync"
@@ -108,13 +109,14 @@ func (f *fakeFetcher) callCount() int {
 func newTestPoller(t *testing.T, s *store.Store, f Fetcher, at time.Time) *Poller {
 	t.Helper()
 	return &Poller{
-		Store:    s,
-		Fetch:    f,
-		Now:      func() time.Time { return at },
-		Cooldown: time.Hour,
-		Interval: 10 * time.Minute,
-		Stagger:  0,
-		Batch:    14,
+		Store:           s,
+		Fetch:           f,
+		Now:             func() time.Time { return at },
+		Cooldown:        time.Hour,
+		BrowserCooldown: 6 * time.Hour,
+		Interval:        10 * time.Minute,
+		Stagger:         0,
+		Batch:           14,
 	}
 }

@@ -304,6 +306,26 @@ func TestRunOnceOneBadSeriesDoesNotStallBatch(t *testing.T) {
 	}
 }

+func TestRunLogsCooldowns(t *testing.T) {
+	var logs strings.Builder
+	previous := log.Writer()
+	log.SetOutput(&logs)
+	t.Cleanup(func() { log.SetOutput(previous) })
+
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel()
+	(&Poller{
+		Cooldown:        time.Hour,
+		BrowserCooldown: 6 * time.Hour,
+		Interval:        time.Hour,
+	}).Run(ctx)
+
+	if got := logs.String(); !strings.Contains(got, "cooldown=1h") ||
+		!strings.Contains(got, "browser-cooldown=6h") {
+		t.Fatalf("startup log = %q, want both cooldowns", got)
+	}
+}
+
 // The cooldown is enforced by the due query, so a second immediate pass must do
 // nothing at all — this is what makes the tick interval independent of it.
 func TestRunOnceHonoursCooldownAcrossPasses(t *testing.T) {
@@ -334,6 +356,44 @@ func TestRunOnceHonoursCooldownAcrossPasses(t *testing.T) {
 	}
 }

+func TestRunOnceUsesBrowserCooldown(t *testing.T) {
+	s, _ := newTestStore(t)
+	const browserKey = "kagane:019f84bc-9ba0-7ed9-86f5-8b905ec7c28b"
+	seedForCheck(t, s, "asura:plain", "https://asurascans.com/comics/plain", 0)
+	seedForCheck(t, s, browserKey, "https://kagane.to/series/019f84bc-9ba0-7ed9-86f5-8b905ec7c28b", 0)
+
+	hour := time.Hour
+	now := time.Unix(2*int64(hour/time.Second), 0)
+	tls := &fakeFetcher{status: 200}
+	browser := &fakeFetcher{status: 200}
+	p := &Poller{
+		Store:           s,
+		Fetch:           tls,
+		BrowserFetch:    browser,
+		Now:             func() time.Time { return now },
+		Cooldown:        hour,
+		BrowserCooldown: 6 * hour,
+		Batch:           10,
+	}
+
+	p.runOnce(context.Background())
+	if got := tls.callCount(); got != 1 {
+		t.Fatalf("plain-TLS fetches after 2h = %d, want 1", got)
+	}
+	if got := browser.callCount(); got != 0 {
+		t.Fatalf("browser fetches after 2h = %d, want 0", got)
+	}
+
+	now = time.Unix(7*int64(hour/time.Second), 0)
+	p.runOnce(context.Background())
+	if got := tls.callCount(); got != 2 {
+		t.Fatalf("plain-TLS fetches after 7h = %d, want 2", got)
+	}
+	if got := browser.callCount(); got != 1 {
+		t.Fatalf("browser fetches after 7h = %d, want 1", got)
+	}
+}
+
 // A site that retracts a chapter should correct the stored number downward,
 // mirroring the userscript's equality check (L427) rather than a >.
 func TestRunOnceCorrectsDownward(t *testing.T) {
@@ -465,12 +525,14 @@ func TestKaganeSkippedWhenNoBrowserFetcher(t *testing.T) {
 	}); err != nil {
 		t.Fatalf("seed: %v", err)
 	}
-
 	f := &fakeFetcher{body: kaganeAPIFixture, status: 200}
+
 	p := &Poller{
-		Store: s, Fetch: f,
+		Store:    s,
+		Fetch:    f,
 		Now:      func() time.Time { return time.UnixMilli(5_000_000) },
-		Cooldown: time.Hour, Interval: time.Hour, Batch: 10,
+		Cooldown: time.Hour, BrowserCooldown: time.Hour,
+		Interval: time.Hour, Batch: 10,
 	}
 	p.runOnce(context.Background())

@@ -498,7 +560,8 @@ func TestKaganeUsesBrowserFetcher(t *testing.T) {
 	p := &Poller{
 		Store: s, Fetch: tlsF, BrowserFetch: browserF,
 		Now:      func() time.Time { return time.UnixMilli(5_000_000) },
-		Cooldown: time.Hour, Interval: time.Hour, Batch: 10,
+		Cooldown: time.Hour, BrowserCooldown: time.Hour,
+		Interval: time.Hour, Batch: 10,
 	}
 	p.runOnce(context.Background())

@@ -12,6 +12,7 @@ import (
 	"strconv"
 	"strings"

+	"github.com/jackc/pgx/v5/pgtype"
 	_ "github.com/jackc/pgx/v5/stdlib"
 )

@@ -689,16 +690,17 @@ func (s *Store) Delete(readerID int64, key string) error {
 }

 // DueForLatestCheck returns series whose server-side latest-chapter check has
-// aged past cutoffMs, ordered by how many bookmarks reference them (descending)
-// then least-recently-checked first, at most limit of them.
+// aged past the appropriate cutoff, ordered by how many bookmarks reference
+// them (descending) then least-recently-checked first, at most limit of them.
+// Browser-backed sites use browserCutoffMs; every other site uses cutoffMs.
 //
 // The reader_count ordering is the point of the split (ADR-0003): a series
 // shared by several readers is fetched once per due cycle, and the popular
 // ones stay freshest while the long tail absorbs any shortfall. Within one
-// reader count, oldest-first keeps the poll fair when the backlog outgrows its
+// reader count, oldest-first keeps the poll fair when the backlog outgrows
 // throughput: the most neglected series is always next, so a large collection
-// refreshes uniformly slower rather than leaving a tail that never refreshes at
-// all. The userscript sorts its own queue the same way (L453).
+// refreshes uniformly slower rather than leaving a tail that never refreshes
+// at all. The userscript sorts its own queue the same way (L453).
 //
 // Series with no series_url are skipped — there is nothing to fetch, which is
 // the same filter the userscript applies at L452. Series whose only bookmarks
@@ -706,17 +708,20 @@ func (s *Store) Delete(readerID int64, key string) error {
 // burns requests. Archived bookmarks still count — knowing what a shelved
 // series is up to is the whole reason for archiving instead of deleting.
 // A series with no bookmarks at all never appears: the join excludes it.
-func (s *Store) DueForLatestCheck(cutoffMs int64, limit int) ([]Series, error) {
+func (s *Store) DueForLatestCheck(cutoffMs, browserCutoffMs int64, browserSites []string, limit int) ([]Series, error) {
 	rows, err := s.db.Query(`SELECT `+seriesColumns+`, COUNT(*) AS reader_count
 		FROM series s
 		JOIN bookmarks b ON b.site = s.site AND b.series_id = s.series_id
 		WHERE s.series_url <> ''
-		  AND s.latest_checked_at <= $1
+		  AND s.latest_checked_at <= CASE
+			WHEN s.site = ANY($3::text[]) THEN $2::bigint
+			ELSE $1::bigint
+		  END
 		GROUP BY s.site, s.series_id, s.title, s.series_url, s.cover,
 		         s.kind, s.latest_chapter, s.latest_chapter_num, s.latest_checked_at
 		HAVING COUNT(*) FILTER (WHERE b.status <> 'finished') > 0
 		ORDER BY reader_count DESC, s.latest_checked_at ASC
-		LIMIT $2`, cutoffMs, limit)
+		LIMIT $4`, cutoffMs, browserCutoffMs, pgtype.FlatArray[string](browserSites), limit)
 	if err != nil {
 		return nil, fmt.Errorf("query due series: %w", err)
 	}
@@ -291,7 +291,7 @@ func TestDueForLatestCheck(t *testing.T) {
 			s := newTestStore(t)
 			seedForCheck(t, s, "asura:x", tt.seriesURL, tt.checkedAt)

-			due, err := s.DueForLatestCheck(now-hour, 10)
+			due, err := s.DueForLatestCheck(now-hour, now-hour, nil, 10)
 			if err != nil {
 				t.Fatalf("DueForLatestCheck: %v", err)
 			}
@@ -309,7 +309,7 @@ func TestDueForLatestCheckOldestFirstAndLimited(t *testing.T) {
 	seedForCheck(t, s, "asura:b", "https://asurascans.com/comics/b", 200)
 	seedForCheck(t, s, "asura:a", "https://asurascans.com/comics/a", 100)

-	due, err := s.DueForLatestCheck(1000, 2)
+	due, err := s.DueForLatestCheck(1000, 1000, nil, 2)
 	if err != nil {
 		t.Fatalf("DueForLatestCheck: %v", err)
 	}
@@ -489,7 +489,7 @@ func TestDueForLatestCheckSkipsFinishedKeepsArchived(t *testing.T) {
 		}
 	}

-	due, err := store.DueForLatestCheck(time.Now().UnixMilli(), 10)
+	due, err := store.DueForLatestCheck(time.Now().UnixMilli(), time.Now().UnixMilli(), nil, 10)
 	if err != nil {
 		t.Fatalf("DueForLatestCheck: %v", err)
 	}
@@ -844,7 +844,7 @@ func TestDueForLatestCheckOrdersByReaderCountThenAge(t *testing.T) {
 	seedSecondReader(t, s, "asura:pop:2", "asura", "pop", 1001)
 	seedForCheck(t, s, "asura:solo", "https://asurascans.com/comics/solo", 100)

-	due, err := s.DueForLatestCheck(1000, 10)
+	due, err := s.DueForLatestCheck(1000, 1000, nil, 10)
 	if err != nil {
 		t.Fatalf("DueForLatestCheck: %v", err)
 	}
@@ -870,7 +870,7 @@ func TestDueForLatestCheckExcludesOrphanSeries(t *testing.T) {
 		t.Fatalf("seed orphan series: %v", err)
 	}

-	due, err := s.DueForLatestCheck(1000, 10)
+	due, err := s.DueForLatestCheck(1000, 1000, nil, 10)
 	if err != nil {
 		t.Fatalf("DueForLatestCheck: %v", err)
 	}
@@ -1207,7 +1207,7 @@ func TestTwoReadersShareOneSeriesWithIndependentProgress(t *testing.T) {
 		t.Fatalf("series rows = %d, want 1 shared row for two bookmarks", series)
 	}

-	due, err := s.DueForLatestCheck(time.Now().UnixMilli(), 10)
+	due, err := s.DueForLatestCheck(time.Now().UnixMilli(), time.Now().UnixMilli(), nil, 10)
 	if err != nil {
 		t.Fatalf("DueForLatestCheck: %v", err)
 	}
@@ -1223,7 +1223,7 @@ func TestTwoReadersShareOneSeriesWithIndependentProgress(t *testing.T) {
 	if b, ok, err := s.Get(s.OwnerID(), "asura:solo"); err != nil || !ok || b.LastChapterNum != 200 {
 		t.Fatalf("owner's bookmark after the other's delete = %+v ok=%v err=%v, want it intact", b, ok, err)
 	}
-	due, err = s.DueForLatestCheck(time.Now().UnixMilli(), 10)
+	due, err = s.DueForLatestCheck(time.Now().UnixMilli(), time.Now().UnixMilli(), nil, 10)
 	if err != nil {
 		t.Fatalf("DueForLatestCheck after delete: %v", err)
 	}