From 56afb9f237be0a365e0b06c5df11f71fb2c908a4 Mon Sep 17 00:00:00 2001 From: Sulthan Zaki Date: Sun, 16 Aug 2026 15:17:30 +0700 Subject: [PATCH] feat: a Reader's Sighting defers a Poll of a solitary Series (#103) A userscript PUT already carries the Latest Chapter the Reader's own browser read; it now stands in for a Poll where being wrong can hurt nobody else. Deferral lives in the due query beside the rest cutoff: one Bookmark, sighted within one rest, under the six-rest ceiling. checkOne judges the Reader whose report raised the value off the comparison it already makes - three contradictions stop them deferring, twenty confirmations forgive. ADR-0011 records the trust model and the rejected alternatives. --- backend/AGENTS.md | 20 +- backend/api_test.go | 40 ++ backend/internal/api/handlers.go | 13 +- backend/internal/latest/poller.go | 38 +- backend/internal/latest/sighting_test.go | 441 ++++++++++++++++++ backend/internal/latest/sites.go | 7 + .../migrations/0011_series_sightings.sql | 10 + backend/internal/store/store.go | 127 ++++- backend/internal/store/store_test.go | 18 +- .../adr/0011-sighting-deferral-trust-model.md | 106 +++++ 10 files changed, 805 insertions(+), 15 deletions(-) create mode 100644 backend/internal/latest/sighting_test.go create mode 100644 backend/internal/store/migrations/0011_series_sightings.sql create mode 100644 docs/adr/0011-sighting-deferral-trust-model.md diff --git a/backend/AGENTS.md b/backend/AGENTS.md index 6c3c5d3..ece7bfc 100644 --- a/backend/AGENTS.md +++ b/backend/AGENTS.md @@ -27,7 +27,8 @@ Guidance for OpenCode (and Claude Code) working under `backend/`. See root `AGEN `series` keyed `(site, series_id)` (`asura`|`demonic`|`comix`|`kagane`|`novelfull`|`lightnovelworld`) owns the shared facts — title, cover, canonical URL, `kind` (`manga`|`novel`), - Latest Chapter, `latest_checked_at` — and `bookmarks` holds only what + Latest Chapter, `latest_checked_at`, and the Sighting pair + `latest_sighted_at`/`latest_raised_by` (issue #103) — and `bookmarks` holds only what differs between readers: progress, favourite, lifecycle bucket, `updated_at`. A bookmark is keyed `(reader_id, site, series_id)` — no surrogate id; the wire `key` is derived as `site:series_id` on read — and @@ -91,6 +92,23 @@ Guidance for OpenCode (and Claude Code) working under `backend/`. See root `AGEN every tick; found chapter written straight to the series row via `Store.SetLatestChapter`, so a bookmark's `updated_at` — and the list order — is never touched. + **Sightings** (issue #103, ADR-0011) let a Reader's own page read defer a + Poll: `Store.RecordSighting` — called by the PUT handler *before* the Upsert, + because the raise test needs the row as it stands — stamps + `series.latest_sighted_at` and, when the report raises the stored number, + names its Reader in `series.latest_raised_by`. The due query's HAVING clause + is where deferral lives: a Series is skipped only while it has exactly one + Bookmark, was sighted within one Rest, and is under the `sightingCeiling` + (six rests) since its last Poll. So a shared Series is never deferred, and no + Series goes six hours unpolled whatever arrives. `checkOne` judges the named + Reader off the comparison it already makes: a lower number is a + contradiction (logged with the Reader and both numbers), the same number an + agreement, a higher number the Site publishing and neither. Three + contradictions (`store.SightingDisagreementLimit`) stop that Reader deferring + — their reports still write the Latest Chapter — and twenty consecutive + agreements (`store.SightingAgreementsToClear`) forgive them, as does the + owner's clear-marks control. Deferral is recomputed from live facts every + round, so nothing needs invalidating when a Series gains a second Bookmark. Refusals and browser loss are Lane-local: two `errChallengeHeld` in one pass stop that Site for `refuseBackoff` (15m) while other Lanes continue; an `errBrowserInterrupted` (remote Chrome restart) sets a shared Poller flag diff --git a/backend/api_test.go b/backend/api_test.go index d2458db..beee5a8 100644 --- a/backend/api_test.go +++ b/backend/api_test.go @@ -621,6 +621,46 @@ func TestPutDoesNotClobberLatestCheckedAt(t *testing.T) { } } +// Sighting deferral (issue #103) only reaches production through the PUT +// handler: the store and poller can be right and the feature still dead if the +// handler never records the report. Asserted where a client can see it - the +// series stops being due the moment the PUT lands. +func TestPutRecordsASighting(t *testing.T) { + s := newTestStore(t) + srv := newRouter(s, testConfig(), nil) + + now := time.Now().UnixMilli() + hour := time.Hour.Milliseconds() + seedForCheck(t, s, "asura:x", "https://asurascans.com/comics/x", now-2*hour) + due, err := s.DueForLatestCheck("asura", now-hour, now-6*hour) + if err != nil { + t.Fatalf("DueForLatestCheck: %v", err) + } + if len(due) != 1 { + t.Fatalf("due before the PUT = %d series, want 1", len(due)) + } + + body := `{"key":"asura:x","site":"asura","series_id":"x", + "series_url":"https://asurascans.com/comics/x", + "last_chapter":"Chapter 5","last_chapter_num":5, + "latest_chapter":"Chapter 9","latest_chapter_num":9}` + req := httptest.NewRequest(http.MethodPut, "/bookmarks/asura:x", strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + srv.ServeHTTP(rec, auth(req)) + if rec.Code != http.StatusOK { + t.Fatalf("PUT status = %d, want 200 (body %s)", rec.Code, rec.Body.String()) + } + + due, err = s.DueForLatestCheck("asura", now-hour, now-6*hour) + if err != nil { + t.Fatalf("DueForLatestCheck: %v", err) + } + if len(due) != 0 { + t.Fatalf("due after the PUT = %d series, want 0: the handler recorded no Sighting", len(due)) + } +} + // The userscript route is registered outside the web UI's Discord auth, so it // must keep working whatever the web config — see internal/userscript for the // handler's own behaviour. The credential in the path is the owner's derived diff --git a/backend/internal/api/handlers.go b/backend/internal/api/handlers.go index aef24f8..94f42bd 100644 --- a/backend/internal/api/handlers.go +++ b/backend/internal/api/handlers.go @@ -99,7 +99,18 @@ func (h *Handler) Put(w http.ResponseWriter, r *http.Request) { // reading progress actually moved. Any client value is ignored. b.UpdatedAt = time.Now().UnixMilli() - stored, err := h.Store.Upsert(httpmw.ReaderID(r), b) + // A userscript PUT is a Sighting: the Reader's browser was on the Series + // page and read its Latest Chapter (issue #103). Recorded before the + // Upsert, which is what makes the raise comparison possible, and never + // from the web UI's own read-modify-write — a Reader toggling a favourite + // has not looked at the Site and must not postpone a Poll. A failure here + // costs a deferral, not the write, so it is logged and dropped. + readerID := httpmw.ReaderID(r) + if err := h.Store.RecordSighting(readerID, b.Site, b.SeriesID, b.LatestChapterNum, b.UpdatedAt); err != nil { + log.Printf("record sighting: %v", err) + } + + stored, err := h.Store.Upsert(readerID, b) if err != nil { log.Printf("upsert: %v", err) http.Error(w, "internal error", http.StatusInternalServerError) diff --git a/backend/internal/latest/poller.go b/backend/internal/latest/poller.go index 90028e0..de79d2a 100644 --- a/backend/internal/latest/poller.go +++ b/backend/internal/latest/poller.go @@ -250,7 +250,8 @@ func (p *Poller) runLanePass(ctx context.Context, name string, paced bool) time. return defaultGap } - due, err := p.Store.DueForLatestCheck(name, now.Add(-s.Rest).UnixMilli()) + due, err := p.Store.DueForLatestCheck(name, + now.Add(-s.Rest).UnixMilli(), now.Add(-sightingCeiling).UnixMilli()) if err != nil { log.Printf("latest poll %s: due query: %v", name, err) st.Gap = defaultGap @@ -461,6 +462,11 @@ func (p *Poller) checkOne(ctx context.Context, sr store.Series) error { return nil } + // The Poll is the oracle for whatever Sighting last raised this Series + // (issue #103), and the judgement is free: the comparison below already + // exists, and no extra request is made to reach it. + p.judgeSighting(sr, facts.Latest.Num) + // Equality, not >, mirroring the userscript (L427): a site that retracts a // chapter should correct the stored number downward. The comparison is // against the due-query snapshot; a concurrent write in between only costs @@ -481,6 +487,36 @@ func (p *Poller) checkOne(ctx context.Context, sr store.Series) error { return nil } +// judgeSighting settles the Sighting the Series' stored Latest Chapter is owed +// to, if any, against what the Site actually publishes. The asymmetry is the +// whole of the detection rule and is what keeps it free of false alarms: a Poll +// finding a *lower* number than stored means the Reader who raised it reported +// a chapter that does not exist, while a Poll finding a higher one is only the +// Site publishing since and means nothing about the report. Equality confirms +// the report, which is how an honest Reader earns back a mark. +// +// A Series with no attribution — the stored value is a Poll's own, or a +// previous Poll already judged the report — is nobody's to answer for. +func (p *Poller) judgeSighting(sr store.Series, found float64) { + if sr.LatestRaisedBy == nil || sr.LatestChapterNum == nil { + return + } + stored := *sr.LatestChapterNum + if found > stored { + return + } + if found < stored { + // Logged with both numbers and the Reader, because that is what tells a + // broken Site adapter (which marks every Reader of that Site at once) + // from one Reader deliberately lying. + log.Printf("latest poll %q: sighting contradicted: reader %d raised it to %v, site publishes %v", + sr.Key(), *sr.LatestRaisedBy, stored, found) + } + if err := p.Store.RecordSightingOutcome(sr.Site, sr.SeriesID, *sr.LatestRaisedBy, found == stored); err != nil { + log.Printf("latest poll %q: record sighting outcome: %v", sr.Key(), err) + } +} + // healCover runs prefetchCover in the background. Cover bytes come from a // different host — often a CDN — and heal once in a Series's life, so they // must not consume a Lane's gap: a large import with many blanks would diff --git a/backend/internal/latest/sighting_test.go b/backend/internal/latest/sighting_test.go new file mode 100644 index 0000000..17f5c7d --- /dev/null +++ b/backend/internal/latest/sighting_test.go @@ -0,0 +1,441 @@ +package latest + +import ( + "context" + "crypto/sha256" + "fmt" + "log" + "strings" + "testing" + "time" + + "bookmarkmanager/backend/internal/store" +) + +// Sightings (issue #103) are specified at the Poller seam, with the store as +// the way in: a Sighting is seeded the way handlers.Put performs one, a round +// is run against the injected fetcher and a frozen clock, and the assertions +// are the two observable facts — whether the Series was fetched, and what the +// stored Latest Chapter is afterwards. Nothing here asserts counter arithmetic +// through an internal call or reads how a deferral is represented in a row. + +const ( + sightingSlug = "chronicles-of-the-demon-faction-f886a8af" + sightingKey = "asura:" + sightingSlug + sightingURL = "https://asurascans.com/comics/" + sightingSlug +) + +// sightingFixtureLatest is the newest chapter asuraSeriesFixture publishes. +const sightingFixtureLatest = 181.0 + +// sight performs one Sighting exactly as the JSON API does (handlers.Put): +// RecordSighting against the row as stored, then the Upsert that stores the +// reported value. The order is load-bearing — the raise comparison has nothing +// to compare against once the Upsert has landed — and the bookmark's own fields +// are carried over untouched, which is what a userscript PUT does when it +// echoes back the row it cached. +func sight(t *testing.T, s *store.Store, readerID int64, key string, num float64, at time.Time) { + t.Helper() + site, seriesID, ok := strings.Cut(key, ":") + if !ok { + t.Fatalf("key %q: no ':' separator", key) + } + b, found, err := s.Get(readerID, key) + if err != nil || !found { + t.Fatalf("sight %q: get: %v found=%v", key, err, found) + } + if err := s.RecordSighting(readerID, site, seriesID, &num, at.UnixMilli()); err != nil { + t.Fatalf("sight %q: %v", key, err) + } + b.LatestChapter = fmt.Sprintf("Chapter %v", num) + b.LatestChapterNum = &num + b.UpdatedAt = at.UnixMilli() + if _, err := s.Upsert(readerID, b); err != nil { + t.Fatalf("sight %q: upsert: %v", key, err) + } +} + +// secondReader is another Reader on the same database. The owner seed is the +// only reader-creation path in this package, so a second Open as a different +// owner is how a test gets one (as TestRunOnceFetchesSharedSeriesOnce does). +func secondReader(t *testing.T, dbURL string) *store.Store { + t.Helper() + other, err := store.Open(dbURL, + store.Owner{DiscordID: "second-reader", TokenHash: sha256.Sum256([]byte("second-token-hash"))}, + t.TempDir(), testCoverBaseURL) + if err != nil { + t.Fatalf("Open second reader: %v", err) + } + t.Cleanup(func() { other.Close() }) + return other +} + +func readLatestNum(t *testing.T, s *store.Store, readerID int64, key string) float64 { + t.Helper() + b, ok, err := s.Get(readerID, key) + if err != nil || !ok { + t.Fatalf("Get %q: %v ok=%v", key, err, ok) + } + if b.LatestChapterNum == nil { + t.Fatalf("%q has no latest chapter", key) + } + return *b.LatestChapterNum +} + +// A Series only one Reader bookmarks is the case where being wrong can hurt +// nobody but the Reader who reported it, so their Sighting stands in for the +// Poll and the round leaves the Series alone. +func TestSightingOnSolitarySeriesDefersPoll(t *testing.T) { + s, _ := newTestStore(t) + now := time.UnixMilli(20 * time.Hour.Milliseconds()) + seedForCheck(t, s, sightingKey, sightingURL, now.Add(-2*time.Hour).UnixMilli()) + sight(t, s, s.OwnerID(), sightingKey, sightingFixtureLatest, now.Add(-10*time.Minute)) + + f := &fakeFetcher{body: asuraSeriesFixture, status: 200} + newTestPoller(t, s, f, now).runOnce(context.Background()) + + if got := f.callCount(); got != 0 { + t.Fatalf("fetched %d times after a Sighting on a solitary Series, want 0", got) + } +} + +// On a shared Series the Sighting still writes the Latest Chapter for everyone, +// but the Poll happens on schedule anyway — which is what corrects a wrong +// value within the hour instead of letting it persist. +func TestSightingOnSharedSeriesDoesNotDeferPoll(t *testing.T) { + s, dbURL := newTestStore(t) + now := time.UnixMilli(20 * time.Hour.Milliseconds()) + seedForCheck(t, s, sightingKey, sightingURL, now.Add(-2*time.Hour).UnixMilli()) + other := secondReader(t, dbURL) + if _, err := s.Upsert(other.OwnerID(), store.Bookmark{ + Key: sightingKey, Site: "asura", SeriesID: sightingSlug, UpdatedAt: 2000, + }); err != nil { + t.Fatalf("seed second reader: %v", err) + } + sight(t, s, s.OwnerID(), sightingKey, 200, now.Add(-10*time.Minute)) + + // The Sighting updated the shared row immediately, before any Poll. + if got := readLatestNum(t, s, s.OwnerID(), sightingKey); got != 200 { + t.Fatalf("latest after the Sighting = %v, want 200", got) + } + + f := &fakeFetcher{body: asuraSeriesFixture, status: 200} + newTestPoller(t, s, f, now).runOnce(context.Background()) + + if got := f.callCount(); got != 1 { + t.Fatalf("fetched %d times after a Sighting on a shared Series, want 1", got) + } + if got := readLatestNum(t, s, s.OwnerID(), sightingKey); got != sightingFixtureLatest { + t.Fatalf("latest after the Poll = %v, want the Site's own %v", got, sightingFixtureLatest) + } +} + +// Reporting a chapter is not reading one: a Sighting may move the Latest +// Chapter and nothing else. Both the solitary and the shared case, because the +// deferral branch must not be where this guarantee lives. +func TestSightingLeavesProgressAndOrderingUntouched(t *testing.T) { + for _, shared := range []bool{false, true} { + name := "solitary" + if shared { + name = "shared" + } + t.Run(name, func(t *testing.T) { + s, dbURL := newTestStore(t) + read := 5.0 + if _, err := s.Upsert(s.OwnerID(), store.Bookmark{ + Key: sightingKey, Site: "asura", SeriesID: sightingSlug, SeriesURL: sightingURL, + LastChapter: "Chapter 5", LastChapterNum: read, + LastChapterURL: sightingURL + "/chapter/5", UpdatedAt: 1000, + }); err != nil { + t.Fatalf("seed: %v", err) + } + if shared { + other := secondReader(t, dbURL) + if _, err := s.Upsert(other.OwnerID(), store.Bookmark{ + Key: sightingKey, Site: "asura", SeriesID: sightingSlug, UpdatedAt: 2000, + }); err != nil { + t.Fatalf("seed second reader: %v", err) + } + } + + sight(t, s, s.OwnerID(), sightingKey, 200, time.UnixMilli(9_000_000)) + + b, ok, err := s.Get(s.OwnerID(), sightingKey) + if err != nil || !ok { + t.Fatalf("Get: %v ok=%v", err, ok) + } + if b.LatestChapterNum == nil || *b.LatestChapterNum != 200 { + t.Fatalf("LatestChapterNum = %v, want 200", b.LatestChapterNum) + } + if b.LastChapterNum != read { + t.Fatalf("LastChapterNum = %v, want %v: a Sighting is not Progress", b.LastChapterNum, read) + } + if b.UpdatedAt != 1000 { + t.Fatalf("updated_at moved to %d: a Sighting must not reorder the list", b.UpdatedAt) + } + }) + } +} + +// The ceiling is what makes trusting a client report safe: however recently a +// Series was sighted, one that has not been Polled in six hours is Polled. +func TestSightingCeilingForcesPoll(t *testing.T) { + s, _ := newTestStore(t) + now := time.UnixMilli(20 * time.Hour.Milliseconds()) + seedForCheck(t, s, sightingKey, sightingURL, now.Add(-7*time.Hour).UnixMilli()) + sight(t, s, s.OwnerID(), sightingKey, sightingFixtureLatest, now.Add(-time.Minute)) + + f := &fakeFetcher{body: asuraSeriesFixture, status: 200} + newTestPoller(t, s, f, now).runOnce(context.Background()) + + if got := f.callCount(); got != 1 { + t.Fatalf("fetched %d times past the %s ceiling, want 1", got, sightingCeiling) + } +} + +// Deferral is decided from live facts every round, so a Series that gains a +// second Bookmark stops deferring at once — and one that loses it defers again. +func TestDeferralFollowsTheBookmarkCount(t *testing.T) { + s, dbURL := newTestStore(t) + now := time.UnixMilli(20 * time.Hour.Milliseconds()) + seedForCheck(t, s, sightingKey, sightingURL, now.Add(-2*time.Hour).UnixMilli()) + sight(t, s, s.OwnerID(), sightingKey, sightingFixtureLatest, now.Add(-10*time.Minute)) + + f := &fakeFetcher{body: asuraSeriesFixture, status: 200} + p := newTestPoller(t, s, f, now) + p.runOnce(context.Background()) + if got := f.callCount(); got != 0 { + t.Fatalf("solitary Series fetched %d times, want 0", got) + } + + other := secondReader(t, dbURL) + if _, err := s.Upsert(other.OwnerID(), store.Bookmark{ + Key: sightingKey, Site: "asura", SeriesID: sightingSlug, UpdatedAt: 2000, + }); err != nil { + t.Fatalf("seed second reader: %v", err) + } + p.runOnce(context.Background()) + if got := f.callCount(); got != 1 { + t.Fatalf("shared Series fetched %d times, want 1", got) + } + + // The Poll above consumed the rest, so move past it before asking again. + if err := other.Delete(other.OwnerID(), sightingKey); err != nil { + t.Fatalf("delete second bookmark: %v", err) + } + later := now.Add(2 * time.Hour) + p.Now = func() time.Time { return later } + sight(t, s, s.OwnerID(), sightingKey, sightingFixtureLatest, later.Add(-time.Minute)) + p.runOnce(context.Background()) + if got := f.callCount(); got != 1 { + t.Fatalf("Series fetched %d times after returning to one Bookmark, want 1", got) + } +} + +// A Series nobody reports any more returns to the normal schedule on its own: +// the Sighting's standing lasts one rest, not forever. +func TestDeferralExpiresWithoutFurtherSightings(t *testing.T) { + s, _ := newTestStore(t) + now := time.UnixMilli(20 * time.Hour.Milliseconds()) + seedForCheck(t, s, sightingKey, sightingURL, now.Add(-2*time.Hour).UnixMilli()) + sight(t, s, s.OwnerID(), sightingKey, sightingFixtureLatest, now.Add(-10*time.Minute)) + + f := &fakeFetcher{body: asuraSeriesFixture, status: 200} + p := newTestPoller(t, s, f, now) + p.runOnce(context.Background()) + if got := f.callCount(); got != 0 { + t.Fatalf("fetched %d times while the Sighting stood, want 0", got) + } + + p.Now = func() time.Time { return now.Add(90 * time.Minute) } + p.runOnce(context.Background()) + if got := f.callCount(); got != 1 { + t.Fatalf("fetched %d times once the Sighting aged out, want 1", got) + } +} + +// demonicFixture publishes one chapter in demonicscans' live page shape, so a +// test can make a Site publish an arbitrary number rather than the one the +// captured fixture froze. +func demonicFixture(num float64) string { + return fmt.Sprintf( + `Chapter %v`, + num, num, num) +} + +const ( + demonicKey = "demonic:Catastrophic-Necromancer" + demonicURL = "https://demonicscans.org/manga/Catastrophic-Necromancer" +) + +// contradictOnce reports a chapter that does not exist and then runs the round +// that catches it, returning when that round ran so a caller can chain the +// next one. The wait is one rest and a minute: a Sighting stands in for exactly +// one rest, so that is the first moment this solitary Series is Polled again. +func contradictOnce(t *testing.T, s *store.Store, p *Poller, sightAt time.Time, real float64) time.Time { + t.Helper() + sight(t, s, s.OwnerID(), demonicKey, real+500, sightAt) + at := sightAt.Add(defaultRest + time.Minute) + p.Now = func() time.Time { return at } + p.runOnce(context.Background()) + if got := readLatestNum(t, s, s.OwnerID(), demonicKey); got != real { + t.Fatalf("latest after the Poll = %v, want the Site's own %v", got, real) + } + return at +} + +func seedDemonic(t *testing.T, s *store.Store, checkedAt int64) { + t.Helper() + seedForCheck(t, s, demonicKey, demonicURL, checkedAt) +} + +// A Poll finding a lower number than stored means the Sighting that raised it +// was false. The Reader is named — not the Series flagged — and both numbers are +// logged, because that is what tells a broken adapter from a deliberate lie. +func TestPollContradictingASightingNamesTheReaderAndBothNumbers(t *testing.T) { + s, _ := newTestStore(t) + now := time.UnixMilli(20 * time.Hour.Milliseconds()) + seedDemonic(t, s, now.Add(-2*time.Hour).UnixMilli()) + + var logs strings.Builder + prev := log.Writer() + log.SetOutput(&logs) + t.Cleanup(func() { log.SetOutput(prev) }) + + f := &fakeFetcher{body: demonicFixture(296), status: 200} + p := newTestPoller(t, s, f, now) + contradictOnce(t, s, p, now, 296) + + got := logs.String() + for _, want := range []string{ + fmt.Sprintf("reader %d", s.OwnerID()), "796", "296", demonicKey, + } { + if !strings.Contains(got, want) { + t.Fatalf("contradiction log = %q, want it to name %q", got, want) + } + } +} + +// Three contradictions cost the Reader the right to defer. Nothing here writes +// a counter: the marks are earned through Polls, which is the only way +// production produces them. +func TestThreeContradictionsStopDeferral(t *testing.T) { + s, _ := newTestStore(t) + start := time.UnixMilli(20 * time.Hour.Milliseconds()) + seedDemonic(t, s, start.Add(-2*time.Hour).UnixMilli()) + + f := &fakeFetcher{body: demonicFixture(296), status: 200} + p := newTestPoller(t, s, f, start) + at := start + for range store.SightingDisagreementLimit { + at = contradictOnce(t, s, p, at.Add(time.Minute), 296) + } + fetchesSoFar := f.callCount() + + // The marked Reader sights the same solitary Series again. It still writes + // the Latest Chapter — the penalty removes a privilege, it does not silence + // anyone — but the Poll is no longer postponed: the round below runs while a + // trusted Reader's Sighting would still be standing, and fetches anyway. + sight(t, s, s.OwnerID(), demonicKey, 900, at.Add(31*time.Minute)) + if got := readLatestNum(t, s, s.OwnerID(), demonicKey); got != 900 { + t.Fatalf("latest after a marked Reader's Sighting = %v, want 900", got) + } + p.Now = func() time.Time { return at.Add(defaultRest + time.Minute) } + p.runOnce(context.Background()) + if got := f.callCount(); got != fetchesSoFar+1 { + t.Fatalf("marked Reader's Sighting still deferred the Poll (fetches %d, want %d)", + got, fetchesSoFar+1) + } +} + +// The owner's remedy for a mark a broken Site adapter produced restores the +// privilege without a wait and without SQL. +func TestClearingMarksRestoresDeferral(t *testing.T) { + s, _ := newTestStore(t) + start := time.UnixMilli(20 * time.Hour.Milliseconds()) + seedDemonic(t, s, start.Add(-2*time.Hour).UnixMilli()) + + f := &fakeFetcher{body: demonicFixture(296), status: 200} + p := newTestPoller(t, s, f, start) + at := start + for range store.SightingDisagreementLimit { + at = contradictOnce(t, s, p, at.Add(time.Minute), 296) + } + if err := s.ClearReaderMarks(s.OwnerID()); err != nil { + t.Fatalf("ClearReaderMarks: %v", err) + } + + fetchesSoFar := f.callCount() + sight(t, s, s.OwnerID(), demonicKey, 900, at.Add(31*time.Minute)) + p.Now = func() time.Time { return at.Add(defaultRest + time.Minute) } + p.runOnce(context.Background()) + if got := f.callCount(); got != fetchesSoFar { + t.Fatalf("fetched %d times after the marks were cleared, want %d: deferral must resume", + got, fetchesSoFar) + } +} + +// Recovery is automatic but expensive: twenty Polls that each confirm a +// Sighting of this Reader's clear the marks. Each round needs a new chapter, +// because only a report that raises the stored number is attributed and so only +// that one can be confirmed. +func TestTwentyAgreementsClearTheMarks(t *testing.T) { + s, _ := newTestStore(t) + start := time.UnixMilli(20 * time.Hour.Milliseconds()) + seedDemonic(t, s, start.Add(-2*time.Hour).UnixMilli()) + + f := &fakeFetcher{body: demonicFixture(296), status: 200} + p := newTestPoller(t, s, f, start) + at := start + for range store.SightingDisagreementLimit { + at = contradictOnce(t, s, p, at.Add(time.Minute), 296) + } + + chapter := 296.0 + for range store.SightingAgreementsToClear { + chapter++ + sight(t, s, s.OwnerID(), demonicKey, chapter, at.Add(time.Minute)) + f.body = demonicFixture(chapter) // the Site publishes what was reported + at = at.Add(defaultRest + time.Minute) + p.Now = func() time.Time { return at } + p.runOnce(context.Background()) + } + + fetchesSoFar := f.callCount() + chapter++ + sight(t, s, s.OwnerID(), demonicKey, chapter, at.Add(31*time.Minute)) + p.Now = func() time.Time { return at.Add(defaultRest + time.Minute) } + p.runOnce(context.Background()) + if got := f.callCount(); got != fetchesSoFar { + t.Fatalf("fetched %d times after %d confirmations, want %d: the marks must be forgiven", + got, store.SightingAgreementsToClear, fetchesSoFar) + } +} + +// A Poll finding a higher number is the Site publishing since the Sighting and +// means nothing about the Reader — no mark, and no credit either. +func TestPollFindingHigherNumberIsNotAContradiction(t *testing.T) { + s, _ := newTestStore(t) + now := time.UnixMilli(20 * time.Hour.Milliseconds()) + seedDemonic(t, s, now.Add(-2*time.Hour).UnixMilli()) + // Reported truthfully, then the Site published one more. + sight(t, s, s.OwnerID(), demonicKey, 295, now.Add(-10*time.Minute)) + + f := &fakeFetcher{body: demonicFixture(296), status: 200} + p := newTestPoller(t, s, f, now) + // One rest on, the Sighting has lapsed and the Poll happens. + p.Now = func() time.Time { return now.Add(7 * time.Hour) } + p.runOnce(context.Background()) + if got := f.callCount(); got != 1 { + t.Fatalf("fetched %d times past the ceiling, want 1", got) + } + + // Unmarked, so a fresh Sighting still defers. + at := now.Add(9 * time.Hour) + sight(t, s, s.OwnerID(), demonicKey, 296, at.Add(-time.Minute)) + p.Now = func() time.Time { return at } + p.runOnce(context.Background()) + if got := f.callCount(); got != 1 { + t.Fatalf("a Reader whose report the Site overtook lost the right to defer (fetches %d, want 1)", got) + } +} diff --git a/backend/internal/latest/sites.go b/backend/internal/latest/sites.go index 0b7182d..e824909 100644 --- a/backend/internal/latest/sites.go +++ b/backend/internal/latest/sites.go @@ -409,6 +409,13 @@ const ( // asleep (ADR-0005 on-demand browser). browserWakeCount = 5 browserWakeAge = 15 * time.Minute + // sightingCeiling caps Sighting deferral (issue #103): however many + // Sightings arrive, a Series unpolled for this long is Polled. It is what + // makes a client report safe to trust — a wrong Latest Chapter dies within + // the ceiling deterministically, rather than in expectation the way a + // randomised audit would have it. Six rests, so a Series a Reader visits + // constantly still gets one authoritative check per working day-part. + sightingCeiling = 6 * defaultRest ) // effectiveGap is a Site's pace: the registry gap, or one rest divided by the diff --git a/backend/internal/store/migrations/0011_series_sightings.sql b/backend/internal/store/migrations/0011_series_sightings.sql new file mode 100644 index 0000000..c647dcc --- /dev/null +++ b/backend/internal/store/migrations/0011_series_sightings.sql @@ -0,0 +1,10 @@ +-- Sighting deferral (issue #103). latest_sighted_at is when a Reader's report +-- last stood in for a Poll; it is separate from latest_checked_at because the +-- six-hour ceiling has to know when the Series was last really fetched, and a +-- Sighting writing the Poll's own column would erase that. +-- latest_raised_by is attribution: whoever last raised this Series' Latest +-- Chapter by Sighting, so a Poll that contradicts the value downwards names a +-- Reader rather than flagging a row. Cleared by the Poll that judges it, NULL +-- whenever the stored value is the Poll's own. +ALTER TABLE series ADD COLUMN latest_sighted_at bigint NOT NULL DEFAULT 0; +ALTER TABLE series ADD COLUMN latest_raised_by bigint REFERENCES readers(id) ON DELETE SET NULL; diff --git a/backend/internal/store/store.go b/backend/internal/store/store.go index 7df73aa..80e83bb 100644 --- a/backend/internal/store/store.go +++ b/backend/internal/store/store.go @@ -79,6 +79,11 @@ type Series struct { LatestChapter string LatestChapterNum *float64 // nil until first captured LatestCheckedAt int64 // unix ms; see MarkLatestChecked + // LatestRaisedBy is the Reader whose Sighting last raised LatestChapter, + // and nil when the stored value is a Poll's own finding. It is what lets a + // Poll that contradicts the value downwards name a Reader instead of + // merely flagging the row (issue #103); the Poll that judges it clears it. + LatestRaisedBy *int64 // readerCount is the number of bookmarks referencing this series, filled // only by the due-queue query that orders on it. @@ -195,7 +200,7 @@ const bookmarkColumns = `b.site, b.series_id, s.title, s.series_url, s.cover_add // due query. latest_checked_at lives only on series — see MarkLatestChecked // for why it stays off every client-visible write. const seriesColumns = `s.site, s.series_id, s.title, s.series_url, s.cover, s.cover_address, - s.kind, s.latest_chapter, s.latest_chapter_num, s.latest_checked_at` + s.kind, s.latest_chapter, s.latest_chapter_num, s.latest_checked_at, s.latest_raised_by` // Owner is the person running the service: the first Reader, seeded at startup // so a fresh deployment has a library before anyone logs in. The seed makes @@ -585,15 +590,17 @@ func (s *Store) scanBookmark(scan func(...any) error) (Bookmark, error) { // scanSeries reads one row in seriesColumns order, plus the due query's // reader_count column. latest_chapter_num is NULL until the first capture, -// same as on the bookmark read path. +// same as on the bookmark read path, and latest_raised_by is NULL whenever no +// Sighting is currently answerable for the stored value. func scanSeries(scan func(...any) error) (Series, error) { var ( sr Series latestChapterNum sql.NullFloat64 + latestRaisedBy sql.NullInt64 ) if err := scan( &sr.Site, &sr.SeriesID, &sr.Title, &sr.SeriesURL, &sr.Cover, &sr.CoverAddress, - &sr.Kind, &sr.LatestChapter, &latestChapterNum, &sr.LatestCheckedAt, + &sr.Kind, &sr.LatestChapter, &latestChapterNum, &sr.LatestCheckedAt, &latestRaisedBy, &sr.readerCount, ); err != nil { return Series{}, err @@ -601,6 +608,9 @@ func scanSeries(scan func(...any) error) (Series, error) { if latestChapterNum.Valid { sr.LatestChapterNum = &latestChapterNum.Float64 } + if latestRaisedBy.Valid { + sr.LatestRaisedBy = &latestRaisedBy.Int64 + } return sr, nil } @@ -947,7 +957,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(site string, cutoffMs int64) ([]Series, error) { +// +// ceilingMs is the Sighting deferral ceiling (issue #103): a Series whose last +// real Poll is older than it appears however recently it was sighted. That is +// what bounds the whole mechanism — a wrong Latest Chapter dies within the +// ceiling deterministically rather than in expectation. Deferral itself is +// decided here, from two facts the query already computes, so a Lane gains no +// query per round: a Sighting younger than cutoffMs holds the Series back, but +// only while COUNT(*) is 1. A Series a second Reader bookmarks is Polled on +// schedule, so a wrong value the whole guild can see is corrected by a check +// that was never postponed; on a solitary Series the only person a wrong value +// reaches is the Reader who reported it. Whether the reporting Reader is +// allowed to defer at all was settled when the Sighting was recorded — see +// RecordSighting. +func (s *Store) DueForLatestCheck(site string, cutoffMs, ceilingMs int64) ([]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 @@ -957,7 +980,10 @@ func (s *Store) DueForLatestCheck(site string, cutoffMs int64) ([]Series, error) 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`, site, cutoffMs) + AND (COUNT(*) > 1 + OR s.latest_sighted_at <= $2::bigint + OR s.latest_checked_at <= $3::bigint) + ORDER BY reader_count DESC, s.latest_checked_at ASC`, site, cutoffMs, ceilingMs) if err != nil { return nil, fmt.Errorf("query due series: %w", err) } @@ -1044,3 +1070,94 @@ func (s *Store) SetLatestChapter(site, seriesID, label string, num float64) erro } return nil } + +// RecordSighting notes that a Reader's browser reported this Series' Latest +// Chapter, which is the half of a Sighting the client body cannot express +// (issue #103). It must be called *before* the Upsert that stores the reported +// value: the raise test compares against what is still on the row, and after +// the Upsert there is nothing left to compare with. A Series that does not +// exist yet — the first Bookmark of it — is not a Sighting at all: nothing has +// ever been Polled, so there is nothing to defer and nobody to attribute. +// +// Two independent effects, hence the two CASE arms. The deferral stamp is only +// written for a Reader below the disagreement limit, so a marked Reader's +// reports keep updating the Latest Chapter but stop postponing anything, and +// clearing their marks restores the privilege on their next Sighting. The +// attribution is written whenever the report raises the stored number, +// including for a marked Reader — their Sightings are still judged, which is +// how they earn the privilege back. +// +// num is the reported chapter number, nil when the client sent none. +func (s *Store) RecordSighting(readerID int64, site, seriesID string, num *float64, ts int64) error { + var reported any + if num != nil { + reported = *num + } + if _, err := s.db.Exec(` + UPDATE series SET + latest_sighted_at = CASE + WHEN (SELECT sighting_disagreements FROM readers WHERE id = $3) < $6 + THEN $4::bigint ELSE latest_sighted_at END, + latest_raised_by = CASE + WHEN $5::double precision IS NOT NULL + AND (latest_chapter_num IS NULL OR $5::double precision > latest_chapter_num) + THEN $3::bigint ELSE latest_raised_by END + WHERE site = $1 AND series_id = $2`, + site, seriesID, readerID, ts, reported, SightingDisagreementLimit); err != nil { + return fmt.Errorf("record sighting %s:%s: %w", site, seriesID, err) + } + return nil +} + +// SightingAgreementsToClear is how many Polls must confirm a Reader's +// Sightings in a row before their disagreements are forgiven. An agreement is +// only recorded when a Poll later confirms a Sighting, so this is twenty Polls +// of Series that Reader bookmarks — hours to days, not twenty page views. That +// is the intended price: recovery is automatic but cannot be outwaited, and a +// disagreement resets the run to zero, so credit cannot be banked in advance. +const SightingAgreementsToClear = 20 + +// RecordSightingOutcome settles what a Poll decided about the Reader whose +// Sighting last raised this Series' Latest Chapter, and clears the attribution +// in the same transaction so one Sighting is judged exactly once. agreed is +// the Poll confirming the stored value; its opposite is the Poll finding a +// lower number, which means the raise was false. +// +// A Poll finding a *higher* number is neither — the Site published — and must +// not reach this at all. +func (s *Store) RecordSightingOutcome(site, seriesID string, readerID int64, agreed bool) error { + tx, err := s.db.Begin() + if err != nil { + return fmt.Errorf("begin sighting outcome %s:%s: %w", site, seriesID, err) + } + defer tx.Rollback() + + // The run length is what "consecutive" means: a disagreement zeroes the + // agreements, and completing a run zeroes both, so the next run starts + // from nothing rather than forgiving every later disagreement instantly. + q := `UPDATE readers SET sighting_disagreements = sighting_disagreements + 1, + sighting_agreements = 0 + WHERE id = $1` + args := []any{readerID} + if agreed { + q = `UPDATE readers SET + sighting_agreements = CASE WHEN sighting_agreements + 1 >= $2 THEN 0 + ELSE sighting_agreements + 1 END, + sighting_disagreements = CASE WHEN sighting_agreements + 1 >= $2 THEN 0 + ELSE sighting_disagreements END + WHERE id = $1` + args = append(args, SightingAgreementsToClear) + } + if _, err := tx.Exec(q, args...); err != nil { + return fmt.Errorf("record sighting outcome for reader %d: %w", readerID, err) + } + if _, err := tx.Exec( + `UPDATE series SET latest_raised_by = NULL WHERE site = $1 AND series_id = $2`, + site, seriesID); err != nil { + return fmt.Errorf("clear sighting attribution %s:%s: %w", site, seriesID, err) + } + if err := tx.Commit(); err != nil { + return fmt.Errorf("commit sighting outcome %s:%s: %w", site, seriesID, err) + } + return nil +} diff --git a/backend/internal/store/store_test.go b/backend/internal/store/store_test.go index 0ae24b2..1b4a83b 100644 --- a/backend/internal/store/store_test.go +++ b/backend/internal/store/store_test.go @@ -277,6 +277,10 @@ func seedForCheck(t *testing.T, s *Store, key, seriesURL string, checkedAt int64 } } +// noCeiling is a Sighting deferral ceiling no Series can reach, for the tests +// that predate the ceiling and are about rest, ordering or buckets instead. +const noCeiling = int64(-1) + func TestDueForLatestCheck(t *testing.T) { const hour = int64(3600_000) now := 10 * hour @@ -298,7 +302,7 @@ func TestDueForLatestCheck(t *testing.T) { s := newTestStore(t) seedForCheck(t, s, "asura:x", tt.seriesURL, tt.checkedAt) - due, err := s.DueForLatestCheck("asura", now-hour) + due, err := s.DueForLatestCheck("asura", now-hour, noCeiling) if err != nil { t.Fatalf("DueForLatestCheck: %v", err) } @@ -319,7 +323,7 @@ func TestDueForLatestCheckOldestFirstAndScopedToSite(t *testing.T) { // asks for one Site, and no Lane may see another's queue. seedForCheck(t, s, "demonic:z", "https://demonicscans.org/manga/z", 0) - due, err := s.DueForLatestCheck("asura", 1000) + due, err := s.DueForLatestCheck("asura", 1000, noCeiling) if err != nil { t.Fatalf("DueForLatestCheck: %v", err) } @@ -499,7 +503,7 @@ func TestDueForLatestCheckSkipsFinishedKeepsArchived(t *testing.T) { } } - due, err := store.DueForLatestCheck("asura", time.Now().UnixMilli()) + due, err := store.DueForLatestCheck("asura", time.Now().UnixMilli(), noCeiling) if err != nil { t.Fatalf("DueForLatestCheck: %v", err) } @@ -1005,7 +1009,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("asura", 1000) + due, err := s.DueForLatestCheck("asura", 1000, noCeiling) if err != nil { t.Fatalf("DueForLatestCheck: %v", err) } @@ -1031,7 +1035,7 @@ func TestDueForLatestCheckExcludesOrphanSeries(t *testing.T) { t.Fatalf("seed orphan series: %v", err) } - due, err := s.DueForLatestCheck("asura", 1000) + due, err := s.DueForLatestCheck("asura", 1000, noCeiling) if err != nil { t.Fatalf("DueForLatestCheck: %v", err) } @@ -1449,7 +1453,7 @@ func TestTwoReadersShareOneSeriesWithIndependentProgress(t *testing.T) { t.Fatalf("series rows = %d, want 1 shared row for two bookmarks", series) } - due, err := s.DueForLatestCheck("asura", time.Now().UnixMilli()) + due, err := s.DueForLatestCheck("asura", time.Now().UnixMilli(), noCeiling) if err != nil { t.Fatalf("DueForLatestCheck: %v", err) } @@ -1465,7 +1469,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("asura", time.Now().UnixMilli()) + due, err = s.DueForLatestCheck("asura", time.Now().UnixMilli(), noCeiling) if err != nil { t.Fatalf("DueForLatestCheck after delete: %v", err) } diff --git a/docs/adr/0011-sighting-deferral-trust-model.md b/docs/adr/0011-sighting-deferral-trust-model.md new file mode 100644 index 0000000..f1bd70c --- /dev/null +++ b/docs/adr/0011-sighting-deferral-trust-model.md @@ -0,0 +1,106 @@ +# ADR-0011: Sightings — a Reader report defers a Poll where being wrong hurts only them + +Date: 2026-08-16 +Status: accepted + +## Decision + +A **Sighting** is what a userscript PUT already carries: the Latest Chapter the +Reader's own browser read off the Series page. It is now allowed to stand in for +a Poll, under one restriction and one ceiling: + +- **Solitary Series only.** A Sighting defers the Poll of a Series exactly one + Bookmark points at. A Series two Readers share is Polled on schedule no matter + how recently it was sighted. +- **One rest of standing.** A Sighting postpones Polls for one Rest + (`defaultRest`, an hour), not forever: a Series nobody visits again returns to + the normal schedule by itself. +- **Six-hour ceiling.** `sightingCeiling = 6 * defaultRest`. However many + Sightings arrive, a Series unpolled for six hours is Polled. + +Both live in the due query's HAVING clause (`store.DueForLatestCheck`), beside +the Rest cutoff — the same place the schedule has always been decided, so no +timer and no second code path can disagree with it. + +Attribution and judgement: + +- `Store.RecordSighting` runs *before* the Upsert that stores the reported + value, because the raise test needs the row as it stands. A report that raises + the stored Latest Chapter names its Reader in `series.latest_raised_by`. +- The Poll is the oracle. `Poller.checkOne` already compares what the Site + publishes against what is stored, so judgement costs no extra request: a lower + number contradicts the Sighting (`sighting_disagreements + 1`, both numbers and + the Reader logged), the same number confirms it (`sighting_agreements + 1`), a + **higher** number is the Site publishing and means nothing either way. +- At `SightingDisagreementLimit` (3) that Reader's Sightings stop deferring + anything. They still write the Latest Chapter — the penalty removes a + privilege, it does not silence anyone. +- `SightingAgreementsToClear` (20) consecutive confirmations forgive the + disagreements. A disagreement resets the run to zero. +- The owner clears marks from the administration page (issue #102, shipped + first precisely so a false mark has a remedy the day the mechanism lands). + +## Why + +Most of the backend's work was redundant. The userscript reads the Latest +Chapter on every Series page visit and PUTs it; minutes later the Poll Lane +fetches the same page for the same number. Deferring on a report converts a +visit into a Poll saved, which is Lane capacity handed back to Series nobody is +reading. + +The restriction is the whole safety argument, and it is about **blast radius**, +not about trust arithmetic: + +- On a solitary Series, a wrong report can only mislead the Reader who made it. + There is nobody else's ember to falsify. +- On a shared Series it could mislead someone else, so a report never postpones + anything there. + +The ceiling bounds the damage in time: a false value dies within six hours +whatever happens, because the Poll that finds it is guaranteed. That is also +what makes lying pointless — the six-hour audit is certain, not sampled, so a +determined attacker buys at most three ceilings' worth of a wrong number on +their own Series and then loses deferral entirely. + +The cost of recovery is deliberate. An agreement is only recorded when a later +Poll confirms a Sighting, so twenty agreements are twenty Polls of Series that +Reader bookmarks — hours to days of real time, not twenty page views. Waiting is +therefore not a strategy, and credit cannot be banked in advance. + +## Tradeoffs and rejections + +- **Trusting a Sighting on a shared Series** rejected: it is the only case where + one Reader's mistake reaches another Reader's list, and no amount of + reputation makes that recoverable within the six-hour window. +- **Cross-Reader agreement, voting, weighting, consensus scoring** rejected: a + single-source report cannot have its confidence evaluated by comparison, and + with the typical two Readers a disagreement identifies nothing. The Poll is + the only oracle in the system, so it is the only judge. +- **A randomised audit** (Poll a fraction of deferred Series) rejected in favour + of the fixed ceiling: a sampled audit makes the attacker's expected cost a + probability, while a ceiling makes it a certainty, and a certainty is what + makes the solitary-Series rule defensible in one sentence. +- **Blocking a marked Reader's writes** rejected: the Latest Chapter they report + is still the best available value, and their Sightings must keep being judged + or they could never earn the privilege back. +- **Per-Series flagging** rejected in favour of per-Reader marks: a Series is + not the thing that can be wrong. Naming the Reader and logging both numbers is + also what distinguishes a broken Site adapter (every Reader of that Site + contradicted at once) from one bad actor. +- **Timers or a background reputation job** rejected: deferral is decided from + live facts every round — Bookmark count, sighting timestamp, the Reader's + marks — so a Series that gains a second Bookmark stops deferring at once, with + nothing to invalidate. + +## Constraints preserved + +- A Sighting is not Progress: it may move the Latest Chapter and nothing else. + `updated_at` never moves, so a report cannot reorder the list (ADR-0004). +- The Latest Chapter is a Series-level fact (ADR-0003): a Sighting writes the + shared row, so every Reader of a shared Series sees it immediately — deferral + is the only thing the solitary rule withholds. +- Ember means new chapter only (`docs/design-system.md`): a marked Reader + renders no differently in their own list, and nothing about the trust model + reaches the Series list's colour. +- The Poll remains authoritative. Where a Sighting and a Poll disagree, the + Poll's value is what gets stored.