Skip to content

fix(gateway): no 'possible duplicate send' warning on turns with text streaming off (#111281, #110590, salvage #105446) - #111467

Merged
teknium1 merged 2 commits into
mainfrom
fix/gwdeliver-dup-send-warning
Sep 15, 2026
Merged

fix(gateway): no 'possible duplicate send' warning on turns with text streaming off (#111281, #110590, salvage #105446)#111467
teknium1 merged 2 commits into
mainfrom
fix/gwdeliver-dup-send-warning

Conversation

@teknium1

@teknium1 teknium1 commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator

Gateway turns with text streaming off no longer log a false possible duplicate send warning on every reply.

Fixes #111281
Fixes #110590
Fixes #105341

  • gateway/run_turn_runner.py::_setup_stream_consumer stamps stream_deltas_enabled = want_stream_deltas on the consumer it builds: with streaming.enabled: false (or a per-platform streaming override off) and display.interim_assistant_messages on, the consumer exists only to relay interim commentary and is never fed the final reply's deltas.
  • gateway/run_turn.py::_run_agent_mark_streamed_delivery skips the DUPLICATE-RISK diagnostic for such consumers; a consumer that was fed deltas (the WeCom ack-race case the warning was written for) still warns.
  • gateway/stream_consumer.py::GatewayStreamConsumer carries the flag (default True, so the proxy construction site and every streaming-on path keep the diagnostic).
  • Two invariant tests (tests/gateway/test_interim_only_consumer_warning.py): delta-fed consumer warns; interim-only consumer does not.

Live repro: HERMES_HOME=$(mktemp -d) .venv/bin/python /tmp/batch111/gwdeliver/probe_dup_warning.py <worktree> — builds a REAL GatewayStreamConsumer through TurnRunner._setup_stream_consumer with StreamingConfig(enabled=False) + interim messages on (both reporters' configs), then runs _run_agent_mark_streamed_delivery with a final that was never streamed.

tree consumer built delta callback wired possible duplicate send
origin/main 9326d9cdc True False fired (RED — every turn)
this head True False not fired (GREEN)
this head, StreamingConfig(enabled=True) control True True still fired (ack-race signal kept)

Root cause: the consumer is constructed whenever want_stream_deltas or want_interim_messages, but deltas are only tee'd into it when want_stream_deltas is true, so the elif _sc is not None diagnostic could not tell "consumer built but never fed" from "consumer streamed but lost its delivery confirmation" and warned on both — the constant streamed=False previewed=False content_delivered=False transformed=False fingerprint both reporters measured.

Not taken: #107643 (skip when _accumulated/_message_id/_last_sent_text are empty — reads private consumer state and stays silent for a streamed-then-reset consumer), #110593 (gate on _streamed or _previewed or _content_delivered — those are exactly the conditions under which the earlier branches already suppress, so the warning could never fire), #111296 (gate on _previewed alone — drops the ack-race case with previewed=False).

Credit

Salvages #105446 by @webtecnica (cherry-picked as-is; the third change-detector test was trimmed). Same symptom reported and analysed in #110590 (@KoNit-K's #110593), #111281 (@kvnloo's #111296) and #107619 (@KoNit-K's #107643).

Infographic

gwdeliver-dup-send-warning

webtecnica and others added 2 commits September 14, 2026 19:07
@webtecnica @teknium1
fix(gateway): skip duplicate-send diagnostic for interim-only stream …
7935d53 
…consumers (#105341)
@teknium1
test(gateway): keep the two invariant tests for the interim-only dupl…
0d95a18 
…icate-send diagnostic

Drop the change-detector asserting the default attribute value; the two remaining tests
pin the invariants (a delta-fed consumer keeps the WeCom ack-race warning, an interim-only
consumer does not).
@github-actions

github-actions Bot commented Sep 15, 2026 •
edited
Loading

Copy link
Copy Markdown

૮ >ﻌ< ა ci review

ran on 0d95a18 — test(gateway): keep the two invariant tests for the interim-

⚠️ Warnings

CI timings · View report · View job

Wall time 7m22s vs 5m28s (+34.8%). 9 job(s) slower, 4 faster, 1 unchanged.

  • Python tests / Run tests: +29.0s
  • OS-specific tests / macOS-only tests: +10.0s
  • Profile artifact check / Reject profile archives: +8.0s
  • OSV scan / Scan lockfiles / osv-scan: -7.0s
  • OS-specific tests / Windows-only tests: +7.0s

OSV vulnerability scan · View job

76 known vulnerabilities found in pinned dependencies.

How to fix:

Review the findings in the Security tab. Update the affected dependencies if a patched version is available.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery area/streaming Streaming responses: gateway delivery, provider wire sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Sep 15, 2026
@kyssta-exe

kyssta-exe commented Sep 15, 2026

Copy link
Copy Markdown

Review: fix(gateway) no duplicate-send warning with text streaming off

Summary: Fixes false possible duplicate send warnings every turn when streaming.enabled: false with interim messages on. The stream consumer exists in that config only for interim commentary and is never fed final deltas, but the diagnostic couldn't distinguish that from a lost delivery confirmation. Now the consumer carries a stream_deltas_enabled flag and the diagnostic skips interim-only consumers. All required checks pass.

What changed:

  • gateway/stream_consumer.py: new self.stream_deltas_enabled = True default with comment
  • gateway/run_turn_runner.py::_setup_stream_consumer: stamps stream_deltas_enabled = want_stream_deltas after construction
  • gateway/run_turn.py::_run_agent_mark_streamed_delivery: skips the warning for interim-only consumers via getattr(_sc, "stream_deltas_enabled", True)
  • tests/gateway/test_interim_only_consumer_warning.py (new): delta-fed consumer warns; interim-only does not

Strengths: Thorough root-cause analysis (live repro table base-vs-head, plus explicit rejection of three alternative gates with reasons). getattr(..., True) default keeps proxy/other construction sites and un-pickled mocks safe. Tests cover both the kept ack-race signal and the silenced false positive.

Findings: Two non-blocking polish tips: (1) the test uses SimpleNamespace consumers rather than a real GatewayStreamConsumer, so the _setup_stream_consumer → flag-stamping wiring is exercised only in the external repro script, not in-repo — a small integration test through _setup_stream_consumer would lock the wiring; (2) consider logging at debug (not silence) when skipping for interim-only, so future triage can still see the branch was reached. Note: body carries three Fixes # lines — confirm all three should be closed by this single flag.

Verdict: Looks good to merge.

Reviewed using Hermes-Agent

@webtecnica webtecnica mentioned this pull request Sep 15, 2026
Closed
@teknium1
teknium1 merged commit 676ae4a into main Sep 15, 2026
39 checks passed
@teknium1
teknium1 deleted the fix/gwdeliver-dup-send-warning branch September 15, 2026 11:31
@DavidMetcalfe DavidMetcalfe mentioned this pull request Sep 24, 2026
Open
5 tasks
@KoNit-K KoNit-K mentioned this pull request Sep 30, 2026
Closed
5 tasks done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

area/streaming Streaming responses: gateway delivery, provider wire comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Milestone

No milestone

4 participants

@teknium1 @kyssta-exe @alt-glitch @webtecnica