From 2d6ec5e1e617e0b5417a776d48cde9e892331a7f Mon Sep 17 00:00:00 2001 From: Sam Date: Sat, 24 May 2025 15:19:48 +1000 Subject: [PATCH] FIX: apply diffs more consistently (#1367) * FIX: apply diffs more consistently 1. Do escaping direct in diff streamer, that way HTML tags and other unsafe chars can be displayed and fixed 2. Add safeguard to ensure streaming always stops when it was terminated elsewhere * lint * bug unsubscribe should unsubscribe --- .../discourse/components/modal/diff-modal.gjs | 25 +++++++++++++------ .../discourse/lib/diff-streamer.gjs | 14 ++++++++--- .../modules/ai-helper/common/ai-helper.scss | 2 +- 3 files changed, 29 insertions(+), 12 deletions(-) diff --git a/assets/javascripts/discourse/components/modal/diff-modal.gjs b/assets/javascripts/discourse/components/modal/diff-modal.gjs index 0fb4f262..81f938b1 100644 --- a/assets/javascripts/discourse/components/modal/diff-modal.gjs +++ b/assets/javascripts/discourse/components/modal/diff-modal.gjs @@ -24,8 +24,8 @@ export default class ModalDiffModal extends Component { @tracked loading = false; @tracked finalResult = ""; - @tracked selectedText = escapeExpression(this.args.model.selectedText); - @tracked diffStreamer = new DiffStreamer(this.selectedText); + @tracked escapedSelectedText = escapeExpression(this.args.model.selectedText); + @tracked diffStreamer = new DiffStreamer(this.args.model.selectedText); @tracked suggestion = ""; @tracked smoothStreamer = new SmoothStreamer( @@ -45,11 +45,16 @@ export default class ModalDiffModal extends Component { // Prevents flash by showing the // original text when the diff is empty - return this.selectedText; + return this.escapedSelectedText; } get isStreaming() { - return this.diffStreamer.isStreaming || this.smoothStreamer.isStreaming; + // diffStreamer stops "streaming" when it is finished with a chunk + return ( + this.diffStreamer.isStreaming || + !this.diffStreamer.isDone || + this.smoothStreamer.isStreaming + ); } get primaryBtnLabel() { @@ -71,7 +76,7 @@ export default class ModalDiffModal extends Component { @bind unsubscribe() { const channel = "/discourse-ai/ai-helper/stream_composer_suggestion"; - this.messageBus.subscribe(channel, this.updateResult); + this.messageBus.unsubscribe(channel, this.updateResult); } @action @@ -105,7 +110,7 @@ export default class ModalDiffModal extends Component { data: { location: "composer", mode: this.args.model.mode, - text: this.selectedText, + text: this.args.model.selectedText, custom_prompt: this.args.model.customPromptValue, force_default_locale: true, client_id: this.messageBus.clientId, @@ -122,7 +127,7 @@ export default class ModalDiffModal extends Component { if (this.suggestion) { this.args.model.toolbarEvent.replaceText( - this.selectedText, + this.args.model.selectedText, this.suggestion ); } @@ -131,8 +136,12 @@ export default class ModalDiffModal extends Component { this.finalResult?.length > 0 ? this.finalResult : this.diffStreamer.suggestion; + if (this.args.model.showResultAsDiff && finalResult) { - this.args.model.toolbarEvent.replaceText(this.selectedText, finalResult); + this.args.model.toolbarEvent.replaceText( + this.args.model.selectedText, + finalResult + ); } } diff --git a/assets/javascripts/discourse/lib/diff-streamer.gjs b/assets/javascripts/discourse/lib/diff-streamer.gjs index f02a4be2..42a591f2 100644 --- a/assets/javascripts/discourse/lib/diff-streamer.gjs +++ b/assets/javascripts/discourse/lib/diff-streamer.gjs @@ -2,8 +2,9 @@ import { tracked } from "@glimmer/tracking"; import { cancel, later } from "@ember/runloop"; import loadJSDiff from "discourse/lib/load-js-diff"; import { parseAsync } from "discourse/lib/text"; +import { escapeExpression } from "discourse/lib/utilities"; -const DEFAULT_CHAR_TYPING_DELAY = 30; +const DEFAULT_CHAR_TYPING_DELAY = 10; const STREAMING_DIFF_TRUNCATE_THRESHOLD = 0.1; const STREAMING_DIFF_TRUNCATE_BUFFER = 10; @@ -51,7 +52,7 @@ export default class DiffStreamer { const originalDiff = this.jsDiff.diffWordsWithSpace( this.selectedText, - this.suggestion + newText ); this.diff = this.#formatDiffWithTags(originalDiff, false); return; @@ -172,6 +173,10 @@ export default class DiffStreamer { } async #streamNextChar() { + if (!this.isStreaming || this.isDone) { + return; + } + if (this.currentWordIndex < this.words.length) { const currentToken = this.words[this.currentWordIndex]; @@ -252,6 +257,7 @@ export default class DiffStreamer { return `${text}`; } + // returns an HTML safe diff (escaping all internals) #formatDiffWithTags(diffArray, highlightLastWord = true) { const wordsWithType = []; const output = []; @@ -280,7 +286,8 @@ export default class DiffStreamer { } for (let i = 0; i <= lastWordIndex; i++) { - const { text, type } = wordsWithType[i]; + let { text, type } = wordsWithType[i]; + text = escapeExpression(text); if (/^\s+$/.test(text)) { output.push(text); @@ -310,6 +317,7 @@ export default class DiffStreamer { i++; } + chunkText = escapeExpression(chunkText); output.push(this.#wrapChunk(chunkText, chunkType)); } } diff --git a/assets/stylesheets/modules/ai-helper/common/ai-helper.scss b/assets/stylesheets/modules/ai-helper/common/ai-helper.scss index 5541cd90..d17d592b 100644 --- a/assets/stylesheets/modules/ai-helper/common/ai-helper.scss +++ b/assets/stylesheets/modules/ai-helper/common/ai-helper.scss @@ -647,7 +647,7 @@ .desktop-view & { // a little extra space for extra narrow desktop view - @media screen and (width <= 675px) { + @media screen and (max-width: 675px) { span { display: none; }