diff --git a/backend/internal/latest/poller.go b/backend/internal/latest/poller.go index d2d2422..fff24dc 100644 --- a/backend/internal/latest/poller.go +++ b/backend/internal/latest/poller.go @@ -719,6 +719,25 @@ func (p *Poller) checkOne(ctx context.Context, sr store.Series) (outcome readOut } else { p.fillBlankCover(ctx, sr, facts.Cover) } + + // Learned from the successful read: the write sits after the error switch + // (a refused, unreachable or errored read reaches nothing) and before the + // returns below — a completed page whose chapter number did not change + // still has to write. The transition is zero-versus-nonzero, not the + // stamp's value: a Series still completed keeps its original stamp, so the + // age #170 prints is "since the Site first said so"; one that stopped + // being completed is zeroed. + stamp := int64(0) + if facts.SiteCompleted { + stamp = p.Now().UnixMilli() + } + if (sr.SiteCompletedAt == 0) != (stamp == 0) { + if err := p.Store.SetSiteCompletedAt(sr.Site, sr.SeriesID, stamp); err != nil { + // Best-effort, like every poller write: never change the outcome + // word the pass counts. + log.Printf("latest poll %q: set site completed: %v", sr.Key(), err) + } + } if !facts.HasLatest { // Most likely a challenge page or a layout change. Either way the row is // already stamped, so this waits out a rest instead of hot-looping. diff --git a/backend/internal/latest/poller_test.go b/backend/internal/latest/poller_test.go index 0fbacfa..d87b534 100644 --- a/backend/internal/latest/poller_test.go +++ b/backend/internal/latest/poller_test.go @@ -2725,6 +2725,7 @@ func TestRunOnceForcedPassReclaimFailureDoesNotFailPoll(t *testing.T) { t.Fatalf("Cover = %q, want the replacement %q", got.Cover, want) } } + // failureRow reads one Series' failure row as stored, for the poller tests // that must see the table the store API writes (ADR-0016). func failureRow(t *testing.T, dbURL, site, seriesID string) (word string, since int64, found bool) { @@ -2940,3 +2941,154 @@ func TestPollFailureDoesNotChangePacing(t *testing.T) { t.Fatalf("second pass fetched %d series, want both again", got) } } + +// readSiteCompletedAt reads the poller's site_completed_at column directly: +// it is a poller fact with no client-visible getter, and #170 is the surface +// that will read it. +func readSiteCompletedAt(t *testing.T, dbURL, site, seriesID string) int64 { + t.Helper() + db, err := sql.Open("pgx", dbURL) + if err != nil { + t.Fatalf("open %s: %v", dbURL, err) + } + defer db.Close() + var at int64 + if err := db.QueryRow( + `SELECT site_completed_at FROM series WHERE site = $1 AND series_id = $2`, + site, seriesID).Scan(&at); err != nil { + t.Fatalf("read site_completed_at %s:%s: %v", site, seriesID, err) + } + return at +} + +// seedSiteCompletedAt writes the column directly, for the refused/unreachable +// tests that need a pre-existing stamp. +func seedSiteCompletedAt(t *testing.T, dbURL, site, seriesID string, at int64) { + t.Helper() + db, err := sql.Open("pgx", dbURL) + if err != nil { + t.Fatalf("open %s: %v", dbURL, err) + } + defer db.Close() + if _, err := db.Exec( + `UPDATE series SET site_completed_at = $3 WHERE site = $1 AND series_id = $2`, + site, seriesID, at); err != nil { + t.Fatalf("seed site_completed_at %s:%s: %v", site, seriesID, err) + } +} + +// A completed page stamps site_completed_at with the pass's own clock; the +// same page an hour later is due again but must not move the stamp — the age +// #170 prints is "since the Site first said so", not the age of the last +// Poll. The transition is zero-versus-nonzero, so the exact value asserted +// here is load-bearing. +func TestRunOnceSiteCompletedStampsOnce(t *testing.T) { + s, dbURL := newTestStore(t) + const ( + key = "asura:chronicles-of-the-demon-faction-f886a8af" + seriesID = "chronicles-of-the-demon-faction-f886a8af" + seriesURL = "https://asurascans.com/comics/chronicles-of-the-demon-faction-f886a8af" + ) + completed := asuraSeriesFixture + asuraCompletedFixture + at := time.UnixMilli(5_000_000) + seedForCheck(t, s, key, seriesURL, at.Add(-time.Hour).UnixMilli()) + + newTestPoller(t, s, &fakeFetcher{body: completed, status: 200}, at).runOnce(context.Background()) + if got := readSiteCompletedAt(t, dbURL, "asura", seriesID); got != at.UnixMilli() { + t.Fatalf("site_completed_at after completed poll = %d, want %d", got, at.UnixMilli()) + } + + hourLater := at.Add(time.Hour) + newTestPoller(t, s, &fakeFetcher{body: completed, status: 200}, hourLater).runOnce(context.Background()) + if got := readSiteCompletedAt(t, dbURL, "asura", seriesID); got != at.UnixMilli() { + t.Fatalf("site_completed_at after a second completed poll = %d, want the original stamp %d", got, at.UnixMilli()) + } +} + +// An ongoing page is the inverse transition: a never-hinted Series stays zero +// (an ordinary Poll of an ongoing Series writes nothing), and a hinted one is +// zeroed — the Site no longer says completed, so the claim must not outlive +// the evidence. +func TestRunOnceOngoingPageClearsOrSkipsSiteCompleted(t *testing.T) { + s, dbURL := newTestStore(t) + const ( + key = "asura:chronicles-of-the-demon-faction-f886a8af" + seriesID = "chronicles-of-the-demon-faction-f886a8af" + seriesURL = "https://asurascans.com/comics/chronicles-of-the-demon-faction-f886a8af" + ) + ongoing := asuraSeriesFixture + asuraOngoingFixture + at := time.UnixMilli(5_000_000) + seedForCheck(t, s, key, seriesURL, at.Add(-time.Hour).UnixMilli()) + + newTestPoller(t, s, &fakeFetcher{body: ongoing, status: 200}, at).runOnce(context.Background()) + if got := readSiteCompletedAt(t, dbURL, "asura", seriesID); got != 0 { + t.Fatalf("site_completed_at after ongoing poll of a never-hinted series = %d, want 0", got) + } + + seedSiteCompletedAt(t, dbURL, "asura", seriesID, at.UnixMilli()) + hourLater := at.Add(time.Hour) + newTestPoller(t, s, &fakeFetcher{body: ongoing, status: 200}, hourLater).runOnce(context.Background()) + if got := readSiteCompletedAt(t, dbURL, "asura", seriesID); got != 0 { + t.Fatalf("site_completed_at after ongoing poll of a hinted series = %d, want 0", got) + } +} + +// A refused or unreachable read reaches nothing after checkOne's error switch: +// a challenge is not evidence about the work, so a pre-seeded stamp must +// survive both. +func TestRunOnceRefusedOrUnreachableLeavesSiteCompletedUntouched(t *testing.T) { + now := time.UnixMilli(5_000_000) + + t.Run("refused", func(t *testing.T) { + s, dbURL := newTestStore(t) + const ( + key = "asura:chronicles-of-the-demon-faction-f886a8af" + seriesID = "chronicles-of-the-demon-faction-f886a8af" + seriesURL = "https://asurascans.com/comics/chronicles-of-the-demon-faction-f886a8af" + ) + seedForCheck(t, s, key, seriesURL, 0) + seedSiteCompletedAt(t, dbURL, "asura", seriesID, now.UnixMilli()) + newTestPoller(t, s, &fakeFetcher{status: 403}, now).runOnce(context.Background()) + if got := readSiteCompletedAt(t, dbURL, "asura", seriesID); got != now.UnixMilli() { + t.Fatalf("site_completed_at after refused poll = %d, want the pre-seeded %d", got, now.UnixMilli()) + } + }) + + t.Run("mid-loop unreachable", func(t *testing.T) { + s, dbURL := newTestStore(t) + seedForCheck(t, s, "comix:c", "https://comix.to/title/c", 0) + seedSiteCompletedAt(t, dbURL, "comix", "c", now.UnixMilli()) + interrupted := fmt.Errorf("%w: %w", errBrowserInterrupted, errors.New("restart")) + browser := &fakeFetcher{status: 200, err: interrupted} + p := newTestPoller(t, s, &fakeFetcher{status: 200}, now) + p.BrowserFetch = browser + p.runLanePass(context.Background(), "comix", false) + if got := readSiteCompletedAt(t, dbURL, "comix", "c"); got != now.UnixMilli() { + t.Fatalf("site_completed_at after unreachable poll = %d, want the pre-seeded %d", got, now.UnixMilli()) + } + }) +} + +// The regression AC2's second sentence exists to prevent: the completed write +// must not ride the chapter setter, so a completed page whose chapter number +// is unchanged — checkOne's equality early return — still stamps the column. +func TestRunOnceSiteCompletedWritesDespiteUnchangedChapter(t *testing.T) { + s, dbURL := newTestStore(t) + const ( + key = "asura:chronicles-of-the-demon-faction-f886a8af" + seriesID = "chronicles-of-the-demon-faction-f886a8af" + seriesURL = "https://asurascans.com/comics/chronicles-of-the-demon-faction-f886a8af" + ) + completed := asuraSeriesFixture + asuraCompletedFixture + at := time.UnixMilli(5_000_000) + seedForCheck(t, s, key, seriesURL, 0) + // The page parses Chapter 181 as the latest; store the same number so the + // equality early return fires and only the completed write can run. + if err := s.SetLatestChapter("asura", seriesID, "Chapter 181", 181); err != nil { + t.Fatalf("seed latest chapter: %v", err) + } + newTestPoller(t, s, &fakeFetcher{body: completed, status: 200}, at).runOnce(context.Background()) + if got := readSiteCompletedAt(t, dbURL, "asura", seriesID); got != at.UnixMilli() { + t.Fatalf("site_completed_at after unchanged-chapter completed poll = %d, want %d", got, at.UnixMilli()) + } +} diff --git a/backend/internal/latest/read.go b/backend/internal/latest/read.go index 8a9c611..dc51b04 100644 --- a/backend/internal/latest/read.go +++ b/backend/internal/latest/read.go @@ -6,15 +6,19 @@ import ( "fmt" ) -// seriesRead carries the two facts the poll and the acquirer both extract -// from a series page. Persistence, stamps and scheduling stay with the -// callers, so the policies that keep the two flows distinct (stamp order, -// rests) are not swallowed by the module. +// seriesRead carries the facts the poll and the acquirer both extract from a +// series page. Persistence, stamps and scheduling stay with the callers, so +// the policies that keep the two flows distinct (stamp order, rests) are not +// swallowed by the module. type seriesRead struct { Latest latestChapter HasLatest bool Cover string HasCover bool + // SiteCompleted is whether the Site's own completed value was on the page. + // A challenge body and a redesign both read false — an absent hint, never + // a claim (issue #168). + SiteCompleted bool // BodyLen is the fetched body's length, surfaced because the no-chapter // log uses it to tell a markup change from a body the size cap cut short. BodyLen int @@ -35,8 +39,8 @@ var ( // readSeriesPage performs the series-page read the poll and the acquirer have // in common: gate the address, choose the route, fetch the page, extract the -// Latest Chapter and the Cover address. It persists nothing and stamps -// nothing. +// Latest Chapter, the Cover address and the Site's completed value. It +// persists nothing and stamps nothing. // // series_url arrives in a client-supplied PUT body (PUT /bookmarks/{key} // accepts any string), so the gate is not an optimisation against burning a @@ -75,5 +79,12 @@ func readSeriesPage(ctx context.Context, site, seriesURL string, browser, tls Fe } latest, hasLatest := latestChapterFrom(site, seriesURL, body) cover, hasCover := coverFrom(site, seriesURL, body) - return seriesRead{Latest: latest, HasLatest: hasLatest, Cover: cover, HasCover: hasCover, BodyLen: len(body)}, nil + return seriesRead{ + Latest: latest, + HasLatest: hasLatest, + Cover: cover, + HasCover: hasCover, + SiteCompleted: siteCompletedFrom(site, seriesURL, body), + BodyLen: len(body), + }, nil } diff --git a/backend/internal/store/migrations/0019_series_site_completed.sql b/backend/internal/store/migrations/0019_series_site_completed.sql new file mode 100644 index 0000000..75783cb --- /dev/null +++ b/backend/internal/store/migrations/0019_series_site_completed.sql @@ -0,0 +1,4 @@ +-- When the last successful Poll read saw the Site's own completed value +-- (#168): epoch-ms, zero meaning it did not. DEFAULT 0 keeps pre-existing +-- rows readable. +ALTER TABLE series ADD COLUMN site_completed_at bigint NOT NULL DEFAULT 0; diff --git a/backend/internal/store/store.go b/backend/internal/store/store.go index bab6b61..1f348f3 100644 --- a/backend/internal/store/store.go +++ b/backend/internal/store/store.go @@ -84,6 +84,10 @@ type Series struct { LatestChapter string LatestChapterNum *float64 // nil until first captured LatestCheckedAt int64 // unix ms; see MarkLatestChecked + // SiteCompletedAt is when the last successful Poll read saw the Site's + // own completed value, zero meaning it did not (issue #168). A poller + // fact, not reading progress: Upsert never touches it. + SiteCompletedAt int64 // 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 @@ -240,7 +244,8 @@ 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.latest_raised_by` + s.kind, s.latest_chapter, s.latest_chapter_num, s.latest_checked_at, s.latest_raised_by, + s.site_completed_at` const lanePassColumns = `p.site, p.ran_at, p.skip, p.due, p.checked, p.gap_ms, p.clamped, p.refused, p.unreachable, p.no_chapter, p.unfetchable, p.errors, p.not_found, @@ -646,7 +651,7 @@ func scanSeries(scan func(...any) error) (Series, error) { if err := scan( &sr.Site, &sr.SeriesID, &sr.Title, &sr.SeriesURL, &sr.Cover, &sr.CoverAddress, &sr.Kind, &sr.LatestChapter, &latestChapterNum, &sr.LatestCheckedAt, &latestRaisedBy, - &sr.Forced, &sr.readerCount, + &sr.SiteCompletedAt, &sr.Forced, &sr.readerCount, ); err != nil { return Series{}, err } @@ -1382,7 +1387,7 @@ func (s *Store) DueForLatestCheck(site string, cutoffMs, ceilingMs int64) ([]Ser AND (s.finished_at = 0 OR s.force_poll_at > s.latest_checked_at) 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, - s.force_poll_at, s.finished_at + s.site_completed_at, s.force_poll_at, s.finished_at HAVING (COUNT(*) > 1 OR s.latest_sighted_at <= $2::bigint OR s.latest_checked_at <= $3::bigint @@ -1517,6 +1522,22 @@ func (s *Store) SetLatestChapter(site, seriesID, label string, num float64) erro return nil } +// SetSiteCompletedAt stores when the last successful read saw the Site's +// completed value, zero meaning it did not. Series-level, like the Latest +// Chapter: it is a fact about the work, not about one Reader's bookmark, and +// the bookmark's updated_at is never touched — this is not reading progress +// and must not reorder any Reader's list, the same rule SetLatestChapter's +// comment states. Touching a missing series is not an error: the row may +// have been orphaned, and the caller's read decides what exists. +func (s *Store) SetSiteCompletedAt(site, seriesID string, at int64) error { + if _, err := s.db.Exec( + `UPDATE series SET site_completed_at = $1 WHERE site = $2 AND series_id = $3`, + at, site, seriesID); err != nil { + return fmt.Errorf("set site completed %s:%s: %w", site, seriesID, err) + } + return nil +} + // SetSeriesURL stores the owner's repair for a Series' source address // (issue #151): the one write that lifts the write-once rule documented on // Series.SeriesURL. It is a store, not a verification — the caller has