Simplify reply context extraction, fix slash commands broken by reply injection, attach reply media regardless of caption
This commit is contained in:
@@ -468,32 +468,14 @@ class TelegramChannel(BaseChannel):
|
|||||||
|
|
||||||
@staticmethod
|
@staticmethod
|
||||||
def _extract_reply_context(message) -> str | None:
|
def _extract_reply_context(message) -> str | None:
|
||||||
"""Extract content from the message being replied to, if any. Truncated to TELEGRAM_REPLY_CONTEXT_MAX_LEN."""
|
"""Extract text from the message being replied to, if any."""
|
||||||
reply = getattr(message, "reply_to_message", None)
|
reply = getattr(message, "reply_to_message", None)
|
||||||
if not reply:
|
if not reply:
|
||||||
return None
|
return None
|
||||||
text = getattr(reply, "text", None) or getattr(reply, "caption", None)
|
text = getattr(reply, "text", None) or getattr(reply, "caption", None) or ""
|
||||||
if text:
|
if len(text) > TELEGRAM_REPLY_CONTEXT_MAX_LEN:
|
||||||
truncated = (
|
text = text[:TELEGRAM_REPLY_CONTEXT_MAX_LEN] + "..."
|
||||||
text[:TELEGRAM_REPLY_CONTEXT_MAX_LEN]
|
return f"[Reply to: {text}]" if text else None
|
||||||
+ ("..." if len(text) > TELEGRAM_REPLY_CONTEXT_MAX_LEN else "")
|
|
||||||
)
|
|
||||||
return f"[Reply to: {truncated}]"
|
|
||||||
# Reply has no text/caption; use type placeholder when it has media.
|
|
||||||
# Note: replied-to media is not attached to this message, so the agent won't receive it.
|
|
||||||
if getattr(reply, "photo", None):
|
|
||||||
return "[Reply to: (image — not attached)]"
|
|
||||||
if getattr(reply, "document", None):
|
|
||||||
return "[Reply to: (document — not attached)]"
|
|
||||||
if getattr(reply, "voice", None):
|
|
||||||
return "[Reply to: (voice — not attached)]"
|
|
||||||
if getattr(reply, "video_note", None) or getattr(reply, "video", None):
|
|
||||||
return "[Reply to: (video — not attached)]"
|
|
||||||
if getattr(reply, "audio", None):
|
|
||||||
return "[Reply to: (audio — not attached)]"
|
|
||||||
if getattr(reply, "animation", None):
|
|
||||||
return "[Reply to: (animation — not attached)]"
|
|
||||||
return "[Reply to: (no text)]"
|
|
||||||
|
|
||||||
async def _download_message_media(
|
async def _download_message_media(
|
||||||
self, msg, *, add_failure_content: bool = False
|
self, msg, *, add_failure_content: bool = False
|
||||||
@@ -629,14 +611,10 @@ class TelegramChannel(BaseChannel):
|
|||||||
message = update.message
|
message = update.message
|
||||||
user = update.effective_user
|
user = update.effective_user
|
||||||
self._remember_thread_context(message)
|
self._remember_thread_context(message)
|
||||||
reply_ctx = self._extract_reply_context(message)
|
|
||||||
content = message.text or ""
|
|
||||||
if reply_ctx:
|
|
||||||
content = reply_ctx + "\n\n" + content
|
|
||||||
await self._handle_message(
|
await self._handle_message(
|
||||||
sender_id=self._sender_id(user),
|
sender_id=self._sender_id(user),
|
||||||
chat_id=str(message.chat_id),
|
chat_id=str(message.chat_id),
|
||||||
content=content,
|
content=message.text or "",
|
||||||
metadata=self._build_message_metadata(message, user),
|
metadata=self._build_message_metadata(message, user),
|
||||||
session_key=self._derive_topic_session_key(message),
|
session_key=self._derive_topic_session_key(message),
|
||||||
)
|
)
|
||||||
@@ -677,17 +655,17 @@ class TelegramChannel(BaseChannel):
|
|||||||
if current_media_paths:
|
if current_media_paths:
|
||||||
logger.debug("Downloaded message media to {}", current_media_paths[0])
|
logger.debug("Downloaded message media to {}", current_media_paths[0])
|
||||||
|
|
||||||
# Reply context: include replied-to content; if reply has media, try to attach it
|
# Reply context: text and/or media from the replied-to message
|
||||||
reply = getattr(message, "reply_to_message", None)
|
reply = getattr(message, "reply_to_message", None)
|
||||||
reply_ctx = self._extract_reply_context(message)
|
if reply is not None:
|
||||||
if reply_ctx is not None and reply is not None:
|
reply_ctx = self._extract_reply_context(message)
|
||||||
if "not attached)]" in reply_ctx:
|
reply_media, reply_media_parts = await self._download_message_media(reply)
|
||||||
reply_media_paths, reply_media_parts = await self._download_message_media(reply)
|
if reply_media:
|
||||||
if reply_media_paths and reply_media_parts:
|
media_paths = reply_media + media_paths
|
||||||
reply_ctx = f"[Reply to: {reply_media_parts[0]}]"
|
logger.debug("Attached replied-to media: {}", reply_media[0])
|
||||||
media_paths = reply_media_paths + media_paths
|
tag = reply_ctx or (f"[Reply to: {reply_media_parts[0]}]" if reply_media_parts else None)
|
||||||
logger.debug("Attached replied-to media: {}", reply_media_paths[0])
|
if tag:
|
||||||
content_parts.insert(0, reply_ctx)
|
content_parts.insert(0, tag)
|
||||||
content = "\n".join(content_parts) if content_parts else "[empty message]"
|
content = "\n".join(content_parts) if content_parts else "[empty message]"
|
||||||
|
|
||||||
logger.debug("Telegram message from {}: {}...", sender_id, content[:50])
|
logger.debug("Telegram message from {}: {}...", sender_id, content[:50])
|
||||||
|
|||||||
@@ -379,32 +379,11 @@ def test_extract_reply_context_truncation() -> None:
|
|||||||
assert len(result) == len("[Reply to: ]") + TELEGRAM_REPLY_CONTEXT_MAX_LEN + len("...")
|
assert len(result) == len("[Reply to: ]") + TELEGRAM_REPLY_CONTEXT_MAX_LEN + len("...")
|
||||||
|
|
||||||
|
|
||||||
def test_extract_reply_context_no_text_no_media() -> None:
|
def test_extract_reply_context_no_text_returns_none() -> None:
|
||||||
"""When reply has no text/caption and no media, return (no text) placeholder."""
|
"""When reply has no text/caption, _extract_reply_context returns None (media handled separately)."""
|
||||||
reply = SimpleNamespace(
|
reply = SimpleNamespace(text=None, caption=None)
|
||||||
text=None,
|
|
||||||
caption=None,
|
|
||||||
photo=None,
|
|
||||||
document=None,
|
|
||||||
voice=None,
|
|
||||||
video_note=None,
|
|
||||||
video=None,
|
|
||||||
audio=None,
|
|
||||||
animation=None,
|
|
||||||
)
|
|
||||||
message = SimpleNamespace(reply_to_message=reply)
|
message = SimpleNamespace(reply_to_message=reply)
|
||||||
assert TelegramChannel._extract_reply_context(message) == "[Reply to: (no text)]"
|
assert TelegramChannel._extract_reply_context(message) is None
|
||||||
|
|
||||||
|
|
||||||
def test_extract_reply_context_reply_to_photo() -> None:
|
|
||||||
"""When reply has photo but no text/caption, return (image — not attached) placeholder."""
|
|
||||||
reply = SimpleNamespace(
|
|
||||||
text=None,
|
|
||||||
caption=None,
|
|
||||||
photo=[SimpleNamespace(file_id="x")],
|
|
||||||
)
|
|
||||||
message = SimpleNamespace(reply_to_message=reply)
|
|
||||||
assert TelegramChannel._extract_reply_context(message) == "[Reply to: (image — not attached)]"
|
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
@@ -518,13 +497,12 @@ async def test_on_message_attaches_reply_to_media_when_available(monkeypatch, tm
|
|||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_on_message_reply_to_media_fallback_when_download_fails() -> None:
|
async def test_on_message_reply_to_media_fallback_when_download_fails() -> None:
|
||||||
"""When reply has media but download fails, keep placeholder and do not attach."""
|
"""When reply has media but download fails, no media attached and no reply tag."""
|
||||||
channel = TelegramChannel(
|
channel = TelegramChannel(
|
||||||
TelegramConfig(enabled=True, token="123:abc", allow_from=["*"], group_policy="open"),
|
TelegramConfig(enabled=True, token="123:abc", allow_from=["*"], group_policy="open"),
|
||||||
MessageBus(),
|
MessageBus(),
|
||||||
)
|
)
|
||||||
channel._app = _FakeApp(lambda: None)
|
channel._app = _FakeApp(lambda: None)
|
||||||
# No get_file on bot -> download will fail
|
|
||||||
channel._app.bot.get_file = None
|
channel._app.bot.get_file = None
|
||||||
handled = []
|
handled = []
|
||||||
async def capture_handle(**kwargs) -> None:
|
async def capture_handle(**kwargs) -> None:
|
||||||
@@ -547,6 +525,75 @@ async def test_on_message_reply_to_media_fallback_when_download_fails() -> None:
|
|||||||
await channel._on_message(update, None)
|
await channel._on_message(update, None)
|
||||||
|
|
||||||
assert len(handled) == 1
|
assert len(handled) == 1
|
||||||
assert "[Reply to: (image — not attached)]" in handled[0]["content"]
|
|
||||||
assert "what is this?" in handled[0]["content"]
|
assert "what is this?" in handled[0]["content"]
|
||||||
assert handled[0]["media"] == []
|
assert handled[0]["media"] == []
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_on_message_reply_to_caption_and_media(monkeypatch, tmp_path) -> None:
|
||||||
|
"""When replying to a message with caption + photo, both text context and media are included."""
|
||||||
|
media_dir = tmp_path / "media" / "telegram"
|
||||||
|
media_dir.mkdir(parents=True)
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"nanobot.channels.telegram.get_media_dir",
|
||||||
|
lambda channel=None: media_dir if channel else tmp_path / "media",
|
||||||
|
)
|
||||||
|
|
||||||
|
channel = TelegramChannel(
|
||||||
|
TelegramConfig(enabled=True, token="123:abc", allow_from=["*"], group_policy="open"),
|
||||||
|
MessageBus(),
|
||||||
|
)
|
||||||
|
app = _FakeApp(lambda: None)
|
||||||
|
app.bot.get_file = AsyncMock(
|
||||||
|
return_value=SimpleNamespace(download_to_drive=AsyncMock(return_value=None))
|
||||||
|
)
|
||||||
|
channel._app = app
|
||||||
|
handled = []
|
||||||
|
async def capture_handle(**kwargs) -> None:
|
||||||
|
handled.append(kwargs)
|
||||||
|
channel._handle_message = capture_handle
|
||||||
|
channel._start_typing = lambda _chat_id: None
|
||||||
|
|
||||||
|
reply_with_caption_and_photo = SimpleNamespace(
|
||||||
|
text=None,
|
||||||
|
caption="A cute cat",
|
||||||
|
photo=[SimpleNamespace(file_id="cat_fid", mime_type="image/jpeg")],
|
||||||
|
document=None,
|
||||||
|
voice=None,
|
||||||
|
audio=None,
|
||||||
|
video=None,
|
||||||
|
video_note=None,
|
||||||
|
animation=None,
|
||||||
|
)
|
||||||
|
update = _make_telegram_update(
|
||||||
|
text="what breed is this?",
|
||||||
|
reply_to_message=reply_with_caption_and_photo,
|
||||||
|
)
|
||||||
|
await channel._on_message(update, None)
|
||||||
|
|
||||||
|
assert len(handled) == 1
|
||||||
|
assert "[Reply to: A cute cat]" in handled[0]["content"]
|
||||||
|
assert "what breed is this?" in handled[0]["content"]
|
||||||
|
assert len(handled[0]["media"]) == 1
|
||||||
|
assert "cat_fid" in handled[0]["media"][0]
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_forward_command_does_not_inject_reply_context() -> None:
|
||||||
|
"""Slash commands forwarded via _forward_command must not include reply context."""
|
||||||
|
channel = TelegramChannel(
|
||||||
|
TelegramConfig(enabled=True, token="123:abc", allow_from=["*"], group_policy="open"),
|
||||||
|
MessageBus(),
|
||||||
|
)
|
||||||
|
channel._app = _FakeApp(lambda: None)
|
||||||
|
handled = []
|
||||||
|
async def capture_handle(**kwargs) -> None:
|
||||||
|
handled.append(kwargs)
|
||||||
|
channel._handle_message = capture_handle
|
||||||
|
|
||||||
|
reply = SimpleNamespace(text="some old message", message_id=2, from_user=SimpleNamespace(id=1))
|
||||||
|
update = _make_telegram_update(text="/new", reply_to_message=reply)
|
||||||
|
await channel._forward_command(update, None)
|
||||||
|
|
||||||
|
assert len(handled) == 1
|
||||||
|
assert handled[0]["content"] == "/new"
|
||||||
|
|||||||
Reference in New Issue
Block a user