Skip to content

fix: preserve prioritized Telegram skill commands - #57938

Closed
cortanapham wants to merge 1 commit into
NousResearch:mainfrom
cortanapham:fix/telegram-priority-skill-menu-cap
Closed

fix: preserve prioritized Telegram skill commands#57938
cortanapham wants to merge 1 commit into
NousResearch:mainfrom
cortanapham:fix/telegram-priority-skill-menu-cap

Conversation

@cortanapham

Copy link
Copy Markdown

Summary

  • preserve configured Telegram command-menu priorities for skill/plugin commands before applying the BotCommand cap
  • collect Telegram skill/plugin menu candidates before trimming, then sort the combined command list by effective priority
  • add a regression test proving a configured priority skill survives a tight Telegram menu cap

Why

platforms.telegram.extra.command_menu.priority lets users pin important commands, but prioritized skill commands could still be dropped because Telegram skill entries were trimmed before the combined priority sort. This meant operational skill commands configured as high priority could disappear from the visible Telegram menu when built-in/core commands consumed the cap first.

Tests

  • python -m pytest tests/hermes_cli/test_commands.py -k 'TelegramMenuCommands' -o 'addopts=' -q

@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard platform/telegram Telegram bot adapter P3 Low — cosmetic, nice to have duplicate This issue or pull request already exists labels Jul 3, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #55192 — same author, byte-for-byte identical hermes_cli/commands.py diff (the max_slots: int | None seam in _collect_gateway_skill_entries), created 4 days later. #55192 is the canonical (earliest open) version. Related to the Telegram menu-priority cluster: #33706, #31099.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the focused regression fix. The premise is confirmed on current main: hermes_cli/commands.py:917-927 limits skill collection before the combined menu can be priority-sorted, while configured priority is applied only to the core/plugin list at hermes_cli/commands.py:912.

Problems

  • hermes_cli/commands.py:897-903 documents a fixed core → plugin → skill ordering, with core commands always included and only skills trimmed. After the new global priority sort, a configured-priority skill can precede and displace an unprioritized core command under the cap, so that contract is stale.

Suggested changes

  • Update that docstring to describe priority application over the combined eligible command list before truncation, and the resulting meaning of hidden_count.

Automated hermes-sweeper review.

Comment thread hermes_cli/commands.py
# Drop the cmd_key — Telegram only needs (name, desc) pairs.
all_commands.extend((n, d) for n, d, _k in entries)
return all_commands[:max_commands], hidden_count + hidden_core_count
all_commands = _prioritize_telegram_menu_commands(all_commands)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new global priority pass means the function docstring above is no longer accurate: it currently promises core → plugin → skill ordering, always-included core commands, and skill-only trimming. Please update that contract to describe sorting the combined eligible list before enforcing the Telegram cap.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 15, 2026
@teknium1

Copy link
Copy Markdown
Contributor

This is now fixed on main by #98280 (commit 3e6229e), which salvaged #82516 — the same priority-before-cap approach you pioneered here first (June 29, the earliest submission of this fix along with your #55192). Sorry we couldn't land your branch directly; the merged version extends the re-rank across core+plugin+skill tiers with a unified priority list. Crediting you here as the first submitter of the fix idea. Thanks @cortanapham!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard duplicate This issue or pull request already exists P3 Low — cosmetic, nice to have platform/telegram Telegram bot adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants