Skip to content

fix(gateway): merge Telegram startup media into text turns - #33272

Closed
Qwinty wants to merge 1 commit into
NousResearch:mainfrom
Qwinty:fix/telegram-text-photo-race
Closed

fix(gateway): merge Telegram startup media into text turns#33272
Qwinty wants to merge 1 commit into
NousResearch:mainfrom
Qwinty:fix/telegram-text-photo-race

Conversation

@Qwinty

@Qwinty Qwinty commented May 27, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes a Telegram cross-modal startup race where a short text message can start an agent turn before a same-session photo finishes downloading and flushing from the photo batch buffer.

The runner now gives Telegram text-only events a short bounded chance to merge same-session image followups before preparing prompt text and native image routing. This preserves the text fast path for normal text-only messages while preventing image-backed questions from becoming text-only turns.

Related Issue

Fixes #33270

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Security fix
  • Documentation update
  • Tests (adding or improving test coverage)
  • Refactor (no behavior change)
  • New skill (bundled or hub)

Changes Made

  • gateway/platforms/telegram.py: tracks in-progress same-session image downloads and exposes a helper to pop/merge startup photo-burst and media-group buffers.
  • gateway/run.py: merges same-session Telegram image followups into text-only events before prompt preparation and image routing, including a bounded grace window controlled by HERMES_TELEGRAM_STARTUP_MEDIA_GRACE_SECONDS.
  • tests/gateway/test_telegram_text_photo_startup_merge.py: covers buffered photo batches, in-progress photo downloads, and image followups already queued by the active-session pending path.

How to Test

  1. Send a short Telegram text question to a cold or just-starting session.
  2. Immediately send a photo/image that provides the question context.
  3. Confirm the first agent turn includes the text and image, and native image routing sees the image before the agent invocation.

Focused regression commands run:

/usr/local/lib/hermes-agent/venv/bin/python -m pytest -q -o addopts='' tests/gateway/test_telegram_text_photo_startup_merge.py tests/gateway/test_telegram_photo_interrupts.py tests/gateway/test_telegram_text_batching.py tests/gateway/test_telegram_text_batch_perf.py tests/gateway/test_native_image_buffer_isolation.py
# 23 passed, 1 warning in 3.63s

/usr/local/lib/hermes-agent/venv/bin/python -m pytest -q -o addopts='' tests/gateway/test_telegram_documents.py tests/gateway/test_telegram_max_doc_bytes.py tests/gateway/test_telegram_caption_merge.py tests/gateway/test_telegram_text_photo_startup_merge.py
# 60 passed, 1 warning in 5.46s

/usr/local/lib/hermes-agent/venv/bin/python -m pytest -q -o addopts='' tests/gateway/test_telegram_text_photo_startup_merge.py
# 3 passed, 1 warning in 0.77s

/usr/local/lib/hermes-agent/venv/bin/python -m ruff check gateway/run.py gateway/platforms/telegram.py tests/gateway/test_telegram_text_photo_startup_merge.py
# All checks passed!

/usr/local/lib/hermes-agent/venv/bin/python -m py_compile gateway/run.py gateway/platforms/telegram.py tests/gateway/test_telegram_text_photo_startup_merge.py

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: Ubuntu 24.04

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — gateway/Telegram async logic only
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A

Screenshots / Logs

Sanitized failing order from the production session:

15:11:54.430 Flushing text batch ... (26 chars)
15:11:54.432 inbound message ... msg='Это реальное исследование?'
15:11:54.554 Cached user photo at /root/.hermes/image_cache/...
15:11:55.376 Flushing photo batch ... with 1 image(s)
15:11:55.996 conversation turn ... history=0 msg='Это реальное исследование?'

The image was cached locally but did not reach image routing before the turn started.

@Qwinty
Qwinty force-pushed the fix/telegram-text-photo-race branch from d65ec65 to ed296ee Compare May 27, 2026 14:22
@Qwinty

Qwinty commented May 27, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up after the first CI run: test (6) exposed that the initial implementation used hasattr(adapter, ...), which is unsafe with bare MagicMock adapters in unit tests because it materializes dynamic methods and can make the startup-media loop treat a mock as a real Telegram adapter extension.

I updated the branch to resolve adapter extension methods via inspect.getattr_static(...) first, so only adapters that actually declare pop_startup_media_event / has_startup_media_pending participate.

Additional local verification after the update:

/usr/local/lib/hermes-agent/venv/bin/python -m pytest -q -o addopts='' tests/gateway/test_status_command.py
# 14 passed in 1.29s

/usr/local/lib/hermes-agent/venv/bin/python -m pytest -q -o addopts='' tests/gateway/test_status_command.py::test_handle_message_persists_agent_token_counts tests/gateway/test_status_command.py::test_first_run_non_slack_home_channel_onboarding_keeps_direct_command tests/gateway/test_status_command.py::test_handle_message_discards_stale_result_after_session_invalidation
# 3 passed in 1.58s

/usr/local/lib/hermes-agent/venv/bin/python -m pytest -q -o addopts='' tests/gateway/test_telegram_text_photo_startup_merge.py tests/gateway/test_telegram_photo_interrupts.py tests/gateway/test_telegram_text_batching.py tests/gateway/test_telegram_text_batch_perf.py tests/gateway/test_native_image_buffer_isolation.py
# 23 passed, 1 warning in 2.87s

/usr/local/lib/hermes-agent/venv/bin/python -m pytest -q -o addopts='' tests/gateway/test_telegram_documents.py tests/gateway/test_telegram_max_doc_bytes.py tests/gateway/test_telegram_caption_merge.py tests/gateway/test_telegram_text_photo_startup_merge.py
# 60 passed, 1 warning in 5.52s

/usr/local/lib/hermes-agent/venv/bin/python -m ruff check gateway/run.py gateway/platforms/telegram.py tests/gateway/test_telegram_text_photo_startup_merge.py tests/gateway/test_status_command.py
# All checks passed!

@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery platform/telegram Telegram bot adapter P2 Medium — degraded but workaround exists labels May 27, 2026
@Qwinty
Qwinty force-pushed the fix/telegram-text-photo-race branch 2 times, most recently from 4d5b9b8 to ee9aee5 Compare June 9, 2026 12:29
@Qwinty

Qwinty commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Rebuilt this PR on current upstream/main after the Telegram adapter moved into plugins/platforms/telegram/adapter.py.

The refreshed implementation preserves the original fix: before prompt/image preparation, a starting Telegram text turn now waits briefly for same-session image work and steals any pending photo/image-document batches so a back-to-back “text + screenshot” arrives as one multimodal turn instead of text-only + queued image.

Local verification:

  • python -m pytest -q -o 'addopts=' tests/gateway/test_telegram_text_photo_startup_merge.py → 3 passed
  • python -m pytest -q -o 'addopts=' tests/gateway/test_telegram_documents.py tests/gateway/test_telegram_text_photo_startup_merge.py → 47 passed
  • python -m ruff check gateway/run.py plugins/platforms/telegram/adapter.py tests/gateway/test_telegram_text_photo_startup_merge.py → passed

@Qwinty
Qwinty force-pushed the fix/telegram-text-photo-race branch from ee9aee5 to 3f83c71 Compare July 3, 2026 17:17
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for rebuilding this after the Telegram adapter migration. The underlying race is present on current main: short text batches can flush at the 0.18s fast-path cap (plugins/platforms/telegram/adapter.py:7683-7702), while photo handling awaits download/cache work before enqueuing the photo (plugins/platforms/telegram/adapter.py:7809-7834); once active, photos are queued for a later turn (gateway/platforms/base.py:4763-4769).

Problems

  • 3f83c712 adds HERMES_TELEGRAM_STARTUP_MEDIA_GRACE_SECONDS in gateway/run.py. This is a user-facing behavioral setting; AGENTS.md:93-96 requires config.yaml rather than a new non-secret HERMES_* environment variable.
  • The in-progress-download test in 3f83c712 pre-seeds the download counter before entering the merge helper. It does not cover the remaining window where the text turn checks before the photo handler has registered work.

Suggested changes

  • Remove the new environment-variable interface; use a fixed internal bound or a documented config.yaml-backed setting.
  • Add a regression covering a photo registration that begins after startup merge starts.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages 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 13, 2026
@Qwinty

Qwinty commented Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

Closing this in favor of #46101. After rebasing #46101 onto current upstream, that branch covers this text+photo startup race and the broader Telegram startup attachment/forwarded-text batching cases in one place, with focused coverage in tests/gateway/test_telegram_forwarded_batch_startup_merge.py plus the existing Telegram batching/document/busy-session slices. Keeping both PRs open would leave two overlapping fixes in the same startup merge path.

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

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists 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 sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Telegram text+photo sends can start a text-only turn before the image is attached

3 participants