Skip to content

Drop attachment captions - #2

Open
Bilb wants to merge 1 commit into
jagerman:attachment-availabilityfrom
Bilb:feat/drop-attachment-caption
Open

Bilb wants to merge 1 commit into
jagerman:attachment-availabilityfrom
Bilb:feat/drop-attachment-caption

Conversation

@Bilb

@Bilb Bilb commented Sep 21, 2026

Copy link
Copy Markdown

Opened against the attachment-availability branch of this fork, so the diff is exactly the one commit this PR is about. Merges after that branch does; it targets client in session-foundation as #168.

caption on an attachment was a pure pass-through with no consumer, so this removes it end to end.

What goes

  • OutgoingAttachment::caption and Attachment::caption (include/session/client/attachment.hpp)
  • the caption TEXT column on message_attachments in full_schema.sql, plus a new migration src/client/schema/006_drop_attachment_caption.sql dropping it from existing databases
  • the four statements in src/client/client.cpp that carried it: the attachment read, the outgoing write, the outgoing protobuf build, and the incoming write
  • optional string caption = 11 in AttachmentPointer
  • the incidental assertions in tests/test_client/attachments.cpp (the tests shrink; none are deleted)

Why it is safe

Nothing reads it. It went protobuf in -> column -> struct field -> column -> protobuf out and nowhere else: no client renders it, nothing searches on it, and it has no index, default or foreign key. The conversation-preview query in client.cpp already excludes it on purpose and says so in a comment.

Removing it also takes four positional statements down by one column each, which is four fewer places for a template parameter list, a ? count and a structured binding to drift apart.

Field 11 is reserved, not freed

AttachmentPointer gains its first reserved block:

reserved 11;
reserved "caption";

Old clients on the network keep sending field 11, so the number must never be reused - anything assigned to it later would parse their captions as its own data. This PR deliberately does not touch any other field number.

Protobuf regeneration

proto/SessionProtos.pb.h, proto/SessionProtos.pb.cc and proto/debug_print.cpp are checked in and are not generated at build time, so they are regenerated here with protoc 3.21.12, the version that produced the existing committed files (WebSocketResources.pb.* regenerates byte-identical, confirming the match). Outside the caption accessors the only churn is the expected _has_bits_ renumbering for the fields after 11.

Verification

On top of session-foundation#168 (a8a81514):

Each of the four positional statements was re-derived against the post-session-foundation#168 code rather than replayed, since session-foundation#168 added unavailable and cached to message_attachments and changed the load query. Recounted by hand afterwards: the attachment read is 12 columns / 12 bindings / 12 template parameters, the outgoing write 8 / 8 / 8, the outgoing protobuf build 8 / 8 / 8, and the incoming write 12 / 12 / 12.

One pre-existing failure further back in the schema-history walk is untouched by this branch: a database from 3df4ae52 (v1.8.0-408) does not reach today's schema because of the group-member state CHECK constraints. It reproduces identically on a pristine client build, so it is unrelated to this change and to session-foundation#168.

Nothing consumes a caption.  It arrived on an incoming protobuf pointer,
went into a column, came back out as a struct field, and went back out on
the wire unchanged: no client renders it, nothing searches it, no index or
constraint refers to it, and the conversation-preview query already skips
it deliberately.  A field that only round-trips is one more column to keep
positionally correct in four statements for no behaviour.

Field 11 is reserved rather than deleted: clients on the network keep
sending it, and the number must never be handed to anything else.

The checked-in protobuf output and the generated debug printer are
regenerated to match, with protoc 3.21.12 - the version that produced the
existing files.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant