Merge pull request #13831 from lokesh/librarian-card

Home page: librarian call-out and a compact stats card replace the five-chart stats block

Author: lokesh

openlibrary/admin/numbers.py

@@ -105,38 +105,64 @@ def _get_cached_bot_accounts(thingdb=None) -> list[int]:
 
 
 # Public functions that are used by stats.py
-def admin_range__human_edits(**kargs):
-    """Calculates the number of edit actions performed by humans
-    between the given `start` and `end` dates. `thingdb` is the database.
-    """
+def _count_transactions(db, start, end, where="", **vars):
+    "Transactions between `start` and `end`, narrowed by `where` (an SQL fragment using `vars`)."
+    sql = f"SELECT count(t.id) AS count FROM transaction t WHERE t.created >= $start and t.created < $end{where}"
+    return db.query(sql, vars={"start": start, "end": end, **vars})[0].count
+
+
+def _range_args(fn_name, kargs):
     try:
-        start = kargs["start"].strftime("%Y-%m-%d")
-        end = kargs["end"].strftime("%Y-%m-%d %H:%M:%S")
-        db = kargs["thingdb"]
+        return kargs["thingdb"], kargs["start"].strftime("%Y-%m-%d"), kargs["end"].strftime("%Y-%m-%d %H:%M:%S")
     except KeyError as k:
-        raise TypeError(f"{k} is a required argument for admin_range__human_edits")
-    bot_ids = _get_cached_bot_accounts(thingdb=kargs["thingdb"])
-    q1 = "SELECT count(t.id) AS count FROM transaction t WHERE t.created >= $start and t.created < $end AND t.author_id NOT IN $bot_ids"
-    result = db.query(q1, vars={"bot_ids": bot_ids, "start": start, "end": end})
-    count = result[0].count
-    return count
+        raise TypeError(f"{k} is a required argument for {fn_name}")
+
+
+def admin_range__human_edits(**kargs):
+    "Transactions by non-bot accounts between `start` and `end`, signups and all. See `admin_range__catalog_edits`."
+    db, start, end = _range_args("admin_range__human_edits", kargs)
+    bot_ids = _get_cached_bot_accounts(thingdb=db)
+    return _count_transactions(db, start, end, " AND t.author_id NOT IN $bot_ids", bot_ids=bot_ids)
 
 
 def admin_range__bot_edits(**kargs):
-    """Calculates the number of edit actions performed by bots between
-    the `start` and `end` dates. `thingdb` is the database.
-    """
-    try:
-        start = kargs["start"].strftime("%Y-%m-%d")
-        end = kargs["end"].strftime("%Y-%m-%d %H:%M:%S")
-        db = kargs["thingdb"]
-    except KeyError as k:
-        raise TypeError(f"{k} is a required argument for admin_range__bot_edits")
-    bot_ids = _get_cached_bot_accounts(thingdb=kargs["thingdb"])
-    q1 = "SELECT count(t.id) AS count FROM transaction t WHERE t.created >= $start and t.created < $end AND t.author_id IN $bot_ids"
-    result = db.query(q1, vars={"bot_ids": bot_ids, "start": start, "end": end})
-    count = result[0].count
-    return count
+    "Transactions by bot accounts between `start` and `end`."
+    db, start, end = _range_args("admin_range__bot_edits", kargs)
+    bot_ids = _get_cached_bot_accounts(thingdb=db)
+    return _count_transactions(db, start, end, " AND t.author_id IN $bot_ids", bot_ids=bot_ids)
+
+
+# Transactions that aren't edits to the catalog. Every signup writes a `new-account`
+# transaction, and at ~10k signups a day they outnumber real edits several times over.
+NON_EDIT_ACTIONS = (
+    "new-account",
+    "register",
+    "create-stats",
+    "edit-blocked-ips",
+    "edit-usergroup-add-member",
+    "edit-usergroup-delete-member",
+    "bulk-edit-permissions",
+)
+
+
+def admin_range__catalog_edits(**kargs):
+    """Human edits that change the catalog: `admin_range__human_edits` without signups and admin
+    housekeeping. A separate key so the daily docs written before it existed keep their meaning."""
+    db, start, end = _range_args("admin_range__catalog_edits", kargs)
+    bot_ids = _get_cached_bot_accounts(thingdb=db)
+    return _count_transactions(
+        db, start, end, " AND t.action NOT IN $non_edit_actions AND t.author_id NOT IN $bot_ids", bot_ids=bot_ids, non_edit_actions=NON_EDIT_ACTIONS
+    )
+
+
+# Someone making or changing a list; `edit-list-seeds` is a save of what's on it.
+LIST_EDIT_ACTIONS = ("create-list", "edit-list-seeds")
+
+
+def admin_range__list_edits(**kargs):
+    "Counts lists created or edited between `start` and `end`."
+    db, start, end = _range_args("admin_range__list_edits", kargs)
+    return _count_transactions(db, start, end, " AND t.action IN $list_edit_actions", list_edit_actions=LIST_EDIT_ACTIONS)
 
 
 def admin_range__covers(**kargs):

