fix: deliver plugin-transformed final_response through streaming gate - #29119
fix: deliver plugin-transformed final_response through streaming gate#29119kenyonxu wants to merge 8 commits into
Conversation
|
This PR includes ~3295 Note: PR #29074 (closed) addressed the ACP-only subset of this same issue. This PR is broader (gateway + ACP + conversation_loop), so it's not a strict duplicate, but please verify the ACP portion doesn't conflict with whatever fix landed from #29074. |
…ut hook now visible When streaming is active, streamed_message=True skipped the final_response update, causing plugin hooks like transform_llm_output to be silently invisible. Remove the `not streamed_message` guard so the final response (possibly transformed by plugins) is always delivered to the ACP client.
…s streaming suppression When a transform_llm_output hook modifies final_response after streaming, the gateway was silently discarding the transformed content because streamed=True / content_delivered=True triggered the final-send suppression. Three changes: 1. conversation_loop: set `_response_transformed=True` when a transform_llm_output hook returns a non-empty string, and expose it as `response_transformed` in the result dict. 2. gateway/run: skip the final-send suppression when `response_transformed` is True — the transformed response must reach the client even if streaming already sent the original text. 3. acp_adapter/server: remove `not streamed_message` guard so final_response is always delivered (ACP path fixed separately).
…turn dict run_sync() cherry-picks fields from the run_conversation result dict into a new response dict for the gateway. response_transformed was missing from the cherry-pick list, so the gateway always saw it as False and suppressed the final send even though a transform_llm_output hook had modified the content.
… response_transformed When a transform_llm_output hook appends content after streaming, the previous fix skipped the final-send suppression which caused the full response to be sent as a NEW message (duplicate). Instead, edit the existing streamed message in-place to append the transformed content, then set already_sent=True. Added stream_consumer.message_id and .accumulated_text public properties.
…ng suppression Document the three-layer suppression bug that prevented plugin-transformed final_response content from reaching users on streaming platforms (Discord, Telegram, ACP). Includes the 5-file coordinated fix and data flow diagram.
148c434 to
3dfa713
Compare
|
Thanks for the review!
|
PR #29119 dropped the 'not streamed_message' guard unconditionally so that plugin-transformed responses (transform_llm_output hook) would reach ACP clients. That regressed test_prompt_does_not_duplicate_streamed_final_message: when no transform happened, the streamed text was re-sent as a duplicate final delivery. Tighten the condition to mirror the gateway side: deliver after streaming only when response_transformed=True. Otherwise keep the old guard. Adds test_prompt_delivers_transformed_response_after_streaming so the transformed path stays covered.
…ming Adds a test that fails without the gateway fix, exercising the response_transformed=True branch in _finalize_response: a streamed response whose final text was modified by a transform_llm_output plugin hook must be edit_message'd in place (not duplicate-sent), with already_sent=True so the normal final-send is skipped. Also drops two minor leftovers from the salvaged PR #29119: * accumulated_text property on GatewayStreamConsumer (unused) * duplicate _response_transformed=False inside the hook try block
PR #29119 dropped the 'not streamed_message' guard unconditionally so that plugin-transformed responses (transform_llm_output hook) would reach ACP clients. That regressed test_prompt_does_not_duplicate_streamed_final_message: when no transform happened, the streamed text was re-sent as a duplicate final delivery. Tighten the condition to mirror the gateway side: deliver after streaming only when response_transformed=True. Otherwise keep the old guard. Adds test_prompt_delivers_transformed_response_after_streaming so the transformed path stays covered.
|
Merged via PR #31433. Your 4 substantive commits were cherry-picked onto current main with your authorship preserved in git log — see commits 7eb6c7f, 8edeebe, a4ceead, 5cb21e3. Follow-ups (a regression test + a tightening of the ACP guard so non-transformed streams stay single-delivery) added on top. Thanks for the fix! |
…ming Adds a test that fails without the gateway fix, exercising the response_transformed=True branch in _finalize_response: a streamed response whose final text was modified by a transform_llm_output plugin hook must be edit_message'd in place (not duplicate-sent), with already_sent=True so the normal final-send is skipped. Also drops two minor leftovers from the salvaged PR NousResearch#29119: * accumulated_text property on GatewayStreamConsumer (unused) * duplicate _response_transformed=False inside the hook try block
…ming Adds a test that fails without the gateway fix, exercising the response_transformed=True branch in _finalize_response: a streamed response whose final text was modified by a transform_llm_output plugin hook must be edit_message'd in place (not duplicate-sent), with already_sent=True so the normal final-send is skipped. Also drops two minor leftovers from the salvaged PR NousResearch#29119: * accumulated_text property on GatewayStreamConsumer (unused) * duplicate _response_transformed=False inside the hook try block
PR NousResearch#29119 dropped the 'not streamed_message' guard unconditionally so that plugin-transformed responses (transform_llm_output hook) would reach ACP clients. That regressed test_prompt_does_not_duplicate_streamed_final_message: when no transform happened, the streamed text was re-sent as a duplicate final delivery. Tighten the condition to mirror the gateway side: deliver after streaming only when response_transformed=True. Otherwise keep the old guard. Adds test_prompt_delivers_transformed_response_after_streaming so the transformed path stays covered.
…ming Adds a test that fails without the gateway fix, exercising the response_transformed=True branch in _finalize_response: a streamed response whose final text was modified by a transform_llm_output plugin hook must be edit_message'd in place (not duplicate-sent), with already_sent=True so the normal final-send is skipped. Also drops two minor leftovers from the salvaged PR #29119: * accumulated_text property on GatewayStreamConsumer (unused) * duplicate _response_transformed=False inside the hook try block
PR #29119 dropped the 'not streamed_message' guard unconditionally so that plugin-transformed responses (transform_llm_output hook) would reach ACP clients. That regressed test_prompt_does_not_duplicate_streamed_final_message: when no transform happened, the streamed text was re-sent as a duplicate final delivery. Tighten the condition to mirror the gateway side: deliver after streaming only when response_transformed=True. Otherwise keep the old guard. Adds test_prompt_delivers_transformed_response_after_streaming so the transformed path stays covered.
…ming Adds a test that fails without the gateway fix, exercising the response_transformed=True branch in _finalize_response: a streamed response whose final text was modified by a transform_llm_output plugin hook must be edit_message'd in place (not duplicate-sent), with already_sent=True so the normal final-send is skipped. Also drops two minor leftovers from the salvaged PR NousResearch#29119: * accumulated_text property on GatewayStreamConsumer (unused) * duplicate _response_transformed=False inside the hook try block
PR NousResearch#29119 dropped the 'not streamed_message' guard unconditionally so that plugin-transformed responses (transform_llm_output hook) would reach ACP clients. That regressed test_prompt_does_not_duplicate_streamed_final_message: when no transform happened, the streamed text was re-sent as a duplicate final delivery. Tighten the condition to mirror the gateway side: deliver after streaming only when response_transformed=True. Otherwise keep the old guard. Adds test_prompt_delivers_transformed_response_after_streaming so the transformed path stays covered.
…ming Adds a test that fails without the gateway fix, exercising the response_transformed=True branch in _finalize_response: a streamed response whose final text was modified by a transform_llm_output plugin hook must be edit_message'd in place (not duplicate-sent), with already_sent=True so the normal final-send is skipped. Also drops two minor leftovers from the salvaged PR NousResearch#29119: * accumulated_text property on GatewayStreamConsumer (unused) * duplicate _response_transformed=False inside the hook try block
PR NousResearch#29119 dropped the 'not streamed_message' guard unconditionally so that plugin-transformed responses (transform_llm_output hook) would reach ACP clients. That regressed test_prompt_does_not_duplicate_streamed_final_message: when no transform happened, the streamed text was re-sent as a duplicate final delivery. Tighten the condition to mirror the gateway side: deliver after streaming only when response_transformed=True. Otherwise keep the old guard. Adds test_prompt_delivers_transformed_response_after_streaming so the transformed path stays covered.
PR NousResearch#29119 dropped the 'not streamed_message' guard unconditionally so that plugin-transformed responses (transform_llm_output hook) would reach ACP clients. That regressed test_prompt_does_not_duplicate_streamed_final_message: when no transform happened, the streamed text was re-sent as a duplicate final delivery. Tighten the condition to mirror the gateway side: deliver after streaming only when response_transformed=True. Otherwise keep the old guard. Adds test_prompt_delivers_transformed_response_after_streaming so the transformed path stays covered.
…ming Adds a test that fails without the gateway fix, exercising the response_transformed=True branch in _finalize_response: a streamed response whose final text was modified by a transform_llm_output plugin hook must be edit_message'd in place (not duplicate-sent), with already_sent=True so the normal final-send is skipped. Also drops two minor leftovers from the salvaged PR NousResearch#29119: * accumulated_text property on GatewayStreamConsumer (unused) * duplicate _response_transformed=False inside the hook try block
…ming Adds a test that fails without the gateway fix, exercising the response_transformed=True branch in _finalize_response: a streamed response whose final text was modified by a transform_llm_output plugin hook must be edit_message'd in place (not duplicate-sent), with already_sent=True so the normal final-send is skipped. Also drops two minor leftovers from the salvaged PR NousResearch#29119: * accumulated_text property on GatewayStreamConsumer (unused) * duplicate _response_transformed=False inside the hook try block
PR NousResearch#29119 dropped the 'not streamed_message' guard unconditionally so that plugin-transformed responses (transform_llm_output hook) would reach ACP clients. That regressed test_prompt_does_not_duplicate_streamed_final_message: when no transform happened, the streamed text was re-sent as a duplicate final delivery. Tighten the condition to mirror the gateway side: deliver after streaming only when response_transformed=True. Otherwise keep the old guard. Adds test_prompt_delivers_transformed_response_after_streaming so the transformed path stays covered.
Problem
Plugin hooks registered on
transform_llm_outputmodify the final response AFTER streaming completes. However, three separate suppression points silently discard the transformed content:acp_adapter/server.py):not streamed_messageguard skipsfinal_responsedelivery when streaming was usedgateway/run.py):_streamed or _content_deliveredcheck suppresses the normal final send entirelyrun_sync()(gateway/run.py): cherry-picks fields from therun_conversationresult dict into a new response dict, but omits critical fieldsThe net effect is that any content appended by a
transform_llm_outputhook never reaches end users on streaming platforms (Discord, Telegram, ACP, etc.).Fix (4 coordinated changes)
1.
agent/conversation_loop.pySet
response_transformed=Truewhen atransform_llm_outputhook returns a non-empty string. Expose this asresponse_transformedin the result dict.2.
gateway/run.py—run_sync()return dictAdd
response_transformedto the cherry-picked fields so the flag survives the gateway's response construction.3.
gateway/run.py— suppression checkWhen
response_transformed=True, edit the existing streamed message in-place (via the adapter'sedit_message) with the full transformed content, then setalready_sent=True. This avoids creating a duplicate message while ensuring the transformed content reaches the client.4.
gateway/stream_consumer.pyExpose
message_idandaccumulated_textas public read-only properties so the gateway can perform the in-place edit.5.
acp_adapter/server.pyRemove the
not streamed_messageguard — the final response (possibly transformed by plugins) should always be delivered to the ACP client.Test plan
transform_llm_outputplugin that appends text