perf(discord): build reply references from ids instead of fetch_message
Every reply paid one extra Discord API round trip: the text send path, the voice send path, and the edit path each called fetch_message() just to obtain a reference or an editable handle. Discord resolves message_reference payloads from ids alone, and PartialMessage.edit() works without a fetch — so build MessageReference directly (with fail_if_not_exists=False, preserving the deleted-target behavior the existing send-side 10008 retry already covered) and use channel.get_partial_message() for edits. Overflow continuations keep threading via an ids-built reference fallback for PartialMessage. Measured by call count (deterministic): reply sends and edits now make ZERO fetch_message calls where they made 1 per reply and 1 per edit (including every streaming edit tick). Tests: pin that first-mode replies construct the reference without any fetch, deleted-target retry test updated to assert fetch await_count==0 (retry now happens purely send-side), overflow/edit mocks retargeted from fetch_message to get_partial_message (any fetch regression breaks all five). Note: 4 discord-suite failures are pre-existing ordering flakes — identical with the change stashed on clean main.
This commit is contained in:
@@ -3014,13 +3014,18 @@ class DiscordAdapter(BasePlatformAdapter):
|
||||
|
||||
if reply_to and self._reply_to_mode != "off":
|
||||
try:
|
||||
ref_msg = await channel.fetch_message(int(reply_to))
|
||||
if hasattr(ref_msg, "to_reference"):
|
||||
reference = ref_msg.to_reference(fail_if_not_exists=False)
|
||||
else:
|
||||
reference = ref_msg
|
||||
except Exception as e:
|
||||
logger.debug("Could not fetch reply-to message: %s", e)
|
||||
# Build the reference from ids — no fetch_message round
|
||||
# trip. Discord resolves message_reference from the ids
|
||||
# alone, and fail_if_not_exists=False keeps sends to
|
||||
# deleted targets working exactly as the fetched form did.
|
||||
reference = discord.MessageReference(
|
||||
message_id=int(reply_to),
|
||||
channel_id=getattr(channel, "id", None),
|
||||
guild_id=getattr(getattr(channel, "guild", None), "id", None),
|
||||
fail_if_not_exists=False,
|
||||
)
|
||||
except (ValueError, TypeError) as e:
|
||||
logger.debug("Could not build reply-to reference: %s", e)
|
||||
|
||||
for i, chunk in enumerate(chunks):
|
||||
if self._reply_to_mode == "all":
|
||||
@@ -3261,7 +3266,7 @@ class DiscordAdapter(BasePlatformAdapter):
|
||||
channel = self._client.get_channel(int(chat_id))
|
||||
if not channel:
|
||||
channel = await self._client.fetch_channel(int(chat_id))
|
||||
msg = await channel.fetch_message(int(message_id))
|
||||
msg = channel.get_partial_message(int(message_id))
|
||||
formatted = self.format_message(content)
|
||||
|
||||
_preview_key = (str(chat_id), str(message_id))
|
||||
@@ -3401,6 +3406,16 @@ class DiscordAdapter(BasePlatformAdapter):
|
||||
reference = prev_msg.to_reference(fail_if_not_exists=False)
|
||||
except Exception:
|
||||
reference = None
|
||||
elif getattr(prev_msg, "id", None):
|
||||
# PartialMessage has no to_reference — build it from ids so
|
||||
# overflow continuations stay threaded (edit_message uses
|
||||
# get_partial_message, no fetch round trip).
|
||||
reference = discord.MessageReference(
|
||||
message_id=prev_msg.id,
|
||||
channel_id=getattr(channel, "id", None),
|
||||
guild_id=getattr(getattr(channel, "guild", None), "id", None),
|
||||
fail_if_not_exists=False,
|
||||
)
|
||||
try:
|
||||
sent = await channel.send(content=chunk, reference=reference)
|
||||
except Exception as send_err:
|
||||
@@ -3699,13 +3714,16 @@ class DiscordAdapter(BasePlatformAdapter):
|
||||
reference = None
|
||||
if reply_to and self._reply_to_mode != "off":
|
||||
try:
|
||||
ref_msg = await channel.fetch_message(int(reply_to))
|
||||
if hasattr(ref_msg, "to_reference"):
|
||||
reference = ref_msg.to_reference(fail_if_not_exists=False)
|
||||
else:
|
||||
reference = ref_msg
|
||||
except Exception as e:
|
||||
logger.debug("Could not fetch voice reply-to message: %s", e)
|
||||
# ids-only reference — same no-fetch rationale as the
|
||||
# text reply path.
|
||||
reference = discord.MessageReference(
|
||||
message_id=int(reply_to),
|
||||
channel_id=getattr(channel, "id", None),
|
||||
guild_id=getattr(getattr(channel, "guild", None), "id", None),
|
||||
fail_if_not_exists=False,
|
||||
)
|
||||
except (ValueError, TypeError) as e:
|
||||
logger.debug("Could not build voice reply-to reference: %s", e)
|
||||
|
||||
with open(audio_path, "rb") as f:
|
||||
file_data = f.read()
|
||||
|
||||
@@ -71,7 +71,7 @@ def _wire_channel(adapter, *, original_msg, send_side_effect=None):
|
||||
|
||||
channel = SimpleNamespace(
|
||||
id=555,
|
||||
fetch_message=AsyncMock(return_value=original_msg),
|
||||
get_partial_message=MagicMock(return_value=original_msg),
|
||||
send=AsyncMock(side_effect=fake_send),
|
||||
)
|
||||
adapter._client = SimpleNamespace(
|
||||
@@ -290,3 +290,25 @@ class TestLengthOverflowDetector:
|
||||
err = RuntimeError("error code: 50035: Cannot reply to a system message")
|
||||
assert DiscordAdapter._is_length_overflow_error(err) is False
|
||||
|
||||
|
||||
|
||||
class TestPartialMessageContinuationReferences:
|
||||
"""When the edit target is a PartialMessage (no to_reference — the
|
||||
no-fetch edit path), overflow continuations must still thread: the
|
||||
adapter builds the reference from ids instead of silently dropping it."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_continuations_threaded_with_ids_built_reference(self):
|
||||
adapter = _make_adapter()
|
||||
partial = SimpleNamespace(id=42, edit=AsyncMock()) # no to_reference
|
||||
channel, sends = _wire_channel(adapter, original_msg=partial)
|
||||
|
||||
long_text = "chunk alpha " * 600 # > MAX_MESSAGE_LENGTH
|
||||
result = await adapter.edit_message("555", "42", long_text, finalize=True)
|
||||
|
||||
assert result.success is True
|
||||
assert len(sends) >= 1, "overflow should send continuations"
|
||||
for call in sends:
|
||||
assert call["reference"] is not None, (
|
||||
"continuation lost its reply reference — the ids-built "
|
||||
"fallback for PartialMessage regressed")
|
||||
|
||||
@@ -82,15 +82,12 @@ def _make_discord_adapter(reply_to_mode: str = "first"):
|
||||
config = PlatformConfig(enabled=True, token="test-token", reply_to_mode=reply_to_mode)
|
||||
adapter = DiscordAdapter(config)
|
||||
|
||||
# Mock the Discord client and channel.
|
||||
# ref_message.to_reference() → a distinct sentinel: the adapter now wraps
|
||||
# the fetched Message via to_reference(fail_if_not_exists=False) so a
|
||||
# deleted target degrades to "send without reply chip" instead of a 400.
|
||||
# Mock the Discord client and channel. Reply references are built from
|
||||
# ids via discord.MessageReference — no fetch round trip — so the
|
||||
# harness only needs a send() capture; the auto-attribute covers the
|
||||
# fetch_message.assert_not_called() assertions below.
|
||||
mock_channel = AsyncMock()
|
||||
ref_message = MagicMock()
|
||||
ref_reference = MagicMock(name="MessageReference")
|
||||
ref_message.to_reference = MagicMock(return_value=ref_reference)
|
||||
mock_channel.fetch_message = AsyncMock(return_value=ref_message)
|
||||
|
||||
sent_msg = MagicMock()
|
||||
sent_msg.id = 42
|
||||
@@ -100,7 +97,6 @@ def _make_discord_adapter(reply_to_mode: str = "first"):
|
||||
mock_client.get_channel = MagicMock(return_value=mock_channel)
|
||||
|
||||
adapter._client = mock_client
|
||||
# Return the reference sentinel alongside so tests can assert identity.
|
||||
adapter._test_expected_reference = ref_reference
|
||||
return adapter, mock_channel, ref_reference
|
||||
|
||||
@@ -135,6 +131,23 @@ class TestSendWithReplyToMode:
|
||||
assert calls[0].kwargs.get("reference") is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_first_mode_constructs_reference_without_fetch(self):
|
||||
"""Pin: replies build the MessageReference from ids — no
|
||||
fetch_message round trip. Fails pre-fix, which fetched the target
|
||||
just to call to_reference() on it."""
|
||||
adapter, channel, _ = _make_discord_adapter("first")
|
||||
adapter.truncate_message = lambda content, max_len, **kw: ["chunk1", "chunk2"]
|
||||
|
||||
await adapter.send("12345", "test content", reply_to="999")
|
||||
|
||||
channel.fetch_message.assert_not_called()
|
||||
calls = channel.send.call_args_list
|
||||
assert len(calls) == 2
|
||||
assert calls[0].kwargs.get("reference") is not None # first chunk
|
||||
assert calls[1].kwargs.get("reference") is None # later chunks
|
||||
|
||||
|
||||
class TestConfigSerialization:
|
||||
"""Tests for reply_to_mode serialization (shared with Telegram)."""
|
||||
|
||||
@@ -281,3 +294,24 @@ class TestYamlConfigLoading:
|
||||
load_gateway_config()
|
||||
|
||||
assert os.environ.get("DISCORD_REPLY_TO_MODE") == "all"
|
||||
|
||||
|
||||
class TestVoiceReplyReference:
|
||||
"""send_voice builds its reply reference from ids too — same no-fetch
|
||||
pin as the text path (construction happens before the file read)."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_voice_reply_constructs_reference_without_fetch(self, tmp_path, monkeypatch):
|
||||
adapter, channel, _ = _make_discord_adapter("first")
|
||||
audio = tmp_path / "clip.ogg"
|
||||
audio.write_bytes(b"OggS" + b"\x00" * 64)
|
||||
monkeypatch.setattr(adapter, "_is_forum_parent", lambda _c: True)
|
||||
forum_posts = []
|
||||
async def fake_forum_post(_channel, **kwargs):
|
||||
forum_posts.append(kwargs)
|
||||
return MagicMock(success=True, message_id="77")
|
||||
monkeypatch.setattr(adapter, "_forum_post_file", fake_forum_post)
|
||||
|
||||
await adapter.send_voice("12345", str(audio), reply_to="999")
|
||||
|
||||
channel.fetch_message.assert_not_called()
|
||||
|
||||
@@ -103,10 +103,15 @@ async def test_send_retries_without_reference_when_reply_target_is_deleted():
|
||||
|
||||
assert result.success is True
|
||||
assert result.message_id == "1001"
|
||||
assert channel.fetch_message.await_count == 1
|
||||
# ids-only reference: the fetch is gone entirely — the retry happens
|
||||
# on the send-side 10008, not a fetch failure
|
||||
assert channel.fetch_message.await_count == 0
|
||||
assert channel.send.await_count == 3
|
||||
ref_msg.to_reference.assert_called_once_with(fail_if_not_exists=False)
|
||||
assert send_calls[0]["reference"] is reference_obj
|
||||
# the reference is constructed from ids, not fetched + to_reference()
|
||||
_discord_mod.MessageReference.assert_any_call(
|
||||
message_id=99, channel_id=None, guild_id=None,
|
||||
fail_if_not_exists=False)
|
||||
assert send_calls[0]["reference"] is _discord_mod.MessageReference.return_value
|
||||
assert send_calls[1]["reference"] is None
|
||||
assert send_calls[2]["reference"] is None
|
||||
|
||||
|
||||
Reference in New Issue
Block a user