openlibrary/admin/tests/__init__.py

@@ -0,0 +1,61 @@
+from datetime import datetime
+
+import web
+
+from openlibrary.admin import numbers
+
+
+class FakeDB:
+    """Records the SQL `numbers` runs and answers every query with one row."""
+
+    def __init__(self, count=7):
+        self.count = count
+        self.queries = []
+
+    def query(self, q, vars=None):
+        self.queries.append((q, vars))
+        return [web.storage(count=self.count)]
+
+
+def run(fn, db):
+    numbers.cached_bot_accounts = [11, 12]
+    try:
+        return fn(thingdb=db, coverdb=None, logroot=None, start=datetime(2026, 10, 15), end=datetime(2026, 10, 15, 21, 0, 0))
+    finally:
+        numbers.cached_bot_accounts = None
+
+
+def test_edit_counts_split_humans_from_bots():
+    for fn, author_clause in [
+        (numbers.admin_range__human_edits, "t.author_id NOT IN (11, 12)"),
+        (numbers.admin_range__bot_edits, "t.author_id IN (11, 12)"),
+    ]:
+        db = FakeDB()
+        assert run(fn, db) == 7
+        [(q, vars)] = db.queries
+        sql = str(web.db.reparam(q, vars))
+        assert author_clause in sql
+        assert "t.action" not in sql
+        assert "t.created >= '2026-10-15' and t.created < '2026-10-15 21:00:00'" in sql
+
+
+def test_catalog_edits_skip_account_creation_and_bots():
+    db = FakeDB()
+    assert run(numbers.admin_range__catalog_edits, db) == 7
+    [(q, vars)] = db.queries
+    sql = str(web.db.reparam(q, vars))
+    assert "t.action NOT IN ('" in sql
+    assert "'new-account'" in sql
+    assert "t.author_id NOT IN (11, 12)" in sql
+    assert "t.created >= '2026-10-15' and t.created < '2026-10-15 21:00:00'" in sql
+
+
+def test_list_edits_count_list_creation_and_seed_edits():
+    db = FakeDB()
+    assert run(numbers.admin_range__list_edits, db) == 7
+    [(q, vars)] = db.queries
+    sql = str(web.db.reparam(q, vars))
+    assert "t.action IN ('" in sql
+    assert "'create-list'" in sql
+    assert "'edit-list-seeds'" in sql
+    assert "t.created >= '2026-10-15' and t.created < '2026-10-15 21:00:00'" in sql

openlibrary/admin/tests/test_numbers.py

@@ -1,6 +1,7 @@
 """Admin functionality."""
 
 import calendar
+import random
 from datetime import UTC, date, datetime, timedelta
 
 import requests
@@ -32,6 +33,13 @@ def __init__(self, docs, key, total_key):
         except KeyError, StopIteration:
             self.total = ""
 
+    def has_counts(self, ndays=28) -> bool:
+        """True when every doc of the last `ndays` carries this key. A key only some of them
+        have is one the stats script started writing recently, and a series read from it would
+        cover part of the window while claiming all of it."""
+        docs = self.docs[-ndays:]
+        return bool(docs) and all(self.key in doc for doc in docs)
+
     def get_counts(self, ndays=28, times=False):
         """Returns the stats for last n days as an array useful for
         plotting. i.e. an array of [x, y] tuples where y is the value
@@ -117,6 +125,11 @@ class LoanStats(Stats):
     loans, so the fallback (e.g. local dev) reports zero.
     """
 
+    def has_counts(self, ndays=28) -> bool:
+        if get_ol_env().LOCAL_DEV or not _get_loan_counts_from_graphite(ndays):
+            return Stats.has_counts(self, ndays)
+        return True
+
     def get_counts(self, ndays=28, times=False):
         # Let dev.openlibrary.org show the true state of things
         if get_ol_env().LOCAL_DEV:
