From 5a2c1585aee3dd3c5dece21eae5f42f2c820ab8c Mon Sep 17 00:00:00 2001 From: John O'Keefe Date: Wed, 14 Oct 2026 13:23:55 -0400 Subject: [PATCH] =?UTF-8?q?refactor(stats):=20'Web=20/=20unsynced'=20bucke?= =?UTF-8?q?t=20label=20=E2=86=92=20'Web';=20analytics=20tests=20isolated?= =?UTF-8?q?=20from=20the=20live=20reader?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bucket name read as confusing — 'unsynced' implied data loss when the push is always captured; it only lacks a device credential. The bucket is just 'Web' now (service fallback, stats.ts fallback, panel copy, tests). Also: analytics_test exact-zero assertions (EmptyHistory, NoReadingHistory) ran against testuser, which is a LIVE reader — real sessions accrued on it mid-suite and both flapped. They now run under the fresh regular user, like the stats tests; the 'testuser is a live reader' rule is recorded in AGENTS.md. --- AGENTS.md | 8 ++++++++ cmd/server/tests/analytics_test.go | 11 +++++++++-- cmd/server/tests/statistics_test.go | 22 +++++++++++++--------- internal/services/statistics_service.go | 8 ++++---- templates/stats.templ | 2 +- templates/stats_templ.go | 2 +- web/src/stats.ts | 2 +- 7 files changed, 37 insertions(+), 18 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index bab1045e..747a9dc5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -46,6 +46,14 @@ were name-only across ALL libraries. Fixed; do not regress. renamed admin escapes the cleanup preserve-list and cascades real data (this wiped the dev annotations once; see user_test.go TestUpdateProfileSelfEdit for the dedicated-user pattern). +- **`testuser` is a LIVE reader**: the dev account genuinely reads in + the web reader while the suite runs, so `reading_history` / + `reading_progress` rows accrue on it mid-test. Exact-total + assertions against that account flap (three stats/analytics tests + broke on 2026-10-14 within hours of session capture shipping). + Tests that need deterministic data use `setup.RegularToken` + + `getRegularUserID` (the fresh per-test user), or assert ranges + instead of equals. ## Timezones diff --git a/cmd/server/tests/analytics_test.go b/cmd/server/tests/analytics_test.go index 07f977e9..d0e85999 100644 --- a/cmd/server/tests/analytics_test.go +++ b/cmd/server/tests/analytics_test.go @@ -93,8 +93,12 @@ func TestAnalyticsReadingStats(t *testing.T) { }) t.Run("GetReadingStats_EmptyHistory", func(t *testing.T) { + // The REGULAR user, deliberately: the shared dev DB carries a + // live reader on `testuser` whose real sessions accrue even + // while the suite runs, so "empty history" only holds for a + // freshly-created account. req, _ := http.NewRequest("GET", setup.Server.URL+"/api/analytics/reading-stats", nil) - req.Header.Set("Authorization", "Bearer "+setup.Token) + req.Header.Set("Authorization", "Bearer "+setup.RegularToken) resp, err := client.Do(req) require.NoError(t, err) @@ -351,8 +355,11 @@ func TestAnalyticsPopularBooks(t *testing.T) { }) t.Run("GetPopularBooks_NoReadingHistory", func(t *testing.T) { + // The REGULAR user, deliberately: the live dev-reader's sessions + // accrue on `testuser` while the suite runs, so an empty-history + // assertion only holds for a fresh account. req, _ := http.NewRequest("GET", setup.Server.URL+"/api/analytics/popular-books", nil) - req.Header.Set("Authorization", "Bearer "+setup.Token) + req.Header.Set("Authorization", "Bearer "+setup.RegularToken) resp, err := client.Do(req) require.NoError(t, err) diff --git a/cmd/server/tests/statistics_test.go b/cmd/server/tests/statistics_test.go index 1e2f07b9..d299b940 100644 --- a/cmd/server/tests/statistics_test.go +++ b/cmd/server/tests/statistics_test.go @@ -40,10 +40,10 @@ func middayDaysAgo(daysAgo int) time.Time { return midnight.AddDate(0, 0, -daysAgo).Add(12 * time.Hour) } -func statsLogin(t *testing.T, setup *TestServerSetup, path string) (*http.Response, []byte) { +func statsLogin(t *testing.T, setup *TestServerSetup, token, path string) (*http.Response, []byte) { t.Helper() req, _ := http.NewRequest("GET", setup.Server.URL+path, nil) - req.Header.Set("Authorization", "Bearer "+setup.Token) + req.Header.Set("Authorization", "Bearer "+token) resp, err := http.DefaultClient.Do(req) require.NoError(t, err) defer func() { @@ -218,7 +218,11 @@ func TestReadingStatsDeviceAttribution(t *testing.T) { func TestReadingStatsSummary(t *testing.T) { setup := setupTestServer(t) - userID := getTestUserID(t, setup.DB) + // The REGULAR test user, deliberately: the shared dev DB carries a + // live reader on `testuser` (the dev account genuinely reads while + // the suite runs), and exact-total assertions cannot share an + // account with reality. The fresh regular user has no live pushes. + userID := getRegularUserID(t, setup.DB) ctx := context.Context(context.Background()) libIDStr := createLibraryDirect(t, setup.DB, userID, fmt.Sprintf("Test Stats Library %d", time.Now().UnixNano()), "ebooks") @@ -290,7 +294,7 @@ func TestReadingStatsSummary(t *testing.T) { offRangeBookID = insertSession(400, 120, 40, 0.2, "Stats OffRange Book", false) // Summary ----------------------------------------------------------- - resp, body := statsLogin(t, setup, "/api/stats/summary?range=7") + resp, body := statsLogin(t, setup, setup.RegularToken, "/api/stats/summary?range=7") require.Equal(t, http.StatusOK, resp.StatusCode) var summary struct { @@ -373,7 +377,7 @@ func TestReadingStatsSummary(t *testing.T) { assert.NotEqual(t, "", dev.DeviceID, "device bucket carries its id") assert.Equal(t, "koreader", dev.Type) } - if dev.Name == "Web / unsynced" { + if dev.Name == "Web" { unattributedFound = true assert.Equal(t, 75, dev.Minutes, "30 + 45 unattributed sessions") assert.Equal(t, 2, dev.Sessions) @@ -381,7 +385,7 @@ func TestReadingStatsSummary(t *testing.T) { } } assert.True(t, kindleFound, "the registered device has its own bucket") - assert.True(t, unattributedFound, "device-less pushes aggregate into 'Web / unsynced'") + assert.True(t, unattributedFound, "device-less pushes aggregate into the Web bucket") deviceMinutes := 0 for _, dev := range summary.DeviceBreakdown { deviceMinutes += dev.Minutes @@ -389,7 +393,7 @@ func TestReadingStatsSummary(t *testing.T) { assert.Equal(t, 155, deviceMinutes, "device buckets reconcile with the total") // Range validation --------------------------------------------------- - badResp, badBody := statsLogin(t, setup, "/api/stats/summary?range=fortnight") + badResp, badBody := statsLogin(t, setup, setup.RegularToken, "/api/stats/summary?range=fortnight") assert.Equal(t, http.StatusBadRequest, badResp.StatusCode) assert.Contains(t, string(badBody), "invalid range") @@ -400,7 +404,7 @@ func TestReadingStatsSummary(t *testing.T) { _ = anonym.Body.Close() // Widget ------------------------------------------------------------- - resp, body = statsLogin(t, setup, "/api/stats/this-week") + resp, body = statsLogin(t, setup, setup.RegularToken, "/api/stats/this-week") require.Equal(t, http.StatusOK, resp.StatusCode) var week struct { WeekMinutes int `json:"week_minutes"` @@ -417,7 +421,7 @@ func TestReadingStatsSummary(t *testing.T) { assert.Equal(t, 0, week.PrevMinutes) // Per-book card (lifetime, so the 400-day book still totals) ----------- - resp, body = statsLogin(t, setup, "/api/stats/books/"+offRangeBookID) + resp, body = statsLogin(t, setup, setup.RegularToken, "/api/stats/books/"+offRangeBookID) require.Equal(t, http.StatusOK, resp.StatusCode) var bookStats struct { TotalMinutes int `json:"total_minutes"` diff --git a/internal/services/statistics_service.go b/internal/services/statistics_service.go index 14998e79..5a2f1881 100644 --- a/internal/services/statistics_service.go +++ b/internal/services/statistics_service.go @@ -115,8 +115,8 @@ type LibraryStats struct { } type DeviceStats struct { - // DeviceID is empty for the unattributed bucket (web reader pushes - // that carry no device credential) — Name then reads "Web / unsynced". + // DeviceID is empty for the web bucket (pushes that carry no device + // credential — browser reading sessions) — Name then reads "Web". DeviceID string `json:"device_id"` Name string `json:"name"` Type string `json:"type"` @@ -410,8 +410,8 @@ func (s *StatisticsService) UserSummary(ctx context.Context, userID pgtype.UUID, Pages: int(row.TotalPages), Sessions: int(row.SessionCount), Books: int(row.BookCount), - Name: "Web / unsynced", - Type: "unattributed", + Name: "Web", + Type: "web", } if row.DeviceName.Valid && row.DeviceName.String != "" { entry.Name = row.DeviceName.String diff --git a/templates/stats.templ b/templates/stats.templ index c2f5ab55..13f7d5c4 100644 --- a/templates/stats.templ +++ b/templates/stats.templ @@ -77,7 +77,7 @@ templ Stats(user User) {

