review: clear stale attribution, gate numberless PUTs, cite the ADR
A Poll finding a higher number now clears series.latest_raised_by: the value it stores is its own, so the next Poll must not credit that Reader with an agreement they did not earn, nor charge them for a retraction of a number they never reported. A PUT carrying no chapter is no Sighting and defers nothing. The ceiling now counts the Site's own rest rather than defaultRest. The userscripts PUT an unchanged read too - the case the whole mechanism exists for was the one they never sent. ADR-0011 carries its citations and states honestly when a Reader's marks take effect.
This commit is contained in:
+20
-12
@@ -79,7 +79,9 @@ Guidance for OpenCode (and Claude Code) working under `backend/`. See root `AGEN
|
||||
each re-checking that Site's bookmarked series' newest published chapter from
|
||||
backend's own network access, so `latest_chapter` stays fresh when the user
|
||||
isn't browsing. Second, parallel signal — the userscript keeps its own
|
||||
`maybeCaptureLatestOnSeriesPage`/`backgroundRefreshLatest` logic unchanged.
|
||||
`maybeCaptureLatestOnSeriesPage`/`backgroundRefreshLatest` schedule, and its
|
||||
`reportLatestChapter` PUTs every read, unchanged numbers included, because an
|
||||
unchanged read is exactly the Sighting worth deferring a Poll on (#103).
|
||||
Two independent clocks: per-series rest (`series.latest_checked_at`,
|
||||
enforced by `Store.DueForLatestCheck`'s WHERE clause — `now - Rest`) and
|
||||
per-Lane gap (the Lane sleeping between fetches, `effectiveGap`). Both live
|
||||
@@ -98,17 +100,23 @@ Guidance for OpenCode (and Claude Code) working under `backend/`. See root `AGEN
|
||||
`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.
|
||||
Bookmark, was sighted within one Rest, and is under the ceiling
|
||||
(`sightingCeilingRests`, six of that Site's 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 — that last one clears the attribution instead, since
|
||||
the value the Poll then stores is its own and a later retraction is not the
|
||||
Reader's fault. 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; the one
|
||||
input read earlier is the Reader's marks, checked when the Sighting is
|
||||
recorded, so crossing the threshold or being cleared takes effect from that
|
||||
Reader's next Sighting and the standing already bought lasts out its rest.
|
||||
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
|
||||
|
||||
@@ -250,8 +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(), now.Add(-sightingCeiling).UnixMilli())
|
||||
due, err := p.Store.DueForLatestCheck(name, now.Add(-s.Rest).UnixMilli(),
|
||||
now.Add(-sightingCeilingRests*s.Rest).UnixMilli())
|
||||
if err != nil {
|
||||
log.Printf("latest poll %s: due query: %v", name, err)
|
||||
st.Gap = defaultGap
|
||||
@@ -503,6 +503,13 @@ func (p *Poller) judgeSighting(sr store.Series, found float64) {
|
||||
}
|
||||
stored := *sr.LatestChapterNum
|
||||
if found > stored {
|
||||
// The report is neither confirmed nor contradicted, but it is answered:
|
||||
// the value about to be stored is the Poll's own, so leaving the
|
||||
// attribution would credit this Reader with the next Poll's agreement
|
||||
// and blame them if the Site later retracts.
|
||||
if err := p.Store.ClearSightingAttribution(sr.Site, sr.SeriesID, *sr.LatestRaisedBy); err != nil {
|
||||
log.Printf("latest poll %q: clear sighting attribution: %v", sr.Key(), err)
|
||||
}
|
||||
return
|
||||
}
|
||||
if found < stored {
|
||||
|
||||
@@ -189,7 +189,7 @@ func TestSightingCeilingForcesPoll(t *testing.T) {
|
||||
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)
|
||||
t.Fatalf("fetched %d times past the %s ceiling, want 1", got, sightingCeilingRests*defaultRest)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -439,3 +439,56 @@ func TestPollFindingHigherNumberIsNotAContradiction(t *testing.T) {
|
||||
t.Fatalf("a Reader whose report the Site overtook lost the right to defer (fetches %d, want 1)", got)
|
||||
}
|
||||
}
|
||||
|
||||
// A Poll that overtakes a Sighting takes ownership of the row: the value stored
|
||||
// afterwards is the Poll's own, so a later retraction is not the Reader's fault
|
||||
// and must not be charged to them.
|
||||
func TestAttributionDoesNotSurviveAPollThatOvertookIt(t *testing.T) {
|
||||
s, _ := newTestStore(t)
|
||||
now := time.UnixMilli(20 * time.Hour.Milliseconds())
|
||||
seedDemonic(t, s, now.Add(-2*time.Hour).UnixMilli())
|
||||
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)
|
||||
at := now.Add(defaultRest + time.Minute)
|
||||
p.Now = func() time.Time { return at }
|
||||
p.runOnce(context.Background())
|
||||
if got := readLatestNum(t, s, s.OwnerID(), demonicKey); got != 296 {
|
||||
t.Fatalf("latest after the Poll = %v, want the Site's own 296", got)
|
||||
}
|
||||
|
||||
var logs strings.Builder
|
||||
prev := log.Writer()
|
||||
log.SetOutput(&logs)
|
||||
t.Cleanup(func() { log.SetOutput(prev) })
|
||||
|
||||
f.body = demonicFixture(290) // the Site retracts what only the Poll wrote
|
||||
p.Now = func() time.Time { return at.Add(defaultRest + time.Minute) }
|
||||
p.runOnce(context.Background())
|
||||
if strings.Contains(logs.String(), "sighting contradicted") {
|
||||
t.Fatalf("a retraction of the Poll's own value was charged to a Reader: %s", logs.String())
|
||||
}
|
||||
}
|
||||
|
||||
// A PUT with no Latest Chapter in it — a favourite toggle, progress written
|
||||
// from a chapter page — is nobody looking at the Series page, so it buys no
|
||||
// deferral. Otherwise a client could suppress a Series' Polls while reporting
|
||||
// nothing, and with nothing reported there would be nothing to judge.
|
||||
func TestPutWithoutALatestChapterDoesNotDefer(t *testing.T) {
|
||||
s, _ := newTestStore(t)
|
||||
now := time.UnixMilli(20 * time.Hour.Milliseconds())
|
||||
seedDemonic(t, s, now.Add(-2*time.Hour).UnixMilli())
|
||||
// The handler's own call, with the field the client omitted.
|
||||
if err := s.RecordSighting(s.OwnerID(), "demonic", "Catastrophic-Necromancer",
|
||||
nil, now.Add(-time.Minute).UnixMilli()); err != nil {
|
||||
t.Fatalf("RecordSighting: %v", err)
|
||||
}
|
||||
|
||||
f := &fakeFetcher{body: demonicFixture(296), status: 200}
|
||||
p := newTestPoller(t, s, f, now)
|
||||
p.runOnce(context.Background())
|
||||
if got := f.callCount(); got != 1 {
|
||||
t.Fatalf("fetched %d times after a PUT carrying no chapter, want 1", got)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -409,13 +409,14 @@ 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
|
||||
// sightingCeilingRests caps Sighting deferral (issue #103): however many
|
||||
// Sightings arrive, a Series unpolled for this many of its Site's rests 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, so a Series a
|
||||
// Reader visits constantly still gets one authoritative check per working
|
||||
// day-part.
|
||||
sightingCeilingRests = 6
|
||||
)
|
||||
|
||||
// effectiveGap is a Site's pace: the registry gap, or one rest divided by the
|
||||
|
||||
@@ -589,9 +589,8 @@ 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, and latest_raised_by is NULL whenever no
|
||||
// Sighting is currently answerable for the stored value.
|
||||
// reader_count column. latest_chapter_num and latest_raised_by are both
|
||||
// nullable, same as latest_chapter_num on the bookmark read path.
|
||||
func scanSeries(scan func(...any) error) (Series, error) {
|
||||
var (
|
||||
sr Series
|
||||
@@ -1087,11 +1086,13 @@ func (s *Store) SetLatestChapter(site, seriesID, label string, num float64) erro
|
||||
// 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.
|
||||
// num is the reported chapter number. A PUT that carries none — a favourite
|
||||
// toggle, or progress written from a chapter page — is no Sighting at all:
|
||||
// nobody read the Series page, so there is nothing to stand in for a Poll and
|
||||
// nothing that could later be judged.
|
||||
func (s *Store) RecordSighting(readerID int64, site, seriesID string, num *float64, ts int64) error {
|
||||
var reported any
|
||||
if num != nil {
|
||||
reported = *num
|
||||
if num == nil {
|
||||
return nil
|
||||
}
|
||||
if _, err := s.db.Exec(`
|
||||
UPDATE series SET
|
||||
@@ -1099,11 +1100,10 @@ func (s *Store) RecordSighting(readerID int64, site, seriesID string, num *float
|
||||
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)
|
||||
WHEN 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 {
|
||||
site, seriesID, readerID, ts, *num, SightingDisagreementLimit); err != nil {
|
||||
return fmt.Errorf("record sighting %s:%s: %w", site, seriesID, err)
|
||||
}
|
||||
return nil
|
||||
@@ -1123,8 +1123,8 @@ const SightingAgreementsToClear = 20
|
||||
// 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.
|
||||
// A Poll finding a *higher* number is neither — the Site published — and takes
|
||||
// ClearSightingAttribution instead.
|
||||
func (s *Store) RecordSightingOutcome(site, seriesID string, readerID int64, agreed bool) error {
|
||||
tx, err := s.db.Begin()
|
||||
if err != nil {
|
||||
@@ -1151,9 +1151,7 @@ func (s *Store) RecordSightingOutcome(site, seriesID string, readerID int64, agr
|
||||
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 {
|
||||
if _, err := tx.Exec(clearAttributionSQL, site, seriesID, readerID); err != nil {
|
||||
return fmt.Errorf("clear sighting attribution %s:%s: %w", site, seriesID, err)
|
||||
}
|
||||
if err := tx.Commit(); err != nil {
|
||||
@@ -1161,3 +1159,21 @@ func (s *Store) RecordSightingOutcome(site, seriesID string, readerID int64, agr
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// ClearSightingAttribution answers a Sighting without judging it: the Poll
|
||||
// found a higher number, so the value about to be stored is its own and this
|
||||
// Reader is no longer answerable for the row. Without it the next Poll's
|
||||
// agreement would be credited to a Reader who did not earn it.
|
||||
func (s *Store) ClearSightingAttribution(site, seriesID string, readerID int64) error {
|
||||
if _, err := s.db.Exec(clearAttributionSQL, site, seriesID, readerID); err != nil {
|
||||
return fmt.Errorf("clear sighting attribution %s:%s: %w", site, seriesID, err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// clearAttributionSQL drops the attribution only while it still names the
|
||||
// Reader being judged: a Sighting landing between the due query's snapshot and
|
||||
// this write is a fresh, unjudged one and must not be erased by the previous
|
||||
// one's verdict.
|
||||
const clearAttributionSQL = `UPDATE series SET latest_raised_by = NULL
|
||||
WHERE site = $1 AND series_id = $2 AND latest_raised_by = $3`
|
||||
|
||||
Reference in New Issue
Block a user