Skip to content

fix(telegram): improve local video delivery metadata - #61570

Open
Qwinty wants to merge 2 commits into
NousResearch:mainfrom
Qwinty:fix/telegram-video-metadata
Open

Qwinty wants to merge 2 commits into
NousResearch:mainfrom
Qwinty:fix/telegram-video-metadata

Conversation

@Qwinty

@Qwinty Qwinty commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes Telegram video delivery so MP4 attachments are sent with the metadata Telegram clients expect for playable video cards.

TelegramAdapter.send_video() now:

  • probes duration, width, and height with ffprobe
  • generates/caches a small JPEG thumbnail with ffmpeg
  • constrains thumbnails to Telegram's 320x320 / 200 KB limits
  • retries sendVideo without the thumbnail if Telegram rejects only the thumbnail
  • runs probing/thumbnail generation off the async event loop via asyncio.to_thread
  • passes supports_streaming=True
  • passes local filesystem paths directly when Telegram local Bot API mode is enabled

This avoids large valid MP4s being accepted by Telegram but rendered in clients as black/empty 00:00 download tiles.

Related Issue

Fixes #61569

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

  • plugins/platforms/telegram/adapter.py
    • add best-effort video metadata probing for sendVideo
    • add cached thumbnail generation for outgoing videos
    • enforce Telegram thumbnail size/dimension constraints
    • retry without thumbnail on thumbnail-specific Telegram errors
    • send local-mode videos by path with metadata/thumbnail
    • keep the old file-object upload path for non-local Bot API mode
  • tests/gateway/test_telegram_documents.py
    • assert supports_streaming is passed for video sends
    • add local-mode coverage for path, thumbnail, duration, width, and height kwargs
    • add thumbnail rejection retry coverage
    • add thumbnail constraint/oversize tests

How to Test

  1. python3 -m pytest tests/gateway/test_telegram_documents.py::TestSendVideo -q -o 'addopts='
  2. python3 -m pytest tests/gateway/test_telegram_documents.py -q -o 'addopts='
  3. python3 -m pytest tests/gateway/test_telegram_max_doc_bytes.py tests/gateway/test_telegram_send_path_health.py -q -o 'addopts='

Latest local results after review fixes:

TestSendVideo: 9 passed
test_telegram_documents.py: 48 passed
test_telegram_max_doc_bytes.py + test_telegram_send_path_health.py: 7 passed

Manual smoke test from my Telegram local Bot API setup returned a normal video object with metadata and thumbnail:

{'message_id': 41624, 'video_duration': 1057, 'width': 1920, 'height': 1080, 'file_size': 88052635, 'thumb': True}

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) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

For New Skills

N/A

Screenshots / Logs

Direct local Bot API smoke result:

{'message_id': 41624, 'video_duration': 1057, 'width': 1920, 'height': 1080, 'file_size': 88052635, 'thumb': True}

@Qwinty
Qwinty force-pushed the fix/telegram-video-metadata branch from bf69a37 to a0fb33e Compare July 9, 2026 16:57
@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery platform/telegram Telegram bot adapter sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages P2 Medium — degraded but workaround exists labels Jul 9, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for the focused Telegram delivery fix. The premise remains current: TelegramAdapter.send_video() on main only passes the video file object and routing/caption kwargs (plugins/platforms/telegram/adapter.py:6204-6218), while #61569 documents the missing metadata/thumbnail symptom.

Problems

  • The new coverage validates metadata and thumbnail forwarding only in local_mode. The ordinary multipart path added in this PR opens both files and constructs send_kwargs, but no test verifies that a successful probe forwards duration, width, height, supports_streaming, and thumbnail there. Existing TestSendVideo coverage on main ends at tests/gateway/test_telegram_documents.py:970 and its success test only checks that send_video was called.

Suggested changes

  • Add a default-mode regression with both helpers stubbed and assert the multipart send_video call receives the metadata plus an open thumbnail handle; retain the local-mode/rejection cases.

Automated hermes-sweeper review.

