fix(discord): keep clarify prompts visible when embeds are hidden - #36186
fix(discord): keep clarify prompts visible when embeds are hidden#36186carltonawong wants to merge 1 commit into
Conversation
mxnstrexgl
left a comment
There was a problem hiding this comment.
LGTM — automated review passed. No security, quality, or test coverage issues detected.
c8bc3d5 to
6dcb9b2
Compare
|
Refreshed this branch on top of current The PR is still intentionally narrow: it only changes the Discord Validated after rebasing: python -m pytest -o addopts='' tests/gateway/test_discord_clarify_buttons.py -q
python -m py_compile plugins/platforms/discord/adapter.py tests/gateway/test_discord_clarify_buttons.py
git diff --check origin/main...HEADResults:
Would appreciate a maintainer review when someone has bandwidth. I think this is complementary to #33681 because it covers the safety-critical clarify case where Discord renders buttons but hides the embed body the user needs before choosing. |
Duplicate the critical clarify prompt body into normal Discord message content while preserving the existing embed and button view. This keeps draft/approval text visible when Discord clients render components but hide the embed body.
6dcb9b2 to
924b461
Compare
|
Refreshed this branch on top of current I also re-checked the related GitHub/source context before pushing:
The rebase conflict was only around current Validated after rebasing: python -m pytest -o addopts='' tests/gateway/test_discord_clarify_buttons.py -q
python -m py_compile plugins/platforms/discord/adapter.py tests/gateway/test_discord_clarify_buttons.py
git diff --check origin/main...HEADResults:
I also reran the prior red |
Discord clarify buttons put the (truncated) choice text directly on the button label, relying on Discord's 80-char label cap plus a word-boundary truncation algorithm. In practice this is still unreadable: Discord mobile clients wrap/cut button text well before 80 chars (often <40 visible), so any truncation strategy on the label itself still garbles longer choices regardless of how the cut point is chosen. PR NousResearch#54969 (merged as 87be36c) already fixed a related but different bug in this same truncation path -- code-point slicing instead of UTF-16 unit slicing, which could corrupt emoji-heavy choice text at the cut boundary. That fix made the truncation correct; this one removes the need for truncation at all. Telegram and WhatsApp adapters already avoid this class of bug entirely: button labels are just a short index, and the full choice text is mirrored in the message body where there's no meaningful length cap. This brings Discord's send_clarify() in line with that same pattern: - ClarifyChoiceView button labels are now just the option number ("1", "2", ...) instead of "1. <truncated text>". Removed the now-dead word/soft-boundary truncation logic for this path. - send_clarify() renders the full, untruncated choice text as a numbered list in the embed's "Choices" field (1024-char cap, truncated only if genuinely absurd) and mirrors it again in the plain-text message content, matching the existing embeds-may-be- invisible-on-some-clients precaution already used elsewhere in this adapter. Related: NousResearch#36186 (different bug in the same file -- clarify text disappearing when Discord hides embed bodies entirely, not button label truncation). Review fixes (per hermes-sweeper automated review on PR NousResearch#62291): - The embed field truncation only capped the option-list portion at 1024 chars, then appended a ~65-char instruction suffix afterward -- the combined value could reach ~1089 chars, over Discord's real 1024-char embed-field limit, causing send_clarify() to fail on exactly the long-choice-list case this fix exists to handle. Now reserves suffix length before truncating so the combined field value never exceeds the cap. Added a boundary regression test with 24 long choices asserting the rendered field stays <= 1024 chars and the suffix is never dropped. - The plain-text message-content mirror now explicitly uses the *untruncated* option list (Discord's 2000-char content cap is much larger than the embed field's 1024), rather than reusing whatever the embed truncated to. - Rebased onto a clean upstream main and dropped four unrelated fork-local commits (three explicitly tagged [carried], one untagged plugin symlink fix) that had accumulated on the source branch before this PR was cut -- this PR is Discord-only again. Tests: tests/gateway/test_discord_clarify_buttons.py -- 5 truncation-behavior tests replaced with simpler "label is always short" tests, 5 dict-unwrap tests updated to assert the full text lands in the embed field rather than the button label, plus 1 new boundary regression test for the field-cap fix. 21/21 pass. ruff clean. git diff --check clean.
|
Thanks for the focused safety fallback. This is now implemented on current
Closing as implemented on main. |
Summary
Discord web/mobile can render the component buttons for an embed-backed prompt while hiding the embed body itself. That leaves
send_clarify()prompts in a dangerous state: the user can see the choices, but not the question/draft text they are choosing about.This adds a narrow defensive fallback for Discord
send_clarify()only: the critical prompt body is also sent as normalmessage.content, while preserving the existing embed and button view.Related: #33681
Changes
plugins/platforms/discord/adapter.pytests/gateway/test_discord_clarify_buttons.pyScope
This intentionally covers only
send_clarify().Issue #33681 notes the same Discord embed+components rendering class may affect
send_exec_approval,send_slash_confirm,send_update_prompt, the model picker, and free-response cards. Those are left for a follow-up so this PR stays small and reviewable.The reason to start with clarify is that clarify prompts can contain draft/approval text where choosing a button without seeing the body is unsafe.
Validation
python -m pytest -o addopts='' tests/gateway/test_discord_clarify_buttons.py -q→14 passedpython -m py_compile plugins/platforms/discord/adapter.py tests/gateway/test_discord_clarify_buttons.pygit diff --check