Skip to content

Improve history sanitization#290

Open
brichet wants to merge 2 commits intojupyterlite:mainfrom
brichet:history-sanitization
Open

Improve history sanitization#290
brichet wants to merge 2 commits intojupyterlite:mainfrom
brichet:history-sanitization

Conversation

@brichet
Copy link
Copy Markdown
Collaborator

@brichet brichet commented Mar 16, 2026

Follow up #284

Currently, the end of the message history is truncated when an unexpected message is detected (e.g. tool-call without result). This fix is applied when an error occurred during stream, including user abort.

This PR change this logic, in favor of including in history only the sanitized message sequence. The whole messages sequence is added to the history (after sanitization) when the response is fully generated.

It is also a workaround for #285: if the stream is stopped, the user message is not added to the history, which avoid continuous responses to an aborted message.

@brichet brichet added the enhancement New feature or request label Mar 16, 2026
@brichet brichet marked this pull request as ready for review March 17, 2026 14:06
* Clears conversation history and resets agent state.
*/
clearHistory(): void {
async clearHistory(): Promise<void> {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Claude found that this change may require updating the following to properly await the function call here:

this._agentManager.clearHistory();

Copy link
Copy Markdown
Collaborator Author

@brichet brichet Mar 17, 2026

Choose a reason for hiding this comment

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

This function is called asynchronously, on click.
AFAIK, awaiting it in the chat model would not prevent the user to send a message... Or we should "lock" the input waiting for it.
Do you think that it worth it ?

}
}
return sanitized;
return sanitized.length === messages.length ? sanitized : [];
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Does this mean all messages are dropped if only one of them is removed during sanitization?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This is actually expected:

  • 'assistant' message and 'tool' messages may be linked by a toolCallId or an approvalId. If one of them is removed, the history may be corrupted. Another solution could be to restart the sanitization, to ensure it did not break anything, but it seems a bit overkill (not sure it'll happen often)
  • if only the 'user' message left, the agent will try to answer it again on next call, so it is probably safer to clean it too for now. But we may handle it in a better way, as @Yahiewi proposed in Improve history sanitization #290 (comment)

@Yahiewi
Copy link
Copy Markdown
Contributor

Yahiewi commented Mar 17, 2026

I haven't run into an error while testing this PR and I like the new logic you've implemented. Thanks for working on this @brichet !
You mentioned this is also a workaround for #285 and I think this only solves part of the problem as, when interrupted, the agent still isn't aware of the fact it got interrupted and the response it started generating.
So I was thinking a follow up to this could be to make a fallback message (for the agent) if the user interrupts the chat instead of just deleting the whole turn.

@brichet
Copy link
Copy Markdown
Collaborator Author

brichet commented Mar 17, 2026

You mentioned this is also a workaround for #285 and I think this only solves part of the problem as, when interrupted, the agent still isn't aware of the fact it got interrupted and the response it started generating.
So I was thinking a follow up to this could be to make a fallback message (for the agent) if the user interrupts the chat instead of just deleting the whole turn.

👍 totally agree, this PR only avoid to respond to a cancelled message endlessly, but it would be nice to instead keep it in history.

@brichet brichet changed the title WIP: improve history sanitization Improve history sanitization Mar 20, 2026
@brichet brichet force-pushed the history-sanitization branch from e5fa0f6 to 3f119f9 Compare March 26, 2026 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants