review: verdigris patina, honest Lane figures, tests that can fail (#102)

Two-axis review found the shipped colour and three weak tests.

- --patina was a warm gold at the same hue family as --brass; the spec
  asked for a cool blue-green so an unhealthy Lane is unmistakably not
  ember. Now verdigris in both branches, with docs/design-system.md
  stating why the far side of the wheel is the point.
- A Lane pass that returns before computing its figures carries the
  previous pass's due count and gap forward instead of recording zeroes,
  and Checked rides beside Due so a stopped Lane is distinguishable from
  a quiet one.
- TestAdminPageWithoutAPollerSaysSo now separates the two causes it
  conflated, TestOwnerClearsReaderMarks seeds real counters so the
  clearing assertion can fail, and TestLaneStatus asserts Checked and
  the carry-forward.
- backend/AGENTS.md records the one deliberate owner comparison outside
  requireOwner and the carry-forward rule.
This commit is contained in:
2026-08-16 14:31:00 +07:00
parent d878580df8
commit 58014eb8dd
10 changed files with 203 additions and 61 deletions
+9
View File
@@ -224,3 +224,12 @@ Guidance for OpenCode (and Claude Code) working under `backend/`. See root `AGEN
rather than zeroes. `main.newRouter` takes the reporter as an interface and
converts a nil `*Poller` to a nil interface — a typed nil would make the page
claim a poller exists.
The one owner comparison left outside `requireOwner` is in `index`
(`view.Owner = readerID == h.store.OwnerID()`): it gates a link, not an
endpoint, so it is a rendering decision a registration-time wrapper cannot
express — do not "unify" it into the gate.
A Lane pass that returns before computing its figures (refusal backoff,
sidecar down) carries the previous pass's due count and gap forward rather
than recording zeroes; a Lane that has never reached a pace renders no gap at
all. `Checked` next to `Due` is what separates a stopped Lane from a quiet
one, so neither figure may be dropped from the row.
+9 -5
View File
@@ -246,19 +246,23 @@ func (p *Poller) runLanePass(ctx context.Context, name string, paced bool) time.
// No fetcher at all right now (browser absent, no fallback): every
// Series stays unstamped and due, so a browser that appears after a
// restart finds its full queue waiting (issue #100).
st.Gap = defaultGap
return defaultGap
}
due, err := p.Store.DueForLatestCheck(name, now.Add(-s.Rest).UnixMilli())
if err != nil {
log.Printf("latest poll %s: due query: %v", name, err)
st.Gap = defaultGap
return defaultGap
}
st.Due = len(due)
if s.Browser != nil && f == p.BrowserFetch && !browserWakeDue(due, now, s.Rest) {
// Below both thresholds Chrome stays asleep (ADR-0005 on-demand
// browser): waking it for a single Poll would cost a challenge solve
// per request.
// per request. The Lane still paces at the default gap, which is what
// the owner's page must show rather than a zero.
st.Gap = defaultGap
return defaultGap
}
if s.Browser != nil {
@@ -274,6 +278,7 @@ func (p *Poller) runLanePass(ctx context.Context, name string, paced bool) time.
eligible, err := p.Store.EligibleSeriesCount(name)
if err != nil {
log.Printf("latest poll %s: eligible count: %v", name, err)
st.Gap = defaultGap
return defaultGap
}
gap, clamped := effectiveGap(s, eligible)
@@ -288,7 +293,6 @@ func (p *Poller) runLanePass(ctx context.Context, name string, paced bool) time.
}
refusals := 0
checked := 0
for i, sr := range due {
if ctx.Err() != nil {
break
@@ -323,10 +327,10 @@ func (p *Poller) runLanePass(ctx context.Context, name string, paced bool) time.
} else {
refusals = 0
}
checked++
st.Checked++
}
if checked > 0 {
log.Printf("latest poll %s: due=%d checked=%d", name, len(due), checked)
if st.Checked > 0 {
log.Printf("latest poll %s: due=%d checked=%d", name, len(due), st.Checked)
}
if refusals >= 2 {
p.setRefusalBackoff(name, now.Add(refuseBackoff))
+21
View File
@@ -1640,6 +1640,27 @@ func TestLaneStatus(t *testing.T) {
if !st.BrowserConfigured || !st.BrowserReachable {
t.Fatalf("browser after round = configured=%v reachable=%v, want true/true (sidecar never lost)", st.BrowserConfigured, st.BrowserReachable)
}
if asura.Checked != 1 {
t.Fatalf("asura Checked = %d, want the one Series it read", asura.Checked)
}
// A pass that declines to look (kagane is now in backoff) must not restate
// the figures it never gathered as zeroes: the last real pass's due count
// and pace stand until a pass replaces them.
before := kagane
if before.Due == 0 || before.Gap == 0 {
t.Fatalf("kagane after its refusing pass = %+v, want the figures that pass gathered", before)
}
p.runOnce(context.Background())
for _, lane := range p.LaneStatus().Lanes {
if lane.Site != "kagane" {
continue
}
if lane.Due != before.Due || lane.Gap != before.Gap {
t.Fatalf("kagane after a skipped pass = due %d gap %s, want the previous pass's %d / %s",
lane.Due, lane.Gap, before.Due, before.Gap)
}
}
// A lost sidecar reads as unreachable for the same window the Lanes skip.
p.setBrowserDown(now)
+18 -7
View File
@@ -3,14 +3,19 @@ package latest
import "time"
// LaneState is the administrative page's view of one Poll Lane (issue #102):
// what the Lane's last completed pass saw. Due and Gap are filled in as the
// pass computes them, so a pass that returned before reaching a figure (Lane
// in refusal backoff, no fetcher) records a zero in its place.
// what the Lane's last pass saw. Due, Gap and Checked are filled in as the
// pass computes them; a pass that returned before reaching a figure (refusal
// backoff, sidecar down) carries the previous pass's figures forward rather
// than overwriting them with zeroes the page would state as fact.
type LaneState struct {
Site string
Due int
LastRun time.Time
Gap time.Duration
Site string
Due int
LastRun time.Time
Gap time.Duration
// Checked is how many Series this pass actually read. A Lane with Series
// due and nothing checked has stopped working; one with nothing due is
// merely quiet, and the page must not draw the two the same (story 13).
Checked int
Clamped bool
Refusing bool
Browser bool
@@ -53,11 +58,17 @@ func (p *Poller) LaneStatus() Status {
// recordLaneState stores one Lane's last pass for LaneStatus. Called deferred
// from runLanePass so every return path records, even a pass that refused.
// A pass that never reached the pace (Gap zero) keeps the last pass's figures:
// the Lane's due count and gap did not become zero because this pass declined
// to look, and the row's own marks say why it declined.
func (p *Poller) recordLaneState(st LaneState) {
p.mu.Lock()
defer p.mu.Unlock()
if p.laneStates == nil {
p.laneStates = make(map[string]LaneState)
}
if prev, ok := p.laneStates[st.Site]; ok && st.Gap == 0 {
st.Due, st.Gap, st.Clamped, st.Checked = prev.Due, prev.Gap, prev.Clamped, prev.Checked
}
p.laneStates[st.Site] = st
}
+29 -7
View File
@@ -31,7 +31,11 @@ type adminView struct {
// browser fact, which is shared by the three browser Sites rather than held
// once per Site.
type lanesView struct {
Rows []laneRow
Rows []laneRow
// PollerOff means no poller is running at all (disabled by config, or its
// client could not be built). The browser line must not answer "not
// configured" then: the sidecar is not the reason nothing is polled.
PollerOff bool
BrowserConfigured bool
BrowserReachable bool
}
@@ -40,9 +44,15 @@ type lanesView struct {
// template renders strings and flags, and every judgement about what they mean
// is made here.
type laneRow struct {
Site string
Due int
Ran string
Site string
Due int
Ran string
// Checked is how many Series the last pass read. Due without Checked is a
// Lane that has stopped working; the two figures side by side are what
// separate that from a Lane with nothing to do.
Checked int
// Gap is empty when no pass has reached the pace yet, so the row omits the
// figure instead of stating a zero.
Gap string
Clamped bool
Refusing bool
@@ -50,6 +60,9 @@ type laneRow struct {
// sidecar while the sidecar is unreachable — including the case where none
// is configured, which stops those Series just as completely.
BrowserLost bool
// Stalled marks a Lane with Series waiting that its last pass did not read
// — the difference between a stopped Lane and a quiet one (story 13).
Stalled bool
// Attention is the one flag the template colours on, so an unhealthy Lane
// is found at a glance rather than read for.
Attention bool
@@ -127,7 +140,7 @@ func (h *Handler) uiLanes(w http.ResponseWriter, r *http.Request) {
// — an empty page a few seconds after a restart must not read as a stopped one.
func (h *Handler) lanesView() lanesView {
if h.lanes == nil {
return lanesView{}
return lanesView{PollerOff: true}
}
snap := h.lanes.LaneStatus()
v := lanesView{
@@ -138,15 +151,24 @@ func (h *Handler) lanesView() lanesView {
now := time.Now()
for _, l := range snap.Lanes {
lost := l.Browser && !snap.BrowserReachable
// Series waiting and none read is the shape of a Lane that has stopped
// working, as distinct from one that is quiet for want of work.
stalled := l.Due > 0 && l.Checked == 0
gap := ""
if l.Gap > 0 {
gap = l.Gap.Truncate(time.Second).String()
}
v.Rows = append(v.Rows, laneRow{
Site: l.Site,
Due: l.Due,
Ran: since(now, l.LastRun),
Gap: l.Gap.Truncate(time.Second).String(),
Checked: l.Checked,
Gap: gap,
Clamped: l.Clamped,
Refusing: l.Refusing,
BrowserLost: lost,
Attention: l.Clamped || l.Refusing || lost,
Stalled: stalled,
Attention: l.Clamped || l.Refusing || lost || stalled,
})
}
return v
+6 -5
View File
@@ -87,10 +87,11 @@
--moss: #7fae86; /* finished */
--clay: #b5906f; /* set chapter */
--trash: #977671; /* remove, resting — icons need 3:1, not 4.5:1 */
/* A Lane needing attention: the admin page's only accent, held at the same
muted weight as --brass so it never competes with ember. Neither ember
(new chapter) nor danger (destruction) may say "system unhealthy". */
--patina: #b08a4a;
/* A Lane needing attention: the admin page's only accent. Verdigris — cool,
the far side of the wheel from ember's crimson, and clear of the archive
blue. Neither ember (new chapter) nor danger (destruction) may say
"system unhealthy". */
--patina: #5fb3a6;
/* Desktop cell borders for the two coloured action states. */
--play-hot-line: #3a1d18;
@@ -150,7 +151,7 @@
--moss: #3d6c46;
--clay: #7c5533;
--trash: #8c6558;
--patina: #7a5a1e;
--patina: #1f6f66;
--play-hot-line: #f0cfc6;
--fav-line: #e3d3a4;
+4 -1
View File
@@ -27,7 +27,10 @@
</form>
</header>
{{template "lanes" .Lanes}}
{{/* The live region wraps the swapped block rather than being it: the
refresh replaces the section wholesale, and a region recreated on every
update is never announced. */}}
<div aria-live="polite">{{template "lanes" .Lanes}}</div>
{{template "readers" .}}
</div>
+9 -4
View File
@@ -7,7 +7,7 @@
a Site absent from Rows has not completed a pass since the last restart,
which the empty state must say — zeroes would read as a stopped Lane. */}}
{{define "lanes"}}
<section class="lanes" id="lanes" aria-live="polite"
<section class="lanes" id="lanes"
hx-get="/ui/admin/lanes" hx-trigger="every 30s" hx-swap="outerHTML">
<h2>Poll Lanes</h2>
{{if .Rows}}
@@ -16,11 +16,13 @@
<li{{if .Attention}} class="attention"{{end}}>
<span class="lane-site">{{.Site}}</span>
<span class="lane-fact">{{.Due}} due</span>
<span class="lane-fact">{{.Checked}} checked</span>
<span class="lane-fact">ran {{.Ran}}</span>
<span class="lane-fact">gap {{.Gap}}</span>
{{if .Gap}}<span class="lane-fact">gap {{.Gap}}</span>{{end}}
{{if .Clamped}}<span class="lane-mark">gap at floor</span>{{end}}
{{if .Refusing}}<span class="lane-mark">refusing</span>{{end}}
{{if .BrowserLost}}<span class="lane-mark">no browser</span>{{end}}
{{if .Stalled}}<span class="lane-mark">not checking</span>{{end}}
</li>
{{end}}
</ul>
@@ -28,9 +30,12 @@
<p class="setup-copy">No data yet — no Lane has completed a pass since the
backend started.</p>
{{end}}
<p class="setup-copy lane-browser">Browser sidecar:
<p class="setup-copy lane-browser">
{{if .PollerOff}}Polling is switched off in this deployment: no Lane runs,
and Latest Chapter comes from the userscripts alone.
{{else}}Browser sidecar:
{{if not .BrowserConfigured}}not configured — comix, kagane and novelfull
pages are not fetched through it{{else if .BrowserReachable}}reachable
{{else}}unreachable{{end}}.</p>
{{else}}unreachable{{end}}.{{end}}</p>
</section>
{{end}}
+94 -29
View File
@@ -1,6 +1,7 @@
package main
import (
"database/sql"
"encoding/json"
"fmt"
"io"
@@ -737,16 +738,19 @@ func TestAdminRoutesAreOwnerOnly(t *testing.T) {
}
// The Lane block reports what the poller says, and marks the Lanes that need
// attention — a clamped gap, a refusal, or a Site whose pages can only be read
// through a sidecar that is not there.
// attention — a clamped gap, a refusal, a Site whose pages can only be read
// through a sidecar that is not there, and a Lane with Series waiting that its
// last pass did not read.
func TestAdminPageShowsLaneStatus(t *testing.T) {
lanes := fakeLanes{latest.Status{
Lanes: []latest.LaneState{
{Site: "asura", Due: 12, LastRun: time.Now().Add(-90 * time.Second), Gap: 40 * time.Second},
{Site: "kagane", Due: 3, LastRun: time.Now().Add(-time.Minute), Gap: time.Minute, Browser: true},
{Site: "demonic", Due: 400, LastRun: time.Now(), Gap: 8 * time.Second, Clamped: true},
{Site: "asura", Due: 12, Checked: 12, LastRun: time.Now().Add(-90 * time.Second), Gap: 40 * time.Second},
{Site: "kagane", Due: 3, Checked: 3, LastRun: time.Now().Add(-time.Minute), Gap: time.Minute, Browser: true},
{Site: "demonic", Due: 400, Checked: 400, LastRun: time.Now(), Gap: 8 * time.Second, Clamped: true},
{Site: "comix", Due: 7, LastRun: time.Now(), Gap: time.Minute, Browser: true},
},
BrowserConfigured: true,
BrowserReachable: true,
}}
router, st, _ := oauthWebTestServer(t, lanes)
@@ -758,54 +762,115 @@ func TestAdminPageShowsLaneStatus(t *testing.T) {
t.Fatalf("GET /ui/admin/lanes status = %d, want 200", rr.Code)
}
body := rr.Body.String()
for _, want := range []string{"asura", "kagane", "12 due", "gap 40s", "ran 1m30s ago", "gap at floor", "no browser", "unreachable"} {
for _, want := range []string{"asura", "kagane", "12 due", "12 checked", "gap 40s", "ran 1m30s ago", "gap at floor", "not checking", "reachable"} {
if !strings.Contains(body, want) {
t.Errorf("lane status lacks %q:\n%s", want, body)
}
}
// Two Lanes need attention: the clamped one and the one cut off from the
// sidecar. The healthy Lane must not be marked.
if n := strings.Count(body, `class="attention"`); n != 2 {
t.Errorf("attention rows = %d, want 2:\n%s", n, body)
// Nothing is refusing and the sidecar is up, so neither mark may appear:
// a mark the owner cannot act on is worse than none.
for _, unwanted := range []string{"refusing", "no browser"} {
if strings.Contains(body, unwanted) {
t.Errorf("lane status marks %q on a healthy run:\n%s", unwanted, body)
}
}
}
// No poller and a poller that has not finished a pass are the same to the page:
// it says so rather than drawing zeroes that read as a stopped backend.
func TestAdminPageWithoutAPollerSaysSo(t *testing.T) {
router, st, _ := oauthWebTestServer(t)
req := httptest.NewRequest(http.MethodGet, "/admin", nil)
// A Lane whose pass never reached a figure must not have that figure drawn as
// a zero: a refusing Lane still reports the due count and gap its last real
// pass saw, and a Lane that has never reached one omits it entirely.
func TestLaneStatusOmitsUnknownGap(t *testing.T) {
lanes := fakeLanes{latest.Status{
Lanes: []latest.LaneState{{Site: "comix", LastRun: time.Now(), Refusing: true, Browser: true}},
BrowserConfigured: true,
BrowserReachable: true,
}}
router, st, _ := oauthWebTestServer(t, lanes)
req := httptest.NewRequest(http.MethodGet, "/ui/admin/lanes", nil)
req.AddCookie(sessionCookie(t, st))
rr := httptest.NewRecorder()
router.ServeHTTP(rr, req)
body := rr.Body.String()
if !strings.Contains(body, "No data yet") {
t.Errorf("admin page with no poller does not say so:\n%s", body)
if strings.Contains(body, "gap 0s") {
t.Errorf("a Lane with no pace yet states a zero gap:\n%s", body)
}
if !strings.Contains(body, "not configured") {
t.Errorf("admin page does not report the missing browser sidecar:\n%s", body)
if !strings.Contains(body, "refusing") {
t.Errorf("a refusing Lane is not marked as such:\n%s", body)
}
}
// Clearing a Reader's marks answers with the whole roster, so the page cannot
// keep showing the record that was just wiped.
// No poller and a poller that has not finished a pass both render "no data
// yet" rather than zeroes that read as a stopped backend — but they are not
// the same fact, so the page must not blame the sidecar when nothing polls.
func TestAdminPageWithoutAPollerSaysSo(t *testing.T) {
for _, tc := range []struct {
name string
lanes []web.LaneReporter
want, unwant string
}{
{"no poller", nil, "Polling is switched off", "not configured"},
{"poller, no pass yet", []web.LaneReporter{fakeLanes{}}, "not configured", "Polling is switched off"},
} {
t.Run(tc.name, func(t *testing.T) {
router, st, _ := oauthWebTestServer(t, tc.lanes...)
req := httptest.NewRequest(http.MethodGet, "/admin", nil)
req.AddCookie(sessionCookie(t, st))
rr := httptest.NewRecorder()
router.ServeHTTP(rr, req)
body := rr.Body.String()
if !strings.Contains(body, "No data yet") {
t.Errorf("admin page with no Lane data does not say so:\n%s", body)
}
if !strings.Contains(body, tc.want) {
t.Errorf("admin page lacks %q:\n%s", tc.want, body)
}
if strings.Contains(body, tc.unwant) {
t.Errorf("admin page states %q, which is not what is wrong:\n%s", tc.unwant, body)
}
})
}
}
// A Reader past the disagreement threshold is rendered as blocked, and
// clearing their marks both zeroes the counters and lifts the block in the
// roster the response carries back.
func TestOwnerClearsReaderMarks(t *testing.T) {
router, st, _ := oauthWebTestServer(t)
theirCookie := signInCookie(t, router)
their, _, err := st.GetSession(theirCookie.Value, time.Now())
st, dsn := newTestStoreURL(t)
router := newRouter(st, testConfig(), nil)
cookie := sessionCookie(t, st)
// The counters are filled by issue #103; until it lands the only way to
// stand a marked Reader up is to write the columns directly.
db, err := sql.Open("pgx", dsn)
if err != nil {
t.Fatalf("GetSession: %v", err)
t.Fatalf("open %s: %v", dsn, err)
}
defer db.Close()
if _, err := db.Exec(`UPDATE readers SET sighting_agreements = 4, sighting_disagreements = 3 WHERE id = $1`, st.OwnerID()); err != nil {
t.Fatalf("mark reader: %v", err)
}
req := httptest.NewRequest(http.MethodPost,
"/readers/"+strconv.FormatInt(their.ReaderID, 10)+"/clear-marks", nil)
req.AddCookie(sessionCookie(t, st))
req := httptest.NewRequest(http.MethodGet, "/admin", nil)
req.AddCookie(cookie)
rr := httptest.NewRecorder()
router.ServeHTTP(rr, req)
body := rr.Body.String()
if !strings.Contains(body, "4 confirmed / 3 contradicted") {
t.Errorf("roster does not report the Reader's marks:\n%s", body)
}
if !strings.Contains(body, "deferral blocked") {
t.Errorf("a Reader at the threshold is not rendered as blocked:\n%s", body)
}
req = httptest.NewRequest(http.MethodPost,
"/readers/"+strconv.FormatInt(st.OwnerID(), 10)+"/clear-marks", nil)
req.AddCookie(cookie)
rr = httptest.NewRecorder()
router.ServeHTTP(rr, req)
if rr.Code != http.StatusOK {
t.Fatalf("clear marks: status = %d, want 200 (body %s)", rr.Code, rr.Body.String())
}
body := rr.Body.String()
body = rr.Body.String()
if !strings.Contains(body, `id="readers"`) {
t.Fatalf("clear marks did not re-render the roster:\n%s", body)
}
+4 -3
View File
@@ -73,7 +73,7 @@ Defined once in `backend/internal/web/static/style.css` `:root`, mirrored in the
| `--moss` | `#7fae86` | `#3d6c46` | finished accent |
| `--clay` | `#b5906f` | `#7c5533` | set-chapter accent |
| `--trash` | `#977671` | `#8c6558` | remove, at rest — icons need 3:1, not 4.5:1 |
| `--patina` | `#b08a4a` | `#7a5a1e` | admin page only — a Poll Lane needing attention, a Reader whose reports are blocked |
| `--patina` | `#5fb3a6` | `#1f6f66` | admin page only — a Poll Lane needing attention, a Reader whose reports are blocked |
| `--play-hot-line` | `#3a1d18` | `#f0cfc6` | desktop cell border, play when `.is-new` |
| `--fav-line` | `#332b14` | `#e3d3a4` | desktop cell border, favourite when on |
| `--asura` | `#7d93a5` | `#4f6b80` | site tag |
@@ -85,8 +85,9 @@ Defined once in `backend/internal/web/static/style.css` `:root`, mirrored in the
`--slate`/`--moss`/`--clay`/`--brass`/`--patina` are held at the same weight
deliberately: one accent per meaning, so a press says which lane it belongs to,
with none of them competing with ember. `--patina` is the admin page's only
colour — system health is neither a new chapter nor destruction, so it borrows
neither `--ember` nor `--danger`.
colour — a cool verdigris, the far side of the wheel from ember's crimson and
clear of the archive blue: system health is neither a new chapter nor
destruction, so it borrows neither `--ember` nor `--danger`.
Dark is the default (`color-scheme: dark light`);
light is a `@media (prefers-color-scheme: light)` override of the same names.
**Any new colour must be added in both branches** — light is not a filter over