Skip to content

fix(matrix): remove forced buffer_only streaming to enable progressive m.replace edits - #58787

Open
Kewe63 wants to merge 2 commits into
NousResearch:mainfrom
Kewe63:fix/58728-matrix-buffer-only-clean2
Open

fix(matrix): remove forced buffer_only streaming to enable progressive m.replace edits#58787
Kewe63 wants to merge 2 commits into
NousResearch:mainfrom
Kewe63:fix/58728-matrix-buffer-only-clean2

Conversation

@Kewe63

@Kewe63 Kewe63 commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Matrix gateway was sending only final/split m.room.message events — zero m.replace (m.relates_to.rel_type == "m.replace") edits, despite streaming being enabled both globally and for the Matrix platform. Long answers arrived as monolithic messages instead of progressive live updates (#58728).

Control tests confirmed the individual pieces worked:

  • Direct Matrix API m.replace edits work in the test room.
  • GatewayStreamConsumer with buffer_only=False produces edits with a fake adapter.

Root Cause

gateway/run.py forced _buffer_only = True exclusively for Matrix at both stream consumer instantiation sites:

1. Proxy/SSE path (~line 16118):

if source.platform == Platform.MATRIX:
    _effective_cursor = ""
    _buffer_only = True          # kills progressive edits

2. Delta/error consumer path (~line 17414):

if source.platform == Platform.MATRIX:
    _effective_cursor = ""
    _buffer_only = True          # same

buffer_only = True makes GatewayStreamConsumer skip the interval/threshold flush path (only got_done / segment_break / commentary trigger edits), effectively making Matrix final/split-only.

This guard was added in #10860 ("make buffered streaming") as a defensive measure during the E2EE/migration fix batch.


Fix

Remove both _buffer_only = True lines. The _buffer_only default (False) already set above remains active, so Matrix now receives progressive m.replace edits like other platforms (Discord, Slack, etc.). Cursor suppression (_effective_cursor = "") is preserved — some Matrix clients render the stream cursor as a tofu/white-box artifact.


How to Test

pytest tests/gateway/test_stream_consumer.py
# ✅ 97 passed

pytest tests/gateway/test_matrix.py::test_edit_payload_uses_m_replace
# ✅ 1 passed — confirms Matrix adapter edit_message produces correct m.replace payloads

Checklist

  • Tests pass — 97/97 stream consumer, 1/1 targeted matrix test
  • Follows Conventional Commits
  • Changes scoped to this fix only
  • Cursor suppression preserved (_effective_cursor = "")

Risk & Impact

Low. Two-line removal — the _buffer_only default (False) was already set above both removed lines. No logic is added; the guard suppressing progressive edits is simply removed. All other platforms are unaffected.

Type: 🐛 Bug fix
Related: #41090 / #37931 / #49815 / #57091 (Matrix streaming delivery family, different mechanisms)
Fixes: #58728

…e m.replace edits

Matrix gateway was forcing buffer_only=True for all Matrix platform
streaming, which silently disabled progressive m.replace edits.
The consumer only delivered final/split messages, never mid-stream
edits — even though direct Matrix API m.replace calls and the
GatewayStreamConsumer with buffer_only=False (fake adapter)
both work correctly.

The buffer_only guard was added in NousResearch#10860 ('make buffered streaming')
as a defensive measure during the E2EE/migration fix batch.

This change removes the forced buffer_only=True from both
stream consumer instantiation sites (proxy path and delta/error
consumer path), reverting Matrix to progressive edit streaming
like other platforms (Discord, Slack, etc.).

Cursor suppression (_effective_cursor='') is preserved to avoid
the visible tofu/white-box artifact on some Matrix clients.

Fixes NousResearch#58728
@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 5, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Clean resubmission of #58770 (closed whole-fork push) — this version isolates the fix to a single 2-line change in gateway/run.py. Addresses #58728 (Matrix sends final/split messages but no m.replace edits). Not a duplicate of the closed #58770; related to it and to #41090 (opt-in matrix_progressive, same area, distinct mechanism).

…meter

The stream consumer now passes finalize= to adapter.edit_message(),
but the test adapters didn't accept this parameter. Fix both
ProgressCaptureAdapter and SmallLimitProgressAdapter to accept
finalize: bool = False for CI compatibility.

Related to fix/58728-matrix-buffer-only-clean2
@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 15, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for isolating both current Matrix stream-consumer sites. The premise is confirmed on current main: gateway/run.py:17028 and gateway/run.py:18362 force buffer_only=True, while gateway/stream_consumer.py:630-639 only permits interval/threshold progressive updates when that flag is false. The Matrix adapter supports the intended mechanism through m.replace in plugins/platforms/matrix/adapter.py:1718-1738.

Problems

  • tests/gateway/test_run_progress_topics.py:1012-1016 does not assert a progressive edit. It accepts final-only delivery, so it passes with the current broken buffer_only=True behavior.
  • The PR removes guards in both the proxy/SSE and _run_agent paths, but the existing test only exercises _run_agent.

Suggested changes

  • Assert that the Matrix fixture records at least one non-finalizing edit before completion, in addition to the existing cursor check.
  • Add proxy/SSE-path coverage, or centralize and test the shared Matrix consumer configuration.

Automated hermes-sweeper review.

return SendResult(success=True, message_id="progress-1")

async def edit_message(self, chat_id, message_id, content) -> SendResult:
async def edit_message(self, chat_id, message_id, content, finalize: bool = False) -> SendResult:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Recording finalize is useful, but please assert it in test_run_agent_matrix_streaming_omits_cursor: require at least one adapter.edits entry with finalize is False. The current assertions accept the existing buffer-only final-only behavior, so they do not regress this fix.

@ZeroOttantotto

Copy link
Copy Markdown

I had opened #66343 (now closed as duplicate) with the same fix plus a regression test — offering it here in case you want to include it.

The test verifies that with buffer_only=False, the GatewayStreamConsumer calls edit_message() as new tokens arrive (progressive edits), catching any future regression that would re-enable buffer-only for Matrix.

Regression test (click to expand)
class TestMatrixProgressiveStreaming:
    """Matrix streaming should use progressive edits, not buffer-only."""

    @pytest.mark.asyncio
    async def test_progressive_streaming_not_buffer_only(self):
        """Regression: buffer_only=True was hardcoded for Matrix in gateway/run.py.

        With buffer_only=False, the consumer sends the initial message then
        progressively calls edit_message() as new tokens arrive.
        """
        from types import SimpleNamespace
        from gateway.stream_consumer import GatewayStreamConsumer, StreamConsumerConfig

        adapter = MagicMock()
        send_result = SimpleNamespace(success=True, message_id="m1")
        edit_result = SimpleNamespace(success=True, message_id="m1")
        adapter.send = AsyncMock(return_value=send_result)
        adapter.edit_message = AsyncMock(return_value=edit_result)
        adapter.SUPPORTS_MESSAGE_EDITING = True
        adapter.MAX_MESSAGE_LENGTH = 4096

        cfg = StreamConsumerConfig(
            edit_interval=0.01,
            buffer_threshold=5,
            cursor="",           # Matrix: no cursor (tofu)
            buffer_only=False,   # Matrix: progressive edits enabled
        )
        consumer = GatewayStreamConsumer(
            adapter=adapter,
            chat_id="!test:matrix.local",
            config=cfg,
        )

        # Feed the first batch of tokens — enough to cross buffer_threshold
        # so the consumer sends the initial message.
        consumer.on_delta("Ciao")
        consumer.on_delta(" mondo")

        # Start the consumer in background so we can feed more tokens
        # while it runs its edit loop.
        run_task = asyncio.create_task(consumer.run())

        # Wait for the initial send to fire
        await asyncio.sleep(0.15)

        # Feed more tokens — this should trigger progressive edits
        consumer.on_delta("! Come")
        consumer.on_delta(" stai?")

        # Wait for edits to fire
        await asyncio.sleep(0.15)

        # Finish the stream
        consumer.finish()

        # Wait for the consumer to complete
        await asyncio.wait_for(run_task, timeout=2.0)

        # Must have sent initial message then edited
        assert adapter.send.called, "initial send() should have fired"
        assert adapter.edit_message.called, (
            "edit_message() must be called for progressive streaming; "
            "buffer_only=True would suppress this"
        )

Feel free to cherry-pick or adapt however you like. No pressure — just didn't want it to go to waste.

@teknium1 teknium1 added the area/streaming Streaming responses: gateway delivery, provider wire label Jul 19, 2026
@xz-dev

xz-dev commented Aug 1, 2026

Copy link
Copy Markdown

Thank you @Kewe63 for identifying the root cause and proposing the original runtime fix here.

I opened #75776 as a current-main adaptation, with this PR explicitly credited in the description. Since #58787 was opened, the two stream-consumer setup sites have been centralized in _build_stream_consumer_config(), so the runtime change is now a one-line removal in that shared helper and applies to both paths.

The new version also addresses the review feedback on this PR:

  • it records the finalize flag;
  • asserts at least one real finalize=False progressive edit before completion;
  • uses deterministic event synchronization instead of sleep-only timing;
  • was verified by 184 focused tests;
  • was exercised against a real Matrix/Element room using a deterministic local producer (no LLM), producing one initial message, four progressive m.replace edits, and one finalizing edit, all targeting the same original event.

New PR: #75776

@alt-glitch alt-glitch added the duplicate This issue or pull request already exists label Aug 1, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Superseded by #75776: current main centralizes the two former construction paths behind one helper, and #75776 removes its sole Matrix buffer_only override with end-to-end regression coverage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/streaming Streaming responses: gateway delivery, provider wire comp/gateway Gateway runner, session dispatch, delivery duplicate This issue or pull request already exists 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-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.

5 participants