Skip to content

fix(telegram): merge forwarded batch attachments into startup turns - #46101

Open
Qwinty wants to merge 2 commits into
NousResearch:mainfrom
Qwinty:fix/telegram-forward-batch-startup-merge
Open

fix(telegram): merge forwarded batch attachments into startup turns#46101
Qwinty wants to merge 2 commits into
NousResearch:mainfrom
Qwinty:fix/telegram-forward-batch-startup-merge

Conversation

@Qwinty

@Qwinty Qwinty commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Summary

Addresses the remaining startup-registration race related to #46100: a normal text turn can begin while a forwarded or standalone attachment is still downloading or waiting for Telegram topic recovery. This is distinct from the existing media_group_id album aggregation already on main.

  • Uses a dedicated startup slot, separate from the generic FIFO head, so /queue and existing pending turns are never absorbed.
  • Tracks accepted photo/voice/audio/video/video-note/document downloads per canonical session key; size-rejected media never creates a false pending signal.
  • Keeps non-photo tracking active through BasePlatformAdapter.handle_message() registration, closing the post-download/topic-recovery gap where voice/video/document could otherwise miss the startup collector.
  • Merges forwarded text, standalone media, photo bursts, and media groups during a bounded 1-second startup grace; ordinary text with no pending forward/media keeps a no-wait fast path.
  • Preserves per-item forward attribution inline and keeps profile + recovered DM-topic session keys consistent across adapter guards, batching, and runner state.
  • Registers filters.VIDEO_NOTE and adds MessageType.VIDEO_NOTE.
  • Disconnect cleanup clears startup slots and download counters.

Fixes #33270

Verification

  • The delayed-registration regression (voice download finishes while topic/busy registration waits) merges into the first text turn.
  • Focused + adjacent Telegram/gateway/platform-registry verification: 155 passed.
  • Ruff, py_compile, and git diff --check pass.

Follow-up generation-safe lifecycle hardening across /stop//new//reset: #81371.

@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 Jun 14, 2026
@Qwinty
Qwinty force-pushed the fix/telegram-forward-batch-startup-merge branch from da68a31 to a756d0b Compare June 14, 2026 11:31
@Qwinty Qwinty changed the title fix(telegram): merge forwarded batch attachments into startup turns fix(telegram): merge batch attachments into startup turns Jun 14, 2026
@Qwinty
Qwinty force-pushed the fix/telegram-forward-batch-startup-merge branch from a756d0b to 956dfdd Compare June 14, 2026 16:24
@Qwinty
Qwinty force-pushed the fix/telegram-forward-batch-startup-merge branch 2 times, most recently from 5564a0d to 54899f3 Compare June 16, 2026 08:56
@Qwinty
Qwinty marked this pull request as ready for review July 10, 2026 17:10
@Qwinty
Qwinty force-pushed the fix/telegram-forward-batch-startup-merge branch 2 times, most recently from 1018f6a to 0147a58 Compare July 13, 2026 21:21

@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 consolidating the Telegram startup-media work from #33272. The underlying race is present on current main: text flushes independently at plugins/platforms/telegram/adapter.py:7660-7702, while photo processing downloads and queues separately at plugins/platforms/telegram/adapter.py:7809-7834.

Problems

  • gateway/run.py:5704 is not scoped to Telegram or to a verified batch. Any platform's busy media event now bypasses configured interrupt/steer behavior and the busy acknowledgement.
  • gateway/run.py:5521-5534 waits through the grace deadline even when there is no pending media signal, adding the default one-second delay to normal Telegram text-only turns.
  • gateway/run.py:5415 adds HERMES_TELEGRAM_STARTUP_MEDIA_GRACE_SECONDS; AGENTS.md:102-106 requires non-secret behavioral configuration to use config.yaml, not a new HERMES_* variable.

Suggested changes

  • Scope the media bypass to verified Telegram startup-batch follow-ups and preserve normal busy-mode semantics otherwise.
  • Avoid unconditional startup waiting; use an adapter-observed signal or an adapter-side batch boundary.
  • Replace the environment-variable interface with a fixed internal bound or config.yaml-backed setting.

Automated hermes-sweeper review.

Comment thread gateway/run.py Outdated
Comment thread gateway/run.py Outdated
Comment thread gateway/run.py Outdated
@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-broad Sweeper blast radius: broad — a core path most sessions hit labels Jul 14, 2026
@Qwinty
Qwinty force-pushed the fix/telegram-forward-batch-startup-merge branch from 9af1d0e to d34ba72 Compare July 19, 2026 13:30
@Qwinty

