From 2080e98b67c9b613c5d8046ec4a505c2731103aa Mon Sep 17 00:00:00 2001 From: MerlinH Date: Tue, 28 Jul 2026 18:08:08 +0000 Subject: [PATCH 1/2] fix(discord): show full long clarify options --- plugins/platforms/discord/adapter.py | 40 +++++++++ tests/gateway/test_discord_clarify_buttons.py | 87 +++++++++++++++++++ 2 files changed, 127 insertions(+) diff --git a/plugins/platforms/discord/adapter.py b/plugins/platforms/discord/adapter.py index 4787bc6b5225..07c06d2ab0be 100644 --- a/plugins/platforms/discord/adapter.py +++ b/plugins/platforms/discord/adapter.py @@ -52,6 +52,8 @@ def __init__(self, id: int) -> None: # noqa: A002 - matches discord API _DISCORD_COMMAND_SYNC_STATE_SUBDIR = "gateway" _DISCORD_COMMAND_SYNC_STATE_FILENAME = "discord_command_sync_state.json" _DISCORD_NONCONVERSATIONAL_STATE_FILENAME = "discord_nonconversational_messages.json" +_DISCORD_FULL_OPTION_THRESHOLD_UTF16_UNITS = 40 +_DISCORD_EMBED_FIELD_UTF16_LIMIT = 1024 _DISCORD_COMMAND_SYNC_MUTATION_INTERVAL_SECONDS = 4.5 _DISCORD_COMMAND_SYNC_MAX_RATE_LIMIT_SLEEP_SECONDS = 30.0 @@ -6803,6 +6805,44 @@ def _flatten_choice(c): value="Pick one below, or click ✏️ Other to type a custom answer.", inline=False, ) + if any( + utf16_len(choice) > _DISCORD_FULL_OPTION_THRESHOLD_UTF16_UNITS + for choice in clean_choices + ): + full_options = "\n".join( + f"{index}. {choice}" for index, choice in enumerate( + clean_choices, start=1 + ) + ) + lines = full_options.split("\n") + chunks = [] + chunk = [] + chunk_units = 0 + for line in lines: + line_units = utf16_len(line) + if not chunk: + chunk.append(line) + chunk_units = line_units + continue + if chunk_units + 1 + line_units <= _DISCORD_EMBED_FIELD_UTF16_LIMIT: + chunk.append(line) + chunk_units += 1 + line_units + else: + chunks.append("\n".join(chunk)) + chunk = [line] + chunk_units = line_units + if chunk: + chunks.append("\n".join(chunk)) + + for chunk_index, chunk_value in enumerate(chunks): + embed.add_field( + name=( + "Full options" + if chunk_index == 0 else "Full options (continued)" + ), + value=chunk_value, + inline=False, + ) view = ClarifyChoiceView( choices=clean_choices, clarify_id=clarify_id, diff --git a/tests/gateway/test_discord_clarify_buttons.py b/tests/gateway/test_discord_clarify_buttons.py index a72d225c6186..238e476bf288 100644 --- a/tests/gateway/test_discord_clarify_buttons.py +++ b/tests/gateway/test_discord_clarify_buttons.py @@ -380,6 +380,12 @@ async def test_multi_choice_attaches_view(self): assert isinstance(kwargs["view"], ClarifyChoiceView) # 3 choice buttons + 1 Other assert len(kwargs["view"].children) == 4 + fields = {f["name"]: f["value"] for f in kwargs["embed"].fields} + assert ( + fields["Choices"] + == "Pick one below, or click ✏️ Other to type a custom answer." + ) + assert "Full options" not in fields @pytest.mark.asyncio async def test_open_ended_omits_view(self): @@ -462,6 +468,47 @@ async def test_filters_empty_and_whitespace_choices(self): assert len(view.children) == 2 assert "real-choice" in view.children[0].label + @pytest.mark.asyncio + async def test_long_choice_renders_numbered_choices_in_embed_field(self): + adapter = _make_adapter() + channel = MagicMock() + sent_msg = MagicMock() + sent_msg.id = 901 + channel.send = AsyncMock(return_value=sent_msg) + adapter._client.get_channel = MagicMock(return_value=channel) + + long_choice = "x" * 41 + short_choice = "Use cached defaults" + assert utf16_len(long_choice) > 40 + + await adapter.send_clarify( + chat_id="9001", + question="Choose one:", + choices=[short_choice, long_choice, "No"], + clarify_id="cidFull", + session_key="sk-Full", + ) + + kwargs = channel.send.call_args.kwargs + embed = kwargs["embed"] + fields = {f["name"]: f["value"] for f in embed.fields} + assert ( + fields["Choices"] + == "Pick one below, or click ✏️ Other to type a custom answer." + ) + expected_full = "\n".join( + [ + "1. Use cached defaults", + f"2. {long_choice}", + "3. No", + ] + ) + assert fields["Full options"] == expected_full + view = kwargs["view"] + assert view.children[0].label == "1. Use cached defaults" + assert view.children[1].label == f"2. {long_choice}" + assert view.children[2].label == "3. No" + @pytest.mark.asyncio async def test_unwraps_dict_choices_to_description(self): # LLMs sometimes emit [{"description": "..."}] instead of bare strings @@ -592,3 +639,43 @@ async def test_unwrap_does_not_pick_value_or_name_alone(self): for label in choice_labels: assert "only_name_here" not in label, f"name leaked: {label!r}" assert "only_value_here" not in label, f"value leaked: {label!r}" + + @pytest.mark.asyncio + async def test_full_options_splits_when_exceeding_1024_utf16_units(self): + adapter = _make_adapter() + channel = MagicMock() + sent_msg = MagicMock() + sent_msg.id = 999 + channel.send = AsyncMock(return_value=sent_msg) + adapter._client.get_channel = MagicMock(return_value=channel) + + choices = ["x" * 41 for _ in range(24)] + assert len(choices) == 24 + assert all(utf16_len(c) > 40 for c in choices) + + await adapter.send_clarify( + chat_id="9001", + question="Choose one:", + choices=choices, + clarify_id="cidSplit", + session_key="sk-Split", + ) + + kwargs = channel.send.call_args.kwargs + embed = kwargs["embed"] + full_option_fields = [ + f + for f in embed.fields + if f["name"] in {"Full options", "Full options (continued)"} + ] + assert len(full_option_fields) > 1 + assert full_option_fields[0]["name"] == "Full options" + assert all( + f["name"] == "Full options (continued)" for f in full_option_fields[1:] + ) + assert all(utf16_len(f["value"]) <= 1024 for f in full_option_fields) + expected = "\n".join( + f"{idx}. {choice}" for idx, choice in enumerate(choices, start=1) + ) + reconstructed = "\n".join(f["value"] for f in full_option_fields) + assert reconstructed == expected From bf63375afe32a521a8ca6b3eefbe38eb7c8a65cc Mon Sep 17 00:00:00 2001 From: MerlinH Date: Tue, 28 Jul 2026 18:21:27 +0000 Subject: [PATCH 2/2] refactor(discord): keep long-option fallback minimal --- plugins/platforms/discord/adapter.py | 41 ++------- tests/gateway/test_discord_clarify_buttons.py | 91 ++----------------- 2 files changed, 15 insertions(+), 117 deletions(-) diff --git a/plugins/platforms/discord/adapter.py b/plugins/platforms/discord/adapter.py index 07c06d2ab0be..db928999d6cc 100644 --- a/plugins/platforms/discord/adapter.py +++ b/plugins/platforms/discord/adapter.py @@ -53,7 +53,6 @@ def __init__(self, id: int) -> None: # noqa: A002 - matches discord API _DISCORD_COMMAND_SYNC_STATE_FILENAME = "discord_command_sync_state.json" _DISCORD_NONCONVERSATIONAL_STATE_FILENAME = "discord_nonconversational_messages.json" _DISCORD_FULL_OPTION_THRESHOLD_UTF16_UNITS = 40 -_DISCORD_EMBED_FIELD_UTF16_LIMIT = 1024 _DISCORD_COMMAND_SYNC_MUTATION_INTERVAL_SECONDS = 4.5 _DISCORD_COMMAND_SYNC_MAX_RATE_LIMIT_SLEEP_SECONDS = 30.0 @@ -6809,40 +6808,14 @@ def _flatten_choice(c): utf16_len(choice) > _DISCORD_FULL_OPTION_THRESHOLD_UTF16_UNITS for choice in clean_choices ): - full_options = "\n".join( - f"{index}. {choice}" for index, choice in enumerate( - clean_choices, start=1 - ) + embed.add_field( + name="Full options", + value="\n".join( + f"{index}. {choice}" + for index, choice in enumerate(clean_choices, start=1) + ), + inline=False, ) - lines = full_options.split("\n") - chunks = [] - chunk = [] - chunk_units = 0 - for line in lines: - line_units = utf16_len(line) - if not chunk: - chunk.append(line) - chunk_units = line_units - continue - if chunk_units + 1 + line_units <= _DISCORD_EMBED_FIELD_UTF16_LIMIT: - chunk.append(line) - chunk_units += 1 + line_units - else: - chunks.append("\n".join(chunk)) - chunk = [line] - chunk_units = line_units - if chunk: - chunks.append("\n".join(chunk)) - - for chunk_index, chunk_value in enumerate(chunks): - embed.add_field( - name=( - "Full options" - if chunk_index == 0 else "Full options (continued)" - ), - value=chunk_value, - inline=False, - ) view = ClarifyChoiceView( choices=clean_choices, clarify_id=clarify_id, diff --git a/tests/gateway/test_discord_clarify_buttons.py b/tests/gateway/test_discord_clarify_buttons.py index 238e476bf288..6dfd7d646acc 100644 --- a/tests/gateway/test_discord_clarify_buttons.py +++ b/tests/gateway/test_discord_clarify_buttons.py @@ -361,11 +361,12 @@ async def test_multi_choice_attaches_view(self): sent_msg.id = 123456 channel.send = AsyncMock(return_value=sent_msg) adapter._client.get_channel = MagicMock(return_value=channel) + long_choice = "x" * 41 result = await adapter.send_clarify( chat_id="9001", question="Pick a color", - choices=["red", "green", "blue"], + choices=["red", long_choice, "blue"], clarify_id="cidM", session_key="sk-M", ) @@ -385,7 +386,10 @@ async def test_multi_choice_attaches_view(self): fields["Choices"] == "Pick one below, or click ✏️ Other to type a custom answer." ) - assert "Full options" not in fields + assert fields["Full options"] == f"1. red\n2. {long_choice}\n3. blue" + assert kwargs["view"].children[0].label == "1. red" + assert kwargs["view"].children[1].label == f"2. {long_choice}" + assert kwargs["view"].children[2].label == "3. blue" @pytest.mark.asyncio async def test_open_ended_omits_view(self): @@ -464,51 +468,12 @@ async def test_filters_empty_and_whitespace_choices(self): ) kwargs = channel.send.call_args.kwargs view = kwargs["view"] + fields = {f["name"]: f["value"] for f in kwargs["embed"].fields} + assert "Full options" not in fields # Only 1 real choice + 1 Other = 2 children assert len(view.children) == 2 assert "real-choice" in view.children[0].label - @pytest.mark.asyncio - async def test_long_choice_renders_numbered_choices_in_embed_field(self): - adapter = _make_adapter() - channel = MagicMock() - sent_msg = MagicMock() - sent_msg.id = 901 - channel.send = AsyncMock(return_value=sent_msg) - adapter._client.get_channel = MagicMock(return_value=channel) - - long_choice = "x" * 41 - short_choice = "Use cached defaults" - assert utf16_len(long_choice) > 40 - - await adapter.send_clarify( - chat_id="9001", - question="Choose one:", - choices=[short_choice, long_choice, "No"], - clarify_id="cidFull", - session_key="sk-Full", - ) - - kwargs = channel.send.call_args.kwargs - embed = kwargs["embed"] - fields = {f["name"]: f["value"] for f in embed.fields} - assert ( - fields["Choices"] - == "Pick one below, or click ✏️ Other to type a custom answer." - ) - expected_full = "\n".join( - [ - "1. Use cached defaults", - f"2. {long_choice}", - "3. No", - ] - ) - assert fields["Full options"] == expected_full - view = kwargs["view"] - assert view.children[0].label == "1. Use cached defaults" - assert view.children[1].label == f"2. {long_choice}" - assert view.children[2].label == "3. No" - @pytest.mark.asyncio async def test_unwraps_dict_choices_to_description(self): # LLMs sometimes emit [{"description": "..."}] instead of bare strings @@ -639,43 +604,3 @@ async def test_unwrap_does_not_pick_value_or_name_alone(self): for label in choice_labels: assert "only_name_here" not in label, f"name leaked: {label!r}" assert "only_value_here" not in label, f"value leaked: {label!r}" - - @pytest.mark.asyncio - async def test_full_options_splits_when_exceeding_1024_utf16_units(self): - adapter = _make_adapter() - channel = MagicMock() - sent_msg = MagicMock() - sent_msg.id = 999 - channel.send = AsyncMock(return_value=sent_msg) - adapter._client.get_channel = MagicMock(return_value=channel) - - choices = ["x" * 41 for _ in range(24)] - assert len(choices) == 24 - assert all(utf16_len(c) > 40 for c in choices) - - await adapter.send_clarify( - chat_id="9001", - question="Choose one:", - choices=choices, - clarify_id="cidSplit", - session_key="sk-Split", - ) - - kwargs = channel.send.call_args.kwargs - embed = kwargs["embed"] - full_option_fields = [ - f - for f in embed.fields - if f["name"] in {"Full options", "Full options (continued)"} - ] - assert len(full_option_fields) > 1 - assert full_option_fields[0]["name"] == "Full options" - assert all( - f["name"] == "Full options (continued)" for f in full_option_fields[1:] - ) - assert all(utf16_len(f["value"]) <= 1024 for f in full_option_fields) - expected = "\n".join( - f"{idx}. {choice}" for idx, choice in enumerate(choices, start=1) - ) - reconstructed = "\n".join(f["value"] for f in full_option_fields) - assert reconstructed == expected