By Device

-

Where the sync pushes came from — registered device names, or "Web / unsynced" when the push carried no device credential.

+

Where the sync pushes came from — registered device names, plus a Web bucket for pushes that carry no device credential (the web reader).

Loading…
diff --git a/templates/stats_templ.go b/templates/stats_templ.go index 1aa06838..2761352d 100644 --- a/templates/stats_templ.go +++ b/templates/stats_templ.go @@ -58,7 +58,7 @@ func Stats(user User) templ.Component { if templ_7745c5c3_Err != nil { return templ_7745c5c3_Err } - templ_7745c5c3_Err = templruntime.WriteString(templ_7745c5c3_Buffer, 4, ") · totals derive from your devices' sync pushes, not a stopwatch.

Reading Time

–

Pages

–

Streaks

–

Books Finished

–

Most Active

–

Daily Reading Minutes

Pages per Day

By Library

Loading…

Most Read

Loading…

No reading sessions recorded in this range yet.

Sessions are captured every time a device syncs reading progress (KOReader, the app, the web reader).

") + templ_7745c5c3_Err = templruntime.WriteString(templ_7745c5c3_Buffer, 4, ") · totals derive from your devices' sync pushes, not a stopwatch.

Reading Time

–

Pages

–

Streaks

–

Books Finished

–

Most Active

–

Daily Reading Minutes

Pages per Day

By Library

Loading…

By Device

Where the sync pushes came from — registered device names, plus a Web bucket for pushes that carry no device credential (the web reader).

Loading…

Most Read

Loading…

No reading sessions recorded in this range yet.

Sessions are captured every time a device syncs reading progress (KOReader, the app, the web reader).

") if templ_7745c5c3_Err != nil { return templ_7745c5c3_Err } diff --git a/web/src/stats.ts b/web/src/stats.ts index 43c1c798..bd9390f0 100644 --- a/web/src/stats.ts +++ b/web/src/stats.ts @@ -254,7 +254,7 @@ function renderDevices(rows: DeviceRow[]): void { const container = document.getElementById("stats-device-breakdown"); if (!container) return; renderBars(container, rows.map((r) => ({ - label: r.name || "Web / unsynced", + label: r.name || "Web", sub: `${r.books} ${r.books === 1 ? "book" : "books"} · ${r.sessions} sessions`, minutes: r.minutes, })));