Skip to content

fix(model): keep callback data short#179

Open
fengjikui wants to merge 2 commits into
grinev:mainfrom
fengjikui:fix/model-callback-data
Open

fix(model): keep callback data short#179
fengjikui wants to merge 2 commits into
grinev:mainfrom
fengjikui:fix/model-callback-data

Conversation

@fengjikui

Copy link
Copy Markdown

Summary

  • keep /models favorite/recent callbacks short by using list references instead of embedding provider/model IDs
  • keep model search result callbacks short by storing the selected models in interaction metadata
  • add regression coverage for long Fireworks-style model IDs that exceed Telegram callback_data limits

Why

Telegram rejects inline keyboard callback_data values longer than 64 bytes. Long provider/model IDs such as Fireworks deployment paths could make the model menu fail with BUTTON_DATA_INVALID before the user could select anything.

Fixes #178.

Validation

  • npx prettier --check src/bot/menus/model-selection-menu.ts src/bot/callbacks/model-selection-callback-handler.ts tests/bot/handlers/model.test.ts
  • npm test -- --run tests/bot/handlers/model.test.ts
  • npm run build
  • npm run lint
  • npm test
  • git diff --check

@grinev

grinev commented Jul 6, 2026

Copy link
Copy Markdown
Owner

@fengjikui thanks for the fix. One issue before merging:

Stale clicks on result buttons can select a wrong model. After the model-search interaction is cleared (e.g. by the next interactionManager.start()), clicking an already-rendered model:result:0 button reaches handleModelSelect and parseLegacyModelCallback parses it as providerID="result", modelID="0" — and the bogus model gets selected. Pre-PR, the real provider/model were embedded in the callback_data, so stale clicks still worked.

Same shape for model:list:recent:0 reaching handleModelSearchResults (the messageId guard mitigates it, but the legacy branch would otherwise read it as providerID="list", modelID="recent:0").

Could you make the legacy fallbacks reject the new prefixes? E.g. in handleModelSelect, explicitly drop/log model:result: instead of falling through to parseLegacyModelCallback, and exclude model:list: from the fallback in handleModelSearchResults. That way stale clicks fail safe instead of selecting a wrong model.

@fengjikui

Copy link
Copy Markdown
Author

Thanks for catching that. I pushed c6f1b53 to make the new short callback prefixes fail safe instead of falling through to the legacy parser:

  • model:result:* is now rejected by handleModelSelect when the search interaction is no longer active, so stale result buttons no longer become providerID="result" / modelID="0".
  • unresolved model:list:* callbacks no longer fall back to legacy parsing in handleModelSelect.
  • handleModelSearchResults also rejects short model:list:* / model:result:* prefixes from its legacy fallback path.

Added regression coverage for stale model:result:0, unresolved model:list:recent:0, and the search-results fallback case.

Validation:

  • npm test -- tests/bot/handlers/model.test.ts (19 tests)
  • npx prettier --check src/bot/callbacks/model-selection-callback-handler.ts tests/bot/handlers/model.test.ts
  • npm run build
  • npm run lint
  • git diff --check
  • npm test (121 files / 1069 tests)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Models with long IDs in recent crash /models menu with BUTTON_DATA_INVALID (Telegram 64-byte callback_data limit)

2 participants