Ensure summary message is returned in non-streaming mode#416
Open
johnkrussell wants to merge 1 commit intonlweb-ai:mainfrom
Open
Ensure summary message is returned in non-streaming mode#416johnkrussell wants to merge 1 commit intonlweb-ai:mainfrom
johnkrussell wants to merge 1 commit intonlweb-ai:mainfrom
Conversation
Summary messages are not currently served back to the browser when streaming mode is turned off.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix: Ensure summary message is returned in non-streaming mode
Overview
This PR fixes an inconsistency between streaming and non-streaming responses in NLWeb where the summary generated during post-ranking is not returned when
streaming=false.Currently, the summary is only sent via:
This works correctly in streaming mode, but when streaming is disabled, the summary is not included in the final response payload because it is never added to the handler’s message collection.
Change
Adds a call to:
in
post_ranking.pywhen emitting the summary.This ensures the summary is:
handler.messages)Why this is needed
When
streaming=false,/askreturns the accumulated messages fromhandler.messages.However:
This results in inconsistent behaviour:
This PR resolves that inconsistency.
Implementation
Minimal change in:
Before
After
Design considerations
This aligns with NLWeb’s goal of keeping the repo simple and minimally complex.
Testing
Tested manually using:
Before fix
After fix
Also verified:
Notes
This change does not alter how summaries are generated, only ensures they are returned consistently regardless of transport mode.
Checklist
CLA
I understand that this contribution requires agreeing to the Microsoft CLA and will follow the instructions from the CLA bot if prompted.