diff --git a/DOCS-changelog.md b/DOCS-changelog.md index 35a9e57..a639899 100644 --- a/DOCS-changelog.md +++ b/DOCS-changelog.md @@ -102,6 +102,8 @@ papież nr 15 z 267, pontyfikat 199-217 wieczorny post pokazuje wreszcie ten dzień, który właśnie mijał - licznik zamyka się o 21:37, więc to, co napiszecie później, wpada do jutra; post sam to mówi, żeby liczba nie udawała całej doby +- **jedna wiadomość, nie kilka**: dzień, tydzień, miesiąc i to, co bot + przegapił, są blokami w jednym poście, a nie osobnymi powiadomieniami - dzień, którego bot przegapił (bo go nie było o 21:37), nadal zostanie wysłany później — nic nie ginie diff --git a/signal_bot/bot.py b/signal_bot/bot.py index 48455a5..c6a4da8 100644 --- a/signal_bot/bot.py +++ b/signal_bot/bot.py @@ -26,16 +26,10 @@ from signal_bot.pack_load import load as load_pack from signal_bot.popes_command import register_pope_command from signal_bot.portraits import PortraitCache from signal_bot.registry import CommandRegistry -from signal_bot.stats import curiosities, periods -from signal_bot.stats.backfill import backfilled_day +from signal_bot.reporting import compose +from signal_bot.stats import periods from signal_bot.stats.commands import register_stats_commands -from signal_bot.stats.facts import gather -from signal_bot.stats.highlights import SLOTS, select -from signal_bot.stats.nightly import closing_periods, days_to_report, mark_reported -from signal_bot.stats.queries_day import day_rows, day_totals, with_silent -from signal_bot.stats.queries_period import Trend, period_rows, total_for -from signal_bot.stats.render_day import DayView, render_day -from signal_bot.stats.render_period import render_period +from signal_bot.stats.nightly import days_to_report, mark_reported from signal_bot.versioning import POPE if TYPE_CHECKING: @@ -84,7 +78,7 @@ class Bot: clock: Clock = now_local async def publish_report(self) -> None: - """Post each group's report for today, plus any day still owed. + """Post each group **one** message: today, plus anything still owed. Today, because 21:37 is when the group expects to read its own numbers; a post at 21:37 about yesterday is a day late to everyone @@ -93,9 +87,12 @@ class Bot: because a total that quietly omits three hours is worse than one that admits to it. - Everything that makes a late day recoverable stays: the day is marked - per group, so it cannot post twice, and days_to_report still yields - every day the bot was down for, oldest first. + One message, always. A run can have several things to say -- a day + the bot was down for, the day it is on now, and the week or month + that ended inside them -- and saying each in its own post turns one + evening into four notifications. They are blocks in one message + instead, which is also what makes the send atomic: either the group + got the lot, or nothing is marked and the next run says it all again. Nothing is deleted afterwards. Each group is isolated on purpose: one group refusing the report -- the bot removed from it, a transient send @@ -103,81 +100,39 @@ class Bot: """ latest = periods.today(self.clock()) for group_id in self.directory.groups: - for day in days_to_report(self.store, group_id, latest): - if not await self._report_one(group_id, day): - # Left unmarked on purpose, so the next run retries it - # rather than skipping the day forever. - break - mark_reported(self.store, group_id, day) - await self._report_closing(group_id, day) + await self._report_group(group_id, latest) - async def _report_one(self, group_id: str, day: str) -> bool: - """Post one group's report for one day. False means try again later.""" - rows = day_rows(self.store.events, group_id, day) - if not rows: - # Settled, not pending. The bot is in every group it was ever - # added to, so posting "nobody spoke" would mean a nightly ping - # into every quiet group, forever. - logger.info("no messages in %s on %s; not posting a report", group_id, day) - return True - named = self.store.events.roster(group_id, self.directory.member_identities(group_id)) - with_zeros = with_silent(rows, [(p.uuid, p.display_name) for p in named]) - backfilled = backfilled_day(self.store, group_id) - facts = gather(self.store.events, group_id, day, with_zeros, exclude=backfilled) - picks = select(curiosities.for_day(facts), SLOTS) - text = render_day( - day, - with_zeros, - day_totals(with_zeros), - DayView( - since=self.store.events.first_day(group_id), - approximate=backfilled == day, - highlights=tuple(pick.text for pick in picks), - # Only for a day that is still running. A day caught up after - # an outage is complete, and telling the group its count - # stopped at 21:37 would be a lie about a finished day. - closed_at=self._closing_time(day), - ), + async def _report_group(self, group_id: str, latest: str) -> None: + """Say everything one group is owed, in one message or not at all.""" + days = days_to_report(self.store, group_id, latest) + if not days: + return + now = self.clock() + text = compose( + self.store, + self.directory, + group_id, + days, + today=periods.today(now), + # Off the clock rather than off REPORT_HOUR: the scheduled run + # fires at the configured minute and so says exactly that, while + # a run started by hand at some other hour tells the truth + # instead of repeating a time nobody waited for. + closed_at=now.strftime("%H:%M"), ) - try: - await self.transport.send_text(text, group_id) - except Exception: - logger.exception("daily report failed for group %s", group_id) - return False - return True - - def _closing_time(self, day: str) -> str | None: - """The "HH:MM" a still-open day was counted up to, or None.""" - if day != periods.today(self.clock()): - return None - return f"{self.config.report_hour:02d}:{self.config.report_minute:02d}" - - async def _report_closing(self, group_id: str, day: str) -> None: - """Post the week, month or year that ended with this day, if any. - - Best effort and deliberately after the day has been marked: a failed - weekly summary must not make the bot re-post the daily one tomorrow. - """ - for period in closing_periods(day): - before = periods.previous(period) - trend = Trend( - now=total_for(self.store.events, group_id, period.start, period.end), - before=total_for(self.store.events, group_id, before.start, before.end), - ) - rows = period_rows(self.store.events, group_id, period) - if not rows: - continue - text = render_period( - period, - rows, - trend, - since=self.store.events.first_day(group_id), - podium=period.kind != "week", - ) + if text: try: await self.transport.send_text(text, group_id) except Exception: - logger.exception("%s summary failed for group %s", period.kind, group_id) + # Nothing is marked, so the next run repeats the whole thing + # rather than losing the days that were inside this message. + logger.exception("report failed for group %s", group_id) + return + # Marked even when there was nothing to say: a day nobody spoke on is + # settled, not pending, and leaving it unmarked would make the bot + # reconsider it every night forever. + for day in days: + mark_reported(self.store, group_id, day) async def announce_release(self) -> None: """Tell every watched group about a version it has not seen yet.""" diff --git a/signal_bot/reporting.py b/signal_bot/reporting.py new file mode 100644 index 0000000..fb746b7 --- /dev/null +++ b/signal_bot/reporting.py @@ -0,0 +1,133 @@ +# Copyright (c) 2026 Krzysztof Rudnicki. MIT licence. +"""Composing the evening report: what it says, but never the sending of it. + +Split out of bot.py at the 250-line cap, and the seam is worth having on its +own terms -- everything here is pure, so "what would the group be told?" is a +question that can be answered without a transport, a group, or a send. + +One run can have several things to say: a day the bot was down for, the day +it is on now, and the week or month that ended inside them. They are blocks +in one message rather than a post each, because four notifications in one +evening is how a daily tally becomes something people mute. +""" + +from __future__ import annotations + +import logging +from typing import TYPE_CHECKING, Final + +from signal_bot.stats import curiosities, periods +from signal_bot.stats.backfill import backfilled_day +from signal_bot.stats.facts import gather +from signal_bot.stats.highlights import SLOTS, select +from signal_bot.stats.nightly import closing_periods +from signal_bot.stats.queries_day import day_rows, day_totals, with_silent +from signal_bot.stats.queries_period import Trend, period_rows, total_for +from signal_bot.stats.render_day import DayView, render_day +from signal_bot.stats.render_period import render_period + +if TYPE_CHECKING: + from collections.abc import Sequence + + from signal_bot.discovery import GroupDirectory + from signal_bot.store import Store + +logger = logging.getLogger(__name__) + +# What separates the day, the week and any caught-up day inside one report. +# A blank line and a rule, because Signal renders no markdown and two blocks +# run straight into each other without one. +BLOCK_SEPARATOR: Final = "\n\n———\n\n" + + +def compose( + store: Store, + directory: GroupDirectory, + group_id: str, + days: Sequence[str], + today: str, + closed_at: str | None, +) -> str: + """The whole message for one group, or "" when there is nothing to say. + + Args: + store: The open database. + directory: Used for the member roster, so people who said nothing are + still named. + group_id: The group being reported on. + days: Days to cover, oldest first. + today: The day that is still running, from the caller's clock. Passed + rather than read here: one injected clock decides what "today" + means for the whole bot, and a second reading of the real one + could disagree with it across midnight. + closed_at: "HH:MM" that still-running day was counted up to. + """ + blocks: list[str] = [] + for day in days: + block = day_block(store, directory, group_id, day, closed_at if day == today else None) + if block: + blocks.append(block) + blocks.extend(closing_blocks(store, group_id, day)) + return BLOCK_SEPARATOR.join(blocks) + + +def day_block( + store: Store, + directory: GroupDirectory, + group_id: str, + day: str, + closed_at: str | None, +) -> str | None: + """One day as the group reads it, or None if nobody spoke that day.""" + rows = day_rows(store.events, group_id, day) + if not rows: + # The bot is in every group it was ever added to, so "nobody spoke" + # would be a nightly ping into every quiet group, forever. + logger.info("no messages in %s on %s; not reporting on it", group_id, day) + return None + named = store.events.roster(group_id, directory.member_identities(group_id)) + with_zeros = with_silent(rows, [(person.uuid, person.display_name) for person in named]) + backfilled = backfilled_day(store, group_id) + facts = gather(store.events, group_id, day, with_zeros, exclude=backfilled) + picks = select(curiosities.for_day(facts), SLOTS) + return render_day( + day, + with_zeros, + day_totals(with_zeros), + DayView( + since=store.events.first_day(group_id), + approximate=backfilled == day, + highlights=tuple(pick.text for pick in picks), + # Only for a day that is still running: telling the group a + # finished day's count "stopped at 21:37" would be a lie. + closed_at=closed_at, + ), + ) + + +def closing_blocks(store: Store, group_id: str, day: str) -> list[str]: + """The week, month or year that ended with this day, if any. + + Rendered rather than posted: they join the day in one message, so a + Sunday evening is one notification and not two. + """ + blocks: list[str] = [] + for period in closing_periods(day): + rows = period_rows(store.events, group_id, period) + if not rows: + continue + before = periods.previous(period) + trend = Trend( + now=total_for(store.events, group_id, period.start, period.end), + before=total_for(store.events, group_id, before.start, before.end), + ) + blocks.append( + render_period( + period, + rows, + trend, + since=store.events.first_day(group_id), + podium=period.kind != "week", + ) + ) + return blocks diff --git a/tests/test_bot_catchup.py b/tests/test_bot_catchup.py index f92929d..b96f9c9 100644 --- a/tests/test_bot_catchup.py +++ b/tests/test_bot_catchup.py @@ -12,7 +12,6 @@ from signal_bot.bot import Bot, build_bot from signal_bot.liveness import Liveness from signal_bot.stats.nightly import LAST_REPORTED_KEY, mark_reported from signal_bot.stats.periods import WARSAW -from signal_bot.transport_fake import FakeTransport from tests.conftest import WATCHED_GROUP, mock_client from tests.test_bot_report import TODAY, YESTERDAY, RefusingTransport, _bot, wrote @@ -20,6 +19,7 @@ if TYPE_CHECKING: from signal_bot.config import Config from signal_bot.discovery import GroupDirectory from signal_bot.store import Store + from signal_bot.transport_fake import FakeTransport def _at( @@ -48,15 +48,20 @@ async def test_a_missed_day_is_posted_when_the_bot_returns( await _bot(config, store, transport, directory).publish_report() - # Five, not three: catching up on the days also catches up the periods - # that closed inside them. The 30th was a Sunday and the 31st ended - # August, so each is followed by its own summary. - assert len(transport.texts) == 5 - assert "30 sierpnia" in transport.texts[0] - assert "tydzień 35" in transport.texts[1] - assert "31 sierpnia" in transport.texts[2] - assert "sierpień 2026" in transport.texts[3] - assert "1 września" in transport.texts[4] + # One message, five blocks. Catching up on the days also catches up the + # periods that closed inside them -- the 30th was a Sunday and the 31st + # ended August -- and posting each separately would turn one evening + # into five notifications. + assert len(transport.texts) == 1 + report = transport.texts[0] + # Whole headers, not bare dates: a curiosity line inside one day's block + # can name another day ("rekord życiowy (poprzedni 1, 31 sierpnia)"), so + # a substring search finds the wrong occurrence and the order looks wrong. + sunday = report.index("Niedziela, 30 sierpnia") + week = report.index("tydzień 35") + monday = report.index("Poniedziałek, 31 sierpnia") + assert sunday < week < monday < report.index("sierpień 2026") + assert "Wtorek, 1 września" in report assert store.meta(WATCHED_GROUP, LAST_REPORTED_KEY) == TODAY @@ -113,9 +118,9 @@ async def test_a_sunday_also_posts_the_week( await _at(config, store, transport, directory, sunday).publish_report() - assert len(transport.texts) == 2 + assert len(transport.texts) == 1 assert "Niedziela, 30 sierpnia" in transport.texts[0] - assert "tydzień 35" in transport.texts[1] + assert "tydzień 35" in transport.texts[0] async def test_a_saturday_closes_nothing( @@ -140,23 +145,21 @@ async def test_an_ordinary_day_posts_only_the_day( assert len(transport.texts) == 1 -async def test_a_failing_weekly_summary_does_not_unmark_the_day( +async def test_a_refused_report_marks_nothing_at_all( config: Config, store: Store, directory: GroupDirectory ) -> None: - """The day was posted; retrying it tomorrow would repeat it.""" + """One message means one outcome: the group got all of it, or none. + + The day used to be marked before its weekly summary was even attempted, + so a summary that failed was simply lost. With both inside one send there + is no half-state to reason about -- the next run says the whole thing. + """ sunday = "2026-08-30" wrote(store, WATCHED_GROUP, "u1", "Alice", 6, day=sunday) mark_reported(store, WATCHED_GROUP, "2026-08-29") + transport = RefusingTransport(refuses=WATCHED_GROUP) - class RefuseSecond(FakeTransport): - """Lets the day through and rejects everything after it.""" - - async def send_text(self, text: str, recipient: str) -> None: - if self.sent: - msg = "no" - raise RuntimeError(msg) - await super().send_text(text, recipient) - - transport = RefuseSecond() await _at(config, store, transport, directory, sunday).publish_report() - assert store.meta(WATCHED_GROUP, LAST_REPORTED_KEY) == sunday + + assert transport.texts == [] + assert store.meta(WATCHED_GROUP, LAST_REPORTED_KEY) == "2026-08-29" diff --git a/tool/tagger.html b/tool/tagger.html index 9c30526..42b7f02 100644 --- a/tool/tagger.html +++ b/tool/tagger.html @@ -31,6 +31,9 @@ .hint { color: var(--dim); font-size: 14px; } .err { color: #f7768e; } label.group { display: block; padding: 4px 0; } + pre.sample { background: #101214; border: 1px solid #33363d; border-radius: 8px; + padding: 12px; overflow-x: auto; } + .dim { color: #6b7280; } @@ -38,12 +41,15 @@