@Qwinty
Qwinty force-pushed the fix/telegram-video-metadata branch from a0fb33e to 1d9f8a6 Compare July 10, 2026 17:01
@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 11, 2026
@Qwinty
Qwinty force-pushed the fix/telegram-video-metadata branch from 1d9f8a6 to be77847 Compare July 19, 2026 13:30
@Qwinty

Qwinty commented Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current upstream/main.

Import conflict resolved by keeping both faulthandler (main) and hashlib (this PR). Video metadata + thumbnail path and multipart coverage retained.

This remains the preferred fix for #61569 from the issue reporter: ffprobe metadata, bounded thumbnail, async offload, tests. Competing PRs #61620 / #62019 cover overlapping ground; happy to align if maintainers prefer one of those shapes.

Local verification:

python -m pytest -q -o 'addopts=' tests/gateway/test_telegram_documents.py
# 49 passed

@Qwinty

Qwinty commented Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current upstream/main and force-pushed safely with an explicit lease.

Final head: ab94611b44335e78a703f9ad6c60ea84c7e57148

Verification:

  • python -m pytest -q tests/gateway/test_telegram_documents.py tests/gateway/test_telegram_media_read_timeout.py tests/gateway/test_telegram_max_doc_bytes.py25 passed
  • Includes the ordinary non-local_mode multipart regression proving video metadata and an open thumbnail stream are forwarded.
  • ruff check plugins/platforms/telegram/adapter.py tests/gateway/test_telegram_documents.pyAll checks passed
  • python -m py_compile ... → passed
  • git diff --check upstream/main...HEAD and git show --check HEAD → clean
  • Confirmed both modified committed files remain present in the final commit tree; worktree is clean.

@Qwinty

Qwinty commented Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

Final verification re-run from the clean rebased worktree confirms the published head is unchanged and complete.

  • Published/PR head: ab94611b44335e78a703f9ad6c60ea84c7e57148 (verified independently with both git ls-remote and the PR API)
  • Explicit safe push attempted with --force-with-lease=refs/heads/fix/telegram-video-metadata:ab94611b44335e78a703f9ad6c60ea84c7e57148; Git reported Everything up-to-date
  • uv run --frozen --extra messaging pytest -q tests/gateway/test_telegram_documents.py tests/gateway/test_telegram_media_read_timeout.py tests/gateway/test_telegram_max_doc_bytes.py25 passed
  • The multipart regression exercises the ordinary non-local send path and verifies probed dimensions/duration, supports_streaming, readable video input, and an open/readable thumbnail stream.
  • uv run --frozen --extra messaging ruff check plugins/platforms/telegram/adapter.py tests/gateway/test_telegram_documents.pyAll checks passed
  • python -m compileall -q for both changed Python files → passed
  • git diff --check upstream/main...HEAD → clean; worktree clean
  • GitHub scheduled the full matrix on this exact head; All required checks pass, with all 12 Python slices, lint, OS-specific tests, scans, Docker builds, and Nix green.

Qwinty added 2 commits August 24, 2026 23:50
Sweeper review on NousResearch#61570 asked for default-mode coverage: stub probe helpers
and assert send_video receives duration/width/height/supports_streaming plus
an open thumbnail handle on the ordinary multipart path.
@Qwinty
Qwinty force-pushed the fix/telegram-video-metadata branch from ab94611 to ed520fc Compare August 24, 2026 21:05
@Qwinty

Qwinty commented Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current upstream/main as ed520fc120055d511e74429401aa5e41ca3a04ce.

Preserved current-main Telegram adapter behavior while retaining the video metadata/thumbnail fix. Added/kept coverage for the ordinary multipart path forwarding duration, width, height, supports_streaming, and an open thumbnail handle.

Local verification on the committed tree:

  • scripts/run_tests.sh tests/gateway/test_telegram_documents.py -q22 passed
  • Ruff — passed
  • 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-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-platform-windows Sweeper risk: may break or behave differently on native Windows type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Telegram videos can render as black cards without sendVideo metadata

3 participants