fix: !gekon really is random (iNaturalist ignores order_by=random)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

signal_bot/animals.py

@@ -4,6 +4,7 @@
 from __future__ import annotations
 
 import json
+import secrets
 from dataclasses import dataclass
 from pathlib import Path
 from typing import TYPE_CHECKING, Final
@@ -27,11 +28,18 @@ class ImageSource:
     api_url: str
     url_path: tuple[str | int, ...]
     rewrite: tuple[str, str] | None = None
+    page_limit: int | None = None
 
 
-# iNaturalist answers with the 75px square thumbnail; the full-size photo is
-# the same URL with one segment swapped. Kept beside the endpoint because it
-# is a property of that endpoint, not of image fetching in general.
+# iNaturalist's pagination ceiling: page * per_page must stay at or under
+# 10 000, and page=10001 comes back empty. Measured, not documented.
+INATURALIST_PAGE_LIMIT: Final = 10_000
+
+# Two iNaturalist quirks, kept beside the endpoint because that is what they
+# are properties of. It answers with a 75px square thumbnail whose full-size
+# sibling is one path segment away; and it accepts `order_by=random` while
+# ignoring it -- five requests in a row returned the same gecko -- so the
+# randomness has to come from asking for a random page instead.
 SOURCES: Final[dict[str, ImageSource]] = {
     "cat": ImageSource(DEFAULT_CAT_API_URL, (0, "url")),
     "dog": ImageSource(DEFAULT_DOG_API_URL, ("message",)),
@@ -39,10 +47,24 @@ SOURCES: Final[dict[str, ImageSource]] = {
         DEFAULT_GECKO_API_URL,
         ("results", 0, "photos", 0, "url"),
         ("/square.", "/large."),
+        INATURALIST_PAGE_LIMIT,
     ),
 }
 
 
+def paged_url(api_url: str, page_limit: int | None) -> str:
+    """Return api_url, pointed at a random page when the source needs one.
+
+    secrets rather than random: the choice is not security-sensitive, but
+    ruff's ALL rule set flags random for exactly this shape, and this repo
+    forbids the noqa that would silence it.
+    """
+    if page_limit is None:
+        return api_url
+    separator = "&" if "?" in api_url else "?"
+    return f"{api_url}{separator}page={secrets.randbelow(page_limit) + 1}"
+
+
 def load_aliases(path: Path = ALIAS_FILE) -> dict[str, list[str]]:
     """Load the trigger table.
 
@@ -58,11 +80,13 @@ def image_handler(
     api_url: str,
     url_path: Sequence[str | int],
     rewrite: tuple[str, str] | None = None,
+    page_limit: int | None = None,
 ) -> Handler:
     """Build a handler that posts one random image from a JSON image API."""
 
     async def handler(ctx: CommandContext) -> None:
-        encoded = await fetcher.fetch_base64(api_url, url_path, rewrite)
+        url = paged_url(api_url, page_limit)
+        encoded = await fetcher.fetch_base64(url, url_path, rewrite)
         await ctx.reply_image(encoded)
 
     return handler
@@ -91,7 +115,9 @@ def register_animal_commands(
     overrides = endpoints or {}
     for name, source in SOURCES.items():
         api_url = overrides.get(name, source.api_url)
-        handler = image_handler(fetcher, api_url, source.url_path, source.rewrite)
+        handler = image_handler(
+            fetcher, api_url, source.url_path, source.rewrite, source.page_limit
+        )
         if decorate is not None:
             handler = decorate(handler)
         registry.register(name, table[name], handler)

signal_bot/config.py

@@ -41,10 +41,12 @@ DEFAULT_DOG_API_URL: Final = "https://dog.ceo/api/breeds/image/random"
 # taxon_id 371057 is Gekkota, the gecko infraorder. The licence filter is not
 # decoration: without it the API happily returns all-rights-reserved photos,
 # and this bot reposts whatever it is handed into a group chat.
+# Deliberately no order_by=random -- see animals.paged_url for why it does
+# not work and what supplies the randomness instead.
 DEFAULT_GECKO_API_URL: Final = (
     "https://api.inaturalist.org/v1/observations"
     "?taxon_id=371057&photos=true&quality_grade=research"
-    "&photo_license=cc-by,cc-by-sa,cc0&per_page=1&order_by=random"
+    "&photo_license=cc-by,cc-by-sa,cc0&per_page=1"
 )
 DEFAULT_BIND_HOST: Final = "127.0.0.1"
 # Not 8000: that port is a magnet for unrelated dev servers and was already

tests/test_animals.py

@@ -11,7 +11,9 @@ import pytest
 
 from signal_bot.animals import (
     ALIAS_FILE,
+    INATURALIST_PAGE_LIMIT,
     load_aliases,
+    paged_url,
     register_animal_commands,
 )
 from signal_bot.decorators import cooldown
@@ -124,6 +126,31 @@ def test_custom_alias_table_is_honoured() -> None:
     assert registry.resolve("!cat") is None
 
 
+def test_paged_url_leaves_a_source_without_a_page_limit_alone() -> None:
+    assert paged_url("https://api/x?a=1", None) == "https://api/x?a=1"
+
+
+@pytest.mark.parametrize(
+    ("api_url", "separator"), [("https://api/x?a=1", "&"), ("https://api/x", "?")]
+)
+def test_paged_url_appends_a_page_with_the_right_separator(api_url: str, separator: str) -> None:
+    url = paged_url(api_url, 10)
+    prefix = f"{api_url}{separator}page="
+    assert url.startswith(prefix)
+    assert 1 <= int(url.removeprefix(prefix)) <= 10
+
+
+def test_paged_url_actually_varies() -> None:
+    """The regression: iNaturalist accepts order_by=random and ignores it.
+
+    0.0.3 shipped with that as the only source of variety and posted one
+    identical gecko every time. Twenty draws over a 10 000-page range
+    collide with probability far below any plausible flake budget.
+    """
+    drawn = {paged_url("https://api/x?a=1", INATURALIST_PAGE_LIMIT) for _ in range(20)}
+    assert len(drawn) > 1
+
+
 async def test_gecko_asks_for_the_full_size_photo(
     transport: FakeTransport, store: Store, config: Config
 ) -> None:
@@ -136,3 +163,17 @@ async def test_gecko_asks_for_the_full_size_photo(
     await command.handler(_ctx(transport, store, config, "gecko", "!gecko"))
     assert "https://cdn/photos/1/large.jpeg" in seen
     assert "https://cdn/photos/1/square.jpeg" not in seen
+    assert "page=" in seen[0]
+
+
+async def test_cat_is_asked_for_without_a_page(
+    transport: FakeTransport, store: Store, config: Config
+) -> None:
+    """Only the sources that need paging get one; thecatapi is random already."""
+    seen: list[str] = []
+    registry = CommandRegistry()
+    register_animal_commands(registry, _fetcher(seen))
+    command = registry.resolve("!cat")
+    assert command is not None
+    await command.handler(_ctx(transport, store, config, "cat", "!cat"))
+    assert "page=" not in seen[0]