From ceb19d6455361a9057c76df73e60797b759088f3 Mon Sep 17 00:00:00 2001 From: Joaquin Mansilla Date: Mon, 10 Aug 2026 10:03:07 -0300 Subject: [PATCH 1/3] filter bot message before create a map point --- chatmap-api/bot/flow.py | 11 +- .../bot/flows/first_time_mapping/flow.py | 29 +++- chatmap-api/conversation_engine/flow.py | 18 ++- chatmap-api/conversation_engine/tool.py | 16 ++- .../store/bot_consumed_messages_store.py | 127 ++++++++++++++++++ chatmap-api/stream.py | 16 +++ .../bot_tests/test_first_time_mapping_flow.py | 101 +++++++++++++- .../test_bot_tool.py | 17 ++- .../conversation_engine_tests/test_flow.py | 11 +- 9 files changed, 327 insertions(+), 19 deletions(-) create mode 100644 chatmap-api/store/bot_consumed_messages_store.py diff --git a/chatmap-api/bot/flow.py b/chatmap-api/bot/flow.py index d363e64..40f76ab 100644 --- a/chatmap-api/bot/flow.py +++ b/chatmap-api/bot/flow.py @@ -1,7 +1,9 @@ from abc import ABC, abstractmethod from conversation_engine.event import EventName from dataclasses import dataclass +from datetime import datetime from enum import Enum +from store.bot_consumed_messages_store import BotConsumedMessagesStore from store.bot_state_store import BotStateStore from store.message_to_send_store import MessageToSendStore from typing import Callable, Awaitable @@ -28,6 +30,8 @@ class BotFlowContext: recipient: str sender: str answer: str + message_id: str + occurred_at: datetime class BotFlow(ABC): @@ -35,12 +39,14 @@ def __init__(self, state: Enum, language: Language, bot_state_store: BotStateStore, - message_to_send_store: MessageToSendStore + message_to_send_store: MessageToSendStore, + bot_consumed_messages_store: BotConsumedMessagesStore ): self.state = state self.language = language self.bot_state_store = bot_state_store self.message_to_send_store = message_to_send_store + self.bot_consumed_messages_store = bot_consumed_messages_store @classmethod @abstractmethod @@ -48,7 +54,8 @@ async def create( cls, bot_state_key: str, bot_state_store: BotStateStore, - message_to_send_store: MessageToSendStore + message_to_send_store: MessageToSendStore, + bot_consumed_messages_store: BotConsumedMessagesStore ): ... diff --git a/chatmap-api/bot/flows/first_time_mapping/flow.py b/chatmap-api/bot/flows/first_time_mapping/flow.py index a705af0..ee38227 100644 --- a/chatmap-api/bot/flows/first_time_mapping/flow.py +++ b/chatmap-api/bot/flows/first_time_mapping/flow.py @@ -1,6 +1,7 @@ from bot.flow import BotFlow, BotTransitions, BotFlowContext, not_handler_created, Language from conversation_engine.event import EventName from enum import Enum, auto +from store.bot_consumed_messages_store import BotConsumedMessagesStore from store.bot_state_store import BotStateStore from store.message_to_send_store import MessageToSendStore @@ -33,7 +34,8 @@ async def create( cls, bot_state_key: str, bot_state_store: BotStateStore, - message_to_send_store: MessageToSendStore + message_to_send_store: MessageToSendStore, + bot_consumed_messages_store: BotConsumedMessagesStore ): result = await bot_state_store.fetch_state(bot_state_key=bot_state_key) @@ -48,8 +50,13 @@ async def create( state = FirstTimeMappingState.IDLE language = Language.default() - return cls(state=state, language=language, bot_state_store=bot_state_store, - message_to_send_store=message_to_send_store) + return cls( + state=state, + language=language, + bot_state_store=bot_state_store, + message_to_send_store=message_to_send_store, + bot_consumed_messages_store=bot_consumed_messages_store + ) async def call(self, current_event: EventName, context: BotFlowContext) -> None: logger.info(f"Calling bot flow: '{self.name}' with state: '{self.state}' for event: '{current_event}'") @@ -66,6 +73,16 @@ async def on_ask_for_help( ctx: BotFlowContext, ) -> None: logger.info("Handling: on_ask_for_help") + + # The text was consumed to open the conversation -- the user got a + # language menu, not a mapped point -- so it must not reach the map. + # Guarded on text because on_fallback reuses this handler for photos + # and locations, and those are content. + if ctx.answer: + await self.bot_consumed_messages_store.mark_consumed( + device=ctx.sender, message_id=ctx.message_id, occurred_at=ctx.occurred_at + ) + logger.info("sending message...") await self.message_to_send_store.send_message( @@ -85,6 +102,12 @@ async def on_ask_for_lang( ) -> None: logger.info("Handling: on_ask_for_lang") + # Answering the bot, not mapping. Marked before validating, so an + # invalid answer is kept out of the map too. + await self.bot_consumed_messages_store.mark_consumed( + device=ctx.sender, message_id=ctx.message_id, occurred_at=ctx.occurred_at + ) + selected_language = ctx.answer displayed_options = {str(i): lang.name for i, lang in enumerate(Language, start=1)} diff --git a/chatmap-api/conversation_engine/flow.py b/chatmap-api/conversation_engine/flow.py index cd329ac..ea3e37e 100644 --- a/chatmap-api/conversation_engine/flow.py +++ b/chatmap-api/conversation_engine/flow.py @@ -8,6 +8,7 @@ from redis.client import Redis as RedisClient +from store.bot_consumed_messages_store import BotConsumedMessagesStore from store.bot_state_store import BotStateStore from store.message_to_send_store import MessageToSendStore @@ -21,9 +22,11 @@ class Flow(ABC): window_time: ClassVar[timedelta] def __init__(self, bot_state_store: BotStateStore, message_to_send_store: MessageToSendStore, + bot_consumed_messages_store: BotConsumedMessagesStore, tools_by_events: Optional[dict[EventName, Tool]] = None): self.bot_state_store = bot_state_store self.message_to_send_store = message_to_send_store + self.bot_consumed_messages_store = bot_consumed_messages_store self.tools_by_events = tools_by_events if tools_by_events is not None else self.default_tools_by_events() def expected_events(self) -> set[EventName]: @@ -52,7 +55,11 @@ class HelpFlow(Flow): window_time = timedelta(minutes=2) def default_tools_by_events(self) -> dict[EventName, Tool]: - bot_tool = BotTool(bot_state_store=self.bot_state_store, message_to_send_store=self.message_to_send_store) + bot_tool = BotTool( + bot_state_store=self.bot_state_store, + message_to_send_store=self.message_to_send_store, + bot_consumed_messages_store=self.bot_consumed_messages_store + ) return { EventName.USER_SEND_TEXT: bot_tool, @@ -66,9 +73,16 @@ def __init__(self, client: RedisClient): self.bot_state_store = BotStateStore(client) self.message_to_send_store = MessageToSendStore(client=client) self.received_messages_store = ReceivedMessagesStore(client=client) + self.bot_consumed_messages_store = BotConsumedMessagesStore(client=client) def registered_flows(self) -> list[Flow]: - return [HelpFlow(bot_state_store=self.bot_state_store, message_to_send_store=self.message_to_send_store)] + return [ + HelpFlow( + bot_state_store=self.bot_state_store, + message_to_send_store=self.message_to_send_store, + bot_consumed_messages_store=self.bot_consumed_messages_store + ) + ] async def call_tools_for(self, event: Event, message: ReceivedMessage, device: str, conversation: Conversation): for flow in self.registered_flows(): diff --git a/chatmap-api/conversation_engine/tool.py b/chatmap-api/conversation_engine/tool.py index 60a53ac..e537328 100644 --- a/chatmap-api/conversation_engine/tool.py +++ b/chatmap-api/conversation_engine/tool.py @@ -7,6 +7,7 @@ from conversation_engine.conversation import Conversation from conversation_engine.event import Event from bot.flows.first_time_mapping.flow import FirstTimeMappingFlow +from store.bot_consumed_messages_store import BotConsumedMessagesStore from store.bot_state_store import BotStateStore from store.message_to_send_store import MessageToSendStore from settings import CHATMAP_ENC_KEY @@ -31,22 +32,31 @@ def _decrypt_text(encoded_data: str) -> str: class BotTool: - def __init__(self, bot_state_store: BotStateStore, message_to_send_store: MessageToSendStore): + def __init__( + self, bot_state_store: BotStateStore, + message_to_send_store: MessageToSendStore, + bot_consumed_messages_store: BotConsumedMessagesStore + ): self.bot_state_store = bot_state_store self.message_to_send_store = message_to_send_store + self.bot_consumed_messages_store = bot_consumed_messages_store async def __call__(self, event: Event, message: ReceivedMessage, device: str, conversation: Conversation): bot_state_key = f"bot_state:{FirstTimeMappingFlow.name}:{message.sender}{message.chat}" flow = await FirstTimeMappingFlow.create( bot_state_key=bot_state_key, - bot_state_store=self.bot_state_store, message_to_send_store=self.message_to_send_store + bot_state_store=self.bot_state_store, + message_to_send_store=self.message_to_send_store, + bot_consumed_messages_store=self.bot_consumed_messages_store ) context = BotFlowContext( state_key=bot_state_key, recipient=message.sender_enc, sender=device, - answer=_decrypt_text(message.text) + answer=_decrypt_text(message.text), + message_id=message.id, + occurred_at=event.occurred_at ) await flow.call(current_event=event.name, context=context) diff --git a/chatmap-api/store/bot_consumed_messages_store.py b/chatmap-api/store/bot_consumed_messages_store.py new file mode 100644 index 0000000..d80f8e1 --- /dev/null +++ b/chatmap-api/store/bot_consumed_messages_store.py @@ -0,0 +1,127 @@ +""" +Keeps track of the messages the bot consumed as answers to its own questions, +so the mapping pipeline can tell them apart from the content a user meant to +put on the map. + +WHY THIS EXISTS +--------------- +Two pipelines read the same Redis stream and know nothing about each other: + + * `stream.py` -> `data.py` -> chatmap_py, the original mapping pipeline. It + pairs every location with the closest message in time from the same user, + in either direction. It has no notion of a conversation. + * `consumers/listener.py` -> conversation_engine, the bot. It asks questions + and the user answers them with short option codes ("1", "2", ...). + +Those answers are just text sitting near a location, so the pairing happily +picks one over the photo it was supposed to describe -- the option code ends +up on the map and the photo is dropped. Marking the answers here, and +filtering them out in `stream.py`, is what keeps the two pipelines from +stepping on each other. + +TRANSITIONAL +------------ +This store is a seam, not a destination. We chose to keep the original mapping +pipeline running as-is rather than rework production code, and this is the +cheapest correct way to make both coexist. The direction we want is to move +mapping into the conversation engine flow, which already knows exactly which +message is a location, which is content and which is an answer -- it does not +have to guess by time proximity. Once mapping lives there, the filtering in +`stream.py` and this whole store should be deleted. +""" + +import logging + +from datetime import datetime +from redis import RedisError +from results.error import StoreUnavailable +from typing import Sequence + +from redis.asyncio.client import Redis as RedisClient + +logger = logging.getLogger(__name__) + + +class BotConsumedMessagesStore: + """ + Stores consumed message ids in a sorted set per device, scored by the time + the message was sent. + + The score is what makes this cheap to keep in sync with the stream: marks + are trimmed by age with the very same cutoff `stream.py` already uses to + trim the stream itself, so a mark is dropped in the same pass as the entry + it refers to -- never before it, which would let the entry be mapped again. + """ + + def __init__(self, client: RedisClient): + self.client = client + + @staticmethod + def _key(device: str) -> str: + return f"bot_consumed:{device}" + + async def mark_consumed(self, device: str, message_id: str, occurred_at: datetime) -> None: + """ + Records that the bot consumed a message, so it is never offered to the + mapping pipeline as content. + + Args: + device (str): session the message belongs to, as used in the stream key. + message_id (str): the Redis stream entry id of the message. + occurred_at (datetime): when the message was sent; becomes the score. + """ + score = occurred_at.timestamp() * 1000 + + try: + await self.client.zadd(self._key(device), {message_id: score}) + logger.debug(f"Message '{message_id}' marked as consumed by the bot") + + except RedisError as error: + logger.error(f"Marking message '{message_id}' as consumed by the bot failed with: '{error}'") + raise StoreUnavailable + + async def discard_bot_messages(self, device: str, entries: Sequence) -> list: + """ + Given a batch of stream entries, returns only the ones the bot did not + consume, preserving their order. + + Args: + device (str): session the entries belong to. + entries (Sequence): stream entries as returned by `xrange`, each a + tuple of (entry_id, fields). + + Returns: + list: the entries that are still candidates for mapping. + """ + if not entries: + return list(entries) + + try: + scores = await self.client.zmscore( + self._key(device), [entry_id for entry_id, _ in entries] + ) + + except RedisError as error: + logger.error(f"Fetching messages consumed by the bot failed with: '{error}'") + raise StoreUnavailable + + # zmscore returns None for members that are not in the sorted set, + # so a score means "the bot consumed this one". + return [entry for entry, score in zip(entries, scores) if score is None] + + async def cleanup(self, device: str, cutoff_time_ms: int) -> None: + """ + Drops marks older than the cutoff. Meant to be called with the same + cutoff used to trim the stream, so both expire together. + + Args: + device (str): session whose marks will be cleaned up. + cutoff_time_ms (int): epoch milliseconds; marks older than this go away. + """ + try: + removed = await self.client.zremrangebyscore(self._key(device), "-inf", cutoff_time_ms) + logger.info(f"cleanup: {removed} bot consumed marks deleted") + + except RedisError as error: + logger.error(f"Cleaning up messages consumed by the bot failed with: '{error}'") + raise StoreUnavailable diff --git a/chatmap-api/stream.py b/chatmap-api/stream.py index c0d5314..ffa994f 100644 --- a/chatmap-api/stream.py +++ b/chatmap-api/stream.py @@ -16,6 +16,7 @@ import asyncio from data import process_chat_entries from settings import STREAM_KEY, EXPIRING_MIN_MS, STREAM_LISTENER_TIME, DISABLE_STREAM_CLEANUP +from store.bot_consumed_messages_store import BotConsumedMessagesStore # Logs logger = logging.getLogger(__name__) @@ -25,6 +26,15 @@ redis_port = int(os.getenv("REDIS_PORT", 6379)) redis_client = redis.Redis(host=redis_host, port=redis_port, db=0) +# TRANSITIONAL: this listener predates the bot and pairs locations with content +# purely by time proximity, so it happily maps a reply the user typed to the +# bot. We chose to keep this pipeline running rather than rework it, so the bot +# tells us which messages it consumed and we skip them here. The direction we +# want is to move mapping into the conversation engine flow, which knows what +# each message means instead of guessing -- when that lands, this store and the +# two calls below should go away. See store/bot_consumed_messages_store.py. +bot_consumed_messages_store = BotConsumedMessagesStore(client=redis_client) + # Cleanup all messages for an user async def clean_user_stream(user: str): await redis_client.delete(f"{STREAM_KEY}:{user}") @@ -49,6 +59,10 @@ async def cleanup(user: str): for entry_id, _ in entries: await redis_client.xdel(f"{STREAM_KEY}:{user}", entry_id) logger.info(f'cleanup: {len(entries)} messages deleted') + # Same cutoff on purpose: a mark is only useful while the message it refers + # to is still in the stream, and dropping it any earlier would let that + # message be mapped again on the next pass. + await bot_consumed_messages_store.cleanup(user, cutoff_time_ms) # Get all sessions async def get_sessions(): @@ -85,6 +99,8 @@ async def stream_listener() -> None: sessions = await get_sessions() for user in sessions: entries = await redis_client.xrange(f'{STREAM_KEY}:{user}', min='-', max='+') + # Drop what the user said to the bot, keep what they meant to map + entries = await bot_consumed_messages_store.discard_bot_messages(user, entries) logger.info(f"{len(entries)} entries for user {user}") await process_chat_entries(user, entries) # Cleanup old messages diff --git a/chatmap-api/test/bot_tests/test_first_time_mapping_flow.py b/chatmap-api/test/bot_tests/test_first_time_mapping_flow.py index 96ef8c5..96c811b 100644 --- a/chatmap-api/test/bot_tests/test_first_time_mapping_flow.py +++ b/chatmap-api/test/bot_tests/test_first_time_mapping_flow.py @@ -1,3 +1,4 @@ +from datetime import datetime, timezone from unittest.mock import AsyncMock, call, patch import pytest @@ -10,21 +11,30 @@ translations, ) from conversation_engine.event import EventName +from store.bot_consumed_messages_store import BotConsumedMessagesStore from store.bot_state_store import BotStateStore from store.message_to_send_store import MessageToSendStore -def _make_flow(state, language=Language.ES, bot_state_store=None, message_to_send_store=None): +OCCURRED_AT = datetime(2026, 8, 9, 21, 7, 41, tzinfo=timezone.utc) + + +def _make_flow(state, language=Language.ES, bot_state_store=None, message_to_send_store=None, + bot_consumed_messages_store=None): return FirstTimeMappingFlow( state=state, language=language, bot_state_store=bot_state_store or AsyncMock(spec=BotStateStore), message_to_send_store=message_to_send_store or AsyncMock(spec=MessageToSendStore), + bot_consumed_messages_store=bot_consumed_messages_store or AsyncMock(spec=BotConsumedMessagesStore), ) def _ctx(**overrides): - fields = dict(state_key="key-1", recipient="user-enc-1", sender="device-1", answer="") + fields = dict( + state_key="key-1", recipient="user-enc-1", sender="device-1", answer="", + message_id="msg-1", occurred_at=OCCURRED_AT, + ) fields.update(overrides) return BotFlowContext(**fields) @@ -39,6 +49,7 @@ async def test_create_defaults_to_idle_and_default_language_when_no_state_stored bot_state_key="key-1", bot_state_store=bot_state_store, message_to_send_store=AsyncMock(spec=MessageToSendStore), + bot_consumed_messages_store=AsyncMock(spec=BotConsumedMessagesStore), ) assert flow.state == FirstTimeMappingState.IDLE @@ -61,6 +72,7 @@ async def test_create_restores_previously_stored_state_and_language( bot_state_key="key-1", bot_state_store=bot_state_store, message_to_send_store=AsyncMock(spec=MessageToSendStore), + bot_consumed_messages_store=AsyncMock(spec=BotConsumedMessagesStore), ) assert flow.state == expected_state @@ -75,6 +87,7 @@ async def test_create_falls_back_to_idle_for_an_unrecognized_state(): bot_state_key="key-1", bot_state_store=bot_state_store, message_to_send_store=AsyncMock(spec=MessageToSendStore), + bot_consumed_messages_store=AsyncMock(spec=BotConsumedMessagesStore), ) assert flow.state == FirstTimeMappingState.IDLE @@ -89,6 +102,7 @@ async def test_create_falls_back_to_default_language_for_an_unrecognized_languag bot_state_key="key-1", bot_state_store=bot_state_store, message_to_send_store=AsyncMock(spec=MessageToSendStore), + bot_consumed_messages_store=AsyncMock(spec=BotConsumedMessagesStore), ) assert flow.state == FirstTimeMappingState.WAITING_PHOTO @@ -328,3 +342,86 @@ async def test_on_fallback_from_mapping_completed_only_sends_the_fallback_messag sender=ctx.sender, to=ctx.recipient, message=translations[language.name]["fallback"], ) bot_state_store.save_state.assert_not_awaited() + + +# ---- messages the bot consumes as answers ---- + +async def test_answering_the_language_question_keeps_the_message_out_of_the_map(): + bot_consumed_messages_store = AsyncMock(spec=BotConsumedMessagesStore) + flow = _make_flow( + FirstTimeMappingState.WAITING_LANG, + bot_consumed_messages_store=bot_consumed_messages_store, + ) + ctx = _ctx(answer="1", message_id="1786309661000-0") + + await flow.on_ask_for_lang(ctx) + + bot_consumed_messages_store.mark_consumed.assert_awaited_once_with( + device=ctx.sender, message_id="1786309661000-0", occurred_at=OCCURRED_AT, + ) + + +async def test_an_invalid_language_answer_is_still_kept_out_of_the_map(): + bot_consumed_messages_store = AsyncMock(spec=BotConsumedMessagesStore) + flow = _make_flow( + FirstTimeMappingState.WAITING_LANG, + bot_consumed_messages_store=bot_consumed_messages_store, + ) + + await flow.on_ask_for_lang(_ctx(answer="no soy una opcion")) + + bot_consumed_messages_store.mark_consumed.assert_awaited_once() + + +async def test_the_text_that_opens_the_conversation_is_kept_out_of_the_map(): + bot_consumed_messages_store = AsyncMock(spec=BotConsumedMessagesStore) + flow = _make_flow( + FirstTimeMappingState.IDLE, + bot_consumed_messages_store=bot_consumed_messages_store, + ) + ctx = _ctx(answer="hola", message_id="1786309600000-0") + + await flow.on_ask_for_help(ctx) + + bot_consumed_messages_store.mark_consumed.assert_awaited_once_with( + device=ctx.sender, message_id="1786309600000-0", occurred_at=OCCURRED_AT, + ) + + +@pytest.mark.parametrize("state", [FirstTimeMappingState.IDLE, FirstTimeMappingState.WAITING_LANG]) +async def test_a_photo_or_location_routed_through_on_ask_for_help_stays_available_to_the_map(state): + # on_fallback delegates to on_ask_for_help in these states, and what + # arrives there is content: it carries no text + bot_consumed_messages_store = AsyncMock(spec=BotConsumedMessagesStore) + flow = _make_flow(state, bot_consumed_messages_store=bot_consumed_messages_store) + + await flow.on_fallback(_ctx(answer="")) + + bot_consumed_messages_store.mark_consumed.assert_not_awaited() + + +@pytest.mark.parametrize("state, handler_name", [ + (FirstTimeMappingState.WAITING_PHOTO, "on_photo_uploaded"), + (FirstTimeMappingState.WAITING_COORDINATES, "on_coordinates_sent"), +]) +async def test_content_stays_available_to_the_map(state, handler_name): + bot_consumed_messages_store = AsyncMock(spec=BotConsumedMessagesStore) + flow = _make_flow(state, bot_consumed_messages_store=bot_consumed_messages_store) + + await getattr(flow, handler_name)(_ctx()) + + bot_consumed_messages_store.mark_consumed.assert_not_awaited() + + +@pytest.mark.parametrize("state", [ + FirstTimeMappingState.WAITING_PHOTO, + FirstTimeMappingState.WAITING_COORDINATES, + FirstTimeMappingState.MAPPING_COMPLETED, +]) +async def test_a_message_that_only_hits_the_fallback_stays_available_to_the_map(state): + bot_consumed_messages_store = AsyncMock(spec=BotConsumedMessagesStore) + flow = _make_flow(state, bot_consumed_messages_store=bot_consumed_messages_store) + + await flow.on_fallback(_ctx(answer="cualquier cosa")) + + bot_consumed_messages_store.mark_consumed.assert_not_awaited() diff --git a/chatmap-api/test/conversation_engine_tests/test_bot_tool.py b/chatmap-api/test/conversation_engine_tests/test_bot_tool.py index b3d931f..0a8b317 100644 --- a/chatmap-api/test/conversation_engine_tests/test_bot_tool.py +++ b/chatmap-api/test/conversation_engine_tests/test_bot_tool.py @@ -12,6 +12,7 @@ from conversation_engine.event import Event, EventName from conversation_engine.tool import BotTool from settings import CHATMAP_ENC_KEY +from store.bot_consumed_messages_store import BotConsumedMessagesStore from store.bot_state_store import BotStateStore from store.message_to_send_store import MessageToSendStore from store.received_messages_store import ReceivedMessage @@ -40,20 +41,28 @@ def _conversation() -> Conversation: return Conversation(key=ConversationKey(sender="sender-1", chat="chat-1")) -def _make_bot_tool(bot_state_store=None, message_to_send_store=None) -> BotTool: +def _make_bot_tool( + bot_state_store=None, + message_to_send_store=None, + bot_consumed_messages_store=None +) -> BotTool: return BotTool( bot_state_store=bot_state_store or AsyncMock(spec=BotStateStore), message_to_send_store=message_to_send_store or AsyncMock(spec=MessageToSendStore), + bot_consumed_messages_store=bot_consumed_messages_store or AsyncMock(spec=BotConsumedMessagesStore), ) async def test_call_decrypts_text_and_delegates_to_the_flow(): plaintext = "hola bot" - message = _message(text=_encrypt(plaintext), sender="sender-1", chat="chat-1", sender_enc="recipient-enc-1") + message = _message( + id="msg-1", text=_encrypt(plaintext), sender="sender-1", chat="chat-1", sender_enc="recipient-enc-1" + ) event = Event(name=EventName.USER_SEND_TEXT, occurred_at=datetime.now(timezone.utc)) bot_state_store = AsyncMock(spec=BotStateStore) message_to_send_store = AsyncMock(spec=MessageToSendStore) - bot_tool = _make_bot_tool(bot_state_store, message_to_send_store) + bot_consumed_messages_store = AsyncMock(spec=BotConsumedMessagesStore) + bot_tool = _make_bot_tool(bot_state_store, message_to_send_store, bot_consumed_messages_store) fake_flow = AsyncMock() with patch.object(FirstTimeMappingFlow, "create", AsyncMock(return_value=fake_flow)) as mock_create: @@ -64,11 +73,13 @@ async def test_call_decrypts_text_and_delegates_to_the_flow(): bot_state_key=expected_key, bot_state_store=bot_state_store, message_to_send_store=message_to_send_store, + bot_consumed_messages_store=bot_consumed_messages_store, ) fake_flow.call.assert_awaited_once_with( current_event=EventName.USER_SEND_TEXT, context=BotFlowContext( state_key=expected_key, recipient="recipient-enc-1", sender="device-1", answer=plaintext, + message_id="msg-1", occurred_at=event.occurred_at, ), ) diff --git a/chatmap-api/test/conversation_engine_tests/test_flow.py b/chatmap-api/test/conversation_engine_tests/test_flow.py index 528dd35..38e0fc9 100644 --- a/chatmap-api/test/conversation_engine_tests/test_flow.py +++ b/chatmap-api/test/conversation_engine_tests/test_flow.py @@ -41,7 +41,7 @@ def expected_events(self): async def test_check_tool_for_event_invokes_the_registered_tool(): tool = AsyncMock() flow = Flow( - bot_state_store=Mock(), message_to_send_store=Mock(), + bot_state_store=Mock(), message_to_send_store=Mock(), bot_consumed_messages_store=Mock(), tools_by_events={EventName.USER_SEND_TEXT: tool}, ) event = _event(EventName.USER_SEND_TEXT) @@ -56,7 +56,7 @@ async def test_check_tool_for_event_invokes_the_registered_tool(): async def test_check_tool_for_event_does_nothing_when_no_tool_registered(): tool = AsyncMock() flow = Flow( - bot_state_store=Mock(), message_to_send_store=Mock(), + bot_state_store=Mock(), message_to_send_store=Mock(), bot_consumed_messages_store=Mock(), tools_by_events={EventName.USER_UPLOAD_PHOTO: tool}, ) event = _event(EventName.USER_SEND_TEXT) @@ -70,7 +70,7 @@ async def test_check_tool_for_event_does_nothing_when_no_tool_registered(): def test_expected_events_returns_the_tools_by_events_keys(): flow = Flow( - bot_state_store=Mock(), message_to_send_store=Mock(), + bot_state_store=Mock(), message_to_send_store=Mock(), bot_consumed_messages_store=Mock(), tools_by_events={EventName.USER_SEND_TEXT: AsyncMock(), EventName.USER_SEND_COORDINATES: AsyncMock()}, ) @@ -80,7 +80,9 @@ def test_expected_events_returns_the_tools_by_events_keys(): # ---- HelpFlow ---- def test_help_flow_shares_a_single_bot_tool_across_its_events(): - help_flow = HelpFlow(bot_state_store=Mock(), message_to_send_store=Mock()) + help_flow = HelpFlow( + bot_state_store=Mock(), message_to_send_store=Mock(), bot_consumed_messages_store=Mock(), + ) tools = help_flow.tools_by_events @@ -105,6 +107,7 @@ def test_registered_flows_returns_a_help_flow_with_its_own_stores(): assert isinstance(help_flow, HelpFlow) assert help_flow.bot_state_store is flows.bot_state_store assert help_flow.message_to_send_store is flows.message_to_send_store + assert help_flow.bot_consumed_messages_store is flows.bot_consumed_messages_store async def test_call_tools_for_dispatches_to_the_matching_flow(): From b71376366b46c5f7ef75ff7b379936d2dbb4d842 Mon Sep 17 00:00:00 2001 From: Joaquin Mansilla Date: Tue, 4 Aug 2026 11:09:46 -0300 Subject: [PATCH 2/3] adding survey functionality --- ...7209ac6b2b3a_add_survey_responses_table.py | 34 ++ chatmap-api/bot/flow.py | 15 +- .../bot/flows/first_time_mapping/flow.py | 187 +++++++++- .../flows/first_time_mapping/messages.json | 38 +- chatmap-api/cli/cli.py | 31 ++ chatmap-api/consumers/listener.py | 5 +- chatmap-api/conversation_engine/flow.py | 20 +- chatmap-api/conversation_engine/tool.py | 17 +- chatmap-api/db.py | 35 +- chatmap-api/results/error.py | 5 + chatmap-api/store/bot_state_store.py | 8 + chatmap-api/store/survey_responses_store.py | 21 ++ .../bot_tests/test_first_time_mapping_flow.py | 334 +++++++++++++++++- .../test_bot_tool.py | 38 +- .../conversation_engine_tests/test_flow.py | 13 +- docs/how-it-works/first_time_mapping_flow.md | 199 +++++++++++ 16 files changed, 937 insertions(+), 63 deletions(-) create mode 100644 chatmap-api/alembic/versions/7209ac6b2b3a_add_survey_responses_table.py create mode 100644 chatmap-api/store/survey_responses_store.py create mode 100644 docs/how-it-works/first_time_mapping_flow.md diff --git a/chatmap-api/alembic/versions/7209ac6b2b3a_add_survey_responses_table.py b/chatmap-api/alembic/versions/7209ac6b2b3a_add_survey_responses_table.py new file mode 100644 index 0000000..6490f54 --- /dev/null +++ b/chatmap-api/alembic/versions/7209ac6b2b3a_add_survey_responses_table.py @@ -0,0 +1,34 @@ +"""add survey_responses table + +Revision ID: 7209ac6b2b3a +Revises: b7b2a3b424b8 +Create Date: 2026-08-03 20:42:01.152034 + +""" +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + + +# revision identifiers, used by Alembic. +revision: str = '7209ac6b2b3a' +down_revision: Union[str, Sequence[str], None] = 'b7b2a3b424b8' +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + """Upgrade schema.""" + op.create_table( + 'survey_responses', + sa.Column('point_id', sa.String(), nullable=False), + sa.Column('answers', postgresql.JSONB(astext_type=sa.Text()), nullable=False), + sa.PrimaryKeyConstraint('point_id'), + ) + + +def downgrade() -> None: + """Downgrade schema.""" + op.drop_table('survey_responses') diff --git a/chatmap-api/bot/flow.py b/chatmap-api/bot/flow.py index 40f76ab..edd0adf 100644 --- a/chatmap-api/bot/flow.py +++ b/chatmap-api/bot/flow.py @@ -6,6 +6,7 @@ from store.bot_consumed_messages_store import BotConsumedMessagesStore from store.bot_state_store import BotStateStore from store.message_to_send_store import MessageToSendStore +from store.survey_responses_store import SurveyResponsesStore from typing import Callable, Awaitable import logging @@ -21,7 +22,7 @@ class Language(Enum): @classmethod def default(cls): - return Language.ES + return Language.EN @dataclass @@ -32,6 +33,11 @@ class BotFlowContext: answer: str message_id: str occurred_at: datetime + point_id: str | None + bot_state_store: BotStateStore + + async def fetch_field(self, field: str) -> str | None: + return await self.bot_state_store.fetch_field(bot_state_key=self.state_key, field=field) class BotFlow(ABC): @@ -40,13 +46,15 @@ def __init__(self, language: Language, bot_state_store: BotStateStore, message_to_send_store: MessageToSendStore, - bot_consumed_messages_store: BotConsumedMessagesStore + bot_consumed_messages_store: BotConsumedMessagesStore, + survey_responses_store: SurveyResponsesStore ): self.state = state self.language = language self.bot_state_store = bot_state_store self.message_to_send_store = message_to_send_store self.bot_consumed_messages_store = bot_consumed_messages_store + self.survey_responses_store = survey_responses_store @classmethod @abstractmethod @@ -55,7 +63,8 @@ async def create( bot_state_key: str, bot_state_store: BotStateStore, message_to_send_store: MessageToSendStore, - bot_consumed_messages_store: BotConsumedMessagesStore + bot_consumed_messages_store: BotConsumedMessagesStore, + survey_responses_store: SurveyResponsesStore ): ... diff --git a/chatmap-api/bot/flows/first_time_mapping/flow.py b/chatmap-api/bot/flows/first_time_mapping/flow.py index ee38227..1726dc7 100644 --- a/chatmap-api/bot/flows/first_time_mapping/flow.py +++ b/chatmap-api/bot/flows/first_time_mapping/flow.py @@ -1,9 +1,12 @@ from bot.flow import BotFlow, BotTransitions, BotFlowContext, not_handler_created, Language from conversation_engine.event import EventName from enum import Enum, auto + +from results.error import BotStateWithoutPointId from store.bot_consumed_messages_store import BotConsumedMessagesStore from store.bot_state_store import BotStateStore from store.message_to_send_store import MessageToSendStore +from store.survey_responses_store import SurveyResponsesStore from pathlib import Path import json @@ -14,16 +17,29 @@ _MESSAGES_PATH = Path(__file__).parent / "messages.json" +FALLBACK_LIMIT = 3 + with open(_MESSAGES_PATH, encoding="utf-8") as f: translations = json.load(f) +def _build_options_message(question: str, options: dict[str, str]) -> str: + options_text = "\n".join(f"{code}️⃣ {label}" for code, label in options.items()) + return f"{question}\n\n{options_text}" + + +def _lang_options() -> dict[str, str]: + return {str(i): lang.value for i, lang in enumerate(Language, start=1)} + + class FirstTimeMappingState(Enum): IDLE = auto() WAITING_LANG = auto() WAITING_PHOTO = auto() WAITING_COORDINATES = auto() + WAITING_DAMAGE_LEVEL = auto() MAPPING_COMPLETED = auto() + WAITING_RECOVERY_CHOICE = auto() class FirstTimeMappingFlow(BotFlow): @@ -35,7 +51,8 @@ async def create( bot_state_key: str, bot_state_store: BotStateStore, message_to_send_store: MessageToSendStore, - bot_consumed_messages_store: BotConsumedMessagesStore + bot_consumed_messages_store: BotConsumedMessagesStore, + survey_responses_store: SurveyResponsesStore ): result = await bot_state_store.fetch_state(bot_state_key=bot_state_key) @@ -43,6 +60,7 @@ async def create( raw_state = result.get("state") state = FirstTimeMappingState.__members__.get(raw_state, FirstTimeMappingState.IDLE) if isinstance( raw_state, str) else FirstTimeMappingState.IDLE + raw_language = result.get("lang") language = Language.__members__.get(raw_language, Language.default()) if isinstance(raw_language, str) else Language.default() @@ -55,7 +73,8 @@ async def create( language=language, bot_state_store=bot_state_store, message_to_send_store=message_to_send_store, - bot_consumed_messages_store=bot_consumed_messages_store + bot_consumed_messages_store=bot_consumed_messages_store, + survey_responses_store=survey_responses_store ) async def call(self, current_event: EventName, context: BotFlowContext) -> None: @@ -76,8 +95,8 @@ async def on_ask_for_help( # The text was consumed to open the conversation -- the user got a # language menu, not a mapped point -- so it must not reach the map. - # Guarded on text because on_fallback reuses this handler for photos - # and locations, and those are content. + # Guarded on text so that anything reaching this handler without text + # is treated as content and stays available to the map. if ctx.answer: await self.bot_consumed_messages_store.mark_consumed( device=ctx.sender, message_id=ctx.message_id, occurred_at=ctx.occurred_at @@ -87,13 +106,14 @@ async def on_ask_for_help( await self.message_to_send_store.send_message( sender=ctx.sender, to=ctx.recipient, - message=translations[self.language.name]["ask_for_lang"] + message=_build_options_message(translations[self.language.name]["ask_for_lang_question"], _lang_options()) ) logger.info("storing new bot event...") await self.bot_state_store.save_state( bot_state_key=ctx.state_key, - state=FirstTimeMappingState.WAITING_LANG + state=FirstTimeMappingState.WAITING_LANG, + bot_info={"fallback_count": "0"} ) async def on_ask_for_lang( @@ -116,7 +136,8 @@ async def on_ask_for_lang( await self.message_to_send_store.send_message( sender=ctx.sender, to=ctx.recipient, - message=translations[self.language.name]["ask_for_lang"] + message=_build_options_message(translations[self.language.name]["ask_for_lang_question"], + _lang_options()) ) return @@ -132,7 +153,7 @@ async def on_ask_for_lang( await self.bot_state_store.save_state( bot_state_key=ctx.state_key, state=FirstTimeMappingState.WAITING_PHOTO, - bot_info={"lang": language_key} + bot_info={"lang": language_key, "fallback_count": "0"} ) async def on_photo_uploaded(self, ctx: BotFlowContext) -> None: @@ -147,13 +168,64 @@ async def on_photo_uploaded(self, ctx: BotFlowContext) -> None: logger.info("storing new bot event...") await self.bot_state_store.save_state( bot_state_key=ctx.state_key, - state=FirstTimeMappingState.WAITING_COORDINATES + state=FirstTimeMappingState.WAITING_COORDINATES, + bot_info={"fallback_count": "0"} ) async def on_coordinates_sent(self, ctx: BotFlowContext) -> None: logger.info("Handling: on_ask_for_help") logger.info("sending message...") + await self.message_to_send_store.send_message( + sender=ctx.sender, to=ctx.recipient, + message=_build_options_message( + translations[self.language.name]["damage_level_question"], + translations[self.language.name]["damage_level_options"] + ) + ) + + logger.info("storing new bot event...") + await self.bot_state_store.save_state( + bot_state_key=ctx.state_key, + state=FirstTimeMappingState.WAITING_DAMAGE_LEVEL, + bot_info={"point_id": ctx.point_id, "fallback_count": "0"} + ) + + async def on_damage_level_answered(self, ctx: BotFlowContext) -> None: + logger.info("Handling: on_damage_level_answered") + + # Answering the survey, not mapping. Marked before validating, so an + # invalid answer is kept out of the map too. + await self.bot_consumed_messages_store.mark_consumed( + device=ctx.sender, message_id=ctx.message_id, occurred_at=ctx.occurred_at + ) + + options = translations[self.language.name]["damage_level_options"] + raw_answer = ctx.answer + + if raw_answer not in options: + logger.info(f"Invalid damage level option received: '{raw_answer}', re-asking...") + + await self.message_to_send_store.send_message( + sender=ctx.sender, to=ctx.recipient, + message=_build_options_message(translations[self.language.name]["damage_level_question"], options) + ) + return + + logger.info("storing survey response...") + point_id = await ctx.fetch_field("point_id") + + if not point_id: + logger.error(f"Trying to store a survey response for state: '{ctx.state_key}' does not exist point id") + raise BotStateWithoutPointId(message_id=ctx.message_id) + + await self.survey_responses_store.add_response( + point_id=point_id, + question=translations[self.language.name]["damage_level_question"], + answer=options[raw_answer] + ) + + logger.info("sending message...") await self.message_to_send_store.send_message( sender=ctx.sender, to=ctx.recipient, message=translations[self.language.name]["end_flow"] @@ -162,7 +234,8 @@ async def on_coordinates_sent(self, ctx: BotFlowContext) -> None: logger.info("storing new bot event...") await self.bot_state_store.save_state( bot_state_key=ctx.state_key, - state=FirstTimeMappingState.MAPPING_COMPLETED + state=FirstTimeMappingState.MAPPING_COMPLETED, + bot_info={"fallback_count": "0"} ) logger.info("bot flow end, deleting state...") @@ -170,31 +243,117 @@ async def on_coordinates_sent(self, ctx: BotFlowContext) -> None: bot_state_key=ctx.state_key, ) + async def on_recovery_choice_answered(self, ctx: BotFlowContext) -> None: + logger.info("Handling: on_recovery_choice_answered") + + # Answering the bot, not mapping. Marked before validating, so an + # invalid answer is kept out of the map too. Restarting delegates to + # on_ask_for_help, which marks the same id again -- zadd is idempotent. + await self.bot_consumed_messages_store.mark_consumed( + device=ctx.sender, message_id=ctx.message_id, occurred_at=ctx.occurred_at + ) + + options = translations[self.language.name]["recovery_options"] + raw_answer = ctx.answer + + if raw_answer not in options: + logger.info(f"Invalid recovery option received: '{raw_answer}', re-asking...") + + await self.message_to_send_store.send_message( + sender=ctx.sender, to=ctx.recipient, + message=_build_options_message(translations[self.language.name]["recovery_question"], options) + ) + return + + if raw_answer == "1": + logger.info("user chose to cancel the flow...") + await self.message_to_send_store.send_message( + sender=ctx.sender, to=ctx.recipient, + message=translations[self.language.name]["flow_cancelled"] + ) + await self.bot_state_store.delete_state(bot_state_key=ctx.state_key) + return + + logger.info("user chose to restart the flow...") + await self.on_ask_for_help(ctx) + async def on_fallback(self, ctx: BotFlowContext) -> None: + raw_count = await ctx.fetch_field("fallback_count") + count = int(raw_count) + 1 if raw_count else 1 + + if count > FALLBACK_LIMIT: + logger.info("fallback limit reached, offering cancel/restart...") + + await self.message_to_send_store.send_message( + sender=ctx.sender, to=ctx.recipient, + message=_build_options_message( + translations[self.language.name]["recovery_question"], + translations[self.language.name]["recovery_options"] + ) + ) + await self.bot_state_store.save_state( + bot_state_key=ctx.state_key, + state=FirstTimeMappingState.WAITING_RECOVERY_CHOICE, + bot_info={"fallback_count": str(count)} + ) + return + await self.message_to_send_store.send_message( sender=ctx.sender, to=ctx.recipient, message=translations[self.language.name]["fallback"] ) match self.state: - case FirstTimeMappingState.IDLE: - await self.on_ask_for_help(ctx) - case FirstTimeMappingState.WAITING_LANG: - await self.on_ask_for_help(ctx) + case FirstTimeMappingState.IDLE | FirstTimeMappingState.WAITING_LANG: + await self.message_to_send_store.send_message( + sender=ctx.sender, to=ctx.recipient, + message=_build_options_message(translations[self.language.name]["ask_for_lang_question"], + _lang_options()) + ) + await self.bot_state_store.save_state( + bot_state_key=ctx.state_key, + state=FirstTimeMappingState.WAITING_LANG, + bot_info={"fallback_count": str(count)} + ) case FirstTimeMappingState.WAITING_PHOTO: await self.message_to_send_store.send_message( sender=ctx.sender, to=ctx.recipient, message=translations[self.language.name]["ask_for_photo"] ) + await self.bot_state_store.save_state( + bot_state_key=ctx.state_key, + state=FirstTimeMappingState.WAITING_PHOTO, + bot_info={"fallback_count": str(count)} + ) case FirstTimeMappingState.WAITING_COORDINATES: await self.message_to_send_store.send_message( sender=ctx.sender, to=ctx.recipient, message=translations[self.language.name]["ask_for_coordinate"] ) + await self.bot_state_store.save_state( + bot_state_key=ctx.state_key, + state=FirstTimeMappingState.WAITING_COORDINATES, + bot_info={"fallback_count": str(count)} + ) + case FirstTimeMappingState.WAITING_DAMAGE_LEVEL: + await self.message_to_send_store.send_message( + sender=ctx.sender, to=ctx.recipient, + message=_build_options_message( + translations[self.language.name]["damage_level_question"], + translations[self.language.name]["damage_level_options"] + ) + ) + await self.bot_state_store.save_state( + bot_state_key=ctx.state_key, + state=FirstTimeMappingState.WAITING_DAMAGE_LEVEL, + bot_info={"fallback_count": str(count)} + ) transitions: BotTransitions = { (FirstTimeMappingState.IDLE, EventName.USER_SEND_TEXT): on_ask_for_help, (FirstTimeMappingState.WAITING_LANG, EventName.USER_SEND_TEXT): on_ask_for_lang, (FirstTimeMappingState.WAITING_PHOTO, EventName.USER_UPLOAD_PHOTO): on_photo_uploaded, (FirstTimeMappingState.WAITING_COORDINATES, EventName.USER_SEND_COORDINATES): on_coordinates_sent, + (FirstTimeMappingState.WAITING_DAMAGE_LEVEL, EventName.USER_SEND_TEXT): on_damage_level_answered, + (FirstTimeMappingState.WAITING_RECOVERY_CHOICE, EventName.USER_SEND_TEXT): on_recovery_choice_answered, } diff --git a/chatmap-api/bot/flows/first_time_mapping/messages.json b/chatmap-api/bot/flows/first_time_mapping/messages.json index c6ba31a..72faff8 100644 --- a/chatmap-api/bot/flows/first_time_mapping/messages.json +++ b/chatmap-api/bot/flows/first_time_mapping/messages.json @@ -1,30 +1,50 @@ { "ES": { - "ask_for_lang": "¡Hola! 👋 Soy el bot de ChatMap\n\n¿Cual es tu idioma?\n\n1️⃣ Español\n2️⃣ Ingles\n3️⃣ Portugués\n4️⃣ Francés", - "ask_for_photo": "Comenzá mandando el contenido:\n1\uFE0F⃣ Tocá el clip (\uD83D\uDCCE).\n2\uFE0F⃣ Elegí el contenido que querés mapear.", + "ask_for_lang_question": "¡Hola! 👋 Soy el bot de ChatMap\n\n¿Cual es tu idioma?", + "ask_for_photo": "Comenzá mandando el contenido:\n1️⃣ Tocá el clip (📎).\n2️⃣ Elegí el contenido que querés mapear.", "ask_for_coordinate": "Ahora compartí la *ubicación*:\n1️⃣ Tocá el clip (📎).\n2️⃣ Elegí *Ubicación*.", + "damage_level_question": "¿Cuál es el nivel de daño?", + "damage_level_options": {"1": "Alto", "2": "Medio", "3": "Bajo"}, "end_flow": "¡Listo! Con esos dos pasos tu archivo queda en el mapa. 🗺️", - "fallback": "Respuesta incorrecta!" + "fallback": "Respuesta incorrecta!", + "recovery_question": "No estamos logrando avanzar. ¿Querés cancelar el mapeo o reiniciarlo desde el principio?", + "recovery_options": {"1": "Cancelar", "2": "Reiniciar"}, + "flow_cancelled": "Mapeo cancelado. Cuando quieras, escribime para empezar de nuevo. 👋" }, "EN": { - "ask_for_lang": "Hi! 👋 I'm the ChatMap bot\n\nWhat is your language?\n\n1️⃣ Spanish\n2️⃣ English\n3️⃣ Portuguese\n4️⃣ French", + "ask_for_lang_question": "Hi! 👋 I'm the ChatMap bot\n\nWhat is your language?", "ask_for_photo": "Start by sending the content:\n1️⃣ Tap the clip (📎).\n2️⃣ Choose the content you want to map.", "ask_for_coordinate": "Now share the *location*:\n1️⃣ Tap the clip (📎).\n2️⃣ Choose *Location*.", + "damage_level_question": "What is the damage level?", + "damage_level_options": {"1": "High", "2": "Medium", "3": "Low"}, "end_flow": "Done! With those two steps your file is now on the map. 🗺️", - "fallback": "Incorrect answer!" + "fallback": "Incorrect answer!", + "recovery_question": "We're not making progress. Do you want to cancel the mapping or restart it from the beginning?", + "recovery_options": {"1": "Cancel", "2": "Restart"}, + "flow_cancelled": "Mapping cancelled. Whenever you're ready, send me a message to start again. 👋" }, "PT": { - "ask_for_lang": "Olá! 👋 Eu sou o bot do ChatMap\n\nQual é o seu idioma?\n\n1️⃣ Espanhol\n2️⃣ Inglês\n3️⃣ Português\n4️⃣ Francês", + "ask_for_lang_question": "Olá! 👋 Eu sou o bot do ChatMap\n\nQual é o seu idioma?", "ask_for_photo": "Comece enviando o conteúdo:\n1️⃣ Toque no clipe (📎).\n2️⃣ Escolha o conteúdo que deseja mapear.", "ask_for_coordinate": "Agora compartilhe a *localização*:\n1️⃣ Toque no clipe (📎).\n2️⃣ Escolha *Localização*.", + "damage_level_question": "Qual é o nível de dano?", + "damage_level_options": {"1": "Alto", "2": "Médio", "3": "Baixo"}, "end_flow": "Pronto! Com esses dois passos seu arquivo já está no mapa. 🗺️", - "fallback": "Resposta incorreta!" + "fallback": "Resposta incorreta!", + "recovery_question": "Não estamos conseguindo avançar. Você quer cancelar o mapeamento ou reiniciá-lo desde o início?", + "recovery_options": {"1": "Cancelar", "2": "Reiniciar"}, + "flow_cancelled": "Mapeamento cancelado. Quando quiser, me mande uma mensagem para começar de novo. 👋" }, "FR": { - "ask_for_lang": "Bonjour ! 👋 Je suis le bot de ChatMap\n\nQuelle est votre langue ?\n\n1️⃣ Espagnol\n2️⃣ Anglais\n3️⃣ Portugais\n4️⃣ Français", + "ask_for_lang_question": "Bonjour ! 👋 Je suis le bot de ChatMap\n\nQuelle est votre langue ?", "ask_for_photo": "Commence par envoyer le contenu :\n1️⃣ Appuie sur le trombone (📎).\n2️⃣ Choisis le contenu que tu veux cartographier.", "ask_for_coordinate": "Maintenant, partage l'*emplacement* :\n1️⃣ Appuie sur le trombone (📎).\n2️⃣ Choisis *Position*.", + "damage_level_question": "Quel est le niveau de dommage ?", + "damage_level_options": {"1": "Élevé", "2": "Moyen", "3": "Faible"}, "end_flow": "Voilà ! Avec ces deux étapes, ton fichier est maintenant sur la carte. 🗺️", - "fallback": "Réponse incorrecte !" + "fallback": "Réponse incorrecte !", + "recovery_question": "On n'avance pas. Tu veux annuler le mapping ou le recommencer depuis le début ?", + "recovery_options": {"1": "Annuler", "2": "Recommencer"}, + "flow_cancelled": "Mapping annulé. Quand tu veux, envoie-moi un message pour recommencer. 👋" } } diff --git a/chatmap-api/cli/cli.py b/chatmap-api/cli/cli.py index 2e3312d..9e69181 100644 --- a/chatmap-api/cli/cli.py +++ b/chatmap-api/cli/cli.py @@ -4,8 +4,10 @@ import asyncio from datetime import datetime, timezone from redis import asyncio as async_redis +from sqlalchemy import delete, select from consumers.listener import ConversationsStateListener +from db import SurveyResponse, get_db_session from store.message_to_send_store import MessageToSendStore logging.basicConfig( @@ -17,9 +19,11 @@ app = typer.Typer() received_message_app = typer.Typer(help="Manage entries on the messages stream (inbound webhook events)") message_to_send_app = typer.Typer(help="Manage entries on the to_send stream (chatmap-im-connector delivery queue)") +survey_response_app = typer.Typer(help="Inspect entries in the survey_responses table") app.add_typer(received_message_app, name="received-message") app.add_typer(message_to_send_app, name="message-to-send") +app.add_typer(survey_response_app, name="survey-response") redis_host = "localhost" redis_port = 6380 @@ -156,6 +160,33 @@ async def run(): asyncio.run(run()) +@survey_response_app.command("list") +def survey_response_list( + point_id: str = typer.Option(None, help="Only list the row for this point id"), +): + """List every row currently in the survey_responses table.""" + db = get_db_session() + stmt = select(SurveyResponse) + if point_id: + stmt = stmt.where(SurveyResponse.point_id == point_id) + for row in db.execute(stmt).scalars(): + typer.echo({"point_id": row.point_id, "answers": row.answers}) + + +@survey_response_app.command("delete") +def survey_response_delete( + point_id: str = typer.Option(None, help="Only delete the row for this point id; omit to delete all rows"), +): + """Delete rows from the survey_responses table.""" + db = get_db_session() + stmt = delete(SurveyResponse) + if point_id: + stmt = stmt.where(SurveyResponse.point_id == point_id) + result = db.execute(stmt) + db.commit() + typer.echo(f"deleted {result.rowcount} row(s) from survey_responses") + + @app.command("conversations-listener") def conversations_listener(): """List every entry in a Redis stream and report which State(s), if any, it matches.""" diff --git a/chatmap-api/consumers/listener.py b/chatmap-api/consumers/listener.py index 86c364a..2f420f6 100644 --- a/chatmap-api/consumers/listener.py +++ b/chatmap-api/consumers/listener.py @@ -3,7 +3,7 @@ from datetime import timedelta from conversation_engine.flow import Flows -from results.error import UnknownConversation, StoreUnavailable +from results.error import UnknownConversation, StoreUnavailable, BotStateWithoutPointId from store.conversation_store import ConversationStore from redis import asyncio as async_redis @@ -71,6 +71,9 @@ async def process_conversation_for(self, device: str, flows: Flows, semaphore: S logger.warning(f"The request failed due to connectivity issues; it will automatically retry") except UnknownConversation: logger.warning(f"Conversation not found; it will automatically retry") + except BotStateWithoutPointId as error: + logger.warning(f"Message: '{error.message_id}' with incorrect state removing from PEL") + await flows.received_messages_store.mark_message_as_processed(message_id=error.message_id, device=device) async def start(self): semaphore = asyncio.Semaphore(10) diff --git a/chatmap-api/conversation_engine/flow.py b/chatmap-api/conversation_engine/flow.py index ea3e37e..19f67e7 100644 --- a/chatmap-api/conversation_engine/flow.py +++ b/chatmap-api/conversation_engine/flow.py @@ -13,6 +13,7 @@ from store.message_to_send_store import MessageToSendStore from store.received_messages_store import ReceivedMessagesStore, ReceivedMessage +from store.survey_responses_store import SurveyResponsesStore Tool = Callable[[Event, ReceivedMessage, str, Conversation], Awaitable[None]] @@ -21,12 +22,18 @@ class Flow(ABC): name: str window_time: ClassVar[timedelta] - def __init__(self, bot_state_store: BotStateStore, message_to_send_store: MessageToSendStore, - bot_consumed_messages_store: BotConsumedMessagesStore, - tools_by_events: Optional[dict[EventName, Tool]] = None): + def __init__( + self, + bot_state_store: BotStateStore, + message_to_send_store: MessageToSendStore, + bot_consumed_messages_store: BotConsumedMessagesStore, + survey_responses_store: SurveyResponsesStore, + tools_by_events: Optional[dict[EventName, Tool]] = None + ): self.bot_state_store = bot_state_store self.message_to_send_store = message_to_send_store self.bot_consumed_messages_store = bot_consumed_messages_store + self.survey_responses_store = survey_responses_store self.tools_by_events = tools_by_events if tools_by_events is not None else self.default_tools_by_events() def expected_events(self) -> set[EventName]: @@ -58,7 +65,8 @@ def default_tools_by_events(self) -> dict[EventName, Tool]: bot_tool = BotTool( bot_state_store=self.bot_state_store, message_to_send_store=self.message_to_send_store, - bot_consumed_messages_store=self.bot_consumed_messages_store + bot_consumed_messages_store=self.bot_consumed_messages_store, + survey_responses_store=self.survey_responses_store ) return { @@ -74,13 +82,15 @@ def __init__(self, client: RedisClient): self.message_to_send_store = MessageToSendStore(client=client) self.received_messages_store = ReceivedMessagesStore(client=client) self.bot_consumed_messages_store = BotConsumedMessagesStore(client=client) + self.survey_responses_store = SurveyResponsesStore() def registered_flows(self) -> list[Flow]: return [ HelpFlow( bot_state_store=self.bot_state_store, message_to_send_store=self.message_to_send_store, - bot_consumed_messages_store=self.bot_consumed_messages_store + bot_consumed_messages_store=self.bot_consumed_messages_store, + survey_responses_store=self.survey_responses_store ) ] diff --git a/chatmap-api/conversation_engine/tool.py b/chatmap-api/conversation_engine/tool.py index e537328..c4182ad 100644 --- a/chatmap-api/conversation_engine/tool.py +++ b/chatmap-api/conversation_engine/tool.py @@ -5,11 +5,12 @@ from bot.flow import BotFlowContext from conversation_engine.conversation import Conversation -from conversation_engine.event import Event +from conversation_engine.event import Event, EventName from bot.flows.first_time_mapping.flow import FirstTimeMappingFlow from store.bot_consumed_messages_store import BotConsumedMessagesStore from store.bot_state_store import BotStateStore from store.message_to_send_store import MessageToSendStore +from store.survey_responses_store import SurveyResponsesStore from settings import CHATMAP_ENC_KEY from store.received_messages_store import ReceivedMessage @@ -35,11 +36,13 @@ class BotTool: def __init__( self, bot_state_store: BotStateStore, message_to_send_store: MessageToSendStore, - bot_consumed_messages_store: BotConsumedMessagesStore + bot_consumed_messages_store: BotConsumedMessagesStore, + survey_responses_store: SurveyResponsesStore ): self.bot_state_store = bot_state_store self.message_to_send_store = message_to_send_store self.bot_consumed_messages_store = bot_consumed_messages_store + self.survey_responses_store = survey_responses_store async def __call__(self, event: Event, message: ReceivedMessage, device: str, conversation: Conversation): bot_state_key = f"bot_state:{FirstTimeMappingFlow.name}:{message.sender}{message.chat}" @@ -48,15 +51,21 @@ async def __call__(self, event: Event, message: ReceivedMessage, device: str, co bot_state_key=bot_state_key, bot_state_store=self.bot_state_store, message_to_send_store=self.message_to_send_store, - bot_consumed_messages_store=self.bot_consumed_messages_store + bot_consumed_messages_store=self.bot_consumed_messages_store, + survey_responses_store=self.survey_responses_store ) + + point_id = message.id if event.name == EventName.USER_SEND_COORDINATES else None + context = BotFlowContext( state_key=bot_state_key, recipient=message.sender_enc, sender=device, answer=_decrypt_text(message.text), message_id=message.id, - occurred_at=event.occurred_at + occurred_at=event.occurred_at, + point_id=point_id, + bot_state_store=self.bot_state_store ) await flow.call(current_event=event.name, context=context) diff --git a/chatmap-api/db.py b/chatmap-api/db.py index b393c98..2da14c3 100644 --- a/chatmap-api/db.py +++ b/chatmap-api/db.py @@ -15,7 +15,7 @@ create_engine, Column, String, select, DateTime, ForeignKey, func, Enum as SqlEnum, Boolean, ) -from sqlalchemy.dialects.postgresql import insert +from sqlalchemy.dialects.postgresql import insert, JSONB from sqlalchemy.pool import NullPool from sqlalchemy.orm import sessionmaker, declarative_base, Session, relationship from geoalchemy2 import Geometry @@ -151,6 +151,39 @@ def add_points(db: Session, points, user_id): db.commit() +# Model representing the accumulated survey answers for a Point +class SurveyResponse(Base): + __tablename__ = "survey_responses" + point_id = Column(String, primary_key=True) + answers = Column(JSONB, nullable=False, default=list) + + +# Append a question/answer pair to a point's survey responses +def add_survey_response(db: Session, point_id: str, question: str, answer: str): + """ + Appends a single {question, answer} pair to the survey_responses row for + a point. Creates the row on first answer; subsequent answers for the + same point_id are appended to the existing JSON array rather than + overwriting it. + + Args: + db (Session): SQLAlchemy database session + point_id (str): id shared with the eventual Point row (no FK - the + Point may not exist yet, or ever, when this is called) + question (str): localized question text + answer (str): localized answer label + """ + stmt = insert(SurveyResponse).values( + point_id=point_id, answers=[{"question": question, "answer": answer}] + ) + stmt = stmt.on_conflict_do_update( + index_elements=["point_id"], + set_={"answers": SurveyResponse.answers.op("||")(stmt.excluded.answers)}, + ) + db.execute(stmt) + db.commit() + + # Dependency to get a database session def get_db_session(): """ diff --git a/chatmap-api/results/error.py b/chatmap-api/results/error.py index 8d2eed1..7efbdb0 100644 --- a/chatmap-api/results/error.py +++ b/chatmap-api/results/error.py @@ -4,3 +4,8 @@ class StoreUnavailable(Exception): class UnknownConversation(Exception): ... + + +class BotStateWithoutPointId(Exception): + def __init__(self, message_id): + self.message_id = message_id diff --git a/chatmap-api/store/bot_state_store.py b/chatmap-api/store/bot_state_store.py index 4ba3655..69d551a 100644 --- a/chatmap-api/store/bot_state_store.py +++ b/chatmap-api/store/bot_state_store.py @@ -24,6 +24,14 @@ async def fetch_state(self, bot_state_key: str) -> dict: logger.error(f"Fetch bot state failed with: '{error}'") raise StoreUnavailable + async def fetch_field(self, bot_state_key: str, field: str) -> str | None: + try: + return await self.client.hget(bot_state_key, field) + + except RedisError as error: + logger.error(f"Fetch bot state field failed with: '{error}'") + raise StoreUnavailable + async def save_state( self, bot_state_key: str, diff --git a/chatmap-api/store/survey_responses_store.py b/chatmap-api/store/survey_responses_store.py new file mode 100644 index 0000000..8b64d91 --- /dev/null +++ b/chatmap-api/store/survey_responses_store.py @@ -0,0 +1,21 @@ +import logging + +from sqlalchemy.exc import SQLAlchemyError + +from db import add_survey_response, get_db_session +from results.error import StoreUnavailable + +logger = logging.getLogger(__name__) + + +class SurveyResponsesStore: + @classmethod + async def add_response(cls, point_id: str, question: str, answer: str) -> None: + db = get_db_session() + try: + add_survey_response(db=db, point_id=point_id, question=question, answer=answer) + logger.debug(f"Survey response saved for point '{point_id}'") + except SQLAlchemyError as error: + db.rollback() + logger.error(f"Save survey response for point '{point_id}' failed with: '{error}'") + raise StoreUnavailable diff --git a/chatmap-api/test/bot_tests/test_first_time_mapping_flow.py b/chatmap-api/test/bot_tests/test_first_time_mapping_flow.py index 96c811b..2df8163 100644 --- a/chatmap-api/test/bot_tests/test_first_time_mapping_flow.py +++ b/chatmap-api/test/bot_tests/test_first_time_mapping_flow.py @@ -9,31 +9,39 @@ FirstTimeMappingFlow, FirstTimeMappingState, translations, + _build_options_message, + _lang_options, ) from conversation_engine.event import EventName +from results.error import BotStateWithoutPointId from store.bot_consumed_messages_store import BotConsumedMessagesStore from store.bot_state_store import BotStateStore from store.message_to_send_store import MessageToSendStore +from store.survey_responses_store import SurveyResponsesStore OCCURRED_AT = datetime(2026, 8, 9, 21, 7, 41, tzinfo=timezone.utc) def _make_flow(state, language=Language.ES, bot_state_store=None, message_to_send_store=None, - bot_consumed_messages_store=None): + bot_consumed_messages_store=None, survey_responses_store=None): return FirstTimeMappingFlow( state=state, language=language, bot_state_store=bot_state_store or AsyncMock(spec=BotStateStore), message_to_send_store=message_to_send_store or AsyncMock(spec=MessageToSendStore), bot_consumed_messages_store=bot_consumed_messages_store or AsyncMock(spec=BotConsumedMessagesStore), + survey_responses_store=survey_responses_store or AsyncMock(spec=SurveyResponsesStore), ) def _ctx(**overrides): + bot_state_store = AsyncMock(spec=BotStateStore) + bot_state_store.fetch_field.return_value = None + fields = dict( - state_key="key-1", recipient="user-enc-1", sender="device-1", answer="", - message_id="msg-1", occurred_at=OCCURRED_AT, + state_key="key-1", recipient="user-enc-1", sender="device-1", answer="", message_id="msg-1", + occurred_at=OCCURRED_AT, point_id=None, bot_state_store=bot_state_store, ) fields.update(overrides) return BotFlowContext(**fields) @@ -50,6 +58,7 @@ async def test_create_defaults_to_idle_and_default_language_when_no_state_stored bot_state_store=bot_state_store, message_to_send_store=AsyncMock(spec=MessageToSendStore), bot_consumed_messages_store=AsyncMock(spec=BotConsumedMessagesStore), + survey_responses_store=AsyncMock(spec=SurveyResponsesStore), ) assert flow.state == FirstTimeMappingState.IDLE @@ -61,6 +70,7 @@ async def test_create_defaults_to_idle_and_default_language_when_no_state_stored ("WAITING_LANG", "ES", FirstTimeMappingState.WAITING_LANG, Language.ES), ("WAITING_PHOTO", "EN", FirstTimeMappingState.WAITING_PHOTO, Language.EN), ("WAITING_COORDINATES", "PT", FirstTimeMappingState.WAITING_COORDINATES, Language.PT), + ("WAITING_DAMAGE_LEVEL", "PT", FirstTimeMappingState.WAITING_DAMAGE_LEVEL, Language.PT), ("MAPPING_COMPLETED", "FR", FirstTimeMappingState.MAPPING_COMPLETED, Language.FR), ]) async def test_create_restores_previously_stored_state_and_language( @@ -73,6 +83,7 @@ async def test_create_restores_previously_stored_state_and_language( bot_state_store=bot_state_store, message_to_send_store=AsyncMock(spec=MessageToSendStore), bot_consumed_messages_store=AsyncMock(spec=BotConsumedMessagesStore), + survey_responses_store=AsyncMock(spec=SurveyResponsesStore), ) assert flow.state == expected_state @@ -88,6 +99,7 @@ async def test_create_falls_back_to_idle_for_an_unrecognized_state(): bot_state_store=bot_state_store, message_to_send_store=AsyncMock(spec=MessageToSendStore), bot_consumed_messages_store=AsyncMock(spec=BotConsumedMessagesStore), + survey_responses_store=AsyncMock(spec=SurveyResponsesStore), ) assert flow.state == FirstTimeMappingState.IDLE @@ -103,6 +115,7 @@ async def test_create_falls_back_to_default_language_for_an_unrecognized_languag bot_state_store=bot_state_store, message_to_send_store=AsyncMock(spec=MessageToSendStore), bot_consumed_messages_store=AsyncMock(spec=BotConsumedMessagesStore), + survey_responses_store=AsyncMock(spec=SurveyResponsesStore), ) assert flow.state == FirstTimeMappingState.WAITING_PHOTO @@ -116,13 +129,15 @@ async def test_create_falls_back_to_default_language_for_an_unrecognized_languag (FirstTimeMappingState.WAITING_LANG, EventName.USER_SEND_TEXT, "on_ask_for_lang"), (FirstTimeMappingState.WAITING_PHOTO, EventName.USER_UPLOAD_PHOTO, "on_photo_uploaded"), (FirstTimeMappingState.WAITING_COORDINATES, EventName.USER_SEND_COORDINATES, "on_coordinates_sent"), + (FirstTimeMappingState.WAITING_DAMAGE_LEVEL, EventName.USER_SEND_TEXT, "on_damage_level_answered"), + (FirstTimeMappingState.WAITING_RECOVERY_CHOICE, EventName.USER_SEND_TEXT, "on_recovery_choice_answered"), ]) def test_transitions_table_wiring(state, event, handler_name): assert FirstTimeMappingFlow.transitions[(state, event)] is getattr(FirstTimeMappingFlow, handler_name) def test_transitions_table_has_no_unexpected_entries(): - assert len(FirstTimeMappingFlow.transitions) == 4 + assert len(FirstTimeMappingFlow.transitions) == 6 # ---- call() dispatch ---- @@ -192,10 +207,11 @@ async def test_on_ask_for_help_sends_tutorial_in_current_language_and_moves_to_w message_store.send_message.assert_awaited_once_with( sender=ctx.sender, to=ctx.recipient, - message=translations[language.name]["ask_for_lang"], + message=_build_options_message(translations[language.name]["ask_for_lang_question"], _lang_options()), ) bot_state_store.save_state.assert_awaited_once_with( bot_state_key=ctx.state_key, state=FirstTimeMappingState.WAITING_LANG, + bot_info={"fallback_count": "0"}, ) @@ -219,7 +235,7 @@ async def test_on_ask_for_lang_resolves_the_selected_language(answer, expected_l ) bot_state_store.save_state.assert_awaited_once_with( bot_state_key=ctx.state_key, state=FirstTimeMappingState.WAITING_PHOTO, - bot_info={"lang": expected_language.name}, + bot_info={"lang": expected_language.name, "fallback_count": "0"}, ) @@ -233,7 +249,7 @@ async def test_on_ask_for_lang_reasks_in_current_language_for_an_invalid_option( message_store.send_message.assert_awaited_once_with( sender=ctx.sender, to=ctx.recipient, - message=translations["EN"]["ask_for_lang"], + message=_build_options_message(translations["EN"]["ask_for_lang_question"], _lang_options()), ) bot_state_store.save_state.assert_not_awaited() @@ -253,34 +269,121 @@ async def test_on_photo_uploaded_asks_for_coordinates_in_current_language(langua ) bot_state_store.save_state.assert_awaited_once_with( bot_state_key=ctx.state_key, state=FirstTimeMappingState.WAITING_COORDINATES, + bot_info={"fallback_count": "0"}, ) @pytest.mark.parametrize("language", list(Language)) -async def test_on_coordinates_sent_completes_the_flow_and_clears_state(language): +async def test_on_coordinates_sent_asks_for_damage_level_and_stores_point_id(language): message_store = AsyncMock(spec=MessageToSendStore) bot_state_store = AsyncMock(spec=BotStateStore) flow = _make_flow(FirstTimeMappingState.WAITING_COORDINATES, language, bot_state_store, message_store) - ctx = _ctx() + ctx = _ctx(point_id="point-99") await flow.on_coordinates_sent(ctx) + message_store.send_message.assert_awaited_once_with( + sender=ctx.sender, to=ctx.recipient, + message=_build_options_message( + translations[language.name]["damage_level_question"], translations[language.name]["damage_level_options"] + ), + ) + bot_state_store.save_state.assert_awaited_once_with( + bot_state_key=ctx.state_key, state=FirstTimeMappingState.WAITING_DAMAGE_LEVEL, + bot_info={"point_id": "point-99", "fallback_count": "0"}, + ) + + +@pytest.mark.parametrize("language, answer, expected_label", [ + (Language.ES, "1", "Alto"), + (Language.ES, "2", "Medio"), + (Language.ES, "3", "Bajo"), + (Language.EN, "1", "High"), + (Language.PT, "2", "Médio"), + (Language.FR, "3", "Faible"), +]) +async def test_on_damage_level_answered_persists_response_and_completes_the_flow(language, answer, expected_label): + message_store = AsyncMock(spec=MessageToSendStore) + bot_state_store = AsyncMock(spec=BotStateStore) + survey_responses_store = AsyncMock(spec=SurveyResponsesStore) + flow = _make_flow( + FirstTimeMappingState.WAITING_DAMAGE_LEVEL, language, bot_state_store, message_store, + survey_responses_store=survey_responses_store, + ) + ctx_bot_state_store = AsyncMock(spec=BotStateStore) + ctx_bot_state_store.fetch_field.return_value = "point-99" + ctx = _ctx(answer=answer, bot_state_store=ctx_bot_state_store) + + await flow.on_damage_level_answered(ctx) + + ctx_bot_state_store.fetch_field.assert_awaited_once_with(bot_state_key=ctx.state_key, field="point_id") + survey_responses_store.add_response.assert_awaited_once_with( + point_id="point-99", + question=translations[language.name]["damage_level_question"], + answer=expected_label, + ) message_store.send_message.assert_awaited_once_with( sender=ctx.sender, to=ctx.recipient, message=translations[language.name]["end_flow"], ) bot_state_store.save_state.assert_awaited_once_with( bot_state_key=ctx.state_key, state=FirstTimeMappingState.MAPPING_COMPLETED, + bot_info={"fallback_count": "0"}, ) bot_state_store.delete_state.assert_awaited_once_with(bot_state_key=ctx.state_key) assert [call[0] for call in bot_state_store.method_calls] == ["save_state", "delete_state"] +async def test_on_damage_level_answered_raises_when_point_id_is_missing_from_state(): + message_store = AsyncMock(spec=MessageToSendStore) + bot_state_store = AsyncMock(spec=BotStateStore) + survey_responses_store = AsyncMock(spec=SurveyResponsesStore) + flow = _make_flow( + FirstTimeMappingState.WAITING_DAMAGE_LEVEL, Language.EN, bot_state_store, message_store, + survey_responses_store=survey_responses_store, + ) + ctx_bot_state_store = AsyncMock(spec=BotStateStore) + ctx_bot_state_store.fetch_field.return_value = None + ctx = _ctx(answer="1", message_id="reply-msg-1", bot_state_store=ctx_bot_state_store) + + with pytest.raises(BotStateWithoutPointId) as exc_info: + await flow.on_damage_level_answered(ctx) + + assert exc_info.value.message_id == "reply-msg-1" + survey_responses_store.add_response.assert_not_awaited() + bot_state_store.save_state.assert_not_awaited() + bot_state_store.delete_state.assert_not_awaited() + + +async def test_on_damage_level_answered_reasks_for_an_invalid_option_without_persisting(): + message_store = AsyncMock(spec=MessageToSendStore) + bot_state_store = AsyncMock(spec=BotStateStore) + survey_responses_store = AsyncMock(spec=SurveyResponsesStore) + flow = _make_flow( + FirstTimeMappingState.WAITING_DAMAGE_LEVEL, Language.EN, bot_state_store, message_store, + survey_responses_store=survey_responses_store, + ) + ctx = _ctx(answer="9") + + await flow.on_damage_level_answered(ctx) + + message_store.send_message.assert_awaited_once_with( + sender=ctx.sender, to=ctx.recipient, + message=_build_options_message( + translations["EN"]["damage_level_question"], translations["EN"]["damage_level_options"] + ), + ) + survey_responses_store.add_response.assert_not_awaited() + bot_state_store.save_state.assert_not_awaited() + bot_state_store.delete_state.assert_not_awaited() + ctx.bot_state_store.fetch_field.assert_not_awaited() + + # ---- on_fallback() ---- @pytest.mark.parametrize("state", [FirstTimeMappingState.IDLE, FirstTimeMappingState.WAITING_LANG]) @pytest.mark.parametrize("language", list(Language)) -async def test_on_fallback_from_idle_or_waiting_lang_delegates_to_on_ask_for_help(state, language): +async def test_on_fallback_from_idle_or_waiting_lang_reasks_for_language(state, language): message_store = AsyncMock(spec=MessageToSendStore) bot_state_store = AsyncMock(spec=BotStateStore) flow = _make_flow(state, language, bot_state_store, message_store) @@ -290,10 +393,12 @@ async def test_on_fallback_from_idle_or_waiting_lang_delegates_to_on_ask_for_hel assert message_store.send_message.await_args_list == [ call(sender=ctx.sender, to=ctx.recipient, message=translations[language.name]["fallback"]), - call(sender=ctx.sender, to=ctx.recipient, message=translations[language.name]["ask_for_lang"]), + call(sender=ctx.sender, to=ctx.recipient, + message=_build_options_message(translations[language.name]["ask_for_lang_question"], _lang_options())), ] bot_state_store.save_state.assert_awaited_once_with( bot_state_key=ctx.state_key, state=FirstTimeMappingState.WAITING_LANG, + bot_info={"fallback_count": "1"}, ) @@ -310,7 +415,10 @@ async def test_on_fallback_from_waiting_photo_reasks_for_photo(language): call(sender=ctx.sender, to=ctx.recipient, message=translations[language.name]["fallback"]), call(sender=ctx.sender, to=ctx.recipient, message=translations[language.name]["ask_for_photo"]), ] - bot_state_store.save_state.assert_not_awaited() + bot_state_store.save_state.assert_awaited_once_with( + bot_state_key=ctx.state_key, state=FirstTimeMappingState.WAITING_PHOTO, + bot_info={"fallback_count": "1"}, + ) @pytest.mark.parametrize("language", list(Language)) @@ -326,7 +434,31 @@ async def test_on_fallback_from_waiting_coordinates_reasks_for_coordinates(langu call(sender=ctx.sender, to=ctx.recipient, message=translations[language.name]["fallback"]), call(sender=ctx.sender, to=ctx.recipient, message=translations[language.name]["ask_for_coordinate"]), ] - bot_state_store.save_state.assert_not_awaited() + bot_state_store.save_state.assert_awaited_once_with( + bot_state_key=ctx.state_key, state=FirstTimeMappingState.WAITING_COORDINATES, + bot_info={"fallback_count": "1"}, + ) + + +@pytest.mark.parametrize("language", list(Language)) +async def test_on_fallback_from_waiting_damage_level_reasks_for_damage_level(language): + message_store = AsyncMock(spec=MessageToSendStore) + bot_state_store = AsyncMock(spec=BotStateStore) + flow = _make_flow(FirstTimeMappingState.WAITING_DAMAGE_LEVEL, language, bot_state_store, message_store) + ctx = _ctx() + + await flow.on_fallback(ctx) + + assert message_store.send_message.await_args_list == [ + call(sender=ctx.sender, to=ctx.recipient, message=translations[language.name]["fallback"]), + call(sender=ctx.sender, to=ctx.recipient, message=_build_options_message( + translations[language.name]["damage_level_question"], translations[language.name]["damage_level_options"] + )), + ] + bot_state_store.save_state.assert_awaited_once_with( + bot_state_key=ctx.state_key, state=FirstTimeMappingState.WAITING_DAMAGE_LEVEL, + bot_info={"fallback_count": "1"}, + ) @pytest.mark.parametrize("language", list(Language)) @@ -344,6 +476,124 @@ async def test_on_fallback_from_mapping_completed_only_sends_the_fallback_messag bot_state_store.save_state.assert_not_awaited() +@pytest.mark.parametrize("state, expected_message_key", [ + (FirstTimeMappingState.WAITING_PHOTO, "ask_for_photo"), + (FirstTimeMappingState.WAITING_COORDINATES, "ask_for_coordinate"), +]) +async def test_on_fallback_persists_the_incremented_count_on_top_of_a_prior_one(state, expected_message_key): + message_store = AsyncMock(spec=MessageToSendStore) + bot_state_store = AsyncMock(spec=BotStateStore) + flow = _make_flow(state, Language.EN, bot_state_store, message_store) + ctx = _ctx() + ctx.bot_state_store.fetch_field.return_value = "2" + + await flow.on_fallback(ctx) + + ctx.bot_state_store.fetch_field.assert_awaited_once_with(bot_state_key=ctx.state_key, field="fallback_count") + bot_state_store.save_state.assert_awaited_once_with( + bot_state_key=ctx.state_key, state=state, + bot_info={"fallback_count": "3"}, + ) + + +@pytest.mark.parametrize("state", list(FirstTimeMappingState)) +async def test_on_fallback_reaching_the_limit_shows_the_recovery_prompt(state): + message_store = AsyncMock(spec=MessageToSendStore) + bot_state_store = AsyncMock(spec=BotStateStore) + flow = _make_flow(state, Language.EN, bot_state_store, message_store) + ctx = _ctx() + ctx.bot_state_store.fetch_field.return_value = "3" + + await flow.on_fallback(ctx) + + message_store.send_message.assert_awaited_once_with( + sender=ctx.sender, to=ctx.recipient, + message=_build_options_message( + translations["EN"]["recovery_question"], translations["EN"]["recovery_options"] + ), + ) + bot_state_store.save_state.assert_awaited_once_with( + bot_state_key=ctx.state_key, state=FirstTimeMappingState.WAITING_RECOVERY_CHOICE, + bot_info={"fallback_count": "4"}, + ) + + +async def test_on_fallback_keeps_reshowing_the_recovery_prompt_once_past_the_limit(): + message_store = AsyncMock(spec=MessageToSendStore) + bot_state_store = AsyncMock(spec=BotStateStore) + flow = _make_flow(FirstTimeMappingState.WAITING_RECOVERY_CHOICE, Language.EN, bot_state_store, message_store) + ctx = _ctx() + ctx.bot_state_store.fetch_field.return_value = "4" + + await flow.on_fallback(ctx) + + message_store.send_message.assert_awaited_once_with( + sender=ctx.sender, to=ctx.recipient, + message=_build_options_message( + translations["EN"]["recovery_question"], translations["EN"]["recovery_options"] + ), + ) + bot_state_store.save_state.assert_awaited_once_with( + bot_state_key=ctx.state_key, state=FirstTimeMappingState.WAITING_RECOVERY_CHOICE, + bot_info={"fallback_count": "5"}, + ) + + +# ---- on_recovery_choice_answered() ---- + +@pytest.mark.parametrize("language", list(Language)) +async def test_on_recovery_choice_answered_cancels_and_deletes_state(language): + message_store = AsyncMock(spec=MessageToSendStore) + bot_state_store = AsyncMock(spec=BotStateStore) + flow = _make_flow(FirstTimeMappingState.WAITING_RECOVERY_CHOICE, language, bot_state_store, message_store) + ctx = _ctx(answer="1") + + await flow.on_recovery_choice_answered(ctx) + + message_store.send_message.assert_awaited_once_with( + sender=ctx.sender, to=ctx.recipient, message=translations[language.name]["flow_cancelled"], + ) + bot_state_store.delete_state.assert_awaited_once_with(bot_state_key=ctx.state_key) + bot_state_store.save_state.assert_not_awaited() + + +@pytest.mark.parametrize("language", list(Language)) +async def test_on_recovery_choice_answered_restarts_the_flow(language): + message_store = AsyncMock(spec=MessageToSendStore) + bot_state_store = AsyncMock(spec=BotStateStore) + flow = _make_flow(FirstTimeMappingState.WAITING_RECOVERY_CHOICE, language, bot_state_store, message_store) + ctx = _ctx(answer="2") + + await flow.on_recovery_choice_answered(ctx) + + message_store.send_message.assert_awaited_once_with( + sender=ctx.sender, to=ctx.recipient, + message=_build_options_message(translations[language.name]["ask_for_lang_question"], _lang_options()), + ) + bot_state_store.save_state.assert_awaited_once_with( + bot_state_key=ctx.state_key, state=FirstTimeMappingState.WAITING_LANG, + bot_info={"fallback_count": "0"}, + ) + bot_state_store.delete_state.assert_not_awaited() + + +async def test_on_recovery_choice_answered_reasks_for_an_invalid_option(): + message_store = AsyncMock(spec=MessageToSendStore) + bot_state_store = AsyncMock(spec=BotStateStore) + flow = _make_flow(FirstTimeMappingState.WAITING_RECOVERY_CHOICE, Language.EN, bot_state_store, message_store) + ctx = _ctx(answer="9") + + await flow.on_recovery_choice_answered(ctx) + + message_store.send_message.assert_awaited_once_with( + sender=ctx.sender, to=ctx.recipient, + message=_build_options_message(translations["EN"]["recovery_question"], translations["EN"]["recovery_options"]), + ) + bot_state_store.save_state.assert_not_awaited() + bot_state_store.delete_state.assert_not_awaited() + + + # ---- messages the bot consumes as answers ---- async def test_answering_the_language_question_keeps_the_message_out_of_the_map(): @@ -388,14 +638,60 @@ async def test_the_text_that_opens_the_conversation_is_kept_out_of_the_map(): ) -@pytest.mark.parametrize("state", [FirstTimeMappingState.IDLE, FirstTimeMappingState.WAITING_LANG]) -async def test_a_photo_or_location_routed_through_on_ask_for_help_stays_available_to_the_map(state): - # on_fallback delegates to on_ask_for_help in these states, and what - # arrives there is content: it carries no text +async def test_answering_the_damage_level_question_keeps_the_message_out_of_the_map(): bot_consumed_messages_store = AsyncMock(spec=BotConsumedMessagesStore) - flow = _make_flow(state, bot_consumed_messages_store=bot_consumed_messages_store) + flow = _make_flow( + FirstTimeMappingState.WAITING_DAMAGE_LEVEL, + bot_consumed_messages_store=bot_consumed_messages_store, + ) + ctx_bot_state_store = AsyncMock(spec=BotStateStore) + ctx_bot_state_store.fetch_field.return_value = "point-99" + ctx = _ctx(answer="1", message_id="1786309700000-0", bot_state_store=ctx_bot_state_store) + + await flow.on_damage_level_answered(ctx) + + bot_consumed_messages_store.mark_consumed.assert_awaited_once_with( + device=ctx.sender, message_id="1786309700000-0", occurred_at=OCCURRED_AT, + ) + + +async def test_an_invalid_damage_level_answer_is_still_kept_out_of_the_map(): + bot_consumed_messages_store = AsyncMock(spec=BotConsumedMessagesStore) + flow = _make_flow( + FirstTimeMappingState.WAITING_DAMAGE_LEVEL, + bot_consumed_messages_store=bot_consumed_messages_store, + ) + + await flow.on_damage_level_answered(_ctx(answer="no soy una opcion")) + + bot_consumed_messages_store.mark_consumed.assert_awaited_once() + + +@pytest.mark.parametrize("answer", ["1", "2", "no soy una opcion"]) +async def test_answering_the_recovery_question_keeps_the_message_out_of_the_map(answer): + bot_consumed_messages_store = AsyncMock(spec=BotConsumedMessagesStore) + flow = _make_flow( + FirstTimeMappingState.WAITING_RECOVERY_CHOICE, + bot_consumed_messages_store=bot_consumed_messages_store, + ) + ctx = _ctx(answer=answer, message_id="1786309800000-0") + + await flow.on_recovery_choice_answered(ctx) + + bot_consumed_messages_store.mark_consumed.assert_awaited_with( + device=ctx.sender, message_id="1786309800000-0", occurred_at=OCCURRED_AT, + ) + + +async def test_a_message_without_text_reaching_on_ask_for_help_stays_available_to_the_map(): + # whatever arrives here carrying no text is content, not an answer + bot_consumed_messages_store = AsyncMock(spec=BotConsumedMessagesStore) + flow = _make_flow( + FirstTimeMappingState.IDLE, + bot_consumed_messages_store=bot_consumed_messages_store, + ) - await flow.on_fallback(_ctx(answer="")) + await flow.on_ask_for_help(_ctx(answer="")) bot_consumed_messages_store.mark_consumed.assert_not_awaited() diff --git a/chatmap-api/test/conversation_engine_tests/test_bot_tool.py b/chatmap-api/test/conversation_engine_tests/test_bot_tool.py index 0a8b317..6e4492f 100644 --- a/chatmap-api/test/conversation_engine_tests/test_bot_tool.py +++ b/chatmap-api/test/conversation_engine_tests/test_bot_tool.py @@ -16,6 +16,7 @@ from store.bot_state_store import BotStateStore from store.message_to_send_store import MessageToSendStore from store.received_messages_store import ReceivedMessage +from store.survey_responses_store import SurveyResponsesStore def _encrypt(plaintext: str) -> str: @@ -44,12 +45,14 @@ def _conversation() -> Conversation: def _make_bot_tool( bot_state_store=None, message_to_send_store=None, - bot_consumed_messages_store=None + bot_consumed_messages_store=None, + survey_responses_store=None ) -> BotTool: return BotTool( bot_state_store=bot_state_store or AsyncMock(spec=BotStateStore), message_to_send_store=message_to_send_store or AsyncMock(spec=MessageToSendStore), bot_consumed_messages_store=bot_consumed_messages_store or AsyncMock(spec=BotConsumedMessagesStore), + survey_responses_store=survey_responses_store or AsyncMock(spec=SurveyResponsesStore), ) @@ -62,7 +65,10 @@ async def test_call_decrypts_text_and_delegates_to_the_flow(): bot_state_store = AsyncMock(spec=BotStateStore) message_to_send_store = AsyncMock(spec=MessageToSendStore) bot_consumed_messages_store = AsyncMock(spec=BotConsumedMessagesStore) - bot_tool = _make_bot_tool(bot_state_store, message_to_send_store, bot_consumed_messages_store) + survey_responses_store = AsyncMock(spec=SurveyResponsesStore) + bot_tool = _make_bot_tool( + bot_state_store, message_to_send_store, bot_consumed_messages_store, survey_responses_store + ) fake_flow = AsyncMock() with patch.object(FirstTimeMappingFlow, "create", AsyncMock(return_value=fake_flow)) as mock_create: @@ -74,16 +80,42 @@ async def test_call_decrypts_text_and_delegates_to_the_flow(): bot_state_store=bot_state_store, message_to_send_store=message_to_send_store, bot_consumed_messages_store=bot_consumed_messages_store, + survey_responses_store=survey_responses_store, ) fake_flow.call.assert_awaited_once_with( current_event=EventName.USER_SEND_TEXT, context=BotFlowContext( state_key=expected_key, recipient="recipient-enc-1", sender="device-1", answer=plaintext, - message_id="msg-1", occurred_at=event.occurred_at, + message_id="msg-1", occurred_at=event.occurred_at, point_id=None, bot_state_store=bot_state_store, ), ) +async def test_call_sets_point_id_to_message_id_only_for_the_coordinates_event(): + message = _message(id="point-42", text="") + event = Event(name=EventName.USER_SEND_COORDINATES, occurred_at=datetime.now(timezone.utc)) + bot_tool = _make_bot_tool() + fake_flow = AsyncMock() + + with patch.object(FirstTimeMappingFlow, "create", AsyncMock(return_value=fake_flow)): + await bot_tool(event=event, message=message, device="device-1", conversation=_conversation()) + + assert fake_flow.call.await_args.kwargs["context"].point_id == "point-42" + + +@pytest.mark.parametrize("event_name", [n for n in EventName if n != EventName.USER_SEND_COORDINATES]) +async def test_call_leaves_point_id_none_for_non_coordinates_events(event_name): + message = _message(id="point-42", text="") + event = Event(name=event_name, occurred_at=datetime.now(timezone.utc)) + bot_tool = _make_bot_tool() + fake_flow = AsyncMock() + + with patch.object(FirstTimeMappingFlow, "create", AsyncMock(return_value=fake_flow)): + await bot_tool(event=event, message=message, device="device-1", conversation=_conversation()) + + assert fake_flow.call.await_args.kwargs["context"].point_id is None + + async def test_call_passes_through_empty_text_without_decrypting(): message = _message(text="") event = Event(name=EventName.USER_SEND_TEXT, occurred_at=datetime.now(timezone.utc)) diff --git a/chatmap-api/test/conversation_engine_tests/test_flow.py b/chatmap-api/test/conversation_engine_tests/test_flow.py index 38e0fc9..b009be4 100644 --- a/chatmap-api/test/conversation_engine_tests/test_flow.py +++ b/chatmap-api/test/conversation_engine_tests/test_flow.py @@ -41,7 +41,8 @@ def expected_events(self): async def test_check_tool_for_event_invokes_the_registered_tool(): tool = AsyncMock() flow = Flow( - bot_state_store=Mock(), message_to_send_store=Mock(), bot_consumed_messages_store=Mock(), + bot_state_store=Mock(), message_to_send_store=Mock(), + bot_consumed_messages_store=Mock(), survey_responses_store=Mock(), tools_by_events={EventName.USER_SEND_TEXT: tool}, ) event = _event(EventName.USER_SEND_TEXT) @@ -56,7 +57,8 @@ async def test_check_tool_for_event_invokes_the_registered_tool(): async def test_check_tool_for_event_does_nothing_when_no_tool_registered(): tool = AsyncMock() flow = Flow( - bot_state_store=Mock(), message_to_send_store=Mock(), bot_consumed_messages_store=Mock(), + bot_state_store=Mock(), message_to_send_store=Mock(), + bot_consumed_messages_store=Mock(), survey_responses_store=Mock(), tools_by_events={EventName.USER_UPLOAD_PHOTO: tool}, ) event = _event(EventName.USER_SEND_TEXT) @@ -70,7 +72,8 @@ async def test_check_tool_for_event_does_nothing_when_no_tool_registered(): def test_expected_events_returns_the_tools_by_events_keys(): flow = Flow( - bot_state_store=Mock(), message_to_send_store=Mock(), bot_consumed_messages_store=Mock(), + bot_state_store=Mock(), message_to_send_store=Mock(), + bot_consumed_messages_store=Mock(), survey_responses_store=Mock(), tools_by_events={EventName.USER_SEND_TEXT: AsyncMock(), EventName.USER_SEND_COORDINATES: AsyncMock()}, ) @@ -81,7 +84,8 @@ def test_expected_events_returns_the_tools_by_events_keys(): def test_help_flow_shares_a_single_bot_tool_across_its_events(): help_flow = HelpFlow( - bot_state_store=Mock(), message_to_send_store=Mock(), bot_consumed_messages_store=Mock(), + bot_state_store=Mock(), message_to_send_store=Mock(), + bot_consumed_messages_store=Mock(), survey_responses_store=Mock(), ) tools = help_flow.tools_by_events @@ -108,6 +112,7 @@ def test_registered_flows_returns_a_help_flow_with_its_own_stores(): assert help_flow.bot_state_store is flows.bot_state_store assert help_flow.message_to_send_store is flows.message_to_send_store assert help_flow.bot_consumed_messages_store is flows.bot_consumed_messages_store + assert help_flow.survey_responses_store is flows.survey_responses_store async def test_call_tools_for_dispatches_to_the_matching_flow(): diff --git a/docs/how-it-works/first_time_mapping_flow.md b/docs/how-it-works/first_time_mapping_flow.md new file mode 100644 index 0000000..f1a38f5 --- /dev/null +++ b/docs/how-it-works/first_time_mapping_flow.md @@ -0,0 +1,199 @@ +# Feature spec — First-time mapping bot flow: fallback recovery prompt + +## Context + +`FirstTimeMappingFlow` (`bot/flows/first_time_mapping/flow.py`) is the bot's +internal state machine for onboarding a new mapper: pick a language, send a +photo, send coordinates, rate the damage level. It's a `Tool` bound inside +the conversation engine (see +[conversation_engine.md](conversation_engine.md#two-layers-of-flow)), but its +own state machine — `FirstTimeMappingState`, the `transitions` table, and +`on_fallback` — is entirely internal, persisted via `BotStateStore`, +independent of the engine's `Conversation`/`Event` model. + +```mermaid +flowchart TD + IDLE --> |"USER_SEND_TEXT"| WAITING_LANG + WAITING_LANG --> |"USER_SEND_TEXT (valid lang)"| WAITING_PHOTO + WAITING_PHOTO --> |"USER_UPLOAD_PHOTO"| WAITING_COORDINATES + WAITING_COORDINATES --> |"USER_SEND_COORDINATES"| WAITING_DAMAGE_LEVEL + WAITING_DAMAGE_LEVEL --> |"USER_SEND_TEXT (valid level)"| MAPPING_COMPLETED + MAPPING_COMPLETED --> |"delete_state (immediate)"| GONE(["key deleted"]) +``` + +`call()` dispatches on `(state, EventName)` against the `transitions` table. +No entry for the pair → `on_fallback`. Two handlers (`on_ask_for_lang`, +`on_damage_level_answered`) additionally re-ask directly, without going +through `on_fallback`, when the event *does* match but the answer's value is +invalid (e.g. an out-of-range option number) — that pattern is unchanged by +this feature. + +This spec covers a new behavior layered on top of `on_fallback`: after 3 +consecutive fallbacks, the 4th offers the user a way out instead of repeating +the same re-ask forever. + +## Purpose + +Give the user stuck in a fallback loop — 4 consecutive events the flow +couldn't handle — an explicit choice to cancel the mapping or restart it from +the beginning, instead of silently repeating the same re-ask indefinitely. + +## Scope + +**In scope** + +- A per-conversation `fallback_count`, persisted in the same + `BotStateStore` hash as `state`/`lang`/`point_id`. +- On the 4th consecutive `on_fallback` call, replace the normal per-state + re-ask with a cancel/restart prompt, and move to a new state, + `WAITING_RECOVERY_CHOICE`. +- Handling the user's answer to that prompt: cancel (delete the flow's + state) or restart (jump back to the language question). +- New `messages.json` entries (all 4 languages) for the prompt, its two + options, and the cancellation confirmation. + +**Out of scope / deferred** + +- `MAPPING_COMPLETED`'s existing `on_fallback` branch — untouched, not part + of the counter (see [Decisions](#decisions), #6). +- Anything in the conversation engine itself (`Flow`/`Event`/`Tool`) — this + feature is entirely internal to `FirstTimeMappingFlow`'s own state + machine. +- The existing "invalid option, re-ask directly" behavior in + `on_ask_for_lang` / `on_damage_level_answered` — still bypasses + `on_fallback` entirely, exactly as today. +- Recording/reporting abandoned mappings (analytics on cancellations) — not + requested, nothing in the codebase tracks this today. + +## Behavior + +| Situation | Trigger | Observable result | +|---|---|---| +| 1st–3rd consecutive fallback | `call()` finds no transition for `(state, event)`; `fallback_count` was 0, 1, or 2 | `fallback_count` incremented and persisted (state unchanged); generic fallback message + existing per-state re-ask sent — unchanged from today | +| 4th consecutive fallback | Same, but `fallback_count` was already 3 | Instead of the per-state re-ask: sends the recovery prompt (cancel/restart options), saves `state=WAITING_RECOVERY_CHOICE` with `fallback_count=4` | +| 5th+ consecutive fallback, already in `WAITING_RECOVERY_CHOICE` | User sends an event type that still doesn't match any transition (e.g. a photo instead of typing 1/2) | Re-shows the same recovery prompt; `fallback_count` keeps growing but has no further observable effect — no special-casing needed since the threshold check runs before the per-state `match` | +| Recovery answer = cancel | `(WAITING_RECOVERY_CHOICE, USER_SEND_TEXT)`, answer is the "cancel" option | `delete_state` (whole Redis hash key removed) + cancellation message sent; next user message starts fresh from `IDLE` | +| Recovery answer = restart | `(WAITING_RECOVERY_CHOICE, USER_SEND_TEXT)`, answer is the "restart" option | Delegates to `on_ask_for_help`: sends the language question, saves `state=WAITING_LANG` with `fallback_count` reset to `"0"` | +| Recovery answer invalid | `(WAITING_RECOVERY_CHOICE, USER_SEND_TEXT)`, answer isn't the cancel/restart option | Re-sends the recovery prompt; no `save_state` call — same pattern as the existing invalid-option handlers | +| Genuine state advance | Any existing handler's success path (valid language, photo uploaded, coordinates sent, valid damage level) | `fallback_count` explicitly reset to `"0"` as part of that handler's existing `save_state` call | +| Fallback while `state == MAPPING_COMPLETED` | Practically unreachable — the state's Redis key is deleted immediately after being set, so no later event can ever be dispatched against a live `MAPPING_COMPLETED` state | Unchanged: sends only the generic fallback message, no `save_state`, not counted — purely defensive dead branch | + +## Contract + +**`fallback_count` field** + +- Stored as a string integer in the existing `bot_state::` hash. Absent field == `0`. +- Only two kinds of writes touch it: an explicit reset to `"0"` on a genuine + state advance, or an explicit increment on an `on_fallback` call. No other + code path touches it. + +**Threshold** + +- `FALLBACK_LIMIT = 3` (module-level constant). `on_fallback` computes + `count = int(fetched or 0) + 1`; `count > FALLBACK_LIMIT` triggers the + recovery prompt instead of the per-state branch. + +**New state** + +- `FirstTimeMappingState.WAITING_RECOVERY_CHOICE`, added to the existing + enum. + +**New transition** + +- `(WAITING_RECOVERY_CHOICE, EventName.USER_SEND_TEXT) → on_recovery_choice_answered` + +**`on_fallback` structure** + +``` +count = fetched fallback_count + 1 + +if count > FALLBACK_LIMIT: + send recovery prompt + save_state(state=WAITING_RECOVERY_CHOICE, bot_info={"fallback_count": str(count)}) + return + +send generic fallback message +match self.state: + IDLE | WAITING_LANG: + send language question directly (no longer delegates to on_ask_for_help) + save_state(state=WAITING_LANG, bot_info={"fallback_count": str(count)}) + WAITING_PHOTO: + send photo re-ask + save_state(state=WAITING_PHOTO, bot_info={"fallback_count": str(count)}) # new: this branch didn't save_state before + WAITING_COORDINATES: + send coordinates re-ask + save_state(state=WAITING_COORDINATES, bot_info={"fallback_count": str(count)}) # new + WAITING_DAMAGE_LEVEL: + send damage-level re-ask + save_state(state=WAITING_DAMAGE_LEVEL, bot_info={"fallback_count": str(count)}) # new + MAPPING_COMPLETED: + send generic fallback message only — unchanged, no save_state +``` + +**New `messages.json` keys** (all 4 languages — ES/EN/PT/FR), final copy: + +| Language | `recovery_question` | `recovery_options` | `flow_cancelled` | +|---|---|---|---| +| ES | "No estamos logrando avanzar. ¿Querés cancelar el mapeo o reiniciarlo desde el principio?" | `{"1": "Cancelar", "2": "Reiniciar"}` | "Mapeo cancelado. Cuando quieras, escribime para empezar de nuevo. 👋" | +| EN | "We're not making progress. Do you want to cancel the mapping or restart it from the beginning?" | `{"1": "Cancel", "2": "Restart"}` | "Mapping cancelled. Whenever you're ready, send me a message to start again. 👋" | +| PT | "Não estamos conseguindo avançar. Você quer cancelar o mapeamento ou reiniciá-lo desde o início?" | `{"1": "Cancelar", "2": "Reiniciar"}` | "Mapeamento cancelado. Quando quiser, me mande uma mensagem para começar de novo. 👋" | +| FR | "On n'avance pas. Tu veux annuler le mapping ou le recommencer depuis le début ?" | `{"1": "Annuler", "2": "Recommencer"}` | "Mapping annulé. Quand tu veux, envoie-moi un message pour recommencer. 👋" | + +## Decisions + +1. **The counter only increments on genuine `on_fallback` invocations** — + unmatched `(state, event)` pairs dispatched from `call()` — not on the + existing invalid-option re-asks inside `on_ask_for_lang` / + `on_damage_level_answered`, which don't go through `on_fallback` today + and still won't. Matches the literal trigger: "falling into + `on_fallback`," not "any wrong answer." +2. **The counter resets to `0` only on genuine state advances**, not on any + matched-handler dispatch. Every handler's existing successful + `save_state` call gets `fallback_count: "0"` added to its `bot_info`. + Discarded: resetting on any matched-handler dispatch regardless of + outcome — would also reset on invalid-option re-asks, which aren't real + progress. +3. **`on_fallback` no longer delegates to `on_ask_for_help` for + `IDLE`/`WAITING_LANG`** — it inlines sending the language prompt and its + own `save_state` call, like the other three per-state branches already + do. Necessary so `on_fallback` stays the sole owner of its own + `fallback_count` write for those two states: delegating would let + `on_ask_for_help`'s own reset-to-`0` stomp the increment `on_fallback` + just made, since Redis `HSET` merge means whichever `save_state` call + happens last for that key wins. +4. **`WAITING_PHOTO`, `WAITING_COORDINATES`, `WAITING_DAMAGE_LEVEL` branches + of `on_fallback` gain a `save_state` call they don't have today** (state + unchanged, only `fallback_count` persisted). Necessary because the + counter must survive across requests — a new `FirstTimeMappingFlow` + instance is rebuilt from Redis on every event — and today those branches + skip `save_state` entirely since nothing needed persisting. +5. **The threshold check runs before the per-state `match` in + `on_fallback`, and short-circuits it entirely once crossed** — regardless + of which state the user was in. This also covers "fallback happens again + while already in `WAITING_RECOVERY_CHOICE`" without a dedicated case: it + just re-shows the same recovery prompt, since the count stays above + `FALLBACK_LIMIT`. +6. **`MAPPING_COMPLETED`'s existing `on_fallback` branch is untouched and + excluded from the counter.** It's dead in practice — + `on_damage_level_answered` calls `delete_state` immediately after + `save_state`, so no later event can ever be dispatched against a live + `MAPPING_COMPLETED` state; the `match` case is purely defensive. +7. **"Restart" reuses `on_ask_for_help` as-is** (send language prompt, + `save_state(state=WAITING_LANG, bot_info={"fallback_count": "0"})`), + without an explicit `delete_state` first. Discarded: clearing the whole + hash before restarting — no observable benefit today, since `lang` / + `point_id` are unconditionally overwritten before a fresh pass through + the flow would ever read them again. +8. **"Cancel" calls `delete_state`** (same call `on_damage_level_answered` + already uses on completion) plus a new farewell message. No other side + effect — nothing in the codebase tracks abandonment today, and adding + that wasn't requested. +9. **Invalid answers to the recovery prompt re-send it without any + `save_state` call** — mirrors the existing invalid-option pattern in + `on_ask_for_lang` / `on_damage_level_answered`, keeping the new handler + consistent with the rest of the flow. + +## Open questions + +None — all decisions above are settled. From b61802c47efa87d0f108393bc90305df601fbd2e Mon Sep 17 00:00:00 2001 From: Joaquin Mansilla Date: Mon, 10 Aug 2026 19:50:13 -0300 Subject: [PATCH 3/3] WIP --- .../82ba73de882d_add_bot_active_to_maps.py | 28 + ...9a2e08_add_bot_conversation_items_table.py | 57 ++ chatmap-api/bot/flow.py | 19 +- .../bot/flows/first_time_mapping/flow.py | 321 ++++---- chatmap-api/consumers/listener.py | 19 +- chatmap-api/conversation_engine/flow.py | 10 +- chatmap-api/conversation_engine/tool.py | 14 +- chatmap-api/db.py | 172 +++- chatmap-api/main.py | 112 ++- chatmap-api/schemas.py | 90 ++- chatmap-api/store/bot_items_store.py | 83 ++ chatmap-api/store/survey_responses_store.py | 23 +- chatmap-api/test/api_tests/__init__.py | 0 .../test/api_tests/test_map_bot_endpoint.py | 300 +++++++ .../bot_tests/test_first_time_mapping_flow.py | 748 +++++++----------- .../test_bot_tool.py | 35 +- .../conversation_engine_tests/test_flow.py | 16 +- .../shoelace/assets/icons/dash-circle.svg | 4 + .../public/shoelace/assets/icons/list-ul.svg | 3 + .../public/shoelace/assets/icons/pencil.svg | 3 + .../shoelace/assets/icons/plus-circle.svg | 4 + .../public/shoelace/assets/icons/robot.svg | 4 + chatmap-ui/src/components/ChatMap/useApi.js | 38 + .../components/EditBotItemDialog/index.jsx | 123 +++ chatmap-ui/src/pages/botSetup/index.jsx | 257 ++++++ chatmap-ui/src/pages/mapList/index.jsx | 5 + chatmap-ui/src/routes.jsx | 2 + chatmap-ui/src/styles/botSetup.css | 174 ++++ chatmap-ui/src/styles/main.css | 4 + chatmap-ui/src/utils/botSetup.js | 126 +++ compose.dev.yml | 3 + docs/how-it-works/bot_setup.md | 377 +++++++++ 32 files changed, 2492 insertions(+), 682 deletions(-) create mode 100644 chatmap-api/alembic/versions/82ba73de882d_add_bot_active_to_maps.py create mode 100644 chatmap-api/alembic/versions/c41d7f9a2e08_add_bot_conversation_items_table.py create mode 100644 chatmap-api/store/bot_items_store.py create mode 100644 chatmap-api/test/api_tests/__init__.py create mode 100644 chatmap-api/test/api_tests/test_map_bot_endpoint.py create mode 100644 chatmap-ui/public/shoelace/assets/icons/dash-circle.svg create mode 100644 chatmap-ui/public/shoelace/assets/icons/list-ul.svg create mode 100644 chatmap-ui/public/shoelace/assets/icons/pencil.svg create mode 100644 chatmap-ui/public/shoelace/assets/icons/plus-circle.svg create mode 100644 chatmap-ui/public/shoelace/assets/icons/robot.svg create mode 100644 chatmap-ui/src/components/EditBotItemDialog/index.jsx create mode 100644 chatmap-ui/src/pages/botSetup/index.jsx create mode 100644 chatmap-ui/src/styles/botSetup.css create mode 100644 chatmap-ui/src/utils/botSetup.js create mode 100644 docs/how-it-works/bot_setup.md diff --git a/chatmap-api/alembic/versions/82ba73de882d_add_bot_active_to_maps.py b/chatmap-api/alembic/versions/82ba73de882d_add_bot_active_to_maps.py new file mode 100644 index 0000000..d120e6e --- /dev/null +++ b/chatmap-api/alembic/versions/82ba73de882d_add_bot_active_to_maps.py @@ -0,0 +1,28 @@ +"""Add bot_active to maps + +Revision ID: 82ba73de882d +Revises: 7209ac6b2b3a +Create Date: 2026-08-10 00:00:00.000000 + +""" +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision: str = '82ba73de882d' +down_revision: Union[str, Sequence[str], None] = '7209ac6b2b3a' +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + """Upgrade schema.""" + op.add_column('maps', sa.Column('bot_active', sa.Boolean(), nullable=False, server_default=sa.false())) + + +def downgrade() -> None: + """Downgrade schema.""" + op.drop_column('maps', 'bot_active') diff --git a/chatmap-api/alembic/versions/c41d7f9a2e08_add_bot_conversation_items_table.py b/chatmap-api/alembic/versions/c41d7f9a2e08_add_bot_conversation_items_table.py new file mode 100644 index 0000000..1ed9c26 --- /dev/null +++ b/chatmap-api/alembic/versions/c41d7f9a2e08_add_bot_conversation_items_table.py @@ -0,0 +1,57 @@ +"""Add bot_conversation_items table + +Revision ID: c41d7f9a2e08 +Revises: 82ba73de882d +Create Date: 2026-08-10 00:00:00.000000 + +""" +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + + +# revision identifiers, used by Alembic. +revision: str = 'c41d7f9a2e08' +down_revision: Union[str, Sequence[str], None] = '82ba73de882d' +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +# create_type=False so create_table does not try to create it a second time: +# the type is created once, explicitly, in upgrade() +bot_item_kind = postgresql.ENUM( + 'start', 'media', 'location', 'single_choice', 'end', 'recovery', 'cancellation', + name='bot_item_kind', + create_type=False, +) + + +def upgrade() -> None: + """Upgrade schema.""" + bot_item_kind.create(op.get_bind(), checkfirst=True) + + op.create_table( + 'bot_conversation_items', + sa.Column('id', sa.String(), nullable=False), + sa.Column('map_id', sa.String(), nullable=False), + sa.Column('kind', bot_item_kind, nullable=False), + sa.Column('position', sa.Integer(), nullable=True), + sa.Column('prompt', sa.String(), nullable=False, server_default=''), + sa.Column('error_message', sa.String(), nullable=True), + sa.Column('options', postgresql.JSONB(astext_type=sa.Text()), nullable=False, + server_default=sa.text("'[]'::jsonb")), + sa.ForeignKeyConstraint(['map_id'], ['maps.id'], ), + sa.PrimaryKeyConstraint('id'), + ) + op.create_index(op.f('ix_bot_conversation_items_id'), 'bot_conversation_items', ['id']) + op.create_index(op.f('ix_bot_conversation_items_map_id'), 'bot_conversation_items', ['map_id']) + + +def downgrade() -> None: + """Downgrade schema.""" + op.drop_index(op.f('ix_bot_conversation_items_map_id'), table_name='bot_conversation_items') + op.drop_index(op.f('ix_bot_conversation_items_id'), table_name='bot_conversation_items') + op.drop_table('bot_conversation_items') + bot_item_kind.drop(op.get_bind(), checkfirst=True) diff --git a/chatmap-api/bot/flow.py b/chatmap-api/bot/flow.py index edd0adf..ff6e176 100644 --- a/chatmap-api/bot/flow.py +++ b/chatmap-api/bot/flow.py @@ -4,6 +4,7 @@ from datetime import datetime from enum import Enum from store.bot_consumed_messages_store import BotConsumedMessagesStore +from store.bot_items_store import BotConversation from store.bot_state_store import BotStateStore from store.message_to_send_store import MessageToSendStore from store.survey_responses_store import SurveyResponsesStore @@ -14,17 +15,6 @@ logger = logging.getLogger(__name__) -class Language(Enum): - ES = "Español" - EN = "English" - PT = "Portugués" - FR = "Francais" - - @classmethod - def default(cls): - return Language.EN - - @dataclass class BotFlowContext: state_key: str @@ -43,14 +33,16 @@ async def fetch_field(self, field: str) -> str | None: class BotFlow(ABC): def __init__(self, state: Enum, - language: Language, + conversation: BotConversation, bot_state_store: BotStateStore, message_to_send_store: MessageToSendStore, bot_consumed_messages_store: BotConsumedMessagesStore, survey_responses_store: SurveyResponsesStore ): self.state = state - self.language = language + # Every message this flow sends comes from here, configured by the + # map owner on the bot setup screen + self.conversation = conversation self.bot_state_store = bot_state_store self.message_to_send_store = message_to_send_store self.bot_consumed_messages_store = bot_consumed_messages_store @@ -61,6 +53,7 @@ def __init__(self, async def create( cls, bot_state_key: str, + conversation: BotConversation, bot_state_store: BotStateStore, message_to_send_store: MessageToSendStore, bot_consumed_messages_store: BotConsumedMessagesStore, diff --git a/chatmap-api/bot/flows/first_time_mapping/flow.py b/chatmap-api/bot/flows/first_time_mapping/flow.py index 1726dc7..4191127 100644 --- a/chatmap-api/bot/flows/first_time_mapping/flow.py +++ b/chatmap-api/bot/flows/first_time_mapping/flow.py @@ -1,43 +1,49 @@ -from bot.flow import BotFlow, BotTransitions, BotFlowContext, not_handler_created, Language +from bot.flow import BotFlow, BotTransitions, BotFlowContext, not_handler_created from conversation_engine.event import EventName from enum import Enum, auto +from db import BotItemKind from results.error import BotStateWithoutPointId from store.bot_consumed_messages_store import BotConsumedMessagesStore +from store.bot_items_store import BotConversation, BotItem from store.bot_state_store import BotStateStore from store.message_to_send_store import MessageToSendStore from store.survey_responses_store import SurveyResponsesStore -from pathlib import Path -import json - import logging logger = logging.getLogger(__name__) -_MESSAGES_PATH = Path(__file__).parent / "messages.json" - FALLBACK_LIMIT = 3 -with open(_MESSAGES_PATH, encoding="utf-8") as f: - translations = json.load(f) +# Options are numbered for the user; the schema caps them at ten for this reason +_KEYCAPS = ["1️⃣", "2️⃣", "3️⃣", "4️⃣", "5️⃣", "6️⃣", "7️⃣", "8️⃣", "9️⃣", "🔟"] -def _build_options_message(question: str, options: dict[str, str]) -> str: - options_text = "\n".join(f"{code}️⃣ {label}" for code, label in options.items()) +def _build_options_message(question: str, options: list[str]) -> str: + options_text = "\n".join( + f"{_KEYCAPS[index] if index < len(_KEYCAPS) else f'{index + 1}.'} {label}" + for index, label in enumerate(options) + ) return f"{question}\n\n{options_text}" -def _lang_options() -> dict[str, str]: - return {str(i): lang.value for i, lang in enumerate(Language, start=1)} +def _selected_option(answer: str, options: list[str]) -> str | None: + """The label the user picked, or None when the reply is not one of the numbers.""" + choice = (answer or "").strip() + + if not choice.isdigit(): + return None + + index = int(choice) + return options[index - 1] if 1 <= index <= len(options) else None class FirstTimeMappingState(Enum): IDLE = auto() - WAITING_LANG = auto() WAITING_PHOTO = auto() WAITING_COORDINATES = auto() - WAITING_DAMAGE_LEVEL = auto() + WAITING_SURVEY_ANSWER = auto() MAPPING_COMPLETED = auto() WAITING_RECOVERY_CHOICE = auto() @@ -49,6 +55,7 @@ class FirstTimeMappingFlow(BotFlow): async def create( cls, bot_state_key: str, + conversation: BotConversation, bot_state_store: BotStateStore, message_to_send_store: MessageToSendStore, bot_consumed_messages_store: BotConsumedMessagesStore, @@ -60,17 +67,12 @@ async def create( raw_state = result.get("state") state = FirstTimeMappingState.__members__.get(raw_state, FirstTimeMappingState.IDLE) if isinstance( raw_state, str) else FirstTimeMappingState.IDLE - - raw_language = result.get("lang") - language = Language.__members__.get(raw_language, Language.default()) if isinstance(raw_language, - str) else Language.default() else: state = FirstTimeMappingState.IDLE - language = Language.default() return cls( state=state, - language=language, + conversation=conversation, bot_state_store=bot_state_store, message_to_send_store=message_to_send_store, bot_consumed_messages_store=bot_consumed_messages_store, @@ -87,83 +89,86 @@ async def call(self, current_event: EventName, context: BotFlowContext) -> None: not_handler_created(self.name, self.state, current_event) await self.on_fallback(ctx=context) - async def on_ask_for_help( - self, - ctx: BotFlowContext, - ) -> None: - logger.info("Handling: on_ask_for_help") - - # The text was consumed to open the conversation -- the user got a - # language menu, not a mapped point -- so it must not reach the map. - # Guarded on text so that anything reaching this handler without text - # is treated as content and stays available to the map. - if ctx.answer: - await self.bot_consumed_messages_store.mark_consumed( - device=ctx.sender, message_id=ctx.message_id, occurred_at=ctx.occurred_at - ) - - logger.info("sending message...") + async def _send(self, ctx: BotFlowContext, message: str) -> None: + await self.message_to_send_store.send_message(sender=ctx.sender, to=ctx.recipient, message=message) - await self.message_to_send_store.send_message( - sender=ctx.sender, to=ctx.recipient, - message=_build_options_message(translations[self.language.name]["ask_for_lang_question"], _lang_options()) - ) + async def _send_start(self, ctx: BotFlowContext, fallback_count: str) -> None: + """ + The greeting and the media prompt go out together: the greeting does + not say what to do, and the media step's own prompt does. + """ + await self._send(ctx, self.conversation.text_of(BotItemKind.START)) + await self._send(ctx, self.conversation.text_of(BotItemKind.MEDIA)) - logger.info("storing new bot event...") await self.bot_state_store.save_state( bot_state_key=ctx.state_key, - state=FirstTimeMappingState.WAITING_LANG, - bot_info={"fallback_count": "0"} + state=FirstTimeMappingState.WAITING_PHOTO, + bot_info={"fallback_count": fallback_count} ) - async def on_ask_for_lang( - self, - ctx: BotFlowContext, - ) -> None: - logger.info("Handling: on_ask_for_lang") + async def _ask(self, ctx: BotFlowContext, question: BotItem) -> None: + await self._send(ctx, _build_options_message(question.prompt, question.options)) - # Answering the bot, not mapping. Marked before validating, so an - # invalid answer is kept out of the map too. - await self.bot_consumed_messages_store.mark_consumed( - device=ctx.sender, message_id=ctx.message_id, occurred_at=ctx.occurred_at - ) + async def _current_question(self, point_id: str) -> BotItem | None: + """ + The first configured question this point has no answer for. There is no + stored cursor: recording an answer is what advances the survey, so a + question the owner deleted simply drops out and a new one shows up as + unanswered. + """ + answered = await self.survey_responses_store.answered_question_ids(point_id=point_id) - selected_language = ctx.answer - displayed_options = {str(i): lang.name for i, lang in enumerate(Language, start=1)} + return next((question for question in self.conversation.questions() if question.id not in answered), None) - if selected_language not in displayed_options: - logger.info(f"Invalid language option received: '{selected_language}', re-asking...") + async def _ask_next_or_finish(self, ctx: BotFlowContext, point_id: str) -> None: + question = await self._current_question(point_id=point_id) - await self.message_to_send_store.send_message( - sender=ctx.sender, to=ctx.recipient, - message=_build_options_message(translations[self.language.name]["ask_for_lang_question"], - _lang_options()) - ) + if question is None: + await self._finish(ctx) return - language_key = displayed_options[selected_language] + await self._ask(ctx, question) + await self.bot_state_store.save_state( + bot_state_key=ctx.state_key, + state=FirstTimeMappingState.WAITING_SURVEY_ANSWER, + bot_info={"point_id": point_id, "fallback_count": "0"} + ) + async def _finish(self, ctx: BotFlowContext) -> None: logger.info("sending message...") - await self.message_to_send_store.send_message( - sender=ctx.sender, to=ctx.recipient, - message=translations[language_key]["ask_for_photo"] - ) + await self._send(ctx, self.conversation.text_of(BotItemKind.END)) logger.info("storing new bot event...") await self.bot_state_store.save_state( bot_state_key=ctx.state_key, - state=FirstTimeMappingState.WAITING_PHOTO, - bot_info={"lang": language_key, "fallback_count": "0"} + state=FirstTimeMappingState.MAPPING_COMPLETED, + bot_info={"fallback_count": "0"} ) + logger.info("bot flow end, deleting state...") + await self.bot_state_store.delete_state(bot_state_key=ctx.state_key) + + async def on_start(self, ctx: BotFlowContext) -> None: + logger.info("Handling: on_start") + + # The text was consumed to open the conversation -- the user got a + # greeting, not a mapped point -- so it must not reach the map. + # Guarded on text so that anything reaching this handler without text + # is treated as content and stays available to the map. + if ctx.answer: + await self.bot_consumed_messages_store.mark_consumed( + device=ctx.sender, message_id=ctx.message_id, occurred_at=ctx.occurred_at + ) + + logger.info("sending message...") + + await self._send_start(ctx, fallback_count="0") + async def on_photo_uploaded(self, ctx: BotFlowContext) -> None: - logger.info("Handling: on_ask_for_help") + logger.info("Handling: on_photo_uploaded") logger.info("sending message...") - await self.message_to_send_store.send_message( - sender=ctx.sender, to=ctx.recipient, - message=translations[self.language.name]["ask_for_coordinate"] - ) + await self._send(ctx, self.conversation.text_of(BotItemKind.LOCATION)) logger.info("storing new bot event...") await self.bot_state_store.save_state( @@ -173,26 +178,16 @@ async def on_photo_uploaded(self, ctx: BotFlowContext) -> None: ) async def on_coordinates_sent(self, ctx: BotFlowContext) -> None: - logger.info("Handling: on_ask_for_help") - logger.info("sending message...") + logger.info("Handling: on_coordinates_sent") - await self.message_to_send_store.send_message( - sender=ctx.sender, to=ctx.recipient, - message=_build_options_message( - translations[self.language.name]["damage_level_question"], - translations[self.language.name]["damage_level_options"] - ) - ) + if not ctx.point_id: + logger.error(f"Coordinates for state: '{ctx.state_key}' arrived without a point id") + raise BotStateWithoutPointId(message_id=ctx.message_id) - logger.info("storing new bot event...") - await self.bot_state_store.save_state( - bot_state_key=ctx.state_key, - state=FirstTimeMappingState.WAITING_DAMAGE_LEVEL, - bot_info={"point_id": ctx.point_id, "fallback_count": "0"} - ) + await self._ask_next_or_finish(ctx, point_id=ctx.point_id) - async def on_damage_level_answered(self, ctx: BotFlowContext) -> None: - logger.info("Handling: on_damage_level_answered") + async def on_survey_answered(self, ctx: BotFlowContext) -> None: + logger.info("Handling: on_survey_answered") # Answering the survey, not mapping. Marked before validating, so an # invalid answer is kept out of the map too. @@ -200,82 +195,66 @@ async def on_damage_level_answered(self, ctx: BotFlowContext) -> None: device=ctx.sender, message_id=ctx.message_id, occurred_at=ctx.occurred_at ) - options = translations[self.language.name]["damage_level_options"] - raw_answer = ctx.answer - - if raw_answer not in options: - logger.info(f"Invalid damage level option received: '{raw_answer}', re-asking...") - - await self.message_to_send_store.send_message( - sender=ctx.sender, to=ctx.recipient, - message=_build_options_message(translations[self.language.name]["damage_level_question"], options) - ) - return - - logger.info("storing survey response...") point_id = await ctx.fetch_field("point_id") if not point_id: logger.error(f"Trying to store a survey response for state: '{ctx.state_key}' does not exist point id") raise BotStateWithoutPointId(message_id=ctx.message_id) - await self.survey_responses_store.add_response( - point_id=point_id, - question=translations[self.language.name]["damage_level_question"], - answer=options[raw_answer] - ) + question = await self._current_question(point_id=point_id) - logger.info("sending message...") - await self.message_to_send_store.send_message( - sender=ctx.sender, to=ctx.recipient, - message=translations[self.language.name]["end_flow"] - ) + if question is None: + logger.info("no questions left to answer, ending the flow...") + await self._finish(ctx) + return - logger.info("storing new bot event...") - await self.bot_state_store.save_state( - bot_state_key=ctx.state_key, - state=FirstTimeMappingState.MAPPING_COMPLETED, - bot_info={"fallback_count": "0"} - ) + answer = _selected_option(ctx.answer, question.options) - logger.info("bot flow end, deleting state...") - await self.bot_state_store.delete_state( - bot_state_key=ctx.state_key, + if answer is None: + logger.info(f"Invalid survey option received: '{ctx.answer}', re-asking...") + + await self._send(ctx, question.error_message or "") + await self._ask(ctx, question) + return + + logger.info("storing survey response...") + await self.survey_responses_store.add_response( + point_id=point_id, + question_id=question.id, + question=question.prompt, + answer=answer ) + await self._ask_next_or_finish(ctx, point_id=point_id) + async def on_recovery_choice_answered(self, ctx: BotFlowContext) -> None: logger.info("Handling: on_recovery_choice_answered") # Answering the bot, not mapping. Marked before validating, so an # invalid answer is kept out of the map too. Restarting delegates to - # on_ask_for_help, which marks the same id again -- zadd is idempotent. + # on_start, which marks the same id again -- zadd is idempotent. await self.bot_consumed_messages_store.mark_consumed( device=ctx.sender, message_id=ctx.message_id, occurred_at=ctx.occurred_at ) - options = translations[self.language.name]["recovery_options"] - raw_answer = ctx.answer + recovery = self.conversation.of_kind(BotItemKind.RECOVERY) + options = recovery.options if recovery else [] + choice = (ctx.answer or "").strip() - if raw_answer not in options: - logger.info(f"Invalid recovery option received: '{raw_answer}', re-asking...") + if choice not in ("1", "2") or len(options) < 2: + logger.info(f"Invalid recovery option received: '{ctx.answer}', re-asking...") - await self.message_to_send_store.send_message( - sender=ctx.sender, to=ctx.recipient, - message=_build_options_message(translations[self.language.name]["recovery_question"], options) - ) + await self._send(ctx, _build_options_message(self.conversation.text_of(BotItemKind.RECOVERY), options)) return - if raw_answer == "1": + if choice == "1": logger.info("user chose to cancel the flow...") - await self.message_to_send_store.send_message( - sender=ctx.sender, to=ctx.recipient, - message=translations[self.language.name]["flow_cancelled"] - ) + await self._send(ctx, self.conversation.text_of(BotItemKind.CANCELLATION)) await self.bot_state_store.delete_state(bot_state_key=ctx.state_key) return logger.info("user chose to restart the flow...") - await self.on_ask_for_help(ctx) + await self.on_start(ctx) async def on_fallback(self, ctx: BotFlowContext) -> None: raw_count = await ctx.fetch_field("fallback_count") @@ -284,11 +263,12 @@ async def on_fallback(self, ctx: BotFlowContext) -> None: if count > FALLBACK_LIMIT: logger.info("fallback limit reached, offering cancel/restart...") - await self.message_to_send_store.send_message( - sender=ctx.sender, to=ctx.recipient, - message=_build_options_message( - translations[self.language.name]["recovery_question"], - translations[self.language.name]["recovery_options"] + recovery = self.conversation.of_kind(BotItemKind.RECOVERY) + await self._send( + ctx, + _build_options_message( + self.conversation.text_of(BotItemKind.RECOVERY), + recovery.options if recovery else [] ) ) await self.bot_state_store.save_state( @@ -298,62 +278,45 @@ async def on_fallback(self, ctx: BotFlowContext) -> None: ) return - await self.message_to_send_store.send_message( - sender=ctx.sender, to=ctx.recipient, - message=translations[self.language.name]["fallback"] - ) - match self.state: - case FirstTimeMappingState.IDLE | FirstTimeMappingState.WAITING_LANG: - await self.message_to_send_store.send_message( - sender=ctx.sender, to=ctx.recipient, - message=_build_options_message(translations[self.language.name]["ask_for_lang_question"], - _lang_options()) - ) - await self.bot_state_store.save_state( - bot_state_key=ctx.state_key, - state=FirstTimeMappingState.WAITING_LANG, - bot_info={"fallback_count": str(count)} - ) + case FirstTimeMappingState.IDLE: + # No greeting has been sent yet, so there is nothing to correct: + # an unexpected first event just starts the conversation + await self._send_start(ctx, fallback_count=str(count)) case FirstTimeMappingState.WAITING_PHOTO: - await self.message_to_send_store.send_message( - sender=ctx.sender, to=ctx.recipient, - message=translations[self.language.name]["ask_for_photo"] - ) + await self._send(ctx, self.conversation.error_of(BotItemKind.MEDIA)) + await self._send(ctx, self.conversation.text_of(BotItemKind.MEDIA)) await self.bot_state_store.save_state( bot_state_key=ctx.state_key, state=FirstTimeMappingState.WAITING_PHOTO, bot_info={"fallback_count": str(count)} ) case FirstTimeMappingState.WAITING_COORDINATES: - await self.message_to_send_store.send_message( - sender=ctx.sender, to=ctx.recipient, - message=translations[self.language.name]["ask_for_coordinate"] - ) + await self._send(ctx, self.conversation.error_of(BotItemKind.LOCATION)) + await self._send(ctx, self.conversation.text_of(BotItemKind.LOCATION)) await self.bot_state_store.save_state( bot_state_key=ctx.state_key, state=FirstTimeMappingState.WAITING_COORDINATES, bot_info={"fallback_count": str(count)} ) - case FirstTimeMappingState.WAITING_DAMAGE_LEVEL: - await self.message_to_send_store.send_message( - sender=ctx.sender, to=ctx.recipient, - message=_build_options_message( - translations[self.language.name]["damage_level_question"], - translations[self.language.name]["damage_level_options"] - ) - ) + case FirstTimeMappingState.WAITING_SURVEY_ANSWER: + point_id = await ctx.fetch_field("point_id") + question = await self._current_question(point_id=point_id) if point_id else None + + if question: + await self._send(ctx, question.error_message or "") + await self._ask(ctx, question) + await self.bot_state_store.save_state( bot_state_key=ctx.state_key, - state=FirstTimeMappingState.WAITING_DAMAGE_LEVEL, + state=FirstTimeMappingState.WAITING_SURVEY_ANSWER, bot_info={"fallback_count": str(count)} ) transitions: BotTransitions = { - (FirstTimeMappingState.IDLE, EventName.USER_SEND_TEXT): on_ask_for_help, - (FirstTimeMappingState.WAITING_LANG, EventName.USER_SEND_TEXT): on_ask_for_lang, + (FirstTimeMappingState.IDLE, EventName.USER_SEND_TEXT): on_start, (FirstTimeMappingState.WAITING_PHOTO, EventName.USER_UPLOAD_PHOTO): on_photo_uploaded, (FirstTimeMappingState.WAITING_COORDINATES, EventName.USER_SEND_COORDINATES): on_coordinates_sent, - (FirstTimeMappingState.WAITING_DAMAGE_LEVEL, EventName.USER_SEND_TEXT): on_damage_level_answered, + (FirstTimeMappingState.WAITING_SURVEY_ANSWER, EventName.USER_SEND_TEXT): on_survey_answered, (FirstTimeMappingState.WAITING_RECOVERY_CHOICE, EventName.USER_SEND_TEXT): on_recovery_choice_answered, } diff --git a/chatmap-api/consumers/listener.py b/chatmap-api/consumers/listener.py index 2f420f6..cd2c6c0 100644 --- a/chatmap-api/consumers/listener.py +++ b/chatmap-api/consumers/listener.py @@ -51,6 +51,23 @@ async def _process(self, message: ReceivedMessage, device: str, flows: Flows) -> else: logger.warning("Currently we are not processing messages from groups") + async def _devices_to_process(self, flows: Flows) -> list[str]: + """ + The devices with a stream in Redis whose live map also has the bot + enabled. Filtering here, rather than when a tool is dispatched, means + a device with the bot off never gets its stream opened and never gets + a consumer group created for it. + """ + devices = await Devices.get_active_devices(self.client) + + try: + bot_devices = await flows.bot_items_store.fetch_bot_active_devices() + except StoreUnavailable: + logger.warning("Could not read which devices have the bot enabled; skipping this cycle") + return [] + + return [device for device in devices if device in bot_devices] + async def process_conversation_for(self, device: str, flows: Flows, semaphore: Semaphore): async with semaphore: try: @@ -82,7 +99,7 @@ async def start(self): logger.debug("Conversations flows is setup!") while True: - devices = await Devices.get_active_devices(self.client) + devices = await self._devices_to_process(flows) try: async with asyncio.TaskGroup() as task_group: diff --git a/chatmap-api/conversation_engine/flow.py b/chatmap-api/conversation_engine/flow.py index 19f67e7..e79afb3 100644 --- a/chatmap-api/conversation_engine/flow.py +++ b/chatmap-api/conversation_engine/flow.py @@ -14,6 +14,7 @@ from store.message_to_send_store import MessageToSendStore from store.received_messages_store import ReceivedMessagesStore, ReceivedMessage from store.survey_responses_store import SurveyResponsesStore +from store.bot_items_store import BotItemsStore Tool = Callable[[Event, ReceivedMessage, str, Conversation], Awaitable[None]] @@ -28,12 +29,14 @@ def __init__( message_to_send_store: MessageToSendStore, bot_consumed_messages_store: BotConsumedMessagesStore, survey_responses_store: SurveyResponsesStore, + bot_items_store: BotItemsStore, tools_by_events: Optional[dict[EventName, Tool]] = None ): self.bot_state_store = bot_state_store self.message_to_send_store = message_to_send_store self.bot_consumed_messages_store = bot_consumed_messages_store self.survey_responses_store = survey_responses_store + self.bot_items_store = bot_items_store self.tools_by_events = tools_by_events if tools_by_events is not None else self.default_tools_by_events() def expected_events(self) -> set[EventName]: @@ -66,7 +69,8 @@ def default_tools_by_events(self) -> dict[EventName, Tool]: bot_state_store=self.bot_state_store, message_to_send_store=self.message_to_send_store, bot_consumed_messages_store=self.bot_consumed_messages_store, - survey_responses_store=self.survey_responses_store + survey_responses_store=self.survey_responses_store, + bot_items_store=self.bot_items_store ) return { @@ -83,6 +87,7 @@ def __init__(self, client: RedisClient): self.received_messages_store = ReceivedMessagesStore(client=client) self.bot_consumed_messages_store = BotConsumedMessagesStore(client=client) self.survey_responses_store = SurveyResponsesStore() + self.bot_items_store = BotItemsStore() def registered_flows(self) -> list[Flow]: return [ @@ -90,7 +95,8 @@ def registered_flows(self) -> list[Flow]: bot_state_store=self.bot_state_store, message_to_send_store=self.message_to_send_store, bot_consumed_messages_store=self.bot_consumed_messages_store, - survey_responses_store=self.survey_responses_store + survey_responses_store=self.survey_responses_store, + bot_items_store=self.bot_items_store ) ] diff --git a/chatmap-api/conversation_engine/tool.py b/chatmap-api/conversation_engine/tool.py index c4182ad..c6f3692 100644 --- a/chatmap-api/conversation_engine/tool.py +++ b/chatmap-api/conversation_engine/tool.py @@ -8,6 +8,7 @@ from conversation_engine.event import Event, EventName from bot.flows.first_time_mapping.flow import FirstTimeMappingFlow from store.bot_consumed_messages_store import BotConsumedMessagesStore +from store.bot_items_store import BotItemsStore from store.bot_state_store import BotStateStore from store.message_to_send_store import MessageToSendStore from store.survey_responses_store import SurveyResponsesStore @@ -37,18 +38,29 @@ def __init__( self, bot_state_store: BotStateStore, message_to_send_store: MessageToSendStore, bot_consumed_messages_store: BotConsumedMessagesStore, - survey_responses_store: SurveyResponsesStore + survey_responses_store: SurveyResponsesStore, + bot_items_store: BotItemsStore ): self.bot_state_store = bot_state_store self.message_to_send_store = message_to_send_store self.bot_consumed_messages_store = bot_consumed_messages_store self.survey_responses_store = survey_responses_store + self.bot_items_store = bot_items_store async def __call__(self, event: Event, message: ReceivedMessage, device: str, conversation: Conversation): + bot_conversation = await self.bot_items_store.fetch_conversation_for(device=device) + + # The listener only hands over devices whose map has the bot enabled, + # but it can be turned off between that read and this dispatch + if not bot_conversation.items: + logger.info(f"Device: '{device}' has no bot messages configured, skipping") + return + bot_state_key = f"bot_state:{FirstTimeMappingFlow.name}:{message.sender}{message.chat}" flow = await FirstTimeMappingFlow.create( bot_state_key=bot_state_key, + conversation=bot_conversation, bot_state_store=self.bot_state_store, message_to_send_store=self.message_to_send_store, bot_consumed_messages_store=self.bot_consumed_messages_store, diff --git a/chatmap-api/db.py b/chatmap-api/db.py index 2da14c3..5b67ced 100644 --- a/chatmap-api/db.py +++ b/chatmap-api/db.py @@ -13,7 +13,7 @@ from enum import Enum from sqlalchemy import ( create_engine, Column, String, select, DateTime, ForeignKey, func, - Enum as SqlEnum, Boolean, + Enum as SqlEnum, Boolean, Integer, delete, ) from sqlalchemy.dialects.postgresql import insert, JSONB from sqlalchemy.pool import NullPool @@ -69,6 +69,7 @@ class Map(Base): created_at = Column(DateTime(timezone=False), default=datetime.now, nullable=False) updated_at = Column(DateTime(timezone=False), default=datetime.now, nullable=False) is_live = Column(Boolean, default=False, nullable=False) + bot_active = Column(Boolean, default=False, nullable=False) centroid = Column(Geometry(geometry_type="POINT", srid=4326), nullable=True, default=None) # Relationship to Point model @@ -105,6 +106,138 @@ def get_or_create_live_map(db, user_id: str) -> str: return new_map.id + +# Enum for the kinds of message a map's bot conversation is made of +class BotItemKind(str, Enum): + START = "start" + MEDIA = "media" + LOCATION = "location" + SINGLE_CHOICE = "single_choice" + END = "end" + RECOVERY = "recovery" + CANCELLATION = "cancellation" + + def __repr__(self) -> str: + return f"<{self.value!r}>" + + +# Model representing one configurable message of a map's bot conversation +class BotConversationItem(Base): + __tablename__ = "bot_conversation_items" + id = Column(String, primary_key=True, index=True, default=lambda: str(uuid.uuid4())) + map_id = Column(String, ForeignKey("maps.id"), index=True, nullable=False) + kind = Column( + SqlEnum(BotItemKind, name="bot_item_kind", + values_callable=lambda enum: [member.value for member in enum]), + nullable=False, + ) + # Ordering among SINGLE_CHOICE questions; null for every other kind + position = Column(Integer, nullable=True) + prompt = Column(String, nullable=False, default="") + error_message = Column(String, nullable=True) + options = Column(JSONB, nullable=False, default=list) + + +# Read every conversation item configured for a map +def fetch_bot_items(db: Session, map_id: str) -> list[BotConversationItem]: + """ + Args: + db (Session): SQLAlchemy database session + map_id (str): Unique identifier of the map + + Returns: + list[BotConversationItem]: items of the map, fixed kinds first and + SINGLE_CHOICE questions in their configured order + """ + stmt = ( + select(BotConversationItem) + .where(BotConversationItem.map_id == map_id) + .order_by(BotConversationItem.position.asc().nullsfirst()) + ) + return list(db.execute(stmt).scalars()) + + +# Replace a map's conversation items with the given list, reconciling by id +def replace_bot_items(db: Session, map_id: str, items: list[dict]) -> list[BotConversationItem]: + """ + Items carrying an id that belongs to this map are updated in place, items + without one are created with a server-generated id, and items of the map + missing from the list are deleted. Preserving ids is what keeps a question + the same question after its text is edited: the survey cursor matches on + them. + + Args: + db (Session): SQLAlchemy database session + map_id (str): Unique identifier of the map + items (list[dict]): the whole desired item list + + Returns: + list[BotConversationItem]: the map's items after the write + """ + existing = {item.id: item for item in fetch_bot_items(db, map_id)} + kept = set() + + for item in items: + row = existing.get(item.get("id")) + + if row is None: + row = BotConversationItem(map_id=map_id) + db.add(row) + else: + kept.add(row.id) + + row.kind = item["kind"] + row.position = item.get("position") + row.prompt = item.get("prompt") or "" + row.error_message = item.get("error_message") + row.options = item.get("options") or [] + + stale = [item_id for item_id in existing if item_id not in kept] + if stale: + db.execute(delete(BotConversationItem).where(BotConversationItem.id.in_(stale))) + + db.commit() + return fetch_bot_items(db, map_id) + + +# Devices whose live map has the bot enabled +def fetch_bot_active_devices(db: Session) -> set[str]: + """ + A device id and the owner id of its live map are the same string: the + connector is started with the owner id as its session (`main.py`), and the + conversation engine derives the device from that same `messages:` + stream key. + + Args: + db (Session): SQLAlchemy database session + + Returns: + set[str]: device ids the bot should answer for + """ + stmt = select(Map.owner_id).where(Map.is_live, Map.bot_active) + return set(db.execute(stmt).scalars()) + + +# Read the conversation items the bot should use for a device +def fetch_bot_items_for_device(db: Session, device: str) -> list[BotConversationItem]: + """ + Args: + db (Session): SQLAlchemy database session + device (str): device id, which is the owner id of its live map + + Returns: + list[BotConversationItem]: items of that device's live map, or an + empty list when it has no live map or the bot is disabled + """ + stmt = select(Map.id).where(Map.owner_id == device, Map.is_live, Map.bot_active) + map_id = db.execute(stmt).scalar_one_or_none() + + if not map_id: + return [] + + return fetch_bot_items(db, map_id) + + # Model representing a geographic point in a map class Point(Base): __tablename__ = "points" @@ -158,23 +291,44 @@ class SurveyResponse(Base): answers = Column(JSONB, nullable=False, default=list) +# Read the answers already given for a point +def get_survey_answers(db: Session, point_id: str) -> list[dict]: + """ + Args: + db (Session): SQLAlchemy database session + point_id (str): id shared with the eventual Point row + + Returns: + list[dict]: the recorded {question_id, question, answer} entries, in + the order they were answered; empty when nothing was answered yet + """ + stmt = select(SurveyResponse.answers).where(SurveyResponse.point_id == point_id) + return db.execute(stmt).scalar_one_or_none() or [] + + # Append a question/answer pair to a point's survey responses -def add_survey_response(db: Session, point_id: str, question: str, answer: str): +def add_survey_response(db: Session, point_id: str, question_id: str, question: str, answer: str): """ - Appends a single {question, answer} pair to the survey_responses row for - a point. Creates the row on first answer; subsequent answers for the - same point_id are appended to the existing JSON array rather than - overwriting it. + Appends a single {question_id, question, answer} entry to the + survey_responses row for a point. Creates the row on first answer; + subsequent answers for the same point_id are appended to the existing JSON + array rather than overwriting it. + + The append is also what advances the survey: the next question the bot + asks is the first configured one whose id is not in this array, so + recording an answer and moving forward are a single write. Args: db (Session): SQLAlchemy database session point_id (str): id shared with the eventual Point row (no FK - the Point may not exist yet, or ever, when this is called) - question (str): localized question text - answer (str): localized answer label + question_id (str): id of the BotConversationItem that was asked + question (str): question text as it stood when it was asked + answer (str): chosen option label """ stmt = insert(SurveyResponse).values( - point_id=point_id, answers=[{"question": question, "answer": answer}] + point_id=point_id, + answers=[{"question_id": question_id, "question": question, "answer": answer}], ) stmt = stmt.on_conflict_do_update( index_elements=["point_id"], diff --git a/chatmap-api/main.py b/chatmap-api/main.py index c4549bb..ff504bd 100644 --- a/chatmap-api/main.py +++ b/chatmap-api/main.py @@ -21,15 +21,19 @@ FastAPI, HTTPException, Depends, Request, APIRouter, File, UploadFile, ) from fastapi.responses import StreamingResponse, FileResponse, HTMLResponse -from typing import Dict +from typing import Dict, List from io import BytesIO from fastapi.middleware.cors import CORSMiddleware from apscheduler.schedulers.asyncio import AsyncIOScheduler -from db import Point, get_db_session, get_or_create_live_map, SharePermission, Map +from db import ( + Point, get_db_session, get_or_create_live_map, SharePermission, Map, + BotConversationItem, fetch_bot_items, replace_bot_items, +) from schemas import ( FeatureCollection, SaveMapFeatureCollection, SaveMapResult, UpdateMap, SaveMediaResponse, PointTags, AddPointsFeatureCollection, AddPointsResult, + BotSetup, BotSetupResult, BotItem, ) from sqlalchemy.exc import NoResultFound, MultipleResultsFound from sqlalchemy.orm import Session @@ -211,6 +215,7 @@ def list_maps_result( "updated_at": map_obj.updated_at, "sharing": map_obj.sharing, "is_live": map_obj.is_live, + "bot_active": map_obj.bot_active, "count": count, "centroid": centroid_coords }) @@ -603,6 +608,109 @@ async def status( ) +def bot_setup_result(map_obj: Map, items: List[BotConversationItem]) -> BotSetupResult: + """ + Build the response shared by both bot endpoints. + + Args: + map_obj (Map): The map the configuration belongs to. + items (List[BotConversationItem]): Its stored conversation items. + + Returns: + BotSetupResult: bot_active plus the whole item list. + """ + return BotSetupResult( + bot_active=map_obj.bot_active, + items=[ + BotItem( + id=item.id, + kind=item.kind.value, + position=item.position, + prompt=item.prompt, + error_message=item.error_message, + options=item.options or [], + ) + for item in items + ], + ) + + +# Get the bot configuration of a map +@api_router.get("/map/{map_id}/bot/") +async def get_bot_setup( + map_id: str, + user: CurrentUser, + db: Session = Depends(get_db_session), +) -> BotSetupResult: + """ + Get the bot_active property of the map and the messages its bot is + configured to send. A map that was never configured comes back with an + empty item list - nothing is created on read. + + Args: + map_id (str): Unique identifier of the map. + user (CurrentUser): Authenticated user. + db (Session): Database session. + + Returns: + BotSetupResult: Current bot_active status and conversation items. + """ + map_obj: Map = db.get(Map, map_id) + if map_obj and user and map_obj.owner_id == user.id: + return bot_setup_result(map_obj, fetch_bot_items(db, map_id)) + else: + # User is not owner of the map + raise HTTPException( + status_code=401, + detail="Unauthorized." + ) + + +# Save the bot configuration of a map +@api_router.put("/map/{map_id}/bot/") +async def set_bot_setup( + map_id: str, + bot_data: BotSetup, + user: CurrentUser, + db: Session = Depends(get_db_session), +) -> BotSetupResult: + """ + Save the whole bot configuration in one write: the messages and whether + the bot is enabled. The item list replaces what is stored, reconciled by + id, so editing a message keeps its id and the survey answers already + recorded against it stay attached. + + Whether the bot can be enabled at all is validated by BotSetup itself, so + an incomplete configuration is rejected with a 422 before anything is + written. + + Args: + map_id (str): Unique identifier of the map. + bot_data (BotSetup): Requested bot_active status and item list. + user (CurrentUser): Authenticated user. + db (Session): Database session. + + Returns: + BotSetupResult: Saved bot_active status and conversation items. + """ + map_obj: Map = db.get(Map, map_id) + if map_obj and user and map_obj.owner_id == user.id: + items = replace_bot_items( + db=db, + map_id=map_id, + items=[item.model_dump() for item in bot_data.items], + ) + map_obj.bot_active = bot_data.bot_active + db.commit() + return bot_setup_result(map_obj, items) + else: + # User is not owner of the map + raise HTTPException( + status_code=401, + detail="Unauthorized." + ) + + # Update map @api_router.put("/map/{map_id}") async def status( diff --git a/chatmap-api/schemas.py b/chatmap-api/schemas.py index 6b79300..6595f7c 100644 --- a/chatmap-api/schemas.py +++ b/chatmap-api/schemas.py @@ -1,7 +1,7 @@ from typing import List, Literal, Tuple from datetime import datetime -from pydantic import BaseModel +from pydantic import BaseModel, model_validator class FeatureGeometry(BaseModel): @@ -81,6 +81,94 @@ class UpdateMap(BaseModel): description: str | None = None +BotItemKind = Literal["start", "media", "location", "single_choice", "end", "recovery", "cancellation"] + +# Options are rendered with keycap emoji, which run out at the tenth +MIN_OPTIONS = 2 +MAX_OPTIONS = 10 + +# Kinds the bot may send but that expect no answer, so they carry no error message +REQUIRED_TEXT_KINDS = ("start", "end", "recovery", "cancellation") +# Kinds the bot asks something with, so they need their "incorrect answer" too +REQUIRED_ANSWERING_KINDS = ("media", "location") + + +def _filled(text: str | None) -> bool: + return bool((text or "").strip()) + + +class BotItem(BaseModel): + """ + One configurable message of a map's bot conversation. An item without an + id is a new one; the id of an existing item is preserved across edits + because the survey cursor matches answers on it. + """ + id: str | None = None + kind: BotItemKind + position: int | None = None + prompt: str = "" + error_message: str | None = None + options: List[str] = [] + + +class BotSetupResult(BaseModel): + """ + A map's whole bot configuration as it is stored. Reports what is there, + without judging it - the rules below govern what may be written, not what + may be read back. + """ + bot_active: bool = False + items: List[BotItem] = [] + + +class BotSetup(BotSetupResult): + """ + An incoming bot configuration. The bot cannot be enabled while a message + it needs is missing, and a half-written question is rejected either way. + """ + + @model_validator(mode="after") + def check_items(self): + singles = [item for item in self.items if item.kind == "single_choice"] + fixed = {} + + for item in self.items: + if item.kind == "single_choice": + continue + if item.kind in fixed: + raise ValueError(f"'{item.kind}' can only be configured once") + fixed[item.kind] = item + + # A half-written question is not something the bot can ask, so this + # holds whether or not the bot is enabled + for question in singles: + if not _filled(question.prompt): + raise ValueError("every single choice question needs its question text") + if not _filled(question.error_message): + raise ValueError("every single choice question needs an incorrect answer message") + if not MIN_OPTIONS <= len([o for o in question.options if _filled(o)]) <= MAX_OPTIONS: + raise ValueError( + f"a single choice question needs between {MIN_OPTIONS} and {MAX_OPTIONS} options" + ) + + if not self.bot_active: + return self + + for kind in REQUIRED_TEXT_KINDS + REQUIRED_ANSWERING_KINDS: + item = fixed.get(kind) + if item is None or not _filled(item.prompt): + raise ValueError(f"the bot cannot be enabled without a '{kind}' message") + + for kind in REQUIRED_ANSWERING_KINDS: + if not _filled(fixed[kind].error_message): + raise ValueError(f"the bot cannot be enabled without an incorrect answer for '{kind}'") + + if len([o for o in fixed["recovery"].options if _filled(o)]) != 2: + raise ValueError("the recovery message needs its two options") + + return self + + class AddPointsFeatureCollection(BaseModel): type: Literal["FeatureCollection"] features: List[SaveMapFeature] diff --git a/chatmap-api/store/bot_items_store.py b/chatmap-api/store/bot_items_store.py new file mode 100644 index 0000000..b597c85 --- /dev/null +++ b/chatmap-api/store/bot_items_store.py @@ -0,0 +1,83 @@ +import logging + +from dataclasses import dataclass, field + +from sqlalchemy.exc import SQLAlchemyError + +from db import BotConversationItem, BotItemKind, fetch_bot_active_devices, fetch_bot_items_for_device, get_db_session +from results.error import StoreUnavailable + +logger = logging.getLogger(__name__) + + +@dataclass +class BotItem: + """One message the bot is configured to send.""" + id: str + kind: BotItemKind + prompt: str + error_message: str | None = None + options: list[str] = field(default_factory=list) + + +@dataclass +class BotConversation: + """ + Everything a map's bot is configured to say. Built once per incoming + message and read by the bot flow instead of a bundled messages file. + """ + items: list[BotItem] = field(default_factory=list) + + def of_kind(self, kind: BotItemKind) -> BotItem | None: + return next((item for item in self.items if item.kind == kind), None) + + def questions(self) -> list[BotItem]: + """The owner's own single choice questions, in their configured order.""" + return [item for item in self.items if item.kind == BotItemKind.SINGLE_CHOICE] + + def text_of(self, kind: BotItemKind) -> str: + item = self.of_kind(kind) + return item.prompt if item else "" + + def error_of(self, kind: BotItemKind) -> str: + item = self.of_kind(kind) + return (item.error_message or "") if item else "" + + +def _to_bot_item(row: BotConversationItem) -> BotItem: + return BotItem( + id=row.id, + kind=row.kind, + prompt=row.prompt or "", + error_message=row.error_message, + options=list(row.options or []), + ) + + +class BotItemsStore: + @classmethod + async def fetch_bot_active_devices(cls) -> set[str]: + """ + The devices whose live map has the bot enabled. Read once per listener + cycle so a device with the bot off is never consumed at all. + """ + db = get_db_session() + try: + return fetch_bot_active_devices(db=db) + except SQLAlchemyError as error: + logger.error(f"Fetch bot active devices failed with: '{error}'") + raise StoreUnavailable + + @classmethod + async def fetch_conversation_for(cls, device: str) -> BotConversation: + """ + The messages configured for the device's live map. Comes back empty + when the device has no live map or its bot is disabled. + """ + db = get_db_session() + try: + rows = fetch_bot_items_for_device(db=db, device=device) + return BotConversation(items=[_to_bot_item(row) for row in rows]) + except SQLAlchemyError as error: + logger.error(f"Fetch bot items for device '{device}' failed with: '{error}'") + raise StoreUnavailable diff --git a/chatmap-api/store/survey_responses_store.py b/chatmap-api/store/survey_responses_store.py index 8b64d91..eae1d92 100644 --- a/chatmap-api/store/survey_responses_store.py +++ b/chatmap-api/store/survey_responses_store.py @@ -2,7 +2,7 @@ from sqlalchemy.exc import SQLAlchemyError -from db import add_survey_response, get_db_session +from db import add_survey_response, get_db_session, get_survey_answers from results.error import StoreUnavailable logger = logging.getLogger(__name__) @@ -10,12 +10,29 @@ class SurveyResponsesStore: @classmethod - async def add_response(cls, point_id: str, question: str, answer: str) -> None: + async def add_response(cls, point_id: str, question_id: str, question: str, answer: str) -> None: db = get_db_session() try: - add_survey_response(db=db, point_id=point_id, question=question, answer=answer) + add_survey_response( + db=db, point_id=point_id, question_id=question_id, question=question, answer=answer + ) logger.debug(f"Survey response saved for point '{point_id}'") except SQLAlchemyError as error: db.rollback() logger.error(f"Save survey response for point '{point_id}' failed with: '{error}'") raise StoreUnavailable + + @classmethod + async def answered_question_ids(cls, point_id: str) -> set[str]: + """ + The ids of the questions this point already has an answer for. This is + the survey's cursor: the next question is the first configured one + missing from here, so recording an answer is what moves it forward. + """ + db = get_db_session() + try: + answers = get_survey_answers(db=db, point_id=point_id) + return {answer["question_id"] for answer in answers if answer.get("question_id")} + except SQLAlchemyError as error: + logger.error(f"Fetch survey responses for point '{point_id}' failed with: '{error}'") + raise StoreUnavailable diff --git a/chatmap-api/test/api_tests/__init__.py b/chatmap-api/test/api_tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/chatmap-api/test/api_tests/test_map_bot_endpoint.py b/chatmap-api/test/api_tests/test_map_bot_endpoint.py new file mode 100644 index 0000000..f957b7e --- /dev/null +++ b/chatmap-api/test/api_tests/test_map_bot_endpoint.py @@ -0,0 +1,300 @@ +from types import SimpleNamespace +from unittest.mock import MagicMock, patch + +import pytest +from fastapi import HTTPException +from pydantic import ValidationError +from sqlalchemy.orm import Session + +from db import BotConversationItem, BotItemKind, Map +from main import get_bot_setup, set_bot_setup +from schemas import BotItem, BotSetup + + +def _map(owner_id="user-1", bot_active=False): + return Map(id="map-1", owner_id=owner_id, bot_active=bot_active) + + +def _db(map_obj): + db = MagicMock(spec=Session) + db.get.return_value = map_obj + return db + + +def _user(user_id="user-1"): + return SimpleNamespace(id=user_id) + + +def _stored(kind, item_id="item-1", position=None, prompt="a message", error_message=None, options=None): + return BotConversationItem( + id=item_id, map_id="map-1", kind=kind, position=position, + prompt=prompt, error_message=error_message, options=options or [], + ) + + +def _complete_items(): + """The smallest item list that lets the bot be enabled.""" + return [ + BotItem(kind="start", prompt="Hi, I'm the ChatMap bot"), + BotItem(kind="media", prompt="Send the content", error_message="That is not a photo"), + BotItem(kind="location", prompt="Now share the location", error_message="That is not a location"), + BotItem(kind="end", prompt="Done, it is on the map"), + BotItem(kind="recovery", prompt="Cancel or restart?", options=["Cancel", "Restart"]), + BotItem(kind="cancellation", prompt="Mapping cancelled"), + ] + + +def _question(prompt="Main material?", error_message="Pick one of the options", options=None): + return BotItem( + kind="single_choice", position=0, prompt=prompt, + error_message=error_message, options=options if options is not None else ["Bricks", "Wood"], + ) + + +# ---- read ---- + +async def test_owner_reads_bot_setup(): + db = _db(_map(bot_active=True)) + stored = [_stored(BotItemKind.START, prompt="Hi")] + + with patch("main.fetch_bot_items", return_value=stored) as fetch: + result = await get_bot_setup(map_id="map-1", user=_user(), db=db) + + assert result.bot_active is True + assert [(item.kind, item.prompt) for item in result.items] == [("start", "Hi")] + fetch.assert_called_once_with(db, "map-1") + + +async def test_an_unconfigured_map_reads_back_empty(): + db = _db(_map(bot_active=False)) + + with patch("main.fetch_bot_items", return_value=[]): + result = await get_bot_setup(map_id="map-1", user=_user(), db=db) + + assert result.bot_active is False + assert result.items == [] + + +async def test_non_owner_cannot_read_bot_setup(): + db = _db(_map(owner_id="another-user", bot_active=True)) + + with pytest.raises(HTTPException) as exc_info: + await get_bot_setup(map_id="map-1", user=_user("user-1"), db=db) + + assert exc_info.value.status_code == 401 + + +async def test_reading_a_missing_map_is_unauthorized(): + db = _db(None) + + with pytest.raises(HTTPException) as exc_info: + await get_bot_setup(map_id="does-not-exist", user=_user(), db=db) + + assert exc_info.value.status_code == 401 + + +async def test_anonymous_user_cannot_read_bot_setup(): + db = _db(_map(bot_active=True)) + + with pytest.raises(HTTPException) as exc_info: + await get_bot_setup(map_id="map-1", user=None, db=db) + + assert exc_info.value.status_code == 401 + + +# ---- write ---- + +async def test_owner_saves_messages_and_enables_the_bot(): + map_obj = _map(bot_active=False) + db = _db(map_obj) + items = _complete_items() + stored = [_stored(BotItemKind.START, prompt="Hi, I'm the ChatMap bot")] + + with patch("main.replace_bot_items", return_value=stored) as replace: + result = await set_bot_setup( + map_id="map-1", + bot_data=BotSetup(bot_active=True, items=items), + user=_user(), + db=db, + ) + + assert result.bot_active is True + assert map_obj.bot_active is True + saved = replace.call_args.kwargs["items"] + assert [item["kind"] for item in saved] == [item.kind for item in items] + db.commit.assert_called_once() + + +async def test_saving_passes_item_ids_through_so_edits_keep_them(): + db = _db(_map()) + items = _complete_items() + [_question()] + items[0].id = "existing-start" + + with patch("main.replace_bot_items", return_value=[]) as replace: + await set_bot_setup( + map_id="map-1", + bot_data=BotSetup(bot_active=False, items=items), + user=_user(), + db=db, + ) + + saved = replace.call_args.kwargs["items"] + assert saved[0]["id"] == "existing-start" + assert saved[-1]["id"] is None + + +async def test_owner_disables_the_bot(): + map_obj = _map(bot_active=True) + db = _db(map_obj) + + with patch("main.replace_bot_items", return_value=[]): + result = await set_bot_setup( + map_id="map-1", + bot_data=BotSetup(bot_active=False, items=[]), + user=_user(), + db=db, + ) + + assert result.bot_active is False + assert map_obj.bot_active is False + + +# ---- unauthorized ---- + +async def test_non_owner_cannot_change_bot_setup(): + map_obj = _map(owner_id="another-user", bot_active=False) + db = _db(map_obj) + + with patch("main.replace_bot_items") as replace: + with pytest.raises(HTTPException) as exc_info: + await set_bot_setup( + map_id="map-1", + bot_data=BotSetup(bot_active=True, items=_complete_items()), + user=_user("user-1"), + db=db, + ) + + assert exc_info.value.status_code == 401 + assert map_obj.bot_active is False + replace.assert_not_called() + db.commit.assert_not_called() + + +async def test_saving_a_missing_map_is_unauthorized(): + db = _db(None) + + with patch("main.replace_bot_items") as replace: + with pytest.raises(HTTPException) as exc_info: + await set_bot_setup( + map_id="does-not-exist", + bot_data=BotSetup(bot_active=False, items=[]), + user=_user(), + db=db, + ) + + assert exc_info.value.status_code == 401 + replace.assert_not_called() + + +async def test_anonymous_user_cannot_change_bot_setup(): + map_obj = _map(bot_active=False) + db = _db(map_obj) + + with patch("main.replace_bot_items") as replace: + with pytest.raises(HTTPException) as exc_info: + await set_bot_setup( + map_id="map-1", + bot_data=BotSetup(bot_active=False, items=[]), + user=None, + db=db, + ) + + assert exc_info.value.status_code == 401 + assert map_obj.bot_active is False + replace.assert_not_called() + + +# ---- validation ---- + +def test_a_complete_configuration_can_enable_the_bot(): + setup = BotSetup(bot_active=True, items=_complete_items()) + + assert setup.bot_active is True + + +def test_the_bot_cannot_be_enabled_with_no_messages(): + with pytest.raises(ValidationError, match="cannot be enabled without a 'start' message"): + BotSetup(bot_active=True, items=[]) + + +@pytest.mark.parametrize("missing", ["start", "media", "location", "end", "recovery", "cancellation"]) +def test_the_bot_cannot_be_enabled_with_a_required_message_missing(missing): + items = [item for item in _complete_items() if item.kind != missing] + + with pytest.raises(ValidationError, match=f"without a '{missing}' message"): + BotSetup(bot_active=True, items=items) + + +@pytest.mark.parametrize("kind", ["media", "location"]) +def test_the_bot_cannot_be_enabled_without_an_incorrect_answer_for_an_answering_step(kind): + items = _complete_items() + for item in items: + if item.kind == kind: + item.error_message = " " + + with pytest.raises(ValidationError, match=f"incorrect answer for '{kind}'"): + BotSetup(bot_active=True, items=items) + + +def test_the_bot_cannot_be_enabled_without_both_recovery_options(): + items = _complete_items() + for item in items: + if item.kind == "recovery": + item.options = ["Cancel"] + + with pytest.raises(ValidationError, match="two options"): + BotSetup(bot_active=True, items=items) + + +def test_an_incomplete_configuration_can_still_be_saved_while_the_bot_is_off(): + setup = BotSetup(bot_active=False, items=[BotItem(kind="start", prompt="Hi")]) + + assert setup.bot_active is False + assert len(setup.items) == 1 + + +def test_the_bot_can_be_enabled_with_no_questions(): + setup = BotSetup(bot_active=True, items=_complete_items()) + + assert [item for item in setup.items if item.kind == "single_choice"] == [] + + +def test_a_question_without_text_is_rejected_even_with_the_bot_off(): + with pytest.raises(ValidationError, match="needs its question text"): + BotSetup(bot_active=False, items=[_question(prompt=" ")]) + + +def test_a_question_without_an_incorrect_answer_is_rejected(): + with pytest.raises(ValidationError, match="needs an incorrect answer message"): + BotSetup(bot_active=False, items=[_question(error_message="")]) + + +@pytest.mark.parametrize("options", [[], ["Only one"], [f"Option {i}" for i in range(11)]]) +def test_a_question_needs_between_two_and_ten_options(options): + with pytest.raises(ValidationError, match="between 2 and 10 options"): + BotSetup(bot_active=False, items=[_question(options=options)]) + + +def test_a_fixed_kind_cannot_be_configured_twice(): + items = _complete_items() + [BotItem(kind="start", prompt="Hi again")] + + with pytest.raises(ValidationError, match="'start' can only be configured once"): + BotSetup(bot_active=False, items=items) + + +def test_several_questions_are_allowed(): + items = _complete_items() + [_question(prompt="First"), _question(prompt="Second")] + + setup = BotSetup(bot_active=True, items=items) + + assert len([item for item in setup.items if item.kind == "single_choice"]) == 2 diff --git a/chatmap-api/test/bot_tests/test_first_time_mapping_flow.py b/chatmap-api/test/bot_tests/test_first_time_mapping_flow.py index 2df8163..8ba44bd 100644 --- a/chatmap-api/test/bot_tests/test_first_time_mapping_flow.py +++ b/chatmap-api/test/bot_tests/test_first_time_mapping_flow.py @@ -1,43 +1,88 @@ from datetime import datetime, timezone -from unittest.mock import AsyncMock, call, patch +from unittest.mock import AsyncMock import pytest -from bot.flow import BotFlowContext, Language -from bot.flows.first_time_mapping import flow as flow_module +from bot.flow import BotFlowContext from bot.flows.first_time_mapping.flow import ( + FALLBACK_LIMIT, FirstTimeMappingFlow, FirstTimeMappingState, - translations, _build_options_message, - _lang_options, + _selected_option, ) from conversation_engine.event import EventName +from db import BotItemKind from results.error import BotStateWithoutPointId from store.bot_consumed_messages_store import BotConsumedMessagesStore +from store.bot_items_store import BotConversation, BotItem from store.bot_state_store import BotStateStore from store.message_to_send_store import MessageToSendStore -from store.survey_responses_store import SurveyResponsesStore +START = "Hi, I'm the ChatMap bot" +MEDIA = "Send the content" +MEDIA_ERROR = "That is not a photo" +LOCATION = "Now share the location" +LOCATION_ERROR = "That is not a location" +END = "Done, it is on the map" +RECOVERY = "Cancel or restart?" +CANCELLED = "Mapping cancelled" OCCURRED_AT = datetime(2026, 8, 9, 21, 7, 41, tzinfo=timezone.utc) -def _make_flow(state, language=Language.ES, bot_state_store=None, message_to_send_store=None, - bot_consumed_messages_store=None, survey_responses_store=None): +class _FakeSurveyStore: + """ + Models the real store: appending an answer is what advances the cursor, + so a question stops being returned once it has been answered. + """ + + def __init__(self, answered=None): + self.answered = set(answered or []) + self.added = [] + + async def answered_question_ids(self, point_id: str) -> set[str]: + return set(self.answered) + + async def add_response(self, point_id: str, question_id: str, question: str, answer: str) -> None: + self.added.append({"point_id": point_id, "question_id": question_id, "question": question, "answer": answer}) + self.answered.add(question_id) + + +def _question(item_id="q-1", prompt="Main material?", options=None, error_message="Pick one of the options"): + return BotItem( + id=item_id, kind=BotItemKind.SINGLE_CHOICE, prompt=prompt, + error_message=error_message, options=options if options is not None else ["Bricks", "Wood"], + ) + + +def _conversation(questions=()): + return BotConversation(items=[ + BotItem(id="start-1", kind=BotItemKind.START, prompt=START), + BotItem(id="media-1", kind=BotItemKind.MEDIA, prompt=MEDIA, error_message=MEDIA_ERROR), + BotItem(id="location-1", kind=BotItemKind.LOCATION, prompt=LOCATION, error_message=LOCATION_ERROR), + BotItem(id="end-1", kind=BotItemKind.END, prompt=END), + BotItem(id="recovery-1", kind=BotItemKind.RECOVERY, prompt=RECOVERY, options=["Cancel", "Restart"]), + BotItem(id="cancellation-1", kind=BotItemKind.CANCELLATION, prompt=CANCELLED), + ] + list(questions)) + + +def _make_flow(state, conversation=None, bot_state_store=None, message_to_send_store=None, + survey_responses_store=None, bot_consumed_messages_store=None): return FirstTimeMappingFlow( state=state, - language=language, + conversation=conversation if conversation is not None else _conversation(), bot_state_store=bot_state_store or AsyncMock(spec=BotStateStore), message_to_send_store=message_to_send_store or AsyncMock(spec=MessageToSendStore), bot_consumed_messages_store=bot_consumed_messages_store or AsyncMock(spec=BotConsumedMessagesStore), - survey_responses_store=survey_responses_store or AsyncMock(spec=SurveyResponsesStore), + survey_responses_store=survey_responses_store or _FakeSurveyStore(), ) -def _ctx(**overrides): +def _ctx(stored=None, **overrides): + stored = stored or {} bot_state_store = AsyncMock(spec=BotStateStore) - bot_state_store.fetch_field.return_value = None + bot_state_store.fetch_field.side_effect = lambda bot_state_key, field: stored.get(field) fields = dict( state_key="key-1", recipient="user-enc-1", sender="device-1", answer="", message_id="msg-1", @@ -47,622 +92,397 @@ def _ctx(**overrides): return BotFlowContext(**fields) -# ---- create() ---- +def _sent(message_to_send_store): + return [call.kwargs["message"] for call in message_to_send_store.send_message.await_args_list] -async def test_create_defaults_to_idle_and_default_language_when_no_state_stored(): - bot_state_store = AsyncMock(spec=BotStateStore) - bot_state_store.fetch_state.return_value = {} - flow = await FirstTimeMappingFlow.create( - bot_state_key="key-1", - bot_state_store=bot_state_store, - message_to_send_store=AsyncMock(spec=MessageToSendStore), - bot_consumed_messages_store=AsyncMock(spec=BotConsumedMessagesStore), - survey_responses_store=AsyncMock(spec=SurveyResponsesStore), - ) +def _saved_state(bot_state_store): + return bot_state_store.save_state.await_args.kwargs - assert flow.state == FirstTimeMappingState.IDLE - assert flow.language == Language.default() - bot_state_store.fetch_state.assert_awaited_once_with(bot_state_key="key-1") +# ---- helpers ---- -@pytest.mark.parametrize("stored_state, stored_lang, expected_state, expected_lang", [ - ("WAITING_LANG", "ES", FirstTimeMappingState.WAITING_LANG, Language.ES), - ("WAITING_PHOTO", "EN", FirstTimeMappingState.WAITING_PHOTO, Language.EN), - ("WAITING_COORDINATES", "PT", FirstTimeMappingState.WAITING_COORDINATES, Language.PT), - ("WAITING_DAMAGE_LEVEL", "PT", FirstTimeMappingState.WAITING_DAMAGE_LEVEL, Language.PT), - ("MAPPING_COMPLETED", "FR", FirstTimeMappingState.MAPPING_COMPLETED, Language.FR), -]) -async def test_create_restores_previously_stored_state_and_language( - stored_state, stored_lang, expected_state, expected_lang): - bot_state_store = AsyncMock(spec=BotStateStore) - bot_state_store.fetch_state.return_value = {"state": stored_state, "lang": stored_lang} +def test_options_are_numbered_for_the_user(): + assert _build_options_message("Material?", ["Bricks", "Wood"]) == "Material?\n\n1️⃣ Bricks\n2️⃣ Wood" - flow = await FirstTimeMappingFlow.create( - bot_state_key="key-1", - bot_state_store=bot_state_store, - message_to_send_store=AsyncMock(spec=MessageToSendStore), - bot_consumed_messages_store=AsyncMock(spec=BotConsumedMessagesStore), - survey_responses_store=AsyncMock(spec=SurveyResponsesStore), - ) - assert flow.state == expected_state - assert flow.language == expected_lang +def test_the_tenth_option_still_gets_a_keycap(): + message = _build_options_message("Pick", [f"Option {i}" for i in range(1, 11)]) + + assert message.endswith("🔟 Option 10") + + +@pytest.mark.parametrize("answer, expected", [ + ("1", "Bricks"), ("2", "Wood"), (" 2 ", "Wood"), + ("0", None), ("3", None), ("", None), ("bricks", None), (None, None), +]) +def test_only_a_valid_option_number_selects_a_label(answer, expected): + assert _selected_option(answer, ["Bricks", "Wood"]) == expected -async def test_create_falls_back_to_idle_for_an_unrecognized_state(): +# ---- create() ---- + +async def test_create_defaults_to_idle_when_no_state_stored(): bot_state_store = AsyncMock(spec=BotStateStore) - bot_state_store.fetch_state.return_value = {"state": "NOT_A_REAL_STATE", "lang": "EN"} + bot_state_store.fetch_state.return_value = {} flow = await FirstTimeMappingFlow.create( bot_state_key="key-1", + conversation=_conversation(), bot_state_store=bot_state_store, message_to_send_store=AsyncMock(spec=MessageToSendStore), bot_consumed_messages_store=AsyncMock(spec=BotConsumedMessagesStore), - survey_responses_store=AsyncMock(spec=SurveyResponsesStore), + survey_responses_store=_FakeSurveyStore(), ) assert flow.state == FirstTimeMappingState.IDLE - assert flow.language == Language.EN + bot_state_store.fetch_state.assert_awaited_once_with(bot_state_key="key-1") -async def test_create_falls_back_to_default_language_for_an_unrecognized_language(): +@pytest.mark.parametrize("stored_state, expected", [ + ("WAITING_PHOTO", FirstTimeMappingState.WAITING_PHOTO), + ("WAITING_COORDINATES", FirstTimeMappingState.WAITING_COORDINATES), + ("WAITING_SURVEY_ANSWER", FirstTimeMappingState.WAITING_SURVEY_ANSWER), + ("WAITING_RECOVERY_CHOICE", FirstTimeMappingState.WAITING_RECOVERY_CHOICE), + ("NOT_A_REAL_STATE", FirstTimeMappingState.IDLE), +]) +async def test_create_restores_the_stored_state(stored_state, expected): bot_state_store = AsyncMock(spec=BotStateStore) - bot_state_store.fetch_state.return_value = {"state": "WAITING_PHOTO", "lang": "DE"} + bot_state_store.fetch_state.return_value = {"state": stored_state} flow = await FirstTimeMappingFlow.create( bot_state_key="key-1", + conversation=_conversation(), bot_state_store=bot_state_store, message_to_send_store=AsyncMock(spec=MessageToSendStore), bot_consumed_messages_store=AsyncMock(spec=BotConsumedMessagesStore), - survey_responses_store=AsyncMock(spec=SurveyResponsesStore), + survey_responses_store=_FakeSurveyStore(), ) - assert flow.state == FirstTimeMappingState.WAITING_PHOTO - assert flow.language == Language.default() - - -# ---- transitions table wiring ---- - -@pytest.mark.parametrize("state, event, handler_name", [ - (FirstTimeMappingState.IDLE, EventName.USER_SEND_TEXT, "on_ask_for_help"), - (FirstTimeMappingState.WAITING_LANG, EventName.USER_SEND_TEXT, "on_ask_for_lang"), - (FirstTimeMappingState.WAITING_PHOTO, EventName.USER_UPLOAD_PHOTO, "on_photo_uploaded"), - (FirstTimeMappingState.WAITING_COORDINATES, EventName.USER_SEND_COORDINATES, "on_coordinates_sent"), - (FirstTimeMappingState.WAITING_DAMAGE_LEVEL, EventName.USER_SEND_TEXT, "on_damage_level_answered"), - (FirstTimeMappingState.WAITING_RECOVERY_CHOICE, EventName.USER_SEND_TEXT, "on_recovery_choice_answered"), -]) -def test_transitions_table_wiring(state, event, handler_name): - assert FirstTimeMappingFlow.transitions[(state, event)] is getattr(FirstTimeMappingFlow, handler_name) + assert flow.state == expected -def test_transitions_table_has_no_unexpected_entries(): - assert len(FirstTimeMappingFlow.transitions) == 6 +async def test_the_configured_messages_are_the_ones_the_flow_uses(): + conversation = _conversation() + flow = _make_flow(FirstTimeMappingState.IDLE, conversation=conversation) + assert flow.conversation is conversation -# ---- call() dispatch ---- -async def test_call_awaits_the_matching_handler_with_self_and_context(): - mock_handler = AsyncMock() - flow = _make_flow(FirstTimeMappingState.IDLE) - flow.transitions = { - (FirstTimeMappingState.IDLE, EventName.USER_SEND_TEXT): mock_handler - } - ctx = _ctx() - - await flow.call(EventName.USER_SEND_TEXT, ctx) - - mock_handler.assert_awaited_once_with(flow, ctx) +# ---- start ---- +async def test_start_greets_and_asks_for_the_media_in_one_turn(): + message_to_send_store = AsyncMock(spec=MessageToSendStore) + bot_state_store = AsyncMock(spec=BotStateStore) + flow = _make_flow(FirstTimeMappingState.IDLE, bot_state_store=bot_state_store, + message_to_send_store=message_to_send_store) -async def test_call_does_not_invoke_a_handler_for_a_different_state_or_event(): - mock_handler = AsyncMock() - flow = _make_flow(FirstTimeMappingState.WAITING_PHOTO) - flow.transitions = { - (FirstTimeMappingState.IDLE, EventName.USER_SEND_TEXT): mock_handler - } - ctx = _ctx() + await flow.call(current_event=EventName.USER_SEND_TEXT, context=_ctx()) - await flow.call(EventName.USER_SEND_TEXT, ctx) + assert _sent(message_to_send_store) == [START, MEDIA] + assert _saved_state(bot_state_store)["state"] == FirstTimeMappingState.WAITING_PHOTO + assert _saved_state(bot_state_store)["bot_info"]["fallback_count"] == "0" - mock_handler.assert_not_awaited() +async def test_an_unexpected_first_event_starts_the_conversation_too(): + message_to_send_store = AsyncMock(spec=MessageToSendStore) + flow = _make_flow(FirstTimeMappingState.IDLE, message_to_send_store=message_to_send_store) -async def test_call_reports_a_missing_handler(): - flow = _make_flow(FirstTimeMappingState.MAPPING_COMPLETED) - ctx = _ctx() + await flow.call(current_event=EventName.USER_UPLOAD_PHOTO, context=_ctx()) - with patch.object(flow_module, "not_handler_created") as mock_not_handler_created: - await flow.call(EventName.USER_SEND_TEXT, ctx) + assert _sent(message_to_send_store) == [START, MEDIA] - mock_not_handler_created.assert_called_once_with( - flow.name, FirstTimeMappingState.MAPPING_COMPLETED, EventName.USER_SEND_TEXT - ) +# ---- media and location ---- -async def test_call_invokes_on_fallback_when_no_handler_matches(): - message_store = AsyncMock(spec=MessageToSendStore) - flow = _make_flow(FirstTimeMappingState.MAPPING_COMPLETED, message_to_send_store=message_store) - ctx = _ctx() +async def test_a_photo_moves_on_to_the_location_question(): + message_to_send_store = AsyncMock(spec=MessageToSendStore) + bot_state_store = AsyncMock(spec=BotStateStore) + flow = _make_flow(FirstTimeMappingState.WAITING_PHOTO, bot_state_store=bot_state_store, + message_to_send_store=message_to_send_store) - with patch.object(flow_module, "not_handler_created"): - await flow.call(EventName.USER_SEND_TEXT, ctx) + await flow.call(current_event=EventName.USER_UPLOAD_PHOTO, context=_ctx()) - message_store.send_message.assert_awaited_once_with( - sender=ctx.sender, to=ctx.recipient, - message=translations[flow.language.name]["fallback"], - ) + assert _sent(message_to_send_store) == [LOCATION] + assert _saved_state(bot_state_store)["state"] == FirstTimeMappingState.WAITING_COORDINATES -# ---- handler behavior ---- +# ---- the survey ---- -@pytest.mark.parametrize("language", list(Language)) -async def test_on_ask_for_help_sends_tutorial_in_current_language_and_moves_to_waiting_lang(language): - message_store = AsyncMock(spec=MessageToSendStore) +async def test_coordinates_ask_the_first_configured_question(): + message_to_send_store = AsyncMock(spec=MessageToSendStore) bot_state_store = AsyncMock(spec=BotStateStore) - flow = _make_flow(FirstTimeMappingState.IDLE, language, bot_state_store, message_store) - ctx = _ctx() - - await flow.on_ask_for_help(ctx) - - message_store.send_message.assert_awaited_once_with( - sender=ctx.sender, to=ctx.recipient, - message=_build_options_message(translations[language.name]["ask_for_lang_question"], _lang_options()), - ) - bot_state_store.save_state.assert_awaited_once_with( - bot_state_key=ctx.state_key, state=FirstTimeMappingState.WAITING_LANG, - bot_info={"fallback_count": "0"}, + flow = _make_flow( + FirstTimeMappingState.WAITING_COORDINATES, + conversation=_conversation([_question("q-1"), _question("q-2", prompt="Second?")]), + bot_state_store=bot_state_store, message_to_send_store=message_to_send_store, ) + await flow.call(current_event=EventName.USER_SEND_COORDINATES, context=_ctx(point_id="point-1")) -@pytest.mark.parametrize("answer, expected_language", [ - ("1", Language.ES), - ("2", Language.EN), - ("3", Language.PT), - ("4", Language.FR), -]) -async def test_on_ask_for_lang_resolves_the_selected_language(answer, expected_language): - message_store = AsyncMock(spec=MessageToSendStore) - bot_state_store = AsyncMock(spec=BotStateStore) - flow = _make_flow(FirstTimeMappingState.WAITING_LANG, Language.ES, bot_state_store, message_store) - ctx = _ctx(answer=answer) - - await flow.on_ask_for_lang(ctx) - - message_store.send_message.assert_awaited_once_with( - sender=ctx.sender, to=ctx.recipient, - message=translations[expected_language.name]["ask_for_photo"], - ) - bot_state_store.save_state.assert_awaited_once_with( - bot_state_key=ctx.state_key, state=FirstTimeMappingState.WAITING_PHOTO, - bot_info={"lang": expected_language.name, "fallback_count": "0"}, - ) + assert _sent(message_to_send_store) == ["Main material?\n\n1️⃣ Bricks\n2️⃣ Wood"] + saved = _saved_state(bot_state_store) + assert saved["state"] == FirstTimeMappingState.WAITING_SURVEY_ANSWER + assert saved["bot_info"]["point_id"] == "point-1" -async def test_on_ask_for_lang_reasks_in_current_language_for_an_invalid_option(): - message_store = AsyncMock(spec=MessageToSendStore) +async def test_coordinates_end_the_flow_when_no_question_is_configured(): + message_to_send_store = AsyncMock(spec=MessageToSendStore) bot_state_store = AsyncMock(spec=BotStateStore) - flow = _make_flow(FirstTimeMappingState.WAITING_LANG, Language.EN, bot_state_store, message_store) - ctx = _ctx(answer="9") + flow = _make_flow(FirstTimeMappingState.WAITING_COORDINATES, bot_state_store=bot_state_store, + message_to_send_store=message_to_send_store) - await flow.on_ask_for_lang(ctx) + await flow.call(current_event=EventName.USER_SEND_COORDINATES, context=_ctx(point_id="point-1")) - message_store.send_message.assert_awaited_once_with( - sender=ctx.sender, to=ctx.recipient, - message=_build_options_message(translations["EN"]["ask_for_lang_question"], _lang_options()), - ) - bot_state_store.save_state.assert_not_awaited() + assert _sent(message_to_send_store) == [END] + assert _saved_state(bot_state_store)["state"] == FirstTimeMappingState.MAPPING_COMPLETED + bot_state_store.delete_state.assert_awaited_once_with(bot_state_key="key-1") -@pytest.mark.parametrize("language", list(Language)) -async def test_on_photo_uploaded_asks_for_coordinates_in_current_language(language): - message_store = AsyncMock(spec=MessageToSendStore) - bot_state_store = AsyncMock(spec=BotStateStore) - flow = _make_flow(FirstTimeMappingState.WAITING_PHOTO, language, bot_state_store, message_store) - ctx = _ctx() - - await flow.on_photo_uploaded(ctx) +async def test_coordinates_without_a_point_id_are_rejected(): + flow = _make_flow(FirstTimeMappingState.WAITING_COORDINATES) - message_store.send_message.assert_awaited_once_with( - sender=ctx.sender, to=ctx.recipient, - message=translations[language.name]["ask_for_coordinate"], - ) - bot_state_store.save_state.assert_awaited_once_with( - bot_state_key=ctx.state_key, state=FirstTimeMappingState.WAITING_COORDINATES, - bot_info={"fallback_count": "0"}, - ) + with pytest.raises(BotStateWithoutPointId): + await flow.call(current_event=EventName.USER_SEND_COORDINATES, context=_ctx(point_id=None)) -@pytest.mark.parametrize("language", list(Language)) -async def test_on_coordinates_sent_asks_for_damage_level_and_stores_point_id(language): - message_store = AsyncMock(spec=MessageToSendStore) - bot_state_store = AsyncMock(spec=BotStateStore) - flow = _make_flow(FirstTimeMappingState.WAITING_COORDINATES, language, bot_state_store, message_store) - ctx = _ctx(point_id="point-99") +async def test_a_valid_answer_is_recorded_and_the_next_question_asked(): + message_to_send_store = AsyncMock(spec=MessageToSendStore) + survey = _FakeSurveyStore() + flow = _make_flow( + FirstTimeMappingState.WAITING_SURVEY_ANSWER, + conversation=_conversation([_question("q-1"), _question("q-2", prompt="Second?", options=["Yes", "No"])]), + message_to_send_store=message_to_send_store, survey_responses_store=survey, + ) - await flow.on_coordinates_sent(ctx) + await flow.call(current_event=EventName.USER_SEND_TEXT, + context=_ctx(stored={"point_id": "point-1"}, answer="2")) - message_store.send_message.assert_awaited_once_with( - sender=ctx.sender, to=ctx.recipient, - message=_build_options_message( - translations[language.name]["damage_level_question"], translations[language.name]["damage_level_options"] - ), - ) - bot_state_store.save_state.assert_awaited_once_with( - bot_state_key=ctx.state_key, state=FirstTimeMappingState.WAITING_DAMAGE_LEVEL, - bot_info={"point_id": "point-99", "fallback_count": "0"}, - ) + assert survey.added == [ + {"point_id": "point-1", "question_id": "q-1", "question": "Main material?", "answer": "Wood"} + ] + assert _sent(message_to_send_store) == ["Second?\n\n1️⃣ Yes\n2️⃣ No"] -@pytest.mark.parametrize("language, answer, expected_label", [ - (Language.ES, "1", "Alto"), - (Language.ES, "2", "Medio"), - (Language.ES, "3", "Bajo"), - (Language.EN, "1", "High"), - (Language.PT, "2", "Médio"), - (Language.FR, "3", "Faible"), -]) -async def test_on_damage_level_answered_persists_response_and_completes_the_flow(language, answer, expected_label): - message_store = AsyncMock(spec=MessageToSendStore) +async def test_answering_the_last_question_ends_the_flow(): + message_to_send_store = AsyncMock(spec=MessageToSendStore) bot_state_store = AsyncMock(spec=BotStateStore) - survey_responses_store = AsyncMock(spec=SurveyResponsesStore) + survey = _FakeSurveyStore() flow = _make_flow( - FirstTimeMappingState.WAITING_DAMAGE_LEVEL, language, bot_state_store, message_store, - survey_responses_store=survey_responses_store, + FirstTimeMappingState.WAITING_SURVEY_ANSWER, conversation=_conversation([_question("q-1")]), + bot_state_store=bot_state_store, message_to_send_store=message_to_send_store, survey_responses_store=survey, ) - ctx_bot_state_store = AsyncMock(spec=BotStateStore) - ctx_bot_state_store.fetch_field.return_value = "point-99" - ctx = _ctx(answer=answer, bot_state_store=ctx_bot_state_store) - await flow.on_damage_level_answered(ctx) + await flow.call(current_event=EventName.USER_SEND_TEXT, + context=_ctx(stored={"point_id": "point-1"}, answer="1")) - ctx_bot_state_store.fetch_field.assert_awaited_once_with(bot_state_key=ctx.state_key, field="point_id") - survey_responses_store.add_response.assert_awaited_once_with( - point_id="point-99", - question=translations[language.name]["damage_level_question"], - answer=expected_label, - ) - message_store.send_message.assert_awaited_once_with( - sender=ctx.sender, to=ctx.recipient, - message=translations[language.name]["end_flow"], - ) - bot_state_store.save_state.assert_awaited_once_with( - bot_state_key=ctx.state_key, state=FirstTimeMappingState.MAPPING_COMPLETED, - bot_info={"fallback_count": "0"}, - ) - bot_state_store.delete_state.assert_awaited_once_with(bot_state_key=ctx.state_key) - assert [call[0] for call in bot_state_store.method_calls] == ["save_state", "delete_state"] + assert survey.added[0]["answer"] == "Bricks" + assert _sent(message_to_send_store) == [END] + assert _saved_state(bot_state_store)["state"] == FirstTimeMappingState.MAPPING_COMPLETED + bot_state_store.delete_state.assert_awaited_once() -async def test_on_damage_level_answered_raises_when_point_id_is_missing_from_state(): - message_store = AsyncMock(spec=MessageToSendStore) - bot_state_store = AsyncMock(spec=BotStateStore) - survey_responses_store = AsyncMock(spec=SurveyResponsesStore) +async def test_an_already_answered_question_is_not_asked_again(): + message_to_send_store = AsyncMock(spec=MessageToSendStore) + survey = _FakeSurveyStore(answered={"q-1"}) flow = _make_flow( - FirstTimeMappingState.WAITING_DAMAGE_LEVEL, Language.EN, bot_state_store, message_store, - survey_responses_store=survey_responses_store, + FirstTimeMappingState.WAITING_SURVEY_ANSWER, + conversation=_conversation([_question("q-1"), _question("q-2", prompt="Second?", options=["Yes", "No"])]), + message_to_send_store=message_to_send_store, survey_responses_store=survey, ) - ctx_bot_state_store = AsyncMock(spec=BotStateStore) - ctx_bot_state_store.fetch_field.return_value = None - ctx = _ctx(answer="1", message_id="reply-msg-1", bot_state_store=ctx_bot_state_store) - with pytest.raises(BotStateWithoutPointId) as exc_info: - await flow.on_damage_level_answered(ctx) + await flow.call(current_event=EventName.USER_SEND_TEXT, + context=_ctx(stored={"point_id": "point-1"}, answer="1")) - assert exc_info.value.message_id == "reply-msg-1" - survey_responses_store.add_response.assert_not_awaited() - bot_state_store.save_state.assert_not_awaited() - bot_state_store.delete_state.assert_not_awaited() + assert survey.added[0]["question_id"] == "q-2" + assert survey.added[0]["answer"] == "Yes" -async def test_on_damage_level_answered_reasks_for_an_invalid_option_without_persisting(): - message_store = AsyncMock(spec=MessageToSendStore) +async def test_an_invalid_answer_re_asks_without_recording_anything(): + message_to_send_store = AsyncMock(spec=MessageToSendStore) bot_state_store = AsyncMock(spec=BotStateStore) - survey_responses_store = AsyncMock(spec=SurveyResponsesStore) + survey = _FakeSurveyStore() flow = _make_flow( - FirstTimeMappingState.WAITING_DAMAGE_LEVEL, Language.EN, bot_state_store, message_store, - survey_responses_store=survey_responses_store, + FirstTimeMappingState.WAITING_SURVEY_ANSWER, conversation=_conversation([_question("q-1")]), + bot_state_store=bot_state_store, message_to_send_store=message_to_send_store, survey_responses_store=survey, ) - ctx = _ctx(answer="9") - await flow.on_damage_level_answered(ctx) + await flow.call(current_event=EventName.USER_SEND_TEXT, + context=_ctx(stored={"point_id": "point-1"}, answer="9")) - message_store.send_message.assert_awaited_once_with( - sender=ctx.sender, to=ctx.recipient, - message=_build_options_message( - translations["EN"]["damage_level_question"], translations["EN"]["damage_level_options"] - ), - ) - survey_responses_store.add_response.assert_not_awaited() + assert survey.added == [] + assert _sent(message_to_send_store) == ["Pick one of the options", "Main material?\n\n1️⃣ Bricks\n2️⃣ Wood"] bot_state_store.save_state.assert_not_awaited() - bot_state_store.delete_state.assert_not_awaited() - ctx.bot_state_store.fetch_field.assert_not_awaited() - - -# ---- on_fallback() ---- - -@pytest.mark.parametrize("state", [FirstTimeMappingState.IDLE, FirstTimeMappingState.WAITING_LANG]) -@pytest.mark.parametrize("language", list(Language)) -async def test_on_fallback_from_idle_or_waiting_lang_reasks_for_language(state, language): - message_store = AsyncMock(spec=MessageToSendStore) - bot_state_store = AsyncMock(spec=BotStateStore) - flow = _make_flow(state, language, bot_state_store, message_store) - ctx = _ctx() - await flow.on_fallback(ctx) - assert message_store.send_message.await_args_list == [ - call(sender=ctx.sender, to=ctx.recipient, message=translations[language.name]["fallback"]), - call(sender=ctx.sender, to=ctx.recipient, - message=_build_options_message(translations[language.name]["ask_for_lang_question"], _lang_options())), - ] - bot_state_store.save_state.assert_awaited_once_with( - bot_state_key=ctx.state_key, state=FirstTimeMappingState.WAITING_LANG, - bot_info={"fallback_count": "1"}, +async def test_deleting_every_question_mid_survey_ends_the_flow(): + message_to_send_store = AsyncMock(spec=MessageToSendStore) + flow = _make_flow( + FirstTimeMappingState.WAITING_SURVEY_ANSWER, conversation=_conversation(), + message_to_send_store=message_to_send_store, ) + await flow.call(current_event=EventName.USER_SEND_TEXT, + context=_ctx(stored={"point_id": "point-1"}, answer="1")) -@pytest.mark.parametrize("language", list(Language)) -async def test_on_fallback_from_waiting_photo_reasks_for_photo(language): - message_store = AsyncMock(spec=MessageToSendStore) - bot_state_store = AsyncMock(spec=BotStateStore) - flow = _make_flow(FirstTimeMappingState.WAITING_PHOTO, language, bot_state_store, message_store) - ctx = _ctx() + assert _sent(message_to_send_store) == [END] - await flow.on_fallback(ctx) - assert message_store.send_message.await_args_list == [ - call(sender=ctx.sender, to=ctx.recipient, message=translations[language.name]["fallback"]), - call(sender=ctx.sender, to=ctx.recipient, message=translations[language.name]["ask_for_photo"]), - ] - bot_state_store.save_state.assert_awaited_once_with( - bot_state_key=ctx.state_key, state=FirstTimeMappingState.WAITING_PHOTO, - bot_info={"fallback_count": "1"}, - ) +async def test_a_survey_answer_without_a_point_id_is_rejected(): + flow = _make_flow(FirstTimeMappingState.WAITING_SURVEY_ANSWER, conversation=_conversation([_question("q-1")])) + with pytest.raises(BotStateWithoutPointId): + await flow.call(current_event=EventName.USER_SEND_TEXT, context=_ctx(answer="1")) -@pytest.mark.parametrize("language", list(Language)) -async def test_on_fallback_from_waiting_coordinates_reasks_for_coordinates(language): - message_store = AsyncMock(spec=MessageToSendStore) - bot_state_store = AsyncMock(spec=BotStateStore) - flow = _make_flow(FirstTimeMappingState.WAITING_COORDINATES, language, bot_state_store, message_store) - ctx = _ctx() - - await flow.on_fallback(ctx) - assert message_store.send_message.await_args_list == [ - call(sender=ctx.sender, to=ctx.recipient, message=translations[language.name]["fallback"]), - call(sender=ctx.sender, to=ctx.recipient, message=translations[language.name]["ask_for_coordinate"]), - ] - bot_state_store.save_state.assert_awaited_once_with( - bot_state_key=ctx.state_key, state=FirstTimeMappingState.WAITING_COORDINATES, - bot_info={"fallback_count": "1"}, - ) +# ---- fallback ---- - -@pytest.mark.parametrize("language", list(Language)) -async def test_on_fallback_from_waiting_damage_level_reasks_for_damage_level(language): - message_store = AsyncMock(spec=MessageToSendStore) +@pytest.mark.parametrize("state, expected_error, expected_prompt", [ + (FirstTimeMappingState.WAITING_PHOTO, MEDIA_ERROR, MEDIA), + (FirstTimeMappingState.WAITING_COORDINATES, LOCATION_ERROR, LOCATION), +]) +async def test_a_wrong_reply_sends_that_step_error_and_asks_again(state, expected_error, expected_prompt): + message_to_send_store = AsyncMock(spec=MessageToSendStore) bot_state_store = AsyncMock(spec=BotStateStore) - flow = _make_flow(FirstTimeMappingState.WAITING_DAMAGE_LEVEL, language, bot_state_store, message_store) - ctx = _ctx() + flow = _make_flow(state, bot_state_store=bot_state_store, message_to_send_store=message_to_send_store) - await flow.on_fallback(ctx) + await flow.call(current_event=EventName.USER_SEND_TEXT, context=_ctx()) - assert message_store.send_message.await_args_list == [ - call(sender=ctx.sender, to=ctx.recipient, message=translations[language.name]["fallback"]), - call(sender=ctx.sender, to=ctx.recipient, message=_build_options_message( - translations[language.name]["damage_level_question"], translations[language.name]["damage_level_options"] - )), - ] - bot_state_store.save_state.assert_awaited_once_with( - bot_state_key=ctx.state_key, state=FirstTimeMappingState.WAITING_DAMAGE_LEVEL, - bot_info={"fallback_count": "1"}, - ) + assert _sent(message_to_send_store) == [expected_error, expected_prompt] + assert _saved_state(bot_state_store)["bot_info"]["fallback_count"] == "1" -@pytest.mark.parametrize("language", list(Language)) -async def test_on_fallback_from_mapping_completed_only_sends_the_fallback_message(language): - message_store = AsyncMock(spec=MessageToSendStore) - bot_state_store = AsyncMock(spec=BotStateStore) - flow = _make_flow(FirstTimeMappingState.MAPPING_COMPLETED, language, bot_state_store, message_store) - ctx = _ctx() - - await flow.on_fallback(ctx) - - message_store.send_message.assert_awaited_once_with( - sender=ctx.sender, to=ctx.recipient, message=translations[language.name]["fallback"], +async def test_a_wrong_reply_during_the_survey_re_asks_the_current_question(): + message_to_send_store = AsyncMock(spec=MessageToSendStore) + flow = _make_flow( + FirstTimeMappingState.WAITING_SURVEY_ANSWER, conversation=_conversation([_question("q-1")]), + message_to_send_store=message_to_send_store, ) - bot_state_store.save_state.assert_not_awaited() + await flow.call(current_event=EventName.USER_UPLOAD_PHOTO, context=_ctx(stored={"point_id": "point-1"})) -@pytest.mark.parametrize("state, expected_message_key", [ - (FirstTimeMappingState.WAITING_PHOTO, "ask_for_photo"), - (FirstTimeMappingState.WAITING_COORDINATES, "ask_for_coordinate"), -]) -async def test_on_fallback_persists_the_incremented_count_on_top_of_a_prior_one(state, expected_message_key): - message_store = AsyncMock(spec=MessageToSendStore) - bot_state_store = AsyncMock(spec=BotStateStore) - flow = _make_flow(state, Language.EN, bot_state_store, message_store) - ctx = _ctx() - ctx.bot_state_store.fetch_field.return_value = "2" - - await flow.on_fallback(ctx) - - ctx.bot_state_store.fetch_field.assert_awaited_once_with(bot_state_key=ctx.state_key, field="fallback_count") - bot_state_store.save_state.assert_awaited_once_with( - bot_state_key=ctx.state_key, state=state, - bot_info={"fallback_count": "3"}, - ) + assert _sent(message_to_send_store) == ["Pick one of the options", "Main material?\n\n1️⃣ Bricks\n2️⃣ Wood"] -@pytest.mark.parametrize("state", list(FirstTimeMappingState)) -async def test_on_fallback_reaching_the_limit_shows_the_recovery_prompt(state): - message_store = AsyncMock(spec=MessageToSendStore) +async def test_the_fallback_count_keeps_growing(): bot_state_store = AsyncMock(spec=BotStateStore) - flow = _make_flow(state, Language.EN, bot_state_store, message_store) - ctx = _ctx() - ctx.bot_state_store.fetch_field.return_value = "3" + flow = _make_flow(FirstTimeMappingState.WAITING_PHOTO, bot_state_store=bot_state_store) - await flow.on_fallback(ctx) + await flow.call(current_event=EventName.USER_SEND_TEXT, context=_ctx(stored={"fallback_count": "2"})) - message_store.send_message.assert_awaited_once_with( - sender=ctx.sender, to=ctx.recipient, - message=_build_options_message( - translations["EN"]["recovery_question"], translations["EN"]["recovery_options"] - ), - ) - bot_state_store.save_state.assert_awaited_once_with( - bot_state_key=ctx.state_key, state=FirstTimeMappingState.WAITING_RECOVERY_CHOICE, - bot_info={"fallback_count": "4"}, - ) + assert _saved_state(bot_state_store)["bot_info"]["fallback_count"] == "3" -async def test_on_fallback_keeps_reshowing_the_recovery_prompt_once_past_the_limit(): - message_store = AsyncMock(spec=MessageToSendStore) +async def test_crossing_the_fallback_limit_offers_cancel_or_restart(): + message_to_send_store = AsyncMock(spec=MessageToSendStore) bot_state_store = AsyncMock(spec=BotStateStore) - flow = _make_flow(FirstTimeMappingState.WAITING_RECOVERY_CHOICE, Language.EN, bot_state_store, message_store) - ctx = _ctx() - ctx.bot_state_store.fetch_field.return_value = "4" + flow = _make_flow(FirstTimeMappingState.WAITING_PHOTO, bot_state_store=bot_state_store, + message_to_send_store=message_to_send_store) - await flow.on_fallback(ctx) + await flow.call(current_event=EventName.USER_SEND_TEXT, + context=_ctx(stored={"fallback_count": str(FALLBACK_LIMIT)})) - message_store.send_message.assert_awaited_once_with( - sender=ctx.sender, to=ctx.recipient, - message=_build_options_message( - translations["EN"]["recovery_question"], translations["EN"]["recovery_options"] - ), - ) - bot_state_store.save_state.assert_awaited_once_with( - bot_state_key=ctx.state_key, state=FirstTimeMappingState.WAITING_RECOVERY_CHOICE, - bot_info={"fallback_count": "5"}, - ) + assert _sent(message_to_send_store) == [f"{RECOVERY}\n\n1️⃣ Cancel\n2️⃣ Restart"] + assert _saved_state(bot_state_store)["state"] == FirstTimeMappingState.WAITING_RECOVERY_CHOICE -# ---- on_recovery_choice_answered() ---- +# ---- recovery ---- -@pytest.mark.parametrize("language", list(Language)) -async def test_on_recovery_choice_answered_cancels_and_deletes_state(language): - message_store = AsyncMock(spec=MessageToSendStore) +async def test_choosing_cancel_says_goodbye_and_drops_the_state(): + message_to_send_store = AsyncMock(spec=MessageToSendStore) bot_state_store = AsyncMock(spec=BotStateStore) - flow = _make_flow(FirstTimeMappingState.WAITING_RECOVERY_CHOICE, language, bot_state_store, message_store) - ctx = _ctx(answer="1") + flow = _make_flow(FirstTimeMappingState.WAITING_RECOVERY_CHOICE, bot_state_store=bot_state_store, + message_to_send_store=message_to_send_store) - await flow.on_recovery_choice_answered(ctx) + await flow.call(current_event=EventName.USER_SEND_TEXT, context=_ctx(answer="1")) - message_store.send_message.assert_awaited_once_with( - sender=ctx.sender, to=ctx.recipient, message=translations[language.name]["flow_cancelled"], - ) - bot_state_store.delete_state.assert_awaited_once_with(bot_state_key=ctx.state_key) + assert _sent(message_to_send_store) == [CANCELLED] + bot_state_store.delete_state.assert_awaited_once_with(bot_state_key="key-1") bot_state_store.save_state.assert_not_awaited() -@pytest.mark.parametrize("language", list(Language)) -async def test_on_recovery_choice_answered_restarts_the_flow(language): - message_store = AsyncMock(spec=MessageToSendStore) +async def test_choosing_restart_greets_again(): + message_to_send_store = AsyncMock(spec=MessageToSendStore) bot_state_store = AsyncMock(spec=BotStateStore) - flow = _make_flow(FirstTimeMappingState.WAITING_RECOVERY_CHOICE, language, bot_state_store, message_store) - ctx = _ctx(answer="2") + flow = _make_flow(FirstTimeMappingState.WAITING_RECOVERY_CHOICE, bot_state_store=bot_state_store, + message_to_send_store=message_to_send_store) - await flow.on_recovery_choice_answered(ctx) + await flow.call(current_event=EventName.USER_SEND_TEXT, context=_ctx(answer="2")) - message_store.send_message.assert_awaited_once_with( - sender=ctx.sender, to=ctx.recipient, - message=_build_options_message(translations[language.name]["ask_for_lang_question"], _lang_options()), - ) - bot_state_store.save_state.assert_awaited_once_with( - bot_state_key=ctx.state_key, state=FirstTimeMappingState.WAITING_LANG, - bot_info={"fallback_count": "0"}, - ) - bot_state_store.delete_state.assert_not_awaited() + assert _sent(message_to_send_store) == [START, MEDIA] + saved = _saved_state(bot_state_store) + assert saved["state"] == FirstTimeMappingState.WAITING_PHOTO + assert saved["bot_info"]["fallback_count"] == "0" -async def test_on_recovery_choice_answered_reasks_for_an_invalid_option(): - message_store = AsyncMock(spec=MessageToSendStore) +async def test_an_invalid_recovery_answer_re_asks_without_saving(): + message_to_send_store = AsyncMock(spec=MessageToSendStore) bot_state_store = AsyncMock(spec=BotStateStore) - flow = _make_flow(FirstTimeMappingState.WAITING_RECOVERY_CHOICE, Language.EN, bot_state_store, message_store) - ctx = _ctx(answer="9") + flow = _make_flow(FirstTimeMappingState.WAITING_RECOVERY_CHOICE, bot_state_store=bot_state_store, + message_to_send_store=message_to_send_store) - await flow.on_recovery_choice_answered(ctx) + await flow.call(current_event=EventName.USER_SEND_TEXT, context=_ctx(answer="7")) - message_store.send_message.assert_awaited_once_with( - sender=ctx.sender, to=ctx.recipient, - message=_build_options_message(translations["EN"]["recovery_question"], translations["EN"]["recovery_options"]), - ) + assert _sent(message_to_send_store) == [f"{RECOVERY}\n\n1️⃣ Cancel\n2️⃣ Restart"] bot_state_store.save_state.assert_not_awaited() bot_state_store.delete_state.assert_not_awaited() - # ---- messages the bot consumes as answers ---- -async def test_answering_the_language_question_keeps_the_message_out_of_the_map(): +async def test_the_text_that_opens_the_conversation_is_kept_out_of_the_map(): bot_consumed_messages_store = AsyncMock(spec=BotConsumedMessagesStore) flow = _make_flow( - FirstTimeMappingState.WAITING_LANG, + FirstTimeMappingState.IDLE, bot_consumed_messages_store=bot_consumed_messages_store, ) - ctx = _ctx(answer="1", message_id="1786309661000-0") + ctx = _ctx(answer="hola", message_id="1786309600000-0") - await flow.on_ask_for_lang(ctx) + await flow.on_start(ctx) bot_consumed_messages_store.mark_consumed.assert_awaited_once_with( - device=ctx.sender, message_id="1786309661000-0", occurred_at=OCCURRED_AT, - ) - - -async def test_an_invalid_language_answer_is_still_kept_out_of_the_map(): - bot_consumed_messages_store = AsyncMock(spec=BotConsumedMessagesStore) - flow = _make_flow( - FirstTimeMappingState.WAITING_LANG, - bot_consumed_messages_store=bot_consumed_messages_store, + device=ctx.sender, message_id="1786309600000-0", occurred_at=OCCURRED_AT, ) - await flow.on_ask_for_lang(_ctx(answer="no soy una opcion")) - - bot_consumed_messages_store.mark_consumed.assert_awaited_once() - -async def test_the_text_that_opens_the_conversation_is_kept_out_of_the_map(): +async def test_a_message_without_text_reaching_on_start_stays_available_to_the_map(): + # whatever arrives here carrying no text is content, not an answer bot_consumed_messages_store = AsyncMock(spec=BotConsumedMessagesStore) flow = _make_flow( FirstTimeMappingState.IDLE, bot_consumed_messages_store=bot_consumed_messages_store, ) - ctx = _ctx(answer="hola", message_id="1786309600000-0") - await flow.on_ask_for_help(ctx) + await flow.on_start(_ctx(answer="")) - bot_consumed_messages_store.mark_consumed.assert_awaited_once_with( - device=ctx.sender, message_id="1786309600000-0", occurred_at=OCCURRED_AT, - ) + bot_consumed_messages_store.mark_consumed.assert_not_awaited() -async def test_answering_the_damage_level_question_keeps_the_message_out_of_the_map(): +async def test_answering_the_survey_keeps_the_message_out_of_the_map(): bot_consumed_messages_store = AsyncMock(spec=BotConsumedMessagesStore) flow = _make_flow( - FirstTimeMappingState.WAITING_DAMAGE_LEVEL, + FirstTimeMappingState.WAITING_SURVEY_ANSWER, + conversation=_conversation([_question()]), bot_consumed_messages_store=bot_consumed_messages_store, ) - ctx_bot_state_store = AsyncMock(spec=BotStateStore) - ctx_bot_state_store.fetch_field.return_value = "point-99" - ctx = _ctx(answer="1", message_id="1786309700000-0", bot_state_store=ctx_bot_state_store) + ctx = _ctx(stored={"point_id": "point-99"}, answer="1", message_id="1786309700000-0") - await flow.on_damage_level_answered(ctx) + await flow.on_survey_answered(ctx) bot_consumed_messages_store.mark_consumed.assert_awaited_once_with( device=ctx.sender, message_id="1786309700000-0", occurred_at=OCCURRED_AT, ) -async def test_an_invalid_damage_level_answer_is_still_kept_out_of_the_map(): +async def test_an_invalid_survey_answer_is_still_kept_out_of_the_map(): bot_consumed_messages_store = AsyncMock(spec=BotConsumedMessagesStore) flow = _make_flow( - FirstTimeMappingState.WAITING_DAMAGE_LEVEL, + FirstTimeMappingState.WAITING_SURVEY_ANSWER, + conversation=_conversation([_question()]), bot_consumed_messages_store=bot_consumed_messages_store, ) - await flow.on_damage_level_answered(_ctx(answer="no soy una opcion")) + await flow.on_survey_answered(_ctx(stored={"point_id": "point-99"}, answer="no soy una opcion")) bot_consumed_messages_store.mark_consumed.assert_awaited_once() @@ -683,28 +503,26 @@ async def test_answering_the_recovery_question_keeps_the_message_out_of_the_map( ) -async def test_a_message_without_text_reaching_on_ask_for_help_stays_available_to_the_map(): - # whatever arrives here carrying no text is content, not an answer +async def test_the_photo_stays_available_to_the_map(): bot_consumed_messages_store = AsyncMock(spec=BotConsumedMessagesStore) flow = _make_flow( - FirstTimeMappingState.IDLE, + FirstTimeMappingState.WAITING_PHOTO, bot_consumed_messages_store=bot_consumed_messages_store, ) - await flow.on_ask_for_help(_ctx(answer="")) + await flow.on_photo_uploaded(_ctx()) bot_consumed_messages_store.mark_consumed.assert_not_awaited() -@pytest.mark.parametrize("state, handler_name", [ - (FirstTimeMappingState.WAITING_PHOTO, "on_photo_uploaded"), - (FirstTimeMappingState.WAITING_COORDINATES, "on_coordinates_sent"), -]) -async def test_content_stays_available_to_the_map(state, handler_name): +async def test_the_location_stays_available_to_the_map(): bot_consumed_messages_store = AsyncMock(spec=BotConsumedMessagesStore) - flow = _make_flow(state, bot_consumed_messages_store=bot_consumed_messages_store) + flow = _make_flow( + FirstTimeMappingState.WAITING_COORDINATES, + bot_consumed_messages_store=bot_consumed_messages_store, + ) - await getattr(flow, handler_name)(_ctx()) + await flow.on_coordinates_sent(_ctx(point_id="point-42")) bot_consumed_messages_store.mark_consumed.assert_not_awaited() diff --git a/chatmap-api/test/conversation_engine_tests/test_bot_tool.py b/chatmap-api/test/conversation_engine_tests/test_bot_tool.py index 6e4492f..80e6d7c 100644 --- a/chatmap-api/test/conversation_engine_tests/test_bot_tool.py +++ b/chatmap-api/test/conversation_engine_tests/test_bot_tool.py @@ -12,7 +12,9 @@ from conversation_engine.event import Event, EventName from conversation_engine.tool import BotTool from settings import CHATMAP_ENC_KEY +from db import BotItemKind from store.bot_consumed_messages_store import BotConsumedMessagesStore +from store.bot_items_store import BotConversation, BotItem, BotItemsStore from store.bot_state_store import BotStateStore from store.message_to_send_store import MessageToSendStore from store.received_messages_store import ReceivedMessage @@ -42,17 +44,27 @@ def _conversation() -> Conversation: return Conversation(key=ConversationKey(sender="sender-1", chat="chat-1")) +def _bot_conversation() -> BotConversation: + return BotConversation(items=[BotItem(id="start-1", kind=BotItemKind.START, prompt="Hi")]) + + def _make_bot_tool( bot_state_store=None, message_to_send_store=None, bot_consumed_messages_store=None, - survey_responses_store=None + survey_responses_store=None, + bot_items_store=None ) -> BotTool: + if bot_items_store is None: + bot_items_store = AsyncMock(spec=BotItemsStore) + bot_items_store.fetch_conversation_for.return_value = _bot_conversation() + return BotTool( bot_state_store=bot_state_store or AsyncMock(spec=BotStateStore), message_to_send_store=message_to_send_store or AsyncMock(spec=MessageToSendStore), bot_consumed_messages_store=bot_consumed_messages_store or AsyncMock(spec=BotConsumedMessagesStore), survey_responses_store=survey_responses_store or AsyncMock(spec=SurveyResponsesStore), + bot_items_store=bot_items_store, ) @@ -66,8 +78,11 @@ async def test_call_decrypts_text_and_delegates_to_the_flow(): message_to_send_store = AsyncMock(spec=MessageToSendStore) bot_consumed_messages_store = AsyncMock(spec=BotConsumedMessagesStore) survey_responses_store = AsyncMock(spec=SurveyResponsesStore) + bot_items_store = AsyncMock(spec=BotItemsStore) + bot_conversation = _bot_conversation() + bot_items_store.fetch_conversation_for.return_value = bot_conversation bot_tool = _make_bot_tool( - bot_state_store, message_to_send_store, bot_consumed_messages_store, survey_responses_store + bot_state_store, message_to_send_store, bot_consumed_messages_store, survey_responses_store, bot_items_store ) fake_flow = AsyncMock() @@ -75,8 +90,10 @@ async def test_call_decrypts_text_and_delegates_to_the_flow(): await bot_tool(event=event, message=message, device="device-1", conversation=_conversation()) expected_key = f"bot_state:{FirstTimeMappingFlow.name}:sender-1chat-1" + bot_items_store.fetch_conversation_for.assert_awaited_once_with(device="device-1") mock_create.assert_awaited_once_with( bot_state_key=expected_key, + conversation=bot_conversation, bot_state_store=bot_state_store, message_to_send_store=message_to_send_store, bot_consumed_messages_store=bot_consumed_messages_store, @@ -140,3 +157,17 @@ async def test_call_passes_the_triggering_event_name_through_unchanged(event_nam await bot_tool(event=event, message=message, device="device-1", conversation=_conversation()) assert fake_flow.call.await_args.kwargs["current_event"] == event_name + + +async def test_call_does_nothing_when_the_device_has_no_configured_messages(): + """The bot can be switched off between the listener's read and this dispatch.""" + bot_items_store = AsyncMock(spec=BotItemsStore) + bot_items_store.fetch_conversation_for.return_value = BotConversation(items=[]) + bot_tool = _make_bot_tool(bot_items_store=bot_items_store) + event = Event(name=EventName.USER_SEND_TEXT, occurred_at=datetime.now(timezone.utc)) + + with patch.object(FirstTimeMappingFlow, "create", AsyncMock()) as mock_create: + await bot_tool(event=event, message=_message(text=_encrypt("hi")), device="device-1", + conversation=_conversation()) + + mock_create.assert_not_awaited() diff --git a/chatmap-api/test/conversation_engine_tests/test_flow.py b/chatmap-api/test/conversation_engine_tests/test_flow.py index b009be4..0e3a087 100644 --- a/chatmap-api/test/conversation_engine_tests/test_flow.py +++ b/chatmap-api/test/conversation_engine_tests/test_flow.py @@ -5,6 +5,7 @@ from conversation_engine.event import Event, EventName from conversation_engine.flow import Flow, Flows, HelpFlow from conversation_engine.tool import BotTool +from store.bot_items_store import BotItemsStore from store.received_messages_store import ReceivedMessage @@ -42,7 +43,7 @@ async def test_check_tool_for_event_invokes_the_registered_tool(): tool = AsyncMock() flow = Flow( bot_state_store=Mock(), message_to_send_store=Mock(), - bot_consumed_messages_store=Mock(), survey_responses_store=Mock(), + bot_consumed_messages_store=Mock(), survey_responses_store=Mock(), bot_items_store=Mock(), tools_by_events={EventName.USER_SEND_TEXT: tool}, ) event = _event(EventName.USER_SEND_TEXT) @@ -58,7 +59,7 @@ async def test_check_tool_for_event_does_nothing_when_no_tool_registered(): tool = AsyncMock() flow = Flow( bot_state_store=Mock(), message_to_send_store=Mock(), - bot_consumed_messages_store=Mock(), survey_responses_store=Mock(), + bot_consumed_messages_store=Mock(), survey_responses_store=Mock(), bot_items_store=Mock(), tools_by_events={EventName.USER_UPLOAD_PHOTO: tool}, ) event = _event(EventName.USER_SEND_TEXT) @@ -73,7 +74,7 @@ async def test_check_tool_for_event_does_nothing_when_no_tool_registered(): def test_expected_events_returns_the_tools_by_events_keys(): flow = Flow( bot_state_store=Mock(), message_to_send_store=Mock(), - bot_consumed_messages_store=Mock(), survey_responses_store=Mock(), + bot_consumed_messages_store=Mock(), survey_responses_store=Mock(), bot_items_store=Mock(), tools_by_events={EventName.USER_SEND_TEXT: AsyncMock(), EventName.USER_SEND_COORDINATES: AsyncMock()}, ) @@ -85,7 +86,7 @@ def test_expected_events_returns_the_tools_by_events_keys(): def test_help_flow_shares_a_single_bot_tool_across_its_events(): help_flow = HelpFlow( bot_state_store=Mock(), message_to_send_store=Mock(), - bot_consumed_messages_store=Mock(), survey_responses_store=Mock(), + bot_consumed_messages_store=Mock(), survey_responses_store=Mock(), bot_items_store=Mock(), ) tools = help_flow.tools_by_events @@ -113,6 +114,13 @@ def test_registered_flows_returns_a_help_flow_with_its_own_stores(): assert help_flow.message_to_send_store is flows.message_to_send_store assert help_flow.bot_consumed_messages_store is flows.bot_consumed_messages_store assert help_flow.survey_responses_store is flows.survey_responses_store + assert help_flow.bot_items_store is flows.bot_items_store + + +def test_flows_owns_the_store_the_listener_filters_devices_with(): + flows = Flows(client=Mock()) + + assert isinstance(flows.bot_items_store, BotItemsStore) async def test_call_tools_for_dispatches_to_the_matching_flow(): diff --git a/chatmap-ui/public/shoelace/assets/icons/dash-circle.svg b/chatmap-ui/public/shoelace/assets/icons/dash-circle.svg new file mode 100644 index 0000000..b4fc4a2 --- /dev/null +++ b/chatmap-ui/public/shoelace/assets/icons/dash-circle.svg @@ -0,0 +1,4 @@ + + + + diff --git a/chatmap-ui/public/shoelace/assets/icons/list-ul.svg b/chatmap-ui/public/shoelace/assets/icons/list-ul.svg new file mode 100644 index 0000000..f1cc202 --- /dev/null +++ b/chatmap-ui/public/shoelace/assets/icons/list-ul.svg @@ -0,0 +1,3 @@ + + + \ No newline at end of file diff --git a/chatmap-ui/public/shoelace/assets/icons/pencil.svg b/chatmap-ui/public/shoelace/assets/icons/pencil.svg new file mode 100644 index 0000000..f5b65c3 --- /dev/null +++ b/chatmap-ui/public/shoelace/assets/icons/pencil.svg @@ -0,0 +1,3 @@ + + + diff --git a/chatmap-ui/public/shoelace/assets/icons/plus-circle.svg b/chatmap-ui/public/shoelace/assets/icons/plus-circle.svg new file mode 100644 index 0000000..7726b67 --- /dev/null +++ b/chatmap-ui/public/shoelace/assets/icons/plus-circle.svg @@ -0,0 +1,4 @@ + + + + diff --git a/chatmap-ui/public/shoelace/assets/icons/robot.svg b/chatmap-ui/public/shoelace/assets/icons/robot.svg new file mode 100644 index 0000000..a224202 --- /dev/null +++ b/chatmap-ui/public/shoelace/assets/icons/robot.svg @@ -0,0 +1,4 @@ + + + + \ No newline at end of file diff --git a/chatmap-ui/src/components/ChatMap/useApi.js b/chatmap-ui/src/components/ChatMap/useApi.js index 98fe567..8619fd2 100644 --- a/chatmap-ui/src/components/ChatMap/useApi.js +++ b/chatmap-ui/src/components/ChatMap/useApi.js @@ -121,6 +121,42 @@ const useApi = (params = {}) => { }); }, []); + // Fetch the bot configuration of a map: whether it is enabled and every + // message it is set up to send + const fetchBotSetup = useCallback(async (id) => { + let setup = null; + await wrapper(async () => { + const response = await fetch(`${config.API_URL}/map/${id}/bot/`, { + method: 'GET', + credentials: 'include', + }); + if (!response.ok) { + throw new Error('Failed to fetch the bot setup'); + } + setup = await response.json(); + }); + return setup; + }, []); + + // Save the whole bot configuration in one request. The API rejects + // enabling the bot while a required message is missing. + const updateBotSetup = useCallback(async (id, setup) => { + let saved = null; + await wrapper(async () => { + const response = await fetch(`${config.API_URL}/map/${id}/bot/`, { + method: 'PUT', + body: JSON.stringify(setup), + headers: {"Content-Type": "application/json"}, + credentials: 'include', + }); + if (!response.ok) { + throw new Error('Failed to save the bot setup'); + } + saved = await response.json(); + }); + return saved; + }, []); + // Update the removed property of a point const removePoint = useCallback(async (id) => { await wrapper(async () => { @@ -165,6 +201,8 @@ const useApi = (params = {}) => { updateMapShare, removePoint, updatePointTags, + fetchBotSetup, + updateBotSetup, mapShare, }; }; diff --git a/chatmap-ui/src/components/EditBotItemDialog/index.jsx b/chatmap-ui/src/components/EditBotItemDialog/index.jsx new file mode 100644 index 0000000..5829b25 --- /dev/null +++ b/chatmap-ui/src/components/EditBotItemDialog/index.jsx @@ -0,0 +1,123 @@ +import { useEffect, useState } from "react"; +import { FormattedMessage, useIntl } from "react-intl"; + +import SlDialog from "@shoelace-style/shoelace/dist/react/dialog/index.js"; +import SlIcon from "@shoelace-style/shoelace/dist/react/icon/index.js"; +import SlInput from "@shoelace-style/shoelace/dist/react/input/index.js"; +import SlTextarea from "@shoelace-style/shoelace/dist/react/textarea/index.js"; +import SlButton from "@shoelace-style/shoelace/dist/react/button/index.js"; + +import { MAX_OPTIONS, MIN_OPTIONS } from "../../utils/botSetup.js"; + +// One shell for every kind of item. Only single choice questions edit a list +// of options; the rest are a single message. +export default function EditBotItemDialog({ + open, setOpen, item, editingError, label, icon, onSave +}) { + const intl = useIntl(); + + const [text, setText] = useState(""); + const [options, setOptions] = useState([]); + + // Reopening on a different item has to start from that item's own values + useEffect(() => { + if (!open || !item) return; + setText((editingError ? item.error_message : item.prompt) || ""); + setOptions(editingError ? [] : [...(item.options || [])]); + }, [open, item, editingError]); + + if (!item) return null; + + const editsOptions = !editingError + && (item.kind === "single_choice" || item.kind === "recovery"); + // The two recovery options mean cancel and restart, in that order, so they + // can be renamed but not added to or removed + const fixedOptions = item.kind === "recovery"; + + function handleOptionChange(index, value) { + setOptions(options.map((option, i) => (i === index ? value : option))); + } + + function handleSave() { + onSave(editingError + ? { error_message: text } + : { prompt: text, ...(editsOptions ? { options } : {}) }); + setOpen(false); + } + + return ( + setOpen(false)}> +

+ +

+ +
+ + {label} +
+ + setText(event.target.value)} + /> + + { editsOptions &&
+ { options.map((option, index) => ( +
+ handleOptionChange(index, event.target.value)} + /> + { !fixedOptions && + setOptions(options.filter((_, i) => i !== index))} + > + + + } +
+ ))} + + { !fixedOptions && options.length < MAX_OPTIONS && + setOptions([...options, ""])} + > + + + + } +
} + +
+ setOpen(false)}> + + + + + +
+ + ); +}; diff --git a/chatmap-ui/src/pages/botSetup/index.jsx b/chatmap-ui/src/pages/botSetup/index.jsx new file mode 100644 index 0000000..0978f58 --- /dev/null +++ b/chatmap-ui/src/pages/botSetup/index.jsx @@ -0,0 +1,257 @@ +import { useNavigate, useParams } from "react-router"; +import { useCallback, useEffect, useMemo, useState } from "react"; +import Header from "../header.jsx"; +import Footer from "../footer.jsx"; +import { FormattedMessage, useIntl } from "react-intl"; + +import SlSwitch from "@shoelace-style/shoelace/dist/react/switch/index.js"; +import SlButton from "@shoelace-style/shoelace/dist/react/button/index.js"; +import SlIcon from "@shoelace-style/shoelace/dist/react/icon/index.js"; + +import useAPI from '../../components/ChatMap/useApi.js'; +import EditBotItemDialog from '../../components/EditBotItemDialog/index.jsx'; +import { + END_STEP, FIXED_STEPS, QUESTION_ICON, RECOVERY_STEPS, + emptyQuestion, itemsFromSetup, itemsToSave, problemsIn, questionsOf, +} from '../../utils/botSetup.js'; +import '../../styles/botSetup.css'; + +export default function BotSetup() { + const { id } = useParams(); + const navigate = useNavigate(); + const intl = useIntl(); + const { fetchBotSetup, updateBotSetup, isLoading, error } = useAPI(); + + const [botActive, setBotActive] = useState(false); + // Starts as the empty form so the steps are on screen from the first paint, + // even if the request never comes back + const [items, setItems] = useState(() => itemsFromSetup(null)); + const [editing, setEditing] = useState(null); + const [showRecovery, setShowRecovery] = useState(false); + const [invalid, setInvalid] = useState(false); + + useEffect(() => { + async function fetchData() { + const setup = await fetchBotSetup(id); + if (setup) { + setBotActive(setup.bot_active); + setItems(itemsFromSetup(setup)); + } + } + fetchData(); + }, [id]); + + const labels = useMemo(() => ({ + start: intl.formatMessage({ id: "app.botSetup.startMessage", defaultMessage: "Start message" }), + media: intl.formatMessage({ id: "app.botSetup.media", defaultMessage: "Media" }), + location: intl.formatMessage({ id: "app.botSetup.location", defaultMessage: "Location" }), + end: intl.formatMessage({ id: "app.botSetup.endMessage", defaultMessage: "End message" }), + single_choice: intl.formatMessage({ id: "app.botSetup.singleChoice", defaultMessage: "Single choice" }), + recovery: intl.formatMessage({ id: "app.botSetup.recovery", defaultMessage: "Recovery question" }), + cancellation: intl.formatMessage({ id: "app.botSetup.cancellation", defaultMessage: "Cancellation message" }), + }), [intl]); + + // Marked rows are the ones that block the save; recomputed as the user edits + // so a fixed row stops being marked without saving again + const problems = useMemo(() => problemsIn(items, botActive), [items, botActive]); + + const questions = questionsOf(items); + const questionKey = (question) => question.id || `new-question-${questions.indexOf(question)}`; + + const updateItem = useCallback((target, changes) => { + setItems((current) => current.map((item) => (item === target ? { ...item, ...changes } : item))); + }, []); + + function openEditor(item, editingError, label, icon) { + setEditing({ item, editingError, label, icon }); + } + + function addQuestion() { + setItems([...items, emptyQuestion(questions.length)]); + } + + function removeQuestion(question) { + setItems(items.filter((item) => item !== question)); + } + + async function handleSave() { + if (problems.size > 0) { + setInvalid(true); + // Those rows are behind a button, so marking them is useless while it is + // collapsed + if (RECOVERY_STEPS.some(({ kind }) => problems.has(kind))) setShowRecovery(true); + return; + } + setInvalid(false); + + const saved = await updateBotSetup(id, { bot_active: botActive, items: itemsToSave(items) }); + if (saved) { + navigate("/maps"); + } + } + + function renderRow(item, definition, label) { + const marked = invalid && problems.has(item.kind === "single_choice" ? questionKey(item) : item.kind); + + return ( +
+
+ + + {item.prompt || label} + +
+ { item.kind === "single_choice" && + + } + +
+
+ + { definition.answers && +
+ + + {item.error_message || intl.formatMessage({ + id: "app.botSetup.incorrectAnswer", defaultMessage: "Incorrect answer", + })} + +
+ +
+
+ } +
+ ); + } + + function rowFor(definition) { + const item = items.find((candidate) => candidate.kind === definition.kind); + return item ? renderRow(item, definition, labels[definition.kind]) : null; + } + + return ( + <> +
+
+ +
+
+
+

+ + +

+
+
+ +
+ {/* Error alerts */} + + + Something went wrong
+ {error} + + + { invalid && +
+ +
+ } + + setBotActive(event.target.checked)} + > + + + +
+ { FIXED_STEPS.map(rowFor) } + + { questions.map((question) => renderRow( + question, { kind: "single_choice", icon: QUESTION_ICON, answers: true }, labels.single_choice + )) } + + + + { rowFor(END_STEP) } +
+ + setShowRecovery(!showRecovery)}> + + + + + { showRecovery && +
+ { RECOVERY_STEPS.map(rowFor) } +
+ } +
+ +
+ +
+ navigate("/maps")}> + + + + + +
+
+ +
+
+ + !open && setEditing(null)} + item={editing?.item} + editingError={editing?.editingError} + label={editing?.label} + icon={editing?.icon} + onSave={(changes) => updateItem(editing.item, changes)} + /> + + ) +} diff --git a/chatmap-ui/src/pages/mapList/index.jsx b/chatmap-ui/src/pages/mapList/index.jsx index 4565008..ac3e3b2 100644 --- a/chatmap-ui/src/pages/mapList/index.jsx +++ b/chatmap-ui/src/pages/mapList/index.jsx @@ -151,6 +151,11 @@ export default function MapList() { handleDeleteRequest(map)}> + { map.is_live && + navigate("/bot-setup/" + map.id)}> + + + } { map.sharing === 'public' && { { config.ENABLE_AUTH && <> } /> + } /> } /> } /> { config.ENABLE_LIVE && item.kind === kind); +} + +export function questionsOf(items) { + return items.filter((item) => item.kind === "single_choice"); +} + +// Which rows are incomplete, as a set of item keys, so the page can mark them. +// `botActive` matters because an empty message only blocks the save when the +// bot is meant to run; a half-written question blocks it either way. +export function problemsIn(items, botActive) { + const problems = new Set(); + + questionsOf(items).forEach((question, index) => { + const key = question.id || `new-question-${index}`; + const options = (question.options || []).filter(filled); + if (!filled(question.prompt) || !filled(question.error_message) + || options.length < MIN_OPTIONS || options.length > MAX_OPTIONS) { + problems.add(key); + } + }); + + if (!botActive) return problems; + + REQUIRED_KINDS.forEach((kind) => { + const item = itemOf(items, kind); + if (!item || !filled(item.prompt)) problems.add(kind); + }); + + KINDS_NEEDING_AN_ERROR.forEach((kind) => { + const item = itemOf(items, kind); + if (item && !filled(item.error_message)) problems.add(kind); + }); + + const recovery = itemOf(items, "recovery"); + if (recovery && (recovery.options || []).filter(filled).length !== 2) { + problems.add("recovery"); + } + + return problems; +} + +// Merge what the API returned into the full row list the screen shows, so a +// map with nothing configured still renders every fixed step as an empty row +export function itemsFromSetup(setup) { + const stored = setup?.items || []; + const fixed = [...FIXED_STEPS, END_STEP, ...RECOVERY_STEPS].map( + ({ kind }) => stored.find((item) => item.kind === kind) || emptyItem(kind) + ); + const questions = questionsOf(stored) + .slice() + .sort((a, b) => (a.position ?? 0) - (b.position ?? 0)); + + return [...fixed, ...questions]; +} + +// Drop rows the owner never filled in, so an untouched setup saves as empty +// instead of storing blank messages +export function itemsToSave(items) { + let questionPosition = 0; + + return items + .filter((item) => filled(item.prompt) + || filled(item.error_message) + || (item.options || []).some(filled)) + .map((item) => ({ + ...item, + position: item.kind === "single_choice" ? questionPosition++ : null, + })); +} diff --git a/compose.dev.yml b/compose.dev.yml index a57cf72..100a4ea 100644 --- a/compose.dev.yml +++ b/compose.dev.yml @@ -70,6 +70,9 @@ services: - CHATMAP_DB=chatmap - CHATMAP_DB_USER=admin - CHATMAP_DB_PASSWORD=0123456789ABCDEF0123456789ABCDEF + volumes: + - ./chatmap-api:/app + - /app/.venv chatmap-go: image: golang:1.25.3 diff --git a/docs/how-it-works/bot_setup.md b/docs/how-it-works/bot_setup.md new file mode 100644 index 0000000..2d39aef --- /dev/null +++ b/docs/how-it-works/bot_setup.md @@ -0,0 +1,377 @@ +# Feature spec — Bot setup: owner-configured conversation + +## Purpose + +Let a map owner write every message the bot sends — and define their own +survey questions — from a screen in the UI, instead of the bot reading a +hardcoded, four-language `messages.json` baked into the image at import time. + +## Scope + +**In scope** + +- A new table, `bot_conversation_items`: one row per configurable message, + scoped to a map. Covers the fixed steps of the flow, the owner's own + single-choice questions, and the recovery/cancellation texts. +- The `GET`/`PUT /map/{map_id}/bot/` endpoints, extended from carrying just + `bot_active` to carrying `bot_active` plus the whole item list. +- The **Bot setup** screen (`chatmap-ui/src/pages/botSetup`): the item list, + the edit modal, adding/removing single-choice questions, and one + page-level save. +- Validation that the bot cannot be enabled while a required message is + missing — enforced on both the frontend and the backend. +- Restricting the conversation engine's device loop to devices whose map has + the bot enabled. +- Reworking `FirstTimeMappingFlow` to build its messages from the map's + configuration: the language step disappears, the hardcoded damage-level + question is replaced by a single survey state driven by the owner's + questions, and `messages.json` stops being read at runtime. + +**Out of scope / deferred** + +- The exact shape of the bot's first turn — whether the start message and the + media prompt go out as one WhatsApp message or two. Decided once the + feature runs end to end. +- Deleting `messages.json` and the `Language` enum. This spec stops reading + them; removing the files is cleanup, see + [Open questions](#open-questions). +- Reordering steps. The fixed steps' order is the shape of the state machine, + not data, and the screen offers no way to reorder single-choice questions + either — they keep the order in which they were added. +- Multi-language bots. Explicitly dropped, see + [Decisions](#decisions) #1. +- Reporting on survey answers. `survey_responses` is written, never read back + by any UI. + +## The device is the map owner + +The bot has to answer one question the conversation engine never had to ask: +**which map does this incoming message belong to?** Today `BotTool` receives a +`device` and knows nothing else. + +The answer is already in the code, spread across four files under two names: + +| | | +|---|---| +| `main.py:98` | `/start-qr?session={user.id}` — the connector writes to `messages:{user.id}` | +| `stream.py:30` | `delete(f"messages:{user}")`, called with `user.id` | +| `stream.py:63-64` | the old consumer scans `messages:*`, strips the prefix, calls the result `user` | +| `conversation_engine/device.py:10-11` | the engine scans the **same** keys, strips the **same** prefix, calls the result `device` | +| `db.py:102` | `Map(owner_id=user_id, is_live=True)` — same `user.id` | + +`device` and `maps.owner_id` are the same string. So the lookup needs no new +column and no notion of users or sessions inside the engine: + +```sql +SELECT owner_id FROM maps WHERE is_live = true AND bot_active = true +``` + +"At most one live map per device" is already assumed by `db.py:96`, which uses +`scalar_one_or_none()`. + +This equality is load-bearing for the whole feature and is currently declared +nowhere. It belongs in the decision record next to +[D-010](live_mode.md#d-010-one-session-one-device) — see +[Open questions](#open-questions). + +## Diagram + +```mermaid +flowchart TD + IDLE --> |"any event
(sends start message + media prompt)"| WAITING_PHOTO + WAITING_PHOTO --> |"USER_UPLOAD_PHOTO
(sends location prompt)"| WAITING_COORDINATES + WAITING_COORDINATES --> |"USER_SEND_COORDINATES
(sends first question)"| WAITING_SURVEY_ANSWER + WAITING_COORDINATES --> |"USER_SEND_COORDINATES
no questions configured"| MAPPING_COMPLETED + WAITING_SURVEY_ANSWER --> |"valid answer,
questions left"| WAITING_SURVEY_ANSWER + WAITING_SURVEY_ANSWER --> |"valid answer,
none left (sends end message)"| MAPPING_COMPLETED + MAPPING_COMPLETED --> |"delete_state (immediate)"| GONE(["key deleted"]) +``` + +`WAITING_LANG` and `WAITING_DAMAGE_LEVEL` are gone. +`WAITING_RECOVERY_CHOICE` and the `fallback_count` mechanism are unchanged +from [first_time_mapping_flow.md](first_time_mapping_flow.md), except that +their texts now come from the map's configuration. + +## Behavior + +### Configuration screen + +| Situation | Trigger | Observable result | +|---|---|---| +| First visit for a map | `GET /map/{id}/bot/` and no items exist | Empty form. Every item row shows its placeholder, nothing is prefilled. The switch is off | +| No single-choice questions yet | Item list contains no `single_choice` | A single dashed row labelled "Single choice" with a ⊕ button, sitting where the questions go | +| Adding a question | ⊕ on the dashed row | The edit modal opens empty. On its "Save changes" a new question row appears above the dashed row, with its own ⊖, pencil and "Incorrect answer" row. The dashed row stays at the bottom of the list | +| Editing any item | Pencil on the row | The "Edit conversation item" modal opens on that item. Its "Save changes" writes to page state only — nothing reaches the server | +| Removing a question | ⊖ on a `single_choice` row | The row and its "Incorrect answer" disappear from page state. Nothing reaches the server until the page is saved | +| Discarding | "Cancel" at the page footer | Every modal edit made since the page loaded is dropped, including the switch | +| Saving | "Save changes" at the page footer | One `PUT` with `bot_active` and the whole item list. Server reconciles by id and returns the saved state | +| Enabling with a required message missing | Switch turned on while a required item is empty | The frontend blocks the save and marks the offending rows. If the request is made anyway, the backend rejects it with `422` and the bot stays disabled | +| Enabling with zero questions | Switch turned on, all required items filled, no `single_choice` | Accepted. Questions are optional | +| A single-choice question left incomplete | Saving with a question missing its text, its error message, or with fewer than two options | Rejected, whether the switch is on or off. A half-written question is not something the bot can send | + +### Bot runtime + +| Situation | Trigger | Observable result | +|---|---|---| +| Device with the bot off | Device appears in `messages:*` but its map has `bot_active = false`, or it has no live map | The device is not in the list the listener iterates. Its stream is never read, no consumer group is created, no event is recorded, the bot never answers | +| Device with the bot on | Map has `is_live` and `bot_active` | Processed exactly as today | +| Conversation starts | Any event while `IDLE` | Start message and media prompt sent; state becomes `WAITING_PHOTO` | +| Wrong reply at a fixed step | Event with no transition from `WAITING_PHOTO` or `WAITING_COORDINATES` | That step's `error_message` sent, then that step's `prompt` re-sent. The old global `fallback` text no longer exists | +| Coordinates received, questions configured | `USER_SEND_COORDINATES` while `WAITING_COORDINATES` | `point_id` saved to bot state as today; first configured question sent; state becomes `WAITING_SURVEY_ANSWER` | +| Coordinates received, no questions configured | Same, but the map has zero `single_choice` items | End message sent, state saved as `MAPPING_COMPLETED` and immediately deleted — the survey state is skipped entirely | +| Valid survey answer, questions left | `USER_SEND_TEXT` while `WAITING_SURVEY_ANSWER`, answer matches an option of the current question | Answer appended to `survey_responses`; next question sent; state stays `WAITING_SURVEY_ANSWER` | +| Valid survey answer, none left | Same, and no configured question is left unanswered | Answer appended; end message sent; `MAPPING_COMPLETED` saved and the state key deleted | +| Invalid survey answer | Same, answer matches no option of the current question | That question's `error_message` sent, then the question re-sent. No write to `survey_responses`, no `save_state` — same pattern as the existing invalid-option handlers | +| Owner adds a question mid-survey | New `single_choice` saved while someone is in `WAITING_SURVEY_ANSWER` | The new question has no answer recorded, so it becomes that person's next question at whatever position it occupies | +| Owner deletes a question mid-survey | The question currently being asked is removed | The cursor recomputes and lands on a different question. The person's next reply is validated against that other question and recorded as its answer, even though they were shown the deleted one. **Accepted — the flow always reads the live configuration** | +| Owner deletes every question mid-survey | Last `single_choice` removed while someone is in `WAITING_SURVEY_ANSWER` | On their next message the cursor finds nothing left: end message sent, flow completed | +| Owner edits a question's text mid-survey | `prompt` changed, id preserved | Nobody who already answered it is asked again. Answers recorded before the edit keep the old wording | +| Owner deletes an option that was already chosen | Option removed from a question that has recorded answers | Recorded answers are untouched and still carry the deleted label | +| Bot enabled while messages are waiting | Switch turned on for a device whose stream holds unread messages | The consumer group is created at `id = 0` (`received_messages_store.py:55`), so the bot answers everything still in the stream — up to `EXPIRING_MIN`, 30 minutes by default. Several people can receive the start message at once, long after they wrote. **Accepted limitation** | +| Bot disabled mid-conversation | Switch turned off while people are partway through the flow | The device leaves the list and the bot simply stops replying. No goodbye message. Messages in the PEL are never acked and the `bot_state:` hashes stay in Redis with no TTL. Re-enabling makes each person resume from where they were, on their next message. **Accepted limitation** | +| `point_id` missing at survey time | `WAITING_SURVEY_ANSWER` reached with no `point_id` in bot state | `BotStateWithoutPointId` raised, message dropped from the PEL — unchanged from today | + +## Contract + +### `bot_conversation_items` + +| column | | +|---|---| +| `id` | PK, uuid. Stable across edits — this is the id the survey cursor matches on | +| `map_id` | FK to `maps.id` | +| `kind` | `start` · `media` · `location` · `single_choice` · `end` · `recovery` · `cancellation` | +| `position` | Integer, ordering among `single_choice` items. Null for every other kind | +| `prompt` | The step's text | +| `error_message` | The "Incorrect answer" text. Null for `start`, `end` and `cancellation` | +| `options` | JSONB, ordered list of labels. Empty except on `single_choice` and `recovery` | + +Every `kind` except `single_choice` appears at most once per map. +`single_choice` appears zero or more times. + +`recovery` carries the recovery question in `prompt` and exactly two labels in +`options`. Their meaning is positional and fixed: the first cancels, the +second restarts. `cancellation` carries the post-cancellation message in +`prompt`. + +### Required for `bot_active = true` + +- `start`, `end`, `recovery`, `cancellation` — non-empty `prompt` + (`recovery` also needs both `options`) +- `media`, `location` — non-empty `prompt` **and** non-empty `error_message` +- `single_choice` — none required. Every one that exists must have a + non-empty `prompt`, a non-empty `error_message`, and between 2 and 10 + options + +The 10-option ceiling is not arbitrary: `_build_options_message` renders +options with keycap emoji, which run out at 🔟. + +### Endpoints + +Both already exist and carry only `bot_active` today. + +**`GET /map/{map_id}/bot/`** → `{"bot_active": bool, "items": [...]}`, owner +only, `401` otherwise. Returns an empty `items` list for a map that was never +configured — no row is created on read. + +**`PUT /map/{map_id}/bot/`** ← the same shape. Reconciliation is by id: + +- item with an `id` that exists → updated in place +- item with no `id` → created, id generated server-side +- item in the database, absent from the payload → deleted + +Validation runs before any write. On failure nothing is persisted and the +response is `422`. Response body on success is the same shape as `GET`. + +`useApi.js`'s `fetchBotActive`/`updateBotActive` become the config-shaped +equivalents; the switch is no longer saved on its own. + +### Survey cursor + +There is no cursor field. Progress is derived: + +``` +answered = { a.question_id for a in survey_responses[point_id].answers } +remaining = [ q for q in configured single_choice, by position, if q.id not in answered ] +current = remaining[0] (or none → the survey is over) +``` + +`survey_responses.answers` entries gain `question_id` alongside the existing +`question` and `answer`. `question` keeps storing the wording as it stood when +the answer was given — recorded answers are never rewritten. + +### Device filtering + +`consumers/listener.py:85` currently iterates every device Redis knows about. +It gains an intersection with the devices whose map has the bot on: + +```python +devices = await Devices.get_active_devices(self.client) +bot_devices = await bot_maps_store.fetch_bot_active_devices() +devices = [d for d in devices if d in bot_devices] +``` + +One query per two-second tick, not one per message. The query lives in a new +store under `store/`, next to `SurveyResponsesStore` — `conversation_engine/` +talks to Redis only. + +## Decisions + +1. **No translations. One text per message, in whatever language the owner + types.** The `Language` enum, the language question and the `WAITING_LANG` + state all disappear; `translations[self.language.name]` goes with them. + Discarded: keeping the four languages and giving each modal a tab per + language — it quadruples the owner's work and requires a control the + design does not have. Also discarded: machine-translating what the owner + writes — a feature of its own, with a dependency the project does not + have. + +2. **The survey is one state plus a derived cursor, not one state per + question.** `transitions` is a class-level `dict` resolved at import time + and `FirstTimeMappingState` is a fixed enum. If each configured question + were its own state, the number of states would become data and the + transition table would have to be built per map at runtime — which + contradicts the conversation engine's + [decision 5](conversation_engine.md#decisions), where sequencing is the + bot's own explicit state machine. One state keeps the table static and the + engine untouched. + +3. **The cursor is derived from `survey_responses`, not stored in Redis.** + The alternative was a `question_index`/`next_question_id` field in the + `BotStateStore` hash, alongside `point_id` and `fallback_count` — which is + exactly what that hash is for, per the engine's + [decision 19](conversation_engine.md#decisions). Deriving wins because the + append to `answers` *is* the advance: one write records the answer and + moves the cursor, so progress and answers cannot drift apart. A separate + Redis cursor can be left stale by a crash between the two writes. The cost + is a Postgres read per message while in the survey state — acceptable, + since the flow already opens a session there to write the answer. + +4. **The cursor matches on question id, not position.** A positional cursor + (`answers[len]`) breaks silently the moment the owner edits the question + list mid-conversation: delete the first question and every recorded answer + shifts onto a different question. Matching ids makes deletions and + insertions harmless — a deleted question is simply not in the configured + set, a new one is simply unanswered. This is what forces `question_id` + into `survey_responses.answers`. + +5. **Editing a question's text keeps its id.** The id identifies a slot in the + survey, not a wording. Discarded: minting a new id on every edit — fixing + a typo would then re-ask the question to everyone mid-survey who had + already answered it. An owner who genuinely wants a different question + deletes and adds, which does mint a new id, and re-asking is then correct. + +6. **Recorded answers are immutable.** Deleting a question, editing its text, + or deleting one of its options never touches rows already in + `survey_responses`. Reporting therefore never depends on the configuration + as it stands today. + +7. **Options are an ordered list of labels; the number is generated.** Today + they are a dict keyed by the number (`{"1": "Alto", ...}`). Letting the + owner own the numbering breaks as soon as they delete the second option or + reorder: gaps, or two options sharing a number. A list renumbers itself. + +8. **Each answering step owns its "Incorrect answer", and it replaces the + global `fallback`.** The bot sends the step's error text and then re-sends + the step's prompt. Discarded: sending only the configured error — an owner + who writes "Wrong answer" and nothing else leaves the user with no idea + what was expected, which is exactly what the current re-ask prevents. + +9. **The recovery and cancellation texts are configurable too**, through their + own button on the screen. Once every other message is owner-written, + leaving three system texts hardcoded in one language reintroduces the + problem decision 1 removes — and it surfaces after four failed turns, when + the user is already lost. Discarded: hardcoding them in English, and + adding a bot-language selector that would apply to those three texts only. + Known limitation: the two recovery options are positional, so an owner who + swaps the labels swaps the behavior with no warning. + +10. **A blank form, not a seeded one.** The bot cannot be enabled until the + required messages exist, so it can never run with empty texts. Discarded: + seeding the configuration from today's `messages.json` on first read, so + the owner would find the form prefilled and the bot would keep working + untouched — rejected in favour of the owner making an explicit choice + about every message the bot sends. + +11. **`bot_active` moves into the form.** It is saved by the same `PUT` as the + messages and validated against them. Leaving it on its own `onSlChange` + write, as it is in the working tree today, would mean enabling the bot, + pressing "Cancel", and finding the bot enabled anyway. + +12. **The whole item list is sent on every save and reconciled by id.** + Discarded: per-item endpoints fired as each modal closes — the bot could + then read a half-edited configuration, and the page's "Cancel" would be + unable to undo what was already written. + +13. **One table with a `kind` column, not a JSONB blob on `maps`.** The + screen is literally a list of conversation items, and the question id is + the survey cursor's key: a real primary key gives stability and + uniqueness for free, where a JSONB document would leave both to + application code. Discarded: a `bot_config` JSONB column on `maps` — + simpler to write atomically, but it puts the identifiers the survey + depends on outside the database's reach. + +14. **Options stay JSONB on the item rather than a child table.** They are + always read and written as a whole, ordered set and are never queried + individually. A child table would add a position column and a cascade for + nothing. + +15. **`position` is meaningful only among `single_choice` items.** The order of + the fixed steps is the shape of the state machine, not configuration. + Giving every row a position would advertise a reordering the screen does + not offer. + +16. **`cancellation` is its own row.** The recovery modal edits both it and + `recovery`, but splitting them keeps every row the same shape instead of + adding a third text column that only one kind would use. + +17. **Devices are filtered in the listener, not inside `BotTool`.** Filtering + at dispatch time would still read the stream, detect the event, append it + to the conversation log and ack it, only to discard the result. Filtering + at `listener.py:85` means a bot-off device's stream is never opened and no + consumer group is ever created for it. Known consequence: the engine's + listener now knows about `bot_active`, a bot-level concept. Today + "devices to process" and "devices with the bot on" are the same set, + because `HelpFlow` is the only registered Flow and `BotTool` its only + Tool. A second, non-bot Flow would be silently switched off by this + filter. + +18. **Step order stays photo → coordinates**, even though the design lists + Location above Media. Nothing forces it either way — `chatmap-py`'s + pairing searches in both directions + ([D-009](live_mode.md#d-009-pairing-match-criteria)) — so the existing + order stands and the design is read as a listing, not a sequence. + +19. **Any event in `IDLE` starts the conversation.** Today `IDLE` + + `USER_SEND_TEXT` asks for the language, and any other event falls through + to `on_fallback`, which also ends up asking for the language. With the + language step gone, both paths collapse into the same thing: send the + start message and the media prompt. `start` therefore needs no + "Incorrect answer", which is what the design shows. + +20. **Turning the bot on answers the backlog, and that is accepted.** The + consumer group is created at `id = 0` and a bot-off device has no group + at all, so enabling the bot makes it reply to everything still in the + stream. Changing `received_messages_store.py:55` to `$` is not the fix: + a device's stream is created by its user's first message and the group is + created up to two seconds later, so `$` would drop that first message and + the bot would never start. Discarded for now: a `maps.bot_activated_at` + column, with messages older than the mark acked without being answered. + +## Open questions + +- **The device/owner equality is undeclared.** That `maps.owner_id` and the + engine's `device` are the same string is what makes this whole feature + possible, and it lives implicitly across `main.py`, `stream.py`, + `conversation_engine/device.py` and `db.py`, under two different names. It + is an existing rule that was never written down rather than a new one to + invent, so it does not block this spec — but it belongs in + [live_mode.md](live_mode.md) next to + [D-010](live_mode.md#d-010-one-session-one-device), and it constrains + D-010: one device, one live map, one owner. +- **The bot's first turn.** Whether the start message and the media prompt go + out as one WhatsApp message or two. Deferred by decision until the feature + runs end to end. +- **What becomes of `messages.json` and `Language`.** Nothing reads them once + this lands. Deleting them is cleanup, but the four-language content is the + only translated copy that exists, so it is worth deciding whether it is + dropped or kept somewhere.