Qwinty commented Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current upstream/main and fixed a conflict-resolution regression in the image-document path:

  • keep startup media download tracking from this PR
  • restore main's get_file() / download_as_bytearray() step
  • keep redacted Telegram error logging from main

Local verification:

python -m pytest -q -o 'addopts=' \
  tests/gateway/test_telegram_forwarded_batch_startup_merge.py \
  tests/gateway/test_telegram_documents.py \
  tests/gateway/test_telegram_text_batching.py \
  tests/gateway/test_busy_session_ack.py
# 97 passed

@Qwinty

Qwinty commented Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

Late findings follow-up

Pushed 7853d71df to harden the startup media buffer path after independent review:

  • track video and video-document downloads in the startup pending counters (same grace window as photos)
  • treat forwarded text still sitting in the Telegram debounce buffer as startup-pending, and consume it via pop_startup_media_event
  • added focused regressions in tests/gateway/test_telegram_startup_buffer_regressions.py

Local verification: targeted Telegram startup/document suites green.

@Qwinty

Qwinty commented Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

CI follow-up

Fixed the slice failure on test_telegram_video_size_gate_rejects_oversized_media_before_download:

  • size gate now runs before startup download tracking for voice/audio/video
  • session-key derivation tolerates events without source (no AttributeError on rejection paths)

Pushed on top of the late-findings commit.

@Qwinty

Qwinty commented Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

Final independent-review follow-up

Pushed 5d11f17c3:

  • media counters now use profile-aware, Telegram topic-recovered session keys
  • metadata-empty [Forwarded message] headers join startup buffering too
  • tests now exercise the real debounce buffer plus live native-video and video-document counter lifecycles

Local verification: 66 passed; ruff clean.

@Qwinty
Qwinty force-pushed the fix/telegram-forward-batch-startup-merge branch from 5d11f17 to 0cbabe2 Compare July 29, 2026 12:24
@Qwinty

Qwinty commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Rebuilt this PR as a single focused commit on current upstream/main (5cc5c58e0). The old broad forwarded-media implementation and conflict baggage are gone.

Current scope:

  • close the text-before-photo registration race with bounded zero-delay event-loop yields
  • wait up to 1s only after the Telegram adapter reports concrete same-session standalone-photo work
  • merge only the adapter's :photo-burst startup buffer; albums/media groups and generic pending FIFO stay on their existing paths
  • keep image download counters profile/topic aware and clear them in finally
  • align the secondary-profile adapter guard with the same profile-aware session key

Verification on head 0cbabe263:

  • 192 focused gateway tests passed
  • focused startup regression file: 14 passed
  • ruff, py_compile, and git diff --check passed
  • independent final review: no blockers

@Qwinty

Qwinty commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up after the asynchronous reviews completed: one reviewer reproduced a real 5 ms callback-registration gap that the zero-delay yields did not cover.

Pushed 7b3c6916b:

  • replace fixed sleep(0) hops with a bounded 10 ms registration grace, polled every 2 ms
  • retain the 1 s download grace only after the adapter reports concrete pending standalone-photo work
  • add a regression with an actual 5 ms timer gap

Verification on the new head:

  • focused startup file: 14 passed
  • wider focused gateway matrix: 193 passed
  • ruff, py_compile, and diff check passed
  • direct probes: 5 ms registration merges reliably; no-photo path stays about 10.5–11 ms; album/FIFO/profile invariants remain intact
  • independent follow-up review: no blockers

This supersedes my previous “final” status comment; I should have waited for all outstanding async reviews before reporting completion.

@kh-mitya

Copy link
Copy Markdown

UP here

@Qwinty
Qwinty force-pushed the fix/telegram-forward-batch-startup-merge branch from 667ef73 to 70504d9 Compare August 21, 2026 13:55
@Qwinty

Qwinty commented Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

Refreshed onto current main as 70504d901b. The normal text-only path now returns immediately unless the Telegram adapter reports concrete pending startup media; the startup merge window uses a fixed internal one-second upper bound with no undocumented HERMES_* behavior setting. Profile/topic session keys and current-main busy-mode behavior are preserved. Added a real Fixes #33270 closing reference. Local verification: 69 focused tests passed; Ruff, py_compile, and git diff --check passed.

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-broad Sweeper blast radius: broad — a core path most sessions hit 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

4 participants