fix(discord): make clarify button labels always legible - #62291
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for addressing a real Discord clarify UX issue. Current main still builds button labels from choice text in plugins/platforms/discord/adapter.py:7552-7584, while Telegram already uses body text plus numeric labels at plugins/platforms/telegram/adapter.py:4689-4724.
Problems
plugins/platforms/discord/adapter.py:5806in this diff truncatesoption_linesto 1024 characters, but the changed embed value appends a 65-character instruction suffix. A long option list can therefore create an 1089-character Choices field, above Discord's 1024-character cap, andsend_clarify()will return a failed send after the caught API error.- This Discord-focused PR also carries unrelated changes in
agent/chat_completion_helpers.py,hermes_cli/doctor.py,hermes_cli/plugins_cmd.py,plugins/memory/hindsight/__init__.py, andtools/lazy_deps.py.
Suggested changes
- Cap the complete Choices-field value, reserving instruction text before truncating options, and add a boundary regression test asserting the field stays within 1024 characters.
- Split the unrelated carried commits so the Discord fix can be reviewed and salvaged independently.
Automated hermes-sweeper review.
| # which has a much higher cap (1024 for embed fields). | ||
| option_lines = "\n".join( | ||
| f"**{i + 1}.** {c}" for i, c in enumerate(clean_choices) | ||
| ) |
There was a problem hiding this comment.
option_lines is capped at 1024 here, but the next field value appends the 65-character instruction suffix. Cap the complete rendered field (or reserve suffix space first), otherwise a long option list exceeds Discord's 1024-character embed-field limit and the prompt send fails.
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.
bde33c0 to
7275c6b
Compare
|
Thanks for the catch — both points were real, fixed in the updated commit:
Verified: |
|
The inline review comment on option_lines_full = "\n".join(
f"**{i + 1}.** {c}" for i, c in enumerate(clean_choices)
)
embed_suffix = (
"\n\nPick a button below, or click ✏️ Other to type a "
"custom answer."
)
max_field = 1024
option_lines = option_lines_full
if len(option_lines) + len(embed_suffix) > max_field:
budget = max_field - len(embed_suffix) - 3
option_lines = option_lines_full[:budget] + "..."Verified the arithmetic directly: with the 65-char suffix reserved before truncation, |
GottZ
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Summary
Four PRs address the Discord clarify-choice readability problem: #62291 replaces clipped choice-text buttons with numeric buttons and mirrors numbered choices, #72606 and #72608 retain shortened text labels while adding full-choice lists, and #73545 conditionally adds a full-options field for choices exceeding a mobile-oriented threshold. The diffs differ mainly in overflow handling and UI semantics, with none fully handling every Discord text-surface limit as currently written.
Related pull requests
- #62291
related— (+174/-133) — preferred, pending one boundary fix: replaces inherently clipped mobile labels with numeric buttons and mirrors normalized choices in the embed and plain content; the reviewed 1,024-character embed-field overflow and unrelated-file problems are resolved in the current diff, but the unbounded plain-text tail still needs a 2,000-character aggregate guard and regression test before merge. - #72606 [closed]
related— (+216/-7) — superseded closed prototype: demonstrates the full-choice-list approach, but its cached diff includes unrelated custom-provider and TLS/header changes, and its 4,088-character description slice can silently discard later choices; it remains relevant as prior implementation evidence for #72608. - #72608
related— (+104/-10) — salvageable but not preferred: keeps truncated choice text on buttons and mirrors full choices into the embed and plain content, but the contributor review identifies two blocking diff-level defects—silent loss after the 4,088-character description slice and content potentially exceeding Discord's 2,000-character limit. Despite the keep_open review on #72608, consolidation onto #62291 is preferable because #62291 also fixes mobile button legibility rather than preserving clipped labels; the review's overflow concerns must still be carried into #62291 before merge. - #73545
related— (+26/-1) — incomplete alternative: minimally and conditionally adds a “Full options” field while preserving existing button semantics, but joins all normalized choices into an unbounded embed-field value with no 1,024-character guard or aggregate-overflow test, so valid long inputs can fail Discord validation.
Duplicates
#72606 and #72608 are substantially the same implementation lineage; #62291, #72608, and #73545 overlap on exposing numbered full-choice text but use competing UI and overflow strategies rather than being exact duplicates.
Suggested consolidation
Merge #62291 after bounding the complete plain-content mirror to Discord's 2,000-character limit with explicit overflow behavior and a boundary regression test. Then close #72608 and #73545 as superseded alternatives, explicitly carrying forward the keep_open review on #72608 rather than treating it as resolved; #72606 is already closed and remains superseded by its cleaned successor and the more complete #62291 design.
Cross-PR triage: Reviewed 4 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 46 kB of PR diffs, 8 kB of issue/PR text, 5 kB of discussion (7 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.
Summary
Discord clarify buttons previously embedded truncated choice text directly in button labels. Mobile clients cut or wrap those labels aggressively, so even valid 80-character labels remain difficult to read.
Changes
[additional choice text truncated]while preserving the prompt and response instructions.Tests
Coverage verifies short button labels, full choice visibility, dict unwrapping, embed limits, and the 2,000-character plain-content cap with oversized choices.
git diff --checkis clean.