diff --git a/lib/stack/ai/client.py b/lib/stack/ai/client.py index 24de4b67..d539f169 100644 --- a/lib/stack/ai/client.py +++ b/lib/stack/ai/client.py @@ -191,12 +191,14 @@ def from_env(cls, *, namespace: str | None = None, ) key = os.environ.get("OPENAI_KEY", "") or "not-needed" # The SDK default timeout is 600s β€” a hung local endpoint would - # freeze a caller for ten minutes. 120s covers a slow prefill - # on a long document; anything beyond that is a stuck server - # and should fail loudly. + # freeze a caller for ten minutes. 300s is the compromise: a + # 30K-token prefill on a local model (a scanned multi-page PDF + # going into a vision classify) needs several minutes before the + # first token, and 120s cancelled those mid-prefill; past 300s + # it is a stuck server and should fail loudly. client = AsyncOpenAI( base_url=url, api_key=key, max_retries=max_retries, - timeout=120.0, + timeout=300.0, ) return cls(client, namespace=namespace, capabilities=capabilities) diff --git a/stacklets/core/bot-runner/microbot.py b/stacklets/core/bot-runner/microbot.py index ebe22409..528c40de 100644 --- a/stacklets/core/bot-runner/microbot.py +++ b/stacklets/core/bot-runner/microbot.py @@ -569,7 +569,7 @@ async def _react(self, room_id: str, event_id: str, emoji: str) -> None: async def _send( self, room_id: str, text: str, reply_to: str | None = None, *, metadata: dict | None = None, thread_root_event_id: str | None = None, - line_breaks: bool = False, + line_breaks: bool = False, msgtype: str = "m.text", ) -> None: """Send a formatted ``m.room.message``: markdown body + HTML. @@ -595,11 +595,15 @@ async def _send( ``line_breaks`` adds the ``nl2br`` markdown extension so every newline becomes a ``
`` β€” chat behaviour (Slack/WhatsApp), needed for a pasted email body where single newlines would otherwise collapse. + + ``msgtype`` is ``m.text`` for anything the family reads as a normal + answer; the framework's own error notices pass ``m.notice`` so + Element renders them in the muted bot styling. """ exts = ["tables", "fenced_code"] + (["nl2br"] if line_breaks else []) html = markdown.markdown(text, extensions=exts) content: dict = { - "msgtype": "m.text", + "msgtype": msgtype, "body": text, "format": "org.matrix.custom.html", "formatted_body": html, @@ -652,7 +656,7 @@ def check_in_thread(cls, event) -> bool: async def _answer( self, room_id: str, text: str, source_event: str | None, - *, metadata: dict | None = None, + *, metadata: dict | None = None, msgtype: str = "m.text", ) -> None: """Post the bot's answer to a processed item. @@ -670,7 +674,9 @@ async def _answer( answer quotes the source via the reply fallback. """ if not (source_event and self._reply_in_thread(room_id)): - await self._send(room_id, text, source_event, metadata=metadata) + await self._send( + room_id, text, source_event, metadata=metadata, msgtype=msgtype, + ) return root = source_event try: @@ -683,7 +689,7 @@ async def _answer( self.name, source_event, e) await self._send( room_id, text, reply_to=source_event, - thread_root_event_id=root, metadata=metadata, + thread_root_event_id=root, metadata=metadata, msgtype=msgtype, ) # The famstack event envelope rides as a custom key on the visible @@ -1011,28 +1017,38 @@ def _format_handler_error(self, event, exc: BaseException) -> str: return "Sorry β€” that took longer than I'm willing to wait. Try again?" return "Sorry β€” something went wrong handling that message." + @staticmethod + def error_anchor(event) -> str | None: + """The message a failure should be reported against. + + For an ordinary event that is the event itself. For a + ``ReactionEvent`` it is ``reacts_to``: the user acted *on* another + message (πŸ“Ž a mail card), so the reaction event id is bot + bookkeeping no client will render a reply to. Anchoring on the + target instead puts the ❌ and the explanation exactly where the + user is looking, next to the πŸ‘€ the handler already left there. + """ + return getattr(event, "reacts_to", None) or getattr(event, "event_id", None) + async def _send_error(self, room_id: str, event, exc: BaseException) -> None: - """Post a user-facing error message into the room. + """Report a handler failure where the user can see it. + + Two signals, both anchored on ``error_anchor``: a ❌ reaction so + the outcome is visible at a glance in the timeline, and the + explanation as a threaded reply, in the same thread every other + answer about that message lands in. Uses ``m.notice`` so Element + renders it with the bot-message styling rather than as a regular + chat line. - Replies to the original event so the user can see which message - triggered the failure. Best-effort β€” if the send itself fails - we log at warning and stop, no recursion. Uses ``m.notice`` so - Element renders it with the bot-message styling rather than as - a regular chat line. + Best-effort β€” if the send itself fails we log at warning and + stop, no recursion. """ + anchor = self.error_anchor(event) try: text = self._format_handler_error(event, exc) - content = {"msgtype": "m.notice", "body": text} - reply_to = getattr(event, "event_id", None) - if reply_to: - content["m.relates_to"] = { - "m.in_reply_to": {"event_id": reply_to}, - } - await self.client.room_send( - room_id=room_id, - message_type="m.room.message", - content=content, - ) + if anchor: + await self._react(room_id, anchor, CROSS) + await self._answer(room_id, text, anchor, msgtype="m.notice") except Exception as e: logger.warning( "[{}] error-response send failed in {}: {}", diff --git a/stacklets/docs/bot/archivist.py b/stacklets/docs/bot/archivist.py index e557089c..84c2f80a 100644 --- a/stacklets/docs/bot/archivist.py +++ b/stacklets/docs/bot/archivist.py @@ -251,6 +251,18 @@ class ArchivistBot(MicroBot): name = "archivist-bot" + # Filing is the slowest thing any famstack bot does, and the framework + # default (180s) is a chat-bot budget, not a document-pipeline one. One + # archived email costs, in sequence: the Paperless upload + OCR wait + # (up to 120s), a vision classify pass, a layout reformat pass, and the + # entity-enrichment pass, each a full LLM call whose prefill on a local + # model can run into minutes for a long scan. Cancelling at 180s threw + # away work that was still progressing. Eight minutes is a stuck-handler + # guard, not a latency target: the pipeline's own per-step timeouts bound + # normal slowness, and a handler that blows this budget now fails + # visibly (❌ + a threaded notice) and can be retried with πŸ”. + HANDLER_TIMEOUT_SECONDS = 480 + def __init__(self, homeserver, user_id, password, session_dir, **settings): super().__init__(homeserver, user_id, password, session_dir, **settings) # Shared config from env vars β€” rendered by the CLI from stack.toml @@ -1767,6 +1779,8 @@ def _reaction_handlers(self) -> dict: "πŸ“Œ": self._react_bookmark, "πŸ“Ž": self._react_archive_source, "πŸ“„": self._react_archive_source, + "πŸ”": self._react_retry, + "πŸ”„": self._react_retry, } async def _on_reaction(self, room, event) -> None: @@ -1875,6 +1889,28 @@ async def _react_archive_source(self, room, event, target, target_id) -> None: extra_tags=[source_type], note_prefix=header or None, ) + async def _react_retry(self, room, event, target, target_id) -> None: + """πŸ” / πŸ”„ β€” do the work on this message again. + + Filing fails for reasons that have nothing to do with the message: + the model was still loading, Paperless was restarting, the handler + ran past its timeout. Rather than a retry queue somebody has to + drain, the recovery gesture is the same shape as every other one + here: react on the message that failed. The ❌ the framework left + tells you which message; this tells you what to do about it. + + Retry re-dispatches the handler that owns that kind of message, so + there is nothing to keep in sync as bindings are added. Both paths + are idempotent (Paperless dedups on content, captures key on the + target event id), so retrying something that actually succeeded is + a no-op rather than a second copy. + """ + content = (getattr(target, "source", {}) or {}).get("content", {}) or {} + if isinstance(content.get(self.SOURCE_KEY), dict): + await self._react_archive_source(room, event, target, target_id) + elif content.get("msgtype") in SUPPORTED_MSGTYPES: + await self._on_file(room, target) + async def _collect_source_attachments( self, room_id: str, card_id: str, max_pages: int = 5, ) -> list[SourceAttachment]: diff --git a/stacklets/docs/bot/messages/archivist.yml b/stacklets/docs/bot/messages/archivist.yml index c56c3c9f..33c76773 100644 --- a/stacklets/docs/bot/messages/archivist.yml +++ b/stacklets/docs/bot/messages/archivist.yml @@ -107,8 +107,8 @@ en: # Error error_processing: "\u274C Error processing file: {error}" - handler_error: "\u26A0\uFE0F Sorry \u2014 something went wrong handling that message. Try again?" - handler_timeout: "\u23F1\uFE0F Sorry \u2014 that took longer than I'm willing to wait. Try again?" + handler_error: "\u26A0\uFE0F Something went wrong handling that, sorry. React \U0001F501 on the message to try again." + handler_timeout: "\u23F1\uFE0F That took too long, so I stopped. React \U0001F501 on the message to try again." # Welcome / Help # @@ -314,8 +314,8 @@ de: # Error error_processing: "\u274C Fehler bei der Verarbeitung: {error}" - handler_error: "\u26A0\uFE0F Da ist was schiefgelaufen \u2014 sorry. Versuch es nochmal?" - handler_timeout: "\u23F1\uFE0F Das hat zu lange gedauert. Versuch es nochmal?" + handler_error: "\u26A0\uFE0F Da ist was schiefgelaufen, sorry. Reagier mit \U0001F501 auf die Nachricht, dann versuche ich es nochmal." + handler_timeout: "\u23F1\uFE0F Das hat zu lange gedauert, ich habe abgebrochen. Reagier mit \U0001F501 auf die Nachricht, dann versuche ich es nochmal." # Welcome / Help -- siehe englischen Block fΓΌr die Vier-Varianten-Logik. welcome_documents: | diff --git a/tests/stacklets/test_archivist_routing.py b/tests/stacklets/test_archivist_routing.py index 582c7d9b..4e5868ea 100644 --- a/tests/stacklets/test_archivist_routing.py +++ b/tests/stacklets/test_archivist_routing.py @@ -394,6 +394,78 @@ async def test_bot_authored_target_not_bookmarked(self, tmp_path): assert not cap and not txt +class TestRetryReaction: + """πŸ” / πŸ”„ β€” the recovery gesture for a filing that failed. + + Filing can time out or hit a service that was restarting, and until + now the work was simply lost: nothing in chat could start it again. + Retry re-dispatches whichever handler owns that kind of message, so + a family member fixes a failed archive the same way they started it, + by reacting on the message the ❌ is sitting on. + """ + + def _bot(self, tmp_path, *, target): + bot = _build_bot(tmp_path) + calls = [] + + async def _archive_source(room, event, tgt, tgt_id): + calls.append(("source", tgt_id)) + + async def _on_file(room, ev): + calls.append(("file", getattr(ev, "event_id", None))) + + bot._react_archive_source = _archive_source + bot._on_file = _on_file + + async def _get_event(room_id, event_id): + return SimpleNamespace(event=target) + + bot._client = SimpleNamespace(room_get_event=_get_event) + return bot, calls + + @staticmethod + def _reaction(key="πŸ”"): + return SimpleNamespace( + key=key, reacts_to="$tgt", sender="@homer:server", + source={"content": {}}, + ) + + @staticmethod + def _target(content): + return SimpleNamespace( + sender="@mail-bot:server", event_id="$tgt", source={"content": content}, + ) + + async def test_retry_on_a_source_card_archives_it_again(self, tmp_path): + bot, calls = self._bot(tmp_path, target=self._target({ + "body": "From: school", + "dev.famstack.source": {"source": "email", "raw_content": "hi"}, + })) + await bot._on_reaction(_room(), self._reaction()) + assert calls == [("source", "$tgt")] + + async def test_retry_on_an_upload_files_it_again(self, tmp_path): + bot, calls = self._bot(tmp_path, target=self._target({ + "msgtype": "m.file", "url": "mxc://server/abc", "body": "scan.pdf", + })) + await bot._on_reaction(_room(), self._reaction()) + assert calls == [("file", "$tgt")] + + async def test_counterclockwise_variant_also_retries(self, tmp_path): + bot, calls = self._bot(tmp_path, target=self._target({ + "msgtype": "m.image", "url": "mxc://server/abc", + })) + await bot._on_reaction(_room(), self._reaction(key="πŸ”„")) + assert calls == [("file", "$tgt")] + + async def test_retry_on_a_plain_message_does_nothing(self, tmp_path): + # Nothing to redo: πŸ” is a recovery gesture, not a second way to + # capture a message that was never processed in the first place. + bot, calls = self._bot(tmp_path, target=self._target({"body": "hello"})) + await bot._on_reaction(_room(), self._reaction()) + assert calls == [] + + class TestOutcomeGlyph: """After a capture/filing finishes, the bot marks the source message with a terminal glyph alongside the πŸ‘€: βœ… when something was filed, diff --git a/tests/stacklets/test_microbot.py b/tests/stacklets/test_microbot.py index c365cf11..7a2eed92 100644 --- a/tests/stacklets/test_microbot.py +++ b/tests/stacklets/test_microbot.py @@ -137,6 +137,24 @@ def _room(room_id="!r:server"): return SimpleNamespace(room_id=room_id) +def _notice(client) -> dict: + """The error notice out of everything the failure path sent. + + A failure now leaves two marks: an ``m.reaction`` (the ❌) and the + explanation itself. Tests about the wording ask for the latter. + """ + messages = [c for _room_id, mtype, c in client.sends + if mtype == "m.room.message"] + assert len(messages) == 1, f"expected one notice, got {messages}" + return messages[0] + + +def _reactions(client) -> list[str]: + """Every emoji the bot annotated an event with, in order sent.""" + return [c["m.relates_to"]["key"] for _room_id, mtype, c in client.sends + if mtype == "m.reaction"] + + # ── Happy path ─────────────────────────────────────────────────────────── class TestHappyPath: @@ -338,10 +356,7 @@ async def handler(room, event): # Typing still cleared on the way out. assert ("!r:server", False) in client.typing_calls # An error notice was sent. - assert len(client.sends) == 1 - room_id, mtype, content = client.sends[0] - assert room_id == "!r:server" - assert mtype == "m.room.message" + content = _notice(client) assert content["msgtype"] == "m.notice" assert "wait" in content["body"].lower() or "longer" in content["body"].lower() # Reply-to threading is set so the user sees which message failed. @@ -364,8 +379,7 @@ async def handler(room, event): await bot._wrapper(_room(), _event()) assert ("!r:server", False) in client.typing_calls - assert len(client.sends) == 1 - _room_id, _mtype, content = client.sends[0] + content = _notice(client) assert content["msgtype"] == "m.notice" # Default English message, no exception details leaked. assert "kaboom" not in content["body"] @@ -386,7 +400,7 @@ async def handler(room, event): bot._format_handler_error = lambda ev, exc: "[stub] something is off" await bot._wrapper(_room(), _event()) - assert client.sends[0][2]["body"] == "[stub] something is off" + assert _notice(client)["body"] == "[stub] something is off" @pytest.mark.asyncio async def test_timeout_routes_distinct_message(self, tmp_path): @@ -406,7 +420,60 @@ def fmt(event, exc): await bot._wrapper(_room(), _event()) assert seen == [asyncio.TimeoutError] - assert client.sends[0][2]["body"] == "TIMEOUT" + assert _notice(client)["body"] == "TIMEOUT" + + +# ── Where a failure is reported ────────────────────────────────────────── + +class TestErrorAnchoring: + """A failure has to be visible on the message that caused it. + + Success already marks the source message (πŸ‘€ then βœ…), so a failure + that posted a loose reply somewhere else read as "nothing happened". + The framework anchors both halves of the bad news β€” the ❌ and the + explanation β€” on the message the user acted on, which for a reaction + is the message reacted *to*, not the reaction event itself. + """ + + @staticmethod + def _failing_bot(tmp_path): + async def handler(_room, _event): + raise RuntimeError("kaboom") + return _build_bot(tmp_path, handler=handler) + + @pytest.mark.asyncio + async def test_failure_marks_the_message_with_a_cross(self, tmp_path): + bot, client = self._failing_bot(tmp_path) + await bot._wrapper(_room(), _event()) + + assert _reactions(client) == ["\U0000274C"] + target = [c for _r, mtype, c in client.sends if mtype == "m.reaction"][0] + assert target["m.relates_to"]["event_id"] == "$evt:server" + + @pytest.mark.asyncio + async def test_explanation_lands_in_the_message_thread(self, tmp_path): + bot, client = self._failing_bot(tmp_path) + await bot._wrapper(_room(), _event()) + + rel = _notice(client)["m.relates_to"] + assert rel["rel_type"] == "m.thread" + assert rel["event_id"] == "$evt:server" + + @pytest.mark.asyncio + async def test_reaction_failure_anchors_on_the_reacted_message(self, tmp_path): + # πŸ“Ž on a mail card: the handler's event is the reaction, but the + # card is what the family is looking at and what carries the πŸ‘€. + bot, client = self._failing_bot(tmp_path) + reaction = SimpleNamespace( + sender="@homer:server", server_timestamp=1_000_000, + event_id="$reaction:server", reacts_to="$card:server", + source={"content": {}}, + ) + await bot._wrapper(_room(), reaction) + + marked = [c for _r, mtype, c in client.sends if mtype == "m.reaction"][0] + assert marked["m.relates_to"]["event_id"] == "$card:server" + assert _notice(client)["m.relates_to"]["event_id"] == "$card:server" # ── Best-effort send ─────────────────────────────────────────────────────