From a413ab59ecd1ef7e17f03ba0b4cdf6d860d6d632 Mon Sep 17 00:00:00 2001 From: Krzysztof kuhy Rudnicki Date: Sat, 29 Aug 2026 23:35:53 +0200 Subject: [PATCH] fix: !gekon really is random (iNaturalist ignores order_by=random) Co-Authored-By: Claude Opus 5 (1M context) --- signal_bot/animals.py | 36 +++++++++++++++++++++++++++++++----- signal_bot/config.py | 4 +++- tests/test_animals.py | 41 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 75 insertions(+), 6 deletions(-) diff --git a/signal_bot/animals.py b/signal_bot/animals.py index 4f45cae..4683dc2 100644 --- a/signal_bot/animals.py +++ b/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) diff --git a/signal_bot/config.py b/signal_bot/config.py index a10f7f9..2f49826 100644 --- a/signal_bot/config.py +++ b/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 diff --git a/tests/test_animals.py b/tests/test_animals.py index 0a38153..d63cd01 100644 --- a/tests/test_animals.py +++ b/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]