Skip to content

fix(matrix): omit reply fallback on split message tails - #61210

Open
nepenth wants to merge 1 commit into
NousResearch:mainfrom
nepenth:local/matrix-ship-v2-pr2-long-reply
Open

nepenth wants to merge 1 commit into
NousResearch:mainfrom
nepenth:local/matrix-ship-v2-pr2-long-reply

Conversation

@nepenth

@nepenth nepenth commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Omit Matrix reply-fallback m.in_reply_to on split-message tails.

Main attaches reply fallback to every chunk and test_long_response_split_preserves_thread_context currently requires that. Long replies then produce N quoted bubbles in clients that render fallback. This PR keeps m.thread on every chunk when threaded, and puts reply fallback only on the first chunk by default.

Behavior

  • Default reply_to_mode: first: first chunk may carry m.in_reply_to; later chunks keep m.thread only.
  • reply_to_mode: all: fallback on every chunk (old behavior).
  • reply_to_mode: off (YAML off / false): no fallback on text or media. Thread relations still apply.
  • Invalid values fall back to first.
  • E2EE key-share retry of a chunk preserves that chunk's relation payload.

Tests

python -m pytest tests/gateway/test_matrix.py -q

141 passed on this head. The previous all-chunk in_reply_to assertion is rewritten.

Non-goals

  • Changing chunk size / 16k default.
  • Changing auto-thread policy.
  • Tool activity or approval cards.

Security

No new network or credentials. Relation metadata only.

@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery platform/matrix Matrix adapter (E2EE) sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages P2 Medium — degraded but workaround exists labels Jul 9, 2026

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Verdict: Approved

Looks Good

  • Fixes Matrix reply fallback behavior on split message tails by omitting the fallback when no prior reply context exists
  • Test coverage added
  • Focused, well-scoped fix
  • No security concerns

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for the focused Matrix fix. Current main repeats m.in_reply_to because send() applies identical relation metadata to every truncated chunk (plugins/platforms/matrix/adapter.py:1591-1597), and the thread helper unconditionally supplies that fallback (plugins/platforms/matrix/adapter.py:4010-4022). The PR limits the fallback to the first chunk while preserving m.thread on tails, and adds threaded plus plain-reply split coverage in tests/gateway/test_matrix.py.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 10, 2026
@nepenth
nepenth marked this pull request as ready for review July 11, 2026 10:16
@nepenth
nepenth force-pushed the local/matrix-ship-v2-pr2-long-reply branch 2 times, most recently from 6cea608 to 4eefa54 Compare July 18, 2026 00:27
@nepenth
nepenth force-pushed the local/matrix-ship-v2-pr2-long-reply branch 2 times, most recently from be03749 to f3043b3 Compare July 21, 2026 14:14
@nepenth

nepenth commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

Current head 3198393e8 remains limited to split-reply fallback relations. tests/gateway/test_matrix.py — 253 passed; Ruff, Python compilation, and git diff --check pass. All 36 current-head GitHub checks are complete with no failures. Ready for maintainer review.

@nepenth
nepenth force-pushed the local/matrix-ship-v2-pr2-long-reply branch 2 times, most recently from f8a82bf to 3198393 Compare July 21, 2026 19:25
@nepenth
nepenth force-pushed the local/matrix-ship-v2-pr2-long-reply branch from 3198393 to 6cdba3c Compare August 3, 2026 15:17
@nepenth

nepenth commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current main (6cdba3c98a41).

Multi-chunk Matrix sends now attach m.in_reply_to reply fallback only on the first chunk; later chunks keep m.thread when present so clients do not re-quote the original mid-answer. Updated long-split tests (including fixtures sized from adapter.max_message_length) assert first-only fallback.

Verification: 4 focused long-reply/thread tests passed.

@nepenth
nepenth force-pushed the local/matrix-ship-v2-pr2-long-reply branch from 6cdba3c to 7d5e63f Compare August 7, 2026 01:11
@nepenth
nepenth requested a review from a team August 7, 2026 01:11
@nepenth
nepenth force-pushed the local/matrix-ship-v2-pr2-long-reply branch from 7d5e63f to 61a1e10 Compare August 10, 2026 14:13
@nepenth

nepenth commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

Status — rebuilt on current main (66666f6e2eca)

Single-commit recut. Current head 9a54a2c561de554befc06786dcc5de8746d3088d.

Focused local result on this head: 3 passed (tests/gateway/test_matrix.py -k 'long_response_split or include_reply_fallback or first_chunk').

Main still applies reply-fallback m.in_reply_to to every split chunk and the existing long-split test asserts that. This PR keeps the relation on the first chunk only and rewrites that test.

@nepenth
nepenth force-pushed the local/matrix-ship-v2-pr2-long-reply branch 2 times, most recently from 225ea83 to c9d9166 Compare August 16, 2026 11:08
@nepenth
nepenth force-pushed the local/matrix-ship-v2-pr2-long-reply branch 2 times, most recently from 49c9b89 to e130388 Compare August 21, 2026 11:28
@nepenth
nepenth force-pushed the local/matrix-ship-v2-pr2-long-reply branch 4 times, most recently from 35e8a00 to da03d1a Compare September 5, 2026 00:56
@nepenth

nepenth commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Updated on upstream main 966637323e6f90864e069dbc12755934c2c86387; current head 004e78c8e6e20f45187b1005f9f5dd611a6b9685.

Current main still attaches reply fallback to every split chunk. This rebase keeps first-chunk-only fallback by default, honors reply_to_mode off/all, and leaves threaded tails as bare thread relations. Verified: Matrix 128 passed.

The rewritten PR description is the current scope and verification reference. Local suite results are not a claim of full-repository or live-homeserver validation; current-head CI is tracked separately by the checks below.

@nepenth
nepenth force-pushed the local/matrix-ship-v2-pr2-long-reply branch 2 times, most recently from 004e78c to 1371051 Compare September 12, 2026 11:13
Attach m.in_reply_to only on the first chunk by default (reply_to_mode
first). Later chunks keep m.thread when present. YAML off suppresses
fallback on text and media. Rewrite the main split test that previously
required reply fallback on every chunk.
@nepenth
nepenth force-pushed the local/matrix-ship-v2-pr2-long-reply branch from cca7580 to c20f4bb Compare September 17, 2026 15:51
@nepenth

nepenth commented Sep 17, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current main (36221c6d32).

Behavior unchanged in intent: split Matrix sends attach reply-fallback m.in_reply_to only on the first chunk by default (reply_to_mode: first). Later chunks keep m.thread when present. off suppresses fallback on text and media. The previous all-chunk in_reply_to assertion in test_long_response_split_preserves_thread_context is rewritten to match.

Verification: python -m pytest tests/gateway/test_matrix.py -q — 141 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/matrix Matrix adapter (E2EE) 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 type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants