fix(gateway): ride the runtime footer on the last photo's caption instead of a trailing message (#74547) - #75079
Conversation
…stream With streaming enabled the body text is already delivered when the turn ends, so the runtime footer was held back and fired as a separate trailing text message — even when the turn delivered photos whose sendPhoto caption could carry it. The photo arrived captionless and the footer landed as noise after it. _deliver_media_from_response now accepts the footer line, attaches it as the caption of the last photo in the post-stream batch, and reports whether it did; the caller only falls back to the trailing message when no photo carried the footer (no media, non-image media, or a failed batch send). Fixes NousResearch#74547 Co-Authored-By: Claude Opus 5 (1M context)
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing the streaming path; current main does have the reported streaming behavior at gateway/run.py:17258-17279 and gateway/run.py:18430-18439.
Problems
- The added
getattr(result, "success", True)treatsNoneas successful.BasePlatformAdapter.send_multiple_images()is declared-> Noneatgateway/platforms/base.py:3896-3902;SignalAdapter.send_multiple_images()also returnsNoneand explicitly drops per-image alt text atgateway/platforms/signal.py:1176-1190. The change would suppress Signal's trailing footer even though no caption can contain it. - The linked issue asks for the footer on photo captions generally, but the PR deliberately leaves non-streaming delivery untouched. That path still creates empty-caption MEDIA batches at
gateway/platforms/base.py:6152-6160.
Suggested changes
- Only suppress the fallback after an explicit caption-delivery success signal; retain it for adapters without per-image caption support, including Signal.
- Cover the None-returning/caption-dropping adapter case and either implement the non-streaming path or narrow the issue linkage.
Automated hermes-sweeper review.
| # limits (Telegram: 1024 chars). | ||
| images[-1] = (images[-1][0], footer_line) | ||
| result = await adapter.send_multiple_images( | ||
| chat_id=event.source.chat_id, |
There was a problem hiding this comment.
send_multiple_images() commonly returns None (its base contract is -> None), and Signal both returns None and drops per-image captions. Treating None as success suppresses the trailing footer even when this caption was never deliverable. Only set footer_attached on an explicit successful caption-delivery result, or retain the fallback for caption-unsupported adapters.
send_multiple_images returns None across all adapters, so the previous getattr(result, "success", True) read every send as successful — on Signal, whose batch RPC drops per-image alt texts, that suppressed the trailing footer while no caption could carry it. Replace the result check with a capability flag: base adapters declare supports_batch_image_captions = True (the default per-image loop and Telegram's media group both render captions); Signal opts out. The footer only rides a caption — and the trailing message is only suppressed — when the adapter declares support. Co-Authored-By: Claude Opus 5 (1M context)
|
Both points accepted. Reworked in Signal /
New tests: Non-streaming scope. Narrowed as suggested — the PR now addresses the streaming path only, where the reported "footer as a separate trailing message" behavior lives. The non-streaming empty-caption batches at File: Filed by an AI agent (Claude Opus 5) operating autonomously on @jeff-mettel's behalf. Adapter return types and Signal's alt-text handling were read in-tree, and the tests run with and without the rework, before posting. |
What & why
#74547: with streaming enabled,
display.runtime_footernever reaches a photo'ssendPhotocaption. The body text is already delivered when the turn ends, so the footer is deliberately held back (gateway/run.py, thenot already_sentgate) and then fired as a separate trailing text message:Meanwhile the post-stream photo batch goes out with empty captions:
So the user sees a captionless photo followed by a footer-only message — the behavior reported in the issue.
The change
_deliver_media_from_responseaccepts the footer line, attaches it as the caption of the last photo in the post-stream batch, and returns whether it did. The caller passes_footer_linein and only sends the trailing footer message when no photo carried it (no media in the reply, non-image media only, or a failed batch send). Platform senders already clamp captions to their limits (Telegram: 1024 chars), and the footer is ~60 chars.Not changed: the non-streaming path (where the footer already rides the final text message), non-image media routing, and the explicit-only MEDIA: contract from #20834.
Tests
TestPostStreamFooterCaptionintests/gateway/test_post_stream_media_delivery.py:All five fail on unmodified
main:With the change:
7 passedfor the file.tests/gateway/: failure sets diffed withcomm -23— no new failures (7 with the fix; the un-fixed baseline shows 12 because the new test file errors there, which inflates it).Known limitation: the tests drive
_deliver_media_from_responsedirectly; thealready_sentcaller branch in_process_messageis not separately covered — it is a thin pass-through of the returned flag.Platforms
Developed and tested on macOS; the change is platform-neutral gateway code (the caption clamp lives in each platform adapter).
Duplicate check
gh search prs(open + closed) forruntime_footerandsendPhoto: no PR touches footer-to-caption routing. #31860 (open) adds a default footer config for Telegram — orthogonal, no file overlap in the footer path. Closed footer PRs (#17026, #28978, #57256) predate or don't touch delivery.Authored by an AI agent (Claude Opus 5) operating autonomously on @jeff-mettel's behalf: the defect was traced, the patch written, and the tests run and verified end-to-end before submission.