Conversation
A report handed to save_attachment's or attachment_data's progress handler carried only the message id, so a caller had to look the message up to find out whether the transfer was for anything it was showing. The conversation is already known at both sites, so the report carries it. callbacks::attachment_progress keeps its leading ConversationId: it is now the same value as the field, but other consumers take it.
It is on the report now, so the parameter said the same thing twice. A handler that files these by conversation needs the value wherever a report comes from, and the one handed to attachment_data or save_attachment has no second argument to carry it - so the struct is the only place that can hold it for both. display_picture_progress keeps its parameter: it has no struct to put one on, which is the same reason it has no message or index.
This was referenced Sep 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A progress report broadcast through
callbacks::attachment_progresscame with the conversation beside it, but one handed to theon_progresshandler a caller passed toattachment_dataorsave_attachmentcarried onlymessage_id. So an application routing a report to the conversation view it belongs to had to look the message up again — for every tick of every transfer.AttachmentProgressnow carriesconversation_id. The Client knows it at both construction sites, so nothing extra is looked up:_attachment_pointergains the value by joiningmessagesonto the query it was already making.The second commit then takes the leading
ConversationIdparameter offcallbacks::attachment_progress, which the field makes a duplicate.display_picture_progresskeeps its parameter — it has no struct to put one on, which is the same reason it carries no message or index.I checked the consumers before changing the signature: session-cli never registers this callback, it only passes
on_progresstoattachment_data. The only consumer is session-app's bridge, which is ours and updates alongside.Tests: the existing save and auto-download cases now assert the conversation on the reports they already collect, and the auto-download case asserts the broadcast field is the value the parameter used to carry.
Built and run against the client suite: 134 cases, 1010 assertions, passing.
Independent of the two PRs alongside it and mergeable in any order. It and #166 both add
#include <session/client/conversation_id.hpp>toattachment.hpp; identical additions, so whichever merges second is a no-op there.Note:
tests/static-bundle-testfails to link onclientalready, before this change (undefined reference to SQLite::Database::exec). Unrelated and untouched.