@@ -155,6 +168,9 @@ def _get_visitor_counts_from_graphite(ndays: int = 28) -> list[list[int]]:
 
 
 class VisitorStats(Stats):
+    def has_counts(self, ndays: int = 28) -> bool:
+        return bool(_get_visitor_counts_from_graphite(ndays))
+
     def get_counts(self, ndays: int = 28, times: bool = False) -> list[tuple[int, int]]:
         visitors = _get_visitor_counts_from_graphite(ndays)
         # Flip the order, convert timestamp to msec, and convert count==None to zero
@@ -180,12 +196,14 @@ def _get_count_docs(ndays):
 def get_stats(ndays=30, use_mock_data=False):
     """Returns the stats for the past `ndays`"""
     if use_mock_data:
-        return mock_get_stats()
+        return mock_get_stats(ndays)
     docs = _get_count_docs(ndays)
     return {
         "human_edits": Stats(docs, "human_edits", "human_edits"),
         "bot_edits": Stats(docs, "bot_edits", "bot_edits"),
+        "catalog_edits": Stats(docs, "catalog_edits", "catalog_edits"),
         "lists": Stats(docs, "lists", "total_lists"),
+        "list_edits": Stats(docs, "list_edits", "list_edits"),
         "visitors": VisitorStats(docs, "visitors", "visitors"),
         "loans": LoanStats(docs, "loans", "loans"),
         "members": Stats(docs, "members", "total_members"),
@@ -214,38 +232,32 @@ async def get_unique_logins_since(since_days=30):
     return count or 0
 
 
-def mock_get_stats():
-    keyNames = [
-        "human_edits",
-        "bot_edits",
-        "lists",
-        "visitors",
-        "loans",
-        "members",
-        "works",
-        "editions",
-        "ebooks",
-        "covers",
-        "authors",
-        "subjects",
-    ]
-    mockKeyValues = [[(1 + x) * y for x in range(len(keyNames))] for y in range(28)][::-1]
-
-    docs = [dict(zip(keyNames, mockKeyValues[x])) for x in range(len(mockKeyValues))]
-    today = date.today()
-    for x in range(28):
-        docs[x]["_key"] = (today - timedelta(days=x + 1)).strftime("counts-%Y-%m-%d")
-    return {
-        "human_edits": Stats(docs, "human_edits", "human_edits"),
-        "bot_edits": Stats(docs, "bot_edits", "bot_edits"),
-        "lists": Stats(docs, "lists", "total_lists"),
-        "visitors": Stats(docs, "visitors", "visitors"),
-        "loans": Stats(docs, "loans", "loans"),
-        "members": Stats(docs, "members", "total_members"),
-        "works": Stats(docs, "works", "total_works"),
-        "editions": Stats(docs, "editions", "total_editions"),
-        "ebooks": Stats(docs, "ebooks", "total_ebooks"),
-        "covers": Stats(docs, "covers", "total_covers"),
-        "authors": Stats(docs, "authors", "total_authors"),
-        "subjects": Stats(docs, "subjects", "total_subjects"),
+def mock_get_stats(ndays=30):
+    """Daily counts shaped like production's for local dev: a weekend dip, some noise and a
+    gentle climb, at roughly production's scale. Seeded per key so a reload doesn't redraw."""
+    daily_scale = {
+        "human_edits": 12_000,
+        "bot_edits": 20_000,
+        "catalog_edits": 3_000,
+        "lists": 150,
+        "list_edits": 1_200,
+        "visitors": 5_800_000,
+        "loans": 10_000,
+        "members": 7_000,
+        "works": 3_000,
+        "editions": 5_000,
+        "ebooks": 100,
+        "covers": 2_000,
+        "authors": 1_500,
+        "subjects": 500,
     }
+    today = date.today()
+    days = [today - timedelta(days=n) for n in range(ndays, 0, -1)]
+    docs = [{"_key": day.strftime("counts-%Y-%m-%d")} for day in days]
+    for key, scale in daily_scale.items():
+        rng = random.Random(key)
+        for i, (day, doc) in enumerate(zip(days, docs, strict=True)):
+            weekend = 0.85 if day.weekday() >= 5 else 1.0
+            climb = 0.85 + 0.3 * i / max(ndays - 1, 1)
+            doc[key] = round(scale * weekend * climb * rng.uniform(0.92, 1.08))
+    return {key: Stats(docs, key, f"total_{key}") for key in daily_scale}