Skip to content

fix(telegram): fail fast when user images cannot be accessed (rebase for moved file) - #63635

Open
HiddenPuppy wants to merge 2 commits into
NousResearch:mainfrom
HiddenPuppy:fix/issue-22385-telegram-image-failfast-rebased
Open

HiddenPuppy wants to merge 2 commits into
NousResearch:mainfrom
HiddenPuppy:fix/issue-22385-telegram-image-failfast-rebased

Conversation

@HiddenPuppy

Copy link
Copy Markdown
Contributor

Summary

Rebase of PR #22413 onto current main, resolving the file-move conflict.

The original PR modified gateway/platforms/telegram.py which no longer exists on main — the Telegram adapter was moved to plugins/platforms/telegram/adapter.py. This PR transplants the MIME fix to the new location.

Changes

File Change
plugins/platforms/telegram/adapter.py Normalize MIME type image/jpgimage/jpeg using standard MIME types (transplanted from old gateway/platforms/telegram.py)
gateway/run.py_build_media_placeholder Return neutral fallback when no media_urls exist, preventing empty user input
gateway/run.py_enrich_message_with_vision Track all_failed across all images. When ALL analyses fail AND the user provided no caption, return a clear fail-fast message
tests/gateway/test_vision_memory_leak.py Add 4 tests: all-failed/no-caption, all-failed/with-caption, exception/no-caption, mixed success/failure

Addresses Review Feedback

Per @teknium1's review on PR #22413:

  • ✅ MIME fix transplanted to plugins/platforms/telegram/adapter.py
  • ✅ 4 new regression tests added in test_vision_memory_leak.py

Fixes #22385
Closes #22413 (replaced by this rebased version)

…for moved file)

Cherry-pick of PR NousResearch#22413 logic onto current main.

Changes:
- MIME normalization image/jpg → image/jpeg in
  plugins/platforms/telegram/adapter.py (file was moved from
  gateway/platforms/telegram.py)
- _build_media_placeholder: return neutral fallback when no
  media_urls exist
- _enrich_message_with_vision: all_failed tracking + fail-fast
  message when all analyses fail and user provided no caption

Fixes NousResearch#22385
Add four new test cases for the all-failed / fail-fast logic in
_enrich_message_with_vision:
- test_all_failed_no_caption_returns_failfast
- test_all_failed_with_caption_returns_original
- test_all_exception_no_caption_returns_failfast
- test_mixed_success_failure_includes_user_text

Requested by reviewer @teknium1.
@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 sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Jul 13, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for rebasing the fix onto the moved Telegram adapter. The no-caption fail-fast branch addresses a behavior that still exists on current main: gateway/run.py:15329-15340 tells the model to retry vision_analyze after a failed analysis, and text-mode image routing calls that helper at gateway/run.py:10566-10574.

Problems

  • The new captioned-failure test says it returns only the original user text, but the changed condition only returns the fail-fast response when not user_text; with a caption, the existing failure placeholders remain. Align the test description/assertions with the intended behavior, or change the implementation accordingly.
  • plugins/platforms/telegram/adapter.py:8132 is the direct .jpg MIME emission site, but the PR adds no regression test for the new image/jpeg normalization.

Suggested changes

  • Add a .jpg photo-handler test asserting event.media_types == ["image/jpeg"].
  • Make the captioned all-failure test assert the actual intended output contract.

Automated hermes-sweeper review.

@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 16, 2026
@egilewski

Copy link
Copy Markdown
Contributor

suggesting changes

The all-failed captioned-image path still preserves the retry loop this change is meant to stop. With vision_analyze_tool returning {"success": false} and user_text="please describe this", _enrich_message_with_vision() still returns:

You can try looking at it yourself with vision_analyze using image_url: /cache/img.jpg

followed by the caption. That happens because the new fail-fast branch is gated by not user_text.strip(). It leaves a common Telegram request such as a captioned “please describe this” image telling the agent to retry the same unavailable analysis.

The new test_all_failed_with_caption_returns_original docstring says there should be “no enriched parts,” but the assertions only check that the caption is present and the new fail-fast phrase is absent, so the old retry instruction passes unnoticed. Please suppress the retry hint/cache path whenever all analyses fail (while preserving the caption as appropriate), and assert that the captioned result contains neither vision_analyze nor the image path.

Security evidence:

  • trust boundary: a failed Telegram image analysis becomes model-visible gateway input.
  • source/sink/invariant: all failed analyses must not return an instruction to retry the same unavailable image.
  • current-main reproduction: failed captioned analysis returns the existing vision_analyze retry instruction.
  • PR-head or patch-replay validation: the commits replay cleanly on current main, but failed captioned analysis still returns that instruction and cache path.
  • positive/negative cases: 7 focused tests pass; the explicit failed-analysis probe shows no-caption is fixed and captioned input is not.
  • residual bypass search: every non-whitespace caption bypasses the new all_failed fail-fast branch.
  • reviewer validation: the probe imported the replayed gateway.run and checked the exact captioned and uncaptioned outputs.

Signed: GPT-5.6-sol-xhigh in Codex

@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Summary

Fourteen PRs address or reference this issue complex across four distinct causes: verbose or capped auxiliary-vision output, lost system prompts, transient auto-vision failures, and inaccessible-image retry loops. The diffs range from internal concise preprocessing and provider-cap propagation (#10852), persona forwarding (#25656), retries (#28999/#29263), fail-fast handling (#22413/#63635), user-facing token configuration (#15430/#29600/#38004/#73438/#74939/#74945), and removal of vision caps (#75253).

Related pull requests

Duplicates

#22413 is superseded by #63635; #61187 substantially duplicates #10852; #29263 duplicates #28999; #15430, #29600, #38004, #73438, #74939, and #74945 share the rejected user-facing auxiliary.vision.max_tokens approach, with #74939 superseded by #74945 and #74945 re-scoped as #75253.

Suggested consolidation

Keep #63635 open with a salvage path, consistent with its automated keep-open verdict: the author should suppress vision_analyze and cached-path retry hints for captioned as well as uncaptioned all-failure cases, preserve the caption, and add the requested .jpg handler test before re-review; this explicitly addresses the contributor's blocking objection rather than merging over it. Close #22413 as a duplicate of #63635; retain #10852, #25656, and #75253 only on their distinct salvage paths, with #25656 requiring the listed test and call-site repairs, and leave the already-closed config-knob and retry duplicates closed.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I22385(["issue #22385 (closed)"])
    I28972(["issue #28972 (open)"])
    subgraph Dup22413 ["PRs duplicating each other"]
        P22413["PR #22413 (open)"]
        P63635["PR #63635 (open)"]
    end
    P63635 -->|best fix| I22385
    P63635 -.->|partial| I28972
    class I22385 closed
    class I28972 open
    class P22413 open
    class P63635 open
    class P63635 best
    class P63635 target
    click I22385 "https://github.com/NousResearch/hermes-agent/issues/22385"
    click I28972 "https://github.com/NousResearch/hermes-agent/issues/28972"
    click P22413 "https://github.com/NousResearch/hermes-agent/pull/22413"
    click P63635 "https://github.com/NousResearch/hermes-agent/pull/63635"
Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 14 pull requests and 5 issues in this complex. Diffs were read for 13 of 14 PRs (rest unavailable); Assessment working set: 110 kB of PR diffs, 48 kB of issue/PR text, 15 kB of discussion (22 comments), 29 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

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 comp/plugins Plugin system and bundled plugins 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-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Telegram: images sent by user are not accessible — agent spends 30+ min failing to read them

5 participants